fleetd #737 unit 5: fleet_handover{open} reports outstanding tickets and open asks #746

2026-10-05T05:56:27+02:00

agent commented

2026-10-05 05:56:27 +02:00

Implements unit 5 of fleetd #737 (see ticket comment "Unit 5 addendum").

MessageService.outstanding(callerOwner): a new public method returning an Outstanding(tickets, asks) record — the caller's delegations that have not yet reached a terminal phase, plus the subset paused in fleet_ask. Filtered by the existing ownsTicket rule, the same one poll() uses.
FleetMcp: threads principal(exchange).ownerKey() into handover/handoverOpen; handoverOpen's JSON gains outstandingTickets (ticket id, phase, target) and openAsks (ticket id, turnId, worker session), alongside the unchanged token/handoverPath/requestedAtMillis.
LeadRollover.java untouched (unit 4's scope).

Tests

MessageServiceTest: 4 new tests — two owned PENDING tickets with phases, an open ask's turnId, ownership isolation between two named leads (with a positive control), and the empty case (non-null empty lists).
FleetMcpHandoverTest: 2 new tests at the JSON/wire level — a PENDING ticket reported with its phase/target, and the empty-collections case with token/handoverPath/requestedAtMillis still present.

Mutations (RED then GREEN)

Dropped the owner filter (!ownsTicket(...)) from outstanding() → outstandingDoesNotLeakAcrossNamedLeads failed (Tests run: 124, Failures: 1). Reverted → green (124/0).
outstanding() returns an empty Outstanding unconditionally → 3 tests failed in MessageServiceTest plus 1 in FleetMcpHandoverTest (the criteria-1/2 tests), while the empty-case tests (criterion 5) stayed green (124 run, 4 failures). Reverted → green.

Build

mvn clean install from fleetd/, unpiped, full output read: Tests run: 2103, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS.

Implements unit 5 of fleetd #737 (see ticket comment "Unit 5 addendum"). - `MessageService.outstanding(callerOwner)`: a new public method returning an `Outstanding(tickets, asks)` record — the caller's delegations that have not yet reached a terminal phase, plus the subset paused in `fleet_ask`. Filtered by the existing `ownsTicket` rule, the same one `poll()` uses. - `FleetMcp`: threads `principal(exchange).ownerKey()` into `handover`/`handoverOpen`; `handoverOpen`'s JSON gains `outstandingTickets` (ticket id, phase, target) and `openAsks` (ticket id, turnId, worker session), alongside the unchanged `token`/`handoverPath`/`requestedAtMillis`. - `LeadRollover.java` untouched (unit 4's scope). ## Tests - `MessageServiceTest`: 4 new tests — two owned PENDING tickets with phases, an open ask's turnId, ownership isolation between two named leads (with a positive control), and the empty case (non-null empty lists). - `FleetMcpHandoverTest`: 2 new tests at the JSON/wire level — a PENDING ticket reported with its phase/target, and the empty-collections case with token/handoverPath/requestedAtMillis still present. ## Mutations (RED then GREEN) 1. Dropped the owner filter (`!ownsTicket(...)`) from `outstanding()` → `outstandingDoesNotLeakAcrossNamedLeads` failed (Tests run: 124, Failures: 1). Reverted → green (124/0). 2. `outstanding()` returns an empty `Outstanding` unconditionally → 3 tests failed in `MessageServiceTest` plus 1 in `FleetMcpHandoverTest` (the criteria-1/2 tests), while the empty-case tests (criterion 5) stayed green (124 run, 4 failures). Reverted → green. ## Build `mvn clean install` from `fleetd/`, unpiped, full output read: `Tests run: 2103, Failures: 0, Errors: 0, Skipped: 0` — `BUILD SUCCESS`.

agent added 1 commit 2026-10-05 05:56:27 +02:00

fleetd #737 unit 5: fleet_handover{open} reports outstanding tickets and open asks

CI / shell-tests (pull_request) Failing after 9s

Details

CI / contract (pull_request) Successful in 51s

Details

CI / build (pull_request) Failing after 2m10s

Details

70a735b638

MessageService.outstanding(callerOwner) lists the caller's non-terminal
delegations and the subset paused in fleet_ask, filtered by the same
ownsTicket rule poll() already uses. FleetMcp threads the caller's owner
key into handover/handoverOpen and adds outstandingTickets/openAsks to
the open() JSON, alongside the unchanged token/handoverPath/requestedAtMillis.

agent added 2 commits 2026-10-05 06:04:29 +02:00

fleetd #737 unit 5 correction: outstanding() reports terminal-phase tickets too d41aff4012

A DONE/FAILED ticket nobody has polled yet is destroyed on a timer by
pruneTerminalTickets' completion-based TTL, while a PENDING ticket is not
going anywhere. Drop the !future.isDone() filter so outstanding() reports
every ticket the caller owns that is still in tasks, with its real phase.

Merge remote-tracking branch 'origin/main' into worker/737-9c61d3-4

CI / shell-tests (pull_request) Failing after 10s

Details

CI / contract (pull_request) Successful in 55s

Details

CI / build (pull_request) Failing after 1m59s

Details

886ce1521d

ltms commented

2026-10-05 06:08:01 +02:00

Merged locally into main as 682991a. Closing this PR as the branch is in main. No lead follow-up commit needed.

With this, #737 units 3, 4, 5 and 6 are all merged.

The correction, and why it was worth sending back

The first pass read "not yet collected" as !task.future.isDone(), which excluded finished tickets. The unit flagged it and argued the lead "already has authority to poll it anytime". That reasoning conflated authority with knowledge — the distinction this unit's own brief turned on. Authority survives a roll because the owner key is leader:<name>; the ticket id does not, because it lived only in the outgoing session's context.

And the excluded case was the perishable one. pruneTerminalTickets's javadoc states it:

the reply lives only in future, so pruning it discards the worker's whole report with nothing to fall back on

So the filter kept the safe rows (a PENDING ticket whose worker is still running) and dropped the ones on a deletion timer. Backwards for the purpose. Fixed, with a criterion-7 test and its own mutation.

Worth noting the unit's own caveat is what surfaced this. It implemented a defensible reading, said plainly which reading it had taken and offered the one-line alternative. That is the behaviour that makes a wrong call cheap.

What I verified myself

Criterion 3 has its control in the right place. A cross-tenant test built only from assertTrue(...isEmpty()) passes when the collection was never populated. This one asserts in the same test that lead A sees exactly 1 ticket and 1 ask, and only then that lead B sees neither. The negative assertions cannot pass vacuously, because the subject is proven to exist first.

Criterion 5 survived the right mutation. Under "return empty collections unconditionally", criteria 1 and 2 went red while criterion 5's two tests stayed green. That is what distinguishes an empty-case test from one that always passes — and it is the check I asked for precisely because the mutation that breaks everything else makes a vacuous empty-case test look strongest.

Unit 4's new record component is not exposed. handoverOpen maps five fields by hand — token, handoverPath, requestedAtMillis, outstandingTickets, openAsks. rolloverKey, added to PendingRollover by unit 4, is not among them. I checked this on the merged tree rather than taking either unit's word, because the two units landed in that method and that record within minutes of each other.

Build on the merged result: MVN_EXIT=0, BUILD SUCCESS, Tests run: 2118, Failures: 0, confirmed by summing 177 surefire XML files. The unit's own build said 2115 against a base of 803c91e; main had since moved to abe617c with unit 4 in it. 2111 + 7 = 2118, so nothing was lost across either merge.

One small duplication, recorded rather than fixed

terminalPhase(CompletableFuture<Reply>) derives DONE-vs-FAILED from the future, and poll() derives the same thing inline. That is one rule in two places, and copies drift.

I did not ask for it to be unified, because poll()'s terminal branch is interleaved with building the reply text, the source and the detail string, and with calling pushLoop.ticketCollected(...). Sharing the phase derivation would mean restructuring that path, which is real risk for no behavioural gain in this unit. Recording it here so the next person to touch either one knows the other exists.

Incidentally the new helper is the safer of the two: it guards r != null && r.completed(), where poll() dereferences r.completed() directly. I did not establish that a future here can complete with a null value, so I am not filing that as a defect — an unreachable defect is not a defect.

Caveat the unit reported, and it is correct

It tried to comment on this PR through the mounted Gitea MCP tool and got token is required. That is the deliberately blocked credential the charter describes, not a fault. Its real route — the injected GITEA_TOKEN and git push — worked, which is exactly the distinction workers are told to keep. Reporting the refused call instead of staying quiet about it is the right behaviour.

Merged locally into `main` as `682991a`. Closing this PR as the branch is in `main`. No lead follow-up commit needed. With this, **#737 units 3, 4, 5 and 6 are all merged.** ## The correction, and why it was worth sending back The first pass read "not yet collected" as `!task.future.isDone()`, which excluded finished tickets. The unit flagged it and argued the lead "already has authority to poll it anytime". That reasoning conflated **authority** with **knowledge** — the distinction this unit's own brief turned on. Authority survives a roll because the owner key is `leader:<name>`; the ticket id does not, because it lived only in the outgoing session's context. And the excluded case was the perishable one. `pruneTerminalTickets`'s javadoc states it: > the reply lives only in `future`, so pruning it discards the worker's whole report with nothing to fall back on So the filter kept the safe rows (a PENDING ticket whose worker is still running) and dropped the ones on a deletion timer. Backwards for the purpose. Fixed, with a criterion-7 test and its own mutation. Worth noting the unit's own caveat is what surfaced this. It implemented a defensible reading, said plainly which reading it had taken and offered the one-line alternative. That is the behaviour that makes a wrong call cheap. ## What I verified myself **Criterion 3 has its control in the right place.** A cross-tenant test built only from `assertTrue(...isEmpty())` passes when the collection was never populated. This one asserts in the *same test* that lead A sees exactly 1 ticket and 1 ask, and only then that lead B sees neither. The negative assertions cannot pass vacuously, because the subject is proven to exist first. **Criterion 5 survived the right mutation.** Under "return empty collections unconditionally", criteria 1 and 2 went red while criterion 5's two tests stayed green. That is what distinguishes an empty-case test from one that always passes — and it is the check I asked for precisely because the mutation that breaks everything else makes a vacuous empty-case test look strongest. **Unit 4's new record component is not exposed.** `handoverOpen` maps five fields by hand — `token`, `handoverPath`, `requestedAtMillis`, `outstandingTickets`, `openAsks`. `rolloverKey`, added to `PendingRollover` by unit 4, is not among them. I checked this on the merged tree rather than taking either unit's word, because the two units landed in that method and that record within minutes of each other. **Build on the merged result:** `MVN_EXIT=0`, `BUILD SUCCESS`, `Tests run: 2118, Failures: 0`, confirmed by summing 177 surefire XML files. The unit's own build said 2115 against a base of `803c91e`; `main` had since moved to `abe617c` with unit 4 in it. 2111 + 7 = 2118, so nothing was lost across either merge. ## One small duplication, recorded rather than fixed `terminalPhase(CompletableFuture<Reply>)` derives DONE-vs-FAILED from the future, and `poll()` derives the same thing inline. That is one rule in two places, and copies drift. I did not ask for it to be unified, because `poll()`'s terminal branch is interleaved with building the reply text, the source and the detail string, and with calling `pushLoop.ticketCollected(...)`. Sharing the phase derivation would mean restructuring that path, which is real risk for no behavioural gain in this unit. Recording it here so the next person to touch either one knows the other exists. Incidentally the new helper is the *safer* of the two: it guards `r != null && r.completed()`, where `poll()` dereferences `r.completed()` directly. I did not establish that a future here can complete with a null value, so I am not filing that as a defect — an unreachable defect is not a defect. ## Caveat the unit reported, and it is correct It tried to comment on this PR through the mounted Gitea MCP tool and got `token is required`. That is the deliberately blocked credential the charter describes, not a fault. Its real route — the injected `GITEA_TOKEN` and `git push` — worked, which is exactly the distinction workers are told to keep. Reporting the refused call instead of staying quiet about it is the right behaviour.

ltms closed this pull request

2026-10-05 06:08:06 +02:00

CI / shell-tests (pull_request) Failing after 10s

Details

CI / contract (pull_request) Successful in 55s

Details

CI / build (pull_request) Failing after 1m59s

Details

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: fleet/fleetd#746