Skip to content

fix(agentos): wait for a spawned process to start before reading its output - #2024

Merged
eersnington merged 2 commits into
mainfrom
fix/client-read-output-waits-for-spawn
Oct 8, 2026
Merged

eersnington merged 2 commits into
mainfrom
fix/client-read-output-waits-for-spawn

Conversation

@eersnington

@eersnington eersnington commented Oct 8, 2026 •

Copy link
Copy Markdown
Member
  • The client's AgentOs::spawn_process returns the pid before the sidecar has the process. The Execute request goes out from a background task.
  • The read_process_output call sent ReadProcessOutputRequest right away, so a read soon after a spawn, or after a launch that failed, got ESRCH. The launch error (for example a missing command) was recorded only in the local exit state, so callers never saw it.
  • Now read_process_output waits until the process has a kernel pid or a settled outcome, then reads, or returns the launch error. wait_process already waits the same way.
before: kernel error [ESRCH]: process proc-1000001-… does not exist or its replay expired
after:  kernel error [ENOENT]: command not found on native sidecar path: nosuchcmd
  • The signal_process_awaited and resize_process_pty_awaited calls also send the process id before the launch lands. They are not changed here.

  • Adds output_read_right_after_a_failed_spawn_returns_the_launch_error to crates/client/tests/process_e2e.rs: a read right after spawning a missing command returns ENOENT. On main it returns ESRCH.

@railway-app

railway-app Bot commented Oct 8, 2026

Copy link
Copy Markdown

This PR was not deployed automatically as @eersnington does not have access to the Railway project.

In order to get automatic PR deploys, please add @eersnington to your workspace on Railway.

@the-company-company the-company-company 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.

🟠 1 medium-severity finding

Reviewed commit b11c4ce.

Comment on lines +1025 to +1026
// request lands or fails, so a read never reaches the sidecar before the process exists.
loop {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Medium · Track spawn acknowledgement separately from the optional kernel PID

kernel_pid is not a reliable acknowledgement flag: ProcessStartedResponse.pid is explicitly optional, and run_spawn accepts a successful response while only updating this watch channel for Some(pid). With a valid process_started response containing no PID, a long-running process leaves both started as None and outcome as Pending, so this call waits indefinitely even though replay is already available. Add a dedicated spawn-result/readiness watch (as the shell path does), and signal it on every successful send_execute response rather than overloading the optional PID mapping.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

not reachable. slop

@the-company-company the-company-company 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.

🟠 1 medium-severity finding

Reviewed commit d84d3a2.

Comment thread crates/client/src/process.rs
@eersnington
eersnington merged commit 5a7b5c3 into main Oct 8, 2026
4 of 5 checks passed
@eersnington
eersnington deleted the fix/client-read-output-waits-for-spawn branch October 8, 2026 19:06
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.

1 participant