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)

Ouverte
#883 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
adapters bug infra-cdk
Langage dominant
TypeScript
Étoiles
143
Forks
46
Merge moyen
3 j 10 h
PR mergées (30 j)
24

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.

Guide de contribution

Ouvrir le guide de contribution

Piste de recherche

Lisez cdk/src/handlers/linear-remove-workspace.ts, en particulier la recherche paginée et la protection MAX_SCAN_PAGES, puis examinez les tests existants du handler. Vérifiez que des slugs actifs en double produisent un 409 avec les IDs des workspaces en conflit et aucun appel à smSend, tandis que le comportement pour un et zéro résultat reste inchangé ; examinez également les types de réponse de la CLI et la documentation des erreurs dans LINEAR_SETUP_GUIDE.md.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
aws, typescript
Domaine
api, backend, cli, databases, documentation
Type d'issue
Bug
Difficulté
4/5
Temps estimé
3-5 jours
Activité
Active
Clarté
Clairement spécifiée
Accessibilité débutants
38/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.