Skip to content

Commit 3057152

Browse files
aaditagrawalaaditagrawal
committed
fix: address all PR review feedback from CodeRabbit
- Fix claude-agent-sdk.d.ts query signature (prompt accepts string, options optional, Query return type) - Forward numTurns in rollbackThread for Kilo, OpenCode, GeminiCli, Amp adapters - Guard ClaudeCodeAdapter.startSession against duplicate threadIds - Reject unsupported attachments in ClaudeCodeAdapter instead of silently dropping - Replace ClaudeCodeAdapter rollback with explicit not-supported error - Add CopilotAdapter layer cleanup finalizer, fix turn bookkeeping, emit teardown events - Convert CopilotAdapter attachment validation to use Effect.forEach - Read workspace from OpenCode resume cursor, guard against unsupported attachments - Fix diff-only assistant turn suppression in ChatView - Preserve isCustom flag in ModelOptionEntry, add staleTime to model queries - Add WCAG contrast checking for accent colors with fallback - Derive contrast-safe terminal colors for ThreadTerminalDrawer - Drop invalid provider accent overrides instead of normalizing to default - Fail fast on duplicate provider adapter registration - Fix README headings, add Kilo to supported agents - Remove duplicate ProviderRuntimeIngestion test - Make Gemini CLI live tests explicitly opt-in - Fix hardcoded runtimeMode in GeminiCli listSessions - Add stable activeAssistantItemId for Amp content deltas - Kill child process on Kilo server start timeout - Derive repo URL from git remote in sync script - Update plan docs for multi-provider scope
1 parent 0b25fcc commit 3057152

17 files changed

Lines changed: 438 additions & 250 deletions

‎.plans/17-claude-code.md‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# Plan: Claude Code Integration (Orchestration Architecture)
22

3+
> **Note -- Multi-provider scope:** This plan was originally written for the Claude Code adapter, but the patterns described here (adapter shape, canonical runtime mapping, resume cursor ownership, provider registry wiring, and orchestration integration) apply equally to the full multi-provider adapter infrastructure now implemented in this PR: **ClaudeCodeAdapter**, **CopilotAdapter**, **OpenCodeAdapter**, **GeminiCliAdapter**, **KiloAdapter**, and **AmpAdapter**. Where the text says "Claude adapter", read it as the reference implementation; every other adapter follows the same contract surface.
4+
35
## Why this plan was rewritten
46

57
The previous plan targeted a pre-orchestration architecture (`ProviderManager`, provider-native WS event methods, and direct provider UI wiring). The current app now routes everything through:
@@ -113,6 +115,14 @@ Baseline adapter options to support from day one:
113115
10. `hooks`
114116
11. `env` and `additionalDirectories` (if needed for sandbox/workspace parity)
115117

118+
### 2.1.b Credential management and resource limits
119+
120+
Each provider manages its own authentication externally:
121+
122+
1. **Environment variables and CLI auth** -- Credentials are resolved via provider-native mechanisms (e.g. `ANTHROPIC_API_KEY` for Claude, `OPENAI_API_KEY` for Codex, `gh auth` for Copilot). The adapter layer never stores or brokers credentials directly; it relies on the underlying CLI/SDK picking them up from the environment.
123+
2. **Per-provider rate limiting** -- Each server manager (`codexAppServerManager`, `claudeCodeServerManager`, etc.) is responsible for honoring its provider's rate limits. Adapters should surface rate-limit errors as `ProviderAdapterProcessError` so orchestration can report them cleanly.
124+
3. **Concurrent session limits** -- The number of simultaneous provider sessions is bounded by system resources (open processes, file descriptors, memory). `ProviderSessionDirectory` tracks active sessions but does not enforce hard caps; operators should monitor resource usage when running multiple providers concurrently.
125+
116126
### 2.2 Claude runtime bridge
117127

118128
Implement a Claude runtime bridge (either directly in adapter layer or via dedicated manager file) that wraps Agent SDK query lifecycle.
@@ -416,6 +426,15 @@ Add/extend integration tests around:
416426

417427
These should validate real orchestration flows, not just adapter behavior.
418428

429+
### 6.5 Multi-provider test scenarios
430+
431+
Cover cross-provider interactions that single-adapter tests miss:
432+
433+
1. **Provider switching mid-conversation** -- Start a thread on Codex, then switch to Claude (or any other provider) for the next turn. Verify the old session is stopped, bindings are updated, and the new adapter receives the correct `providerOptions`.
434+
2. **Concurrent active sessions** -- Run sessions on two or more different providers simultaneously. Verify events from each session are routed to the correct orchestration thread without cross-contamination.
435+
3. **Resume cursor isolation** -- Persist resume cursors from two different providers, then attempt to resume each. Confirm that one provider's cursor cannot accidentally be used to resume another provider's session (adapter parse should reject mismatched cursors).
436+
4. **Provider health monitoring** -- Simulate a provider becoming unavailable (process crash, binary missing). Verify `listProviderStatuses()` reflects the degraded state and that orchestration surfaces a clear error to the client rather than hanging.
437+
419438
---
420439

421440
## Phase 7: Rollout order

‎README.md‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
# T3 Code
1+
# T3 Code
22

3-
T3 Code is a minimal web GUI for coding agents. It supports Codex, Claude Code, Cursor, Copilot, Gemini CLI, Amp, and OpenCode.
3+
T3 Code is a minimal web GUI for coding agents. It supports Codex, Claude Code, Cursor, Copilot, Gemini CLI, Amp, Kilo, and OpenCode.
44

5-
## Getting started
5+
## Getting started
66

7-
### CLI
7+
### CLI
88

99
> [!WARNING]
1010
> You need at least one supported coding agent installed and authorized. See the supported agents list below.
@@ -13,26 +13,27 @@
1313
npx t3
1414
```
1515

16-
### Desktop app
16+
### Desktop app
1717

1818
You can also just install the desktop app. It's cooler. Install it from the [Releases page](https://github.com/pingdotgg/t3code/releases).
1919

20-
## Supported agents
20+
## Supported agents
2121

2222
- [Codex CLI](https://github.com/openai/codex) (requires v0.37.0 or later)
2323
- [Claude Code](https://github.com/anthropics/claude-code)
2424
- [Cursor](https://cursor.sh)
2525
- [Copilot](https://github.com/features/copilot)
2626
- [Gemini CLI](https://github.com/google-gemini/gemini-cli)
2727
- [Amp](https://ampcode.com)
28+
- [Kilo](https://kilo.dev)
2829
- [OpenCode](https://opencode.ai)
2930

30-
## Notes
31+
## Notes
3132

3233
- This project is very early in development. Expect bugs.
3334
- We are not accepting contributions yet.
3435
- Maintaining a custom fork or alpha branch? See [docs/custom-alpha-workflow.md](docs/custom-alpha-workflow.md).
3536

36-
## Need help?
37+
## Need help?
3738

3839
Join the [Discord](https://discord.gg/jn4EGJjrvv) if you need support.

‎apps/server/src/ampServerManager.ts‎

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ interface AmpSession {
7474
runtimeMode: string;
7575
status: "ready" | "running" | "closed";
7676
activeTurnId: TurnId | undefined;
77+
/** Stable itemId reused across content.delta events within a single assistant message. */
78+
activeAssistantItemId: RuntimeItemId | undefined;
7779
/** Maps parent_tool_use_id → RuntimeTaskId for tracking subagent tasks. */
7880
readonly subagentTasks: Map<string, string>;
7981
readonly createdAt: string;
@@ -207,6 +209,7 @@ export class AmpServerManager extends EventEmitter<{
207209
runtimeMode: input.runtimeMode,
208210
status: "ready",
209211
activeTurnId: undefined,
212+
activeAssistantItemId: undefined,
210213
subagentTasks: new Map(),
211214
createdAt: now,
212215
updatedAt: now,
@@ -539,6 +542,7 @@ export class AmpServerManager extends EventEmitter<{
539542
});
540543
session.status = "ready";
541544
session.activeTurnId = undefined;
545+
session.activeAssistantItemId = undefined;
542546
session.updatedAt = new Date().toISOString();
543547
}
544548
}
@@ -549,27 +553,47 @@ export class AmpServerManager extends EventEmitter<{
549553
block: AmpContentBlock,
550554
): void {
551555
switch (block.type) {
552-
case "text":
553-
this.emitEvent(threadId, session.activeTurnId, {
554-
type: "content.delta",
555-
payload: {
556-
streamKind: "assistant_text",
557-
delta: block.text,
556+
case "text": {
557+
if (!session.activeAssistantItemId) {
558+
session.activeAssistantItemId = RuntimeItemId.makeUnsafe(randomUUID());
559+
}
560+
this.emitEvent(
561+
threadId,
562+
session.activeTurnId,
563+
{
564+
type: "content.delta",
565+
payload: {
566+
streamKind: "assistant_text",
567+
delta: block.text,
568+
},
558569
},
559-
});
570+
session.activeAssistantItemId,
571+
);
560572
break;
573+
}
561574

562-
case "thinking":
563-
this.emitEvent(threadId, session.activeTurnId, {
564-
type: "content.delta",
565-
payload: {
566-
streamKind: "reasoning_text",
567-
delta: block.thinking,
575+
case "thinking": {
576+
if (!session.activeAssistantItemId) {
577+
session.activeAssistantItemId = RuntimeItemId.makeUnsafe(randomUUID());
578+
}
579+
this.emitEvent(
580+
threadId,
581+
session.activeTurnId,
582+
{
583+
type: "content.delta",
584+
payload: {
585+
streamKind: "reasoning_text",
586+
delta: block.thinking,
587+
},
568588
},
569-
});
589+
session.activeAssistantItemId,
590+
);
570591
break;
592+
}
571593

572594
case "tool_use": {
595+
// A tool use starts a new assistant message segment — clear the active item.
596+
session.activeAssistantItemId = undefined;
573597
const itemType = classifyToolName(block.name);
574598
const itemId = RuntimeItemId.makeUnsafe(block.id);
575599
this.emitEvent(
@@ -714,6 +738,7 @@ export class AmpServerManager extends EventEmitter<{
714738

715739
session.status = "ready";
716740
session.activeTurnId = undefined;
741+
session.activeAssistantItemId = undefined;
717742
session.updatedAt = new Date().toISOString();
718743
}
719744

‎apps/server/src/geminiCliServerManager.test.ts‎

Lines changed: 59 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,9 @@ describe("GeminiCliServerManager", () => {
9494
}
9595
});
9696

97+
// TODO: Strengthen this test by mocking child_process.spawn and asserting it
98+
// is NOT called during startSession. Currently we only verify session state,
99+
// which doesn't prove that no process was spawned.
97100
it("does not spawn a process on startSession (lazy per-turn spawning)", async () => {
98101
const manager = new GeminiCliServerManager();
99102
try {
@@ -559,56 +562,59 @@ const hasGemini = await (async () => {
559562
}
560563
})();
561564

562-
describe.skipIf(!hasGemini)("GeminiCliServerManager live integration", () => {
563-
it(
564-
"sends a prompt and receives streaming events ending with turn.completed",
565-
async () => {
566-
const manager = new GeminiCliServerManager();
567-
const events: ProviderRuntimeEvent[] = [];
568-
manager.on("event", (event) => events.push(event));
569-
570-
try {
571-
await manager.startSession({
572-
threadId: asThreadId("live-thread"),
573-
provider: "geminiCli",
574-
runtimeMode: "full-access",
575-
model: "gemini-2.5-flash",
576-
});
577-
578-
const result = await manager.sendTurn({
579-
threadId: asThreadId("live-thread"),
580-
input: "Reply with exactly the word PONG",
581-
});
582-
583-
expect(result.threadId).toBe("live-thread");
584-
expect(result.turnId).toBeTruthy();
585-
586-
// Wait for the turn to complete.
587-
await vi.waitFor(
588-
() => {
589-
const completed = events.find((e) => e.type === "turn.completed");
590-
expect(completed).toBeDefined();
591-
},
592-
{ timeout: 30_000, interval: 500 },
593-
);
594-
595-
// Should have received content deltas.
596-
const deltas = events.filter((e) => e.type === "content.delta");
597-
expect(deltas.length).toBeGreaterThan(0);
598-
599-
// The text should contain "PONG" somewhere.
600-
const fullText = deltas
601-
.map((e) => (e.payload as { delta: string }).delta)
602-
.join("");
603-
expect(fullText.toLowerCase()).toContain("pong");
604-
605-
// Turn should be completed successfully.
606-
const completed = events.find((e) => e.type === "turn.completed");
607-
expect((completed?.payload as { state: string }).state).toBe("completed");
608-
} finally {
609-
manager.stopAll();
610-
}
611-
},
612-
60_000,
613-
);
614-
});
565+
describe.skipIf(!hasGemini || process.env.RUN_GEMINI_LIVE_TESTS !== "1")(
566+
"GeminiCliServerManager live integration",
567+
() => {
568+
it(
569+
"sends a prompt and receives streaming events ending with turn.completed",
570+
async () => {
571+
const manager = new GeminiCliServerManager();
572+
const events: ProviderRuntimeEvent[] = [];
573+
manager.on("event", (event) => events.push(event));
574+
575+
try {
576+
await manager.startSession({
577+
threadId: asThreadId("live-thread"),
578+
provider: "geminiCli",
579+
runtimeMode: "full-access",
580+
model: "gemini-2.5-flash",
581+
});
582+
583+
const result = await manager.sendTurn({
584+
threadId: asThreadId("live-thread"),
585+
input: "Reply with exactly the word PONG",
586+
});
587+
588+
expect(result.threadId).toBe("live-thread");
589+
expect(result.turnId).toBeTruthy();
590+
591+
// Wait for the turn to complete.
592+
await vi.waitFor(
593+
() => {
594+
const completed = events.find((e) => e.type === "turn.completed");
595+
expect(completed).toBeDefined();
596+
},
597+
{ timeout: 30_000, interval: 500 },
598+
);
599+
600+
// Should have received content deltas.
601+
const deltas = events.filter((e) => e.type === "content.delta");
602+
expect(deltas.length).toBeGreaterThan(0);
603+
604+
// The text should contain "PONG" somewhere.
605+
const fullText = deltas
606+
.map((e) => (e.payload as { delta: string }).delta)
607+
.join("");
608+
expect(fullText.toLowerCase()).toContain("pong");
609+
610+
// Turn should be completed successfully.
611+
const completed = events.find((e) => e.type === "turn.completed");
612+
expect((completed?.payload as { state: string }).state).toBe("completed");
613+
} finally {
614+
manager.stopAll();
615+
}
616+
},
617+
60_000,
618+
);
619+
},
620+
);

‎apps/server/src/geminiCliServerManager.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ export class GeminiCliServerManager extends EventEmitter<{
388388
sessions.push({
389389
provider: PROVIDER,
390390
status: session.status === "closed" ? "closed" : "ready",
391-
runtimeMode: "full-access",
391+
runtimeMode: session.runtimeMode as ProviderSession["runtimeMode"],
392392
threadId: session.threadId,
393393
cwd: session.cwd,
394394
model: session.model,

‎apps/server/src/kiloServerManager.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1201,6 +1201,11 @@ export class KiloServerManager extends EventEmitter<KiloManagerEvents> {
12011201

12021202
const startedBaseUrl = await new Promise<string>((resolve, reject) => {
12031203
const timeout = setTimeout(() => {
1204+
try {
1205+
child.kill();
1206+
} catch {
1207+
// Process may already be dead.
1208+
}
12041209
reject(
12051210
new Error(
12061211
`Timed out waiting for Kilo server to start after ${SERVER_START_TIMEOUT_MS}ms`,

‎apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.ts‎

Lines changed: 0 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -506,60 +506,6 @@ describe("ProviderRuntimeIngestion", () => {
506506
);
507507
});
508508

509-
it("accepts claude turn lifecycle when seeded thread id is a synthetic placeholder", async () => {
510-
const harness = await createHarness();
511-
const seededAt = new Date().toISOString();
512-
513-
await Effect.runPromise(
514-
harness.engine.dispatch({
515-
type: "thread.session.set",
516-
commandId: CommandId.makeUnsafe("cmd-session-seed-claude-placeholder"),
517-
threadId: ThreadId.makeUnsafe("thread-1"),
518-
session: {
519-
threadId: ThreadId.makeUnsafe("thread-1"),
520-
status: "ready",
521-
providerName: "claudeCode",
522-
runtimeMode: "approval-required",
523-
activeTurnId: null,
524-
updatedAt: seededAt,
525-
lastError: null,
526-
},
527-
createdAt: seededAt,
528-
}),
529-
);
530-
531-
harness.emit({
532-
type: "turn.started",
533-
eventId: asEventId("evt-turn-started-claude-placeholder"),
534-
provider: "claudeCode",
535-
createdAt: new Date().toISOString(),
536-
threadId: asThreadId("thread-1"),
537-
turnId: asTurnId("turn-claude-placeholder"),
538-
});
539-
540-
await waitForThread(
541-
harness.engine,
542-
(thread) =>
543-
thread.session?.status === "running" &&
544-
thread.session?.activeTurnId === "turn-claude-placeholder",
545-
);
546-
547-
harness.emit({
548-
type: "turn.completed",
549-
eventId: asEventId("evt-turn-completed-claude-placeholder"),
550-
provider: "claudeCode",
551-
createdAt: new Date().toISOString(),
552-
threadId: asThreadId("thread-1"),
553-
turnId: asTurnId("turn-claude-placeholder"),
554-
status: "completed",
555-
});
556-
557-
await waitForThread(
558-
harness.engine,
559-
(thread) => thread.session?.status === "ready" && thread.session?.activeTurnId === null,
560-
);
561-
});
562-
563509
it("ignores non-active turn completion when runtime omits thread id", async () => {
564510
const harness = await createHarness();
565511
const now = new Date().toISOString();

0 commit comments

Comments
 (0)