Skip to content

Commit 24dd509

Browse files
Merge pull request #5 from hrishikeshmane/kiro-acp
Kiro acp
2 parents 128f8f0 + a140597 commit 24dd509

9 files changed

Lines changed: 672 additions & 21 deletions

File tree

PATCH.md

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ Kiro as a first-class ACP provider, layered on top of upstream's shared ACP infr
123123
- Dynamic slash commands from `_kiro.dev/commands/available` notifications
124124
- Context window usage from `_kiro.dev/metadata` notifications
125125
- Agent selection is persisted per-thread in the composer draft store
126+
- Subagent crews fan out into collapsible Work-log groups: `_kiro.dev/subagent/list_update` roster transitions become `task.started` / `task.completed` envelopes keyed by the crew's ACP sessionId; per-tool activity inside each crew is pulsed as `task.progress` rows (one per distinct `toolCallId`) with labels formatted as `"{title}: {detail}"` (e.g. `Read file: src/foo.ts`, `Ran command: bun test`). Subagent ContentDelta / AssistantItem / PlanUpdated / ModeChanged events are dropped from the main thread — matching Claude Code's SDK behavior of hiding subagent internals behind task notifications.
126127

127128
### Server-only additions (new files)
128129

@@ -164,7 +165,8 @@ Every shared-file edit is a _pure addition_ (new case in a union, new entry in a
164165
| `provider/Layers/ProviderAdapterRegistry.ts` | Register `KiroAdapter` |
165166
| `provider/providerStatusCache.ts` | Add `"kiro"` to `PROVIDER_CACHE_IDS` |
166167
| `provider/makeManagedServerProvider.ts` | Add `patchSnapshot` (additive, does not replace `enrichSnapshot`) |
167-
| `provider/acp/AcpSessionRuntime.ts` | `authMethodId` made optional (Kiro uses OIDC, skips authenticate) |
168+
| `provider/acp/AcpSessionRuntime.ts` | `authMethodId` optional (Kiro uses OIDC); thread `sessionId` through `AssistantSegmentState` + re-emitted `ToolCallUpdated` so subagent events can be filtered out of the main thread |
169+
| `provider/acp/AcpRuntimeModel.ts` | `AcpParsedSessionEvent` union gains `sessionId` on every variant so downstream consumers can tell main-session vs subagent events apart |
168170
| `git/Services/TextGeneration.ts` | Handle kiro provider kind |
169171

170172
**Web** (`apps/web/src/`):
@@ -247,7 +249,8 @@ Upstream's `makeManagedServerProvider` exposes `enrichSnapshot` for static provi
247249
| `apps/server/src/server.ts` | RuntimeServicesLive layer chain |
248250
| `apps/server/src/provider/Layers/ProviderRegistry.ts` | Provider registration list |
249251
| `apps/server/src/provider/providerStatusCache.ts` | `PROVIDER_CACHE_IDS` array |
250-
| `apps/server/src/provider/acp/AcpSessionRuntime.ts` | Optional `authMethodId` |
252+
| `apps/server/src/provider/acp/AcpSessionRuntime.ts` | Optional `authMethodId`; `sessionId` plumbing for subagent filtering |
253+
| `apps/server/src/provider/acp/AcpRuntimeModel.ts` | `sessionId` on `AcpParsedSessionEvent` variants |
251254
| `apps/server/src/provider/makeManagedServerProvider.ts` | Added `patchSnapshot` |
252255
| `apps/web/src/composerDraftStore.ts` | Three provider-kind lists |
253256
| `apps/web/src/components/settings/SettingsPanels.tsx` | Provider panel registration |
@@ -311,6 +314,23 @@ Fork runs in lockstep with upstream (currently Effect v4 beta.45+). If you see t
311314

312315
- Refactor to a single `PROVIDER_KINDS` const tuple exported from contracts to eliminate the three-list hazard permanently (also makes `normalizeProviderModelOptionsWithCapabilities` exhaustiveness-check at compile time).
313316

317+
## Session Reflections (2026-04-20 — round 3: subagent grouping)
318+
319+
### What broke and what fixed it
320+
321+
**Bug: Kiro subagent crews flooded the main chat — one flat stream of "Read file" / "Ran command" / assistant-message rows per subagent, no grouping.**
322+
323+
- Kiro's ACP transport multiplexes `session/update` for the main session *and* every spawned subagent crew over one channel, tagged with `sessionId`. Upstream's `AcpRuntimeModel.parseSessionUpdateEvent` dropped that `sessionId` before reaching the adapter, so everything looked like main-session activity.
324+
- Fix landed in three passes:
325+
1. Thread `sessionId` through `AcpParsedSessionEvent` and `AcpSessionRuntime`, track a roster of in-flight subagents by their ACP sessionId, and translate `_kiro.dev/subagent/list_update` transitions into `task.started` / `task.completed` envelopes. Subagent session/update events are dropped on the main thread.
326+
2. Dropping *everything* from subagents made the Work-log look stuck — the group sat silent until the crew terminated. Emit one `task.progress` per distinct subagent `toolCallId` (tracked per-subagent in `seenToolCallIds`) so the Work-log shows live per-tool activity inside the collapsible group, matching Claude Code's native SDK behavior.
327+
3. Work-log rows initially rendered only the generic category ("Ran command", "Read file"). Kiro's typed tool-call presentation puts the action in `title` and the payload in `detail`; combine them as `"{title}: {detail}"` via a new `formatSubagentToolLabel` helper so rows show the actual command / path / query. 8 unit tests cover the helper.
328+
329+
### Lessons worth keeping
330+
331+
1. **Multiplexed channels need identity on every event.** Any time a transport fans in multiple logical streams, the parser must preserve the stream identity all the way to the consumer. Dropping it at the parser layer is irreversible downstream.
332+
2. **"Don't route" is not the same as "don't show".** The first pass filtered subagent events out of main-thread routing entirely; the fix was to keep a single summarized breadcrumb (task.progress per tool call) so the user sees progress without being drowned in subagent internals.
333+
314334
## Session Reflections (2026-04-20 — round 2)
315335

316336
### What broke and what fixed it

apps/server/scripts/acp-mock-agent.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,15 @@ const program = Effect.gen(function* () {
248248
),
249249
);
250250

251+
yield* agent.handleSetSessionModel((request) =>
252+
Effect.gen(function* () {
253+
if (typeof request.modelId === "string") {
254+
currentModelId = request.modelId;
255+
}
256+
return {};
257+
}),
258+
);
259+
251260
yield* agent.handleSetSessionConfigOption((request) =>
252261
Effect.gen(function* () {
253262
if (exitOnSetConfigOption) {

apps/server/scripts/kiro-mock-agent.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
/**
33
* Mock ACP agent that imitates kiro-cli ACP protocol for integration tests.
44
*
5-
* Responds to: initialize, session/new, session/prompt, session/cancel.
5+
* Responds to: initialize, session/new, session/prompt, session/cancel,
6+
* session/set_model, session/set_config_option.
67
* Emits: session/update (agent_message_chunk), _kiro.dev/metadata extension.
78
*/
89
import * as Effect from "effect/Effect";
@@ -14,6 +15,22 @@ import * as EffectAcpAgent from "effect-acp/agent";
1415

1516
const sessionId = "kiro-mock-session-1";
1617

18+
/** Available models — returned in session responses. */
19+
const availableModels = ["auto", "claude-sonnet-4-20250514", "claude-opus-4-20250918"];
20+
let currentModel = "auto";
21+
22+
/** Build the model config option structure used in ACP responses. */
23+
function makeModelConfigOption(model: string) {
24+
return {
25+
id: "model",
26+
name: "Model",
27+
type: "select" as const,
28+
category: "model" as const,
29+
currentValue: model,
30+
options: availableModels.map((m) => ({ value: m, name: m })),
31+
};
32+
}
33+
1734
const program = Effect.gen(function* () {
1835
const agent = yield* EffectAcpAgent.AcpAgent;
1936

@@ -33,13 +50,32 @@ const program = Effect.gen(function* () {
3350
yield* agent.handleCreateSession(() =>
3451
Effect.succeed({
3552
sessionId,
53+
configOptions: [makeModelConfigOption(currentModel)],
3654
}),
3755
);
3856

3957
yield* agent.handleLoadSession(() => Effect.succeed({}));
4058

4159
yield* agent.handleCancel(() => Effect.void);
4260

61+
yield* agent.handleSetSessionModel((request) =>
62+
Effect.gen(function* () {
63+
if (typeof request.modelId === "string") {
64+
currentModel = request.modelId;
65+
}
66+
return {};
67+
}),
68+
);
69+
70+
yield* agent.handleSetSessionConfigOption((request) =>
71+
Effect.gen(function* () {
72+
if (request.configId === "model" && "value" in request && typeof request.value === "string") {
73+
currentModel = request.value;
74+
}
75+
return { configOptions: [makeModelConfigOption(currentModel)] };
76+
}),
77+
);
78+
4379
yield* agent.handlePrompt((request) =>
4480
Effect.gen(function* () {
4581
const requestedSessionId = String(request.sessionId ?? sessionId);

apps/server/src/provider/Layers/KiroAdapter.integration.test.ts

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,4 +341,141 @@ describe("KiroAdapterLive integration", () => {
341341
yield* adapter.stopSession(threadId);
342342
}).pipe(Effect.scoped, Effect.provide(adapterLayer)),
343343
);
344+
345+
it.effect(
346+
"switches model in-session without restarting the process",
347+
() =>
348+
Effect.gen(function* () {
349+
const adapter = yield* KiroAdapter;
350+
const threadId = ThreadId.make("kiro-int-model-switch-1");
351+
352+
yield* adapter.startSession({
353+
threadId,
354+
provider: "kiro",
355+
cwd: process.cwd(),
356+
runtimeMode: "full-access",
357+
modelSelection: { provider: "kiro", model: "auto" },
358+
});
359+
360+
const spawnCountAfterStart = capturedArgs.length;
361+
362+
// First turn with default model
363+
yield* adapter.sendTurn({
364+
threadId,
365+
input: "first turn",
366+
attachments: [],
367+
});
368+
369+
// Second turn with different model — should NOT respawn
370+
yield* adapter.sendTurn({
371+
threadId,
372+
input: "second turn after model switch",
373+
attachments: [],
374+
modelSelection: {
375+
provider: "kiro",
376+
model: "claude-sonnet-4-20250514",
377+
},
378+
});
379+
380+
// Session should NOT have been restarted — only one spawn
381+
expect(capturedArgs.length).toBe(spawnCountAfterStart);
382+
383+
yield* adapter.stopSession(threadId);
384+
}).pipe(Effect.scoped, Effect.provide(adapterLayer)),
385+
);
386+
387+
it.effect(
388+
"updates session.model immediately after in-session model switch",
389+
() =>
390+
Effect.gen(function* () {
391+
const adapter = yield* KiroAdapter;
392+
const threadId = ThreadId.make("kiro-int-model-state-1");
393+
394+
const session = yield* adapter.startSession({
395+
threadId,
396+
provider: "kiro",
397+
cwd: process.cwd(),
398+
runtimeMode: "full-access",
399+
modelSelection: { provider: "kiro", model: "auto" },
400+
});
401+
402+
expect(session.model).toBe("auto");
403+
404+
// Collect turn.started events to verify model is correct
405+
const eventsFiber = yield* adapter.streamEvents.pipe(
406+
Stream.filter(
407+
(event) => event.type === "turn.started" || event.type === "turn.completed",
408+
),
409+
Stream.take(2), // turn.started + turn.completed for the model-switch turn
410+
Stream.runCollect,
411+
Effect.forkChild,
412+
);
413+
414+
// Send turn with new model
415+
const turn = yield* adapter.sendTurn({
416+
threadId,
417+
input: "switch model turn",
418+
attachments: [],
419+
modelSelection: {
420+
provider: "kiro",
421+
model: "claude-sonnet-4-20250514",
422+
},
423+
});
424+
425+
expect(turn.threadId).toBe(threadId);
426+
427+
const events = yield* Fiber.join(eventsFiber);
428+
const turnStarted = events.find((e) => e.type === "turn.started");
429+
expect(turnStarted).toBeDefined();
430+
if (turnStarted?.type === "turn.started") {
431+
expect(turnStarted.payload.model).toBe("claude-sonnet-4-20250514");
432+
}
433+
434+
// Verify session state reflects the new model
435+
const sessions = yield* adapter.listSessions();
436+
const currentSession = sessions.find((s) => s.threadId === threadId);
437+
expect(currentSession?.model).toBe("claude-sonnet-4-20250514");
438+
439+
yield* adapter.stopSession(threadId);
440+
}).pipe(Effect.scoped, Effect.provide(adapterLayer)),
441+
);
442+
443+
it.effect(
444+
"does not call setModel when model is unchanged between turns",
445+
() =>
446+
Effect.gen(function* () {
447+
const adapter = yield* KiroAdapter;
448+
const threadId = ThreadId.make("kiro-int-model-unchanged-1");
449+
450+
yield* adapter.startSession({
451+
threadId,
452+
provider: "kiro",
453+
cwd: process.cwd(),
454+
runtimeMode: "full-access",
455+
modelSelection: { provider: "kiro", model: "auto" },
456+
});
457+
458+
// Two turns with the same model — should not trigger setModel
459+
yield* adapter.sendTurn({
460+
threadId,
461+
input: "first turn",
462+
attachments: [],
463+
modelSelection: { provider: "kiro", model: "auto" },
464+
});
465+
466+
yield* adapter.sendTurn({
467+
threadId,
468+
input: "second turn same model",
469+
attachments: [],
470+
modelSelection: { provider: "kiro", model: "auto" },
471+
});
472+
473+
// Both turns should succeed without issues (no setModel called)
474+
const sessions = yield* adapter.listSessions();
475+
const currentSession = sessions.find((s) => s.threadId === threadId);
476+
expect(currentSession?.model).toBe("auto");
477+
478+
yield* adapter.stopSession(threadId);
479+
}).pipe(Effect.scoped, Effect.provide(adapterLayer)),
480+
);
344481
});

0 commit comments

Comments
 (0)