fix(agent-workspace): validate workspace name against hostname limit#2464
Merged
serbangeorge-m merged 5 commits intoJul 21, 2026
Merged
Conversation
Reject workspace names longer than 64 characters in the create wizard and backend to avoid sandbox creation failures. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: serbangeorge-m <serbangeorge.m@gmail.com>
serbangeorge-m
requested review from
MarsKubeX and
benoitf
and removed request for
a team
July 15, 2026 12:32
Contributor
📝 WalkthroughWalkthroughAdds a shared 56-character sandbox name limit, validates explicit and source-derived names in the renderer and OpenShell creation flow, and adds boundary and rejection tests across API, main, and renderer packages. ChangesWorkspace Name Validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant WorkspaceWizard
participant AgentWorkspaceManager
participant OpenShellCLI
User->>WorkspaceWizard: Enter or derive workspace name
WorkspaceWizard->>WorkspaceWizard: Validate effective name
WorkspaceWizard->>AgentWorkspaceManager: Start workspace creation
AgentWorkspaceManager->>AgentWorkspaceManager: Validate sandbox name
AgentWorkspaceManager->>OpenShellCLI: createSandbox(valid name)
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte`:
- Around line 439-446: Update the workspace creation payload in the creation
flow near the guard to send the validated effective name instead of directly
using draftSnapshot.sessionName. Reuse the fallback logic from startAsIs() and
ensure the payload name matches getEffectiveWorkspaceName(), including when the
user clears the name input.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3c769842-7c1f-4104-9113-505176817122
📒 Files selected for processing (8)
packages/api/src/agent-workspace-info.spec.tspackages/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/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.svelte
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
/@/path aliases instead of relative paths for imports outside the current directory's module group; use relative imports only for sibling modules within the same directory
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/api/src/agent-workspace-info.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.tspackages/api/src/agent-workspace-info.ts
packages/main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/main/src/**/*.{ts,tsx}: UseipcHandle()to expose handlers in the main process with naming convention<registry-name>:<action>(e.g.,container-provider-registry:listContainers)
UseapiSender.send()to send events from main process to renderer for real-time updates
Long-running operations should useTaskManager.createTask()with title and action configuration
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
packages/{main,renderer,preload}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Container operations must include
engineIdparameter to identify the container engine
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.ts
**/*.spec.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.{ts,tsx,js,jsx}: Usetest()instead ofit()for test cases in Vitest unit tests
Usevi.mock(import('...'))for auto-mocking modules in unit tests; avoid manual mock factories when possible
Usevi.resetAllMocks()inbeforeEachhooks instead ofvi.clearAllMocks()for resetting mocks between tests
When an auto-mocked function or class method needs a real implementation, usevi.mocked(...)with the prototype pattern for class methods:vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/api/src/agent-workspace-info.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.ts
🧠 Learnings (11)
📚 Learning: 2026-03-09T08:47:09.657Z
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 1077
File: packages/main/src/plugin/skill/skill-manager.ts:80-109
Timestamp: 2026-03-09T08:47:09.657Z
Learning: In the kortex-hub/kortex repository, IPC handlers (via ipcHandle()) may be registered directly inside feature manager/service classes (e.g., SkillManager in packages/main/src/plugin/skill/skill-manager.ts) rather than exclusively in packages/main/src/plugin/index.ts. Treat this as an accepted design pattern for files under the plugin directory. Reviewers should not require centralization in index.ts; allow IPC registration proximity to the feature that owns the handler. When reviewing code, accept direct ipcHandle() registrations inside feature managers and ensure the pattern is consistently applied across similar feature-manager modules.
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
📚 Learning: 2026-05-12T17:14:02.153Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1850
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte:66-70
Timestamp: 2026-05-12T17:14:02.153Z
Learning: When reviewing code that uses `AgentWorkspaceSummaryUI.runtime`, treat it as a required, non-null `string` per the `openkaiden/kdn-api` 0.12.0 schema. Therefore, code like `a.runtime.localeCompare(b.runtime)` is safe and should not trigger warnings about possible `undefined`/`null` values or suggestions to use nullish coalescing/optional chaining for `runtime` (unless the current local types still mark `runtime` as optional, indicating a schema/version mismatch).
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/api/src/agent-workspace-info.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.tspackages/api/src/agent-workspace-info.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-06-29T13:16:53.102Z
Learnt from: benoitf
Repo: openkaiden/kaiden PR: 2296
File: extensions/container/packages/extension/src/helper/socket-finder/_socket-finder-module.ts:28-29
Timestamp: 2026-06-29T13:16:53.102Z
Learning: When reviewing imports in openkaiden/kaiden TypeScript/JavaScript files, prefer the configured `/@/` path alias instead of relative imports that would require traversing out of the current directory/module group (i.e., paths containing `..` that cross boundaries).
Do not require alias conversion for descendant-path relative imports within the socket-finder module directory—for example, in `extensions/container/packages/extension/src/helper/socket-finder/**`, imports like `./podman/podman-version-detector` and `./podman/podman-windows-finder` are acceptable and should not be flagged.
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/api/src/agent-workspace-info.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.tspackages/api/src/agent-workspace-info.ts
📚 Learning: 2026-04-15T08:51:08.199Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1336
File: packages/renderer/src/lib/guided-setup/GuidedSetup.spec.ts:64-69
Timestamp: 2026-04-15T08:51:08.199Z
Learning: For Svelte component test files under `packages/renderer/src/**` with names ending in `.spec.ts` that use `render` from `testing-library/svelte`, call `vi.useFakeTimers({ shouldAdvanceTime: true })` inside a `beforeEach`. Keep it in `beforeEach` for these tests to avoid flakiness from Svelte’s internal tick scheduling; do not remove or flag it based on the presence/absence of explicit timer manipulation in individual tests.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.ts
📚 Learning: 2026-06-16T06:15:26.225Z
Learnt from: gastoner
Repo: openkaiden/kaiden PR: 2145
File: packages/renderer/src/lib/mcp/MCPServerRemoteListActions.spec.ts:46-49
Timestamp: 2026-06-16T06:15:26.225Z
Learning: For renderer unit tests in this repo (files included in `packages/renderer/vite.config.js` via `setupFiles`), do not manually mock `window` API methods in `beforeEach` or within the spec. The test setup (`packages/renderer/vite.tests.setup.js`) reads `packages/preload/exposedInMainWorld.d.ts` and defines each declared `Window` method on `window` as a `vi.fn()` using `Object.defineProperty`, so `window.*` methods (e.g., `startMcpServer`, `stopMcpServer`, `showMessageBox`) are already present as mocks. Only add an explicit mock if the method is not declared in `exposedInMainWorld.d.ts` (in which case update the `.d.ts` so the shared setup can generate the mock).
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.ts
📚 Learning: 2026-03-17T11:49:39.964Z
Learnt from: MarsKubeX
Repo: kortex-hub/kortex PR: 1111
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCard.svelte:46-52
Timestamp: 2026-03-17T11:49:39.964Z
Learning: In Svelte components like AgentWorkspaceCard.svelte and CustomPick.svelte, using a div with role="button" and tabindex="0" as the clickable card container with an inner native <button> (e.g., for a remove action) is acceptable because nested <button> elements are invalid per HTML spec. Ensure the inner button's events do not bubble by calling stopPropagation on both click and keydown handlers. Do not flag this pattern as an accessibility issue when implemented this way, but verify that keyboard activation (Enter/Space) and ARIA semantics are preserved and that focus management remains clear.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-04-15T08:04:32.031Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1336
File: packages/renderer/src/lib/guided-setup/GuidedSetup.svelte:9-11
Timestamp: 2026-04-15T08:04:32.031Z
Learning: For Svelte components in this repo, if a callback prop is typed as `() => void`, TypeScript idiomatically allows passing async functions (e.g., `() => Promise<void>`), because `() => void` indicates the caller ignores the return value rather than requiring `undefined`. Do not recommend changing these prop types to `() => void | Promise<void>` solely to “fix” async compatibility—unless there is an actual need for the caller to observe the returned value.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-04-28T13:34:51.610Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1431
File: packages/renderer/src/lib/guided-setup/panels/OpenCodePanel.svelte:14-20
Timestamp: 2026-04-28T13:34:51.610Z
Learning: In this repo’s Svelte renderer (packages/renderer/src/**/*.svelte), `podman-desktop/ui-svelte`’s `Link` component does not accept an `href` prop. For opening external URLs, use the established pattern `on:click={() => window.openExternal(url)}` (optionally typed as `on:click={(): Promise<void> => window.openExternal(url)}`), consistent with existing components like `ProviderLinks.svelte`, `WelcomePage.svelte`, and `OpenCodePanel.svelte`. Do not treat `window.openExternal()` usage in renderer Svelte components as bypassing a security restriction, and do not recommend replacing it with `Link`/`href`-based navigation. (There is also no `setupSecurityRestrictionsOnLinks` utility in the renderer.)
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-04-29T11:54:22.423Z
Learnt from: vancura
Repo: openkaiden/kaiden PR: 1494
File: packages/renderer/src/Loader.svelte:13-13
Timestamp: 2026-04-29T11:54:22.423Z
Learning: In this codebase’s renderer, timer variables in Svelte files (e.g., in `packages/renderer/src/Loader.svelte`) that store `setTimeout` handles are intentionally typed as `NodeJS.Timeout` (and not `ReturnType<typeof setTimeout>`). When reviewing, do not flag or recommend changing this to `ReturnType<typeof setTimeout>` for portability—treat `NodeJS.Timeout` as the established convention for renderer timer variables.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-07-14T11:55:38.534Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 2454
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceTerminal.svelte:143-160
Timestamp: 2026-07-14T11:55:38.534Z
Learning: In this Electron app, the main process prevents renderer `beforeunload` from firing on window close/app quit/navigation by calling `e.preventDefault()` on `browserWindow` `close` and then either `hide()` (no unload) or `destroy()` (bypasses `beforeunload`), and `app.on('before-quit')` also calls `browserWindow.destroy()`. As a result, renderer `beforeunload` (and any “reload confirmation” handler) should only run for an explicit page reload (e.g., `location.reload()`) and not for close/quit actions. During review, do NOT flag hardcoded `location.reload()` calls inside `beforeunload`/reload-confirmation handlers as “hijacking” close/quit behavior.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-06-10T15:22:39.639Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 2115
File: packages/renderer/src/lib/models/SemanticRouterCreate.svelte:134-137
Timestamp: 2026-06-10T15:22:39.639Z
Learning: In multi-step wizard UIs, it’s acceptable for “Next step” buttons to be temporarily non-interactive during incremental work delivered across multiple PRs. When reviewing Svelte files under `packages/renderer/src/lib/**`, do not flag these UX issues if the button lacks an onClick handler *and* there is an explicit TODO comment indicating deferred wiring, and the PR author confirms (in the PR description) that this is an intentional staged implementation. If no TODO/deferred-wiring marker (or no author confirmation) is present, treat missing handlers as a potential UX issue.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
🔇 Additional comments (8)
packages/api/src/agent-workspace-info.ts (1)
61-70: LGTM!packages/api/src/agent-workspace-info.spec.ts (1)
19-35: LGTM!packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.svelte (2)
55-67: LGTM!
147-152: LGTM!packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.ts (1)
106-134: LGTM!packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.ts (1)
264-287: LGTM!packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (1)
199-202: LGTM!packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts (1)
376-397: LGTM!
…back Merge main and combine hostname-length validation with duplicate-name checks. Use the effective workspace name when creating workspaces from the wizard. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: serbangeorge-m <serbangeorge.m@gmail.com>
serbangeorge-m
enabled auto-merge (squash)
July 15, 2026 12:45
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
|
I've read that DNS hostname limit is 63 chars, but in practice, I can't create an openshell workspace with a name bigger than 56 chars. 57 fails:
56 pass:
|
Contributor
Author
ok, I can set the limit to 56 |
Use a conservative OpenShell/Podman hostname limit based on observed sandbox creation failures above 56 characters. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: serbangeorge-m <serbangeorge.m@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
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)
411-419: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEnsure basename derivation handles Windows paths correctly.
Using
.split('/')will fail to extract the basename from Windows paths (e.g.,C:\path\to\folder), causing the entire host path to be mapped as the target folder name in the sandbox. Use the cross-platform regex pattern to support both separators, matching the logic ingetDefaultSessionName().🐛 Proposed fix
case 'custom': { const filtered = mounts .filter(m => m.host.trim() !== '') .map(m => { const host = m.host.trim(); const trimmedTarget = m.target.trim(); - const basename = host.split('/').filter(Boolean).pop(); + const basename = host.replace(/[\\/]+$/, '').split(/[\\/]/).filter(Boolean).pop(); const target = trimmedTarget !== '' ? trimmedTarget : (basename ?? host); return { host, target, ro: m.ro }; }); return filtered.length > 0 ? filtered : undefined;🤖 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/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte` around lines 411 - 419, Update basename derivation in the mounts mapping logic to split host paths using the same cross-platform separator pattern as getDefaultSessionName(), supporting both forward and backward slashes. Preserve the existing fallback behavior for empty targets and hosts without a basename.
🤖 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 `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts`:
- Around line 216-256: Remove the temporary performance timing instrumentation
from the workspace creation flow: delete the performance.now() variables and all
related [workspace-timing] console.log calls surrounding enableV2Provider,
createSandbox, and updatePolicy, while preserving the existing OpenShell
operations and error handling.
In `@packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte`:
- Around line 265-272: Update the validationErrors derived block to compute the
effective workspace name from wizard.draft.sessionName, falling back to the name
derived from sourcePath when the input is blank, then use that value for the
duplicate check against $allOpenshellSandboxes. Preserve the existing error
message and explicit-name behavior.
---
Outside diff comments:
In `@packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte`:
- Around line 411-419: Update basename derivation in the mounts mapping logic to
split host paths using the same cross-platform separator pattern as
getDefaultSessionName(), supporting both forward and backward slashes. Preserve
the existing fallback behavior for empty targets and hosts without a basename.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e7bca515-28c6-4feb-ab0f-26e03b90fc0a
📒 Files selected for processing (7)
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/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.svelte
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: unit tests / windows-2022
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: unit tests / macos-15
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: macOS
- GitHub Check: linter, formatters
- GitHub Check: Linux
- GitHub Check: typecheck
- GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
/@/path aliases instead of relative paths for imports outside the current directory's module group; use relative imports only for sibling modules within the same directory
Files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/api/src/agent-workspace-info.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
**/*.spec.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.{ts,tsx,js,jsx}: Usetest()instead ofit()for test cases in Vitest unit tests
Usevi.mock(import('...'))for auto-mocking modules in unit tests; avoid manual mock factories when possible
Usevi.resetAllMocks()inbeforeEachhooks instead ofvi.clearAllMocks()for resetting mocks between tests
When an auto-mocked function or class method needs a real implementation, usevi.mocked(...)with the prototype pattern for class methods:vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)
Files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
packages/{main,renderer,preload}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Container operations must include
engineIdparameter to identify the container engine
Files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
packages/main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/main/src/**/*.{ts,tsx}: UseipcHandle()to expose handlers in the main process with naming convention<registry-name>:<action>(e.g.,container-provider-registry:listContainers)
UseapiSender.send()to send events from main process to renderer for real-time updates
Long-running operations should useTaskManager.createTask()with title and action configuration
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
🧠 Learnings (11)
📚 Learning: 2026-04-15T08:51:08.199Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1336
File: packages/renderer/src/lib/guided-setup/GuidedSetup.spec.ts:64-69
Timestamp: 2026-04-15T08:51:08.199Z
Learning: For Svelte component test files under `packages/renderer/src/**` with names ending in `.spec.ts` that use `render` from `testing-library/svelte`, call `vi.useFakeTimers({ shouldAdvanceTime: true })` inside a `beforeEach`. Keep it in `beforeEach` for these tests to avoid flakiness from Svelte’s internal tick scheduling; do not remove or flag it based on the presence/absence of explicit timer manipulation in individual tests.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.ts
📚 Learning: 2026-05-12T17:14:02.153Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1850
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte:66-70
Timestamp: 2026-05-12T17:14:02.153Z
Learning: When reviewing code that uses `AgentWorkspaceSummaryUI.runtime`, treat it as a required, non-null `string` per the `openkaiden/kdn-api` 0.12.0 schema. Therefore, code like `a.runtime.localeCompare(b.runtime)` is safe and should not trigger warnings about possible `undefined`/`null` values or suggestions to use nullish coalescing/optional chaining for `runtime` (unless the current local types still mark `runtime` as optional, indicating a schema/version mismatch).
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/api/src/agent-workspace-info.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.sveltepackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-06-16T06:15:26.225Z
Learnt from: gastoner
Repo: openkaiden/kaiden PR: 2145
File: packages/renderer/src/lib/mcp/MCPServerRemoteListActions.spec.ts:46-49
Timestamp: 2026-06-16T06:15:26.225Z
Learning: For renderer unit tests in this repo (files included in `packages/renderer/vite.config.js` via `setupFiles`), do not manually mock `window` API methods in `beforeEach` or within the spec. The test setup (`packages/renderer/vite.tests.setup.js`) reads `packages/preload/exposedInMainWorld.d.ts` and defines each declared `Window` method on `window` as a `vi.fn()` using `Object.defineProperty`, so `window.*` methods (e.g., `startMcpServer`, `stopMcpServer`, `showMessageBox`) are already present as mocks. Only add an explicit mock if the method is not declared in `exposedInMainWorld.d.ts` (in which case update the `.d.ts` so the shared setup can generate the mock).
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.ts
📚 Learning: 2026-06-29T13:16:53.102Z
Learnt from: benoitf
Repo: openkaiden/kaiden PR: 2296
File: extensions/container/packages/extension/src/helper/socket-finder/_socket-finder-module.ts:28-29
Timestamp: 2026-06-29T13:16:53.102Z
Learning: When reviewing imports in openkaiden/kaiden TypeScript/JavaScript files, prefer the configured `/@/` path alias instead of relative imports that would require traversing out of the current directory/module group (i.e., paths containing `..` that cross boundaries).
Do not require alias conversion for descendant-path relative imports within the socket-finder module directory—for example, in `extensions/container/packages/extension/src/helper/socket-finder/**`, imports like `./podman/podman-version-detector` and `./podman/podman-windows-finder` are acceptable and should not be flagged.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/api/src/agent-workspace-info.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
📚 Learning: 2026-03-09T08:47:09.657Z
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 1077
File: packages/main/src/plugin/skill/skill-manager.ts:80-109
Timestamp: 2026-03-09T08:47:09.657Z
Learning: In the kortex-hub/kortex repository, IPC handlers (via ipcHandle()) may be registered directly inside feature manager/service classes (e.g., SkillManager in packages/main/src/plugin/skill/skill-manager.ts) rather than exclusively in packages/main/src/plugin/index.ts. Treat this as an accepted design pattern for files under the plugin directory. Reviewers should not require centralization in index.ts; allow IPC registration proximity to the feature that owns the handler. When reviewing code, accept direct ipcHandle() registrations inside feature managers and ensure the pattern is consistently applied across similar feature-manager modules.
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
📚 Learning: 2026-03-17T11:49:39.964Z
Learnt from: MarsKubeX
Repo: kortex-hub/kortex PR: 1111
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCard.svelte:46-52
Timestamp: 2026-03-17T11:49:39.964Z
Learning: In Svelte components like AgentWorkspaceCard.svelte and CustomPick.svelte, using a div with role="button" and tabindex="0" as the clickable card container with an inner native <button> (e.g., for a remove action) is acceptable because nested <button> elements are invalid per HTML spec. Ensure the inner button's events do not bubble by calling stopPropagation on both click and keydown handlers. Do not flag this pattern as an accessibility issue when implemented this way, but verify that keyboard activation (Enter/Space) and ARIA semantics are preserved and that focus management remains clear.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-04-15T08:04:32.031Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1336
File: packages/renderer/src/lib/guided-setup/GuidedSetup.svelte:9-11
Timestamp: 2026-04-15T08:04:32.031Z
Learning: For Svelte components in this repo, if a callback prop is typed as `() => void`, TypeScript idiomatically allows passing async functions (e.g., `() => Promise<void>`), because `() => void` indicates the caller ignores the return value rather than requiring `undefined`. Do not recommend changing these prop types to `() => void | Promise<void>` solely to “fix” async compatibility—unless there is an actual need for the caller to observe the returned value.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-04-28T13:34:51.610Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1431
File: packages/renderer/src/lib/guided-setup/panels/OpenCodePanel.svelte:14-20
Timestamp: 2026-04-28T13:34:51.610Z
Learning: In this repo’s Svelte renderer (packages/renderer/src/**/*.svelte), `podman-desktop/ui-svelte`’s `Link` component does not accept an `href` prop. For opening external URLs, use the established pattern `on:click={() => window.openExternal(url)}` (optionally typed as `on:click={(): Promise<void> => window.openExternal(url)}`), consistent with existing components like `ProviderLinks.svelte`, `WelcomePage.svelte`, and `OpenCodePanel.svelte`. Do not treat `window.openExternal()` usage in renderer Svelte components as bypassing a security restriction, and do not recommend replacing it with `Link`/`href`-based navigation. (There is also no `setupSecurityRestrictionsOnLinks` utility in the renderer.)
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-04-29T11:54:22.423Z
Learnt from: vancura
Repo: openkaiden/kaiden PR: 1494
File: packages/renderer/src/Loader.svelte:13-13
Timestamp: 2026-04-29T11:54:22.423Z
Learning: In this codebase’s renderer, timer variables in Svelte files (e.g., in `packages/renderer/src/Loader.svelte`) that store `setTimeout` handles are intentionally typed as `NodeJS.Timeout` (and not `ReturnType<typeof setTimeout>`). When reviewing, do not flag or recommend changing this to `ReturnType<typeof setTimeout>` for portability—treat `NodeJS.Timeout` as the established convention for renderer timer variables.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-07-14T11:55:38.534Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 2454
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceTerminal.svelte:143-160
Timestamp: 2026-07-14T11:55:38.534Z
Learning: In this Electron app, the main process prevents renderer `beforeunload` from firing on window close/app quit/navigation by calling `e.preventDefault()` on `browserWindow` `close` and then either `hide()` (no unload) or `destroy()` (bypasses `beforeunload`), and `app.on('before-quit')` also calls `browserWindow.destroy()`. As a result, renderer `beforeunload` (and any “reload confirmation” handler) should only run for an explicit page reload (e.g., `location.reload()`) and not for close/quit actions. During review, do NOT flag hardcoded `location.reload()` calls inside `beforeunload`/reload-confirmation handlers as “hijacking” close/quit behavior.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-06-10T15:22:39.639Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 2115
File: packages/renderer/src/lib/models/SemanticRouterCreate.svelte:134-137
Timestamp: 2026-06-10T15:22:39.639Z
Learning: In multi-step wizard UIs, it’s acceptable for “Next step” buttons to be temporarily non-interactive during incremental work delivered across multiple PRs. When reviewing Svelte files under `packages/renderer/src/lib/**`, do not flag these UX issues if the button lacks an onClick handler *and* there is an explicit TODO comment indicating deferred wiring, and the PR author confirms (in the PR description) that this is an intentional staged implementation. If no TODO/deferred-wiring marker (or no author confirmation) is present, treat missing handlers as a potential UX issue.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
🔇 Additional comments (5)
packages/api/src/agent-workspace-info.ts (1)
61-68: LGTM!packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.svelte (1)
58-68: LGTM!Also applies to: 149-150
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.ts (1)
106-135: LGTM!Also applies to: 189-203
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.ts (1)
268-291: LGTM!packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts (1)
378-399: LGTM!
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
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)
411-419: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEnsure basename derivation handles Windows paths correctly.
Using
.split('/')will fail to extract the basename from Windows paths (e.g.,C:\path\to\folder), causing the entire host path to be mapped as the target folder name in the sandbox. Use the cross-platform regex pattern to support both separators, matching the logic ingetDefaultSessionName().🐛 Proposed fix
case 'custom': { const filtered = mounts .filter(m => m.host.trim() !== '') .map(m => { const host = m.host.trim(); const trimmedTarget = m.target.trim(); - const basename = host.split('/').filter(Boolean).pop(); + const basename = host.replace(/[\\/]+$/, '').split(/[\\/]/).filter(Boolean).pop(); const target = trimmedTarget !== '' ? trimmedTarget : (basename ?? host); return { host, target, ro: m.ro }; }); return filtered.length > 0 ? filtered : undefined;🤖 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/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte` around lines 411 - 419, Update basename derivation in the mounts mapping logic to split host paths using the same cross-platform separator pattern as getDefaultSessionName(), supporting both forward and backward slashes. Preserve the existing fallback behavior for empty targets and hosts without a basename.
🤖 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 `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts`:
- Around line 216-256: Remove the temporary performance timing instrumentation
from the workspace creation flow: delete the performance.now() variables and all
related [workspace-timing] console.log calls surrounding enableV2Provider,
createSandbox, and updatePolicy, while preserving the existing OpenShell
operations and error handling.
In `@packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte`:
- Around line 265-272: Update the validationErrors derived block to compute the
effective workspace name from wizard.draft.sessionName, falling back to the name
derived from sourcePath when the input is blank, then use that value for the
duplicate check against $allOpenshellSandboxes. Preserve the existing error
message and explicit-name behavior.
---
Outside diff comments:
In `@packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte`:
- Around line 411-419: Update basename derivation in the mounts mapping logic to
split host paths using the same cross-platform separator pattern as
getDefaultSessionName(), supporting both forward and backward slashes. Preserve
the existing fallback behavior for empty targets and hosts without a basename.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e7bca515-28c6-4feb-ab0f-26e03b90fc0a
📒 Files selected for processing (7)
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/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.svelte
📜 Review details
🔇 Additional comments (5)
packages/api/src/agent-workspace-info.ts (1)
61-68: LGTM!packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.svelte (1)
58-68: LGTM!Also applies to: 149-150
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreateStepWorkspace.spec.ts (1)
106-135: LGTM!Also applies to: 189-203
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.ts (1)
268-291: LGTM!packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts (1)
378-399: LGTM!
🛑 Comments failed to post (2)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (1)
216-256: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Remove debug timing artifacts.
The
performance.now()measurements andconsole.logstatements appear to be temporary debug artifacts for tracing OpenShell creation phases. Please consider removing them to clean up the production logs.♻️ Proposed refactor
- const t0 = performance.now(); - const v2Globally = await this.openshellCli.isV2ProviderEnabled(); if (!v2Globally) { await this.openshellCli.enableV2Provider(); } - const tV2 = performance.now(); - console.log(`[workspace-timing] enableV2Provider: ${(tV2 - t0).toFixed(0)}ms`); - await this.openshellCli.createSandbox({ name: sandboxName, from: agent.baseImage, providers: options.secrets, env: env && Object.keys(env).length > 0 ? env : undefined, labels: { ...encodeWorkspaceLabels(options.sourcePath), [AGENT_LABEL]: options.agent }, uploads: dedupedUploads.length > 0 ? dedupedUploads : undefined, noTty: true, command: ['true'], }); - const tSandbox = performance.now(); - console.log(`[workspace-timing] createSandbox: ${(tSandbox - tV2).toFixed(0)}ms`); - const networkPolicy = buildPolicyObject(workspace.network, endpoint); if (networkPolicy) { const endpointFlags = collectEndpointFlags(networkPolicy); if (endpointFlags.length > 0) { try { await this.openshellCli.updatePolicy(sandboxName, endpointFlags, collectBinaryFlags(networkPolicy)); } catch (err) { await this.openshellCli.deleteSandbox(sandboxName).catch(() => {}); throw err; } } } - const tPolicy = performance.now(); - console.log(`[workspace-timing] updatePolicy: ${(tPolicy - tSandbox).toFixed(0)}ms`); - console.log(`[workspace-timing] total createOpenshell: ${(tPolicy - t0).toFixed(0)}ms`); - return { id: sandboxName }; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const v2Globally = await this.openshellCli.isV2ProviderEnabled(); if (!v2Globally) { await this.openshellCli.enableV2Provider(); } await this.openshellCli.createSandbox({ name: sandboxName, from: agent.baseImage, providers: options.secrets, env: env && Object.keys(env).length > 0 ? env : undefined, labels: { ...encodeWorkspaceLabels(options.sourcePath), [AGENT_LABEL]: options.agent }, uploads: dedupedUploads.length > 0 ? dedupedUploads : undefined, noTty: true, command: ['true'], }); const networkPolicy = buildPolicyObject(workspace.network, endpoint); if (networkPolicy) { const endpointFlags = collectEndpointFlags(networkPolicy); if (endpointFlags.length > 0) { try { await this.openshellCli.updatePolicy(sandboxName, endpointFlags, collectBinaryFlags(networkPolicy)); } catch (err) { await this.openshellCli.deleteSandbox(sandboxName).catch(() => {}); throw err; } } } return { id: sandboxName }; }🤖 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/main/src/plugin/agent-workspace/agent-workspace-manager.ts` around lines 216 - 256, Remove the temporary performance timing instrumentation from the workspace creation flow: delete the performance.now() variables and all related [workspace-timing] console.log calls surrounding enableV2Provider, createSandbox, and updatePolicy, while preserving the existing OpenShell operations and error handling.packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte (1)
265-272: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate duplicate names against the effective fallback name.
If the user clears the name input, the backend relies on the default name derived from
sourcePath. Currently, this duplicate-name validation block only checkswizard.draft.sessionName. If the fallback name conflicts with an existing sandbox, the UI will incorrectly allow progression, resulting in a backend failure.Check the effective workspace name to ensure both explicit and fallback names are validated.
🐛 Proposed fix
let validationErrors = $derived.by(() => { const errors: { name?: string } = {}; - const name = wizard.draft.sessionName.trim().toLowerCase(); + const name = getEffectiveWorkspaceName().toLowerCase(); if (name && $allOpenshellSandboxes.some(s => s.name.toLowerCase() === name)) { errors.name = 'A workspace with this name already exists. Please choose a different name.'; } return errors; });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.let validationErrors = $derived.by(() => { const errors: { name?: string } = {}; const name = getEffectiveWorkspaceName().toLowerCase(); if (name && $allOpenshellSandboxes.some(s => s.name.toLowerCase() === name)) { errors.name = 'A workspace with this name already exists. Please choose a different name.'; } return errors; });🤖 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/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte` around lines 265 - 272, Update the validationErrors derived block to compute the effective workspace name from wizard.draft.sessionName, falling back to the name derived from sourcePath when the input is blank, then use that value for the duplicate check against $allOpenshellSandboxes. Preserve the existing error message and explicit-name behavior.
serbangeorge-m
disabled auto-merge
July 16, 2026 07:43
benoitf
reviewed
Jul 17, 2026
| export type AgentWorkspaceMount = configComponents['schemas']['Mount']; | ||
|
|
||
| /** Maximum sandbox name length for OpenShell. Podman/crun hostname composition leaves a conservative 56-char limit. */ | ||
| export const SANDBOX_NAME_MAX_LENGTH = 56; |
Contributor
There was a problem hiding this comment.
question:should it be a configuration setting rather than an hardcoded value ?
another question: should it depend on the driver being used ?
Contributor
Author
There was a problem hiding this comment.
I think it can be dynamic, yes. Should I update it?
Contributor
Author
There was a problem hiding this comment.
or we can leave it like this for now and create a follow up ticket to make it dynamic as it will require more testing? I'm ok with both options. What do you think?
MarsKubeX
approved these changes
Jul 20, 2026
MarsKubeX
left a comment
MarsKubeX
left a comment
Contributor
There was a problem hiding this comment.
LGTM. Just one thing, when creating a workspace with empty name, the workspace name default is the project folder name. If creating 2 workspace with empty name for the same project folder, the "A workspace with this name already exists. Please choose a different name." error won't be displayed. This could be solved by forcing to be a non-empty field. Could be checked in a follow-up issue since it's not fully related to the issue.
Contributor
|
@MarsKubeX can be dealt with as part of #1250 |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte (1)
427-434: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGuard against an empty effective workspace name in
startAsIs.The
startAsIsearly return omits the check for!getEffectiveWorkspaceName(). If a user selects a root path like/(wheregetDefaultSessionNameyields"") and leaves the name input empty, the validation passes because an empty string is less than 56 characters. This results in the workspace creation payload sendingname: "", which bypasses the backend's nullish coalescing fallback (??) and attempts to create a sandbox with an empty name.Add the missing
!getEffectiveWorkspaceName()check to align withstartWorkspace.🐛 Proposed fix
async function startAsIs(): Promise<void> { if ( + !getEffectiveWorkspaceName() || !wizard.draft.sourcePath.trim() || !wizard.draft.selectedModel || !isWorkspaceNameValid() ||🤖 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/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte` around lines 427 - 434, Add the missing !getEffectiveWorkspaceName() guard to the startAsIs early-return validation alongside the existing draft and validation checks, matching startWorkspace so an empty effective workspace name cannot reach workspace creation.packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (1)
775-775: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix Biome lint error by using a
for...ofloop.The
useIterableCallbackReturnlint rule complains when a concise arrow function conceptually returns a value in an iterable method likeforEach, even if the underlying function (dispose()) returnsvoid. Using afor...ofloop resolves the static analysis warning and is slightly cleaner.♻️ Proposed fix
- this.disposables.forEach(disposable => disposable.dispose()); + for (const disposable of this.disposables) { + disposable.dispose(); + }🤖 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/main/src/plugin/agent-workspace/agent-workspace-manager.ts` at line 775, Replace the forEach call in the disposal logic with a for...of loop over this.disposables, invoking disposable.dispose() for each item while preserving the existing disposal order and behavior.Source: Linters/SAST tools
🤖 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.
Outside diff comments:
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts`:
- Line 775: Replace the forEach call in the disposal logic with a for...of loop
over this.disposables, invoking disposable.dispose() for each item while
preserving the existing disposal order and behavior.
In `@packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte`:
- Around line 427-434: Add the missing !getEffectiveWorkspaceName() guard to the
startAsIs early-return validation alongside the existing draft and validation
checks, matching startWorkspace so an empty effective workspace name cannot
reach workspace creation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8d1b12dd-dfbf-40fa-bf17-dbec7b2c4dfe
📒 Files selected for processing (5)
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/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: unit tests / macos-15
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: macOS
- GitHub Check: unit tests / windows-2022
- GitHub Check: typecheck
- GitHub Check: linter, formatters
- GitHub Check: Linux
- GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
/@/path aliases instead of relative paths for imports outside the current directory's module group; use relative imports only for sibling modules within the same directory
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/api/src/agent-workspace-info.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.ts
**/*.spec.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.{ts,tsx,js,jsx}: Usetest()instead ofit()for test cases in Vitest unit tests
Usevi.mock(import('...'))for auto-mocking modules in unit tests; avoid manual mock factories when possible
Usevi.resetAllMocks()inbeforeEachhooks instead ofvi.clearAllMocks()for resetting mocks between tests
When an auto-mocked function or class method needs a real implementation, usevi.mocked(...)with the prototype pattern for class methods:vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.ts
packages/main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/main/src/**/*.{ts,tsx}: UseipcHandle()to expose handlers in the main process with naming convention<registry-name>:<action>(e.g.,container-provider-registry:listContainers)
UseapiSender.send()to send events from main process to renderer for real-time updates
Long-running operations should useTaskManager.createTask()with title and action configuration
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
packages/{main,renderer,preload}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Container operations must include
engineIdparameter to identify the container engine
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.ts
🧠 Learnings (11)
📚 Learning: 2026-03-09T08:47:09.657Z
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 1077
File: packages/main/src/plugin/skill/skill-manager.ts:80-109
Timestamp: 2026-03-09T08:47:09.657Z
Learning: In the kortex-hub/kortex repository, IPC handlers (via ipcHandle()) may be registered directly inside feature manager/service classes (e.g., SkillManager in packages/main/src/plugin/skill/skill-manager.ts) rather than exclusively in packages/main/src/plugin/index.ts. Treat this as an accepted design pattern for files under the plugin directory. Reviewers should not require centralization in index.ts; allow IPC registration proximity to the feature that owns the handler. When reviewing code, accept direct ipcHandle() registrations inside feature managers and ensure the pattern is consistently applied across similar feature-manager modules.
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
📚 Learning: 2026-05-12T17:14:02.153Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1850
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte:66-70
Timestamp: 2026-05-12T17:14:02.153Z
Learning: When reviewing code that uses `AgentWorkspaceSummaryUI.runtime`, treat it as a required, non-null `string` per the `openkaiden/kdn-api` 0.12.0 schema. Therefore, code like `a.runtime.localeCompare(b.runtime)` is safe and should not trigger warnings about possible `undefined`/`null` values or suggestions to use nullish coalescing/optional chaining for `runtime` (unless the current local types still mark `runtime` as optional, indicating a schema/version mismatch).
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/api/src/agent-workspace-info.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-06-29T13:16:53.102Z
Learnt from: benoitf
Repo: openkaiden/kaiden PR: 2296
File: extensions/container/packages/extension/src/helper/socket-finder/_socket-finder-module.ts:28-29
Timestamp: 2026-06-29T13:16:53.102Z
Learning: When reviewing imports in openkaiden/kaiden TypeScript/JavaScript files, prefer the configured `/@/` path alias instead of relative imports that would require traversing out of the current directory/module group (i.e., paths containing `..` that cross boundaries).
Do not require alias conversion for descendant-path relative imports within the socket-finder module directory—for example, in `extensions/container/packages/extension/src/helper/socket-finder/**`, imports like `./podman/podman-version-detector` and `./podman/podman-windows-finder` are acceptable and should not be flagged.
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/api/src/agent-workspace-info.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.ts
📚 Learning: 2026-04-15T08:51:08.199Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1336
File: packages/renderer/src/lib/guided-setup/GuidedSetup.spec.ts:64-69
Timestamp: 2026-04-15T08:51:08.199Z
Learning: For Svelte component test files under `packages/renderer/src/**` with names ending in `.spec.ts` that use `render` from `testing-library/svelte`, call `vi.useFakeTimers({ shouldAdvanceTime: true })` inside a `beforeEach`. Keep it in `beforeEach` for these tests to avoid flakiness from Svelte’s internal tick scheduling; do not remove or flag it based on the presence/absence of explicit timer manipulation in individual tests.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.ts
📚 Learning: 2026-06-16T06:15:26.225Z
Learnt from: gastoner
Repo: openkaiden/kaiden PR: 2145
File: packages/renderer/src/lib/mcp/MCPServerRemoteListActions.spec.ts:46-49
Timestamp: 2026-06-16T06:15:26.225Z
Learning: For renderer unit tests in this repo (files included in `packages/renderer/vite.config.js` via `setupFiles`), do not manually mock `window` API methods in `beforeEach` or within the spec. The test setup (`packages/renderer/vite.tests.setup.js`) reads `packages/preload/exposedInMainWorld.d.ts` and defines each declared `Window` method on `window` as a `vi.fn()` using `Object.defineProperty`, so `window.*` methods (e.g., `startMcpServer`, `stopMcpServer`, `showMessageBox`) are already present as mocks. Only add an explicit mock if the method is not declared in `exposedInMainWorld.d.ts` (in which case update the `.d.ts` so the shared setup can generate the mock).
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.spec.ts
📚 Learning: 2026-03-17T11:49:39.964Z
Learnt from: MarsKubeX
Repo: kortex-hub/kortex PR: 1111
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCard.svelte:46-52
Timestamp: 2026-03-17T11:49:39.964Z
Learning: In Svelte components like AgentWorkspaceCard.svelte and CustomPick.svelte, using a div with role="button" and tabindex="0" as the clickable card container with an inner native <button> (e.g., for a remove action) is acceptable because nested <button> elements are invalid per HTML spec. Ensure the inner button's events do not bubble by calling stopPropagation on both click and keydown handlers. Do not flag this pattern as an accessibility issue when implemented this way, but verify that keyboard activation (Enter/Space) and ARIA semantics are preserved and that focus management remains clear.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-04-15T08:04:32.031Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1336
File: packages/renderer/src/lib/guided-setup/GuidedSetup.svelte:9-11
Timestamp: 2026-04-15T08:04:32.031Z
Learning: For Svelte components in this repo, if a callback prop is typed as `() => void`, TypeScript idiomatically allows passing async functions (e.g., `() => Promise<void>`), because `() => void` indicates the caller ignores the return value rather than requiring `undefined`. Do not recommend changing these prop types to `() => void | Promise<void>` solely to “fix” async compatibility—unless there is an actual need for the caller to observe the returned value.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-04-28T13:34:51.610Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1431
File: packages/renderer/src/lib/guided-setup/panels/OpenCodePanel.svelte:14-20
Timestamp: 2026-04-28T13:34:51.610Z
Learning: In this repo’s Svelte renderer (packages/renderer/src/**/*.svelte), `podman-desktop/ui-svelte`’s `Link` component does not accept an `href` prop. For opening external URLs, use the established pattern `on:click={() => window.openExternal(url)}` (optionally typed as `on:click={(): Promise<void> => window.openExternal(url)}`), consistent with existing components like `ProviderLinks.svelte`, `WelcomePage.svelte`, and `OpenCodePanel.svelte`. Do not treat `window.openExternal()` usage in renderer Svelte components as bypassing a security restriction, and do not recommend replacing it with `Link`/`href`-based navigation. (There is also no `setupSecurityRestrictionsOnLinks` utility in the renderer.)
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-04-29T11:54:22.423Z
Learnt from: vancura
Repo: openkaiden/kaiden PR: 1494
File: packages/renderer/src/Loader.svelte:13-13
Timestamp: 2026-04-29T11:54:22.423Z
Learning: In this codebase’s renderer, timer variables in Svelte files (e.g., in `packages/renderer/src/Loader.svelte`) that store `setTimeout` handles are intentionally typed as `NodeJS.Timeout` (and not `ReturnType<typeof setTimeout>`). When reviewing, do not flag or recommend changing this to `ReturnType<typeof setTimeout>` for portability—treat `NodeJS.Timeout` as the established convention for renderer timer variables.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-07-14T11:55:38.534Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 2454
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceTerminal.svelte:143-160
Timestamp: 2026-07-14T11:55:38.534Z
Learning: In this Electron app, the main process prevents renderer `beforeunload` from firing on window close/app quit/navigation by calling `e.preventDefault()` on `browserWindow` `close` and then either `hide()` (no unload) or `destroy()` (bypasses `beforeunload`), and `app.on('before-quit')` also calls `browserWindow.destroy()`. As a result, renderer `beforeunload` (and any “reload confirmation” handler) should only run for an explicit page reload (e.g., `location.reload()`) and not for close/quit actions. During review, do NOT flag hardcoded `location.reload()` calls inside `beforeunload`/reload-confirmation handlers as “hijacking” close/quit behavior.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-06-10T15:22:39.639Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 2115
File: packages/renderer/src/lib/models/SemanticRouterCreate.svelte:134-137
Timestamp: 2026-06-10T15:22:39.639Z
Learning: In multi-step wizard UIs, it’s acceptable for “Next step” buttons to be temporarily non-interactive during incremental work delivered across multiple PRs. When reviewing Svelte files under `packages/renderer/src/lib/**`, do not flag these UX issues if the button lacks an onClick handler *and* there is an explicit TODO comment indicating deferred wiring, and the PR author confirms (in the PR description) that this is an intentional staged implementation. If no TODO/deferred-wiring marker (or no author confirmation) is present, treat missing handlers as a potential UX issue.
Applied to files:
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
🪛 Biome (2.5.3)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
[error] 775-775: This callback passed to forEach() iterable method should not return a value.
(lint/suspicious/useIterableCallbackReturn)
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
4 participants
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
AgentWorkspaceManagerfor explicit and basename-derived namesFixes #2462
Test plan
Made with Cursor