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

fleetd #729: fold a per-boot nonce into every turnId #732

Closed
agent wants to merge 0 commits from worker/729-5961c6-3 into main
pull from: worker/729-5961c6-3
merge into: fleet:main
Conversation 1 Commits - Files Changed -
agent commented 2026-10-04 18:53:48 +02:00
Member
Copy Link
Copy Source

Mirrors #719's fix for ticket ids.

askSeq (Rendezvous.java:104) is a per-instance AtomicLong that restarts at 0 on every daemon boot. A turnId minted at openAsk (line ~199) was session + "#" + askSeq.incrementAndGet() - if a member terminal id can ever recur in a later boot and reach the same sequence number, answerAsk would resolve an unrelated waiter with no error.

Fix: added askBootNonce (a UUID-derived per-instance nonce, same shape as MessageService.ticketBootNonce) and folded it into the mint: session + "#" + askBootNonce + "-" + askSeq.incrementAndGet().

Tests added to RendezvousTest:

  • twoInstancesMintDisjointTurnIds
  • foreignInstanceTurnIdDoesNotResolve (with a positive control that the minting instance still resolves its own turnId, and other driven to the same sequence number so the test is not vacuous)
  • openAskStillCoalescesDuplicatesAndStillMintsDistinctIdsPerAsk (existing coalescing behavior unchanged)

Mutation evidence (reported in full on the ticket):

  • Removing the nonce from the mint turns the two cross-instance tests RED (AssertionFailedError on both).
  • Restoring it turns the full RendezvousTest class GREEN (20/20).
  • Separately, deleting just the line that drives other to the same sequence number, with the nonce still removed, makes foreignInstanceTurnIdDoesNotResolve falsely PASS (1/1) - confirming that line is load-bearing and the test is not vacuous.

mvn clean install: BUILD SUCCESS, 2057 tests run, 0 failures, 0 errors.

Scope is exactly the two files named in the ticket: Rendezvous.java and RendezvousTest.java. MessageService.java was read only as the pattern reference and is untouched.

Mirrors #719's fix for ticket ids. `askSeq` (Rendezvous.java:104) is a per-instance `AtomicLong` that restarts at 0 on every daemon boot. A `turnId` minted at `openAsk` (line ~199) was `session + "#" + askSeq.incrementAndGet()` - if a member terminal id can ever recur in a later boot and reach the same sequence number, `answerAsk` would resolve an unrelated waiter with no error. Fix: added `askBootNonce` (a `UUID`-derived per-instance nonce, same shape as `MessageService.ticketBootNonce`) and folded it into the mint: `session + "#" + askBootNonce + "-" + askSeq.incrementAndGet()`. Tests added to `RendezvousTest`: - `twoInstancesMintDisjointTurnIds` - `foreignInstanceTurnIdDoesNotResolve` (with a positive control that the minting instance still resolves its own turnId, and `other` driven to the same sequence number so the test is not vacuous) - `openAskStillCoalescesDuplicatesAndStillMintsDistinctIdsPerAsk` (existing coalescing behavior unchanged) Mutation evidence (reported in full on the ticket): - Removing the nonce from the mint turns the two cross-instance tests RED (AssertionFailedError on both). - Restoring it turns the full `RendezvousTest` class GREEN (20/20). - Separately, deleting just the line that drives `other` to the same sequence number, with the nonce still removed, makes `foreignInstanceTurnIdDoesNotResolve` falsely PASS (1/1) - confirming that line is load-bearing and the test is not vacuous. `mvn clean install`: BUILD SUCCESS, 2057 tests run, 0 failures, 0 errors. Scope is exactly the two files named in the ticket: `Rendezvous.java` and `RendezvousTest.java`. `MessageService.java` was read only as the pattern reference and is untouched.
agent added 1 commit 2026-10-04 18:53:48 +02:00
fleetd #729: fold a per-boot nonce into every turnId
CI / shell-tests (pull_request) Failing after 6s
Details
CI / contract (pull_request) Successful in 54s
Details
CI / build (pull_request) Failing after 1m57s
Details
1fc9e85bf1 
askSeq restarts at 0 on every daemon boot, so a turnId (session#n)
minted by one Rendezvous instance could be minted again by a later
instance and resolve to an unrelated ask. Fold a per-instance nonce
into the mint, the same way #719 fixed MessageService's ticket ids.
ltms commented 2026-10-04 19:00:32 +02:00
Owner
Copy Link
Copy Source

Merged locally as 8bb2aa0 — closing this PR

Merged into main and pushed. Verified by ref, not by the push output:

local main : 8bb2aa0be45ffe543f393ef7caf1e21a4bfcb9b9
origin/main: 8bb2aa0be45ffe543f393ef7caf1e21a4bfcb9b9

We merge locally, so Gitea does not close the PR by itself. Closing it by hand.

What I checked myself, rather than taking the report

The build. Throwaway worktree on origin/worker/729-5961c6-3, tree hash confirmed equal to the
branch's, mvn clean install unpiped: BUILD SUCCESS, Tests run: 2057, Failures: 0, Errors: 0,
RendezvousTest 20 green. Matches the report exactly.

No rebuild after the merge, and the reason is a measurement rather than an assumption. The merged
tree hash is a2933ef6cc6f764eeb72c85f214bb69335de2d94, byte-identical to the branch tree I had just
built. So the 2057-green run is this merge result, not a proxy for it.

That nothing parses a turnId. This is the risk the format change actually carried, and the
report only covered the prefix half. I grepped every turnId use in src/main/java for
split|substring|indexOf|parse|lastIndexOf|replace|matches|charAt: the three hits are prose inside
comments, not code. Six files touch turnId at all (Rendezvous, ReplyPushLoop, MessageService,
Authz, FleetMcp, FleetApp), and in Authz it appears only in a javadoc line. It is used as an
opaque key everywhere, so session#<nonce>-<seq> is safe.

That the test is not vacuous. foreignInstanceTurnIdDoesNotResolve drives other through
open → close → open so it reaches the same sequence number, and carries the positive control that the
minting instance still resolves its own id. Both halves are what #719's first version was missing.
twoInstancesMintDisjointTurnIds is the simplest possible check and dies on the nonce's removal,
because without it both instances mint W#1.

The criterion-3 reading is the one that mattered: with the nonce removed AND the sequence-driving line
deleted, the test falsely passed. That is the proof the line is load-bearing, and it is the test I
most wanted to see.

The sweep is being kept, not dropped

The three same-shape identifiers you found — BackendOutagePolicy.java:107,
UnixSocketHerdrClient.java:69, HerdrPeerLauncher.java:558 — are filed as their own ticket rather
than lost in this PR. Your reachability caveats are carried over verbatim: you did not establish that
any of them can collide, and that is the right thing to have said rather than guessed. Your four
ruled-out cases were checked and the reasoning is correct in each.

## Merged locally as `8bb2aa0` — closing this PR Merged into `main` and pushed. Verified by ref, not by the push output: ``` local main : 8bb2aa0be45ffe543f393ef7caf1e21a4bfcb9b9 origin/main: 8bb2aa0be45ffe543f393ef7caf1e21a4bfcb9b9 ``` We merge locally, so Gitea does not close the PR by itself. Closing it by hand. ### What I checked myself, rather than taking the report **The build.** Throwaway worktree on `origin/worker/729-5961c6-3`, tree hash confirmed equal to the branch's, `mvn clean install` unpiped: `BUILD SUCCESS`, `Tests run: 2057, Failures: 0, Errors: 0`, `RendezvousTest` 20 green. Matches the report exactly. **No rebuild after the merge, and the reason is a measurement rather than an assumption.** The merged tree hash is `a2933ef6cc6f764eeb72c85f214bb69335de2d94`, byte-identical to the branch tree I had just built. So the 2057-green run *is* this merge result, not a proxy for it. **That nothing parses a `turnId`.** This is the risk the format change actually carried, and the report only covered the prefix half. I grepped every `turnId` use in `src/main/java` for `split|substring|indexOf|parse|lastIndexOf|replace|matches|charAt`: the three hits are prose inside comments, not code. Six files touch `turnId` at all (`Rendezvous`, `ReplyPushLoop`, `MessageService`, `Authz`, `FleetMcp`, `FleetApp`), and in `Authz` it appears only in a javadoc line. It is used as an opaque key everywhere, so `session#<nonce>-<seq>` is safe. **That the test is not vacuous.** `foreignInstanceTurnIdDoesNotResolve` drives `other` through open → close → open so it reaches the same sequence number, and carries the positive control that the minting instance still resolves its own id. Both halves are what #719's first version was missing. `twoInstancesMintDisjointTurnIds` is the simplest possible check and dies on the nonce's removal, because without it both instances mint `W#1`. The criterion-3 reading is the one that mattered: with the nonce removed AND the sequence-driving line deleted, the test falsely **passed**. That is the proof the line is load-bearing, and it is the test I most wanted to see. ### The sweep is being kept, not dropped The three same-shape identifiers you found — `BackendOutagePolicy.java:107`, `UnixSocketHerdrClient.java:69`, `HerdrPeerLauncher.java:558` — are filed as their own ticket rather than lost in this PR. Your reachability caveats are carried over verbatim: you did not establish that any of them can collide, and that is the right thing to have said rather than guessed. Your four ruled-out cases were checked and the reasoning is correct in each.
ltms closed this pull request 2026-10-04 19:00:35 +02:00
Some checks are pending
CI / shell-tests (pull_request) Failing after 6s
Details
CI / contract (pull_request) Successful in 54s
Details
CI / build (pull_request) Failing after 1m57s
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#732
Delete Branch "worker/729-5961c6-3"

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?