mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 08:03:13 +00:00
fix(pi): resolve OMP tab identity from the outer wrapper, not the wrapped pi (#6364)
OMP runs as a `shell → omp → pi` process tree and Orca recognizes both `omp`
and `pi` as distinct agents. The foreground-process readers scored the deepest
`+` foreground descendant, so they returned the wrapped `pi` engine instead of
the `omp` the user actually launched — and because the sampled `+` frame moves
between omp and pi across command boundaries, the read oscillated and the tab
icon/label flickered OMP↔Pi. It surfaced on Remote Host / SSH (only in a
worktree, only while working) where a mirrored/restored pane has no client-side
launchAgent to anchor identity, so the (host-read) foreground was the only
pi/omp signal — and it was wrong.
Fix the detection at its source so foregroundAgent is trustworthy everywhere
(local tab icon, exit detection, and the remote mirror):
- Add `resolveOuterWrapperForegroundProcess` (src/shared/foreground-wrapper-agent.ts):
when the recognized foreground winner shares a `titleIdentityGroup` with a
shallower recognized ancestor, return the shallowest — the outer wrapper.
Stable regardless of which of omp/pi holds the foreground at the sample; bare
Pi and cross-group reads (Codex, etc.) are unchanged.
- Wire it into both readers: local `agent-foreground-process.ts` and the SSH
relay `pty-shell-utils.ts`. Also harden the relay's fallback short-circuit so a
pi-compatible process reported by node-pty rescans for the omp wrapper instead
of trusting `pi` raw (never downgrading below the fallback).
Also unify the host-side publish path onto the shared owner resolver: the mobile
session snapshot builders now resolve pane ownership via `resolvePaneAgentOwner`
({launchAgent, hookAgent}) instead of a bespoke `launchAgent ?? foregroundAgent`,
so the host stops being a divergent fourth identity path and the hook's
host-stamped identity survives when launchAgent is dropped on a mirrored pane.
Tests: unit coverage for the wrapper resolver, `shell→omp→pi` ps trees for both
readers (outer omp, oscillating-frame, bare pi, node-pty pi fallback), and a
host-snapshot regression that keeps an OMP hook labeled OMP when the wrapped pi
child owns the foreground.
This commit is contained in:
@@ -109,6 +109,29 @@ 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.
|
||||
it('reports the outer omp wrapper, not the wrapped pi child', async () => {
|
||||
mockPs(['101 100 S+ omp', '102 101 S+ pi'].join('\n'))
|
||||
|
||||
await expect(resolveAgentForegroundProcess(100, 'omp')).resolves.toBe('omp')
|
||||
})
|
||||
|
||||
it('reports omp even when the wrapped pi child holds the foreground alone', async () => {
|
||||
// Why: across command boundaries only the deeper `pi` carries `+`; the
|
||||
// wrapper identity must stay omp regardless of which frame we sampled.
|
||||
mockPs(['101 100 S omp', '102 101 S+ pi'].join('\n'))
|
||||
|
||||
await expect(resolveAgentForegroundProcess(100, 'omp')).resolves.toBe('omp')
|
||||
})
|
||||
|
||||
it('reports bare pi when no omp wrapper is present', async () => {
|
||||
mockPs(['101 100 S+ pi'].join('\n'))
|
||||
|
||||
await expect(resolveAgentForegroundProcess(100, 'pi')).resolves.toBe('pi')
|
||||
})
|
||||
|
||||
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')
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition'
|
||||
import { resolveOuterWrapperForegroundProcess } from '../../shared/foreground-wrapper-agent'
|
||||
import {
|
||||
getFreshProcessTableSnapshot,
|
||||
getProcessTableSnapshot,
|
||||
@@ -125,7 +126,9 @@ function resolveAgentForegroundProcessFromPs(
|
||||
}
|
||||
const recognized = recognizeAgentProcessFromCommandLine(candidate.command)
|
||||
if (recognized) {
|
||||
return recognized.processName
|
||||
// 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 null
|
||||
|
||||
@@ -14611,6 +14611,55 @@ 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.
|
||||
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)
|
||||
runtime.setPtyController({
|
||||
spawn,
|
||||
write: () => true,
|
||||
kill: () => true,
|
||||
// Why: the remote relay reads the deeper `pi` child of the omp process tree.
|
||||
getForegroundProcess: async () => 'pi'
|
||||
})
|
||||
const events: RuntimeMobileSessionTabsResult[] = []
|
||||
const unsubscribe = runtime.onMobileSessionTabsChanged((snapshot) => events.push(snapshot))
|
||||
|
||||
await runtime.createTerminal(`id:${TEST_WORKTREE_ID}`, {
|
||||
tabId: 'omp-tab',
|
||||
leafId: HEADLESS_LEAF_ID
|
||||
})
|
||||
// Restored/mirrored pane: no launchAgent, only the pi foreground read remains.
|
||||
const pty = (
|
||||
runtime as unknown as {
|
||||
ptysById: Map<string, { launchAgent: string | null; foregroundAgent: string | null }>
|
||||
}
|
||||
).ptysById.get('omp-flicker-pty')!
|
||||
pty.launchAgent = null
|
||||
pty.foregroundAgent = 'pi'
|
||||
events.length = 0
|
||||
|
||||
runtime.onPtyData(
|
||||
'omp-flicker-pty',
|
||||
'\x1b]9999;{"state":"working","prompt":"fix the bug","agentType":"omp"}\x07',
|
||||
100
|
||||
)
|
||||
|
||||
await waitForMobileSessionTabsEvents(events, 1)
|
||||
expect(events[0]?.tabs[0]).toEqual(
|
||||
expect.objectContaining({
|
||||
type: 'terminal',
|
||||
agentStatus: expect.objectContaining({ state: 'working', agentType: 'omp' })
|
||||
})
|
||||
)
|
||||
|
||||
unsubscribe()
|
||||
})
|
||||
|
||||
it('does not republish mobile session tabs for repeated identical OSC 9999 payloads', async () => {
|
||||
const spawn = vi.fn().mockResolvedValue({ id: 'hook-ping-pty' })
|
||||
const runtime = new OrcaRuntimeService(store)
|
||||
|
||||
@@ -39,6 +39,7 @@ import {
|
||||
normalizeCompatibleAgentStatusEntryForOwner,
|
||||
normalizeCompatibleAgentTitleForOwner
|
||||
} from '../../shared/agent-title-owner'
|
||||
import { resolvePaneAgentOwner } from '../../shared/pane-agent-owner'
|
||||
import {
|
||||
createAgentStatusOscProcessor,
|
||||
type ProcessedAgentStatusChunk
|
||||
@@ -22424,7 +22425,16 @@ export class OrcaRuntimeService {
|
||||
)
|
||||
: null
|
||||
const launchAgent = tab.launchAgent ?? liveLeafPty?.launchAgent ?? pty?.launchAgent ?? null
|
||||
const ownerAgent = launchAgent ?? liveLeafPty?.foregroundAgent ?? pty?.foregroundAgent ?? 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.
|
||||
const ownerAgent =
|
||||
resolvePaneAgentOwner({ launchAgent, hookAgent: tab.agentStatus?.agentType ?? null }) ??
|
||||
liveLeafPty?.foregroundAgent ??
|
||||
pty?.foregroundAgent ??
|
||||
null
|
||||
const title = normalizeCompatibleAgentTitleForOwner(
|
||||
leafTitle ?? ptyTitle ?? syncedTab?.title ?? tab.title,
|
||||
ownerAgent
|
||||
@@ -22566,7 +22576,16 @@ export class OrcaRuntimeService {
|
||||
return {}
|
||||
}
|
||||
}
|
||||
const ownerAgent = tab.launchAgent ?? pty?.launchAgent ?? pty?.foregroundAgent ?? null
|
||||
// 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.
|
||||
const ownerAgent =
|
||||
resolvePaneAgentOwner({
|
||||
launchAgent: tab.launchAgent ?? pty?.launchAgent ?? null,
|
||||
hookAgent: retained?.payload.agentType ?? null
|
||||
}) ??
|
||||
pty?.foregroundAgent ??
|
||||
null
|
||||
const terminalTitle = normalizeCompatibleAgentTitleForOwner(
|
||||
(pty ? getLatestPtyTitle(pty) : null) ?? tab.title,
|
||||
ownerAgent
|
||||
|
||||
@@ -269,6 +269,51 @@ 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.
|
||||
it('reports the outer omp wrapper over the wrapped pi child from a shell fallback', async () => {
|
||||
await withProcessPlatform('linux', async () => {
|
||||
mockExecFile((_command, args) => {
|
||||
if (args[0] === '-axo') {
|
||||
return {
|
||||
stdout: ['100 99 Ss bash -l', '101 100 S+ omp', '102 101 S+ pi'].join('\n')
|
||||
}
|
||||
}
|
||||
return new Error('unexpected command')
|
||||
})
|
||||
|
||||
await expect(getForegroundProcessName(100, 'bash')).resolves.toBe('omp')
|
||||
})
|
||||
})
|
||||
|
||||
it('rescans for the omp wrapper when node-pty reports the wrapped pi as foreground', async () => {
|
||||
await withProcessPlatform('linux', async () => {
|
||||
mockExecFile((_command, args) => {
|
||||
if (args[0] === '-axo') {
|
||||
return {
|
||||
stdout: ['100 99 Ss bash -l', '101 100 S+ omp', '102 101 S+ pi'].join('\n')
|
||||
}
|
||||
}
|
||||
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) => {
|
||||
if (args[0] === '-axo') {
|
||||
return { stdout: ['100 99 Ss bash -l', '101 100 S+ pi'].join('\n') }
|
||||
}
|
||||
return new Error('unexpected command')
|
||||
})
|
||||
|
||||
await expect(getForegroundProcessName(100, 'pi')).resolves.toBe('pi')
|
||||
})
|
||||
})
|
||||
|
||||
it('falls back to the root process command when descendant inspection fails', async () => {
|
||||
mockExecFile((_command, args) => {
|
||||
if (args[0] === '-axo') {
|
||||
|
||||
@@ -11,6 +11,8 @@ 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 { isShellProcess } from '../shared/shell-process-detection'
|
||||
import {
|
||||
resolveWindowsAgentForegroundProcess,
|
||||
@@ -232,7 +234,9 @@ async function getRecognizedForegroundDescendant(
|
||||
for (const candidate of inspectionCandidates) {
|
||||
const recognized = recognizeAgentProcessFromCommandLine(candidate.command)
|
||||
if (recognized) {
|
||||
return recognized.processName
|
||||
// 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)
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
@@ -251,6 +255,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) {
|
||||
return (
|
||||
(await getRecognizedForegroundDescendant(pid, fallbackProcess)) ??
|
||||
fallbackRecognition.processName
|
||||
)
|
||||
}
|
||||
return fallbackRecognition.processName
|
||||
}
|
||||
if (process.platform === 'win32') {
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { resolveOuterWrapperForegroundProcess } from './foreground-wrapper-agent'
|
||||
|
||||
describe('resolveOuterWrapperForegroundProcess', () => {
|
||||
const omp = { agent: 'omp' as const, processName: 'omp' }
|
||||
const pi = { agent: 'pi' as const, processName: 'pi' }
|
||||
|
||||
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 }
|
||||
])
|
||||
).toBe('omp')
|
||||
})
|
||||
|
||||
it('keeps bare pi when no same-group wrapper is present', () => {
|
||||
expect(resolveOuterWrapperForegroundProcess(pi, 1, [{ command: 'pi', depth: 1 }])).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 }
|
||||
])
|
||||
).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 }
|
||||
])
|
||||
).toBe('omp')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,45 @@
|
||||
import {
|
||||
recognizeAgentProcessFromCommandLine,
|
||||
type RecognizedAgentProcess
|
||||
} from './agent-process-recognition'
|
||||
import { getSyntheticAgentTitleProfile } from './synthetic-agent-title'
|
||||
|
||||
export type ForegroundAgentCandidate = { command: string; depth: number }
|
||||
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
export function resolveOuterWrapperForegroundProcess(
|
||||
winner: RecognizedAgentProcess,
|
||||
winnerDepth: number,
|
||||
descendants: readonly ForegroundAgentCandidate[]
|
||||
): string {
|
||||
const winnerGroup = getSyntheticAgentTitleProfile(winner.agent)?.titleIdentityGroup
|
||||
if (!winnerGroup) {
|
||||
return winner.processName
|
||||
}
|
||||
let outerProcessName = winner.processName
|
||||
let outerDepth = winnerDepth
|
||||
for (const candidate of descendants) {
|
||||
if (candidate.depth >= outerDepth) {
|
||||
continue
|
||||
}
|
||||
const recognized = recognizeAgentProcessFromCommandLine(candidate.command)
|
||||
if (!recognized) {
|
||||
continue
|
||||
}
|
||||
if (getSyntheticAgentTitleProfile(recognized.agent)?.titleIdentityGroup === winnerGroup) {
|
||||
outerProcessName = recognized.processName
|
||||
outerDepth = candidate.depth
|
||||
}
|
||||
}
|
||||
return outerProcessName
|
||||
}
|
||||
Reference in New Issue
Block a user