raphaelvigee/exec runners host exec - #422
Closed
raphaelvigee wants to merge 2 commits into
Closed
Conversation
…erve `spawn` hands back a `proc_exec::Handle`, which owns a real child of *this* process. A driver behind the plugin seam cannot manufacture one from bytes arriving over an ABI, so any driver written against `spawn` can only ever run in-process — which is the root of why a plugin driver cannot use a container or a session runner today. `stream` is the third shape: chunks on a channel as they are produced, the exit status as the return value. Same tagged-channel shape as `Handle::take_output`, so a consumer that already tees an `OutputReader` changes almost nothing — and both streams share one channel for the same reason they do there, to remove head-of-line blocking between them on macOS. It also retires a hazard rather than restating it. With `spawn`, the wait must not share a task with the reader or a child that fills a pipe wedges forever; here the status is the future's own return value, so there is no second task to get wrong. `tokio` moves from a dev-dependency to a dependency for `mpsc::Sender` in the signature — `sync` feature only. Nothing here starts a task; `hproc` still owns every spawn. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RC5ykWRMPjRKGk1Tz1KTja
A driver in a cdylib cannot be handed the session its target must run in — a session is a live object and the plugin links its own copy of every type. So it was sent an approximation instead: the session's environment variables, from which it rebuilt a stand-in. That is exact for `Local` and `Env`, and wrong for every session that also rewrites the command. Under `mode = "session"` a plugin driver ran its target on the host with the shell's variables applied and never entered the shell, while still echoing `runner_key` back — walking past the very ack that exists to catch a degraded run. The previous commit made that a loud refusal. This removes the reason for one. The driver no longer creates the process. It hands the spec to the host through `StableExecService`, and the host — which holds the real `ExecSession` — applies it in full and returns the output. Nothing about the session crosses, and every runner mode now works for every driver: `go_*` targets build inside an `oci_runner` container like anything else. `DynExecService` rides `run` exactly as a `DynExecutor` rides a provider's `list`; that precedent is why it is a parameter rather than a global. The body is spawned on the host runtime via `SeamSpawn`, because this future is polled by a *guest* worker and creating a process touches the host reactor — doing that with no host runtime context panics, and a panic at the extern seam aborts. Deleted rather than fixed: `ExecSession::flattens_to_env`, `runner_opaque` (now `reserved 14`), the refusal branch and both its tests, and the `EnvSession` flattening in `guest_session`. Two scope calls, both documented at the code: - **No runner selected → `LocalSession` in the guest**, unchanged. The common path stays in-process with no round trip and keeps live streaming. - **Batch only across the seam.** Nine of the ten plugin-side call sites are batch already; the tenth (`docker_build` showing buildx progress) loses liveness only when it is itself under a runner. A streaming lane can come when something needs it. The end-to-end test earned its place immediately: it failed on the first run with empty stdout, because `ExecSpecPatch` deliberately carries no stdio and the host rebuilt the spec with defaults. The runner lane hides that by re-applying the host's own stdio; this lane has none to re-apply and must be told the kind (`stdio_kind_from_pb`). A unit test on argv would have passed while the feature was broken. ABI-BREAK-ACK: 0.6.0 is unreleased, so this is redefined in place. Workspace suite 2782 passed, 0 failed. `lint` clean, `abi-check` clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RC5ykWRMPjRKGk1Tz1KTja
Member
Author
|
Folded into #418, where the behaviour it replaced was introduced — the stack is unmerged, so the refusal it deleted should simply never exist rather than be added and removed across two PRs. The |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack created with GitHub Stacks CLI • Give Feedback 💬