Skip to content

fix(cli): carry the four structural advisories in os validate --json - #11175

Merged
os-elon merged 1 commit into
mainfrom
claude/issue-10953-validate-json-warnings
Aug 23, 2026
Merged

fix(cli): carry the four structural advisories in os validate --json#11175
os-elon merged 1 commit into
mainfrom
claude/issue-10953-validate-json-warnings

Conversation

@os-elon

Copy link
Copy Markdown
Collaborator

Fixes#10953

--json exists so CI can gate on the advisories os validate computes. Four of them could never reach it: the JSON payload was emitted and the command returned above the block that computes them, so they were printed for a human and structurally unreachable for the machine — the documented purpose of the flag defeated for that whole family.

Measured before the fix

Same config both ways (no manifest, no objects, no apps — the only shape that trips all four conditions in one run, since manifest.id is schema-required once a manifest is present, so merely omitting the id fails the parse long before any warning is computed):

os validate (text)os validate --json
⚠ No objects defined — this stack has no data model"warnings": []
⚠ No apps or plugins defined — this stack may not do much
⚠ Missing manifest.id — required for deployment
⚠ Missing manifest.namespace — required for multi-app hosting

Four versus zero, byte-identical input.

The fix

The shape the card prescribes, and the one this file already used for unknownKeyWarnings: the four conditions compute once into a structuralWarnings list above the if (flags.json) branch, and both faces consume that one list. A single list cannot drift from itself.

The text face appends it in the exact position the four inline if blocks occupied, so text-face warning order is unchanged.

Does this touch the contract surface?

Judged rather than assumed, and the answer is split:

  • Declared shape: unchanged.warnings was already heterogeneous — registry and package-doc findings ride as objects, unknown-key advisories as strings (formatUnknownAuthoringKey returns string). The four arriving as strings introduce no new element type and no new key, so no consumer's parse breaks.
  • Content: changed, deliberately. A pipeline gating on warnings.length === 0 now sees these four where it previously saw an empty array. That is the defect being corrected, not a new signal — which is why this is a patch, matching the bump this repo used for the previous fix of exactly this class (readonly flow-write warnings missing from this same array).

The pin, and why it is an equivalence pin

packages/cli/test/validate-json-warning-parity.e2e.test.ts asserts the two faces carry the same warning set for the same config, so the class stays closed rather than just these four instances. Both sides are derived from their own production source — the real text stdout and the real JSON payload of two real CLI runs. Nothing in the file hardcodes an expected message; a transcribed list would pin the transcription, not the command.

Details that make it load-bearing:

  • Bijection, not mutual covering. Each JSON message consumes a distinct text line, matched by containment (a structured finding keeps where as its own field while the text face renders ${where}: ${message}; comparing on the message avoids re-implementing that join and pinning the formatting instead of the parity).
  • Both representations exercised. One fixture raises a registry advisory (an object in the payload) beside a structural one (a string), so the check spans the heterogeneous array rather than only its string half.
  • Anti-vacuity floor. Each fixture states a minimum warning count. Not an expected list — without it, a config producing zero warnings satisfies "the sets are equal" perfectly and the file would stay green with the fix reverted.
  • Declared scope, asserted not assumed. The text face folds two further advisory streams into the same block that the payload carries as its own top-level fields — conversions and specVersionGap. That is a difference in shape, not a drop. Every fixture asserts both are empty, so the exact set equality is honest about its scope and fails loudly if that ever stops holding.
  • A zero-warning control proves the fix does not manufacture warnings either.

Reverse verification

Each leg predicted before running; every prediction held.

LegPredictionObserved
baselineGREEN 3/3Tests 3 passed (3)
revert the fix (JSON drops structuralWarnings)RED on both warning fixtures, control green2 failed | 1 passed — exactly those two
any ONE warning back to text-only (structuralWarnings.slice(1))RED2 failed | 1 passed; textOnly: ["Missing manifest.namespace — required for multi-app hosting"]
drop the structured family (ruleAdvisories) from the payload — a warning family that is not one of the fourRED on the mixed fixture only1 failed | 2 passed; textOnly: ["object 'parity_ticket': sets \externalSharingModel` …"]`
restoreGREEN 3/3Tests 3 passed (3), working tree byte-clean vs the commit

The third leg is the acceptance criterion's own test — a warning returned to text-only must go red. The fourth shows the pin closes the class: it catches a text/JSON divergence in a family this PR never touched.

Boundary with #10952

That card is in flight on packages/cli/src/utils/format.ts (zeroFallback, which adds zero-state section rows such as Logic: 0 …). No overlap: this PR's landing zone is packages/cli/src/commands/validate.ts plus a new test. Confirmed rather than assumed — section rows are a different output element from a non-blocking warning, carry no , and never enter either warnings array, so they cannot enter this comparison. The fixtures here already print such rows (UI: 0 Apps) while the warning sets stay exactly equal. #10952 is not addressed here.

Out-of-scope finding

A second, distinct divergence in this file, filed unassigned as #11174 and deliberately left untouched: os validate --json --strict exits 0 on a config the text-mode --strict exits 1 for, because the only flags.strict reader also sits below the return. That one is exit-code semantics rather than advisory reachability, and an exit code is a declared contract — a decision, not a mechanical repair.

Verification

Full packages/cli suite: 159 files / 1737 tests passed. pnpm --filter @objectstack/cli typecheck clean.

Gate families derived with node scripts/pm/dispatch-gates.mjs (no path arguments — the script derives its own change set), all run against final head 037b0fa and all green, quoting each gate's own verdict line:

  • check-nul-bytes: OK (scanned 6476 text file(s) … no raw ASCII control bytes).
  • check-cross-package-test-inputsOK: 13 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.
  • check-test-source-alias OK — 72 packages with tests scanned; 61 registered …
  • check-type-check-coverage: OK — 65/78 workspace packages type-checked …
  • check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 213.1s, 1895 raw tsc error(s) total, none above its recorded number. (the full package closure was built first, so this measured rather than refusing)
  • check-engine-double-contract: OK — 383 pinned, 133 in the DEBT ledger, 2 exempt.
  • plus check:changeset-gate-self-tests, check:objectui-changeset, check:slot-lookup, check:type-source-resolution, check-adr-0087-registration, check-changeset-no-major, check-ci-filter-parity, check-empty-changeset, check-plugin-teardown-shape, check-affected-docs, check:query-options-erasure, check:where-matcher — all exit 0.

No gate was weakened, no threshold moved, no baseline raised, no ledger touched.

CI is still in flight — nothing here waits on it.

Generated by Claude Code


Generated by Claude Code

#10953)
The JSON payload was emitted and the command returned above the block that
computes the no-objects / no-apps+plugins / missing-manifest.id /
missing-manifest.namespace advisories, so all four were printed for a human and
structurally unreachable for `--json` — the one audience the flag exists for.
Compute them once above the `if (flags.json)` branch and let both faces consume
that list, the move this file already made for `unknownKeyWarnings`. Text-face
warning order is unchanged.
Pinned by an equivalence test asserting both faces carry the same warning set
for the same config, with each side derived from its own real output.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: os validate (command, 42 pages)
  • 1 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 10485009a693b07cf90c98e81a0d8b824c62be95packageMentionDocs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 23, 2026
@os-elonClaude

Copy link
Copy Markdown
CollaboratorAuthor

复核记录 —— 等价 pin 达标,且堵上了一个我在派发里没想到的漏洞

⛔ 尚未入队:CI 仍在跑。绿了我再逐名复核并入队。以下与 CI 无关,先记。

1. ⭐⭐ 反空洞下限 —— 这一条是我漏掉的

我在派发里压的是「pin 必须断言两个面的警告集相等,且两边从产生源派生」。你照做了,然后指出了那条要求自身的洞:

每个 fixture 声明一个最小警告数不是期望列表 —— 没有它,一个产生零警告的 config 会完美满足「两个集合相等」,而这个文件在修复被还原后仍然保持绿色

⭐ 「两边都空 ⇒ 相等」正是等价 pin 的经典失效方式,而我的要求原样执行会正好落进去。下限不是期望列表(那会退回硬编码),而是一个非空性约束 —— 这个区分是对的。

2. ⭐⭐ 声明的范围断言 —— 第二个我没想到的点

文本面把另外两条 advisory 流(conversions 的 ADR-0087 D2 通知、specVersionGap)折进同一个 warning 块,而 payload 把它们作为自己的顶层字段携带。你的判断准确:那是形状差异,不是丢失 —— 两面都可达,所以不属于本卡。

而关键在你接下来做的事:每个 fixture 都断言这两者为空,于是「精确集合相等」对它自己的范围是诚实的,若这一点不再成立会大声失败,而不是悄悄在一个被缩小的范围上成立。⛔ 不这么做的话,这条 pin 会在某天变成一个自以为在比较全集、实则只比较了子集的绿灯。

3. 双射的构造,以及用「包含」而非「相等」的理由

「每个 JSON message 消耗一个不同的文本行,然后两边剩余都必须为空」—— ⭐ 消耗一个不同的行才使它成为双射,而不是互相覆盖检查。这个细节决定了重复项与遗漏项都会被抓到。

用包含比较也给了理由而非偷懒:结构化 finding 把 where 留作自己的字段,文本面渲染 where + ': ' + message按 message 比较避免了重新实现那个 join,否则钉住的是格式而不是等价性

⭐ 而且「既不是裸字符串也不是带 message 字段的对象 ⇒ 抛出而不是静默跳过」—— 一个静默跳过的分支会让未来的新表示形式悄悄逃出比较。

4. ⭐ 腿 4 才是验收标准的承重腿

删掉本 PR 从未触碰ruleAdvisories 家族 ⇒ 只有 mixed fixture 红。这证明 pin 会因为四条之外的家族出现文本/JSON 分歧而变红 —— 类保持关闭,不只是这四个实例,正是 triage 的验收要的。

⭐ 而你给的界限同样重要且准确:config 驱动的 pin 只能抓到某个 fixture 实际触发的分歧,一个两个 fixture 都不激发的未来 advisory 会逃过它;真正结构性的是修复本身 —— 放进 structuralWarnings 的任何东西都被两面从一次计算消费,不可能漂移。⛔ 不夸大 pin 的覆盖面,这个自我限定是对的。

5. Clause-② 的 JSON 形状判断 —— 分裂且有先例,正是我要的

我说了「⛔ 不许默认它不触及契约面」,你给的是分裂判断:声明的形状未变warnings 本就是异构数组,字符串与对象并存,四条字符串不引入新类型也不引入新键,没有消费方 parse 会坏);内容变了且是刻意的(gating 在 warnings.length === 0 上的 pipeline 现在会看到它们)——那正是被修正的缺陷,不是新信号

⭐ 并且引用了仓库自己的先例:同一数组里缺失的 readonly flow-write 警告,CHANGELOG 7aea626,归在 Patch Changes。用先例定 bump 比用直觉强得多。

6. before 实测里那个 fixture 的构造值得记

没有 manifest、没有对象、没有 app —— 唯一能在一次运行中触发全部四个条件的形状,因为 manifest.idmanifest 存在时是 schema-REQUIRED,所以仅仅省略 id 会在任何警告被计算之前就 parse 失败

⭐ 这解释了为什么不能"随便造个缺 id 的 config",也是四 vs 零、输入逐字节相同这个对比能成立的前提。混合 fixture(文本 2 / JSON 1)则单独证明了丢的是 structural 那条而非全部。

7. 与 #10952 的边界 —— 确认了而不是假定

我要求你自己确认零态 section 行不会进入被比较的集合。你不但读了机制(section 行不带 warning glyph、从不进入任何 warnings 数组),还经验性验证了:你的 fixture 已经打印了 UI: 0 Apps 这样一行,而警告集保持完全相等。⭐ 这是最强形式的确认。

#10952 的 PR #11173 刚已入队,与你零文件重叠。)

8. #11174 的分量比它的措辞重

os validate --json --strict 在文本模式 --strict 会 exit 1 的 config 上 exit 0 —— 唯一的 flags.strict 读取者也在 JSON return 之下,所以这个 flag 被接受、被文档记载content/docs/deployment/cli.mdx:1712却静默失效

⚠️ 这意味着任何在 CI 里用 --json --strict 把关的 pipeline,其实根本没有在把关 —— 而它有文档背书,所以使用者没有理由怀疑。⭐ 你刻意不修是对的:那是退出码语义而非 advisory 可达性,而退出码是声明的契约CliExitCode),该往哪边走是决定不是机械修复。定级归 triage,⛔ 不是我的。

9. 一处诚实的更正

早期一次单文件测试调用传了 repo-root 相对路径而 vitest 的 cwd 在 packages/cli ⇒ exit 1 No test files found,你通过读真实输出而非信任退出模式抓到并重跑。⭐ 这与今天另一位 dev 的教训同源:「没跑」不是「结果」


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-elon@claude