fix(onboarding): recover compatible models in guided setup#2250
Open
bmahabirbu wants to merge 1 commit into
Open
fix(onboarding): recover compatible models in guided setup#2250bmahabirbu wants to merge 1 commit into
bmahabirbu wants to merge 1 commit into
Conversation
bmahabirbu
requested review from
benoitf and
gastoner
and removed request for
a team
Contributor
📝 WalkthroughWalkthroughAdds a ChangesGuided-setup onboarding refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
![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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/renderer/src/lib/guided-setup/panels/ClaudePanel.svelte (1)
39-42: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
advanceOnSuccessis bypassed when a connection already exists.
validate()now returnsadvanceOnSuccesson the create-connection success path, but thealreadyConnectedbranch still returnstrue. That makesadvanceOnSuccess={false}ineffective for existing connections.Suggested change
async function validate(): Promise<boolean> { if (alreadyConnected) { - return true; + return advanceOnSuccess; }Also applies to: 95-95
🤖 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/renderer/src/lib/guided-setup/panels/ClaudePanel.svelte` around lines 39 - 42, The validate() function in ClaudePanel.svelte returns true directly when alreadyConnected is true, which bypasses the advanceOnSuccess parameter that should control the behavior in all cases. Modify the alreadyConnected branch to return advanceOnSuccess instead of true, so that the advanceOnSuccess setting is respected consistently whether the connection already exists or is being newly created.packages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.svelte (1)
61-64: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
advanceOnSuccessis ignored in thealreadyConnectedfast path.The new success-control prop is applied after connection creation, but not when
alreadyConnectedis true, so validation can still advance unexpectedly.Suggested change
async function validate(): Promise<boolean> { if (alreadyConnected) { - return true; + return advanceOnSuccess; }Also applies to: 143-143
🤖 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/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.svelte` around lines 61 - 64, The validate() function in ClaudeVertexPanel.svelte returns true immediately when alreadyConnected is true, bypassing the advanceOnSuccess prop handling that should apply to all successful validations. Modify the early return path in the alreadyConnected condition to ensure the advanceOnSuccess prop is applied before returning true, so the success-control behavior is consistent across both the alreadyConnected fast path and the regular connection flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/renderer/src/lib/guided-setup/ModelStep.spec.ts`:
- Around line 103-118: The code currently creates only a single summary by
accessing inferenceConnections[0], which collapses all configured connections
for a provider into one entry. Replace the single representative assignment with
a loop that iterates through each element in the inferenceConnections array. For
each connection in the array, create a separate summary object with the same
structure currently shown, using that specific connection's properties instead
of always using the first connection. This ensures each configured inference
connection gets its own summary entry rather than hiding multiple connections
behind a single representative.
In `@packages/renderer/src/lib/guided-setup/ModelStep.svelte`:
- Around line 79-87: The effect hook's early return condition on line 80 skips
setting the requireCompatibleModel guard when usesInlineConnectionValidation is
true, which allows advancing without a selected effectiveModel. Modify the
condition to only skip the guard setup when effectiveModel already exists,
removing the usesInlineConnectionValidation check from the early return logic.
This ensures the requireCompatibleModel gate remains active to prevent advancing
from the "No compatible models" state even when inline connection validation has
passed, until an actual effectiveModel is selected. Apply the same logic fix to
the related code at lines 132-133.
- Around line 64-72: The $effect block in ModelStep.svelte returns early when
effectiveModel is falsy without clearing the previous onboarding.model value,
leaving stale data that gets persisted later when agents are switched or
compatibility is lost. Modify the early return condition to explicitly clear
onboarding.model (set it to null or undefined) before returning, ensuring no
stale model data remains when effectiveModel is absent.
In `@packages/renderer/src/lib/guided-setup/panels/ClaudePanel.svelte`:
- Around line 8-10: Replace the relative import paths with the configured `/@/`
alias for the three imports that reference modules outside the panels directory.
Specifically, convert the imports for AgentDefinition from '../agent-registry',
OnboardingState from '../guided-setup-steps', and the refreshInferenceSources
function from '../refresh-inference-sources' to use paths prefixed with `/@/`
instead, following the coding guideline that imports outside the current module
group should use the alias style.
In `@packages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.svelte`:
- Around line 13-15: Replace the relative imports in ClaudeVertexPanel.svelte
that use `../` paths with the `/@/` alias convention. The three imports for
AgentDefinition, OnboardingState, and refreshInferenceSources all climb to
parent directories and should be converted to use the repository's alias path
pattern instead of relative paths, as per the coding guidelines for non-sibling
imports.
In `@packages/renderer/src/lib/guided-setup/panels/OpenRuntimePanel.svelte`:
- Around line 8-9: Replace the relative imports that point outside the current
directory with their corresponding `/@/` aliases. Specifically, convert the
import of AgentDefinition from '../agent-registry' and the import of
refreshInferenceSources from '../refresh-inference-sources' to use the `/@/`
alias convention instead of relative paths. This follows the repo's coding
guidelines where relative imports should be reserved only for sibling modules
within the same directory.
---
Outside diff comments:
In `@packages/renderer/src/lib/guided-setup/panels/ClaudePanel.svelte`:
- Around line 39-42: The validate() function in ClaudePanel.svelte returns true
directly when alreadyConnected is true, which bypasses the advanceOnSuccess
parameter that should control the behavior in all cases. Modify the
alreadyConnected branch to return advanceOnSuccess instead of true, so that the
advanceOnSuccess setting is respected consistently whether the connection
already exists or is being newly created.
In `@packages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.svelte`:
- Around line 61-64: The validate() function in ClaudeVertexPanel.svelte returns
true immediately when alreadyConnected is true, bypassing the advanceOnSuccess
prop handling that should apply to all successful validations. Modify the early
return path in the alreadyConnected condition to ensure the advanceOnSuccess
prop is applied before returning true, so the success-control behavior is
consistent across both the alreadyConnected fast path and the regular connection
flow.
🪄 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: 06e34def-7337-4dca-8f36-b4a32050e260
📒 Files selected for processing (11)
packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.tspackages/renderer/src/lib/guided-setup/CodingAgentStep.sveltepackages/renderer/src/lib/guided-setup/GuidedSetup.sveltepackages/renderer/src/lib/guided-setup/ModelStep.spec.tspackages/renderer/src/lib/guided-setup/ModelStep.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudePanel.spec.tspackages/renderer/src/lib/guided-setup/panels/ClaudePanel.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.spec.tspackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.sveltepackages/renderer/src/lib/guided-setup/panels/OpenRuntimePanel.sveltepackages/renderer/src/lib/guided-setup/refresh-inference-sources.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (9)
- GitHub Check: Linux
- GitHub Check: Windows
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: unit tests / macos-15
- GitHub Check: macOS
- GitHub Check: unit tests / windows-2022
- GitHub Check: linter, formatters
🧰 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:
packages/renderer/src/lib/guided-setup/refresh-inference-sources.tspackages/renderer/src/lib/guided-setup/CodingAgentStep.spec.tspackages/renderer/src/lib/guided-setup/panels/ClaudePanel.spec.tspackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.spec.tspackages/renderer/src/lib/guided-setup/ModelStep.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/renderer/src/lib/guided-setup/refresh-inference-sources.tspackages/renderer/src/lib/guided-setup/CodingAgentStep.spec.tspackages/renderer/src/lib/guided-setup/panels/ClaudePanel.spec.tspackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.spec.tspackages/renderer/src/lib/guided-setup/ModelStep.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/renderer/src/lib/guided-setup/CodingAgentStep.spec.tspackages/renderer/src/lib/guided-setup/panels/ClaudePanel.spec.tspackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.spec.tspackages/renderer/src/lib/guided-setup/ModelStep.spec.ts
🧠 Learnings (10)
📚 Learning: 2026-04-15T08:04:32.031Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1336
File: packages/renderer/src/lib/guided-setup/GuidedSetup.svelte:9-11
Timestamp: 2026-04-15T08:04:32.031Z
Learning: For Svelte components in this repo, if a callback prop is typed as `() => void`, TypeScript idiomatically allows passing async functions (e.g., `() => Promise<void>`), because `() => void` indicates the caller ignores the return value rather than requiring `undefined`. Do not recommend changing these prop types to `() => void | Promise<void>` solely to “fix” async compatibility—unless there is an actual need for the caller to observe the returned value.
Applied to files:
packages/renderer/src/lib/guided-setup/panels/OpenRuntimePanel.sveltepackages/renderer/src/lib/guided-setup/CodingAgentStep.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudePanel.sveltepackages/renderer/src/lib/guided-setup/ModelStep.sveltepackages/renderer/src/lib/guided-setup/GuidedSetup.svelte
📚 Learning: 2026-04-20T14:29:13.040Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1396
File: packages/renderer/src/lib/guided-setup/AgentTileCard.svelte:30-32
Timestamp: 2026-04-20T14:29:13.040Z
Learning: In this repository’s guided-setup Svelte components, allow the hard-coded Tailwind success/badge indicator classes `text-green-400` and `bg-green-400/10` (e.g., for the “Recommended” badge in `AgentTileCard.svelte` and the probe-detected success state in `CodingAgentStep.svelte`). Do not treat these specific green classes as a “color-registry/semantic token” violation as long as no semantic color-registry tokens for badge-style success states exist; reassess and update once the color registry is extended to include theme-safe success/badge tokens.
Applied to files:
packages/renderer/src/lib/guided-setup/panels/OpenRuntimePanel.sveltepackages/renderer/src/lib/guided-setup/CodingAgentStep.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudePanel.sveltepackages/renderer/src/lib/guided-setup/ModelStep.sveltepackages/renderer/src/lib/guided-setup/GuidedSetup.svelte
📚 Learning: 2026-04-23T11:33:35.749Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1431
File: packages/renderer/src/lib/guided-setup/CodingAgentStep.svelte:43-52
Timestamp: 2026-04-23T11:33:35.749Z
Learning: In the guided-setup UI for openkaiden/kaiden, treat `CliAgent` as the source-of-truth union type for agent names supported by the `kdn` CLI (e.g., the values accepted by `kdn init --agent`). The UI registry `agentDefinitions` (in `agent-registry.ts`) must be a subset that satisfies/conforms to `CliAgent` and must not define or derive `CliAgent`. Do not suggest deriving `CliAgent` from the UI registry (that would invert the intended dependency direction). In places like `CodingAgentStep.svelte`, an `as CliAgent` cast is acceptable only when it’s safe by construction—i.e., when the source value such as `agentDefinitions[].cliName` is already typed as `CliAgent`.
Applied to files:
packages/renderer/src/lib/guided-setup/panels/OpenRuntimePanel.sveltepackages/renderer/src/lib/guided-setup/refresh-inference-sources.tspackages/renderer/src/lib/guided-setup/CodingAgentStep.spec.tspackages/renderer/src/lib/guided-setup/panels/ClaudePanel.spec.tspackages/renderer/src/lib/guided-setup/CodingAgentStep.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudePanel.sveltepackages/renderer/src/lib/guided-setup/ModelStep.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.spec.tspackages/renderer/src/lib/guided-setup/GuidedSetup.sveltepackages/renderer/src/lib/guided-setup/ModelStep.spec.ts
📚 Learning: 2026-04-28T13:34:51.610Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1431
File: packages/renderer/src/lib/guided-setup/panels/OpenCodePanel.svelte:14-20
Timestamp: 2026-04-28T13:34:51.610Z
Learning: In this repo’s Svelte renderer (packages/renderer/src/**/*.svelte), `podman-desktop/ui-svelte`’s `Link` component does not accept an `href` prop. For opening external URLs, use the established pattern `on:click={() => window.openExternal(url)}` (optionally typed as `on:click={(): Promise<void> => window.openExternal(url)}`), consistent with existing components like `ProviderLinks.svelte`, `WelcomePage.svelte`, and `OpenCodePanel.svelte`. Do not treat `window.openExternal()` usage in renderer Svelte components as bypassing a security restriction, and do not recommend replacing it with `Link`/`href`-based navigation. (There is also no `setupSecurityRestrictionsOnLinks` utility in the renderer.)
Applied to files:
packages/renderer/src/lib/guided-setup/panels/OpenRuntimePanel.sveltepackages/renderer/src/lib/guided-setup/CodingAgentStep.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudePanel.sveltepackages/renderer/src/lib/guided-setup/ModelStep.sveltepackages/renderer/src/lib/guided-setup/GuidedSetup.svelte
📚 Learning: 2026-04-29T11:54:22.423Z
Learnt from: vancura
Repo: openkaiden/kaiden PR: 1494
File: packages/renderer/src/Loader.svelte:13-13
Timestamp: 2026-04-29T11:54:22.423Z
Learning: In this codebase’s renderer, timer variables in Svelte files (e.g., in `packages/renderer/src/Loader.svelte`) that store `setTimeout` handles are intentionally typed as `NodeJS.Timeout` (and not `ReturnType<typeof setTimeout>`). When reviewing, do not flag or recommend changing this to `ReturnType<typeof setTimeout>` for portability—treat `NodeJS.Timeout` as the established convention for renderer timer variables.
Applied to files:
packages/renderer/src/lib/guided-setup/panels/OpenRuntimePanel.sveltepackages/renderer/src/lib/guided-setup/CodingAgentStep.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudePanel.sveltepackages/renderer/src/lib/guided-setup/ModelStep.sveltepackages/renderer/src/lib/guided-setup/GuidedSetup.svelte
📚 Learning: 2026-05-04T17:39:49.418Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1603
File: packages/renderer/src/lib/guided-setup/panels/ClaudePanel.svelte:61-70
Timestamp: 2026-05-04T17:39:49.418Z
Learning: When reviewing renderer Svelte code that calls the IPC `createSecret` handler via `window.createSecret`, don’t suggest switching to checks like `err.code` or a typed/custom error class until the backend IPC layer exposes structured error information. Today the IPC handler only throws a plain `Error` with a human-readable message string, so the only viable way to distinguish cases like “secret already exists” is to perform message substring matching (e.g., `err.message.includes('already exists')`) and handle that branch explicitly.
Applied to files:
packages/renderer/src/lib/guided-setup/panels/OpenRuntimePanel.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudePanel.svelte
📚 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/renderer/src/lib/guided-setup/panels/OpenRuntimePanel.sveltepackages/renderer/src/lib/guided-setup/refresh-inference-sources.tspackages/renderer/src/lib/guided-setup/CodingAgentStep.spec.tspackages/renderer/src/lib/guided-setup/panels/ClaudePanel.spec.tspackages/renderer/src/lib/guided-setup/CodingAgentStep.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudePanel.sveltepackages/renderer/src/lib/guided-setup/ModelStep.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.spec.tspackages/renderer/src/lib/guided-setup/GuidedSetup.sveltepackages/renderer/src/lib/guided-setup/ModelStep.spec.ts
📚 Learning: 2026-06-10T15:22:39.639Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 2115
File: packages/renderer/src/lib/models/SemanticRouterCreate.svelte:134-137
Timestamp: 2026-06-10T15:22:39.639Z
Learning: In multi-step wizard UIs, it’s acceptable for “Next step” buttons to be temporarily non-interactive during incremental work delivered across multiple PRs. When reviewing Svelte files under `packages/renderer/src/lib/**`, do not flag these UX issues if the button lacks an onClick handler *and* there is an explicit TODO comment indicating deferred wiring, and the PR author confirms (in the PR description) that this is an intentional staged implementation. If no TODO/deferred-wiring marker (or no author confirmation) is present, treat missing handlers as a potential UX issue.
Applied to files:
packages/renderer/src/lib/guided-setup/panels/OpenRuntimePanel.sveltepackages/renderer/src/lib/guided-setup/CodingAgentStep.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.sveltepackages/renderer/src/lib/guided-setup/panels/ClaudePanel.sveltepackages/renderer/src/lib/guided-setup/ModelStep.sveltepackages/renderer/src/lib/guided-setup/GuidedSetup.svelte
📚 Learning: 2026-04-15T08:51:08.199Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1336
File: packages/renderer/src/lib/guided-setup/GuidedSetup.spec.ts:64-69
Timestamp: 2026-04-15T08:51:08.199Z
Learning: For Svelte component test files under `packages/renderer/src/**` with names ending in `.spec.ts` that use `render` from `testing-library/svelte`, call `vi.useFakeTimers({ shouldAdvanceTime: true })` inside a `beforeEach`. Keep it in `beforeEach` for these tests to avoid flakiness from Svelte’s internal tick scheduling; do not remove or flag it based on the presence/absence of explicit timer manipulation in individual tests.
Applied to files:
packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.tspackages/renderer/src/lib/guided-setup/panels/ClaudePanel.spec.tspackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.spec.tspackages/renderer/src/lib/guided-setup/ModelStep.spec.ts
📚 Learning: 2026-06-16T06:15:26.225Z
Learnt from: gastoner
Repo: openkaiden/kaiden PR: 2145
File: packages/renderer/src/lib/mcp/MCPServerRemoteListActions.spec.ts:46-49
Timestamp: 2026-06-16T06:15:26.225Z
Learning: For renderer unit tests in this repo (files included in `packages/renderer/vite.config.js` via `setupFiles`), do not manually mock `window` API methods in `beforeEach` or within the spec. The test setup (`packages/renderer/vite.tests.setup.js`) reads `packages/preload/exposedInMainWorld.d.ts` and defines each declared `Window` method on `window` as a `vi.fn()` using `Object.defineProperty`, so `window.*` methods (e.g., `startMcpServer`, `stopMcpServer`, `showMessageBox`) are already present as mocks. Only add an explicit mock if the method is not declared in `exposedInMainWorld.d.ts` (in which case update the `.d.ts` so the shared setup can generate the mock).
Applied to files:
packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.tspackages/renderer/src/lib/guided-setup/panels/ClaudePanel.spec.tspackages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.spec.tspackages/renderer/src/lib/guided-setup/ModelStep.spec.ts
🔇 Additional comments (9)
packages/renderer/src/lib/guided-setup/GuidedSetup.svelte (1)
145-149: LGTM!packages/renderer/src/lib/guided-setup/CodingAgentStep.svelte (1)
10-22: LGTM!Also applies to: 32-40, 55-59, 75-79
packages/renderer/src/lib/guided-setup/ModelStep.svelte (1)
3-3: LGTM!Also applies to: 16-16, 25-25, 43-43, 56-63, 89-131, 134-144
packages/renderer/src/lib/guided-setup/refresh-inference-sources.ts (1)
1-7: LGTM!packages/renderer/src/lib/guided-setup/panels/OpenRuntimePanel.svelte (1)
31-39: LGTM!packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.ts (1)
25-46: LGTM!Also applies to: 170-170, 226-240
packages/renderer/src/lib/guided-setup/panels/ClaudePanel.spec.ts (1)
26-47: LGTM!Also applies to: 129-134, 159-159, 186-207
packages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.spec.ts (1)
27-48: LGTM!Also applies to: 154-159, 209-209, 236-265
packages/renderer/src/lib/guided-setup/ModelStep.spec.ts (1)
25-55: LGTM!Also applies to: 121-157, 174-178, 192-212, 229-237, 303-359
Comment on lines
+79
to
87
| $effect(() => { | ||
| if (effectiveModel || usesInlineConnectionValidation) return; | ||
| onboarding.beforeAdvance = requireCompatibleModel; | ||
| return (): void => { | ||
| if (onboarding.beforeAdvance === requireCompatibleModel) { | ||
| onboarding.beforeAdvance = undefined; | ||
| } | ||
| }; | ||
| }); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the no-model gate active after inline panel validation.
When usesInlineConnectionValidation is true, line 80 skips requireCompatibleModel, so the inline panel becomes the only beforeAdvance guard. But ClaudePanel.svelte returns true immediately for alreadyConnected, before applying advanceOnSuccess, which can advance from the “No compatible models” state with no selectable effectiveModel. Ensure the final continue result is still false while !effectiveModel, even when the inline panel has validated or found an existing connection.
Also applies to: 132-133
🤖 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/renderer/src/lib/guided-setup/ModelStep.svelte` around lines 79 -
87, The effect hook's early return condition on line 80 skips setting the
requireCompatibleModel guard when usesInlineConnectionValidation is true, which
allows advancing without a selected effectiveModel. Modify the condition to only
skip the guard setup when effectiveModel already exists, removing the
usesInlineConnectionValidation check from the early return logic. This ensures
the requireCompatibleModel gate remains active to prevent advancing from the "No
compatible models" state even when inline connection validation has passed,
until an actual effectiveModel is selected. Apply the same logic fix to the
related code at lines 132-133.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Move compatible connection setup to the model step so onboarding can recover when an agent has no usable models without sending users away from the flow. Follow up with the guided-setup and warning fixes needed to keep the recovery flow safe. Fixes openkaiden#2241. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Brian <bmahabir@bu.edu>
bmahabirbu
force-pushed
the
fix/2241-guided-setup-flow
branch
from
5c07b0f to
a2cb2f9
Compare
MarsKubeX
reviewed
Jun 23, 2026
Contributor
There was a problem hiding this comment.
- I find a bit confusing when you are in step 2 and does not have any compatible model and you are able to create a connection to get models: you click on "go to dashboard" when the action performed is "create a connection". IMO the create connection should have a dedicated button in step 2 to create connection and once created, listing models, all in step 2. So when the user clicks on go to dashboard he really goes to dashboard.
- Also I have a question, should we keep the "API KEY" section in step 1? could it be better to only let the agents types and check them in the next step?
WDYT @jeffmaury?
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.
Fixes #2241.
Summary
Screencast.From.2026-06-22.21-02-30.mp4
Test plan
pnpm vitest run packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.ts packages/renderer/src/lib/guided-setup/ModelStep.spec.ts packages/renderer/src/lib/guided-setup/GuidedSetup.spec.ts packages/renderer/src/lib/guided-setup/panels/ClaudePanel.spec.ts packages/renderer/src/lib/guided-setup/panels/ClaudeVertexPanel.spec.tsReadLintson the edited guided-setup filesMade with Cursor