fleetd #705: close the REST ticket-poll door #716

2026-10-04T07:23:56+02:00

agent commented

2026-10-04 07:23:56 +02:00

Follow-up to the MCP half merged in c468953 (PR #712).

Part 1 -- close the REST door

GET /tasks/{ticket} (FleetApp.taskStatus) called MessageService.poll(ticket), the no-check overload. It now resolves the caller the same way allow(...) does (ctx.attribute(CALLER)) and threads caller.terminal() into MessageService.poll(ticket, callerTerminal), matching the shape of the MCP fleet_poll handler. A caller with no terminal (the unnamed primary) is unaffected -- that behaviour lives in MessageService.ownsTicket and was not touched.

Part 2 -- pin both handlers

Added a source-scrape guard test (with its own control assertion, following FleetMcpAuthzTest#theFleetListHandlerActuallyConsultsCollaboratorsVisibleTo's idiom) for:

the fleet_poll MCP handler (FleetMcpAuthzTest#theFleetPollHandlerActuallyThreadsCallerTerminalIntoPoll)
the REST taskStatus route (FleetAppAuthTest#theTaskStatusRouteActuallyThreadsTheCallersTerminalIntoPoll)

Also added a behavioural REST test (FleetAppAuthTest#restPollRefusesADifferentWorkerButAllowsTheCreatorAndTheUnnamedPrimary) that drives GET /tasks/{ticket} with three differently-resolved callers (two different workers, plus the unnamed primary) sharing one MessageService. The ticket is minted directly on the shared service (the way MessageServiceTest does), since REST's own wait:false send path (FleetApp.sendMessage -> messages.sendAsync(id, content)) does not thread a creator terminal through yet -- out of this ticket's scope, noted below.

Verification

mvn clean install: BUILD SUCCESS, Tests run: 2011, Failures: 0, Errors: 0, Skipped: 0 (main was 2008; +3 new tests).
Mutation on the MCP handler: replaced callerTerminal(exchange) with null at the fleet_poll handler's poll(...) call (line-anchored sed). mvn compile stayed green (mutation live). Full suite: 1 failure -- FleetMcpAuthzTest.theFleetPollHandlerActuallyThreadsCallerTerminalIntoPoll. Restored, full suite green again.
Mutation on the REST route: same procedure on the new taskStatus line. Compile green. Full suite: 2 failures -- FleetAppAuthTest.theTaskStatusRouteActuallyThreadsTheCallersTerminalIntoPoll and FleetAppAuthTest.restPollRefusesADifferentWorkerButAllowsTheCreatorAndTheUnnamedPrimary. Restored, full suite green again.
Control check on both new scrape guards: changed the searched-for call name to a non-matching string; each failed with its own control message ("control failed: ... the anchors have drifted ..."), not a downstream assertion. Restored.

Out of scope (not fixed, noted only)

FleetApp.sendMessage's wait:false path calls messages.sendAsync(id, content) -- the overload that records no creator terminal -- while the MCP fleet_send path already threads one through. A named lead/architect creating an async ticket over REST therefore has no recorded owner on it, which would make that same lead's own later REST poll of it look identical to an unrelated caller's. Left alone since it's outside this ticket's stated scope (the taskStatus/poll route only).

Also spotted the same "an identifier used as an authorization token" shape is worth a broader sweep elsewhere in this codebase, per the brief's instruction -- not investigated further here.

Follow-up to the MCP half merged in c468953 (PR #712). ## Part 1 -- close the REST door `GET /tasks/{ticket}` (`FleetApp.taskStatus`) called `MessageService.poll(ticket)`, the no-check overload. It now resolves the caller the same way `allow(...)` does (`ctx.attribute(CALLER)`) and threads `caller.terminal()` into `MessageService.poll(ticket, callerTerminal)`, matching the shape of the MCP `fleet_poll` handler. A caller with no terminal (the unnamed primary) is unaffected -- that behaviour lives in `MessageService.ownsTicket` and was not touched. ## Part 2 -- pin both handlers Added a source-scrape guard test (with its own control assertion, following `FleetMcpAuthzTest#theFleetListHandlerActuallyConsultsCollaboratorsVisibleTo`'s idiom) for: - the `fleet_poll` MCP handler (`FleetMcpAuthzTest#theFleetPollHandlerActuallyThreadsCallerTerminalIntoPoll`) - the REST `taskStatus` route (`FleetAppAuthTest#theTaskStatusRouteActuallyThreadsTheCallersTerminalIntoPoll`) Also added a behavioural REST test (`FleetAppAuthTest#restPollRefusesADifferentWorkerButAllowsTheCreatorAndTheUnnamedPrimary`) that drives `GET /tasks/{ticket}` with three differently-resolved callers (two different workers, plus the unnamed primary) sharing one `MessageService`. The ticket is minted directly on the shared service (the way `MessageServiceTest` does), since REST's own `wait:false` send path (`FleetApp.sendMessage` -> `messages.sendAsync(id, content)`) does not thread a creator terminal through yet -- out of this ticket's scope, noted below. ## Verification - `mvn clean install`: BUILD SUCCESS, Tests run: 2011, Failures: 0, Errors: 0, Skipped: 0 (main was 2008; +3 new tests). - Mutation on the MCP handler: replaced `callerTerminal(exchange)` with `null` at the `fleet_poll` handler's `poll(...)` call (line-anchored sed). `mvn compile` stayed green (mutation live). Full suite: 1 failure -- `FleetMcpAuthzTest.theFleetPollHandlerActuallyThreadsCallerTerminalIntoPoll`. Restored, full suite green again. - Mutation on the REST route: same procedure on the new `taskStatus` line. Compile green. Full suite: 2 failures -- `FleetAppAuthTest.theTaskStatusRouteActuallyThreadsTheCallersTerminalIntoPoll` and `FleetAppAuthTest.restPollRefusesADifferentWorkerButAllowsTheCreatorAndTheUnnamedPrimary`. Restored, full suite green again. - Control check on both new scrape guards: changed the searched-for call name to a non-matching string; each failed with its own control message ("control failed: ... the anchors have drifted ..."), not a downstream assertion. Restored. ## Out of scope (not fixed, noted only) `FleetApp.sendMessage`'s `wait:false` path calls `messages.sendAsync(id, content)` -- the overload that records no creator terminal -- while the MCP `fleet_send` path already threads one through. A named lead/architect creating an async ticket over REST therefore has no recorded owner on it, which would make that same lead's own later REST poll of it look identical to an unrelated caller's. Left alone since it's outside this ticket's stated scope (the `taskStatus`/poll route only). Also spotted the same "an identifier used as an authorization token" shape is worth a broader sweep elsewhere in this codebase, per the brief's instruction -- not investigated further here.

agent added 1 commit 2026-10-04 07:23:56 +02:00

fleetd #705 : close the REST ticket-poll door and pin both handlers' caller-terminal wiring

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

Details

CI / contract (pull_request) Successful in 51s

Details

CI / build (pull_request) Failing after 1m47s

Details

0ba597e394

GET /tasks/{ticket} now resolves the caller the same way allow(...) does and
threads that terminal into MessageService.poll(ticket, callerTerminal) instead
of the no-check overload, so a worker can no longer read a ticket a different
session created over REST. Adds a source-scrape guard (with its own control
assertion) for both the fleet_poll MCP handler and this REST route, plus a
behavioural test driving GET /tasks/{ticket} with three differently-resolved
callers against one shared MessageService.

ltms commented

2026-10-04 07:27:01 +02:00

Lead review — one more thing must ship in this PR before I merge it

I checked your work in the main clone and on your pushed commit 0ba597e. Part 1 and Part 2 are
good, and I confirmed the two production call sites myself:

$ git grep -n "\.poll(" 0ba597e -- fleetd/src/main/java
FleetMcp.java:1203:  messages.poll(ticket, callerTerminal)
FleetApp.java:898:   messages.poll(ctx.pathParam("ticket"), caller == null ? null : caller.terminal())

I also checked the caller == null branch you added. It is sound, and here is why, so nobody has
to re-derive it: CALLER is only set by the app.before filter when auth != null
(FleetApp.java:244), and allow(...) returns true with no check in that same auth == null
case. So the null branch is reachable only in legacy mode, where nothing is enforced at all. And
when auth != null, Authz.permits denies a null or anonymous caller outright at
auth/Authz.java:97, so no unauthenticated caller ever reaches line 898.

The follow-up: your "out of scope" note is a real break, and it is this PR's to fix

You flagged that FleetApp.sendMessage's wait:false path calls messages.sendAsync(id, content),
the overload that records no creator terminal. I verified it at FleetApp.java:697 on your commit.
You were right to flag it, and I am ruling that it has to be fixed here rather than deferred.

The reason it cannot wait: ownsTicket is
callerTerminal == null || callerTerminal.equals(task.creatorTerminal). A REST-created ticket has
creatorTerminal == null, so for a terminal-bearing caller the comparison is
"term_x".equals(null), which is false. So after your fix, a lead that creates an async ticket
over REST can no longer poll its own ticket. Only the unnamed primary can. Driving the fleet over
REST is a documented fallback for when the MCP mount drops, so this is reachable, not theoretical.

Merging Part 1 on its own would close a read hole and break a working path in the same commit. I am
not shipping that half.

Scope of the follow-up — this and nothing else

Thread the creating caller's terminal into the REST wait:false send. Use the 4-arg
sendAsync(target, content, onAccepted, creatorTerminal) overload, and take the terminal from the
same ctx.attribute(CALLER) source you used for taskStatus — not a second resolution path.
Keep passing whatever onAccepted that path passes today; do not change the delegation-ownership
hook.
One behavioural test: create the ticket over REST as a terminal-bearing caller, then poll it
over REST as that same caller, and assert it is not refused. Add the refusal half too — a
different terminal-bearing caller must still be refused — so the test cannot pass by simply
disabling the check.
Mutate your own fix to prove the test holds it: line-anchored sed on the new
creatorTerminal argument, mvn -o compile confirmed green first so you know the mutation is
live and not a compile error, then the suite, then restore and confirm the file is byte-identical.
Break the new test's own control and quote the exact message it prints, as you did for the
other two.

What I do not want

Do not touch ownsTicket, Authz, Principal, or the role table.
Do not change the MCP send path; it already threads a creator.
Do not go hunting for other instances of the "identifier used as an authorization token" shape.
That is #715 and it is already designed; it is not yours.
Do not merge. Push to the same branch so this PR updates.

Report back

Give me the real mvn clean install numbers from fleetd/, written to a file rather than piped —
a pipe hides a failure behind a zero exit — and state the test-count arithmetic against the 2011
you reported. End your turn with exactly one fleet_reply carrying the whole report.

## Lead review — one more thing must ship in this PR before I merge it I checked your work in the main clone and on your pushed commit `0ba597e`. Part 1 and Part 2 are good, and I confirmed the two production call sites myself: ``` $ git grep -n "\.poll(" 0ba597e -- fleetd/src/main/java FleetMcp.java:1203: messages.poll(ticket, callerTerminal) FleetApp.java:898: messages.poll(ctx.pathParam("ticket"), caller == null ? null : caller.terminal()) ``` I also checked the `caller == null` branch you added. It is sound, and here is why, so nobody has to re-derive it: `CALLER` is only set by the `app.before` filter when `auth != null` (`FleetApp.java:244`), and `allow(...)` returns `true` with no check in that same `auth == null` case. So the null branch is reachable only in legacy mode, where nothing is enforced at all. And when `auth != null`, `Authz.permits` denies a null or anonymous caller outright at `auth/Authz.java:97`, so no unauthenticated caller ever reaches line 898. ### The follow-up: your "out of scope" note is a real break, and it is this PR's to fix You flagged that `FleetApp.sendMessage`'s `wait:false` path calls `messages.sendAsync(id, content)`, the overload that records no creator terminal. I verified it at `FleetApp.java:697` on your commit. You were right to flag it, and I am ruling that it has to be fixed here rather than deferred. The reason it cannot wait: `ownsTicket` is `callerTerminal == null || callerTerminal.equals(task.creatorTerminal)`. A REST-created ticket has `creatorTerminal == null`, so for a terminal-bearing caller the comparison is `"term_x".equals(null)`, which is `false`. **So after your fix, a lead that creates an async ticket over REST can no longer poll its own ticket.** Only the unnamed primary can. Driving the fleet over REST is a documented fallback for when the MCP mount drops, so this is reachable, not theoretical. Merging Part 1 on its own would close a read hole and break a working path in the same commit. I am not shipping that half. ### Scope of the follow-up — this and nothing else 1. **Thread the creating caller's terminal into the REST `wait:false` send.** Use the 4-arg `sendAsync(target, content, onAccepted, creatorTerminal)` overload, and take the terminal from the same `ctx.attribute(CALLER)` source you used for `taskStatus` — not a second resolution path. Keep passing whatever `onAccepted` that path passes today; do not change the delegation-ownership hook. 2. **One behavioural test**: create the ticket over REST as a terminal-bearing caller, then poll it over REST as that same caller, and assert it is **not** refused. Add the refusal half too — a different terminal-bearing caller must still be refused — so the test cannot pass by simply disabling the check. 3. **Mutate your own fix** to prove the test holds it: line-anchored `sed` on the new `creatorTerminal` argument, `mvn -o compile` confirmed green first so you know the mutation is live and not a compile error, then the suite, then restore and confirm the file is byte-identical. 4. **Break the new test's own control** and quote the exact message it prints, as you did for the other two. ### What I do not want - Do not touch `ownsTicket`, `Authz`, `Principal`, or the role table. - Do not change the MCP send path; it already threads a creator. - Do not go hunting for other instances of the "identifier used as an authorization token" shape. That is #715 and it is already designed; it is not yours. - Do not merge. Push to the same branch so this PR updates. ### Report back Give me the real `mvn clean install` numbers from `fleetd/`, written to a file rather than piped — a pipe hides a failure behind a zero exit — and state the test-count arithmetic against the 2011 you reported. End your turn with exactly one `fleet_reply` carrying the whole report.

ltms referenced this pull request

2026-10-04 07:35:37 +02:00

Two convenience overloads have dangerous defaults, and one of them fails open: pin that no production code reaches for MessageService.poll(String) #718

agent added 1 commit 2026-10-04 07:41:18 +02:00

fleetd: thread the creating caller's terminal into the REST fire-and-poll send path

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

Details

CI / contract (pull_request) Successful in 53s

Details

CI / build (pull_request) Failing after 1m53s

Details

29a2f97c25

sendMessage's wait:false branch now records the resolved caller's own
terminal as the ticket's creatorTerminal, the same way taskStatus already
resolves its caller, so a REST-created ticket's own creator can still poll
it under the ownership check that now gates GET /tasks/{ticket}.

agent commented

2026-10-04 07:41:30 +02:00

Pushed the third unit from comment 18645.

Fix: FleetApp.sendMessage's wait:false branch now reads ctx.attribute(CALLER) (the same way taskStatus and allow(...) do) and calls the 4-arg MessageService.sendAsync(id, content, null, creatorTerminal) overload, instead of the no-creator 2-arg overload. onAccepted stays null, same as before.

New behavioral test (FleetAppAuthTest.restSendAsyncRecordsTheCreatingCallersTerminalSoItCanStillPollItsOwnTicket): creates a ticket over REST as a named-lead caller (terminal-bearing, has SEND), polls it over REST as that same caller (not refused), and polls it as a different terminal-bearing caller (refused, with the exact ownsTicket detail message as the control).

Mutation test: line-anchored sed replaced the new caller == null ? null : caller.terminal() argument at sendAsync(...) with a literal null. mvn -o compile stayed green (mutation is live, not a compile error). Full suite run: exactly 1 failure, in exactly the new test, with message the session that created the ticket over REST must be able to poll it: ..."detail":"forbidden: this ticket was created by a different session". Restored from backup; diff confirmed byte-identical to pre-mutation.

Control break: temporarily changed the new test's refusal-detail literal to a non-matching string and ran only that test; it failed with its own message: a different terminal must still be refused with the ownership detail, not some other rejection: .... Restored; diff confirmed byte-identical.

Full build: mvn clean install -> BUILD SUCCESS, Tests run: 2012, Failures: 0, Errors: 0, Skipped: 0 (2011 baseline + 1 new test = 2012, confirmed against the aggregated surefire reports).

Did not touch ownsTicket, Authz, Principal, the role table, or the MCP send path. Not merging.

Pushed the third unit from comment 18645. **Fix:** `FleetApp.sendMessage`'s `wait:false` branch now reads `ctx.attribute(CALLER)` (the same way `taskStatus` and `allow(...)` do) and calls the 4-arg `MessageService.sendAsync(id, content, null, creatorTerminal)` overload, instead of the no-creator 2-arg overload. `onAccepted` stays `null`, same as before. **New behavioral test** (`FleetAppAuthTest.restSendAsyncRecordsTheCreatingCallersTerminalSoItCanStillPollItsOwnTicket`): creates a ticket over REST as a named-lead caller (terminal-bearing, has SEND), polls it over REST as that same caller (not refused), and polls it as a different terminal-bearing caller (refused, with the exact `ownsTicket` detail message as the control). **Mutation test:** line-anchored `sed` replaced the new `caller == null ? null : caller.terminal()` argument at `sendAsync(...)` with a literal `null`. `mvn -o compile` stayed green (mutation is live, not a compile error). Full suite run: exactly 1 failure, in exactly the new test, with message `the session that created the ticket over REST must be able to poll it: ..."detail":"forbidden: this ticket was created by a different session"`. Restored from backup; `diff` confirmed byte-identical to pre-mutation. **Control break:** temporarily changed the new test's refusal-detail literal to a non-matching string and ran only that test; it failed with its own message: `a different terminal must still be refused with the ownership detail, not some other rejection: ...`. Restored; `diff` confirmed byte-identical. **Full build:** `mvn clean install` -> BUILD SUCCESS, `Tests run: 2012, Failures: 0, Errors: 0, Skipped: 0` (2011 baseline + 1 new test = 2012, confirmed against the aggregated surefire reports). Did not touch `ownsTicket`, `Authz`, `Principal`, the role table, or the MCP send path. Not merging.

ltms referenced this issue from a commit

2026-10-04 07:48:29 +02:00

Merge PR #716: fleetd #705 — gate the REST ticket routes by the creating caller

ltms commented

2026-10-04 07:49:49 +02:00

Merged to `main` as `9a64d42`. Closing by hand — a local merge does not close a PR here.

All three parts are in. Good unit, and the thing that made it good was flagging the REST send gap
instead of filing it away as someone else's problem.

I resolved a conflict, and the test count is how I checked the resolution

This branch and PR #717 both appended tests to FleetMcpAuthzTest.java, so the merge conflicted. It
was addition against addition, so I kept both sides and closed the two tests properly.

The count is the real check on that: main was 2014 after #717, this branch adds 3 from the first
round and 1 from the second, and the merged build reports 2018. If my resolution had swallowed a
test it would read 2017. All five tests are present exactly once — I checked each by name.

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

Three mutations, all mine, each confirmed live before the suite ran

mutation	compile	kills
`FleetMcp.java:527` — poll's `callerTerminal(exchange)` → `null`	exit 0, live	1: `theFleetPollHandlerActuallyThreadsCallerTerminalIntoPoll`
`FleetApp.java:698` — REST `sendAsync` creator → `null`	exit 0, live	1: `restSendAsyncRecordsTheCreatingCallersTerminalSoItCanStillPollItsOwnTicket`
`FleetApp.java:899` — REST poll caller → `null`	exit 0, live	3, including `restPollRefusesADifferentWorkerButAllowsTheCreatorAndTheUnnamedPrimary`

FleetMcp.java:527 is the one that matters most. Before this unit, that exact mutation survived
the full suite — the fix could be switched off and nothing noticed. It now fails. That is the
specific thing this unit was asked to close, and it is closed.

Every file was restored and confirmed byte-identical, and the merged tree is byte-identical to the
tree I built and mutated (143e08f).

On your javadoc caveat

You asked whether "a caller with SEND permission" on the new startOnSharedService overload reads as
a current contract rather than history. It does. It says what the parameter is for, which is what a
maintainer of that line needs. It names no ticket, no past behaviour and no reason-for-a-reviewer, so
it is on the right side of the rule.

Documentation, which was mine

wiki/11-Features.md now carries an entry for this gate, including the two gotchas that are easy to
lose: the unnamed primary is never refused, and that is safe only because an anonymous caller is
denied TASK_READ first — nothing in the code pins that coupling. The second gotcha is the split
this PR's follow-up fixed, written down so nobody reintroduces half of it.

One thing I took from your work into a new ticket

Your fix leaves MessageService.poll(String) with no production caller. It defaults the terminal to
null, which skips the ownership check — it fails open, silently, which is exactly the hole this
PR closed. It has 44 test callers, so deleting it is the wrong trade. #718 tracks pinning it with a
scrape test. I also found a second instance of the same shape while reviewing: FleetMcp.java:975
forwards sendAsync with creatorTerminal defaulted to null. Not yours to fix; recorded so it is
not rediscovered.

## Merged to `main` as `9a64d42`. Closing by hand — a local merge does not close a PR here. All three parts are in. Good unit, and the thing that made it good was flagging the REST send gap instead of filing it away as someone else's problem. ### I resolved a conflict, and the test count is how I checked the resolution This branch and PR #717 both appended tests to `FleetMcpAuthzTest.java`, so the merge conflicted. It was addition against addition, so I kept both sides and closed the two tests properly. The count is the real check on that: main was 2014 after #717, this branch adds 3 from the first round and 1 from the second, and the merged build reports **2018**. If my resolution had swallowed a test it would read 2017. All five tests are present exactly once — I checked each by name. ``` Tests run: 2018, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` ### Three mutations, all mine, each confirmed live before the suite ran | mutation | compile | kills | |---|---|---| | `FleetMcp.java:527` — poll's `callerTerminal(exchange)` → `null` | exit 0, live | 1: `theFleetPollHandlerActuallyThreadsCallerTerminalIntoPoll` | | `FleetApp.java:698` — REST `sendAsync` creator → `null` | exit 0, live | 1: `restSendAsyncRecordsTheCreatingCallersTerminalSoItCanStillPollItsOwnTicket` | | `FleetApp.java:899` — REST poll caller → `null` | exit 0, live | 3, including `restPollRefusesADifferentWorkerButAllowsTheCreatorAndTheUnnamedPrimary` | `FleetMcp.java:527` is the one that matters most. Before this unit, that exact mutation **survived** the full suite — the fix could be switched off and nothing noticed. It now fails. That is the specific thing this unit was asked to close, and it is closed. Every file was restored and confirmed byte-identical, and the merged tree is byte-identical to the tree I built and mutated (`143e08f`). ### On your javadoc caveat You asked whether "a caller with SEND permission" on the new `startOnSharedService` overload reads as a current contract rather than history. It does. It says what the parameter is for, which is what a maintainer of that line needs. It names no ticket, no past behaviour and no reason-for-a-reviewer, so it is on the right side of the rule. ### Documentation, which was mine `wiki/11-Features.md` now carries an entry for this gate, including the two gotchas that are easy to lose: the unnamed primary is never refused, and that is safe **only** because an anonymous caller is denied `TASK_READ` first — nothing in the code pins that coupling. The second gotcha is the split this PR's follow-up fixed, written down so nobody reintroduces half of it. ### One thing I took from your work into a new ticket Your fix leaves `MessageService.poll(String)` with no production caller. It defaults the terminal to `null`, which **skips** the ownership check — it fails open, silently, which is exactly the hole this PR closed. It has 44 test callers, so deleting it is the wrong trade. #718 tracks pinning it with a scrape test. I also found a second instance of the same shape while reviewing: `FleetMcp.java:975` forwards `sendAsync` with `creatorTerminal` defaulted to `null`. Not yours to fix; recorded so it is not rediscovered.

ltms closed this pull request

2026-10-04 07:49:53 +02:00

ltms referenced this pull request

2026-10-04 07:50:16 +02:00

Two convenience overloads have dangerous defaults, and one of them fails open: pin that no production code reaches for MessageService.poll(String) #718

ltms referenced this pull request

2026-10-04 07:50:57 +02:00

Any unconfigured herdr pane resolves to WORKER, and WORKER holds TASK_READ — so an unlisted tab can walk every ticket and read other sessions' delegation replies #705

ltms referenced this pull request

2026-10-04 07:51:33 +02:00

Any unconfigured herdr pane resolves to WORKER, and WORKER holds TASK_READ — so an unlisted tab can walk every ticket and read other sessions' delegation replies #705

ltms referenced this pull request