fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

#290: restore reapIdle per-session guard coverage via a launcher.stop() trigger #292

Closed
agent wants to merge 0 commits from worker/fix-290-reapidle-guard-coverage-9b0dd1-1 into main
pull from: worker/fix-290-reapidle-guard-coverage-9b0dd1-1
merge into: fleet:main
Conversation 1 Commits - Files Changed -
agent commented 2026-09-04 06:24:25 +02:00
Member
Copy Link
Copy Source

#290 — Restore a test for reapIdle's per-session guard, which #283 left uncovered

What changed

  • FakeHerdr.java: added paneCloseFailsForPane(paneId, code) — makes pane.close fail for
    exactly one pane id, while every other pane's close still succeeds. The existing
    paneCloseFailsWith(code) fails every pane.close call regardless of target, which cannot
    isolate a single session's launcher.stop() failure inside a multi-session reap. This is general
    enough to be reused by other "one session fails" tests in this suite, per the ticket — no other
    test was touched.
  • SessionManagerTest.java: added reapIdleSurvivesOneSessionWhoseLauncherStopFails, placed
    right beside reapIdleCountsAllThreeSessionsWhenOnlyItsWorktreeRemovalFails (#283's test, kept
    exactly as-is — not replaced). Three worktree sessions are acquired and marked idle; the middle
    one's herdr pane is set to fail pane.close via the new fake method, so release()'s
    launcher.stop(paneId) call (which has no try/catch of its own, SessionManager.java:335)
    throws uncaught out of release(). The test asserts:
    • reapIdle still returns 2 (the failing session isn't counted, but the pass doesn't abort)
    • the first and third sessions are still deregistered and their worktrees still removed
    • the middle session is deregistered too (its registry removal happens before launcher.stop()
      runs, so that part is unaffected by the later throw)
    • the middle session's worktree removal never runs (release() throws before reaching it)
    • a WARN is logged naming the failing pane/terminal ("reap failed for pane=...")

No SessionManager.java (or any other main-source) change was made — this is test and
test-infrastructure only, per the ticket's scope.

Mutation proof (per the ticket's "prove it" requirement)

I temporarily deleted reapIdle's try/catch (SessionManager.java:852-860), ran only the new test,
and restored the guard afterward. git diff -- src/main/java/dev/ltms/fleet/session/SessionManager.java
is empty on this branch — confirming the guard is back exactly as it was.

With the guard removed, the new test fails with the exception propagating out of reapIdle
uncaught:

[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.204 s <<< FAILURE! -- in dev.ltms.fleet.session.SessionManagerTest
[ERROR] dev.ltms.fleet.session.SessionManagerTest.reapIdleSurvivesOneSessionWhoseLauncherStopFails -- Time elapsed: 0.193 s <<< ERROR!
dev.ltms.fleet.herdr.HerdrException: herdr error [internal_error]: pane.close failed
	at dev.ltms.fleet.herdr.FakeHerdr.call(FakeHerdr.java:381)
	at dev.ltms.fleet.herdr.AgentControl.close(AgentControl.java:162)
	at dev.ltms.fleet.member.HerdrPeerLauncher.stop(HerdrPeerLauncher.java:931)
	at dev.ltms.fleet.session.SessionManager.release(SessionManager.java:335)
	at dev.ltms.fleet.session.SessionManager.release(SessionManager.java:257)
	at dev.ltms.fleet.session.SessionManager.reapIdle(SessionManager.java:854)
	at dev.ltms.fleet.session.SessionManagerTest.reapIdleSurvivesOneSessionWhoseLauncherStopFails(SessionManagerTest.java:1113)
	...
[ERROR]   SessionManagerTest.reapIdleSurvivesOneSessionWhoseLauncherStopFails:1113 » Herdr herdr error [internal_error]: pane.close failed
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0
[INFO] BUILD FAILURE

With the guard present (the actual diff in this PR), that same test passes.

Build

Full build, unpiped, on the restored (guard-present) tree:

[INFO] Tests run: 55, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.124 s -- in dev.ltms.fleet.session.SessionManagerTest
...
[INFO] Tests run: 1297, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS

Note for review — another guard-lost-its-trigger candidate

Not fixed here (out of scope): HerdrPeerLauncher.stop's tab-cleanup path
(spaces.closeTab(loc.tabId()) at HerdrPeerLauncher.java:937) has no try/catch of its own inside
stop(), unlike the releaseZdotdir step right below it, which is explicitly wrapped. I did not
check whether any existing test still exercises a closeTab failure here — flagging only, not
verifying or fixing, per the ticket's "name it, don't fix it" instruction.

## #290 — Restore a test for reapIdle's per-session guard, which #283 left uncovered ### What changed - **`FakeHerdr.java`**: added `paneCloseFailsForPane(paneId, code)` — makes `pane.close` fail for exactly one pane id, while every other pane's close still succeeds. The existing `paneCloseFailsWith(code)` fails every `pane.close` call regardless of target, which cannot isolate a single session's `launcher.stop()` failure inside a multi-session reap. This is general enough to be reused by other "one session fails" tests in this suite, per the ticket — no other test was touched. - **`SessionManagerTest.java`**: added `reapIdleSurvivesOneSessionWhoseLauncherStopFails`, placed right beside `reapIdleCountsAllThreeSessionsWhenOnlyItsWorktreeRemovalFails` (#283's test, kept exactly as-is — not replaced). Three worktree sessions are acquired and marked idle; the middle one's herdr pane is set to fail `pane.close` via the new fake method, so `release()`'s `launcher.stop(paneId)` call (which has no try/catch of its own, `SessionManager.java:335`) throws uncaught out of `release()`. The test asserts: - `reapIdle` still returns `2` (the failing session isn't counted, but the pass doesn't abort) - the first and third sessions are still deregistered and their worktrees still removed - the middle session is deregistered too (its registry removal happens before `launcher.stop()` runs, so that part is unaffected by the later throw) - the middle session's worktree removal never runs (release() throws before reaching it) - a WARN is logged naming the failing pane/terminal ("reap failed for pane=...") No `SessionManager.java` (or any other main-source) change was made — this is test and test-infrastructure only, per the ticket's scope. ### Mutation proof (per the ticket's "prove it" requirement) I temporarily deleted `reapIdle`'s try/catch (`SessionManager.java:852-860`), ran only the new test, and restored the guard afterward. `git diff -- src/main/java/dev/ltms/fleet/session/SessionManager.java` is empty on this branch — confirming the guard is back exactly as it was. **With the guard removed**, the new test fails with the exception propagating out of `reapIdle` uncaught: ``` [ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.204 s <<< FAILURE! -- in dev.ltms.fleet.session.SessionManagerTest [ERROR] dev.ltms.fleet.session.SessionManagerTest.reapIdleSurvivesOneSessionWhoseLauncherStopFails -- Time elapsed: 0.193 s <<< ERROR! dev.ltms.fleet.herdr.HerdrException: herdr error [internal_error]: pane.close failed at dev.ltms.fleet.herdr.FakeHerdr.call(FakeHerdr.java:381) at dev.ltms.fleet.herdr.AgentControl.close(AgentControl.java:162) at dev.ltms.fleet.member.HerdrPeerLauncher.stop(HerdrPeerLauncher.java:931) at dev.ltms.fleet.session.SessionManager.release(SessionManager.java:335) at dev.ltms.fleet.session.SessionManager.release(SessionManager.java:257) at dev.ltms.fleet.session.SessionManager.reapIdle(SessionManager.java:854) at dev.ltms.fleet.session.SessionManagerTest.reapIdleSurvivesOneSessionWhoseLauncherStopFails(SessionManagerTest.java:1113) ... [ERROR] SessionManagerTest.reapIdleSurvivesOneSessionWhoseLauncherStopFails:1113 » Herdr herdr error [internal_error]: pane.close failed [ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0 [INFO] BUILD FAILURE ``` **With the guard present** (the actual diff in this PR), that same test passes. ### Build Full build, unpiped, on the restored (guard-present) tree: ``` [INFO] Tests run: 55, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.124 s -- in dev.ltms.fleet.session.SessionManagerTest ... [INFO] Tests run: 1297, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ``` ### Note for review — another guard-lost-its-trigger candidate Not fixed here (out of scope): `HerdrPeerLauncher.stop`'s tab-cleanup path (`spaces.closeTab(loc.tabId())` at `HerdrPeerLauncher.java:937`) has no try/catch of its own inside `stop()`, unlike the `releaseZdotdir` step right below it, which is explicitly wrapped. I did not check whether any existing test still exercises a `closeTab` failure here — flagging only, not verifying or fixing, per the ticket's "name it, don't fix it" instruction.
agent added 1 commit 2026-09-04 06:24:25 +02:00
#290: restore reapIdle's per-session guard coverage via a new launcher.stop() trigger
CI / contract (pull_request) Successful in 1m29s
Details
CI / build (pull_request) Successful in 1m56s
Details
ef507bcd12 
#283 fixed release() to catch and log a worktree-removal failure, which closed off
reapIdleCountsAllThreeSessionsWhenOnlyItsWorktreeRemovalFails as a trigger for
reapIdle's own per-session try/catch (CB-581) — that test now proves a different,
still-real thing (a swallowed removal failure doesn't shrink the reaped count),
but the try/catch itself lost its test.

Add FakeHerdr.paneCloseFailsForPane(paneId, code) so a test can make exactly one
session's launcher.stop() fail while its siblings still tear down normally
(paneCloseFailsWith already existed but fails every pane, which cannot isolate
one session in a three-session reap). Add
reapIdleSurvivesOneSessionWhoseLauncherStopFails beside the #283 test, using
launcher.stop() as the trigger the ticket names, and prove it catches removal of
reapIdle's try/catch: deleting the guard makes the test fail with the
HerdrException propagating out of reapIdle uncaught (quoted in the PR body).
ltms commented 2026-09-04 06:28:20 +02:00
Owner
Copy Link
Copy Source

Merged locally as 61097e5. One follow-up commit d5128a1 replaces the fully-qualified java.util.concurrent.ConcurrentHashMap / java.util.Map with the plain names, since FakeHerdr already imports java.util.Map. No behaviour change.

The out-of-scope note about HerdrPeerLauncher.stop()'s bare closeTab was correct and is now #293 — the consequence turned out to include a leaked worktree, not just the ZDOTDIR. Good catch.

Merged locally as `61097e5`. One follow-up commit `d5128a1` replaces the fully-qualified `java.util.concurrent.ConcurrentHashMap` / `java.util.Map` with the plain names, since `FakeHerdr` already imports `java.util.Map`. No behaviour change. The out-of-scope note about `HerdrPeerLauncher.stop()`'s bare `closeTab` was correct and is now #293 — the consequence turned out to include a leaked worktree, not just the ZDOTDIR. Good catch.
ltms closed this pull request 2026-09-04 06:28:32 +02:00
Some checks are pending
CI / contract (pull_request) Successful in 1m29s
Details
CI / build (pull_request) Successful in 1m56s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#292
Delete Branch "worker/fix-290-reapidle-guard-coverage-9b0dd1-1"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?