SubprocessModule: bridge import Subprocess to Shell.processLauncher#5
Merged
SubprocessModule: bridge import Subprocess to Shell.processLauncher#5
import Subprocess to Shell.processLauncher#5Conversation
Closes the SwiftScript half of the polyglot subprocess unification. ShellKit shipped `Shell.processLauncher` (Cocoanetics/ShellKit#1) and SwiftBash registered `BashProcessLauncher` (Cocoanetics/SwiftBash#19); this bridge lets a SwiftScript script write idiomatic swift-subprocess code and have it route through whatever launcher the bound shell carries. ```swift import Subprocess let r = try await Subprocess.run( Executable.name("git"), arguments: ["status"], output: Output.string(limit: 4096)) if r.terminationStatus.isSuccess { print(r.standardOutput ?? "") } ``` Routing: - **Standalone** (no embedder) → `DefaultProcessLauncher` → swift-subprocess `run(...)` → real `posix_spawn` (macOS / Linux / Windows / Android). - **Under SwiftBash** → `BashProcessLauncher` → resolves against the bash command registry. No `posix_spawn`, ever — pure-Swift builtins and registered SwiftPorts CLIs. - **Sandbox-bound** without a virtualised launcher → `ProcessLaunchDenied`, surfaced to the script as a thrown error. What v1 ships: * `Executable.name(_:)` / `.path(_:)` * `Output.string(limit:)` / `Output.discarded` and the symmetric `ErrorOutput.string(limit:)` / `ErrorOutput.discarded`. Modeled as separate type names because Swift's `Error` protocol identifier collides with the swift-subprocess `Error` generic parameter when used script-side. * `Subprocess.run(_:arguments:output:)` and `Subprocess.run(_:arguments:output:error:)` overloads, dispatched by argument count behind a single empty-paren bridge key (matches the existing convention for static methods like `Double.minimum`). * `ExecutionRecord.{terminationStatus, standardOutput, standardError, processIdentifier}`. `standardOutput` / `standardError` are Optional<String> — `nil` for `.discarded`, the captured UTF-8 text for `.string(limit:)`. * `TerminationStatus.{isSuccess, exitedCode, signaledCode}`. The case-extracting accessors stand in for pattern-matching the underlying enum (`if case .exited(let code) = ...`), which SwiftScript doesn't model for opaque enum values. What v1 defers (script-author-visible items the bridge could grow later, ordered by likely demand): `environment:` / `workingDirectory:` / `input:` parameters on `run`, closure-form `run(...) { execution, stdout in ... }` overloads, streaming `AsyncSequence<Data>` outputs, `PlatformOptions`. For v1 the environment + cwd come from `ShellKit.Shell.current` and stdin is empty — covers the 95%-case shell-out pattern. Implementation note: the bridge body wraps the `Output.string(limit:)` adapter in a buffering `OutputSink` that the launcher streams into, then folds the captured bytes back into the returned `ExecutionRecord`. Honors the `.discarded` contract by dropping launcher-supplied bytes even when `DefaultProcessLauncher` populated its own record. Tests: `SubprocessBridgeTests` covers dispatch routing through a recording launcher (executable name/path passthrough, argv shape, exit-status mapping, stdout/stderr capture, sandbox-deny error surfacing) plus one platform-gated test (`#if os(macOS) || os(Linux)`) that goes end-to-end through `DefaultProcessLauncher` → real swift-subprocess. Suite: 10 tests, all pass; full SwiftScript suite 446/446 still green. Package.resolved bumps ShellKit pin to the `Shell.processLauncher`-bearing commit. SwiftScript itself does NOT add swift-subprocess as a direct dep — ShellKit owns that relationship; this module uses ShellKit's mirroring types (`Executable`, `Arguments`, `Environment`, `ExecutionRecord`, `TerminationStatus`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af2f06cc21
ℹ️ 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".
… on overflow Two issues from the automated review on #5: 1. **P1 — Preserve empty captured output instead of returning `nil`.** `Subprocess.run(..., output: .string(limit:))` with zero captured bytes was mapping `ExecutionRecord.standardOutput` to `nil` because of a naive `isEmpty` check in the accessor. That conflated two different states: `.discarded` (script asked for nothing → `nil`) versus `.string(limit:)` with an empty capture (script asked for capture, command was silent → `Optional("")`). A script could not tell a silent command apart from a discarded stream. The same pattern applied to `standardError`. Fix: the bridge now wraps the launcher's `ExecutionRecord` in a private `ScriptExecutionRecord` that carries per-stream "captured" flags (`outputCaptured` / `errorCaptured`). `.string(limit:)` sets the flag; `.discarded` doesn't. The property accessors return `nil` only when the flag is false, `Optional(String(...))` otherwise — even for an empty buffer. 2. **P1 — Throw on output-limit overflow instead of silently truncating.** `Output.string(limit:)` is documented in swift-subprocess to throw if emitted bytes exceed the limit. The previous bridge used `d.prefix(remaining)` and continued silently. A script parsing JSON or a status payload would get incomplete data with no error signal. Fix: `BufferBox` now records an `overflowed` flag when an incoming chunk would push past the limit. The run body inspects the captured state after `launch(...)` returns and throws a new `SubprocessOutputLimitExceeded` error (carrying the offending stream + the configured limit) when either stream overflowed. The threshold check also covers the fall-through case where the launcher populated `record.standardOutput` directly without going through the bridge sink. `OutputSink.onWrite` is non-throwing (`(Data) -> Void`), so the throw has to land in the run body rather than the sink closure — overflow is recorded in-band and surfaced at the first opportunity. Tests: - `outputStringPreservesEmptyCaptureAsEmptyString` — silent command + `.string(limit: 4096)` → `Optional("")`. - `outputStringThrowsOnLimitOverflow` — 50-byte stub against a 10-byte limit throws; error message names the stream and the limit. - `errorStringThrowsOnLimitOverflow` — symmetric coverage on the stderr side. Suite: 13/13 (was 10). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Closes the SwiftScript half of the polyglot subprocess unification. ShellKit shipped
Shell.processLauncher(Cocoanetics/ShellKit#1) and SwiftBash registeredBashProcessLauncher(Cocoanetics/SwiftBash#19); this PR lets a SwiftScript script write idiomatic swift-subprocess code and have it route through whatever launcher the bound shell carries.Routing
DefaultProcessLauncher→ swift-subprocessrun(...)→ realposix_spawn(macOS / Linux / Windows / Android).BashProcessLauncher→ resolves against the bash command registry. Noposix_spawn, ever — pure-Swift builtins and registered SwiftPorts CLIs.ProcessLaunchDenied, surfaced to the script as a thrown error.What v1 ships
Executable.name(_:)/.path(_:)Output.string(limit:)/Output.discardedErrorOutput.string(limit:)/ErrorOutput.discardedSubprocess.run(_:arguments:output:)and(_:arguments:output:error:)Double.minimum)ExecutionRecord.{terminationStatus, standardOutput, standardError, processIdentifier}TerminationStatus.{isSuccess, exitedCode, signaledCode}OutputandErrorOutputare separate type names because Swift'sErrorprotocol identifier would collide with the swift-subprocessErrorgeneric parameter when used script-side.What v1 defers
environment:/workingDirectory:/input:parameters onrun, closure-formrun(...) { execution, stdout in ... }overloads, streamingAsyncSequence<Data>outputs,PlatformOptions. For v1 the environment + cwd come fromShellKit.Shell.currentand stdin is empty — covers the 95%-case shell-out pattern.Tests
SubprocessBridgeTestscovers dispatch routing through a recording launcher:runRoutesThroughBoundProcessLauncher— argv passes through correctlyexecutablePathPassesThroughVerbatim—Executable.path(...)preservedoutputStringCapturesStdoutToStandardOutput—.string(limit:)captures bytesoutputDiscardedReturnsNil—.discardedhonors the contract even when launcher provides byteserrorStringCapturesStderr— stderr captureterminationStatusIsSuccessForExitZero/ExitedCodeForNonZero/SignaledCodeForSignaledsandboxedDenyLauncherSurfacesAsThrownError—ProcessLaunchDeniedpropagates to the scriptPlus one platform-gated end-to-end test (
#if os(macOS) || os(Linux)):standalonePathDelegatesToRealExec—Subprocess.run(.path(\"/bin/echo\"), arguments: [\"hi\"], output: .string(limit: 4096))round-trips throughDefaultProcessLauncher→ real swift-subprocess and captures\"hi\\n\".Suite: 10/10 pass. Full SwiftScript suite: 446/446 still green.
Notes
Package.resolvedbumps ShellKit pin to commit4395986(the one that landedShell.processLauncher).Executable,Arguments,Environment,ExecutionRecord,TerminationStatus).()parens (\"static func Subprocess.run()\") — matches the existing convention for static methods (Double.minimum,Bool.random). Caller-supplied labels are stripped before the body sees the args; positional order is the contract.Test plan
🤖 Generated with Claude Code