Skip to content

feat: chat-anchored monitors & shells with same-turn delivery wire - #1009

Merged
AmiteshwarRandhawa merged 17 commits into
mainfrom
monitor-shell-improvements
Aug 7, 2026
Merged

AmiteshwarRandhawa merged 17 commits into
mainfrom
monitor-shell-improvements

Conversation

@AmiteshwarRandhawa

Copy link
Copy Markdown
Contributor

What

The chat becomes the home for its monitors and shells, and the queue-item wire gains same-turn delivery fields.

Protocol (chat.subscribe, still-unshipped 1.6 line — no version ritual needed, verified absent from all release tags):

  • The managed-command queue item gains delivery/targetTurnId and a steering status, all defaulted so rows written by earlier builds rehydrate as the next-turn fallback.

GUI:

  • Floating top-right menu on the chat tile — present exactly while the chat owns a command in any state (never yanked while open); badge: quiet / running count / attention count (attention only on unexpected exits, beats running, acknowledged by opening the menu, re-arms on re-failure).
  • Menu rows: all the chat's commands, running first; stop/start/delete with the destructive-delete confirmation; click opens the output window; drag a row out to open — or move — its output window at the drop target (rides the canvas dnd machinery; move-vs-open resolves by content id).
  • Epic sidebar "Monitors & Shells" list removed — persisted layouts rehydrate safely (unknown panel ids are filtered, not layout-resetting).
  • Background panel unified: managed commands are ordinary rows in the one list (kind icons carry the monitor/shell distinction; the harness's own "monitor" kind keeps its label and screen icon), the "Not stopped by Stop all" disclaimer is gone because Stop all now genuinely stops everything listed — one aggregated action, single pending state, single partial-failure toast. Managed-row stop no longer gated on the chat stream.
  • Output window follows the Terminal font settings (family + size, via a shared resolution hook also adopted by the terminal tile and appearance panel).
  • Stale-list / host-too-old notices re-homed into the menu.

Review

Built by two independent implementation rounds, then an adversarial review (probe tests, not inspection) that confirmed 8 defects — including drag-out being a complete no-op due to popover unmount killing the dnd payload, and the sidebar-layout upgrade reset — followed by a fix round in which every fix landed with the test that would have caught it, proof-of-red verified.

Validation

Targeted suites green (drag/pointer-gesture suite included), gui-app compile, ESLint, Prettier, react-doctor all clean. Known residual: drag-out deserves one manual browser pass (jsdom can't fully prove the pointer gesture).

🤖 Generated with Claude Code

Protocol: the managed-command queue item gains delivery/targetTurnId and a
steering status (all defaulted; the chat.subscribe line carrying the variant
is still unshipped, so old rows rehydrate as the next-turn fallback).

GUI: the chat becomes the home for its monitors and shells - a floating
top-right menu (presence keyed to existence, attention badge acknowledged by
opening, rows with lifecycle controls, drag a row out to place or move its
output window), the epic sidebar list is gone, the Background panel folds
managed rows into its one list with a Stop all that genuinely stops them,
and the output window follows the terminal font settings.

Signed-off-by: Amiteshwar Randhawa <amiteshwar04@gmail.com>
@coderabbitai

coderabbitai Bot commented Aug 5, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Summary by CodeRabbit

  • New Features

    • Managed commands now appear in the chat menu and Background panel with status, timing, attention indicators, output navigation, and lifecycle controls.
    • Added “Stop all” support for active background items and managed commands.
    • Managed-command output can be dragged onto the canvas to open or reposition a tile.
    • Added same-turn delivery support, including a “Delivering” status for queued commands.
  • Improvements

    • Terminal and command-output views now honor configured typography settings.
    • Removed the separate Monitors & Shells sidebar panel.
    • Improved recovery of saved sidebar layouts containing retired or invalid panels.

Walkthrough

Managed commands move from the Epic sidebar and strip into chat background rows and a chat menu. The change adds attention tracking, aggregate stopping, canvas drag-and-drop, shared terminal font resolution, queue metadata, and persisted-panel normalization.

Changes

Managed command surfaces

Layer / File(s) Summary
Command state and queue contracts
protocol/src/host/agent/gui/subscribe.ts, clients/gui-app/src/hooks/managed-command/..., clients/gui-app/src/stores/managed-commands/...
Adds queue delivery metadata, attention tracking, chat-scoped command filtering, aggregate stop handling, and steering-state rendering.
Chat background and menu surfaces
clients/gui-app/src/components/chat/..., clients/gui-app/src/components/managed-commands/..., clients/gui-app/src/components/epic-canvas/renderers/chat-tile.tsx, clients/gui-app/src/__tests__/acceptance/...
Renders managed commands in Background rows and a chat menu with lifecycle actions, attention badges, output navigation, connection states, and updated acceptance coverage.
Managed command output dragging
clients/gui-app/src/components/epic-canvas/dnd/..., clients/gui-app/src/components/managed-commands/__tests__/..., clients/gui-app/src/components/chat/composer/...
Adds managed-command output drag sources, canvas placement, deduplication, and exclusion from composer attachments.
Shared terminal typography
clients/gui-app/src/hooks/settings/use-effective-terminal-font.ts, clients/gui-app/src/components/epic-canvas/renderers/..., clients/gui-app/src/components/settings/...
Centralizes terminal font resolution for xterm, previews, and managed-command output timelines.
Sidebar retirement and persisted layouts
clients/gui-app/src/components/epic-canvas/sidebar/..., clients/gui-app/src/stores/epics/..., clients/gui-app/src/components/epic-canvas/__tests__/...
Removes the managed-command sidebar panel and normalizes persisted layouts that contain unknown or retired panel IDs.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested labels: protocol-compat-override

Suggested reviewers: tanveergill

Poem

A rabbit moves commands from rail to chat,
With badges bright and rows that stop on tap.
Tiles hop canvas-wide, fonts settle right,
Old panels fade into the night.
Queue fields steer each turn with care—
New paths bloom everywhere.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 29.58% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main changes: chat-anchored monitors and shells plus same-turn delivery support.
Description check ✅ Passed The description directly explains the chat migration, protocol changes, GUI behavior, validation, and known residual work.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch monitor-shell-improvements

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

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b8ee4e2111

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread clients/gui-app/src/components/chat/chat-background-items-panel.tsx Outdated

@coderabbitai coderabbitai 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.

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 `@clients/gui-app/src/components/chat/chat-background-items-panel.tsx`:
- Around line 642-658: Update the stop-all disabled logic and stopAll handler so
each mechanism uses its own capability: keep the button enabled when
managedCommands can be stopped even if canAct is false, and invoke
props.onStopAll only when the chat harness is actionable. Preserve
managed-command mutation independently, and add coverage in
managed-command-chat-surfaces.test.tsx for canAct: false.

In
`@clients/gui-app/src/components/epic-canvas/renderers/__tests__/managed-command-output-tile.test.tsx`:
- Around line 264-292: Update the test helper timeline() to query the rendered
timeline with screen.getByRole("log") instead of its test ID, and keep the
existing font assertions unchanged while binding them to the exposed log
semantics.

In
`@clients/gui-app/src/hooks/managed-command/use-managed-command-lifecycle-mutations.ts`:
- Around line 145-167: Update useManagedCommandStopAll to resolve client once
before Promise.allSettled and fail immediately when unavailable. Preserve a
representative HostRpcError when every stop rejects for the same host error, and
normalize mixed failures to a HostRpcError while retaining the existing
failure-count message. Change onError to call toastFromHostError(error,
error.message).
🪄 Autofix

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: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: fb358d7e-5e11-4206-aaca-866af7c19270

📥 Commits

Reviewing files that changed from the base of the PR and between 802c8dd and b8ee4e2.

📒 Files selected for processing (41)
  • clients/gui-app/src/__tests__/acceptance/managed-command-s5-sidebar.test.tsx
  • clients/gui-app/src/__tests__/acceptance/managed-command-s6-s7-chat-doors.test.tsx
  • clients/gui-app/src/__tests__/acceptance/managed-command-s8-s9-tile-ref-resources.test.tsx
  • clients/gui-app/src/components/chat/__tests__/chat-background-items-panel.test.tsx
  • clients/gui-app/src/components/chat/__tests__/managed-command-chat-surfaces.test.tsx
  • clients/gui-app/src/components/chat/__tests__/queued-message-reorder-dnd.test.ts
  • clients/gui-app/src/components/chat/__tests__/queued-message-surface.test.tsx
  • clients/gui-app/src/components/chat/__tests__/queued-message-utils.test.ts
  • clients/gui-app/src/components/chat/chat-background-items-panel.tsx
  • clients/gui-app/src/components/chat/composer/composer-drag-attachment.ts
  • clients/gui-app/src/components/chat/managed-command-strip-rows.tsx
  • clients/gui-app/src/components/epic-canvas/__tests__/epic-sidebar.test.tsx
  • clients/gui-app/src/components/epic-canvas/__tests__/left-panel-registry.test.ts
  • clients/gui-app/src/components/epic-canvas/__tests__/root-dnd-commits.test.ts
  • clients/gui-app/src/components/epic-canvas/dnd/dnd.ts
  • clients/gui-app/src/components/epic-canvas/dnd/root-dnd-commits.ts
  • clients/gui-app/src/components/epic-canvas/renderers/__tests__/managed-command-output-tile.test.tsx
  • clients/gui-app/src/components/epic-canvas/renderers/chat-tile.tsx
  • clients/gui-app/src/components/epic-canvas/renderers/managed-command-output-tile.tsx
  • clients/gui-app/src/components/epic-canvas/renderers/terminal-tile-xterm.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/__tests__/managed-command-sidebar.test.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/epic-sidebar.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/left-panel-registry.ts
  • clients/gui-app/src/components/epic-canvas/sidebar/managed-command-sidebar.tsx
  • clients/gui-app/src/components/managed-commands/__tests__/managed-command-menu-drag-out.test.tsx
  • clients/gui-app/src/components/managed-commands/managed-command-chat-menu.tsx
  • clients/gui-app/src/components/settings/SETTINGS.md
  • clients/gui-app/src/components/settings/panels/appearance-settings-panel.tsx
  • clients/gui-app/src/hooks/managed-command/__tests__/use-managed-command-stop-all.test.tsx
  • clients/gui-app/src/hooks/managed-command/use-managed-command-lifecycle-mutations.ts
  • clients/gui-app/src/hooks/settings/use-effective-terminal-font.ts
  • clients/gui-app/src/lib/managed-commands/managed-command-copy.ts
  • clients/gui-app/src/lib/query-keys/managed-command-mutation-keys.ts
  • clients/gui-app/src/stores/chats/__tests__/chat-queue-reconciler.test.ts
  • clients/gui-app/src/stores/chats/__tests__/optimistic-queue.test.ts
  • clients/gui-app/src/stores/chats/__tests__/profile-durability-d1-queue-restamp.test.ts
  • clients/gui-app/src/stores/epics/__tests__/left-panel-store.test.ts
  • clients/gui-app/src/stores/epics/left-panel-store.ts
  • clients/gui-app/src/stores/managed-commands/managed-command-attention-store.ts
  • clients/gui-app/src/stores/managed-commands/managed-command-list-registry.ts
  • protocol/src/host/agent/gui/subscribe.ts
💤 Files with no reviewable changes (9)
  • clients/gui-app/src/components/epic-canvas/sidebar/managed-command-sidebar.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/epic-sidebar.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/left-panel-registry.ts
  • clients/gui-app/src/tests/acceptance/managed-command-s5-sidebar.test.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/tests/managed-command-sidebar.test.tsx
  • clients/gui-app/src/components/chat/managed-command-strip-rows.tsx
  • clients/gui-app/src/components/epic-canvas/tests/epic-sidebar.test.tsx
  • clients/gui-app/src/components/epic-canvas/tests/left-panel-registry.test.ts
  • clients/gui-app/src/components/epic-canvas/tests/root-dnd-commits.test.ts

Comment thread clients/gui-app/src/components/chat/chat-background-items-panel.tsx
Review round: Stop all now offers and sends each of its two halves on that
half's own capability, so a reconnecting chat keeps the one-click stop for
runaway monitors (the harness half is simply skipped); the aggregated stop
fails once with the real host error when no client can be built instead of
manufacturing N identical rejections; the output-tile timeline test binds to
the exposed log role rather than a test id.

Signed-off-by: Amiteshwar Randhawa <amiteshwar04@gmail.com>
@AmiteshwarRandhawa

Copy link
Copy Markdown
Contributor Author

Review feedback addressed in 60272aa:

  • Stop all per-half gating (Codex P2 + CodeRabbit): the button is offered when either half can act and each half is sent on its own capability — a reconnecting chat keeps the one-click stop for managed commands, and a press during the managed fan-out can no longer re-send the set. Button-level assertions added alongside the existing row-level test, including the disabled-only-when-neither-half-can-act case.
  • Stop-all hook fail-fast (CodeRabbit): when no host client can be built, the mutation throws the real hostClientUnavailableError once instead of manufacturing N identical rejections that collapse into an uninformative count.
  • Role query (CodeRabbit): the output-tile timeline test now binds to the exposed role="log".

Skipped: the toastFromHostError fallback-argument suggestion — the hook doesn't call that helper; the single-toast-with-message design stands.

🤖 Generated with Claude Code

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 60272aaf55

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread protocol/src/host/agent/gui/subscribe.ts

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
clients/gui-app/src/hooks/managed-command/use-managed-command-lifecycle-mutations.ts (1)

139-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a dynamic-client variant of useHostMutation

useHostMutation binds args.client at render time. It cannot select a client from variables.hostId. Extend the shared abstraction for per-mutation client resolution, then use it here. Preserve withHostMutationLifecycleBoundary and toastFromHostError instead of using toast.error.

🤖 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
`@clients/gui-app/src/hooks/managed-command/use-managed-command-lifecycle-mutations.ts`
around lines 139 - 158, Extend useHostMutation to resolve the HostClient from
mutation variables at execution time, while retaining
withHostMutationLifecycleBoundary and toastFromHostError behavior. Update the
managedCommand stopAll mutation to use this dynamic-client variant with
variables.hostId instead of manually constructing the client and handling its
unavailable error. Preserve the existing stop-all request and settled-outcome
behavior.

Sources: Coding guidelines, Learnings

🤖 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
`@clients/gui-app/src/components/chat/__tests__/managed-command-chat-surfaces.test.tsx`:
- Around line 584-585: Update the Stop all button assertions in the
managed-command chat surface tests to query the button by role with the
accessible name “Stop all” instead of using the background-stop-all test ID.
Assert its native disabled state through the HTMLButtonElement.disabled
property, including the corresponding assertion around the later test case.

---

Outside diff comments:
In
`@clients/gui-app/src/hooks/managed-command/use-managed-command-lifecycle-mutations.ts`:
- Around line 139-158: Extend useHostMutation to resolve the HostClient from
mutation variables at execution time, while retaining
withHostMutationLifecycleBoundary and toastFromHostError behavior. Update the
managedCommand stopAll mutation to use this dynamic-client variant with
variables.hostId instead of manually constructing the client and handling its
unavailable error. Preserve the existing stop-all request and settled-outcome
behavior.
🪄 Autofix

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: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8fbe031a-d28c-4f55-b8aa-726e7f96228d

📥 Commits

Reviewing files that changed from the base of the PR and between b8ee4e2 and 60272aa.

📒 Files selected for processing (4)
  • clients/gui-app/src/components/chat/__tests__/managed-command-chat-surfaces.test.tsx
  • clients/gui-app/src/components/chat/chat-background-items-panel.tsx
  • clients/gui-app/src/components/epic-canvas/renderers/__tests__/managed-command-output-tile.test.tsx
  • clients/gui-app/src/hooks/managed-command/use-managed-command-lifecycle-mutations.ts

… in tests

A managed-command chip in the new steering state locked its controls with no
visible reason - the label logic still assumed only pending|paused could
occur. The handover window now reads "Delivering", so the closed cancel
lever explains itself. The new Stop all assertions bind to the button's role
and accessible name per house guidelines.

Signed-off-by: Amiteshwar Randhawa <amiteshwar04@gmail.com>
@AmiteshwarRandhawa

Copy link
Copy Markdown
Contributor Author

Second round addressed in 4d466aa:

  • Steering chip state (Codex P2): a managed-command chip in the handover window now labels itself "Delivering" — the locked controls and closed cancel lever explain themselves. Test added asserting the label, aria-busy, and the absent cancel.
  • Role queries (CodeRabbit): the new Stop all assertions bind to the button's role and accessible name, asserting HTMLButtonElement.disabled.

🤖 Generated with Claude Code

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4d466aa3a8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…ments

Signed-off-by: Amiteshwar Randhawa <amiteshwar04@gmail.com>

# Conflicts:
#	clients/gui-app/src/__tests__/acceptance/managed-command-s5-sidebar.test.tsx
#	clients/gui-app/src/__tests__/acceptance/managed-command-s8-s9-tile-ref-resources.test.tsx
#	clients/gui-app/src/components/epic-canvas/sidebar/__tests__/managed-command-sidebar.test.tsx
The badge floated over the transcript's top-right corner, which put it
where the reading happens and gave it a plate of its own to stay legible
over live text. It now sits in the workspace-controls row under the
composer, next to the host, workspace and context-usage chips - the other
per-chat facts a person checks between turns - and wears their styling:
no plate, no border, dimmed until hovered, popover opening upward.

It rides the row's leading cell rather than becoming a third grid column:
the usage chip's pinned breakdown spans the row with `col-span-full`,
which only works while it is a direct child of ComposerWorkspaceRow's
two-column grid. `justify-between` parks the menu at that cell's trailing
edge, so it reads as the chip's left-hand neighbour and the workspace
selector gives up width first when the row is tight.

Presence rule is unchanged: nothing renders while the chat owns no
commands, and the menu never unmounts under an open popover.

Signed-off-by: Amiteshwar Randhawa <amiteshwar04@gmail.com>
b8ee4e2 taught BackgroundItemsPanel to read the tile's bound host and to
mint a managed-command "Stop all" mutation, but four suites that render it
still stood outside <TabHostProvider> or mocked the lifecycle-mutations
module without `useManagedCommandStopAll`; the chat-tile suites likewise
mocked the stream-runtime context without the support probe the monitors
menu makes. Each render threw where the panel used to be inert.

Signed-off-by: Amiteshwar Randhawa <amiteshwar04@gmail.com>
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@AmiteshwarRandhawa

Copy link
Copy Markdown
Contributor Author

Pushed 0353601 (with the main merge):

  • Placement change per product-owner review: the Monitors & Shells trigger moved from the floating top-right overlay into the workspace-controls row below the composer, left of the context-usage chip — in-flow, natural tab order, popover opens upward; the overlay wrapper and its minimap special-casing are deleted.
  • Merge of main resolved.
  • Fixed four suites that were red on the branch prior to the merge (renders missing the tab-host provider / mock exports the panel now requires). Full gui-app suite: 11,126 tests passing.

🤖 Generated with Claude Code

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 03536019f0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread clients/gui-app/src/components/managed-commands/managed-command-chat-menu.tsx Outdated

@coderabbitai coderabbitai 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.

Actionable comments posted: 5

🤖 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
`@clients/gui-app/src/components/chat/__tests__/managed-command-chat-surfaces.test.tsx`:
- Around line 611-615: Update the Stop Command assertion in the managed-command
chat surface test to cast the queried button to HTMLButtonElement and assert its
native disabled property, matching the neighboring Stop-all assertions; do not
inspect the raw disabled attribute.

In `@clients/gui-app/src/components/chat/queued-message-surface.tsx`:
- Around line 830-834: Update the pulse's assistive label in the queued-message
surface to derive from props.label instead of using the hardcoded “Steering
queued message” text, so it matches the visible “Delivering” state returned for
steering items by the status-label logic.

In
`@clients/gui-app/src/components/managed-commands/managed-command-chat-menu.tsx`:
- Around line 125-147: Update the managed-command menu trigger’s accessible name
to include clear attention and running count labels, preserving the existing
“Monitors and shells” context and handling zero counts appropriately. Mark the
rendered count text in ManagedCommandMenuBadge as aria-hidden so screen readers
announce the labeled counts only once.

In
`@clients/gui-app/src/hooks/managed-command/__tests__/use-managed-command-stop-all.test.tsx`:
- Around line 31-34: Add a test in the use-managed-command stop-all suite that
configures the mocked useHostDirectory().findById to return null, then verifies
no stop RPC is sent and exactly one unavailable-host error toast is emitted.
Preserve the existing mock behavior and assertions for available hosts.

In `@clients/gui-app/src/hooks/settings/use-effective-terminal-font.ts`:
- Line 25: Rename useEffectiveTerminalFont to follow the
use<Namespace><Verb><Noun> convention, such as useSettingsResolveTerminalFont,
and update every caller and import to use the new name consistently.
🪄 Autofix

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: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7795b220-264d-4a37-9bb3-ded05dcec15e

📥 Commits

Reviewing files that changed from the base of the PR and between 1cafd58 and 0353601.

📒 Files selected for processing (47)
  • clients/gui-app/src/__tests__/acceptance/managed-command-s5-sidebar.test.tsx
  • clients/gui-app/src/__tests__/acceptance/managed-command-s6-s7-chat-doors.test.tsx
  • clients/gui-app/src/__tests__/acceptance/managed-command-s8-s9-tile-ref-resources.test.tsx
  • clients/gui-app/src/components/chat/__tests__/chat-background-items-panel.test.tsx
  • clients/gui-app/src/components/chat/__tests__/chat-lower-dock.test.tsx
  • clients/gui-app/src/components/chat/__tests__/chat-scrollbar-composer-overlay.test.tsx
  • clients/gui-app/src/components/chat/__tests__/managed-command-chat-surfaces.test.tsx
  • clients/gui-app/src/components/chat/__tests__/queued-message-reorder-dnd.test.ts
  • clients/gui-app/src/components/chat/__tests__/queued-message-surface.test.tsx
  • clients/gui-app/src/components/chat/__tests__/queued-message-utils.test.ts
  • clients/gui-app/src/components/chat/chat-background-items-panel.tsx
  • clients/gui-app/src/components/chat/composer/composer-drag-attachment.ts
  • clients/gui-app/src/components/chat/managed-command-strip-rows.tsx
  • clients/gui-app/src/components/chat/queued-message-surface.tsx
  • clients/gui-app/src/components/epic-canvas/__tests__/chat-tile-queue-edit-steer.test.tsx
  • clients/gui-app/src/components/epic-canvas/__tests__/chat-tile.test.tsx
  • clients/gui-app/src/components/epic-canvas/__tests__/epic-sidebar.test.tsx
  • clients/gui-app/src/components/epic-canvas/__tests__/left-panel-registry.test.ts
  • clients/gui-app/src/components/epic-canvas/__tests__/root-dnd-commits.test.ts
  • clients/gui-app/src/components/epic-canvas/dnd/dnd.ts
  • clients/gui-app/src/components/epic-canvas/dnd/root-dnd-commits.ts
  • clients/gui-app/src/components/epic-canvas/renderers/__tests__/chat-lower-background-spacing.test.tsx
  • clients/gui-app/src/components/epic-canvas/renderers/__tests__/managed-command-output-tile.test.tsx
  • clients/gui-app/src/components/epic-canvas/renderers/chat-tile.tsx
  • clients/gui-app/src/components/epic-canvas/renderers/managed-command-output-tile.tsx
  • clients/gui-app/src/components/epic-canvas/renderers/terminal-tile-xterm.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/__tests__/managed-command-sidebar.test.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/epic-sidebar.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/left-panel-registry.ts
  • clients/gui-app/src/components/epic-canvas/sidebar/managed-command-sidebar.tsx
  • clients/gui-app/src/components/managed-commands/__tests__/managed-command-menu-drag-out.test.tsx
  • clients/gui-app/src/components/managed-commands/managed-command-chat-menu.tsx
  • clients/gui-app/src/components/settings/SETTINGS.md
  • clients/gui-app/src/components/settings/panels/appearance-settings-panel.tsx
  • clients/gui-app/src/hooks/managed-command/__tests__/use-managed-command-stop-all.test.tsx
  • clients/gui-app/src/hooks/managed-command/use-managed-command-lifecycle-mutations.ts
  • clients/gui-app/src/hooks/settings/use-effective-terminal-font.ts
  • clients/gui-app/src/lib/managed-commands/managed-command-copy.ts
  • clients/gui-app/src/lib/query-keys/managed-command-mutation-keys.ts
  • clients/gui-app/src/stores/chats/__tests__/chat-queue-reconciler.test.ts
  • clients/gui-app/src/stores/chats/__tests__/optimistic-queue.test.ts
  • clients/gui-app/src/stores/chats/__tests__/profile-durability-d1-queue-restamp.test.ts
  • clients/gui-app/src/stores/epics/__tests__/left-panel-store.test.ts
  • clients/gui-app/src/stores/epics/left-panel-store.ts
  • clients/gui-app/src/stores/managed-commands/managed-command-attention-store.ts
  • clients/gui-app/src/stores/managed-commands/managed-command-list-registry.ts
  • protocol/src/host/agent/gui/subscribe.ts
💤 Files with no reviewable changes (9)
  • clients/gui-app/src/components/epic-canvas/tests/left-panel-registry.test.ts
  • clients/gui-app/src/components/epic-canvas/sidebar/managed-command-sidebar.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/epic-sidebar.tsx
  • clients/gui-app/src/components/epic-canvas/tests/epic-sidebar.test.tsx
  • clients/gui-app/src/components/chat/managed-command-strip-rows.tsx
  • clients/gui-app/src/components/epic-canvas/tests/root-dnd-commits.test.ts
  • clients/gui-app/src/tests/acceptance/managed-command-s5-sidebar.test.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/tests/managed-command-sidebar.test.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/left-panel-registry.ts

Comment thread clients/gui-app/src/components/chat/queued-message-surface.tsx Outdated
Comment thread clients/gui-app/src/hooks/settings/use-effective-terminal-font.ts

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 696d3c8434

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread clients/gui-app/src/components/chat/chat-background-items-panel.tsx Outdated
Comment thread clients/gui-app/src/components/managed-commands/managed-command-chat-menu.tsx Outdated
…l rule

The delete confirmation survives its popover (same focus-outside guard as the
notifications bell); the popover caps to the viewport and scrolls; the
trigger's accessible name carries what its counts count; drag ids are unique
per mounted row so the same chat open twice cannot cross wires; endings that
arrive while the menu is open count as seen; the delivery pulse announces the
visible label; Stop all is one button with one in-flight state; plus
unavailable-host coverage and native disabled assertions.

Signed-off-by: Amiteshwar Randhawa <amiteshwar04@gmail.com>
@AmiteshwarRandhawa

Copy link
Copy Markdown
Contributor Author

Third round addressed in b46b38d:

  • Delete confirmation survives its popover (Codex): same focus-outside guard as the notifications bell — the portaled dialog no longer dismisses the menu out from under itself.
  • Popover capped to the viewport (Codex): max-height + scroll, so many finished shells can't push rows off screen.
  • Per-row drag ids (Codex): a useId occurrence key, same pattern as the active-agent rows, so the same chat open in two tiles can't cross drag wires.
  • Endings that arrive while the menu is open count as seen (Codex): acknowledgement now tracks what is rendered, not just the open transition; the re-arm test was split into seen-while-open and re-arm-after-close cases.
  • Stop all: one button, one in-flight state (Codex): disabled while anything it started is still running — no resubmitting the finished half.
  • CodeRabbit: pulse label follows the visible "Delivering" state; badge counts joined the trigger's accessible name (extracted helper, no nested ternary); native disabled assertion; unavailable-host coverage for the stop-all hook.

Skipped, with reasons: the useEffectiveTerminalFont rename — the use<Namespace><Verb><Noun> convention governs the RPC query/mutation hooks; this is a settings resolution and the proposed rename trades clarity for conformance.

🤖 Generated with Claude Code

coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 6, 2026

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9567177a9e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread clients/gui-app/src/components/managed-commands/managed-command-chat-menu.tsx Outdated
Comment thread clients/gui-app/src/components/chat/chat-background-items-panel.tsx Outdated
The same chat can be open in two canvas tiles, and each panel owned its own
mutation observer - the second tile's button stayed live while the first
tile's batch ran and could re-submit the same command ids. The button now
reads the batch's in-flight state through useIsMutating, shared across every
mounted panel.

Signed-off-by: Amiteshwar Randhawa <amiteshwar04@gmail.com>

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e5b8823928

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread clients/gui-app/src/components/chat/chat-background-items-panel.tsx
…h state

The shared pending read exists for the same chat open in two tiles - an
app-wide key let one chat's batch disable every other chat's button, so the
mutation key now carries the chat id. Per-row stops also go dead during the
batch: a row press mid-batch re-sent a stop the batch already carries.

Signed-off-by: Amiteshwar Randhawa <amiteshwar04@gmail.com>
Gated at the shared lifecycle action - which reads the batch state off the
command's own chat id - so the menu row, the output window, and the panel row
follow one rule instead of each surface re-implementing it; the panel's own
belt-and-suspenders hiding rule collapses away.

Signed-off-by: Amiteshwar Randhawa <amiteshwar04@gmail.com>

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 29b7149971

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread clients/gui-app/src/components/chat/queued-message-surface.tsx Outdated
A digest aimed at the running turn read identically to a next-turn one;
"Will deliver" - the delivery-vocabulary sibling of the prompt rows' "Will
steer" - tells the user the cancel window is the current turn.

Signed-off-by: Amiteshwar Randhawa <amiteshwar04@gmail.com>
coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 7, 2026

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b959be71ef

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@AmiteshwarRandhawa

Copy link
Copy Markdown
Contributor Author

On "keep the unsupported-host notice reachable" — deliberately not taken. A host too old for managedCommand.subscribeList predates the managed-command subsystem entirely (no agent tools either), so no monitors or shells can exist on it: the absent trigger is truthful, per the presence-equals-existence rule, and an always-rendered unavailable affordance would put a permanently dead button on every chat of every older host. The genuine reachable case — the menu held open across a swap to an older host — already renders the too-old notice inside the popover.

🤖 Generated with Claude Code

…m is gone

The standalone managedCommand.subscribeList existed to serve a global panel
that no longer exists; every consumer is chat-scoped. chat.subscribe snapshots
now carry managedCommands (default [], never null-checked - an old host and an
empty chat both truthfully read as none) and a dedicated whole-set
managedCommandsChanged frame carries updates, deliberately not riding
turnStateChanged because a command's lifecycle is not a turn transition.

Because chat.subscribe is tab-host-bound, a chat tile now reads its own
host's commands by construction - the cross-host defect where a tile bound to
one host rendered another host's list is unrepresentable. The list stream
mount, store, registry, factory override, stale-list plumbing and the menu's
now-unrepresentable host-too-old branch are deleted; the re-entry path for a
future epic-wide list is documented where the method lived.

Output-window tab titles narrow: with no epic-wide list, a window whose
owning chat has no live session falls back to its persisted name until one
opens.

Signed-off-by: Amiteshwar Randhawa <amiteshwar04@gmail.com>
@AmiteshwarRandhawa

Copy link
Copy Markdown
Contributor Author

Architecture change (product-owner directed): managed commands now ride the chat stream; the epic list stream is deleted (3a85baf here, be467ff339 internal).

managedCommand.subscribeList existed to serve a global sidebar panel that no longer exists; every remaining consumer is chat-scoped. chat.subscribe snapshots now carry managedCommands (.default([]) — an old host and an empty chat both truthfully read as none, so consumers never null-check and the menu's host-too-old branch is deleted as unrepresentable) plus a dedicated whole-set managedCommandsChanged frame. Both wire lines are unshipped, so no versioning ritual applies.

This resolves the cross-host P1 by construction: chat.subscribe is tab-host-bound, so a chat tile reads its own host's commands — the mixed shape (app-wide stream feeding a tile-bound control) no longer exists. The list mount/store/registry/factory and stale-list plumbing are deleted; a future global panel's re-entry path is documented where the method lived (the epic-activity dual-plane is the intended rail for cross-host ambient summaries).

Known narrowing: an output window whose owning chat has no live session falls back to its persisted tab name until one opens. Follow-ups noted for a later PR: an exhaustiveness guard on the chat-stream client's frame switch (its absence let a new frame kind be silently dropped — caught and probed during this migration), and the output tile's capability check reading the app-wide client (useStreamMethodSupportFor exists for the per-tab form).

🤖 Generated with Claude Code

coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 7, 2026

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3a85baf1d2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@AmiteshwarRandhawa

Copy link
Copy Markdown
Contributor Author

On "read resource stats from the tile-bound host" — valid, deferred to the cross-host follow-up alongside its two siblings (the output tile's capability check, and the chat-stream client exhaustiveness guard). The command list itself is now tab-host-bound by construction; the resource chips still join against the app-wide resources registry, so on the rare cross-host tile a chip goes ABSENT (a foreign command id misses the active host's snapshot) — which is the resources design's stated preference over a wrong or zero readout, and the readouts are opt-in. Re-scoping the resources stream per host reshapes a surface the app-level resources popover also rides, so it belongs to the resources work package, not this PR.

🤖 Generated with Claude Code

@AmiteshwarRandhawa
AmiteshwarRandhawa enabled auto-merge (squash) August 7, 2026 11:33

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f442aec693

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…r policy

A batch that failed entirely almost always failed for one systemic reason
(revoked access, host gone, unsupported method). The typed error is thrown and
rendered by the standard policy - recoverable-unauthorized suppression,
upgrade/reconnect guidance, dedup - exactly as the per-row path does; the
count summary stays for partial failures with mixed causes.

Signed-off-by: Amiteshwar Randhawa <amiteshwar04@gmail.com>

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 362c46d574

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@AmiteshwarRandhawa

Copy link
Copy Markdown
Contributor Author

On "key managed-command chat slices by host" — refuted after verification, no change. The premise ("cross-host clones retain the same epic/chat in tabs bound to different hosts") is not a reachable state: clone-not-migrate creates a SIBLING chat with a new id on the target host (clone-chat-on-host-switch.ts → openNewChatInActiveTile/createChat; its doc comment says exactly this), and chat tabs are host-bound for life, so (epicId, chatId) uniquely determines one host and the chat record itself carries that hostId. The registry's dispose-on-scope-mismatch handles sequential scope changes (user/transport rebind over time), not two concurrent hosts owning one chat id. The managed-commands slice inherits the same keying as the transcript, composer, and queue — if this premise were reachable, the entire chat surface would thrash, not just this selector.

🤖 Generated with Claude Code

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant