Skip to content

fix(cli): nav-contribution-groups imports the artifact package-id owner instead of carrying a second copy - #18710

Merged
os-support-ai merged 2 commits into
mainfrom
claude/issue-18490-artifact-package-id-second-copy
Sep 17, 2026
Merged

os-support-ai merged 2 commits into
mainfrom
claude/issue-18490-artifact-package-id-second-copy

Conversation

@os-support-ai

@os-support-ai os-support-ai commented Sep 17, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #18490

What changed

packages/cli/src/utils/nav-contribution-groups.ts no longer carries its own copy of the artifact package-id rule. artifactPackagesOf is deleted; artifactPackages — the owner, in packages/cli/src/utils/artifact-packages.ts — is imported and used as findNavGroupDiagnostics' default package walk. This is the same move permission-set-name-collisions.ts already made, for the reason that module states in its own header.

CompiledPackage stays: it is a structural parameter type naming the two facts this module reads, not a second spelling of the id rule.

The half the card left open, measured before anything was edited

The card is explicit that importing the owner is a decision, not a merge: the owner answers '' for a package whose id keys are empty, the deleted copy answered the positional spelling packages[0]. The dispatch asked which is right for the nav path, and required it measured rather than reasoned.

M1 — the divergent input is REACHABLE. ManifestSchema requires id and name as strings and constrains neither to be non-empty, so { manifest: { id: '', name: '', … } } parses green through the very normalizeStackInput + ObjectStackDefinitionSchema chain both commands run. It narrows the card's framing: both keys have to be empty. With id absent the parse refuses (packages.0.manifest.id: expected string, received undefined); with name absent, likewise. With id: 'ok' and name: '' the two rules agreed already.

M2 — behaviour does change, and only there. On the reachable input the two rules produce different diagnostics; on every other input, byte-identical output (control legs: id ok + name empty, and both ok).

package identity copy (deleted) owner (imported)
id: '', name: '' packageId: 'packages[0]' packageId: ''
id: 'com.example…', name: '' com.example… com.example… (identical)
id: 'com.example…', name: 'Orders' com.example… com.example… (identical)

M3 — the runtime is the judge, and it answers ''. nav-contribution-groups.ts' own header says the id it carries is "the string the runtime registers a contribution under, so a command names a package the same way the fold does". Asked of a real ObjectQL: registerApp derives manifest.id || manifest.name, which has no positional fallback at all, so the fold registers that package under '' and prints:

[Registry] [nav_contribution_group_missing] Package "" contributes 1 navigation item [nav_orders] into group "sales_grp" of app "multi_crm", …

The deleted copy made os build print Package "packages[0]" for that same artifact. ⇒ the owner's '' is not merely different, it is the one that matches the runtime, and packages[0] is a name the runtime cannot produce. The STOP-AND-REPORT condition in the dispatch is therefore not triggered — the measurement came out in favour of the direction triage settled, and it could have come out the other way: had registerApp carried a positional fallback, or dropped a contribution whose id is empty, the copy would have been the runtime-matching side.

M2b — the id is carried and printed, never keyed on. Two packages that both resolve to '' still produce two findings under both rules; nothing on this path uses the id as a map key, a dedupe key, a route segment or a sort key. Downstream it is spread into the warnings array of both commands' JSON payloads, unkeyed.

M4/M5 — the one robustness delta, and why it cannot be reached. The owner does not re-check entry shape (its header declares that precondition). A null element of packages[] throws under the owner where the copy returned a positional id. Every malformed element — null, a string, a non-object manifest, a missing one — is refused by ArtifactPackageSchema before either command's findNavGroupDiagnostics(result.data) sees it; measured, all five refused. The remaining shapes (manifest a string, manifest absent) produce identical output under both rules anyway.

⚠️ Two different claims, kept different: this PR shows no current shipped artifact with empty id keys — it does not look for one, and the card says the blast radius is unmeasured. What it shows is that the input is accepted by the schema both commands run, so the divergence was reachable rather than latent-by-construction.

Tests

packages/cli/src/utils/nav-contribution-groups.package-id.test.ts — new, four pins:

  1. the empty-id-and-name artifact parses (the floor: if a spec change starts refusing it, this reds first and says the pins under it now measure nothing);
  2. the build names that package exactly as the runtime fold does — build side from the shipped findNavGroupDiagnostics, runtime side from a real ObjectQL, neither rule re-spelled, asserting the two packageId strings and the two messages are equal;
  3. and that shared name is not the deleted copy's positional spelling (stated separately, because pin 2 would also pass if both doors moved to packages[0]);
  4. two empty-id packages still produce two findings.

Why a separate file. new ObjectQL( is a KERNEL signal in packages/cli/vitest-tiers.ts, so a file carrying it is integration tier by derivation. Measured: adding the pin to nav-contribution-groups.test.ts moved that whole file — and its nine existing #14553 pins — out of the unit tier (unit 212 → 211, integration 47 → 48). Splitting confines the tier change to the cases that actually boot a registry. The unit file now differs only by the import repoint and two comments, and unitTestFiles() places it back in unit.

Ablation — the fix reverted to its pre-change blob, the new pins re-run, then restored:

pre-mutation : fe0c8f3bf05ff3b2d00c8f46d4aaebaa61b7f77e   (== HEAD blob)
post-mutation: d4b626b38c4a72926b92f0b9bc3d5bfe842d77df   (== the BASE blob)
  artifactPackagesOf occurrences: 0 -> 2
  'artifactPackages(parsed)'    : 1 -> 0
ablated run: 3 failed | 1 passed (4)
  AssertionError: expected 'packages[0]' to be ''
post-restore : fe0c8f3bf05ff3b2d00c8f46d4aaebaa61b7f77e == HEAD blob; git diff HEAD empty

The one pin that stays green under ablation is the reachability floor, which reads the owner directly — the in-run control that the harness is not simply broken.

Verification, at 6a886b8a4

what result
pnpm lint — repo-wide, eslint . --no-inline-config, full population, no narrowing exit 0
pnpm --filter @objectstack/cli typecheck (incl. check:test-typecheck) exit 0
pnpm --filter @objectstack/cli exec vitest run --project unit 212 files, 3026 tests, exit 0
pnpm --filter @objectstack/cli exec vitest run --project integration 48 files, 413 tests, exit 0
dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, every family run, reconciled with --ran carrying each exit code 60 derived, 60 run, 0 NOT-MEASURED, 0 UNRUN

The integration tier was run locally because this diff adds an integration-tier file; it touches no existing one, and no spawn entry.

An earlier reconciliation at an intermediate commit recorded exit 3 for check:dual-build-cjs-loads and check:i18n-coverage (PREREQUISITE NOT MET — unbuilt dist). Both were re-run at the final commit and returned real verdicts (104 published require entry point(s) … load; 13 config(s), 621 baselined untranslated string(s), none new).

Changeset

Clause-②: no

patch for @objectstack/cli. The declaration above is measured, not assumed. Published bytes move: files[] is ["dist", …] and artifactPackagesOf appears in dist/utils/nav-contribution-groups.js and its .d.ts (positive control findNavGroupDiagnostics also hits). ⇒ skip-changeset does not apply. The diff adds no key, arm, export or registration; it removes one, and that removal reaches no consumer — the package's exports map publishes ., ./console, ./hook-body and ./package.json, and none of the three entry .d.ts files names the symbol.

In-flight fence — verified rather than trusted

PR #18675 (card #18491) is the only in-flight claim whose roster names findNavGroupDiagnostics, which is defined in the file edited here. Read at its head: its diff is one file, packages/cli/test/validate-build-gate-parity.test.ts, and its closed-ledger scan extracts bare-identifier call sites in compile.ts and validate.ts. This change touches neither command, and moves that function's name, signature (two parameters, same names, same types — only the default argument's expression changes) and export not at all. ⇒ the seat's benign judgement holds. The other claim, card #18402, faces packages/rest/src/ and is disjoint.

Acceptance notes

  • Docs-drift sweep — NOT FALSIFIED. Predicate stated before reading: hand-written docs describe the artifact package-id fallback, or show a nav_contribution_group_missing package id, such that this change makes that text wrong. Swept by symbol (artifactPackagesOf, artifactPackages, nav_contribution_group_missing) and by input shape (packages[0], packages[i], the bracketed-index spelling, Package "") over content/docs/, skills/, docs/adr/. Controls both ways: positive navigationContributions hit 6 files, negative nonsense token hit 0. One hand-written hit, content/docs/ui/setup-app.mdx, says the diagnostic names "the contributing package" generically and shows no id spelling — still true, and marginally more true, since its "the same finding" claim about the two doors was what the divergence quietly falsified. The other two hits are under content/docs/references/, auto-generated, and reference the error code rather than the id rule.
  • collectNavGroupInputs' single-package branch (no packages[]) derives its own packageId from the top-level manifest — manifest.id then manifest.name, both string-guarded, with no positional fallback. It is not the artifact package-id rule and cannot be: with no packages[] there is no index. Noted, not filed; no PR or seat is routed to that expression by this change.
  • A cross-door pin now exists on the CLI side only. The runtime half of decision(objectql): a navigationContributions[].group that names no group in the target app is silently RELOCATED to the top level — refuse, warn, or leave to the consumer? #14553 is pinned in packages/objectql/src/registry-nav-contribution-group-semantics.test.ts, and nothing there covers the empty-id identity. Noted, not filed — out of the declared face (packages/cli/src/utils/), and the new pin reads the real runtime, so the fact is guarded from one side. Would-be carrier: the next card touching that objectql suite.

Generated by Claude Code

@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 24 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 72dd95fa5a87df8990986490b4638506dd8b2d3a → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 70e9810ec17a2f7fbefad249066f3bb345a0eeec — the merge of head 6a886b8a4c6271639400cf4d4400e0ba97ab9d1b into base 72dd95fa5a87df8990986490b4638506dd8b2d3a, 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 70e9810ec17a2f7fbefad249066f3bb345a0eeec && git checkout 70e9810ec17a2f7fbefad249066f3bb345a0eeec
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 72dd95fa5a87df8990986490b4638506dd8b2d3a 6a886b8a4c6271639400cf4d4400e0ba97ab9d1b && git checkout -B drift-repro 72dd95fa5a87df8990986490b4638506dd8b2d3a && git merge --no-ff 6a886b8a4c6271639400cf4d4400e0ba97ab9d1b

node scripts/docs-audit/affected-docs.mjs --json 72dd95fa5a87df8990986490b4638506dd8b2d3a

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

@os-support-ai
os-support-ai marked this pull request as ready for review September 17, 2026 16:36
@os-support-ai
os-support-ai added this pull request to the merge queue Sep 17, 2026
Merged via the queue into main with commit 93917d1 Sep 17, 2026
42 of 43 checks passed
@os-support-ai
os-support-ai deleted the claude/issue-18490-artifact-package-id-second-copy branch September 17, 2026 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants