Skip to content

test(spawn): add regression test for reused pooled worktree hook identity - #24

Merged
trillium merged 1 commit into
mainfrom
fm/spawn-hook-refresh-fm-spawn-reused-pool-worktree-inherits-p-b6
Aug 1, 2026
Merged

test(spawn): add regression test for reused pooled worktree hook identity#24
trillium merged 1 commit into
mainfrom
fm/spawn-hook-refresh-fm-spawn-reused-pool-worktree-inherits-p-b6

Conversation

@trillium

@trillium trillium commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Intent

Fix bin/fm-spawn.sh so a reused pooled treehouse worktree never inherits the prior tenant's per-task Claude hook identity (task id, busy-gen). Investigation found the write path already unconditionally overwrites every harness's hook artifact on every spawn (no skip-if-exists), so no functional code change is needed. Add a permanent regression test (tests/fm-spawn-reused-worktree-hooks.test.sh) covering spawn -> teardown -> respawn into the same pooled worktree, asserting the new task's hooks reference only its own id/gen and that firing its Stop hook never touches the prior task's turn-ended marker, plus a one-line invariant comment in fm-spawn.sh documenting the always-overwrite contract for future harness wiring.

What Changed

  • Added tests/fm-spawn-reused-worktree-hooks.test.sh, covering spawn → teardown → respawn into the same pooled treehouse worktree, asserting the new task's hooks reference only its own task id/busy-gen and that firing its Stop hook never touches the prior tenant's turn-ended marker.
  • Added a one-line invariant comment in bin/fm-spawn.sh documenting that the hook-artifact write path unconditionally overwrites every harness's hook identity on each spawn (no skip-if-exists), for future harness wiring.
  • No functional code change: the write path already overwrote hook artifacts unconditionally, so this closes the gap with a permanent regression test rather than a fix.

Risk Assessment

✅ Low: The change is a comment-only addition to fm-spawn.sh (no functional code change) plus a new regression test whose assertions I verified against the actual hook-write, teardown-scrub, and busy-gen/turn-ended paths in fm-spawn.sh, fm-teardown.sh, and fm-busy-event.sh.

Testing

The diff (a one-line invariant comment in bin/fm-spawn.sh plus the new tests/fm-spawn-reused-worktree-hooks.test.sh) was exercised directly: the new test drives a real spawn -> teardown -> respawn sequence into the same pooled worktree via fm-spawn.sh/fm-teardown.sh with faked tmux/harness binaries, asserts the reused worktree's .claude/settings.local.json hooks reference only the second task's id and a distinct busy-gen, and fires the installed Stop hook to confirm it touches only the new task's turn-ended marker and never resurrects the prior tenant's — all assertions pass. The test is properly registered in the coverage guard and a portable CI lane, and sibling fm-spawn regression tests remain green, confirming the comment-only source change caused no regressions.

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • bin/fm-test-run.sh tests/fm-spawn-reused-worktree-hooks.test.sh — full end-to-end scenario, passes
  • bin/fm-test-run.sh --check-coverage — new test is accounted for in the complete regression partition (total=106)
  • bin/fm-test-run.sh --list --lane portable-serial — confirmed new test is scheduled in a CI lane (not silently orphaned)
  • bin/fm-test-run.sh tests/fm-spawn-batch.test.sh tests/fm-spawn-dispatch-profile.test.sh tests/fm-spawn-worktree-settle.test.sh — sibling fm-spawn.sh regression tests still pass after the comment-only change
  • git status --porcelain — worktree left clean, no test artifacts committed
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed stale automation settings when reusing a previously occupied worktree.
    • Ensured refreshed task hooks are applied correctly during respawn.
    • Prevented completion signals from affecting tasks other than the active one.
  • Tests

    • Added end-to-end coverage for worktree reuse, teardown, respawn, and task-specific completion handling.

…e regression test

bin/fm-spawn.sh already rewrites every harness's hook artifact
unconditionally on every spawn (cat/printf >, never skip-if-exists), so a
worktree treehouse hands back from its reuse pool gets hooks bound to the
current incarnation's id/gen, never a prior tenant's - confirmed by tracing
every branch of the harness case statement and by an end-to-end spawn ->
teardown -> respawn-into-the-same-worktree reproduction.

Adds tests/fm-spawn-reused-worktree-hooks.test.sh as a permanent regression
guard for that sequence (claude harness: settings.local.json hook identity,
busy-gen uniqueness, and the Stop hook only ever touching its own task's
turn-ended marker), and a short comment on the case statement making the
overwrite invariant explicit for future harness wiring.
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9b962f83-845c-40c9-91c7-363d32a566cc

📥 Commits

Reviewing files that changed from the base of the PR and between 1e11ba8 and 8f4b12d.

📒 Files selected for processing (2)
  • bin/fm-spawn.sh
  • tests/fm-spawn-reused-worktree-hooks.test.sh

📝 Walkthrough

Walkthrough

The change documents unconditional hook artifact rewrites and adds an end-to-end test for spawning, tearing down, and respawning in the same pooled worktree.

Changes

Reused worktree hook validation

Layer / File(s) Summary
Hook rewrite and reuse regression
bin/fm-spawn.sh, tests/fm-spawn-reused-worktree-hooks.test.sh
Comments document task-incarnation and busy-generation bindings. The regression test verifies stale hook cleanup, fresh hook creation, and current-task Stop hook markers.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: kunchenguid

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding a regression test for hook identity in reused pooled worktrees.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fm/spawn-hook-refresh-fm-spawn-reused-pool-worktree-inherits-p-b6

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@trillium
trillium merged commit 3ab66f6 into main Aug 1, 2026
12 checks passed
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