Repository navigation
Conversation
A native `hack restart` installed no SIGINT or SIGTERM handler until its new foreground start did. A first Ctrl-C during preflight, the cleanup of the old run or the early start was therefore either dropped (a SIGINT inherited as ignored, as for a background job, which is how the restart reached ready and needed a second Ctrl-C) or ended the frontend mid-restart with its restart lock and intent left behind. The restart now handles stop requests from its first step until the new start installs its own handlers, then hands them off: - during preflight it ends with nothing stopped; - during cleanup the owned stop runs to completion, the old frontend's finalizers are awaited and the new run is not started; - before the hand-off the start is abandoned before any hook or service; - after the hand-off the start stops its run as an interrupted `up`. Every cancel prints the same lines as Ctrl-C after `up` (one "Stopping..." line, then the confirmed-stop line), removes the restart intent so a plain `hack up` resumes the retained data, and exits 130 (143 for SIGTERM). Refs HACK-1953 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 378b05f9f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
… early After the stop-signal hand-off, a first Ctrl-C while the new start was still discovering host dependencies or waiting for `runtime up` made `startNativeProject` throw rather than return an interrupted exit, so the restart rethrew and left its pending intent behind although no new graph was admitted; the next plain `hack up` refused. `startNativeProject` now reports a confirmed cancellation: a stop request before any service was requested, with no HTTPS acquisition failure, ends in `NativeStartupCanceledError` with `beforeServices` set once its own cleanup (hooks, finalization) completed. A cleanup failure still replaces it. The restart retires the intent on that outcome and exits with the start's interrupted code; any other failure keeps the pending restart. Refs HACK-1953 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Summary
The first Ctrl-C (SIGINT) or SIGTERM now stops a native
hack restartat any phase. Before this change, a native restart installed no stop handler of its own until its new foreground start did. A first SIGINT during preflight, cleanup of the old run or the early start was therefore either:A debug trace on 5.0.0-next.11 confirmed the cause. A SIGINT 2 s into a restart arrived during the old run's cleanup, and no process listener existed at that point.
Behavior
The restart now handles stop requests from its first step. When the new start installs its own handlers (
nativeForegroundStopSignals), the restart hands stop requests over to them and removes its own. The first stop request cancels at the next safe point:Native restart canceled before cleanup; nothing was stopped.)updoesEvery cancel:
Stopping the native project; waiting for owned cleanup…line, then the confirmed-stop line, the same output as Ctrl-C afterup;hack upresumes the retained data instead of reporting a pending restart;Sometimes a stop request arrives after the hand-off but before the start has requested any service, for example while it discovers host dependencies or waits for
runtime up. If the start's own cleanup then completes, it ends withNativeStartupCanceledError(beforeServices). The restart treats that as a confirmed cancellation and retires the intent. Any other failure keeps the pending restart, including a cleanup failure, and so does a start that fails without a stop request.startNativeProjectgains anonStopSignalscallback, called once its own handlers are installed. This is the hand-off point. The restart is now split into small helpers (select, cleanup, start-or-stop) to stay within the complexity limit.Validation
Native, on this host. This used the 5.0.0-next.11
hack-nativewith this branch's CLI and a synthetic single-service project. Each case started in a fresh private native home and was run the way the report describes:hack restartas a background job (SIGINT inherited as ignored) and one SIGINT.upkeeps runningpsabsent after thatupis stopped;hack upreadyhack upreadyPreparing native project input)hack upreadyStarting services)Native project stopped.hack upready (no pending restart)On 5.0.0-next.11 the same 2 s SIGINT let the restart run to ready, and it stopped only on a second SIGINT.
Tests.
tests/native-project-restart.test.tscover: a stop request in preflight, in cleanup, before hand-off, after hand-off and before ready, and after hand-off; SIGTERM → 143; a failed start keeps its intent.tests/fixtures/native-restart-signal-worker.ts). The worker starts under/bin/shwithtrap "" INT(inherited ignore), in its own process group, with stdin closed. The test holds the worker in each of five phases and checks the exit code 130, the output lines, a single "stopping" line and no SIGINT listener left at exit.native-project-restart,native-project-start,project-restart-command,native-https-startup-failure,native-project-https): 230 pass, 0 fail. Every test file that touches up or restart (106 files): 2232 pass, 96 fail. All 96 failures also fail on a cleanorigin/nextworktree.bun run --cwd packages/cli typecheck: only the pre-existingtests/fixtures/atomic-write-foreign-owner-worker.ts(39,25)error, which also fails onorigin/next.native-project-start.ts.origin/nextworktree in this environment. I reran each failing file there. They are innative-compose-*/native-vm-file-*,git-info-exclude,pre-commit-hook,runtime-turbo-inputs,test-config-compiler,native-ca-doctor-cliandbun-version-guard.Gaps
Native project stopped.without the retention sentence. That is the existingupbehavior at the same point, and it is left unchanged.up, it does not force a running down hook.Refs HACK-1953
🤖 Generated with Claude Code