fix(desktop): run agent in separate pane#1582
Merged
Merged
Conversation
…tric CORS - Store agent command separately in PendingTerminalSetup instead of appending to initialCommands, so setup script and agent run in separate panes within the same tab - Add X-Electric-Backend to CORS Access-Control-Allow-Headers (needed after Electric Cloud header was added in #1572) - Use wildcard for Access-Control-Expose-Headers to expose all Electric response headers (electric-offset, electric-handle, electric-schema, etc.) - Fix auth.apikeys where clause to use LIKE instead of ::jsonb cast which Docker Electric doesn't support
Contributor
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughThis PR routes agentCommand through terminal/session setup to create panes with that command, updates CORS expose/allow headers for the proxy, changes API key metadata filtering from jsonb extraction to a string LIKE, and removes two debug logs in trpc storage. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Watcher as useCommandWatcher
participant Session as Session Init
participant Terminal as TerminalSetup
participant Workspace as WorkspaceInitEffects
participant Tabs as Tabs Store
User->>Watcher: run command / invoke agent
Watcher->>Session: build session payload (extract agentCommand)
Session->>Terminal: create pending setup (agentCommand + initialCommands)
Terminal->>Workspace: persist pending terminal setup
rect rgba(100, 200, 255, 0.5)
Note over Workspace: detects agentCommand in pending setup
Workspace->>Tabs: call addPane(initialCommands: [agentCommand])
Tabs->>Tabs: create pane and populate terminal
end
Tabs-->>User: new pane shown with agent command
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 (1)
apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/tools/start-claude-session.ts (1)
29-29:⚠️ Potential issue | 🟡 MinorStale comment — update to reflect the new separation of concerns.
The comment says "Append command to pending terminal setup", but the behaviour is now the opposite:
agentCommandis stored separately andinitialCommandsis preserved unchanged (not appended to).✏️ Suggested update
- // Append command to pending terminal setup for the existing workspace + // Set agent command in pending terminal setup, keeping the setup script commands separate🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/tools/start-claude-session.ts` at line 29, Update the stale inline comment in start-claude-session.ts to reflect the new separation of concerns: explain that agentCommand is stored separately (e.g., saved to agentCommand or similar variable) while initialCommands are preserved and not appended to; replace the phrase "Append command to pending terminal setup" with a clear note that the agent command is stored independently and initialCommands remain unchanged so callers can rely on the original initialCommands value.
🧹 Nitpick comments (2)
apps/api/src/app/api/electric/[...path]/utils.ts (1)
116-116: Performance: leading%wildcard prevents index use onmetadata.The LIKE
'%...'pattern with a leading wildcard cannot use a B-tree index onmetadata. Ifauth.apikeysgrows, this causes a full sequential scan per sync request. Consider a GIN index onmetadatacast totext(or a generated column) if query latency becomes an issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/api/electric/`[...path]/utils.ts at line 116, The current fragment variable uses a leading-wildcard LIKE which prevents index use; change the fragment in utils.ts from the LIKE expression to a JSON-aware predicate such as using metadata->>'organizationId' = $1 (or metadata @> jsonb_build_object('organizationId', $1)) so the DB can use an expression or GIN index, and ensure a GIN index (or an expression index on metadata->>'organizationId') exists on the metadata JSONB column to avoid sequential scans when auth.apikeys grows.apps/desktop/src/renderer/screens/main/components/WorkspaceInitEffects.tsx (1)
142-147: Duplicated agent-tab creation block — extract into a shared helper.The identical five-line sequence (lines 142–146 and 152–156) is repeated verbatim across the
hasPresetsandagentCommand-only branches. Extracting it keeps the callback DRY and makes future changes (e.g., adding a pane layout option) a one-line edit.♻️ Suggested refactor
Inside
handleTerminalSetup, before the branch chain:+ const openAgentTab = () => { + const { tabId: agentTabId } = addTab(setup.workspaceId, { + initialCommands: [agentCommand!], + }); + setTabAutoTitle(agentTabId, "Agent"); + }; if (hasPresets) { for (const preset of presets) { openPreset(setup.workspaceId, preset); } - if (agentCommand) { - const { tabId: agentTabId } = addTab(setup.workspaceId, { - initialCommands: [agentCommand], - }); - setTabAutoTitle(agentTabId, "Agent"); - } + if (agentCommand) openAgentTab(); onComplete(); return; } if (agentCommand) { - const { tabId: agentTabId } = addTab(setup.workspaceId, { - initialCommands: [agentCommand], - }); - setTabAutoTitle(agentTabId, "Agent"); + openAgentTab(); onComplete(); return; }Also applies to: 152-158
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceInitEffects.tsx` around lines 142 - 147, There’s duplicated logic adding an agent tab in handleTerminalSetup; extract the five-line sequence that calls addTab(setup.workspaceId, { initialCommands: [agentCommand] }) and setTabAutoTitle(agentTabId, "Agent") into a small helper (e.g., createAgentTab or addAgentTab) declared inside handleTerminalSetup before the branch, then replace both repeated blocks (the hasPresets branch and the agentCommand-only branch) with a single call to that helper using the existing agentCommand and setup.workspaceId variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/proxy.ts`:
- Around line 28-29: The CORS response header "Access-Control-Expose-Headers" is
currently set to "*" which is ignored when "Access-Control-Allow-Credentials" is
"true"; update the proxy response headers in proxy.ts to explicitly list the
Electric headers instead of using "*". Replace the wildcard value for
"Access-Control-Expose-Headers" with a comma-separated string enumerating
"electric-offset", "electric-handle", "electric-schema", and "electric-cursor"
(keeping the existing "Access-Control-Allow-Credentials": "true") so the browser
can access those headers.
---
Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/tools/start-claude-session.ts`:
- Line 29: Update the stale inline comment in start-claude-session.ts to reflect
the new separation of concerns: explain that agentCommand is stored separately
(e.g., saved to agentCommand or similar variable) while initialCommands are
preserved and not appended to; replace the phrase "Append command to pending
terminal setup" with a clear note that the agent command is stored independently
and initialCommands remain unchanged so callers can rely on the original
initialCommands value.
---
Nitpick comments:
In `@apps/api/src/app/api/electric/`[...path]/utils.ts:
- Line 116: The current fragment variable uses a leading-wildcard LIKE which
prevents index use; change the fragment in utils.ts from the LIKE expression to
a JSON-aware predicate such as using metadata->>'organizationId' = $1 (or
metadata @> jsonb_build_object('organizationId', $1)) so the DB can use an
expression or GIN index, and ensure a GIN index (or an expression index on
metadata->>'organizationId') exists on the metadata JSONB column to avoid
sequential scans when auth.apikeys grows.
In `@apps/desktop/src/renderer/screens/main/components/WorkspaceInitEffects.tsx`:
- Around line 142-147: There’s duplicated logic adding an agent tab in
handleTerminalSetup; extract the five-line sequence that calls
addTab(setup.workspaceId, { initialCommands: [agentCommand] }) and
setTabAutoTitle(agentTabId, "Agent") into a small helper (e.g., createAgentTab
or addAgentTab) declared inside handleTerminalSetup before the branch, then
replace both repeated blocks (the hasPresets branch and the agentCommand-only
branch) with a single call to that helper using the existing agentCommand and
setup.workspaceId variables.
- Replace wildcard Access-Control-Expose-Headers with explicit list (wildcard is ignored when Access-Control-Allow-Credentials is true) - Remove stray console.log calls from trpc-storage.ts
saddlepaddle
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
&&in the same terminal — prevents the agent from crashing when the setup script runsX-Electric-Backendto allowed headers and expose all response headers via wildcard (fixesMissingHeadersErrorforelectric-offset,electric-handle,electric-schema,electric-cursor)auth.apikeysElectric where clause: useLIKEinstead of::jsonbcast which Docker Electric doesn't supportTest plan
.superset/config.jsonstart_claude_session— verify setup script and agent run in separate panesstart_claude_sessionwithout a setup script — verify agent opens in its own tabauth.apikeyscollection syncs without jsonb cast errorSummary by CodeRabbit
New Features
Bug Fixes
Chores