app-server: Add streaming and tty/pty capabilities to command/exec#13640
app-server: Add streaming and tty/pty capabilities to command/exec#13640euroelessar wants to merge 1 commit intoruslan/pty-and-streamingfrom
command/exec#13640Conversation
euroelessar
force-pushed
the
ruslan/exec-stream
branch
from
64bb41c to
4473b52
Compare
euroelessar
force-pushed
the
ruslan/exec-stream
branch
from
4473b52 to
fce9180
Compare
euroelessar
force-pushed
the
ruslan/pty-and-streaming
branch
from
f06a405 to
7c5bb76
Compare
euroelessar
force-pushed
the
ruslan/exec-stream
branch
from
fce9180 to
e61a5d0
Compare
euroelessar
force-pushed
the
ruslan/pty-and-streaming
branch
from
7c5bb76 to
6cfc6f8
Compare
Define the v2 command/exec contract and wire it through app-server and core in one slice: processId, env overrides, timeout and output-cap controls, streaming notifications, and PTY write/resize/terminate support. Keep the generated schema, README updates, backend plumbing, and test harness changes together so reviewers can read the API and runtime behavior in the same commit.
euroelessar
force-pushed
the
ruslan/pty-and-streaming
branch
from
6cfc6f8 to
f59dcaf
Compare
euroelessar
force-pushed
the
ruslan/exec-stream
branch
from
e61a5d0 to
e2cc0a2
Compare
|
@codex review |
owenlin0
left a comment
owenlin0
left a comment
There was a problem hiding this comment.
api design looks great to me! 🚢
fyi I triggered a codex review to see if it catches anything in the impl
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
codex/codex-rs/core/src/exec.rs
Line 221 in e2cc0a2
Preserve ExecParams arg0 in build_exec_request
build_exec_request discards ExecParams.arg0 (arg0: _). Since process_exec_tool_call now always goes through this builder, caller-supplied argv[0] overrides are silently lost unless a sandbox transformation injects one. This changes execution behavior versus the prior direct spawn path and can break consumers that rely on custom arg0 semantics.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }; | ||
|
|
||
| for control in controls { | ||
| if let CommandExecSession::Active { control_tx } = control { |
There was a problem hiding this comment.
connection_closed only sends Terminate for CommandExecSession::Active and skips UnsupportedWindowsSandbox. Those Windows sessions are launched in start() via execute_env without a control channel, so a disconnect does not stop the child process. Long-running commands (especially with disableTimeout) can continue orphaned, leaking resources and violating connection-scoped lifecycle cleanup.
Useful? React with 👍 / 👎.
| - `thread/realtime/stop` — stop the active realtime session for the thread (experimental); returns `{}`. | ||
| - `review/start` — kick off Codex’s automated reviewer for a thread; responds like `turn/start` and emits `item/started`/`item/completed` notifications with `enteredReviewMode` and `exitedReviewMode` items, plus a final assistant `agentMessage` containing the review. | ||
| - `command/exec` — run a single command under the server sandbox without starting a thread/turn (handy for utilities and validation). | ||
| - `command/exec/resize` — resize a running PTY-backed `command/exec` session by `processId`; returns `{}`. |
There was a problem hiding this comment.
oh, probably worth documenting the rest of the API
| "sandboxPolicy": { "type": "workspaceWrite" }, // optional; defaults to user config | ||
| "timeoutMs": 10000 // optional; ms timeout; defaults to server timeout | ||
| "outputBytesCap": 1048576, // optional; per-stream capture cap | ||
| "disableOutputCap": false, // optional; cannot be combined with outputBytesCap |
There was a problem hiding this comment.
should we add docstrings / move these to v2.rs so they get included in the exported TS and JSON schema types?
|
@euroelessar btw I triggered a local codex review and it mentioned this:
seems worthwhile to address? |
2 participants
command/exec/terminate)command/exec/resize)