Skip to content

feat: updated workspace manager with kortex-cli calls#1214

Merged
gastoner merged 1 commit intokortex-hub:mainfrom
gastoner:workspace_manager_update
Apr 1, 2026
Merged

feat: updated workspace manager with kortex-cli calls#1214
gastoner merged 1 commit intokortex-hub:mainfrom
gastoner:workspace_manager_update

Conversation

@gastoner
Copy link
Copy Markdown
Contributor

@gastoner gastoner commented Apr 1, 2026

This PR updates the workspace manger with kortex-cli calls

Closes #1124

You can tesst this by having the kortex-cli in PATH
You should be able to start/stop/delete and list the workspaces

Known issue: the calls like list etc are called only during the startup so there is no refreshing of the wokspaces (unless you e.g. delete one workspace => the list will get updated) - but this is a different issue

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Evzen Gasta <evzen.ml@seznam.cz>
@gastoner gastoner requested a review from a team as a code owner April 1, 2026 10:16
@gastoner gastoner requested review from bmahabirbu and fbricon and removed request for a team April 1, 2026 10:16
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 1, 2026

📝 Walkthrough

Walkthrough

The AgentWorkspaceManager is being refactored to perform real workspace operations via injected Exec runner for kortex-cli commands instead of delegating to mock functions. All public methods convert to async, returning Promises. The mock data module is deleted entirely. Tests are updated to verify CLI invocations and YAML parsing.

Changes

Cohort / File(s) Summary
Implementation
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
Converted list, remove, getConfiguration, start, and stop methods from synchronous to async by injecting Exec dependency. Methods now execute kortex-cli workspace CLI commands with JSON output parsing via new execKortex<T> helper. getConfiguration now retrieves workspace ID via list(), reads configuration file via readFile, and parses YAML instead of returning mock data.
Test Suite
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
Removed mocking of mock-data module. Added mocks for node:fs/promises (readFile) and yaml (parse). Converted test assertions from synchronous "delegates to mock" expectations to asynchronous verifications of exec.exec calls with specific kortex-cli workspace arguments. Added promise rejection scenarios for missing workspace IDs and file read failures.
Mock Data Removal
packages/main/src/plugin/agent-workspace/agent-workspace-mock-data.ts
Entire module deleted. Removed five exported functions (mockListWorkspaces, mockRemoveWorkspace, mockGetWorkspaceConfiguration, mockStartWorkspace, mockStopWorkspace) that previously provided in-memory workspace management.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • #1111: Both PRs modify AgentWorkspaceManager.getConfiguration implementation and related tests, replacing mock-based returns with CLI/YAML-backed async operations.
  • #1083: Introduced the initial AgentWorkspaceManager with mock-based workspace operations; this PR substantially refactors it to use real CLI invocations.
  • #1137: Modifies AgentWorkspaceManager start and stop operations; this PR extends those same methods with async/CLI-driven implementations via injected Exec.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: updating the workspace manager to use kortex-cli calls instead of mock implementations.
Linked Issues check ✅ Passed The PR successfully addresses issue #1124 by replacing mock workspace operations with real kortex-cli invocations across all workspace manager methods.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the objective of replacing mock implementations with kortex-cli calls; no extraneous modifications detected.
Description check ✅ Passed The pull request description directly addresses the changeset, explaining the update to the workspace manager with kortex-cli calls and referencing the related issue.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (1)

48-51: Consider wrapping JSON.parse with error context.

If kortex-cli returns non-JSON output (e.g., an error message or corrupted output), JSON.parse throws a generic SyntaxError that won't indicate which command failed or what the actual output was. This makes debugging CLI integration issues harder.

♻️ Proposed improvement
  private async execKortex<T>(args: string[]): Promise<T> {
    const result = await this.exec.exec('kortex-cli', ['workspace', ...args, '--output', 'json']);
-   return JSON.parse(result.stdout) as T;
+   try {
+     return JSON.parse(result.stdout) as T;
+   } catch (err) {
+     throw new Error(`Failed to parse kortex-cli output for "workspace ${args.join(' ')}": ${result.stdout}`);
+   }
  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts` around
lines 48 - 51, The execKortex method directly JSON.parse's result.stdout which
will throw an opaque SyntaxError if kortex-cli returns non-JSON; wrap the
JSON.parse call in a try/catch inside execKortex (the function name to change)
and on parse failure throw a new Error that includes the command invoked
(kortex-cli + ['workspace', ...args]), the raw result.stdout and result.stderr
and the original error message so callers can see which CLI invocation and
output caused the parse failure; keep rethrowing (or throw the enriched Error)
so callers still receive an exception.
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts (1)

26-26: Rename imported type to avoid shadowing the global Proxy.

The static analysis tool flags this as shadowing the global Proxy. While it's a type-only import that doesn't affect runtime, renaming it silences the linter and improves clarity.

♻️ Suggested fix
-import type { Proxy } from '/@/plugin/proxy.js';
+import type { Proxy as ProxyService } from '/@/plugin/proxy.js';

Then update line 60:

-} as unknown as Proxy;
+} as unknown as ProxyService;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts` at
line 26, Rename the type-only import named "Proxy" to avoid shadowing the global
Proxy: change the import to use an alias (e.g., import type { Proxy as ProxyType
} from '...') and then update all type annotations/usages that currently
reference "Proxy" (for example the spec's variable/type at the test usage around
the previous line 60) to use "ProxyType" instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts`:
- Line 26: Rename the type-only import named "Proxy" to avoid shadowing the
global Proxy: change the import to use an alias (e.g., import type { Proxy as
ProxyType } from '...') and then update all type annotations/usages that
currently reference "Proxy" (for example the spec's variable/type at the test
usage around the previous line 60) to use "ProxyType" instead.

In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts`:
- Around line 48-51: The execKortex method directly JSON.parse's result.stdout
which will throw an opaque SyntaxError if kortex-cli returns non-JSON; wrap the
JSON.parse call in a try/catch inside execKortex (the function name to change)
and on parse failure throw a new Error that includes the command invoked
(kortex-cli + ['workspace', ...args]), the raw result.stdout and result.stderr
and the original error message so callers can see which CLI invocation and
output caused the parse failure; keep rethrowing (or throw the enriched Error)
so callers still receive an exception.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 716dfc00-e36b-44f2-be36-f162ef65e4ad

📥 Commits

Reviewing files that changed from the base of the PR and between 409886b and 0848c31.

📒 Files selected for processing (3)
  • packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
  • packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
  • packages/main/src/plugin/agent-workspace/agent-workspace-mock-data.ts
💤 Files with no reviewable changes (1)
  • packages/main/src/plugin/agent-workspace/agent-workspace-mock-data.ts

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but error while started is not reported but maybe unrelated to this PR will create an issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update the mocked workspace manager by CLI invocations

3 participants