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

fleetd #209: resolve agentSessionId lazily via the retained PeerHandle #210

Closed
agent wants to merge 0 commits from worker/cb209-agentsessionid-4dfdb6-2 into main
pull from: worker/cb209-agentsessionid-4dfdb6-2
merge into: fleet:main
Conversation 2 Commits - Files Changed -
agent commented 2026-08-31 17:04:41 +02:00
Member
Copy Link
Copy Source

Fixes #209

SessionManager called PeerHandle.agentSessionId() exactly once, at spawn, and froze the answer into the immutable MemberSession. For an opencode member that value is always null at spawn time (opencode has not written its on-disk session row yet), and no caller ever re-asked the handle — it went out of scope at the end of the spawn method. fleet_list never reported agentSessionId for an opencode member, and fleet_spawn{resumeSessionId} was unusable for that backend.

Fix: SessionManager now retains each spawn's PeerHandle keyed by paneId, and re-resolves a still-null agentSessionId against it from roster(), get(paneId), and release() (so a released member's ReleaseDetail also carries a late-resolved id). Resolution is bounded: only sessions with a still-null id do any work, a resolved id is never looked up again (CAS-swapped into the registry), and a handle that throws degrades to "stays unresolved" rather than breaking the caller. MemberSession gains a withAgentSessionId wither in the same style as withState/withActivity.

Tests added (SessionManagerTest, driven through the real spawn path with a fake PeerLauncher/PeerHandle whose agentSessionId() answers null then a real id):

  • roster() resolves a late id from the retained handle
  • get(paneId) resolves it too
  • release() carries a late-resolved id into ReleaseDetail
  • a throwing handle does not break roster()
  • once resolved, the id is not looked up again (call count asserted)

Each of the first four tests was watched red against the pre-fix behavior (temporarily reverted the resolve calls, ran the 5 new tests, saw 4 fail with the expected null/count mismatches), then restored.

Build: mvn clean install from fleetd/ — Tests run: 1066, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.

Left as noted in the ticket: OpenCodeLauncher.SessionAwareHandle.agentSessionId()'s javadoc claiming "the caller re-calls later" is now true and was left unchanged.

Fixes #209 SessionManager called PeerHandle.agentSessionId() exactly once, at spawn, and froze the answer into the immutable MemberSession. For an opencode member that value is always null at spawn time (opencode has not written its on-disk session row yet), and no caller ever re-asked the handle — it went out of scope at the end of the spawn method. fleet_list never reported agentSessionId for an opencode member, and fleet_spawn{resumeSessionId} was unusable for that backend. **Fix:** SessionManager now retains each spawn's PeerHandle keyed by paneId, and re-resolves a still-null agentSessionId against it from roster(), get(paneId), and release() (so a released member's ReleaseDetail also carries a late-resolved id). Resolution is bounded: only sessions with a still-null id do any work, a resolved id is never looked up again (CAS-swapped into the registry), and a handle that throws degrades to "stays unresolved" rather than breaking the caller. MemberSession gains a withAgentSessionId wither in the same style as withState/withActivity. **Tests added** (SessionManagerTest, driven through the real spawn path with a fake PeerLauncher/PeerHandle whose agentSessionId() answers null then a real id): - roster() resolves a late id from the retained handle - get(paneId) resolves it too - release() carries a late-resolved id into ReleaseDetail - a throwing handle does not break roster() - once resolved, the id is not looked up again (call count asserted) Each of the first four tests was watched red against the pre-fix behavior (temporarily reverted the resolve calls, ran the 5 new tests, saw 4 fail with the expected null/count mismatches), then restored. **Build:** mvn clean install from fleetd/ — Tests run: 1066, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS. Left as noted in the ticket: OpenCodeLauncher.SessionAwareHandle.agentSessionId()'s javadoc claiming "the caller re-calls later" is now true and was left unchanged.
agent added 1 commit 2026-08-31 17:04:42 +02:00
fleetd #209: re-poll agentSessionId against the retained PeerHandle
CI / contract (pull_request) Successful in 45s
Details
CI / build (pull_request) Successful in 1m40s
Details
f9fb387427 
SessionManager used to call PeerHandle.agentSessionId() exactly once at
spawn and freeze the answer into the immutable MemberSession. For
opencode that call always came back null, because opencode has not
written its on-disk session row yet when the pane is created, and no
caller ever re-asked the handle — it went out of scope at the end of
the spawn method. fleet_list therefore never reported agentSessionId
for an opencode member, and resumeSessionId was unusable for it.

Retain each spawn's PeerHandle in SessionManager, keyed by paneId, and
re-resolve a still-null agentSessionId against it from roster(), get(),
and release() (so a released member's detail also carries a
late-resolved id). Resolution is bounded: only sessions with a still-
null id do any work, a resolved id is never looked up again, and a
throwing handle degrades to "unresolved" rather than breaking the
caller. MemberSession gains a withAgentSessionId wither in the same
style as withState/withActivity.
agent added 1 commit 2026-08-31 17:15:35 +02:00
fleetd #209 follow-up: keep roster() off the resolve path
CI / build (pull_request) Successful in 1m20s
Details
CI / contract (pull_request) Successful in 1m21s
Details
1fdaa74eb3 
roster() is the supplier for LeadHeartbeatLoop and FleetHealthMonitor
(both timer-driven) and for placement/exhaustion checks and the
metrics scrape — none of which read agentSessionId. Resolving there
meant every tick could open a lazy-resolving adapter's (opencode's)
on-disk session database once per member whose id was still unknown,
with no bound: a member whose id never appears would pay that cost for
the life of the process.

roster() goes back to its pre-#209 behavior (no resolve, no I/O). A
new rosterResolved() carries the resolve logic, and is used only by
the two surfaces that actually report agentSessionId to a caller:
fleet_list (FleetMcp.listFleet) and the REST roster
(FleetApp.listMembers). fleet_whoami's roster().stream() at
FleetMcp.java:768 does not surface the field, so it stays on the plain
roster(). get(paneId) (fleet_status) and the release() resolve are
caller-driven, not timers, and are unchanged.

Retargeted the roster-facing tests from #209 at rosterResolved(), and
added plainRosterDoesNotResolveAgentSessionId, which pins the split by
asserting the handle's agentSessionId() is not called again by
roster().
agent commented 2026-08-31 17:15:57 +02:00
Author
Member
Copy Link
Copy Source

Follow-up pushed (1fdaa74) addressing review feedback: roster() is on the LeadHeartbeatLoop/FleetHealthMonitor timers, placement/exhaustion checks, and the metrics scrape, none of which read agentSessionId — resolving there meant every tick could open opencode's on-disk session DB per unresolved member, unbounded. roster() now reverts to its pre-#209 behavior (no resolve). A new rosterResolved() carries the resolve logic and is used only where agentSessionId is actually reported to a caller: fleet_list (FleetMcp.listFleet) and the REST roster (FleetApp.listMembers). fleet_whoami's roster read at FleetMcp.java:768 does not surface the field, so it stays on the plain roster(). get(paneId)/fleet_status and the release() resolve are caller-driven, not timers, and are unchanged.

Added plainRosterDoesNotResolveAgentSessionId, which pins the split (asserts the handle's agentSessionId() is not called again by roster()) — watched red against a temporary regression, then restored. Full mvn clean install: Tests run: 1067, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.

Follow-up pushed (1fdaa74) addressing review feedback: roster() is on the LeadHeartbeatLoop/FleetHealthMonitor timers, placement/exhaustion checks, and the metrics scrape, none of which read agentSessionId — resolving there meant every tick could open opencode's on-disk session DB per unresolved member, unbounded. roster() now reverts to its pre-#209 behavior (no resolve). A new rosterResolved() carries the resolve logic and is used only where agentSessionId is actually reported to a caller: fleet_list (FleetMcp.listFleet) and the REST roster (FleetApp.listMembers). fleet_whoami's roster read at FleetMcp.java:768 does not surface the field, so it stays on the plain roster(). get(paneId)/fleet_status and the release() resolve are caller-driven, not timers, and are unchanged. Added plainRosterDoesNotResolveAgentSessionId, which pins the split (asserts the handle's agentSessionId() is not called again by roster()) — watched red against a temporary regression, then restored. Full mvn clean install: Tests run: 1067, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.
ltms commented 2026-08-31 17:22:28 +02:00
Owner
Copy Link
Copy Source

Merged to main in 735c837. Full suite on merged main: 1071 tests, 0 failures, BUILD SUCCESS.

Reviewed both rounds. The first round was correct on the fix itself and the red-before-green evidence was exactly what I asked for — including being straight about aThrowingHandleDoesNotBreakRoster staying green both ways rather than dressing it up as a bite test. That honesty is worth more than the test was.

The problem I sent back was not in the diff, it was in the call sites: roster() is the roster supplier for LeadHeartbeatLoop and FleetHealthMonitor (Fleetd.java:440 and :459), so resolving there would have opened opencode's ~841MB SQLite database on every heartbeat tick and every health tick, once per unresolved member — and forever for a member whose row never appears, since "not yet found" is not a bounded state. The split into roster() / rosterResolved() fixes that, and plainRosterDoesNotResolveAgentSessionId (asserting the call count stays at 1) is the guard that keeps someone from "tidying up" the inconsistency later.

The answer on FleetMcp.java:768 was right — that is fleet_whoami, and it never puts agentSessionId — and checking rather than assuming was the correct move.

Live verification of the end-to-end behaviour is still mine to do; I will redeploy and confirm fleet_list reports an agentSessionId for a fresh opencode member once the current fleet drains.

Merged to `main` in `735c837`. Full suite on merged main: 1071 tests, 0 failures, BUILD SUCCESS. Reviewed both rounds. The first round was correct on the fix itself and the red-before-green evidence was exactly what I asked for — including being straight about `aThrowingHandleDoesNotBreakRoster` staying green both ways rather than dressing it up as a bite test. That honesty is worth more than the test was. The problem I sent back was not in the diff, it was in the call sites: `roster()` is the roster supplier for `LeadHeartbeatLoop` and `FleetHealthMonitor` (`Fleetd.java:440` and `:459`), so resolving there would have opened opencode's ~841MB SQLite database on every heartbeat tick and every health tick, once per unresolved member — and forever for a member whose row never appears, since "not yet found" is not a bounded state. The split into `roster()` / `rosterResolved()` fixes that, and `plainRosterDoesNotResolveAgentSessionId` (asserting the call count stays at 1) is the guard that keeps someone from "tidying up" the inconsistency later. The answer on `FleetMcp.java:768` was right — that is `fleet_whoami`, and it never puts `agentSessionId` — and checking rather than assuming was the correct move. Live verification of the end-to-end behaviour is still mine to do; I will redeploy and confirm `fleet_list` reports an `agentSessionId` for a fresh opencode member once the current fleet drains.
ltms closed this pull request 2026-08-31 17:22:42 +02:00
Some checks are pending
CI / build (pull_request) Successful in 1m20s
Details
CI / contract (pull_request) Successful in 1m21s
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#210
Delete Branch "worker/cb209-agentsessionid-4dfdb6-2"

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?