feat(vertex-ai): persist connection IDs in workspace configuration#2074
Merged
Conversation
bmahabirbu
requested review from
gastoner and
jeffmaury
and removed request for
a team
Contributor
|
PR changed again? Review this PR in Change Stack to compare snapshots and stay oriented. Warning Review limit reached
More reviews will be available in 21 minutes and 47 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR extends the Vertex AI provider extension with workspace configuration persistence for provider connections. It adds schema fields for connection metadata, wires the Configuration API into the extension, implements per-connection token secret storage and workspace-configuration updates, and updates tests to cover registration, restoration, and deletion flows. ChangesConfiguration Persistence for Vertex AI Connections
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
bmahabirbu
force-pushed
the
worktree-vertexapi
branch
from
d388b9c to
2a9f1ee
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/vertex-ai/src/vertex-ai.ts`:
- Around line 366-370: The provider registration can remain active if await
this.setConnectionConfiguration(connection, config) throws, leaving
connectionDisposable registered in this.connections and with the provider; wrap
the call in a try/catch and on error dispose/unregister the created registration
(call connectionDisposable.dispose() or the appropriate deregistration method)
and remove the entry from this.connections before rethrowing the error so
runtime state is rolled back consistently (references:
registerInferenceProviderConnection, connectionDisposable, this.connections,
setConnectionConfiguration, factory()).
🪄 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: acee0bcb-5aa3-4a48-9b36-4ecddabc2285
📒 Files selected for processing (4)
extensions/vertex-ai/package.jsonextensions/vertex-ai/src/extension.tsextensions/vertex-ai/src/vertex-ai.spec.tsextensions/vertex-ai/src/vertex-ai.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). (5)
- GitHub Check: unit tests / windows-2025
- GitHub Check: linter, formatters
- GitHub Check: macOS
- GitHub Check: Windows
- GitHub Check: Linux
🧰 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/vertex-ai/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/vertex-ai/src/extension.tsextensions/vertex-ai/src/vertex-ai.tsextensions/vertex-ai/src/vertex-ai.spec.ts
extensions/*/src/extension.ts
📄 CodeRabbit inference engine (AGENTS.md)
Extensions should export a standard activation API from their entry point
Files:
extensions/vertex-ai/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/vertex-ai/src/extension.tsextensions/vertex-ai/src/vertex-ai.tsextensions/vertex-ai/src/vertex-ai.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/vertex-ai/src/vertex-ai.spec.ts
🧠 Learnings (25)
📓 Common learnings
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: 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: 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: 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: 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: bmahabirbu
Repo: openkaiden/kaiden PR: 1517
File: packages/main/src/plugin/kdn-cli/kdn-cli.ts:167-177
Timestamp: 2026-05-03T09:13:45.692Z
Learning: In the openkaiden/kaiden repository, URL-based (remote) MCPs that require API tokens use the secret manager (`SafeStorageRegistry`) for credential storage. Command/package-spawned MCPs (e.g., uvx, npx) generally do not require API keys, so their env vars (e.g., UV_SYSTEM_CERTS) are safely persisted in plaintext in workspace.json. Flagging plaintext env persistence in `writeWorkspaceConfig` (packages/main/src/plugin/kdn-cli/kdn-cli.ts) as a security issue for command-based MCPs is not currently applicable, but may be worth a follow-up if command-based MCPs ever need secrets.
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1517
File: packages/main/src/plugin/kdn-cli/kdn-cli.ts:181-189
Timestamp: 2026-05-05T03:38:00.098Z
Learning: In the openkaiden/kaiden repository, the explicit design decision for `writeWorkspaceConfig` (packages/main/src/plugin/kdn-cli/kdn-cli.ts) is: only non-secret runtime config flags (e.g. UV_SYSTEM_CERTS) are written as plaintext env vars to workspace.json for command/package-spawned MCPs. When a spawned MCP requires a secret env var, the planned approach is to route it through `kdn secret create` and write a secret reference (not the raw value) into workspace.json. This will be implemented in a separate follow-up PR.
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1396
File: packages/renderer/src/lib/guided-setup/CodingAgentStep.svelte:54-65
Timestamp: 2026-04-20T15:05:12.921Z
Learning: In `packages/renderer/src/lib/guided-setup/CodingAgentStep.svelte`, the `$effect` block unconditionally writes all credential/vertex fields (`anthropicApiKey`, `vertexProjectId`, `vertexRegion`, `vertexMountGcloud`, `vertexMountClaudeConfig`) into the `onboarding` object regardless of `selectedVariant`. This is intentional: (1) only `onboarding.agent` is persisted today (to `settings.json` via `window.updateConfigurationValue`); all credential fields are ephemeral in-memory and discarded when the wizard closes, so leaving them populated is harmless. (2) Preserving inputs across variant switches is a deliberate UX decision to avoid data loss on accidental clicks. Variant-scoped filtering will be added in the follow-up agents.json IPC work. Do not flag the unconditional credential mirroring as a bug or suggest clearing fields on variant switch until that IPC work lands.
📚 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/vertex-ai/package.json
📚 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:
extensions/vertex-ai/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/vertex-ai/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/vertex-ai/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/vertex-ai/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/vertex-ai/src/extension.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:
extensions/vertex-ai/src/extension.tsextensions/vertex-ai/src/vertex-ai.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/vertex-ai/src/extension.tsextensions/vertex-ai/src/vertex-ai.tsextensions/vertex-ai/src/vertex-ai.spec.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/vertex-ai/src/extension.tsextensions/vertex-ai/src/vertex-ai.tsextensions/vertex-ai/src/vertex-ai.spec.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/vertex-ai/src/extension.tsextensions/vertex-ai/src/vertex-ai.tsextensions/vertex-ai/src/vertex-ai.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/vertex-ai/src/extension.tsextensions/vertex-ai/src/vertex-ai.tsextensions/vertex-ai/src/vertex-ai.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/vertex-ai/src/extension.tsextensions/vertex-ai/src/vertex-ai.tsextensions/vertex-ai/src/vertex-ai.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:
extensions/vertex-ai/src/vertex-ai.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/vertex-ai/src/vertex-ai.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/vertex-ai/src/vertex-ai.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/vertex-ai/src/vertex-ai.ts
📚 Learning: 2026-05-06T11:17:00.135Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1671
File: extensions/vertex-ai/src/extension.spec.ts:43-51
Timestamp: 2026-05-06T11:17:00.135Z
Learning: In the openkaiden/kaiden repo, for extensions under `extensions/*/src/extension.ts`, `deactivate()` only clears the module-level instance reference (e.g., sets `vertexAi = undefined`). The `dispose()` method on the instance is NOT called directly by `deactivate()` — it is invoked by the extension host when it processes `extensionContext.subscriptions`. Do not suggest asserting `dispose()` in tests for the `deactivate()` function.
Applied to files:
extensions/vertex-ai/src/vertex-ai.tsextensions/vertex-ai/src/vertex-ai.spec.ts
📚 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:
extensions/vertex-ai/src/vertex-ai.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:
extensions/vertex-ai/src/vertex-ai.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/vertex-ai/src/vertex-ai.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: 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/vertex-ai/src/vertex-ai.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/vertex-ai/src/vertex-ai.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/vertex-ai/src/vertex-ai.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/vertex-ai/src/vertex-ai.spec.ts
🔇 Additional comments (4)
extensions/vertex-ai/package.json (1)
38-50: LGTM!extensions/vertex-ai/src/extension.ts (1)
20-20: LGTM!Also applies to: 29-30
extensions/vertex-ai/src/vertex-ai.ts (1)
275-295: LGTM!extensions/vertex-ai/src/vertex-ai.spec.ts (1)
25-33: LGTM!Also applies to: 70-82, 123-139, 615-677
bmahabirbu
force-pushed
the
worktree-vertexapi
branch
from
2a9f1ee to
08ffd90
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
bmahabirbu
force-pushed
the
worktree-vertexapi
branch
3 times, most recently
from
43e862e to
fbdd1fb
Compare
Contributor
Author
|
@CodeRabbit resolve |
Contributor
✅ Action performedComments resolved and changes approved. |
bmahabirbu
force-pushed
the
worktree-vertexapi
branch
from
fbdd1fb to
586ea91
Compare
Contributor
Author
|
@CodeRabbit resolve |
Contributor
✅ Action performedComments resolved and changes approved. |
jeffmaury
requested changes
Jun 5, 2026
| const secretName = this.getSecretName(connection.id); | ||
| await this.secrets.store(secretName, config.credentialsFile); | ||
|
|
||
| const cfg = this.configurationAPI.getConfiguration('vertex-ai.connection', connection); |
Contributor
There was a problem hiding this comment.
issue: replace section by undefined
|
|
||
| const cfg = this.configurationAPI.getConfiguration('vertex-ai.connection', connection); | ||
| await cfg.update('_type', PROVIDER_ID); | ||
| await cfg.update('token', secretName); |
Contributor
There was a problem hiding this comment.
issue: should be the full property name (as in package.json)
bmahabirbu
force-pushed
the
worktree-vertexapi
branch
from
586ea91 to
b93d469
Compare
Align with OpenShell's Vertex AI provider pattern by storing per- connection configuration using full property names and the standard VERTEX_AI_PROJECT_ID / VERTEX_AI_REGION keys. - Add configurationAPI to store scoped connection properties - Store credentials file path in per-connection secret storage - Use getConfiguration(undefined, connection) with fully-qualified property names (vertex-ai.connection.*) - Add VERTEX_AI_PROJECT_ID and VERTEX_AI_REGION connection properties Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Brian <bmahabir@bu.edu>
bmahabirbu
force-pushed
the
worktree-vertexapi
branch
from
b93d469 to
0f84b33
Compare
Contributor
Author
|
relates to NVIDIA/OpenShell#1763 as if 1763 gets in then we can keep this change as it and it will work. We will need to automate |
jeffmaury
approved these changes
Jun 9, 2026
jeffmaury
left a comment
jeffmaury
left a comment
Contributor
There was a problem hiding this comment.
LGTM.
Just passing configuration namespace is the constructor is not required as it is a global namespace but no blocker
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.
Summary
InferenceProviderConnectionscope in the extension configuration to support workspace-level connection metadataFixes: #1845
🤖 Generated with Claude Code