executor: replace custom spawn engine with execa wrapper - #20
Merged
Conversation
Drop the hand-rolled cancellable-timeout / output-collector / close-handler
plumbing in src/executor.ts; delegate process spawning, timeout enforcement,
and output buffering to execa@5. Public surface (CommandExecutor.execute,
executeWithRedirect, cleanup, registerTempFile, getTempFiles) is unchanged so
no helper call sites had to move.
Behavior preserved:
- ulimit -v / -Xmx memory-limit wrapping
- MLE detection from exit 137, SIGABRT, and stderr patterns
- cross-platform process-tree kill (taskkill /T /F on Windows,
process-group SIGKILL on Unix) layered on top of execa's per-process kill
- onSuccess / onError / onTimeout / onMemoryExceeded callback semantics
- Windows file-handle release delay
- SIGINT/SIGTERM/uncaught/unhandled cleanup hooks
tests/executor.test.ts rewritten to mock execa (default export) instead of
child_process.spawn. The behavioral surface area covered is identical; the
mock just stops emitting close/error events on a fake ChildProcess and
instead resolves a fake ExecaReturnValue.
src/executor.ts: 1003 -> 466 lines.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
child_process.spawnplumbing insrc/executor.tswith a thin wrapper overexeca@5. Public surface (execute,executeWithRedirect,cleanup,registerTempFile,getTempFiles) is byte-for-byte identical, so every helper call site (helpers/{checker,validator,generator,solution,utils}.ts) is untouched.src/executor.ts: 1003 → 466 lines. Gone: the cancellable-timeout dance, manual stdout/stderr collectors, close/error event wiring, and the resolve/reject relay throughhandleProcessClose/handleProcessError/handleTimeout.ulimit -v/-Xmxmemory limits, MLE detection (exit 137, SIGABRT,bad_alloc/OutOfMemory/OOM/MemoryErrorin stderr), cross-platform process-tree kill (taskkill /T /Fon Windows, process-group SIGKILL on Unix) layered on top of execa's per-process kill, all four callbacks (onSuccess/onError/onTimeout/onMemoryExceeded), Windows file-handle release delay, signal-handler cleanup.tests/executor.test.tsrewritten to mockexecainstead ofchild_process.spawn. Same behavioral coverage (47 tests, all the success/failure/timeout/MLE/redirect/cleanup/platform-specific assertions); the mock just resolves a fakeExecaReturnValueinstead of emitting events on a fakeChildProcess.Test plan
npm run build— cleannpm run lint— cleannpm run format:check— cleannpx vitest run— 726/726 passing across 20 files (47 inexecutor.test.ts)polyman new /tmp/x && cd /tmp/x && polyman download-testlib && polyman verifyagainst a sample problem (Linux)taskkill /T /F(the gnarly path called out inNOTES.md)🤖 Generated with Claude Code