Repository navigation
fix: read group liveness and the sentinel pgid from full ps listings - #13
Merged
Merged
Conversation
BusyBox ships ps and pgrep without selection flags, and its usage errors exit 1 (the same status pgrep uses for "no members"), so on Alpine a cancellation read the failed pgrep -g probe as an already-emptied group, ended the grace period at once and dropped the SIGKILL escalation. The lifeline sentinel's ps -o pgid= -p read failed outright, leaving it nothing to kill. Both sites now read a full ps -A -o listing (pid and pgid columns) filtered in bash, the one shape procps, BSD and BusyBox ps share, and the SDK no longer invokes pgrep at all. A ps that itself fails still degrades to the raw kill -0 group check, never to "empty", so a probe that cannot answer delays the kill rather than dropping it. The test harness leaned on the same missing flags. Its liveness probes used ps -o state= -p (BusyBox has neither the state keyword nor -p selection), and one test hardcoded /usr/bin/true, which Alpine does not have. The probes now share one _mcp_proc_state helper reading a pid=,stat= listing, the fixture process census reads stat=,args=, and true is resolved with type -P. The pgrep-cannot-answer regression test became a ps stub that intercepts only the liveness scan's column signature, so the unknown-liveness fallback stays pinned without degrading the harness's own ps reads. With the suite green on Debian, Alpine and macOS alike, the Alpine failure tolerances are gone. The CI leg gates merges again, scripts/test-linux.sh no longer allows an Alpine failure on bare runs, and the README requirements drop pgrep and the procps caveat. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes out the BusyBox follow-up work #12 left open: the Alpine test leg is green and gates merges again.
The defect
BusyBox ships
psandpgrepwithout selection flags, and its usage errors exit 1 — the same statuspgrepuses for "group has no members". On Alpine a cancellation therefore read the failedpgrep -gprobe as an already-emptied group, ended the grace period at once and dropped the SIGKILL escalation, so a tool that ignores SIGTERM outlived its own cancellation. The lifeline sentinel'sps -o pgid= -pread failed outright, leaving it no group id to kill once the server was gone.The fix
Both sites now read a full
ps -A -olisting (pidandpgidcolumns) and filter it in bash — the one shape procps, BSD/macOS and BusyBoxpsall share.pgrepis no longer invoked anywhere, so it drops out of the requirements; apsthat itself fails still degrades to the rawkill -0group check, never to "empty", keeping the safe direction: a probe that cannot answer delays the kill rather than dropping it.The test harness had the same dependence. Its liveness probes (
ps -o state= -p— BusyBox has neither thestatekeyword nor-p) now share one_mcp_proc_statehelper reading apid=,stat=listing, the fixture process census readsstat=,args=, and the hardcoded/usr/bin/trueis resolved withtype -P(truelives in/binon Alpine). The pgrep-cannot-answer regression test became apsstub that intercepts only the liveness scan's column signature, so the unknown-liveness fallback stays pinned without degrading the harness's ownpsreads.Tolerances removed
With the suite green on Debian, Alpine and macOS alike, the Alpine allowances from #12 are gone: the CI leg no longer runs
continue-on-error,scripts/test-linux.shfails a bare run on an Alpine failure, and the README requirements droppgrepand the procps caveat for Alpine consumers.