From 539e91d55e2107d57206b88ddcb031db95e9ca44 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Sun, 6 Sep 2026 18:05:15 -0700 Subject: [PATCH] fix(chat): authorize mobile commands and bound clear-chain projection --- ...uctured-conversation-command-controller.ts | 39 +++++++----- ...ructured-conversation-replacements.test.ts | 62 +++++++++++++++++++ src/main/runtime/mobile-rpc-allowlist.test.ts | 1 + .../runtime-rpc-mobile-method-allowlist.ts | 1 + ...se-native-chat-structured-composer-send.ts | 6 +- .../structured-agent-session-client.ts | 9 +-- ...ss-version-agent-session-wire.unit.test.ts | 13 ++++ 7 files changed, 109 insertions(+), 22 deletions(-) create mode 100644 src/main/native-chat/agent-session-wire/structured-conversation-replacements.test.ts diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-command-controller.ts b/src/main/native-chat/agent-session-wire/structured-conversation-command-controller.ts index f115de3692a..e5a0283ba83 100644 --- a/src/main/native-chat/agent-session-wire/structured-conversation-command-controller.ts +++ b/src/main/native-chat/agent-session-wire/structured-conversation-command-controller.ts @@ -55,22 +55,33 @@ export class StructuredConversationCommandController { const store = this.context().deps.store const records = store.listRecords() const visible = new Set(store.listVisibleSessionIds()) - return records.flatMap((record) => { - let command = record.conversationCommand - let sessionId: string | undefined - const visited = new Set([record.sessionId]) - while ( - command?.command === 'clear' && - command.phase === 'committed' && - command.replacementSessionId - ) { - sessionId = command.replacementSessionId - if (visited.has(sessionId)) { - return [] + const byId = new Map(records.map((record) => [record.sessionId, record])) + const destinations = new Map() + const destination = (source: string): string | null => { + const path = new Set() + let current = source + while (!destinations.has(current) && !path.has(current)) { + path.add(current) + const command = byId.get(current)?.conversationCommand + if ( + command?.command !== 'clear' || + command.phase !== 'committed' || + !command.replacementSessionId + ) { + destinations.set(current, current) + break } - visited.add(sessionId) - command = store.getRecord(sessionId)?.conversationCommand + current = command.replacementSessionId } + const target = destinations.get(current) ?? null + for (const id of path) { + destinations.set(id, target) + } + return target + } + return records.flatMap((record) => { + const target = destination(record.sessionId) + const sessionId = target !== record.sessionId ? target : null // Explicit history reveals remain readable; closed replacements stay closed. return sessionId && visible.has(sessionId) && !visible.has(record.sessionId) ? [ diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-replacements.test.ts b/src/main/native-chat/agent-session-wire/structured-conversation-replacements.test.ts new file mode 100644 index 00000000000..3179e994fdb --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-conversation-replacements.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it, vi } from 'vitest' +import type { AgentSessionRecord } from '../../../shared/agent-session-record' +import { StructuredConversationCommandController } from './structured-conversation-command-controller' + +function replacements(records: AgentSessionRecord[], visible: string[]) { + const store = { + listRecords: () => records, + listVisibleSessionIds: () => visible, + getRecord: (id: string) => records.find((record) => record.sessionId === id) + } + const controller = new StructuredConversationCommandController( + () => ({ deps: { store } }) as never, + {} as never + ) + return controller.replacements() +} + +function record(id: string, next?: string): AgentSessionRecord { + return { + sessionId: id, + provider: 'codex', + location: { workspaceId: 'folder' }, + conversationCommand: next + ? { command: 'clear', phase: 'committed', replacementSessionId: next } + : undefined + } as AgentSessionRecord +} + +describe('conversation replacement projection', () => { + it('visits a long clear chain only once per snapshot', () => { + const reads = vi.fn() + const records = Array.from({ length: 200 }, (_, index) => { + const entry = record(String(index), index < 199 ? String(index + 1) : undefined) + const command = entry.conversationCommand + Object.defineProperty(entry, 'conversationCommand', { + get: () => { + reads() + return command + } + }) + return entry + }) + const result = replacements(records, ['199']) + expect(result).toHaveLength(199) + expect(result.every((entry) => entry.sessionId === '199')).toBe(true) + expect(reads.mock.calls.length).toBeLessThanOrEqual(records.length * 2) + }) + + it('keeps revealed history and closed chains out, and ignores cycles', () => { + const records = [ + record('a', 'b'), + record('b', 'c'), + record('c'), + record('x', 'y'), + record('y', 'x') + ] + expect(replacements(records, ['b', 'c', 'x'])).toEqual([ + { sourceSessionId: 'a', sessionId: 'c', workspaceId: 'folder', agent: 'codex' } + ]) + expect(replacements(records, [])).toEqual([]) + }) +}) diff --git a/src/main/runtime/mobile-rpc-allowlist.test.ts b/src/main/runtime/mobile-rpc-allowlist.test.ts index 1a27de18d55..1390cebf649 100644 --- a/src/main/runtime/mobile-rpc-allowlist.test.ts +++ b/src/main/runtime/mobile-rpc-allowlist.test.ts @@ -167,6 +167,7 @@ describe('mobile RPC allowlist', () => { 'agentSession.setOption', 'agentSession.handoffStatus', 'agentSession.options', + 'agentSession.conversationCommand', 'agentSession.history', 'agentSession.subscribe', 'agentSession.unsubscribe', diff --git a/src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts b/src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts index 05666add0bc..02021cb3767 100644 --- a/src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts +++ b/src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts @@ -214,6 +214,7 @@ export const MOBILE_RPC_METHOD_ALLOWLIST = new Set([ 'agentSession.setOption', 'agentSession.handoffStatus', 'agentSession.options', + 'agentSession.conversationCommand', 'agentSession.history', 'agentSession.subscribe', 'agentSession.unsubscribe', diff --git a/src/renderer/src/components/native-chat/use-native-chat-structured-composer-send.ts b/src/renderer/src/components/native-chat/use-native-chat-structured-composer-send.ts index 7911c6c511f..f6e6427d4e6 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-structured-composer-send.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-structured-composer-send.ts @@ -1,4 +1,4 @@ -import { useCallback, useRef } from 'react' +import { useCallback, useLayoutEffect, useRef } from 'react' import { emitNativeChatMessageSent } from '@/lib/native-chat-telemetry' import { isStructuredAgentSessionComposerCommand } from '../../../../shared/structured-agent-session-composer' import type { AgentType } from '../../../../shared/agent-status-types' @@ -36,7 +36,9 @@ export function useNativeChatStructuredComposerSend({ attachments?: readonly NativeChatComposerImageAttachment[] ) => void { const composition = useRef({ draft, imageAttachments }) - composition.current = { draft, imageAttachments } + useLayoutEffect(() => { + composition.current = { draft, imageAttachments } + }, [draft, imageAttachments]) return useCallback( (text: string, attachments = imageAttachments): void => { if (!structuredTransport) { diff --git a/src/renderer/src/runtime/structured-agent-session-client.ts b/src/renderer/src/runtime/structured-agent-session-client.ts index 2cf339d28b7..728689288b1 100644 --- a/src/renderer/src/runtime/structured-agent-session-client.ts +++ b/src/renderer/src/runtime/structured-agent-session-client.ts @@ -11,12 +11,9 @@ export function callStructuredAgentSession( method: string, params?: unknown ): Promise { - return callRuntimeRpc( - target, - method, - params, - method === 'agentSession.conversationCommand' ? { timeoutMs: 195_000 } : undefined - ) + return method === 'agentSession.conversationCommand' + ? callRuntimeRpc(target, method, params, { timeoutMs: 195_000 }) + : callRuntimeRpc(target, method, params) } async function subscribeStructuredAgentSessionMethod( diff --git a/tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts b/tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts index 8a407a29d15..607d37b0f26 100644 --- a/tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts +++ b/tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts @@ -68,6 +68,11 @@ const STRUCTURED_CALLS: { hostMethod: 'attach', result: { ok: true, replayed: false, value: { sessionId: SESSION } } }, + { + method: 'agentSession.conversationCommand', + hostMethod: 'conversationCommand', + result: { ok: true, value: { command: 'compact', state: 'completed' } } + }, { method: 'agentSession.send', hostMethod: 'send', result: { ok: true, replayed: false } }, { method: 'agentSession.cancel', hostMethod: 'cancel', result: { ok: true, replayed: false } }, { method: 'agentSession.close', hostMethod: 'close', result: { ok: true } }, @@ -210,6 +215,10 @@ function paramsFor(method: string): unknown { return createIntentParams() case 'agentSession.ensure': return attachParams(fence) + case 'agentSession.conversationCommand': { + const fields = { command: 'compact' } + return { envelope: envelope({ method, fields, fence }), ...fields } + } case 'agentSession.send': return sendParams('hi', fence) case 'agentSession.cancel': @@ -323,6 +332,10 @@ function structuredHostStub(): Record> { // supports creating there. A real host always answers; leaving it unstubbed made every // `ensure` refuse for the harness's own reason rather than the location's. supportsCreate: vi.fn(() => true), + conversationCommand: vi.fn(async () => ({ + ok: true, + value: { command: 'compact', state: 'completed' } + })), send: vi.fn(async () => ({ ok: true, replayed: false })), cancel: vi.fn(async () => ({ ok: true, replayed: false })), close: vi.fn(async () => undefined),