You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
fix(api): duplicate active rows for one linear workspace_slug are silently first-match-wins on remove-workspace — return 409 (N8, follow-up to #306) #883
Parent context: surfaced during PR #681 review by @isadeks (issue #306), review 5181793802 finding N8. Deferred from #681 deliberately because it changes API semantics; a bugfix PR is the wrong vehicle. Needs maintainer approved before implementation (ADR-003).
Finding
DELETE /v1/linear/workspaces/{slug} resolves the target row by scanning LinearWorkspaceRegistryTable with a FilterExpression on workspace_slug + status = 'active', then takes Items?.[0] and stops on the first match (cdk/src/handlers/linear-remove-workspace.ts, the do { … } while (!row && scanKey) lookup).
If two active rows share a workspace_slug, one is torn down and the other keeps status='active'and a live OAuth secret — and the caller is told the removal succeeded. The operator has no signal that a second live grant survives.
This is the same absent-vs-ambiguous conflation the rest of #681 was tightened to avoid: "I found a row" is being reported as "I found the row".
How duplicates arise
workspace_slug is the Linear urlKey, which is not immutable. A workspace renamed in Linear frees its old urlKey; a different workspace can then take it and be onboarded. Nothing in bgagent linear setup / add-workspace enforces uniqueness on workspace_slug — the table's key schema is on linear_workspace_id, so two distinct workspace ids may legitimately carry the same slug.
Note this is not reachable via the removal path itself: #681 added ConditionExpression: '#status = :active' to the revoke, so concurrent DELETEs cannot both succeed on the same row. N8 is about two different rows that share a slug.
Suggested fix (reviewer's wording)
Consider paginating to completion and returning 409 on a match count > 1.
On matches.length > 1, return 409 with a distinct error code (e.g. WORKSPACE_SLUG_AMBIGUOUS) whose body lists the colliding linear_workspace_id values, so the operator can re-issue the removal against an unambiguous identifier.
Leave the single-match path byte-for-byte as-is.
Open design question worth settling in review: should the endpoint additionally accept linear_workspace_id as a disambiguator so a 409 is recoverable through the API rather than only through the manual runbook? Without it, the 409 is honest but leaves the operator in LINEAR_SETUP_GUIDE.md's manual fallback.
Why this is a semantics change, not a bugfix
Adding a 409 introduces a response the CLI and any other client must handle, and the paginate-to-completion change makes the lookup cost proportional to the whole table rather than to the position of the first match. Both belong behind their own review.
Acceptance criteria
Lookup collects allactive matches for the slug, bounded by MAX_SCAN_PAGES.
matches.length > 1 → 409 + distinct error code naming the colliding workspace ids; no revoke, noDeleteSecret, no row delete.
Single-match and zero-match behaviour unchanged (200 / 404).
Handler test seeding two active rows with one workspace_slug asserts the 409 and asserts smSend was never called.
Error code documented wherever WORKSPACE_NOT_FOUND / SECRET_DELETE_FAILED are (docs/guides/LINEAR_SETUP_GUIDE.md), and cli/src/types.ts updated if the response shape grows.
CLI surfaces the 409 with the colliding ids rather than a generic failure.
Parent context: surfaced during PR #681 review by @isadeks (issue #306), review
5181793802finding N8. Deferred from #681 deliberately because it changes API semantics; a bugfix PR is the wrong vehicle. Needs maintainerapprovedbefore implementation (ADR-003).Finding
DELETE /v1/linear/workspaces/{slug}resolves the target row by scanningLinearWorkspaceRegistryTablewith aFilterExpressiononworkspace_slug+status = 'active', then takesItems?.[0]and stops on the first match (cdk/src/handlers/linear-remove-workspace.ts, thedo { … } while (!row && scanKey)lookup).If two
activerows share aworkspace_slug, one is torn down and the other keepsstatus='active'and a live OAuth secret — and the caller is told the removal succeeded. The operator has no signal that a second live grant survives.This is the same absent-vs-ambiguous conflation the rest of #681 was tightened to avoid: "I found a row" is being reported as "I found the row".
How duplicates arise
workspace_slugis the LinearurlKey, which is not immutable. A workspace renamed in Linear frees its oldurlKey; a different workspace can then take it and be onboarded. Nothing inbgagent linear setup/add-workspaceenforces uniqueness onworkspace_slug— the table's key schema is onlinear_workspace_id, so two distinct workspace ids may legitimately carry the same slug.Note this is not reachable via the removal path itself: #681 added
ConditionExpression: '#status = :active'to the revoke, so concurrent DELETEs cannot both succeed on the same row. N8 is about two different rows that share a slug.Suggested fix (reviewer's wording)
Concretely:
MAX_SCAN_PAGESguard added in feat(cli): bgagent linear remove-workspace + DELETE route + fail-closed resolver (#306) #681) and collect all matches.matches.length > 1, return 409 with a distinct error code (e.g.WORKSPACE_SLUG_AMBIGUOUS) whose body lists the collidinglinear_workspace_idvalues, so the operator can re-issue the removal against an unambiguous identifier.Open design question worth settling in review: should the endpoint additionally accept
linear_workspace_idas a disambiguator so a 409 is recoverable through the API rather than only through the manual runbook? Without it, the 409 is honest but leaves the operator inLINEAR_SETUP_GUIDE.md's manual fallback.Why this is a semantics change, not a bugfix
Adding a 409 introduces a response the CLI and any other client must handle, and the paginate-to-completion change makes the lookup cost proportional to the whole table rather than to the position of the first match. Both belong behind their own review.
Acceptance criteria
activematches for the slug, bounded byMAX_SCAN_PAGES.matches.length > 1→ 409 + distinct error code naming the colliding workspace ids; no revoke, noDeleteSecret, no row delete.activerows with oneworkspace_slugasserts the 409 and assertssmSendwas never called.WORKSPACE_NOT_FOUND/SECRET_DELETE_FAILEDare (docs/guides/LINEAR_SETUP_GUIDE.md), andcli/src/types.tsupdated if the response shape grows.