fix(agent-workspace): auto-create kdn vault secret from provider credentials on workspace creation - #1762
Conversation
…entials on workspace creation When creating a workspace with a cloud model selected, the provider connection's credentials are now used to automatically create a matching kdn vault secret so the CLI can inject API keys at runtime. This avoids requiring users to manually duplicate their API key in the secret vault after configuring a provider via Settings. Skips providers that don't use simple API-key auth (Vertex AI, Ollama) and defers to existing secrets when the onboarding flow has already configured them via workspaceConfiguration. Also fixes a pre-existing bug in KdnCli.createSecret where the --headerTemplate flag was incorrectly passed as --header-template (kebab-case), causing all type=other secret creations with a header template to fail silently. Closes openkaiden#1738 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Marcel Bertagnini <mbertagn@redhat.com>
📝 WalkthroughWalkthroughThis PR implements automatic API-key secret creation during workspace provisioning. It adds a credential resolution interface, enables ProviderRegistry to look up inference connection credentials by composite model identifier, updates KdnCli flag naming, and integrates credential discovery and secret auto-creation into AgentWorkspaceManager with comprehensive test coverage. ChangesInference Credentials & Workspace Secrets
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
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 `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts`:
- Around line 308-564: Add a regression test that omits options.name and
verifies secret naming uses the workspace folder from sourcePath (not a fallback
constant): in the ensureModelSecret tests create an
AgentWorkspaceCreateOptions-like object without name (e.g., sourcePath:
'/tmp/my-project', model: 'anthropic::claude::'), mock
providerRegistry.getInferenceConnectionCredentials to return anthropic
credentials and mock secretManager.create to resolve, then call
manager.ensureModelSecret(...) and assert secretManager.create was invoked with
name 'my-project-anthropic' and that options.secrets contains
'my-project-anthropic'; you can also add a corresponding buildSecretOptions unit
test calling manager.buildSecretOptions(...) with llmMetadataName 'anthropic'
and sourcePath '/tmp/my-project' (and no workspace name) and assert the returned
name is 'my-project-anthropic'.
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts`:
- Around line 129-130: The current fallback to the literal 'workspace' when
calling buildSecretOptions (using options.name ?? 'workspace') causes
secret-name collisions for unnamed workspaces; change the fallback to a unique
identifier instead (for example use options.name ?? options.id ??
connectionInfo.id ?? generate a short UUID/timestamp) so each unnamed workspace
produces a distinct secret name, ensure the same change is applied to the other
occurrence where options.name ?? 'workspace' is used, and keep secret-name
normalization/sanitization logic inside buildSecretOptions so generated IDs
produce valid secret keys.
🪄 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: 8172f023-0b46-4ccf-a465-bdfc5e74fc41
📒 Files selected for processing (7)
packages/api/src/provider-info.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/kdn-cli/kdn-cli.spec.tspackages/main/src/plugin/kdn-cli/kdn-cli.tspackages/main/src/plugin/provider-registry.spec.tspackages/main/src/plugin/provider-registry.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 (dev) / ubuntu-24.04 (ollama)
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: unit tests / macos-15
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: Windows
- GitHub Check: unit tests / windows-2025
- GitHub Check: typecheck
- GitHub Check: Linux
- GitHub Check: macOS
- GitHub Check: linter, formatters
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
/@/path aliases (e.g.,'/@/plugin/provider-registry.js') instead of relative paths (e.g.,'../plugin/provider-registry.js') for imports outside the current directory's module group. Relative imports are only used for sibling modules within the same directory.
Files:
packages/main/src/plugin/kdn-cli/kdn-cli.spec.tspackages/main/src/plugin/provider-registry.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/api/src/provider-info.tspackages/main/src/plugin/provider-registry.spec.tspackages/main/src/plugin/kdn-cli/kdn-cli.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.{ts,tsx}: Usetest()instead ofit()for test cases in Vitest unit tests
Usevi.mock(import('...'))for auto-mocking modules in Vitest. Avoid manual mock factories (vi.mock('...', () => ({...}))) when possible
Usevi.resetAllMocks()inbeforeEachfor resetting mocks in Vitest unit tests, notvi.clearAllMocks()
When an auto-mocked function or class method needs a real implementation in Vitest, usevi.mocked(...). For class methods, use the prototype pattern:vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)
Files:
packages/main/src/plugin/kdn-cli/kdn-cli.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/provider-registry.spec.ts
packages/main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/main/src/**/*.{ts,tsx}: Long-running operations should useTaskManagerwithcreateTask()to provide user feedback on operation status
Container operations in the main process must useContainerProviderRegistrywith theengineIdparameter to identify the container engine
Kubernetes operations in the main process should useKubernetesClientfor context management, resource operations, port forwarding, and exec operations
IPC handlers in the main process must follow the naming convention:<registry-name>:<action>(e.g.,container-provider-registry:listContainers)
Store credentials and sensitive setup data securely viaSafeStorageRegistryinstead of plain configuration
Files:
packages/main/src/plugin/kdn-cli/kdn-cli.spec.tspackages/main/src/plugin/provider-registry.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/provider-registry.spec.tspackages/main/src/plugin/kdn-cli/kdn-cli.ts
🧠 Learnings (1)
📚 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/kdn-cli/kdn-cli.spec.tspackages/main/src/plugin/provider-registry.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/provider-registry.spec.tspackages/main/src/plugin/kdn-cli/kdn-cli.ts
🔇 Additional comments (5)
packages/main/src/plugin/kdn-cli/kdn-cli.ts (1)
308-310: Good fix for CLI flag casing.Using
--headerTemplatehere correctly aligns with the expected kdn argument format and unblockstype=othersecret creation with header templates.packages/main/src/plugin/kdn-cli/kdn-cli.spec.ts (1)
822-831: Test expectation update looks correct.The assertion now checks
--headerTemplate, matching the runtime argument fix.packages/api/src/provider-info.ts (1)
201-209: Type addition is clear and fit-for-purpose.The interface cleanly captures exactly what downstream secret creation needs.
packages/main/src/plugin/provider-registry.spec.ts (1)
2658-2763: Coverage for credential resolution is solid.Nice set of match and non-match cases for model, provider metadata, and endpoint behavior.
packages/main/src/plugin/provider-registry.ts (1)
2073-2101: Resolver implementation is straightforward and consistent with tests.The composite model-id match plus metadata/endpoint return shape is implemented cleanly.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…ce name is absent Use basename(options.sourcePath) instead of the literal 'workspace' fallback to avoid secret-name collisions across unnamed workspaces. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Marcel Bertagnini <mbertagn@redhat.com>
|
e2e tests fail |
fbricon
left a comment
There was a problem hiding this comment.
works great! Now you can merge as soon as the tests pass
|
@coderabbitai resolve |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me check the CI status and failing e2e tests for this PR. 🏁 Script executed: Length of output: 2128
It appears the e2e failures that fbricon noted have been resolved in the latest push. The PR looks green and ready for review/merge consideration. |
✅ Actions performedComments resolved and changes approved. |
When creating a workspace with a cloud model selected, the provider connection's credentials are now used to automatically create a matching kdn vault secret so the CLI can inject API keys at runtime. This avoids requiring users to manually duplicate their API key in the secret vault after configuring a provider via Settings. Skips providers that don't use simple API-key auth (Vertex AI, Ollama) and defers to existing secrets when the onboarding flow has already configured them via workspaceConfiguration.
Also fixes a pre-existing bug in KdnCli.createSecret where the --headerTemplate flag was incorrectly passed as --header-template (kebab-case), causing all type=other secret creations with a header template to fail silently.
Test plan
Closes #1738
Closes #1726