Skip to content

fix(bash): clean up background process groups - #27

Merged
Blankeos merged 2 commits into
Blankeos:mainfrom
yan-ad:fix/acp-background-process-cleanup
Aug 24, 2026
Merged

fix(bash): clean up background process groups#27
Blankeos merged 2 commits into
Blankeos:mainfrom
yan-ad:fix/acp-background-process-cleanup

Conversation

@yan-ad

Copy link
Copy Markdown
Contributor

Summary

  • terminate remaining process-group members after the foreground shell exits
  • avoid hanging on stdout/stderr pipes inherited by background Node or shell processes
  • add a regression test covering successful commands that leave a background process

Validation

  • cargo fmt --all
  • cargo check --bin crabcode
  • git diff --check

Test note

The full test target is currently blocked by pre-existing input.render(...) test call sites that are missing a newly required boolean argument.

@yan-ad

Copy link
Copy Markdown
ContributorAuthor

@Blankeos, could you please review this PR when you get a chance? This fixes leaked background processes that can accumulate and cause excessive memory usage. Thank you!

@Blankeos

Blankeos commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Maintainer update

Merge confidence: 5/5

Merged main into this branch (cab551c8b1f4c4) so it is no longer ~23 commits behind.

What I did

  • Merged origin/main into fix/acp-background-process-cleanup (clean merge, no conflicts)
  • Verified cargo check succeeds on the merged tip
  • Re-ran bash tool regressions:
    • successful_command_kills_background_processes
    • interactive_read_receives_eof_instead_of_hanging
  • Manual orphan smoke (process-group mirror of the tool):
    • OLD (wait shell only, no killpg): sleep 60 & left a live orphan ✅ reproduces bug
    • NEW (wait + killpg): orphan gone ✅
  • No additional code changes to the bash cleanup logic itself — the original fix still stands

CI note
Repo workflows here are Release/Publish only. On this PR, plan is green and artifact jobs are skipped — there is no cargo test gate to wait on.

Why this PR is necessary
On main, success path never killpgs; only timeout/abort does. Background jobs from the bash tool (cmd &) leak. This captures PGID at spawn and kills the group when the foreground shell exits.

Ready to merge.

@Blankeos

Copy link
Copy Markdown
Owner

Thank you again yan-ad, I really appreciate this!

@Blankeos
Blankeos merged commit 34e0aa4 into Blankeos:mainAug 24, 2026
6 checks passed
Sign up for freeto 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.

2 participants

@yan-ad@Blankeos