split distribution builds + task output - #9
Conversation
|
@codex review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8019a42b89
ℹ️ 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".
| print_error(&args, &error); | ||
| std::process::exit(exit_code(&error)); | ||
| } | ||
| let exit = match ProcessSignalGuard::install() { |
There was a problem hiding this comment.
Keep cancellation handlers scoped to run commands
Installing ProcessSignalGuard before dispatch also replaces SIGINT/SIGTERM/SIGHUP for check, ps, down, and clean, but those control paths never create or inspect a CancellationToken. Consequently Ctrl-C or an orchestrator's SIGTERM merely sets PROCESS_SIGNAL_CANCELED and these commands continue running instead of terminating; install the cancellation handlers only for run, or make every dispatched command consume the signal state.
Useful? React with 👍 / 👎.
| // Replay writes must observe EPIPE as a typed projection issue. The | ||
| // default disposition would terminate the runtime before the worker can | ||
| // report the broken pipe. The prior disposition is restored by Drop. | ||
| match install_ignore_handler(libc::SIGPIPE) { |
There was a problem hiding this comment.
Reset SIGPIPE before spawning declared children
Ignoring SIGPIPE is process-wide, and an ignored disposition survives exec, so every task and service child spawned while this guard is active inherits SIG_IGN. A declared executable that does not reset SIGPIPE will receive EPIPE rather than the normal signal termination when one of its own pipes closes, potentially changing its exit status or behavior; reset SIGPIPE to SIG_DFL in child pre_exec hooks while retaining the runtime's ignored disposition.
AGENTS.md reference: AGENTS.md:L61-L63
Useful? React with 👍 / 👎.
| match print_error(&args, &error) { | ||
| Ok(()) => exit, | ||
| Err(projection_error) => exit_code(&projection_error), |
There was a problem hiding this comment.
Preserve safety failures when error output breaks
When a containment, registry, lease, or state failure has already been selected and writing its final diagnostic to stderr fails—for example because the sink is closed—print_error returns OUTPUT_PROJECTION_FAILED and this branch replaces the original safety failure's exit status with 38. That reverses the documented failure precedence and leaves automation unable to distinguish a process escape or corrupt registry from an output problem; combine the two errors using the same priority rules as run finalization and retain the higher-priority exit code.
AGENTS.md reference: AGENTS.md:L87-L90
Useful? React with 👍 / 👎.
| "--both" => both = true, | ||
| "--json" => json = true, |
There was a problem hiding this comment.
Honor the final metadata output flag for errors
Metadata output flags are accepted together and parse_run_options uses the last one, but this error-only scanner remembers every earlier --json/--both occurrence and ignores --summary. Thus --both --json emits JSON-only on success but human plus JSON on failure, while --json --summary still emits a JSON error instead of the selected human projection; parse the flags sequentially into one mode so success and failure use the same final selection.
AGENTS.md reference: AGENTS.md:L87-L90
Useful? React with 👍 / 👎.
No description provided.