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

fleetd #756/#758: panes array reads the architect-slot gate and widens to observer #762

Closed
agent wants to merge 0 commits from worker/756-758-observer-pane-discovery-7e6ffd-1 into main
pull from: worker/756-758-observer-pane-discovery-7e6ffd-1
merge into: fleet:main
Conversation 1 Commits - Files Changed -
agent commented 2026-10-05 10:24:15 +02:00
Member
Copy Link
Copy Source

Closes #756 and #758.

#756 — paneRole now reads CallerResolver#boundToArchitectSlot (widened to public, not re-derived) so a pane bound to a configured architect slot with no live member session reports role: "architect", matching what sendableObserverTarget already permits as a SEND target. The PaneSource record gained an architectSlot predicate for this, following the existing deliverable field's pattern of reading the gate's own source.

#758 — panesVisibleTo now admits an observer (it holds SEND to another observer pane). The observer's panes rows are filtered to CallerResolver#sendableObserverTarget (same predicate the SEND gate runs) and reduced to exactly sessionId, label, status, role, deliverable — dropping paneId, workspaceId, tabId, agentType, cwd. A primary/architect/collaborator row is unchanged. Authz.TASK_READ and Authz.COORD_SEND are untouched.

Tests: 4 new behavioral tests (via the existing listFleet-direct-call precedent in FleetMcpTest, driving a real CallerResolver) plus one pre-existing FleetMcpAuthzTest test updated — it encoded the old "observer never sees panes" invariant that #758 explicitly overturns.

mvn clean install: BUILD SUCCESS, Tests run: 2140, Failures: 0, Errors: 0, Skipped: 0.

Closes #756 and #758. **#756** — `paneRole` now reads `CallerResolver#boundToArchitectSlot` (widened to `public`, not re-derived) so a pane bound to a configured architect slot with no live member session reports `role: "architect"`, matching what `sendableObserverTarget` already permits as a SEND target. The `PaneSource` record gained an `architectSlot` predicate for this, following the existing `deliverable` field's pattern of reading the gate's own source. **#758** — `panesVisibleTo` now admits an observer (it holds SEND to another observer pane). The observer's `panes` rows are filtered to `CallerResolver#sendableObserverTarget` (same predicate the SEND gate runs) and reduced to exactly `sessionId`, `label`, `status`, `role`, `deliverable` — dropping `paneId`, `workspaceId`, `tabId`, `agentType`, `cwd`. A primary/architect/collaborator row is unchanged. `Authz.TASK_READ` and `Authz.COORD_SEND` are untouched. Tests: 4 new behavioral tests (via the existing listFleet-direct-call precedent in `FleetMcpTest`, driving a real `CallerResolver`) plus one pre-existing `FleetMcpAuthzTest` test updated — it encoded the old "observer never sees panes" invariant that #758 explicitly overturns. `mvn clean install`: BUILD SUCCESS, Tests run: 2140, Failures: 0, Errors: 0, Skipped: 0.
agent added 1 commit 2026-10-05 10:24:16 +02:00
fleetd #756/#758: panes array reads the architect-slot gate and widens to observer
CI / shell-tests (pull_request) Failing after 8s
Details
CI / contract (pull_request) Successful in 52s
Details
CI / build (pull_request) Failing after 2m9s
Details
5c2f296bc3 
paneRole now reads CallerResolver#boundToArchitectSlot (made public, no
second definition) so a slot-bound pane with no live member reports
"architect", matching what sendableObserverTarget already allowed as a
SEND target.

panesVisibleTo now admits an observer, since an observer holds SEND to
another observer pane. Its panes rows are filtered to
sendableObserverTarget and reduced to sessionId/label/status/role/
deliverable; every other caller's rows are unchanged.
ltms commented 2026-10-05 10:34:11 +02:00
Owner
Copy Link
Copy Source

Merged locally to main as 5f7f388. Closing this PR because the commits are in; it was not merged through the forge.

The seam is the right one

paneRole now calls panes.architectSlot(), wired to callers::boundToArchitectSlot — the same method sendableObserverTarget() already used. Widening it from private to public rather than writing a second check is exactly right: the row's reported role and the gate's real decision now read one source, which is the whole point of #756. Same for the filter, which uses callers.sendableObserverTarget() itself rather than a copy.

I also checked the branch order in paneRole against CallerResolver.resolve: session → lead → architect slot → collaborator → observer, in both. They agree, so a pane matching two maps resolves the same way in the row as at the gate.

I did not accept the "before-failure" evidence as given

You were straight about this, and you were right to be: the change is structural, so reverting production left a compile failure, not a test failure. That proves the tests touch new API. It does not prove they would catch a regression. So I mutated the merged code and ran the two test classes against each mutation.

Mutation Result
panesVisibleTo drops || caller.isObserver() killed — FleetMcpAuthzTest.onlyPrimaryArchitectCollaboratorAndObserverMaySeeThePanesArray
the row filter becomes .filter(a -> true) (observer sees every pane) killed — FleetMcpTest.listFiltersAndReducesThePanesArrayForAnObserver
paneRow keeps paneId/workspaceId/tabId for an observer killed — same test
paneRole's architect branch deleted killed — FleetMcpTest.listReportsArchitectForASlotBoundPaneWithNoLiveMember
paneRow keeps cwd for an observer survived

Baseline unmutated: green. So the probe works and four of five sites are genuinely pinned.

The survivor is unreachable, not a hole

listFiltersAndReducesThePanesArrayForAnObserver does assert assertFalse(out.contains("\"cwd\"")). It survived because cwd is only written when session != null && session.cwd() != null — a spawned member — and the filter has already removed every spawned member, since sendableObserverTarget requires spawnedMemberRole.apply(target) == null. The fixture therefore never builds a row where cwd could appear, and in production it cannot either.

So the !observerView && on the cwd line is redundant defence in depth, and it is structurally unexercisable through listFleet: any pane that could carry a cwd is a pane the filter drops. I am not asking for a change — keeping the guard is correct if the filter is ever widened — but nothing would tell anyone if that guard broke, and the passing assertion reads as coverage it does not have. Noted on #758 rather than fixed here.

This is the same shape as the earlier case where a positive control passed while its subject was unreachable: the assertion was right, the state was never constructed.

Checks on your test edit

Flipping onlyPrimaryArchitectAndCollaboratorMaySeeThePanesArray was the correct call — it encoded the invariant #758 exists to overturn — and reversing a pre-existing assertion is the moment a control usually gets lost. It did not here: the renamed test still carries assertFalse(panesVisibleTo(WORKER_A)) and assertFalse(panesVisibleTo(ANON)), so a mutation that admitted everyone would still fail. Confirmed by M1 killing through that test.

Counts

MVN_EXIT=0, surefire XML summed myself: xml=178 tests=2140 failures=0 errors=0 skipped=0.

Your reported 2140 against main's 2137 is +3, while you described four new tests, so I checked the arithmetic rather than assume: the diff adds 4 void test methods and removes 1 (the renamed one), net +3. Consistent.

Merged tree 5fb13129 equals main's tree after the merge, so the bytes I mutated and built are the bytes that shipped.

Your two out-of-scope notes

  • The two near-duplicate listFleet overloads. Following the file's existing pattern was the right call for this PR. It is worth a ticket of its own; assembleAndStart is capped for the same reason and this is the same pressure.
  • gitea get_comments worked for you. That contradicts the project note that the mounted forge MCP server holds a blocked credential. Useful and unexpected — I will re-measure it rather than edit the note on one observation, since the note exists to stop workers trusting a tool that fails silently.
Merged locally to `main` as `5f7f388`. Closing this PR because the commits are in; it was not merged through the forge. ## The seam is the right one `paneRole` now calls `panes.architectSlot()`, wired to `callers::boundToArchitectSlot` — the same method `sendableObserverTarget()` already used. Widening it from `private` to `public` rather than writing a second check is exactly right: the row's reported role and the gate's real decision now read one source, which is the whole point of #756. Same for the filter, which uses `callers.sendableObserverTarget()` itself rather than a copy. I also checked the branch order in `paneRole` against `CallerResolver.resolve`: session → lead → architect slot → collaborator → observer, in both. They agree, so a pane matching two maps resolves the same way in the row as at the gate. ## I did not accept the "before-failure" evidence as given You were straight about this, and you were right to be: the change is structural, so reverting production left a **compile** failure, not a test failure. That proves the tests touch new API. It does not prove they would catch a regression. So I mutated the merged code and ran the two test classes against each mutation. | Mutation | Result | |---|---| | `panesVisibleTo` drops `\|\| caller.isObserver()` | **killed** — `FleetMcpAuthzTest.onlyPrimaryArchitectCollaboratorAndObserverMaySeeThePanesArray` | | the row filter becomes `.filter(a -> true)` (observer sees every pane) | **killed** — `FleetMcpTest.listFiltersAndReducesThePanesArrayForAnObserver` | | `paneRow` keeps `paneId`/`workspaceId`/`tabId` for an observer | **killed** — same test | | `paneRole`'s architect branch deleted | **killed** — `FleetMcpTest.listReportsArchitectForASlotBoundPaneWithNoLiveMember` | | `paneRow` keeps **`cwd`** for an observer | **survived** | Baseline unmutated: green. So the probe works and four of five sites are genuinely pinned. ## The survivor is unreachable, not a hole `listFiltersAndReducesThePanesArrayForAnObserver` *does* assert `assertFalse(out.contains("\"cwd\""))`. It survived because `cwd` is only written when `session != null && session.cwd() != null` — a spawned member — and the filter has already removed every spawned member, since `sendableObserverTarget` requires `spawnedMemberRole.apply(target) == null`. The fixture therefore never builds a row where `cwd` could appear, and in production it cannot either. So the `!observerView &&` on the `cwd` line is **redundant defence in depth**, and it is structurally unexercisable through `listFleet`: any pane that could carry a `cwd` is a pane the filter drops. I am not asking for a change — keeping the guard is correct if the filter is ever widened — but nothing would tell anyone if that guard broke, and the passing assertion reads as coverage it does not have. Noted on #758 rather than fixed here. This is the same shape as the earlier case where a positive control passed while its subject was unreachable: the assertion was right, the state was never constructed. ## Checks on your test edit Flipping `onlyPrimaryArchitectAndCollaboratorMaySeeThePanesArray` was the correct call — it encoded the invariant #758 exists to overturn — and reversing a pre-existing assertion is the moment a control usually gets lost. It did not here: the renamed test still carries `assertFalse(panesVisibleTo(WORKER_A))` and `assertFalse(panesVisibleTo(ANON))`, so a mutation that admitted everyone would still fail. Confirmed by M1 killing through that test. ## Counts `MVN_EXIT=0`, surefire XML summed myself: `xml=178 tests=2140 failures=0 errors=0 skipped=0`. Your reported 2140 against main's 2137 is +3, while you described four new tests, so I checked the arithmetic rather than assume: the diff adds 4 `void` test methods and removes 1 (the renamed one), net +3. Consistent. Merged tree `5fb13129` equals `main`'s tree after the merge, so the bytes I mutated and built are the bytes that shipped. ## Your two out-of-scope notes - **The two near-duplicate `listFleet` overloads.** Following the file's existing pattern was the right call for this PR. It is worth a ticket of its own; `assembleAndStart` is capped for the same reason and this is the same pressure. - **`gitea get_comments` worked for you.** That contradicts the project note that the mounted forge MCP server holds a blocked credential. Useful and unexpected — I will re-measure it rather than edit the note on one observation, since the note exists to stop workers trusting a tool that fails silently.
ltms closed this pull request 2026-10-05 10:34:15 +02:00
Some checks are pending
CI / shell-tests (pull_request) Failing after 8s
Details
CI / contract (pull_request) Successful in 52s
Details
CI / build (pull_request) Failing after 2m9s
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#762
Delete Branch "worker/756-758-observer-pane-discovery-7e6ffd-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?