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

The assembly's requireOperatorConfirm wiring is unpinned: dropping it silently reverts fleetd #621 with a fully green suite #630

New Issue
Open
opened 2026-09-22 07:45:26 +02:00 by ltms · 0 comments
ltms commented 2026-09-22 07:45:26 +02:00
Owner
Copy Link
Copy Source

What this is

FleetdAssembly.assembleAndStart builds the LeadHeartbeatLoop with a 14th argument:

boolean requireOperatorConfirm = cfg.leadRollover() == null || cfg.leadRollover().requireOperatorConfirm();
heartbeat = new LeadHeartbeatLoop(..., Boolean.TRUE.equals(hb.contextHighNudge()), requireOperatorConfirm);

Delete that argument and the whole suite stays green.

Measured

On the resolved #612 Unit A tree, with requireOperatorConfirm removed from the constructor call:

Tests run: 1883, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

Restored, same tree:

Tests run: 1883, Failures: 0, Errors: 0, Skipped: 0

Identical. Nothing observes which overload the assembly calls.

Why it goes unnoticed

fleetd #621 deliberately kept the 13-argument overload, which delegates with true so every pre-#621 message stays byte-identical. That backward-compatibility choice is correct on its own terms, and it is exactly what makes the deletion invisible: the daemon keeps starting, keeps nudging, and simply goes back to telling every lead to ask the operator before a context roll — on a host that has set requireOperatorConfirm: false precisely so it does not have to.

LeadHeartbeatLoopTest covers contextNotice(...) directly, including the 4-argument overload with false. That proves the method branches correctly. It says nothing about which overload the assembly calls — the seam-versus-caller gap this ticket's parent, #612, exists to close.

How it was found

Not by a sweep. It surfaced while resolving the Fleetd.java conflict when #612 Unit A merged main. #622 added the line above after Unit A forked, inside the block Unit A had already moved into FleetdAssembly. Taking Unit A's side of the conflict — the mechanical resolution, and the one a merge tool suggests — would have dropped it and reverted the operator's #621 fix with a clean build and no failing test.

The line was carried across by hand in 72f46d7 and the merge is on main (26f1986, 1883 tests, 0 failures). So the fix is live and correct today. This ticket is only about the missing pin.

Suggested fix

Assert it through the assembly, not the seam. FleetdRuntime.heartbeat() already exposes the real LeadHeartbeatLoop, so a test can assemble with leadRollover.requireOperatorConfirm: false and prove the notice the assembled loop produces omits the operator ask — then again with true and prove it includes it. Both directions, because a one-directional assertion here passes on a constant.

Same family as #625 (the subscription guard's call site) and #629 (the herdr boot wait). All three are call sites the assembly owns and no test observes.

## What this is `FleetdAssembly.assembleAndStart` builds the `LeadHeartbeatLoop` with a 14th argument: ```java boolean requireOperatorConfirm = cfg.leadRollover() == null || cfg.leadRollover().requireOperatorConfirm(); heartbeat = new LeadHeartbeatLoop(..., Boolean.TRUE.equals(hb.contextHighNudge()), requireOperatorConfirm); ``` Delete that argument and the whole suite stays green. ## Measured On the resolved #612 Unit A tree, with `requireOperatorConfirm` removed from the constructor call: ``` Tests run: 1883, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` Restored, same tree: ``` Tests run: 1883, Failures: 0, Errors: 0, Skipped: 0 ``` Identical. Nothing observes which overload the assembly calls. ## Why it goes unnoticed fleetd #621 deliberately kept the 13-argument overload, which delegates with `true` so every pre-#621 message stays byte-identical. That backward-compatibility choice is correct on its own terms, and it is exactly what makes the deletion invisible: the daemon keeps starting, keeps nudging, and simply goes back to telling **every** lead to ask the operator before a context roll — on a host that has set `requireOperatorConfirm: false` precisely so it does not have to. `LeadHeartbeatLoopTest` covers `contextNotice(...)` directly, including the 4-argument overload with `false`. That proves the method branches correctly. It says nothing about which overload the assembly calls — the seam-versus-caller gap this ticket's parent, #612, exists to close. ## How it was found Not by a sweep. It surfaced while resolving the `Fleetd.java` conflict when #612 Unit A merged `main`. #622 added the line above **after** Unit A forked, inside the block Unit A had already moved into `FleetdAssembly`. Taking Unit A's side of the conflict — the mechanical resolution, and the one a merge tool suggests — would have dropped it and reverted the operator's #621 fix with a clean build and no failing test. The line was carried across by hand in `72f46d7` and the merge is on `main` (`26f1986`, 1883 tests, 0 failures). So the fix is live and correct today. This ticket is only about the missing pin. ## Suggested fix Assert it through the assembly, not the seam. `FleetdRuntime.heartbeat()` already exposes the real `LeadHeartbeatLoop`, so a test can assemble with `leadRollover.requireOperatorConfirm: false` and prove the notice the **assembled** loop produces omits the operator ask — then again with `true` and prove it includes it. Both directions, because a one-directional assertion here passes on a constant. Same family as #625 (the subscription guard's call site) and #629 (the herdr boot wait). All three are call sites the assembly owns and no test observes.
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#630
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?