Skip to content

fix(issue): headless classifies child exit code 0 as 'exited unexpectedly' when no terminal notify observed (headless.js:780)#6169

Open
jeremymcs wants to merge 2 commits into
mainfrom
issue/5737-headless-classifies-child-exit-code-0-as-1778892823
Open

fix(issue): headless classifies child exit code 0 as 'exited unexpectedly' when no terminal notify observed (headless.js:780)#6169
jeremymcs wants to merge 2 commits into
mainfrom
issue/5737-headless-classifies-child-exit-code-0-as-1778892823

Conversation

@jeremymcs
Copy link
Copy Markdown
Collaborator

@jeremymcs jeremymcs commented May 16, 2026

Summary

  • Treated headless child exit code 0 as success in the fallback exit handler and verified with focused headless test suites (68 passing).

Verification

  • Completed in the repository worktree before push.

Related Issue

Repo

  • gsd-build/gsd-2

Branch

  • issue/5737-headless-classifies-child-exit-code-0-as-1778892823

Summary by CodeRabbit

  • Bug Fixes

    • Refined process exit handling so clean child exits are recorded as success (non-0/missing exits are treated as errors) and completion is resolved consistently, improving status accuracy.
  • Tests

    • Added unit tests covering clean exit, non-zero exit, and missing-exit-notification scenarios to validate the new behavior.

Review Change Stack

…edly' when no terminal notify observed (headless.js:780)
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2841c55b-03db-41c1-b450-da3064ca7222

📥 Commits

Reviewing files that changed from the base of the PR and between 39aeca9 and bf36642.

📒 Files selected for processing (1)
  • src/tests/headless-v2-migration.test.ts

📝 Walkthrough

Walkthrough

The child process exit handler in the headless supervisor now treats exit code 0 as a clean exit (sets EXIT_SUCCESS) when no terminal notification was observed; a helper and unit tests were added to validate mapping of exit codes to success/error.

Changes

Child exit code handling

Layer / File(s) Summary
Child exit code distinction
src/headless.ts
The exit handler branches on code === 0 to log a clean-exit message and set EXIT_SUCCESS; non-zero codes log unexpected-exit messages and set EXIT_ERROR. Completion resolution behavior for unfinished runs remains unchanged.
Test helper and unit tests
src/tests/headless-v2-migration.test.ts
Adds handleChildExitWithoutTerminalNotification(code, state) to finalize runs when no terminal notification was seen and sets state.exitCode from the child exit code; tests cover exit 0, exit 1, and null.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through logs at break of dawn,
Saw a child exit, code zero—no alarm,
Not a crash but a whisper, soft and neat,
Headless now nods, the run ends sweet,
I munch carrots and cheer this tiny calm.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: treating child exit code 0 as success instead of 'exited unexpectedly' when no terminal notification is observed in the headless module.
Linked Issues check ✅ Passed The code changes correctly implement the primary objective from issue #5737: distinguishing exit code 0 as success (EXIT_SUCCESS) from non-zero codes as errors, and logging appropriate diagnostic messages for each case.
Out of Scope Changes check ✅ Passed All changes are within scope, directly addressing the exit-code handling fallback and adding tests to verify the expected behavior for exit codes 0, 1, and null.

✏️ 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 issue/5737-headless-classifies-child-exit-code-0-as-1778892823

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 16, 2026

🟢 PR Risk Report — LOW

Files changed 2
Systems affected 1
Overall risk 🟢 LOW

Affected Systems

Risk System
🟢 low Headless Mode
File Breakdown
Risk File Systems
🟢 src/headless.ts Headless Mode
src/tests/headless-v2-migration.test.ts (unclassified)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

headless classifies child exit code 0 as 'exited unexpectedly' when no terminal notify observed (headless.js:780)

1 participant