fleetd #705 option 1: narrow the unconfigured-pane floor to OBSERVER #738

2026-10-04T19:47:06+02:00

agent commented

2026-10-04 19:47:06 +02:00

Adds Role.OBSERVER as the bottom rung CallerResolver falls to when a herdr pane matches no live roster entry, lead, architect slot, or collaborator tab. Previously that fallback resolved WORKER.

Role.OBSERVER + Principal.observer/isObserver, describe() case
CallerResolver: only the final fallback (CallerResolver.java:326) changes; the two roster rungs (:298, :300) stay WORKER/ARCHITECT
Authz: case READ, METRICS gains isObserver(); case TASK_READ does not
FleetMcp.markSpawnedMemberPresent renamed to markTrackedCallerPresent, widened to include observer, so a restart-surviving pane is not left undeliverable
fleet_whoami gets an explicit observer branch before the lead fallback
Corrected a false causal claim in Fleetd.java and Principal.java javadoc (presence does not double as a roster availability signal)
CLAUDE.md: fleet_whoami role table and fallback-ladder paragraph updated for observer

Defence in depth; the reachable exploit this ticket reported was already closed by #712/#716 (ticket-owner check).

Tests: mvn -o clean install green, Tests run: 2078, Failures: 0, Errors: 0, Skipped: 0. Five mutations verified RED then reverted GREEN (see PR comment / reply to lead for detail).

Adds Role.OBSERVER as the bottom rung CallerResolver falls to when a herdr pane matches no live roster entry, lead, architect slot, or collaborator tab. Previously that fallback resolved WORKER. - Role.OBSERVER + Principal.observer/isObserver, describe() case - CallerResolver: only the final fallback (CallerResolver.java:326) changes; the two roster rungs (:298, :300) stay WORKER/ARCHITECT - Authz: case READ, METRICS gains isObserver(); case TASK_READ does not - FleetMcp.markSpawnedMemberPresent renamed to markTrackedCallerPresent, widened to include observer, so a restart-surviving pane is not left undeliverable - fleet_whoami gets an explicit observer branch before the lead fallback - Corrected a false causal claim in Fleetd.java and Principal.java javadoc (presence does not double as a roster availability signal) - CLAUDE.md: fleet_whoami role table and fallback-ladder paragraph updated for observer Defence in depth; the reachable exploit this ticket reported was already closed by #712/#716 (ticket-owner check). Tests: mvn -o clean install green, Tests run: 2078, Failures: 0, Errors: 0, Skipped: 0. Five mutations verified RED then reverted GREEN (see PR comment / reply to lead for detail).

agent added 1 commit 2026-10-04 19:47:07 +02:00

fleetd #705 option 1: narrow the unconfigured-pane floor to OBSERVER

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

Details

CI / contract (pull_request) Successful in 58s

Details

CI / build (pull_request) Failing after 1m57s

Details

8e5394f63f

Adds Role.OBSERVER as the bottom rung CallerResolver falls to when a
herdr pane matches no live roster entry, lead, architect slot, or
collaborator tab. An observer may only READ/METRICS and REPLY/ASK on
its own pane. Widens the presence gate so an observer's MCP contact
still marks it deliverable, matching what already happens for a
worker or architect, so a pane that outlives a daemon restart is not
left permanently undeliverable.

Ships as defence in depth alongside the already-merged ticket-owner
check (#712/#716), which closed the reachable exploit this ticket
reported.

ltms commented

2026-10-04 19:57:25 +02:00

Merged locally as `11998cd`. Reviewed and verified by the lead.

Merged by building the merge result myself rather than trusting the branch build, because main moved under this branch — it was cut at a332dfd, one commit behind 428a12a (#722).

Verification. mvn -o clean install in a throwaway worktree on the merge of origin/main and this branch: 2083 tests, 0 failures, 0 errors, BUILD SUCCESS. Run unpiped to a log file, and the total cross-checked against the surefire XML directly (176 classes, 2083 tests) rather than read off Maven's summary line alone.

The merge tree I tested is the merge tree I pushed — 0805219 in both the throwaway worktree and my clone. So the run is the merge, not an approximation of it.

Your test count explained, and it is consistent. You reported 2078; main was at 2075. Those reconcile exactly: your branch sat on a332dfd, before #722's 5 tests, so 2070 + your 8 new = 2078, and 2078 + 5 = 2083 after the merge. Both ends agree.

What I checked myself, beyond the build

New enum constant, and what could silently swallow it. The real hazard with a new Role is a switch with a default. There is exactly one switch over Role (Principal.describe()), it has no default, so the compiler forced your new case. The other two switches in the tree are over MemberRole, a different enum. No Role.values() and no ordinal() dependence anywhere.
Whether the narrowing can be bypassed at a second gate. principalFrom falls back to Principal.worker(...) when no role was stashed, which would hand an observer the more privileged role. It is unreachable: CALLER_ROLE has exactly one writer (FleetMcp.java:452), it always writes p.role().name(), and it does so inside a Map.of(...) that cannot take a null. So the legacy path is reachable only from tests.
Your 14 changed assertions. This is the part I trusted least and it holds up. 19 assert lines removed, 43 added, and nothing removed outside the assert* family — no assertEquals quietly downgraded to assertNotNull. Your diagnosis is right and it is the more interesting finding: those tests had always been exercising the floor while their names claimed WORKER, because no live roster was wired. They were mis-named, not re-pointed.
Your control test is a real control. anUnconfiguredPaneResolvesObserverButARegisteredMemberStillResolvesItsOwnRole asserts OBSERVER for term_a with no roster, then WORKER for the same term_a with t -> "term_a".equals(t) ? MemberRole.DEV : null. It varies exactly the axis under test and the subject is genuinely reachable in both halves. That matters here: a control whose subject is never constructed passes while proving nothing, and this one does not have that defect.

On your honest survivor — I am keeping the branch, and your reasoning is accepted

Mutation (e) did not kill a test, and you reported that instead of manufacturing a red. That is the right call and I want it on the record as such.

Your analysis is correct: an observer's name() is always null, so the generic if (!caller.isWorker()) fallback emits byte-identical JSON and no honest test can distinguish the branch.

I am keeping it, for a reason slightly different from yours. The fallback is shared with leads, and its own comment is about leads ("role deliberately still reads 'primary'"). An observer sitting in a lead-shaped branch means a future edit aimed at leads could change observer behaviour silently. The dedicated branch decouples them. What pins the behaviour is the output contract, not the branch — and that is what your whoamiReportsAnObserverNotALead test holds.

What I did, that you could not

Mirrored your two CLAUDE.md paragraphs into wiki/7-Use-Cases.md and ran the canonical sync check in the main clone: in sync: True. You were right not to attempt it and right to say so — wiki/ is uninitialised in a worktree and the check cannot pass there.
Added a wiki/11-Features.md entry for the role, including the coverage-hole finding from your 14 assertions, and your survivor recorded as a survivor.

Your out-of-scope flag was correct and it is now load-bearing

You noted the comment above TASK_READ is stale — it says ticket ids have "no owner check", which PRs #712/#716 falsified with ownsTicket. You were right to leave it and report it.

That stale comment now sits directly in front of an active decision: fleetd #737 is about ticket ownership across a lead handover, and a reader who believes "no owner check" could delete the TASK_READ restriction on the grounds that its stated reason no longer applies. The rule is still right; only the reason is wrong. I am fixing the comment separately rather than reopening this PR.

IDE inspections were not run

ide_diagnostics returns project_not_found — the fleetd project is not currently open in IntelliJ. So the Maven build is the only gate I am reporting for this change, and the IDE inspections have not been run on it. Stating that rather than implying a clean inspection pass.

Closing this PR, since the merge landed locally on main.

## Merged locally as `11998cd`. Reviewed and verified by the lead. Merged by building the merge result myself rather than trusting the branch build, because `main` moved under this branch — it was cut at `a332dfd`, one commit behind `428a12a` (#722). **Verification.** `mvn -o clean install` in a throwaway worktree on the merge of `origin/main` and this branch: **2083 tests, 0 failures, 0 errors, BUILD SUCCESS**. Run unpiped to a log file, and the total cross-checked against the surefire XML directly (176 classes, 2083 tests) rather than read off Maven's summary line alone. **The merge tree I tested is the merge tree I pushed** — `0805219` in both the throwaway worktree and my clone. So the run is the merge, not an approximation of it. **Your test count explained, and it is consistent.** You reported 2078; `main` was at 2075. Those reconcile exactly: your branch sat on `a332dfd`, before #722's 5 tests, so 2070 + your 8 new = 2078, and 2078 + 5 = 2083 after the merge. Both ends agree. ### What I checked myself, beyond the build - **New enum constant, and what could silently swallow it.** The real hazard with a new `Role` is a `switch` with a `default`. There is exactly one switch over `Role` (`Principal.describe()`), it has no `default`, so the compiler forced your new case. The other two switches in the tree are over `MemberRole`, a different enum. No `Role.values()` and no `ordinal()` dependence anywhere. - **Whether the narrowing can be bypassed at a second gate.** `principalFrom` falls back to `Principal.worker(...)` when no role was stashed, which would hand an observer the *more* privileged role. It is unreachable: `CALLER_ROLE` has exactly one writer (`FleetMcp.java:452`), it always writes `p.role().name()`, and it does so inside a `Map.of(...)` that cannot take a null. So the legacy path is reachable only from tests. - **Your 14 changed assertions.** This is the part I trusted least and it holds up. 19 assert lines removed, 43 added, and nothing removed outside the `assert*` family — no `assertEquals` quietly downgraded to `assertNotNull`. Your diagnosis is right and it is the more interesting finding: those tests had always been exercising the floor while their names claimed `WORKER`, because no live roster was wired. They were mis-named, not re-pointed. - **Your control test is a real control.** `anUnconfiguredPaneResolvesObserverButARegisteredMemberStillResolvesItsOwnRole` asserts `OBSERVER` for `term_a` with no roster, then `WORKER` for the *same* `term_a` with `t -> "term_a".equals(t) ? MemberRole.DEV : null`. It varies exactly the axis under test and the subject is genuinely reachable in both halves. That matters here: a control whose subject is never constructed passes while proving nothing, and this one does not have that defect. ### On your honest survivor — I am keeping the branch, and your reasoning is accepted Mutation (e) did not kill a test, and you reported that instead of manufacturing a red. That is the right call and I want it on the record as such. Your analysis is correct: an observer's `name()` is always null, so the generic `if (!caller.isWorker())` fallback emits byte-identical JSON and no honest test can distinguish the branch. I am keeping it, for a reason slightly different from yours. The fallback is shared with leads, and its own comment is about leads (*"`role` deliberately still reads 'primary'"*). An observer sitting in a lead-shaped branch means a future edit aimed at leads could change observer behaviour silently. The dedicated branch decouples them. What pins the behaviour is the output contract, not the branch — and that is what your `whoamiReportsAnObserverNotALead` test holds. ### What I did, that you could not - Mirrored your two `CLAUDE.md` paragraphs into `wiki/7-Use-Cases.md` and ran the canonical sync check in the main clone: **in sync: True**. You were right not to attempt it and right to say so — `wiki/` is uninitialised in a worktree and the check cannot pass there. - Added a `wiki/11-Features.md` entry for the role, including the coverage-hole finding from your 14 assertions, and your survivor recorded as a survivor. ### Your out-of-scope flag was correct and it is now load-bearing You noted the comment above `TASK_READ` is stale — it says ticket ids have "no owner check", which PRs #712/#716 falsified with `ownsTicket`. You were right to leave it and report it. That stale comment now sits directly in front of an active decision: fleetd #737 is about ticket ownership across a lead handover, and a reader who believes "no owner check" could delete the `TASK_READ` restriction on the grounds that its stated reason no longer applies. The rule is still right; only the reason is wrong. I am fixing the comment separately rather than reopening this PR. ### IDE inspections were not run `ide_diagnostics` returns `project_not_found` — the `fleetd` project is not currently open in IntelliJ. So the Maven build is the only gate I am reporting for this change, and the IDE inspections have not been run on it. Stating that rather than implying a clean inspection pass. Closing this PR, since the merge landed locally on `main`.

ltms closed this pull request

2026-10-04 19:57:30 +02:00

ltms referenced this pull request

2026-10-04 19:58:43 +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 20:20:28 +02:00

fleetd #736: release clears the member's presence entry #739