Skip to content

test(cli): one built-CLI prerequisite for all four bin/run.js spawners, each refusing in its own terms - #12617

Merged
os-litant merged 4 commits into
mainfrom
claude/issue-12539-built-cli-prerequisite-coverage
Aug 27, 2026
Merged

test(cli): one built-CLI prerequisite for all four bin/run.js spawners, each refusing in its own terms#12617
os-litant merged 4 commits into
mainfrom
claude/issue-12539-built-cli-prerequisite-coverage

Conversation

@os-litant

@os-litant os-litant commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Fixes #12539

What the measurement says, before what the diff does

The card claims 4 failing files on a closure-only build. ⭐ The number is right; the attribution in the dispatch order was not. Three independent derivations agree on the same four files:

  1. The repo's own CI detector. scripts/check-cli-test-child-env.mjs exports builtEntrypointSpawns(), which is what the check:cli-test-child-env gate uses to find spawns of bin/run.js. Run over all 99 files under packages/cli/test/** it names exactly four, one spawn each:

    file requireBuiltCli() on origin/main
    serve-mcp-capability-collision.e2e.test.ts ✅ private copy (test(cli): reach the built CLI in the three serve e2e spawners #12459)
    serve-mcp-stdio-answers.e2e.test.ts ✅ private copy (test(cli): reach the built CLI in the three serve e2e spawners #12459)
    serve-stdio-stdout-purity.e2e.test.ts ✅ private copy (test(cli): reach the built CLI in the three serve e2e spawners #12459)
    serve-node-env-production-default.e2e.test.ts none

    The gate's own summary line prints the same count independently: all 4 spawn(s) of the built CLI keep their child out of development/test.

  2. The throw sites. serve exited N before "Server is ready" is produced by each of those four files' own inline boot helper. ⛔ It is not produced by helpers/serve-process.ts — that helper's runServe() path spawns bin/run-dev.js through tsx, reads src/, and needs no packages/cli/dist at all.

  3. The empirical run, below.

⚠️ So the dispatch order's hypothesis that the four are the run-dev.js + tsx family is falsified: the failing set is exactly the bin/run.js family, and requireBuiltCli() is the right instrument for all four. invocation-loudness.e2e.test.ts resolves bin/run.js but only ever asserts on the string, and is correctly outside the population (the gate pins that distinction with its own case).

What changed

The premise, measured — with controls

Closure-only tree: pnpm --filter '@objectstack/cli^...' build (note ^... — dependencies excluding @objectstack/cli), then packages/cli/dist removed and its absence verified on disk.

Before this PR:

 ❯ test/serve-stdio-stdout-purity.e2e.test.ts         (1 test  | 1 skipped)
 ❯ test/serve-mcp-capability-collision.e2e.test.ts    (3 tests | 3 skipped)
 ❯ test/serve-mcp-stdio-answers.e2e.test.ts           (1 test  | 1 skipped)
 ❯ test/serve-node-env-production-default.e2e.test.ts (3 tests | 1 failed)
     × NODE_ENV unset: the localhost trusted-origin convenience gate is CLOSED

 Test Files  4 failed | 2 passed (6)

The two that passed are the controlsserve-app-runtime-hooks.e2e.test.ts and serve-no-artifact.e2e.test.ts, both runServe()/tsx files. ⇒ the failing set is the bin/run.js family and nothing else.

The mechanism, reproduced directly on the same tree:

$ env -u NODE_ENV node packages/cli/bin/run.js serve --help
 ›   Error: Command serve not found.
EXIT=2

And the fourth file's failure was a partial green — which is the whole reason its guard is file-scoped. 3 tests | 1 failed: the unset pin failed, and the two rerouted legs passed from src/, reporting green for a program the file's own header says it does not measure.

Both directions

Same tree, both legs in one run, the restore proven on disk between them.

Positive — the guard fires, on all four, each in its own words:

 Test Files  4 failed | 1 passed (5)

serve-node-env-production-default.e2e.test.ts now reports (3 tests | 3 skipped) — ⭐ all three legs refused, where before the guard it was 3 tests | 1 failed. The partial green is gone. Its refusal carries its own sentence:

Error: packages/cli is not built: .../packages/cli/dist/commands/serve.js does not exist.
Only the unset-NODE_ENV pin here resolves from dist/: unset is the one value that leaves
@oclif/core's isProd() true, so that leg globs the real dist/commands and answers "command
serve not found" on an unbuilt tree. The other two legs set NODE_ENV to development/test,
which reroutes them to src/commands — they would PASS without a built dist/, reporting green
for a program this file does not measure. So the whole file refuses, not just that leg.
CI declares the build (turbo: @objectstack/cli#test dependsOn build); a direct vitest run does not.
Run: pnpm exec turbo run build --filter=@objectstack/cli
 ❯ unbuiltCliError test/helpers/serve-process.ts:80:10

…while the three siblings still carry RUN_JS_RESOLVES_FROM_DIST verbatim. ⇒ ⛔ the bin/run.js sentence did not travel outward, and both refusals name the build command.

Negative — the guard is silent on a correctly built tree. ⚠️ The expensive half: a guard that fired here would make every green in this directory a coin flip.

 Test Files  5 passed (5)
      Tests  15 passed (15)
leg tree exit result
1 closure-only (dist/commands/serve.js absent, verified) 1 4 failed, 1 passed — every refusal names the build command
2 pnpm --filter @objectstack/cli build 0 restore proven on disk: artifact present
3 built 0 5 passed (5) · 15 passed (15) — guard silent

Scope — what is covered and what is left

Coverage is complete for the population, not partial. The bin/run.js family has exactly four members and all four are now guarded.

⚠️ The fence was re-derived, not inherited, and it has changed since dispatch: all 9 open PRs were checked against packages/cli/test/** (git diff --name-only vs each branch's merge base) and none of them touches a single file there. #12561, named as a live holder on serve-node-env-production-default.e2e.test.ts at dispatch time, is closed (PR #12569, merged); PR #12565 merged too. So nothing was left out for fencing reasons.

Deliberately not changed:

Changeset

None — skip-changeset, applied as a label and read back after the size-labeler settled (size/m, tests, skip-changeset). Every file in this diff is under packages/cli/test/**: no src/, no public surface, nothing published. AGENTS.md's rule is "add a changeset for feature work … pure bug fixes do not require a changeset", and pr-automation.yml defines the label as "this PR declares no release of its own".

⚠️ One thing this PR's own typecheck does NOT say

pnpm --filter @objectstack/cli typecheck exits 0, and that is NOT MEASURED for this diff: packages/cli/tsconfig.json is include: ["src"], so tsc --listFiles reads 0 of 1279 files under packages/cli/test/ (reverse-checked — src/commands/serve.ts scores 1 in the same output). Every file here lives in test/. pnpm check:type-check-coverage exits 0, so this is a ledgered, pre-existing state this PR does not move — but the green typecheck should not be read as covering these files.

Verdict on the inherited wip: commit (3c84e9fc5)

Adopted as-is, and extended. Judged adversarially rather than rewritten:

  • The hoist mechanics are sound. The three private copies were genuinely byte-identical, so the hoist removes a real triplication rather than inventing one.
  • ⭐ It had already made the mechanism a required parameter with no default — the single most important decision on this card, and the one this PR's fourth adopter depends on. Nothing needed changing there.
  • One prose correction, made by measurement: the claim comment called them "21-line" functions; they are 19 lines. The inherited helper's own docblock already said 19, and it is right.
  • ⚠️ What the commit did not contain — and what its wip: label meant — is the premise measurement. That is supplied above.

Generated by Claude Code

claude added 4 commits August 26, 2026 11:29
… spawner

`serve-node-env-production-default.e2e.test.ts` is the one spawner of the built
entrypoint that had no `requireBuiltCli()` preflight. It gets one -- with its
OWN mechanism string rather than the siblings' `RUN_JS_RESOLVES_FROM_DIST`.

Only this file's unset-NODE_ENV leg resolves from `dist/`; its other two legs
hand the child `development`/`test`, which flips oclif's `isProd()` and reroutes
them to `src/commands`. The siblings' sentence ("every boot below times out")
is therefore false here, and a true refusal carrying a false explanation is the
defect class #12498 / #12561 / #12563 were filed for.

The guard is file-scoped deliberately: measured on a closure-only tree, this
file reported `3 tests | 1 failed` -- the unset pin failing while the two
rerouted legs PASSED from `src/`, reporting green for a program the file's own
header says it does not measure.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

4 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot 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
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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 — 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 527e0505d8729ae022121a992b62326035953a82packageMentionDocs.

Which tree this was computed on

This run read content/docs from 114baa35577136b9d1dc00544f7ffb7c81eb5c04 — the merge of head 764847a35d575eec075ced57e90ac852366a4bec into base 527e0505d8729ae022121a992b62326035953a82, 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 114baa35577136b9d1dc00544f7ffb7c81eb5c04 && git checkout 114baa35577136b9d1dc00544f7ffb7c81eb5c04
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 527e0505d8729ae022121a992b62326035953a82 764847a35d575eec075ced57e90ac852366a4bec && git checkout -B drift-repro 527e0505d8729ae022121a992b62326035953a82 && git merge --no-ff 764847a35d575eec075ced57e90ac852366a4bec

node scripts/docs-audit/affected-docs.mjs --json 527e0505d8729ae022121a992b62326035953a82

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

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

Labels

size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate tests

Projects

None yet

2 participants