diff --git a/Cargo.toml b/Cargo.toml index a20460bf14f..556d5c902a6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,4 +50,13 @@ proptest = "1.4" sha2 = "0.10" hex = "0.4" glob = "0.3" -ureq = "2" \ No newline at end of file +ureq = "2" + +# Strip debug info from both profiles — this repo produces dev tools, +# not debuggable production binaries. Cuts artifact size ~60% and +# avoids the linker OOM / SIGBUS failures we saw on CI runners. +[profile.dev] +debug = 0 + +[profile.release] +debug = 0 \ No newline at end of file diff --git a/TODO/TODO_URGENT_logging_consolidation.md b/TODO/TODO_URGENT_logging_consolidation.md new file mode 100644 index 00000000000..3a55ec08379 --- /dev/null +++ b/TODO/TODO_URGENT_logging_consolidation.md @@ -0,0 +1,309 @@ +# URGENT: Consolidate DAG-Level Logging & Error Reporting + +**Status**: Active +**Date**: 2026-02-13 +**Priority**: High + +## Motivating Incident: CI Log Explosion (2026-02-13) + +Two consecutive CI runs on PR #39 produced logs so massive they were +impractical to paste or read. Root-cause analysis follows. + +### What happened + +1. The CI runner ran out of disk space (`Free space left: 41 MB`), which + caused the `rust-lld` linker to crash with `signal 7 [Bus error]` while + linking `gunbc-deps`. This is an infra issue, not a code bug. + +2. The build failure triggered the CI DAG's report node. The report dumped + **the entire linker command line** (~4,000 chars of `.rlib` paths) into + the `--- Build stderr ---` section of the report. This made the report + itself unreadable. + +3. Independently, the `execute_testgen` CI group contained the **full stdout + of the testgen binary**. The testgen binary runs its own DAG via + `execute_and_display` in classic mode, which prints every node's outputs. + For 23 DAG targets with compare/execute nodes, this produced hundreds of + lines of `[compare_*_content] skip: true / fresh: true` noise. The full + generated test source code was also visible. + +4. The `parse_build` CI group then **re-printed the entire build stderr** + (including the linker command) via `print_log_entry`, which had no + truncation at all. So the linker command appeared **three times**: once in + the `execute_build` group, once in `parse_build`, and once in the report. + +### Why it was so verbose + +Three layers of the output system contributed, each independently too verbose: + +| Layer | What it dumped | Why | +|-------|---------------|-----| +| `print_log_entry` (execute.rs) | Full stderr/stdout for every node in CI groups | No truncation, no line limit | +| `print_value` (display.rs) | Full stderr/stdout in classic mode | No truncation, no secret redaction | +| `build_report_blocks` (ci/ops.rs) | Full build stderr in final report | No truncation (fixed 2026-02-13 via `truncate_for_report`) | + +### What was fixed + +| Fix | Date | Scope | +|-----|------|-------| +| `truncate_for_report` in CI report | 2026-02-13 | Report sections capped at 60 lines, 500 chars/line | +| Unified `print_log_entry` → `print_value` | 2026-02-13 | Eliminated duplicate function; both CI and classic use same path | +| `truncate_log_value` for CI group output | 2026-02-13 | Multi-line values in CI groups capped at 40 lines (5 head + 35 tail) | +| Secret redaction in `print_value` | 2026-02-13 | `print_value` now redacts `Value::Secret` (was missing before) | + +### What remains unfixed + +- Testgen binary runs `execute_and_display` internally; its stdout is + captured and re-printed by the CI DAG → double-printing of all node outputs +- No per-stage error extractors (like `extract_test_failures` for test output) +- Stderr not captured from all stages (testgen, bootstrap, pragma only + capture stderr, not stdout) +- No single, configurable verbosity control across CI/local/progress modes + +--- + +## Problem + +Output logging, error reporting, and progress tracking are fragmented across +four different execution contexts with inconsistent behavior. Users see +different output depending on whether they're in CI, a TTY, a non-TTY +terminal, or verify mode. Stderr from some stages is silently discarded. + +--- + +## 1. Four Execution Contexts, Four Output Paths + +**Where**: `core/exec/src/execute.rs`, `core/exec/src/display.rs`, all `gunbc-dag/src/bin/*.rs` + +**What happens**: There are four separate code paths that handle DAG output: + +| Context | Entry point | What gets printed | +|---------|-------------|-------------------| +| CI with groups | `execute_with_mode_and_ci` | Every node's outputs via `print_log_entry`, inside CI groups | +| Local progress | `execute_and_display` (TTY) | Only boundary node outputs via `print_boundary_outputs` | +| Local classic | `execute_and_display` (no TTY) | Every node's outputs via `print_value` | +| Verify mode | `execute_with_mode_and_inputs` (direct) | Caller decides (varies per binary) | + +**Why this is a problem**: +- CI dumps everything (too verbose), local progress shows almost nothing (too terse) +- Debugging failures locally in progress mode is hard because you can't see intermediate node outputs +- Each binary reimplements its own error handling for the verify path + +**Suggested fix**: Unify into a single `ExecutionDisplay` trait with configurable verbosity. +Concrete levels: `Quiet` (exit code only), `Summary` (final report + failures), +`Verbose` (all node outputs), `CiGrouped` (verbose but collapsible). All binaries +should go through the same display path. + +--- + +## 2. `print_value` vs `print_log_entry` — Duplicate Logic with Drift + +**Status**: Fixed (2026-02-13) + +`print_log_entry` now delegates to `print_value`. Both CI group logging and +classic display use the same code path with truncation and secret redaction. + +The deeper issue (secret redaction depending on each call site rather than +the type system) is tracked in §8 below. + +--- + +## 3. Inconsistent Stderr Capture Across CI Stages + +**Where**: `gunbc-dag/src/ci/ops.rs` — parse operations for each stage + +**What happens**: Some stages capture both stdout and stderr, others only stderr: + +| Stage | stdout | stderr | Notes | +|-------|--------|--------|-------| +| Build | stdout + stderr | stderr | `build_stdout` exists but report ignores it | +| Test | stdout + stderr | stderr | `extract_test_failures` uses stdout | +| Lint | stdout + stderr | stderr | `lint_stdout` exists but report ignores it | +| Testgen | **missing** | stderr | stdout silently discarded | +| Bootstrap | **missing** | stderr | stdout silently discarded | +| Pragma | **missing** | stderr | stdout silently discarded | +| Guardrail | **missing** | stderr | stdout silently discarded | +| Verify | **missing** | stderr | stdout silently discarded | + +**Why this is a problem**: If testgen/bootstrap/pragma/guardrail/verify write +diagnostic info to stdout (e.g., test names, progress, warnings), it's lost. +When debugging CI failures, users can't see what these stages actually printed. + +**Suggested fix**: All parse operations should capture both stdout and stderr. +The report node should have a unified `format_stage_output(stage, stdout, stderr)` +that consistently presents output for any failing stage. This also makes it +possible to apply `extract_test_failures`-style summarization to any stage. + +--- + +## 4. `ci.rs` Binary Has Completely Separate CI vs Local Paths + +**Where**: `gunbc-dag/src/bin/ci.rs:160-210` + +**What happens**: The CI binary detects whether it's running in CI and takes an +entirely different execution path: + +``` +if is_ci { + execute_with_mode_and_ci(...) // manual log iteration, manual exit code +} else { + execute_and_display(...) // uses display infrastructure +} +``` + +**Why this is a problem**: +- The CI path manually iterates log entries looking for `overall_success` — this + logic is duplicated from what `execute_and_display` already does via its + `success_port` parameter +- Changes to display/reporting only apply to one path +- The `report` node is special-cased to not get a CI group (`node_id.0 != "report"`) + but `print_log_entry` is still called, so report output appears both as a raw + log entry AND as the report — potential duplication + +**Suggested fix**: Have a single execution path that accepts a `DisplayConfig` +struct. CI detection should only affect the display config (grouping, verbosity), +not the execution path. + +--- + +## 5. Preflight Output Bypasses Everything + +**Where**: `lib/transport/src/preflight.rs:52, 306-367` + +**What happens**: Preflight runs *before* DAG execution and uses raw +`println!`/`eprint!`/`eprintln!` for progress and error reporting: + +```rust +println!("preflight: lint-upsert ({})", state); +eprint!(" [{}/{}] {}...", i + 1, total, label); +eprintln!(" {:.1}s", start.elapsed().as_secs_f64()); +``` + +**Why this is a problem**: +- Not wrapped in CI groups on GitHub Actions — preflight output appears as + unstructured noise before the DAG log +- No progress observer integration — can't show preflight in the DAG progress view +- Errors are plain `format!()` strings with no structured fields +- If preflight fails in CI, there's no `::error::` annotation for GitHub + +**Suggested fix**: Either: +- (a) Make preflight a DAG itself (preflight DAG feeds into main DAG), or +- (b) At minimum, accept a `CiContext` and wrap output in CI groups, and use + `ci.error()` for failures + +--- + +## 6. Report Node Gets Raw Unstructured Text + +**Where**: `gunbc-dag/src/ci/ops.rs:971-1101` — `build_report_blocks` + +**What happens**: The report node receives stderr as raw strings and formats them +into `StructuredBlock::Raw(...)`. The `truncate_for_report` function (added +2026-02-13) caps output at 60 lines / 500 chars per line, but the underlying +data is still unstructured text that varies wildly by stage. + +**Why this is a problem**: +- Truncation is a band-aid — the real issue is that raw compiler/linker output + shouldn't flow unfiltered into a summary report +- `extract_test_failures` exists for test output, but there's no equivalent for + build errors (`extract_build_errors`), lint errors, etc. +- Different tooling (cargo build, cargo test, cargo clippy) has different output + formats, but they're all treated as opaque strings + +**Suggested fix**: Add per-stage error extractors: +- `extract_build_errors(stderr)` — pull `error[E...]` lines, filter linker noise +- `extract_lint_warnings(stderr)` — pull clippy warnings with file locations +- `extract_test_failures(stdout)` — already exists, good pattern to follow +- Generic fallback: last N lines of stderr if no structured extraction works + +--- + +## 7. No Unified Error Field Convention + +**Where**: Various ops across all DAGs + +**What happens**: Different ops use different field names for status/error info: + +| Pattern | Usage | +|---------|-------| +| `"report"` | Multi-line summary (CI report, build report) | +| `"message"` | Single-line status (CI status messages) | +| `"stderr"` / `"stdout"` | Raw command output | +| `"error"` | Rare — sometimes used for error details | +| `"skip_reason"` | Why a node was skipped | +| `"success"` / `"overall_success"` | Boolean status | + +**Why this is a problem**: No convention for which field contains human-readable +error details. Tooling can't generically "find the error message" for a failed +node. The display layer can't highlight errors without knowing the field name +convention for each op. + +**Suggested fix**: Adopt a convention. Every op that can fail should produce: +- `success: bool` — did it succeed? +- `error_summary: String` — one-line human-readable error (empty on success) +- `detail: String` — full output / diagnostic text (optional) + +Then the display layer can generically show `error_summary` on failure without +knowing anything about the specific op. + +--- + +## 8. Secret Redaction Depends on Display Functions, Not the Type System + +**Where**: `core/exec/src/display.rs` (`print_value`), `core/exec/src/execute.rs` (`print_log_entry`, now unified), any code that formats `Value` for output + +**What happened**: `Value::Secret` exists as a dedicated enum variant — the type +system already distinguishes secrets from plain strings. Yet `print_value` was +missing a `Secret` arm entirely and would have fallen through to the `_ => {}` +catch-all, silently dropping the value. Meanwhile `print_log_entry` had a +manual `Secret => "***"` match. The fix (2026-02-13) added `Secret => "***"` to +`print_value` too, but this is still the wrong architecture. + +**Why this is a problem**: Every function that ever renders, logs, serializes, +or formats a `Value` needs to remember to handle `Secret` correctly. This is +a classic "defense by convention" pattern that fails the moment someone writes +a new display path and forgets the `Secret` arm. The type system already has +the information — it should enforce redaction, not hope each call site does. + +Today there are at least these output paths for `Value`: +- `print_value` (display.rs) — now redacts, didn't before +- `truncate_for_report` / `build_report_blocks` (ci/ops.rs) — receives + strings, not `Value`; secrets would already be stringified by parse ops +- `StructuredRenderer` — receives `StructuredBlock::Raw(String)`, opaque +- `serde_json::to_string` in mock outputs — serializes `Value` directly +- Any future display/export path + +**Root cause**: There is no single "render `Value` to displayable string" +chokepoint. Instead, each consumer pattern-matches on `Value` independently. + +**Suggested fix**: Add a `Value::display_redacted(&self) -> String` method +(or a `Display` impl that always redacts secrets) as the **only** sanctioned +way to produce human-visible output from a `Value`. All display/log paths +should call this method rather than matching on variants directly. The `Secret` +variant's inner value should only be extractable via an explicit +`Value::expose_secret()` method that makes the intent clear in code review. + +This would turn "every call site must remember to redact" into "you can't +accidentally get the plaintext without calling a function that says `expose` +in its name." + +--- + +## Refactoring Order + +1. ~~**Immediate**: Unify `print_value` + `print_log_entry`~~ (done 2026-02-13) +2. **Immediate**: Secret redaction via `Value::display_redacted` chokepoint +3. **Short-term**: Add stdout capture to all CI stages (testgen, bootstrap, pragma, guardrail, verify) +4. **Short-term**: Add per-stage error extractors for the CI report +5. **Medium-term**: Unify CI vs local execution paths in `ci.rs` +6. **Medium-term**: Wrap preflight in CI groups +7. **Long-term**: Unified `ExecutionDisplay` trait with configurable verbosity +8. **Long-term**: Standardize error field conventions across all ops + +--- + +## Related + +- `TODO_workflow_audit.md` — broader workflow consolidation plan +- `TODO_hacks.md` — other fallback/hack patterns +- PR #39 — `truncate_for_report` added as immediate mitigation diff --git a/TODO/TODO_hacks.md b/TODO/TODO_hacks.md index 2416a5288cb..f0578d96fdf 100644 --- a/TODO/TODO_hacks.md +++ b/TODO/TODO_hacks.md @@ -252,3 +252,38 @@ are followed by `.expect()`. All enclosing functions return `Result`. of returning a diagnostic error. **Suggested fix**: Replace `.expect()` / `.unwrap()` with `.ok_or_else(|| ...)?`. + +--- + +## 14. Fermi guard skips live tests instead of running them in CI + +**Where**: `core/test/src/fermi.rs` — `guard()`, `guard_test_with_env()` + +**What happened**: The fermi guard previously panicked in CI when secrets +were missing and `GUNBC_TEST_MAX_COST` wasn't explicitly set. This was +designed to catch CI misconfigurations, but it conflated two concerns: +cost limits (CI config) and secret availability (environment provisioning). +The panic was removed (2026-02-13) because it caused real CI failures for +tests like `test_live_flow_github_credential_lifecycle` that require GCP +WIF credentials not provisioned on the runner. + +**What remains**: Live flow tests (`live_flow_tests` in testgen) are +intended to run in CI but currently can't because the runner lacks the +required secrets. These tests are silently skipped. + +**Intended fix**: The CI workflow should derive secret requirements from +the repo's testgen metadata (the `live_required` / `live_required_any_of` +annotations on mock specs). This would allow the workflow to: +1. Scan all `testgen_target` annotations for `live_required` secrets +2. Provision exactly the secrets that live tests need (via GitHub Actions + secrets + GCP Workload Identity Federation) +3. Set `GUNBC_TEST_MAX_COST` appropriately for the live test tier + +This turns secret provisioning from a manual CI configuration step into +something derivable from the state of the repo — when a new live test is +added with `live_required("NEW_SECRET")`, CI should automatically know +it needs to provision `NEW_SECRET`. + +**Blocked on**: GCP WIF setup for the GitHub Actions runner + a codegen +pass that extracts secret requirements from testgen metadata into the +CI workflow YAML. diff --git a/core/cli/src/lib.rs b/core/cli/src/lib.rs index 8bb7b88f98a..309a6e170e0 100644 --- a/core/cli/src/lib.rs +++ b/core/cli/src/lib.rs @@ -32,9 +32,14 @@ impl ParamType { impl From<&str> for ParamType { fn from(s: &str) -> Self { match s { + "String" | "Str" => Self::Str, "Bool" => Self::Bool, "Int" => Self::Int, - _ => Self::Str, + other => panic!( + "ParamType::from(\"{}\") — unknown type; \ + use ParamType::Str, ParamType::Int, or ParamType::Bool", + other + ), } } } @@ -228,7 +233,12 @@ pub fn parse(argv: &[String], schema: &[CliParam]) -> Result { let value = match param.type_id { ParamType::Bool => Value::Bool(val == "true"), - ParamType::Int => Value::Int(val.parse::().unwrap_or(0)), + ParamType::Int => Value::Int( + val.parse::().map_err(|_| ParseError::InvalidInt { + flag: param.flag_name(), + value: val.clone(), + })?, + ), ParamType::Str => Value::Str(val.clone()), }; values.insert(param.port_name.clone(), value); @@ -407,11 +417,17 @@ mod tests { } #[test] - fn test_invalid_int_defaults_to_zero() { - // Matches generated CLI behavior: .parse().unwrap_or(0) + fn test_invalid_int_returns_error() { let schema = vec![CliParam::new("count", "Int")]; - let result = parse(&argv(&["prog", "--count", "not-a-number"]), &schema).unwrap(); - assert_eq!(result.values["count"], Value::Int(0)); + let result = parse(&argv(&["prog", "--count", "not-a-number"]), &schema); + assert!(result.is_err()); + assert_eq!( + result.unwrap_err(), + ParseError::InvalidInt { + flag: "count".to_string(), + value: "not-a-number".to_string() + } + ); } #[test] diff --git a/core/codegen/src/cli_gen.rs b/core/codegen/src/cli_gen.rs index b92715262d4..8a03b4ecd0f 100644 --- a/core/codegen/src/cli_gen.rs +++ b/core/codegen/src/cli_gen.rs @@ -427,11 +427,15 @@ fn generate_arg_parsing(entrypoints: &[CliEntrypoint]) -> String { ep.port_name ).unwrap(), "Int" => { - let default = ep - .default_value - .as_deref() - .and_then(|d| d.parse::().ok()) - .unwrap_or(0); + let default = match ep.default_value.as_deref() { + Some(d) => d.parse::().unwrap_or_else(|_| { + panic!( + "entrypoint '{}' has invalid default int value: {:?}", + ep.port_name, d + ) + }), + None => 0, + }; writeln!(code, "let {} = match cli_inputs.get(\"{}\") {{ Some(Value::Int(i)) => *i, _ => {} }};", ep.var_name(), ep.port_name, default diff --git a/core/codegen/src/testgen/codegen.rs b/core/codegen/src/testgen/codegen.rs index 971db53880c..c3ed79430a8 100644 --- a/core/codegen/src/testgen/codegen.rs +++ b/core/codegen/src/testgen/codegen.rs @@ -48,17 +48,30 @@ enum SeedContext { RealSingleNodeRequiredInput, } +/// Determine seed policy for a type. +/// +/// This is **fail-closed**: only types on the known-safe whitelist use +/// generated placeholders. Unknown types (including new semantic carriers +/// and aliases) default to `ExplicitSeedRequired`, preventing silent +/// shape-valid-but-behavior-invalid placeholders from sneaking through. fn seed_policy_for_type(type_id: &str) -> SeedPolicy { match type_id { - // Semantic carrier types: shape-valid placeholders are often behavior-invalid. - "TransportRequest" - | "TransportResponse" - | "Credential" - | "Secret" - | "FilesystemHandle" - | "NetworkHandle" - | "ToolHandle" => SeedPolicy::ExplicitSeedRequired, - _ => SeedPolicy::Generated, + // Primitives: generated placeholders are safe. + "String" | "Bool" | "Int" | "Unit" | "Json" | "Void" + // Refined primitives + | "NonEmptyString" | "Url" | "FilePath" | "Path" | "Email" + | "PositiveInt" | "NonNegativeInt" + // Container/wrapped types + | "OptionalString" | "OptionalInt" | "OptionalBool" | "OptionalJson" + | "OptionalUrl" + | "StringList" | "IntList" | "BoolList" | "JsonList" + | "UrlList" | "FilePathList" + | "NonEmptyStringList" | "NonEmptyFilePathList" + => SeedPolicy::Generated, + // Everything else (TransportRequest, TransportResponse, Credential, + // Secret, FilesystemHandle, NetworkHandle, ToolHandle, Platform, and + // any future types) requires explicit seeds — fail closed. + _ => SeedPolicy::ExplicitSeedRequired, } } @@ -2109,6 +2122,18 @@ impl<'a, T: Clone> TestGenerator<'a, T> { node: &gunbc_ir::Node, base_inputs: &BTreeMap, ) { + // Nodes explicitly marked skip_node_example are resource/boundary nodes + // that won't be tested in single-node Real mode — no seed assertion needed. + if let Some(spec) = &self.mock_spec { + if spec + .skipped_node_examples + .iter() + .any(|s| s == &node.id.0) + { + return; + } + } + let mut missing = Vec::new(); for port in &node.inputs { @@ -3664,11 +3689,26 @@ impl<'a, T: Clone> TestGenerator<'a, T> { // must use the lowered node IDs since test execution happens on the flat DAG. let lowered = match gunbc_exec::lower(self.dag) { Ok(lowered) => lowered, - Err(_) => return None, // DAG can't be lowered; skip chain tests + Err(e) => { + // Lowering failed — emit a diagnostic test instead of silently skipping. + let err_msg = format!("DAG lowering failed, probe-observer tests skipped: {}", e); + return Some(TestSection { + title: "Probe-Observer Integration Tests".to_string(), + notes: vec![err_msg.clone()], + tests: vec![TestFn { + name: "test_probe_observer_lowering_failed".to_string(), + doc: vec!["Lowering must succeed for probe-observer tests.".to_string()], + body: vec![Stmt::Expr(Expr::call( + "panic!", + vec![Expr::Str(err_msg)], + ))], + }], + }); + } }; let lowered_analysis = analyze_dag(&lowered.dag); let po_analysis = analyze_probe_observers(&lowered.dag, spec, &lowered_analysis); - if po_analysis.tests.is_empty() { + if po_analysis.tests.is_empty() && po_analysis.gaps.is_empty() { return None; } @@ -3851,10 +3891,43 @@ impl<'a, T: Clone> TestGenerator<'a, T> { }); } + // Generate a failing test for coverage gaps (observability invariant). + // The module doc states: "Every terminal node reachable from any probe + // must have an OutputMatcher. Testgen emits an error for unobserved terminals." + if !po_analysis.gaps.is_empty() { + let gap_lines: Vec = po_analysis + .gaps + .iter() + .map(|g| { + format!( + " terminal '{}' reachable from probe '{}' has no OutputMatcher", + g.terminal_node, g.probe_node + ) + }) + .collect(); + let msg = format!( + "Observability invariant violated: {} unobserved terminal(s):\n{}\n\ + Add OutputMatchers via NodeExample or live_expected_output for these nodes.", + po_analysis.gaps.len(), + gap_lines.join("\n") + ); + tests.push(TestFn { + name: "test_observability_invariant_no_gaps".to_string(), + doc: vec![ + "Every terminal node reachable from a probe must have an OutputMatcher.".to_string(), + "This test fails when coverage gaps exist — add observers to fix.".to_string(), + ], + body: vec![Stmt::Expr(Expr::call( + "panic!", + vec![Expr::Str(msg)], + ))], + }); + } + Some(TestSection { title: "Probe-Observer Integration Tests".to_string(), notes: vec![ - format!("Probes: {} | Observers: {} | Tests: {}", + format!("Probes: {} | Observers: {} | Tests: {}", po_analysis.probes.len(), po_analysis.observers.len(), po_analysis.tests.len()), @@ -6068,6 +6141,7 @@ mod tests { #[test] fn test_seed_policy_marks_semantic_types_explicit() { + // Known semantic carriers require explicit seeds. assert_eq!( seed_policy_for_type("TransportResponse"), SeedPolicy::ExplicitSeedRequired @@ -6084,8 +6158,29 @@ mod tests { seed_policy_for_type("Secret"), SeedPolicy::ExplicitSeedRequired ); + assert_eq!( + seed_policy_for_type("FilesystemHandle"), + SeedPolicy::ExplicitSeedRequired + ); + + // Primitive/structural types are safe for generated placeholders. assert_eq!(seed_policy_for_type("String"), SeedPolicy::Generated); assert_eq!(seed_policy_for_type("Int"), SeedPolicy::Generated); + assert_eq!(seed_policy_for_type("Bool"), SeedPolicy::Generated); + assert_eq!(seed_policy_for_type("OptionalString"), SeedPolicy::Generated); + assert_eq!(seed_policy_for_type("StringList"), SeedPolicy::Generated); + + // Fail-closed: unknown/new types default to ExplicitSeedRequired. + assert_eq!( + seed_policy_for_type("SomeNewCarrierType"), + SeedPolicy::ExplicitSeedRequired, + "unknown types must fail closed" + ); + assert_eq!( + seed_policy_for_type("CustomAuthToken"), + SeedPolicy::ExplicitSeedRequired, + "unknown types must fail closed" + ); assert!(requires_explicit_seed( "TransportResponse", @@ -6100,6 +6195,8 @@ mod tests { #[test] #[should_panic(expected = "Optional input tests require explicit seeds for required semantic inputs in Real single-node mode")] fn test_optional_inputs_require_explicit_semantic_seed() { + use gunbc_test::{NodeExample, OutputMatcher}; + let mut dag: Dag<()> = Dag::new(); dag.add_node(Node::opaque( "parse", @@ -6111,9 +6208,16 @@ mod tests { (), )); + // Provide a node_example with outputs (satisfies the I/O example + // requirement) but do NOT seed the TransportResponse input — the + // seed assertion should fire. let spec = MockSpec::new("opt") .boundary("parse", "result", Value::Str("ok".into())) - .skip_node_example("parse"); + .node_example( + NodeExample::new("parse") + .input("fallback", Value::Str("fb".into())) + .output("result", OutputMatcher::non_empty()), + ); let generator = TestGenerator::new(&dag) .with_mock_spec(spec) @@ -6121,6 +6225,33 @@ mod tests { let _ = generator.generate_test_module("opt", "build_opt_graph()"); } + #[test] + fn test_skip_node_example_bypasses_seed_assertion() { + // Nodes with skip_node_example should NOT panic about missing seeds + // — they are known resource/boundary nodes that won't be tested + // in single-node Real mode. + let mut dag: Dag<()> = Dag::new(); + dag.add_node(Node::opaque( + "resolver", + vec![ + port("config", "TransportResponse"), + optional("name", "OptionalString"), + ], + vec![port("result", "String")], + (), + )); + + let spec = MockSpec::new("skip") + .boundary("resolver", "result", Value::Str("ok".into())) + .skip_node_example("resolver"); + + let generator = TestGenerator::new(&dag) + .with_mock_spec(spec) + .with_mock_spec_fn("crate::mock_spec()"); + // Should NOT panic — skip_node_example suppresses the seed check. + let _ = generator.generate_test_module("skip", "build_skip_graph()"); + } + #[test] fn test_generate_with_satisfies_matcher_uses_runtime_check() { use gunbc_test::{NodeExample, OutputMatcher}; diff --git a/core/codegen/src/testgen/probe_observer.rs b/core/codegen/src/testgen/probe_observer.rs index a6f706d65de..0ae4a7adfee 100644 --- a/core/codegen/src/testgen/probe_observer.rs +++ b/core/codegen/src/testgen/probe_observer.rs @@ -54,6 +54,8 @@ pub enum ProbeSource { BoundaryMock, /// From a NodeExample's inputs (pure node with developer-specified inputs). NodeExampleInputs, + /// From an input_mock in MockSpec (entry node with developer-specified input values). + InputMock, /// From an intermediate observer that was promoted to a probe. /// The observer's OutputMatcher::Exact values become the new probe values. IntermediateObserver { @@ -84,6 +86,11 @@ pub struct MatcherDescription { /// Whether this matcher is input-independent (valid regardless of what /// input the node receives). Exact matchers are input-dependent. pub is_input_independent: bool, + /// Whether this is a "weak" matcher (Any, IsRequest, IsResponse) that + /// asserts minimal properties. Weak matchers count as observers for + /// chain discovery, but an all-weak terminal observer is flagged in the + /// coverage report as needing stronger assertions. + pub is_weak: bool, } /// A discovered integration test: probe -> observer through a subgraph. @@ -149,6 +156,19 @@ fn is_input_independent(matcher: &OutputMatcher) -> bool { } } +/// Check if an OutputMatcher is a "weak" assertion (matches almost anything). +/// +/// Weak matchers like `Any`, `IsRequest`, and `IsResponse` pass regardless +/// of the actual value, so they provide minimal observability. They're still +/// valid for chain discovery, but an all-weak terminal observer is flagged +/// in the coverage report. +fn is_weak_matcher(matcher: &OutputMatcher) -> bool { + matches!( + matcher, + OutputMatcher::Any | OutputMatcher::IsRequest | OutputMatcher::IsResponse + ) +} + // ============================================================================ // Discovery // ============================================================================ @@ -188,6 +208,16 @@ fn extract_probes(spec: &MockSpec, _analysis: &DagAnalysis) -> Vec { } } + // Input mocks → probes at entry nodes (dangling input ports) + for im in &spec.input_mocks { + if seen.insert(im.node.clone()) { + probes.push(Probe { + node_id: im.node.clone(), + source: ProbeSource::InputMock, + }); + } + } + probes } @@ -195,63 +225,59 @@ fn extract_probes(spec: &MockSpec, _analysis: &DagAnalysis) -> Vec { fn extract_observers(spec: &MockSpec, dag: &Dag) -> Vec { let terminal_nodes = find_terminal_nodes(dag); let mut observers = Vec::new(); - let mut seen = HashSet::new(); - // NodeExamples with outputs → observers (only input-independent matchers) + // Merge matchers from NodeExamples and LiveExpectedOutputs by node_id. + // This avoids a bug where NodeExamples with only input-dependent matchers + // (Exact/Contains/Satisfies) would mark a node as "seen" without creating + // an observer, suppressing valid chain-safe matchers from live_expected_outputs. + let mut matchers_by_node: BTreeMap> = + BTreeMap::new(); + + // Collect chain-safe matchers from NodeExamples. for ex in &spec.node_examples { - if !ex.outputs.is_empty() && seen.insert(ex.node_id.clone()) { - let matchers: BTreeMap = ex - .outputs - .iter() - .filter(|(_, matcher)| is_input_independent(matcher)) - .map(|(port, matcher)| { - ( + for (port, matcher) in &ex.outputs { + if is_input_independent(matcher) { + matchers_by_node + .entry(ex.node_id.clone()) + .or_default() + .insert( port.clone(), MatcherDescription { description: format!("{:?}", matcher), has_concrete_value: matches!(matcher, OutputMatcher::Exact(_)), is_input_independent: true, + is_weak: is_weak_matcher(matcher), }, - ) - }) - .collect(); - - // Only add as observer if there are chain-compatible matchers. - if !matchers.is_empty() { - observers.push(Observer { - node_id: ex.node_id.clone(), - matchers, - is_terminal: terminal_nodes.contains(&ex.node_id), - }); + ); } } } - // LiveExpectedOutputs → observers - // Group by node, multiple ports per node. - let mut live_by_node: BTreeMap> = BTreeMap::new(); + // Merge chain-safe matchers from LiveExpectedOutputs (union of ports). for leo in &spec.live_expected_outputs { - if !seen.contains(&leo.node) && is_input_independent(&leo.matcher) { - live_by_node + if is_input_independent(&leo.matcher) { + matchers_by_node .entry(leo.node.clone()) .or_default() - .insert( - leo.port.clone(), - MatcherDescription { - description: format!("{:?}", leo.matcher), - has_concrete_value: matches!(leo.matcher, OutputMatcher::Exact(_)), - is_input_independent: true, - }, - ); + .entry(leo.port.clone()) + .or_insert_with(|| MatcherDescription { + description: format!("{:?}", leo.matcher), + has_concrete_value: matches!(leo.matcher, OutputMatcher::Exact(_)), + is_input_independent: true, + is_weak: is_weak_matcher(&leo.matcher), + }); } } - for (node_id, matchers) in live_by_node { - seen.insert(node_id.clone()); - observers.push(Observer { - node_id: node_id.clone(), - matchers, - is_terminal: terminal_nodes.contains(&node_id), - }); + + // Build observers from merged matchers. + for (node_id, matchers) in matchers_by_node { + if !matchers.is_empty() { + observers.push(Observer { + node_id: node_id.clone(), + matchers, + is_terminal: terminal_nodes.contains(&node_id), + }); + } } observers @@ -426,6 +452,7 @@ pub fn analyze_probe_observers( // For each probe, find nearest observers and generate tests. // Then promote intermediate observers to probes and recurse. let mut processed_pairs: HashSet<(String, String)> = HashSet::new(); + let mut all_probes = probes.clone(); let mut probe_queue: Vec = probes.clone(); while let Some(probe) = probe_queue.pop() { @@ -453,22 +480,18 @@ pub fn analyze_probe_observers( }); // If this is an intermediate observer (not terminal), promote to probe. + // All intermediate observers are promoted because test execution + // seeds from a full baseline DryRun — concrete Exact values at the + // observer are not needed to derive downstream window inputs. if !observer.is_terminal { - // Only promote if the observer has Exact matchers (concrete values - // that can feed downstream). - let has_exact = observer - .matchers - .values() - .any(|m| m.has_concrete_value); - - if has_exact { - probe_queue.push(Probe { - node_id: obs_node_id.clone(), - source: ProbeSource::IntermediateObserver { - upstream_probe: probe.node_id.clone(), - }, - }); - } + let promoted = Probe { + node_id: obs_node_id.clone(), + source: ProbeSource::IntermediateObserver { + upstream_probe: probe.node_id.clone(), + }, + }; + all_probes.push(promoted.clone()); + probe_queue.push(promoted); } } } @@ -510,7 +533,7 @@ pub fn analyze_probe_observers( gaps.dedup_by(|a, b| a.probe_node == b.probe_node && a.terminal_node == b.terminal_node); ProbeObserverAnalysis { - probes, + probes: all_probes, observers, tests, gaps, @@ -527,6 +550,7 @@ pub fn observability_report(analysis: &ProbeObserverAnalysis) -> String { ProbeSource::TransportMock => "transport mock", ProbeSource::BoundaryMock => "boundary mock", ProbeSource::NodeExampleInputs => "node example inputs", + ProbeSource::InputMock => "input mock", ProbeSource::IntermediateObserver { upstream_probe } => { // Use a temporary string for this case &format!("intermediate (from {})", upstream_probe) @@ -539,19 +563,42 @@ pub fn observability_report(analysis: &ProbeObserverAnalysis) -> String { lines.push(format!("Observers: {}", analysis.observers.len())); for obs in &analysis.observers { let terminal_tag = if obs.is_terminal { " [terminal]" } else { "" }; + let all_weak = obs.matchers.values().all(|m| m.is_weak); + let weak_tag = if all_weak { " [WEAK]" } else { "" }; let matcher_desc: Vec = obs .matchers .iter() .map(|(port, m)| format!("{}: {}", port, m.description)) .collect(); lines.push(format!( - " {}{} ({})", + " {}{}{} ({})", obs.node_id, terminal_tag, + weak_tag, matcher_desc.join(", ") )); } + // Flag weak terminal observers as a coverage concern. + let weak_terminals: Vec<&Observer> = analysis + .observers + .iter() + .filter(|o| o.is_terminal && o.matchers.values().all(|m| m.is_weak)) + .collect(); + if !weak_terminals.is_empty() { + lines.push(String::new()); + lines.push(format!( + "Weak terminal observers: {} (all matchers are Any/IsRequest/IsResponse)", + weak_terminals.len() + )); + for obs in &weak_terminals { + lines.push(format!( + " {} — consider adding stronger assertions (NonEmpty, IsString, IntGe, etc.)", + obs.node_id + )); + } + } + lines.push(String::new()); lines.push(format!( "Integration tests: {}", @@ -742,10 +789,10 @@ mod tests { let result = analyze_probe_observers(&dag, &spec, &analysis); - // Should have tests from probe a through observers at b and c. - // Since b is an intermediate observer with an input-independent matcher, - // it splits the chain: a->b and a->c (b isn't promoted since it has no - // concrete Exact value to use as downstream input). + // b is an intermediate observer that splits the chain: + // a->b (probe a reaches observer b) + // b->c (promoted observer b reaches terminal c) + // a->c is NOT generated because BFS from a stops at observer b. let a_to_b = result .tests .iter() @@ -756,15 +803,86 @@ mod tests { .find(|t| t.probe.node_id == "a" && t.observer.node_id == "c"); assert!(a_to_b.is_some(), "should have a->b test"); - // Since b is an intermediate observer but doesn't have Exact values, - // the chain continues past b to reach c directly from a. assert!(a_to_c.is_none(), "a->c should be segmented: a hits observer b first"); - // But b->c should exist because b is also a probe (from NodeExample inputs) + + // b is promoted to a probe (intermediate observer), so b->c exists. + // b is also a probe from NodeExample inputs, so there are two sources. let b_to_c = result .tests .iter() .find(|t| t.probe.node_id == "b" && t.observer.node_id == "c"); - assert!(b_to_c.is_some(), "should have b->c test (b is also a probe from example inputs)"); + assert!(b_to_c.is_some(), "should have b->c test (b promoted + example inputs)"); + + // Verify the promoted probe appears in the analysis. + let promoted = result + .probes + .iter() + .any(|p| p.node_id == "b" && matches!(p.source, ProbeSource::IntermediateObserver { .. })); + assert!(promoted, "b should be promoted to probe from intermediate observer"); + assert!(result.gaps.is_empty(), "no coverage gaps"); } + + #[test] + fn test_live_expected_outputs_not_suppressed_by_exact_node_example() { + // Regression test: a NodeExample with only Exact outputs (input-dependent) + // should not prevent live_expected_outputs for the same node from becoming + // observers. + let dag = linear_dag(); + + let spec = MockSpec::new("test") + .transport_mock("a", "response", gunbc_ir::Value::Str("hello".into())) + // Node c has a NodeExample with only Exact (input-dependent) output... + .node_example( + NodeExample::new("c") + .input("in", gunbc_ir::Value::Str("hello".into())) + .output("out", OutputMatcher::Exact(Box::new(gunbc_ir::Value::Str("world".into())))), + ) + // ...and a live_expected_output with chain-safe NonEmpty matcher. + .live_expected_output("c", "out", OutputMatcher::non_empty()); + + let observers = extract_observers(&spec, &dag); + + // The live_expected_output's NonEmpty matcher should create an observer + // at c, even though the NodeExample had only Exact matchers. + assert_eq!(observers.len(), 1, "should have observer at c"); + assert_eq!(observers[0].node_id, "c"); + assert!( + observers[0].matchers.contains_key("out"), + "observer should have 'out' matcher" + ); + } + + #[test] + fn test_intermediate_observer_promoted_without_exact() { + // Intermediate observers are promoted to probes even without Exact matchers. + // Tests are seeded from baseline DryRun, so concrete values aren't needed. + let dag = linear_dag(); + let analysis = simple_analysis(); + + // Probe at a, intermediate observer at b with only NonEmpty (no Exact). + let spec = MockSpec::new("test") + .transport_mock("a", "response", gunbc_ir::Value::Str("hello".into())) + .node_example( + NodeExample::new("b").output("out", OutputMatcher::non_empty()), + ) + .node_example( + NodeExample::new("c").output("out", OutputMatcher::non_empty()), + ); + + let result = analyze_probe_observers(&dag, &spec, &analysis); + + // b should be promoted as an intermediate observer → probe. + let promoted = result + .probes + .iter() + .any(|p| p.node_id == "b" && matches!(p.source, ProbeSource::IntermediateObserver { .. })); + assert!(promoted, "b should be promoted even without Exact matchers"); + + // Should have: a->b, b->c (b is promoted probe) + let a_to_b = result.tests.iter().any(|t| t.probe.node_id == "a" && t.observer.node_id == "b"); + let b_to_c = result.tests.iter().any(|t| t.probe.node_id == "b" && t.observer.node_id == "c"); + assert!(a_to_b, "should have a->b test"); + assert!(b_to_c, "should have b->c test from promoted probe"); + } } diff --git a/core/codegen/src/testgen/render_rust.rs b/core/codegen/src/testgen/render_rust.rs index a03b1d8d3f8..3a7aa09f1ef 100644 --- a/core/codegen/src/testgen/render_rust.rs +++ b/core/codegen/src/testgen/render_rust.rs @@ -10,7 +10,9 @@ use std::fmt::Write; /// Escape a string for embedding in a Rust string literal. fn escape_rust_str(s: &str) -> String { - s.replace('\\', "\\\\").replace('"', "\\\"") + s.replace('\\', "\\\\") + .replace('"', "\\\"") + .replace('\n', "\\n") } /// Generic Rust code renderer parameterized over output medium. diff --git a/core/exec/src/display.rs b/core/exec/src/display.rs index a60407a6652..03936f5c227 100644 --- a/core/exec/src/display.rs +++ b/core/exec/src/display.rs @@ -288,18 +288,22 @@ fn run_with_progress( /// Print a single output value in the standard format. pub fn print_value(port: &str, value: &Value) { match value { + Value::Secret(_) => { + println!(" {}: ***", port); + } Value::Str(s) => { if port.ends_with("stderr") || port.ends_with("stdout") { if !s.is_empty() { - println!(" {}: {}", port, s); + let t = truncate_log_value(s); + println!(" {}: {}", port, t); } } else if s.contains('\n') { - // Multi-line values (reports, etc.) — print in full - println!(" {}: {}", port, s); - } else if s.len() < 80 { + let t = truncate_log_value(s); + println!(" {}: {}", port, t); + } else if s.len() < 120 { println!(" {}: {}", port, s); } else { - println!(" {}: {}...", port, truncate_str(s, 60)); + println!(" {}: {}...", port, truncate_str(s, 80)); } } Value::Int(i) => println!(" {}: {}", port, i), @@ -308,6 +312,7 @@ pub fn print_value(port: &str, value: &Value) { Value::Set(set) => println!(" {}: {{{} items}}", port, set.len()), Value::Map(map) => println!(" {}: {{{} entries}}", port, map.len()), Value::Json(_) => println!(" {}: ", port), + Value::Skipped => {} _ => {} } } @@ -340,3 +345,36 @@ fn truncate_str(s: &str, max_chars: usize) -> &str { None => s, } } + +/// Maximum lines to display for a single port value in log output. +const MAX_LOG_VALUE_LINES: usize = 40; + +/// Truncate a multi-line string for display in CI groups and classic log output. +/// +/// Keeps the first 5 and last 35 lines, inserting a truncation marker. +/// Also truncates individual lines longer than 500 characters. +pub(crate) fn truncate_log_value(s: &str) -> String { + let lines: Vec<&str> = s.lines().collect(); + if lines.len() <= MAX_LOG_VALUE_LINES { + return s.to_string(); + } + + let head = 5; + let tail = MAX_LOG_VALUE_LINES - head; + let omitted = lines.len() - head - tail; + + let mut out = String::new(); + for line in &lines[..head] { + out.push_str(line); + out.push('\n'); + } + out.push_str(&format!(" ... ({omitted} lines omitted) ...")); + out.push('\n'); + for (i, line) in lines[lines.len() - tail..].iter().enumerate() { + out.push_str(line); + if i < tail - 1 { + out.push('\n'); + } + } + out +} diff --git a/core/exec/src/execute.rs b/core/exec/src/execute.rs index b45e28467df..b3c7346af1d 100644 --- a/core/exec/src/execute.rs +++ b/core/exec/src/execute.rs @@ -1559,34 +1559,7 @@ fn execute_loop_body( /// of context. This is the last line of defense against credential leaks. fn print_log_entry(entry: &LogEntry) { for (port, value) in &entry.outputs { - match value { - Value::Secret(_) => { - // Always redact secrets — never print actual values - println!(" {port}: ***"); - } - Value::Str(s) => { - if port.ends_with("stderr") || port.ends_with("stdout") { - if !s.is_empty() { - println!(" {port}: {s}"); - } - } else if s.contains('\n') { - // Multi-line values (reports, etc.) — print in full - println!(" {port}: {s}"); - } else if s.len() < 120 { - println!(" {port}: {s}"); - } else { - println!(" {port}: {}...", &s[..80]); - } - } - Value::Int(i) => println!(" {port}: {i}"), - Value::Bool(b) => println!(" {port}: {b}"), - Value::List(list) => println!(" {port}: [{} items]", list.len()), - Value::Set(set) => println!(" {port}: {{{} items}}", set.len()), - Value::Map(map) => println!(" {port}: {{{} entries}}", map.len()), - Value::Json(_) => println!(" {port}: "), - Value::Skipped => {} // Don't print skipped outputs - _ => {} - } + crate::display::print_value(port, value); } } @@ -2614,4 +2587,279 @@ mod tests { other => panic!("expected Value::List, got {:?}", other), } } + + // ========================================================================= + // execute_with_mode_and_inputs unit tests + // ========================================================================= + + #[test] + fn test_input_mocks_inject_into_entrypoint() { + // Node with no upstream edges receives value from input mocks + let mut dag: Dag = Dag::new(); + dag.add_node(Node::opaque( + "echo", + vec![port("data", "String")], + vec![port("data", "String")], + TestOp::echo(), + )); + + let mut mocks = BoundaryMocks::new(); + mocks.set_input("echo", "data", Value::Str("injected".into())); + + let log = execute_with_mode_and_inputs(&dag, ExecutionMode::Real, Some(&mocks)).unwrap(); + + let entry = log.get("echo").unwrap(); + assert_eq!( + entry.outputs.get("data"), + Some(&Value::Str("injected".into())), + "input mock should be injected into entrypoint" + ); + } + + #[test] + fn test_input_mocks_with_dry_run_mode() { + // Combine input mocks with DryRun boundary interception + let mut dag: Dag = Dag::new(); + dag.add_node(Node::opaque( + "prepare", + vec![port("arg", "String")], + vec![port("request", "TransportRequest")], + TestOp::produce("request", Value::Str("built-request".into())), + )); + dag.add_node(Node::opaque( + "execute_http", + vec![port("request", "TransportRequest")], + vec![port("response", "TransportResponse")], + TestOp::produce("response", Value::Str("real-response".into())), + )); + dag.add_edge(edge("prepare", "request", "execute_http", "request")); + + // DryRun mocks intercept the transport executor + let mut dry_mocks = BoundaryMocks::new(); + dry_mocks.set_value( + "execute_http", + "response", + Value::Str("mock-response".into()), + ); + + // Input mocks inject the entrypoint arg + let mut input_mocks = BoundaryMocks::new(); + input_mocks.set_input("prepare", "arg", Value::Str("injected-arg".into())); + + let log = execute_with_mode_and_inputs( + &dag, + ExecutionMode::DryRun(dry_mocks), + Some(&input_mocks), + ) + .unwrap(); + + // prepare should run normally with the injected input + let prepare = log.get("prepare").unwrap(); + assert!(!prepare.was_intercepted); + + // execute_http should be intercepted + let exec = log.get("execute_http").unwrap(); + assert!(exec.was_intercepted); + assert_eq!( + exec.outputs.get("response"), + Some(&Value::Str("mock-response".into())) + ); + } + + #[test] + fn test_input_mocks_per_port_on_non_root_node() { + // Node B has two inputs: x (wired from A) and y (unwired entrypoint). + // Input mock injects B.y; B.x should come from A's output. + // This verifies per-port entrypoint injection, not per-node. + let mut dag: Dag = Dag::new(); + dag.add_node(Node::opaque( + "A", + vec![], + vec![port("out", "String")], + TestOp::produce("out", Value::Str("from-A".into())), + )); + dag.add_node(Node::opaque( + "B", + vec![port("x", "String"), port("y", "String")], + vec![port("x", "String"), port("y", "String")], + TestOp::echo(), // echoes all inputs as outputs + )); + dag.add_edge(edge("A", "out", "B", "x")); + + // Provide input mock for the unwired entrypoint port B.y + let mut input_mocks = BoundaryMocks::new(); + input_mocks.set_input("B", "y", Value::Str("from-mock".into())); + + let log = + execute_with_mode_and_inputs(&dag, ExecutionMode::Real, Some(&input_mocks)).unwrap(); + + let b = log.get("B").unwrap(); + assert_eq!( + b.outputs.get("x"), + Some(&Value::Str("from-A".into())), + "wired port B.x should receive value from upstream A" + ); + assert_eq!( + b.outputs.get("y"), + Some(&Value::Str("from-mock".into())), + "unwired entrypoint port B.y should receive value from input mock" + ); + } + + #[test] + fn test_input_mocks_none_works() { + // Passing None for input_mocks should work the same as execute_with_mode + let mut dag: Dag = Dag::new(); + dag.add_node(Node::opaque( + "A", + vec![], + vec![port("out", "String")], + TestOp::produce("out", Value::Str("hello".into())), + )); + + let log = execute_with_mode_and_inputs(&dag, ExecutionMode::Real, None).unwrap(); + assert_eq!(log.entries.len(), 1); + assert_eq!( + log.entries[0].outputs.get("out"), + Some(&Value::Str("hello".into())) + ); + } + + #[test] + fn test_remap_input_mocks_preserves_non_subdag() { + // remap_input_mocks should keep original entries alongside remapped ones. + // We test this indirectly via execute_with_mode_and_inputs on a flat DAG. + let mut dag: Dag = Dag::new(); + dag.add_node(Node::opaque( + "a", + vec![port("x", "String")], + vec![port("x", "String")], + TestOp::echo(), + )); + dag.add_node(Node::opaque( + "b", + vec![port("y", "String")], + vec![port("y", "String")], + TestOp::echo(), + )); + + let mut input = BoundaryMocks::new(); + input.set_input("a", "x", Value::Str("alpha".into())); + input.set_input("b", "y", Value::Str("beta".into())); + + let log = execute_with_mode_and_inputs(&dag, ExecutionMode::Real, Some(&input)).unwrap(); + + assert_eq!( + log.get("a").unwrap().outputs.get("x"), + Some(&Value::Str("alpha".into())) + ); + assert_eq!( + log.get("b").unwrap().outputs.get("y"), + Some(&Value::Str("beta".into())) + ); + } + + // ========================================================================= + // remap_input_mocks unit tests + // ========================================================================= + + #[test] + fn test_remap_input_mocks_with_remaps() { + let mut mocks = BoundaryMocks::new(); + mocks.set_input("subdag", "port_a", Value::Str("value".into())); + + let mut remaps: HashMap<(String, String), Vec<(String, String)>> = HashMap::new(); + remaps.insert( + ("subdag".to_string(), "port_a".to_string()), + vec![("subdag/inner_entry".to_string(), "inner_port".to_string())], + ); + + let result = remap_input_mocks(&mocks, &remaps); + + // Original key should still exist + assert_eq!( + result.get_input("subdag", "port_a"), + Some(&Value::Str("value".into())) + ); + // Remapped key should also exist + assert_eq!( + result.get_input("subdag/inner_entry", "inner_port"), + Some(&Value::Str("value".into())) + ); + } + + #[test] + fn test_remap_input_mocks_empty_remaps() { + let mut mocks = BoundaryMocks::new(); + mocks.set_input("node", "port", Value::Int(42)); + + let remaps: HashMap<(String, String), Vec<(String, String)>> = HashMap::new(); + + let result = remap_input_mocks(&mocks, &remaps); + assert_eq!( + result.get_input("node", "port"), + Some(&Value::Int(42)), + "empty remaps should preserve all inputs" + ); + } + + #[test] + fn test_remap_input_mocks_multi_target() { + let mut mocks = BoundaryMocks::new(); + mocks.set_input("subdag", "data", Value::Str("shared".into())); + + let mut remaps: HashMap<(String, String), Vec<(String, String)>> = HashMap::new(); + remaps.insert( + ("subdag".to_string(), "data".to_string()), + vec![ + ("subdag/inner_a".to_string(), "input_a".to_string()), + ("subdag/inner_b".to_string(), "input_b".to_string()), + ], + ); + + let result = remap_input_mocks(&mocks, &remaps); + + // Both targets should receive the value + assert_eq!( + result.get_input("subdag/inner_a", "input_a"), + Some(&Value::Str("shared".into())) + ); + assert_eq!( + result.get_input("subdag/inner_b", "input_b"), + Some(&Value::Str("shared".into())) + ); + } + + #[test] + fn test_remap_mode_inputs_dry_run() { + let mut dry_mocks = BoundaryMocks::new(); + dry_mocks.set_input("subdag", "port", Value::Int(99)); + + let mut remaps: HashMap<(String, String), Vec<(String, String)>> = HashMap::new(); + remaps.insert( + ("subdag".to_string(), "port".to_string()), + vec![("subdag/inner".to_string(), "inner_port".to_string())], + ); + + let mode = ExecutionMode::DryRun(dry_mocks); + let result = remap_mode_inputs(mode, &remaps); + + match result { + ExecutionMode::DryRun(mocks) => { + assert_eq!( + mocks.get_input("subdag/inner", "inner_port"), + Some(&Value::Int(99)), + "DryRun mocks should be remapped" + ); + } + _ => panic!("expected DryRun mode"), + } + } + + #[test] + fn test_remap_mode_inputs_real_unchanged() { + let remaps: HashMap<(String, String), Vec<(String, String)>> = HashMap::new(); + let mode = remap_mode_inputs(ExecutionMode::Real, &remaps); + assert!(matches!(mode, ExecutionMode::Real)); + } } diff --git a/core/ir/src/transport/ci/render.rs b/core/ir/src/transport/ci/render.rs index fb689494d62..9f1182eed51 100644 --- a/core/ir/src/transport/ci/render.rs +++ b/core/ir/src/transport/ci/render.rs @@ -107,6 +107,19 @@ pub struct RenderConfig { /// Job-level timeout in minutes. GitHub Actions defaults to 360 (6 hours) /// if unset, which is far too long for most CI jobs. Default: 30 minutes. pub timeout_minutes: u32, + + /// Workflow-level permission scopes (e.g., `("id-token", "write")`). + /// + /// Rendered as a `permissions:` block in GitHub Actions. Entries are + /// sorted by key for deterministic output. + pub permissions: Vec<(String, String)>, + + /// Secret names to expose as environment variables in the CI run step. + /// + /// Each entry is a secret name; the rendering layer maps it to the + /// provider's secret reference syntax (e.g., `${{ secrets.NAME }}` for + /// GitHub Actions). + pub secrets_env: Vec, } impl Default for RenderConfig { @@ -124,6 +137,8 @@ impl Default for RenderConfig { git: GitConfig::default(), cache: None, timeout_minutes: 30, + permissions: Vec::new(), + secrets_env: Vec::new(), } } } @@ -199,6 +214,22 @@ impl RenderConfig { self } + /// Set workflow-level permissions. + /// + /// Accepts `(scope, level)` pairs like `("id-token", "write")`. + /// Entries are sorted by key for deterministic output. + pub fn with_permissions(mut self, mut perms: Vec<(String, String)>) -> Self { + perms.sort_by(|a, b| a.0.cmp(&b.0)); + self.permissions = perms; + self + } + + /// Set secret names to expose as environment variables in the CI run step. + pub fn with_secrets_env(mut self, secrets: Vec) -> Self { + self.secrets_env = secrets; + self + } + /// Build the complete set of environment variables for CI rendering. /// /// Merges cargo-derived env vars with any manually-added env vars. diff --git a/core/test/src/fermi.rs b/core/test/src/fermi.rs index cc3827730e1..978b47ec3ee 100644 --- a/core/test/src/fermi.rs +++ b/core/test/src/fermi.rs @@ -103,6 +103,28 @@ pub fn max_cost_from_env() -> FermiCost { }) } +/// True when `GUNBC_TEST_MAX_COST` is explicitly set in the environment. +/// +/// When the cost limit is explicit, guards should skip silently instead of +/// panicking — the caller made a deliberate choice (e.g., the preflight +/// test gate limits to `S` to avoid running live tests that require secrets). +fn cost_limit_is_explicit() -> bool { + env::var("GUNBC_TEST_MAX_COST") + .ok() + .and_then(|v| FermiCost::parse(&v)) + .is_some() +} + +/// True when an env var is set to a non-empty value. +/// +/// GitHub Actions exports undefined secrets as empty strings (`${{ secrets.X }}` +/// resolves to `""` when `X` is not configured). Plain `env::var(k).is_ok()` +/// would treat that as "present", allowing live tests to run with blank +/// credentials. This helper rejects both unset and empty-string values. +fn env_is_present(key: &str) -> bool { + env::var(key).map(|v| !v.is_empty()).unwrap_or(false) +} + /// True only when running inside a GitHub Actions workflow. /// /// `GITHUB_ACTIONS` is the authoritative signal — it is set automatically @@ -118,10 +140,21 @@ fn in_github_actions() -> bool { /// Guard a test based on its metadata. /// /// Returns true if the test should run, false if it should be skipped. +/// +/// In GitHub Actions with the default cost budget, exceeding the budget +/// causes a panic (to catch CI misconfigurations). When the cost limit is +/// explicitly set via `GUNBC_TEST_MAX_COST`, tests that exceed the budget +/// are silently skipped — the caller made a deliberate choice. +/// +/// Missing secrets always cause a graceful skip (never panic). Secret +/// availability depends on the CI environment's provisioning, not on test +/// configuration — a runner may legitimately lack specific cloud credentials. pub fn guard(meta: TestMeta<'_>) -> bool { let max_cost = max_cost_from_env(); + let explicit = cost_limit_is_explicit(); + if meta.cost > max_cost { - if in_github_actions() { + if in_github_actions() && !explicit { panic!( "skipping {}: cost {} exceeds max {} (set GUNBC_TEST_MAX_COST=...)", meta.name, @@ -129,6 +162,12 @@ pub fn guard(meta: TestMeta<'_>) -> bool { max_cost.as_str() ); } + eprintln!( + "[guard] skipping {}: cost {} exceeds max {}", + meta.name, + meta.cost.as_str(), + max_cost.as_str() + ); return false; } @@ -137,16 +176,14 @@ pub fn guard(meta: TestMeta<'_>) -> bool { .secrets .iter() .copied() - .filter(|k| env::var(k).is_err()) + .filter(|k| !env_is_present(k)) .collect(); if !missing.is_empty() { - if in_github_actions() { - panic!( - "skipping {}: missing secrets [{}]", - meta.name, - missing.join(", ") - ); - } + eprintln!( + "[guard] skipping {}: missing secrets [{}]", + meta.name, + missing.join(", ") + ); return false; } } @@ -181,6 +218,9 @@ pub fn guard_test( /// /// `required` are checked directly. Each group in `required_any_of` requires at /// least one env var to be present. +/// +/// Missing secrets always skip gracefully. Cost-exceeded panics in CI are +/// suppressed when `GUNBC_TEST_MAX_COST` is explicitly set. pub fn guard_test_with_env( name: &str, _class: TestClass, @@ -190,8 +230,10 @@ pub fn guard_test_with_env( required_any_of: &[&[&str]], ) -> bool { let max_cost = max_cost_from_env(); + let explicit = cost_limit_is_explicit(); + if cost > max_cost { - if in_github_actions() { + if in_github_actions() && !explicit { panic!( "skipping {}: cost {} exceeds max {} (set GUNBC_TEST_MAX_COST=...)", name, @@ -199,6 +241,12 @@ pub fn guard_test_with_env( max_cost.as_str() ); } + eprintln!( + "[guard] skipping {}: cost {} exceeds max {}", + name, + cost.as_str(), + max_cost.as_str() + ); return false; } @@ -206,16 +254,14 @@ pub fn guard_test_with_env( let missing: Vec<&str> = required .iter() .copied() - .filter(|k| env::var(k).is_err()) + .filter(|k| !env_is_present(k)) .collect(); if !missing.is_empty() { - if in_github_actions() { - panic!( - "skipping {}: missing secrets [{}]", - name, - missing.join(", ") - ); - } + eprintln!( + "[guard] skipping {}: missing secrets [{}]", + name, + missing.join(", ") + ); return false; } } @@ -223,19 +269,17 @@ pub fn guard_test_with_env( if !required_any_of.is_empty() { let mut missing_groups: Vec = Vec::new(); for group in required_any_of { - let present = group.iter().any(|k| env::var(k).is_ok()); + let present = group.iter().any(|k| env_is_present(k)); if !present { missing_groups.push(group.join(" | ")); } } if !missing_groups.is_empty() { - if in_github_actions() { - panic!( - "skipping {}: missing secrets [{}]", - name, - missing_groups.join(", ") - ); - } + eprintln!( + "[guard] skipping {}: missing secrets [{}]", + name, + missing_groups.join(", ") + ); return false; } } @@ -246,3 +290,289 @@ pub fn guard_test_with_env( true } + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Mutex; + + // Env-var tests must run serially to avoid races on global state. + static ENV_LOCK: Mutex<()> = Mutex::new(()); + + /// Helper: run `f` with the given env overrides, restoring afterward. + fn with_env(overrides: &[(&str, Option<&str>)], f: impl FnOnce()) { + // Accept poisoned lock: prior #[should_panic] tests may have poisoned it. + let lock = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let saved: Vec<(&str, Option)> = overrides + .iter() + .map(|(k, _)| (*k, env::var(k).ok())) + .collect(); + for (k, v) in overrides { + match v { + Some(val) => env::set_var(k, val), + None => env::remove_var(k), + } + } + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(f)); + for (k, v) in &saved { + match v { + Some(val) => env::set_var(k, val), + None => env::remove_var(k), + } + } + // Release lock before re-panicking to avoid poisoning the mutex. + drop(lock); + if let Err(e) = result { + std::panic::resume_unwind(e); + } + } + + #[test] + fn test_cost_limit_is_explicit_when_set() { + with_env(&[("GUNBC_TEST_MAX_COST", Some("S"))], || { + assert!(cost_limit_is_explicit()); + }); + } + + #[test] + fn test_cost_limit_is_not_explicit_when_unset() { + with_env(&[("GUNBC_TEST_MAX_COST", None)], || { + assert!(!cost_limit_is_explicit()); + }); + } + + #[test] + fn test_cost_limit_is_not_explicit_for_invalid_value() { + with_env(&[("GUNBC_TEST_MAX_COST", Some("bogus"))], || { + assert!(!cost_limit_is_explicit()); + }); + } + + #[test] + fn test_guard_skips_silently_with_explicit_cost_limit_in_ci() { + // Simulates the preflight scenario: GITHUB_ACTIONS=true + explicit cost limit. + // The guard should return false (skip), NOT panic. + with_env( + &[ + ("GITHUB_ACTIONS", Some("true")), + ("GUNBC_TEST_MAX_COST", Some("S")), + ], + || { + let result = guard(TestMeta { + name: "expensive_test", + class: TestClass::Integration, + cost: FermiCost::M, + requires: &[], + secrets: &[], + }); + assert!(!result, "test should be skipped, not run"); + }, + ); + } + + #[test] + fn test_guard_skips_missing_secrets_in_ci_without_explicit_limit() { + // Missing secrets always skip gracefully, even in CI without an + // explicit cost limit. Secret availability is an environment concern, + // not a test configuration bug. + with_env( + &[ + ("GITHUB_ACTIONS", Some("true")), + ("GUNBC_TEST_MAX_COST", None), + // Ensure the secret is missing + ("NONEXISTENT_SECRET_FOR_TEST", None), + ], + || { + let result = guard(TestMeta { + name: "secret_test", + class: TestClass::Integration, + cost: FermiCost::XS, // within budget + requires: &[], + secrets: &["NONEXISTENT_SECRET_FOR_TEST"], + }); + assert!(!result, "test should be skipped, not run"); + }, + ); + } + + #[test] + fn test_guard_skips_missing_secrets_with_explicit_cost_limit() { + // With explicit cost limit, missing secrets should skip, not panic. + with_env( + &[ + ("GITHUB_ACTIONS", Some("true")), + ("GUNBC_TEST_MAX_COST", Some("XL")), + ("NONEXISTENT_SECRET_FOR_TEST", None), + ], + || { + let result = guard(TestMeta { + name: "secret_test", + class: TestClass::Integration, + cost: FermiCost::XS, + requires: &[], + secrets: &["NONEXISTENT_SECRET_FOR_TEST"], + }); + assert!(!result, "test should be skipped, not run"); + }, + ); + } + + #[test] + fn test_guard_test_with_env_skips_with_explicit_limit() { + // Simulates the exact CI failure scenario: live flow test with required + // secrets, running in GitHub Actions with an explicit cost limit. + with_env( + &[ + ("GITHUB_ACTIONS", Some("true")), + ("GUNBC_TEST_MAX_COST", Some("S")), + ], + || { + let result = guard_test_with_env( + "test_live_flow", + TestClass::Integration, + FermiCost::M, + &["fs", "shell"], + &["GCP_WIF_PROVIDER", "GCP_SECRETS_PROJECT"], + &[&["GCP_SECRETS_SA", "GCP_SECRETS_IMPERSONATE_SA"]], + ); + assert!(!result, "live test should be skipped with explicit cost limit"); + }, + ); + } + + #[test] + fn test_guard_test_with_env_skips_missing_secrets_without_explicit_limit() { + // Missing secrets always skip gracefully, even in CI without an + // explicit cost limit. + with_env( + &[ + ("GITHUB_ACTIONS", Some("true")), + ("GUNBC_TEST_MAX_COST", None), + ], + || { + let result = guard_test_with_env( + "test_live_flow", + TestClass::Integration, + FermiCost::XS, // within default XL budget + &[], + &["DEFINITELY_MISSING_SECRET"], + &[], + ); + assert!(!result, "test should be skipped, not run"); + }, + ); + } + + #[test] + fn test_env_is_present_rejects_empty_string() { + // GitHub Actions exports undefined secrets as "" — guard must treat that as missing. + with_env(&[("EMPTY_SECRET_TEST", Some(""))], || { + assert!( + !env_is_present("EMPTY_SECRET_TEST"), + "empty string should be treated as absent" + ); + }); + } + + #[test] + fn test_env_is_present_accepts_non_empty() { + with_env(&[("PRESENT_SECRET_TEST", Some("value"))], || { + assert!(env_is_present("PRESENT_SECRET_TEST")); + }); + } + + #[test] + fn test_env_is_present_rejects_unset() { + with_env(&[("MISSING_SECRET_TEST", None)], || { + assert!(!env_is_present("MISSING_SECRET_TEST")); + }); + } + + #[test] + fn test_guard_skips_on_empty_string_secret_in_ci() { + // Empty-string secrets (from undefined GitHub Actions secrets) must be + // detected as missing, not silently treated as present. + with_env( + &[ + ("GITHUB_ACTIONS", Some("true")), + ("GUNBC_TEST_MAX_COST", None), + ("EMPTY_GCP_SECRET_TEST", Some("")), + ], + || { + let result = guard(TestMeta { + name: "empty_secret_test", + class: TestClass::Integration, + cost: FermiCost::XS, + requires: &[], + secrets: &["EMPTY_GCP_SECRET_TEST"], + }); + assert!(!result, "empty-string secret should be treated as missing"); + }, + ); + } + + #[test] + fn test_guard_test_with_env_skips_on_empty_string_required() { + with_env( + &[ + ("GITHUB_ACTIONS", Some("true")), + ("GUNBC_TEST_MAX_COST", None), + ("EMPTY_REQUIRED_TEST", Some("")), + ], + || { + let result = guard_test_with_env( + "empty_required_test", + TestClass::Integration, + FermiCost::XS, + &[], + &["EMPTY_REQUIRED_TEST"], + &[], + ); + assert!(!result, "empty-string secret should be treated as missing"); + }, + ); + } + + #[test] + fn test_guard_test_with_env_skips_on_empty_string_any_of() { + with_env( + &[ + ("GITHUB_ACTIONS", Some("true")), + ("GUNBC_TEST_MAX_COST", None), + ("EMPTY_GROUP_A", Some("")), + ("EMPTY_GROUP_B", Some("")), + ], + || { + let result = guard_test_with_env( + "empty_any_of_test", + TestClass::Integration, + FermiCost::XS, + &[], + &[], + &[&["EMPTY_GROUP_A", "EMPTY_GROUP_B"]], + ); + assert!(!result, "empty-string secrets should be treated as missing"); + }, + ); + } + + #[test] + fn test_guard_runs_within_budget() { + with_env( + &[ + ("GITHUB_ACTIONS", None), + ("GUNBC_TEST_MAX_COST", Some("M")), + ], + || { + let result = guard(TestMeta { + name: "cheap_test", + class: TestClass::Hermetic, + cost: FermiCost::S, + requires: &[], + secrets: &[], + }); + assert!(result, "test within budget should run"); + }, + ); + } +} diff --git a/core/test/src/mock_spec.rs b/core/test/src/mock_spec.rs index 85ba01690d1..fe8f601aa85 100644 --- a/core/test/src/mock_spec.rs +++ b/core/test/src/mock_spec.rs @@ -1567,4 +1567,154 @@ mod tests { } )); } + + // ======================================================================== + // to_boundary_mocks tests + // ======================================================================== + + use gunbc_ir::{NodeId, PortName}; + + /// Helper to get the static value from a BoundaryMocks output mock. + fn get_output_value(bm: &BoundaryMocks, node: &str, port: &str) -> Option { + bm.get_mock(&NodeId::from(node), &PortName::from(port)) + .map(|m| m.value.clone()) + } + + #[test] + fn test_to_boundary_mocks_includes_boundary_mocks() { + let spec = MockSpec::new("test") + .boundary("env_net", "network", Value::Str("mock-net".into())) + .boundary("env_fs", "filesystem", Value::Bool(true)); + + let bm = spec.to_boundary_mocks(); + assert_eq!( + get_output_value(&bm, "env_net", "network"), + Some(Value::Str("mock-net".into())), + "boundary mock should appear in BoundaryMocks" + ); + assert_eq!( + get_output_value(&bm, "env_fs", "filesystem"), + Some(Value::Bool(true)), + "second boundary mock should appear" + ); + } + + #[test] + fn test_to_boundary_mocks_includes_transport_mocks() { + let spec = MockSpec::new("test") + .transport_mock("execute_http", "response", Value::Str("200 OK".into())); + + let bm = spec.to_boundary_mocks(); + assert_eq!( + get_output_value(&bm, "execute_http", "response"), + Some(Value::Str("200 OK".into())), + "transport mock should be included in BoundaryMocks" + ); + } + + #[test] + fn test_to_boundary_mocks_includes_input_mocks() { + let spec = MockSpec::new("test") + .input_mock("prepare", "provider", Value::Str("gcp".into())); + + let bm = spec.to_boundary_mocks(); + assert_eq!( + bm.get_input("prepare", "provider"), + Some(&Value::Str("gcp".into())), + "input mock should be included via set_input" + ); + } + + #[test] + fn test_to_boundary_mocks_combines_all_mock_types() { + let spec = MockSpec::new("combined") + .boundary("env", "net", Value::Str("net-mock".into())) + .transport_mock("exec", "resp", Value::Int(42)) + .input_mock("entry", "arg", Value::Bool(false)); + + let bm = spec.to_boundary_mocks(); + assert_eq!( + get_output_value(&bm, "env", "net"), + Some(Value::Str("net-mock".into())) + ); + assert_eq!(get_output_value(&bm, "exec", "resp"), Some(Value::Int(42))); + assert_eq!(bm.get_input("entry", "arg"), Some(&Value::Bool(false))); + } + + #[test] + fn test_to_boundary_mocks_sequence() { + let spec = MockSpec::new("test").boundary_sequence( + "poll", + "status", + Value::Str("done".into()), + vec![ + Value::Str("pending".into()), + Value::Str("running".into()), + ], + ); + + let bm = spec.to_boundary_mocks(); + assert!( + bm.get_mock(&NodeId::from("poll"), &PortName::from("status")) + .is_some(), + "sequence mock should be retrievable" + ); + } + + #[test] + fn test_to_boundary_mocks_empty_spec() { + let spec = MockSpec::new("empty"); + let bm = spec.to_boundary_mocks(); + assert!( + get_output_value(&bm, "any", "port").is_none(), + "empty spec produces empty BoundaryMocks" + ); + assert!( + bm.get_input("any", "port").is_none(), + "empty spec has no input mocks" + ); + } + + #[test] + fn test_to_dry_run_mocks_excludes_input_mocks() { + let spec = MockSpec::new("test") + .boundary("env", "net", Value::Str("mock".into())) + .input_mock("entry", "arg", Value::Bool(true)); + + let dry = spec.to_dry_run_mocks(); + assert_eq!( + get_output_value(&dry, "env", "net"), + Some(Value::Str("mock".into())), + "boundary mock should be in dry run mocks" + ); + assert!( + dry.get_input("entry", "arg").is_none(), + "input mocks should NOT be in dry run mocks" + ); + } + + #[test] + fn test_to_boundary_mocks_vs_to_dry_run_mocks_difference() { + let spec = MockSpec::new("diff") + .boundary("env", "net", Value::Str("mock".into())) + .transport_mock("exec", "resp", Value::Int(1)) + .input_mock("entry", "arg", Value::Bool(true)); + + let boundary = spec.to_boundary_mocks(); + let dry_run = spec.to_dry_run_mocks(); + + // Both should have boundary and transport mocks + assert_eq!( + get_output_value(&boundary, "env", "net"), + get_output_value(&dry_run, "env", "net") + ); + assert_eq!( + get_output_value(&boundary, "exec", "resp"), + get_output_value(&dry_run, "exec", "resp") + ); + + // Only to_boundary_mocks should have input mocks + assert!(boundary.get_input("entry", "arg").is_some()); + assert!(dry_run.get_input("entry", "arg").is_none()); + } } diff --git a/core/test/src/window.rs b/core/test/src/window.rs index baa318a0141..561915b3b0b 100644 --- a/core/test/src/window.rs +++ b/core/test/src/window.rs @@ -643,6 +643,298 @@ mod window_helper_tests { } } +#[cfg(test)] +mod window_subdag_tests { + use super::*; + use gunbc_ir::build::{edge, port}; + use gunbc_ir::{Dag, Node}; + + #[test] + fn subdag_contains_only_window_nodes() { + // DAG: A -> B -> C + let mut dag: Dag<()> = Dag::new(); + dag.add_node(Node::opaque("a", vec![], vec![port("out", "Int")], ())); + dag.add_node(Node::opaque( + "b", + vec![port("in", "Int")], + vec![port("out", "Int")], + (), + )); + dag.add_node(Node::opaque("c", vec![port("in", "Int")], vec![], ())); + dag.add_edge(edge("a", "out", "b", "in")); + dag.add_edge(edge("b", "out", "c", "in")); + + let window = Window::from_nodes(&dag, vec!["a", "b"]); + let sub = window_subdag(&dag, &window); + + assert_eq!(sub.nodes.len(), 2, "subdag should have 2 nodes"); + let ids: Vec<&str> = sub.nodes.iter().map(|n| n.id.0.as_str()).collect(); + assert!(ids.contains(&"a")); + assert!(ids.contains(&"b")); + assert!(!ids.contains(&"c"), "node c should be excluded"); + } + + #[test] + fn subdag_retains_internal_edges_only() { + // DAG: A -> B -> C + let mut dag: Dag<()> = Dag::new(); + dag.add_node(Node::opaque("a", vec![], vec![port("out", "Int")], ())); + dag.add_node(Node::opaque( + "b", + vec![port("in", "Int")], + vec![port("out", "Int")], + (), + )); + dag.add_node(Node::opaque("c", vec![port("in", "Int")], vec![], ())); + dag.add_edge(edge("a", "out", "b", "in")); + dag.add_edge(edge("b", "out", "c", "in")); + + let window = Window::from_nodes(&dag, vec!["a", "b"]); + let sub = window_subdag(&dag, &window); + + assert_eq!( + sub.edges.len(), + 1, + "only the a->b edge should be in the subdag" + ); + assert_eq!(sub.edges[0].from_node.0, "a"); + assert_eq!(sub.edges[0].to_node.0, "b"); + } + + #[test] + fn subdag_single_node_has_no_edges() { + let mut dag: Dag<()> = Dag::new(); + dag.add_node(Node::opaque( + "a", + vec![port("in", "Int")], + vec![port("out", "Int")], + (), + )); + dag.add_node(Node::opaque("b", vec![port("in", "Int")], vec![], ())); + dag.add_edge(edge("a", "out", "b", "in")); + + let window = Window::from_nodes(&dag, vec!["a"]); + let sub = window_subdag(&dag, &window); + + assert_eq!(sub.nodes.len(), 1); + assert_eq!(sub.edges.len(), 0, "single-node window has no internal edges"); + } + + #[test] + fn subdag_preserves_node_ports() { + let mut dag: Dag<()> = Dag::new(); + dag.add_node(Node::opaque( + "a", + vec![port("x", "String"), port("y", "Int")], + vec![port("out", "Bool")], + (), + )); + + let window = Window::from_nodes(&dag, vec!["a"]); + let sub = window_subdag(&dag, &window); + + let node = sub.get_node(&"a".into()).expect("node a should exist"); + assert_eq!(node.inputs.len(), 2, "input ports should be preserved"); + assert_eq!(node.outputs.len(), 1, "output ports should be preserved"); + } + + #[test] + fn subdag_diamond_topology() { + // Diamond: A -> B, A -> C, B -> D, C -> D + let mut dag: Dag<()> = Dag::new(); + dag.add_node(Node::opaque("a", vec![], vec![port("out", "Int")], ())); + dag.add_node(Node::opaque( + "b", + vec![port("in", "Int")], + vec![port("out", "Int")], + (), + )); + dag.add_node(Node::opaque( + "c", + vec![port("in", "Int")], + vec![port("out", "Int")], + (), + )); + dag.add_node(Node::opaque( + "d", + vec![port("x", "Int"), port("y", "Int")], + vec![], + (), + )); + dag.add_edge(edge("a", "out", "b", "in")); + dag.add_edge(edge("a", "out", "c", "in")); + dag.add_edge(edge("b", "out", "d", "x")); + dag.add_edge(edge("c", "out", "d", "y")); + + // Window all 4 nodes + let window = Window::from_nodes(&dag, vec!["a", "b", "c", "d"]); + let sub = window_subdag(&dag, &window); + + assert_eq!(sub.nodes.len(), 4); + assert_eq!(sub.edges.len(), 4); + + // Window only middle layer + let window2 = Window::from_nodes(&dag, vec!["b", "c"]); + let sub2 = window_subdag(&dag, &window2); + + assert_eq!(sub2.nodes.len(), 2); + assert_eq!(sub2.edges.len(), 0, "b and c have no edges between them"); + } +} + +#[cfg(test)] +mod assert_chain_outputs_tests { + use super::*; + use crate::mock_spec::OutputMatcher; + use gunbc_exec::{ExecutionLog, LogEntry}; + use gunbc_ir::Value; + use std::collections::HashMap; + + fn make_log(entries: Vec<(&str, Vec<(&str, Value)>)>) -> ExecutionLog { + ExecutionLog { + entries: entries + .into_iter() + .map(|(node, outputs)| LogEntry { + node_id: node.to_string(), + outputs: outputs + .into_iter() + .map(|(p, v)| (p.to_string(), v)) + .collect(), + was_intercepted: false, + }) + .collect(), + } + } + + #[test] + fn passes_when_all_matchers_match() { + let log = make_log(vec![ + ("parse", vec![("content", Value::Str("hello world".into()))]), + ("validate", vec![("ok", Value::Bool(true))]), + ]); + + let mut matchers = HashMap::new(); + matchers.insert( + ("parse".to_string(), "content".to_string()), + OutputMatcher::contains("hello"), + ); + matchers.insert( + ("validate".to_string(), "ok".to_string()), + OutputMatcher::exact(Value::Bool(true)), + ); + + assert!(assert_chain_outputs(&log, &matchers).is_ok()); + } + + #[test] + fn fails_when_matcher_does_not_match() { + let log = make_log(vec![( + "parse", + vec![("content", Value::Str("goodbye".into()))], + )]); + + let mut matchers = HashMap::new(); + matchers.insert( + ("parse".to_string(), "content".to_string()), + OutputMatcher::contains("hello"), + ); + + let err = assert_chain_outputs(&log, &matchers).expect_err("should fail"); + match err { + WindowError::MatcherFailed { node, port, .. } => { + assert_eq!(node, "parse"); + assert_eq!(port, "content"); + } + other => panic!("expected MatcherFailed, got {:?}", other), + } + } + + #[test] + fn fails_when_node_missing_from_log() { + let log = make_log(vec![]); + + let mut matchers = HashMap::new(); + matchers.insert( + ("missing_node".to_string(), "out".to_string()), + OutputMatcher::non_empty(), + ); + + let err = assert_chain_outputs(&log, &matchers).expect_err("should fail"); + match err { + WindowError::MissingObserverNode { node } => { + assert_eq!(node, "missing_node"); + } + other => panic!("expected MissingObserverNode, got {:?}", other), + } + } + + #[test] + fn fails_when_port_missing_from_log() { + let log = make_log(vec![("node_a", vec![("out", Value::Int(1))])]); + + let mut matchers = HashMap::new(); + matchers.insert( + ("node_a".to_string(), "missing_port".to_string()), + OutputMatcher::non_empty(), + ); + + let err = assert_chain_outputs(&log, &matchers).expect_err("should fail"); + match err { + WindowError::MissingObserverPort { node, port } => { + assert_eq!(node, "node_a"); + assert_eq!(port, "missing_port"); + } + other => panic!("expected MissingObserverPort, got {:?}", other), + } + } + + #[test] + fn empty_matchers_always_passes() { + let log = make_log(vec![("some_node", vec![("out", Value::Int(42))])]); + let matchers = HashMap::new(); + assert!(assert_chain_outputs(&log, &matchers).is_ok()); + } + + #[test] + fn works_with_typed_matchers() { + let log = make_log(vec![ + ("a", vec![("flag", Value::Bool(true))]), + ("b", vec![("count", Value::Int(10))]), + ("c", vec![("name", Value::Str("test".into()))]), + ]); + + let mut matchers = HashMap::new(); + matchers.insert( + ("a".to_string(), "flag".to_string()), + OutputMatcher::IsBool, + ); + matchers.insert( + ("b".to_string(), "count".to_string()), + OutputMatcher::IntGe(5), + ); + matchers.insert( + ("c".to_string(), "name".to_string()), + OutputMatcher::IsString, + ); + + assert!(assert_chain_outputs(&log, &matchers).is_ok()); + } + + #[test] + fn int_ge_fails_below_threshold() { + let log = make_log(vec![("b", vec![("count", Value::Int(3))])]); + + let mut matchers = HashMap::new(); + matchers.insert( + ("b".to_string(), "count".to_string()), + OutputMatcher::IntGe(5), + ); + + let err = assert_chain_outputs(&log, &matchers).expect_err("should fail"); + assert!(matches!(err, WindowError::MatcherFailed { .. })); + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/gunbc-dag/src/bin/codegen_cli.rs b/gunbc-dag/src/bin/codegen_cli.rs index ee75a37dfe9..7218987eeb3 100644 --- a/gunbc-dag/src/bin/codegen_cli.rs +++ b/gunbc-dag/src/bin/codegen_cli.rs @@ -317,12 +317,32 @@ fn cmd_cigen(dry_run: bool) { // Generate CI YAML for gunbc-ci let codegen = WorkspaceBinary::Codegen.invocation(); let tool = WorkspaceBinary::Ci.invocation(); + + // Derive permissions from CI workflow integrations (checkout, GCP WIF, etc.) + let ci_perms: Vec<(String, String)> = gunbc_dag::ci::graph::ci_workflow_permissions() + .into_iter() + .map(|(scope, level)| { + ( + scope.as_yaml_key().to_string(), + level.as_yaml_value().to_string(), + ) + }) + .collect(); + + // GCP secrets required by live flow tests + let ci_secrets: Vec = gunbc_dag::ci::graph::ci_gcp_secrets() + .into_iter() + .map(|s| s.to_string()) + .collect(); + let config = RenderConfig::new("ci", tool) .with_generator(&codegen.binary, &format!("{} -- cigen", codegen.command())) .with_runner(gunbc_ir::transport::github_actions::ubuntu_latest()) .with_cargo_env(gunbc_ir::CargoEnv::ci()) .with_git(gunbc_ir::GitConfig::default()) - .with_cache(CacheConfig::rust()); + .with_cache(CacheConfig::rust()) + .with_permissions(ci_perms) + .with_secrets_env(ci_secrets); let outputs: Vec<(&str, String, String)> = vec![ ( @@ -376,6 +396,13 @@ fn generate_github_actions_template(config: &RenderConfig) -> String { format!(" - {}", b) }); + yaml_block( + &mut yaml, + "permissions:", + &config.permissions, + |(scope, level)| format!(" {}: {}", scope, level), + ); + yaml_block(&mut yaml, "env:", &config.all_env(), |(k, v)| { format!(" {}: {}", k, v) }); @@ -417,6 +444,16 @@ fn generate_github_actions_template(config: &RenderConfig) -> String { config.tool.command(), )); + if !config.secrets_env.is_empty() { + yaml.push_str(" env:\n"); + for secret in &config.secrets_env { + yaml.push_str(&format!( + " {}: ${{{{ secrets.{} }}}}\n", + secret, secret + )); + } + } + yaml } diff --git a/gunbc-dag/src/bootstrap/graph_mock.rs b/gunbc-dag/src/bootstrap/graph_mock.rs index 50200704d39..e624fa4cc22 100644 --- a/gunbc-dag/src/bootstrap/graph_mock.rs +++ b/gunbc-dag/src/bootstrap/graph_mock.rs @@ -258,6 +258,9 @@ pub fn bootstrap_mock_spec() -> MockSpec { ) .description("Generates .gitignore content from build config"), ) + // Probe-observer: transport terminals need chain-safe observers + .live_expected_output("execute_makefile_transport", "skip", OutputMatcher::IsBool) + .live_expected_output("execute_gitignore_transport", "skip", OutputMatcher::IsBool) // Primitive nodes — tested in their own crates .skip_node_example("prepare_read_makefile") .skip_node_example("prepare_write_makefile") diff --git a/gunbc-dag/src/ci/graph.rs b/gunbc-dag/src/ci/graph.rs index 9dc5f792d0d..8fdc086a835 100644 --- a/gunbc-dag/src/ci/graph.rs +++ b/gunbc-dag/src/ci/graph.rs @@ -146,6 +146,25 @@ pub fn ci_workflow_permissions() -> Permissions { ci_workflow_config().permissions } +/// GCP secret names required by live tests. +/// +/// These must be configured as GitHub repository secrets. The CI workflow +/// template passes them as environment variables so that live flow tests can +/// authenticate via Workload Identity Federation. +/// +/// Note: `ACTIONS_ID_TOKEN_REQUEST_URL` and `ACTIONS_ID_TOKEN_REQUEST_TOKEN` +/// are automatically provided by GitHub Actions when the `id-token: write` +/// permission is granted — they do not need to be set as repository secrets. +pub fn ci_gcp_secrets() -> Vec<&'static str> { + vec![ + "GCP_WIF_PROVIDER", + "GCP_SECRETS_PROJECT", + "GCP_SECRETS_PREFIX", + "GCP_SECRETS_SA", + "GCP_SECRETS_IMPERSONATE_SA", + ] +} + // ============================================================================ // Graph Builder // ============================================================================ @@ -999,6 +1018,20 @@ mod tests { perms.get(&PermissionScope::Contents), Some(&PermissionLevel::Read) ); + assert_eq!( + perms.get(&PermissionScope::IdToken), + Some(&PermissionLevel::Write) + ); + } + + #[test] + fn test_ci_gcp_secrets() { + let secrets = ci_gcp_secrets(); + assert!(secrets.contains(&"GCP_WIF_PROVIDER")); + assert!(secrets.contains(&"GCP_SECRETS_PROJECT")); + assert!(secrets.contains(&"GCP_SECRETS_PREFIX")); + assert!(secrets.contains(&"GCP_SECRETS_SA")); + assert!(secrets.contains(&"GCP_SECRETS_IMPERSONATE_SA")); } #[test] diff --git a/gunbc-dag/src/ci/graph_mock.rs b/gunbc-dag/src/ci/graph_mock.rs index 6219de62d71..c4e3be0743e 100644 --- a/gunbc-dag/src/ci/graph_mock.rs +++ b/gunbc-dag/src/ci/graph_mock.rs @@ -498,6 +498,9 @@ pub fn ci_mock_spec() -> MockSpec { .output("verify_success", OutputMatcher::exact(Value::Bool(true))) .description("Aggregate verify checks into final verify_success"), ) + // Probe-observer: report terminal needs chain-safe observer + .live_expected_output("report", "overall_success", OutputMatcher::IsBool) + .live_expected_output("report", "report", OutputMatcher::NonEmpty) // Primitive nodes — tested in their own crates .skip_node_example("prepare_deps_exists") // I/O node — transport execute, tested via DryRun diff --git a/gunbc-dag/src/ci/ops.rs b/gunbc-dag/src/ci/ops.rs index 468245f70b6..21673e85831 100644 --- a/gunbc-dag/src/ci/ops.rs +++ b/gunbc-dag/src/ci/ops.rs @@ -1021,64 +1021,74 @@ fn build_report_blocks( } ))); - // Failure details + // Failure details — all sections are truncated to keep the report readable. if !build_success && !build_stderr.is_empty() { + let summary = truncate_for_report(build_stderr); blocks.push(StructuredBlock::Raw(format!( - "\n--- Build stderr ---\n{build_stderr}\n" + "\n--- Build stderr ---\n{summary}\n" ))); } if !test_success { if let Some(failures_section) = extract_test_failures(test_stdout) { + let summary = truncate_for_report(&failures_section); blocks.push(StructuredBlock::Raw(format!( - "\n--- Test failures ---\n{failures_section}\n" + "\n--- Test failures ---\n{summary}\n" ))); } else if !test_stdout.is_empty() { + let summary = truncate_for_report(test_stdout); blocks.push(StructuredBlock::Raw(format!( - "\n--- Test stdout ---\n{test_stdout}\n" + "\n--- Test stdout ---\n{summary}\n" ))); } if !test_stderr.is_empty() { + let summary = truncate_for_report(test_stderr); blocks.push(StructuredBlock::Raw(format!( - "\n--- Test stderr ---\n{test_stderr}\n" + "\n--- Test stderr ---\n{summary}\n" ))); } } if !lint_success && !lint_stderr.is_empty() { + let summary = truncate_for_report(lint_stderr); blocks.push(StructuredBlock::Raw(format!( - "\n--- Lint stderr ---\n{lint_stderr}\n" + "\n--- Lint stderr ---\n{summary}\n" ))); } if !bootstrap_success && !bootstrap_stderr.is_empty() { + let summary = truncate_for_report(bootstrap_stderr); blocks.push(StructuredBlock::Raw(format!( - "\n--- Bootstrap stderr ---\n{bootstrap_stderr}\n" + "\n--- Bootstrap stderr ---\n{summary}\n" ))); } if !pragma_success && !pragma_stderr.is_empty() { + let summary = truncate_for_report(pragma_stderr); blocks.push(StructuredBlock::Raw(format!( - "\n--- Pragma stderr ---\n{pragma_stderr}\n" + "\n--- Pragma stderr ---\n{summary}\n" ))); } if !testgen_success && !testgen_stderr.is_empty() { + let summary = truncate_for_report(testgen_stderr); blocks.push(StructuredBlock::Raw(format!( - "\n--- Testgen stderr ---\n{testgen_stderr}\n" + "\n--- Testgen stderr ---\n{summary}\n" ))); } if !guardrail_success && !guardrail_stderr.is_empty() { + let summary = truncate_for_report(guardrail_stderr); blocks.push(StructuredBlock::Raw(format!( - "\n--- Guardrails stderr ---\n{guardrail_stderr}\n" + "\n--- Guardrails stderr ---\n{summary}\n" ))); } if !verify_success && !verify_stderr.is_empty() { + let summary = truncate_for_report(verify_stderr); blocks.push(StructuredBlock::Raw(format!( - "\n--- Verify stderr ---\n{verify_stderr}\n" + "\n--- Verify stderr ---\n{summary}\n" ))); } @@ -1091,6 +1101,51 @@ fn build_report_blocks( Ok(blocks) } +/// Maximum lines per stderr/stdout section in the CI report. +const MAX_REPORT_SECTION_LINES: usize = 60; +/// Maximum characters per line before truncation. +const MAX_REPORT_LINE_WIDTH: usize = 500; + +/// Truncate verbose output for the CI report. +/// +/// - Individual lines longer than [`MAX_REPORT_LINE_WIDTH`] are truncated +/// (catches massive linker commands with hundreds of `.rlib` paths). +/// - If the total exceeds [`MAX_REPORT_SECTION_LINES`], the middle is +/// replaced with a marker keeping the first 10 and last 50 lines. +fn truncate_for_report(text: &str) -> String { + let raw_lines: Vec<&str> = text.lines().collect(); + + // Truncate individual long lines + let lines: Vec = raw_lines + .iter() + .map(|line| { + if line.len() > MAX_REPORT_LINE_WIDTH { + format!( + "{}... ({} more chars)", + &line[..MAX_REPORT_LINE_WIDTH], + line.len() - MAX_REPORT_LINE_WIDTH + ) + } else { + (*line).to_string() + } + }) + .collect(); + + if lines.len() <= MAX_REPORT_SECTION_LINES { + return lines.join("\n"); + } + + let head = 10; + let tail = MAX_REPORT_SECTION_LINES - head; + let omitted = lines.len() - head - tail; + + let mut result = Vec::with_capacity(head + 1 + tail); + result.extend_from_slice(&lines[..head]); + result.push(format!("... ({omitted} lines omitted) ...")); + result.extend_from_slice(&lines[lines.len() - tail..]); + result.join("\n") +} + /// Extract the "failures:" section from cargo test output. /// This includes the failure list and any panic messages. fn extract_test_failures(stdout: &str) -> Option { @@ -1430,4 +1485,62 @@ mod tests { Some(false) ); } + + #[test] + fn test_truncate_for_report_short() { + let text = "line 1\nline 2\nline 3"; + assert_eq!(truncate_for_report(text), text); + } + + #[test] + fn test_truncate_for_report_long_lines() { + let long_line = "x".repeat(600); + let result = truncate_for_report(&long_line); + assert!(result.len() < long_line.len()); + assert!(result.contains("more chars)")); + } + + #[test] + fn test_truncate_for_report_many_lines() { + let lines: Vec = (0..200).map(|i| format!("line {i}")).collect(); + let text = lines.join("\n"); + let result = truncate_for_report(&text); + let result_lines: Vec<&str> = result.lines().collect(); + // Should be capped at MAX_REPORT_SECTION_LINES + 1 (truncation marker) + assert!(result_lines.len() <= MAX_REPORT_SECTION_LINES + 1); + assert!(result.contains("lines omitted")); + // First 10 lines preserved + assert!(result.contains("line 0")); + assert!(result.contains("line 9")); + // Last 50 lines preserved + assert!(result.contains("line 199")); + } + + #[test] + fn test_report_build_fail_truncates_stderr() { + let mut inputs = HashMap::new(); + inputs.insert("build_success".to_string(), Value::Bool(false)); + // Build a massive stderr with 200 lines + let lines: Vec = (0..200).map(|i| format!("error line {i}")).collect(); + inputs.insert( + "build_stderr".to_string(), + Value::Str(lines.join("\n")), + ); + inputs.insert("test_success".to_string(), Value::Bool(true)); + inputs.insert("test_stdout".to_string(), Value::Str(String::new())); + inputs.insert("test_stderr".to_string(), Value::Str(String::new())); + inputs.insert("lint_success".to_string(), Value::Bool(true)); + inputs.insert("lint_stderr".to_string(), Value::Str(String::new())); + inputs.insert("testgen_success".to_string(), Value::Bool(true)); + inputs.insert("bootstrap_success".to_string(), Value::Bool(true)); + inputs.insert("pragma_success".to_string(), Value::Bool(true)); + inputs.insert("guardrail_success".to_string(), Value::Bool(true)); + inputs.insert("verify_success".to_string(), Value::Bool(true)); + + let result = execute_report(inputs).unwrap(); + let report = result.get("report").and_then(|v| v.as_str()).unwrap(); + assert!(report.contains("lines omitted")); + // Last lines should be preserved (errors tend to be at the end) + assert!(report.contains("error line 199")); + } } diff --git a/gunbc-dag/src/credential_lifecycle.rs b/gunbc-dag/src/credential_lifecycle.rs index c76271eef82..cf0071d894a 100644 --- a/gunbc-dag/src/credential_lifecycle.rs +++ b/gunbc-dag/src/credential_lifecycle.rs @@ -158,6 +158,12 @@ pub fn github_credential_lifecycle_mock_spec() -> MockSpec { .output("ok", OutputMatcher::exact(Value::Bool(true))) .description("Parse status extracts HTTP status and ok flag"), ) + // Probe-observer: GCP sub-DAG terminal needs chain-safe observer + .live_expected_output( + "cloud_credential/gcp_wif_secret/parse_set_iam", + "ok", + OutputMatcher::IsBool, + ) .skip_node_example("cloud_env") .skip_node_example("cloud_credential") .skip_node_example("bind_secret") diff --git a/gunbc-dag/src/makegen/graph_mock.rs b/gunbc-dag/src/makegen/graph_mock.rs index e7687e60fdc..f9495325915 100644 --- a/gunbc-dag/src/makegen/graph_mock.rs +++ b/gunbc-dag/src/makegen/graph_mock.rs @@ -168,6 +168,8 @@ pub fn makegen_mock_spec() -> MockSpec { .output("makefile_content", OutputMatcher::contains("gist")) .description("Rendered Makefile contains gist target"), ) + // Probe-observer: transport terminal needs chain-safe observer + .live_expected_output("execute_makegen_transport", "skip", OutputMatcher::IsBool) // Primitive nodes — tested in their own crates .skip_node_example("prepare_read_makegen") .skip_node_example("prepare_write_makegen") diff --git a/gunbc-dag/src/makegen/registry.rs b/gunbc-dag/src/makegen/registry.rs index 7b797188750..532a18686e3 100644 --- a/gunbc-dag/src/makegen/registry.rs +++ b/gunbc-dag/src/makegen/registry.rs @@ -155,21 +155,25 @@ impl BuildConfig { .warnings(w)), build: c(CargoCommand::new(Subcommand::Build) .all_targets() + .release() .warnings(w)), - test: c(CargoCommand::new(Subcommand::Test).warnings(w)), + test: c(CargoCommand::new(Subcommand::Test).release().warnings(w)), lint: c(CargoCommand::new(Subcommand::Clippy) .all_targets() + .release() .warnings(w)), lint_fix: c(CargoCommand::new(Subcommand::Clippy) .fix() .workspace() .allow_dirty() .allow_staged() + .release() .warnings(w)), fmt: c(CargoCommand::new(Subcommand::Fmt)), fmt_check: c(CargoCommand::new(Subcommand::Fmt).check()), check: c(CargoCommand::new(Subcommand::Check) .all_targets() + .release() .warnings(w)), ci_yaml: c(CargoCommand::new(Subcommand::Run(codegen_inv.clone())) .release() diff --git a/gunbc-dag/src/pragma/graph_mock.rs b/gunbc-dag/src/pragma/graph_mock.rs index 35e0a3b8740..eabf24f417d 100644 --- a/gunbc-dag/src/pragma/graph_mock.rs +++ b/gunbc-dag/src/pragma/graph_mock.rs @@ -283,6 +283,10 @@ pub fn pragma_mock_spec() -> MockSpec { ) .description("Renders pragma lint policy"), ) + // Probe-observer: transport terminals need chain-safe observers + .live_expected_output("execute_clippy_transport", "skip", OutputMatcher::IsBool) + .live_expected_output("execute_allowlist_transport", "skip", OutputMatcher::IsBool) + .live_expected_output("execute_policy_transport", "skip", OutputMatcher::IsBool) // Primitive nodes — tested in their own crates .skip_node_example("prepare_read_clippy") .skip_node_example("prepare_write_clippy") diff --git a/gunbc-dag/src/testgen_dag/graph_mock.rs b/gunbc-dag/src/testgen_dag/graph_mock.rs index c5c728bfe93..2ab8e7f37a1 100644 --- a/gunbc-dag/src/testgen_dag/graph_mock.rs +++ b/gunbc-dag/src/testgen_dag/graph_mock.rs @@ -118,6 +118,15 @@ pub fn testgen_dag_mock_spec() -> MockSpec { .resource_lock(format!("fs:{}/generated_tests.rs", name.replace('-', "_"))); } + // Probe-observer: transport terminals need chain-safe observers + for name in &["mock-alpha", "mock-beta"] { + spec = spec.live_expected_output( + format!("execute_{}_transport", name), + "skip", + OutputMatcher::IsBool, + ); + } + spec = spec .node_example( NodeExample::new("fs_env") diff --git a/gunbc-dag/tests/mock_spec_registration.rs b/gunbc-dag/tests/mock_spec_registration.rs index 18ad9264a07..dd1168dd6e8 100644 --- a/gunbc-dag/tests/mock_spec_registration.rs +++ b/gunbc-dag/tests/mock_spec_registration.rs @@ -1,7 +1,113 @@ use gunbc_ir::resource::ResourceIo; use gunbc_lib_transport::TransportIo; +use gunbc_test::FermiCost; use std::path::Path; +/// Testgen targets with `live_required` secrets must have live_fermi_cost > S. +/// +/// The preflight test gate uses `GUNBC_TEST_MAX_COST=S` to skip expensive tests. +/// If a live test requires secrets but has cost <= S, it would run during +/// preflight and panic on missing secrets in CI (since the CI workflow does +/// not provide cloud credentials for the preflight sanity check). +#[test] +fn live_targets_with_secrets_have_cost_above_preflight_gate() { + // File-based check: scan testgen_target annotations for live_required + // and verify they also have live_fermi above "S". + let io = TransportIo::new(); + let root = Path::new(env!("CARGO_MANIFEST_DIR")) + .parent() + .expect("workspace root"); + let pattern = format!("{}/**/*.rs", root.display()); + + let mut violations = Vec::new(); + + let paths = io + .glob_paths(&pattern) + .expect("glob pattern should be valid"); + + for path in paths { + let path_str = path.to_string_lossy(); + if path_str.contains("/target/") || path_str.contains("/buck-out/") { + continue; + } + + let content = io + .read_file(&path) + .map_err(|e| format!("failed to read {}: {}", path.display(), e)) + .and_then(|bytes| String::from_utf8(bytes).map_err(|e| e.to_string())) + .unwrap_or_else(|e| panic!("failed to read {}: {}", path.display(), e)); + let lines: Vec<&str> = content.lines().collect(); + + // Find testgen_target attribute blocks with live_required. + let mut i = 0; + while i < lines.len() { + let trimmed = lines[i].trim(); + // Only match attribute annotations, not macro definitions or comments. + if trimmed.starts_with("#[") && trimmed.contains("testgen_target(") { + // Scan the attribute block for live_required and live_fermi. + let start = i; + let mut has_live_required = false; + let mut live_fermi: Option = None; + let mut is_skip = false; + + // Scan forward to find the end of the attribute + function. + let mut j = i; + while j < lines.len() { + let line = lines[j].trim(); + if line.contains("skip") && j == start { + is_skip = true; + } + if line.contains("live_required(") || line.contains("live_required_any_of(") { + has_live_required = true; + } + if let Some(fermi_str) = extract_live_fermi(line) { + live_fermi = FermiCost::parse(fermi_str); + } + // End of attribute block: next line starts with `pub fn` or `fn`. + if j > start && (line.starts_with("pub fn") || line.starts_with("fn ")) { + break; + } + j += 1; + } + + if !is_skip && has_live_required { + let cost = live_fermi.unwrap_or(FermiCost::S); + if cost <= FermiCost::S { + violations.push(format!( + "{}:{}: testgen_target has live_required secrets but live_fermi={} (must be > S for preflight gate)", + path.display(), + start + 1, + cost.as_str() + )); + } + } + + i = j + 1; + } else { + i += 1; + } + } + } + + assert!( + violations.is_empty(), + "testgen targets with live_required secrets need live_fermi > S \ + (preflight gate uses GUNBC_TEST_MAX_COST=S):\n{}", + violations.join("\n") + ); +} + +/// Extract the live_fermi value from a line like `live_fermi = "M"`. +fn extract_live_fermi(line: &str) -> Option<&str> { + let needle = "live_fermi"; + let idx = line.find(needle)?; + let rest = &line[idx + needle.len()..]; + let quote_start = rest.find('"')? + 1; + let rest2 = &rest[quote_start..]; + let quote_end = rest2.find('"')?; + Some(&rest2[..quote_end]) +} + #[test] fn all_mock_specs_are_registered() { let io = TransportIo::new(); diff --git a/lib/gcp-ops/src/graph_mock.rs b/lib/gcp-ops/src/graph_mock.rs index 254501e0f6c..b162fd89abe 100644 --- a/lib/gcp-ops/src/graph_mock.rs +++ b/lib/gcp-ops/src/graph_mock.rs @@ -286,6 +286,8 @@ pub fn gcp_local_mock_spec() -> MockSpec { .output("credential", OutputMatcher::Any) .description("Builds a bearer credential from secret material"), ) + // Probe-observer: parse_set_iam terminal needs chain-safe observer + .live_expected_output("parse_set_iam", "ok", OutputMatcher::IsBool) // Sub-DAG internal nodes — tested at their own level .skip_node_example("local_auth_upsert") .skip_node_example("should_impersonate") @@ -427,6 +429,8 @@ pub fn gcp_github_upsert_mock_spec() -> MockSpec { Value::Response(TransportResponse::Rest(add_version_response)), ) .boundary("net_env", "net", mock_net_handle()) + // Probe-observer: terminal needs chain-safe observer + .live_expected_output("parse_secret_add_version", "version", OutputMatcher::IsString) // Pure nodes — skip example enforcement for now .skip_node_example("prepare_github_oidc") .skip_node_example("parse_github_oidc") diff --git a/lib/llm-ops/src/graph_mock.rs b/lib/llm-ops/src/graph_mock.rs index 2ef8223f7d3..902eb10f50e 100644 --- a/lib/llm-ops/src/graph_mock.rs +++ b/lib/llm-ops/src/graph_mock.rs @@ -73,6 +73,12 @@ fn with_cloud_env(spec: MockSpec) -> MockSpec { "cloud_credential/gcp_wif_secret", &gunbc_lib_gcp_ops::graph_mock::gcp_local_mock_spec(), ) + // Probe-observer: GCP sub-DAG terminal needs chain-safe observer + .live_expected_output( + "cloud_credential/gcp_wif_secret/parse_set_iam", + "ok", + OutputMatcher::IsBool, + ) } /// Mock specification for OpenAI chat completion. diff --git a/lib/review/src/graph_mock.rs b/lib/review/src/graph_mock.rs index acb3aad85ed..1f278c0a407 100644 --- a/lib/review/src/graph_mock.rs +++ b/lib/review/src/graph_mock.rs @@ -267,6 +267,12 @@ pub fn inline_review_mock_spec() -> MockSpec { .output("errors", OutputMatcher::Any) .description("parses LLM answer into review output"), ) + // Probe-observer: GCP sub-DAG terminal needs chain-safe observer + .live_expected_output( + "cloud_credential/gcp_wif_secret/parse_set_iam", + "ok", + OutputMatcher::IsBool, + ) .skip_node_example("cloud_env") .skip_node_example("cloud_credential") .skip_node_example("fs_env") @@ -469,6 +475,12 @@ diff --git a/src/main.rs b/src/main.rs .output("errors", OutputMatcher::Any) .description("parses LLM answer into review output"), ) + // Probe-observer: GCP sub-DAG terminal needs chain-safe observer + .live_expected_output( + "cloud_credential/gcp_wif_secret/parse_set_iam", + "ok", + OutputMatcher::IsBool, + ) .skip_node_example("cloud_env") .skip_node_example("cloud_credential") .skip_node_example("fs_env") diff --git a/lib/tools/clippy/src/graph_mock.rs b/lib/tools/clippy/src/graph_mock.rs index b819a1f4e79..b3f0b965efc 100644 --- a/lib/tools/clippy/src/graph_mock.rs +++ b/lib/tools/clippy/src/graph_mock.rs @@ -8,7 +8,7 @@ use gunbc_ir::transport::{ShellResponse, TransportResponse}; use gunbc_ir::Value; -use gunbc_test::{InputConstraint, MockSpec}; +use gunbc_test::{InputConstraint, MockSpec, OutputMatcher}; /// Mock specification for the clippy DAG. /// @@ -72,4 +72,10 @@ pub fn clippy_mock_spec() -> MockSpec { .skip_node_example("prepare_resolve") .skip_node_example("execute_resolve") .skip_node_example("parse_resolve") + // Observer: parse_resolve is the terminal node — needs an OutputMatcher + // so the observability invariant (every terminal reachable from a probe + // must have an observer) is satisfied. Uses live_expected_output rather + // than NodeExample because the op's output keys (success, exit_code, ...) + // don't match the DAG port name (result), preventing standalone execution. + .live_expected_output("parse_resolve", "result", OutputMatcher::NonEmpty) } diff --git a/lib/tools/deps/src/graph_mock.rs b/lib/tools/deps/src/graph_mock.rs index f7b9446052f..cac2db19271 100644 --- a/lib/tools/deps/src/graph_mock.rs +++ b/lib/tools/deps/src/graph_mock.rs @@ -240,6 +240,8 @@ pub fn deps_mock_spec() -> MockSpec { .output("stderr", OutputMatcher::exact(Value::Str("".into()))) .description("Parses install execution result"), ) + // Probe-observer: terminal needs chain-safe observer + .live_expected_output("parse_execute_result", "success", OutputMatcher::IsBool) } /// Mock spec for testing sudo elevation scenarios. diff --git a/lib/tools/gist/src/graph_mock.rs b/lib/tools/gist/src/graph_mock.rs index a7753dec19e..830c55db5a7 100644 --- a/lib/tools/gist/src/graph_mock.rs +++ b/lib/tools/gist/src/graph_mock.rs @@ -350,6 +350,13 @@ fn gist_mock_spec(mode: &GistMode) -> MockSpec { .output("url", OutputMatcher::contains("gist.github.com")) .description("Extracts gist URL from response JSON"), ) + // Probe-observer: terminals need chain-safe observers + .live_expected_output("parse_gist_response", "url", OutputMatcher::NonEmpty) + .live_expected_output( + "cloud_credential/gcp_wif_secret/parse_set_iam", + "ok", + OutputMatcher::IsBool, + ) .skip_node_example("cloud_env") .skip_node_example("cloud_credential") .skip_node_example("bind_secret") diff --git a/lib/transport/src/preflight.rs b/lib/transport/src/preflight.rs index 1ea1a129cef..22120b3f71d 100644 --- a/lib/transport/src/preflight.rs +++ b/lib/transport/src/preflight.rs @@ -340,8 +340,10 @@ fn run_lint_upsert(resource_id: &ResourceId) -> Result<(), ResourceError> { return Err(ResourceError::CreateFailed( resource_id.clone(), format!( - "cargo clippy failed after fix (exit {})\n{}", - verify_result.exit_code, verify_result.stderr + "cargo clippy failed after fix (exit {})\n{}\n{}", + verify_result.exit_code, + verify_result.stdout, + verify_result.stderr ), )); } @@ -351,8 +353,9 @@ fn run_lint_upsert(resource_id: &ResourceId) -> Result<(), ResourceError> { } // Test gate: run lib tests to catch contract mismatches (e.g., wrong - // secret names, stale generated tests). Integration tests are skipped - // for speed — CI covers those. + // secret names, stale generated tests). Only hermetic tests run here — + // live/integration tests that require secrets or external services are + // skipped via GUNBC_TEST_MAX_COST=S. The full CI pipeline covers those. eprint!(" [{}/{}] test --lib...", total, total); let _ = std::io::Write::flush(&mut std::io::stderr()); let test_start = std::time::Instant::now(); @@ -360,7 +363,7 @@ fn run_lint_upsert(resource_id: &ResourceId) -> Result<(), ResourceError> { .workspace() .lib_only() .warnings(Warnings::Deny); - run_cargo_command(resource_id, &test_cmd)?; + run_cargo_command_with_env(resource_id, &test_cmd, &[("GUNBC_TEST_MAX_COST", "S")])?; eprintln!(" {:.1}s", test_start.elapsed().as_secs_f64()); Ok(()) @@ -368,17 +371,39 @@ fn run_lint_upsert(resource_id: &ResourceId) -> Result<(), ResourceError> { /// Run a cargo command, failing on non-zero exit. fn run_cargo_command(resource_id: &ResourceId, cmd: &CargoCommand) -> Result<(), ResourceError> { - let response = run_cargo_command_response(resource_id, cmd)?; + run_cargo_command_with_env(resource_id, cmd, &[]) +} + +/// Run a cargo command with extra environment variables, failing on non-zero exit. +fn run_cargo_command_with_env( + resource_id: &ResourceId, + cmd: &CargoCommand, + extra_env: &[(&str, &str)], +) -> Result<(), ResourceError> { + let response = run_cargo_command_response_with_env(resource_id, cmd, extra_env)?; if response.success() { Ok(()) } else { + // Include both stdout and stderr — cargo test writes test failure + // details (which tests failed, panic messages) to stdout, while + // stderr only gets the terse "error: test failed" line. + let mut detail = String::new(); + if !response.stdout.is_empty() { + detail.push_str(&response.stdout); + } + if !response.stderr.is_empty() { + if !detail.is_empty() { + detail.push('\n'); + } + detail.push_str(&response.stderr); + } Err(ResourceError::CreateFailed( resource_id.clone(), format!( "command failed (exit {}): {}\n{}", response.exit_code, cmd.to_shell(), - response.stderr + detail ), )) } @@ -388,12 +413,24 @@ fn run_cargo_command(resource_id: &ResourceId, cmd: &CargoCommand) -> Result<(), fn run_cargo_command_response( resource_id: &ResourceId, cmd: &CargoCommand, +) -> Result { + run_cargo_command_response_with_env(resource_id, cmd, &[]) +} + +/// Run a cargo command with extra environment variables and return the response. +fn run_cargo_command_response_with_env( + resource_id: &ResourceId, + cmd: &CargoCommand, + extra_env: &[(&str, &str)], ) -> Result { // Preflight steps compile + run cargo binaries; in CI with cold caches // this can take well over 5 minutes. Use FermiCost::L (30 min). const PREFLIGHT_TIMEOUT_MS: u64 = 1_800_000; // FermiCost::L = 30 min - let request = cmd.to_shell_request().timeout(PREFLIGHT_TIMEOUT_MS); + let mut request = cmd.to_shell_request().timeout(PREFLIGHT_TIMEOUT_MS); + for (k, v) in extra_env { + request = request.env(*k, *v); + } let response = execute_request(&TransportRequest::Shell(request)) .map_err(|e| ResourceError::CreateFailed(resource_id.clone(), e.to_string()))?;