Compare commits
4 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| cbe872b538 | |||
| 8915e40c7d | |||
| 6cb31a10e4 | |||
| 8368a274a0 |
@@ -577,13 +577,18 @@ public final class Fleetd {
|
||||
// (keyed by configDir+sessionId), so this costs at most one extra bounded tail read per
|
||||
// TTL window, never a shared-mutable-state hazard between the two callers.
|
||||
var leadContextGauge = new LeadContextGauge();
|
||||
// fleetd #621: the context-high notice's own wording must track this same effective
|
||||
// value — LeadRollover.confirm(...) already gates the roll on it (LeadRollover.java:480),
|
||||
// and absent `leadRollover:` entirely the roll is unusable regardless (NOT_CONFIGURED),
|
||||
// so `true` (the FleetConfig.LeadRollover default) is the safe, byte-identical fallback.
|
||||
boolean requireOperatorConfirm = cfg.leadRollover() == null || cfg.leadRollover().requireOperatorConfirm();
|
||||
heartbeat = new LeadHeartbeatLoop(primaryRegistry, router.leadAgents(), replyInbox, sessions::roster,
|
||||
pushLoop, heartbeatScheduler, System::nanoTime,
|
||||
TimeUnit.SECONDS.toNanos(hb.idleAfterSeconds()), hb.backoffMs(), hb.quietNudgeCap(),
|
||||
metrics,
|
||||
leadContextSource(leadContextGauge, router.leadAgents(), leads,
|
||||
leadConfigDirLookup(() -> config.get().profiles(), leaders)),
|
||||
Boolean.TRUE.equals(hb.contextHighNudge()));
|
||||
Boolean.TRUE.equals(hb.contextHighNudge()), requireOperatorConfirm);
|
||||
heartbeat.start();
|
||||
} else {
|
||||
heartbeat = null;
|
||||
|
||||
@@ -2240,11 +2240,11 @@ public record FleetConfig(
|
||||
* information — which profiles, and now both values, so they can fix it without reading the
|
||||
* source — without ever taking the fleet down.
|
||||
*
|
||||
* <p>Which of the two inputs Claude Code actually follows when they disagree is intentionally
|
||||
* <em>not</em> asserted here. {@code ClaudeCodeArguments}'s javadoc used to state the
|
||||
* environment variable always wins; nobody had measured that, and this host's own
|
||||
* {@code fleetd.yaml} asserts the opposite in a comment. This method only detects and reports
|
||||
* the disagreement — see {@link dev.ltms.fleet.launch.ClaudeCodeArguments}.
|
||||
* <p>fleetd #618 measured which of the two inputs Claude Code actually follows when they
|
||||
* disagree: the environment variable wins, so {@code autoCompactWindow} is inert on a profile
|
||||
* that also sets the env var. This method only detects and reports the disagreement — it does
|
||||
* not correct it — see {@link dev.ltms.fleet.launch.ClaudeCodeArguments} for the full measured
|
||||
* precedence.
|
||||
*
|
||||
* <p>Equal values never warn: either input then produces the same session window, so there is
|
||||
* nothing to reconcile.
|
||||
@@ -2285,9 +2285,11 @@ public record FleetConfig(
|
||||
names.sort(String::compareTo);
|
||||
detail.sort(String::compareTo);
|
||||
log.warn("Claude Code profile(s) {} set disagreeing autoCompactWindow and env."
|
||||
+ "CLAUDE_CODE_AUTO_COMPACT_WINDOW — the daemon starts anyway. Fix by "
|
||||
+ "removing one key or setting equal values on each: {}. Which input Claude "
|
||||
+ "Code actually follows when they disagree is not verified here.",
|
||||
+ "CLAUDE_CODE_AUTO_COMPACT_WINDOW — the daemon starts anyway: {}. fleetd "
|
||||
+ "#618 measured that CLAUDE_CODE_AUTO_COMPACT_WINDOW wins, so "
|
||||
+ "autoCompactWindow is inert on these profiles. Set equal values on each "
|
||||
+ "to resolve this — do not just delete the env var, since that LOWERS the "
|
||||
+ "live window to autoCompactWindow's value rather than fixing anything.",
|
||||
names, String.join(", ", detail));
|
||||
}
|
||||
|
||||
|
||||
@@ -15,13 +15,15 @@ public final class ClaudeCodeArguments {
|
||||
* Append the configured Claude Code auto-compaction window when the profile opts in.
|
||||
*
|
||||
* <p>This flag and the environment variable {@code CLAUDE_CODE_AUTO_COMPACT_WINDOW} can
|
||||
* disagree. Which one Claude Code actually follows when they do is NOT verified here — this
|
||||
* javadoc used to claim the environment variable always wins, but nobody had measured that, and
|
||||
* this host's own {@code fleetd.yaml} asserts the opposite in a comment. So this javadoc no
|
||||
* longer picks a side. {@link FleetConfig#load(java.nio.file.Path)} only WARNS when a Claude
|
||||
* Code profile sets both to different values (see {@code
|
||||
* disagree, and fleetd #618 measured which one Claude Code actually follows: the environment
|
||||
* variable wins, ahead of this {@code --autocompact} flag, ahead of the settings file, ahead of
|
||||
* clientdata, the experiment, and the model default. So when a profile sets both, the flag this
|
||||
* method appends has NO effect — Claude Code reads {@code CLAUDE_CODE_AUTO_COMPACT_WINDOW}
|
||||
* first and never consults the flag. {@link FleetConfig#load(java.nio.file.Path)} only WARNS
|
||||
* when a Claude Code profile sets both to different values (see {@code
|
||||
* FleetConfig.warnConflictingAutoCompactWindows}) — it does not stop the daemon from starting,
|
||||
* and a launched session may end up honouring either window.
|
||||
* and the launched session honours the env var, not this flag. Measured against Claude Code
|
||||
* 2.1.278 (fleetd #618) — a later version could reorder this precedence.
|
||||
*/
|
||||
public static List<String> withAutoCompactWindow(List<String> argv, FleetConfig.Profile profile) {
|
||||
if (profile.autoCompactWindow() == null) {
|
||||
|
||||
@@ -76,6 +76,7 @@ public final class LeadHeartbeatLoop {
|
||||
private final Metrics metrics; // CB-512 pattern: nullable — no registry in unit tests
|
||||
private final LeadContextSource contextSource; // fleetd #609
|
||||
private final boolean contextHighNudge; // fleetd #609: opt-in, like the loop itself
|
||||
private final boolean requireOperatorConfirm; // fleetd #621: mirrors leadRollover.requireOperatorConfirm
|
||||
|
||||
/** When the current idle stretch began (nanos), or {@link #NOT_IDLE}. Single scheduler thread only. */
|
||||
private long idleSinceNanos = NOT_IDLE;
|
||||
@@ -106,12 +107,32 @@ public final class LeadHeartbeatLoop {
|
||||
* fleetd #609: as above, plus the lead's own context source and whether a HIGH reading should
|
||||
* append a hand-over notice to the loop's nudge. Pass {@link LeadContextSource#none()} and
|
||||
* {@code false} to keep the pre-#609 behaviour exactly (both existing public constructors do).
|
||||
*
|
||||
* <p>fleetd #621: delegates to the full constructor with {@code requireOperatorConfirm=true} —
|
||||
* the pre-#621 wording ("ask the operator ... only the operator can approve the roll") assumed
|
||||
* the config default, so every caller of this overload keeps that text byte-identical.
|
||||
*/
|
||||
public LeadHeartbeatLoop(PrimaryRegistry primaryRegistry, AgentControl agents, ReplyInbox inbox,
|
||||
Supplier<List<MemberSession>> roster, ReplyPushLoop pushLoop,
|
||||
ScheduledExecutorService scheduler, LongSupplier clock,
|
||||
long idleAfterNanos, long backoffMs, int quietNudgeCap, Metrics metrics,
|
||||
LeadContextSource contextSource, boolean contextHighNudge) {
|
||||
this(primaryRegistry, agents, inbox, roster, pushLoop, scheduler, clock,
|
||||
idleAfterNanos, backoffMs, quietNudgeCap, metrics, contextSource, contextHighNudge, true);
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #621: as above, plus the daemon's effective {@code leadRollover.requireOperatorConfirm}
|
||||
* value — threaded into {@link #contextNotice(boolean, LeadContextGauge.Reading, boolean, boolean)}
|
||||
* so the notice's wording tracks the config the daemon actually enforces (see {@code
|
||||
* LeadRollover.confirm}) instead of always asserting the operator gate is on.
|
||||
*/
|
||||
public LeadHeartbeatLoop(PrimaryRegistry primaryRegistry, AgentControl agents, ReplyInbox inbox,
|
||||
Supplier<List<MemberSession>> roster, ReplyPushLoop pushLoop,
|
||||
ScheduledExecutorService scheduler, LongSupplier clock,
|
||||
long idleAfterNanos, long backoffMs, int quietNudgeCap, Metrics metrics,
|
||||
LeadContextSource contextSource, boolean contextHighNudge,
|
||||
boolean requireOperatorConfirm) {
|
||||
this.primaryRegistry = primaryRegistry;
|
||||
this.agents = agents;
|
||||
this.inbox = inbox;
|
||||
@@ -125,6 +146,7 @@ public final class LeadHeartbeatLoop {
|
||||
this.metrics = metrics;
|
||||
this.contextSource = contextSource;
|
||||
this.contextHighNudge = contextHighNudge;
|
||||
this.requireOperatorConfirm = requireOperatorConfirm;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -344,7 +366,7 @@ public final class LeadHeartbeatLoop {
|
||||
// d.contextNotified() is the value to persist once delivery is confirmed, not the value the text
|
||||
// itself should be built from. Otherwise a HIGH stretch that is still latched would never see the
|
||||
// notice at all, defeating the very check this fixes.
|
||||
String notice = contextNotice(contextHighNudge, reading, contextNotified);
|
||||
String notice = contextNotice(contextHighNudge, reading, contextNotified, requireOperatorConfirm);
|
||||
var lead = primaryRegistry.primaryTerminal();
|
||||
boolean sent = lead.isPresent() && trySend(lead.get(), fleet.nudgeText() + notice, notice);
|
||||
// The latch becomes true only when all three hold: decide() chose to notify, a notice was
|
||||
@@ -381,8 +403,9 @@ public final class LeadHeartbeatLoop {
|
||||
/**
|
||||
* fleetd #609: the text appended to a nudge when the lead's own context is full — {@code ""}
|
||||
* whenever the notice does not apply, so callers can unconditionally append this without an extra
|
||||
* branch. Wording stays plain (CEFR B1) and honest that only the operator approves a roll — this
|
||||
* loop only ever prints text, it never calls {@code fleet_handover} itself.
|
||||
* branch. Wording stays plain (CEFR B1) and honest about who actually gates the roll — see the
|
||||
* {@code requireOperatorConfirm} overload (fleetd #621) for which check that is. This loop only
|
||||
* ever prints text, it never calls {@code fleet_handover} itself.
|
||||
*
|
||||
* @param enabled the {@code leadHeartbeat.contextHighNudge} config flag
|
||||
* @param reading the lead's current {@link LeadContextGauge} reading
|
||||
@@ -401,9 +424,33 @@ public final class LeadHeartbeatLoop {
|
||||
* closing sentence ("You will not be told again until your context reads ok.") false. {@link
|
||||
* #injectNudge} is the only caller that passes a non-default {@code alreadyNotified}.
|
||||
*
|
||||
* <p>fleetd #621: delegates with {@code requireOperatorConfirm=true} — the pre-#621 default and the
|
||||
* value every existing caller of this overload (including every test written before #621) already
|
||||
* assumed, so the text this overload returns stays byte-identical.
|
||||
*
|
||||
* @param alreadyNotified whether the lead has already been told about the current HIGH stretch
|
||||
*/
|
||||
static String contextNotice(boolean enabled, LeadContextGauge.Reading reading, boolean alreadyNotified) {
|
||||
return contextNotice(enabled, reading, alreadyNotified, true);
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #621: as {@link #contextNotice(boolean, LeadContextGauge.Reading, boolean)}, but the closing
|
||||
* instructions also track the daemon's effective {@code leadRollover.requireOperatorConfirm} value,
|
||||
* instead of always asserting that only the operator can approve the roll.
|
||||
*
|
||||
* <p>{@code LeadRollover.confirm(...)} already honours this flag: when it is {@code false}, the daemon
|
||||
* itself gates the roll on the three handover-file checks alone (exists, modified after the {@code
|
||||
* open()} request, and no older than {@code maxDocAgeSeconds}) and never consults {@code
|
||||
* operatorConfirmed}. Before this parameter existed, this notice told the lead to ask the operator
|
||||
* regardless — so a lead that followed its own instructions asked anyway, and setting the config knob
|
||||
* to {@code false} stopped the daemon refusing the roll without stopping the operator being
|
||||
* interrupted. This parameter is how the text is kept honest about which gate is actually live.
|
||||
*
|
||||
* @param requireOperatorConfirm the effective {@code leadRollover.requireOperatorConfirm} value
|
||||
*/
|
||||
static String contextNotice(boolean enabled, LeadContextGauge.Reading reading, boolean alreadyNotified,
|
||||
boolean requireOperatorConfirm) {
|
||||
if (!enabled || alreadyNotified || reading.state() != LeadContextGauge.State.HIGH) {
|
||||
return "";
|
||||
}
|
||||
@@ -420,10 +467,18 @@ public final class LeadHeartbeatLoop {
|
||||
sb.append(" (").append(reading.compactions()).append(' ').append(compactionWord)
|
||||
.append(" so far).");
|
||||
}
|
||||
sb.append(" A fresh session would work better. To hand over: call fleet_handover(action=\"open\"), "
|
||||
+ "write the file it names, ask the operator, then call fleet_handover(action=\"confirm\", "
|
||||
+ "token, operatorConfirmed). Only the operator can approve the roll. You will not be told "
|
||||
+ "again until your context reads ok.");
|
||||
if (requireOperatorConfirm) {
|
||||
sb.append(" A fresh session would work better. To hand over: call fleet_handover(action=\"open\"), "
|
||||
+ "write the file it names, ask the operator, then call fleet_handover(action=\"confirm\", "
|
||||
+ "token, operatorConfirmed). Only the operator can approve the roll. You will not be told "
|
||||
+ "again until your context reads ok.");
|
||||
} else {
|
||||
sb.append(" A fresh session would work better. To hand over: call fleet_handover(action=\"open\"), "
|
||||
+ "write the file it names, then call fleet_handover(action=\"confirm\", token). Decide for "
|
||||
+ "yourself when to confirm: the roll goes through if the handover file exists, was "
|
||||
+ "changed after you opened it, and is not older than maxDocAgeSeconds. You will not be "
|
||||
+ "told again until your context reads ok.");
|
||||
}
|
||||
return sb.toString();
|
||||
}
|
||||
|
||||
|
||||
@@ -476,6 +476,26 @@ class LeadHeartbeatLoopTest {
|
||||
assertTrue(notice.contains("2 compactions"), notice);
|
||||
}
|
||||
|
||||
// ── fleetd #621: the notice must track the effective requireOperatorConfirm value ─────────────
|
||||
|
||||
@Test
|
||||
void contextNoticeKeepsAskingTheOperatorWhenRequireOperatorConfirmIsTrue() {
|
||||
var reading = new LeadContextGauge.Reading(LeadContextGauge.State.HIGH, 260_771L, 1);
|
||||
String notice = LeadHeartbeatLoop.contextNotice(true, reading, false, true);
|
||||
assertTrue(notice.contains("ask the operator"), notice);
|
||||
assertTrue(notice.contains("Only the operator can approve the roll"), notice);
|
||||
}
|
||||
|
||||
@Test
|
||||
void contextNoticeDropsTheOperatorAskWhenRequireOperatorConfirmIsFalse() {
|
||||
var reading = new LeadContextGauge.Reading(LeadContextGauge.State.HIGH, 260_771L, 1);
|
||||
String notice = LeadHeartbeatLoop.contextNotice(true, reading, false, false);
|
||||
assertFalse(notice.contains("ask the operator"), notice);
|
||||
assertFalse(notice.contains("Only the operator can approve the roll"), notice);
|
||||
assertTrue(notice.contains("fleet_handover"), notice);
|
||||
assertTrue(notice.contains("maxDocAgeSeconds"), notice);
|
||||
}
|
||||
|
||||
// ── fleetd #609 review: the latch must mean "the notice reached the pane" ────────────────────
|
||||
//
|
||||
// These four drive LeadHeartbeatLoop.tick() directly (package-private, same reasoning as
|
||||
|
||||
Reference in New Issue
Block a user