fix: clean up packed smoke processes - #62
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the packed-install smoke infrastructure to ensure subprocesses (mock API + any spawned commands) are reliably cleaned up after cancellation or failure, while preserving conventional signal exit codes and prioritizing primary failures over cleanup failures.
Changes:
- Refactors
smoke-packed-install.mtsto run subprocess commands asynchronously under a shared interruption/abort controller, and to perform bounded cleanup on exit. - Introduces a reusable
process-lifecycle.mtshelper (plus Node test coverage) to centralize interruption capture, process-group termination, and outcome precedence. - Adjusts the test runner configuration so the new Node-based lifecycle tests run in the packed-smoke gate but are excluded from the existing Vite/Vitest test discovery.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.ts | Excludes scripts/**/*.test.* from Vite/Vitest discovery so Node test fixtures aren’t picked up inadvertently. |
| scripts/smoke-packed-install.mts | Converts packed-install smoke to async execution with shared abort signal and structured cleanup/exit-code resolution. |
| scripts/process-lifecycle.test.mts | Adds Node node:test coverage for signal handling, process-group termination, and lifecycle outcome precedence. |
| scripts/process-lifecycle.mts | Adds lifecycle utilities for interruption handling and “owned” subprocess termination with TERM→KILL escalation. |
| package.json | Runs the new lifecycle Node tests as part of smoke:pack before executing the packed-install smoke. |
Suppressed comments (1)
scripts/process-lifecycle.mts:68
- On Unix this always signals
-child.pid(process group). If the child wasn't spawned detached / as its own process-group leader, this can fail with ESRCH and leave the actual child process running. Consider falling back to signaling the individual PID when the group signal fails.
try {
if (process.platform === "win32") {
if (!hasExited(child)) child.kill(signal);
} else {
process.kill(-child.pid, signal);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
putio-releaser Bot
added a commit
that referenced
this pull request
Aug 15, 2026
## [1.6.2](v1.6.1...v1.6.2) (2026-08-15) ### Bug Fixes * clean up packed smoke processes ([#62](#62)) ([d6f496c](d6f496c))
Contributor
|
🎉 This PR is included in version 1.6.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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
Prevent the packed-install smoke from leaving its exact mock API child behind after failure or interruption.
Changed
Risks
The change is limited to contributor smoke infrastructure and one directly owned child; it does not add a general lifecycle framework.
Verification
pnpm exec vp run verifyComplexity
One existing script, +97/-20 lines.