Skip to content

Commit 4921a95

Browse files
os-zhuangclaude
andauthored
fix(cli,i18n): finish the --json truncation sweep; close #3762 where the debt was 1 string, not 231 (#3803)
Two follow-ups to #3780. Both were smaller in the work and larger in the finding than the issue text implied. ## 1 — `--json` truncation: every command, and the second document it hid #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. 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. Measured on a 300 009-byte payload: `process.exit(1)` 65 536, `throw` 65 536, natural return 300 009, `process.exitCode = 1` 300 009. 73 of the 104 sites had that shape, and `lint` was itself 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), making it async — 32 call sites now await it. - Control flow untouched: a following `this.exit(1)` stays, simply safe once the buffer has drained. - Output bytes unchanged. Roughly half the sites emitted compact and half indented; `{ compact: true }` preserves whichever each had, keeping this a truncation fix rather than an output-format change. - An ESLint rule (root script runs `--no-inline-config`, so no per-site opt-out) rejects `console.log(JSON.stringify(…))` under packages/cli/src and un-awaited `formatOutput`. It earned its place immediately: #3789 landed `os validate`'s authoring lints with the old pattern and the rule caught it on the merge. Draining the write exposed a second defect underneath. 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` 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`). `os validate --json`, 900 schema errors, piped: 131 072 bytes unparseable before; 1 514 711 in two documents with the truncation fix alone; 1 514 648 as one parseable document with both. Deliberately NOT fixed by forcing stdout into blocking mode process-wide, which would be one line and cover everything: 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 "77 strings short per locale in apps.*/dashboards.*, needs an emit decision (drop --objects-only, or a companion .apps.generated.ts)". 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 — the config declares SETUP_APP / STUDIO_APP / ACCOUNT_APP and SystemOverviewDashboard, but its `translations` merge baseline listed only the two GENERATED subtrees. Neither proposed emit is right, and the second would have caused damage. The Setup app is a shell of empty group anchors; its ~25 entries are contributed at RUNTIME by SETUP_NAV_CONTRIBUTIONS and by capability plugins (ADR-0029 D7), so a bundle generated from a static walk 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 the objects bundle and renames the export the baseline imports. The split is correct and is now written down: objects/metadataForms are generated and gated by the bundle-drift check; apps/dashboards/pages are hand-authored and gated by the coverage ratchet. Only the baseline was wrong. - Extract config's `translations` carries the per-locale assemblers, with objects/metadataForms still pinned to the committed generated files. - 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. - Ratchet baselined 231 -> 0 (repo total 996 -> 765), making platform-objects the ninth package where the ratchet is a strict gate. - A 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 — no translation outlives its nav item. 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. ## On the gates themselves Every new gate was checked in BOTH directions — red before the fix, green after. That discipline caught a defect in my own test: the truncation control case initially raced the reader's drain, passing locally at 65 536 bytes and failing 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 it was rewritten to remove the race — never read the pipe until the child exits, 4 MB payload, assert only the shortfall. The technique is valid only for the broken pattern: pointed at `emitJson` it hangs forever, because awaiting the write callback is what the fix does. Verified rather than assumed. Closes #3762. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent c7f4417 commit 4921a95

47 files changed

Lines changed: 904 additions & 245 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
fix(cli): finish the `--json` truncation fix — every command, and the second document it was hiding (#3780 follow-up)
6+
7+
#3780 routed three commands through `emitJson`. The other ~100 emission sites
8+
still wrote machine output with `console.log`, which on a **pipe** is cut off
9+
at one 64 KiB buffer when the command exits right after: Node buffers pipe
10+
writes asynchronously and the exit tears the process down mid-drain. It is
11+
invisible to whoever writes it — stdout to a TTY is synchronous, so every
12+
interactive run looks perfect while every scripted consumer, the only audience
13+
`--json` has, gets invalid JSON.
14+
15+
The exit does not have to be an explicit `process.exit`. oclif ends failing
16+
commands with `handle()``Exit.exit()``process.exit()` and flushes
17+
nothing on that path (`flush()` runs only on `execute()`'s success path), so a
18+
plain `this.exit(1)` — or any thrown error — truncates identically. 73 of the
19+
104 sites had exactly that shape. Even `lint` was only half fixed: `--eval
20+
--json` writes a whole corpus report and was still on `console.log`.
21+
22+
All 104 sites now go through `emitJson`, `formatOutput`'s `json`/`yaml`
23+
branches through the same drain-aware write (`--format yaml` truncated too),
24+
and an ESLint rule keeps the pattern from growing back one command at a time.
25+
Control flow is untouched — a following `this.exit(1)` stays, it is simply
26+
safe once the buffer has drained. Output bytes are unchanged: roughly half the
27+
sites emitted compact and half indented, and each keeps whichever it had.
28+
29+
**Draining the write exposed a second defect underneath it.** Because
30+
`this.exit(1)` *throws*, a command whose body sits in one `try` unwinds its
31+
inner "report and stop" into the outer `catch`, which reports again — so
32+
`os validate --json` on a failing config printed **two** JSON documents, which
33+
is neither valid JSON nor valid JSONL. Truncation had been hiding the second
34+
one. Nine commands had this shape; their catch clauses now re-throw the exit
35+
signal (`isExitSignal`) instead of describing it as a failure.
36+
37+
Measured on `os validate --json` against a config with 900 schema errors,
38+
piped:
39+
40+
| | bytes | parses |
41+
|---|---:|---|
42+
| before | 131072 (exactly two buffers, cut mid-string) | no |
43+
| truncation fix alone | 1514711 (two documents) | no |
44+
| both fixes | 1514648 (one document, 900 errors) | **yes** |
45+
46+
Pinned end to end: a real command, a real pipe, a payload past several
47+
buffers — plus a control case asserting the `console.log` pattern it replaced
48+
genuinely truncates, so the gate cannot quietly stop testing anything.
49+
50+
Not covered: the ~30 human-facing `console.log` paths, which are unaffected,
51+
and `os serve` / `os dev` logging. This is deliberately not fixed by forcing
52+
stdout into blocking mode process-wide, which would be one line and cover
53+
everything — the same binary runs the dev server, and a blocking write to a
54+
pipe with a slow reader blocks the event loop, trading truncated JSON for a
55+
server that stalls on its own logs.
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
---
2+
"@objectstack/platform-objects": patch
3+
---
4+
5+
fix(i18n): platform-objects' 231 untranslated strings were 1 — close the real gap and stop the phantom (#3762)
6+
7+
Closes the rest of #3762. The remaining item was recorded as "platform-objects
8+
is 77 strings short per locale, in `apps.*` / `dashboards.*`, and its
9+
`--objects-only` extract cannot scaffold them — needs an emit decision (drop
10+
`--objects-only`, or a companion `.apps.generated.ts`) before any translating."
11+
12+
Measured, the premise did not hold. Of the 77 declared keys per locale, **76
13+
were already translated** in the hand-authored `<locale>.ts` files and had been
14+
for months. Exactly one was genuinely missing —
15+
`apps.studio.navigation.nav_app_builder.label`, absent in all four locales
16+
including `en`. The 231 was a measurement artifact: this config declares
17+
SETUP_APP / STUDIO_APP / ACCOUNT_APP and SystemOverviewDashboard, but its
18+
`translations` merge baseline listed only the two GENERATED subtrees
19+
(`objects`, `metadataForms`), so coverage counted every hand-authored
20+
app/dashboard key as untranslated.
21+
22+
**Neither proposed emit is right, and the second would have caused damage.**
23+
The Setup app is a shell of empty group anchors; its ~25 menu entries are
24+
contributed at runtime by `SETUP_NAV_CONTRIBUTIONS` and by capability plugins
25+
(ADR-0029 D7). A bundle generated from a static walk of `SETUP_APP` is
26+
therefore structurally incomplete, and regenerating over the hand-authored
27+
files would have **deleted 40 live nav translations per locale**. Dropping
28+
`--objects-only` fails differently: `kind: 'full'` folds all 803 metadata-form
29+
keys into `<locale>.objects.generated.ts` and renames the export the baseline
30+
imports.
31+
32+
The split is correct as it stands and is now written down: `objects` /
33+
`metadataForms` are generated and gated by the bundle-drift check; `apps` /
34+
`dashboards` / `pages` are hand-authored and gated by the coverage ratchet.
35+
What was wrong was only that the baseline omitted the hand-authored half.
36+
37+
- Extract config's `translations` now carries the per-locale assemblers, with
38+
`objects`/`metadataForms` still pinned to the committed generated files.
39+
Safe for the emit — `--objects-only` writes `data.objects` alone, so nothing
40+
added here can reach a generated bundle, and `check:i18n` stays in sync
41+
across all nine packages.
42+
- `nav_app_builder` translated in all four locales, wording taken from the
43+
repo's own precedent for "builder" (`构建器` / `ビルダー` / `generador`).
44+
- `nav_workflows` removed from all four: its menu entry is gone from
45+
`STUDIO_APP` and nothing contributes to that app, so the translation was
46+
dead.
47+
- Coverage ratchet baselined 231 → **0**, making platform-objects the ninth
48+
package where the ratchet is a strict gate — verified to go red on a single
49+
removed translation.
50+
- A local, CLI-independent parity test walks the statically declared Studio and
51+
Account navigation plus the dashboard's widgets and asserts a translation in
52+
every locale — and the reverse, that no translation survives its nav item.
53+
Both directions verified to fail before passing.
54+
55+
An untranslated nav id is invisible in the UI — it falls back to the app's
56+
English label, so a Chinese Studio menu just shows one English entry among
57+
thirty. That is why this needed a gate rather than a one-time sweep.
58+
59+
Still out of scope: the ~25 Setup entries contributed at runtime. Bringing them
60+
under a static gate needs either an objectql dependency in this package (it
61+
depends only on spec and metadata-core) or extractor support for
62+
`navigationContributions` — a real follow-up, not something to half-do here.

eslint.config.mjs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,56 @@ export default [
7878
}],
7979
},
8080
},
81+
// Machine output must not be written with `console.log`.
82+
//
83+
// `console.log(big)` followed by an exit hands a PIPE reader a payload cut
84+
// off at one 64 KiB buffer: Node writes stdout asynchronously to a pipe and
85+
// the exit tears the process down mid-drain. `os lint … --json` shipped that
86+
// for months at exactly 65536 bytes, and it is invisible to whoever writes
87+
// it — stdout to a TTY is synchronous, so every interactive run looks right
88+
// while every scripted consumer, the only audience `--json` has, gets
89+
// invalid JSON. The exit need not be explicit: oclif ends failing commands
90+
// with `handle()` → `Exit.exit()` → `process.exit()` and flushes nothing on
91+
// that path, so a plain `this.exit(1)` truncates the same way.
92+
//
93+
// `emitJson` / `emitText` (packages/cli/src/utils/format.ts) await the write
94+
// callback first. The whole CLI was swept onto them; this keeps the pattern
95+
// from growing back one command at a time. Note the root lint script runs
96+
// with `--no-inline-config`, so there is no per-site opt-out — which is the
97+
// point: every past instance of this was written by someone who had no
98+
// reason to suspect it.
99+
{
100+
files: ['packages/cli/src/**/*.{ts,tsx,mts,cts}'],
101+
ignores: ['**/node_modules/**', '**/dist/**', '**/*.test.ts'],
102+
languageOptions: {
103+
parser: tsParser,
104+
parserOptions: { ecmaVersion: 'latest', sourceType: 'module' },
105+
},
106+
rules: {
107+
'no-restricted-syntax': ['error',
108+
{
109+
selector:
110+
"CallExpression[callee.object.name='console'][callee.property.name='log']" +
111+
" > CallExpression[callee.object.name='JSON'][callee.property.name='stringify']",
112+
message:
113+
'Write machine output with `await emitJson(payload)` from utils/format.js, not ' +
114+
'console.log(JSON.stringify(…)). On a pipe, console.log followed by an exit ' +
115+
'(including oclif\'s this.exit / any thrown error) truncates the payload at 64 KiB. ' +
116+
'Pass `{ compact: true }` as the third argument to keep single-line output.',
117+
},
118+
{
119+
// `formatOutput` became async for the same reason — its json and yaml
120+
// branches go through emitText. An un-awaited call at statement
121+
// position silently reopens the hole. (An awaited one nests under an
122+
// AwaitExpression and does not match.)
123+
selector: "ExpressionStatement > CallExpression[callee.name='formatOutput']",
124+
message:
125+
'`formatOutput` is async — await it. Its json/yaml branches drain stdout before ' +
126+
'the command can exit; dropping the await reintroduces the 64 KiB pipe truncation.',
127+
},
128+
],
129+
},
130+
},
81131
// issue #2035 — authoring-entry guard. Flags exported consts in metadata
82132
// files that are annotated with a spec domain type (simple `Page` or qualified
83133
// `UI.Page`) instead of being wrapped in the `defineX` factory. AST-only (no

packages/cli/src/commands/cloud/login.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import * as readline from 'node:readline/promises';
1414
import { stdin as input, stdout as output } from 'node:process';
1515
import { Command, Flags } from '@oclif/core';
16-
import { printHeader, printKV, printSuccess, printError } from '../../utils/format.js';
16+
import { printHeader, printKV, printSuccess, printError, emitJson } from '../../utils/format.js';
1717
import { loginWithBrowser, loginWithPassword } from '../../utils/auth-flows.js';
1818
import { DEFAULT_CLOUD_URL, readCloudConfig, writeCloudConfig } from '../../utils/cloud-config.js';
1919

@@ -105,9 +105,7 @@ export default class CloudLogin extends Command {
105105
const existing = await readCloudConfig();
106106
if (existing?.token) {
107107
if (flags.json) {
108-
console.log(
109-
JSON.stringify({ success: false, error: 'Already logged in', email: existing.email, url: existing.url }),
110-
);
108+
await emitJson({ success: false, error: 'Already logged in', email: existing.email, url: existing.url }, 0, { compact: true });
111109
} else {
112110
printSuccess(`Already logged in to ${existing.url} as ${existing.email || existing.userId}`);
113111
console.log('');
@@ -145,7 +143,7 @@ export default class CloudLogin extends Command {
145143
});
146144

147145
if (flags.json) {
148-
console.log(JSON.stringify({ success: true, email: result.user?.email, userId: result.user?.id, url }, null, 2));
146+
await emitJson({ success: true, email: result.user?.email, userId: result.user?.id, url });
149147
} else {
150148
printSuccess('Cloud authentication successful');
151149
if (result.user?.email) printKV('Email', result.user.email);
@@ -157,7 +155,7 @@ export default class CloudLogin extends Command {
157155
}
158156
} catch (error: any) {
159157
if (flags.json) {
160-
console.log(JSON.stringify({ success: false, error: error.message }, null, 2));
158+
await emitJson({ success: false, error: error.message });
161159
this.exit(1);
162160
}
163161
printError(error.message || String(error));

packages/cli/src/commands/cloud/logout.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import { Command, Flags } from '@oclif/core';
1010
import { ObjectStackClient } from '@objectstack/client';
11-
import { printHeader, printSuccess, printError } from '../../utils/format.js';
11+
import { printHeader, printSuccess, printError, emitJson } from '../../utils/format.js';
1212
import { deleteCloudConfig, tryReadCloudConfig } from '../../utils/cloud-config.js';
1313

1414
export default class CloudLogout extends Command {
@@ -39,14 +39,14 @@ export default class CloudLogout extends Command {
3939
await deleteCloudConfig();
4040

4141
if (flags.json) {
42-
console.log(JSON.stringify({ success: true, message: 'Cloud credentials cleared' }, null, 2));
42+
await emitJson({ success: true, message: 'Cloud credentials cleared' });
4343
} else {
4444
printSuccess('Cloud credentials cleared');
4545
console.log('');
4646
}
4747
} catch (error: any) {
4848
if (flags.json) {
49-
console.log(JSON.stringify({ success: false, error: error.message }, null, 2));
49+
await emitJson({ success: false, error: error.message });
5050
this.exit(1);
5151
}
5252
printError(error.message || String(error));

packages/cli/src/commands/cloud/whoami.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import { Command, Flags } from '@oclif/core';
10-
import { printHeader, printKV, printSuccess, printError } from '../../utils/format.js';
10+
import { printHeader, printKV, printSuccess, printError, emitJson, isExitSignal } from '../../utils/format.js';
1111
import { tryReadCloudConfig } from '../../utils/cloud-config.js';
1212

1313
export default class CloudWhoami extends Command {
@@ -26,7 +26,7 @@ export default class CloudWhoami extends Command {
2626
const config = await tryReadCloudConfig();
2727
if (!config?.token) {
2828
if (flags.json) {
29-
console.log(JSON.stringify({ logged_in: false }, null, 2));
29+
await emitJson({ logged_in: false });
3030
} else {
3131
printError('Not logged in to ObjectStack Cloud. Run `os cloud login` first.');
3232
}
@@ -54,7 +54,7 @@ export default class CloudWhoami extends Command {
5454
const valid = !!sessionUser;
5555

5656
if (flags.json) {
57-
console.log(JSON.stringify({ logged_in: true, valid, url: config.url, email, userId }, null, 2));
57+
await emitJson({ logged_in: true, valid, url: config.url, email, userId });
5858
} else {
5959
printHeader('ObjectStack Cloud Identity');
6060
printKV('Server', config.url);
@@ -70,8 +70,9 @@ export default class CloudWhoami extends Command {
7070
console.log('');
7171
}
7272
} catch (error: any) {
73+
if (isExitSignal(error)) throw error;
7374
if (flags.json) {
74-
console.log(JSON.stringify({ success: false, error: error.message }, null, 2));
75+
await emitJson({ success: false, error: error.message });
7576
this.exit(1);
7677
}
7778
printError(error.message || String(error));

0 commit comments

Comments
 (0)