Repository navigation
fix: capture killed/aborted on record; remove internal export leakage - #130
Merged
Merged
Conversation
#129 Capture `killed` separately from `signal`. Replay synth previously derived `killed` from `signal !== null`, conflating "subprocess.kill() was called" with "process was terminated by any signal". An external SIGTERM has signal !== null but killed === false on the real result; replay reported killed: true. Both adapters now store `r.killed === true` on capture; serialize emits the field when defined; synth uses `rec.result.killed ?? signal !== null` so cassettes recorded before this change auto-upgrade. #126 tinyexec record path: snapshot `aborted`/`killed` from the ExecProcess before await. tinyexec's Result is `PromiseLike<Output> & OutputApi`; awaiting drops the OutputApi getters, so the previous adapter recorded aborted: false / killed: false on every tinyexec call regardless of outcome. The adapter's realCall now grabs the ExecProcess, awaits it, then enriches the resolved Output with proc.aborted / proc.killed before captureResult runs. #102 Skip the inputFile double-read on record. RunnerHooks.realCall gains a fourth parameter `resolvedStdin` (Call.stdin when buildCall ran, undefined otherwise). The execa adapter swaps `options.inputFile` for `input: resolvedStdin` when both are set, so real execa does not re-read a file buildCall already consumed. #128 Move captureResult into non-exported modules. The `_captureResultForTesting` re-exports leaked into the published `.d.ts` for shell-cassette/execa and shell-cassette/tinyexec; each adapter now imports captureResult from a sibling (src/execa-capture.ts, src/tinyexec-capture.ts) that is not in package.json's exports map. Tests import the same path. Schema stays additive: cassettes without the `killed` field load and replay correctly via the synth fallback. RunnerHooks lives in src/wrapper.ts, which is not in the package exports map, so the signature change is internal.
Removes inline `Closes #N` / `Fixes #N` lines from comments in src/execa.ts, src/tinyexec.ts, src/types.ts, and the new tests/integration/record-replay-aborted.test.ts banner. Issue closures belong in commit messages and PR bodies, not source. Also rewords the killed-field JSDoc on Result to describe the semantic distinction directly instead of pointing at the issue that surfaced it, and replaces one em-dash in the new wrapper test with a colon.
The project's tone-and-voice rules ban em-dashes and a short list of slangy phrases (`landed`, `lands here`, `load-bearing`) in any prose that ships with the package: source comments, JSDoc, docs, and the PR template. This sweep replaces every remaining occurrence with commas, colons, semicolons, or parentheses as the surrounding context fits. Also brings two doc tables current with the recent shape-fidelity fixes: `docs/execa.md`'s `killed` row now describes the captured field with its legacy fallback (was: "Derived: signal !== null"), and `docs/tinyexec.md` drops the paragraph that flagged `aborted: false` record-side capture as outstanding (the adapter now snapshots aborted/killed pre-await, so the cassette stores real values). No source-logic or test-behavior changes; lint, typecheck, full test suite all clean.
slgoodrich
added a commit
that referenced
this pull request
May 3, 2026
Patch release covering two bug-fix PRs since v0.6.0: - #127 (replay shape fidelity): timedOut, failed-derivation correctness, and execa subprocess-API stubs - #130 (record-side flag capture): killed/aborted captured correctly on both adapters; inputFile double-read retired; internal export leakage removed Cassette schema stays at version 2; new Result fields are JSON-additive.
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
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
Bundles four open issues into one PR (#72 and #81 deferred as features).
killedseparately fromsignalaborted/killedfromExecProcesspre-awaitinputFiledouble-read by threadingCall.stdinthroughrealCallcaptureResultinto non-exported modules so_captureResultForTestingno longer leaks into the published.d.tsWhy
#129: replay synth conflated
killedwithsignal !== nullexeca's
killedis true only whensubprocess.kill()was called. A process terminated by an external signal hassignal !== nullbutkilled === falseon the real result. Replay synth derivedkilled: signal !== null, so cassettes recorded for externally-signaled processes replayed with the wrongkilledvalue. Both adapters now storer.killed === trueon capture;serializeemitskilledwhen defined; synth usesrec.result.killed ?? signal !== nullso cassettes recorded before this change still replay correctly.#126: tinyexec recorded
aborted: false/killed: falseon every calltinyexec's
Result = PromiseLike<Output> & OutputApi. Awaiting it drops the OutputApi getters (aborted,killed) that live on theExecProcess. The adapter previously read those on the awaited Output and gotundefined, so every cassette storedaborted: false/killed: falseregardless of the call's outcome. The adapter'srealCallnow grabs theExecProcesssynchronously, awaits it for the Output, then enriches the resolved value withproc.aborted/proc.killedbeforecaptureResultruns.#102:
inputFilewas read twice on the record pathbuildCallreadsinputFileto populateCall.stdin(for canonical matching). Real execa then re-reads the same file because we forwarded the original options.RunnerHooks.realCallgains a fourth parameterresolvedStdin:call.stdinon the record path,undefinedon passthrough/no-session paths. The execa adapter swapsoptions.inputFileforinput: resolvedStdinwhen both are present, so real execa consumes the resolved bytes instead of re-reading the file.#128:
_captureResultForTestingleaked into public sub-pathsThe underscore-prefixed test export appeared in the published
.d.tsforshell-cassette/execaandshell-cassette/tinyexec. Each adapter now importscaptureResultfrom a sibling internal module (src/execa-capture.ts,src/tinyexec-capture.ts) that is not inpackage.json'sexportsmap. Tests import the same path.Schema and API
Schema stays additive. Cassettes without the
killedfield load and replay correctly via the synth fallback. Format stays at version 2.RunnerHookslives insrc/wrapper.ts, which is not in the package'sexportsmap, so the signature change (4th param) is internal.Related Issues
Closes #126
Closes #128
Closes #129
Closes #102
Test Plan
npm run lintnpm run typechecknpm test(777 passing, 2 skipped)npm run buildtests/integration/record-replay-aborted.test.tsextended to cover tinyexectests/unit/execa-capture-result.test.tsandtinyexec-capture-result.test.tscover the newkilledfieldtests/integration/wrapper.test.tscovers synth fallback (legacy cassette withoutkilled, new cassette withkilled: falseandsignal: 'SIGTERM')tests/unit/wrapper-resolved-stdin.test.tscoversresolvedStdinthreading on all five wrapper pathstests/unit/execa-input-file-swap.test.tscovers the execa adapter'sinputFile->inputswaptests/unit/tinyexec-adapter.test.tsupdated for the pre-await snapshot contractNotes
tests/integration/record-replay-aborted.test.tspreviously had a banner deferring tinyexec coverage to #126. That banner is removed; the file now covers both adapters.The two tests in
tests/unit/tinyexec-adapter.test.tsthat asserted onaborted: true/killed: truewere rewritten to mock the realistic shape: aPromisecarrying the OutputApi fields as own properties, mirroring how real tinyexec exposes them. The previous mocks (flat object onmockResolvedValueOnce) implicitly tested the buggy path.