Skip to content

fix(desktop): localize expected errors - #4457

Open
orangeCatDeveloper wants to merge 14 commits into
apache:mainfrom
orangeCatDeveloper:fix/desktop-error-codes
Open

fix(desktop): localize expected errors#4457
orangeCatDeveloper wants to merge 14 commits into
apache:mainfrom
orangeCatDeveloper:fix/desktop-error-codes

Conversation

@orangeCatDeveloper

@orangeCatDeveloper orangeCatDeveloper commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Desktop surfaces rendered raw exception text as user-facing copy: a zh user hitting an expected failure saw an untranslated English error.message, and unexpected exceptions could leak internal details into toasts and error banners. Producers (guest-session mounts, Work Board IPC, WorkHub controller, the composer attachment pipeline) now return machine-readable codes — { code, params } where a message needs data — and the renderer maps codes through the locale catalogs. Expected failures throw a typed ExpectedOperationError; everything else logs a redacted diagnostic and shows a localized fallback. This also deletes WorkHub's English-message-regex classifier and the MAKA_SESSION_READ_MESSAGES_ERROR marker protocol.

Per review, the global error mapper is split: each domain maps its own code union at its presenter; the ExpectedOperationError transport and redacted diagnostics live once in the renderer's application/contracts/operation-diagnostics entry, now delegating to reportUnexpectedOperation/unexpectedOperationFallback in @maka/core/redaction so copy catalogs (bare-import-only) and the renderer share one diagnostics channel. They are Desktop-only, and hosting them in @maka/ui pulled the icon bundle into the main-process test bundles. The 11 attachment validations reject with stable attachment_ingest:<code> tokens that survive the Electron IPC wrapper, mapped at the shared localizedShellErrorMessage entry — both locales previously lost these reasons to a generic fallback. The Runtime's 8 session-control guards emit session_control_blocked:<code> tokens; today no in-repo caller reaches those guards (Desktop changes session settings through session.configuration.update, whose failures already carry protocol codes, and the CLI drives SessionManager only through the Runtime Host protocol), so the tokens stay as a typed, test-pinned contract for the future CLI-local path. A structured {ok, code} envelope for configuration setters and attachment failures is the follow-up.

The WorkHub waiting summary is a complete catalog message in each locale, independent of the separately displayed status and retry paragraphs. This preserves existing output while letting future translations choose their own sentence structure.

Refs #2672

Verification

Before (zh UI, stale Work Board write / import over mount limit — raw English internals):

Work Board item wb-01 revision changed from 1 to 3
At most 12 shared Sessions can be retained

After (same failures; raw detail now only in the redacted console diagnostic):

工作板内容已更新,请刷新后重试。
共享 Session 数量已达上限。
apps/desktop full dist suite:   2228 pass / 0 fail
packages/core tests:            829 pass / 0 fail
packages/runtime tests:         3246 pass (5 sandbox-only filesystem-worker failures, unrelated)
typecheck (4 tsconfigs):        0 errors
renderer architecture check:    passes under the #4581 checker
repo format (biome):            clean on all touched files

Rebase/review follow-up (2a7c11d6a) re-verified all of the above after moving the redacted unexpected-failure diagnostics into @maka/core/redaction (reportUnexpectedOperation / unexpectedOperationFallback) so the copy catalog and the renderer entry share one channel; operation-diagnostics keeps its public surface for its seven renderer importers and delegates to the core helper. No behavior change.

Review follow-up (575f10b39 + fe1190919): folded the duplicate attachmentIngestBlocked imports; extracted workBoardActionErrorText in work-board-panel.tsx (exported test seam) so the presentation test drives the panel's real code-to-copy branch instead of re-implementing it — a panel mis-map now fails the test. The copy catalog stays free of transport imports (copy catalogs may only hold bare package runtime imports), and the expected-code lookup lives in the catalog as workBoardErrorCodeCopy so the panel adds no dependency-debt edge. Re-verified against the PR base: desktop 2228/0, four typechecks 0 errors, renderer architecture ratchet (--base 411512bd9), biome clean.

Review follow-up round 1 (16a2fe84f) and round 2 (e0ea0c3e3), addressing the two review comments:

  • P3 (shell-copy): the attachment_ingest:<code> matcher is anchored at the message tail (bare token or the IPC-wrapped error line end), so an unrelated message that merely contains the substring — e.g. a path like /tmp/attachment_ingest:count_limit/report.txt — keeps its fallback and the unexpected-diagnostics path. Negative tests pin both the anchor and the prototype-pollution cases.
  • P2 (plan-mode-panel), envelope round: the second review confirmed via a real Electron probe that the IPC boundary strips custom fields from thrown Errors, so the renderer can never see RuntimeHostOperationError — the previous instanceof fix was unreachable in production. The five plan-mode control channels (requestRevision, abandon, approve, resume, abandonExecution) now return a structured PlanControlIpcResult envelope (mirroring the WorkBoardIpcResult precedent): main catches the typed rejection and returns {ok:false, error:{code,message}}; the panel checks the envelope and maps error.code through planModeCopy.controlFailure (all three locales), with unknown/future codes falling back to the generic line. abandonPlanProposal keeps its throwing shape (the app-shell legacy closure is token-frozen; its generic failure copy flows through sessionSettingFailureCopy). The envelope type lives in plan-mode-copy.ts (bare imports exempt from the debt ratchet) and shared/plan-mode-ipc.ts (main/preload). RuntimeHostOperationError stays in the protocol package under the declared compatible extension at epoch 117. The panel seam test now drives planControlFailureCopy with the wire envelope shape.

Re-verified after the envelope round: desktop 2230/0 (presentation tests drive the wire envelope: expected codes per locale, success envelopes, unknown-code fallback), core 829/0, four typechecks 0 errors, renderer architecture ratchet against the PR base, biome clean, epoch guard passes. The one runtime-host failure (execution-model-composition sandbox ENOENT under macOS /var/folders) reproduces on the unmodified base and is unrelated to this diff.

Review follow-up round 3 (67072ee53): the envelope's catch blocks converted an unrecognized rejection (network fault, programming error, non-plan.* operation) into fall-through success — emitModeChanged fired and the renderer got ok:true for a failed action. Every plan-mode handler now rethrows when planControlIpcFailure produces no envelope, and a new test drives all five channels with a non-plan control failure asserting the exact cause propagates. Re-verified: desktop 2231/0, typecheck clean, architecture ratchet and biome pass.

Review follow-up round 4 (4624383de, rebased onto main c0229b0c6): the cleanup in 0f25f62cf had dropped setPending(true) from the plan panel's run(), so the four controls gated on planMode.pending stayed enabled while an action was in flight. Restored, with a hook-level regression that mounts usePlanModeState through the real providers, holds one bridge promise open with deferred, and asserts pending stays true until it settles (fails without the fix: pending must hold while the bridge call is unsettled). Two P3s along with it: planControlFailureCopy takes the envelope error directly (the ok branch was unreachable), and localizedShellErrorMessage records the unexpected diagnostic only when the generalized classifier has no category, so a timeout or 429 no longer logs an "operation failed" stack. Re-verified: targeted dist tests 10/0, four typechecks clean except stories/app-shell.stories.tsx (rightCollapsed, identical to main), renderer architecture ratchet against c0229b0c6, biome clean, epoch guard passes.

Earlier follow-up verification still holds: all 122 WorkHub tests pass, including three-locale summary assertions that remain valid when the separate paragraph copy changes; all 101 architecture-checker tests pass. The branch base is the current main (411512bd9), so no prerequisite PRs remain; the draft blocker (#4493 checker policy) landed on 2026-09-02.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code — analysis, implementation, tests, and this description, under the contributor's direction; the commit carries a Generated-by: Claude Code trailer.

OpenCode implemented the complete waiting-summary catalog messages and added output tests; its follow-up commit carries Generated-by: OpenCode. Claude Code landed the rebase/review follow-up (2a7c11d6a: single diagnostics channel in @maka/core/redaction, operation-diagnostics delegating), verified the branch against current main, and updated this description.

Checklist

  • Tests cover the change and fail without it

@orangeCatDeveloper

Copy link
Copy Markdown
Contributor Author

Converting to draft: blocked on the copy-gate/architecture-ratchet collision described in #2672 (comment)#4493 resolves the checker policy; this branch then rebases (and splits the global error mapper per review). Will mark ready once green.

@orangeCatDeveloper
orangeCatDeveloper force-pushed the fix/desktop-error-codes branch 2 times, most recently from adf4f4a to 6b28e30 Compare September 2, 2026 08:40
Astro-Han pushed a commit that referenced this pull request Sep 2, 2026
…cture ratchet (#4493)

Two repository gates deadlocked. The locale policy (#2672) moves user-visible copy out of business files into src/renderer/locales/*-copy.ts catalogs, which adds an import edge; the renderer architecture ratchet forbids legacy files from growing their dependency count and rejects new AppShell-closure entries. #4457 failed CI on exactly that collision, and no placement of a catalog could satisfy both gates.

Admit one dependency class instead of loosening the ratchet: a validated copy catalog, recognized structurally and re-verified on every run, never grandfathered. It must live under src/renderer/locales/*-copy.ts, carry the UiCatalog marker from @maka/core/ui-locale, have zero tracked capabilities by the checker's own metrics, and import bare package specifiers only, so it can never become a tunnel to renderer implementation. Admitted edges are excluded from the dependency-count ratchets and closure admission, and the ledger drops 26 budget entries that only existed for catalog imports. Root-entry files get no discount, and a catalog that grows a hook or a relative import loses admission immediately.

The environment-capability predicate now counts identifiers only in value-reference positions, so a copy key named history or a parameter named location no longer reads as a browser global, and type-only imports and exports are not counted as runtime dependencies. Adversarial fixtures cover hook smuggling, implementation-import smuggling, a missing marker, dynamic imports and an unrelated dependency added beside a valid catalog.

Part of #2672.

Generated-by: Codex
@orangeCatDeveloper
orangeCatDeveloper force-pushed the fix/desktop-error-codes branch 4 times, most recently from 13b0d58 to ad06382 Compare September 2, 2026 18:13
@github-actions github-actions Bot added effort/XL Under 2500 readable lines and removed effort/L Under 1000 readable lines labels Sep 3, 2026
@orangeCatDeveloper
orangeCatDeveloper force-pushed the fix/desktop-error-codes branch 12 times, most recently from 9050c08 to 47537fd Compare September 5, 2026 04:24

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

Re-reviewed exact head 9b2f54c5b6060b1c655535f9ea89145e15fe01c0 against current main (d2d7efe645b6f1af2cb47ec9c85f28a85b32b5f9).

The previous unexpected-error handling issue is fixed. The four Plan control handlers that previously fell through after planControlIpcFailure() returned no expected envelope now rethrow the original error; the existing approve path has equivalent rejection behavior. The added regression covers all five Plan control channels with the same unexpected error and verifies that each rejects, so these failures no longer emit a false change event or return { ok: true }.

I found no new P0-P3 issue in the current PR diff. This head is not merge-ready yet because current main advanced after the PR was based and the merge tree now conflicts in apps/desktop/renderer-architecture.json; the branch needs to be rebased and revalidated.

Validation completed on this head: clean install, build:test, the focused main-process suite (15/15), Desktop tests (2238/2238), Desktop typecheck, renderer architecture tests (101/101), Biome, format, ASF header, and diff checks. The exact-head Windows package workflow passed. The required test workflow failed only in product-composer-slash-menu--context-switch-starts-with-a-loading-catalog; I reproduced the identical visibility assertion failure in an isolated worktree at the PR's exact base ef2a724537188aaafc4e3a8f2340b137109e6c5f, so I do not attribute it to this PR. I did not run native macOS behavior locally.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

@orangeCatDeveloper orangeCatDeveloper left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks — no new P0-P3 in this diff. I addressed the Astro-Han review in 4c9e7bb10 (bridge import, seam dedup, unauthorized declaration, report-then-classify shell presenter; details in the thread below), and the branch is now rebased onto current main at 0f25f62cf with the renderer-architecture.json token count resolved (verified against origin/main: ratchet passes, presentation 7/7, plan/session-domain 16/16, epoch guard passes, biome clean).

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

Reviewed exact head 0f25f62cfca23484934b1509913464e19c0ddeb0. The expected-error refactor is mostly coherent, but the latest cleanup introduces one P2 regression in the Plan Mode action guard, so I do not recommend merging this head yet.

Local validation passed: clean install, build:test, focused presentation/Plan IPC tests (23/23), full Desktop tests (2247/2247), Desktop typecheck after clearing a stale incremental cache, renderer architecture (101/101 plus base-relative ratchet), Biome, format, locale hygiene, ASF headers, protocol epoch guard, diff check, and a clean merge-tree against current main (a5022e562). A real React hook probe also reproduced the finding: while a deferred plan-control request remained unsettled, pending rendered as false.

Both required hosted checks are currently red. test fails in the unchanged transcript-scroll E2E surface, and package terminates while the Windows verifier queries the packaged process; neither failure overlaps this commit's changed paths, but the required gates still need to return green. Native macOS/Windows behavior was not run locally.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

}, [copy.operationFailed, session?.id, session?.collaborationMode, refresh]);
const run = useCallback(
async (action: () => Promise<PlanControlIpcResult<unknown>>): Promise<void> => {
setError(undefined);

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.

[P2] Restore setPending(true) before starting the action. This hook still clears the flag in finally, and all four Plan controls use planMode.pending as their only in-flight guard, but the latest formatting/refactor commit removed the matching transition to true. A React hook probe with a deferred requestPlanRevision rendered pending=false for the entire unsettled request, so users can click again and dispatch concurrent revision/approve/resume/abandon operations against the same Plan. Please restore the transition and add a production-path regression that holds one bridge promise open and verifies the controls stay disabled until it settles.

@Astro-Han Astro-Han 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.

Re-reviewed at 0f25f62 against main (a5a99a633). The increment over 9b2f54c is one commit of yours plus a rebase; the transcript files in the range are main drift from #4886, not your change.

Both P2s are fixed, and better than asked: PlanControlIpcResult now lives once in packages/runtime-host/src/protocol/plan.ts:75-89 and is re-exported from bridge-contract.d.ts:186, so shared/plan-mode-ipc.ts is gone and the duplicate-declaration P3 closes with it; the panel calls planControlFailureCopy(result, copy) (plan-mode-panel.tsx:102) and the test hits the same entry. The shell path now records the diagnostic and then classifies, unauthorized is in CONTROL_ERRORS with copy in three locales, and the epoch declaration is updated.

P1 (path ①): the plan panel lost setPending(true). main and 9b2f54c both open run with setPending(true) (main:89, 9b2f54c5:96); the head's run (plan-mode-panel.tsx:97-111) starts at setError(undefined) and only has the finally { setPending(false) }. So pending is never true and the four buttons gated on planMode.pending (:247,254,334,341) stay enabled while an action is in flight: approve twice lands on operation_conflict through expectedRevision, abandon re-prompts and re-sends. No test covers it. One line back, above setError(undefined).

P3:

  • planControlFailureCopy's result.ok branch (plan-mode-copy.ts:176-183) is unreachable now that the only caller is inside !result.ok; take error instead of result.
  • shell-copy.ts:2369-2377 calls unexpectedOperationFallback unconditionally for its console.error before classifying, so a 429 or timeout also logs an "operation failed" stack across the 52 call sites. Report only in the unclassified branch.
  • Still open from last time: the ExpectedOperationError<any> narrowing, the plan_control:<code>:<message> re-encoding in preload.ts:2377 (which the restored classifier will now read as "auth" for unauthorized), the eight session_control_blocked:* codes with no catalog, and newSessionFallbackTitle through the routing policy. None blocks.

On CI at this head: two failures, neither yours. test fails on transcript-scroll-cost.spec.ts:239 (.poll timeout, 34 passed / 1 failed); that spec and the code it drives came in with #4886, whose own run on main is green, so it is that spec's flake. Release Windows check was killed on a Get-CimInstance timeout. I will rerun once the P1 is pushed.

Evidence boundary: static read; the setPending regression is a line-by-line comparison of the same function at main, 9b2f54c and the head, not reproduced in the app; CI logs read with gh run view --log-failed.

AI-assisted review: drafted with Maka; I verified the missing setPending(true) against both earlier heads and the two P2 fixes myself.

@orangeCatDeveloper

Copy link
Copy Markdown
Contributor Author

@Astro-Han Fixed in 4624383de (rebased onto current main, c0229b0c6).

  • P1: setPending(true) is back at the top of run(). The regression test mounts usePlanModeState through the real LocaleProvider/ToastProvider chain with a stubbed window.maka.sessions, holds requestPlanRevision open with deferred, and asserts pending stays true until the promise settles — it fails on the previous head with pending must hold while the bridge call is unsettled.
  • P3: planControlFailureCopy(error, copy) now takes the envelope error; the unreachable ok branch is gone.
  • P3: localizedShellErrorMessage classifies first and only calls unexpectedOperationFallback when there is no category, with a test pinning that a timeout renders its category with zero console.error calls while an unrecognized message logs once.

The remaining non-blocking items (plan_control: re-encoding on the app-shell abandon path, the ExpectedOperationError<any> narrowing, the uncatalogued session_control_blocked:* codes) stay as follow-ups.

Verified: targeted dist tests 10/0, Desktop typecheck clean apart from stories/app-shell.stories.tsx (rightCollapsed, same on main), renderer architecture ratchet against c0229b0c6, biome, epoch guard.

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

Re-reviewed exact head 4624383dec43de8d55fdd8174928ef6f8591ee10. The previously reported Plan Mode concurrency regression is fixed, and I found no remaining P0-P3 issue in this revision.

usePlanModeState.run() now enters pending before invoking the bridge and clears it in finally, so all four controls that share this path become disabled while an action is unsettled. The new real-hook regression holds requestPlanRevision open and observes the rendered state transition. An independent mutation that removed setPending(true) made the new test fail at the pending assertion. The same commit also removes the unreachable success branch from the Plan control copy helper and reports unexpected shell diagnostics only after generalized classification fails.

Validation on this head: clean install, build:test, focused expected-error/Plan tests (25/25), full Desktop tests (2308/2308), renderer architecture (101/101), changed-file Biome, format, locale hygiene, ASF headers, and diff check. The head is based directly on current main (c0229b0c6) and the merge tree is clean. The Windows package check is green.

The required hosted test check remains red at the current-main Electron E2E budget mismatch (session-workbar.spec.ts has 6 tests while the budget records 5). Local Desktop typecheck likewise reaches unchanged current-main Storybook errors around WorkbarLayoutState.rightCollapsed. Neither surface is changed by this final commit, so I am not filing a PR finding for them, but the required gate is not green and this is not a merge approval. Native macOS/Windows behavior was not run locally.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

@Astro-Han Astro-Han 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.

Re-reviewed at a0ef3e1 against main, clean merge, CI green.

All three are fixed at the owner: setPending(true) is the first line of run() again with the finally clearing it (plan-mode-panel.tsx:99, :112), planControlFailureCopy takes the error branch only (plan-mode-copy.ts:175-180), and localizedShellErrorMessage classifies before it falls back so a classified error no longer logs (shell-copy.ts:2373-2377), with the same copy per case as before.

The regression test is the right shape: it mounts the production usePlanModeState under the real LocaleProvider / ToastProvider with only window.maka.sessions stubbed, holds requestPlanRevision open and asserts pending before, during and after (plan-mode-panel-pending.test.ts:77, :118). On the previous head pending starts false and run() wrote nothing before its await, so the middle assertion fails there. The DOM boilerplate matches goal-dialog.test.ts, which is this directory's convention.

Interdiff from 0f25f62 is those three fixes, the two signature follow-ups plus one new case in expected-error-presentation.test.ts, the new test, and rebase noise. The three follow-ups you listed stay P3: the plan_control: re-encoding has no parser and only unauthorized could misclassify, on a path Desktop's local Host does not take; the <any> narrowing is type-only; the eight session_control_blocked:* codes sit in setters the Desktop bypasses through updateConfiguration.

AI-assisted review: drafted with Maka; I verified the three fixes, the test's failure on the old head and the interdiff myself.

@Astro-Han

Astro-Han commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Approved, but the merge is refused: renderer-architecture.json conflicts with main after #4491, #3828 and #4227 landed. Please rebase and regenerate the ledger with node apps/desktop/scripts/check-renderer-architecture.mjs --write; I will merge once CI is green on the rebased head.

orangeCatDeveloper and others added 8 commits September 6, 2026 02:14
Raw exception text no longer reaches the UI: expected failures carry
machine-readable codes (with params where needed) that the renderer maps
through locale catalogs, and unexpected failures show a localized
fallback while redacted diagnostics go to the console. WorkHub waiting
summaries are full per-locale templates so they translate independently
of the status and retry paragraphs.

Generated-by: Claude Code
Claude-Session: https://claude.ai/code/session_01AqdSkg56F2x55wEGRWvzcB
Keep message-free WebSearch failures valid at the canonical result boundary so settlement cannot mark them successful. Retain redacted Work Board corruption details without exposing them in product copy.

Generated-by: OpenCode
The session-control and attachment-ingest code unions lived in the
renderer catalog while the Runtime and preload threw string literals,
so a new producer code could miss the catalog without a type error.

Generated-by: Claude Code
Desktop changes session settings through the Runtime Host's
session.configuration.update, whose failures already carry protocol
codes; the SessionManager guards that emit session_control_blocked
tokens run only on the CLI's local runtime.

Generated-by: Claude Code
Move the redacted unexpected-failure diagnostics into
@maka/core/redaction as reportUnexpectedOperation /
unexpectedOperationFallback: copy catalogs may only hold bare package
runtime imports, so shell-copy inlined the channel; the core helper
removes the inline copy and lets operation-diagnostics delegate, keeping
its public surface for the seven renderer importers.

No behavior change: the same redacted console.error line, same fallbacks.

Generated-by: Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
Code review follow-ups on the expected-errors PR:

- Fold the duplicate attachmentIngestBlocked imports in
  attachment-ingest.ts and attachment-ingest-payload.ts into the
  existing @maka/core/attachments import.
- Extract workBoardActionErrorText in work-board-panel.tsx (module-level,
  exported as a test seam): runAction and the presentation test now drive
  the exact same code-to-copy branch, so a panel mis-map fails the test
  instead of the test asserting its own re-implemented ternary. The
  expected-code lookup uses lookupCopy, so unknown/inherited keys keep
  the caller fallback.
- The copy catalog stays import-free (copy catalogs may only hold bare
  package runtime imports), keeping the transport concern at the panel.

Generated-by: Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
CI ratchet flagged the new @maka/core/ui-locale edge on
work-board-panel.tsx as dependency debt (7 -> 8). Move the expected-code
lookup into work-board-error-copy.ts as workBoardErrorCodeCopy: a
validated copy catalog's bare-package runtime imports are exempt from
pricing, so the panel sheds the new edge and the catalog's existing
lookupCopy import becomes live again.

No behavior change; the panel's workBoardActionErrorText keeps the same
branch shape over the catalog helper.

Generated-by: Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
@orangeCatDeveloper

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (00cada01c); head is now 364a6467b. The renderer-architecture.json conflict was resolved by regenerating the ledger with --write at each affected commit, so every commit in the range carries a consistent ledger rather than one fixup at the tip.

Verified on the rebased head: Desktop suite 2311/0, four typechecks 0 errors (the rightCollapsed story drift is gone now that #4895 is in), renderer architecture ratchet against 00cada01c, E2E budget holds (31 tests in 16 files), Biome clean.

orangeCatDeveloper and others added 6 commits September 6, 2026 02:18
Code-review follow-ups on the expected-errors PR:

- P2: plan-mode-panel classified every rejection as unexpected and showed
  only the generic line. The plan.control / plan.turn.start protocol
  failures carry stable codes (RuntimeHostOperationError), so the panel
  now maps them through a new planModeCopy.controlFailure table
  (session_busy, operation_conflict, not_found, persistence_failed, ...)
  in all three locales, and keeps the unexpected diagnostics channel for
  genuinely unknown failures.
- P3: the attachment_ingest token matcher now anchors at the message
  tail, so an unrelated error message that merely contains the substring
  (e.g. a path like /tmp/attachment_ingest:count_limit/report.txt) keeps
  its fallback instead of rendering attachment copy; negative tests pin
  this.
- RuntimeHostOperationError moves to the protocol package so the renderer
  can consume the structured code without a client-layer import; the
  client re-exports it for existing importers.
- The plan code lookup lives in plan-mode-copy.ts (a validated copy
  catalog whose bare imports the debt ratchet exempts), so neither
  surface gains a new dependency edge; workBoardActionErrorText keeps
  its shape and the presentation test drives both seams.

Generated-by: Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
The automated review's second pass was right: Electron IPC strips custom
fields from thrown Errors, so the panel's instanceof check never matched
in production - the renderer only ever saw a plain Error with a message.
A real bridge path test (thrown typed error arrives as {name: Error,
ownKeys: []}) confirmed the boundary; the previous unit test constructed
the class renderer-side and missed it.

The plan-mode control channels now return a structured envelope
(PlanControlIpcResult, mirroring the WorkBoardIpcResult precedent):

- main: the five plan-mode handlers catch RuntimeHostOperationError and
  return {ok:false, error:{code,message}} for expected plan-control
  codes; anything else still rejects so it takes the
  unexpected-diagnostics path.
- preload/bridge-contract: the four panel-only methods pass the envelope
  through; abandonPlanProposal keeps its throwing shape (app-shell's
  legacy closure is token-frozen, and the session-setting intent renders
  its generic failure copy through sessionSettingFailureCopy).
- renderer: planModeActionErrorText is gone; run() checks the envelope
  and maps error.code through planModeCopy.controlFailure with the
  generic line as fallback. The envelope type and the code re-export
  live in plan-mode-copy.ts, whose bare imports the debt ratchet
  exempts; shared/plan-mode-ipc.ts (main/preload) holds the same shape
  for the main side. app-shell.tsx is untouched.
- Tests: the presentation test drives planControlFailureCopy with the
  wire envelope (expected codes per locale, success envelopes, and an
  unknown future code falling back), and the IPC test's plan assertions
  expect the envelope.

RuntimeHostOperationError stays in the protocol package under the
declared compatible extension at epoch 117.

Generated-by: Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
Code-review follow-up: planControlIpcFailure returns undefined for any
rejection outside the expected plan-control set (network faults,
programming errors, non-plan.* operations), and the four handlers that
caught it fell through to emitModeChanged plus an ok:true return,
silently reporting failed user actions as success and bypassing the
renderer's unexpected-error diagnostics. approve already rethrew via
its rejection handler.

Every handler now rethrows when no envelope was produced, and a new
test drives all five channels with a non-plan control failure
('socket exploded') asserting the exact cause propagates.

Generated-by: Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
- bridge-contract.d.ts re-exports PlanControlIpcResult from the protocol
  package, so the five plan bridge methods stop degrading to Promise<any>
  under skipLibCheck
- plan-mode-panel.tsx resolves failures through planControlFailureCopy
  (the tested seam) instead of an inline duplicate; the seam now takes
  (result, copy) and looks up through lookupCopy
- PlanControlIpcResult moves into @maka/runtime-host/protocol (a wire
  contract next to the codes it carries) and the duplicated shared/
  plan-mode-ipc.ts declaration is deleted; the catalog re-exports it as a
  bare-package type import so the renderer closure stays unchanged
- 'unauthorized' is declared on plan control operations (the dispatcher
  already admitted it unconditionally) with copy in all three locales;
  the epoch compatible-change declaration is updated
- localizedShellErrorMessage reports the redacted diagnostic first, then
  keeps the shell's generalized classifier, so shell call sites stay as
  specific as the sibling presenters that still classify
- trailing whitespace in the panel parameter list

Verified: desktop typecheck (4 tsconfigs) 0 errors, renderer
architecture ratchet passes against ef2a724, presentation 7/7 +
plan-mode/session-domain tests 16/16, protocol epoch guard passes,
biome clean on touched files.

Generated-by: Claude Code
The expected-error refactor dropped the pending transition, so the four
plan controls stayed enabled during an in-flight request and a second
click raced the first. Also report an unexpected-failure diagnostic only
when the shell classifier has no category, and let the plan failure
lookup take the envelope error directly.

Generated-by: Claude Code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Under 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants