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

CompositePeerLauncher.clearContext silently stops working for every peer that survives a restart #352

New Issue
Closed
opened 2026-09-04 11:41:35 +02:00 by ltms · 1 comment
ltms commented 2026-09-04 11:41:35 +02:00
Owner
Copy Link
Copy Source

Reported by the #342 implementer under that ticket's "find what else has this shape" tail, and
correctly left unfixed as out of scope. Same root cause as #342, different failure mode.

The shape

CompositePeerLauncher routes by an in-memory spawnedBy map, so that map is empty after any
daemon restart. #342 fixed what stop() does on a cache miss. clearContext does something
different — CompositePeerLauncher.java:637-641:

@Override
public boolean clearContext(String id) {
    HerdrPeerLauncher delegate = spawnedBy.get(id);
    if (delegate == null) {
        log.debug("clearContext({}) ignored — no recorded owning adapter", id);
        return false;

It no-ops. No shortcut, no probe, and a debug log nobody has on. stop() at least had the
single-daemon shortcut; this path has nothing.

So after a restart, context reset stops happening for every still-live peer, and the only sign is a
false return value and a debug line.

I have read the method and it is as quoted. I have not measured what a false return does to the
caller, and that is the first thing this ticket needs.

First job — measure, do not fix

Two questions, in order:

  1. Who calls clearContext, and what does each caller do with false? If a caller already
    treats false as "could not, carry on" and tells somebody, the harm is much smaller than it
    looks. If it is dropped, say so and name the line.
  2. What is the real consequence of a skipped context reset? Not the shape — the outcome for a
    member. State it in one sentence, with the code path behind it.

If the answer is that nothing important depends on it, say so and close this. A fix for a defect
with no consequence costs complexity for nothing.

Goal, if it is confirmed

Goal: a peer that survives a daemon restart keeps working the same way as one that did not,
including context reset.

Invariants:

  1. A cache miss must not become a thrown exception. clearContext is called on paths that must
    tolerate a peer that is already gone.
  2. Whatever a caller does today with false must keep working. Do not change the return contract
    without naming every caller you checked.

Candidate mechanisms, as candidates only:

  1. Give it the same fallback ladder stop() has — single-daemon shortcut, then probeOwner.
    Note that probeOwner groups by HerdrClient identity, so with two delegates sharing one daemon
    it returns the first-inserted delegate; that was measured under #342 and is why the shortcut and
    the probe collapse to the same answer in a one-daemon fleet.
  2. Raise the log to warn and change nothing else — the cheapest honest option if question 1 shows
    the caller handles false properly and only the silence is wrong.
  3. Make spawnedBy survive a restart. Much larger, and it would close this whole class rather than
    one instance. Say what it would cost before proposing it.

Option 2 may well be the right answer. Do not assume the biggest fix is the correct one.

Related: #342 (the stop() half, fixed in 73aab3f).

Reported by the #342 implementer under that ticket's "find what else has this shape" tail, and correctly left unfixed as out of scope. Same root cause as #342, different failure mode. ## The shape `CompositePeerLauncher` routes by an in-memory `spawnedBy` map, so that map is empty after any daemon restart. `#342` fixed what `stop()` does on a cache miss. `clearContext` does something different — `CompositePeerLauncher.java:637-641`: ```java @Override public boolean clearContext(String id) { HerdrPeerLauncher delegate = spawnedBy.get(id); if (delegate == null) { log.debug("clearContext({}) ignored — no recorded owning adapter", id); return false; ``` It no-ops. No shortcut, no probe, and a `debug` log nobody has on. `stop()` at least had the single-daemon shortcut; this path has nothing. So after a restart, context reset stops happening for every still-live peer, and the only sign is a `false` return value and a debug line. I have read the method and it is as quoted. **I have not measured what a `false` return does to the caller**, and that is the first thing this ticket needs. ## First job — measure, do not fix Two questions, in order: 1. **Who calls `clearContext`, and what does each caller do with `false`?** If a caller already treats `false` as "could not, carry on" and tells somebody, the harm is much smaller than it looks. If it is dropped, say so and name the line. 2. **What is the real consequence of a skipped context reset?** Not the shape — the outcome for a member. State it in one sentence, with the code path behind it. If the answer is that nothing important depends on it, say so and close this. A fix for a defect with no consequence costs complexity for nothing. ## Goal, if it is confirmed **Goal:** a peer that survives a daemon restart keeps working the same way as one that did not, including context reset. **Invariants:** 1. A cache miss must not become a thrown exception. `clearContext` is called on paths that must tolerate a peer that is already gone. 2. Whatever a caller does today with `false` must keep working. Do not change the return contract without naming every caller you checked. **Candidate mechanisms, as candidates only:** 1. Give it the same fallback ladder `stop()` has — single-daemon shortcut, then `probeOwner`. Note that `probeOwner` groups by `HerdrClient` identity, so with two delegates sharing one daemon it returns the first-inserted delegate; that was measured under #342 and is why the shortcut and the probe collapse to the same answer in a one-daemon fleet. 2. Raise the log to `warn` and change nothing else — the cheapest honest option if question 1 shows the caller handles `false` properly and only the silence is wrong. 3. Make `spawnedBy` survive a restart. Much larger, and it would close this whole class rather than one instance. Say what it would cost before proposing it. Option 2 may well be the right answer. Do not assume the biggest fix is the correct one. Related: #342 (the `stop()` half, fixed in `73aab3f`).
ltms commented 2026-09-09 02:16:59 +02:00
Author
Owner
Copy Link
Copy Source

Measured 2026-09-08 UTC. This ticket describes a state the daemon cannot reach. Closing without a fix.

The ticket asked for measurement before any change, and said "if the answer is that nothing important depends on it, say so and close this". That is the answer.

What the measurement found

A worker did the sweep; I checked every load-bearing claim myself before accepting it, because a worker's conclusion is not a fact until the lead re-runs it.

clearContext has exactly one production caller.

$ grep -rn "clearContext" fleetd/src/main/java
ClaudeCodeLauncher.java:966      public boolean clearContext(String id)      <- impl
CompositePeerLauncher.java:637   public boolean clearContext(String id)      <- impl
HerdrPeerLauncher.java:314       public boolean clearContext(String id)      <- impl
PeerLauncher.java:194            boolean clearContext(String id);            <- interface
SessionManager.java:905          return launcher.clearContext(...)           <- the ONLY caller

That caller cannot run for a peer that predates the restart. SessionManager.registry is a plain ConcurrentHashMap created at field initialisation (SessionManager.java:49) and written only at spawn/acquire (:243, :660). There is no restore, rehydrate or reload path — I grepped for one and found nothing. So after a restart the registry is empty, completeTurn is never called for an older peer, and the cache-miss branch at CompositePeerLauncher.java:637-641 is never reached for one.

Separately, those peers are usually gone anyway. Fleetd.java:233-241 calls reapOrphanWorkers() once herdr answers, and HerdrPeerLauncher.reapOrphanWorkers() tears down every peer whose name carries a nonce other than this process's. Note this is a second reason, not the main one: if herdr does not answer within the wait, the log says orphans were not reaped — and even then nothing changes, because the registry is still empty.

Why the stop() twin (#342) was real and this one is not

Worth writing down, because the two look identical in the source. They differ in who calls them. stop() is reached on paths that operate on a pane id directly. clearContext is reached only through a live MemberSession, and a session is exactly the thing that does not survive the restart. Same cache, same miss, different reachability.

This is the "a defect on paper is not a reachable defect" rule doing its job. Both candidate fixes in the ticket body would have been real code protecting nothing.

Also checked

stop() is the only other production route through spawnedBy (CompositePeerLauncher.java:541-567), and it already has its own cache-miss ladder from #342. No third route exists.

What would reopen this — re-measure, do not trust this comment

This conclusion rests on one property: a member session does not survive a daemon restart. If that stops being true, this ticket becomes real again immediately.

# 1. Is there still exactly one production caller?
grep -rn "clearContext" fleetd/src/main/java

# 2. Has anything started repopulating the session registry at startup?
grep -rn "restore\|rehydrate\|loadSessions" fleetd/src/main/java/dev/ltms/fleet/session/SessionManager.java
  • Command 1 still shows SessionManager.java as the only caller, and command 2 prints nothing ⇒ leave this closed.
  • Command 2 prints anything ⇒ sessions now outlive a restart, the premise is gone, reopen and fix. Option 1 in the ticket body (the stop() fallback ladder) would then be the place to start, but re-read it first: probeOwner matches on pane ids, while clearContext needs the handle-to-pane mapping in HerdrPeerLauncher.paneByAgentId, so the ladder is not a drop-in.
  • Either command errors instead of printing ⇒ that is a third answer and it settles nothing. Fix the command before drawing a conclusion.

Delete this section when it stops reproducing. Do not annotate it and do not keep it for history.

Measured 2026-09-08 UTC. **This ticket describes a state the daemon cannot reach.** Closing without a fix. The ticket asked for measurement before any change, and said "if the answer is that nothing important depends on it, say so and close this". That is the answer. ## What the measurement found A worker did the sweep; I checked every load-bearing claim myself before accepting it, because a worker's conclusion is not a fact until the lead re-runs it. **`clearContext` has exactly one production caller.** ``` $ grep -rn "clearContext" fleetd/src/main/java ClaudeCodeLauncher.java:966 public boolean clearContext(String id) <- impl CompositePeerLauncher.java:637 public boolean clearContext(String id) <- impl HerdrPeerLauncher.java:314 public boolean clearContext(String id) <- impl PeerLauncher.java:194 boolean clearContext(String id); <- interface SessionManager.java:905 return launcher.clearContext(...) <- the ONLY caller ``` **That caller cannot run for a peer that predates the restart.** `SessionManager.registry` is a plain `ConcurrentHashMap` created at field initialisation (`SessionManager.java:49`) and written only at spawn/acquire (`:243`, `:660`). There is no restore, rehydrate or reload path — I grepped for one and found nothing. So after a restart the registry is empty, `completeTurn` is never called for an older peer, and the cache-miss branch at `CompositePeerLauncher.java:637-641` is never reached for one. **Separately, those peers are usually gone anyway.** `Fleetd.java:233-241` calls `reapOrphanWorkers()` once herdr answers, and `HerdrPeerLauncher.reapOrphanWorkers()` tears down every peer whose name carries a nonce other than this process's. Note this is a *second* reason, not the main one: if herdr does not answer within the wait, the log says orphans were **not** reaped — and even then nothing changes, because the registry is still empty. ## Why the `stop()` twin (#342) was real and this one is not Worth writing down, because the two look identical in the source. They differ in who calls them. `stop()` is reached on paths that operate on a pane id directly. `clearContext` is reached only through a live `MemberSession`, and a session is exactly the thing that does not survive the restart. Same cache, same miss, different reachability. This is the "a defect on paper is not a reachable defect" rule doing its job. Both candidate fixes in the ticket body would have been real code protecting nothing. ## Also checked `stop()` is the only other production route through `spawnedBy` (`CompositePeerLauncher.java:541-567`), and it already has its own cache-miss ladder from #342. No third route exists. ## What would reopen this — re-measure, do not trust this comment This conclusion rests on one property: **a member session does not survive a daemon restart.** If that stops being true, this ticket becomes real again immediately. ```bash # 1. Is there still exactly one production caller? grep -rn "clearContext" fleetd/src/main/java # 2. Has anything started repopulating the session registry at startup? grep -rn "restore\|rehydrate\|loadSessions" fleetd/src/main/java/dev/ltms/fleet/session/SessionManager.java ``` - Command 1 still shows `SessionManager.java` as the only caller, **and** command 2 prints nothing ⇒ leave this closed. - Command 2 prints anything ⇒ sessions now outlive a restart, the premise is gone, **reopen and fix**. Option 1 in the ticket body (the `stop()` fallback ladder) would then be the place to start, but re-read it first: `probeOwner` matches on pane ids, while `clearContext` needs the handle-to-pane mapping in `HerdrPeerLauncher.paneByAgentId`, so the ladder is not a drop-in. - Either command errors instead of printing ⇒ that is a third answer and it settles nothing. Fix the command before drawing a conclusion. **Delete this section when it stops reproducing.** Do not annotate it and do not keep it for history.
ltms closed this issue 2026-09-09 02:17:04 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
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
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#352
Delete Branch "%!s()"

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?