fix(process): resolve shells and the process table portably; keep the sandboxed kill honest #68

Closed
Taha wants to merge 0 commits from taha/w-proc-process-shell-portability into main AGit
Contributor

fix(bash): prefer /bin/bash where it exists, then the first bash on PATH, and refuse with a clear error when neither exists - no silent sh fallback.\n\nfix(process/runtime): read /proc//stat and cmdline on Linux for kill verification (ps is missing on NixOS and in container images); zombies stay "gone" and an empty command line stays an error. The ps path stays for unix platforms without /proc.\n\nfix(process/remote): the endpoint launcher resolves bash (/bin/bash first, then PATH) instead of hardcoding it, and records exit 127 with an error when no bash exists instead of dying and leaving the record "running" for ever. The launcher lookup helper reads /proc on Linux.\n\nfix(process/sandboxed): the kill tree walk recursed inside a shell for-loop whose variable the recursion clobbered, so only the leader and the deepest leaf were signalled and a kill reported success while the command shell kept running; the pending pids now travel in the positional parameters. The status script re-reads the exit file with a bounded grace before reporting an unknown exit, because the subshell writes the exit code with an external mv and then exits.\n\nEvery fix has a regression test proven red against the old behaviour (reverted hunk, failing assertion, restored). Validated with go build ./..., go vet, go test -race -count=1 on internal/tool/bash, internal/process/runtime, internal/process/remote and internal/process/sandboxed, CGO_ENABLED=0 GOOS=darwin vet/test-compile, and gofmt on the changed paths.

fix(bash): prefer /bin/bash where it exists, then the first bash on PATH, and refuse with a clear error when neither exists - no silent sh fallback.\n\nfix(process/runtime): read /proc/<pid>/stat and cmdline on Linux for kill verification (ps is missing on NixOS and in container images); zombies stay "gone" and an empty command line stays an error. The ps path stays for unix platforms without /proc.\n\nfix(process/remote): the endpoint launcher resolves bash (/bin/bash first, then PATH) instead of hardcoding it, and records exit 127 with an error when no bash exists instead of dying and leaving the record "running" for ever. The launcher lookup helper reads /proc on Linux.\n\nfix(process/sandboxed): the kill tree walk recursed inside a shell for-loop whose variable the recursion clobbered, so only the leader and the deepest leaf were signalled and a kill reported success while the command shell kept running; the pending pids now travel in the positional parameters. The status script re-reads the exit file with a bounded grace before reporting an unknown exit, because the subshell writes the exit code with an external mv and then exits.\n\nEvery fix has a regression test proven red against the old behaviour (reverted hunk, failing assertion, restored). Validated with go build ./..., go vet, go test -race -count=1 on internal/tool/bash, internal/process/runtime, internal/process/remote and internal/process/sandboxed, CGO_ENABLED=0 GOOS=darwin vet/test-compile, and gofmt on the changed paths.
NixOS hosts and container images have no /bin at all, so every default-shell invocation failed at Start with a fork/exec /bin/bash: no such file or directory error. The default now prefers /bin/bash where it exists and falls back to the first bash on PATH; with neither it refuses with a clear error instead of silently running sh, because the tool promises bash semantics and quietly reinterpreting them would be a wrong answer, not a fallback. A caller-provided Shell is still used verbatim.
processCommand shelled out to ps on every platform, but NixOS hosts and container images ship without procps, so every kill verification failed with exec: ps: executable file not found instead of reading the same facts from /proc, which Linux always mounts. Linux now reads /proc/<pid>/stat (state after the last paren, so a comm containing parens cannot fake a zombie) and /proc/<pid>/cmdline; the ps path stays for the unix platforms without /proc. Zombies are answered before the cmdline read because a zombie cmdline is empty by definition, and an empty cmdline on a live process stays an error, never an empty success. The kill tests that could hit the exec window now wait for /proc to expose a command line.
The endpoint launcher hardcoded /bin/bash -lc command; an endpoint without /bin (NixOS, container images) made it die with an unhandled FileNotFoundError before it could record anything, so every command on such an endpoint stayed running until a poll settled it as the process exited without recording a status. The launcher now resolves /bin/bash first and the first bash on PATH second, and with neither it records exit 127 with an error naming bash instead of crashing. The launcher lookup helper in the tests reads /proc/<pid>/cmdline on Linux; the ps path stays for macOS/BSD.
The tree walk recursed inside a for loop over a shell variable, and POSIX sh has no local variables: the recursion clobbered the loop variable, so tree() returned only the deepest leaf. A kill signalled the leader and the leaf, reported success (the leader in the pid file was dead, so the status check passed), and the shell that ran the command kept running - TestContract kill/close cells showed a live process tick after the manager reported it dead. The pending pids now travel in the positional parameters, which are restored across a call.

The status script could also report an unknown exit for a process that exited 0: the subshell writes the exit code with an external mv and then exits, so a reader that sampled the file before the rename and the pid after the reap landed in the else branch. That branch now re-reads the exit file with the same bounded grace the entry guard gives, and still reports the honest unknown exit for a subshell that died without recording.

The ps-lstart fingerprint fallback test leads PATH with a stub, so it runs on hosts without procps too.
kreato closed this pull request 2026-11-10 18:21:10 +00:00

Pull request closed

Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kreato/yazi!68
No description provided.