feat: wire kortex-cli init and add CLI logging#1231
Open
bmahabirbu wants to merge 2 commits intokortex-hub:mainfrom
Open
feat: wire kortex-cli init and add CLI logging#1231bmahabirbu wants to merge 2 commits intokortex-hub:mainfrom
bmahabirbu wants to merge 2 commits intokortex-hub:mainfrom
Conversation
Wire up create IPC handler, resolve CLI path from CliToolRegistry, and connect the renderer form to the backend. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Brian <bmahabir@bu.edu>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Brian <bmahabir@bu.edu>
bmahabirbu
requested review from
benoitf and
fbricon
and removed request for
a team
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughA new workspace creation feature is added through the agent workspace subsystem. The changes introduce a Changes
Sequence DiagramsequenceDiagram
actor User
participant Renderer as Renderer (UI)
participant Preload as Preload Bridge
participant Main as Main Process (IPC)
participant CLI as kortex-cli
User->>Renderer: Fill form & click "Start Workspace"
Renderer->>Renderer: Validate inputs (sessionName, workingDir, agent)
Renderer->>Renderer: Set creating=true, clear error
Renderer->>Preload: createAgentWorkspace(options)
Preload->>Main: IPC send 'agent-workspace:create'
Main->>Main: Resolve kortex-cli path via CliToolRegistry
Main->>CLI: Execute: kortex-cli workspace init<br/> --sourcePath --agent<br/> --runtime [--name] [--project]<br/> --output json
CLI-->>Main: Return JSON with workspace ID
Main->>Main: Parse JSON & emit<br/>'agent-workspace-update'
Main-->>Preload: Return AgentWorkspaceId
Preload-->>Renderer: Resolve Promise
Renderer->>Renderer: Set creating=false
Renderer->>Renderer: Navigate to AGENT_WORKSPACES
Renderer-->>User: Show workspace list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte (1)
121-131:⚠️ Potential issue | 🟠 MajorRemove the full renderer-side config dump.
This logs a superset of the actual IPC payload — free-form description, local/custom paths, skill IDs, and MCP IDs — even though most of those fields are intentionally not sent to the backend. In this codebase renderer console output is retained for troubleshooting export, so this becomes a privacy leak and a misleading trace.
✂️ Proposed fix
- const config = { - name: sessionName, - workingDir, - description, - agent: selectedAgent, - fileAccess: selectedFileAccess, - customPaths: selectedFileAccess === 'custom' ? customPaths.filter(p => p.trim()) : undefined, - skills: selectedSkillIds, - mcpServers: selectedMcpIds, - }; - console.log('Starting workspace with config:', config);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte` around lines 121 - 131, The console.log emitting the full renderer-side config (the const config object created from sessionName, workingDir, description, selectedAgent, selectedFileAccess, customPaths, selectedSkillIds, selectedMcpIds) must be removed or replaced with a minimal/sanitized log; do not print free-form description, customPaths, skill IDs, or MCP IDs. Update the code around the config variable in AgentWorkspaceCreate.svelte so that either the console.log line is deleted or it only logs non-sensitive fields (e.g., sessionName and selectedAgent) and explicitly omits description, customPaths, selectedSkillIds, and selectedMcpIds.
🧹 Nitpick comments (1)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts (1)
124-188: Add assertions for the new CLI logging behavior.The PR adds command/error logging, but the spec currently validates only command execution/results. Add log/error expectations to prevent regressions.
Example test additions
test('executes kortex-cli init with required flags and returns the workspace id', async () => { + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); vi.spyOn(exec, 'exec').mockResolvedValue(mockExecResult(JSON.stringify({ id: 'ws-new' }))); const result = await manager.create(defaultOptions); + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining('Executing:')); expect(exec.exec).toHaveBeenCalledWith(KORTEX_CLI_PATH, [ 'init', '/tmp/my-project', @@ expect(result).toEqual({ id: 'ws-new' }); }); test('rejects when CLI fails', async () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); vi.spyOn(exec, 'exec').mockRejectedValue(new Error('command not found')); await expect(manager.list()).rejects.toThrow('command not found'); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('kortex-cli failed:')); });Also applies to: 214-218
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts` around lines 124 - 188, Tests currently assert exec.exec calls and results but not the new CLI logging; update the tests that mock exec.exec and call manager.create to also assert the logger methods are invoked: for successful runs (the tests that mockResolvedValue) add an expectation that logger.info (or the module's CLI logger used in the implementation) was called with the executed command/args (matching KORTEX_CLI_PATH and the args array), and for the failure test (the one that mockRejectedValue) add an expectation that logger.error was called with the thrown error message. Reference the existing mocks around exec.exec and manager.create when adding these assertions so you modify the same test cases (including the test around lines 214-218).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts`:
- Line 27: The imported type named Proxy shadows the global Proxy and triggers
the lint rule; rename the import to a non-conflicting alias (e.g., ProxyType) in
the import statement in agent-workspace-manager.spec.ts and update all
references to that type within the file (search for usages of Proxy and replace
with the new alias) so the linter no longer flags noShadowRestrictedNames while
preserving the same type.
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts`:
- Around line 74-95: The create method shells out via this.exec.exec in
agent-workspace-manager.ts but doesn't use TaskManager so long-running init
calls can outlive UI; update create(AgentWorkspaceCreateOptions) to create and
run a Task via TaskManager (e.g., createTask/runTask) around the call to
this.exec.exec, publish incremental status (starting, success, failure) and wire
cancellation to abort the exec if TaskManager supports it, then send the
existing this.apiSender.send('agent-workspace-update') on success and rethrow
errors after marking the task failed; ensure you reference the create method,
this.exec.exec call, TaskManager APIs you have in the codebase, and keep
existing JSON parsing of result.stdout and error logging behavior.
In `@packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte`:
- Around line 136-140: The create call in AgentWorkspaceCreate.svelte currently
invokes window.createAgentWorkspace with only sourcePath, agent and name which
causes AgentWorkspaceManager.create() to default runtime to 'podman'; update the
call to include the runtime selected in the UI (e.g., pass runtime: runtime or
runtime: selectedRuntime alongside sourcePath: workingDir, agent: selectedAgent,
name: sessionName) so the UI's runtime selection is forwarded to
window.createAgentWorkspace and ultimately to AgentWorkspaceManager.create().
---
Outside diff comments:
In `@packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte`:
- Around line 121-131: The console.log emitting the full renderer-side config
(the const config object created from sessionName, workingDir, description,
selectedAgent, selectedFileAccess, customPaths, selectedSkillIds,
selectedMcpIds) must be removed or replaced with a minimal/sanitized log; do not
print free-form description, customPaths, skill IDs, or MCP IDs. Update the code
around the config variable in AgentWorkspaceCreate.svelte so that either the
console.log line is deleted or it only logs non-sensitive fields (e.g.,
sessionName and selectedAgent) and explicitly omits description, customPaths,
selectedSkillIds, and selectedMcpIds.
---
Nitpick comments:
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts`:
- Around line 124-188: Tests currently assert exec.exec calls and results but
not the new CLI logging; update the tests that mock exec.exec and call
manager.create to also assert the logger methods are invoked: for successful
runs (the tests that mockResolvedValue) add an expectation that logger.info (or
the module's CLI logger used in the implementation) was called with the executed
command/args (matching KORTEX_CLI_PATH and the args array), and for the failure
test (the one that mockRejectedValue) add an expectation that logger.error was
called with the thrown error message. Reference the existing mocks around
exec.exec and manager.create when adding these assertions so you modify the same
test cases (including the test around lines 214-218).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e0d0d8d6-0491-4f5f-aa3d-d76b0e0ccad4
📒 Files selected for processing (6)
packages/api/src/agent-workspace-info.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/index.tspackages/preload/src/index.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
|
|
||
| import type { IPCHandle } from '/@/plugin/api.js'; | ||
| import type { CliToolRegistry } from '/@/plugin/cli-tool-registry.js'; | ||
| import type { Proxy } from '/@/plugin/proxy.js'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "import type \{ Proxy( as .+)? \}" packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
rg -n "as unknown as Proxy\\b" packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tsRepository: kortex-hub/kortex
Length of output: 138
🏁 Script executed:
#!/bin/bash
# Get full context around the import and usage
sed -n '25,65p' packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts | cat -n
# Search for all uses of "Proxy" (not just imports) in the file to ensure fix is complete
rg -n "\bProxy\b" packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tsRepository: kortex-hub/kortex
Length of output: 1701
Rename imported Proxy type to avoid lint-blocking global shadowing.
The Proxy import at line 27 shadows the global Proxy object, triggering lint/suspicious/noShadowRestrictedNames and blocking CI.
Proposed fix
-import type { Proxy } from '/@/plugin/proxy.js';
+import type { Proxy as KortexProxy } from '/@/plugin/proxy.js';
...
-} as unknown as Proxy;
+} as unknown as KortexProxy;🧰 Tools
🪛 Biome (2.4.9)
[error] 27-27: Do not shadow the global "Proxy" property.
(lint/suspicious/noShadowRestrictedNames)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts` at
line 27, The imported type named Proxy shadows the global Proxy and triggers the
lint rule; rename the import to a non-conflicting alias (e.g., ProxyType) in the
import statement in agent-workspace-manager.spec.ts and update all references to
that type within the file (search for usages of Proxy and replace with the new
alias) so the linter no longer flags noShadowRestrictedNames while preserving
the same type.
Comment on lines
+74
to
95
| async create(options: AgentWorkspaceCreateOptions): Promise<AgentWorkspaceId> { | ||
| const cliPath = this.getCliPath(); | ||
| const runtime = options.runtime ?? 'podman'; | ||
| const args = ['init', options.sourcePath, '--runtime', runtime, '--agent', options.agent, '--output', 'json']; | ||
| if (options.name) { | ||
| args.push('--name', options.name); | ||
| } | ||
| if (options.project) { | ||
| args.push('--project', options.project); | ||
| } | ||
| console.log(`Executing: ${cliPath} ${args.join(' ')}`); | ||
| try { | ||
| const result = await this.exec.exec(cliPath, args); | ||
| const workspaceId = JSON.parse(result.stdout) as AgentWorkspaceId; | ||
| this.apiSender.send('agent-workspace-update'); | ||
| return workspaceId; | ||
| } catch (err: unknown) { | ||
| const message = err instanceof Error ? err.message : String(err); | ||
| console.log(`kortex-cli failed: ${cliPath} ${args.join(' ')} — ${message}`); | ||
| throw err; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Track init through TaskManager.
This shells out to an external CLI and can easily outlive the form-local spinner, but it never creates a task or publishes status the rest of the app can observe. Mirroring the other main-process CLI flows here would give users durable progress and failure reporting even if they navigate away. As per coding guidelines, "Long-running operations should use TaskManager to create and manage tasks with appropriate status updates".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts` around
lines 74 - 95, The create method shells out via this.exec.exec in
agent-workspace-manager.ts but doesn't use TaskManager so long-running init
calls can outlive UI; update create(AgentWorkspaceCreateOptions) to create and
run a Task via TaskManager (e.g., createTask/runTask) around the call to
this.exec.exec, publish incremental status (starting, success, failure) and wire
cancellation to abort the exec if TaskManager supports it, then send the
existing this.apiSender.send('agent-workspace-update') on success and rethrow
errors after marking the task failed; ensure you reference the create method,
this.exec.exec call, TaskManager APIs you have in the codebase, and keep
existing JSON parsing of result.stdout and error logging behavior.
Comment on lines
+136
to
+140
| await window.createAgentWorkspace({ | ||
| sourcePath: workingDir, | ||
| agent: selectedAgent, | ||
| name: sessionName, | ||
| }); |
There was a problem hiding this comment.
Forward runtime in the create request.
This call only sends sourcePath, agent, and name. Because AgentWorkspaceManager.create() defaults a missing runtime to 'podman', the new flow still hard-codes the default instead of propagating runtime from the UI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte`
around lines 136 - 140, The create call in AgentWorkspaceCreate.svelte currently
invokes window.createAgentWorkspace with only sourcePath, agent and name which
causes AgentWorkspaceManager.create() to default runtime to 'podman'; update the
call to include the runtime selected in the UI (e.g., pass runtime: runtime or
runtime: selectedRuntime alongside sourcePath: workingDir, agent: selectedAgent,
name: sessionName) so the UI's runtime selection is forwarded to
window.createAgentWorkspace and ultimately to AgentWorkspaceManager.create().
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
kortex-cli initfrom the workspace create UI form (name, agent, source path, runtime)console.logfor allkortex-clicommands (init, list, remove, start, stop) with error loggingAgentWorkspaceCreateOptionstype, IPC handler, and preload bridge for createCliToolRegistrywith fallback tokortex-clifrom PATHkortex-cli(skills, MCP servers, file access) are kept in the form but not sent to the backendTest plan
AgentWorkspaceManagerExecuting:log appears in terminalkortex-cli failed:log and red error banner in UI🤖 Generated with Claude Code