mirror of
https://github.com/deepseek-ai/deepseek-harness.git
synced 2026-09-14 04:01:35 +00:00
fix(subagent): stabilize Codex permission diagnostics
This commit is contained in:
@@ -154,21 +154,27 @@ export async function startCodexRun(
|
||||
spec.permissionMode,
|
||||
)
|
||||
const onStderr = (chunk: Buffer | string): void => {
|
||||
process.stderr.write(chunk)
|
||||
wire.observeStderr(chunk.toString())
|
||||
}
|
||||
const onStderrError = (): void => {
|
||||
// Stderr observation is auxiliary. JSON-RPC and child.done remain the
|
||||
// only terminal authorities if the diagnostic stream itself fails.
|
||||
}
|
||||
const onHostStderrError = (): void => {
|
||||
// Host stderr is an observation sink, not a child-run failure authority.
|
||||
}
|
||||
child.stderr?.on('data', onStderr)
|
||||
child.stderr?.on('error', onStderrError)
|
||||
process.stderr.on('error', onHostStderrError)
|
||||
child.stderr?.pipe(process.stderr, { end: false })
|
||||
const disposeProcess = async (): Promise<void> => {
|
||||
try {
|
||||
await disposeCodexChild(wire, child)
|
||||
} finally {
|
||||
child.stderr?.unpipe(process.stderr)
|
||||
child.stderr?.off('data', onStderr)
|
||||
child.stderr?.off('error', onStderrError)
|
||||
process.stderr.off('error', onHostStderrError)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -214,10 +220,17 @@ export async function startCodexRun(
|
||||
|
||||
const collectOutput = (): ContentBlock[] => wire.collectOutput()
|
||||
const result: Promise<SubagentResult> = settleRunResult({
|
||||
attempt: () => Promise.race([
|
||||
wire.runTurn(texts, runAbort.signal),
|
||||
processFailure,
|
||||
]),
|
||||
attempt: async () => {
|
||||
try {
|
||||
return await Promise.race([
|
||||
wire.runTurn(texts, runAbort.signal),
|
||||
processFailure,
|
||||
])
|
||||
} catch (error: unknown) {
|
||||
await new Promise<void>((resolve) => { setImmediate(resolve) })
|
||||
throw error
|
||||
}
|
||||
},
|
||||
collectOutput,
|
||||
collectDiagnostic: () => wire.collectDiagnostic(),
|
||||
cancelled: () => runAbort.signal.aborted,
|
||||
|
||||
@@ -47,6 +47,21 @@ const STDERR_SIGNATURE_TAIL_CHARS = Math.max(
|
||||
...STDERR_PERMISSION_SIGNATURES.map(signature => signature.text.length),
|
||||
) - 1
|
||||
|
||||
function stderrSignatureTail(value: string): string {
|
||||
for (
|
||||
let length = Math.min(STDERR_SIGNATURE_TAIL_CHARS, value.length)
|
||||
; length > 0
|
||||
; length -= 1
|
||||
) {
|
||||
const tail = value.slice(-length)
|
||||
if (STDERR_PERMISSION_SIGNATURES.some(signature =>
|
||||
tail.length < signature.text.length && signature.text.startsWith(tail))) {
|
||||
return tail
|
||||
}
|
||||
}
|
||||
return ''
|
||||
}
|
||||
|
||||
function object(value: unknown, label: string): JsonObject {
|
||||
if (value === null || typeof value !== 'object' || Array.isArray(value)) {
|
||||
throw new Error(`subagent-codex: app-server returned invalid ${label}`)
|
||||
@@ -318,7 +333,7 @@ export class CodexAppServerWire {
|
||||
if (latest !== undefined) {
|
||||
this.recordDiagnostic(latest.request, latest.decision, latest.reason)
|
||||
}
|
||||
this.stderrTail = observed.slice(-STDERR_SIGNATURE_TAIL_CHARS)
|
||||
this.stderrTail = stderrSignatureTail(observed)
|
||||
}
|
||||
|
||||
/** Detach JSON-RPC listeners and reject outstanding requests. Idempotent. */
|
||||
@@ -399,6 +414,26 @@ export class CodexAppServerWire {
|
||||
)
|
||||
}
|
||||
|
||||
private recordDeclinedItem(item: JsonObject): boolean {
|
||||
if (item.type === 'commandExecution' && item.status === 'declined') {
|
||||
this.recordDiagnostic(
|
||||
'command execution',
|
||||
'declined',
|
||||
'Codex declined the command under the selected permission mode',
|
||||
)
|
||||
return true
|
||||
}
|
||||
if (item.type === 'fileChange' && item.status === 'declined') {
|
||||
this.recordDiagnostic(
|
||||
'file change',
|
||||
'declined',
|
||||
'Codex declined the file change under the selected permission mode',
|
||||
)
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
private handleServerRequest(method: string, params: JsonObject): Promise<unknown> {
|
||||
try {
|
||||
switch (method) {
|
||||
@@ -475,28 +510,15 @@ export class CodexAppServerWire {
|
||||
if (this.turnId === undefined) {
|
||||
if (this.turnCompleted !== undefined) {
|
||||
this.observePendingTurnId(id)
|
||||
const item = object(params.item, 'item/completed item')
|
||||
if (this.recordDeclinedItem(item)) return
|
||||
this.earlyTurnNotifications.push({ method, params })
|
||||
}
|
||||
return
|
||||
}
|
||||
if (id !== this.turnId) return
|
||||
const item = object(params.item, 'item/completed item')
|
||||
if (item.type === 'commandExecution' && item.status === 'declined') {
|
||||
this.recordDiagnostic(
|
||||
'command execution',
|
||||
'declined',
|
||||
'Codex declined the command under the selected permission mode',
|
||||
)
|
||||
return
|
||||
}
|
||||
if (item.type === 'fileChange' && item.status === 'declined') {
|
||||
this.recordDiagnostic(
|
||||
'file change',
|
||||
'declined',
|
||||
'Codex declined the file change under the selected permission mode',
|
||||
)
|
||||
return
|
||||
}
|
||||
if (this.recordDeclinedItem(item)) return
|
||||
if (item.type !== 'agentMessage') return
|
||||
const text = typeof item.text === 'string'
|
||||
? item.text
|
||||
|
||||
@@ -868,7 +868,7 @@ describe('CodexAppServerWire', () => {
|
||||
|
||||
it('does not reapply an old stderr signature after a newer request diagnostic', async () => {
|
||||
const { child, wire } = await initializeWire()
|
||||
wire.observeStderr('approval policy is Never; reject command')
|
||||
wire.observeStderr('recorded sandbox violation:')
|
||||
const result = wire.runTurn(['task'], new AbortController().signal)
|
||||
const turnStart = await child.peer.nextMethod('turn/start')
|
||||
child.peer.respond(turnStart, { turn: { id: 'turn-1' } })
|
||||
@@ -891,6 +891,36 @@ describe('CodexAppServerWire', () => {
|
||||
wire.close()
|
||||
})
|
||||
|
||||
it('keeps a newer request diagnostic after replaying an older early item', async () => {
|
||||
const { child, wire } = await initializeWire()
|
||||
const result = wire.runTurn(['task'], new AbortController().signal)
|
||||
const turnStart = await child.peer.nextMethod('turn/start')
|
||||
child.peer.send({
|
||||
method: 'item/completed',
|
||||
params: {
|
||||
threadId: 'thread-1',
|
||||
turnId: 'turn-1',
|
||||
item: { type: 'fileChange', status: 'declined' },
|
||||
},
|
||||
})
|
||||
await nextTask()
|
||||
child.peer.send({
|
||||
id: 'newer-command-request',
|
||||
method: 'item/commandExecution/requestApproval',
|
||||
params: {
|
||||
threadId: 'thread-1',
|
||||
turnId: 'turn-1',
|
||||
availableDecisions: ['cancel'],
|
||||
},
|
||||
})
|
||||
await child.peer.nextResponse('newer-command-request')
|
||||
child.peer.respond(turnStart, { turn: { id: 'turn-1' } })
|
||||
child.peer.send(agentMessage('answer', 'final_answer'), turnCompleted('completed'))
|
||||
await expect(result).resolves.toMatchObject({ stopReason: 'completed' })
|
||||
expect(wire.collectDiagnostic()).toContain('request: command approval')
|
||||
wire.close()
|
||||
})
|
||||
|
||||
it('fails the run on unknown requests or wrong request association', async () => {
|
||||
for (const serverRequest of [
|
||||
{
|
||||
@@ -1222,11 +1252,37 @@ describe('run lifecycle and quiescence', () => {
|
||||
await run.dispose()
|
||||
})
|
||||
|
||||
it('drains queued stderr before settling a failed published run', async () => {
|
||||
const write = vi.spyOn(process.stderr, 'write').mockImplementation(() => true)
|
||||
const { child, run, turnStart } = await publishRun()
|
||||
child.peer.respond(turnStart, { turn: { id: 'turn-1' } })
|
||||
child.peer.send(turnCompleted('failed', 'turn-1', 'thread-1', {
|
||||
message: 'fixture terminal failure',
|
||||
codexErrorInfo: 'badRequest',
|
||||
}))
|
||||
setImmediate(() => {
|
||||
child.stderr.write('approval policy is Never; reject command')
|
||||
})
|
||||
await expect(run.result).resolves.toEqual({
|
||||
output: [],
|
||||
diagnostic: 'Codex unattended decision (mode: never; request: command execution; decision: denied): Codex rejected an escalation because the selected policy never asks for approval',
|
||||
stopReason: 'error',
|
||||
})
|
||||
await run.dispose()
|
||||
write.mockRestore()
|
||||
})
|
||||
|
||||
it('forwards stderr while extracting only a fixed safe permission signature', async () => {
|
||||
const child = fakeChild()
|
||||
const forwarded: string[] = []
|
||||
let writes = 0
|
||||
const write = vi.spyOn(process.stderr, 'write').mockImplementation((chunk) => {
|
||||
forwarded.push(String(chunk))
|
||||
writes += 1
|
||||
if (writes === 1) {
|
||||
setImmediate(() => { process.stderr.emit('drain') })
|
||||
return false
|
||||
}
|
||||
return true
|
||||
})
|
||||
const { run, turnStart } = await publishRun(child)
|
||||
@@ -1243,11 +1299,28 @@ describe('run lifecycle and quiescence', () => {
|
||||
stopReason: 'error',
|
||||
})
|
||||
expect(forwarded.join('')).toContain('SECRET_TOKEN')
|
||||
expect(writes).toBe(2)
|
||||
await run.dispose()
|
||||
expect(child.stderr.listenerCount('data')).toBe(0)
|
||||
write.mockRestore()
|
||||
})
|
||||
|
||||
it('contains host stderr errors without changing run settlement', async () => {
|
||||
const child = fakeChild()
|
||||
const initialErrorListeners = process.stderr.listenerCount('error')
|
||||
const { run, turnStart } = await publishRun(child)
|
||||
child.peer.respond(turnStart, { turn: { id: 'turn-1' } })
|
||||
expect(process.stderr.listenerCount('error')).toBeGreaterThan(initialErrorListeners)
|
||||
process.stderr.emit('error', new Error('host stderr broke'))
|
||||
child.peer.send(agentMessage('answer', 'final_answer'), turnCompleted('completed'))
|
||||
await expect(run.result).resolves.toEqual({
|
||||
output: [{ type: 'text', text: 'answer' }],
|
||||
stopReason: 'completed',
|
||||
})
|
||||
await run.dispose()
|
||||
expect(process.stderr.listenerCount('error')).toBe(initialErrorListeners)
|
||||
})
|
||||
|
||||
it('rejects before spawn when pre-aborted and rolls back startup failures', async () => {
|
||||
const controller = new AbortController()
|
||||
controller.abort()
|
||||
|
||||
Reference in New Issue
Block a user