Skip to content

PR 025 Finding B: Bound fallback child termination - #26

Merged
LogicDuke merged 4 commits into
repair/pr010-hardening-failure-settlementfrom
repair/pr025-termination-fallback-child-leak
Aug 17, 2026
Merged

PR 025 Finding B: Bound fallback child termination#26
LogicDuke merged 4 commits into
repair/pr010-hardening-failure-settlementfrom
repair/pr025-termination-fallback-child-leak

Conversation

@LogicDuke

@LogicDuke LogicDuke commented Aug 17, 2026

Copy link
Copy Markdown
Owner

Purpose

Stacked validation PR for:

PR025-CODEX-TERMINATION-FALLBACK-CHILD-LEAK

This PR is intentionally narrow and quarantined.

It targets protected parent PR #25.

It does NOT target main.

It does NOT directly target protected PR #10.

Finding

Classification after fresh independent reproduction:

CURRENT P2

Codex originally labelled the finding P1.

The normal hardening-failure termination strategy could fault on hostile
ChildProcess state before delivering any termination signal.

The invocation would reject, but the direct child could remain alive.

The prior regression probe then performed its own SIGKILL cleanup before
measuring transport abandonment, allowing a false LEAKED=0.

The underlying runtime lifecycle defect pre-existed PR #25.

Repair

The bounded repair performs one guarded best-effort direct-child termination
attempt if the ordinary termination strategy faults before signalling.

It does not strengthen descendant or process-tree termination guarantees.

The regression probe now records transport abandonment before harness cleanup,
then performs targeted cleanup afterwards.

Changed files exactly:

  • src/adapters/process-transport.ts
  • tests/adapters/process-transport.test.ts

Protected invariants

The repair preserves:

  • original hardening error visibility;
  • eventual outer settlement;
  • fallback failure containment;
  • cleanup failure containment;
  • no SPAWN_FAILED laundering;
  • no AgentExchange laundering;
  • no discarded internal rejected Promise;
  • spawn-failure absorber safety;
  • degraded/platform-qualified termination semantics;
  • frozen AgentBridge architecture;
  • AgentBridge V1 managed-repository read-only authority;
  • dormant/unwired transport state.

No stronger descendant/tree termination guarantee is introduced.

Exact quarantine identity

Parent PR #25 HEAD:

bbdc9d4405627417cc14874fe1cf50c19c24b054

Repair commit:

b55ac5877e93cc4fafd27e5bd1089c9031fcc68f

Validated patch SHA-256:

329B03598FCF547AE6AD4CAD0DE6DFFAEDBA3CD7F7C372DFBEAB00E2A01535E9

Patch bytes:

10979

The committed patch was mechanically verified byte-for-byte identical to the
candidate that passed fresh independent validation.

Independent validation

A fresh validator separate from the implementing agent independently reproduced
the original failure and verified the candidate.

Validation result:

PASS

Required focused tests, process-transport tests, full test suite, typecheck,
lint, build, and git diff --check passed.

Separate outstanding finding

CodeRabbit Finding A remains:

CURRENT P3

That separate test-harness finding is NOT repaired here and must be handled
through its own bounded quarantine after this repair is integrated and PR #25's
new HEAD is established.

Quarantine rule

This DRAFT PR is evidence/proposal only.

Do not merge because the implementing agent or validator says it is fixed.

Required before upward integration into PR #25:

Summary by CodeRabbit

  • Bug Fixes
    • Improved process cleanup when termination fails, including a guarded direct-child termination attempt.
    • Prevented abandoned child processes after transport termination errors.
    • Preserved the original hardening error while avoiding unhandled rejection issues.

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 92d8e5ac-85f7-4537-a8d0-2453312907f3

📥 Commits

Reviewing files that changed from the base of the PR and between b55ac58 and bb9a21b.

📒 Files selected for processing (2)
  • src/adapters/process-transport.ts
  • tests/adapters/process-transport.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/adapters/process-transport.ts

Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The transport now makes one guarded direct-child termination attempt when platform termination fails. Tests distinguish transport abandonment from harness leaks and verify direct-child signaling without process-tree helper creation.

Changes

Termination fallback hardening

Layer / File(s) Summary
Guarded direct-child fallback
src/adapters/process-transport.ts
The termination error path makes one guarded direct-child SIGKILL attempt. The fallback absorbs its own failure, does not wait, and does not signal descendants.
Abandonment and leak validation
tests/adapters/process-transport.test.ts
The probes synchronize signal-ignoring children, track direct-child signals and owned PIDs, measure abandonment separately from harness leaks, and verify cleanup. Assertions cover the original rejection, no unhandled rejection, one direct-child SIGKILL attempt, no helper spawn, no abandonment, and no leftover readiness file.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to bb9a2

The change is limited to fallback child termination and its regression coverage. No actionable merge-blocking risk remains once normal checks and review are complete.

Possibly related PRs

Poem

A rabbit saw the child resist,
One SIGKILL ended the mist.
No helper came, no leak remained,
The readiness sign was cleanly drained.
The transport’s contract was maintained.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the finding and the primary change: bounding fallback child termination.
✨ 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 repair/pr025-termination-fallback-child-leak

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.

@LogicDuke

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@LogicDuke

Copy link
Copy Markdown
Owner Author

@
@codex Please independently review exact CURRENT child HEAD b55ac58 for the bounded PR025-CODEX-TERMINATION-FALLBACK-CHILD-LEAK repair. Review only the delta from parent bbdc9d4. Verify that if normal termination faults before signalling, exactly one guarded best-effort direct-child attempt still occurs; fallback failure cannot prevent settlement; the original hardening error remains caller-visible; no SPAWN_FAILED or AgentExchange laundering occurs; no discarded internal rejected Promise or double settlement is introduced; transport abandonment is measured before test-harness cleanup; no stronger descendant/tree termination guarantee is introduced; architecture and V1 authority remain unchanged; and Finding A remains outside this repair. Treat findings on any other SHA as stale. Do not propose unrelated refactors or scope expansion.
@

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
❌ Action failed

Review failed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@LogicDuke

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@LogicDuke

Copy link
Copy Markdown
Owner Author

@codex review

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@LogicDuke

Copy link
Copy Markdown
Owner Author

@coderabbitai help

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

CodeRabbit commands

These commands are invoked using PR/Issue comments.

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai review --preview-config to test the unmerged CodeRabbit configuration on a draft PR. The requester must have repository write access; preview results are non-authoritative.
  • @coderabbitai rate limit to show your current review rate limit status.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai evaluate custom pre-merge check --instructions <custom-checks-instructions> --name <custom-checks-title> [--mode <error|warning>] to validate the custom pre-merge checks instructions. Defaults to error when --mode is omitted.
  • @coderabbitai ignore pre-merge checks to override pre-merge checks and get an approval on PR.
  • @coderabbitai run pre-merge checks to run pre-merge checks on the pull request.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai emit path instructions to emit generated path-specific review instructions for this repository.
  • @coderabbitai resolve merge conflict to automatically resolve merge conflicts.
  • @coderabbitai autofix to automatically fix issues identified in unresolved review comments.
  • @coderabbitai fix-ci to automatically fix failing CI checks in a stacked pull request.
  • @coderabbitai fix-ci commit to automatically fix failing CI checks by committing fixes to the current branch.
  • @coderabbitai resolve to resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai generate configuration to create a PR that adds the current resolved configuration as .coderabbit.yaml (or show it if already present).
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit configuration file (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, support, documentation and community

  • Visit our status page to check the current availability of CodeRabbit.
  • Create a ticket on our support page for assistance with any issues or questions.
  • Visit our documentation site for detailed information on how to use CodeRabbit.
  • Join our Discord community to connect with other users and get help from the community.
  • Follow us on X/Twitter for updates and announcements.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: b55ac5877e

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

@LogicDuke
LogicDuke marked this pull request as ready for review August 17, 2026 15:39
@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

killDirectChild(child);

P2 Badge Use a non-ignorable signal for the fallback

On POSIX, when dispatch hardening fails, a hostile exitCode/signalCode accessor makes terminate() reject, and the spawned child ignores or blocks SIGTERM, this default killDirectChild(child) sends only SIGTERM and then immediately releases the handle without waiting or escalating. Such a child can therefore survive indefinitely after the exchange rejects, so the new fallback still abandons the process in precisely the failure path it is intended to contain; the added probe masks this because its child does not install a SIGTERM handler. Use a non-ignorable direct-child fallback such as SIGKILL on this no-further-observation path.

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

@LogicDuke
LogicDuke marked this pull request as draft August 17, 2026 17:19
@LogicDuke

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@LogicDuke

Copy link
Copy Markdown
Owner Author

@codex Please perform a fresh substantive review of PR #26 against exact CURRENT HEAD bb9a21b.

Review the complete current PR diff against base bbdc9d4.

Focus especially on:

  • post-hardening-failure settlement;
  • direct-child fallback termination;
  • explicit SIGKILL fallback;
  • hostile lifecycle accessors;
  • exact-once fallback attempt;
  • settlement/terminal-cause precedence;
  • no discarded internal rejection;
  • no double settlement;
  • cleanup failure containment;
  • POSIX SIGTERM-ignore regression;
  • stale readiness-marker resistance;
  • Windows non-regression;
  • process ownership and descendant-termination claims;
  • raw-output preservation;
  • environment isolation;
  • frozen AgentBridge architecture;
  • V1 managed-repository read-only authority;
  • dormant/unwired transport;
  • no unrelated scope.

Known inherited finding:

AUDIT-PR026-TEMPDIR-SILENT-LEAK
CURRENT P3

This finding is inherited unchanged from PR #10 lineage and is NOT owned by PR #26.

Do not treat it as introduced by PR #26.

If you independently find it, report that it is CURRENT but inherited/out of scope for this bounded PR.

Treat findings anchored only to older SHAs as stale until reverified against CURRENT HEAD.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: bb9a21b1df

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

@LogicDuke
LogicDuke marked this pull request as ready for review August 17, 2026 22:41
@LogicDuke
LogicDuke merged commit 0ced0ef into repair/pr010-hardening-failure-settlement Aug 17, 2026
2 checks passed
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.

1 participant