feat(dsh-box): DeepSeek Harness provider backed by Upstash Box by alitariksahin · Pull Request #228 · upstash/box

alitariksahin · 2026-08-25T13:16:38Z

Adds @upstash/dsh-box, a DeepSeek Harness provider that runs the harness's subprocess seam inside a remote Upstash Box. The harness stays local (agent, model calls, session state, tools); only the process world moves, so dsh-bash-local and anything else built on ctx.subprocess executes in the box with no fork.

Shape

One package, two plugins via subpath exports — the shape dsh profiles already use elsewhere (@deepseek-ai/dsh-headless/startup):

Plugin	`ctx` key	Role
`@upstash/dsh-box`	`ctx.box`	Box lifecycle: create, prepare `cwd`, delete on disposal
`@upstash/dsh-box/subprocess`	`ctx.subprocess`	The seam over Box live exec sessions

dsh.bundle + cordis.patch.yml mount both through dsh plugin add @upstash/dsh-box. The patch disables dsh-base's local provider by id first — Cordis permits one implementation per service, so an insert alone leaves two racing to register ctx.subprocess and the profile fails to boot.

What it implements

spawn, resolveExecutable, bounded collect readers, tree-scoped termination, inherit output, and terminal sessions on a real PTY sized at creation. inspectForeground() resolves the foreground process group from /proc and signalForeground() delivers to that group rather than the session leader, so interrupting a running command leaves the shell alive.

Pinned to the DeepSeek Harness seam at 0.1.1-rc.2, which exports SubprocessRuntime directly.

Verified against a real dsh install

Installed the published CLI (@deepseek-ai/dsh@0.1.1-rc.2) in a clean directory and ran dsh plugin add on the packed tarball. That caught the duplicate-subprocess-row bug above, confirmed the bundle appends to dsh.profile.bundles, and confirmed pnpm pack rewrites workspace:* to @upstash/box@0.7.1.

Tests

Unit (27): bounded reader, environment serialization and removals, and fake-session handle transitions — handshake failure, pre-handshake stdin and termination, abort cleanup, stream settlement, inherit routing, waitForExit cancellation.
Gated live integration via pnpm -r test:integration: separated collect streams with exit code, batch stdin, lossy bounded tail, explicit env crossing while a host sentinel does not, a tombstone genuinely unsetting a box-owned name, terminate() reaping the tree and reporting the delivered signal, box deleted on disposal — plus a terminal session proving /dev/pts, 24 80, the foreground group moving off the shell before SIGINT, and the child dying from that signal rather than from teardown.

Unit-only would not catch a handshake, environment, or tree-kill regression, which is the whole point of this package. describe.skipIf(!process.env.UPSTASH_BOX_API_KEY) keeps it inert without a key.

Environment boundary

A local provider starts from the scrubbed parent environment because parent and child share a machine. This one does not: only entries a spawn asks for explicitly cross into the box, so the host's PATH, HOME, USER, SSH_AUTH_SOCK, and CI variables never reach a remote process implicitly. Removals are applied through an env -u NAME -- argv wrapper, which carries no values, so the server's blocked-name policy still governs everything sent rather than being routed around.

Not implemented

Spill files and a filesystem adapter (ctx.fs still resolves locally, so bash is the workspace). pid is -1 for the handshake window, since spawn() is synchronous in the seam. Sessions cannot be reattached — a dropped connection ends the command, which matches the seam's requirement that a helper process not outlive its handle. All listed in the README.

Release

One changeset (minor), so merging this then the Version Packages PR publishes @upstash/dsh-box@0.1.0.

Adds @upstash/dsh-box, which runs the harness's subprocess seam inside a remote box. The harness stays local: agent, model calls, session state, and tools all run on the caller's machine, and only the process world moves. dsh-bash-local and anything else built on ctx.subprocess execute in the box with no fork, because the seam is the whole integration point. One package ships two plugins through subpath exports, the shape dsh profiles already use elsewhere: @upstash/dsh-box owns the box lifecycle as ctx.box, and @upstash/dsh-box/subprocess implements the seam as ctx.subprocess. The bundle manifest and cordis.patch.yml mount both through `dsh plugin add`. The patch disables dsh-base's local provider by id before inserting the Box one. Cordis allows a single implementation per service, so an insert alone leaves two providers racing to register ctx.subprocess and the profile fails to boot; verified by composing a real profile with `dsh --dump-config`. Covers spawn, resolveExecutable, bounded collect readers, and tree-scoped termination. spawnTerminal, inherit output, spill files, and a filesystem adapter are not implemented; the README lists those alongside the rest. Environment handling deliberately departs from a local provider. A local seam starts from the scrubbed parent environment because parent and child share one machine; here they do not, so only entries a spawn asks for explicitly cross into the box and host ambient values never reach a remote process implicitly. Tested against a real box rather than mocks, since a bounded-reader unit test cannot catch a handshake, environment, or tree-kill regression: the gated integration suite runs through pnpm -r test:integration and asserts separated collect streams with the exit code, batch stdin, a lossy bounded tail, explicit env crossing while a host sentinel does not, terminate reaping the tree and reporting the delivered signal, and disposal deleting the box. Unit tests cover the reader and environment serialization. The published seam is 0.0.1-rc.1, which still exports the subprocess base class as SubprocessService; the harness has since renamed it to SubprocessRuntime, so the import is aliased and the README records it next to the version pin.

linear-code · 2026-08-25T13:16:41Z

DX-2959

Copilot

Pull request overview

Adds @upstash/dsh-box, moving DeepSeek Harness subprocess execution into a remotely managed Upstash Box.

Changes:

Implements Box lifecycle, subprocess execution, output collection, and termination.
Adds Cordis bundle configuration and package documentation.
Adds unit and live integration coverage plus release metadata.

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`.changeset/dsh-box-initial.md`	Defines the initial package release.
`pnpm-lock.yaml`	Locks the new package dependencies.
`packages/dsh-box/.prettierignore`	Configures formatting exclusions.
`packages/dsh-box/.prettierrc`	Defines package formatting rules.
`packages/dsh-box/README.md`	Documents installation, configuration, and limitations.
`packages/dsh-box/cordis.patch.yml`	Mounts the Box and subprocess plugins.
`packages/dsh-box/package.json`	Defines package exports, dependencies, and scripts.
`packages/dsh-box/src/index.ts`	Implements Box creation and lifecycle ownership.
`packages/dsh-box/src/output.ts`	Implements bounded collected-output readers.
`packages/dsh-box/src/process.ts`	Implements Box-backed subprocess handles.
`packages/dsh-box/src/subprocess.ts`	Registers the subprocess service adapter.
`packages/dsh-box/tests/live.integration.test.ts`	Exercises behavior against a live Box.
`packages/dsh-box/tests/output.spec.ts`	Tests output buffering and environment serialization.
`packages/dsh-box/tsconfig.json`	Configures TypeScript compilation.
`packages/dsh-box/vitest.config.ts`	Configures unit tests.
`packages/dsh-box/vitest.integration.config.ts`	Configures live integration tests.

Files not reviewed (1)

pnpm-lock.yaml: Generated file

Suppressed comments (3)

packages/dsh-box/src/process.ts:133

Before the session handshake, this stores the caller's Buffer by reference and immediately acknowledges the write. The caller may legally reuse or mutate that buffer after its callback, corrupting the bytes eventually sent to the box. Copy every queued chunk before acknowledging it.

        const bytes = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk);
        if (this.session === undefined) this.pendingStdin.push(bytes);

packages/dsh-box/src/process.ts:200

Settling done closes the readable sides but leaves piped stdin writable. Writes after process exit are still accepted (and startup failures can keep accumulating pendingStdin), so consumers waiting for stdin closure or retaining the handle see incorrect stream state and retained buffers. Destroy stdin and clear pending input when finishing.

    this.exited = true;
    this.stdout?.push(null);
    this.stderr?.push(null);

packages/dsh-box/src/process.ts:239

When done wins this race, the abort listener remains attached indefinitely. Repeated bounded waits using one retained signal accumulate listeners even though every wait has completed. Remove the handler in a finally block.

        signal.addEventListener(
          "abort",
          () => {
            resolve(false);
          },

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Promise.withResolvers is Node 22, while this package declares Node 18 and CI tests Node 20, so constructing a handle threw on two supported versions. It was invisible because no unit test built a handle: the Node 20 job only exercised the bounded reader, and the live suite pins Node 22. Replaced with a local deferred helper, and the gap that hid it is closed by fake-session unit tests covering handshake failure, pre-handshake stdin and termination, abort cleanup, stream settlement, and waitForExit cancellation. A tombstone now unsets a name instead of blanking it. `KEY=` left the entry present but empty, which is not what the seam means by removal, and the protocol has no removal verb. Removals are applied in the child through an `env -u NAME -- argv` wrapper, which carries no values, so the server's blocked-name policy still governs everything this adapter sends rather than being routed around. Verified against a real box: `${HOSTNAME-absent}` reports absent for a name the container itself sets. The abort listener is now removed when a handle settles. A long-lived controller shared across spawns retained every completed handle and would eventually warn about max listeners. A box whose setup failed is retained so disposal can retry deleting it. If open()'s rollback delete also failed, `ready` stayed rejected and teardown returned without a handle, leaking the remote box on a transient double failure. The README's override example named the disabled local row rather than the inserted one, so following it would remount both providers and reproduce the boot failure the patch exists to avoid. The live suite now scopes cleanup from the moment the owner allocates a box, with owner disposal in its own finally, so neither a setup failure nor a rejecting subprocess disposal can leak the integration box.

…seam Implements the two seam methods Phase 0 left throwing. Both were plugin gaps: the box already allocated PTYs and streamed their bytes, so no backend work was involved. spawnTerminal() opens an exec session with tty set, so the PTY is sized by rows/cols at creation and a shell reports the right dimensions on its first read rather than after a resize. The session protocol carries no foreground notion, so inspectForeground() resolves the group from the session leader's tpgid in /proc and signalForeground() delivers to that group rather than the leader, which is what keeps an interrupt from killing the shell along with the command it was running. The spec's signal cancels allocation, which spans two awaits, so a terminal published after it fires is terminated instead of returned. Teardown is idempotent and reaps the session. inherit writes a remote process's bytes to the harness's own stdout and stderr. A remote process has no descriptor to hand over, so this copies rather than inherits and the child cannot detect a TTY through it; the README says so rather than implying parity with a local spawn. Moves the pin to 0.1.1-rc.2, which exports SubprocessRuntime under its own name, so the alias and its README paragraph are gone and the lockfile matches. The live terminal test waits for the foreground group to move off the shell before signalling, and proves the child died from that signal rather than from teardown. Polling only for a group id above 1 would have passed instantly against the idle shell, sent SIGINT to bash, and still gone green because terminate() reaps the child afterwards. inherit is covered by a unit test that spies on the harness streams instead of writing through to CI logs.

Copilot

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated 7 comments.

Files not reviewed (1)

pnpm-lock.yaml: Generated file

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

packages/dsh-box/src/index.ts:165

Values above Node's maximum timer delay pass validation, but the SDK uses this timeout in setTimeout; Node clamps an overflowing delay to 1 ms. A large configured request timeout therefore causes near-immediate session/request timeouts. Enforce the timer upper bound here.

    if (!Number.isFinite(this.config.requestTimeoutMs) || this.config.requestTimeoutMs <= 0) {
      throw new Error("dsh-box: requestTimeoutMs must be a positive finite number");
    }

packages/dsh-box/tests/live.integration.test.ts:157

The owner is disposed only by the try that begins after subprocess plugin setup. If ctx.plugin(BoxSubprocessRuntime) rejects, the already-created remote box is never disposed and leaks from this integration test. Start the owner try/finally before loading the subprocess plugin, as the first test does.

    const ownerFiber = await ctx.plugin(BoxRuntime, { cwd: CWD });
    const subprocessFiber = await ctx.plugin(BoxSubprocessRuntime, {});

…l fidelity A lost terminal reported success. The SDK returns -1 when a live session's connection closes or errors, while a real exit always carries its code, so resolving that as { exitCode: -1 } left consumers unable to tell a dropped terminal from a clean one. `done` now rejects for it and ends the output stream, with the rejection kept observed so an unwatched terminal cannot take the process down with an unhandled rejection. Termination could settle while a probe was still running. The seam requires that no write, inspection, or signal remains in flight once terminate() resolves, so operations are gated once teardown starts and the ones already running are drained before the session closes; otherwise a probe could race the owner deleting the box. 130 no longer maps to SIGINT. Termination sends TERM then KILL, so a process that handles TERM and exits 130 was being reported as killed by an interrupt nothing delivered. The map now names only the signals this adapter can deliver during termination, in both the process and terminal handles. waitForExit kept its abort listener whenever done won the race, so repeated bounded waits on one long-lived signal retained a listener per call, and an abort landing between the pre-check and registration could leave the race unresolved. The listener is now named, re-checked after registration, and removed in a finally. Terminal dimensions must be positive safe integers. NaN, Infinity, and fractional values do not survive the exec-session request intact, so the PTY could be created at a size the spec never asked for. Folds the two changesets into one, which also drops the stale claim that spawnTerminal and inherit are unimplemented.

Copilot

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.

Files not reviewed (1)

pnpm-lock.yaml: Generated file

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

packages/dsh-box/package.json:67

The PR and README say the provider is pinned to 0.1.1-rc.2, but this caret peer range also accepts later 0.1.x releases, including stable releases that may change the pre-1.0 seam. Declare the exact version if compatibility is intentionally limited to rc.2.

    "@deepseek-ai/dsh-subprocess": "^0.1.1-rc.2",

packages/dsh-box/src/index.ts:186

Box.exec.command() does not reject for a nonzero command exit, so a failed chmod is silently accepted and setup publishes a box without enforcing the intended runtime-root mode. Check the returned exit code and fail setup so the existing rollback deletes the unusable box.

      await box.exec.command(`chmod 700 -- ${quoteBoxShellArg(this.runtimeRoot)}`);

packages/dsh-box/src/subprocess.ts:174

Removing a terminal from the managed set as soon as done settles skips terminate(), which is also responsible for gating and draining already-running foreground probes. If the terminal exits while a probe is in flight, disposal can forget it and the box owner can be deleted before that operation settles. Run terminate() before deleting the terminal, and retain it if cleanup fails.

    const release = (): void => {
      this.terminals.delete(terminal);
    };
    void terminal.done.then(release, release);

packages/dsh-box/tests/live.integration.test.ts:238

If subprocess disposal rejects, the following owner disposal is skipped and the live integration test leaves its allocated box behind. Nest the owner cleanup in finally, as the preceding integration test does.

      await subprocessFiber.dispose();
      await ownerFiber.dispose();

signalForeground reported delivery that never happened. exec.command resolves for a nonzero exit, so a kill against a group that exited between inspection and delivery still returned its id as if signalled. Confirmed against a real box: the failing kill resolves with exit code 1 and "No such process". The exit code is now the evidence, and a failed delivery throws. Terminal allocation ignored its abort signal. The SDK handshake takes no abort, so awaiting it alone meant a cancel went unobserved for up to the full request timeout. Allocation now races the signal and hands a session that lands after we stop waiting to a cleanup that terminates it, since aborting the wait cannot cancel the underlying request. Service disposal could return while a terminal was still being allocated. A setup awaiting getBox() or spawnBoxTerminal is not in `terminals` yet, so snapshotting that set first let the owner delete the box out from under a live allocation. Pending setups are tracked and awaited before teardown proceeds. requestTimeoutMs above the maximum timer delay is rejected. Node clamps such a delay to 1ms, and the SDK uses this value for its request and handshake timers, so a large finite value made requests abort almost immediately rather than waiting as configured. It now shares dsh-timeout's bound with graceMs.

Copilot

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.

Files not reviewed (1)

pnpm-lock.yaml: Generated file

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

packages/dsh-box/tests/live.integration.test.ts:157

The owner allocates a live box before entering the cleanup scope. If subprocess plugin setup rejects, no finally runs; additionally, a rejection from subprocessFiber.dispose() in the current cleanup skips owner disposal. Either path leaks the remote box. Put everything after owner creation in an outer try/finally, with subprocess disposal nested inside it.

    const ownerFiber = await ctx.plugin(BoxRuntime, { cwd: CWD });
    const subprocessFiber = await ctx.plugin(BoxSubprocessRuntime, {});

packages/dsh-box/package.json:68

The README and PR describe these Harness dependencies as pinned to 0.1.1-rc.2, but caret ranges also accept later stable pre-1.0 versions. That can pair this provider with an unverified breaking seam. Use exact peer versions, or update the compatibility claim and test the broader range.

    "@deepseek-ai/dsh-subprocess": "^0.1.1-rc.2",
    "@deepseek-ai/dsh-timeout": "^0.1.1-rc.2"

packages/dsh-box/package.json:12

This published Node package omits the runtime floor even though it explicitly supports Node 18. Both existing published JavaScript packages declare "node": ">=18.0.0" (packages/sdk/package.json:56 and packages/cli/package.json:54). Add the same engine constraint so unsupported runtimes fail at installation rather than at load time.

  "license": "MIT",

…ures Piped stdin could exhaust host memory. Each write callback completed as soon as the chunk landed in a private array, which is exactly the signal the Writable uses to decide it may accept more, so a producer could enqueue unbounded input while a slow handshake was still in flight. The array is gone: callbacks now stay pending until the session accepts the chunk, which is what makes the high water mark real, and they fail if the handshake does. A cancellation raised during the handshake replayed input before delivering the stop, so a command could act on stdin it had already been cancelled for. Termination is now checked first and the queued or batch input is discarded. done resolved { exitCode: -1 } for a lost connection. The SDK uses -1 when a live session ends without an exit frame, while a real exit always carries its code, so this misreported a transport failure as a process outcome. It now rejects, matching the terminal handle, with the rejection kept observed for callers that only use waitForExit. The terminal's input-wait probe matched tty_write as well as tty_read, so a process blocked on output backpressure was reported as waiting for input, which contradicts both the seam and the README. Only a tty read counts.

Failing a write callback destroys the Writable and emits "error", so a test that only observed the callback left the emission unhandled and Node escalated it. The case now attaches a listener, which is what a piped consumer has to do, and the README says so.

Copilot

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

pnpm-lock.yaml: Generated file

Suppressed comments (3)

packages/dsh-box/src/subprocess.ts:171

terminalSetups settles as soon as the session is allocated, before the post-allocation disposal check runs. If disposal is waiting here, it can resume, find the terminal absent from terminals, and return while terminal.terminate() is still running; the box owner may then delete the box underneath that cleanup. Keep the tracked setup pending through the disposal check/rollback and terminal registration.

    const setup = (async (): Promise<SubprocessTerminalHandle> => {
      const box = await this.ctx.box.getBox();
      return await spawnBoxTerminal(box, spec);
    })();

packages/dsh-box/src/subprocess.ts:170

An abort that arrives while getBox() is provisioning the box is not observed until that await completes; the abort race in spawnBoxTerminal starts only afterward. Because this signal cancels terminal allocation, callers can otherwise remain blocked for the full request timeout after cancellation. Race this acquisition against spec.signal as well, leaving any late box result owned by BoxRuntime.

      const box = await this.ctx.box.getBox();
      return await spawnBoxTerminal(box, spec);

packages/dsh-box/tests/live.integration.test.ts:159

The owner has already allocated a real remote box before the subprocess plugin is awaited, but the try starts only after that await. If subprocess plugin setup rejects, the finally is never entered and the box is leaked. Begin the protected region immediately after acquiring ownerFiber, and dispose subprocessFiber conditionally when it was created.

    const ownerFiber = await ctx.plugin(BoxRuntime, { cwd: CWD });
    const subprocessFiber = await ctx.plugin(BoxSubprocessRuntime, {});

    try {

Service disposal used Promise.all, which rejects the moment one cleanup fails while the others keep running. Disposal therefore returned early, and the owner teardown is a separate effect, so the shared box could be deleted while another handle was still terminating. Every attempt now settles first and the failures are reported together, one rethrown as-is and several as an AggregateError. The regression test turns on timing rather than the outcome: one handle fails its cleanup immediately while a sibling takes longer to exit, and disposal must not return until the slow one has closed. Asserting the rejection would not have worked, since cordis absorbs an effect's rejection, and asserting only that the sibling was cleaned up passes either way.

Copilot

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.

Files not reviewed (1)

pnpm-lock.yaml: Generated file

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

packages/dsh-box/package.json:67

The PR and README say the provider is pinned to 0.1.1-rc.2, but this caret range also accepts later 0.1.x releases. Since the package explicitly warns that this pre-1.0 seam may break, consumers can otherwise resolve an incompatible seam without changing this package.

    "@deepseek-ai/dsh-subprocess": "^0.1.1-rc.2",

packages/dsh-box/tests/live.integration.test.ts:157

The box owner is allocated before this second plugin setup, but the cleanup try starts afterward. If subprocess plugin construction rejects, the test exits without disposing ownerFiber, leaking the remote box. Enter an outer try/finally immediately after creating the owner, as the first test does.

This issue also appears on line 237 of the same file.

    const subprocessFiber = await ctx.plugin(BoxSubprocessRuntime, {});

packages/dsh-box/src/process.ts:189

endStdin() has the same close race as write(): if the SDK throws while the socket is closing, this promise handler rejects without ever invoking callback, so stream finalization hangs. Catch the synchronous transport error and settle the callback.

            if (!this.exited) session.endStdin();

packages/dsh-box/tests/live.integration.test.ts:238

If subprocess disposal rejects, the following owner disposal is skipped and the remotely billed box leaks. The first integration case deliberately nests these disposals for exactly this failure mode; apply the same pattern here so box deletion always runs.

      await subprocessFiber.dispose();
      await ownerFiber.dispose();

Exit outcomes no longer name a signal. The session protocol reports an exit code and nothing else, and a requested stop does not prove which signal produced that code: a process can catch SIGTERM and exit 143 or 137 on its own. Naming a signal there fabricated a fact and, worse, replaced the real exit code with null. Both handles now pass the server's code through, and the README says why. Populating `signal` honestly needs the exec-session protocol to carry one. This reverses an earlier review instruction to map 143/137 onto SIGTERM/SIGKILL when termination was requested. The trade is real either way; preserving the fact the server actually reported wins over reconstructing one it did not. Terminal setup now tracks the whole transaction rather than just allocation. The tracked promise used to settle as soon as the allocation returned, while the disposal branch was still awaiting terminate(), so disposal could finish and the owner delete the box mid-teardown. A synchronous throw from session.write() inside the stdin continuation skipped its callback, which wedges the Writable permanently and surfaces as an unhandled rejection rather than a stream error. Both the write and end paths now complete the callback with the transport error.

Copilot

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.

Files not reviewed (1)

pnpm-lock.yaml: Generated file

Suppressed comments (5)

Previously missed (2) — in code that hasn't changed since the last review.

packages/dsh-box/src/process.ts:124

These constructors discard the collect mode's spill request. In the pinned seam, SubprocessCollect.spill requests recoverable full output, and dsh-bash-local includes it for both collected streams; once output exceeds maxBytes, normal bash results therefore become irrecoverably truncated with no spillPath. Implement the requested spill behavior, or fail unsupported specs explicitly rather than silently weakening them.

    this.stdoutReader = isCollect(spec.stdio.stdout)
      ? new BoxOutputReader(spec.stdio.stdout.maxBytes)
      : undefined;
    this.stderrReader = isCollect(spec.stdio.stderr)
      ? new BoxOutputReader(spec.stdio.stderr.maxBytes)

packages/dsh-box/package.json:73

This is not pinned to 0.1.1-rc.2 as the PR and README state. A caret prerelease range can accept the stable 0.1.1 and later 0.1.x releases, despite the documented expectation of breaking pre-1.0 seam changes. Use exact versions for the seam packages in both peer and development dependencies.

    "@deepseek-ai/dsh-subprocess": "^0.1.1-rc.2",

packages/dsh-box/src/process.ts:172

A piped write queued during the handshake is released after sessionReady even when terminate() was requested meanwhile. The code sends the terminate frame and then this continuation sends stdin, allowing a TERM-trapping command to act on input after cancellation. Drop queued and subsequent input once termination has begun.

              if (!this.exited) session.write(bytes);

packages/dsh-box/src/subprocess.ts:117

The abort signal is checked only after this remote request settles, so aborting during lookup can still block for the configured request timeout (ten minutes by default). The subprocess seam requires the signal to abort remote lookup; race both exec.command branches against it while safely observing the late request.

      const probe = await box.exec.command(
        `test -f ${quoteBoxShellArg(command)} -a -x ${quoteBoxShellArg(command)} && echo ok`,
      );

packages/dsh-box/tests/live.integration.test.ts:245

If subprocess disposal rejects, the following owner disposal is skipped and the live integration test leaks its remote box. The first test deliberately nests cleanup to avoid this same failure mode; preserve owner cleanup here as well.

      await subprocessFiber.dispose();
      await ownerFiber.dispose();

…l setups The publish job's package -> directory case statement had no branch for @upstash/dsh-box, so a release would exit 1 with "Unknown package". Disposal awaited in-flight terminal setups but never cancelled them, so a handshake that stalls held teardown for the full request timeout. Each setup now carries an AbortController combined with the caller's signal, aborted before the allSettled wait.

Copilot

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated 2 comments.

Files not reviewed (1)

pnpm-lock.yaml: Generated file

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

packages/dsh-box/src/subprocess.ts:140

command -v prefers shell builtins, so common bare executable names such as echo, printf, and test produce a bare word here. The validation below then rejects them even when /usr/bin/echo (etc.) exists on PATH, which breaks the advertised bare-PATH lookup. Use a path-only lookup (for example Bash type -P) or scan PATH and validate executable files.

    const result = await box.exec.command(
      `cd ${quoteBoxShellArg(this.ctx.box.cwd)} && ${prefix}command -v -- ${quoteBoxShellArg(command)}`,
    );

packages/dsh-box/src/process.ts:174

This queued write is released after the handshake even when terminate() was requested beforehand. start() sends termination and then resolves sessionReady; because exited is usually still false, pre-handshake piped input is delivered to a command that was already cancelled (unlike batch input, which is explicitly dropped). Also gate the write on terminateRequested.

            try {
              if (!this.exited) session.write(bytes);
              callback();

packages/dsh-box/package.json:68

The PR and README say the provider is pinned to 0.1.1-rc.2, but a caret prerelease range also accepts later compatible releases (including stable 0.1.x versions). Since this pre-1.0 seam is expected to break, consumers can install an unverified API despite the stated pin. Use exact versions for both seam peers/dev dependencies and update the lockfile specifiers.

    "@deepseek-ai/dsh-subprocess": "^0.1.1-rc.2",
    "@deepseek-ai/dsh-timeout": "^0.1.1-rc.2"

… Node 18 Disposal terminated and waited on every live handle, but terminate() can only set a flag while box.exec.session() is in flight, so waitForExit() blocked on an unanswered handshake for the whole request timeout. Handles without a session are now abandoned at teardown, and a session that lands afterwards is terminated and closed instead of left running. AbortSignal.any is Node 18.17+, while the package supports Node 18 (the reason deferred.ts exists), so a caller-supplied signal threw before allocation even started on older runtimes. Replaced with a local anySignal() helper whose dispose() detaches listeners, and declared engines >=18.0.0 to match the other packages in the repo.

Copilot

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated 3 comments.

Files not reviewed (1)

pnpm-lock.yaml: Generated file

Suppressed comments (4)

Previously missed (3) — in code that hasn't changed since the last review.

packages/dsh-box/package.json:70

^0.1.1-rc.2 is not the pin described by the PR and README; it permits later stable 0.1.x releases. Since the seam is explicitly pre-1.0 and expected to make breaking changes, consumers can receive an untested incompatible interface. Use the exact release-candidate version until compatibility with a wider range is verified.

    "@deepseek-ai/dsh-subprocess": "^0.1.1-rc.2",

packages/dsh-box/tests/live.integration.test.ts:224

A PTY echoes the input line, so seen can contain STILL-ALIVE even if bash never executes the command. Use an output marker that does not appear verbatim in the typed input so this actually proves the shell survived the foreground interrupt.

      await terminal.write("echo STILL-ALIVE\n");
      await expect.poll(() => seen, { interval: 100, timeout: 20_000 }).toContain("STILL-ALIVE");

packages/dsh-box/tests/live.integration.test.ts:245

If subprocess disposal rejects, execution skips ownerFiber.dispose() and leaks the live box. Nest the cleanup so owner disposal always runs, matching the safer pattern already used by the preceding integration test.

      await subprocessFiber.dispose();
      await ownerFiber.dispose();

packages/dsh-box/src/subprocess.ts:215

The setup cancellation does not cover getBox(): an abort/disposal while that promise is pending still waits for it, and then spawnBoxTerminal starts a session before its abort race is installed. This can hold disposal for the full request timeout and briefly allocate a process after cancellation. Race the getBox() wait against allocation.signal and recheck the signal before opening the session.

      const box = await this.ctx.box.getBox();
      const allocated = await spawnBoxTerminal(box, { ...spec, signal: allocation.signal });

…l output abandon() rejected done but left sessionReady pending, so a piped stdin write queued before the handshake kept its callback (and the writable's backpressure) pending forever. It now rejects with the same disposal error. Pipe and terminal output ignored Readable.push() returning false. The session transport has no pause, so unlike a local child pipe the producer cannot be slowed and a consumer that stops reading grows host memory without bound. Both paths now fail once more than MAX_UNREAD_OUTPUT_BYTES sits unread, terminating the process that is filling the buffer. The stream is ended rather than destroyed: nothing is reading in that scenario, so an error event would likely be unhandled and take the host process down, while done is always observed. Proper transport flow control needs pause/resume in @upstash/box and the agent protocol, which this adapter cannot add on its own.

Copilot

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated 3 comments.

Files not reviewed (1)

pnpm-lock.yaml: Generated file

Suppressed comments (6)

Previously missed (3) — in code that hasn't changed since the last review.

packages/dsh-box/src/subprocess.ts:129

The abort signal is only checked after this remote request settles, so aborting during a stalled lookup still blocks for up to requestTimeoutMs (10 minutes by default). The subprocess seam requires the signal to abort lookup; race this request against the signal (and safely observe the late request) so callers regain control promptly.

This issue also appears on line 143 of the same file.

      const probe = await box.exec.command(
        `test -f ${quoteBoxShellArg(command)} -a -x ${quoteBoxShellArg(command)} && echo ok`,
      );

packages/dsh-box/package.json:70

This peer dependency is not actually pinned as described: ^0.1.1-rc.2 also accepts the stable 0.1.1 and later 0.1.x releases. Since the package targets this exact pre-1.0 seam and warns that later revisions may break, publish an exact peer version rather than silently allowing incompatible seam revisions.

This issue also appears on line 75 of the same file.

    "@deepseek-ai/dsh-subprocess": "^0.1.1-rc.2",

packages/dsh-box/tests/live.integration.test.ts:160

The owner has already allocated a live box before the subprocess plugin is installed, but the cleanup try starts only afterward. If plugin installation regresses and rejects, this test exits without disposing the owner and leaks the remote box. Put subprocess setup inside an owner-level try/finally.

This issue also appears on line 244 of the same file.

    const ownerFiber = await ctx.plugin(BoxRuntime, { cwd: CWD });
    const subprocessFiber = await ctx.plugin(BoxSubprocessRuntime, {});

packages/dsh-box/src/subprocess.ts:145

This PATH lookup has the same cancellation gap as the absolute-path probe: signal is not observed while exec.command() is pending. A deadline abort can therefore leave resolveExecutable() blocked until the SDK request timeout instead of aborting lookup promptly.

    const result = await box.exec.command(
      `cd ${quoteBoxShellArg(this.ctx.box.cwd)} && ${prefix}command -v -- ${quoteBoxShellArg(command)}`,
    );

packages/dsh-box/tests/live.integration.test.ts:245

If subprocess disposal throws, the following owner disposal is skipped and the live integration test leaks its remote box. The first test correctly nests these cleanups; apply the same try { await subprocessFiber.dispose(); } finally { ... } pattern here.

      await subprocessFiber.dispose();
      await ownerFiber.dispose();

packages/dsh-box/package.json:75

The development dependency also uses the broad caret range, so regenerating the lockfile can compile and test against a later 0.1.x seam even though the package claims to target rc.2 exactly. Keep the dev dependency pinned to the same exact version as the peer dependency.

    "@deepseek-ai/dsh-subprocess": "^0.1.1-rc.2",

…utput The output ceiling settled the handle itself, which broke two seam contracts: done rejects only for spawn-level failures, and waitForExit() means the tree exited. Marking the handle exited on overflow reported quiescence while the TERM-to-KILL escalation was still running, and let the terminal's terminate() skip awaiting session.wait() entirely. Settlement now stays tied to session.wait() on both paths. A piped stream is destroyed with the reason (the seam gives piped streams to the caller, and done cannot carry it), while the terminal records the failure and rejects at real exit, which its contract allows for a live transport failure. inherit had no ceiling at all: it writes to the harness's own stream, whose buffer grows just as unboundedly when the destination drains slower than the box produces. It now stops copying and terminates the process, without ever ending a stream it does not own.

…ment Nothing awaits an abandoned session's exit. The SDK resolves it with -1 on close today, so attaching a handler keeps a future rejection from surfacing as an unhandled one. Pinned by asserting on the process unhandledRejection event, since an unhandled rejection does not fail a vitest run by itself. The live spawn test still claimed terminate() reports the delivered signal, which stopped being true when signal inference was removed.

Copilot

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

pnpm-lock.yaml: Generated file

Suppressed comments (7)

Previously missed (6) — in code that hasn't changed since the last review.

packages/dsh-box/package.json:70

The package is described as pinned to the 0.1.1-rc.2 seam, but this caret range also accepts stable 0.1.1 and later 0.1.x releases. Because this pre-1.0 seam is explicitly expected to break, a future Harness install can satisfy the peer range while exposing an incompatible API. Use the exact version for the published compatibility contract.

    "@deepseek-ai/dsh-subprocess": "^0.1.1-rc.2",

packages/dsh-box/src/subprocess.ts:129

The pinned seam specifies that signal aborts executable lookup, but getBox() and both exec.command() calls are awaited directly. An abort during box startup or an HTTP request therefore leaves the caller blocked for up to requestTimeoutMs (10 minutes by default), and may still launch a probe after cancellation. Race every remote await against the signal and detach the abort listener when either side settles.

    const box = await this.ctx.box.getBox();
    if (posix.isAbsolute(command)) {
      const probe = await box.exec.command(
        `test -f ${quoteBoxShellArg(command)} -a -x ${quoteBoxShellArg(command)} && echo ok`,
      );

packages/dsh-box/src/subprocess.ts:239

Removing the terminal as soon as done settles skips terminate()'s in-flight-operation drain. If the shell exits while inspectForeground() or signalForeground() is still awaiting a probe, service disposal no longer sees this terminal and the box owner can delete the box under that request. Keep the terminal tracked until terminate() has drained operations; retain it when automatic cleanup fails so disposal can retry.

    const release = (): void => {
      this.terminals.delete(terminal);
    };
    void terminal.done.then(release, release);

packages/dsh-box/src/index.ts:35

Message matching can misclassify a non-404 deletion failure as success and silently leak the box. The SDK exposes statusCode on BoxError, and the existing repository pattern uses the exact 404 check (packages/box-pi/src/box.ts:39). Restrict this predicate to the status code instead of arbitrary error text.

function isAlreadyGone(error: unknown): boolean {
  if ((error as { statusCode?: number } | undefined)?.statusCode === 404) return true;
  const message = error instanceof Error ? error.message.toLowerCase() : "";
  return message.includes("not found") || message.includes("does not exist");
}

packages/dsh-box/tests/live.integration.test.ts:247

If subprocess disposal rejects, the following owner disposal is skipped and the live remote box can leak. Nest the cleanup so box deletion is attempted regardless of the subprocess teardown result, as the first integration test already does.

    } finally {
      await subprocessFiber.dispose();
      await ownerFiber.dispose();
    }

packages/dsh-box/tests/live.integration.test.ts:161

The owner allocates a remote box before this second plugin is created, but the cleanup try starts only afterward. If subprocess plugin setup rejects, execution never reaches the finally and the box is leaked. Start the outer try immediately after ownerFiber is acquired and make subprocessFiber optional for cleanup.

    const ownerFiber = await ctx.plugin(BoxRuntime, { cwd: CWD });
    const subprocessFiber = await ctx.plugin(BoxSubprocessRuntime, {});

packages/dsh-box/src/subprocess.ts:215

Cancellation is not observed while getBox() is pending. Disposal aborts allocation.signal and then waits for terminalSetups, so a setup blocked here can hold teardown for the full box request timeout—the exact stalled-setup case the surrounding logic intends to avoid. Race getBox() against allocation.signal before starting the terminal handshake.

      const box = await this.ctx.box.getBox();
      const allocated = await spawnBoxTerminal(box, { ...spec, signal: allocation.signal });

…irectory Driving a real dsh profile end to end showed the first wall a new integration hits: the bash tool passes the harness's own session cwd, which is a path on the host, and the box answers that with "could not determine session pid" -- naming neither the path nor the cause. macOS paths cannot be created in the box at all, since the exec user does not own /. A failed handshake now probes the working directory once and, when that is the reason, says so. The probe costs a round trip only on a path that already failed, and any other failure surfaces unchanged. README documents the two stock-profile assumptions that break a remote execution world: the host-side working directory, and the local confinement runner (sandbox-exec / bwrap) that cannot exist inside the box.

Copilot

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated no new comments.

Files not reviewed (1)

pnpm-lock.yaml: Generated file

Suppressed comments (6)

Previously missed (4) — in code that hasn't changed since the last review.

packages/dsh-box/src/process.ts:175

A piped write queued during the session handshake is still replayed after terminate() or an abort. The handshake path sends the termination request and then resolves sessionReady; because exited is still false until the remote process reports its exit, this callback writes the queued bytes into a process that was already cancelled. Apply the same drop policy used for batch stdin by checking terminateRequested here.

              if (!this.exited) session.write(bytes);

packages/dsh-box/src/subprocess.ts:239

Removing the terminal immediately when done settles can drop it while an inspectForeground() or signalForeground() command is still in inFlight. Disposal then omits this handle and the owner can delete the box before that operation settles. Call terminate() before deleting the terminal from the set; for an already-exited terminal it drains in-flight operations and closes the session without signaling it again.

    const release = (): void => {
      this.terminals.delete(terminal);
    };
    void terminal.done.then(release, release);

packages/dsh-box/src/subprocess.ts:128

resolveExecutable only checks the signal before and after each remote await. An abort during box readiness or exec.command() therefore leaves the lookup pending for up to requestTimeoutMs, even though the subprocess API specifies that this signal aborts remote lookup. Race each remote await against the signal and detach the abort listener when either side settles.

    const box = await this.ctx.box.getBox();
    if (posix.isAbsolute(command)) {
      const probe = await box.exec.command(
        `test -f ${quoteBoxShellArg(command)} -a -x ${quoteBoxShellArg(command)} && echo ok`,

packages/dsh-box/package.json:71

The PR and README state that the provider is pinned to 0.1.1-rc.2, but caret ranges are not pins: ^0.1.1-rc.2 also accepts later prereleases and stable 0.1.x releases. Since this pre-1.0 seam is explicitly expected to make breaking changes, use exact peer/dev versions (and update the lockfile) or revise the documented compatibility claim.

    "@deepseek-ai/dsh-subprocess": "^0.1.1-rc.2",
    "@deepseek-ai/dsh-timeout": "^0.1.1-rc.2"

packages/dsh-box/src/subprocess.ts:215

The allocation signal does not cover getBox(). If box creation/readiness is stalled, caller cancellation and service disposal both leave this setup pending until the full request timeout, despite the surrounding code claiming cancellation stops the allocation wait. Race this await against allocation.signal as well, with listener cleanup after either branch settles.

      const box = await this.ctx.box.getBox();
      const allocated = await spawnBoxTerminal(box, { ...spec, signal: allocation.signal });

packages/dsh-box/tests/live.integration.test.ts:246

If subprocess disposal rejects, the owner disposal is skipped and this credential-gated test can leak a live box—the first test already nests cleanup specifically to avoid that failure mode. Put owner disposal in a finally around subprocess disposal.

      await subprocessFiber.dispose();
      await ownerFiber.dispose();

alitariksahin requested a balanced review from Copilot August 25, 2026 13:17

Copilot started reviewing on behalf of alitariksahin August 25, 2026 13:18 View session