## Problem Three defects in the verifier and the cron that drives it, found in a full audit of the module. ### 1. Editing an acknowledgment skips both guards the add path enforces `acknowledgeChainReset()` takes the per-chain write lock and refuses a range that bisects a segment, with a long comment (and a pinning test) explaining that a bisecting ack later makes `hasValidAnchors()` fail silently once the segment is archived: the ack stops covering the break it was recorded for. `updateAcknowledgment()` — reached from the acknowledgment edit form, route `audit_trail.acknowledgment_edit` — ran `assertValid()` and `assertWithinHead()` and nothing else. No lock, no bisection scan. An operator could record a legal range and then move it, from the edit form, to exactly the range the add form refuses. The guard is now a method both paths call. ### 2. The transient-column check was one SELECT and one secret resolve per row `verifyTransientColumn()` -> `findAttestedSegment()` ran an uncached `audit_trail_segment` SELECT, then resolved a secret and recomputed an HMAC, for EVERY row whose `context_transient` is NULL with a non-empty hash — that is, every row of every transient-purged or imported range. The answer is per SEGMENT, and there are a handful of those per chain. `isSegmentCompactionSuperseded()` had the same shape: it rescanned and re-decoded every `segment_compacted` event on the chain per missing segment reference, while `SegmentLifecycleAction::COMPACTED`'s docblock already described that scan as cached per walk. The module's own standard says this is a defect: `checkRow()` takes the predecessor hash as an argument precisely because "a method that looked it up itself would be one query per row", and threads a secret cache for the same reason. The shape it lands in matters, because a first attempt traded one regression for another. Verifying every segment's signature while building the list would put one HMAC per segment on a walk that may only ever ask about one row, and `evaluateCheckpoint()` does exactly that on every verify. So: one SELECT per chain per walk, narrowed to the columns the two readers actually use and to segments that carry a lifecycle stamp at all, with each candidate's signature verified lazily and cached for the walk. A bare segment can never attest anything, so it never enters the scan. Measured: a 12-row purged range cost **14** reads of the segment table on one walk. After: **3**, and it does not move with the row count. Three more questions on the same walk had the same shape: - **A gap in the live rows is not a purge.** `audit_trail.id` is one sequence shared by every chain, so on any site running two chains the live rows of one are separated by the rows of the other, and the walk reaches the gap branch for nearly every row — asking `audit_trail_segment` each time whether an archive bridged the hole. Measured on a twelve-row chain interleaved with a second one: **11** segment reads, one per row, on the ordinary two-chain shape and on the weekly full walk. Only a live-purged segment whose whole range sits inside the gap can bridge one, so the gap is tested against those ranges first, read from the segment list the walk already holds. Tested against each range rather than against their span: a chain that has been purging old buckets for years has purged ranges at both ends of its id space, and every gap between them falls inside that span. - **An unacknowledged break asked per row.** A row no acknowledgment covers is recorded broken and the walk carries on, so it can report the whole tamper map in one pass; it asked `audit_trail_acknowledgment` for each of those rows, for an answer drawn from a table holding one row per incident an operator has explained. Cron re-walks on a schedule, so an unacknowledged break kept paying it every tick. The chain's acknowledgments are read once instead, and lazily, so an intact chain still does not touch that table. - **A rejected acknowledgment was rejected per row.** An acknowledgment stops covering its range when the chain moves under it, and establishing that costs two single-column reads of `audit_trail`. A rejected candidate is skipped and the next broken row asked again, so the pair was paid per row for an answer that belongs to the acknowledgment. The lifecycle-signature check also had two spellings, a cached wrapper and the raw one, and `findArchiveBridging()` called the raw one. There is one method again, with the cache inside it, so every caller gets it and no call site has to know which name was the cached one. ### 3. Cron stored the full verifier verdict for every chain, on every tick `AuditTrailCronVerifyHooks::cron()` wrote `verifyAll()`'s output to State whole. Two of its fields are unbounded — `archives` carries one entry per bridged segment, `acknowledgments` one per acknowledged range — so a chain that has been archived and purged for years rewrote a steadily larger row every cron run. And nothing read either: the status report reads `ok`, `message`, the first broken range, whether anything was acknowledged, and `checkpoint_forged`. The verifier already bounds exactly this data one layer up, where it caps the message rather than emit "a multi-kilobyte status flash". While fixing this: `walkChain()` returned a shape the class documents as always complete ("consumers don't need `??` defaulting at read sites"), and two of its returns left keys out, with all three call sites carrying the `??` the documentation says is unnecessary. The summary therefore reads those keys without defaulting too; the one key it does default is `checkpoint_forged`, which genuinely belongs to the incremental verdict only. And `verifyChainPublic()` said it "doesn't touch the HMAC" while its acknowledgment branch resolves a secret — the behavior is right (an unverifiable ack is not applied), the docblock did not say so. ## Proposed resolution Give the edit path the same lock and the same guard, answer the two per-row questions once per walk without paying for segments the question never reaches, and store only the fields the status report reads. ## Tests - `ChainArchiverTest::testUpdateAcknowledgmentRefusesBisectingSegment` — the edit path refuses the range the add path refuses, and leaves the stored range alone. Fails without the fix. - `AuditTrailVerifierTest::testWalkingPurgedRangeDoesNotQueryPerRow` — a budget, not an exact count: a 12-row purged range costs fewer segment reads than it has rows, and exactly what a 2-row one costs. Fails without the fix (14, needed fewer than 12). - `AuditTrailVerifierTest::testWalkingInterleavedChainsDoesNotQueryPerRow` — a twelve-row chain interleaved with a second one reads the segment table a constant number of times. Fails without the fix (11, one per row). - `AuditTrailVerifierTest::testInterleavingBetweenTwoPurgedRangesCostsNothing` — the same, with a purged range at each end of the chain's id space, which is the shape a gate built on the span between the first and last purged id would let straight through. - `AuditTrailVerifierTest::testWalkingBrokenRangeAsksForAcknowledgmentsOnce` — a break spanning eleven rows asks once. Every row of the range is tampered and the range is asserted broken end to end before counting, because the walk re-anchors on a broken row's stored hash: one tampered row breaks one row and asks once whatever the code does. - `AuditTrailVerifierTest::testWalkingBrokenRangeRejectsAnAcknowledgmentOnce` — rejecting one acknowledgment over ten broken rows costs one pair of row-hash reads. It also asserts the acknowledgment's own signature still re-derives and its anchors are what no longer hold, so the reads being counted are the anchor reads and not somebody else's. - `AuditTrailVerifierTest::testCleanWalkDoesNotReadAcknowledgments` — an intact chain does not query that table at all, which is why the read is lazy rather than made alongside the segment list. - `CronVerifyTest::testCronStoresOnlyWhatTheStatusReportReads` — State carries the reported fields and neither unbounded list, on a chain that has both an acknowledgment the walk used and a bridged archive, so both lists would be non-empty in the shape this replaces. --- AI-Generated: Yes (Claude Code was used to audit the module, draft this issue summary, and write the code and tests on the merge request. Each new test was verified to fail without its fix and to pass with it, and phpcs, PHPStan, cspell and the module's kernel suite were run before pushing.)