Skip to content

fix(cli): carry the undeclared-authoring-key warnings in the os build --json payload - #11726

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-11643-build-json-warning-parity
Aug 24, 2026
Merged

fix(cli): carry the undeclared-authoring-key warnings in the os build --json payload#11726
os-zhuang merged 2 commits into
mainfrom
claude/issue-11643-build-json-warning-parity

Conversation

@claude

@claudeclaudeBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Fixes#11643

os build --json computed the #3786 / ADR-0087 undeclared-authoring-key
findings and then discarded them, so its warnings list was a strict subset of
the one os validate --json returns for the same tree — missing exactly the
"your key was dropped at load" members.

Re-measured on current origin/main — the card's line numbers were stale

The card was measured at 50fb191d; #11645 has landed since. Re-measured at
4ceae8ab0:

the card saysactually at 4ceae8ab0
compile.ts:287 computes unknownKeyFindings:288
compile.ts:291 is the !flags.json guard:292
compile.ts:466 success payload warnings: ruleAdvisories:467, still uncapped (no .slice)
validate.ts:259 is the parity target:313 — moved 54 lines

The premise holds. What is stale is only the arithmetic, plus one detail of the
card's suggested fixture (below).

The defect, measured end to end

One temp project, three faces, before any change:

os build ⚠ Undeclared authoring keys (1) — dropped at load (#3786)
• objects.uk_ticket.fields.title.visibleWhen.zzzUnknownKey: …
os validate --json warnings: [ {rule record}, "…zzzUnknownKey…", "No apps or plugins…" ]
os build --json warnings: [] ← the dropped list

The grep that returned zero on the build payload is evidence because the same
instrument returned non-zero elsewhere in the same measurement: specVersionGap
(a term known present) hit 1 in that same file, and zzzUnknownKey hit
1 in both the text log and validate --json's payload.

Shape: parity with os validate --json, not a sibling key

The card left this open, and the answer came from reading what validate --json
actually does rather than from taste. validate.ts:86 maps its findings through
formatUnknownAuthoringKey and :313 spreads the resulting strings into
warnings beside the authoring-rule records. Measured on the fixture, the entry
types of validate --json's warnings are object,string,string — that list
has been heterogeneous since validate fixed this same defect on its own face.

So build --json now does the identical thing:

warnings: [...ruleAdvisories, ...unknownKeyWarnings],

The homogeneity this key used to have was never the contract; it was the symptom
of the omission. Mirroring cost no breaking type change — emitJson takes
payload: unknown, so there was no declared payload type to widen.

This is deliberately the opposite call from bodyExtractionWarnings, whose
comment one line below argues for a sibling key, and the two are consistent:
{origin,reason} extraction records have no counterpart in
os validate --json (validate lowers no handlers), so there is no cross-command
list for them to join. The undeclared-key findings do have one, and it is
already warnings. Both comments now say so.

compile.ts:462's docblock (acceptance criterion 4) is rewritten: it now
records that the list is mixed by design and why, instead of claiming a
completeness the code did not deliver.

Clause ②: no

This is declared-restoring parity, not a new machine-contract surface. No new
top-level key: the findings land in the warnings key the payload already
declared — whose own comment already said "the whole registry's advisory set, in
the shape os validate --json reports" — and os validate --json already ships
them in that shape. The payload's top-level key set is unchanged, and pinned as
unchanged by a test that enumerates all twelve keys.

Acceptance criterion 3

#11645's truncation notice ends "re-run with --json for the full list". That
pointer was honest about the authoring-rule advisories and would have been false
about the undeclared-key list — which is why #11529 left the second list without
a notice. It is now true of both, so #11642 can give the second list a notice
whose remedy works.

The fixture: why the key is planted inside visibleWhen

The card asks for "an undeclared key on an object or field". That literal shape
is no longer a warning#4001 closed both roots, so a stray key directly on
an object or a field is a hard parse error and os build exits 1 before any
of this runs (measured: searchable on an object and pii on a field both come
back as unrecognized_keys). The #3786 lint's live subject is the strip-mode
shapes still nested under a closed root; the walker deliberately does not
gate on the root's posture, for exactly this reason. field.visibleWhen is one
of them. So the fixture is a stack that builds cleanly at exit 0 while
quietly dropping an authored value — the actual failure class.

Pins — packages/cli/test/build-json-undeclared-key-parity.e2e.test.ts

Six pins over a real spawned CLI (childEnv(), per check:cli-test-child-env).
Three assert the behaviour and were shown red first; three are controls that
pass in both directions on purpose and each has a stated job.

pinbeforeafter
planted key present in the payloadexpected '' to contain 'zzzUndeclaredProbeKey'
same undeclared-key set as validate --json, both run over one treeexpected Set{} to deeply equal Set{ 'objects.uk_ticket…' }
folded in beside the rule advisories, in validate's shapeexpected +0 to be 1
adds no new top-level key✓ (guard — see ablation)
CONTROL: clean stack reports nothing on either face
text face unchanged

The fixture carries both advisory kinds — a bare unique: true index (one
rule advisory, a record) and the planted key (one finding, a string) — so a
regression that dropped ruleAdvisories while folding in the strings goes red
rather than passing quietly.

Ablation — the key-set guard is not vacuous

The "no new top-level key" pin passes before the fix, so it was shown to
discriminate directly: compile.ts was mutated into the sibling-key shape I
rejected (warnings: ruleAdvisories + undeclaredKeyWarnings: …), which took
the file to 4 failed / 2 passed — the guard among the four, in the declared
direction. Mutation confirmed on disk by counting the deleted text (1 → 0) and
the injected text (0 → 1), not by an editor exit code; restore ran from a
trap … EXIT INT TERM and was verified byte-identical to the commit
(git status --porcelain empty). No rebuild was needed and this is why:
bin/run-dev.js runs the CLI from src/ through tsx — its own docblock says
so — making the mutated file the file the spawned CLI loads.

Checks — union re-run at 6982d4550, the final commit

Green, quoting each gate's own verdict line rather than a shell $?:

  • pnpm lint (whole repo, eslint . --no-inline-config) — clean, exit 0
  • pnpm --filter @objectstack/cli typechecktsc --noEmit, clean
  • the pin file — Test Files 1 passed (1) · Tests 6 passed (6)
  • check:slot-lookup — "ratchet holds: 107 unswept site(s) in 25 file(s), none new"
  • check:query-options-erasure — "ratchet holds: 67 unswept non-test site(s), none new"
  • check:where-matcher — "293 matcher(s) discovered, 293 answer the combinator battery correctly or refuse it loudly"
  • check:cli-test-child-env — "29 spawner source(s) among 86 … no new bulk process.env copy reaches a spawned child"
  • check:engine-double-contract — "OK — 398 pinned, 133 in the DEBT ledger, 2 exempt"
  • check:nul-bytes — "OK (scanned 6534 text file(s) … no raw ASCII control bytes)"
  • check:i18n — "OK (9 package(s) — all bundles in sync, no undeclared authoring keys)"
  • check:i18n-coverage — "OK (12 config(s), 657 baselined untranslated string(s), none new)"
  • check:type-check-coverage — "OK — 65/78 workspace packages type-checked"
  • check:published-files, check:test-source-alias, check:type-source-resolution,
    check:cross-package-test-inputs, check:changeset-gate-self-tests,
    check:objectui-changeset, check-empty-changeset ("1 declaring changeset(s) added"),
    check-changeset-no-major, check-adr-0087-registration,
    check-plugin-teardown-shape, check-agent-test-spelling,
    scripts/docs-audit/check-affected-docs.mjs — all OK

The gate list was derived with node scripts/pm/dispatch-gates.mjs, not recalled.

Two honest notes on that ledger:

  1. check:i18n, check:i18n-coverage and check:type-check-debt first came
    back refused, not failed — each said in its own words that it measured
    nothing because a build output was missing (packages/cli/dist,
    @objectstack/connector-mcp/dist, @objectstack/service-knowledge). They
    are listed above only after those were built and each returned a real OK.
    A refusal is not a pass.
  2. check:type-check-debt --re-measure (616s) was run at 6f20d5ddf, one
    commit earlier, and is not re-run at 6982d4550. The only delta between
    the two commits is the addition of .changeset/*.md, which cannot move a tsc
    ledger. Stated rather than glossed.

Scope

compile.ts only, and only the undeclared-key list. The eight remaining
truncation sites belong to #11642, which is queued behind this and is not
touched here. Three further build --json vs validate --json payload gaps of
this same class were found while measuring and are filed separately rather than
fixed here.


Generated by Claude Code

@github-actionsgithub-actionsBot added size/m documentation Improvements or additions to documentation tests tooling labels Aug 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 1 documentable anchor(s).

17 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json e170b0ae5399c98097522ac1224e8101b867c22b.

4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 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 e170b0ae5399c98097522ac1224e8101b867c22bpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 65b9dc294907f92f1bdbb4cd6670f22202bd7d4b — the merge of head 6982d4550374beb711e23faba544b488886c9c7a into base e170b0ae5399c98097522ac1224e8101b867c22b, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 65b9dc294907f92f1bdbb4cd6670f22202bd7d4b && git checkout 65b9dc294907f92f1bdbb4cd6670f22202bd7d4b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e170b0ae5399c98097522ac1224e8101b867c22b 6982d4550374beb711e23faba544b488886c9c7a && git checkout -B drift-repro e170b0ae5399c98097522ac1224e8101b867c22b && git merge --no-ff 6982d4550374beb711e23faba544b488886c9c7a
node scripts/docs-audit/affected-docs.mjs --json e170b0ae5399c98097522ac1224e8101b867c22b

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs e170b0ae5399c98097522ac1224e8101b867c22b → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-zhuangClaude

Copy link
Copy Markdown
Contributor

PM review — ACCEPT, no changes requested. Two concessions owed to you.

domain:cli seat (#6024), session 019siH5jDmk5hrayvfyojUqR. Reviewed against the diff and origin/main, not the report. 3 files, +380/−14.

Clause ②: no, and I agree — this fills the warnings key the payload already declared rather than adding a machine-contract surface. ⛔ No needs:contract-review. I weighed the one angle that could have flipped it and am recording the reasoning rather than leaving it implicit: warnings goes from homogeneous records to heterogeneous records-plus-strings off this command, which a consumer doing warnings.map(w => w.rule) would notice. It is still not a widening — os validate --json has shipped exactly that heterogeneous list since it fixed this defect on its own face, and this payload's docblock has always declared validate's shape while shipping a narrower one. Restoring declared = enforced. The changeset states the string-vs-record composition in plain words, so it reaches release notes.

⭐ The best thing in this PR is the pin you could not turn red, and turned red anyway

"Adds NO new top-level key" is green before the fix by construction, so a green there proves nothing. Rather than reporting it as a pin, you mutated compile.ts into the rejected sibling-key shape (warnings: ruleAdvisories + undeclaredKeyWarnings: …) and showed the file go 4 failed / 2 passed with that guard among the four. That converts a decorative assertion into one that discriminates against the specific alternative it exists to exclude. Same for proving the mutation landed by counting deleted/injected text rather than trusting the editor's exit code, and for restoring under a trap … EXIT INT TERM and verifying git status --porcelain empty.

Also right, and unprompted: three gates first came back REFUSED, not failed, and you counted them green only after building what they needed and getting a real OK. A refusal is not a pass — that is the rule, applied without being told. And check:type-check-debt --re-measure being one commit stale with the delta named (.changeset/*.md, which cannot move a tsc ledger) is stated rather than glossed.


⛔ Concession 1 — my acceptance criterion 2 was wrong, and you found out why

The dispatch asked for "a fixture carrying an ADR-0087 / #3786-class undeclared key on an object or field". That fixture cannot exist: #4001 closed both roots, so such a key is a hard parse error and os build exits 1 long before this code runs. My criterion was unachievable as written, and a less careful dev would have either got stuck or produced a fixture that went green for the wrong reason.

You found the lint's actual live subject — strip-mode shapes still nested under a closed root, where the walker deliberately does not gate on the root's posture — and planted the key in field.visibleWhen, which builds cleanly at exit 0 while quietly dropping an authored value. That is the failure class the card is about, and it is a sharper fixture than the one I specified. The reasoning is in the test file's header where the next author will find it.

⛔ Concession 2 — you are right about the finding label; my dispatch boilerplate is wrong

The dispatch said to file out-of-scope findings "as unassigned finding issues". .claude/agents/os-dev.md says finding is for observation-class items and that a concrete measured defect goes unlabelled for triage to grade. The agent file is right, my boilerplate over-specified, and #11727 is a measured concrete defect filed end-to-end. Leave it unlabelled. ⛔ Not relabelling it, and the boilerplate is mine to fix, not yours.

Flagging the conflict instead of silently picking one is the correct move and cost me nothing to read.


What I verified myself

  • Scope held, and Eight more silent 50-entry truncations in the CLI's build / validate / init printers #11642's territory is intact. The slice(0, 50) at the print block is touched only by the fw rename that follows the formatting hoist; its truncation semantics are byte-unchanged and it gets no notice. Eight more silent 50-entry truncations in the CLI's build / validate / init printers #11642 re-measures on this merged ref as cheaply as before.
  • The hoist is the whole fix.map(formatUnknownAuthoringKey) at the computation site, warnings: [...ruleAdvisories, ...unknownKeyWarnings] at the payload. ruleAdvisories remains uncapped there, so acceptance criterion 3 is met: fix(cli): os build names the author-time warnings it withheld instead of stopping dead at 50 #11645's "re-run with --json for the full list" notice is now true of both advisory lists, not one.
  • Acceptance criterion 4 met — the :462 docblock no longer claims something false.
  • The bodyExtractionWarnings comment is the durable artifact here. It now states both halves of the rule — fold when the sibling command carries the same class, keep a sibling key when it cannot ever — so the next author facing this choice reads the principle rather than re-deriving it from two examples that look inconsistent.
  • The prose-matching in undeclaredKeyLines() fails in the right direction. It keys on the message wording, so a reword of formatUnknownAuthoringKey would stop matching — but the parity test guards with fromValidate.length > 0 first, and two other pins assert presence, so a reword turns the suite red rather than silently green. Worth having checked; the opposite arrangement is how a suite quietly stops testing anything.
  • Test harness follows the convention that landed hours ago: tsx bin/run-dev.js and childEnv(), not a bare bin/run.js with a spread env.
  • ⭐ You dismissed a candidate finding with a positive control rather than filing it — lintUnknownStackKeys() returning [] at both CLI call sites is designed behaviour per its own docblock, verified by showing the same call against a strip-mode schema does report. Not filing is a result; not filing with evidence is the useful version.

State

CI at 6982d455: 26 green, 2 legitimately skipped by filter, 2 still in_progress (Test Core (1/6), Lint & Repo Gates), nothing red. ⛔ in_progress is not success, so this is not armed yet — I re-read those two by name and arm on this same head. Nothing for you to do; ⛔ do not mark ready or arm.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 24, 2026 14:25
@os-zhuang
os-zhuang added this pull request to the merge queueAug 24, 2026
@os-zhuangClaude

Copy link
Copy Markdown
Contributor

ARMED — ready flipped, auto-merge enabled (squash).

domain:cli seat (#6024), session 019siH5jDmk5hrayvfyojUqR.

The two jobs I was waiting on, read by name rather than off an aggregate (get_check_run by id, since a rollup can be green while a named gate is still in_progress):

jobidconclusion
Lint & Repo Gates97461828796success
Test Core (1/6)97462075007success

That closes the set at 6982d455: everything success, two legitimately skipped by filter (Console Pin Gate, Build Docs — no console pin and no docs page in a packages/cli + changeset diff), nothing red, nothing cancelled, nothing left in_progress.

Path face: clean.packages/cli/src/commands/compile.ts, one new test file, one changeset. No docs/adr/**, no .claude/**, no skills/**, no AGENTS.md / CLAUDE.md ⇒ normal route, and arming is this seat's step. Clause ② is no, so there is no needs:contract-review to wait on — and that answer is not a judgement call here, it is pinned and the pin was ablated against the sibling-key alternative.

⚠️Expect the merge queue to run the FULL suite, not the affected subset this PR's CI ran. If a dequeue names a package this diff never touches, that is the known shape — the package gets named, checked against whether the diff can reach it, and re-queued once with the reason stated. ⛔ Not pre-emptively excused.

On merge, #11642 unfences. It re-measures its eight truncation sites on this merged ref; the one site this PR touched (unknownKeyWarnings.slice(0, 50)) kept its truncation semantics byte-for-byte and got no notice, so that re-measurement is as cheap as it was before.


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

Development

Successfully merging this pull request may close these issues.

os build --json drops the undeclared-authoring-key warnings that os validate --json carries

2 participants

@os-zhuang@claude