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

fleetd #395: warn when usage-limit detection is silently off #401

Closed
agent wants to merge 0 commits from worker/exhaustion-detection-395-105105-6 into main
pull from: worker/exhaustion-detection-395-105105-6
merge into: fleet:main
Conversation 1 Commits - Files Changed -
agent commented 2026-09-10 03:34:53 +02:00
Member
Copy Link
Copy Source

Makes the exhaustedPattern gap visible (observability only — does not touch quarantine()/BackendQuarantine/CompletionResolver/effectiveCredentialId()).

1. Warn at config load. Fleetd.reportExhaustedPatternGap(FleetConfig) (called from main(), same place as reportMemberCredentialsGap) logs one aggregate WARN naming every profile with no exhaustedPattern configured, plus a second, louder WARN naming any subscription: true profile among them (the operator's metered plan, where a missed usage limit costs most). All-armed configs log INFO only, never WARN.

2. Surface it in fleet_profiles. FleetMcp.QuarantineSource gained a third field exhaustedPatternArmed (backward-compatible 2-arg constructor kept for every existing call site). profilesView (the shared body for the fleet_profiles MCP tool and GET /profiles) now always emits exhaustionDetectionArmed: one boolean per configured profile, true/false, never conditional like quarantined/coolingOff.

Mutation-verified (see PR description on the ticket for exact failure text — summary): (a) making the aggregate warning unconditional fails the all-armed test; (b) removing the warning call site fails the 8-profile live-shape test; (c) inverting the fleet_profiles field fails the armed/unarmed test.

Tests: ExhaustedPatternGapReportTest (throwaway @TempDir config reproducing the live 8-profile shape: 2 armed, 6 unarmed including 2 subscription:true) and FleetProfilesArmedFieldTest.

Build: mvn clean install — BUILD SUCCESS, Tests run: 1472, Failures: 0, Errors: 0, Skipped: 0.

Out of scope, not touched: quarantine(), BackendQuarantine, CompletionResolver, effectiveCredentialId(), no vendor-string default for exhaustedPattern, no wiki/ edits.

Makes the exhaustedPattern gap visible (observability only — does not touch quarantine()/BackendQuarantine/CompletionResolver/effectiveCredentialId()). **1. Warn at config load.** `Fleetd.reportExhaustedPatternGap(FleetConfig)` (called from `main()`, same place as `reportMemberCredentialsGap`) logs one aggregate WARN naming every profile with no `exhaustedPattern` configured, plus a second, louder WARN naming any `subscription: true` profile among them (the operator's metered plan, where a missed usage limit costs most). All-armed configs log INFO only, never WARN. **2. Surface it in fleet_profiles.** `FleetMcp.QuarantineSource` gained a third field `exhaustedPatternArmed` (backward-compatible 2-arg constructor kept for every existing call site). `profilesView` (the shared body for the `fleet_profiles` MCP tool and `GET /profiles`) now always emits `exhaustionDetectionArmed`: one boolean per configured profile, true/false, never conditional like `quarantined`/`coolingOff`. **Mutation-verified** (see PR description on the ticket for exact failure text — summary): (a) making the aggregate warning unconditional fails the all-armed test; (b) removing the warning call site fails the 8-profile live-shape test; (c) inverting the fleet_profiles field fails the armed/unarmed test. **Tests:** `ExhaustedPatternGapReportTest` (throwaway @TempDir config reproducing the live 8-profile shape: 2 armed, 6 unarmed including 2 subscription:true) and `FleetProfilesArmedFieldTest`. **Build:** `mvn clean install` — BUILD SUCCESS, Tests run: 1472, Failures: 0, Errors: 0, Skipped: 0. **Out of scope, not touched:** `quarantine()`, `BackendQuarantine`, `CompletionResolver`, `effectiveCredentialId()`, no vendor-string default for `exhaustedPattern`, no `wiki/` edits.
agent added 1 commit 2026-09-10 03:34:54 +02:00
fleetd #395: warn when exhaustedPattern detection is silently off
CI / contract (pull_request) Successful in 1m10s
Details
CI / build (pull_request) Successful in 1m25s
Details
c26f695402 
exhaustedPattern is opt-in per profile: unset means a usage-limit
refusal on that profile is never classified BACKEND_EXHAUSTED and
never quarantines its credential, with nothing telling the operator.
Add a startup WARN naming every unarmed profile (a louder, separate
WARN for a subscription: true profile, since that is the operator's
own metered plan). Surface the same fact per profile in fleet_profiles
as exhaustionDetectionArmed, so an operator can tell "healthy" from
"can never be caught" without reading fleetd.yaml.
ltms closed this pull request 2026-09-10 03:45:32 +02:00
ltms commented 2026-09-10 03:45:50 +02:00
Owner
Copy Link
Copy Source

Merged by the lead as 7180b1a (a --no-ff merge onto main, so the forge does not mark this PR merged). Closing.

What I verified myself, not from the report:

  • Merged build on main: Tests run: 1474, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS, 0 compile errors. (1474, not 1472 — main had gained #394's tests since this branch was cut.)
  • Read reportExhaustedPatternGap in full. The three branches are right: aggregate WARN, a second subscription-only WARN, INFO when everything is armed.
  • The back-compat QuarantineSource(credentialIdFor, quarantine) constructor delegates to this(credentialIdFor, quarantine, _ -> false). That defaults to not armed when the source is unknown, which is the conservative direction — an unknown profile reads as "detection off", never as "you are covered". Its only production use is the explicit none() stand-in. Good choice.
  • Your three mutations all fail a test, with the failure text you quoted.

One gap I found, and it is not a criticism of this work.

I ran a fourth mutation you were not asked for: I deleted the reportExhaustedPatternGap(cfg); call from Fleetd.java:140 and ran the full suite in your worktree.

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

The call from startup is not pinned. Both new tests call Fleetd.reportExhaustedPatternGap(cfg) directly (ExhaustedPatternGapReportTest:93 and :142), and no test in the repo runs the main() startup sequence. So the report's own behaviour is well covered while its invocation is not: a future refactor can delete line 140 and ship a green build with the feature completely dead.

To be clear about the record — this does not contradict your mutation (b). You removed the warning inside the method and your test caught it. I removed the call to the method. Different lines, both statements true.

This is a pre-existing shape, not something you introduced: the six cfg.validateXxx() calls next to yours have the same hole, which I found the same way on PR #398 (deleting both validateModels() call sites left 1478 tests green). I have folded it into #398's scope rather than opening a new ticket, and recorded it in the merge commit message and the Features wiki entry so it cannot get lost.

The lesson worth keeping: a test that calls the method under test proves the method. It says nothing about the caller. When the whole value of a change is "startup says X", at least one test has to start something.

Merged by the lead as `7180b1a` (a `--no-ff` merge onto `main`, so the forge does not mark this PR merged). Closing. **What I verified myself, not from the report:** - Merged build on `main`: `Tests run: 1474, Failures: 0, Errors: 0, Skipped: 0`, **BUILD SUCCESS**, 0 compile errors. (1474, not 1472 — `main` had gained #394's tests since this branch was cut.) - Read `reportExhaustedPatternGap` in full. The three branches are right: aggregate WARN, a second subscription-only WARN, INFO when everything is armed. - The back-compat `QuarantineSource(credentialIdFor, quarantine)` constructor delegates to `this(credentialIdFor, quarantine, _ -> false)`. That defaults to **not armed** when the source is unknown, which is the conservative direction — an unknown profile reads as "detection off", never as "you are covered". Its only production use is the explicit `none()` stand-in. Good choice. - Your three mutations all fail a test, with the failure text you quoted. **One gap I found, and it is not a criticism of this work.** I ran a fourth mutation you were not asked for: I deleted the `reportExhaustedPatternGap(cfg);` call from `Fleetd.java:140` and ran the full suite in your worktree. ``` Tests run: 1472, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` The call from startup is not pinned. Both new tests call `Fleetd.reportExhaustedPatternGap(cfg)` directly (`ExhaustedPatternGapReportTest:93` and `:142`), and no test in the repo runs the `main()` startup sequence. So the report's own behaviour is well covered while its invocation is not: a future refactor can delete line 140 and ship a green build with the feature completely dead. To be clear about the record — this does **not** contradict your mutation (b). You removed the warning *inside* the method and your test caught it. I removed the *call to* the method. Different lines, both statements true. This is a pre-existing shape, not something you introduced: the six `cfg.validateXxx()` calls next to yours have the same hole, which I found the same way on PR #398 (deleting both `validateModels()` call sites left 1478 tests green). I have folded it into #398's scope rather than opening a new ticket, and recorded it in the merge commit message and the Features wiki entry so it cannot get lost. **The lesson worth keeping:** a test that calls the method under test proves the method. It says nothing about the caller. When the whole value of a change is "startup says X", at least one test has to start something.
Some checks are pending
CI / contract (pull_request) Successful in 1m10s
Details
CI / build (pull_request) Successful in 1m25s
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#401
Delete Branch "worker/exhaustion-detection-395-105105-6"

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?