Repository navigation
fix(agents): sidebar scrollbar, pane-picker overlap, Global Assistant in drive spawn - #2294
Conversation
… in drive spawn - Hide the Agents sidebar's session-list scrollbar entirely (new .scrollbar-none utility) instead of the Radix ScrollArea's always-visible overlay bar, which collided with row "+" buttons. - Fix PanePicker's split-pane overlap: its sub-list wrappers used min-h-0 on non-scrolling flex boxes, letting a short pane crush them while their shrink-0 buttons refused to shrink — pushing "Global Assistant" past its box and stealing clicks from "Agents" below. Switched to shrink-0. - Offer Global Assistant as the first pick in a drive's "+" spawn palette, landing the new session under that drive (loosened the session-spawn validation gate, which previously only allowed a driveless assistant spawn even though drive-scoped assistant conversations were already supported everywhere else). - Default naming: "Assistant" -> "Global Assistant" for the auto-named session label, the naming-step placeholder, and the expanded-conversation fallback. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMC7fGVwuwyD7NChEaQobP
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThe API now supports drive-scoped Global Assistant sessions. The Agents UI exposes these sessions, propagates their drive context to chat requests, updates naming, preserves pane-picker sizing, and hides session-list scrollbars. ChangesGlobal Assistant sessions
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant AgentsSidebar
participant AgentSessionsRoute
participant GlobalAssistantChat
participant useAssistantSessionChat
User->>AgentsSidebar: select Global Assistant for a drive
AgentsSidebar->>AgentSessionsRoute: create session with driveId and null agentId
AgentSessionsRoute-->>AgentsSidebar: return created session
AgentsSidebar->>GlobalAssistantChat: open session with driveId
GlobalAssistantChat->>useAssistantSessionChat: send conversation message
useAssistantSessionChat->>useAssistantSessionChat: use session drive context
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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: bf68715e85
ℹ️ 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".
| <CommandItem | ||
| value="assistant-Global Assistant" | ||
| disabled={spawning} | ||
| onSelect={() => onPickTarget({ kind: 'assistant', agentPageId: null, label: 'Global Assistant' })} |
There was a problem hiding this comment.
When a user opens a drive's + from the global /dashboard/agents view and chooses this new item, the session is stored under that drive, but useAssistantSessionChat derives contextRef solely from the unchanged pathname via buildContextRef. That route provides no selected-session drive context, so the global assistant receives no current drive, omits its drive prompt and page-tree scope, and can resolve instructions such as “create this here” against the wrong workspace or no workspace at all. Thread the spawned session's driveId into the assistant request context rather than relying on the console URL.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 92b52a8. The gap was actually bigger than described: agents isn't in tab-title.ts's DRIVE_SPECIAL_ROUTES at all, so buildContextRef has never resolved a drive for any Global Assistant conversation inside an agent session (drive-scoped or global) reached from the Agents console — this predates this PR and also affects the pre-existing split-pane-picker path, not just the new spawn-palette entry point.
Fix: threaded the session's own driveId (already available on AgentPanes) through PaneChat -> AssistantSessionChat -> useAssistantSessionChat, and use it directly ({ routeType: drive, driveId }) instead of parsing the pathname whenever the session has a drive. A driveless global-assistant session keeps the pathname-derived context (unaffected — there's no session driveId to prefer there). Added useAssistantSessionChat.test.ts covering both the override and the fallback. Left useAgentSessionChat untouched: an agent conversation's drive is already anchored server-side via its agentPageId regardless of contextRef, so the same pathname imprecision there only affects incidental "this page" references, not core drive scoping — a smaller, separate, pre-existing concern I left out of scope for this PR.
Leaving this thread open for verification rather than resolving it myself.
…xtRef Review feedback on #2294 (chatgpt-codex-connector): when Global Assistant is spawned for a drive from the global /dashboard/agents view, the session correctly gets that driveId, but the chat itself derived its drive context from the current pathname (buildContextRef), which never carries a drive for that route. Turns out the gap is bigger than the specific report: `agents` isn't in tab-title.ts's DRIVE_SPECIAL_ROUTES at all, so pathname-based context has never worked for a Global Assistant conversation in ANY agent session, drive-scoped or global — this predates this PR and affects the existing split-pane picker path too, not just the new spawn-palette entry point. Fix: thread the session's own driveId (AgentPanes already has it) through PaneChat -> AssistantSessionChat -> useAssistantSessionChat, and use it directly ({ routeType: 'drive', driveId }) instead of parsing the pathname whenever the session has a drive. A driveless global-assistant session (driveId null) keeps the pathname-derived context — that path had nothing to override with and is unaffected. Not touched: useAgentSessionChat has the identical pathname pattern, but an agent conversation's drive is already anchored server-side via its agentPageId regardless of contextRef, so pathname imprecision there only affects incidental "this page" references, not core drive scoping — a materially smaller, pre-existing, separate concern left out of scope here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMC7fGVwuwyD7NChEaQobP
…e spawn Matches this repo's convention of manual user-facing changelog entries per notable fix/feature (the automated changelog:generate script is known-stale, per prior review — entries here are hand-written like recent precedent, e.g. d8f882e, ebddd23, 5463797). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMC7fGVwuwyD7NChEaQobP
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
.scrollbar-noneutility) instead of the RadixScrollArea's always-visible overlay bar, which collided with row "+" buttons.PanePicker's split-pane overlap: its sub-list wrappers usedmin-h-0on non-scrolling flex boxes, letting a short pane crush them while theirshrink-0buttons refused to shrink — pushing "Global Assistant" past its box and stealing clicks from "Agents" below. Switched toshrink-0.driveIdinto the Global Assistant's chat request context instead of deriving it from the pathname. Turned outagentsisn't a routebuildContextRef/tab-title.tsrecognizes as drive-scoped at all, so a Global Assistant conversation inside an agent session never got its drive's prompt/page-tree scope from any entry point (pre-existing, not introduced by this PR, but the new spawn-palette path made it reachable in one more way). See the review thread for detail.CHANGELOG.mdentry (manual, matching this repo's convention — thechangelog:generatescript is known-stale).Test plan
bun run typecheck(apps/web) — cleanroute.test.ts— 34/34 passing, including new tests for the drive-scoped assistant spawn and the narrowed validation gateAgentsSidebar.test.tsx— 48/48 passing, including a new test for picking Global Assistant from a drive's "+"PanePicker.test.tsx,AgentPanes.test.tsx,SessionPanes.test.tsx— 65/65 passinguseAssistantSessionChat.test.ts(new) — covers the driveId-override and the driveless fallbackbun run test:unit(full monorepo lib + web suite) — 15502/15503 passing; the 1 failure (grouping.test.tsmidnight-boundary case) is a pre-existing, unrelated TZ-environment flake (fails under local CDT, passes under UTC), not touched by this PRbun run test:security— 44/50 suites passing; the 6 failures match this repo's known pre-existing baseline (deleted Login/Signup/Mobile-Login routes, excluded Session Service/Device Auth/Permissions suites) — confirmed identical failure set, nothing newbun run lint(repo-wide build+lint) — cleanmaster/dashboard/agents's left sidebar, at rest or on hover/dashboard/agentsview, spawn Global Assistant for a specific drive and confirm the assistant has that drive's context (page tree / system prompt)🤖 Generated with Claude Code
https://claude.ai/code/session_01MMC7fGVwuwyD7NChEaQobP
Summary by CodeRabbit
New Features
Bug Fixes