feat: start stop button in workspace#1140
Open
MarsKubeX wants to merge 2 commits intokortex-hub:mainfrom
Open
Conversation
Signed-off-by: Marcel Bertagnini <mbertagn@redhat.com>
Signed-off-by: Marcel Bertagnini <mbertagn@redhat.com>
MarsKubeX
requested review from
fbricon and
gastoner
and removed request for
a team
📝 WalkthroughWalkthroughThis PR adds start/stop workspace controls by introducing a new Svelte store ( Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Component
participant Store as agentWorkspaceStatuses Store
participant API as window.startAgentWorkspace/<br/>stopAgentWorkspace
UI->>Store: startAgentWorkspace(id)
Store->>Store: set status to 'starting'
Store-->>UI: status = 'starting' (UI shows loading)
Store->>API: call window.startAgentWorkspace(id)
alt Success
API-->>Store: resolve with {id}
Store->>Store: set status to 'running'
Store-->>UI: status = 'running' (button updates)
else Failure
API-->>Store: reject error
Store->>Store: set status to 'stopped' (revert)
Store-->>UI: status = 'stopped'
end
sequenceDiagram
participant UI as UI Component
participant Store as agentWorkspaceStatuses Store
participant API as window.startAgentWorkspace/<br/>stopAgentWorkspace
UI->>Store: stopAgentWorkspace(id)
Store->>Store: set status to 'stopping'
Store-->>UI: status = 'stopping' (UI shows loading)
Store->>API: call window.stopAgentWorkspace(id)
alt Success
API-->>Store: resolve with {id}
Store->>Store: set status to 'stopped'
Store-->>UI: status = 'stopped' (button updates)
else Failure
API-->>Store: reject error
Store->>Store: set status to 'running' (revert)
Store-->>UI: status = 'running'
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/renderer/src/stores/agent-workspaces.spec.ts (1)
64-101: Add a regression test for concurrent updates across different workspace IDs.Current tests are single-workspace only. A multi-workspace concurrent scenario would protect against stale snapshot overwrites in async stop/start flows.
🧪 Suggested test case
+test('stopAgentWorkspace should preserve updates made to other workspaces while awaiting', async () => { + agentWorkspaceStatuses.set( + new Map([ + ['ws-1', 'running'], + ['ws-2', 'stopped'], + ]), + ); + + let resolveStop: (value: { id: string }) => void = () => {}; + vi.mocked(window.stopAgentWorkspace).mockReturnValue( + new Promise(resolve => { + resolveStop = resolve; + }), + ); + + const stopPromise = stopAgentWorkspace('ws-1'); + + // Simulate another workspace changing while ws-1 stop is in-flight. + agentWorkspaceStatuses.update(statuses => { + const next = new Map(statuses); + next.set('ws-2', 'running'); + return next; + }); + + resolveStop({ id: 'ws-1' }); + await stopPromise; + + expect(get(agentWorkspaceStatuses).get('ws-2')).toBe('running'); +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/stores/agent-workspaces.spec.ts` around lines 64 - 101, Add a regression test that exercises concurrent updates across different workspace IDs to ensure async state updates don't overwrite each other: initialize agentWorkspaceStatuses with two entries (e.g., 'ws-1' => 'running', 'ws-2' => 'running'), mock window.stopAgentWorkspace to return a controllable promise for one id (using a resolver variable) and an immediate resolved value for the other, call stopAgentWorkspace for both IDs concurrently, assert that the in-flight one sets its status to 'stopping' while the other transitions to 'stopped' immediately, then resolve the pending promise and assert both end up 'stopped' (use get(agentWorkspaceStatuses), stopAgentWorkspace and window.stopAgentWorkspace to locate code paths).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/renderer/src/stores/agent-workspaces.ts`:
- Around line 57-69: The function stopAgentWorkspace captures a Map snapshot in
statuses and reuses it across an await, which can overwrite concurrent updates;
instead make status changes atomically against the current store (use
agentWorkspaceStatuses.update or re-read agentWorkspaceStatuses with get right
before each write) so you never apply a stale Map. Concretely: don't keep and
call agentWorkspaceStatuses.set(new Map(statuses)) after the await; set
'stopping' by updating the live store, then after the await update the live
store again to 'stopped' or in the catch to 'running' (use the update callback
to only modify the entry for id), ensuring other workspace entries remain
untouched; reference stopAgentWorkspace, agentWorkspaceStatuses, and the local
statuses variable when making the change.
---
Nitpick comments:
In `@packages/renderer/src/stores/agent-workspaces.spec.ts`:
- Around line 64-101: Add a regression test that exercises concurrent updates
across different workspace IDs to ensure async state updates don't overwrite
each other: initialize agentWorkspaceStatuses with two entries (e.g., 'ws-1' =>
'running', 'ws-2' => 'running'), mock window.stopAgentWorkspace to return a
controllable promise for one id (using a resolver variable) and an immediate
resolved value for the other, call stopAgentWorkspace for both IDs concurrently,
assert that the in-flight one sets its status to 'stopping' while the other
transitions to 'stopped' immediately, then resolve the pending promise and
assert both end up 'stopped' (use get(agentWorkspaceStatuses),
stopAgentWorkspace and window.stopAgentWorkspace to locate code paths).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e00ca8c6-eaf7-4691-90b1-c1ce23208fc5
📒 Files selected for processing (6)
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCard.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceCard.sveltepackages/renderer/src/lib/agent-workspaces/AgentWorkspaceDetails.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceDetails.sveltepackages/renderer/src/stores/agent-workspaces.spec.tspackages/renderer/src/stores/agent-workspaces.ts
Comment on lines
+57
to
+69
| export async function stopAgentWorkspace(id: string): Promise<void> { | ||
| const statuses = get(agentWorkspaceStatuses); | ||
| statuses.set(id, 'stopping'); | ||
| agentWorkspaceStatuses.set(new Map(statuses)); | ||
| try { | ||
| await window.stopAgentWorkspace(id); | ||
| statuses.set(id, 'stopped'); | ||
| } catch (error: unknown) { | ||
| statuses.set(id, 'running'); | ||
| console.error('Failed to stop agent workspace', error); | ||
| } | ||
| agentWorkspaceStatuses.set(new Map(statuses)); | ||
| } |
There was a problem hiding this comment.
stopAgentWorkspace can clobber concurrent status updates.
Line 58 captures a Map snapshot and reuses it after await (Lines 62-68). If another workspace status changes in between, the final set(new Map(statuses)) can overwrite that newer state.
🔧 Suggested fix
export async function stopAgentWorkspace(id: string): Promise<void> {
- const statuses = get(agentWorkspaceStatuses);
- statuses.set(id, 'stopping');
- agentWorkspaceStatuses.set(new Map(statuses));
+ agentWorkspaceStatuses.update(statuses => {
+ const next = new Map(statuses);
+ next.set(id, 'stopping');
+ return next;
+ });
try {
await window.stopAgentWorkspace(id);
- statuses.set(id, 'stopped');
+ agentWorkspaceStatuses.update(statuses => {
+ const next = new Map(statuses);
+ next.set(id, 'stopped');
+ return next;
+ });
} catch (error: unknown) {
- statuses.set(id, 'running');
+ agentWorkspaceStatuses.update(statuses => {
+ const next = new Map(statuses);
+ next.set(id, 'running');
+ return next;
+ });
console.error('Failed to stop agent workspace', error);
}
- agentWorkspaceStatuses.set(new Map(statuses));
}📝 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.
Suggested change
| export async function stopAgentWorkspace(id: string): Promise<void> { | |
| const statuses = get(agentWorkspaceStatuses); | |
| statuses.set(id, 'stopping'); | |
| agentWorkspaceStatuses.set(new Map(statuses)); | |
| try { | |
| await window.stopAgentWorkspace(id); | |
| statuses.set(id, 'stopped'); | |
| } catch (error: unknown) { | |
| statuses.set(id, 'running'); | |
| console.error('Failed to stop agent workspace', error); | |
| } | |
| agentWorkspaceStatuses.set(new Map(statuses)); | |
| } | |
| export async function stopAgentWorkspace(id: string): Promise<void> { | |
| agentWorkspaceStatuses.update(statuses => { | |
| const next = new Map(statuses); | |
| next.set(id, 'stopping'); | |
| return next; | |
| }); | |
| try { | |
| await window.stopAgentWorkspace(id); | |
| agentWorkspaceStatuses.update(statuses => { | |
| const next = new Map(statuses); | |
| next.set(id, 'stopped'); | |
| return next; | |
| }); | |
| } catch (error: unknown) { | |
| agentWorkspaceStatuses.update(statuses => { | |
| const next = new Map(statuses); | |
| next.set(id, 'running'); | |
| return next; | |
| }); | |
| console.error('Failed to stop agent workspace', error); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/renderer/src/stores/agent-workspaces.ts` around lines 57 - 69, The
function stopAgentWorkspace captures a Map snapshot in statuses and reuses it
across an await, which can overwrite concurrent updates; instead make status
changes atomically against the current store (use agentWorkspaceStatuses.update
or re-read agentWorkspaceStatuses with get right before each write) so you never
apply a stale Map. Concretely: don't keep and call
agentWorkspaceStatuses.set(new Map(statuses)) after the await; set 'stopping' by
updating the live store, then after the await update the live store again to
'stopped' or in the catch to 'running' (use the update callback to only modify
the entry for id), ensuring other workspace entries remain untouched; reference
stopAgentWorkspace, agentWorkspaceStatuses, and the local statuses variable when
making the change.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
gastoner
approved these changes
Mar 20, 2026
Contributor
gastoner
left a comment
gastoner
left a comment
There was a problem hiding this comment.
LGTM otherwise
On the detail page the start/stop button seems like it is not rouded in the same way as in the Card
| export type AgentWorkspaceStatus = 'stopped' | 'running' | 'starting' | 'stopping'; | ||
|
|
||
| export const agentWorkspaces: Writable<AgentWorkspaceSummary[]> = writable([]); | ||
| export const agentWorkspaceStatuses: Writable<Map<string, AgentWorkspaceStatus>> = writable(new Map()); |
Contributor
There was a problem hiding this comment.
(NIT: not req) Thinking if e.g. usage of SvelteMap here would be better? + renaming the file or just a Map
WDYT @benoitf
jeffmaury
reviewed
Mar 20, 2026
bmahabirbu
approved these changes
Mar 20, 2026
Contributor
bmahabirbu
left a comment
bmahabirbu
left a comment
There was a problem hiding this comment.
LGTM id check out the coderabbit suggestions also!
jeffmaury
approved these changes
Mar 20, 2026
Contributor
jeffmaury
left a comment
jeffmaury
left a comment
There was a problem hiding this comment.
LGTM. Tested on Windows
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added start and stop buttons ford workspaces in workspace list view and in workspace detail view
Grabacion.de.pantalla.2026-03-20.a.las.10.29.01.mov
Closes #1123