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

Act on a HIGH lead context: tell the idle lead to hand over #609

New Issue
Open
opened 2026-09-20 11:54:04 +02:00 by ltms · 3 comments
ltms commented 2026-09-20 11:54:04 +02:00
Owner
Copy Link
Copy Source

Why

LeadContextGauge (#602, #606) can now read how full a lead's own Claude Code context is,
and fleet_list reports it per lead row as {state: "ok"|"high"|"unknown", tokens?, compactions}.

Measured on this host just now, on the live daemon (jar 597af9057197):

"context":{"state":"high","tokens":260771,"compactions":1}

So fleetd can see that a lead is full. Nothing acts on it. No lead is told, and no handover is
offered. The lead keeps working until Claude Code compacts it, which is the thing the operator
asked us to stop:

the session compact too regularly - we urgently need handoff feature, fleetd should track
leader and help with refresh/handoff to ensure efficient context for fleet's operations
— operator, 2026-09-19

fleet_handover already exists (#480) and has run end-to-end once. The missing half is the
notice: something that reads the gauge and tells the lead, once, that it should hand over.

Where it goes

LeadHeartbeatLoop (CB-551), fleetd/src/main/java/dev/ltms/fleet/msg/LeadHeartbeatLoop.java.

It already has every piece this needs: an opt-in config gate, a status gate, an idle debounce, a
nudge cap, a stand-down against ReplyPushLoop, and a pure decide(...) function that is unit
testable with a fake clock.

Hard limits

  1. The daemon must never roll a lead by itself. This unit only ever sends text. fleet_handover
    keeps needing operatorConfirmed, and requireOperatorConfirm stays true.
  2. Opt-in. A daemon upgraded with this change and no new config key must behave exactly as it
    does today.
  3. Tell the lead once per HIGH stretch, not every tick. A full lead is the one we least want to
    spend turns on.
  4. Re-arm only on a positive OK reading. UNKNOWN means "I could not look", not "it got
    better". Re-arming on UNKNOWN would let a flapping gauge nudge the lead again and again.

Note for whoever picks this up

leadHeartbeat: is absent from this host's fleetd.yaml today (measured 2026-09-20:
grep -n 'leadHeartbeat' fleetd/fleetd.yaml returns nothing; only leadRollover: at line 563).
So the heartbeat loop is not constructed on the live fleet, and this feature will be inert here
until that block is added. That is a separate config change, not part of this unit.

## Why `LeadContextGauge` (#602, #606) can now read how full a lead's own Claude Code context is, and `fleet_list` reports it per lead row as `{state: "ok"|"high"|"unknown", tokens?, compactions}`. Measured on this host just now, on the live daemon (jar `597af9057197`): ``` "context":{"state":"high","tokens":260771,"compactions":1} ``` So fleetd can **see** that a lead is full. Nothing acts on it. No lead is told, and no handover is offered. The lead keeps working until Claude Code compacts it, which is the thing the operator asked us to stop: > the session compact too regularly - we urgently need handoff feature, fleetd should track > leader and help with refresh/handoff to ensure efficient context for fleet's operations > — operator, 2026-09-19 `fleet_handover` already exists (#480) and has run end-to-end once. The missing half is the **notice**: something that reads the gauge and tells the lead, once, that it should hand over. ## Where it goes `LeadHeartbeatLoop` (CB-551), `fleetd/src/main/java/dev/ltms/fleet/msg/LeadHeartbeatLoop.java`. It already has every piece this needs: an opt-in config gate, a status gate, an idle debounce, a nudge cap, a stand-down against `ReplyPushLoop`, and a pure `decide(...)` function that is unit testable with a fake clock. ## Hard limits 1. **The daemon must never roll a lead by itself.** This unit only ever sends text. `fleet_handover` keeps needing `operatorConfirmed`, and `requireOperatorConfirm` stays `true`. 2. **Opt-in.** A daemon upgraded with this change and no new config key must behave exactly as it does today. 3. **Tell the lead once per HIGH stretch**, not every tick. A full lead is the one we least want to spend turns on. 4. **Re-arm only on a positive `OK` reading.** `UNKNOWN` means "I could not look", not "it got better". Re-arming on `UNKNOWN` would let a flapping gauge nudge the lead again and again. ## Note for whoever picks this up `leadHeartbeat:` is **absent** from this host's `fleetd.yaml` today (measured 2026-09-20: `grep -n 'leadHeartbeat' fleetd/fleetd.yaml` returns nothing; only `leadRollover:` at line 563). So the heartbeat loop is not constructed on the live fleet, and this feature will be inert here until that block is added. That is a separate config change, not part of this unit.
ltms commented 2026-09-20 11:55:39 +02:00
Author
Owner
Copy Link
Copy Source

Correction to the brief — one garbled sentence

My brief said, about the nudge text:

reading.tokens() can be null even when the state is HIGH is NOT true today, but do not
assume it

That sentence is broken. Here is what I meant, and it is what you should build:

Today a HIGH reading always carries a non-null tokens. LeadContextGauge only sets
HIGH after comparing a token count against HIGH_THRESHOLD_TOKENS, so it cannot reach HIGH
with no number. I checked this in LeadContextGauge.java.

Still write the null branch. That invariant lives in another class, nothing asserts it, and
the nudge text is the wrong place to discover it is gone. If tokens() is null, drop that clause
from the sentence rather than printing null. Add one test for it.

This is a "free test case": a comment claiming an invariant is worth asserting, because the next
reader widens a gate on its strength.

Second thing, same brief

The brief tells you to keep the existing two public LeadHeartbeatLoop constructors working by
delegating them to a new one. To be exact about what "working" means: no existing test file may
need an edit to compile. If one does, that is a signal the new constructor's defaults are wrong,
not a signal to edit the test. Say so in your reply if it happens.

## Correction to the brief — one garbled sentence My brief said, about the nudge text: > `reading.tokens()` can be null even when the state is HIGH is NOT true today, but do not > assume it That sentence is broken. Here is what I meant, and it is what you should build: **Today a `HIGH` reading always carries a non-null `tokens`.** `LeadContextGauge` only sets `HIGH` after comparing a token count against `HIGH_THRESHOLD_TOKENS`, so it cannot reach `HIGH` with no number. I checked this in `LeadContextGauge.java`. **Still write the null branch.** That invariant lives in another class, nothing asserts it, and the nudge text is the wrong place to discover it is gone. If `tokens()` is null, drop that clause from the sentence rather than printing `null`. Add one test for it. This is a "free test case": a comment claiming an invariant is worth asserting, because the next reader widens a gate on its strength. ## Second thing, same brief The brief tells you to keep the existing two public `LeadHeartbeatLoop` constructors working by delegating them to a new one. To be exact about what "working" means: **no existing test file may need an edit to compile.** If one does, that is a signal the new constructor's defaults are wrong, not a signal to edit the test. Say so in your reply if it happens.
ltms commented 2026-09-20 12:22:53 +02:00
Author
Owner
Copy Link
Copy Source

Review of PR #610 — one blocker, one known gap, one comment already fixed

I built the branch myself in a clean worktree: mvn -o clean install, exit 0,
1860 tests, 0 failures, 0 errors (149 surefire reports). Both required mutations went red on
the right tests. The implementation is good. Two reviewers found one real defect between them.

I already pushed d7390cc to the branch, repairing a garbled comment the worker copied from my
own brief. Code was fine; only the comment was unreadable.

BLOCKER — the latch means "decide() chose to tell", but it must mean "the text reached the pane"

LeadHeartbeatLoop.tick() commits the latch before it tries to send:

Decision d = decide(...);
applyDecision(d);                              // contextNotified = true, committed here
switch (d.action()) {
    case INJECT -> injectNudge(fleet, reading);  // agents.send(...) may throw

and injectNudge swallows the failure:

} catch (RuntimeException e) {
    log.warn("idle-heartbeat: failed to nudge lead {}: {}", leadTerminal, e.toString());
    countNudge("failed");
}

So one transient herdr failure marks the lead as told when nothing reached its pane. The lead
then gets zero notices for the rest of the HIGH stretch — the exact moment the whole feature
exists for.

On the config this fleet will actually run, this is the normal path, not an edge case. The
operator chose quietNudgeCap: 0 (2026-09-20), so the quietCount < quietNudgeCap gate can
never fire and there is no later nudge to carry the notice by accident. One failed send loses it
permanently.

Second half of the same flaw

contextNotice(boolean enabled, LeadContextGauge.Reading reading) takes no latch at all:

if (!enabled || reading.state() != LeadContextGauge.State.HIGH) {
    return "";
}

So every pending-driven INJECT re-appends the full 368-character notice while the context stays
HIGH. That makes the notice's own closing sentence false:

You will not be told again until your context reads ok.

These are not two bugs. They are one design flaw seen from both ends: the latch is set by the
decision, and read by nothing that knows whether the send worked.

The fix

Make the latch mean "the notice text was actually delivered to the pane", and make both the
decision gate and the text agree on it:

  1. injectNudge reports whether agents.send returned without throwing.
  2. tick persists contextNotified as true only when the decision said to notify and a
    notice was included and the send succeeded. idleSinceNanos and quietCount keep being
    applied unconditionally, exactly as today.
  3. contextNotice gates on the latch as well, so a nudge only carries the notice when the lead
    has not already been told in this HIGH stretch. Then "once per HIGH stretch" is true on all
    three INJECT routes, not just the forced one, and the sentence above stops being a lie.

Keep the existing latch re-arm rule unchanged: only a positive OK reading clears it, never
UNKNOWN.

NOT a blocker — the main call-site gap is class-wide and pre-existing

The second reviewer reported that no test pins Fleetd.java:577, so swapping
leadContextSource(...) for LeadContextSource.none() would compile and leave the feature
permanently silent.

That is true, and the PR says so itself. FleetdLeadContextSourceWiringTest's own javadoc:

What this class does not and cannot cover: main's own one-line call to this factory could
itself be swapped for LeadContextSource.none(), bypassing this factory entirely — the same
structural gap FleetdLeadConfigDirSourceWiringTest names for its own factory, and for the same
reason (no test in this codebase calls Fleetd.main far enough to observe which factory call it
made).

So #610 pins its factory to exactly the standard #602/#606 set, and is no worse than the
precedent. Holding this PR to a bar nothing else in the codebase meets would block the operator's
top-priority feature on a pre-existing debt.

A sweep is measuring the whole family right now, by mutating each main call site to its inert
variant and running the full suite. That will get one ticket covering every site, rather than one
PR paying for all of them. Do not add a main-level test in this PR.

## Review of PR #610 — one blocker, one known gap, one comment already fixed I built the branch myself in a clean worktree: `mvn -o clean install`, exit 0, **1860 tests, 0 failures, 0 errors** (149 surefire reports). Both required mutations went red on the right tests. The implementation is good. Two reviewers found one real defect between them. I already pushed `d7390cc` to the branch, repairing a garbled comment the worker copied from my own brief. Code was fine; only the comment was unreadable. --- ## BLOCKER — the latch means "decide() chose to tell", but it must mean "the text reached the pane" `LeadHeartbeatLoop.tick()` commits the latch **before** it tries to send: ```java Decision d = decide(...); applyDecision(d); // contextNotified = true, committed here switch (d.action()) { case INJECT -> injectNudge(fleet, reading); // agents.send(...) may throw ``` and `injectNudge` swallows the failure: ```java } catch (RuntimeException e) { log.warn("idle-heartbeat: failed to nudge lead {}: {}", leadTerminal, e.toString()); countNudge("failed"); } ``` So one transient herdr failure marks the lead as told when nothing reached its pane. The lead then gets **zero** notices for the rest of the HIGH stretch — the exact moment the whole feature exists for. **On the config this fleet will actually run, this is the normal path, not an edge case.** The operator chose `quietNudgeCap: 0` (2026-09-20), so the `quietCount < quietNudgeCap` gate can never fire and there is no later nudge to carry the notice by accident. One failed send loses it permanently. ### Second half of the same flaw `contextNotice(boolean enabled, LeadContextGauge.Reading reading)` takes no latch at all: ```java if (!enabled || reading.state() != LeadContextGauge.State.HIGH) { return ""; } ``` So every pending-driven `INJECT` re-appends the full 368-character notice while the context stays HIGH. That makes the notice's own closing sentence false: > You will not be told again until your context reads ok. These are not two bugs. They are one design flaw seen from both ends: **the latch is set by the decision, and read by nothing that knows whether the send worked.** ### The fix Make the latch mean "the notice text was actually delivered to the pane", and make both the decision gate and the text agree on it: 1. `injectNudge` reports whether `agents.send` returned without throwing. 2. `tick` persists `contextNotified` as *true* only when the decision said to notify **and** a notice was included **and** the send succeeded. `idleSinceNanos` and `quietCount` keep being applied unconditionally, exactly as today. 3. `contextNotice` gates on the latch as well, so a nudge only carries the notice when the lead has not already been told in this HIGH stretch. Then "once per HIGH stretch" is true on all three INJECT routes, not just the forced one, and the sentence above stops being a lie. Keep the existing latch re-arm rule unchanged: only a positive `OK` reading clears it, never `UNKNOWN`. --- ## NOT a blocker — the `main` call-site gap is class-wide and pre-existing The second reviewer reported that no test pins `Fleetd.java:577`, so swapping `leadContextSource(...)` for `LeadContextSource.none()` would compile and leave the feature permanently silent. That is true, and the PR says so itself. `FleetdLeadContextSourceWiringTest`'s own javadoc: > What this class does not and cannot cover: `main`'s own one-line call to this factory could > itself be swapped for `LeadContextSource.none()`, bypassing this factory entirely — the same > structural gap `FleetdLeadConfigDirSourceWiringTest` names for its own factory, and for the same > reason (no test in this codebase calls `Fleetd.main` far enough to observe which factory call it > made). So #610 pins its factory to exactly the standard #602/#606 set, and is no worse than the precedent. Holding this PR to a bar nothing else in the codebase meets would block the operator's top-priority feature on a pre-existing debt. A sweep is measuring the whole family right now, by mutating each `main` call site to its inert variant and running the full suite. That will get one ticket covering every site, rather than one PR paying for all of them. **Do not add a `main`-level test in this PR.**
ltms commented 2026-09-20 12:38:09 +02:00
Author
Owner
Copy Link
Copy Source

Merged as 955b9ea (PR #610).

The merged tree is byte-identical to the tree I built and mutation-tested (52007dc5d6a953a9bdfdaf30793f644d519bc6ac), so these numbers apply to main exactly rather than to a similar tree:

  • 1864 tests, 0 failures, 0 errors, 149 surefire reports, mvn exit 0.
  • Four mutations, all killed. Two were the implementer's, re-run by me. One was mine, placed at the line the logic moved to — setting the latch regardless of the send outcome — which is the check that proves the tests pin the invariant and not just the old code location. The fourth was a control on a line nobody touched (quietCount < quietNudgeCap to <=), which is what makes the other three kills evidence rather than an absence of evidence.

Still to do before this does anything on this host

A merge is not a deployment. The running daemon holds the jar it started with, and leadHeartbeat: is in ConfigRef.DEFERRED_KEYS, so the config half is frozen too. I am holding the redeploy until the worker on the CI fix finishes, because a restart drops in-flight tickets.

Configuration chosen for this host

The operator chose the narrow shape on 2026-09-20, after being shown both:

leadHeartbeat:
  idleAfterSeconds: 300
  backoffMs: 60000
  quietNudgeCap: 0          # never a content-free nudge
  contextHighNudge: true

quietNudgeCap: 0 is legal because the compact constructor rejects < 0, not <= 0. With it, the loop can nudge the lead in only two cases: real fleet work is waiting, or the lead's own context reads HIGH and it has not been told yet in this stretch. It can never send "you are idle, nothing is pending".

That choice is also why the latch defect was a blocker rather than a nuisance. With no quiet nudges, a nudge carrying the notice is the only nudge, so one swallowed herdr failure would have marked the lead as told when nothing reached its pane, with no later nudge to carry the notice by accident.

Not fixed here

Fleetd.main's own one-line call to leadContextSource is still unpinned. Pre-existing and class-wide — tracked in #612.

Merged as `955b9ea` (PR #610). The merged tree is byte-identical to the tree I built and mutation-tested (`52007dc5d6a953a9bdfdaf30793f644d519bc6ac`), so these numbers apply to main exactly rather than to a similar tree: - 1864 tests, 0 failures, 0 errors, 149 surefire reports, `mvn` exit 0. - Four mutations, all killed. Two were the implementer's, re-run by me. One was mine, placed at the line the logic moved **to** — setting the latch regardless of the send outcome — which is the check that proves the tests pin the invariant and not just the old code location. The fourth was a control on a line nobody touched (`quietCount < quietNudgeCap` to `<=`), which is what makes the other three kills evidence rather than an absence of evidence. ## Still to do before this does anything on this host **A merge is not a deployment.** The running daemon holds the jar it started with, and `leadHeartbeat:` is in `ConfigRef.DEFERRED_KEYS`, so the config half is frozen too. I am holding the redeploy until the worker on the CI fix finishes, because a restart drops in-flight tickets. ## Configuration chosen for this host The operator chose the narrow shape on 2026-09-20, after being shown both: ```yaml leadHeartbeat: idleAfterSeconds: 300 backoffMs: 60000 quietNudgeCap: 0 # never a content-free nudge contextHighNudge: true ``` `quietNudgeCap: 0` is legal because the compact constructor rejects `< 0`, not `<= 0`. With it, the loop can nudge the lead in only two cases: real fleet work is waiting, or the lead's own context reads HIGH and it has not been told yet in this stretch. It can never send "you are idle, nothing is pending". That choice is also why the latch defect was a blocker rather than a nuisance. With no quiet nudges, a nudge carrying the notice is the only nudge, so one swallowed herdr failure would have marked the lead as told when nothing reached its pane, with no later nudge to carry the notice by accident. ## Not fixed here `Fleetd.main`'s own one-line call to `leadContextSource` is still unpinned. Pre-existing and class-wide — tracked in #612.
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#609
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?