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

fleetd #778: let an observer read its own ticket, never nudge a refused call #795

Closed
agent wants to merge 0 commits from worker/778-e301b0-1 into main
pull from: worker/778-e301b0-1
merge into: fleet:main
Conversation 2 Commits - Files Changed -
+536 -76
agent commented 2026-10-06 13:56:06 +02:00
Member
Copy Link
Copy Source

Closes #778.

Part 1 — Authz.Action.TASK_READ now grants an observer the ticket it created (a new observerOwnsTicket classifier, backed by a side-effect-free MessageService.ownsTicket(String, String)), while fleet_status and fleet_poll{target} (DRAIN) stay refused for an observer. REST (FleetApp.taskStatus) and MCP (FleetMcp.pollHandler) share the same gate.

Part 2 — ReplyPushLoop no longer nudges a reply/question delegator to run fleet_poll(target=...) or fleet_send(turnId=...) unless Authz actually grants that call to its role. FleetMcp.sendHandler asks Authz.permits(DRAIN)/permits(ANSWER) once, eagerly, while the real Principal is in scope, and threads the two booleans through PrimaryRegistry.recordDelegation into new Delegation fields and nudgeMayDrainFor/nudgeMayAnswerFor accessors. This is deliberate: threading a typed Role/Authz reference from mcp into msg (or adding a new edge inside the already-cyclic auth↔mcp pair) would create a new or widened package cycle, which PackageCyclesTest's exact-edge baseline forbids. Plain booleans (and Optional<Boolean> accessors, both JDK types) cross the existing ReplyPushLoop -> PrimaryRegistry edge with no new ArchUnit edge at all. A reply/question whose grant is false is dropped from the pending set that builds the nudge text — it falls back to the durable inbox (reply) or the ask window lapsing (question), the same as when no nudge target is known at all. Log lines that called a non-lead nudge target "lead" now say "nudge target".

Part 3 — surveyed, not fixed (per brief, report only):

  • LeadHeartbeatLoop's fleet_poll(target=...) nudge text is not vulnerable: its target is always the PrimaryRegistry singleton, which FleetMcp.recordPrimarySingleton only ever sets for caller.isPrimary() — never for an architect/observer/collaborator — so DRAIN is always genuinely available to that recipient.
  • LeadCoordLoop's lead-to-lead mail delivery embeds no gated tool-call instruction, so it is not this shape at all.
  • Open gap, left unfixed (out of scope): ticket nudges (ReplyPushLoop.onTicketTerminal / formatTicketsNudge, fleet_poll(ticket=...)) are not gated by this change. Authz.Action.TASK_READ's full grant is primary || worker || architect || (observer && ownsTicket) — a collaborator is never granted TASK_READ at all, including for a ticket it created. A collaborator may fleet_send(wait:false) to another collaborator (SEND's knownLeadOrCollaborator classifier permits collaborator→collaborator), so a collaborator that delegates this way and then has that ticket go terminal will be nudged to run fleet_poll(ticket=...), which Authz always refuses it. Same shape as the bug this PR fixes; the brief scoped Part 2 to the reply/question nudges only, so this is reported, not changed.
  • The architect+DRAIN reply-nudge mismatch flagged in an earlier pass is resolved as a side effect of Part 2, since Authz.permits(ARCHITECT, DRAIN, ...) is false exactly like for an observer, and the new filtering suppresses it identically.

CLAUDE.md lines that need a lead-side update (not touched by this PR):

  • The observer paragraph under §Bridge communication — OBSERVER's grant list should mention the conditional TASK_READ for a ticket it created.
  • Invariant 3 ("reply/ask/inbox are only-as-itself…") could note that a ticket's TASK_READ is now also conditionally open to its observer-creator.

Build: mvn clean install in fleetd/, unpiped — BUILD SUCCESS, Tests run: 2225, Failures: 0, Errors: 0, Skipped: 0 (includes PackageCyclesTest green, confirming no new/widened cycle). Targeted reruns of the touched classes (AuthzTest, FleetMcpAuthzTest, MessageServiceTest, FleetAppAuthTest, PrimaryRegistryTest, ReplyPushLoopTest) all green beforehand too.

I asked Claude to implement this from the ticket and run the build; I have not reviewed the diff myself yet.

Closes #778. **Part 1** — `Authz.Action.TASK_READ` now grants an observer the ticket it created (a new `observerOwnsTicket` classifier, backed by a side-effect-free `MessageService.ownsTicket(String, String)`), while `fleet_status` and `fleet_poll{target}` (DRAIN) stay refused for an observer. REST (`FleetApp.taskStatus`) and MCP (`FleetMcp.pollHandler`) share the same gate. **Part 2** — `ReplyPushLoop` no longer nudges a reply/question delegator to run `fleet_poll(target=...)` or `fleet_send(turnId=...)` unless `Authz` actually grants that call to its role. `FleetMcp.sendHandler` asks `Authz.permits(DRAIN)`/`permits(ANSWER)` once, eagerly, while the real `Principal` is in scope, and threads the two booleans through `PrimaryRegistry.recordDelegation` into new `Delegation` fields and `nudgeMayDrainFor`/`nudgeMayAnswerFor` accessors. This is deliberate: threading a typed `Role`/`Authz` reference from `mcp` into `msg` (or adding a new edge inside the already-cyclic `auth↔mcp` pair) would create a new or widened package cycle, which `PackageCyclesTest`'s exact-edge baseline forbids. Plain `boolean`s (and `Optional<Boolean>` accessors, both JDK types) cross the existing `ReplyPushLoop -> PrimaryRegistry` edge with no new ArchUnit edge at all. A reply/question whose grant is `false` is dropped from the pending set that builds the nudge text — it falls back to the durable inbox (reply) or the ask window lapsing (question), the same as when no nudge target is known at all. Log lines that called a non-lead nudge target "lead" now say "nudge target". **Part 3 — surveyed, not fixed (per brief, report only):** - `LeadHeartbeatLoop`'s `fleet_poll(target=...)` nudge text is **not** vulnerable: its target is always the `PrimaryRegistry` singleton, which `FleetMcp.recordPrimarySingleton` only ever sets for `caller.isPrimary()` — never for an architect/observer/collaborator — so DRAIN is always genuinely available to that recipient. - `LeadCoordLoop`'s lead-to-lead mail delivery embeds no gated tool-call instruction, so it is not this shape at all. - **Open gap, left unfixed (out of scope):** ticket nudges (`ReplyPushLoop.onTicketTerminal` / `formatTicketsNudge`, `fleet_poll(ticket=...)`) are not gated by this change. `Authz.Action.TASK_READ`'s full grant is `primary || worker || architect || (observer && ownsTicket)` — a **collaborator is never granted TASK_READ at all**, including for a ticket it created. A collaborator may `fleet_send(wait:false)` to another collaborator (`SEND`'s `knownLeadOrCollaborator` classifier permits collaborator→collaborator), so a collaborator that delegates this way and then has that ticket go terminal will be nudged to run `fleet_poll(ticket=...)`, which `Authz` always refuses it. Same shape as the bug this PR fixes; the brief scoped Part 2 to the reply/question nudges only, so this is reported, not changed. - The architect+DRAIN reply-nudge mismatch flagged in an earlier pass is resolved as a side effect of Part 2, since `Authz.permits(ARCHITECT, DRAIN, ...)` is `false` exactly like for an observer, and the new filtering suppresses it identically. **CLAUDE.md lines that need a lead-side update (not touched by this PR):** - The observer paragraph under §Bridge communication — OBSERVER's grant list should mention the conditional `TASK_READ` for a ticket it created. - Invariant 3 ("reply/ask/inbox are only-as-itself…") could note that a ticket's `TASK_READ` is now also conditionally open to its observer-creator. **Build:** `mvn clean install` in `fleetd/`, unpiped — `BUILD SUCCESS`, `Tests run: 2225, Failures: 0, Errors: 0, Skipped: 0` (includes `PackageCyclesTest` green, confirming no new/widened cycle). Targeted reruns of the touched classes (`AuthzTest`, `FleetMcpAuthzTest`, `MessageServiceTest`, `FleetAppAuthTest`, `PrimaryRegistryTest`, `ReplyPushLoopTest`) all green beforehand too. I asked Claude to implement this from the ticket and run the build; I have not reviewed the diff myself yet.
agent added 1 commit 2026-10-06 13:56:06 +02:00
fleetd #778: let an observer read its own ticket, never nudge a role to run a call it is refused
CI / shell-tests (pull_request) Failing after 8s
Details
CI / contract (pull_request) Successful in 59s
Details
CI / build (pull_request) Failing after 2m15s
Details
6022aef612 
Part 1: Authz.TASK_READ now grants an observer the ticket it created (a new
observerOwnsTicket classifier), via MessageService.ownsTicket(String, String)
(side-effect-free) and a new Authz.permits overload. fleet_status and
fleet_poll{target} (DRAIN) stay refused for an observer. REST and MCP share
the same gate.

Part 2: ReplyPushLoop no longer nudges a reply/question delegator to run
fleet_poll(target=...) or fleet_send(turnId=...) unless Authz would actually
grant that call to its role. FleetMcp.sendHandler asks Authz.permits(DRAIN)/
permits(ANSWER) once, eagerly, while the real Principal is in scope, and
threads the booleans through PrimaryRegistry.recordDelegation into the new
Delegation fields and nudgeMayDrainFor/nudgeMayAnswerFor accessors --
plain booleans, not a Role reference, so no new or widened edge crosses the
frozen auth<->mcp<->msg package-cycle baseline. ReplyPushLoop filters a
reply/question out of its pending set when the grant is false, falling back
to the durable inbox / the ask window lapsing, same as when no nudge target
is known at all. Log lines that called a non-lead nudge target "lead" now
say "nudge target".
ltms commented 2026-10-07 05:06:04 +02:00
Owner
Copy Link
Copy Source

Merged locally as 52cd047 and closing here — Gitea does not see a local merge.

Verified myself, not on the PR's word. Built the branch head 6022aef in a throwaway worktree: [INFO] BUILD SUCCESS, and Tests run: 2225, Failures: 0, Errors: 0, Skipped: 0. I confirmed that total two ways — maven's own line, and my own tally of the 183 surefire-reports/*.xml files, which agreed exactly.

The build of the merge result on main is still running as I write this; I will add its verdict in a follow-up comment. A clean auto-merge is not a compiling merge, so that check is not done until I post the number.

I read all 225 lines of changed main source: Authz, Role, FleetMcp, PrimaryRegistry, MessageService, ReplyPushLoop, FleetApp. The gate is fail-closed by default — NO_OBSERVER_OWNED_TICKET for every caller that cannot supply a real classifier, and orElse(false) on both nudge grants — and ownsTicket is side-effect-free, so consulting it never consumes a ticket.

One real defect, filed as #803: fleet_poll{} with no ticket resolves to TASK_READ with a null ticket, and the new predicate reaches ConcurrentHashMap.get(null), which throws. Only an observer can reach it, and it is not live yet. The fix is one line in MessageService.ownsTicket, plus the null case in the test. The deploy is held until that lands.

A note on reading this diff: git diff main 6022aef is misleading here, because the branch predates four commits on main. It shows 19 files and reports my own LeadRollover/CLAUDE.md/skill changes as deletions. The PR's own diff is git diff $(git merge-base main 6022aef) 6022aef — 13 files, 536 insertions, 76 deletions, all Java.

I asked Claude to verify this branch and review the diff; the build totals and the finding above are from this session.

Edited: the first version of this comment said the merge build had already passed. It had not — it was still running. Corrected above.

Merged locally as `52cd047` and closing here — Gitea does not see a local merge. **Verified myself, not on the PR's word.** Built the branch head `6022aef` in a throwaway worktree: `[INFO] BUILD SUCCESS`, and `Tests run: 2225, Failures: 0, Errors: 0, Skipped: 0`. I confirmed that total two ways — maven's own line, and my own tally of the 183 `surefire-reports/*.xml` files, which agreed exactly. The build of the **merge result** on `main` is still running as I write this; I will add its verdict in a follow-up comment. A clean auto-merge is not a compiling merge, so that check is not done until I post the number. I read all 225 lines of changed main source: `Authz`, `Role`, `FleetMcp`, `PrimaryRegistry`, `MessageService`, `ReplyPushLoop`, `FleetApp`. The gate is fail-closed by default — `NO_OBSERVER_OWNED_TICKET` for every caller that cannot supply a real classifier, and `orElse(false)` on both nudge grants — and `ownsTicket` is side-effect-free, so consulting it never consumes a ticket. One real defect, filed as #803: `fleet_poll{}` with no ticket resolves to `TASK_READ` with a null ticket, and the new predicate reaches `ConcurrentHashMap.get(null)`, which throws. Only an observer can reach it, and it is not live yet. The fix is one line in `MessageService.ownsTicket`, plus the null case in the test. The deploy is held until that lands. A note on reading this diff: `git diff main 6022aef` is misleading here, because the branch predates four commits on `main`. It shows 19 files and reports my own `LeadRollover`/`CLAUDE.md`/skill changes as deletions. The PR's own diff is `git diff $(git merge-base main 6022aef) 6022aef` — 13 files, 536 insertions, 76 deletions, all Java. --- I asked Claude to verify this branch and review the diff; the build totals and the finding above are from this session. *Edited: the first version of this comment said the merge build had already passed. It had not — it was still running. Corrected above.*
ltms closed this pull request 2026-10-07 05:06:31 +02:00
ltms commented 2026-10-07 05:07:24 +02:00
Owner
Copy Link
Copy Source

Merge-result build, as promised above. Built 52cd047 in a second throwaway worktree:

[INFO] BUILD SUCCESS
[INFO] Tests run: 2227, Failures: 0, Errors: 0, Skipped: 0
EXIT=0 at 05:06:43

Tallied independently from the 183 surefire-reports/*.xml files: tests=2227 failures=0 errors=0 skipped=0, which agrees with maven's line.

2227 and not 2225 because main carries two LeadRollover tests from #796 that this branch predates. The merge added no failure.

Also filed #804 from this PR's own Part 3 survey: a collaborator is granted no TASK_READ at all, yet it may create a ticket by sending to another collaborator, so that reply is unreachable. Same shape as the bug this PR fixed. Keeping it in an issue rather than only in a closed PR body.

I asked Claude to run and tally this build; the numbers are from this session.

Merge-result build, as promised above. Built `52cd047` in a second throwaway worktree: ``` [INFO] BUILD SUCCESS [INFO] Tests run: 2227, Failures: 0, Errors: 0, Skipped: 0 EXIT=0 at 05:06:43 ``` Tallied independently from the 183 `surefire-reports/*.xml` files: `tests=2227 failures=0 errors=0 skipped=0`, which agrees with maven's line. 2227 and not 2225 because `main` carries two `LeadRollover` tests from #796 that this branch predates. The merge added no failure. Also filed #804 from this PR's own Part 3 survey: a collaborator is granted no `TASK_READ` at all, yet it may create a ticket by sending to another collaborator, so that reply is unreachable. Same shape as the bug this PR fixed. Keeping it in an issue rather than only in a closed PR body. --- I asked Claude to run and tally this build; the numbers are from this session.
Some checks are pending
CI / shell-tests (pull_request) Failing after 8s
Details
CI / contract (pull_request) Successful in 59s
Details
CI / build (pull_request) Failing after 2m15s
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#795
Delete Branch "worker/778-e301b0-1"

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?