Distillation

Date: Sep 8, 2026

• 🔴 [requested-review] (05:15) User requested resumption of the same read-only correctness review against the unchanged exact source under /home/byk/.local/share/opencode-v2-pilot/supervisor, requiring substantive file:line findings or the exact blocking tool/error; coverage of every source, test, unit, protocol, and documentation file; verification of pinned go-systemd/godbus API semantics; PASS/CONCERN/MUST-FIX classifications; no edits, service installation/start, privileged operations; and an exact final line of MERGE or DO-NOT-MERGE.
• 🟡 (05:15) Assistant identified a concrete launcher mismatch: script -c gives the child a PTY but invokes its shell non-interactively, whereas the design requires an interactive inner shell.
• 🟡 (05:16) Exact-host probe of the launcher shell behavior returned noninteractive.
• 🟡 (05:20) A command intended to invoke sha256sum failed with /usr/bin/bash: -c: line 1: syntax error near unexpected token 'placeholder' because the literal invalid shell expression sha256sum(placeholder) was used.
• 🟡 (05:20) Assistant’s ordered correctness findings were: 1. MUST-FIX — Node plugin does not implement the protocol: supervisor listens on /run/opencode-pty/supervisor.sock at internal/supervisor/server.go:23-28 and requires a framed start request plus one SCM_RIGHTS descriptor at internal/supervisor/server.go:219-257, while /home/byk/.local/share/opencode-v2-pilot/config/opencode/plugins/pty.ts:223-230 still directly spawns /usr/bin/script and never opens the socket, constructs OPTY frames, transfers a descriptor, or parses output/exit frames; Node standard net lacks Unix sendmsg(2) FD transfer, so proposed integration alternatives were a native binding or narrowly scoped helper. 2. MUST-FIX — authentication-to-start race: authenticate closes its pidfd on return at internal/supervisor/auth.go:12-18; the server subsequently accepts an attacker-controlled request/descriptor and calls Start at internal/supervisor/server.go:100-124, allowing an authenticated inherited/open connection to outlive a MainPID identity change; fix is to pin peer identity for the connection and reassert immediately before admission/StartTransientUnit. 3. MUST-FIX — completion deadlock: Wait considers SubState=exited complete at internal/supervisor/systemd.go:101-125, and RemainAfterExit=true at internal/supervisor/systemd.go:254 permits descendants to retain stdout; serveSession then synchronously waits for outputDone at internal/supervisor/server.go:167-176, preventing cancellation/input handling and delaying deferred stop at internal/supervisor/server.go:130; fix is to stop the unit before final EOF waiting or coordinate completion without blocking the select loop. 4. MUST-FIX — disconnect not detected during blocked input: copyInput may block indefinitely writing the unit stream at internal/supervisor/server.go:188-200; if the command does not read stdin, the client can fill the socketpair and disconnect while the session slot/unit remain indefinitely when RuntimeSec=0; needs cancellable I/O or independent disconnect detection. 5. MUST-FIX — cleanup errors discarded: successful starts use an unchecked cleanup defer at internal/supervisor/server.go:130; StopAndWait D-Bus, subscription, or timeout failures from internal/supervisor/systemd.go:128-165 are ignored before releasing the session slot, violating complete-control-group cleanup guarantees. 6. MUST-FIX — non-interactive inner shell: cmd/launcher/main.go:49 executes script -qefc COMMAND /dev/null, matching PROTOCOL.md:15, but the exact-host $- probe returned noninteractive; invocation must explicitly produce an interactive shell and have an exact smoke test. 7. MUST-FIX — plugin/protocol limits conflict: supervisor allows 4,096 bytes per argument, 3,600 seconds runtime, and 32,768-byte input payloads at internal/protocol/frame.go:11-24, while plugins/pty.ts:19-27,45-65 allows 16,384-byte arguments, timeout through 2,147,483 seconds, and 65,536-byte writes; transport integration would therefore reject otherwise valid tool inputs unless inputs are explicitly split/clamped and documented. 8. MUST-FIX — inadequate lifecycle/protocol tests: server_test.go:16-131 only covers descriptor receipt, one fragmented header, one invalid input type, and a locally reconstructed exit payload—not serveSession, start/disconnect races, backpressure, stream EOF ordering, cleanup failure, or structured exit delivery; systemd_test.go:27-49 checks property names but not D-Bus signatures/effective behavior; auth_test.go:30-58 uses same-process socketpairs and omits PID replacement, inherited connections, pidfd death, wrong cgroup, and authentication-to-start races; ROOT-ACCEPTANCE.md:17-29 marks these tests mandatory and forbids service startup before they pass. 9. CONCERN — FD receipt lacks atomic close-on-exec: internal/supervisor/server.go:260-289 uses ReadMsgUnix without MSG_CMSG_CLOEXEC and calls CloseOnExec only after parsing; current lack of local child execution limits exposure, but kernel-atomic MSG_CMSG_CLOEXEC is safer; exact-one descriptor enforcement, truncation checks, error cleanup, directory validation, and st_dev/st_ino comparison were otherwise correct at internal/supervisor/server.go:219-257. 10. CONCERN — incomplete wire specification: PROTOCOL.md:11-13 omits exact widths/order of runtime, argument count, argument lengths, and error-frame payload format; those details exist only in internal/protocol/frame.go:76-143, which an independent Node client cannot treat as a language-neutral wire spec. 11. CONCERN — no per-session limits: opencode-pty.slice:5-10 caps all PTYs collectively at 2 GiB and 512 tasks, while transient properties at internal/supervisor/systemd.go:236-260 set no per-unit memory/task limit, allowing one PTY to starve the other 15 sessions. 12. CONCERN — pre-install unit verification incomplete: unprivileged systemd-analyze verify parsed both units but reported Command /usr/local/libexec/opencode-pty-supervisor is not executable: No such file or directory; this matches the pre-install state and root-only gate in ROOT-ACCEPTANCE.md:5-15, but effective verification remains outstanding.
• 🟡 (05:20) Assistant’s verified passes were: 1. Frame parsing validates magic, version, reserved bytes, known type, exact packet length, and the 65,536-byte cap at internal/protocol/frame.go:45-74,159-184. 2. Start decoding rejects empty commands, excessive counts/lengths, NUL bytes, invalid runtimes, truncation, and trailing data at internal/protocol/frame.go:76-143. 3. Descriptor handling requires exactly one descriptor, checks MSG_TRUNC/MSG_CTRUNC, closes descriptors on all returned errors, requires a directory, and compares st_dev plus st_ino at internal/supervisor/server.go:219-289. 4. Cwd stays inode-pinned through D-Bus/launcher startup: systemd receives it as StandardErrorFileDescriptor at internal/supervisor/systemd.go:256-258, and the launcher performs fchdir(2) before replacing stderr at cmd/launcher/main.go:24-34. 5. Launcher argv uses canonical URL-safe unpadded base64 with strict count, length, grammar, NUL, and shell-quoting validation at cmd/launcher/main.go:52-72; script -e propagates ordinary command exit status. 6. Launcher clears inherited environment state and establishes fixed values before exec at cmd/launcher/main.go:35-49. 7. StartTransientUnitContext usage matches pinned go-systemd v22.5.0; PropExecStart expects executable path followed by argv, D-Bus signature is s s a(sv) a(sa(sv)) -> o, and descriptor properties correctly use dbus.UnixFD for D-Bus type h. 8. Unit names use 128 random bits and strict lowercase grammar at internal/supervisor/server.go:349-355 and internal/supervisor/systemd.go:22-27,287-289. 9. KillMode=control-group, TERM-to-KILL settings, Delegate=false, fixed ownership, slice placement, and supervisor binding are set at internal/supervisor/systemd.go:236-260; opencode-pty-supervisor.service:6-33 uses a root-controlled executable path and substantial hardening. 10. Wait subscribes before inspecting unit state, avoiding the common completion-subscription race at internal/supervisor/systemd.go:101-125. 11. Orphan cleanup requires strict unit-name matching and exact BindsTo=opencode-pty-supervisor.service ownership at internal/supervisor/systemd.go:206-227. 12. Structured exit encoding preserves systemd Result, ExecMainCode, and ExecMainStatus at internal/supervisor/systemd.go:271-284 and internal/supervisor/server.go:177-183. 13. go.mod:3-8 and go.sum:1-7 pin and checksum the reviewed dependencies. 14. cmd/supervisor/main.go:13-25 propagates initialization/serving failures and uses signal cancellation. 15. Both checked-in binaries matched fresh go build -trimpath outputs byte-for-byte; observed comparison results were launcher_cmp=0 and supervisor_cmp=0, with SHA-256 f97c546f46b6f2b818638c69ced6e461174549207225cd034ecf65da1017c20d for bin/opencode-pty-launcher and /tmp/opencode/review-opencode-pty-launcher, and 65f00b20269806f513f5c6a479a7ca051db6a334ca4549121e5b813bbdf75989 for bin/opencode-pty-supervisor and /tmp/opencode/review-opencode-pty-supervisor. 16. Final hashes of all 19 reviewed files remained unchanged throughout the review. 17. go test -count=1 ./..., go test -race -count=1 ./..., and go vet ./... all passed.
• 🟡 (05:20) Assistant concluded the current source cannot safely replace the direct-spawn plugin, must not pass the root acceptance gate, and issued the final verdict DO-NOT-MERGE.