feat(goose): implement agent configuration for workspace preparation#2192
feat(goose): implement agent configuration for workspace preparation#2192bmahabirbu wants to merge 3 commits into
Conversation
|
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:
📝 WalkthroughWalkthroughThis PR implements Goose agent configuration handling: registering a config file at GOOSE_CONFIG_PATH, adding a YAML codec, and updating preWorkspaceStart to set model/provider/endpoint/MCP extensions and workspace environment variables for vertexai. It also passes agent baseImage into OpenShell sandbox creation, with corresponding tests for both areas. ChangesGoose Workspace Config Handling
Estimated code review effort: 3 (Moderate) | ~25 minutes OpenShell Sandbox Base Image
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ff4e136 to
bc0b280
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@extensions/goose/src/extension.ts`:
- Around line 56-57: The filter condition in the split operation only removes
lines that start with 'GOOSE_MODEL:' at column 0, but it fails to remove entries
that have leading whitespace (like indented YAML keys). Modify the filter to
trim each line before checking if it starts with 'GOOSE_MODEL:' so that both
`GOOSE_MODEL:` and ` GOOSE_MODEL:` entries are properly removed before
appending the new value. This prevents duplicate YAML keys in the config file.
🪄 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: 01dfb65c-d9ed-4cf4-b625-ba614e4eb913
📒 Files selected for processing (2)
extensions/goose/src/extension.spec.tsextensions/goose/src/extension.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: macOS
- GitHub Check: typecheck
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: Linux
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: Windows
- GitHub Check: unit tests / macos-15
- GitHub Check: linter, formatters
- GitHub Check: unit tests / windows-2022
🧰 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:
extensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
extensions/*/src/extension.ts
📄 CodeRabbit inference engine (AGENTS.md)
Extensions should export a standard activation API from their entry point
Files:
extensions/goose/src/extension.ts
extensions/*/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Register inference, container, and Kubernetes providers through the
ProviderRegistryvia extension APIs
Files:
extensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
**/*.spec.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.{ts,tsx,js,jsx}: Usetest()instead ofit()for test cases in Vitest unit tests
Usevi.mock(import('...'))for auto-mocking modules in unit tests; avoid manual mock factories when possible
Usevi.resetAllMocks()inbeforeEachhooks instead ofvi.clearAllMocks()for resetting mocks between tests
When an auto-mocked function or class method needs a real implementation, usevi.mocked(...)with the prototype pattern for class methods:vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)
Files:
extensions/goose/src/extension.spec.ts
🧠 Learnings (5)
📚 Learning: 2026-05-05T17:30:20.418Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/src/vertex-ai.ts:272-302
Timestamp: 2026-05-05T17:30:20.418Z
Learning: In the openkaiden/kaiden repo, for cloud inference provider extension code under `extensions/*/src/*.ts`, treat `ProviderConnectionStatus = 'unknown'` as a valid/expected value when registering provider connections (e.g., Gemini/Claude/Mistral/OpenAI-compatible/Vertex AI). `'unknown'` indicates the connection was set up but is not continuously monitored—so do not flag it as incorrect. Only Ollama is expected to use `'started'` because it actively polls a local server.
Applied to files:
extensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
📚 Learning: 2026-05-05T17:44:50.991Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/src/vertex-ai.ts:363-387
Timestamp: 2026-05-05T17:44:50.991Z
Learning: In this repo (openkaiden/kaiden), do not raise a code review issue when an extension’s `InferenceProviderConnectionFactory.create` factory method implementation omits (or does not use) the optional `logger` and/or `CancellationToken` parameters in its method signature/implementation. Current extensions (e.g., Vertex AI, Gemini, Claude, Mistral, OpenAI-compatible) follow this pattern, so reviewers should treat it as acceptable for `extensions/*` TypeScript source files.
Applied to files:
extensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
📚 Learning: 2026-05-06T11:15:56.238Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/src/extension.spec.ts:43-51
Timestamp: 2026-05-06T11:15:56.238Z
Learning: In all extensions under extensions/*/src/extension.ts, deactivate() should only clear the module-level instance reference (e.g., set the instance to undefined) and must not call dispose() directly. The dispose() method is invoked by the extension host when processing extensionContext.subscriptions. Do not suggest asserting dispose() in tests for deactivate(); such assertions are unnecessary because disposal is handled by the host and CI checks should validate subscriptions handling instead.
Applied to files:
extensions/goose/src/extension.ts
📚 Learning: 2026-05-12T10:01:14.248Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1810
File: extensions/kdn/src/kdn-extension.ts:43-46
Timestamp: 2026-05-12T10:01:14.248Z
Learning: In this repo’s extension code, when logging from binary discovery/resolution logic (e.g., choosing/validating custom paths, extension storage locations, or bundled resource paths), it’s intentional to include full filesystem paths in `console.log`/`console.warn` (such as in `extensions/**/src/*-extension.ts`). During review, do not flag these specific full-path messages as a privacy/security issue as long as they are clearly part of the binary resolution steps. If full-path logging appears outside binary discovery/resolution, review/flag it as usual.
Applied to files:
extensions/goose/src/extension.tsextensions/goose/src/extension.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:
extensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
🔇 Additional comments (2)
extensions/goose/src/extension.ts (1)
19-42: LGTM!Also applies to: 49-55, 58-60
extensions/goose/src/extension.spec.ts (1)
19-24: LGTM!Also applies to: 29-37, 77-154
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| } | ||
|
|
||
| const content = await configFile.read(); | ||
| const lines = content.split('\n').filter(line => !line.startsWith('GOOSE_MODEL:')); |
There was a problem hiding this comment.
it looks fragile to parse using split function
https://block-goose.mintlify.app/configuration#config-yaml-structure
goose config file is using yaml so we should parse using a yaml library (we do that in some extensions/core of Podman Desktop or kaiden)
a716034 to
ed15a78
Compare
7aab882 to
0fb7be9
Compare
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 `@extensions/goose/package.json`:
- Line 27: The yaml package dependency is pinned to version 2.8.2 which contains
a MODERATE severity Stack Overflow vulnerability that can be triggered through
deeply nested YAML collections. Update the yaml package version constraint in
the package.json file from "^2.8.2" to "^2.9.0" to resolve this vulnerability.
This will ensure the package is updated to the latest stable release that
includes the security fix.
In `@extensions/goose/src/extension.spec.ts`:
- Around line 87-94: The createContext helper function is returning an object
that is missing the required workspace property from the AgentWorkspaceContext
interface. Add the workspace property to the returned object in the
createContext function. The workspace property should be a valid mock object
that satisfies the AgentWorkspaceContext interface requirements. Ensure all
required properties of the workspace object are included to resolve the
TypeScript compilation error.
🪄 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: a02333c5-64fd-42f2-8120-53487a3d22de
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
extensions/goose/package.jsonextensions/goose/src/extension.spec.tsextensions/goose/src/extension.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 / ubuntu-24.04
- GitHub Check: Linux
- 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-2022
- GitHub Check: linter, formatters
- GitHub Check: unit tests / macos-15
- GitHub Check: Windows
- GitHub Check: typecheck
- GitHub Check: macOS
🧰 Additional context used
📓 Path-based instructions (5)
extensions/*/package.json
📄 CodeRabbit inference engine (AGENTS.md)
extensions/*/package.json: Extensions must declareengines.kaidenversion compatibility in theirpackage.json
Extensionpackage.jsonmust havemainfield pointing to./dist/extension.js
Configuration properties for API keys, tokens, or secrets must use"format": "password"in the configuration definition to ensure input masking in the UI
Files:
extensions/goose/package.json
**/*.{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:
extensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
extensions/*/src/extension.ts
📄 CodeRabbit inference engine (AGENTS.md)
Extensions should export a standard activation API from their entry point
Files:
extensions/goose/src/extension.ts
extensions/*/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Register inference, container, and Kubernetes providers through the
ProviderRegistryvia extension APIs
Files:
extensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
**/*.spec.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.{ts,tsx,js,jsx}: Usetest()instead ofit()for test cases in Vitest unit tests
Usevi.mock(import('...'))for auto-mocking modules in unit tests; avoid manual mock factories when possible
Usevi.resetAllMocks()inbeforeEachhooks instead ofvi.clearAllMocks()for resetting mocks between tests
When an auto-mocked function or class method needs a real implementation, usevi.mocked(...)with the prototype pattern for class methods:vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)
Files:
extensions/goose/src/extension.spec.ts
🧠 Learnings (7)
📚 Learning: 2026-05-06T11:29:33.170Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/package.json:9-11
Timestamp: 2026-05-06T11:29:33.170Z
Learning: In the openkaiden/kaiden repo, all built-in extensions under extensions/ should specify engines with kaiden: "^0.0.1" in package.json. Do not flag each extension individually; enforce a repo-wide alignment in a single PR. During reviews, verify that every extensions/*/package.json has "engines": { "kaiden": "^0.0.1" }. If a file deviates, surface the discrepancy as a single repo-wide task rather than per-file.
Applied to files:
extensions/goose/package.json
📚 Learning: 2026-05-05T17:30:20.418Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/src/vertex-ai.ts:272-302
Timestamp: 2026-05-05T17:30:20.418Z
Learning: In the openkaiden/kaiden repo, for cloud inference provider extension code under `extensions/*/src/*.ts`, treat `ProviderConnectionStatus = 'unknown'` as a valid/expected value when registering provider connections (e.g., Gemini/Claude/Mistral/OpenAI-compatible/Vertex AI). `'unknown'` indicates the connection was set up but is not continuously monitored—so do not flag it as incorrect. Only Ollama is expected to use `'started'` because it actively polls a local server.
Applied to files:
extensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
📚 Learning: 2026-05-05T17:44:50.991Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/src/vertex-ai.ts:363-387
Timestamp: 2026-05-05T17:44:50.991Z
Learning: In this repo (openkaiden/kaiden), do not raise a code review issue when an extension’s `InferenceProviderConnectionFactory.create` factory method implementation omits (or does not use) the optional `logger` and/or `CancellationToken` parameters in its method signature/implementation. Current extensions (e.g., Vertex AI, Gemini, Claude, Mistral, OpenAI-compatible) follow this pattern, so reviewers should treat it as acceptable for `extensions/*` TypeScript source files.
Applied to files:
extensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
📚 Learning: 2026-05-06T11:15:56.238Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/src/extension.spec.ts:43-51
Timestamp: 2026-05-06T11:15:56.238Z
Learning: In all extensions under extensions/*/src/extension.ts, deactivate() should only clear the module-level instance reference (e.g., set the instance to undefined) and must not call dispose() directly. The dispose() method is invoked by the extension host when processing extensionContext.subscriptions. Do not suggest asserting dispose() in tests for deactivate(); such assertions are unnecessary because disposal is handled by the host and CI checks should validate subscriptions handling instead.
Applied to files:
extensions/goose/src/extension.ts
📚 Learning: 2026-06-18T08:20:05.553Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 2185
File: extensions/gemini/src/extension.ts:45-52
Timestamp: 2026-06-18T08:20:05.553Z
Learning: When reviewing openkaiden/kaiden extension code that registers `configurationFiles` via `agents.registerAgent()` (typically in `extensions/*/src/extension.ts`), do not flag an intentionally stub `read()` (e.g., returning `'{}'`) as causing data loss or as a missing file-I/O implementation. Per the framework, runtime behavior uses the declared `path` and the real, file-backed `read()`/`update()` implementations are supplied through `context.configurationFiles` during `preWorkspaceStart`.
Applied to files:
extensions/goose/src/extension.ts
📚 Learning: 2026-05-12T10:01:14.248Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1810
File: extensions/kdn/src/kdn-extension.ts:43-46
Timestamp: 2026-05-12T10:01:14.248Z
Learning: In this repo’s extension code, when logging from binary discovery/resolution logic (e.g., choosing/validating custom paths, extension storage locations, or bundled resource paths), it’s intentional to include full filesystem paths in `console.log`/`console.warn` (such as in `extensions/**/src/*-extension.ts`). During review, do not flag these specific full-path messages as a privacy/security issue as long as they are clearly part of the binary resolution steps. If full-path logging appears outside binary discovery/resolution, review/flag it as usual.
Applied to files:
extensions/goose/src/extension.tsextensions/goose/src/extension.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:
extensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
🔇 Additional comments (4)
extensions/goose/src/extension.ts (3)
19-42: LGTM!
55-62: LGTM!
70-78: LGTM!extensions/goose/src/extension.spec.ts (1)
78-84: LGTM!Also applies to: 106-167
0fb7be9 to
af81505
Compare
jeffmaury
left a comment
There was a problem hiding this comment.
no MCP support and looks too simple to support all providers
6b21847 to
0c063ca
Compare
|
@CodeRabbit resolve |
✅ Action performedComments resolved and changes approved. |
8751cb5 to
1b60c28
Compare
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 `@extensions/goose/src/extension.spec.ts`:
- Around line 65-504: The test suite repeats the same activate-and-agent lookup
in almost every case, so hoist that setup into a shared beforeEach and reuse a
suite-level agent variable. Update the extension.spec.ts tests around
activate(extensionContextMock) and
vi.mocked(agents.registerAgent).mock.calls[0]![0] so they initialize once per
test, while keeping it compatible with the existing top-level beforeEach that
resets mocks and context.
In `@extensions/goose/src/extension.ts`:
- Around line 63-64: The Goose extension is shipping a temporary personal base
image via the baseImage setting, which should be replaced before release. Update
the extension config in extension.ts to point to the official image instead of
the quay.io/bmahabir/openkaiden/openshell-goose:latest placeholder, and pin it
to an immutable digest or versioned tag rather than :latest. Keep the TODO
removed or updated so the baseImage value in the Goose extension is
deterministic and release-safe.
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts`:
- Around line 213-223: The sandbox creation path in AgentWorkspaceManager is
ignoring AgentWorkspaceSettings.DefaultBaseImage, so createOpenshell() still
passes only agent.baseImage into createSandbox(). Update the logic around
createSandbox() in agent-workspace-manager.ts to read the registered
DefaultBaseImage setting and use it as a fallback when agent.baseImage is
missing, keeping the existing agent value as the override. Make sure the change
is applied in the createOpenshell()/sandbox setup flow so the configured default
actually reaches openshellCli.createSandbox().
🪄 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: 94dd62fc-0278-4194-b98d-a8b2f23e0c98
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
extensions/goose/package.jsonextensions/goose/src/extension.spec.tsextensions/goose/src/extension.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/extension/extension-loader.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: unit tests / windows-2022
- GitHub Check: unit tests / macos-15
- GitHub Check: Linux
- GitHub Check: typecheck
- GitHub Check: Windows
- GitHub Check: macOS
- GitHub Check: linter, formatters
⚠️ CI failures not shown inline (4)
GitHub Actions: fullsend / dispatch _ Route: feat(goose): implement agent configuration for workspace preparation
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1m�[0m
�[36;1mif [[ ! "$STAGE" =~ ^[a-z][a-z0-9_-]*$ ]]; then�[0m
�[36;1m echo "::error::Invalid stage name: must start with lowercase letter and contain only [a-z0-9_-]"�[0m
GitHub Actions: fullsend / dispatch _ Route: feat(goose): implement agent configuration for workspace preparation
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mif [[ -f .fullsend/config.yaml ]]; then�[0m
�[36;1m KILL_SWITCH=$(yq '.kill_switch // false' .fullsend/config.yaml)�[0m
�[36;1m if [[ "$KILL_SWITCH" == "true" ]]; then�[0m
�[36;1m echo "::error::Kill switch is active — all agent dispatch halted"�[0m
GitHub Actions: fullsend / 6_dispatch _ Route.txt: feat(goose): implement agent configuration for workspace preparation
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1m�[0m
�[36;1mif [[ ! "$STAGE" =~ ^[a-z][a-z0-9_-]*$ ]]; then�[0m
�[36;1m echo "::error::Invalid stage name: must start with lowercase letter and contain only [a-z0-9_-]"�[0m
GitHub Actions: fullsend / dispatch _ Route: feat(goose): implement agent configuration for workspace preparation
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mEVENT_PAYLOAD=$(jq -c '{�[0m
�[36;1m issue: (.issue // null | if . then {number, html_url} else null end),�[0m
�[36;1m pull_request: (.pull_request // null | if . then {number, html_url,�[0m
�[36;1m head: {ref: .head.ref, sha: .head.sha, repo: {full_name: .head.repo.full_name}},�[0m
�[36;1m base: {ref: .base.ref, repo: {full_name: .base.repo.full_name}}} else null end),�[0m
�[36;1m comment: (.comment // null | if . then {body: .body[:4096]} else null end)�[0m
�[36;1m}' "$GITHUB_EVENT_PATH") || {�[0m
�[36;1m echo "::error::Failed to extract event payload from GITHUB_EVENT_PATH"�[0m
🧰 Additional context used
📓 Path-based instructions (7)
extensions/*/package.json
📄 CodeRabbit inference engine (AGENTS.md)
extensions/*/package.json: Extensions must declareengines.kaidenversion compatibility in theirpackage.json
Extensionpackage.jsonmust havemainfield pointing to./dist/extension.js
Configuration properties for API keys, tokens, or secrets must use"format": "password"in the configuration definition to ensure input masking in the UI
Files:
extensions/goose/package.json
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
/@/path aliases instead of relative paths for imports outside the current directory's module group; use relative imports only for sibling modules within the same directory
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/extension/extension-loader.tsextensions/goose/src/extension.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tsextensions/goose/src/extension.ts
packages/main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/main/src/**/*.{ts,tsx}: UseipcHandle()to expose handlers in the main process with naming convention<registry-name>:<action>(e.g.,container-provider-registry:listContainers)
UseapiSender.send()to send events from main process to renderer for real-time updates
Long-running operations should useTaskManager.createTask()with title and action configuration
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/extension/extension-loader.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
packages/{main,renderer,preload}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Container operations must include
engineIdparameter to identify the container engine
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/extension/extension-loader.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
**/*.spec.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.{ts,tsx,js,jsx}: Usetest()instead ofit()for test cases in Vitest unit tests
Usevi.mock(import('...'))for auto-mocking modules in unit tests; avoid manual mock factories when possible
Usevi.resetAllMocks()inbeforeEachhooks instead ofvi.clearAllMocks()for resetting mocks between tests
When an auto-mocked function or class method needs a real implementation, usevi.mocked(...)with the prototype pattern for class methods:vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)
Files:
extensions/goose/src/extension.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
extensions/*/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Register inference, container, and Kubernetes providers through the
ProviderRegistryvia extension APIs
Files:
extensions/goose/src/extension.spec.tsextensions/goose/src/extension.ts
extensions/*/src/extension.ts
📄 CodeRabbit inference engine (AGENTS.md)
Extensions should export a standard activation API from their entry point
Files:
extensions/goose/src/extension.ts
🧠 Learnings (9)
📚 Learning: 2026-05-06T11:29:33.170Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/package.json:9-11
Timestamp: 2026-05-06T11:29:33.170Z
Learning: In the openkaiden/kaiden repo, all built-in extensions under extensions/ should specify engines with kaiden: "^0.0.1" in package.json. Do not flag each extension individually; enforce a repo-wide alignment in a single PR. During reviews, verify that every extensions/*/package.json has "engines": { "kaiden": "^0.0.1" }. If a file deviates, surface the discrepancy as a single repo-wide task rather than per-file.
Applied to files:
extensions/goose/package.json
📚 Learning: 2026-03-09T08:47:09.657Z
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 1077
File: packages/main/src/plugin/skill/skill-manager.ts:80-109
Timestamp: 2026-03-09T08:47:09.657Z
Learning: In the kortex-hub/kortex repository, IPC handlers (via ipcHandle()) may be registered directly inside feature manager/service classes (e.g., SkillManager in packages/main/src/plugin/skill/skill-manager.ts) rather than exclusively in packages/main/src/plugin/index.ts. Treat this as an accepted design pattern for files under the plugin directory. Reviewers should not require centralization in index.ts; allow IPC registration proximity to the feature that owns the handler. When reviewing code, accept direct ipcHandle() registrations inside feature managers and ensure the pattern is consistently applied across similar feature-manager modules.
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/extension/extension-loader.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
📚 Learning: 2026-05-12T17:14:02.153Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1850
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte:66-70
Timestamp: 2026-05-12T17:14:02.153Z
Learning: When reviewing code that uses `AgentWorkspaceSummaryUI.runtime`, treat it as a required, non-null `string` per the `openkaiden/kdn-api` 0.12.0 schema. Therefore, code like `a.runtime.localeCompare(b.runtime)` is safe and should not trigger warnings about possible `undefined`/`null` values or suggestions to use nullish coalescing/optional chaining for `runtime` (unless the current local types still mark `runtime` as optional, indicating a schema/version mismatch).
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/extension/extension-loader.tsextensions/goose/src/extension.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tsextensions/goose/src/extension.ts
📚 Learning: 2026-06-29T13:16:53.102Z
Learnt from: benoitf
Repo: openkaiden/kaiden PR: 2296
File: extensions/container/packages/extension/src/helper/socket-finder/_socket-finder-module.ts:28-29
Timestamp: 2026-06-29T13:16:53.102Z
Learning: When reviewing imports in openkaiden/kaiden TypeScript/JavaScript files, prefer the configured `/@/` path alias instead of relative imports that would require traversing out of the current directory/module group (i.e., paths containing `..` that cross boundaries).
Do not require alias conversion for descendant-path relative imports within the socket-finder module directory—for example, in `extensions/container/packages/extension/src/helper/socket-finder/**`, imports like `./podman/podman-version-detector` and `./podman/podman-windows-finder` are acceptable and should not be flagged.
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/extension/extension-loader.tsextensions/goose/src/extension.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tsextensions/goose/src/extension.ts
📚 Learning: 2026-05-05T17:30:20.418Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/src/vertex-ai.ts:272-302
Timestamp: 2026-05-05T17:30:20.418Z
Learning: In the openkaiden/kaiden repo, for cloud inference provider extension code under `extensions/*/src/*.ts`, treat `ProviderConnectionStatus = 'unknown'` as a valid/expected value when registering provider connections (e.g., Gemini/Claude/Mistral/OpenAI-compatible/Vertex AI). `'unknown'` indicates the connection was set up but is not continuously monitored—so do not flag it as incorrect. Only Ollama is expected to use `'started'` because it actively polls a local server.
Applied to files:
extensions/goose/src/extension.spec.tsextensions/goose/src/extension.ts
📚 Learning: 2026-05-05T17:44:50.991Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/src/vertex-ai.ts:363-387
Timestamp: 2026-05-05T17:44:50.991Z
Learning: In this repo (openkaiden/kaiden), do not raise a code review issue when an extension’s `InferenceProviderConnectionFactory.create` factory method implementation omits (or does not use) the optional `logger` and/or `CancellationToken` parameters in its method signature/implementation. Current extensions (e.g., Vertex AI, Gemini, Claude, Mistral, OpenAI-compatible) follow this pattern, so reviewers should treat it as acceptable for `extensions/*` TypeScript source files.
Applied to files:
extensions/goose/src/extension.spec.tsextensions/goose/src/extension.ts
📚 Learning: 2026-05-12T10:01:14.248Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1810
File: extensions/kdn/src/kdn-extension.ts:43-46
Timestamp: 2026-05-12T10:01:14.248Z
Learning: In this repo’s extension code, when logging from binary discovery/resolution logic (e.g., choosing/validating custom paths, extension storage locations, or bundled resource paths), it’s intentional to include full filesystem paths in `console.log`/`console.warn` (such as in `extensions/**/src/*-extension.ts`). During review, do not flag these specific full-path messages as a privacy/security issue as long as they are clearly part of the binary resolution steps. If full-path logging appears outside binary discovery/resolution, review/flag it as usual.
Applied to files:
extensions/goose/src/extension.spec.tsextensions/goose/src/extension.ts
📚 Learning: 2026-05-06T11:15:56.238Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/src/extension.spec.ts:43-51
Timestamp: 2026-05-06T11:15:56.238Z
Learning: In all extensions under extensions/*/src/extension.ts, deactivate() should only clear the module-level instance reference (e.g., set the instance to undefined) and must not call dispose() directly. The dispose() method is invoked by the extension host when processing extensionContext.subscriptions. Do not suggest asserting dispose() in tests for deactivate(); such assertions are unnecessary because disposal is handled by the host and CI checks should validate subscriptions handling instead.
Applied to files:
extensions/goose/src/extension.ts
📚 Learning: 2026-06-18T08:20:05.553Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 2185
File: extensions/gemini/src/extension.ts:45-52
Timestamp: 2026-06-18T08:20:05.553Z
Learning: When reviewing openkaiden/kaiden extension code that registers `configurationFiles` via `agents.registerAgent()` (typically in `extensions/*/src/extension.ts`), do not flag an intentionally stub `read()` (e.g., returning `'{}'`) as causing data loss or as a missing file-I/O implementation. Per the framework, runtime behavior uses the declared `path` and the real, file-backed `read()`/`update()` implementations are supplied through `context.configurationFiles` during `preWorkspaceStart`.
Applied to files:
extensions/goose/src/extension.ts
🔇 Additional comments (9)
extensions/goose/src/extension.spec.ts (1)
63-79: LGTM!Also applies to: 81-266, 268-406, 408-504
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (1)
137-146: LGTM!Also applies to: 177-243
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts (1)
354-367: LGTM! Well-isolated test usingexpect.objectContainingto verifyfromis forwarded fromagent.baseImage.packages/main/src/plugin/extension/extension-loader.ts (1)
410-421: LGTM!extensions/goose/src/extension.ts (4)
19-51: LGTM!
81-112: LGTM!
124-154: LGTM!
113-122: 🎯 Functional CorrectnessThe non-Vertex AI path can stay config-only —
GOOSE_PROVIDER/OPENAI_BASE_URLenv mirroring is limited to the Vertex AI branch, and there’s no sandbox-level env injection for those keys elsewhere.> Likely an incorrect or invalid review comment.extensions/goose/package.json (1)
26-27: LGTM!
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: 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 `@extensions/goose/src/extension.spec.ts`:
- Around line 65-504: The test suite repeats the same activate-and-agent lookup
in almost every case, so hoist that setup into a shared beforeEach and reuse a
suite-level agent variable. Update the extension.spec.ts tests around
activate(extensionContextMock) and
vi.mocked(agents.registerAgent).mock.calls[0]![0] so they initialize once per
test, while keeping it compatible with the existing top-level beforeEach that
resets mocks and context.
In `@extensions/goose/src/extension.ts`:
- Around line 63-64: The Goose extension is shipping a temporary personal base
image via the baseImage setting, which should be replaced before release. Update
the extension config in extension.ts to point to the official image instead of
the quay.io/bmahabir/openkaiden/openshell-goose:latest placeholder, and pin it
to an immutable digest or versioned tag rather than :latest. Keep the TODO
removed or updated so the baseImage value in the Goose extension is
deterministic and release-safe.
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts`:
- Around line 213-223: The sandbox creation path in AgentWorkspaceManager is
ignoring AgentWorkspaceSettings.DefaultBaseImage, so createOpenshell() still
passes only agent.baseImage into createSandbox(). Update the logic around
createSandbox() in agent-workspace-manager.ts to read the registered
DefaultBaseImage setting and use it as a fallback when agent.baseImage is
missing, keeping the existing agent value as the override. Make sure the change
is applied in the createOpenshell()/sandbox setup flow so the configured default
actually reaches openshellCli.createSandbox().
🪄 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: 94dd62fc-0278-4194-b98d-a8b2f23e0c98
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
extensions/goose/package.jsonextensions/goose/src/extension.spec.tsextensions/goose/src/extension.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/extension/extension-loader.ts
📜 Review details
🔇 Additional comments (9)
extensions/goose/src/extension.spec.ts (1)
63-79: LGTM!Also applies to: 81-266, 268-406, 408-504
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (1)
137-146: LGTM!Also applies to: 177-243
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts (1)
354-367: LGTM! Well-isolated test usingexpect.objectContainingto verifyfromis forwarded fromagent.baseImage.packages/main/src/plugin/extension/extension-loader.ts (1)
410-421: LGTM!extensions/goose/src/extension.ts (4)
19-51: LGTM!
81-112: LGTM!
124-154: LGTM!
113-122: 🎯 Functional CorrectnessThe non-Vertex AI path can stay config-only —
GOOSE_PROVIDER/OPENAI_BASE_URLenv mirroring is limited to the Vertex AI branch, and there’s no sandbox-level env injection for those keys elsewhere.> Likely an incorrect or invalid review comment.extensions/goose/package.json (1)
26-27: LGTM!
🛑 Comments failed to post (3)
extensions/goose/src/extension.spec.ts (1)
65-504: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Extract repeated
activate+agentretrieval intobeforeEach.Nearly every
test()in this suite repeats the same two lines (await activate(extensionContextMock); const agent = vi.mocked(agents.registerAgent).mock.calls[0]![0];) about 20 times. Sinceagentdoesn't vary across tests, this can be hoisted into a sharedbeforeEach(or alet agentset once per suite), reducing duplication significantly.♻️ Suggested refactor
+ let agent: Parameters<typeof agents.registerAgent>[0]; + + beforeEach(async () => { + await activate(extensionContextMock); + agent = vi.mocked(agents.registerAgent).mock.calls[0]![0]; + }); + test('registered agent supports all model types', async () => { - await activate(extensionContextMock); - - const agent = vi.mocked(agents.registerAgent).mock.calls[0]![0]; expect(agent.isSupportedModelType!({ name: 'gemini' })).toBe(true); expect(agent.isSupportedModelType!({ name: 'openai' })).toBe(true); });Note: this needs to interact correctly with the existing top-level
beforeEach(not shown in this diff) that presumably resetsextensionContextMock/mocks between tests.🤖 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 `@extensions/goose/src/extension.spec.ts` around lines 65 - 504, The test suite repeats the same activate-and-agent lookup in almost every case, so hoist that setup into a shared beforeEach and reuse a suite-level agent variable. Update the extension.spec.ts tests around activate(extensionContextMock) and vi.mocked(agents.registerAgent).mock.calls[0]![0] so they initialize once per test, while keeping it compatible with the existing top-level beforeEach that resets mocks and context.extensions/goose/src/extension.ts (1)
63-64: 📐 Maintainability & Code Quality | 🔵 Trivial
Temporary personal base image shipped in the extension.
baseImagepoints to a personal quay namespace pinned to:latest(quay.io/bmahabir/openkaiden/openshell-goose:latest). A mutable:latesttag on a personal registry is non-deterministic and a supply-chain/availability risk for a released extension. The TODO acknowledges this is temporary.Want me to open a tracking issue to swap in the official, digest-pinned image before release?
🤖 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 `@extensions/goose/src/extension.ts` around lines 63 - 64, The Goose extension is shipping a temporary personal base image via the baseImage setting, which should be replaced before release. Update the extension config in extension.ts to point to the official image instead of the quay.io/bmahabir/openkaiden/openshell-goose:latest placeholder, and pin it to an immutable digest or versioned tag rather than :latest. Keep the TODO removed or updated so the baseImage value in the Goose extension is deterministic and release-safe.packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (1)
213-223: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Check if DefaultBaseImage setting is read/consumed anywhere in the codebase. rg -n 'DefaultBaseImage' -C3Repository: openkaiden/kaiden
Length of output: 155
🏁 Script executed:
#!/bin/bash set -euo pipefail printf 'Files of interest:\n' git ls-files 'packages/main/src/plugin/agent-workspace/*' 'packages/main/src/**/agent*' 'packages/main/src/**/configuration*' | sed 's#^`#-` #' printf '\nSearch for agent workspace settings and base image handling:\n' rg -n 'AgentWorkspaceSettings|baseImage|createSandbox\(|getAgentRegistration|DefaultBaseImage' packages/main/src -C 3 printf '\nOutline candidate file:\n' ast-grep outline packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts --view expandedRepository: openkaiden/kaiden
Length of output: 39142
🏁 Script executed:
#!/bin/bash set -euo pipefail printf 'agent-registry.ts excerpt:\n' sed -n '1,220p' packages/main/src/plugin/agent-registry.ts | cat -n | sed -n '1,220p' printf '\nagent-workspace-manager.ts excerpt around createOpenshell and init:\n' sed -n '130,230p' packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts | cat -n printf '\n' sed -n '529,550p' packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts | cat -n printf '\nAll DefaultBaseImage references in repo:\n' rg -n 'DefaultBaseImage' .Repository: openkaiden/kaiden
Length of output: 13245
🏁 Script executed:
#!/bin/bash set -euo pipefail printf 'openshell-cli.ts around createSandbox args:\n' sed -n '150,230p' packages/main/src/plugin/openshell-cli/openshell-cli.ts | cat -n printf '\nCreateSandboxOptions definition:\n' rg -n 'type CreateSandboxOptions|interface CreateSandboxOptions' packages/main/src/plugin/openshell-cli/openshell-cli.ts -n -C 3Repository: openkaiden/kaiden
Length of output: 3080
Wire
DefaultBaseImageinto sandbox creation.
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts:213-223, 529-544
AgentWorkspaceSettings.DefaultBaseImageis registered, but nothing reads it, socreateOpenshell()still passes onlyagent.baseImage. When an agent omitsbaseImage, the configured default never reachescreateSandbox().🤖 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 213 - 223, The sandbox creation path in AgentWorkspaceManager is ignoring AgentWorkspaceSettings.DefaultBaseImage, so createOpenshell() still passes only agent.baseImage into createSandbox(). Update the logic around createSandbox() in agent-workspace-manager.ts to read the registered DefaultBaseImage setting and use it as a fallback when agent.baseImage is missing, keeping the existing agent value as the override. Make sure the change is applied in the createOpenshell()/sandbox setup flow so the configured default actually reaches openshellCli.createSandbox().
|
@CodeRabbit resolve |
✅ Action performedComments resolved and changes approved. |
| if (!content.trim()) { | ||
| return {}; | ||
| } | ||
| return (load(content) as GooseConfig) ?? {}; |
There was a problem hiding this comment.
suggestion: use zod to parse the output and validate
| }, | ||
| command: 'goose', | ||
| // TODO: replace with official image once available — temporary testing image | ||
| baseImage: 'quay.io/bmahabir/openkaiden/openshell-goose:latest', |
There was a problem hiding this comment.
will remove but how can I point reviewers to use this image for testing? maybe ask them for a small code change?
There was a problem hiding this comment.
ok removed and updated testing plan
| expect(agent.isSupportedModelType!({ name: 'gemini' })).toBe(true); | ||
| expect(agent.isSupportedModelType!({ name: 'openai' })).toBe(true); | ||
| expect(agent.isSupportedModelType!({ name: 'anthropic' })).toBe(true); | ||
| expect(agent.isSupportedModelType!({ name: 'vertexai' })).toBe(false); |
There was a problem hiding this comment.
thanks good catch
0be3a53 to
cee2f9d
Compare
|
@CodeRabbit resolve |
✅ Action performedComments resolved and changes approved. |
422ccdd to
57b21a6
Compare
|
@CodeRabbit resolve |
✅ Action performedComments resolved and changes approved. |
57b21a6 to
c290987
Compare
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 `@extensions/goose/package.json`:
- Around line 26-28: The dependency list in package.json still pins js-yaml to a
vulnerable 4.1.1 release; update the js-yaml entry to 4.2.0 or newer in the
extensions/goose package manifest. Keep the rest of the dependency block
unchanged and verify any lockfile or workspace resolution is refreshed so the
upgraded version is actually used.
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts`:
- Line 219: The agent workspace image setup is ignoring the configured default
base image because `from` is always sourced from `agent.baseImage` in
`createAgentWorkspace`; update this call to fall back to
`agentWorkspace.defaultBaseImage` when the agent does not provide `baseImage`,
or remove the unused config if it should not apply. Use the
`agentWorkspace.defaultBaseImage` setting alongside `agent.baseImage` in the
`agent-workspace-manager` path so the workspace build gets a valid image source.
🪄 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: 1a0084f7-b801-44f0-a971-0bdd5122309d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
extensions/goose/package.jsonextensions/goose/src/extension.spec.tsextensions/goose/src/extension.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: macOS
- GitHub Check: typecheck
- GitHub Check: Detect pnpm lock or pr-check files changes
- GitHub Check: Linux
- GitHub Check: linter, formatters
- GitHub Check: dispatch / Route
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
/@/path aliases instead of relative paths for imports outside the current directory's module group; use relative imports only for sibling modules within the same directory
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tsextensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
packages/main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/main/src/**/*.{ts,tsx}: UseipcHandle()to expose handlers in the main process with naming convention<registry-name>:<action>(e.g.,container-provider-registry:listContainers)
UseapiSender.send()to send events from main process to renderer for real-time updates
Long-running operations should useTaskManager.createTask()with title and action configuration
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
packages/{main,renderer,preload}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Container operations must include
engineIdparameter to identify the container engine
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
**/*.spec.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.{ts,tsx,js,jsx}: Usetest()instead ofit()for test cases in Vitest unit tests
Usevi.mock(import('...'))for auto-mocking modules in unit tests; avoid manual mock factories when possible
Usevi.resetAllMocks()inbeforeEachhooks instead ofvi.clearAllMocks()for resetting mocks between tests
When an auto-mocked function or class method needs a real implementation, usevi.mocked(...)with the prototype pattern for class methods:vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tsextensions/goose/src/extension.spec.ts
extensions/*/package.json
📄 CodeRabbit inference engine (AGENTS.md)
extensions/*/package.json: Extensions must declareengines.kaidenversion compatibility in theirpackage.json
Extensionpackage.jsonmust havemainfield pointing to./dist/extension.js
Configuration properties for API keys, tokens, or secrets must use"format": "password"in the configuration definition to ensure input masking in the UI
Files:
extensions/goose/package.json
extensions/*/src/extension.ts
📄 CodeRabbit inference engine (AGENTS.md)
Extensions should export a standard activation API from their entry point
Files:
extensions/goose/src/extension.ts
extensions/*/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Register inference, container, and Kubernetes providers through the
ProviderRegistryvia extension APIs
Files:
extensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
🧠 Learnings (9)
📚 Learning: 2026-03-09T08:47:09.657Z
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 1077
File: packages/main/src/plugin/skill/skill-manager.ts:80-109
Timestamp: 2026-03-09T08:47:09.657Z
Learning: In the kortex-hub/kortex repository, IPC handlers (via ipcHandle()) may be registered directly inside feature manager/service classes (e.g., SkillManager in packages/main/src/plugin/skill/skill-manager.ts) rather than exclusively in packages/main/src/plugin/index.ts. Treat this as an accepted design pattern for files under the plugin directory. Reviewers should not require centralization in index.ts; allow IPC registration proximity to the feature that owns the handler. When reviewing code, accept direct ipcHandle() registrations inside feature managers and ensure the pattern is consistently applied across similar feature-manager modules.
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
📚 Learning: 2026-05-12T17:14:02.153Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1850
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte:66-70
Timestamp: 2026-05-12T17:14:02.153Z
Learning: When reviewing code that uses `AgentWorkspaceSummaryUI.runtime`, treat it as a required, non-null `string` per the `openkaiden/kdn-api` 0.12.0 schema. Therefore, code like `a.runtime.localeCompare(b.runtime)` is safe and should not trigger warnings about possible `undefined`/`null` values or suggestions to use nullish coalescing/optional chaining for `runtime` (unless the current local types still mark `runtime` as optional, indicating a schema/version mismatch).
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tsextensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
📚 Learning: 2026-06-29T13:16:53.102Z
Learnt from: benoitf
Repo: openkaiden/kaiden PR: 2296
File: extensions/container/packages/extension/src/helper/socket-finder/_socket-finder-module.ts:28-29
Timestamp: 2026-06-29T13:16:53.102Z
Learning: When reviewing imports in openkaiden/kaiden TypeScript/JavaScript files, prefer the configured `/@/` path alias instead of relative imports that would require traversing out of the current directory/module group (i.e., paths containing `..` that cross boundaries).
Do not require alias conversion for descendant-path relative imports within the socket-finder module directory—for example, in `extensions/container/packages/extension/src/helper/socket-finder/**`, imports like `./podman/podman-version-detector` and `./podman/podman-windows-finder` are acceptable and should not be flagged.
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tsextensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
📚 Learning: 2026-05-06T11:29:33.170Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/package.json:9-11
Timestamp: 2026-05-06T11:29:33.170Z
Learning: In the openkaiden/kaiden repo, all built-in extensions under extensions/ should specify engines with kaiden: "^0.0.1" in package.json. Do not flag each extension individually; enforce a repo-wide alignment in a single PR. During reviews, verify that every extensions/*/package.json has "engines": { "kaiden": "^0.0.1" }. If a file deviates, surface the discrepancy as a single repo-wide task rather than per-file.
Applied to files:
extensions/goose/package.json
📚 Learning: 2026-05-05T17:30:20.418Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/src/vertex-ai.ts:272-302
Timestamp: 2026-05-05T17:30:20.418Z
Learning: In the openkaiden/kaiden repo, for cloud inference provider extension code under `extensions/*/src/*.ts`, treat `ProviderConnectionStatus = 'unknown'` as a valid/expected value when registering provider connections (e.g., Gemini/Claude/Mistral/OpenAI-compatible/Vertex AI). `'unknown'` indicates the connection was set up but is not continuously monitored—so do not flag it as incorrect. Only Ollama is expected to use `'started'` because it actively polls a local server.
Applied to files:
extensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
📚 Learning: 2026-05-05T17:44:50.991Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/src/vertex-ai.ts:363-387
Timestamp: 2026-05-05T17:44:50.991Z
Learning: In this repo (openkaiden/kaiden), do not raise a code review issue when an extension’s `InferenceProviderConnectionFactory.create` factory method implementation omits (or does not use) the optional `logger` and/or `CancellationToken` parameters in its method signature/implementation. Current extensions (e.g., Vertex AI, Gemini, Claude, Mistral, OpenAI-compatible) follow this pattern, so reviewers should treat it as acceptable for `extensions/*` TypeScript source files.
Applied to files:
extensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
📚 Learning: 2026-05-06T11:15:56.238Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/src/extension.spec.ts:43-51
Timestamp: 2026-05-06T11:15:56.238Z
Learning: In all extensions under extensions/*/src/extension.ts, deactivate() should only clear the module-level instance reference (e.g., set the instance to undefined) and must not call dispose() directly. The dispose() method is invoked by the extension host when processing extensionContext.subscriptions. Do not suggest asserting dispose() in tests for deactivate(); such assertions are unnecessary because disposal is handled by the host and CI checks should validate subscriptions handling instead.
Applied to files:
extensions/goose/src/extension.ts
📚 Learning: 2026-06-18T08:20:05.553Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 2185
File: extensions/gemini/src/extension.ts:45-52
Timestamp: 2026-06-18T08:20:05.553Z
Learning: When reviewing openkaiden/kaiden extension code that registers `configurationFiles` via `agents.registerAgent()` (typically in `extensions/*/src/extension.ts`), do not flag an intentionally stub `read()` (e.g., returning `'{}'`) as causing data loss or as a missing file-I/O implementation. Per the framework, runtime behavior uses the declared `path` and the real, file-backed `read()`/`update()` implementations are supplied through `context.configurationFiles` during `preWorkspaceStart`.
Applied to files:
extensions/goose/src/extension.ts
📚 Learning: 2026-05-12T10:01:14.248Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1810
File: extensions/kdn/src/kdn-extension.ts:43-46
Timestamp: 2026-05-12T10:01:14.248Z
Learning: In this repo’s extension code, when logging from binary discovery/resolution logic (e.g., choosing/validating custom paths, extension storage locations, or bundled resource paths), it’s intentional to include full filesystem paths in `console.log`/`console.warn` (such as in `extensions/**/src/*-extension.ts`). During review, do not flag these specific full-path messages as a privacy/security issue as long as they are clearly part of the binary resolution steps. If full-path logging appears outside binary discovery/resolution, review/flag it as usual.
Applied to files:
extensions/goose/src/extension.tsextensions/goose/src/extension.spec.ts
🔇 Additional comments (4)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts (1)
378-391: LGTM!extensions/goose/src/extension.ts (2)
78-168: LGTM!
45-63: No change needed herez.codec()withinvalid_format/z.NEVERis valid in Zod 4.> Likely an incorrect or invalid review comment.extensions/goose/src/extension.spec.ts (1)
63-465: LGTM!
| "@openkaiden/api": "workspace:*", | ||
| "js-yaml": "^4.1.1", | ||
| "zod": "^4.3.5" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check js-yaml package for security advisories
# Check npm registry for latest version
echo "=== Latest js-yaml version ==="
npm view js-yaml version 2>/dev/null
# Check for security advisories via GitHub
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: NPM, package: "js-yaml") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}' 2>/dev/null || echo "Could not query GitHub API"Repository: openkaiden/kaiden
Length of output: 1770
🌐 Web query:
Are there any known security vulnerabilities in js-yaml version 4.1.1? What is the latest stable version of js-yaml?
💡 Result:
Yes, js-yaml version 4.1.1 has a known security vulnerability. It is susceptible to a denial-of-service (DoS) attack through algorithmic CPU exhaustion, identified as CVE-2026-53550 [1][2]. This vulnerability involves the merge-key processing (<<) feature, where a crafted YAML document containing repeated aliases can cause quadratic parse-time behavior [1][2]. This can result in significant CPU consumption, potentially blocking the Node.js event loop [1][2]. This issue was addressed and fixed in version 4.2.0 [2]. As of July 9, 2026, the latest stable version of js-yaml is 5.2.1 [3][4][5]. Users are encouraged to upgrade to the latest version to benefit from security fixes and improvements [3][2].
Citations:
- 1: GHSA-h67p-54hq-rp68
- 2: https://nvd.nist.gov/vuln/detail/cve-2026-53550
- 3: https://www.npmjs.com/package/js-yaml
- 4: https://registry.npmjs.org/js-yaml
- 5: https://security.snyk.io/package/npm/js-yaml
Upgrade js-yaml from 4.1.1 — that version is still covered by a moderate DoS advisory in merge-key handling; use 4.2.0 or newer.
🤖 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 `@extensions/goose/package.json` around lines 26 - 28, The dependency list in
package.json still pins js-yaml to a vulnerable 4.1.1 release; update the
js-yaml entry to 4.2.0 or newer in the extensions/goose package manifest. Keep
the rest of the dependency block unchanged and verify any lockfile or workspace
resolution is refreshed so the upgraded version is actually used.
| try { | ||
| await this.openshellCli.createSandbox({ | ||
| name: sandboxName, | ||
| from: agent.baseImage, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for DefaultBaseImage usage across the codebase
rg -n 'DefaultBaseImage' --type=tsRepository: openkaiden/kaiden
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file and nearby config definition
sed -n '200,260p' packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
printf '\n---\n'
sed -n '530,570p' packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
# Look for any non-TypeScript references or alternative names for the setting
rg -n 'DefaultBaseImage|default base image|base image for agent workspaces' .Repository: openkaiden/kaiden
Length of output: 4109
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the settings definition and any code that reads the default base image
sed -n '1,120p' packages/api/src/agent-workspace/agent-workspace-settings.ts
printf '\n---\n'
rg -n 'defaultBaseImage|DefaultBaseImage|AgentWorkspaceSettings\.DefaultBaseImage|SectionName' packages -g '!**/dist/**' -g '!**/build/**'Repository: openkaiden/kaiden
Length of output: 23589
Fallback to agentWorkspace.defaultBaseImage here packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts:219
agent.baseImage is passed through directly, and agentWorkspace.defaultBaseImage is never read anywhere else. If an agent omits baseImage, this call leaves from undefined and the configured default never takes effect. Wire the setting in here, or remove the unused config if it’s not meant to apply.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts` at line
219, The agent workspace image setup is ignoring the configured default base
image because `from` is always sourced from `agent.baseImage` in
`createAgentWorkspace`; update this call to fall back to
`agentWorkspace.defaultBaseImage` when the agent does not provide `baseImage`,
or remove the unused config if it should not apply. Use the
`agentWorkspace.defaultBaseImage` setting alongside `agent.baseImage` in the
`agent-workspace-manager` path so the workspace build gets a valid image source.
…mage support Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Brian <bmahabir@bu.edu>
…ion disabled Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Brian Mahabir <56164556+bmahabirbu@users.noreply.github.com>
- Add tags: ['Local'] to registerAgent for UI categorization
- Keep goose (and mistral) in the disabled-by-default extensions list
- Use String(err) in codec error message to preserve parse error details
- Extract nonEmpty() helper for conditional record inclusion
- Use direct mutation for config.extensions instead of spread copy
- Simplify tests: extract beforeEach for agent setup, streamline
createConfigFile/createContext helpers
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Brian <bmahabir@bu.edu>
c290987 to
bc5b1b4
Compare
Summary
configurationFilesandpreWorkspaceStartstubs with a real implementation that writesGOOSE_MODELinto.goose/config.yamlfrom the workspace context, preserving existing configuration fields.Fixes #2174
Test plan
Before testing youll need to set the baseImage property locally in extensions/goose/src/extension.ts:
baseImage: 'quay.io/bmahabir/openkaiden/openshell-goose:latest',Now create a sandbox using goose with a skill and specifically the playwright mcp server!
Vertex ai claude support is blocked because of goose requires GOOSE_PROVIDER=anthropic but openshell will set it to google-vertex-ai because of how kaiden created the provider secret. openshell will override any user set env var override here
for goose
-provider anthropicon the CLI overrides the openshell-injected GOOSE_PROVIDER=gcp_vertex_aiBesides testing just the goose command with a model assert mcp and skills are there too
Goose does not have an mcp list feature so enter configure and scroll down to see playwright mcp listed and ready to go
For skills u can list them