mirror of
https://github.com/deepseek-ai/deepseek-harness
synced 2026-08-15 21:04:50 +00:00
fix(e2b): address the v7 review round
- restore the parent's AGENTS.md body around this branch's e2b row (the replay had resurrected the pre-profile-CLI layout, deleting the api/, bundle/, settings/, and credentials/ groups); the fs/lsp row condensations stay for the word budget - aggregate the composite rollback failure, not the original error, when private state cleanup also fails — a surviving remote group is no longer hidden by a later cleanup error (triple-failure regression) - own the state directory before the makeDir RPC, matching the terminal transaction, so a cancellation racing a committed creation still enters cleanup - drop the hidden pollMs parameter defaults; the schemastery default is the one home, and tests pass the cadence explicitly - restore spawn/spawnTerminal graceMs validation to the seam's documented bound (the earlier removal cited subprocess-local as not validating; it does), with rejection regressions
This commit is contained in:
@@ -9,6 +9,7 @@ import { posix } from 'node:path'
|
||||
import { Context } from 'cordis'
|
||||
import z from 'schemastery'
|
||||
import { SubprocessService } from '@deepseek-ai/dsh-subprocess'
|
||||
import { MAX_TIMER_DELAY_MS } from '@deepseek-ai/dsh-timeout'
|
||||
import type {
|
||||
SubprocessHandle,
|
||||
SubprocessSpawnSpec,
|
||||
@@ -35,6 +36,18 @@ interface TerminalSetup {
|
||||
controller: AbortController
|
||||
}
|
||||
|
||||
/**
|
||||
* Enforce the seam's documented grace bound (positive, finite, one Node timer),
|
||||
* matching subprocess-local's spawn-time check; an unbounded grace would make
|
||||
* the remote force-escalation deadline unreachable.
|
||||
* @param graceMs - The spec's cleanup grace in milliseconds.
|
||||
*/
|
||||
function requireRepresentableGrace(graceMs: number): void {
|
||||
if (!Number.isFinite(graceMs) || graceMs <= 0 || graceMs > MAX_TIMER_DELAY_MS) {
|
||||
throw new Error(`subprocess graceMs must be a positive finite number no greater than ${MAX_TIMER_DELAY_MS}`)
|
||||
}
|
||||
}
|
||||
|
||||
/** E2B command manager registered as `ctx.subprocess`. */
|
||||
export class E2BSubprocessService extends SubprocessService {
|
||||
static inject = ['e2b']
|
||||
@@ -130,6 +143,7 @@ export class E2BSubprocessService extends SubprocessService {
|
||||
if (program === undefined || program.length === 0) {
|
||||
throw new Error('invalid argv: expected a non-empty program name at argv[0]')
|
||||
}
|
||||
requireRepresentableGrace(spec.graceMs)
|
||||
if (spec.signal?.aborted === true) {
|
||||
throw new Error(`aborted before spawn: ${String(spec.signal.reason)}`)
|
||||
}
|
||||
@@ -153,6 +167,7 @@ export class E2BSubprocessService extends SubprocessService {
|
||||
if (program === undefined || program.length === 0) {
|
||||
throw new Error('subprocess-e2b: terminal argv must contain a program')
|
||||
}
|
||||
requireRepresentableGrace(spec.graceMs)
|
||||
spec.signal?.throwIfAborted()
|
||||
const stateDir = posix.join(this.ctx.e2b.runtimeRoot, 'terminals', randomUUID())
|
||||
const done = Promise.withResolvers<void>()
|
||||
|
||||
@@ -194,7 +194,7 @@ export class E2BSubprocessHandle implements SubprocessHandle {
|
||||
private readonly runtime: E2BSandboxService,
|
||||
private readonly spec: SubprocessSpawnSpec,
|
||||
readonly stateDir: string,
|
||||
private readonly pollMs = 20,
|
||||
private readonly pollMs: number,
|
||||
) {
|
||||
this.paths = {
|
||||
pid: posix.join(stateDir, 'pid'),
|
||||
@@ -378,7 +378,7 @@ export class E2BSubprocessHandle implements SubprocessHandle {
|
||||
await this.removeFailedState(sandbox)
|
||||
} catch (cleanupError: unknown) {
|
||||
failure = new AggregateError(
|
||||
[error, cleanupError],
|
||||
[failure, cleanupError],
|
||||
'subprocess-e2b: command failed and private state cleanup failed',
|
||||
)
|
||||
}
|
||||
@@ -398,8 +398,10 @@ export class E2BSubprocessHandle implements SubprocessHandle {
|
||||
const signal = this.terminationController.signal
|
||||
const ambient = await readRemoteEnvironment(sandbox, signal)
|
||||
this.controlEnvs = bootstrapEnvironment(ambient)
|
||||
await sandbox.files.makeDir(this.stateDir, { signal })
|
||||
// Own the directory before the request: a cancellation racing a committed
|
||||
// creation must still enter cleanup (removal tolerates an absent path).
|
||||
this.stateDirectoryCreated = true
|
||||
await sandbox.files.makeDir(this.stateDir, { signal })
|
||||
await sandbox.commands.run(
|
||||
`chmod 700 -- ${quoteE2BShellArg(this.stateDir)}`,
|
||||
commandOpts(this.controlEnvs, signal),
|
||||
|
||||
@@ -459,7 +459,7 @@ export async function spawnE2BTerminal(
|
||||
runtime: E2BSandboxService,
|
||||
spec: SubprocessTerminalSpawnSpec,
|
||||
stateDir: string,
|
||||
pollMs = 20,
|
||||
pollMs: number,
|
||||
): Promise<E2BTerminalHandle> {
|
||||
const sandbox = await runtime.getSandbox()
|
||||
spec.signal?.throwIfAborted()
|
||||
|
||||
Reference in New Issue
Block a user