fix(pi): harden wrapper foreground ownership

This commit is contained in:
Brennan Benson
2026-07-20 12:02:26 -07:00
parent 0ac48225e7
commit e101d9703d
9 changed files with 255 additions and 81 deletions
@@ -109,9 +109,7 @@ describe('resolveAgentForegroundProcess', () => {
await expect(resolveAgentForegroundProcess(100, 'node')).resolves.toBe('codex')
})
// #6364 / STA-944: OMP runs as shell→omp→pi and both are recognized agents.
// The deepest-foreground scan must not surface the wrapped `pi` engine — the
// user launched `omp`, so the outer wrapper is the identity.
// Why: OMP embeds Pi, but the outer process is the user-visible identity (#6364).
it('reports the outer omp wrapper, not the wrapped pi child', async () => {
mockPs(['101 100 S+ omp', '102 101 S+ pi'].join('\n'))
@@ -132,6 +130,76 @@ describe('resolveAgentForegroundProcess', () => {
await expect(resolveAgentForegroundProcess(100, 'pi')).resolves.toBe('pi')
})
it('reports the outer omp wrapper on Windows', async () => {
Object.defineProperty(process, 'platform', { value: 'win32' })
execFileMock.mockImplementation(
(_cmd: string, _args: string[], _opts: unknown, cb: unknown) => {
const callback = cb as (err: unknown, result: { stdout: string; stderr: string }) => void
callback(null, {
stdout: windowsProcessJsonRows([
{
CommandLine: 'powershell.exe',
Name: 'powershell.exe',
ParentProcessId: 99,
ProcessId: 100
},
{
CommandLine: 'omp.exe',
Name: 'omp.exe',
ParentProcessId: 100,
ProcessId: 101
},
{
CommandLine: 'pi.exe',
Name: 'pi.exe',
ParentProcessId: 101,
ProcessId: 102
}
]),
stderr: ''
})
}
)
await expect(resolveAgentForegroundProcess(100, 'pi.exe')).resolves.toBe('omp')
})
it('keeps the Windows omp ancestor when context selects one of multiple pi descendants', async () => {
Object.defineProperty(process, 'platform', { value: 'win32' })
mockPs(
windowsProcessJsonRows([
{
CommandLine: 'powershell.exe',
Name: 'powershell.exe',
ParentProcessId: 99,
ProcessId: 100
},
{
CommandLine: 'omp.exe',
Name: 'omp.exe',
ParentProcessId: 100,
ProcessId: 101
},
{
CommandLine: 'pi.exe --cwd C:\\repo\\orca',
Name: 'pi.exe',
ParentProcessId: 101,
ProcessId: 102
},
{
CommandLine: 'pi.exe --cwd C:\\repo\\other',
Name: 'pi.exe',
ParentProcessId: 100,
ProcessId: 103
}
])
)
await expect(
resolveAgentForegroundProcess(100, 'pi.exe', { contextPaths: ['C:\\repo\\orca'] })
).resolves.toBe('omp')
})
it('treats a fresh POSIX snapshot missing the PTY root as unavailable', async () => {
mockPs('101 999 S+ node /Users/dev/.nvm/versions/node/bin/codex')
@@ -128,7 +128,7 @@ function resolveAgentForegroundProcessFromPs(
if (recognized) {
// Why: return the outer wrapper (omp) rather than the deeper wrapped child
// (pi) of a shell→omp→pi tree — see resolveOuterWrapperForegroundProcess.
return resolveOuterWrapperForegroundProcess(recognized, candidate.depth, candidates)
return resolveOuterWrapperForegroundProcess(recognized, candidate, candidates)
}
}
return null
@@ -1,8 +1,14 @@
import {
isAgentForegroundWrapperProcess,
isExpectedAgentProcess,
recognizeAgentProcessFromCommandLine
recognizeAgentProcess,
recognizeAgentProcessFromCommandLine,
type RecognizedAgentProcess
} from '../../shared/agent-process-recognition'
import {
resolveOuterWrapperForegroundProcess,
shouldInspectOuterWrapperForegroundProcess
} from '../../shared/foreground-wrapper-agent'
import { isShellProcess } from '../../shared/shell-process-detection'
import {
queryWindowsProcessDescendants,
@@ -26,7 +32,12 @@ export type WindowsAgentForegroundResolution = {
}
export function shouldInspectWindowsAgentForeground(fallbackProcess: string): boolean {
return isAgentForegroundWrapperProcess(fallbackProcess) || isShellProcess(fallbackProcess)
const recognized = recognizeAgentProcess(fallbackProcess)
return (
isAgentForegroundWrapperProcess(fallbackProcess) ||
isShellProcess(fallbackProcess) ||
(recognized !== null && shouldInspectOuterWrapperForegroundProcess(recognized))
)
}
export async function resolveWindowsAgentForegroundProcess(
@@ -106,14 +117,18 @@ function resolveWindowsProcessName(
windowsCandidateMatchesFallbackWrapper(candidate, fallbackProcess)
)
if (wrapperCandidates.length !== 1) {
return resolveWrapperForegroundProcessFromWindowsCandidates(wrapperCandidates, contextPaths)
return resolveWrapperForegroundProcessFromWindowsCandidates(
wrapperCandidates,
candidates,
contextPaths
)
}
const [candidate] = wrapperCandidates
const recognized =
recognizeAgentProcessFromCommandLine(candidate.command) ??
recognizeAgentProcessFromCommandLine(candidate.name)
if (recognized) {
return recognized.processName
return resolveOuterWrapperForegroundProcess(recognized, candidate, candidates)
}
return null
}
@@ -132,6 +147,7 @@ function resolveShellForegroundProcessFromWindowsCandidates(
function resolveWrapperForegroundProcessFromWindowsCandidates(
candidates: readonly WindowsProcessCandidate[],
allCandidates: readonly WindowsProcessCandidate[],
contextPaths: readonly string[] | undefined
): string | null {
const contextCandidates = createRecognizedWindowsProcessCandidates(
@@ -139,7 +155,7 @@ function resolveWrapperForegroundProcessFromWindowsCandidates(
contextPaths
).filter((candidate) => candidate.contextMatch)
return contextCandidates.length > 0
? resolveRecognizedWindowsProcessCandidates(contextCandidates, candidates)
? resolveRecognizedWindowsProcessCandidates(contextCandidates, allCandidates)
: null
}
@@ -147,6 +163,7 @@ type RecognizedWindowsProcessCandidate = WindowsProcessRow & {
contextMatch: boolean
depth: number
processName: string
recognized: RecognizedAgentProcess
}
function createRecognizedWindowsProcessCandidates(
@@ -163,7 +180,8 @@ function createRecognizedWindowsProcessCandidates(
{
...candidate,
contextMatch: candidateMatchesContextPath(candidate, normalizedContextPaths),
processName: recognized
processName: recognized.processName,
recognized
}
]
})
@@ -176,13 +194,6 @@ function resolveRecognizedWindowsProcessCandidates(
if (recognizedCandidates.length === 0) {
return null
}
const recognizedProcessNames = new Set(
recognizedCandidates.map((candidate) => candidate.processName)
)
if (recognizedProcessNames.size === 1) {
return [...recognizedProcessNames][0]
}
const candidatesByPid = new Map(allCandidates.map((candidate) => [candidate.pid, candidate]))
const leafCandidates = recognizedCandidates.filter(
(candidate) =>
@@ -192,7 +203,11 @@ function resolveRecognizedWindowsProcessCandidates(
windowsCandidateIsAncestor(candidate, other, candidatesByPid)
)
)
const leafProcessNames = new Set(leafCandidates.map((candidate) => candidate.processName))
const leafProcessNames = new Set(
leafCandidates.map((candidate) =>
resolveOuterWrapperForegroundProcess(candidate.recognized, candidate, allCandidates)
)
)
// Why: Windows lacks a cheap PTY foreground marker like POSIX '+'. A single
// recognized lineage leaf is strong enough; sibling agent leaves are not.
return leafProcessNames.size === 1 ? [...leafProcessNames][0] : null
@@ -267,11 +282,13 @@ function commandLineContainsPath(haystack: string, contextPath: string): boolean
return false
}
function recognizeWindowsProcessCandidate(candidate: WindowsProcessRow): string | null {
const recognized =
function recognizeWindowsProcessCandidate(
candidate: WindowsProcessRow
): RecognizedAgentProcess | null {
return (
recognizeAgentProcessFromCommandLine(candidate.command) ??
recognizeAgentProcessFromCommandLine(candidate.name)
return recognized?.processName ?? null
)
}
function windowsCandidateMatchesFallbackWrapper(
+9 -7
View File
@@ -14611,11 +14611,7 @@ describe('OrcaRuntimeService', () => {
unsubscribe()
})
// STA-944 / #6364: OMP runs as shell→omp→pi, and its status extension posts
// /hook/omp so the hook durably carries `omp`. On a restored/mirrored pane the
// omp launchAgent is dropped, and the foreground reader returns the deeper `pi`
// child. The host must keep publishing `omp` from the hook, not normalize it
// onto the (oscillating) pi foreground — otherwise the mirrored tab flips OMP↔Pi.
// Why: restored OMP panes can retain the hook while the wrapped Pi owns foreground (#6364).
it('keeps an OMP hook labeled OMP when the wrapped pi child owns the foreground', async () => {
const spawn = vi.fn().mockResolvedValue({ id: 'omp-flicker-pty' })
const runtime = new OrcaRuntimeService(store)
@@ -14645,7 +14641,8 @@ describe('OrcaRuntimeService', () => {
runtime.onPtyData(
'omp-flicker-pty',
'\x1b]9999;{"state":"working","prompt":"fix the bug","agentType":"omp"}\x07',
'\x1b]0;⠋ Pi\x07' +
'\x1b]9999;{"state":"working","prompt":"fix the bug","agentType":"omp"}\x07',
100
)
@@ -14653,7 +14650,12 @@ describe('OrcaRuntimeService', () => {
expect(events[0]?.tabs[0]).toEqual(
expect.objectContaining({
type: 'terminal',
agentStatus: expect.objectContaining({ state: 'working', agentType: 'omp' })
title: '⠋ OMP',
agentStatus: expect.objectContaining({
state: 'working',
agentType: 'omp',
terminalTitle: '⠋ OMP'
})
})
)
+21 -12
View File
@@ -22412,6 +22412,12 @@ export class OrcaRuntimeService {
const paneKey = isTerminalLeafId(tab.leafId)
? makePaneKey(tab.parentTabId, tab.leafId)
: `${tab.parentTabId}:${legacyPaneId ?? tab.leafId}`
const mobileStatusPty = livePty ?? pty
// Why: headless hooks live only in main's retained rows; reuse this lookup
// for both title ownership and status publication so the two cannot diverge.
const retainedAgentStatus = tab.agentStatus
? null
: this.getFreshRetainedAgentStatusForMobileTab(paneKey, liveLeafPty ?? mobileStatusPty, tab)
const leafTitle = leaf
? getLatestAgentCandidateTitle(
{ title: leaf.paneTitle, updatedAt: leaf.paneTitleUpdatedAt },
@@ -22425,13 +22431,12 @@ export class OrcaRuntimeService {
)
: null
const launchAgent = tab.launchAgent ?? liveLeafPty?.launchAgent ?? pty?.launchAgent ?? null
// Why: rank the pane's own hook identity above foregroundAgent. OMP's hook
// posts /hook/omp (runtime exec detection) so it durably carries `omp`, while
// foregroundAgent reads the deeper wrapped `pi` of a shell→omp→pi tree and
// oscillates — normalizing the hook onto it flipped OMP tabs to Pi once
// launchAgent was dropped (restored/mirrored pane). Unify on the shared owner.
// Why: a retained OMP hook stays stable while wrapper foreground reads can report Pi.
const ownerAgent =
resolvePaneAgentOwner({ launchAgent, hookAgent: tab.agentStatus?.agentType ?? null }) ??
resolvePaneAgentOwner({
launchAgent,
hookAgent: tab.agentStatus?.agentType ?? retainedAgentStatus?.payload.agentType ?? null
}) ??
liveLeafPty?.foregroundAgent ??
pty?.foregroundAgent ??
null
@@ -22493,7 +22498,13 @@ export class OrcaRuntimeService {
...(tab.ptyId ? { ptyId: tab.ptyId } : {}),
...(tab.terminalTheme ? { terminalTheme: tab.terminalTheme } : {}),
...(launchAgent ? { launchAgent } : {}),
...(agentStatus ?? this.buildPtyMobileAgentStatus(livePty ?? pty, tab, terminalHandle)),
...(agentStatus ??
this.buildPtyMobileAgentStatus(
mobileStatusPty,
tab,
terminalHandle,
retainedAgentStatus
)),
...(tab.parentLayout ? { parentLayout: tab.parentLayout } : {}),
...(tab.startupCwd ? { startupCwd: tab.startupCwd } : {}),
...(tab.color != null ? { color: tab.color } : {}),
@@ -22548,10 +22559,10 @@ export class OrcaRuntimeService {
private buildPtyMobileAgentStatus(
pty: RuntimePtyWorktreeRecord | null,
tab: RuntimeMobileSessionTerminalTab,
terminalHandle: string | null
terminalHandle: string | null,
retained: RuntimeAgentRowSnapshot | null
): { agentStatus: AgentStatusEntry } | Record<string, never> {
const paneKey = this.getMobileTerminalPaneKey(tab)
const retained = this.getFreshRetainedAgentStatusForMobileTab(paneKey, pty, tab)
if (!pty?.lastAgentStatus && !retained) {
return {}
}
@@ -22576,9 +22587,7 @@ export class OrcaRuntimeService {
return {}
}
}
// Why: the retained hook identity outranks foregroundAgent — OMP's hook posts
// /hook/omp so it durably carries `omp`, while foregroundAgent reads the wrapped
// `pi` of a shell→omp→pi tree and oscillates. See buildMobileSessionSnapshot.
// Why: a retained OMP hook stays stable while wrapper foreground reads can report Pi.
const ownerAgent =
resolvePaneAgentOwner({
launchAgent: tab.launchAgent ?? pty?.launchAgent ?? null,
+47 -2
View File
@@ -8,6 +8,7 @@ vi.mock('child_process', () => ({
execFile: execFileMock
}))
import { resetWindowsProcessRowsSnapshotForTests } from '../main/providers/windows-foreground-process-rows'
import { resetProcessTableSnapshotForTests } from '../shared/process-table-snapshot'
import {
getForegroundProcessName,
@@ -50,6 +51,7 @@ async function withProcessPlatform<T>(
beforeEach(() => {
execFileMock.mockReset()
resetProcessTableSnapshotForTests()
resetWindowsProcessRowsSnapshotForTests()
})
describe('isProcessAlive', () => {
@@ -269,8 +271,7 @@ describe('getForegroundProcessName', () => {
})
})
// #6364 / STA-944: OMP runs as shell→omp→pi. The relay scan must report the
// outer `omp` wrapper the user launched, not the deeper wrapped `pi` engine.
// Why: OMP embeds Pi, but the outer process is the user-visible identity (#6364).
it('reports the outer omp wrapper over the wrapped pi child from a shell fallback', async () => {
await withProcessPlatform('linux', async () => {
mockExecFile((_command, args) => {
@@ -301,6 +302,50 @@ describe('getForegroundProcessName', () => {
})
})
it('returns an outer omp fallback without a process-table scan', async () => {
mockExecFile(() => new Error('unexpected process-table scan'))
await expect(getForegroundProcessName(100, 'omp')).resolves.toBe('omp')
expect(execFileMock).not.toHaveBeenCalled()
})
it('rescans a Windows pi fallback for its outer omp wrapper', async () => {
await withProcessPlatform('win32', async () => {
mockExecFile((command) => {
if (command === 'powershell.exe') {
return {
stdout: JSON.stringify([
{
CommandLine: 'powershell.exe',
ExecutablePath: 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe',
Name: 'powershell.exe',
ParentProcessId: 99,
ProcessId: 100
},
{
CommandLine: 'omp.exe',
ExecutablePath: 'C:\\Tools\\omp.exe',
Name: 'omp.exe',
ParentProcessId: 100,
ProcessId: 101
},
{
CommandLine: 'pi.exe',
ExecutablePath: 'C:\\Tools\\pi.exe',
Name: 'pi.exe',
ParentProcessId: 101,
ProcessId: 102
}
])
}
}
return new Error('unexpected command')
})
await expect(getForegroundProcessName(100, 'pi')).resolves.toBe('omp')
})
})
it('keeps a pi fallback as pi when no omp wrapper is in the tree', async () => {
await withProcessPlatform('linux', async () => {
mockExecFile((_command, args) => {
+14 -7
View File
@@ -11,8 +11,10 @@ import {
} from '../shared/agent-process-recognition'
import { getFirstCommandToken } from '../shared/command-token-scanner'
import { getProcessTableSnapshot, type ProcessTableRow } from '../shared/process-table-snapshot'
import { resolveOuterWrapperForegroundProcess } from '../shared/foreground-wrapper-agent'
import { getSyntheticAgentTitleProfile } from '../shared/synthetic-agent-title'
import {
resolveOuterWrapperForegroundProcess,
shouldInspectOuterWrapperForegroundProcess
} from '../shared/foreground-wrapper-agent'
import { isShellProcess } from '../shared/shell-process-detection'
import {
resolveWindowsAgentForegroundProcess,
@@ -236,7 +238,7 @@ async function getRecognizedForegroundDescendant(
if (recognized) {
// Why: return the outer wrapper (omp) rather than the deeper wrapped child
// (pi) of a shell→omp→pi tree — see resolveOuterWrapperForegroundProcess.
return resolveOuterWrapperForegroundProcess(recognized, candidate.depth, candidates)
return resolveOuterWrapperForegroundProcess(recognized, candidate, candidates)
}
}
} catch {
@@ -255,10 +257,15 @@ export async function getForegroundProcessName(
if (fallbackProcess) {
const fallbackRecognition = recognizeAgentProcess(fallbackProcess)
if (fallbackRecognition) {
// Why: node-pty may report the wrapped `pi` of a shell→omp→pi tree as the
// foreground. A pi-compatible fallback still needs the descendant scan to
// surface the outer `omp` wrapper; never downgrade below the fallback.
if (getSyntheticAgentTitleProfile(fallbackRecognition.agent)?.titleIdentityGroup) {
// Why: node-pty can report OMP's wrapped Pi; enrich only that ambiguous
// fallback so authoritative OMP reads keep the zero-subprocess fast path.
if (shouldInspectOuterWrapperForegroundProcess(fallbackRecognition)) {
if (process.platform === 'win32') {
return (
(await resolveWindowsAgentForegroundProcess(pid, fallbackProcess, {})) ??
fallbackRecognition.processName
)
}
return (
(await getRecognizedForegroundDescendant(pid, fallbackProcess)) ??
fallbackRecognition.processName
+26 -11
View File
@@ -8,33 +8,48 @@ describe('resolveOuterWrapperForegroundProcess', () => {
it('collapses a wrapped pi read onto the shallower omp wrapper', () => {
// Winner is the deeper pi (depth 2); omp is its wrapper at depth 1.
expect(
resolveOuterWrapperForegroundProcess(pi, 2, [
{ command: 'pi', depth: 2 },
{ command: 'omp', depth: 1 }
resolveOuterWrapperForegroundProcess(pi, { pid: 102, ppid: 101, command: 'pi' }, [
{ pid: 102, ppid: 101, command: 'pi' },
{ pid: 101, ppid: 100, command: 'omp' }
])
).toBe('omp')
})
it('keeps bare pi when no same-group wrapper is present', () => {
expect(resolveOuterWrapperForegroundProcess(pi, 1, [{ command: 'pi', depth: 1 }])).toBe('pi')
const barePi = { pid: 101, ppid: 100, command: 'pi' }
expect(resolveOuterWrapperForegroundProcess(pi, barePi, [barePi])).toBe('pi')
})
it('leaves a cross-group agent (codex) untouched even under a deeper same-name child', () => {
const codex = { agent: 'codex' as const, processName: 'codex' }
expect(
resolveOuterWrapperForegroundProcess(codex, 2, [
{ command: 'node /usr/bin/codex', depth: 2 },
{ command: 'bash -l', depth: 1 }
])
resolveOuterWrapperForegroundProcess(
codex,
{ pid: 102, ppid: 101, command: 'node /usr/bin/codex' },
[
{ pid: 102, ppid: 101, command: 'node /usr/bin/codex' },
{ pid: 101, ppid: 100, command: 'bash -l' }
]
)
).toBe('codex')
})
it('does not promote a deeper pi over an already-outer omp winner', () => {
expect(
resolveOuterWrapperForegroundProcess(omp, 1, [
{ command: 'omp', depth: 1 },
{ command: 'pi', depth: 2 }
resolveOuterWrapperForegroundProcess(omp, { pid: 101, ppid: 100, command: 'omp' }, [
{ pid: 101, ppid: 100, command: 'omp' },
{ pid: 102, ppid: 101, command: 'pi' }
])
).toBe('omp')
})
it('does not treat an unrelated shallower omp sibling as the pi wrapper', () => {
expect(
resolveOuterWrapperForegroundProcess(pi, { pid: 103, ppid: 102, command: 'pi' }, [
{ pid: 101, ppid: 100, command: 'omp' },
{ pid: 102, ppid: 100, command: 'node server.js' },
{ pid: 103, ppid: 102, command: 'pi' }
])
).toBe('pi')
})
})
+32 -21
View File
@@ -4,42 +4,53 @@ import {
} from './agent-process-recognition'
import { getSyntheticAgentTitleProfile } from './synthetic-agent-title'
export type ForegroundAgentCandidate = { command: string; depth: number }
export type ForegroundAgentCandidate = {
pid: number
ppid: number
command: string
name?: string
}
export function shouldInspectOuterWrapperForegroundProcess(
process: RecognizedAgentProcess
): boolean {
// Why: only Pi is currently embedded by a same-group wrapper; scanning OMP would add a subprocess to every relay poll.
return process.agent === 'pi'
}
/**
* Collapse a foreground read onto the OUTER wrapper when one agent embeds
* another of the same title-identity group. OMP runs as `shell → omp → pi` and
* both are recognized agents, so the deepest-foreground scan otherwise returns
* the wrapped `pi` child — but `omp` is the agent the user launched and sees.
* Given the reader's recognized winner plus every descendant, return the
* shallowest same-group agent (the wrapper). Un-nested or cross-group reads
* (bare Pi, Codex, etc.) are returned unchanged, and the result is stable
* regardless of which of omp/pi holds the terminal foreground at the sampled
* instant — which is what stops the OMP↔Pi tab-icon flicker at its source.
* Collapse a foreground read onto its outermost same-title-group ancestor.
* Why: OMP embeds Pi, while depth alone cannot distinguish wrappers from sibling jobs.
*/
export function resolveOuterWrapperForegroundProcess(
winner: RecognizedAgentProcess,
winnerDepth: number,
winnerCandidate: ForegroundAgentCandidate,
descendants: readonly ForegroundAgentCandidate[]
): string {
const winnerGroup = getSyntheticAgentTitleProfile(winner.agent)?.titleIdentityGroup
if (!winnerGroup) {
return winner.processName
}
const candidatesByPid = new Map(descendants.map((candidate) => [candidate.pid, candidate]))
const seen = new Set<number>([winnerCandidate.pid])
let outerProcessName = winner.processName
let outerDepth = winnerDepth
for (const candidate of descendants) {
if (candidate.depth >= outerDepth) {
continue
let parentPid = winnerCandidate.ppid
while (!seen.has(parentPid)) {
seen.add(parentPid)
const candidate = candidatesByPid.get(parentPid)
if (!candidate) {
break
}
const recognized = recognizeAgentProcessFromCommandLine(candidate.command)
if (!recognized) {
continue
}
if (getSyntheticAgentTitleProfile(recognized.agent)?.titleIdentityGroup === winnerGroup) {
const recognized =
recognizeAgentProcessFromCommandLine(candidate.command) ??
recognizeAgentProcessFromCommandLine(candidate.name)
if (
recognized &&
getSyntheticAgentTitleProfile(recognized.agent)?.titleIdentityGroup === winnerGroup
) {
outerProcessName = recognized.processName
outerDepth = candidate.depth
}
parentPid = candidate.ppid
}
return outerProcessName
}