Skip to content

fix process output capture and timeouts - #463

Merged
Sayan- merged 13 commits into
mainfrom
hypeship/process-output-lifecycle
Oct 23, 2026
Merged

Sayan- merged 13 commits into
mainfrom
hypeship/process-output-lifecycle

Conversation

@Sayan-

@Sayan- Sayan- commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

commands can stall when nobody consumes their output, and process exit can race capture and lose final output or the exit event. exec timeouts kill only the direct child and return a retryable 500; spawn does not enforce its timeout.

capture non-TTY stdout/stderr into bounded temporary files independently of readers. cursorless streams keep shared consumption, so reconnects continue unread output without replaying consumed output or exit events. explicit offset=0, saved offsets, and Last-Event-ID opt into independent replay/resume. Last-Event-ID takes precedence over the query cursor so automatic EventSource reconnects do not repeat output. the response visitor writes SSE directly. retained output expires after 10 seconds, including stalled streams; the default limits are 64 MiB per process and 256 MiB total.

exec and spawn enforce deadlines against the process group. exec failures return 422 with a reason and partial output. capture readers report completion and errors through one result channel; terminal status is published after both results are processed. manual /kill still forwards the requested signal to the direct process, and TTY sessions no longer allocate unused capture files. inherited pipes have a one-second drain bound; deliberately detached descendants may survive.

Validation

built and ran both headless and headful images from fc08a38, then sent requests to their running APIs. these are the key checks and observed results:

  • left output unread: ran a command that wrote 4 MiB followed by tail. it finished without a reader. connecting two seconds later returned every byte, including tail, followed by the exit event.
  • disconnected and reconnected: received first, disconnected, then let the command print second. reconnecting returned only second. explicitly requesting the history returned firstsecond.
  • timed out a command: ran printf partial; sleep 30 with a one-second timeout. the API returned 422 process_timeout and preserved partial. the SDK sent only one request. separate checks confirmed ordinary child processes also stopped.
  • interrupted clients and the server: disconnecting an exec request and shutting down the server stopped the commands and their ordinary children. a client that stopped reading output was released after about 10 seconds.
  • filled the output storage: the command stopped with output_error, previously captured output remained readable, and new commands could write again after cleanup. exceeding the configured output limit similarly reported output_limit.
  • used an interactive shell with output storage unavailable: WebSocket attach, resizing, and typing still worked. sending hello produced reply:hello.

example output printed by the SDK timeout check, on both images:

{"requests":1,"status":422,"code":"process_timeout"}

known trade off: a child that deliberately detaches can survive a timeout. that check returned after about two seconds with output marked incomplete; the surviving child was cleaned up afterward.


Note

High Risk
Changes core remote command execution (process groups, output limits, HTTP 422 contracts, and long-lived SSE), which can affect callers relying on old timeout/500 behavior or unbounded output.

Overview
Refactors process exec/spawn so non-TTY stdout/stderr are captured into bounded on-disk buffers (per-process and global quotas via PROCESS_OUTPUT_*) whether or not anyone is streaming, with 10s retention and replay/resume via SSE offset / Last-Event-ID.

Exec and spawn share startProcess lifecycle: process-group deadlines and cancellation/shutdown use SIGKILL on the group (with a short post-exit drain), /process/exec timeouts and server-driven stops return HTTP 422 with termination_reason and partial base64 output instead of retryable 500s, and status/stream exit events expose output_truncated. TTY spawn skips file capture; manual /kill still signals only the direct process.

Adds docs/processes.md, OpenAPI/schema updates, scale-to-zero fix so handler SetWriteDeadline is not overridden on long streams, plus broad unit/e2e coverage.

Reviewed by Cursor Bugbot for commit fc08a38. Bugbot is set up for automated code reviews on this repo. Configure here.

@Sayan-
Sayan- marked this pull request as ready for review October 14, 2026 22:09

@chruffins chruffins left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

reviewed — requesting changes. two correctness bugs and one structural item to address first.

Bugs

  • server/cmd/api/api/process_runner.go:202 — exitCode is published before pending capture failures are drained (:208-220). GET status can return exited without the output_limit/output_error reason or output_truncated: true. SSE and exec wait on doneCh, so they're unaffected. consider draining failures before publishing exitCode; all capture producers have finished at that point, so close(failures) plus a range is safe.
  • server/cmd/api/api/process_runner.go:29 — PTY spawns create a capture file they never use (TTY output never reaches captureOutput, :134), so a PTY spawn fails when PROCESS_OUTPUT_DIR is unusable or disk/fd pressure is high. consider opening capture only on the non-TTY path.
  • server/cmd/api/api/process.go:292 (fallback in child_process_other.go:20-29) — the non-Linux fallback turns INT/HUP into Kill(), where base forwarded the requested signal. non-Linux is dev-only, so low urgency, but the fallback could forward non-KILL signals with Process.Signal.
  • server/cmd/api/api/process_runner.go:201 with process_wait_linux.go:16 — on Unikraft (staging lib/posix-process/wait.c), waitid reaps an already-exited child even with WNOWAIT. the following cmd.Wait() then gets ECHILD, so ProcessState is nil and exit_code reports -1 for fast exits. this also breaks the PID-reuse invariant described at process_wait_linux.go:12-13. not yet confirmed on the deployed runtime; worth an exec of true on a Unikraft build. one possible fix is passing a non-nil infop and reading the status from it.

Structural / Maintainability

  • server/cmd/api/api/process.go:393-450 — each SSE request builds an io.Pipe, a cancel wrapper, and about four goroutines before the response visitor (:465) writes bytes. the visitor could own the replay loop and write directly, which removes the pipe and its goroutines rather than moving them.

Coverage

19 non-generated changed files reviewed statically. server/lib/oapi/oapi.go is generated and was checked for schema consistency only. no tests were run.

@Sayan-
Sayan- requested a review from chruffins October 15, 2026 00:38

@chruffins chruffins left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

reviewed — approving. one structural item below worth doing as a follow-up or before merging if you have time.

Structural / Maintainability

  • server/cmd/api/api/process_runner.go:136-204 — capture completion and capture failure use separate channels, a WaitGroup, a waiter goroutine, two completion flags, and a post-loop drain. each reader could send exactly one result (nil on success) onto a single channel, and the loop would consume those before publishing terminal status. that removes the waiter goroutine, the separate failure queue, the drain pass, and the redundant flags, and the runner's coordination gets much easier to follow.

Coverage

23 changed files reviewed statically at ae4aca7c. server/lib/oapi/oapi.go is generated and was checked for schema consistency only. no tests were run.

@hiroTamada hiroTamada left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

reviewed — no blocking findings. two nonblocking suggestions:

  • server/cmd/api/api/process.go:388–391 — consider letting Last-Event-ID override the query cursor. an EventSource initialized with ?offset=0 otherwise replays everything on automatic reconnect.
  • server/cmd/api/api/process_lifecycle_test.go:208 — consider adding permanent tests for exec request cancellation and spawn shutdown. temporary checks covering both passed.

validation: full non-e2e suite passed with -race; go vet ./... passed. container e2e compiled but wasn't run.

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit fcb2ed6. Configure here.

Comment thread server/cmd/api/api/process_runner.go
@Sayan-
Sayan- merged commit 2c3fabc into main Oct 23, 2026
12 checks passed
@Sayan-
Sayan- deleted the hypeship/process-output-lifecycle branch October 23, 2026 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants