Repository navigation
feat(agents): restore the per-pane agent selector in the pane bar - #2276
Conversation
Restores the /development AISelector as the pane bar's chat-pane identity — a follow-up from the adversarial audit of PR #2258/#2259 (Dev → Agents Rebuild): the old machine grid could switch a pane's agent at any time via a dropdown in the bar; the pane-system restoration replaced that with a static conversation name, leaving the picker (Split-only) as the sole way to choose an agent. Zero data-model impact: panes are local, unaddressed UI state, and a session already owns many conversations with any of its drive's agents. Switching a pane's agent never rebinds a conversation (binding stays congenital and permanent) — it points the pane at a different conversation in the same session, focusing an existing one or minting a fresh one lazily. No schema change, no new API route. - select-pane-agent.ts: pure focus-or-mint decision (noop when the pick matches the pane's current agent, focus the session's most recently active conversation with that agent, or mint), with its own test file per the project's branching rule. - AISelector.tsx: accepts an optional pre-scoped `agents` list and a `canPickAssistant` flag. `driveId` alone can't express "no drive" — an absent driveId means "every drive" to usePageAgents — so a session-scoped pane (which may own no drive at all, in the global-assistant case) supplies its own list instead of the internal cross-drive fetch. - AgentPanes.tsx: chat panes render the selector, fed by the same pickableAgents already computed for the split picker; switching reuses the store's dedup-aware openConversation for an existing thread, or the picker's own mint path for a new one. Disabled while the pane's chat is streaming. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DfWmmVg3RqktRfAHtzEikF
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cea414ad70
ℹ️ 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".
| { revalidateOnFocus: false, refreshInterval: 20_000 }, | ||
| ); | ||
| const sessionConversations: SessionConversationSummary[] = useMemo( | ||
| () => (sessionsData?.sessions ?? []).find((session) => session.sessionId === sessionId)?.conversations ?? [], |
There was a problem hiding this comment.
On a cold mount or while this request is retrying, sessionsData is undefined and this normalization treats it as an authoritative empty conversation list. If the agent options are already cached, a user can use the selector before the session listing completes; selecting an agent that already has a thread then takes the mint path and persists a duplicate instead of resuming it. Preserve the loading/error distinction and disable switching or fetch on demand until the conversation list is available.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in #2278: the selector now stays disabled until sessionConversations has loaded at least once (tracked via the SWR hook's own isLoading), same treatment as the existing mid-mint/mid-stream disable. This PR (#2276) merged before this review finished, so the fix landed as a follow-up rather than a push here.
| }); | ||
| return; | ||
| } | ||
| void handlePickAgent(paneId, nextAgentPageId); |
There was a problem hiding this comment.
After this selector-driven mint succeeds, only the pane store is updated; sessionConversations still lacks the new row until the 20-second SWR refresh. In a one-pane session, switching from newly minted agent B back to existing agent A and then to B again within that interval reaches this handlePickAgent call a second time and persists another B conversation. Optimistically add the new summary or mutate/revalidate the session-list cache before allowing another selection.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in #2278: a successful mint now writes the new conversation straight into the sessionsData SWR cache locally (mutate(..., { revalidate: false })), so a switch-back-then-forward inside the 20s poll window resumes it instead of minting a duplicate. This PR (#2276) merged before this review finished, so the fix landed as a follow-up rather than a push here.
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
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 |
Restores the missing level in the pane grid's model: session -> conversation (agent listing) -> panes. Previously a session's pane grid was keyed per-session only, so closing the last pane bound to one agent's conversation was a pure layout act — its row stayed in the sidebar forever (and held a conversation-cap slot) unless the grid's actual last pane was closed, which ended the whole session instead. - packages/db/schema/conversations.ts: additive nullable closedInSessionAt timestamp, deliberately separate from isActive (history soft-delete) — closing a listing never touches history. - lib/agent-sessions/close-conversation-in-session.ts: pure decision module (closed | already_closed | not_in_session | last_conversation), fail-closed on the never-empty invariant. Wired in agent-sessions-runtime.ts with a per-session transaction + advisory lock (the #2272 createIfUnderLimit pattern) so two racing closes of the last two listings serialize. - New DELETE /api/agent-sessions/[sessionId]/conversations/[conversationId] route, session-scoped and uniform for page-agent + assistant threads. - components/agents/panes/close-pane.ts: pure client verdict (noop | close-pane | close-conversation | end-session) covering duplicate-view panes, an unloaded/stale listing, grid-last rebind to the most recently active other conversation, and ending the session only when its LAST open listing closes (even with terminal panes still in the grid). - AgentPanes.tsx wired to the verdict; cleanupOrphanedConversation now uses the session-scoped route (fixes a pre-existing defect where an orphaned mid-mint conversation stayed listed and held a cap slot forever). AgentsSurface/AgentPageView follow a close's rebind for their own independent "current conversation" tracking. Coordination: PR #2276 (pu/pane-agent-selector) also adds a sessions- listing SWR fetch and a pure panes/ module (select-pane-agent.ts, deciding *switch*; this PR's close-pane.ts decides *close* — no logical conflict). Whichever lands second should dedupe the listing fetch and share a SessionConversationSummary type. This fixes a directly-assigned bug report, not a filed GitHub issue — there is no issue number for this PR to close. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5FJqcnWyZXBmuc9iTFd6U
Restores the missing level in the pane grid's model: session -> conversation (agent listing) -> panes. Previously a session's pane grid was keyed per-session only, so closing the last pane bound to one agent's conversation was a pure layout act — its row stayed in the sidebar forever (and held a conversation-cap slot) unless the grid's actual last pane was closed, which ended the whole session instead. - packages/db/schema/conversations.ts: additive nullable closedInSessionAt timestamp, deliberately separate from isActive (history soft-delete) — closing a listing never touches history. - lib/agent-sessions/close-conversation-in-session.ts: pure decision module (closed | already_closed | not_in_session | last_conversation), fail-closed on the never-empty invariant. Wired in agent-sessions-runtime.ts with a per-session transaction + advisory lock (the #2272 createIfUnderLimit pattern) so two racing closes of the last two listings serialize. - New DELETE /api/agent-sessions/[sessionId]/conversations/[conversationId] route, session-scoped and uniform for page-agent + assistant threads. - components/agents/panes/close-pane.ts: pure client verdict (noop | close-pane | close-conversation | end-session) covering duplicate-view panes, an unloaded/stale listing, grid-last rebind to the most recently active other conversation, and ending the session only when its LAST open listing closes (even with terminal panes still in the grid). - AgentPanes.tsx wired to the verdict; cleanupOrphanedConversation now uses the session-scoped route (fixes a pre-existing defect where an orphaned mid-mint conversation stayed listed and held a cap slot forever). AgentsSurface/AgentPageView follow a close's rebind for their own independent "current conversation" tracking. Coordination: PR #2276 (pu/pane-agent-selector) also adds a sessions- listing SWR fetch and a pure panes/ module (select-pane-agent.ts, deciding *switch*; this PR's close-pane.ts decides *close* — no logical conflict). Whichever lands second should dedupe the listing fetch and share a SessionConversationSummary type. This fixes a directly-assigned bug report, not a filed GitHub issue — there is no issue number for this PR to close. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5FJqcnWyZXBmuc9iTFd6U
…owup fix(agents): close two pane-selector mint races flagged in #2276 review
Restores the missing level in the pane grid's model: session -> conversation (agent listing) -> panes. Previously a session's pane grid was keyed per-session only, so closing the last pane bound to one agent's conversation was a pure layout act — its row stayed in the sidebar forever (and held a conversation-cap slot) unless the grid's actual last pane was closed, which ended the whole session instead. - packages/db/schema/conversations.ts: additive nullable closedInSessionAt timestamp, deliberately separate from isActive (history soft-delete) — closing a listing never touches history. - lib/agent-sessions/close-conversation-in-session.ts: pure decision module (closed | already_closed | not_in_session | last_conversation), fail-closed on the never-empty invariant. Wired in agent-sessions-runtime.ts with a per-session transaction + advisory lock (the #2272 createIfUnderLimit pattern) so two racing closes of the last two listings serialize. - New DELETE /api/agent-sessions/[sessionId]/conversations/[conversationId] route, session-scoped and uniform for page-agent + assistant threads. - components/agents/panes/close-pane.ts: pure client verdict (noop | close-pane | close-conversation | end-session) covering duplicate-view panes, an unloaded/stale listing, grid-last rebind to the most recently active other conversation, and ending the session only when its LAST open listing closes (even with terminal panes still in the grid). - AgentPanes.tsx wired to the verdict; cleanupOrphanedConversation now uses the session-scoped route (fixes a pre-existing defect where an orphaned mid-mint conversation stayed listed and held a cap slot forever). AgentsSurface/AgentPageView follow a close's rebind for their own independent "current conversation" tracking. Coordination: PR #2276 (pu/pane-agent-selector) also adds a sessions- listing SWR fetch and a pure panes/ module (select-pane-agent.ts, deciding *switch*; this PR's close-pane.ts decides *close* — no logical conflict). Whichever lands second should dedupe the listing fetch and share a SessionConversationSummary type. This fixes a directly-assigned bug report, not a filed GitHub issue — there is no issue number for this PR to close. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5FJqcnWyZXBmuc9iTFd6U
Restores the missing level in the pane grid's model: session -> conversation (agent listing) -> panes. Previously a session's pane grid was keyed per-session only, so closing the last pane bound to one agent's conversation was a pure layout act — its row stayed in the sidebar forever (and held a conversation-cap slot) unless the grid's actual last pane was closed, which ended the whole session instead. - packages/db/schema/conversations.ts: additive nullable closedInSessionAt timestamp, deliberately separate from isActive (history soft-delete) — closing a listing never touches history. - lib/agent-sessions/close-conversation-in-session.ts: pure decision module (closed | already_closed | not_in_session | last_conversation), fail-closed on the never-empty invariant. Wired in agent-sessions-runtime.ts with a per-session transaction + advisory lock (the #2272 createIfUnderLimit pattern) so two racing closes of the last two listings serialize. - New DELETE /api/agent-sessions/[sessionId]/conversations/[conversationId] route, session-scoped and uniform for page-agent + assistant threads. - components/agents/panes/close-pane.ts: pure client verdict (noop | close-pane | close-conversation | end-session) covering duplicate-view panes, an unloaded/stale listing, grid-last rebind to the most recently active other conversation, and ending the session only when its LAST open listing closes (even with terminal panes still in the grid). - AgentPanes.tsx wired to the verdict; cleanupOrphanedConversation now uses the session-scoped route (fixes a pre-existing defect where an orphaned mid-mint conversation stayed listed and held a cap slot forever). AgentsSurface/AgentPageView follow a close's rebind for their own independent "current conversation" tracking. Coordination: PR #2276 (pu/pane-agent-selector) also adds a sessions- listing SWR fetch and a pure panes/ module (select-pane-agent.ts, deciding *switch*; this PR's close-pane.ts decides *close* — no logical conflict). Whichever lands second should dedupe the listing fetch and share a SessionConversationSummary type. This fixes a directly-assigned bug report, not a filed GitHub issue — there is no issue number for this PR to close. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5FJqcnWyZXBmuc9iTFd6U
1 participant
Summary
Restores the
/developmentAISelectoras the pane bar's chat-pane identity — an audit follow-up from PR #2258/#2259 (Dev → Agents Rebuild). The old machine grid'sMachinePaneChathosted anAISelectordropdown as the pane bar's own identity (selectedAgent={pane.selectedAgent} onSelectAgent={pane.selectAgent}, disabled while streaming), so a pane's agent could be switched at any time. The pane-system restoration ported the grid but replaced the bar's identity with a static conversation name — the only remaining agent choice was thePanePicker, reachable only via Split on an unbound pane.Zero data-model impact. Panes are local, unaddressed UI state; a session already owns many conversations with any of its drive's agents. Switching a pane's agent never rebinds a conversation — binding stays congenital and permanent — it just points the pane at a different conversation in the same session (minting one lazily if none exists yet), exactly the semantics the historical code documented: "returning to null resumes the conversation as-is, it never mints a new row." No schema change, no new API route.
select-pane-agent.ts(new, pure module + test): the switch decision — no-op when the pick matches the pane's current agent,focusthe session's most-recently-active conversation with the picked agent, ormintwhen none exists. MirrorsuseAgentWorkspaceStore'sopenConversationfocus-or-open policy, scoped to one specific pane.AISelector.tsx: adds an optional pre-scopedagentslist and acanPickAssistantflag, both additive and backward-compatible (existing callers —GlobalAssistantView,SidebarChatTab— are unaffected).driveIdalone can't express "no drive at all": an absentdriveIdmeans "every drive" to the internalusePageAgentsfetch, which would leak cross-drive agents into a global-assistant session's pane (a session that owns no drive to pick agents from). A session-scoped caller supplies its own already-correctly-scoped list instead.AgentPanes.tsx: chat panes now render the selector in the pane bar identity slot, fed by the samepickableAgentsalready computed for the split picker (so the bar and the picker never disagree about what's choosable). Switching reuses the store's dedup-awareopenConversationfor an existing thread, or the picker's own mint path (handlePickAgent) for a fresh one. Disabled while the pane's chat is streaming or mid-mint. The session's conversation list rides the same bulk/api/agent-sessionslisting the sidebar already polls (SWR dedupes the shared key) — no new endpoint.Side benefit: the bar now displays which agent each pane is talking to, closing the "which agent is this pane" gap the static conversation name left open.
Test plan
bun run typecheck(monorepo, via turbo)bun run lint(monorepo, via turbo) — only pre-existing warnings in unrelated filesbun run test:unit(monorepo) — all green except one pre-existing, unrelated, timezone-dependent flake (src/lib/messages/__tests__/grouping.test.ts, untouched by this branch)bun run knip:check— within baselineselect-pane-agent.test.ts,AISelector.test.tsx(agents override +canPickAssistant),AgentPanes.test.tsx(selector renders current agent, switch focuses an existing conversation, switch mints when none exists, disabled while streaming, no-op on same agent)bun run dev→ Agents console Chat tab → pane bar shows the agent dropdown → switch to another drive agent → pane swaps to that agent's conversation (existing one focused if present, else a new thread appears in the sidebar under the same session); terminal pane bar unchanged; selector disabled mid-stream🤖 Generated with Claude Code
https://claude.ai/code/session_01DfWmmVg3RqktRfAHtzEikF