fix(i18n): map expected failure codes instead of raw messages - #4641
fix(i18n): map expected failure codes instead of raw messages#4641orangeCatDeveloper wants to merge 1 commit into
Conversation
14a80d6 to
95b736e
Compare
Runtime Host management and thread search already carry stable failure codes, so the renderer maps them through locale catalogs with an explicit unknown fallback. The CJK sniffs in the provider and artifact error presenters guarded producers that no longer throw Chinese copy. Generated-by: Claude Code
95b736e to
6efe16e
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
The two surfaces that actually got codes (the management dialog and thread search) trace cleanly, and I verified the CJK guards really were dead (every throw in runtime-host-connections-ipc-main.ts is English, and generalizedErrorMessage returns the fallback rather than the raw text on a miss), but three things should change before merge.
P2: failure codes still have no single authority. The 26-code union is re-declared at the presenter while the producers keep their unions inline in four CLI constructors, and the only link between them is a regex that scrapes CLI source at test time. Details inline on settings-projects-copy.ts.
P3: the PR title reads wider than the change. Only the management dialog and thread search became code-driven. provider-panel-shared.ts and artifact-pane.tsx kept raw-message matching as their main path (the lastTest table keyed on lowercased message text, the connection_stale regex, and the keyword classifier); this PR only removed their CJK passthrough. Worth saying that in the body so a reader does not close out the whole item.
P3: shell-controls-copy.ts still carries search.privacyTitle, search.errorTitle and search.statusRegionLabel, none of which search-modal.tsx references any more (its full set of copy uses is title, placeholder, conversationsLabel, resultsLabel, empty, introduction, unavailable, errorByReason, errorFallback), and that file is the only consumer of getShellControlsCopy(...).search. Since this PR is renovating exactly that block, they can go with it.
P3: reconnectWarning is now effectively a boolean. runtime-host-management-dialog.tsx:142 declares useState() but line 596 can only ever store copy.managementReconnectFailed. Make it a boolean, and type the applyReconnectWarning parameter as DesktopRuntimeHostManagementResult['reconnectError'] instead of re-spelling the shape.
|
|
||
| export function runtimeHostManagementErrorMessage(code: string, locale: UiLocale): string { | ||
| const messages = getSettingsProjectsCopy(locale).runtimeHost.managementError; | ||
| return (messages as Record<string, string>)[code] ?? messages.unknown; |
There was a problem hiding this comment.
P2: this ?? reaches Object.prototype. The catalog is a plain object literal, so a code of constructor, toString or valueOf indexes to the inherited function, ?? never fires, and setError() renders function Object() { [native code] } into the banner. The wire schema bounds the code's length but not its characters, and the code is chosen by the operator side, so this is reachable across a trust boundary rather than only by a local bug. The PR body says this uses an Object.hasOwn fallback; the code does not. Smallest fix: return Object.hasOwn(messages, code) ? (messages as Record<string, string>)[code]! : messages.unknown; which is the shape already used in model-connection-errors.ts:51. searchErrorText in packages/ui/src/search-modal.tsx:128 has the same shape and can take the same fix.
| import type { UiCatalog, UiLocale } from '@maka/core/ui-locale'; | ||
|
|
||
| // The operator CLI keeps code an open string for version skew; this is the subset Desktop presents distinctly. | ||
| export type RuntimeHostManagementErrorCode = |
There was a problem hiding this comment.
P2: this re-declares a code list the producers already own, so there are now two spellings of it and no compiler link between them. The four CLI error classes keep their unions inline in the constructor (packages/cli/src/runtime-host-service-manager.ts:287), and the only thing tying the two together is extractErrorCodes() scraping that source with a regex. A rename on the CLI side fails the desktop suite with an opaque assertion, and if either anchor moves (readonly code: / message: string, e.g. someone adds a field between them) the scrape silently stops covering anything while the test stays green and new codes fall to unknown.
@maka/runtime-host/operator is a seam both sides already depend on: packages/cli imports from it, and bridge-contract.d.ts already pulls types from it. Declaring the union next to SERVICE_ERROR_SCHEMA there, having the CLI error classes reference it, and typing managementError as Record<Code | 'unknown', string> gets exhaustiveness from tsc and lets extractErrorCodes/readCliSource be deleted.
| }, [result]); | ||
|
|
||
| function reportManagementError(code: string): string { | ||
| const message = runtimeHostManagementErrorMessage(code, locale); |
There was a problem hiding this comment.
P2: response.error.message is now dropped entirely here and at the load path around line 193, with nothing logged. That message is the only place the operator says which path or unit failed (deployment_io_failed carries the EACCES path, for example), so a user hitting this has nothing to report and a maintainer has nothing to read. This PR added console.error for the reconnect path at line 599 and for thread search at search-modal.tsx:112, so this is also inconsistent with its own choices. Smallest fix: console.error('[runtime-host] management failed', response.error); inside reportManagementError, and the same at the load path.
| // Main-process handlers throw display-ready Chinese copy; keep it instead | ||
| // of flattening it into a coarser classification or the generic fallback. | ||
| if (/[\u3400-\u9fff]/.test(cleaned)) return cleaned; | ||
| if (/connection_stale|Unable to delete Connection: connection_stale/i.test(cleaned)) { |
There was a problem hiding this comment.
P3: this is the exact pattern the PR title targets, left in the file the PR edits. Main throws new Error('Unable to delete Connection: connection_stale') at runtime-host-connections-ipc-main.ts:333 and the renderer regexes the code back out of the sentence. Same for the lastTest lookup on line 38, which is keyed on lowercased message text. Not asking you to fix it here, but the body should say that this surface and artifact-pane.tsx only lost their CJK passthrough this round and are still raw-message classified, otherwise it reads as if the whole path is code-driven now.
| input.onErrorChange({ | ||
| reason: 'provider_error', | ||
| message: input.thrownErrorMessage(caught), | ||
| message: caught instanceof Error ? caught.message : String(caught), |
There was a problem hiding this comment.
P3: message is now dead data. Nothing renders it any more (emptySearchText uses only reason) and the raw error already goes to console.error on the line above, yet it is still threaded through ThreadSearchSourceInput.onErrorChange at line 57, both onErrorChange calls, and the useState at line 152. Narrowing all of them to { reason } removes the last carrier of the raw text.
Summary
Five renderer sites still rendered a raw
error.messageor decided what to show by sniffing for CJK characters, so English users saw operator/Host prose verbatim (or, where sniffed, lost the information). Each is now code → catalog:error.codeas an openstringon the wire (operator version skew), so the closed union lives at the presenter —RuntimeHostManagementErrorCode(26 codes) mapped per locale insettings-projects-copy.tswith anObject.hasOwnunknown fallback; all fiveresponse.error.messagerenders go through it. A test reads the CLI'sRuntimeHostServiceManagerErrorcode union from source and asserts every code is mapped, so a new CLI code fails the desktop test suite instead of silently falling back.error.reasonwas already a typedSearchErrorReason; the five reasons thread search emits are mapped inshell-controls-copy.ts, the rest fall back, and thethrownErrorMessageseam that renderederror.messageis gone (raw error goes toconsole.error).provider-panel-shared.tsandartifact-pane.tsx: the CJK-passthrough guards were dead — the main handlers they defend against throw English only today — so they are deleted with a guard test.Not changed:
skill-status.tssniffsskill.description, which is third-party SKILL.md data rather than our copy; the right fix is to show data as-is and drop the keyword blurbs, a product decision left for a separate discussion.Refs #2672
Verification
AI use
Select exactly one:
Tool(s) and scope: Claude Code — producer tracing, implementation, tests, and this description, under the contributor's direction; the commit carries a
Generated-by: Claude Codetrailer.Checklist