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

fleetd #659: remove the dead back-compat forms around LeadConfigDirSource/leadContextLookup #662

Closed
agent wants to merge 0 commits from worker/659-remove-dead-backcompat-ba5e6f-1 into main
pull from: worker/659-remove-dead-backcompat-ba5e6f-1
merge into: fleet:main
Conversation 1 Commits - Files Changed -
agent commented 2026-10-03 18:56:03 +02:00
Member
Copy Link
Copy Source

Removes the three dead back-compat forms named in fleetd #659:

  • FleetMcp.LeadConfigDirSource 1-arg constructor (delegated with _ -> null)
  • Fleetd.leadContextLookup 4-arg overload (its only caller was the dead leadContextSource 4-arg wrapper)
  • Fleetd.leadContextSource 4-arg overload

Each silently resolved no auto-compact window, reverting a caller to LeadContextGauge's fixed 200000 HIGH threshold -- the defect #637 fixed. No production call site used any of the three (confirmed by grep; see the hand-off for the exact commands and counts). Updated the 8 test call sites across 4 files to pass the window lookup explicitly (_ -> null where the test is not about window resolution).

Added FleetdLeadContextSourceWindowAssemblyTest: no existing test called the real assembled LeadHeartbeatLoop far enough to prove FleetdAssembly's window-lookup argument into Fleetd.leadContextSource actually reaches the gauge. Mutating that argument to _ -> null compiled clean and left the whole suite green; the new test fails against that mutation and passes once reverted. Needed a small, additive FakeHerdr change (agentSessionId(..)) since its default agent.get response carries no session id by default.

Build: mvn -o clean install -> BUILD SUCCESS, Tests run: 1923, Failures: 0, Errors: 0, Skipped: 0.

Removes the three dead back-compat forms named in fleetd #659: - FleetMcp.LeadConfigDirSource 1-arg constructor (delegated with `_ -> null`) - Fleetd.leadContextLookup 4-arg overload (its only caller was the dead leadContextSource 4-arg wrapper) - Fleetd.leadContextSource 4-arg overload Each silently resolved no auto-compact window, reverting a caller to LeadContextGauge's fixed 200000 HIGH threshold -- the defect #637 fixed. No production call site used any of the three (confirmed by grep; see the hand-off for the exact commands and counts). Updated the 8 test call sites across 4 files to pass the window lookup explicitly (`_ -> null` where the test is not about window resolution). Added FleetdLeadContextSourceWindowAssemblyTest: no existing test called the real assembled LeadHeartbeatLoop far enough to prove FleetdAssembly's window-lookup argument into Fleetd.leadContextSource actually reaches the gauge. Mutating that argument to `_ -> null` compiled clean and left the whole suite green; the new test fails against that mutation and passes once reverted. Needed a small, additive FakeHerdr change (`agentSessionId(..)`) since its default `agent.get` response carries no session id by default. Build: `mvn -o clean install` -> BUILD SUCCESS, Tests run: 1923, Failures: 0, Errors: 0, Skipped: 0.
agent added 1 commit 2026-10-03 18:56:04 +02:00
fleetd #659: remove the dead back-compat forms around LeadConfigDirSource/leadContextLookup
CI / shell-tests (pull_request) Failing after 8s
Details
CI / contract (pull_request) Successful in 1m41s
Details
CI / build (pull_request) Failing after 2m31s
Details
03d92be751 
FleetMcp.LeadConfigDirSource's 1-arg constructor, Fleetd.leadContextLookup's
4-arg overload, and Fleetd.leadContextSource's 4-arg overload each existed only
to keep old call sites compiling, and each silently resolved no auto-compact
window — reverting any caller that picked one up to LeadContextGauge's fixed
200,000 HIGH threshold, the exact defect #637 fixed. Removed all three and
updated the 8 call sites across 4 test files to pass the window lookup
explicitly.

Added FleetdLeadContextSourceWindowAssemblyTest: no existing test called the
real assembled LeadHeartbeatLoop far enough to prove FleetdAssembly's
window-lookup argument into Fleetd.leadContextSource actually reaches the
gauge. Mutating that argument to `_ -> null` compiled clean and left the whole
suite green; the new test fails against that mutation. Needed a small FakeHerdr
addition (agentSessionId(..)) since its default agent.get response carries no
session id.
ltms commented 2026-10-03 19:13:14 +02:00
Owner
Copy Link
Copy Source

Merged locally as 9417de1 and pushed to main. We merge locally, so Gitea does not close this
by itself — closing by hand.

$ git log --format='%H %P %s' -1
9417de1123c03b288ba9a341eb79b66e1658e134 136bec8e28c941cf39823b967bb8adf37ac40a7c 03d92be751932ee36632e6c451abb1bb3e59921e Merge PR #662: fleetd #659 — remove the dead back-compat forms around LeadConfigDirSource/leadContextLookup
$ git rev-list --count origin/main..HEAD
0

What I measured myself

Everything below I ran in this session. I did not take the implementer's numbers.

Build on the PR head, in a throwaway worktree (mvn -o clean install):
Tests run: 1923, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS, exit 0, 170 surefire
report files. I removed target/surefire-reports first so a deleted class could not be re-counted.

Build on the merged tree, in the main clone (mvn -o install, no clean — that would delete
the running daemon's jar): Tests run: 1923, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS,
exit 0, 170 report files.

The property-1 mutation — the one claim this PR rests on. I replaced FleetdAssembly.java:409's
window-lookup argument with _ -> null:

expected: <HIGH> but was: <OK>
Reading[state=OK, tokens=90000, compactions=0]

That is a runtime assertion failure, so it is behavioural evidence, not a compile break. Reverting
gave an empty git diff --stat and the test passed again.

One trap worth recording: restoring the file from a sed -i.bak backup put back an older mtime
than the mutated .class, so maven skipped the recompile and re-ran the mutated class — a false
red. touch on the source fixed it. Anyone repeating this mutation should check
stat -f '%m' on the source against the class before trusting the result.

I also measured the value of the new test, which the implementer asked for a second opinion on.
Under the same mutation, the whole suite minus the new test is green at
Tests run: 1922, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS. So
FleetdLeadContextSourceWindowAssemblyTest is the only thing pinning the property that the
heartbeat's window lookup reaches the gauge. It earns its place, and the 223 lines are justified —
an assembly-level test is the only thing that can prove the caller.

The discriminator is exact. LeadContextGauge.HIGH_THRESHOLD_TOKENS = 200_000,
HIGH_THRESHOLD_FRACTION = 2.0 / 3.0, so the threshold is (long)(100000 * 2/3) = 66666, and
line 329 compares tokens >= highThreshold. 90000 >= 66666 is HIGH; 90000 >= 200000 is OK. The
test cannot pass by accident on this value.

Call sites. Every surviving caller uses the new form. Two in production:
FleetdAssembly.java:407 and Fleetd.java:1078. The seven test call-site updates each add the
_ -> null the removed overload delegated to, so they are behaviour-preserving. No test was
deleted.

FakeHerdr is a shared helper, so I checked it separately. With the default
agentSessionId == null the appended field is the empty string, leaving the agent.get JSON
byte-identical to before. Only the new test calls the setter — every other agentSessionId() hit
in the tree is PeerHandle.agentSessionId(), a different method. No test asserts that
agent_session is absent. So the change cannot affect any existing test.

Review

Two reviewers, neither the implementer, briefed from the diff and split by dimension.

The test reviewer found no defect and reported what it checked: every upstream failure path
(liveLeadTerminals miss, agents.get throwing, missing transcript, unparsable lines) returns
Reading.unknown(), which fails the assertion outright rather than passing by accident; the
herdrPollWait sentinel is unreachable by construction, not by timing, because it is gated on a
HerdrException from ping and FakeHerdr.healthy defaults true; and FleetdRuntime.close()
tears everything down unconditionally with no real port and no real broker bound. It also
established that the FakeHerdr setter is load-bearing rather than decorative: AgentControl.get()
hits agent.get, which only carries a session id via the new setter, so without it
live.sessionId() is null and the test fails.

The removals reviewer reported one finding: the surviving leadContextLookup javadoc names
fleetd #609, against this repo's rule that source comments carry no ticket history. I checked it
and it is pre-existing and out of scope — the diff adds no such line, and Fleetd.java alone
already carries 108 ticket references. That is a house-style question for the whole file, not this
PR's to fix.

Not done in this change

No wiki entry. The change removes dead internal constructors and adds a test, so there is no new
capability an operator can use, configure or observe, and nothing in the operator-visible contract
moved.

A redeploy is owed and is next: this touches Fleetd.java and FleetMcp.java under
fleetd/src/main/, and the running daemon still holds its old jar.

Merged locally as `9417de1` and pushed to `main`. We merge locally, so Gitea does not close this by itself — closing by hand. ``` $ git log --format='%H %P %s' -1 9417de1123c03b288ba9a341eb79b66e1658e134 136bec8e28c941cf39823b967bb8adf37ac40a7c 03d92be751932ee36632e6c451abb1bb3e59921e Merge PR #662: fleetd #659 — remove the dead back-compat forms around LeadConfigDirSource/leadContextLookup $ git rev-list --count origin/main..HEAD 0 ``` ## What I measured myself Everything below I ran in this session. I did not take the implementer's numbers. **Build on the PR head**, in a throwaway worktree (`mvn -o clean install`): `Tests run: 1923, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`, exit 0, 170 surefire report files. I removed `target/surefire-reports` first so a deleted class could not be re-counted. **Build on the merged tree**, in the main clone (`mvn -o install`, no `clean` — that would delete the running daemon's jar): `Tests run: 1923, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`, exit 0, 170 report files. **The property-1 mutation — the one claim this PR rests on.** I replaced `FleetdAssembly.java:409`'s window-lookup argument with `_ -> null`: ``` expected: <HIGH> but was: <OK> Reading[state=OK, tokens=90000, compactions=0] ``` That is a runtime assertion failure, so it is behavioural evidence, not a compile break. Reverting gave an empty `git diff --stat` and the test passed again. One trap worth recording: restoring the file from a `sed -i.bak` backup put back an **older** mtime than the mutated `.class`, so maven skipped the recompile and re-ran the mutated class — a false red. `touch` on the source fixed it. Anyone repeating this mutation should check `stat -f '%m'` on the source against the class before trusting the result. **I also measured the value of the new test, which the implementer asked for a second opinion on.** Under the same mutation, the whole suite **minus** the new test is green at `Tests run: 1922, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS`. So `FleetdLeadContextSourceWindowAssemblyTest` is the only thing pinning the property that the heartbeat's window lookup reaches the gauge. It earns its place, and the 223 lines are justified — an assembly-level test is the only thing that can prove the *caller*. **The discriminator is exact.** `LeadContextGauge.HIGH_THRESHOLD_TOKENS = 200_000`, `HIGH_THRESHOLD_FRACTION = 2.0 / 3.0`, so the threshold is `(long)(100000 * 2/3) = 66666`, and line 329 compares `tokens >= highThreshold`. `90000 >= 66666` is HIGH; `90000 >= 200000` is OK. The test cannot pass by accident on this value. **Call sites.** Every surviving caller uses the new form. Two in production: `FleetdAssembly.java:407` and `Fleetd.java:1078`. The seven test call-site updates each add the `_ -> null` the removed overload delegated to, so they are behaviour-preserving. No test was deleted. **`FakeHerdr` is a shared helper, so I checked it separately.** With the default `agentSessionId == null` the appended field is the empty string, leaving the `agent.get` JSON byte-identical to before. Only the new test calls the setter — every other `agentSessionId()` hit in the tree is `PeerHandle.agentSessionId()`, a different method. No test asserts that `agent_session` is absent. So the change cannot affect any existing test. ## Review Two reviewers, neither the implementer, briefed from the diff and split by dimension. The test reviewer found no defect and reported what it checked: every upstream failure path (`liveLeadTerminals` miss, `agents.get` throwing, missing transcript, unparsable lines) returns `Reading.unknown()`, which fails the assertion outright rather than passing by accident; the `herdrPollWait` sentinel is unreachable by construction, not by timing, because it is gated on a `HerdrException` from `ping` and `FakeHerdr.healthy` defaults true; and `FleetdRuntime.close()` tears everything down unconditionally with no real port and no real broker bound. It also established that the `FakeHerdr` setter is load-bearing rather than decorative: `AgentControl.get()` hits `agent.get`, which only carries a session id via the new setter, so without it `live.sessionId()` is null and the test fails. The removals reviewer reported one finding: the surviving `leadContextLookup` javadoc names `fleetd #609`, against this repo's rule that source comments carry no ticket history. I checked it and it is **pre-existing and out of scope** — the diff adds no such line, and `Fleetd.java` alone already carries 108 ticket references. That is a house-style question for the whole file, not this PR's to fix. ## Not done in this change No wiki entry. The change removes dead internal constructors and adds a test, so there is no new capability an operator can use, configure or observe, and nothing in the operator-visible contract moved. A redeploy is owed and is next: this touches `Fleetd.java` and `FleetMcp.java` under `fleetd/src/main/`, and the running daemon still holds its old jar.
ltms closed this pull request 2026-10-03 19:13:22 +02:00
Some checks are pending
CI / shell-tests (pull_request) Failing after 8s
Details
CI / contract (pull_request) Successful in 1m41s
Details
CI / build (pull_request) Failing after 2m31s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
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
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#662
Delete Branch "worker/659-remove-dead-backcompat-ba5e6f-1"

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?