feat(openai): persist connection IDs in workspace configuration - #2049
Merged
Merged
Conversation
Add per-connection secret storage and scoped configuration properties for OpenAI-compatible inference provider connections. Introduce InferenceProviderConnection scope in the configuration system to support workspace-level connection metadata. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Evzen Gasta <evzen.ml@seznam.cz>
gastoner
requested review from
benoitf and
fbricon
and removed request for
a team
June 3, 2026 06:14
Contributor
📝 WalkthroughWalkthroughThe PR extends the workspace configuration API to support per-connection token and metadata storage for OpenAI inference provider connections. It adds a new ChangesInference provider connection configuration
🎯 3 (Moderate) | ⏱️ ~20 minutes 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}
Contributor
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/configuration-impl.ts (1)
185-193: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winDrop the redundant
elsebranches to satisfy BiomenoUselessElse.Static analysis flags this chain as an error (
lint/style/noUselessElse) since every branch returns. Converting to sequential guard clauses clears the lint.♻️ Proposed refactor
getConfigurationKey(): string { if (this.isContainerProviderConnection(this.scope)) { return `container-connection:${this.scope.name}.${this.scope.endpoint.socketPath}`; - } else if (this.isKubernetesProviderConnection(this.scope)) { + } + if (this.isKubernetesProviderConnection(this.scope)) { return `kubernetes-connection:${this.scope.endpoint.apiURL}`; - } else if (this.isInferenceProviderConnection(this.scope)) { + } + if (this.isInferenceProviderConnection(this.scope)) { return `inference-connection:${this.scope.id}`; - } else if (this.scope === CONFIGURATION_SYSTEM_MANAGED_DEFAULTS_SCOPE) { + } + if (this.scope === CONFIGURATION_SYSTEM_MANAGED_DEFAULTS_SCOPE) { return CONFIGURATION_SYSTEM_MANAGED_DEFAULTS_SCOPE; - } else if (this.scope === CONFIGURATION_SYSTEM_MANAGED_LOCKED_SCOPE) { + } + if (this.scope === CONFIGURATION_SYSTEM_MANAGED_LOCKED_SCOPE) { return CONFIGURATION_SYSTEM_MANAGED_LOCKED_SCOPE; - } else { - return CONFIGURATION_DEFAULT_SCOPE; } + return CONFIGURATION_DEFAULT_SCOPE; }🤖 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/configuration-impl.ts` around lines 185 - 193, The chain of conditional returns should drop redundant else blocks: replace the "else if (this.isInferenceProviderConnection(this.scope))", "else if (this.scope === CONFIGURATION_SYSTEM_MANAGED_DEFAULTS_SCOPE)", and "else if (this.scope === CONFIGURATION_SYSTEM_MANAGED_LOCKED_SCOPE)" branches with sequential guard clauses (plain if checks) and finish with a single return CONFIGURATION_DEFAULT_SCOPE; keep the same return values and use the existing symbols (this.isInferenceProviderConnection, this.scope, CONFIGURATION_SYSTEM_MANAGED_DEFAULTS_SCOPE, CONFIGURATION_SYSTEM_MANAGED_LOCKED_SCOPE, CONFIGURATION_DEFAULT_SCOPE) so every branch returns directly without trailing elses to satisfy the noUselessElse lint.
🤖 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/configuration-impl.ts`:
- Around line 185-193: The chain of conditional returns should drop redundant
else blocks: replace the "else if
(this.isInferenceProviderConnection(this.scope))", "else if (this.scope ===
CONFIGURATION_SYSTEM_MANAGED_DEFAULTS_SCOPE)", and "else if (this.scope ===
CONFIGURATION_SYSTEM_MANAGED_LOCKED_SCOPE)" branches with sequential guard
clauses (plain if checks) and finish with a single return
CONFIGURATION_DEFAULT_SCOPE; keep the same return values and use the existing
symbols (this.isInferenceProviderConnection, this.scope,
CONFIGURATION_SYSTEM_MANAGED_DEFAULTS_SCOPE,
CONFIGURATION_SYSTEM_MANAGED_LOCKED_SCOPE, CONFIGURATION_DEFAULT_SCOPE) so every
branch returns directly without trailing elses to satisfy the noUselessElse
lint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 64978498-4b0c-4ddc-9a78-a069c6f56cef
📒 Files selected for processing (7)
extensions/openai-compatible/package.jsonextensions/openai-compatible/src/extension.tsextensions/openai-compatible/src/openAI.spec.tsextensions/openai-compatible/src/openAI.tspackages/api/src/configuration/models.tspackages/main/src/plugin/configuration-impl.spec.tspackages/main/src/plugin/configuration-impl.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: linter, formatters
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: typecheck
- GitHub Check: Windows
- GitHub Check: unit tests / macos-15
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: Linux
- GitHub Check: macOS
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: unit tests / windows-2025
🧰 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/configuration-impl.tsextensions/openai-compatible/src/extension.tsextensions/openai-compatible/src/openAI.tspackages/api/src/configuration/models.tspackages/main/src/plugin/configuration-impl.spec.tsextensions/openai-compatible/src/openAI.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/configuration-impl.tspackages/main/src/plugin/configuration-impl.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/configuration-impl.tspackages/main/src/plugin/configuration-impl.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/openai-compatible/package.json
extensions/*/src/extension.ts
📄 CodeRabbit inference engine (AGENTS.md)
Extensions should export a standard activation API from their entry point
Files:
extensions/openai-compatible/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/openai-compatible/src/extension.tsextensions/openai-compatible/src/openAI.tsextensions/openai-compatible/src/openAI.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/configuration-impl.spec.tsextensions/openai-compatible/src/openAI.spec.ts
🧠 Learnings (31)
📓 Common learnings
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 2001
File: packages/main/src/plugin/provider-registry.ts:1966-1971
Timestamp: 2026-05-29T13:43:13.292Z
Learning: In `openkaiden/kaiden`, `getInferenceSDK`, `getInferenceConnectionType`, and `getInferenceConnectionEndpoint` in `packages/main/src/plugin/provider-registry.ts` intentionally resolve inference connections by `name` (not `id`), because downstream consumers (InferenceParameters, chat history DB) persist `connectionName` rather than `connectionId`. Migrating these helpers to id-based lookup is deferred to the storage format redesign epic (`#1917`). Do not flag these name-based lookups as issues in reviews until that epic is addressed.
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 2025
File: extensions/vertex-ai/src/vertex-ai.ts:166-170
Timestamp: 2026-06-01T15:06:34.983Z
Learning: In openkaiden/kaiden, the Vertex AI extension's `removeConnection` (previously `removeConnectionConfig`) intentionally removes stored entries by config hash rather than by persisted `id`. This is safe because `factory()` (line ~423) has an in-memory duplicate guard (`this.connections.has(this.getConfigHash(config))`) that rejects same-config calls before any `saveConnection` write occurs, making the hash-collision/race scenario impossible. Switching to ID-based removal is considered a future design improvement, not a correctness fix, and was explicitly scoped out of PR `#2025` (issue `#1942`).
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/src/vertex-ai.ts:272-302
Timestamp: 2026-05-05T17:30:24.999Z
Learning: In the `openkaiden/kaiden` repository, cloud provider extensions (Gemini, Claude, Mistral, OpenAI-compatible, Vertex AI) use `ProviderConnectionStatus = 'unknown'` when registering inference provider connections. This means "connection was set up but is not continuously monitored." Only Ollama uses `'started'` because it actively polls a local server. Do not flag `'unknown'` status as incorrect for cloud provider extension connections in `extensions/*/src/*.ts`.
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1603
File: packages/renderer/src/lib/guided-setup/guided-setup-steps.ts:35-45
Timestamp: 2026-05-04T17:36:14.229Z
Learning: In openkaiden/kaiden, `OnboardingModelSelection` (packages/renderer/src/lib/guided-setup/guided-setup-steps.ts) intentionally omits `connectionName` because the Claude extension sets `connectionName` to the raw API key value. Persisting `connectionName` would write the raw API key to settings.json, which is a security risk. The `providerId + label` pair is sufficient for the CLI `--model` flag and for workspace creation; `connectionName` must not be re-added to this interface unless a safe (non-secret) identifier can be substituted.
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 2018
File: extensions/claude/src/manager/claude-inference-manager.ts:91-94
Timestamp: 2026-06-01T13:20:13.832Z
Learning: In the Claude extension (`extensions/claude/src/manager/claude-inference-manager.ts`), `removeConnection` intentionally filters stored `StoredConnection[]` records by token rather than by connection ID. This is safe because the `connections.has(tokenHash)` guard in `registerInferenceProviderConnection` throws if a duplicate token is already registered, making it impossible for two `StoredConnection` records with the same token to coexist in secret storage. Token-based removal is therefore equivalent to ID-based removal in practice. Do not flag this as a bug in future reviews.
Learnt from: CR
Repo: openkaiden/kaiden PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-12T16:35:51.592Z
Learning: Applies to extensions/*/src/**/*.{ts,tsx} : Register inference, container, and Kubernetes providers through the `ProviderRegistry` via extension APIs
Learnt from: jeffmaury
Repo: openkaiden/kaiden PR: 1524
File: extensions/openshift-ai/src/openshiftai.ts:233-235
Timestamp: 2026-04-30T12:38:39.371Z
Learning: In `extensions/openshift-ai/src/openshiftai.ts` (openkaiden/kaiden), `getInferenceServices()` intentionally swallows all API/auth/network exceptions and returns `[]`. This is by design: a cluster may restrict visibility of certain resources via RBAC, so an empty result is a valid unified signal for both "no inference services exist" and "no inference services are visible to this user." Do not flag this error-swallowing as hiding failures — the caller (`registerInferenceProviderConnection`) then throws a meaningful error when `connectionInfos.length === 0`.
📚 Learning: 2026-04-17T20:26:32.946Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/lib/models/models-utils.ts:11-20
Timestamp: 2026-04-17T20:26:32.946Z
Learning: In `packages/renderer/src/lib/models/models-utils.ts` (openkaiden/kaiden), `InferenceConnectionSummary.connectionType` is intentionally optional. It is only `undefined` for the single synthetic `'not-configured'` entry (emitted when a provider has `inferenceProviderConnectionCreation` but no active `inferenceConnections`). All consumers guard with optional chaining. A discriminated union was considered but deferred as unnecessary complexity for v1, since the invariant (`connectionType` is defined iff `status !== 'not-configured'`) is self-evident from the `status` field. Do not flag this as a type-safety issue.
Applied to files:
packages/main/src/plugin/configuration-impl.tsextensions/openai-compatible/package.jsonextensions/openai-compatible/src/openAI.tspackages/api/src/configuration/models.tspackages/main/src/plugin/configuration-impl.spec.ts
📚 Learning: 2026-05-29T13:43:13.292Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 2001
File: packages/main/src/plugin/provider-registry.ts:1966-1971
Timestamp: 2026-05-29T13:43:13.292Z
Learning: In `openkaiden/kaiden`, `getInferenceSDK`, `getInferenceConnectionType`, and `getInferenceConnectionEndpoint` in `packages/main/src/plugin/provider-registry.ts` intentionally resolve inference connections by `name` (not `id`), because downstream consumers (InferenceParameters, chat history DB) persist `connectionName` rather than `connectionId`. Migrating these helpers to id-based lookup is deferred to the storage format redesign epic (`#1917`). Do not flag these name-based lookups as issues in reviews until that epic is addressed.
Applied to files:
packages/main/src/plugin/configuration-impl.tsextensions/openai-compatible/src/openAI.tspackages/main/src/plugin/configuration-impl.spec.ts
📚 Learning: 2026-04-13T15:59:29.742Z
Learnt from: jeffmaury
Repo: openkaiden/kaiden PR: 1295
File: packages/extension-api/src/extension-api.d.ts:651-651
Timestamp: 2026-04-13T15:59:29.742Z
Learning: In the openkaiden/kaiden repository, the project relies on TypeScript's static type checking (not runtime validation) to enforce type correctness for `InferenceProviderConnection.type` (`InferenceProviderConnectionType`). Runtime normalization/validation guards for this field are not needed or desired.
Applied to files:
packages/main/src/plugin/configuration-impl.tsextensions/openai-compatible/src/openAI.ts
📚 Learning: 2026-05-12T16:35:51.592Z
Learnt from: CR
Repo: openkaiden/kaiden PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-12T16:35:51.592Z
Learning: Applies to extensions/*/src/**/*.{ts,tsx} : Register inference, container, and Kubernetes providers through the `ProviderRegistry` via extension APIs
Applied to files:
packages/main/src/plugin/configuration-impl.tsextensions/openai-compatible/src/openAI.tspackages/main/src/plugin/configuration-impl.spec.ts
📚 Learning: 2026-05-05T17:30:24.999Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/src/vertex-ai.ts:272-302
Timestamp: 2026-05-05T17:30:24.999Z
Learning: In the `openkaiden/kaiden` repository, cloud provider extensions (Gemini, Claude, Mistral, OpenAI-compatible, Vertex AI) use `ProviderConnectionStatus = 'unknown'` when registering inference provider connections. This means "connection was set up but is not continuously monitored." Only Ollama uses `'started'` because it actively polls a local server. Do not flag `'unknown'` status as incorrect for cloud provider extension connections in `extensions/*/src/*.ts`.
Applied to files:
packages/main/src/plugin/configuration-impl.tsextensions/openai-compatible/package.jsonpackages/api/src/configuration/models.ts
📚 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/configuration-impl.tspackages/main/src/plugin/configuration-impl.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/configuration-impl.tsextensions/openai-compatible/src/extension.tsextensions/openai-compatible/src/openAI.tspackages/api/src/configuration/models.tspackages/main/src/plugin/configuration-impl.spec.tsextensions/openai-compatible/src/openAI.spec.ts
📚 Learning: 2026-05-12T16:35:51.592Z
Learnt from: CR
Repo: openkaiden/kaiden PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-12T16:35:51.592Z
Learning: Applies to extensions/*/package.json : Configuration properties for API keys, tokens, or secrets must use `"format": "password"` in the configuration definition to ensure input masking in the UI
Applied to files:
extensions/openai-compatible/package.json
📚 Learning: 2026-04-12T15:29:39.027Z
Learnt from: jeffmaury
Repo: openkaiden/kaiden PR: 1314
File: packages/extension-api/package.json:3-3
Timestamp: 2026-04-12T15:29:39.027Z
Learning: In `packages/extension-api/package.json` (openkaiden/kaiden), the `scripts.publish:next` field intentionally uses a hardcoded older prerelease base (e.g., `0.0.1-"$(date +%s)"`) even after the package `version` field is bumped (e.g., to `0.1.0-next`). This is by design: the project keeps older minor lines (like `0.0.x`) alive for backported fixes and security releases. Do not flag this as inconsistent with the current `version` field in future reviews.
Applied to files:
extensions/openai-compatible/package.json
📚 Learning: 2026-04-12T15:29:06.175Z
Learnt from: jeffmaury
Repo: openkaiden/kaiden PR: 1314
File: extensions/container/packages/api/package.json:3-3
Timestamp: 2026-04-12T15:29:06.175Z
Learning: In `extensions/container/packages/api/package.json` (openkaiden/kaiden), the `scripts.publish:next` field intentionally uses a hardcoded older prerelease line (e.g., `0.0.1-"$(date +%s)"`) even after the package `version` field is bumped (e.g., to `0.1.0-next`). This is by design: the project keeps minor versions (like `0.0.1`) alive for backported fixes and security releases. Do not flag this as inconsistent with the current `version` field in future reviews.
Applied to files:
extensions/openai-compatible/package.json
📚 Learning: 2026-05-12T16:35:51.592Z
Learnt from: CR
Repo: openkaiden/kaiden PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-12T16:35:51.592Z
Learning: Applies to extensions/*/package.json : Extensions must declare `engines.kaiden` version compatibility in their `package.json`
Applied to files:
extensions/openai-compatible/package.json
📚 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/openai-compatible/package.json
📚 Learning: 2026-05-12T16:35:51.592Z
Learnt from: CR
Repo: openkaiden/kaiden PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-12T16:35:51.592Z
Learning: Applies to extensions/*/src/extension.ts : Extensions should export a standard activation API from their entry point
Applied to files:
extensions/openai-compatible/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/openai-compatible/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/openai-compatible/src/extension.tsextensions/openai-compatible/src/openAI.tsextensions/openai-compatible/src/openAI.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/openai-compatible/src/extension.tsextensions/openai-compatible/src/openAI.tsextensions/openai-compatible/src/openAI.spec.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/openai-compatible/src/extension.tsextensions/openai-compatible/src/openAI.tsextensions/openai-compatible/src/openAI.spec.ts
📚 Learning: 2026-06-01T13:20:13.832Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 2018
File: extensions/claude/src/manager/claude-inference-manager.ts:91-94
Timestamp: 2026-06-01T13:20:13.832Z
Learning: In the Claude extension (`extensions/claude/src/manager/claude-inference-manager.ts`), `removeConnection` intentionally filters stored `StoredConnection[]` records by token rather than by connection ID. This is safe because the `connections.has(tokenHash)` guard in `registerInferenceProviderConnection` throws if a duplicate token is already registered, making it impossible for two `StoredConnection` records with the same token to coexist in secret storage. Token-based removal is therefore equivalent to ID-based removal in practice. Do not flag this as a bug in future reviews.
Applied to files:
extensions/openai-compatible/src/openAI.ts
📚 Learning: 2026-06-01T15:06:34.983Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 2025
File: extensions/vertex-ai/src/vertex-ai.ts:166-170
Timestamp: 2026-06-01T15:06:34.983Z
Learning: In openkaiden/kaiden, the Vertex AI extension's `removeConnection` (previously `removeConnectionConfig`) intentionally removes stored entries by config hash rather than by persisted `id`. This is safe because `factory()` (line ~423) has an in-memory duplicate guard (`this.connections.has(this.getConfigHash(config))`) that rejects same-config calls before any `saveConnection` write occurs, making the hash-collision/race scenario impossible. Switching to ID-based removal is considered a future design improvement, not a correctness fix, and was explicitly scoped out of PR `#2025` (issue `#1942`).
Applied to files:
extensions/openai-compatible/src/openAI.ts
📚 Learning: 2026-04-30T12:38:39.371Z
Learnt from: jeffmaury
Repo: openkaiden/kaiden PR: 1524
File: extensions/openshift-ai/src/openshiftai.ts:233-235
Timestamp: 2026-04-30T12:38:39.371Z
Learning: In `extensions/openshift-ai/src/openshiftai.ts` (openkaiden/kaiden), `getInferenceServices()` intentionally swallows all API/auth/network exceptions and returns `[]`. This is by design: a cluster may restrict visibility of certain resources via RBAC, so an empty result is a valid unified signal for both "no inference services exist" and "no inference services are visible to this user." Do not flag this error-swallowing as hiding failures — the caller (`registerInferenceProviderConnection`) then throws a meaningful error when `connectionInfos.length === 0`.
Applied to files:
extensions/openai-compatible/src/openAI.tspackages/main/src/plugin/configuration-impl.spec.tsextensions/openai-compatible/src/openAI.spec.ts
📚 Learning: 2026-05-04T17:36:14.229Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1603
File: packages/renderer/src/lib/guided-setup/guided-setup-steps.ts:35-45
Timestamp: 2026-05-04T17:36:14.229Z
Learning: In openkaiden/kaiden, `OnboardingModelSelection` (packages/renderer/src/lib/guided-setup/guided-setup-steps.ts) intentionally omits `connectionName` because the Claude extension sets `connectionName` to the raw API key value. Persisting `connectionName` would write the raw API key to settings.json, which is a security risk. The `providerId + label` pair is sufficient for the CLI `--model` flag and for workspace creation; `connectionName` must not be re-added to this interface unless a safe (non-secret) identifier can be substituted.
Applied to files:
extensions/openai-compatible/src/openAI.ts
📚 Learning: 2026-05-12T16:35:51.592Z
Learnt from: CR
Repo: openkaiden/kaiden PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-12T16:35:51.592Z
Learning: Unit tests are co-located with source files using *.spec.ts naming convention; E2E tests are located in tests/playwright/src/; test configuration is in vitest.config.js at root
Applied to files:
extensions/openai-compatible/src/openAI.spec.ts
📚 Learning: 2026-04-20T14:31:10.155Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1396
File: packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.ts:114-123
Timestamp: 2026-04-20T14:31:10.155Z
Learning: In `packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.ts`, the team's convention is that a test's name plus its stub setup lines serve as sufficient documentation of intent; inline comments restating what the stubs do are considered redundant and are intentionally omitted. Do not flag the absence of such comments as a clarity issue.
Applied to files:
extensions/openai-compatible/src/openAI.spec.ts
📚 Learning: 2026-04-30T12:45:43.072Z
Learnt from: fbricon
Repo: openkaiden/kaiden PR: 1509
File: packages/api/src/agent-workspace-info.ts:53-53
Timestamp: 2026-04-30T12:45:43.072Z
Learning: In `packages/api/src/agent-workspace-info.ts`, the `model` field on `AgentWorkspaceCreateOptions` is intentionally typed as `model?: string` (not a narrowed template-literal type). The current CLI-side filtering (only forwarding `ollama::` / `ramalama::` prefixes) in `kdn-cli.ts` is a temporary measure until broader agent support lands (tracked in openkaiden/kdn#354). Do not suggest narrowing this type to a union or template-literal type in future reviews.
Applied to files:
extensions/openai-compatible/src/openAI.spec.ts
📚 Learning: 2026-04-30T12:44:46.782Z
Learnt from: fbricon
Repo: openkaiden/kaiden PR: 1509
File: packages/main/src/plugin/kdn-cli/kdn-cli.ts:103-112
Timestamp: 2026-04-30T12:44:46.782Z
Learning: In `packages/main/src/plugin/kdn-cli/kdn-cli.ts` (`createWorkspace`), silently skipping `--model` with a `console.warn` when `options.model` does not start with `ollama::` or `ramalama::` is intentional and temporary. This is because only OpenCode currently supports that scheme for local runtimes; broader support is tracked in openkaiden/kdn#354. Do not flag this as a silent-failure bug or suggest throwing an error in future reviews.
Applied to files:
extensions/openai-compatible/src/openAI.spec.ts
📚 Learning: 2026-04-23T04:28:25.544Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1417
File: extensions/kdn/vite.config.js:60-62
Timestamp: 2026-04-23T04:28:25.544Z
Learning: In openkaiden/kaiden, the `__mocks__/` directory (containing `vitest-generate-api-global-setup.ts` and `openkaiden/api.js`) lives at the repository root, NOT under `extensions/`. Extensions (e.g., `extensions/kdn`, `extensions/gemini`) use `join(PACKAGE_ROOT, '..', '..', '__mocks__', ...)` in their `vite.config.js` to correctly resolve to the repo-root `__mocks__/` folder. Do not flag this two-level-up path traversal as incorrect in future reviews.
Applied to files:
extensions/openai-compatible/src/openAI.spec.ts
📚 Learning: 2026-05-12T16:35:51.592Z
Learnt from: CR
Repo: openkaiden/kaiden PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-12T16:35:51.592Z
Learning: Applies to **/*.spec.{ts,tsx,js,jsx} : Use `vi.mock(import('...'))` for auto-mocking modules in unit tests; avoid manual mock factories when possible
Applied to files:
extensions/openai-compatible/src/openAI.spec.ts
📚 Learning: 2026-05-12T16:35:51.592Z
Learnt from: CR
Repo: openkaiden/kaiden PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-12T16:35:51.592Z
Learning: Applies to **/*.spec.{ts,tsx,js,jsx} : When an auto-mocked function or class method needs a real implementation, use `vi.mocked(...)` with the prototype pattern for class methods: `vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)`
Applied to files:
extensions/openai-compatible/src/openAI.spec.ts
📚 Learning: 2026-05-12T16:35:51.592Z
Learnt from: CR
Repo: openkaiden/kaiden PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-12T16:35:51.592Z
Learning: Applies to **/*.spec.{ts,tsx,js,jsx} : Use `vi.resetAllMocks()` in `beforeEach` hooks instead of `vi.clearAllMocks()` for resetting mocks between tests
Applied to files:
extensions/openai-compatible/src/openAI.spec.ts
📚 Learning: 2026-04-20T14:30:15.867Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1396
File: packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.ts:30-45
Timestamp: 2026-04-20T14:30:15.867Z
Learning: In `packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.ts`, the `stubOllama` helper intentionally only re-stubs `fetch` and relies on `beforeEach`'s `stubRamalama(false)` call to keep `getProviderInfos` in place. This is a deliberate standard Vitest `beforeEach` + per-test override pattern. Do not flag the implicit dependency between `stubOllama` and the `beforeEach` ramalama stub as a robustness issue.
Applied to files:
extensions/openai-compatible/src/openAI.spec.ts
🪛 Biome (2.4.16)
packages/main/src/plugin/configuration-impl.ts
[error] 185-193: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
🔇 Additional comments (10)
packages/api/src/configuration/models.ts (1)
80-91: LGTM!packages/main/src/plugin/configuration-impl.ts (1)
157-162: Type guard is sound; no scope collision risk.
InferenceProviderConnection.endpointis an optionalstring, whileisContainerProviderConnection/isKubernetesProviderConnectionboth requireendpointto be an object, so the inference branch can't be shadowed by them ingetConfigurationKey().packages/main/src/plugin/configuration-impl.spec.ts (1)
229-295: LGTM!extensions/openai-compatible/package.json (1)
28-40: LGTM!extensions/openai-compatible/src/openAI.ts (3)
134-154: LGTM!
182-206: LGTM!
208-211: ⚡ Quick winWrite OpenAI-compatible connection configuration before registering the connection to avoid transient missing token state.
extensions/openai-compatible/src/openAI.tsregisters the inference connection (and stores it inthis.connections) beforesetConnectionConfiguration(connection, token)writes the per-connection secret/config keys. Sinceprovider-impl.tsinvokes the registration callback immediately as part ofregisterInferenceProviderConnection, any listener that readsopenai.connection/token config during that callback could observe missing data; additionally, ifsetConnectionConfigurationrejects, the connection is already registered and stored, leaving inconsistent state.♻️ Proposed reordering
- const connectionDisposable = this.provider.registerInferenceProviderConnection(connection); - this.connections.set(id, connectionDisposable); - - await this.setConnectionConfiguration(connection, token); + await this.setConnectionConfiguration(connection, token); + + const connectionDisposable = this.provider.registerInferenceProviderConnection(connection); + this.connections.set(id, connectionDisposable);Confirm whether any
onDidRegisterInferenceConnectionlisteners readopenai.connection(or token/_type) during registration; if so, the current ordering is racy.extensions/openai-compatible/src/extension.ts (1)
20-27: LGTM!extensions/openai-compatible/src/openAI.spec.ts (2)
73-97: LGTM!
243-308: LGTM!
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
benoitf
approved these changes
Jun 3, 2026
This was referenced Jun 3, 2026
This was referenced Jun 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add per-connection secret storage and scoped configuration properties for OpenAI-compatible inference provider connections. Introduce InferenceProviderConnection scope in the configuration system to support workspace-level connection metadata.
Closes #1843