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)

Aperta
#883 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
adapters bug infra-cdk
Lingua principale
TypeScript
Stelle
143
Fork
46
Merge medio
3g 9h
PR unite (30g)
20

Descrizione

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.

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

Leggi cdk/src/handlers/linear-remove-workspace.ts, in particolare la ricerca paginata e la protezione MAX_SCAN_PAGES, quindi esamina i test esistenti dell'handler. Verifica che gli slug attivi duplicati producano un 409 con gli ID dei workspace in conflitto e nessuna chiamata a smSend, mentre il comportamento con una e zero corrispondenze rimanga invariato; traccia inoltre i tipi di risposta della CLI e la documentazione degli errori in LINEAR_SETUP_GUIDE.md.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
aws, typescript
Ambito
api, backend, cli, databases, documentation
Tipo di issue
Bug
Difficoltà
4/5
Tempo stimato
3-5 giorni
Stato di attività
Attiva
Chiarezza
Specificata chiaramente
Idoneità per principianti
38/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.