aws-samples / aws-samples/sample-autonomous-cloud-coding-agents

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)

Open
#883 0 comments 0 reactions 0 assignees View on GitHub
adapters bug infra-cdk
Dominant language
TypeScript
Stars
143
Forks
46
Avg merge
3d 9h
Merged PRs (30d)
20

Description

Parent context: surfaced during PR #681 review by @isadeks (issue #306), review [`5181793802`](https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/pull/681#pullrequestreview-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.

Concretely:

- Continue the scan past the first match to the end of the keyspace (bounded by the existing `MAX_SCAN_PAGES` guard added in #681) and collect all matches.
- 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 **all** `active` matches for the slug, bounded by `MAX_SCAN_PAGES`.
- [ ] `matches.length > 1` → 409 + distinct error code naming the colliding workspace ids; **no** revoke, **no** `DeleteSecret`, **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.

Contributor guide

Open the contributing guide

Research direction

Read cdk/src/handlers/linear-remove-workspace.ts, especially the paginated lookup and MAX_SCAN_PAGES guard, then inspect the existing handler tests. Verify duplicate active slugs produce a 409 with colliding workspace IDs and no smSend call, while single- and zero-match behavior remains unchanged; also trace the CLI response types and LINEAR_SETUP_GUIDE.md error documentation.

Written by the indexing model from the issue text.

Assessment

Tech stack
aws, typescript
Domain
api, backend, cli, databases, documentation
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.