From efaa661db103262eb752e020f5198426201d0ac0 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Wed, 2 Sep 2026 16:49:41 -0700 Subject: [PATCH] Fix ladder tranche zero review findings --- ...tab-agent-identity-decision-table.test.ts} | 169 ++++++++---------- src/shared/pane-agent-evidence-sources.ts | 19 ++ src/shared/pane-agent-identity-adapter.ts | 16 +- .../pane-agent-identity-resolver.test.ts | 9 + src/shared/pane-agent-identity-resolver.ts | 21 +-- src/shared/pane-agent-owner.test.ts | 36 ++++ src/shared/pane-agent-owner.ts | 56 +++--- 7 files changed, 174 insertions(+), 152 deletions(-) rename src/{shared/pane-agent-identity-decision-table.test.ts => renderer/src/lib/tab-agent-identity-decision-table.test.ts} (52%) create mode 100644 src/shared/pane-agent-evidence-sources.ts diff --git a/src/shared/pane-agent-identity-decision-table.test.ts b/src/renderer/src/lib/tab-agent-identity-decision-table.test.ts similarity index 52% rename from src/shared/pane-agent-identity-decision-table.test.ts rename to src/renderer/src/lib/tab-agent-identity-decision-table.test.ts index 1f9d06f431f..6c543db3594 100644 --- a/src/shared/pane-agent-identity-decision-table.test.ts +++ b/src/renderer/src/lib/tab-agent-identity-decision-table.test.ts @@ -5,15 +5,19 @@ import { describe, expect, it } from 'vitest' import { resolveCanonicalPaneAgentIdentity, type CanonicalPaneAgentIdentity -} from './pane-agent-identity-adapter' -import type { TuiAgent } from './tui-agent' +} from '../../../shared/pane-agent-identity-adapter' +import { resolveTabAgentFromSignals } from './tab-agent-from-signals' +import type { TuiAgent } from '../../../shared/tui-agent' const AGENTS: readonly TuiAgent[] = ['claude', 'codex'] const SLOT_COUNT = 7 const SHAPE_COUNT = 3 ** SLOT_COUNT * 4 * 2 const TITLES: readonly string[] = ['', 'zsh', 'Task - claude', 'Task - codex'] -type Breakdown = Record<'launch' | 'completed-hook' | 'sleeping-session' | 'process', number> +type Breakdown = Record< + 'launch' | 'completed-hook' | 'sleeping-session' | 'process' | 'sibling' | 'title', + number +> function slotValues(mask: number): (TuiAgent | null)[] { let remaining = mask @@ -24,39 +28,6 @@ function slotValues(mask: number): (TuiAgent | null)[] { }) } -function oldTabResult(values: readonly (TuiAgent | null)[], title: string, remote: boolean) { - const [hook, siblingHook, completed, siblingCompleted, process, sleeping, launch] = values - void remote - if (hook) { - return hook - } - if (process) { - return process - } - if (completed && !sleeping && (title === 'Task - claude' || title === 'Task - codex')) { - return title === 'Task - claude' ? 'claude' : 'codex' - } - if (completed) { - return completed - } - if (sleeping) { - return sleeping - } - if (!launch && siblingHook && siblingCompleted && siblingHook !== siblingCompleted) { - return null - } - if (title === 'Task - claude') { - return 'claude' - } - if (title === 'Task - codex') { - return 'codex' - } - if (launch) { - return launch - } - return siblingHook ?? siblingCompleted ?? null -} - function canonicalResult( values: readonly (TuiAgent | null)[], title: string, @@ -88,6 +59,25 @@ function canonicalResult( }) } +function realResult(values: readonly (TuiAgent | null)[], title: string, remote: boolean) { + const [hook, siblingHook, completed, siblingCompleted, process, sleeping, launch] = values + // The seven slots model steady-state observations; this runtime memory bit is intentionally + // held true instead of adding an eighth dimension to the approved 17,496-shape table. + return resolveTabAgentFromSignals({ + hasObservedAgentSignal: true, + isRemote: remote, + title, + hookAgent: hook, + siblingHookAgent: siblingHook, + focusedCompletedHookAgent: completed, + siblingCompletedHookAgent: siblingCompleted, + processAgent: process, + processShellForeground: false, + sleepingSessionAgent: sleeping, + launchAgent: launch ?? undefined + }) +} + function runDecisionTable(withProof: boolean) { let disagreements = 0 let flipped = 0 @@ -95,19 +85,21 @@ function runDecisionTable(withProof: boolean) { launch: 0, 'completed-hook': 0, 'sleeping-session': 0, - process: 0 + process: 0, + sibling: 0, + title: 0 } for (let mask = 0; mask < 3 ** SLOT_COUNT; mask += 1) { const values = slotValues(mask) for (const title of TITLES) { for (const remote of [false, true]) { - const old = oldTabResult(values, title, remote) + const real = realResult(values, title, remote) const canonical = canonicalResult(values, title, withProof) - // The table groups only the approved residual rungs; process-only/ambiguous mismatches are - // accounted for separately by the 1,872 process-starvation flip count below. - if (old !== canonical.agent && canonical.source !== null && canonical.source in breakdown) { + if (real !== canonical.agent) { disagreements += 1 - breakdown[canonical.source as keyof Breakdown] += 1 + if (canonical.source !== null) { + breakdown[canonical.source] += 1 + } } if (!withProof) { const proven = canonicalResult(values, title, true) @@ -127,8 +119,8 @@ function runDecisionTable(withProof: boolean) { return { disagreements, flipped, breakdown } } -describe('approved pane-agent ladder decision table', () => { - it('replays all 17,496 shapes and asserts totals plus per-rung breakdown', () => { +describe('renderer ladder decision table', () => { + it('replays the real shipping ladder and records all rung disagreements', () => { const proofFree = runDecisionTable(false) const freshProof = runDecisionTable(true) const result = { @@ -138,42 +130,57 @@ describe('approved pane-agent ladder decision table', () => { flippedByAddingProof: proofFree.flipped } writeFileSync( - join(tmpdir(), 'orca-pane-agent-identity-decision-table.json'), + join(tmpdir(), 'orca-pane-agent-identity-decision-table-real.json'), `${JSON.stringify(result, null, 2)}\n` ) - expect(proofFree.disagreements).toBe(2_520) - expect(proofFree.breakdown).toEqual({ - launch: 1_908, - 'completed-hook': 468, - 'sleeping-session': 144, - process: 0 + // Re-derived against resolveTabAgentFromSignals (not a hand-written model). These differ from + // the approved 2,520/648 totals and 396/144/72/36 breakdown; see the PR comment. + expect(proofFree).toEqual({ + disagreements: 2_622, + flipped: 1_872, + breakdown: { + launch: 1_884, + 'completed-hook': 478, + 'sleeping-session': 144, + process: 0, + sibling: 54, + title: 6 + } }) - expect(freshProof.disagreements).toBe(648) - expect(freshProof.breakdown).toEqual({ - launch: 612, - 'completed-hook': 36, - 'sleeping-session': 0, - process: 0 + expect(freshProof).toEqual({ + disagreements: 658, + flipped: 0, + breakdown: { + launch: 588, + 'completed-hook': 46, + 'sleeping-session': 0, + process: 0, + sibling: 6, + title: 2 + } }) expect(proofFree.flipped).toBe(1_872) }) - it('requires every freshness field before the process rung can win', () => { + it('requires both freshness fields before process evidence can change the no-proof result', () => { const values = [null, null, null, null, 'codex', null, 'claude'] as const - const missingAge = canonicalResult(values, '', true) - const missingFreshness = resolveCanonicalPaneAgentIdentity({ - foregroundAgent: 'codex', - processProof: { - agent: 'codex', - processIncarnation: 'fixture-process', - authorityId: 'fixture-authority', - capturedAgeMs: undefined as unknown as number, - validForMs: 1_000 - }, - launchAgent: 'claude' + expect(canonicalResult(values, '', false)).toMatchObject({ + agent: 'claude', + source: 'launch' }) - expect(missingAge).toMatchObject({ agent: 'codex', source: 'process' }) - expect(missingFreshness).toMatchObject({ agent: 'claude', source: 'launch' }) + expect( + resolveCanonicalPaneAgentIdentity({ + foregroundAgent: 'codex', + processProof: { + agent: 'codex', + processIncarnation: 'fixture-process', + authorityId: 'fixture-authority', + capturedAgeMs: undefined as unknown as number, + validForMs: 1_000 + }, + launchAgent: 'claude' + }) + ).toMatchObject({ agent: 'claude', source: 'launch' }) expect( resolveCanonicalPaneAgentIdentity({ foregroundAgent: 'codex', @@ -188,24 +195,4 @@ describe('approved pane-agent ladder decision table', () => { }) ).toMatchObject({ agent: 'claude', source: 'launch' }) }) - - it('fences equal-rank conflicts, superseded runs, and title-last fallback', () => { - expect( - resolveCanonicalPaneAgentIdentity({ - siblingAgents: ['claude', 'codex'], - allowSibling: true - }) - ).toMatchObject({ agent: null, ambiguousAt: 'sibling' }) - expect( - resolveCanonicalPaneAgentIdentity({ - completedHookAgent: 'claude', - completedHookRun: { authorityId: 'fixture', incarnation: 1 }, - currentRun: { authorityId: 'fixture', incarnation: 2 }, - title: 'Task - codex' - }) - ).toMatchObject({ agent: 'codex', source: 'title', supersededSources: ['completed-hook'] }) - expect( - resolveCanonicalPaneAgentIdentity({ launchAgent: 'claude', title: 'Codex' }) - ).toMatchObject({ agent: 'claude', source: 'launch' }) - }) }) diff --git a/src/shared/pane-agent-evidence-sources.ts b/src/shared/pane-agent-evidence-sources.ts new file mode 100644 index 00000000000..32b3648494a --- /dev/null +++ b/src/shared/pane-agent-evidence-sources.ts @@ -0,0 +1,19 @@ +/** Evidence classes in canonical strength order, strongest first. */ +export const PANE_AGENT_EVIDENCE_SOURCES = [ + /** A live provider hook for a turn in progress. The agent is running and said so. */ + 'live-hook', + /** The pane's foreground process, as read on the execution host. */ + 'process', + /** Orca launched, resumed, or accepted a command for this agent. A fact Orca owns. */ + 'launch', + /** A provider hook from a turn that finished. Still authoritative about identity. */ + 'completed-hook', + /** A sleeping session record restored for this pane. */ + 'sleeping-session', + /** Another pane in the same tab. Tab-level surfaces only; never pane-scoped routing. */ + 'sibling', + /** Parsed from the terminal title. A decoration channel; anyone can type an agent's name. */ + 'title' +] as const + +export type PaneAgentEvidenceSource = (typeof PANE_AGENT_EVIDENCE_SOURCES)[number] diff --git a/src/shared/pane-agent-identity-adapter.ts b/src/shared/pane-agent-identity-adapter.ts index ec9b6b6accd..4bbd42ba647 100644 --- a/src/shared/pane-agent-identity-adapter.ts +++ b/src/shared/pane-agent-identity-adapter.ts @@ -1,11 +1,12 @@ import { collectAgentTitleEvidence } from './agent-title-evidence' +import { PANE_AGENT_EVIDENCE_SOURCES } from './pane-agent-evidence-sources' import type { PaneAgentEvidence, - PaneAgentEvidenceSource, PaneAgentIdentity, PaneAgentIdentityInput, PaneAgentRunKey } from './pane-agent-identity-resolver' +import type { PaneAgentEvidenceSource } from './pane-agent-evidence-sources' import type { TuiAgent } from './tui-agent' /** @@ -115,15 +116,10 @@ export type CanonicalPaneAgentIdentity = { } /** Authority order, strongest first. This is the only place precedence is expressed. */ -const SOURCE_RANK: readonly PaneAgentEvidenceSource[] = [ - 'live-hook', - 'process', - 'launch', - 'completed-hook', - 'sleeping-session', - 'sibling', - 'title' -] +const SOURCE_RANK: readonly PaneAgentEvidenceSource[] = PANE_AGENT_EVIDENCE_SOURCES + +/** Exported for the source/rank drift ratchet; the rank is the canonical source list itself. */ +export const PANE_AGENT_SOURCE_RANK = SOURCE_RANK /** Run keys only supersede evidence from the same authority; unknown authorities stay eligible. */ function isPaneAgentRunEligible( diff --git a/src/shared/pane-agent-identity-resolver.test.ts b/src/shared/pane-agent-identity-resolver.test.ts index e28fb0cbd22..ee8f5c287bc 100644 --- a/src/shared/pane-agent-identity-resolver.test.ts +++ b/src/shared/pane-agent-identity-resolver.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest' +import { PANE_AGENT_SOURCE_RANK } from './pane-agent-identity-adapter' import { PANE_AGENT_EVIDENCE_SOURCES, type PaneAgentEvidence, @@ -11,6 +12,14 @@ const resolve = (evidence: PaneAgentEvidence[], extra = {}) => const H = 'authority-a' describe('resolvePaneAgentIdentity', () => { + it('keeps every evidence source ranked exactly once', () => { + expect(PANE_AGENT_SOURCE_RANK).toBe(PANE_AGENT_EVIDENCE_SOURCES) + expect(new Set(PANE_AGENT_SOURCE_RANK).size).toBe(PANE_AGENT_SOURCE_RANK.length) + for (const source of PANE_AGENT_EVIDENCE_SOURCES) { + expect(PANE_AGENT_SOURCE_RANK.indexOf(source)).toBeGreaterThanOrEqual(0) + } + }) + describe('a display title is the last thing consulted', () => { it.each(PANE_AGENT_EVIDENCE_SOURCES.filter((s) => s !== 'title' && s !== 'sibling'))( 'lets %s outrank a conflicting title', diff --git a/src/shared/pane-agent-identity-resolver.ts b/src/shared/pane-agent-identity-resolver.ts index c05bb639436..b6dd8acd2c8 100644 --- a/src/shared/pane-agent-identity-resolver.ts +++ b/src/shared/pane-agent-identity-resolver.ts @@ -1,6 +1,9 @@ import { resolveCanonicalPaneAgentEvidence } from './pane-agent-identity-adapter' +import type { PaneAgentEvidenceSource } from './pane-agent-evidence-sources' import type { TuiAgent } from './tui-agent' +export { PANE_AGENT_EVIDENCE_SOURCES } from './pane-agent-evidence-sources' + /** * One place that answers "which agent is in this pane". * @@ -24,24 +27,6 @@ import type { TuiAgent } from './tui-agent' * launch, a recognized command at a shell prompt, a host-confirmed foreground change, a new * provider session. Never by a title changing, and never by transport loss. */ -export const PANE_AGENT_EVIDENCE_SOURCES = [ - /** A live provider hook for a turn in progress. The agent is running and said so. */ - 'live-hook', - /** The pane's foreground process, as read on the execution host. */ - 'process', - /** Orca launched, resumed, or accepted a command for this agent. A fact Orca owns. */ - 'launch', - /** A provider hook from a turn that finished. Still authoritative about identity. */ - 'completed-hook', - /** A sleeping session record restored for this pane. */ - 'sleeping-session', - /** Another pane in the same tab. Tab-level surfaces only; never pane-scoped routing. */ - 'sibling', - /** Parsed from the terminal title. A decoration channel; anyone can type an agent's name. */ - 'title' -] as const -export type PaneAgentEvidenceSource = (typeof PANE_AGENT_EVIDENCE_SOURCES)[number] - /** * Which agent run a piece of evidence belongs to. * diff --git a/src/shared/pane-agent-owner.test.ts b/src/shared/pane-agent-owner.test.ts index 13cfd217c45..b0d61801392 100644 --- a/src/shared/pane-agent-owner.test.ts +++ b/src/shared/pane-agent-owner.test.ts @@ -37,6 +37,42 @@ describe('resolvePaneAgentOwner', () => { ).toBe('omp') }) + it('preserves the pre-tranche precedence for every conflicting owner tier', () => { + expect( + resolvePaneAgentOwnerRecord({ + launchAgent: 'claude', + hookAgent: 'codex', + siblingHookAgent: 'gemini', + completedHookAgent: 'pi', + sleepingSessionAgent: 'omp' + }) + ).toEqual({ agent: 'claude', ownerIsLaunch: true }) + expect( + resolvePaneAgentOwnerRecord({ + hookAgent: 'claude', + siblingHookAgent: 'codex', + completedHookAgent: 'gemini', + siblingCompletedHookAgent: 'pi', + sleepingSessionAgent: 'omp' + }) + ).toEqual({ agent: 'claude', ownerIsLaunch: false }) + expect( + resolvePaneAgentOwnerRecord({ + siblingHookAgent: 'codex', + completedHookAgent: 'claude', + siblingCompletedHookAgent: 'gemini', + sleepingSessionAgent: 'omp' + }) + ).toEqual({ agent: 'codex', ownerIsLaunch: false }) + expect( + resolvePaneAgentOwnerRecord({ + completedHookAgent: 'claude', + siblingCompletedHookAgent: 'codex', + sleepingSessionAgent: 'gemini' + }) + ).toEqual({ agent: 'claude', ownerIsLaunch: false }) + }) + it('returns null when no owner evidence exists', () => { expect(resolvePaneAgentOwner({})).toBeNull() expect(resolvePaneAgentOwner({ launchAgent: null, hookAgent: undefined })).toBeNull() diff --git a/src/shared/pane-agent-owner.ts b/src/shared/pane-agent-owner.ts index 4dea4e6b54f..5b5563670e8 100644 --- a/src/shared/pane-agent-owner.ts +++ b/src/shared/pane-agent-owner.ts @@ -1,5 +1,4 @@ import type { AgentType } from './agent-status-types' -import { resolveCanonicalPaneAgentEvidence } from './pane-agent-identity-adapter' /** * The owner-evidence signals a terminal pane can carry, strongest launch intent @@ -33,10 +32,26 @@ export type ResolvedPaneAgentOwner = { ownerIsLaunch: boolean } +const PANE_OWNER_RANK: readonly { + key: keyof PaneAgentOwnerSignals + ownerIsLaunch: boolean +}[] = [ + { key: 'launchAgent', ownerIsLaunch: true }, + { key: 'startupLaunchAgent', ownerIsLaunch: true }, + { key: 'initialStatusAgent', ownerIsLaunch: true }, + { key: 'commandInferredAgent', ownerIsLaunch: true }, + { key: 'hookAgent', ownerIsLaunch: false }, + { key: 'siblingHookAgent', ownerIsLaunch: false }, + { key: 'completedHookAgent', ownerIsLaunch: false }, + { key: 'siblingCompletedHookAgent', ownerIsLaunch: false }, + { key: 'sleepingSessionAgent', ownerIsLaunch: false } +] + /** - * The single authoritative resolver for "which agent owns this pane", shared by - * the tab-icon resolver, the terminal-pane display/renderer owner, and the - * mirrored-tab title owner so they cannot drift apart. + * Compatibility owner lookup shared by the existing consumer surfaces. + * + * Tranche 0 intentionally preserves this pre-migration precedence byte-for-byte; switching + * these consumers to canonical evidence belongs to tranche 1. * * Why this precedence: launch intent is the authoritative bootstrap before any * process signal exists, so it leads. Once launch metadata is gone — a mirrored @@ -52,38 +67,13 @@ export type ResolvedPaneAgentOwner = { export function resolvePaneAgentOwnerRecord( signals: PaneAgentOwnerSignals ): ResolvedPaneAgentOwner | null { - const evidence = [] as { - source: 'launch' | 'completed-hook' | 'sleeping-session' | 'sibling' - agent: AgentType - }[] - const launchAgent = - signals.launchAgent ?? - signals.startupLaunchAgent ?? - signals.initialStatusAgent ?? - signals.commandInferredAgent - if (launchAgent) { - evidence.push({ source: 'launch', agent: launchAgent }) - } - // This compatibility signal has no liveness bit; treat it as the durable completed-hook rung. - if (signals.hookAgent) { - evidence.push({ source: 'completed-hook', agent: signals.hookAgent }) - } - if (signals.completedHookAgent) { - evidence.push({ source: 'completed-hook', agent: signals.completedHookAgent }) - } - if (signals.sleepingSessionAgent) { - evidence.push({ source: 'sleeping-session', agent: signals.sleepingSessionAgent }) - } - for (const agent of [signals.siblingHookAgent, signals.siblingCompletedHookAgent]) { + for (const { key, ownerIsLaunch } of PANE_OWNER_RANK) { + const agent = signals[key] if (agent) { - evidence.push({ source: 'sibling', agent }) + return { agent, ownerIsLaunch } } } - const identity = resolveCanonicalPaneAgentEvidence({ evidence, allowSibling: true }) - if (!identity.agent) { - return null - } - return { agent: identity.agent, ownerIsLaunch: identity.source === 'launch' } + return null } export function resolvePaneAgentOwner(signals: PaneAgentOwnerSignals): AgentType | null {