Split out of #629, which fixed the hang but not this. Recording the failed attempt so the next person does not repeat it.
What leaks
queryCodexPlugin runs codex plugin list --available --json. Where codex is a wrapper rather than a binary, the wrapper exits immediately and leaves a descendant behind:
2236158 13:56 /bin/bash ~/.local/bin/codex plugin list --available --json
2236159 13:56 npm exec which @openai/codex
#629 bounded basecamp doctor — it returns in about six seconds now instead of hanging for ten minutes — but that descendant still survives. One resident process per timed-out doctor on an affected machine.
This predates #629. The same orphan was created before, the hang just made it less visible. #629 does not introduce or worsen it.
Why the obvious fix does not work
The natural approach is a process group: SysProcAttr{Setpgid: true} plus cmd.Cancel killing -pgid. I implemented it and it does not fire. Measured, with an instrumented cmd.Cancel:
Output returned after 1.010779833s, err=exec: WaitDelay expired before I/O complete, cancelCalled=false
grandchild pid=17320
PID PGID STAT ARGS
17320 17318 S sleep 120
alive after return: true
alive after +2s: true
Two things to read off that:
Setpgid worked — the grandchild's PGID (17318) is the wrapper's pid, so kill(-17318) would have reached it.cancelCalled=false — cmd.Cancel is never invoked. The wrapper exits within milliseconds, long before the 5s deadline, so Go's context watcher is done before the context expires. WaitDelay is what ends the call, and by then nothing kills the group.
So cmd.Cancel cannot carry this fix. The deadline that matters expires after the direct child is already gone.
What a real fix needs
Stop using Output() and manage the lifecycle directly: Start, capture stdout, run our own watchdog that kills the process group when the context expires regardless of whether the direct child already exited, then Wait.
The subtlety to design around is pid recycling. The pgid equals the child's pid, and Wait reaps that child — so killing -pgid after the reap can in principle hit an unrelated group. The kill has to happen while the group is still ours, or be otherwise proven safe. That is a real rewrite of the probe's exec handling with its own tests, which is why it did not ride along on a release blocker.
WaitDelay should stay regardless: a descendant that leaves the group deliberately (setsid) is out of any group kill's reach, and Wait still has to return.
Split out of #629, which fixed the hang but not this. Recording the failed attempt so the next person does not repeat it.
What leaks
queryCodexPluginrunscodex plugin list --available --json. Wherecodexis a wrapper rather than a binary, the wrapper exits immediately and leaves a descendant behind:#629 bounded
basecamp doctor— it returns in about six seconds now instead of hanging for ten minutes — but that descendant still survives. One resident process per timed-outdoctoron an affected machine.This predates #629. The same orphan was created before, the hang just made it less visible. #629 does not introduce or worsen it.
Why the obvious fix does not work
The natural approach is a process group:
SysProcAttr{Setpgid: true}pluscmd.Cancelkilling-pgid. I implemented it and it does not fire. Measured, with an instrumentedcmd.Cancel:Two things to read off that:
Setpgidworked — the grandchild's PGID (17318) is the wrapper's pid, sokill(-17318)would have reached it.cancelCalled=false—cmd.Cancelis never invoked. The wrapper exits within milliseconds, long before the 5s deadline, so Go's context watcher is done before the context expires.WaitDelayis what ends the call, and by then nothing kills the group.So
cmd.Cancelcannot carry this fix. The deadline that matters expires after the direct child is already gone.What a real fix needs
Stop using
Output()and manage the lifecycle directly:Start, capture stdout, run our own watchdog that kills the process group when the context expires regardless of whether the direct child already exited, thenWait.The subtlety to design around is pid recycling. The pgid equals the child's pid, and
Waitreaps that child — so killing-pgidafter the reap can in principle hit an unrelated group. The kill has to happen while the group is still ours, or be otherwise proven safe. That is a real rewrite of the probe's exec handling with its own tests, which is why it did not ride along on a release blocker.WaitDelayshould stay regardless: a descendant that leaves the group deliberately (setsid) is out of any group kill's reach, andWaitstill has to return.