Skip to content

fix(sandbox-driver): honor cwd + env passed to SubprocessSandboxDriver constructor#4

Merged
drewstone merged 1 commit into
mainfrom
fix/subprocess-driver-cwd-constructor
Apr 24, 2026
Merged

fix(sandbox-driver): honor cwd + env passed to SubprocessSandboxDriver constructor#4
drewstone merged 1 commit into
mainfrom
fix/subprocess-driver-cwd-constructor

Conversation

@drewstone
Copy link
Copy Markdown
Contributor

@drewstone drewstone commented Apr 24, 2026

Summary

SubprocessSandboxDriver's constructor accepted any-shaped options (no declared interface) and the exec() method only read cwd / env from the per-call HarnessConfig. Calling new SubprocessSandboxDriver({ cwd }) therefore silently did nothing — the subprocess inherited the Node process's cwd instead of the caller's intent.

How this surfaced

End-to-end probing in tangle-network/starter-foundry — the Gen 8 compile-gate thesis is "run pnpm exec tsc --noEmit on the composed scaffold to catch type errors at proposal time." The promoter did:

const driver = new SubprocessSandboxDriver({ cwd: composedOutDir })
// ...
session.ship({ harness: { setupCommand, testCommand, timeoutMs } })

HarnessConfig did not set cwd. The driver read config.cwdundefinedspawn inherited the Node process cwd (the starter-foundry repo root). Every tsc --noEmit ran against starter-foundry's own (clean) source tree, so the build-gate scored 1.0 on every proposal — including one with a verified TS2345 error I planted as a probe.

starter-foundry shipped a workaround (spread cwd into HarnessConfig at call sites — PR #53), but the silent-drop belongs at this layer: a constructor that accepts { cwd } should honor it.

Changes

  • New SubprocessSandboxDriverOptions interface documents the constructor shape.
  • Constructor stores { cwd, env } as defaults.
  • exec() uses config.cwd ?? this.defaultCwd — per-call wins, constructor is the fallback.
  • exec() merges constructor env + per-call env (per-call wins on collision).

Backwards-compatible: callers that only use config.cwd see no change because the fallback is lower-priority.

Tests

+3 in tests/sandbox-harness.test.ts covering:

  • constructor cwd is used when HarnessConfig.cwd is unset
  • per-call HarnessConfig.cwd overrides constructor cwd
  • constructor env merges with per-call env (per-call wins on collision)

319/319 → 322/322. No existing tests changed.

Test plan

  • pnpm build — clean
  • pnpm test — 322/322
  • New test fails without the src change (verified by reverting src temporarily)
  • Once merged: bump to 0.8 + update starter-foundry to prefer the cleaner constructor-cwd pattern

…r constructor

The constructor accepted any-shape options (no declared interface) and
the exec() method only read cwd/env from the per-call HarnessConfig. That
made `new SubprocessSandboxDriver({ cwd })` a silent no-op — the subprocess
inherited the Node process cwd instead of the caller's intent.

This surfaced end-to-end in starter-foundry's promoter: Gen 8's strict
`pnpm exec tsc --noEmit` gate ran against starter-foundry's own (clean)
source tree instead of the composed scaffold being promoted, so every
build-gate scored 1.0 regardless of content. starter-foundry shipped
a workaround (spread cwd into HarnessConfig) but the underlying silent
drop belongs at this layer.

Changes:
- SubprocessSandboxDriverOptions interface documents constructor shape
- Constructor stores { cwd, env } as defaults
- exec() uses config.cwd ?? this.defaultCwd (per-call wins)
- exec() merges constructor env + per-call env (per-call wins on collision)

Backwards compatible: callers that only used config.cwd see no change
because the fallback is lower-priority.

Tests: +3 in tests/sandbox-harness.test.ts covering:
  - constructor cwd is used when HarnessConfig.cwd is unset
  - per-call HarnessConfig.cwd overrides constructor cwd
  - constructor env merges with per-call env (per-call wins on collision)
All 322/322 tests pass (319 → 322).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@drewstone drewstone merged commit 95495f3 into main Apr 24, 2026
drewstone added a commit that referenced this pull request Apr 24, 2026
…r constructor (#4)

The constructor accepted any-shape options (no declared interface) and
the exec() method only read cwd/env from the per-call HarnessConfig. That
made `new SubprocessSandboxDriver({ cwd })` a silent no-op — the subprocess
inherited the Node process cwd instead of the caller's intent.

This surfaced end-to-end in starter-foundry's promoter: Gen 8's strict
`pnpm exec tsc --noEmit` gate ran against starter-foundry's own (clean)
source tree instead of the composed scaffold being promoted, so every
build-gate scored 1.0 regardless of content. starter-foundry shipped
a workaround (spread cwd into HarnessConfig) but the underlying silent
drop belongs at this layer.

Changes:
- SubprocessSandboxDriverOptions interface documents constructor shape
- Constructor stores { cwd, env } as defaults
- exec() uses config.cwd ?? this.defaultCwd (per-call wins)
- exec() merges constructor env + per-call env (per-call wins on collision)

Backwards compatible: callers that only used config.cwd see no change
because the fallback is lower-priority.

Tests: +3 in tests/sandbox-harness.test.ts covering:
  - constructor cwd is used when HarnessConfig.cwd is unset
  - per-call HarnessConfig.cwd overrides constructor cwd
  - constructor env merges with per-call env (per-call wins on collision)
All 322/322 tests pass (319 → 322).
drewstone added a commit that referenced this pull request Apr 24, 2026
PR #4 shipped the same SubprocessSandboxDriver constructor-fallback fix
while this branch was open. Resolved:
- src/sandbox-harness.ts + tests/sandbox-harness.test.ts: take main's version (functionally equivalent; type named SubprocessSandboxDriverOptions instead of SubprocessDriverDefaults — main's name is better, already shipped)
- src/index.ts: export SubprocessSandboxDriverOptions (main forgot to export the new type)
- tests: also fix the env-merge test's printenv form — BSD printenv on macOS only prints the first matched var, making the test platform-flaky. Switch to env|grep which survives missing vars.

Net: keep SKILL.md + README/CLAUDE pointers + version bump + type re-export + macOS test fix. All 322 tests pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant