Merge origin/master: scope-aware fusion of the tools/execute seam, session-prefix, and tool-cordis
Master brought 50 commits (the tool-cordis group, dsh-code-runtime + worker, the tools/execute around-dispatch seam + timeout-policy, repeat-tool-guard, agent/session-prefix, the ui reorganization). Beyond the ten textual conflicts, the merge reconciles master's new seams with this branch's scoped-registration world: - tools/execute (new waterfall around core dispatch): dispatched with the SAME exec.agent carrier as the pre/post waterfalls — an agent.ctx wrapper times/retries only its own agent's calls — and its base thunk resolves the tool through the caller's visible view (get(exec.name, exec.agent)), so a scoped/shadowed tool dispatches and a restricted-away global stays UNKNOWN_TOOL. Declared this: Scoped<ToolRegistry> with the scope-filtered doc sentence; invariants table + verify-scoped-dispatch pin it (21 events). - agent/session-prefix (new waterfall, once per loop instance): composed via the fused agentEvents dispatcher (scope-filtered like every agent-subject event), declared this: Scoped<Agent>, table-pinned. agent/pre-step keeps master's new sessionPrefix parameter with this branch's Scoped this. - timeout-policy reads the budget through the caller's visible view (get(exec.name, exec.agent)): a scoped tool's own timeoutMs governs its calls; a global name-twin's budget is never misapplied to a shadowing per-agent variant. - tool-cordis: cordis_inspect's tools section lists the CALLING agent's view (its description promises "what you can call"); the sandbox tool façade's reads resolve through the mount's own scope, mirroring where its register lands writes; sandboxRegisterTool's return type carries the exact-disposer union honestly. dsh-scope declared as peer+dev with the project reference. - doc-sync chain unions master's verify-cordis-api with this branch's verify-scoped-dispatch; the generated catalogs, event matrix (the zero-dispatcher guard passes over master's new events), module graph, and the cordis api-catalog are regenerated on the merged surface. Full gate sequence green on the merged tree: typecheck, lint, per-file 100% coverage (2668 tests), snapshots (38), doc-sync, module graph, build, hygiene, demo smoke.
This commit is contained in:
@@ -23,6 +23,7 @@
|
||||
"license": "BSD-3-Clause",
|
||||
"peerDependencies": {
|
||||
"@deepseek-ai/dsh-bash": "^0.0.1",
|
||||
"@deepseek-ai/dsh-timeout": "^0.0.1",
|
||||
"cordis": "^4.0.0-rc.6"
|
||||
},
|
||||
"dependencies": {
|
||||
@@ -30,6 +31,7 @@
|
||||
},
|
||||
"devDependencies": {
|
||||
"@deepseek-ai/dsh-bash": "workspace:^",
|
||||
"@deepseek-ai/dsh-timeout": "workspace:^",
|
||||
"cordis": "^4.0.0-rc.6"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,6 +18,7 @@ import { Context } from 'cordis'
|
||||
import z from 'schemastery'
|
||||
import { BashExecutor, BashTaskId } from '@deepseek-ai/dsh-bash'
|
||||
import type { BashExecRequest, BashExecSpec, BashRunResult, BashTask, BashTaskRead, OwnerToken } from '@deepseek-ai/dsh-bash'
|
||||
import { clampTimeout, deadline, timeoutOf } from '@deepseek-ai/dsh-timeout'
|
||||
import { DEFAULT_GRACE_MS, runBash } from './run.ts'
|
||||
import type { RunInternals, RunningBash } from './run.ts'
|
||||
|
||||
@@ -114,8 +115,12 @@ export class LocalBashExecutor extends BashExecutor {
|
||||
* values and never re-default.
|
||||
*/
|
||||
resolve(request: BashExecRequest): BashExecSpec {
|
||||
if (request.timeoutMs !== undefined) assertPositiveFinite('request.timeoutMs', request.timeoutMs)
|
||||
const timeoutMs = Math.min(request.timeoutMs ?? this.config.timeoutMs, this.config.maxTimeoutMs)
|
||||
const timeoutMs = clampTimeout(
|
||||
request.timeoutMs,
|
||||
this.config.timeoutMs,
|
||||
this.config.maxTimeoutMs,
|
||||
'bash-local: request.timeoutMs',
|
||||
)
|
||||
return {
|
||||
command: request.command,
|
||||
workdir: request.workdir ?? this.config.cwd ?? process.cwd(),
|
||||
@@ -132,29 +137,39 @@ export class LocalBashExecutor extends BashExecutor {
|
||||
}
|
||||
|
||||
async run(spec: BashExecSpec): Promise<BashRunResult> {
|
||||
// One fused deadline drives both the timeout and upstream cancellation;
|
||||
// runBash listens on d.signal and runs the SIGTERM→grace→SIGKILL kill.
|
||||
// `using` clears the timer across the awaited process lifetime.
|
||||
using d = deadline(spec.signal, spec.timeoutMs, 'BASH_TIMEOUT')
|
||||
const outcome = await runBash({
|
||||
command: spec.command,
|
||||
cwd: spec.workdir,
|
||||
timeoutMs: spec.timeoutMs,
|
||||
maxOutputBytes: this.config.maxOutputBytes,
|
||||
graceMs: this.config.graceMs,
|
||||
signal: spec.signal,
|
||||
signal: d.signal,
|
||||
stdin: spec.stdin,
|
||||
env: spec.env,
|
||||
}, this.internals).done
|
||||
return { ...outcome, timeoutMs: spec.timeoutMs }
|
||||
// Classify the FIRST abort reason: a BASH_TIMEOUT TimeoutReason means our
|
||||
// timeout cut the command short; any other abort — an upstream cancel, or a
|
||||
// foreign (outer) deadline's timeout under nesting — is aborted. Scoping to
|
||||
// our own code keeps a nested outer deadline from reading as our timeout.
|
||||
// Mutually exclusive by construction — the fused signal reports one cause.
|
||||
const timedOut = timeoutOf(d.signal, 'BASH_TIMEOUT') !== undefined
|
||||
const aborted = d.signal.aborted && !timedOut
|
||||
return { ...outcome, timedOut, aborted, timeoutMs: spec.timeoutMs }
|
||||
}
|
||||
|
||||
start(spec: BashExecSpec): BashTask {
|
||||
// No timeout for background tasks (matches Claude Code, which detaches
|
||||
// the timeout when backgrounding); callers stop tasks via kill() — or
|
||||
// via spec.signal, which the seam contract honors for background runs
|
||||
// too (runBash wires it to the group kill). spec.timeoutMs is ignored
|
||||
// here by design.
|
||||
// too (runBash wires it to the group kill). No deadline is created here,
|
||||
// so spec.timeoutMs is ignored by design — background tasks stay
|
||||
// timeout-free (see the timeout-library RFC).
|
||||
const running = runBash({
|
||||
command: spec.command,
|
||||
cwd: spec.workdir,
|
||||
timeoutMs: 0,
|
||||
maxOutputBytes: this.config.maxOutputBytes,
|
||||
graceMs: this.config.graceMs,
|
||||
signal: spec.signal,
|
||||
@@ -174,8 +189,10 @@ export class LocalBashExecutor extends BashExecutor {
|
||||
stdoutOffset: 0,
|
||||
stderrOffset: 0,
|
||||
done: running.done.then((outcome) => {
|
||||
// Abort-killed tasks report as killed, not completed.
|
||||
if (task.status === 'running') task.status = outcome.aborted ? 'killed' : 'completed'
|
||||
// Abort-killed tasks report as killed, not completed. Background runs
|
||||
// forward only the upstream signal (no timeout), so its aborted state
|
||||
// is the authoritative "was this cancelled" signal.
|
||||
if (task.status === 'running') task.status = spec.signal?.aborted === true ? 'killed' : 'completed'
|
||||
task.exitCode = outcome.exitCode
|
||||
task.signal = outcome.signal
|
||||
this.notifyTaskDone(task)
|
||||
|
||||
@@ -6,6 +6,12 @@
|
||||
* Everything here is deliberately free of Cordis concepts so it can be unit
|
||||
* tested in isolation; `LocalBashExecutor` owns lifecycle and configuration.
|
||||
*
|
||||
* runBash owns NO timing: it kills the process group when its `spec.signal`
|
||||
* fires and does not distinguish a timeout from a cancel. The executor fuses
|
||||
* timeout + upstream cancellation into that one signal via
|
||||
* `@deepseek-ai/dsh-timeout`'s `deadline`, and classifies the outcome from the
|
||||
* signal afterward — the timing/classification half is shared, the kill is not.
|
||||
*
|
||||
* Design notes (surveyed against Claude Code, OpenCode, Codex, and pi — see
|
||||
* the package README): spawn-per-call with `detached: true` so the child
|
||||
* leads its own process group; kills target the group (`kill(-pid)`) so
|
||||
@@ -71,13 +77,17 @@ export function childEnv(extra?: Record<string, string>): NodeJS.ProcessEnv {
|
||||
export interface SpawnSpec {
|
||||
command: string
|
||||
cwd: string
|
||||
/** Kill the process group after this many milliseconds. 0 = no timeout. */
|
||||
timeoutMs: number
|
||||
/** Per-stream in-memory cap; overflow spills to disk (tail kept in memory). */
|
||||
maxOutputBytes: number
|
||||
/** Grace period between the SIGTERM and the SIGKILL escalation on a kill. */
|
||||
graceMs: number
|
||||
/** Abort signal — kills the process group when fired. */
|
||||
/**
|
||||
* Abort signal — kills the process group when it fires. The executor owns
|
||||
* timing: `run()` passes a fused timeout/cancel deadline signal (see
|
||||
* `@deepseek-ai/dsh-timeout`), `start()` passes the bare upstream signal.
|
||||
* runBash only listens and kills; it does NOT classify why (the executor
|
||||
* reads the signal's reason afterward).
|
||||
*/
|
||||
signal?: AbortSignal | undefined
|
||||
/**
|
||||
* Bytes to write to the child's stdin, then close it. Absent (or empty)
|
||||
@@ -94,12 +104,15 @@ export interface SpawnSpec {
|
||||
env?: Record<string, string> | undefined
|
||||
}
|
||||
|
||||
/** Raw outcome of one closed process (before result shaping). */
|
||||
/**
|
||||
* Raw outcome of one closed process (before result shaping). Deliberately
|
||||
* carries NO timeout/cancel classification: runBash kills on abort but does not
|
||||
* decide why — the executor's `run()`/`start()` reads the deadline signal it
|
||||
* owns to classify `timedOut`/`aborted` (see the package README).
|
||||
*/
|
||||
export interface SpawnOutcome {
|
||||
exitCode: number | null
|
||||
signal: NodeJS.Signals | null
|
||||
timedOut: boolean
|
||||
aborted: boolean
|
||||
stdout: CollectedOutput
|
||||
stderr: CollectedOutput
|
||||
}
|
||||
@@ -343,9 +356,6 @@ export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningB
|
||||
child.stdout.on('data', (chunk: Buffer) => { stdout.push(chunk) })
|
||||
child.stderr.on('data', (chunk: Buffer) => { stderr.push(chunk) })
|
||||
|
||||
let timedOut = false
|
||||
let aborted = false
|
||||
let killTimer: NodeJS.Timeout | undefined
|
||||
let graceTimer: NodeJS.Timeout | undefined
|
||||
|
||||
// pid is undefined when the spawn itself fails (bad cwd, missing binary);
|
||||
@@ -358,17 +368,12 @@ export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningB
|
||||
graceTimer = setTimeout(() => { killGroup(pid, 'SIGKILL') }, spec.graceMs)
|
||||
}
|
||||
|
||||
if (spec.timeoutMs > 0) {
|
||||
killTimer = setTimeout(() => {
|
||||
timedOut = true
|
||||
kill()
|
||||
}, spec.timeoutMs)
|
||||
}
|
||||
|
||||
const onAbort = (): void => {
|
||||
aborted = true
|
||||
kill()
|
||||
}
|
||||
// runBash owns no timer: the executor's `run()` fuses timeout+cancel into one
|
||||
// deadline signal (`@deepseek-ai/dsh-timeout`) and passes it here; we only
|
||||
// listen and run the SIGTERM→grace→SIGKILL kill. Whether the abort was a
|
||||
// timeout or an upstream cancel is classified by the executor from that
|
||||
// signal, not tracked here.
|
||||
const onAbort = (): void => { kill() }
|
||||
spec.signal?.addEventListener('abort', onAbort, { once: true })
|
||||
|
||||
// Write stdin and close it, but ONLY when the caller supplied bytes — with no
|
||||
@@ -401,14 +406,11 @@ export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningB
|
||||
resolve({
|
||||
exitCode,
|
||||
signal,
|
||||
timedOut,
|
||||
aborted,
|
||||
stdout: stdout.finalize(),
|
||||
stderr: stderr.finalize(),
|
||||
})
|
||||
})
|
||||
function cleanup(): void {
|
||||
if (killTimer !== undefined) clearTimeout(killTimer)
|
||||
if (graceTimer !== undefined) clearTimeout(graceTimer)
|
||||
spec.signal?.removeEventListener('abort', onAbort)
|
||||
}
|
||||
|
||||
@@ -40,12 +40,14 @@ async function readUntil(
|
||||
): Promise<BashTaskRead> {
|
||||
const deadline = Date.now() + timeoutMs
|
||||
let last: BashTaskRead | undefined
|
||||
let delta = ''
|
||||
while (Date.now() < deadline) {
|
||||
last = bash.readOutput(id)
|
||||
if (last.delta.includes(expected)) return last
|
||||
delta += last.delta
|
||||
if (delta.includes(expected)) return { ...last, delta }
|
||||
await new Promise(resolve => setTimeout(resolve, 20))
|
||||
}
|
||||
throw new Error(`task ${id} output did not include ${JSON.stringify(expected)}; last delta was ${JSON.stringify(last?.delta ?? '')}`)
|
||||
throw new Error(`task ${id} output did not include ${JSON.stringify(expected)}; output was ${JSON.stringify(delta)}, last delta was ${JSON.stringify(last?.delta ?? '')}`)
|
||||
}
|
||||
|
||||
describe('LocalBashExecutor.run', () => {
|
||||
@@ -90,8 +92,8 @@ describe('LocalBashExecutor.run', () => {
|
||||
|
||||
it('kill escalation uses the configured graceMs (a TERM-trapping task dies by SIGKILL)', async () => {
|
||||
const { bash } = await setup() // setup pins graceMs: 200 via config
|
||||
const task = bash.start(bash.resolve({ command: 'trap \'\' TERM; sleep 60' }))
|
||||
await new Promise(resolve => setTimeout(resolve, 100))
|
||||
const task = bash.start(bash.resolve({ command: 'trap \'\' TERM; echo ready; while :; do sleep 60 & wait $!; done' }))
|
||||
await readUntil(bash, task.id, 'ready\n')
|
||||
bash.kill(task.id)
|
||||
await task.done
|
||||
expect(task.signal).toBe('SIGKILL')
|
||||
@@ -101,6 +103,8 @@ describe('LocalBashExecutor.run', () => {
|
||||
const { bash } = await setup({ timeoutMs: 60_000 })
|
||||
const result = await bash.run(bash.resolve({ command: 'sleep 60', timeoutMs: 100 }))
|
||||
expect(result.timedOut).toBe(true)
|
||||
// Mutually exclusive: a timeout classifies as timedOut, never also aborted.
|
||||
expect(result.aborted).toBe(false)
|
||||
expect(result.timeoutMs).toBe(100)
|
||||
})
|
||||
|
||||
@@ -111,6 +115,20 @@ describe('LocalBashExecutor.run', () => {
|
||||
setTimeout(() => { controller.abort() }, 50)
|
||||
const result = await pending
|
||||
expect(result.aborted).toBe(true)
|
||||
// Mutually exclusive: an upstream cancel classifies as aborted, never also timedOut.
|
||||
expect(result.timedOut).toBe(false)
|
||||
})
|
||||
|
||||
it('classifies a self-killed command as neither timed out nor aborted', async () => {
|
||||
// The command kills itself (SIGTERM) with no timeout and no upstream abort:
|
||||
// the deadline signal never fires, so both classifications are false — the
|
||||
// fused-signal classification reports the cause that cut the command short,
|
||||
// and here nothing the executor owns did.
|
||||
const { bash } = await setup({ timeoutMs: 60_000 })
|
||||
const result = await bash.run(bash.resolve({ command: 'kill -TERM $$' }))
|
||||
expect(result.signal).toBe('SIGTERM')
|
||||
expect(result.timedOut).toBe(false)
|
||||
expect(result.aborted).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects on spawn failure (bad workdir)', async () => {
|
||||
|
||||
@@ -26,7 +26,6 @@ function spec(command: string, overrides: Partial<Parameters<typeof runBash>[0]>
|
||||
return {
|
||||
command,
|
||||
cwd: process.cwd(),
|
||||
timeoutMs: 0,
|
||||
maxOutputBytes: 64_000,
|
||||
graceMs: 3_000,
|
||||
...overrides,
|
||||
@@ -56,13 +55,25 @@ async function waitForStdout(running: RunningBash, expected: string, timeoutMs =
|
||||
throw new Error(`stdout did not include ${JSON.stringify(expected)} after ${timeoutMs}ms`)
|
||||
}
|
||||
|
||||
async function waitForPidFile(path: string, timeoutMs = 5_000): Promise<number> {
|
||||
const deadline = Date.now() + timeoutMs
|
||||
while (Date.now() < deadline) {
|
||||
try {
|
||||
const pid = Number(readFileSync(path, 'utf8').trim())
|
||||
if (Number.isSafeInteger(pid) && pid > 0) return pid
|
||||
} catch {
|
||||
// The child shell has not written the pid file yet.
|
||||
}
|
||||
await new Promise(resolve => setTimeout(resolve, 20))
|
||||
}
|
||||
throw new Error(`pid file ${path} was not written after ${timeoutMs}ms`)
|
||||
}
|
||||
|
||||
describe('runBash', () => {
|
||||
it('captures stdout on success', async () => {
|
||||
const result = await runBash(spec('echo hello')).done
|
||||
expect(result.exitCode).toBe(0)
|
||||
expect(result.signal).toBeNull()
|
||||
expect(result.timedOut).toBe(false)
|
||||
expect(result.aborted).toBe(false)
|
||||
expect(result.stdout.text).toBe('hello\n')
|
||||
expect(result.stdout.truncated).toBe(false)
|
||||
expect(result.stderr.text).toBe('')
|
||||
@@ -97,17 +108,22 @@ describe('runBash', () => {
|
||||
expect(result.stdout.text.trim()).toMatch(/\/tmp$/)
|
||||
})
|
||||
|
||||
it('kills with SIGTERM on timeout', async () => {
|
||||
it('kills the process group with SIGTERM when the signal fires', async () => {
|
||||
// runBash owns no timer: it kills on abort. The executor drives the timeout
|
||||
// by firing this signal via a deadline (see executor.spec.ts); here we
|
||||
// assert the kill itself lands as SIGTERM.
|
||||
const controller = new AbortController()
|
||||
const start = Date.now()
|
||||
const result = await runBash(spec('sleep 60', { timeoutMs: 100 })).done
|
||||
const running = runBash(spec('sleep 60', { signal: controller.signal }))
|
||||
setTimeout(() => { controller.abort('deadline') }, 100)
|
||||
const result = await running.done
|
||||
expect(Date.now() - start).toBeLessThan(5_000)
|
||||
expect(result.timedOut).toBe(true)
|
||||
expect(result.signal).toBe('SIGTERM')
|
||||
expect(result.exitCode).toBeNull()
|
||||
})
|
||||
|
||||
it('escalates to SIGKILL when SIGTERM is trapped', async () => {
|
||||
const running = runBash(spec('trap \'\' TERM; echo ready; sleep 60', { graceMs: 200 }))
|
||||
const running = runBash(spec('trap \'\' TERM; echo ready; while :; do sleep 60 & wait $!; done', { graceMs: 200 }))
|
||||
await waitForStdout(running, 'ready\n')
|
||||
running.kill()
|
||||
const result = await running.done
|
||||
@@ -119,8 +135,7 @@ describe('runBash', () => {
|
||||
// group must take the sleep down with bash.
|
||||
const pidFile = join(spillDir, `grandchild-${Date.now()}.pid`)
|
||||
const running = runBash(spec(`sleep 60 & echo $! > ${pidFile}; wait`))
|
||||
await new Promise(resolve => setTimeout(resolve, 300))
|
||||
const grandchild = Number(readFileSync(pidFile, 'utf8').trim())
|
||||
const grandchild = await waitForPidFile(pidFile)
|
||||
expect(grandchild).toBeGreaterThan(0)
|
||||
|
||||
running.kill()
|
||||
@@ -134,7 +149,6 @@ describe('runBash', () => {
|
||||
const running = runBash(spec('sleep 60', { signal: controller.signal }))
|
||||
setTimeout(() => { controller.abort('user cancelled') }, 50)
|
||||
const result = await running.done
|
||||
expect(result.aborted).toBe(true)
|
||||
expect(result.signal).toBe('SIGTERM')
|
||||
})
|
||||
|
||||
@@ -211,7 +225,6 @@ describe('stdin and extra env (set by in-process plugins)', () => {
|
||||
const big = 'x'.repeat(1024 * 1024)
|
||||
const result = await runBash(spec('exit 7', { stdin: big })).done
|
||||
expect(result.exitCode).toBe(7)
|
||||
expect(result.aborted).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -339,11 +352,11 @@ describe('abort edge cases', () => {
|
||||
.toThrow(/aborted before spawn: aborted/)
|
||||
})
|
||||
|
||||
it('reports an externally self-killed command without the timeout marker', async () => {
|
||||
it('reports the terminating signal of an externally self-killed command', async () => {
|
||||
// runBash reports the raw signal; whether it counts as timeout/cancel is the
|
||||
// executor's classification (a self-kill is neither) — see executor.spec.ts.
|
||||
const result = await runBash(spec('kill -TERM $$')).done
|
||||
expect(result.signal).toBe('SIGTERM')
|
||||
expect(result.timedOut).toBe(false)
|
||||
expect(result.aborted).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -396,10 +409,9 @@ describe('review fixes: env scrubbing and spill hardening', () => {
|
||||
|
||||
it('honors AbortSignal on background-style runs (no timeout)', async () => {
|
||||
const controller = new AbortController()
|
||||
const running = runBash(spec('sleep 60', { timeoutMs: 0, signal: controller.signal }))
|
||||
const running = runBash(spec('sleep 60', { signal: controller.signal }))
|
||||
setTimeout(() => { controller.abort() }, 50)
|
||||
const result = await running.done
|
||||
expect(result.aborted).toBe(true)
|
||||
expect(result.signal).toBe('SIGTERM')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -20,6 +20,9 @@
|
||||
{
|
||||
"path": "../../util/brand"
|
||||
},
|
||||
{
|
||||
"path": "../../util/timeout"
|
||||
},
|
||||
{
|
||||
"path": "../../bash/bash"
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user