Commit dd5fefd
Close out Review 16 and its fix session [skip changelog]
Records Report 16 (Reviewed at: 1b4445f, base 707a3a1, 43 commits) and findings
#123-#132, with the fix commit b8448cb recorded at commit time - the second
review running that this line did not have to be reconstructed later.
Coverage Matrix: 19 rows re-dated after the mechanical staleness pass, and two
row labels corrected where their counts had drifted (Admin JS 9 -> 10 non-test
files, Frontend JS 19 -> 20 bundles, each with its counting rule written beside
it now). All nine whole-repo sweeps re-run and re-derived rather than carried
forward - the Frontend JS one deliberately re-derived AFTER the fix session's
code was staged, because writing it mid-session is what #116 named.
Patterns:
- New PA-16, the quiet sibling of PA-15. PA-15 asks what the plugin switches
off for everybody else; this asks what it moves beyond everybody else's
reach. A wp_enqueue_script handle is a site's whole grip on a request -
wp_dequeue_script, script_loader_tag, dependency order, and the vendor's
domain appearing in the served HTML where a scanner can see it. Fetch the
same file from JavaScript and all of it disappears, silently. Written with
the gate as its reference implementation and with the four things that decide
whether such a marker works, including the one found while building it: a
bare "is it allowed" check cannot tell a blocked gate from no gate at all.
- PA-11 extended to the enqueue side. Its cheap-precondition rule was written
about a REST callback and read as a REST rule, so a second delivery channel
reproduced the same cost through wp_enqueue_script. The method note is worth
more than the rule: the finding was raised from the pattern, corroborated
against WooCommerce's own 7.8 guidance and two comparable plugins, and then
settled by a docblock four hundred lines away in the same class. When a new
channel duplicates an old one, read the old one's comments first.
- PA-2 gained "the copy can be more correct than the original". The third copy
of the identifier predicate carried an anchor the shared one lacked, so a
behaviour diff would have argued for keeping it. A duplicate is a finding on
sight, and its reasoning moves onto the survivor rather than going with the
code.
- PA-9 gained the destroy-then-rebuild rule: a tracker that replaces the embed
before calling a vendor owes a restore on every failure path, and a promise's
catch cannot see a synchronous throw. Plus blast-radius containment, since
one callback's exception was abandoning every remaining embed and provider.
- RI-4 gained a fourth ledger-failure shape, and it is the one "re-derive,
don't read" cannot catch, because re-deriving is what produced it: the same
count kept in two of our own files with only one corrected. FP-1's caller
ledger said 16 where the tree has 12 - it counted grep hits and presented
them as call sites, named two classes that only mention the sink in comments,
and omitted a real caller - because #115 fixed that figure in the sweep row
and left this copy alone. Corrected, with its counting rule beside it.
FP-5 re-derived under the oldest-first rule: the conclusion holds and all three
of its conditions are now demonstrable in code rather than asserted, including
the one #78 closed. The re-derivation produced #130. Next review re-derives
FP-2.
Two open toolchain findings had their counted surface widened inside the range
with neither ledger touched, so both were corrected: #81 (five dispatchers now,
not two) and #118 (three SessionStart hooks, not two).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>1 parent b8448cb commit dd5fefd
2 files changed
Lines changed: 232 additions & 32 deletions
0 commit comments