Skip to content

fix: handle absent llmMetadata.name when creating secret - #1796

Merged
jeffmaury merged 1 commit into
openkaiden:mainfrom
jeffmaury:GH-1795
May 11, 2026
Merged

jeffmaury merged 1 commit into
openkaiden:mainfrom
jeffmaury:GH-1795

Conversation

@jeffmaury

Copy link
Copy Markdown
Contributor

Fixes #1795

Fixes openkaiden#1795

Signed-off-by: Jeff MAURY <jmaury@redhat.com>
@jeffmaury
jeffmaury requested a review from a team as a code owner May 10, 2026 20:29
@jeffmaury
jeffmaury requested review from bmahabirbu and fbricon and removed request for a team May 10, 2026 20:29
@coderabbitai

coderabbitai Bot commented May 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c1ae9656-dab0-48ba-be46-d3f2a9b9a383

📥 Commits

Reviewing files that changed from the base of the PR and between 0ad0ab0 and 7c719fd.

📒 Files selected for processing (2)
  • packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
  • packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
📜 Recent review details
🧰 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/agent-workspace/agent-workspace-manager.spec.ts
  • packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
**/*.spec.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.spec.{ts,tsx}: Use test() instead of it() for test cases in Vitest unit tests
Use vi.mock(import('...')) for auto-mocking modules in Vitest. Avoid manual mock factories (vi.mock('...', () => ({...}))) when possible
Use vi.resetAllMocks() in beforeEach for resetting mocks in Vitest unit tests, not vi.clearAllMocks()
When an auto-mocked function or class method needs a real implementation in Vitest, use vi.mocked(...). For class methods, use the prototype pattern: vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)

Files:

  • packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
packages/main/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

packages/main/src/**/*.{ts,tsx}: Long-running operations should use TaskManager with createTask() to provide user feedback on operation status
Container operations in the main process must use ContainerProviderRegistry with the engineId parameter to identify the container engine
Kubernetes operations in the main process should use KubernetesClient for 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 via SafeStorageRegistry instead of plain configuration

Files:

  • packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
  • packages/main/src/plugin/agent-workspace/agent-workspace-manager.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/agent-workspace/agent-workspace-manager.spec.ts
  • packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
🔇 Additional comments (2)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (1)

180-181: Good fix for missing provider metadata fallback.

Routing llmMetadataName === undefined through the OpenAI-compatible path closes the secret-creation gap described in #1795 and keeps host/header behavior consistent.

packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts (1)

626-654: Nice coverage added for the undefined-provider regression path.

These two tests correctly validate both host derivation from endpoint and fallback to api.openai.com, matching the new behavior.


📝 Walkthrough

Walkthrough

This PR fixes handling of LLM providers that do not set a metadata name, such as OpenShift AI. When llmMetadataName is undefined, the buildSecretOptions method now routes to OpenAI-compatible secret configuration instead of returning undefined, deriving the host from the endpoint or defaulting to api.openai.com.

Changes

Handle undefined llmMetadataName in secret options

Layer / File(s) Summary
Core Implementation
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
Switch case extends openai case to also match provider === undefined, applying OpenAI secret configuration (endpoint-derived host with api.openai.com fallback, Bearer token auth) instead of falling through to default.
Tests
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
Two new buildSecretOptions test cases verify correct behavior when llmMetadataName is undefined: one with endpoint-derived host extraction, one with api.openai.com default.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • openkaiden/kaiden#1762: Both PRs modify AgentWorkspaceManager's secret-creation logic—specifically how provider credentials (OpenAI/undefined llmMetadataName and endpoint-derived hosts) are mapped to KDN secret options.

Suggested reviewers

  • fbricon
  • bmahabirbu
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: handling the case when llmMetadata.name is absent during secret creation.
Description check ✅ Passed The description references the linked issue #1795, which is directly related to the changeset addressing the handling of absent llmMetadata.name.
Linked Issues check ✅ Passed The code changes directly address issue #1795 by implementing logic to handle undefined llmMetadataName in buildSecretOptions, treating it as OpenAI-compatible and creating secrets as expected.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the absent llmMetadata.name handling in secret creation, with no unrelated modifications present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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 and usage tips.

@codecov

codecov Bot commented May 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@slemeur

slemeur commented May 10, 2026

Copy link
Copy Markdown
Contributor

@CodeRabbit full review

@coderabbitai

coderabbitai Bot commented May 10, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@jeffmaury
jeffmaury merged commit 1c0d016 into openkaiden:main May 11, 2026
15 checks passed
@jeffmaury
jeffmaury deleted the GH-1795 branch May 11, 2026 04:55
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.

Provider not setting lllMetadata.name (OpenShift AI) are not handled correctly

4 participants