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

fleetd #612 Shape A r4: pin quarantineSource + outageSource through both operator windows #648

Closed
agent wants to merge 0 commits from worker/612-a-r4-quarantine-outage-7ab0e8-5 into main
pull from: worker/612-a-r4-quarantine-outage-7ab0e8-5
merge into: fleet:main
Conversation 1 Commits - Files Changed -
agent commented 2026-10-02 04:21:14 +02:00
Member
Copy Link
Copy Source

Closes part of fleetd #612 (Shape A, unit r4).

Scope: FleetdAssembly.java's quarantineSource (:471-472) and outageSource (:473-476), each wired twice -- into FleetMcp (fleet_profiles, :484/:486) and again into FleetApp (GET /profiles, :529). No existing test distinguished the two windows for either source.

What the new test does: drives the real FleetdAssembly.assembleAndStart, classifies a real exhaustion (quarantine) and two real backend errors on distinct targets (cool-off) through the real CompletionResolver, then reads the result back through a real McpSyncClient call to fleet_profiles and a real HttpClient GET /profiles -- both authenticated via token-mode auth to sidestep the in-JVM pid-resolution dead end (client and server share one JVM pid, so loopback-trust auth never resolves in-process). No assertion reads .java source text. No production code was changed.

Verification: six mutation cycles (3 per site x 2 sites), each line-anchored (grep -n, anchor count 1 before/after), each run against the full unfiltered mvn -o test suite, each reverted immediately after:

Site Cycle Mutation Full-suite result
quarantineSource i FleetMcp arg -> QuarantineSource.none() 1896 run, 4 failed (my MCP test + 3 pre-existing tests sharing the FleetMcp#quarantineSource() accessor for their own unrelated assertions)
quarantineSource ii FleetApp arg -> QuarantineSource.none() 1896 run, 1 failed (my REST test only)
quarantineSource iii mis-wire: FleetMcp arg -> disconnected fresh BackendQuarantine 1896 run, 3 failed (my MCP test + 2 of the same pre-existing tests)
outageSource i FleetMcp arg -> OutageSource.none() 1896 run, 1 failed (my MCP test only)
outageSource ii FleetApp arg -> OutageSource.none() 1896 run, 1 failed (my REST test only)
outageSource iii mis-wire: FleetMcp arg -> disconnected fresh BackendOutagePolicy 1896 run, 1 failed (my MCP test only)

In every cycle my own test's two assertions (MCP vs REST) moved independently, confirming neither window masks the other. git diff --stat on FleetdAssembly.java is empty after all reverts.

Pre-existing coupling noted, not introduced here: FleetdBackendQuarantineAssemblyTest, FleetdExhaustedPatternAssemblyTest and FleetdOpenCodeExhaustionForwardingAssemblyTest all read the live BackendQuarantine via runtime.mcp().quarantineSource().quarantine() for their own, different assertions, so a fully-disconnected (.none()) quarantineSource mutation incidentally trips them too. This is accidental sharing of a test-only accessor, not a duplicate of the property this PR pins (that fleet_profiles/GET /profiles actually report the quarantined/coolingOff JSON sections).

Teardown: @AfterEach stops the bound Javalin app and runs the captured ResourcePorts shutdown hook (FleetdRuntime.close() path), per the ticket's one-JVM-fork Surefire warning.

Out of scope, noted per the ticket's own 2026-10-02 dispatch comment: loopHealthSource (FleetdAssembly.java:478) shares this exact two-consumer (FleetMcp + FleetApp) shape and has no distinguishing test either, but it is already assigned to a separate unit, r10. Not investigated further here.

Build: mvn clean install in my own worktree, unpiped -- Tests run: 1896, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.

Closes part of fleetd #612 (Shape A, unit r4). **Scope**: `FleetdAssembly.java`'s `quarantineSource` (:471-472) and `outageSource` (:473-476), each wired twice -- into `FleetMcp` (`fleet_profiles`, :484/:486) and again into `FleetApp` (`GET /profiles`, :529). No existing test distinguished the two windows for either source. **What the new test does**: drives the real `FleetdAssembly.assembleAndStart`, classifies a real exhaustion (quarantine) and two real backend errors on distinct targets (cool-off) through the real `CompletionResolver`, then reads the result back through a real `McpSyncClient` call to `fleet_profiles` and a real `HttpClient` `GET /profiles` -- both authenticated via token-mode auth to sidestep the in-JVM pid-resolution dead end (client and server share one JVM pid, so loopback-trust auth never resolves in-process). No assertion reads `.java` source text. No production code was changed. **Verification**: six mutation cycles (3 per site x 2 sites), each line-anchored (`grep -n`, anchor count 1 before/after), each run against the full unfiltered `mvn -o test` suite, each reverted immediately after: | Site | Cycle | Mutation | Full-suite result | |---|---|---|---| | quarantineSource | i | FleetMcp arg -> `QuarantineSource.none()` | 1896 run, 4 failed (my MCP test + 3 pre-existing tests sharing the `FleetMcp#quarantineSource()` accessor for their own unrelated assertions) | | quarantineSource | ii | FleetApp arg -> `QuarantineSource.none()` | 1896 run, 1 failed (my REST test only) | | quarantineSource | iii | mis-wire: FleetMcp arg -> disconnected fresh `BackendQuarantine` | 1896 run, 3 failed (my MCP test + 2 of the same pre-existing tests) | | outageSource | i | FleetMcp arg -> `OutageSource.none()` | 1896 run, 1 failed (my MCP test only) | | outageSource | ii | FleetApp arg -> `OutageSource.none()` | 1896 run, 1 failed (my REST test only) | | outageSource | iii | mis-wire: FleetMcp arg -> disconnected fresh `BackendOutagePolicy` | 1896 run, 1 failed (my MCP test only) | In every cycle my own test's two assertions (MCP vs REST) moved independently, confirming neither window masks the other. `git diff --stat` on `FleetdAssembly.java` is empty after all reverts. **Pre-existing coupling noted, not introduced here**: `FleetdBackendQuarantineAssemblyTest`, `FleetdExhaustedPatternAssemblyTest` and `FleetdOpenCodeExhaustionForwardingAssemblyTest` all read the live `BackendQuarantine` via `runtime.mcp().quarantineSource().quarantine()` for their own, different assertions, so a fully-disconnected (`.none()`) quarantineSource mutation incidentally trips them too. This is accidental sharing of a test-only accessor, not a duplicate of the property this PR pins (that `fleet_profiles`/`GET /profiles` actually report the quarantined/coolingOff JSON sections). **Teardown**: `@AfterEach` stops the bound Javalin app and runs the captured `ResourcePorts` shutdown hook (`FleetdRuntime.close()` path), per the ticket's one-JVM-fork Surefire warning. **Out of scope, noted per the ticket's own 2026-10-02 dispatch comment**: `loopHealthSource` (`FleetdAssembly.java`:478) shares this exact two-consumer (FleetMcp + FleetApp) shape and has no distinguishing test either, but it is already assigned to a separate unit, r10. Not investigated further here. **Build**: `mvn clean install` in my own worktree, unpiped -- `Tests run: 1896, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`.
agent added 1 commit 2026-10-02 04:21:14 +02:00
fleetd #612 Shape A unit r4: pin quarantineSource + outageSource through both operator windows
CI / shell-tests (pull_request) Failing after 9s
Details
CI / contract (pull_request) Successful in 1m29s
Details
CI / build (pull_request) Failing after 1m56s
Details
91792e11fc 
FleetdAssembly.java's quarantineSource (:471-472) and outageSource (:473-476)
each feed two consumers: FleetMcp (fleet_profiles, :484/:486) and FleetApp
(GET /profiles, :529). No existing test distinguished the two windows for
either source.

New test drives the real FleetdAssembly.assembleAndStart, classifies a real
exhaustion/outage through the real CompletionResolver, and reads the result
back through a real McpSyncClient (fleet_profiles) and a real HttpClient
(GET /profiles), both authenticated via token-mode auth (sidesteps the
in-JVM pid-resolution dead end). No source text is read; no production code
changed.

Verified with six mutation cycles (3 per site x 2 sites: FleetMcp starved,
FleetApp starved, mis-wire with a disconnected collaborator), each run
against the full unfiltered suite and reverted after confirming the
expected test(s) alone went red. The quarantineSource FleetMcp-starve and
mis-wire cycles also trip three pre-existing tests that read the live
BackendQuarantine via FleetMcp#quarantineSource() for their own unrelated
assertions - a pre-existing incidental coupling, not newly introduced here.

Out of scope, noted per the ticket's dispatch comment: loopHealthSource
(FleetdAssembly.java:478) shares this same two-consumer shape and is
already assigned to a separate unit, r10.
ltms commented 2026-10-02 04:53:08 +02:00
Owner
Copy Link
Copy Source

Lead verification — merging. The REST half is unique coverage; the MCP quarantine half is not.

I reproduced the mutation evidence myself rather than taking the report's word for it, and I had the duplicate-coverage claim checked independently. Both halves of the result are below. Two reviewers ran against the diff.

1. The dual-window claim holds — my own measurement

Line-anchored mutations in a throwaway detached worktree at 91792e1, anchor grep -c 1 → 0 before/after each edit.

Starve the FleetApp pass site only (FleetdAssembly.java:529, both args → none()):

Tests run: 4, Failures: 2 — BUILD FAILURE
  quarantinedCredentialIsReportedByTheRealAssembledFleetApp:333
  coolingOffCredentialIsReportedByTheRealAssembledFleetApp:363

Both ...FleetMcp tests stayed green.

Starve the FleetMcp pass site only (:484 and :486 → none()):

Tests run: 4, Failures: 2 — BUILD FAILURE
  quarantinedCredentialIsReportedByTheRealAssembledFleetMcp:319
  coolingOffCredentialIsReportedByTheRealAssembledFleetMcp:349

Both ...FleetApp tests stayed green.

So the two windows really are pinned apart — starving either kills exactly its own two tests and no others. And the failure messages carry the live JSON response body from the real round-trip, not a source-text match, which is the behavioural evidence this ticket family is after.

2. The REST window was completely unpinned — this is what makes the PR worth merging

The question the PR could not answer about itself: does anything else already catch a starved REST pass site? Measured by parking this PR's test file and starving :529, then running the full unfiltered suite:

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

Zero pre-existing tests notice that the REST window lost its quarantine and outage sources. An operator reads GET /profiles exactly when the MCP mount is down, and until this PR nothing protected it. That is real, unique coverage.

Arithmetic control, because a total is a claim: 1892 with this PR's 4 tests parked, + 4 = 1896, which is main's total at 6539efe. The counts reconcile.

3. The duplicate-coverage caveat — the report's conclusion was wrong, not its observation

The report noted that mutating the quarantineSource site also tripped three pre-existing tests, and read that as "pre-existing incidental coupling, not introduced by this PR." An independent reviewer measured it on a clean origin/main worktree with this PR's file absent:

  • baseline 1896/0/0/0
  • MCP arg → FleetMcp.QuarantineSource.none(): 1896/3/0/0 — all three named tests fail
  • MCP arg → a separate escalating BackendQuarantine: 1896/2/0/0

Confirmed: FleetdBackendQuarantineAssemblyTest:167, FleetdExhaustedPatternAssemblyTest:202 and FleetdOpenCodeExhaustionForwardingAssemblyTest:182 all reach it through runtime.mcp().quarantineSource().quarantine().

The coupling is pre-existing — that half of the report is right. But it follows that the FleetMcp quarantineSource site was already pinned, so this PR's MCP-side quarantine assertion (:313) is duplicate coverage, and the claim that the site was unpinned is false.

Severity low, and not a reason to hold the PR: a redundant assertion costs a little runtime and nothing else. Leaving it in place rather than asking for a revision, since it is still a valid assertion about fleet_profiles output. But the record needed correcting — a later reader finding :313 and treating it as the sole guard for that site would be wrong twice over.

The reasoning trap worth naming

"Those other tests also fail" is consistent with two opposite conclusions: harmless incidental coupling, or the site was already covered. The report picked the first and stopped. Nothing about the observation favours it — only a mutation on clean origin/main separates them, and that measurement was never run.

Generalising, because this shape will recur in the rest of Shape A: a mutation that kills extra tests is evidence about coverage you already have, not noise to explain away. The instinct to classify an unexpected red as "unrelated" is the instinct that loses the finding. Credit to the report for flagging the anomaly at all instead of tidying it away — that is what made it checkable.

Merging locally and pushing. Detail on #612.

## Lead verification — merging. The REST half is unique coverage; the MCP quarantine half is not. I reproduced the mutation evidence myself rather than taking the report's word for it, and I had the duplicate-coverage claim checked independently. Both halves of the result are below. Two reviewers ran against the diff. ### 1. The dual-window claim holds — my own measurement Line-anchored mutations in a throwaway detached worktree at `91792e1`, anchor `grep -c` 1 → 0 before/after each edit. **Starve the FleetApp pass site only** (`FleetdAssembly.java:529`, both args → `none()`): ``` Tests run: 4, Failures: 2 — BUILD FAILURE quarantinedCredentialIsReportedByTheRealAssembledFleetApp:333 coolingOffCredentialIsReportedByTheRealAssembledFleetApp:363 ``` Both `...FleetMcp` tests stayed green. **Starve the FleetMcp pass site only** (`:484` and `:486` → `none()`): ``` Tests run: 4, Failures: 2 — BUILD FAILURE quarantinedCredentialIsReportedByTheRealAssembledFleetMcp:319 coolingOffCredentialIsReportedByTheRealAssembledFleetMcp:349 ``` Both `...FleetApp` tests stayed green. So the two windows really are pinned apart — starving either kills exactly its own two tests and no others. And the failure messages carry the **live JSON response body** from the real round-trip, not a source-text match, which is the behavioural evidence this ticket family is after. ### 2. The REST window was completely unpinned — this is what makes the PR worth merging The question the PR could not answer about itself: does anything *else* already catch a starved REST pass site? Measured by parking this PR's test file and starving `:529`, then running the full unfiltered suite: ``` Tests run: 1892, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` **Zero pre-existing tests notice that the REST window lost its quarantine and outage sources.** An operator reads `GET /profiles` exactly when the MCP mount is down, and until this PR nothing protected it. That is real, unique coverage. Arithmetic control, because a total is a claim: 1892 with this PR's 4 tests parked, + 4 = 1896, which is `main`'s total at `6539efe`. The counts reconcile. ### 3. The duplicate-coverage caveat — the report's conclusion was wrong, not its observation The report noted that mutating the `quarantineSource` site also tripped three pre-existing tests, and read that as "pre-existing incidental coupling, not introduced by this PR." An independent reviewer measured it on a clean `origin/main` worktree with this PR's file absent: - baseline `1896/0/0/0` - MCP arg → `FleetMcp.QuarantineSource.none()`: `1896/3/0/0` — all three named tests fail - MCP arg → a separate escalating `BackendQuarantine`: `1896/2/0/0` Confirmed: `FleetdBackendQuarantineAssemblyTest:167`, `FleetdExhaustedPatternAssemblyTest:202` and `FleetdOpenCodeExhaustionForwardingAssemblyTest:182` all reach it through `runtime.mcp().quarantineSource().quarantine()`. The coupling **is** pre-existing — that half of the report is right. But it follows that **the FleetMcp `quarantineSource` site was already pinned**, so this PR's MCP-side quarantine assertion (`:313`) is duplicate coverage, and the claim that the site was unpinned is false. Severity low, and not a reason to hold the PR: a redundant assertion costs a little runtime and nothing else. Leaving it in place rather than asking for a revision, since it is still a valid assertion about `fleet_profiles` output. **But the record needed correcting** — a later reader finding `:313` and treating it as the sole guard for that site would be wrong twice over. ### The reasoning trap worth naming "Those other tests also fail" is consistent with two opposite conclusions: harmless incidental coupling, or the site was already covered. The report picked the first and stopped. Nothing about the observation favours it — only a mutation on clean `origin/main` separates them, and that measurement was never run. Generalising, because this shape will recur in the rest of Shape A: **a mutation that kills extra tests is evidence about coverage you already have, not noise to explain away.** The instinct to classify an unexpected red as "unrelated" is the instinct that loses the finding. Credit to the report for flagging the anomaly at all instead of tidying it away — that is what made it checkable. Merging locally and pushing. Detail on #612.
ltms closed this pull request 2026-10-02 05:00:48 +02:00
Some checks are pending
CI / shell-tests (pull_request) Failing after 9s
Details
CI / contract (pull_request) Successful in 1m29s
Details
CI / build (pull_request) Failing after 1m56s
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#648
Delete Branch "worker/612-a-r4-quarantine-outage-7ab0e8-5"

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?