fix(cli): capture signal kills + guard empty env paths (#128 followups)

[rohitg00]

Follow-up to #128. CodeRabbit flagged two remaining minor issues on the Windows diagnostics PR that landed before they were addressed. Both real, both minor, both cheap.

1. Signal-only exits weren't captured

Location: `src/cli.ts` exit handler inside `spawnEngineBackground`

When a child process is killed by a signal (SIGSEGV, SIGABRT, SIGKILL, SIGTERM) Node reports `code === null` and `signal !== null`. The previous guard was:

```ts
if (code !== 0 && code !== null) { ... }
```

which explicitly excluded signal-only exits. A segfault in iii-engine would be silently dropped from `startupFailure`, so the user got the generic "did not become ready within 15s" timeout instead of a real crash diagnosis. On Windows this rarely fires (abnormal exits usually carry numeric codes), but on Linux/macOS a segfaulting iii-engine would be invisible to the CLI.

Fix:

```ts
const abnormal =
(code !== null && code !== 0) || (code === null && signal !== null);
if (abnormal) { ... }
```

Also improved the fallback message when stderr is empty so it reads "process killed by signal SIGSEGV" rather than the old "process exited with code null (SIGSEGV)".

2. Empty `USERPROFILE`/`HOME` produced relative paths

Location: `src/cli.ts` `fallbackIiiPaths()`

Previously:

```ts
const userProfile = process.env["USERPROFILE"] || "";
return [
join(userProfile, ".local", "bin", "iii.exe"),
join(userProfile, "bin", "iii.exe"),
].filter(Boolean);
```

When `USERPROFILE` is unset, `join("", ".local", "bin", "iii.exe")` produces `.local/bin/iii.exe` — a relative path. Harmless today (the path is then fed to `existsSync` which fails silently), but theoretically it could match a file in the user's current working directory and then trigger an unintended `spawn()`.

Fix: guard the env var up front, return an empty list (Windows) or skip the per-user path (Unix) when unset. Keep the absolute `/usr/local/bin/iii` fallback on Unix since it's valid regardless of `HOME`.

Test plan

• `npm test` → 684 / 684 passing
• `npm run build` → clean
• After merge, await further report from @Gakko-IT2 on Doesnt work on Windows Claude #127 to see if the fix(cli): surface real engine startup errors, Windows support (#127) #128 + this follow-up resolve their Windows issue (or if we need another round)

Related

• Follow-up to fix(cli): surface real engine startup errors, Windows support (#127) #128 (Windows startup diagnostics)
• CodeRabbit findings on fix(cli): surface real engine startup errors, Windows support (#127) #128: signal-exits + empty-env, both marked ⚠️ Potential issue (Minor)

Summary by CodeRabbit

• Bug Fixes

  • Fixed path resolution on Windows to prevent empty directory entries.
  • Improved engine failure detection to properly identify crash conditions.

• Improvements

  • Enhanced startup error messages with clearer diagnostics for engine failures.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4493311d-630b-4f88-83ae-04e25b1ad8fa

📥 Commits

Reviewing files that changed from the base of the PR and between a477f46 and 397a085.

📒 Files selected for processing (1)

• src/cli.ts

---

📝 Walkthrough

Walkthrough

Updated path fallback logic in fallbackIiiPaths() to avoid empty-string entries, modified engine crash detection in spawnEngineBackground() to treat abnormal conditions as early failures, and adjusted stderr reporting to distinguish signal termination from code exit.

Changes

Cohort / File(s)	Summary
CLI startup and engine spawning src/cli.ts	Updated fallbackIiiPaths() to conditionally return paths based on HOME/USERPROFILE environment variables without fallback empty strings. Modified crash detection logic in spawnEngineBackground() to classify non-zero exit codes or signal termination as abnormal. Refined stderr reporting to separately message signal kills versus code exits.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix(cli): surface real engine startup errors, Windows support (#127) #128: Refines the same CLI startup logic—tightening fallbackIiiPaths() behavior and adjusting spawnEngineBackground()'s early-failure detection and stderr messaging.

Poem

🐰 A rabbit hops through paths anew,
No empty strings left to pursue,
When engines crash with signals bright,
We'll tell the tale both clear and right—
Cleaner code hops into view! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the two main fixes: signal kill capture and empty environment path guarding, matching the primary objectives of the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/windows-cli-followups-128

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}