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

fleetd #711: re-key two pane-tab hazard reasons onto the unregistered-pane condition #714

Closed
agent wants to merge 0 commits from worker/task-12-4d0479-9 into main
pull from: worker/task-12-4d0479-9
merge into: fleet:main
Conversation 2 Commits - Files Changed -
agent commented 2026-10-04 06:55:07 +02:00
Member
Copy Link
Copy Source

Both refusals in FleetConfig.java (the tabLabel/tabPrefix collision check and validatePanePlacementAgainstLeadTabs) gave a reason that is no longer accurate on its own: that a member landing in a lead's or collaborator's labelled tab would be read back as that identity and granted its authority.

That consequence cannot happen for a live, registered member. CallerResolver.resolve (src/main/java/dev/ltms/fleet/auth/CallerResolver.java:294-299) consults the spawned-member roster first, ahead of every tab map: a live spawned member's identity is its own, whatever the tab label says. The hazard is real only in the two windows where a pane is alive but the roster holds no entry for it yet: before a newly spawned member's launcher has registered it, and after a member's session ends but its pane/terminal is reused before the roster entry is cleared.

This change re-keys the reason in both throw messages, the javadoc on validatePanePlacementAgainstLeadTabs, and two FleetConfigTest comments (lines ~853 and ~1091 pre-edit) onto that condition: the hazard applies while the pane is alive and the roster carries no entry for it. No behavior changed — only prose. Neither check is deleted or weakened.

Comment rules followed: no used to/no longer/previously/now/ticket numbers/dates/file:line refs/we/I in the new or touched comment text (one such phrase, fleetd #669, existed in the old FleetConfigTest comment text being replaced and is removed, not added).

Tests run (mvn clean install, this worktree): Tests run: 2005, Failures: 0, Errors: 0, Skipped: 0 — unchanged from before the edit, BUILD SUCCESS.

Revert-proof: with the throw in validatePanePlacementAgainstLeadTabs temporarily deleted, mvn test -Dtest=FleetConfigTest turned 3 tests red (aPanePlacedProfileWithALeadTabRefusesToStart, aPanePlacedProfileWithACollaboratorTabRefusesToStartEvenWithNoLeaders, validateAllAlsoRefusesPanePlacementAgainstALeadTab), proving the tests really drive the validator. The throw was then restored and the full build re-verified green.

Ref: fleetd #711.

Both refusals in FleetConfig.java (the tabLabel/tabPrefix collision check and validatePanePlacementAgainstLeadTabs) gave a reason that is no longer accurate on its own: that a member landing in a lead's or collaborator's labelled tab would be read back as that identity and granted its authority. That consequence cannot happen for a live, registered member. CallerResolver.resolve (src/main/java/dev/ltms/fleet/auth/CallerResolver.java:294-299) consults the spawned-member roster first, ahead of every tab map: a live spawned member's identity is its own, whatever the tab label says. The hazard is real only in the two windows where a pane is alive but the roster holds no entry for it yet: before a newly spawned member's launcher has registered it, and after a member's session ends but its pane/terminal is reused before the roster entry is cleared. This change re-keys the reason in both throw messages, the javadoc on validatePanePlacementAgainstLeadTabs, and two FleetConfigTest comments (lines ~853 and ~1091 pre-edit) onto that condition: the hazard applies while the pane is alive and the roster carries no entry for it. No behavior changed — only prose. Neither check is deleted or weakened. Comment rules followed: no `used to`/`no longer`/`previously`/`now`/ticket numbers/dates/file:line refs/`we`/`I` in the new or touched comment text (one such phrase, `fleetd #669`, existed in the old FleetConfigTest comment text being replaced and is removed, not added). Tests run (mvn clean install, this worktree): `Tests run: 2005, Failures: 0, Errors: 0, Skipped: 0` — unchanged from before the edit, `BUILD SUCCESS`. Revert-proof: with the throw in validatePanePlacementAgainstLeadTabs temporarily deleted, `mvn test -Dtest=FleetConfigTest` turned 3 tests red (`aPanePlacedProfileWithALeadTabRefusesToStart`, `aPanePlacedProfileWithACollaboratorTabRefusesToStartEvenWithNoLeaders`, `validateAllAlsoRefusesPanePlacementAgainstALeadTab`), proving the tests really drive the validator. The throw was then restored and the full build re-verified green. Ref: fleetd #711.
agent added 1 commit 2026-10-04 06:55:08 +02:00
fleetd #711: re-key two pane-tab hazard reasons onto the unregistered-pane condition
CI / shell-tests (pull_request) Failing after 7s
Details
CI / contract (pull_request) Successful in 51s
Details
CI / build (pull_request) Failing after 1m39s
Details
7d497aa423 
Both refusals said a member landing in a lead's or collaborator's
labelled tab would be read back as that identity. CallerResolver
consults the spawned-member roster ahead of every tab map, so a live
registered member is never misread this way. State the real condition
instead: the hazard applies only while the pane is alive and carries
no entry in the spawned-member roster.
ltms commented 2026-10-04 07:04:44 +02:00
Owner
Copy Link
Copy Source

Merged locally in c468953. Closing by hand, because a local merge never closes a PR here.

Verified

Merged onto origin/main in a throwaway worktree and ran mvn clean install myself, output to a
file and not piped: exit 0, BUILD SUCCESS, Tests run: 2008, Failures: 0,
FleetConfigTest: 170.

2008 is unchanged from main, which is the right answer for a change that edits text only. The
pushed tree is byte-identical to the tree I built (9877e51).

Criterion 3 — I re-ran it myself, with the control the other way round

Your control compared against whole-file content on origin/main, which is a looser bound than the
property. I ran the pattern against the diff in both directions instead:

ADDED-LINE MATCHES:   0
REMOVED-LINE MATCHES: 1

The 1 removed match is the fleetd #669: prefix you deleted. That is a better control than a
whole-file count: it proves the pattern fires on this diff, so the zero on added lines is a
real zero and not a dead pattern. Good instinct to pair it with something; this is the sharper
version.

The re-keyed text reads correctly

Each message now states the condition rather than the bare consequence — "while its pane carries no
entry in the spawned-member roster, is read back as a lead or collaborator". Tense moved from
"would be read back" to "is read back", which is right: the comment describes what the code does
now, and the conditional lives in the clause rather than in the verb. No history, no ticket number,
no line reference on any added line.

Dropping the fleetd #669: prefix from the test javadoc was the correct call and I did not ask for
it specifically.

Revert proof

Three tests red on deleting the throw — aPanePlacedProfileWithALeadTabRefusesToStart,
aPanePlacedProfileWithACollaboratorTabRefusesToStartEvenWithNoLeaders,
validateAllAlsoRefusesPanePlacementAgainstALeadTab — then green again after restore. That is the
criterion met, and naming which three is what makes it checkable.

The wiki half

wiki/11-Features.md carried the same stale reason in two places. That was correctly out of your
scope — it is a submodule and uninitialized in your worktree. I have taken it, and I am matching
your wording so the page and the code do not drift apart.

Merged locally in `c468953`. Closing by hand, because a local merge never closes a PR here. ## Verified Merged onto `origin/main` in a throwaway worktree and ran `mvn clean install` myself, output to a file and not piped: exit 0, `BUILD SUCCESS`, `Tests run: 2008, Failures: 0`, `FleetConfigTest: 170`. 2008 is unchanged from main, which is the right answer for a change that edits text only. The pushed tree is byte-identical to the tree I built (`9877e51`). ## Criterion 3 — I re-ran it myself, with the control the other way round Your control compared against whole-file content on `origin/main`, which is a looser bound than the property. I ran the pattern against the diff in both directions instead: ``` ADDED-LINE MATCHES: 0 REMOVED-LINE MATCHES: 1 ``` The 1 removed match is the `fleetd #669:` prefix you deleted. That is a better control than a whole-file count: it proves the pattern fires **on this diff**, so the zero on added lines is a real zero and not a dead pattern. Good instinct to pair it with something; this is the sharper version. ## The re-keyed text reads correctly Each message now states the condition rather than the bare consequence — "while its pane carries no entry in the spawned-member roster, is read back as a lead or collaborator". Tense moved from "would be read back" to "is read back", which is right: the comment describes what the code does now, and the conditional lives in the clause rather than in the verb. No history, no ticket number, no line reference on any added line. Dropping the `fleetd #669:` prefix from the test javadoc was the correct call and I did not ask for it specifically. ## Revert proof Three tests red on deleting the throw — `aPanePlacedProfileWithALeadTabRefusesToStart`, `aPanePlacedProfileWithACollaboratorTabRefusesToStartEvenWithNoLeaders`, `validateAllAlsoRefusesPanePlacementAgainstALeadTab` — then green again after restore. That is the criterion met, and naming which three is what makes it checkable. ## The wiki half `wiki/11-Features.md` carried the same stale reason in two places. That was correctly out of your scope — it is a submodule and uninitialized in your worktree. I have taken it, and I am matching your wording so the page and the code do not drift apart.
ltms closed this pull request 2026-10-04 07:04:47 +02:00
ltms commented 2026-10-04 07:05:07 +02:00
Owner
Copy Link
Copy Source

One correction to the PR description, so it does not propagate. None of it is in the code, so
nothing shipped wrong.

The description names the two unregistered-pane windows as:

before a newly spawned member's launcher has registered it, and after a member's session ends but
its pane/terminal is reused before the roster entry is cleared

The first one does not exist. I believed it too and an architect disproved it earlier in this
session. spawnReadyTimeoutMs is a timeout on reaching idle, not a duration the member spends
unregistered, and idle comes before the MCP connect: MemberPresence's own javadoc says
herdr's agent_status "reports idle for a worker whose Claude is still booting", and presence is
populated from the MCP transport, where initialize is the first contact. So
waitUntilInjectableOrThrow returns, registry.put follows a few in-memory statements later, and
the member's first MCP call arrives after that. The gap is microseconds with no I/O in it.

The measurement that settles it: READY is reachable only via transitionByTerminal, which scans
the registry, and onDelivered refuses BUSY unless the state is already READY or DONE
(SessionManager.java:911). Any member reporting state: "busy" therefore had a
post-registration MCP contact.

The second window is real but stated slightly off. It is not pane reuse — it is that
release() removes the registry entry first and then shells out to git twice before
launcher.stop(paneId). The pane is alive and unregistered for the length of two git commands.
That is #702.

The real two windows, as #711 records them, are #702's teardown window and a pane that
outlives a daemon restart (FleetMcp.java:1303 names the second: "a worker the registry has no
record of — one that outlived a daemon restart").

This changes nothing about the merge. The re-keyed text in the code says "while its pane carries no
entry in the spawned-member roster", which is true under either window and does not depend on
enumerating them.

One correction to the PR description, so it does not propagate. None of it is in the code, so nothing shipped wrong. The description names the two unregistered-pane windows as: > before a newly spawned member's launcher has registered it, and after a member's session ends but > its pane/terminal is reused before the roster entry is cleared **The first one does not exist.** I believed it too and an architect disproved it earlier in this session. `spawnReadyTimeoutMs` is a timeout on reaching `idle`, not a duration the member spends unregistered, and `idle` comes *before* the MCP connect: `MemberPresence`'s own javadoc says herdr's `agent_status` "reports `idle` for a worker whose Claude is still booting", and presence is populated from the MCP transport, where `initialize` is the first contact. So `waitUntilInjectableOrThrow` returns, `registry.put` follows a few in-memory statements later, and the member's first MCP call arrives after that. The gap is microseconds with no I/O in it. The measurement that settles it: `READY` is reachable only via `transitionByTerminal`, which scans the registry, and `onDelivered` refuses `BUSY` unless the state is already `READY` or `DONE` (`SessionManager.java:911`). Any member reporting `state: "busy"` therefore had a post-registration MCP contact. The second window is real but stated slightly off. It is not pane reuse — it is that `release()` removes the registry entry **first** and then shells out to `git` twice before `launcher.stop(paneId)`. The pane is alive and unregistered for the length of two git commands. That is #702. The real two windows, as #711 records them, are **#702's teardown window** and **a pane that outlives a daemon restart** (`FleetMcp.java:1303` names the second: "a worker the registry has no record of — one that outlived a daemon restart"). This changes nothing about the merge. The re-keyed text in the code says "while its pane carries no entry in the spawned-member roster", which is true under either window and does not depend on enumerating them.
Some checks are pending
CI / shell-tests (pull_request) Failing after 7s
Details
CI / contract (pull_request) Successful in 51s
Details
CI / build (pull_request) Failing after 1m39s
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#714
Delete Branch "worker/task-12-4d0479-9"

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?