diff --git a/.changeset/i18n-extract-check-json-compares.md b/.changeset/i18n-extract-check-json-compares.md new file mode 100644 index 0000000000..880ec0c6ea --- /dev/null +++ b/.changeset/i18n-extract-check-json-compares.md @@ -0,0 +1,20 @@ +--- +"@objectstack/cli": patch +--- + +`os i18n extract --check --json` now COMPARES. It used to exit 0 having compared nothing, on a tree whose bundles had provably drifted. + +The machine face returned before the comparison ran: `if (flags.json) { … return; }` sat ahead of both the `--check` needs-`--out` guard and the comparison block. Driven on one fixture, two invocations differing only by `--json` — the first exited 1 with `missing: OUT/zh-CN.objects.generated.ts` and `Translation bundles have drifted from the schema`, the second exited 0 with the ordinary extract payload. The first run is the second one's positive control: the drift was really there. Same shape as the `--dry-run` branch repaired one release earlier, and `--json` is if anything the more likely CI spelling of the two, because a pipeline that wants to parse the result reaches for it. + +⚠️ **A pipeline that runs `os i18n extract … --check --json` and was green may now go red, and that is this repair working.** The green was a comparison that never happened; the red is the drift that was already in the tree. The fix is the one the failure names — re-run the same command without `--check` **and without `--json`**, then commit what it writes. Neither of those two flags writes files, and the command the failure prints now has both taken out of it. + +What each invocation now does, with no new member on any published payload: + +- **drift found** — the run ends on this command's existing `{ "error": … }` envelope with exit 1, carrying the same sentence the console face prints, the regenerate-and-commit command included. Deliberately not a new `drift` / `missing` / `stale` payload member: every other way this command can fail already speaks that envelope, and naming the drifted files in the machine payload would widen a published output face. +- **in sync** — unchanged: the ordinary extract payload, exit 0. +- **`--check` with no `--out`** — the refusal is now reachable under `--json` too, in the same `{ "error": … }` envelope with exit 1. It used to exit 0 with a payload, having been asked for a comparison it could not make. +- **`--json` without `--check`** — unchanged in every respect. + +The run leaves through exactly one of those faces, so stdout still parses as exactly one JSON document. + +One more thing moved with it: the command a drifted `--check` prints as its remedy now has `--json` taken out of it as well as `--check`. It used to keep `--json`, so the machine face named a command that emits a payload, writes zero files, and leaves the next run failing with the same advice. diff --git a/packages/cli/src/commands/i18n/extract.ts b/packages/cli/src/commands/i18n/extract.ts index bf9f8909e0..997160d6b2 100644 --- a/packages/cli/src/commands/i18n/extract.ts +++ b/packages/cli/src/commands/i18n/extract.ts @@ -31,6 +31,15 @@ import { const FILL_STRATEGIES: FillStrategy[] = ['empty', 'default', 'todo']; +/** + * The refusal `--check` without `--out` ends on — one string, because two faces + * now reach it. The console run throws it below the skeleton summary; a + * `--json` run throws it from the machine face, where it lands in this + * command's ordinary `{ error }` envelope (#16600). + */ +const CHECK_NEEDS_OUT = + '--check needs --out= — it compares a fresh extract against the bundles committed there.'; + /** * A path for one of this command's output lines: relative to the cwd while that * is still a NAME for the file, absolute once it stops being one. @@ -95,40 +104,60 @@ function shellToken(token: string): string { * An assembled command is wrong in exactly one way and it is unbounded — every * flag that exists now, and every flag added later, has to be remembered at * this print site or it silently goes missing. So this does not enumerate - * flags at all. It takes the argv oclif was handed and removes one token from - * it, which makes the echo correct for flags this file has never heard of. + * flags at all. It takes the argv oclif was handed and removes the tokens that + * make a run WRITE NOTHING, which keeps the echo correct for flags this file + * has never heard of. + * + * ## Which tokens, and why it is not just `--check` (#16600) + * + * There are exactly two, and both are "write nothing" spellings: + * + * - `--check` — the mode being escaped. Removing it is the whole point. + * - `--json` — "output JSON instead of writing files", so a run carrying it + * regenerates nothing either. It became reachable here the moment the + * machine face started reporting drift, and until it was dropped this + * function named a command that emits a payload, writes zero files, and + * leaves the next `--check --json` failing with the same advice: the + * #14895 loop above, reproduced one face over. A remedy that cannot heal + * the failure it is printed under is worse than none, because it looks + * like one. * - * ⛔ It also never GUESSES. If `--check` is not in the argv the flag was not - * spelled there, this function cannot point at what it removed, and the caller - * prints "re-run the same command without `--check`" instead — the degraded - * line the report itself asked for, on the grounds that a correct vague - * sentence beats a complete-looking wrong command. Today's flag surface has no - * other way to set `--check` (no `env`, no default, no `allowNo`), so that is - * defence rather than a path a user can reach; it is what keeps "assemble an + * ⛔ It never GUESSES. If `--check` is not in the argv the flag was not spelled + * there, this function cannot point at what it removed, and the caller prints a + * degraded sentence instead — on the grounds that a correct vague sentence + * beats a complete-looking wrong command. `--json`'s absence is NOT such a + * signal: it is dropped when present and its absence means only that the run + * was on the console face. Today's flag surface has no other way to set + * `--check` (no `env`, no default, no `allowNo`), so the guard is defence + * rather than a path a user can reach; it is what keeps "assemble an * approximation" from ever becoming the fallback. * * `--` is honoured because it changes what a token MEANS: after it, `--check` * is a positional argument and removing it would rewrite the invocation rather - * than trim it. + * than trim it. The same holds for `--json`. * * @param bin `config.bin` — `os`, the name the command is installed under * @param id `this.id` — `i18n:extract`, oclif's colon spelling of the path * @param argv `this.argv` — the arguments as typed, the command id stripped * @returns the command to print, or `undefined` when it cannot be built */ -function rerunWithoutCheck(bin: string, id: string | undefined, argv: readonly string[]): string | undefined { +function rerunThatRegenerates(bin: string, id: string | undefined, argv: readonly string[]): string | undefined { const kept: string[] = []; - let dropped = 0; + let droppedCheck = 0; let afterTerminator = false; for (const token of argv) { if (!afterTerminator && token === '--') afterTerminator = true; else if (!afterTerminator && (token === '--check' || token.startsWith('--check='))) { - dropped += 1; + droppedCheck += 1; + continue; + } else if (!afterTerminator && (token === '--json' || token.startsWith('--json='))) { + // Dropped without being counted: only `--check`'s absence means "this + // function cannot say what it removed". continue; } kept.push(token); } - if (dropped === 0) return undefined; + if (droppedCheck === 0) return undefined; return [bin, ...(id ?? 'i18n:extract').split(':'), ...kept.map(shellToken)].join(' '); } @@ -431,7 +460,135 @@ export default class I18nExtract extends Command { return narrowToCommittedSections(table, committed); }; + /** + * Every file a normal run would write into `dir`, paired with its + * rendered content — the ONE list every face that names this run's files + * reads: the write loop, the console `--check`, and the `--json` + * `--check` below. So no two of them can disagree about what this run + * produces, and in particular `--check` can never compare something the + * write path would not have written. + * + * It was a straight-line `const emitted` built after the `--dry-run` + * branch, which is below the machine face and therefore out of its reach. + * A `--json --check` run needs the same list, so the list moved rather + * than being rebuilt beside it (#16600). + */ + const emittedFiles = (dir: string): Array<{ file: string; content: string; keys: number }> => { + const files: Array<{ file: string; content: string; keys: number }> = []; + for (const locale of localesEmitted) { + for (const mod of emittedModules(locale)) { + files.push({ + file: path.join(dir, `${locale}.${mod.suffix}`), + content: renderTranslationModule(result.bundles[locale], { locale, kind: mod.kind }), + keys: mod.keys, + }); + } + // The provenance companion rides in the SAME list, so `--check` compares + // it by the same byte-for-byte rule as the bundles it belongs to and can + // never diverge from what a real extract writes. + const table = committedSourceHashes(locale); + if (flags['source-hashes'] && table) { + files.push({ + file: path.join(dir, `${locale}.source-hashes.generated.ts`), + content: renderSourceHashModule(table, { locale }), + keys: Object.keys(table).length, + }); + } + } + return files; + }; + + /** What `--check` found: committed files that are absent, and ones whose bytes differ. */ + const compareCommitted = ( + files: ReadonlyArray<{ file: string; content: string }>, + ): { missing: string[]; stale: string[] } => { + const missing: string[] = []; + const stale: string[] = []; + for (const { file, content } of files) { + const shown = displayPath(file); + if (!fs.existsSync(file)) missing.push(shown); + else if (fs.readFileSync(file, 'utf8') !== content) stale.push(shown); + } + return { missing, stale }; + }; + + /** + * The sentence a drifted `--check` ends on, built once so both faces end + * on the same words. {@link rerunThatRegenerates} says which tokens the + * command it names has had deleted and why it is spelled as a deletion. + * + * ⭐ The degraded line names the SAME tokens the built command would have + * removed, so the two spellings of this advice cannot prescribe different + * things: under `--json` a run without `--check` still writes nothing, and + * a fallback that said only "without `--check`" would send an operator + * round the #14895 loop exactly as a built command carrying `--json` did. + */ + const driftMessage = (): string => { + const rerun = rerunThatRegenerates(this.config.bin, this.id, this.argv); + const degraded = flags.json + ? ' re-run the same command without `--check` and without `--json` — neither of them writes files' + : ' re-run the same command without `--check`'; + return ( + 'Translation bundles have drifted from the schema. Regenerate and commit:\n' + + (rerun ? ` ${rerun}` : degraded) + ); + }; + if (flags.json) { + /** + * ⭐ `--check` is a VERDICT mode, so under `--json` the comparison runs + * HERE — before the one document this run is allowed to write (#16600). + * + * ## What was wrong + * + * This branch emitted and returned unconditionally, which put it ahead + * of both the `--check` needs-`--out` guard and the comparison itself. + * Driven on one drifted fixture, the two invocations differing ONLY by + * `--json`: + * + * $ os i18n extract CONFIG --locales=zh-CN --no-metadata-forms + * --out=OUT --check + * missing: OUT/zh-CN.objects.generated.ts + * Translation bundles have drifted from the schema. … + * -> exit 1 + * + * $ … --out=OUT --check --json + * {"totalExpected":…,"counts":…,"bundles":…} + * -> exit 0, nothing compared + * + * The first run is the second one's positive control: the drift is + * provably there and the second reported success. Same shape as the + * `--dry-run` branch in #16480, and `--json` is if anything the more + * likely CI spelling of the two — a pipeline that wants to parse the + * result reaches for it. A check that cannot fail is indistinguishable + * from a check that finds nothing. + * + * ## Why the failure is this command's `{ error }` envelope and NOT a + * new payload member + * + * ⛔ The drift report is deliberately NOT widened into the published + * payload — no `drift` / `missing` / `stale` member is added here. This + * command already has exactly one machine-readable failure envelope — + * the `catch` at the end of this method: `{ error, …errorCodeFields }`, + * compact, exit 1. Every other way this command can fail already speaks + * it, the `--check` needs-`--out` refusal above included, so routing + * drift through the same `throw` is copying the convention rather than + * settling a second one for the same mode. Which files drifted is a + * genuine addition to a published output face and is its own card. + * + * ⚠️ And it must stay ONE document: emitting the payload here and an + * error envelope afterwards is the two-JSON-documents defect + * {@link isExitSignal} records — unparseable as either one document or + * as JSONL. So the verdict is reached before anything is written, and + * the run leaves through exactly one of the two faces. + * + * ⛔ Returning 0 without comparing must not come back. + */ + if (flags.check) { + if (!flags.out) throw new Error(CHECK_NEEDS_OUT); + const { missing, stale } = compareCommitted(emittedFiles(outDir as string)); + if (missing.length > 0 || stale.length > 0) throw new Error(driftMessage()); + } await emitJson({ totalExpected: result.totalExpected, // Leaves of the `bundles` payload below, locale by locale, so this @@ -526,7 +683,7 @@ export default class I18nExtract extends Command { console.log(''); if (flags.check && !flags.out) { - throw new Error('--check needs --out= — it compares a fresh extract against the bundles committed there.'); + throw new Error(CHECK_NEEDS_OUT); } /** @@ -579,39 +736,12 @@ export default class I18nExtract extends Command { // under `--check`, and `--check` without `--out` already threw. const resolvedOutDir = outDir as string; - // Every file a normal run would emit, paired with its rendered content. - // Both branches below iterate this, so `--check` can never diverge from - // what a real extract writes. - const emitted: Array<{ file: string; content: string; keys: number }> = []; - for (const locale of localesEmitted) { - for (const mod of emittedModules(locale)) { - emitted.push({ - file: path.join(resolvedOutDir, `${locale}.${mod.suffix}`), - content: renderTranslationModule(result.bundles[locale], { locale, kind: mod.kind }), - keys: mod.keys, - }); - } - // The provenance companion rides in the SAME list, so `--check` compares - // it by the same byte-for-byte rule as the bundles it belongs to and can - // never diverge from what a real extract writes. - const table = committedSourceHashes(locale); - if (flags['source-hashes'] && table) { - emitted.push({ - file: path.join(resolvedOutDir, `${locale}.source-hashes.generated.ts`), - content: renderSourceHashModule(table, { locale }), - keys: Object.keys(table).length, - }); - } - } + // Every file a normal run would emit, paired with its rendered content — + // {@link emittedFiles}, the same list the machine face compares. + const emitted = emittedFiles(resolvedOutDir); if (flags.check) { - const stale: string[] = []; - const missing: string[] = []; - for (const { file, content } of emitted) { - const shown = displayPath(file); - if (!fs.existsSync(file)) missing.push(shown); - else if (fs.readFileSync(file, 'utf8') !== content) stale.push(shown); - } + const { missing, stale } = compareCommitted(emitted); if (missing.length === 0 && stale.length === 0) { console.log(''); printSuccess(`${emitted.length} bundle(s) are in sync with the schema ${chalk.dim(`(${timer.display()})`)}`); @@ -621,15 +751,11 @@ export default class I18nExtract extends Command { for (const shown of stale) printError(`out of date: ${shown}`); console.log(''); // The command that regenerates these bytes is THIS run without - // `--check` — the two branches share the `emitted` list above, so the + // `--check` — the two faces share the `emittedFiles` list above, so the // write path cannot produce anything other than what was just - // compared. {@link rerunWithoutCheck} says why it is spelled as a - // deletion and what the degraded line is for. - const rerun = rerunWithoutCheck(this.config.bin, this.id, this.argv); - printError( - 'Translation bundles have drifted from the schema. Regenerate and commit:\n' + - (rerun ? ` ${rerun}` : ' re-run the same command without `--check`'), - ); + // compared. {@link driftMessage} is the sentence, built once so the + // `--json` face ends on the same words. + printError(driftMessage()); process.exit(1); } diff --git a/packages/cli/test/i18n-extract-check-json.test.ts b/packages/cli/test/i18n-extract-check-json.test.ts new file mode 100644 index 0000000000..57a2fc965b --- /dev/null +++ b/packages/cli/test/i18n-extract-check-json.test.ts @@ -0,0 +1,392 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os i18n extract --check --json` COMPARES, and reaches the same verdict as + * the same run without `--json` (#16600). + * + * ## What was wrong + * + * The machine face returned before anything was compared. Driven on one + * drifted fixture, the two invocations differing ONLY by `--json`: + * + * $ os i18n extract CONFIG --locales=zh-CN --no-metadata-forms --out=OUT --check + * missing: OUT/zh-CN.objects.generated.ts + * Translation bundles have drifted from the schema. Regenerate and commit: + * -> exit 1 + * + * $ os i18n extract CONFIG --locales=zh-CN --no-metadata-forms --out=OUT --check --json + * {"totalExpected":775,"counts":{"zh-CN":2},"bundles":{…}} + * -> exit 0, with no drift field, no comparison and no failure + * + * The first run is the second one's positive control: the drift is provably + * there, and the second reported success. `if (flags.json) { … return; }` sat + * ahead of both the `--check` needs-`--out` guard and the comparison block — + * the same shape the `--dry-run` branch had in #16480, and `--json` is if + * anything the more likely CI spelling of the two, because a pipeline that + * wants to parse the result reaches for it. A check that cannot fail is + * indistinguishable from a check that finds nothing. + * + * ## Why these shapes + * + * ⚠️ A case asserting only the new exit code would be satisfied by a `--check` + * that still compares nothing and merely fails, so every drift case here pins + * WHAT WAS REPORTED beside the code — and the report is read out of the JSON + * document, not out of the console text, because the document is the face this + * card is about. + * + * - the drifted cases are stated as an EQUALITY against the same invocation + * WITHOUT `--json`, which is the card's own method rather than a + * re-derivation of it, and the expected values are spelled out as well, + * because an equality alone is also satisfied by two runs that are both + * broken; + * - the in-sync case is what no unconditional failure can pass. ⚠️ It is NOT + * a falsifier for this defect and must not be read as one: before the + * repair, `--check --json` on an in-sync tree ALSO exited 0 carrying the + * payload, so this case is green on both sides of the mutation. That is + * the #16480 lesson in this card's own terms — an exit-code-only suite + * would have stayed green over the very regression it was written for — + * and it is why the drift cases assert the reported drift; + * - `--json` PURITY is asserted on every machine run: stdout has to parse as + * exactly ONE document. A repair that emitted the ordinary payload and + * then an error envelope would pass an "exit 1 and the word drifted" + * reading while producing output that is neither one document nor JSONL — + * the two-document defect `isExitSignal` records in `utils/format.ts`; + * - the REMEDY is pinned as well as the sentence, on its own two booleans. + * The drift envelope is two lines and the second one is a command; an + * earlier revision of this file read only the first, and a remedy that + * named a `--json` run — which emits a payload and writes zero files — + * sat green under it. Running exactly what the failure prints then heals + * nothing and the next `--check --json` fails identically: the #14895 + * loop, one face over. So the remedy must name an `--out` and must not + * carry `--json`; + * - the needs-`--out` refusal is pinned because it is the OTHER thing the + * early return skipped: `--check --json` with no `--out` used to exit 0 + * with a payload, having been asked for a comparison it could not make; + * - a plain `--json` run with no `--check` is pinned unchanged, so the + * repair cannot be satisfied by turning the machine face into a checker. + * + * ⛔ Nothing here asserts a `drift` / `missing` / `stale` MEMBER on the + * payload. The repair routes drift through this command's existing + * `{ error, …errorCodeFields }` envelope — the one every other failure of this + * command already speaks — and adds no new member to a published output face. + * Naming the drifted files in the machine payload is a widening, and a + * widening is its own card; a case pinning one here would settle that contract + * by test instead. + * + * ## Why this file is not named `.e2e` + * + * The `.e2e` filename tier runs NIGHTLY on `main` and not on a pull request or + * in the merge queue (`scripts/nightly-tiers.mjs`). That is the wrong trade for + * this card's class, for the reason its `--dry-run` sibling records: the + * regression reads GREEN, so between reintroduction and the next nightly every + * run of the pair reports success about a comparison that is not happening, and + * PRs merge on top of it. Its PROJECT is still decided by what it does — it + * spawns the CLI, so `vitest-tiers.ts` classifies it `integration` either way. + * + * ## Fixture placement + * + * The stack config goes under this package's git-ignored `tmp/` and the `--out` + * roots in the system temp dir, for the reason `i18n-extract-check-hint.e2e` + * records: `bundle-require` writes its bundled module next to the config, so + * Node resolves the bare `@objectstack/spec` specifier from THAT directory, and + * only under `packages/cli/tmp/` does that lookup reach this package's real + * `node_modules`. `afterAll` removes only this suite's own `mkdtemp` + * directories — several suites share that root and run concurrently. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { spawnSync } from 'node:child_process'; +import { cpSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { childEnv } from './helpers/serve-process.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); +const CLI_PACKAGE_ROOT = resolve(HERE, '..'); + +/** One object, and a `defaultLocale` equal to the only locale asked for. */ +const STACK_CONFIG = [ + "import { defineStack } from '@objectstack/spec';", + '', + 'export default defineStack({', + " i18n: { defaultLocale: 'zh-CN', supportedLocales: ['zh-CN'] },", + " objects: [{ name: 'kpi_metric', label: 'Metric', fields: { name: { type: 'text', label: 'Name' } } }],", + '});', + '', +].join('\n'); + +/** The one bundle this invocation commits. `--no-metadata-forms` keeps it at one. */ +const BUNDLE = 'zh-CN.objects.generated.ts'; +const FLAGS = ['--locales=zh-CN', '--no-metadata-forms']; + +/** The first line of the sentence both faces end a drifted `--check` on. */ +const DRIFTED = 'Translation bundles have drifted from the schema.'; +/** The refusal a `--check` with no `--out` ends on, on both faces. */ +const NEEDS_OUT = '--check needs --out='; + +let fixtureRoot: string; +let outRoot: string; +let CONFIG: string; +/** An `--out` whose committed bundle is in sync — written once, copied per case. */ +let syncedOut: string; + +/** Text with SGR sequences removed — chalk is off through a pipe, belt and braces. */ +function plain(text: string): string { + // The escape byte is SPELLED, never embedded: a raw control byte in a source + // file renders as nothing and is findable by neither spelling. + return text.replace(/\u001b\[[0-9;]*m/g, ''); +} + +interface Run { + /** stdout ALONE — the machine channel, kept apart so a document can be parsed from it. */ + stdout: string; + /** stdout and stderr together, for the console face's own lines. */ + output: string; + status: number | null; +} + +/** The CLI, from source, with `--check`'s non-zero exit treated as data. */ +function runCli(args: readonly string[]): Run { + const child = spawnSync(TSX, [CLI, 'i18n', 'extract', ...args], { + cwd: CLI_PACKAGE_ROOT, + encoding: 'utf8', + env: childEnv(), + timeout: 180_000, + }); + const stdout = plain(child.stdout ?? ''); + return { stdout, output: `${stdout}${plain(child.stderr ?? '')}`, status: child.status }; +} + +/** + * What the MACHINE face said, in one comparable reading. + * + * `documents` is the count stdout parses into — 1 for a well-formed run, and + * the reading that catches the payload-then-error-envelope shape, which is + * unparseable as one document and would otherwise look like a repair. + * + * ⭐ The drift envelope is TWO lines — a sentence and the command that heals + * it — and both are read, because the second one is where this face can go + * wrong on its own. Reading only the first line is what let a remedy naming a + * `--json` run (which emits a payload and writes zero files) sit green: an + * operator or CI log reader who runs exactly what the failure prints gets + * nothing written and the identical failure next time. That is the #14895 + * loop — "the failure is self-healable and the advice is what stops it + * healing" — so the remedy's two load-bearing properties are pinned as their + * own booleans rather than left to a substring check on line one. + */ +function jsonVerdict(run: Run): { + status: number | null; + documents: number; + /** The `error` sentence's first line, or `null` when the run carried no envelope. */ + error: string | null; + /** Whether the remedy line names an `--out`, i.e. whether it writes anywhere. */ + remedyNamesOut: boolean; + /** Whether the remedy line still carries `--json`, which writes nothing. */ + remedyCarriesJson: boolean; + /** Whether the ordinary extract payload was emitted (its `bundles` member). */ + payload: boolean; +} { + let parsed: unknown; + let documents = 0; + try { + parsed = JSON.parse(run.stdout); + documents = 1; + } catch { + // Anything that is not exactly one document — none, or two concatenated. + documents = run.stdout.trim() === '' ? 0 : 2; + } + const doc = (parsed ?? {}) as { error?: unknown; bundles?: unknown }; + const lines = typeof doc.error === 'string' ? doc.error.split('\n') : []; + const remedy = lines[1] ?? ''; + return { + status: run.status, + documents, + error: lines.length > 0 ? (lines[0] as string) : null, + remedyNamesOut: remedy.includes('--out='), + remedyCarriesJson: remedy.includes('--json'), + payload: typeof doc.bundles === 'object' && doc.bundles !== null, + }; +} + +/** What the CONSOLE face said — the positive control's own reading. */ +function consoleVerdict(run: Run): { + status: number | null; + drift: string[]; + drifted: boolean; + inSync: boolean; +} { + return { + status: run.status, + drift: [...run.output.matchAll(/(missing:|out of date:)\s+(\S+)/g)].map((m) => `${m[1]} ${m[2]}`), + drifted: run.output.includes(DRIFTED), + inSync: run.output.includes('in sync with the schema'), + }; +} + +/** A private `--out` for one case, optionally seeded from the in-sync tree. */ +function outDir(name: string, seeded = false): string { + const dir = join(outRoot, name); + if (seeded) cpSync(syncedOut, dir, { recursive: true }); + else mkdirSync(dir, { recursive: true }); + return dir; +} + +beforeAll(() => { + const sharedRoot = join(CLI_PACKAGE_ROOT, 'tmp'); + mkdirSync(sharedRoot, { recursive: true }); + fixtureRoot = mkdtempSync(join(sharedRoot, 'os-i18n-16600-fixture-')); + CONFIG = join(fixtureRoot, 'objectstack.config.ts'); + writeFileSync(CONFIG, STACK_CONFIG, 'utf8'); + outRoot = mkdtempSync(join(tmpdir(), 'os-i18n-16600-')); + + // A real extract, so the "in sync" tree is what the command itself writes + // rather than bytes this file predicted. + syncedOut = join(outRoot, 'synced'); + const wrote = runCli([CONFIG, ...FLAGS, `--out=${syncedOut}`]); + expect({ status: wrote.status, files: readdirSync(syncedOut) }).toEqual({ status: 0, files: [BUNDLE] }); +}, 300_000); + +afterAll(() => { + // This suite's own directories only. Never the shared `tmp/` root. + rmSync(fixtureRoot, { recursive: true, force: true }); + rmSync(outRoot, { recursive: true, force: true }); +}); + +describe('os i18n extract --check --json — compares, and reports what it found (#16600)', () => { + /** + * The card's own table: two invocations differing only by `--json` must reach + * the same exit code, and the `--json` one must say it found DRIFT rather + * than merely failing. + * + * Falsifier: restoring the unconditional `return` in the `--json` branch + * gives `{ status: 0, documents: 1, error: null, payload: true }` against the + * control's exit 1 and reported `missing:`. + */ + it('reports nothing committed with the same exit code as the same run without --json', () => { + const out = outDir('missing'); + const args = [CONFIG, ...FLAGS, `--out=${out}`, '--check']; + + const control = runCli(args); + const json = runCli([...args, '--json']); + + // The positive control the card supplies: the drift is really there. + expect(consoleVerdict(control)).toEqual({ + status: 1, + drift: [`missing: ${join(out, BUNDLE)}`], + drifted: true, + inSync: false, + }); + // The reading under test — spelled out as well as compared, so two runs + // that BOTH compare nothing cannot satisfy this by agreeing with each other. + expect(jsonVerdict(json)).toEqual({ + status: 1, + documents: 1, + error: DRIFTED + ' Regenerate and commit:', + // The remedy has to be a command that actually regenerates: it keeps the + // `--out` it was given and sheds the `--json` that writes nothing. + remedyNamesOut: true, + remedyCarriesJson: false, + payload: false, + }); + // The card's acceptance, stated as the equality it is. + expect(json.status).toBe(control.status); + // …while writing nothing, which is the half `--check` contributes. + expect(readdirSync(out)).toEqual([]); + }); + + /** + * The second drift shape, and the stronger "writes nothing": a committed + * bundle that is out of date is REPORTED and left byte-for-byte alone. + */ + it('reports a stale committed bundle without rewriting it', () => { + const out = outDir('stale', true); + const bundle = join(out, BUNDLE); + const stale = `${readFileSync(bundle, 'utf8')}\n// edited by hand\n`; + writeFileSync(bundle, stale, 'utf8'); + + // Spelled out rather than compared against a second control run: the + // control-vs-json equality is the case above, and naming the expected + // report is the stronger half of it anyway. One spawn saved. + expect(jsonVerdict(runCli([CONFIG, ...FLAGS, `--out=${out}`, '--check', '--json']))).toEqual({ + status: 1, + documents: 1, + error: DRIFTED + ' Regenerate and commit:', + remedyNamesOut: true, + remedyCarriesJson: false, + payload: false, + }); + expect(readFileSync(bundle, 'utf8')).toBe(stale); + }); + + /** + * The direction no unconditional failure can pass: an in-sync tree exits 0 on + * both faces and the machine one still emits its ordinary payload. + * + * ⚠️ Green on BOTH sides of this card's mutation, deliberately — see the file + * header. Its job is to stop "always fail under `--check --json`" from + * satisfying every other case here, not to detect the defect. + */ + it('passes an in-sync tree on both faces, and still emits the payload', () => { + const out = outDir('in-sync', true); + const args = [CONFIG, ...FLAGS, `--out=${out}`, '--check']; + + const control = runCli(args); + const json = runCli([...args, '--json']); + + expect(consoleVerdict(control)).toEqual({ status: 0, drift: [], drifted: false, inSync: true }); + expect(jsonVerdict(json)).toEqual({ + status: 0, + documents: 1, + error: null, + remedyNamesOut: false, + remedyCarriesJson: false, + payload: true, + }); + expect(json.status).toBe(control.status); + expect(readdirSync(out)).toEqual([BUNDLE]); + }); + + /** + * The other thing the early return skipped. `--check` with no `--out` has + * nothing to compare against, and the console face has always refused it; + * under `--json` the refusal was unreachable, so the run exited 0 with a + * payload having been asked for a comparison it could not make. + */ + it('refuses --check with no --out on the machine face too', () => { + const json = runCli([CONFIG, ...FLAGS, '--check', '--json']); + + expect(json.status).toBe(1); + const verdict = jsonVerdict(json); + expect({ documents: verdict.documents, payload: verdict.payload }).toEqual({ documents: 1, payload: false }); + expect(verdict.error).toContain(NEEDS_OUT); + }); + + /** + * The repair may not be satisfied by turning the machine face into a checker: + * a `--json` run that did not ask for `--check` still emits the payload and + * exits 0, on the very tree the case above fails on. + */ + it('leaves a --json run that did not ask for --check alone', () => { + const out = outDir('no-check'); + const json = runCli([CONFIG, ...FLAGS, `--out=${out}`, '--json']); + + expect(jsonVerdict(json)).toEqual({ + status: 0, + documents: 1, + error: null, + remedyNamesOut: false, + remedyCarriesJson: false, + payload: true, + }); + // `--json` is "output JSON instead of writing files", so the directory the + // drift cases found empty is still empty. + expect(readdirSync(out)).toEqual([]); + }); + // Every case spawns the CLI through `tsx`; measured at ~4 s per run on a + // shared box, over vitest's 5 s default once a case spawns twice. Same + // instrument and the same generous ceiling as the sibling CLI-spawning pins + // in this directory. +}, 900_000);