The fromAdmin Early Return That Left Production Caching On

Four reviewers ran in parallel over a finished diff, and one of them flagged a preview render that would be written into the production page cache and served to real visitors. The ticket had passed staging, and the last step before release was our read of the diff. Staging could never have shown us this.

The ticket was a page-layout templates feature from a teammate. It adds a preview mode driven by a URL param, websiteTemplateID, and a bulk-apply path that pushes a template across a site’s files.

The cache bug

Module pages come out of a DB-backed render cache. A CacheEnable method on the cache class holds a bail list of query params that mean “this is a preview, don’t read or write cache”: dev, convPrev, templateID, emptyText, backup, stylePreview, previewArea, previewMobile.

The teammate had added websiteTemplateID to the page’s fromAdmin check, which looks right. It reads like the place where admin-only modes get registered. But fromAdmin triggers an early return that skips the CacheDisable() call further down. A param handled only there leaves caching fully on.

That gives two failures:

1. Poisoning. The preview render is written under the site’s normal cache key and served to every anonymous visitor until the entry rolls.
2. Wrong preview. The cache read happens first, so on a warm cache the preview iframe shows the live site instead of the template.

Neither reproduces off prod. The cache is gated on the production domain, so no clone or staging box takes that path. The ticket had every reason to look fine, and it did.

The fix is one line in the bail list. We wrote it up as a reference note so the next preview mode gets added there on day one.

The rest of what came back

The bulk-apply path had a cluster of file-handling bugs. One copies the wrong site’s images. One deletes customer logos with no backup. Those two are the ones we would not have wanted to discover from a support ticket.

We held back the bundle of fixes. We put six suggested fixes on a separate review branch so the teammate can cherry-pick them one at a time and reject any they disagree with. Then we handed the ticket back with numbered findings and four design questions. Those were the calls where guessing at the intent would have been worse than leaving the code alone, like ownership models and what a destructive action should be allowed to do.

The wrong turn that shaped how we ran it

We did not start this day trusting parallel reviewers. Earlier, walking a migrated backlog, we fanned the review out to sub-sessions and took their verdicts. An adversarial audit of those verdicts refuted 51 of 55. Two of the three findings the migration had called source-verified were wrong. Four tickets carried unmerged code where we had been told two.

We had already repeated some of those conclusions to our partner, so we had to walk them back. It also cost us the habit of treating a confident verdict as a fact. Agents that were never made to show the line of code they were reading will happily produce a plausible sentence.

So the review here ran under a different rule: every finding had to point at a file and a line. We re-read each claim against the code before it went into the handoff. The cache finding survived because the early return sits above CacheDisable() in plain view, and anyone can check that by reading the method. Findings we could not confirm that way did not go into the numbered list.

Making the pattern repeatable

The review was a one-off until we checked what already existed. We had a read-only review brief that runs before work starts, a cross-review skill for getting a second model’s opinion, and a role file for staff escalating customer tickets to a developer. None of them covered a finished branch coming from another developer.

The tracker gave us nothing either. There is a Dev-Spec option value, 10536, labelled Review, that sits in the option table and appears nowhere in code or docs. Nothing says when to set it or what flips it back. The only transition that comes close is a generic reopen from Posted to Staging back to Coding. So the handoff ran on convention and had no defined state.

We built a skill for the pattern with the following shape:

• Input: a finished branch or diff, not a fresh ticket. That separates it from the pre-work triage brief, which we reused only for its output format.
• Decision per finding: fix it ourselves, send it back, or approve. The skill forces one of the three.
• Fixes: committed locally on a separate review branch, never on the teammate’s branch.
• Return trip: numbered findings plus the open design questions.

We also saved notes for the pattern and for the preview-cache rule, since the cache rule is the kind of thing that only bites in production.

The skill leaves the question of who signs off alone. Work from a teammate still goes past us before it reaches anyone else, and the review output goes back to the teammate only after we have confirmed it. That rule was already in place, and the skill has to respect it.

What we check first now

Any new preview mode gets grepped against the cache bail list before anything else. The tests and the staging box will both tell you it works, and they will both be wrong.

If you run a page cache, find where your preview and admin-only modes are registered, and check that each one is also excluded from the production cache. In this case a param added to the wrong check left caching on, and neither tests nor staging could show it. A read of the finished diff, with each finding tied to a file and a line, is what caught it.

dxdev LESSON · paste into your AI coding agent
LESSON: Register Every Preview Parameter In The Production Cache Bail List
SOURCE: dxdev.com/blog/parallel-code-review-dev-handoff
WHAT HAPPENED: Four parallel reviewers examined a finished diff for a template preview and bulk-apply feature. One found that `websiteTemplateID` caused an early return before `CacheDisable()` and was absent from the cache bail list. Preview content could therefore be stored under the normal site cache key, while a warm cache could make the preview iframe show the live site. The reviewer confirmed every finding against a file and line before handing the ticket back, then placed six proposed fixes on a separate review branch for selective cherry-picking. The author turned that approach into a review skill for finished branches, requiring each finding to be fixed, returned, or approved.
THE RULE: When a feature introduces a preview query parameter, audit the production cache bypass logic directly, not only the feature's admin-mode checks or staging behavior. Accept review findings only after they can be traced to the relevant code location and independently confirmed.
CHECK MY CODE, then report PASS or FAIL with file:line for each:
      1. Every query parameter that activates a preview mode appears in the render cache bail list or otherwise prevents both cache reads and writes.
      2. No early return for an admin or preview condition can execute before the code that disables caching for that request.
      3. Each review finding in a handoff identifies a file and line that the reviewer has re-read against the finished diff.
THEN PRINT: a table (check, PASS/FAIL, evidence, fix) + a verdict (applies / partially / OUT_OF_SCOPE / no) + the single most important next action.