Release v0.51.304 — Release JT (stage-p2a — un-held terminal reaper + opt-in Docker GPU)

[nesquena-hermes]

Release v0.51.304 — Release JT (stage-p2a)

Un-held terminal-reaper fix + opt-in Docker GPU image. Both rebased onto fresh master (stale bases), fidelity-verified byte-identical to PR head.

Fixed

• fix(terminal): reap embedded terminal descendants (#2577) #3725 (@rodboev, fixes [bug] api/terminal.py: Node child processes become zombies when Popen handle is not reaped (same class as #1912) #2577) — UN-HELD this sweep. Embedded-terminal descendant reaper was process-wide (os.waitpid(-1, WNOHANG)), which could reap an unrelated WebUI subprocess child (git/update/provider) before its owner called .wait() and silently coerce its exit code to 0. Now scoped to the terminal's own process group (-terminal_pgid; terminals spawn start_new_session=True so pid==pgid). Bounded + lock-guarded.

Added

• docs(docker): add opt-in GPU runtime image path (#3243) #3721 (@rodboev, addresses Feature Request: Add GPU hardware acceleration libraries (for video editing scenario) to the Docker image #3243) — optional GPU runtime image via --build-arg INSTALL_GPU_LIBS=1 (default 0 = no-op, CPU-only); docker_init.bash preserves --group-add supplemental device groups when dropping privileges.

Gates

• Full suite: 8132 passed, 0 failed
• Codex regression gate: SAFE TO SHIP (targeted check: terminal-PGID child reaped, unrelated-PGID child stays waitable)
• Opus correctness gate: SAFE TO SHIP (audited group loop under set -e — all failure modes non-fatal, no-op for stock docker run)
• Ruff forward gate: CLEAN
• Revert-guard: origin/master is ancestor

Closes #3725, closes #3721.

[greptile-apps]

Greptile Summary

This PR ships two independent fixes rebased onto master: a scoped terminal-descendant reaper in api/terminal.py and an opt-in Docker GPU image path.

• Terminal reaper fix: replaces the process-wide os.waitpid(-1, WNOHANG) with os.waitpid(-terminal_pgid, WNOHANG), scoping zombie reaping to the terminal's own process group (possible because terminals are spawned with start_new_session=True, making pid == pgid). Called from both _reader_loop and close_terminal, guarded by a threading lock. Four targeted tests cover PGID isolation, bounded iteration, error handling, and the close-terminal integration.
• Optional GPU image: ARG INSTALL_GPU_LIBS=0 (default no-op) installs VA-API libraries when set to 1; docker_init.bash loops over Docker-provided supplemental GIDs and ensures hermeswebui inherits them before su drops privileges, with graceful fallbacks throughout. Docs and snapshot tests accompany both changes.

Confidence Score: 4/5

Safe to merge; the terminal reaper logic is well-scoped and well-tested, and the GPU/Docker path is purely opt-in with graceful fallbacks everywhere.

Both changes are narrow and self-contained. The PGID-scoped reaper correctly avoids the original cross-session clobber, and the Docker group loop suppresses every failure mode with || true. The only issues found are a dead guard in docker_init.bash, an over-broad threading lock in the reaper, and a hardcoded old-version sentinel in one test that would raise ValueError if that changelog entry is ever pruned.

docker_init.bash (dead guard) and tests/test_docker_gpu_runtime_docs.py (fragile changelog sentinel) deserve a quick look; api/terminal.py and the rest of the test suite look solid.

Important Files Changed

Filename	Overview
api/terminal.py	Adds _reap_terminal_descendants() — scoped waitpid(-pgid, WNOHANG) replacing the old process-wide form — called from both _reader_loop and close_terminal; guarded by a global lock (unnecessarily broad but correct).
docker_init.bash	Adds a group-preservation loop that maps Docker --group-add GIDs to named groups and adds hermeswebui to each before privilege drop; contains one dead if [ -n "$group_name" ] guard after a continue.
Dockerfile	Introduces opt-in ARG INSTALL_GPU_LIBS=0 block; default path is a no-op, GPU libs only installed on =1; Intel non-free driver has graceful apt-cache show fallback.
tests/test_terminal_zombie_reaper.py	Linux-only tests covering the PGID-scoped reaper: fork/exec integration test, monkeypatched close_terminal flow, error-handling, and bounded-iteration tests — all logically sound.
tests/test_docker_gpu_runtime_docs.py	Snapshot tests asserting Dockerfile, docker_init.bash, docs, and CHANGELOG content; the changelog test anchors its upper bound on a hardcoded older version string that will raise ValueError if that entry is ever pruned.
docs/docker.md	Adds GPU runtime documentation covering Intel/AMD VA-API and NVIDIA passthrough; explicitly disclaims native verification and documents the supplemental-group preservation behavior.
CHANGELOG.md	Adds v0.51.304 entry documenting the PGID-scoped reaper fix and the opt-in GPU image path.

Sequence Diagram

sequenceDiagram
    participant RT as _reader_loop (thread)
    participant CT as close_terminal (caller)
    participant R as _reap_terminal_descendants
    participant OS as os.waitpid

    Note over RT,OS: Shell exits naturally
    RT->>RT: proc.poll() → stores returncode
    RT->>R: _reap_terminal_descendants(proc.pid)
    R->>OS: waitpid(-pgid, WNOHANG) [scoped to terminal PGID]
    OS-->>R: (child_pid, status) or (0,0)
    R-->>RT: reaped count

    Note over CT,OS: close_terminal called concurrently
    CT->>CT: killpg(SIGHUP) + proc.wait(1.5s)
    CT->>R: _reap_terminal_descendants(proc.pid)
    Note over R: global lock serializes concurrent calls
    R->>OS: waitpid(-pgid, WNOHANG)
    OS-->>R: ECHILD / (0,0) — already reaped
    R-->>CT: 0

Loading

_{Reviews (1): Last reviewed commit: "docs(changelog): stamp v0.51.304 — Relea..." | Re-trigger Greptile}

[greptile-apps]

The if [ -n "$group_name" ] check on line 285 is unreachable dead code. The continue two lines above already guarantees that group_name is non-empty when execution reaches this point — any empty-group_name path has already bailed out. The guard adds noise and could mislead future readers into thinking there is a case where group_name is empty here.

Suggested change

	if [ -z "$group_name" ]; then
	echo "!! WARNING: Could not create supplemental group for GID $gid; GPU device access may be unavailable"
	continue
	fi
	if [ -n "$group_name" ]; then
	usermod -a -G "$group_name" hermeswebui 2>/dev/null || echo "!! WARNING: Could not add hermeswebui to supplemental group $group_name ($gid)"
	fi
	if [ -z "$group_name" ]; then
	echo "!! WARNING: Could not create supplemental group for GID $gid; GPU device access may be unavailable"
	continue
	fi
	usermod -a -G "$group_name" hermeswebui 2>/dev/null || echo "!! WARNING: Could not add hermeswebui to supplemental group $group_name ($gid)"

[greptile-apps]

The test anchors the upper bound of the changelog slice on the literal string "## [v0.51.293]". If changelog history is ever trimmed and that entry is removed, the test raises ValueError: substring not found rather than giving a useful assertion failure. A safer sentinel is the nearest preceding release entry.

Suggested change

	def test_changelog_mentions_optional_gpu_runtime_path():
	changelog = _repo_text("CHANGELOG.md")
	unreleased = changelog[changelog.index("## [Unreleased]"):changelog.index("## [v0.51.293]")]
	def test_changelog_mentions_optional_gpu_runtime_path():
	changelog = _repo_text("CHANGELOG.md")
	# Use the immediately preceding release as the lower-bound sentinel so the
	# slice stays valid even if older entries are pruned from the changelog.
	unreleased = changelog[changelog.index("## [Unreleased]"):changelog.index("## [v0.51.303]")]

[greptile-apps]

Global lock over-serializes independent PGIDs

_terminal_descendant_reaper_lock is process-wide, so two concurrent calls for different terminal sessions block each other even though waitpid(-pgid_A, …) and waitpid(-pgid_B, …) are completely independent at the kernel level. The lock is only required to prevent two callers from racing on the same terminal's PGID. In practice the impact is negligible (each call exits after one ECHILD in the common case), but a per-session lock stored on TerminalSession would be more precise if this code ever needs to scale to many concurrent terminal closings.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!