Script timeout bounds the verdict but not the wait

[bguidolim]

Follow-up to #374 / #375.

Problem

#375 made the timeout verdict reliable: a script that exceeds its budget is now always reported as timed out, even when the killer work item is starved. It did not make the wait bounded.

ScriptRunner.executeWithTimeout reads both pipes to EOF and then calls waitUntilExit() before evaluating the verdict. process.terminate() sends SIGTERM to the direct child only. So there are two routes to waiting far past the budget:

• the script traps or ignores TERM, or
• a descendant inherited the output pipes and keeps them open after the shell dies.

In both cases the caller blocks for the script's full natural duration and is then told it timed out. The type doc now states this honestly rather than papering over it, but the behaviour remains: mcs sync or mcs doctor can hang on a wedged pack script and only afterwards report a timeout — which reads as a bug to a user who just waited several minutes.

Proposed fix

Replace read-then-wait with a deadline-driven loop: poll() on the two pipe file descriptors against a deadline derived from timeout, plus waitpid(WNOHANG), escalating SIGTERM to SIGKILL. Signal the process group rather than the single pid so descendants are covered.

The primitive already exists in this repo — ShellRunner.runInteractive runs exactly this poll() bridge loop, though with an infinite -1 deadline. Once the wait is bounded this way, the elapsed measurement from #375 becomes a cheap cross-check rather than the load-bearing signal.

Note that process-group signalling is a behaviour change on its own merits (it will kill children that today survive), which is part of why it was kept out of the verdict fix.

Consider putting it on ShellRunner

ScriptRunner.executeWithTimeout is close to a line-for-line copy of ShellRunner.run — same environment construction, same two pipes, same null-device stdin with the same anti-deadlock rationale, same read-before-wait, same result shape — differing only by the timeout. Meanwhile ScriptRunner holds a ShellRunning and uses it for one thing: reading environment.pathWithBrew. Its genuine value-add is path containment, auto-chmod, and the MCS_* variables; process spawning is borrowed rather than owned.

ShellRunner currently has no timeout at all, and every network git call goes through it — git ls-remote in UpdateChecker, git clone/git fetch in PackFetcher. The check-updates hook path is bounded externally by the Claude Code hook timeout, but mcs sync and mcs update hitting a wedged remote have nothing bounding them.

So a timeout: TimeInterval? = nil on ShellRunning.run, implemented once where the poll() loop would live, would close both gaps at once and remove the duplicate spawn path. The real cost is reconciling error semantics: ScriptRunner needs a distinct ScriptError.timeout and ScriptError.scriptNotFound on launch failure, where ShellRunner folds launch failure into exitCode: 1. That is an API decision, not a mechanical extraction, which is why it did not belong in the #375 bug fix.