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)

Offen
#883 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
adapters bug infra-cdk
Vorherrschende Sprache
TypeScript
Sterne
143
Forks
46
Ø Merge
3 T. 9 Std.
Gemergte PRs (30 T.)
20

Beschreibung

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.

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Lies cdk/src/handlers/linear-remove-workspace.ts, insbesondere die paginierte Suche und die MAX_SCAN_PAGES-Schutzvorrichtung, und prüfe anschließend die vorhandenen Handler-Tests. Verifiziere, dass doppelte aktive Slugs einen 409 mit den kollidierenden Workspace-IDs und keinen smSend-Aufruf erzeugen, während das Verhalten bei einem bzw. null Treffern unverändert bleibt; verfolge außerdem die CLI-Antworttypen und die Fehlerdokumentation in LINEAR_SETUP_GUIDE.md.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
aws, typescript
Bereich
api, backend, cli, databases, documentation
Issue-Typ
Bug
Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Aktivitätsstatus
Aktiv
Klarheit
Klar beschrieben
Anfängerfreundlichkeit
38/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.