Uh oh!
There was an error while loading. Please reload this page.
fix(objectql,cli): a navigation contribution relocated past a missing group now says so, at warn and at build time - #14920
Conversation
…iagnostic `SchemaRegistry.applyNavContributions` relocates a contribution whose `group` names no group in the target app to the app's top level. That stays — the read-time fold is order-independent by design and contributions into optional groups must keep working — but the only trace was one `log()` call gated at `info`/`debug`, so at `OS_REGISTRY_LOG=warn` an app's information architecture changed in complete silence. The trace is now an ADR-0038 BuildIssue-family record (ADR-0112 D6c: a diagnostics code, lowercase and out of the error ledger) naming the contributing package, the app, the missing group id and the relocated items. It is carried on the app (`getAppNavDiagnostics`) and announced through a new level-aware `warn()`, once per registry per distinct mis-aim. `os build` answers the same question at compile time over a composed artifact, through the same predicate, and reports it loudly without refusing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m
- `packages/runtime/src/dispatcher-error-vocabulary.ts`: classify `nav_contribution_group_missing` as `foreign-vocabulary` / door `none`. ADR-0112 D6c on all four of its tests — payload of a success, describes an artifact, severity `warning`, never routed to `error.code` — so it is lowercase and stays OUT of the error-code ledger. The gate reports it here rather than delegating to `check:error-code-casing` because the constant is referenced (`objlitconst`), not quoted at the stamp. - `content/docs/permissions/system-context.mdx`: line rot repaired by `check-system-context-census --fix` after the registry edit shifted anchors. - `packages/cli/src/utils/nav-contribution-groups.test.ts`: module-top side-effect load of the dist-resolved dependency, per `check:test-source-alias` — a first load inside a clocked window is the measured cliff that gate exists to stop. - changeset: objectql + cli patch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m
…rose `check:doc-authoring` — a runtime string reaches authors, operators and generated surfaces, none of whom can resolve `#NNNN`. The anchor moves to an adjacent comment, where the reader who can resolve it is already looking. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m
📓 Docs Drift CheckThis PR changes 3 package(s): 27 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 45 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 0ed8e406d776a697ce96b1a508060bb60cc95db4 && git checkout 0ed8e406d776a697ce96b1a508060bb60cc95db4
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 3210b3d5da5fe0e87c1aabd74f0e117c805ab5b9 d22e7541d4664ead7d1f089101c294f1926d05b4 && git checkout -B drift-repro 3210b3d5da5fe0e87c1aabd74f0e117c805ab5b9 && git merge --no-ff d22e7541d4664ead7d1f089101c294f1926d05b4
node scripts/docs-audit/affected-docs.mjs --json 3210b3d5da5fe0e87c1aabd74f0e117c805ab5b9
|
…that is not there The Setup app page explains the shell/anchor mechanism — the shell publishes empty group anchors and capability plugins contribute into them — but stopped short of the failure the anchor id makes possible. A contributor cannot see the shell's ids at authoring time, so a typo is undetectable from its own source, and the platform relocates rather than refusing: the menu renders, a smoke test passes, and the entry has moved one level up. Documents the rule and the diagnostic that now reports it: the `warn`-level `nav_contribution_group_missing`, the per-app `getAppNavDiagnostics` reader, and the `os build --json` `navigationGroupDiagnostics` key — following the `bodyExtractionWarnings` precedent, which documents a build-only JSON key on the page that owns the behaviour rather than in a CLI schema dump. No hand-written page was falsified by the code change; this fills a gap rather than correcting an error. The `packages/spec` describe() for the key has the same gap and is filed separately — that string is embedded in ~14 generated artifacts, so it belongs in its own PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m
It landed mid-intro on the first pass, which orphaned the intro's closing paragraph under an H3 and put that H3 ahead of the page's first H2. It now sits at the end of `## Navigation`, directly after the anchor table — where a reader has just learned the anchor ids and would ask what happens if one is wrong. Content unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m
…d key Two standing pins caught the first cut and both are right: `build-json-advisory-parity.e2e.test.ts` and `build-json-undeclared-key-parity.e2e.test.ts`, each titled "adds NO new top-level key to the payload — this fills a declared key, it is not a new surface". #11643 and #11727 each faced this choice and filled `warnings`; that payload's shape is mirrored from `os validate --json` so a consumer reads one shape per class from either command. `navigationGroupDiagnostics` is gone and the findings ride `warnings`. A third pin in the same file settles what that implies: the only permitted residue between the two payloads is the structural advisory set, and "nothing rides in build that validate does not also report". So `os validate` computes the same list — which is the better answer on its own terms, since an author running `validate` should see a mis-aimed contribution exactly as one running `build` does. `findNavGroupDiagnostics` now derives the artifact's package entries itself (`artifactPackagesOf`), so both commands reach the check through one call that needs only the parsed stack, and the test uses that shipped derivation rather than a second copy of the id rule. `...navGroupWarnings` is APPENDED after `structuralWarnings` in validate, and that position is load-bearing: #12047's `the order lives at ONE site` pin matches the five existing members as contiguous source text, which is how it proves the order is defined once rather than re-spelled per exit. Appending keeps that pin guarding exactly what it was written to guard — no gate was loosened to get green. Reproduced first: 2 failed / 12 passed on the two key-set pins, naming `navigationGroupDiagnostics`; 14 passed after. The #12047 pin was then caught locally by the same discipline and is green with the other three (27 passed across the four files). Full cli unit project: 165 files / 2176 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m
All four were self-inflicted by the previous commit, which deleted the `navigationGroupDiagnostics` key and made `os validate` compute the list — and all four are exactly the class the docs-drift bot says it cannot detect, since a page stating a rule by its inputs shares no identifier with the emitter. 1. `content/docs/ui/setup-app.mdx` documented the deleted key. That page is what a contributor reads before authoring an anchor id, so it is the worst place in the repo for the sentence to be wrong. It now says the findings ride the declared `warnings` key and that `os validate` reports them too. 2. The changeset named the deleted key. This one propagates: the changeset is this PR's input to the release notes, so a wrong key here would have become a wrong key in a published release. 3. `nav-contribution-diagnostics.ts` claimed this file is registered in `check-error-code-casing`'s `EXEMPT_FILES`. It is not, and the reason is the discovery the previous commits recorded: the code is REFERENCED at the stamp (`objlitconst`), so that gate's lowercase delegation never reaches this position and `dispatcher-error-vocabulary` classifies it instead. The docstring was the pre-discovery version and sent the next reader to the wrong file to look for a row that is not in it. It now describes what was actually done, and says not to add an `EXEMPT_FILES` entry — that list exempts files from a gate this one does not trip. 4. The vocabulary row's own `why` prose carried the dead key. Corrected to the `warnings` list of both commands; the four-test D6c argument is untouched. The historical note in `nav-contribution-groups.ts` keeps the old key name on purpose — it describes what the first cut did and why the pins rejected it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m
hotlong
commented
Sep 3, 2026
Review — accepted, with one correction to a sentence in the PR bodyPM seat for epic #14122, session Correction — the body asserts two gates do not exist, and they doUnder New public surface needs no generated-artifact update:
That is false. Both exist as scripts of PR #14854's verification record names both by those exact script names, which is what prompted the check. The paragraph's verdict is unaffected and stands. Both scripts are scoped to Recording it here rather than editing the body because the body was edited after I last read it and a blind overwrite would have clobbered that edit. It is worth correcting rather than shrugging off: in a repo this gate-driven, a PR body stating that two gates do not exist is the sentence a future author will quote when skipping them on a change that does touch What I verified rather than took on the report
The two deviations from the ruling's literal wording — acceptedNot stamping the protocol's Flipping out of draft. Clause-② is no — no accept-set change, a diagnostic added and no refusal — so this needs no contract review. ⛔ Not arming auto-merge, and not approving: that is the human reviewer's. Generated by Claude Code |
Fixes#14553
Maintainer ruling on that card (comment 5518065119, verbatim 「同意」): option B + C. This is the B (platform) half. C is hotcrm's own assertion, tracked as hotcrm#1456 and not touched here.
What was wrong
A package injecting navigation into another package's app names the target container by id (
navigationContributions[].group). When that id matches notype: "group"node in the target app,SchemaRegistry.applyNavContributionsappends the items at the app's top level and continues.The only trace was one
this.log(...)gated atinfo/debug. AtOS_REGISTRY_LOG=warn— the levelOS_REGISTRY_LOGexists to select, and the level objectql's own vitest config pins — the relocation was completely silent. A typo'd group id, exactly what an AI author emits, turned a nested menu entry into a top-level one; because the entry was still present, no smoke test noticed. That is the worse of the two failures the card weighed: a dropped entry is missing and someone notices.What changed, and what deliberately did not
The fold is unchanged. The relocation still happens, the merge stays an order-independent read-time fold (
registerAppNavContributionstill does not require the target app to exist yet), and contributions into optional groups keep working. No refusal was added — option A, with the registration-order constraint it implies, was weighed and not taken.PROPOSITION 2 (DOES NOT HOLD AS POSED)is unchanged and is what would notice if the fold ever moved.Runtime half —
packages/objectql:nav-contribution-diagnostics.tsholds one resolution (findNavGroup) and one wording.SchemaRegistry.findNavGroupnow delegates to it rather than keeping a second copy of the walk.BuildIssue-family record naming the contributing package, the app, the missing group id and the relocated item ids. It is carried on the app —registry.getAppNavDiagnostics(appName)— and announced through a new level-awarewarn()(silent only aterror/silent), so it survivesOS_REGISTRY_LOG=warnand reachesos doctor/ boot output.Set: a process-global one would silence the second registry to hit the same mis-aim, which is exactly the shape the visibility pin reads.silentstill gets silence and still keeps the record — muting the log must not destroy the app's verdict.Authoring-time half —
packages/cli:os buildandos validateanswer the same question over a composed artifact, through the same imported predicate, and report it in the text output and in--jsonunder the existingwarningskey.os buildaccepts.examples/app-multi-packagenow demonstrates the mechanism it was missing: the App package publishes asales_groupcontainer and the Orders module contributes its nav entry into it — the shape a module split converts an app's own navigation into.The
--jsonpayload shape — four pins, and how they were settledThis is the part of the design that changed under review, so it is recorded rather than quietly fixed. The first cut gave
os build --jsona new top-levelnavigationGroupDiagnosticskey. That was wrong, and CI said so.What went red, reproduced locally before any fix (
Test Core (1/6), headd47ad8961):build-json-advisory-parity.e2e.test.tsandbuild-json-undeclared-key-parity.e2e.test.ts, both on the assertion titled "adds NO new top-level key to the payload — this fills a declared key, it is not a new surface" — 2 failed / 12 passed, the diff naming+ "navigationGroupDiagnostics".What I read before choosing.#11643 and #11727 each landed an advisory class into this payload and each was required to fill
warningsrather than add a key; the #11727 header records that the payload's shape is mirrored fromos validate --jsonprecisely "so a consumer reads one shape per class from either command rather than learning two". So the payload is deliberately closed, and my earlier reading of thebodyExtractionWarningscomment was a misread: that key is pre-existing and declared, and its note explains why it is separate — it is not a licence to add a sixth.A third pin decides the rest. The same file asserts that the only permitted residue between the two payloads is the structural advisory set, and that "nothing rides in build that validate does not also report". Folding the findings into build's
warningsalone would have turned that pin red. Soos validatecomputes the same list — which is the better answer on its own terms, since these two commands are one wall with two doors (#4409 / #4463), and an author runningvalidateshould see a mis-aimed contribution exactly as one runningbuilddoes.No gate was edited. After the fix a fourth pin fired locally — #12047's "the order lives at ONE site", which matches validate's five
warningsSoFar()members as contiguous source text, that being how it proves the order is defined once rather than re-spelled per exit. My member had been slotted (with a comment) betweencapProviderWarningsandstructuralWarnings. The fix was to append after the five rather than loosen the regex, so the pin still guards exactly what it was written to guard; a comment at the site says so, and says not to loosen it.After: 14 passed on the two originally-red files, and 67 passed across all seven build/validate
--jsonpayload pin files.Docs — including four sentences this PR falsified itself
Prompted by the docs-drift bot, and answered by hand rather than by trusting its anchor list — its own caveat is that a page stating a rule by its inputs shares no identifier with the emitter implementing it, and this diff is emitter-side.
No PRE-EXISTING hand-written page was falsified. Measured on the tree the bot read (
8175a067f): 8 pages namenavigationContributions, and no page anywhere incontent/docsnamesOS_REGISTRY_LOG. Of those 8, the only two carrying thegroupsemantics (references/kernel/manifest.mdx,references/ui/app.mdx) areAUTO-GENERATEDfrom the schema's.describe(), and their claim — "omit to append at the app top level" — is about the omitted case and remains true. The hand-written ones were re-read in full; none states the missing-group case.What was missing was documentation, not a correction.
content/docs/ui/setup-app.mdxnow carries a section at the end of## Navigation, right after the anchor table, stating the relocation, why it is not a refusal, thewarn-level diagnostic,getAppNavDiagnostics, and the fact that both commands carry the finding in--jsonunderwarnings.navigationGroupDiagnostics— precisely the class the bot says it cannot detect. Fixed in a follow-up commit, listed because a reviewer should see that the prose was re-checked against the code and not assumed:content/docs/ui/setup-app.mdxdocumented the deleted key. That page is what a contributor reads before authoring an anchor id, so it was the worst place in the repo for the sentence to be wrong.nav-contribution-diagnostics.tsclaimed the file is registered incheck-error-code-casing'sEXEMPT_FILES. It is not, and the reason is the discovery made later in this PR: the code is REFERENCED at the stamp (objlitconst), so that gate's lowercase delegation never reaches this position. The docstring was the pre-discovery version and sent the next reader to the wrong file to look for a row that is not in it. It now describes what was actually done and says not to add anEXEMPT_FILESentry — that list exempts files from a gate this one does not trip.whyprose carried the dead key; corrected to thewarningslist of both commands.The historical note in
nav-contribution-groups.tskeeps the old key name on purpose — it records what the first cut did and why the pins rejected it.⛔ The 4 release-owned pages were not touched (AGENTS.md Documentation Guardrails). Checked for factual error anyway: no page under
content/docs/releases/mentionsnavigationContributions, the nav-group rule, orOS_REGISTRY_LOG, so none is falsified and none needs a follow-up.New public surface needs no generated-artifact update.
packages/spec/api-surface-signatures.jsontracks only@objectstack/spec'sdefine*authoring functions — it names neitherobjectqlnorSchemaRegistry, and the pre-existing siblinggetAppNavContributionsis likewise absent, which is the precedent. The repo has nocheck:api-surface/check:generatedscript.Filed, not fixed here: #14925 (with a follow-up comment correcting it for the final shape). The schema
.describe()forgrouphas the same gap the page did. That one string is embedded in ~14 generated artifacts, so extending it pulls the spec regeneration family into a PR scoped to an objectql/cli diagnostic.Where the compile-time check went, and why
In the
os build/os validatestep (packages/cli/src/utils/nav-contribution-groups.ts), not incomposeStacks. That is the ruling's own wording, and it avoids a three-way conflict onpackages/spec/src/stack.zod.ts, which PR #14854 and the #14512 producer half are both editing.It is also not an
@objectstack/lintauthoring rule, and that was a real fork in the road worth recording.runAuthoringRuleshands a rule one stack (run(stack, ctx)) and every member of that table reads one; no rule readsparsed.packages. This question is cross-package by construction — the group id is declared by package A's app and named by package B's manifest — so the per-package walk sees neither half alone and only the artifact sees both.No spec-side change was needed: the commands already walk
parsed.packages[].manifest, which carries bothappsandnavigationContributions. That the commands' ownnormalizeStackInput+ObjectStackDefinitionSchemaparse preserves that path is asserted rather than assumed — see the parse-chain pin.Two recorded deviations from the literal wording
_diagnosticsenvelope. Stamping_diagnosticson the app was measured to be harmful in two independent ways:protocol.ts' governance report readsitem?._diagnostics ?? computeMetadataDiagnostics(t, item), so a pre-stamped envelope would suppress the app's spec-validation verdict; andMetadataValidationResult.warnings[]is{ path, message }with nocode, under a closed schema, so carrying an ADR-0112-shaped entry there would be an accept-set widening — which clause-② rules out.warning, and nothing routes it toerror.code. D6c: such codes "stay lowercase and out of the ledger". Which gate wants a row was measured, and the first guess was wrong:check:error-code-casingnever sees the value (the code is referenced, not quoted, at the stamp — theobjlitconstshape), socheck:dispatcher-error-vocabularyis the gate that reports it, and it now carries aforeign-vocabulary/ doornonerow with that evidence. NoERROR_CODE_LEDGERregistration, and noEXEMPT_FILESentry.Pins, and the reverse-verification of each
packages/objectql/src/registry-nav-contribution-group-semantics.test.ts— 10 tests, all green:PROPOSITION 2 (DOES NOT HOLD AS POSED)PROPOSITION 2 (visibility)warn,infoandsilent, plus the carried recordPROPOSITION 2 (authoring door)packages/cli/src/utils/nav-contribution-groups.test.ts— 9 tests, all green, composed through the realcomposeStacks(…, { manifest: 'preserve' }), including a pin that both commands' one-argument call form agrees with the explicit one.Ablation A — runtime (source-resolved). Removed only the
warn()emission, leaving the record: 2 pins red (visibility, once-per-mis-aim), the relocation pin stayed green — which is what proves it pins the fold and not the log. Mutation confirmed on disk before the run (emission sites 1 → 0, marker count 1, blobc0699cf0→3e66307f). Restored withgit checkout HEAD --naming the file by ABSOLUTE path; blob back toc0699cf0== the HEAD blob,git diff HEADempty, 0 markers left. Restored leg: 10/10 green.Ablation B — compile-time (dist-resolved, both legs rebuilt).
@objectstack/objectqlis aKNOWN_UNALIASED_TEST_IMPORTSentry for@objectstack/cli, so the cli pin's subject resolves throughdist/. NeuteredcheckNavContributionGroups, rebuilt, andablation-dist-preflightconfirmed the marker present in 4 built files: 5 pins red. Restored, rebuilt, preflight--absentconfirmed the marker gone from all 14 built files and the whole tree clean; restored leg green.Two false measurements were caught by that discipline rather than by luck:
grep -cread the empty-brackets in the anchor as a regex character class and reported 0 sites on a mutation that had landed; and the first marker was written as a//comment, which the bundler strips — the preflight refused the run because the marker survived only in sourcemaps.Verification
CI is fully green on the current head
d22e7541d— 37 check runs, all complete, zero failures (2 deliberate skips:Console Pin Gate,Packed-tarball smoke). That includesLint & Repo Gates(the wholecheck:*farm), all sixTest Coreshards plus the aggregate, all fourType Checkjobs, everyDogfoodshard plus its aggregate,Dogfood Verify CLI,Temporal Conformance,Build CoreandBuild Docs. The preceding head799d57e15— same code, before the four-sentence prose correction — was green on the same 37.Gate union locally, family derived by
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, clean tree: 64 gates — 57 green, 0 findings, 7 NOT MEASURED locally. Each of the 7 self-declares an unmet prerequisite and refuses a verdict; CI satisfies all of them, and itsLint & Repo Gatesis green.Three gates went red on my own work and were fixed, not exempted:
check:test-source-alias(a dist-resolved first load inside a clocked window — moved to a module-top side-effect import, keeping the production import lazy),check:doc-authoring(a tracker id in runtime string prose — moved to an adjacent comment, per the 2026-08-12 ruling 「处理 issue 时犯的错应该总结成经验,保留 issue id没有意义」), andcheck-system-context-census(pure line rot from the registry edit, repaired with its own--fix).Suites and typechecks, all re-run on the final head:
pnpm --filter @objectstack/objectql test→ exit 0, 267 files / 4595 tests.pnpm --filter @objectstack/objectql typecheck→ exit 0.pnpm --filter @objectstack/cli exec tsc --noEmit→ exit 0, 0 errors (measured for real once the 57-package dependency closure was built for the e2e reproduction — it had been NOT MEASURED before that).pnpm --filter @objectstack/cli --project unit→ exit 0, 165 files / 2176 tests.--jsonpayload pin files → exit 0, 67 tests.Changeset:
@objectstack/objectqlpatch +@objectstack/clipatch.Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m
🤖 Generated with Claude Code