fix(agent-workspaces): inject Vertex AI config into workspace at creation time - #1877
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent 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)
🧰 Additional context used📓 Path-based instructions (4)**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/main/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/{main,renderer,preload}/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.spec.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2026-03-09T08:47:09.657ZApplied to files:
📚 Learning: 2026-05-12T17:14:02.153ZApplied to files:
🔇 Additional comments (6)
📝 WalkthroughWalkthroughDetects ChangesVertex AI Workspace Configuration
Sequence Diagram(s)sequenceDiagram
participant AgentWorkspaceManager
participant applyVertexAiConfiguration
participant WorkspaceConfiguration
participant SecretManager
AgentWorkspaceManager->>applyVertexAiConfiguration: delegate with vertexai credentials
applyVertexAiConfiguration->>WorkspaceConfiguration: set env vars and ADC read-only mount (expand ~/)
AgentWorkspaceManager--x SecretManager: skip secretManager.create (no secret created)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (1)
135-143:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVertex AI config is skipped when any workspace secrets already exist.
Line 135 returns before the Vertex AI branch, so a workspace with pre-existing
workspaceConfiguration.secretsnever gets the required Vertex env vars/mounts. That can still cause auth failures for Vertex models.Suggested fix
async ensureModelSecret(options: AgentWorkspaceCreateOptions): Promise<void> { if (!options.model) return; - if (options.workspaceConfiguration?.secrets?.length) return; - const connectionInfo = this.providerRegistry.getInferenceConnectionCredentials(options.model); if (!connectionInfo) return; if (connectionInfo.llmMetadataName === 'vertexai') { this.applyVertexAiConfiguration(options, connectionInfo.credentials); return; } + + if (options.workspaceConfiguration?.secrets?.length) return;🤖 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 135 - 143, The current early return when options.workspaceConfiguration?.secrets?.length is truthy prevents Vertex AI configuration from being applied; update the logic so Vertex AI env/mount setup always runs when providerRegistry.getInferenceConnectionCredentials(options.model) yields connectionInfo with connectionInfo.llmMetadataName === 'vertexai'. Concretely, either move the vertexai branch (check connectionInfo.llmMetadataName === 'vertexai' and call this.applyVertexAiConfiguration(options, connectionInfo.credentials)) before the secrets check, or change the secrets guard to skip only secret creation but not the vertexai branch; keep references to options.workspaceConfiguration, providerRegistry.getInferenceConnectionCredentials, connectionInfo.llmMetadataName, and applyVertexAiConfiguration to locate and modify the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts`:
- Around line 135-143: The current early return when
options.workspaceConfiguration?.secrets?.length is truthy prevents Vertex AI
configuration from being applied; update the logic so Vertex AI env/mount setup
always runs when
providerRegistry.getInferenceConnectionCredentials(options.model) yields
connectionInfo with connectionInfo.llmMetadataName === 'vertexai'. Concretely,
either move the vertexai branch (check connectionInfo.llmMetadataName ===
'vertexai' and call this.applyVertexAiConfiguration(options,
connectionInfo.credentials)) before the secrets check, or change the secrets
guard to skip only secret creation but not the vertexai branch; keep references
to options.workspaceConfiguration,
providerRegistry.getInferenceConnectionCredentials,
connectionInfo.llmMetadataName, and applyVertexAiConfiguration to locate and
modify the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f49cb52f-af16-4118-99f8-3176060828d2
📒 Files selected for processing (2)
packages/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 of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: macOS
- GitHub Check: typecheck
- GitHub Check: Linux
- GitHub Check: Windows
- GitHub Check: unit tests / macos-15
- GitHub Check: unit tests / windows-2025
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: linter, formatters
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
/@/path aliases instead of relative paths for imports outside the current directory's module group; use relative imports only for sibling modules within the same directory
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.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.ts
🧠 Learnings (2)
📚 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.ts
|
It's a quick n' dirty fix until we implement #1834 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…tion time When a Vertex AI model is selected, ensureModelSecret now populates the workspace configuration with the required environment variables (CLAUDE_CODE_USE_VERTEX, CLOUD_ML_REGION, ANTHROPIC_VERTEX_PROJECT_ID) and mounts the ADC credentials file. Previously this was only done by the onboarding panel, which silently skipped it when a connection already existed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Fred Bricon <fbricon@gmail.com>
|
(hit the rebase button to see if e2e tests are passing now) |
2 participants
When a Vertex AI model is selected, ensureModelSecret now populates
the workspace configuration with the required environment variables
(CLAUDE_CODE_USE_VERTEX, CLOUD_ML_REGION, ANTHROPIC_VERTEX_PROJECT_ID)
and mounts the ADC credentials file. Previously this was only done
by the onboarding panel, which silently skipped it when a connection
already existed.
Fixes #1861