fix(desktop): localize expected errors - #4457
Conversation
53b08a5 to
b6ef648
Compare
|
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. |
adf4f4a to
6b28e30
Compare
…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
13b0d58 to
ad06382
Compare
9050c08 to
47537fd
Compare
3c22d3a to
5925b9f
Compare
hqhq1025
left a comment
There was a problem hiding this comment.
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.
Astro-Han
left a comment
There was a problem hiding this comment.
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'sresult.okbranch (plan-mode-copy.ts:176-183) is unreachable now that the only caller is inside!result.ok; takeerrorinstead ofresult.shell-copy.ts:2369-2377callsunexpectedOperationFallbackunconditionally for itsconsole.errorbefore 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, theplan_control:<code>:<message>re-encoding inpreload.ts:2377(which the restored classifier will now read as "auth" forunauthorized), the eightsession_control_blocked:*codes with no catalog, andnewSessionFallbackTitlethrough 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.
0f25f62 to
4624383
Compare
|
@Astro-Han Fixed in
The remaining non-blocking items ( Verified: targeted dist tests 10/0, Desktop typecheck clean apart from |
hqhq1025
left a comment
There was a problem hiding this comment.
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.
4624383 to
a0ef3e1
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
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.
a0ef3e1 to
364a646
Compare
|
Rebased onto current Verified on the rebased head: Desktop suite 2311/0, four typechecks 0 errors (the |
364a646 to
df0f399
Compare
|
@Astro-Han Rebased and CI passed now |
6e6bef9 to
5625712
Compare
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed the complete 50-file merge-base diff at 5625712. The earlier Plan false-success and missing-pending defects are fixed. I found no reproduced P0–P2 production defect; one nonblocking P3 regression-test gap is detailed inline.
This changes expected-error transport and presentation, not just copy: Plan preserves structured Host failures over IPC; Work Board, attachments and collaboration map stable codes to locale catalogs; unexpected errors use redacted diagnostics; WorkHub copy is separated; WebSearch settlement retains a nonempty error message. I followed producer/transport/presenter paths and verified all declared Plan error codes through the registered handlers, including an actual missing-proposal storage failure. The deferred React hook test also confirms pending stays true until the bridge settles.
A clean lockfile install, repository dependency patches, full build:test and 158 focused compiled tests passed. The independent producer/IPC/pending probes passed separately. Hosted test and package are successful on this head. No real Electron UI, native-platform acceptance or full-repository test run is claimed.
This head is not merge-ready: GitHub reports CONFLICTING/DIRTY, and merge-tree against current main 2310035 independently reports conflicts in renderer-architecture.json and workhub-surface.tsx. Resolve those and validate the resulting new head; the current review cannot certify that future merge result.
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.
5625712 to
ae0e0cf
Compare
hqhq1025
left a comment
There was a problem hiding this comment.
Re-reviewed the complete rebased change at ae0e0cf. The previous P3 regression-test gap is fixed; no new substantiated P0–P3 finding.
The new test exercises all five registered Plan IPC handlers with typed Host failures, checks the preserved error code and asserts that no mode change is emitted. Disabling the actual production conversion now makes that test fail, unlike the previous head. The unexpected-error rethrow and React pending regressions also remain covered. I checked the rebase adaptations for current WorkHub resume outcomes/clarifications, locale catalogs, fallback-title callers and the protocol compatibility acknowledgment.
On this isolated head, clean install plus repository dependency patches, full build:test, 167 focused compiled tests and 101 renderer-architecture tests all passed; the actual architecture check passed too. Real Electron visual behavior, the full Desktop suite and native-platform acceptance were not run.
The previous main-conflict caveat is now resolved: GitHub reports MERGEABLE/CLEAN, hosted test/package are successful, and a local merge-tree against current main 2310035 is clean. This COMMENT records source and test results; it is not a human review or merge authorization.
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.
Squash of the stacked increment, rebased onto fix/desktop-error-codes (apache#4457) after its rewrite onto main: typed AttachmentIngestBlockedError with the attachment_blocked envelope, ExpectedOperationError across the preload and session-settings surface, typed session-configuration transition errors in the Runtime, and the sessionSettingsActions updateFailures catalog per locale. localizedShellErrorMessage no longer parses any token. Generated-by: Claude Code Generated-by: OpenCode
Squash of the stacked increment, rebased onto fix/desktop-error-codes (apache#4457) after its rewrite onto main: typed AttachmentIngestBlockedError with the attachment_blocked envelope, ExpectedOperationError across the preload and session-settings surface, typed session-configuration transition errors in the Runtime, and the sessionSettingsActions updateFailures catalog per locale. localizedShellErrorMessage no longer parses any token. Generated-by: Claude Code Generated-by: OpenCode
|
@Astro-Han Rebased. PR is ready to be merged |
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for the localization work. Additional independent review of ae0e0cff46611dcdf26472b95f0455c0f8d4f108 found one remaining attachment acceptance gap in the normal new-task send path. The IPC envelope behavior was independently exercised in Electron; the finding concerns a renderer preflight that runs before that boundary.
AI disclosure: Codex agents performed this review, and the coordinating agent checked the production call path and reran the preflight/presenter counterexample. This is not an independent human review.
中文摘要
重新审查发现新建任务发送附件时仍有一条未覆盖的正常路径。Electron IPC envelope 已独立实测;这里的问题发生在进入 IPC 前的 renderer 预检。主审已核对真实发送调用链,并重跑预检与错误映射反例。这是自动化评审,不冒充人工验收。
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for the updates. A fresh, multi-reviewer pass at ae0e0cff46611dcdf26472b95f0455c0f8d4f108 found one additional, optional ownership simplification below. The previously reported new-task attachment preflight issue remains unchanged. This review was AI-assisted and cross-checked by the coordinating AI reviewer; it does not represent completed human visual acceptance.
中文
谢谢更新。本轮独立深审新增一条可选的职责归属精简意见;此前新任务附件预检的问题仍未变化。本次由 AI 协助审查并由主审 AI 交叉验证,不代表已经完成人工视觉验收。
6474464 to
f1735c5
Compare
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. Plan controls and work-board actions carry those codes across IPC in a structured envelope, since Electron strips custom fields off thrown Errors. Generated-by: Claude Code
…est tokens The new-task send path runs preflightAttachmentItems in the renderer before newTasks.create, so the main-side token validation never sees an over-limit request - and the preflight threw localized prose, which the send catch's token-only presenter cannot recognize. A user sending nine attachments got "The message could not be sent. Try again later." in both locales (retrying the unchanged input can never succeed) plus a spurious unexpected-error diagnostic, losing the actual count reason. The preflight now rejects with the same stable attachment_ingest:<code> tokens the main-side resolveIngestItems validation rejects with (count_limit / item_too_large / duplicate_source - same semantics as the mirrored main checks), so localizedShellErrorMessage maps the real reason with no new code path; renderer-local throws never cross the IPC boundary, so the end-anchored tail matcher sees the bare token. The quote-companion send catch rendered error.message directly and would have leaked the raw token, so it now routes through localizedShellErrorMessage. The conversation-copy attachments prose the preflight was the only consumer of is deleted. Regression drives the real createAppShellChatActions send path with nine staged attachments: the send is rejected, newTasks.create is never called, the toast carries the count_limit reason in zh-CN and en, and console.error stays silent. It fails on the previous head with the generic fallback; the rewritten preflight unit test pins the exact tokens (and the oversized-approval-by-pending-size case). Verified: desktop full dist suite 2336 pass / 0 fail (send-path test fails on the previous head, passes here), typecheck (4 tsconfigs) 0 errors, renderer architecture ratchet against 2310035, biome clean. Generated-by: pi (GLM)
f1735c5 to
9e1759d
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks. A focused size/complexity follow-up is attached inline. This is AI-assisted source review, cross-checked by the coordinating reviewer; no new full-suite run.
中文
感谢更新,行内补充一条规模与复杂度复核意见。本次为 AI 辅助源码审查,经主审交叉核对,未重跑全仓测试。| errorClass: string; | ||
| }; | ||
|
|
||
| export type SessionControlBlockedCode = |
There was a problem hiding this comment.
[P3] Leave the future-only Session-control token contract out of this localization change
Thanks for documenting the reachability limit explicitly. The PR body says these eight guards have no current in-repository caller path and the token contract is reserved for a future CLI-local caller. The new session_control_blocked:* family has no renderer presenter; current Desktop configuration uses the Host protocol errors instead. Could we revert this new union/helper, the eight exception-text replacements and the two corresponding regex-only test edits, while preserving the guards themselves? This leaves the reachable Desktop localization behavior intact and avoids adding an unused error contract. This is scope/maintenance cleanup, not a hypothetical future runtime defect; the future-only path is not being reported as a reachable bug.
中文
感谢明确说明可达性限制。正文确认这八个 guard 当前没有仓内调用路径,新增 token 是为未来 CLI-local 调用方保留;现有 Desktop 配置走 Host 协议错误,这套 token 也没有 renderer 映射。建议撤回新增 union/helper、八处错误文本替换及两处仅更换正则的测试修改,保留 guard 本身。这样不影响当前可达的 Desktop 本地化验收,也不增加尚未使用的错误契约。这是 P3 范围和维护性精简,不把未来假设路径当作运行时缺陷。
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for the localization work and the production-path fixes. The combined review/simplification audit found three additional P3 opportunities to reduce new contracts; they are inline. The existing future-only Session-control token comment still applies.
One further optional consolidation spans several files: reportUnexpectedError and unexpectedErrorFallback merely forward to the new core diagnostic helpers. Diagnostic callers can use those core helpers directly, removing the forwarding API and its dependency edges; keep ExpectedOperationError, which has actual WorkBoard/WorkHub consumers.
The Plan IPC envelope itself, domain-owned catalogs, attachment preflight validation, and the real pending/IPC regression coverage have current obligations and should remain.
AI-assisted review by Codex; checked against the production producers and consumers.
中文
本轮新增三项 P3 简化意见见行内,之前未来 Session-control token 的意见仍适用。另可让诊断调用方直接使用 core helper,删除两个纯转发 API;ExpectedOperationError 有真实消费者,应保留。Plan IPC envelope、领域 catalog、附件预检及真实 pending/IPC 回归保护都有当前义务,不建议整片删除。
| readonly results: ReadonlyArray<WebSearchResultRow>; | ||
| } | ||
| | { readonly ok: false; readonly reason: WebSearchErrorReason; readonly message: string }; | ||
| | { readonly ok: false; readonly reason: WebSearchErrorReason; readonly message?: string }; |
There was a problem hiding this comment.
[P3] Keep the existing Web Search error contract for this Desktop localization change
The Desktop settings consumer already renders copy.errors[result.reason] for query/test failures. Removing the preload diagnostic message therefore does not improve localization there, but making this public field optional spreads the change into the Runtime fallback and the new four-case ToolRuntime settlement matrix. The Host wire decoder still requires a string message, and the real Host/Tavily producers still supply it.
Could we keep message required, have the preload early returns supply a stable locale-independent diagnostic, and remove the optional-field change plus the Runtime fallback/test expansion? This preserves the existing producer/codec contract and the localized UI behavior. This is a normal-path scope/maintenance concern, not a claim that current Host producers omit the field.
AI-assisted review by Codex; checked against the production producers and consumers.
中文
设置页已经按 reason 查本地化文案,不直接展示 message。为了删除 preload 诊断文本而把公共 message 改为可选,又引出 Runtime 兜底与四组 settlement 测试;Host wire decoder 仍要求必填字符串。建议保留原必填契约,preload 返回稳定、与 locale 无关的诊断文本,撤掉由可选字段引出的整条扩张。当前真实 Host producer 并未省略字段,此项为 P3 范围与维护问题。
| /** Thrown client-side when a Runtime Host operation resolves to a failure | ||
| * outcome; the structured `code` lets presenters map expected failures to | ||
| * copy instead of treating them as unexpected. */ | ||
| export class RuntimeHostOperationError extends Error { |
There was a problem hiding this comment.
[P3] Keep the client exception in its existing client export
RuntimeHostOperationError is constructed by the Host client when a wire failure becomes a local exception. The new Desktop main adapter can import the existing class from @maka/runtime-host/client, as the other production consumers do. Moving it into /protocol adds a public runtime export, a client re-export and compatibility bookkeeping without changing any wire behavior.
Please retain the original class owner/export and switch the new main-process catch to that import. Likewise, PlanControlErrorCode can use a file-private CONTROL_ERRORS; exporting that runtime array is not required by its type alias. Keep the actual Plan code type/spec changes and adjust, rather than blindly delete, their compatibility declaration.
AI-assisted review by Codex; checked against the production producers and consumers.
中文
这个异常由 Host client 将 wire failure 转成本地异常时构造;Desktop main 可直接使用原来的 client 导出。搬进 protocol 再从 client 转导出,新增公共运行时接口和兼容维护,却没有新增 wire 语义。建议恢复原归属;CONTROL_ERRORS 也只需在文件内供类型别名使用。保留真正需要的 Plan 错误码类型/spec 改动及相应声明。
| } | ||
|
|
||
| export type AttachmentIngestBlockedCode = | ||
| | 'count_exceeded' |
There was a problem hiding this comment.
[P3] Use one attachment reason per validation outcome
The new token union distinguishes preload count_exceeded / size_exceeded / payload_invalid from main/preflight count_limit / item_too_large / items_invalid. These represent the same count limit, single-item size limit and invalid-input outcomes; the presenter does not branch by producer and instead maintains parallel copy entries in every locale.
Could preload emit the latter three codes as well, removing the three aliases and duplicate catalog entries? Keep the separate total-size, duplicate-source and expired-source outcomes. These tokens are introduced by this unmerged change, so this consolidation does not require a legacy token reader. The visible difference is consistent wording for the same failure at each layer.
AI-assisted review by Codex; checked against the production producers and consumers.
中文
preload 与 main/preflight 对同一数量上限、单件大小上限、无效输入分别引入两套名字,展示层并不按生产层区分,却要维护三种语言的重复文案。建议统一成 count_limit / item_too_large / items_invalid,删除另三个别名及重复 catalog;总量超限、来源重复、来源过期仍是不同语义,应保留。
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 typedExpectedOperationError; everything else logs a redacted diagnostic and shows a localized fallback. This also deletes WorkHub's English-message-regex classifier and theMAKA_SESSION_READ_MESSAGES_ERRORmarker protocol.Per review, the global error mapper is split: each domain maps its own code union at its presenter; the
ExpectedOperationErrortransport and redacted diagnostics live once in the renderer'sapplication/contracts/operation-diagnosticsentry, now delegating toreportUnexpectedOperation/unexpectedOperationFallbackin@maka/core/redactionso copy catalogs (bare-import-only) and the renderer share one diagnostics channel. They are Desktop-only, and hosting them in@maka/uipulled the icon bundle into the main-process test bundles. The attachment validations (15 in the main-side ingest resolver plus 3 in the preload payload cap and 3 in the renderer preflight) reject with stableattachment_ingest:<code>tokens that survive the Electron IPC wrapper, mapped at the sharedlocalizedShellErrorMessageentry — both locales previously lost these reasons to a generic fallback, and thetotal_size_exceededbudget rejections introduced by #4956 are tokenized in this branch too. The Runtime's 8 session-control guards emitsession_control_blocked:<code>tokens; today no in-repo caller reaches those guards (Desktop changes session settings throughsession.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):
After (same failures; raw detail now only in the redacted console diagnostic):
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-diagnosticskeeps its public surface for its seven renderer importers and delegates to the core helper. No behavior change.Review follow-up (
575f10b39+fe1190919): folded the duplicateattachmentIngestBlockedimports; extractedworkBoardActionErrorTextinwork-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 asworkBoardErrorCodeCopyso 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: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.RuntimeHostOperationError— the previousinstanceoffix was unreachable in production. The five plan-mode control channels (requestRevision,abandon,approve,resume,abandonExecution) now return a structuredPlanControlIpcResultenvelope (mirroring theWorkBoardIpcResultprecedent): main catches the typed rejection and returns{ok:false, error:{code,message}}; the panel checks the envelope and mapserror.codethroughplanModeCopy.controlFailure(all three locales), with unknown/futurecodes falling back to the generic line.abandonPlanProposalkeeps its throwing shape (the app-shell legacy closure is token-frozen; its generic failure copy flows throughsessionSettingFailureCopy). The envelope type lives inplan-mode-copy.ts(bare imports exempt from the debt ratchet) andshared/plan-mode-ipc.ts(main/preload) — its final placement is round 6 below.RuntimeHostOperationErrorstays in the protocol package under the declared compatible extension (epoch synced tomain's current value in round 6). The panel seam test now drivesplanControlFailureCopywith 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-compositionsandbox 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 —emitModeChangedfired and the renderer gotok:truefor a failed action. Every plan-mode handler now rethrows whenplanControlIpcFailureproduces 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 ontomainc0229b0c6): the cleanup in0f25f62cfhad droppedsetPending(true)from the plan panel'srun(), so the four controls gated onplanMode.pendingstayed enabled while an action was in flight. Restored, with a hook-level regression that mountsusePlanModeStatethrough the real providers, holds one bridge promise open withdeferred, and assertspendingstaystrueuntil it settles (fails without the fix:pending must hold while the bridge call is unsettled). Two P3s along with it:planControlFailureCopytakes the envelopeerrordirectly (theokbranch was unreachable), andlocalizedShellErrorMessagerecords 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 exceptstories/app-shell.stories.tsx(rightCollapsed, identical tomain), renderer architecture ratchet againstc0229b0c6, 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 draft blocker (#4493 checker policy) landed on 2026-09-02.
Review follow-up round 5 (
9e1759de0, on the branch rebased ontomain): the new-task send path ran the renderer-sidepreflightAttachmentItemsbeforenewTasks.create, so the main-side token validation never saw an over-limit request — and the preflight threw localized prose the token-only presenter cannot recognize, so a nine-attachment send got the generic "try again later" fallback in both locales (retrying the unchanged input can never succeed) plus a spurious unexpected diagnostic. The preflight now rejects with the same stableattachment_ingest:<code>tokens main'sresolveIngestItemsrejects with (count_limit/item_too_large/duplicate_source, same semantics as the mirrored main checks), solocalizedShellErrorMessagemaps the real reason with no new code path; renderer-local throws never cross the IPC boundary, so the end-anchored tail matcher sees the bare token. The quote-companion send catch — which renderederror.messagedirectly and would have leaked the raw token — now routes throughlocalizedShellErrorMessage. The conversation-copy attachments prose the preflight was the only consumer of is deleted. The regression drives the realcreateAppShellChatActionssend path with nine staged attachments (send rejected,newTasks.createnever called, toast carries thecount_limitreason in zh-CN and en,console.errorsilent) and fails on the previous head; the rewritten preflight unit test pins the exact tokens (and the oversized-approval-by-pending-size case).Review follow-up round 6 (folded into the first commit, same rebase): the plan-control envelope moved out of the Host protocol package — no wire codec uses it and only Desktop consumes it — into a type-only
apps/desktop/src/shared/plan-mode-ipc.d.tsbeside theWorkBoardIpcResultseam, following thesession-collaboration.d.tsprecedent so it stays outside the renderer architecture debt closure (a plainsrc/sharedmodule imported from the app-shell closure would be a newlegacyAppShellClosuredebt entry, which the checker forbids).PlanControlErrorCodestays in the protocol next to theCONTROL_ERRORSlist it aliases; the compatible-change declaration keeps covering exactly the real protocol changes (the error-class move, the code alias, theunauthorizeddeclaration).Review follow-up round 7 (rebase onto
main7779d69a7, which landed #4956 durable local delivery): the rebase resolution keeps main's newprepareIngestItems/commit(admit)structure with every prose rejection converted to a token — the pre-existing checks map to their existing codes, and #4956's new total-size budget rejections (5 sites across the main-side prepare/commit and the preload payload cap) map to a newtotal_size_exceededcode with copy in all three locales (the existingsize_exceededcopy describes a single oversized item, which would misrender a total-budget breach). Main's new tests that asserted the prose (/过期|无效/,/附件总量超出大小限制/) now pin the tokens instead. The compatible-change declaration synced to epoch 124 (equal tomain's). Branch history:a83b34bb4(localization + envelope placement + #4956 reconciliation) +9e1759de0(attachment preflight tokens) on7779d69a7.Verified on the rebased head: desktop full dist suite 2357 pass / 0 fail, four typechecks 0 errors, renderer architecture ratchet against
7779d69a7(no new debt entries; the ledger is consistent at each commit — checked out and re-verified the first commit separately), biome clean on all touched files, protocol diff vsmainis exactly the two declared files at epoch 124.AI use
Select exactly one:
Tool(s) and scope: Claude Code — analysis, implementation, tests, and this description, under the contributor's direction; the commit carries a
Generated-by: Claude Codetrailer.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-diagnosticsdelegating), verified the branch against currentmain, and updated this description.pi (GLM) landed the attachment-preflight token round and the plan-control envelope ownership move (
9e1759de0; the envelope move folded intoa83b34bb4), rebased the branch onto currentmainwith the epoch declaration reconciled to main's current value, and verified the full suite; those commits carryGenerated-by: pi (GLM)trailers.Checklist