#185: refuse an unowned paneId when more than one herdr daemon could own it #187

2026-08-28T04:45:46+02:00

ltms commented

2026-08-28 04:45:46 +02:00

Part of #185. Opened on behalf of the implementing member, whose authenticated POST was blocked by its command classifier.

Why

Members are going to run on a second herdr daemon under a different OS user, so they cannot read the operator's credentials (#184). I measured what two live herdr daemons do to ids:

server A: workspaces [('w1','~'), ('w2','exp-A')]   panes [w1:p3, w1:p1, w2:p1]
server B: workspaces [('w1','exp-B')]               panes [w1:p1]

Workspace, tab and pane ids are per-daemon sequential counters. Both daemons held w1:p1 at once, pointing at different panes owned by different users. Collision is guaranteed, not unlucky. fleet_stop{paneId} takes a paneId, so today's fallback — "no recorded owner, route to the first adapter" — would close a pane on an arbitrary daemon.

What changed

CompositePeerLauncher.stop keeps its spawn-time paneId→adapter record. When there is no record it now counts distinct herdr daemons and refuses the id if there is more than one, instead of guessing.

The count is over HerdrClient identity, deliberately: two adapter kinds sharing one client are one daemon, and identity keeps that true even if a client ever gains value equality.

The bug this went through first

The first revision guarded on delegates.size() != 1. That conflates adapter kinds with daemons — the live Mac fleet runs sonnet/opus/local/local-direct as claude-code and gx/sol/terra/xf as opencode, so it holds 2 delegates on 1 daemon. That revision would have thrown on the current fleet for any pane with no recorded owner, most obviously after a restart. Caught in review; the added test now pins the two-adapters-one-daemon case.

Compatibility

Single-daemon behaviour is unchanged, which matters because a live fleet is running against this. fleet_list output and fleet_stop input are untouched — no id format change, no new required prefix. Today the throw is unreachable: nothing can yet supply a second client. It becomes reachable on its own once the HerdrRouter (#186) does, with no flag to remember.

Known follow-up, not addressed here

spawnedBy is in-memory, so it is empty after a restart. Once two-daemon mode is real, every surviving pane is unowned and every fleet_stop on one will refuse. Recovering pane ownership at boot is a separate unit. Refusing is still strictly safer than the old behaviour of closing a pane on an arbitrary daemon.

CompositePeerLauncher.list() also still deduplicates by raw paneId, which assumes global uniqueness — reported by the implementer, deliberately not fixed here.

Tests

Tests run: 990, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS.

stopKeepsSameHerdrPaneIdSeparateByOwningAdapter
stopAllowsALegacyBarePaneIdWithOneDaemon
stopRejectsAnUnownedPaneIdWhenMultipleDaemonsCouldOwnIt
stopAllowsAnUnownedPaneIdWithTwoAdaptersSharingOneDaemon — the live Mac shape

Part of #185. Opened on behalf of the implementing member, whose authenticated POST was blocked by its command classifier. ## Why Members are going to run on a second herdr daemon under a different OS user, so they cannot read the operator's credentials (#184). I measured what two live herdr daemons do to ids: ``` server A: workspaces [('w1','~'), ('w2','exp-A')] panes [w1:p3, w1:p1, w2:p1] server B: workspaces [('w1','exp-B')] panes [w1:p1] ``` Workspace, tab and pane ids are **per-daemon sequential counters**. Both daemons held `w1:p1` at once, pointing at different panes owned by different users. Collision is guaranteed, not unlucky. `fleet_stop{paneId}` takes a paneId, so today's fallback — "no recorded owner, route to the first adapter" — would close a pane on an arbitrary daemon. ## What changed `CompositePeerLauncher.stop` keeps its spawn-time paneId→adapter record. When there is no record it now counts **distinct herdr daemons** and refuses the id if there is more than one, instead of guessing. The count is over `HerdrClient` **identity**, deliberately: two adapter kinds sharing one client are one daemon, and identity keeps that true even if a client ever gains value equality. ## The bug this went through first The first revision guarded on `delegates.size() != 1`. That conflates adapter kinds with daemons — the live Mac fleet runs `sonnet`/`opus`/`local`/`local-direct` as `claude-code` and `gx`/`sol`/`terra`/`xf` as `opencode`, so it holds **2 delegates on 1 daemon**. That revision would have thrown on the current fleet for any pane with no recorded owner, most obviously after a restart. Caught in review; the added test now pins the two-adapters-one-daemon case. ## Compatibility Single-daemon behaviour is unchanged, which matters because a live fleet is running against this. `fleet_list` output and `fleet_stop` input are untouched — no id format change, no new required prefix. Today the throw is unreachable: nothing can yet supply a second client. It becomes reachable on its own once the `HerdrRouter` (#186) does, with no flag to remember. ## Known follow-up, not addressed here `spawnedBy` is in-memory, so it is empty after a restart. Once two-daemon mode is real, every surviving pane is unowned and every `fleet_stop` on one will refuse. Recovering pane ownership at boot is a separate unit. Refusing is still strictly safer than the old behaviour of closing a pane on an arbitrary daemon. `CompositePeerLauncher.list()` also still deduplicates by raw paneId, which assumes global uniqueness — reported by the implementer, deliberately not fixed here. ## Tests `Tests run: 990, Failures: 0, Errors: 0, Skipped: 0` — `BUILD SUCCESS`. - `stopKeepsSameHerdrPaneIdSeparateByOwningAdapter` - `stopAllowsALegacyBarePaneIdWithOneDaemon` - `stopRejectsAnUnownedPaneIdWhenMultipleDaemonsCouldOwnIt` - `stopAllowsAnUnownedPaneIdWithTwoAdaptersSharingOneDaemon` — the live Mac shape

ltms added 2 commits 2026-08-28 04:45:47 +02:00

#185 : reject ambiguous unowned pane ids 25726a5ae7

#185 : count herdr owners for stop fallback

CI / contract (pull_request) Successful in 43s

Details

CI / build (pull_request) Successful in 1m8s

Details

5ba05d0bdb

ltms added 1 commit 2026-08-29 01:09:57 +02:00

#185 review: keep the stop owner on failure, key list() by daemon

CI / contract (pull_request) Successful in 43s

Details

CI / build (pull_request) Successful in 1m30s

Details

31d5516991

Three fixes on top of the pane-id PR, from my own read and the reviewer's:

- stop() removed the spawnedBy record BEFORE the delegate accepted the stop. A
  delegate that threw left the pane alive with its owner forgotten, so the retry
  fell into the ambiguous branch and refused the id for good. Remove after.
- list() deduplicated on the raw pane id. Pane ids are per-daemon counters, so
  two daemons can each hold w1:p1 on different panes, and one of the two real
  agents was silently dropped from fleet_list and every view built on it. The
  key is now (owning daemon, pane id). Delegates sharing one daemon still
  collapse, which is what the dedupe was for.
- The class javadoc still stated the single-herdr-connection premise as fact,
  next to the bullet this PR had just corrected for stop(). Fixed there too.

Also drops a redundantly qualified java.util.Collections.

Tests: 993 run, 0 failures, BUILD SUCCESS.

ltms commented

2026-08-29 01:10:16 +02:00

Lead review — approved with three fixes, pushed as `31d5516`

I read the diff myself and ran one reviewer against it. Both found real problems. I fixed them here
rather than send a 3-line change back.

1. `stop()` lost the owner when the delegate refused (reviewer)

spawnedBy.remove(id) ran before d.stop(id). HerdrPeerLauncher.stop rethrows any
pane.close error that is not "already gone", so a delegate that threw left the pane alive with its
owner forgotten. The retry then fell into the new ambiguous branch and refused the id for good — the
pane became unstoppable. The record is now dropped only after the delegate accepted the stop.

Test: stopKeepsTheOwnerRecordWhenTheDelegateRefusesTheStop.

2. `list()` deduplicated on the raw pane id (reviewer, rated high)

The PR body listed this as a known follow-up. I fixed it here instead, because #186 is what makes it
reachable and the two would land together. Pane ids are per-daemon counters, so two daemons can each
hold w1:p1 on different panes; keying on the pane id alone silently dropped one of two real agents
from fleet_list and from every status view built on it. The key is now (owning daemon, pane id).
Delegates that share one daemon still collapse — that is what the dedupe was for, and it still works.

Tests: listKeepsBothPanesWhenTwoDaemonsShareAPaneId and
listStillDeduplicatesTwoAdaptersSharingOneDaemon.

3. The class javadoc still asserted the old premise (mine)

The "Fleet-wide" bullet still said list is deduplicated by pane id "because every herdr-backed
delegate shares one herdr connection" — stated as fact, directly under the bullet this PR had just
corrected for stop. Fixed there too. Also dropped a redundantly qualified
java.util.Collections.

What I checked and did not change

herdrDaemonCount() counts distinct HerdrClient identities, not delegates. That is the right
count, and identity is the right comparison — the live Mac fleet is 2 adapter kinds on 1 daemon,
and the first revision of this PR would have thrown on it.
Single-daemon behaviour is unchanged. fleet_list output and fleet_stop input are untouched.
spawnedBy still does not survive a restart. Still a real follow-up, still strictly safer than the
old "close a pane on an arbitrary daemon".
HerdrPeerLauncher.stop has the same shape one level down: paneByAgentId.remove(idOrPane) also
runs before the close. A failed stop there drops to the raw-pane fallback rather than refusing, so
it is not the same severity. Not fixed here — noted so it is not lost.
Public AgentControl.herdr() / HerdrPeerLauncher.herdr() are wider than strictly needed. Left as
is: list() now needs the client for keying too, so a narrower sharesHerdr(...) no longer covers
the use.

Build — run by me, not piped

mvn -f fleetd/pom.xml clean install in a clean worktree at 31d5516:
Tests run: 993, Failures: 0, Errors: 0 — BUILD SUCCESS.
(990 before, plus the three tests above.)

Merging.

## Lead review — approved with three fixes, pushed as `31d5516` I read the diff myself and ran one reviewer against it. Both found real problems. I fixed them here rather than send a 3-line change back. ### 1. `stop()` lost the owner when the delegate refused (reviewer) `spawnedBy.remove(id)` ran **before** `d.stop(id)`. `HerdrPeerLauncher.stop` rethrows any `pane.close` error that is not "already gone", so a delegate that threw left the pane alive with its owner forgotten. The retry then fell into the new ambiguous branch and refused the id for good — the pane became unstoppable. The record is now dropped only after the delegate accepted the stop. Test: `stopKeepsTheOwnerRecordWhenTheDelegateRefusesTheStop`. ### 2. `list()` deduplicated on the raw pane id (reviewer, rated high) The PR body listed this as a known follow-up. I fixed it here instead, because #186 is what makes it reachable and the two would land together. Pane ids are per-daemon counters, so two daemons can each hold `w1:p1` on different panes; keying on the pane id alone silently dropped one of two real agents from `fleet_list` and from every status view built on it. The key is now (owning daemon, pane id). Delegates that share one daemon still collapse — that is what the dedupe was for, and it still works. Tests: `listKeepsBothPanesWhenTwoDaemonsShareAPaneId` and `listStillDeduplicatesTwoAdaptersSharingOneDaemon`. ### 3. The class javadoc still asserted the old premise (mine) The "Fleet-wide" bullet still said `list` is deduplicated by pane id "because every herdr-backed delegate shares one herdr connection" — stated as fact, directly under the bullet this PR had just corrected for `stop`. Fixed there too. Also dropped a redundantly qualified `java.util.Collections`. ### What I checked and did not change - `herdrDaemonCount()` counts distinct `HerdrClient` identities, not delegates. That is the right count, and identity is the right comparison — the live Mac fleet is 2 adapter kinds on 1 daemon, and the first revision of this PR would have thrown on it. - Single-daemon behaviour is unchanged. `fleet_list` output and `fleet_stop` input are untouched. - `spawnedBy` still does not survive a restart. Still a real follow-up, still strictly safer than the old "close a pane on an arbitrary daemon". - `HerdrPeerLauncher.stop` has the same shape one level down: `paneByAgentId.remove(idOrPane)` also runs before the close. A failed stop there drops to the raw-pane fallback rather than refusing, so it is not the same severity. Not fixed here — noted so it is not lost. - Public `AgentControl.herdr()` / `HerdrPeerLauncher.herdr()` are wider than strictly needed. Left as is: `list()` now needs the client for keying too, so a narrower `sharesHerdr(...)` no longer covers the use. ### Build — run by me, not piped `mvn -f fleetd/pom.xml clean install` in a clean worktree at `31d5516`: `Tests run: 993, Failures: 0, Errors: 0` — `BUILD SUCCESS`. (990 before, plus the three tests above.) Merging.

ltms merged commit a237fbff9d into main

2026-08-29 01:10:22 +02:00

ltms referenced this pull request

2026-08-29 01:11:34 +02:00

Run members on a second herdr under a dedicated user (optional; single-herdr stays the default) #185

ltms referenced this pull request

2026-08-29 01:12:01 +02:00

CB-185: route members to separate herdr #186

ltms referenced this pull request

2026-08-29 01:24:20 +02:00

CB-185: fix connection-identity/status/health gaps a second herdr daemon exposes #188

Sign in to join this conversation.