From 1281fe4ca7bea4065ace2af7d1e3ec0f9303cb51 Mon Sep 17 00:00:00 2001 From: Nathan Font-Fife Date: Thu, 9 Jul 2026 05:03:53 -0600 Subject: [PATCH 01/11] feat(rpc): server-request answering, notification subscription, outbound notify --- src/main/runtime/capabilities/jsonRpc.test.ts | 12 ++++- src/main/runtime/capabilities/jsonRpc.ts | 46 ++++++++++++++++++- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/main/runtime/capabilities/jsonRpc.test.ts b/src/main/runtime/capabilities/jsonRpc.test.ts index f67897e..4ba0773 100644 --- a/src/main/runtime/capabilities/jsonRpc.test.ts +++ b/src/main/runtime/capabilities/jsonRpc.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest' -import { parseRpcLine, LineDecoder } from './jsonRpc' +import { parseRpcLine, LineDecoder, classifyRpcMessage } from './jsonRpc' describe('parseRpcLine', () => { it('parses codex-style responses that omit the jsonrpc field', () => { @@ -30,3 +30,13 @@ describe('LineDecoder', () => { expect(d.push(Buffer.from('1}\n{"id":2}\n', 'utf8'))).toEqual(['{"id":1}', '{"id":2}']) }) }) + +describe('classifyRpcMessage', () => { + it('separates responses, server requests, and notifications', () => { + expect(classifyRpcMessage({ id: 3, result: { stopReason: 'end_turn' } })).toBe('response') + // Real captured frame: copilot's permission request arrived with id 0 — a server REQUEST has method+id. + expect(classifyRpcMessage({ id: 0, method: 'session/request_permission' })).toBe('server-request') + expect(classifyRpcMessage({ method: 'session/update' })).toBe('notification') + expect(classifyRpcMessage({ id: 1, error: { code: -32601 } })).toBe('response') + }) +}) diff --git a/src/main/runtime/capabilities/jsonRpc.ts b/src/main/runtime/capabilities/jsonRpc.ts index bcf99a8..15d98a6 100644 --- a/src/main/runtime/capabilities/jsonRpc.ts +++ b/src/main/runtime/capabilities/jsonRpc.ts @@ -9,6 +9,7 @@ export interface RpcMessage { result?: unknown error?: { code?: number; message?: string } method?: string + params?: unknown } /** Pure + exported for testing: chunk-boundary-safe newline splitter (multi-byte UTF-8 can split across chunks). */ @@ -42,24 +43,38 @@ export function parseRpcLine(line: string): RpcMessage | null { return m } +/** Pure + exported for testing: incoming message kind. A server-initiated message carries `method`; + * with an id it's a request we must answer, without one a notification. Anything else is a response. */ +export function classifyRpcMessage(m: RpcMessage): 'response' | 'server-request' | 'notification' { + if (m.method !== undefined) return m.id !== undefined ? 'server-request' : 'notification' + return 'response' +} + export class JsonRpcClient { private child: ChildProcess private lines = new LineDecoder() private nextId = 1 private pending = new Map void; reject: (e: Error) => void }>() + private notificationHandlers = new Map void>() + private requestHandlers = new Map Promise | unknown>() constructor(command: string, args: string[]) { this.child = spawn(command, args, { stdio: ['pipe', 'pipe', 'ignore'] }) this.child.stdout?.on('data', (chunk: Buffer) => { for (const line of this.lines.push(chunk)) { const msg = parseRpcLine(line) - if (msg?.id !== undefined && this.pending.has(msg.id)) { + if (!msg) continue + const kind = classifyRpcMessage(msg) + if (kind === 'response' && msg.id !== undefined && this.pending.has(msg.id)) { const p = this.pending.get(msg.id)! this.pending.delete(msg.id) if (msg.error) p.reject(new Error(msg.error.message ?? `rpc error ${msg.error.code}`)) else p.resolve(msg.result) + } else if (kind === 'server-request') { + this.answer(msg) + } else if (kind === 'notification') { + this.notificationHandlers.get(msg.method!)?.(msg.params) } - // notifications (method, no id) are ignored — discovery only awaits responses } }) this.child.on('error', (err) => this.failAll(err)) @@ -71,6 +86,33 @@ export class JsonRpcClient { this.pending.clear() } + private answer(msg: RpcMessage): void { + const handler = this.requestHandlers.get(msg.method!) + const write = (body: object): void => { + this.child.stdin?.write(JSON.stringify({ jsonrpc: '2.0', id: msg.id, ...body }) + '\n') + } + if (!handler) { + write({ error: { code: -32601, message: `unhandled: ${msg.method}` } }) + return + } + Promise.resolve() + .then(() => handler(msg.params)) + .then((result) => write({ result })) + .catch((e: Error) => write({ error: { code: -32000, message: e.message ?? 'handler error' } })) + } + + onNotification(method: string, handler: (params: unknown) => void): void { + this.notificationHandlers.set(method, handler) + } + + onRequest(method: string, handler: (params: unknown) => Promise | unknown): void { + this.requestHandlers.set(method, handler) + } + + notify(method: string, params?: unknown): void { + this.child.stdin?.write(JSON.stringify({ jsonrpc: '2.0', method, params: params ?? {} }) + '\n') + } + request(method: string, params?: unknown, timeoutMs = 5000): Promise { const id = this.nextId++ const payload = JSON.stringify({ jsonrpc: '2.0', id, method, params: params ?? {} }) From 5988c3f442d14bd62fe2003f0a059254311ee9f7 Mon Sep 17 00:00:00 2001 From: Nathan Font-Fife Date: Thu, 9 Jul 2026 05:07:36 -0600 Subject: [PATCH 02/11] feat(events): tool/permission canonical events + pure ACP mappers from captured frames Co-Authored-By: Claude Fable 5 --- src/main/runtime/acp/mapAcp.test.ts | 46 ++++++++++++++++++ src/main/runtime/acp/mapAcp.ts | 74 +++++++++++++++++++++++++++++ src/shared/runtime.ts | 12 ++++- 3 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 src/main/runtime/acp/mapAcp.test.ts create mode 100644 src/main/runtime/acp/mapAcp.ts diff --git a/src/main/runtime/acp/mapAcp.test.ts b/src/main/runtime/acp/mapAcp.test.ts new file mode 100644 index 0000000..33023bc --- /dev/null +++ b/src/main/runtime/acp/mapAcp.test.ts @@ -0,0 +1,46 @@ +import { describe, it, expect } from 'vitest' +import { mapAcpUpdate, mapPermissionRequest } from './mapAcp' + +const TOOL_CALL = { sessionUpdate: 'tool_call', toolCallId: 'call_MHx', title: 'Run echo nac-probe-ok', kind: 'execute', status: 'pending', rawInput: { command: 'echo nac-probe-ok', description: 'Run echo nac-probe-ok', mode: 'sync' } } +const TOOL_DONE = { sessionUpdate: 'tool_call_update', toolCallId: 'call_MHx', status: 'completed', content: [{ type: 'content', content: { type: 'text', text: 'nac-probe-ok\n' } }], rawOutput: { content: 'nac-probe-ok\n' } } +const CHUNK = { sessionUpdate: 'agent_message_chunk', content: { type: 'text', text: 'It printed ' } } +const PERM = { sessionId: 's1', toolCall: { toolCallId: 'call_MHx', title: 'Run echo nac-probe-ok', kind: 'execute', status: 'pending', rawInput: { command: 'echo nac-probe-ok' } }, options: [{ optionId: 'allow_once', kind: 'allow_once', name: 'Allow once' }, { optionId: 'allow_always', kind: 'allow_always', name: 'Always allow' }, { optionId: 'reject_once', kind: 'reject_once', name: 'Deny' }] } + +describe('mapAcpUpdate', () => { + it('maps tool_call to a pending tool.updated carrying the command as detail', () => { + expect(mapAcpUpdate('r', TOOL_CALL)).toEqual([{ type: 'tool.updated', runId: 'r', toolCallId: 'call_MHx', title: 'Run echo nac-probe-ok', kind: 'execute', status: 'pending', detail: 'echo nac-probe-ok' }]) + }) + it('maps a completed tool_call_update carrying output text as detail', () => { + const [e] = mapAcpUpdate('r', TOOL_DONE) + expect(e).toMatchObject({ type: 'tool.updated', toolCallId: 'call_MHx', status: 'completed' }) + expect((e as { detail?: string }).detail).toContain('nac-probe-ok') + }) + it('maps agent_message_chunk to content.delta and ignores unknown/junk updates', () => { + expect(mapAcpUpdate('r', CHUNK)).toEqual([{ type: 'content.delta', runId: 'r', streamKind: 'assistant_text', text: 'It printed ' }]) + expect(mapAcpUpdate('r', { sessionUpdate: 'plan' })).toEqual([]) + expect(mapAcpUpdate('r', null)).toEqual([]) + }) + it('preserves a tool_call_update without status as a running upsert', () => { + const [e] = mapAcpUpdate('r', { sessionUpdate: 'tool_call_update', toolCallId: 'call_MHx', content: [{ type: 'content', content: { type: 'text', text: 'partial' } }] }) + expect(e).toMatchObject({ type: 'tool.updated', status: 'running', detail: 'partial' }) + }) +}) + +describe('mapPermissionRequest', () => { + it('maps the captured request with normalized option kinds', () => { + const e = mapPermissionRequest('r', 'req1', PERM) + expect(e).toEqual({ + type: 'permission.requested', runId: 'r', requestId: 'req1', title: 'Run echo nac-probe-ok', + detail: 'echo nac-probe-ok', + options: [ + { id: 'allow_once', label: 'Allow once', kind: 'allow' }, + { id: 'allow_always', label: 'Always allow', kind: 'allow_always' }, + { id: 'reject_once', label: 'Deny', kind: 'deny' } + ] + }) + }) + it('returns null for junk', () => { + expect(mapPermissionRequest('r', 'x', null)).toBeNull() + expect(mapPermissionRequest('r', 'x', { options: [] })).toBeNull() + }) +}) diff --git a/src/main/runtime/acp/mapAcp.ts b/src/main/runtime/acp/mapAcp.ts new file mode 100644 index 0000000..690d5c8 --- /dev/null +++ b/src/main/runtime/acp/mapAcp.ts @@ -0,0 +1,74 @@ +import type { AgentEvent, PermissionOption } from '../../../shared/runtime' + +// Pure mappers from copilot ACP frames (live-captured 2026-07-09, docs/research/ +// acp-prompt-frames-copilot-1.0.69.txt) to canonical AgentEvents. + +interface AcpContentEntry { + content?: { text?: string } +} +interface AcpUpdate { + sessionUpdate?: string + toolCallId?: string + title?: string + kind?: string + status?: string + rawInput?: { command?: string } + rawOutput?: { content?: string } + content?: AcpContentEntry[] | { text?: string } +} + +const TOOL_STATUSES = new Set(['pending', 'running', 'completed', 'failed']) + +function contentText(u: AcpUpdate): string | undefined { + if (Array.isArray(u.content)) { + const texts = u.content.map((c) => c?.content?.text).filter((t): t is string => Boolean(t)) + return texts.length ? texts.join('') : undefined + } + return undefined +} + +/** One session/update frame → 0..n AgentEvents. Unknown update kinds are ignored. */ +export function mapAcpUpdate(runId: string, update: unknown): AgentEvent[] { + const u = update as AcpUpdate | null + if (!u || typeof u !== 'object') return [] + switch (u.sessionUpdate) { + case 'agent_message_chunk': { + const text = (u.content as { text?: string } | undefined)?.text + return text ? [{ type: 'content.delta', runId, streamKind: 'assistant_text', text }] : [] + } + case 'tool_call': + case 'tool_call_update': { + if (!u.toolCallId) return [] + const status = (u.status && TOOL_STATUSES.has(u.status) ? u.status : u.sessionUpdate === 'tool_call' ? 'pending' : 'running') as 'pending' | 'running' | 'completed' | 'failed' + const detail = u.rawOutput?.content ?? contentText(u) ?? u.rawInput?.command + return [{ type: 'tool.updated', runId, toolCallId: u.toolCallId, title: u.title ?? u.toolCallId, kind: u.kind, status, ...(detail ? { detail } : {}) }] + } + default: + return [] + } +} + +const OPTION_KINDS: Record = { + allow_once: 'allow', + allow_always: 'allow_always', + reject_once: 'deny', + reject_always: 'deny' +} + +/** session/request_permission params → a permission.requested event (null for junk/no options). */ +export function mapPermissionRequest(runId: string, requestId: string, params: unknown): Extract | null { + const p = params as { toolCall?: { title?: string; rawInput?: { command?: string } }; options?: { optionId?: string; kind?: string; name?: string }[] } | null + if (!p || typeof p !== 'object' || !Array.isArray(p.options) || p.options.length === 0) return null + const options: PermissionOption[] = [] + for (const o of p.options) { + if (!o?.optionId) continue + options.push({ id: o.optionId, label: o.name ?? o.optionId, kind: OPTION_KINDS[o.kind ?? ''] ?? 'deny' }) + } + if (options.length === 0) return null + return { + type: 'permission.requested', runId, requestId, + title: p.toolCall?.title ?? 'Permission request', + ...(p.toolCall?.rawInput?.command ? { detail: p.toolCall.rawInput.command } : {}), + options + } +} diff --git a/src/shared/runtime.ts b/src/shared/runtime.ts index ff7a5b8..94841da 100644 --- a/src/shared/runtime.ts +++ b/src/shared/runtime.ts @@ -27,18 +27,28 @@ export interface TurnUsage { costUsd?: number } +export interface PermissionOption { + id: string + label: string + kind: 'allow' | 'allow_always' | 'deny' +} + export type AgentEvent = | { type: 'run.started'; runId: string; sessionId?: string } | { type: 'content.delta'; runId: string; streamKind: 'assistant_text' | 'reasoning'; text: string } | { type: 'run.completed'; runId: string; stopReason: 'end_turn' | 'error' | 'canceled'; usage?: TurnUsage } | { type: 'run.errored'; runId: string; message: string } + | { type: 'tool.updated'; runId: string; toolCallId: string; title: string; kind?: string; status: 'pending' | 'running' | 'completed' | 'failed'; detail?: string } + | { type: 'permission.requested'; runId: string; requestId: string; title: string; detail?: string; options: PermissionOption[] } + | { type: 'permission.resolved'; runId: string; requestId: string; optionId: string } // IPC channel names shared by main and preload. export const RUN_CHANNELS = { start: 'run:start', cancel: 'run:cancel', event: 'run:event', - summarize: 'run:summarize' + summarize: 'run:summarize', + respondPermission: 'run:respondPermission' } as const export const STATE_CHANNELS = { From 567a7bac9b312bd56b3fca7b03dbbb279b394507 Mon Sep 17 00:00:00 2001 From: Nathan Font-Fife Date: Thu, 9 Jul 2026 05:11:21 -0600 Subject: [PATCH 03/11] feat(acp): persistent AcpSession + SessionManager behind the TransportSession seam --- src/main/runtime/acp/acpSession.test.ts | 15 +++ src/main/runtime/acp/acpSession.ts | 143 ++++++++++++++++++++++++ src/main/runtime/acp/sessionManager.ts | 84 ++++++++++++++ 3 files changed, 242 insertions(+) create mode 100644 src/main/runtime/acp/acpSession.test.ts create mode 100644 src/main/runtime/acp/acpSession.ts create mode 100644 src/main/runtime/acp/sessionManager.ts diff --git a/src/main/runtime/acp/acpSession.test.ts b/src/main/runtime/acp/acpSession.test.ts new file mode 100644 index 0000000..d2b30e7 --- /dev/null +++ b/src/main/runtime/acp/acpSession.test.ts @@ -0,0 +1,15 @@ +import { describe, it, expect } from 'vitest' +import { pickAutoApprove } from './acpSession' + +describe('pickAutoApprove', () => { + it('picks the first allow-kind option', () => { + expect(pickAutoApprove([ + { id: 'reject_once', label: 'Deny', kind: 'deny' }, + { id: 'allow_once', label: 'Allow once', kind: 'allow' }, + { id: 'allow_always', label: 'Always', kind: 'allow_always' } + ])?.id).toBe('allow_once') + }) + it('returns undefined when no allow option exists', () => { + expect(pickAutoApprove([{ id: 'reject_once', label: 'Deny', kind: 'deny' }])).toBeUndefined() + }) +}) diff --git a/src/main/runtime/acp/acpSession.ts b/src/main/runtime/acp/acpSession.ts new file mode 100644 index 0000000..ab30c3a --- /dev/null +++ b/src/main/runtime/acp/acpSession.ts @@ -0,0 +1,143 @@ +import { JsonRpcClient } from '../capabilities/jsonRpc' +import type { AgentEvent, PermissionOption } from '../../../shared/runtime' +import { mapAcpUpdate, mapPermissionRequest } from './mapAcp' + +export const PROMPT_TIMEOUT_MS = 1_800_000 // 30 min — cancellation, not timeout, is the stop lever +const HANDSHAKE_TIMEOUT_MS = 10_000 + +export interface TransportSession { + prompt(runId: string, text: string): void + respondPermission(requestId: string, optionId: string): void + cancel(): void + dispose(): void +} + +/** Pure + exported for testing: YOLO auto-approval picks the first allow-ish option. */ +export function pickAutoApprove(options: PermissionOption[]): PermissionOption | undefined { + return options.find((o) => o.kind === 'allow' || o.kind === 'allow_always') +} + +interface PendingPermission { + resolve: (optionId: string) => void +} + +export class AcpSession implements TransportSession { + private client: JsonRpcClient + private sessionId: string | null = null + private currentRunId: string | null = null + private replaying = false // suppress session/load history replay + private permissionSeq = 0 + private pendingPermissions = new Map() + private onEvent: (e: AgentEvent) => void + private yolo: boolean + + constructor(onEvent: (e: AgentEvent) => void, yolo: boolean) { + this.onEvent = onEvent + this.yolo = yolo + this.client = new JsonRpcClient('copilot', ['--acp']) + this.client.onNotification('session/update', (params) => { + if (this.replaying || !this.currentRunId) return + const update = (params as { update?: unknown } | null)?.update + for (const e of mapAcpUpdate(this.currentRunId, update)) this.onEvent(e) + }) + this.client.onRequest('session/request_permission', (params) => this.handlePermission(params)) + } + + setYolo(y: boolean): void { + this.yolo = y + } + + /** Resolves the ACP handshake; throws on failure so the caller can fall back. */ + async connect(cwd: string | undefined, existingSessionId: string | undefined): Promise { + await this.client.request('initialize', { + protocolVersion: 1, + clientCapabilities: { fs: { readTextFile: false, writeTextFile: false } } + }, HANDSHAKE_TIMEOUT_MS) + if (existingSessionId) { + try { + this.replaying = true // session/load re-emits history as session/update — never re-append it + await this.client.request('session/load', { sessionId: existingSessionId, cwd: cwd ?? process.cwd(), mcpServers: [] }, HANDSHAKE_TIMEOUT_MS) + this.sessionId = existingSessionId + return existingSessionId + } catch { + // fall through to a fresh session (caller seeds it with the replay prompt on the next send) + } finally { + this.replaying = false + } + } + const res = (await this.client.request('session/new', { cwd: cwd ?? process.cwd(), mcpServers: [] }, HANDSHAKE_TIMEOUT_MS)) as { sessionId?: string } + if (!res?.sessionId) throw new Error('acp: session/new returned no sessionId') + this.sessionId = res.sessionId + return res.sessionId + } + + get loadedSessionId(): string | null { + return this.sessionId + } + + private handlePermission(params: unknown): Promise { + const runId = this.currentRunId ?? 'unknown' + const requestId = `perm_${++this.permissionSeq}` + const event = mapPermissionRequest(runId, requestId, params) + if (!event) return Promise.resolve({ outcome: { outcome: 'cancelled' } }) // zero options: never hang + if (this.yolo) { + const auto = pickAutoApprove(event.options) + if (auto) return Promise.resolve({ outcome: { outcome: 'selected', optionId: auto.id } }) + } + this.onEvent(event) + return new Promise((resolve) => { + this.pendingPermissions.set(requestId, { + resolve: (optionId) => { + this.onEvent({ type: 'permission.resolved', runId, requestId, optionId }) + resolve({ outcome: { outcome: 'selected', optionId } }) + } + }) + }) + } + + /** True while a turn is in flight — the idle reaper must never dispose a busy session. */ + get busy(): boolean { + return this.currentRunId !== null + } + + prompt(runId: string, text: string): void { + if (!this.sessionId) throw new Error('acp: no session') + this.currentRunId = runId + this.onEvent({ type: 'run.started', runId, sessionId: this.sessionId }) + this.client + .request('session/prompt', { sessionId: this.sessionId, prompt: [{ type: 'text', text }] }, PROMPT_TIMEOUT_MS) + .then((res) => { + const stop = (res as { stopReason?: string } | null)?.stopReason + this.onEvent({ type: 'run.completed', runId, stopReason: stop === 'cancelled' ? 'canceled' : 'end_turn' }) + }) + .catch((e: Error) => this.onEvent({ type: 'run.errored', runId, message: e.message })) + .finally(() => { + this.currentRunId = null + this.expirePermissions() + }) + } + + respondPermission(requestId: string, optionId: string): void { + const p = this.pendingPermissions.get(requestId) + if (!p) return + this.pendingPermissions.delete(requestId) + p.resolve(optionId) + } + + private expirePermissions(): void { + // A turn ended with cards still open (error/cancel): answer the protocol with a deny-equivalent. + for (const [requestId, p] of this.pendingPermissions) { + this.pendingPermissions.delete(requestId) + p.resolve('reject_once') + } + } + + cancel(): void { + if (this.sessionId) this.client.notify('session/cancel', { sessionId: this.sessionId }) + } + + dispose(): void { + this.expirePermissions() + this.client.close() + } +} diff --git a/src/main/runtime/acp/sessionManager.ts b/src/main/runtime/acp/sessionManager.ts new file mode 100644 index 0000000..7c48bc1 --- /dev/null +++ b/src/main/runtime/acp/sessionManager.ts @@ -0,0 +1,84 @@ +import type { AgentEvent } from '../../../shared/runtime' +import { AcpSession } from './acpSession' + +// One live ACP session per chat. Sessions die on provider switch (a new prompt for the same chat +// with a different transport never reaches here), app quit, or idle timeout. + +export const IDLE_MS = 15 * 60_000 + +interface Entry { + session: AcpSession + idleTimer: ReturnType | null +} + +const byChat = new Map() +const runToChat = new Map() + +function touch(chatId: string): void { + const e = byChat.get(chatId) + if (!e) return + if (e.idleTimer) clearTimeout(e.idleTimer) + e.idleTimer = setTimeout(() => disposeChat(chatId), IDLE_MS) +} + +function disposeChat(chatId: string): void { + const e = byChat.get(chatId) + if (!e) return + if (e.session.busy) { + // A turn can run up to 30 min (PROMPT_TIMEOUT_MS) — re-arm instead of killing mid-turn. + touch(chatId) + return + } + byChat.delete(chatId) + if (e.idleTimer) clearTimeout(e.idleTimer) + e.session.dispose() +} + +/** Try the interactive path. Resolves { ok: false } when ACP is unavailable — caller falls back. */ +export async function promptViaAcp(opts: { + chatId: string + runId: string + prompt: string + cwd?: string + yolo?: boolean + sessionId?: string + onEvent: (e: AgentEvent) => void +}): Promise<{ ok: boolean }> { + let entry = byChat.get(opts.chatId) + if (!entry) { + const session = new AcpSession(opts.onEvent, opts.yolo === true) + try { + await session.connect(opts.cwd, opts.sessionId) + } catch { + session.dispose() + return { ok: false } + } + entry = { session, idleTimer: null } + byChat.set(opts.chatId, entry) + } + entry.session.setYolo(opts.yolo === true) + runToChat.set(opts.runId, opts.chatId) + entry.session.prompt(opts.runId, opts.prompt) + touch(opts.chatId) + return { ok: true } +} + +export function respondPermission(runId: string, requestId: string, optionId: string): void { + const chatId = runToChat.get(runId) + if (!chatId) return + byChat.get(chatId)?.session.respondPermission(requestId, optionId) + touch(chatId) +} + +export function cancelRun(runId: string): boolean { + const chatId = runToChat.get(runId) + if (!chatId) return false + const e = byChat.get(chatId) + if (!e) return false + e.session.cancel() + return true +} + +export function disposeAll(): void { + for (const chatId of [...byChat.keys()]) disposeChat(chatId) +} From 26d151d43cca96a0c02eb96eca07992a67bde18f Mon Sep 17 00:00:00 2001 From: Nathan Font-Fife Date: Thu, 9 Jul 2026 05:15:48 -0600 Subject: [PATCH 04/11] fix(acp): bound runToChat lifetime + force disposal on app quit - Delete runToChat entries when terminal run events occur (run.completed/run.errored) - Add force flag to disposeChat to allow killing busy sessions on app quit - disposeAll now force-disposes all sessions (process is exiting) - Idle reaper path unchanged (re-arms instead of killing mid-turn) Co-Authored-By: Claude Fable 5 --- src/main/runtime/acp/sessionManager.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/main/runtime/acp/sessionManager.ts b/src/main/runtime/acp/sessionManager.ts index 7c48bc1..74d952f 100644 --- a/src/main/runtime/acp/sessionManager.ts +++ b/src/main/runtime/acp/sessionManager.ts @@ -21,11 +21,11 @@ function touch(chatId: string): void { e.idleTimer = setTimeout(() => disposeChat(chatId), IDLE_MS) } -function disposeChat(chatId: string): void { +function disposeChat(chatId: string, force = false): void { const e = byChat.get(chatId) if (!e) return - if (e.session.busy) { - // A turn can run up to 30 min (PROMPT_TIMEOUT_MS) — re-arm instead of killing mid-turn. + if (!force && e.session.busy) { + // Idle reaper path: a turn can run up to 30 min — re-arm instead of killing mid-turn. touch(chatId) return } @@ -46,7 +46,10 @@ export async function promptViaAcp(opts: { }): Promise<{ ok: boolean }> { let entry = byChat.get(opts.chatId) if (!entry) { - const session = new AcpSession(opts.onEvent, opts.yolo === true) + const session = new AcpSession((e) => { + if (e.type === 'run.completed' || e.type === 'run.errored') runToChat.delete(e.runId) + opts.onEvent(e) + }, opts.yolo === true) try { await session.connect(opts.cwd, opts.sessionId) } catch { @@ -80,5 +83,6 @@ export function cancelRun(runId: string): boolean { } export function disposeAll(): void { - for (const chatId of [...byChat.keys()]) disposeChat(chatId) + // App quit: force — a busy session must still be torn down (the process is exiting). + for (const chatId of [...byChat.keys()]) disposeChat(chatId, true) } From abf56f3a4fe36a73ddabf240a5e9723042853605 Mon Sep 17 00:00:00 2001 From: Nathan Font-Fife Date: Thu, 9 Jul 2026 05:20:34 -0600 Subject: [PATCH 05/11] feat(runtime): interactive copilot routing, permission IPC, tool/permission turn state --- src/main/runtime/ipc.ts | 24 ++++++++--- src/preload/index.ts | 2 + src/renderer/src/store/persist.test.ts | 9 ++++ src/renderer/src/store/persist.ts | 9 +++- src/renderer/src/store/runtime.ts | 10 +++++ src/renderer/src/store/store.test.ts | 20 +++++++++ src/renderer/src/store/store.ts | 58 +++++++++++++++++++++++++- src/shared/runtime.ts | 1 + 8 files changed, 126 insertions(+), 7 deletions(-) diff --git a/src/main/runtime/ipc.ts b/src/main/runtime/ipc.ts index 4fbc360..5d594d6 100644 --- a/src/main/runtime/ipc.ts +++ b/src/main/runtime/ipc.ts @@ -12,6 +12,7 @@ import { probeProviders } from './registry' import { getCapabilities, invalidateCapabilities } from './capabilities' import { classifyModelRejection } from './capabilities/ledger' import { recordOutcome } from './capabilities/ledgerStore' +import { promptViaAcp, respondPermission as acpRespondPermission, cancelRun as acpCancelRun, disposeAll as acpDisposeAll } from './acp/sessionManager' const runs = new Map() let counter = 0 @@ -52,6 +53,8 @@ function stubHarnessPath(): string { * (ACP / app-server / SDK) slots in behind the same AgentEvent stream later. */ export function registerRuntimeIpc(getWindow: () => BrowserWindow | null): void { + app.on('will-quit', () => acpDisposeAll()) + ipcMain.handle(RUN_CHANNELS.start, (_e, req: RunRequest): { runId: string } => { const runId = `run_${++counter}` const send = (event: AgentEvent): void => getWindow()?.webContents.send(RUN_CHANNELS.event, event) @@ -66,17 +69,25 @@ export function registerRuntimeIpc(getWindow: () => BrowserWindow | null): void } if (event.type === 'run.completed' || event.type === 'run.errored') runs.delete(runId) } + if (req.provider === 'copilot') { + // Interactive-first: persistent ACP session; on { ok: false } fall back to the one-shot path. + void promptViaAcp({ chatId: req.chatId ?? runId, runId, prompt: req.prompt, cwd: req.cwd, yolo: req.yolo, sessionId: req.sessionId, onEvent: handler }).then(({ ok }) => { + if (!ok) { + handler({ type: 'content.delta', runId, streamKind: 'assistant_text', text: '\n[interactive session unavailable — ran headless]\n' }) + runs.set(runId, startCopilotRun(runId, { prompt: req.prompt, cwd: req.cwd, yolo: req.yolo, sessionId: req.sessionId, effort: req.effort, model: req.model }, handler)) + } + }) + return { runId } + } // Real Claude adapter for provider 'claude'; the NDJSON stub for the rest (until those adapters land). const run = req.provider === 'claude' ? startClaudeRun(runId, { prompt: req.prompt, sessionId: req.sessionId, cwd: req.cwd, yolo: req.yolo, model: req.model, effort: req.effort, fast: req.fast }, handler) : req.provider === 'codex' ? startCodexRun(runId, { prompt: req.prompt, cwd: req.cwd, yolo: req.yolo, sessionId: req.sessionId, effort: req.effort, model: req.model }, handler) - : req.provider === 'copilot' - ? startCopilotRun(runId, { prompt: req.prompt, cwd: req.cwd, yolo: req.yolo, sessionId: req.sessionId, effort: req.effort, model: req.model }, handler) - : req.provider === 'opencode' - ? startOpenCodeRun(runId, { prompt: req.prompt, model: req.model, cwd: req.cwd, yolo: req.yolo, sessionId: req.sessionId, variant: req.effort }, handler) - : startHarnessRun( + : req.provider === 'opencode' + ? startOpenCodeRun(runId, { prompt: req.prompt, model: req.model, cwd: req.cwd, yolo: req.yolo, sessionId: req.sessionId, variant: req.effort }, handler) + : startHarnessRun( runId, { prompt: req.prompt, @@ -92,10 +103,13 @@ export function registerRuntimeIpc(getWindow: () => BrowserWindow | null): void }) ipcMain.handle(RUN_CHANNELS.cancel, (_e, runId: string): void => { + if (acpCancelRun(runId)) return // live interactive session: protocol-level stop runs.get(runId)?.cancel() runs.delete(runId) }) + ipcMain.handle(RUN_CHANNELS.respondPermission, (_e, runId: string, requestId: string, optionId: string) => acpRespondPermission(runId, requestId, optionId)) + ipcMain.handle(RUN_CHANNELS.summarize, async (_e, req: SummarizeRequest): Promise<{ summary: string }> => { const summary = await runOnce(req.provider, `${SUMMARIZE_INSTRUCTION}\n\n${req.text}`, req.model) return { summary } diff --git a/src/preload/index.ts b/src/preload/index.ts index 46dc678..63aa0dc 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -10,6 +10,8 @@ const api = { start: (req: RunRequest): Promise<{ runId: string }> => ipcRenderer.invoke(RUN_CHANNELS.start, req), cancel: (runId: string): Promise => ipcRenderer.invoke(RUN_CHANNELS.cancel, runId), summarize: (req: SummarizeRequest): Promise<{ summary: string }> => ipcRenderer.invoke(RUN_CHANNELS.summarize, req), + respondPermission: (runId: string, requestId: string, optionId: string): Promise => + ipcRenderer.invoke(RUN_CHANNELS.respondPermission, runId, requestId, optionId), // Subscribe to streamed AgentEvents; returns an unsubscribe function. onEvent: (cb: (event: AgentEvent) => void): (() => void) => { const listener = (_e: unknown, event: AgentEvent): void => cb(event) diff --git a/src/renderer/src/store/persist.test.ts b/src/renderer/src/store/persist.test.ts index 0cc6e7e..cb9ccdb 100644 --- a/src/renderer/src/store/persist.test.ts +++ b/src/renderer/src/store/persist.test.ts @@ -21,3 +21,12 @@ describe('normalizeChat — thinking → effort migration', () => { expect(normalizeChat({} as never, 'c_none').fast).toBe(false) }) }) + +describe('normalizeChat — never restore live-looking tool/permission state', () => { + it('hydration never restores live-looking tool/permission state', () => { + const raw = { fast: false, messages: [{ id: 'a', role: 'assistant', text: 'x', tools: [{ toolCallId: 't', title: 'T', status: 'running' }], permissions: [{ requestId: 'p', title: 'P', options: [] }] }] } as never + const c = normalizeChat(raw, 'c_live') + expect(c.messages[0].tools?.[0].status).toBe('failed') + expect(c.messages[0].permissions?.[0].resolvedOptionId).toBe('stale') + }) +}) diff --git a/src/renderer/src/store/persist.ts b/src/renderer/src/store/persist.ts index dd75786..3a2038e 100644 --- a/src/renderer/src/store/persist.ts +++ b/src/renderer/src/store/persist.ts @@ -38,7 +38,14 @@ export function normalizeChat(c: Partial & { claudeSessionId?: string | nu contextK: c.contextK ?? 0, windowK: c.windowK ?? 200, branchedFrom: c.branchedFrom ?? null, - messages: Array.isArray(c.messages) ? c.messages : [], + messages: Array.isArray(c.messages) + ? c.messages.map((m) => ({ + ...m, + // never restore live-looking state (the `compacting` doctrine) + tools: m.tools?.map((t) => (t.status === 'pending' || t.status === 'running' ? { ...t, status: 'failed' as const } : t)), + permissions: m.permissions?.map((p) => (p.resolvedOptionId ? p : { ...p, resolvedOptionId: 'stale' })) + })) + : [], sessionId: c.sessionId ?? c.claudeSessionId ?? null, // migrate legacy claudeSessionId sessionProvider: c.sessionProvider ?? (c.claudeSessionId ? 'claude' : null), summary: c.summary ?? null, diff --git a/src/renderer/src/store/runtime.ts b/src/renderer/src/store/runtime.ts index cfcf262..e0270b5 100644 --- a/src/renderer/src/store/runtime.ts +++ b/src/renderer/src/store/runtime.ts @@ -31,6 +31,15 @@ export function initRuntime(): void { s.endTurn(chatId, event.message) delete runToChat[event.runId] break + case 'tool.updated': + s.upsertTool(chatId, { toolCallId: event.toolCallId, title: event.title, kind: event.kind, status: event.status, detail: event.detail }) + break + case 'permission.requested': + s.upsertPermission(chatId, { requestId: event.requestId, title: event.title, detail: event.detail, options: event.options }) + break + case 'permission.resolved': + s.resolvePermission(chatId, event.requestId, event.optionId) + break } }) } @@ -95,6 +104,7 @@ export async function sendMessage(text: string): Promise { const { runId } = await window.nac.runs.start({ prompt: useNative ? message : contextBlock + buildReplayPrompt(chat.summary, tail, message), provider: chat.provider, + chatId, sessionId: useNative ? chat.sessionId ?? undefined : undefined, cwd, yolo: chat.yolo, diff --git a/src/renderer/src/store/store.test.ts b/src/renderer/src/store/store.test.ts index 55c9df3..7636770 100644 --- a/src/renderer/src/store/store.test.ts +++ b/src/renderer/src/store/store.test.ts @@ -185,4 +185,24 @@ describe('app store — per-chat spine', () => { s.setModel(chat.provider, chat.model) expect(useApp.getState().chats[s.activeChatId].effort).toBe('low') // still valid → kept }) + + it('upsertTool merges by toolCallId on the streaming turn', () => { + const s = useApp.getState() + const id = s.activeChatId + s.pushTurn(id, { id: 'a1', role: 'assistant', text: '', streaming: true }) + s.upsertTool(id, { toolCallId: 't1', title: 'Run x', status: 'pending', detail: 'x' }) + s.upsertTool(id, { toolCallId: 't1', title: 'Run x', status: 'completed', detail: 'done' }) + const turn = useApp.getState().chats[id].messages.at(-1)! + expect(turn.tools).toEqual([{ toolCallId: 't1', title: 'Run x', status: 'completed', detail: 'done' }]) + }) + + it('permission cards resolve in place', () => { + const s = useApp.getState() + const id = s.activeChatId + s.pushTurn(id, { id: 'a2', role: 'assistant', text: '', streaming: true }) + s.upsertPermission(id, { requestId: 'p1', title: 'Run x', options: [{ id: 'allow_once', label: 'Allow once', kind: 'allow' }] }) + s.resolvePermission(id, 'p1', 'allow_once') + const turn = useApp.getState().chats[id].messages.at(-1)! + expect(turn.permissions?.[0].resolvedOptionId).toBe('allow_once') + }) }) diff --git a/src/renderer/src/store/store.ts b/src/renderer/src/store/store.ts index 975a4d3..72e682a 100644 --- a/src/renderer/src/store/store.ts +++ b/src/renderer/src/store/store.ts @@ -2,7 +2,7 @@ import { create } from 'zustand' import { CONFIGS_BY_ID } from '../data/configs' import { STATIC_CAPABILITIES, effortScaleFor, modelIdFor } from '../../../shared/capabilities' import type { ContextItem } from '../data/context' -import type { TurnUsage, ProviderCapabilities } from '../../../shared/runtime' +import type { TurnUsage, ProviderCapabilities, PermissionOption } from '../../../shared/runtime' // The per-chat state spine (FR-4.1): every chat owns its own provider/model/agent/attached/config/transcript. // Mutations target a specific chat — nothing is global. Switching chats is lossless (FR-4.2). @@ -20,6 +20,21 @@ export interface Workspace { defaults?: WorkspaceDefaults // new chats here inherit these (else fall back to active-chat inheritance — M0-4) } +export interface ToolRow { + toolCallId: string + title: string + kind?: string + status: 'pending' | 'running' | 'completed' | 'failed' + detail?: string +} +export interface PermissionCard { + requestId: string + title: string + detail?: string + options: PermissionOption[] + resolvedOptionId?: string +} + // A turn in the provider-neutral transcript (M0-8 source of truth — renders the UI and, later, powers replay). export interface Turn { id: string @@ -27,6 +42,8 @@ export interface Turn { text: string streaming?: boolean error?: boolean + tools?: ToolRow[] // render-only history — NEVER read by buildReplayPrompt + permissions?: PermissionCard[] } // Accumulated metering for a chat, keyed by provider (each provider reports in its own units). @@ -111,6 +128,9 @@ interface AppState { endTurn: (chatId: string, error?: string) => void setSession: (chatId: string, sessionId: string, provider: string) => void recordUsage: (chatId: string, provider: string, usage: TurnUsage) => void + upsertTool: (chatId: string, row: ToolRow) => void + upsertPermission: (chatId: string, card: PermissionCard) => void + resolvePermission: (chatId: string, requestId: string, optionId: string) => void // user-authored context library items (notes + files), persisted userItems: ContextItem[] addNote: (name: string, content: string) => void @@ -338,6 +358,42 @@ export const useApp = create()((set, get) => ({ } return { chats: { ...s.chats, [chatId]: { ...c, usage: { ...c.usage, [provider]: next } } } } }), + upsertTool: (chatId, row) => + set((s) => { + const c = s.chats[chatId] + if (!c) return {} + const messages = updateLast(c.messages, (t) => { + const tools = t.tools ? [...t.tools] : [] + const i = tools.findIndex((x) => x.toolCallId === row.toolCallId) + if (i >= 0) tools[i] = { ...tools[i], ...row } + else tools.push(row) + return { ...t, tools } + }) + return { chats: { ...s.chats, [chatId]: { ...c, messages } } } + }), + upsertPermission: (chatId, card) => + set((s) => { + const c = s.chats[chatId] + if (!c) return {} + const messages = updateLast(c.messages, (t) => { + const permissions = t.permissions ? [...t.permissions] : [] + const i = permissions.findIndex((x) => x.requestId === card.requestId) + if (i >= 0) permissions[i] = { ...permissions[i], ...card } + else permissions.push(card) + return { ...t, permissions } + }) + return { chats: { ...s.chats, [chatId]: { ...c, messages } } } + }), + resolvePermission: (chatId, requestId, optionId) => + set((s) => { + const c = s.chats[chatId] + if (!c) return {} + const messages = updateLast(c.messages, (t) => ({ + ...t, + permissions: (t.permissions ?? []).map((p) => (p.requestId === requestId ? { ...p, resolvedOptionId: optionId } : p)) + })) + return { chats: { ...s.chats, [chatId]: { ...c, messages } } } + }), addNote: (name, content) => set((s) => ({ userItems: [...s.userItems, { id: `u_${Date.now()}_${++chatSeq}`, type: 'instruction', name: name.trim() || 'note', description: content.trim().slice(0, 80), tokens: Math.ceil(content.length / 4), scope: 'workspace', source: 'user', tags: ['note'], content, user: true }] diff --git a/src/shared/runtime.ts b/src/shared/runtime.ts index 94841da..0cc1d92 100644 --- a/src/shared/runtime.ts +++ b/src/shared/runtime.ts @@ -4,6 +4,7 @@ export interface RunRequest { prompt: string provider?: string // harness driver id; selects the adapter (e.g. 'claude' → real, else stub) + chatId?: string // session-affinity key for persistent transports sessionId?: string // native session id to resume (e.g. Claude `--resume`) — same-provider fast-path (FR-4.2) cwd?: string // working directory for the harness = the chat's workspace folder (agents act on real code) yolo?: boolean // autonomy: on = full file/command access; off (default) = restricted per harness (M0-2) From bf7d96247988de06a1c04a4f6b280250cbb529a3 Mon Sep 17 00:00:00 2001 From: Nathan Font-Fife Date: Thu, 9 Jul 2026 05:25:08 -0600 Subject: [PATCH 06/11] fix(store): tool/permission reducers target the last assistant turn (late-event race) Co-Authored-By: Claude Fable 5 --- src/renderer/src/store/store.test.ts | 11 +++++++++++ src/renderer/src/store/store.ts | 19 ++++++++++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/renderer/src/store/store.test.ts b/src/renderer/src/store/store.test.ts index 7636770..87b51a3 100644 --- a/src/renderer/src/store/store.test.ts +++ b/src/renderer/src/store/store.test.ts @@ -205,4 +205,15 @@ describe('app store — per-chat spine', () => { const turn = useApp.getState().chats[id].messages.at(-1)! expect(turn.permissions?.[0].resolvedOptionId).toBe('allow_once') }) + + it('tool/permission events after a new user turn still target the last assistant turn', () => { + const s = useApp.getState() + const id = s.activeChatId + s.pushTurn(id, { id: 'a9', role: 'assistant', text: 'done', streaming: false }) + s.pushTurn(id, { id: 'u9', role: 'user', text: 'next question' }) + s.upsertTool(id, { toolCallId: 'late', title: 'Late tool', status: 'completed' }) + const msgs = useApp.getState().chats[id].messages + expect(msgs.at(-1)!.tools).toBeUndefined() // user turn untouched + expect(msgs.at(-2)!.tools?.[0].toolCallId).toBe('late') + }) }) diff --git a/src/renderer/src/store/store.ts b/src/renderer/src/store/store.ts index 72e682a..e9bdc1e 100644 --- a/src/renderer/src/store/store.ts +++ b/src/renderer/src/store/store.ts @@ -159,6 +159,19 @@ const updateLast = (msgs: Turn[], patch: (t: Turn) => Turn): Turn[] => { return copy } +// Tool/permission events may arrive after endTurn (expiry fires post-completion) or after the user's +// next message — target the last ASSISTANT turn, never whatever happens to be last. +const updateLastAssistant = (msgs: Turn[], patch: (t: Turn) => Turn): Turn[] => { + for (let i = msgs.length - 1; i >= 0; i--) { + if (msgs[i].role === 'assistant') { + const copy = msgs.slice() + copy[i] = patch(copy[i]) + return copy + } + } + return msgs +} + // Collision-proof chat id (Date.now() alone collides on rapid creates within the same ms). let chatSeq = 0 const nextChatId = (): string => `c_${Date.now()}_${++chatSeq}` @@ -362,7 +375,7 @@ export const useApp = create()((set, get) => ({ set((s) => { const c = s.chats[chatId] if (!c) return {} - const messages = updateLast(c.messages, (t) => { + const messages = updateLastAssistant(c.messages, (t) => { const tools = t.tools ? [...t.tools] : [] const i = tools.findIndex((x) => x.toolCallId === row.toolCallId) if (i >= 0) tools[i] = { ...tools[i], ...row } @@ -375,7 +388,7 @@ export const useApp = create()((set, get) => ({ set((s) => { const c = s.chats[chatId] if (!c) return {} - const messages = updateLast(c.messages, (t) => { + const messages = updateLastAssistant(c.messages, (t) => { const permissions = t.permissions ? [...t.permissions] : [] const i = permissions.findIndex((x) => x.requestId === card.requestId) if (i >= 0) permissions[i] = { ...permissions[i], ...card } @@ -388,7 +401,7 @@ export const useApp = create()((set, get) => ({ set((s) => { const c = s.chats[chatId] if (!c) return {} - const messages = updateLast(c.messages, (t) => ({ + const messages = updateLastAssistant(c.messages, (t) => ({ ...t, permissions: (t.permissions ?? []).map((p) => (p.requestId === requestId ? { ...p, resolvedOptionId: optionId } : p)) })) From 00b0a282f03e93078337171efb2b71f667966ce6 Mon Sep 17 00:00:00 2001 From: Nathan Font-Fife Date: Thu, 9 Jul 2026 05:29:39 -0600 Subject: [PATCH 07/11] feat(ui): permission cards, expandable tool rows, stop button --- src/renderer/src/components/ChatView.tsx | 43 +++++++++++++++---- .../src/components/PermissionCard.tsx | 34 +++++++++++++++ src/renderer/src/components/ToolRow.tsx | 25 +++++++++++ src/renderer/src/store/runtime.ts | 5 +++ 4 files changed, 98 insertions(+), 9 deletions(-) create mode 100644 src/renderer/src/components/PermissionCard.tsx create mode 100644 src/renderer/src/components/ToolRow.tsx diff --git a/src/renderer/src/components/ChatView.tsx b/src/renderer/src/components/ChatView.tsx index 92b63eb..d56adc6 100644 --- a/src/renderer/src/components/ChatView.tsx +++ b/src/renderer/src/components/ChatView.tsx @@ -1,8 +1,10 @@ import { useEffect, useRef, useState, type CSSProperties } from 'react' -import { useApp, selectActiveChat, contextPending } from '../store/store' -import { sendMessage, isStreaming } from '../store/runtime' +import { useApp, selectActiveChat, contextPending, type Turn } from '../store/store' +import { sendMessage, isStreaming, runIdForChat } from '../store/runtime' import { CONFIGURATIONS, CONFIGS_BY_ID, configTokens } from '../data/configs' import { effortScaleFor } from '../../../shared/capabilities' +import ToolRow from './ToolRow' +import PermissionCard from './PermissionCard' // Center pane: chat header · thread · composer. Send drives a real run (Claude adapter) or the stub; // streamed AgentEvents land in the chat's transcript via the run controller. @@ -25,6 +27,7 @@ export default function ChatView() { const messages = active.messages ?? [] // defensive: tolerate stale data missing the field const pending = contextPending(active) const bottomRef = useRef(null) + const runId = runIdForChat(active.id) // cards only appear on the streaming turn, so this is unambiguous const cwd = useApp((s) => s.workspaces.find((w) => w.id === active.workspaceId)?.path) ?? '' const [changed, setChanged] = useState(0) useEffect(() => { @@ -106,7 +109,7 @@ export default function ChatView() {

)} {messages.map((m) => ( - + ))}
@@ -163,11 +166,22 @@ export default function ChatView() { toggleYolo()} style={{ ...toolbarItem, color: active.yolo ? 'var(--warning)' : 'var(--muted)', fontWeight: active.yolo ? 600 : 400 }}> YOLO{active.yolo ? ' ●' : ''} + {streaming && ( + + )} + ))} + + + ) +} + +const card: CSSProperties = { margin: '6px 0', padding: '10px 12px', background: 'var(--card)', border: '1px solid var(--warning)', borderRadius: 8 } +const detail: CSSProperties = { margin: '4px 0 0', padding: '6px 10px', background: 'var(--card-3, var(--panel))', borderRadius: 6, fontSize: 11.5, color: 'var(--text-2)', whiteSpace: 'pre-wrap' } +const btn: CSSProperties = { background: 'var(--accent-tint-3)', color: 'var(--accent-light)', border: '1px solid var(--accent)', borderRadius: 6, padding: '4px 12px', fontSize: 12, cursor: 'pointer' } +const denyBtn: CSSProperties = { background: 'var(--card)', color: 'var(--muted)', border: '1px solid var(--line-2)' } +const resolvedLine: CSSProperties = { margin: '4px 0', fontSize: 11.5, color: 'var(--muted)' } diff --git a/src/renderer/src/components/ToolRow.tsx b/src/renderer/src/components/ToolRow.tsx new file mode 100644 index 0000000..54bf4f4 --- /dev/null +++ b/src/renderer/src/components/ToolRow.tsx @@ -0,0 +1,25 @@ +import { useState, type CSSProperties } from 'react' +import type { ToolRow as ToolRowData } from '../store/store' + +const GLYPH: Record = { pending: '·', running: '⟳', completed: '✓', failed: '✗' } +const GLYPH_COLOR: Record = { pending: 'var(--muted)', running: 'var(--accent-light)', completed: 'var(--success)', failed: 'var(--error)' } + +export default function ToolRow(props: { tool: ToolRowData }) { + const [open, setOpen] = useState(false) + const t = props.tool + return ( +
+ + {open && t.detail && ( + 
{t.detail}
+ )} +
+ ) +} + +const row: CSSProperties = { display: 'flex', alignItems: 'center', gap: 8, width: '100%', background: 'var(--card)', border: '1px solid var(--line)', borderRadius: 6, padding: '4px 10px', cursor: 'pointer', textAlign: 'left' } +const detailBox: CSSProperties = { margin: '2px 0 0 22px', padding: '6px 10px', background: 'var(--card)', border: '1px solid var(--line)', borderRadius: 6, fontSize: 11, color: 'var(--muted)', whiteSpace: 'pre-wrap', maxHeight: 200, overflow: 'auto' } diff --git a/src/renderer/src/store/runtime.ts b/src/renderer/src/store/runtime.ts index e0270b5..2a717d5 100644 --- a/src/renderer/src/store/runtime.ts +++ b/src/renderer/src/store/runtime.ts @@ -123,3 +123,8 @@ export function isStreaming(chat: Chat): boolean { if (!msgs || msgs.length === 0) return false return Boolean(msgs[msgs.length - 1]?.streaming) } + +// Reverse-lookup: the run id currently driving a chat's streaming turn (for Stop / permission responses). +export function runIdForChat(chatId: string): string | undefined { + return Object.keys(runToChat).find((r) => runToChat[r] === chatId) +} From 908c11cb940a974fd9254a70c2b6439d1dfb12c2 Mon Sep 17 00:00:00 2001 From: Nathan Font-Fife Date: Thu, 9 Jul 2026 05:39:43 -0600 Subject: [PATCH 08/11] fix(acp/ui): expire permissions before terminal events; never rehydrate streaming; neutral resolved glyph Co-Authored-By: Claude Fable 5 --- src/main/runtime/acp/acpSession.ts | 7 +++++-- src/renderer/src/components/PermissionCard.tsx | 2 +- src/renderer/src/store/persist.test.ts | 5 +++++ src/renderer/src/store/persist.ts | 1 + 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/main/runtime/acp/acpSession.ts b/src/main/runtime/acp/acpSession.ts index ab30c3a..f0a4145 100644 --- a/src/main/runtime/acp/acpSession.ts +++ b/src/main/runtime/acp/acpSession.ts @@ -108,12 +108,15 @@ export class AcpSession implements TransportSession { .request('session/prompt', { sessionId: this.sessionId, prompt: [{ type: 'text', text }] }, PROMPT_TIMEOUT_MS) .then((res) => { const stop = (res as { stopReason?: string } | null)?.stopReason + this.expirePermissions() // resolve open cards BEFORE the terminal event unmaps the run (Critical 1: order matters) this.onEvent({ type: 'run.completed', runId, stopReason: stop === 'cancelled' ? 'canceled' : 'end_turn' }) }) - .catch((e: Error) => this.onEvent({ type: 'run.errored', runId, message: e.message })) + .catch((e: Error) => { + this.expirePermissions() + this.onEvent({ type: 'run.errored', runId, message: e.message }) + }) .finally(() => { this.currentRunId = null - this.expirePermissions() }) } diff --git a/src/renderer/src/components/PermissionCard.tsx b/src/renderer/src/components/PermissionCard.tsx index 5129a00..51fe2b8 100644 --- a/src/renderer/src/components/PermissionCard.tsx +++ b/src/renderer/src/components/PermissionCard.tsx @@ -5,7 +5,7 @@ export default function PermissionCard(props: { card: CardData; onRespond: (opti const c = props.card if (c.resolvedOptionId) { const chosen = c.options.find((o) => o.id === c.resolvedOptionId) - const label = c.resolvedOptionId === 'stale' ? '· expired' : `${chosen?.kind === 'deny' ? '✗' : '✓'} ${chosen?.label ?? c.resolvedOptionId}` + const label = c.resolvedOptionId === 'stale' ? '· expired' : chosen ? `${chosen.kind === 'deny' ? '✗' : '✓'} ${chosen.label}` : `· ${c.resolvedOptionId}` return (
{label} — {c.title} diff --git a/src/renderer/src/store/persist.test.ts b/src/renderer/src/store/persist.test.ts index cb9ccdb..10eba92 100644 --- a/src/renderer/src/store/persist.test.ts +++ b/src/renderer/src/store/persist.test.ts @@ -29,4 +29,9 @@ describe('normalizeChat — never restore live-looking tool/permission state', ( expect(c.messages[0].tools?.[0].status).toBe('failed') expect(c.messages[0].permissions?.[0].resolvedOptionId).toBe('stale') }) + + it('never rehydrates a streaming flag', () => { + const raw = { fast: false, messages: [{ id: 'a', role: 'assistant', text: 'x', streaming: true }] } as never + expect(normalizeChat(raw, 'c_stream').messages[0].streaming).toBe(false) + }) }) diff --git a/src/renderer/src/store/persist.ts b/src/renderer/src/store/persist.ts index 3a2038e..1bc3c15 100644 --- a/src/renderer/src/store/persist.ts +++ b/src/renderer/src/store/persist.ts @@ -42,6 +42,7 @@ export function normalizeChat(c: Partial & { claudeSessionId?: string | nu ? c.messages.map((m) => ({ ...m, // never restore live-looking state (the `compacting` doctrine) + streaming: false, tools: m.tools?.map((t) => (t.status === 'pending' || t.status === 'running' ? { ...t, status: 'failed' as const } : t)), permissions: m.permissions?.map((p) => (p.resolvedOptionId ? p : { ...p, resolvedOptionId: 'stale' })) })) From 77747cfa572abd333f5c48cf05357d49a8d9f6c5 Mon Sep 17 00:00:00 2001 From: Nathan Font-Fife Date: Thu, 9 Jul 2026 05:46:31 -0600 Subject: [PATCH 09/11] fix(acp): expand ~ in session cwd (copilot session/new rejects non-absolute paths) Found during live verification: interactive copilot runs silently fell back to headless because AcpSession passed the stored workspace path (~/Code/...) straight to session/new, which returns -32603 'Directory path must be absolute'. Route it through resolveCwd like every one-shot adapter does. --- src/main/runtime/acp/acpSession.test.ts | 15 ++++++++++++++- src/main/runtime/acp/acpSession.ts | 12 ++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/main/runtime/acp/acpSession.test.ts b/src/main/runtime/acp/acpSession.test.ts index d2b30e7..ac02943 100644 --- a/src/main/runtime/acp/acpSession.test.ts +++ b/src/main/runtime/acp/acpSession.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from 'vitest' -import { pickAutoApprove } from './acpSession' +import { homedir } from 'os' +import { pickAutoApprove, acpCwd } from './acpSession' describe('pickAutoApprove', () => { it('picks the first allow-kind option', () => { @@ -13,3 +14,15 @@ describe('pickAutoApprove', () => { expect(pickAutoApprove([{ id: 'reject_once', label: 'Deny', kind: 'deny' }])).toBeUndefined() }) }) + +describe('acpCwd', () => { + it('expands a stored ~ workspace path to absolute (copilot session/new rejects non-absolute)', () => { + expect(acpCwd('~/Code/nac-code')).toBe(`${homedir()}/Code/nac-code`) + expect(acpCwd('~')).toBe(homedir()) + }) + it('passes an absolute path through and falls back to process cwd when unset', () => { + expect(acpCwd('/abs/path')).toBe('/abs/path') + expect(acpCwd(undefined)).toBe(process.cwd()) + expect(acpCwd('')).toBe(process.cwd()) + }) +}) diff --git a/src/main/runtime/acp/acpSession.ts b/src/main/runtime/acp/acpSession.ts index f0a4145..afdbf78 100644 --- a/src/main/runtime/acp/acpSession.ts +++ b/src/main/runtime/acp/acpSession.ts @@ -1,6 +1,7 @@ import { JsonRpcClient } from '../capabilities/jsonRpc' import type { AgentEvent, PermissionOption } from '../../../shared/runtime' import { mapAcpUpdate, mapPermissionRequest } from './mapAcp' +import { resolveCwd } from '../paths' export const PROMPT_TIMEOUT_MS = 1_800_000 // 30 min — cancellation, not timeout, is the stop lever const HANDSHAKE_TIMEOUT_MS = 10_000 @@ -17,6 +18,13 @@ export function pickAutoApprove(options: PermissionOption[]): PermissionOption | return options.find((o) => o.kind === 'allow' || o.kind === 'allow_always') } +/** Pure + exported for testing: ACP session cwd. copilot's session/new rejects a non-absolute path + * (`-32603 "Directory path must be absolute"`), so a stored `~/…` workspace path MUST be expanded — + * the same resolveCwd every one-shot adapter uses. Falls back to process cwd when unset. */ +export function acpCwd(cwd: string | undefined): string { + return resolveCwd(cwd) ?? process.cwd() +} + interface PendingPermission { resolve: (optionId: string) => void } @@ -56,7 +64,7 @@ export class AcpSession implements TransportSession { if (existingSessionId) { try { this.replaying = true // session/load re-emits history as session/update — never re-append it - await this.client.request('session/load', { sessionId: existingSessionId, cwd: cwd ?? process.cwd(), mcpServers: [] }, HANDSHAKE_TIMEOUT_MS) + await this.client.request('session/load', { sessionId: existingSessionId, cwd: acpCwd(cwd), mcpServers: [] }, HANDSHAKE_TIMEOUT_MS) this.sessionId = existingSessionId return existingSessionId } catch { @@ -65,7 +73,7 @@ export class AcpSession implements TransportSession { this.replaying = false } } - const res = (await this.client.request('session/new', { cwd: cwd ?? process.cwd(), mcpServers: [] }, HANDSHAKE_TIMEOUT_MS)) as { sessionId?: string } + const res = (await this.client.request('session/new', { cwd: acpCwd(cwd), mcpServers: [] }, HANDSHAKE_TIMEOUT_MS)) as { sessionId?: string } if (!res?.sessionId) throw new Error('acp: session/new returned no sessionId') this.sessionId = res.sessionId return res.sessionId From 9cb92a08a5cf3dee1fa3b34553edd51a7d7d8940 Mon Sep 17 00:00:00 2001 From: Nathan Font-Fife Date: Thu, 9 Jul 2026 05:51:44 -0600 Subject: [PATCH 10/11] =?UTF-8?q?docs:=20interactive=20run=20transport=20p?= =?UTF-8?q?illar=201=20done=20=E2=80=94=20copilot=20ACP=20verified=20live?= =?UTF-8?q?=20(allow/deny/cancel/YOLO/continuity/switch)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- docs/DECISIONS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index 56d8866..9f4da80 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -4,6 +4,8 @@ ## Current phase +**✅ Interactive run transport — pillar 1, copilot ACP** (`77747cf`): copilot runs are INTERACTIVE. A persistent `copilot --acp` session per chat (spawn → initialize → session/new, revived via session/load) sits behind a provider-generic `TransportSession` seam; the M4 JSON-RPC client learned to answer server→client requests (that's how permission responses flow). The harness's permission requests render as inline transcript cards (Allow once / Always allow / Deny — the harness's own options), tool calls render as live expandable rows (status glyph + output on expand), and Stop is a real protocol-level `session/cancel`. YOLO auto-approves (no cards). Tool/permission history is render-only — the replay invariant holds (`buildReplayPrompt` reads only turn text), so cross-provider switches stay bounded and clean. Every ACP failure falls back to the one-shot headless path (floor = previous release). **Verified live (computer-use matrix):** allow → card resolves + tool ✓ + reply; deny → red ✗; cancel mid-`sleep` → "cancelled by user"; YOLO → no card; two-turn native continuity (no replay block on turn 2); copilot→Claude switch recalled all five prior shell commands (context preserved, no tool chatter in replay). **Live verification caught a real bug the 111-test suite could not:** AcpSession passed the stored `~/…` workspace path straight to session/new, which returns `-32603 "Directory path must be absolute"` — every interactive run silently fell back to headless; fixed by routing through `resolveCwd` like the one-shot adapters. Pillars 2-4 (codex app-server, claude, opencode acp) reuse this seam. Spec: `docs/superpowers/specs/2026-07-09-interactive-run-transport-design.md`. + **✅ Per-account capability discovery — M4 pillar one** (`1f48c8a`): the model picker now shows what each harness ACTUALLY provides for the owner's account — no hardcoded model lists. Codex: app-server v2 `model/list` over stdio JSON-RPC (per-model reasoning efforts + defaults; verified in-app: the account's real 5 models rendered, GPT-5.5 picked → run completed with `-m gpt-5.5`); Copilot: ACP `session/new` `availableModels` (11 models with usage multipliers + current default; verified in-app: Claude Sonnet 4.6 picked → run completed with `--model claude-sonnet-4.6`) — codex/copilot model selection is now WIRED, superseding the 2026-06-29 "needs M4" lock. Claude stays static-base (no headless list exists; raw API `/v1/models` rejected — wrapper invariant) merged with a persisted **gating ledger** (`userData/nac-capability-ledger.json`) that learns per-account verdicts from real run outcomes — verified: both test runs recorded `works` with correct attribution, and a gated entry renders the model chip warning-tinted after refresh (still clickable — fail-honest stands). Effort is provider-real: per-model scales (codex incl. xhigh), claude's 6 documented levels (+session-only note), copilot's 7, opencode variants; `null` = harness default; **verified in-app: switching codex→copilot reset effort to default**. Legacy `thinking` migrated (pre-feature cosmetic values → null). Degradation ladder protocol → static+learned → static keeps the app at the previous floor when discovery fails. Spec: `docs/superpowers/specs/2026-07-08-per-account-capability-discovery-design.md`. **✅ Provider-first model picker + real options** (`dcbf0ad`): the model modal is two-page (detected providers → provider page); availability = live CLI probe (CliRegistry v0 — starts M4; Cursor dropped until it has an adapter). Thinking/effort is REAL on all four harnesses (claude `--effort`, codex `model_reasoning_effort`, copilot `--reasoning-effort`, opencode `--variant`; universal none/low/medium/high, 'none' = harness default) — closes the "thinking-level wiring" next-option. Claude extras: fast mode via per-run `--settings '{"fastMode":true}'` (no --fast flag exists) and a Sonnet 1M-context variant (`sonnet[1m]`), **both verified end-to-end in the running app** (2026-07-08 GUI check: picked `Sonnet 4.6 · 1M` in the new picker → run completed and `claude-sonnet-4-6[1m]` appeared in the harness's model usage; toggled Fast mode on → resumed run with the injected settings completed. Fast mode = the flag is accepted and sent per-run; the speed gain itself is server-side and not asserted). Gated options fail honestly (harness stderr → transcript). Live-verified `--effort low` on all four binaries: claude/codex/copilot completed cleanly first try; opencode's account-default model hung past 120s (unrelated to `--variant` itself — the flag worked once a responsive model was targeted), confirmed by retrying with `-m opencode/big-pickle`, which completed in ~1s — no `OptionDef` change needed, the existing "model-dependent" note already covers this. Spec: `docs/superpowers/specs/2026-07-08-provider-first-model-picker-design.md`. Review fixes: codex/copilot model chips are display-only ("account default · needs M4 discovery" — honest UI per the locked M4 decision), and effort defaults migrated to 'none' (pre-feature persisted 'medium' was cosmetic; runs gain flags only when the user picks a level). From ea2d75bf4d604849ab796508fca98eb74c28624b Mon Sep 17 00:00:00 2001 From: Nathan Font-Fife Date: Thu, 9 Jul 2026 06:05:47 -0600 Subject: [PATCH 11/11] fix(review): session-load throws to preserve context, dead-child recovery, no ledger poisoning, live event sink, provider-switch dispose, react-safe detail Co-Authored-By: Claude Fable 5 --- docs/DECISIONS.md | 2 +- src/main/runtime/acp/acpSession.ts | 22 +++++++++++++--- src/main/runtime/acp/mapAcp.test.ts | 5 ++++ src/main/runtime/acp/mapAcp.ts | 13 +++++++--- src/main/runtime/acp/sessionManager.ts | 33 +++++++++++++++++++++--- src/main/runtime/capabilities/jsonRpc.ts | 12 ++++++++- src/main/runtime/ipc.ts | 10 ++++--- 7 files changed, 81 insertions(+), 16 deletions(-) diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index 9f4da80..d7ece30 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -4,7 +4,7 @@ ## Current phase -**✅ Interactive run transport — pillar 1, copilot ACP** (`77747cf`): copilot runs are INTERACTIVE. A persistent `copilot --acp` session per chat (spawn → initialize → session/new, revived via session/load) sits behind a provider-generic `TransportSession` seam; the M4 JSON-RPC client learned to answer server→client requests (that's how permission responses flow). The harness's permission requests render as inline transcript cards (Allow once / Always allow / Deny — the harness's own options), tool calls render as live expandable rows (status glyph + output on expand), and Stop is a real protocol-level `session/cancel`. YOLO auto-approves (no cards). Tool/permission history is render-only — the replay invariant holds (`buildReplayPrompt` reads only turn text), so cross-provider switches stay bounded and clean. Every ACP failure falls back to the one-shot headless path (floor = previous release). **Verified live (computer-use matrix):** allow → card resolves + tool ✓ + reply; deny → red ✗; cancel mid-`sleep` → "cancelled by user"; YOLO → no card; two-turn native continuity (no replay block on turn 2); copilot→Claude switch recalled all five prior shell commands (context preserved, no tool chatter in replay). **Live verification caught a real bug the 111-test suite could not:** AcpSession passed the stored `~/…` workspace path straight to session/new, which returns `-32603 "Directory path must be absolute"` — every interactive run silently fell back to headless; fixed by routing through `resolveCwd` like the one-shot adapters. Pillars 2-4 (codex app-server, claude, opencode acp) reuse this seam. Spec: `docs/superpowers/specs/2026-07-09-interactive-run-transport-design.md`. +**✅ Interactive run transport — pillar 1, copilot ACP** (`77747cf`): copilot runs are INTERACTIVE. A persistent `copilot --acp` session per chat (spawn → initialize → session/new, revived via session/load) sits behind a provider-generic `TransportSession` seam; the M4 JSON-RPC client learned to answer server→client requests (that's how permission responses flow). The harness's permission requests render as inline transcript cards (Allow once / Always allow / Deny — the harness's own options), tool calls render as live expandable rows (status glyph + output on expand), and Stop is a real protocol-level `session/cancel`. YOLO auto-approves (no cards). Tool/permission history is render-only — the replay invariant holds (`buildReplayPrompt` reads only turn text), so cross-provider switches stay bounded and clean. Every ACP failure falls back to the one-shot headless path (floor = previous release). **Verified live (computer-use matrix):** allow → card resolves + tool ✓ + reply; deny → red ✗; cancel mid-`sleep` → "cancelled by user"; YOLO → no card; two-turn native continuity (no replay block on turn 2); copilot→Claude switch recalled all five prior shell commands (context preserved, no tool chatter in replay). **Live verification caught a real bug the 111-test suite could not:** AcpSession passed the stored `~/…` workspace path straight to session/new, which returns `-32603 "Directory path must be absolute"` — every interactive run silently fell back to headless; fixed by routing through `resolveCwd` like the one-shot adapters. Pillars 2-4 (codex app-server, claude, opencode acp) reuse this seam. Spec: `docs/superpowers/specs/2026-07-09-interactive-run-transport-design.md`. **Known pillar-1 limitation:** the ACP session runs copilot's account-default model — the picker's model choice isn't yet forwarded over ACP (a pillar-1 follow-up); ledger verdicts are not recorded for copilot to avoid mis-attribution. **✅ Per-account capability discovery — M4 pillar one** (`1f48c8a`): the model picker now shows what each harness ACTUALLY provides for the owner's account — no hardcoded model lists. Codex: app-server v2 `model/list` over stdio JSON-RPC (per-model reasoning efforts + defaults; verified in-app: the account's real 5 models rendered, GPT-5.5 picked → run completed with `-m gpt-5.5`); Copilot: ACP `session/new` `availableModels` (11 models with usage multipliers + current default; verified in-app: Claude Sonnet 4.6 picked → run completed with `--model claude-sonnet-4.6`) — codex/copilot model selection is now WIRED, superseding the 2026-06-29 "needs M4" lock. Claude stays static-base (no headless list exists; raw API `/v1/models` rejected — wrapper invariant) merged with a persisted **gating ledger** (`userData/nac-capability-ledger.json`) that learns per-account verdicts from real run outcomes — verified: both test runs recorded `works` with correct attribution, and a gated entry renders the model chip warning-tinted after refresh (still clickable — fail-honest stands). Effort is provider-real: per-model scales (codex incl. xhigh), claude's 6 documented levels (+session-only note), copilot's 7, opencode variants; `null` = harness default; **verified in-app: switching codex→copilot reset effort to default**. Legacy `thinking` migrated (pre-feature cosmetic values → null). Degradation ladder protocol → static+learned → static keeps the app at the previous floor when discovery fails. Spec: `docs/superpowers/specs/2026-07-08-per-account-capability-discovery-design.md`. diff --git a/src/main/runtime/acp/acpSession.ts b/src/main/runtime/acp/acpSession.ts index afdbf78..d783883 100644 --- a/src/main/runtime/acp/acpSession.ts +++ b/src/main/runtime/acp/acpSession.ts @@ -27,6 +27,7 @@ export function acpCwd(cwd: string | undefined): string { interface PendingPermission { resolve: (optionId: string) => void + denyId: string } export class AcpSession implements TransportSession { @@ -67,8 +68,13 @@ export class AcpSession implements TransportSession { await this.client.request('session/load', { sessionId: existingSessionId, cwd: acpCwd(cwd), mcpServers: [] }, HANDSHAKE_TIMEOUT_MS) this.sessionId = existingSessionId return existingSessionId - } catch { - // fall through to a fresh session (caller seeds it with the replay prompt on the next send) + } catch (e) { + // Re-throw: the caller sent a BARE message (renderer chose native continuity, no replay + // text seeded). Falling through to session/new here would silently start an empty + // session and drop the conversation — a hard context-preservation violation. Rejecting + // connect() instead makes promptViaAcp resolve { ok: false }, so ipc.ts falls back to the + // one-shot startCopilotRun(sessionId) path, which uses --resume to preserve context. + throw e instanceof Error ? e : new Error(String(e)) } finally { this.replaying = false } @@ -93,8 +99,10 @@ export class AcpSession implements TransportSession { if (auto) return Promise.resolve({ outcome: { outcome: 'selected', optionId: auto.id } }) } this.onEvent(event) + const denyId = event.options.find((o) => o.kind === 'deny')?.id ?? event.options[event.options.length - 1].id return new Promise((resolve) => { this.pendingPermissions.set(requestId, { + denyId, resolve: (optionId) => { this.onEvent({ type: 'permission.resolved', runId, requestId, optionId }) resolve({ outcome: { outcome: 'selected', optionId } }) @@ -136,13 +144,19 @@ export class AcpSession implements TransportSession { } private expirePermissions(): void { - // A turn ended with cards still open (error/cancel): answer the protocol with a deny-equivalent. + // A turn ended with cards still open (error/cancel): answer the protocol with a deny-equivalent + // — the actual deny option id offered for that request, not a hardcoded guess. for (const [requestId, p] of this.pendingPermissions) { this.pendingPermissions.delete(requestId) - p.resolve('reject_once') + p.resolve(p.denyId) } } + /** True once the underlying ACP child process has exited — the session can no longer be used. */ + get dead(): boolean { + return this.client.isClosed + } + cancel(): void { if (this.sessionId) this.client.notify('session/cancel', { sessionId: this.sessionId }) } diff --git a/src/main/runtime/acp/mapAcp.test.ts b/src/main/runtime/acp/mapAcp.test.ts index 33023bc..39f3307 100644 --- a/src/main/runtime/acp/mapAcp.test.ts +++ b/src/main/runtime/acp/mapAcp.test.ts @@ -24,6 +24,11 @@ describe('mapAcpUpdate', () => { const [e] = mapAcpUpdate('r', { sessionUpdate: 'tool_call_update', toolCallId: 'call_MHx', content: [{ type: 'content', content: { type: 'text', text: 'partial' } }] }) expect(e).toMatchObject({ type: 'tool.updated', status: 'running', detail: 'partial' }) }) + it('never surfaces a non-string rawOutput.content as detail (Minor 6: React would crash on an object child)', () => { + const [e] = mapAcpUpdate('r', { sessionUpdate: 'tool_call_update', toolCallId: 'call_MHx', status: 'completed', rawOutput: { content: { type: 'image', data: 'base64...' } } }) + expect(e).toMatchObject({ type: 'tool.updated', toolCallId: 'call_MHx', status: 'completed' }) + expect((e as { detail?: unknown }).detail).toBeUndefined() + }) }) describe('mapPermissionRequest', () => { diff --git a/src/main/runtime/acp/mapAcp.ts b/src/main/runtime/acp/mapAcp.ts index 690d5c8..39bafd2 100644 --- a/src/main/runtime/acp/mapAcp.ts +++ b/src/main/runtime/acp/mapAcp.ts @@ -13,12 +13,18 @@ interface AcpUpdate { kind?: string status?: string rawInput?: { command?: string } - rawOutput?: { content?: string } + rawOutput?: { content?: unknown } content?: AcpContentEntry[] | { text?: string } } const TOOL_STATUSES = new Set(['pending', 'running', 'completed', 'failed']) +/** rawOutput.content / rawInput.command can be structured (non-string) — only strings are safe to + * hand to React as event detail, so anything else is dropped rather than crashing the renderer. */ +function asStringDetail(x: unknown): string | undefined { + return typeof x === 'string' ? x : undefined +} + function contentText(u: AcpUpdate): string | undefined { if (Array.isArray(u.content)) { const texts = u.content.map((c) => c?.content?.text).filter((t): t is string => Boolean(t)) @@ -40,7 +46,7 @@ export function mapAcpUpdate(runId: string, update: unknown): AgentEvent[] { case 'tool_call_update': { if (!u.toolCallId) return [] const status = (u.status && TOOL_STATUSES.has(u.status) ? u.status : u.sessionUpdate === 'tool_call' ? 'pending' : 'running') as 'pending' | 'running' | 'completed' | 'failed' - const detail = u.rawOutput?.content ?? contentText(u) ?? u.rawInput?.command + const detail = asStringDetail(u.rawOutput?.content) ?? contentText(u) ?? u.rawInput?.command return [{ type: 'tool.updated', runId, toolCallId: u.toolCallId, title: u.title ?? u.toolCallId, kind: u.kind, status, ...(detail ? { detail } : {}) }] } default: @@ -65,10 +71,11 @@ export function mapPermissionRequest(runId: string, requestId: string, params: u options.push({ id: o.optionId, label: o.name ?? o.optionId, kind: OPTION_KINDS[o.kind ?? ''] ?? 'deny' }) } if (options.length === 0) return null + const detail = asStringDetail(p.toolCall?.rawInput?.command) return { type: 'permission.requested', runId, requestId, title: p.toolCall?.title ?? 'Permission request', - ...(p.toolCall?.rawInput?.command ? { detail: p.toolCall.rawInput.command } : {}), + ...(detail ? { detail } : {}), options } } diff --git a/src/main/runtime/acp/sessionManager.ts b/src/main/runtime/acp/sessionManager.ts index 74d952f..45607d0 100644 --- a/src/main/runtime/acp/sessionManager.ts +++ b/src/main/runtime/acp/sessionManager.ts @@ -1,14 +1,18 @@ import type { AgentEvent } from '../../../shared/runtime' import { AcpSession } from './acpSession' -// One live ACP session per chat. Sessions die on provider switch (a new prompt for the same chat -// with a different transport never reaches here), app quit, or idle timeout. +// One live ACP session per chat. Sessions are disposed on provider switch (promptViaAcp detects +// this when the renderer sends no sessionId — see below), a dead child process being replaced on +// the next prompt, app quit, or idle timeout. export const IDLE_MS = 15 * 60_000 interface Entry { session: AcpSession idleTimer: ReturnType | null + // Mutable indirection so a reused session's event sink always points at the CURRENT caller's + // onEvent, not the closure captured when the session was first created (Important 4). + ref: { onEvent: (e: AgentEvent) => void } } const byChat = new Map() @@ -45,10 +49,29 @@ export async function promptViaAcp(opts: { onEvent: (e: AgentEvent) => void }): Promise<{ ok: boolean }> { let entry = byChat.get(opts.chatId) + + // Important 2: a dead child (process exited mid-lifetime) must never be reused — its stdin is + // gone, so a session/prompt against it would hang until the 30-min timeout with Stop a no-op. + if (entry && entry.session.dead) { + if (entry.idleTimer) clearTimeout(entry.idleTimer) + entry.session.dispose() + byChat.delete(opts.chatId) + entry = undefined + } + + // Important 5: no sessionId means the renderer built a replay prompt — it believes there's no + // native session (provider changed, or the session was otherwise dropped client-side). Any ACP + // session we're still holding for this chat is stale and must be disposed, per spec. + if (entry && opts.sessionId === undefined) { + disposeChat(opts.chatId) + entry = undefined + } + if (!entry) { + const ref = { onEvent: opts.onEvent } const session = new AcpSession((e) => { if (e.type === 'run.completed' || e.type === 'run.errored') runToChat.delete(e.runId) - opts.onEvent(e) + ref.onEvent(e) }, opts.yolo === true) try { await session.connect(opts.cwd, opts.sessionId) @@ -56,8 +79,10 @@ export async function promptViaAcp(opts: { session.dispose() return { ok: false } } - entry = { session, idleTimer: null } + entry = { session, idleTimer: null, ref } byChat.set(opts.chatId, entry) + } else { + entry.ref.onEvent = opts.onEvent } entry.session.setYolo(opts.yolo === true) runToChat.set(opts.runId, opts.chatId) diff --git a/src/main/runtime/capabilities/jsonRpc.ts b/src/main/runtime/capabilities/jsonRpc.ts index 15d98a6..efff5b5 100644 --- a/src/main/runtime/capabilities/jsonRpc.ts +++ b/src/main/runtime/capabilities/jsonRpc.ts @@ -57,6 +57,7 @@ export class JsonRpcClient { private pending = new Map void; reject: (e: Error) => void }>() private notificationHandlers = new Map void>() private requestHandlers = new Map Promise | unknown>() + private closed = false constructor(command: string, args: string[]) { this.child = spawn(command, args, { stdio: ['pipe', 'pipe', 'ignore'] }) @@ -78,7 +79,15 @@ export class JsonRpcClient { } }) this.child.on('error', (err) => this.failAll(err)) - this.child.on('close', () => this.failAll(new Error('rpc server closed'))) + this.child.on('close', () => { + this.closed = true + this.failAll(new Error('rpc server closed')) + }) + } + + /** True once the child process has exited — further requests would hang forever on a dead pipe. */ + get isClosed(): boolean { + return this.closed } private failAll(err: Error): void { @@ -114,6 +123,7 @@ export class JsonRpcClient { } request(method: string, params?: unknown, timeoutMs = 5000): Promise { + if (this.closed) return Promise.reject(new Error('rpc: server closed')) const id = this.nextId++ const payload = JSON.stringify({ jsonrpc: '2.0', id, method, params: params ?? {} }) return new Promise((resolve, reject) => { diff --git a/src/main/runtime/ipc.ts b/src/main/runtime/ipc.ts index 5d594d6..3f40e24 100644 --- a/src/main/runtime/ipc.ts +++ b/src/main/runtime/ipc.ts @@ -58,14 +58,18 @@ export function registerRuntimeIpc(getWindow: () => BrowserWindow | null): void ipcMain.handle(RUN_CHANNELS.start, (_e, req: RunRequest): { runId: string } => { const runId = `run_${++counter}` const send = (event: AgentEvent): void => getWindow()?.webContents.send(RUN_CHANNELS.event, event) + // ACP runs the account-default model; don't attribute ledger verdicts to the picked model + // (pillar-1 limitation) — copilot never forwards the picker's model choice over ACP, and its + // headless fallback is also default-model in practice, so gate the ledger off for copilot entirely. + const ledgerModel = req.provider === 'copilot' ? undefined : req.model const handler = (event: AgentEvent): void => { send(event) // Gating ledger: learn per-account model verdicts from real outcomes (explicit model only). - if (req.model && req.provider) { + if (ledgerModel && req.provider) { if (event.type === 'run.errored' && classifyModelRejection(event.message)) { - recordOutcome(req.provider, req.model, 'gated', event.message) + recordOutcome(req.provider, ledgerModel, 'gated', event.message) invalidateCapabilities(req.provider) // next loadCaps (picker mount) re-fetches + re-merges the ledger - } else if (event.type === 'run.completed' && event.stopReason === 'end_turn') recordOutcome(req.provider, req.model, 'works') + } else if (event.type === 'run.completed' && event.stopReason === 'end_turn') recordOutcome(req.provider, ledgerModel, 'works') } if (event.type === 'run.completed' || event.type === 'run.errored') runs.delete(runId) }