From 35564248519afce383dac855ede104ba67f9ea65 Mon Sep 17 00:00:00 2001 From: BenKurrek Date: Fri, 24 Jul 2026 14:07:32 -0400 Subject: [PATCH 1/2] fix(coverage): declare RUST_MIN_STACK on the push-to-main coverage lane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Code Coverage` has been red on every push to main for 20+ consecutive commits, aborting with `has overflowed its stack` / `fatal runtime error: stack overflow` (SIGABRT, exit 101) — on a *different* test each time as unrelated PRs shifted which future sat deepest: 096e8f843 unbound_telegram_actor_pairs_via_web_minted_code_… (extension_delivery) 8f4d832f1 duplicate_and_restart_replay_converge_exactly_once::case_1 (extension_ingress) d06bde940 extension_install_survives_independent_reopen (durable) Root cause is a workflow gap, not test depth. libtest gives each test thread a 2 MiB stack. `reborn-tests.yml` splits this package's suites across two jobs and gives each the headroom it needs — `reborn-integration-coverage` carries 8 MiB (llvm-cov inflates the integration harness's async frames; #6609) and `root-reborn-parity-tests` carries 64 MiB (reborn_qa_smoke_scenarios_e2e drives whole turns on the libtest stack, ~10 MiB uninstrumented). `coverage.yml` runs `cargo llvm-cov --workspace`, i.e. BOTH tiers in one job, and declared neither. Set it to the union's requirement, 64 MiB. This also explains why the per-test fixes did not converge: the depth lives in shared harness code (group build -> submit_turn -> composition), so #6609's `Box::pin` lowered one test below the ceiling and the next-deepest test simply became the new failure. The controlled comparison at d06bde940: `Reborn integration coverage (1)` ran reborn_integration_durable instrumented with RUST_MIN_STACK=8388608 and passed, while `Coverage (all-features)`/`Coverage (default)` ran the same suite under the same instrumentation with no setting and SIGABRT'd. Same code, same instrumentation — only the stack size differed. Regression coverage: tests/coverage_lane_stack_headroom.rs pins the invariant on both workflows, sized per tier (whole-workspace lanes need 64 MiB; integration-tier-only lanes need 8 MiB). Verified red before this change (`coverage.yml:coverage … declares no job-level RUST_MIN_STACK`) and green after. Mutation-tested three ways: a below-floor value, a whole-workspace lane set to the integration tier's 8 MiB, and dropping reborn-tests.yml's own value each fail the guard. Non-vacuity assertions keep a renamed job or reworded `run:` line from silently emptying the scan. Note: coverage.yml triggers only on `push: branches: [main]`, so this PR's own CI cannot exercise the fixed lane — it is validated by the guard test plus the CI evidence above, and proven by the next push to main. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/coverage.yml | 17 ++ tests/coverage_lane_stack_headroom.rs | 251 ++++++++++++++++++++++++++ 2 files changed, 268 insertions(+) create mode 100644 tests/coverage_lane_stack_headroom.rs diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 0b4235cc2ae..16f613a6fb2 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -48,6 +48,23 @@ jobs: env: CARGO_PROFILE_DEV_DEBUG: 0 CARGO_PROFILE_TEST_DEBUG: 0 + # `--workspace` below runs BOTH root-package test tiers in one job: the + # Reborn integration suites (tests/integration/*) and the root QA suites + # (tests/reborn_qa_*). Both overrun libtest's 2 MiB default test-thread + # stack and abort the whole run with `has overflowed its stack` (SIGABRT) + # instead of failing a test: + # * integration tier — llvm-cov inflates async frames past the harness's + # own depth (group build -> submit_turn -> composition). reborn-tests. + # yml's reborn-integration-coverage lane carries 8 MiB for this. + # * root QA tier — reborn_qa_smoke_scenarios_e2e drives whole turns on + # the libtest stack and measures ~10 MiB *uninstrumented*, so 8 MiB is + # not enough. reborn-tests.yml's root-reborn-parity-tests lane carries + # 64 MiB for this. + # This job's scope is the union, so it takes the larger of the two. The + # depth lives in shared harness code, so per-test Box::pin only moves the + # crash to the next-deepest test. + # Pinned by tests/coverage_lane_stack_headroom.rs. + RUST_MIN_STACK: "67108864" permissions: id-token: write contents: read diff --git a/tests/coverage_lane_stack_headroom.rs b/tests/coverage_lane_stack_headroom.rs new file mode 100644 index 00000000000..ea6f79b5715 --- /dev/null +++ b/tests/coverage_lane_stack_headroom.rs @@ -0,0 +1,251 @@ +//! Guards the coverage-lane stack-overflow class: every CI job that runs this +//! package's test targets under `cargo llvm-cov` must declare `RUST_MIN_STACK` +//! headroom at the job level, sized to the tier it actually executes. +//! +//! Motivation. libtest gives each test thread a 2 MiB stack by default. Two +//! tiers in this package need more than that, and when either overruns, the +//! process aborts (`has overflowed its stack` / `fatal runtime error: stack +//! overflow`, SIGABRT, exit 101) — killing the whole run rather than failing +//! one test: +//! +//! - **integration tier** (`tests/integration/*`): llvm-cov instrumentation +//! inflates async frames past the harness's own depth (group build -> +//! `submit_turn` -> composition). `reborn-tests.yml`'s +//! `reborn-integration-coverage` lane has carried 8 MiB since #6609. +//! - **root QA tier** (`tests/reborn_qa_*`): `reborn_qa_smoke_scenarios_e2e` +//! drives whole turns on the libtest stack and measures ~10 MiB +//! *uninstrumented*, so 8 MiB does not cover it. +//! `reborn-tests.yml`'s `root-reborn-parity-tests` lane carries 64 MiB. +//! +//! `reborn-tests.yml` splits the two tiers across separate jobs, so each +//! declares only what it needs. `coverage.yml` runs `cargo llvm-cov +//! --workspace`, i.e. **both tiers in one job**, so it needs the larger value. +//! It previously declared neither, which is why `Code Coverage` was red on +//! every push to main for 20+ consecutive commits — on a *different* test each +//! time, as unrelated PRs shifted which future sat deepest: +//! +//! - `unbound_telegram_actor_pairs_via_web_minted_code_…` (extension_delivery) +//! - `duplicate_and_restart_replay_converge_exactly_once::case_1` (extension_ingress) +//! - `extension_install_survives_independent_reopen` (durable) +//! +//! Because the depth lives in shared harness code rather than in any one test, +//! `Box::pin`-ing individual futures only moves the failure to the next-deepest +//! test — the headroom has to be declared per lane. This test pins the +//! invariant on every covered lane so they cannot drift apart again. +//! +//! Out of scope: llvm-cov subcommands that spawn no test threads (`clean`, +//! `show-env`, `report`) and per-crate lanes (`llvm-cov -p `, which never +//! reach this package's `tests/`). + +use std::path::PathBuf; + +/// Headroom for a lane that runs only the integration tier, in bytes (8 MiB) — +/// the value `reborn-tests.yml`'s `reborn-integration-coverage` lane uses. +const INTEGRATION_TIER_BYTES: u64 = 8 * 1024 * 1024; + +/// Headroom for a lane that runs the whole workspace, in bytes (64 MiB) — the +/// value `reborn-tests.yml`'s `root-reborn-parity-tests` lane uses. A +/// whole-workspace lane also executes `tests/reborn_qa_*`, so the integration +/// tier's 8 MiB is not sufficient. +const WHOLE_WORKSPACE_BYTES: u64 = 64 * 1024 * 1024; + +/// Workflows scanned for covered jobs. +const WORKFLOWS: &[&str] = &[ + ".github/workflows/coverage.yml", + ".github/workflows/reborn-tests.yml", +]; + +fn repo_file(relative: &str) -> PathBuf { + let repo_root = std::env::var_os("CARGO_MANIFEST_DIR") + .map(PathBuf::from) + .or_else(|| std::env::current_dir().ok()) + .expect("repo root should be discoverable"); + repo_root.join(relative) +} + +/// One top-level job in a workflow file, with the lines of its block. +struct Job { + name: String, + body: Vec, +} + +/// Which test tier a job executes under llvm-cov, and thus how much headroom it +/// must declare. +enum Scope { + /// `cargo llvm-cov … --workspace` — integration tier *and* root QA tier. + WholeWorkspace, + /// The shared lane runner — integration tier only. + IntegrationTier, +} + +impl Scope { + fn required_bytes(&self) -> u64 { + match self { + Self::WholeWorkspace => WHOLE_WORKSPACE_BYTES, + Self::IntegrationTier => INTEGRATION_TIER_BYTES, + } + } +} + +/// Indentation of a line, in spaces (tabs are invalid in these workflows). +fn indent_of(line: &str) -> usize { + line.len() - line.trim_start_matches(' ').len() +} + +/// Split a workflow into its top-level jobs. Job headers sit at indent 2 under +/// the `jobs:` key; the block runs until the next indent-2 key. +fn parse_jobs(workflow: &str) -> Vec { + let mut jobs = Vec::new(); + let mut in_jobs = false; + let mut current: Option = None; + + for line in workflow.lines() { + let trimmed = line.trim_end(); + if trimmed.is_empty() || trimmed.trim_start().starts_with('#') { + if let Some(job) = current.as_mut() { + job.body.push(line.to_string()); + } + continue; + } + + if indent_of(trimmed) == 0 { + // A new top-level key ends the `jobs:` mapping. + if let Some(job) = current.take() { + jobs.push(job); + } + in_jobs = trimmed.starts_with("jobs:"); + continue; + } + + if in_jobs && indent_of(trimmed) == 2 && trimmed.trim_end().ends_with(':') { + if let Some(job) = current.take() { + jobs.push(job); + } + current = Some(Job { + name: trimmed.trim().trim_end_matches(':').to_string(), + body: Vec::new(), + }); + continue; + } + + if let Some(job) = current.as_mut() { + job.body.push(line.to_string()); + } + } + + if let Some(job) = current.take() { + jobs.push(job); + } + jobs +} + +/// Classify a job by which instrumented test tier it executes, if any. +/// +/// Whole-workspace wins when both shapes appear, since it is the wider scope. +fn instrumented_scope(job: &Job) -> Option { + let mut integration_tier = false; + + for line in &job.body { + if line.contains("reborn-coverage-lane-run.sh") { + integration_tier = true; + } + if !line.contains("cargo llvm-cov") || !line.contains("--workspace") { + continue; + } + // `clean` / `show-env` / `report` do not spawn test threads. + let non_test = ["llvm-cov clean", "llvm-cov show-env", "llvm-cov report"]; + if !non_test.iter().any(|sub| line.contains(sub)) { + return Some(Scope::WholeWorkspace); + } + } + + integration_tier.then_some(Scope::IntegrationTier) +} + +/// Read `RUST_MIN_STACK` from the job-level `env:` mapping (indent 4, keys at +/// indent 6). Step-level `env:` blocks sit deeper and are ignored on purpose — +/// a step-scoped value would not cover the test-running step. +fn job_level_rust_min_stack(job: &Job) -> Option { + let mut in_env = false; + for line in &job.body { + let trimmed = line.trim_end(); + if trimmed.is_empty() || trimmed.trim_start().starts_with('#') { + continue; + } + let indent = indent_of(trimmed); + if in_env { + if indent < 6 { + in_env = false; + } else if indent == 6 { + if let Some(value) = trimmed.trim().strip_prefix("RUST_MIN_STACK:") { + return value.trim().trim_matches(['"', '\'']).parse::().ok(); + } + continue; + } + } + if indent == 4 && trimmed.trim() == "env:" { + in_env = true; + } + } + None +} + +#[test] +fn instrumented_lanes_declare_stack_headroom_for_their_tier() { + let mut whole_workspace = Vec::new(); + let mut integration_tier = Vec::new(); + let mut violations = Vec::new(); + + for workflow in WORKFLOWS { + let contents = std::fs::read_to_string(repo_file(workflow)) + .unwrap_or_else(|e| panic!("{workflow} should be readable: {e}")); + + for job in parse_jobs(&contents) { + let Some(scope) = instrumented_scope(&job) else { + continue; + }; + let required = scope.required_bytes(); + let label = format!("{workflow}:{}", job.name); + + match job_level_rust_min_stack(&job) { + Some(bytes) if bytes >= required => match scope { + Scope::WholeWorkspace => whole_workspace.push(label), + Scope::IntegrationTier => integration_tier.push(label), + }, + Some(bytes) => violations.push(format!( + "{label} declares RUST_MIN_STACK={bytes}, below the {required}-byte floor \ + for the tier it runs" + )), + None => violations.push(format!( + "{label} runs this package's test targets under llvm-cov but declares no \ + job-level RUST_MIN_STACK (needs {required} bytes)" + )), + } + } + } + + assert!( + violations.is_empty(), + "instrumented lanes are missing stack headroom; the suites overrun libtest's 2 MiB \ + default and the run aborts with a stack overflow instead of failing a test:\n {}", + violations.join("\n ") + ); + + // Non-vacuity: the scan must actually reach both known lanes. Without this, + // a renamed job or a reworded `run:` line would silently empty the scan and + // leave the assertion above trivially true. + assert!( + whole_workspace + .iter() + .any(|job| job.starts_with(".github/workflows/coverage.yml:")), + "expected coverage.yml to contribute a whole-workspace instrumented lane; \ + found whole_workspace={whole_workspace:?} integration_tier={integration_tier:?}" + ); + assert!( + integration_tier + .iter() + .any(|job| job.starts_with(".github/workflows/reborn-tests.yml:")), + "expected reborn-tests.yml to contribute an integration-tier instrumented lane; \ + found whole_workspace={whole_workspace:?} integration_tier={integration_tier:?}" + ); +} From 5e69f90061537bd894519dbc528838b871a9eb60 Mon Sep 17 00:00:00 2001 From: BenKurrek Date: Fri, 24 Jul 2026 14:09:49 -0400 Subject: [PATCH 2/2] fix(ci): route the coverage-headroom guard into the Reborn root test lanes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard added in the previous commit never ran: `root-reborn-parity-tests` gates on `has_reborn_tests`, and `classify-test-scope.sh`'s `is_reborn_test_path` matches root suites by the `tests/reborn_*` prefix. `tests/coverage_lane_stack_headroom.rs` did not match, so a PR touching only it and a workflow classified as `has_reborn_tests=false` and skipped every Reborn test lane — the guard was dead weight on exactly the PR shape it exists to police (a workflow edit). Caught on PR CI for this branch: `Reborn root tests` reported `skipping`. Rename to `tests/reborn_coverage_lane_stack_headroom.rs`, matching the convention every other root suite already uses. Verified with the real staged file set: the classifier now reports `has_reborn_tests=true`, and the suite passes under its new target name. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/coverage.yml | 2 +- ...stack_headroom.rs => reborn_coverage_lane_stack_headroom.rs} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename tests/{coverage_lane_stack_headroom.rs => reborn_coverage_lane_stack_headroom.rs} (100%) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 16f613a6fb2..1de246113bb 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -63,7 +63,7 @@ jobs: # This job's scope is the union, so it takes the larger of the two. The # depth lives in shared harness code, so per-test Box::pin only moves the # crash to the next-deepest test. - # Pinned by tests/coverage_lane_stack_headroom.rs. + # Pinned by tests/reborn_coverage_lane_stack_headroom.rs. RUST_MIN_STACK: "67108864" permissions: id-token: write diff --git a/tests/coverage_lane_stack_headroom.rs b/tests/reborn_coverage_lane_stack_headroom.rs similarity index 100% rename from tests/coverage_lane_stack_headroom.rs rename to tests/reborn_coverage_lane_stack_headroom.rs