fix(cli): evaluate os lint's duplicate-name advisory per package - #17952
fix(cli): evaluate os lint's duplicate-name advisory per package#17952claude[bot] wants to merge 2 commits into
os lint's duplicate-name advisory per package#17952Conversation
`os lint`'s `naming/namespace-prefix` advisory read ONE flattened array per collection key with no package boundary, so two packages that each legitimately declare `home` were reported as one package declaring it twice — with a prescription to rename a name that was already correct, using the OTHER package's namespace prefix, under a closing sentence saying distinct packages may reuse a name freely. ADR-0130 D4/D5 already ruled that artifacts register per package, and `compile.ts` step 3b-ii runs the author-time rule table once per package through `artifactPackages` + `packageBodyAsStack`. Those two move to `utils/artifact-packages.ts` and `lintConfig` calls the same pair, so the advisory is evaluated in the registration unit instead of over the union. ⛔ Not a narrowing: every finding the advisory exists to raise is still raised, on the package that owns it. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
…ckage Two packages each declaring the same bare name must not warn; the same two names inside ONE package must still warn. Only the pair is a test — a run of the false-positive side alone is satisfied by an advisory that was switched off, which is the outcome `compile.ts`' fence names. Both ADR-0130 D4 stack shapes are exercised, and the `registryKey` composite key (`<objectName>:<name>`, object-less falling back to the canonical global key) is pinned cross-package and same-package so the package axis cannot swallow it. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
📓 Docs Drift CheckThis PR changes 1 package(s): 32 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 8 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 24 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 4efc3373568bbd05f617467c9387c43420dddd4d && git checkout 4efc3373568bbd05f617467c9387c43420dddd4d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin bdb247d9ec0ed1b652cee6a418a2e8d87da060b7 7ccfdd189ade791818b9f23aa3595e9f64457f84 && git checkout -B drift-repro bdb247d9ec0ed1b652cee6a418a2e8d87da060b7 && git merge --no-ff 7ccfdd189ade791818b9f23aa3595e9f64457f84
node scripts/docs-audit/affected-docs.mjs --json bdb247d9ec0ed1b652cee6a418a2e8d87da060b7
|
Contract reviewPR #17952 · card #17821 · head
What this review measured itself vs. relayed, up front:
① The clause-② condition this seat declared — checked, and it did NOT materialiseThe claim comment declared ⇒ The new module is internal to two command modules. ② The ruled shape was ported, ⛔ not re-invented — and the original site SHRANK⭐ ③ The fence is quoted where it binds, and nothing is silencedTriage's hard fence was
Measured across all 591 added lines: ④ ⭐⭐ The ablation the order did not ask for — the dev built the instrument that would catch a silencingTwo ablations, each with the mutation proved to have reached disk by blob hash (⛔ not
⭐ The order required the fix not to silence the advisory. The dev went one further and pinned the shape of silencing so a future change cannot do it quietly. Restore verified by hash on both legs ( ⑤ The two-direction acceptance, both stack shapes, through the real binaryRelayed; ⭐ And the reading that matters most, which is not a count: the surviving true positive's prescription changed from
⑥ The boundary is stated in the code, ⛔ not left impliedWith two or more packages, a top-level declaration that no ⑦ Fences and declared gapsNOT MEASURED, declared rather than smoothed: Three out-of-scope findings, each with a prospective handler named — including one honest "none" — which is the discipline that keeps a findings list from becoming a wish list. The first ( Verdict and what remainsPASS on substance.
Generated by Claude Code |
Fixes #17821
os lint'snaming/namespace-prefixadvisory read ONE flattened array per collection key —lint.ts:428,const items: any[] = Array.isArray(stack[key]) ? stack[key] : [];— with no package boundary, while its own comment atlint.ts:362asserted "We only see one package's config here". On a composed multi-package project, which is exactly the shapecomposeStacks(…, { manifest: 'preserve' })emits, that array carries every package's items. So two packages that each legitimately declarehomewere reported as one package declaring it twice, prescribing a rename of a name that was already correct — with the OTHER package's namespace as the suggested prefix — under a closing sentence saying distinct packages may reuse a name freely.What this does
ADR-0130 D4/D5 already ruled that artifacts register per package, and
compile.tsstep 3b-ii runs the author-time rule table once per package throughartifactPackages+packageBodyAsStack. This ports that ruled shape into the entry that missed it; it does not invent a new one.Those two functions move verbatim to
packages/cli/src/utils/artifact-packages.ts, andlintConfigcalls the same pair. The alternative to a shared module was importing one oclif command from another — which would pullcompile.ts' whole module graph, the lowerer and the docs sweep included, into everyos lintinvocation — or a second copy of the package-id rule, free to drift from the one the runtime registers under.⛔ Not a narrowing, an opt-out, or a skip.
compile.ts:118's fence — "This is NOT 'skip the site per package' — that would silence the gate" — binds here too. Every finding the advisory exists to raise is still raised, on the package that owns it. What changed is that a name is judged against its OWN package's declarations instead of against the union of every package's. The defect was missing attribution, never the signal.Fewer than two packages keeps today's read verbatim: a stack with no
packages[]is one package by definition, and with exactly one the flattened top level carries that package's collections and nothing else, in its own order. With two or more, the flattened top level mixes packages, so the path is written whole (packages[1].manifest.apps[1].name) and resolves in BOTH ADR-0130 D4 shapes — a bareapps[1]would, on the additive shape, point at a different item than the one being reported.Measured — both directions, both stack shapes
Through the real binary (
node packages/cli/bin/run.js lint),os lintexit 0 on every row. Before:origin/main84e6b05b6. After: this branch at7ccfdd189. Count isnaming/namespace-prefixwarnings.home(the card's repro)home(the card's repro)packages[]-onlyhomeapps inside ONE packagehomeapps inside ONE packagepackages[]-onlylog_callaction per package, sameobjectNamelog_callactions on oneobjectName, one package⛔ The true-positive rows are not decoration: a run of the false-positive side alone is satisfied by an advisory that was simply switched off, and that is the outcome the fence above names.
The prescription is fixed in the same motion. The surviving true positive used to read
rename one, e.g. "alpha_portal"—alphabeing the namespace of the package that does not own the reported item — and now reads"beta_portal", becausensis read off the same package stack the items came from.Both shapes are live today: #17528 landed (
8305ad6df) and folds an option-B stack before the hand-written checks run, so the card's "after #17528" column is the present tense and repairing only the additive shape would have been half a fix.Ablations
Both legs mutate
packages/cli/src/commands/lint.tson disk, prove the bytes changed (git hash-objectbefore vs after, and an anchor occurrence count), run the suites, then restore withgit checkout HEAD -- …and prove the restore (git diff HEADempty, blob hash equal toHEAD:packages/cli/src/commands/lint.ts). The mutated module is imported by the tests through a relative specifier intosrc/, so nodist/is on this consumption path.packageEntries.length > 1becomes> 999, i.e. always the union leg): 12 of the 15 new pins go red, and BOTH pre-existing files —lint-namespace-prefix.test.tsandlint-handwritten-checks-package-fold.test.ts— stay green. That green is why the new file had to exist: the old pins are all single-package and cannot see this axis at all.return issues;): every TRUE-POSITIVE row goes red while every "THE CARD'S REPRO" row stays GREEN. That is the signature of the outcome the fence forbids, and the pair is what detects it.Verification
pnpm --filter '@objectstack/cli^...' build— exit 0.pnpm --filter @objectstack/cli typecheck— exit 0 (includescheck:test-typecheck).pnpm --filter @objectstack/cli exec vitest run --project unit— 205 files / 2950 tests passed. Theintegrationtier is declared to CI: the diff touches no spawn entry, notest/helpers/serve-process.ts, and no driver/kernel boot path.pnpm lint(repo-wide,eslint . --no-inline-config) — exit 0, 96s. Not narrowed.node scripts/pm/dispatch-gates.mjsfrom this tree: 62 derived, 62 run, 0 NOT MEASURED, 0 UNRUN, every one recording exit 0 (reconciled with--ran).origin/mainhas moved tobd25e897dsince this branch's base, andscripts/check-cli-test-child-env.mjschanged there — the local run read this tree's copy. CI runs the current one.Clause-②: no
packages/cli/src/utils/artifact-packages.tsis an internal util. It is re-exported by none of the package's three published entry points (.,./console,./hook-body), andpnpm check:published-filesconfirms no resolvable surface moved.Acceptance notes
Observations found while working here, none filed, each with the reason:
packages/cli/src/utils/nav-contribution-groups.tsexportsartifactPackagesOf, a second implementation of the same package-id rule (manifest.id, thenname, then the positional spelling) whose own docblock says it "Mirrorscompile.ts'artifactPackagesid rule". Folding it onto the module this PR adds is mechanical, but it is a different defect class from this card and the file carries its own pins — noted, not filed. Prospective handler: the next PR innav-contribution-groups.ts.packages[]entry carries is outside every registration unit and is judged by neither leg.composeStackscannot emit that shape (its flattened top level is exactly the union of the packages it composed), so it is a hand-authored-only shape. Stated in the code comment rather than filed; prospective handler: none.lintConfigreports an index into the folded stack, which on apackages[]-only project is a synthetic coordinate that resolves nowhere in the author's file. That is pre-existing (it arrived with the os lint's hand-written checks andscoreMetadatastill read the top level alone: a packages[]-only project gets✓ All checks passedand a rubric computed over nothing (the half #17069 did not scope) #17528 fold) and untouched here; this advisory is now the one rule whose multi-package path resolves. Prospective handler: none — noted, not filed.Generated by Claude Code