Repository navigation
feat(agents): tabbable panes, fix agent-switch sidebar desync - #2304
Conversation
Switching a pane's agent used to mint a new conversation without closing the old one, leaving a stray row in the sidebar every time. Switching now replaces the pane's active tab and closes the outgoing conversation (unless it's still shown as another tab or in another pane). Formalizes the accidental "tabs" this produced into a real feature: - Pane header gains a conversation tab strip + "+" to deliberately open a second tab, reusing an existing conversation before minting. - Pane grid layout and tabs are now server-persisted (agent_sessions.workspaceState), synced via a debounced hook so a refresh or a different device restores the same panes — an earlier version of this system synced eagerly and was torn down for making every split a write, so this one debounces instead. - AgentsSidebar now renders Session -> Pane -> Tabs (one row per pane, updating in place) instead of a flat conversation list, falling back to the old flat list for any session with no saved grid. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XRLJTmce8guxm1H4ZsFcYp
|
Warning Review limit reached
Next review available in: 54 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 (2)
📝 WalkthroughWalkthroughAdds persisted agent-session workspaces with pane tabs, authenticated workspace APIs, server synchronization, tab-aware pane lifecycle behavior, and sidebar rendering of saved panes and background tabs. ChangesPersisted agent workspace
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant User
participant AgentPanes
participant AgentWorkspaceStore
participant WorkspaceAPI
participant AgentSessionDB
User->>AgentPanes: Open, switch, or close conversation tab
AgentPanes->>AgentWorkspaceStore: Apply tab transition
AgentWorkspaceStore-->>AgentPanes: Updated pane workspace
AgentPanes->>WorkspaceAPI: Debounced PUT workspace
WorkspaceAPI->>AgentSessionDB: Save workspaceState
AgentSessionDB-->>WorkspaceAPI: Save result
WorkspaceAPI-->>AgentPanes: Success or error response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![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: 37f39c3915
ℹ️ 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".
| const current = useAgentWorkspaceStore.getState().workspaces[sessionId]; | ||
| if (current !== capturedWorkspace) return; // something local already moved on — respect it |
There was a problem hiding this comment.
On every normal AgentPanes mount, the later openConversation effect seeds or selects the initial conversation while this GET is in flight; even selecting an already-active pane creates a new workspace object. Consequently current !== capturedWorkspace is always true before a network response can arrive, so the saved server grid is never hydrated and cross-device restoration is effectively disabled. Distinguish the component's initial seeding from a user layout mutation, or hydrate before applying that selection.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b954ea1. Root cause confirmed: pane-reducer.ts's selectPane always returned a new object even when re-selecting the already-active pane, so the mount-time openConversation seed effect (which runs in the same commit as this hook's GET is kicked off) always broke the current !== capturedWorkspace reference-equality guard. Two changes: (1) selectPane now short-circuits to the same reference when the target is already active; (2) removed the guard entirely — hydration now applies unconditionally on first load, matching the file header's own documented intent ("server response wins over any stale localStorage seed"). Covered by an updated test in useWorkspaceServerSync.test.ts and a new reference-stability test in pane-reducer.test.ts.
| export const persistedPaneStateSchema = z.object({ | ||
| id: z.string().min(1), | ||
| scope: paneScopeSchema.nullable(), | ||
| tabs: z.array(paneScopeSchema).default([]), |
There was a problem hiding this comment.
Existing localStorage grids were written without tabs; parsing them with this default produces chat panes whose active scope points at a conversation but whose tab list is empty, violating the invariant documented immediately above. For upgraded users, tab-aware close logic sees no conversation IDs, so closing a non-last pane skips the session-scoped conversation DELETE and leaves an orphaned listing, while opening another tab can discard the old active conversation from the tab model. The migration should initialize a legacy chat pane's tabs from its resolved scope, not unconditionally to [].
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b954ea1. persistedPaneStateSchema now has a .transform() that backfills tabs from scope when scope.kind === 'chat' and tabs is empty (covers both "key missing entirely" and "explicit empty array"). Updated persisted-workspace.test.ts's existing backfill test (it was asserting the old, incorrect tabs: [] result) and added two new contract-level tests.
| <button | ||
| type="button" | ||
| className="flex min-w-0 flex-1 items-center text-left" | ||
| onClick={() => onOpenConversation(entry)} |
There was a problem hiding this comment.
When a nested background-tab row is clicked, this generic conversation selection eventually invokes AgentPanes.openConversation, whose lookup only recognizes active pane scopes rather than background tabs. The target therefore is not found as already shown and the active pane is reassigned; in a pane with tabs [A, B] and active A, clicking the sidebar row for B can replace A with another B instead of simply activating the existing B tab. Carry the pane identity through this click and invoke the workspace's switchTab operation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b954ea1. Background-tab clicks in PaneRow now call the workspace store's switchTab(sessionId, paneId, targetId) directly (pane-local, synchronous) before the existing onOpenConversation selection call, instead of relying solely on the session-wide openConversation/focusOrAssignScope path, whose paneShowing check only looks at a pane's active scope and never its tabs. Added a test that seeds the real workspace store and asserts the clicked pane's scope moves to the target while both tabs stay open.
| { | ||
| label: 'Close', | ||
| icon: X, | ||
| onSelect: () => void onCloseConversation(pane.scope.targetId), |
There was a problem hiding this comment.
The new pane/tab Close menu calls the old session-listing DELETE directly and never removes the target from useAgentWorkspaceStore. Thus the persisted workspace continues rendering a row and pane for a conversation the server now considers closed; moreover, if the same conversation is tabbed in another pane, this bypasses decideCloseTab's shown-elsewhere check and closes the shared server listing out from under that other pane. This action needs to use the same tab-aware close path as the pane header so it performs a local-only removal for duplicate views and updates every affected workspace tab after a successful DELETE.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b954ea1. PaneRow's Close menu (both the active-tab row and nested background-tab rows) now routes through a new closePaneTab handler in SessionRow that runs the same decideCloseTab decision AgentPanes.tsx's own close control uses: shown-elsewhere closes locally only (no DELETE), otherwise DELETEs and then calls the store's closeTab to remove it from the persisted grid. Added two tests: one confirming the local grid is updated after a successful close, one confirming a tab shown in another pane is removed locally without ever calling DELETE.
| await fetchWithAuth(workspaceUrl(sessionId), { | ||
| method: 'PUT', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ workspace: toSave }), | ||
| }); | ||
| // Only clear pending if no newer layout arrived while this save was in-flight. | ||
| if (pendingRef.current === toSave) pendingRef.current = null; |
There was a problem hiding this comment.
fetchWithAuth resolves with a Response for HTTP failures, so a 400, 403, or 502 reaches this line and clears pendingRef as though the layout were saved; only network-level rejection enters the catch block. If the user makes no subsequent mutation, or then unmounts, the latest layout is permanently absent from the server with no retry or warning. Check response.ok before clearing the pending snapshot.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b954ea1, together with the duplicate finding on this same function (round 2 review, line 74) — performSave now checks response.ok before clearing pendingRef, so a 400/403/502 leaves the save pending for the next mutation or the unmount flush to retry. Added a test asserting a non-2xx response keeps the save pending and gets flushed on unmount.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (13)
apps/web/src/app/api/agent-sessions/[sessionId]/workspace/route.ts (2)
69-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the validation error body with the repository convention.
This route returns
parsed.error.issuesverbatim. Other API routes returnvalidation.error.flatten().fieldErrors. Use the same shape so clients see one error contract, and so the planned repository-wide migration toz.flattenError()covers this route too.♻️ Proposed change
const parsed = persistedWorkspaceStateSchema.safeParse(body.workspace); if (!parsed.success) { - return NextResponse.json({ error: 'Invalid workspace payload', issues: parsed.error.issues }, { status: 400 }); + return NextResponse.json( + { error: 'Invalid workspace payload', details: parsed.error.flatten().fieldErrors }, + { status: 400 }, + ); }Based on learnings: "Maintain the existing pattern of using validation.error.flatten().fieldErrors across API routes."
🤖 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/api/agent-sessions/`[sessionId]/workspace/route.ts around lines 69 - 72, Update the validation failure response in the workspace route around persistedWorkspaceStateSchema.safeParse to return parsed.error.flatten().fieldErrors instead of parsed.error.issues, preserving the existing 400 status and error message while matching the repository-wide API error shape.Source: Learnings
62-79: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBound the accepted workspace payload.
The handler parses the body and writes it into a
jsonbcolumn with no size or cardinality limit.persistedWorkspaceStateSchemasets no.max()oncolumns,panes, ortabs. An authenticated client can therefore persist an arbitrarily large grid, which inflates the row, the sessions collection response, and every sidebar poll that reads it.Add upper bounds in the shared schema so both the API route and the client hydration path reject oversized grids.
🛡️ Suggested bounds in `packages/lib/src/agent-sessions/contract.ts`
export const persistedPaneStateSchema = z.object({ id: z.string().min(1), scope: paneScopeSchema.nullable(), - tabs: z.array(paneScopeSchema).default([]), + tabs: z.array(paneScopeSchema).max(50).default([]), }); export const persistedColumnStateSchema = z.object({ id: z.string().min(1), - panes: z.array(persistedPaneStateSchema).min(1), + panes: z.array(persistedPaneStateSchema).min(1).max(20), }); export const persistedWorkspaceStateSchema = z.object({ id: z.string().min(1), - columns: z.array(persistedColumnStateSchema).min(1), + columns: z.array(persistedColumnStateSchema).min(1).max(20), activePaneId: z.string().min(1), pendingPickerPaneId: z.string().nullable(), });Choose limits above any layout the UI can produce, so a legitimate grid never fails validation.
🤖 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/api/agent-sessions/`[sessionId]/workspace/route.ts around lines 62 - 79, Update the shared persistedWorkspaceStateSchema in the agent-session contract to apply maximum bounds to the columns, panes, and tabs collections, choosing limits above the largest layout the UI can produce. Keep the schema shared so both the workspace API validation and client hydration path reject oversized grids before persistence or rendering.apps/web/src/stores/agent-workspace/__tests__/useAgentWorkspaceStore.test.ts (1)
461-468: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for closing the last tab.
closeTabis documented to revert the pane to the picker when the closed tab was the only one, andpane-reducer.closeTabsetspendingPickerPaneIdin that branch. No test exercises it. Add a case so a regression in that branch fails the suite.💚 Proposed test
it('closeTab on the last tab unbinds the pane and re-opens the picker', () => { store().ensureWorkspace('ses-1', scope()); const paneId = grid().activePaneId; store().closeTab('ses-1', paneId, 'conv-1'); expect(panesOf(grid())[0].tabs).toEqual([]); expect(panesOf(grid())[0].scope).toBeNull(); expect(grid().pendingPickerPaneId).toBe(paneId); });🤖 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/stores/agent-workspace/__tests__/useAgentWorkspaceStore.test.ts` around lines 461 - 468, Add a test alongside the existing closeTab coverage for closing the pane’s only tab. Exercise store().closeTab with the initial conversation, then assert the pane has no tabs, its scope is null, and grid().pendingPickerPaneId equals the closed pane ID.packages/lib/src/agent-sessions/contract.ts (1)
124-128: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider narrowing
tabsto addressable chat scopes.
paneScopeSchemaallowstargetId: nulland anykind. The reducer addresses tabs only bytargetId(switchTab,closeTabcomparetab.targetId === targetId), and the doc comment states that only chat panes tab. A persisted tab withtargetId: nullorkind: 'terminal'would therefore be unclosable through the tab strip.♻️ Optional tightening of the tab element schema
+const paneTabSchema = paneScopeSchema.extend({ + kind: z.literal('chat'), + targetId: z.string().min(1), +}); + export const persistedPaneStateSchema = z.object({ id: z.string().min(1), scope: paneScopeSchema.nullable(), - tabs: z.array(paneScopeSchema).default([]), + tabs: z.array(paneTabSchema).default([]), });Note: this changes the accepted payload shape, so confirm no existing client writes non-chat tabs before applying it.
🤖 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 `@packages/lib/src/agent-sessions/contract.ts` around lines 124 - 128, Update persistedPaneStateSchema so tabs accepts only addressable chat pane scopes: require a non-null targetId and restrict kind to the chat value used by the pane model. Preserve paneScopeSchema for scope itself, and verify existing clients do not persist terminal or targetless tabs before enforcing the narrower schema.apps/web/src/stores/agent-workspace/__tests__/useWorkspaceServerSync.test.ts (1)
32-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd cases for a rejected save and for an
enabledtransition.Every mocked response uses
ok: true, so the suite never exercises a failed PUT. Two gaps follow from that:
- No test asserts what happens when the server rejects the payload. Add a case where the PUT resolves with
{ ok: false, status: 400 }and assert that the pending grid is retried (for example, that the unmount flush still fires a PUT). This case fails today, which matches the response-status issue raised onuseWorkspaceServerSync.ts.- No test covers
enabledchanging fromtruetofalsewith a pending save. Add a case that rerenders the hook with{ enabled: false }and asserts the pending grid is flushed.💚 Sketch for the rejected-save case
it('keeps the pending grid when the server rejects the save', async () => { store().ensureWorkspace('ses-1', scope()); const { unmount } = renderHook(() => useWorkspaceServerSync('ses-1', { debounceMs: 10 })); mockFetch.mockResolvedValue({ ok: false, status: 400, json: () => Promise.resolve({}) }); act(() => { store().splitRight('ses-1', grid().activePaneId); }); await new Promise((r) => setTimeout(r, 20)); mockFetch.mockClear(); unmount(); await new Promise((r) => setTimeout(r, 0)); expect(mockFetch.mock.calls.filter((c) => c[1]?.method === 'PUT')).toHaveLength(1); });Also applies to: 111-139
🤖 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/stores/agent-workspace/__tests__/useWorkspaceServerSync.test.ts` around lines 32 - 38, Add tests in useWorkspaceServerSync.test.ts for both missing behaviors: verify a PUT resolving with ok: false and status 400 retains the pending grid so unmount triggers another PUT, and verify rerendering the hook from enabled: true to enabled: false flushes a pending save. Reuse the existing store setup, renderHook helpers, and mockFetch assertions.apps/web/src/lib/agent-sessions/__tests__/session-workspace-runtime.test.ts (1)
34-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReset the queued responses between tests.
vi.clearAllMocks()clears call history but leaves the module-levelresponsesarray intact. If a future test queues a response that its assertion path never consumes, the leftover row leaks into the next test and makes the suite order-dependent. Export a reset hook and call it inbeforeEach.♻️ Proposed reset hook
- return { db, __queueResponse: (rows: unknown[]) => responses.push(rows) }; + return { + db, + __queueResponse: (rows: unknown[]) => responses.push(rows), + __resetResponses: () => { responses.length = 0; }, + }; });beforeEach(() => { vi.clearAllMocks(); + (dbModule as unknown as { __resetResponses(): void }).__resetResponses(); });Also applies to: 62-64
🤖 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/lib/agent-sessions/__tests__/session-workspace-runtime.test.ts` around lines 34 - 35, Expose a reset hook alongside __queueResponse that clears the module-level responses array, then invoke that hook in beforeEach together with vi.clearAllMocks(). Ensure each test starts with no queued database responses, including the setup covered by the repeated lines.apps/web/src/lib/agent-sessions/session-workspace-runtime.ts (1)
52-61: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConsider scoping the write by owner as well as by id.
saveSessionWorkspaceupdates bysessionIdalone. The route callscheckSessionAccessfirst, so the current call path is authorized. Adding the owner to the predicate makes the helper safe against a future caller that forgets the access check, and it costs nothing on the existing index path.🛡️ Optional defense-in-depth predicate
export async function saveSessionWorkspace(input: { sessionId: string; + ownerId?: string; workspace: PersistedWorkspaceState; }): Promise<void> { await db .update(agentSessions) .set({ workspaceState: input.workspace }) - .where(eq(agentSessions.id, input.sessionId)); + .where( + input.ownerId + ? and(eq(agentSessions.id, input.sessionId), eq(agentSessions.ownerId, input.ownerId)) + : eq(agentSessions.id, input.sessionId), + ); }Note that a shared session may be writable by a non-owner, so verify the access model before making
ownerIdrequired.🤖 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/lib/agent-sessions/session-workspace-runtime.ts` around lines 52 - 61, Update saveSessionWorkspace to scope the database update by both sessionId and the session owner, reusing the existing session ownership/access model and preserving writes for authorized shared-session users. Do not require ownerId if that would prevent non-owner shared-session writes; instead use the appropriate existing ownership predicate or access-compatible condition.apps/web/src/stores/agent-workspace/useWorkspaceServerSync.ts (1)
77-91: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
justHydratedRefdepends on the store applying every hydration write.The flag is set before
hydrateWorkspace, and only the resulting effect run clears it. IfhydrateWorkspaceever becomes conditional and no-ops, no store change follows, the effect does not run, and the flag staystrue. The next genuine mutation would then be skipped from persistence.Clear the flag from the value it guards instead of relying on the next effect run — for example, compare the observed
workspaceagainst the hydrated object by reference.♻️ Suggested hardening
- const justHydratedRef = useRef(false); + // The exact object hydration wrote; only that value is skipped. + const hydratedValueRef = useRef<WorkspaceState | null>(null);if (!enabled || !workspace) return; - if (justHydratedRef.current) { - justHydratedRef.current = false; + if (hydratedValueRef.current === workspace) { + hydratedValueRef.current = null; return; }Set
hydratedValueRef.current = parsed.data;in place ofjustHydratedRef.current = true;.🤖 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/stores/agent-workspace/useWorkspaceServerSync.ts` around lines 77 - 91, Update the hydration tracking around the workspace sync effect to store the hydrated workspace object in a ref instead of setting justHydratedRef. In the effect containing performSave, compare the observed workspace against that stored hydrated value by reference and clear the ref when matched, skipping persistence only for that exact hydrated object; ensure conditional or no-op hydration cannot cause the next genuine mutation to be skipped.apps/web/src/stores/agent-workspace/pane-reducer.ts (1)
165-175: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider rejecting a non-chat
newScopeinreplaceTab.
assignPaneclearstabsfor a non-chat scope, butreplaceTabpasses any scope straight intotabsForChatAssign. A terminal or page scope would then be appended totabs, which contradicts thePaneState.tabsdocblock ("Always[]for a terminal/page pane"). Every current caller passeskind: 'chat', so this is defensive only.♻️ Proposed guard
const pane = state.columns[location.columnIndex].panes[location.paneIndex]; + if (newScope.kind !== 'chat') return withPaneScopeAndTabs(state, paneId, newScope, []); return withPaneScopeAndTabs(state, paneId, newScope, tabsForChatAssign(pane.tabs, oldTargetId, newScope));🤖 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/stores/agent-workspace/pane-reducer.ts` around lines 165 - 175, Update replaceTab to defensively reject non-chat newScope values before calling tabsForChatAssign, returning the unchanged state for terminal or page scopes. Preserve the existing chat replacement behavior and ensure PaneState.tabs remains empty for non-chat panes.apps/web/src/app/api/agent-sessions/route.ts (1)
92-100: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the bulk-query comment to three queries.
The comment states "TWO bulk queries" and "1+2N", but the code now issues three bulk queries. The comment describes a poll-cost invariant, so keep the count accurate.
♻️ Proposed comment update
- // Children in TWO bulk queries, however many sessions listed — this is - // polled by every open sidebar, and the per-session shape was 1+2N - // queries per poll (review M4). + // Children in THREE bulk queries, however many sessions listed — this is + // polled by every open sidebar, and the per-session shape would be 1+3N + // queries per poll (review M4).🤖 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/api/agent-sessions/route.ts` around lines 92 - 100, Update the comment above the bulk Promise.all call to state that children are fetched in THREE bulk queries and that the previous per-session cost was 1+2N queries, while preserving the existing poll-cost context. Keep the code and query behavior unchanged.apps/web/src/components/layout/left-sidebar/__tests__/AgentsSidebar.test.tsx (1)
134-135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the
workspacefixture against the persisted contract.
workspace?: unknownremoves all shape checking fromworkspaceFixtureandsingleTabWorkspace. This cohort's whole risk is the persisted workspace contract. IfpersistedWorkspaceStateSchemagains or renames a field, these fixtures keep compiling and the tests keep asserting against a shape the sidebar no longer receives.♻️ Proposed typing
- /** Omitted (undefined) in most fixtures — the sidebar reads that as "fall back to the flat conversation list above." */ - workspace?: unknown; + /** Omitted (undefined) in most fixtures — the sidebar reads that as "fall back to the flat conversation list above." */ + workspace?: PersistedWorkspaceState;Add the import:
import type { PersistedWorkspaceState } from '`@pagespace/lib/agent-sessions/contract`';🤖 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/components/layout/left-sidebar/__tests__/AgentsSidebar.test.tsx` around lines 134 - 135, Replace the `workspace?: unknown` fixture field with the persisted contract type by importing `PersistedWorkspaceState` from `@pagespace/lib/agent-sessions/contract` and declaring `workspace?: PersistedWorkspaceState`. Apply this typing to the shared fixture definitions, including `workspaceFixture` and `singleTabWorkspace`, so contract changes are caught by compilation.apps/web/src/components/layout/left-sidebar/AgentsSidebar.tsx (1)
1035-1035: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the
as stringassertion with a type predicate in the filter.
backgroundTabsalready excludestargetId === null, but the narrowing is lost, so line 1066 needstab.targetId as string. A predicate on the filter keeps TypeScript strict and removes the assertion.♻️ Proposed refactor
- const backgroundTabs = pane.tabs.filter((tab) => tab.targetId !== null && tab.targetId !== pane.scope.targetId); + const backgroundTabs = pane.tabs.filter( + (tab): tab is (typeof pane.tabs)[number] & { targetId: string } => + tab.targetId !== null && tab.targetId !== pane.scope.targetId, + );{backgroundTabs.map((tab) => { - const targetId = tab.targetId as string; + const targetId = tab.targetId; const entry = conversationEntryForTarget(targetId, tab.agentPageId);Also applies to: 1063-1067
🤖 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/components/layout/left-sidebar/AgentsSidebar.tsx` at line 1035, Update the backgroundTabs filter to use a TypeScript type predicate that narrows tab.targetId to string while excluding null and the current pane scope target. Then remove the unnecessary “as string” assertion in the related lines around the background tab handling, preserving the existing filtering behavior.Source: Coding guidelines
apps/web/src/components/agents/panes/AgentPanes.tsx (1)
1876-1918: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReconsider the
tablist/tabroles on the conversation tab strip.The element with
role="tab"is adiv. It is not focusable, it has noaria-controls, and it contains two nestedbuttonelements. Assistive technology expects atabelement to be focusable itself and to own the tab semantics. Nested buttons inside atabelement are not announced as tabs, soaria-selectednever reaches the control the user actually focuses.Two workable shapes:
- Make the chip button itself carry
role="tab"andaria-selected, and place the close button as a sibling outside the tab element.- Drop the ARIA tab pattern and use plain buttons with
aria-current="true"for the active conversation.The second option is simpler and matches what the strip does today (it switches the pane's active conversation, it does not toggle sibling panels).
♻️ Option: plain buttons with `aria-current`
- <div role="tablist" aria-label="Open conversations" className="flex min-w-0 items-center gap-0.5 overflow-x-auto pt-0.5"> + <div aria-label="Open conversations" className="flex min-w-0 items-center gap-0.5 overflow-x-auto pt-0.5"> {tabs.map((tab) => { if (tab.targetId === null) return null; const targetId = tab.targetId; const isActive = targetId === activeTargetId; const label = paneTabAgentName(tab.agentPageId, pickableAgents); return ( <div key={targetId} - role="tab" - aria-selected={isActive} className={cn( 'group/tab flex shrink-0 items-center gap-1 rounded px-1.5 py-0.5 text-[10px]', isActive ? 'bg-accent text-foreground' : 'text-muted-foreground hover:bg-accent/60', )} > <button type="button" title={label} aria-label={`Switch to ${label}`} + aria-current={isActive ? 'true' : undefined} onClick={(e) => {🤖 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/components/agents/panes/AgentPanes.tsx` around lines 1876 - 1918, Replace the tablist/tab ARIA pattern in the conversation strip with plain button semantics: remove role="tablist", role="tab", and aria-selected from the surrounding elements, make the conversation-switching button the primary control, and mark the active one with aria-current="true". Keep the separate close button and existing selection, closing, labels, and styling behavior unchanged.
🤖 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/src/components/agents/panes/__tests__/open-pane-tab.test.ts`:
- Around line 53-67: Correct the test title in the decideOpenTab test so it
describes focusing the existing matching tab rather than opening a second
separate tab. Keep the assertion and inline comment unchanged.
In `@apps/web/src/components/agents/panes/AgentPanes.tsx`:
- Around line 803-811: Update the History “New Conversation” call site in
onCreateNewFromHistory around handlePickAgent so it passes mode: 'append' when
minting a thread for the same agent, preventing the pane’s previous conversation
from being closed or deleted. Preserve the default replace behavior for actual
agent switches.
In `@apps/web/src/lib/agent-sessions/session-workspace-runtime.ts`:
- Around line 21-28: Update getSessionWorkspace to validate row?.workspaceState
with persistedWorkspaceStateSchema.safeParse(), returning the parsed data only
when validation succeeds and null for missing or malformed workspace states.
Remove the direct PersistedWorkspaceState cast so invalid jsonb rows are
excluded from the list response.
In
`@apps/web/src/stores/agent-workspace/__tests__/useAgentWorkspaceStore.test.ts`:
- Around line 444-450: Rename the test describing openTab in the workspace store
tests so it states that appending a tab also activates the new scope. Keep the
existing assertions and test behavior unchanged.
In `@apps/web/src/stores/agent-workspace/useAgentWorkspaceStore.ts`:
- Around line 112-113: Update the doc comment for switchTab in the agent
workspace store to remove the inaccurate “local only, no network” claim and
describe that switching the tab updates workspace state, which is persisted
through the debounced workspace sync.
- Around line 312-313: Reject workspace payloads whose parsed id differs from
the sessionId route parameter in the PUT handler using
persistedWorkspaceStateSchema validation, returning a 400 response before
persistence. Also update hydrateWorkspace in useAgentWorkspaceStore to ignore or
reject workspaces whose id does not match the target sessionId, preserving the
session-key invariant.
In `@apps/web/src/stores/agent-workspace/useWorkspaceServerSync.ts`:
- Around line 123-140: Update the force-flush useEffect cleanup to include
enabled in its dependency array, preserving the existing pendingRef flush
behavior when enabled changes and when the component unmounts or
sessionId/syncId changes.
- Around line 56-74: Update performSave to retain the fetchWithAuth response and
validate res.ok before clearing pendingRef. Treat non-2xx responses as failed
saves so the current toSave remains pending for unmount force-flushes and
subsequent mutations, while preserving the existing cleanup through
endEditing(syncId).
In `@packages/lib/src/agent-sessions/contract.ts`:
- Around line 131-143: Update pendingPickerPaneId in
persistedWorkspaceStateSchema to use the same non-empty string validation as
activePaneId while retaining nullable support, so empty picker IDs are rejected.
---
Nitpick comments:
In `@apps/web/src/app/api/agent-sessions/`[sessionId]/workspace/route.ts:
- Around line 69-72: Update the validation failure response in the workspace
route around persistedWorkspaceStateSchema.safeParse to return
parsed.error.flatten().fieldErrors instead of parsed.error.issues, preserving
the existing 400 status and error message while matching the repository-wide API
error shape.
- Around line 62-79: Update the shared persistedWorkspaceStateSchema in the
agent-session contract to apply maximum bounds to the columns, panes, and tabs
collections, choosing limits above the largest layout the UI can produce. Keep
the schema shared so both the workspace API validation and client hydration path
reject oversized grids before persistence or rendering.
In `@apps/web/src/app/api/agent-sessions/route.ts`:
- Around line 92-100: Update the comment above the bulk Promise.all call to
state that children are fetched in THREE bulk queries and that the previous
per-session cost was 1+2N queries, while preserving the existing poll-cost
context. Keep the code and query behavior unchanged.
In `@apps/web/src/components/agents/panes/AgentPanes.tsx`:
- Around line 1876-1918: Replace the tablist/tab ARIA pattern in the
conversation strip with plain button semantics: remove role="tablist",
role="tab", and aria-selected from the surrounding elements, make the
conversation-switching button the primary control, and mark the active one with
aria-current="true". Keep the separate close button and existing selection,
closing, labels, and styling behavior unchanged.
In
`@apps/web/src/components/layout/left-sidebar/__tests__/AgentsSidebar.test.tsx`:
- Around line 134-135: Replace the `workspace?: unknown` fixture field with the
persisted contract type by importing `PersistedWorkspaceState` from
`@pagespace/lib/agent-sessions/contract` and declaring `workspace?:
PersistedWorkspaceState`. Apply this typing to the shared fixture definitions,
including `workspaceFixture` and `singleTabWorkspace`, so contract changes are
caught by compilation.
In `@apps/web/src/components/layout/left-sidebar/AgentsSidebar.tsx`:
- Line 1035: Update the backgroundTabs filter to use a TypeScript type predicate
that narrows tab.targetId to string while excluding null and the current pane
scope target. Then remove the unnecessary “as string” assertion in the related
lines around the background tab handling, preserving the existing filtering
behavior.
In `@apps/web/src/lib/agent-sessions/__tests__/session-workspace-runtime.test.ts`:
- Around line 34-35: Expose a reset hook alongside __queueResponse that clears
the module-level responses array, then invoke that hook in beforeEach together
with vi.clearAllMocks(). Ensure each test starts with no queued database
responses, including the setup covered by the repeated lines.
In `@apps/web/src/lib/agent-sessions/session-workspace-runtime.ts`:
- Around line 52-61: Update saveSessionWorkspace to scope the database update by
both sessionId and the session owner, reusing the existing session
ownership/access model and preserving writes for authorized shared-session
users. Do not require ownerId if that would prevent non-owner shared-session
writes; instead use the appropriate existing ownership predicate or
access-compatible condition.
In
`@apps/web/src/stores/agent-workspace/__tests__/useAgentWorkspaceStore.test.ts`:
- Around line 461-468: Add a test alongside the existing closeTab coverage for
closing the pane’s only tab. Exercise store().closeTab with the initial
conversation, then assert the pane has no tabs, its scope is null, and
grid().pendingPickerPaneId equals the closed pane ID.
In
`@apps/web/src/stores/agent-workspace/__tests__/useWorkspaceServerSync.test.ts`:
- Around line 32-38: Add tests in useWorkspaceServerSync.test.ts for both
missing behaviors: verify a PUT resolving with ok: false and status 400 retains
the pending grid so unmount triggers another PUT, and verify rerendering the
hook from enabled: true to enabled: false flushes a pending save. Reuse the
existing store setup, renderHook helpers, and mockFetch assertions.
In `@apps/web/src/stores/agent-workspace/pane-reducer.ts`:
- Around line 165-175: Update replaceTab to defensively reject non-chat newScope
values before calling tabsForChatAssign, returning the unchanged state for
terminal or page scopes. Preserve the existing chat replacement behavior and
ensure PaneState.tabs remains empty for non-chat panes.
In `@apps/web/src/stores/agent-workspace/useWorkspaceServerSync.ts`:
- Around line 77-91: Update the hydration tracking around the workspace sync
effect to store the hydrated workspace object in a ref instead of setting
justHydratedRef. In the effect containing performSave, compare the observed
workspace against that stored hydrated value by reference and clear the ref when
matched, skipping persistence only for that exact hydrated object; ensure
conditional or no-op hydration cannot cause the next genuine mutation to be
skipped.
In `@packages/lib/src/agent-sessions/contract.ts`:
- Around line 124-128: Update persistedPaneStateSchema so tabs accepts only
addressable chat pane scopes: require a non-null targetId and restrict kind to
the chat value used by the pane model. Preserve paneScopeSchema for scope
itself, and verify existing clients do not persist terminal or targetless tabs
before enforcing the narrower schema.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 496c94f0-45b8-45f0-8099-fa3fab5706b5
📒 Files selected for processing (33)
apps/web/src/app/api/agent-sessions/[sessionId]/workspace/__tests__/route.test.tsapps/web/src/app/api/agent-sessions/[sessionId]/workspace/route.tsapps/web/src/app/api/agent-sessions/__tests__/route.test.tsapps/web/src/app/api/agent-sessions/route.tsapps/web/src/components/agents/panes/AgentPanes.tsxapps/web/src/components/agents/panes/__tests__/AgentPanes.test.tsxapps/web/src/components/agents/panes/__tests__/close-pane-tab.test.tsapps/web/src/components/agents/panes/__tests__/close-pane.test.tsapps/web/src/components/agents/panes/__tests__/open-pane-tab.test.tsapps/web/src/components/agents/panes/__tests__/select-pane-agent.test.tsapps/web/src/components/agents/panes/close-pane-tab.tsapps/web/src/components/agents/panes/close-pane.tsapps/web/src/components/agents/panes/open-pane-tab.tsapps/web/src/components/agents/panes/select-pane-agent.tsapps/web/src/components/agents/panes/session-conversations.tsapps/web/src/components/layout/left-sidebar/AgentsSidebar.tsxapps/web/src/components/layout/left-sidebar/__tests__/AgentsSidebar.test.tsxapps/web/src/lib/agent-sessions/__tests__/session-workspace-runtime.test.tsapps/web/src/lib/agent-sessions/session-workspace-runtime.tsapps/web/src/stores/agent-workspace/__tests__/pane-reducer.test.tsapps/web/src/stores/agent-workspace/__tests__/persisted-workspace.test.tsapps/web/src/stores/agent-workspace/__tests__/useAgentWorkspaceStore.test.tsapps/web/src/stores/agent-workspace/__tests__/useWorkspaceServerSync.test.tsapps/web/src/stores/agent-workspace/pane-reducer.tsapps/web/src/stores/agent-workspace/persisted-workspace.tsapps/web/src/stores/agent-workspace/useAgentWorkspaceStore.tsapps/web/src/stores/agent-workspace/useWorkspaceServerSync.tspackages/db/drizzle/0245_low_kid_colt.sqlpackages/db/drizzle/meta/0245_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/agent-sessions.tspackages/lib/src/agent-sessions/__tests__/contract.test.tspackages/lib/src/agent-sessions/contract.ts
💤 Files with no reviewable changes (2)
- apps/web/src/components/agents/panes/tests/close-pane.test.ts
- apps/web/src/components/agents/panes/close-pane.ts
# Conflicts: # apps/web/src/components/agents/panes/AgentPanes.tsx # apps/web/src/components/layout/left-sidebar/AgentsSidebar.tsx
# Conflicts: # apps/web/src/components/layout/left-sidebar/AgentsSidebar.tsx
…sistence - selectPane now short-circuits when the target is already active, returning the SAME reference — fixes useWorkspaceServerSync's hydration guard, which otherwise saw every mount's seeding effect as a local change and skipped cross-device restore entirely - useWorkspaceServerSync: hydration now applies unconditionally on first load (server wins, per the original design intent) instead of backing off whenever ANY local write raced ahead of the GET - useWorkspaceServerSync: performSave checks response.ok before clearing pendingRef, and the force-flush effect depends on 'enabled' so disabling mid-session still flushes - persistedPaneStateSchema backfills a legacy chat pane's tabs from its own scope instead of defaulting to [], and pendingPickerPaneId now shares activePaneId's non-empty-string rule - hydrateWorkspace (store + PUT route) rejects a workspace whose id doesn't match the target session - session-workspace-runtime re-validates workspaceState on the way out instead of trusting an unvalidated jsonb cast - AgentsSidebar: a nested background-tab click now goes through switchTab (pane-local) instead of the session-wide openConversation path, which only recognizes a pane's ACTIVE scope; closing a tab from the sidebar now goes through the same decideCloseTab decision AgentPanes uses, updating the local workspace store and respecting shown-elsewhere instead of a raw DELETE - History's 'New Conversation' now opens as a new tab (append) rather than replacing the pane's current conversation - test title fixes for two assertions that contradicted their names Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XRLJTmce8guxm1H4ZsFcYp
Follow-up to b954ea1, after an independent adversarial review of that commit surfaced three further issues: - useWorkspaceServerSync's 'hydrate once' tracking was a per-mount ref, but AgentPanes is rendered key={selectedSessionId} — switching sessions and back is a fresh mount every time, reopening the server-wins clobber window on every switch instead of once per page load. Moved the tracking to module scope (once per page load), only marking a session hydrated on a settled fetch so a cancelled/failed attempt still retries on the next mount. - AgentsSidebar's closePaneTab computed its 'shown elsewhere' check from the session.workspace SWR prop (fresh only to the last 20s poll or onChanged()) instead of the live workspace store AgentPanes.tsx itself reads for the identical decision — a tab opened in another pane moments ago could go undetected and get DELETEd out from under it. Now reads the live store directly. - pane-reducer's assignPaneShowing (used when a deleted conversation's row is replaced) only repointed the ONE pane whose ACTIVE scope matched the old id, leaving the same conversation dangling if it sat as a BACKGROUND tab in another pane (or the same pane) — a state tabs make routinely reachable. Now repoints every pane referencing the old id, only stealing grid focus for the pane where it was active. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XRLJTmce8guxm1H4ZsFcYp
The unmount/disable flush duplicated performSave's fetch, headers, and editing-store bookkeeping inline. Reuses performSave directly instead — same request shape, same response.ok handling, one less place to keep in sync. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XRLJTmce8guxm1H4ZsFcYp
…desktop grid) An independent full-diff audit found the desktop branch of SessionPanes missing the same activePaneId-resolves-to-a-live-pane fallback the mobile branch already has. The persisted-workspace schema doesn't cross-validate that activePaneId names a real pane in columns, so a saved grid restored from the server isn't guaranteed to satisfy it — on desktop this meant no pane got the active/focused styling at all, rather than falling back to the first one. Also adds the missing pane-reducer coverage the same audit flagged: closeTab on a pane other than the grid's active one (only ever tested against the sole/active pane before). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XRLJTmce8guxm1H4ZsFcYp
There was a problem hiding this comment.
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 (2)
apps/web/src/app/api/agent-sessions/route.ts (2)
394-396: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve generated-name uniqueness after truncation.
Line 396 checks uniqueness before it truncates the candidate to 120 characters. A long claimed conversation title can differ from an existing name before truncation but become identical after truncation. Generate and check each candidate within
MAX_SESSION_NAME_LENGTH, while reserving space for its numeric suffix.🤖 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/api/agent-sessions/route.ts` around lines 394 - 396, Update the generated-name logic around nextUniqueSessionName so uniqueness is evaluated on the final truncated value, not before truncation. Generate candidates within MAX_SESSION_NAME_LENGTH while reserving space for any numeric suffix, and compare each bounded candidate against existingSessions names to prevent collisions after truncation.
209-220: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject invalid and non-object request bodies.
Line 217 catches invalid JSON and continues with
{}. This creates a global session because an empty object is valid for this route. A valid JSONnullbody also reaches Line 221 and throws when the code readsbody.driveId.Return
400from the catch block. Validate that the parsed value is a non-null, non-array object before reading its properties.As per coding guidelines, API routes must get the request body with
const body = await request.json();.🤖 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/api/agent-sessions/route.ts` around lines 209 - 220, Update the request-body parsing in the route handler to use `const body = await request.json()` and return a 400 response when JSON parsing fails. Before accessing properties such as `driveId` or `agentPageId`, reject null, array, and other non-object parsed values with the same 400 response, while preserving valid object handling.Source: Coding guidelines
🧹 Nitpick comments (1)
apps/web/src/components/layout/left-sidebar/__tests__/AgentsSidebar.test.tsx (1)
323-326: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the workspace fixtures as
WorkspaceStateto remove the repeated casts.The
as WorkspaceStatecast repeats at Line 324, Line 381, Line 425, and Line 489. Each cast suppresses type checking on a fixture that is already structurally complete. If a field is added toPaneStateorWorkspaceState, these fixtures drift silently.Declare each fixture with an explicit
WorkspaceStateannotation, then pass it without a cast.♻️ Example for the shared-workspace fixture
- const sharedWorkspace = { + const sharedWorkspace: WorkspaceState = { id: 'ses-1',- useAgentWorkspaceStore.getState().hydrateWorkspace('ses-1', sharedWorkspace as WorkspaceState); + useAgentWorkspaceStore.getState().hydrateWorkspace('ses-1', sharedWorkspace);🤖 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/components/layout/left-sidebar/__tests__/AgentsSidebar.test.tsx` around lines 323 - 326, Update the workspace fixtures in AgentsSidebar tests to be explicitly declared as WorkspaceState, including the shared-workspace fixture and the fixtures used near hydrateWorkspace calls. Remove the repeated “as WorkspaceState” casts and pass each typed fixture directly, preserving structural type checking for future PaneState or WorkspaceState changes.
🤖 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/src/components/agents/panes/SessionPanes.tsx`:
- Around line 90-97: Update hydrateWorkspace to validate the saved activePaneId
against the hydrated workspace panes and replace it with the first pane’s ID
when no match exists. Keep the activeId fallback in the rendering logic as a
final safety net, but ensure the store is normalized during hydration.
---
Outside diff comments:
In `@apps/web/src/app/api/agent-sessions/route.ts`:
- Around line 394-396: Update the generated-name logic around
nextUniqueSessionName so uniqueness is evaluated on the final truncated value,
not before truncation. Generate candidates within MAX_SESSION_NAME_LENGTH while
reserving space for any numeric suffix, and compare each bounded candidate
against existingSessions names to prevent collisions after truncation.
- Around line 209-220: Update the request-body parsing in the route handler to
use `const body = await request.json()` and return a 400 response when JSON
parsing fails. Before accessing properties such as `driveId` or `agentPageId`,
reject null, array, and other non-object parsed values with the same 400
response, while preserving valid object handling.
---
Nitpick comments:
In
`@apps/web/src/components/layout/left-sidebar/__tests__/AgentsSidebar.test.tsx`:
- Around line 323-326: Update the workspace fixtures in AgentsSidebar tests to
be explicitly declared as WorkspaceState, including the shared-workspace fixture
and the fixtures used near hydrateWorkspace calls. Remove the repeated “as
WorkspaceState” casts and pass each typed fixture directly, preserving
structural type checking for future PaneState or WorkspaceState changes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 764377a4-ef0e-45de-a95a-d176d7b94045
📒 Files selected for processing (22)
apps/web/src/app/api/agent-sessions/[sessionId]/workspace/__tests__/route.test.tsapps/web/src/app/api/agent-sessions/[sessionId]/workspace/route.tsapps/web/src/app/api/agent-sessions/__tests__/route.test.tsapps/web/src/app/api/agent-sessions/route.tsapps/web/src/components/agents/panes/AgentPanes.tsxapps/web/src/components/agents/panes/SessionPanes.tsxapps/web/src/components/agents/panes/__tests__/AgentPanes.test.tsxapps/web/src/components/agents/panes/__tests__/SessionPanes.test.tsxapps/web/src/components/agents/panes/__tests__/open-pane-tab.test.tsapps/web/src/components/layout/left-sidebar/AgentsSidebar.tsxapps/web/src/components/layout/left-sidebar/__tests__/AgentsSidebar.test.tsxapps/web/src/lib/agent-sessions/__tests__/session-workspace-runtime.test.tsapps/web/src/lib/agent-sessions/session-workspace-runtime.tsapps/web/src/stores/agent-workspace/__tests__/pane-reducer.test.tsapps/web/src/stores/agent-workspace/__tests__/persisted-workspace.test.tsapps/web/src/stores/agent-workspace/__tests__/useAgentWorkspaceStore.test.tsapps/web/src/stores/agent-workspace/__tests__/useWorkspaceServerSync.test.tsapps/web/src/stores/agent-workspace/pane-reducer.tsapps/web/src/stores/agent-workspace/useAgentWorkspaceStore.tsapps/web/src/stores/agent-workspace/useWorkspaceServerSync.tspackages/lib/src/agent-sessions/__tests__/contract.test.tspackages/lib/src/agent-sessions/contract.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/web/src/lib/agent-sessions/tests/session-workspace-runtime.test.ts
- packages/lib/src/agent-sessions/tests/contract.test.ts
- apps/web/src/app/api/agent-sessions/tests/route.test.ts
- apps/web/src/app/api/agent-sessions/[sessionId]/workspace/route.ts
- apps/web/src/stores/agent-workspace/tests/useAgentWorkspaceStore.test.ts
- apps/web/src/stores/agent-workspace/pane-reducer.ts
- apps/web/src/components/layout/left-sidebar/AgentsSidebar.tsx
- apps/web/src/lib/agent-sessions/session-workspace-runtime.ts
- apps/web/src/stores/agent-workspace/useAgentWorkspaceStore.ts
- packages/lib/src/agent-sessions/contract.ts
- apps/web/src/components/agents/panes/tests/open-pane-tab.test.ts
- apps/web/src/components/agents/panes/AgentPanes.tsx
CodeRabbit's follow-up on the SessionPanes.tsx fix (c4eb78f): repairing activePaneId only at render time meant every future consumer of a hydrated workspace would need its own fallback. Normalizing it once in hydrateWorkspace — the single place a saved grid enters the store — means the store itself never holds an invalid activePaneId after this point; SessionPanes.tsx's own fallback stays as a second, independent safety net. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XRLJTmce8guxm1H4ZsFcYp
1 participant
Summary
agent_sessions.workspaceState), synced via a debounced hook — an earlier version of this pane system synced eagerly and was torn down for making every split a network write, so this one debounces instead. A refresh or a different device now restores the same panes.AgentsSidebarnow renders Session → Pane → Tabs (one row per pane, updating in place) instead of a flat conversation list, falling back to the old flat list for any session with no saved grid yet.Test plan
typecheckandlintpassknip:check— within baseline, no new unused exportsgrouping.test.ts, unrelated to this change — file untouched by this branch)select-pane-agent,open-pane-tab,close-pane-tab),pane-reducertab bookkeeping,useWorkspaceServerSync(debounce, race guard, force-flush, hydrate-once-per-page-load, non-2xx handling), the new[sessionId]/workspaceAPI route, the bulk-workspace list endpoint,AgentPanesintegration tests, andAgentsSidebarpane-grouped renderingassignPaneShowing— with the same "active-scope-only" gap affecting background tabs). All fixed with regression tests; CodeRabbit has independently re-reviewed and self-resolved 9/14 of its original threads.🤖 Generated with Claude Code
https://claude.ai/code/session_01XRLJTmce8guxm1H4ZsFcYp
Summary by CodeRabbit
New Features
Bug Fixes