Skip to content

Conversation

@o-az
Copy link
Collaborator

@o-az o-az commented Nov 4, 2025

This improves on the current process termination flow for running foundry tools through node/npm.

Without the fix, the wrapper swallows SIGINT/SIGTERM: the parent exits with 0 and the child gets a SIGTERM, so callers can’t tell a tool was interrupted and the tool never sees Ctrl+C. Forwarding the exact signal and then re‑emitting it locally restores the original behavior. Tools handle signals themselves and the wrapper exits with the standard 130/143 codes.

How I verified this:

  1. Create a temp js file: /tmp/test.mjs:
import { spawn } from 'node:child_process'

// Simulate the npm wrapper’s behaviour.
const child = spawn('node', ['-e', 'process.on("SIGINT", () => process.exit(42)); setInterval(()=>{}, 1000)'], {
  stdio: 'inherit'
})

const signalHandlers = {
  SIGINT: () => forwardSignal('SIGINT'),
  SIGTERM: () => forwardSignal('SIGTERM')
}

for (const [signal, handler] of Object.entries(signalHandlers))
  process.on(signal, handler)

function forwardSignal(signal) {
  if (!child.killed) {
    try {
      child.kill(signal)
    } catch (error) {
      if (!error || error.code !== 'ESRCH') throw error
    }
  }
  process.off(signal, signalHandlers[signal])
  process.kill(process.pid, signal)
}
  1. Run the following in bash:
node /tmp/test.mjs &
PID=$!
echo "pid=$PID"

kill -INT "$PID"
wait "$PID"
echo "exit code: $?"

^ should print exit code: 130. Replace kill -INT "$PID" with kill -TERM "$PID", should see 143 instead of 130.

Example where this matters: GitHub Actions runners.

When cancelling a job (or the platform aborts it for timing out), it sends SIGINT or SIGTERM to every process. Before this fix the Foundry npm wrapper would catch that signal, kill the child with a plain SIGTERM, and then exit with status 0. GitHub interprets 0 as success, so the workflow step looks green even though it was canceled midway.

With this change, the wrapper exits with 130/143, so CI marks the step as “canceled” or “failed” as it should, and downstream steps/devices don’t keep running on a half-finished build.

@grandizzy grandizzy added this pull request to the merge queue Nov 4, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 4, 2025
@grandizzy grandizzy added this pull request to the merge queue Nov 4, 2025
Merged via the queue into master with commit 3dbb87a Nov 4, 2025
15 checks passed
@grandizzy grandizzy deleted the o-az/update-node-exit branch November 4, 2025 11:16
@github-project-automation github-project-automation bot moved this to Done in Foundry Nov 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants