fix(hooks): drain detached hook runs on bridge dispose
The emit-shaped hook points (SessionStart, SubagentStart, SubagentStop) run fire-and-forget: no seam awaits the run chain, so disposing a bridge could strand a live hook process and let a late continuation inject into a disposed context. The floating continuation also made the coverage gate racy: the only coverage of the SubagentStart continuation's no-context branch arm rode on an un-awaited .then, and on a loaded CI runner the fork's per-file coverage snapshot beat it — master run 28798191671 failed the 100% branch gate on hooks-claude/src/index.ts at 99.03% (uncovered line 336) with the identical tree passing the PR run three minutes earlier. New shared primitive createDetachedRuns() in dsh-hook-protocol: a bridge tracks each detached run chain, passes the tracker's abort signal to runHook, and registers drain() as its effect disposer — drain aborts still-running hook processes (a kill via the bash seam, not a wait out to the 10-minute default hook timeout), then resolves once every chain has settled. fiber.dispose() resolving now means the bridge's detached work is quiescent (docs/defensive-patterns.md). The subagent marker test disposes the bridge as its sync point, so the formerly racy branch arm is executed deterministically before the file's coverage snapshot; new tests pin abort-on-dispose promptness for both bridges and the tracker's settle/drain contract in hook-protocol.
This commit is contained in:
@@ -13,6 +13,7 @@ Why a shared lib at all: Codex deliberately reimplements a *subset* of the Claud
|
||||
| Decode output | `parseHookOutput(exit, stdout, stderr)` → neutral `HookOutput` | maps the neutral `HookOutput` onto a seam-specific typed Decision |
|
||||
| Merge N hooks | `mergeHookOutputs(outputs)` → most-restrictive `MergedHookOutcome` | — |
|
||||
| Durable record | `appendHookInvoked` / `appendHookResult` (`hook/*` session events; the result's `decision`/`stderrSummary` derive from the `HookOutput` here) | calls them around each invocation |
|
||||
| Detached-run quiescence | `createDetachedRuns()` — track fire-and-forget run chains; `drain()` aborts, then awaits them | passes `signal` to each detached `runHook`, registers `drain` as its effect disposer |
|
||||
|
||||
## Primitives
|
||||
|
||||
@@ -20,6 +21,7 @@ Why a shared lib at all: Codex deliberately reimplements a *subset* of the Claud
|
||||
- **`runHook(bash, hook, options, now)`** — serialize `options.payload` to the hook's stdin (with a trailing newline iff `options.trailingNewline`), merge `options.env` after the executor's credential scrub (the `dsh-bash` trusted-plugin surface), honor the hook's `timeoutSec` (else `options.defaultTimeoutMs` — the bridge owns the default, its config defaulting to the lib's `DEFAULT_HOOK_TIMEOUT_MS` 10-minute reference), and decode the result (threading `options.expectedEventName` to the codec). Never throws: an executor rejection (infra fault) becomes a `HookOutput` with `exitCode: undefined` (a non-blocking error). `now` is injected for testable durations.
|
||||
- **`parseHookOutput(exitCode, stdout, stderr, expectedEventName?)`** — the exit-code + structured-stdout codec. Exit `0` → parse JSON stdout (lenient: non-JSON is left for the bridge); exit `2` → blocking error, `stderr` is the block reason (surfaced as `decision: 'block'`); other → non-blocking error. `hookSpecificOutput.permissionDecision` (allow/deny/ask) overrides a legacy top-level `decision`; `additionalContext`/`updatedInput`/`systemMessage`/`continue`/`stopReason` are parsed too. The schemas key the `hookSpecificOutput` block by `hookEventName`, so passing `expectedEventName` (the firing event) DISCARDS a block whose `hookEventName` names a different event — or omits it entirely — its event-scoped fields don't take effect (a `PreToolUse` block on a `Stop` hook is malformed, and so is a discriminator-less block that would otherwise apply to any event), while the event-agnostic top-level fields still apply. Pure and total.
|
||||
- **`mergeHookOutputs(outputs)`** — fold the results of every hook that matched one point: permission precedence **deny > ask > allow**, halt sticky on the first `continue:false`, block reasons joined with `\n\n`, `additionalContext`/`systemMessages` accumulated in order.
|
||||
- **`createDetachedRuns()`** — quiescence tracking for the emit-shaped points, which run detached (no seam awaits them). The bridge tracks each run chain — the hook run PLUS its continuation — and registers `drain()` as its effect disposer: drain fires the tracker's abort `signal` (so a still-running hook process is killed via `runHook`, not awaited out to its timeout), then resolves once every tracked chain has settled. `fiber.dispose()` resolving therefore means no detached hook work is left to fire into a disposed context ([defensive patterns](../../../docs/defensive-patterns.md): dispose must reach quiescence).
|
||||
|
||||
## `hook/*` session events
|
||||
|
||||
|
||||
71
packages/hooks/hook-protocol/src/detached.ts
Normal file
71
packages/hooks/hook-protocol/src/detached.ts
Normal file
@@ -0,0 +1,71 @@
|
||||
/**
|
||||
* Quiescence tracking for a bridge's DETACHED hook runs. The waterfall-shaped
|
||||
* hook points (`UserPromptSubmit`, `PreToolUse`, …) are awaited by their seams,
|
||||
* but the emit-shaped points (`SessionStart`, `SubagentStart`, `SubagentStop`)
|
||||
* run fire-and-forget: no seam awaits them, so without tracking a bridge's
|
||||
* disposal could strand a live hook process and let a late continuation fire
|
||||
* into a disposed context (docs/defensive-patterns.md: dispose must reach
|
||||
* quiescence). A bridge creates one tracker in `apply()`, passes
|
||||
* {@link DetachedRuns.signal} to each detached {@link runHook} call, wraps the
|
||||
* full run chain (the hook run PLUS its `.then` continuation) in
|
||||
* {@link DetachedRuns.track}, and registers {@link DetachedRuns.drain} as its
|
||||
* disposer.
|
||||
*
|
||||
* @module @deepseek-ai/dsh-hook-protocol/detached
|
||||
*/
|
||||
|
||||
/** In-flight registry for one bridge's detached hook runs; see the module doc for the wiring contract. */
|
||||
export interface DetachedRuns {
|
||||
/**
|
||||
* The abort signal every tracked run must hand to {@link runHook} (via its
|
||||
* `signal` option). {@link drain} fires it so a still-running hook process is
|
||||
* killed rather than awaited out to its timeout (default 10 minutes).
|
||||
*/
|
||||
readonly signal: AbortSignal
|
||||
/**
|
||||
* Register one detached run until it settles. Pass the FULL chain — the hook
|
||||
* run and its continuation/error handler — so {@link drain} waits for the
|
||||
* side effects (an inject, a warn), not just the process exit. A rejected
|
||||
* chain is absorbed here (settlement bookkeeping only), but rejection
|
||||
* handling is still the caller's job: an untracked `.catch` is what turns a
|
||||
* failure into a logged warning instead of silence.
|
||||
* @param run - the detached run chain to hold until settled.
|
||||
*/
|
||||
track(run: Promise<unknown>): void
|
||||
/**
|
||||
* Abort {@link signal}, then resolve once every tracked chain has settled —
|
||||
* including chains tracked while the drain is in progress. The bridge
|
||||
* registers this as its effect disposer; cordis awaits it, so
|
||||
* `fiber.dispose()` resolving means the bridge's detached work is quiescent.
|
||||
* A run tracked AFTER drain resolves is not awaited by anyone — by then the
|
||||
* bridge's listeners are disposed, so nothing can start one.
|
||||
* @returns resolves when all tracked runs have settled.
|
||||
*/
|
||||
drain(): Promise<void>
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a {@link DetachedRuns} tracker (one per bridge `apply()`); settled
|
||||
* runs are pruned so a long-lived session does not accumulate them.
|
||||
* @returns the tracker.
|
||||
*/
|
||||
export function createDetachedRuns(): DetachedRuns {
|
||||
const inflight = new Set<Promise<unknown>>()
|
||||
const controller = new AbortController()
|
||||
return {
|
||||
signal: controller.signal,
|
||||
track(run: Promise<unknown>): void {
|
||||
inflight.add(run)
|
||||
const settled = (): void => { inflight.delete(run) }
|
||||
void run.then(settled, settled)
|
||||
},
|
||||
async drain(): Promise<void> {
|
||||
controller.abort(new Error('hook bridge disposed'))
|
||||
// Re-check after each wave: a chain can be tracked while a prior wave is
|
||||
// settling; loop until the registry is observed empty.
|
||||
while (inflight.size > 0) {
|
||||
await Promise.allSettled([...inflight])
|
||||
}
|
||||
},
|
||||
}
|
||||
}
|
||||
@@ -15,6 +15,8 @@
|
||||
* session-event helpers (declaration-merged into `SessionEventMap`);
|
||||
* `appendHookResult` derives the durable `decision`/`stderrSummary` from the
|
||||
* {@link HookOutput} so the shared event's semantics live in one place.
|
||||
* - {@link createDetachedRuns} — quiescence tracking for the fire-and-forget
|
||||
* hook points: disposal aborts and drains a bridge's detached runs.
|
||||
*
|
||||
* Each bridge owns what genuinely DIFFERS: building the per-event stdin payload
|
||||
* (CC vs Codex field sets), the dialect's env/substitution, and mapping the
|
||||
@@ -38,3 +40,5 @@ export { mergeHookOutputs } from './merge.ts'
|
||||
export type { MergedDecision, MergedHookOutcome } from './merge.ts'
|
||||
export { appendHookInvoked, appendHookResult, DEFAULT_STDERR_SUMMARY_MAX_CHARS, summarizeStderr } from './events.ts'
|
||||
export type { HookInvocation, HookResultRecord } from './events.ts'
|
||||
export { createDetachedRuns } from './detached.ts'
|
||||
export type { DetachedRuns } from './detached.ts'
|
||||
|
||||
68
packages/hooks/hook-protocol/tests/detached.spec.ts
Normal file
68
packages/hooks/hook-protocol/tests/detached.spec.ts
Normal file
@@ -0,0 +1,68 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { createDetachedRuns } from '@deepseek-ai/dsh-hook-protocol'
|
||||
|
||||
/** A promise settled from outside, so a test controls exactly when a tracked run finishes. */
|
||||
function deferred(): { promise: Promise<void>; resolve: () => void; reject: (error: Error) => void } {
|
||||
let resolve!: () => void
|
||||
let reject!: (error: Error) => void
|
||||
const promise = new Promise<void>((res, rej) => { resolve = res; reject = rej })
|
||||
return { promise, resolve, reject }
|
||||
}
|
||||
|
||||
describe('createDetachedRuns', () => {
|
||||
it('starts with an unfired signal; drain fires it (so still-running hook processes get killed)', async () => {
|
||||
const detached = createDetachedRuns()
|
||||
expect(detached.signal.aborted).toBe(false)
|
||||
await detached.drain()
|
||||
expect(detached.signal.aborted).toBe(true)
|
||||
expect(String(detached.signal.reason)).toContain('hook bridge disposed')
|
||||
})
|
||||
|
||||
it('drain with nothing tracked resolves immediately', async () => {
|
||||
await expect(createDetachedRuns().drain()).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
it('drain waits for a tracked run to settle', async () => {
|
||||
const detached = createDetachedRuns()
|
||||
const run = deferred()
|
||||
detached.track(run.promise)
|
||||
let drained = false
|
||||
const draining = detached.drain().then(() => { drained = true })
|
||||
// Give the drain every chance to (wrongly) resolve before the run settles.
|
||||
await new Promise(resolve => setTimeout(resolve, 10))
|
||||
expect(drained).toBe(false)
|
||||
run.resolve()
|
||||
await draining
|
||||
expect(drained).toBe(true)
|
||||
})
|
||||
|
||||
it('drain waits for a run tracked WHILE a prior wave was settling', async () => {
|
||||
const detached = createDetachedRuns()
|
||||
const first = deferred()
|
||||
const second = deferred()
|
||||
detached.track(first.promise)
|
||||
// The late run enters the registry from the first run's own continuation —
|
||||
// after drain() snapshotted its first wave.
|
||||
void first.promise.then(() => { detached.track(second.promise) })
|
||||
let drained = false
|
||||
const draining = detached.drain().then(() => { drained = true })
|
||||
first.resolve()
|
||||
await new Promise(resolve => setTimeout(resolve, 10))
|
||||
expect(drained).toBe(false)
|
||||
second.resolve()
|
||||
await draining
|
||||
expect(drained).toBe(true)
|
||||
})
|
||||
|
||||
it('a rejected tracked run is absorbed by the settlement bookkeeping (drain still resolves)', async () => {
|
||||
const detached = createDetachedRuns()
|
||||
const run = deferred()
|
||||
detached.track(run.promise)
|
||||
// The caller-side handler every bridge attaches; the tracker's own
|
||||
// bookkeeping must not depend on it, but an UNHANDLED rejection would fail
|
||||
// the test run, which is exactly the guarantee under test.
|
||||
run.promise.catch(() => {})
|
||||
run.reject(new Error('hook run boom'))
|
||||
await expect(detached.drain()).resolves.toBeUndefined()
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user