Skip to content

fix(vertex-ai): expand tilde in credentials path before storing secret#2342

Merged
fbricon merged 1 commit into
openkaiden:mainfrom
fbricon:worktree-fix-vertex-ai-adc-path
Jul 2, 2026
Merged

fix(vertex-ai): expand tilde in credentials path before storing secret#2342
fbricon merged 1 commit into
openkaiden:mainfrom
fbricon:worktree-fix-vertex-ai-adc-path

Conversation

@fbricon

@fbricon fbricon commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

The credentials file path (e.g. ~/.config/gcloud/application_default_credentials.json)
was stored as-is in the secret. The openshell CLI does not expand ~, causing
workspace creation to fail with 'No such file or directory'.

Resolve the path via resolveCredentialsPath() before storing so the secret
always contains an absolute path.

fixes #2341

The credentials file path (e.g. ~/.config/gcloud/application_default_credentials.json)
was stored as-is in the secret. The openshell CLI does not expand ~, causing
workspace creation to fail with 'No such file or directory'.

Resolve the path via resolveCredentialsPath() before storing so the secret
always contains an absolute path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Fred Bricon <fbricon@gmail.com>
@fbricon
fbricon requested a review from a team as a code owner July 2, 2026 08:50
@fbricon
fbricon requested review from MarsKubeX and bmahabirbu and removed request for a team July 2, 2026 08:50
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Modified VertexAi.setConnectionConfiguration to resolve ~ in the credentials file path via resolveCredentialsPath before storing it in secret configuration, and added a test verifying secret storage receives the expanded absolute path.

Changes

Credentials Path Resolution

Layer / File(s) Summary
Resolve tilde in credentials path
extensions/vertex-ai/src/vertex-ai.ts, extensions/vertex-ai/src/vertex-ai.spec.ts
setConnectionConfiguration now stores the resolved credentials path (expanding ~) instead of the raw string; a new test asserts secret storage receives the expanded path.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • openkaiden/kaiden#1671: Builds on the same Vertex AI extension credentials/connection code and test suite.
  • openkaiden/kaiden#2074: Modifies the same connection/workspace configuration flow in vertex-ai.ts and its tests around storing credential secrets.
  • openkaiden/kaiden#2277: Modifies the same credentials file/secret configuration logic in vertex-ai.ts and its workspace-configuration tests.

Suggested reviewers: MarsKubeX, benoitf

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: expanding tilde paths before storing Vertex AI credentials.
Description check ✅ Passed The description matches the bug fix and explains the path expansion change clearly.
Linked Issues check ✅ Passed The code and test address the linked issue by resolving ~ in the credentials path before storing it.
Out of Scope Changes check ✅ Passed The changes are narrowly scoped to the Vertex AI credentials path fix and its test coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
extensions/vertex-ai/src/vertex-ai.ts (1)

367-373: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Return the resolved credentials path here too
credentials() is surfaced through the shared provider registry, so exposing config.credentialsFile here keeps the ~ form visible even though storage and validation use the expanded path. Use resolveCredentialsPath(...) for consistency.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@extensions/vertex-ai/src/vertex-ai.ts` around lines 367 - 373, The
credentials() method in VertexAI is returning the raw config.credentialsFile
value instead of the resolved path expected by the shared provider registry.
Update credentials() to use resolveCredentialsPath(...) for the credentialsFile
field so the exposed value matches the stored/validated path while keeping the
same projectId and region behavior.
🤖 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 `@extensions/vertex-ai/src/vertex-ai.ts`:
- Around line 367-373: The credentials() method in VertexAI is returning the raw
config.credentialsFile value instead of the resolved path expected by the shared
provider registry. Update credentials() to use resolveCredentialsPath(...) for
the credentialsFile field so the exposed value matches the stored/validated path
while keeping the same projectId and region behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b43c6cf2-88ab-43d5-97d3-84b027c162d2

📥 Commits

Reviewing files that changed from the base of the PR and between 99d98f6 and a0d0cc2.

📒 Files selected for processing (2)
  • extensions/vertex-ai/src/vertex-ai.spec.ts
  • extensions/vertex-ai/src/vertex-ai.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
  • GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
  • GitHub Check: unit tests / macos-15
  • GitHub Check: unit tests / ubuntu-24.04
  • GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
  • GitHub Check: unit tests / windows-2022
  • GitHub Check: linter, formatters
  • GitHub Check: typecheck
  • GitHub Check: macOS
  • GitHub Check: Linux
  • GitHub Check: Windows
⚠️ CI failures not shown inline (4)

GitHub Actions: fullsend / dispatch _ Route: fix(vertex-ai): expand tilde in credentials path before storing secret

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1mif [[ -f .fullsend/config.yaml ]]; then�[0m
 �[36;1m  KILL_SWITCH=$(yq '.kill_switch // false' .fullsend/config.yaml)�[0m
 �[36;1m  if [[ "$KILL_SWITCH" == "true" ]]; then�[0m
 �[36;1m    echo "::error::Kill switch is active — all agent dispatch halted"�[0m

GitHub Actions: fullsend / dispatch _ Route: fix(vertex-ai): expand tilde in credentials path before storing secret

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m�[0m
 �[36;1mif [[ ! "$STAGE" =~ ^[a-z][a-z0-9_-]*$ ]]; then�[0m
 �[36;1m  echo "::error::Invalid stage name: must start with lowercase letter and contain only [a-z0-9_-]"�[0m

GitHub Actions: fullsend / dispatch _ Route: fix(vertex-ai): expand tilde in credentials path before storing secret

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1mEVENT_PAYLOAD=$(jq -c '{�[0m
 �[36;1m  issue: (.issue // null | if . then {number, html_url} else null end),�[0m
 �[36;1m  pull_request: (.pull_request // null | if . then {number, html_url,�[0m
 �[36;1m    head: {ref: .head.ref, sha: .head.sha, repo: {full_name: .head.repo.full_name}},�[0m
 �[36;1m    base: {ref: .base.ref, repo: {full_name: .base.repo.full_name}}} else null end),�[0m
 �[36;1m  comment: (.comment // null | if . then {body: .body[:4096]} else null end)�[0m
 �[36;1m}' "$GITHUB_EVENT_PATH") || {�[0m
 �[36;1m  echo "::error::Failed to extract event payload from GITHUB_EVENT_PATH"�[0m

GitHub Actions: fullsend / 6_dispatch _ Route.txt: fix(vertex-ai): expand tilde in credentials path before storing secret

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m�[0m
 �[36;1mif [[ ! "$STAGE" =~ ^[a-z][a-z0-9_-]*$ ]]; then�[0m
 �[36;1m  echo "::error::Invalid stage name: must start with lowercase letter and contain only [a-z0-9_-]"�[0m
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/vertex-ai.ts
  • extensions/vertex-ai/src/vertex-ai.spec.ts
extensions/*/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Register inference, container, and Kubernetes providers through the ProviderRegistry via extension APIs

Files:

  • extensions/vertex-ai/src/vertex-ai.ts
  • extensions/vertex-ai/src/vertex-ai.spec.ts
**/*.spec.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.spec.{ts,tsx,js,jsx}: Use test() instead of it() for test cases in Vitest unit tests
Use vi.mock(import('...')) for auto-mocking modules in unit tests; avoid manual mock factories when possible
Use vi.resetAllMocks() in beforeEach hooks instead of vi.clearAllMocks() for resetting mocks between tests
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(...)

Files:

  • extensions/vertex-ai/src/vertex-ai.spec.ts
🧠 Learnings (5)
📚 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/vertex-ai.ts
  • extensions/vertex-ai/src/vertex-ai.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/vertex-ai/src/vertex-ai.ts
  • extensions/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/vertex-ai.ts
  • extensions/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/vertex-ai.ts
  • extensions/vertex-ai/src/vertex-ai.spec.ts
📚 Learning: 2026-06-29T13:16:53.102Z
Learnt from: benoitf
Repo: openkaiden/kaiden PR: 2296
File: extensions/container/packages/extension/src/helper/socket-finder/_socket-finder-module.ts:28-29
Timestamp: 2026-06-29T13:16:53.102Z
Learning: When reviewing imports in openkaiden/kaiden TypeScript/JavaScript files, prefer the configured `/@/` path alias instead of relative imports that would require traversing out of the current directory/module group (i.e., paths containing `..` that cross boundaries). 

Do not require alias conversion for descendant-path relative imports within the socket-finder module directory—for example, in `extensions/container/packages/extension/src/helper/socket-finder/**`, imports like `./podman/podman-version-detector` and `./podman/podman-windows-finder` are acceptable and should not be flagged.

Applied to files:

  • extensions/vertex-ai/src/vertex-ai.ts
  • extensions/vertex-ai/src/vertex-ai.spec.ts
🔇 Additional comments (2)
extensions/vertex-ai/src/vertex-ai.ts (1)

280-293: LGTM!

extensions/vertex-ai/src/vertex-ai.spec.ts (1)

669-687: LGTM!

@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fbricon
fbricon merged commit 5df76b0 into openkaiden:main Jul 2, 2026
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

~ in vertex ai credentials file path causes silent failure

2 participants