From 4548fef3768f06a10d98fc1855ded092116a76f6 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 8 Aug 2026 19:44:33 -0700 Subject: [PATCH] refactor(runtime): collapse duplicate identity comparisons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit G6 requires one identity comparison; five implementations existed across two concepts. Worktree-namespace identity had two: `runtimeWorktreeIdsEqual` and `runtimeWorktreeIdentityKey` independently re-derived repoId plus normalized path. Equality now derives from the key, so the comparison and the sleep / mutation-queue keying cannot drift into two different rules — which is exactly how the suffix-stripping bug reached production once. Pane identity had three byte-identical leaf-UUID comparisons, in orchestration `db.ts`, `lifecycle-reconciliation.ts`, and `orchestration-legacy-process-identity.ts`. One copy moved to `stable-pane-id.ts`, which already owns `PaneKey`, `parsePaneKey` and `makePaneKey` and which all three already imported. No new module, no branded type, no parallel comparison. Net -14 production lines. The namespace oracle still bites: restoring the filesystem parser inside the identity key reddens exactly its five cases. The raw counts are not the actionable set, and the classification is worth recording: of 409 non-test `worktreeId` comparisons, 71 are typeof guards and 81 are sentinel tag checks. Most of the remainder are renderer predicates over store rows where both operands are the same main-minted id, so normalizing there would widen equality rather than correct it. Co-authored-by: Orca --- src/main/runtime/orca-runtime.ts | 10 ++++------ src/main/runtime/orchestration/db.ts | 12 +----------- .../orchestration/lifecycle-reconciliation.ts | 15 ++------------- .../rpc/orchestration-legacy-process-identity.ts | 11 ++++------- src/shared/stable-pane-id.ts | 12 ++++++++++++ 5 files changed, 23 insertions(+), 37 deletions(-) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 2e8daee6bfb..fbe6d7d5061 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -37006,16 +37006,14 @@ function runtimePathsEqual(left: string, right: string): boolean { * Windows/WSL/SSH ids still match themselves across hosts. */ function runtimeWorktreeIdsEqual(left: string, right: string): boolean { - const parsedLeft = splitWorktreeId(left) - const parsedRight = splitWorktreeId(right) - return parsedLeft && parsedRight - ? parsedLeft.repoId === parsedRight.repoId && - runtimePathsEqual(parsedLeft.worktreePath, parsedRight.worktreePath) - : left === right + // Why: derived from the key rather than re-parsed, so equality and the sleep / + // mutation-queue keying can never drift apart into two different identity rules. + return runtimeWorktreeIdentityKey(left) === runtimeWorktreeIdentityKey(right) } function runtimeWorktreeIdentityKey(worktreeId: string): string { // Same suffix rule: this keys PTY refresh, sleep, and mutation-queue state per session. + // NUL cannot occur in a repoId or a path, so the joined key is unambiguous. const parsed = splitWorktreeId(worktreeId) return parsed ? `${parsed.repoId}\0${normalizeRuntimePathForComparison(parsed.worktreePath)}` diff --git a/src/main/runtime/orchestration/db.ts b/src/main/runtime/orchestration/db.ts index 3608ee7d754..5dc08b96f7a 100644 --- a/src/main/runtime/orchestration/db.ts +++ b/src/main/runtime/orchestration/db.ts @@ -39,7 +39,7 @@ import type { } from './types' import { buildOrchestrationTaskDisplayMetadata } from '../../../shared/orchestration-task-display' import { ORCHESTRATION_LEGACY_RUN_ID } from '../../../shared/orchestration-rpc-contract' -import { parsePaneKey } from '../../../shared/stable-pane-id' +import { isEquivalentPaneKey } from '../../../shared/stable-pane-id' import { OrchestrationError } from './orchestration-error' import { resolveOrchestrationMigrationStartVersion } from './orchestration-schema-version-skew' import { @@ -54,16 +54,6 @@ import { import { ORCHESTRATION_RUN_PAGE_LIMIT } from '../../../shared/orchestration-run-pagination' import { ORCHESTRATION_CONTRACT_VERSION } from '../../../shared/protocol-version' -// Why: leaf UUID is the remint-stable pane identity (tab half changes on break-out); exact match covers legacy/unparseable keys. -function isEquivalentPaneKey(a: string, b: string): boolean { - if (a === b) { - return true - } - const aLeaf = parsePaneKey(a)?.leafId - const bLeaf = parsePaneKey(b)?.leafId - return Boolean(aLeaf && bLeaf && aLeaf === bLeaf) -} - // Why: indexable pre-filter for isEquivalentPaneKey — equal strings and equal leaves both share the // text after the first ':', so this narrows candidates without deciding equivalence itself. const RUN_PANE_KEY_MATCH_SUFFIX_SQL = diff --git a/src/main/runtime/orchestration/lifecycle-reconciliation.ts b/src/main/runtime/orchestration/lifecycle-reconciliation.ts index 3d2793413b7..7e9c2bc0efa 100644 --- a/src/main/runtime/orchestration/lifecycle-reconciliation.ts +++ b/src/main/runtime/orchestration/lifecycle-reconciliation.ts @@ -1,17 +1,6 @@ import type { OrchestrationDb } from './db' import type { MessageRow, WorkerReportOutcome } from './types' -import { parsePaneKey } from '../../../shared/stable-pane-id' - -// Why: the tab half can change on pane break-out, while opaque legacy keys -// have no safe equivalence beyond exact equality. -function isSamePane(assigneePaneKey: string, senderPaneKey: string): boolean { - if (assigneePaneKey === senderPaneKey) { - return true - } - const assigneeLeaf = parsePaneKey(assigneePaneKey)?.leafId - const senderLeaf = parsePaneKey(senderPaneKey)?.leafId - return Boolean(assigneeLeaf && senderLeaf && assigneeLeaf === senderLeaf) -} +import { isEquivalentPaneKey } from '../../../shared/stable-pane-id' function hasLifecycleAuthority( dispatch: { assignee_handle: string | null; assignee_pane_key: string | null }, @@ -19,7 +8,7 @@ function hasLifecycleAuthority( ): boolean { if (dispatch.assignee_pane_key) { return Boolean( - msg.sender_pane_key && isSamePane(dispatch.assignee_pane_key, msg.sender_pane_key) + msg.sender_pane_key && isEquivalentPaneKey(dispatch.assignee_pane_key, msg.sender_pane_key) ) } // Why: rows created before pane identity existed can only use the exact diff --git a/src/main/runtime/rpc/orchestration-legacy-process-identity.ts b/src/main/runtime/rpc/orchestration-legacy-process-identity.ts index 35054c61279..14e4f3e46f3 100644 --- a/src/main/runtime/rpc/orchestration-legacy-process-identity.ts +++ b/src/main/runtime/rpc/orchestration-legacy-process-identity.ts @@ -1,6 +1,8 @@ -import { parsePaneKey } from '../../../shared/stable-pane-id' +import { isEquivalentPaneKey } from '../../../shared/stable-pane-id' import { OrchestrationError } from '../orchestration/orchestration-error' +// Why: legacy rows carry nullable pane keys, and a missing key is never proof of +// identity — so absence is refused here rather than inside the shared comparison. export function equivalentLegacyPaneKey( a: string | null | undefined, b: string | null | undefined @@ -8,12 +10,7 @@ export function equivalentLegacyPaneKey( if (!a || !b) { return false } - if (a === b) { - return true - } - const aLeaf = parsePaneKey(a)?.leafId - const bLeaf = parsePaneKey(b)?.leafId - return Boolean(aLeaf && bLeaf && aLeaf === bLeaf) + return isEquivalentPaneKey(a, b) } export function legacyReadOnlyError(): OrchestrationError { diff --git a/src/shared/stable-pane-id.ts b/src/shared/stable-pane-id.ts index a68117f3d74..47d973022bf 100644 --- a/src/shared/stable-pane-id.ts +++ b/src/shared/stable-pane-id.ts @@ -44,6 +44,18 @@ export function parsePaneKey( return { tabId, leafId, stablePaneId: leafId } } +// Why: only the leaf UUID is remint-stable pane identity (the tab half changes on +// pane break-out), while opaque legacy keys that do not parse have no safe +// equivalence beyond exact equality. +export function isEquivalentPaneKey(a: string, b: string): boolean { + if (a === b) { + return true + } + const aLeaf = parsePaneKey(a)?.leafId + const bLeaf = parsePaneKey(b)?.leafId + return Boolean(aLeaf && bLeaf && aLeaf === bLeaf) +} + export function parseLegacyNumericPaneKey( paneKey: unknown ): { tabId: string; numericPaneId: string; paneKey: string } | null {