Skip to content

fix(apps): route error states through ErrorNotice (batch apps-3) - #8810

Merged
chenmingwei23 merged 1 commit into
mainfrom
fix/error-notice-apps-3
Sep 5, 2026
Merged

fix(apps): route error states through ErrorNotice (batch apps-3)#8810
chenmingwei23 merged 1 commit into
mainfrom
fix/error-notice-apps-3

Conversation

@CrysisDeu

@CrysisDeu CrysisDeu commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

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, TrustAppModal and the Library page now renders through the shared ErrorNotice, per the blocking AUTOSDE rule errors-use-error-notice. Reference implementation: aws-control's AwsErrorNotice (#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_error keys orphaned by the migration — papyrus, spec-builder — are deleted from all 13 catalogs + en-XA as a pure two-line diff each). The askAgent decision 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 the draft (askAgent={!newName && !cloneUrl}, the aws-control handOff pattern). 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 read
  • ops-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, rotation schedule_problem
  • ops-mission-control/SignalsPanel.tsx — 3 sites: provider poll failed (inline in the table cell; backing_off stays a text-warn warning, it is not a failure), signals read, claim failure
  • personal-shopper/HistoryTab.tsxsilent 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 through ErrorNotice instead 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 unchanged
  • components/appstore/TrustAppModal.tsx — grant/enable failure (trust dialog holds no draft)
  • pages/apps/LibraryPage.tsx — the existing notice's No hand-off comment named no concrete draft (the audit's missing-askAgent-decision); the page has no text input (search is a filter, uninstall-confirm checkboxes reset on close), so askAgent is on
  • mochi/MochiPage.tsx OfflineLanding — 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.tsx5 silent failures fixed: add/delete preference, add/delete group, inline edit — one notice each, beside the new-preference / group-name / edit inputs
  • pptx-maker/LibraryPanel.tsx RenameRow — the new name input
  • spec-builder/SpecBuilderPage.tsx — new-spec form or spec chat composer beneath the banner; onDismiss kept
  • mochi/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 inputs
  • meetings/SettingsView.tsxtoast-only failures fixed: config save, term add, term remove (remove had no onError at all) now also render in-page beside the term inputs, one notice each
  • meetings/hooks/useMeetingSession.tsmeetings/components/AgentPanel.tsxtoast-only failures fixed: minutes save / revert. revertOutput now returns the mutation promise (like saveOutput already 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.tsxaskAgent={!newName && !cloneUrl} (report said on; the list has a create form)
  • personal-shopper/SitesTab.tsxaskAgent={!newName && !newUrl} (form state persists even when the form is collapsed)
  • personal-shopper/PersonalShopperPage.tsxaskAgent={activeTab === 'history'} (report said on; Preferences and Sites tabs mount forms)
  • pptx-maker/LibraryPanel.tsx panel notice — askAgent={!renaming}
  • pptx-maker/PptxMakerPage.tsx SettingsView — askAgent={draft === null} (report said on; the deck-root input is right above it)
  • workflows/WorkflowsRuns.tsxaskAgent={!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,562left 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 would window.location.assign('/chat') inside the pet window. ErrorNotice is not reachable there; needs its own primitive (rule-gap, below).
  • The six gated sites above where the report said askAgent on — each has a live input the report did not count.
  • 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)

  • a .ts hook owning onError -> notify toasts whose render surface is a different .tsx (useMeetingSession → AgentPanel): the rule's src/**/*.tsx file-pattern never sees the hook
  • an error string threaded as a string prop and rendered by a child (ActRulesCard error, RenameRow error, OfflineLanding)
  • a status enum where only one value is a failure (SignalsPanel failed vs backing_off; WorkflowRunTree failed vs cancelled) — the hand-written branch mixed both in one element
  • a React tree mounted OUTSIDE the dashboard shell (mochi renderer windows) where ErrorNotice's Tailwind classes and navigation hand-off do not apply

Pattern harvest

Rule candidate: errors-use-error-notice should say that when several mutations report into one surface, each gets its OWN ErrorNotice — a ternary / ?? chain over mutationA.error ?? mutationB.error lets a persistent earlier failure mask a later one (found by GPT at four sites in this batch), and that the hand-off button counts toward max-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)

Verification

  • Local: tsc -b clean, eslint clean (0 errors), I18N_BASE_REF=origin/main npm run i18n:check all gates OK (no new keys), jscpd . 0 clones, no non-inclusive terms in the diff.
  • No test suite run locally by instruction — verification via CI.

Screenshot Evidence

Capture harness: website/capture/error-notice-apps-3.{html,tsx} + website/scripts/capture-error-notice-apps-3.mjs (Vite + Playwright, self-checking: 6 role="alert" notices and exactly 4 hand-offs in the AFTER column, 0 hand-offs in BEFORE; WorkflowRunTree is the real component in its failed state).

Before (origin/main) vs after (this branch), dark:

before/after dark

Light:

before/after light

@CrysisDeu
CrysisDeu requested a review from a team September 5, 2026 20:23
@CrysisDeu
CrysisDeu requested a review from a team as a code owner September 5, 2026 20:23
@CrysisDeu
CrysisDeu requested a review from buluoray September 5, 2026 20:23
@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: checking Automated validation is still running labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

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

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 ?? chains) and the draft-gated hand-off are the right generalization of the aws-control reference, and the six deviations from the report's recommendations are each grounded in a live input the report missed — the correct direction of override. The one genuinely unresolvable site (mochi's Electron settings window, no Tailwind/Router) is left alone and recorded as a rule-gap owned by #8729 rather than force-fitted. Silent-failure fixes (revoke, thumbs feedback, cancel/reopen/unpin) remove real user harm beyond the mechanical migration. Capture harness and screenshot conventions match batches 1–2.

[DESIGN-REVIEWED] 8f98903

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — 🟡 CONCERNS

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

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

  • Off-locus error placement: HistoryTab.tsx renders feedbackMutation failures above all session cards ("A rejected thumbs-up / purchased / skipped used to vanish"), and MochiPage.tsx puts cancel/reopen/unpin errors above their lists. A thumbs-up on a card several viewports down fails → the notice appears at the top, outside the viewport → the user sees nothing change and re-clicks or assumes breakage — the exact failure this PR claims fixed, half-fixed. Rare (backend rejection) × moderate friction × every failure. Fix: scope the notice to the acted-on row via mutation.variables, the pattern this same PR already uses for decideMutation in OpsMissionControlPage.

Suggestions

  • meetings/SettingsView.tsx: removeTerm shows raw (removeTerm.error as Error).message while sibling addTerm two lines above shows "Could not add that correction." — give remove a matching catalog string ("Could not remove that correction.").

[UX-REVIEWED] 8f98903

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 8f9890334b10d848ab52fcc187ded7538f7a8308 — 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 done. The sweep is mandated by the documented errors-use-error-notice invariant in website/AGENTS.md; the capture-page/screenshot pattern is a documented repo convention (temp-screenshots/README.md, ~205 sibling capture files); remaining text-danger occurrences in the touched files are icon tones and hover styling, not error surfaces. The notable premise risks are the empty description, the meetings hunks absent from the batch's own stated scope, and the Library page hand-off reversal.

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 ships

Intent: 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).

  1. ~20 hand-written danger texts/boxes now render as ErrorNotice — justified (mandated by website/AGENTS.md errors-use-error-notice)
  2. Those notices gain the agent hand-off, gated per-surface with a stated draft-safety reason — justified (same invariant)
  3. Previously silent rejections now show a message (mochi cancel/reopen/unpin/seen, shopper mutations, ops credential revoke) — rides along, harm named
  4. Meetings save/add-term/remove-term/revert failures now persist in-page; revert plumbed to a promise — undeclared (absent from the batch's own capture list)
  5. Library page error notice now offers the hand-off, reversing its prior "No hand-off" comment — undeclared reversal
  6. Ops decide-refusal moved below the button row — justified move (hand-off would be a third in-row action)
  7. Papyrus compile failure after an agent turn now shows a banner (was swallowed) — rides along, harm named
  8. "Dismiss error" strings deleted from 13 locale catalogs — justified subtraction
  9. Failed workflow run renders through ErrorNotice; cancelled/finished panel simplified — justified
  10. Capture page, script, two committed screenshots — justified (documented convention, temp-screenshots/README.md)

Watch

  • Description is empty, so every item above is technically undeclared; the title carries the whole declaration. The meetings changes (SettingsView, AgentPanel, useMeetingSession) are undeclared even by the PR's own evidence: the capture entry's comment scopes the batch to "ops-mission-control, papyrus, personal-shopper, pptx-maker, spec-builder, workflows, mochi, TrustAppModal, Library page", and the self-check counts 6 notices that include none of them.
  • LibraryPage.tsx:1628 flips askAgent on where the prior comment said "navigating away would discard in-flight state"; the new comment only rebuts text drafts. If the old premise (in-flight management actions) was ever true, this reverses it without saying what changed.
  • Sweep completeness inside touched files verified: remaining text-danger hits there (grep, 81 across 44 app files) are icon tones/hover styling; untouched apps (issue-radar, code-review-sage, auto-research) are the accepted-and-deferred later batches.

Subtractions

  • Defer the three meetings files to the batch that covers meetings (or their own), where their before/after evidence and self-check can exist — this batch's capture artifact cannot vouch for them.

[FIRST-PRINCIPLES-REVIEWED] 8f98903

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

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

Review details

The discovery pass reported no candidates, and my Step 2 examination of the highest-risk changes confirms the refactor is clean: ErrorNotice returns null on a falsy message (line 77), so every unconditional <ErrorNotice message={error} /> is safe; the SignalsPanel failed/backing_off/ok branches are balanced and complete; SettingsPanel's configMutation/secretMutation/revokeMutation all exist and the removed writeError is unused elsewhere; the WorkflowRunTree status split exhaustively covers failed vs finished/cancelled; and the test updates match the migrated markup. Nothing grounds a finding at 80+.

No findings.

[OPUS-REVIEWED] 8f98903

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

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

@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 8f9890334b10d848ab52fcc187ded7538f7a8308 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 8f98903

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

@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 5, 2026
@CrysisDeu
CrysisDeu force-pushed the fix/error-notice-apps-3 branch from 72fb37e to d4753f0 Compare September 5, 2026 21:53
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

/ai-review override gpt d4753f0: the sole BLOCKING finding (SessionColorPicker.tsx:47, in-flight color PATCH ordering) is in a file this PR does not touch — it landed on main via #8743 (chat-2) and is out of this diff's scope; nothing in batch apps-3 changes that component.

@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 d4753f0d1ef7d09a64b718bed36ae96099aead35.

the sole BLOCKING finding (SessionColorPicker.tsx:47, in-flight color PATCH ordering) is in a file this PR does not touch — it landed on main via #8743 (chat-2) and is out of this diff's scope; nothing in batch apps-3 changes that component.

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

@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 5, 2026
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.
@CrysisDeu
CrysisDeu force-pushed the fix/error-notice-apps-3 branch from d4753f0 to 8f98903 Compare September 5, 2026 22:40
@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
@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Sep 5, 2026

@chenmingwei23 chenmingwei23 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved: checks green at head, readiness passed, no blocking reviewer verdict outstanding.

@chenmingwei23
chenmingwei23 merged commit 44f4892 into main Sep 5, 2026
74 of 86 checks passed
@chenmingwei23
chenmingwei23 deleted the fix/error-notice-apps-3 branch September 5, 2026 23:12
@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.

2 participants