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

Remove the dead back-compat forms around LeadConfigDirSource and leadContextLookup — they silently restore the old fixed threshold #659

New Issue
Closed
opened 2026-10-03 16:06:32 +02:00 by ltms · 2 comments
ltms commented 2026-10-03 16:06:32 +02:00
Owner
Copy Link
Copy Source

Split out of PR 657 / #637 so a correctness fix lands clean. See #637's lead adjudication comment for the full context.

What is dead

These three forms exist only so existing tests keep compiling. No production path reaches any of them. Counts I ran myself at PR 657's head:

Form Production callers
FleetMcp.LeadConfigDirSource(Function<String,String>) — the one-arg constructor 0
Fleetd.leadContextSource(gauge, agents, liveLeadTerminals, configDirForLeadName) — narrow 0
Fleetd.leadContextLookup(gauge, agents, liveLeadTerminals, configDirForLeadName) — narrow 1, and it is dead

The one caller of narrow leadContextLookup:1127 is the body of narrow leadContextSource:1165 at line :1168 — and nothing calls leadContextSource:1165. So the two form a closed, unreachable chain.

The live wiring is: FleetdAssembly.java:407 → wide leadContextSource, and Fleetd.java:1078 → the two-arg LeadConfigDirSource constructor.

git grep -n -E 'new[[:space:]]+FleetMcp\.LeadConfigDirSource[[:space:]]*\(|leadContextLookup[[:space:]]*\(|leadContextSource[[:space:]]*\(' -- 'fleetd/src/main/java'

Why this is worth removing, beyond tidiness

Each dead form supplies no window, and the code then falls back to the fixed HIGH_THRESHOLD_TOKENS = 200_000. So the day someone reaches for one of them, #637's fix is silently inert on that path — which is exactly the defect #637 was filed about. A back-compat form whose fallback is the old bug is a trap, not a convenience.

The same reasoning applies to LeadContextGauge.read(configDir, sessionId, agentType), the 3-arg overload, which passes null. It is also what makes #637's cache-key defect reachable: with the 4-arg form only, every caller for a given lead resolves the same window, and the stale-cache failure needs two different windows. Removing the narrow overload closes the correctness hole and the dead surface in one move. Check its production caller count before removing it — unlike the three above, I have not confirmed it is zero.

The ask

Remove the three dead forms. Decide on read's 3-arg overload after measuring its production callers; if it has none, remove it too and say so.

Then migrate the test call sites. There are 8 old-form call sites across 4 files:

fleetd/src/test/java/dev/ltms/fleet/FleetdLeadConfigDirSourceWiringTest.java        1
fleetd/src/test/java/dev/ltms/fleet/FleetdLeadContextLookupTest.java                4
fleetd/src/test/java/dev/ltms/fleet/FleetdLeadContextSourceWiringTest.java          2
fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpLeadContextGaugeWiringTest.java     1

Note a reviewer reported 7 in 3 files; that count misses FleetMcpLeadContextGaugeWiringTest.java, because its pattern required the FleetMcp.-qualified form. Use the 8/4 figure, and re-run the grep rather than trusting either number.

How to migrate a test, and the trap

Passing _ -> null at a test call site is the mechanical fix, and for most of these it is correct. But do not pass _ -> null in a test whose job is to prove the wiring resolves a real value — that converts a guard into a test that passes against broken code. FleetdLeadConfigDirSourceWiringTest is the one to read first: its whole point is that leadConfigDirSource returns a source resolving a real configDir, which would be false if the factory body were replaced with LeadConfigDirSource.none().

If a test needs a real window, give it a real one from a FleetConfig.Profile fixture.

Acceptance

  • git grep for each removed form returns zero hits outside its own removal commit. Pair each zero with a positive control — a broken pattern returns a clean 0 that reads exactly like success.
  • cd fleetd && mvn -o clean install passes, and you report the real Tests run: line.
  • The test count does not drop. If it does, say which test went and why — a deleted test looks identical whether it was superseded or quietly dropped.

Blocked on #637's revision landing first, since both touch Fleetd.java and FleetMcp.java.

Split out of PR 657 / #637 so a correctness fix lands clean. See #637's lead adjudication comment for the full context. ## What is dead These three forms exist only so existing tests keep compiling. **No production path reaches any of them.** Counts I ran myself at PR 657's head: | Form | Production callers | |---|---| | `FleetMcp.LeadConfigDirSource(Function<String,String>)` — the one-arg constructor | **0** | | `Fleetd.leadContextSource(gauge, agents, liveLeadTerminals, configDirForLeadName)` — narrow | **0** | | `Fleetd.leadContextLookup(gauge, agents, liveLeadTerminals, configDirForLeadName)` — narrow | **1, and it is dead** | The one caller of narrow `leadContextLookup:1127` is the body of narrow `leadContextSource:1165` at line `:1168` — and nothing calls `leadContextSource:1165`. So the two form a closed, unreachable chain. The live wiring is: `FleetdAssembly.java:407` → **wide** `leadContextSource`, and `Fleetd.java:1078` → the **two-arg** `LeadConfigDirSource` constructor. ```bash git grep -n -E 'new[[:space:]]+FleetMcp\.LeadConfigDirSource[[:space:]]*\(|leadContextLookup[[:space:]]*\(|leadContextSource[[:space:]]*\(' -- 'fleetd/src/main/java' ``` ## Why this is worth removing, beyond tidiness Each dead form supplies **no window**, and the code then falls back to the fixed `HIGH_THRESHOLD_TOKENS = 200_000`. So the day someone reaches for one of them, #637's fix is silently inert on that path — which is exactly the defect #637 was filed about. A back-compat form whose fallback is the old bug is a trap, not a convenience. The same reasoning applies to `LeadContextGauge.read(configDir, sessionId, agentType)`, the 3-arg overload, which passes `null`. It is also what makes #637's cache-key defect reachable: with the 4-arg form only, every caller for a given lead resolves the same window, and the stale-cache failure needs two different windows. Removing the narrow overload closes the correctness hole and the dead surface in one move. **Check its production caller count before removing it** — unlike the three above, I have not confirmed it is zero. ## The ask Remove the three dead forms. Decide on `read`'s 3-arg overload after measuring its production callers; if it has none, remove it too and say so. Then migrate the test call sites. There are **8 old-form call sites across 4 files**: ``` fleetd/src/test/java/dev/ltms/fleet/FleetdLeadConfigDirSourceWiringTest.java 1 fleetd/src/test/java/dev/ltms/fleet/FleetdLeadContextLookupTest.java 4 fleetd/src/test/java/dev/ltms/fleet/FleetdLeadContextSourceWiringTest.java 2 fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpLeadContextGaugeWiringTest.java 1 ``` Note a reviewer reported 7 in 3 files; that count misses `FleetMcpLeadContextGaugeWiringTest.java`, because its pattern required the `FleetMcp.`-qualified form. Use the 8/4 figure, and re-run the grep rather than trusting either number. ## How to migrate a test, and the trap Passing `_ -> null` at a test call site is the mechanical fix, and for most of these it is correct. But **do not pass `_ -> null` in a test whose job is to prove the wiring resolves a real value** — that converts a guard into a test that passes against broken code. `FleetdLeadConfigDirSourceWiringTest` is the one to read first: its whole point is that `leadConfigDirSource` returns a source resolving a real `configDir`, which would be false if the factory body were replaced with `LeadConfigDirSource.none()`. If a test needs a real window, give it a real one from a `FleetConfig.Profile` fixture. ## Acceptance - `git grep` for each removed form returns zero hits outside its own removal commit. Pair each zero with a positive control — a broken pattern returns a clean `0` that reads exactly like success. - `cd fleetd && mvn -o clean install` passes, and you report the real `Tests run:` line. - The test count does **not** drop. If it does, say which test went and why — a deleted test looks identical whether it was superseded or quietly dropped. Blocked on #637's revision landing first, since both touch `Fleetd.java` and `FleetMcp.java`.
ltms commented 2026-10-03 16:31:15 +02:00
Author
Owner
Copy Link
Copy Source

Unblocked. #637's revision landed in main as 136bec8 (PR #660), so Fleetd.java and FleetMcp.java are free. This ticket can now be delegated.

Re-measured by me on 136bec8, with the output. These replace the counts in the ticket body, which were taken on the pre-merge tree:

$ git rev-parse --short HEAD
136bec8

$ grep -rln "LeadConfigDirSource\|leadContextLookup\|leadContextSource" fleetd/src/test --include='*.java' | wc -l
8

$ grep -rln "LeadConfigDirSource" fleetd/src/main --include='*.java'
fleetd/src/main/java/dev/ltms/fleet/Fleetd.java
fleetd/src/main/java/dev/ltms/fleet/msg/LeadHeartbeatLoop.java
fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java

The 8 test files are:

FleetdLeadConfigDirLookupTest.java
FleetdLeadConfigDirSourceWiringTest.java
FleetdLeadContextSourceWiringTest.java
FleetdLeadContextLookupTest.java
FleetdLeadContextWindowLookupTest.java
FleetdLeadConfigDirSourceAssemblyTest.java
FleetdLeadConfigDirSourceWindowWiringTest.java
mcp/FleetMcpLeadContextGaugeWiringTest.java

A note on why "8" here is not the same 8 the ticket body mentions: the body's figure counted call sites of the back-compat forms, while this one counts files touching any of the three names. They agree by coincidence, not because they measure the same thing. Whoever picks this up must run their own grep for whatever they actually need and report that, rather than reusing either number. Last round a reviewer reported 7 sites in 3 files and missed FleetMcpLeadContextGaugeWiringTest.java, so this is a real trap here.

The dead forms to remove, as of 136bec8:

  • FleetMcp.LeadConfigDirSource's 1-arg constructor, which delegates with _ -> null.
  • Fleetd.leadContextLookup's 4-arg overload, whose body is return leadContextLookup(gauge, agents, liveLeadTerminals, configDirForLeadName, _ -> null);.
  • Fleetd.leadContextSource's 4-arg overload.

One caution for the brief: the narrow leadContextLookup has exactly one caller and it is that dead wrapper's own body. A grep that counts it as a live caller will conclude the form is in use. Count callers outside the wrapper itself.

**Unblocked.** #637's revision landed in `main` as `136bec8` (PR #660), so `Fleetd.java` and `FleetMcp.java` are free. This ticket can now be delegated. Re-measured by me on `136bec8`, with the output. These replace the counts in the ticket body, which were taken on the pre-merge tree: ``` $ git rev-parse --short HEAD 136bec8 $ grep -rln "LeadConfigDirSource\|leadContextLookup\|leadContextSource" fleetd/src/test --include='*.java' | wc -l 8 $ grep -rln "LeadConfigDirSource" fleetd/src/main --include='*.java' fleetd/src/main/java/dev/ltms/fleet/Fleetd.java fleetd/src/main/java/dev/ltms/fleet/msg/LeadHeartbeatLoop.java fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java ``` The 8 test files are: ``` FleetdLeadConfigDirLookupTest.java FleetdLeadConfigDirSourceWiringTest.java FleetdLeadContextSourceWiringTest.java FleetdLeadContextLookupTest.java FleetdLeadContextWindowLookupTest.java FleetdLeadConfigDirSourceAssemblyTest.java FleetdLeadConfigDirSourceWindowWiringTest.java mcp/FleetMcpLeadContextGaugeWiringTest.java ``` A note on why "8" here is not the same 8 the ticket body mentions: the body's figure counted *call sites* of the back-compat forms, while this one counts *files* touching any of the three names. They agree by coincidence, not because they measure the same thing. Whoever picks this up must run their own grep for whatever they actually need and report that, rather than reusing either number. Last round a reviewer reported 7 sites in 3 files and missed `FleetMcpLeadContextGaugeWiringTest.java`, so this is a real trap here. The dead forms to remove, as of `136bec8`: - `FleetMcp.LeadConfigDirSource`'s 1-arg constructor, which delegates with `_ -> null`. - `Fleetd.leadContextLookup`'s 4-arg overload, whose body is `return leadContextLookup(gauge, agents, liveLeadTerminals, configDirForLeadName, _ -> null);`. - `Fleetd.leadContextSource`'s 4-arg overload. One caution for the brief: the narrow `leadContextLookup` has exactly one caller and it is that dead wrapper's own body. A grep that counts it as a live caller will conclude the form is in use. Count callers outside the wrapper itself.
ltms commented 2026-10-03 19:13:34 +02:00
Author
Owner
Copy Link
Copy Source

Done, merged as 9417de1 on main via PR #662 (closed by hand — we merge locally, so Gitea does
not close it itself). Full verification detail is in
the PR comment.

All three dead back-compat forms are gone:

  • FleetMcp.LeadConfigDirSource's 1-argument constructor
  • Fleetd.leadContextLookup's 4-argument overload
  • Fleetd.leadContextSource's 4-argument overload

Every surviving caller passes the window lookup explicitly. The two production callers are
FleetdAssembly.java:407 and Fleetd.java:1078. Seven test call sites were updated to pass
_ -> null, which is exactly what the removed overloads delegated to, so no test changed meaning
and none was deleted.

mvn -o install on the merged tree in the main clone: Tests run: 1923, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS, exit 0, 170 surefire report files.

This ticket also closed a coverage gap nobody had found

Worth recording, because it was not in the original scope. Nothing proved that
FleetdAssembly's window-lookup argument actually reaches the gauge. I measured this myself:
replacing that argument with _ -> null left the whole suite green at 1922 before the new test
existed, and still leaves it green at 1922 with the new test excluded. So
FleetdLeadContextSourceWindowAssemblyTest is the only thing pinning that property. Under the
mutation it fails at runtime with expected: <HIGH> but was: <OK>.

That gap predates both this ticket and #637. It is the same lesson as #612 and #589: a test on the
seam does not prove the caller, and only an assembly-level test can.

One thing left over, not fixed here

The surviving leadContextLookup javadoc names fleetd #609, which breaks this repo's rule that
source comments carry no ticket history. It is pre-existing — the diff adds no such line — and
Fleetd.java alone already carries 108 ticket references, so it is a house-style question for the
file rather than something to fix inside this change. Not filed as a ticket; raising it here so it
is on the record.

Done, merged as `9417de1` on `main` via PR #662 (closed by hand — we merge locally, so Gitea does not close it itself). Full verification detail is in [the PR comment](https://git.ltms.dev/fleet/fleetd/pulls/662#issuecomment-18027). All three dead back-compat forms are gone: - `FleetMcp.LeadConfigDirSource`'s 1-argument constructor - `Fleetd.leadContextLookup`'s 4-argument overload - `Fleetd.leadContextSource`'s 4-argument overload Every surviving caller passes the window lookup explicitly. The two production callers are `FleetdAssembly.java:407` and `Fleetd.java:1078`. Seven test call sites were updated to pass `_ -> null`, which is exactly what the removed overloads delegated to, so no test changed meaning and none was deleted. `mvn -o install` on the merged tree in the main clone: `Tests run: 1923, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`, exit 0, 170 surefire report files. ## This ticket also closed a coverage gap nobody had found Worth recording, because it was not in the original scope. Nothing proved that `FleetdAssembly`'s window-lookup argument actually reaches the gauge. I measured this myself: replacing that argument with `_ -> null` left the whole suite green at 1922 before the new test existed, and still leaves it green at 1922 with the new test excluded. So `FleetdLeadContextSourceWindowAssemblyTest` is the only thing pinning that property. Under the mutation it fails at runtime with `expected: <HIGH> but was: <OK>`. That gap predates both this ticket and #637. It is the same lesson as #612 and #589: a test on the seam does not prove the caller, and only an assembly-level test can. ## One thing left over, not fixed here The surviving `leadContextLookup` javadoc names `fleetd #609`, which breaks this repo's rule that source comments carry no ticket history. It is pre-existing — the diff adds no such line — and `Fleetd.java` alone already carries 108 ticket references, so it is a house-style question for the file rather than something to fix inside this change. Not filed as a ticket; raising it here so it is on the record.
ltms closed this issue 2026-10-03 19:13:37 +02:00
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#659
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?