mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
fix(pi): claim the status pane when the inherited owner PID is dead (STA-5245) (#16631)
* fix(pi): claim the status pane when the inherited owner PID is dead (STA-5245) The managed pi/omp/prime-agent status extension suppressed itself whenever ORCA_PI_STATUS_OWNED held a PID other than its own, with no check that the owner still existed. A restart leaves the previous owner's PID in the inherited env, so every later load returned early and the pane stopped reporting status permanently. Probe the owner before suppressing. Only ESRCH proves it is gone; any other probe result keeps suppression so a live foreign owner still cannot double-report. This mirrors the tri-state in main/agent-hooks/managed-hook-owner-identity.ts, which the extension cannot import because it loads inside the pi/omp runtime with no Orca deps. Also extracts the generated-source test harness into its own module so the suite stays under the max-lines limit. * fix(pi): validate inherited status owner pid markers --------- Co-authored-by: Neil <neil@stably.ai>
This commit is contained in:
@@ -0,0 +1,29 @@
|
||||
name: Pi owner runtime verification
|
||||
on:
|
||||
pull_request:
|
||||
paths:
|
||||
- 'src/main/pi/agent-status-handler-source.ts'
|
||||
- 'tests/tools/pi-owner-runtime-smoke.mjs'
|
||||
- '.github/workflows/pi-owner-runtime.yml'
|
||||
workflow_dispatch:
|
||||
permissions:
|
||||
contents: read
|
||||
jobs:
|
||||
runtime:
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
os: [ubuntu-latest, macos-latest, windows-latest]
|
||||
runs-on: ${{ matrix.os }}
|
||||
timeout-minutes: 20
|
||||
env:
|
||||
ORCA_BACKGROUND_LAUNCH: '1'
|
||||
steps:
|
||||
- uses: actions/checkout@v6
|
||||
with:
|
||||
persist-credentials: false
|
||||
- uses: ./.github/actions/install-node-dependencies
|
||||
- name: Install pinned extension loader
|
||||
run: npm install --prefix .cache/pi-owner --ignore-scripts --no-audit --no-fund @earendil-works/pi-coding-agent@0.83.0
|
||||
- name: Verify real owner exit and hook delivery
|
||||
run: node tests/tools/pi-owner-runtime-smoke.mjs .cache/pi-owner/node_modules/@earendil-works/pi-coding-agent
|
||||
@@ -24,6 +24,7 @@ type FakeCurlChild = {
|
||||
}
|
||||
|
||||
export type AgentStatusExtensionHarness = {
|
||||
killMock: ReturnType<typeof vi.fn>
|
||||
fetchMock: ReturnType<typeof vi.fn>
|
||||
spawnMock: ReturnType<typeof vi.fn>
|
||||
spawnedChildren: FakeCurlChild[]
|
||||
@@ -57,6 +58,7 @@ export const AGENT_STATUS_EXTENSION_SELF_PID = 4242
|
||||
|
||||
export function createAgentStatusExtensionHarness(args: {
|
||||
kind: 'pi' | 'omp' | 'prime-agent'
|
||||
killImpl?: (pid: number, signal: number) => void
|
||||
env?: Record<string, string | undefined>
|
||||
pid?: number
|
||||
title?: string
|
||||
@@ -115,7 +117,9 @@ export function createAgentStatusExtensionHarness(args: {
|
||||
throw new Error(`unexpected require(${specifier})`)
|
||||
})
|
||||
|
||||
const killMock = vi.fn(args.killImpl ?? (() => undefined))
|
||||
const processMock = {
|
||||
kill: killMock,
|
||||
env: {
|
||||
...BASE_ENV,
|
||||
...(args.kind === 'prime-agent' ? { PRIME_AGENT_INTERNAL_DAEMON_WORKER: '1' } : {}),
|
||||
@@ -172,6 +176,7 @@ export function createAgentStatusExtensionHarness(args: {
|
||||
|
||||
return {
|
||||
fetchMock,
|
||||
killMock,
|
||||
spawnMock,
|
||||
spawnedChildren,
|
||||
fsMock,
|
||||
|
||||
@@ -88,13 +88,31 @@ export function getPiAgentStatusHandlerSourceLines(kind: PiAgentKind): string[]
|
||||
'// etc.), so we forward the raw object verbatim under the same field',
|
||||
'// names Claude uses (tool_name / tool_input) and let the server pick the',
|
||||
'// preview. Keeps tool-name knowledge centralized on the receiver side.',
|
||||
'// Why: a restarted agent inherits the previous owner PID through env, so a',
|
||||
'// dead owner must be claimable or the pane goes silent for good. Only ESRCH',
|
||||
'// proves the owner is gone -- every other probe result keeps suppression, so',
|
||||
'// a live foreign owner still cannot double-report. Mirrors the tri-state in',
|
||||
'// main/agent-hooks/managed-hook-owner-identity.ts, which this runtime cannot',
|
||||
'// import (the extension loads inside pi/omp with no Orca deps).',
|
||||
'function isStatusOwnerAlive(pid: string): boolean {',
|
||||
' const parsed = Number(pid)',
|
||||
' if (!Number.isSafeInteger(parsed) || parsed < 1 || parsed > 0x7fffffff) return false',
|
||||
" if (typeof process.kill !== 'function') return true",
|
||||
' try {',
|
||||
' process.kill(parsed, 0)',
|
||||
' return true',
|
||||
' } catch (err: unknown) {',
|
||||
" return (err as { code?: string } | null)?.code !== 'ESRCH'",
|
||||
' }',
|
||||
'}',
|
||||
'',
|
||||
"// Why: child agents inherit the lead's pane env; only its process may",
|
||||
'// register status hooks. PID identity keeps in-process reloads reporting.',
|
||||
'export default function (pi): void {',
|
||||
...primeDaemonWorkerGuard,
|
||||
` const ownerPid = process.env.${ownerEnv}`,
|
||||
' const selfPid = String(process.pid)',
|
||||
' if (ownerPid && ownerPid !== selfPid) return',
|
||||
' if (ownerPid && ownerPid !== selfPid && isStatusOwnerAlive(ownerPid)) return',
|
||||
` process.env.${ownerEnv} = selfPid`,
|
||||
...sessionStartHandler,
|
||||
` pi.on('before_agent_start', (event${ctxParam}) => {`,
|
||||
|
||||
@@ -0,0 +1,89 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
createAgentStatusExtensionHarness as createHarness,
|
||||
AGENT_STATUS_EXTENSION_SELF_PID as SELF_PID
|
||||
} from './agent-status-extension-test-harness'
|
||||
|
||||
describe('Pi status owner recovery', () => {
|
||||
it.each(['pi', 'omp', 'prime-agent'] as const)(
|
||||
'claims the pane for a restarted %s agent whose inherited owner PID is dead',
|
||||
async (kind) => {
|
||||
// Why: STA-5245 -- a restart leaves a dead owner PID in the inherited env.
|
||||
// Without a liveness probe the guard suppresses every later load, so the
|
||||
// pane never reports status again.
|
||||
const ownerKey =
|
||||
kind === 'prime-agent' ? 'ORCA_PRIME_AGENT_STATUS_OWNED' : 'ORCA_PI_STATUS_OWNED'
|
||||
const harness = createHarness({
|
||||
kind,
|
||||
pid: SELF_PID,
|
||||
env: { [ownerKey]: String(SELF_PID - 1) },
|
||||
killImpl: () => {
|
||||
throw Object.assign(new Error('ESRCH'), { code: 'ESRCH' })
|
||||
}
|
||||
})
|
||||
|
||||
expect(harness.killMock).toHaveBeenCalledWith(SELF_PID - 1, 0)
|
||||
expect(harness.handlers.agent_end).toBeTypeOf('function')
|
||||
expect(harness.processEnv[ownerKey]).toBe(String(SELF_PID))
|
||||
|
||||
await harness.callHook('agent_end')
|
||||
expect(harness.fetchMock).toHaveBeenCalledTimes(1)
|
||||
}
|
||||
)
|
||||
|
||||
it.each(['EPERM', 'EACCES', 'EINVAL', undefined])(
|
||||
'keeps suppression for unverifiable probe error %s',
|
||||
(code) => {
|
||||
// Why: EPERM means the owner exists but belongs to another user, so
|
||||
// claiming the pane there would reintroduce double-reporting.
|
||||
const harness = createHarness({
|
||||
kind: 'pi',
|
||||
pid: SELF_PID,
|
||||
env: { ORCA_PI_STATUS_OWNED: String(SELF_PID - 1) },
|
||||
killImpl: () => {
|
||||
throw Object.assign(new Error('probe failed'), { code })
|
||||
}
|
||||
})
|
||||
|
||||
expect(harness.handlers).toEqual({})
|
||||
expect(harness.processEnv.ORCA_PI_STATUS_OWNED).toBe(String(SELF_PID - 1))
|
||||
}
|
||||
)
|
||||
|
||||
it('claims the pane when the inherited owner PID is not a usable pid', () => {
|
||||
// Why: a truncated/garbage marker is not evidence of a live owner.
|
||||
const harness = createHarness({
|
||||
kind: 'pi',
|
||||
pid: SELF_PID,
|
||||
env: { ORCA_PI_STATUS_OWNED: 'not-a-pid' }
|
||||
})
|
||||
|
||||
expect(harness.killMock).not.toHaveBeenCalled()
|
||||
expect(harness.handlers.agent_end).toBeTypeOf('function')
|
||||
expect(harness.processEnv.ORCA_PI_STATUS_OWNED).toBe(String(SELF_PID))
|
||||
})
|
||||
|
||||
it('claims the pane when the inherited owner PID exceeds safe integer precision', () => {
|
||||
const harness = createHarness({
|
||||
kind: 'pi',
|
||||
pid: SELF_PID,
|
||||
env: { ORCA_PI_STATUS_OWNED: '99999999999999999999999' }
|
||||
})
|
||||
|
||||
expect(harness.killMock).not.toHaveBeenCalled()
|
||||
expect(harness.handlers.agent_end).toBeTypeOf('function')
|
||||
expect(harness.processEnv.ORCA_PI_STATUS_OWNED).toBe(String(SELF_PID))
|
||||
})
|
||||
|
||||
it('claims the pane when the inherited owner PID exceeds the process API range', () => {
|
||||
const harness = createHarness({
|
||||
kind: 'pi',
|
||||
pid: SELF_PID,
|
||||
env: { ORCA_PI_STATUS_OWNED: String(2 ** 31) }
|
||||
})
|
||||
|
||||
expect(harness.killMock).not.toHaveBeenCalled()
|
||||
expect(harness.handlers.agent_end).toBeTypeOf('function')
|
||||
expect(harness.processEnv.ORCA_PI_STATUS_OWNED).toBe(String(SELF_PID))
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,128 @@
|
||||
// Run: node tests/tools/pi-owner-runtime-smoke.mjs /path/to/pi-coding-agent
|
||||
import assert from 'node:assert/strict'
|
||||
import { once } from 'node:events'
|
||||
import { mkdtemp, writeFile, rm } from 'node:fs/promises'
|
||||
import { createServer } from 'node:http'
|
||||
import { createRequire } from 'node:module'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join, resolve } from 'node:path'
|
||||
import { pathToFileURL } from 'node:url'
|
||||
import { build } from 'esbuild'
|
||||
|
||||
const piRoot = resolve(process.argv[2] || '')
|
||||
assert.ok(process.argv[2], 'Pass an installed pi-coding-agent package directory')
|
||||
const scratch = await mkdtemp(join(tmpdir(), 'orca-pi-owner-'))
|
||||
const received = []
|
||||
const server = createServer(async (request, response) => {
|
||||
let body = ''
|
||||
for await (const chunk of request) {
|
||||
body += chunk
|
||||
}
|
||||
received.push(JSON.parse(body))
|
||||
response.end('{}')
|
||||
})
|
||||
try {
|
||||
const bundle = join(scratch, 'orca.cjs')
|
||||
await build({
|
||||
stdin: {
|
||||
contents: [
|
||||
"export { getPiAgentStatusExtensionSource } from './src/main/pi/agent-status-extension-source';",
|
||||
"export { runProcess } from './src/shared/child-process/run-process';"
|
||||
].join('\n'),
|
||||
resolveDir: process.cwd()
|
||||
},
|
||||
bundle: true,
|
||||
platform: 'node',
|
||||
format: 'cjs',
|
||||
outfile: bundle,
|
||||
packages: 'external'
|
||||
})
|
||||
const { getPiAgentStatusExtensionSource, runProcess } = createRequire(import.meta.url)(bundle)
|
||||
server.listen(0, '127.0.0.1')
|
||||
await once(server, 'listening')
|
||||
const dead = await runProcess({
|
||||
program: process.execPath,
|
||||
args: ['-e', 'console.log(process.pid)']
|
||||
})
|
||||
assert.equal(dead.code, 0)
|
||||
const deadPid = Number(dead.stdout.trim())
|
||||
assert.throws(() => process.kill(deadPid, 0), { code: 'ESRCH' })
|
||||
const worker = join(scratch, 'worker.mjs')
|
||||
const moduleUrl = (file) => JSON.stringify(pathToFileURL(join(piRoot, file)).href)
|
||||
await writeFile(
|
||||
worker,
|
||||
`
|
||||
import assert from 'node:assert/strict'
|
||||
import { loadExtensions } from ${moduleUrl('dist/core/extensions/loader.js')}
|
||||
import { ExtensionRunner } from ${moduleUrl('dist/core/extensions/runner.js')}
|
||||
import { SessionManager } from ${moduleUrl('dist/core/session-manager.js')}
|
||||
const loaded = await loadExtensions([process.argv[2]], process.cwd())
|
||||
assert.deepEqual(loaded.errors, [])
|
||||
const runner = new ExtensionRunner(loaded.extensions, loaded.runtime, process.cwd(), SessionManager.inMemory(process.cwd()), undefined)
|
||||
const errors = []
|
||||
runner.onError(error => errors.push(error))
|
||||
await runner.emit({ type: 'agent_start' })
|
||||
await new Promise(resolve => setTimeout(resolve, 250))
|
||||
assert.deepEqual(errors, [])
|
||||
console.log(JSON.stringify({pid: process.pid, owner: process.env[process.argv[3]], handlers: loaded.extensions[0].handlers.size}))
|
||||
`
|
||||
)
|
||||
const results = []
|
||||
for (const kind of ['pi', 'omp', 'prime-agent']) {
|
||||
const ownerKey =
|
||||
kind === 'prime-agent' ? 'ORCA_PRIME_AGENT_STATUS_OWNED' : 'ORCA_PI_STATUS_OWNED'
|
||||
for (const scenario of ['baseline-dead', 'fixed-dead', 'fixed-live']) {
|
||||
let source = getPiAgentStatusExtensionSource(kind)
|
||||
if (scenario === 'baseline-dead') {
|
||||
const guard = 'if (ownerPid && ownerPid !== selfPid && isStatusOwnerAlive(ownerPid)) return'
|
||||
assert.ok(
|
||||
source.includes(guard),
|
||||
'Baseline mutation must replace the actual ownership guard'
|
||||
)
|
||||
source = source.replace(guard, 'if (ownerPid && ownerPid !== selfPid) return')
|
||||
}
|
||||
const extension = join(scratch, `${kind}-${scenario}.ts`)
|
||||
await writeFile(extension, source)
|
||||
const before = received.length
|
||||
const owner = scenario === 'fixed-live' ? process.pid : deadPid
|
||||
const child = await runProcess({
|
||||
program: process.execPath,
|
||||
args: [worker, extension, ownerKey],
|
||||
cwd: scratch,
|
||||
env: {
|
||||
...process.env,
|
||||
ORCA_BACKGROUND_LAUNCH: '1',
|
||||
ORCA_PANE_KEY: 'owner-proof',
|
||||
ORCA_AGENT_HOOK_PORT: String(server.address().port),
|
||||
ORCA_AGENT_HOOK_TOKEN: 'isolated-proof-token',
|
||||
ORCA_AGENT_HOOK_ENV: 'proof',
|
||||
ORCA_AGENT_HOOK_ENDPOINT: '',
|
||||
ORCA_PI_STATUS_OWNED: '',
|
||||
ORCA_PRIME_AGENT_STATUS_OWNED: '',
|
||||
PRIME_AGENT_INTERNAL_DAEMON_WORKER: kind === 'prime-agent' ? '1' : '',
|
||||
[ownerKey]: String(owner)
|
||||
},
|
||||
timeoutMs: 15000
|
||||
})
|
||||
assert.equal(child.code, 0, child.stderr)
|
||||
const observation = JSON.parse(child.stdout.trim().split('\n').at(-1))
|
||||
const shouldReport = scenario === 'fixed-dead'
|
||||
assert.equal(
|
||||
received.length - before,
|
||||
shouldReport ? 1 : 0,
|
||||
`${kind}/${scenario}: HTTP delivery`
|
||||
)
|
||||
assert.equal(observation.owner, String(shouldReport ? observation.pid : owner))
|
||||
assert.equal(observation.handlers > 0, shouldReport)
|
||||
if (shouldReport) {
|
||||
assert.equal(received.at(-1).payload.hook_event_name, 'agent_start')
|
||||
}
|
||||
results.push({ kind, scenario, posts: received.length - before, ...observation })
|
||||
}
|
||||
}
|
||||
console.log(JSON.stringify({ platform: process.platform, results }, null, 2))
|
||||
} finally {
|
||||
server.closeAllConnections()
|
||||
server.close()
|
||||
await rm(scratch, { recursive: true, force: true })
|
||||
}
|
||||
Reference in New Issue
Block a user