fix(apps): route error states through ErrorNotice (batch apps-3) - #8810
Conversation
Design Review (Fable 5) — ✅ PASSDesign-level review of Design-Verdict: PASS Batch 3 of an audited, rule-driven sweep; each site's askAgent decision is justified against actual on-screen drafts, and deviations from the audit are per-site evidence, not drift. The per-mutation-notice choice (no [DESIGN-REVIEWED] 8f98903 |
UX Review (Fable 5) — 🟡 CONCERNSUX-level review of UX-Verdict: CONCERNS The newly visible mutation errors in HistoryTab and Mochi's watchlist render at the container top, so a scrolled user still sees their click fail silently. Watch
Suggestions
[UX-REVIEWED] 8f98903 |
First Principles Review (Fable 5) — 🟡 CONCERNSPremise-level review of All checks done. The sweep is mandated by the documented First-Principles-Verdict: CONCERNS Invariant-mandated sweep, well aimed — but the description is empty, the meetings hunks fall outside the batch's own stated scope, and LibraryPage reverses a recorded no-hand-off decision. What this change shipsIntent: make every app failure visible through the shared error surface, with the agent hand-off wherever it loses nothing — a FIX (batch 3 of a declared sweep).
Watch
Subtractions
[FIRST-PRINCIPLES-REVIEWED] 8f98903 |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsThe discovery pass reported no candidates, and my Step 2 examination of the highest-risk changes confirms the refactor is clean: No findings. [OPUS-REVIEWED] 8f98903 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
72fb37e to
d4753f0
Compare
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. |
Batch apps-3 of the dashboard error-state sweep (audit report §5.3): every hand-written error surface in ops-mission-control, papyrus, personal-shopper, pptx-maker, spec-builder, workflows, mochi's dashboard page, TrustAppModal and the Library page now renders through the shared ErrorNotice, per the blocking AUTOSDE rule errors-use-error-notice. Each site passes the same string it already rendered (no new i18n keys; two orphaned dismiss_error keys removed). askAgent is stated at every site: on where the hand-off loses nothing, off with a `No hand-off: <draft>` comment beside an unsaved draft, or gated on the draft being empty where the component can see it. Several mutations reporting into one surface each get their own notice (a coalescing chain lets an earlier persistent failure mask a later one), and a notice never sits inside an existing action row (its hand-off button would be a third action). Silent failures fixed: mochi enable/cancel/reopen/unpin/seen, ops-mission-control credential revoke, papyrus post-co-author compile request, personal-shopper feedback + all five preference mutations, meetings config save / term add and remove / minutes save and revert (toast kept as transient feedback; the panel keeps an in-page notice, in the chat view too). Tests pinned to the old surfaces updated (TrustAppModal, SpecBuilderPage, opsMissionControl source scan). Screenshot evidence: website/capture/error-notice-apps-3 + capture script.
d4753f0 to
8f98903
Compare
chenmingwei23
left a comment
There was a problem hiding this comment.
Approved: checks green at head, readiness passed, no blocking reviewer verdict outstanding.
Summary
Batch apps-3 of the dashboard error-state sweep (audit report §5.3 "Batch apps-3", 21 files): every hand-written error surface in ops-mission-control, papyrus, personal-shopper, pptx-maker, spec-builder, workflows, mochi's dashboard page,
TrustAppModaland the Library page now renders through the sharedErrorNotice, per the blocking AUTOSDE ruleerrors-use-error-notice. Reference implementation: aws-control'sAwsErrorNotice(#8320).no linked issue: driven by the audit report (
research/error-states-audit.md, section 5.3).Each site passes the SAME string it already rendered (no new i18n keys; the two
dismiss_errorkeys orphaned by the migration — papyrus, spec-builder — are deleted from all 13 catalogs + en-XA as a pure two-line diff each). TheaskAgentdecision is stated at every site: on where the hand-off loses nothing, off with aNo hand-off: <draft>comment beside an unsaved draft, or gated on the draft being empty where the component can see the draft (askAgent={!newName && !cloneUrl}, the aws-controlhandOffpattern). Several mutations reporting into one surface each get their OWN notice (a??/ternary chain lets a persistent earlier failure mask a later one), and a notice never sits inside an existing action row (its hand-off button would be a third action).Sites
askAgent on (reads, status panels, actions on persisted state — nothing on screen is a draft)
ops-mission-control/HandoverPanel.tsx— digest readops-mission-control/OpsMissionControlPage.tsx— 7 sites: postmortem read (inline), closed-incidents read, verification-failed (still_firing) retraction, dispatch failure, approve/reject refusal (403 message + backend{ok:false,error}; on its own row BELOW the two buttons), transition failure, rotationschedule_problemops-mission-control/SignalsPanel.tsx— 3 sites: provider pollfailed(inline in the table cell;backing_offstays atext-warnwarning, it is not a failure), signals read, claim failurepersonal-shopper/HistoryTab.tsx— silent failure fixed: thumbs feedback rejection now renders (search box is a filter)pptx-maker/PptxMakerPage.tsx— start-chat failure (decks view holds no draft)workflows/WorkflowRunTree.tsx— failed run renders throughErrorNoticeinstead of the red-bordered panel (same decision as the collapsed row / progress bar in fix(chat): route error states through ErrorNotice (batch chat-2) #8743 that feed this tree: composer draft is persisted per slot); finished/cancelled panel unchangedcomponents/appstore/TrustAppModal.tsx— grant/enable failure (trust dialog holds no draft)pages/apps/LibraryPage.tsx— the existing notice'sNo hand-offcomment named no concrete draft (the audit'smissing-askAgent-decision); the page has no text input (search is a filter, uninstall-confirm checkboxes reset on close), so askAgent is onmochi/MochiPage.tsxOfflineLanding — silent failure fixed: a rejected enable now renders (landing holds no draft)No hand-off (an unsaved draft shares the screen)
ops-mission-control/SettingsPanel.tsx— 5 sites: provider config / secret / revoke writes (revoke was a silent failure; one notice per write), GitHub login save, roster error (same card as the login input), autonomy rule form, mode card (the page's provider rows hold identity / login / secret drafts)papyrus/PapyrusPage.tsx— workspace banner (open editor buffer), project-list banner (ProjectList's new-paper / clone-URL inputs below it); silent failure fixed: after a successful post-co-author refresh, the compile request failing now populates the banner (the refresh swallow stays, as the audit allows; no compile is attempted when the refresh failed, as before)personal-shopper/PreferencesTab.tsx— 5 silent failures fixed: add/delete preference, add/delete group, inline edit — one notice each, beside the new-preference / group-name / edit inputspptx-maker/LibraryPanel.tsxRenameRow — the new name inputspec-builder/SpecBuilderPage.tsx— new-spec form or spec chat composer beneath the banner;onDismisskeptmochi/MochiPage.tsx— add-watch failure, silent failures fixed: cancel / reopen / unpin / mark-seen (one notice each) — all on the page with the watch form's label / target / category inputsmeetings/SettingsView.tsx— toast-only failures fixed: config save, term add, term remove (remove had noonErrorat all) now also render in-page beside the term inputs, one notice eachmeetings/hooks/useMeetingSession.ts→meetings/components/AgentPanel.tsx— toast-only failures fixed: minutes save / revert.revertOutputnow returns the mutation promise (likesaveOutputalready did) so the panel keeps an in-page notice after the toast fades — in the editing view, the default view AND the chat view (a revert can reject after the user switched views)Gated on the draft being empty
papyrus/ProjectList.tsx—askAgent={!newName && !cloneUrl}(report said on; the list has a create form)personal-shopper/SitesTab.tsx—askAgent={!newName && !newUrl}(form state persists even when the form is collapsed)personal-shopper/PersonalShopperPage.tsx—askAgent={activeTab === 'history'}(report said on; Preferences and Sites tabs mount forms)pptx-maker/LibraryPanel.tsxpanel notice —askAgent={!renaming}pptx-maker/PptxMakerPage.tsxSettingsView —askAgent={draft === null}(report said on; the deck-root input is right above it)workflows/WorkflowsRuns.tsx—askAgent={!saveName && !saveDescription}(report said on; the save modal has name/description inputs)Deviations from the report's recommended_action (author decisions)
mochi/src/renderer/SettingsPanel.tsx:514,562— left as-is, per the report's own caveat ("confirm ErrorNotice is reachable there"): this component renders in the pet's Electron settings window (settings.html), which loads no Tailwind / theme stylesheet and mounts without a Router, and the hand-off wouldwindow.location.assign('/chat')inside the pet window.ErrorNoticeis not reachable there; needs its own primitive (rule-gap, below).LibraryPage.tsx— turned on rather than naming a draft, because there is none.Rule-gap variants seen in this batch (NOT editing
website/AUTOSDE.yaml— #8729 owns that).tshook owningonError -> notifytoasts whose render surface is a different.tsx(useMeetingSession → AgentPanel): the rule'ssrc/**/*.tsxfile-pattern never sees the hookstringprop and rendered by a child (ActRulesCard error,RenameRow error,OfflineLanding)SignalsPanelfailedvsbacking_off;WorkflowRunTreefailedvscancelled) — the hand-written branch mixed both in one elementErrorNotice's Tailwind classes and navigation hand-off do not applyPattern harvest
Rule candidate:
errors-use-error-noticeshould say that when several mutations report into one surface, each gets its OWNErrorNotice— a ternary /??chain overmutationA.error ?? mutationB.errorlets a persistent earlier failure mask a later one (found by GPT at four sites in this batch), and that the hand-off button counts towardmax-two-buttons-per-row, so an inline notice never belongs inside an existing action row (the rule-text edit itself is owned by #8729).CI rounds (now one squashed commit, rebased past #8828, head
2b9e5d916)outputErrorin the chat view too). PapyrusPage compiles only after a successful refresh, as before. Three tests pinned to the old surfaces updated (TrustAppModaltoContain, SpecBuilderPage dismiss label + implicitaria-live).opsMissionControl.test.tssource-scan re-anchored to the per-mutation notice marker.website/src/pages/chat/SessionColorPicker.tsx, which this PR does not touch (landed via fix(chat): route error states through ErrorNotice (batch chat-2) #8743) — overridden.Frontend Tests (1)red was inherited:src/test/RemoteCrewPanel.test.tsxanchored on[role="status"]after fix(settings): route error states through ErrorNotice (batch settings-2) #8749 moved the diagnosis note ontoErrorNoticeand fix(settings): route error states through ErrorNotice (batch settings-1) #8729 merged the stale helper. Fixed on main by test(settings): point the RemoteCrewPanel diagnosis helper at the ErrorNotice #8828; this PR is rebased past it. PR Hygiene / Screenshot Evidence reds at 22:40Z ran against a momentarily blank body (rerun replays that payload); re-triggered via this edit.Verification
tsc -bclean,eslintclean (0 errors),I18N_BASE_REF=origin/main npm run i18n:checkall gates OK (no new keys),jscpd .0 clones, no non-inclusive terms in the diff.Screenshot Evidence
Capture harness:
website/capture/error-notice-apps-3.{html,tsx}+website/scripts/capture-error-notice-apps-3.mjs(Vite + Playwright, self-checking: 6role="alert"notices and exactly 4 hand-offs in the AFTER column, 0 hand-offs in BEFORE;WorkflowRunTreeis the real component in its failed state).Before (origin/main) vs after (this branch), dark:
Light: