Skip to content

fix(destroy): forward SIGINT and SIGTERM to the copy that re-runs in a new session - #281

Merged
ThomasK33 merged 4 commits into
mainfrom
fix/263-forward-signals
Nov 3, 2026
Merged

fix(destroy): forward SIGINT and SIGTERM to the copy that re-runs in a new session#281
ThomasK33 merged 4 commits into
mainfrom
fix/263-forward-signals

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Nov 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Why

The operator's goal is to close every deferred follow-up issue from the first autopilot round. This PR fixes #263. A destroy that runs inside the program's session re-runs as a detached copy in a new session (#216, #246, #259), so the copy can print its result after the session's terminal closes. Ctrl-C or SIGTERM to the original CLI stopped only the original, and the copy kept going and destroyed the session anyway.

What changed

  • rerunInNewSession in src/cli/commands/destroy.ts forwards SIGINT and SIGTERM to the copy while it waits, and removes those handlers once the copy exits.
  • It exits as the copy did. A copy that a signal ended now gives 128 plus the signal's number (130 for SIGINT, 143 for SIGTERM), the convention that batch uses. Before, such a copy gave 1.
  • It does not forward SIGHUP. The end of the program's session hangs up the original CLI, and the copy must outlive that.
  • docs/USAGE.md and the verify-session-lifecycle SKILL describe the forwarding. The Exit Codes table in docs/USAGE.md lists 130 and 143, which batch already gives too.

Behavior for CLI and JSON users

  • Ctrl-C on a destroy typed into the session's shell, or SIGTERM to such a destroy, now also stops the copy. The destroy exits 130 or 143 and prints no result. A destroy request that already reached the host still ends the session, as with an in-place destroy.
  • No schema or error-code changes.

Verification

  • Test commit aef7542 fails on main: the unit tests forwards SIGINT|SIGTERM to the copy and exits as the copy does find no kill on the copy. They pass here. They also check the exit code (130, 143) and that the CLI's signal listeners are back to their count before the re-run. Commit c0a5914 (verifier note) adds does not forward SIGHUP to the copy, which fails when SIGHUP joins the forwarded signals.
  • Live (node --import tsx src/cli/main.ts, isolated AGENT_TTY_HOME and XDG_STATE_HOME): a session whose program ignores SIGHUP. Then destroy with AGENT_TTY_SESSION_ID set, which re-runs, and SIGINT to the original CLI 0.3 s after its copy started.
    • main a279368: original exit 130, the copy still runs right after the SIGINT, and it prints its ok: true envelope later.
    • this branch: original exit 130, the copy has ended, and no envelope is printed.
  • Unit tests on c0a5914: 1734 passed. That commit changes only a test and docs/USAGE.md.
  • CI-shaped gate on 099116f (unit, integration in 4 shards, e2e in 3 shards, --maxWorkers=1 --retry=2): unit 1733, integration 335, e2e 34 passed. That includes the existing tests of a destroy typed into the session's shell. The only later commit syncs coverage.json. npm run typecheck, npm run lint and npm run format:check exit 0.
  • .agents/skills/verify-session-lifecycle/scripts/check.sh --update-coverage: exit 0, 64 runs as expected, 22,897,048 and 22,308,620 distinct states. The model does not change. It has no signals to the CLI.

Closes #263

Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: xhigh

@ThomasK33
test: a destroy that re-runs forwards SIGINT and SIGTERM to its copy
aef7542 
Fails on main: the original CLI never signals the detached copy.

Refs #263
@ThomasK33
fix(destroy): forward SIGINT and SIGTERM to the copy that re-runs in …
099116f 
…a new session

A destroy from inside the program's session re-runs as a detached copy
in a new session, which no Ctrl-C from the terminal reaches. The
original CLI now forwards SIGINT and SIGTERM to the copy and exits as
the copy did, 128 plus the signal's number when a signal ended it. It
does not forward the SIGHUP of the session's end.

Closes #263
@ThomasK33
chore(skills): sync the lifecycle coverage record
4bcd215
@ThomasK33

ThomasK33 commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

ThomasK33 commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T09:32:37.786226Z c0a5914 Manual request
🔒 Security Review ✅ Completed 2026-11-03T09:32:49.205497Z c0a5914 Manual request
ℹ️ 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" or "@codex security review".

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

chatgpt-codex-connector Bot commented Nov 3, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 4bcd215e32

ℹ️ 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".

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 4bcd215e32

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@ThomasK33
test: a destroy that re-runs does not forward SIGHUP; list exit codes…
c0a5914 
… 130 and 143

The verifier of #281 found that forwarding SIGHUP passed every unit
test. The new test fails then. docs/USAGE.md lists 130 and 143, which
batch and a destroy whose copy a signal ended give.

Refs #263
@ThomasK33

ThomasK33 commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

ThomasK33 commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: c0a5914fa1

ℹ️ 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".

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: c0a5914fa1

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@ThomasK33
ThomasK33 added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit ea897c8 Nov 3, 2026
13 of 14 checks passed
@ThomasK33
ThomasK33 deleted the fix/263-forward-signals branch November 3, 2026 10:17
@github-actions github-actions Bot mentioned this pull request Nov 3, 2026
Open
@ThomasK33 ThomasK33 mentioned this pull request Nov 3, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

destroy: Ctrl-C on a destroy that re-runs in a new session does not reach the copy

1 participant

@ThomasK33