feat(aws-control): remove an account from the accounts pane

[bolichen97]

Summary

A user on Discord asked: "once you add an AWS account to AWS Control… you can't remove it via the UX??" — and they were right: registration was one-way. This adds the removal path.

• Backend: new POST /api/apps/aws-control/profiles/unregister ({names: [...]}), mirroring /profiles/register. Registry-only by construction — the handler never reaches deploy.profiles.create_aws_profile (the one aws configure writer) or the AWS CLI, so the operator's AWS CLI configuration and every AWS resource (drive bucket included) are untouched. Names are validated against the shared profile pattern but not against the machine's profile list, so a stale entry whose profile was already deleted locally is removable. The registry default is re-picked when the removed profile held it.
• Consent: grants are keyed by service, so a new aws_consent.revoke_for_profile sweeps the gated services and withdraws every grant naming a removed profile — a later re-registration under the same name starts unconsented. Share / library / backup ledgers are deliberately left alone: they describe the bucket, which still exists and still bills.
• Frontend: every account row gets a ⋮ overflow menu beside the select button (not inside it, so opening the menu cannot select and jump to the account about to be removed). The one item reveals the same inline Cancel + danger strip the Files/Library folders use; it names the account (or, for the unresolved pseudo-row, the key it will forget) and states that nothing in AWS or the AWS CLI configuration changes. The strip stays open until the request resolves; a refusal renders there with the agent hand-off. Success refetches accounts, Add-accounts, and the consent receipts.
• i18n: 5 new keys under apps.awsControl.page in all 12 catalogs; remove_account_confirm registered in QUOTED_OPERAND_CONFIRM_KEYS.
• Spec: docs/system-specs/features/aws-control.md (consent section + HTTP surface).

Screenshots

Row menu (dark, 1280):

Confirm strip:

Refused removal, narrow (light, 390):

More frames

Tests

• Backend: TestProfileUnregister (10 cases: body/pattern 400s, all-unknown 404, removal + default re-pick + skipped names, last-profile empties default, grant sweep is profile-scoped, structural pin that no credential writer or CLI is reached, SEL profiles_unregister success audit); ("POST", "/profiles/unregister") added to P0_ROUTES so the disabled / non-owner / restricted-session gates cover it; revoke_for_profile unit test in test_aws_consent.py.
• Frontend: 5 cases in AwsControlPage.test.tsx (menu does not select, confirm posts every key and refetches, cancel sends nothing, refusal stays on the strip, unresolved row names its key).

Verification

tsc, eslint, jscpd, i18n:check, isort / flake8 / mypy / black — clean. Full backend suite: 89 217 passed; the 219 failures are the host's pre-existing set (218 reproduce on a clean origin/main checkout of the same files; the one extra, test_api_health::test_a_forgetful_pre_audit_refusal_is_still_audited_by_position, also fails alone on clean main). Full frontend suite: 29 473 passed; PromptsTab.test.tsx failed once in the full run and passes alone on both trees (ordering flake, untouched by this diff).

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 0451650dd20436eb5666c71dd8bb386e374f4ba2 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Established temp-screenshots/ convention confirmed (hundreds of sibling directories), spec updated in the same commit, and the endpoint mirrors the existing register path. Review complete.

Design-Verdict: PASS

A real reported gap closed with the exact inverse of the existing register path; registry-only scope, consent-sweep ordering, and failure sides are all deliberate and documented.

[DESIGN-REVIEWED] 0451650

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 0451650dd20436eb5666c71dd8bb386e374f4ba2 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All mechanical checks are done: temp-screenshots/ is a sanctioned PR-evidence convention, TileConfirm is genuine reuse, no prior removal mechanism exists, and consentWithdrawn has zero real consumers (4 files: producer, type declaration, two test files — no rendering code reads it; the register mutation's added/skipped are likewise discarded, so the pre-existing count shape is convention, but the new field goes beyond it).

First-Principles-Verdict: CONCERNS

consentWithdrawn ships to a client that never reads it — zero counted consumers; every other item traces to the Discord report or the consent boundary.

What this change ships

Intent: let an operator remove a mistakenly-added AWS account from the page that added it — an ADDITION, anchored to a quoted user report.

1. Every account row gains a ⋮ menu with one "Remove from AWS Control…" item — justified (quoted Discord report; no prior removal path exists).
2. Removal asks first via the same inline confirm strip Files/Library use — justified; reuses TileConfirm (now exported), not a second spelling.
3. New POST /profiles/unregister, registry-only by construction — justified; mirrors /profiles/register.
4. Removing a profile withdraws its paid-service consent grants — justified by the consent boundary (re-registration must not inherit authorization).
5. The registry default re-picks when the removed profile held it — justified (nightly backup resolves through it).
6. A stale entry whose profile left ~/.aws is still removable — justified.
7. Response field consentWithdrawn — zero consumers.
8. The menu item carries a muted "Nothing in AWS changes" hint — undeclared in the description (declared only in the spec hunk); harm is named.
9. Route docstring now lists the two pre-existing profile endpoints — rides along, harmless.
10. Twelve screenshots under temp-screenshots/ — sanctioned by the PR template and pruned by cleanup-temp-screenshots.yml.

Watch

• The description claims "5 new keys under apps.awsControl.page"; the diff adds 8 per catalog (account_actions, remove_account, remove_account_action, remove_account_confirm, remove_account_error, remove_account_hint, forget_key_confirm, forget_key_action). The keys themselves are each used; only the declaration is stale.

Subtractions

• Drop consentWithdrawn from the /profiles/unregister response (routes.py::_handle_profiles_unregister) and from UnregisterProfilesResult (website/src/apps/aws-control/types.ts:109) — grep consentWithdrawn: 4 files, all producer/type/tests; the page only invalidates the ['awsConsent'] query and never reads the list. The sweep itself stays; return {removed, skipped} exactly like register, and stop documenting the field in the spec hunk.

[FIRST-PRINCIPLES-REVIEWED] 0451650

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 0451650dd20436eb5666c71dd8bb386e374f4ba2 — this comment is updated in place on each push.

Review details

I've analyzed both candidates against the diff and the code the diff contains.

Candidate 1 (partial consent withdrawal across a multi-key account): The scenario requires a transient OSError/ValueError on the second revoke_for_profile call after the first already wrote — a "could/might" failure window, not a condition that occurs deterministically. More importantly, the outcome is not a defect of consequence: the handler's own design explicitly tolerates "registered-but-unconsented" as the intended safe failure state (the ordering comment), and a partial version of that is consistent with it. Withdrawing consent errs toward removing authorization, is fully reversible (a re-consent prompt), and involves no credential exposure, data loss, or corruption. On retry, revoke_for_profile("alpha") finds nothing and heals cleanly. The observable wrong outcome is a re-consent prompt on the next paid op — well below the bar for a real defect. Dropped.

Candidate 2 (committed temp-screenshots/ PNGs): Not a code defect — there is no concrete input, call path, or observable wrong runtime outcome. The files sit at repo root (outside src/kiro_crew/, so not packaged) and amount to repo hygiene, a category this review does not own. The candidate itself is "not a behavioural defect." Dropped.

No grounded Step 2 findings: the feature validates names against _PROFILE_RE, never reaches the aws configure writer or the CLI, re-picks the registry default, invalidates the cache, and is covered by tests.

No findings.

[OPUS-REVIEWED] 0451650

Verdict parsed from the review's SHA-scoped output markers for commit 0451650dd20436eb5666c71dd8bb386e374f4ba2.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 0451650dd20436eb5666c71dd8bb386e374f4ba2: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ human override accepted

Human judgment by @bolichen97 overrides the GPT 5.6 finding for 0451650dd20436eb5666c71dd8bb386e374f4ba2; the recorded reason is authoritative for this commit.

This comment is updated in place on each push.

The model was not re-run because an authorized human decision supersedes it.

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 0451650dd20436eb5666c71dd8bb386e374f4ba2: <one-sentence reason>

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

UX-level review of 0451650dd20436eb5666c71dd8bb386e374f4ba2 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Reconciliation is complete. Every control this PR adds appears in the screenshots, both themes and widths are covered, the blind reader correctly identified the overflow menu, the remove item, both confirm strips, the error state and the success state, and dared to use the primary path. Two genuine risks survive: the open dropdown visually detaches from its row (the reader couldn't tell which account it would remove), and the unresolved pseudo-row groups all orphan keys into one row, so its singular "Forget key" confirm can silently cover several keys.

UX-Verdict: CONCERNS

The remove flow reads well end-to-end, but the open row menu doesn't say which row it belongs to, and "Forget key" can silently mean several keys.

Watch

• Menu-target ambiguity: the Radix dropdown opens below the ⋯ trigger, overlapping the next row (shot-03/04). The blind reader: "I can't be fully sure which row it will remove… that uncertainty alone would make me pause before clicking." Every removal passes through this moment (high frequency, hesitation not failure — the named confirm recovers it). Smallest fix: give the owning row or trigger the same visible ring in the menu-open state (data-[state=open]) that shot-05 shows during confirm.
• Destructive scope mismatch on the unresolved row: _build_snapshot folds every orphan profile into one pseudo-row, but forget_key_confirm renders "Forget this row's key “{{name}}”" with profiles.map(p => p.name).join(', ') — with two orphans the singular sentence and the "Forget key" button forget both. Rare state, but the label under-states a destructive action's scope. Add a plural variant ("Forget this row's {{count}} keys…") keyed on profiles.length.

Suggestions

• totals_summary ("{{accounts}} accounts · …") renders "1 accounts" in this PR's own success state (shot-09/10); the blind reader tripped on it — add plural forms to the key.

[UX-REVIEWED] 0451650

[bolichen97]

Pushed 319ec7c63 addressing round 1:

GPT 5.6 (both fenced findings fixed)

• F1 — revoke_for_profile now does the profile match and the delete under one _ConsentLock over a single read/write of the store, so a grant re-recorded for another profile in the window is never the one deleted. Pinned by test_aws_consent.py::test_revoke_for_profile_withdraws_every_grant_naming_that_profile (real store, real lock).
• F2 — the handler now withdraws consent before the registry write, scoped to targets ∩ registered. A withdrawal that fails answers 500 consent_unwritable and leaves the profile registered (retryable); the reverse order was the only way to reach "unregistered but still authorized". Pinned by test_withdraws_the_removed_profiles_grants_before_touching_the_registry (asserts the call trace ["revoke:alpha", "registry"]) and test_a_failed_withdrawal_leaves_the_registry_untouched. The residual "a consent POST lands between the two writes" is inert by construction — every AWS Control call resolves account→profile through the registry and the deploy engine refuses an unregistered profile — and the grant stays visible and withdrawable on the usage receipts; no rollback was added for it.

UX Review

• Blocker — the menu item now reads "Remove from AWS Control…" with a muted second line "Nothing in AWS changes", so the reassurance precedes the click. Screenshot 02 re-captured.
• Watch — the unresolved pseudo-row gets its own confirm string, forget_key_confirm ("Forget the key “old-laptop”? …"), so the strip no longer asks about an account name the row never showed. Screenshot 06 re-captured.
• Suggestion (totals_summary plural) — left out of this PR: the sentence interpolates three already-formatted numbers, so the proper fix is a count-keyed plural key set across 12 catalogs, which is its own change.

First Principles — consentWithdrawn stays in the response: it is the only place the sweep's outcome is stated to the caller and the frontend test asserts it; the refetch alone cannot tell "withdrawn" from "never granted".

Frontend Tests (3) — the shard fails on deadKeys.test.ts (30 vs baseline 29) because main gained a dead key in #8859; this branch is rebased onto that main and inherits it. Fix is #8951 (separate one-line PR); once it lands, a re-run here goes green.

[bolichen97]

Pushed 3554a2321 addressing round 2:

GPT 5.6 F1 (fixed) — revoke_for_profile no longer goes through the fail-soft _read_all. It reads the store itself under the lock: a missing file is the ordinary no-grants case (returns []), and an unreadable or unparseable one now raises (OSError / ValueError), which the route maps to 500 consent_unwritable before the registry is touched. Pinned by test_aws_consent.py::test_revoke_for_profile_raises_on_an_unreadable_store (corrupt store → raises, bytes untouched; deleted store → []).

UX Review (both Watch items)

• Verb mismatch — new forget_key_action ("Forget key") is the strip's button when the row is the unresolved pseudo-row, so sentence and button use the same verb. Asserted in AwsControlPage.test.tsx.
• Unseen key — forget_key_confirm now anchors the name to the row: "Forget this row's key “old-laptop”? …". Screenshot 06 re-captured.
• totals_summary plural — still deferred (see round 1).

[bolichen97]

GPT 5.6 round 3 — not fixing this one; reasoning below.

The finding is the third residual of the same class: an operator confirming consent for profile A concurrently with removing profile A, then re-registering A. The adjudicator's own record (FLAG, "likely edge case") already states why the harm is bounded:

• authorize() re-probes the live account uncached on every paid call and revokes on mismatch, so a surviving grant can only ever bill the exact account the operator freshly confirmed in the racing POST — never a different one.
• The residual grant is visible on the usage receipts and withdrawable there.
• Reaching it needs the same single owner to issue two contradictory requests inside a subprocess-probe window, then re-register the same name.

Closing it "properly" means the consent confirm handler (dashboard/handlers/aws_consent.py) checking registry membership under the registry lock before recording — a new cross-module coupling between the generic consent store and one app's registry, added for a same-account, operator-self-inflicted race. That is mechanism beyond what a concrete defect requires, so I'm leaving the code as is and asking a repository writer to override:

/ai-review override gpt 3554a2321d538a62a6ed2765755d6603d66f5033: same-account edge race, harm bounded by the uncached live-account re-probe on every paid call; residual grant is visible and withdrawable

Design Review's suggestion (surface the re-picked default) is noted but deferred: First Principles already flags unread response fields, and the default is visible on the Keys card via the star.

[bolichen97]

/ai-review override gpt 0451650: same-account edge race, harm bounded by the uncached live-account re-probe on every paid call; residual grant is visible and withdrawable

[github-actions]

Human judgment recorded

@bolichen97 marked the gpt AI finding as false positive, not applicable, or explicitly accepted for 0451650dd20436eb5666c71dd8bb386e374f4ba2.

same-account edge race, harm bounded by the uncached live-account re-probe on every paid call; residual grant is visible and withdrawable

This decision applies only to this commit. A new push requires a new judgment.