OXY-162: New Oxygen command-execution API (CommandService / Command2) - #304
Open
Kalin-Rudnicki wants to merge 7 commits into
Open
OXY-162: New Oxygen command-execution API (CommandService / Command2)#304Kalin-Rudnicki wants to merge 7 commits into
Kalin-Rudnicki wants to merge 7 commits into
Conversation
…le state Brings the exploratory command-execution API from the `current/feature/command-improvements` brainstorm branch to a compiling, tested state. - Fix a blocking bug: `CommandInputSource.Empty` mapped to `Redirect.DISCARD` (a WRITE-only redirect), which made the process fail to start for every no-stdin execution. Use `PIPE` and close the child's stdin immediately so it observes EOF. - Make oxygen-zio compile on JS and Native again: the shared `JavaCommandService` used the JVM-only `ZSink.fromOutputStream` and `java.lang.ProcessBuilder`. Rewrote the stream-stdin path to a portable blocking chunk-write (compiles on Native), and excluded the file from the JS source set (JS keeps `UnimplementedCommandService`). - Resolve WIP loose ends: correct `BuiltCommand` shell-escaping to the POSIX `'\''` idiom (display-only), document `Command2` vs legacy `Command` coexistence, tidy comments. - Add `Command2Spec` covering builder, escaping, and live JVM execution. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011YxWKdsz97QT9BD7AdpSq6
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.
Derives from the
current/feature/command-improvementsbrainstorm branch (a chain of"WIP : savepoint" commits) and brings that exploratory work to a compiling, tested state.
Jira: OXY-162
What the new command API is
A "v2" API in
oxygen-ziofor running external OS processes from ZIO, intended to eventuallyreplace the legacy
oxygen.zio.system.Command(which shells out viascala.sys.process):CommandService— pluggable execution SPI (executeSync,executeSyncStreamErr,executeCode+ derived helpers), held in aFiberRefwith a platform-specific default.Command2— immutable fluent builder (command / args / cwd / env / sudo) building aBuiltCommand; execute surface for sync capture, streamed stderr, exit-code, and typeddecode-to-
A(String / PlainText schema / JSON), plus legacy-compatibleexecute*shims.JavaCommandService— JVM/Native impl overjava.lang.ProcessBuilder: scoped processlifecycle, concurrent stdout/stderr draining (avoids pipe-buffer deadlock), stdin sources.
UnimplementedCommandService— Scala.js default (typedUnimplementedfailure).CommandInputSource/CommandOutputSource/ShowCommand) and aCommandErrorADT.StringDecoder.DecodingFailureandjson.JsonErrornow extend oxygenError.What this PR did to finish it
CommandInputSource.Emptymapped toRedirect.DISCARD(a WRITE-onlyredirect), so the process failed to start for every no-stdin execution — i.e. the API failed on
first use. Now uses
PIPE+ closes the child's stdin (EOF).JavaCommandServiceusedZSink.fromOutputStream(JVM-only) and
ProcessBuilder(absent on JS). Rewrote the stream-stdin path to a portableblocking chunk-write (Native compiles it), and excluded the file from the JS source set.
BuiltCommandshell-escaping to the POSIX'\''idiom(display/logging only — execution never goes through a shell), documented the
Command2vs legacyCommandcoexistence, cleaned upFIX-PRE-MERGEnotes.Command2Spec(13 tests) — builder, escaping, and live JVM execution.Verification
oxygen-ziocompiles on JVM / JS / Native (JS and Native were broken before).sbt utJVM/test→ 743 passed, 0 failed (newCommand2Spec+ legacyCommandSpec+ allcore/schema/json/zio specs; confirms the
Error-trait migration caused no regressions).sbt oxygen-executableJVM/Test/compilesucceeds (downstream consumer of the changed core/json APIs).sbt fmtapplied.Honest mergeability assessment
Reviewable and non-broken, but an incremental landing of an in-progress API (~6.5/10). The JVM
happy paths are solid, test-covered, and green, and the previously API-breaking
Empty-stdin bug isfixed. Caveats:
Command2is still parallel to the legacyCommand(no call-site migration yet);the JS backend is intentionally unimplemented; Native compiles but is not exercised by any
test; and several stdio/decode branches (Stream/File/Log, streamed-stderr, JSON/PlainText decode)
are only lightly or not covered. Safe to merge behind the existing unused
Command2name; not afinished, fully-migrated replacement for
Command.🤖 Generated with Claude Code
https://claude.ai/code/session_011YxWKdsz97QT9BD7AdpSq6