diff --git a/src/main/codex/codex-subagent-activity.ts b/src/main/codex/codex-subagent-activity.ts index 09e8272be1d..6bedd723b1d 100644 --- a/src/main/codex/codex-subagent-activity.ts +++ b/src/main/codex/codex-subagent-activity.ts @@ -19,6 +19,9 @@ import { readCodexCollabAgentToolCall } from './codex-collab-agent-tool-call' import type { CodexThreadItem } from './codex-thread-item-identity' +import { codexSubagentPathLabel, codexSubagentPathSegments } from '../../shared/codex-agent-path' + +export { codexSubagentPathSegments } from '../../shared/codex-agent-path' export const CODEX_SUBAGENT_ITEM_TYPE = 'subAgentActivity' export const CODEX_TOKEN_USAGE_METHOD = 'thread/tokenUsage/updated' @@ -54,11 +57,6 @@ export function readCodexSubagentActivity(item: CodexThreadItem): CodexSubagentA } } -/** Path segments, empty ones dropped: `/root/list_directory` → 2 segments. */ -export function codexSubagentPathSegments(agentPath: string | null): string[] { - return agentPath === null ? [] : agentPath.split('/').filter((part) => part.length > 0) -} - /** The one path segment that names the parent turn itself rather than a child. * Compared after the same normalization the label uses, not against the raw * string: `/root/` and `/root//` are the same node as `/root`, and a check that @@ -88,8 +86,7 @@ export function isCodexRootAgentActivity(activity: CodexSubagentActivity): boole * because the caller keys its collision ordinals on this string: ` read ` and * `read` render identically and must therefore collide. */ export function codexSubagentLabel(activity: CodexSubagentActivity): string | null { - const trailing = codexSubagentPathSegments(activity.agentPath).at(-1)?.trim() - return trailing !== undefined && trailing.length > 0 ? trailing : null + return codexSubagentPathLabel(activity.agentPath) } /** A child the item says exists, from either wire shape Codex announces one with. */ diff --git a/src/shared/agent-child-row-model.ts b/src/shared/agent-child-row-model.ts index c0dcfed2d04..8ff7eaf5ccd 100644 --- a/src/shared/agent-child-row-model.ts +++ b/src/shared/agent-child-row-model.ts @@ -143,13 +143,15 @@ type AgentChildRowDetailSource = { settled: boolean } -// A role that restates the name adds nothing. +// A default role or one that restates the name adds nothing. function roleDetail(source: AgentChildRowDetailSource): AgentChildRowDetail | null { if (source.kind !== 'agent') { return null } const agentType = source.agentType ?? 'unknown' - return source.name.trim() === agentType.trim() ? null : { kind: 'role', agentType } + return agentType.trim() === 'default' || source.name.trim() === agentType.trim() + ? null + : { kind: 'role', agentType } } function messageOrRole(source: AgentChildRowDetailSource): AgentChildRowDetail | null { diff --git a/src/shared/agent-hook-listener-codex-subagent-names.test.ts b/src/shared/agent-hook-listener-codex-subagent-names.test.ts new file mode 100644 index 00000000000..ec587bbd68b --- /dev/null +++ b/src/shared/agent-hook-listener-codex-subagent-names.test.ts @@ -0,0 +1,89 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { appendFileSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { createHookListenerState } from './agent-hook-listener/listener-state' +import { normalizeAndAccept } from './agent-hook-listener-test-harness' + +describe('Codex child names from provider metadata', () => { + const directories: string[] = [] + + afterEach(() => { + for (const directory of directories.splice(0)) { + rmSync(directory, { recursive: true, force: true }) + } + }) + + function fixture() { + const directory = mkdtempSync(join(tmpdir(), 'codex-child-name-')) + directories.push(directory) + const transcriptPath = join(directory, 'rollout-child.jsonl') + const state = createHookListenerState() + return { + transcriptPath, + writeMeta(fields: Record, nested = true, id = 'child') { + const payload = nested + ? { id, source: { subagent: { thread_spawn: fields } } } + : { id, ...fields } + writeFileSync(transcriptPath, `${JSON.stringify({ type: 'session_meta', payload })}\n`) + }, + post(hookEventName = 'SubagentStart') { + return normalizeAndAccept(state, 'codex', { + hook_event_name: hookEventName, + agent_id: 'child', + agent_type: 'default', + transcript_path: transcriptPath + })?.payload.subagents + } + } + } + + it('names a hook-only child, retains its name across waits and clears it on stop', () => { + const { writeMeta, post, transcriptPath } = fixture() + writeMeta({ agent_path: '/root/adversarial_visual', agent_nickname: 'Scout' }) + appendFileSync(transcriptPath, 'x'.repeat(2 * 1024 * 1024)) + + expect(post()).toEqual([ + expect.objectContaining({ + id: 'child', + agentType: 'default', + description: 'adversarial_visual', + state: 'working' + }) + ]) + rmSync(transcriptPath) + expect(post('PermissionRequest')?.[0]).toMatchObject({ + description: 'adversarial_visual', + state: 'waiting' + }) + expect(post('PostToolUse')?.[0]).toMatchObject({ + description: 'adversarial_visual', + state: 'working' + }) + expect(post('SubagentStop')).toBeUndefined() + }) + + it('recovers a child first seen in a tool hook and retries metadata that was not yet written', () => { + const { writeMeta, post } = fixture() + expect(post('PostToolUse')?.[0]?.description).toBeUndefined() + writeMeta({ agent_path: '/root/review/mobile' }) + expect(post('PostToolUse')?.[0]?.description).toBe('mobile') + }) + + it('uses top-level metadata and falls back to a nickname on older releases', () => { + const { writeMeta, post } = fixture() + writeMeta({ agent_path: ' ', agent_nickname: 'Scout' }, false) + expect(post()?.[0]?.description).toBe('Scout') + }) + + it('keeps the role fallback when metadata belongs to another thread or has no name', () => { + const { writeMeta, post } = fixture() + writeMeta({ agent_path: '/root/wrong-child' }, true, 'another-child') + expect(post()?.[0]).toMatchObject({ agentType: 'default', description: undefined }) + writeMeta({ agent_role: 'default' }) + expect(post('PostToolUse')?.[0]).toMatchObject({ + agentType: 'default', + description: undefined + }) + }) +}) diff --git a/src/shared/agent-hook-listener/providers/codex-events.ts b/src/shared/agent-hook-listener/providers/codex-events.ts index aa2f0f08e52..c15ed5496af 100644 --- a/src/shared/agent-hook-listener/providers/codex-events.ts +++ b/src/shared/agent-hook-listener/providers/codex-events.ts @@ -11,6 +11,7 @@ import { } from '../../agent-lead-status-fold' import { codexRosterToSnapshots, + enrichCodexSubagentIdentity, finishCodexSubagent, upsertCodexSubagent } from '../../codex-subagent-roster' @@ -107,6 +108,11 @@ export function normalizeCodexSubagentLifecycleEvent( }, Date.now() ) + enrichCodexSubagentIdentity( + roster, + agentId, + readFirstString(hookPayload, ['transcript_path', 'transcriptPath']) + ) } else { finishCodexSubagent(roster, agentId) } @@ -237,6 +243,11 @@ export function normalizeCodexEvent( }, Date.now() ) + enrichCodexSubagentIdentity( + getOrCreateCodexSubagentRoster(state, paneKey), + agentId, + transcriptPath + ) return buildCodexChildDrivenStatusPayload(state, eventName, paneKey, hookPayload) } diff --git a/src/shared/codex-agent-path.ts b/src/shared/codex-agent-path.ts new file mode 100644 index 00000000000..b1e56d63f3c --- /dev/null +++ b/src/shared/codex-agent-path.ts @@ -0,0 +1,8 @@ +export function codexSubagentPathSegments(agentPath: string | null): string[] { + return agentPath === null ? [] : agentPath.split('/').filter((part) => part.length > 0) +} + +export function codexSubagentPathLabel(agentPath: string | null): string | null { + const trailing = codexSubagentPathSegments(agentPath).at(-1)?.trim() + return trailing !== undefined && trailing.length > 0 ? trailing : null +} diff --git a/src/shared/codex-rollout-jsonl-cursor.test.ts b/src/shared/codex-rollout-jsonl-cursor.test.ts new file mode 100644 index 00000000000..9fb48fdf922 --- /dev/null +++ b/src/shared/codex-rollout-jsonl-cursor.test.ts @@ -0,0 +1,45 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { mkdtempSync, rmSync, symlinkSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' +import { runProcess } from '@orca/process-host' + +const directories: string[] = [] + +afterEach(() => { + for (const directory of directories.splice(0)) { + rmSync(directory, { recursive: true, force: true }) + } +}) + +describe('readJsonlFirstRecord', () => { + it.skipIf(process.platform === 'win32')( + 'rejects writer-less FIFOs and symlinks to them without blocking', + async () => { + const directory = mkdtempSync(join(tmpdir(), 'codex-rollout-fifo-')) + directories.push(directory) + const fifo = join(directory, 'pipe.jsonl') + const link = join(directory, 'link.jsonl') + const created = await runProcess({ program: 'mkfifo', args: [fifo], timeoutMs: 5000 }) + expect(created.code).toBe(0) + symlinkSync(fifo, link) + const readerPath = resolve('src/shared/codex-rollout-jsonl-cursor.ts') + const result = await runProcess({ + program: process.execPath, + args: [ + '--import', + 'tsx', + '--eval', + `const assert = require('node:assert/strict'); +const { readJsonlFirstRecord } = require(${JSON.stringify(readerPath)}); +for (const path of ${JSON.stringify([fifo, link, directory])}) { + assert.equal(readJsonlFirstRecord(path), undefined); +}` + ], + timeoutMs: 5000 + }) + expect(result.timedOut).toBe(false) + expect(result.code, result.stderr).toBe(0) + } + ) +}) diff --git a/src/shared/codex-rollout-jsonl-cursor.ts b/src/shared/codex-rollout-jsonl-cursor.ts index 57f68df3b54..0abf27a4f27 100644 --- a/src/shared/codex-rollout-jsonl-cursor.ts +++ b/src/shared/codex-rollout-jsonl-cursor.ts @@ -1,4 +1,13 @@ -import { closeSync, openSync, readSync, readdirSync, statSync, type Stats } from 'node:fs' +import { + closeSync, + constants, + fstatSync, + openSync, + readSync, + readdirSync, + statSync, + type Stats +} from 'node:fs' const TRANSCRIPT_READ_MAX_BYTES = 1024 * 1024 const TRANSCRIPT_LINE_MAX_BYTES = 256 * 1024 @@ -17,6 +26,33 @@ export function record(value: unknown): JsonRecord | undefined { return typeof value === 'object' && value !== null ? (value as JsonRecord) : undefined } +export function readJsonlFirstRecord(filePath: string): JsonRecord | undefined { + let fd: number | undefined + try { + // Nonblocking open prevents a FIFO from stalling hook handling before the descriptor check. + fd = openSync( + filePath, + constants.O_RDONLY | (process.platform === 'win32' ? 0 : constants.O_NONBLOCK) + ) + if (!fstatSync(fd).isFile()) { + return undefined + } + const buffer = Buffer.allocUnsafe(64 * 1024) + const bytesRead = readSync(fd, buffer, 0, buffer.length, 0) + const newline = buffer.subarray(0, bytesRead).indexOf(0x0a) + if (newline === -1) { + return undefined + } + return record(JSON.parse(buffer.toString('utf8', 0, newline))) + } catch { + return undefined + } finally { + if (fd !== undefined) { + closeSync(fd) + } + } +} + /** Returns undefined when the file is unreadable, distinguishing a vanished rollout from one with no new lines. * `lineFilter` skips JSON.parse for raw lines the caller can reject by substring. */ export function readJsonlCursor( diff --git a/src/shared/codex-subagent-identity.ts b/src/shared/codex-subagent-identity.ts new file mode 100644 index 00000000000..9e2a9df5e58 --- /dev/null +++ b/src/shared/codex-subagent-identity.ts @@ -0,0 +1,29 @@ +import { extname, isAbsolute } from 'node:path' +import { readJsonlFirstRecord, record } from './codex-rollout-jsonl-cursor' +import { codexSubagentPathLabel } from './codex-agent-path' + +function text(value: unknown): string | undefined { + return typeof value === 'string' ? value.trim() || undefined : undefined +} + +/** Immutable provider metadata names the child even when its parent emits no activity items. */ +export function readCodexSubagentIdentity( + transcriptPath: string, + agentId: string +): { description?: string } | undefined { + if (!isAbsolute(transcriptPath) || extname(transcriptPath) !== '.jsonl') { + return undefined + } + const head = readJsonlFirstRecord(transcriptPath) + const payload = record(head?.payload) + if (head?.type !== 'session_meta' || payload?.id !== agentId) { + return undefined + } + const spawn = record(record(record(payload.source)?.subagent)?.thread_spawn) + return { + description: + codexSubagentPathLabel(text(spawn?.agent_path) ?? text(payload.agent_path) ?? null) ?? + text(spawn?.agent_nickname) ?? + text(payload.agent_nickname) + } +} diff --git a/src/shared/codex-subagent-roster.ts b/src/shared/codex-subagent-roster.ts index 37f8d6f909e..eb7ab485a6e 100644 --- a/src/shared/codex-subagent-roster.ts +++ b/src/shared/codex-subagent-roster.ts @@ -5,6 +5,7 @@ import { type AgentSubagentSnapshot } from './agent-status-types' import { normalizeOptionalField } from './agent-status-field-normalization' +import { readCodexSubagentIdentity } from './codex-subagent-identity' import { agentChildWorkLiveness, type AgentChildWorkLiveness @@ -20,6 +21,7 @@ type TrackedCodexSubagent = { model?: string state: 'working' | 'waiting' startedAt: number + identityTranscriptPath?: string } export function upsertCodexSubagent( @@ -61,6 +63,25 @@ export function finishCodexSubagent(roster: CodexSubagentRoster, id: string): vo roster.delete(id.trim()) } +export function enrichCodexSubagentIdentity( + roster: CodexSubagentRoster, + id: string, + transcriptPath: string | undefined +): void { + const child = roster.get(id.trim()) + if (!child || !transcriptPath || child.identityTranscriptPath === transcriptPath) { + return + } + const identity = readCodexSubagentIdentity(transcriptPath, id.trim()) + if (!identity) { + return + } + child.identityTranscriptPath = transcriptPath + child.description = + normalizeOptionalField(identity.description, AGENT_STATUS_TOOL_INPUT_MAX_LENGTH) ?? + child.description +} + /** * Record the model a already-tracked child is running. Deliberately narrower * than `upsertCodexSubagent`: it never creates a roster entry and never touches