Repository navigation
ci(nextest): give the whole-tree architecture scans real timeout headroom - #8060
Conversation
…room The three reborn_*_location_scan binaries each carry one scan of the entire crates/ tree. On the last green run they finished at 176.8s against the ci profile's 60s x 3 = 180s hard kill; the next run was terminated at 180.008s with no code change that touches their runtime. reborn_sealed_evidence_mint_ratchet rides the same ladder to 136-141s. One override for that family at 60s x 6 keeps the SLOW cadence and stops every PR's architecture bucket being a coin flip; the default stays as is for every other binary. Match verified locally: with the override's period set to 150s no SLOW marker fires (tests take ~110s); with 60s the markers return at 60s. Rollback: delete the override block. Co-Authored-By: Claude Code <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X6JWfzEzWKThikJ9wsHAer (cherry picked from commit f0cdace)
|
🚅 Deployed to the ironclaw-pr-8060 environment in ironclaw-ci-preview
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
github-actions
Bot
added
size: S
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe CI nextest configuration adds a six-minute termination threshold for selected slow-test binaries. ChangesCI timeout configuration
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to This change extends CI timeouts only for the selected slow architecture-test binaries. No remaining merge-readiness risk is identified. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Full details: Description checkExplanation The description is mostly complete and matches the CI timeout configuration change. It includes the summary, change type, linked issue, validation evidence, test strategy, security impact, trust-boundary status, database impact, blast radius, rollback plan, review follow-through, and review track. The Test Strategy uses condensed fields instead of the template's detailed checkbox layout, but it provides the required information for this CI-only change. 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 |
There was a problem hiding this comment.
🟡 Changes recommended
The new nextest override filter should be scoped to package(ironclaw_architecture_tests) to avoid unintentionally matching similarly named binaries in other packages.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adjusts cargo nextest CI timeout policy to stop flaky hard-kills of long-running whole-crates/ architecture scan test binaries in ironclaw_architecture_tests, without increasing the default timeout for the rest of the suite.
Changes:
- Adds a
profile.cioverride that increases the hard-kill window for_location_scanandreborn_sealed_evidence_mint_ratchetbinaries from 180s to 360s. - Documents the rationale and observed runtimes directly in
nextest.tomlfor future maintainers.
File summaries
| File | Description |
|---|---|
| .config/nextest.toml | Adds a CI-only nextest override to give architecture scan binaries more termination headroom while keeping the default timeout unchanged. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| [[profile.ci.overrides]] | ||
| filter = 'binary(/_location_scan$/) | binary(~reborn_sealed_evidence_mint_ratchet)' | ||
| slow-timeout = { period = "60s", terminate-after = 6 } |
Review · Status🟩 CompletedIronLoop completed the review and posted it to GitHub. ResultRun detailsAutomatic trigger · attempt 1 of 3 · completed in 3m 55s |
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review · Summary
🟢 No actionable findings
No additional actionable findings. Existing unresolved review feedback was inspected and not duplicated.
Validation
- ✅ Nextest configuration — The changed configuration parses as TOML and contains the intended CI slow-timeout override of 60s × 6.
- ✅ Current CI scope — Static target inventory found the four currently matching architecture-test binaries, and the CI crate lane invokes Nextest with the ci profile.
Review details
- Run:
571481cc-9b1e-4205-ad5b-490576be5bbb - Attempts: 1
lloydmak99
left a comment
lloydmak99
left a comment
There was a problem hiding this comment.
The change only widens the CI timeout headroom for the architecture-scan binaries (60s × 6 → 360s hard kill), with no production, data, or API-contract impact. Config is well-formed and correctly scoped.
.config/nextest.toml:newprofile.ci.overrides[5]parses cleanly and is well-formed (period="60s",terminate-after=6).- The filter
binary(/_location_scan$/) | binary(~reborn_sealed_evidence_mint_ratchet)matches exactly the 4 intended binaries undercrates/app/ironclaw_architecture_tests/tests/, with no repo-wide collisions and no precedence conflict with the pre-existingreborn_group_/e2e/heavy_integrationoverrides. - Worst case a real hang now surfaces at 360s instead of 180s, still well within the workflow's outer timeout cap.
Local checks: TOML parsed via tomllib; git diff --check clean; filterset validated against real cargo-nextest 0.9.143 via an A/B show-config (broken copy rejected, PR's filter parses and advances to build); targeted CI planner/workflow contract tests passed. Full cargo nextest run skipped (no cc linker in sandbox) — not needed for a timeout-only change.
lloydmak99
left a comment
lloydmak99
left a comment
There was a problem hiding this comment.
Well-scoped CI-config change: adds one [[profile.ci.overrides]] block giving the four architecture-scan binaries 60s × 6 = 360s before termination (up from the default 180s), keeping the 60s SLOW cadence. TOML parses cleanly, the new block is correctly last-placed so it wins precedence, and the filter matches only the four intended targets.
Optional follow-up (non-blocking):
.config/nextest.toml:77— the filterbinary(/_location_scan$/) | binary(~reborn_sealed_evidence_mint_ratchet)isn't package-scoped, so a future binary with a matching name added to any other crate would silently inherit the 360s timeout. No such collision exists today. Could prefix withpackage(ironclaw_architecture_tests) & (...)if desired.
Local checks: TOML validity + override precedence verified via Python tomllib (6 CI overrides; new block resolves to {period=60s, terminate-after=6}); find/grep across crates/ confirmed only the four intended binaries match, all in ironclaw_architecture_tests; CI logs confirmed the PR run passing 1025/1025. Live cargo nextest filterset validation skipped — cargo-nextest not installed in this environment.
Summary
ironclaw_architecture_testsbinariesreborn_{extension_contract,loop_port,product_contract}_location_scaneach run one whole-crates/-tree scan. On the last green run of feat(loop): derive the prompt context budget from the model's advertised window #8053 they finished at 176.8 s against theciprofile's60s × 3 = 180 shard kill; the very next run, with no code change touching their runtime, was terminated at 180.008 s.reborn_sealed_evidence_mint_ratchetrides the same ladder to 136–141 s.[[profile.ci.overrides]]for that family at60s × 6(same SLOW cadence, real headroom). The default is unchanged for every other binary.Change Type
Linked Issue
None — observed on #8053's CI (runs 33803597916 green at 176.8 s, 33810122502 terminated at 180.008 s).
Validation
periodtemporarily set to150s,cargo nextest run --profile ci -p ironclaw_architecture_tests -E 'binary(/_location_scan$/)'produced no SLOW marker (tests take ~110 s locally); with60srestored,SLOW [> 60.000s]fires at 60 s for the three scans and all 21 tests pass.Test Strategy
User behavior: none. Risk areas: none (CI timeout policy for four test binaries).
Tests added or updated: Not applicable: nextest configuration.
What the tests prove: n/a. Commands run: see Validation.
Security Impact
None.
Reborn Trust-Boundary Checklist
N/A — CI configuration.
Database Impact
None.
Blast Radius
Four architecture-test binaries get 360 s instead of 180 s before termination. A genuine hang in one of them now takes 6 minutes to surface instead of 3.
Rollback Plan
Delete the override block.
Review Follow-Through
The same commit is cherry-picked onto #8053 so its architecture bucket stops flipping; when this lands it becomes a no-op there.
Review track: C (CI)
🤖 Generated with Claude Code