objectstack-ai
diff --git a/‎.changeset/validate-json-strict-exit-parity.md‎
Lines changed: 74 additions & 0 deletions b/‎.changeset/validate-json-strict-exit-parity.md‎
Lines changed: 74 additions & 0 deletions
diff --git a/‎packages/cli/src/commands/validate-json-strict-exit.e2e.test.ts‎
Lines changed: 195 additions & 0 deletions b/‎packages/cli/src/commands/validate-json-strict-exit.e2e.test.ts‎
Lines changed: 195 additions & 0 deletions
diff --git a/‎packages/cli/src/commands/validate.ts‎
Lines changed: 67 additions & 20 deletions b/‎packages/cli/src/commands/validate.ts‎
Lines changed: 67 additions & 20 deletions
@@ -0,0 +1,74 @@
+---
+"@objectstack/cli": minor
+---
+
+fix(cli): `os validate --json --strict` exits 1 on the configs `--strict` already exits 1 for (#11174)
+
+`commands/validate.ts` emitted the `--json` payload and `return`ed *above* the only
+`flags.strict` reader, which sat inside the text-rendering block. So on one config,
+one flag, two answers:
+
+```
+os validate --strict          → exit 1   ("Strict mode: warnings treated as errors")
+os validate --json --strict   → exit 0
+```
+
+`--strict` was accepted, documented — `content/docs/deployment/cli.mdx` spells
+`os validate --json --strict` twice in its CI/CD section, once as a GitHub Actions
+step — and inert whenever `--json` was also passed. That combination is the one
+audience the flag exists for: a pipeline gating on the exit status of the documented
+invocation read 0 and concluded the stack was clean.
+
+The `--strict` gate now reads the text face's own warning list, which is assembled
+once and consumed by both faces, so the two exit codes cannot drift apart again. The
+gate deliberately does **not** read the payload's `warnings` field: the two differ by
+the ADR-0087 load-time conversion notices, which the text face folds into its warning
+block while the payload carries them under `conversions`. Gating on the field would
+have left the same divergence in place for a config whose only advisories are
+conversion notices. `specVersionGap` stays outside `--strict` on both faces, as it
+always has been on the text one.
+
+`valid: true` beside a non-zero exit is the text face verbatim, not a contradiction:
+that path prints "Validation passed" and *then* fails for strict. The stack is
+schema-valid; `--strict` is what promotes its advisories to a failure.
+
+**BREAKING** for one caller shape, and the reason this is not a patch: a pipeline
+running `os validate --json --strict` over a stack that raises non-blocking
+advisories was green and will now be red. Nothing was removed or renamed and no
+authored metadata changes — the accept set is identical and the exit status is the
+only thing that moves — but a release a CI system can take unattended must not flip
+a green build to red, so this does not belong in a patch. It is not a major either:
+the new behaviour *restores* what `--strict` declares ("Treat warnings as errors")
+and what the docs already advertise, rather than contradicting a contract. Under
+this repo's launch-window convention (breaking changes ship as `minor` while the
+stack versions in lockstep) `minor` is the honest slot.
+
+## What a pipeline gating on the payload has to read
+
+`--strict` gates on the text face's warning list, and that list is **not** the payload's
+`warnings` field. The two differ by the ADR-0087 load-time conversion notices: the text
+face folds them into its warning block, while the payload carries them separately under
+`conversions`. So a pipeline that wants to reproduce `--strict` from the document must
+read **both**:
+
+```
+warnings.length > 0 || conversions.length > 0
+```
+
+Gating on `warnings` alone is strictly weaker than `--strict` — a config whose only
+advisories are conversion notices passes that check and fails `--strict`. That is the
+same silent under-reporting this change exists to remove, so do not reach for the
+narrower spelling.
+
+The consequence is reachable and worth stating outright, because it is surprising: a
+conversions-only config now exits **1** with `"warnings": []` and a populated
+`conversions`. Predicting the exit code from `warnings.length` alone will be wrong for
+exactly that config. Nothing is missing from the document — both advisory streams are in
+it — but they sit in two fields and the exit code answers to both.
+
+If a pipeline genuinely wants the old exit status, the honest fix is to say so rather
+than to keep passing a flag that means the opposite: drop `--strict` and read the
+payload. If it goes red instead, the advisories were always there — the text face had
+been printing them all along.
+
+<!-- adr-0087: not-required (no-migration-prescription) An exit-code parity fix on a CLI flag. No authorable key, export, config field or stored `sys_metadata` shape changes, so there is nothing for `objectstack migrate meta` or the upgrade guide to carry — the remedy is a pipeline-side choice of flag, not a rewrite of anything an author wrote. -->
@@ -0,0 +1,195 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #11174 — `os validate --strict` reaches the SAME exit status with `--json` as
+ * without it, over the real CLI process.
+ *
+ * ## The defect this pins shut
+ *
+ * `commands/validate.ts` emitted the `--json` payload and `return`ed *above* the
+ * only `flags.strict` reader, which lived inside the text-rendering block. So on
+ * a config raising four non-blocking advisories:
+ *
+ *     os validate --strict          → exit 1   ("Strict mode: warnings treated as errors")
+ *     os validate --json --strict   → exit 0   (same config, same flag)
+ *
+ * `--strict` was accepted, documented — `content/docs/deployment/cli.mdx` spells
+ * `os validate --json --strict` twice in its CI/CD section, once as a GitHub
+ * Actions step — and inert. A pipeline gating on the exit status of the exact
+ * documented invocation read 0 and concluded the stack was clean.
+ *
+ * This is the second half of a pair. The first (#10953) made the four structural
+ * advisories *reachable* in the payload, so a pipeline could at least gate on
+ * `warnings.length` itself; it did not touch the exit code, and a pipeline
+ * trusting the exit status still could not.
+ *
+ * ## Why the assertions are a PARITY matrix and not `expect(code).toBe(1)`
+ *
+ * A hardcoded `1` pins one cell and says nothing about the property the card is
+ * about: that the two faces of one command agree. Both sides here are read from
+ * their own production source — the real exit status of two real CLI runs of the
+ * same config — and compared to each other. Nothing states an expected code.
+ *
+ * That comparison alone is satisfiable two dishonest ways, and both are closed:
+ *
+ *  - **vacuously**, by a config that exits 0 on both faces. The `warns` fixture
+ *    carries a FLOOR — its text run must exit non-zero — so equality is only
+ *    ever asserted over a run that genuinely had something to fail on. Without
+ *    it this file stays green with the fix reverted.
+ *  - **by breaking the other face** — making `--json --strict` and text agree at
+ *    0. The `clean` fixture pins the zero end, so both faces are held to 1 on
+ *    warnings and to 0 without them; neither can move to meet the other.
+ *
+ * And one further run, without `--strict`, separates "gates on `--strict`" from
+ * "fails whenever `--json` sees a warning" — two fixes that pass the matrix
+ * above identically, only one of which is the one asked for.
+ *
+ * ## Why a real child process
+ *
+ * `process.exitCode` set inside a vitest worker is not an exit status: the
+ * number only exists once Node has exited and the kernel has masked it to
+ * `& 0xFF`. `test/migrate-exit-code.e2e.test.ts` is the precedent and states
+ * this in the same words — the audience `--json` exists for reads the SHELL, so
+ * that is what gets asserted. Spawned through `bin/run-dev.js` + tsx, so the
+ * suite does not depend on `packages/cli/dist` having been built.
+ *
+ * ## Why this file is not beside its siblings in `packages/cli/test/`
+ *
+ * That directory was held by another in-flight card while this one was written,
+ * so it was read-only to this change. `src/` turns out to be the stronger of the
+ * two homes anyway, and deliberately so for the same reason
+ * `utils/format.exit-code.test.ts` gives for living here: `packages/cli/
+ * tsconfig.json` includes `src`, so `pnpm typecheck` compiles this file, while
+ * no tsc program reads `packages/cli/test/`. `tsconfig.build.json` excludes
+ * `src/**\/*.test.ts`, so nothing here ships.
+ */
+
+import { describe, it, expect, beforeAll, afterAll } from 'vitest';
+import { execFile } from 'node:child_process';
+import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join, resolve } from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+const HERE = resolve(fileURLToPath(import.meta.url), '..');
+const CLI = resolve(HERE, '../../bin/run-dev.js');
+const TSX = resolve(HERE, '../../../../node_modules/.bin/tsx');
+
+/**
+ * Raises all four structural advisories at once and nothing else: no
+ * `manifest`, no objects, no apps. The card's own measurement used this shape,
+ * and it still parses — `manifest.id` is schema-REQUIRED once a `manifest` is
+ * present, so a config that merely omits the id fails the parse and exits long
+ * before any advisory is computed.
+ */
+const WARNS_SOURCE = `
+export default {
+  objects: [],
+  apps: [],
+};
+`;
+
+/** The zero-warning control — pins the other end of the matrix. */
+const CLEAN_SOURCE = `
+export default {
+  manifest: { id: 'com.example.strictexit', name: 'strictexit', version: '1.0.0', type: 'app', namespace: 'strictexit' },
+  objects: [{
+    name: 'strictexit_ticket',
+    label: 'Ticket',
+    sharingModel: 'private',
+    fields: { title: { type: 'text', label: 'Title' } },
+  }],
+  apps: [{ name: 'strictexit_app', label: 'Strict Exit App' }],
+};
+`;
+
+interface Run {
+  code: number;
+  stdout: string;
+  stderr: string;
+}
+
+function runCli(args: string[], cwd: string): Promise<Run> {
+  return new Promise((resolvePromise) => {
+    execFile(
+      TSX,
+      [CLI, ...args],
+      { cwd, maxBuffer: 16 * 1024 * 1024, env: { ...process.env, NO_COLOR: '1' } },
+      (err, stdout, stderr) => {
+        resolvePromise({
+          code: err ? (typeof (err as { code?: unknown }).code === 'number' ? (err as unknown as { code: number }).code : 1) : 0,
+          stdout: String(stdout),
+          stderr: String(stderr),
+        });
+      },
+    );
+  });
+}
+
+let warnsDir: string;
+let cleanDir: string;
+
+beforeAll(() => {
+  warnsDir = mkdtempSync(join(tmpdir(), 'os-validate-strict-exit-warns-'));
+  writeFileSync(join(warnsDir, 'objectstack.config.ts'), WARNS_SOURCE);
+  cleanDir = mkdtempSync(join(tmpdir(), 'os-validate-strict-exit-clean-'));
+  writeFileSync(join(cleanDir, 'objectstack.config.ts'), CLEAN_SOURCE);
+});
+
+afterAll(() => {
+  rmSync(warnsDir, { recursive: true, force: true });
+  rmSync(cleanDir, { recursive: true, force: true });
+});
+
+describe('#11174 — --strict reaches the same exit status on both faces', () => {
+  it('a config with advisories: --json --strict exits exactly as --strict does', async () => {
+    const text = await runCli(['validate', '--strict'], warnsDir);
+    const json = await runCli(['validate', '--json', '--strict'], warnsDir);
+
+    // Anti-vacuity floor: equality below is only meaningful over a run that had
+    // something to fail on. This is the reference face, so it is also the
+    // statement that the text side has not moved to meet the JSON one.
+    expect(
+      text.code,
+      `text --strict must fail on this config for the parity below to mean anything:\n${text.stdout}\n${text.stderr}`,
+    ).not.toBe(0);
+
+    expect(
+      json.code,
+      `--json --strict exited ${json.code} where --strict exited ${text.code}, same config.\n` +
+        `json stdout:\n${json.stdout}\njson stderr:\n${json.stderr}`,
+    ).toBe(text.code);
+  }, 120_000);
+
+  it('the failing --json run still emits exactly one parseable document, carrying the cause', async () => {
+    // The exit code must not cost the payload. This file's sibling defect
+    // (`isExitSignal` in `utils/format.ts`) was a `--json` failure path that
+    // emitted TWO documents back to back, parseable as neither one document nor
+    // as JSONL — so a non-zero `--json` run is pinned on the document too.
+    const json = await runCli(['validate', '--json', '--strict'], warnsDir);
+
+    const payload = JSON.parse(json.stdout) as { valid?: unknown; warnings?: unknown };
+
+    // `valid: true` beside exit 1 is the text face's contract verbatim: it
+    // prints "Validation passed" and THEN fails for strict. The config is
+    // schema-valid; `--strict` is what turns its advisories into a failure.
+    // Pinned because the pairing is new here and reads like a bug otherwise.
+    expect(payload.valid).toBe(true);
+    expect(Array.isArray(payload.warnings) && (payload.warnings as unknown[]).length).toBeGreaterThan(0);
+  }, 120_000);
+
+  it('control: a config with nothing to warn about exits 0 on BOTH faces under --strict', async () => {
+    const text = await runCli(['validate', '--strict'], cleanDir);
+    expect(text.code, `text --strict:\n${text.stdout}\n${text.stderr}`).toBe(0);
+
+    const json = await runCli(['validate', '--json', '--strict'], cleanDir);
+    expect(json.code, `json --strict:\n${json.stdout}\n${json.stderr}`).toBe(0);
+  }, 120_000);
+
+  it('control: without --strict, the same advisory-raising config still exits 0 under --json', async () => {
+    // Separates "gates on --strict" from "fails whenever --json sees a warning".
+    // Both satisfy the parity matrix above; only the first is the flag's meaning.
+    const json = await runCli(['validate', '--json'], warnsDir);
+    expect(json.code, `--json without --strict must stay 0:\n${json.stdout}\n${json.stderr}`).toBe(0);
+  }, 120_000);
+});
@@ -247,24 +247,23 @@ export default class Validate extends Command {
         structuralWarnings.push('Missing manifest.namespace — required for multi-app hosting');
       }
 
-      if (flags.json) {
-        await emitJson({
-          valid: true,
-          manifest: config.manifest,
-          stats,
-          // One advisory list for the whole registry. This used to be a
-          // hand-maintained concatenation of per-gate arrays, and it leaked
-          // twice: warnings computed and then dropped from `--json` while the
-          // console printed them. A single list cannot drift from itself.
-          warnings: [...ruleAdvisories, ...docWarnings, ...unknownKeyWarnings, ...capProviderWarnings, ...structuralWarnings],
-          conversions: conversionNotices,
-          specVersionGap: specGap,
-          duration: timer.elapsed(),
-        });
-        return;
-      }
-
-      // 5. Warnings (non-blocking)
+      // 5. Warnings (non-blocking) — assembled HERE, above the `if (flags.json)`
+      //    branch, because this is the list `--strict` gates on and the JSON
+      //    face has to reach the SAME verdict from it. It could not: the payload
+      //    was emitted and `return`ed above the only `flags.strict` reader, so
+      //    `os validate --json --strict` exited 0 on the very configs
+      //    `os validate --strict` exited 1 for. The flag was accepted,
+      //    documented (`content/docs/deployment/cli.mdx` spells the pair twice
+      //    in its CI/CD section, once as a GitHub Actions step) and inert — a
+      //    pipeline gating on the exit status of the documented invocation read
+      //    0 and called the stack clean.
+      //
+      //    Hoisting the assembly rather than restating the condition is the same
+      //    move `structuralWarnings` just above and `unknownKeyWarnings` up
+      //    beside `normalized` already made, for the third time in this file:
+      //    ONE list, consumed by both faces, so the two exit codes cannot drift
+      //    from each other by construction. The push ORDER is unchanged, so the
+      //    text face's warning output is byte-for-byte what it was.
       const warnings: string[] = [];
 
       // [#3366] Installable-provider hints — a declared capability whose provider
@@ -295,12 +294,56 @@ export default class Validate extends Command {
         warnings.push(`${w.path}: ${w.message}`);
       }
 
-      // The four structural advisories, computed above the `if (flags.json)`
-      // branch so `--json` carries them too. Appended HERE, in the position the
+      // The four structural advisories, computed further up so the `--json`
+      // payload can carry them too. Appended HERE, last, in the position the
       // four inline `if` blocks used to occupy, so the text face's warning ORDER
       // is byte-for-byte what it was.
       warnings.push(...structuralWarnings);
 
+      if (flags.json) {
+        await emitJson(
+          {
+            valid: true,
+            manifest: config.manifest,
+            stats,
+            // One advisory list for the whole registry. This used to be a
+            // hand-maintained concatenation of per-gate arrays, and it leaked
+            // twice: warnings computed and then dropped from `--json` while the
+            // console printed them. A single list cannot drift from itself.
+            warnings: [...ruleAdvisories, ...docWarnings, ...unknownKeyWarnings, ...capProviderWarnings, ...structuralWarnings],
+            conversions: conversionNotices,
+            specVersionGap: specGap,
+            duration: timer.elapsed(),
+          },
+          // `--strict` means one thing — "treat warnings as errors" — and it now
+          // means it on both faces. The gate reads `warnings`, the text face's
+          // OWN list, rather than the payload's `warnings` field: the two differ
+          // by the ADR-0087 conversion notices, which the text face folds into
+          // its `⚠` block while the payload carries them under `conversions`.
+          // Gating on the payload field would have left `--json --strict` at 0
+          // for a config whose only advisories are conversion notices — the
+          // same divergence one collection narrower. `specVersionGap` stays out
+          // on both faces; it is never gated by `--strict` (see below).
+          //
+          // `valid: true` beside a 1 is not a contradiction, it is the text
+          // face verbatim: that path prints "Validation passed" and THEN fails
+          // for strict. The stack IS schema-valid; `--strict` is what promotes
+          // its advisories to a failure.
+          //
+          // The status rides in `emitJson`'s `CliExitCode` slot rather than a
+          // following `this.exit(1)`, unlike the failure paths above: those
+          // must stop a fall-through into the text rendering, while here the
+          // payload is complete and the `return` is right there. The slot is
+          // the declared channel for pairing a `--json` document with the
+          // status the shell reads (`utils/format.ts`; pinned by
+          // `utils/format.exit-code.test.ts` and `test/migrate-exit-code.e2e.test.ts`),
+          // and it emits the one document without an ExitError unwinding
+          // through the catch below.
+          flags.strict && warnings.length > 0 ? 1 : 0,
+        );
+        return;
+      }
+
       // 6. Display results
       console.log('');
       printSuccess(`Validation passed ${chalk.dim(`(${timer.display()})`)}`);
@@ -321,6 +364,10 @@ export default class Validate extends Command {
         for (const w of warnings) {
           console.log(chalk.yellow(`  ⚠ ${w}`));
         }
+        // The text face's half of the `--strict` gate. Its JSON counterpart is
+        // the `CliExitCode` argument at the `emitJson` call above, reading this
+        // same `warnings` list — change one and change the other, or the two
+        // faces start disagreeing about the exit status again.
         if (flags.strict) {
           console.log('');
           printError('Strict mode: warnings treated as errors');