feat(fleet): give every sister agent the fleet surfaces (ADR 0048 §8)#2294
Conversation
"New agent" + "Fleet settings" in the header switcher, the ⌘K Fleet Room, and
Settings ▸ Box ▸ Fleet were gated to the host console to prevent NESTED agents.
That fear never applied to a sister agent's slug window: /api/fleet and
/api/archetypes are hub paths in the console router (isHubPath) — never
slug-scoped — so from /app/agent/<slug>/ all three were already driving the
HUB's fleet. Same roster, same members to start/stop, same fleet to create into.
The gate now asks the one question that distinguishes the real nesting case:
is this a spawned member reached DIRECTLY on its own port? There its
workspaces root is empty by construction, so creating would spawn a grandchild
the hub never sees and would break the "a member's empty workspaces root keeps
shutdown_all hub-only" invariant. That window keeps the tooltip.
Two guards ship with it, reachable only now that member windows have these
panels: a window can't Stop or Remove the agent SERVING it. Both guards keyed
on `host` (correct only on the host console) — on a sister agent's console the
focused member's own row offered a Stop that kills the console you're in.
Settings ▸ Box narrows rather than disappears off the host: Overview and
Telemetry read the focused agent's own endpoints and stay host-only, Fleet
doesn't. Narrowing (not dropping) the group is also what makes the "Fleet
settings" deep-link RESOLVE in a member window — with the group gone,
openGlobalSettings("fleet") fell back to an unrelated section, which is the
half that was genuinely broken.
Unchanged on purpose: the Fleet panel's "Box runtime" chip needs no gate.
QuickSetting picks its layer from field scope (all-host → layer "host") and the
Host layer file is box-tier (box_root/host-config.yaml), one machine-wide file
every instance shares — so a sister agent editing network.bind / fleet.* writes
the same box default the host console writes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WalkthroughFleet settings and Fleet Room surfaces now remain accessible from sister-agent windows. Box visibility is filtered per section, while controls that could stop or remove the serving agent are suppressed. Tests cover navigation, roster behavior, and settings visibility. ChangesSister-agent Fleet behavior
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SisterAgentWindow
participant FleetSwitcher
participant FleetSettings
participant FleetRoom
SisterAgentWindow->>FleetSwitcher: Open Fleet settings or Fleet Room
FleetSwitcher->>FleetSettings: Resolve roster-based access
FleetSettings-->>SisterAgentWindow: Show Fleet surface
SisterAgentWindow->>FleetRoom: Open hub roster
FleetRoom-->>SisterAgentWindow: Show sibling actions and hide serving-agent actions
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/e2e/fleet.spec.ts`:
- Around line 423-436: The Fleet Room test around openFleetRoom must verify
sibling controls are usable, not only visible. After the visibility assertions
for the sibling member controls in the test “a sister agent's window: the ⌘K
Fleet Room opens on the hub's roster” (including the roxy Start button), add
enabled-state assertions with toBeEnabled while preserving the existing
own-agent control exclusion.
In `@apps/web/src/app/fleetSettingsGate.ts`:
- Around line 36-43: Update fleetSettingsDisabledReason to accept roster
readiness/loading state and return the disabled result until the host entry is
known, treating an empty roster as unknown rather than authorized. In
apps/web/src/app/fleetSettingsGate.ts lines 36-43, preserve the existing member
and resolved-roster behavior; in apps/web/src/app/fleetSettingsGate.test.ts
lines 54-57, replace the []-enables expectation with coverage confirming the
unknown/loading state remains blocked.
In `@apps/web/src/settings/FleetManagerPanel.tsx`:
- Around line 411-417: The active remote member must not expose unregister
controls. Update the earlier remote-action branch in FleetManagerPanel,
identified by its “Remove from this fleet” rendering, to require !isActive
before showing Remove, while preserving existing behavior for inactive remote
members.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 47dad014-853b-4da4-aab2-4e3bf11e6b5c
⛔ Files ignored due to path filters (1)
CHANGELOG.mdis excluded by!*.md
📒 Files selected for processing (13)
apps/web/e2e/fleet.spec.tsapps/web/e2e/settings.spec.tsapps/web/src/app/FleetRoom.tsxapps/web/src/app/FleetSwitcher.tsxapps/web/src/app/fleetSettingsGate.test.tsapps/web/src/app/fleetSettingsGate.tsapps/web/src/app/usePaletteRegistry.tsapps/web/src/settings/FleetManagerPanel.tsxapps/web/src/settings/SettingsSurface.tsxapps/web/src/settings/boxSectionGate.test.tsapps/web/src/settings/secretsGate.test.tsapps/web/src/settings/sectionGate.tsdocs/adr/0048-settings-ia-two-scope-homes.md
| test("a sister agent's window: the ⌘K Fleet Room opens on the hub's roster", async ({ page }) => { | ||
| await page.goto("/app/agent/ava/", { waitUntil: "load" }); | ||
| await openFleetRoom(page); | ||
| const room = page.locator(".flr"); | ||
| // The hub's real roster — its siblings are here, not an empty fleet-of-one. | ||
| await expect(room.locator(".flr__member", { hasText: "main" })).toBeVisible(); | ||
| await expect(room.locator(".flr__member", { hasText: "roxy" })).toBeVisible(); | ||
| // But no Stop on its OWN row: that button would kill the agent serving this window. | ||
| await expect( | ||
| room.locator(".flr__member", { hasText: "ava" }).getByRole("button", { name: /^(Stop|Start) ava$/ }), | ||
| ).toHaveCount(0); | ||
| // A sibling still toggles normally. | ||
| await expect(room.locator(".flr__member", { hasText: "roxy" }).getByRole("button", { name: "Start roxy" })).toBeVisible(); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo files matching fleet spec:"
fd -a 'fleet\.spec\.ts$' . || true
echo
echo "Relevant fleet.spec.ts excerpts:"
if [ -f apps/web/e2e/fleet.spec.ts ]; then
sed -n '390,460p' apps/web/e2e/fleet.spec.ts | nl -ba -v390
fi
echo
echo "Search for similar member button expectations in fleet spec:"
rg -n 'flr__member|toBeVisible\(\)|toBeEnabled\(\)|Start .*ava|Start .*roxy|Stop .*ava|Stop .*roxy' apps/web/e2e/fleet.spec.ts || true
echo
echo "Check Playwright version if present:"
for f in package.json pnpm-lock.yaml package-lock.json yarn.lock; do
[ -e "$f" ] && echo "--- $f" && rg -n '"`@playwright/test`"|"playwright"|"playwright-core"|version:' "$f" | head -40 || true
doneRepository: protoLabsAI/protoAgent
Length of output: 313
🏁 Script executed:
#!/bin/bash
set -euo pipefail
REPO=/home/jailuser/git
echo "Relevant fleet.spec.ts excerpts with line numbers:"
awk 'NR>=390 && NR<=460 { printf "%6d\t%s\n", NR, $0 }' "$REPO/apps/web/e2e/fleet.spec.ts"
echo
echo "Search for member button expectations in fleet.spec.ts:"
rg -n 'flr__member|toBeVisible|toBeEnabled|Start .*ava|Start .*roxy|Stop .*ava|Stop .*roxy' "$REPO/apps/web/e2e/fleet.spec.ts" || true
echo
echo "Playwright version references:"
for f in package.json pnpm-lock.yaml package-lock.json yarn.lock; do
[ -e "$f" ] && {
echo "--- $f"
rg -n '"`@playwright/test`"|"playwright"|"playwright-core"|version:' "$f" | head -60 || true
}
done
echo
echo "Behavioral probe: toHaveCount(0) and toHaveCount(1) with disabled visible elements in current env if installed:"
node - <<'JS'
try {
const pw = require('`@playwright/test`');
console.log("`@playwright/test` module:", pw && typeof pw.test);
} catch (e) {
console.log("`@playwright/test` module: unavailable in NODE_PATH");
}
JSRepository: protoLabsAI/protoAgent
Length of output: 9925
🌐 Web query:
Playwright toHaveCount matcher does disabled elements count visible to match visible assertions
💡 Result:
In Playwright, the toHaveCount assertion counts all elements that match the provided locator, regardless of whether they are disabled or hidden [1][2]. It does not automatically filter out elements based on their visibility or enabled state [2]. If you need to count only visible elements or only enabled elements, you must refine your locator to explicitly include those criteria: For visible elements: Use the:visible CSS pseudo-class or the.filter({ visible: true }) method [3][2][4]. Example: await expect(page.locator('button:visible')).toHaveCount(3); Example: await expect(page.locator('button').filter({ visible: true })).toHaveCount(3); For enabled elements: Use the:disabled CSS pseudo-class or filter by state [5]. Example: await expect(page.locator('button:not(:disabled)')).toHaveCount(2); By default, standard locators match all elements present in the DOM that satisfy the selector, including those that are disabled (e.g., have a [disabled] attribute) or hidden (e.g., have display: none) [1][2]. The toHaveCount assertion simply asserts that the total count of matches meets your expectation [6][7].
Citations:
- 1: https://playwright.dev/docs/actionability
- 2: https://runebook.dev/en/docs/playwright/api/class-locator/locator-count
- 3: https://stackoverflow.com/questions/79514828/tobevisible-count-of-onscreen-items
- 4: https://playwright.dev/docs/other-locators
- 5: https://stackoverflow.com/questions/77225196/how-to-select-count-disabled-enabled-elements-in-playwright
- 6: https://playwright.dev/docs/api/class-locatorassertions
- 7: https://playwright.dev/docs/locators
🌐 Web query:
Playwright toBeVisible assertion button disabled default aria-disabled ignored
💡 Result:
In Playwright, the toBeVisible assertion checks only for element visibility (a non-empty bounding box and no visibility:hidden style) [1][2][3]. It does not check whether an element is enabled or disabled [4][5]. Regarding your query about disabled states: 1. toBeVisible and disabled state: The toBeVisible assertion is completely independent of whether a button is disabled, enabled, or has aria-disabled attributes [4][1][5]. An element can be both visible and disabled at the same time [1][2]. 2. Playwright's definition of "disabled": Playwright considers an element disabled if it meets specific criteria [1][3]. Crucially, for descendants of an element with the aria-disabled=true attribute, Playwright recognizes the element as disabled [1][6][2]. 3. Default behavior: Playwright does not ignore aria-disabled. If a button is a descendant of an element with aria-disabled="true", Playwright correctly identifies it as disabled [1][3]. If you are experiencing issues where Playwright is not respecting aria-disabled, ensure that the locator is targeting the correct element and that the attribute is correctly applied to a parent in the DOM hierarchy [6]. If you need to verify that a button is disabled, you should use the dedicated toBeDisabled assertion rather than relying on visibility checks [4][7]. Summary of Key Assertions: - toBeVisible: Checks if the element is visible in the DOM (non-empty bounding box, not visibility:hidden) [1][3]. - toBeDisabled: Checks if the element is disabled via the disabled attribute, fieldset, or parent aria-disabled="true" attribute [4][1][3].
Citations:
- 1: https://playwright.dev/docs/actionability
- 2: https://playwright.dev/docs/next/actionability
- 3: https://github.com/microsoft/playwright/blob/main/docs/src/actionability.md
- 4: https://playwright.dev/docs/api/class-locatorassertions
- 5: https://playwright.dev/docs/test-assertions
- 6: [Bug]: aria-disabled=true attribute not respected in Enabled actionability check microsoft/playwright#35423
- 7: https://playwright.dev/docs/api/class-locator
Assert sibling controls are enabled, not merely rendered.
toBeVisible() only checks DOM visibility, so a disabled sibling Start button can still pass. Append toBeEnabled() after the visibility asserts at Line 435 and Line 448 to catch a guard that disables all fleet controls instead of only the current agent’s own controls.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/e2e/fleet.spec.ts` around lines 423 - 436, The Fleet Room test
around openFleetRoom must verify sibling controls are usable, not only visible.
After the visibility assertions for the sibling member controls in the test “a
sister agent's window: the ⌘K Fleet Room opens on the hub's roster” (including
the roxy Start button), add enabled-state assertions with toBeEnabled while
preserving the existing own-agent control exclusion.
| * Why the fleet surfaces are unavailable in this window — or `null` when they're allowed. | ||
| * `agents` is the polled /api/fleet list. The window's URL slug deliberately does NOT enter | ||
| * into it: a sister agent's slug window drives the hub's fleet, exactly as the host console does. | ||
| */ | ||
| export function fleetSettingsDisabledReason(agents: FleetAgent[], slug: string): string | null { | ||
| if (slug !== "host") return FLEET_SETTINGS_MEMBER_TOOLTIP; // hub slug window on a member | ||
| export function fleetSettingsDisabledReason(agents: FleetAgent[]): string | null { | ||
| const self = agents.find((a) => a.host); | ||
| if (self?.member) return FLEET_SETTINGS_MEMBER_TOOLTIP; // spawned member, reached directly | ||
| return null; // fleet host, or a standalone instance (no fleet configured yet) | ||
| return null; // host, sister-agent slug window, standalone, or a remote's own console |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Treat an empty roster as unknown, not authorized. The callers use [] before the fleet poll resolves, so enabling the gate at that point can expose “New agent” in a directly reached member window and allow nested fleet creation.
apps/web/src/app/fleetSettingsGate.ts#L36-L43: accept roster readiness/loading state and keep controls disabled until the host entry is known.apps/web/src/app/fleetSettingsGate.test.ts#L54-L57: replace the[] => enabledexpectation with coverage for the blocked unknown/loading state.
📍 Affects 2 files
apps/web/src/app/fleetSettingsGate.ts#L36-L43(this comment)apps/web/src/app/fleetSettingsGate.test.ts#L54-L57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/fleetSettingsGate.ts` around lines 36 - 43, Update
fleetSettingsDisabledReason to accept roster readiness/loading state and return
the disabled result until the host entry is known, treating an empty roster as
unknown rather than authorized. In apps/web/src/app/fleetSettingsGate.ts lines
36-43, preserve the existing member and resolved-roster behavior; in
apps/web/src/app/fleetSettingsGate.test.ts lines 54-57, replace the []-enables
expectation with coverage confirming the unknown/loading state remains blocked.
| {/* The agent serving THIS console can't act on itself — its display name | ||
| is edited in Settings → Identity instead. On the host console that's | ||
| the host row; in a sister agent's window (where this panel now also | ||
| renders) it's that member's row, so the guard keys on the focused slug | ||
| as well as `host` — otherwise "Stop"/"Remove" would kill or delete the | ||
| very agent serving the window you're in. */} | ||
| {a.host || a.remote || isActive ? null : ( |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Guard the active remote row’s unregister controls too.
This condition only covers the generic controls rendered below it. The earlier a.remote branch still renders “Remove from this fleet” when isActive is true. In a sister-agent window, that can unregister the member serving the current console despite the stated self-protection. Add !isActive to that remote action branch, at least for Remove.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/settings/FleetManagerPanel.tsx` around lines 411 - 417, The
active remote member must not expose unregister controls. Update the earlier
remote-action branch in FleetManagerPanel, identified by its “Remove from this
fleet” rendering, to require !isActive before showing Remove, while preserving
existing behavior for inactive remote members.
![protoreview[bot]](https://avatars.githubusercontent.com/in/4233969?s=60&v=4){kind=small}
There was a problem hiding this comment.
QA panel review — PASS
code-review-structural · head 9a3a407800f7 · formal
[review-synthesizer completed: workflow code-review-structural:report]
Brief
Overall risk: Low. The PR diff touches only FleetRoom.tsx, FleetSwitcher.tsx, fleetSettingsGate.ts, fleetSettingsGate.test.ts, plus e2e and CHANGELOG changes. No correctness, security, or cross-file defects were identified.
Fix-first item: None — the panel found no actionable defects.
Panel disagreement: None. Both structural findings were refuted by verification as they referenced files not present in this PR.
Verification changes: Both findings from the structural pass (protoPatch) were marked REFUTED — usePaletteRegistry.ts and SettingsSurface.tsx do not exist in this PR (404 on both head and main). The structural finder appears to have scanned stale paths.
Coverage gap: The structural pass (protoPatch) produced two false positives on non-existent files. The actual diff was not flagged by the structural engine, which is unusual but not necessarily problematic — it may indicate the structural pass operates on a different code snapshot or has a path-resolution issue. The LLM panel findings (if any) were not included in this input, so I cannot assess whether they were also refuted or carried forward.
[]1 participant
What
The three fleet affordances — header switcher ▸ New agent + Fleet settings, ⌘K ▸ Fleet Room, and Settings ▸ Box ▸ Fleet — render in a fleet member's window now, not only on the host console.
Why the old gate was too broad
It existed to prevent nested agents. That fear doesn't apply to a sister agent's slug window:
/api/fleetand/api/archetypesare hub paths in the console router (isHubPath,apps/web/src/lib/api.ts) — never slug-scoped — so from/app/agent/<slug>/all three surfaces were already driving the hub's fleet. Same roster, same members to start/stop, same fleet to create into. Nothing nested; the surfaces were just switched off.The gate now asks the one question that distinguishes the real case: is this a spawned member reached directly on its own port? There its
workspaces_root()is empty by construction (graph/workspaces/manager.py), so creating would spawn a grandchild the hub never lists and break the "a member's empty workspaces root keepsshutdown_allhub-only" invariant. That window keeps the disabled items + point-at-the-host tooltip.Only the URL-slug clause was removed from
fleetSettingsDisabledReason; themember: trueself-report clause stays.Two guards that ship with it
Reachable only now that member windows have these panels — a window may not act destructively on the agent serving it:
FleetManagerPanel— the "can't act on itself" guard keyed ona.host, correct only on the host console. On a sister agent's console the focused member's own row offered Stop (kills the console you're standing in) and Remove (deletes its workspace out from under you). Now keys on the focused slug too.FleetRoom— same for the roster's start/stop toggle. The room already computedherefor DM/broadcast targets; the toggle just didn't use it.Settings ▸ Box narrows, it doesn't disappear
hostOnlyis now per section, in the purevisibleSectionsgate alongside the ADR 0068 flag axis. Overview and Telemetry read the focused agent's own endpoints (/api/runtime,/api/telemetry) and stay host-only; Fleet names the same box from any window and doesn't.Narrowing rather than dropping the group is also what makes the "Fleet settings" deep-link resolve in a member window — with the group gone,
openGlobalSettings("fleet")fell back to an unrelated section. That was the half that was genuinely broken.Left alone on purpose
The Fleet panel's "Box runtime" chip needs no gate.
QuickSettingpicks its layer from field scope (all-host →layer: "host"), and the Host layer file is box-tier (box_root/host-config.yaml,infra/paths.py) — one machine-wide file every instance on the box shares. A sister agent editingnetwork.bind/fleet.*writes the same box default the host console writes.Tests
fleetSettingsGate.test.tsrewritten for the new rule (incl. an empty-roster case sodisabledis never the cold-open default).boxSectionGate.test.ts— both gate axes plus source guards on thehostOnlykeys./app/agent/ava/window: Fleet settings opens the panel, New agent opens the picker, the Fleet Room shows the hub's roster with no Stop on its own row, and the fleet panel offers no Stop/Remove on its own row (siblings keep theirs).Green:
npm run test:unit(734),npx playwright test e2e/fleet.spec.ts e2e/settings.spec.ts(34), ande2e/fleet.spec.ts --repeat-each=2(44). The full 248-test e2e suite flakes one random test per run on unmodifiedorigin/maintoo (verified by stashing this branch and re-running) — different test each time.Zero Python files touched, so the Python gates are unaffected.
Draft per the UI local-test gate — CI can't judge whether these surfaces feel right in a real member window.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation