Fix headless PTY background launches on macOS#26
Conversation
…not a TTY When running hcom in headless/background mode, the launcher spawns the PTY process with stdin=/dev/null. On /dev/null, poll() immediately returns POLLIN (always readable), and the subsequent read returns Ok(0) (EOF). The existing code treated any stdin EOF as 'terminal gone' and broke the poll loop, which then killed the child agent with SIGTERM and marked it as launch_failed. However, the poll-timeout path already had an isatty() guard for this exact scenario — but when stdin=/dev/null, poll() never times out because /dev/null is always readable, so the isatty check was bypassed. Fix: guard the Ok(0) => break path with an isatty check. Only break on stdin EOF if stdin is actually a TTY. If stdin is /dev/null or a pipe, ignore EOF and continue polling. Fixes: background/headless agent launches being killed immediately.
Set HCOM_LAUNCHED=1 in hcom pty child environments so downstream hooks and tool integrations correctly recognize PTY-managed sessions as hcom launches.
The previous fix used PollFlags::empty() for stdin when poll_stdin=false, but on macOS poll() may still return immediately for a readable fd even with events=0 (e.g. /dev/null in headless mode). This caused a tight busy-wait loop consuming 99.9% CPU. Fix: dynamically build poll_fds and completely omit stdin when we're no longer polling it. Adjust inject_listener and client indices accordingly. Also wraps stdin handling in 'if poll_stdin' to avoid accidentally checking inject listener revents when stdin is absent.
- propagate HCOM_INSTANCE_NAME into background PTY runner env - apply tool-specific PTY env on background launches - back off inject listener after WouldBlock on macOS - drop stdin from PTY polling on POLLNVAL - reject bare Claude headless launches without a prompt/task - add regression tests for launcher env and PTY helpers
- ignore stdin POLLHUP for non-TTY headless PTY sessions - make inject accept regression test resilient to non-blocking timing
There was a problem hiding this comment.
Pull request overview
Restores stable macOS headless/background launches on the PTY-backed path by fixing env propagation, hardening the PTY poll loop against non-interactive stdin edge cases, and adding an explicit guard for unsupported “bare” headless Claude launches.
Changes:
- Propagate instance/tool-specific environment into background PTY runner launches and mark PTY-spawned children as hcom-launched.
- Harden the PTY event loop for macOS headless stdin/poll quirks and add inject-listener backoff behavior.
- Fail fast for
hcom claude --headlesswhen no prompt/task is provided, with regression tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/pty/mod.rs |
Adds child env injection support and updates poll loop behavior for stdin + inject listener handling. |
src/pty/inject.rs |
Changes accept() to return whether a connection was accepted; adds focused tests. |
src/main.rs |
Supplies PTY child env (HCOM_LAUNCHED=1) and adds a regression test. |
src/launcher.rs |
Ensures background PTY runner env includes HCOM_INSTANCE_NAME and tool_extra_env; adds tests. |
src/commands/launch.rs |
Adds validation to reject bare headless Claude without prompt/task; adds tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Only include stdin in poll set while we're actively polling it. | ||
| // When stdin is a non-TTY (e.g. /dev/null in headless mode), we stop | ||
| // polling it to avoid busy-waiting — but we must fully remove it from | ||
| // the poll set, not just pass empty events, because some platforms | ||
| // (macOS) may still return immediately for a readable fd even with | ||
| // events=0. | ||
| if poll_stdin { | ||
| poll_fds.push(PollFd::new(stdin_borrowed, PollFlags::POLLIN)); | ||
| } |
There was a problem hiding this comment.
poll_stdin can become false specifically when poll reports POLLNVAL (invalid/closed stdin fd). In that case, the next loop iteration still constructs stdin_borrowed via unsafe { BorrowedFd::borrow_raw(stdin_raw) } (a few lines above this hunk) even though stdin may be closed, which is undefined behavior. Suggestion: only create/borrow stdin fd inside the if poll_stdin { ... } block (or switch the isatty checks to libc::isatty(stdin_raw) so no BorrowedFd is needed for potentially-invalid fds).
| // Include the inject listener unless we're in backoff (macOS spurious POLLIN). | ||
| // Reset backoff here so it applies for exactly one iteration. | ||
| let include_listener = !listener_backoff; | ||
| listener_backoff = false; | ||
| let inject_listener_idx: Option<usize> = if include_listener { | ||
| let idx = poll_fds.len(); | ||
| poll_fds.push(PollFd::new(inject_listener_fd, PollFlags::POLLIN)); | ||
| Some(idx) | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
The one-iteration listener_backoff strategy can introduce large accept latency: when the listener is excluded, poll() can block for up to poll_timeout (10s) if the PTY is idle, so new inject connections (including delivery thread injections) may not be accepted until the timeout expires. Consider bounding this by using a much shorter poll timeout during backoff, or keeping the listener in the poll set and applying a small sleep/yield when accept() returns WouldBlock.
|
nice |
…istener backoff (#27) * fix(status): unify hook detection with verify_* and silence expected dev_root warnings hcom status used its own naive detection (substring match on settings.json, file-existence for the opencode plugin) while hcom hooks used the canonical verify_* functions. After hcom hooks remove, status kept reporting Claude/Gemini as installed because permission strings like "Bash(hcom ...)" still contained "hcom". Route all four tool checks through the same verify_* functions so status and hooks never disagree. router.read_dev_root_from_kv logged WARN on every expected "not configured" state (fresh HCOM_DIR where the db doesn't exist yet, unset dev_root key returning QueryReturnedNoRows). These fired on every hcom invocation and every hook dispatch. Treat them silently; keep WARN for real errors (permission denied, corruption, etc.). * fix(launch): normalize claude + --headless + prompt to print-headless Before: `hcom claude --headless --hcom-prompt 'task'` (or a positional task) passed validation but launched as a detached plain `claude` — the spec never saw `-p`/`--print`, so `add_background_defaults` didn't fire and the child never got `--output-format stream-json --verbose`. Thread initial_prompt into prepare_launch_execution. When tool == "claude" && background && a prompt is present, inject `-p` into the merged args before calling add_background_defaults so the child always runs through claude's print mode. Claude-specific — --headless semantics for codex/gemini/opencode unchanged. Addresses finding 1 of the PR #26 post-merge review. * fix(pty): cap poll timeout during inject-listener backoff The macOS kqueue spurious-POLLIN workaround drops the inject listener from the poll set for one iteration after WouldBlock. The poll timeout stayed at 10s (or 5s with debug) for that iteration, so an inject connection arriving while the listener was backed off could wait the full timeout before the listener re-entered the poll set. Cap the poll timeout at 100ms whenever the listener is excluded so the next iteration (with the listener re-included) runs quickly. Addresses finding 2 of the PR #26 post-merge review. * fix(scripts): pass -p explicitly to bundled claude --headless launches The launcher now normalizes claude + --headless + prompt to inject -p, but adding it explicitly in the bundled scripts makes intent clear and is belt-and-suspenders. Guard the non-claude cases (fatcow/debate can launch any tool) so -p only appears when the tool is claude. * fix(relay): enforce claude headless validation on remote launch path The local CLI short-circuits into dispatch_remote before local validate/prepare fires when --device is set, so the remote handler has to enforce the Claude invariant itself. Before this commit, bare remote `hcom claude --headless --device X` would prepare as background=true pty=false with no -p and fall straight through launcher::launch, preserving the unsupported detached plain-claude path on the remote. Call validate_claude_headless_launch after prepare_remote_launch so remote and local paths reject the same inputs. Bare claude --headless returns the same prompt/task error; claude --headless --hcom-prompt is normalized through the -p injection in prepare_launch_execution. Addresses @mafo's blocker in the PR #27 review thread. --------- Co-authored-by: aannoo <aannoo@users.noreply.github.com>
PR #26 landed `--headless` for claude via the detached print-mode path (-p --output-format stream-json --verbose, one-shot, exits after the prompt), and deferred a true PTY-backed live headless session. This commit adds that path. User-facing - New `--pty` launch flag. For claude, combines with `--headless` to launch the interactive TUI in a PTY wrapper inside a detached runner — same shape other tools already use, giving claude a live background session that accepts hcom inject. `--pty` alone (no --headless) keeps today's foreground claude-pty behavior. - `hcom claude --headless --pty` with no prompt is now a valid launch; the session sits idle waiting for hcom messages. - `--pty` added to `SHARED_LAUNCH_FLAGS` so it shows up in help for both fresh launches and resume/fork. Plumbing - `HcomLaunchFlags.pty` parsed in `extract_launch_flags`; forwarded into `prepare_launch_execution` and the dispatch-remote payload. - `prepare_launch_execution` now skips the -p injection + background defaults when `--pty` is set — the PTY wrapper hosts the TUI, and forcing print mode there would end the session on first reply. - `validate_claude_headless_launch` gets a `use_pty` parameter. Bare `claude --headless --pty` is allowed; the prompt-required invariant still applies to the NativePrint path. - `RemoteLaunchRequest.pty` carries the flag across the relay. Launcher - Drop the `bail!("Claude PTY does not support headless/background mode")` guard in `launch()`. - Route `LaunchTool::ClaudePty` through `launch_pty_or_background` the way gemini/codex/opencode do, so background mode spawns the PTY wrapper in a detached runner. - Introduce `LaunchBackend` enum (`InteractiveVisible` / `HeadlessPty` / `NativePrint`). Resolved once in `launch()` from the already- prepared (tool, background, pty) triple. Dispatch branches on the enum where it clarifies intent (claude's NativePrint-vs-visible split); the other tools' paths already encode the split via LaunchTool and use `launch_pty_or_background` directly. Tests - Unit: `--pty --headless` claude does not inject -p and keeps use_pty true; interactive claude-pty unchanged. - Unit: validator allows no-prompt `--pty --headless` and still rejects no-prompt non-PTY headless. - Unit: `LaunchBackend::resolve` matrix for each (tool, background, pty) combination. Live - `env -u HCOM_INSTANCE_NAME hcom claude --headless --pty --hcom-prompt 'respond via hcom send then stop'` → round-trip reply. - `env -u HCOM_INSTANCE_NAME hcom claude --headless --pty` (no prompt) → reaches listening, replies to `hcom send`.
…#28) * feat: PTY-headless claude via --pty, plus LaunchBackend enum PR #26 landed `--headless` for claude via the detached print-mode path (-p --output-format stream-json --verbose, one-shot, exits after the prompt), and deferred a true PTY-backed live headless session. This commit adds that path. User-facing - New `--pty` launch flag. For claude, combines with `--headless` to launch the interactive TUI in a PTY wrapper inside a detached runner — same shape other tools already use, giving claude a live background session that accepts hcom inject. `--pty` alone (no --headless) keeps today's foreground claude-pty behavior. - `hcom claude --headless --pty` with no prompt is now a valid launch; the session sits idle waiting for hcom messages. - `--pty` added to `SHARED_LAUNCH_FLAGS` so it shows up in help for both fresh launches and resume/fork. Plumbing - `HcomLaunchFlags.pty` parsed in `extract_launch_flags`; forwarded into `prepare_launch_execution` and the dispatch-remote payload. - `prepare_launch_execution` now skips the -p injection + background defaults when `--pty` is set — the PTY wrapper hosts the TUI, and forcing print mode there would end the session on first reply. - `validate_claude_headless_launch` gets a `use_pty` parameter. Bare `claude --headless --pty` is allowed; the prompt-required invariant still applies to the NativePrint path. - `RemoteLaunchRequest.pty` carries the flag across the relay. Launcher - Drop the `bail!("Claude PTY does not support headless/background mode")` guard in `launch()`. - Route `LaunchTool::ClaudePty` through `launch_pty_or_background` the way gemini/codex/opencode do, so background mode spawns the PTY wrapper in a detached runner. - Introduce `LaunchBackend` enum (`InteractiveVisible` / `HeadlessPty` / `NativePrint`). Resolved once in `launch()` from the already- prepared (tool, background, pty) triple. Dispatch branches on the enum where it clarifies intent (claude's NativePrint-vs-visible split); the other tools' paths already encode the split via LaunchTool and use `launch_pty_or_background` directly. Tests - Unit: `--pty --headless` claude does not inject -p and keeps use_pty true; interactive claude-pty unchanged. - Unit: validator allows no-prompt `--pty --headless` and still rejects no-prompt non-PTY headless. - Unit: `LaunchBackend::resolve` matrix for each (tool, background, pty) combination. Live - `env -u HCOM_INSTANCE_NAME hcom claude --headless --pty --hcom-prompt 'respond via hcom send then stop'` → round-trip reply. - `env -u HCOM_INSTANCE_NAME hcom claude --headless --pty` (no prompt) → reaches listening, replies to `hcom send`. * fix(launch): reject --pty + -p/--print conflict `hcom claude --headless --pty -p 'task'` (or bare `--pty -p`) routed through ClaudePty and spawned the PTY wrapper, but -p is claude's one-shot print mode — it answers and exits, tearing down the live session the moment the first reply lands. Silent misbehavior. Reject the combination explicitly in validate_claude_headless_launch rather than stripping -p, so the user notices: --pty means live TUI session, -p means print mode. Shared validator runs on both local and remote paths, so the remote path inherits the check. Addresses @mafo's edge-case finding on PR #28. --------- Co-authored-by: aannoo <aannoo@users.noreply.github.com>
Fix headless PTY background launches on macOS
...oh man, this was an odyssey.
Summary
Fixes the macOS
--headless --goregressions on the PTY-backed launch path.Validated end result:
Motivation
Headless launches on the PTY path had regressed in several overlapping ways, which made this investigation a bit deceptive at first:
hcom ptyeven startedWouldBlockhcom claude --headlesswithout a prompt/task looked like “headless PTY is broken” but is actually a separate unsupported detached-Claude pathUser-facing symptoms included:
launch_faileddelivery.start/notify.registeredWhat changed
Launcher / background PTY env
HCOM_INSTANCE_NAMEinto the background PTY runner envtool_extra_env(tool)on background PTY launches tooPTY loop hardening
POLLNVALPOLLHUPfor non-TTY headless PTY sessionsWouldBlockPTY child identity
HCOM_LAUNCHED=1Claude safety guard
hcom claude --headlesswithout a prompt/taskNotes / implementation details
HCOM_INSTANCE_NAMEon the background PTY path so delivery can initialize for the correct instance./dev/null, closed pipes, invalid fds).WouldBlock.HCOM_LAUNCHED=1PTY-child env fix ensures downstream hooks/tool integrations still recognize PTY-managed child sessions as hcom-launched.Validation
Automated
cargo testManual
./target/release/hcom codex --headless --golistening./target/release/hcom opencode --headless --golistening./target/release/hcom claude --headless --go./target/release/hcom claude --headless --go 'Ping!'Active-binary PTY re-check
./target/release/hcomdelivery.start/notify.registeredproxy.poll_spin_trace/POLLNVALlog entriesFinal pre-PR hardening
POLLHUPdisables stdin polling instead of breaking the PTY loopcargo testsuccessfullyDeferred follow-up
Not included in this PR:
(those are deferred follow-up items rather than blockers for restoring stable headless Codex/OpenCode PTY launches)
Commit stack
fix(pty): don't treat stdin EOF as terminal disconnect when stdin is not a TTYfix(pty): mark PTY child processes as hcom-launchedfix(pty): fully remove stdin from poll set when not a TTYfix(headless): restore stable PTY background launchesfix(pty): tighten headless stdin and inject test handling