Scope spec retries to transient failures, enable random ordering, tier timeouts (#382)

[justin808]

Closes #382.

Single-file change to spec/spec_helper.rb (stacked on #414; retargets to main when it merges):

1. Retries scoped to transient live-API failures via rspec-retry's exceptions_to_retry: network/socket/SSL errors, Timeout::Error (covers Net::OpenTimeout/ReadTimeout), and a custom matcher for the bare RuntimeError that ControlplaneApiDirect#call raises on 5xx/429 (all 5xx classes 500–511 + the Net::HTTPServerError fallback for unregistered codes). Assertion/deterministic failures now fail on the first attempt everywhere — including live runs.
2. RSPEC_RETRY_COUNT normalized with Integer(ENV.fetch("RSPEC_RETRY_COUNT", "3")) — empty/garbage values fail at load instead of silently degrading.
3. Random ordering enabled: config.order = :random + Kernel.srand config.seed; RSpec prints Randomized with seed N in every run (CI logs carry the repro seed natively).
4. Tiered per-example timeout: 600s when a live org is configured or the example is :slow; 60s for offline/unit runs. Live untagged specs keep their full original budget — this cannot tighten live CI.

Codex Decision Log

• Non-blocking: No :integration tag exists (the Test suite cannot load without CPLN_ORG — even pure unit specs require live-org configuration #373 taxonomy is live-vs-offline + :slow), so metadata-scoped retries would need path lists.
  • Decision: Exception-class scoping instead — transient failures retry regardless of file; deterministic failures never retry.
  • Why: Verified against rspec-retry 0.6.2 source (exception.is_a?(klass) || klass === exception) and empirically (assertion failure: 1 attempt; Errno::ECONNRESET/fake-5xx: retried and passed).
  • Review later: Live failures that surface as assertion failures (e.g. via run_cpflow_command converting API errors to exit status) no longer retry — watch live flake rates after merge.
• Non-blocking: Hangs cut off by the tiered Timeout.timeout hook surface as Timeout::ExitException (not Timeout::Error) and are not retried — they fail fast on attempt 1.
  • Decision: Documented as intended behavior.
  • Why: Test flakiness hygiene: scope retries to live-API specs, enable random ordering, tier timeouts #382 itself complains a hung spec blocks CI for 10 minutes; retrying a hang would triple that.
  • Review later: None.
• Non-blocking: Timeout tiering keyed off cpln_org_configured? (per-process, frozen at load) rather than per-example tags.
  • Decision: Safe branch — live runs keep 600s for every example; only offline runs tighten to 60s (slowest offline group avg 0.04s).
  • Review later: Once per-spec live tagging exists, tighten fast live specs too.
• Non-blocking: MultipleExceptionError (transient error + after-hook failure) is not retried under exception scoping.
  • Decision: Accepted edge case.
  • Review later: Only if live CI shows retry-worthy multi-exception failures.

Confidence

• Offline suite (documented README list incl. the 8 new Add offline unit specs for eight untested lib/ files (#379) #414 specs): 195 examples, 0 failures across seeds 101, 1, 7777, 31337, 999983, 24601, 86753; also green with RSPEC_RETRY_COUNT=0 (seed 555).
• Retry scoping verified empirically with a throwaway probe (assertion → no retry; ECONNRESET/5xx → retried), probe deleted.
• Independent skeptical review against rspec-retry 0.6.2 + timeout gem sources: no blockers; all three findings (two comment-accuracy, one 5xx-class coverage) fixed and revalidated.
• Known first-run risk: 541 live examples get their first-ever shuffled execution — design reviewed as order-safe (suite-level fixtures, SecureRandom suffixes unaffected by srand), but watch the first live CI run; the printed seed reproduces any failure.
• Note: RSpec (Fast) is currently red on all PRs from a pre-existing shared-org regression (see Add offline unit specs for eight untested lib/ files (#379) #414 triage comment; identical 13 failures on Fix scheduled slow-suite regressions #413) — unrelated to this diff.

Batch: CPFLOW B 07-16 15:43 (cpf-b-20260716-1543), lane issue382, epic #387.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5fb43778-1d96-4995-b267-aef95433057e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch justin808/issue-382

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Greptile Summary

This PR makes RSpec execution faster to diagnose and more selective about retries. The main changes are:

• Retries only transient network, timeout, HTTP 5xx, and rate-limit failures.
• Validates RSPEC_RETRY_COUNT when the suite loads.
• Enables randomized ordering with reproducible seeds.
• Uses 60-second offline and 600-second live or slow-example timeouts.

Confidence Score: 5/5

This looks safe to merge.

• No blocking issues found in the changed code.
• The retry matcher follows the configured gem matching contract.
• Invalid retry counts and non-retried example-level timeouts are intentional behaviors.

Important Files Changed

Filename	Overview
spec/spec_helper.rb	Adds transient retry matching, strict retry-count parsing, randomized ordering, and tiered example timeouts.

_{Reviews (1): Last reviewed commit: "Scope spec retries to transient failures..." | Re-trigger Greptile}

[claude]

Review: Scope spec retries to transient failures, enable random ordering, tier timeouts (#382)

Single-file change to spec/spec_helper.rb. Traced through ControlplaneApiDirect#call's bare RuntimeError message format, Ruby's Net::HTTP response class hierarchy, and where Timeout/OpenSSL get loaded relative to the new top-level constants.

Overview

1. TransientServerHTTPError — a custom === matcher that scopes retryable RuntimeErrors to the 5xx/429 subset raised by ControlplaneApiDirect#call.
2. TRANSIENT_FAILURE_MATCHERS wired into config.exceptions_to_retry, so only transient network/timeout/5xx failures retry; assertion and other deterministic failures fail on attempt 1 everywhere, including live runs.
3. RSPEC_RETRY_COUNT normalized via Integer(ENV.fetch(...)) instead of ||, so garbage/empty values raise at load time instead of silently degrading.
4. config.order = :random + Kernel.srand config.seed uncommented/enabled (previously dead boilerplate).
5. Tiered per-example timeout: 600s for live-org or :slow examples, 60s otherwise (was a flat 600s for everything).

Correctness

• Verified ControlplaneApiDirect#call's raise("#{response} #{response.body}") produces response.to_s, which is Ruby's default Object#to_s format (#<Net::HTTPInternalServerError:0x...>) since Net::HTTPResponse doesn't override to_s. The MESSAGE_PATTERN regex anchors on that correctly, and the alternation ordering in RESPONSE_CLASSES doesn't cause any mismatch risk (anchored literal match, not substring search).
• RESPONSE_CLASSES correctly enumerates all Net::HTTPServerError subclasses (500-511) plus the generic Net::HTTPServerError fallback for unregistered 5xx codes (e.g. 509), plus TooManyRequests (429) — matches the PR description.
• Load-order check: TRANSIENT_FAILURE_MATCHERS references OpenSSL::SSL::SSLError and Timeout::Error at top-level file-load time (not deferred like the old Timeout.timeout call in config.around). Both are safe here because require_relative "../lib/cpflow" (before this block) requires net/http, which transitively requires net/protocol (requires timeout) and lazily loads openssl — so no NameError risk at load time.
• The tiering condition (CommandHelpers.cpln_org_configured? || example.metadata[:slow]) reuses an existing, cheap (env-check only) helper — no meaningful per-example overhead.

Minor / nit

• Left one inline comment: TransientServerHTTPError and TRANSIENT_FAILURE_MATCHERS are new top-level (global) constants, which is a first for this file — everything else lives inside RSpec.configure or spec/support/. Purely a namespace-hygiene suggestion, not a bug.

Risks already called out by the author (agree, non-blocking)

• Enabling random ordering is a first-ever shuffle for 541 live examples; the PR body documents extensive offline validation across multiple seeds, and the printed seed makes any live-CI failure reproducible. Worth keeping an eye on the first live run.
• Live failures that surface as assertion failures (e.g. via run_cpflow_command translating API errors into exit-status checks) won't retry under exception-class scoping — flagged in the PR's own decision log as a live-flake-rate watch item.

Security
No concerns — this is dev/test-only code (spec/ is excluded from the packaged gem), doesn't touch production request paths, and the new regex operates only on internally-generated exception messages (no untrusted/user-controlled input).

Overall: solid, well-reasoned change with good test coverage of the retry-scoping behavior and thorough self-review already baked into the PR description. No blocking issues found.