Skip to content

test(reliability): bound release acceptance CLI subprocesses - #667

Merged
seonghobae merged 2 commits into
mainfrom
fix/release-acceptance-subprocess-deadlines-555
Aug 11, 2026
Merged

test(reliability): bound release acceptance CLI subprocesses#667
seonghobae merged 2 commits into
mainfrom
fix/release-acceptance-subprocess-deadlines-555

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Buyer-visible reliability gap

Advances #555 with a disjoint release-acceptance operation class. Protected main a7be9eb8ca0b807364173f8471fc300e3cf9350f ran each release-acceptance CLI step through scripts/release_acceptance.py::_run_cli() without a subprocess deadline. A hung simulate/fit/diagnostics/report child could therefore hold the acceptance path until the outer job ceiling, and timeout failure was not normalized at the package boundary.

This slice does not overlap #578 (run_ignored_rust_shard.py scientific-study process groups), #657 (build_pr_queue_governance.py GitHub CLI retry behavior), or #666 (build_benchmark_report.py local Git metadata timeout).

Preserved exact RED evidence

Fail-first source head: ca0bb34c31232718d332e3614087d13aa67b7ba0, directly based on protected main a7be9eb8ca0b807364173f8471fc300e3cf9350f.

CI run 31414709391, Python job 93540867087, checked out synthetic merge 9fd675c07bc0a551921e17aadd1ec684fbf066dc and completed with exactly 1 failed, 2948 passed, 2 skipped. The sole failure was tests/test_release_acceptance_subprocess_deadlines.py::test_fit_cli_timeout_is_bounded_and_fails_closed_without_reflection: _run_cli() called subprocess.run without timeout=, the sentinel observed timeout=None, and raw subprocess.TimeoutExpired escaped. This is the intended production boundary, not setup/import/fixture failure.

On that same fail-first integration, Rust workspace/PyO3, package/build/reinstall/release-acceptance smoke, enterprise sales readiness, explicit GPU parity and fuzz were GREEN; Security Scan 31414709319 and SAST 31414709366 were GREEN. The existing marginal _log_sigmoid warning remains #583/#564 and is unrelated.

Current GREEN implementation under verification

Exact current source head: 8463eef25fb864801348966a5d5e6d30232d56ea, still over protected main a7be9eb8ca0b807364173f8471fc300e3cf9350f.

The implementation now:

  • defines an explicit finite deadline map for every fixed acceptance subprocess;
  • gives fit/dimensionality work a 900-second ceiling, diagnose-fit 300 seconds, and simulation/report rendering 120 seconds rather than imposing one tiny universal deadline;
  • passes the selected deadline to subprocess.run;
  • catches subprocess.TimeoutExpired before the existing non-timeout path and raises stable RuntimeError("<package-owned-operation> timed out") without child command/stdout/stderr reflection;
  • rejects unknown internal operation labels before child execution; and
  • preserves normal JSON success/result behavior and the pre-existing ordinary non-timeout child-failure semantics.

Focused tests now cover the original timeout/redaction boundary, complete operation-vocabulary coverage, deadline ordering/bounds, successful timeout forwarding, and unknown-operation fail-closed behavior.

Primary technical basis

Python 3.14's supported high-level subprocess API provides timeout= and raises subprocess.TimeoutExpired; GitHub Actions' outer job/step timeout remains an independent ceiling. These operational deadlines are support/resource boundaries, not scientific recommendations or claims about estimator runtime.

Remaining Draft gate

Keep Draft. Current-head CI/Security/SAST must prove the source+test implementation GREEN before reliability doctoring/changelog material is added. After product GREEN:

  1. add concise reliability: add operation-specific subprocess deadlines and timeout evidence #555 doctoring grounded in current Python/GitHub primary documentation and an authoritative docs/changelog.d fragment;
  2. deterministically render/check managed CHANGELOG.md;
  3. require one unchanged final head with focused/full Python, Rust/PyO3, package/reinstall/release acceptance, explicit GPU-no-skip, fuzz, Security Scan and SAST;
  4. obtain current-head automated/independent review with zero valid unresolved findings and satisfy repository merge policy.

Do not add process-group machinery unless real descendant leakage is demonstrated for this caller, do not shorten scientific work to fit a tiny timeout, and do not touch canonical #604 docs, numerical formulas, dependencies/workflows, package version/release, hosted state or reviewer authority.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 35 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d88f0ef2-8b76-42dd-89c6-47587481a21d

📥 Commits

Reviewing files that changed from the base of the PR and between e8c65b1 and fffd324.

📒 Files selected for processing (2)
  • scripts/release_acceptance.py
  • tests/test_release_acceptance_subprocess_deadlines.py

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

Primary-source feasibility note for the eventual GREEN implementation: current Python 3.14 subprocess.run(..., timeout=...) is the supported high-level boundary; when the timeout expires Python raises subprocess.TimeoutExpired after terminating/waiting for the child. GitHub Actions independently supports step/job timeout-minutes; an unspecified job timeout defaults to 360 minutes, so the package job's outer ceiling is not a substitute for a bounded per-operation release-acceptance deadline. The protected-main package job currently has no explicit job timeout and successfully executes the ordinary small acceptance smoke, so this slice should preserve that behavior while failing a hung child earlier. Record these official Python/GitHub sources in APA 7 doctoring after RED→GREEN; do not claim an arbitrary deadline is a scientific runtime limit.

Copy link
Copy Markdown
Contributor Author

@opencode-agent address

Sole bounded completion writer for Draft #667 on exact head 479edc0b6666cfbc7a9e33fed964534c28de4db8 over protected main a7be9eb8ca0b807364173f8471fc300e3cf9350f. Before writing, refetch head/base/comments and abort if either ref moved or another source writer appeared.

The production timeout implementation was already GREEN on predecessor head 8463eef25fb864801348966a5d5e6d30232d56ea with CI 31415481608, Security Scan 31415481605, and SAST 31415481596 all successful. This maintainer pass then added only docs/doctoring/release_acceptance_subprocess_deadlines.md and docs/changelog.d/555-release-acceptance-subprocess-deadlines.md, grounded in current Python 3.14 subprocess and GitHub Actions timeout documentation.

Do only deterministic completion: run python scripts/render_changelog_fragments.py --update CHANGELOG.md, then --check CHANGELOG.md; run the focused release-acceptance deadline tests, changelog contract, and git diff --check; commit only the renderer-managed CHANGELOG.md synchronization if all pass. Do not change timeout values, production code/tests, canonical #604 docs, dependencies/workflows, package version/release, or reviewer authority. Stop writing after that single deterministic synchronization so fresh exact-head CI/Security/SAST/review can be recreated.

@seonghobae
seonghobae force-pushed the fix/release-acceptance-subprocess-deadlines-555 branch from fd8ab98 to fffd324 Compare August 11, 2026 05:54
@seonghobae
seonghobae merged commit c1df8bc into main Aug 11, 2026
33 checks passed
@seonghobae
seonghobae deleted the fix/release-acceptance-subprocess-deadlines-555 branch August 11, 2026 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant