Repository navigation
Phase 6b: redirect — documentation and phase record - #80
Merged
Merged
Conversation
Phase 6b, Task 2. Widen phase 6a's Dexpace::Resilience::Resend in place with the body-only predicate REDIR-6 asks -- no body, or a body answering #replayable? -- beside .eligible?, which folds in RETRY-7's idempotency clause and would refuse a body-less POST 307 the allowed-method set admits. The clear error a re-send site raises over a present, non-replayable body is Dexpace::NotReplayableError, flat under Dexpace:: on 6a's P6-56 precedent for the same namespace (RetryPredicateError), not under Resilience::. Neither file is required by the entry file yet; the step's commit wires the whole phase-6b block in dependency order.
Phase 6b, Tasks 3-6 and the helpers Task 12 split out. Under lib/dexpace/redirect/: Origin (private) -- REDIR-8's [scheme, host, port] triple under the no-argument fold, compared against the SEED; Location (private) -- one URI::RFC3986_PARSER.join against the current hop, a screen for an http/https scheme AND a host (join resolves mailto:, ftp:, http:foo and http:///p without raising), the userinfo strip spelled userinfo = "" and never = nil, then the freeze, raising URI::InvalidURIError by class for the step to convert; ConditionSnapshot -- REDIR-20's read-only, defensively copied Data in the phase-1 shape; Events and Keys -- REDIR-28's five names and four keys under one http.redirect. prefix; SchemeDowngradeError -- REDIR-15's refusal, naming the two authorities and never a path or query; Chain (private) -- one call's seed, count, visited set and current request; Emitter (private) -- the four contained, redacted emissions and the one raw exception, over the logger's redactor; Reissue (private) -- REDIR-15's downgrade check, REDIR-7's unconditional Authorization strip, REDIR-9/10's cross-origin Cookie strip, REDIR-5's 303 rebuild by Content-* prefix and REDIR-6's replayable-body gate. Every file has a sig/ mirror; the private ones carry hooks.rbs's comment.
Phase 6b, Tasks 8-13. One iterative follower at Stages::REDIRECT that forks a
fresh cursor for EVERY drive, the first included, with the cross-origin
marker as cursor state -- { cross_origin: bool } on every hop, false on the
seed's own and a same-origin one, judged against the SEED origin -- and never
a request header. Per hop: the fast path on a non-redirect status, the
Location resolved once, the snapshot always allocated and a predicate always
consulted on a recognized 3xx, REDIR-17's cap applied over that answer, the
follow-up built inside a frame that closes the current response before any
raise, the superseded response closed BEFORE the next fork, the hop recorded.
Built through .build with .new private, frozen, with logger: as its own
keyword (the Cursor's bundle carries no logger) and no redactor: (one policy
per logging path, the logger's). Wires the ten-file phase-6b block into the
entry file after 6c's, adds REDIRECT_LAYER to the smoke suite's layer table,
and repairs the one existing test the private constructor invalidates: 6c's
guarded convergence test called Redirect::Step.new, and its defined? guard
stops skipping the moment this class exists, so the test now runs and
passes here.
Phase 6b, Task 13a -- the two constructors phase 4c postponed until the redirect, retry and instrumentation families all existed (PIPE-39). Pipeline.standard(over, redirect: nil, settings:, http_tracer_factory:, logger:, level:, preview_bytes:) installs Redirect::Step, RetryStep and Instrumentation::Step; AsyncPipeline.standard(over, redirect:, ...) installs AsyncRetryStep and Instrumentation::AsyncStep and nothing at REDIRECT, with redirect: a REQUIRED keyword admitting :unsupported alone so PIPE-32's asymmetry is spelled at the call site. Both are written over Builder#install_preset and nothing else; the positional is a transport or a Pipeline::Builder already holding one, which is what makes PIPE-24's empty-pillars rule reachable through the constructor. A nil http_tracer_factory: leaves the retry family's own private default in place. Rewrites 4c's three "postponed to phase 6b" YARD sites and the PIPE-32 paragraph on AsyncPipeline, adds .standard to both signatures, and flips 4c's two refute_respond_to pins, which this commit invalidates.
Phase 6b, Task 14. `bundle exec rake surface:regenerate`, once, and the 28 new rows read against the object model: AsyncPipeline.standard, NotReplayableError, Pipeline.standard, Redirect, ConditionSnapshot with its three readers and .build, Events (five), Keys (four), SchemeDowngradeError, Step with #call, #stage, .build, DEFAULT_ALLOWED_METHODS and DEFAULT_MAX_HOPS, and Resend#replayable_body?. Nothing private appears -- no Origin, Location, Chain, Emitter, Reissue or RECOGNIZED_CODES -- and the step exposes no configuration reader. 1 109 rows become 1 137; every one is a widening (NFR-4).
…rgence test Phase 6b, Tasks 1-14. redirect/matrix_facts_test.rb: the design's four Location facts and the ones the build found, a standing test on every CI row (the plan's malformed fixture is a VALID URI; the design's Set-of-URI rationale is false; join resolves host-less and unsupported-scheme targets without raising; URI#to_s elides an explicit default port). redirect/step_test.rb, ten nested classes: construction and REDIR-26, the built-in decision and the predicate route with R9's cap-over-predicate order, the credential hygiene on EVERY intermediate request through an AUTH-position probe, both REDIR-8 chains (A -> A -> B and A -> B -> B, the second pinning the Cookie off the wire), the marker on every drive and 4c's R11 assertions 4 and 5 against the real step, Location resolution against the current hop with the userinfo strip asserted on the rendered URL, the downgrade, the 303 rebuild by prefix, the replayable-body gate, REDIR-22a's ORDER read through a callable transport entry as the follow-up arrives, 5,000 hops flat on the stack, REDIR-28's records with the raw malformed exception, and the fork-for-every-drive contract on a SpyCursor. condition_snapshot_test.rb, events_test.rb, scheme_downgrade_error_test.rb and error/not_replayable_error_test.rb mirror their files; pipeline/standard_test.rb is the two constructors' suite (PIPE-24 through a builder handed in, PIPE-32's required keyword, the keywords reaching the steps, a source scan for a second installation path). Two new top-level doubles: RedirectFixtures and CredentialProbe; 6a's ScriptedTransport is reused as it is. Two earlier-phase tests extended: 6a's resend_test.rb gains a nested ReplayableBodyTest, and 6c's cross_origin_convergence_test loses its now-dead skip guard and says who un-guarded it -- run red first against a scratch REDIRECT stub that forked without the marker.
Review round 0 of the phase-6b stack ran two mutations that survived on 4.0.6 and 3.2.11, each behind a checklist row whose cited test did not prove the clause it claimed. Both are coverage gaps in this layer; no lib/ line changes. R0-1, PIPE-39: dropping `settings:` from the sync `Pipeline.standard`'s `RetryStep.build` left `standard_test.rb` green, because the default settings retry a 503 too -- after a real backoff on Clock::SYSTEM. The wiring case now holds its FakeClock in a local and asserts the one wait ran on that clock at the flat settings' zero delay (`[0.0]`), which the default schedule cannot produce; a second case drives `settings(max_retries: 0)` and asserts the 503 comes back unretried after one call. The `settings` fixture takes `**overrides` for it. `WiringTest` crossed Metrics/ClassLength with the addition, so the two PIPE-24 cases and the install_preset source scan moved to a new `InstallationTest`; every case is unchanged. R0-2, REDIR-9: keeping Cookie and Proxy-Authorization on a CROSS-ORIGIN 303 GET rebuild left the redirect suites green -- every 303 case was same-origin and carried neither header. `ReissueTest` gains one case over an `ORIGIN_SCOPED_HEADERS` POST under `follow303: true`: a 303 to a foreign origin rebuilds a GET carrying none of Cookie, Proxy-Authorization, Authorization or Content-Type and keeping Accept; the same 303 to the seed's origin keeps the two origin-scoped headers and still drops Authorization and Content-Type (REDIR-10 on the rebuild). The mutation now fails on `to not include "Cookie"`. Both mutations were re-applied against the new cases and went red on 4.0.6 and 3.2.11; the reviewer's neighbouring mutations (m01, m06, m07, m26, m30, m33, m35, m41) are still caught.
Review round 1 of the phase-6b stack found one mutation surviving on 4.0.6 and 3.2.11: dropping the fragment from the resolved target left every redirect suite green. REDIR-13 names "path, query, and fragment", the REDIR-14 case at LocationTest was titled "a fragment-only ... reference" over a script carrying none, and no test in the phase drove a fragment-carrying Location. LocationTest now drives what it claims: the REDIR-14 chain gains a fragment-only reference, which resolves against the CURRENT hop and keeps that hop's path and query (page=2#only, not page=1#only), and a REDIR-13 case sends a fragment, a percent-encoded fragment, an empty query and an empty fragment, each asserted byte for byte on the URL the transport received. The fragment dropped, the empty query dropped, the empty fragment dropped and a percent-encoded fragment decoded are each red on both interpreters; the fragment dropped fails the REDIR-14 case too, because the fragment-only hop then revisits the current URL and the loop check stops the chain. The addition pushed LocationTest over Metrics/ClassLength (106/100), so REDIR-15's three downgrade cases and REDIR-18's two screen cases move to a RefusedTargetTest -- the targets the step refuses to follow -- a split by concern and not an inline disable, the way the suite's other classes were cut. Every moved case is unchanged; the header lists the eighth concern. No lib/ line changed.
Review round 2 found two MUST clauses with no test: an automatic POST -> GET rewrite on a followed 301 or 302 (the method rewritten, the body dropped, or both, on 301/302 only) and the case fold dropped from the 303 rebuild's Content-* prefix test both left every redirect suite green on 4.0.6 and 3.2.11, although REDIR-3 says "there is deliberately NO automatic POST -> GET rewrite" and REDIR-5 says "case-insensitively". The code was right; the suite had a gap. ReissueTest gains a REDIR-3 case that follows a 301 and a 302 on a POST and on a PUT under an allowed set that admits the method, asserting on the transport's second call the original method token, the SAME body object and the Content-Type still travelling. RebuildTest, new, gains a REDIR-5 / HTTP-13 case whose POST carries content-type, CONTENT-LENGTH and cOnTeNt-Language in the caller's casing (asserted on #names before the hop) and asserts each gone from the rebuilt GET with Accept alone left. The four mutations are red on both interpreters; the suite's neighbouring guards were re-run and still fire. ReissueTest was at 97 code lines, so the five 303 cases and their two helpers moved unchanged into RebuildTest -- a split by concern (the method-preserving re-issue against the rebuild that changes the method), never an inline disable; the step suite is twelve nested classes and its header lists ten concerns. No lib/ line changed.
…e counts Phase 6b, Task 14. The checklist at docs/work/mvp/phase6/phase6b/2026-09-09-phase6b-redirect-checklist.md, written from what was built: twenty-eight own rows (twenty-seven checked, REDIR-27 deferred to first-release.md's entry), the phase-level PIPE-39, PIPE-32, PIPE-24 and convergence-point rows, fifteen cross-reference rows, the matrix facts on every interpreter, fifty guards run red on 4.0.6 and 3.2.11 with the one equivalent mutant named, the audit groups, thirty departures from the plan's text and the findings routed. The design gains its As-built addendum, P6-91 to P6-100, correcting its own ledger paragraph's cap-before-predicate order to R9's. docs/sdk-documentation/ redirect.md is the fourteenth as-built page, every example run on 4.0.6 and 3.2.11; pipelines.md's two "no Pipeline.standard yet" passages are repaired; architecture.md, the two READMEs and docs/README.md point at the page. CLAUDE.md's built-phases paragraph gains the redirect layer and the two standard constructors, its counts move to 184 lib files, eighteen private constants without a test mirror and fourteen checklists, and its constraints list gains four lines. first-release.md's PIPE-32/REDIR-25 entry now says the constructors were built in that shape; the roadmap gains the phase-6b status note and one phase-10 inbound bullet (the default-port residue, by date and content); docs/knowledge/notes/redirect-handling.md gains one Reference entry closing the userinfo note's floor caveat.
Round 0 returned changes_requested with two should-fix findings and two nits, none touching lib/. The tests-branch commit below this one closes the two should-fix findings; this commit is the record. The checklist's REDIR-9 row now cites the cross-origin 303 rebuild case and states that each of the requirement's two clauses has its own proof; the PIPE-39 row cites the FakeClock wait and the max_retries: 0 case instead of inferring settings: from a call count the default schedule reproduces, and counts thirteen cases; the PIPE-24 row cites InstallationTest, where the split moved its cases; the REDIR-5 row names its four 303 cases as REDIR-5's now that a fifth exists. Guard 48 is re-described as the tracer half alone, guards 51 and 52 are the round's two survivors run red on 4.0.6 and 3.2.11, and the battery's intro counts fifty-two with fifty-one caught. Departures from the plan gain items 31 and 32 (the two added cases; the three-class split). The NFR-13 row miscounted the new test files: nine were added under test/ (seven suites and two doubles), not ten -- R0-4. The design's As-built addendum gains a review-round-1 paragraph in 6a's shape, adding no ledger row: the round found no behaviour the document states that the code fails to honour. The roadmap's 6b note gains its round-1 paragraph, append-only, which also records why the two over-long commit subjects (R0-3) stay as they are: the fix rules forbid amending the implementer's commits.
The checklist's REDIR-13 row now cites the fragment case and states the clause it proves, its REDIR-14 row says the fragment-only reference resolves against the current hop (a claim the row made before the test carried one), and every row and guard whose case moved to RefusedTargetTest cites it: REDIR-9, REDIR-15, REDIR-18, REDIR-22 and guards 15-18, 22 and 23. Guards 53-56 are the round's four, the battery intro reads fifty-six with fifty-five caught on both rows, guard 14's message gains its second case, and departure 33 records the addition and the split. Review round 1's nit: departure 32 said step_test.rb's header names "seven classes" where the suite had ten, and reads eleven now. Two more stale counts found beside it are corrected in the same pass -- the Minitest audit row and departure 12 still said the constructors' suite was two nested classes, and it has been three since round 1's split. The design's As-built addendum gains a review-round-2 paragraph (no ledger row: the code honoured every stated behaviour and only the suite had the gap), the roadmap's 6b note its append-only round-2 paragraph, and redirect.md's REDIR-13 sentence names the fragment beside the path and the query, which the suite now proves.
Round 2 found two coverage gaps behind MUST rows and one stale count. The checklist's REDIR-3 row now cites the case that follows a 301 and a 302 on a POST and a PUT with the method, the same body object and the Content-Type asserted; its REDIR-5 row and the HTTP-13 cross-reference row cite the case whose non-canonical content-* names are removed from the rebuilt GET; guards 57-60 carry the four mutations with their first failure lines, the battery intro reads sixty with fifty-nine caught on both rows, and guards 1, 33, 34, 35 and 52 cite RebuildTest where their case moved, each re-run at the new tip. Departure 34 records the addition and the split; departures 31-33 and the Minitest audit row read the counts as they are now. The design's As-built addendum and the roadmap's 6b note each gain their append-only round-3 paragraph (no ledger row: the code honoured both clauses, the suite had the gap). redirect.md names the case fold beside the prefix strip. docs/README.md says fourteen pages, which is how many it lists.
Wahbeh-Mohammad
added
type:feature
Contributor
Author
Review record for the phase 6b stack (#78 → #79 → #80)4 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 pattern across the four rounds is the point of the record: no round found a code defect — the code branch is the implementer's, unchanged through all four rounds — and every finding was a test gap the reviewer's own mutation battery exposed (a preset keyword unproven, a 303-rebuild clause, the fragment clause, the method-and-body preservation on a followed 301/302, the Round 0 → fixed in round 1
Round 1 → fixed in round 2
Round 2 → fixed in round 3
Round 3 (final) — open, carried into the PR bodies
What the final reviewer verified by experiment, both interpreters
Tips reviewed at the final round: code |
Wahbeh-Mohammad
changed the base branch from
23-phase-6b-redirect-tests
to
main
September 19, 2026 16:12
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 #23. Third and last PR of phase 6b's stack — the documentation and the phase record for the two PRs below it, and the PR that completes phase 6. Targets the tests PR's branch.
What lands
12 files, +1,303 / −53, all Markdown.
docs/work/mvp/phase6/phase6b/2026-09-09-phase6b-redirect-checklist.md, written from the build: 28 own rows — 27 ✅ andREDIR-27⏳ (declined for v1, citingdocs/first-release.md);REDIR-11's three clauses stated in its row ((a) structural, (b) 6c'sAUTH-29, (c) a fortiori) andREDIR-25no longer vacuous now thatAsyncPipeline.standardexists — plus the phase-level rowsPIPE-39✅ (phase 4c's ⏳ row closes here; 4c's own row stays as its record),PIPE-32✅,PIPE-24by construction, the convergence-point-1 row citing 6c's test file, and 15 cross-reference rows; the matrix facts on every interpreter; fifty guards run red; 36 deviations from the plan; the findings routed; postponed work: none..build/no-redactor:shape, the flat error, the scheme-and-host screen, the default-port residue, the downgrade emission, the status key, the raising predicate, the standard constructors' shape. 6a's and 6c's rows are cited as "6a's P6-n" / "6c's P6-n"; phase 10 consolidates all three ledgers into design §10.docs/sdk-documentation/redirect.md— new as-built page, every example run on 4.0.6 and 3.2.11, no credential in any printed result;pipelines.md's two "Pipeline.standarddoes not exist yet" sentences repaired with a worked preset example;architecture.md, the core README,README.mdanddocs/README.md("the fourteen pages written so far") point at it.docs/first-release.md— thePIPE-32/REDIR-25behavioural-asymmetries entry moved from the future tense to "built by Task 13a on 2026-09-19 in that shape"; theREDIR-27entry cited, not rewritten.docs/knowledge/notes/redirect-handling.md— a new entry closing the userinfo-no-op note's "floor-straddling and not yet closed" caveat with the four-row re-measurement (never an in-place edit).CLAUDE.md— "Phases 0, 1, 2, 3a, 3b, 4a, 4b, 4c, 5a, 5b, 5c, 6a, 6b and 6c are built — the whole of phase 6", the opening paragraph gains the redirect layer and the two standard constructors, 174 → 184lib/files besideversion.rb, fifteenprivate_constanttest-mirror exceptions (redirect/origin.rbandredirect/location.rbjoin), fourteen checklists, and the "Constraints that will bite" lines (the marker is cursor state;userinfo = ""never= nil;URI::RFC3986_PARSER.join; the seed origin; one preset path).REDIR-25stopped being vacuous, theCursorwidening was consumed not at all, and phase 6 is complete; one phase-10 inbound bullet (the REDIR-13 default-port residue), cited by date.Verification
bundle exec rakeon Ruby 4.0.6 at this tip: exit 0, all seventeen gates green (2,705 / 67,656, 0 skips, 99.98%, YARD 0 undocumented); honest RuboCop 503 files clean.ruby .claude/skills/housekeeping/probe.rb: no drift, all eight checks;verify_knowledge_structure.rbOK (2,166 harvested, 56 notes).docs/product-spec/,docs/sdk-design-ruby/,docs/knowledge/harvested/,docs/deviations.md, every earlier phase's documents, the phase-6 charter, and 6a's and 6c's documents.CLAUDE.mdand the status note state against the tree and ran everyredirect.mdandpipelines.mdexample on both interpreters.Known follow-ups (not blocking)