Skip to content

Commit a7ea328

Browse files
Elon Muskclaude
andauthored
fix(cli): withhold doctor's ✓ over a tree it never examined (#10679) (#10799)
`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). Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9ca303e commit a7ea328

5 files changed

Lines changed: 589 additions & 39 deletions

File tree

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
`os doctor` no longer prints `✓ Test coverage` / `✓ Deprecations` about a tree it
6+
never examined, and no longer warns `@objectstack/spec Not built` about a
7+
workspace that is not part of the tree (#10679).
8+
9+
`findMissingTests()` and `findDeprecatedUsages()` both walk
10+
`<cwd>/packages/spec/src` — a path that exists in this monorepo and in no
11+
application built with the framework. Both answered "that directory is not here"
12+
with the same value they return for "I walked it and found nothing wrong" (an
13+
empty array), so in a stock `create-objectstack -t blank` scaffold every run
14+
printed, verbatim:
15+
16+
```
17+
✓ Test coverage All *.zod.ts files have matching tests
18+
✓ Deprecations No @deprecated tags found
19+
```
20+
21+
about files doctor never opened. The command exits 0 either way, so "no problems
22+
found" and "I never looked" were byte-identical to every downstream reader.
23+
24+
Doctor already refuses to do this one screen down: the ADR-0120 D5e advisory's
25+
`✓ Unique scope` is withheld unless `ledgerReadingIsComplete()` says the ledger
26+
half was read in full. These two checks escaped that discipline; this restores
27+
it, in the same shape #5413 used for the ledger — whether the tree was examined
28+
is now a fact in the return type rather than an absence, so the print site
29+
cannot reach the `` from the unexamined arm. Where the tree is absent doctor
30+
prints an informational, named-reason skip instead:
31+
32+
```
33+
ℹ Test coverage Skipped — no packages/spec/src in this directory (monorepo-only check)
34+
ℹ Deprecations Skipped — no packages/spec/src in this directory (monorepo-only check)
35+
```
36+
37+
`--verbose` adds the resolved directory it looked for. The skip is deliberately
38+
not a warning: nothing is wrong in an application that has no
39+
`packages/spec/src`, and withholding a false `` must not manufacture a false
40+
``.
41+
42+
The adjacent `⚠ @objectstack/spec Not built` probe read `<cwd>/packages/spec/dist`
43+
with no check that the workspace it names exists, so in an application it warned
44+
about an absent package and prescribed `pnpm --filter @objectstack/spec build`, a
45+
command that cannot succeed there. It is now gated on `packages/spec/package.json`
46+
being present. Inside the monorepo the row is unchanged; outside it there is no
47+
row, and an application's spec dependency stays covered by the `Dependencies`
48+
check and by the spec-version-gap advisory.
49+
50+
Exit codes are untouched — 1 exactly when an error row exists, warnings never
51+
flip it. One visible consequence: a stock scaffold with no other findings now
52+
ends on `✅ Environment is healthy and ready for development!` instead of
53+
`⚠️ Environment is functional but has some warnings`, because the warning it
54+
used to carry was about a workspace that was never there.

packages/cli/src/commands/doctor-env-provenance.test.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,18 @@ describe('os doctor, end to end, against a posture that only exists in .env', ()
447447
const healthy = await runDoctor();
448448

449449
expect(healthy.exitCode).toBeUndefined();
450-
expect(healthy.out).toContain('Environment is functional');
450+
// #10679 — this used to read `toContain('Environment is functional')`, and
451+
// it passed for a reason that had nothing to do with #5387: the temp cwd
452+
// has no `packages/spec`, and doctor warned `@objectstack/spec Not built`
453+
// about that absent workspace on every run. Removing that phantom warning
454+
// leaves this cwd with no findings at all, so the summary is now the
455+
// healthy one. What the control actually claims — doctor reached its
456+
// summary and did NOT refuse to call this environment usable — is what the
457+
// matcher says instead, and it still cannot pass for the broken leg below
458+
// (that one prints `Some critical issues found`).
459+
expect(healthy.out).toMatch(
460+
/Environment is (healthy and ready for development|functional but has some warnings)/,
461+
);
451462
expect(healthy.out).not.toContain('Tenancy posture');
452463
// The report says what it read even when everything is fine — that is the
453464
// "not a silent merge" half, and it is only observable on a healthy run.

packages/cli/src/commands/doctor-tenancy-posture-report.test.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,8 +295,19 @@ describe('os doctor reports an unrecognized posture and exits non-zero', () => {
295295

296296
// Doctor completes normally. This is the sentence #5382 quoted, and here it
297297
// is CORRECT: this environment really can start.
298+
//
299+
// #10679 — the matcher accepts either non-error summary. The control used
300+
// to pin `Environment is functional but has some warnings` literally, and
301+
// it held only because the temp cwd has no `packages/spec` and doctor
302+
// warned `@objectstack/spec Not built` about that absent workspace every
303+
// time. With that phantom warning gone this cwd has no findings, so the
304+
// summary is the healthy one. Either sentence proves the control's actual
305+
// claim; neither can be produced by the broken leg below, which prints
306+
// `Some critical issues found` and exits 1.
298307
expect(healthy.exitCode).toBeUndefined();
299-
expect(healthy.out).toContain('Environment is functional');
308+
expect(healthy.out).toMatch(
309+
/Environment is (healthy and ready for development|functional but has some warnings)/,
310+
);
300311
expect(healthy.out).not.toContain('Tenancy posture');
301312

302313
// ── The case: one character changed ──────────────────────────────────

0 commit comments

Comments
 (0)