Skip to content

os validate --json drops advisory lists it has already computed on all five of its failure exits #12047

Description

@os-zhuang

Found while implementing #11772, which closes exactly this defect one command over (os build / compile.ts). Out of that card's scope: the maintainer ruling on #11772 is worded for compile.ts specifically, and extending the same widening to a second command's public --json payload is its own machine-contract decision (Clause ②), not a mechanical port. Recorded rather than fixed.

Measured

Read from source at origin/main c804f0ca5, packages/cli/src/commands/validate.ts. Computation sites vs. emitJson exits, by line:

 86  const unknownKeyWarnings = […]        <- computed FIRST, pre-parse
 94  emitJson  parse failure               errors            (no warnings)
126  const { advisories: ruleAdvisories }
133  emitJson  rule errors                 errors, warnings: ruleAdvisories
169  const capProviderWarnings = […]
175  emitJson  capability errors           errors            (no warnings)
203  const docWarnings = […]
206  emitJson  doc errors                  errors, warnings: ruleAdvisories
238  const structuralWarnings = []
306  emitJson  success                     warnings: all five lists
391  emitJson  catch-all                   error             (no warnings)

So on every one of the five failure exits the payload carries strictly less than the run had already computed:

exit computed by then carried dropped
parse failure (94) unknownKey — 1
rule errors (133) unknownKey, rule rule 1
capability errors (175) unknownKey, rule, capProvider — 3
doc errors (206) unknownKey, rule, capProvider, doc rule 3
catch-all (391) whatever the run reached — all

Note the parse-failure exit in particular: validate.ts computes unknownKeyWarnings at line 86, before the parse, precisely so the finding survives an unrelated schema error — and then the parse-failure payload drops it anyway. That is a stronger instance than the one #11772 fixes, where nothing is computed that early.

Why it matters

Same shape as #11643 / #11391 / #11772: the text face prints these advisory blocks with — re-run with --json for the full list, and the --json re-run of a tree that fails a later gate returns a payload without the list in it. The remedy the notice names is unreachable until an unrelated failure is fixed.

Not proposed here

The obvious route is the one the maintainer ruled for os build on 2026-08-25 (option 1: every failure exit carries the lists the run has already computed, so warnings means the same thing on every exit). Whether os validate --json's contract should widen the same way — and whether its structuralWarnings member changes the answer — is a decision for triage, not this filing. #11772 is not addressed by this issue; it is already closed out on the build side.

Filed unassigned, no labels — severity and route are triage's call.


Generated by Claude Code

Activity

  1. yinlianghui-tw commented on Aug 25, 2026

    @yinlianghui-tw
    Collaborator

    Triage: pm:queue + domain:cli, type Bug — the text face promises "re-run with --json for the full list" and the JSON face drops the list on every failure exit; that is the same declared-≠-enforced promise #11772 was ruled on.

    Route inherited under the same-family rule rather than re-opened: the maintainer ruled option 1 for os build on 2026-08-25 (#11772 — every failure exit carries the lists the run has already computed, so warnings means the same thing on every exit), and the ruling's reason applies verbatim to validate.ts, including the strongest instance here (line 86 computes unknownKeyWarnings before the parse precisely so it survives a schema error). One genuine-difference check is required before implementing, not after: whether structuralWarnings changes the answer — if measurement shows it does (a semantic difference, not just an extra list member), stop and fork back to triage instead of improvising.

    Clause-② yes — this widens the public --json failure-payload face, so the dispatch runs at the contract-review tier; the claim comment must declare it. Serial note: #11772 is pm:dispatched in flight — different file (validate.ts vs compile.ts), but if the fix wants a shared emit helper, that is a fold-or-serial answer at claim time, not an improvised refactor.

    Size/model suggestion: S–M; tier is pinned by Clause-② regardless.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions