Skip to content

fix(cli): withhold doctor's ✓ over a tree it never examined (#10679) - #10799

Merged
os-elon merged 1 commit into
mainfrom
claude/issue-10679-doctor-unexamined-tree-checkmark
Aug 21, 2026
Merged

fix(cli): withhold doctor's ✓ over a tree it never examined (#10679)#10799
os-elon merged 1 commit into
mainfrom
claude/issue-10679-doctor-unexamined-tree-checkmark

Conversation

@os-elon

@os-elonos-elon commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Fixes#10679

The defect

findMissingTests() and findDeprecatedUsages() both walk packages/spec/srcunder the process cwd — a path that exists in this monorepo and in no application built with the framework. Both opened with the same two lines:

constspecSrcDir=path.join(cwd,'packages/spec/src');if(!fs.existsSync(specSrcDir))return[];

so "that directory is not here" and "I walked it and found nothing wrong" reached the caller as the same value, and the caller's else branch printed a clean bill of health about files doctor never opened. The command exits 0 either way, so "no problems found" and "I never looked" were byte-identical to every downstream reader.

Doctor already refuses to do this one screen down: the ADR-0120 D5e advisory's ✓ Unique scope is withheld unless ledgerReadingIsComplete() says the ledger half was read in full (#5412 / #5413 / #5644). These two checks escaped that discipline. This restores it — it is not a new convention.

The change

Three edits, all in packages/cli/src/commands/doctor.ts.

  1. Whether the tree was examined is now a fact in the return type, not an absence — the shape LocalManifestSource.list() 静默丢弃损坏的 ledger 条目 —— 已装应用在 boot 时消失、在控制台列表里缺席,且没有任何一条日志 #5413 used for the installed-package ledger. findMissingTests() / findDeprecatedUsages() return a MonorepoTreeScan, whose scanned: false arm carries the directory doctor resolved and looked for. The print site cannot reach the from that arm even by accident.

  2. An unexamined tree prints an informational skip naming the reason — triage's first admissible shape. -v adds the resolved directory. The step line ("Checking for missing test files…") moved inside the scanned arm too: announcing a scan that never started is the same overclaim one line earlier.

    The skip is deliberately not a warning. Nothing is wrong in an application that has no packages/spec/src, and routing it through printWarning would flip hasWarnings and end every healthy scaffold's report on "functional but has some warnings". Withholding a false must not manufacture a false .

  3. The adjacent ⚠ @objectstack/spec Not built probe is gated on the workspace existing (in scope on the same card). It read packages/spec/dist under the cwd with no check that the workspace it names is part of the tree, so in an application it warned about an absent package and prescribed pnpm --filter @objectstack/spec build, a command that cannot succeed there. Inside the monorepo the row is unchanged. Outside it there is no row: an application consumes @objectstack/spec from node_modules, where "built" is not a state it can be in, and that dependency stays covered by the Dependencies row and by checkSpecVersionGap().

Exit codes are untouched — 1 exactly when an error row exists, warnings never flip it, and the skip contributes neither. The fence on this card was not to argue an exit-code change into existence, and none is proposed.

One visible consequence, stated rather than buried: a stock scaffold with no other findings now ends on ✅ Environment is healthy and ready for development! instead of ⚠️ Environment is functional but has some warnings, because the warning it used to carry was about a workspace that was never there.

Reverse verification, in a real scaffold cwd

packages/create-objectstack/src/templates/blank copied verbatim into a temp directory (its packages/ directory does not exist), then the real Doctor.run() executed from source with the process cwd set to it. The two legs differ only in the contents of doctor.ts; the runner imports that file by path, so it is resolved from src/ and no rebuild sits between the legs. The fix was committed first, git checkout origin/main -- packages/cli/src/commands/doctor.ts stood the pre-fix file up, and git checkout HEAD -- … restored it — byte identity proved with git hash-object (f16540ff… both sides) and a clean git status --porcelain.

Before (pre-fix doctor.ts, blob 5b640533…):

 ✓ Dependencies Installed
⚠ @objectstack/spec Not built
✓ Git git version 2.43.0
✓ Environment files No .env* files here (node_env=production) — …
⚠ NODE_ENV Not set — this environment is being treated as production
→ Checking for missing test files...
✓ Test coverage All *.zod.ts files have matching tests
→ Scanning for @deprecated usage...
✓ Deprecations No @deprecated tags found
⚠️ Environment is functional but has some warnings. EXIT 0

After (this branch), same directory, same command:

 ✓ Dependencies Installed
✓ Git git version 2.43.0
✓ Environment files No .env* files here (node_env=production) — …
⚠ NODE_ENV Not set — this environment is being treated as production
ℹ Test coverage Skipped — no packages/spec/src in this directory (monorepo-only check)
ℹ Deprecations Skipped — no packages/spec/src in this directory (monorepo-only check)
⚠️ Environment is functional but has some warnings. EXIT 0

The two real findings in that scaffold (NODE_ENV, and a config-load warning from the uninstalled copy) survive untouched in both legs — this removes only the verdicts about a tree that was never there.

With -v, each skip names the directory it looked for:

 → Looked for /…/scaffold/packages/spec/src and found nothing to walk. This check reads the
monorepo's own spec sources, so outside that checkout it has no subject — and a ✓ here would
be a claim about files doctor never opened.

And inside the monorepo itself, nothing changed — ✓ @objectstack/spec Built, both step lines print, and the real findings (25 missing tests, the @deprecated tag in api/protocol.zod.ts:174) are reported exactly as before.

Tests

New: packages/cli/src/commands/doctor-unexamined-spec-tree.test.ts — 14 cases. Every end-to-end case is anchored on the user-app cwd where the tree is absent; the monorepo-shaped cwd appears only as the no-regression half, because a test proving the still appears when the tree was walked passes against the defect and touches nothing.

It pins: both clean bills of health withheld; two skip rows present with their reason (present, not merely absent — an operator scans the name column); no step line announcing a scan that never started; the skip rendered with and neither nor ; the resolved directory named under -v only; the Not built row and its unusable prescription gone in both flag modes; the exit contract unchanged. Then, from the monorepo-shaped cwd: both lines when the walked tree is clean, the real findings when it is not, ⚠ Not built still firing where the workspace really exists and is unbuilt, and ✓ Built once it has a dist/.

Two existing control assertions were re-adjudicated rather than re-spelled.doctor-env-provenance.test.ts and doctor-tenancy-posture-report.test.ts each pinned Environment is functional but has some warnings on a healthy control run — and held only because of the phantom Not built warning this PR removes. Their subject is the broken leg; the control's actual claim is that doctor reached its summary and did not call the environment broken. They now assert that, with a matcher accepting either non-error summary, which the broken leg still cannot produce.

Gates

Run against e34b763a0b, the final commit on this branch, with the workspace closure built. Derived with node scripts/pm/dispatch-gates.mjs from the real change set rather than from a recalled list, and quoted by each gate's own verdict line:

  • pnpm --filter @objectstack/cli exec vitest runTest Files 140 passed (140) · Tests 1561 passed (1561)
  • pnpm --filter @objectstack/cli typechecktsc --noEmit, exit 0
  • check-nul-bytes: OK (scanned 6239 text file(s) … no raw ASCII control bytes)
  • check-cross-package-test-inputsOK: 13 package(s) read outside themselves, all declared
  • check-test-source-alias OK — 72 packages with tests scanned
  • check-type-source-resolution OK — 76 packages with a tsconfig.json scanned
  • check-type-check-coverage: OK — 64/77 workspace packages type-checked … 13 in the DEBT ledger
  • check-engine-double-contract: OK — 371 pinned, 133 in the DEBT ledger, 2 exempt
  • check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 264.3s, 1912 raw tsc error(s) total, none above its recorded number — the ratchet half, run after turbo run build --filter=./packages/* --filter=./packages/*/* so it measured rather than refused
  • check:where-matcher, check:query-options-erasure, check:slot-lookup, check:objectui-changeset, check:changeset-gate-self-tests, scripts/docs-audit/check-affected-docs.mjs — all exit 0

The re-measure prints one standing surplus — @objectstack/plugin-auth records 109 and now measures 97 — which predates this branch and belongs to no file in this diff, so it is reported rather than lowered here.

Scope

#10680 is deliberately untouched. It edits the same file but is a different defect shape (a wrong prescribed codemod command), and triage's default was serial — this card first. The prescription line it owns is not in this diff.

Out-of-scope finding filed, unassigned: #10798 — three texts on the cli.doctor-health-report checklist item (its EXPECTED-FAIL clause, the row enumeration naming @objectstack/spec, and the step that captures the two lines verbatim) go stale once this merges. Filed rather than folded in: docs/qa/platform-checklist/areas/cli.json is the hot shared file of the #10663 lane, a clause edit owes a revision bump plus a history entry, and this repo already treats checklist-text corrections as their own card. Nothing is red today — check:platform-checklist is structural and never judges whether a clause is still true.


Generated by Claude Code

`findMissingTests()` and `findDeprecatedUsages()` both walk
`<cwd>/packages/spec/src`, a path present in this monorepo and in no
application. Both returned `[]` for "that directory is not here" — the same
value they return for "I walked it and found nothing wrong" — so every stock
scaffold printed `✓ Test coverage` and `✓ Deprecations` about files doctor
never opened, exiting 0 either way.
Whether the tree was examined is now a fact in the return type
(`MonorepoTreeScan`), the shape #5413 used for the installed-package ledger,
so the print site cannot reach the `✓` from the unexamined arm. An unexamined
tree prints an informational skip naming the reason, with the resolved
directory under `--verbose`. It is deliberately not a warning: withholding a
false ✓ must not manufacture a false ⚠.
The adjacent `⚠ @objectstack/spec Not built` probe is gated on the workspace
existing — outside the monorepo it warned about an absent package and
prescribed a command that cannot succeed there. Inside it, unchanged.
Two control assertions that pinned `Environment is functional but has some
warnings` held only because of that phantom warning; they now assert the claim
they actually make (doctor reached its summary and did not call the
environment broken).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

15 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/indexing.mdx(via os doctor (command))
  • content/docs/deployment/cli.mdx(via os doctor (command))
  • content/docs/deployment/self-hosting.mdx(via package.json (literal))
  • content/docs/deployment/tenancy-modes.mdx(via package.json (literal))
  • content/docs/deployment/troubleshooting.mdx(via package.json (literal))
  • content/docs/getting-started/examples.mdx(via package.json (literal))
  • content/docs/getting-started/your-first-project.mdx(via package.json (literal))
  • content/docs/plugins/development.mdx(via package.json (literal))
  • content/docs/plugins/index.mdx(via package.json (literal))
  • content/docs/protocol/backward-compatibility.mdx(via os doctor (command))
  • content/docs/protocol/kernel/config-resolution.mdx(via os doctor (command))
  • content/docs/protocol/kernel/http-protocol.mdx(via os doctor (command))
  • content/docs/protocol/kernel/index.mdx(via package.json (literal))
  • content/docs/protocol/kernel/plugin-spec.mdx(via package.json (literal))
  • content/docs/protocol/objectql/schema.mdx(via package.json (literal))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx(via os doctor (command))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

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 112a8c6731ba7ffac424cdcfd3b9cf2edf800748packageMentionDocs.

Which tree this was computed on

This run read content/docs from 906247bba51e41436ee3f16009a58dd9d46fac9c — the merge of head e34b763a0b9ecebad936f3e5b366be7ae7224d93 into base 112a8c6731ba7ffac424cdcfd3b9cf2edf800748, 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 906247bba51e41436ee3f16009a58dd9d46fac9c && git checkout 906247bba51e41436ee3f16009a58dd9d46fac9c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 112a8c6731ba7ffac424cdcfd3b9cf2edf800748 e34b763a0b9ecebad936f3e5b366be7ae7224d93 && git checkout -B drift-repro 112a8c6731ba7ffac424cdcfd3b9cf2edf800748 && git merge --no-ff e34b763a0b9ecebad936f3e5b366be7ae7224d93
node scripts/docs-audit/affected-docs.mjs --json 112a8c6731ba7ffac424cdcfd3b9cf2edf800748

⚠️ 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 112a8c6731ba7ffac424cdcfd3b9cf2edf800748 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

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

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

os doctor prints ✓ Test coverage / ✓ Deprecations about a tree it never examined (in any user app)

2 participants

@os-elon@claude