Repository navigation
Phase 6a: retry — documentation and phase record - #74
Merged
Merged
Conversation
Add Dexpace::Resilience -- Policy (the two-axis classifier consult, the
backoff calculator, the total pacing-header parser over the private
PacingParsers, the retry-count resolver, the recovery-only budget and
RETRY-12's constants), Resend.eligible? (the re-sendability gate over
phase 1's idempotent set and phase 3b's replayability), and the frozen
RetrySettings both retry stacks build from, the first reader of 5a's
Keys::MAX_RETRY_ATTEMPTS -- plus the flat Dexpace::RetryPredicateError.
Widen two earlier-phase files in place: ProtocolError bakes XCUT-5's
retryable_by_status? from 5a's Retryability at construction (phase 4b's
postponement; deliberately not #retryable?, P6-10), and HTTPDate's day
group becomes (\d{1,2}) so RETRY-15's single-digit day parses through the
one RFC 1123 parser (R1); 5a's rejection loop swaps that element for the
bare-date row CFG-31 still refuses.
One calculator, two budget policies: backoff_delay takes no total-timeout
and budget_remaining is a separate function only the recovery engine
names (R6, P6-5). A zero initial delay answers 0.0 before the power is
taken, because 0.0 * Infinity is NaN and [NaN, 8.0].min raises (P6-53).
Widen Pipeline::Cursor with one read-only member, #bundle -- CTX-14's per-call correlation bundle -- seeded by a new optional `bundle:` keyword on Pipeline#call and AsyncPipeline#call (Bundle::NONE when omitted), validated in Cursor.build, threaded through both private drivers' #advance and carried across #fork exactly as #options is. No existing signature moves, and Transport.conforms? still accepts both runtimes because the keyword is optional beside the SPI's three positionals (P6-51). The one consumer is 5b's instrumentation step, on both runtimes: #open_span now takes the cursor's bundle and prefers its tracer factory when the bundle is not NONE, else the step's own keyword, and Tracing.correlate is handed the same bundle, so a populated one pushes its trace and span ids onto the diagnostic context. That is the reconciled precedence's first clause, which phase 5 recorded as having no implementation path; a call that seeds nothing resolves exactly as phase 5 did. The bundle carries no operation name, so the span is still named by the method token (P6-52), and it is not the retry step's HTTP-tracer source (P6-7). Existing tests only widened where the code required it: the cursor's public-method pin gains #bundle, and the two step suites' hand-rolled cursor stand-ins answer #bundle with NONE.
Add Resilience::RetryStep and Resilience::AsyncRetryStep at Stages::RETRY, over the private RetryStepHelpers mixin that holds the decision, the delay resolution, the two override hooks and the trail handling both share. Each drives the downstream chain once per attempt through a fresh Cursor#fork -- the first drive included -- so hop 1 and hop n are one object and each attempt re-executes the chain with fresh per-attempt state (RETRY-44); neither writes cursor state of its own. The synchronous step waits through 5a's cancellable Clock#sleep and checks the token at the top of every attempt. The async step is a trampoline, not a recursion: one Pump per call resumes under a re-arm flag held by a Thread::Mutex across the flip only, so an inline settlement re-enters #resume without growing the stack (the design's recursive sketch overflows at about 1,500 attempts on every supported Ruby); a positive delay goes through Dexpace::Async.delay, which without a Fiber.scheduler raises SeamError synchronously and fails the future with the trail attached (R2's third route), and a zero-length delay continues inline. A settled fatal answering #retryable? is delivered unclassified rather than retried (P6-55). OBS-29's per-attempt group is emitted through the tracer the `http_tracer_factory:` keyword produces once per operation, called with the cursor (R3, P6-7); retries_exhausted fires only when a retryable failure met a spent budget. The interface _HTTPTracer phase 5c declined to declare arrives in the http_tracer sig with the wiring, `context` untyped because the recovery stack hands a Request where the stage stack hands a Cursor. The error-status response is closed before the wait and before any throwable propagates (RETRY-35); no total-timeout is named on this stack (RETRY-28, P6-5).
Add Resilience::RecoveryRetry, chapter 9's first stack (RECOV-17 to RECOV-30 and RECOV-34, which phase 4 postponed), installed as Recovery::Orchestrator's `transport:` decorator rather than as a ResponseChain step (P6-3): Transform#apply carries no request to re-send, and only a transport-decorator position can dispatch its own re-sends while the request chain's stamping stays once per exchange. Sitting below the orchestrator's rescue, a raising transport is retried here as an exception (P6-8), each attempt's response is classified through phase 4b's own Recovery.buffer_error_body then ProtocolError.for_or_nil -- the buffering is what frees the connection before the wait -- and the two terminal shapes are a returned response when the failure was never retryable and a raised throwable otherwise, with every prior failure attached as suppressed (P6-9). This is the only stack with a total-timeout: the budget is a maximum-attempts cap counting the first send as attempt 1 (max_retries plus one, an identity, P6-6) and the time remaining through Policy.budget_remaining, so a delay that would overshoot is suppressed and the last failure surfaced. The HTTP tracer is produced once per operation with the Request, there being no cursor on this stack. The entry file now requires the whole retry layer, the entry test pins the six Resilience constants and the flat error with the private helpers unreachable, and the runtime surface manifest gains the forty-eight rows the phase's public surface adds.
Review round 0 of the phase-6a stack found four behaviours the design states and the code did not fully honour, and one comment nit. The pacing parsers converted an unbounded digit run: a 10 MB header value stalled the retry decision for seconds in String#to_f / #to_i and, past ~309 digits, emitted Ruby's out-of-range warning that the suite's NFR-6 raiser turned into an error Policy#parse_form's rescue swallowed. Both grammars now bound their runs at fifteen digits -- the largest run a Float carries exactly, and 10**15 seconds is thirty million years, so a longer run is RETRY-16's out-of-range value and answers nil without a conversion -- and a 64-byte ceiling in front of every parser keeps the HTTP-date attempt from reading a hostile value either (the longest well-formed form is the 29-byte RFC 1123 date). RetryStep#wait emitted the tracer's attempt_failed outside the RETRY-35 fence, so a tracer that raised left the superseded response open on the sync driver while the async driver closed it. The emission now runs inside the same fence as the delay resolution, so the response is closed before the raise propagates on both drivers. A caller's should_retry answering true retried a downstream CancelledError. Policy.cancellation? walks the cause chain for a CancelledError; Policy.retryable? answers false for one before either branch, and the stage drivers' decision answers :stop for one before the re-sendability gate and before the predicate, so RETRY-23's "never" holds on all three drivers whatever a capability or a predicate says. A negative configured MAX_RETRY_ATTEMPTS raised at RetrySettings.build, so RETRY-41's clamp-and-log was unreachable from any driver. RetrySettings.build takes a logger: keyword, read once at build, and resolves the configured value through Policy.effective_max_retries, so a negative configured value is clamped to the default and the clamp logged as one contained config diagnostic; an explicit negative max_retries: is still refused by RECOV-34. Also: step.rb's class comment cited P6-52 for the no-operation-name statement, which is P6-51's; the RBS mirrors follow each change and the core manifest gains Policy#cancellation? (1004 -> 1005 rows).
Review round 1 of the phase-6a stack found the sync/async drift round 0 closed for attempt_failed still open on the terminal path: RetryStep's settle emitted the tracer's retries_exhausted outside any fence, so a tracer that raised there propagated while the terminal error-status response it had just been handed stayed open, whereas the async pump's finish already ran inside its guarded block and closed it. The trail attachment and the retries_exhausted emission now run inside the same fence as the decision and the delay resolution, so a throwing tracer propagates (OBS-30) with the terminal response closed first on both drivers; a tracer that does not raise leaves the returned response open exactly as before (RETRY-34), and the exception path is untouched. The class comment and the method's comment state the terminal path too.
Add the three doubles phase 6a's retry suites fold over, all under test/support/ and top level like every double since phase 2: ScriptedTransport, which consumes a per-call script of responses, Exceptions and callables so a suite can stage 503, 503, 200 or a run of throwables and assert with assert_same that one request object was re-sent every time; ScriptedAsyncTransport, its SEAM-16 twin over a fresh Completer per call, settling inline by default (the shape that overflowed the plan's recursive pump) or held pending under settle_later: with settle_next! for the cancel-in-flight cases; and the RetryFixtures mixin over RecoveryFixtures -- a request per method, a response over a real ResponseBody so body.closed? is the RETRY-35 assertion, RetryableError and UnretryableError answering XCUT-6's capability, RetryableFatal for RETRY-25, jitter-free settings over a FakeClock, and the RecordingCursor that forwards to the real cursor the driver minted so the fork-for-every-drive rule is asserted against the real runtime. Phase 2's FakeTransport is untouched: seven suites and three doubles require it by name and shape.
Add policy_test.rb (the two-axis classifier consult, the backoff calculator including the 2,000-attempt run that found the NaN at attempt 1,025, the pacing-header parse in every form RETRY-15 names, the retry-count resolver's clamp and its logged diagnostic, and the recovery-only budget), resend_test.rb (the re-sendability gate over phase 1's idempotent set and 3b's replayability, with the streaming and chunked bodies that can never be re-sent) and retry_settings_test.rb (the frozen settings, its validators, the UNSET sentinel and the first read of 5a's Keys::MAX_RETRY_ATTEMPTS under a fake config source). Extend two earlier-phase suites exactly as their postponements said: protocol_error_test.rb gains the nested RetryableTest over the baked retryable_by_status? flag, agreeing with Retryability across 400..599 and deliberately not answering #retryable? (P6-10); http_date_test.rb gains RETRY-15's two single-digit-day tests, one for what the widening admits and one for what it still refuses.
Extend five phase-4c and phase-5b suites with the tests of the one widening Task 8 made. cursor_test.rb's BundleTest: Cursor#bundle is NONE by default, the seeded object at every position and across every fork, refused when not a Bundle, and no writer of any name appears. pipeline_test.rb's and async_pipeline_test.rb's BundleSeedingTest: the `bundle:` keyword seeds the cursor for one call, the empty pipeline still dispatches with no cursor, both runtimes remain a Transport by the duck type, and a non-Bundle is refused before any step runs. step_test.rb's and async_step_test.rb's BundlePrecedenceTest: the reconciled precedence's first clause -- the seeded bundle's tracer factory wins over the step's keyword, the keyword over the constant, a call that seeds nothing resolves exactly as phase 5 did, and the seeded bundle's ids reach the diagnostic context through Tracing.correlate.
Add retry_step_test.rb and async_retry_step_test.rb, both driven through a REAL Pipeline or AsyncPipeline over the scripted transports and a RecordingCursor, so the fork-for-every-drive rule is asserted against the runtime that mints the cursor. The synchronous suite covers eligibility (the two axes, the re-sendability gate no predicate can override), the three terminal paths with the trail attached to the surfaced instance, the delay ladder in RETRY-39's order with the override hook's fall-through logged, the predicate's abort as RetryPredicateError, the budget, cancellation at the top of every attempt and inside the wait, the HTTP-tracer group in OBS-29's order with retries_exhausted only after a retryable failure, and the source guards (no Kernel#sleep, no total-timeout named on this stack). The asynchronous suite adds the trampoline: 2,000 inline-settling attempts complete without growing the stack, a positive delay without a Fiber.scheduler fails the future synchronously with the trail attached and a zero-length delay continues inline (R2), a positive delay under 5a's ParkingScheduler parks a fiber and resumes, a cancelled future aborts at the next boundary while an in-flight attempt is allowed to land, and a settled fatal answering #retryable? is delivered unclassified (P6-55).
Add recovery_retry_test.rb: the classification of each attempt through 4b's own buffer-and-classify primitives with the buffered response what every later line sees, the two terminal shapes (a returned response when the failure was never retryable, a raised throwable otherwise, with the trail attached to the constructed ProtocolError so RETRY-34's cause discrimination holds on this stack), the attempts cap counting the first send as attempt 1, the total-timeout applied as time remaining so no wait can overshoot and a hint past the budget suppresses the wait, the cancellation token at every boundary and inside the wait, the HTTP-tracer group produced once per operation with the Request, and the engine installed as Recovery::Orchestrator's transport: with the request chain's stamping applied once per exchange rather than once per attempt. Add budget_equivalence_test.rb, the sub-phase's convergence point: the three drivers built from ONE RetrySettings and driven against an identical failure sequence exhaust after the same number of wire sends, three under the shared defaults -- max-retries plus one is max-attempts, an identity and not a coincidence (P6-6).
Review round 0's two blocking findings and the test halves of its five should-fix findings, on the tests layer. The three driver suites built their default settings off the live configuration slot, so a host MAX_RETRY_ATTEMPTS changed their answers (=0 broke the recovery suite, =-1 all three). Each suite's Fixtures module now installs a FakeConfigSource env seam in setup and resets the slot in teardown, the shape retry_settings_test.rb and budget_equivalence_test.rb already had; the seven suites run identically with the variable set to 0 and to -1. error/retry_predicate_error.rb, a public lib file, had no test mirror; retry_predicate_error_test.rb pins its shape (StandardError, Dexpace:: Error, flat under Dexpace::, the predicate's raise as #cause). For the code fix on the branch below: policy_test.rb's PacingBoundsTest proves a 10 MB value in every form answers nil in under 50 ms on the parser alone, that a 400-digit run emits no Ruby warning (recorded with WarningCapture, since the suite's raiser and Policy#parse_form's fence had hidden it), the fifteen-digit boundary and the 64-byte ceiling; the '9' * 300 clamp case moves to the out-of-range set and the unexplained retry-after-ms exemption is gone. The at-the-cap jitter test asserts samples on BOTH sides of the cap, which the jitter-then-clip order cannot pass (the reviewer's surviving mutation m02). A tracer raising in attempt_failed is asserted to close the superseded response on both stage drivers. A should_retry answering true is asserted not to retry a downstream CancelledError on both stage drivers, a cancellation wrapped by a retryable error is terminal on all three, and Policy.cancellation? has its own cases. A negative configured MAX_RETRY_ATTEMPTS is asserted clamped and logged at RetrySettings.build, contained, with an explicit negative max_retries: still refused; the Policy method pin gains cancellation?.
Review round 2 of the phase-6a stack: the two guards behind round 1's findings, each seen red on 4.0.6 and 3.2.11. A tracer raising in retries_exhausted now closes the terminal error-status response on both stage drivers: the sync case is the one the fenced settle makes true (round 1's tree leaves the body open) and the async case pins Pump#finish's guarded block, which closed it all along. The async suite's tracer tests move into a TracerFencesTest of their own, because TerminalPathsTest and SharedShapeTest were both at the class-length ceiling with the new case. RETRY-31's "never a blocking sleep" clause was stated and not mechanised: a blocking Clock#sleep inserted beside Async.delay in the pump's wait left the whole async suite green, because every async case runs on a FakeClock whose #sleep records and returns and nothing read it. The inline, parked and no-scheduler cases now assert clock.sleeps empty, and a text scan refuses the token sleep in async_retry_step.rb, whose one wait is Async.delay's.
Add the phase-6a checklist, one row per ID in scope: sixty own rows (RETRY-1 to RETRY-45 and the fifteen RECOV IDs phase 4 handed over), RECOV-31, the inherited CFG-35 and twenty cross-reference rows, with the what-was-built summary, the interpreter-matrix facts, the thirty-one guards run red with the one that stays green and why, the audit groups, the departures from the plan's text and the findings routed to their owners. The design gains its As-built addendum, P6-51 to P6-58. Add docs/sdk-documentation/retry.md, the twelfth as-built page: the one policy core, the two stacks and where each sits, the delay ladder, the budget on each stack, the trail, the HTTP-tracer group and what is still emitted by nothing, every example run on 4.0.6 and 3.2.11 and identical on both. architecture.md, docs/README.md, the root README and the core README point at it. CLAUDE.md's built-phases paragraph gains the retry layer, its counts move to one hundred and forty-nine lib/dexpace/ files, thirteen private_constants without a test/ mirror, twelve checklists and twelve pages, and its constraints-that-bite list gains five lines. The roadmap gains its forty-third inbound bullet, the design's recursive async pump that overflows at about 1,500 attempts, and the 2026-09-18 status note. docs/first-release.md and docs/deviations.md change in no line.
The checklist's RETRY-10, RETRY-16, RETRY-18, RETRY-19, RETRY-23, RETRY-35, RETRY-41, RECOV-34 and OBS-20 rows say what round 1 changed and where it is proven; guards 32-37 join the guards-run-red table, each seen red on 4.0.6 and 3.2.11; "Deviations from the plan" item 13 is made true (the three driver suites now carry the configuration seam) and items 25-30 record the round's repairs against the plan's text; the "What was built" counts move to eight Policy functions and a 1005-row manifest and name the error's new test mirror. The design's As-built addendum gains P6-59 (the configured retry count clamped and logged at RetrySettings.build through its logger: keyword), P6-60 (Policy.cancellation? ahead of both classification branches and the caller's predicate) and P6-61 (the pacing parser's fifteen-digit runs and 64-byte ceiling), a round-1 paragraph for the fenced tracer emission, and the P6-2 as-built statement the review asked for: RetrySettings#backoff_arguments and #header_order are public surface the manifest locks. docs/sdk-documentation/retry.md describes the cancellation guard, the bounded parser and the configured clamp with examples run on 4.0.6 and 3.2.11; CLAUDE.md's retry paragraph names .cancellation? and its constraints list gains the round's three structural facts; the roadmap's phase-6a status note gains the round-1 record. The probe is clean.
Round 1 of the stack's review returned one blocking finding, two should-fix and one nit; this is the documentation half of the repairs. The checklist's NFR-13 row had claimed every new .rbs opens with the SPDX header. None does, as no .rbs in the repository does: the header reaches sig/ with phase 10's Task 5 and its gates:spdx_rbs, so the row now claims the twenty new .rb files and points the .rbs half at its owner. Three counts stale since round 1 read true again: the manifest grew by 49 rows to 1005, and the as-built ledger runs to P6-61. The RETRY-35, RETRY-33 and OBS-30 rows state the terminal path's fence beside the retry path's, the RETRY-26 and RETRY-31 rows say how the async driver's "never a blocking sleep" is now asserted rather than stated, guards 38 and 39 join the table with their red messages on both interpreters, and deviations 31-33 itemise the round. The design's As-built addendum gains a round-2 paragraph (no ledger row: the terminal fence deviates from nothing the design states), retry.md's stage-step paragraph and CLAUDE.md's constraints line say the same, and the roadmap's 6a note gains the round-2 record.
Wahbeh-Mohammad
added
type:feature
Contributor
Author
Review record for the phase 6a stack (#72 → #73 → #74)Three independent reviews, each by a fresh agent with no memory of the previous one, each re-running every gate itself on every tip (4.0.6 all seventeen individually at the code tip and the full
Nothing was skipped. The pre-dispatch cross-check of the plan against the tree as built (the async pump that recurses, the missing Round 0 → fixed in round 1
Round 1 → fixed in round 2
Round 2 (final) — approve; open nits carried into the PR bodies
What the final reviewer verified by experiment, both interpreters
Tips reviewed at the final round: code |
This was referenced Sep 18, 2026
Wahbeh-Mohammad
changed the base branch from
22-phase-6a-retry-tests
to
main
September 19, 2026 06:54
This was referenced Sep 19, 2026
Closed
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.
Closes #22. Third and last PR of phase 6a's stack — the documentation and the phase record for the two PRs below it. Targets the tests PR's branch. Its
CLAUDE.md, READMEs,architecture.mdand roadmap text are written for 6a on top ofmain; phase 6c's stack rewrites the same sentences for 6c, and whichever merges second re-derives the counts.What lands
9 files, +1,344 / −43, all Markdown.
docs/work/mvp/phase6/phase6a/2026-09-09-phase6a-retry-checklist.md, written from the build: 62 own rows — 42RETRY✅,RETRY-4owned by phase 8a's Task 2,RETRY-29/RETRY-38/RETRY-43⏳ (declined for v1), the fifteenRECOVrows ✅ each naming its 6a task with itsRETRYtwin as an annotation,RECOV-31⏳ besideRETRY-38,CFG-35✅ (the inherited throwable half) — plus 20 cross-reference rows; the matrix facts re-run on every interpreter; 38 guards run red with their messages; the audit groups; 33 deviations from the plan; the findings routed; postponed work: none.RetrySettings#randomas the::Randomclass, the zero-initial-delay guard, the trampoline (the design's recursive sketch overflows), the async fatal passthrough, the flatRetryPredicateError, the clamp/fallback diagnostics under 5b's events,retries_exhausted's exact trigger,RetrySettings.build'slogger:,Policy.cancellation?, the bounded pacing grammars. The design's P6-1–P6-12 stand; 6c's design rows collide with them by number (recorded by the roadmap on 2026-09-10) and phase 10 consolidates.docs/sdk-documentation/retry.md— new as-built page, 76 example values run on 4.0.6 and 3.2.11;architecture.md, the core README,README.md(whose "nothing emits the HTTP-tracer vocabulary yet" is now false) anddocs/README.mdpoint at it.CLAUDE.md— "Phases 0, 1, 2, 3a, 3b, 4a, 4b, 4c, 5a, 5b, 5c and 6a are built", the opening paragraph gains the retry layer, 140 → 149 lib files besideversion.rb, thirteenprivate_constanttest-mirror exceptions, twelve checklists, and four "Constraints that will bite" lines (one calculator and two budget policies; the baked flag is#retryable_by_status?and never#retryable?; every pillar step forks for every drive; the async driver's delay needs a scheduler and a zero delay completes inline).OBS-29residuals that stay on phase 10's list, and one phase-10 inbound bullet (the design's pump sketch).Pipeline.standardis explicitly not claimed — 6b's Task 13a.docs/first-release.mduntouched: the P6-4 transport-wrapping entry, theRECOV-31/RETRY-38entry and theOBS-29behavioural-asymmetry entry were verified present and cited, not re-filed.docs/deviations.mduntouched (phase 10 flips the rows).Verification
bundle exec rakeon Ruby 4.0.6 at this tip: exit 0, all seventeen gates green (2,285 / 65,255, 99.98%, YARD 0 undocumented).ruby .claude/skills/housekeeping/probe.rb: no drift, all eight checks (--only registers,citationsalso clean);verify_knowledge_structure.rbOK (2,166 harvested entries, 53 notes).docs/product-spec/,docs/sdk-design-ruby/,docs/knowledge/,docs/deviations.md,docs/first-release.md, every earlier phase's documents, the phase-6 charter, and 6b's and 6c's documents.CLAUDE.mdand the status note state against the tree (the probe does not read the spelled-out lib-file count) and ran everyretry.mdexample on both interpreters.Known follow-ups (not blocking)
AsyncRetryStepsketch and itsRETRY-30paragraph stay in the design text (frozen by convention); the correction lives in the As-built addendum (P6-54) and on phase 10's inbound list, and a human applies it.