Repository navigation
fix: report why a provider failed to load instead of hiding it - #14942
Merged
Merged
Conversation
When an organization-scoped Kilo token is rejected with a 401, the model catalog comes back empty, which removes the provider from `all` in the /provider response. `pickBy` can only filter existing keys, so the `failedSet.has(id)` retention clause was unreachable: clients received `failed: ["kilo"]` with no row to attach a reason to, and the picker just showed "No results found" with the cause visible only in the logs. Add a typed `failures` array to the provider list contract and emit a models-less placeholder for any failed provider missing from `all`, so the CLI, TUI, and VS Code can name the cause and offer re-authentication. The placeholder is synthetic and never sourced from the raw catalog, so the public models.dev model list still cannot leak into an org context, and it respects enabled_providers/disabled_providers.
github-actions
Bot
added
area:cli
The provider list contract gained a failures array, so the remote session list_models response now includes failures: []. Update the RemoteSender expectation to match.
github-actions
Bot
removed
needs-triage
kirillk
marked this pull request as ready for review
October 14, 2026 21:30
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The incremental pass covers the two follow-up commits. Both previously reported issues are fixed:
No new correctness, security, performance, or memory-leak concerns were found in the changed lines. Files Reviewed (3 files)
Previous Review Summary (commit 36c0801)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 36c0801)Status: 2 Issues Found | Recommendation: Address before merge Overview
The core contract and reporting change is sound: the Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (45 files)
Reviewed by deepseek-v4.1-flash · Input: 116.4K · Output: 7.6K · Cached: 311.3K Review guidance: REVIEW.md from base branch |
Address review feedback on the provider-failure reporting change. - ProvidersTab: read the failure inside a createMemo so a changed reason re-renders instead of keeping the previous text. A non-keyed Show does not re-run its render function when one truthy value replaces another. - Cover the no-status branches for unauthorized and http in the TUI dialog test, and add a direct unit test for providerFailureMessageKey, providerFailure, and providerFailureNeedsSignIn.
marius-kilocode
approved these changes
Oct 15, 2026
This was referenced Oct 15, 2026
This was referenced Oct 16, 2026
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
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.
Issue
No existing issue. Found while diagnosing a customer log bundle where
/connectshowed "No results found" forkiloand the real cause was visible only in the CLI log file.What Problem This Solves
When the Kilo gateway rejects a user's credentials, the provider silently disappears from the picker instead of reporting why.
From the customer's logs, the gateway returned
401for an organization-scoped token:Two separate defects turned that into a dead end:
Provider.ListResultcarriedfailed: string[]— provider IDs with no error kind. The TUI even documented this as the reason it had to show a generic message./providerhandler, an org-scoped catalog is removed (delete filtered.kilo), andProvider.Service.list()drops zero-model providers.validProvidersis then computed with remedapickBy, which can only filter existing keys and can never re-add one — so thefailedSet.has(id)retention clause was unreachable. The response containedfailed: ["kilo"]but nokiloentry inall.The result: the server knew the provider had failed and said so, but every client had nowhere to put that fact. The TUI's
renderGutter/failedDescriptionhelpers were effectively dead code in exactly the case they were written for, VS Code collapsedfailedinto a single boolean it never forwarded to the webview, and the user saw "No results found".Why This Change Was Made
The fix deliberately keeps the existing 401 behavior for org-scoped tokens. Skipping the public-catalog retry is intentional — it is pinned by
kilo-gateway/test/api/models.test.ts("never retries an organization-scoped 401 against the public catalog"), because serving public models to an org-governed user would bypass that org's model policy. This PR closes the reporting gap, not the fallback rule.Because
pickBycannot re-add a key, the placeholder has to be an explicit insert. It is built synthetically withmodels: {}and takes onlynamefrom the raw catalog, never models — so the public models.dev list still cannot leak into an org context. The existingnot.toContain("public/leak")assertions are the regression guard.Changes:
failures: [{ providerID, kind, status? }]array to the provider list contract, withkindcoveringunauthorized,unauthenticated,network,http, andschema.failed: string[]is kept unchanged for back-compat.allfor any failed provider missing from it, filtered through the sameenabled_providers/disabled_providersgate so a disabled provider is never resurrected.unauthenticatedkind, so a rejected token reportsunauthorizedrather than "not signed in".failuresto the webview and surface it in the provider picker, the Providers tab Kilo Gateway row, and the model picker label, offering Sign in only forunauthorized/unauthenticated(a retry button would be misleading for a network or 5xx error).JetBrains is intentionally not included — see Dependency.
User Impact
A user whose Kilo credentials are rejected now sees "Credentials rejected (HTTP 401). Sign in again." with a working sign-in action, instead of the provider vanishing with no explanation. Network, HTTP, and schema failures report their own cause and status code without offering a misleading sign-in action.
Nothing changes for users whose providers load normally. Clients that ignore
failureskeep working, and an older client still receives the unchangedfailedarray.Screenshots / Video
Not yet captured — this PR has visual changes and should not be marked ready for review until they are added.
Reproducing the states requires a Kilo token that the gateway rejects while an
accountIdis still stored, which I cannot stage without valid org credentials. I did not want to synthesize a product screenshot. The affected surfaces are the CLI/connectdialog, the VS Code provider picker, the Providers tab Kilo Gateway row, and the model picker label.Evidence
All commands below were executed by the agent in this worktree on the final diff.
bun run typecheck(packages/opencode/): passed, no errors.bun test ./test/kilocode/server/provider-auth-failure.test.ts ./test/kilocode/cli/tui/component/dialog-provider.test.ts ./test/kilocode/provider-list-failed-state.test.ts ./test/kilocode/catalog-recovery.test.ts ./test/kilocode/kilo-models-401-fallback.test.ts ./test/kilocode/model-cache-org.test.ts ./test/cli/run/runtime.boot.test.ts ./test/kilocode/sessions/remote-model-catalog.test.ts ./test/server/httpapi-provider.test.ts: 87 pass, 1 pre-existing skip, 0 fail.bun test tests/unit/ --dots(packages/kilo-vscode/): 6895 pass, 2 skip, 0 fail.bun run check-types:webview,bun run lint,bun run knip,bun run check-kilocode-change(packages/kilo-vscode/): all passed.bun run script/check-opencode-annotations.ts --worktree: passed — every shared upstream edit carries akilocode_changemarker.turbo typecheckacross all 29 packages: ran automatically via the pre-push hook and passed, including the JetBrains Gradle typecheck. The hook initially blocked the push on a real type error in my new test file; that is fixed ine2c83aeand the hook then passed. The hook was never bypassed.Blocked check
bun test(fullpackages/opencode/suite) exceeded my local 15-minute command timeout and was not run to completion. Substitute verification was the targeted 9-file run above, which covers every test touching the changed provider/catalog paths, plus the repo-wideturbo typecheck. CI will run the full suite.Reviewer test steps
accountIdpoints at an org the token is no longer valid for, runkiloand open/connect.kilorow is present with a red!and reads "Credentials rejected (HTTP 401) — sign in again", rather than "No results found".A changeset is included (
patchfor@kilocode/cliandkilo-code).Dependency
None for merging.
Follow-up: the JetBrains plugin needs the same treatment but is blocked on a CLI pin bump. It is pinned to a published CLI release (
kilo.cli.pinned=true, currently7.8.7inpackages/kilo-jetbrains/package.json), and that pin feeds both the runtime binary and the generated OpenAPI spec. Until the pin moves to a release containing this change, the plugin would never receivefailures, and any test against the generatedProviderList200Response.failureswould not compile. VS Code has no such constraint because it builds and bundles the CLI from monorepo source, which is why both ship together here.