feat: add Kiro CLI ACP provider#4087
Conversation
Add Kiro as a first-party provider over the existing ACP lifecycle so streaming, permissions, cancellation, resume, and model switching stay shared with Grok rather than forked into a third session state machine. - Wire Kiro driver/settings/models and optional ACP authenticate skip - Reuse hardened ACP adapter path with Kiro spawn (`kiro-cli acp`) - Scope mock ACP cancellation to the active prompt for follow-up turns - Add focused Kiro unit/integration coverage
|
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}
ApprovabilityVerdict: Needs human review 3 blocking correctness issues found. New provider feature (Kiro CLI) with substantial new code and modifications to core adapter logic. A high-severity unresolved comment identifies a bug where the idle watchdog can incorrectly cancel legitimate user interactions waiting for approval or elicitation responses. You can customize Macroscope's approvability policy. Learn more. |
|
Addressed both review findings:
Kiro + GrokAdapter tests: 33 passed. |
- AcpSessionRuntime: scope assistant item ids with a per-runtime nonce. A resumed session (session/load) reuses the ACP session id but a new runtime resets the segment counter, so assistant item ids collided with a prior turn's message. The upsert kept the stale created_at, sorting the resumed reply before the new prompt and making it invisible. - Shared ACP adapter: handle session/elicitation (form mode) via the existing user-input flow; advertise elicitation.form capability for Kiro. url-mode elicitations are declined (not renderable). - build-desktop-artifact: allow overriding appId/productName via T3CODE_DESKTOP_APP_ID / T3CODE_DESKTOP_PRODUCT_NAME for side-by-side builds. - Tests: resume-safety regression + updated cursor item-id assertions.
github-actions
Bot
added
size:XXL
- coerceAnswer: only accept recognized boolean representations (yes/true -> true, no/false -> false); omit the key for unrecognized input instead of silently defaulting to false. - UserInputQuestion: add an optional flag (default false). Mark non-required elicitation properties optional so the form can be submitted without inventing values; unanswered optional questions are omitted from the response. - Honor optional questions in the web and mobile submission gates. - Tests: AcpElicitation coercion/optional coverage + web optional-field cases.
| if (property.type === "number" || property.type === "integer") { | ||
| const parsed = Number(value); | ||
| if (!Number.isFinite(parsed)) { | ||
| return undefined; | ||
| } | ||
| return property.type === "integer" ? Math.trunc(parsed) : parsed; | ||
| } |
There was a problem hiding this comment.
🟡 Medium acp/AcpElicitation.ts:165
For an integer property, coerceAnswer accepts fractional input like 1.9 and applies Math.trunc, silently truncating it to 1 in the response. A fractional value does not satisfy an integer schema, so it should be rejected (return undefined) rather than rounded toward zero. Consider checking that parsed is already an integer before returning it for type === "integer".
if (property.type === "number" || property.type === "integer") {
const parsed = Number(value);
if (!Number.isFinite(parsed)) {
return undefined;
}
- return property.type === "integer" ? Math.trunc(parsed) : parsed;
+ if (property.type === "integer" && !Number.isInteger(parsed)) {
+ return undefined;
+ }
+ return parsed;
}🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/acp/AcpElicitation.ts around lines 165-171:
For an `integer` property, `coerceAnswer` accepts fractional input like `1.9` and applies `Math.trunc`, silently truncating it to `1` in the response. A fractional value does not satisfy an integer schema, so it should be rejected (return `undefined`) rather than rounded toward zero. Consider checking that `parsed` is already an integer before returning it for `type === "integer"`.
| return []; | ||
| } | ||
| const schema = formRequestedSchema(request); | ||
| const header = truncateHeader(trimmed(request.message) ?? schema.title ?? "Input requested"); |
There was a problem hiding this comment.
🟡 Medium acp/AcpElicitation.ts:96
extractElicitationQuestions returns options: [] for free-form string/number properties, but the mobile parseUserInputQuestions rejects every question with an empty options array. A Kiro elicitation containing only free-form fields is dropped entirely on mobile, leaving the agent paused with no visible prompt; mixed forms silently omit those fields as well. Consider providing a placeholder option (e.g. a free-text entry) so the question survives validation on mobile.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/acp/AcpElicitation.ts around line 96:
`extractElicitationQuestions` returns `options: []` for free-form string/number properties, but the mobile `parseUserInputQuestions` rejects every question with an empty `options` array. A Kiro elicitation containing only free-form fields is dropped entirely on mobile, leaving the agent paused with no visible prompt; mixed forms silently omit those fields as well. Consider providing a placeholder option (e.g. a free-text entry) so the question survives validation on mobile.
- Idle turn watchdog (shared by Kiro/Grok/Cursor): when a prompt is in flight but the agent produces no ACP activity for longer than T3CODE_ACP_TURN_IDLE_TIMEOUT_MS (default 240s, 0 disables), surface a runtime error and cancel the turn instead of spinning forever. Fixes the Kiro hang where a shell tool never returned a result. - Empty-completion notice: when a turn completes with no assistant content and no tool activity (e.g. Grok ending the turn after a 402 usage-limit inference failure), emit a runtime.warning so the user sees why instead of a silent stop. - Track per-turn output + last activity on the session context. - Tests + a T3_ACP_EMIT_EMPTY_COMPLETION mock-agent flag.
| * spinning forever. Generous by default so genuinely long-running tools are not | ||
| * killed; override with `T3CODE_ACP_TURN_IDLE_TIMEOUT_MS` (0 disables). | ||
| */ | ||
| const DEFAULT_TURN_IDLE_TIMEOUT_MS = 240_000; |
There was a problem hiding this comment.
🟠 High Layers/GrokAdapter.ts:91
The turn watchdog cancels legitimate user-interaction waits after 240 seconds. When a prompt is blocked on user approval or elicitation, there may be no ACP activity while the user decides, but the watchdog still fires, interrupts the turn, and rejects the later response as stale. The idle timeout does not exclude or pause for nonempty pendingApprovals/pendingUserInputs, so any user interaction lasting longer than DEFAULT_TURN_IDLE_TIMEOUT_MS is wrongly terminated. Consider pausing or resetting the idle watchdog while user-interaction callbacks are pending, or excluding those states from the watchdog check.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/GrokAdapter.ts around line 91:
The turn watchdog cancels legitimate user-interaction waits after 240 seconds. When a prompt is blocked on user approval or elicitation, there may be no ACP activity while the user decides, but the watchdog still fires, interrupts the turn, and rejects the later response as stale. The idle timeout does not exclude or pause for nonempty `pendingApprovals`/`pendingUserInputs`, so any user interaction lasting longer than `DEFAULT_TURN_IDLE_TIMEOUT_MS` is wrongly terminated. Consider pausing or resetting the idle watchdog while user-interaction callbacks are pending, or excluding those states from the watchdog check.
|
Hey, for now we're not adding new providers now. Might revisit this after #2829 has been released |
|
Cool. |
2 participants
What Changed
authenticateoptional for agents such as Kiro that authenticate outside the protocol (kiro-cli login/ env).Why
Kiro CLI exposes a standards-compliant JSON-RPC ACP server through
kiro-cli acp. Wiring it through the existing provider driver SPI keeps T3 Code’s provider-instance and orchestration behavior consistent while avoiding another forked ACP session implementation.UI Changes
Settings — Kiro provider
Kiro appears under Settings → Providers with display name, accent color, environment variables, binary path, and discovered models.
Model picker — Kiro models
Kiro is available in the provider rail; its models show in the composer model search/picker.
Validation
vp check(0 errors)vp run typecheckKiroAdapter,KiroProvider,KiroAcpSupport)Checklist
Note
Add Kiro CLI as an ACP provider with full session, status, and text generation support
KiroDriver,KiroAdapter, andKiroAcpSupportlayer that connects to the Kiro CLI via thekiro-cli acpcommand, following the same ACP lifecycle used by the Grok provider.KiroSettingsto the contracts layer (enabled, binaryPath, customModels) and registerskiroin provider maps, driver registry, and the web settings/provider picker UI.checkKiroProviderStatusprobes the CLI with--version, discovers available models via ACP, and reportserror/warning/readystatus with version advisory enrichment.makeGrokAdapteris generalized to accept injectedprovider,providerDisplayName,makeAcpRuntime, andresolveModelIdhooks, enabling Kiro to reuse the shared adapter and text generation logic without duplication.makeGrokAdapter: if no ACP activity occurs within the configured timeout, aruntime.errorevent is emitted and the active turn is cancelled.user-input.requested/user-input.resolvedevents; non-form modes are immediately declined.optionalfield onUserInputQuestion) no longer block submission on web and mobile; unanswered optional questions are omitted from answers.Macroscope summarized 983df76.