feat: add Kilo SDK-backed provider adapter#4089
Closed
Githubguy132010 wants to merge 17 commits into
Closed
Conversation
Add a first-party `kilo` provider that spawns managed `kilo serve`, inventories connected models as providerID/modelID, and streams turns via @kilocode/sdk (OpenCode-shaped runtime/adapter pattern).
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
ApprovabilityVerdict: Needs human review This PR introduces an entirely new provider integration (Kilo) with ~3800 lines of new code including driver, adapter, runtime process management, and UI integration. New features adding user-facing capabilities and external SDK integrations warrant human review. You can customize Macroscope's approvability policy. Learn more. |
Keep draining Kilo serve stdout/stderr after ready, terminate on startup timeout, clear interrupt/failed turn state, and drop phantom lockfile deps.
Avoid racing concurrent startSession calls that replace the map entry while an earlier call is still tearing down the previous context.
Treat unexpected clean event-stream ends as session transport errors, and stop retaining stdout/stderr after ready while still draining pipes.
Prevents clean-stream-end teardown from racing permission-reply tests.
Avoid hanging the adapter layer teardown while still preventing clean stream-end from racing mid-test permission replies.
Stop retaining stdout/stderr after ready while still draining pipes. Leave clean SSE completion silent (matches OpenCode) so finite test streams do not race session teardown.
Avoid dropping concurrent stderr diagnostics between URL detection and startup completion.
Serialize steer-vs-open with Ref.modify so two idle sendTurn calls cannot both emit turn.started or orphan the first turn id.
Avoid wiping a turn that concurrent steers still rely on when the original promptAsync fails after steer reuse.
Clear activeTurnId before remote abort and skip ready reset if a newer claim arrived; fail startSession when the session dies during event-pump setup.
Cursor Bugbot review of 4dd769b flagged three race conditions in the Kilo adapter lifecycle. Each was an ownership bug on activeTurnId where concurrent steer / interrupt / idle events could clear or emit lifecycle events for a turn the caller no longer owned. - ensureSessionContext is now an Effect.fn so it surfaces ProviderAdapterSessionNotFoundError / ProviderAdapterSessionClosedError on the Effect error channel instead of as defects; stopSession does the same. Matches OpenCodeAdapter and lets callers recover from unknown or closed threads. - session.status idle handler uses Ref.modify to compare-and-set: it only clears activeTurnId and emits turn.completed when the ref still matches the turn id it observed at the top of the handler. A concurrent interrupt or steer no longer gets its claim clobbered by a late idle. - interruptTurn records whether the call actually owned the claimed turn; if the requested turn id is stale (e.g. interrupted twice, or a newer claim exists), session.abort is skipped so the SDK abort cannot cancel an unrelated newer turn. Adds three regression tests covering each path. Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
- Compare-and-delete in onSessionEnd/onSessionClosed so a concurrent
startSession replacing the entry isn't orphaned.
- Read the live activeTurnId for late session.status=busy events instead
of a stale snapshot, avoiding resurrecting an already-interrupted turn.
- Split interruptTurn so it clears the claim BEFORE session.abort and
restores it on abort failure, emitting turn.completed(failed) when
the abort RPC fails after we owned the claim.
- Drop unused ProviderDriverKind import and stale PROVIDER argument
from providerModelsFromSettings calls in KiloProvider.
- Register kilo: { enabled: false } in ProviderRegistry.test default
settings so the registry stays correct when Kilo is added.
…bort Cursor Bugbot flagged a race in interruptTurn: clearing activeTurnId before calling session.abort opens a window where a concurrent sendTurn can claim a fresh turn id which the in-flight (session-wide) abort RPC then unintentionally cancels. Leave the claim in place across the abort, clear it once the RPC resolves, and drop the now-unneeded restore logic in the abort-failure path. Adds a regression test that pins interruptTurn inside the abort RPC and verifies a concurrent sendTurn steers into the existing turn id instead of opening a new one. Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
Member
|
we'll start adding new providers after orchestrator has merged (#2829) |
Author
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.
What Changed
Add a first-party
kiloprovider adapter that runs Kilo via@kilocode/sdk(managedkilo serve+ SSE), not CLI scraping.KiloSettings(enabled,binaryPath,customModels), model defaults/display name, raw event sourcekilo.sdk.eventkiloRuntime,KiloProvider,KiloAdapter,KiloDriver,BUILT_IN_DRIVERS+ runtime layer wiringproviderID/modelIDfrom connected upstreamspromptAsync, interrupt, best-effort permission replies (once/always/reject)code; plan interaction mode → agentplanKiloIcon, settings card, model picker entryWhy
Users need Kilo as a top-level T3 provider with the same reliability pattern as OpenCode (SDK + managed server), not a brittle CLI wrapper. Scope stays v1: no agent picker, no external server URL/password settings, no hard resume guarantees, text-gen intentionally unsupported.
UI Changes
new) and provider settings with binary path onlyNo motion/interaction changes.
Checklist
Note
Medium Risk
Large new adapter with child-process lifecycle and subtle concurrent turn/interrupt races; mitigated by extensive tests and scope finalizers, but it is new core provider infrastructure parallel to OpenCode.
Overview
Adds Kilo as a first-party provider using
@kilocode/sdk(managedkilo serve, Basic auth, SSE) instead of CLI scraping, following the same driver/runtime/adapter pattern as OpenCode.Contracts & UI:
KiloSettings(enabled,binaryPath,customModels),kilodriver kind, default models, raw event sourcekilo.sdk.event, settings card,KiloIcon, and model-picker entry.Server:
KiloRuntimespawns/supervises the local server and loads upstream inventory;KiloProviderprobes the CLI and flattens connectedproviderID/modelIDmodels (plus custom slugs);KiloAdaptermaps SDK events toProviderRuntimeEvent, handles sessions/turns, permissions, user-input, read/rollback, and race-safe interrupt/steer logic (session-widesession.abort, compare-and-set on idle/busy).KiloDriverwires snapshot, adapter, and stubKiloTextGeneration(git/title ops fail with a clear message). Registered inBUILT_IN_DRIVERSwithKiloRuntimeLiveon the server runtime layer.v1 behavior: default agent
code, plan mode →plan; no agent picker, no external server URL settings, no Kilo text generation.Reviewed by Cursor Bugbot for commit 5797ecf. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add Kilo SDK-backed provider adapter, driver, and runtime integration
KiloRuntimeservice (kiloRuntime.ts) that spawns and manages a local Kilo server process, connects to it, and loads provider/model inventory via the@kilocode/sdk.KiloDriver(KiloDriver.ts) andKiloAdapter(KiloAdapter.ts) to handle session lifecycle, turn management, permission replies, and error mapping.KiloProvider(KiloProvider.ts) with a probing pipeline that classifies failures (ENOENT, auth, quarantine) into structured provider snapshots with installed/version/auth status.KILO_DRIVER_KIND, default models, display name), server settings (KiloSettings), built-in driver registry, and web UI provider picker and settings panel.TextGenerationErrorindicating Kilo does not yet support git text generation.KiloRuntimeLiveis now merged into the server'sRuntimeCoreDependenciesLive, meaning any startup failure in Kilo runtime initialization affects the full server boot path.Macroscope summarized 5797ecf.