fix(sidebar): surface folder-create failures inline (#8229) - #8547
Conversation
Design Review (Fable 5) — 🟡 CONCERNSDesign-level review of Established conventions confirmed — 425 capture scripts and a committed Design-Verdict: CONCERNS Sound fix for a real silent-failure, but the notice's four-mount ownership matrix duplicates folder-visibility logic that will drift. Watch
Suggestions
[DESIGN-REVIEWED] df8b554 |
UX Review (Fable 5) — ✅ PASSUX-level review of UX-Verdict: PASS A previously silent failure now lands inline, names the exact stale path, and offers the remedy — comprehensible, actionable, and dismissible. Suggestions
[UX-REVIEWED] df8b554 |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of All checks complete. The change is unusually well-scoped: every fallback mount guards a reachable state (the New-menu folder picker at First-Principles-Verdict: PASS A silent, rollback-masked create failure now renders where it happened; every mount, message, and rider is declared, derived, and reuses the shared error surface. What this change shipsIntent: let a user see why a new chat in a folder vanished, and where to fix it — a FIX.
Sibling count: 0 remaining console-only [FIRST-PRINCIPLES-REVIEWED] df8b554 |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsFINDING -- website/src/pages/ChatSidebar.tsx:6479 -- False positive or not applicable? A repository writer can comment: |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsTwo new inline-notice fallbacks misfire: a duplicate alert in board view and a silent failure under a collapsed folder. FINDING — website/src/pages/ChatSidebar.tsx:6479 — the board lane-level fallback fires when FINDING — website/src/pages/ChatSidebar.tsx:4971 — [OPUS-REVIEWED] df8b554 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
df3f94f to
7edb663
Compare
|
🤖 Kiro Crew Auto-Pipeline [operator: CrysisDeu#0c98c3a2] Status: functionally review-ready. Everything this PR owns is green at head The remaining reds are inherited from main, not from this diff (which touches only
Per the repo's cross-merge discipline this branch will not be patched or rebased for main-side breakage; I am watching main's recovery and will report back when the inherited reds clear. Human review of the diff can proceed now — the change itself is fully verified. |
7edb663 to
ef0d3f5
Compare
ef0d3f5 to
7549413
Compare
7549413 to
d3e16c3
Compare
|
Both UX suggestions from the
|
|
Rebutted (disproportionate remedy, twice-adjudicated). The
The single-latest-attempt scalar is the deliberate, documented design (comment at the state declaration): it exists to stop a stale notice resurrecting, the failure is still |
|
/ai-review override gpt d3e16c3: this round's own Opus adjudication ruled DOWNGRADE disproportionate-remedy for the 4562 finding (same span downgraded last round too) but its verdict lines were indented into a code block so the parser counted 0 well-formed verdicts and the block stood by default — adjudication formatting fault, not a code defect. |
Human judgment recorded@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for
This decision applies only to this commit. A new push requires a new judgment. |
d3e16c3 to
80e2ef7
Compare
1c056d0 to
13e2d34
Compare
|
Fixed in
Exactly one mount renders in every cell (the fallback condition is false whenever a scoped mount is live), so no duplicate |
13e2d34 to
634517b
Compare
|
Both First Principles watch items on
|
634517b to
df8b554
Compare
|
Fixed in Completeness argument — the notice's owning mount is a function of exactly three variables, each now total:
Every lane has a fallback, every fallback condition is the complement of its scoped mount's render condition, and |
Problem / Motivation
Clicking the per-folder + button in the chat sidebar when the folder's
project_dirno longer exists on disk makes a new session row flash into the list and vanish about a second later — no toast, no inline error, no folder marker. The only trace is a DevTools console line (Failed to create chat in folder: ApiError: Not a directory). In the reporter's case a renamed parent directory made every folder fail identically, so the sidebar looked completely broken with no explanation.Why it matters
The rollback is correct behaviour (an unscoped session must not be published), but a user who watched the row appear and disappear has no way to learn why — or that the fix is one setting away in the folder's own menu. Anyone who renames or removes a project directory hits this, and the failure mode reads as data loss.
What changed (motivation → approach → change)
createSlotdeletes the session server-side and rethrows when the project-scope POST is refused with HTTP 400{"error": "Not a directory"}(api_chat_slot_project, correct backend behaviour). The sidebar'screateChatInFolderMutation.onErroronlyconsole.error'd.folders.jsonpaths is a design decision and out of scope;createSlot's rollback contract is untouched):createChatInFolderMutation.onErrornow maps the failure to a user-facing message and stores{folderId, columnId, message}state. The specific 400Not a directorycase gets an i18n message naming the folder's resolved project path (resolveFolderProjectDir) and pointing at Folder settings (⋯ menu); anything else falls back toerr.messagelike the sibling notices.createAsyncThunkserializes thrown errors down to{name, message, stack}, so the mapping matches both the liveApiErrorinstance and the serialized shape (verified against the real built SPA).ErrorNoticesurface (AUTOSDEerrors-use-error-notice) directly under the folder's header row, in list and board view, withrole="alert", the ⚡ agent hand-off (askAgent— nothing to destroy here: the sidebar holds no unsaved draft and survives the navigation), and the ✕ dismiss.flex-wraplets the message take the full line in the ~250px drawer with the controls folding under it.pages.chatSidebarkeys translated in all 12 catalogs (sorted placement),en-XAregenerated.Tests
website/src/test/ChatSidebarW3Coverage.test.tsx:logs a failed folder-scoped create instead of failing silently— the inline notice renders with the generic message androle="alert";names the stale project directory when the backend refuses the folder scope—chatSlotProjectrejectsApiError(400, 'Not a directory'); the notice names the folder's resolved project path (locks in the serialized-error mapping);clears the inline notice on the next successful create in the folder.Manual verification
website/scripts/capture-folder-create-stale-dir.mjs(committed) runs the REAL built SPA against the shared fixture stub, drives the failing create end to end, and asserts: create → 400-refused project POST → rollback DELETE all fired; the notice renders, is an alert, names the stale path; ✕ dismisses it. All assertions pass; the screenshots below are its output.Screenshots / video
Folder create fails → inline notice names the stale directory and where to fix it, with the agent hand-off:
Before / after dismiss
Related Issues
Closes #8229
Pattern harvest
Rule candidate: review-prompt
Pattern: "mutation onError that only console.errors — a rejected user-initiated mutation must reach a rendered error surface (ErrorNotice), not just the console; also: createAsyncThunk serializes thrown errors, so
instanceof/.statuschecks silently never match in onError handlers."Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)