From 61af4518e9ec54343c65a537284460e50512b10a Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Fri, 21 Aug 2026 19:38:07 +0000 Subject: [PATCH 01/19] =?UTF-8?q?ci(tests):=20full-signal=20reds=20?= =?UTF-8?q?=E2=80=94=20--no-fail-fast,=20per-test=20JUnit=20roll-up=20(gat?= =?UTF-8?q?e-audit=20R1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every plain cargo-test shape reports all failures in one run; the group loop runs every suite before exiting non-zero. The required aggregator now renders actual failing test names (from nextest's JUnit output, wired by Tasks 3-5) instead of a job-status table the Checks UI already shows. Leaves the coverage arm (:404) and the two crate-tests cargo-test arms (:417-419, :427-428, converted to nextest in Task 4) untouched. Co-Authored-By: Claude Fable 5 --- .github/workflows/reborn-tests.yml | 38 +++++++++++- scripts/ci/junit_summary.py | 86 ++++++++++++++++++++++++++ scripts/ci/run-reborn-group-tests.sh | 19 +++++- scripts/ci/test_junit_summary.py | 75 ++++++++++++++++++++++ scripts/ci/test_reborn_pr_test_plan.py | 2 +- 5 files changed, 214 insertions(+), 6 deletions(-) create mode 100644 scripts/ci/junit_summary.py create mode 100644 scripts/ci/test_junit_summary.py diff --git a/.github/workflows/reborn-tests.yml b/.github/workflows/reborn-tests.yml index f13caf56b1a..1e74b1ae948 100644 --- a/.github/workflows/reborn-tests.yml +++ b/.github/workflows/reborn-tests.yml @@ -666,16 +666,16 @@ jobs: docker build -f Dockerfile.sandbox-worker -t ironclaw-worker:latest . - name: Verify production local-sandbox profile wiring - run: cargo test -p ironclaw local_sandbox_profile_selects_docker_process_binding_when_required -- --nocapture + run: cargo test -p ironclaw local_sandbox_profile_selects_docker_process_binding_when_required --no-fail-fast -- --nocapture - name: Run real-Docker user sandbox contract # Each test creates a transport that reconciles the daemon-wide user-container # inventory. Run this shared-Docker contract serially so one test's sweeper # cannot adopt and stop another test's intentionally active container. - run: cargo test -p ironclaw_sandbox --test user_sandbox_docker_live -- --nocapture --test-threads=1 + run: cargo test -p ironclaw_sandbox --test user_sandbox_docker_live --no-fail-fast -- --nocapture --test-threads=1 - name: Run full-turn sandbox shell integration - run: cargo test -p ironclaw_integration_tests --test reborn_integration_sandbox_shell_turn -- --nocapture + run: cargo test -p ironclaw_integration_tests --test reborn_integration_sandbox_shell_turn --no-fail-fast -- --nocapture reborn-integration-coverage: name: Reborn integration tests (${{ matrix.lane }}) @@ -1135,6 +1135,7 @@ jobs: cargo test \ -p ironclaw_integration_tests \ --test reborn_qa_recorded_behavior \ + --no-fail-fast \ -- \ --nocapture @@ -1265,3 +1266,34 @@ jobs: if [[ "${plan_mode}" == "none" ]]; then echo "No affected Reborn test surface — baseline recorded replay ran as planned" fi + + - name: Download JUnit reports + if: always() + continue-on-error: true + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + pattern: reborn-junit-* + path: junit-reports + merge-multiple: true + + - name: Roll up per-job results and per-test failures + if: always() + env: + NEEDS_JSON: ${{ toJSON(needs) }} + run: | + { + echo "### Tests (Reborn) — job roll-up" + echo "" + echo "Non-nextest jobs (group suites, sandbox Docker, QA replay have no" + echo "per-test report; see their own job logs for failures):" + echo "" + echo "| Job | Result |" + echo "|---|---|" + jq -r 'to_entries[] | select(.key == "reborn-group-tests" or .key == "sandbox-docker-tests" or .key == "qa-recorded-fixtures") | "| \(.key) | \(.value.result) |"' <<< "${NEEDS_JSON}" + if compgen -G "junit-reports/*.xml" > /dev/null; then + python3 scripts/ci/junit_summary.py junit-reports/*.xml + else + echo "" + echo "_No JUnit reports were downloaded (nextest jobs may not have run yet, or all passed with no artifact needed)._" + fi + } >> "$GITHUB_STEP_SUMMARY" diff --git a/scripts/ci/junit_summary.py b/scripts/ci/junit_summary.py new file mode 100644 index 00000000000..86a8b2608a5 --- /dev/null +++ b/scripts/ci/junit_summary.py @@ -0,0 +1,86 @@ +#!/usr/bin/env python3 +"""Summarize nextest JUnit reports into a Markdown per-test failure table. + +Consumes one or more JUnit XML files (nextest's `[profile.ci.junit]` +output, one per nextest invocation) and prints a Markdown table of every +failed or errored : binary (classname), test name, and the first +line of the failure/error message. Passing tests are not listed -- +GitHub's own Checks UI already shows per-job pass/fail; this table exists +so a red run's *test* names are visible without opening every job log. + +Usage: python3 scripts/ci/junit_summary.py [ ...] +Exit status is always 0 -- this is a reporting step, never a gate; the +job's own test command already failed the step if there were failures. +""" +from __future__ import annotations + +import sys +import xml.etree.ElementTree as ET +from dataclasses import dataclass + + +@dataclass(frozen=True) +class FailedTest: + classname: str + name: str + kind: str # "failure" or "error" + message: str + + +def _first_line(text: str | None) -> str: + if not text: + return "(no message)" + stripped = text.strip() + if not stripped: + return "(no message)" + return stripped.splitlines()[0][:200] + + +def parse_junit(path: str) -> list[FailedTest]: + tree = ET.parse(path) + root = tree.getroot() + testsuites = [root] if root.tag == "testsuite" else list(root.iter("testsuite")) + failed: list[FailedTest] = [] + for suite in testsuites: + for case in suite.iter("testcase"): + for kind in ("failure", "error"): + node = case.find(kind) + if node is not None: + failed.append( + FailedTest( + classname=case.get("classname", suite.get("name", "?")), + name=case.get("name", "?"), + kind=kind, + message=_first_line(node.get("message") or node.text), + ) + ) + break + return failed + + +def render_markdown(failures: list[FailedTest]) -> str: + if not failures: + return "" + lines = ["### Failed tests", "", "| Binary | Test | Message |", "|---|---|---|"] + for failure in sorted(failures, key=lambda f: (f.classname, f.name)): + message = failure.message.replace("|", "\\|") + lines.append(f"| `{failure.classname}` | `{failure.name}` | {message} |") + return "\n".join(lines) + "\n" + + +def main(argv: list[str]) -> int: + if not argv: + print("usage: junit_summary.py [ ...]", file=sys.stderr) + return 2 + failures: list[FailedTest] = [] + for path in argv: + try: + failures.extend(parse_junit(path)) + except (ET.ParseError, FileNotFoundError) as exc: + print(f"::warning::could not parse JUnit report {path}: {exc}", file=sys.stderr) + print(render_markdown(failures)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv[1:])) diff --git a/scripts/ci/run-reborn-group-tests.sh b/scripts/ci/run-reborn-group-tests.sh index 234eb95c219..c77e87c8480 100755 --- a/scripts/ci/run-reborn-group-tests.sh +++ b/scripts/ci/run-reborn-group-tests.sh @@ -32,9 +32,24 @@ if [ "${#test_names[@]}" -eq 0 ]; then exit 1 fi +# Sequential loop retained (Decision 3, T2 plan): every reborn_group_* +# binary shares one libsql-backed group store; PR #5751 traced a real +# SIGABRT to concurrent access against that shape, and the fix required a +# 90-run soak (50 libsql + 40 in-memory, 0 flakes) to trust +# (tests/integration/group_approvals/main.rs:9-12). Each binary already +# runs its own scenarios sequentially by construction; this job's whole +# purpose is keeping DIFFERENT group binaries from running concurrently +# against each other, which is exactly what a nextest pool would do. +# +# Full-signal (R1): run every group binary before exiting non-zero. +failed=0 for test_name in "${test_names[@]}"; do echo "::group::cargo test --test ${test_name}" - timeout --signal=INT --kill-after=30s "${test_timeout}" \ - cargo test -p ironclaw_integration_tests --test "${test_name}" -- --nocapture + if ! timeout --signal=INT --kill-after=30s "${test_timeout}" \ + cargo test -p ironclaw_integration_tests --test "${test_name}" --no-fail-fast -- --nocapture; then + echo "::error::group suite failed: ${test_name}" + failed=1 + fi echo "::endgroup::" done +exit "${failed}" diff --git a/scripts/ci/test_junit_summary.py b/scripts/ci/test_junit_summary.py new file mode 100644 index 00000000000..c259fad5513 --- /dev/null +++ b/scripts/ci/test_junit_summary.py @@ -0,0 +1,75 @@ +#!/usr/bin/env python3 +"""Regression tests for scripts/ci/junit_summary.py.""" +from __future__ import annotations + +import contextlib +import io +import pathlib +import sys +import tempfile +import unittest + +sys.path.insert(0, str(pathlib.Path(__file__).parent)) +import junit_summary # noqa: E402 + +PASSING_XML = """ + + + + + +""" + +FAILING_XML = """ + + + + + panic at tests/reborn_scope_isolation_suite/reborn_project_scope_isolation_parity.rs:88 + + + + + SIGABRT + + + +""" + + +class JunitSummaryTest(unittest.TestCase): + def _write(self, content: str) -> str: + handle = tempfile.NamedTemporaryFile(mode="w", suffix=".xml", delete=False) + handle.write(content) + handle.close() + return handle.name + + def test_all_passing_yields_empty_report(self) -> None: + path = self._write(PASSING_XML) + failures = junit_summary.parse_junit(path) + self.assertEqual(failures, []) + self.assertEqual(junit_summary.render_markdown(failures), "") + + def test_failures_and_errors_both_render(self) -> None: + path = self._write(FAILING_XML) + failures = junit_summary.parse_junit(path) + self.assertEqual(len(failures), 2) + self.assertEqual({f.kind for f in failures}, {"failure", "error"}) + markdown = junit_summary.render_markdown(failures) + self.assertIn("reborn_scope_isolation_suite", markdown) + self.assertIn("project_scope_leaks_across_tenant", markdown) + self.assertIn("reborn_group_approvals", markdown) + self.assertIn("concurrent_dual_gate_resume", markdown) + + def test_unparseable_file_warns_and_continues(self) -> None: + bad = self._write("not xml") + good = self._write(FAILING_XML) + buf = io.StringIO() + with contextlib.redirect_stderr(buf): + status = junit_summary.main([bad, good]) + self.assertEqual(status, 0) + self.assertIn("::warning::", buf.getvalue()) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/ci/test_reborn_pr_test_plan.py b/scripts/ci/test_reborn_pr_test_plan.py index 63e87c5c949..7247900af77 100644 --- a/scripts/ci/test_reborn_pr_test_plan.py +++ b/scripts/ci/test_reborn_pr_test_plan.py @@ -2075,7 +2075,7 @@ def test_workflow_consumes_plan_and_bounds_each_rust_matrix(self) -> None: self.assertIn("needs.changes.outputs.run_sandbox_docker", workflow) self.assertIn( "cargo test -p ironclaw_sandbox --test user_sandbox_docker_live " - "-- --nocapture --test-threads=1", + "--no-fail-fast -- --nocapture --test-threads=1", workflow, ) self.assertIn("--test reborn_integration_sandbox_shell_turn", workflow) From 30feabfa1cb5a038d8a0dcffab8ad85280576c2c Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Fri, 21 Aug 2026 19:38:42 +0000 Subject: [PATCH 02/19] ci(tests): raise PR max-parallel 3->6 (crate buckets), 1->2 (root, integration) Worker-stress constraint keeps this well under the queue's 14/4/5. No new in-workflow gate: the changes planner already fronts every job, and a serial lint gate would cost every PR ~2 min to save minutes only on trivially-red pushes. Co-Authored-By: Claude Fable 5 --- .github/workflows/reborn-tests.yml | 6 +++--- scripts/ci/test_reborn_pr_test_plan.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/reborn-tests.yml b/.github/workflows/reborn-tests.yml index 1e74b1ae948..54029524c11 100644 --- a/.github/workflows/reborn-tests.yml +++ b/.github/workflows/reborn-tests.yml @@ -222,7 +222,7 @@ jobs: RUST_MIN_STACK: "8388608" strategy: fail-fast: false - max-parallel: ${{ github.event_name == 'pull_request' && 3 || 14 }} + max-parallel: ${{ github.event_name == 'pull_request' && 6 || 14 }} matrix: bucket: ${{ fromJSON(needs.changes.outputs.crate_buckets) }} steps: @@ -462,7 +462,7 @@ jobs: RUST_MIN_STACK: "67108864" strategy: fail-fast: false - max-parallel: ${{ github.event_name == 'pull_request' && 1 || 4 }} + max-parallel: ${{ github.event_name == 'pull_request' && 2 || 4 }} matrix: partition: ${{ fromJSON(needs.changes.outputs.root_partitions) }} steps: @@ -708,7 +708,7 @@ jobs: RUST_MIN_STACK: "8388608" strategy: fail-fast: false - max-parallel: ${{ github.event_name == 'pull_request' && 1 || 5 }} + max-parallel: ${{ github.event_name == 'pull_request' && 2 || 5 }} matrix: lane: ${{ fromJSON(needs.changes.outputs.integration_lanes) }} steps: diff --git a/scripts/ci/test_reborn_pr_test_plan.py b/scripts/ci/test_reborn_pr_test_plan.py index 7247900af77..6bf03df2cc6 100644 --- a/scripts/ci/test_reborn_pr_test_plan.py +++ b/scripts/ci/test_reborn_pr_test_plan.py @@ -2090,15 +2090,15 @@ def test_workflow_consumes_plan_and_bounds_each_rust_matrix(self) -> None: ) self.assertNotIn('coverage/${package}.lcov', workflow) self.assertIn( - "max-parallel: ${{ github.event_name == 'pull_request' && 3 || 14 }}", + "max-parallel: ${{ github.event_name == 'pull_request' && 6 || 14 }}", workflow, ) self.assertIn( - "max-parallel: ${{ github.event_name == 'pull_request' && 1 || 4 }}", + "max-parallel: ${{ github.event_name == 'pull_request' && 2 || 4 }}", workflow, ) self.assertIn( - "max-parallel: ${{ github.event_name == 'pull_request' && 1 || 5 }}", + "max-parallel: ${{ github.event_name == 'pull_request' && 2 || 5 }}", workflow, ) self.assertIn("github.event.merge_group.base_sha", workflow) From 93c5e4a27e1585d16a45ce99ece6e0128766844b Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Sat, 22 Aug 2026 00:00:54 +0000 Subject: [PATCH 03/19] ci(tests): one runner-selection seam, real nextest hermetic proof, root partitions on nextest scripts/ci/lib/select-test-runner.sh replaces quality_gate.sh's inline copy and is the ONE thing every CI runner script sources (policy require-in-ci) -- no second implementation. IRONCLAW_GATE_TEST_RUNNER now crosses the hermetic wrapper's env allowlist. test-hermetic-test-process.sh gets a real negative control: a nextest-run test that must be blocked by the network guard when active and must NOT be blocked when sabotaged, proving the guard applies to nextest's own child-process spawn path (the prior file-scanning self-test never invoked cargo or nextest at all). Root partitions swap to cargo-nextest --profile ci: behavior-identical (equivalence proof in PR body), same tests, same partitioning. Empirically, scripts/ci/hermetic-network-runner.sh forces its own exit code to 86 whenever any non-loopback attempt is logged, even one the guard's interposer correctly blocked with EPERM -- so the negative control's pass/fail signal is the test's own reported outcome in nextest's captured output ("... ok" vs a PermissionDenied-mismatch panic), not the wrapper's process exit code. Co-Authored-By: Claude Fable 5 --- .config/nextest.toml | 34 +++++++++++-- .github/workflows/code_style.yml | 4 ++ .github/workflows/reborn-tests.yml | 15 +++++- scripts/ci/lib/select-test-runner.sh | 61 +++++++++++++++++++++++ scripts/ci/quality_gate.sh | 50 ++----------------- scripts/ci/run-hermetic-test-process.sh | 2 +- scripts/ci/run-reborn-root-partition.sh | 31 ++++++++---- scripts/ci/test-hermetic-test-process.sh | 60 +++++++++++++++++++++++ scripts/ci/test-quality-gate-runner.sh | 62 ++++++++++++++++++++++++ tests/hermetic_network_guard_probe.rs | 53 ++++++++++++++++++++ 10 files changed, 312 insertions(+), 60 deletions(-) create mode 100644 scripts/ci/lib/select-test-runner.sh create mode 100644 tests/hermetic_network_guard_probe.rs diff --git a/.config/nextest.toml b/.config/nextest.toml index 33d898ba8a5..564e9fd6663 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -18,12 +18,30 @@ leak-timeout = "100ms" [profile.ci] # One flake shouldn't mask other real failures. fail-fast = false +# Determinism over green-washing: no automatic retries in CI. +retries = 0 # Inline failures, suppress passing noise, keep Actions logs scannable. failure-output = "immediate-final" success-output = "never" -# Stricter CI timeout: a hung test should crash the job fast, not sit -# against the workflow-level 25-minute cap. -slow-timeout = { period = "60s", terminate-after = 3 } +# Per-test hang ceiling: warn every 60s, kill after 10 missed warnings +# (10m). Replaces the old per-BINARY 28m `timeout` as the hang backstop, +# catching a stuck test at test granularity instead of job granularity +# (the 91-122min queue hangs the gate-audit measured were bounded only by +# a 90m job timeout). +slow-timeout = { period = "60s", terminate-after = 10 } +# 2-core/~7GB runners: bound concurrent test PROCESSES to the actual core +# count. nextest's built-in default already resolves to logical-CPU +# count, but leaving it implicit makes "RUST_MIN_STACK=64MiB (job env) x +# concurrent per-test tokio runtimes" an unreviewable number. Pin it. +test-threads = "num-cpus" + +[profile.ci.junit] +# One JUnit report per nextest invocation. Job steps copy it out before +# the next invocation in the same step would overwrite it (crate-tests' +# exact-targets loop runs nextest more than once per step) -- see Tasks +# 3-5's recipes. Consumed by scripts/ci/junit_summary.py in the required +# aggregator (Task 1) to report failing TEST names, not just job names. +path = "junit.xml" # Per-test windows for the handful of scenarios that genuinely need more # than 60s (live-recorded fixtures + heavy integration). @@ -34,3 +52,13 @@ slow-timeout = { period = "300s", terminate-after = 2 } [[profile.ci.overrides]] filter = "test(~e2e_thread_scheduling) | test(~heavy_integration)" slow-timeout = { period = "300s", terminate-after = 2 } + +# Root/integration-tier suites drive whole Reborn turns; give them a +# 20-minute per-test ceiling (still far under the old 28m per-binary cap). +# Must stay BELOW the test-name override above: nextest takes the first +# matching [[profile.ci.overrides]] block, and a package-level filter +# would otherwise shadow the two named-test overrides for suites inside +# ironclaw_integration_tests. +[[profile.ci.overrides]] +filter = "package(ironclaw_integration_tests)" +slow-timeout = { period = "120s", terminate-after = 10 } diff --git a/.github/workflows/code_style.yml b/.github/workflows/code_style.yml index d9d01602878..12d65f83d5f 100644 --- a/.github/workflows/code_style.yml +++ b/.github/workflows/code_style.yml @@ -165,6 +165,10 @@ jobs: uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable with: components: rustfmt + - name: Install cargo-nextest + uses: taiki-e/install-action@62b0f2dec647a8e604c6a0fda0e38530180dce20 # v2 + with: + tool: cargo-nextest@0.9.143 - name: Install Python uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: diff --git a/.github/workflows/reborn-tests.yml b/.github/workflows/reborn-tests.yml index 54029524c11..2252b526774 100644 --- a/.github/workflows/reborn-tests.yml +++ b/.github/workflows/reborn-tests.yml @@ -452,7 +452,7 @@ jobs: OPENAI_API_KEY: "" REBORN_ROOT_TEST_PARTITIONS: 4 REBORN_ROOT_TEST_PARTITION: ${{ matrix.partition }} - REBORN_ROOT_TEST_TIMEOUT: 28m + REBORN_ROOT_TEST_TIMEOUT: 40m CARGO_INCREMENTAL: "0" # The QA smoke binaries build a full Reborn runtime and drive whole # turns on the libtest current-thread stack; the combined debug async @@ -483,6 +483,11 @@ jobs: - name: Install Rust uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + - name: Install cargo-nextest + uses: taiki-e/install-action@62b0f2dec647a8e604c6a0fda0e38530180dce20 # v2 + with: + tool: cargo-nextest@0.9.143 + - name: Install mold and clang run: | scripts/ci/install-ci-apt-packages.sh clang mold @@ -533,6 +538,14 @@ jobs: command scripts/ci/run-reborn-root-partition.sh + - name: Upload JUnit report + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: reborn-junit-root-${{ matrix.partition }} + path: target/nextest/ci/junit.xml + if-no-files-found: ignore + reborn-group-tests: name: Reborn group tests needs: changes diff --git a/scripts/ci/lib/select-test-runner.sh b/scripts/ci/lib/select-test-runner.sh new file mode 100644 index 00000000000..45622e4fd6e --- /dev/null +++ b/scripts/ci/lib/select-test-runner.sh @@ -0,0 +1,61 @@ +#!/usr/bin/env bash +# +# Shared by scripts/ci/quality_gate.sh (local dev) and every CI-facing +# runner script (scripts/ci/run-reborn-root-partition.sh, +# scripts/ci/reborn-coverage-lane-run.sh, and +# scripts/ci/run-hermetic-deterministic-suite.sh's run_crate_tests / +# run_integration_tier). ONE selection seam — a prior draft of this plan +# proposed a second, parallel implementation for CI; that was a fork, not +# an extension, and is not what this file is. +# +# select_test_runner +# policy = optional -- local contract: nextest when installed, +# cargo test otherwise, regardless of CI. +# policy = require-in-ci -- CI runner scripts' contract: nextest when +# installed; when absent AND CI=true, a hard, +# loud failure (the job installed nextest, so +# absence means the job is broken -- never a +# silent 3x-slower fallback); when absent and +# CI is unset (local reproduction), the same +# warn + fallback as "optional". +# IRONCLAW_GATE_TEST_RUNNER (both policies): cargo | nextest | auto +# (default). An explicit "nextest" always hard-fails if the binary is +# missing, in EITHER policy. An explicit "cargo" always forces the +# sequential runner -- the bisect knob: force cargo even inside CI to +# isolate a nextest-only failure. +select_test_runner() { + local policy="${1:?select_test_runner requires a policy: optional|require-in-ci}" + case "${policy}" in + optional|require-in-ci) ;; + *) + echo "select_test_runner: unknown policy '${policy}' (expected optional|require-in-ci)" >&2 + return 1 + ;; + esac + + case "${IRONCLAW_GATE_TEST_RUNNER:-auto}" in + cargo) echo "cargo" ;; + nextest) + if command -v cargo-nextest >/dev/null 2>&1; then + echo "nextest" + else + echo "IRONCLAW_GATE_TEST_RUNNER=nextest requires cargo-nextest" >&2 + return 1 + fi + ;; + auto) + if command -v cargo-nextest >/dev/null 2>&1; then + echo "nextest" + elif [[ "${policy}" == "require-in-ci" && "${CI:-}" == "true" ]]; then + echo "cargo-nextest is required in CI but was not found on PATH" >&2 + return 1 + else + echo "cargo" + fi + ;; + *) + echo "unknown IRONCLAW_GATE_TEST_RUNNER: ${IRONCLAW_GATE_TEST_RUNNER}" >&2 + return 1 + ;; + esac +} diff --git a/scripts/ci/quality_gate.sh b/scripts/ci/quality_gate.sh index 40d39ff6ddd..80262601547 100755 --- a/scripts/ci/quality_gate.sh +++ b/scripts/ci/quality_gate.sh @@ -24,54 +24,12 @@ run_cargo_ci() { echo "==> clippy (CI parity: all features, all warnings)" run_cargo_ci cargo clippy --locked --all --tests --examples --all-features -- -D warnings -# Which runner executes the workspace tests. -# -# `cargo test` runs each test binary strictly sequentially — it parallelises -# *within* a binary but never across them. This workspace builds well over 400 -# test binaries, the large majority finishing in under a second, so the gate -# spends most of its wall clock starting and tearing down processes one at a -# time. `cargo nextest` runs them in a single parallel pool instead. -# -# Coverage is equivalent: `--all-targets` expands to -# `--lib --bins --tests --benches --examples` and so has never included -# doctests, which nextest also does not run. This swap therefore changes -# scheduling only, not what is executed. -# -# nextest stays OPTIONAL. `tests/fixtures/llm_traces/README.md` documents that -# local development must keep working without it, so an absent binary falls -# back rather than failing. `.config/nextest.toml` already carries the -# repository's profiles and per-test slow-timeouts; CI's insta gate uses them. -# -# auto (default) — nextest when installed, otherwise cargo test -# nextest — require nextest, fail if missing -# cargo — force the sequential runner -select_test_runner() { - case "${IRONCLAW_GATE_TEST_RUNNER:-auto}" in - cargo) echo "cargo" ;; - nextest) - if command -v cargo-nextest >/dev/null 2>&1; then - echo "nextest" - else - echo "IRONCLAW_GATE_TEST_RUNNER=nextest requires cargo-nextest" >&2 - return 1 - fi - ;; - auto) - if command -v cargo-nextest >/dev/null 2>&1; then - echo "nextest" - else - echo "cargo" - fi - ;; - *) - echo "unknown IRONCLAW_GATE_TEST_RUNNER: ${IRONCLAW_GATE_TEST_RUNNER}" >&2 - return 1 - ;; - esac -} +# See scripts/ci/lib/select-test-runner.sh for the runner-selection +# rationale and contract (shared with every CI runner script). +source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib/select-test-runner.sh" if [ "${IRONCLAW_PREPUSH_TEST:-1}" = "1" ]; then - runner="$(select_test_runner)" + runner="$(select_test_runner optional)" if [ "$runner" = "nextest" ]; then echo "==> tests (nextest: workspace, all targets, all features; skip with IRONCLAW_PREPUSH_TEST=0)" run_cargo_ci cargo nextest run --locked --workspace --all-targets --all-features --no-fail-fast diff --git a/scripts/ci/run-hermetic-test-process.sh b/scripts/ci/run-hermetic-test-process.sh index 981a71bca9f..029fa66c8ae 100755 --- a/scripts/ci/run-hermetic-test-process.sh +++ b/scripts/ci/run-hermetic-test-process.sh @@ -62,7 +62,7 @@ if [[ "${sabotage}" != "env" ]]; then REBORN_COV_LANE_PARTITIONS|REBORN_COV_LANE_TEST_TIMEOUT|\ REBORN_GROUP_TEST_TIMEOUT|REBORN_ROOT_TEST_PARTITION|\ REBORN_ROOT_TEST_PARTITIONS|REBORN_ROOT_TEST_TIMEOUT|\ - IRONCLAW_E2E_EMULATE_SLACK_CHANNEL_BEARER|IRONCLAW_EMULATE_CLI|\ + IRONCLAW_E2E_EMULATE_SLACK_CHANNEL_BEARER|IRONCLAW_EMULATE_CLI|IRONCLAW_GATE_TEST_RUNNER|\ IRONCLAW_GENERATED_SEQUENCE_DEPTH|IRONCLAW_JOURNEY_ORDER|\ IRONCLAW_PROVIDER_OPERATION_SHARD) ;; diff --git a/scripts/ci/run-reborn-root-partition.sh b/scripts/ci/run-reborn-root-partition.sh index 6c648d00ad8..9e417a34e03 100755 --- a/scripts/ci/run-reborn-root-partition.sh +++ b/scripts/ci/run-reborn-root-partition.sh @@ -42,22 +42,35 @@ if [ "${#test_names[@]}" -eq 0 ]; then exit 1 fi -selected=false +selected_names=() for index in "${!test_names[@]}"; do if (( index % partition_count_int != partition_index_int )); then continue fi - - selected=true - test_name="${test_names[$index]}" - echo "::group::cargo test --test ${test_name}" - timeout --signal=INT --kill-after=30s "${test_timeout}" \ - cargo test -p ironclaw_integration_tests --test "${test_name}" -- --nocapture - echo "::endgroup::" + selected_names+=("${test_names[$index]}") done -if [ "${selected}" != true ]; then +if [ "${#selected_names[@]}" -eq 0 ]; then # Empty partitions are valid when the matrix has more partitions than tests # or when the sorted test list leaves a sparse tail for this partition. echo "No Reborn root tests assigned to partition ${partition_index_int} of ${partition_count_int}; passing by design" + exit 0 +fi + +source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib/select-test-runner.sh" +runner="$(select_test_runner require-in-ci)" +test_args=(); for name in "${selected_names[@]}"; do test_args+=(--test "${name}"); done +if [[ "${runner}" == "nextest" ]]; then + echo "::group::cargo nextest run --profile ci (partition ${partition_index_int}/${partition_count_int}: ${selected_names[*]})" + timeout --signal=INT --kill-after=30s "${test_timeout}" \ + cargo nextest run --profile ci -p ironclaw_integration_tests \ + "${test_args[@]}" --ignore-rust-version + echo "::endgroup::" +else + for test_name in "${selected_names[@]}"; do + echo "::group::cargo test --test ${test_name}" + timeout --signal=INT --kill-after=30s "${test_timeout}" \ + cargo test -p ironclaw_integration_tests --test "${test_name}" -- --nocapture + echo "::endgroup::" + done fi diff --git a/scripts/ci/test-hermetic-test-process.sh b/scripts/ci/test-hermetic-test-process.sh index fe3add774b3..e5e43fd1aad 100755 --- a/scripts/ci/test-hermetic-test-process.sh +++ b/scripts/ci/test-hermetic-test-process.sh @@ -411,4 +411,64 @@ if ! grep -Fq "resolve_webui_frontend_dir" "${repo_root}/scripts/ci/run-hermetic exit 1 fi +# Negative control: prove cargo-nextest's own child-process spawn path +# (not just a directly-launched probe binary) inherits the hermetic +# network guard. See tests/hermetic_network_guard_probe.rs. +# +# NOTE on exit codes: scripts/ci/hermetic-network-runner.sh (the target +# runner) forces its own exit to 86 whenever ANY non-loopback connection +# attempt is observed via IRONCLAW_HERMETIC_NETWORK_VIOLATIONS -- even one +# the guard's interposer correctly EPERM'd. That is deliberate defense in +# depth (no process should reach for the network at all, guarded or not), +# but it means the *process* exit code cannot distinguish "guard blocked +# the connect and the test's own assertion passed" from "nothing blocked +# it and the test's own assertion failed" -- both exit non-zero. The real +# signal is the test's own reported OUTCOME, captured in nextest's stdout: +# "... ok" / "1 passed" when the interposer's EPERM satisfied the test's +# assertion, vs a panic (mismatched error kind, e.g. TimedOut instead of +# PermissionDenied) when it did not. Empirically verified locally +# (2026-08-21): guarded run exits 86 but stdout shows "test +# nextest_child_process_is_network_guarded ... ok" / "1 passed"; sabotaged +# run exits non-zero via a genuine panic: "assertion `left == right` +# failed: ... left: TimedOut, right: PermissionDenied". +if command -v cargo-nextest >/dev/null 2>&1; then + set +e + guarded_output="$( + IRONCLAW_HERMETIC_SABOTAGE="${sabotage}" \ + "${runner}" -- cargo nextest run --profile ci -p ironclaw_integration_tests \ + --test hermetic_network_guard_probe --run-ignored ignored-only \ + --ignore-rust-version 2>&1 + )" + set -e + if [[ "${guarded_output}" != *"nextest_child_process_is_network_guarded ... ok"* ]]; then + echo "nextest negative control: guarded run's own test assertion should have PASSED (EPERM from the guard) but it did not" >&2 + printf '%s\n' "${guarded_output}" >&2 + exit 1 + fi + + set +e + unguarded_output="$( + IRONCLAW_HERMETIC_SABOTAGE=network \ + "${runner}" -- cargo nextest run --profile ci -p ironclaw_integration_tests \ + --test hermetic_network_guard_probe --run-ignored ignored-only \ + --ignore-rust-version 2>&1 + )" + set -e + if [[ "${unguarded_output}" == *"nextest_child_process_is_network_guarded ... ok"* ]]; then + echo "nextest negative control: run with the guard SABOTAGED should have FAILED the test's own assertion (nothing should have blocked the connect) but it passed -- this proves the guarded run's pass above was not caused by the guard" >&2 + printf '%s\n' "${unguarded_output}" >&2 + exit 1 + fi + if [[ "${unguarded_output}" != *"PermissionDenied"* ]]; then + echo "nextest negative control: sabotaged run failed, but not with the expected PermissionDenied-mismatch panic -- investigate before trusting this control" >&2 + printf '%s\n' "${unguarded_output}" >&2 + exit 1 + fi +elif [[ "${CI:-}" == "true" ]]; then + echo "cargo-nextest is required in CI for the nextest negative control but was not found on PATH" >&2 + exit 1 +else + echo "WARNING: cargo-nextest not installed locally; skipping the nextest negative control (it still runs in CI)" >&2 +fi + echo "hermetic test-process self-test: OK" diff --git a/scripts/ci/test-quality-gate-runner.sh b/scripts/ci/test-quality-gate-runner.sh index 4aa949748f4..0b75924ec98 100755 --- a/scripts/ci/test-quality-gate-runner.sh +++ b/scripts/ci/test-quality-gate-runner.sh @@ -141,6 +141,68 @@ capture_gate with-nextest expect_status "coverage parity: gate succeeds" "$gate_status" 0 expect_contains "nextest run keeps --all-targets --all-features" "$gate_log" "--all-targets --all-features" +# --- Direct unit tests of the shared lib (scripts/ci/lib/select-test-runner.sh) --- +# Exercises the "require-in-ci" policy the quality_gate.sh end-to-end +# cases above never touch (quality_gate.sh always calls "optional"). +LIB="$SCRIPT_DIR/lib/select-test-runner.sh" + +run_lib() { + local with_nextest="$1" policy="$2" + shift 2 + local sandbox + sandbox="$(mktemp -d)" + cat >"$sandbox/cargo-nextest" <<'STUB' +#!/usr/bin/env bash +exit 0 +STUB + if [ "$with_nextest" = "with-nextest" ]; then + chmod +x "$sandbox/cargo-nextest" + else + rm -f "$sandbox/cargo-nextest" + fi + # PATH is REPLACED, and CI is force-unset before any trailing "$@" + # NAME=value overrides are applied -- `env -u CI ... CI=true ...` + # unsets first, then a later CI=true argument re-adds it, so callers + # opt IN to CI=true by passing it as a trailing arg rather than via a + # `VAR=val function` prefix (which a shell function does not reliably + # forward into a subsequently-`env`-invoked subprocess's environment). + # The status is captured via an `if`, not a bare `set +e`/`set -e` + # toggle: `set -e` is a global shell option, not function-scoped, so + # flipping it back on inside this function would leak out and fight + # whatever errexit state the *caller* had established around its own + # call to run_lib. + local status status_code + if status="$( + env -u CI PATH="$sandbox:/usr/bin:/bin" "$@" \ + bash -c "source '$LIB' && select_test_runner '$policy'" + )"; then + status_code=0 + else + status_code=$? + fi + rm -rf "$sandbox" + echo "$status" + return "$status_code" +} + +lib_out="$(run_lib with-nextest require-in-ci)" +expect_contains "require-in-ci, nextest present: picks nextest" "$lib_out" "nextest" + +# CI unset explicitly (env -u CI): GitHub Actions runners export CI=true +# ambiently, so a case meaning "local reproduction, nextest absent" must +# unset it or it silently tests the wrong branch. +lib_out="$(run_lib without-nextest require-in-ci)" +expect_contains "require-in-ci, nextest absent, CI unset: falls back to cargo" "$lib_out" "cargo" + +set +e +run_lib without-nextest require-in-ci CI=true >/dev/null +require_ci_status=$? +set -e +expect_failure "require-in-ci, nextest absent, CI=true: hard fails" "$require_ci_status" + +lib_out="$(run_lib with-nextest optional CI=true)" +expect_contains "optional policy ignores CI=true when nextest is present" "$lib_out" "nextest" + if [ "$failures" -ne 0 ]; then echo "$failures check(s) failed" exit 1 diff --git a/tests/hermetic_network_guard_probe.rs b/tests/hermetic_network_guard_probe.rs new file mode 100644 index 00000000000..5e96b23f0e9 --- /dev/null +++ b/tests/hermetic_network_guard_probe.rs @@ -0,0 +1,53 @@ +//! Negative-control proof that cargo-nextest's per-test child processes are +//! still wrapped by the hermetic network guard +//! (`CARGO_TARGET__RUNNER` -> scripts/ci/hermetic-network-runner.sh). +//! `scripts/ci/test-hermetic-test-process.sh`'s existing checks prove the +//! guard blocks a directly-launched compiled probe; they never invoke +//! `cargo`/`cargo nextest`, so they prove nothing about whether nextest's +//! test-process spawn path inherits the same env. This test does: run it +//! once under `cargo nextest` inside the hermetic wrapper with the guard +//! ACTIVE (must PASS -- connection refused), and once with +//! `IRONCLAW_HERMETIC_SABOTAGE=network` (guard DISABLED, must FAIL -- +//! connection not refused within the timeout). Only the differential +//! between those two runs is the actual proof; see +//! scripts/ci/test-hermetic-test-process.sh for the orchestration. +//! +//! Deliberately NOT `reborn_`-prefixed (precedent: trace_format.rs, +//! trace_llm_tests.rs) so it is invisible to the root-partition, group, +//! and planner `tests/reborn_*` globs -- it must never run as part of an +//! ordinary PR's test selection, only via this file's explicit invocation. +#[test] +#[ignore = "network-guard negative control; run explicitly via cargo \ + nextest inside scripts/ci/run-hermetic-test-process.sh, see \ + scripts/ci/test-hermetic-test-process.sh"] +fn nextest_child_process_is_network_guarded() { + use std::io::ErrorKind; + use std::net::TcpStream; + use std::time::Duration; + + // TEST-NET-1 (RFC 5737): non-routable, never answers. Un-guarded, a + // real connect() to it times out or reports unreachable; guarded, the + // interposer's connect() wrapper sets errno=EPERM immediately + // (scripts/ci/hermetic-network-guard.c) -- so PermissionDenied is a + // guard-specific signature, not "any connection error". + let outcome = TcpStream::connect_timeout( + &"192.0.2.1:80".parse().expect("literal address parses"), + Duration::from_secs(5), + ); + match outcome { + Ok(_) => panic!( + "connected to a non-loopback TEST-NET-1 address; the network \ + guard did not intercept this nextest-run test process" + ), + Err(err) => { + assert_eq!( + err.kind(), + ErrorKind::PermissionDenied, + "connect() failed but not via EPERM (the hermetic network \ + guard's interposer sets EPERM on a blocked connect; a \ + plain timeout or unreachable-host error means this \ + process was NOT guarded): {err}" + ); + } + } +} From 4f7785079b9dcebf1f4aea0445d4b7ea2a82a85c Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Sat, 22 Aug 2026 01:29:42 +0000 Subject: [PATCH 04/19] ci(tests): crate buckets run cargo-nextest (non-coverage arms), local repro matches Both non-coverage arms of the crate-tests "Run crate tests" step (the per-exact-target loop and the bulk-package invocation) swap cargo test for cargo nextest run --profile ci, keeping per-package feature flags, the RUSTC_BOOTSTRAP/-Zcrate-attr env, CARGO_INCREMENTAL=0 for ironclaw_composition, and the hermetic prepare-command/command wrappers untouched. The coverage arm (line 404's cargo llvm-cov invocation) is byte-identical -- this task does not touch it (Decision 2). scripts/ci/run-hermetic-deterministic-suite.sh's run_crate_tests (the canonical local full-workspace reproduction, never invoked from a workflow) now routes through the same scripts/ci/lib/select-test-runner.sh seam Task 3 introduced, closing the local/CI parity gap the nextest swap would otherwise have widened. Co-Authored-By: Claude Fable 5 --- .github/workflows/reborn-tests.yml | 24 +++++++++++++++---- .../ci/run-hermetic-deterministic-suite.sh | 13 +++++++--- scripts/ci/test_reborn_pr_test_plan.py | 4 ++-- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/.github/workflows/reborn-tests.yml b/.github/workflows/reborn-tests.yml index 2252b526774..b7eaf60f9ac 100644 --- a/.github/workflows/reborn-tests.yml +++ b/.github/workflows/reborn-tests.yml @@ -249,6 +249,11 @@ jobs: toolchain: nightly-2025-11-01 components: llvm-tools-preview + - name: Install cargo-nextest + uses: taiki-e/install-action@62b0f2dec647a8e604c6a0fda0e38530180dce20 # v2 + with: + tool: cargo-nextest@0.9.143 + - name: Resolve bucket settings id: bucket-settings env: @@ -405,6 +410,7 @@ jobs: echo "::endgroup::" else echo "::group::cargo test affected bucket ${BUCKET_NAME}" + mkdir -p junit-reports if [[ "$(jq 'length' <<< "${EXACT_TARGETS}")" -gt 0 ]]; then while IFS=$'\t' read -r package kind name; do target_feature_args=() @@ -414,9 +420,10 @@ jobs: fi scripts/ci/run-hermetic-deterministic-suite.sh command \ timeout --signal=INT --kill-after=30s 28m \ - "${incremental_env[@]}" cargo test \ + "${incremental_env[@]}" cargo nextest run --profile ci \ -p "${package}" "--${kind}" "${name}" \ - "${target_feature_args[@]}" --ignore-rust-version -- --nocapture + "${target_feature_args[@]}" --all-targets --ignore-rust-version + cp target/nextest/ci/junit.xml "junit-reports/${BUCKET_NAME}-${name}.xml" 2>/dev/null || true done < <( jq -r '.[] | [.package, .kind, .name] | @tsv' \ <<< "${EXACT_TARGETS}" @@ -424,8 +431,9 @@ jobs: else scripts/ci/run-hermetic-deterministic-suite.sh command \ timeout --signal=INT --kill-after=30s 28m \ - "${incremental_env[@]}" cargo test "${package_args[@]}" \ - "${feature_args[@]}" --ignore-rust-version --all-targets -- --nocapture + "${incremental_env[@]}" cargo nextest run --profile ci "${package_args[@]}" \ + "${feature_args[@]}" --all-targets --ignore-rust-version + cp target/nextest/ci/junit.xml "junit-reports/${BUCKET_NAME}-bulk.xml" 2>/dev/null || true fi echo "::endgroup::" fi @@ -438,6 +446,14 @@ jobs: path: bucket-${{ matrix.bucket.name }}.lcov if-no-files-found: error + - name: Upload JUnit reports + if: always() && needs.changes.outputs.run_coverage != 'true' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: reborn-junit-bucket-${{ matrix.bucket.name }} + path: junit-reports/ + if-no-files-found: ignore + root-reborn-parity-tests: name: Reborn root tests (${{ matrix.partition }}) needs: changes diff --git a/scripts/ci/run-hermetic-deterministic-suite.sh b/scripts/ci/run-hermetic-deterministic-suite.sh index 7ef923aec33..f5c40c2888f 100755 --- a/scripts/ci/run-hermetic-deterministic-suite.sh +++ b/scripts/ci/run-hermetic-deterministic-suite.sh @@ -76,12 +76,19 @@ discover_reborn_packages() { } run_crate_tests() { - local package feature_flags + local package feature_flags runner prepare_postgres_test_image + source "${repo_root}/scripts/ci/lib/select-test-runner.sh" + runner="$(select_test_runner optional)" while IFS= read -r package; do feature_flags="$("${repo_root}/scripts/ci/package-feature-flags.sh" "${package}")" - # shellcheck disable=SC2086 # feature_flags is the checked-in CI argument list. - run cargo test -p "${package}" ${feature_flags} --all-targets -- --nocapture + if [[ "${runner}" == "nextest" ]]; then + # shellcheck disable=SC2086 # feature_flags is the checked-in CI argument list. + run cargo nextest run --profile ci -p "${package}" ${feature_flags} --all-targets --ignore-rust-version + else + # shellcheck disable=SC2086 + run cargo test -p "${package}" ${feature_flags} --all-targets -- --nocapture + fi done < <(discover_reborn_packages) } diff --git a/scripts/ci/test_reborn_pr_test_plan.py b/scripts/ci/test_reborn_pr_test_plan.py index 6bf03df2cc6..ea0f630b3d1 100644 --- a/scripts/ci/test_reborn_pr_test_plan.py +++ b/scripts/ci/test_reborn_pr_test_plan.py @@ -2080,7 +2080,7 @@ def test_workflow_consumes_plan_and_bounds_each_rust_matrix(self) -> None: ) self.assertIn("--test reborn_integration_sandbox_shell_turn", workflow) self.assertIn( - '"${feature_args[@]}" --ignore-rust-version --all-targets', + '"${feature_args[@]}" --all-targets --ignore-rust-version', workflow, ) self.assertIn( @@ -2138,7 +2138,7 @@ def test_workflow_consumes_plan_and_bounds_each_rust_matrix(self) -> None: workflow, ) self.assertIn( - '"${incremental_env[@]}" cargo test \\\n' + '"${incremental_env[@]}" cargo nextest run --profile ci \\\n' ' -p "${package}" "--${kind}" "${name}"', workflow, ) From 504c93b457c3453a6330afc320268820cf93f6e0 Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Sat, 22 Aug 2026 01:32:20 +0000 Subject: [PATCH 05/19] ci(tests): integration lanes run cargo-nextest when uninstrumented; group mode stays on cargo The uninstrumented (REBORN_COV_COLLECT=false) else-arm of reborn-coverage-lane-run.sh swaps cargo test for cargo nextest run --profile ci in flat-partition mode. group mode gets an explicit code-level carve-out (Decision 3): it forces runner=cargo regardless of what select_test_runner would otherwise pick, since this mode selects the identical reborn_group_* set as the dedicated group job, and that job's whole purpose is keeping those binaries out of a cross-binary-concurrent pool. Verified end-to-end in scripts/ci/test-quality-gate-runner.sh with a stubbed cargo-nextest present on PATH to prove it is ignored. The instrumented (REBORN_COV_COLLECT=true) coverage arm is untouched (Decision 2). run_integration_tier in run-hermetic-deterministic-suite.sh (the canonical local full-tier reproduction) now routes through the same runner-selection seam; unlike the lane script it does not split group vs. flat (a deliberate, narrower scope difference documented inline, since this local function's scope was always "the full tier" rather than "one lane"). Co-Authored-By: Claude Fable 5 --- .github/workflows/reborn-tests.yml | 13 +++++ scripts/ci/reborn-coverage-lane-run.sh | 29 ++++++++++-- .../ci/run-hermetic-deterministic-suite.sh | 16 +++++-- scripts/ci/test-quality-gate-runner.sh | 47 +++++++++++++++++++ scripts/ci/test_reborn_pr_test_plan.py | 15 ++++-- 5 files changed, 109 insertions(+), 11 deletions(-) diff --git a/.github/workflows/reborn-tests.yml b/.github/workflows/reborn-tests.yml index b7eaf60f9ac..575b9296181 100644 --- a/.github/workflows/reborn-tests.yml +++ b/.github/workflows/reborn-tests.yml @@ -767,6 +767,11 @@ jobs: toolchain: nightly-2025-11-01 components: llvm-tools-preview + - name: Install cargo-nextest + uses: taiki-e/install-action@62b0f2dec647a8e604c6a0fda0e38530180dce20 # v2 + with: + tool: cargo-nextest@0.9.143 + - name: Enable pnpm for setup-node cache run: corepack enable pnpm @@ -856,6 +861,14 @@ jobs: with: name: reborn-integration-cov-part-${{ matrix.lane }} path: part-${{ matrix.lane }}.lcov + + - name: Upload JUnit report + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: reborn-junit-lane-${{ matrix.lane }} + path: target/nextest/ci/junit.xml + if-no-files-found: ignore if-no-files-found: error coverage-report: diff --git a/scripts/ci/reborn-coverage-lane-run.sh b/scripts/ci/reborn-coverage-lane-run.sh index 0458bed38e7..117428afae0 100755 --- a/scripts/ci/reborn-coverage-lane-run.sh +++ b/scripts/ci/reborn-coverage-lane-run.sh @@ -138,10 +138,29 @@ if [[ "${collect_coverage}" == "true" ]]; then cargo llvm-cov --branch --skip-functions --workspace test "${test_args[@]}" \ --lcov --output-path "${output_lcov}" \ --ignore-rust-version -- --nocapture + echo "::endgroup::" else - echo "::group::cargo test -p ironclaw_integration_tests ${test_args[*]}" - timeout --signal=INT --kill-after=30s "${test_timeout}" \ - cargo test -p ironclaw_integration_tests "${test_args[@]}" \ - --ignore-rust-version -- --nocapture + if [[ "${mode}" == "group" ]]; then + # Decision 3 (T2 plan): this mode selects the identical reborn_group_* + # set as the dedicated group job (see the comment on + # scripts/ci/run-reborn-group-tests.sh) -- force cargo regardless of + # what select_test_runner would otherwise pick, so a nextest install + # in this job never silently pulls group suites into the pool. + runner="cargo" + else + source "${script_dir}/lib/select-test-runner.sh" + runner="$(select_test_runner require-in-ci)" + fi + if [[ "${runner}" == "nextest" ]]; then + echo "::group::cargo nextest run --profile ci -p ironclaw_integration_tests ${test_args[*]}" + timeout --signal=INT --kill-after=30s "${test_timeout}" \ + cargo nextest run --profile ci -p ironclaw_integration_tests "${test_args[@]}" \ + --ignore-rust-version + else + echo "::group::cargo test -p ironclaw_integration_tests ${test_args[*]}" + timeout --signal=INT --kill-after=30s "${test_timeout}" \ + cargo test -p ironclaw_integration_tests "${test_args[@]}" \ + --ignore-rust-version -- --nocapture + fi + echo "::endgroup::" fi -echo "::endgroup::" diff --git a/scripts/ci/run-hermetic-deterministic-suite.sh b/scripts/ci/run-hermetic-deterministic-suite.sh index f5c40c2888f..3283f876eb9 100755 --- a/scripts/ci/run-hermetic-deterministic-suite.sh +++ b/scripts/ci/run-hermetic-deterministic-suite.sh @@ -93,13 +93,23 @@ run_crate_tests() { } run_integration_tier() { - local test_name + local test_name runner + local -a test_args=() prepare_postgres_test_image + source "${repo_root}/scripts/ci/lib/select-test-runner.sh" + runner="$(select_test_runner optional)" while IFS= read -r test_name; do [[ "${test_name}" == --test ]] && continue - run cargo test -p ironclaw_integration_tests \ - --test "${test_name}" -- --nocapture + test_args+=(--test "${test_name}") done < <("${repo_root}/scripts/ci/reborn-coverage-int-tier-tests.sh") + if [[ "${runner}" == "nextest" ]]; then + run cargo nextest run --profile ci -p ironclaw_integration_tests "${test_args[@]}" --ignore-rust-version + else + for test_name in "${test_args[@]}"; do + [[ "${test_name}" == --test ]] && continue + run cargo test -p ironclaw_integration_tests --test "${test_name}" -- --nocapture + done + fi } run_python_e2e() { diff --git a/scripts/ci/test-quality-gate-runner.sh b/scripts/ci/test-quality-gate-runner.sh index 0b75924ec98..a5cebded676 100755 --- a/scripts/ci/test-quality-gate-runner.sh +++ b/scripts/ci/test-quality-gate-runner.sh @@ -203,6 +203,53 @@ expect_failure "require-in-ci, nextest absent, CI=true: hard fails" "$require_ci lib_out="$(run_lib with-nextest optional CI=true)" expect_contains "optional policy ignores CI=true when nextest is present" "$lib_out" "nextest" +# --- reborn-coverage-lane-run.sh's group-mode carve-out (Task 5, T2 plan) --- +# Decision 3: `group` mode must force the sequential cargo-test runner +# regardless of what select_test_runner would otherwise pick, so a +# nextest install in this job never silently pulls reborn_group_* suites +# into the parallel pool. Exercised end-to-end (stubbed cargo/nextest, +# stubbed suite discovery) rather than grepped, since the carve-out is +# inline control flow, not a separately-sourceable function. +run_lane_group_carveout() { + local sandbox + sandbox="$(mktemp -d)" + cat >"$sandbox/cargo" <<'STUB' +#!/usr/bin/env bash +echo "cargo $*" >>"$GATE_TEST_LOG" +STUB + chmod +x "$sandbox/cargo" + cat >"$sandbox/cargo-nextest" <<'STUB' +#!/usr/bin/env bash +echo "cargo-nextest $*" >>"$GATE_TEST_LOG" +STUB + chmod +x "$sandbox/cargo-nextest" + mkdir -p "$sandbox/scripts/ci/lib" + cp "$SCRIPT_DIR/reborn-coverage-lane-run.sh" "$sandbox/scripts/ci/reborn-coverage-lane-run.sh" + cp "$SCRIPT_DIR/lib/select-test-runner.sh" "$sandbox/scripts/ci/lib/select-test-runner.sh" + cat >"$sandbox/scripts/ci/reborn-coverage-int-tier-tests.sh" <<'STUB' +#!/usr/bin/env bash +printf -- '--test\nreborn_group_fixture\n' +STUB + chmod +x "$sandbox/scripts/ci/reborn-coverage-int-tier-tests.sh" + local status + set +e + status="$( + env \ + GATE_TEST_LOG="$sandbox/log" \ + PATH="$sandbox:/usr/bin:/bin" \ + REBORN_COV_COLLECT=false \ + REBORN_COV_LANE_MODE=group \ + "$sandbox/scripts/ci/reborn-coverage-lane-run.sh" "$sandbox/unused.lcov" 2>&1 + )" + set -e + rm -rf "$sandbox" + echo "$status" +} + +lane_out="$(run_lane_group_carveout)" +expect_contains "group mode: forces cargo even with nextest present" "$lane_out" "cargo test -p ironclaw_integration_tests" +expect_not_contains "group mode: never invokes nextest" "$lane_out" "nextest run" + if [ "$failures" -ne 0 ]; then echo "$failures check(s) failed" exit 1 diff --git a/scripts/ci/test_reborn_pr_test_plan.py b/scripts/ci/test_reborn_pr_test_plan.py index ea0f630b3d1..8780df6f248 100644 --- a/scripts/ci/test_reborn_pr_test_plan.py +++ b/scripts/ci/test_reborn_pr_test_plan.py @@ -943,8 +943,8 @@ def test_generated_integration_suites_are_assigned_to_flat_lanes(self) -> None: ).read_text(encoding="utf-8") self.assertIn("reborn_(integration_|generated_)", lane_runner) self.assertIn( - 'cargo test -p ironclaw_integration_tests "${test_args[@]}" ' - "\\\n --ignore-rust-version", + 'cargo nextest run --profile ci -p ironclaw_integration_tests ' + '"${test_args[@]}" \\\n --ignore-rust-version', lane_runner, ) @@ -952,9 +952,18 @@ def test_selected_integration_lane_keeps_msrv_override(self) -> None: lane_runner = ( ROOT / "scripts/ci/reborn-coverage-lane-run.sh" ).read_text(encoding="utf-8") + # nextest arm (flat-partition mode, when nextest is selected). + self.assertIn( + 'cargo nextest run --profile ci -p ironclaw_integration_tests ' + '"${test_args[@]}" \\\n --ignore-rust-version', + lane_runner, + ) + # cargo fallback arm (group mode always forces this; flat-partition + # falls back to it when nextest is unavailable) -- byte-identical to + # today's shape so this pin still holds regardless of which arm ran. self.assertIn( 'cargo test -p ironclaw_integration_tests "${test_args[@]}" ' - "\\\n --ignore-rust-version -- --nocapture", + "\\\n --ignore-rust-version -- --nocapture", lane_runner, ) From ac9bc6be008119f2e4c223c3f2e5dcabe6110820 Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Sat, 22 Aug 2026 01:40:16 +0000 Subject: [PATCH 06/19] ci(tests): wire T4's Pattern B REPRO reporting into the four job types this plan touches Root-partition, group-tests, integration-lane, and crate-bucket steps each set REPRO to the exact reproducing command, persist it to $GITHUB_ENV before executing, then eval it (root-partition, group-tests, crate-bucket) or run the step's own real invocation unchanged (integration-lane, where REPRO is a PR-path-only reproduction distinct from the step's actual REBORN_COV_COLLECT-conditional command). A fixed "Local repro for this job" step renders whatever $GITHUB_ENV holds on failure, identical text across every job so it cannot go stale on a rename. Cites T4's Task 4 Interfaces (scratchpad/plans/T4-canonical-preflight.md) as the pattern's source, re-verified directly against that plan file since T4 has not merged yet. The crate-bucket REPRO strings include the timeout wrapper and incremental_env prefix (CARGO_INCREMENTAL=0 for ironclaw_composition) that actually ran, not just the bare nextest invocation, since eval'ing a stripped-down REPRO would silently change what runs. This also updates two of Task 4's own pinned test literals, which this task's REPRO edit supersedes at the same lines. The integration-lane step's REPRO targets the uninstrumented PR/queue reproduction only; the coverage arm (push-to-main-only, byte-identical per Decision 2) is out of scope regardless. Co-Authored-By: Claude Fable 5 --- .github/workflows/reborn-tests.yml | 70 ++++++++++++++++++++------ scripts/ci/test_reborn_pr_test_plan.py | 6 +-- 2 files changed, 59 insertions(+), 17 deletions(-) diff --git a/.github/workflows/reborn-tests.yml b/.github/workflows/reborn-tests.yml index 575b9296181..b4a4c9164f6 100644 --- a/.github/workflows/reborn-tests.yml +++ b/.github/workflows/reborn-tests.yml @@ -418,26 +418,33 @@ jobs: if [[ -n "${feature_flags}" ]]; then read -ra target_feature_args <<< "${feature_flags}" fi - scripts/ci/run-hermetic-deterministic-suite.sh command \ - timeout --signal=INT --kill-after=30s 28m \ - "${incremental_env[@]}" cargo nextest run --profile ci \ - -p "${package}" "--${kind}" "${name}" \ - "${target_feature_args[@]}" --all-targets --ignore-rust-version + REPRO="scripts/ci/run-hermetic-deterministic-suite.sh command timeout --signal=INT --kill-after=30s 28m ${incremental_env[*]} cargo nextest run --profile ci -p ${package} --${kind} ${name} ${target_feature_args[*]} --all-targets --ignore-rust-version" + echo "REPRO=${REPRO}" >> "$GITHUB_ENV" + eval "${REPRO}" cp target/nextest/ci/junit.xml "junit-reports/${BUCKET_NAME}-${name}.xml" 2>/dev/null || true done < <( jq -r '.[] | [.package, .kind, .name] | @tsv' \ <<< "${EXACT_TARGETS}" ) else - scripts/ci/run-hermetic-deterministic-suite.sh command \ - timeout --signal=INT --kill-after=30s 28m \ - "${incremental_env[@]}" cargo nextest run --profile ci "${package_args[@]}" \ - "${feature_args[@]}" --all-targets --ignore-rust-version + REPRO="scripts/ci/run-hermetic-deterministic-suite.sh command timeout --signal=INT --kill-after=30s 28m ${incremental_env[*]} cargo nextest run --profile ci ${package_args[*]} ${feature_args[*]} --all-targets --ignore-rust-version" + echo "REPRO=${REPRO}" >> "$GITHUB_ENV" + eval "${REPRO}" cp target/nextest/ci/junit.xml "junit-reports/${BUCKET_NAME}-bulk.xml" 2>/dev/null || true fi echo "::endgroup::" fi + - name: Local repro for this job + if: failure() + run: | + { + echo "### Local repro for this job" + echo '```bash' + echo "${REPRO:-}" + echo '```' + } | tee -a "$GITHUB_STEP_SUMMARY" + - name: Upload bucket lcov artifact if: needs.changes.outputs.run_coverage == 'true' uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 @@ -549,10 +556,20 @@ jobs: redis-password: ${{ secrets.SCCACHE_REDIS_PASSWORD }} - name: Run Reborn root tests - run: >- - scripts/ci/run-hermetic-deterministic-suite.sh - command - scripts/ci/run-reborn-root-partition.sh + run: | + REPRO='REBORN_ROOT_TEST_PARTITIONS=4 REBORN_ROOT_TEST_PARTITION=${{ matrix.partition }} RUST_MIN_STACK=67108864 scripts/ci/run-hermetic-deterministic-suite.sh command scripts/ci/run-reborn-root-partition.sh' + echo "REPRO=${REPRO}" >> "$GITHUB_ENV" + eval "${REPRO}" + + - name: Local repro for this job + if: failure() + run: | + { + echo "### Local repro for this job" + echo '```bash' + echo "${REPRO:-}" + echo '```' + } | tee -a "$GITHUB_STEP_SUMMARY" - name: Upload JUnit report if: always() @@ -653,7 +670,20 @@ jobs: redis-password: ${{ secrets.SCCACHE_REDIS_PASSWORD }} - name: Run Reborn group tests - run: scripts/ci/run-hermetic-deterministic-suite.sh groups + run: | + REPRO='scripts/ci/run-hermetic-deterministic-suite.sh groups' + echo "REPRO=${REPRO}" >> "$GITHUB_ENV" + eval "${REPRO}" + + - name: Local repro for this job + if: failure() + run: | + { + echo "### Local repro for this job" + echo '```bash' + echo "${REPRO:-}" + echo '```' + } | tee -a "$GITHUB_STEP_SUMMARY" sandbox-docker-tests: name: Reborn user-sandbox Docker tests @@ -850,11 +880,23 @@ jobs: # matrix leg. Clear only profiling data, not compiled binaries. cargo llvm-cov clean --profraw-only fi + REPRO="REBORN_COV_COLLECT=false REBORN_COV_LANE_MODE=${REBORN_COV_LANE_MODE} REBORN_COV_LANE_PARTITIONS=4 REBORN_COV_LANE_INDEX=${REBORN_COV_LANE_INDEX} scripts/ci/run-hermetic-deterministic-suite.sh command scripts/ci/reborn-coverage-lane-run.sh /tmp/repro.lcov" + echo "REPRO=${REPRO}" >> "$GITHUB_ENV" scripts/ci/run-hermetic-deterministic-suite.sh prepare-command IRONCLAW_HERMETIC_SUITE_SKIP_PREPARE=1 \ scripts/ci/run-hermetic-deterministic-suite.sh command \ scripts/ci/reborn-coverage-lane-run.sh part-${{ matrix.lane }}.lcov + - name: Local repro for this job + if: failure() + run: | + { + echo "### Local repro for this job" + echo '```bash' + echo "${REPRO:-}" + echo '```' + } | tee -a "$GITHUB_STEP_SUMMARY" + - name: Upload lane lcov artifact if: needs.changes.outputs.run_coverage == 'true' uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 diff --git a/scripts/ci/test_reborn_pr_test_plan.py b/scripts/ci/test_reborn_pr_test_plan.py index 8780df6f248..afa0bb1cbe3 100644 --- a/scripts/ci/test_reborn_pr_test_plan.py +++ b/scripts/ci/test_reborn_pr_test_plan.py @@ -2089,7 +2089,7 @@ def test_workflow_consumes_plan_and_bounds_each_rust_matrix(self) -> None: ) self.assertIn("--test reborn_integration_sandbox_shell_turn", workflow) self.assertIn( - '"${feature_args[@]}" --all-targets --ignore-rust-version', + '${feature_args[*]} --all-targets --ignore-rust-version', workflow, ) self.assertIn( @@ -2147,8 +2147,8 @@ def test_workflow_consumes_plan_and_bounds_each_rust_matrix(self) -> None: workflow, ) self.assertIn( - '"${incremental_env[@]}" cargo nextest run --profile ci \\\n' - ' -p "${package}" "--${kind}" "${name}"', + "${incremental_env[*]} cargo nextest run --profile ci " + "-p ${package} --${kind} ${name}", workflow, ) From f62b9ad10498f110e8f623f01a79986d22ecfa64 Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Sat, 22 Aug 2026 01:40:50 +0000 Subject: [PATCH 07/19] docs(testing): note lock_env()'s cross-test race rationale is process-model-dependent scripts/ci/check-hermetic-env.sh's lock_env()/EnvGuard requirement was written for cargo test's thread-per-test model; cargo nextest (now wired for root partitions, crate buckets, and uninstrumented integration lanes) runs one process per test, so the cross-test race the guard exists to prevent cannot occur the same way there. Leaves the check itself unchanged -- documentation only, so nobody deletes the guard believing it is now dead weight everywhere. Co-Authored-By: Claude Fable 5 --- .claude/rules/testing.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index 769d1a2a8b6..654c782f95e 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -71,6 +71,18 @@ delegation, redaction, retry, or policy behavior survives every wrapper. - Extend an existing suite when it owns the same seam. - Use `tempfile` for test files and directories. Never hardcode system temporary-directory paths. +- `lock_env()`/`EnvGuard` (`crates/contracts/ironclaw_common/src/env_helpers.rs`, + enforced by `scripts/ci/check-hermetic-env.sh`) serializes raw + `std::env::set_var`/`remove_var` against sibling tests **that share a + process** — the classic `cargo test` thread-per-test model. Under + `cargo nextest` (root partitions, crate buckets, uninstrumented + integration lanes — see `.config/nextest.toml [profile.ci]`), each test + is its own process, so this specific cross-test race cannot happen for + nextest-run binaries; the guard is still required for any test that + itself spawns multiple threads mutating env within one process. Do not + treat the guard's presence as proof of cross-test serialization under + nextest, and do not delete it as "now redundant" — confirm which + process model actually applies to the binary in question first. - Avoid ignored or TODO-pinned tests; landed tests run in CI. - Prefer real in-memory implementations, deterministic fakes, or recording adapters over mocks that duplicate internal behavior. From aa84811e92049b45f31d7902c995d3b9640f6d89 Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Sun, 23 Aug 2026 05:13:33 +0000 Subject: [PATCH 08/19] fix(ci): run the nextest network-guard control where cargo can resolve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The control added earlier in this branch shells out to `cargo nextest`, which runs `cargo metadata` under the hermetic wrapper. `fast-checks` is deliberately a cache-less, toolchain-light lane, so that resolution had no warm registry and failed offline with "no matching package named `async-trait` found" — reddening `Fast deterministic checks` and the required `Code Style (fmt + clippy)` roll-up on this PR. On main this self-test invokes cargo zero times; the regression was putting a cargo-dependent check into the one job built to avoid cargo. The control is now opt-in via IRONCLAW_HERMETIC_NEXTEST_CONTROL=1 and runs in root-reborn-parity-tests, which already installs the toolchain and cargo-nextest, restores the registry cache, and builds the probe crate. The default path runs exactly the stub-based checks main runs. This also closes an ordering hazard that would have bitten anyway: the `all` stage of run-hermetic-deterministic-suite.sh invokes this self-test *before* prepare_rust_dependencies, so the control could never have resolved there. Regression test: scripts/ci/test-hermetic-nextest-control-gating.sh pins both halves of the contract in a PATH sandbox — the default run must exit 0, must report the control as skipped, and must never invoke cargo (a stub records any invocation); an opted-in run without cargo-nextest under CI must still fail loudly. Proven red-first: with the gate removed it reports 3 failures including the recorded cargo call, and passes once restored. Wired into the fast-checks self-test list beside the script it guards. Verified: ws12 self-tests 94 OK; ws12 live gate passed; planner suite 87 OK; both workflows parse. Co-Authored-By: Claude Fable 5 --- .github/workflows/code_style.yml | 1 + .github/workflows/reborn-tests.yml | 10 +++ .../test-hermetic-nextest-control-gating.sh | 83 +++++++++++++++++++ scripts/ci/test-hermetic-test-process.sh | 15 +++- 4 files changed, 108 insertions(+), 1 deletion(-) create mode 100755 scripts/ci/test-hermetic-nextest-control-gating.sh diff --git a/.github/workflows/code_style.yml b/.github/workflows/code_style.yml index 12d65f83d5f..e8b4344a93b 100644 --- a/.github/workflows/code_style.yml +++ b/.github/workflows/code_style.yml @@ -216,6 +216,7 @@ jobs: python3 scripts/ci/test_check_h2_advisory_exception.py python3 scripts/ci/test_cut_ironclaw_release.py scripts/ci/test-hermetic-test-process.sh + scripts/ci/test-hermetic-nextest-control-gating.sh scripts/ci/test-reborn-docker-entrypoint.sh # #7144: this 204-test module had never been run by any lane, so five # of its assertions had silently drifted out of sync with the code they diff --git a/.github/workflows/reborn-tests.yml b/.github/workflows/reborn-tests.yml index b4a4c9164f6..aedfd465f2f 100644 --- a/.github/workflows/reborn-tests.yml +++ b/.github/workflows/reborn-tests.yml @@ -555,6 +555,16 @@ jobs: cache-ssh-known-hosts: ${{ secrets.SCCACHE_CACHE_SSH_KNOWN_HOSTS }} redis-password: ${{ secrets.SCCACHE_REDIS_PASSWORD }} + # The hermetic wrapper's network guard must survive nextest's own + # process spawning, not just a directly-launched test binary. This is + # the only place that control can run: it shells out to cargo, so it + # needs the toolchain, a warm registry and cargo-nextest -- all of + # which this job already has and fast-checks deliberately does not. + - name: Hermetic nextest network-guard control + env: + IRONCLAW_HERMETIC_NEXTEST_CONTROL: "1" + run: bash scripts/ci/test-hermetic-test-process.sh + - name: Run Reborn root tests run: | REPRO='REBORN_ROOT_TEST_PARTITIONS=4 REBORN_ROOT_TEST_PARTITION=${{ matrix.partition }} RUST_MIN_STACK=67108864 scripts/ci/run-hermetic-deterministic-suite.sh command scripts/ci/run-reborn-root-partition.sh' diff --git a/scripts/ci/test-hermetic-nextest-control-gating.sh b/scripts/ci/test-hermetic-nextest-control-gating.sh new file mode 100755 index 00000000000..d55f06c0fc1 --- /dev/null +++ b/scripts/ci/test-hermetic-nextest-control-gating.sh @@ -0,0 +1,83 @@ +#!/usr/bin/env bash +# Regression test for the fast-checks/cargo split. +# +# scripts/ci/test-hermetic-test-process.sh runs in `fast-checks`, which is +# deliberately cache-less and toolchain-light. Its nextest network-guard +# control shells out to cargo (nextest runs `cargo metadata`), which cannot +# resolve offline inside the hermetic wrapper without a warm registry -- +# that reddened `Fast deterministic checks` and the required `Code Style` +# roll-up with "no matching package named `async-trait` found". +# +# The control is therefore opt-in. This test pins BOTH halves of that +# contract, using a PATH sandbox (same shape as test-quality-gate-runner.sh): +# a stub `cargo`/`cargo-nextest` that records any invocation and fails. +set -uo pipefail + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +under_test="${repo_root}/scripts/ci/test-hermetic-test-process.sh" +sandbox="$(mktemp -d)" +trap 'rm -rf "${sandbox}"' EXIT +marker="${sandbox}/cargo-was-invoked" +failures=0 + +assert_success() { + local label="$1" status="$2" + if [[ "${status}" -ne 0 ]]; then + echo "FAIL: ${label}: expected exit 0, got ${status}" >&2 + failures=$((failures + 1)) + fi +} + +assert_failure() { + local label="$1" status="$2" + if [[ "${status}" -eq 0 ]]; then + echo "FAIL: ${label}: expected a non-zero exit, got 0" >&2 + failures=$((failures + 1)) + fi +} + +assert_no_cargo_invocation() { + local label="$1" + if [[ -e "${marker}" ]]; then + echo "FAIL: ${label}: invoked cargo. fast-checks has no warm registry," \ + "so this is the regression that reddened the required roll-up:" >&2 + cat "${marker}" >&2 + failures=$((failures + 1)) + fi +} + +for stub in cargo cargo-nextest; do + cat > "${sandbox}/${stub}" <> "${marker}" +exit 1 +STUB + chmod +x "${sandbox}/${stub}" +done + +# 1. Default (the fast-checks shape): control skipped, script green, and +# cargo never invoked even though it is on PATH. +PATH="${sandbox}:${PATH}" env -u IRONCLAW_HERMETIC_NEXTEST_CONTROL \ + bash "${under_test}" >"${sandbox}/default.log" 2>&1 +assert_success "default run (fast-checks shape)" "$?" +assert_no_cargo_invocation "default run (fast-checks shape)" +if ! grep -q "control not requested" "${sandbox}/default.log"; then + echo "FAIL: default run did not report that the control was skipped" >&2 + failures=$((failures + 1)) +fi + +# 2. Opted in without cargo-nextest under CI: must fail loudly rather than +# silently skipping a control the lane explicitly asked for. +rm -f "${marker}" +missing="${sandbox}/no-nextest" +mkdir -p "${missing}" +cp "${sandbox}/cargo" "${missing}/cargo" +PATH="${missing}:/usr/bin:/bin" CI=true IRONCLAW_HERMETIC_NEXTEST_CONTROL=1 \ + bash "${under_test}" >"${sandbox}/optin.log" 2>&1 +assert_failure "opted-in run with cargo-nextest missing under CI" "$?" + +if [[ "${failures}" -ne 0 ]]; then + echo "hermetic nextest-control gating: ${failures} failure(s)" >&2 + exit 1 +fi +echo "hermetic nextest-control gating: OK" diff --git a/scripts/ci/test-hermetic-test-process.sh b/scripts/ci/test-hermetic-test-process.sh index e5e43fd1aad..1b0c5260111 100755 --- a/scripts/ci/test-hermetic-test-process.sh +++ b/scripts/ci/test-hermetic-test-process.sh @@ -431,6 +431,19 @@ fi # nextest_child_process_is_network_guarded ... ok" / "1 passed"; sabotaged # run exits non-zero via a genuine panic: "assertion `left == right` # failed: ... left: TimedOut, right: PermissionDenied". +# The control below is the only part of this self-test that invokes cargo. +# It therefore needs a prepared Rust dependency graph: nextest shells out to +# `cargo metadata`, which cannot resolve offline inside the hermetic wrapper +# unless the registry cache is already warm. `fast-checks` is deliberately a +# cache-less, toolchain-light lane, so this stays OFF by default and the +# stub-based checks above run everywhere unchanged. Jobs that already have a +# toolchain, a restored cargo registry and cargo-nextest opt in by setting +# IRONCLAW_HERMETIC_NEXTEST_CONTROL=1 (see reborn-tests.yml). +if [[ "${IRONCLAW_HERMETIC_NEXTEST_CONTROL:-0}" != "1" ]]; then + echo "hermetic test-process self-test: OK (nextest network-guard control not requested)" + exit 0 +fi + if command -v cargo-nextest >/dev/null 2>&1; then set +e guarded_output="$( @@ -465,7 +478,7 @@ if command -v cargo-nextest >/dev/null 2>&1; then exit 1 fi elif [[ "${CI:-}" == "true" ]]; then - echo "cargo-nextest is required in CI for the nextest negative control but was not found on PATH" >&2 + echo "cargo-nextest is required when IRONCLAW_HERMETIC_NEXTEST_CONTROL=1 but was not found on PATH" >&2 exit 1 else echo "WARNING: cargo-nextest not installed locally; skipping the nextest negative control (it still runs in CI)" >&2 From 20df88eab97f8c80ecc56af436df147196128665 Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Sun, 23 Aug 2026 17:19:42 +0000 Subject: [PATCH 09/19] fix(ci): build test commands as argv arrays instead of eval-ing a string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The crate-bucket lane interpolated planner-derived target names into a REPRO string and then `eval`-ed it. Those names are `Path(...).stem` of a CHANGED FILE (scripts/ci/reborn_pr_test_plan.py:1187), with no sanitisation anywhere on the path, so a contributor adding `crates//tests/$(cmd).rs` reached `eval` and executed `cmd` on the runner — in a job that sets up OVH sccache credentials. Fork PRs carry no secrets, but same-repo branches do. `main` has zero evals in this workflow and invokes the same arm as a quoted array, so this was introduced by the REPRO work, not inherited. All four sites now build the command as an array and run it directly, and the REPRO hint is derived FROM that argv with printf '%q '. That fixes three things at once: the value is never re-parsed (no injection), the printed line is genuinely paste-able (the reviewers' separate complaint that REPRO lines were not runnable), and %q keeps the value single-line so a newline in a filename cannot forge extra $GITHUB_ENV entries. Demonstrated: the old shape runs `$(touch /tmp/pwned)` from a filename; the array shape treats it as data and emits `echo target= \$\(touch\ /tmp/pwned\)`. A newline-bearing name renders as $'a\nEVIL_VAR=1' on one line. validate_no_eval_in_workflow_run_blocks makes the class unrepresentable rather than a review habit: the live gate passes now, fails with the file and line when an eval is reintroduced, and passes again once reverted. Four unit tests cover the failing shape, the argv shape, a false-positive guard (`evaluate_results.sh`), and the live tree. Co-Authored-By: Claude Fable 5 --- .github/workflows/reborn-tests.yml | 38 ++++++++++++---- scripts/ci/test_ws12_workflow_contracts.py | 50 ++++++++++++++++++++++ scripts/ci/ws12_workflow_contracts.py | 32 ++++++++++++++ 3 files changed, 112 insertions(+), 8 deletions(-) diff --git a/.github/workflows/reborn-tests.yml b/.github/workflows/reborn-tests.yml index aedfd465f2f..684d7f14b76 100644 --- a/.github/workflows/reborn-tests.yml +++ b/.github/workflows/reborn-tests.yml @@ -418,18 +418,34 @@ jobs: if [[ -n "${feature_flags}" ]]; then read -ra target_feature_args <<< "${feature_flags}" fi - REPRO="scripts/ci/run-hermetic-deterministic-suite.sh command timeout --signal=INT --kill-after=30s 28m ${incremental_env[*]} cargo nextest run --profile ci -p ${package} --${kind} ${name} ${target_feature_args[*]} --all-targets --ignore-rust-version" + # Run the argv directly; never eval. `name` is a changed + # file's stem, so a path like `tests/$(cmd).rs` would be + # re-parsed as shell here. Deriving the REPRO line FROM the + # argv with %q makes the hint both injection-proof and + # genuinely paste-able, and keeps it single-line so it + # cannot forge extra $GITHUB_ENV entries. + cmd=(scripts/ci/run-hermetic-deterministic-suite.sh command + timeout --signal=INT --kill-after=30s 28m + "${incremental_env[@]}" cargo nextest run --profile ci + -p "${package}" "--${kind}" "${name}" + "${target_feature_args[@]}" --all-targets --ignore-rust-version) + printf -v REPRO '%q ' "${cmd[@]}" echo "REPRO=${REPRO}" >> "$GITHUB_ENV" - eval "${REPRO}" + "${cmd[@]}" cp target/nextest/ci/junit.xml "junit-reports/${BUCKET_NAME}-${name}.xml" 2>/dev/null || true done < <( jq -r '.[] | [.package, .kind, .name] | @tsv' \ <<< "${EXACT_TARGETS}" ) else - REPRO="scripts/ci/run-hermetic-deterministic-suite.sh command timeout --signal=INT --kill-after=30s 28m ${incremental_env[*]} cargo nextest run --profile ci ${package_args[*]} ${feature_args[*]} --all-targets --ignore-rust-version" + cmd=(scripts/ci/run-hermetic-deterministic-suite.sh command + timeout --signal=INT --kill-after=30s 28m + "${incremental_env[@]}" cargo nextest run --profile ci + "${package_args[@]}" "${feature_args[@]}" + --all-targets --ignore-rust-version) + printf -v REPRO '%q ' "${cmd[@]}" echo "REPRO=${REPRO}" >> "$GITHUB_ENV" - eval "${REPRO}" + "${cmd[@]}" cp target/nextest/ci/junit.xml "junit-reports/${BUCKET_NAME}-bulk.xml" 2>/dev/null || true fi echo "::endgroup::" @@ -567,9 +583,14 @@ jobs: - name: Run Reborn root tests run: | - REPRO='REBORN_ROOT_TEST_PARTITIONS=4 REBORN_ROOT_TEST_PARTITION=${{ matrix.partition }} RUST_MIN_STACK=67108864 scripts/ci/run-hermetic-deterministic-suite.sh command scripts/ci/run-reborn-root-partition.sh' + cmd=(env REBORN_ROOT_TEST_PARTITIONS=4 + "REBORN_ROOT_TEST_PARTITION=${{ matrix.partition }}" + RUST_MIN_STACK=67108864 + scripts/ci/run-hermetic-deterministic-suite.sh command + scripts/ci/run-reborn-root-partition.sh) + printf -v REPRO '%q ' "${cmd[@]}" echo "REPRO=${REPRO}" >> "$GITHUB_ENV" - eval "${REPRO}" + "${cmd[@]}" - name: Local repro for this job if: failure() @@ -681,9 +702,10 @@ jobs: - name: Run Reborn group tests run: | - REPRO='scripts/ci/run-hermetic-deterministic-suite.sh groups' + cmd=(scripts/ci/run-hermetic-deterministic-suite.sh groups) + printf -v REPRO '%q ' "${cmd[@]}" echo "REPRO=${REPRO}" >> "$GITHUB_ENV" - eval "${REPRO}" + "${cmd[@]}" - name: Local repro for this job if: failure() diff --git a/scripts/ci/test_ws12_workflow_contracts.py b/scripts/ci/test_ws12_workflow_contracts.py index 36fdf1a6d5a..71e269bfa1d 100755 --- a/scripts/ci/test_ws12_workflow_contracts.py +++ b/scripts/ci/test_ws12_workflow_contracts.py @@ -11,6 +11,7 @@ import ws12_workflow_contracts from ws12_workflow_contracts import ( + validate_no_eval_in_workflow_run_blocks, CODE_STYLE_WORKFLOW, CRATE_NAME_RESIDUE, CRATE_SCOPE_FILTERS, @@ -2125,5 +2126,54 @@ def test_libsql_job_failures_reach_the_top_level_contract(self) -> None: self.assertTrue(any("fixed sequential loop" in error for error in errors), errors) + +class NoEvalInWorkflowRunBlocksTests(unittest.TestCase): + """Workflows build argv arrays; they never re-parse a command string. + + The crate-bucket lane interpolates planner-derived target names, and + those come from changed FILENAMES -- `tests/$(cmd).rs` under `eval` + executes on a runner holding sccache credentials. + """ + + def test_eval_in_a_run_block_fails(self): + workflows = { + ".github/workflows/x.yml": ( + " run: |\n" + ' REPRO="cargo test ${name}"\n' + ' eval "${REPRO}"\n' + ) + } + errors = validate_no_eval_in_workflow_run_blocks(workflows) + self.assertTrue(any("x.yml:3" in e for e in errors), errors) + + def test_argv_array_invocation_passes(self): + workflows = { + ".github/workflows/x.yml": ( + " run: |\n" + ' cmd=(cargo test -p "${package}")\n' + " printf -v REPRO '%q ' \"${cmd[@]}\"\n" + ' "${cmd[@]}"\n' + ) + } + self.assertEqual( + [], validate_no_eval_in_workflow_run_blocks(workflows) + ) + + def test_the_word_eval_inside_a_longer_token_is_not_flagged(self): + workflows = { + ".github/workflows/x.yml": " evaluate_results.sh\n" + } + self.assertEqual( + [], validate_no_eval_in_workflow_run_blocks(workflows) + ) + + def test_live_workflows_contain_no_eval(self): + workflows = ws12_workflow_contracts.load_workflows(ROOT) + self.assertEqual( + [], validate_no_eval_in_workflow_run_blocks(workflows) + ) + + + if __name__ == "__main__": unittest.main() diff --git a/scripts/ci/ws12_workflow_contracts.py b/scripts/ci/ws12_workflow_contracts.py index 876d0466383..5f4fdd6647a 100755 --- a/scripts/ci/ws12_workflow_contracts.py +++ b/scripts/ci/ws12_workflow_contracts.py @@ -1750,6 +1750,37 @@ def validate_crate_name_residue( return errors +WORKFLOW_EVAL = re.compile(r"^[ \t]*eval[ \t]", re.MULTILINE) + + +def validate_no_eval_in_workflow_run_blocks( + workflows: dict[str, str], +) -> list[str]: + """No workflow may `eval` a command it just built as a string. + + Commands are assembled as argv arrays and run directly; the REPRO hint + is derived FROM the argv with printf '%q '. `eval` re-parses whatever + was interpolated, and the crate-bucket lane interpolates + planner-derived target names that come from changed FILENAMES -- so a + contributor adding `tests/$(cmd).rs` would execute `cmd` on a runner + that holds sccache credentials. A newline in the same value can also + forge extra $GITHUB_ENV entries. Both become unrepresentable once the + argv is never re-parsed, so this stays a hard contract rather than a + review habit. + """ + + errors: list[str] = [] + for path, text in workflows.items(): + for match in WORKFLOW_EVAL.finditer(text): + line = text[: match.start()].count("\n") + 1 + errors.append( + f"{path}:{line}: workflow run block uses `eval`. Build the " + "command as an array and run it directly; derive any REPRO " + "hint from the argv with printf '%q '." + ) + return errors + + def validate_workflow_texts( workflows: dict[str, str], root: Path = ROOT ) -> list[str]: @@ -1780,6 +1811,7 @@ def validate_workflow_texts( errors.extend(validate_crate_scope_filters(workflows, root)) errors.extend(validate_crate_name_residue(workflows, root)) errors.extend(validate_webui_frontend_sites(workflows, root)) + errors.extend(validate_no_eval_in_workflow_run_blocks(workflows)) try: sccache_action = (root / SCCACHE_SETUP_ACTION).read_text(encoding="utf-8") except OSError as error: From 78bbdf1ff43ee04f74f447203484a022a6c86b51 Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Sun, 23 Aug 2026 17:21:34 +0000 Subject: [PATCH 10/19] test(ci): pin the crate-bucket arms as argv, not as an eval-able string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two assertions in test_workflow_consumes_plan_and_bounds_each_rust_matrix pinned the flat `${incremental_env[*]} ... ${name}` / `${feature_args[*]}` string form of the crate-bucket commands. That form is exactly what made the previous commit's injection possible, so re-pinning it verbatim would have re-invited the shape the fix removes. Both pins now assert the quoted-array form the workflow actually uses, and two assertions are added rather than removed: `printf -v REPRO '%q '` must be how the hint is produced, and `eval "${REPRO}"` must not appear. The assertions' intent (feature/package args and both flags reach the bulk arm; package/kind/name reach the exact-target arm) is unchanged — only the shape they pin moved from string to argv. Verified: test_reborn_pr_test_plan.py 87 OK, test_ws12_workflow_contracts.py 98 OK, ws12 live gate passed, reborn-tests.yml parses. Co-Authored-By: Claude Fable 5 --- scripts/ci/test_reborn_pr_test_plan.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/scripts/ci/test_reborn_pr_test_plan.py b/scripts/ci/test_reborn_pr_test_plan.py index afa0bb1cbe3..1cf4f769541 100644 --- a/scripts/ci/test_reborn_pr_test_plan.py +++ b/scripts/ci/test_reborn_pr_test_plan.py @@ -2088,10 +2088,20 @@ def test_workflow_consumes_plan_and_bounds_each_rust_matrix(self) -> None: workflow, ) self.assertIn("--test reborn_integration_sandbox_shell_turn", workflow) + # The bulk crate-bucket arm passes the package/feature args and both + # flags. It is asserted as ARGV (quoted `[@]` expansions inside a + # `cmd=(...)` array), not as a flat `[*]` string: the string form was + # eval-ed, and a planner-derived target name comes from a changed + # filename, so `tests/$(cmd).rs` executed on the runner. Keep this + # pinned to the array form so the injectable shape cannot return + # silently (validate_no_eval_in_workflow_run_blocks enforces the rest). self.assertIn( - '${feature_args[*]} --all-targets --ignore-rust-version', + '"${package_args[@]}" "${feature_args[@]}"\n' + ' --all-targets --ignore-rust-version)', workflow, ) + self.assertIn('printf -v REPRO \'%q \' "${cmd[@]}"', workflow) + self.assertNotIn('eval "${REPRO}"', workflow) self.assertIn( 'cargo llvm-cov --branch --skip-functions \\\n' ' "${package_args[@]}" "${feature_args[@]}"', @@ -2146,9 +2156,12 @@ def test_workflow_consumes_plan_and_bounds_each_rust_matrix(self) -> None: "${{ toJSON(matrix.bucket.exact_targets || fromJSON('[]')) }}", workflow, ) + # Exact-target arm, pinned as ARGV. `name` is a changed file's stem, + # so the previous flat-string form was an injection sink once it was + # eval-ed; keep the quoted array shape pinned here. self.assertIn( - "${incremental_env[*]} cargo nextest run --profile ci " - "-p ${package} --${kind} ${name}", + '"${incremental_env[@]}" cargo nextest run --profile ci\n' + ' -p "${package}" "--${kind}" "${name}"', workflow, ) From 5dda762c8b7cb48bcfecbcf613745ef90a0b54eb Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Sun, 23 Aug 2026 19:50:01 +0000 Subject: [PATCH 11/19] fix(ci): unbreak the JUnit uploads and stop the planner's narrowing regressing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five review findings on the nextest lane, all in the reporting path this PR added: - The JUnit upload in reborn-integration-coverage declared `if-no-files-found` TWICE (`ignore` then `error`). YAML keeps the last key, so the step was pinned to `error` — and coverage lanes run `cargo llvm-cov`, which never writes target/nextest/ci/junit.xml, so every coverage run would have hard-failed on a missing file. Keeps `ignore`, matching the sibling bucket/root uploads. - The exact-target arm appended `--all-targets`, re-broadening exactly the run the planner had narrowed via `-p`/`--test`. Dropped there; kept on the bulk arm where it belongs. - Root partitions and coverage lanes both uploaded an inner file named plain `junit.xml`, and the roll-up downloads `reborn-junit-*` with `merge-multiple: true`, which flattens them into one directory — every leg silently overwrote the previous one. Both now stage under a partition/lane-unique name first, as the crate-bucket arm already did. - A failing test run aborted the step before the `cp` that stages its report, losing precisely the JUnit file worth reading. The status is now captured, the report staged, and the original exit code re-raised — the failure still fails. - junit_summary.py parsed the report with entity resolution enabled. Test names and failure messages come from the branch under test, so a billion-laughs payload would hang the summary step. It now refuses a DTD outright (dependency-free; defusedxml is not a dependency here). tests/CLAUDE.md's counts are updated (39 -> 40 top-level bins) with a row for hermetic_network_guard_probe.rs, per its same-commit maintenance rule. validate_no_duplicate_yaml_keys closes the class rather than this one instance: PyYAML's safe_load silently keeps the last duplicate, so the local `yaml.safe_load` check I had been running passed while the intended value was discarded. Three unit tests cover it — a duplicated `if-no-files-found` reported with file and line, a clean workflow, and the live tree — and the gate was proven to fail with file+line when a duplicate is reintroduced and pass again once removed. Verified: ws12 self-tests 101 OK, ws12 live gate passed, planner suite 87 OK, check-guidance OK, both workflows parse under a strict no-duplicate-key loader. Co-Authored-By: Claude Fable 5 --- .github/workflows/reborn-tests.yml | 39 +++++++++++++--- scripts/ci/junit_summary.py | 10 +++++ scripts/ci/test_ws12_workflow_contracts.py | 52 ++++++++++++++++++++++ scripts/ci/ws12_workflow_contracts.py | 47 +++++++++++++++++++ tests/CLAUDE.md | 6 +-- 5 files changed, 145 insertions(+), 9 deletions(-) diff --git a/.github/workflows/reborn-tests.yml b/.github/workflows/reborn-tests.yml index 684d7f14b76..71c130193bb 100644 --- a/.github/workflows/reborn-tests.yml +++ b/.github/workflows/reborn-tests.yml @@ -428,11 +428,16 @@ jobs: timeout --signal=INT --kill-after=30s 28m "${incremental_env[@]}" cargo nextest run --profile ci -p "${package}" "--${kind}" "${name}" - "${target_feature_args[@]}" --all-targets --ignore-rust-version) + "${target_feature_args[@]}" --ignore-rust-version) printf -v REPRO '%q ' "${cmd[@]}" echo "REPRO=${REPRO}" >> "$GITHUB_ENV" - "${cmd[@]}" + # Capture the status so the report of a FAILING run still gets + # staged: under `bash -e` the step would otherwise abort here + # and lose exactly the JUnit file worth reading. + status=0 + "${cmd[@]}" || status=$? cp target/nextest/ci/junit.xml "junit-reports/${BUCKET_NAME}-${name}.xml" 2>/dev/null || true + [ "${status}" -eq 0 ] || exit "${status}" done < <( jq -r '.[] | [.package, .kind, .name] | @tsv' \ <<< "${EXACT_TARGETS}" @@ -445,8 +450,10 @@ jobs: --all-targets --ignore-rust-version) printf -v REPRO '%q ' "${cmd[@]}" echo "REPRO=${REPRO}" >> "$GITHUB_ENV" - "${cmd[@]}" + status=0 + "${cmd[@]}" || status=$? cp target/nextest/ci/junit.xml "junit-reports/${BUCKET_NAME}-bulk.xml" 2>/dev/null || true + [ "${status}" -eq 0 ] || exit "${status}" fi echo "::endgroup::" fi @@ -602,12 +609,24 @@ jobs: echo '```' } | tee -a "$GITHUB_STEP_SUMMARY" + # Every lane's nextest writes the same `target/nextest/ci/junit.xml`. + # The roll-up downloads `reborn-junit-*` with `merge-multiple: true`, + # which flattens them into one directory -- identical inner filenames + # would silently overwrite each other and lose whole partitions. Stage + # under a partition-unique name first. + - name: Stage JUnit report + if: always() + run: | + mkdir -p junit-reports + cp target/nextest/ci/junit.xml \ + "junit-reports/root-${{ matrix.partition }}.xml" 2>/dev/null || true + - name: Upload JUnit report if: always() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: reborn-junit-root-${{ matrix.partition }} - path: target/nextest/ci/junit.xml + path: junit-reports/ if-no-files-found: ignore reborn-group-tests: @@ -936,14 +955,22 @@ jobs: name: reborn-integration-cov-part-${{ matrix.lane }} path: part-${{ matrix.lane }}.lcov + # Same collision as the root partitions above: stage under a + # lane-unique name before the merge-multiple download flattens them. + - name: Stage JUnit report + if: always() + run: | + mkdir -p junit-reports + cp target/nextest/ci/junit.xml \ + "junit-reports/lane-${{ matrix.lane }}.xml" 2>/dev/null || true + - name: Upload JUnit report if: always() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: reborn-junit-lane-${{ matrix.lane }} - path: target/nextest/ci/junit.xml + path: junit-reports/ if-no-files-found: ignore - if-no-files-found: error coverage-report: name: Reborn integration-tier coverage report diff --git a/scripts/ci/junit_summary.py b/scripts/ci/junit_summary.py index 86a8b2608a5..7d552a453f4 100644 --- a/scripts/ci/junit_summary.py +++ b/scripts/ci/junit_summary.py @@ -37,6 +37,16 @@ def _first_line(text: str | None) -> str: def parse_junit(path: str) -> list[FailedTest]: + # Refuse a DTD outright instead of parsing it. JUnit XML has no + # legitimate use for one, and expat expands INTERNAL entities, so a + # billion-laughs payload would hang this step. The file is CI-produced + # but its content (test names, failure messages) comes from the branch + # under test, so it is not trusted input. Dependency-free on purpose: + # defusedxml is not a dependency of this repo. + with open(path, encoding="utf-8", errors="replace") as handle: + head = handle.read(4096) + if " None: + +class NoDuplicateYamlKeysTests(unittest.TestCase): + """A duplicated mapping key silently discards the intended value. + + PyYAML's safe_load keeps the LAST duplicate, so a local + `yaml.safe_load` sanity check passes while the author's value is + dropped. That is how a JUnit upload ended up pinned to + `if-no-files-found: error`, which would have hard-failed every + coverage lane (they legitimately produce no junit.xml). + """ + + def _workflows_dir(self, root): + directory = root / ".github" / "workflows" + directory.mkdir(parents=True) + return directory + + def test_duplicate_key_is_reported_with_file_and_line(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + directory = self._workflows_dir(root) + (directory / "dupe.yml").write_text( + "jobs:\n" + " build:\n" + " with:\n" + " if-no-files-found: ignore\n" + " if-no-files-found: error\n", + encoding="utf-8", + ) + errors = validate_no_duplicate_yaml_keys(root) + self.assertTrue( + any("if-no-files-found" in e and "dupe.yml" in e for e in errors), + errors, + ) + + def test_clean_workflow_passes(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + directory = self._workflows_dir(root) + (directory / "ok.yml").write_text( + "jobs:\n" + " build:\n" + " with:\n" + " if-no-files-found: ignore\n", + encoding="utf-8", + ) + self.assertEqual([], validate_no_duplicate_yaml_keys(root)) + + def test_live_workflows_have_no_duplicate_keys(self): + self.assertEqual([], validate_no_duplicate_yaml_keys(ROOT)) + + class NoEvalInWorkflowRunBlocksTests(unittest.TestCase): """Workflows build argv arrays; they never re-parse a command string. diff --git a/scripts/ci/ws12_workflow_contracts.py b/scripts/ci/ws12_workflow_contracts.py index 5f4fdd6647a..afb52c02a0f 100755 --- a/scripts/ci/ws12_workflow_contracts.py +++ b/scripts/ci/ws12_workflow_contracts.py @@ -1781,6 +1781,52 @@ def validate_no_eval_in_workflow_run_blocks( return errors +def validate_no_duplicate_yaml_keys(root: Path = ROOT) -> list[str]: + """No workflow may declare the same mapping key twice. + + PyYAML's safe_load silently keeps the LAST duplicate, so a local + `yaml.safe_load` sanity check passes while the value the author + intended is discarded -- and GitHub's own parser is stricter. That is + exactly the green-locally/red-in-CI class this repo keeps paying for: + a duplicated `if-no-files-found` on a JUnit upload pinned the step to + `error`, so coverage lanes (which legitimately produce no junit.xml) + would have hard-failed. + """ + + import yaml + + class _NoDuplicates(yaml.SafeLoader): + pass + + errors: list[str] = [] + + def _mapping(loader, node, deep=False): + seen: dict = {} + for key_node, value_node in node.value: + key = loader.construct_object(key_node, deep=deep) + if key in seen: + raise ValueError( + f"duplicate key {key!r} at line {key_node.start_mark.line + 1}" + ) + seen[key] = loader.construct_object(value_node, deep=deep) + return seen + + _NoDuplicates.add_constructor( + yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, _mapping + ) + + for path in sorted((root / ".github" / "workflows").glob("*.y*ml")): + try: + yaml.load(path.read_text(encoding="utf-8"), _NoDuplicates) + except ValueError as error: + rel = path.relative_to(root).as_posix() + errors.append(f"{rel}: {error}") + except yaml.YAMLError as error: + rel = path.relative_to(root).as_posix() + errors.append(f"{rel}: could not parse: {error}") + return errors + + def validate_workflow_texts( workflows: dict[str, str], root: Path = ROOT ) -> list[str]: @@ -1811,6 +1857,7 @@ def validate_workflow_texts( errors.extend(validate_crate_scope_filters(workflows, root)) errors.extend(validate_crate_name_residue(workflows, root)) errors.extend(validate_webui_frontend_sites(workflows, root)) + errors.extend(validate_no_duplicate_yaml_keys(root)) errors.extend(validate_no_eval_in_workflow_run_blocks(workflows)) try: sccache_action = (root / SCCACHE_SETUP_ACTION).read_text(encoding="utf-8") diff --git a/tests/CLAUDE.md b/tests/CLAUDE.md index 6ef4c602702..3292e7b1518 100644 --- a/tests/CLAUDE.md +++ b/tests/CLAUDE.md @@ -63,7 +63,7 @@ Tier-selection rule: `.claude/rules/testing.md`. | Coverage/meta gates | — | 2 | ✓ | ✓ | Totals: **59** group scenarios · **62** flat integration bins (55 in -`tests/integration/`, 7 in `tests/integration/auth/`) · **39** top-level Rust bins · +`tests/integration/`, 7 in `tests/integration/auth/`) · **40** top-level Rust bins · **102** Python scenario files (**870** test functions) registered in the active Reborn coverage map below. Section 6 separately inventories retained and legacy Python scenarios, so its exhaustive totals are intentionally broader. @@ -303,7 +303,7 @@ channel-delivery journeys (two-lane model): --- -## 5. Binary, parity & QA-trace bins — `tests/*.rs` (39) +## 5. Binary, parity & QA-trace bins — `tests/*.rs` (40) **QA workflow phrases** — real manual-QA sentences, replayed against the Reborn binary. | The user asks… | Evidence | @@ -350,7 +350,7 @@ manual-run and scheduled-run denial evidence), **Policy & format**: `e2e_trace_runtime_policy_org_ceiling_yolo.rs` (org policy ceiling × yolo narrowing), `e2e_trace_runtime_policy_serde.rs` (wire-stable policy enums), `trace_format.rs`, `trace_llm_tests.rs`, -`reborn_coverage_lane_stack_headroom.rs` (CI job must declare stack headroom). +`reborn_coverage_lane_stack_headroom.rs` (CI job must declare stack headroom), `hermetic_network_guard_probe.rs` (the hermetic wrapper's network guard must survive nextest's own process spawning, not just a directly-launched binary; `#[ignore]`d — driven by scripts/ci/test-hermetic-test-process.sh under IRONCLAW_HERMETIC_NEXTEST_CONTROL=1). --- From fa1796e941e8601ac1cbe34fe91953ccc378a7fe Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Sun, 23 Aug 2026 22:59:10 +0000 Subject: [PATCH 12/19] fix(ci): keep the duplicate-key gate stdlib-only and register the guard probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two failures this PR's own CI caught that no local run could. 1. The duplicate-key validator imported PyYAML. No checker under scripts/ci does — they are deliberately stdlib-only — and the fast-checks job installs no such dependency, so the import raised inside validate_workflow_texts and errored 70 self-tests in CI while passing locally. Exactly the green-locally/red-in-CI class the gate exists to catch, introduced by the gate itself. Replaced with a stdlib line scanner tracking one key set per mapping block. PyYAML was the wrong tool anyway: safe_load silently keeps the LAST duplicate, so a `yaml.safe_load` check passes while the author's value is discarded — which is how a duplicated `if-no-files-found` pinned a JUnit upload to `error` and would have hard-failed every coverage lane. The scanner's first draft mis-parsed `- &anchor` list items, whose marker carries no key, and reported two false duplicates in release-plz.yml. Element boundaries are now tracked whether or not the marker carries its key inline, and block scalars are skipped so a `run: |` body's prose is never read as YAML. 2. tests/hermetic_network_guard_probe.rs, added earlier in this branch, was not in the planner's root inventory: the glob matches `tests/reborn_*.rs` plus a short extras list, so the fail-closed arm rejected it with "unmapped test or CI path" and failed Detect Reborn test scope for the whole PR. It joins the extras beside dockerfile_runtime_home — the right lane, since the hermetic control that drives it runs in the root-partition job. Regression tests: anchor-only list items and block-scalar bodies must not false-positive, the checker must not import yaml, and the probe must appear in the root inventory. All four fail against the respective bug. Verified: ws12 self-tests 104 OK, ws12 live gate passed, planner suite 88 OK, check-guidance OK, and the planner now succeeds on this PR's own changed-file list — the input that was failing. Co-Authored-By: Claude Fable 5 --- scripts/ci/reborn_pr_test_plan.py | 11 ++- scripts/ci/test_reborn_pr_test_plan.py | 13 +++ scripts/ci/test_ws12_workflow_contracts.py | 47 +++++++++ scripts/ci/ws12_workflow_contracts.py | 110 ++++++++++++++------- 4 files changed, 143 insertions(+), 38 deletions(-) diff --git a/scripts/ci/reborn_pr_test_plan.py b/scripts/ci/reborn_pr_test_plan.py index 199998804ac..f61eb809cc9 100644 --- a/scripts/ci/reborn_pr_test_plan.py +++ b/scripts/ci/reborn_pr_test_plan.py @@ -751,7 +751,16 @@ def _bound_pr_buckets( def _root_test_partitions() -> dict[str, int]: extra_tests = [ name - for name in ("dockerfile_runtime_home", "support_unit_tests") + for name in ( + "dockerfile_runtime_home", + "support_unit_tests", + # Not `reborn_`-prefixed, so the glob above misses it. It is the + # fixture the hermetic network-guard control drives, and that + # control runs in the root-partition job -- so a change to the + # probe must select the lane that exercises it. Without this the + # fail-closed arm rejects the path outright. + "hermetic_network_guard_probe", + ) if (ROOT / f"tests/{name}.rs").is_file() ] names = sorted( diff --git a/scripts/ci/test_reborn_pr_test_plan.py b/scripts/ci/test_reborn_pr_test_plan.py index 3e7aaa9adac..51d0ec05a5f 100644 --- a/scripts/ci/test_reborn_pr_test_plan.py +++ b/scripts/ci/test_reborn_pr_test_plan.py @@ -2308,5 +2308,18 @@ def test_main_coverage_cancels_superseded_commits(self) -> None: self.assertIn("cancel-in-progress: true", workflow) + def test_hermetic_guard_probe_is_a_known_root_test(self) -> None: + """A new top-level test must be registered or the planner fails closed. + + tests/hermetic_network_guard_probe.rs is not `reborn_`-prefixed, so + the inventory glob misses it; before it joined the extras list the + fail-closed arm rejected the path with "unmapped test or CI path" + and reddened Detect Reborn test scope for the whole PR. + """ + partitions = planner._root_test_partitions() + self.assertIn("tests/hermetic_network_guard_probe.rs", partitions) + + + if __name__ == "__main__": unittest.main() diff --git a/scripts/ci/test_ws12_workflow_contracts.py b/scripts/ci/test_ws12_workflow_contracts.py index 67f667821ad..e6c415752b7 100755 --- a/scripts/ci/test_ws12_workflow_contracts.py +++ b/scripts/ci/test_ws12_workflow_contracts.py @@ -2175,6 +2175,53 @@ def test_clean_workflow_passes(self): ) self.assertEqual([], validate_no_duplicate_yaml_keys(root)) + def test_anchor_only_list_items_are_not_false_positives(self): + """`- &anchor` opens an item without carrying a key on that line. + + The first scanner draft missed that boundary and reported the next + item's `name:`/`uses:` as duplicates of the previous item's. + """ + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + directory = self._workflows_dir(root) + (directory / "anchors.yml").write_text( + "jobs:\n" + " release:\n" + " steps:\n" + " - &checkout\n" + " name: Checkout repository\n" + " uses: actions/checkout@v6\n" + " - &install\n" + " name: Install Rust toolchain\n" + " uses: dtolnay/rust-toolchain@stable\n", + encoding="utf-8", + ) + self.assertEqual([], validate_no_duplicate_yaml_keys(root)) + + def test_block_scalar_text_is_not_parsed_as_yaml(self): + """A `run: |` body may contain `key:`-looking lines; they are prose.""" + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + directory = self._workflows_dir(root) + (directory / "scalar.yml").write_text( + "jobs:\n" + " build:\n" + " steps:\n" + " - name: Echo\n" + " run: |\n" + " echo 'name: not-a-key'\n" + " echo 'name: still-not-a-key'\n", + encoding="utf-8", + ) + self.assertEqual([], validate_no_duplicate_yaml_keys(root)) + + def test_the_checker_needs_no_third_party_yaml_module(self): + """scripts/ci checkers are stdlib-only; PyYAML is absent in fast-checks.""" + source = ( + Path(ws12_workflow_contracts.__file__).read_text(encoding="utf-8") + ) + self.assertNotIn("import yaml", source) + def test_live_workflows_have_no_duplicate_keys(self): self.assertEqual([], validate_no_duplicate_yaml_keys(ROOT)) diff --git a/scripts/ci/ws12_workflow_contracts.py b/scripts/ci/ws12_workflow_contracts.py index 849e7226917..cdef35c1b43 100755 --- a/scripts/ci/ws12_workflow_contracts.py +++ b/scripts/ci/ws12_workflow_contracts.py @@ -1786,49 +1786,85 @@ def validate_no_eval_in_workflow_run_blocks( return errors -def validate_no_duplicate_yaml_keys(root: Path = ROOT) -> list[str]: - """No workflow may declare the same mapping key twice. - - PyYAML's safe_load silently keeps the LAST duplicate, so a local - `yaml.safe_load` sanity check passes while the value the author - intended is discarded -- and GitHub's own parser is stricter. That is - exactly the green-locally/red-in-CI class this repo keeps paying for: - a duplicated `if-no-files-found` on a JUnit upload pinned the step to - `error`, so coverage lanes (which legitimately produce no junit.xml) - would have hard-failed. - """ +KEY_LINE = re.compile( + r"^(?P[ ]*)(?P-[ ]+)?(?P\"[^\"]+\"|'[^']+'|[A-Za-z_][\w.\-]*)[ ]*:" + r"(?P[ ].*|)$" +) +BLOCK_SCALAR = re.compile(r"^[|>][+-]?[0-9]*[ ]*(#.*)?$") - import yaml - class _NoDuplicates(yaml.SafeLoader): - pass +def validate_no_duplicate_yaml_keys(root: Path = ROOT) -> list[str]: + """No workflow may declare the same mapping key twice in one block. + + Deliberately stdlib-only: every checker in scripts/ci is, and adding + PyYAML here would make the gate depend on a package the fast-checks job + does not install (learned the hard way -- an earlier PyYAML version of + this function errored 70 self-tests in CI while passing locally). + + It is also the wrong tool: PyYAML's safe_load silently keeps the LAST + duplicate, so a local `yaml.safe_load` sanity check passes while the + value the author wrote is discarded -- which is how a duplicated + `if-no-files-found` pinned a JUnit upload to `error` and would have + hard-failed every coverage lane. + + The scan tracks one key set per mapping block, keyed by indent. A `- ` + item opens a fresh block, and block scalars (`|`, `>`) are skipped so + their free text is never parsed as YAML. + """ errors: list[str] = [] - - def _mapping(loader, node, deep=False): - seen: dict = {} - for key_node, value_node in node.value: - key = loader.construct_object(key_node, deep=deep) + for path in sorted((root / ".github" / "workflows").glob("*.y*ml")): + rel = path.relative_to(root).as_posix() + scopes: list[tuple[int, set[str]]] = [] + skip_until_indent: int | None = None + pending_item: int | None = None + for number, raw in enumerate( + path.read_text(encoding="utf-8").splitlines(), start=1 + ): + stripped = raw.strip() + if not stripped or stripped.startswith("#"): + continue + indent = len(raw) - len(raw.lstrip(" ")) + if skip_until_indent is not None: + if indent > skip_until_indent: + continue + skip_until_indent = None + # A list element always opens a fresh mapping. The marker may + # carry its key inline (`- name: x`) or stand alone with only an + # anchor (`- &checkout`), so record the boundary either way. + if stripped.startswith("-"): + while scopes and scopes[-1][0] > indent: + scopes.pop() + pending_item = indent + match = KEY_LINE.match(raw) + if match is None: + continue + key = match.group("key").strip("\"'") + dash = match.group("dash") + effective = indent + (len(dash) if dash else 0) + while scopes and scopes[-1][0] > effective: + scopes.pop() + starts_item = bool(dash) or ( + pending_item is not None and effective > pending_item + ) + if starts_item: + while scopes and scopes[-1][0] >= effective: + scopes.pop() + scopes.append((effective, set())) + pending_item = None + elif not scopes or scopes[-1][0] < effective: + scopes.append((effective, set())) + seen = scopes[-1][1] if key in seen: - raise ValueError( - f"duplicate key {key!r} at line {key_node.start_mark.line + 1}" + errors.append( + f"{rel}:{number}: duplicate key {key!r} in the same mapping. " + "YAML keeps the last one, so the value above it is silently " + "discarded." ) - seen[key] = loader.construct_object(value_node, deep=deep) - return seen - - _NoDuplicates.add_constructor( - yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, _mapping - ) - - for path in sorted((root / ".github" / "workflows").glob("*.y*ml")): - try: - yaml.load(path.read_text(encoding="utf-8"), _NoDuplicates) - except ValueError as error: - rel = path.relative_to(root).as_posix() - errors.append(f"{rel}: {error}") - except yaml.YAMLError as error: - rel = path.relative_to(root).as_posix() - errors.append(f"{rel}: could not parse: {error}") + else: + seen.add(key) + if BLOCK_SCALAR.match(match.group("rest").strip()): + skip_until_indent = effective return errors From 32985168654d67818c9e07e78a4b695d209ab808 Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Mon, 24 Aug 2026 01:47:01 +0000 Subject: [PATCH 13/19] fix(ci): serialise the CLI smoke tests that boot a real serve listener MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first real nextest run on this branch surfaced exactly one failure out of 1417 tests: onboard_login_link_then_bearer_authorizes_a_protected_request died with "connect to serve listener failed: Connection refused" in the reborn-core bucket. Cause is an interaction, not a broken test. smoke.rs's `unused_local_port` binds :0, reads the assigned port, then RELEASES it before the spawned `serve` child re-binds — a TOCTOU window. Its own comment refuses to grow past that ("a small serialization fix, not a port-reservation framework — do not extend it into a pool or retry-with-backoff mechanism"). Under `cargo test` the 20 listener tests shared one process and the window stayed narrow; nextest gives each test its own process, widening it until two race for the same port and the loser cannot connect. This follows the rule the plan set for exactly this case: a nextest-only failure is serialised in .config/nextest.toml, never "fixed" by editing the test. A `cli-serve-listener` group (max-threads = 1) takes the smoke binary's serve/onboard tests; the other ~128 smoke tests stay parallel. The filter is a deliberately slightly over-inclusive glob rather than a list of 20 names, so a future serve_*/onboard_* test joins the group automatically instead of reintroducing the race. scripts/ci/test_nextest_config.py pins it with four assertions: the group exists, keeps max-threads = 1, is bound in BOTH profiles, and still covers the test that actually failed. Proven red-first — relaxing the bound to 4 fails it, restoring passes. Verified: `cargo nextest show-config test-groups --profile ci` resolves the group and binds the intended tests; planner suite 88 OK; ws12 self-tests 104 OK; ws12 live gate passed. This is what the throwaway exercise branches (#7838/#7839) were opened to find. It arrived early because registering the guard probe flipped this PR's own plan from "no Rust lanes" to the full matrix — the PR now tests itself. Co-Authored-By: Claude Fable 5 --- .config/nextest.toml | 29 ++++++++++ .github/workflows/code_style.yml | 1 + scripts/ci/test_nextest_config.py | 93 +++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+) create mode 100755 scripts/ci/test_nextest_config.py diff --git a/.config/nextest.toml b/.config/nextest.toml index 564e9fd6663..4f0e10e08ed 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -43,6 +43,35 @@ test-threads = "num-cpus" # aggregator (Task 1) to report failing TEST names, not just job names. path = "junit.xml" +# --- Serialised groups ------------------------------------------------- +# +# `ironclaw`'s CLI smoke tests boot a real `serve` listener. Their helper +# picks a port by binding :0, reading the number, then RELEASING it before +# the child re-binds -- a TOCTOU window the helper's own comment refuses to +# paper over ("a small serialization fix, not a port-reservation +# framework -- do not extend it into a pool or retry-with-backoff +# mechanism"). Under `cargo test` those 20 tests shared one process and the +# window stayed narrow; nextest runs each test in its own process, widening +# it until two tests race for the same port and the loser reports +# "connect to serve listener failed: Connection refused". +# +# Observed on this branch: onboard_login_link_then_bearer_authorizes_a_ +# protected_request, 1 of 1417 tests, in the reborn-core bucket. +# +# The rule this follows is the plan's own: a nextest-only failure gets +# serialised here, never "fixed" by editing the test. Only the listener +# tests join the group -- the other ~128 smoke tests stay parallel. +[test-groups] +cli-serve-listener = { max-threads = 1 } + +[[profile.default.overrides]] +filter = 'package(ironclaw) & binary(smoke) & (test(~serve) | test(~onboard_) | test(~stored_key_reaches) | test(a_real_env_var_beats_the_config_default_end_to_end))' +test-group = "cli-serve-listener" + +[[profile.ci.overrides]] +filter = 'package(ironclaw) & binary(smoke) & (test(~serve) | test(~onboard_) | test(~stored_key_reaches) | test(a_real_env_var_beats_the_config_default_end_to_end))' +test-group = "cli-serve-listener" + # Per-test windows for the handful of scenarios that genuinely need more # than 60s (live-recorded fixtures + heavy integration). [[profile.default.overrides]] diff --git a/.github/workflows/code_style.yml b/.github/workflows/code_style.yml index 371386b940f..635e6695cce 100644 --- a/.github/workflows/code_style.yml +++ b/.github/workflows/code_style.yml @@ -218,6 +218,7 @@ jobs: python3 scripts/ci/test_cut_ironclaw_release.py scripts/ci/test-hermetic-test-process.sh scripts/ci/test-hermetic-nextest-control-gating.sh + python3 scripts/ci/test_nextest_config.py scripts/ci/test-reborn-docker-entrypoint.sh # #7144: this 204-test module had never been run by any lane, so five # of its assertions had silently drifted out of sync with the code they diff --git a/scripts/ci/test_nextest_config.py b/scripts/ci/test_nextest_config.py new file mode 100755 index 00000000000..6d203eff7d8 --- /dev/null +++ b/scripts/ci/test_nextest_config.py @@ -0,0 +1,93 @@ +#!/usr/bin/env python3 +"""Pin the nextest serialisation that keeps port-racing tests deterministic. + +`crates/app/ironclaw_cli/tests/smoke.rs` boots a real `serve` listener. Its +`unused_local_port` helper binds :0, reads the port, then releases it before +the spawned child re-binds — a TOCTOU window the helper's own comment +declines to grow past. Under `cargo test` those tests shared one process and +the window stayed narrow; nextest runs each test in its own process, and the +window widened until two raced for the same port and one reported +"connect to serve listener failed: Connection refused". + +The fix is serialisation in .config/nextest.toml, not a test edit. These +tests fail if that group is removed, loses its single-thread bound, stops +applying to a profile, or stops covering the test that actually failed. +""" + +from __future__ import annotations + +import tomllib +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CONFIG = ROOT / ".config" / "nextest.toml" +GROUP = "cli-serve-listener" +# The test that failed in CI, plus one representative of each other shape the +# filter must keep covering. +MUST_COVER = ( + "onboard_login_link_then_bearer_authorizes_a_protected_request", + "serve_boots_without_user_id_env_var", + "stored_key_reaches_real_turn_via_product_surface", +) +GLOB_FRAGMENTS = ("serve", "onboard_", "stored_key_reaches") + + +def _config() -> dict: + return tomllib.loads(CONFIG.read_text(encoding="utf-8")) + + +def _bound_filters(config: dict, profile: str) -> str: + overrides = config.get("profile", {}).get(profile, {}).get("overrides", []) + return " ".join( + override.get("filter", "") + for override in overrides + if override.get("test-group") == GROUP + ) + + +class NextestSerialisationTests(unittest.TestCase): + def test_the_listener_group_exists(self) -> None: + groups = _config().get("test-groups", {}) + self.assertIn( + GROUP, + groups, + "the CLI smoke listener tests race for a port without this group", + ) + + def test_the_group_stays_single_threaded(self) -> None: + group = _config().get("test-groups", {}).get(GROUP, {}) + self.assertEqual( + 1, + group.get("max-threads"), + "more than one thread reopens the port race this group closed", + ) + + def test_both_profiles_bind_the_group(self) -> None: + config = _config() + for profile in ("default", "ci"): + with self.subTest(profile=profile): + self.assertNotEqual( + "", + _bound_filters(config, profile).strip(), + f"profile {profile!r} would run the listener tests in parallel", + ) + + def test_the_filter_still_covers_the_test_that_failed(self) -> None: + config = _config() + for profile in ("default", "ci"): + expression = _bound_filters(config, profile) + for name in MUST_COVER: + with self.subTest(profile=profile, test=name): + covered = name in expression or any( + fragment in name + for fragment in GLOB_FRAGMENTS + if f"test(~{fragment})" in expression + ) + self.assertTrue( + covered, f"{name} is no longer serialised in {profile!r}" + ) + + +if __name__ == "__main__": + unittest.main() From 36133d8e6b70bc6f699b3167876a8b5db6943144 Mon Sep 17 00:00:00 2001 From: Henry Park Date: Tue, 25 Aug 2026 00:25:10 +0000 Subject: [PATCH 14/19] Address PR review feedback (#7817) - preserve complete JUnit and replay evidence across failing lanes - harden workflow and XML guardrails with executable regressions - keep nextest runner selection and group suites on canonical paths --- .github/workflows/code_style.yml | 8 +- .github/workflows/reborn-tests.yml | 74 +++++----- scripts/ci/junit_summary.py | 30 ++-- .../ci/run-hermetic-deterministic-suite.sh | 21 ++- ...est-hermetic-deterministic-suite-runner.sh | 139 ++++++++++++++++++ .../test-hermetic-nextest-control-gating.sh | 22 ++- scripts/ci/test-run-reborn-group-tests.sh | 66 +++++++++ scripts/ci/test-write-local-repro-summary.sh | 17 +++ scripts/ci/test_junit_summary.py | 40 +++++ scripts/ci/test_reborn_pr_test_plan.py | 60 ++++++++ scripts/ci/test_ws12_workflow_contracts.py | 87 ++++++++++- scripts/ci/write-local-repro-summary.sh | 15 ++ scripts/ci/ws12_workflow_contracts.py | 54 +++---- tests/AGENTS.md | 2 +- tests/hermetic_network_guard_probe.rs | 4 +- 15 files changed, 543 insertions(+), 96 deletions(-) mode change 100644 => 100755 scripts/ci/junit_summary.py create mode 100755 scripts/ci/test-hermetic-deterministic-suite-runner.sh create mode 100755 scripts/ci/test-run-reborn-group-tests.sh create mode 100755 scripts/ci/test-write-local-repro-summary.sh mode change 100644 => 100755 scripts/ci/test_junit_summary.py mode change 100644 => 100755 scripts/ci/test_reborn_pr_test_plan.py create mode 100755 scripts/ci/write-local-repro-summary.sh diff --git a/.github/workflows/code_style.yml b/.github/workflows/code_style.yml index 635e6695cce..e18e6c999a1 100644 --- a/.github/workflows/code_style.yml +++ b/.github/workflows/code_style.yml @@ -165,10 +165,6 @@ jobs: uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable with: components: rustfmt - - name: Install cargo-nextest - uses: taiki-e/install-action@62b0f2dec647a8e604c6a0fda0e38530180dce20 # v2 - with: - tool: cargo-nextest@0.9.143 - name: Install Python uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: @@ -218,7 +214,11 @@ jobs: python3 scripts/ci/test_cut_ironclaw_release.py scripts/ci/test-hermetic-test-process.sh scripts/ci/test-hermetic-nextest-control-gating.sh + scripts/ci/test-hermetic-deterministic-suite-runner.sh + scripts/ci/test-run-reborn-group-tests.sh + scripts/ci/test-write-local-repro-summary.sh python3 scripts/ci/test_nextest_config.py + python3 scripts/ci/test_junit_summary.py scripts/ci/test-reborn-docker-entrypoint.sh # #7144: this 204-test module had never been run by any lane, so five # of its assertions had silently drifted out of sync with the code they diff --git a/.github/workflows/reborn-tests.yml b/.github/workflows/reborn-tests.yml index 71c130193bb..c9fde3b4af1 100644 --- a/.github/workflows/reborn-tests.yml +++ b/.github/workflows/reborn-tests.yml @@ -412,7 +412,10 @@ jobs: echo "::group::cargo test affected bucket ${BUCKET_NAME}" mkdir -p junit-reports if [[ "$(jq 'length' <<< "${EXACT_TARGETS}")" -gt 0 ]]; then + overall_status=0 + target_index=0 while IFS=$'\t' read -r package kind name; do + target_index=$((target_index + 1)) target_feature_args=() feature_flags="$(scripts/ci/package-feature-flags.sh "${package}")" if [[ -n "${feature_flags}" ]]; then @@ -434,14 +437,21 @@ jobs: # Capture the status so the report of a FAILING run still gets # staged: under `bash -e` the step would otherwise abort here # and lose exactly the JUnit file worth reading. + rm -f target/nextest/ci/junit.xml status=0 "${cmd[@]}" || status=$? - cp target/nextest/ci/junit.xml "junit-reports/${BUCKET_NAME}-${name}.xml" 2>/dev/null || true - [ "${status}" -eq 0 ] || exit "${status}" + if [[ -f target/nextest/ci/junit.xml ]]; then + cp target/nextest/ci/junit.xml \ + "junit-reports/${BUCKET_NAME}-exact-${target_index}.xml" + fi + if [[ "${status}" -ne 0 && "${overall_status}" -eq 0 ]]; then + overall_status="${status}" + fi done < <( jq -r '.[] | [.package, .kind, .name] | @tsv' \ <<< "${EXACT_TARGETS}" ) + [ "${overall_status}" -eq 0 ] || exit "${overall_status}" else cmd=(scripts/ci/run-hermetic-deterministic-suite.sh command timeout --signal=INT --kill-after=30s 28m @@ -460,13 +470,7 @@ jobs: - name: Local repro for this job if: failure() - run: | - { - echo "### Local repro for this job" - echo '```bash' - echo "${REPRO:-}" - echo '```' - } | tee -a "$GITHUB_STEP_SUMMARY" + run: scripts/ci/write-local-repro-summary.sh - name: Upload bucket lcov artifact if: needs.changes.outputs.run_coverage == 'true' @@ -586,13 +590,16 @@ jobs: - name: Hermetic nextest network-guard control env: IRONCLAW_HERMETIC_NEXTEST_CONTROL: "1" - run: bash scripts/ci/test-hermetic-test-process.sh + run: | + scripts/ci/run-hermetic-deterministic-suite.sh prepare-command + bash scripts/ci/test-hermetic-test-process.sh - name: Run Reborn root tests run: | cmd=(env REBORN_ROOT_TEST_PARTITIONS=4 "REBORN_ROOT_TEST_PARTITION=${{ matrix.partition }}" - RUST_MIN_STACK=67108864 + "REBORN_ROOT_TEST_TIMEOUT=${REBORN_ROOT_TEST_TIMEOUT}" + "RUST_MIN_STACK=${RUST_MIN_STACK}" scripts/ci/run-hermetic-deterministic-suite.sh command scripts/ci/run-reborn-root-partition.sh) printf -v REPRO '%q ' "${cmd[@]}" @@ -601,13 +608,7 @@ jobs: - name: Local repro for this job if: failure() - run: | - { - echo "### Local repro for this job" - echo '```bash' - echo "${REPRO:-}" - echo '```' - } | tee -a "$GITHUB_STEP_SUMMARY" + run: scripts/ci/write-local-repro-summary.sh # Every lane's nextest writes the same `target/nextest/ci/junit.xml`. # The roll-up downloads `reborn-junit-*` with `merge-multiple: true`, @@ -721,20 +722,17 @@ jobs: - name: Run Reborn group tests run: | - cmd=(scripts/ci/run-hermetic-deterministic-suite.sh groups) + cmd=(env + "REBORN_GROUP_TEST_TIMEOUT=${REBORN_GROUP_TEST_TIMEOUT}" + "RUST_MIN_STACK=${RUST_MIN_STACK}" + scripts/ci/run-hermetic-deterministic-suite.sh groups) printf -v REPRO '%q ' "${cmd[@]}" echo "REPRO=${REPRO}" >> "$GITHUB_ENV" "${cmd[@]}" - name: Local repro for this job if: failure() - run: | - { - echo "### Local repro for this job" - echo '```bash' - echo "${REPRO:-}" - echo '```' - } | tee -a "$GITHUB_STEP_SUMMARY" + run: scripts/ci/write-local-repro-summary.sh sandbox-docker-tests: name: Reborn user-sandbox Docker tests @@ -931,22 +929,22 @@ jobs: # matrix leg. Clear only profiling data, not compiled binaries. cargo llvm-cov clean --profraw-only fi - REPRO="REBORN_COV_COLLECT=false REBORN_COV_LANE_MODE=${REBORN_COV_LANE_MODE} REBORN_COV_LANE_PARTITIONS=4 REBORN_COV_LANE_INDEX=${REBORN_COV_LANE_INDEX} scripts/ci/run-hermetic-deterministic-suite.sh command scripts/ci/reborn-coverage-lane-run.sh /tmp/repro.lcov" - echo "REPRO=${REPRO}" >> "$GITHUB_ENV" scripts/ci/run-hermetic-deterministic-suite.sh prepare-command - IRONCLAW_HERMETIC_SUITE_SKIP_PREPARE=1 \ - scripts/ci/run-hermetic-deterministic-suite.sh command \ - scripts/ci/reborn-coverage-lane-run.sh part-${{ matrix.lane }}.lcov + cmd=(env + "REBORN_COV_COLLECT=${REBORN_COV_COLLECT}" + "REBORN_COV_LANE_MODE=${REBORN_COV_LANE_MODE}" + "REBORN_COV_LANE_PARTITIONS=${REBORN_COV_LANE_PARTITIONS}" + "REBORN_COV_LANE_INDEX=${REBORN_COV_LANE_INDEX}" + IRONCLAW_HERMETIC_SUITE_SKIP_PREPARE=1 + scripts/ci/run-hermetic-deterministic-suite.sh command + scripts/ci/reborn-coverage-lane-run.sh "part-${REBORN_COV_LANE_INDEX}.lcov") + printf -v REPRO '%q ' "${cmd[@]}" + echo "REPRO=${REPRO}" >> "$GITHUB_ENV" + "${cmd[@]}" - name: Local repro for this job if: failure() - run: | - { - echo "### Local repro for this job" - echo '```bash' - echo "${REPRO:-}" - echo '```' - } | tee -a "$GITHUB_STEP_SUMMARY" + run: scripts/ci/write-local-repro-summary.sh - name: Upload lane lcov artifact if: needs.changes.outputs.run_coverage == 'true' diff --git a/scripts/ci/junit_summary.py b/scripts/ci/junit_summary.py old mode 100644 new mode 100755 index 7d552a453f4..b10a16a9468 --- a/scripts/ci/junit_summary.py +++ b/scripts/ci/junit_summary.py @@ -19,6 +19,9 @@ from dataclasses import dataclass +MAX_REPORT_BYTES = 64 * 1024 * 1024 + + @dataclass(frozen=True) class FailedTest: classname: str @@ -37,18 +40,21 @@ def _first_line(text: str | None) -> str: def parse_junit(path: str) -> list[FailedTest]: - # Refuse a DTD outright instead of parsing it. JUnit XML has no - # legitimate use for one, and expat expands INTERNAL entities, so a - # billion-laughs payload would hang this step. The file is CI-produced - # but its content (test names, failure messages) comes from the branch - # under test, so it is not trusted input. Dependency-free on purpose: - # defusedxml is not a dependency of this repo. - with open(path, encoding="utf-8", errors="replace") as handle: - head = handle.read(4096) - if " MAX_REPORT_BYTES: + raise ET.ParseError( + f"JUnit report exceeds the {MAX_REPORT_BYTES}-byte limit" + ) + if b""${sandbox}/repo/frontend/package.json" + +cat >"${sandbox}/repo/scripts/ci/run-hermetic-test-process.sh" <<'STUB' +#!/usr/bin/env bash +shift +cd "${HERMETIC_TEST_REPO_ROOT}" +exec "$@" +STUB +cat >"${sandbox}/repo/scripts/ci/crate-dir.sh" <<'STUB' +#!/usr/bin/env bash +printf '%s\n' "${HERMETIC_TEST_REPO_ROOT}" +STUB +cat >"${sandbox}/repo/scripts/ci/discover-reborn-package-crates.sh" <<'STUB' +#!/usr/bin/env bash +printf '["ironclaw_fixture"]\n' +STUB +cat >"${sandbox}/repo/scripts/ci/package-feature-flags.sh" <<'STUB' +#!/usr/bin/env bash +exit 0 +STUB +cat >"${sandbox}/repo/scripts/ci/reborn-coverage-int-tier-tests.sh" <<'STUB' +#!/usr/bin/env bash +printf '%s\n' --test reborn_group_fixture --test reborn_integration_fixture +STUB +chmod +x "${sandbox}/repo/scripts/ci/"*.sh + +cat >"${sandbox}/bin/cargo" <<'STUB' +#!/usr/bin/env bash +printf 'cargo %s\n' "$*" >>"${HERMETIC_TEST_LOG}" +STUB +cat >"${sandbox}/bin/cargo-nextest" <<'STUB' +#!/usr/bin/env bash +printf 'nextest %s\n' "$*" >>"${HERMETIC_TEST_LOG}" +STUB +cp "${sandbox}/bin/cargo-nextest" "${sandbox}/cargo-nextest.stub" +cat >"${sandbox}/bin/docker" <<'STUB' +#!/usr/bin/env bash +exit 0 +STUB +cat >"${sandbox}/bin/corepack" <<'STUB' +#!/usr/bin/env bash +exit 0 +STUB +cat >"${sandbox}/bin/timeout" <<'STUB' +#!/usr/bin/env bash +while [[ "$1" == --* || "$1" == *[smhd] ]]; do + case "$1" in + --signal=*|--kill-after=*) shift ;; + *) shift; break ;; + esac +done +exec "$@" +STUB +chmod +x "${sandbox}/bin/"* + +run_suite() { + local stage="$1" ci_value="$2" with_nextest="$3" + local -a env_args=( + env -u CI + HERMETIC_TEST_REPO_ROOT="${sandbox}/repo" + HERMETIC_TEST_LOG="${sandbox}/commands.log" + PATH="${sandbox}/bin:/usr/bin:/bin" + ) + [[ "${ci_value}" == set ]] && env_args+=(CI=true) + if [[ "${with_nextest}" == no ]]; then + rm -f "${sandbox}/bin/cargo-nextest" + else + cp "${sandbox}/cargo-nextest.stub" "${sandbox}/bin/cargo-nextest" + chmod +x "${sandbox}/bin/cargo-nextest" + fi + "${env_args[@]}" bash "${sandbox}/repo/scripts/ci/run-hermetic-deterministic-suite.sh" "${stage}" +} + +failures=0 +for stage in crates integration; do + : >"${sandbox}/commands.log" + if run_suite "${stage}" set no >"${sandbox}/${stage}-ci.log" 2>&1; then + echo "FAIL: ${stage} silently fell back to cargo when nextest was absent in CI" >&2 + failures=$((failures + 1)) + elif ! grep -Fq 'cargo-nextest is required in CI' "${sandbox}/${stage}-ci.log"; then + echo "FAIL: ${stage} did not explain the missing CI runner" >&2 + cat "${sandbox}/${stage}-ci.log" >&2 + failures=$((failures + 1)) + fi + + : >"${sandbox}/commands.log" + if ! run_suite "${stage}" unset no >"${sandbox}/${stage}-local.log" 2>&1; then + echo "FAIL: ${stage} lost the local cargo fallback" >&2 + cat "${sandbox}/${stage}-local.log" >&2 + failures=$((failures + 1)) + elif ! grep -Fq 'cargo test ' "${sandbox}/commands.log"; then + echo "FAIL: ${stage} local fallback did not execute cargo test" >&2 + cat "${sandbox}/commands.log" >&2 + failures=$((failures + 1)) + fi +done + +: >"${sandbox}/commands.log" +if ! run_suite integration set yes >"${sandbox}/integration-nextest.log" 2>&1; then + echo "FAIL: integration nextest caller sandbox failed" >&2 + cat "${sandbox}/integration-nextest.log" >&2 + failures=$((failures + 1)) +else + if grep -F 'nextest ' "${sandbox}/commands.log" | grep -Fq 'reborn_group_'; then + echo "FAIL: integration pooled reborn_group_* under nextest" >&2 + failures=$((failures + 1)) + fi + if ! grep -Fq 'cargo test -p ironclaw_integration_tests --test reborn_group_fixture' "${sandbox}/commands.log"; then + echo "FAIL: integration did not preserve canonical cargo execution for group suites" >&2 + failures=$((failures + 1)) + fi + if ! grep -F 'nextest ' "${sandbox}/commands.log" | grep -Fq 'reborn_integration_fixture'; then + echo "FAIL: integration did not retain non-group suites in nextest" >&2 + failures=$((failures + 1)) + fi +fi + +if [[ "${failures}" -ne 0 ]]; then + echo "hermetic deterministic-suite runner: ${failures} failure(s)" >&2 + exit 1 +fi +echo "hermetic deterministic-suite runner: OK" diff --git a/scripts/ci/test-hermetic-nextest-control-gating.sh b/scripts/ci/test-hermetic-nextest-control-gating.sh index d55f06c0fc1..20df8aae112 100755 --- a/scripts/ci/test-hermetic-nextest-control-gating.sh +++ b/scripts/ci/test-hermetic-nextest-control-gating.sh @@ -11,7 +11,7 @@ # The control is therefore opt-in. This test pins BOTH halves of that # contract, using a PATH sandbox (same shape as test-quality-gate-runner.sh): # a stub `cargo`/`cargo-nextest` that records any invocation and fails. -set -uo pipefail +set -euo pipefail repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" under_test="${repo_root}/scripts/ci/test-hermetic-test-process.sh" @@ -57,9 +57,13 @@ done # 1. Default (the fast-checks shape): control skipped, script green, and # cargo never invoked even though it is on PATH. -PATH="${sandbox}:${PATH}" env -u IRONCLAW_HERMETIC_NEXTEST_CONTROL \ - bash "${under_test}" >"${sandbox}/default.log" 2>&1 -assert_success "default run (fast-checks shape)" "$?" +if PATH="${sandbox}:${PATH}" env -u IRONCLAW_HERMETIC_NEXTEST_CONTROL \ + bash "${under_test}" >"${sandbox}/default.log" 2>&1; then + default_status=0 +else + default_status=$? +fi +assert_success "default run (fast-checks shape)" "${default_status}" assert_no_cargo_invocation "default run (fast-checks shape)" if ! grep -q "control not requested" "${sandbox}/default.log"; then echo "FAIL: default run did not report that the control was skipped" >&2 @@ -72,9 +76,13 @@ rm -f "${marker}" missing="${sandbox}/no-nextest" mkdir -p "${missing}" cp "${sandbox}/cargo" "${missing}/cargo" -PATH="${missing}:/usr/bin:/bin" CI=true IRONCLAW_HERMETIC_NEXTEST_CONTROL=1 \ - bash "${under_test}" >"${sandbox}/optin.log" 2>&1 -assert_failure "opted-in run with cargo-nextest missing under CI" "$?" +if PATH="${missing}:/usr/bin:/bin" CI=true IRONCLAW_HERMETIC_NEXTEST_CONTROL=1 \ + bash "${under_test}" >"${sandbox}/optin.log" 2>&1; then + optin_status=0 +else + optin_status=$? +fi +assert_failure "opted-in run with cargo-nextest missing under CI" "${optin_status}" if [[ "${failures}" -ne 0 ]]; then echo "hermetic nextest-control gating: ${failures} failure(s)" >&2 diff --git a/scripts/ci/test-run-reborn-group-tests.sh b/scripts/ci/test-run-reborn-group-tests.sh new file mode 100755 index 00000000000..a5f555be681 --- /dev/null +++ b/scripts/ci/test-run-reborn-group-tests.sh @@ -0,0 +1,66 @@ +#!/usr/bin/env bash +set -euo pipefail + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +under_test="${repo_root}/scripts/ci/run-reborn-group-tests.sh" +sandbox="$(mktemp -d)" +trap 'rm -rf "${sandbox}"' EXIT + +mkdir -p \ + "${sandbox}/bin" \ + "${sandbox}/tests/integration/group_first" \ + "${sandbox}/tests/integration/group_second" \ + "${sandbox}/tests/integration/group_third" +touch \ + "${sandbox}/tests/integration/group_first/main.rs" \ + "${sandbox}/tests/integration/group_second/main.rs" \ + "${sandbox}/tests/integration/group_third/main.rs" + +cat >"${sandbox}/bin/cargo" <<'STUB' +#!/usr/bin/env bash +printf '%s\n' "$*" >>"${GROUP_RUNNER_LOG}" +if [[ "$*" == *"--test reborn_group_second"* ]]; then + exit 23 +fi +STUB +chmod +x "${sandbox}/bin/cargo" + +cat >"${sandbox}/bin/timeout" <<'STUB' +#!/usr/bin/env bash +while [[ "$1" == --* || "$1" == *[smhd] ]]; do + case "$1" in + --signal=*|--kill-after=*) shift ;; + *) shift; break ;; + esac +done +exec "$@" +STUB +chmod +x "${sandbox}/bin/timeout" + +status=0 +if output="$( + cd "${sandbox}" + PATH="${sandbox}/bin:/usr/bin:/bin" \ + GROUP_RUNNER_LOG="${sandbox}/cargo.log" \ + bash "${under_test}" 2>&1 +)"; then + echo "FAIL: a failed group suite did not make the runner fail" >&2 + status=1 +fi + +if [[ "${status}" -eq 0 ]]; then + if [[ "${output}" != *"group suite failed: reborn_group_second"* ]]; then + echo "FAIL: failed group suite was not reported" >&2 + status=1 + fi + if ! grep -Fq -- '--test reborn_group_third' "${sandbox}/cargo.log"; then + echo "FAIL: group runner stopped before the later suite" >&2 + status=1 + fi +fi + +if [[ "${status}" -ne 0 ]]; then + printf '%s\n' "${output}" >&2 + exit "${status}" +fi +echo "reborn group runner contract: OK" diff --git a/scripts/ci/test-write-local-repro-summary.sh b/scripts/ci/test-write-local-repro-summary.sh new file mode 100755 index 00000000000..9e4cca8751d --- /dev/null +++ b/scripts/ci/test-write-local-repro-summary.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash +set -euo pipefail + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +under_test="${repo_root}/scripts/ci/write-local-repro-summary.sh" +sandbox="$(mktemp -d)" +trap 'rm -rf "${sandbox}"' EXIT + +GITHUB_STEP_SUMMARY="${sandbox}/unset.md" "${under_test}" >/dev/null +grep -Fq "" \ + "${sandbox}/unset.md" + +GITHUB_STEP_SUMMARY="${sandbox}/set.md" REPRO="cargo nextest run --profile ci" \ + "${under_test}" >/dev/null +grep -Fq "cargo nextest run --profile ci" "${sandbox}/set.md" + +echo "local repro summary: OK" diff --git a/scripts/ci/test_junit_summary.py b/scripts/ci/test_junit_summary.py old mode 100644 new mode 100755 index c259fad5513..8fd92494757 --- a/scripts/ci/test_junit_summary.py +++ b/scripts/ci/test_junit_summary.py @@ -8,6 +8,7 @@ import sys import tempfile import unittest +from unittest import mock sys.path.insert(0, str(pathlib.Path(__file__).parent)) import junit_summary # noqa: E402 @@ -70,6 +71,45 @@ def test_unparseable_file_warns_and_continues(self) -> None: self.assertEqual(status, 0) self.assertIn("::warning::", buf.getvalue()) + def test_dtd_after_initial_4096_bytes_warns_and_continues(self) -> None: + unsafe = self._write( + '\n' + + (" " * 4096) + + ']>\n' + + '' + + '' + ) + good = self._write(FAILING_XML) + stdout = io.StringIO() + stderr = io.StringIO() + + with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): + status = junit_summary.main([unsafe, good]) + + self.assertEqual(status, 0) + self.assertIn("refusing JUnit XML containing a DTD or entity", stderr.getvalue()) + self.assertNotIn("expanded", stdout.getvalue()) + self.assertIn("project_scope_leaks_across_tenant", stdout.getvalue()) + + def test_oversized_report_warns_and_continues(self) -> None: + oversized = self._write( + '' + + ('' * 40) + + "" + ) + good = self._write(FAILING_XML) + stdout = io.StringIO() + stderr = io.StringIO() + + with mock.patch.object(junit_summary, "MAX_REPORT_BYTES", 1024, create=True): + with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): + status = junit_summary.main([oversized, good]) + + self.assertEqual(status, 0) + self.assertIn("exceeds the 1024-byte limit", stderr.getvalue()) + self.assertNotIn("should not be parsed", stdout.getvalue()) + self.assertIn("project_scope_leaks_across_tenant", stdout.getvalue()) + if __name__ == "__main__": unittest.main() diff --git a/scripts/ci/test_reborn_pr_test_plan.py b/scripts/ci/test_reborn_pr_test_plan.py old mode 100644 new mode 100755 index 51d0ec05a5f..a18005f5e12 --- a/scripts/ci/test_reborn_pr_test_plan.py +++ b/scripts/ci/test_reborn_pr_test_plan.py @@ -2184,6 +2184,66 @@ def test_workflow_consumes_plan_and_bounds_each_rust_matrix(self) -> None: ' -p "${package}" "--${kind}" "${name}"', workflow, ) + # Every selected exact target must run even when an earlier target + # fails, and every target's nextest report must survive under a unique + # name before the aggregate status fails the step. + self.assertIn("overall_status=0", workflow) + self.assertIn( + '"junit-reports/${BUCKET_NAME}-exact-${target_index}.xml"', + workflow, + ) + self.assertIn("rm -f target/nextest/ci/junit.xml", workflow) + self.assertIn( + '[ "${overall_status}" -eq 0 ] || exit "${overall_status}"', + workflow, + ) + + # Repro commands describe the command CI actually ran, including the + # timeout/stack controls and coverage mode/output path. + self.assertIn( + '"REBORN_ROOT_TEST_TIMEOUT=${REBORN_ROOT_TEST_TIMEOUT}"', workflow + ) + self.assertIn( + '"REBORN_GROUP_TEST_TIMEOUT=${REBORN_GROUP_TEST_TIMEOUT}"', workflow + ) + self.assertGreaterEqual(workflow.count('"RUST_MIN_STACK=${RUST_MIN_STACK}"'), 2) + self.assertIn('"REBORN_COV_COLLECT=${REBORN_COV_COLLECT}"', workflow) + self.assertIn( + 'scripts/ci/reborn-coverage-lane-run.sh "part-${REBORN_COV_LANE_INDEX}.lcov"', + workflow, + ) + + # The opted-in nextest network control runs only after Cargo's locked + # dependencies have been prepared for hermetic offline execution. + control_start = workflow.index( + " - name: Hermetic nextest network-guard control" + ) + control_end = workflow.index(" - name: Run Reborn root tests", control_start) + control_step = workflow[control_start:control_end] + self.assertIn( + "scripts/ci/run-hermetic-deterministic-suite.sh prepare-command", + control_step, + ) + self.assertLess( + control_step.index("run-hermetic-deterministic-suite.sh prepare-command"), + control_step.index("test-hermetic-test-process.sh"), + ) + + # Keep the fallback prose out of shell parameter expansion: its + # apostrophe made the generated workflow command syntactically invalid. + self.assertNotIn("${REPRO:- None: code_style = (ROOT / ".github/workflows/code_style.yml").read_text( diff --git a/scripts/ci/test_ws12_workflow_contracts.py b/scripts/ci/test_ws12_workflow_contracts.py index e6c415752b7..8a013d6b06f 100755 --- a/scripts/ci/test_ws12_workflow_contracts.py +++ b/scripts/ci/test_ws12_workflow_contracts.py @@ -3,6 +3,7 @@ from __future__ import annotations +import ast import copy import dataclasses import tempfile @@ -49,6 +50,22 @@ ) +def _yaml_import_nodes(source: str) -> list[ast.Import | ast.ImportFrom]: + tree = ast.parse(source) + return [ + node + for node in ast.walk(tree) + if ( + isinstance(node, ast.Import) + and any(alias.name.split(".", 1)[0] == "yaml" for alias in node.names) + ) + or ( + isinstance(node, ast.ImportFrom) + and (node.module or "").split(".", 1)[0] == "yaml" + ) + ] + + class SccacheSetupActionContractTests(unittest.TestCase): """The optional compiler cache must never gate the tests it accelerates.""" @@ -2157,8 +2174,30 @@ def test_duplicate_key_is_reported_with_file_and_line(self): encoding="utf-8", ) errors = validate_no_duplicate_yaml_keys(root) + self.assertEqual( + 1, + sum( + ".github/workflows/dupe.yml:5: duplicate key " + "'if-no-files-found'" in error + for error in errors + ), + errors, + ) + + def test_flow_mapping_is_rejected_before_duplicate_keys_can_hide(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + directory = self._workflows_dir(root) + (directory / "flow.yml").write_text( + "jobs:\n" + " build:\n" + " with: { if-no-files-found: ignore, " + "if-no-files-found: error }\n", + encoding="utf-8", + ) + errors = validate_no_duplicate_yaml_keys(root) self.assertTrue( - any("if-no-files-found" in e and "dupe.yml" in e for e in errors), + any("flow.yml:3" in error and "flow mapping" in error for error in errors), errors, ) @@ -2220,7 +2259,12 @@ def test_the_checker_needs_no_third_party_yaml_module(self): source = ( Path(ws12_workflow_contracts.__file__).read_text(encoding="utf-8") ) - self.assertNotIn("import yaml", source) + self.assertEqual([], _yaml_import_nodes(source)) + + def test_yaml_import_guard_rejects_both_import_forms(self): + for source in ("import yaml\n", "from yaml import safe_load\n"): + with self.subTest(source=source.strip()): + self.assertEqual(1, len(_yaml_import_nodes(source))) def test_live_workflows_have_no_duplicate_keys(self): self.assertEqual([], validate_no_duplicate_yaml_keys(ROOT)) @@ -2245,6 +2289,30 @@ def test_eval_in_a_run_block_fails(self): errors = validate_no_eval_in_workflow_run_blocks(workflows) self.assertTrue(any("x.yml:3" in e for e in errors), errors) + def test_eval_after_shell_command_separators_fails(self): + separators = ( + "prepare; eval", + "prepare && eval", + "prepare || eval", + "prepare | eval", + ) + for command in separators: + with self.subTest(command=command): + workflows = { + ".github/workflows/x.yml": f' {command} "$REPRO"\n' + } + errors = validate_no_eval_in_workflow_run_blocks(workflows) + self.assertTrue(any("x.yml:1" in error for error in errors), errors) + + def test_eval_after_shell_control_words_fails(self): + for command in ( + 'if ready; then eval "$REPRO"; fi', + 'while ready; do eval "$REPRO"; done', + ): + with self.subTest(command=command): + workflows = {".github/workflows/x.yml": f" {command}\n"} + self.assertTrue(validate_no_eval_in_workflow_run_blocks(workflows)) + def test_argv_array_invocation_passes(self): workflows = { ".github/workflows/x.yml": ( @@ -2272,6 +2340,21 @@ def test_live_workflows_contain_no_eval(self): [], validate_no_eval_in_workflow_run_blocks(workflows) ) + def test_loader_includes_yaml_extension(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + directory = root / ".github" / "workflows" + directory.mkdir(parents=True) + (directory / "long.yaml").write_text("name: long\n", encoding="utf-8") + (directory / "short.yml").write_text("name: short\n", encoding="utf-8") + self.assertEqual( + { + ".github/workflows/long.yaml", + ".github/workflows/short.yml", + }, + set(load_workflows(root)), + ) + if __name__ == "__main__": diff --git a/scripts/ci/write-local-repro-summary.sh b/scripts/ci/write-local-repro-summary.sh new file mode 100755 index 00000000000..8aa02c05be9 --- /dev/null +++ b/scripts/ci/write-local-repro-summary.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash +set -euo pipefail + +summary_path="${GITHUB_STEP_SUMMARY:?GITHUB_STEP_SUMMARY must name the job summary}" + +{ + echo "### Local repro for this job" + echo '```bash' + if [[ -n "${REPRO:-}" ]]; then + echo "${REPRO}" + else + echo "" + fi + echo '```' +} | tee -a "${summary_path}" diff --git a/scripts/ci/ws12_workflow_contracts.py b/scripts/ci/ws12_workflow_contracts.py index cdef35c1b43..768feb95557 100755 --- a/scripts/ci/ws12_workflow_contracts.py +++ b/scripts/ci/ws12_workflow_contracts.py @@ -1755,7 +1755,11 @@ def validate_crate_name_residue( return errors -WORKFLOW_EVAL = re.compile(r"^[ \t]*eval[ \t]", re.MULTILINE) +WORKFLOW_EVAL = re.compile( + r"(?:^[ \t]*|(?:;|&&|\|\||\|)[ \t]*|(?:\bthen|\bdo)[ \t]+)" + r"eval(?=[ \t;&|]|$)", + re.MULTILINE, +) def validate_no_eval_in_workflow_run_blocks( @@ -1763,15 +1767,9 @@ def validate_no_eval_in_workflow_run_blocks( ) -> list[str]: """No workflow may `eval` a command it just built as a string. - Commands are assembled as argv arrays and run directly; the REPRO hint - is derived FROM the argv with printf '%q '. `eval` re-parses whatever - was interpolated, and the crate-bucket lane interpolates - planner-derived target names that come from changed FILENAMES -- so a - contributor adding `tests/$(cmd).rs` would execute `cmd` on a runner - that holds sccache credentials. A newline in the same value can also - forge extra $GITHUB_ENV entries. Both become unrepresentable once the - argv is never re-parsed, so this stays a hard contract rather than a - review habit. + Commands are argv arrays; REPRO is derived from them with printf '%q '. + The crate-bucket target names come from changed filenames, so reparsing + them would permit command execution or forged GITHUB_ENV entries. """ errors: list[str] = [] @@ -1796,24 +1794,16 @@ def validate_no_eval_in_workflow_run_blocks( def validate_no_duplicate_yaml_keys(root: Path = ROOT) -> list[str]: """No workflow may declare the same mapping key twice in one block. - Deliberately stdlib-only: every checker in scripts/ci is, and adding - PyYAML here would make the gate depend on a package the fast-checks job - does not install (learned the hard way -- an earlier PyYAML version of - this function errored 70 self-tests in CI while passing locally). - - It is also the wrong tool: PyYAML's safe_load silently keeps the LAST - duplicate, so a local `yaml.safe_load` sanity check passes while the - value the author wrote is discarded -- which is how a duplicated - `if-no-files-found` pinned a JUnit upload to `error` and would have - hard-failed every coverage lane. - - The scan tracks one key set per mapping block, keyed by indent. A `- ` - item opens a fresh block, and block scalars (`|`, `>`) are skipped so - their free text is never parsed as YAML. + Stdlib-only: fast-checks has no PyYAML and safe_load keeps the last duplicate. + Track indentation, reset list items, skip block scalars, and reject unsupported flow mappings. """ errors: list[str] = [] - for path in sorted((root / ".github" / "workflows").glob("*.y*ml")): + workflows_dir = root / ".github" / "workflows" + paths = sorted( + (*workflows_dir.glob("*.yml"), *workflows_dir.glob("*.yaml")) + ) + for path in paths: rel = path.relative_to(root).as_posix() scopes: list[tuple[int, set[str]]] = [] skip_until_indent: int | None = None @@ -1840,6 +1830,13 @@ def validate_no_duplicate_yaml_keys(root: Path = ROOT) -> list[str]: if match is None: continue key = match.group("key").strip("\"'") + if match.group("rest").strip().startswith("{"): + errors.append( + f"{rel}:{number}: inline YAML flow mapping is unsupported. " + "Use an indented block mapping so duplicate keys remain visible " + "to the stdlib-only validator." + ) + continue dash = match.group("dash") effective = indent + (len(dash) if dash else 0) while scopes and scopes[-1][0] > effective: @@ -1910,7 +1907,7 @@ def validate_workflow_texts( def load_workflows(root: Path) -> dict[str, str]: - """Every `.github/workflows/*.yml` file, repo-relative path -> text. + """Every `.github/workflows/*.yml`/`*.yaml` file, repo-relative path -> text. A handful of contracts key off one specific known path (REQUIRED_MARKERS, CRATE_SCOPE_FILTERS, CRATE_NAME_RESIDUE); `validate_webui_frontend_sites` @@ -1922,9 +1919,12 @@ def load_workflows(root: Path) -> dict[str, str]: """ workflows_dir = root / ".github" / "workflows" + paths = sorted( + (*workflows_dir.glob("*.yml"), *workflows_dir.glob("*.yaml")) + ) return { path.relative_to(root).as_posix(): path.read_text(encoding="utf-8") - for path in sorted(workflows_dir.glob("*.yml")) + for path in paths } diff --git a/tests/AGENTS.md b/tests/AGENTS.md index 91e00a63bb9..bcdff2bc1cc 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -388,7 +388,7 @@ manual-run and scheduled-run denial evidence), **Policy & format**: `e2e_trace_runtime_policy_org_ceiling_yolo.rs` (org policy ceiling × yolo narrowing), `e2e_trace_runtime_policy_serde.rs` (wire-stable policy enums), `trace_format.rs`, `trace_llm_tests.rs`, -`reborn_coverage_lane_stack_headroom.rs` (CI job must declare stack headroom), `hermetic_network_guard_probe.rs` (the hermetic wrapper's network guard must survive nextest's own process spawning, not just a directly-launched binary; `#[ignore]`d — driven by scripts/ci/test-hermetic-test-process.sh under IRONCLAW_HERMETIC_NEXTEST_CONTROL=1). +`reborn_coverage_lane_stack_headroom.rs` (CI job must declare stack headroom), `hermetic_network_guard_probe.rs` (hermetic nextest runs cannot make non-loopback connections; `#[ignore]`d — driven by scripts/ci/test-hermetic-test-process.sh under IRONCLAW_HERMETIC_NEXTEST_CONTROL=1). --- diff --git a/tests/hermetic_network_guard_probe.rs b/tests/hermetic_network_guard_probe.rs index 5e96b23f0e9..24373c9f96b 100644 --- a/tests/hermetic_network_guard_probe.rs +++ b/tests/hermetic_network_guard_probe.rs @@ -6,9 +6,9 @@ //! `cargo`/`cargo nextest`, so they prove nothing about whether nextest's //! test-process spawn path inherits the same env. This test does: run it //! once under `cargo nextest` inside the hermetic wrapper with the guard -//! ACTIVE (must PASS -- connection refused), and once with +//! ACTIVE (must PASS -- the interposer returns `PermissionDenied`), and once with //! `IRONCLAW_HERMETIC_SABOTAGE=network` (guard DISABLED, must FAIL -- -//! connection not refused within the timeout). Only the differential +//! any non-`PermissionDenied` outcome). Only the differential //! between those two runs is the actual proof; see //! scripts/ci/test-hermetic-test-process.sh for the orchestration. //! From f93fdd13f395cd23b20d006f998b56f5254abc75 Mon Sep 17 00:00:00 2001 From: Henry Park Date: Tue, 25 Aug 2026 00:25:34 +0000 Subject: [PATCH 15/19] Fix PR CI listener race (#7817) - make CLI listener tests globally exclusive during port handoff - replace substring filter checks with an exact ordered config contract --- .config/nextest.toml | 9 +++- scripts/ci/test_nextest_config.py | 74 ++++++++++++++++++------------- 2 files changed, 49 insertions(+), 34 deletions(-) diff --git a/.config/nextest.toml b/.config/nextest.toml index 4f0e10e08ed..0209a09c796 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -59,18 +59,23 @@ path = "junit.xml" # protected_request, 1 of 1417 tests, in the reborn-core bucket. # # The rule this follows is the plan's own: a nextest-only failure gets -# serialised here, never "fixed" by editing the test. Only the listener -# tests join the group -- the other ~128 smoke tests stay parallel. +# serialised here, never "fixed" by editing the test. `max-threads = 1` +# orders the listener tests relative to each other; `threads-required = +# "num-test-threads"` also excludes unrelated tests during each listener's +# bind-close-rebind window. Without that global exclusion, the first fix still +# failed while ungrouped tests ran concurrently. [test-groups] cli-serve-listener = { max-threads = 1 } [[profile.default.overrides]] filter = 'package(ironclaw) & binary(smoke) & (test(~serve) | test(~onboard_) | test(~stored_key_reaches) | test(a_real_env_var_beats_the_config_default_end_to_end))' test-group = "cli-serve-listener" +threads-required = "num-test-threads" [[profile.ci.overrides]] filter = 'package(ironclaw) & binary(smoke) & (test(~serve) | test(~onboard_) | test(~stored_key_reaches) | test(a_real_env_var_beats_the_config_default_end_to_end))' test-group = "cli-serve-listener" +threads-required = "num-test-threads" # Per-test windows for the handful of scenarios that genuinely need more # than 60s (live-recorded fixtures + heavy integration). diff --git a/scripts/ci/test_nextest_config.py b/scripts/ci/test_nextest_config.py index 6d203eff7d8..254d379127d 100755 --- a/scripts/ci/test_nextest_config.py +++ b/scripts/ci/test_nextest_config.py @@ -23,27 +23,22 @@ ROOT = Path(__file__).resolve().parents[2] CONFIG = ROOT / ".config" / "nextest.toml" GROUP = "cli-serve-listener" -# The test that failed in CI, plus one representative of each other shape the -# filter must keep covering. -MUST_COVER = ( - "onboard_login_link_then_bearer_authorizes_a_protected_request", - "serve_boots_without_user_id_env_var", - "stored_key_reaches_real_turn_via_product_surface", +LISTENER_FILTER = ( + "package(ironclaw) & binary(smoke) & " + "(test(~serve) | test(~onboard_) | test(~stored_key_reaches) | " + "test(a_real_env_var_beats_the_config_default_end_to_end))" ) -GLOB_FRAGMENTS = ("serve", "onboard_", "stored_key_reaches") def _config() -> dict: return tomllib.loads(CONFIG.read_text(encoding="utf-8")) -def _bound_filters(config: dict, profile: str) -> str: +def _listener_override(config: dict, profile: str) -> dict | None: overrides = config.get("profile", {}).get(profile, {}).get("overrides", []) - return " ".join( - override.get("filter", "") - for override in overrides - if override.get("test-group") == GROUP - ) + if not overrides or overrides[0].get("filter") != LISTENER_FILTER: + return None + return overrides[0] class NextestSerialisationTests(unittest.TestCase): @@ -63,30 +58,45 @@ def test_the_group_stays_single_threaded(self) -> None: "more than one thread reopens the port race this group closed", ) - def test_both_profiles_bind_the_group(self) -> None: + def test_both_profiles_make_listener_tests_globally_exclusive(self) -> None: config = _config() for profile in ("default", "ci"): with self.subTest(profile=profile): - self.assertNotEqual( - "", - _bound_filters(config, profile).strip(), - f"profile {profile!r} would run the listener tests in parallel", + override = _listener_override(config, profile) + self.assertIsNotNone( + override, + f"profile {profile!r} must keep the canonical listener " + "override first so its semantics cannot be shadowed", + ) + self.assertEqual(GROUP, override.get("test-group")) + self.assertEqual( + "num-test-threads", + override.get("threads-required"), + "a one-thread group serialises only its own members; the " + "bind-close-rebind window must exclude every other test", ) - def test_the_filter_still_covers_the_test_that_failed(self) -> None: - config = _config() - for profile in ("default", "ci"): - expression = _bound_filters(config, profile) - for name in MUST_COVER: - with self.subTest(profile=profile, test=name): - covered = name in expression or any( - fragment in name - for fragment in GLOB_FRAGMENTS - if f"test(~{fragment})" in expression - ) - self.assertTrue( - covered, f"{name} is no longer serialised in {profile!r}" - ) + def test_filter_guard_rejects_heuristic_false_positives_and_shadowing(self) -> None: + for expression in ( + "!test(~serve)", + "package(other) & test(~serve)", + "binary(other) & test(~serve)", + ): + with self.subTest(expression=expression): + config = {"profile": {"ci": {"overrides": [{"filter": expression}]}}} + self.assertIsNone(_listener_override(config, "ci")) + + shadowed = { + "profile": { + "ci": { + "overrides": [ + {"filter": "test(~serve)", "test-group": "other"}, + {"filter": LISTENER_FILTER, "test-group": GROUP}, + ] + } + } + } + self.assertIsNone(_listener_override(shadowed, "ci")) if __name__ == "__main__": From 10306c7e20fb53d57defcbf11624398e7078cb5e Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Tue, 25 Aug 2026 00:55:45 +0000 Subject: [PATCH 16/19] Prepare WebUI dependencies for hermetic nextest control The root package build invokes the WebUI build script, so warm both Cargo and pnpm dependencies before the network guard closes external access. --- .github/workflows/reborn-tests.yml | 5 +++-- scripts/ci/run-hermetic-deterministic-suite.sh | 2 +- .../ci/test-hermetic-deterministic-suite-runner.sh | 13 +++++++++++++ scripts/ci/test-hermetic-test-process.sh | 5 +++-- 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/.github/workflows/reborn-tests.yml b/.github/workflows/reborn-tests.yml index 370423f2b86..3755b07d442 100644 --- a/.github/workflows/reborn-tests.yml +++ b/.github/workflows/reborn-tests.yml @@ -563,8 +563,9 @@ jobs: # The hermetic wrapper's network guard must survive nextest's own # process spawning, not just a directly-launched test binary. This is # the only place that control can run: it shells out to cargo, so it - # needs the toolchain, a warm registry and cargo-nextest -- all of - # which this job already has and fast-checks deliberately does not. + # needs the toolchain, prepared Cargo/WebUI dependencies and + # cargo-nextest -- all of which this job already has and fast-checks + # deliberately does not. - name: Hermetic nextest network-guard control env: IRONCLAW_HERMETIC_NEXTEST_CONTROL: "1" diff --git a/scripts/ci/run-hermetic-deterministic-suite.sh b/scripts/ci/run-hermetic-deterministic-suite.sh index 0522007bdb5..966af7c8904 100755 --- a/scripts/ci/run-hermetic-deterministic-suite.sh +++ b/scripts/ci/run-hermetic-deterministic-suite.sh @@ -231,7 +231,7 @@ case "${stage}" in run_python_e2e ;; prepare-command) - prepare_command_dependencies + prepare_rust_dependencies ;; all) "${repo_root}/scripts/ci/test-hermetic-test-process.sh" diff --git a/scripts/ci/test-hermetic-deterministic-suite-runner.sh b/scripts/ci/test-hermetic-deterministic-suite-runner.sh index ab4522da550..218e146892f 100755 --- a/scripts/ci/test-hermetic-deterministic-suite-runner.sh +++ b/scripts/ci/test-hermetic-deterministic-suite-runner.sh @@ -58,6 +58,7 @@ exit 0 STUB cat >"${sandbox}/bin/corepack" <<'STUB' #!/usr/bin/env bash +printf 'corepack %s\n' "$*" >>"${HERMETIC_TEST_LOG}" exit 0 STUB cat >"${sandbox}/bin/timeout" <<'STUB' @@ -91,6 +92,18 @@ run_suite() { } failures=0 + +: >"${sandbox}/commands.log" +if ! run_suite prepare-command unset no >"${sandbox}/prepare-command.log" 2>&1; then + echo "FAIL: prepare-command stage failed" >&2 + cat "${sandbox}/prepare-command.log" >&2 + failures=$((failures + 1)) +elif ! grep -Fq 'corepack pnpm install --frozen-lockfile' "${sandbox}/commands.log"; then + echo "FAIL: prepare-command did not provision frontend dependencies for Rust build scripts" >&2 + cat "${sandbox}/commands.log" >&2 + failures=$((failures + 1)) +fi + for stage in crates integration; do : >"${sandbox}/commands.log" if run_suite "${stage}" set no >"${sandbox}/${stage}-ci.log" 2>&1; then diff --git a/scripts/ci/test-hermetic-test-process.sh b/scripts/ci/test-hermetic-test-process.sh index 1b0c5260111..29b4345c466 100755 --- a/scripts/ci/test-hermetic-test-process.sh +++ b/scripts/ci/test-hermetic-test-process.sh @@ -433,8 +433,9 @@ fi # failed: ... left: TimedOut, right: PermissionDenied". # The control below is the only part of this self-test that invokes cargo. # It therefore needs a prepared Rust dependency graph: nextest shells out to -# `cargo metadata`, which cannot resolve offline inside the hermetic wrapper -# unless the registry cache is already warm. `fast-checks` is deliberately a +# `cargo metadata`, and compiling the root package can invoke the WebUI build +# script. Neither Cargo nor pnpm can resolve dependencies inside the hermetic +# wrapper. `fast-checks` is deliberately a # cache-less, toolchain-light lane, so this stays OFF by default and the # stub-based checks above run everywhere unchanged. Jobs that already have a # toolchain, a restored cargo registry and cargo-nextest opt in by setting From 9e559959a5cdb6855a5163ffb8a50e29d6ff7b20 Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Tue, 25 Aug 2026 05:21:20 +0000 Subject: [PATCH 17/19] Address latest PR feedback and root CI failures (#7817) - preserve full integration failure signal after group failures - scope frontend preparation and retain caller-owned Corepack caches - close workflow eval, YAML, and matrix interpolation gaps --- .github/workflows/reborn-tests.yml | 5 +- .../ci/run-hermetic-deterministic-suite.sh | 53 +++++++++--- ...est-hermetic-deterministic-suite-runner.sh | 59 ++++++++++++- scripts/ci/test_ws12_workflow_contracts.py | 82 +++++++++++++++++++ scripts/ci/ws12_workflow_contracts.py | 63 +++++++++++++- 5 files changed, 242 insertions(+), 20 deletions(-) diff --git a/.github/workflows/reborn-tests.yml b/.github/workflows/reborn-tests.yml index 3755b07d442..2056c425152 100644 --- a/.github/workflows/reborn-tests.yml +++ b/.github/workflows/reborn-tests.yml @@ -575,8 +575,9 @@ jobs: - name: Run Reborn root tests run: | - cmd=(env REBORN_ROOT_TEST_PARTITIONS=4 - "REBORN_ROOT_TEST_PARTITION=${{ matrix.partition }}" + cmd=(env IRONCLAW_HERMETIC_SUITE_SKIP_PREPARE=1 + REBORN_ROOT_TEST_PARTITIONS=4 + "REBORN_ROOT_TEST_PARTITION=${REBORN_ROOT_TEST_PARTITION}" "REBORN_ROOT_TEST_TIMEOUT=${REBORN_ROOT_TEST_TIMEOUT}" "RUST_MIN_STACK=${RUST_MIN_STACK}" scripts/ci/run-hermetic-deterministic-suite.sh command diff --git a/scripts/ci/run-hermetic-deterministic-suite.sh b/scripts/ci/run-hermetic-deterministic-suite.sh index 966af7c8904..40335a67528 100755 --- a/scripts/ci/run-hermetic-deterministic-suite.sh +++ b/scripts/ci/run-hermetic-deterministic-suite.sh @@ -9,11 +9,12 @@ if [[ "$#" -gt 0 ]]; then fi frontend_corepack_home="" +frontend_corepack_home_owned=0 postgres_image_prepared=0 webui_frontend_dir="" cleanup() { - if [[ -n "${frontend_corepack_home}" && -d "${frontend_corepack_home}" ]]; then + if [[ "${frontend_corepack_home_owned}" == "1" && -n "${frontend_corepack_home}" && -d "${frontend_corepack_home}" ]]; then rm -rf -- "${frontend_corepack_home}" fi } @@ -38,10 +39,8 @@ resolve_webui_frontend_dir() { prepare_rust_dependencies() { # Dependency acquisition is setup, not test behavior. Fetch once before the - # hermetic process switches Cargo into offline mode. Rust builds can invoke - # the WebUI build script, so prepare its pinned package manager too. + # hermetic process switches Cargo into offline mode. prepare_command_dependencies - prepare_frontend_dependencies } prepare_command_dependencies() { @@ -93,7 +92,8 @@ run_crate_tests() { } run_integration_tier() { - local test_name runner + local test_name runner status overall_status=0 + local group_status local -a test_args=() local -a group_test_names=() prepare_postgres_test_image @@ -112,19 +112,33 @@ run_integration_tier() { # Delegate them to the same canonical runner used by the dedicated groups # stage instead of admitting them to nextest's parallel process pool. if [[ "${#group_test_names[@]}" -gt 0 ]]; then + group_status=0 REBORN_GROUP_TEST_TIMEOUT="${REBORN_GROUP_TEST_TIMEOUT:-28m}" \ RUST_MIN_STACK=67108864 \ - run "${repo_root}/scripts/ci/run-reborn-group-tests.sh" "${group_test_names[@]}" + run "${repo_root}/scripts/ci/run-reborn-group-tests.sh" "${group_test_names[@]}" || group_status=$? + if [[ "${group_status}" -ne 0 ]]; then + overall_status="${group_status}" + fi fi if [[ "${runner}" == "nextest" && "${#test_args[@]}" -gt 0 ]]; then - run cargo nextest run --profile ci -p ironclaw_integration_tests "${test_args[@]}" --ignore-rust-version + status=0 + run cargo nextest run --profile ci -p ironclaw_integration_tests "${test_args[@]}" --ignore-rust-version || status=$? + if [[ "${status}" -ne 0 && "${overall_status}" -eq 0 ]]; then + overall_status="${status}" + fi else for test_name in "${test_args[@]}"; do [[ "${test_name}" == --test ]] && continue - run cargo test -p ironclaw_integration_tests --test "${test_name}" -- --nocapture + status=0 + run cargo test -p ironclaw_integration_tests --test "${test_name}" -- --nocapture || status=$? + if [[ "${status}" -ne 0 && "${overall_status}" -eq 0 ]]; then + overall_status="${status}" + fi done fi + + return "${overall_status}" } run_python_e2e() { @@ -168,9 +182,15 @@ prepare_frontend_dependencies() { return 2 fi - frontend_corepack_home="$( - mktemp -d "${RUNNER_TEMP:-${TMPDIR:-/tmp}}/ironclaw-corepack.XXXXXX" - )" + if [[ -n "${COREPACK_HOME:-}" ]]; then + frontend_corepack_home="${COREPACK_HOME}" + mkdir -p "${frontend_corepack_home}" + else + frontend_corepack_home="$( + mktemp -d "${RUNNER_TEMP:-${TMPDIR:-/tmp}}/ironclaw-corepack.XXXXXX" + )" + frontend_corepack_home_owned=1 + fi COREPACK_HOME="${frontend_corepack_home}" \ corepack install --global "${package_manager}" export COREPACK_HOME="${frontend_corepack_home}" @@ -196,6 +216,7 @@ case "${stage}" in ;; root) prepare_rust_dependencies + prepare_frontend_dependencies run_root_partitions ;; crates) @@ -204,6 +225,7 @@ case "${stage}" in ;; groups) prepare_rust_dependencies + prepare_frontend_dependencies REBORN_GROUP_TEST_TIMEOUT="${REBORN_GROUP_TEST_TIMEOUT:-28m}" \ RUST_MIN_STACK=67108864 \ run "${repo_root}/scripts/ci/run-reborn-group-tests.sh" @@ -232,10 +254,14 @@ case "${stage}" in ;; prepare-command) prepare_rust_dependencies + if [[ "${IRONCLAW_HERMETIC_NEXTEST_CONTROL:-0}" == "1" ]]; then + prepare_frontend_dependencies + fi ;; all) "${repo_root}/scripts/ci/test-hermetic-test-process.sh" prepare_rust_dependencies + prepare_frontend_dependencies run_crate_tests run_root_partitions REBORN_GROUP_TEST_TIMEOUT="${REBORN_GROUP_TEST_TIMEOUT:-28m}" \ @@ -256,7 +282,10 @@ case "${stage}" in exit 2 fi case "${IRONCLAW_HERMETIC_SUITE_SKIP_PREPARE:-0}" in - 0) prepare_rust_dependencies ;; + 0) + prepare_rust_dependencies + prepare_frontend_dependencies + ;; 1) ;; *) echo "IRONCLAW_HERMETIC_SUITE_SKIP_PREPARE must be 0 or 1" >&2 diff --git a/scripts/ci/test-hermetic-deterministic-suite-runner.sh b/scripts/ci/test-hermetic-deterministic-suite-runner.sh index 218e146892f..20f9a791b17 100755 --- a/scripts/ci/test-hermetic-deterministic-suite-runner.sh +++ b/scripts/ci/test-hermetic-deterministic-suite-runner.sh @@ -46,6 +46,9 @@ chmod +x "${sandbox}/repo/scripts/ci/"*.sh cat >"${sandbox}/bin/cargo" <<'STUB' #!/usr/bin/env bash printf 'cargo %s\n' "$*" >>"${HERMETIC_TEST_LOG}" +if [[ "${HERMETIC_FAIL_GROUP:-0}" == "1" && "$*" == *'--test reborn_group_fixture'* ]]; then + exit 1 +fi STUB cat >"${sandbox}/bin/cargo-nextest" <<'STUB' #!/usr/bin/env bash @@ -59,6 +62,12 @@ STUB cat >"${sandbox}/bin/corepack" <<'STUB' #!/usr/bin/env bash printf 'corepack %s\n' "$*" >>"${HERMETIC_TEST_LOG}" +mkdir -p "${COREPACK_HOME:?}" +printf 'prepared by corepack\n' >"${COREPACK_HOME}/prepared-pnpm" +if [[ "${1:-}" == "pnpm" && "${2:-}" == "install" ]]; then + mkdir -p node_modules + printf 'prepared project dependencies\n' >node_modules/.hermetic-fixture +fi exit 0 STUB cat >"${sandbox}/bin/timeout" <<'STUB' @@ -74,14 +83,17 @@ STUB chmod +x "${sandbox}/bin/"* run_suite() { - local stage="$1" ci_value="$2" with_nextest="$3" + local stage="$1" ci_value="$2" with_nextest="$3" fail_group="${4:-0}" root_control="${5:-0}" corepack_mode="${6:-none}" local -a env_args=( env -u CI HERMETIC_TEST_REPO_ROOT="${sandbox}/repo" HERMETIC_TEST_LOG="${sandbox}/commands.log" + HERMETIC_FAIL_GROUP="${fail_group}" + IRONCLAW_HERMETIC_NEXTEST_CONTROL="${root_control}" PATH="${sandbox}/bin:/usr/bin:/bin" ) [[ "${ci_value}" == set ]] && env_args+=(CI=true) + [[ "${corepack_mode}" == caller ]] && env_args+=(COREPACK_HOME="${sandbox}/caller-corepack") if [[ "${with_nextest}" == no ]]; then rm -f "${sandbox}/bin/cargo-nextest" else @@ -94,16 +106,42 @@ run_suite() { failures=0 : >"${sandbox}/commands.log" -if ! run_suite prepare-command unset no >"${sandbox}/prepare-command.log" 2>&1; then +if ! run_suite prepare-command unset no 0 1 >"${sandbox}/prepare-command.log" 2>&1; then echo "FAIL: prepare-command stage failed" >&2 cat "${sandbox}/prepare-command.log" >&2 failures=$((failures + 1)) elif ! grep -Fq 'corepack pnpm install --frozen-lockfile' "${sandbox}/commands.log"; then - echo "FAIL: prepare-command did not provision frontend dependencies for Rust build scripts" >&2 + echo "FAIL: root-control preparation did not provision frontend dependencies for Rust build scripts" >&2 cat "${sandbox}/commands.log" >&2 failures=$((failures + 1)) fi +: >"${sandbox}/commands.log" +if ! run_suite prepare-command unset no 0 1 caller >"${sandbox}/prepare-command-caller-corepack.log" 2>&1; then + echo "FAIL: prepare-command with caller-provided COREPACK_HOME failed" >&2 + cat "${sandbox}/prepare-command-caller-corepack.log" >&2 + failures=$((failures + 1)) +elif [[ ! -f "${sandbox}/caller-corepack/prepared-pnpm" ]]; then + echo "FAIL: caller-provided COREPACK_HOME was not populated" >&2 + failures=$((failures + 1)) +elif [[ ! -f "${sandbox}/repo/frontend/node_modules/.hermetic-fixture" ]]; then + echo "FAIL: prepared project dependencies were not left visible" >&2 + failures=$((failures + 1)) +fi + +: >"${sandbox}/commands.log" +if run_suite prepare-command unset no >"${sandbox}/prepare-command-no-frontend.log" 2>&1; then + if grep -Fq 'corepack pnpm install --frozen-lockfile' "${sandbox}/commands.log"; then + echo "FAIL: ordinary prepare-command installed frontend dependencies" >&2 + cat "${sandbox}/commands.log" >&2 + failures=$((failures + 1)) + fi +else + echo "FAIL: ordinary prepare-command stage failed" >&2 + cat "${sandbox}/prepare-command-no-frontend.log" >&2 + failures=$((failures + 1)) +fi + for stage in crates integration; do : >"${sandbox}/commands.log" if run_suite "${stage}" set no >"${sandbox}/${stage}-ci.log" 2>&1; then @@ -127,6 +165,21 @@ for stage in crates integration; do fi done +: >"${sandbox}/commands.log" +if run_suite integration set yes 1 >"${sandbox}/integration-group-failure.log" 2>&1; then + echo "FAIL: integration returned success after a group suite failed" >&2 + failures=$((failures + 1)) +else + if ! grep -Fq 'cargo test -p ironclaw_integration_tests --test reborn_group_fixture' "${sandbox}/commands.log"; then + echo "FAIL: integration did not preserve canonical cargo execution for group suites" >&2 + failures=$((failures + 1)) + fi + if ! grep -F 'nextest ' "${sandbox}/commands.log" | grep -Fq 'reborn_integration_fixture'; then + echo "FAIL: integration did not run flat suites after a group failure" >&2 + failures=$((failures + 1)) + fi +fi + : >"${sandbox}/commands.log" if ! run_suite integration set yes >"${sandbox}/integration-nextest.log" 2>&1; then echo "FAIL: integration nextest caller sandbox failed" >&2 diff --git a/scripts/ci/test_ws12_workflow_contracts.py b/scripts/ci/test_ws12_workflow_contracts.py index 8921ddf0e69..421603d81d0 100755 --- a/scripts/ci/test_ws12_workflow_contracts.py +++ b/scripts/ci/test_ws12_workflow_contracts.py @@ -1891,6 +1891,26 @@ def test_reintroducing_a_bare_cd_site_fails_loudly(self) -> None: errors, ) + def test_a_yaml_workflow_hardcoding_the_flat_site_fails_loudly(self) -> None: + """The same flat-path guard applies to GitHub's `.yaml` extension.""" + sabotaged = copy.deepcopy(self.workflows) + sabotaged[".github/workflows/sneaky.yaml"] = ( + "jobs:\n" + " frontend:\n" + " steps:\n" + f" - run: cd {self.webui_dir}/frontend\n" + ) + + errors = validate_webui_frontend_sites(sabotaged, ROOT) + + self.assertTrue( + any( + "sneaky.yaml" in error and "hardcodes" in error + for error in errors + ), + errors, + ) + def test_reintroducing_a_bare_working_directory_site_fails_loudly(self) -> None: sabotaged = self.sabotage( CODE_STYLE_WORKFLOW, @@ -3347,6 +3367,13 @@ def test_eval_after_shell_command_separators_fails(self): errors = validate_no_eval_in_workflow_run_blocks(workflows) self.assertTrue(any("x.yml:1" in error for error in errors), errors) + def test_eval_after_shell_negation_fails(self): + workflows = { + ".github/workflows/x.yml": ' ! eval "$REPRO"\n' + } + errors = validate_no_eval_in_workflow_run_blocks(workflows) + self.assertTrue(any("x.yml:1" in error for error in errors), errors) + def test_eval_after_shell_control_words_fails(self): for command in ( 'if ready; then eval "$REPRO"; fi', @@ -3411,6 +3438,61 @@ def test_loader_includes_yaml_extension(self): ) +class NoPlannerDerivedMatrixInShellTests(unittest.TestCase): + """Planner-derived matrix values do not become shell source.""" + + def test_root_test_command_rejects_direct_matrix_interpolation(self): + workflows = { + ".github/workflows/reborn-tests.yml": ( + " root-reborn-parity-tests:\n" + " steps:\n" + " - name: Run Reborn root tests\n" + " run: |\n" + ' cmd=(env "REBORN_ROOT_TEST_PARTITION=${{ matrix.partition }}")\n' + ) + } + errors = ws12_workflow_contracts.validate_no_matrix_interpolation_in_root_test_command( + workflows + ) + self.assertTrue(any("matrix" in error for error in errors), errors) + + def test_root_test_command_uses_the_job_environment(self): + workflows = { + ".github/workflows/reborn-tests.yml": ( + " root-reborn-parity-tests:\n" + " steps:\n" + " - name: Run Reborn root tests\n" + " run: |\n" + ' cmd=(env IRONCLAW_HERMETIC_SUITE_SKIP_PREPARE=1\n' + ' "REBORN_ROOT_TEST_PARTITION=${REBORN_ROOT_TEST_PARTITION}")\n' + ) + } + self.assertEqual( + [], + ws12_workflow_contracts.validate_no_matrix_interpolation_in_root_test_command( + workflows + ), + ) + + def test_root_test_command_requires_skip_prepare_after_control(self): + workflows = { + ".github/workflows/reborn-tests.yml": ( + " root-reborn-parity-tests:\n" + " steps:\n" + " - name: Run Reborn root tests\n" + " run: |\n" + ' cmd=(env "REBORN_ROOT_TEST_PARTITION=${REBORN_ROOT_TEST_PARTITION}")\n' + ) + } + errors = ws12_workflow_contracts.validate_no_matrix_interpolation_in_root_test_command( + workflows + ) + self.assertTrue( + any("IRONCLAW_HERMETIC_SUITE_SKIP_PREPARE" in error for error in errors), + errors, + ) + + if __name__ == "__main__": unittest.main() diff --git a/scripts/ci/ws12_workflow_contracts.py b/scripts/ci/ws12_workflow_contracts.py index a727e6d891e..dadb893b67a 100755 --- a/scripts/ci/ws12_workflow_contracts.py +++ b/scripts/ci/ws12_workflow_contracts.py @@ -1568,7 +1568,7 @@ def validate_crate_scope_filters( # path it replaced. # # Two contracts, one per site shape: -# `validate_webui_frontend_sites` scans every `.github/workflows/*.yml` for +# `validate_webui_frontend_sites` scans every `.github/workflows/*.{yml,yaml}` for # the flat WebUI frontend literal. The ONLY sanctioned shape is the # cache-dependency-path pairing (flat line immediately followed by its # nested sibling); anything else is the dynamic-site regression. @@ -1669,7 +1669,9 @@ def validate_webui_frontend_sites( cache_sites = 0 for path in sorted(workflows): - if not path.startswith(".github/workflows/") or not path.endswith(".yml"): + if not path.startswith(".github/workflows/") or not path.endswith( + (".yml", ".yaml") + ): continue lines = workflows[path].splitlines() for index, raw_line in enumerate(lines): @@ -1742,7 +1744,7 @@ def validate_crate_name_residue( WORKFLOW_EVAL = re.compile( - r"(?:^[ \t]*|(?:;|&&|\|\||\|)[ \t]*|[({][ \t]+|(?:\bthen|\bdo|\))[ \t]+)" + r"(?:^[ \t]*|(?:;|&&|\|\||\|)[ \t]*|![ \t]+|[({][ \t]+|(?:\bthen|\bdo|\))[ \t]+)" r"eval(?=[ \t;&|]|$)", re.MULTILINE, ) @@ -1770,6 +1772,60 @@ def validate_no_eval_in_workflow_run_blocks( return errors +ROOT_TEST_WORKFLOW = ".github/workflows/reborn-tests.yml" +ROOT_TEST_JOB = "root-reborn-parity-tests" +ROOT_TEST_STEP = "Run Reborn root tests" +ROOT_TEST_SKIP_PREPARE_ENV = "IRONCLAW_HERMETIC_SUITE_SKIP_PREPARE=1" +MATRIX_EXPRESSION = re.compile( + r"\$\{\{\s*matrix(?:\s*\.\s*[A-Za-z_][A-Za-z0-9_]*)+\s*\}\}" +) + + +def validate_no_matrix_interpolation_in_root_test_command( + workflows: dict[str, str], +) -> list[str]: + """Keep planner-derived matrix values out of the root test shell source. + + The job-level environment already receives the selected partition. The + command must consume that environment value rather than embedding a + planner-derived GitHub expression into the shell script that runs it. + The preceding nextest-control step also prepares Cargo and WebUI + dependencies, so this command must opt out of repeating that work. This + stays scoped to the one command step; matrix expressions used for display + names and artifact paths remain valid workflow metadata. + """ + + text = workflows.get(ROOT_TEST_WORKFLOW) + if text is None: + return [] + job = job_body(text, ROOT_TEST_JOB) + if job is None: + return [ + f"{ROOT_TEST_WORKFLOW}: missing {ROOT_TEST_JOB!r} job, so the root " + "test command cannot be checked for direct matrix interpolation" + ] + step = step_body(job, ROOT_TEST_STEP) + if step is None: + return [ + f"{ROOT_TEST_WORKFLOW}: missing {ROOT_TEST_STEP!r} step in " + f"{ROOT_TEST_JOB!r}, so its shell command cannot be checked" + ] + + errors = [ + f"{ROOT_TEST_WORKFLOW}: {ROOT_TEST_STEP!r} embeds planner-derived " + f"{match.group(0)!r} in shell source; consume " + "REBORN_ROOT_TEST_PARTITION from the job environment instead" + for match in MATRIX_EXPRESSION.finditer(step) + ] + if ROOT_TEST_SKIP_PREPARE_ENV not in step: + errors.append( + f"{ROOT_TEST_WORKFLOW}: {ROOT_TEST_STEP!r} must pass " + f"{ROOT_TEST_SKIP_PREPARE_ENV} after the hermetic nextest-control " + "step prepares Cargo and WebUI dependencies" + ) + return errors + + KEY_LINE = re.compile( r"^(?P[ ]*)(?P-[ ]+)?(?P\"[^\"]+\"|'[^']+'|[A-Za-z_][\w.\-]*)[ ]*:" r"(?P[ ].*|)$" @@ -1886,6 +1942,7 @@ def validate_workflow_texts( errors.extend(validate_webui_frontend_sites(workflows, root)) errors.extend(validate_no_duplicate_yaml_keys(root)) errors.extend(validate_no_eval_in_workflow_run_blocks(workflows)) + errors.extend(validate_no_matrix_interpolation_in_root_test_command(workflows)) try: sccache_action = (root / SCCACHE_SETUP_ACTION).read_text(encoding="utf-8") except OSError as error: From 2f9a7d2ab772791b7ccfda346f86cd4b8ef99384 Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Tue, 25 Aug 2026 05:39:56 +0000 Subject: [PATCH 18/19] Persist Corepack cache for root hermetic control The control and probe run in separate workflow processes, so give the root job the same caller-owned Corepack home as the other guarded Rust lanes and pin its presence/order with workflow contracts. --- .github/workflows/reborn-tests.yml | 3 + scripts/ci/test-hermetic-test-process.sh | 4 +- scripts/ci/test_ws12_workflow_contracts.py | 49 +++++++++++++++++ scripts/ci/ws12_workflow_contracts.py | 64 ++++++++++++++++++++++ 4 files changed, 118 insertions(+), 2 deletions(-) diff --git a/.github/workflows/reborn-tests.yml b/.github/workflows/reborn-tests.yml index 2056c425152..0cf312108fb 100644 --- a/.github/workflows/reborn-tests.yml +++ b/.github/workflows/reborn-tests.yml @@ -560,6 +560,9 @@ jobs: cache-ssh-known-hosts: ${{ secrets.SCCACHE_CACHE_SSH_KNOWN_HOSTS }} redis-password: ${{ secrets.SCCACHE_REDIS_PASSWORD }} + - name: Configure isolated Corepack path + run: echo "COREPACK_HOME=${RUNNER_TEMP}/ironclaw-corepack" >> "$GITHUB_ENV" + # The hermetic wrapper's network guard must survive nextest's own # process spawning, not just a directly-launched test binary. This is # the only place that control can run: it shells out to cargo, so it diff --git a/scripts/ci/test-hermetic-test-process.sh b/scripts/ci/test-hermetic-test-process.sh index 29b4345c466..0b531866f52 100755 --- a/scripts/ci/test-hermetic-test-process.sh +++ b/scripts/ci/test-hermetic-test-process.sh @@ -357,8 +357,8 @@ corepack_path_count="$( 'COREPACK_HOME=${RUNNER_TEMP}/ironclaw-corepack' \ "${repo_root}/.github/workflows/reborn-tests.yml" )" -if [[ "${corepack_path_count}" != "3" ]]; then - echo "expected three guarded Rust lanes to pin COREPACK_HOME, found ${corepack_path_count}" >&2 +if [[ "${corepack_path_count}" != "4" ]]; then + echo "expected four guarded Rust lanes to pin COREPACK_HOME, found ${corepack_path_count}" >&2 exit 1 fi diff --git a/scripts/ci/test_ws12_workflow_contracts.py b/scripts/ci/test_ws12_workflow_contracts.py index 421603d81d0..4d5f31062ef 100755 --- a/scripts/ci/test_ws12_workflow_contracts.py +++ b/scripts/ci/test_ws12_workflow_contracts.py @@ -3493,6 +3493,55 @@ def test_root_test_command_requires_skip_prepare_after_control(self): ) +class RootTestJobSetupContractTests(unittest.TestCase): + """The root job shares one prepared Cargo/Corepack environment.""" + + def workflow(self, corepack_step: str) -> dict[str, str]: + return { + ".github/workflows/reborn-tests.yml": ( + " root-reborn-parity-tests:\n" + " steps:\n" + f"{corepack_step}" + " - name: Hermetic nextest network-guard control\n" + " env:\n" + ' IRONCLAW_HERMETIC_NEXTEST_CONTROL: "1"\n' + " run: scripts/ci/run-hermetic-deterministic-suite.sh prepare-command\n" + ) + } + + def test_exact_corepack_setup_before_control_passes(self): + workflows = self.workflow( + " - name: Configure isolated Corepack path\n" + ' run: echo "COREPACK_HOME=${RUNNER_TEMP}/ironclaw-corepack" >> "$GITHUB_ENV"\n' + ) + self.assertEqual( + [], + ws12_workflow_contracts.validate_root_test_job_setup(workflows), + ) + + def test_corepack_setup_sabotage_fails_loudly(self): + workflows = self.workflow( + " - name: Configure isolated Corepack path\n" + ' run: echo "COREPACK_HOME=${RUNNER_TEMP}/wrong-corepack" >> "$GITHUB_ENV"\n' + ) + errors = ws12_workflow_contracts.validate_root_test_job_setup(workflows) + self.assertTrue(any("COREPACK_HOME" in error for error in errors), errors) + + def test_corepack_setup_after_control_fails_loudly(self): + workflows = { + ".github/workflows/reborn-tests.yml": ( + " root-reborn-parity-tests:\n" + " steps:\n" + " - name: Hermetic nextest network-guard control\n" + " run: scripts/ci/run-hermetic-deterministic-suite.sh prepare-command\n" + " - name: Configure isolated Corepack path\n" + ' run: echo "COREPACK_HOME=${RUNNER_TEMP}/ironclaw-corepack" >> "$GITHUB_ENV"\n' + ) + } + errors = ws12_workflow_contracts.validate_root_test_job_setup(workflows) + self.assertTrue(any("must precede" in error for error in errors), errors) + + if __name__ == "__main__": unittest.main() diff --git a/scripts/ci/ws12_workflow_contracts.py b/scripts/ci/ws12_workflow_contracts.py index dadb893b67a..9a65efc0e68 100755 --- a/scripts/ci/ws12_workflow_contracts.py +++ b/scripts/ci/ws12_workflow_contracts.py @@ -1776,6 +1776,11 @@ def validate_no_eval_in_workflow_run_blocks( ROOT_TEST_JOB = "root-reborn-parity-tests" ROOT_TEST_STEP = "Run Reborn root tests" ROOT_TEST_SKIP_PREPARE_ENV = "IRONCLAW_HERMETIC_SUITE_SKIP_PREPARE=1" +ROOT_TEST_COREPACK_STEP = "Configure isolated Corepack path" +ROOT_TEST_CONTROL_STEP = "Hermetic nextest network-guard control" +ROOT_TEST_COREPACK_SETUP = ( + 'run: echo "COREPACK_HOME=${RUNNER_TEMP}/ironclaw-corepack" >> "$GITHUB_ENV"' +) MATRIX_EXPRESSION = re.compile( r"\$\{\{\s*matrix(?:\s*\.\s*[A-Za-z_][A-Za-z0-9_]*)+\s*\}\}" ) @@ -1826,6 +1831,64 @@ def validate_no_matrix_interpolation_in_root_test_command( return errors +def validate_root_test_job_setup(workflows: dict[str, str]) -> list[str]: + """Require the root job's exact shared Corepack setup before its control.""" + + text = workflows.get(ROOT_TEST_WORKFLOW) + if text is None: + return [] + job = job_body(text, ROOT_TEST_JOB) + if job is None: + return [ + f"{ROOT_TEST_WORKFLOW}: missing {ROOT_TEST_JOB!r} job, so its " + "Corepack setup cannot be checked" + ] + + corepack = step_body(job, ROOT_TEST_COREPACK_STEP) + if corepack is None: + return [ + f"{ROOT_TEST_WORKFLOW}: {ROOT_TEST_JOB!r} must contain the " + f"{ROOT_TEST_COREPACK_STEP!r} step" + ] + errors: list[str] = [] + corepack_lines = [line.strip() for line in corepack.splitlines() if line.strip()] + if not corepack_lines or corepack_lines[0] != ROOT_TEST_COREPACK_SETUP: + errors.append( + f"{ROOT_TEST_WORKFLOW}: {ROOT_TEST_COREPACK_STEP!r} must run " + f"{ROOT_TEST_COREPACK_SETUP!r}" + ) + + control = step_body(job, ROOT_TEST_CONTROL_STEP) + if control is None: + return errors + [ + f"{ROOT_TEST_WORKFLOW}: {ROOT_TEST_JOB!r} must contain the " + f"{ROOT_TEST_CONTROL_STEP!r} step after Corepack setup" + ] + corepack_heading = next( + ( + heading + for heading in STEP_HEADING.finditer(job) + if heading.group("name").strip() == ROOT_TEST_COREPACK_STEP + ), + None, + ) + control_heading = next( + ( + heading + for heading in STEP_HEADING.finditer(job) + if heading.group("name").strip() == ROOT_TEST_CONTROL_STEP + ), + None, + ) + if corepack_heading is not None and control_heading is not None: + if corepack_heading.start() > control_heading.start(): + errors.append( + f"{ROOT_TEST_WORKFLOW}: {ROOT_TEST_COREPACK_STEP!r} must " + f"precede {ROOT_TEST_CONTROL_STEP!r}" + ) + return errors + + KEY_LINE = re.compile( r"^(?P[ ]*)(?P-[ ]+)?(?P\"[^\"]+\"|'[^']+'|[A-Za-z_][\w.\-]*)[ ]*:" r"(?P[ ].*|)$" @@ -1943,6 +2006,7 @@ def validate_workflow_texts( errors.extend(validate_no_duplicate_yaml_keys(root)) errors.extend(validate_no_eval_in_workflow_run_blocks(workflows)) errors.extend(validate_no_matrix_interpolation_in_root_test_command(workflows)) + errors.extend(validate_root_test_job_setup(workflows)) try: sccache_action = (root / SCCACHE_SETUP_ACTION).read_text(encoding="utf-8") except OSError as error: From 03853386be6ecdc86552d3ac88c3f9f0f7a28389 Mon Sep 17 00:00:00 2001 From: Henry Park <16583448+henrypark133@users.noreply.github.com> Date: Tue, 25 Aug 2026 05:43:37 +0000 Subject: [PATCH 19/19] Cover matrix index syntax in workflow guard Reject both dot-property and quoted bracket references when planner-derived matrix values would enter root-test shell source. --- scripts/ci/test_ws12_workflow_contracts.py | 18 ++++++++++++++++++ scripts/ci/ws12_workflow_contracts.py | 6 +++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/scripts/ci/test_ws12_workflow_contracts.py b/scripts/ci/test_ws12_workflow_contracts.py index 4d5f31062ef..43d0529092d 100755 --- a/scripts/ci/test_ws12_workflow_contracts.py +++ b/scripts/ci/test_ws12_workflow_contracts.py @@ -3456,6 +3456,24 @@ def test_root_test_command_rejects_direct_matrix_interpolation(self): ) self.assertTrue(any("matrix" in error for error in errors), errors) + def test_root_test_command_rejects_quoted_matrix_index_interpolation(self): + for expression in ("${{ matrix['partition'] }}", '${{ matrix["partition"] }}'): + with self.subTest(expression=expression): + workflows = { + ".github/workflows/reborn-tests.yml": ( + " root-reborn-parity-tests:\n" + " steps:\n" + " - name: Run Reborn root tests\n" + " run: |\n" + ' cmd=(env "IRONCLAW_HERMETIC_SUITE_SKIP_PREPARE=1"\n' + f' "REBORN_ROOT_TEST_PARTITION={expression}")\n' + ) + } + errors = ws12_workflow_contracts.validate_no_matrix_interpolation_in_root_test_command( + workflows + ) + self.assertTrue(any("matrix" in error for error in errors), errors) + def test_root_test_command_uses_the_job_environment(self): workflows = { ".github/workflows/reborn-tests.yml": ( diff --git a/scripts/ci/ws12_workflow_contracts.py b/scripts/ci/ws12_workflow_contracts.py index 9a65efc0e68..7d96042a615 100755 --- a/scripts/ci/ws12_workflow_contracts.py +++ b/scripts/ci/ws12_workflow_contracts.py @@ -1782,7 +1782,11 @@ def validate_no_eval_in_workflow_run_blocks( 'run: echo "COREPACK_HOME=${RUNNER_TEMP}/ironclaw-corepack" >> "$GITHUB_ENV"' ) MATRIX_EXPRESSION = re.compile( - r"\$\{\{\s*matrix(?:\s*\.\s*[A-Za-z_][A-Za-z0-9_]*)+\s*\}\}" + r"\$\{\{\s*matrix" + r"(?:" + r"\s*\.\s*[A-Za-z_][A-Za-z0-9_]*" + r"|\s*\[\s*(?:'[^'\r\n]+'|\"[^\"\r\n]+\")\s*\]" + r")+\s*\}\}" )