feat(openclaw): implement agent configuration for workspace preparation#2193
feat(openclaw): implement agent configuration for workspace preparation#2193bmahabirbu wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThe Openclaw extension gains typed ChangesOpenclaw configuration file handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 |
2ff701d to
26facc5
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| const content = await configFile.read(); | ||
| let config: Record<string, unknown>; | ||
| try { | ||
| const parsed: unknown = JSON.parse(content); | ||
| config = | ||
| typeof parsed === 'object' && parsed !== null && !Array.isArray(parsed) | ||
| ? (parsed as Record<string, unknown>) | ||
| : {}; | ||
| } catch { | ||
| config = {}; | ||
| } | ||
|
|
||
| const agents = (config.agents as Record<string, unknown> | undefined) ?? {}; | ||
| const defaults = (agents.defaults as Record<string, unknown> | undefined) ?? {}; |
There was a problem hiding this comment.
might use zod to have validation of the JSON as well as easing how to retrieve the values
It seems there is work in progress or a command to grab the schema from openclaw
309d431 to
e97937b
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/openclaw/src/extension.spec.ts`:
- Around line 83-89: The `createContext` helper function is returning an object
missing the required `workspace` property defined in the `AgentWorkspaceContext`
interface. Add the `workspace` property to the return object (alongside the
existing `model` and `configurationFiles` properties) with an appropriate value
that satisfies the interface requirements.
🪄 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: da9138ea-52e9-43aa-984b-4620d41245ff
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
extensions/openclaw/package.jsonextensions/openclaw/src/extension.spec.tsextensions/openclaw/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: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: Linux
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: macOS
- GitHub Check: unit tests / windows-2022
- GitHub Check: unit tests / macos-15
- GitHub Check: linter, formatters
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: typecheck
- GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/openclaw/src/extension.tsextensions/openclaw/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/openclaw/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/openclaw/src/extension.tsextensions/openclaw/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/openclaw/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/openclaw/package.json
🧠 Learnings (7)
📚 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/openclaw/src/extension.tsextensions/openclaw/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/openclaw/src/extension.tsextensions/openclaw/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/openclaw/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/openclaw/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/openclaw/src/extension.tsextensions/openclaw/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/openclaw/src/extension.tsextensions/openclaw/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/openclaw/package.json
🔇 Additional comments (3)
extensions/openclaw/package.json (1)
23-24: LGTM!extensions/openclaw/src/extension.ts (1)
19-52: LGTM!Also applies to: 65-96
extensions/openclaw/src/extension.spec.ts (1)
19-23: LGTM!Also applies to: 70-77, 91-198
| return { | ||
| model: { | ||
| model: { label: modelLabel }, | ||
| }, | ||
| configurationFiles: configFiles, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify AgentWorkspaceContext contract and helper shape.
set -euo pipefail
echo "1) Confirm AgentWorkspaceContext requires workspace"
rg -n -C3 "interface AgentWorkspaceContext|readonly workspace" packages/extension-api/src/extension-api.d.ts
echo
echo "2) Inspect createContext helper implementation"
rg -n -C5 "function createContext|return \\{" extensions/openclaw/src/extension.spec.tsRepository: openkaiden/kaiden
Length of output: 1189
Add required workspace property to createContext return value.
The createContext helper returns an object typed as AgentWorkspaceContext but omits the required workspace property. This violates the interface contract and will cause TypeScript type-checking to fail.
Proposed fix
return {
model: {
model: { label: modelLabel },
},
configurationFiles: configFiles,
+ workspace: {} as AgentWorkspaceContext['workspace'],
};📝 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.
| return { | |
| model: { | |
| model: { label: modelLabel }, | |
| }, | |
| configurationFiles: configFiles, | |
| }; | |
| } | |
| return { | |
| model: { | |
| model: { label: modelLabel }, | |
| }, | |
| configurationFiles: configFiles, | |
| workspace: {} as AgentWorkspaceContext['workspace'], | |
| }; |
🤖 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/openclaw/src/extension.spec.ts` around lines 83 - 89, The
`createContext` helper function is returning an object missing the required
`workspace` property defined in the `AgentWorkspaceContext` interface. Add the
`workspace` property to the return object (alongside the existing `model` and
`configurationFiles` properties) with an appropriate value that satisfies the
interface requirements.
e97937b to
6fd1893
Compare
Replaces the configurationFiles and preWorkspaceStart stubs with a real implementation that writes the selected model into openclaw.json at agents.defaults.model, preserving existing configuration fields. Fixes: openkaiden#2175 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Brian <bmahabir@bu.edu>
…ation Use zod schemas for type-safe config parsing with graceful fallbacks on malformed input, replacing ad-hoc inline type assertions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Brian <bmahabir@bu.edu>
6fd1893 to
6fef9d7
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
extensions/openclaw/src/extension.spec.ts (1)
83-88:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd required
workspaceproperty tocreateContextreturn value.The
createContexthelper returns an object typed asAgentWorkspaceContextbut omits the requiredworkspaceproperty. This violates the interface contract.🔧 Proposed fix
return { model: { model: { label: modelLabel }, }, configurationFiles: configFiles, + workspace: {} as AgentWorkspaceContext['workspace'], };🤖 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/openclaw/src/extension.spec.ts` around lines 83 - 88, The createContext helper function return value is missing the required workspace property that is defined in the AgentWorkspaceContext interface. Update the return object in the createContext function to include the workspace property alongside the existing model and configurationFiles properties to satisfy the interface contract.
🤖 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.
Duplicate comments:
In `@extensions/openclaw/src/extension.spec.ts`:
- Around line 83-88: The createContext helper function return value is missing
the required workspace property that is defined in the AgentWorkspaceContext
interface. Update the return object in the createContext function to include the
workspace property alongside the existing model and configurationFiles
properties to satisfy the interface contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5e55d6a4-dc61-4c3a-9a25-8eda79f57415
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
extensions/openclaw/package.jsonextensions/openclaw/src/extension.spec.tsextensions/openclaw/src/extension.ts
📜 Review details
🧰 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/openclaw/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/openclaw/src/extension.spec.tsextensions/openclaw/src/extension.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/openclaw/src/extension.spec.ts
extensions/*/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Register inference, container, and Kubernetes providers through the
ProviderRegistryvia extension APIs
Files:
extensions/openclaw/src/extension.spec.tsextensions/openclaw/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/openclaw/src/extension.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/openclaw/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/openclaw/src/extension.spec.tsextensions/openclaw/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/openclaw/src/extension.spec.tsextensions/openclaw/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/openclaw/src/extension.spec.tsextensions/openclaw/src/extension.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/openclaw/src/extension.spec.tsextensions/openclaw/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/openclaw/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/openclaw/src/extension.ts
🔇 Additional comments (6)
extensions/openclaw/src/extension.ts (2)
19-52: LGTM!
65-95: LGTM!extensions/openclaw/src/extension.spec.ts (3)
19-23: LGTM!
70-77: LGTM!
91-198: LGTM!extensions/openclaw/package.json (1)
23-24: No issues found. Theengines.kaidenfield is properly declared with"^0.0.1".
Summary
configurationFilesandpreWorkspaceStartstubs with a real implementation that writes the selected model intoopenclaw.jsonatagents.defaults.model, preserving existing configuration fields.Fixes #2175
Test plan
🤖 Generated with Claude Code