From ba4e79c2504233890754f64e3ad2ba73a7cfdec4 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 6 Sep 2026 23:32:00 -0700 Subject: [PATCH] fix(runtime): apply the structured-chat setting to every RPC caller (#18700) * fix(runtime): apply the structured-chat setting to every RPC caller supportsStructuredAgentSessions only consulted experimentalStructuredNativeChat when clientKind === 'mobile', so identical host settings admitted desktop and in-process callers while refusing a phone. The server branched on client surface. The setting is now one rule for every caller. The negotiated capability stays a wire term asked of remote clients only, so a capability-less in-process caller is still admitted on the setting alone. Making the projection's structuredNativeChatEnabled argument required surfaced eight call sites that passed `undefined` for non-mobile clients; they now read the host setting, so tab projection follows the same single rule. Announced behaviour change: with the flag off, session.tabs.list/listAll no longer restore structured tabs for desktop. The desktop renderer already discards them in that state, and startup record/lease reconciliation is unaffected. * fix(runtime): keep structured session cleanup available * test(runtime): enable structured chat in desktop projection fixture * test(agent-session): settle merged fixtures against the all-clients structured policy The merge with main left three fixtures written for the old mobile-only rule: a duplicate getClientSettings key, a create fixture with no host settings at all, and a projection call whose 'old client' is now the mobile fallback-title case. * fix(native-chat): let an admitted caller close a chat after the setting is off Turning `experimentalStructuredNativeChat` off revoked admission for every `agentSession.*` method, including `close`. A chat opened while the setting was on stays mounted, so its owner was left with a live provider child and an X button that answered `structured_agent_session_unsupported`. Split the surface by what a method does to work in flight rather than by how it sounds, and write that rule where the gate lives so the next method lands on the right side: starting, extending, retaining or reading needs admission; stopping or retiring work the caller already owns does not. Moves `close` and `cancel` onto the cleanup gate alongside `unsubscribe` and `release`. The tightening is unchanged - the cleanup gate still demands the negotiated wire capability and never creates a host, so an incapable client still cannot see the surface and no method that starts work is reachable with the setting off. Extracts the dispatcher harness and the method-to-gate table into fixtures so the new admission suite can share them without a max-lines disable. * Drop a duplicate lastActivityAt key carried in from main The main commit this branch merged (fb322046e8) had two lastActivityAt properties in the same object literal at both journal stubs, which fails TS1117 and oxlint. Upstream has since kept only the later value; match it. Not introduced here, but merged in, so it has to be fixed here. --------- Co-authored-by: Merge Sim --- src/main/ipc/runtime.test.ts | 1 + ...ude-structured-session-integration.test.ts | 1 + ...ion-tab-agent-capability-mutations.test.ts | 4 +- ...ession-tab-agent-status-projection.test.ts | 57 +++- .../session-tab-agent-status-projection.ts | 2 +- .../rpc/methods/session-tab-close-methods.ts | 8 +- .../methods/session-tab-mutation-methods.ts | 8 +- .../rpc/methods/session-tabs-inventory.ts | 12 +- .../session-tabs-snapshot.test-fixture.ts | 23 ++ .../session-tabs-structured-restore.test.ts | 109 ++++--- .../runtime/rpc/methods/session-tabs.test.ts | 25 +- src/main/runtime/rpc/methods/session-tabs.ts | 6 +- ...structured-agent-session-admission.test.ts | 112 +++++++ ...ession-gate-classification.test-fixture.ts | 79 +++++ .../methods/structured-agent-session-gate.ts | 38 ++- .../structured-agent-session-hold.test.ts | 51 +++ .../methods/structured-agent-session-hold.ts | 3 +- .../structured-agent-session-policy.test.ts | 101 ++++++ .../structured-agent-session-policy.ts | 27 +- ...ed-agent-session-precommit-refusal.test.ts | 3 + ...ructured-agent-session-rpc.test-fixture.ts | 270 ++++++++++++++++ .../methods/structured-agent-session.test.ts | 301 ++++-------------- .../rpc/methods/structured-agent-session.ts | 12 +- ...d-agent-session-integration-replay.test.ts | 1 + ...ructured-agent-session-integration.test.ts | 1 + ...ss-version-agent-session-wire.unit.test.ts | 1 + 26 files changed, 886 insertions(+), 370 deletions(-) create mode 100644 src/main/runtime/rpc/methods/session-tabs-snapshot.test-fixture.ts create mode 100644 src/main/runtime/rpc/methods/structured-agent-session-admission.test.ts create mode 100644 src/main/runtime/rpc/methods/structured-agent-session-gate-classification.test-fixture.ts create mode 100644 src/main/runtime/rpc/methods/structured-agent-session-policy.test.ts create mode 100644 src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts diff --git a/src/main/ipc/runtime.test.ts b/src/main/ipc/runtime.test.ts index 07010087363..3e4e34161a4 100644 --- a/src/main/ipc/runtime.test.ts +++ b/src/main/ipc/runtime.test.ts @@ -147,6 +147,7 @@ describe('registerRuntimeHandlers', () => { } const runtime = { getRuntimeId: vi.fn().mockReturnValue('runtime-1'), + getClientSettings: vi.fn(() => ({ experimentalStructuredNativeChat: true })), restoreStructuredAgentSessionTabs: vi.fn(async () => undefined), listMobileSessionTabs: vi.fn(async () => ({ worktree: 'workspace-1', diff --git a/src/main/runtime/claude-structured-session-integration.test.ts b/src/main/runtime/claude-structured-session-integration.test.ts index e9cba45ffa9..d464d87e8f7 100644 --- a/src/main/runtime/claude-structured-session-integration.test.ts +++ b/src/main/runtime/claude-structured-session-integration.test.ts @@ -391,6 +391,7 @@ beforeEach(async () => { } const runtime = { getRuntimeId: () => 'runtime-1', + getClientSettings: () => ({ experimentalStructuredNativeChat: true }), getStructuredAgentSessionCreateSupport: async () => ({ supported: true }), resolveStructuredAgentSessionCreateIntent: async (input: { envelope: unknown }) => ({ ...ensureParams(1), diff --git a/src/main/runtime/rpc/methods/session-tab-agent-capability-mutations.test.ts b/src/main/runtime/rpc/methods/session-tab-agent-capability-mutations.test.ts index 183f981ccee..0a1076bd8f6 100644 --- a/src/main/runtime/rpc/methods/session-tab-agent-capability-mutations.test.ts +++ b/src/main/runtime/rpc/methods/session-tab-agent-capability-mutations.test.ts @@ -180,7 +180,9 @@ function createFixture( getRuntimeId: () => 'test-runtime', listMobileSessionTabs: vi.fn().mockResolvedValue(snapshot), getClientSettings: () => ({ - experimentalStructuredNativeChat: options.structuredNativeChatEnabled === true + // Why: defaults on, so a fixture that says nothing about the setting exercises capability + // gating alone; callers opt into the off case explicitly. + experimentalStructuredNativeChat: options.structuredNativeChatEnabled !== false }), ...calls } as unknown as OrcaRuntimeService diff --git a/src/main/runtime/rpc/methods/session-tab-agent-status-projection.test.ts b/src/main/runtime/rpc/methods/session-tab-agent-status-projection.test.ts index cf68f7739c0..61bb30bdbcf 100644 --- a/src/main/runtime/rpc/methods/session-tab-agent-status-projection.test.ts +++ b/src/main/runtime/rpc/methods/session-tab-agent-status-projection.test.ts @@ -87,7 +87,9 @@ describe('projectSessionTabAgentStatus', () => { } ] } - const oldClient = projectSessionTabAgentStatus(snapshot, 'mobile', []) + // A paired client that never negotiated the capability, with the setting on: mobile keeps an + // unrenderable row under a fallback title, so only a non-mobile old client still loses them. + const oldClient = projectSessionTabAgentStatus(snapshot, 'runtime', [], true) expect(oldClient.tabs.map((tab) => tab.type)).toEqual(['terminal']) expect(oldClient.activeTabId).toBe('tab-1::leaf-1') expect(oldClient.activeTabType).toBe('terminal') @@ -96,11 +98,6 @@ describe('projectSessionTabAgentStatus', () => { expect(oldClient.tabGroups).toHaveLength(1) expect(oldClient.tabGroupLayout).toEqual({ type: 'leaf', groupId: 'group-a' }) - expect( - projectSessionTabAgentStatus(snapshot, 'mobile', [ - STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY - ]) - ).toEqual(oldClient) expect( projectSessionTabAgentStatus( snapshot, @@ -118,10 +115,25 @@ describe('projectSessionTabAgentStatus', () => { ) expect(capableMobile).toBe(snapshot) - const capable = projectSessionTabAgentStatus(snapshot, 'runtime', [ - STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY - ]) + const capable = projectSessionTabAgentStatus( + snapshot, + 'runtime', + [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY], + true + ) expect(capable).toBe(snapshot) + + // The host setting is policy for every caller, so a capable desktop client with the + // setting off sees the same projection an old client does. + expect( + projectSessionTabAgentStatus( + snapshot, + 'runtime', + [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY], + false + ) + ).toEqual(oldClient) + expect(projectSessionTabAgentStatus(snapshot, undefined, undefined, false)).toEqual(oldClient) }) const claudeSnapshot = { @@ -276,8 +288,10 @@ describe('projectSessionTabAgentStatus', () => { ) it('keeps Claude rows on the local renderer, which negotiates nothing', () => { - expect(projectSessionTabAgentStatus(claudeSnapshot, undefined, undefined)).toBe(claudeSnapshot) - expect(projectSessionTabAgentStatus(claudeSnapshot, undefined, [])).toBe(claudeSnapshot) + expect(projectSessionTabAgentStatus(claudeSnapshot, undefined, undefined, true)).toBe( + claudeSnapshot + ) + expect(projectSessionTabAgentStatus(claudeSnapshot, undefined, [], true)).toBe(claudeSnapshot) }) it('leaves Codex rows untouched whether or not the Claude capability is present', () => { @@ -295,11 +309,11 @@ describe('projectSessionTabAgentStatus', () => { ) } } - expect(projectSessionTabAgentStatus(codexOnly, undefined, undefined)).toBe(codexOnly) + expect(projectSessionTabAgentStatus(codexOnly, undefined, undefined, true)).toBe(codexOnly) }) it('withholds session boundaries from legacy paired clients', () => { - const projected = projectSessionTabAgentStatus(makeSnapshot(true), 'runtime', []) + const projected = projectSessionTabAgentStatus(makeSnapshot(true), 'runtime', [], true) expect(projected.tabs[0]).not.toHaveProperty('agentStatus') }) @@ -308,7 +322,12 @@ describe('projectSessionTabAgentStatus', () => { const snapshot = makeSnapshot(true) expect( - projectSessionTabAgentStatus(snapshot, 'runtime', [AGENT_SESSION_BOUNDARY_RUNTIME_CAPABILITY]) + projectSessionTabAgentStatus( + snapshot, + 'runtime', + [AGENT_SESSION_BOUNDARY_RUNTIME_CAPABILITY], + true + ) ).toBe(snapshot) }) @@ -317,8 +336,12 @@ describe('projectSessionTabAgentStatus', () => { const mobileBoundary = makeSnapshot(true) const runtimeCompletion = makeSnapshot(false) - expect(projectSessionTabAgentStatus(localBoundary, undefined, undefined)).toBe(localBoundary) - expect(projectSessionTabAgentStatus(mobileBoundary, 'mobile', [])).toBe(mobileBoundary) - expect(projectSessionTabAgentStatus(runtimeCompletion, 'runtime', [])).toBe(runtimeCompletion) + expect(projectSessionTabAgentStatus(localBoundary, undefined, undefined, true)).toBe( + localBoundary + ) + expect(projectSessionTabAgentStatus(mobileBoundary, 'mobile', [], true)).toBe(mobileBoundary) + expect(projectSessionTabAgentStatus(runtimeCompletion, 'runtime', [], true)).toBe( + runtimeCompletion + ) }) }) diff --git a/src/main/runtime/rpc/methods/session-tab-agent-status-projection.ts b/src/main/runtime/rpc/methods/session-tab-agent-status-projection.ts index 4496fdc5435..e2aa9ae7b00 100644 --- a/src/main/runtime/rpc/methods/session-tab-agent-status-projection.ts +++ b/src/main/runtime/rpc/methods/session-tab-agent-status-projection.ts @@ -55,7 +55,7 @@ export function projectSessionTabAgentStatus[2], - structuredNativeChatEnabled?: boolean + structuredNativeChatEnabled: boolean ): RuntimeMobileSessionTabsResult { return projectSessionTabBrowserPlacements( projectSessionTabAgentStatus( @@ -41,12 +41,6 @@ export function projectSessionTabsForClient( ) } -function structuredNativeChatEnabledForContext(context: RpcContext): boolean | undefined { - return context.clientKind === 'mobile' - ? isStructuredNativeChatEnabled(context.runtime) - : undefined -} - function projectInventory( inventory: SessionTabsInventory, context: RpcContext @@ -57,7 +51,7 @@ function projectInventory( snapshot, context.clientKind, context.clientCapabilities, - structuredNativeChatEnabledForContext(context) + isStructuredNativeChatEnabled(context.runtime) ) ), ...(inventory.authoritative && clientUnderstandsAuthoritativeInventory(context) @@ -128,7 +122,7 @@ export async function subscribeSessionTabsInventory( snapshot, context.clientKind, context.clientCapabilities, - structuredNativeChatEnabledForContext(context) + isStructuredNativeChatEnabled(context.runtime) ) as SessionTabsChange const withoutNavigationIntent = (snapshot: SessionTabsChange): SessionTabsChange => { if (snapshot.navigationIntent === undefined) { diff --git a/src/main/runtime/rpc/methods/session-tabs-snapshot.test-fixture.ts b/src/main/runtime/rpc/methods/session-tabs-snapshot.test-fixture.ts new file mode 100644 index 00000000000..1346512e52d --- /dev/null +++ b/src/main/runtime/rpc/methods/session-tabs-snapshot.test-fixture.ts @@ -0,0 +1,23 @@ +export function visibleSnapshot() { + return { + worktree: 'wt-1', + publicationEpoch: 'epoch-1', + snapshotVersion: 1, + activeGroupId: 'group-1', + activeTabId: 'tab-1::leaf-1', + activeTabType: 'terminal' as const, + tabGroups: [{ id: 'group-1', activeTabId: 'tab-1', tabOrder: ['tab-1'] }], + tabs: [ + { + type: 'terminal' as const, + id: 'tab-1::leaf-1', + parentTabId: 'tab-1', + leafId: 'leaf-1', + title: 'Terminal', + status: 'ready' as const, + terminal: 'pty-1', + isActive: true + } + ] + } +} diff --git a/src/main/runtime/rpc/methods/session-tabs-structured-restore.test.ts b/src/main/runtime/rpc/methods/session-tabs-structured-restore.test.ts index 083f334285e..c520294edba 100644 --- a/src/main/runtime/rpc/methods/session-tabs-structured-restore.test.ts +++ b/src/main/runtime/rpc/methods/session-tabs-structured-restore.test.ts @@ -1,22 +1,79 @@ -import { describe, expect, it, vi } from 'vitest' +import { describe, expect, it, vi, type Mock } from 'vitest' import { RpcDispatcher } from '../dispatcher' import type { RpcRequest } from '../core' import type { OrcaRuntimeService } from '../../orca-runtime' import { STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' import { SESSION_TAB_METHODS } from './session-tabs' +import { visibleSnapshot } from './session-tabs-snapshot.test-fixture' function makeRequest(method: string, params?: unknown): RpcRequest { return { id: 'req-1', authToken: 'tok', method, params } } +function makeRuntime(experimentalStructuredNativeChat: boolean): OrcaRuntimeService { + return { + getRuntimeId: () => 'test-runtime', + getClientSettings: vi.fn(() => ({ experimentalStructuredNativeChat })), + restoreStructuredAgentSessionTabs: vi.fn(), + listMobileSessionTabs: vi.fn().mockResolvedValue(visibleSnapshot()) + } as unknown as OrcaRuntimeService +} + +describe('structured session tab restoration follows one rule for every caller', () => { + it('does not restore for the desktop renderer while the host setting is off', async () => { + const runtime = makeRuntime(false) + const dispatcher = new RpcDispatcher({ runtime, methods: SESSION_TAB_METHODS }) + + const response = await dispatcher.dispatch( + makeRequest('session.tabs.list', { worktree: 'id:wt-1' }), + { + clientKind: 'runtime', + clientCapabilities: [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY] + } + ) + + expect(response.ok).toBe(true) + expect(runtime.restoreStructuredAgentSessionTabs).not.toHaveBeenCalled() + }) + + it('restores for the desktop renderer once the host setting is on', async () => { + const runtime = makeRuntime(true) + const dispatcher = new RpcDispatcher({ runtime, methods: SESSION_TAB_METHODS }) + + const response = await dispatcher.dispatch( + makeRequest('session.tabs.list', { worktree: 'id:wt-1' }), + { + clientKind: 'runtime', + clientCapabilities: [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY] + } + ) + + expect(response.ok).toBe(true) + expect(runtime.restoreStructuredAgentSessionTabs).toHaveBeenCalledTimes(1) + }) + + it('restores for an in-process caller on the same setting that admits remote clients', async () => { + const restoreCallsBySetting = new Map() + for (const enabled of [false, true]) { + const runtime = makeRuntime(enabled) + const dispatcher = new RpcDispatcher({ runtime, methods: SESSION_TAB_METHODS }) + + await dispatcher.dispatch(makeRequest('session.tabs.list', { worktree: 'id:wt-1' })) + + restoreCallsBySetting.set( + enabled, + (runtime.restoreStructuredAgentSessionTabs as unknown as Mock).mock.calls.length + ) + } + + expect(restoreCallsBySetting.get(false)).toBe(0) + expect(restoreCallsBySetting.get(true)).toBe(1) + }) +}) + describe('session tab structured restore gating', () => { it('does not restore structured tabs for mobile while the host setting is off', async () => { - const runtime = { - getRuntimeId: () => 'test-runtime', - getClientSettings: vi.fn(() => ({ experimentalStructuredNativeChat: false })), - restoreStructuredAgentSessionTabs: vi.fn(), - listMobileSessionTabs: vi.fn().mockResolvedValue(visibleSnapshot()) - } as unknown as OrcaRuntimeService + const runtime = makeRuntime(false) const dispatcher = new RpcDispatcher({ runtime, methods: SESSION_TAB_METHODS }) const response = await dispatcher.dispatch( @@ -34,12 +91,7 @@ describe('session tab structured restore gating', () => { // Why: an old build has no capability to advertise, and skipping the restore left it with // nothing to project after a desktop restart — neither the chat nor its fallback row. it('restores structured tabs for a mobile client that advertises no capability', async () => { - const runtime = { - getRuntimeId: () => 'test-runtime', - getClientSettings: vi.fn(() => ({ experimentalStructuredNativeChat: true })), - restoreStructuredAgentSessionTabs: vi.fn(), - listMobileSessionTabs: vi.fn().mockResolvedValue(visibleSnapshot()) - } as unknown as OrcaRuntimeService + const runtime = makeRuntime(true) const dispatcher = new RpcDispatcher({ runtime, methods: SESSION_TAB_METHODS }) const response = await dispatcher.dispatch( @@ -52,12 +104,7 @@ describe('session tab structured restore gating', () => { }) it('restores structured tabs for mobile once the setting is present', async () => { - const runtime = { - getRuntimeId: () => 'test-runtime', - getClientSettings: vi.fn(() => ({ experimentalStructuredNativeChat: true })), - restoreStructuredAgentSessionTabs: vi.fn(), - listMobileSessionTabs: vi.fn().mockResolvedValue(visibleSnapshot()) - } as unknown as OrcaRuntimeService + const runtime = makeRuntime(true) const dispatcher = new RpcDispatcher({ runtime, methods: SESSION_TAB_METHODS }) const response = await dispatcher.dispatch( @@ -72,27 +119,3 @@ describe('session tab structured restore gating', () => { expect(runtime.restoreStructuredAgentSessionTabs).toHaveBeenCalledTimes(1) }) }) - -function visibleSnapshot() { - return { - worktree: 'wt-1', - publicationEpoch: 'epoch-1', - snapshotVersion: 1, - activeGroupId: 'group-1', - activeTabId: 'tab-1::leaf-1', - activeTabType: 'terminal' as const, - tabGroups: [{ id: 'group-1', activeTabId: 'tab-1', tabOrder: ['tab-1'] }], - tabs: [ - { - type: 'terminal' as const, - id: 'tab-1::leaf-1', - parentTabId: 'tab-1', - leafId: 'leaf-1', - title: 'Terminal', - status: 'ready' as const, - terminal: 'pty-1', - isActive: true - } - ] - } -} diff --git a/src/main/runtime/rpc/methods/session-tabs.test.ts b/src/main/runtime/rpc/methods/session-tabs.test.ts index be61fc55edf..f295d2626da 100644 --- a/src/main/runtime/rpc/methods/session-tabs.test.ts +++ b/src/main/runtime/rpc/methods/session-tabs.test.ts @@ -4,6 +4,7 @@ import type { RpcRequest } from '../core' import type { OrcaRuntimeService } from '../../orca-runtime' import { SESSION_TAB_CLOSE_INTENT_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' import { SESSION_TAB_METHODS } from './session-tabs' +import { visibleSnapshot } from './session-tabs-snapshot.test-fixture' function makeRequest(method: string, params?: unknown): RpcRequest { return { id: 'req-1', authToken: 'tok', method, params } @@ -816,27 +817,3 @@ describe('session tab RPC methods', () => { ) }) }) - -function visibleSnapshot() { - return { - worktree: 'wt-1', - publicationEpoch: 'epoch-1', - snapshotVersion: 1, - activeGroupId: 'group-1', - activeTabId: 'tab-1::leaf-1', - activeTabType: 'terminal' as const, - tabGroups: [{ id: 'group-1', activeTabId: 'tab-1', tabOrder: ['tab-1'] }], - tabs: [ - { - type: 'terminal' as const, - id: 'tab-1::leaf-1', - parentTabId: 'tab-1', - leafId: 'leaf-1', - title: 'Terminal', - status: 'ready' as const, - terminal: 'pty-1', - isActive: true - } - ] - } -} diff --git a/src/main/runtime/rpc/methods/session-tabs.ts b/src/main/runtime/rpc/methods/session-tabs.ts index 34d50a2a76b..6296c462a39 100644 --- a/src/main/runtime/rpc/methods/session-tabs.ts +++ b/src/main/runtime/rpc/methods/session-tabs.ts @@ -28,7 +28,7 @@ export const SESSION_TAB_METHODS: RpcAnyMethod[] = [ await runtime.listMobileSessionTabs(params.worktree, pairedDeviceId), clientKind, clientCapabilities, - clientKind === 'mobile' ? isStructuredNativeChatEnabled(runtime) : undefined + isStructuredNativeChatEnabled(runtime) ) } }), @@ -121,7 +121,7 @@ export const SESSION_TAB_METHODS: RpcAnyMethod[] = [ initial, clientKind, clientCapabilities, - clientKind === 'mobile' ? isStructuredNativeChatEnabled(runtime) : undefined + isStructuredNativeChatEnabled(runtime) ) }) initialized = true @@ -137,7 +137,7 @@ export const SESSION_TAB_METHODS: RpcAnyMethod[] = [ snapshot, clientKind, clientCapabilities, - clientKind === 'mobile' ? isStructuredNativeChatEnabled(runtime) : undefined + isStructuredNativeChatEnabled(runtime) ) }) } diff --git a/src/main/runtime/rpc/methods/structured-agent-session-admission.test.ts b/src/main/runtime/rpc/methods/structured-agent-session-admission.test.ts new file mode 100644 index 00000000000..de62b6b5b52 --- /dev/null +++ b/src/main/runtime/rpc/methods/structured-agent-session-admission.test.ts @@ -0,0 +1,112 @@ +// Admission can be revoked while sessions are still open: the host setting is turned off with a +// chat already on screen. What the caller may still do to that chat is the rule this suite pins. + +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' +import { + ADMISSION_METHODS, + CLEANUP_METHODS +} from './structured-agent-session-gate-classification.test-fixture' +import { + call, + clearStructuredHostStub, + envelope, + hostCalls, + installStructuredHostStub, + SESSION, + STRUCTURED_CLIENT +} from './structured-agent-session-rpc.test-fixture' + +beforeEach(() => { + installStructuredHostStub() +}) + +afterEach(() => { + clearStructuredHostStub() +}) + +describe('admission revoked while a session is still open', () => { + // The host setting is admission control. Turning it off must not strand a chat that was opened + // while it was on: the pane is still mounted, so its close has to land. + const SETTING_OFF = { getClientSettings: () => ({ experimentalStructuredNativeChat: false }) } + + it.each(CLEANUP_METHODS)( + 'still serves $method after the host setting is turned off', + async ({ method, params, hostCall }) => { + const response = await call(method, params, STRUCTURED_CLIENT, SETTING_OFF) + + expect(response).toMatchObject({ ok: true }) + // `unsubscribe` retires runtime-owned subscriptions rather than calling the host, so its + // result payload is the observable effect. + if (hostCall === 'unsubscribe') { + expect(response).toMatchObject({ result: { unsubscribed: true } }) + } else { + expect(hostCalls[hostCall]).toHaveBeenCalled() + } + } + ) + + it('stops the provider child when closing a chat the setting no longer admits', async () => { + const response = await call('agentSession.close', { sessionId: SESSION }, STRUCTURED_CLIENT, { + ...SETTING_OFF + }) + + expect(response).toMatchObject({ ok: true, result: { ok: true } }) + expect(hostCalls.close).toHaveBeenCalledWith(SESSION) + // The durable tab has to be retired too, or the chat comes back on the next sync. + expect(hostCalls.setSessionTabVisibility).toHaveBeenCalledWith(SESSION, false) + }) + + it('cancels an in-flight turn the setting no longer admits', async () => { + const response = await call( + 'agentSession.cancel', + { envelope: envelope(), turnId: 'turn-1' }, + STRUCTURED_CLIENT, + SETTING_OFF + ) + + expect(response).toMatchObject({ ok: true }) + expect(hostCalls.cancel).toHaveBeenCalledOnce() + }) + + it.each(['runtime', 'mobile'] as const)( + 'lets a %s client close a chat it already owns', + async (clientKind) => { + const response = await call( + 'agentSession.close', + { sessionId: SESSION }, + { clientKind, clientCapabilities: [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY] }, + SETTING_OFF + ) + + expect(response).toMatchObject({ ok: true }) + expect(hostCalls.close).toHaveBeenCalledWith(SESSION) + } + ) + + it('lets an in-process caller close, which is how terminal disposal retires a chat', async () => { + const response = await call( + 'agentSession.close', + { sessionId: SESSION }, + undefined, + SETTING_OFF + ) + + expect(response).toMatchObject({ ok: true }) + expect(hostCalls.close).toHaveBeenCalledWith(SESSION) + }) + + it.each(ADMISSION_METHODS)( + 'keeps $method refused once the setting is off', + async ({ method, params }) => { + const response = await call(method, params, STRUCTURED_CLIENT, SETTING_OFF) + + // Asserting the gate's own code, not merely `ok: false`: a params-validation failure would + // pass a bare falsy check and hide a gate that had stopped refusing. + expect(response).toMatchObject({ + ok: false, + error: { message: expect.stringContaining('structured_agent_session_unsupported') } + }) + } + ) +}) diff --git a/src/main/runtime/rpc/methods/structured-agent-session-gate-classification.test-fixture.ts b/src/main/runtime/rpc/methods/structured-agent-session-gate-classification.test-fixture.ts new file mode 100644 index 00000000000..07616a9d843 --- /dev/null +++ b/src/main/runtime/rpc/methods/structured-agent-session-gate-classification.test-fixture.ts @@ -0,0 +1,79 @@ +// The method-to-gate classification from `structured-agent-session-gate.ts`, as a table the +// suites iterate. Adding an `agentSession.*` method means adding it to exactly one of these. + +import { + attachParams, + envelope, + sendParams, + SESSION +} from './structured-agent-session-rpc.test-fixture' +import { computeAgentSessionPayloadFingerprint } from '../../../../shared/agent-session-mutation-envelope' + +/** Stops or retires work the caller already owns, so admission may already have been revoked. */ +export const CLEANUP_METHODS = [ + { + method: 'agentSession.close', + params: { sessionId: SESSION }, + hostCall: 'close' + }, + { + method: 'agentSession.cancel', + params: { envelope: envelope(), turnId: 'turn-1' }, + hostCall: 'cancel' + }, + { + method: 'agentSession.release', + params: { sessionId: SESSION, holderId: 'surface-1' }, + hostCall: 'release' + }, + { + method: 'agentSession.unsubscribe', + params: { sessionId: SESSION }, + hostCall: 'unsubscribe' + } +] as const + +/** Starts, extends, retains or reads work, so every one stays refused once the setting is off. */ +export const ADMISSION_METHODS = [ + { method: 'agentSession.createSupport', params: { worktree: 'id:workspace-1', agent: 'codex' } }, + { + method: 'agentSession.create', + params: { + envelope: envelope({ + expectedRuntimeFence: null, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method: 'agentSession.create', + sessionId: SESSION, + fields: { worktree: 'id:workspace-1', agent: 'codex' } + }) + }), + worktree: 'id:workspace-1', + agent: 'codex' + } + }, + { method: 'agentSession.ensure', params: attachParams() }, + { method: 'agentSession.send', params: sendParams() }, + { + method: 'agentSession.respondToApproval', + params: { envelope: envelope(), itemId: 'item-1', expectedRevision: 1, optionId: 'allow' } + }, + { + method: 'agentSession.respondToQuestion', + params: { envelope: envelope(), itemId: 'item-1', expectedRevision: 1, optionId: 'yes' } + }, + { + method: 'agentSession.setOption', + params: { envelope: envelope(), key: 'model', value: 'gpt-live' } + }, + { + method: 'agentSession.requestHandoff', + params: { envelope: envelope(), direction: 'to-tui', mode: 'now' } + }, + { method: 'agentSession.handoffStatus', params: { sessionId: SESSION } }, + { method: 'agentSession.options', params: { sessionId: SESSION } }, + { method: 'agentSession.history', params: { sessionId: SESSION, direction: 'tail' } }, + { method: 'agentSession.subscribe', params: { sessionId: SESSION } }, + { method: 'agentSession.hold', params: { sessionId: SESSION, holderId: 'surface-1' } }, + { method: 'agentSession.reveal', params: { sessionId: SESSION } }, + { method: 'agentSession.subscribeStatus', params: null } +] as const diff --git a/src/main/runtime/rpc/methods/structured-agent-session-gate.ts b/src/main/runtime/rpc/methods/structured-agent-session-gate.ts index d918614ed47..de83820e9c3 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-gate.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-gate.ts @@ -12,7 +12,10 @@ import { getStructuredAgentSessionHost } from '../../../native-chat/agent-sessio import type { StructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-host' import type { StructuredAgentSessionCaller } from '../../../native-chat/agent-session-wire/structured-agent-session-host-types' import type { RpcContext } from '../core' -import { supportsStructuredAgentSessions } from './structured-agent-session-policy' +import { + supportsStructuredAgentSessionCapability, + supportsStructuredAgentSessions +} from './structured-agent-session-policy' /** * In-process callers are the same build as the host, so they carry no negotiated @@ -37,6 +40,39 @@ export function requireStructuredHost(ctx: RpcContext): StructuredAgentSessionHo return host } +/** + * WHICH GATE DOES A NEW `agentSession.*` METHOD GET? + * + * The host setting is admission control, and admission can be revoked while sessions are still + * open. So the surface splits by what a method does to work in flight, not by how dangerous it + * sounds: + * + * - Starts, extends, retains or reads work -> `requireStructuredHost`. Revoked admission means + * no new turns, no new holds, no new reads. create, send, ensure, setOption, requestHandoff, + * subscribe, hold, reveal, history, options and the status stream all live here. + * - Stops or retires work the caller already owns -> `requireStructuredCleanupHost`. close, + * cancel, unsubscribe and release live here. + * + * Cleanup keeps working after the setting is turned off because the alternative strands the user: + * a session opened while the setting was on stays open, and refusing its close leaves a chat with + * a live provider child that its own owner can no longer shut down. Stopping is never the thing + * the policy exists to prevent. + * + * Cleanup is not an escape hatch. It still demands the negotiated wire capability, so a client + * that never advertised the surface still cannot see it, and it never creates a host — it can + * only retire what already exists. + */ +export function requireStructuredCleanupHost(ctx: RpcContext): StructuredAgentSessionHost { + if (!supportsStructuredAgentSessionCapability(ctx)) { + throw new Error('structured_agent_session_unsupported') + } + const host = getStructuredAgentSessionHost() + if (!host) { + throw new Error('structured_agent_session_unsupported') + } + return host +} + /** Builds the host for the calls that address a session by durable record rather than by live * state: attach, which is the only way a session comes into being, plus hold and reveal, which * each reach for a record on disk this process may not have opened yet. Every other method diff --git a/src/main/runtime/rpc/methods/structured-agent-session-hold.test.ts b/src/main/runtime/rpc/methods/structured-agent-session-hold.test.ts index 61bb3849bc7..4e6dfdf45bf 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-hold.test.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-hold.test.ts @@ -41,6 +41,7 @@ let runtime: OrcaRuntimeService let dispatcher: RpcDispatcher let closeSession: Mock> let requests = 0 +let structuredNativeChatEnabled = true async function call(method: string, params: unknown): Promise { const replies: RpcResponse[] = [] @@ -57,6 +58,7 @@ beforeEach(async () => { root = await mkdtemp(join(tmpdir(), 'orca-hold-wire-')) resetHostTestOperationIds() requests = 0 + structuredNativeChatEnabled = true closeSession = vi.fn(async () => true) store = await AgentSessionRecordStore.open({ directory: join(root, 'store'), hostId: 'local' }) host = new StructuredAgentSessionHost({ @@ -86,6 +88,13 @@ beforeEach(async () => { }) setStructuredAgentSessionHost(host) runtime = new OrcaRuntimeService() + // The structured surface is settings-gated for every caller, in-process included. + vi.spyOn(runtime, 'getClientSettings').mockImplementation( + () => + ({ experimentalStructuredNativeChat: structuredNativeChatEnabled }) as ReturnType< + OrcaRuntimeService['getClientSettings'] + > + ) dispatcher = new RpcDispatcher({ runtime, methods: STRUCTURED_AGENT_SESSION_METHODS }) expect(await host.attach({ callerKey: 'client-1' }, hostTestAttachParams(null))).toMatchObject({ ok: true @@ -121,6 +130,23 @@ describe('a client that holds a session', () => { expect(closeSession).toHaveBeenCalledWith(SESSION) }) + it('releases its hold and cleanup after the setting is disabled', async () => { + const release = vi.spyOn(host, 'release') + await call('agentSession.hold', { sessionId: SESSION, holderId: 'chat-1' }) + structuredNativeChatEnabled = false + + expect( + await call('agentSession.release', { sessionId: SESSION, holderId: 'chat-1' }) + ).toMatchObject({ ok: true }) + const releaseCallsAfterRpc = release.mock.calls.length + runtime.cleanupSubscriptionsForConnection(CONNECTION) + + expect(releaseCallsAfterRpc).toBe(2) + expect(release).toHaveBeenCalledTimes(releaseCallsAfterRpc) + await vi.waitFor(() => expect(host.hasSession(SESSION)).toBe(false)) + expect(closeSession).toHaveBeenCalledWith(SESSION) + }) + it('does not report success when no provider child can be acquired', async () => { const response = await call('agentSession.hold', { sessionId: 'session-missing', @@ -213,6 +239,31 @@ describe('a client that disappears without cleanup', () => { expect(closeSession).toHaveBeenCalledWith(SESSION) }) + it('unsubscribes and releases stream retention after the setting is disabled', async () => { + await dispatcher.dispatchStreaming( + { + id: 'stream-disabled-cleanup', + authToken: 'token', + method: 'agentSession.subscribe', + params: { sessionId: SESSION } + }, + () => {}, + CLIENT + ) + expect(host.isHeld(SESSION)).toBe(true) + structuredNativeChatEnabled = false + + expect( + await call('agentSession.unsubscribe', { + sessionId: SESSION, + subscriptionId: 'stream-disabled-cleanup' + }) + ).toMatchObject({ ok: true }) + + await vi.waitFor(() => expect(host.hasSession(SESSION)).toBe(false)) + expect(closeSession).toHaveBeenCalledWith(SESSION) + }) + it('does not let a stream alone resume a released session', async () => { await host.close(SESSION) expect(host.hasSession(SESSION)).toBe(false) diff --git a/src/main/runtime/rpc/methods/structured-agent-session-hold.ts b/src/main/runtime/rpc/methods/structured-agent-session-hold.ts index 346082bd576..280804711e6 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-hold.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-hold.ts @@ -12,6 +12,7 @@ import { defineMethod, type RpcAnyMethod, type RpcContext } from '../core' import { ensureStructuredHostInstalled, + requireStructuredCleanupHost, requireStructuredHost } from './structured-agent-session-gate' import { HoldParams } from './structured-agent-session-schemas' @@ -53,7 +54,7 @@ export const STRUCTURED_AGENT_SESSION_HOLD_METHODS: RpcAnyMethod[] = [ name: 'agentSession.release', params: HoldParams, handler: async (params, ctx) => { - const host = requireStructuredHost(ctx) + const host = requireStructuredCleanupHost(ctx) const holderKey = holderKeyFor(ctx, params.holderId) host.release(params.sessionId, holderKey) // Retires the backstop too; its release is a no-op against a holder already gone. diff --git a/src/main/runtime/rpc/methods/structured-agent-session-policy.test.ts b/src/main/runtime/rpc/methods/structured-agent-session-policy.test.ts new file mode 100644 index 00000000000..6c765119375 --- /dev/null +++ b/src/main/runtime/rpc/methods/structured-agent-session-policy.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, it } from 'vitest' +import { STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' +import type { OrcaRuntimeService } from '../../orca-runtime' +import { supportsStructuredAgentSessions } from './structured-agent-session-policy' + +function runtimeWithSetting( + experimentalStructuredNativeChat: boolean +): Pick { + return { + getClientSettings: () => ({ experimentalStructuredNativeChat }) + } as unknown as Pick +} + +const CAPABLE = [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY] + +/** Every caller shape that reaches the policy: desktop renderer, paired phone, in-process. */ +const CALLERS = [ + { name: 'desktop renderer', clientKind: 'runtime' as const, clientCapabilities: CAPABLE }, + { name: 'paired mobile', clientKind: 'mobile' as const, clientCapabilities: CAPABLE }, + { name: 'in-process', clientKind: undefined, clientCapabilities: undefined } +] + +describe('supportsStructuredAgentSessions', () => { + it.each([true, false])('admits every caller alike when the setting is %s', (enabled) => { + const decisions = CALLERS.map((caller) => + supportsStructuredAgentSessions({ + clientKind: caller.clientKind, + clientCapabilities: caller.clientCapabilities, + runtime: runtimeWithSetting(enabled) + }) + ) + + expect(decisions).toEqual([enabled, enabled, enabled]) + }) + + it('admits a capability-less in-process caller, which negotiates nothing', () => { + expect( + supportsStructuredAgentSessions({ + clientKind: undefined, + clientCapabilities: undefined, + runtime: runtimeWithSetting(true) + }) + ).toBe(true) + }) + + it('still refuses a remote client that did not advertise the capability', () => { + for (const clientKind of ['runtime', 'mobile'] as const) { + expect( + supportsStructuredAgentSessions({ + clientKind, + clientCapabilities: [], + runtime: runtimeWithSetting(true) + }) + ).toBe(false) + } + }) + + it('leaves desktop launch admission unchanged, because launches require the setting anyway', () => { + // `agent-launch-routing.ts` refuses to route a structured launch unless + // `experimentalStructuredNativeChat` is on, so the only state a desktop launch can + // reach the host in is setting-on — which admits exactly as it did before. + expect( + supportsStructuredAgentSessions({ + clientKind: 'runtime', + clientCapabilities: CAPABLE, + runtime: runtimeWithSetting(true) + }) + ).toBe(true) + }) + + it('reads the setting from the caller-supplied value when no runtime is available', () => { + expect( + supportsStructuredAgentSessions({ + clientKind: 'runtime', + clientCapabilities: CAPABLE, + structuredNativeChatEnabled: true + }) + ).toBe(true) + expect( + supportsStructuredAgentSessions({ + clientKind: 'runtime', + clientCapabilities: CAPABLE, + structuredNativeChatEnabled: false + }) + ).toBe(false) + }) + + it('treats an unreadable settings store as off rather than admitting', () => { + expect( + supportsStructuredAgentSessions({ + clientKind: 'runtime', + clientCapabilities: CAPABLE, + runtime: { + getClientSettings: () => { + throw new Error('settings unavailable') + } + } as unknown as Pick + }) + ).toBe(false) + }) +}) diff --git a/src/main/runtime/rpc/methods/structured-agent-session-policy.ts b/src/main/runtime/rpc/methods/structured-agent-session-policy.ts index 4fe38474ec6..46a1ee34c45 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-policy.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-policy.ts @@ -20,18 +20,24 @@ export function isStructuredNativeChatEnabled( } } -export function supportsStructuredAgentSessions(context: StructuredPolicyContext): boolean { - if (context.clientKind === undefined) { - return true - } - const hasCapability = +export function supportsStructuredAgentSessionCapability( + context: Pick +): boolean { + return ( + context.clientKind === undefined || context.clientCapabilities?.includes(STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY) === true - if (!hasCapability) { + ) +} + +/** + * One rule for every caller. The host setting is policy and applies to desktop, mobile and + * in-process callers alike; the negotiated capability is a wire term, so it is asked of remote + * clients only — in-process callers are the same build as the host and never negotiate one. + */ +export function supportsStructuredAgentSessions(context: StructuredPolicyContext): boolean { + if (!supportsStructuredAgentSessionCapability(context)) { return false } - if (context.clientKind !== 'mobile') { - return true - } return ( context.structuredNativeChatEnabled === true || (context.runtime ? isStructuredNativeChatEnabled(context.runtime) : false) @@ -41,7 +47,8 @@ export function supportsStructuredAgentSessions(context: StructuredPolicyContext export function structuredNativeChatProjectionEnabled(args: { clientKind: 'mobile' | 'runtime' | undefined clientCapabilities: readonly RuntimeCapability[] | undefined - structuredNativeChatEnabled?: boolean + // Required so no call site can silently project as if the host setting were off. + structuredNativeChatEnabled: boolean }): boolean { return supportsStructuredAgentSessions(args) } diff --git a/src/main/runtime/rpc/methods/structured-agent-session-precommit-refusal.test.ts b/src/main/runtime/rpc/methods/structured-agent-session-precommit-refusal.test.ts index 1a62045c85b..f34ecb5d6cd 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-precommit-refusal.test.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-precommit-refusal.test.ts @@ -71,6 +71,9 @@ async function create( ): Promise { const runtime = { getRuntimeId: () => 'runtime-1', + // The structured surface is settings-gated for every caller; these fixtures probe the + // pre-commit boundary, which only runs once the gate admits the call. + getClientSettings: () => ({ experimentalStructuredNativeChat: true }), registerSubscriptionCleanup: vi.fn(), cleanupSubscription: vi.fn(), cleanupSubscriptionsByPrefix: vi.fn(), diff --git a/src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts b/src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts new file mode 100644 index 00000000000..360af5d4d31 --- /dev/null +++ b/src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts @@ -0,0 +1,270 @@ +// The `agentSession.*` dispatcher harness, shared by the suites that exercise the wire +// boundary. `hostCalls` and `runtimeCalls` keep one identity for the process and are +// repopulated per test, so a suite can read `hostCalls.close` without re-importing it. + +import { vi } from 'vitest' +import type { AgentJournalRenderItem } from '../../../../shared/agent-session-journal-types' +import type { AgentSessionJournal } from '../../../native-chat/agent-session-journal/journal-store' +import type { StructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-host' +import { setStructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-registry' +import { + StructuredAgentSessionStatusFeed, + type StructuredAgentSessionStatusSubscriber +} from '../../../native-chat/agent-session-wire/structured-agent-session-status-feed' +import type { OrcaRuntimeService } from '../../orca-runtime' +import { STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' +import type { RpcRequest, RpcResponse } from '../core' +import { RpcDispatcher } from '../dispatcher' +import { STRUCTURED_AGENT_SESSION_METHODS } from './structured-agent-session' + +export const SESSION = 'session-alpha' +export const FINGERPRINT = 'f'.repeat(64) +export const OPERATION = '1800000000000-00000000000000000000000000000001' + +export function envelope(overrides: Record = {}) { + return { + sessionId: SESSION, + clientOperationId: OPERATION, + expectedRuntimeFence: 1, + payloadFingerprint: FINGERPRINT, + ...overrides + } +} + +export function sendParams(overrides: Record = {}) { + return { + envelope: envelope(), + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'hi' }] }, + ...overrides + } +} + +export function attachParams(overrides: Record = {}) { + return { + envelope: envelope({ expectedRuntimeFence: null }), + location: { + executionHostId: 'local', + wslDistro: null, + workspaceId: 'workspace-1', + workspaceKind: 'git-worktree' + }, + provider: 'codex', + agent: 'codex', + accountHome: { variable: 'CODEX_HOME', path: '/home/dev/.codex' }, + runtimeKind: 'native', + providerHandle: { kind: 'codex', threadId: 'thread-1' }, + ...overrides + } +} + +function request(method: string, params: unknown): RpcRequest { + return { id: 'request-1', authToken: 'token', method, params } +} + +export const hostCalls: Record> = {} +export const runtimeCalls: Record> = {} + +function reset(record: Record>): void { + for (const key of Object.keys(record)) { + delete record[key] + } +} + +export const STATUS_SESSION = 'session-status' +export const STATUS_ITEMS: AgentJournalRenderItem[] = [ + { + itemId: 'user-1', + sequence: 1, + revision: 1, + observedAt: 1, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'write a poem' }] } + }, + { + itemId: 'turn-1', + sequence: 2, + revision: 1, + observedAt: 2, + body: { kind: 'status', text: 'Working', turnLifecycle: { turnId: 'turn-1', state: 'running' } } + } +] + +/** One indexed session over a journal that reads back fixed items; the projection is real. */ +function statusFeed(): StructuredAgentSessionStatusFeed { + return new StructuredAgentSessionStatusFeed({ + sessions: new Map([ + [ + STATUS_SESSION, + { + journal: { + isReadOnly: false, + lastActivityAt: () => 2, + snapshot: () => ({ items: STATUS_ITEMS }) + } as unknown as AgentSessionJournal, + params: { location: { workspaceId: 'workspace-1' }, provider: 'codex' as const } + } + ] + ]), + getRecord: () => null, + now: () => 1_000 + }) +} + +export function hostStub(): StructuredAgentSessionHost { + reset(hostCalls) + Object.assign(hostCalls, { + attach: vi.fn(async () => ({ + ok: true, + replayed: false, + fence: 1, + cursor: { epoch: 'epoch-a', sequence: 0 }, + value: { + sessionId: SESSION, + fence: 1, + page: { + sessionId: SESSION, + epoch: 'epoch-a', + direction: 'tail', + items: [], + removedItemIds: [], + submissions: [], + window: { + oldest: null, + newest: null, + nextCursor: { epoch: 'epoch-a', sequence: 0 } + }, + liveCursor: { epoch: 'epoch-a', sequence: 0 }, + hasOlder: false, + hasNewer: false + }, + unconfirmedClientMessageIds: [] + } + })), + send: vi.fn(async () => ({ ok: true, replayed: false })), + cancel: vi.fn(async () => ({ ok: true, replayed: false })), + close: vi.fn(async () => undefined), + revealSession: vi.fn(async () => ({ + sessionId: SESSION, + workspaceId: 'workspace-1', + agent: 'codex' as const, + readable: true + })), + setSessionTabVisibility: vi.fn(async () => undefined), + respondToPrompt: vi.fn(async () => ({ ok: true, replayed: false })), + setOption: vi.fn(async () => ({ ok: true, replayed: false })), + requestHandoff: vi.fn(async () => ({ + ok: true, + replayed: false, + fence: 1, + cursor: { epoch: 'epoch-a', sequence: 0 }, + value: { + status: { + owner: 'native', + direction: null, + phase: 'idle', + stage: null, + operationId: null + } + } + })), + supportsCreate: vi.fn(() => true), + handoffStatus: vi.fn(async () => ({ owner: 'native' })), + readOptions: vi.fn(async () => ({ + models: [{ id: 'gpt-live', label: 'GPT Live', isDefault: true, efforts: [] }], + current: { model: 'gpt-live' } + })), + history: vi.fn(() => ({ ok: true, page: { items: [] } })), + subscribe: vi.fn(() => () => undefined), + // A real feed, so the snapshot this method hands back is a genuine projection rather + // than a shape the stub restated. + subscribeStatus: vi.fn((subscriber: StructuredAgentSessionStatusSubscriber) => + statusFeed().subscribe(subscriber) + ), + unsubscribe: vi.fn(), + release: vi.fn() + }) + return hostCalls as unknown as StructuredAgentSessionHost +} + +export function dispatcher(runtimeOverrides: Record = {}): RpcDispatcher { + reset(runtimeCalls) + Object.assign(runtimeCalls, { + getStructuredAgentSessionCreateSupport: vi.fn(async () => ({ supported: true })), + resolveStructuredAgentSessionCreateIntent: vi.fn(async (params) => ({ + envelope: params.envelope, + location: { + executionHostId: 'local', + wslDistro: null, + workspaceId: 'workspace-1', + workspaceKind: 'git-worktree' + }, + provider: params.agent, + agent: params.agent, + accountHome: { + variable: params.agent === 'claude' ? 'CLAUDE_CONFIG_DIR' : 'CODEX_HOME', + path: params.agent === 'claude' ? '/host/.claude' : '/host/.codex' + }, + options: + params.agent === 'claude' + ? { model: 'opus', effort: 'high' } + : { model: 'gpt-5.6-sol', effort: 'medium' }, + runtimeKind: 'native' + })), + publishStructuredAgentSessionTab: vi.fn() + }) + const runtime = { + getRuntimeId: () => 'runtime-1', + getClientSettings: () => ({ experimentalStructuredNativeChat: true }), + registerSubscriptionCleanup: vi.fn(), + cleanupSubscription: vi.fn(), + cleanupSubscriptionsByPrefix: vi.fn(), + ...runtimeCalls, + ...runtimeOverrides + } + return new RpcDispatcher({ + runtime: runtime as unknown as OrcaRuntimeService, + methods: STRUCTURED_AGENT_SESSION_METHODS + }) +} + +/** The reply path is the only one that carries a client's negotiated identity, + * which is exactly what the capability gate reads. */ +export async function call( + method: string, + params: unknown, + client?: { + clientId?: string + clientKind?: 'mobile' | 'runtime' + clientCapabilities?: string[] + }, + runtimeOverrides: Record = {} +): Promise { + const replies: RpcResponse[] = [] + await dispatcher(runtimeOverrides).dispatchStreaming( + request(method, params), + (raw) => replies.push(JSON.parse(raw) as RpcResponse), + client + ) + const first = replies[0] + if (!first) { + throw new Error(`no reply for ${method}`) + } + return first +} + +export const STRUCTURED_CLIENT = { + clientKind: 'runtime' as const, + clientCapabilities: [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY] +} +export const STRUCTURED_MOBILE_CLIENT = { + clientKind: 'mobile' as const, + clientCapabilities: [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY] +} + +/** Every suite wants the same lifecycle: a fresh stub per test, no host left installed. */ +export function installStructuredHostStub(): void { + setStructuredAgentSessionHost(hostStub()) +} + +export function clearStructuredHostStub(): void { + setStructuredAgentSessionHost(null) +} diff --git a/src/main/runtime/rpc/methods/structured-agent-session.test.ts b/src/main/runtime/rpc/methods/structured-agent-session.test.ts index 5a38ae4ce2d..13e2383e667 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session.test.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session.test.ts @@ -1,15 +1,8 @@ // The wire boundary: who may see `agentSession.*` at all, and what shapes it -// accepts once they can. +// accepts once they can. The dispatcher harness lives in the shared fixture. import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import type { AgentJournalRenderItem } from '../../../../shared/agent-session-journal-types' -import type { AgentSessionJournal } from '../../../native-chat/agent-session-journal/journal-store' -import type { StructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-host' import { setStructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-registry' -import { - StructuredAgentSessionStatusFeed, - type StructuredAgentSessionStatusSubscriber -} from '../../../native-chat/agent-session-wire/structured-agent-session-status-feed' import { RUNTIME_CAPABILITIES, RUNTIME_PROTOCOL_VERSION, @@ -17,252 +10,31 @@ import { STRUCTURED_AGENT_SESSION_REVEAL_RUNTIME_CAPABILITY, STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' -import type { OrcaRuntimeService } from '../../orca-runtime' -import type { RpcRequest, RpcResponse } from '../core' -import { RpcDispatcher } from '../dispatcher' +import { computeAgentSessionPayloadFingerprint } from '../../../../shared/agent-session-mutation-envelope' import { ALL_RPC_METHODS } from './index' import { STRUCTURED_AGENT_SESSION_METHODS } from './structured-agent-session' -import { computeAgentSessionPayloadFingerprint } from '../../../../shared/agent-session-mutation-envelope' - -const SESSION = 'session-alpha' -const FINGERPRINT = 'f'.repeat(64) -const OPERATION = '1800000000000-00000000000000000000000000000001' - -function envelope(overrides: Record = {}) { - return { - sessionId: SESSION, - clientOperationId: OPERATION, - expectedRuntimeFence: 1, - payloadFingerprint: FINGERPRINT, - ...overrides - } -} - -function sendParams(overrides: Record = {}) { - return { - envelope: envelope(), - body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'hi' }] }, - ...overrides - } -} - -function attachParams(overrides: Record = {}) { - return { - envelope: envelope({ expectedRuntimeFence: null }), - location: { - executionHostId: 'local', - wslDistro: null, - workspaceId: 'workspace-1', - workspaceKind: 'git-worktree' - }, - provider: 'codex', - agent: 'codex', - accountHome: { variable: 'CODEX_HOME', path: '/home/dev/.codex' }, - runtimeKind: 'native', - providerHandle: { kind: 'codex', threadId: 'thread-1' }, - ...overrides - } -} - -function request(method: string, params: unknown): RpcRequest { - return { id: 'request-1', authToken: 'token', method, params } -} - -let hostCalls: Record> -let runtimeCalls: Record> - -const STATUS_SESSION = 'session-status' -const STATUS_ITEMS: AgentJournalRenderItem[] = [ - { - itemId: 'user-1', - sequence: 1, - revision: 1, - observedAt: 1, - body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'write a poem' }] } - }, - { - itemId: 'turn-1', - sequence: 2, - revision: 1, - observedAt: 2, - body: { kind: 'status', text: 'Working', turnLifecycle: { turnId: 'turn-1', state: 'running' } } - } -] - -/** One indexed session over a journal that reads back fixed items; the projection is real. */ -function statusFeed(): StructuredAgentSessionStatusFeed { - return new StructuredAgentSessionStatusFeed({ - sessions: new Map([ - [ - STATUS_SESSION, - { - journal: { - isReadOnly: false, - lastActivityAt: () => 2, - snapshot: () => ({ items: STATUS_ITEMS }) - } as unknown as AgentSessionJournal, - params: { location: { workspaceId: 'workspace-1' }, provider: 'codex' as const } - } - ] - ]), - getRecord: () => null, - now: () => 1_000 - }) -} - -function hostStub(): StructuredAgentSessionHost { - hostCalls = { - attach: vi.fn(async () => ({ - ok: true, - replayed: false, - fence: 1, - cursor: { epoch: 'epoch-a', sequence: 0 }, - value: { - sessionId: SESSION, - fence: 1, - page: { - sessionId: SESSION, - epoch: 'epoch-a', - direction: 'tail', - items: [], - removedItemIds: [], - submissions: [], - window: { - oldest: null, - newest: null, - nextCursor: { epoch: 'epoch-a', sequence: 0 } - }, - liveCursor: { epoch: 'epoch-a', sequence: 0 }, - hasOlder: false, - hasNewer: false - }, - unconfirmedClientMessageIds: [] - } - })), - send: vi.fn(async () => ({ ok: true, replayed: false })), - cancel: vi.fn(async () => ({ ok: true, replayed: false })), - close: vi.fn(async () => undefined), - revealSession: vi.fn(async () => ({ - sessionId: SESSION, - workspaceId: 'workspace-1', - agent: 'codex' as const, - readable: true - })), - setSessionTabVisibility: vi.fn(async () => undefined), - respondToPrompt: vi.fn(async () => ({ ok: true, replayed: false })), - setOption: vi.fn(async () => ({ ok: true, replayed: false })), - requestHandoff: vi.fn(async () => ({ - ok: true, - replayed: false, - fence: 1, - cursor: { epoch: 'epoch-a', sequence: 0 }, - value: { - status: { - owner: 'native', - direction: null, - phase: 'idle', - stage: null, - operationId: null - } - } - })), - supportsCreate: vi.fn(() => true), - handoffStatus: vi.fn(async () => ({ owner: 'native' })), - readOptions: vi.fn(async () => ({ - models: [{ id: 'gpt-live', label: 'GPT Live', isDefault: true, efforts: [] }], - current: { model: 'gpt-live' } - })), - history: vi.fn(() => ({ ok: true, page: { items: [] } })), - subscribe: vi.fn(() => () => undefined), - // A real feed, so the snapshot this method hands back is a genuine projection rather - // than a shape the stub restated. - subscribeStatus: vi.fn((subscriber: StructuredAgentSessionStatusSubscriber) => - statusFeed().subscribe(subscriber) - ), - unsubscribe: vi.fn() - } - return hostCalls as unknown as StructuredAgentSessionHost -} - -function dispatcher(runtimeOverrides: Record = {}): RpcDispatcher { - runtimeCalls = { - getStructuredAgentSessionCreateSupport: vi.fn(async () => ({ supported: true })), - resolveStructuredAgentSessionCreateIntent: vi.fn(async (params) => ({ - envelope: params.envelope, - location: { - executionHostId: 'local', - wslDistro: null, - workspaceId: 'workspace-1', - workspaceKind: 'git-worktree' - }, - provider: params.agent, - agent: params.agent, - accountHome: { - variable: params.agent === 'claude' ? 'CLAUDE_CONFIG_DIR' : 'CODEX_HOME', - path: params.agent === 'claude' ? '/host/.claude' : '/host/.codex' - }, - options: - params.agent === 'claude' - ? { model: 'opus', effort: 'high' } - : { model: 'gpt-5.6-sol', effort: 'medium' }, - runtimeKind: 'native' - })), - publishStructuredAgentSessionTab: vi.fn() - } - const runtime = { - getRuntimeId: () => 'runtime-1', - registerSubscriptionCleanup: vi.fn(), - cleanupSubscription: vi.fn(), - cleanupSubscriptionsByPrefix: vi.fn(), - ...runtimeCalls, - ...runtimeOverrides - } - return new RpcDispatcher({ - runtime: runtime as unknown as OrcaRuntimeService, - methods: STRUCTURED_AGENT_SESSION_METHODS - }) -} - -/** The reply path is the only one that carries a client's negotiated identity, - * which is exactly what the capability gate reads. */ -async function call( - method: string, - params: unknown, - client?: { - clientId?: string - clientKind?: 'mobile' | 'runtime' - clientCapabilities?: string[] - }, - runtimeOverrides: Record = {} -): Promise { - const replies: RpcResponse[] = [] - await dispatcher(runtimeOverrides).dispatchStreaming( - request(method, params), - (raw) => replies.push(JSON.parse(raw) as RpcResponse), - client - ) - const first = replies[0] - if (!first) { - throw new Error(`no reply for ${method}`) - } - return first -} - -const STRUCTURED_CLIENT = { - clientKind: 'runtime' as const, - clientCapabilities: [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY] -} -const STRUCTURED_MOBILE_CLIENT = { - clientKind: 'mobile' as const, - clientCapabilities: [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY] -} +import { CLEANUP_METHODS } from './structured-agent-session-gate-classification.test-fixture' +import { + attachParams, + call, + clearStructuredHostStub, + envelope, + hostCalls, + installStructuredHostStub, + runtimeCalls, + SESSION, + sendParams, + STATUS_SESSION, + STRUCTURED_CLIENT, + STRUCTURED_MOBILE_CLIENT +} from './structured-agent-session-rpc.test-fixture' beforeEach(() => { - setStructuredAgentSessionHost(hostStub()) + installStructuredHostStub() }) afterEach(() => { - setStructuredAgentSessionHost(null) + clearStructuredHostStub() }) describe('agentSession.reveal', () => { @@ -454,6 +226,41 @@ describe('capability gating', () => { expect(hostCalls.send).toHaveBeenCalledTimes(1) }) + it.each(CLEANUP_METHODS)( + 'keeps $method hidden from remote clients without the capability', + async ({ method, params, hostCall }) => { + const response = await call(method, params, { + clientKind: 'runtime', + clientCapabilities: [] + }) + + expect(response).toMatchObject({ + ok: false, + error: { message: expect.stringContaining('structured_agent_session_unsupported') } + }) + expect(hostCalls[hostCall]).not.toHaveBeenCalled() + } + ) + + it.each(CLEANUP_METHODS)( + 'does not install a host for cleanup-only method $method', + async ({ method, params }) => { + const ensureHost = vi.fn() + setStructuredAgentSessionHost(null) + + const response = await call(method, params, STRUCTURED_CLIENT, { + getClientSettings: () => ({ experimentalStructuredNativeChat: false }), + ensureStructuredAgentSessionHost: ensureHost + }) + + expect(response).toMatchObject({ + ok: false, + error: { message: expect.stringContaining('structured_agent_session_unsupported') } + }) + expect(ensureHost).not.toHaveBeenCalled() + } + ) + it('serves an in-process caller, which negotiates no capabilities at all', async () => { const response = await call('agentSession.send', sendParams()) expect(response).toMatchObject({ ok: true }) diff --git a/src/main/runtime/rpc/methods/structured-agent-session.ts b/src/main/runtime/rpc/methods/structured-agent-session.ts index f086fa7ed66..ba3a5d7d6a0 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session.ts @@ -14,6 +14,7 @@ import { defineMethod, defineStreamingMethod, type RpcAnyMethod, type RpcContext import { ensureStructuredHostInstalled as ensureHostInstalled, requireStructuredCapability, + requireStructuredCleanupHost, requireStructuredHost as requireHost, structuredCallerFor as callerFor, supportsStructuredSessions @@ -178,9 +179,10 @@ export const STRUCTURED_AGENT_SESSION_METHODS: RpcAnyMethod[] = [ handler: async (params, ctx) => requireHost(ctx).send(callerFor(ctx), params) }), defineMethod({ + // Stopping a turn, so it stays available after admission is revoked: see the gate's rule. name: 'agentSession.cancel', params: CancelParams, - handler: async (params, ctx) => requireHost(ctx).cancel(callerFor(ctx), params) + handler: async (params, ctx) => requireStructuredCleanupHost(ctx).cancel(callerFor(ctx), params) }), defineMethod({ // Releasing a chat view, not ending a conversation: the record and journal stay on disk so the @@ -188,7 +190,9 @@ export const STRUCTURED_AGENT_SESSION_METHODS: RpcAnyMethod[] = [ name: 'agentSession.close', params: OptionsParams, handler: async (params, ctx) => { - const host = requireHost(ctx) + // Cleanup gate: turning the host setting off must not strand an open chat whose owner can + // then never close it. See the rule on `requireStructuredCleanupHost`. + const host = requireStructuredCleanupHost(ctx) // Terminal-disposal closes use this RPC without the session-tabs retirement RPC. if (typeof host.setSessionTabVisibility === 'function') { await host.setSessionTabVisibility(params.sessionId, false) @@ -284,7 +288,9 @@ export const STRUCTURED_AGENT_SESSION_METHODS: RpcAnyMethod[] = [ name: 'agentSession.unsubscribe', params: UnsubscribeParams, handler: async (params, ctx) => { - requireHost(ctx) + // Why: cleanup must stay available after the setting is disabled, so an admitted caller can + // retire resources it already owns; the base still comes from main's shared helper. + requireStructuredCleanupHost(ctx) const base = subscriptionBaseFor(ctx, params.sessionId) if (params.subscriptionId) { ctx.runtime.cleanupSubscription(`${base}:${params.subscriptionId}`) diff --git a/src/main/runtime/structured-agent-session-integration-replay.test.ts b/src/main/runtime/structured-agent-session-integration-replay.test.ts index e5baa032341..990aa293ee4 100644 --- a/src/main/runtime/structured-agent-session-integration-replay.test.ts +++ b/src/main/runtime/structured-agent-session-integration-replay.test.ts @@ -242,6 +242,7 @@ beforeEach(async () => { configuredCodexProfile = 'configured' const runtime = { getRuntimeId: () => 'runtime-1', + getClientSettings: () => ({ experimentalStructuredNativeChat: true }), getStructuredAgentSessionCreateSupport: async () => ({ supported: true }), resolveStructuredAgentSessionCreateIntent: async () => { const { diff --git a/src/main/runtime/structured-agent-session-integration.test.ts b/src/main/runtime/structured-agent-session-integration.test.ts index 2982a6530b2..aa14aaa5639 100644 --- a/src/main/runtime/structured-agent-session-integration.test.ts +++ b/src/main/runtime/structured-agent-session-integration.test.ts @@ -290,6 +290,7 @@ beforeEach(async () => { configuredCodexProfile = 'configured' const runtime = { getRuntimeId: () => 'runtime-1', + getClientSettings: () => ({ experimentalStructuredNativeChat: true }), getStructuredAgentSessionCreateSupport: async () => ({ supported: true }), resolveStructuredAgentSessionCreateIntent: async () => { const { 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 ef35eefc7f2..e939a479f58 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 @@ -262,6 +262,7 @@ function runtimeStub(): unknown { const cleanups = new Map void>() return { getRuntimeId: () => 'runtime-1', + getClientSettings: () => ({ experimentalStructuredNativeChat: true }), ensureStructuredAgentSessionHost: async () => undefined, getStructuredAgentSessionCreateSupport: async () => ({ supported: true }), resolveStructuredAgentSessionCreateIntent: async () => {