mirror of
https://github.com/deepseek-ai/deepseek-harness.git
synced 2026-08-29 04:26:38 +00:00
fix(cli): drain native handles on normal exit
This commit is contained in:
+2
-2
@@ -2,5 +2,5 @@
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-08-03-cli-signal-shutdown-escalation.md
|
||||
2026-08-03-cli-signal-shutdown-escalation.md: c8aac6e2be927bd1f4a445c00e0aaa870b10a465
|
||||
2026-08-03-cli-signal-shutdown-escalation.zh.md: 14e2149eb5153b3743e188c6b233d000b36d60db
|
||||
2026-08-03-cli-signal-shutdown-escalation.md: 55917400fac2728d13dc2cdd799a7e234b6ed661
|
||||
2026-08-03-cli-signal-shutdown-escalation.zh.md: c7897a8d77e8c2ebad43cec4e12170b04c837350
|
||||
|
||||
@@ -20,11 +20,13 @@ The fix has two ownership layers. The OTel backend adds `shutdownTimeoutMillis`
|
||||
|
||||
Web and headless share `createProcessShutdown`, one process-level controller around root disposal:
|
||||
|
||||
- Normal shutdown calls coalesce onto one disposal and retain the first requested exit code; they never escalate one another.
|
||||
- Normal shutdown calls coalesce onto one disposal and retain the first requested exit code; they never escalate one another. Successful disposal records that code through `process.exitCode` and lets Node drain its remaining handles naturally. Disposal failure still forces process exit because the launcher cannot assume the failed tree reached quiescence.
|
||||
- The first signal starts the same graceful disposal and a referenced five-second exit backstop. Disposal success or failure exits once; neither can cancel the process exit.
|
||||
- A signal received while shutdown is pending forces immediate exit with that signal path's code. This includes the first `Ctrl+C` after headless normal completion has already entered disposal, and a second signal after a signal initiated the drain.
|
||||
- The five-second bound is a process-safety invariant, not a deployment tunable. It is long enough for the telemetry deployment's ordinary drain ceiling while still bounding any wedged disposer at the launcher boundary.
|
||||
|
||||
Normal completion deliberately avoids `process.exit()`: an immediately forced exit after an Undici request can hit Node's [Windows libuv async-handle assertion](https://github.com/nodejs/node/issues/56645) before the completed request's native handle cleanup drains. A signal can still force exit after normal disposal has completed if another handle keeps the process alive.
|
||||
|
||||
Headless preserves exit 0 for a completed turn, exit 1 for another turn-end reason or API business error, 130 for SIGINT, and 143 for SIGTERM. Web preserves its existing SIGTERM exit 0 and SIGINT exit 130 behavior.
|
||||
|
||||
This supersedes the [telemetry deployment Note's](../feature/2026-07-31-web-telemetry-default-mount.md) assumption that SDK exporter/processor timeouts bound complete provider shutdown, and its earlier decision to defer a process-level backstop. The backend owns its export loss/latency policy and closes the known SDK `forceFlush()` gap; the launcher owns the outer guarantee that no plugin can trap the process indefinitely.
|
||||
@@ -37,15 +39,17 @@ This supersedes the [telemetry deployment Note's](../feature/2026-07-31-web-tele
|
||||
|
||||
**Add only the five-second timeout.** Rejected because a user pressing `Ctrl+C` again is asking to stop waiting now. Swallowing that intent for the rest of the grace period recreates the reported behavior at a shorter duration.
|
||||
|
||||
**Always call `process.exit()` after successful disposal.** Rejected because root disposal proves the application tree is quiescent, not that Node and its native dependencies have finished retiring every asynchronous handle. Setting `process.exitCode` preserves the requested status while letting the runtime finish that work.
|
||||
|
||||
## Consequences
|
||||
|
||||
A healthy exit still disposes the complete Cordis tree. The known telemetry wait releases after at most three seconds; any other wedged exit lasts at most five seconds without further input, and a repeated signal ends it immediately. Forced or deadline-bounded exit can interrupt telemetry export or remaining cleanup, which is intentional only after the graceful contract has failed or the user has explicitly escalated.
|
||||
A healthy normal exit still disposes the complete Cordis tree and then waits for Node's event loop to drain. The known telemetry wait releases after at most three seconds; any other wedged exit lasts at most five seconds without further input, and a signal ends a lingering normal completion or pending shutdown immediately. Forced or deadline-bounded exit can interrupt telemetry export or remaining cleanup, which is intentional only after the graceful contract has failed or the user has explicitly escalated.
|
||||
|
||||
The controller is launcher infrastructure rather than a Cordis plugin: it makes no claim that disposal completed, and it does not weaken the lifecycle rule that ordinary disposers must reach quiescence.
|
||||
|
||||
## Testing
|
||||
|
||||
`apps/cli/tests/process-shutdown.spec.ts` pins resolved and rejected disposal, the five-second backstop, normal-call coalescing, a signal interrupting normal disposal, and second-signal escalation.
|
||||
`apps/cli/tests/process-shutdown.spec.ts` pins natural completion after resolved disposal, forced exit after rejected disposal, the five-second backstop, normal-call coalescing, signal-owned disposal, a signal interrupting normal disposal or post-disposal handle draining, and second-signal escalation.
|
||||
|
||||
`apps/cli/tests/headless-shutdown.e2e.ts` boots the real shipped Web/headless Loader tree in a PTY with a test-only plugin whose disposer announces entry and never settles. The test sends SIGINT after the observation URL, waits for proof that disposal started, sends SIGINT again, and requires exit 130. The source/artifact launch resolver keeps the same regression on both execution planes. This PTY case covers the user-visible process state; no model-output snapshot changes.
|
||||
|
||||
|
||||
@@ -20,11 +20,13 @@ Status: implemented
|
||||
|
||||
Web 与 headless 共用 `createProcessShutdown`,它是围绕根级 dispose 建立的进程级控制器:
|
||||
|
||||
- 多次正常关闭调用会汇合到同一次 dispose,并保留首次请求的退出码;这些调用不会相互触发强制退出。
|
||||
- 多次正常关闭调用会汇合到同一次 dispose,并保留首次请求的退出码;这些调用不会相互触发强制退出。dispose 成功后,控制器通过 `process.exitCode` 记录该退出码,让 Node 自然排空剩余句柄;dispose 失败时仍强制退出,因为启动器不能假定失败的插件树已经完全停稳。
|
||||
- 第一个信号会启动同一次优雅 dispose,并设置一个带引用的 5 秒退出兜底。dispose 无论成功或失败都会触发且仅触发一次退出;任何一种结果都无法取消进程退出。
|
||||
- 关闭待结算期间收到信号时,会立即按该信号路径的退出码强制退出。这既包括 headless 正常完成已经进入 dispose 后收到的第一次 `Ctrl+C`,也包括由信号启动排空后收到的第二个信号。
|
||||
- 5 秒上限是进程安全不变式,而不是部署调节项。它足以覆盖遥测部署的常规排空时限,同时仍在启动器边界为任何卡死的 disposer 设置等待上限。
|
||||
|
||||
正常完成会刻意避免调用 `process.exit()`:Undici 请求刚完成后立即强制退出,可能会在原生句柄清理尚未排空时触发 Node 的 [Windows libuv 异步句柄断言](https://github.com/nodejs/node/issues/56645)。如果正常 dispose 已经完成,但仍有其他句柄让进程保持存活,信号依然可以强制退出。
|
||||
|
||||
headless 对完成的轮次仍以 0 退出,对其他轮次结束原因或 API 业务错误仍以 1 退出,对 SIGINT 以 130 退出,对 SIGTERM 以 143 退出。Web 保留现有行为:SIGTERM 以 0 退出,SIGINT 以 130 退出。
|
||||
|
||||
这项决策取代了[遥测部署 Agent Note](../feature/2026-07-31-web-telemetry-default-mount.md) 中 SDK 导出器/处理器超时能够限制提供方完整关闭流程的假设,也取代了其中暂缓进程级退出兜底的决定。后端负责导出数据丢失与延迟策略,并封住已知的 SDK `forceFlush()` 缺口;启动器负责最外层保证,确保任何插件都无法无限期困住进程。
|
||||
@@ -37,15 +39,17 @@ headless 对完成的轮次仍以 0 退出,对其他轮次结束原因或 API
|
||||
|
||||
**只增加 5 秒超时。** 不予采纳:用户再次按下 `Ctrl+C`,就是要求立即停止等待。若在剩余宽限期内继续吞掉这一意图,只是缩短了报告中故障的持续时间,并未解决问题。
|
||||
|
||||
**dispose 成功后仍总是调用 `process.exit()`。** 不予采纳:根级 dispose 只能证明应用插件树已经完全停稳,不能证明 Node 及其原生依赖已经回收所有异步句柄。设置 `process.exitCode` 既保留请求的状态码,也允许运行时完成这部分工作。
|
||||
|
||||
## 后果
|
||||
|
||||
健康的退出流程仍会对整棵 Cordis 插件树执行 dispose。已知的遥测等待最多会在 3 秒后解除;其他退出流程卡死时,如无进一步输入,最多等待 5 秒,再次收到信号则立即结束进程。强制退出或受截止时间限制的退出可能中断遥测导出或尚未完成的清理工作;只有优雅关闭约定已经失败,或用户明确要求强制退出时,才会有意接受这一结果。
|
||||
健康的正常退出流程仍会对整棵 Cordis 插件树执行 dispose,随后等待 Node 事件循环自然排空。已知的遥测等待最多会在 3 秒后解除;其他退出流程卡死时,如无进一步输入,最多等待 5 秒;收到信号时,仍在排空句柄的正常完成流程或待结算的关闭流程都会立即结束进程。强制退出或受截止时间限制的退出可能中断遥测导出或尚未完成的清理工作;只有优雅关闭约定已经失败,或用户明确要求强制退出时,才会有意接受这一结果。
|
||||
|
||||
该控制器属于启动器基础设施,而不是 Cordis 插件:它不会声称 dispose 已经完成,也不会削弱普通 disposer 必须达到完全停稳状态的生命周期规则。
|
||||
|
||||
## 测试
|
||||
|
||||
`apps/cli/tests/process-shutdown.spec.ts` 固定了 dispose 成功与失败、5 秒退出兜底、正常调用汇合、信号中断正常 dispose,以及第二次信号强制退出的行为。
|
||||
`apps/cli/tests/process-shutdown.spec.ts` 固定了 dispose 成功后的自然完成、dispose 失败后的强制退出、5 秒退出兜底、正常调用汇合、信号拥有的 dispose、信号中断正常 dispose 或 dispose 后句柄排空,以及第二次信号强制退出的行为。
|
||||
|
||||
`apps/cli/tests/headless-shutdown.e2e.ts` 在 PTY 中启动真实交付的 Web/headless Loader 插件树,并挂载一个仅用于测试的插件;该插件的 disposer 会声明已经进入清理流程,但永不结算。测试在观察地址出现后发送 SIGINT,等待 dispose 已启动的证据,再次发送 SIGINT,并要求进程以 130 退出。源码/产物启动解析器使两个执行平面都覆盖同一项回归。该 PTY 用例覆盖用户可见的进程状态;模型输出快照没有变化。
|
||||
|
||||
|
||||
@@ -5,54 +5,73 @@ export const PROCESS_SHUTDOWN_TIMEOUT_MS = 5_000
|
||||
|
||||
/** Process-exit controller shared by normal completion and Unix signal handlers. */
|
||||
export interface ProcessShutdown {
|
||||
/** Start or join graceful disposal before exiting with `code`. */
|
||||
/** Start or join graceful disposal before allowing natural completion with `code`. */
|
||||
shutdown(code: number): Promise<void>
|
||||
/** Start graceful disposal, or force exit when a shutdown is already running. */
|
||||
/** Start graceful disposal followed by exit, or force exit when shutdown is already running. */
|
||||
interrupt(code: number): void
|
||||
}
|
||||
|
||||
/**
|
||||
* Create one process-exit controller around an application disposer.
|
||||
* @param dispose - Whole-application teardown that resolves at quiescence.
|
||||
* @param exit - Process exit boundary, replaceable by tests.
|
||||
* @param forceExit - Forced process exit boundary, replaceable by tests.
|
||||
* @param complete - Natural process completion boundary, replaceable by tests.
|
||||
* @param timeoutMs - Grace before forced exit, replaceable by tests.
|
||||
* @returns A controller whose normal calls coalesce and whose repeated signal call escalates.
|
||||
*/
|
||||
export function createProcessShutdown(
|
||||
dispose: () => Promise<void>,
|
||||
exit: (code: number) => void = (code) => { process.exit(code) },
|
||||
forceExit: (code: number) => void = (code) => { process.exit(code) },
|
||||
complete: (code: number) => void = (code) => { process.exitCode = code },
|
||||
timeoutMs = PROCESS_SHUTDOWN_TIMEOUT_MS,
|
||||
): ProcessShutdown {
|
||||
let pending: Promise<void> | undefined
|
||||
let timeout: ReturnType<typeof setTimeout> | undefined
|
||||
let exited = false
|
||||
let completed = false
|
||||
let forceExited = false
|
||||
|
||||
const exitOnce = (code: number): void => {
|
||||
if (exited) return
|
||||
exited = true
|
||||
const clearExitTimeout = (): void => {
|
||||
/* v8 ignore else -- shutdown() arms the timer before any asynchronous exit path can run. */
|
||||
if (timeout !== undefined) clearTimeout(timeout)
|
||||
exit(code)
|
||||
}
|
||||
|
||||
const shutdown = (code: number): Promise<void> => {
|
||||
const forceExitOnce = (code: number): void => {
|
||||
if (forceExited) return
|
||||
forceExited = true
|
||||
clearExitTimeout()
|
||||
forceExit(code)
|
||||
}
|
||||
|
||||
const completeOnce = (code: number): void => {
|
||||
if (completed || forceExited) return
|
||||
completed = true
|
||||
clearExitTimeout()
|
||||
complete(code)
|
||||
}
|
||||
|
||||
const start = (code: number, forceAfterDispose: boolean): Promise<void> => {
|
||||
if (pending !== undefined) return pending
|
||||
timeout = setTimeout(() => { exitOnce(code) }, timeoutMs)
|
||||
timeout = setTimeout(() => { forceExitOnce(code) }, timeoutMs)
|
||||
pending = Promise.resolve().then(dispose).then(
|
||||
() => { exitOnce(code) },
|
||||
() => { exitOnce(code) },
|
||||
() => {
|
||||
if (forceAfterDispose) forceExitOnce(code)
|
||||
else completeOnce(code)
|
||||
},
|
||||
() => { forceExitOnce(code) },
|
||||
)
|
||||
return pending
|
||||
}
|
||||
|
||||
return {
|
||||
shutdown,
|
||||
shutdown(code) {
|
||||
return start(code, false)
|
||||
},
|
||||
interrupt(code) {
|
||||
if (pending !== undefined) {
|
||||
exitOnce(code)
|
||||
forceExitOnce(code)
|
||||
return
|
||||
}
|
||||
void shutdown(code)
|
||||
void start(code, true)
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -20,35 +20,50 @@ afterEach(() => {
|
||||
})
|
||||
|
||||
describe('process shutdown', () => {
|
||||
it('exits once after graceful disposal resolves or rejects', async () => {
|
||||
it('completes naturally after disposal resolves and forces exit when it rejects', async () => {
|
||||
const resolvedExit = vi.fn()
|
||||
const resolved = createProcessShutdown(() => Promise.resolve(), resolvedExit)
|
||||
const resolvedComplete = vi.fn()
|
||||
const resolved = createProcessShutdown(() => Promise.resolve(), resolvedExit, resolvedComplete)
|
||||
await resolved.shutdown(0)
|
||||
expect(resolvedExit).toHaveBeenCalledOnce()
|
||||
expect(resolvedExit).toHaveBeenCalledWith(0)
|
||||
expect(resolvedComplete).toHaveBeenCalledOnce()
|
||||
expect(resolvedComplete).toHaveBeenCalledWith(0)
|
||||
expect(resolvedExit).not.toHaveBeenCalled()
|
||||
|
||||
const rejectedExit = vi.fn()
|
||||
const rejected = createProcessShutdown(() => Promise.reject(new Error('dispose failed')), rejectedExit)
|
||||
const rejectedComplete = vi.fn()
|
||||
const rejected = createProcessShutdown(
|
||||
() => Promise.reject(new Error('dispose failed')),
|
||||
rejectedExit,
|
||||
rejectedComplete,
|
||||
)
|
||||
await rejected.shutdown(1)
|
||||
expect(rejectedExit).toHaveBeenCalledOnce()
|
||||
expect(rejectedExit).toHaveBeenCalledWith(1)
|
||||
expect(rejectedComplete).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('uses process.exit as the default process boundary', async () => {
|
||||
it('uses process.exitCode for default normal completion', async () => {
|
||||
const exit = vi.spyOn(process, 'exit').mockImplementation(_code => undefined as never)
|
||||
const originalExitCode = process.exitCode
|
||||
process.exitCode = undefined
|
||||
const shutdown = createProcessShutdown(() => Promise.resolve())
|
||||
|
||||
await shutdown.shutdown(7)
|
||||
try {
|
||||
await shutdown.shutdown(7)
|
||||
|
||||
expect(exit).toHaveBeenCalledOnce()
|
||||
expect(exit).toHaveBeenCalledWith(7)
|
||||
expect(process.exitCode).toBe(7)
|
||||
expect(exit).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
process.exitCode = originalExitCode
|
||||
}
|
||||
})
|
||||
|
||||
it('forces exit when graceful disposal reaches its bound', async () => {
|
||||
vi.useFakeTimers()
|
||||
const disposal = deferred()
|
||||
const exit = vi.fn()
|
||||
const shutdown = createProcessShutdown(() => disposal.promise, exit)
|
||||
const complete = vi.fn()
|
||||
const shutdown = createProcessShutdown(() => disposal.promise, exit, complete)
|
||||
const pending = shutdown.shutdown(0)
|
||||
|
||||
await vi.advanceTimersByTimeAsync(PROCESS_SHUTDOWN_TIMEOUT_MS - 1)
|
||||
@@ -60,13 +75,14 @@ describe('process shutdown', () => {
|
||||
disposal.resolve()
|
||||
await pending
|
||||
expect(exit).toHaveBeenCalledOnce()
|
||||
expect(complete).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('honors a caller-supplied grace period', async () => {
|
||||
vi.useFakeTimers()
|
||||
const disposal = deferred()
|
||||
const exit = vi.fn()
|
||||
const shutdown = createProcessShutdown(() => disposal.promise, exit, 25)
|
||||
const shutdown = createProcessShutdown(() => disposal.promise, exit, vi.fn(), 25)
|
||||
const pending = shutdown.shutdown(0)
|
||||
|
||||
await vi.advanceTimersByTimeAsync(24)
|
||||
@@ -81,7 +97,8 @@ describe('process shutdown', () => {
|
||||
it('lets Ctrl+C force a normal shutdown already stuck in disposal', async () => {
|
||||
const disposal = deferred()
|
||||
const exit = vi.fn()
|
||||
const shutdown = createProcessShutdown(() => disposal.promise, exit)
|
||||
const complete = vi.fn()
|
||||
const shutdown = createProcessShutdown(() => disposal.promise, exit, complete)
|
||||
const pending = shutdown.shutdown(0)
|
||||
|
||||
shutdown.interrupt(130)
|
||||
@@ -91,13 +108,29 @@ describe('process shutdown', () => {
|
||||
disposal.resolve()
|
||||
await pending
|
||||
expect(exit).toHaveBeenCalledOnce()
|
||||
expect(complete).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('forces exit after disposal started by a signal', async () => {
|
||||
const disposal = deferred()
|
||||
const exit = vi.fn()
|
||||
const complete = vi.fn()
|
||||
const shutdown = createProcessShutdown(() => disposal.promise, exit, complete)
|
||||
|
||||
shutdown.interrupt(143)
|
||||
disposal.resolve()
|
||||
await shutdown.shutdown(0)
|
||||
|
||||
expect(exit).toHaveBeenCalledOnce()
|
||||
expect(exit).toHaveBeenCalledWith(143)
|
||||
expect(complete).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('drains on the first signal and forces on the second signal', async () => {
|
||||
const disposal = deferred()
|
||||
const dispose = vi.fn(() => disposal.promise)
|
||||
const exit = vi.fn()
|
||||
const shutdown = createProcessShutdown(dispose, exit)
|
||||
const shutdown = createProcessShutdown(dispose, exit, vi.fn())
|
||||
|
||||
shutdown.interrupt(143)
|
||||
await Promise.resolve()
|
||||
@@ -116,7 +149,8 @@ describe('process shutdown', () => {
|
||||
it('coalesces normal shutdown calls without treating them as escalation', async () => {
|
||||
const disposal = deferred()
|
||||
const exit = vi.fn()
|
||||
const shutdown = createProcessShutdown(() => disposal.promise, exit)
|
||||
const complete = vi.fn()
|
||||
const shutdown = createProcessShutdown(() => disposal.promise, exit, complete)
|
||||
|
||||
const first = shutdown.shutdown(0)
|
||||
const second = shutdown.shutdown(1)
|
||||
@@ -125,7 +159,21 @@ describe('process shutdown', () => {
|
||||
|
||||
disposal.resolve()
|
||||
await first
|
||||
expect(complete).toHaveBeenCalledOnce()
|
||||
expect(complete).toHaveBeenCalledWith(0)
|
||||
expect(exit).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('lets a signal force exit while natural completion drains remaining handles', async () => {
|
||||
const exit = vi.fn()
|
||||
const complete = vi.fn()
|
||||
const shutdown = createProcessShutdown(() => Promise.resolve(), exit, complete)
|
||||
|
||||
await shutdown.shutdown(0)
|
||||
shutdown.interrupt(130)
|
||||
|
||||
expect(complete).toHaveBeenCalledOnce()
|
||||
expect(exit).toHaveBeenCalledOnce()
|
||||
expect(exit).toHaveBeenCalledWith(0)
|
||||
expect(exit).toHaveBeenCalledWith(130)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user