fix(coverage): declare RUST_MIN_STACK on the push-to-main coverage lane - #6660
Merged
Merged
Conversation
`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: 096e8f8 unbound_telegram_actor_pairs_via_web_minted_code_… (extension_delivery) 8f4d832 duplicate_and_restart_replay_converge_exactly_once::case_1 (extension_ingress) d06bde9 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 d06bde9: `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) <noreply@anthropic.com>
Contributor
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6660
July 24, 2026 18:08
Destroyed
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughCoverage CI now sets a 64 MiB Rust thread stack, and a new test validates stack thresholds for instrumented coverage jobs in both workflow files. ChangesCoverage headroom enforcement
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
risk: medium
…lanes 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) <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6660
July 24, 2026 18:09
Destroyed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/coverage_lane_stack_headroom.rs`:
- Around line 1-3: Update the module documentation and related guard wording
around WORKFLOWS to state that validation covers only the explicitly enumerated
workflow files and their jobs, rather than every CI job. Do not claim universal
coverage unless the implementation is changed to discover all workflow files;
preserve the existing job classification behavior, including its independence
from Job::name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2526a42f-4227-4012-9a3e-3953f15895c4
📒 Files selected for processing (2)
.github/workflows/coverage.ymltests/coverage_lane_stack_headroom.rs
Comment on lines
+1
to
+3
| //! 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. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Bound the guard’s documented coverage.
The test scans only the two fixed paths in WORKFLOWS, not every CI job; also, renaming a job does not empty this scan because classification ignores Job::name. State that it covers enumerated workflows, or discover all workflow files before retaining the universal guarantee.
As per coding guidelines, “Comments promising cross-layer guarantees must be enforced by code or tests, or softened to describe intent.”
Also applies to: 234-236
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/coverage_lane_stack_headroom.rs` around lines 1 - 3, Update the module
documentation and related guard wording around WORKFLOWS to state that
validation covers only the explicitly enumerated workflow files and their jobs,
rather than every CI job. Do not claim universal coverage unless the
implementation is changed to discover all workflow files; preserve the existing
job classification behavior, including its independence from Job::name.
Source: Coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/reborn_coverage_lane_stack_headroom.rs`:
- Around line 168-191: Update job_level_rust_min_stack to distinguish a declared
but malformed RUST_MIN_STACK from an absent variable instead of converting parse
failures to None with .ok(). Return or propagate an explicit parsing error for
malformed values, and update the caller and violation message to report that
error while preserving the existing absent-variable behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 196afb6d-bfb9-4aab-94ba-e3e697b4ae60
📒 Files selected for processing (2)
.github/workflows/coverage.ymltests/reborn_coverage_lane_stack_headroom.rs
Contributor
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 86.23% — 314392 / 364600 lines Per-crate breakdown (61 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
20 of 29 tasks
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Code Coveragehas been red on every push to main for 20+ consecutive commits, aborting withhas 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:096e8f843unbound_telegram_actor_pairs_via_web_minted_code_…extension_delivery8f4d832f1duplicate_and_restart_replay_converge_exactly_once::case_1extension_ingressd06bde940extension_install_survives_independent_reopendurableRoot cause
A workflow gap, not test depth. libtest gives each test thread a 2 MiB stack.
reborn-tests.ymlsplits this package's suites across two jobs and gives each the headroom it needs:reborn-integration-coverage→ 8 MiB — llvm-cov inflates the integration harness's async frames (added in fix(test-infra): repair the #6520 audit fallout — coverage-lane crash, blind auth suites, weakened guards #6609)root-reborn-parity-tests→ 64 MiB —reborn_qa_smoke_scenarios_e2edrives whole turns on the libtest stack (~10 MiB uninstrumented, per that file's own header)coverage.ymlrunscargo llvm-cov --workspace— both tiers in one job — and declared neither. This sets it to the union's requirement, 64 MiB.This also explains why the earlier per-test fixes did not converge: the depth lives in shared harness code (group build →
submit_turn→ composition), so #6609'sBox::pinlowered one test below the ceiling and the next-deepest test simply became the new failure. Boxing individual futures can't fix a lane-wide ceiling.Verification
Local reproduction, same command and instrumentation, only the env differs:
The same contrast is visible in CI at
d06bde940:Reborn integration coverage (1)ranreborn_integration_durableinstrumented withRUST_MIN_STACK=8388608and passed, whileCoverage (all-features)/Coverage (default)ran the same suite under the same instrumentation with no setting and SIGABRT'd.Regression coverage
tests/reborn_coverage_lane_stack_headroom.rspins the invariant across both workflows, sized per tier (whole-workspace lanes need 64 MiB; integration-tier-only lanes need 8 MiB).coverage.yml:coverage runs this package's test targets under llvm-cov but declares no job-level RUST_MIN_STACK; green after.reborn-tests.yml's own value — each fails the guard. (The second mutation is not hypothetical: it caught an 8 MiB value in an earlier draft of this very PR.)run:line from silently emptying the scan and leaving the check trivially true.Reviewer notes
coverage.ymltriggers only onpush: branches: [main], so this PR's own CI cannot exercise the fixed lane. It is covered by the guard test plus the evidence above; the lane itself is proven by the next push to main.On the 64 MiB choice — one honest caveat. I verified the integration-tier need directly (repro above). I did not reproduce the QA-tier need: on macOS/aarch64,
reborn_qa_smoke_scenarios_e2epassed instrumented at 8 MiB (26 passed). The 64 MiB therefore rests on the repo's own documented measurement (tests/reborn_qa_smoke_scenarios_e2e.rsheader: ~10 MiB uninstrumented, i.e. already over 8) and on matchingroot-reborn-parity-tests, whose scope this lane subsumes — not on a measurement of mine on CI's platform. SinceRUST_MIN_STACKreserves virtual address space per thread rather than committing pages, sizing to the sibling lane is the cheap and safe direction. If a reviewer prefers the tighter 8 MiB, the guard'sWHOLE_WORKSPACE_BYTESconstant is the single place to change.The
e2e-coveragejob is deliberately untouched: it runspytestagainst a built binary rather than libtest test threads, and has not been failing. Itsllvm-covcalls (show-env,clean,report) spawn no test threads and are excluded from the guard by name.Per-crate lanes (
llvm-cov -p <pkg>) are also out of scope — they never reach this package'stests/.Second commit (
fix(ci): route the coverage-headroom guard …): the guard's first filename put it outsideclassify-test-scope.sh'stests/reborn_*routing, so the first push classified ashas_reborn_tests=falseand skipped every Reborn test lane — the guard was dead weight on exactly the PR shape it polices. Renamed to the convention every other root suite uses; verified the classifier now reportshas_reborn_tests=truefor this PR's real file set.🤖 Generated with Claude Code