Skip to content

fix(aws-control): implement the UX review debt across accounts and drive - #8716

Merged
bolichen97 merged 1 commit into
mainfrom
fix/aws-control-ux-debt
Sep 6, 2026
Merged

fix(aws-control): implement the UX review debt across accounts and drive#8716
bolichen97 merged 1 commit into
mainfrom
fix/aws-control-ux-debt

Conversation

@CrysisDeu

@CrysisDeu CrysisDeu commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Problem / Motivation

The UX Review lane raised CONCERNS on seven aws-control PRs (#5517, #6446, #6780, #7801, #7741, #7972, #7983) and each PR merged with its Watch items dispositioned as accepted-and-deferred or left unanswered. The debt is user-visible on every visit to the app: the rail says "Access" above "Accounts & credentials", the totals strip counts "2 healthy" of nothing, one thing is called keys / profiles / Connections on the same page, the pinned Actions column is a permanently tinted stripe, a file can only be moved by dragging it, and the Share dialog and breadcrumb menu ignore Escape and never return focus.

Why it matters

These are the front-door surfaces of the app (rail, accounts list, file table). Each item alone is small; together they make a first-run reader mis-navigate ("Acc…" twice), misread account health, and — for keyboard and touch users — make moving a file impossible. The lane already found them; leaving them deferred with no owner is how advisory findings ship anyway.

What changed (motivation → approach → change)

Every Watch item from the seven source PRs, with the change that answers it. Items already fixed on main (by #8320 / #8475) are listed so the reader sees the whole ledger.

#7801 — flatten the drive into an in-app rail

  • "Access" is ambiguous as first-level navaccess_title is now "Share links" (rail and pane header share the key; 13 catalogs).
  • Selecting an account row yanks the user to FilesuseAccount only sets the stored account; the reader stays on Accounts and the keys section below re-renders for the picked account. The now-unreachable push-marker branch in the row handler is deleted.
  • "Connections" scope is unlabeled → the section header is "Keys for {{account}}" (keys_for_account), naming the account whose keys it lists.

#5517 — account portal + cloud drive

  • One object, three names → "keys" everywhere a reader sees the rows; "profile" only for the literal ~/.aws name in Add accounts. Copy aligned across AwsControlPage, ConsoleView, and the consent receipt.
  • Empty state points away from its own fix → main's fix(aws-control): stop naming a Settings page that does not exist #8475 already rewrote empty_body; this PR adds the second half: when the account list is empty, Add accounts opens itself (autoOpen, edge-triggered so a reader who collapsed it keeps their choice).
  • Hand-rolled layers lose Escape and focusShareDialog now uses useDialogFocusTrap (Escape closes — except mid-mint — and focus returns to the menu that opened it — all three openers: list row, grid tile and search hit). The breadcrumb-ancestor menu is Radix DropdownMenu. The Share dialog also names the file it is about to publish (share_title → "Share “{{name}}”", 13 catalogs), matching the Move picker — a wrong-file share link is unrecoverable until it expires (UX Review, round 7).
  • Silent mutation failures (shareMut/nightlyMut/restoreMut/forgetMut/removeMut) → already on main via fix(aws-control): route every error state through the shared notice #8320 (AwsErrorNotice, "route every error state through the shared notice"). Not duplicated here; every error surface this PR adds (the Move picker's refusal) also renders through AwsErrorNotice.

#7741 — density / disclosure polish

  • "{{healthy}} healthy" has no nountotals_summary reads "{{accounts}} accounts · {{healthy}} of {{keys}} keys healthy". The health word on each row takes the status dot's colour, and the account switcher's dropdown says "Needs attention" in words for a non-healthy account, not just a dot.
  • Closing the folder disclosure strands keyboard focuscloseFolderDisclosure focuses the "New folder" toggle after the input unmounts.
  • Compact receipt hides the credential source → the compact AwsConsentGate row shows the credential source inline.
  • Also: when Cost Explorer consent is missing, the reason for the cost "—" is visible text beside the figure instead of hover-only. A failed bill read is not given a second inline sentence: that is an error surface and the AwsErrorNotice under the card already reports it with the hand-off (errors-use-error-notice).

#7972 — icon-only view toggle, borderless table

  • Pinned Actions column renders as a mismatched stripe → sticky cells and seam gradients paint the surface they sit on: LibraryTableHead takes a surface parameter (bg/card); the drive table passes bg, the library keeps card. Pinned by a test that asserts no bg-card on the drive's sticky cells. Both drive tables on the page surface pass surface="bg" — the folder listing and the search-results table.
  • The reason the view toggle is icon-only is now a code comment where the toggle is drawn (it was only in the PR thread).

#7983 — drag and drop

  • Drag is the only way to move a file → the row menu gains "Move to folder…", opening a small picker (top level, parent, and the sub-folders on screen — the same destinations a drop can reach). Keyboard and touch reachable; a 409 / share_active refusal is reported inside the picker, which stays open for another pick.
  • A move shows nothing while in flight → the source row dims (opacity-50), reports aria-busy, and says "Moving…" in words where its Modified cell (list) or size line (grid) was, so the state is not encoded by opacity alone once the picker is gone; the picker shows "Moving…" too. The picker can be dismissed at any point, mid-move included — the copy keeps running, the row stays busy until it lands, and a refusal that arrives after dismissal is reported on the pane's error strip — so a slow or hung copy never traps the reader in a modal (Design Review finding on the first head). Moves are serialized — one at a time. While a copy runs, the other rows are not draggable, their "Move to folder…" item is disabled, and a drop is ignored. This is a product decision taken after review rounds 2, 3 and 6 each surfaced another sibling of the same class (two overlapping moves sharing one busy marker / one error slot / one "picker open" flag): the original finding asked for a visible busy state, never for concurrent moves, so the concurrency class — and the per-key state it needed — is removed rather than made correct.
  • Multi-file drop failures overwrite one anothernot in this PR; still open, tracked below.

#6446 — the cloud drive becomes a page

  • folder_deleted parenthetical plural → real _one/_other keys registered in pluralKeys.json, 13 catalogs, and the line now names the folder ("Deleted “archive”: 14 items.") so a reader can tell how one click became fourteen. It is scoped to the folder the delete happened in — the deleted folder's parent, taken from the request rather than from wherever the reader is when the response lands — shows only while that folder is the current path, and leaving the folder retires it (rather than merely hiding it), so a later return does not replay it. Separately, a delete or rename that fails after its row's strip is gone has its own page-level slot AND its own notice (pageError, drive-page-error) instead of borrowing the Move picker's: sharing the slot would have shown "could not delete X" inside a picker moving Y, and sharing the notice would have let a stale drag refusal stand in for the later delete failure.
  • de.json ASCII transliterationfolder_bad_name uses real umlauts.
  • Breadcrumb-ancestor menu is a hand-rolled popover → Radix DropdownMenu (see feat: AWS Control - account portal + S3-backed cloud drive #5517 above).
  • Also: the two filename-validation sentences (drive_bad_name, folder_bad_name) say the same thing the same way.

#6780 — rebuild the Drive around real contents

  • "No artifacts match." shown when nothing was searched → an empty library says library_add_empty ("nothing here yet"), distinct from a search that matched nothing; the one-way cost disclosure reads in body tone.
  • Blank image previews in the picker → a screenshot-evidence finding on that PR, not a code defect here; out of scope.

Copy

  • "Nothing was hot-swapped." → "Your current data was not replaced." (backup_restored_note).
  • Rail "Library" → "Artifact library" so it does not collide with the dashboard's own Library entry one column over.

Not in this PR

Tests

15 new frontend tests across AwsControlPage.test.tsx, ConsoleView.test.tsx, DrivePage.test.tsx, AwsConsentGate.test.tsx, ArtifactsPage.stickyActions.test.ts:

  • a row select stays on Accounts and re-renders the keys section for that account; the keys header names the account
  • totals strip reads "N of M keys healthy"; row health word carries the status colour; switcher shows "Needs attention" text
  • Add accounts auto-opens when the list is empty and does not force itself back open once collapsed
  • Share dialog: Escape closes and returns focus to the row menu, but not mid-mint
  • Move to folder: menu item present; picker lists exactly the reachable folders; root/parent offered only when applicable; 409 reported in the picker and the page strip does not double it; "nowhere to move" when there are no folders; source row aria-busy + dimmed only while pending; Escape mid-move dismisses the picker while the row stays busy and says "Moving…" in words (gone once the move lands); the Share dialog opened from a search hit returns focus to that hit's menu; the pinned header of the search-results table paints the page surface too; a refusal arriving after dismissal lands on the page strip, not in a re-opened picker; one move at a time — while a move runs the other rows are not draggable and their "Move to folder…" item is disabled, and the gate reopens when the move settles
  • focus returns to the row menu even when the pointer opened it without focusing it (Safari's pointer-click behaviour)
  • a failed costs read shows no inline reason string — only the costs-error notice (the consent-missing reason stays inline, it is status not failure)
  • folder_deleted renders a real plural with no (s) and names the folder; the count attaches to the folder the delete happened in even when the reader navigated during the request, and leaving that folder retires it; a delete that fails while the Move picker is open lands on the page strip, not inside the picker; a stale drag refusal does not stand in for a later delete failure (two notices)
  • closing the folder disclosure returns focus to "New folder"
  • empty library shows library_add_empty, not library_add_none
  • drive table sticky cells paint bg-bg, never bg-card; the library table keeps its card surface
  • compact consent receipt includes the credential source

Locally: tsc -b --force, eslint on touched files, npm run i18n:check, src/i18n style tests, and every touched vitest file (47 files, 863 tests) green after rebasing onto 6d1b51704.

Manual verification

Captured on a Vite dev server with the API answered from fixtures (website/scripts/capture-aws-control-ux-debt.mjs, same harness as #8320's capture-aws-control-errors.mjs), once on main and once on this branch. Walked: rail labels, totals strip, keys section header, Share links pane, row menu → Move to folder… → picker, row menu → Share dialog.

Screenshots / video

Accounts pane — rail, totals strip, keys section (before → after: "Access"→"Share links", "Library"→"Artifact library", "3 keys · 2 healthy"→"2 of 3 keys healthy", "Connections"→"Keys for prod-main")

accounts before
accounts after

Files — row menu → Move to folder… (before: Download / Share / Delete only, and the pinned Actions column is a tinted stripe; after: the picker, on a table whose sticky cells paint the page surface)

move before
move after

Share links pane and Share dialog (before / after; the after-frame is the current head's, with the file name in the title)

share links before
share links after
share dialog before
share dialog after — names its file

States the UX lane asked to see on this branch (all "after"; captured by website/scripts/capture-aws-control-ux-debt-more.mjs)

Row menu with "Move to folder…" · a move in flight (source row dimmed and aria-busy, picker says "Moving…") · the other row's menu while that move runs (Move disabled — one move at a time) · a refused move reported inside the picker

row menu open
move in flight
move disabled while busy
move refused

No accounts yet (Add accounts auto-expanded; the orphan consent receipt also shows the credential source) · account switcher open ("Needs attention" in words) · Usage pane with the consent-missing cost reason inline and the compact S3 receipt's credential source · Add-from-Artifacts on an empty library · the Share dialog naming its file

accounts empty
switcher open
usage consent
library add empty
share dialog named

Move picker opened two levels down ("Files (top level)" · "Up to contracts" · the sub-folder) · picker at a top level with no folder (move_no_folders) · picker dismissed mid-move: the dimmed row says "Moving…" · a folder deleted ("Deleted 14 items.") · a backup restored ("Your current data was not replaced…" + staging path)

move picker nested
move no folders
move row moving
folder deleted
backup restored

Search results (pinned header on the page surface, same as the folder table) · a refusal arriving after the picker was dismissed, on the pane's strip · the Share dialog mid-mint ("Creating…", Close dimmed, Escape held)

search results
move refused on strip
share mid-mint

Breadcrumb ancestor menu open (shared dropdown, two levels down) · grid view mid-move: the tile says "Moving…" and its neighbour's Move item is disabled

crumb menu open
grid moving

Related Issues

no linked issue: this closes out the UX Review Watch ledger on #5517, #6446, #6780, #7801, #7741, #7972 and #7983 directly; no issue was filed for those dispositions.

Pattern harvest

Rule candidate: review-prompt
Pattern: an advisory UX Watch item dispositioned "accepted-and-deferred" with no issue, owner or date is debt with no owner — the disposition contract now requires all three (deferred-finding label, assignee, Due:), which is exactly the gap that let these seven PRs' items sit.

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable) — docs/system-specs/features/aws-control.md
  • No secrets, credentials, or internal references in the diff

@CrysisDeu
CrysisDeu requested a review from a team September 5, 2026 09:05
@CrysisDeu
CrysisDeu requested a review from a team as a code owner September 5, 2026 09:05
@CrysisDeu
CrysisDeu requested a review from cixuuz September 5, 2026 09:05
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — 🟡 CONCERNS

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

I have everything I need: the blind read validated the PR's main-path controls, and I've reconciled every added/changed control against the report, the diff, and the screenshots. Two details stand out — the disabled "Move to folder…" item gives no reason (shot-11), and the committed 04-share-dialog-after.png still shows the old unnamed "Share this file" title even though the branch renders the named title (confirmed in shot-17 and share_title in en.json).

UX-Verdict: CONCERNS

Cold reader understood every main-path control — move flow, renames, busy states — but a disabled Move item is unexplained and one "after" screenshot is stale.

Watch

  • Disabled "Move to folder…" during another file's move gives no reason. Reader (shot-11): "I can't tell WHY it's disabled — nothing says so." Low frequency, friction only, every concurrent attempt — add a reason on the disabled item (e.g. a muted "Moving…" suffix or title).
  • 04-share-dialog-after.png is stale: it shows the old "Share this file" title while the branch renders "Share "Q3-report.pdf"" (share_title, shot-17). It contradicts the PR's own evidence and misled the blind reader ("doesn't name the file — I have to trust I clicked the right row"). Recapture.
  • "Ask the agent" on both new move-refusal surfaces left the reader hesitant twice (shots 12, 24): "I don't know what the agent would do (overwrite? rename?)". Shared pattern, but this PR gives it two new placements; a hint of what the hand-off does would close it.

Evidence gaps

  • Changed error strings shown in no screenshot: drive_bad_name/drive_bad_name_named (new safe-characters sentence) and add_accounts_cap_reached ("key limit") — a bad-name upload capture and a cap-reached capture would close both.

Suggestions

  • move_no_folders ("Create one first.") is a dead end — reference or embed the existing New folder control in the picker.
  • library_add_oneway "Copies here fill cloud storage." was promoted to body tone but the reader could only guess it; say the consequence plainly, e.g. "Files copied here count toward this account's cloud storage."

[UX-REVIEWED] 7b7b832

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 7b7b832c4e4f3bfb6efab5cd5b35a2fea64c7ce0 and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- website/src/apps/aws-control/DrivePage.tsx:1912 -- deletion completing after navigation does not retrigger "[path]", so returning replays retired status -> Fix: rerun retirement when deleted changes.
[GPT-REVIEWED] 7b7b832

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

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

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

Design-Verdict: CONCERNS

Sound debt-paydown, but the Share dialog's mid-mint lockout rebuilds the exact modal trap the Move picker was redesigned to avoid.

Watch

  • Share dialog while minting: Escape is refused, Close is disabled, and backdrop dismiss goes through the same refusing close() — so a hung presign request (mutations have no timeout) leaves the reader with no exit but a page reload. The PR's own Move-picker rationale ("a slow or hung copy never traps the reader in a modal") applies unchanged here: allow dismissal mid-mint and report the late-landing link on the Share links pane / page strip, the same shape already built for moves.

Suggestions

  • The menu-opens-dialog handshake is three wiring points per site (rememberMenuOpener on the trigger, onCloseAutoFocus={skipRestoreIfDialog}, the deferred openShare/openMove), now repeated at three menus; a fourth site that misses one strands focus silently. Fold it into one shared trigger/menu wrapper so the invariant is structural, not conventional.

[DESIGN-REVIEWED] 7b7b832

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 7b7b832c4e4f3bfb6efab5cd5b35a2fea64c7ce0 — 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 evidence is gathered: the patch decomposed, consumers counted (PINNED_SURFACE/surface: 2 call sites pass bg, library default card used at 2; new i18n keys all consumed; useDialogFocusTrap is the established shared hook, 28 files), conventions checked (screenshots/capture scripts are documented deliverables; spec updated same commit per AGENTS.md). Final review:

First-Principles-Verdict: PASS

Seven recorded review findings paid down, with the standout items being deletions: the move-concurrency class, a dead navigation branch, and a hand-rolled popover.

What this change ships

Intent: make the aws-control app's front-door surfaces (rail, accounts list, file table) answer the seven UX-review reports that merged as deferred debt — a FIX.

  1. Rail entry "Access" renamed "Share links", 13 catalogs — justified
  2. Selecting an account row stays on Accounts; unreachable push-marker branch deleted — justified
  3. "Connections" → "Keys for {account}"; keys/profile wording aligned page-wide — justified
  4. Row menu gains "Move to folder…" picker, the keyboard/touch path a drag never offered — justified
  5. Concurrent moves removed: one move at a time, other rows locked meanwhile — justified subtraction
  6. Moving row dims, says "Moving…"; picker dismissible mid-move, refusal falls to page strip — justified
  7. Share dialog and crumb menu gain Escape/focus-return via shared hook and shared Radix menu; dialog names the file — justified
  8. Totals strip reads "{healthy} of {keys} keys healthy"; switcher spells non-healthy state in words — justified
  9. Empty account list auto-opens Add accounts, edge-triggered — justified
  10. Pinned Actions column paints the page surface (surface param, both variants consumed); delete/rename failures get their own notice slot; folder-deleted line pluralized, named, scoped to its parent — justified

More items exist (inline cost-dash reason, compact consent credential source, library empty-vs-no-match split, catalog polish, screenshots and capture scripts per repo convention); each traces to a named source finding or documented convention.

Every item carries a nameable harm from a cited report, reuses the existing mechanism where one exists (useDialogFocusTrap, ui/dropdown-menu, AwsErrorNotice, the plural-key registry), and the two capability changes point subtractive rather than additive.

[FIRST-PRINCIPLES-REVIEWED] 7b7b832

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 7b7b832c4e4f3bfb6efab5cd5b35a2fea64c7ce0 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 7b7b832

Verdict parsed from the review's SHA-scoped output markers for commit 7b7b832c4e4f3bfb6efab5cd5b35a2fea64c7ce0.

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

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 5, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

First Principles Watch — menuOpenedDialogRef + skipRestoreIfDialog re-spells the menu→dialog focus suppression ChatSidebar.tsx already hand-rolls — disposition: accepted-and-deferred, tracked in #8722 (deferred-finding, assigned, Due 2026-09-19).

menuOpenedDialogRef + skipRestoreIfDialog (DrivePage.tsx) is a second hand-rolled spelling of "suppress Radix's trigger-restore because a menu item opened another layer" — ChatSidebar.tsx:2592 (suppressMenuRestoreRef) already spells it; grep onCloseAutoFocus hits 8 files, each solving it locally. The general fix (a helper on ui/dropdown-menu) is larger than this change — accepted-and-deferred, but this PR grows the count.

Agreed on both counts: the pattern is duplicated, and the shared helper belongs in a change that also migrates ChatSidebar and the other onCloseAutoFocus sites, which is wider than paying down this app's UX ledger. #8722 names the helper shape, the two sites to migrate first, and the ordering note the next site would otherwise re-derive. The Design lane's Suggestion on the same head asks for the same hoist and is covered by the same issue.

@CrysisDeu
CrysisDeu force-pushed the fix/aws-control-ux-debt branch from b0cb8ff to 176d1b6 Compare September 5, 2026 13:04
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 5, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Design Watch — MoveDialog refuses every dismissal path while moveMut.isPending — disposition: fixed in 176d1b6.

MoveDialog refuses Escape, the close button, and the backdrop while moveMut.isPending ("refuses Escape until the outcome is known"), yet the PR's own comment expects long pendings ("a server-side copy of a large object leaves the row otherwise inert"). Cause → mechanism → consequence: a large or hung copy → all dismissal paths return early → the reader is trapped in a modal for minutes with no cancel. The share dialog's rationale (a minted URL would be silently lost) does not transfer here: the page already dims the source row and the error strip already reports a move outcome when moveTarget is null, so allowing dismissal mid-move loses nothing.

Agreed — nothing is lost by dismissing. DrivePage.tsx now routes every close (Escape, X, backdrop, landed move) through one closeMoveDialog that is never refused; the X button is no longer disabled while pending; moveDialogOpenRef lets a late onSuccess skip the focus hand-back when the reader already left. The source row stays aria-busy until the move lands, and a refusal that arrives after dismissal renders on the pane's drive-move-error strip (the strip is suppressed only while the picker is open). Tests: "the source row dims and reports busy while its move is in flight, and only then" now asserts Escape mid-move closes the picker while the row stays busy; new "a move refused after the picker was dismissed is reported on the page strip instead". Spec sentence added in docs/system-specs/features/aws-control.md.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

First Principles Watch — undeclared setDeletedCount(null) on folder open — disposition: fixed (description) as of the body accompanying 176d1b6.

Undeclared: setDeletedCount(null) on folder open (DrivePage.tsx) clears the "Deleted N items" line on navigation — a real micro-fix riding along that the description never mentions.

Correct — it was in the diff and not in the ledger. The body's #6446 entry now states it: the "Deleted N files" line is cleared when the reader opens another folder, because it described the folder they just left. The code is unchanged; only the description was incomplete.

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision readiness: checking Automated validation is still running and removed readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision labels Sep 5, 2026
@CrysisDeu
CrysisDeu force-pushed the fix/aws-control-ux-debt branch from 176d1b6 to 0e50bd2 Compare September 5, 2026 14:00
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author
  • span=c4d0d28de45e — website/src/apps/aws-control/DrivePage.tsx — a second drag-move started mid-flight un-dims the first row — disposition: fixed in 0e50bd2.

a second drag-move (moveIntomoveMut.mutate, no pending guard) started while the first is in flight overwrites moveMut.variables, so movingKey = moveMut.isPending ? moveMut.variables?.fromKey ?? null : null flips to the new key and the first source row drops aria-busy/opacity-50 while its copy is still running → Fix: track in-flight source keys in a set (add in onMutate, remove in onSettled) and test each row's key for membership instead of reading the single moveMut.variables.

Correct: moveMut.variables is the latest call's input, so two overlapping copies shared one busy marker. The single movingKey is replaced by movingKeys: ReadonlySet<string> maintained on the mutation itself — a key enters in onMutate and leaves in onSettled (success or failure), and both the list row and the gallery tile test membership. Pinned by the new test "a second move started mid-flight does not un-dim the first row", which starts two moves against two held promises and asserts each row stays aria-busy until its own copy settles.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 5, 2026
@CrysisDeu
CrysisDeu force-pushed the fix/aws-control-ux-debt branch from 0e50bd2 to c69f653 Compare September 5, 2026 14:30
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 5, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author
  • span=43353d2ebf0c — website/src/apps/aws-control/ConsoleView.tsx — query failure bypasses the shared error surface — disposition: fixed in c69f653.

<span ...>{i18nT('apps.awsControl.console.costs_unavailable')}</span>
Cost rejection -> costsQ.isError -> error text renders outside AwsErrorNotice.
Anchor: errors-use-error-notice
Fix: Remove this span; the existing AwsErrorNotice reports the failure.

Correct, and the adjudication's reading of the rule's scope text is the right one: costsQ.isError is a failed useQuery, so its sentence belongs to the notice, not to an inline span beside it. The isError span is removed; the costs-error AwsErrorNotice (with Retry and the hand-off) is the one place the failure is said. The consent-missing reason stays inline because it is status about a gate that has not been granted, not the outcome of a failed request — the rule's own exclusion. ConsoleView.test.tsx now asserts the inline reason slot is empty on a failed read and the notice carries the sentence; the PR body's cost-reason line is corrected to match.

@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Sep 6, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

FP Watch — focus-return patched 2 of the 3 menus that open ShareDialog — disposition: fixed in 21f2bf9.

The search-results menu still calls raw setShare (DrivePage.tsx:2630) with no rememberMenuOpener/onCloseAutoFocus, so ShareDialog.onClose now calls returnFocusToOpener() with a stale or null opener on that path — the same root cause, made slightly worse there.

Correct, and a completeness gap in this PR's own fix rather than a new design: returnFocusToOpener() on close is what this PR added, so the third opener has to remember itself. The search-hit trigger now carries onPointerDown/onKeyDown={rememberMenuOpener}, its content onCloseAutoFocus={skipRestoreIfDialog}, and Share goes through openShare(hit.key) — the same three lines as the list row and grid tile. New test: the Share dialog opened from a search hit hands focus back to that hit's menu on Escape. (The hoisting / Radix-Dialog alternative for the plumbing itself stays in #8722.)

FP Watch — surface="bg" passed to one of two LibraryTableHead sites — disposition: fixed in 21f2bf9.

The search-results table (DrivePage.tsx:2566) keeps the card default on the same page surface, and the new test scopes to drive-listing so it cannot see it.

Also correct: the search table sits directly on the page surface with no card wrapper. It now passes surface="bg", and the stripe test continues into search mode and asserts the search header's pinned cell paints bg-bg, not bg-card. Frame 18-search-results-after in the body.

FP Subtraction — fold capture-aws-control-ux-debt-more.mjs's fixtures into the base script's constants — disposition: rebutted (disproportional).

~60 lines re-declared from capture-aws-control-ux-debt.mjs; two drifting copies of one stub world is the only duplication this diff adds.

The two scripts stub different worlds on purpose: the first is a before/after pair on one flat listing; the second carries mode switches, path-keyed nested listings, a late-409 and a hanging mint, backup remotes and a deletable folder. Sharing the base constants would need a third module plus per-script overrides, for two evidence generators that are run by hand and never by CI, in a directory whose convention (407 scripts) is self-contained files. Drift between them cannot break anything a user sees. Kept separate.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

UX Watch — move_no_folders names the fix but offers no way to do it — disposition: accepted-and-deferred#8722 (deferred-finding, assignee, Due 2026-09-19; task added to its body).

Add a New-folder action to the picker's empty state (or focus New folder on close).

Legitimate, and a new affordance with a design choice in it (an action inside a picker that today has no write actions, vs. handing focus to New folder on close). This PR's Move work adds the missing move path for keyboard/touch users; what the picker does when there is nowhere to move is a follow-up, tracked with the two items below.

UX Watch — "Ask the agent" is the only visible exit on a move refusal — disposition: accepted-and-deferred#8722.

The refusal sentence could name the safe next step ("pick another folder or rename it") so the hand-off isn't the only visible exit.

Agreed on the copy; it is a 13-catalog wording change on the shared refusal strings plus a decision about which sentence carries it. Tracked in #8722 alongside the empty-state item so the picker's copy is revised in one pass.

UX Watch — disabled "Move to folder…" carries no reason during a serialized move — disposition: accepted-and-deferred#8722.

A short suffix ("another move is running") on the disabled item removes the guess.

Agreed; needs a new key ×13 and a disabled-item affordance the shared dropdown does not currently have. Same follow-up.

Evidence gaps — both captured on 21f2bf9 and added to the body's second <details> block: a 409 that arrives after the picker was dismissed, reported on the pane's error strip (19-move-refused-on-strip-after); and the Share dialog mid-mint — "Creating…", Close dimmed, Escape held (20-share-mid-mint-after).

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 6, 2026
@CrysisDeu
CrysisDeu force-pushed the fix/aws-control-ux-debt branch from 21f2bf9 to cb6dcd0 Compare September 6, 2026 04:11
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 6, 2026
@CrysisDeu

CrysisDeu commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator Author
  • span=a096c30f1f49 — GPT FINDING — navigating during a folder deletion attaches the count to the destination folder — disposition: fixed in cb6dcd0.

navigating during deletion makes "inPath: path" attribute the count to the destination folder -> Fix: derive inPath from the mutation's folder argument.

Correct. folderDeleteMut.onSuccess now takes inPath from the request's folder (its parent path) and the folder's own name, not from path at response time. New test: the reader walks into docs while invoices is being deleted; the count is absent in docs and waiting at the top level when they return.

@CrysisDeu

CrysisDeu commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator Author
  • span=c4d0d28de45e — Opus FINDING — deleteMut.onError / renameMut.onError write the Move picker's error slot — disposition: fixed in cb6dcd0.

deleteMut.onError (1986) and renameMut.onError (2150) write the shared setMoveError slot the Move picker owns; with the picker open (moveTarget set) the page strip suppresses it (error={moveTarget ? undefined : moveError?.error}, 2459) while MoveDialog renders it.

Correct — reachable when a delete's confirmation has moved to another row and the picker is open for a third file when the delete fails. Page-level delete/rename failures now have their own pageError slot (cleared on the next delete/rename attempt); moveError is move-only and is the only thing the picker renders. The page strip shows moveTarget ? pageError : moveError ?? pageError, so a delete failure is never held back by an open picker and never appears inside it. New test locks it: a delete failing while the picker is open lands on the strip naming the file, and move-error is absent.

@CrysisDeu

CrysisDeu commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator Author
  • span=c4d0d28de45e — Opus FINDING — deleted is never reset, so "Deleted N items" replays on every return to that folder — disposition: fixed in cb6dcd0.

deleted is only ever set in folderDeleteMut.onSuccess and never reset, so deletedCount = deleted && deleted.inPath === path ? deleted.count : null re-satisfies on return and re-renders "Deleted N items" every time the reader re-enters that folder for the component's life.

Correct; the earlier wording "leaving retires it" described hiding, not retiring. A useEffect on path now clears deleted once the current path is not the folder it belongs to, so the line shows in that folder and is gone for good after the reader leaves it. Covered by the same new test (leave, return, leave again, return: absent).

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

UX Watch — "Ask the agent" carries no outcome clause — disposition: rebutted (out of this PR's scope).

Smallest fix: one clause of outcome next to the link ("opens a chat about this error; nothing changes until you approve").

AwsErrorNotice and its hand-off link are an inherited component shared by every error surface in the app; this PR adds two more call sites of it and does not change the component. The clause you propose is a good one, but it belongs on the component (one place, every surface) — changing it here would be a cross-app copy change ridden in on a drive-page PR. Not taken here.

UX Watch — picker rows commit on click; mid-move disabled state is opacity-only — disposition: rebutted.

Reversible, so no confirm needed; consider a visible disabled cue beyond opacity-50.

While a move runs the picker's own body says "Moving…" (move-pending) directly above the dimmed rows, so the disabled state is already named in words on the same surface; the round-8 fix put the same words on the dimmed row once the picker is gone. A further per-row cue would say the same thing a third time.

Evidence gaps — both captured on cb6dcd0 and added to the body: the breadcrumb ancestor menu open two levels down (21-crumb-menu-open-after), and the grid view mid-move with the "Moving…" tile and its neighbour's disabled Move item (22-grid-moving-after).

Suggestion — folder_deleted should name the folder — disposition: fixed in cb6dcd0: "Deleted “archive”: 14 items." in all 13 catalogs (each with its own quotation marks), pseudolocale regenerated; the count line test asserts the name. Frame 16-folder-deleted-after re-captured.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 6, 2026
@CrysisDeu
CrysisDeu force-pushed the fix/aws-control-ux-debt branch from cb6dcd0 to 4be6145 Compare September 6, 2026 04:38
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 6, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

UX Watch — the picker's refusal notice wraps around the right-floated "Ask the agent" link; the hand-off should name its scope — disposition: accepted-and-deferred#8722 (deferred-finding, assignee, Due 2026-09-19).

Stack the action below the message in narrow containers, and let the hand-off name its scope ("Ask the agent to resolve this name clash").

Both halves are AwsErrorNotice, the shared notice every error surface in this app renders; its layout and its hand-off label are not this PR's to change piecemeal from one call site. Recorded in #8722 next to the round-9 item on the same hand-off link ("Ask the agent" is the only visible exit) so the notice is revised once, for every surface: a stacked layout below a max-w-sm breakpoint and a scoped label.

UX Watch — library_add_oneway "Copies here fill cloud storage." is emphasis without comprehension — disposition: rebutted.

Reword, e.g. "Copies added here use cloud storage, which AWS bills."

The sentence is the one-way-door line the earlier UX rounds on the source PRs asked for, promoted to body tone in this PR because it was the dialog's only cost disclosure and had been styled as a caption. Its content — that a copy occupies storage in the reader's own bucket — is what the dialog is about; the proposed rewording adds "which AWS bills", which is true of every object on this page and is what the Usage & costs pane exists to show. Wording on a line that eleven rounds of this lane have already passed is not changing at round 12.

Evidence gapsdrive_bad_name/drive_bad_name_named, add_accounts_cap_reached, files_empty_body, library_add_one — disposition: rebutted (evidence stands as tests).

These are copy-only changes to strings whose surfaces this PR does not restyle: the bad-name upload error (drive-upload-error), the empty Files body (drive-empty), the Accounts cap line and the Add-from-Artifacts button. Each string is exercised by a unit test that renders the surface and asserts the rendered text, and the PR body carries 22 frames of every surface this PR does restyle. A screenshot of an unchanged component showing a sentence the test already pins adds no evidence the test does not.

Suggestion — a "New folder" action inside the empty picker — already accepted-and-deferred in the round-9 disposition (see #8722, "Move picker / refusal copy"). No change here.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 6, 2026
@CrysisDeu
CrysisDeu force-pushed the fix/aws-control-ux-debt branch from 4be6145 to 7b7b832 Compare September 6, 2026 05:08
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author
  • span=a50d2b7dd622 — GPT BLOCKING — stale move errors suppress later mutation failures — disposition: fixed in 7b7b832.

const stripError = moveTarget ? pageError : moveError ?? pageError — drag-move refusal -> later delete/rename fails after its inline strip closes -> stale move error hides the new failure. Fix: Render pageError separately; show moveError only while the picker is closed.

Correct, and introduced by the previous round's fix (the shared stripError precedence). The two slots now have two notices: drive-move-error renders moveError only while the picker is closed (as before this PR's round 11), and a new drive-page-error renders pageError unconditionally — neither can stand in for the other. The five page-level delete/rename tests now assert on drive-page-error, and a new test locks the exact sequence in the finding: a refused drop leaves its refusal on screen, a delete then fails after the view swapped to search, and the failure appears on its own notice naming the file. Span a50d2b7dd622 has now recurred twice on this PR (round 6, round 12) on unrelated instances; noted for the stall counter.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

UX Watch — disabled "Move to folder…" gives no reason — disposition: accepted-and-deferred#8722 (already recorded there from the round-9 disposition: "a reason on the disabled Move item"; the title/hint form you propose is noted in the task).

UX Watch — mid-move close reads as maybe-cancel — disposition: accepted-and-deferred#8722 (task added).

Extend move_moving in the picker to say closing won't cancel it.

Agreed on the copy; it is a 13-catalog change to the picker's pending line, and the picker's copy is being revised in one pass under #8722 (empty state, refusal wording, disabled reason). Added there as "Moving… — closing this does not cancel it."

UX Watch — destination click commits with no confirm — disposition: rebutted, as the lane itself frames it: the design (no confirm, visible "Moving…" state, trivially reversible move-back) is the intended one and the lane is flagging the reader's hesitation for a human, not asking for a confirm dialog. Left as is; a maintainer reading this PR sees the note here.

Suggestion — "New folder" inside the empty picker — already accepted-and-deferred (round 9, #8722). No change.

@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 6, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Design Watch — the Share dialog refuses dismissal mid-mint — disposition: rebutted (same item the UX lane raised on head 3ccc844; the round-8 disposition stands, restated here for this lane).

allow dismissal mid-mint and report the late-landing link on the Share links pane / page strip, the same shape already built for moves.

The Move picker may close mid-flight because a move's outcome lands on the row: nothing the reader needs is lost with the dialog. A mint's outcome is a URL, and the Share dialog is the only surface that ever shows it — the Share links pane stores the record and, by design, never the URL (ShareRecord: "The URL itself is never here"), so "report the late-landing link on the pane" is not available without changing that contract. Letting the request complete into an unmounted dialog would create a live, externally valid link the reader never saw and cannot retrieve: a silent external publish, which is a worse failure than a lock bounded by one request. The honest alternatives — hoisting the mutation with a URL-bearing page-strip success state, or an abort that also revokes a link that already minted — are new surface, not a fix to what this PR ships, and the lock has been stated in the body since round 1 ("Escape closes — except mid-mint") and passed the Design lane on every earlier head. Kept.

Design Suggestion — fold the three-point menu-opens-dialog handshake into one shared wrapper — disposition: accepted-and-deferred#8722 (deferred-finding, assignee, Due 2026-09-19). That issue's "Done when" is exactly this: one exported helper replacing the per-site rememberMenuOpener / onCloseAutoFocus / deferred-open wiring in DrivePage.tsx and ChatSidebar.tsx, with the Radix-Dialog migration weighed as the alternative. This PR's third site (the search-hit menu, round 10) is why the issue names the count.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

UX Watch — disabled "Move to folder…" gives no reason — already accepted-and-deferred (round 9 and round 12 dispositions; #8722 carries the task including the title/suffix form).

UX Watch — 04-share-dialog-after.png is stale (old "Share this file" title) — disposition: fixed in the PR body (no code change): the before/after pair now uses frame 12-share-dialog-named-after — captured on this branch, "Share “Q3-report.pdf”" — as its after-frame, and the <details> summary says so. The old frame predated the round-8 share_title change; thank you for catching the contradiction.

UX Watch — "Ask the agent" on the two new refusal surfaces — already accepted-and-deferred (round 11; #8722: scoped hand-off label on the shared AwsErrorNotice, one change for every surface).

Evidence gaps — drive_bad_name/drive_bad_name_named, add_accounts_cap_reached — disposition unchanged from round 11: rebutted; copy-only changes on surfaces this PR does not restyle, each rendered and asserted by a unit test.

Suggestions — New folder inside the empty picker; library_add_oneway wording — unchanged: the first is deferred (#8722, round 9), the second rebutted (round 11).

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author
  • span=a096c30f1f49 — GPT FINDING — deletion completing after navigation does not retrigger the [path] effect, so returning "replays retired status" — disposition: rebutted (the described behaviour is the intended one, and nothing retired is replayed).

deletion completing after navigation does not retrigger "[path]", so returning replays retired status -> Fix: rerun retirement when deleted changes.

The sequence in the finding is: delete invoices at the top level, walk into docs before the response lands, the response lands (deleted = {inPath: ''}) while the reader is in docs, the reader returns to the top level and sees "Deleted “invoices”: 12 items." That count was never shown before — the reader left before it arrived — so there is no retired status to replay; the line appears once, in the folder the delete happened in, which is exactly what the round-10 GPT finding (same span, inPath from the request rather than the path at response time) asked for. Leaving that folder afterwards retires it, and a later return shows nothing — the new test walks precisely this path (leave before the response, return, see it once, leave, return, gone). Re-running retirement on deleted changes, as proposed, would retire the count the instant it arrives whenever the reader is elsewhere, so a slow delete would never be reported at all; that is the opposite of the surface's purpose. Kept as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants