feat: forward user signals to builds/runs#1088
Conversation
github-actions
Bot
added
the
t-tooling
There was a problem hiding this comment.
Pull request overview
This PR improves Ctrl+C / signal handling in the CLI by forwarding user interrupts to spawned local processes and by aborting in-progress Apify platform Builds/Runs when the user interrupts the CLI.
Changes:
- Add
forwardSignalssupport toexecWithLog()to forward signals from the CLI to spawned child processes. - Introduce
useSignalHandler()(Disposable-based) and use it to abort platform Runs/Builds on SIGINT/SIGTERM/SIGHUP. - Update TS/ESLint configuration to support
Disposable/usingpatterns and underscore-prefixed unused bindings.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds ESNext.Disposable lib typing support. |
| src/lib/hooks/useSignalHandler.ts | New Disposable-based signal hook with optional terminal-line cleanup. |
| src/lib/hooks/useCLIVersionAssets.ts | Simplifies asset-name destructuring (relies on underscore unused-vars convention). |
| src/lib/exec.ts | Adds forwardSignals plumbing and process-level forwarding handlers. |
| src/lib/commands/run-on-cloud.ts | Aborts platform Runs on signals while streaming/waiting. |
| src/commands/run.ts | Enables signal forwarding for local Node/Python runs via execWithLog. |
| src/commands/builds/create.ts | Aborts platform Builds on signals while streaming build logs. |
| eslint.config.mjs | Allows underscore-prefixed unused vars for using _x = ... patterns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Let's discuss whether to swallow signals forever or not -> leaning towards "its fine as is right now", the only thing I agree with is the execa signal ?? exitCode change |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
l2ysho
left a comment
l2ysho
left a comment
There was a problem hiding this comment.
gj @vladfrangu looks good,
btw I tried replicate that abort problem Copilot raised, but with now luck
|
@vladfrangu to resolve #885 we need to use useSignalHandler also in push command (I guess, because now it is not working) |
- consolidates duplicated signal-handler logic from builds/create and run-on-cloud into a single reusable hook - applies the same hook in actors/push so build aborts work there too
| // user gives up waiting (Ctrl+C, SIGTERM from a parent process, | ||
| // SIGHUP from a closing terminal). The `using` binding guarantees | ||
| // the listener is removed before we poll for final status. | ||
| using _signalHandler = useAbortJobOnSignal({ |
There was a problem hiding this comment.
isnt this supported only on node 24+?
There was a problem hiding this comment.
It gets downcompiled by tsup/down 👀
There was a problem hiding this comment.
Ignore the terminal breaking (volta doesn't forward signals LOL)
Are you running |
|
It seems to be specific to opus 4.7 but I did not know that slash cmd existed! |
|
You might need to install it, but its from their official skills. |
Closes #631
Closes #885
Unfortunately Claude doesn't know how to not overdocument but it's fine