fix(cli,i18n): finish the --json truncation sweep, and close #3762 — where the debt was 1 string, not 231 - #3803
Merged
Conversation
…econd document it was hiding #3780 routed three commands through `emitJson`. The other ~100 emission sites still wrote machine output with `console.log`, which on a PIPE is cut off at one 64 KiB buffer when the command exits right after. It is invisible to whoever writes it: stdout to a TTY is synchronous, so every interactive run looks perfect while every scripted consumer — the only audience `--json` has — gets invalid JSON. The exit need not be explicit. oclif ends failing commands with `handle()` → `Exit.exit()` → `process.exit()` and flushes nothing on that path (`flush()` runs only on `execute()`'s success path), so a plain `this.exit(1)` or any thrown error truncates identically — 73 of the 104 sites had that shape. `lint` was only half fixed by #3780: `--eval --json` writes a whole corpus report and was still on `console.log`. - All 104 sites go through `emitJson`; `formatOutput`'s json/yaml branches through the same drain-aware `emitText` (`--format yaml` truncated too, and it is now async — 32 call sites await it). - Control flow is untouched. A following `this.exit(1)` stays; it is simply safe once the buffer has drained. - Output bytes are unchanged. Roughly half the sites emitted compact and half indented; `{ compact: true }` preserves whichever each had, so this is a truncation fix and not an output-format change. Unifying that split is worth doing as its own decision. - An ESLint rule (`--no-inline-config`, so no per-site opt-out) rejects `console.log(JSON.stringify(…))` under packages/cli/src and un-awaited `formatOutput`. Draining the write exposed a second defect underneath it. Because `this.exit(1)` THROWS, a command whose body sits in one `try` unwinds its inner "report and stop" into the outer `catch`, which reports again: `os validate --json` on a failing config printed TWO JSON documents — neither valid JSON nor valid JSONL. Truncation had been hiding the second one. Nine commands had this shape; their catches now re-throw the exit signal (`isExitSignal`) rather than describing it as a failure. Measured on `os validate --json`, 900 schema errors, piped: before 131072 bytes (exactly two buffers, cut mid-string) unparseable truncation fix only 1514711 bytes (two documents) unparseable both fixes 1514648 bytes (one document, 900 errors) parses Verified both directions rather than only the green one: the integration test goes red against origin/main sources, and a control case asserts the `console.log` pattern it replaced genuinely truncates, so the gate cannot quietly stop testing anything. Deliberately NOT fixed by forcing stdout into blocking mode process-wide, which would be one line and cover every command including future ones: the same binary runs `os serve` / `os dev`, and a blocking write to a pipe with a slow reader blocks the event loop — trading truncated JSON for a server that stalls on its own logs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BQwnjqgCyAhSQjhNriW3BW
Closes the rest of #3762, recorded as "platform-objects is 77 strings short per locale in apps.*/dashboards.*, and its --objects-only extract cannot scaffold them — needs an emit decision (drop --objects-only, or a companion .apps.generated.ts) before any translating." Measured, the premise did not hold. Of the 77 declared keys per locale, 76 were ALREADY translated in the hand-authored <locale>.ts files and had been for months. Exactly one was genuinely missing — apps.studio.navigation.nav_app_builder.label, absent in all four locales including en. The 231 was a measurement artifact: this config declares SETUP_APP / STUDIO_APP / ACCOUNT_APP and SystemOverviewDashboard, but its `translations` merge baseline listed only the two GENERATED subtrees, so coverage counted every hand-authored app/dashboard key as untranslated. Neither proposed emit is right, and the second would have caused damage. The Setup app is a shell of empty group anchors; its ~25 menu entries are contributed at RUNTIME by SETUP_NAV_CONTRIBUTIONS and by capability plugins (ADR-0029 D7), so a bundle generated from a static walk of SETUP_APP is structurally incomplete — regenerating over the hand-authored files would have DELETED 40 live nav translations per locale. Dropping --objects-only fails differently: kind:'full' folds all 803 metadata-form keys into <locale>.objects.generated.ts and renames the export the baseline imports. The split is correct as it stands, and is now written down in the config: objects/metadataForms are generated and gated by the bundle-drift check; apps/dashboards/pages are hand-authored and gated by the coverage ratchet. What was wrong was only that the baseline omitted the hand-authored half. - Extract config's `translations` carries the per-locale assemblers, with objects/metadataForms still pinned to the committed generated files. Safe for the emit — --objects-only writes data.objects alone, so nothing added here can reach a generated bundle; check:i18n stays in sync across all nine packages. - nav_app_builder translated in all four locales, wording harvested from the repo's own precedent for "builder" (构建器 / ビルダー / generador). - nav_workflows removed from all four: its entry is gone from STUDIO_APP and nothing contributes to that app, so the translation was dead. - Coverage ratchet baselined 231 -> 0 (repo total 996 -> 765), making platform-objects the ninth package where the ratchet is a strict gate. Verified red on a single removed translation, green on restore. - A local, CLI-independent parity test walks the statically declared Studio and Account navigation plus the dashboard's widgets and asserts a translation in every locale — and the reverse, that no translation outlives its nav item. Both directions verified to fail before they pass. An untranslated nav id is invisible in the UI: it falls back to the app's own English label, so a Chinese Studio menu shows one English entry among thirty. That is why this needs a gate and not a one-time sweep. Setup's ~25 runtime-contributed entries stay out of scope. A static gate for them needs either an objectql dependency in this package (it depends only on spec and metadata-core) or extractor support for navigationContributions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BQwnjqgCyAhSQjhNriW3BW
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 19 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…i-types-spec-53x3pt
…ate's new lint path Two CI failures on #3803, both real. 1. ESLint — #3789 landed `os validate`'s four authoring lints on main after this branch forked, and its `--json` branch was written with `console.log(JSON.stringify(…))`. The new rule caught it on the merge, which is exactly the case the rule exists for: the pattern coming back one command at a time, written by someone with no reason to suspect it. Converted to `await emitJson(…)`. 2. Test Core — the CONTROL case in emit-json-pipe.test.ts failed with "expected 300017 to be less than 300017". Whether a run truncates is a RACE between the writer's exit and the reader's drain: it truncated at 65536 locally and delivered the whole payload on a runner whose reader kept up. A gate that goes red by machine speed is no better than one that cannot go red, so this was my defect, not a flake to retry. Rewritten to remove the race rather than widen the tolerance: never read the pipe until the child has exited, so the kernel buffer caps at its capacity and everything still queued in userspace dies with process.exit. Payload raised to 4 MB, ~50x margin over even a 1 MiB pipe-max-size; the exact byte count is allowed to vary because only the shortfall is the claim. Measured 80640-117184 bytes delivered across runs, and stable over repeated runs. The technique is valid ONLY for the broken pattern, and the comment now says so — pointing it at `emitJson` hangs forever, because awaiting the write callback is what the fix does and with no reader the write never completes. Verified rather than assumed: still running after 8s. That deadlock is the fix working. Merges origin/main (c7f4417). Full check after the merge: eslint clean, cli tsc clean, cli 682 tests, platform-objects 239 tests, i18n bundle gate 9/9 in sync, coverage ratchet none-new. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BQwnjqgCyAhSQjhNriW3BW
os-zhuang
marked this pull request as ready for review
July 28, 2026 06:27
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two follow-ups to #3780, one per commit. Both turned out to be smaller in the work and larger in the finding than the issue text implied.
1 —
--jsontruncation: every command, and the second document it was hiding#3780 routed three commands through
emitJson. The other ~100 emission sites still wrote machine output withconsole.log, which on a pipe is cut off at one 64 KiB buffer when the command exits right after.It is invisible to whoever writes it: stdout to a TTY is synchronous, so every interactive run looks perfect while every scripted consumer — the only audience
--jsonhas — gets invalid JSON.The exit does not have to be explicit. oclif ends failing commands with
handle()→Exit.exit()→process.exit()and flushes nothing on that path (flush()runs only onexecute()'s success path), so a plainthis.exit(1)— or any thrown error — truncates identically. Measured, not assumed:process.exit(1)throwprocess.exitCode = 173 of the 104 sites had that shape. Even
lintwas only half fixed by #3780:--eval --jsonwrites a whole corpus report and was still onconsole.log.emitJson;formatOutput'sjson/yamlbranches through the same drain-awareemitText—--format yamltruncated too — which makes it async, so 32 call sites now await it.this.exit(1)stays; it is simply safe once the buffer has drained.{ compact: true }preserves whichever each had, so this is a truncation fix and not an output-format change. Unifying that split is worth doing — as its own decision.--no-inline-config, so there is no per-site opt-out) rejectsconsole.log(JSON.stringify(…))underpackages/cli/srcand un-awaitedformatOutput. Verified to fire on both — and it earned its place immediately: see CI below.The second defect underneath it
Draining the write made something visible that truncation had been hiding. Because
this.exit(1)throws, a command whose body sits in onetryunwinds its inner "report and stop" into the outercatch, which reports again — soos validate --jsonon a failing config printed two JSON documents, which is neither valid JSON nor valid JSONL. Nine commands had this shape; their catches now re-throw the exit signal (isExitSignal) rather than describing it as a failure.os validate --json, 900 schema errors, piped:Pinned end to end — a real command, a real pipe, a payload past several buffers — and the integration test was run against
origin/mainsources to confirm it goes red.Deliberately not fixed by forcing stdout into blocking mode process-wide, which would be one line and cover every command including future ones: the same binary runs
os serve/os dev, and a blocking write to a pipe with a slow reader blocks the event loop — trading truncated JSON for a server that stalls on its own logs.2 — #3762's remainder: the debt was 1 string, not 231
The open item read: platform-objects is 77 strings short per locale in
apps.*/dashboards.*, and its--objects-onlyextract cannot scaffold them — needs an emit decision (drop--objects-only, or a companion.apps.generated.ts) before any translating.Measured, the premise did not hold. Of the 77 declared keys per locale, 76 were already translated in the hand-authored
<locale>.tsfiles and had been for months. Exactly one was genuinely missing —apps.studio.navigation.nav_app_builder.label, absent in all four locales includingen.The 231 was a measurement artifact: the config declares
SETUP_APP/STUDIO_APP/ACCOUNT_APPandSystemOverviewDashboard, but itstranslationsmerge baseline listed only the two generated subtrees, so coverage counted every hand-authored app/dashboard key as untranslated.Neither proposed emit is right, and the second would have caused damage
The Setup app is a shell of empty group anchors; its ~25 menu entries are contributed at runtime by
SETUP_NAV_CONTRIBUTIONSand by capability plugins (ADR-0029 D7). A bundle generated from a static walk ofSETUP_APPis therefore structurally incomplete — regenerating over the hand-authored files would have deleted 40 live nav translations per locale. Dropping--objects-onlyfails differently:kind: 'full'folds all 803 metadata-form keys into<locale>.objects.generated.tsand renames the export the baseline imports.The split is correct as it stands, and is now written down in the config:
objects/metadataFormsare generated and gated by the bundle-drift check;apps/dashboards/pagesare hand-authored and gated by the coverage ratchet. What was wrong was only that the baseline omitted the hand-authored half.translationscarries the per-locale assemblers, withobjects/metadataFormsstill pinned to the committed generated files. Safe for the emit —--objects-onlywritesdata.objectsalone — andcheck:i18nstays in sync across all nine packages.nav_app_buildertranslated in all four locales, wording harvested from the repo's own precedent for "builder" (构建器/ビルダー/generador).nav_workflowsremoved from all four: its entry is gone fromSTUDIO_APPand nothing contributes to that app, so the translation was dead.An untranslated nav id is invisible in the UI: it falls back to the app's own English label, so a Chinese Studio menu shows one English entry among thirty. That is why this needs a gate and not a one-time sweep.
CI — two failures on the first push, both mine, both fixed
Recorded because the second one is a lesson about the gate, not a hiccup.
ESLint. #3789 landed
os validate's four authoring lints onmainafter this branch forked, and its--jsonbranch was written withconsole.log(JSON.stringify(…)). The new rule caught it on the merge — precisely the case the rule exists for, the pattern coming back one command at a time. Converted.Test Core — my own control test. It failed with
expected 300017 to be less than 300017: on the runner, theconsole.logpattern delivered the whole payload. Whether a run truncates is a race between the writer's exit and the reader's drain, and the first version used a 300 KB payload with a continuously-readingexecFileP— it truncated at 65536 locally and did not on a runner whose reader kept up. A gate that goes red by machine speed is no better than one that cannot go red, so this was a defect in my test, not a flake to retry.Rewritten to remove the race rather than widen the tolerance: never read the pipe until the child has exited, so the kernel buffer caps at its capacity and everything still queued in userspace dies with
process.exit. Payload raised to 4 MB — ~50× margin over even a 1 MiBpipe-max-size— and only the shortfall is asserted, since the exact byte count legitimately varies (measured 80 640–117 184 delivered).That technique is valid only for the broken pattern, and the comment now says so: pointing it at
emitJsonhangs forever, because awaiting the write callback is what the fix does and with no reader the write never completes. Verified rather than assumed — still running after 8 s. That deadlock is the fix working.Verification
Local, after merging
main(c7f4417):packages/clivitestpackages/platform-objectsvitesttsc --noEmit(both packages)eslint . --no-inline-configcheck-i18n-bundlescheck-i18n-coverageEvery new gate was checked in both directions — red before the fix, green after — rather than only confirming the green.
Not in scope
console.logpaths in the CLI, which are unaffected, andos serve/os devlogging.specandmetadata-core) or extractor support fornavigationContributions— a real follow-up, not something to half-do here.--jsonsplit, kept out so this stays a pure truncation fix.