chore(gitignore): keep the cli serve NODE_ENV fixture leftover out of git - #12631
Merged
Merged
Conversation
… 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-litant
marked this pull request as ready for review
August 27, 2026 02:10
This was referenced Aug 27, 2026
This was referenced Aug 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #12583
One anchored, directory-only ignore entry plus a one-line comment naming the card. Net +2 / -0, in
.gitignoreonly. No test file touched, no fixture relocated, nothing reordered.The fixture root is deliberate and stays put
packages/cli/test/serve-node-env-production-default.e2e.test.ts:224setsFIXTURES_ROOT = HERE, and the docblock immediately above it (:215-223) gives the measured reason: the generated config does a real, staticimport { AuthPlugin } from '@objectstack/plugin-auth', which resolves only becausepackages/cli/node_modules/@objectstack/plugin-authis reachable by Node's upward walk. "A fixture rooted inos.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:mkdtempSync(call lines (imports excluded); 350 root attmpdir()directly.tmpdir()-derived (artifact-pinned-boot.e2e.test.ts:113→root,driver-turso/src/turso-authtoken-url-channel.test.ts:221→harnessDir,packages/spec/scripts/openapi-self-consistency.test.ts:172→sandbox,check-console-injection.mjs:706→tmpdir('check-console-injection')) orRUNNER_TEMP//tmp(check-console-injection.mjs:453).packages/cli/:test/init-scaffold-authoring-rules.test.ts:63packages/cli/tmp/scaffold-*.gitignore:53tmp/test/init-template-comments-self-contained.test.ts:92packages/cli/tmp/render-*.gitignore:53tmp/test/serve-no-artifact.e2e.test.ts:148packages/cli/tmp/os-no-artifact-definestack-*.gitignore:53tmp/test/serve-node-env-production-default.e2e.test.ts:436packages/cli/test/tmp-node-env-default-*So: one entry, not a set and not a class pattern. The class already has a convention — root under
packages/cli/tmp/, which the existingtmp/rule covers — and this file is the single outlier. Widening the pattern would buy nothing thattmp/does not already provide.packages/cli/tmp/also satisfies the docblock'snode_modules-ancestor requirement, so the three siblings solve the same problem with zero.gitignoresurface. 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:
Negative — nothing real is swallowed:
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:
packages/cli/test/tmp-node-env-default-probe/objectstack.config.ts.gitignore:56)packages/cli/test/serve-node-env-production-default.e2e.test.tspackages/cli/test/helpers/serve-process.tspackages/cli/test/tmp-node-env-default-README.md(a file)packages/cli/src/tmp-node-env-default-x/f.ts(elsewhere)packages/cli/test/nested/tmp-node-env-default-x/f.ts(deeper)Probe removed afterwards;
git status --porcelainclean.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:diffexit 0)..gitignorereverted to base0043c9224, 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'spackage.jsonandobjectstack.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, blobe97fcdbf0 → c2b172434), and the restore leg proven after it (git checkout HEAD -- .gitignore, blob back toe97fcdbf0,git diff HEADempty), under anEXIT INT TERMtrap with absolute paths.check:nul-bytesindependently reports6986 tracked, 0 untracked-not-ignored— a second consumer the leftover perturbs.Two card premises corrected
:388for themkdtempSynccall and:397forafterAll. On the merged ref they are:436and:445(rmSyncat:447,FIXTURES_ROOTat: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.packages/cli/distabsent, then run this one file". On the merged refbeforeAllcallsrequireBuiltCli(...)at :434, beforemkdtempSyncat :436, and it throws whendist/commands/serve.jsis 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
rmSyncinto afinallyaround 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, itsPromiseexecutor never callsreject, bothchild.kill()calls sit intry/catch, and a 10ssetTimeoutguarantees settle even ifexitnever fires.await stop(child)therefore always resolves, and afinallyaround the loop would guard an exception that cannot occur.The timeout half is bounded too: every child is already stopped by
probeOriginCheck's ownfinallyat:408-410,childrenholds at most 3 entries (one perit), andstop()early-returns for each — so theafterAllsweep 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
afterAllwholesale. No in-processfinallycan 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.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 0snode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack0 matched · 36 undetermined · 140 silentGate 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 lintis not owed by this diff — proven, not skipped⛔ Not reported as a green.
.gitignoreis not a member of eslint's lintable population, and this is read from eslint's own config rather than assumed:files:pattern ineslint.config.mjsis 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/**..gitignorematches none.eslint.config.mjsnever reads.gitignore— noincludeIgnoreFile, no@eslint/compat, no literal.gitignore(grep exit 1). So this rule cannot move any file into or out of the linted set.pnpm installed — the diff needs no dependency to verify, and both commands above are dependency-free Node scripts. A straypnpm exec eslintattempt returnedERR_MODULE_NOT_FOUNDfor@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-changesetRule 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-changesetlabel rather than a.changeset/*.md.Generated by Claude Code