From d8e07bf4b6ad6e7d5f0eec3a692d69ea0d775e60 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sun, 20 Sep 2026 23:41:48 -0400 Subject: [PATCH] refactor(terminal): one pane-identity predicate beside the identity type Main held the same three-field comparison twice, once inline in the reveal reply handler and once in a file of its own. --- .../runtime/legacy-worker-reveal-identity.ts | 20 ------------------ ...act-persisted-terminal-surface-identity.ts | 4 ++-- src/main/window/runtime-window-lifecycle.ts | 15 +++++-------- .../runtime-window-reveal-identity.test.ts | 21 +++++++++---------- src/shared/terminal-reveal-identity.ts | 18 ++++++++++++++++ 5 files changed, 35 insertions(+), 43 deletions(-) delete mode 100644 src/main/runtime/legacy-worker-reveal-identity.ts diff --git a/src/main/runtime/legacy-worker-reveal-identity.ts b/src/main/runtime/legacy-worker-reveal-identity.ts deleted file mode 100644 index 0f4e99c6de3..00000000000 --- a/src/main/runtime/legacy-worker-reveal-identity.ts +++ /dev/null @@ -1,20 +0,0 @@ -import type { TerminalRevealIdentity } from '../../shared/terminal-reveal-identity' - -/** - * Whether the renderer materialized the exact pane a legacy worker recovery asked for. - * - * Why no worktreeId: ownership is tab-keyed, so the renderer decides which workspace key the row - * is filed under, and re-asserting the caller's key rolled back a reveal that had in fact - * surfaced the right pane under a different one (STA-7961). The pane identity is still asserted. - */ -export function revealedLegacyWorkerIdentityMatches( - identity: TerminalRevealIdentity | undefined, - candidate: { tabId: string; leafId: string; ptyId: string } -): boolean { - return Boolean( - identity && - identity.tabId === candidate.tabId && - identity.leafId === candidate.leafId && - identity.ptyId === candidate.ptyId - ) -} diff --git a/src/main/runtime/orca-runtime-has-exact-persisted-terminal-surface-identity.ts b/src/main/runtime/orca-runtime-has-exact-persisted-terminal-surface-identity.ts index 8d561d8cfcb..cdb94f83c29 100644 --- a/src/main/runtime/orca-runtime-has-exact-persisted-terminal-surface-identity.ts +++ b/src/main/runtime/orca-runtime-has-exact-persisted-terminal-surface-identity.ts @@ -5,7 +5,7 @@ import { runtimeWorktreeIdsEqual } from './runtime-worktree-path-identity' import { makePaneKey } from '../../shared/stable-pane-id' -import { revealedLegacyWorkerIdentityMatches } from './legacy-worker-reveal-identity' +import { revealedPaneMatches } from '../../shared/terminal-reveal-identity' import type { LegacyWorkerTerminalRecoveryPlan } from './orchestration/orchestration-legacy-worker-terminal-recovery' import { retireTerminalSurfacesFromSnapshot } from './mobile-session-terminal-retirement' import type { @@ -170,7 +170,7 @@ export class OrcaRuntimeWithHasExactPersistedTerminalSurfaceIdentity extends Orc incarnationId: candidate.incarnationId } }) - return revealedLegacyWorkerIdentityMatches(reveal?.identity, candidate) + return revealedPaneMatches(reveal?.identity, candidate) } setAutomationService(service: AutomationService): void { diff --git a/src/main/window/runtime-window-lifecycle.ts b/src/main/window/runtime-window-lifecycle.ts index f37787b67b0..f5635625859 100644 --- a/src/main/window/runtime-window-lifecycle.ts +++ b/src/main/window/runtime-window-lifecycle.ts @@ -9,7 +9,10 @@ import type { RuntimeMarkdownSaveTabResult } from '../../shared/mobile-markdown-document' import type { RuntimeMobileSessionTabMove } from '../../shared/runtime-types' -import type { TerminalTabCreateReply } from '../../shared/terminal-reveal-identity' +import { + revealedPaneMatches, + type TerminalTabCreateReply +} from '../../shared/terminal-reveal-identity' import { runWorktreeChangeInvalidators } from '../ipc/worktree-change-invalidators' import type { OrcaRuntimeService } from '../runtime/orca-runtime' import { requestMobileMarkdownFromRenderer } from './mobile-markdown-request-relay' @@ -98,15 +101,7 @@ export function registerRuntimeWindowLifecycle( reject(new Error(reply.error)) return } - // Why no worktreeId here: the renderer's answer is authoritative for the workspace key, - // and rejecting it stranded a reveal whose owner row is filed elsewhere (STA-7961). - if ( - expectedIdentity && - (!reply.identity || - reply.identity.tabId !== expectedIdentity.tabId || - reply.identity.leafId !== expectedIdentity.leafId || - reply.identity.ptyId !== expectedIdentity.ptyId) - ) { + if (expectedIdentity && !revealedPaneMatches(reply.identity, expectedIdentity)) { reject(new Error('terminal_reveal_identity_mismatch')) return } diff --git a/src/main/window/runtime-window-reveal-identity.test.ts b/src/main/window/runtime-window-reveal-identity.test.ts index 4a8da26735a..9829a14d78b 100644 --- a/src/main/window/runtime-window-reveal-identity.test.ts +++ b/src/main/window/runtime-window-reveal-identity.test.ts @@ -2,8 +2,10 @@ // renderer decides which workspace key holds the row; re-asserting the caller's key here rejected // a reveal that had surfaced exactly the right pane under a different one (STA-7961). import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { TerminalTabCreateReply } from '../../shared/terminal-reveal-identity' -import { revealedLegacyWorkerIdentityMatches } from '../runtime/legacy-worker-reveal-identity' +import { + revealedPaneMatches, + type TerminalTabCreateReply +} from '../../shared/terminal-reveal-identity' let lastWebContents: unknown = null const sentByChannel: [string, ...unknown[]][] = [] @@ -184,22 +186,19 @@ describe('revealTerminalSession identity assertion', () => { }) }) -describe('revealedLegacyWorkerIdentityMatches', () => { +describe('revealedPaneMatches', () => { const candidate = { tabId: 'tab-a', leafId: 'leaf-a', ptyId: 'pty-a' } it('accepts a reveal filed under another worktree key', () => { // Without this the recovery rolled the surface back and the worker never materialized. - expect( - revealedLegacyWorkerIdentityMatches( - { worktreeId: OWNER_WORKTREE_ID, ...candidate }, - candidate - ) - ).toBe(true) + expect(revealedPaneMatches({ worktreeId: OWNER_WORKTREE_ID, ...candidate }, candidate)).toBe( + true + ) }) it('refuses a reply that names a different pane', () => { expect( - revealedLegacyWorkerIdentityMatches( + revealedPaneMatches( { worktreeId: CALLER_WORKTREE_ID, ...candidate, leafId: 'leaf-other' }, candidate ) @@ -207,6 +206,6 @@ describe('revealedLegacyWorkerIdentityMatches', () => { }) it('refuses a reply with no identity at all', () => { - expect(revealedLegacyWorkerIdentityMatches(undefined, candidate)).toBe(false) + expect(revealedPaneMatches(undefined, candidate)).toBe(false) }) }) diff --git a/src/shared/terminal-reveal-identity.ts b/src/shared/terminal-reveal-identity.ts index 514e45e3b6d..44720cbeaed 100644 --- a/src/shared/terminal-reveal-identity.ts +++ b/src/shared/terminal-reveal-identity.ts @@ -12,3 +12,21 @@ export type TerminalTabCreateReply = { identity?: TerminalRevealIdentity error?: string } + +/** + * Whether a reveal reply attests the exact pane the caller asked for. No `worktreeId`: ownership + * is tab-keyed, so the renderer decides which workspace key the row is filed under, and + * re-asserting the caller's key rolled back reveals that had surfaced the right pane elsewhere + * (STA-7961). + */ +export function revealedPaneMatches( + identity: TerminalRevealIdentity | undefined, + expected: { tabId: string; leafId: string; ptyId: string } +): boolean { + return Boolean( + identity && + identity.tabId === expected.tabId && + identity.leafId === expected.leafId && + identity.ptyId === expected.ptyId + ) +}