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

fleetd #669 follow-up: deliverableTo also opens for a collaborator #706

Closed
agent wants to merge 0 commits from worker/669-collab-deliverability-9ba859-3 into main
pull from: worker/669-collab-deliverability-9ba859-3
merge into: fleet:main
Conversation 1 Commits - Files Changed -
agent commented 2026-10-04 06:14:02 +02:00
Member
Copy Link
Copy Source

A configured collaborator's terminal was authorized at Authz but never deliverable: it is never enrolled in MemberPresence (only spawned members mark presence) and never discovered by the lead scan (LeadTabScanner.get() is LEAD-only), so a send to a collaborator sat on the Injector's readiness gate for ~60s and failed, never typed into the pane (NOT_DELIVERED). Today: collaborator to lead works (a lead is in leads); lead to collaborator and collaborator to collaborator both failed.

fleetd #669 Unit E already fixed the equivalent herdr-router predicate at FleetdAssembly.java:148-151; this fixes the separate injector readiness gate at Fleetd.deliverableTo, which Unit E did not touch.

Change: deliverableTo(presence, leads, collaborators) now takes a third Supplier<Map<String,String>> collaborators disjunct, read through on each call (not snapshotted), matching the existing lead-supplier pattern and its javadoc guarantee. FleetdAssembly wires the existing collaboratorTerminals supplier (already built at :262/:290/:296, already used by the herdr router) into the call at :371.

Scope: Fleetd.java (deliverableTo), FleetdAssembly.java (:371 call site), FleetDeliverabilityTest.java (new coverage). Did not touch Authz, Principal, CallerResolver, or the herdr router at FleetdAssembly.java:148-151, per the ticket's explicit scope.

Tests: mvn clean install -- Tests run: 1995, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS. FleetDeliverabilityTest: 9 tests (3 new: collaboratorIsDeliverableWithoutPresenceOrLeadStatus, collaboratorSetIsReadThroughOnEveryCall, forgetDoesNotDisarmACollaborator), all green.

Revert-proof: reverted just the behavioral change (kept the 3-arg signature so the test file still compiles, dropped the collaborators-check disjunct) and reran FleetDeliverabilityTest: Tests run: 9, Failures: 3 -- exactly the 3 new collaborator tests went red, the 6 pre-existing tests (presence/lead behavior) stayed green. Restored the fix and reran the full suite green again.

Ref: fleetd #669 follow-up.

A configured collaborator's terminal was authorized at Authz but never deliverable: it is never enrolled in MemberPresence (only spawned members mark presence) and never discovered by the lead scan (LeadTabScanner.get() is LEAD-only), so a send to a collaborator sat on the Injector's readiness gate for ~60s and failed, never typed into the pane (NOT_DELIVERED). Today: collaborator to lead works (a lead is in leads); lead to collaborator and collaborator to collaborator both failed. fleetd #669 Unit E already fixed the equivalent herdr-router predicate at FleetdAssembly.java:148-151; this fixes the separate injector readiness gate at Fleetd.deliverableTo, which Unit E did not touch. Change: deliverableTo(presence, leads, collaborators) now takes a third Supplier<Map<String,String>> collaborators disjunct, read through on each call (not snapshotted), matching the existing lead-supplier pattern and its javadoc guarantee. FleetdAssembly wires the existing collaboratorTerminals supplier (already built at :262/:290/:296, already used by the herdr router) into the call at :371. Scope: Fleetd.java (deliverableTo), FleetdAssembly.java (:371 call site), FleetDeliverabilityTest.java (new coverage). Did not touch Authz, Principal, CallerResolver, or the herdr router at FleetdAssembly.java:148-151, per the ticket's explicit scope. Tests: mvn clean install -- Tests run: 1995, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS. FleetDeliverabilityTest: 9 tests (3 new: collaboratorIsDeliverableWithoutPresenceOrLeadStatus, collaboratorSetIsReadThroughOnEveryCall, forgetDoesNotDisarmACollaborator), all green. Revert-proof: reverted just the behavioral change (kept the 3-arg signature so the test file still compiles, dropped the collaborators-check disjunct) and reran FleetDeliverabilityTest: Tests run: 9, Failures: 3 -- exactly the 3 new collaborator tests went red, the 6 pre-existing tests (presence/lead behavior) stayed green. Restored the fix and reran the full suite green again. Ref: fleetd #669 follow-up.
agent added 1 commit 2026-10-04 06:14:03 +02:00
fleetd #669 follow-up: deliverableTo also opens for a collaborator
CI / shell-tests (pull_request) Failing after 11s
Details
CI / contract (pull_request) Successful in 59s
Details
CI / build (pull_request) Failing after 1m48s
Details
20fc42b572 
A collaborator's terminal was authorized at Authz but never deliverable: it
is never enrolled in MemberPresence and never discovered by the lead scan,
so a send to a collaborator sat on the injector's readiness gate for
~60s and failed, never typed into the pane. deliverableTo now takes a
third collaborators supplier, read through on each call like the lead
supplier, and FleetdAssembly wires the existing collaboratorTerminals
supplier into it.
ltms commented 2026-10-04 06:20:17 +02:00
Owner
Copy Link
Copy Source

Merged locally in 3e8e314, together with #707. Closing by hand, because a local merge never
closes the PR here.

Verified by the lead before merging. mvn clean install on the merged tree, output to a file and
not piped: exit code 0, BUILD SUCCESS, Tests run: 1999, Failures: 0, Errors: 0, and
FleetDeliverabilityTest: Tests run: 9.

The one thing that decides whether this fix is live is the key shape of the two maps. If
collaborators() were keyed by collaborator name, containsKey(target) would never fire and the
fix would be dead in the same way the defect was. I checked: LeadTabScanner.get() is documented
as terminal_id → lead name and collaborators() as terminal_id → collaborator name, both built
by byKind from one scan. So the third disjunct is keyed the same way as the leads one and really
fires.

The tests are sound rather than merely green. Each new collaborator test passes an empty leads map
and an empty MemberPresence, so only the third disjunct can make it true, and
strangerIsNotDeliverable now carries a non-empty collaborator map as the control, so the
disjunct cannot be a blanket true.

Two notes, neither blocking.

The revert proof is the right experiment: keeping the 3-argument signature and dropping only
|| collaborators.get().containsKey(target) turned exactly the 3 new tests red and left the 6
pre-existing ones green. I did not re-run it myself.

On provenance: the worker reported that it launched a fork subagent for the read-only
extra-scope search, and that the fork ran the whole commit, push and PR-open sequence itself. The
worker flagged this unprompted, which is the honest thing to do. The content is not in question —
I verified the diff and the build independently, and the merge decision was mine. But a member
must not delegate its own commit and push, because then nobody with the context reviewed what was
pushed. I am treating this as a process defect to fix in the implementer skill, not a reason to
reject the code.

The extra-scope sweep found no other instance of the authorized-but-undeliverable shape. That
matches my own reading of Authz, where every case already carries an explicit note on whether
COLLABORATOR is included.

Merged locally in `3e8e314`, together with #707. Closing by hand, because a local merge never closes the PR here. Verified by the lead before merging. `mvn clean install` on the merged tree, output to a file and not piped: exit code 0, `BUILD SUCCESS`, `Tests run: 1999, Failures: 0, Errors: 0`, and `FleetDeliverabilityTest: Tests run: 9`. The one thing that decides whether this fix is live is the key shape of the two maps. If `collaborators()` were keyed by collaborator *name*, `containsKey(target)` would never fire and the fix would be dead in the same way the defect was. I checked: `LeadTabScanner.get()` is documented as `terminal_id → lead name` and `collaborators()` as `terminal_id → collaborator name`, both built by `byKind` from one scan. So the third disjunct is keyed the same way as the leads one and really fires. The tests are sound rather than merely green. Each new collaborator test passes an empty leads map and an empty `MemberPresence`, so only the third disjunct can make it true, and `strangerIsNotDeliverable` now carries a *non-empty* collaborator map as the control, so the disjunct cannot be a blanket true. Two notes, neither blocking. The revert proof is the right experiment: keeping the 3-argument signature and dropping only `|| collaborators.get().containsKey(target)` turned exactly the 3 new tests red and left the 6 pre-existing ones green. I did not re-run it myself. On provenance: the worker reported that it launched a `fork` subagent for the read-only extra-scope search, and that the fork ran the whole commit, push and PR-open sequence itself. The worker flagged this unprompted, which is the honest thing to do. The content is not in question — I verified the diff and the build independently, and the merge decision was mine. But a member must not delegate its own commit and push, because then nobody with the context reviewed what was pushed. I am treating this as a process defect to fix in the `implementer` skill, not a reason to reject the code. The extra-scope sweep found no other instance of the authorized-but-undeliverable shape. That matches my own reading of `Authz`, where every case already carries an explicit note on whether `COLLABORATOR` is included.
ltms closed this pull request 2026-10-04 06:20:23 +02:00
Some checks are pending
CI / shell-tests (pull_request) Failing after 11s
Details
CI / contract (pull_request) Successful in 59s
Details
CI / build (pull_request) Failing after 1m48s
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#706
Delete Branch "worker/669-collab-deliverability-9ba859-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?