Skip to content

chore(gitignore): keep the cli serve NODE_ENV fixture leftover out of git - #12631

Merged
os-litant merged 1 commit into
mainfrom
claude/issue-12583-fixture-leftover-gitignore
Aug 27, 2026
Merged

chore(gitignore): keep the cli serve NODE_ENV fixture leftover out of git#12631
os-litant merged 1 commit into
mainfrom
claude/issue-12583-fixture-leftover-gitignore

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes#12583

One anchored, directory-only ignore entry plus a one-line comment naming the card. Net +2 / -0, in .gitignore only. No test file touched, no fixture relocated, nothing reordered.

 # Temporary files
tmp/
temp/
+# Killed-run fixture leftover; its in-tree root is deliberate (objectstack#12583)
+packages/cli/test/tmp-node-env-default-*/

The fixture root is deliberate and stays put

packages/cli/test/serve-node-env-production-default.e2e.test.ts:224 sets FIXTURES_ROOT = HERE, and the docblock immediately above it (:215-223) gives the measured reason: the generated config does a real, static import { AuthPlugin } from '@objectstack/plugin-auth', which resolves only because packages/cli/node_modules/@objectstack/plugin-auth is reachable by Node's upward walk. "A fixture rooted in os.tmpdir() has no such ancestor and the import fails." The defect was that the leftover is visible to git, not that the fixture is in the wrong place.

The census — measured, not assumed

The card body asserts "This is the only file in the repo that roots a fixture this way", citing a grep for two literal variable names (join(HERE / join(FIXTURES_ROOT). That grep is narrow by construction. Re-measured across the whole tree:

  • 359mkdtempSync( call lines (imports excluded); 350 root at tmpdir() directly.
  • Of the 9 that do not, 5 are still outside the tracked tree — their base is itself tmpdir()-derived (artifact-pinned-boot.e2e.test.ts:113root, driver-turso/src/turso-authtoken-url-channel.test.ts:221harnessDir, packages/spec/scripts/openapi-self-consistency.test.ts:172sandbox, check-console-injection.mjs:706tmpdir('check-console-injection')) or RUNNER_TEMP//tmp (check-console-injection.mjs:453).
  • 4 create directories inside the tracked tree, all under packages/cli/:
sitedirectory createdcovered before this PR?
test/init-scaffold-authoring-rules.test.ts:63packages/cli/tmp/scaffold-*yes — .gitignore:53tmp/
test/init-template-comments-self-contained.test.ts:92packages/cli/tmp/render-*yes — .gitignore:53tmp/
test/serve-no-artifact.e2e.test.ts:148packages/cli/tmp/os-no-artifact-definestack-*yes — .gitignore:53tmp/
test/serve-node-env-production-default.e2e.test.ts:436packages/cli/test/tmp-node-env-default-*no

So: one entry, not a set and not a class pattern. The class already has a convention — root under packages/cli/tmp/, which the existing tmp/ rule covers — and this file is the single outlier. Widening the pattern would buy nothing that tmp/ does not already provide.

⚠️ Worth noting for whoever revisits this: packages/cli/tmp/ also satisfies the docblock's node_modules-ancestor requirement, so the three siblings solve the same problem with zero.gitignore surface. Relocating this fixture was explicitly out of scope for this card and is not done here; it is recorded as an observation only.

Both directions proven

Positive — a real probe directory carrying the same two files the leak produces:

$ git check-ignore -v packages/cli/test/tmp-node-env-default-probe
.gitignore:56:packages/cli/test/tmp-node-env-default-*/ packages/cli/test/tmp-node-env-default-probe
$ git status --porcelain
M .gitignore # probe absent — 0 lines mention tmp-node-env-default
$ git add -A --dry-run
add '.gitignore' # the exact failure mode, closed

Negative — nothing real is swallowed:

$ git ls-files packages/cli/test | git check-ignore --stdin # 100 tracked files
exit=1, 0 matched
$ git ls-files | git check-ignore --stdin # 6993 tracked files, repo-wide
exit=1, 0 matched

Reverse-verified so those zeros are informative rather than vacuous — the same pipeline fed three known-ignored paths returns exit 0 with 3 matches. It can say yes.

Boundary probes confirm the rule is as tight as it looks — anchored and directory-only:

pathverdict
packages/cli/test/tmp-node-env-default-probe/objectstack.config.tsignored (.gitignore:56)
packages/cli/test/serve-node-env-production-default.e2e.test.tsnot ignored
packages/cli/test/helpers/serve-process.tsnot ignored
packages/cli/test/tmp-node-env-default-README.md (a file)not ignored
packages/cli/src/tmp-node-env-default-x/f.ts (elsewhere)not ignored
packages/cli/test/nested/tmp-node-env-default-x/f.ts (deeper)not ignored

Probe removed afterwards; git status --porcelain clean.

Cost 2 measured directly, with its counterfactual

The card reports that a stray fixture perturbs scripts/pm/dispatch-gates.mjs. Confirmed, and larger than the card measured:

  • With the leftover present and this rule in place, the derivation is byte-identical to the clean-tree run (diff exit 0).
  • With the leftover present and .gitignore reverted to base 0043c9224, the derivation gains 18 spurious families — including the three the card named (check:override-consistency, check-changeset-fixed.mjs, check-osv-exemptions.mjs) plus 15 more (check:cli-test-child-env, check:i18n, check:published-files, check:type-check-debt, …), matched solely through the leftover's package.json and objectstack.config.ts.

Not a contradiction of the card's "three" — that was the marginal count against a different change set. Same mechanism, measured here against a change set that otherwise matches zero families. The mutation was confirmed on disk before the reading (rule occurrences 1 → 0, blob e97fcdbf0 → c2b172434), and the restore leg proven after it (git checkout HEAD -- .gitignore, blob back to e97fcdbf0, git diff HEAD empty), under an EXIT INT TERM trap with absolute paths.

check:nul-bytes independently reports 6986 tracked, 0 untracked-not-ignored — a second consumer the leftover perturbs.

Two card premises corrected

⚠️ Reported, not reconciled.

  1. Line anchors. The card body cites :388 for the mkdtempSync call and :397 for afterAll. On the merged ref they are :436 and :445 (rmSync at :447, FIXTURES_ROOT at :224). The card was filed while this file was fenced by docs(cli-test): scope the production-default header to the pin it describes #12569, which has since merged.
  2. The documented reproduction no longer produces a leftover. The card's re-check recipe is "build only the dependency closure, leave packages/cli/dist absent, then run this one file". On the merged ref beforeAll calls requireBuiltCli(...) at :434, beforemkdtempSync at :436, and it throws when dist/commands/serve.js is absent — so on that exact path the directory is never created. The defect still stands (the leak survives on any death after :436); only the recipe is stale.

The reading owed on hardening the teardown — it is a no-op, and not worth its own card

The card proposes, as its second remedy, moving rmSync into a finally around the child-teardown loop, on the stated grounds that "any throw or timeout in the child-teardown loop skips the removal entirely."

Measured: there is no reachable throw.stop() (:413-427) cannot reject on any path — it early-returns when the child has already exited, its Promise executor never calls reject, both child.kill() calls sit in try/catch, and a 10s setTimeout guarantees settle even if exit never fires. await stop(child) therefore always resolves, and a finally around the loop would guard an exception that cannot occur.

The timeout half is bounded too: every child is already stopped by probeOriginCheck's own finally at :408-410, children holds at most 3 entries (one per it), and stop() early-returns for each — so the afterAll sweep is worst-case ~30s against its declared 60s budget.

That leaves exactly one real producer: a hard kill of the vitest process (SIGTERM/SIGKILL, or a CI/container foreground cap), which skips afterAll wholesale. No in-process finally can address that — the ignore rule is the only thing that can, and it now does. Recommendation: do not file the hardening card; the proposed remedy would be a no-op and the residual path is already covered.

Verification

TREE_AT=db972a636 dirty=[] — the final commit, clean tree. Exit codes captured before any pipe; each gate quoted by its own verdict line.

checkresult
pnpm check:nul-bytes (under the shared verify lock)check-nul-bytes: OK (scanned 6986 text file(s) — ... no raw ASCII control bytes) · os-verify-lock: VERDICT command-exit 0 · held the lock 2s · waited 0s
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackexit 0 — "No check family names the given paths in its own source, and no workflow's path filter schedules one for them" · 0 matched · 36 undetermined · 140 silent

Gate derivation re-run after the final commit; its provenance line reads derived from the tree of 'objectstack-ai/objectstack' at commit db972a636, and it is identical to the pre-commit derivation.

pnpm lint is not owed by this diff — proven, not skipped

⛔ Not reported as a green. .gitignore is not a member of eslint's lintable population, and this is read from eslint's own config rather than assumed:

  1. Population: every files: pattern in eslint.config.mjs is one of **/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}, **/*.{ts,tsx,mts,cts}, packages/**/*.{ts,tsx,mts,cts}, packages/cli/src/**/*.{ts,tsx,mts,cts}, examples/** + packages/apps/**. .gitignore matches none.
  2. Invariance: eslint.config.mjs never reads .gitignore — no includeIgnoreFile, no @eslint/compat, no literal .gitignore (grep exit 1). So this rule cannot move any file into or out of the linted set.
  3. Therefore no untouched file's verdict can change. This is stronger than a run: lint cannot be affected by this diff.

⚠️ This worktree was deliberately not pnpm installed — the diff needs no dependency to verify, and both commands above are dependency-free Node scripts. A stray pnpm exec eslint attempt returned ERR_MODULE_NOT_FOUND for @typescript-eslint/parser; that is nothing measured, not a red gate, and is recorded here so it is not misread later. CI runs the farm regardless.

No changeset — skip-changeset

Rule applied: AGENTS.md:1107-1108 — "Add a changeset for feature work... Pure bug fixes do not require a changeset." This diff is a single repo-hygiene ignore rule: it publishes nothing, changes no runtime behaviour, and is invisible to every package's build output. It takes the skip-changeset label rather than a .changeset/*.md.


Generated by Claude Code

… git
`packages/cli/test/serve-node-env-production-default.e2e.test.ts` roots its
fixture inside the tracked test tree on purpose: the generated config does a
static `import { AuthPlugin } from '@objectstack/plugin-auth'`, which resolves
only because `packages/cli/node_modules` is reachable by Node's upward walk.
A fixture in `os.tmpdir()` has no such ancestor.
The root is correct; its visibility to git was not. A run killed after the
directory is created skips the `afterAll` removal, and the leftover was
matched by no ignore rule -- `*.tmp` and `tmp/` do not cover
`tmp-node-env-default-*`, so the next `git add -A` swept it into a PR.
Three sibling fixtures under `packages/cli/` already root at `packages/cli/tmp/`
and are covered by the existing `tmp/` rule; this file is the only in-tree
producer left uncovered, so this is one anchored, directory-only entry rather
than a widened pattern.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd
@os-litantos-litant added skip-changeset PR has no user-facing published change; bypasses the changeset gate and removed size/xs labels Aug 27, 2026 — with Claude
@os-litant
os-litant marked this pull request as ready for review August 27, 2026 02:10
@os-litant
os-litant added this pull request to the merge queueAug 27, 2026
Merged via the queue into main with commit c4f3263Aug 27, 2026
32 checks passed
@os-litant
os-litant deleted the claude/issue-12583-fixture-leftover-gitignore branch August 27, 2026 02:51
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants

@os-litant@claude