From f4c7a6764d7d2654b666226fa3a0415e5daaa657 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 15:20:38 +0000 Subject: [PATCH] chore(cli,pm): root the NODE_ENV serve fixture at packages/cli/tmp/, with the hazard pinned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three parts, indivisible by the ruling: the outlier fixture's root moves from packages/cli/test/ to packages/cli/tmp/ where its three siblings already sit; the bespoke .gitignore entry written for the old root is deleted; and the fixture's "why this root" docblock is rewritten so the move does not land under a rationale that no longer explains it. The docblock now separates the two reasons that were previously conflated. The static-import constraint is real and rules out os.tmpdir(), but measured, it does NOT distinguish the two in-tree roots — packages/cli/test/ and packages/cli/tmp/ have the same node_modules ancestor. The root choice is a convention (three of four fixtures, zero bespoke ignore surface), not a technical necessity. dispatch-gates' self-test gains the property that makes the move worth making: a leftover fixture at an uncovered in-tree root reaches the derived change set and names gate families of its own, while one under packages/cli/tmp/ is invisible to the same derivation in the same repo. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd --- .gitignore | 2 - ...ve-node-env-production-default.e2e.test.ts | 49 ++++++++++++--- scripts/pm/dispatch-gates.mjs | 61 +++++++++++++++++++ 3 files changed, 101 insertions(+), 11 deletions(-) diff --git a/.gitignore b/.gitignore index e97fcdbf06..c2b172434f 100644 --- a/.gitignore +++ b/.gitignore @@ -52,8 +52,6 @@ coverage/ # Temporary files tmp/ temp/ -# Killed-run fixture leftover; its in-tree root is deliberate (objectstack#12583) -packages/cli/test/tmp-node-env-default-*/ # Package builds packages/*/dist/ diff --git a/packages/cli/test/serve-node-env-production-default.e2e.test.ts b/packages/cli/test/serve-node-env-production-default.e2e.test.ts index 0e70d0fcd4..042f0de7b6 100644 --- a/packages/cli/test/serve-node-env-production-default.e2e.test.ts +++ b/packages/cli/test/serve-node-env-production-default.e2e.test.ts @@ -163,7 +163,7 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { spawn, type ChildProcessByStdio } from 'node:child_process'; -import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { join, resolve } from 'node:path'; import type { Readable } from 'node:stream'; import { fileURLToPath } from 'node:url'; @@ -214,15 +214,45 @@ const UNSET_LEG_MEASURES_THE_BUILT_DIST = 'refuses, not just that leg.'; /** - * The fixture's parent directory sits INSIDE `packages/cli/test/`, not the - * system tmpdir: the config below does a real, static - * `import { AuthPlugin } from '@objectstack/plugin-auth'`, and that only - * resolves because `packages/cli/node_modules/@objectstack/plugin-auth` - * (a real dependency of this package) is reachable by Node's ordinary - * upward `node_modules` walk from wherever the config file lives. A fixture - * rooted in `os.tmpdir()` has no such ancestor and the import fails. + * WHY THE FIXTURE IS ROOTED AT `packages/cli/tmp/` — two independent reasons, + * and only ONE of them is technical. They are written out separately because + * collapsing them is the defect this root was moved to end. + * + * 1. NOT the system tmpdir. This half IS technical and it is measured: the + * config below does a real, static + * `import { AuthPlugin } from '@objectstack/plugin-auth'`, which resolves + * only because `packages/cli/node_modules/@objectstack/plugin-auth` (a real + * dependency of this package) is reachable by Node's ordinary upward + * `node_modules` walk from wherever the config file lives. A fixture rooted + * in `os.tmpdir()` has no such ancestor and the import fails. + * + * 2. `tmp/` rather than `test/`. This half is a CONVENTION, and it does NOT + * follow from (1). ⛔ Do not re-derive it from the import constraint: this + * package's `test/` directory has exactly the SAME `node_modules` ancestor + * that its `tmp/` directory has, so (1) rules out `os.tmpdir()` and says + * nothing whatever about the choice between the two in-tree roots. Writing + * (1) as "so it must live in `tmp/`" would just swap one false reason for + * another. The choice is settled by convention instead: of the four fixtures + * this package creates inside the tracked tree, three already root at + * `packages/cli/tmp/` (`init-scaffold-authoring-rules.test.ts`, + * `init-template-comments-self-contained.test.ts`, + * `serve-no-artifact.e2e.test.ts`), and that root costs ZERO bespoke ignore + * surface — the repo-wide `tmp/` rule in `.gitignore` already covers it, + * where this file's former in-`test/` root needed an ignore entry written + * for it alone. Maintainer ruling, 2026-08-27, adopting Option A. + * + * ⚠️ The ignore coverage is the load-bearing half of (2), not tidiness. A + * killed run leaves this directory behind, and a leftover in the tracked tree + * with NO ignore rule is not inert: `scripts/pm/dispatch-gates.mjs` derives + * every dispatch's gate list from the working tree, untracked files included, + * so an unignored leftover joins that change set and inflates it — measured on + * this tree, a two-file leftover adds 20 gate families that the branch's real + * diff does not implicate, corrupting other seats' dispatch decisions for as + * long as it sits there. Both directions of that property are pinned in + * `dispatch-gates.mjs`'s own `--self-test`; ⛔ a fifth fixture that picks a new + * in-tree root inherits this obligation and does not inherit the pin. */ -const FIXTURES_ROOT = HERE; +const FIXTURES_ROOT = resolve(HERE, '../tmp'); function configFor(port: number): string { return ` @@ -434,6 +464,7 @@ describe('#11113: os serve defaults NODE_ENV to production when unset', () => { // from `dist/`, and the two rerouted legs must not report green without it. requireBuiltCli(UNSET_LEG_MEASURES_THE_BUILT_DIST); + mkdirSync(FIXTURES_ROOT, { recursive: true }); dir = mkdtempSync(join(FIXTURES_ROOT, 'tmp-node-env-default-')); writeFileSync( join(dir, 'package.json'), diff --git a/scripts/pm/dispatch-gates.mjs b/scripts/pm/dispatch-gates.mjs index a3b3b8052c..f6f7ed0389 100644 --- a/scripts/pm/dispatch-gates.mjs +++ b/scripts/pm/dispatch-gates.mjs @@ -7753,6 +7753,67 @@ function selfTest() { missingBaseErr = err; } t('an unresolvable base ref is refused on its own terms', !!missingBaseErr && /does not resolve/.test(missingBaseErr.message)); + + // ── A leftover in-tree test fixture must not reach the change set (#12632) + // + // The derivation reads untracked files on purpose, so `.gitignore` is the + // only thing standing between a killed test run's leftover fixture and + // every seat's gate list. `packages/cli` creates four fixtures inside the + // tracked tree; the one that used to root in `packages/cli/test/` needed an + // ignore entry written for it alone, and now roots at `packages/cli/tmp/` + // with the other three, under the repo-wide `tmp/` rule. + // + // The CONTROL is the load-bearing half and it runs the SAME derivation on + // the SAME repo: asserting only that the covered path is absent would pass + // just as happily against a `.gitignore` that ignores the whole tree, or + // against a fixture that never planted anything. So an UNCOVERED in-tree + // path is planted alongside it and has to come back VISIBLE, and it has to + // come back carrying gate families — a leftover that reached the change set + // is not inert. Measured on the tree at the time of writing: a two-file + // leftover (the `package.json` and `objectstack.config.ts` that fixture + // writes) named 20 families the branch's own diff does not implicate. + // The count is not asserted — the family inventory grows same-day, which is + // this whole tool's premise — only that it is non-empty. + // + // The uncovered path is deliberately the fixture's FORMER root, so this + // control also fails if the bespoke ignore entry is ever restored: a rule + // covering a root nothing uses would make the control silently green and + // take the pin with it. + // + // The repo's REAL `.gitignore` is copied in rather than an excerpt written + // here: an excerpt would pin the excerpt. + const ignoreRepo = join(gitTmp, 'ignore-coverage'); + mkdirSync(ignoreRepo, { recursive: true }); + g(['init', '--initial-branch=main', '.'], ignoreRepo); + write(ignoreRepo, '.gitignore', readFileSync(join(ROOT, '.gitignore'), 'utf8')); + g(['add', '-A'], ignoreRepo); + g(['commit', '-m', 'the real ignore rules'], ignoreRepo); + g(['update-ref', 'refs/remotes/origin/main', g(['rev-parse', 'main'], ignoreRepo)], ignoreRepo); + + const leftoverAtCoveredRoot = 'packages/cli/tmp/tmp-node-env-default-selftest/objectstack.config.ts'; + const leftoverAtUncoveredRoot = 'packages/cli/test/tmp-node-env-default-selftest/objectstack.config.ts'; + for (const rel of [leftoverAtCoveredRoot, leftoverAtUncoveredRoot]) { + write(ignoreRepo, rel, "import { AuthPlugin } from '@objectstack/plugin-auth';\n"); + write(ignoreRepo, join(dirname(rel), 'package.json'), '{ "private": true, "type": "module" }\n'); + } + const leftovers = changedPathsFromGit({ cwd: ignoreRepo }); + + t( + 'the CONTROL reproduces the hazard: a leftover fixture at an UNCOVERED in-tree root does reach the change set', + leftovers.paths.includes(leftoverAtUncoveredRoot), + ); + t( + 'and reaching it is not free — the uncovered leftover names gate families of its own', + [...discoverFamilies().byCheck.values()].some((e) => classifyEntry(e, [leftoverAtUncoveredRoot]).verdict === 'matched'), + ); + t( + 'a leftover fixture at packages/cli/tmp/ is invisible to the derivation, under the same rules in the same repo', + !leftovers.paths.includes(leftoverAtCoveredRoot), + ); + t( + 'and it is the repo-wide tmp/ rule doing it, with no bespoke entry for the fixture former root', + !readFileSync(join(ROOT, '.gitignore'), 'utf8').includes('packages/cli/test/tmp-node-env-default'), + ); } finally { rmSync(gitTmp, { recursive: true, force: true }); }