Merge PR1 review fixes into Claude provider
# Conflicts: # vitest.config.ts
This commit is contained in:
@@ -90,41 +90,15 @@ export const DEFAULT_DISPOSE_EOF_GRACE_MS = 6_000
|
|||||||
/** Default POSIX grace between SIGTERM and SIGKILL on dispose (the `disposeGraceMs` config). */
|
/** Default POSIX grace between SIGTERM and SIGKILL on dispose (the `disposeGraceMs` config). */
|
||||||
export const DEFAULT_DISPOSE_GRACE_MS = 3_000
|
export const DEFAULT_DISPOSE_GRACE_MS = 3_000
|
||||||
|
|
||||||
/** Largest delay Node schedules without collapsing it to one millisecond. */
|
/** Bounded whole-tree exit wait: polls the handle's tree liveness until it exits or `ms` elapses. */
|
||||||
const MAX_TIMER_DELAY_MS = 2_147_483_647n
|
async function treeExitsWithin(child: SubprocessHandle, ms: number): Promise<boolean> {
|
||||||
|
const controller = new AbortController()
|
||||||
function scaledFiniteMilliseconds(ms: number, scale: number): bigint {
|
const timer = setTimeout(() => { controller.abort() }, ms)
|
||||||
const whole = Math.floor(ms)
|
try {
|
||||||
return BigInt(whole) * BigInt(scale)
|
return await child.waitForExit(controller.signal)
|
||||||
+ BigInt(Math.ceil((ms - whole) * scale))
|
} finally {
|
||||||
}
|
clearTimeout(timer)
|
||||||
|
|
||||||
/**
|
|
||||||
* Bounded whole-tree exit wait across Node-safe timer segments.
|
|
||||||
* @param child - process tree whose liveness is authoritative.
|
|
||||||
* @param ms - positive finite base window in milliseconds.
|
|
||||||
* @param scale - integer multiplier applied without Number overflow.
|
|
||||||
*/
|
|
||||||
async function treeExitsWithin(
|
|
||||||
child: SubprocessHandle,
|
|
||||||
ms: number,
|
|
||||||
scale = 1,
|
|
||||||
): Promise<boolean> {
|
|
||||||
let remaining = scaledFiniteMilliseconds(ms, scale)
|
|
||||||
while (remaining > 0n) {
|
|
||||||
const chunk = remaining > MAX_TIMER_DELAY_MS
|
|
||||||
? MAX_TIMER_DELAY_MS
|
|
||||||
: remaining
|
|
||||||
remaining -= chunk
|
|
||||||
const controller = new AbortController()
|
|
||||||
const timer = setTimeout(() => { controller.abort() }, Number(chunk))
|
|
||||||
try {
|
|
||||||
if (await child.waitForExit(controller.signal)) return true
|
|
||||||
} finally {
|
|
||||||
clearTimeout(timer)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
return false
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -151,7 +125,7 @@ export async function disposeAcpChild(child: SubprocessHandle, eofGraceMs: numbe
|
|||||||
// (this plugin passes disposeGraceMs there), so the bound covers both the
|
// (this plugin passes disposeGraceMs there), so the bound covers both the
|
||||||
// escalation window and an equal confirmation window after the SIGKILL.
|
// escalation window and an equal confirmation window after the SIGKILL.
|
||||||
child.terminate()
|
child.terminate()
|
||||||
if (!(await treeExitsWithin(child, graceMs, 2))) {
|
if (!(await treeExitsWithin(child, graceMs * 2))) {
|
||||||
throw new Error('ACP child process tree did not exit within its dispose windows')
|
throw new Error('ACP child process tree did not exit within its dispose windows')
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { describe, expect, it, vi } from 'vitest'
|
import { describe, expect, it } from 'vitest'
|
||||||
import { Context } from 'cordis'
|
import { Context } from 'cordis'
|
||||||
import Loader from '@cordisjs/plugin-loader'
|
import Loader from '@cordisjs/plugin-loader'
|
||||||
import { chmodSync, existsSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'
|
import { chmodSync, existsSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'
|
||||||
@@ -190,72 +190,6 @@ describe('disposeAcpChild (the backend-owned teardown ladder over seam verbs)',
|
|||||||
await expect(disposeAcpChild(never, 20, 20)).rejects.toThrow(/did not exit within its dispose windows/)
|
await expect(disposeAcpChild(never, 20, 20)).rejects.toThrow(/did not exit within its dispose windows/)
|
||||||
})
|
})
|
||||||
|
|
||||||
it('keeps an oversized finite escalation window instead of collapsing it to one millisecond', async () => {
|
|
||||||
vi.useFakeTimers()
|
|
||||||
try {
|
|
||||||
let waitCount = 0
|
|
||||||
let reportExited!: (exited: boolean) => void
|
|
||||||
const terminate = vi.fn()
|
|
||||||
const waitForExit = vi.fn((signal?: AbortSignal) => {
|
|
||||||
waitCount += 1
|
|
||||||
return new Promise<boolean>((resolve) => {
|
|
||||||
signal?.addEventListener('abort', () => { resolve(false) }, { once: true })
|
|
||||||
if (waitCount === 2) reportExited = resolve
|
|
||||||
})
|
|
||||||
})
|
|
||||||
const child: Parameters<typeof disposeAcpChild>[0] = {
|
|
||||||
pid: 1,
|
|
||||||
stdin: undefined,
|
|
||||||
stdout: undefined,
|
|
||||||
stderr: undefined,
|
|
||||||
collected: {},
|
|
||||||
done: new Promise(() => {}),
|
|
||||||
terminate,
|
|
||||||
waitForExit,
|
|
||||||
}
|
|
||||||
const disposal = disposeAcpChild(child, 0.25, Number.MAX_VALUE)
|
|
||||||
await vi.advanceTimersByTimeAsync(1)
|
|
||||||
expect(terminate).toHaveBeenCalledOnce()
|
|
||||||
expect(waitForExit).toHaveBeenCalledTimes(2)
|
|
||||||
const escalationSignal = waitForExit.mock.calls[1]?.[0]
|
|
||||||
await vi.advanceTimersByTimeAsync(1)
|
|
||||||
expect(escalationSignal?.aborted).toBe(false)
|
|
||||||
reportExited(true)
|
|
||||||
await expect(disposal).resolves.toBeUndefined()
|
|
||||||
expect(vi.getTimerCount()).toBe(0)
|
|
||||||
} finally {
|
|
||||||
vi.useRealTimers()
|
|
||||||
}
|
|
||||||
})
|
|
||||||
|
|
||||||
it('chains a doubled grace beyond one Node timer segment', async () => {
|
|
||||||
vi.useFakeTimers()
|
|
||||||
try {
|
|
||||||
const waitForExit = vi.fn((signal?: AbortSignal) => new Promise<boolean>((resolve) => {
|
|
||||||
signal?.addEventListener('abort', () => { resolve(false) }, { once: true })
|
|
||||||
}))
|
|
||||||
const child: Parameters<typeof disposeAcpChild>[0] = {
|
|
||||||
pid: 1,
|
|
||||||
stdin: undefined,
|
|
||||||
stdout: undefined,
|
|
||||||
stderr: undefined,
|
|
||||||
collected: {},
|
|
||||||
done: new Promise(() => {}),
|
|
||||||
terminate: vi.fn(),
|
|
||||||
waitForExit,
|
|
||||||
}
|
|
||||||
const disposal = disposeAcpChild(child, 0.25, 1_073_741_823.75)
|
|
||||||
const rejected = expect(disposal).rejects.toThrow(/did not exit within its dispose windows/)
|
|
||||||
await vi.advanceTimersByTimeAsync(1)
|
|
||||||
await vi.advanceTimersByTimeAsync(2_147_483_647)
|
|
||||||
expect(waitForExit).toHaveBeenCalledTimes(3)
|
|
||||||
await vi.advanceTimersByTimeAsync(1)
|
|
||||||
await rejected
|
|
||||||
} finally {
|
|
||||||
vi.useRealTimers()
|
|
||||||
}
|
|
||||||
})
|
|
||||||
|
|
||||||
it('observes a spawn-level rejection and returns without a process to reap', async () => {
|
it('observes a spawn-level rejection and returns without a process to reap', async () => {
|
||||||
const child = spawnSubprocess({
|
const child = spawnSubprocess({
|
||||||
argv: ['bash', '-c', 'true'],
|
argv: ['bash', '-c', 'true'],
|
||||||
|
|||||||
@@ -170,7 +170,7 @@ describe('real @openai/codex 0.146.0 product', () => {
|
|||||||
expect(recorded.headers.authorization).toBe('Bearer dsh-fake-openai-key')
|
expect(recorded.headers.authorization).toBe('Bearer dsh-fake-openai-key')
|
||||||
expect(responseInputTexts(recorded.body)).toContain(task)
|
expect(responseInputTexts(recorded.body)).toContain(task)
|
||||||
await expectQuiescent(harness.handles)
|
await expectQuiescent(harness.handles)
|
||||||
}, 20_000)
|
}, 60_000)
|
||||||
|
|
||||||
it('cancels a real app-server command approval without executing the command', async () => {
|
it('cancels a real app-server command approval without executing the command', async () => {
|
||||||
const { harness, fixture } = await realHarness([
|
const { harness, fixture } = await realHarness([
|
||||||
@@ -206,7 +206,7 @@ describe('real @openai/codex 0.146.0 product', () => {
|
|||||||
requestEntry.headers.authorization === 'Bearer dsh-fake-openai-key',
|
requestEntry.headers.authorization === 'Bearer dsh-fake-openai-key',
|
||||||
)).toBe(true)
|
)).toBe(true)
|
||||||
await expectQuiescent(harness.handles)
|
await expectQuiescent(harness.handles)
|
||||||
}, 20_000)
|
}, 60_000)
|
||||||
|
|
||||||
it('settles cancellation locally and leaves the real app-server tree quiescent', async () => {
|
it('settles cancellation locally and leaves the real app-server tree quiescent', async () => {
|
||||||
const { harness, fixture } = await realHarness([{ kind: 'hold' }])
|
const { harness, fixture } = await realHarness([{ kind: 'hold' }])
|
||||||
@@ -221,5 +221,5 @@ describe('real @openai/codex 0.146.0 product', () => {
|
|||||||
await expect(run.result).resolves.toMatchObject({ stopReason: 'aborted' })
|
await expect(run.result).resolves.toMatchObject({ stopReason: 'aborted' })
|
||||||
await run.dispose()
|
await run.dispose()
|
||||||
await expectQuiescent(harness.handles)
|
await expectQuiescent(harness.handles)
|
||||||
}, 20_000)
|
}, 60_000)
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -54,8 +54,6 @@ const coverageExemptExcludes = coverageExemptRaw === '1'
|
|||||||
// Keep the narrow exception in forks while the rest of the inventory avoids per-file processes.
|
// Keep the narrow exception in forks while the rest of the inventory avoids per-file processes.
|
||||||
const processBoundTests = [
|
const processBoundTests = [
|
||||||
'packages/subprocess/subprocess-local/tests/spawn.spec.ts',
|
'packages/subprocess/subprocess-local/tests/spawn.spec.ts',
|
||||||
'packages/subagent/subagent-claude-code/tests/real-product.spec.ts',
|
|
||||||
'packages/subagent/subagent-codex/tests/real-product.spec.ts',
|
|
||||||
'packages/context/time-context/tests/time-context.spec.ts',
|
'packages/context/time-context/tests/time-context.spec.ts',
|
||||||
'packages/llm/llm-pi-ai/tests/adapter.spec.ts',
|
'packages/llm/llm-pi-ai/tests/adapter.spec.ts',
|
||||||
'packages/ui/app-boot/tests/app-boot.spec.ts',
|
'packages/ui/app-boot/tests/app-boot.spec.ts',
|
||||||
|
|||||||
Reference in New Issue
Block a user