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

fleetd #637: pin fleet_list's window wiring and fix the gauge's cache-key bug #660

Closed
agent wants to merge 0 commits from worker/637-revision-60a488-23 into main
pull from: worker/637-revision-60a488-23
merge into: fleet:main
Conversation 1 Commits - Files Changed -
agent commented 2026-10-03 16:19:29 +02:00
Member
Copy Link
Copy Source

Supersedes PR #657. Lead adjudication on issue #637 (comment id 17857, "Lead adjudication of PR 657 — one revision round before merge") found two must-fix gaps in PR #657 and left one recorded, not fixed.

Fix 1 — fleet_list's window wiring was unguarded. Fleetd.leadConfigDirSource (Fleetd.java:1079) built its windowFor argument from leadContextWindowLookup(profiles, leaders), but nothing called the factory itself, so mutating that argument to _ -> null left the full 1918-test suite green. Added FleetdLeadConfigDirSourceWindowWiringTest, modeled on FleetdLeadConfigDirSourceWiringTest's shape for the configDir half of the same factory: it calls Fleetd.leadConfigDirSource(...) directly with real FleetConfig.Profile/FleetConfig.Leader fixtures and asserts on source.windowFor().

Proved RED by mutation: applied return new FleetMcp.LeadConfigDirSource(leadConfigDirLookup(profiles, leaders), _ -> null); at Fleetd.java:1079 and the new test failed with expected: <250000> but was: <null> (a real assertion failure against compiling code, not a compile error). Reverted the mutation (git diff --stat on Fleetd.java came back empty, confirming a clean revert), then the test passed.

Fix 2 — the gauge's result cache ignored the threshold. LeadContextGauge's TTL cache keyed only on (configDir, sessionId) (LeadContextGauge.java:195), but Reading.state() depends on the caller's effectiveWindowTokens. Two calls for the same session with different windows inside the 5s TTL returned the first call's cached state. Added a regression test in LeadContextGaugeHighThresholdTest using one LeadContextGauge instance, one config dir, and two different windows inside the TTL — the three pre-existing tests in that class each build a fresh gauge and @TempDir, so none could see this.

Proved RED: before the fix, the new test failed with expected: <OK> but was: <HIGH>, matching the lead's reproduction. Fixed by folding the derived highThreshold into the cache key. After the fix, the test passes.

Left untouched, as directed. Finding 3 (the dead back-compat overloads — the narrow leadContextLookup/leadContextSource, the 1-arg LeadConfigDirSource constructor, FleetMcp.java:301) is recorded in the adjudication as deliberately not fixed here — split out separately so this correctness fix lands clean. The "also noted" live-snapshot-vs-launched-flag behavior is inherited, pre-existing, and out of scope here too. Neither is touched by this diff.

Build: cd fleetd && mvn -o clean install → Tests run: 1922, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS, exit 0. Baseline before this PR's two new/extended test classes, measured in this worktree after cherry-picking PR #657's commit onto main at 3fab743, was Tests run: 1918 — matching the lead's own measured baseline exactly. The lead's adjudication predicted 1920 (1918 + 1 new test per finding); the actual count is 1922 because Fix 1's wiring test was modeled on the full 3-test shape of its precedent (FleetdLeadConfigDirSourceWiringTest: positive resolution, null-fallback, unrecognized-lead) rather than a single assertion, adding 3 tests instead of 1, plus Fix 2's single regression test — 1918 + 3 + 1 = 1922.

Files changed:

  • fleetd/src/main/java/dev/ltms/fleet/lead/LeadContextGauge.java — cache key now folds in the derived highThreshold
  • fleetd/src/test/java/dev/ltms/fleet/FleetdLeadConfigDirSourceWindowWiringTest.java — new, pins Fix 1
  • fleetd/src/test/java/dev/ltms/fleet/lead/LeadContextGaugeHighThresholdTest.java — extended with Fix 2's regression test
Supersedes PR #657. Lead adjudication on issue #637 (comment id 17857, "Lead adjudication of PR 657 — one revision round before merge") found two must-fix gaps in PR #657 and left one recorded, not fixed. **Fix 1 — `fleet_list`'s window wiring was unguarded.** `Fleetd.leadConfigDirSource` (`Fleetd.java:1079`) built its `windowFor` argument from `leadContextWindowLookup(profiles, leaders)`, but nothing called the factory itself, so mutating that argument to `_ -> null` left the full 1918-test suite green. Added `FleetdLeadConfigDirSourceWindowWiringTest`, modeled on `FleetdLeadConfigDirSourceWiringTest`'s shape for the `configDir` half of the same factory: it calls `Fleetd.leadConfigDirSource(...)` directly with real `FleetConfig.Profile`/`FleetConfig.Leader` fixtures and asserts on `source.windowFor()`. Proved RED by mutation: applied `return new FleetMcp.LeadConfigDirSource(leadConfigDirLookup(profiles, leaders), _ -> null);` at `Fleetd.java:1079` and the new test failed with `expected: <250000> but was: <null>` (a real assertion failure against compiling code, not a compile error). Reverted the mutation (`git diff --stat` on `Fleetd.java` came back empty, confirming a clean revert), then the test passed. **Fix 2 — the gauge's result cache ignored the threshold.** `LeadContextGauge`'s TTL cache keyed only on `(configDir, sessionId)` (`LeadContextGauge.java:195`), but `Reading.state()` depends on the caller's `effectiveWindowTokens`. Two calls for the same session with different windows inside the 5s TTL returned the first call's cached state. Added a regression test in `LeadContextGaugeHighThresholdTest` using one `LeadContextGauge` instance, one config dir, and two different windows inside the TTL — the three pre-existing tests in that class each build a fresh gauge and `@TempDir`, so none could see this. Proved RED: before the fix, the new test failed with `expected: <OK> but was: <HIGH>`, matching the lead's reproduction. Fixed by folding the derived `highThreshold` into the cache key. After the fix, the test passes. **Left untouched, as directed.** Finding 3 (the dead back-compat overloads — the narrow `leadContextLookup`/`leadContextSource`, the 1-arg `LeadConfigDirSource` constructor, `FleetMcp.java:301`) is recorded in the adjudication as deliberately not fixed here — split out separately so this correctness fix lands clean. The "also noted" live-snapshot-vs-launched-flag behavior is inherited, pre-existing, and out of scope here too. Neither is touched by this diff. **Build:** `cd fleetd && mvn -o clean install` → `Tests run: 1922, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`, exit 0. Baseline before this PR's two new/extended test classes, measured in this worktree after cherry-picking PR #657's commit onto `main` at `3fab743`, was `Tests run: 1918` — matching the lead's own measured baseline exactly. The lead's adjudication predicted 1920 (1918 + 1 new test per finding); the actual count is 1922 because Fix 1's wiring test was modeled on the full 3-test shape of its precedent (`FleetdLeadConfigDirSourceWiringTest`: positive resolution, null-fallback, unrecognized-lead) rather than a single assertion, adding 3 tests instead of 1, plus Fix 2's single regression test — 1918 + 3 + 1 = 1922. Files changed: - `fleetd/src/main/java/dev/ltms/fleet/lead/LeadContextGauge.java` — cache key now folds in the derived `highThreshold` - `fleetd/src/test/java/dev/ltms/fleet/FleetdLeadConfigDirSourceWindowWiringTest.java` — new, pins Fix 1 - `fleetd/src/test/java/dev/ltms/fleet/lead/LeadContextGaugeHighThresholdTest.java` — extended with Fix 2's regression test
agent added 2 commits 2026-10-03 16:19:30 +02:00
fleetd #637: scale LeadContextGauge's HIGH threshold with the effective auto-compact window 436b026696 
HIGH_THRESHOLD_TOKENS was a hardcoded 200_000, while the event it warns about
(auto-compaction) is configured per profile via autoCompactWindow and can legally go
as low as 100_000 — making HIGH unreachable before a compaction on such a profile.

LeadContextGauge.read now takes an optional effective window and fires HIGH at 2/3 of
it, falling back to the fixed 200_000 when no window is resolvable (unresolved callers,
including the pre-existing 3-arg read(), keep today's behaviour exactly).

FleetConfig.Profile.effectiveAutoCompactWindow() resolves that window the way a
launched Claude Code session actually reads it: env.CLAUDE_CODE_AUTO_COMPACT_WINDOW
wins over the autoCompactWindow launch flag when both are set.

Wired into both real consumers: fleet_list's context row (FleetMcp.LeadConfigDirSource,
widened with a back-compat constructor so no unrelated call site changes) and the lead
heartbeat's context-high nudge (Fleetd.leadContextLookup/leadContextSource, widened the
same way).
fleetd #637: pin the fleet_list window wiring and fold the threshold into the gauge's cache key
CI / shell-tests (pull_request) Failing after 9s
Details
CI / contract (pull_request) Successful in 1m1s
Details
CI / build (pull_request) Failing after 2m22s
Details
ae94d511d7 
Fleetd.leadConfigDirSource built its window argument with nothing calling the
factory itself to prove it, so a mutation to a no-op lookup left the whole
suite green. Add a wiring test that calls the factory directly, modeled on
FleetdLeadConfigDirSourceWiringTest's shape for the configDir half of the
same factory.

LeadContextGauge's result cache keyed only on (configDir, sessionId), but the
cached Reading's state now depends on the caller's resolved effective window.
Fold the derived threshold into the cache key so two reads of the same
session with different windows inside the TTL each report against their own
window.
ltms commented 2026-10-03 16:30:37 +02:00
Owner
Copy Link
Copy Source

Merged locally into main as 136bec8 and pushed. We merge locally, so this PR is closed by hand.

Lead verification, all run by me on the merged tree in this turn, not taken from the worker:

  • mvn -o install in fleetd/: Tests run: 1922, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS, exit 0.
  • bash scripts/test-config-edit.sh: PASS, exit 0.
  • Mutation 1, Finding 1's test: replaced leadContextWindowLookup(profiles, leaders) with _ -> null at Fleetd.java:1079. FleetdLeadConfigDirSourceWindowWiringTest.resolvesTheRealConfiguredWindow failed with expected: <250000> but was: <null>. This is the exact mutation that left the whole suite green before this round. Reverted, empty diffstat, green again.
  • Mutation 2, Finding 2's test: restored the cache key to base + NUL + sessionId. The new TTL test failed with expected: <OK> but was: <HIGH>, matching my own reproduction from the adjudication comment. Reverted, empty diffstat, green again.
  • I read the whole production diff myself: 6 files under fleetd/src/main/. No defect found.

On the test count: I predicted about 1920 and the worker measured 1922. The worker was right and my prediction was wrong. Finding 1's wiring test has 3 tests, not 1, because it mirrors the shape of its precedent FleetdLeadConfigDirSourceWiringTest. That is the correct shape, and the extra two tests are wanted. The worker flagged the gap itself instead of hiding it, which is the right behaviour.

One note on the cache-key fix, checked and not a defect: the key folds in the derived highThreshold, not the raw window. Reading is (state, tokens, compactions), and only state depends on the window, as tokens >= highThreshold. So the threshold fully captures the window's effect, and two different windows that derive the same threshold produce an identical Reading — sharing that entry is correct, not stale.

Also checked: Profile.effectiveAutoCompactWindow() calls env.get(...), and Profile's canonical constructor normalises env = (env == null) ? Map.of() : Map.copyOf(env), so a profile with no env: block cannot throw there.

Finding 3, the dead back-compat overloads, was left untouched as instructed and stays with #659, which is now unblocked.

Merged locally into `main` as `136bec8` and pushed. We merge locally, so this PR is closed by hand. Lead verification, all run by me on the merged tree in this turn, not taken from the worker: - `mvn -o install` in `fleetd/`: `Tests run: 1922, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`, exit 0. - `bash scripts/test-config-edit.sh`: `PASS`, exit 0. - Mutation 1, Finding 1's test: replaced `leadContextWindowLookup(profiles, leaders)` with `_ -> null` at `Fleetd.java:1079`. `FleetdLeadConfigDirSourceWindowWiringTest.resolvesTheRealConfiguredWindow` failed with `expected: <250000> but was: <null>`. This is the exact mutation that left the whole suite green before this round. Reverted, empty diffstat, green again. - Mutation 2, Finding 2's test: restored the cache key to `base + NUL + sessionId`. The new TTL test failed with `expected: <OK> but was: <HIGH>`, matching my own reproduction from the adjudication comment. Reverted, empty diffstat, green again. - I read the whole production diff myself: 6 files under `fleetd/src/main/`. No defect found. On the test count: I predicted about 1920 and the worker measured 1922. The worker was right and my prediction was wrong. Finding 1's wiring test has 3 tests, not 1, because it mirrors the shape of its precedent `FleetdLeadConfigDirSourceWiringTest`. That is the correct shape, and the extra two tests are wanted. The worker flagged the gap itself instead of hiding it, which is the right behaviour. One note on the cache-key fix, checked and not a defect: the key folds in the *derived* `highThreshold`, not the raw window. `Reading` is `(state, tokens, compactions)`, and only `state` depends on the window, as `tokens >= highThreshold`. So the threshold fully captures the window's effect, and two different windows that derive the same threshold produce an identical `Reading` — sharing that entry is correct, not stale. Also checked: `Profile.effectiveAutoCompactWindow()` calls `env.get(...)`, and `Profile`'s canonical constructor normalises `env = (env == null) ? Map.of() : Map.copyOf(env)`, so a profile with no `env:` block cannot throw there. Finding 3, the dead back-compat overloads, was left untouched as instructed and stays with #659, which is now unblocked.
ltms closed this pull request 2026-10-03 16:30:40 +02:00
Some checks are pending
CI / shell-tests (pull_request) Failing after 9s
Details
CI / contract (pull_request) Successful in 1m1s
Details
CI / build (pull_request) Failing after 2m22s
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#660
Delete Branch "worker/637-revision-60a488-23"

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?