Skip to content

feat(console): Fleet Room composer — @-address a member#2132

Merged
mabry1985 merged 7 commits into
mainfrom
feat/fleet-room-polish
Jul 22, 2026
Merged

feat(console): Fleet Room composer — @-address a member#2132
mabry1985 merged 7 commits into
mainfrom
feat/fleet-room-polish

Conversation

@mabry1985

@mabry1985 mabry1985 commented Jul 22, 2026

Copy link
Copy Markdown
Member

Follow-up to #2129 (Fleet Room).

Fixes the regression

In the DM rework the composer became broadcast-only, so you could no longer address a specific member from the input. This restores it:

  • @ in the composer → a member picker → an address chip (@name).
  • Send to a member opens its DM with the message pre-sent — the wired chat streams the reply, so you see the response (not a fire-into-the-void).
  • Broadcast stays the default (no chip, or ⌘↵).

Row-click still opens a member's DM directly; this just adds the composer path back.

Changes

  • FleetRoom: @-mention autocomplete + address-target chip + send routing (member → its DM with the message; else broadcast). Footer updated.
  • PaletteChat: optional initial prop auto-sends once on open; memberDmView threads it from ctx.props.
  • e2e: @-address a member → send → its DM opens.

Testing

tsc ✓ · vitest 628 ✓ · build ✓ · e2e fleet 14 (incl. new @-address test) + keybindings 7 ✓.

Draft

UI — leaving as draft for the feel. The activity feed is still real-events-only (presence + turn.usage); wiring the GitHub/HITL sources for the richer PR/approval rows is separate.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LyQuxgkFpMUJAQbVXnqz2d


Update — event sources are wired + fleet controls folded in (2026-07-22)

8e0d44a: the two missing feed sources are real end to end — the executor fires the progress hook on a HITL pause and _a2a_progress publishes turn.input_required for every context (console: "⚠ needs your approval" feed row + roster pill); GitHub github.pr.opened/merged rows land via the github-plugin's emit (separate plugin PR).

2961e3e:

  • HITL resume hand-back: turn_started frames carry resumed; the server publishes turn.resumed, so the pill flips needs-approval → running the moment the answer lands. 8 new tests cover the publisher + executor frames (both were untested).
  • Folded Feature: Quick-chat with any fleet agent via desktop command palette #1733 quick-chat + Feature: Toggle fleet agents on/off from the command palette #1769 toggle into the room: per-member root commands and the Toggle submorph are gone; member names ride the Fleet Room command's keywords (⌘K "ava" → the room), roster rows carry DM / open / Start / Stop.
  • DM header names the member ("@ava") — closes the known rough edge.
  • e2e: room-based toggle coverage, fold pins, and a full HITL pill-lifecycle walk over a slug-gated mock stream.

Gates on 2961e3e: pytest 3956 ✓ · ruff ✓ · lint-imports ✓ · live-smoke ✓ · tsc/build ✓ · vitest 618 ✓ · e2e fleet 15 + keybindings 7 ✓.
QA: :7882 restarted on this build — the publisher is live, so a real member approval will light the pill.

Restores addressing a specific member from the composer — removed in the DM rework,
which left it broadcast-only (you couldn't @ an agent in the input anymore). Type "@"
in the composer → a member picker → an address chip; sending to that member opens its
DM with the message pre-sent (the wired chat streams the reply), so you see the response
instead of firing into the void. Broadcast stays the default (no chip, or ⌘↵).

- FleetRoom: @-mention autocomplete + address-target chip + send routing (a member →
  its DM with the message; otherwise broadcast). Footer: "@ address in composer".
- PaletteChat: optional `initial` prop auto-sends once on open; memberDmView threads it
  through from ctx.props.
- e2e: @-address ava → send → ava's DM opens.

Follow-up to #2129. Still real-events-only in the activity feed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyQuxgkFpMUJAQbVXnqz2d
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@mabry1985, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 45 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: fec860d1-8165-4875-8b5e-6a6c622a7021

📥 Commits

Reviewing files that changed from the base of the PR and between 18b3c88 and 7b1fbb2.

📒 Files selected for processing (14)
  • a2a_impl/executor.py
  • apps/web/e2e/fleet.spec.ts
  • apps/web/e2e/mock-server.mjs
  • apps/web/src/app/FleetActivity.tsx
  • apps/web/src/app/FleetRoom.tsx
  • apps/web/src/app/PaletteChat.tsx
  • apps/web/src/app/fleet-activity.css
  • apps/web/src/app/fleet-room.css
  • apps/web/src/app/fleetPalette.test.ts
  • apps/web/src/app/fleetPalette.ts
  • apps/web/src/app/usePaletteRegistry.ts
  • server/a2a.py
  • tests/test_a2a_handler.py
  • tests/test_background.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/fleet-room-polish

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

mabry1985 and others added 6 commits July 22, 2026 04:19
…" pill

Enriches the activity feed + roster from the signals that actually flow on the member
buses (no faked rows):
- Live "running a turn" roster pill: autonomous member turns bracket with
  turn.started/turn.finished (already on the bus); a direct broadcast is marked
  optimistically on send and cleared by its terminal turn.usage.
- More mapped topics + richer text: chat.resumed, goal.changed, plus the existing
  turn.usage / activity.message (with text) / inbox.item / scheduler (with name) /
  goal.achieved|failed / background.completed.

Deferred (need NEW sources — documented, not faked):
- "needs approval" pill/rows: requires a bus publish on the executor's input_required
  frame (a2a_impl/executor.py:517 → notify → server/a2a.py publishes "turn.input_required").
  Touches the core HITL path, so it's a supervised change, not done unattended overnight.
- "opened PR" rows: the GitHub plugin is out-of-tree; it would publish github.pr.* and
  the feed would map it.

Follow-up to #2129 / #2132.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyQuxgkFpMUJAQbVXnqz2d
…ge cases

From a self-review pass on the branch:
- Broadcast send-failure now clears the optimistic "running" mark (was stuck forever
  and survived room reopens via the module-level store).
- The running count decrements ONLY on turn.usage (not turn.finished) — autonomous
  turns emit both, which double-decremented and corrupted a concurrent member's count.
- A member's in-flight count resets when its stream closes / it goes offline; the pill
  renders only on online members.
- Errored SSE streams reopen on a backoff (the roster sig doesn't change, so the effect
  wouldn't re-run); the token-fetch continuation bails if unmounted.
- Addressing a member who left the fleet no longer silently broadcasts — it toasts and
  resets to broadcast.
- The DM initial-message auto-send skips when the member's thread has a turn still
  streaming (the self-heal owns that reconnect), avoiding a clobbering concurrent send.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyQuxgkFpMUJAQbVXnqz2d
…Hub PRs)

Both sources the activity feed was missing are now real, end to end.

HITL "needs approval" (in-tree):
- a2a_impl/executor.py: the input_required branch now fires the existing best-effort
  progress hook with {phase: "input_required", prompt}. The hook swallows everything by
  contract, so a parked HITL turn can't be broken by a consumer.
- server/a2a.py: _a2a_progress publishes `turn.input_required` for EVERY context (it
  previously early-returned on non-background), carrying task/context ids + a short prompt.
- Console: maps it to an actionable "⚠ needs your approval — <what>" feed row and a
  roster "needs approval" pill; the pill clears when the parked turn reaches turn.usage.
  A member awaiting input is no longer counted as "running".

GitHub PR events (github-plugin, separate PR):
- The plugin emits `github.pr.opened` / `github.pr.merged` via registry.emit (ADR 0039,
  auto-namespaced), threaded in as a guarded optional `emit` so an older host still loads.
- Console maps both to "opened PR #N · title" / "merged PR #N".

Gates: pytest ✓, ruff ✓, import contracts ✓ (a2a_impl/server layering kept), tsc ✓,
build ✓, e2e 21 ✓, unit 628 ✓; github-plugin 91 ✓.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyQuxgkFpMUJAQbVXnqz2d
…hand-back

Folds the two remaining standalone ⌘K fleet features into the room:
- quick-chat (#1733): the per-member root commands are gone; member names ride
  the Fleet Room command's keywords (typing "ava" surfaces the room) and the
  roster row's DM/open cover the flows. fleetPalette.ts slims to the recency
  store.
- Toggle Fleet Agent (#1769): the submorph + root command are gone; the roster
  row's Start/Stop is the control.

Completes the HITL loop 8e0d44a opened (turn.input_required):
- executor turn_started frames now carry resumed=<bool>; the server publishes
  turn.resumed for every context, so "needs approval" hands BACK to "running"
  the moment the answer lands (previously the pill sat on needs-approval until
  the terminal frame). Console consumes it (pill + "resumed — input received"
  feed row). 8 new tests cover the publisher + the executor frames — both were
  untested.

DM polish: the member-dm header now names the member ("@ava"). PaletteView
titles are registration-static, so the DM body records its target in a tiny
store the title node reads — the known rough edge from the DM rework.

e2e: the toggle-picker specs became room-based coverage (stop/start from the
roster); new specs pin the fold (no per-member root commands; names route to
the room) and walk the full HITL pill lifecycle over a slug-gated mock stream
(x-e2e-hitl); the DM test asserts the "@ava" header.

Gates: pytest 3956 ✓ · ruff ✓ · lint-imports ✓ · live-smoke ✓ (isolated
host-config — the local box host-config.yaml binds 0.0.0.0, which trips the
open-bind guard outside CI) · tsc/build ✓ · vitest 618 ✓ · e2e fleet 15 +
keybindings 7 ✓

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… broadcast count

Two things that bit during QA:

- Typing "@scout do the thing" and hitting ↵ BROADCAST the literal text, because the
  address chip was only ever set by picking from the @-picker. A typed leading "@name" is
  now resolved on send (exact match, else prefix, excluding this instance) and stripped
  from the message — the picker is a convenience, not a requirement. "@name" with no
  message toasts instead of silently broadcasting, and an explicit chip still wins.
- The broadcast chip read "All online · 2" beside a "3 online · 3" header, which reads
  like a bug. It deliberately excludes the instance you're already in, so it's relabelled
  "Everyone else · N" with a tooltip that says exactly that.

e2e: a TYPED @name (never touching the picker) opens that member's DM and does not broadcast.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyQuxgkFpMUJAQbVXnqz2d
The fleet is managed from its HOST instance. On a member window `/api/fleet` is a
fleet-of-one by construction, so the room rendered an empty, misleading surface with no
DM or broadcast targets. The command is now disabled there with a "host instance only"
hint, reusing `fleetSettingsDisabledReason` so the room and Fleet settings gate on the
exact same host/member/standalone matrix — a standalone instance stays enabled (it's
where you create your first member), and a remote member reached directly stays enabled
because it may legitimately run its own fleet.

Disabled rather than hidden, so the command stays discoverable and explains itself.

Closes the last open Fleet Room item.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyQuxgkFpMUJAQbVXnqz2d
@mabry1985
mabry1985 marked this pull request as ready for review July 22, 2026 18:38
@mabry1985
mabry1985 merged commit 241dde6 into main Jul 22, 2026
10 checks passed
@mabry1985
mabry1985 deleted the feat/fleet-room-polish branch July 22, 2026 18:38

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA panel review — FAIL

code-review-structural · head 7b1fbb23e205 · formal

[review-synthesizer completed: workflow code-review-structural:report]

Verification Pass — Final Synthesis

What changed: Finding 6 was fully refuted — the verifier confirmed mapTopic does have a case "turn.usage" entry, so the comment is accurate. Finding 5's sub-claim about "10 unit tests dropped from fleetPalette.test.ts" was also refuted (this PR doesn't touch that file), but its core claim — zero unit tests for the six new state-mutation methods — is confirmed. Findings 1–4 are all confirmed as stated.

Coverage cross-check: FleetActivity.tsx (the most heavily-changed file, ~150 lines of new state-machine logic) has four surviving findings across three categories — good coverage. FleetRoom.tsx has one confirmed finding. No silent files.


Prose brief:

This PR replaces per-component useState tracking with a centralized Zustand store for Fleet turn state. The change is moderate risk: the cleanup-loop bug (finding 1) can leak stale "needs approval" state across remounts, and the SSE-gap counter drift (finding 2) can silently overcount. Fix finding 1 first — it's the only major and the likeliest to surface in normal usage (a turn parked in input_required at unmount). The panel disagreed on finding 6 (refuted — the turn.usage comment was correct all along), and the verifier qualified finding 5 by noting the "10 dropped tests" sub-claim isn't from this PR's diff. No structural-pass gaps to report.

[
  {
    "file": "apps/web/src/app/FleetActivity.tsx",
    "line": 310,
    "severity": "major",
    "category": "correctness",
    "claim": "The cleanup useEffect only clears zustand `running` entries on unmount, missing `awaiting`-only entries — causing stale 'needs approval' pills to persist across remounts when a turn was parked at unmount time and completed while the component was unmounted.",
    "evidence": "The loop `for (const slug of Object.keys(st.running)) st.clear(slug)` iterates only `running` keys. When a turn is in `input_required` state, `markDone` already deleted the slug from `running` and `markAwaiting` added it to `awaiting`. The `clear` method clears BOTH maps, but the loop never reaches the slug because it's no longer in `running`. On remount, `awaiting[slug]` remains `true` with no future frame to clear it if the turn already completed.",
    "source": "removed-behavior",
    "verdict": "confirmed",
    "note": "Cleanup at lines ~306-310 iterates only `Object.keys(st.running)`. A slug that was in `input_required` at unmount (markDone removed it from `running`, markAwaiting added to `awaiting`) is never passed to `clear()`, so its `awaiting[slug]` survives unmount."
  },
  {
    "file": "apps/web/src/app/FleetActivity.tsx",
    "line": 245,
    "severity": "minor",
    "category": "correctness",
    "claim": "If an SSE reconnection gap causes the client to miss a `turn.input_required` frame but receive the subsequent `turn.resumed` frame, the running counter permanently overcounts by +1, leaving a stuck 'running' pill on the roster.",
    "evidence": "The state machine decrements on `turn.input_required` (`markDone`) and increments on `turn.resumed` (`markRunning`), balancing only when both arrive. If `input_required` is dropped: running = started(+1) → [missed input_required] → resumed(+1) → usage(−1) = 1 stuck. `markDone` clamps at 0 but nothing clamps the overflow from an unmatched `turn.resumed`.",
    "source": "correctness",
    "verdict": "confirmed",
    "note": "`turn.resumed` handler unconditionally calls `markRunning(slug)` (increment), but `turn.input_required` is the only path that decrements to offset it. A missed `input_required` means an unmatched +1. `markDone` clamps at 0 but `markRunning` has no ceiling."
  },
  {
    "file": "apps/web/src/app/FleetActivity.tsx",
    "line": 54,
    "severity": "minor",
    "category": "tests",
    "claim": "The PR adds six state-mutation methods to the FleetActivity zustand store (markRunning, markDone, clear, markAwaiting, clearAwaiting) with non-trivial ref-counting, clamping, and bidirectional state transitions — but includes zero unit tests for any of them, while 10 unit tests for removed functions were dropped from fleetPalette.test.ts.",
    "evidence": "`markDone` clamps: `const n = Math.max(0, (s.running[slug] ?? 0) - 1);` — the comment in useFleetStreams says 'a missed pause/resume still settles at zero (markDone clamps)' but this edge case is never unit-tested. Only e2e happy-path coverage exists.",
    "source": "tests",
    "verdict": "confirmed",
    "note": "Zero unit tests for the six new methods — confirmed (diff adds only e2e tests). The '10 unit tests dropped from fleetPalette.test.ts' sub-claim is refuted: this PR's diff does NOT touch fleetPalette.test.ts; the current file has 2 tests, but the reduction is not attributable to this PR."
  },
  {
    "file": "apps/web/src/app/FleetActivity.tsx",
    "line": 100,
    "severity": "minor",
    "category": "conventions",
    "claim": "The exported `clearMemberRunning` name is misleading: it calls `clear(slug)` which wipes BOTH `running` and `awaiting` state, not just running — contrary to its name and JSDoc ('Reset a member's in-flight count entirely').",
    "evidence": "`export const clearMemberRunning = (slug: string) => useFleetActivity.getState().clear(slug);` — but `clear()` deletes both `running[slug]` AND `awaiting[slug]`, while a separate `clearAwaiting` export already exists for the awaiting half.",
    "source": "conventions",
    "verdict": "confirmed",
    "note": "`clearMemberRunning` calls `clear()` which deletes both `running[slug]` and `awaiting[slug]`. The name and JSDoc ('Reset a member's in-flight count entirely') don't mention the awaiting side, and a separate `clearAwaiting` export exists. Naming is indeed misleading."
  },
  {
    "file": "apps/web/src/app/FleetRoom.tsx",
    "line": 174,
    "severity": "minor",
    "category": "correctness",
    "claim": "When `target` points to a member who has left the roster, `submit()` resets to broadcast via `setTarget('broadcast')` and returns — but the composed `draft` may still contain a stripped `@name` prefix from a prior `pickMention` call, and the next Enter press will broadcast text that was originally addressed to someone else.",
    "evidence": "User types `@ava ship it`, presses Enter (picks ava, draft becomes `ship it`), then ava leaves the roster before the second Enter: `targetAgent` goes undefined, `target !== 'broadcast'` is true, the error toast fires, `setTarget('broadcast')` runs, and `submit` returns. On the next Enter, `resolveTypedMention(raw)` finds no agent (draft is `ship it` — no `@` prefix), falls through to `broadcast(raw)`, sending `ship it` to everyone.",
    "source": "correctness",
    "verdict": "confirmed",
    "note": "After `setTarget('broadcast')` the draft still holds `ship it`. Next Enter: target IS 'broadcast' so the error-guard at `!targetAgent && target !== 'broadcast'` no longer fires. `resolveTypedMention('ship it')` finds no `@` prefix, no agent → falls through to `broadcast(raw)`. Text that was meant for ava gets broadcast to all."
  }
]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant