Repository navigation
Fix Linear integration setup and docs - #18
Conversation
|
WalkthroughAdds first-class Linear integration across broker, CLI, DB, models, and macOS: Linear OAuth flows and deep links, connection persistence and local-access seeding, project binding and autosync subscriptions, ticket sync (pull/push), conflict recording/review, macOS UI for account/settings/tickets, many model and API additions, and supporting docs/plans. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant macOS as rgba(66,133,244,0.5) macOS App
participant Browser as rgba(244,180,0,0.5) Browser
participant Broker as rgba(52,168,83,0.5) Auth Broker
participant Linear as rgba(153,51,255,0.5) Linear API
participant DB as rgba(120,120,120,0.5) Broker DB
participant Local as rgba(200,100,50,0.5) Local Keychain
User->>macOS: Start Linear OAuth flow
macOS->>Broker: POST /v1/auth/linear/start
Broker->>DB: create flow record (flowId, PKCE)
Broker->>macOS: return authorize_url + flowId
macOS->>Browser: Open authorize_url
User->>Linear: Approve app
Linear->>Broker: Callback (code,state)
Broker->>Linear: Exchange code -> tokens
Broker->>DB: persist linear_connections (sealed/encrypted)
Broker->>macOS: deep-link redirect (hack://.../complete?flowId=)
macOS->>Broker: GET /v1/auth/linear/status?flowId=
Broker->>DB: claim flow, issue managementToken
Broker->>macOS: return profile + token
macOS->>Local: Store profile in keychain
macOS->>macOS: Update UI (linearOAuthDeepLinkContext)
sequenceDiagram
participant macOS as rgba(66,133,244,0.5) macOS App
participant Broker as rgba(52,168,83,0.5) Auth Broker
participant Linear as rgba(153,51,255,0.5) Linear API
participant DB as rgba(120,120,120,0.5) Broker DB
participant Local as rgba(200,100,50,0.5) Local Ticket Store
macOS->>Broker: Request sync / run autosync
Broker->>DB: read linear_sync_subscriptions
Broker->>Linear: fetch issues / apply mutations
Linear->>Broker: return issues or mutation results
Broker->>Local: merge issues, create checkpoints, record conflicts
Broker->>DB: persist webhook events / sync metadata
Broker->>macOS: stream sync progress / conflicts
macOS->>User: show review queue / conflicts
User->>macOS: resolve conflict
macOS->>Broker: resolveTicketConflict API
Broker->>Local: apply resolution, persist audit
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
services/auth-broker/src/better-auth-link.ts (1)
67-105:⚠️ Potential issue | 🟠 MajorGate email-based user lookup and provisioning on verified-email signal.
The
OAuthFlowAccounttype includes anaccountEmailVerifiedfield, butresolveBetterAuthUserFromOAuthAccount(lines 67–105) never checks it before usingaccountEmailto find or create users. This allows unverified email addresses from providers like Linear to link or auto-provision accounts, creating an ownership-binding vulnerability.Add a check like
if (!input.account.accountEmailVerified) { return { state: "unverified_email" }; }before the email lookup at line 85, or ensure all provider flows guaranteeaccountEmailVerifiedistruewhenaccountEmailis populated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/auth-broker/src/better-auth-link.ts` around lines 67 - 105, In resolveBetterAuthUserFromOAuthAccount, gate the email-based lookup/provisioning on the accountEmailVerified flag: before calling findUserIdByEmail or createBetterAuthUserByEmail, check input.account.accountEmailVerified and return { state: "unverified_email" } (or equivalent) when false; this ensures normalizeText(input.account.accountEmail) is not used for linking/provisioning unless accountEmailVerified is true and prevents unverified-email association when input.autoProvision is used.apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/TicketsView.swift (1)
1836-1854:⚠️ Potential issue | 🟠 MajorGuard
ticketDetailagainst stale async responses.Switching selection quickly can let an older
showTicketcall win and replace the inspector with the wrong ticket. Capture the requested ID and ignore late success/error paths onceselectedTicketIdchanges.Suggested fix
private func loadTicketDetail() async { guard let selectedTicketId else { ticketDetail = nil detailErrorMessage = nil isDetailLoading = false return } + let requestedTicketId = selectedTicketId isDetailLoading = true detailErrorMessage = nil isHistoryExpanded = false isPropertiesExpanded = false do { - let detail = try await model.showTicket(for: project, ticketId: selectedTicketId) - ticketDetail = detail - ticketDetailCache[selectedTicketId] = detail + let detail = try await model.showTicket(for: project, ticketId: requestedTicketId) + guard requestedTicketId == self.selectedTicketId else { return } + ticketDetail = detail + ticketDetailCache[requestedTicketId] = detail } catch { + guard requestedTicketId == self.selectedTicketId else { return } detailErrorMessage = error.localizedDescription } isDetailLoading = false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/TicketsView.swift` around lines 1836 - 1854, In loadTicketDetail(), prevent stale async responses from overwriting the current inspector by capturing the current selectedTicketId into a local let (e.g. requestedId) before calling await, then after each await/try/catch return path check that selectedTicketId == requestedId before assigning ticketDetail, ticketDetailCache[requestedId], detailErrorMessage or changing isDetailLoading; if they differ, ignore the result (and ensure isDetailLoading is cleared only when the current request completes for the matching id). This ensures older showTicket(for:ticketId:) completions are discarded when selection changes.apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/ProjectDetailView.swift (1)
2223-2382:⚠️ Potential issue | 🟠 MajorCanceled reloads can still overwrite newer state.
queueExecutionTargetReload()cancels the previous task, but after Line 2293 this method still has more suspension points (seedLinearLocalAccess,loadLinearRoutingProjectOptionsIfNeeded,loadLinearAutosyncRouteKeys) before the final state writes. If an older reload resumes after a newer one, it can publish stale project/profile data over the current selection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/ProjectDetailView.swift` around lines 2223 - 2382, The reloadExecutionTargetState() can resume after long suspensions and overwrite newer state; to fix, record a local reload token (e.g. let currentReloadId = UUID()) or capture Task.isCancelled checkpoints and validate it again immediately before any further suspension and right before the final state writes; specifically, in reloadExecutionTargetState() check Task.isCancelled (or compare the saved token against a shared/latest token) after calling/awaiting seedLinearLocalAccess, after await loadLinearRoutingProjectOptionsIfNeeded(), and after await loadLinearAutosyncRouteKeys(targets:), and return early if cancelled/mismatched so stale resumption cannot clobber executionTargetNodes, githubResolvedProfile, linearResolvedProfile, executionMode, etc.
🟠 Major comments (26)
services/auth-broker/README.md-248-265 (1)
248-265:⚠️ Potential issue | 🟠 MajorClarify the default linking policy vs. the auto-provision override.
This section reads as an absolute rule, but the env docs later say the broker can create a Better Auth user when no existing match exists. In an auth README, that contradiction is risky: operators could assume unmatched-provider sign-in is always refused when those flags actually relax it.
Please qualify lines 252-263 as the default behavior, then explicitly call out how
BETTER_AUTH_GITHUB_AUTO_PROVISION_USERSandBETTER_AUTH_LINEAR_AUTO_PROVISION_USERSchange that outcome.Also applies to: 302-303
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/auth-broker/README.md` around lines 248 - 265, Update the "Verified-email account linking" block to state these checks (provider returns email, email verified, normalized email matches existing Hack user; refused when missing/unverified/mismatch) are the default linking policy, and add a clear sentence that the environment flags BETTER_AUTH_GITHUB_AUTO_PROVISION_USERS and BETTER_AUTH_LINEAR_AUTO_PROVISION_USERS override this default by allowing creation of a Better Auth user when no existing Hack match exists; also apply the same clarification to the similar note around lines 302-303 so both places explicitly document the auto-provision override behavior.docs/plans/2026-03-09-linear-autosync-tickets-design.md-37-39 (1)
37-39:⚠️ Potential issue | 🟠 MajorClarify webhook fallback behavior and user visibility.
The design mentions "webhook-backed autosync where possible" with "periodic reconcile remains as a fallback/backstop" but doesn't specify:
- What conditions trigger the fallback to periodic reconcile?
- How will users know if webhooks are failing and the system has fallen back?
- What is the reconciliation interval?
This ambiguity could lead to inconsistent implementation or users being unaware of sync delays.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-03-09-linear-autosync-tickets-design.md` around lines 37 - 39, Update the "Sync from Linear" section to explicitly define the fallback conditions, user-facing visibility, and interval: state the exact failure conditions that trigger fallback from "webhook-backed autosync" to "periodic reconcile" (e.g., webhook delivery errors, repeated webhook verification failures, or detector thresholds), describe how and where users will be notified or can view status (e.g., UI banner, sync status page, admin logs, and alerts tied to "webhook-backed autosync" failures), and specify the concrete "periodic reconcile" cadence (e.g., every X minutes/hours) and any backoff/ retry behavior; reference the existing phrases "webhook-backed autosync", "periodic reconcile", and the "Sync from Linear" heading so the clarifications appear in that paragraph.docs/plans/2026-03-09-linear-autosync-tickets-design.md-57-62 (1)
57-62:⚠️ Potential issue | 🟠 MajorSpecify where manual repair actions will be located.
The design lists three possible locations for manual sync/repair ("such as"):
- Tickets overflow menu
- Project advanced settings
- CLI-only repair flows
The use of "such as" creates ambiguity. Specify which surface(s) will actually expose manual repair, as this affects both implementation effort and user discoverability during recovery scenarios.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-03-09-linear-autosync-tickets-design.md` around lines 57 - 62, Replace the ambiguous "such as" phrasing with a definitive list of where manual repair actions will be exposed: explicitly state which of the three candidate surfaces will host manual sync (e.g., "Tickets overflow menu" and "Project advanced settings" for UI recovery, and "CLI-only repair flows" for advanced/automation usage), update the sentence to remove ambiguity and reflect the chosen surfaces, and ensure the same wording appears in the "Manual sync remains available" paragraph and any related headings so consumers can implement the UI and CLI changes accordingly.docs/plans/2026-03-09-linear-autosync-tickets-design.md-44-47 (1)
44-47:⚠️ Potential issue | 🟠 MajorSpecify deduplication and conflict handling for auto-ingested tickets.
The design states that Linear-origin tickets "auto-ingest into Hack for the bound project" but doesn't address:
- How does the system handle deduplication if a ticket with the same Linear ID already exists locally?
- What happens if a local Hack-origin ticket has metadata that conflicts with an incoming Linear ticket?
- Are there any edge cases where auto-ingest should be blocked or require manual review?
These details are essential for implementers to build a robust sync system.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-03-09-linear-autosync-tickets-design.md` around lines 44 - 47, Update the design to explicitly define deduplication and conflict-resolution rules for auto-ingested tickets: state that ingestion should first match incoming Linear tickets against local tickets by Linear ID (if found, treat as update rather than new ingest), describe merge semantics for conflicting fields (e.g., preserve field ownership: Linear-owned fields overwrite local values, Hack-owned fields retain local values, and for shared fields use last-write-wins or field-level merge with timestamps), explain how to merge or append comments (append-only behavior preserves both comment streams with source metadata), and enumerate edge conditions that block auto-ingest and require manual review (e.g., same Linear ID but mismatched project/binding, divergent ownership metadata, conflicting status transitions, or missing trustable timestamps), referencing "Linear ID", "auto-ingest", "Hack-origin", "bound project", and the current "append-only comment rules" so implementers can locate where to apply these rules.docs/plans/2026-03-09-linear-autosync-tickets-design.md-143-143 (1)
143-143:⚠️ Potential issue | 🟠 MajorDefine "bounded and stale-safe" caching requirements.
The statement "Ticket list caching should remain bounded and stale-safe" uses undefined terms. Specify:
- What are the bounds? (e.g., max cache size, time-to-live)
- What makes the cache "stale-safe"? (e.g., validation strategy, invalidation triggers)
Clear definitions ensure consistent implementation and prevent performance or correctness issues.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-03-09-linear-autosync-tickets-design.md` at line 143, Replace the vague sentence "Ticket list caching should remain bounded and stale-safe" with explicit caching requirements: state the concrete bounds (e.g., max entries or memory limit and TTL like "max 10k entries or 100MB, TTL 5m"), define eviction policy (LRU or size-based) and refresh strategy, and list what "stale-safe" means (e.g., validation-on-access with ETag/last-modified checks, background refresh, and invalidation triggers such as ticket mutations, webhook/CDC events, or manual purge). Also specify consistency expectations (eventual vs. strong) and failure behavior (serve stale on backend error or return miss) so implementers can follow the exact rules referenced by "Ticket list caching" in the design.docs/plans/2026-03-09-linear-autosync-tickets-design.md-151-156 (1)
151-156:⚠️ Potential issue | 🟠 MajorEnsure validation steps cover all identified risks.
The Risks section identifies important concerns, but the Validation section doesn't fully address them:
- Line 154: Risk of "surprising users with project-wide noise" when auto-create is enabled → No corresponding validation step to verify appropriate warnings or gradual rollout
- Line 155: Risk that "Density changes must preserve scannability and keyboard navigation" → No accessibility or keyboard navigation validation step
Add validation criteria for these risks to ensure they're properly mitigated.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-03-09-linear-autosync-tickets-design.md` around lines 151 - 156, Update the Validation section to explicitly cover the Risks entries: add a validation criterion that verifies auto-create (Hack-to-Linear) behavior includes clear warning copy, an opt-in/gradual rollout plan, and telemetry/rollback hooks to prevent project-wide surprising noise (referencing "auto-create" / "Hack-to-Linear"); add accessibility and keyboard-navigation validation for "density changes" that tests scannability, focus order, keyboard shortcuts, and screen-reader semantics; and add a validation step ensuring manual sync remains discoverable and repairable (referencing "manual sync") with tests for visibility and recoverability under different autosync states.scripts/install-cli.ts-178-184 (1)
178-184:⚠️ Potential issue | 🟠 MajorUse POSIX-safe quoting for wrapper paths.
shellQuote()produces double-quoted strings, but bash still expands$VAR,$(...), and backticks inside double quotes. If either the Bun executable path or repo path contains these characters, invoking the installed wrapper can execute unintended commands or fail. Switch to POSIX-safe single-quoted literals instead.Suggested fix
- `exec ${shellQuote(bunExecutable)} ${shellQuote(resolve(opts.repoRoot, "index.ts"))} "$@"`, + `exec ${shellQuotePosix(bunExecutable)} ${shellQuotePosix(resolve(opts.repoRoot, "index.ts"))} "$@"`,function shellQuotePosix(value: string): string { return `'${value.replaceAll("'", "'\\''")}'` }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/install-cli.ts` around lines 178 - 184, The wrapper currently uses shellQuote() which emits double-quoted strings and allows shell expansions; replace it with a POSIX-safe single-quote quoting helper (e.g., shellQuotePosix that escapes single quotes by replacing ' with '\'' ) and use that when composing the exec line in the install-cli generation (where resolveBunExecutablePath() and resolve(opts.repoRoot, "index.ts") are used) so the generated line becomes exec <single-quoted bun path> <single-quoted repo/index.ts> "$@". Ensure you add the new shellQuotePosix function and swap its usage for both the bunExecutable and resolved index.ts path.services/auth-broker/src/modules/better-auth/management-token.ts-143-148 (1)
143-148:⚠️ Potential issue | 🟠 MajorUse separate key material for management tokens.
Falling back to
BETTER_AUTH_SECRET/AUTH_SECRETmakes this a second HMAC token protocol on the same root auth secret. That widens blast radius and prevents independent rotation; a dedicated management-token secret or a derived subkey is safer here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/auth-broker/src/modules/better-auth/management-token.ts` around lines 143 - 148, Replace the current fallback to general auth secrets in resolveManagementTokenSecret: stop returning BETTER_AUTH_SECRET/AUTH_SECRET for management tokens and instead read a dedicated secret (e.g., BETTER_AUTH_MANAGEMENT_SECRET) via normalizeOptionalString; if you must derive from the root secret, perform a deterministic key-derivation (HKDF/Argon2id with a fixed context/salt like "management-token" and adequate length) inside resolveManagementTokenSecret so the management key is a separate subkey (document the chosen env var name and remove the fallback to BETTER_AUTH_SECRET/AUTH_SECRET to ensure independent rotation and reduced blast radius).services/auth-broker/src/modules/better-auth/management-token.ts-27-28 (1)
27-28:⚠️ Potential issue | 🟠 MajorValidate numeric override inputs before using them in expiry calculations.
The
nowMsandexpiresInMsinputs currently accept anynumber, allowing invalid values likeNaNorInfinityto break both sides of this API:
- On issuance: Invalid overrides cause
expto becomeNaN, which throws "Invalid time value" at line 73 when callingtoISOString().- On verification:
nowMs: NaNcausesnowSecondsto beNaN, making the expiry check at line 131 (parsedClaims.exp <= NaN) always evaluate tofalse, so expired tokens are incorrectly verified as valid.The
readPositiveIntegerhelper already exists (lines 226–229) and is used to validate incoming claims; reuse it for input overrides.🛠️ Minimal fix
- const nowMs = input.nowMs ?? Date.now(); - const expiresInMs = Math.max( - input.expiresInMs ?? DEFAULT_MANAGEMENT_TOKEN_TTL_MS, - 60_000 - ); + const nowMs = readPositiveInteger(input.nowMs) ?? Date.now(); + const requestedExpiresInMs = + readPositiveInteger(input.expiresInMs) ?? DEFAULT_MANAGEMENT_TOKEN_TTL_MS; + const expiresInMs = Math.max(requestedExpiresInMs, 60_000); … - const nowSeconds = Math.floor((input.nowMs ?? Date.now()) / 1000); + const nowSeconds = Math.floor( + (readPositiveInteger(input.nowMs) ?? Date.now()) / 1000 + );Also applies to: 35-39, 73, 83, 130-136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/auth-broker/src/modules/better-auth/management-token.ts` around lines 27 - 28, Validate the numeric override inputs nowMs and expiresInMs with the existing readPositiveInteger helper before using them: in the token issuance flow (where expiresInMs and nowMs are used to compute exp and toISOString() is called) call readPositiveInteger on those overrides and treat invalid results as undefined so you never compute exp with NaN/Infinity; likewise in the verification flow (where nowMs is converted to nowSeconds and compared to parsedClaims.exp) validate nowMs using readPositiveInteger and fall back to the real current time if invalid so comparisons like parsedClaims.exp <= nowSeconds behave correctly. Ensure you update the code paths that construct claims (using expiresInMs) and the expiry check (using nowMs) to use the validated values returned by readPositiveInteger.services/auth-broker/src/config.ts-159-203 (1)
159-203:⚠️ Potential issue | 🟠 MajorThe new root-env fallback never reaches the GitHub config.
readFirstEnv()is used for Linear and the provider token key, butresolveGitHubConfig()still reads everyGITHUB_*setting straight fromprocess.env. In the repo-root.env/.env.locallayout this patch is introducing, the broker can still fail to boot becauseGITHUB_CLIENT_IDandGITHUB_CLIENT_SECRETnever consult the fallback reader. Please route the GitHub reads throughreadFirstEnv()too, or makenormalizeRequiredEnv()fallback-aware.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/auth-broker/src/config.ts` around lines 159 - 203, resolveGitHubConfig currently reads GITHUB_* values directly from process.env so the new root-env fallback (readFirstEnv) is never consulted; update resolveGitHubConfig to call readFirstEnv("GITHUB_CLIENT_ID"), readFirstEnv("GITHUB_CLIENT_SECRET") and use readFirstEnv for other GITHUB_* lookups (or alter normalizeRequiredEnv to internally call readFirstEnv) so all github values (githubClientId, githubClientSecret, githubAppSlug, githubAppInstallUrl, githubScopes, githubAuthorizeUrl, githubTokenUrl, githubApiBaseUrl, githubRedirectUri) will honor the root-env fallback; reference resolveGitHubConfig and normalizeRequiredEnv/readFirstEnv when making the change.services/auth-broker/src/db/ensure-columns.ts-35-48 (1)
35-48:⚠️ Potential issue | 🟠 MajorMake the DDL itself idempotent.
This helper does a read-then-alter sequence. Two broker instances can both observe the same column as missing, and one will then fail on
ALTER TABLE ... ADD COLUMNduring startup. Since this is a runtime fail-safe, the SQL should useIF NOT EXISTSas well.Suggested change
for (const column of input.columns) { if (existingColumns.has(column.name)) { continue; } await input.db.execute( sql.raw( - `ALTER TABLE ${quoteIdentifier(input.tableName)} ADD COLUMN ${quoteIdentifier(column.name)} ${column.definition}` + `ALTER TABLE ${quoteIdentifier(input.tableName)} ADD COLUMN IF NOT EXISTS ${quoteIdentifier(column.name)} ${column.definition}` ) ); + existingColumns.add(column.name); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/auth-broker/src/db/ensure-columns.ts` around lines 35 - 48, The current read-then-alter in ensure-columns.ts can race across instances because ALTER TABLE ... ADD COLUMN fails if another process added the column first; update the raw SQL issued in the loop (the call to input.db.execute / sql.raw that builds `ALTER TABLE ${quoteIdentifier(input.tableName)} ADD COLUMN ${quoteIdentifier(column.name)} ${column.definition}`) to include an idempotent clause (use `IF NOT EXISTS` for adding the column) so the DDL is safe under concurrent starts; keep using quoteIdentifier and column.definition but change the statement text to use `ADD COLUMN IF NOT EXISTS`.apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/DashboardView.swift-578-583 (1)
578-583:⚠️ Potential issue | 🟠 MajorWhitelist browser-safe schemes before opening
accountURL.This currently opens whatever scheme comes back in
state.accountURL. A malformed or compromised value could trigger local app/file handlers instead of a browser page. Restrict this tohttps/httpbefore callingNSWorkspace.shared.open.Suggested fix
- if let accountURL = state.accountURL, let url = URL(string: accountURL) { + if + let accountURL = state.accountURL, + let url = URL(string: accountURL), + let scheme = url.scheme?.lowercased(), + scheme == "https" || scheme == "http" + { Button { NSWorkspace.shared.open(url) } label: { Label("Manage account in browser", systemImage: "globe") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/DashboardView.swift` around lines 578 - 583, The code currently opens state.accountURL directly via NSWorkspace.shared.open in DashboardView; update the Button action to first validate the URL scheme by creating the URL (as already done) and then checking url.scheme?.lowercased() is "http" or "https" before calling NSWorkspace.shared.open. If the scheme is not allowed, skip opening (or optionally log/handle the invalid-scheme case). Use the existing URL instance and a simple whitelist check on its scheme to ensure only browser-safe schemes are opened.services/auth-broker/src/modules/better-auth/plugin.ts-48-56 (1)
48-56:⚠️ Potential issue | 🟠 MajorDon't expose raw storage errors from
/api/auth/*.This public route now echoes the underlying readiness exception text back to the client. If the storage layer throws a DB/path/config error, that detail becomes externally visible. Return a generic message here and keep the original error in server-side logs instead.
Suggested fix
} catch (error) { set.status = 503; return { ok: false, error: "better_auth_storage_unavailable", - message: error instanceof Error ? error.message : String(error), + message: "Better Auth storage is temporarily unavailable.", } as const; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/auth-broker/src/modules/better-auth/plugin.ts` around lines 48 - 56, The handler currently returns raw storage error text when ensureBetterAuthRuntimeReady(runtime) throws; change it to log the original error server-side (e.g., using console.error or the existing logger) and return a generic client-facing response instead: keep setting set.status = 503 and return the same shape with error: "better_auth_storage_unavailable" but replace message with a generic string like "service unavailable" (or similar), ensuring the original error object from the catch block is only written to server logs and not included in the response.docs/plans/2026-03-07-linear-local-access-seeding-design.md-98-113 (1)
98-113:⚠️ Potential issue | 🟠 MajorProtect remote token custody from stale overwrites.
The write-back flow is last-write-wins right now. If two Macs rotate credentials around the same time, an older client can overwrite a newer refresh token and break later reseeds. Add a monotonic
lastUpdated/version check or other compare-and-swap guard to the broker update path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-03-07-linear-local-access-seeding-design.md` around lines 98 - 113, The broker write-back endpoint that updates the stored encrypted token envelope (the protected broker endpoint handling the Linear callback and updating the Hack-owned connection row / encrypted token envelope) currently uses last-write-wins; add a monotonic lastUpdated timestamp or integer version on the connection row and implement a compare-and-swap guard in the broker update logic so updates only succeed when the incoming lastUpdated/version is strictly newer than the stored value; ensure the CLI/local flow that calls saveLinearToken() and the local refresh path include the new lastUpdated/version in their update requests and handle CAS failures (return a conflict) so stale clients cannot overwrite newer envelopes.apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/SettingsOverlayView.swift-3225-3231 (1)
3225-3231:⚠️ Potential issue | 🟠 MajorRefresh saved mappings when the team scope changes.
refreshLinearAssigneeMappings()is only triggered whenassigneeMappingProfilechanges. Editing the team scope leaves “Saved mappings” showing the previous scope, soUse/Removecan target the wrong rows. Add the same reload hook forassigneeMappingTeamId.Suggested fix
.onChange(of: assigneeMappingProfile) { _, _ in Task { await refreshLinearAssigneeMappings() } } + .onChange(of: assigneeMappingTeamId) { _, _ in + Task { await refreshLinearAssigneeMappings() } + } .onDisappear { cancelLinearAuthFlow(userInitiated: false) }Also applies to: 3353-3355
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/SettingsOverlayView.swift` around lines 3225 - 3231, When the team scope field changes the view doesn't reload saved mappings, causing “Saved mappings” actions to target the wrong rows; call refreshLinearAssigneeMappings() whenever assigneeMappingTeamId changes just like you do for assigneeMappingProfile. In SettingsOverlayView add an onChange (or equivalent state observer) for assigneeMappingTeamId that invokes refreshLinearAssigneeMappings(), and apply the identical change to the other team-scope field referenced around the assigneeMapping (lines noted near 3353-3355) so both mapping UIs refresh when their team id changes.apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/SettingsOverlayView.swift-3344-3355 (1)
3344-3355:⚠️ Potential issue | 🟠 MajorKeep the provider panes in sync with model refreshes.
LinearExtensionSettingsViewandGitHubExtensionSettingsViewonly reload on the initial.taskand OAuth deep-link changes. After sign-in/out or any backgroundmodel.refresh(), the account lists and status pills stay stale until the pane is recreated. These panes should mirror theonChange(of: model.lastUpdated)reload pattern used by the other settings views, and they should also refresh when auth state flips.Also applies to: 4374-4385
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/SettingsOverlayView.swift` around lines 3344 - 3355, LinearExtensionSettingsView and GitHubExtensionSettingsView are only reloading on .task and OAuth deep-link changes, so hook them into the same model refresh/auth-change triggers as other panes: add an .onChange(of: model.lastUpdated) block to both views to call their existing reload logic (the same work done in .task), and also add an .onChange for the auth state flip (the same condition you observe for model.linearOAuthDeepLinkContext or sign-in/out) so account lists and status pills refresh when auth changes; locate the panes by the types LinearExtensionSettingsView and GitHubExtensionSettingsView and reuse the existing handlers (e.g., the logic around handleLinearOAuthDeepLink and refreshLinearAssigneeMappings) to perform the refresh to avoid duplicating initialization code.apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/CommandPalette.swift-355-363 (1)
355-363:⚠️ Potential issue | 🟠 MajorPost the routing request after the project view is active.
openProjectRouting()emits the notification synchronously right after changingselectedItem, unlikeopenProjectTicketReviewQueue(). If the routing listener lives under the newly selected project subtree, this can fire before that view mounts and the palette action becomes a no-op. Defer the post to the next run loop, and select the owning tab first if routing lives under Tickets.Suggested fix
private func openProjectRouting(_ project: ProjectSummary) { - model.selectedItem = .project(project.id) - NotificationCenter.default.post( - name: .hackProjectRoutingRequested, - object: nil, - userInfo: [ - ProjectRoutingRequest.projectIdKey: project.id - ] - ) + openProjectTickets(project) + DispatchQueue.main.async { + NotificationCenter.default.post( + name: .hackProjectRoutingRequested, + object: nil, + userInfo: [ + ProjectRoutingRequest.projectIdKey: project.id + ] + ) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/CommandPalette.swift` around lines 355 - 363, openProjectRouting currently sets model.selectedItem and immediately posts the .hackProjectRoutingRequested notification, which can fire before the newly selected project view mounts; update openProjectRouting(_ project: ProjectSummary) to first set model.selectedItem (and if routing belongs under Tickets ensure the Tickets tab is selected) and then defer the NotificationCenter.default.post call to the next run loop (e.g. DispatchQueue.main.async) so the receiver under the newly selected subtree has a chance to mount before handling the routing request, mirroring the approach used in openProjectTicketReviewQueue().services/auth-broker/src/modules/better-auth/session.ts-184-199 (1)
184-199:⚠️ Potential issue | 🟠 MajorFail closed when session or profile scope is missing.
hasBetterAuthProfileAccess()currently returnstrueforsession: null, and it also returnstruewhen a profile-scoped management token is used without a concreteprofileId. That turns an exported access helper into a fail-open path whenever a caller forgets to resolve auth first or passes an optional profile. Require a non-null session and a concreteprofileIdfor scoped tokens.Suggested fix
export function hasBetterAuthProfileAccess(input: { readonly session: BrokerBetterAuthSession | null; readonly profileId?: string | null; }): boolean { + if (!input.session) { + return false + } const managementTokenProfileId = normalizeOptionalString( - input.session?.managementTokenProfileId - ); + input.session.managementTokenProfileId + ) if (!managementTokenProfileId) { - return true; + return true } - const requestedProfileId = normalizeOptionalString(input.profileId); + const requestedProfileId = normalizeOptionalString(input.profileId) if (!requestedProfileId) { - return true; + return false } - return requestedProfileId === managementTokenProfileId; + return requestedProfileId === managementTokenProfileId }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/auth-broker/src/modules/better-auth/session.ts` around lines 184 - 199, hasBetterAuthProfileAccess currently treats missing session or profile ID as allowed; change it to fail-closed: first return false if input.session is null, then keep using normalizeOptionalString on input.session.managementTokenProfileId and input.profileId but treat a missing managementTokenProfileId as an unscoped token (still allowed) while treating a missing requested profileId as forbidden when the management token is scoped; finally return requestedProfileId === managementTokenProfileId for scoped tokens. Refer to hasBetterAuthProfileAccess, BrokerBetterAuthSession, input.session.managementTokenProfileId, and normalizeOptionalString to locate and update the logic.services/auth-broker/src/better-auth.ts-34-47 (1)
34-47:⚠️ Potential issue | 🟠 MajorNormalize blank Better Auth env vars to
undefinedbefore schema parsing.
readBetterAuthEnv()parses optional fields withz.string().min(1).optional()/z.string().url().optional(). In.envfiles,VAR=comes through as""(empty string), which fails the.min(1)constraint and causesbetterAuthEnvSchema.parse()to throw beforeresolveBetterAuthRequiredConfig()can fall back toenabled: false. Transform blank strings toundefinedbefore parsing to allow graceful degradation in local/dev modes.Also applies to: 131-149
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/auth-broker/src/better-auth.ts` around lines 34 - 47, The schema betterAuthEnvSchema currently rejects empty-string env values (e.g., VAR=) because fields use z.string().min(1).optional(); update readBetterAuthEnv() to normalize the raw env object by mapping any "" values to undefined before calling betterAuthEnvSchema.parse(), so blank strings pass the optional check and allow resolveBetterAuthRequiredConfig() to set enabled: false; apply the same normalization logic where the schema is parsed again (the block around lines 131-149) to ensure consistent behavior across readBetterAuthEnv and the other parse site.apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/ProjectDetailView.swift-1922-1940 (1)
1922-1940:⚠️ Potential issue | 🟠 MajorThe shared status callout is losing source and severity.
remoteConfigMessagecollapses execution, GitHub, and Linear messages into one string,remoteConfigToneinfers severity fromhasPrefix("Failed"), andremoteConfigActionssends every warning to Linear settings. Validation errors like “Choose a valid Linear project before saving.” render as “Settings updated”, and non-Linear failures can get the wrong CTA.Also applies to: 1942-1970
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/ProjectDetailView.swift` around lines 1922 - 1940, remoteConfigMessage collapses multiple sources into one string and remoteConfigTone infers severity via hasPrefix, causing loss of original source/severity (and wrong CTAs in remoteConfigActions); update the implementation to preserve the origin and severity by returning a structured value (e.g., an enum or small struct) from the combined accessor instead of a plain String — include the original sources executionTargetMessage, githubProfileMessage, and linearProjectMessage as distinct cases/fields and compute tone per-source (not via hasPrefix) so remoteConfigTone and remoteConfigActions read the structured result to decide the proper StatusTone and action (e.g., only show Linear settings CTA for Linear-related failures and map validation errors to warn vs success correctly).apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/TicketsView.swift-289-292 (1)
289-292:⚠️ Potential issue | 🟠 MajorUse autosync routes when deciding whether the background loop should run.
ProjectDetailViewcan enable autosync on additional Linear routes independently, but this gate ignoreslinearAutosyncRouteKeys. With the default inbound toggle off and an additional route on, the loop never starts, so those subscriptions never get processed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/TicketsView.swift` around lines 289 - 292, The current shouldRunBackgroundSyncLoop gate ignores autosync-enabled routes, so modify the computed property shouldRunBackgroundSyncLoop to also return true when any route in linearAutosyncRouteKeys is enabled for autosync; specifically, update the boolean expression that references linearRouteStatus, hasAnyLinearProjectRouting, linearSyncFromLinearEnabled, and linearCreateHackTicketsEnabled to include a check like "|| !linearAutosyncRouteKeys.isEmpty" or a predicate that inspects linearAutosyncRouteKeys for active autosync entries so the loop starts when autosync routes are present (use the existing symbols shouldRunBackgroundSyncLoop, linearRouteStatus, hasAnyLinearProjectRouting, linearSyncFromLinearEnabled, linearCreateHackTicketsEnabled, and linearAutosyncRouteKeys to locate and change the logic).apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/TicketsView.swift-237-239 (1)
237-239:⚠️ Potential issue | 🟠 MajorKeep manual reconcile separate from the auto-sync toggle.
canSyncProjectFromLinearcurrently gates the explicit “Reconcile from Linear” affordances too, so turning offsyncFromLinearremoves every manual import path. That contradicts the project-settings copy and leaves no way to refresh from Linear without re-enabling background sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/TicketsView.swift` around lines 237 - 239, The computed property canSyncProjectFromLinear is incorrectly using linearSyncFromLinearEnabled to gate manual “Reconcile from Linear” actions; change the logic so only automated/background sync respects linearSyncFromLinearEnabled while manual reconcile checks only that linearRouteStatus?.tokenResolved == true and hasAnyLinearProjectRouting (e.g., introduce or use a separate computed property like canManuallyReconcileFromLinear that omits linearSyncFromLinearEnabled), then update any UI affordances that currently read canSyncProjectFromLinear to use the new/manual-only property for explicit imports while leaving canSyncProjectFromLinear (or the existing one) to control the auto-sync toggle behavior.apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/TicketsView.swift-1890-1897 (1)
1890-1897:⚠️ Potential issue | 🟠 MajorMake sync exclusivity real, not just cosmetic.
runSyncActiononly updatesactiveSyncAction; it never guards re-entry, and the background loop bypasses it entirely. A manual reconcile can still overlap another manual/background sync and issue duplicatesyncLinearProject/syncLinearIssuecalls.Also applies to: 2002-2065
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/TicketsView.swift` around lines 1890 - 1897, runSyncAction currently only sets activeSyncAction without preventing concurrent runs; make it enforce mutual exclusion so manual/background loops cannot overlap syncLinearProject/syncLinearIssue calls. Implement a real async lock (e.g., an actor or AsyncSemaphore) and have runSyncAction acquire the lock before setting activeSyncAction and performing operation, releasing it in defer; update the background loop and any callers (the manual reconcile path and places that call syncLinearProject/syncLinearIssue) to use runSyncAction so all entry points go through the same lock and respect exclusivity. Ensure activeSyncAction is still set for UI/status purposes while holding the lock and that callers return immediately or await the lock to prevent duplicate concurrent syncs.apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/ProjectDetailView.swift-2597-2623 (1)
2597-2623:⚠️ Potential issue | 🟠 MajorDon't overwrite autosync failures with success copy.
toggleLinearAutosyncreports failure only by mutatinglinearProjectMessage, and both callers here immediately replace that with a success string. If the subscription change fails after the config/binding write, the UI still says inbound sync or project binding succeeded even though autosync was not actually updated.Also applies to: 2678-2695, 2774-2821
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/ProjectDetailView.swift` around lines 2597 - 2623, persistLinearSyncFromLinearSetting currently calls toggleLinearAutosync which signals failure by mutating linearProjectMessage, but the caller always overwrites that message with a success string; change toggleLinearAutosync to return an explicit Bool (async) representing success/failure and update callers (persistLinearSyncFromLinearSetting and the other spots noted around the 2678–2695 and 2774–2821 ranges) to await that Bool and only set linearProjectMessage to the success text when toggleLinearAutosync returned true; if it returns false, leave linearProjectMessage as-is (or set an appropriate failure text) so autosync failures aren’t hidden. Ensure you update the toggleLinearAutosync signature and all call sites accordingly.apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/DashboardModel.swift-720-750 (1)
720-750:⚠️ Potential issue | 🟠 MajorAwait the in-flight Hack account refresh instead of returning stale state.
guard !isLoadingHackAccountState else { return }drops overlapping callers on the floor. While a dashboard refresh is already fetching,inspectHackAccountSettingsState(force: true),loginHackAccount(),logoutHackAccount(), and the deep-link poller can all observe the previoushackAccountState/nilinstead of the fresh result. This needs a shared in-flight task that callers await, with forced callers able to trigger a follow-up refresh after the current one finishes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/DashboardModel.swift` around lines 720 - 750, The current refreshHackAccountState uses isLoadingHackAccountState to drop overlapping callers; instead introduce a shared in-flight Task property (e.g. hackAccountRefreshTask: Task<Void, Error>? or Task<HackAccountState, Error>?) that is assigned when starting client.inspectHackAccountSettingsState() so concurrent callers can await it rather than returning early, await the existing task when isLoadingHackAccountState is true, and if the caller passed force: true schedule/trigger a follow-up refresh after the current task completes to ensure a fresh result; update assignments to hackAccountState and lastHackAccountRefreshAt inside the task's success path and clear the shared task in its finally/finalize block while preserving updateErrorMessage behavior on failure.apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/DashboardModel.swift-1309-1319 (1)
1309-1319:⚠️ Potential issue | 🟠 MajorThe deep-link parsers no longer match the configurable callback URLs.
HackCLIClientnow builds callback URLs fromHackAuthCompletionURL,HackGitHubOAuthCallbackURL, andHackLinearOAuthCallbackURL, but these helpers still only accept the hard-coded/auth/...routes. Any non-default callback path will open the app and then be ignored here, leaving auth/OAuth stuck in the finalization state. Parse the configured URLs from the bundle, or compare against the exact configured scheme/host/path tuples, instead of hard-coding the routes.Also applies to: 1321-1358, 1360-1394
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/DashboardModel.swift` around lines 1309 - 1319, The isHackAuthCompletionDeepLink(url:) function (and the sibling deep-link checks around the 1321-1358 and 1360-1394 ranges) currently match only hard-coded paths like "/auth/complete" and must be updated to accept the app-configured callback URLs; fetch the configured callback URLs (HackAuthCompletionURL, HackGitHubOAuthCallbackURL, HackLinearOAuthCallbackURL) from the bundle or their helpers, parse each into scheme/host/path tuples, and then compare the incoming URL's scheme, host and normalized path exactly against those tuples rather than matching hard-coded strings so any custom callback path is recognized and routed correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: abadb8a5-e188-437c-925a-157139b08a43
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (130)
.hack/hack.config.jsonapps/macos/App/HackDesktopApp.swiftapps/macos/App/Info.plistapps/macos/Config/Base.xcconfigapps/macos/Config/Debug.xcconfigapps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/CommandPalette.swiftapps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/DashboardModel.swiftapps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/DashboardView.swiftapps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/NodeTopologySettingsView.swiftapps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/ProjectDetailView.swiftapps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/SettingsOverlayView.swiftapps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/TicketsView.swiftapps/macos/Packages/Services/HackCLI/Sources/HackCLIService/HackCLIClient.swiftapps/macos/Packages/Shared/Models/Sources/HackDesktopModels/Models.swiftapps/macos/Packages/Shared/Models/Tests/HackDesktopModelsTests/LinearSyncUXModelTests.swiftapps/macos/Packages/Shared/Models/Tests/HackDesktopModelsTests/TicketDetailReviewModelTests.swiftapps/macos/README.mddocs/cli.mddocs/env.mddocs/extensions.mddocs/guides/linear-integration-architecture.mddocs/plans/2026-03-05-linear-sync-phase-2-design.mddocs/plans/2026-03-05-linear-sync-phase-2-plan.mddocs/plans/2026-03-05-linear-sync-ux-design.mddocs/plans/2026-03-05-linear-sync-ux-plan.mddocs/plans/2026-03-06-hack-account-auth-and-provider-ux-design.mddocs/plans/2026-03-06-hack-account-auth-and-provider-ux-plan.mddocs/plans/2026-03-06-linear-desktop-ux-cleanup-design.mddocs/plans/2026-03-06-linear-desktop-ux-cleanup-plan.mddocs/plans/2026-03-06-linear-phase-3-completion-design.mddocs/plans/2026-03-06-linear-phase-3-completion-plan.mddocs/plans/2026-03-06-remote-encrypted-project-env-portability-plan.mddocs/plans/2026-03-07-linear-browser-handoff-and-connection-repair-design.mddocs/plans/2026-03-07-linear-browser-handoff-and-connection-repair-plan.mddocs/plans/2026-03-07-linear-local-access-seeding-design.mddocs/plans/2026-03-07-linear-local-access-seeding-plan.mddocs/plans/2026-03-09-linear-autosync-tickets-design.mddocs/plans/2026-03-09-linear-autosync-tickets-plan.mdexamples/basic/.hack/hack.config.jsonexamples/next-app/.hack/hack.config.jsonpackages/db/src/schema/core.tspackages/db/tests/smoke.test.tsscripts/install-cli.tsservices/auth-broker/.env.exampleservices/auth-broker/README.mdservices/auth-broker/package.jsonservices/auth-broker/src/app.tsservices/auth-broker/src/better-auth-link.tsservices/auth-broker/src/better-auth.tsservices/auth-broker/src/config.tsservices/auth-broker/src/db.tsservices/auth-broker/src/db/ensure-columns.tsservices/auth-broker/src/db/schema.tsservices/auth-broker/src/flow-store.tsservices/auth-broker/src/linear.tsservices/auth-broker/src/modules/better-auth/management-token.tsservices/auth-broker/src/modules/better-auth/plugin.tsservices/auth-broker/src/modules/better-auth/session.tsservices/auth-broker/src/modules/better-auth/shell-plugin.tsservices/auth-broker/src/modules/github-oauth/model.tsservices/auth-broker/src/modules/github-oauth/service.tsservices/auth-broker/src/modules/linear-agent/plugin.tsservices/auth-broker/src/modules/linear-autosync/plugin.tsservices/auth-broker/src/modules/linear-autosync/service.tsservices/auth-broker/src/modules/linear-connections/local-access.tsservices/auth-broker/src/modules/linear-connections/plugin.tsservices/auth-broker/src/modules/linear-connections/service.tsservices/auth-broker/src/modules/linear-oauth/callback-routes-plugin.tsservices/auth-broker/src/modules/linear-oauth/flow-status-routes-plugin.tsservices/auth-broker/src/modules/linear-oauth/model.tsservices/auth-broker/src/modules/linear-oauth/plugin.tsservices/auth-broker/src/modules/linear-oauth/refresh-routes-plugin.tsservices/auth-broker/src/modules/linear-oauth/service.tsservices/auth-broker/src/modules/linear-oauth/start-routes-plugin.tsservices/auth-broker/src/modules/linear-sync-store/plugin.tsservices/auth-broker/src/modules/linear-sync-store/service.tsservices/auth-broker/src/modules/providers/plugin.tsservices/auth-broker/src/types.tsservices/auth-broker/tests/better-auth-link.test.tsservices/auth-broker/tests/better-auth-schema.test.tsservices/auth-broker/tests/config.test.tsservices/auth-broker/tests/deeplink.test.tsservices/auth-broker/tests/ensure-columns.test.tsservices/auth-broker/tests/flow-store.test.tsservices/auth-broker/tests/index.test.tsservices/auth-broker/tests/linear-auth-header.test.tsservices/auth-broker/tests/linear-ownership-compat.test.tsservices/auth-broker/tests/linear-store-schema.test.tsservices/auth-broker/tests/linear-token-custody.test.tsservices/auth-broker/tests/package-manifest.test.tsservices/auth-broker/tests/session-auth.test.tssrc/cli/run.tssrc/cli/spec.tssrc/commands/auth.tssrc/commands/doctor-utils.tssrc/commands/doctor.tssrc/commands/env.tssrc/commands/global.tssrc/commands/linear.tssrc/constants.tssrc/control-plane/extensions/builtins.tssrc/control-plane/extensions/linear/auth.tssrc/control-plane/extensions/linear/client.tssrc/control-plane/extensions/linear/commands.tssrc/control-plane/extensions/linear/extension.tssrc/control-plane/extensions/tickets/commands.tssrc/control-plane/extensions/tickets/store.tssrc/control-plane/extensions/tickets/tickets-git-channel.tssrc/control-plane/sdk/config.tssrc/lib/auth-session.tssrc/lib/config.tssrc/lib/nodes-registry.tssrc/lib/secret-store.tssrc/mcp/server.tstests/auth-command.test.tstests/config.test.tstests/doctor-utils.test.tstests/env-backend-command.test.tstests/github-auth.test.tstests/global-command.macos.test.tstests/linear-auth.test.tstests/linear-client.test.tstests/linear-command-alias.test.tstests/linear-commands.test.tstests/mcp.test.tstests/nodes-registry.test.tstests/secret-store.test.tstests/tickets-extension.test.tstests/tickets-git-channel.test.tstests/tickets-store.test.ts
| /// Starts cloud Linear OAuth with the dedicated auth broker surface. | ||
| public func startLinearOAuthFlow( | ||
| profileId: String, | ||
| setDefault: Bool | ||
| ) async throws -> LinearOAuthFlowStartResponse { | ||
| var args = [ | ||
| "x", | ||
| "linear", | ||
| "oauth-connect", | ||
| "--json", | ||
| "--start-only", | ||
| "--profile", | ||
| profileId, | ||
| "--desktop-redirect-url", | ||
| desktopLinearOAuthCallbackURL(), | ||
| ] | ||
| if setDefault { | ||
| args.append("--set-default") | ||
| } | ||
| let result = try await run(args, allowNonZeroExit: true) | ||
| return try decodeJsonOrThrow(LinearOAuthFlowStartResponse.self, result: result) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the repo for the Linear OAuth start-flow serializer and tests.
rg -n -C3 --glob '!**/.build/**' --glob '!**/node_modules/**' \
'LinearOAuth(StartEnvelope|FlowStartResponse)|oauth-connect|start-only|pollUrl|deviceCode|statusUrl'Repository: hack-dance/hack
Length of output: 50371
🏁 Script executed:
fd -t f '\.swift$' -path '*HackCLI*' | head -20Repository: hack-dance/hack
Length of output: 227
🏁 Script executed:
find . -path '*HackCLI*' -name '*.swift' | head -20Repository: hack-dance/hack
Length of output: 223
🏁 Script executed:
rg -n 'LinearOAuth(FlowStartResponse|StartEnvelope)' apps/macos/Packages/Services/HackCLI/Sources/HackCLIService/HackCLIClient.swift -A 10Repository: hack-dance/hack
Length of output: 1399
🏁 Script executed:
rg -n 'struct LinearOAuthFlowStartResponse' apps/macos/Packages/Services/HackCLI/Sources/HackCLIService/HackCLIClient.swift -A 15Repository: hack-dance/hack
Length of output: 41
🏁 Script executed:
rg -n 'LinearOAuthFlowStartResponse' apps/macos/Packages/Services/HackCLI/Sources/HackCLIService/HackCLIClient.swift -B 2 -A 10Repository: hack-dance/hack
Length of output: 1043
🏁 Script executed:
rg -n 'LinearOAuthFlowStartResponse' apps/macos/Packages/Services/HackCLI/Sources/HackCLIService/HackCLIClient.swift | head -20Repository: hack-dance/hack
Length of output: 203
🏁 Script executed:
rg -n 'LinearOAuthFlowStartResponse' --glob '!**/.build/**' --glob '!**/node_modules/**'Repository: hack-dance/hack
Length of output: 649
🏁 Script executed:
rg -n 'LinearOAuthFlowStartResponse' apps/macos/Packages/Shared/Models/Sources/HackDesktopModels/Models.swift -A 20Repository: hack-dance/hack
Length of output: 681
🏁 Script executed:
rg -n 'startGitHubOAuthFlow' apps/macos/Packages/Services/HackCLI/Sources/HackCLIService/HackCLIClient.swift -A 25Repository: hack-dance/hack
Length of output: 1188
🏁 Script executed:
rg -n 'startGitHubOAuthFlow' apps/macos/Packages/Services/HackCLI/Sources/HackCLIService/HackCLIClient.swift -A 50 | head -80Repository: hack-dance/hack
Length of output: 2403
🏁 Script executed:
sed -n '1075,1150p' apps/macos/Packages/Services/HackCLI/Sources/HackCLIService/HackCLIClient.swiftRepository: hack-dance/hack
Length of output: 2883
🏁 Script executed:
rg -n 'struct GitHubOAuthStartEnvelope' apps/macos/Packages/Services/HackCLI/Sources/HackCLIService/HackCLIClient.swift -A 10Repository: hack-dance/hack
Length of output: 569
The Linear start-flow decode is incomplete and will fail with the CLI's actual wrapped response payload.
The CLI (x linear oauth-connect --start-only --json) emits a wrapped LinearOAuthStartEnvelope with separate deviceCode and pollUrl fields, but startLinearOAuthFlow() only attempts to decode the flat LinearOAuthFlowStartResponse. Unlike startGitHubOAuthFlow(), which handles both formats by building statusUrl from the wrapped response, the Linear method lacks this fallback and will crash when receiving the real CLI output.
The codebase already shows the correct pattern in fetchLinearOAuthFlowStatus(), which uses tryDecodeLenient() to attempt a direct decode first, then falls back to unwrapping and normalizing. Apply the same pattern to startLinearOAuthFlow(): try decoding to flat response first, then fall back to decoding LinearOAuthStartEnvelope and construct statusUrl from pollUrl + deviceCode + query parameters.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/macos/Packages/Services/HackCLI/Sources/HackCLIService/HackCLIClient.swift`
around lines 1075 - 1095, startLinearOAuthFlow currently decodes only a flat
LinearOAuthFlowStartResponse and will fail for the CLI's wrapped payload; change
its decoding to the same lenient pattern used in fetchLinearOAuthFlowStatus:
after running run(...) attempt to decode directly into
LinearOAuthFlowStartResponse (using tryDecodeLenient or equivalent), and if that
fails decode into LinearOAuthStartEnvelope and construct the normalized
LinearOAuthFlowStartResponse by building statusUrl from envelope.pollUrl plus
the envelope.deviceCode and the necessary query parameters (client id/redirect
URI matching desktopLinearOAuthCallbackURL()), then return the normalized
response; use the same helper/decoding calls (tryDecodeLenient,
decodeJsonOrThrow, run) and follow the
startGitHubOAuthFlow/fetchLinearOAuthFlowStatus pattern.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
scripts/run-tests-deterministic.ts (2)
15-19: Drop the type assertion in the membership check.The cast here turns an arbitrary glob result into the literal union instead of narrowing it. A
Set<string>keeps this strict and simpler.♻️ Suggested refactor
+const isolatedTestPathSet = new Set<string>(isolatedTestPaths) const remainingTests = allTests.filter( - (testPath) => - !isolatedTestPaths.includes( - testPath as (typeof isolatedTestPaths)[number] - ) + (testPath) => !isolatedTestPathSet.has(testPath) )As per coding guidelines, "Leverage TypeScript's type narrowing instead of type assertions".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/run-tests-deterministic.ts` around lines 15 - 19, The membership check currently uses a type assertion when filtering allTests into remainingTests; remove the cast and instead construct a Set<string> from isolatedTestPaths (e.g., isolatedTestSet = new Set(isolatedTestPaths)) and use isolatedTestSet.has(testPath) inside the filter to perform the membership test without type assertions, updating references to remainingTests, allTests, and isolatedTestPaths accordingly.
3-6: Make isolated-test metadata single-source.
isolatedTestPathsandtestLabel()now have to stay in sync manually. A rename/addition here can easily miss the label mapping and silently fall back to the raw path. Prefer one typed config array and derive both the loop and labels from that.♻️ Suggested refactor
+interface IsolatedTest { + readonly path: string + readonly label: string +} + -const isolatedTestPaths = [ - "tests/node-workspace-bootstrap.test.ts", - "tests/nodes-registry.test.ts", -] as const +const isolatedTests: readonly IsolatedTest[] = [ + { + path: "tests/node-workspace-bootstrap.test.ts", + label: "bootstrap workspace", + }, + { + path: "tests/nodes-registry.test.ts", + label: "nodes registry auth", + }, +] as constAs per coding guidelines, "Prefer
interfacefor defining object shapes in TypeScript".Also applies to: 54-62
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@scripts/run-tests-deterministic.ts`:
- Around line 15-19: The membership check currently uses a type assertion when
filtering allTests into remainingTests; remove the cast and instead construct a
Set<string> from isolatedTestPaths (e.g., isolatedTestSet = new
Set(isolatedTestPaths)) and use isolatedTestSet.has(testPath) inside the filter
to perform the membership test without type assertions, updating references to
remainingTests, allTests, and isolatedTestPaths accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 692d7551-da3e-49aa-96ff-9dbfe8cd2eab
📒 Files selected for processing (3)
scripts/run-tests-deterministic.tstests/linear-auth.test.tstests/linear-command-alias.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/auth-broker/src/better-auth-link.ts (1)
68-83:⚠️ Potential issue | 🟠 MajorResolve existing provider links before enforcing email-based checks.
Line 70-75 returns
missing_email/email_not_verifiedbefore the(providerId, accountId)lookup on Line 79. That can lock out already-linked users whenever the OAuth provider omits an email or marks it unverified, even though the account link is already sufficient to identify them.Suggested fix
const accountId = normalizeText(input.account.accountId) const accountEmail = normalizeText(input.account.accountEmail) - if (!accountEmail) { - return { state: "missing_email" } - } - if (input.account.accountEmailVerified === false) { - return { state: "email_not_verified" } - } try { if (accountId) { const linkedUserId = await findUserIdByProviderAccountId({ runtime, providerId: input.providerId, accountId, }) if (linkedUserId) { return { state: "linked_existing", userId: linkedUserId } } } + + if (!accountEmail) { + return { state: "missing_email" } + } + if (input.account.accountEmailVerified === false) { + return { state: "email_not_verified" } + } const existingUserId = await findUserIdByEmail({ runtime, email: accountEmail,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/auth-broker/src/better-auth-link.ts` around lines 68 - 83, The current early returns for missing or unverified email (using input.account.accountEmail and input.account.accountEmailVerified) run before checking existing provider links and can lock out users; move the provider-link resolution (call to findUserIdByProviderAccountId with accountId and providerId) ahead of the email checks so that when accountId is present and findUserIdByProviderAccountId returns a linked userId you short-circuit and return that link (preserving existing behavior for linked accounts), and only if no linkedUserId is found fall back to the normalizeText(accountEmail) / email verification checks; adjust logic around accountId/accountEmail variables and returns accordingly.
🧹 Nitpick comments (4)
services/auth-broker/src/better-auth-link.ts (1)
31-61: Extract the shared resolver input shapes into interfaces.These signatures repeat the same object shape with only
providerIdvarying. Pull them into sharedinterfaces so the public contract stays centralized and easier to evolve.As per coding guidelines, "Prefer
interfacefor defining object shapes in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/auth-broker/src/better-auth-link.ts` around lines 31 - 61, Extract the repeated object shapes used by resolveBetterAuthUserFromGitHubAccount, resolveBetterAuthUserFromLinearAccount, and resolveBetterAuthUserFromOAuthAccount into shared TypeScript interfaces: create a public interface (e.g., BetterAuthOAuthInput) for the externally passed shape { runtime, account, autoProvision } and an internal interface (e.g., ResolveOAuthInput) that adds providerId for the resolver; update the two public functions to accept the public interface and pass a ResolveOAuthInput to resolveBetterAuthUserFromOAuthAccount, and change resolveBetterAuthUserFromOAuthAccount's parameter type to the internal interface, using the interface keyword (not type) for both definitions so the shape is centralized and easier to evolve.services/auth-broker/src/better-auth.ts (3)
13-22: Preferinterfacefor these exported object shapes.
BetterAuthSocialProvider,BetterAuthAccountLinkingPolicy, and the publicBetterAuthRuntimecontract are plain object shapes. Usinginterfacehere matches the repo convention and keeps the public surface easier to extend.As per coding guidelines, "Prefer
interfacefor defining object shapes in TypeScript".Also applies to: 24-32
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/auth-broker/src/better-auth.ts` around lines 13 - 22, Replace the exported object-shaped type aliases with exported interfaces: change "export type BetterAuthSocialProvider = { ... }", "export type BetterAuthAccountLinkingPolicy = { ... }" and the public runtime contract "BetterAuthRuntime" (the object shape at lines ~24-32) into "export interface" declarations, preserving each property's names and readonly modifiers so the public surface and semantics remain identical while following the repo convention to prefer interface for object shapes.
125-128: Keep the new public helper on the named-parameter convention.
ensureBetterAuthRuntimeReady({ runtime })will fit the repo style better and gives you room to add more context later without another breaking signature change.As per coding guidelines, "Always default to using named parameters in functions (e.g.,
myFunction({ ctx, other })instead ofmyFunction(ctx, other))".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/auth-broker/src/better-auth.ts` around lines 125 - 128, Change the function to use a single named-parameter object for future extensibility: update the signature of ensureBetterAuthRuntimeReady to accept ({ runtime }: { runtime: BetterAuthRuntime }) and keep the body awaiting runtime.ready; export stays the same. Also update all call sites to pass an object (e.g., ensureBetterAuthRuntimeReady({ runtime })) and adjust any TypeScript types/imports accordingly so the behavior and return type remain Promise<void>.
241-412: Avoid owning the auth schema in raw SQL andauthBrokerSchemaat the same time.This bootstrap duplicates the Better Auth/organization tables and indexes that are already modeled through
authBrokerSchema. The next schema change can easily update one side and not the other, and a failure halfway through this sequence leaves a partially bootstrapped auth schema. Prefer a migration or a single schema-derived bootstrap path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/auth-broker/src/better-auth.ts` around lines 241 - 412, The function ensureBetterAuthTables duplicates table/index creation already defined by authBrokerSchema; remove the raw SQL DDL and instead drive bootstrapping from the single source of truth (authBrokerSchema) or run the migration path so schema changes aren't applied in two places. Edit ensureBetterAuthTables to stop calling input.db.execute(sql`...`) / sql.raw for the auth tables and indexes and replace that logic with a single call that applies authBrokerSchema (or invokes the existing migration routine) so creation of "user", "organization", "team", "session", "account", "verification", "member", "invitation", and "team_member" and their indexes is performed only via authBrokerSchema; keep the function name ensureBetterAuthTables and the input.db usage but delegate to the schema/migration API instead of duplicating SQL.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@services/auth-broker/src/better-auth-link.ts`:
- Around line 68-83: The current early returns for missing or unverified email
(using input.account.accountEmail and input.account.accountEmailVerified) run
before checking existing provider links and can lock out users; move the
provider-link resolution (call to findUserIdByProviderAccountId with accountId
and providerId) ahead of the email checks so that when accountId is present and
findUserIdByProviderAccountId returns a linked userId you short-circuit and
return that link (preserving existing behavior for linked accounts), and only if
no linkedUserId is found fall back to the normalizeText(accountEmail) / email
verification checks; adjust logic around accountId/accountEmail variables and
returns accordingly.
---
Nitpick comments:
In `@services/auth-broker/src/better-auth-link.ts`:
- Around line 31-61: Extract the repeated object shapes used by
resolveBetterAuthUserFromGitHubAccount, resolveBetterAuthUserFromLinearAccount,
and resolveBetterAuthUserFromOAuthAccount into shared TypeScript interfaces:
create a public interface (e.g., BetterAuthOAuthInput) for the externally passed
shape { runtime, account, autoProvision } and an internal interface (e.g.,
ResolveOAuthInput) that adds providerId for the resolver; update the two public
functions to accept the public interface and pass a ResolveOAuthInput to
resolveBetterAuthUserFromOAuthAccount, and change
resolveBetterAuthUserFromOAuthAccount's parameter type to the internal
interface, using the interface keyword (not type) for both definitions so the
shape is centralized and easier to evolve.
In `@services/auth-broker/src/better-auth.ts`:
- Around line 13-22: Replace the exported object-shaped type aliases with
exported interfaces: change "export type BetterAuthSocialProvider = { ... }",
"export type BetterAuthAccountLinkingPolicy = { ... }" and the public runtime
contract "BetterAuthRuntime" (the object shape at lines ~24-32) into "export
interface" declarations, preserving each property's names and readonly modifiers
so the public surface and semantics remain identical while following the repo
convention to prefer interface for object shapes.
- Around line 125-128: Change the function to use a single named-parameter
object for future extensibility: update the signature of
ensureBetterAuthRuntimeReady to accept ({ runtime }: { runtime:
BetterAuthRuntime }) and keep the body awaiting runtime.ready; export stays the
same. Also update all call sites to pass an object (e.g.,
ensureBetterAuthRuntimeReady({ runtime })) and adjust any TypeScript
types/imports accordingly so the behavior and return type remain Promise<void>.
- Around line 241-412: The function ensureBetterAuthTables duplicates
table/index creation already defined by authBrokerSchema; remove the raw SQL DDL
and instead drive bootstrapping from the single source of truth
(authBrokerSchema) or run the migration path so schema changes aren't applied in
two places. Edit ensureBetterAuthTables to stop calling
input.db.execute(sql`...`) / sql.raw for the auth tables and indexes and replace
that logic with a single call that applies authBrokerSchema (or invokes the
existing migration routine) so creation of "user", "organization", "team",
"session", "account", "verification", "member", "invitation", and "team_member"
and their indexes is performed only via authBrokerSchema; keep the function name
ensureBetterAuthTables and the input.db usage but delegate to the
schema/migration API instead of duplicating SQL.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c50f4b12-4e7c-4664-90df-da316269b5c6
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
services/auth-broker/package.jsonservices/auth-broker/src/better-auth-link.tsservices/auth-broker/src/better-auth.tsservices/auth-broker/src/modules/better-auth/session.tsservices/auth-broker/src/modules/linear-connections/plugin.tsservices/auth-broker/src/modules/linear-sync-store/plugin.tsservices/auth-broker/tests/better-auth-link.test.tsservices/auth-broker/tests/index.test.tsservices/auth-broker/tests/session-auth.test.tstests/linear-client.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- services/auth-broker/src/modules/better-auth/session.ts
- services/auth-broker/package.json
## <small>1.17.1 (2026-03-09)</small> * fix: trigger release after merged linear integration ([c8991f9](c8991f9)) * fix(tests): avoid keychain dependency in linear alias test ([43654c2](43654c2)) * fix(tests): harden tickets and linear alias CI behavior ([4ef36e5](4ef36e5)) * fix(tests): parse linear alias json payload robustly ([6f4f9fb](6f4f9fb)) * fix(tests): use config-only linear alias coverage ([42ca8b1](42ca8b1)) * Fix Linear integration setup and docs (#18) ([3a46a21](3a46a21)), closes [#18](#18)
Summary
What changed
.hackconfig where the new wiring needs to be representedCI fixes in this branch
tests/nodes-registry.test.tsin the deterministic Bun runner to avoid cross-file mock/env bleedhack linear status --jsonalias test assert alias forwarding without depending on whether the local machine already has Linear credentialsTesting
bun run --cwd packages/cli testNot run in this pass
Summary by CodeRabbit
New Features
Documentation