Skip to content

feat(process-daemon): write pipe output to a file - #60701

Closed
jlongster wants to merge 9 commits into
devfrom
daemon-pipe-output
Closed

jlongster wants to merge 9 commits into
devfrom
daemon-pipe-output

Conversation

@jlongster

@jlongster jlongster commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Issue for this PR

Closes #

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Lets a pipe process write its output straight to a file, so OpenCode can keep a command's complete output even when it exceeds the 2 MiB replay window, finishes before the server subscribes, or runs while no server is attached during a restart handoff. Stacked on #60697.

  • create takes output_path (pipe only; feature output_path, so clients only send it to a daemon that lists it). It must be absolute; PTYs reject it.
  • The daemon opens the file before spawning: created with mode 0600 if missing, otherwise appended to. A path that can't be opened fails create without starting the process.
  • The daemon opens it with O_NOFOLLOW, so a symlink as the final path component is refused.
  • The reader thread writes each chunk to the file before handing it to the actor, so the file always holds at least what subscribers have seen and is never held back by a slow subscriber. The file is closed before the exit is published.
  • A later write error (disk full, quota) stops the file and sets output_error on TerminalInfo and on the exited event (omitted otherwise), ordered before the output the file lacks, so OpenCode can tell the saved file is incomplete. The process keeps running.
  • output_path is left out of the JSON when unset, so PTY create requests serialize exactly as before.

OpenCode will point this at the shell tool's existing sh_*.out files, so Shell.output and the "full output saved to …" notice keep working unchanged.

How did you verify your code works?

cargo fmt --check, cargo clippy --all-targets --all-features -- -D warnings and the macOS test suite in packages/process-daemon; clippy for x86_64-pc-windows-msvc and x86_64-unknown-linux-gnu. New tests:

  • with a 1 KiB replay window and no subscriber, seq 1 20000 plus stderr lands in the file in full (the replay is truncated), with mode 0600
  • an existing file is appended to
  • a relative path, a PTY with output_path, an unopenable path and a symlink are rejected, and nothing is spawned
  • Linux: writing to /dev/full sets output_error while output and exit still reach subscribers
  • the real-daemon handoff test now also checks the file holds the full output after the restart handoff
  • protocol test for output_path on the wire

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Added by the opencode bot (AI-generated).

Public API: process-daemon protocol: adds the output_path feature, an optional create.output_path, and optional output_error on TerminalInfo and the exited event (backwards compatible)

@jlongster
jlongster requested a review from a team as a code owner November 2, 2026 17:30
Base automatically changed from daemon-pipe-mode to dev November 2, 2026 18:27

@opencodejr opencodejr Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks sound and works as described. I built the daemon on Linux and ran the lib, ownership and pipe tests. All the new output-file tests pass, and the handoff test confirms the file holds the full output across a restart handoff. Two older pipe tests fail here, and they fail the same way on the base: the sandbox doesn't reap orphaned processes. I didn't run clippy, fmt, or the macOS or Windows targets.

Public API: this changes only the process-daemon wire protocol, not the SDK, HTTP API, plugins or config. Every change is an addition:

  • features gains output_path on Unix.
  • create accepts an optional output_path.
  • TerminalInfo and the exited event gain an optional output_error.

All of it is backwards compatible. The new fields are left out of the JSON when unset, so PTY create requests and normal exited frames are byte-identical to before. Clients only send output_path to a daemon that lists the feature.

No blocking concerns.

@jlongster

Copy link
Copy Markdown
Contributor Author

Superseded by #61875, which includes these commits (rebased) plus protocol v8.

@jlongster jlongster closed this Nov 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant