feat-cli-multi-provider-onboarding - #1
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| return { type: "local-config-yaml" }; | ||
| } | ||
|
|
||
| return { type: "no-config" }; |
There was a problem hiding this comment.
WARNING: Persisted config URI is now unreachable (session continuity regression)
determineConfigSource only returns cli-flag, local-config-yaml, or no-config. loadConfiguration still calls updateConfigUri(getUriFromSource(...)) (configLoader.ts:64), but on the next run that saved URI is ignored because this function never returns saved-uri. For slug-based configs (cn --config owner/package) and the previously supported remote default, the saved config is silently dropped and the CLI falls back to no-config instead of restoring it. Either re-introduce a saved-uri branch here or stop persisting it.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return undefined; | ||
| } | ||
|
|
||
| await createOrUpdateProviderConfig(setup.provider, setup.values); |
There was a problem hiding this comment.
WARNING: Unhandled throw from createOrUpdateProviderConfig can crash headless startup
prepareHeadlessConfiguration calls await createOrUpdateProviderConfig(...) without a guard, and the caller in services/index.ts does not catch it. resolveHeadlessProviderSetup auto-selects any provider whose environment is detected, including bedrock via ambient AWS_REGION/AWS_PROFILE or azure via AZURE_*. When the selected provider is missing a required field (e.g. bedrock has no model and no CONTINUE_BEDROCK_MODEL), buildProviderModels -> validateProviderSetup throws ("Model is required for Amazon Bedrock"), which propagates and aborts CLI startup in any AWS/CI environment that merely exports AWS_REGION. Validate that buildProviderModels succeeds (or that required fields are present) before writing, and skip headless config creation on failure instead of throwing.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } | ||
| }; | ||
|
|
||
| internal_eventEmitter?.on("input", handleUnexposedNavigationKeys); |
There was a problem hiding this comment.
SUGGESTION: Home/End navigation is handled twice
This raw internal_eventEmitter "input" listener processes Home/End escape sequences, while useInput (see the navigationKey.home / navigationKey.end branches below) also handles Home/End. Both fire onNavigate, so the same keypress triggers redundant navigation calls. Consider relying on ink's useInput key.home/key.end exclusively and removing the duplicate raw-byte listener to avoid divergence and extra re-renders.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| const nextValues = { ...values, [activeField.id]: value }; | ||
| const nextIndex = fieldIndex + 1; | ||
| if (nextIndex >= activeFields.length) { |
There was a problem hiding this comment.
SUGGESTION: A provider with zero fields makes onboarding impossible to complete
completeField returns early when !activeField (line 108), and activeFields.length is 0 for a field-less provider, so pressing Enter never reaches the onComplete branch and the flow hangs with no visible prompt. The current registry always supplies fields, but this is a latent trap for any future supportsCustomModel/none-auth provider without fields. Add an early onComplete({ provider, values }) path when activeFields.length === 0.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| const typedValue = currentValue.trim(); | ||
| const detected = detectFieldEnvironment(selectedProvider, activeField); | ||
| const value = typedValue || detected?.value || undefined; |
There was a problem hiding this comment.
SUGGESTION: Optional fields detected from the environment cannot be cleared
const value = typedValue || detected?.value || undefined means that if a user backspaces an env-detected value to empty, typedValue is empty so it falls back to the detected env value. For optional fields (e.g. ollama/lmstudio endpoint when OLLAMA_HOST is set), the user cannot intentionally leave the field blank. Prefer using the typed value when the user has edited the field, e.g. track whether the field was touched and only fall back to detected when untouched and empty.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| name: preset.name, | ||
| provider: provider.adapterProvider, | ||
| model: preset.model, | ||
| ...(apiKey ? { apiKey } : {}), |
There was a problem hiding this comment.
SUGGESTION: Provider secrets are materialized into the on-disk YAML config in plaintext
buildProviderModels attaches the resolved apiKey directly to the ModelConfig (as does setup.apiKey in onboarding.ts:149), and upsertProviderModelsInYaml serializes it to config.yaml. The "mask secrets" work here only covers display-while-typing; the credential is still written to disk. writeConfigAtomically does set 0o600, which is good, but consider documenting this clearly and/or supporting a reference to an env var (e.g. env: { ... }) rather than inlining the secret, to reduce exposure of long-lived keys at rest.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 6 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (15 of 50 changed files)
Fix these issues in Kilo Cloud Reviewed by hy3:free · Input: 116.5K · Output: 25K · Cached: 1.2M |
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
- Restore saved-uri session continuity: persist and read the last config URI
so --config owner/slug selections survive across runs (configSource,
workos getConfigUri/updateConfigUri, GlobalContext cliConfigUri).
- Guard prepareHeadlessConfiguration: wrap provider creation in try/catch and
warn instead of crashing headless/CI startup on optional auto-detected
providers missing required fields.
- Remove duplicate Home/End handling from Selector's raw event listener; keep a
single source of truth via useInput (raw listener kept minimal to preserve
Ink stdin processing for Enter/Esc/Ctrl+C).
- Allow onboarding completion when a provider has zero/optional fields.
- Allow clearing env-detected fields (explicit empty input clears the value
rather than falling back to the detected value).
- Avoid writing apiKey to disk in plaintext: persist a secrets reference
(${{ secrets.<ENV> }}) that the CLI resolves from the environment at runtime.
- Selector: guard against undefined terminal rows (NaN visibleOptionCount produced an empty option list in non-TTY/headless environments); default to 24 rows. Remove dead no-op stdin listener. - onboarding: replace hard throw on interactive cancellation with a clean user-facing error and exit(1) instead of an unhandled rejection/stack. - Remove committed vitest output artifact (cli-test-out.txt).
The previous fix left a no-op raw stdin listener (its body was `void data`) and relied on `key.home`/`key.end` in `useInput`. Ink 6 never populates those properties - only `pageUp`/`pageDown` are mapped - so Home/End navigation was dead and `Selector.test.tsx` failed. Handle Home/End in the raw listener again (the only place the sequences are available), covering the xterm/gnome/rxvt/putty variants Ink itself parses, and keep a single source of truth by dropping the unreachable `useInput` branches. PageUp/PageDown now use the properly typed `key` fields instead of a cast, and the listener no longer navigates while the selector is loading or in an error state.
…variable
Masking an apiKey into a `${{ secrets.<ENV> }}` reference had three gaps:
- It was skipped whenever `auth.envNames` was empty, so Vertex AI (auth kind
"none" plus an `apiKey` field bound to GOOGLE_API_KEY) still wrote its key to
config.yaml in plaintext.
- It always referenced `auth.envNames[0]`, so a key detected from a secondary
alias (Gemini's GOOGLE_API_KEY, Azure's AZURE_API_KEY) was rewritten as a
reference to a different, unset variable.
- An interactively typed key was discarded: nothing wrote it to disk or to
process.env, so the config referenced a secret that could never resolve and
the CLI could not authenticate - without any warning.
`getProviderApiKeyEnvNames` now unions `auth.envNames` with the `apiKey` field's
own `envNames`, and `resolveApiKeyEnvName` prefers the variable that actually
holds the value. `persistProviderApiKey` writes a typed key to the CLI's
`~/.continue/.env` (0600, preserving other entries) and exports it into
process.env so `CLIPlatformClient` resolves it in the current run too; it warns
when an existing export would shadow the stored value. `runOnboardingFlow`
reports where the credential was saved.
Also updates the stale comment in `buildProviderModels`, which claimed the
resolved key is persisted in plaintext.
The file was committed by mistake and leaked an unrelated local workspace path in its vitest header. Ignore it so a test run cannot re-add it.
`buildProviderModels` returns `defaultCompletionOptions` from `@continuedev/config-yaml`, where every entry is optional, so passing those models to `upsertProviderModelsInYaml` failed `tsc --noEmit`.
…ttle Ink writes at most one frame per 32ms, so fixed 20ms sleeps made the typed-key test fail roughly one run in three. Poll for the expected frame instead, and let Ink's input effect resubscribe before sending the next keystroke.
|
Cloud Agent result for continued-agent/continued (code, agent_a1d76cd1-7bbb-4a68-8432-19ae72b5ea57, completed): All fixes are pushed to What I fixed1. I verified with the installed ink (6.2.2, matching
2.
3. Stray artifact — commit Also (both pre-existing blockers found while verifying):
I also rebased onto Tests actually runI unblocked the environment by installing
Not fixed / pre-existing: the full suite is 1729 passed / 11 failed. All 11 failures are in One thing I deliberately left alone: ink's |
|
"This branch has conflicts that must be resolved README.md |
…er-onboarding # Conflicts: # README.md
|
The conflict on
The PR is ready to merge: #1 Note: this was verified by a read-only (ask-mode) Cloud Agent, not by me directly. If you'd like, I can have a code-mode agent re-confirm the live GitHub mergeable status once more before you click merge. |
1 participant
Summary
--config,FORCE_NO_TTY, and Bedrock support explicit.Validation
npm run build:validate: passed.extensions/cli.Environment Limitations
The full CLI suite, global type-checking, npm linting, build process, and headless E2E tests remain blocked by missing
distartifacts or dependencies in sibling packages (config-yaml,openai-adapters,terminal-security, etc.). No actual provider calls were made.