refactor(http): tail polish — 5 follow-on beads from round-2 audit#1031
Merged
Conversation
added 5 commits
May 14, 2026 15:35
…ction (rf2-sz4n0)
Round-2 audit findings 1.1, 1.2: two thin helpers from the earlier
http-encoding split don't pay their cost.
- failure-map was (assoc tags :kind kind) — 11 call sites in
http-transport. Inlined as map literals; the call sites read more
directly and the helper indirection is gone.
- realise-body was (if (fn? body) (body) body) with a single call
site in run-attempt!. Inlined.
Audit finding 3.3 (default-accept-fn closure-per-response) had already
landed inside run-accept; the docstring is updated to reflect all three
inlines.
No behaviour change. All 136 tests / 385 assertions still pass.
…l (rf2-e5h1b) Round-2 audit finding 5.2: redact-url-query-string is hand-written split/walk parser territory — exactly where coverage matters most. The existing suite covered fragment-after-query, malformed pairs, and empty values for token-named params, but missed: - fragment-only URL with no query string - empty `?` query string with no params - denylisted param with empty value (api_key=) - sensitive? true preserves the fragment alongside all-params redact - fragments containing = and & are not mis-parsed as query syntax Also adds a focused block of tests for the redact-url single-value wrapper that callers in redact-url-in / tag walkers reach for: pin the input-to-output shape so a refactor of the underlying redact-url-query-string can't silently change the reading. Eight new tests, no source changes. Was 53 privacy tests → now 61.
…2-kdwnq) Round-2 audit finding 5.3: managed-abort-handler at http_handlers.cljc silently no-ops when the registry has no entry for the id — the when-let collapses, the handler returns nil, no throw, no reply dispatch. The shape is correct (idempotent abort) but unasserted; a regression here would only surface as flake in apps racing abort-then-cleanup. New test asserts: - dispatch returns nil (no throw) - second abort of the same unknown id is also a no-op (idempotent) - no :on-failure reply fires (registry had nothing to look up) - no :rf.error/* trace event escapes the abort path Test-only addition; no source changes.
…2-kg5nw)
Round-2 audit finding 5.5: Spec 014 §Chain order documents replace-in-
place (re-registering an existing id keeps the slot's position), but
the clear-then-re-register path was unpinned. http_middleware.cljc
clears by id via `(vec (remove ...))` — the slot is removed entirely —
and a subsequent reg appends to the end.
Decision: clear-then-reg lands at the end. Rationale: clear removes
the slot entirely, so the prior position is forgotten; re-registering
is a fresh registration. Hot-reload tools wanting mutate-in-place
should re-reg directly without clearing; tools wanting a fresh end-of-
chain slot use clear-then-reg explicitly. These are two intentionally
different paths.
Changes:
- Spec 014: clarification under §Chain order and frame scope.
- http_middleware.cljc: docstring on reg-http-interceptor adds the
clear-then-reg note alongside the existing replace-in-place note.
- http_interceptors_test.clj: two new tests pin the contract — one
walks the post-clear-then-reg chain and confirms the order both in
the atom and at dispatch time; the second is a regression guard
asserting bare replace-in-place and clear-then-reg do NOT collapse
into one path.
No source-behaviour change — the impl already had the right shape;
this bead documents and pins it.
Round-2 audit finding 5.4: http_managed.cljc gates the
:rf.http/managed-canned-success / :rf.http/managed-canned-failure fx
registrations on `interop/debug-enabled?`. The CLJS elision contract
is already pinned by sentinels in scripts/check-elision.cjs (lines
102-116) which grep the production bundle.
This JVM-side test pins the complementary GATE BEHAVIOUR — flips
debug-enabled? to false, reloads re-frame.http-managed, and asserts:
- production-eligible :rf.http/managed + :rf.http/managed-abort ARE
still registered (dev+prod regardless of the gate)
- the canned-stub fxs are NOT registered (load-bearing for prod-
bundle isolation per Spec 014 §Testing)
- companion test under debug-enabled? true confirms BOTH stubs DO
register (methodology check so the negative assertion isn't vacuous)
A regression that moved the canned stubs outside the gate (or
registered an additional dev-only fx without gating it) would fail
here long before a CLJS bundle build could surface it.
The fixture's `(require 're-frame.http-managed :reload)` plus an
explicit alter-var-root restore in (finally) keeps the test hermetic.
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.
Summary
Round-2 http audit follow-ons (rf2-jklja). 5 beads, batched-by-surface inside
implementation/http/. Hot-file sequencing — one commit per bead.failure-mapandrealise-bodyinto call sites; the audit-flagged third closuredefault-accept-fnhad already landed insiderun-accept. 11 call sites simplified, helper indirection gone. Pure refactor.?-only query, denylisted-param-with-fragment, empty-value with denylisted param, sensitive-true preserves fragment, fragment containing=/&, plus theredact-urlconvenience-wrapper shape. Hand-written split/walk parser territory.:rf.http/managed-aborton unknown id as a silent no-op (no throw, idempotent, no reply, no trace error). Thewhen-letcollapse was correct but unasserted.reg-http-interceptordocstring updated; two tests pin the contract (one walks the post-clear chain, one is a regression guard separating bare replace-in-place from clear-then-reg).interop/debug-enabled?to false viaalter-var-root, reloadsre-frame.http-managed, asserts the canned-stub fxs are absent and the production-eligible fxs are present. Complements the existing CLJS bundle-grep elision check inscripts/check-elision.cjs.LoC delta: 8 files, +349 / -65 (net +284, mostly test additions).
Test plan
cd implementation/http && clojure -M:test— 148 tests / 416 assertions, 0 failures (was 136/385 pre-bead).cd implementation && npm run test:cljs— 1822 tests / 5131 assertions, 0 failures.cd implementation && npm run test:bundle-isolation— PASS (http section: all sentinels OK; consumer allow-list 2 hits as expected).cd implementation && npm run test:elision— PASS (all 30 dev-only sentinels absent in prod bundle, present in control).Exclusions