fix: show workspaces during creation and deletion#2119
Draft
fbricon wants to merge 1 commit into
Draft
Conversation
fbricon
requested review from
benoitf,
jeffmaury and
slemeur
and removed request for
a team
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBackend tracks in-progress workspace creates and deletes and synthesizes transient "creating"/"deleting" entries in list(); renderer types, creation form, components, and utilities are updated to show progress, disable interactions, and preserve original state during deletion. ChangesWorkspace Lifecycle State Visibility
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 112-120: The current inProgressWorkspaces map is keyed only by
workspaceName so concurrent operations overwrite each other and a later finally
can clear state for a still-running operation; update create/delete lifecycle
tracking in agent-workspace-manager.ts to guard against overlapping requests by
adding an operation token (e.g., a unique id) stored alongside the workspace
entry in this.inProgressWorkspaces and only clearing/removing when the token
matches the current invocation, or alternatively throw/reject if an entry
already exists; apply the same change where inProgressWorkspaces is written/read
(the block that sets workspaceName with startedAt, the cleanup in finally, and
the code paths around remove() and apiSender.send('agent-workspace-update')) so
the UI state and the non-idempotent remove() call are only affected by the
matching operation token.
In `@packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte`:
- Around line 370-402: The draft is reset immediately after calling
window.createAgentWorkspace, which discards draftSnapshot before the background
promise resolves; move the resetDraft() call into the success path of
window.createAgentWorkspace (i.e., run resetDraft() after the promise fulfills)
and do not clear the draft in the .catch() handler—or alternatively capture and
restore draftSnapshot inside the .catch() so user inputs remain available for
retry; locate the create call and adjust placement of resetDraft() around
window.createAgentWorkspace and its .catch() handler accordingly.
In
`@packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.svelte`:
- Around line 73-75: The Remove button is still enabled during start/stop
transitions because enabled only checks !isTransitioning; update the enabled
prop to also guard against inProgress so removal is disabled while a start/stop
is running (use enabled={!isTransitioning && !inProgress}), ensuring the Remove
action (handleRemove) cannot run concurrently with start/stop operations.
🪄 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: 92019063-475c-475b-9ebd-5a15010c147f
📒 Files selected for processing (6)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/renderer/src/stores/agent-workspaces.svelte.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: unit tests / macos-15
- GitHub Check: Linux
- GitHub Check: unit tests / windows-2025
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: Windows
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: macOS
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: linter, formatters
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/workspace-utils.tspackages/renderer/src/stores/agent-workspaces.svelte.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/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/renderer/src/stores/agent-workspaces.svelte.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.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.ts
🧠 Learnings (7)
📚 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/workspace-utils.tspackages/renderer/src/stores/agent-workspaces.svelte.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.sveltepackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
📚 Learning: 2026-04-17T20:27:03.231Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/stores/model-catalog.ts:5-21
Timestamp: 2026-04-17T20:27:03.231Z
Learning: In openkaiden/kaiden’s Electron renderer (single window, no shared localStorage origin across tabs/windows), do not suggest using `window.addEventListener('storage', ...)` for localStorage cross-tab synchronization. For store code like `packages/renderer/src/stores/model-catalog.ts`, treat `storage` events as not applicable for syncing state; use other intra-window state/update mechanisms instead.
Applied to files:
packages/renderer/src/stores/agent-workspaces.svelte.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/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.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/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.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/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.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/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 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.ts
🔇 Additional comments (4)
packages/renderer/src/stores/agent-workspaces.svelte.ts (1)
25-25: LGTM!Also applies to: 29-32
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.svelte (1)
21-28: LGTM!Also applies to: 46-56
packages/renderer/src/lib/agent-workspaces/workspace-utils.ts (1)
21-21: LGTM!Also applies to: 26-33
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.svelte (1)
63-64: IncludeinProgressinenabledto prevent unintended clicks during starting/stopping
InAgentWorkspaceActions.sveltethe buttons are disabled only whenisTransitioning, whilehandleStartStopshort-circuits oninProgress. IfListItemButtonIcondoesn’t fully disable click/UX based oninProgress, this can lead to an “enabled-looking” button that immediately does nothing. Considerenabled={!inProgress && !isTransitioning}(applies to lines 63-64 and 68-69).
benoitf
approved these changes
Jun 10, 2026
fbricon
force-pushed
the
worktree-GH-2030
branch
from
34c7558 to
6654705
Compare
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)
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.ts (1)
47-83: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd transition-state coverage for the new name-cell behavior.
Current tests only cover
state: 'stopped'. Please add cases forcreating/deletingto assert disabled navigation (and spinner visibility forcreating) so this new UX path is regression-proof.🤖 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/columns/AgentWorkspaceName.spec.ts` around lines 47 - 83, Add tests to cover the transition-state UX in AgentWorkspaceName: create new test cases similar to the existing "Clicking button navigates to workspace overview" but set workspace.state to 'creating' and 'deleting'; render AgentWorkspaceName with those workspaces, assert the button is disabled (no navigation) by checking router.goto was not called after fireEvent.click, and for the 'creating' case assert the spinner element (the loading indicator rendered by the component) is present/visible; keep using the same workspace object shape and router mock used in the current tests to locate AgentWorkspaceName, router.goto, and the button.
🤖 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 395-399: The navigation and draft reset are incorrectly placed
inside the promise .then callback so the UI waits for creation to finish; move
resetDraft() and handleNavigation({ page: NavigationPage.AGENT_WORKSPACES }) to
run immediately after scheduling the create call (i.e., right after invoking the
function that returns the creation promise instead of inside its .then), and
keep the existing .catch/.then handlers only for success/error handling of the
async task so errors are still logged/handled without blocking immediate
navigation.
---
Outside diff comments:
In
`@packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.ts`:
- Around line 47-83: Add tests to cover the transition-state UX in
AgentWorkspaceName: create new test cases similar to the existing "Clicking
button navigates to workspace overview" but set workspace.state to 'creating'
and 'deleting'; render AgentWorkspaceName with those workspaces, assert the
button is disabled (no navigation) by checking router.goto was not called after
fireEvent.click, and for the 'creating' case assert the spinner element (the
loading indicator rendered by the component) is present/visible; keep using the
same workspace object shape and router mock used in the current tests to locate
AgentWorkspaceName, router.goto, and the button.
🪄 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: 2fd8f653-6671-405f-9c68-6964738a41b4
📒 Files selected for processing (7)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/renderer/src/stores/agent-workspaces.svelte.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: linter, formatters
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: typecheck
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: Linux
- GitHub Check: Windows
- GitHub Check: unit tests / macos-15
- GitHub Check: macOS
- GitHub Check: unit tests / windows-2025
🧰 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/columns/AgentWorkspaceName.spec.tspackages/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/renderer/src/stores/agent-workspaces.svelte.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.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/columns/AgentWorkspaceName.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/columns/AgentWorkspaceName.spec.tspackages/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/renderer/src/stores/agent-workspaces.svelte.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.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.ts
🧠 Learnings (8)
📚 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/columns/AgentWorkspaceName.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/columns/AgentWorkspaceName.spec.tspackages/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.sveltepackages/renderer/src/stores/agent-workspaces.svelte.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.sveltepackages/main/src/plugin/agent-workspace/agent-workspace-manager.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.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.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.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.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.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.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.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.svelte
📚 Learning: 2026-04-17T20:27:03.231Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/stores/model-catalog.ts:5-21
Timestamp: 2026-04-17T20:27:03.231Z
Learning: In openkaiden/kaiden’s Electron renderer (single window, no shared localStorage origin across tabs/windows), do not suggest using `window.addEventListener('storage', ...)` for localStorage cross-tab synchronization. For store code like `packages/renderer/src/stores/model-catalog.ts`, treat `storage` events as not applicable for syncing state; use other intra-window state/update mechanisms instead.
Applied to files:
packages/renderer/src/stores/agent-workspaces.svelte.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.ts
🔇 Additional comments (9)
packages/renderer/src/stores/agent-workspaces.svelte.ts (2)
25-32: LGTM!
43-53: LGTM!packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (4)
62-77: LGTM!
106-150: LGTM!
296-342: LGTM!
344-384: LGTM!packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.svelte (1)
22-24: LGTM!Also applies to: 63-75
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.svelte (1)
2-2: LGTM!Also applies to: 21-27, 46-53
packages/renderer/src/lib/agent-workspaces/workspace-utils.ts (1)
21-32: LGTM!
fbricon
force-pushed
the
worktree-GH-2030
branch
2 times, most recently
from
1d744a6 to
32a9721
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 357-402: The list() method in AgentWorkspaceManager builds
synthetic entries with state='creating'|'deleting' and an _originalState field
which diverges from the declared Promise<AgentWorkspaceSummary[]> return type;
fix this by introducing a shared extended type (e.g., AgentWorkspaceSummaryUI or
AgentWorkspaceSummaryIPC) that extends AgentWorkspaceSummary with
state<'creating'|'deleting'> and optional _originalState, update the list()
signature to Promise<AgentWorkspaceSummaryUI[]> and replace the cast sites (the
inProgressEntries/deletingEntries maps) to construct that extended type, and
update the IPC handler signature (the handler currently declared as returning
AgentWorkspaceSummary[]) to use the new shared type so the backend/renderer
contract is explicit and type-safe.
- Around line 180-184: The current try/finally around
this.ensureModelSecretFromConfig(options) uses an empty finally so any thrown
exception prevents the fallback credential resolution from running; change the
finally to a catch that handles errors from ensureModelSecretFromConfig(options)
(log or ignore the error) and allow execution to continue to the existing
fallback credential resolution after the try/catch, while preserving the
early-return when ensureModelSecretFromConfig returns true.
🪄 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: 35502346-9bb6-4811-b98d-f3320de390fb
📒 Files selected for processing (7)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/renderer/src/stores/agent-workspaces.svelte.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: unit tests / windows-2025
- GitHub Check: Linux
- GitHub Check: unit tests / macos-15
- GitHub Check: Windows
- GitHub Check: macOS
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: linter, formatters
🧰 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/workspace-utils.tspackages/renderer/src/stores/agent-workspaces.svelte.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.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/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/renderer/src/stores/agent-workspaces.svelte.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.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/columns/AgentWorkspaceName.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.ts
🧠 Learnings (8)
📚 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/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.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/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.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/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.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/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 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/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/stores/agent-workspaces.svelte.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
📚 Learning: 2026-04-17T20:27:03.231Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/stores/model-catalog.ts:5-21
Timestamp: 2026-04-17T20:27:03.231Z
Learning: In openkaiden/kaiden’s Electron renderer (single window, no shared localStorage origin across tabs/windows), do not suggest using `window.addEventListener('storage', ...)` for localStorage cross-tab synchronization. For store code like `packages/renderer/src/stores/model-catalog.ts`, treat `storage` events as not applicable for syncing state; use other intra-window state/update mechanisms instead.
Applied to files:
packages/renderer/src/stores/agent-workspaces.svelte.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/columns/AgentWorkspaceName.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.ts
🔇 Additional comments (10)
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte (1)
368-408: LGTM!Also applies to: 410-417
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.svelte (1)
22-24: LGTM!Also applies to: 63-64, 68-69, 73-75
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.svelte (1)
2-2: LGTM!Also applies to: 21-27, 46-49, 52-53
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.ts (1)
52-53: LGTM!Also applies to: 62-63, 72-73, 85-145
packages/renderer/src/lib/agent-workspaces/workspace-utils.ts (1)
21-22: LGTM!Also applies to: 27-31
packages/renderer/src/stores/agent-workspaces.svelte.ts (2)
25-25: LGTM!
29-32: LGTM!packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (3)
63-78: LGTM!Also applies to: 111-129, 147-150
215-265: LGTM!
405-444: 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
🤖 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 357-402: The list() method in AgentWorkspaceManager builds
synthetic entries with state='creating'|'deleting' and an _originalState field
which diverges from the declared Promise<AgentWorkspaceSummary[]> return type;
fix this by introducing a shared extended type (e.g., AgentWorkspaceSummaryUI or
AgentWorkspaceSummaryIPC) that extends AgentWorkspaceSummary with
state<'creating'|'deleting'> and optional _originalState, update the list()
signature to Promise<AgentWorkspaceSummaryUI[]> and replace the cast sites (the
inProgressEntries/deletingEntries maps) to construct that extended type, and
update the IPC handler signature (the handler currently declared as returning
AgentWorkspaceSummary[]) to use the new shared type so the backend/renderer
contract is explicit and type-safe.
- Around line 180-184: The current try/finally around
this.ensureModelSecretFromConfig(options) uses an empty finally so any thrown
exception prevents the fallback credential resolution from running; change the
finally to a catch that handles errors from ensureModelSecretFromConfig(options)
(log or ignore the error) and allow execution to continue to the existing
fallback credential resolution after the try/catch, while preserving the
early-return when ensureModelSecretFromConfig returns true.
🪄 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: 35502346-9bb6-4811-b98d-f3320de390fb
📒 Files selected for processing (7)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/renderer/src/stores/agent-workspaces.svelte.ts
📜 Review details
🔇 Additional comments (10)
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte (1)
368-408: LGTM!Also applies to: 410-417
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.svelte (1)
22-24: LGTM!Also applies to: 63-64, 68-69, 73-75
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.svelte (1)
2-2: LGTM!Also applies to: 21-27, 46-49, 52-53
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.ts (1)
52-53: LGTM!Also applies to: 62-63, 72-73, 85-145
packages/renderer/src/lib/agent-workspaces/workspace-utils.ts (1)
21-22: LGTM!Also applies to: 27-31
packages/renderer/src/stores/agent-workspaces.svelte.ts (2)
25-25: LGTM!
29-32: LGTM!packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (3)
63-78: LGTM!Also applies to: 111-129, 147-150
215-265: LGTM!
405-444: LGTM!
🛑 Comments failed to post (2)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (2)
180-184:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winEmpty
finallyblock does not catch errors — fallback method will be skipped on failure.The
try/finallywith an emptyfinallydoes not suppress exceptions. IfensureModelSecretFromConfig()throws (e.g., storage access error, missing property), the exception propagates and the fallback credential resolution starting at line 186 is never reached.If the intent is to try the config-based method first and fall back to the original method on failure, use
try/catch:🐛 Proposed fix: convert to try/catch for graceful fallback
- try { - if (await this.ensureModelSecretFromConfig(options)) return; - } finally { - /* empty */ - } + try { + if (await this.ensureModelSecretFromConfig(options)) return; + } catch { + // Fall through to original credential resolution method + }📝 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.try { if (await this.ensureModelSecretFromConfig(options)) return; } catch { // Fall through to original credential resolution method }🤖 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 180 - 184, The current try/finally around this.ensureModelSecretFromConfig(options) uses an empty finally so any thrown exception prevents the fallback credential resolution from running; change the finally to a catch that handles errors from ensureModelSecretFromConfig(options) (log or ignore the error) and allow execution to continue to the existing fallback credential resolution after the try/catch, while preserving the early-return when ensureModelSecretFromConfig returns true.
357-402: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
Type contract diverges from actual return value.
list()returns objects withstate: 'creating' | 'deleting'and an_originalStatefield, but the declared return type isPromise<AgentWorkspaceSummary[]>wherestateis constrained to CLI-valid states and_originalStatedoesn't exist.The renderer works around this with a cast to
AgentWorkspaceSummaryUI[], but the IPC handler contract (line 593) claimsAgentWorkspaceSummary[]. This creates a fragile implicit contract.Consider introducing a shared extended type that both backend and renderer reference for the IPC boundary, or documenting the intentional cast at the IPC handler.
🤖 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 357 - 402, The list() method in AgentWorkspaceManager builds synthetic entries with state='creating'|'deleting' and an _originalState field which diverges from the declared Promise<AgentWorkspaceSummary[]> return type; fix this by introducing a shared extended type (e.g., AgentWorkspaceSummaryUI or AgentWorkspaceSummaryIPC) that extends AgentWorkspaceSummary with state<'creating'|'deleting'> and optional _originalState, update the list() signature to Promise<AgentWorkspaceSummaryUI[]> and replace the cast sites (the inProgressEntries/deletingEntries maps) to construct that extended type, and update the IPC handler signature (the handler currently declared as returning AgentWorkspaceSummary[]) to use the new shared type so the backend/renderer contract is explicit and type-safe.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
fbricon
force-pushed
the
worktree-GH-2030
branch
2 times, most recently
from
20dc4e0 to
1e29f72
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 394-400: The current logic deduplicates inProgressEntries against
existingWorkspaces before removing rows marked for deletion, causing recreated
workspaces to be suppressed; change the order and/or reference so that you first
compute the post-delete set (use filteredExisting =
existingWorkspaces.filter(... this.deletingWorkspaces ...)) and then produce
filteredInProgress by filtering inProgressEntries against filteredExisting
(i.e., replace the use of existingWorkspaces in the filteredInProgress check
with filteredExisting) so new "creating" entries are not deduped against rows
that are being deleted.
- Around line 357-402: The manager's list() returns synthetic UI-only entries
(created from inProgressWorkspaces and deletingWorkspaces) causing backend
methods to operate on fake ids/paths; change the implementation so operational
flows use a real CLI-backed listing (e.g., add listForBackend() or rename list()
to listForUI() and add listReal() that calls this.kdnCli.listWorkspaces()) and
ensure all backend methods (remove(), getConfiguration(), updateConfiguration(),
terminal IPC lookup, and any other callers) use the real list or explicitly
filter out entries with state 'creating' or 'deleting' (and ignore ids matching
/^creating-/) before performing CLI/file operations; update references to
this.list() across the class to use the real list method or apply the filter so
synthetic rows never reach operational logic.
In `@packages/renderer/src/lib/agent-workspaces/workspace-utils.ts`:
- Around line 26-33: Add unit tests for isActiveWorkspace (in
workspace-utils.ts) to cover: 1) a workspace with state 'creating' returns true;
2) a workspace with state 'deleting' and _originalState set to an active state
(e.g., 'running') returns true; 3) a workspace with state 'deleting' and
_originalState set to a stopped state (e.g., 'stopped') returns false; and 4) a
workspace with state 'deleting' and no _originalState defaults to false. Use the
AgentWorkspaceSummaryUI shape for test objects and assert boolean results via
your test runner (e.g., vitest).
🪄 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: 0272cea5-7d78-4005-88ea-1fff88c2915b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceContext.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/renderer/src/stores/agent-workspaces.svelte.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: Linux
- GitHub Check: linter, formatters
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: typecheck
- GitHub Check: unit tests / macos-15
- GitHub Check: macOS
- 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/stores/agent-workspaces.svelte.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.tspackages/renderer/src/lib/agent-workspaces/workspace-utils.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/renderer/src/stores/agent-workspaces.svelte.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.tspackages/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.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/columns/AgentWorkspaceName.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.ts
🧠 Learnings (9)
📚 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/columns/AgentWorkspaceContext.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.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/columns/AgentWorkspaceContext.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.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/columns/AgentWorkspaceContext.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.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/columns/AgentWorkspaceContext.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 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/columns/AgentWorkspaceContext.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.sveltepackages/renderer/src/stores/agent-workspaces.svelte.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.tspackages/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.sveltepackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
📚 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/columns/AgentWorkspaceContext.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-04-17T20:27:03.231Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/stores/model-catalog.ts:5-21
Timestamp: 2026-04-17T20:27:03.231Z
Learning: In openkaiden/kaiden’s Electron renderer (single window, no shared localStorage origin across tabs/windows), do not suggest using `window.addEventListener('storage', ...)` for localStorage cross-tab synchronization. For store code like `packages/renderer/src/stores/model-catalog.ts`, treat `storage` events as not applicable for syncing state; use other intra-window state/update mechanisms instead.
Applied to files:
packages/renderer/src/stores/agent-workspaces.svelte.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/columns/AgentWorkspaceName.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.ts
🔇 Additional comments (7)
packages/renderer/src/lib/agent-workspaces/workspace-utils.ts (1)
21-21: LGTM!Also applies to: 26-33
packages/renderer/src/stores/agent-workspaces.svelte.ts (1)
25-31: LGTM!packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte (1)
368-409: LGTM!Also applies to: 410-417
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.svelte (1)
22-24: LGTM!Also applies to: 63-64, 68-69, 73-75
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceContext.svelte (1)
12-17: LGTM!Also applies to: 22-23, 30-31
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.svelte (1)
2-2: LGTM!Also applies to: 21-23, 25-27, 46-49, 52-53
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.ts (1)
52-53: LGTM!Also applies to: 62-63, 72-73, 85-145
Comment on lines
357
to
+402
| async list(): Promise<AgentWorkspaceSummary[]> { | ||
| return this.kdnCli.listWorkspaces(); | ||
| const existingWorkspaces = await this.kdnCli.listWorkspaces(); | ||
|
|
||
| // Create synthetic workspace entries for in-progress creations | ||
| // Cast through unknown to allow 'creating' state which is not in the CLI schema | ||
| // but is valid in the UI layer (AgentWorkspaceState) | ||
| const inProgressEntries = Array.from(this.inProgressWorkspaces.values()).map( | ||
| ({ name, options, startedAt }) => | ||
| ({ | ||
| id: `creating-${name}`, | ||
| name, | ||
| project: options.project ?? '', | ||
| agent: options.agent, | ||
| model: options.model, | ||
| state: 'creating' as const, | ||
| runtime: options.runtime ?? 'podman', | ||
| paths: { | ||
| source: options.sourcePath, | ||
| configuration: '', | ||
| }, | ||
| timestamps: { created: startedAt }, | ||
| forwards: [], | ||
| }) as unknown as AgentWorkspaceSummary, | ||
| ); | ||
|
|
||
| // Create synthetic workspace entries for in-progress deletions | ||
| // Cast through unknown to allow 'deleting' state which is not in the CLI schema | ||
| // Preserve originalState in a custom field so UI can determine active/stopped group | ||
| const deletingEntries = Array.from(this.deletingWorkspaces.values()).map( | ||
| ({ workspace, originalState }) => | ||
| ({ | ||
| ...workspace, | ||
| state: 'deleting' as const, | ||
| _originalState: originalState, | ||
| }) as unknown as AgentWorkspaceSummary, | ||
| ); | ||
|
|
||
| // Filter out in-progress entries that may have completed and now exist in the real list | ||
| const filteredInProgress = inProgressEntries.filter( | ||
| ip => !existingWorkspaces.some(existing => existing.name === ip.name), | ||
| ); | ||
|
|
||
| // Filter out workspaces that are being deleted from the existing list | ||
| const filteredExisting = existingWorkspaces.filter(existing => !this.deletingWorkspaces.has(existing.id)); | ||
|
|
||
| return [...filteredInProgress, ...deletingEntries, ...filteredExisting]; |
Contributor
There was a problem hiding this comment.
Keep synthetic rows out of the manager's operational lookup path.
list() now returns UI-only creating/deleting records, but this class still uses this.list() for backend actions like remove(), getConfiguration(), updateConfiguration(), and the terminal IPC lookup. That makes fake IDs like creating-${name} and placeholder paths like configuration: '' reachable on non-UI code paths, so a transient row can now be fed back into the manager and produce CLI calls or file reads against a non-existent workspace. Split the UI-facing list from the real CLI list, or filter transient states before any operational lookup.
🤖 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 357 - 402, The manager's list() returns synthetic UI-only entries (created
from inProgressWorkspaces and deletingWorkspaces) causing backend methods to
operate on fake ids/paths; change the implementation so operational flows use a
real CLI-backed listing (e.g., add listForBackend() or rename list() to
listForUI() and add listReal() that calls this.kdnCli.listWorkspaces()) and
ensure all backend methods (remove(), getConfiguration(), updateConfiguration(),
terminal IPC lookup, and any other callers) use the real list or explicitly
filter out entries with state 'creating' or 'deleting' (and ignore ids matching
/^creating-/) before performing CLI/file operations; update references to
this.list() across the class to use the real list method or apply the filter so
synthetic rows never reach operational logic.
Comment on lines
+394
to
+400
| // Filter out in-progress entries that may have completed and now exist in the real list | ||
| const filteredInProgress = inProgressEntries.filter( | ||
| ip => !existingWorkspaces.some(existing => existing.name === ip.name), | ||
| ); | ||
|
|
||
| // Filter out workspaces that are being deleted from the existing list | ||
| const filteredExisting = existingWorkspaces.filter(existing => !this.deletingWorkspaces.has(existing.id)); |
Contributor
There was a problem hiding this comment.
Don't dedupe new creates against rows already marked for deletion.
filteredInProgress checks existingWorkspaces before filteredExisting removes deleting rows. If foo is being deleted and the user recreates foo immediately, the old real row suppresses the new synthetic creating entry, then the old row is filtered out, so the recreation disappears from the list until delete completes. Deduplicate against the post-delete set instead.
🤖 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 394 - 400, The current logic deduplicates inProgressEntries against
existingWorkspaces before removing rows marked for deletion, causing recreated
workspaces to be suppressed; change the order and/or reference so that you first
compute the post-delete set (use filteredExisting =
existingWorkspaces.filter(... this.deletingWorkspaces ...)) and then produce
filteredInProgress by filtering inProgressEntries against filteredExisting
(i.e., replace the use of existingWorkspaces in the filteredInProgress check
with filteredExisting) so new "creating" entries are not deduped against rows
that are being deleted.
Comment on lines
26
to
33
| export function isActiveWorkspace(ws: AgentWorkspaceSummaryUI): boolean { | ||
| // For deleting workspaces, use the original state to determine grouping | ||
| // so they stay in the same group (Active or Stopped) they were in before deletion | ||
| if (ws.state === 'deleting' && ws._originalState) { | ||
| return ACTIVE_STATES.includes(ws._originalState); | ||
| } | ||
| return ACTIVE_STATES.includes(ws.state); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Consider adding unit tests for the new grouping logic.
The updated isActiveWorkspace function now handles creating and deleting states with special logic for preserving grouping during deletion. Unit tests would help verify:
- Creating workspaces are classified as Active
- Deleting workspaces preserve their original Active/Stopped group when
_originalStateis present - The fallback behavior when
_originalStateis missing (defaults to Stopped)
🧪 Example test structure
import { describe, test, expect } from 'vitest';
import { isActiveWorkspace } from './workspace-utils';
import type { AgentWorkspaceSummaryUI } from '/@/stores/agent-workspaces.svelte';
describe('isActiveWorkspace', () => {
test('creating workspaces are active', () => {
const ws: AgentWorkspaceSummaryUI = { state: 'creating', /* ... */ };
expect(isActiveWorkspace(ws)).toBe(true);
});
test('deleting workspace preserves original active state', () => {
const ws: AgentWorkspaceSummaryUI = {
state: 'deleting',
_originalState: 'running',
/* ... */
};
expect(isActiveWorkspace(ws)).toBe(true);
});
test('deleting workspace preserves original stopped state', () => {
const ws: AgentWorkspaceSummaryUI = {
state: 'deleting',
_originalState: 'stopped',
/* ... */
};
expect(isActiveWorkspace(ws)).toBe(false);
});
test('deleting workspace without originalState defaults to stopped', () => {
const ws: AgentWorkspaceSummaryUI = {
state: 'deleting',
/* ... */
};
expect(isActiveWorkspace(ws)).toBe(false);
});
});🤖 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/workspace-utils.ts` around lines
26 - 33, Add unit tests for isActiveWorkspace (in workspace-utils.ts) to cover:
1) a workspace with state 'creating' returns true; 2) a workspace with state
'deleting' and _originalState set to an active state (e.g., 'running') returns
true; 3) a workspace with state 'deleting' and _originalState set to a stopped
state (e.g., 'stopped') returns false; and 4) a workspace with state 'deleting'
and no _originalState defaults to false. Use the AgentWorkspaceSummaryUI shape
for test objects and assert boolean results via your test runner (e.g., vitest).
bmahabirbu
approved these changes
Jun 12, 2026
bmahabirbu
left a comment
bmahabirbu
left a comment
Contributor
There was a problem hiding this comment.
LGTM just need a rebase/check coderabbit
but i think the functionality is slightly different than in podman desktop where if something is created it will route to its details page? maybe we should do the same for the sandbox creation @jeffmaury what are your thoughts
Display workspaces in the list immediately when creation or deletion starts, with visual feedback showing the operation is in progress. Changes: - Track creating/deleting states in renderer (following existing starting/stopping pattern) - Show synthetic workspace entries with "creating" state and spinner - Show workspaces with "deleting" state and spinner on delete button - Preserve workspace grouping (Active/Stopped) during deletion via _originalState field - Navigate immediately from wizard; workspace appears in list while creating in background - Draft preserved on creation error for retry - Apply deleting tracking to all removal locations (list, details, settings) Backend only tracks operations and emits update events; all UI state management stays in renderer. API package contains pure CLI types. Added comprehensive unit tests for workspace utility functions covering all transitional states. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Fred Bricon <fbricon@gmail.com>
fbricon
force-pushed
the
worktree-GH-2030
branch
from
1e29f72 to
96c70e9
Compare
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 (1)
packages/renderer/src/stores/agent-workspaces.svelte.ts (1)
43-52: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueThe backend-to-UI type assumption is valid but relies on an implicit contract.
The cast at line 44 treats the backend response as
AgentWorkspaceSummaryUI[], which works because:
- The backend
list()now returns synthetic entries withstate: 'creating'orstate: 'deleting'and_originalState- These fields exist at runtime but aren't part of the
AgentWorkspaceSummaryschemaThis is intentional per the PR design, but the cast through
unknownon the backend side (lines 432, 444 in agent-workspace-manager.ts) and direct cast here creates a fragile cross-layer contract. Consider adding a brief comment explaining that the backend injects UI-specific fields (state: 'creating'|'deleting',_originalState) that extend beyond the CLI schema.🤖 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/stores/agent-workspaces.svelte.ts` around lines 43 - 52, Add a brief comment above or near the cast of the backend response to AgentWorkspaceSummaryUI[] in the listWorkspaces function to document the implicit contract that the backend injects UI-specific fields (state as 'creating' or 'deleting', and _originalState) that extend beyond the CLI schema. This comment should clarify that this type conversion is intentional and explain why the backend response can be safely cast to AgentWorkspaceSummaryUI[] even though those extra fields exist at runtime but aren't part of the base AgentWorkspaceSummary type definition.
♻️ Duplicate comments (2)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (2)
447-455:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDedupe order causes recreated workspace to disappear during concurrent delete.
If workspace "foo" is being deleted and user immediately recreates "foo":
inProgressWorkspaceshas the new "creating-foo" entryexistingWorkspacesstill has the old "foo" (CLI hasn't removed it yet)deletingWorkspaceshas the old "foo" by idCurrent flow:
- Line 448-450:
filteredInProgressexcludes "creating-foo" becauseexistingWorkspacescontains "foo" by name- Line 453:
filteredExistingexcludes old "foo" because it's indeletingWorkspaces- Result: "foo" disappears from the list entirely until delete completes
Fix: dedupe
inProgressEntriesagainst the post-deletion set (filteredExisting) instead ofexistingWorkspaces.Proposed fix
- // Filter out in-progress entries that may have completed and now exist in the real list - const filteredInProgress = inProgressEntries.filter( - ip => !existingWorkspaces.some(existing => existing.name === ip.name), - ); - // Filter out workspaces that are being deleted from the existing list const filteredExisting = existingWorkspaces.filter(existing => !this.deletingWorkspaces.has(existing.id)); + // Filter out in-progress entries that may have completed and now exist in the real list + // Use filteredExisting (post-delete set) so recreations aren't suppressed by rows being deleted + const filteredInProgress = inProgressEntries.filter( + ip => !filteredExisting.some(existing => existing.name === ip.name), + ); + return [...filteredInProgress, ...deletingEntries, ...filteredExisting];🤖 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 447 - 455, The deduplication logic incorrectly filters in-progress entries by comparing against existingWorkspaces before removing entries that are being deleted. When a workspace is deleted and immediately recreated with the same name, the new in-progress entry gets filtered out because it matches the old workspace in existingWorkspaces, even though that old workspace is in the process of being deleted. Fix this by changing the filter on inProgressEntries (lines 448-450) to compare against filteredExisting instead of existingWorkspaces, so that in-progress entries are only deduplicated against workspaces that will actually remain in the final result.
500-525:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftSynthetic entries can reach operational code paths.
getConfiguration(),updateConfiguration(), and the terminal IPC handler all callthis.list(), which now returns synthetic'creating'entries with:
id: 'creating-${name}'(not a real workspace id)paths.configuration: ''(empty string)If a caller passes a synthetic id:
getConfigurationwould attemptreadFile(join('', 'workspace.json'), ...)→ likely fails but with a confusing pathupdateConfigurationwould pass an empty path tokdnCli.updateWorkspaceConfig- Terminal handler could match a creating workspace and attempt to shell into a non-existent workspace
While the UI disables these actions during transitions, direct IPC calls or race conditions could still trigger these paths.
Consider either:
- Add a private
listReal()that returns only CLI-backed workspaces for operational methods- Filter out synthetic states at the start of
getConfiguration,updateConfiguration, and terminal handlers:async getConfiguration(id: string): Promise<AgentWorkspaceConfiguration> { if (id.startsWith('creating-') || this.deletingWorkspaces.has(id)) { throw new Error(`Cannot get configuration for workspace "${id}" in transition state`); } // ... rest of method }Also applies to: 694-726
🤖 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 500 - 525, The getConfiguration(), updateConfiguration(), and terminal handler methods all call this.list() which now returns synthetic 'creating' entries with fake IDs (prefixed with 'creating-') and empty configuration paths. Add validation at the start of getConfiguration() and updateConfiguration() methods to reject synthetic workspace IDs by checking if the id parameter starts with 'creating-' or exists in this.deletingWorkspaces set, throwing an error if true. Apply the same validation check to the terminal handler that attempts to shell into workspaces (around line 694-726). This prevents synthetic workspace entries from reaching operational code that would fail with confusing error messages when trying to access empty paths or non-existent configurations.
🤖 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/renderer/src/stores/agent-workspaces.svelte.ts`:
- Around line 43-52: Add a brief comment above or near the cast of the backend
response to AgentWorkspaceSummaryUI[] in the listWorkspaces function to document
the implicit contract that the backend injects UI-specific fields (state as
'creating' or 'deleting', and _originalState) that extend beyond the CLI schema.
This comment should clarify that this type conversion is intentional and explain
why the backend response can be safely cast to AgentWorkspaceSummaryUI[] even
though those extra fields exist at runtime but aren't part of the base
AgentWorkspaceSummary type definition.
---
Duplicate comments:
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts`:
- Around line 447-455: The deduplication logic incorrectly filters in-progress
entries by comparing against existingWorkspaces before removing entries that are
being deleted. When a workspace is deleted and immediately recreated with the
same name, the new in-progress entry gets filtered out because it matches the
old workspace in existingWorkspaces, even though that old workspace is in the
process of being deleted. Fix this by changing the filter on inProgressEntries
(lines 448-450) to compare against filteredExisting instead of
existingWorkspaces, so that in-progress entries are only deduplicated against
workspaces that will actually remain in the final result.
- Around line 500-525: The getConfiguration(), updateConfiguration(), and
terminal handler methods all call this.list() which now returns synthetic
'creating' entries with fake IDs (prefixed with 'creating-') and empty
configuration paths. Add validation at the start of getConfiguration() and
updateConfiguration() methods to reject synthetic workspace IDs by checking if
the id parameter starts with 'creating-' or exists in this.deletingWorkspaces
set, throwing an error if true. Apply the same validation check to the terminal
handler that attempts to shell into workspaces (around line 694-726). This
prevents synthetic workspace entries from reaching operational code that would
fail with confusing error messages when trying to access empty paths or
non-existent configurations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c324b5ab-ea71-4d5d-ac17-44edc94e23b4
📒 Files selected for processing (8)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceContext.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/renderer/src/stores/agent-workspaces.svelte.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: unit tests / macos-15
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: unit tests / windows-2022
- GitHub Check: Linux
- GitHub Check: typecheck
- GitHub Check: linter, formatters
- GitHub Check: macOS
- 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/stores/agent-workspaces.svelte.tspackages/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.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/renderer/src/stores/agent-workspaces.svelte.tspackages/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.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/columns/AgentWorkspaceName.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.ts
🧠 Learnings (9)
📚 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/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceContext.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/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceContext.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/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceContext.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/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceContext.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 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/columns/AgentWorkspaceActions.sveltepackages/renderer/src/stores/agent-workspaces.svelte.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/workspace-utils.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.tspackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceContext.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.sveltepackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
📚 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/columns/AgentWorkspaceActions.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.sveltepackages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceContext.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte
📚 Learning: 2026-04-17T20:27:03.231Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/stores/model-catalog.ts:5-21
Timestamp: 2026-04-17T20:27:03.231Z
Learning: In openkaiden/kaiden’s Electron renderer (single window, no shared localStorage origin across tabs/windows), do not suggest using `window.addEventListener('storage', ...)` for localStorage cross-tab synchronization. For store code like `packages/renderer/src/stores/model-catalog.ts`, treat `storage` events as not applicable for syncing state; use other intra-window state/update mechanisms instead.
Applied to files:
packages/renderer/src/stores/agent-workspaces.svelte.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/columns/AgentWorkspaceName.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.ts
🔇 Additional comments (12)
packages/renderer/src/stores/agent-workspaces.svelte.ts (1)
25-32: LGTM!packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (5)
64-79: LGTM!
112-158: LGTM!
160-202: LGTM!
293-318: LGTM!
458-498: LGTM!packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCreate.svelte (1)
368-408: LGTM!Also applies to: 410-411, 416-416
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceActions.svelte (1)
22-24: LGTM!Also applies to: 63-64, 68-69, 73-75
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceContext.svelte (1)
12-17: LGTM!Also applies to: 22-23, 30-31
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.svelte (1)
2-2: LGTM!Also applies to: 21-23, 25-27, 46-49, 52-52
packages/renderer/src/lib/agent-workspaces/columns/AgentWorkspaceName.spec.ts (1)
52-53: LGTM!Also applies to: 62-63, 72-73, 85-145
packages/renderer/src/lib/agent-workspaces/workspace-utils.ts (1)
21-21: LGTM!Also applies to: 27-31
Contributor
Author
|
New openshell sandboxes change the game. Waiting for them to stabilize, taking over kdn workspaces, to resume work on this. |
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
3 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.
Workspaces are now visible in the workspace list while being created
or deleted, with clear visual indicators showing their status.
Problem: When creating a workspace, users would be stuck on the
creation wizard page with no feedback while the workspace was being
created. If they navigated to the workspace list, the workspace
wouldn't appear until creation completed, causing confusion about
whether the creation succeeded. Similarly, deleted workspaces would
disappear immediately without showing deletion progress.
Solution:
Backend changes:
Frontend changes:
User experience:
create-new-workspace.mp4
Fixes #2030
Fixes #2033