[Bug]: Worktree containment test races workspace teardown after tool completion

[dennisonbertram]

Work type

Bug / regression

Observed behavior

On PR #1033, GitHub Actions test-fast run 30551198514 failed TestWorktreeContainment_ToolCwdIsWorktree: the test received tool.call.completed, then both <worktree>/out.txt and <worktree>/marker.txt were already absent. The same exact head passed the local normal/race/full regression gates. The failure is intermittent because the runner can finish its next provider turn and remove the worktree before the buffered completion event is consumed.

Expected behavior

The containment regression must inspect the real bash tool's relative-path outputs while the provisioned worktree is guaranteed to remain alive. It must still fail if bash executes outside the worktree, leaks to the daemon cwd, errors, or never emits completion.

Reproduction

Preconditions: Linux GitHub Actions or a sufficiently fast/concurrent local run.

Command: go test ./internal/harness -run '^TestWorktreeContainment_ToolCwdIsWorktree$' -count=100

CI evidence: run 30551198514, job 90900009854.

Actual: after tool.call.completed, stat .../out.txt: no such file or directory and open .../marker.txt: no such file or directory.

Reproduction rate: timing-dependent; observed on the rebased #1033 head.

User and operational impact

Severity P1 for delivery process: the accepted baseline is red and blocks merging otherwise-green cron/callback GUI correctness work. No production data or security impact; the defect is in a concurrency-sensitive regression test.

Suspected seam and search evidence

Owning seam: internal/harness/workspace_selection_test.go, worktreeContainmentProvider, and TestWorktreeContainment_ToolCwdIsWorktree.

Searched stubProvider.Complete, runner subscription handling, workspace lifecycle, and completion-event assertions. The test assumes consuming a buffered tool.call.completed event happens before the runner's immediately following provider turn completes and destroys the worktree. Event emission does not provide that consumer-side lifetime guarantee. Production worktree routing is not implicated: the failure reports no tool error and both files disappear together at teardown.

Blast-radius impact map

• Callers/data flow: test-only provider -> real bash tool -> buffered runner events -> workspace cleanup.
• Config/env/defaults: None — isolated test fixtures.
• API/CLI/wire formats/tools: real bash invocation retained; no public changes.
• Persistence/schema/cache: None.
• Concurrency/lifecycle: affected; test must synchronize teardown with assertion completion.
• Security/auth/privacy: None.
• TUI/web/macOS: None directly; CI gate only.
• Provider/model/catalog: test stub only.
• Deployment/observability: GitHub Actions fast gate.
• Compatibility: no production behavior change.
• Existing tests/fixtures: containment regression and full normal/race gates.
• Documentation: engineering log, plan, impact map, indexes.

Regression test first

Test file/name: existing TestWorktreeContainment_ToolCwdIsWorktree with a test-provider release handshake.

Red command: focused test with the release disabled or teardown deliberately allowed before the assertion.

Expected failure: missing out.txt/marker.txt despite a successful completion event.

Why it proves the bug: it demonstrates the assertion races lifecycle cleanup rather than tool routing.

False-positive controls: continue asserting no daemon-cwd leak, exact resolved pwd, tool error visibility, provisioned/completed events, and real bash execution.

Fix boundaries

In scope: hold the provider's terminal turn until the subscriber completes containment assertions, then release normal cleanup.

Out of scope: production runner/worktree lifecycle changes, sleeps/retries around missing files, or weakening containment assertions.

Existing abstraction: test provider synchronization; no parallel production abstraction.

Diagnostic and observability evidence

Preserve the GitHub Actions error, emit explicit timeout/failure if the assertion never releases the provider, and retain the real tool error payload in failures.

Verification plan

• Deterministic red demonstrating cleanup can beat subscriber consumption.
• Focused green, -count=100.
• Focused race green, -race -count=100.
• go test ./internal/harness/....
• Full ./scripts/test-regression.sh normal/race/coverage.
• Re-run PR CI.

Rollout and rollback

Test-only change. Land before rebasing #1033. Roll back the commit if it masks a real routing failure or deadlocks; no data repair.

Documentation and handoff

Update docs/logs/engineering-log.md, docs/logs/long-term-thinking-log.md, docs/plans/INDEX.md, and add issue plan/impact map. Related epic: #1000.

Definition of done

• Deterministic reproduction recorded
• Regression test failed before the fix
• Test lifecycle seam fixed without production changes
• Containment assertions remain strict
• Focused, race, and full regression green
• CI re-run is green
• Engineering log, plan, impact map, and indexes current

Process acknowledgement

I will preserve the failing regression test, update the issue if the root cause changes, and record any newly discovered bug as its own issue.