From c300913f902ac754b01ebb3d569ea2594cc9ab15 Mon Sep 17 00:00:00 2001 From: blade035 Date: Mon, 7 Sep 2026 09:27:13 +0300 Subject: [PATCH 1/9] fix(mobile): stop double-scaling commit timestamps in history rows (#17731) Co-authored-by: Claude Co-authored-by: Jinwoo-H --- .../MobileGitHistoryList.test.tsx | 18 +++++++++++++++++- .../source-control/mobile-git-history.test.ts | 19 ++++++++++--------- .../src/source-control/mobile-git-history.ts | 7 ++++--- src/shared/git-history-types.ts | 1 + src/shared/git-history.test.ts | 4 +++- 5 files changed, 35 insertions(+), 14 deletions(-) diff --git a/mobile/src/source-control/MobileGitHistoryList.test.tsx b/mobile/src/source-control/MobileGitHistoryList.test.tsx index b19a069c80f..c6d7a9895a5 100644 --- a/mobile/src/source-control/MobileGitHistoryList.test.tsx +++ b/mobile/src/source-control/MobileGitHistoryList.test.tsx @@ -27,11 +27,24 @@ vi.mock('react-native', () => ({ vi.mock('lucide-react-native', () => ({ ChevronDown: 'ChevronDown', ChevronRight: 'ChevronRight' })) vi.mock('../transport/client-context', () => ({ useForceReconnect: () => vi.fn() })) +// Captured at module scope: the list renders rows against Date.now() a few ms later, +// so a 3h offset stays inside the '3h' relative-time bucket. +const RENDER_NOW = Date.now() + function historyResponse(subject: string) { return { ok: true, result: { - items: [{ id: 'commit-1', displayId: 'c0mm1t1', subject, author: 'Ada', parentIds: [] }] + items: [ + { + id: 'commit-1', + displayId: 'c0mm1t1', + subject, + author: 'Ada', + parentIds: [], + timestamp: RENDER_NOW - 3 * 3_600_000 + } + ] } } } @@ -92,6 +105,9 @@ describe('MobileGitHistoryList', () => { await render(client, 'connected') expect(tree()).toContain('first load') + // Rows format the RPC timestamp (epoch ms); a regression to seconds-scaling + // renders every commit as 'just now' instead. + expect(tree()).toContain('3h') await update(client, 'reconnecting') expect(tree()).toContain('first load') diff --git a/mobile/src/source-control/mobile-git-history.test.ts b/mobile/src/source-control/mobile-git-history.test.ts index 7357f76aeff..7660d3f72d8 100644 --- a/mobile/src/source-control/mobile-git-history.test.ts +++ b/mobile/src/source-control/mobile-git-history.test.ts @@ -11,20 +11,21 @@ function item(overrides: Partial = {}): GitHistoryItem { subject: 'feat: thing', message: 'feat: thing\n\nbody', author: 'Jane', - timestamp: NOW / 1000 - 3600, + timestamp: NOW - 3_600_000, ...overrides } } describe('formatCommitTime', () => { - it('formats across thresholds', () => { - const s = NOW / 1000 - expect(formatCommitTime(s - 30, NOW)).toBe('just now') - expect(formatCommitTime(s - 5 * 60, NOW)).toBe('5m') - expect(formatCommitTime(s - 3 * 3600, NOW)).toBe('3h') - expect(formatCommitTime(s - 2 * 86400, NOW)).toBe('2d') - expect(formatCommitTime(s - 60 * 86400, NOW)).toBe('2mo') - expect(formatCommitTime(s - 800 * 86400, NOW)).toBe('2y') + it('formats across thresholds from epoch-millisecond timestamps', () => { + // GitHistoryItem.timestamp is epoch ms (git-history-log-parser scales git %at by 1000). + const ms = { min: 60_000, hour: 3_600_000, day: 86_400_000 } + expect(formatCommitTime(NOW - 3 * ms.hour, NOW)).toBe('3h') + expect(formatCommitTime(NOW - 30_000, NOW)).toBe('just now') + expect(formatCommitTime(NOW - 5 * ms.min, NOW)).toBe('5m') + expect(formatCommitTime(NOW - 2 * ms.day, NOW)).toBe('2d') + expect(formatCommitTime(NOW - 60 * ms.day, NOW)).toBe('2mo') + expect(formatCommitTime(NOW - 800 * ms.day, NOW)).toBe('2y') }) it('returns empty for missing timestamp', () => { diff --git a/mobile/src/source-control/mobile-git-history.ts b/mobile/src/source-control/mobile-git-history.ts index 416b761d214..d0d42929ade 100644 --- a/mobile/src/source-control/mobile-git-history.ts +++ b/mobile/src/source-control/mobile-git-history.ts @@ -12,12 +12,13 @@ export type MobileCommitRow = { } // Short relative time for a commit list (just now / Xm / Xh / Xd / Xmo / Xy). -export function formatCommitTime(timestampSeconds: number | undefined, nowMs: number): string { +// `timestampMs` is epoch ms, the unit GitHistoryItem.timestamp already carries. +export function formatCommitTime(timestampMs: number | undefined, nowMs: number): string { // Nullish — not falsy — so a real epoch-0 timestamp still formats. - if (timestampSeconds == null) { + if (timestampMs == null) { return '' } - const delta = nowMs - timestampSeconds * 1000 + const delta = nowMs - timestampMs if (delta < 60_000) { return 'just now' } diff --git a/src/shared/git-history-types.ts b/src/shared/git-history-types.ts index 4e99d4b2eb2..ede5ba19fac 100644 --- a/src/shared/git-history-types.ts +++ b/src/shared/git-history-types.ts @@ -48,6 +48,7 @@ export type GitHistoryItem = { displayId?: string author?: string authorEmail?: string + /** Epoch milliseconds (git %at seconds × 1000). */ timestamp?: number statistics?: GitHistoryItemStatistics references?: GitHistoryItemRef[] diff --git a/src/shared/git-history.test.ts b/src/shared/git-history.test.ts index 617fa33c2ef..38f7cdd2bd5 100644 --- a/src/shared/git-history.test.ts +++ b/src/shared/git-history.test.ts @@ -115,7 +115,9 @@ describe('git history parsing', () => { message: 'feat: add graph\n\nbody line', author: 'Ada Lovelace', authorEmail: 'ada@example.com', - displayId: HEAD_OID.slice(0, 7) + displayId: HEAD_OID.slice(0, 7), + // The format feeds %at seconds; consumers get epoch milliseconds. + timestamp: 1_700_000_000_000 }) expect(item?.references?.map((ref) => [ref.id, ref.name, ref.category])).toEqual([ ['refs/heads/feature', 'feature', 'branches'], 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 2/9] 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 () => { From fa5ef9988596987c425b83da4a3c3041d19d1a9f Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 6 Sep 2026 23:34:50 -0700 Subject: [PATCH 3/9] fix(native-chat): settle structured chat turns stranded by a restart (#19122) * fix: settle structured chat turns after restart * fix: preserve unconfirmed turn cancellation state * test: preserve unconfirmed turn lifecycle * test: narrow unconfirmed cancellation coverage * fix: keep intentional TUI closes out of recovery * test: keep branch rename journal mock current * fix: settle dead TUI handoffs before reacquire * fix: preserve handoff stage after retry settlement --------- Co-authored-by: Merge Sim --- ...-agent-session-handoff-flow-runner.test.ts | 1 + ...-agent-session-handoff-owner-close.test.ts | 52 ++++++ ...tured-agent-session-handoff-owner-close.ts | 3 +- ...tructured-agent-session-handoff-reverse.ts | 7 + ...-agent-session-handoff-test-coordinator.ts | 1 + .../structured-agent-session-handoff-types.ts | 1 + .../structured-agent-session-handoff.test.ts | 2 + .../structured-agent-session-host-handoff.ts | 8 + .../structured-agent-session-host.ts | 2 +- ...-session-live-tui-restart-survival.test.ts | 2 + ...ed-agent-session-proven-dead-retry.test.ts | 24 +++ ...ed-agent-session-readable-restorer.test.ts | 1 + ...uctured-agent-session-readable-restorer.ts | 4 + ...ured-agent-session-restart-restore.test.ts | 99 +++++++++++- ...tructured-agent-session-restart-restore.ts | 5 + .../structured-agent-session-reveal.test.ts | 1 + .../structured-agent-session-reveal.ts | 8 +- ...ructured-agent-session-settlement-retry.ts | 35 ++-- .../structured-agent-session-turns.test.ts | 46 ++++++ ...tructured-agent-session-unexpected-exit.ts | 6 +- ...t-session-wedged-profile-migration.test.ts | 149 +++++++++++++++++- ...-session-eviction-settlement-latch.test.ts | 115 ++++++++++++++ ...agent-session-handoff-lease-transitions.ts | 2 +- .../agent-session-lease-transitions.ts | 9 +- .../runtime/agent-session-record-store.ts | 4 +- ...nt-session-restart-handoff-adjudication.ts | 3 + ...agent-session-restart-lease-transitions.ts | 8 +- .../agent-session-lease-adjudication.ts | 15 +- 28 files changed, 586 insertions(+), 27 deletions(-) create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-handoff-owner-close.test.ts create mode 100644 src/main/runtime/agent-session-eviction-settlement-latch.test.ts diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-flow-runner.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-flow-runner.test.ts index 20402c8c05e..286a0dca059 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-flow-runner.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-flow-runner.test.ts @@ -70,6 +70,7 @@ async function failingFlowRunner( throw new Error('unused') }, importTuiHistory: async () => {}, + retryPendingSettlement: async () => true, publish: () => {}, schedule: async () => { throw new Error('scheduling failed') diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-owner-close.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-owner-close.test.ts new file mode 100644 index 00000000000..a947b5e8ecc --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-owner-close.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it, vi } from 'vitest' +import { + agentSessionLeaseFixture, + agentSessionRecordFixture +} from '../../../shared/agent-session-record.test-fixture' +import type { StructuredAgentSessionHandoffDeps } from './structured-agent-session-handoff-types' +import { closeRetainedTuiOwner } from './structured-agent-session-handoff-owner-close' + +const NOW = 1_800_000_000_000 + +describe('closeRetainedTuiOwner', () => { + it('does not latch unexpected-exit settlement after an intentional close', async () => { + let record = agentSessionRecordFixture(agentSessionLeaseFixture()) + const closeTuiOwner = vi.fn(async () => ({})) + const releaseOwner = vi.fn() + const owner = { + terminal: { handle: 'terminal-1', tabId: 'tab-1', paneKey: 'pane-1', ptyId: 'pty-1' }, + process: record.lease.ownerProcess!, + link: record.providerHandleChain[0]! + } + const deps = { + store: { + transitionHandoff: async ( + _sessionId: string, + transition: (current: typeof record) => typeof record + ) => { + record = transition(record) + return record + } + }, + transport: { closeTuiOwner }, + now: () => NOW + } as unknown as StructuredAgentSessionHandoffDeps + + await closeRetainedTuiOwner({ + sessionId: record.sessionId, + deps, + owner: () => owner, + requireRecord: () => record, + releaseOwner + }) + + expect(closeTuiOwner).toHaveBeenCalledWith(owner) + expect(releaseOwner).toHaveBeenCalledWith(record.sessionId) + expect(record.lease).toMatchObject({ + claimStatus: 'released', + deathEvidence: { kind: 'exit-observed' } + }) + expect(record.lease.settlementRetryRequired).toBeUndefined() + expect(record.lease.settlementRetryId).toBeUndefined() + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-owner-close.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-owner-close.ts index 56cbe4cdc53..634eab51423 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-owner-close.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-owner-close.ts @@ -26,7 +26,8 @@ export async function closeRetainedTuiOwner(input: { record: current, expectedFence: record.lease.runtimeFence, probe: { outcome: 'exit-observed' }, - now: input.deps.now() + now: input.deps.now(), + journalSettlement: 'not-required' }) ) input.releaseOwner(input.sessionId) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-reverse.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-reverse.ts index ebfca81525c..1dcd4904895 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-reverse.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-reverse.ts @@ -95,6 +95,13 @@ export async function handoffStructuredSessionToNative( ...(transcriptPath ? { transcriptPath } : {}) }) } + if (record.lease.settlementRetryRequired) { + const settled = await deps.retryPendingSettlement(sessionId) + if (!settled) { + throw new Error('The provider-exit terminal journal settlement is still pending.') + } + record = context.requireRecord(sessionId) + } const spawnToken = randomUUID() record = await reserveStoredAgentSessionHandoffOwner(deps.store, { sessionId, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-test-coordinator.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-test-coordinator.ts index e5dd478f719..81ef23aeb3b 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-test-coordinator.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-test-coordinator.ts @@ -73,6 +73,7 @@ export function createStructuredAgentSessionHandoffTestCoordinator( { fence, recovered: true } ) }, + retryPendingSettlement: async () => true, publish: (_sessionId, status) => input.statuses.push(status), schedule: async (_sessionId, task) => task(), now: () => input.now diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-types.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-types.ts index 7155826702d..218db8c539c 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-types.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-types.ts @@ -81,6 +81,7 @@ export type StructuredAgentSessionHandoffDeps = { fence: number transcriptPath?: string }) => Promise + retryPendingSettlement: (sessionId: string) => Promise prepareTuiHistoryCatchup?: (sessionId: string, fence: number) => Promise recoverTuiHistoryCatchup?: (sessionId: string, fence: number) => Promise activateTuiHistoryCatchup?: (sessionId: string) => Promise diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff.test.ts index f0f410b66aa..beca21cb63a 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff.test.ts @@ -179,6 +179,7 @@ function createCoordinator(): StructuredAgentSessionHandoffCoordinator { { fence, recovered: true } ) }, + retryPendingSettlement: async () => true, prepareTuiHistoryCatchup, recoverTuiHistoryCatchup, activateTuiHistoryCatchup, @@ -274,6 +275,7 @@ describe('structured session handoff failure handling', () => { }), acquireNativeStop: (_sessionId, turnId) => acquireNativeStop(turnId), importTuiHistory: vi.fn(async () => undefined), + retryPendingSettlement: vi.fn(async () => true), prepareTuiHistoryCatchup, recoverTuiHistoryCatchup, activateTuiHistoryCatchup, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts index abd2268c809..cb316850e5b 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts @@ -14,6 +14,7 @@ import { recoverDeadTuiHandoffStatus } from './structured-agent-session-dead-tui import { readNativeSessionOptions } from './structured-agent-session-option-restoration' import type { AgentSessionSubscribers } from './structured-agent-session-subscribers' import { StructuredTuiTranscriptCatchup } from './structured-tui-transcript-catchup' +import { retryLoadedStructuredAgentSessionSettlement } from './structured-agent-session-settlement-retry' type HostHandoffAccess = { session: (sessionId: string) => StructuredAgentSessionHostSession @@ -92,6 +93,13 @@ export function createStructuredAgentSessionHostHandoff( acquireNativeStop: async (sessionId, turnId, fence) => (await deps.adapter.cancelTurn({ sessionId, turnId, fence })).cancelled, importTuiHistory: (input) => importTuiHistory(deps, host, input), + retryPendingSettlement: (sessionId) => + retryLoadedStructuredAgentSessionSettlement({ + deps, + sessionId, + session: host.session(sessionId), + now: host.now + }), prepareTuiHistoryCatchup: (sessionId, fence) => tuiHistoryCatchup.prepare(sessionId, fence), recoverTuiHistoryCatchup: (sessionId, fence) => tuiHistoryCatchup.recover(sessionId, fence), activateTuiHistoryCatchup: (sessionId) => tuiHistoryCatchup.activate(sessionId), diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts index 14ca9c5b7b5..22557e87c52 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts @@ -127,7 +127,7 @@ export class StructuredAgentSessionHost { ), evict: (sessionId) => this.close(sessionId) }) - this.restore = createStructuredAgentSessionHostRestore(deps, { + this.restore = createStructuredAgentSessionHostRestore(deps, this.sessions, () => this.now(), { reconcile: this.reconcileLeases, resolveRecovery: (sessionId) => this.runtimeState.resolveRecovery(sessionId), serialize: (sessionId, task) => this.serialize(sessionId, task), diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-live-tui-restart-survival.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-live-tui-restart-survival.test.ts index 90b68194c4f..3a8de86c3de 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-live-tui-restart-survival.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-live-tui-restart-survival.test.ts @@ -104,6 +104,7 @@ describe('structured session live TUI restart survival', () => { suspendNative: vi.fn(), acquireNative: vi.fn(), importTuiHistory: vi.fn(), + retryPendingSettlement: vi.fn(async () => true), publish: vi.fn(), schedule: async (_sessionId, task) => task(), now: () => NOW @@ -224,6 +225,7 @@ describe('structured session live TUI restart survival', () => { suspendNative: vi.fn(), acquireNative: vi.fn(), importTuiHistory: vi.fn(), + retryPendingSettlement: vi.fn(async () => true), publish: vi.fn(), schedule: async (_sessionId, task) => task(), now: () => NOW diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-proven-dead-retry.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-proven-dead-retry.test.ts index 3caba894cb9..aed78f3e5b9 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-proven-dead-retry.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-proven-dead-retry.test.ts @@ -3,12 +3,14 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' import { computeAgentSessionPayloadFingerprint } from '../../../shared/agent-session-mutation-envelope' +import { activeStructuredAgentSessionTurnId } from '../../../shared/structured-agent-session-projection' import type { AgentSessionHandoffRequest } from '../../../shared/agent-session-wire' import { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import { recoverStoredDeadTuiOwnerForHandoff } from '../../runtime/agent-session-handoff-record-transitions' import { openAgentSessionJournal } from '../agent-session-journal/journal-store-factory' import { StructuredAgentSessionHandoffCoordinator } from './structured-agent-session-handoff' import type { StructuredAgentSessionHandoffTransport } from './structured-agent-session-handoff-types' +import { retryLoadedStructuredAgentSessionSettlement } from './structured-agent-session-settlement-retry' const NOW = 1_800_000_000_000 const SESSION = 'session-proven-dead-retry' @@ -89,6 +91,15 @@ describe('structured session proven-dead TUI retry', () => { }, journalDir: join(root, 'journal') }) + await journal.appendItem( + { provider: 'orca', clientMessageId: 'running-turn' }, + { + kind: 'status', + text: 'Working', + turnLifecycle: { turnId: 'turn-1', state: 'running' } + }, + { fence: store.getRecord(SESSION)?.lease.runtimeFence ?? tuiFence } + ) const closeTuiOwner = vi.fn>() const coordinator = new StructuredAgentSessionHandoffCoordinator({ @@ -134,6 +145,17 @@ describe('structured session proven-dead TUI retry', () => { }, acquireNativeStop: vi.fn(async () => true), importTuiHistory: vi.fn(), + retryPendingSettlement: (sessionId) => + retryLoadedStructuredAgentSessionSettlement({ + deps: { store }, + sessionId, + session: { + journal, + fence: store.getRecord(sessionId)?.lease.runtimeFence ?? 1, + acquisitionGeneration: null + }, + now: () => NOW + }), publish: vi.fn(), schedule: async (_sessionId, task) => task(), now: () => NOW @@ -174,5 +196,7 @@ describe('structured session proven-dead TUI retry', () => { claimStatus: 'live', handoffStage: null }) + expect(store.getRecord(SESSION)?.lease.settlementRetryRequired).toBeUndefined() + expect(activeStructuredAgentSessionTurnId(journal.snapshot().items)).toBe(null) }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-readable-restorer.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-readable-restorer.test.ts index 8859365b117..1bb03e95b2e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-readable-restorer.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-readable-restorer.test.ts @@ -27,6 +27,7 @@ describe('StructuredAgentSessionReadableRestorer', () => { serialize: async (_sessionId, task) => task(), hasSession: () => false, onReadable: () => undefined, + retrySettlement: async () => true, restoreHandoff: async () => undefined }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-readable-restorer.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-readable-restorer.ts index e3b96dbd36a..a0bb32ad737 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-readable-restorer.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-readable-restorer.ts @@ -20,6 +20,10 @@ export class StructuredAgentSessionReadableRestorer { serialize: (sessionId: string, task: () => Promise) => Promise hasSession: (sessionId: string) => boolean onReadable: (sessionId: string, restored: RestoredStructuredAgentSessionRead) => void + retrySettlement: ( + sessionId: string, + params: RestoredStructuredAgentSessionRead['params'] + ) => Promise restoreHandoff: (sessionId: string) => Promise } ) {} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.test.ts index d0dcd38b986..d6a4a4397e4 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { AgentSessionRecord } from '../../../shared/agent-session-record' +import type { AgentSessionAttachParams } from './structured-agent-session-attach' const { restoreRead } = vi.hoisted(() => ({ restoreRead: vi.fn() @@ -9,7 +10,10 @@ vi.mock('./structured-agent-session-read-restore', () => ({ restoreStructuredAgentSessionRead: restoreRead })) -import { restoreStructuredAgentSessionsOnRestart } from './structured-agent-session-restart-restore' +import { + restoreOneStructuredAgentSessionRead, + restoreStructuredAgentSessionsOnRestart +} from './structured-agent-session-restart-restore' describe('restart journal restoration', () => { beforeEach(() => restoreRead.mockReset()) @@ -45,6 +49,7 @@ describe('restart journal restoration', () => { serialize: async (_sessionId, task) => task(), hasSession: () => false, onReadable: () => undefined, + retrySettlement: async () => true, restoreHandoff: async () => undefined }) @@ -56,4 +61,96 @@ describe('restart journal restoration', () => { expect(restoreRead).toHaveBeenCalledTimes(records.length) expect(peak).toBe(4) }) + + it('runs pending settlement retry after recovery resolution and before handoff', async () => { + const calls: string[] = [] + const params: AgentSessionAttachParams = { + envelope: { + sessionId: 'session-1', + clientOperationId: 'read-restore:session-1', + expectedRuntimeFence: 4, + payloadFingerprint: 'fingerprint' + }, + location: { + executionHostId: 'local', + wslDistro: null, + workspaceId: 'workspace-1', + workspaceKind: 'folder' + }, + provider: 'codex', + agent: 'codex', + accountHome: { variable: 'CODEX_HOME', path: '/tmp/codex' }, + runtimeKind: 'native' + } + restoreRead.mockResolvedValue({ + journal: {}, + params, + fence: 4, + hasProviderChild: false, + acquisitionGeneration: null + }) + + await restoreOneStructuredAgentSessionRead( + { + store: {} as never, + journalRoot: '/tmp/journals', + reconcile: async () => null, + resolveRecovery: async () => { + calls.push('resolveRecovery') + }, + serialize: async (_sessionId, task) => task(), + hasSession: () => false, + onReadable: () => { + calls.push('onReadable') + }, + retrySettlement: async (_sessionId, restoredParams) => { + calls.push( + restoredParams === params ? 'retrySettlement:restored-params' : 'retrySettlement' + ) + return true + }, + restoreHandoff: async () => { + calls.push('restoreHandoff') + } + }, + 'session-1' + ) + + expect(calls).toEqual([ + 'resolveRecovery', + 'onReadable', + 'retrySettlement:restored-params', + 'restoreHandoff' + ]) + }) + + it('does not rerun settlement retry when a second restore finds the session already open', async () => { + const retrySettlement = vi.fn(async () => true) + const restoreHandoff = vi.fn(async () => undefined) + restoreRead.mockResolvedValue({ + journal: {}, + params: {}, + fence: 4, + hasProviderChild: false, + acquisitionGeneration: null + }) + + await restoreOneStructuredAgentSessionRead( + { + store: {} as never, + journalRoot: '/tmp/journals', + reconcile: async () => null, + resolveRecovery: async () => undefined, + serialize: async (_sessionId, task) => task(), + hasSession: () => true, + onReadable: () => undefined, + retrySettlement, + restoreHandoff + }, + 'session-1' + ) + + expect(retrySettlement).not.toHaveBeenCalled() + expect(restoreHandoff).toHaveBeenCalledOnce() + }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.ts index 174aac7d72b..7e697efbdc7 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.ts @@ -30,6 +30,10 @@ export type StructuredAgentSessionReadRestoreDeps = { serialize: (sessionId: string, task: () => Promise) => Promise hasSession: (sessionId: string) => boolean onReadable: (sessionId: string, restored: RestoredStructuredAgentSessionRead) => void + retrySettlement: ( + sessionId: string, + params: RestoredStructuredAgentSessionRead['params'] + ) => Promise restoreHandoff: (sessionId: string) => Promise } @@ -64,6 +68,7 @@ export async function restoreOneStructuredAgentSessionRead( return } input.onReadable(sessionId, restored) + await input.retrySettlement(sessionId, restored.params) await input.restoreHandoff(sessionId) }) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-reveal.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-reveal.test.ts index e2ef20503d6..8d99ea9098f 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-reveal.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-reveal.test.ts @@ -58,6 +58,7 @@ function harness( serialize, hasSession: (sessionId) => live.has(sessionId), onReadable: (sessionId, restored) => live.set(sessionId, restored), + retrySettlement: async () => true, restoreHandoff }) return { restorer, live, restoreHandoff, serializedIds } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-reveal.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-reveal.ts index 41774a9d719..d940fb323bd 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-reveal.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-reveal.ts @@ -16,8 +16,10 @@ import { StructuredAgentSessionReadableRestorer } from './structured-agent-sessi import { StructuredAgentSessionRestartRestoreGate } from './structured-agent-session-restart-restore-gate' import type { StructuredAgentSessionHostDeps, + StructuredAgentSessionHostSession, StructuredAgentSessionReveal } from './structured-agent-session-host-types' +import { retryPendingStructuredAgentSessionSettlement } from './structured-agent-session-settlement-retry' /** Throws its refusal as the code itself, matching `resumeHeldStructuredAgentSession`. */ export async function revealStructuredAgentSession( @@ -55,9 +57,11 @@ export async function revealStructuredAgentSession( */ export function createStructuredAgentSessionHostRestore( deps: StructuredAgentSessionHostDeps, + sessions: Map, + now: () => number, wiring: Omit< ConstructorParameters[0], - 'store' | 'journalRoot' | 'supportsRecord' + 'store' | 'journalRoot' | 'supportsRecord' | 'retrySettlement' > ): { restoreReadableSessions: (sessionIds?: readonly string[]) => Promise @@ -67,6 +71,8 @@ export function createStructuredAgentSessionHostRestore( store: deps.store, journalRoot: deps.journalRoot, supportsRecord: (record) => adapterSupportsRecord(deps.adapter, record), + retrySettlement: (sessionId, params) => + retryPendingStructuredAgentSessionSettlement({ deps, sessions, sessionId, params, now }), ...wiring }) const gate = new StructuredAgentSessionRestartRestoreGate() diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-settlement-retry.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-settlement-retry.ts index fc60d6c4696..9fc68a9fca2 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-settlement-retry.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-settlement-retry.ts @@ -46,15 +46,27 @@ export async function retryPendingStructuredAgentSessionSettlement(input: { hasProviderChild: false, acquisitionGeneration: null } as StructuredAgentSessionHostSession) + return retryLoadedStructuredAgentSessionSettlement({ + deps: input.deps, + sessionId: input.sessionId, + session: retrySession, + now: input.now + }) +} + +export async function retryLoadedStructuredAgentSessionSettlement(input: { + deps: Pick + sessionId: string + session: Pick + now: () => number +}): Promise { + const record = input.deps.store.getRecord(input.sessionId) + if (!record?.lease.settlementRetryRequired || !record.lease.settlementRetryId) { + return true + } + const retrySession = input.session retrySession.fence = record.lease.runtimeFence - const context: StructuredAgentSessionUnexpectedExitContext = { - store: input.deps.store, - sessions: input.sessions, - flushLifecycle: async () => ({ ok: true as const }), - publishFence: () => undefined, - hasResumeCapableHolder: () => false, - serialize: async (_id: string, task: () => Promise) => task(), - now: input.now, + const context: Pick = { onBarrierError: (id, error) => input.deps.onEventSinkError?.({ sessionId: id, error }) } const ok = await retryUnexpectedExitSettlement({ @@ -65,7 +77,7 @@ export async function retryPendingStructuredAgentSessionSettlement(input: { reason: record.lease.deathEvidence?.detail ?? 'provider exited', cause: 'unexpected-exit', fence: record.lease.runtimeFence, - acquisitionGeneration: current?.acquisitionGeneration ?? 'recovery' + acquisitionGeneration: retrySession.acquisitionGeneration ?? 'recovery' }, session: retrySession, stableSettlementId: record.lease.settlementRetryId @@ -81,11 +93,14 @@ export async function retryPendingStructuredAgentSessionSettlement(input: { ) { throw new Error('agent_session_checkpoint_stale') } + // A dead-TUI retry still needs its stopped-owner stage; recovery-only stages end here. + const preserveHandoff = latest.lease.handoffStage === 'old-owner-stopped' return { ...latest, lease: { ...latest.lease, - handoffStage: null, + handoffStage: preserveHandoff ? latest.lease.handoffStage : null, + handoffOperationId: preserveHandoff ? latest.lease.handoffOperationId : null, settlementRetryRequired: undefined, settlementRetryId: undefined, lastRenewedAt: input.now() diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-turns.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.test.ts index 31df2c44551..aa0785da31a 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-turns.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.test.ts @@ -74,6 +74,52 @@ describe('performCancel', () => { ]) }) + it('keeps the running lifecycle when cancellation cannot be confirmed', async () => { + root = await mkdtemp(join(tmpdir(), 'orca-turn-cancel-unconfirmed-')) + const journal = await journals.open({ identity: IDENTITY, journalDir: root }) + await journal.appendItem( + { + provider: 'legacy', + agent: 'codex', + sessionId: 'session-1', + recordId: 'turn-lifecycle:turn-1' + }, + { + kind: 'status', + text: 'Agent is working…', + turnLifecycle: { turnId: 'turn-1', state: 'running' } + }, + { fence: 1 } + ) + const ctx: AgentSessionTurnContext = { + sessionId: 'session-1', + journal, + fence: 1, + adapter: { + cancelTurn: vi.fn(async () => ({ cancelled: false })) + } as unknown as StructuredAgentSessionAdapter, + persistOptions: async () => undefined, + resolvedBy: 'client-1', + publish: vi.fn(), + now: () => 1 + } + + const result = await performCancel(ctx, { + clientOperationId: 'cancel-unconfirmed-1', + turnId: 'turn-1' + }) + + expect(result).toEqual({ ok: true, value: { turnId: 'turn-1', cancelled: false } }) + expect(journal.snapshot().items.map((item) => item.body)).toEqual([ + { + kind: 'status', + text: 'Agent is working…', + turnLifecycle: { turnId: 'turn-1', state: 'running' } + }, + { kind: 'status', text: 'The provider had already finished this turn.' } + ]) + }) + it('stops background tasks without interrupting the foreground turn or writing a row', async () => { root = await mkdtemp(join(tmpdir(), 'orca-background-task-cancel-')) const journal = await journals.open({ identity: IDENTITY, journalDir: root }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts index af7f2ccfde3..87fe0cbbeec 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts @@ -161,9 +161,9 @@ export function isStructuredAgentSessionRecoveryTicketCurrent( } export async function retryUnexpectedExitSettlement(input: { - context: StructuredAgentSessionUnexpectedExitContext + context: Pick event: UnexpectedExitLifecycleEvent - session: StructuredAgentSessionHostSession + session: Pick stableSettlementId: string }): Promise { try { @@ -189,7 +189,7 @@ export async function retryUnexpectedExitSettlement(input: { function unexpectedExitFallbackMutations( event: UnexpectedExitLifecycleEvent, - session: StructuredAgentSessionHostSession, + session: Pick, stableSettlementId: string ): JournalLifecycleMutationInput[] { const mutations: JournalLifecycleMutationInput[] = [] diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-wedged-profile-migration.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-wedged-profile-migration.test.ts index 4af342227c2..dfeaa650129 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-wedged-profile-migration.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-wedged-profile-migration.test.ts @@ -15,6 +15,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' import { evaluateAgentSessionAcquisition } from '../../../shared/agent-session-lease-adjudication' +import { activeStructuredAgentSessionTurnId } from '../../../shared/structured-agent-session-projection' import type { AgentSessionClaimStatus, AgentSessionHandoffStage, @@ -25,6 +26,9 @@ import type { import { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import { AGENT_SESSION_STORE_FILE_NAME } from '../../runtime/agent-session-record-store-file' import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' +import { openAgentSessionJournal } from '../agent-session-journal/journal-store-factory' +import { journalDirectoryFor } from '../agent-session-journal/journal-paths' +import type { AgentSessionJournal } from '../agent-session-journal/journal-store' import { StructuredAgentSessionHost } from './structured-agent-session-host' import type { StructuredAgentSessionHostDeps } from './structured-agent-session-host-types' import { @@ -126,7 +130,8 @@ function openHost(overrides: Partial = {}): void dispatch: vi.fn(), cancelTurn: vi.fn(), answerPrompt: vi.fn(), - setOption: vi.fn() + setOption: vi.fn(), + supportsCreate: () => true } as unknown as StructuredAgentSessionAdapter, journalRoot: root, claimKeyId: 'key-1', @@ -174,7 +179,149 @@ function isAcquirable(lease: NonNullable>['le ) } +async function seedRunningTurn(provider: 'codex' | 'claude' = 'codex'): Promise { + const journal = await openAgentSessionJournal({ + identity: { + sessionId: SESSION, + workspaceId: LOCATION.workspaceId, + hostId: LOCATION.executionHostId, + agent: provider, + providerHandle: + provider === 'codex' + ? { kind: 'codex', threadId: THREAD } + : { kind: 'claude', sessionId: 'provider-session-alpha-1', leafUuid: null } + }, + journalDir: journalDirectoryFor(root, { workspaceId: LOCATION.workspaceId, sessionId: SESSION }) + }) + await journal.appendItem( + provider === 'codex' + ? { provider: 'codex', threadId: THREAD, turnId: 'turn-1', ordinal: 0 } + : { provider: 'claude', sessionId: 'provider-session-alpha-1', uuid: 'uuid-running' }, + { + kind: 'status', + text: 'Agent is working...', + turnLifecycle: { turnId: 'turn-1', state: 'running' } + }, + { fence: 13 } + ) + await journal.close() +} + +function restoredJournal(): AgentSessionJournal { + const restored = ( + host as unknown as { sessions: Map } + ).sessions.get(SESSION) + if (!restored) { + throw new Error('expected a restored session journal') + } + return restored.journal +} + describe('already-wedged profiles become usable on load', () => { + it.each(['codex', 'claude'] as const)( + 'settles a wedged %s journal on boot without opening a provider child', + async (provider) => { + const record = wedgedRecord({ + claimStatus: 'live', + handoffStage: null, + ownerProcess: DEAD_OWNER + }) + const providerRecord: AgentSessionRecord = + provider === 'codex' + ? record + : { + ...record, + provider: 'claude', + accountHome: { variable: 'CLAUDE_CONFIG_DIR', path: '/home/dev/.claude' }, + lease: { ...record.lease, provenHandleLinkId: 'claude-13-link' }, + providerHandleChain: [ + { + linkId: 'claude-13-link', + handle: { + provider: 'claude', + sessionId: 'provider-session-alpha-1', + leafUuid: null + }, + origin: 'created', + mintedAtFence: 13, + observedAt: NOW - 10_000 + } + ] + } + await seedStore(providerRecord) + await seedRunningTurn(provider) + openHost() + + await host.restoreReadableSessions() + + expect(host.hasSession(SESSION)).toBe(true) + const firstCursor = restoredJournal().cursor() + expect(activeStructuredAgentSessionTurnId(restoredJournal().snapshot().items)).toBe(null) + expect(store.getRecord(SESSION)?.lease).toMatchObject({ + claimStatus: 'released', + handoffStage: null, + settlementRetryRequired: undefined, + settlementRetryId: undefined + }) + expect(acquire).not.toHaveBeenCalled() + + await host.flushAllStreamedEvents() + store = await AgentSessionRecordStore.open({ + directory: join(root, 'store'), + hostId: 'local' + }) + openHost() + await host.restoreReadableSessions() + + expect(restoredJournal().cursor()).toEqual(firstCursor) + expect(activeStructuredAgentSessionTurnId(restoredJournal().snapshot().items)).toBe(null) + } + ) + + it('settles restart eviction through attach when a hold arrives before the boot sweep', async () => { + await seedStore( + wedgedRecord({ claimStatus: 'live', handoffStage: null, ownerProcess: DEAD_OWNER }) + ) + await seedRunningTurn() + openHost() + + await host.hold(SESSION, 'desktop-chat:restart') + + expect(acquire).toHaveBeenCalledOnce() + expect(activeStructuredAgentSessionTurnId(restoredJournal().snapshot().items)).toBe(null) + expect(store.getRecord(SESSION)?.lease).toMatchObject({ + claimStatus: 'live', + handoffStage: null, + settlementRetryRequired: undefined, + settlementRetryId: undefined + }) + }) + + it('settles an observed-exit latch through attach before the boot sweep', async () => { + const record = wedgedRecord({ claimStatus: 'released', handoffStage: 'recovering' }) + record.lease.settlementRetryRequired = true + record.lease.settlementRetryId = `provider-exit:${SESSION}:12:generation-1` + record.lease.deathEvidence = { + kind: 'exit-observed', + detail: 'provider exited: transport closed', + observedAt: NOW - 1_000 + } + await seedStore(record) + await seedRunningTurn() + openHost() + + expect(await host.attach(CALLER, hostTestAttachParams(13))).toMatchObject({ ok: true }) + + expect(acquire).toHaveBeenCalledOnce() + expect(activeStructuredAgentSessionTurnId(restoredJournal().snapshot().items)).toBe(null) + expect(store.getRecord(SESSION)?.lease).toMatchObject({ + claimStatus: 'live', + handoffStage: null, + settlementRetryRequired: undefined, + settlementRetryId: undefined + }) + }) + it('re-adjudicates a conflicted manual-recovery record whose owner is provably gone', async () => { // A crash can leave a conflicted current-schema row in manual recovery; positive death proof // must make it acquirable again without discarding the provider handle. diff --git a/src/main/runtime/agent-session-eviction-settlement-latch.test.ts b/src/main/runtime/agent-session-eviction-settlement-latch.test.ts new file mode 100644 index 00000000000..c6ae4fc2ffa --- /dev/null +++ b/src/main/runtime/agent-session-eviction-settlement-latch.test.ts @@ -0,0 +1,115 @@ +import { describe, expect, it } from 'vitest' +import { + agentSessionLeaseFixture, + agentSessionRecordFixture +} from '../../shared/agent-session-record.test-fixture' +import { evictAgentSessionOwner } from './agent-session-lease-transitions' +import { applyAgentSessionRestartAdjudication } from './agent-session-restart-lease-transitions' + +const NOW = 1_800_000_000_000 + +describe('proven-dead agent session eviction settlement', () => { + it('latches restart eviction with a stable id while keeping the lease resumable', () => { + const record = agentSessionRecordFixture( + agentSessionLeaseFixture({ runtimeKind: 'native', unreconciled: true }) + ) + + const evicted = applyAgentSessionRestartAdjudication({ + record, + probe: { outcome: 'pid-absent' }, + now: NOW + }) + + expect(evicted.lease).toMatchObject({ + claimStatus: 'released', + runtimeFence: 8, + handoffStage: null, + settlementRetryRequired: true, + settlementRetryId: 'restart-eviction:session-alpha-1:8', + deathEvidence: { kind: 'pid-absent', detail: 'recorded pid absent on host' } + }) + }) + + it('latches recovery eviction from the same evicted disposition', () => { + const record = agentSessionRecordFixture( + agentSessionLeaseFixture({ runtimeKind: 'native', handoffStage: 'recovering' }) + ) + + const evicted = evictAgentSessionOwner({ + record, + expectedFence: 7, + probe: { outcome: 'identity-mismatch', field: 'process-start-time' }, + now: NOW, + journalSettlement: 'required' + }) + + expect(evicted.lease).toMatchObject({ + claimStatus: 'released', + runtimeFence: 8, + handoffStage: null, + settlementRetryRequired: true, + settlementRetryId: 'restart-eviction:session-alpha-1:8', + deathEvidence: { kind: 'identity-mismatch', detail: 'mismatched process-start-time' } + }) + }) + + it('never latches an indeterminate owner', () => { + const restartRecord = agentSessionRecordFixture( + agentSessionLeaseFixture({ runtimeKind: 'native', unreconciled: true }) + ) + const recovered = applyAgentSessionRestartAdjudication({ + record: restartRecord, + probe: { outcome: 'indeterminate', reason: 'remote host unavailable' }, + now: NOW + }) + const recoveryRecord = agentSessionRecordFixture( + agentSessionLeaseFixture({ runtimeKind: 'native', handoffStage: 'recovering' }) + ) + + expect(recovered.lease).toMatchObject({ + handoffStage: 'recovering', + ownerProcess: { pid: 4242 } + }) + expect(recovered.lease).not.toHaveProperty('settlementRetryRequired') + expect(recovered.lease).not.toHaveProperty('settlementRetryId') + expect(() => + evictAgentSessionOwner({ + record: recoveryRecord, + expectedFence: 7, + probe: { outcome: 'indeterminate', reason: 'remote host unavailable' }, + now: NOW, + journalSettlement: 'required' + }) + ).toThrow('agent_session_ownership_unknown') + expect(recoveryRecord.lease).not.toHaveProperty('settlementRetryRequired') + expect(recoveryRecord.lease).not.toHaveProperty('settlementRetryId') + }) + + it('preserves a null handoff stage when the latch survives another restart', () => { + const record = agentSessionRecordFixture( + agentSessionLeaseFixture({ + runtimeKind: 'native', + ownerProcess: null, + reservedSpawnToken: null, + claimStatus: 'released', + handoffStage: null, + settlementRetryRequired: true, + settlementRetryId: 'restart-eviction:session-alpha-1:8', + unreconciled: true + }) + ) + + const restored = applyAgentSessionRestartAdjudication({ + record, + probe: { outcome: 'indeterminate', reason: 'remote host unavailable' }, + now: NOW + }) + + expect(restored.lease).toMatchObject({ + handoffStage: null, + settlementRetryRequired: true, + settlementRetryId: 'restart-eviction:session-alpha-1:8', + unreconciled: false + }) + }) +}) diff --git a/src/main/runtime/agent-session-handoff-lease-transitions.ts b/src/main/runtime/agent-session-handoff-lease-transitions.ts index 894d7643abc..2987d06e21c 100644 --- a/src/main/runtime/agent-session-handoff-lease-transitions.ts +++ b/src/main/runtime/agent-session-handoff-lease-transitions.ts @@ -28,7 +28,7 @@ export function recoverDeadTuiOwnerForHandoff(args: { ) { throw new Error('agent_session_ownership_unknown') } - const evicted = evictAgentSessionOwner(args) + const evicted = evictAgentSessionOwner({ ...args, journalSettlement: 'required' }) return withLease(evicted, { ...evicted.lease, handoffStage: 'old-owner-stopped', diff --git a/src/main/runtime/agent-session-lease-transitions.ts b/src/main/runtime/agent-session-lease-transitions.ts index bcb0f2adf53..28617817647 100644 --- a/src/main/runtime/agent-session-lease-transitions.ts +++ b/src/main/runtime/agent-session-lease-transitions.ts @@ -8,6 +8,7 @@ import { adjudicateAgentSessionRestart, + agentSessionRestartEvictionSettlementId, evaluateAgentSessionAcquisition, type AgentSessionOwnerProbe } from '../../shared/agent-session-lease-adjudication' @@ -207,6 +208,7 @@ export function evictAgentSessionOwner(args: { expectedFence: number probe: AgentSessionOwnerProbe now: number + journalSettlement: 'required' | 'not-required' }): AgentSessionRecord { const { record } = args assertFence(record.lease, args.expectedFence) @@ -232,6 +234,7 @@ export function evictAgentSessionOwner(args: { if (adjudication.disposition !== 'evicted') { throw new Error('agent_session_ownership_unknown') } + const settlementRequired = args.journalSettlement === 'required' return withLease(record, { ...record.lease, runtimeFence: adjudication.nextFence, @@ -242,7 +245,11 @@ export function evictAgentSessionOwner(args: { claimStatus: 'released', lastRenewedAt: args.now, handoffOperationId: null, - deathEvidence: adjudication.evidence + deathEvidence: adjudication.evidence, + settlementRetryRequired: settlementRequired ? true : undefined, + settlementRetryId: settlementRequired + ? agentSessionRestartEvictionSettlementId(record.lease, adjudication) + : undefined }) } diff --git a/src/main/runtime/agent-session-record-store.ts b/src/main/runtime/agent-session-record-store.ts index 577baa16b94..4325410ed81 100644 --- a/src/main/runtime/agent-session-record-store.ts +++ b/src/main/runtime/agent-session-record-store.ts @@ -254,7 +254,9 @@ export class AgentSessionRecordStore { probe: AgentSessionOwnerProbe now: number }): Promise { - return this.mutate(args.sessionId, (record) => evictAgentSessionOwner({ ...args, record })) + return this.mutate(args.sessionId, (record) => + evictAgentSessionOwner({ ...args, record, journalSettlement: 'required' }) + ) } async transitionHandoff( diff --git a/src/main/runtime/agent-session-restart-handoff-adjudication.ts b/src/main/runtime/agent-session-restart-handoff-adjudication.ts index 99849248ace..3f78fa2e456 100644 --- a/src/main/runtime/agent-session-restart-handoff-adjudication.ts +++ b/src/main/runtime/agent-session-restart-handoff-adjudication.ts @@ -17,6 +17,9 @@ export function adjudicateRestartedAgentSessionHandoff( if (adjudication.disposition === 'readopt') { return updateLease(record, { ...record.lease, unreconciled: false, lastRenewedAt: now }) } + if (adjudication.disposition === 'settlement-pending') { + return updateLease(record, { ...record.lease, unreconciled: false, lastRenewedAt: now }) + } if (adjudication.disposition === 'free') { return updateLease(record, { ...record.lease, diff --git a/src/main/runtime/agent-session-restart-lease-transitions.ts b/src/main/runtime/agent-session-restart-lease-transitions.ts index 93e6c4333a7..a50fd4cd94e 100644 --- a/src/main/runtime/agent-session-restart-lease-transitions.ts +++ b/src/main/runtime/agent-session-restart-lease-transitions.ts @@ -8,6 +8,7 @@ import { adjudicateAgentSessionRestart, + agentSessionRestartEvictionSettlementId, type AgentSessionOwnerProbe } from '../../shared/agent-session-lease-adjudication' import type { @@ -50,6 +51,9 @@ export function applyAgentSessionRestartAdjudication(args: { // Why: re-adoption is not a new generation, so the fence does not move. return withLease(record, { ...record.lease, unreconciled: false, lastRenewedAt: args.now }) } + if (adjudication.disposition === 'settlement-pending') { + return withLease(record, { ...record.lease, unreconciled: false, lastRenewedAt: args.now }) + } if (adjudication.disposition === 'free') { // Why: an already-free lease that reloads into `recovering` is unopenable forever; clearing // the stage restores it without moving the fence or touching the recorded death evidence. @@ -74,7 +78,9 @@ export function applyAgentSessionRestartAdjudication(args: { unreconciled: false, lastRenewedAt: args.now, handoffOperationId: null, - deathEvidence: adjudication.evidence + deathEvidence: adjudication.evidence, + settlementRetryRequired: true, + settlementRetryId: agentSessionRestartEvictionSettlementId(record.lease, adjudication) }) } const stage: AgentSessionHandoffStage = diff --git a/src/shared/agent-session-lease-adjudication.ts b/src/shared/agent-session-lease-adjudication.ts index cff6eef6ca2..0181b440c90 100644 --- a/src/shared/agent-session-lease-adjudication.ts +++ b/src/shared/agent-session-lease-adjudication.ts @@ -47,12 +47,21 @@ export type AgentSessionAcquisitionDecision = export type AgentSessionRestartAdjudication = | { disposition: 'readopt' } + /** A journal settlement latch survives restart without changing its handoff stage. */ + | { disposition: 'settlement-pending' } /** Nothing is outstanding — no owner, no reservation. Clear any latched stage; the fence stays. */ | { disposition: 'free'; reason: string } | { disposition: 'evicted'; nextFence: number; evidence: AgentSessionDeathEvidence } | { disposition: 'recovering'; stage: AgentSessionHandoffStage; reason: string } | { disposition: 'conflicted'; reason: string } +export function agentSessionRestartEvictionSettlementId( + lease: Pick, + eviction: Extract +): string { + return `restart-eviction:${lease.sessionId}:${eviction.nextFence}` +} + /** Stages that can legally admit a new owner at all; the rest have an owner or no evidence. */ const STAGES_ADMITTING_NEW_OWNER: ReadonlySet = new Set([ 'old-owner-stopped', @@ -201,11 +210,7 @@ export function adjudicateAgentSessionRestart(args: { if (lease.settlementRetryRequired) { // A watched provider death can leave terminal rows unsettled. This latch is not owner // uncertainty and must survive restart until the journal settlement is durably accepted. - return { - disposition: 'recovering', - stage: 'recovering', - reason: 'provider-exit settlement requires retry' - } + return { disposition: 'settlement-pending' } } if (lease.reservedSpawnToken === null && lease.claimStatus !== 'reserved') { // Why: the spawn token is minted before the child and is the only thing a child could be From 2ccf35b13580c470a24e5c19eff7d48d9647c0f1 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 6 Sep 2026 23:41:49 -0700 Subject: [PATCH 4/9] fix: avoid quadratic trimming during fullscreen terminal redraws (#19214) --- config/reliability-gates.jsonc | 69 +++++++++++++++++++ src/main/runtime/terminal-tail-buffer.ts | 2 +- .../runtime/terminal-tail-redraw-buffer.ts | 2 +- .../runtime/terminal-tail-whitespace.test.ts | 32 +++++++++ 4 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 src/main/runtime/terminal-tail-whitespace.test.ts diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 2bcba7cb737..72bcb4b4d7f 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -10,6 +10,75 @@ } }, "gates": [ + { + "id": "terminal-performance.padded-fullscreen-redraw", + "title": "Fullscreen redraw padding does not stall terminal delivery", + "maturity": "experimental", + "protection": "partial", + "owner": "terminal-runtime", + "layer": "runtime-unit-and-electron-cdp", + "surfaces": ["terminal transcript preview", "fullscreen TUI scrolling"], + "platforms": ["macos", "linux", "windows"], + "providers": ["local", "daemon", "ssh", "remote-runtime"], + "coveredPlatforms": ["macos"], + "coveredProviders": ["local", "daemon"], + "coverageNotes": "The trim operation is platform-independent and preserves the same spaces/tabs policy for all providers. Real Pi 0.84.2 was exercised in a hidden macOS Electron renderer through CDP using a folder workspace.", + "motivatingLinks": ["https://github.com/stablyai/orca/issues/14770"], + "invariant": "Transcript preview trimming preserves internal whitespace and terminal read contents without quadratic main-process work on padded fullscreen redraws.", + "oracle": "Preserve 32,000 spaces before a marker while trimming trailing spaces/tabs in both retained-row and carried-prefix redraw paths; four redraws must finish within 500 ms. Existing tail equivalence tests preserve cursor, retention, and pagination behavior.", + "commands": [ + "ORCA_BACKGROUND_LAUNCH=1 pnpm test src/main/runtime/terminal-tail-whitespace.test.ts src/main/runtime/terminal-tail-buffer.test.ts src/main/runtime/retained-tail-redraw-window.equivalence.test.ts" + ], + "testFiles": [ + "src/main/runtime/terminal-tail-whitespace.test.ts", + "src/main/runtime/terminal-tail-buffer.test.ts", + "src/main/runtime/retained-tail-redraw-window.equivalence.test.ts" + ], + "assertionRefs": [ + { + "file": "src/main/runtime/terminal-tail-whitespace.test.ts", + "assertions": [ + "handles padded redraws across %i retained rows without stalling", + "preserves terminal text while trimming spaces and tabs: %j" + ] + } + ], + "evidenceRuns": [ + { + "date": "2026-09-06", + "runner": "local", + "platform": "macos", + "command": "ORCA_BACKGROUND_LAUNCH=1 pnpm test src/main/runtime/terminal-tail-whitespace.test.ts src/main/runtime/terminal-tail-buffer.test.ts src/main/runtime/retained-tail-redraw-window.equivalence.test.ts", + "result": "passed", + "durationSeconds": 3.96, + "summary": "17 tests passed. Before the fix both padding budget cases failed, taking approximately 1.7 seconds each." + } + ], + "runtimeBudget": { + "p95Seconds": 30, + "scope": "Three unit test files; padding cases allow 500 ms for four redraws." + }, + "flakeHistory": { + "status": "not-started", + "evidence": "Initial local red/green validation; no CI soak history yet." + }, + "redGreenEvidence": { + "status": "complete", + "evidence": "Both padding budget cases fail with regex trimming and pass with the existing linear trim. A 60-event CDP wheel stream in Pi fullscreen had about 2.1 seconds of output tail before the fix and 14 ms after rebuilding." + }, + "performanceBudget": { + "required": true, + "evidence": "The main CPU profile attributed 3.1 seconds to redraw-row whitespace trimming. Reusing the linear trim adds no timers, caches, provider calls, or output dropping." + }, + "knownGaps": [ + "The user manually compared the fixed dev app with production and confirmed improved responsiveness. A live Terminal.app comparison was not exercised; timing measurements used CDP wheel events.", + "Linux, Windows, and live SSH rendering were not exercised; the shared trimming behavior is covered by unit tests." + ], + "promotionCriteria": [ + "Complete CI soak requirements and retain the padding budget and tail equivalence oracles." + ], + "demotionRule": "Keep experimental until CI soak is stable; investigate any budget failure without weakening transcript preservation." + }, { "id": "ssh.localhost-terminal-agent-hooks", "title": "Localhost SSH terminal and agent hooks reach the owning pane", diff --git a/src/main/runtime/terminal-tail-buffer.ts b/src/main/runtime/terminal-tail-buffer.ts index 141b14da6ec..b3e15d1f375 100644 --- a/src/main/runtime/terminal-tail-buffer.ts +++ b/src/main/runtime/terminal-tail-buffer.ts @@ -293,7 +293,7 @@ function appendNormalizedToMultilineTailBuffer( const line = rewritten[index]! const lastChar = line.charCodeAt(line.length - 1) if (lastChar === 32 || lastChar === 9) { - rewritten[index] = line.replace(/[ \t]+$/g, '') + rewritten[index] = trimTerminalLineRight(line) } } for (const line of windowed.lines) { diff --git a/src/main/runtime/terminal-tail-redraw-buffer.ts b/src/main/runtime/terminal-tail-redraw-buffer.ts index cf90605fcd1..7ebb06753dd 100644 --- a/src/main/runtime/terminal-tail-redraw-buffer.ts +++ b/src/main/runtime/terminal-tail-redraw-buffer.ts @@ -185,7 +185,7 @@ function finalizeRetainedTerminalRows( newlyCompletedLines: string[] } { let truncated = initialTruncated - let retainedRows = rows.map((row) => ({ ...row, text: row.text.replace(/[ \t]+$/g, '') })) + let retainedRows = rows.map((row) => ({ ...row, text: trimTerminalLineRight(row.text) })) if (retainedRows.length > MAX_TAIL_LINES + 1) { const removeCount = retainedRows.length - (MAX_TAIL_LINES + 1) diff --git a/src/main/runtime/terminal-tail-whitespace.test.ts b/src/main/runtime/terminal-tail-whitespace.test.ts new file mode 100644 index 00000000000..280d3a02d50 --- /dev/null +++ b/src/main/runtime/terminal-tail-whitespace.test.ts @@ -0,0 +1,32 @@ +import { performance } from 'node:perf_hooks' +import { describe, expect, it } from 'vitest' +import { appendNormalizedToTailBuffer } from './terminal-tail-buffer' +import { trimTerminalLineRight } from './terminal-tail-line-controls' + +describe('terminal redraw whitespace', () => { + it.each([ + ['hello \t', 'hello'], + [' \thello \t world \t', ' \thello \t world'], + [' \t', ''], + ['hello\u00a0 \t', 'hello\u00a0'], + ['hello\n', 'hello\n'] + ])('preserves terminal text while trimming spaces and tabs: %j', (input, expected) => { + expect(trimTerminalLineRight(input)).toBe(expected) + }) + + it.each([2, 20])('handles padded redraws across %i retained rows without stalling', (rows) => { + const padded = `${' '.repeat(32_000)}marker \t` + const previousLines = Array.from({ length: rows }, (_, index) => + index === 0 ? padded : `row ${index}` + ) + const start = performance.now() + let result: ReturnType | undefined + for (let frame = 0; frame < 4; frame += 1) { + result = appendNormalizedToTailBuffer(previousLines, 'footer', '\x1b[1A\rupdated') + } + const elapsedMs = performance.now() - start + expect(result?.lines[0]).toBe(`${' '.repeat(32_000)}marker`) + // Interior padding made the trailing-whitespace regex backtrack quadratically. + expect(elapsedMs).toBeLessThan(500) + }) +}) From e4770d712f4dc16fe9b2c3ddb500be6e2cd6ac38 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Mon, 7 Sep 2026 02:58:32 -0400 Subject: [PATCH 5/9] Restore independent push gateway deployment (#19225) * Restore isolated push gateway deployment workflow * Register push deployment in the shared SQL lease census * Restore push workflow inventory and identity contracts --- .github/workflows/cloud-push-deploy.yml | 363 ++++++++++++++++++ .../scripts/cloud-sql-rollout-lock-census.mjs | 2 + ...ay-production-identity-boundaries.test.mjs | 3 +- .../relay-public-workflow-contract.test.mjs | 2 +- 4 files changed, 368 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/cloud-push-deploy.yml diff --git a/.github/workflows/cloud-push-deploy.yml b/.github/workflows/cloud-push-deploy.yml new file mode 100644 index 00000000000..38ad0664e54 --- /dev/null +++ b/.github/workflows/cloud-push-deploy.yml @@ -0,0 +1,363 @@ +name: Deploy Push Gateway Production + +on: + workflow_dispatch: + inputs: + source_sha: + description: Full reviewed commit SHA to build (feature may remain unmerged) + required: true + type: string + confirmation: + description: Enter DEPLOY_PUSH_GATEWAY to shift production traffic + required: true + type: string + +permissions: + contents: read + id-token: write + +# The gateway applies its own schema at startup against the shared Cloud SQL instance, so a +# deploy is a connection-budget rollout and belongs in the same serialized group as the relay. +concurrency: + group: production-cloud-sql-rollout + cancel-in-progress: false + +defaults: + run: + working-directory: cloud + +jobs: + deploy: + if: >- + ${{ vars.ORCA_CLOUD_OPERATIONS_ENABLED == 'true' && + github.ref == 'refs/heads/main' }} + runs-on: blacksmith-2vcpu-ubuntu-2204 + environment: production + env: + GCP_PROJECT_ID: onorca-cloud + GCP_REGION: ${{ vars.PRODUCTION_GCP_REGION }} + SERVICE_NAME: orca-cloud-push + REPOSITORY_ID: orca-cloud + IMAGE_NAME: push + PUSH_ORIGIN: https://push.onorca.dev + PUSH_RUNTIME_SERVICE_ACCOUNT: orca-cloud-push@onorca-cloud.iam.gserviceaccount.com + # Scaling the serving revision must already hold, matching push_min_instances and + # push_max_instances. Terraform owns both, and the candidate inherits them from the + # service, so this deploy never passes a scaling flag: doing so would write a + # Terraform-owned field that `lifecycle.ignore_changes` does not cover, and a later + # `push_max_instances` raise would then be reverted by every deploy. These two values + # are the expected shape, asserted before the candidate is created and again on the + # candidate itself, so a deploy that would change the gateway's Cloud SQL draw fails. + PUSH_MIN_INSTANCES: 1 + PUSH_MAX_INSTANCES: 2 + CONFIRMATION: ${{ inputs.confirmation }} + SOURCE_SHA: ${{ inputs.source_sha }} + steps: + - uses: actions/checkout@v4 + + - name: Require the explicit deploy confirmation + shell: bash + run: | + set -euo pipefail + test "${CONFIRMATION}" = DEPLOY_PUSH_GATEWAY + [[ "${SOURCE_SHA}" =~ ^[a-f0-9]{40}$ ]] + + # Keep the workflow and rollout lease on main; only the Docker build uses candidate code. + - name: Fetch the immutable gateway source + shell: bash + run: | + set -euo pipefail + git fetch --no-tags origin "${SOURCE_SHA}" + test "$(git rev-parse FETCH_HEAD)" = "${SOURCE_SHA}" + mkdir -p "${RUNNER_TEMP}/push-source" + git archive "${SOURCE_SHA}" cloud | tar -x -C "${RUNNER_TEMP}/push-source" + + - uses: google-github-actions/auth@v2 + with: + workload_identity_provider: ${{ vars.PRODUCTION_GCP_RELAY_DEPLOY_WORKLOAD_IDENTITY_PROVIDER }} + service_account: ${{ vars.PRODUCTION_GCP_RELAY_DEPLOY_SERVICE_ACCOUNT }} + + - uses: google-github-actions/setup-gcloud@v2 + + - uses: docker/setup-buildx-action@v3 + + - name: Configure Docker auth + run: gcloud auth configure-docker "${GCP_REGION}-docker.pkg.dev" --quiet + + # Why: the build runs before the lease. Artifact Registry is not the Cloud SQL instance, + # and a multi-minute image build inside the lease blocks every relay deploy and rehome for + # its duration. The lease below covers exactly the connection-budget window: deploy, probe, + # shift. + - name: Build and publish the immutable gateway image + shell: bash + run: | + set -euo pipefail + image_tag="${GCP_REGION}-docker.pkg.dev/${GCP_PROJECT_ID}/${REPOSITORY_ID}/${IMAGE_NAME}:sha-${SOURCE_SHA}" + docker build -f "${RUNNER_TEMP}/push-source/cloud/apps/push/Dockerfile" \ + -t "${image_tag}" "${RUNNER_TEMP}/push-source/cloud" + docker push "${image_tag}" + digest="$(gcloud artifacts docker images describe "${image_tag}" \ + --format='value(image_summary.digest)')" + [[ "${digest}" =~ ^sha256:[a-f0-9]{64}$ ]] + echo "IMAGE=${GCP_REGION}-docker.pkg.dev/${GCP_PROJECT_ID}/${REPOSITORY_ID}/${IMAGE_NAME}@${digest}" \ + >> "${GITHUB_ENV}" + echo "IMAGE_DIGEST=${digest}" >> "${GITHUB_ENV}" + + # Held across the deploy, not just a separate schema step: the gateway opens its pool and + # applies its schema while the new revision starts, so the revision is the schema step. + - uses: ./.github/actions/cloud-sql-rollout-lease + with: + bucket: onorca-cloud-terraform-state + object: terraform/state/cloud-sql-rollout/production.lock + + # Why: the candidate inherits the serving revision's scaling. A serving revision that has + # drifted below the floor would hand the candidate a cold start on every notification, and + # one that has drifted above the ceiling would hand it a larger Cloud SQL draw than the + # rollout lease was taken for. Refuse to inherit either rather than latch it. + - name: Record the serving revision and require its Terraform-owned scaling + shell: bash + run: | + set -euo pipefail + serving="$(gcloud run services describe "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --format=json \ + | jq -r '[.status.traffic[] | select((.percent // 0) > 0)] + | if length == 1 and .[0].percent == 100 then .[0].revisionName else empty end')" + test -n "${serving}" + floor="$(gcloud run revisions describe "${serving}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" \ + --format="value(metadata.annotations['autoscaling.knative.dev/minScale'])")" + if [[ "${floor:-0}" -lt "${PUSH_MIN_INSTANCES}" ]]; then + echo "serving revision ${serving} holds ${floor:-0} minimum instances," \ + "below ${PUSH_MIN_INSTANCES}; deploying would inherit and latch it." >&2 + echo "Restore the floor first: gcloud run services update ${SERVICE_NAME}" \ + "--region ${GCP_REGION} --min-instances=${PUSH_MIN_INSTANCES}" >&2 + exit 1 + fi + ceiling="$(gcloud run revisions describe "${serving}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" \ + --format="value(metadata.annotations['autoscaling.knative.dev/maxScale'])")" + test "${ceiling}" = "${PUSH_MAX_INSTANCES}" + echo "serving revision ${serving} holds ${floor} minimum and ${ceiling} maximum instances" + echo "ROLLBACK_REVISION=${serving}" >> "${GITHUB_ENV}" + + # No traffic and a per-revision tag: the candidate boots, applies schema, and is probed on + # its own URL while every phone and desktop still reaches the previous revision. + - name: Deploy the candidate revision with no traffic + shell: bash + run: | + set -euo pipefail + tag="c${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}" + echo "CANDIDATE_TAG=${tag}" >> "${GITHUB_ENV}" + echo "CANDIDATE_REVISION=${SERVICE_NAME}-${tag}" >> "${GITHUB_ENV}" + gcloud run deploy "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" \ + --region "${GCP_REGION}" \ + --image "${IMAGE}" \ + --tag "${tag}" \ + --revision-suffix "${tag}" \ + --no-traffic \ + --quiet + candidate="$(gcloud run services describe "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --format=json \ + | jq -er --arg tag "${tag}" \ + '[.status.traffic[] | select(.tag == $tag)] + | if length == 1 then .[0] else error("tagged candidate is not unique") end')" + test "$(jq -r '.revisionName' <<< "${candidate}")" = "${SERVICE_NAME}-${tag}" + echo "CANDIDATE_URL=$(jq -r '.url' <<< "${candidate}")" >> "${GITHUB_ENV}" + + # A tagged revision is directly addressable and sits outside the service-wide cap, so the + # candidate and the serving revision each draw up to the ceiling during the probe window. + # The lease is taken for exactly that doubling; a candidate that inherited a wider ceiling + # would exceed it, so the inherited scaling is asserted here too. + - name: Require the candidate to serve the exact image and inherited scaling + shell: bash + run: | + set -euo pipefail + served="$(gcloud run revisions describe "${CANDIDATE_REVISION}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" \ + --format='value(spec.containers[0].image)')" + test "${served}" = "${IMAGE}" + test "${CANDIDATE_REVISION}" != "${ROLLBACK_REVISION}" + candidate_ceiling="$(gcloud run revisions describe "${CANDIDATE_REVISION}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" \ + --format="value(metadata.annotations['autoscaling.knative.dev/maxScale'])")" + test "${candidate_ceiling}" = "${PUSH_MAX_INSTANCES}" + + - name: Probe the candidate readiness endpoint + shell: bash + run: | + set -euo pipefail + [[ "${CANDIDATE_URL}" =~ ^https://[^/]+$ ]] + for attempt in $(seq 1 30); do + code="$(curl -sS -o "${RUNNER_TEMP}/push-ready.json" -w '%{http_code}' \ + --max-time 10 "${CANDIDATE_URL}/ready" || true)" + if test "${code}" = 200; then + jq -e . < "${RUNNER_TEMP}/push-ready.json" > /dev/null + curl --fail --silent --show-error --max-time 10 "${CANDIDATE_URL}/health" \ + | jq -e '.ok == true and .deliveryProtocol == 2' > /dev/null + echo "candidate ${CANDIDATE_REVISION} is ready after ${attempt} attempt(s)" + exit 0 + fi + echo "attempt ${attempt}: /ready returned ${code}" + sleep 5 + done + echo "candidate ${CANDIDATE_REVISION} never reported ready" >&2 + exit 1 + + # Why: a gateway that boots and answers /ready can still be unable to send. This proves the + # runtime account's FCM grant end to end without delivering anything: validate_only stops + # Google before any push, and the deliberately invalid token means a healthy credential + # answers INVALID_ARGUMENT. PERMISSION_DENIED is the failure this step exists to catch. + # + # Only the four verdicts below are conclusive. A 429, a 5xx, or a transport failure says + # nothing about the credential, so it is retried rather than treated as either answer; a + # denied credential still fails on the first attempt, without burning the retries. + - name: Prove the runtime identity can reach FCM + shell: bash + run: | + set -euo pipefail + token="$(gcloud auth print-access-token \ + --impersonate-service-account "${PUSH_RUNTIME_SERVICE_ACCOUNT}")" + test -n "${token}" + echo "::add-mask::${token}" + body='{"validate_only":true,"message":{"token":"orca-push-deploy-probe-invalid-token","notification":{"title":"Orca","body":"deploy probe"}}}' + for attempt in $(seq 1 5); do + code="$(curl -sS -o "${RUNNER_TEMP}/push-fcm.json" -w '%{http_code}' --max-time 20 \ + -X POST "https://fcm.googleapis.com/v1/projects/${GCP_PROJECT_ID}/messages:send" \ + -H "Authorization: Bearer ${token}" \ + -H 'Content-Type: application/json' \ + --data "${body}" || true)" + status="$(jq -r '.error.status // empty' < "${RUNNER_TEMP}/push-fcm.json" || true)" + echo "attempt ${attempt}: FCM validate-only send returned HTTP ${code} status ${status:-OK}" + if test "${status}" = PERMISSION_DENIED || test "${status}" = INVALID_ARGUMENT || + test "${code}" = 401 || test "${code}" = 403; then + break + fi + sleep 5 + done + if test "${status}" = PERMISSION_DENIED || test "${code}" = 401 || test "${code}" = 403; then + echo "the push runtime identity cannot send through FCM" >&2 + exit 1 + fi + test "${status}" = INVALID_ARGUMENT + + - name: Shift all traffic to the verified candidate + shell: bash + run: | + set -euo pipefail + echo "TRAFFIC_SHIFT_ATTEMPTED=true" >> "${GITHUB_ENV}" + gcloud run services update-traffic "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" \ + --region "${GCP_REGION}" \ + --to-revisions "${CANDIDATE_REVISION}=100" \ + --quiet + serving="$(gcloud run services describe "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --format=json \ + | jq -r '[.status.traffic[] | select((.percent // 0) > 0)] + | if length == 1 and .[0].percent == 100 then .[0].revisionName else empty end')" + test "${serving}" = "${CANDIDATE_REVISION}" + echo "TRAFFIC_SHIFTED=true" >> "${GITHUB_ENV}" + + # Why: the summary is written before the origin check, not after it. Once traffic has + # moved, the rollback target is the single thing an operator needs, and a summary that only + # appeared on success would be missing in exactly the run that needs it. + - name: Publish the rollout summary + if: ${{ always() && env.CANDIDATE_REVISION != '' && env.ROLLBACK_REVISION != '' }} + shell: bash + run: | + set -euo pipefail + { + echo '### Push gateway rollout' + echo + echo "Source: ${SOURCE_SHA}" + echo + echo "Revision: \`${CANDIDATE_REVISION}\`" + echo + echo "Image: \`${IMAGE_DIGEST}\`" + echo + echo "Rollback: \`gcloud run services update-traffic ${SERVICE_NAME}" \ + "--region ${GCP_REGION} --to-revisions ${ROLLBACK_REVISION}=100\`" + } >> "${GITHUB_STEP_SUMMARY}" + + - name: Verify the public origin after the shift + shell: bash + run: | + set -euo pipefail + for attempt in $(seq 1 30); do + code="$(curl -sS -o /dev/null -w '%{http_code}' --max-time 10 \ + "${PUSH_ORIGIN}/ready" || true)" + if test "${code}" = 200; then + curl --fail --silent --show-error --max-time 10 "${PUSH_ORIGIN}/health" \ + | jq -e '.ok == true and .deliveryProtocol == 2' > /dev/null + echo "${PUSH_ORIGIN} is ready after ${attempt} attempt(s)" + exit 0 + fi + echo "attempt ${attempt}: ${PUSH_ORIGIN}/ready returned ${code}" + sleep 5 + done + echo "${PUSH_ORIGIN} never reported ready after the shift" >&2 + exit 1 + + # Why: everything after the shift runs with production on the candidate. A failure there + # is not a failure to deploy, it is a live gateway that has to go back, so the traffic move + # is undone here rather than left to whoever reads the run. + - name: Roll traffic back to the previous revision + if: ${{ (failure() || cancelled()) && env.TRAFFIC_SHIFT_ATTEMPTED == 'true' }} + shell: bash + run: | + set -euo pipefail + test -n "${ROLLBACK_REVISION:-}" + gcloud run services update-traffic "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" \ + --region "${GCP_REGION}" \ + --to-revisions "${ROLLBACK_REVISION}=100" \ + --quiet + serving="$(gcloud run services describe "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --format=json \ + | jq -r '[.status.traffic[] | select((.percent // 0) > 0)] + | if length == 1 and .[0].percent == 100 then .[0].revisionName else empty end')" + test "${serving}" = "${ROLLBACK_REVISION}" + echo "TRAFFIC_ROLLED_BACK=true" >> "${GITHUB_ENV}" + { + echo + echo '### Push gateway rolled back' + echo + echo "Traffic returned to \`${ROLLBACK_REVISION}\`; the candidate" \ + "\`${CANDIDATE_REVISION}\` no longer serves." + } >> "${GITHUB_STEP_SUMMARY}" + + # Why: a candidate that never took traffic is a revision holding a warm floor and a Cloud + # SQL pool for nothing. Its tag comes off first, because Cloud Run refuses to delete a + # revision a traffic target still names, and clearing CANDIDATE_TAG makes the always() tag + # step below a no-op rather than a second failure. + - name: Delete the rejected candidate revision + if: ${{ (failure() || cancelled()) && (env.TRAFFIC_SHIFT_ATTEMPTED != 'true' || env.TRAFFIC_ROLLED_BACK == 'true') }} + shell: bash + run: | + set -euo pipefail + test -n "${CANDIDATE_REVISION:-}" || exit 0 + if test -n "${CANDIDATE_TAG:-}"; then + gcloud run services update-traffic "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" \ + --region "${GCP_REGION}" \ + --remove-tags "${CANDIDATE_TAG}" \ + --quiet + echo "CANDIDATE_TAG=" >> "${GITHUB_ENV}" + fi + gcloud run revisions delete "${CANDIDATE_REVISION}" \ + --project "${GCP_PROJECT_ID}" \ + --region "${GCP_REGION}" \ + --quiet + echo "deleted the candidate revision ${CANDIDATE_REVISION}" + + - name: Drop the candidate traffic tag + if: always() + shell: bash + run: | + set -euo pipefail + test -n "${CANDIDATE_TAG:-}" || exit 0 + gcloud run services update-traffic "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" \ + --region "${GCP_REGION}" \ + --remove-tags "${CANDIDATE_TAG}" \ + --quiet diff --git a/cloud/dev/scripts/cloud-sql-rollout-lock-census.mjs b/cloud/dev/scripts/cloud-sql-rollout-lock-census.mjs index 76193746f2c..2f7157d823c 100644 --- a/cloud/dev/scripts/cloud-sql-rollout-lock-census.mjs +++ b/cloud/dev/scripts/cloud-sql-rollout-lock-census.mjs @@ -283,6 +283,8 @@ export const LEASED_WORKFLOWS = named([ 'operate-relay-production-rehome.yml', production({ leaseFiles: ['operate-relay-production-rehome-job.yml'] }) ], + // The gateway applies its schema at startup, so its deploy revision is the schema step. + ['push-deploy.yml', production()], ['deploy-relay-asia-topology.yml', eitherEnvironment()], ['operate-relay-asia-admission.yml', eitherEnvironment()], ['deploy-relay-staging.yml', staging()], diff --git a/cloud/dev/scripts/relay-production-identity-boundaries.test.mjs b/cloud/dev/scripts/relay-production-identity-boundaries.test.mjs index 7e8ea2a05c1..f97e742215b 100644 --- a/cloud/dev/scripts/relay-production-identity-boundaries.test.mjs +++ b/cloud/dev/scripts/relay-production-identity-boundaries.test.mjs @@ -32,7 +32,8 @@ test('no workflow names the retired generic production deploy identity', async ( 'deploy-relay-production.yml', 'operate-relay-asia-admission.yml', 'operate-relay-production-rehome-job.yml', - 'publish-relay-production.yml' + 'publish-relay-production.yml', + 'push-deploy.yml' ].map((name) => relayWorkflowFile(name)).sort()) }) diff --git a/cloud/dev/scripts/relay-public-workflow-contract.test.mjs b/cloud/dev/scripts/relay-public-workflow-contract.test.mjs index 56393d07bd1..d25ffb221f4 100644 --- a/cloud/dev/scripts/relay-public-workflow-contract.test.mjs +++ b/cloud/dev/scripts/relay-public-workflow-contract.test.mjs @@ -20,7 +20,7 @@ const UNGATED = relayWorkflowFile('verify.yml') const relayWorkflows = () => workflowFiles().filter((file) => file !== UNGATED) test('the copy carries every relay workflow', () => { - assert.equal(relayWorkflows().length, 24) + assert.equal(relayWorkflows().length, 25) }) // Why: workflow_run chains match by display name, not filename. Renaming a file is safe; renaming From a62cfedad8a668305f88d4d9ddd1ca00a166af29 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Mon, 7 Sep 2026 03:04:19 -0400 Subject: [PATCH 6/9] Resolve push source archive from repository root (#19231) --- .github/workflows/cloud-push-deploy.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/cloud-push-deploy.yml b/.github/workflows/cloud-push-deploy.yml index 38ad0664e54..ea19a589bb2 100644 --- a/.github/workflows/cloud-push-deploy.yml +++ b/.github/workflows/cloud-push-deploy.yml @@ -70,7 +70,8 @@ jobs: git fetch --no-tags origin "${SOURCE_SHA}" test "$(git rev-parse FETCH_HEAD)" = "${SOURCE_SHA}" mkdir -p "${RUNNER_TEMP}/push-source" - git archive "${SOURCE_SHA}" cloud | tar -x -C "${RUNNER_TEMP}/push-source" + git -C "${GITHUB_WORKSPACE}" archive "${SOURCE_SHA}" cloud \ + | tar -x -C "${RUNNER_TEMP}/push-source" - uses: google-github-actions/auth@v2 with: From 3f4793b6c95959b508e549659c02c93b627f545a Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 7 Sep 2026 00:12:32 -0700 Subject: [PATCH 7/9] Reorganize MiniMax modules and de-duplicate shared test state (#19197) * Move MiniMax quota fetch modules into rate-limits/minimax The five MiniMax fetch/transport modules sat flat among ~110 files covering eight providers. Nest them so the provider's fetch surface is one directory; credential stores (main/minimax) and the IPC handler (main/ipc) stay where their siblings are. * Build rate-limit and settings test state from shared factories RateLimitState was hand-copied in 9 places and the full GlobalSettings object in 2 more, so adding one provider field forced edits in unrelated providers' files -- which is how MiniMax fields ended up in codex-accounts and the Grok usage-pane test. Add createEmptyRateLimitState and createGlobalSettingsFixture and route the copies through them. Values that deviated from the defaults are passed as explicit overrides, so the fixtures produce what they produced before. rate-limit-types.test.ts keeps its literal (it exists to assert the shape) and service-state.ts keeps its own (InternalRateLimitState is a subset, not the same type). * Share the codex-account settings fixture between both harnesses The two codex-account fixtures still carried the same 30-line override block verbatim, which is the duplication the shared fixture was meant to remove. Move it into one createCodexAccountSettings and have both call it. Also drop the hardcoded POSIX workspaceDir default; callers supply the real directory and a '/tmp' literal would be a trap on Windows. --- .../codex-account-settings-fixture.ts | 37 ++++++ .../runtime-home-settings-test-fixtures.ts | 125 +----------------- .../service-reset-credit-test-fixtures.ts | 19 +-- .../codex-accounts/service-test-harness.ts | 125 +----------------- src/main/ipc/minimax-credentials.test.ts | 2 +- src/main/ipc/minimax-credentials.ts | 2 +- ...roxy-guarded-fetch-call-site-audit.test.ts | 2 +- .../{ => minimax}/minimax-fetcher-data.ts | 2 +- .../{ => minimax}/minimax-fetcher-parse.ts | 2 +- .../{ => minimax}/minimax-fetcher.test.ts | 0 .../{ => minimax}/minimax-fetcher.ts | 4 +- .../minimax-request-context.test.ts | 0 .../{ => minimax}/minimax-request-context.ts | 2 +- .../rate-limit-service-test-harness.ts | 2 +- .../service-account-target-selection.test.ts | 2 +- .../service-antigravity-usage.test.ts | 2 +- .../service-inactive-account-previews.test.ts | 2 +- .../service-live-claude-usage.test.ts | 2 +- .../rate-limits/service-minimax-usage.test.ts | 4 +- .../service-refresh-orchestration.test.ts | 4 +- .../service-window-activation.test.ts | 4 +- .../service/service-full-cycle-preparation.ts | 2 +- .../components/stats/GrokUsagePane.test.tsx | 20 +-- .../status-bar-provider-visibility.test.ts | 33 +---- .../useIpcEvents-rate-limit-hydration.test.ts | 13 +- src/renderer/src/store/slices/rate-limits.ts | 19 +-- .../web/preload-api/web-rate-limits-api.ts | 20 +-- src/shared/global-settings-test-fixture.ts | 26 ++++ src/shared/rate-limit-state-factory.ts | 23 ++++ 29 files changed, 125 insertions(+), 375 deletions(-) create mode 100644 src/main/codex-accounts/codex-account-settings-fixture.ts rename src/main/rate-limits/{ => minimax}/minimax-fetcher-data.ts (99%) rename src/main/rate-limits/{ => minimax}/minimax-fetcher-parse.ts (98%) rename src/main/rate-limits/{ => minimax}/minimax-fetcher.test.ts (100%) rename src/main/rate-limits/{ => minimax}/minimax-fetcher.ts (96%) rename src/main/rate-limits/{ => minimax}/minimax-request-context.test.ts (100%) rename src/main/rate-limits/{ => minimax}/minimax-request-context.ts (99%) create mode 100644 src/shared/global-settings-test-fixture.ts create mode 100644 src/shared/rate-limit-state-factory.ts diff --git a/src/main/codex-accounts/codex-account-settings-fixture.ts b/src/main/codex-accounts/codex-account-settings-fixture.ts new file mode 100644 index 00000000000..b048bb8e3d6 --- /dev/null +++ b/src/main/codex-accounts/codex-account-settings-fixture.ts @@ -0,0 +1,37 @@ +import type { GlobalSettings } from '../../shared/global-settings-types' +import { createGlobalSettingsFixture } from '../../shared/global-settings-test-fixture' + +// Why: these values predate buildDefaultSettings' current defaults; codex-account suites assert against them. +export function createCodexAccountSettings( + workspaceDir: string, + overrides: Partial = {} +): GlobalSettings { + return createGlobalSettingsFixture({ + workspaceDir, + nestWorkspaces: false, + autoRenameBranchFromWork: false, + terminalCursorBlink: false, + terminalThemeDark: 'orca-dark', + terminalDividerColorDark: '#000000', + terminalUseSeparateLightTheme: false, + terminalThemeLight: 'orca-light', + terminalDividerColorLight: '#ffffff', + terminalPaneOpacityTransitionMs: 150, + terminalDividerThicknessPx: 1, + setupScriptLaunchMode: 'split-vertical', + localAccountRuntime: 'host', + floatingTerminalEnabled: false, + terminalMacOptionAsAlt: 'false', + terminalMacOptionAsAltMigrated: true, + experimentalActivity: true, + terminalWindowsPowerShellImplementation: 'powershell.exe', + ...overrides, + diffWordWrap: overrides.diffWordWrap ?? false, + diffShowWhitespace: overrides.diffShowWhitespace ?? false, + localWindowsRuntimeDefault: overrides.localWindowsRuntimeDefault ?? { kind: 'windows-host' }, + leftSidebarAppearanceMode: overrides.leftSidebarAppearanceMode ?? 'default', + appFontFamily: overrides.appFontFamily ?? 'Geist', + agentStatusHooksEnabled: overrides.agentStatusHooksEnabled ?? true, + tabAutoGenerateTitle: overrides.tabAutoGenerateTitle ?? false + }) +} diff --git a/src/main/codex-accounts/runtime-home-settings-test-fixtures.ts b/src/main/codex-accounts/runtime-home-settings-test-fixtures.ts index c7be08b3509..d4f6b40c2d8 100644 --- a/src/main/codex-accounts/runtime-home-settings-test-fixtures.ts +++ b/src/main/codex-accounts/runtime-home-settings-test-fixtures.ts @@ -1,4 +1,5 @@ import type { GlobalSettings } from '../../shared/global-settings-types' +import { createCodexAccountSettings } from './codex-account-settings-fixture' import { setShellStartupEnvProbeSupportedForTest, testState @@ -12,130 +13,8 @@ type TestSettingsOverrides = Partial & { } export function createSettings(overrides: TestSettingsOverrides = {}): GlobalSettings { - const appFontFamily = overrides.appFontFamily ?? 'Geist' - const agentStatusHooksEnabled = overrides.agentStatusHooksEnabled ?? true - const tabAutoGenerateTitle = overrides.tabAutoGenerateTitle ?? false // Mirror-path tests assert the shared runtime home, which production still uses // on Windows; opt these cases onto that lane unless a test overrides it. setShellStartupEnvProbeSupportedForTest(overrides.shellStartupEnvProbeSupported ?? false) - return { - workspaceDir: testState.fakeHomeDir, - nestWorkspaces: false, - refreshLocalBaseRefOnWorktreeCreate: false, - localBaseRefSuggestionDismissed: false, - autoRenameBranchFromWork: false, - branchPrefix: 'git-username', - branchPrefixCustom: '', - theme: 'system', - uiLanguage: 'system', - appIcon: overrides.appIcon ?? 'classic', - editorAutoSave: false, - editorAutoSaveDelayMs: 1000, - editorMinimapEnabled: false, - markdownReviewToolsEnabled: true, - terminalFontSize: 14, - terminalFontFamily: 'JetBrains Mono', - terminalFontWeightBold: 700, - terminalFontWeight: 500, - terminalLineHeight: 1, - terminalScrollSensitivity: 1.15, - terminalFastScrollSensitivity: 5, - terminalTuiScrollSensitivity: 1, - terminalGpuAcceleration: 'auto', - terminalLigatures: 'auto', - terminalCursorStyle: 'block', - terminalCursorBlink: false, - terminalThemeDark: 'orca-dark', - terminalDividerColorDark: '#000000', - terminalUseSeparateLightTheme: false, - terminalThemeLight: 'orca-light', - terminalDividerColorLight: '#ffffff', - terminalInactivePaneOpacity: 0.5, - terminalActivePaneOpacity: 1, - terminalPaneOpacityTransitionMs: 150, - terminalDividerThicknessPx: 1, - terminalRightClickToPaste: false, - terminalFocusFollowsMouse: false, - terminalClipboardOnSelect: false, - terminalAllowOsc52Clipboard: true, - setupScriptLaunchMode: 'split-vertical', - terminalScrollbackRows: 5_000, - localAccountRuntime: 'host', - localAccountWslDistro: null, - openLinksInApp: false, - openLinksInAppPreferencePrompted: false, - rightSidebarOpenByDefault: true, - sourceControlViewMode: 'list', - sourceControlGroupOrder: 'changes-first', - sourceControlCompareAgainstUpstream: false, - showTitlebarAppName: true, - showTasksButton: true, - floatingTerminalEnabled: false, - floatingTerminalCwd: '~', - floatingTerminalTriggerLocation: 'floating-button', - diffDefaultView: 'inline', - combinedDiffFileTreeVisibleByDefault: false, - prBotAuthorOverrides: [], - notifications: { - enabled: true, - agentTaskComplete: true, - terminalBell: false, - suppressWhenFocused: true, - customSoundId: 'system', - customSoundPath: null, - customSoundVolume: 100 - }, - promptCacheTimerEnabled: false, - promptCacheTtlMs: 300_000, - codexManagedAccounts: [], - activeCodexManagedAccountId: null, - claudeManagedAccounts: [], - activeClaudeManagedAccountId: null, - terminalScopeHistoryByWorktree: true, - defaultTuiAgent: null, - disabledTuiAgents: [], - pluginSystemEnabled: false, - disabledPlugins: [], - pluginConsents: {}, - devPluginPaths: [], - skipDeleteWorktreeConfirm: false, - skipCloseTerminalWithRunningProcessConfirm: false, - skipDeleteAutomationConfirm: false, - skipDeleteArtifactConfirm: false, - skipCodexRateLimitResetConfirm: false, - defaultTaskViewPreset: 'all', - defaultTaskSource: 'github', - visibleTaskProviders: ['github', 'gitlab', 'linear', 'jira'], - visibleTaskProvidersDefaultedForJira: true, - defaultRepoSelection: null, - defaultLinearTeamSelection: null, - opencodeSessionCookie: '', - opencodeWorkspaceId: '', - minimaxGroupId: '', - minimaxUsageModels: 'general', - minimaxEndpoint: 'overseas', - geminiCliOAuthEnabled: false, - agentCmdOverrides: {}, - keepComputerAwakeWhileAgentsRun: false, - confirmClosePinnedTab: true, - terminalMacOptionAsAlt: 'false', - terminalMacOptionAsAltMigrated: true, - terminalJISYenToBackslash: false, - experimentalMobile: false, - mobileAutoRestoreFitMs: null, - experimentalPet: false, - experimentalActivity: true, - experimentalTerminalAttention: false, - compactWorktreeCards: false, - terminalWindowsShell: 'powershell.exe', - terminalWindowsPowerShellImplementation: 'powershell.exe', - ...overrides, - diffWordWrap: overrides.diffWordWrap ?? false, - diffShowWhitespace: overrides.diffShowWhitespace ?? false, - localWindowsRuntimeDefault: overrides.localWindowsRuntimeDefault ?? { kind: 'windows-host' }, - leftSidebarAppearanceMode: overrides.leftSidebarAppearanceMode ?? 'default', - appFontFamily, - agentStatusHooksEnabled, - tabAutoGenerateTitle - } + return createCodexAccountSettings(testState.fakeHomeDir, overrides) } diff --git a/src/main/codex-accounts/service-reset-credit-test-fixtures.ts b/src/main/codex-accounts/service-reset-credit-test-fixtures.ts index d47c967e34c..39b52bf5d1f 100644 --- a/src/main/codex-accounts/service-reset-credit-test-fixtures.ts +++ b/src/main/codex-accounts/service-reset-credit-test-fixtures.ts @@ -1,4 +1,5 @@ import type { ProviderRateLimits, RateLimitState } from '../../shared/rate-limit-types' +import { createEmptyRateLimitState } from '../../shared/rate-limit-state-factory' export function createResetCreditLimits(updatedAt = 30): ProviderRateLimits { return { @@ -26,21 +27,5 @@ export function createResetRateLimitState( codex: ProviderRateLimits, target: RateLimitState['codexTarget'] = { runtime: 'host', wslDistro: null } ): RateLimitState { - return { - claude: null, - codex, - gemini: null, - opencodeGo: null, - kimi: null, - antigravity: null, - minimax: null, - grok: null, - minimaxCookieConfigured: false, - minimaxApiKeyConfigured: false, - grokAuthConfigured: false, - claudeTarget: { runtime: 'host', wslDistro: null }, - codexTarget: target, - inactiveClaudeAccounts: [], - inactiveCodexAccounts: [] - } + return createEmptyRateLimitState({ codex, codexTarget: target }) } diff --git a/src/main/codex-accounts/service-test-harness.ts b/src/main/codex-accounts/service-test-harness.ts index 6c0a33135ab..4587788ec67 100644 --- a/src/main/codex-accounts/service-test-harness.ts +++ b/src/main/codex-accounts/service-test-harness.ts @@ -3,6 +3,7 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import type { GlobalSettings } from '../../shared/global-settings-types' +import { createCodexAccountSettings } from './codex-account-settings-fixture' import type { CodexResetCreditAttemptLedger } from '../../shared/codex-reset-credit-attempt-ledger' import type { CodexRateLimitHomeResolution } from './runtime-home-service' @@ -36,129 +37,7 @@ export function registerCodexAccountsTestHomes(): void { } export function createSettings(overrides: Partial = {}): GlobalSettings { - const appFontFamily = overrides.appFontFamily ?? 'Geist' - const agentStatusHooksEnabled = overrides.agentStatusHooksEnabled ?? true - const tabAutoGenerateTitle = overrides.tabAutoGenerateTitle ?? false - return { - workspaceDir: testState.fakeHomeDir, - nestWorkspaces: false, - refreshLocalBaseRefOnWorktreeCreate: false, - localBaseRefSuggestionDismissed: false, - autoRenameBranchFromWork: false, - branchPrefix: 'git-username', - branchPrefixCustom: '', - theme: 'system', - uiLanguage: 'system', - appIcon: overrides.appIcon ?? 'classic', - editorAutoSave: false, - editorAutoSaveDelayMs: 1000, - editorMinimapEnabled: false, - markdownReviewToolsEnabled: true, - terminalFontSize: 14, - terminalFontFamily: 'JetBrains Mono', - terminalFontWeightBold: 700, - terminalFontWeight: 500, - terminalLineHeight: 1, - terminalScrollSensitivity: 1.15, - terminalFastScrollSensitivity: 5, - terminalTuiScrollSensitivity: 1, - terminalGpuAcceleration: 'auto', - terminalLigatures: 'auto', - terminalCursorStyle: 'block', - terminalCursorBlink: false, - terminalThemeDark: 'orca-dark', - terminalDividerColorDark: '#000000', - terminalUseSeparateLightTheme: false, - terminalThemeLight: 'orca-light', - terminalDividerColorLight: '#ffffff', - terminalInactivePaneOpacity: 0.5, - terminalActivePaneOpacity: 1, - terminalPaneOpacityTransitionMs: 150, - terminalDividerThicknessPx: 1, - terminalRightClickToPaste: false, - terminalFocusFollowsMouse: false, - terminalClipboardOnSelect: false, - terminalAllowOsc52Clipboard: true, - setupScriptLaunchMode: 'split-vertical', - terminalScrollbackRows: 5_000, - localAccountRuntime: 'host', - localAccountWslDistro: null, - openLinksInApp: false, - openLinksInAppPreferencePrompted: false, - rightSidebarOpenByDefault: true, - sourceControlViewMode: 'list', - sourceControlGroupOrder: 'changes-first', - sourceControlCompareAgainstUpstream: false, - showTitlebarAppName: true, - showTasksButton: true, - floatingTerminalEnabled: false, - floatingTerminalCwd: '~', - floatingTerminalTriggerLocation: 'floating-button', - diffDefaultView: 'inline', - combinedDiffFileTreeVisibleByDefault: false, - prBotAuthorOverrides: [], - notifications: { - enabled: true, - agentTaskComplete: true, - terminalBell: false, - suppressWhenFocused: true, - customSoundId: 'system', - customSoundPath: null, - customSoundVolume: 100 - }, - promptCacheTimerEnabled: false, - promptCacheTtlMs: 300_000, - codexManagedAccounts: [], - activeCodexManagedAccountId: null, - claudeManagedAccounts: [], - activeClaudeManagedAccountId: null, - terminalScopeHistoryByWorktree: true, - defaultTuiAgent: null, - disabledTuiAgents: [], - pluginSystemEnabled: false, - disabledPlugins: [], - pluginConsents: {}, - devPluginPaths: [], - skipDeleteWorktreeConfirm: false, - skipCloseTerminalWithRunningProcessConfirm: false, - skipDeleteAutomationConfirm: false, - skipDeleteArtifactConfirm: false, - skipCodexRateLimitResetConfirm: false, - defaultTaskViewPreset: 'all', - defaultTaskSource: 'github', - visibleTaskProviders: ['github', 'gitlab', 'linear', 'jira'], - visibleTaskProvidersDefaultedForJira: true, - defaultRepoSelection: null, - defaultLinearTeamSelection: null, - opencodeSessionCookie: '', - opencodeWorkspaceId: '', - minimaxGroupId: '', - minimaxUsageModels: 'general', - minimaxEndpoint: 'overseas', - geminiCliOAuthEnabled: false, - agentCmdOverrides: {}, - keepComputerAwakeWhileAgentsRun: false, - confirmClosePinnedTab: true, - terminalMacOptionAsAlt: 'false', - terminalMacOptionAsAltMigrated: true, - terminalJISYenToBackslash: false, - experimentalMobile: false, - mobileAutoRestoreFitMs: null, - experimentalPet: false, - experimentalActivity: true, - experimentalTerminalAttention: false, - compactWorktreeCards: false, - terminalWindowsShell: 'powershell.exe', - terminalWindowsPowerShellImplementation: 'powershell.exe', - ...overrides, - diffWordWrap: overrides.diffWordWrap ?? false, - diffShowWhitespace: overrides.diffShowWhitespace ?? false, - localWindowsRuntimeDefault: overrides.localWindowsRuntimeDefault ?? { kind: 'windows-host' }, - leftSidebarAppearanceMode: overrides.leftSidebarAppearanceMode ?? 'default', - appFontFamily, - agentStatusHooksEnabled, - tabAutoGenerateTitle - } + return createCodexAccountSettings(testState.fakeHomeDir, overrides) } export function createStore(settings: GlobalSettings) { diff --git a/src/main/ipc/minimax-credentials.test.ts b/src/main/ipc/minimax-credentials.test.ts index 242ee2217bc..e4d39e38a29 100644 --- a/src/main/ipc/minimax-credentials.test.ts +++ b/src/main/ipc/minimax-credentials.test.ts @@ -32,7 +32,7 @@ vi.mock('../minimax/minimax-api-key-store', () => ({ hasMiniMaxApiKey: hasMiniMaxApiKeyMock })) -vi.mock('../rate-limits/minimax-request-context', () => ({ +vi.mock('../rate-limits/minimax/minimax-request-context', () => ({ clearMiniMaxSessionCookieJar: clearMiniMaxSessionCookieJarMock })) diff --git a/src/main/ipc/minimax-credentials.ts b/src/main/ipc/minimax-credentials.ts index bd0368a1c9e..12138967f2c 100644 --- a/src/main/ipc/minimax-credentials.ts +++ b/src/main/ipc/minimax-credentials.ts @@ -9,7 +9,7 @@ import { hasMiniMaxApiKey, saveMiniMaxApiKey } from '../minimax/minimax-api-key-store' -import { clearMiniMaxSessionCookieJar } from '../rate-limits/minimax-request-context' +import { clearMiniMaxSessionCookieJar } from '../rate-limits/minimax/minimax-request-context' import type { RateLimitService } from '../rate-limits/service' export type MiniMaxCredentialsStatus = { diff --git a/src/main/proxy-guarded-fetch-call-site-audit.test.ts b/src/main/proxy-guarded-fetch-call-site-audit.test.ts index cf9e1f60609..4427b11f55d 100644 --- a/src/main/proxy-guarded-fetch-call-site-audit.test.ts +++ b/src/main/proxy-guarded-fetch-call-site-audit.test.ts @@ -17,7 +17,7 @@ const AUDITED_NON_NET_FETCH_CALLS = new Map([ ['main/rate-limits/opencode-go-usage-fetcher.ts', 2], // Isolated cookie-jar session that does NOT apply the proxy — a pre-existing gap, not a // regression: no proxy has ever reached this partition. Keep it listed so it stays visible. - ['main/rate-limits/minimax-request-context.ts', 2], + ['main/rate-limits/minimax/minimax-request-context.ts', 2], // Injected HttpClient, not a session: resolves to net.fetch on defaultSession // (main/host/electron-http-client.ts) or to the global-fetch-audited Node fallback. ['main/jira/authenticated-request.ts', 1] diff --git a/src/main/rate-limits/minimax-fetcher-data.ts b/src/main/rate-limits/minimax/minimax-fetcher-data.ts similarity index 99% rename from src/main/rate-limits/minimax-fetcher-data.ts rename to src/main/rate-limits/minimax/minimax-fetcher-data.ts index b2c6eddad6e..19e6d3f6768 100644 --- a/src/main/rate-limits/minimax-fetcher-data.ts +++ b/src/main/rate-limits/minimax/minimax-fetcher-data.ts @@ -1,4 +1,4 @@ -import type { ProviderRateLimits, RateLimitWindow } from '../../shared/rate-limit-types' +import type { ProviderRateLimits, RateLimitWindow } from '../../../shared/rate-limit-types' // Why: pure data-shape helpers for the MiniMax Coding Plan API. Lives in its // own file so both minimax-fetcher.ts (transport) and minimax-fetcher-parse.ts diff --git a/src/main/rate-limits/minimax-fetcher-parse.ts b/src/main/rate-limits/minimax/minimax-fetcher-parse.ts similarity index 98% rename from src/main/rate-limits/minimax-fetcher-parse.ts rename to src/main/rate-limits/minimax/minimax-fetcher-parse.ts index 98efb88cd6d..8e776654b15 100644 --- a/src/main/rate-limits/minimax-fetcher-parse.ts +++ b/src/main/rate-limits/minimax/minimax-fetcher-parse.ts @@ -1,4 +1,4 @@ -import type { ProviderRateLimits } from '../../shared/rate-limit-types' +import type { ProviderRateLimits } from '../../../shared/rate-limit-types' import { logMiniMaxFetchFailure, redactMiniMaxSecret, diff --git a/src/main/rate-limits/minimax-fetcher.test.ts b/src/main/rate-limits/minimax/minimax-fetcher.test.ts similarity index 100% rename from src/main/rate-limits/minimax-fetcher.test.ts rename to src/main/rate-limits/minimax/minimax-fetcher.test.ts diff --git a/src/main/rate-limits/minimax-fetcher.ts b/src/main/rate-limits/minimax/minimax-fetcher.ts similarity index 96% rename from src/main/rate-limits/minimax-fetcher.ts rename to src/main/rate-limits/minimax/minimax-fetcher.ts index a56edb2d257..36c15fe0b51 100644 --- a/src/main/rate-limits/minimax-fetcher.ts +++ b/src/main/rate-limits/minimax/minimax-fetcher.ts @@ -1,5 +1,5 @@ -import type { ProviderRateLimits } from '../../shared/rate-limit-types' -import type { MiniMaxEndpoint } from '../../shared/global-settings-types' +import type { ProviderRateLimits } from '../../../shared/rate-limit-types' +import type { MiniMaxEndpoint } from '../../../shared/global-settings-types' import { extractMiniMaxCookieValue, fetchMiniMaxWithApiKey, diff --git a/src/main/rate-limits/minimax-request-context.test.ts b/src/main/rate-limits/minimax/minimax-request-context.test.ts similarity index 100% rename from src/main/rate-limits/minimax-request-context.test.ts rename to src/main/rate-limits/minimax/minimax-request-context.test.ts diff --git a/src/main/rate-limits/minimax-request-context.ts b/src/main/rate-limits/minimax/minimax-request-context.ts similarity index 99% rename from src/main/rate-limits/minimax-request-context.ts rename to src/main/rate-limits/minimax/minimax-request-context.ts index 10a1ea07b91..579d8ae3d0b 100644 --- a/src/main/rate-limits/minimax-request-context.ts +++ b/src/main/rate-limits/minimax/minimax-request-context.ts @@ -1,5 +1,5 @@ import { net, session, type Session } from 'electron' -import type { MiniMaxEndpoint } from '../../shared/global-settings-types' +import type { MiniMaxEndpoint } from '../../../shared/global-settings-types' const MINIMAX_USAGE_PATH = '/v1/api/openplatform/coding_plan/remains' const MINIMAX_OVERSEAS_BASE = 'https://platform.minimax.io' diff --git a/src/main/rate-limits/rate-limit-service-test-harness.ts b/src/main/rate-limits/rate-limit-service-test-harness.ts index 00d3db16002..041ede99b62 100644 --- a/src/main/rate-limits/rate-limit-service-test-harness.ts +++ b/src/main/rate-limits/rate-limit-service-test-harness.ts @@ -5,7 +5,7 @@ import type { RateLimitService } from './service' import { fetchCodexRateLimits } from './codex-fetcher' import { fetchGeminiRateLimits } from './gemini-usage-fetcher' import { fetchKimiRateLimits } from './kimi-fetcher' -import { fetchMiniMaxRateLimits } from './minimax-fetcher' +import { fetchMiniMaxRateLimits } from './minimax/minimax-fetcher' import { fetchGrokRateLimits } from './grok-fetcher' import { readGrokAuthSession } from './grok-auth' import { fetchOpenCodeGoRateLimits } from './opencode-go-usage-fetcher' diff --git a/src/main/rate-limits/service-account-target-selection.test.ts b/src/main/rate-limits/service-account-target-selection.test.ts index 86008a4bd84..9241831bdf6 100644 --- a/src/main/rate-limits/service-account-target-selection.test.ts +++ b/src/main/rate-limits/service-account-target-selection.test.ts @@ -32,7 +32,7 @@ vi.mock('./opencode-go-usage-fetcher', () => ({ fetchOpenCodeGoRateLimits: vi.fn() })) -vi.mock('./minimax-fetcher', () => ({ +vi.mock('./minimax/minimax-fetcher', () => ({ fetchMiniMaxRateLimits: vi.fn() })) diff --git a/src/main/rate-limits/service-antigravity-usage.test.ts b/src/main/rate-limits/service-antigravity-usage.test.ts index b617d909944..e0975af0db0 100644 --- a/src/main/rate-limits/service-antigravity-usage.test.ts +++ b/src/main/rate-limits/service-antigravity-usage.test.ts @@ -31,7 +31,7 @@ vi.mock('./opencode-go-usage-fetcher', () => ({ fetchOpenCodeGoRateLimits: vi.fn() })) -vi.mock('./minimax-fetcher', () => ({ +vi.mock('./minimax/minimax-fetcher', () => ({ fetchMiniMaxRateLimits: vi.fn() })) diff --git a/src/main/rate-limits/service-inactive-account-previews.test.ts b/src/main/rate-limits/service-inactive-account-previews.test.ts index e734b52172a..3c629351382 100644 --- a/src/main/rate-limits/service-inactive-account-previews.test.ts +++ b/src/main/rate-limits/service-inactive-account-previews.test.ts @@ -39,7 +39,7 @@ vi.mock('./opencode-go-usage-fetcher', () => ({ fetchOpenCodeGoRateLimits: vi.fn() })) -vi.mock('./minimax-fetcher', () => ({ +vi.mock('./minimax/minimax-fetcher', () => ({ fetchMiniMaxRateLimits: vi.fn() })) diff --git a/src/main/rate-limits/service-live-claude-usage.test.ts b/src/main/rate-limits/service-live-claude-usage.test.ts index 59c1300532d..668cfc113b9 100644 --- a/src/main/rate-limits/service-live-claude-usage.test.ts +++ b/src/main/rate-limits/service-live-claude-usage.test.ts @@ -36,7 +36,7 @@ vi.mock('./opencode-go-usage-fetcher', () => ({ fetchOpenCodeGoRateLimits: vi.fn() })) -vi.mock('./minimax-fetcher', () => ({ +vi.mock('./minimax/minimax-fetcher', () => ({ fetchMiniMaxRateLimits: vi.fn() })) diff --git a/src/main/rate-limits/service-minimax-usage.test.ts b/src/main/rate-limits/service-minimax-usage.test.ts index 7c3db21c63e..819b4c93f5d 100644 --- a/src/main/rate-limits/service-minimax-usage.test.ts +++ b/src/main/rate-limits/service-minimax-usage.test.ts @@ -3,7 +3,7 @@ import type { ProviderRateLimits } from '../../shared/rate-limit-types' import { RateLimitService } from './service' import { fetchClaudeRateLimits } from './claude-fetcher' import { fetchCodexRateLimits } from './codex-fetcher' -import { fetchMiniMaxRateLimits } from './minimax-fetcher' +import { fetchMiniMaxRateLimits } from './minimax/minimax-fetcher' import { hasMiniMaxSessionCookie } from '../minimax/minimax-cookie-store' import { deferred, @@ -33,7 +33,7 @@ vi.mock('./opencode-go-usage-fetcher', () => ({ fetchOpenCodeGoRateLimits: vi.fn() })) -vi.mock('./minimax-fetcher', () => ({ +vi.mock('./minimax/minimax-fetcher', () => ({ fetchMiniMaxRateLimits: vi.fn() })) diff --git a/src/main/rate-limits/service-refresh-orchestration.test.ts b/src/main/rate-limits/service-refresh-orchestration.test.ts index 7ac7f22f164..44f4b05b3ba 100644 --- a/src/main/rate-limits/service-refresh-orchestration.test.ts +++ b/src/main/rate-limits/service-refresh-orchestration.test.ts @@ -5,7 +5,7 @@ import { fetchClaudeRateLimits } from './claude-fetcher' import { fetchCodexRateLimits } from './codex-fetcher' import { fetchGeminiRateLimits } from './gemini-usage-fetcher' import { fetchKimiRateLimits } from './kimi-fetcher' -import { fetchMiniMaxRateLimits } from './minimax-fetcher' +import { fetchMiniMaxRateLimits } from './minimax/minimax-fetcher' import { fetchGrokRateLimits } from './grok-fetcher' import { readGrokAuthSession } from './grok-auth' import { fetchOpenCodeGoRateLimits } from './opencode-go-usage-fetcher' @@ -40,7 +40,7 @@ vi.mock('./opencode-go-usage-fetcher', () => ({ fetchOpenCodeGoRateLimits: vi.fn() })) -vi.mock('./minimax-fetcher', () => ({ +vi.mock('./minimax/minimax-fetcher', () => ({ fetchMiniMaxRateLimits: vi.fn() })) diff --git a/src/main/rate-limits/service-window-activation.test.ts b/src/main/rate-limits/service-window-activation.test.ts index a4a1dc14f76..ec813a53d94 100644 --- a/src/main/rate-limits/service-window-activation.test.ts +++ b/src/main/rate-limits/service-window-activation.test.ts @@ -5,7 +5,7 @@ import { fetchClaudeRateLimits } from './claude-fetcher' import { fetchCodexRateLimits } from './codex-fetcher' import { fetchGeminiRateLimits } from './gemini-usage-fetcher' import { fetchKimiRateLimits } from './kimi-fetcher' -import { fetchMiniMaxRateLimits } from './minimax-fetcher' +import { fetchMiniMaxRateLimits } from './minimax/minimax-fetcher' import { fetchGrokRateLimits } from './grok-fetcher' import { fetchOpenCodeGoRateLimits } from './opencode-go-usage-fetcher' import { @@ -41,7 +41,7 @@ vi.mock('./opencode-go-usage-fetcher', () => ({ fetchOpenCodeGoRateLimits: vi.fn() })) -vi.mock('./minimax-fetcher', () => ({ +vi.mock('./minimax/minimax-fetcher', () => ({ fetchMiniMaxRateLimits: vi.fn() })) diff --git a/src/main/rate-limits/service/service-full-cycle-preparation.ts b/src/main/rate-limits/service/service-full-cycle-preparation.ts index c5bf533bc86..7551652bca1 100644 --- a/src/main/rate-limits/service/service-full-cycle-preparation.ts +++ b/src/main/rate-limits/service/service-full-cycle-preparation.ts @@ -3,7 +3,7 @@ import { fetchCodexRateLimits } from '../codex-fetcher' import { fetchGeminiRateLimits } from '../gemini-usage-fetcher' import { fetchGrokRateLimits } from '../grok-fetcher' import { readGrokAuthSession } from '../grok-auth' -import { fetchMiniMaxRateLimits } from '../minimax-fetcher' +import { fetchMiniMaxRateLimits } from '../minimax/minimax-fetcher' import { fetchOpenCodeGoRateLimits } from '../opencode-go-usage-fetcher' import { RateLimitServiceFetchPolicy } from './service-fetch-policy' import type { diff --git a/src/renderer/src/components/stats/GrokUsagePane.test.tsx b/src/renderer/src/components/stats/GrokUsagePane.test.tsx index 42e8e6577ab..f8b83587ad6 100644 --- a/src/renderer/src/components/stats/GrokUsagePane.test.tsx +++ b/src/renderer/src/components/stats/GrokUsagePane.test.tsx @@ -7,6 +7,7 @@ import { cleanup, render, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { AppState } from '../../store' +import { createEmptyRateLimitState } from '../../../../shared/rate-limit-state-factory' const storeMocks = vi.hoisted(() => ({ refreshGrokRateLimits: vi.fn(), @@ -16,14 +17,7 @@ const storeMocks = vi.hoisted(() => ({ })) const mockStoreState = { - rateLimits: { - claude: null, - codex: null, - gemini: null, - opencodeGo: null, - kimi: null, - antigravity: null, - minimax: null, + rateLimits: createEmptyRateLimitState({ grok: { provider: 'grok', session: null, @@ -37,14 +31,8 @@ const mockStoreState = { error: null, status: 'ok' }, - minimaxCookieConfigured: false, - minimaxApiKeyConfigured: false, - grokAuthConfigured: true, - claudeTarget: { runtime: 'host', wslDistro: null }, - codexTarget: { runtime: 'host', wslDistro: null }, - inactiveClaudeAccounts: [], - inactiveCodexAccounts: [] - }, + grokAuthConfigured: true + }), refreshGrokRateLimits: storeMocks.refreshGrokRateLimits, openSettingsPage: storeMocks.openSettingsPage, openSettingsTarget: storeMocks.openSettingsTarget, diff --git a/src/renderer/src/components/status-bar/status-bar-provider-visibility.test.ts b/src/renderer/src/components/status-bar/status-bar-provider-visibility.test.ts index 41a933a850b..ab2a0eca23f 100644 --- a/src/renderer/src/components/status-bar/status-bar-provider-visibility.test.ts +++ b/src/renderer/src/components/status-bar/status-bar-provider-visibility.test.ts @@ -3,6 +3,7 @@ import type { ProviderRateLimits, ProviderRateLimitStatus } from '../../../../shared/rate-limit-types' +import { createEmptyRateLimitState } from '../../../../shared/rate-limit-state-factory' import { getVisibleUsageProvider, hasUsageProviderSettings, @@ -383,21 +384,7 @@ describe('getVisibleUsageProvider', () => { describe('isUsageEmptyState', () => { it('waits for provider snapshots before showing the setup CTA', () => { - expect( - isUsageEmptyState( - { - claude: null, - codex: null, - gemini: null, - opencodeGo: null, - kimi: null, - antigravity: null, - minimax: null, - grok: null - }, - usageSettings() - ) - ).toBe(false) + expect(isUsageEmptyState(createEmptyRateLimitState(), usageSettings())).toBe(false) }) it('treats provider keys omitted by an older main process as pending', () => { @@ -466,21 +453,7 @@ describe('isUsageEmptyState', () => { }) it('waits for settings before showing the setup CTA', () => { - expect( - isUsageEmptyState( - { - claude: null, - codex: null, - gemini: null, - opencodeGo: null, - kimi: null, - antigravity: null, - minimax: null, - grok: null - }, - null - ) - ).toBe(false) + expect(isUsageEmptyState(createEmptyRateLimitState(), null)).toBe(false) }) it('shows the setup CTA for a loaded profile with no configured usage provider', () => { diff --git a/src/renderer/src/hooks/useIpcEvents-rate-limit-hydration.test.ts b/src/renderer/src/hooks/useIpcEvents-rate-limit-hydration.test.ts index f1790583292..a95d1095d5e 100644 --- a/src/renderer/src/hooks/useIpcEvents-rate-limit-hydration.test.ts +++ b/src/renderer/src/hooks/useIpcEvents-rate-limit-hydration.test.ts @@ -1,5 +1,6 @@ import type * as ReactModule from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { createEmptyRateLimitState } from '../../../shared/rate-limit-state-factory' describe('useIpcEvents rate-limit hydration', () => { beforeEach(() => { @@ -9,17 +10,7 @@ describe('useIpcEvents rate-limit hydration', () => { it('does not miss startup usage updates that land between get and subscription', async () => { const setRateLimitsFromPush = vi.fn() - const staleState = { - claude: null, - codex: null, - gemini: null, - opencodeGo: null, - kimi: null, - claudeTarget: { runtime: 'host', wslDistro: null }, - codexTarget: { runtime: 'host', wslDistro: null }, - inactiveClaudeAccounts: [], - inactiveCodexAccounts: [] - } + const staleState = createEmptyRateLimitState() const freshState = { ...staleState, claude: { diff --git a/src/renderer/src/store/slices/rate-limits.ts b/src/renderer/src/store/slices/rate-limits.ts index 7b045c74e3b..9a6d41b701d 100644 --- a/src/renderer/src/store/slices/rate-limits.ts +++ b/src/renderer/src/store/slices/rate-limits.ts @@ -1,5 +1,6 @@ import type { StateCreator } from 'zustand' import type { RateLimitRuntimeTarget, RateLimitState } from '../../../../shared/rate-limit-types' +import { createEmptyRateLimitState } from '../../../../shared/rate-limit-state-factory' import type { AppState } from '../types' export type RateLimitSlice = { @@ -16,23 +17,7 @@ export type RateLimitSlice = { } export const createRateLimitSlice: StateCreator = (set, get) => ({ - rateLimits: { - claude: null, - codex: null, - gemini: null, - opencodeGo: null, - kimi: null, - antigravity: null, - minimax: null, - grok: null, - minimaxCookieConfigured: false, - minimaxApiKeyConfigured: false, - grokAuthConfigured: false, - claudeTarget: { runtime: 'host', wslDistro: null }, - codexTarget: { runtime: 'host', wslDistro: null }, - inactiveClaudeAccounts: [], - inactiveCodexAccounts: [] - }, + rateLimits: createEmptyRateLimitState(), fetchRateLimits: async () => { try { diff --git a/src/renderer/src/web/preload-api/web-rate-limits-api.ts b/src/renderer/src/web/preload-api/web-rate-limits-api.ts index 023b7e3fd3a..d5fe7bc9080 100644 --- a/src/renderer/src/web/preload-api/web-rate-limits-api.ts +++ b/src/renderer/src/web/preload-api/web-rate-limits-api.ts @@ -1,25 +1,9 @@ import type { PreloadApi } from '../../../../preload/api-types' -import type { RateLimitState } from '../../../../shared/rate-limit-types' +import { createEmptyRateLimitState } from '../../../../shared/rate-limit-state-factory' import { noopUnsubscribe } from './web-storage' export function createRateLimitsApi(): NonNullable['rateLimits']> { - const empty: RateLimitState = { - claude: null, - codex: null, - gemini: null, - opencodeGo: null, - kimi: null, - antigravity: null, - minimax: null, - grok: null, - minimaxCookieConfigured: false, - minimaxApiKeyConfigured: false, - grokAuthConfigured: false, - claudeTarget: { runtime: 'host', wslDistro: null }, - codexTarget: { runtime: 'host', wslDistro: null }, - inactiveClaudeAccounts: [], - inactiveCodexAccounts: [] - } + const empty = createEmptyRateLimitState() return { get: () => Promise.resolve(empty), refresh: () => Promise.resolve(empty), diff --git a/src/shared/global-settings-test-fixture.ts b/src/shared/global-settings-test-fixture.ts new file mode 100644 index 00000000000..dd87c6a17af --- /dev/null +++ b/src/shared/global-settings-test-fixture.ts @@ -0,0 +1,26 @@ +import type { GlobalSettings } from './global-settings-types' +import { getDefaultNotificationSettings, getDefaultVoiceSettings } from './constants' +import { buildDefaultSettings } from './default-global-settings' + +// Why: tests need a complete GlobalSettings without hand-copying every field, so +// new settings only have to be added to buildDefaultSettings, not each fixture. +export function createGlobalSettingsFixture( + overrides: Partial = {} +): GlobalSettings { + return { + ...buildDefaultSettings({ + // Callers supply the real directory; no platform-specific default belongs here. + workspaceDir: overrides.workspaceDir ?? '', + appFontFamily: 'Geist', + editorAutoSaveDelayMs: 1000, + primarySelectionMiddleClickPaste: false, + primarySelectionDefaultedForLinux: false, + terminalFontFamily: 'JetBrains Mono', + terminalInactivePaneOpacity: 0.5, + terminalRightClickToPaste: false, + notifications: getDefaultNotificationSettings(), + voice: getDefaultVoiceSettings() + }), + ...overrides + } +} diff --git a/src/shared/rate-limit-state-factory.ts b/src/shared/rate-limit-state-factory.ts new file mode 100644 index 00000000000..bf8d97e541e --- /dev/null +++ b/src/shared/rate-limit-state-factory.ts @@ -0,0 +1,23 @@ +import type { RateLimitState } from './rate-limit-types' + +// Why: single source of the empty shape so a new provider field never forces edits at unrelated call sites. +export function createEmptyRateLimitState(overrides: Partial = {}): RateLimitState { + return { + claude: null, + codex: null, + gemini: null, + opencodeGo: null, + kimi: null, + antigravity: null, + minimax: null, + grok: null, + minimaxCookieConfigured: false, + minimaxApiKeyConfigured: false, + grokAuthConfigured: false, + claudeTarget: { runtime: 'host', wslDistro: null }, + codexTarget: { runtime: 'host', wslDistro: null }, + inactiveClaudeAccounts: [], + inactiveCodexAccounts: [], + ...overrides + } +} From 374c676f6df0de88a95a79bf6fe22269f2494c8e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 7 Sep 2026 00:25:16 -0700 Subject: [PATCH 8/9] fix: repaint hidden output overflow after answered restore deadline (#18904) --- ...ection-hidden-restore-fit-overflow.test.ts | 199 ++++++++++++++++++ .../hidden-output-restore-drain.ts | 15 +- ...icial-opencode-hidden-pressure-scenario.ts | 18 +- 3 files changed, 225 insertions(+), 7 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/pty-connection-hidden-restore-fit-overflow.test.ts diff --git a/src/renderer/src/components/terminal-pane/pty-connection-hidden-restore-fit-overflow.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-hidden-restore-fit-overflow.test.ts new file mode 100644 index 00000000000..5610f557459 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection-hidden-restore-fit-overflow.test.ts @@ -0,0 +1,199 @@ +import type * as React from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { flushAsyncTicks, createDeferred, renderHeadlessBuffer } from './pty-connection-test-async' +import { + createMockTransport, + createPane, + createManager, + type ConnectCallbacks, + type MockTransport +} from './pty-connection-test-pane-fixtures' +import { buildPaneConnectionDeps } from './pty-connection-test-deps' +import { createInitialStoreState } from './pty-connection-test-store-fixtures' +import type { StoreState } from './pty-connection-test-store-state' +import { + installTerminalTestGlobals, + restoreTerminalTestGlobals +} from './pty-connection-test-environment' + +const { + resetAndRefreshAllTerminalWebglAtlases, + scheduleTerminalWebglAtlasRecovery, + scheduleRuntimeGraphSync, + shouldSeedCacheTimerOnInitialTitle, + toastInfo, + notifyCodexPaneBoundForStaleSweep +} = vi.hoisted(() => ({ + resetAndRefreshAllTerminalWebglAtlases: vi.fn(), + scheduleTerminalWebglAtlasRecovery: vi.fn(), + scheduleRuntimeGraphSync: vi.fn(), + shouldSeedCacheTimerOnInitialTitle: vi.fn(() => false), + toastInfo: vi.fn(), + notifyCodexPaneBoundForStaleSweep: vi.fn() +})) + +let mockStoreState: StoreState +let transportFactoryQueue: MockTransport[] = [] +let createdTransportOptions: Record[] = [] +let storeSubscribers: ((state: StoreState) => void)[] = [] + +vi.mock('@/runtime/sync-runtime-graph', () => ({ + scheduleRuntimeGraphSync +})) + +vi.mock('@/lib/pane-manager/pane-manager-registry', async (importOriginal) => ({ + ...(await importOriginal>()), + resetAndRefreshAllTerminalWebglAtlases +})) + +vi.mock('./terminal-webgl-atlas-recovery', () => ({ + scheduleTerminalWebglAtlasRecovery +})) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => mockStoreState, + subscribe: (listener: (state: StoreState) => void) => { + storeSubscribers.push(listener) + return () => { + storeSubscribers = storeSubscribers.filter((candidate) => candidate !== listener) + } + } + } +})) + +vi.mock('@/lib/agent-status', async (importOriginal) => { + const { buildAgentStatusModuleMock } = await import('./pty-connection-test-environment') + return buildAgentStatusModuleMock(await importOriginal>()) +}) + +vi.mock('./cache-timer-seeding', () => ({ + shouldSeedCacheTimerOnInitialTitle +})) + +vi.mock('sonner', () => ({ + toast: { + info: toastInfo + } +})) + +vi.mock('@/lib/codex-stale-pane-sweep', () => ({ + notifyCodexPaneBoundForStaleSweep +})) + +// The connection fixture invokes hooks without mounting React. +vi.mock('react', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useCallback: unknown>(fn: T): T => fn + } +}) + +vi.mock('./pty-transport', () => ({ + createIpcPtyTransport: vi.fn((options: Record) => { + createdTransportOptions.push(options) + const nextTransport = transportFactoryQueue.shift() + if (!nextTransport) { + throw new Error('No mock transport queued') + } + return nextTransport + }) +})) + +vi.mock('./remote-runtime-pty-transport', () => ({ + createRemoteRuntimePtyTransport: vi.fn( + (_environmentId: string, options: Record) => { + createdTransportOptions.push(options) + const nextTransport = transportFactoryQueue.shift() + if (!nextTransport) { + throw new Error('No mock transport queued') + } + return nextTransport + } + ) +})) + +// Why: stub only getEagerPtyBufferHandle so tests can simulate a live eager buffer (adopt path) without standing up the real IPC dispatcher. +vi.mock('./pty-dispatcher', async (importOriginal) => { + const actual = await importOriginal>() + return { + ...actual, + getEagerPtyBufferHandle: vi.fn(() => undefined) + } +}) + +const { safeFitAndThen } = vi.hoisted(() => ({ safeFitAndThen: vi.fn() })) +vi.mock('@/lib/pane-manager/pane-tree-ops', async (importOriginal) => ({ + ...(await importOriginal>()), + safeFitAndThen +})) + +function createDeps(overrides: Record = {}) { + return buildPaneConnectionDeps(() => mockStoreState, overrides) +} + +describe('connectPanePty', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + transportFactoryQueue = [] + createdTransportOptions = [] + storeSubscribers = [] + mockStoreState = createInitialStoreState(() => mockStoreState) + installTerminalTestGlobals() + }) + + afterEach(async () => { + await restoreTerminalTestGlobals() + }) + + it('repaints overflowed live output when the deadline interrupts an answered snapshot', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-id') + let onData: ConnectCallbacks['onData'] + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + onData = callbacks.onData + return 'pty-id' + }) + transportFactoryQueue.push(transport) + const fit = createDeferred() + safeFitAndThen.mockReturnValue({ completion: Promise.resolve(true), cancel: vi.fn() }) + + const getSnapshot = vi.mocked(window.api.pty.getMainBufferSnapshot) + const hidden = 'hidden-before-flood\r\n' + const overflow = 'v'.repeat(512 * 1024 + 1) + const done = 'HIDDEN_FLOOD_DONE\r\n' + getSnapshot + .mockResolvedValueOnce({ data: hidden, cols: 120, rows: 40, seq: hidden.length }) + .mockResolvedValue({ data: done, cols: 120, rows: 40, seq: hidden.length + overflow.length }) + const pane = createPane(1) + const deps = createDeps({ isVisibleRef: { current: false }, startup: { command: 'codex' } }) + const disposable = connectPanePty(pane as never, createManager(1) as never, deps as never) + await flushAsyncTicks(6) + safeFitAndThen.mockClear() + safeFitAndThen.mockReturnValueOnce({ + completion: fit.promise, + cancel: vi.fn(() => fit.resolve(false)) + }) + vi.useFakeTimers() + onData?.(hidden, { seq: hidden.length, rawLength: hidden.length }) + ;(deps.isVisibleRef as { current: boolean }).current = true + onData?.('v', { seq: hidden.length + 1, rawLength: 1 }) + await flushAsyncTicks(30) + expect(safeFitAndThen).toHaveBeenCalledTimes(1) + onData?.(overflow, { seq: hidden.length + overflow.length, rawLength: overflow.length }) + await vi.advanceTimersByTimeAsync(750) + fit.resolve(true) + await flushAsyncTicks(30) + await vi.advanceTimersByTimeAsync(2_000) + await flushAsyncTicks(30) + const output = pane.terminal.write.mock.calls.map(([data]) => data).join('') + expect(output).toContain(done) + expect(output).not.toContain('main recovery was unavailable') + expect(getSnapshot).toHaveBeenCalledTimes(2) + disposable.dispose() + vi.useRealTimers() + expect(await renderHeadlessBuffer([output])).toEqual(await renderHeadlessBuffer([done])) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-drain.ts b/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-drain.ts index a003d272444..1705fbe2cdf 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-drain.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-drain.ts @@ -129,7 +129,20 @@ export function bindHiddenOutputRestoreDrain(session: ConnectPanePtySession): vo ) { return } - session.abandonHiddenOutputRestoreAndDrainPendingForeground(ptyId) + // A fetched snapshot plus live overflow is backpressure, not unavailable recovery. The + // replay paints synchronously before it awaits its fit, so this deadline never lands + // mid-paint: adopt that painted image as the baseline — the overflow abandon below + // returns before arming one, and without it main's ACK backlog repaints as duplicates. + const replayed = session.hiddenOutputRestorePendingOverflow + ? session.hiddenOutputRestoreReplayingSnapshot + : null + if (replayed) { + session.setRestoredSnapshotBaseline(ptyId, replayed, replayed.paintsContent === true) + session.noteHiddenOutputRestoreFloodBackpressure() + } + session.abandonHiddenOutputRestoreAndDrainPendingForeground(ptyId, { + quiet: replayed !== null + }) }, HIDDEN_OUTPUT_RESTORE_FOREGROUND_TIMEOUT_MS) } diff --git a/tests/e2e/artificial-opencode-hidden-pressure-scenario.ts b/tests/e2e/artificial-opencode-hidden-pressure-scenario.ts index 5eb60025870..905e113dc52 100644 --- a/tests/e2e/artificial-opencode-hidden-pressure-scenario.ts +++ b/tests/e2e/artificial-opencode-hidden-pressure-scenario.ts @@ -16,11 +16,12 @@ import { waitForSessionReady } from './helpers/store' import { - getTerminalContent, + resolveActiveTabId, sendToTerminal, waitForActivePanePtyId, waitForActiveTerminalManager } from './helpers/terminal' +import { readActiveScreen } from './helpers/alt-screen-frame' type HiddenPressurePane = { ptyId: string @@ -83,9 +84,9 @@ type HiddenPressureAckGate = { // Why: restore still has to finish promptly, but parallel Electron workers on // Linux CI can overshoot the 1s product target without a responsiveness regression. -// Main relaxed this to 4s for drain-plus-poll overhead on loaded OSS runners; this -// branch keeps a far stricter budget with only a small margin for the whole-buffer -// serialize-poll overhead (seen at ~1.5s), so a genuinely slow restore is still caught. +// 4s covers drain-plus-poll overhead on loaded OSS runners. The post-flood repaint path +// spends ~2.75s of that (750ms deadline + 2s suppression), so the poll below reads the +// viewport on a fixed interval rather than serializing scrollback on a backoff. const MAX_HIDDEN_RESTORE_LATENCY_MS = 4_000 // Why: Phase-4 hidden-delivery gate contract — hidden PTY bytes are dropped in // main after model ingestion, so renderer-delivery pressure must stay FAR @@ -267,10 +268,15 @@ async function measureHiddenOutputRestoreLatency( ): Promise { const restoreStart = performance.now() await switchToWorktree(orcaPage, worktreeId) + // Why resolve rather than read activeTabId: after a worktree switch the active tab can + // still be the previous worktree's, or a non-terminal one; this picks the worktree's own. + const tabId = (await resolveActiveTabId(orcaPage)) ?? '' await expect - .poll(() => getTerminalContent(orcaPage, 20_000), { + .poll(async () => (await readActiveScreen(orcaPage, tabId))?.rows.join('\n') ?? '', { timeout: 20_000, - message: 'Hidden PTY output was not restored from main buffer on return' + // One-second backoff can dominate the measured restore latency. + intervals: [50], + message: 'No restored output from main buffer on return (or no active terminal pane)' }) .toContain(`OPENCODE_PRESSURE_DONE_${runId}_`) return performance.now() - restoreStart From 314506003a16297006225147fef8bdcec2186da8 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 7 Sep 2026 00:35:55 -0700 Subject: [PATCH 9/9] fix: retain MSYS shell descendants in their terminal job (#19068) * fix: retain MSYS shell descendants in their terminal job * test: complete MSYS regression CI registration and teardown contract * fix(windows): deny job breakaway for the whole Cygwin/MSYS shell family The per-PTY job probed only msys-2.0.dll, and only for bash.exe/sh.exe. Cygwin ships the same spawn.cc breakaway logic under cygwin1.dll, and an MSYS2 zsh escapes exactly like its bash does, so both kept the orphan bug. Probe the runtime DLL on the shell's own search path instead of matching shell names: that is the property that decides whether the runtime will ask for CREATE_BREAKAWAY_FROM_JOB, and it drops the name special-casing. * chore(patch): restore the conpty.cc index line The earlier hand-edit dropped it while every sibling section kept one. Recomputed against the real blobs: applying this patch to 7b286d3d yields exactly 4b06d185, so git apply -3 has its fallback back. --- .github/workflows/pr.yml | 1 + config/patches/node-pty@1.1.0.patch | 107 +++++++++++------- config/scripts/pr-code-change-scope.mjs | 1 + docs/reference/windows-process-enumeration.md | 14 +++ pnpm-lock.yaml | 6 +- .../windows/windows-msys-job.win32.test.ts | 65 +++++++++++ 6 files changed, 153 insertions(+), 41 deletions(-) create mode 100644 src/main/windows/windows-msys-job.win32.test.ts diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 9279f35b39f..268b6ad66e3 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -856,6 +856,7 @@ jobs: src/main/agent-hooks/windows-hook-payload-delivery.test.ts src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts src/main/windows/windows-pty-job.win32.test.ts + src/main/windows/windows-msys-job.win32.test.ts src/main/windows/windows-host-job.win32.test.ts src/main/windows/windows-process-tree-command-line-patch.test.ts src/main/windows/windows-process-table-native-addon.win32.test.ts diff --git a/config/patches/node-pty@1.1.0.patch b/config/patches/node-pty@1.1.0.patch index 8f5045b932a..961e750da6b 100644 --- a/config/patches/node-pty@1.1.0.patch +++ b/config/patches/node-pty@1.1.0.patch @@ -603,7 +603,7 @@ index 7b4b9e1f990fbf95b51528bb56dc9717f5b87532..2ae787c5bd4f3eba470584dc658a01a5 } #endif diff --git a/src/win/conpty.cc b/src/win/conpty.cc -index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c97209248e 100644 +index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c6678cf86 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -18,6 +18,7 @@ @@ -614,7 +614,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9 #include #include #include -@@ -44,12 +45,39 @@ struct pty_baton { +@@ -44,12 +45,40 @@ struct pty_baton { HANDLE hOut; HPCON hpc; @@ -630,6 +630,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9 + // refused to create or assign one (an outer job without breakaway rights), + // in which case callers fall back to their pre-job behaviour. + HANDLE hJob = nullptr; ++ bool allowJobBreakaway = true; + + // Orca: teardown needs BOTH the shell's death and an explicit kill() before + // the baton can be freed, so each side records that it has run. Whichever @@ -655,7 +656,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9 static volatile LONG ptyCounter; static pty_baton* get_pty_baton(int id) { -@@ -102,8 +130,31 @@ void SetupExitCallback(Napi::Env env, Napi::Function cb, pty_baton* baton) { +@@ -102,8 +131,31 @@ void SetupExitCallback(Napi::Env env, Napi::Function cb, pty_baton* baton) { // Get process exit code. GetExitCodeProcess(baton->hShell, (LPDWORD)(&exit_event->exit_code)); // Clean up handles @@ -689,7 +690,36 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9 auto status = tsfn.BlockingCall(exit_event, callback); // In main thread switch (status) { -@@ -409,6 +460,15 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { +@@ -242,6 +294,20 @@ + return HRESULT_FROM_WIN32(GetLastError()); + } + ++// Cygwin and MSYS request breakaway for every child whenever the job allows it, ++// so their shells need one that does not. The runtime DLL on the exe's search ++// path is the signal; Git for Windows ships bash.exe in bin\ beside usr\bin\. ++static bool usesCygwinRuntime(const std::wstring& shellpath) { ++ const size_t separator = shellpath.find_last_of(L"\\/"); ++ if (separator == std::wstring::npos) return false; ++ const std::wstring directory = shellpath.substr(0, separator + 1); ++ for (const wchar_t* dll : {L"msys-2.0.dll", L"cygwin1.dll"}) { ++ if (path_util::file_exists(directory + dll) || ++ path_util::file_exists(directory + L"..\\usr\\bin\\" + dll)) return true; ++ } ++ return false; ++} ++ + static Napi::Value PtyStartProcess(const Napi::CallbackInfo& info) { + Napi::Env env(info.Env()); + Napi::HandleScope scope(env); +@@ -303,6 +369,7 @@ + marshal.Set("pty", Napi::Number::New(env, ptyId)); + ptyHandles.emplace_back( + std::make_unique(ptyId, hIn, hOut, hpc)); ++ ptyHandles.back()->allowJobBreakaway = !usesCygwinRuntime(shellpath); + } else { + throw Napi::Error::New(env, "Cannot launch conpty"); + } +@@ -409,6 +476,15 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { throw errorWithCode(info, "UpdateProcThreadAttribute failed"); } @@ -705,7 +735,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9 PROCESS_INFORMATION piClient{}; fSuccess = !!CreateProcessW( nullptr, -@@ -416,7 +476,10 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { +@@ -416,7 +492,10 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { nullptr, // lpProcessAttributes nullptr, // lpThreadAttributes false, // bInheritHandles VERY IMPORTANT that this is false @@ -717,7 +747,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9 envArg, // lpEnvironment mutableCwd.get(), // lpCurrentDirectory &siEx.StartupInfo, // lpStartupInfo -@@ -426,8 +489,47 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { +@@ -426,8 +505,48 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { throw errorWithCode(info, "Cannot create process"); } @@ -735,13 +765,14 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9 + // EXPLICIT teardown exact, not to redefine what a clean exit means. + HANDLE hJob = CreateJobObjectW(nullptr, nullptr); + if (hJob != nullptr) { -+ // Why BREAKAWAY_OK and not a bare job: with no limits set, a child asking -+ // for CREATE_BREAKAWAY_FROM_JOB is refused with ERROR_ACCESS_DENIED. -+ // Installers, msiexec and some updater and service-control paths spawn that -+ // way deliberately, so a bare job breaks them ONLY inside an Orca terminal. -+ // With this flag a child has to ask, so ordinary descendants stay owned. ++ // Native shells retain explicit breakaway for installers and updaters. ++ // Cygwin/MSYS shells take it automatically for ordinary children whenever ++ // this flag is present, so they get strict per-PTY membership instead. ++ // Explicit breakaway requests inside such a pane are consequently denied; ++ // ordinary backgrounding and clean shell exit remain supported. + JOBOBJECT_EXTENDED_LIMIT_INFORMATION jobLimits{}; -+ jobLimits.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_BREAKAWAY_OK; ++ jobLimits.BasicLimitInformation.LimitFlags = ++ handle->allowJobBreakaway ? JOB_OBJECT_LIMIT_BREAKAWAY_OK : 0; + if (!SetInformationJobObject(hJob, JobObjectExtendedLimitInformation, &jobLimits, sizeof(jobLimits)) || + !AssignProcessToJobObject(hJob, piClient.hProcess)) { + // Why tolerate failure: an outer job without JOB_OBJECT_LIMIT_BREAKAWAY_OK @@ -767,7 +798,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9 if (useConptyDll && fLoadedDll) { PFNRELEASEPSEUDOCONSOLE const pfnReleasePseudoConsole = (PFNRELEASEPSEUDOCONSOLE)GetProcAddress( -@@ -440,6 +542,8 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { +@@ -440,6 +559,8 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { // Update handle handle->hShell = piClient.hProcess; @@ -776,11 +807,16 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9 // Close the thread handle to avoid resource leak CloseHandle(piClient.hThread); -@@ -544,29 +648,215 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) { +@@ -544,27 +665,213 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) { int id = info[0].As().Int32Value(); const bool useConptyDll = info[1].As().Value(); - const pty_baton* handle = get_pty_baton(id); +- +- if (handle != nullptr) { +- HANDLE hLibrary = LoadConptyDll(info, useConptyDll); +- bool fLoadedDll = hLibrary != nullptr; +- if (fLoadedDll) + // Orca: resolve the DLL BEFORE touching any baton state, for the same reason + // PtyConnect does it before creating anything. LoadConptyDll throws when + // conpty.dll is missing, and a throw after consoleClosed was set would strand @@ -794,18 +830,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9 + (HMODULE)hLibrary, + useConptyDll ? "ConptyClosePseudoConsole" : "ClosePseudoConsole"); + } - -- if (handle != nullptr) { -- HANDLE hLibrary = LoadConptyDll(info, useConptyDll); -- bool fLoadedDll = hLibrary != nullptr; -- if (fLoadedDll) -- { -- PFNCLOSEPSEUDOCONSOLE const pfnClosePseudoConsole = (PFNCLOSEPSEUDOCONSOLE)GetProcAddress( -- (HMODULE)hLibrary, -- useConptyDll ? "ConptyClosePseudoConsole" : "ClosePseudoConsole"); -- if (pfnClosePseudoConsole) -- { -- pfnClosePseudoConsole(handle->hpc); ++ + // Orca: the baton now outlives the shell, so this runs on a self-exited pty + // too -- that is the whole point. Take what we need under the lock: the + // watcher thread nulls hShell the moment the shell dies, and TerminateProcess @@ -841,18 +866,26 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9 + const bool removed = remove_pty_baton(id); + assert(removed); + (void)removed; - } ++ } + // Else the shell is still running and the watcher frees the baton. - } -- if (useConptyDll) { -- TerminateProcess(handle->hShell, 1); ++ } + } + + // Why outside the lock: ClosePseudoConsole blocks until the conout side has + // drained, and the watcher must be able to take the lock while it does. + if (owed) { + if (pfnClosePseudoConsole) -+ { + { +- PFNCLOSEPSEUDOCONSOLE const pfnClosePseudoConsole = (PFNCLOSEPSEUDOCONSOLE)GetProcAddress( +- (HMODULE)hLibrary, +- useConptyDll ? "ConptyClosePseudoConsole" : "ClosePseudoConsole"); +- if (pfnClosePseudoConsole) +- { +- pfnClosePseudoConsole(handle->hpc); +- } +- } +- if (useConptyDll) { +- TerminateProcess(handle->hShell, 1); + pfnClosePseudoConsole(hpc); + } + if (hShellDup != nullptr) { @@ -862,8 +895,8 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9 } return env.Undefined(); - } - ++} ++ +/** + * Orca: confirm a baton really is the pty the caller means. + * @@ -1001,12 +1034,10 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9 + } + hHostJob = job; + return Napi::Boolean::New(env, true); -+} -+ + } + /** - * Init - */ -@@ -577,6 +867,9 @@ Napi::Object init(Napi::Env env, Napi::Object exports) { +@@ -577,6 +884,9 @@ Napi::Object init(Napi::Env env, Napi::Object exports) { exports.Set("resize", Napi::Function::New(env, PtyResize)); exports.Set("clear", Napi::Function::New(env, PtyClear)); exports.Set("kill", Napi::Function::New(env, PtyKill)); diff --git a/config/scripts/pr-code-change-scope.mjs b/config/scripts/pr-code-change-scope.mjs index befcb06fe1f..96917d23ef1 100644 --- a/config/scripts/pr-code-change-scope.mjs +++ b/config/scripts/pr-code-change-scope.mjs @@ -224,6 +224,7 @@ const WINDOWS_PACKAGE_TESTS = [ 'src/main/agent-hooks/windows-hook-payload-delivery.test.ts', 'src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts', 'src/main/windows/windows-pty-job.win32.test.ts', + 'src/main/windows/windows-msys-job.win32.test.ts', 'src/main/windows/windows-host-job.win32.test.ts', 'src/main/windows/windows-process-tree-command-line-patch.test.ts', 'src/main/windows/windows-process-table-native-addon.win32.test.ts', diff --git a/docs/reference/windows-process-enumeration.md b/docs/reference/windows-process-enumeration.md index 0f7f17bd433..80cc7663f4c 100644 --- a/docs/reference/windows-process-enumeration.md +++ b/docs/reference/windows-process-enumeration.md @@ -575,6 +575,20 @@ running, so typing `exit` in a pane reaped a `start /b` server that used to survive. The job exists to make an _explicit_ teardown exact, not to redefine what a clean exit means. +Git Bash needs one additional restriction. The Cygwin runtime — and the MSYS2 +fork of it that Git for Windows ships — reads `JOB_OBJECT_LIMIT_BREAKAWAY_OK` +off its own job and then adds `CREATE_BREAKAWAY_FROM_JOB` to **every** child it +spawns when that flag is set (`spawn.cc`, there since 2011), so offering +breakaway hands the whole tree its escape. The per-PTY job therefore omits +`BREAKAWAY_OK` whenever `msys-2.0.dll` or `cygwin1.dll` sits on the shell's DLL +search path — beside the executable, or under `usr/bin` for Git's `bin` +launcher. Native shells keep explicit breakaway. Denying it costs Cygwin +nothing, because it *pre-checks* the limit rather than retrying, so no spawn +fails; but a *native* program that passes `CREATE_BREAKAWAY_FROM_JOB` itself +inside such a pane now gets `ERROR_ACCESS_DENIED`. `nohup` and `disown` are +unaffected — they are Cygwin signal/session concepts, unrelated to job +membership. The daemon's host job is unchanged. + Reaping a dead daemon's shells (#9195, #10415) is therefore a **second, nested job**, not this one. The terminal daemon assigns itself to a kill-on-close job at startup (`assignHostProcessToKillOnCloseJob`); children inherit membership, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 103ed90f4fe..e4a40c0fe47 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -116,7 +116,7 @@ patchedDependencies: '@xterm/addon-webgl@0.20.0-beta.299': 94687e89a0115e6e6aa102837f986debdc029c091527ee5eb4a4e17ceaf9473e '@xterm/xterm@6.1.0-beta.303': 98756bcedc402bcdb7c6ab7b015d2e59cd18e97b03a2c06a27e95bb3ba429d9d lint-staged@16.4.0: 7333b3837f80a7fbd045964db6d76ba4fc118e49134bdbabb00585b6b7b60673 - node-pty@1.1.0: 7cc9d45f3d2c38f142490d0805e75db55f0eef5174ad41c4b52abc5fbe079ad1 + node-pty@1.1.0: bac3a53fb15efc9b3b944fbe3c4718b5174a0b3bd6ead84e21975edad4bc6615 importers: @@ -160,7 +160,7 @@ importers: version: 3.3.1 node-pty: specifier: ^1.1.0 - version: 1.1.0(patch_hash=7cc9d45f3d2c38f142490d0805e75db55f0eef5174ad41c4b52abc5fbe079ad1) + version: 1.1.0(patch_hash=bac3a53fb15efc9b3b944fbe3c4718b5174a0b3bd6ead84e21975edad4bc6615) posthog-node: specifier: ^5.33.3 version: 5.33.3 @@ -12285,7 +12285,7 @@ snapshots: node-int64@0.4.0: {} - node-pty@1.1.0(patch_hash=7cc9d45f3d2c38f142490d0805e75db55f0eef5174ad41c4b52abc5fbe079ad1): + node-pty@1.1.0(patch_hash=bac3a53fb15efc9b3b944fbe3c4718b5174a0b3bd6ead84e21975edad4bc6615): dependencies: node-addon-api: 7.1.1 diff --git a/src/main/windows/windows-msys-job.win32.test.ts b/src/main/windows/windows-msys-job.win32.test.ts new file mode 100644 index 00000000000..e7e0bee950a --- /dev/null +++ b/src/main/windows/windows-msys-job.win32.test.ts @@ -0,0 +1,65 @@ +import { mkdtempSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it, vi } from 'vitest' +import { removeTreeSync } from '../../shared/windows-transient-lock-removal' +import { resolveGitBashPath } from '../git-bash' +import { quotePosixShell } from '../../shared/wsl-login-shell-command' +import { listPtyJobProcessIds, terminatePtyJob } from './windows-pty-job' + +const describeOnWindows = process.platform === 'win32' ? describe : describe.skip + +function isAlive(pid: number): boolean { + try { + process.kill(pid, 0) + return true + } catch (error) { + return (error as NodeJS.ErrnoException).code === 'EPERM' + } +} + +describeOnWindows('MSYS terminal job ownership', () => { + it('retains and terminates a child across Git Bash shell replacement', async () => { + const shell = resolveGitBashPath() + expect(shell, 'Git for Windows must be installed on the native test runner').not.toBeNull() + const directory = mkdtempSync(join(tmpdir(), 'orca-msys-job-')) + const script = join(directory, 'owned-child.js') + writeFileSync( + script, + "console.log('MSYS_OWNED_CHILD=' + process.pid); setInterval(() => {}, 1000)\n" + ) + const pty = await import('node-pty') + const proc = pty.spawn(shell!, ['-c', 'exec "$BASH" --noprofile --norc -i'], { + cwd: tmpdir(), + cols: 120, + rows: 30, + useConptyDll: true + }) + let output = '' + let childPid: number | undefined + proc.onData((chunk) => { + output += chunk + const match = /MSYS_OWNED_CHILD=(\d+)/.exec(output) + if (match) { + childPid = Number(match[1]) + } + }) + try { + proc.write( + `${quotePosixShell(process.execPath.replace(/\\/g, '/'))} ${quotePosixShell(script.replace(/\\/g, '/'))}\r` + ) + await vi.waitFor(() => expect(childPid).toBeDefined(), { timeout: 15_000 }) + expect(isAlive(childPid!)).toBe(true) + expect(listPtyJobProcessIds(proc)).toContain(childPid) + expect(terminatePtyJob(proc)).toBe('terminated') + await vi.waitFor(() => expect(isAlive(childPid!)).toBe(false), { timeout: 5_000 }) + } finally { + // The failing baseline can leave this exact fixture child outside the job. + if (childPid && isAlive(childPid)) { + process.kill(childPid) + } + proc.kill() + removeTreeSync(directory) + } + }, 30_000) +})