Skip to content

fix(sidebar): surface folder-create failures inline (#8229) - #8547

Merged
bolichen97 merged 1 commit into
mainfrom
fix/folder-create-stale-project-dir-error-8229
Sep 5, 2026
Merged

fix(sidebar): surface folder-create failures inline (#8229)#8547
bolichen97 merged 1 commit into
mainfrom
fix/folder-create-stale-project-dir-error-8229

Conversation

@CrysisDeu

@CrysisDeu CrysisDeu commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Problem / Motivation

Clicking the per-folder + button in the chat sidebar when the folder's project_dir no 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)

  • Symptom → the row appears (optimistic publish), then createSlot deletes 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's createChatInFolderMutation.onError only console.error'd.
  • Root cause → the error was thrown away at the UI boundary; nothing rendered it.
  • Change (scope deliberately limited to issue part 1 — surfacing; startup re-validation of folders.json paths is a design decision and out of scope; createSlot's rollback contract is untouched):
    • createChatInFolderMutation.onError now maps the failure to a user-facing message and stores {folderId, columnId, message} state. The specific 400 Not a directory case gets an i18n message naming the folder's resolved project path (resolveFolderProjectDir) and pointing at Folder settings (⋯ menu); anything else falls back to err.message like the sibling notices. createAsyncThunk serializes thrown errors down to {name, message, stack}, so the mapping matches both the live ApiError instance and the serialized shape (verified against the real built SPA).
    • The notice renders through the shared ErrorNotice surface (AUTOSDE errors-use-error-notice) directly under the folder's header row, in list and board view, with role="alert", the ⚡ agent hand-off (askAgent — nothing to destroy here: the sidebar holds no unsaved draft and survives the navigation), and the ✕ dismiss. flex-wrap lets the message take the full line in the ~250px drawer with the controls folding under it.
    • Cleared on dismissal or on the next successful create in that folder. A monotonic attempt counter makes settle callbacks act only for the latest attempt, so an older create failing late can't resurrect a stale notice (and a stale success can't clear a newer failure).
    • Board view scopes the notice to the column the create was issued from, under a column-unique test id — a root folder repeated across columns announces ONE alert.
    • Layout-independent fallbacks complete the coverage: the flat-view lane renders the notice at its top (flat view mounts no folder headers), and the board lane carries a lane-level fallback for errors with no originating column, for board-flat mode (which hides the columnId-scoped mounts), and for an error whose column was deleted. Exactly one mount renders in every (error scope × layout) cell — the full ownership table is in this review comment.
    • A folder that owns the active notice stays visible while the list is narrowed by search, so a create fired from the folder-picker menu into a filtered-out folder cannot fail silently again.
    • i18n: 2 new pages.chatSidebar keys translated in all 12 catalogs (sorted placement), en-XA regenerated.

Tests

website/src/test/ChatSidebarW3Coverage.test.tsx:

  • extended logs a failed folder-scoped create instead of failing silently — the inline notice renders with the generic message and role="alert";
  • names the stale project directory when the backend refuses the folder scopechatSlotProject rejects ApiError(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:

inline notice after failed folder create

Before / after dismiss

folder before create

notice dismissed

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/.status checks silently never match in onError handlers."

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) — N/A, behaviour is self-describing UI
  • No secrets, credentials, or internal references in the diff

@CrysisDeu
CrysisDeu requested a review from a team September 4, 2026 19:57
@CrysisDeu
CrysisDeu requested a review from a team as a code owner September 4, 2026 19:57
@CrysisDeu
CrysisDeu requested a review from Zedmor September 4, 2026 19:57
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

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

Established conventions confirmed — 425 capture scripts and a committed temp-screenshots/ tree already exist on base, so those additions follow the repo pattern. I have what I need for the design review.

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

  • folderCreateMountAbsent re-derives the hidden/filtered exclusion — the comment itself says it "Mirrors the exclusion applied at visibleRootFolders / renderFolderBlock's child filter". Two copies of one visibility rule, kept in sync only by discipline: a future change to folder hiding/filtering that touches one side breaks the "exactly one mount per (error scope × layout) cell" invariant, yielding either duplicate role="alert" announcements or the original silent failure. Extract a single shared predicate both sites call, and pin the untested cells (board-flat, deleted column, no-columnId) with tests — the ownership table currently lives only in a PR comment.
  • The stale-dir detection keys on the exact backend English string (err.message === 'Not a directory', doubled for the serialized-thunk shape). If the backend ever rewords or adds a code field, the specific message silently degrades to the generic one. Follow-up: give the 400 body a machine-readable code (per the AGENTS.md non-2xx rule) and match that.

Suggestions

  • Fold offerSettings into the mapping that produces message/title (one derived "kind" instead of three parallel flags), so a second specific-failure case doesn't grow another boolean per concern.

[DESIGN-REVIEWED] df8b554

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — ✅ PASS

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

UX-Verdict: PASS

A previously silent failure now lands inline, names the exact stale path, and offers the remedy — comprehensible, actionable, and dismissible.

Suggestions

  • In 02-inline-notice-stale-project-dir.png the AlertTriangle renders orphaned on its own line above the wrapped message (the flex-wrap w-full override lets the long message span wrap below the icon); group the icon with the message's first line so the glyph doesn't read as a stray artifact every time the notice shows.
  • Fallback mounts (flat-view lane top, board lane, hidden-folder tree fallback) render "Could not create the chat." with no folder name anywhere adjacent — a user returning to a persisted notice can't tell which folder refused; pass the folder name into folder_create_failed (e.g. "Could not create a chat in {{name}}.") for the lane-level renders.

[UX-REVIEWED] df8b554

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of df8b55460f0f39d040efc071992554961b78d4f2 — 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 checks complete. The change is unusually well-scoped: every fallback mount guards a reachable state (the New-menu folder picker at ChatSidebar.tsx:5597 can target folders whose headers never render), the screenshots/capture script follow the documented temp-screenshots/<feature>/ PR-template convention (389 sibling capture-*.mjs scripts exist), the shared ErrorNotice surface is mandated by the errors-use-error-notice AUTOSDE rule, and within the sidebar this was the last console-only onError (the other seven sites at lines 2972, 3010, 3398, 3484, 4100, 4707 all surface or roll back). The one remaining console-only sibling repo-wide (paletteActions.ts:111) documents a real constraint — the palette has closed, so no surface remains — and the author filed the general pattern as a rule candidate.

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 ships

Intent: let a user see why a new chat in a folder vanished, and where to fix it — a FIX.

  1. Failed folder create shows an inline notice under the folder header (list + board) — justified
  2. Stale-project-dir case names the missing path — justified
  3. Other failures show the raw reason with a generic lead — justified
  4. "Folder settings" link on the stale-dir notice opens the folder modal — justified (declared in manual-verification)
  5. Notice clears on ✕ or the next successful create; late-settling attempts can't resurrect/clear it — justified
  6. Notice still renders when the folder's header doesn't (flat view, hidden/filtered folder, deleted column/folder) — justified; folder-picker create (ChatSidebar.tsx:5597) reaches unrendered folders
  7. Folder owning the notice stays visible under search narrowing — justified
  8. Failure announced to screen readers (role="alert") — justified, via shared ErrorNotice
  9. Capture script + 3 screenshots in temp-screenshots/ — justified, documented repo convention
  10. 2 i18n keys × 12 catalogs + en-XA — mandated by the i18n invariant

Sibling count: 0 remaining console-only onError in ChatSidebar (8 sites checked); 1 repo-wide (paletteActions.ts:111), which documents why no surface exists there, and the author filed the general rule candidate. Root-cause deferral (startup re-validation of folders.json paths) is stated explicitly in the description with what is left.

[FIRST-PRINCIPLES-REVIEWED] df8b554

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

FINDING -- website/src/pages/ChatSidebar.tsx:6479 -- "folderCreateMountAbsent(...)" treats a filtered folder as absent even though board columns still render it, producing duplicate alerts -> Fix: use a board-specific mount check that ignores list filters.
[GPT-REVIEWED] df8b554

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

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

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

Review details

Two 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 folderCreateMountAbsent(folderCreateError.folderId) is true, but board columns render every root folder from relevantFolders = rootFolders (unfiltered) so a hidden or filter-excluded folder's per-column mount at line 4864 already renders; a failed create issued from that folder's column + button then paints two role="alert" notices sharing one col-<colId>-folder-create-error-<F> data-testid → Fix: gate the board fallback on genuine board-mount absence (deleted folder / flat view), not on the tree-semantics folderCreateMountAbsent.

FINDING — website/src/pages/ChatSidebar.tsx:4971 — folderCreateMountAbsent tests only isFolderHidden/isFolderFilteredOut, not a collapsed ancestor; a create fired from the New-menu folder picker (line 5597, no columnId) into a subfolder whose parent is collapsed renders its only notice mount (line 5296) inside the parent's FolderBody, which stays inert/aria-hidden/visibility:hidden, so the tree-lane fallback at 6395 does not fire and the failure is invisible and unannounced — the exact silent failure #8229 set out to fix → Fix: have folderCreateMountAbsent also return true when any ancestor folder is collapsed.

[OPUS-REVIEWED] df8b554

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

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

@CrysisDeu
CrysisDeu force-pushed the fix/folder-create-stale-project-dir-error-8229 branch from df3f94f to 7edb663 Compare September 4, 2026 20:35
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

🤖 Kiro Crew Auto-Pipeline [operator: CrysisDeu#0c98c3a2]

Status: functionally review-ready. Everything this PR owns is green at head 7edb663fd: all 4 Frontend Tests shards, all Backend 3.12 shards, Frontend Lint & Type Check, Frontend Coverage Merge, all i18n gates, and every AI review lane (GPT 5.6 PASS after the report-prop fix, Opus PASS, Design/UX/First Principles PASS). mergeable: MERGEABLE.

The remaining reds are inherited from main, not from this diff (which touches only website/ + temp-screenshots/):

Red check Evidence of main-inheritance
Backend Tests (Windows) (4) main's own ci.yml run fails the identical job; deterministic AttributeError: '_Req' object has no attribute 'can_read_body' in test/test_slot_close_recreation_race.py (merged via #7212); reproduced on both heads of this PR; upstream fix branch test/slot-close-race-req-double is already in flight
Bundle Size Gate red on main's same run
E2E (stub ACP backend, offline) red on main's same run
Coverage Gate (still queued here) red on main's same run

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.

@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 4, 2026
@CrysisDeu
CrysisDeu force-pushed the fix/folder-create-stale-project-dir-error-8229 branch from 7edb663 to ef0d3f5 Compare September 4, 2026 23:52
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 4, 2026
@CrysisDeu
CrysisDeu force-pushed the fix/folder-create-stale-project-dir-error-8229 branch from ef0d3f5 to 7549413 Compare September 5, 2026 00:09
@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
CrysisDeu force-pushed the fix/folder-create-stale-project-dir-error-8229 branch from 7549413 to d3e16c3 Compare September 5, 2026 00:17
@CrysisDeu

CrysisDeu commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

Both UX suggestions from the ef0d3f57b review are fixed in d3e16c35d (implemented per operator's standing instruction to implement UX concerns, not only blocking findings):

  • "(⋯ menu)" names a hover-only glyph that doesn't match the rendered ⋮ — took the review's own better option: the stale-directory notice now renders a direct Folder settings link that opens setFolderModal({mode:'edit'}) for that folder (own line under the notice, keeping the notice row at two actions). The folder_project_dir_missing message was reworded in all 12 catalogs to drop the glyph description; en-XA regenerated. Screenshots re-captured at the new head show the link.
  • Generic branch renders raw transport text without a task-level lead — the generic onError branch now passes title={i18nT('pages.chatSidebar.folder_create_failed')} to ErrorNotice, so "no capacity"-style transport text always appears under a "Could not create the chat." lead. The stale-directory branch stays title-less (its message is already a full task-level sentence), and message remains the journal lookup key.

@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 commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

Rebutted (disproportionate remedy, twice-adjudicated). The ChatSidebar.tsx:4562 concurrent-create suppression finding is a repeat of the previous round's F2 on ef0d3f57b, which the Opus adjudicator downgraded (DOWNGRADE F2 ... reason=disproportionate-remedy). This round's adjudication reached the same substantive ruling — harm LOW, self-correcting on retry, per-folder attempt-map cost exceeds the rare recoverable harm, and errors-use-error-notice governs rendering-through-ErrorNotice (which this code does), not race-based suppression — but its verdict lines were indented into a code block, so the parser counted 0 well-formed verdicts and GPT's block stood by default. The verdict text is quoted verbatim in the review body's own adjudication section:

DOWNGRADE F1 website/src/pages/ChatSidebar.tsx:4562 reason=disproportionate-remedy

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 console.error-logged, no session appears (the user is never told success), and retrying surfaces the notice. Requesting a lane re-run so a well-formed adjudication can register.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

/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.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Human judgment recorded

@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for d3e16c35d76636451415abf6378ff8ed99d0a2c4.

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.

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

@CrysisDeu
CrysisDeu force-pushed the fix/folder-create-stale-project-dir-error-8229 branch from d3e16c3 to 80e2ef7 Compare September 5, 2026 00:35
@CrysisDeu
CrysisDeu force-pushed the fix/folder-create-stale-project-dir-error-8229 branch 3 times, most recently from 1c056d0 to 13e2d34 Compare September 5, 2026 00:55
@CrysisDeu

CrysisDeu commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

Fixed in 13e2d340f — and the whole visibility class is now closed, not just the named cell. The 6441 finding (board-flat strands columnId-scoped errors) is the third sibling in one class: "the mount that owns this error is not rendered in the current layout". Rather than another point fix, 13e2d340f makes ownership total. Coverage table — every (error scope × layout) cell and its owner:

Error scope \ Layout Tree List-flat Board (folders shown) Board-flat Column deleted
no columnId (folder row / New menu) folder-header mount (kept visible even when search-narrowed) flat-lane fallback board lane fallback (!columnId) board lane fallback n/a
columnId-scoped (column ⊕) tree mount now owns ANY error for its folder outside board view (covers switching back to tree) flat-lane fallback (passes stored columnId) that column's own mount (exact match — one alert, not N) board lane fallback (flatView) board lane fallback (column-liveness check)

Exactly one mount renders in every cell (the fallback condition is false whenever a scoped mount is live), so no duplicate role="alert" announcements. Ownership rule is centralized in renderFolderCreateError with the rationale in comments.

@CrysisDeu

CrysisDeu commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

Both First Principles watch items on 634517b0a are answered:

  • Two unfixed siblings of the console-only onError cause (ChatPage.tsx:1641 steer, ProjectDetailPage.tsx:99 plan export) — accepted-and-deferred: filed as #8625 with the migration pattern this PR establishes (raw-text findReport lookup, ErrorNotice + askAgent where safe) spelled out so it is directly actionable. Fixing them here would widen a New chat in a sidebar folder fails silently when the folder's project_dir no longer exists #8229 fix into an unrelated two-page migration; the Pattern harvest rule proposal covers detection of future regressions of the class.
  • Items 5 and 7 (flat-lane and board lane-level fallbacks) shipped but undeclared in the descriptionfixed: the PR body's "What changed" now declares both fallback surfaces and links the full (error scope × layout) ownership table. Description-only edit; the diff is unchanged.

@CrysisDeu
CrysisDeu force-pushed the fix/folder-create-stale-project-dir-error-8229 branch from 634517b to df8b554 Compare September 5, 2026 02:17
@CrysisDeu

CrysisDeu commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

Fixed in df8b55460 — and this closes the visibility class exhaustively, by enumeration. The 4971 finding (deleted target folder reads as mounted) was the last unreached cell. folderCreateMountAbsent now returns true for a non-existent folder (the definitionally strongest mount-absent case), and the board lane fallback gained the same folderCreateMountAbsent disjunct.

Completeness argument — the notice's owning mount is a function of exactly three variables, each now total:

  1. Folder existence: exists → walk visibility; deleted → mount-absent, lane fallback owns it (this round's fix).
  2. Folder visibility (exists case): visible → its header mount; hidden/filtered (self or ancestor, not revealed) → mount-absent, lane fallback (round-5 fix).
  3. Layout: tree → tree-lane fallback; list-flat → flat-lane fallback; board with folders → per-column mount (exact columnId match) or board fallback for no-column/dead-column errors; board-flat → board fallback (rounds 2 and 4 fixes).

Every lane has a fallback, every fallback condition is the complement of its scoped mount's render condition, and renderFolderCreateError's ownership rule guarantees exactly one role="alert" per state. There is no fourth variable that gates rendering, so no further sibling cells exist in this class.

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: action required A blocking check or review needs attention labels Sep 5, 2026
@bolichen97
bolichen97 enabled auto-merge (squash) September 5, 2026 06:47
@bolichen97
bolichen97 merged commit 6df5759 into main Sep 5, 2026
66 checks passed
@bolichen97
bolichen97 deleted the fix/folder-create-stale-project-dir-error-8229 branch September 5, 2026 06:53
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Sep 5, 2026
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.

New chat in a sidebar folder fails silently when the folder's project_dir no longer exists

3 participants