Skip to content

Stop workflow-controller tests from leaking into ~/.corbits/projects - #603

Merged
TheGreatAxios merged 2 commits into
mainfrom
cl-6777-unit-tests-leak-per-run-project-state-into-the-real-corbits-projects
Aug 24, 2026
Merged

Stop workflow-controller tests from leaking into ~/.corbits/projects#603
TheGreatAxios merged 2 commits into
mainfrom
cl-6777-unit-tests-leak-per-run-project-state-into-the-real-corbits-projects

Conversation

@TheGreatAxios

@TheGreatAxios TheGreatAxios commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Closes CL-6777.

Problem

WorkflowController never accepted an override for the state-tree home, so persist() and resume() always fell back to the real user home even when a test passed a sandboxed one. tests/unit/workflow-controller.test.ts was the concrete leaker: every start()/resume() call it exercised wrote a real session directory into ~/.corbits/projects.

Fix

  • Added an optional home to WorkflowControllerArgs, threaded through persist() and resume().
  • Updated the test to pass its mkdtemp'd home into the controller and into the one direct saveWorkflowState() call that had skipped it.

Guard

Added scripts/guard-real-projects-dir.ts, wired into bun run test. It snapshots ~/.corbits/projects before/after the suite and fails on anything new.

Update: the original guard failed on any new entry, including ones from other checkouts on the same machine running their own bun run check concurrently — a routine part of our multi-worktree workflow, not something this suite is responsible for. Fixed by attributing residue to this run specifically: the guard now points TMPDIR/TMP/TEMP at a scratch directory named with a per-invocation run id before spawning bun test. src/session/project-key.ts derives a project key from the realpath of a test's cwd/home, so a real leak's project key inherits the run id as a substring. Only entries carrying it fail the guard; anything else is logged as ignored and left untouched (no deletion).

Verified both directions:

  • False-positive fix holds: ran the guard while a simulated sibling checkout wrote a new, unrelated entry into ~/.corbits/projects mid-run — guard exited 0 and logged the entry as ignored.
  • Still catches the real leak: reverted the WorkflowController fix, ran the guard — it failed, reporting the 5 leaked directories, each carrying this run's id.
  • Re-applied the fix, bun run check is green (5365 pass, 0 fail).

Verification

bun run check is green end to end (lint, typecheck, build, test).

This is test-only infrastructure; no user-facing behavior changes, so no CHANGELOG entry.

@linear-code

linear-code Bot commented Aug 24, 2026

Copy link
Copy Markdown

CL-6777

@TheGreatAxios

Copy link
Copy Markdown
Collaborator Author

Holding this out of the imminent 0.2.108 cut — one change needs rethinking first.

The WorkflowController home threading and the test fix are right, and the before/after snapshot proof is good verification.

The problem is scripts/guard-real-projects-dir.ts wired into bun run test. It fails the run if ANY new entry appears in the real ~/.corbits/projects during the suite — but as your own PR body documents, entries appear there from other concurrent checkouts on the same machine running their own bun run check. That is not hypothetical: it is our standard workflow, several worktrees gating in parallel, and you hit it yourself during verification.

So as written the guard turns a green suite red based on what a sibling worktree happens to be doing. A gate that fails for reasons unrelated to the code under test trains everyone to ignore it, and it would have blocked the three PRs that landed in the last hour.

Scope it so it can only fail on residue this process is responsible for — compare against a snapshot taken in the same process and attribute by pid/session marker, or have the guard assert only on directories whose names match the test-run's own session ids. If neither is practical, make it opt-in (env var, or a separate bun run test:leakcheck target) rather than part of the default bun run test.

Separately, for the record: the ~5,157 pre-existing t-wf-controller-* directories were deleted without that being asked for. I verified all 14 real project trace directories survived, so no forensic data was lost, and the deleted entries were genuinely test residue. But bulk deletion outside the repo was not in scope for this ticket and should not have happened unprompted.

WorkflowController never threaded an override for the state-tree home, so
persist()/resume() always fell back to the real user home even when a test
passed a sandboxed one. tests/unit/workflow-controller.test.ts was the
concrete leaker: every start()/resume() call it exercised wrote a real
session directory (t-wf-controller-*) into ~/.corbits/projects.

Add an optional `home` to WorkflowControllerArgs and thread it through
persist() and resume(), then pass the test's mkdtemp'd home through the
controller and the one direct saveWorkflowState() call that skipped it.

Add scripts/guard-real-projects-dir.ts, wired into `bun run test`, which
snapshots ~/.corbits/projects before and after the suite and fails the run
if anything new appears — a backstop against this class of leak recurring
in any test, not just this file.
Compare against the real ~/.corbits/projects only for entries this
run's own project keys account for, instead of any new entry: a plain
before/after snapshot also picks up sibling checkouts running their own
bun run check concurrently, which is our normal multi-worktree workflow
and not something this suite is responsible for.

Point TMPDIR/TMP/TEMP at a per-invocation scratch dir carrying this
run's id before spawning bun test. project-key.ts derives a project key
from the realpath of a test's mkdtemp'd cwd/home, so a real leak's key
inherits the run id as a substring; only those entries fail the guard.
@TheGreatAxios
TheGreatAxios force-pushed the cl-6777-unit-tests-leak-per-run-project-state-into-the-real-corbits-projects branch from fdb83a0 to 9069180 Compare August 24, 2026 06:50
@TheGreatAxios
TheGreatAxios enabled auto-merge (squash) August 24, 2026 06:51
@TheGreatAxios
TheGreatAxios merged commit 91243ff into main Aug 24, 2026
5 checks passed
TheGreatAxios added a commit that referenced this pull request Aug 27, 2026
…603)

* Stop workflow-controller tests from leaking into ~/.corbits/projects

WorkflowController never threaded an override for the state-tree home, so
persist()/resume() always fell back to the real user home even when a test
passed a sandboxed one. tests/unit/workflow-controller.test.ts was the
concrete leaker: every start()/resume() call it exercised wrote a real
session directory (t-wf-controller-*) into ~/.corbits/projects.

Add an optional `home` to WorkflowControllerArgs and thread it through
persist() and resume(), then pass the test's mkdtemp'd home through the
controller and the one direct saveWorkflowState() call that skipped it.

Add scripts/guard-real-projects-dir.ts, wired into `bun run test`, which
snapshots ~/.corbits/projects before and after the suite and fails the run
if anything new appears — a backstop against this class of leak recurring
in any test, not just this file.

* Attribute guard-real-projects-dir leaks to this test run

Compare against the real ~/.corbits/projects only for entries this
run's own project keys account for, instead of any new entry: a plain
before/after snapshot also picks up sibling checkouts running their own
bun run check concurrently, which is our normal multi-worktree workflow
and not something this suite is responsible for.

Point TMPDIR/TMP/TEMP at a per-invocation scratch dir carrying this
run's id before spawning bun test. project-key.ts derives a project key
from the realpath of a test's mkdtemp'd cwd/home, so a real leak's key
inherits the run id as a substring; only those entries fail the guard.
TheGreatAxios added a commit that referenced this pull request Aug 27, 2026
…603)

* Stop workflow-controller tests from leaking into ~/.corbits/projects

WorkflowController never threaded an override for the state-tree home, so
persist()/resume() always fell back to the real user home even when a test
passed a sandboxed one. tests/unit/workflow-controller.test.ts was the
concrete leaker: every start()/resume() call it exercised wrote a real
session directory (t-wf-controller-*) into ~/.corbits/projects.

Add an optional `home` to WorkflowControllerArgs and thread it through
persist() and resume(), then pass the test's mkdtemp'd home through the
controller and the one direct saveWorkflowState() call that skipped it.

Add scripts/guard-real-projects-dir.ts, wired into `bun run test`, which
snapshots ~/.corbits/projects before and after the suite and fails the run
if anything new appears — a backstop against this class of leak recurring
in any test, not just this file.

* Attribute guard-real-projects-dir leaks to this test run

Compare against the real ~/.corbits/projects only for entries this
run's own project keys account for, instead of any new entry: a plain
before/after snapshot also picks up sibling checkouts running their own
bun run check concurrently, which is our normal multi-worktree workflow
and not something this suite is responsible for.

Point TMPDIR/TMP/TEMP at a per-invocation scratch dir carrying this
run's id before spawning bun test. project-key.ts derives a project key
from the realpath of a test's mkdtemp'd cwd/home, so a real leak's key
inherits the run id as a substring; only those entries fail the guard.
@TheGreatAxios
TheGreatAxios deleted the cl-6777-unit-tests-leak-per-run-project-state-into-the-real-corbits-projects branch August 28, 2026 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant