Repository navigation
feat(protocol): Add shared output streams - #15
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🔵 Needs a closer look
The coordinated protocol transition and post-fork descriptor changes warrant human sign-off after prerequisite #14 merges.
No open issues 🎉
What changed in this PR
Adds protocol 2 shared output streams so structured requests can produce a combined stdout/stderr log without a wrapper shell. This PR depends on #14’s shutdown repair.
Changes:
- Adds stderr-only
Merge_Stdout, sharing stdout’s descriptor and file offset. - Updates protocol fixtures and adds shared-stream regression tests.
- Aligns documentation and Pages with the fixed protocol-2 contract.
| File | Description |
|---|---|
| WORK_QUEUE.md | Updates shared-output delivery and follow-up status. |
| tools/spawn_manager.adb | Generalizes a protocol comment. |
| tools/spawn_manager_processes.ads | Adds the merged-stream launch policy. |
| tools/spawn_manager_processes.adb | Maps and validates merged streams. |
| tests/spawn-transport-tests.adb | Updates the transport fixture to V2. |
| tests/spawn-protocol-tests.ads | Declares merged-stream tests. |
| tests/spawn-protocol-tests.adb | Adds golden bytes and rejection checks. |
| tests/spawn-posix-tests.c | Tests shared offsets, truncation and descriptors. |
| tests/spawn-pool-tests.ads | Declares shared-stream integration tests. |
| tests/spawn-pool-tests.adb | Tests logs, failures, timeouts and isolation. |
| tests/spawn_manager_tests.adb | Updates version acceptance and rejection fixtures. |
| tests/protocol-failure-manager.c | Updates independent result frames to V2. |
| src/spawn-protocol.ads | Defines V2 and Merge_Stdout. |
| src/spawn-protocol.adb | Decodes and validates merged-stream requests. |
| src/spawn-posix.h | Adds the C merged-stream policy. |
| src/spawn-posix.c | Duplicates stdout for merged stderr. |
| src/spawn-pool.adb | Generalizes failure-classification comments. |
| SECURITY.md | Extends documented guarantees to V2. |
| README.md | Describes shared output and coordinated delivery. |
| Makefile | Runs signal-reaping checks with V2. |
| doc/protocol-v2.md | Defines the new wire and stream contract. |
| doc/protocol-v1.md | Marks V1 historical and pins its source link. |
| doc/Makefile | Builds and links both protocol documents. |
| doc/lifecycle-v1.md | Clarifies lifecycle applicability across wire versions. |
| CONTRIBUTING.md | Generalizes fixed-version guidance. |
| CHANGELOG.md | Records protocol 2 under Unreleased. |
| AGENTS.md | Points development guidance to V2. |
| .github/workflows/pages.yml | Checks generated V2 documentation. |
| .github/ci/README.md | Updates documentation-output descriptions. |
Review effort: Balanced
mweidle73
force-pushed
the
codex/exec-v2
branch
3 times, most recently
from
October 18, 2026 08:38
d4aee47 to
0f1f456
Compare
Introduce fixed protocol 2 with an explicit stderr alias of stdout. Open the target once and duplicate fd 1 without changing process ownership, privilege restrictions or the shell request payload. Pin merged frames independently and exercise descriptor offsets, ordered bytes, truncation, timeout and concurrent request isolation. A deliberate second-open mutation fails the direct-core oracle. Update current and historical protocol documentation and Pages gates. The Ada and direct C suites, coverage and documentation pass.
Describe null, truncate and stderr-to-stdout mapping explicitly. Remove the stale two-mode wording without changing execution.
Check null-output descriptor identities in direct and manager tests. Make the shared-log permission fixture independent of test order. Both null-output oracles reject a deliberately skipped stderr dup. Align current wire and lifecycle guidance, preserve protocol page titles and clarify unchanged security requirements for both versions. Keep historical protocol documentation explicit rather than obsolete. Refresh the completed organization-transfer follow-up and rebalance documentation paragraphs without changing execution behavior.
mweidle73
force-pushed
the
codex/exec-v2
branch
from
October 18, 2026 09:02
0f1f456 to
e832faf
Compare
mweidle73
commented
Oct 18, 2026
Explain why the version-1 contract pins its normative source to the 0.2.0 release instead of using a checkout-relative link. The Pages build maps relative source links to current master rather than preserving the historical V1 implementation.
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.
Purpose
Support a combined stdout/stderr build log without a wrapper shell or two
independent opens of the same file.
The shutdown repair in #14 is merged into canonical
master. This PR nowtargets
master; the reviewed V2 changes are retained, with a shortdocumentation clarification explaining the historical source link.
Changes
Fixed protocol 2 adds stderr-only
Merge_Stdout. The executor opens stdoutonce and duplicates its descriptor for stderr, preserving shared offsets and
write order. Independent streams remain explicit. Invalid stdout aliases and
protocol-version mismatches fail closed without negotiation or fallback.
Verification
Rootless Debian Trixie builds,
make tests, library-Ada coverage andmake docpass. Independent golden frames, shared-offset/order/truncation,null-sink descriptor checks, failure, timeout and parallel cases are covered.
Second-open and skipped-stderr-dup mutations fail their oracles. The first V2
commit also passes a standalone build and tests with version-2 signal checks.
Coverage does not claim to measure the manager Ada body or C core.
Compatibility and documentation
Library and manager must be delivered together with protocol 2. Shell request
semantics, exact argv/environment, relative sockets, privilege restrictions
and supervision are retained from the prerequisite. Protocol, lifecycle,
contributor/security guidance, Pages and the Unreleased changelog are aligned.
No release tag or native performance claim is included.
Co-Authored-By: GPT-5.6 noreply@openai.com
Co-Authored-By: GPT-6.1 noreply@openai.com