Allow actions to opt into Ninja's console pool for streamed output (22.1.2, 22.2.1) (#946)

docs/adrs/adr-0061-select-ninja-console-pool-per-build-edge.md

@@ -0,0 +1,82 @@
+# Architectural decision record (ADR) 0061: Select Ninja's console pool per build edge
+
+## Status
+
+Accepted. Actions and targets may set `console: true`; manifest lowering stores
+the backend-neutral request as `BuildEdge.output_visibility`, and the Ninja
+adapter maps `Immediate` to `pool = console` on the generated build edge. The
+option defaults to `false` and does not change `Action` identity.
+
+## Date
+
+2026-10-25.
+
+## Context and problem statement
+
+Ninja buffers a command's output until it exits unless the edge uses Ninja's
+special `console` pool. A manifest currently cannot select that pool. Long
+developer gates can therefore appear idle while they run, even when their
+commands are waiting on a lock or making progress.
+
+The IR shares actions when their resolved recipes and file sets match. Making
+console selection part of `Action` would make a scheduling choice affect action
+identity and could prevent otherwise identical edges from sharing an action.
+The pool is also an edge-level Ninja binding, so the graph edge is its natural
+owner.
+
+## Decision drivers
+
+- Preserve action hashing and rule sharing when only one edge needs streamed
+  output.
+- Keep scheduling validation common to direct generation and dyndep bundles.
+- Reject combinations that Ninja cannot represent without silently dropping a
+  requested guarantee.
+- Explain the serialization and output-forwarding limits to manifest authors.
+
+## Decision outcome
+
+- Add a default-false `console` Boolean to manifest actions and targets, and
+  lower it to `EdgeOutputVisibility::Immediate` or `Buffered` on the
+  `BuildEdge`.
+- Keep `EdgeOutputVisibility` backend-neutral. Manifest lowering owns the
+  conversion from the YAML option; each backend owns the mapping to its
+  scheduling and output mechanisms. The Ninja adapter alone knows the pool name
+  and edge-level clearing syntax.
+- Emit an edge-level `pool = console` binding only for selected user edges.
+  When an action rule declares `pool = console`, emit an empty edge-level
+  `pool =` binding on its ordinary edges so they do not inherit console
+  scheduling. Synthetic serial gates and dyndep sidecars do not inherit either
+  binding.
+- Keep `Action.pool` and action hashing unchanged. A pre-existing
+  `Action.pool: console` is accepted as redundant; a different custom pool
+  conflicts with console selection and is rejected by the generator.
+- Reject console selection on dependency-only aggregates. Reject console plus
+  a named contention class while continuing to allow console selection by
+  itself.
+- Validate these rules before both direct Ninja generation and dyndep-bundle
+  generation, preserving the existing missing-action diagnostic path.
+
+Adding fields to the public `Target` and `BuildEdge` structs breaks downstream
+Rust callers that construct either type with a struct literal. Both structs
+expose public fields and neither is `#[non_exhaustive]`, so callers must add
+`console` to `Target` and `output_visibility` to `BuildEdge`. The package
+currently declares version `0.1.0-beta4`, and this source break is accepted for
+the next pre-release API. The default only preserves manifest compatibility
+through Serde.
+
+Ninja runs console-pool edges one at a time in one invocation. Output from
+other edges is held while a console edge runs. Netsuke forwards Ninja output as
+it reads it outside JSON mode, but it does not allocate a PTY; recipes see a
+pipe and their own buffering can still delay visible output.
+
+## Consequences
+
+Manifest authors can opt long or interactive gates into prompt output without
+changing reusable rules or action identity. They also accept reduced edge
+concurrency while any console edge is running. The option cannot provide
+interactive terminal semantics, and it does not bypass buffering inside a
+recipe.
+
+The contention-class RFC must reject only the combination of console and a
+contention class, not console selection in general. Generator validation also
+guards against incompatible custom pools supplied through existing IR callers.

docs/contents.md

@@ -318,6 +318,9 @@ operator, user, and contributor references are easier to find.
   moving package verification into `make test-package-verify`.
 - [ADR-0060](adrs/adr-0060-readable-inline-recipe-progress.md):
   Bounded, localized Ninja progress labels for inline recipe edges.
+- [ADR-0061](adrs/adr-0061-select-ninja-console-pool-per-build-edge.md):
+  Select Ninja's console pool on individual build edges while preserving action
+  identity and rejecting incompatible scheduling annotations.
 - [ADR-0068](adrs/adr-0068-detect-cycles-across-all-prerequisite-kinds.md):
   Include every prerequisite kind in whole-manifest cycle detection while
   keeping output aliases and serial scheduling semantics explicit.

docs/developers-guide.md

@@ -6502,6 +6502,40 @@ variants, and `parallel` remains the default. The ordering policy applies only
 to a manifest `deps` list; never infer it from the number or shape of graph
 edges, and do not apply it to inputs or order-only dependencies.
 
+The manifest `console` Boolean lowers to `BuildEdge.output_visibility`, using
+`EdgeOutputVisibility::Immediate` for `true` and `Buffered` for `false`. This
+backend-neutral request belongs to the edge because two targets can share one
+action and rule while only one target needs streamed output. `Action.pool`
+remains unchanged, so console selection does not alter action identity or
+action deduplication. `src/ninja_gen/display_edge.rs` emits `pool = console` on
+that edge. If a shared action rule already declares `pool = console`, ordinary
+edges get an empty edge-level `pool =` binding to clear that inherited
+selection. Shared console-scheduling validation runs before both direct Ninja
+generation and dyndep-bundle generation. That validation rejects
+dependency-only edges and custom action pools that conflict with `console`; it
+accepts an existing `Action.pool` value of `console`. Synthetic serial gates
+and dyndep sidecars never inherit either binding.
+
+`ninja_gen::CONSOLE_POOL` is the single production spelling of Ninja's built-in
+pool. Keep its use within the edge renderer and shared console-scheduling
+validator; tests continue to assert the emitted `console` value directly.
+
+The emitted `pool = console` binding requires Ninja 1.5 or newer. This minimum
+is separate from the conditional Ninja 1.10 requirement for staged serial
+dependency bundles described below.
+
+`IrGenError::InvalidManifest` retains its stable English schema diagnostic when
+formatted. Manifest loading localizes that diagnostic at the parsing boundary;
+IR error formatting must not resolve the process-wide active locale.
+
+Manifest validation keeps stable diagnostic details separate from localized
+display text. `src/manifest/mod.rs` maps a recognized detail to the localized
+parse message. Direct IR lowering retains the stable detail in
+`IrGenError::InvalidManifest`, whose formatting does not consult global locale
+state. Ninja generator errors use an output-specific message when an edge is
+available; the build-script localization slice does not depend on either
+library-side path.
+
 `src/ir/cycle/detector.rs::CycleDetector::visit_known_edge` traverses `inputs`,
 `implicit_deps`, and `order_only_deps` when detecting cycles. Order-only
 dependencies do not trigger a rebuild when they change, but they still have to
@@ -6602,11 +6636,12 @@ or add manifest-controlled values to telemetry.
 
 The runner-internal `GraphGenerationContext` in
 `src/runner/graph/generation/mod.rs` groups the selected `RecipeShell`, an
-optional preflighted `BashExecutable`, and injected monotonic clock solely for
-this graph-generation composition path. Build and Ninja-tool operations attach
-the runtime binding after preflight; query-only generation leaves it absent. It
-is not a general runner context, shared state container, or reusable public
-API; keep unrelated runner inputs and concerns outside it.
+optional preflighted `BashExecutable`, and injected monotonic clock for the
+graph-generation composition path. The build runner also uses that clock to
+measure its Ninja invocation. Build and Ninja-tool operations attach the
+runtime binding after preflight; query-only generation leaves it absent. It is
+not a general runner context, shared state container, or reusable public API;
+keep unrelated runner inputs and concerns outside it.
 
 The intended serial guarantee is path-scoped. A later dependency that is
 independently reachable elsewhere in the requested graph may start via that
@@ -7310,6 +7345,29 @@ The production generator has its own `netsuke::ninja_gen::GeneratedNinja`
 output type; `test_support::ninja_semantics::GeneratedNinja` is the test-side
 borrowed view of generated text.
 
+### `test_support::ninja_edge`
+
+`test_support::ninja_edge::edge_has_console_pool`
+(`test_support/src/ninja_edge.rs`) checks edge-local pool metadata on a named
+generated `build` statement. Snapshot, BDD, and documentation-example tests use
+this shared helper, so their assertions agree about which edge owns an indented
+`pool = console` binding. Its scope is deliberately narrow: it inspects the
+generated text used by these tests and does not parse general Ninja syntax.
+Permitted call sites are test-support and test code only; production code must
+not depend on `test_support`.
+
+`test_support::ninja_edge::edge_pool_bindings` returns the named edge's exact
+`pool =` bindings. It distinguishes `pool = console` for an opted-in edge, an
+empty `pool =` binding that clears an inherited console pool, and no binding
+for an ordinary edge. The property test in
+`src/ninja_gen/property_tests/console.rs` uses it for user-edge, serial-parent
+and synthetic-gate assertions. Keep this helper within test-support and test
+code; it does not parse general Ninja syntax.
+
+The local `edge_factory` fixture in `src/ninja_gen/tests/console.rs` constructs
+parallel, single-output edges for console generator tests. Keep it within that
+module; other graph shapes use their own fixtures.
+
 ### Temporary Ninja build files
 
 `runner::process::create_temp_ninja_file` writes, flushes, and synchronizes a
@@ -10158,7 +10216,9 @@ success envelope; diagnostic serialization remains in
 generator metadata from the private `src/json_envelope.rs` module. Within
 process execution, `forward_stdout` is the single composition point for
 choosing status-aware or plain child-output draining, and its callers select
-either the terminal or a JSON-mode sink.
+either the terminal or a JSON-mode sink. The copy loop itself, and the
+forwarding-failure vocabulary around it, are owned by
+[`runner::process::streaming`](#module-runnerprocessstreaming).
 
 `ExecutionContext` is the private dispatch context shared by build and clean
 handlers. `run_with_ninja_program` constructs it after resolving output mode
@@ -10249,6 +10309,38 @@ no fixture sentinel text. The governing decision record is
 which forbids unbounded or caller-controlled values in metric labels and trace
 fields.
 
+### Module: `runner::console_telemetry`
+
+`src/runner/console_telemetry/mod.rs` instruments only the actual Ninja build
+invocation in `execute_build`. It supplements the complete legacy-recipe
+operation timing with subprocess duration and outcome grouped by
+`console_scheduling=enabled|disabled`. Its `runner.ninja.build` span and
+completion event include the aggregate `console_edge_count`, scheduling state,
+`outcome=success|error`, and `error_category=none|io`. Paths, action
+identities, recipe contents, and process error text never enter this boundary.
+The runner's injected monotonic clock supplies the duration measurement.
+
+The generator stores the count of selected user edges privately in
+`GeneratedNinja`; the runner reads that aggregate before consuming the bundle.
+Synthetic serial gates and dyndep sidecars do not contribute to it. The count
+describes configured edges in each attempted build graph, rather than edges
+that Ninja actually executed or time spent in its pool. The subprocess boundary
+emits `netsuke_runner_ninja_build_executions_total`,
+`netsuke_runner_ninja_build_execution_duration_seconds`, and
+`netsuke_runner_ninja_build_console_edges_total`. The first two use scheduling,
+outcome, and error-category labels; the edge counter uses only outcome.
+
+Bundle-generation telemetry records the same scheduling state and aggregate
+count. `netsuke_ninja_dyndep_bundle_generations_total` and
+`netsuke_ninja_dyndep_bundle_generation_duration_seconds` carry scheduling,
+outcome, and the generator's fixed error category, including
+`console_dependency_only` and `console_pool_conflict`.
+`netsuke_ninja_dyndep_bundle_console_edges_total` counts configured console
+edges by generation outcome. `src/observability/recorder/console.rs` owns the
+production retention policy for these six series: it admits their exact kinds
+and finite label sets, rejecting unknown values, extra labels, and missing
+labels. Both emission and retention tests use local recorders.
+
 ### Module: `runner::recipe_shell::telemetry`
 
 `src/runner/recipe_shell/telemetry/mod.rs` owns bounded observability for shell
@@ -10365,6 +10457,58 @@ measuring the parent test process. It compares progress parsing with
 `--progress never` and permits a fixed 16 MiB overhead, rejecting memory growth
 proportional to the payload.
 
+### Module: `runner::process::streaming`
+
+`src/runner/process/streaming/` owns the only byte-for-byte copy loop between a
+child's standard streams and a forwarding writer, the bounded Ninja
+status-parsing adapter, and the closed forwarding-failure vocabulary in
+`forward_failure.rs`. No other module may add a second copy loop.
+
+#### Streaming reuse boundary
+
+- **Ownership:** the streaming module owns chunk reads, per-chunk writes and
+  flushes, interrupted-read retries, draining after a failure, the
+  `ForwardStats` record, and the failure classification that telemetry
+  publishes.
+- **Permitted call-sites:** exactly two. `output_forwarding::forward_stdout`
+  forwards stdout, choosing `forward_child_output_with_ninja_status` when
+  progress updates are wanted and `forward_child_output` otherwise.
+  `failure_attribution::forward_stderr_with_attribution` forwards stderr
+  through its marker-observing writer. Both pass the stream identity as
+  `ForwardStream`; no caller may invent a stream label.
+- **Composition rules:** flush each chunk before reading the next one, so a
+  flush failure cannot cause already-accepted bytes to be written again. Retry
+  `io::ErrorKind::Interrupted` reads rather than reporting them. Treat a reader
+  that reports more bytes than the buffer holds as a read failure, because the
+  count is only meaningful as a read result. After any failure, drain the
+  remaining child output so the child cannot block on a full pipe.
+
+#### Failure classification and telemetry
+
+A forwarding failure is classified as one of `read`, `write`, `flush`, or
+`drain`, and attributed to `stdout` or `stderr`. A drain failure after an
+earlier failure is counted separately and never replaces the failure that
+stopped forwarding. The counter `netsuke_ninja_forward_failures_total` carries
+exactly the two labels `stream` and `failure_kind`, whose permitted values are
+the closed arrays `FORWARD_STREAM_VALUES` and `FORWARD_FAILURE_KIND_VALUES`.
+`ConfigMetricsRecorder` admits only those exact series, so a widened label
+value is dropped rather than retained. Each failure also emits a warning event
+carrying the same two fields plus the I/O error. Neither label admits output
+content, paths, or reader and writer types. Adding a failure kind means adding
+an enum variant and its array entry together, never passing a raw string.
+
+#### Result policy
+
+Forwarding loss never changes the reported build result. The child's exit
+status remains authoritative, and a truncated output stream is reported through
+the classified counter and warning event instead. This keeps a deliberately
+closed parent pipe, as produced by `netsuke build | head`, from being reported
+as a build failure. `finalize_streaming` therefore joins the stderr forwarding
+thread for its command-list attribution and discards the forwarding statistics
+the helper has already published. The policy tests in
+`src/runner/process/tests/mod.rs` cover a successful child, a failing child,
+and a failed wait, each with forwarding loss.
+
 ### Module: `runner::process::ninja::program`
 
 `src/runner/process/ninja/program.rs` owns the executable-resolution boundary.