2 Commits

Include renames

Author	SHA1	Message	Date
PierrunoYTandClaude Opus 5	27b319ca88	test(execution): fail loudly when ps cannot answer instead of skipping (#961) ... * test(execution): fail loudly when ps cannot answer instead of skipping Every test in process_unix_test.go proves something about zombie detection, and every one of those proofs is read out of ps. When ps could not answer, the tests skipped — which is indistinguishable from success in CI output — and one assertion passed vacuously for the same reason. A regression in ps parsing or availability would have shown up as green. requireProcessState probes BOTH ps pipelines against this test process, which is by definition alive and not a zombie, so a working ps must report a live state for it. When it cannot: - Under CI (or ZERO_REQUIRE_PS=1) this is a hard failure. ps is present on every image used today, so its absence means the pipeline regressed and that has to surface immediately. - Otherwise it stays a skip, so a contributor on an unsupported box is not blocked. Both pipelines are probed because they are deliberately separate: the tests assert what the production process-table parser concludes, so this file keeps its own per-PID ps oracle rather than borrowing that parser to establish its own preconditions. One regression must not be able to silence both sides at once. Once the probe passes, failing to observe an EXPECTED state is a defect rather than an environment problem, so the remaining escapes become failures: - The zombie waits (awaitZombie) fail instead of skipping. The caller never waits on the child, so its zombie row persists until reaped — reaching the deadline means the child never exited or ps stopped answering, and the two are reported distinctly. - A zombie group that is not signalable fails instead of skipping: a zombie still occupies the process table, so if kill(-pid, 0) fails the case under test is not present and the assertions after it would prove nothing. - processSignalTarget returning anything but the individual PID fails instead of skipping: the child inherited this process's group, so its PGID is this process's PID and can never equal its own PID. The vacuous assertion in TestTerminateProcessTreeStopsRunningGroup now requires ok. That is sound rather than merely stricter: the test never waits on the child, so the group's zombie row must still be present after termination, and an unknown state means ps stopped answering rather than that the group is clean. Verified on Linux (WSL, go1.26.6) rather than by cross-compilation alone: - all three tests pass unmodified - with a stub ps that exits 1: CI unset skips with the reason named; CI=true fails, naming which pipeline could not answer - the same broken-ps run against the ORIGINAL file produced two silent skips plus one unrelated failure after 12s, versus an immediate, explicit failure now Windows is unaffected (the file is !windows); go vet passes for linux, darwin and windows. Fixes #863 Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01U389qmQUhoZB3YtXkFmiTq * test(execution): treat an unsignalable zombie group as a kernel difference The macOS Smoke job failed: the zombie group 5931 is not signalable (operation not permitted) That was my error. I converted this skip along with the ps-availability ones, but it never had anything to do with ps: whether kill(2) may signal a group whose only member is a zombie is a KERNEL difference. Linux permits it, so signalTargetRunning goes on to consult ps — the branch this test was written for. Darwin returns EPERM, so signalTargetRunning short-circuits on the kill and never reaches ps. Rather than restore the blanket skip, the test now records which branch answers: - Signal permitted: assert processGroupHasRunningMember directly, so a ps-side regression cannot hide behind the kill result. This is new coverage; the old test only checked signalTargetRunning, which conflates the two paths. - Signal refused: log it and continue. The outcome assertion — a zombie-only group must not count as running — now runs on EVERY platform. Previously Darwin skipped before reaching it, so this is strictly more coverage there, not less. Any errno other than EPERM still fails: it means the group is not in the state the test needs, and nothing after it would be meaningful. TerminateProcessTree is asserted only where the signal is permitted, because terminateTarget's own kill(2) would hit the same EPERM and return it as a cleanup error. Worth a maintainer's eye: that may be a real gap in TerminateProcessTree on such a platform — a zombie-only tree is already exited and arguably should terminate cleanly rather than report failure, which is the same spurious-cleanup-failure class #774 set out to remove. Confirming that needs a Darwin host and is a production change, so it is flagged rather than guessed at here. Verified on Linux (WSL, go1.26.6): all three tests pass, including the new direct ps assertion. The Darwin branch was exercised by forcing the EPERM path on Linux — it logs both explanations and passes with the outcome assertion still enforced. go vet passes for linux, darwin and windows. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01U389qmQUhoZB3YtXkFmiTq --------- Co-authored-by: Claude Opus 5 (1M context) <[email protected]>	2026-08-26 13:22:43 +08:00
5 people	9c292092a2	fix(daemon): clean up child after startup timeout (#774) ... * fix(daemon): clean up child after startup timeout * fix(daemon): terminate startup process tree * test(background): accept reaped-later zombie children * fix(background): retain configured process group target * fix(background): address termination review feedback * fix(daemon): use launch-time group identity, fix zombie/reap classification Addresses jatmn's three #774 review findings: - TerminateCommand rediscovered the process group via execution.TerminateProcessTree's Getpgid(pid) lookup. On Darwin, that lookup can return ESRCH once an unreaped group leader has already exited, even though live descendants remain in the group it configured at launch — silently falling back to signalling only the (already dead) leader PID and leaving descendants running (the required macOS smoke job's failure). Add execution.TerminateProcessGroup, which signals the negative PID directly using the launch-time Setpgid invariant instead of rediscovering it, and route TerminateCommand through it. - signalTargetRunning's zombie check matched the individual-PID fallback target (a process that is not its own group leader) against the process table by PGID. That target's actual PGID differs from its own PID by definition, so no row ever matched, unknown resulted every time, and a zombie individual-PID target was always conservatively reported as still running. Branch on the sign of the target: group members by PGID, individual PIDs by PID. - TerminateCommand returned the termination error even when the subsequent reap succeeded — a successful cmd.Wait is authoritative proof the leader is gone and collected, regardless of how the kill attempt itself went (e.g. taskkill /T racing an already-exited PID on Windows). Classify a successful reap as success. All three regression tests are verified to fail against the pre-fix code (confirmed by temporarily reverting each fix and rerunning). Co-Authored-By: Claude Sonnet 5 <[email protected]> * fix(daemon): harden command termination inputs Amp-Thread-ID: https://ampcode.com/threads/T-019fa018-abb8-73de-9489-51e9e6514c6b Co-authored-by: Pierre Bruno <[email protected]> * fix(background): preserve process tree failures Amp-Thread-ID: https://ampcode.com/threads/T-019fa7a0-1223-701d-9529-48ba5d7cf8c8 Co-authored-by: Pierre Bruno <[email protected]> * docs(daemon): clarify boundary-probe and Setsid-guard comments per PR #774 review Co-authored-by: Cursor <[email protected]> --------- Co-authored-by: Claude Sonnet 5 <[email protected]> Co-authored-by: Amp <[email protected]> Co-authored-by: Cursor <[email protected]>	2026-08-15 20:31:10 +08:00