fix(cli): settle signal-terminated shell commands as 128 + signum

[marius-kilocode]

When a command run through the Bash tool dies from a signal, the spawned process produces no numeric exit code (Node reports code: null, signal: 'SIGSEGV'). This is the common case, not an edge case: bash -c execs a single simple command, so any segfaulting binary kills the spawned process by signal.

The spawner's exitCode effect failed in that situation, and the shell tool's Effect.raceAll only settles on the first success, so the call kept waiting for the abort or timeout branches. The tool looked hung until the full Bash timeout elapsed, and other consumers of exitCode (AppProcess, command timeout handling, project git helpers) surfaced an opaque Unknown: ChildProcess.exitCode error instead of a status.

Report signal termination the way every POSIX shell does, as 128 + signum (139 for SIGSEGV, 143 for SIGTERM). The mapping lives in a Kilo-owned helper (packages/core/src/kilocode/exit-code.ts) and the shared spawner keeps only two kilocode_change marked lines, so all exitCode consumers settle with a meaningful code at once. Regression coverage sits in Kilo-owned test files: a mapping and live-spawner test in packages/core, and an end-to-end Bash tool test that terminates the shell with SIGSEGV and asserts the call settles promptly with exit 139 instead of waiting out its configured timeout.

Supersedes #12679, which mapped the failure to exit code 1 inside the shell tool only. That left the other exitCode consumers hanging and discarded the signal information that a 128 + signum code preserves.

Fixes #12677

[kilo-code-bot]

SUGGESTION: This null check is now duplicated inside settle, so the settle([code, signal]) branch below can never observe a non-null code.

Since settle already handles the numeric case and returns an Effect, the whole callback could collapse to exitCode: Effect.flatMap(Deferred.await(signal), settle). That removes the dead branch and shrinks the diff against upstream in this shared file to a single changed line, which matches the stated intent of the PR. Your call.

---

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

[kilo-code-bot]

SUGGESTION: Settling immediately on signal termination also changes process-group cleanup, and the release handler above isn't updated for it.

The acquireRelease release path only group-kills when the leader exited with a non-null, non-zero code (if (code !== 0 && Predicate.isNotNull(code)) return yield* Effect.ignore(kill(killGroup))). For a signal death code stays null, so no killGroup runs. Previously the shell tool never reached scope close promptly: it waited out its timeout branch and then called handle.kill({ forceKillAfter }), which does killGroup and reaped descendants of the shell. Now the scope closes as soon as the leader dies, so background descendants of a signal-killed shell (e.g. sleep 300 & kill -SEGV $$) survive with their pipes still open.

Extending the release condition to also group-kill when the leader died from a signal would preserve the old cleanup guarantee. Not a blocker for the hang fix itself.

---

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

[kilo-code-bot]

SUGGESTION: constants.signals is typed as carrying every NodeJS.Signals key, but at runtime it only contains the signals the current platform defines (SIGINFO is absent on Linux, and Windows exposes a small subset). An unmapped name evaluates to 128 + undefined, i.e. ExitCode(NaN), which then flows into metadata.exit and out over the SDK as a non-numeric exit code — a quieter failure mode than the error this replaces.

Node normally only reports signal names that exist on the platform, so this is defensive rather than a live bug, but a guarded lookup (check the value is a number, otherwise fall back to 128) would keep the result numeric in all cases. No suggestion block here because ?? on the non-nullable typed lookup would itself trip the typechecker.

---

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Incremental review of 1d360c8c -> 4d093bb6. All previously raised suggestions are resolved:

• The duplicated null-code check in cross-spawn-spawner.ts is gone; exitCode now pipes Deferred.await(signal) straight into settle, keeping the shared-file diff to a single marked line.
• settle now guards the constants.signals lookup, so an unknown/platform-absent signal falls back to exit code 1 instead of producing ExitCode(NaN), with a regression assertion covering it.
• The process-group cleanup concern raised earlier is pre-existing behavior in the unchanged acquireRelease release path (a null code already skipped killGroup before this PR), so it is not carried forward as a finding on this diff.

Files Reviewed (3 files)

• packages/core/src/cross-spawn-spawner.ts
• packages/core/src/kilocode/exit-code.ts
• packages/core/test/kilocode/exit-code.test.ts

Previous Review Summary (commit 1d360c8)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 1d360c8)

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	3

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
packages/core/src/cross-spawn-spawner.ts	446	Null-code check is duplicated inside settle; passing settle directly reduces the shared-file diff to one line and drops a dead branch
packages/core/src/cross-spawn-spawner.ts	447	Immediate settle means the release handler skips killGroup for signal deaths (code === null), so background descendants of a signal-killed shell are no longer reaped
packages/core/src/kilocode/exit-code.ts	12	constants.signals[signal] can be undefined at runtime despite its type, yielding ExitCode(NaN); a guarded lookup keeps the code numeric

Files Reviewed (5 files)

• packages/core/src/cross-spawn-spawner.ts - 2 issues
• packages/core/src/kilocode/exit-code.ts - 1 issue
• packages/core/test/kilocode/exit-code.test.ts - 0 issues
• packages/opencode/test/kilocode/tool/shell-signal.test.ts - 0 issues
• .changeset/fuzzy-otters-signal.md - 0 issues

The core approach looks right: mapping signal termination to 128 + signum settles every exitCode consumer at once, the Kilo-specific logic lives in a Kilo-owned file, the shared spawner keeps a minimal marked diff, and both regression tests exercise the real spawner/tool rather than mocks. All findings are non-blocking suggestions.

Fix these issues in Kilo Cloud

---

_{Reviewed by claude-opus-5 · Input: 26 · Output: 3.9K · Cached: 506.8K}

_{Review guidance: REVIEW.md from base branch main}