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

fleetd #737 units 1+2: key tickets and turns on a stable owner, not a terminal #741

Closed
agent wants to merge 0 commits from worker/737-owner-key-ff061f-10 into main
pull from: worker/737-owner-key-ff061f-10
merge into: fleet:main
Conversation 2 Commits - Files Changed -
agent commented 2026-10-04 20:43:09 +02:00
Member
Copy Link
Copy Source

Finishes units 1+2 of #737, picked up from a member that died mid-task on a shared-credential usage limit (see ticket comment titled "Units 1+2: the member died on a usage limit"). The uncommitted work in that comment is unchanged in behaviour; this PR adds the required mutation evidence and commits/pushes it.

What changed

  • Principal.ownerKey(): a role-prefixed, stable identity used to own tickets and open turns. A named lead and a collaborator key on their name (survives a lead handover's terminal change); a worker, architect and observer key on their terminal; ANONYMOUS gets a distinct "anonymous" value instead of sharing null with the unnamed primary, which no longer depends on Authz refusing anonymous callers first to stay safe. The unnamed primary keeps a null key, preserving the message layer's primary-wide ticket rule.
  • Task.creatorOwner, Rendezvous.Owner, MessageService.poll/pendingAsk/answer, and their MCP (FleetMcp) and REST (FleetApp) call sites now carry this owner key instead of a raw terminal.
  • Acceptance: a named lead's old terminal creates a ticket/ask, its new terminal (after a simulated handover) can poll and answer it, and a different lead is refused both — see namedLeadCanPollItsTicketAfterItsTerminalChanges and namedLeadCanSeeAndAnswerAnAskAfterItsTerminalChangesWhileLeadBIsRefused in MessageServiceTest.

Mutation evidence

Five one-line mutations against Principal.ownerKey(), each run, confirmed RED with its failure line, then reverted to GREEN:

  1. PRIMARY case made unconditional (prefixed("leader", name) dropping the null-name guard) -> ownerKeyCoversEveryRole dies: expected: <null> but was: <leader:null>
  2. OBSERVER case changed to the "worker" prefix -> ownerKeyCoversEveryRole dies: expected: <observer:term_observer> but was: <worker:term_observer>
  3. prefixed() changed to drop the role prefix entirely (bare identity) -> ownerKeyCoversEveryRole and rolePrefixesKeepLeadAndArchitectKeysDistinct both die on a lead/architect collision: expected: <leader:opus> but was: <opus>
  4. PRIMARY case changed to key on terminal instead of name -> both tests die: expected: <leader:opus> but was: <leader:term_lead>
  5. ARCHITECT case changed to key on the slot name instead of terminal -> both tests die: expected: <architect:opus> but was: <architect:design>

All five were caught by the existing/inherited test suite; no test needed adding.

Build

mvn clean install (surefire-reports cleared first): BUILD SUCCESS, Tests run: 2094, Failures: 0, Errors: 0, Skipped: 0 across 177 classes.

Scope note

Per the ticket, units 3 (routing), 4 (rollover) and 5 (warning) are explicitly out of scope here and not yet briefed. A lead review already on the ticket (comment 19002) flagged that the Principal-typed guard against passing a raw terminal is applied only to sendAsync, not to send/answer/poll/pendingAsk (all five still take a bare String callerOwner) — every misuse direction fails closed, so it is not a blocker, and the lead explicitly asked this unit not to change behaviour, so it is left as a follow-up for a later unit.

Finishes units 1+2 of #737, picked up from a member that died mid-task on a shared-credential usage limit (see ticket comment titled "Units 1+2: the member died on a usage limit"). The uncommitted work in that comment is unchanged in behaviour; this PR adds the required mutation evidence and commits/pushes it. ## What changed - `Principal.ownerKey()`: a role-prefixed, stable identity used to own tickets and open turns. A named lead and a collaborator key on their *name* (survives a lead handover's terminal change); a worker, architect and observer key on their *terminal*; `ANONYMOUS` gets a distinct `"anonymous"` value instead of sharing `null` with the unnamed primary, which no longer depends on `Authz` refusing anonymous callers first to stay safe. The unnamed primary keeps a `null` key, preserving the message layer's primary-wide ticket rule. - `Task.creatorOwner`, `Rendezvous.Owner`, `MessageService.poll`/`pendingAsk`/`answer`, and their MCP (`FleetMcp`) and REST (`FleetApp`) call sites now carry this owner key instead of a raw terminal. - Acceptance: a named lead's old terminal creates a ticket/ask, its *new* terminal (after a simulated handover) can poll and answer it, and a different lead is refused both — see `namedLeadCanPollItsTicketAfterItsTerminalChanges` and `namedLeadCanSeeAndAnswerAnAskAfterItsTerminalChangesWhileLeadBIsRefused` in `MessageServiceTest`. ## Mutation evidence Five one-line mutations against `Principal.ownerKey()`, each run, confirmed RED with its failure line, then reverted to GREEN: 1. PRIMARY case made unconditional (`prefixed("leader", name)` dropping the null-name guard) -> `ownerKeyCoversEveryRole` dies: `expected: <null> but was: <leader:null>` 2. OBSERVER case changed to the `"worker"` prefix -> `ownerKeyCoversEveryRole` dies: `expected: <observer:term_observer> but was: <worker:term_observer>` 3. `prefixed()` changed to drop the role prefix entirely (bare identity) -> `ownerKeyCoversEveryRole` and `rolePrefixesKeepLeadAndArchitectKeysDistinct` both die on a lead/architect collision: `expected: <leader:opus> but was: <opus>` 4. PRIMARY case changed to key on `terminal` instead of `name` -> both tests die: `expected: <leader:opus> but was: <leader:term_lead>` 5. ARCHITECT case changed to key on the slot name instead of `terminal` -> both tests die: `expected: <architect:opus> but was: <architect:design>` All five were caught by the existing/inherited test suite; no test needed adding. ## Build `mvn clean install` (surefire-reports cleared first): **BUILD SUCCESS**, `Tests run: 2094, Failures: 0, Errors: 0, Skipped: 0` across 177 classes. ## Scope note Per the ticket, units 3 (routing), 4 (rollover) and 5 (warning) are explicitly out of scope here and not yet briefed. A lead review already on the ticket (comment 19002) flagged that the `Principal`-typed guard against passing a raw terminal is applied only to `sendAsync`, not to `send`/`answer`/`poll`/`pendingAsk` (all five still take a bare `String callerOwner`) — every misuse direction fails closed, so it is not a blocker, and the lead explicitly asked this unit not to change behaviour, so it is left as a follow-up for a later unit.
agent added 1 commit 2026-10-04 20:43:09 +02:00
fleetd #737 units 1+2: key tickets and turns on a stable owner, not a terminal
CI / shell-tests (pull_request) Failing after 9s
Details
CI / contract (pull_request) Successful in 57s
Details
CI / build (pull_request) Failing after 2m4s
Details
efd9cdb983 
Add Principal.ownerKey(): role-prefixed, keyed on name for a named lead and
a collaborator (survives a handover's terminal change), on terminal for a
worker, architect and observer, and on a distinct "anonymous" value for an
unauthenticated caller so that case no longer relies on Authz refusing it
first. The unnamed primary keeps a null key, preserving its primary-wide
ticket rule.

Thread that key through Task.creatorOwner, Rendezvous.Owner, poll,
pendingAsk and answer in place of a raw terminal, in both the MCP and REST
surfaces, so a named lead whose terminal changes can still poll and answer
its own delegations while a different lead is refused both.

Mutation evidence (each one-line change killed a named test, then reverted
to green):
- Principal.ownerKey() PRIMARY case made unconditional (dropped the
  null-name guard) -> ownerKeyCoversEveryRole dies:
  "expected: <null> but was: <leader:null>"
- OBSERVER case changed to use the "worker" prefix -> ownerKeyCoversEveryRole
  dies: "expected: <observer:term_observer> but was: <worker:term_observer>"
- prefixed() changed to drop the role prefix entirely -> both
  ownerKeyCoversEveryRole and rolePrefixesKeepLeadAndArchitectKeysDistinct
  die on a lead/architect key collision: "expected: <leader:opus> but was:
  <opus>"
- PRIMARY case changed to key on terminal instead of name ->
  rolePrefixesKeepLeadAndArchitectKeysDistinct and ownerKeyCoversEveryRole
  die: "expected: <leader:opus> but was: <leader:term_lead>"
- ARCHITECT case changed to key on the slot name instead of terminal ->
  same two tests die: "expected: <architect:opus> but was: <architect:design>"

All five mutations were caught by the existing test suite; no test needed
adding.
ltms commented 2026-10-04 21:11:54 +02:00
Owner
Copy Link
Copy Source

Merged locally as cd1f04c. Closing this PR.

main moved aabecce..cd1f04c, verified by ref:

$ git ls-remote origin refs/heads/main
cd1f04cbb40bf2c7f77768cec09edae8aceee400	refs/heads/main

The merge is a --no-ff commit whose tree is byte-identical to this branch's tip efd9cdb, which I had already built green (2094 tests, 0 failures). I checked that rather than assumed it:

$ [ "$(git rev-parse HEAD^{tree})" = "$(git rev-parse origin/worker/737-owner-key-ff061f-10^{tree})" ]
YES - same tree I built green

A confirming mvn clean install on the merged main is running now; I will report its real count and result, pass or fail.

What I checked before merging

  1. I read all five main-source files myself — Principal, MessageService, Rendezvous, FleetMcp, FleetApp.
  2. I reproduced 2 of the 5 reported mutations independently, with character-identical failure lines and clean reverts.
  3. One reviewer finding, adjudicated — the null owner key wildcard. Not introduced by this PR, same rule on main, one of its claims factually wrong. Full working on #737, and it is now #737 unit 6.
  4. A hunter swept the six changed test files for weakening. It found none, and proved reachability by mutating ownsTicket to return true (6 refusal tests died) and Rendezvous.Owner.permits to return true (the direct unit test plus every hijack test died).
  5. I checked the no-weakening claim myself rather than taking the hunter's word, by counting assertions and test methods on both sides:
file assert main → branch @Test main → branch
PrincipalTest 0 → 13 (new) 0 → 2
FleetMcpAuthzTest 139 → 139 34 → 34
FleetMcpTest 398 → 398 107 → 107
MessageServicePollUsageTest 11 → 11 2 → 2
MessageServiceTest 407 → 422 99 → 102
RendezvousTest 67 → 67 20 → 20
FleetAppAuthTest 115 → 115 25 → 25

Nothing decreased anywhere. A count is a weak instrument on its own — it cannot see an assertion that was kept but made weaker — which is why the hunter's mutation runs are the real evidence and this table is only a cross-check on them.

One honest note the hunter raised, which I agree is not a finding

Some tests hardcode the key format as a literal, such as "worker:term_attacker", instead of building it with Principal.worker(...).ownerKey(). That is correct today — I checked the literals against the real formula — and it loses no test power. It does duplicate the role:identity convention into test code, so a later change to the prefix format could pass those tests while breaking production. Worth tidying when someone is next in these files; not worth holding a verified diff for.

## Merged locally as `cd1f04c`. Closing this PR. `main` moved `aabecce..cd1f04c`, verified by ref: ``` $ git ls-remote origin refs/heads/main cd1f04cbb40bf2c7f77768cec09edae8aceee400 refs/heads/main ``` The merge is a `--no-ff` commit whose tree is byte-identical to this branch's tip `efd9cdb`, which I had already built green (2094 tests, 0 failures). I checked that rather than assumed it: ``` $ [ "$(git rev-parse HEAD^{tree})" = "$(git rev-parse origin/worker/737-owner-key-ff061f-10^{tree})" ] YES - same tree I built green ``` A confirming `mvn clean install` on the merged `main` is running now; I will report its real count and result, pass or fail. ### What I checked before merging 1. **I read all five main-source files myself** — `Principal`, `MessageService`, `Rendezvous`, `FleetMcp`, `FleetApp`. 2. **I reproduced 2 of the 5 reported mutations independently**, with character-identical failure lines and clean reverts. 3. **One reviewer finding, adjudicated** — the `null` owner key wildcard. Not introduced by this PR, same rule on `main`, one of its claims factually wrong. Full working on #737, and it is now #737 unit 6. 4. **A hunter swept the six changed test files for weakening.** It found none, and proved reachability by mutating `ownsTicket` to `return true` (6 refusal tests died) and `Rendezvous.Owner.permits` to `return true` (the direct unit test plus every hijack test died). 5. **I checked the no-weakening claim myself** rather than taking the hunter's word, by counting assertions and test methods on both sides: | file | `assert` main → branch | `@Test` main → branch | |---|---|---| | `PrincipalTest` | 0 → 13 (new) | 0 → 2 | | `FleetMcpAuthzTest` | 139 → 139 | 34 → 34 | | `FleetMcpTest` | 398 → 398 | 107 → 107 | | `MessageServicePollUsageTest` | 11 → 11 | 2 → 2 | | `MessageServiceTest` | 407 → 422 | 99 → 102 | | `RendezvousTest` | 67 → 67 | 20 → 20 | | `FleetAppAuthTest` | 115 → 115 | 25 → 25 | Nothing decreased anywhere. A count is a weak instrument on its own — it cannot see an assertion that was kept but made weaker — which is why the hunter's mutation runs are the real evidence and this table is only a cross-check on them. ### One honest note the hunter raised, which I agree is not a finding Some tests hardcode the key format as a literal, such as `"worker:term_attacker"`, instead of building it with `Principal.worker(...).ownerKey()`. That is correct today — I checked the literals against the real formula — and it loses no test power. It does duplicate the `role:identity` convention into test code, so a later change to the prefix format could pass those tests while breaking production. Worth tidying when someone is next in these files; not worth holding a verified diff for.
ltms closed this pull request 2026-10-04 21:11:58 +02:00
ltms commented 2026-10-04 21:12:49 +02:00
Owner
Copy Link
Copy Source

Confirming build on the merged main at cd1f04c, run unpiped with the exit code captured and fleetd/target/surefire-reports cleared first:

[INFO] BUILD SUCCESS
MAVEN_EXIT_CODE=0
[INFO] Tests run: 2094, Failures: 0, Errors: 0, Skipped: 0

177 test classes, counted from the log's Running dev.ltms lines. Same count as the pre-merge build of the identical tree, which is what I expected given the trees match.

Confirming build on the merged `main` at `cd1f04c`, run unpiped with the exit code captured and `fleetd/target/surefire-reports` cleared first: ``` [INFO] BUILD SUCCESS MAVEN_EXIT_CODE=0 [INFO] Tests run: 2094, Failures: 0, Errors: 0, Skipped: 0 ``` 177 test classes, counted from the log's `Running dev.ltms` lines. Same count as the pre-merge build of the identical tree, which is what I expected given the trees match.
Some checks are pending
CI / shell-tests (pull_request) Failing after 9s
Details
CI / contract (pull_request) Successful in 57s
Details
CI / build (pull_request) Failing after 2m4s
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#741
Delete Branch "worker/737-owner-key-ff061f-10"

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?