From cbe872b5387ca65677217812e662236383fa8383 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Tue, 22 Sep 2026 11:27:17 +0700 Subject: [PATCH] fleetd #621: make the context-roll notice obey requireOperatorConfirm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit contextNotice hardcoded 'ask the operator' and 'Only the operator can approve the roll', so setting leadRollover.requireOperatorConfirm to false stopped the daemon refusing the roll but never stopped the lead being told to ask. Thread the effective config value into contextNotice: when true the text stays byte-identical, when false it tells the lead to confirm on its own judgement against the three handover-file checks instead. LeadRollover.confirm's own enforcement is untouched — this is the message only. --- .../src/main/java/dev/ltms/fleet/Fleetd.java | 7 +- .../dev/ltms/fleet/msg/LeadHeartbeatLoop.java | 69 +++++++++++++++++-- .../ltms/fleet/msg/LeadHeartbeatLoopTest.java | 20 ++++++ 3 files changed, 88 insertions(+), 8 deletions(-) diff --git a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java index f9b42d6..62da133 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java @@ -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; diff --git a/fleetd/src/main/java/dev/ltms/fleet/msg/LeadHeartbeatLoop.java b/fleetd/src/main/java/dev/ltms/fleet/msg/LeadHeartbeatLoop.java index c40f31d..37c6caa 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/msg/LeadHeartbeatLoop.java +++ b/fleetd/src/main/java/dev/ltms/fleet/msg/LeadHeartbeatLoop.java @@ -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). + * + *

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> 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> 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}. * + *

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. + * + *

{@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(); } diff --git a/fleetd/src/test/java/dev/ltms/fleet/msg/LeadHeartbeatLoopTest.java b/fleetd/src/test/java/dev/ltms/fleet/msg/LeadHeartbeatLoopTest.java index 9f48af9..4cee349 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/msg/LeadHeartbeatLoopTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/msg/LeadHeartbeatLoopTest.java @@ -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