From b2200b89b51642c4db8b8604d88702da5be21640 Mon Sep 17 00:00:00 2001 From: Ephriam Nagler Date: Sat, 12 Sep 2026 07:02:10 -0400 Subject: [PATCH 1/9] fix(a11y): let an authored [autofocus] win over the container MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backlog item 6. A collection page logged "Autofocus processing was blocked because a document already has a focused element" on every soft navigation, reproduced twice. The message is the browser declining to run autofocus on swapped-in content — the document already has a focused element by then, so it never tries. The attribute is therefore silently dead on every PJAX navigation, and Pusha's focus-to-container arrived afterwards and took the focus the author had asked for. Apply it in the browser's place. Precedence is hash target, then [autofocus] in the incoming content, then the container. The hash stays first: a click on an anchor link is intent expressed later than the markup was written. Verified rather than trusted — [autofocus] on a disabled input, a hidden element or a plain
is a no-op, so focus is checked and falls through to the container when it did not take. Leaving focus on the outgoing page's link would be worse than the default. Also stubs Element.scrollIntoView in the jsdom fixture. jsdom leaves it undefined rather than warning, so calling it is a TypeError, and the hash branch of focusContainer could not be reached by any test until now — the existing hash tests pass because their hash matches no element. Surfaces updated together: README "Accessibility", CLAUDE.md, and PATTERNS.md step 8, which the porting agent reads. No audit rule or init behaviour changes. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8 --- CLAUDE.md | 2 +- README.md | 3 ++- skill/PATTERNS.md | 2 +- src/focus.ts | 21 +++++++++++++++++++++ test/dom.ts | 6 ++++++ test/smoke.test.ts | 47 ++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 78 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index d029f21..c2e4bdf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -410,7 +410,7 @@ Themes that want fine-grained control populate `cartStatefulRoutes` with the rou PJAX nav has real a11y implications. The runtime handles: -- **Focus restoration**: after DOM swap, focus is moved to either the URL hash target (if any) or to `#MainContent` (or whatever element matches `[data-page-container]`). Without this, keyboard users get lost after every nav. +- **Focus restoration**: after DOM swap, focus is moved to the first of these that exists — the URL hash target, an `[autofocus]` element inside the incoming content, `#MainContent` (or whatever matches `[data-page-container]`). Without this, keyboard users get lost after every nav. The `[autofocus]` step exists because the browser will not run its own autofocus processing on swapped-in content (the document already has a focused element, so it declines and logs a warning), which would otherwise make the attribute silently dead on every soft navigation. The hash wins over `[autofocus]`: it is intent expressed later than the markup. - **Screen reader announcement**: an offscreen `aria-live="polite"` region is updated with the new page title after PJAX swap, so screen readers announce the navigation. Without this, the URL silently changes and assistive tech doesn't know. - **Reduced motion**: when `prefers-reduced-motion: reduce` matches, transitions are skipped (instant swap, no `is-transitioning-*` classes). Themes can override per-transition. diff --git a/README.md b/README.md index 8278699..f6ce249 100644 --- a/README.md +++ b/README.md @@ -906,7 +906,8 @@ Sections written to be re-init-safe under page swaps work for the theme editor a Built in, not configurable: -- **Focus restoration** — focus moves to the URL hash target (if any) or to the swap container after every nav. Without this, keyboard users get stuck on the now-removed link. +- **Focus restoration** — after every nav focus moves to the first of these that exists: the URL hash target, an `[autofocus]` element in the incoming content, the swap container. Without this, keyboard users get stuck on the now-removed link. +- **`[autofocus]` keeps working on a soft navigation** — the browser declines to process it, because swapped-in content arrives while the document already has a focused element. Pusha applies it instead. Put `autofocus` on the search field of a search template and it behaves the same whether the page was loaded or navigated to. An `[autofocus]` that cannot take focus (disabled, hidden, not focusable) falls through to the container. - **Screen-reader announcement** — an offscreen `aria-live="polite"` region is updated with the new page title. - **Reduced motion** — when `prefers-reduced-motion: reduce` matches, transitions are skipped (instant swap, no `is-transitioning-*` classes). diff --git a/skill/PATTERNS.md b/skill/PATTERNS.md index 81f5ecc..6b1c759 100644 --- a/skill/PATTERNS.md +++ b/skill/PATTERNS.md @@ -24,7 +24,7 @@ What `data-section-type` and `sectionInits[handle](root)` plug into. On every PJ - Runs `registry.setupGlobal()` once for any newly-registered components. - Walks `newContainer.querySelectorAll('[data-section-type]')` and calls `window.theme.sectionInits[handle](root)` for each match. - Fires `onAfterSwap` → `onAfterInit` hooks and the legacy `pjax:content-swap` event. -8. Fires the analytics bridges — a prefixed `pusha:page_viewed` custom event (the only publish path that reaches Web Pixels), plus `Shopify.analytics.page()` or `ShopifyAnalytics.lib.page()` for admin reporting, never both. Publishing under standard event names is rejected by the platform, so Pusha does not attempt it; see README "Analytics & tracking". Then a11y focus + screen-reader announcement of the new title. +8. Fires the analytics bridges — a prefixed `pusha:page_viewed` custom event (the only publish path that reaches Web Pixels), plus `Shopify.analytics.page()` or `ShopifyAnalytics.lib.page()` for admin reporting, never both. Publishing under standard event names is rejected by the platform, so Pusha does not attempt it; see README "Analytics & tracking". Then a11y focus — the URL hash target, else an `[autofocus]` element in the incoming content, else the container — and a screen-reader announcement of the new title. Leave an authored `autofocus` attribute alone when porting: the browser declines to process it on swapped-in content, and Pusha applies it in the browser's place. 9. For cached navs, revalidates `[data-island]` sections via Shopify's Section Rendering API in the background. What this implies for wrappers: diff --git a/src/focus.ts b/src/focus.ts index 9c774f2..e3a050d 100644 --- a/src/focus.ts +++ b/src/focus.ts @@ -40,6 +40,27 @@ export function focusContainer(container: HTMLElement, hash: string): void { return; } } + + // An `[autofocus]` in the incoming content is the page author naming where + // focus belongs, which is more specific than this module's default — a search + // page wants the query field, not the container. The browser will not do it + // for us: swapped-in content arrives while the document already has a focused + // element, so autofocus processing is declined ("Autofocus processing was + // blocked because a document already has a focused element" — observed on a + // collection page). Honouring it here is the difference between the attribute + // working on a soft navigation and being silently dead. + // + // The hash still wins: the buyer clicked a link to a specific anchor, which + // is intent expressed later than the markup was written. + const autofocus = container.querySelector('[autofocus]'); + if (autofocus) { + autofocus.focus({ preventScroll: true }); + // Verified rather than assumed: [autofocus] on a disabled input, a hidden + // element, or a plain
is a no-op, and leaving focus on the outgoing + // page's link is worse than the default. + if (document.activeElement === autofocus) return; + } + makeFocusable(container); container.focus({ preventScroll: true }); } diff --git a/test/dom.ts b/test/dom.ts index 6e30507..98922ed 100644 --- a/test/dom.ts +++ b/test/dom.ts @@ -99,6 +99,12 @@ export function setupDom(initialTemplate = 'index', initialBody = '

Home

// jsdom doesn't implement scrollTo; stub to avoid Not implemented warnings. w.scrollTo = (() => {}) as typeof w.scrollTo; + // jsdom leaves Element.scrollIntoView undefined — not "not implemented", but + // absent, so calling it is a TypeError rather than a warning. focus.ts calls + // it on the hash target, which is why nothing in the suite could reach the + // hash branch of focusContainer until this stub existed. + w.Element.prototype.scrollIntoView = function scrollIntoView() {}; + // Reset history.scrollRestoration setter — jsdom allows it. (w.history as unknown as { scrollRestoration: ScrollRestoration }).scrollRestoration = 'auto'; diff --git a/test/smoke.test.ts b/test/smoke.test.ts index 9cfee21..6ba1af5 100644 --- a/test/smoke.test.ts +++ b/test/smoke.test.ts @@ -424,6 +424,53 @@ test('customEvents off leaves publish() untouched — Pusha makes no rejected ca ); }); +test('[autofocus] in the incoming content takes focus instead of the container', async () => { + // The browser will not do this for us: swapped-in content arrives while the + // document already has a focused element, so autofocus processing is + // declined. Without this, the attribute is silently dead on every soft + // navigation and a search template loads with focus on the container. + fetchResponder = () => ({ + status: 200, + body: makePageHtml('search', '

Search

'), + }); + + runtime.initRuntime(); + document.querySelector('a[href="/products/foo"]')!.click(); + await new Promise((r) => setTimeout(r, 60)); + + assert.equal(document.activeElement?.id, 'q', 'the authored target has focus'); +}); + +test('a hash target still outranks [autofocus] — the click is the later intent', async () => { + fetchResponder = () => ({ + status: 200, + body: makePageHtml('search', '
R
'), + }); + + document.body.innerHTML += 'Results'; + runtime.initRuntime(); + document.querySelector('#deep')!.click(); + await new Promise((r) => setTimeout(r, 60)); + + assert.equal(document.activeElement?.id, 'results', 'hash wins over autofocus'); +}); + +test('an [autofocus] that cannot take focus falls through to the container', async () => { + // autofocus on a disabled input, a hidden element or a plain
is a + // no-op. Trusting it without checking would leave focus on the outgoing + // page's link, which is worse than the default. + fetchResponder = () => ({ + status: 200, + body: makePageHtml('search', '

Search

'), + }); + + runtime.initRuntime(); + document.querySelector('a[href="/products/foo"]')!.click(); + await new Promise((r) => setTimeout(r, 60)); + + assert.equal(document.activeElement?.id, 'MainContent', 'container took focus'); +}); + test('data-no-transition on link skips PJAX', async () => { const link = document.querySelector('a[href="/products/foo"]')!; link.setAttribute('data-no-transition', ''); From f6c785a277d361dd852dc86bcf96c0f66358c722 Mon Sep 17 00:00:00 2001 From: Ephriam Nagler Date: Sat, 12 Sep 2026 07:07:48 -0400 Subject: [PATCH 2/9] docs: reconcile the audit surfaces with what the CLI actually does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pass over the four surfaces against `bin/pusha.js`, which is the first thing the new consistency rule asks for. Three real drifts, all in the direction the rule predicts — the CLI moved and the prose did not. - Bucket X was missing from CLAUDE.md's audit description, which still stopped at P, while CLAUDE.md discusses X at length five sections later. SKILL.md and the CLI both had it. - Bucket K sat in the README's *surface* group and in SKILL.md's *per-script* group. The CLI settles it: a K finding is one custom element with a list of render sites, not a whole-surface check. The README moves. - `--skip-detect` and `--ignore-manifest` ship in the CLI and were absent from the README, the surface that is supposed to be the contract of record. `--ignore-manifest` was in SKILL.md, so the agent knew about a flag the human-facing docs did not mention. Also documents the `--json`-only narrowing flags together rather than scattered. No behaviour change; no source touched. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8 --- CLAUDE.md | 4 +++- README.md | 12 ++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c2e4bdf..9236e04 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -118,7 +118,9 @@ This is the command that converts a curious reader into a user. It's also where #### `npx @mimeticthemes/pusha audit` -Runs the audit against the current directory (or path argument). Buckets A–H (per-script) plus the surface buckets: J (analytics), K (portal-to-body custom elements), L (per-request Liquid in the shell), M (persistent-shell stateful UI), P (partials). `--json` emits structured output for tools/agents. The CLI is the canonical audit implementation — there is no longer a parallel bash script. +Runs the audit against the current directory (or path argument). Buckets A–H and K (per-script) plus the surface buckets: J (analytics), L (per-request Liquid in the shell), M (persistent-shell stateful UI), P (partials), X (theme app extensions). `--json` emits structured output for tools/agents. The CLI is the canonical audit implementation — there is no longer a parallel bash script. + +The authority on this list is `BUCKET_RULES` in `bin/pusha.js`; `skill/SKILL.md` restates it for the porting agent. All three drifted once already — X shipped in the CLI and the skill while this line still stopped at P. #### `npx @mimeticthemes/pusha init` diff --git a/README.md b/README.md index f6ce249..c270dd0 100644 --- a/README.md +++ b/README.md @@ -157,7 +157,7 @@ npx github:mimetic-themes/pusha init This copies `pusha.min.js` into `assets/`, `pusha.liquid` into `snippets/`, and (with confirmation) inserts `{% render 'pusha' %}` into `layout/theme.liquid` before ``. -Flags: `--dry-run`, `--force`, `--yes` / `-y`. +Flags: `--dry-run`, `--force`, `--yes` / `-y`, `--skip-detect`. ### Path B — bundler @@ -968,18 +968,22 @@ Until Pusha is published, run the CLI straight from this repository with `npx github:mimetic-themes/pusha `. After a Path B install it is on your project's `$PATH` as `pusha`. -`init` flags: `--dry-run`, `--force`, `--yes` / `-y`. +`init` flags: `--dry-run`, `--force`, `--yes` / `-y`, `--skip-detect` (install +even when the directory does not look like a theme). `audit` flags: `--json` for structured output, `--full` to append the whole of `PATTERNS.md` to the report (one self-contained doc an agent can read in a single pass), `--no-whitelist` to disable the false-positive filters and see -every raw finding. Pass a path to audit a different directory. +every raw finding, `--ignore-manifest` to ignore `.pusha/MANIFEST.md` and treat +every finding as unsettled. With `--json`, narrow the report with `--bucket`, +`--action` (`transform` | `decide` | `verify` | `none`) and `--file`. Pass a +path to audit a different directory. `skill` flags: `--print` to dump `SKILL.md` + `PATTERNS.md` to stdout, or `--claude` / `--cursor` / `--aider` to install the skill for that agent. Add `--global` to install into `~/` instead of the project. -The audit classifies every script in the theme by transformation difficulty (buckets A–H), plus the buckets that cover a whole surface rather than a single script: J (analytics — coverage, payload conformance, placement, and raw pixels that bypass Customer Events), K (portal-to-body custom elements needing `data-pusha-cleanup`), L (per-request Liquid frozen in the shell), M (persistent-shell stateful UI), P (`{% partial %}` regions), and X (theme app extensions — which installed app blocks and embeds sit inside the swap container, and which survive it). The `pusha` skill consumes this output to apply the wrappers; agents without the skill can act on the audit's prescriptive "Next steps" block directly. +The audit classifies every script in the theme by transformation difficulty (buckets A–H, plus K for portal-to-body custom elements needing `data-pusha-cleanup`), and then the buckets that cover a whole surface rather than a single script: J (analytics — coverage, payload conformance, placement, and raw pixels that bypass Customer Events), L (per-request Liquid frozen in the shell), M (persistent-shell stateful UI), P (`{% partial %}` regions), and X (theme app extensions — which installed app blocks and embeds sit inside the swap container, and which survive it). The `pusha` skill consumes this output to apply the wrappers; agents without the skill can act on the audit's prescriptive "Next steps" block directly. A `create-pusha` scaffolder for new themes is on the roadmap, once Pusha is published. From abb748556146a57f235da1aa5aea3477f28595b6 Mon Sep 17 00:00:00 2001 From: Ephriam Nagler Date: Sat, 12 Sep 2026 07:09:21 -0400 Subject: [PATCH 3/9] docs: CONTRIBUTING, CHANGELOG and SECURITY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backlog item 5, the part that does not need a decision. CONTRIBUTING carries the four-surfaces rule, because a contributor is the reader most likely to change one surface and not the other three, and two things this repo learned the hard way: check that a regression test fails without its fix, and treat jsdom coverage as not-browser-coverage. Element.scrollIntoView is the example — jsdom leaves it undefined rather than warning, so the hash branch of focus.ts looked covered and was unreachable. CHANGELOG starts at 0.1.0 rather than reconstructing forty commits of pre-publication churn. git log has that, with the reasoning attached, which is the part a rebuilt changelog would throw away. The known limitations are in it on purpose: the pixel gap and the inert-app-code gap are the two things a reader needs before adopting, and burying them would be the kind of omission the honesty pass was for. SECURITY routes through GitHub private reporting. It deliberately names what is NOT a vulnerability — the pixel gap and app code going inert are documented, measured limitations, and a triage queue that has to re-litigate them each time is worse than one that says so up front. There is no security email. If you want one, add it and I will wire it in — I am not inventing an address that would bounce. No source, CLI, skill or runtime-contract change. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8 --- CHANGELOG.md | 66 +++++++++++++++++++++++++++++++++++++++++ CONTRIBUTING.md | 78 +++++++++++++++++++++++++++++++++++++++++++++++++ SECURITY.md | 49 +++++++++++++++++++++++++++++++ 3 files changed, 193 insertions(+) create mode 100644 CHANGELOG.md create mode 100644 CONTRIBUTING.md create mode 100644 SECURITY.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..ecaf0a7 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,66 @@ +# Changelog + +Notable changes per release. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); +versioning follows [SemVer](https://semver.org/), with the 0.x caveat that +breaking changes are allowed in minors until 1.0. + +Everything below is pre-publication. Nothing has been released to npm yet, so +this file starts at the first release rather than reconstructing the churn that +led to it — `git log` has that, and it carries the reasoning. + +## [Unreleased] — 0.1.0 + +First release. The runtime, the CLI, and the porting skill. + +### Runtime + +- PJAX navigation over a single configurable container (`#MainContent` by + default), with link interception, history, scroll restoration, and named + transitions. +- Component registry — `setupGlobal()` once, `init(root)` on every load and + swap, optional `destroy(root)`. +- Hooks: `onBeforeNav`, `onBeforeLeave`, `onAfterSwap`, `onAfterInit`, + `onFirstLoad`, `onNavError`. Async handlers are awaited. +- Events on `document`: `pjax:before-nav`, `pjax:content-swap`, + `pjax:islands-revalidated`, and `cart:mutated` as the cart contract. +- Prefetch with per-template TTLs, soft/hard staleness, hover, pointer-down, + focus and viewport warming, and a concurrency cap of two so speculative work + never starves a real request. +- Islands — `[data-island][data-section-id]` regions revalidated through the + Section Rendering API after a cached navigation. +- Head sync for title, meta, `body[data-template]`, scripts and stylesheets. +- Analytics bridges: `shopify`, `customEvents`, `standardEvents`, `trekkie`, + `ga4`, `dataLayer`. Standard Web Pixel events are **not** reachable on a soft + navigation — this is a platform constraint, measured rather than assumed, and + the README's "Analytics & tracking" section says what does and does not work. +- Standard storefront events — `shopify:page:view` is re-dispatched per + navigation, which is the sanctioned way app code learns a soft navigation + happened. +- Accessibility: focus moves to the hash target, then an `[autofocus]` element + in the incoming content, then the container; an `aria-live` region announces + the new title; `prefers-reduced-motion` skips transitions. +- Theme editor: PJAX is off under `designMode`; `shopify:section:load` and + `shopify:section:unload` are handled. +- `appCompat.sectionEvents` and `appCompat.reexecuteExtensionScripts` — + experimental theme-app-extension repairs, both **off by default**. Enabling + both together compounds multiplicatively and warns at boot. + +### CLI + +- `pusha audit` — classifies every script in a theme into buckets A–H and K, + plus the surface buckets J, L, M, P and X. `--json` makes the report + addressable by an agent, narrowable with `--bucket`, `--action` and `--file`. +- `pusha init` — Path A (vendored `pusha.min.js` + snippet) or Path B (npm), + detected from the theme. +- `pusha skill` — installs the porting skill for Claude Code, Cursor or Aider. + +### Known limitations + +- Pixels wired through Shopify admin (Meta, GA4, TikTok, Klaviyo) do not + receive a pageview on a soft navigation. No supported theme-side fix exists + today. See the README. +- Theme app extension code with no re-init hook goes inert after a swap, + regardless of how it is loaded. Apps that listen for `shopify:page:view` or + that ship custom elements recover for free. +- `` and `` are not synced. Route language switches + through a full browser navigation. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..8407b3e --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,78 @@ +# Contributing + +Pusha is small and opinionated, and it is pre-1.0. The most useful contribution +right now is a theme it breaks on — a real Online Store 2.0 theme, the audit +output, and what went wrong in the browser. + +## Setup + +```sh +npm install +npm test # node's test runner over test/*.test.ts, jsdom fixtures +npm run typecheck +npm run build # vite ESM + UMD, then tsc for the .d.ts files +``` + +Node `^22.13.0 || >=24.0.0`. CI runs both, and it has caught a real difference +between them at least once — if a test passes locally and fails on 22.13, that +is a signal, not noise. + +## The four surfaces must agree + +`pusha init`, `pusha audit`, the skill (`skill/SKILL.md` + `skill/PATTERNS.md`), +and the README describe one contract to four different readers. Three of them +are consumed by an agent or a machine rather than a person, so a stale one does +not read as out of date — it reads as authoritative and wrong. An audit rule +that describes a removed config key teaches the removed key to every agent that +runs it. + +Any change to config keys, defaults, hook or event names and payloads, bucket +rules or their remediation text, install layout, or the `--json` contract +touches all of these: + +1. Source (`src/`) — the behaviour. +2. `bin/pusha.js` — `audit` rules and remediation, `init` writes. +3. `skill/SKILL.md` and `skill/PATTERNS.md` — procedure and patterns. +4. `README.md` — the documented contract, and the tie-breaker when two + surfaces disagree. +5. Tests, including the audit golden files. + +If one genuinely does not apply, say so in the commit message. Silence reads as +an oversight, because usually it is. + +## Tests + +Every bug fix gets a test that fails without the fix. Check that it does — +comment the fix out, watch it go red, put it back. A regression test that never +failed is decoration. + +Assert on the damage, not on the mechanism. A test that asserts "no stray +request reached the wire" keeps working when the internals move; one that +asserts on a private counter breaks for reasons that are not bugs. + +The jsdom fixture in `test/dom.ts` is deliberately thin. jsdom leaves some DOM +APIs undefined rather than warning, so a code path can be silently unreachable +from the suite — `Element.scrollIntoView` hid the whole hash branch of +`focus.ts` until it was stubbed. If a branch looks covered but has never gone +red, check that the API it depends on exists in the fixture. + +## Commits + +Write the reasoning, not the diff. The subject says what changed; the body says +what was true that made it necessary and what you ruled out. `git log` is where +this project keeps its reasoning, so a message that only restates the patch +throws that away. + +## Browser verification + +jsdom coverage is not browser coverage, and the gap is not theoretical: the +abort race, the navigation timeout, and hash-link handling all needed a real +browser to be believed. Anything touching navigation timing, focus, or +transitions should say in the commit message whether it was walked in a browser +or only tested in jsdom. + +## Scope + +Cart is theme code. Analytics is framework code. If you are not sure which side +of that line something falls on, the README's "Cart is theme code" section +explains the test being applied. diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..fa73457 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,49 @@ +# Security + +## Reporting + +Use **GitHub's private vulnerability reporting** on this repository (Security → +Report a vulnerability). Please do not open a public issue for anything +exploitable. + +Include what you did, what happened, and the theme shape it happened on. A +proof of concept helps; a working exploit against someone else's live store +does not — do not test against storefronts you do not control. + +Expect an acknowledgement within a week. Pusha is maintained by a one-person +studio, so that is a realistic figure rather than an aspirational one. + +## Scope + +Pusha runs in the storefront, in the buyer's browser, with whatever privileges +the theme already has. What is in scope: + +- Parsing fetched HTML into the live document — the swap path, head sync, and + Section Rendering API island responses. +- The origin checks on navigation and prefetch. Pusha refuses cross-origin + redirects rather than parsing another origin's markup as if it were the + shop's; a way around that is a vulnerability. +- `syncHeadScripts` / `syncHeadStyles`, which execute script and style elements + from a fetched document. +- The `appCompat.reexecuteExtensionScripts` flag, which re-executes bundles + from `cdn.shopify.com/extensions/`. It is off by default and its known cost + (unbounded listener accumulation) is documented, not a finding — but a way to + make it execute from another origin is. +- `bin/pusha.js`, which writes into a developer's theme directory. Path + traversal or writing outside the target theme is in scope. + +What is not: + +- Theme code Pusha navigates between. A theme that injects untrusted HTML is + vulnerable whether or not it soft-navigates. +- Shopify's own platform, including Web Pixels, the Section Rendering API, and + Standard storefront events. Report those to Shopify. +- The fact that a soft navigation does not re-run a third-party app's + once-per-document script. That is a documented compatibility limitation with + measurements behind it, not a security issue. + +## Supported versions + +Pre-1.0. Fixes land on the current minor; there are no backports. Breaking +changes are allowed in 0.x minors, so pinning an exact version is the right +move for production themes until 1.0. From 8b6552706c2a1232893352aed61677654d60d154 Mon Sep 17 00:00:00 2001 From: Ephriam Nagler Date: Sat, 12 Sep 2026 07:14:34 -0400 Subject: [PATCH 4/9] docs: the bundle numbers were stale, and one of them inverted an argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Caught by rebuilding to refresh the rig's vendored bundle. CLAUDE.md and the README both claimed a measurement from 2026-09-11 and disagreed with each other: 32.2 kB / 10.7 kB against 34.7 kB / 11.5 kB. A clean build today says 35.3 kB raw / 11.7 kB gzipped for the UMD bundle. The second number was doing real work and doing it backwards. CLAUDE.md argued the modularity hold was safe because Pusha was "already under `@barba/core` alone (9.9 kB)". At 11.7 kB it is larger, not smaller — so the sentence that justified not splitting the bundle rested on a comparison that goes the other way. The README had the honest framing on the same day: a comparable feature set is @barba/core + prefetch + css + router at ~12.5 kB, still without an analytics bridge or Section Rendering API revalidation. That argument survives a reader checking it; the core-only one does not. The wrong claim is recorded rather than quietly replaced, since it is the kind that gets re-derived. Both surfaces now carry the same measured figures, and a note that these go stale on every merge. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8 --- CLAUDE.md | 29 +++++++++++++++++++++-------- README.md | 6 +++--- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9236e04..e3a0b1f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -520,14 +520,27 @@ Earlier text here described prefetch as an "opt-in plugin (Barba-style — core small, prefetch is optional)"; that was intent, never implementation, and it is removed rather than restated so nobody designs against it. -**This is a deliberate hold, not an oversight.** Measured 2026-09-11 on a clean -build: 32.2 kB raw / **10.7 kB gzipped** for the UMD bundle — already under -`@barba/core` alone (9.9 kB) before Barba adds the plugins needed to match, and -that is with the analytics bridges, islands and the a11y handling included. -Tree-shaking would save single-digit kB at best, Path A cannot benefit from it at -all (one UMD file by construction), and moving optional subsystems behind dynamic -imports would add a round-trip to the first navigation — the exact latency the -product sells against. +**This is a deliberate hold, not an oversight.** Measured 2026-09-12 on a clean +build: 35.2 kB raw / **11.7 kB gzipped** for the UMD bundle, 27.1 kB / 8.1 kB for +the ESM entry — with the analytics bridges, islands and the a11y handling +included. Tree-shaking would save single-digit kB at best, Path A cannot benefit +from it at all (one UMD file by construction), and moving optional subsystems +behind dynamic imports would add a round-trip to the first navigation — the exact +latency the product sells against. + +⚠ **This paragraph previously claimed 10.7 kB gzipped and "already under +`@barba/core` alone (9.9 kB)".** Both were wrong, and the second inverted the +argument: at 11.7 kB Pusha is *larger* than Barba's core, not smaller. The README +had the honest framing on the same day — comparable feature set means +`@barba/core` + `@barba/prefetch` + `@barba/css` + `@barba/router` at ~12.5 kB, +and that is still without an analytics bridge or Section Rendering API +revalidation. Keep the comparison feature-for-feature; the core-only number +flatters us and does not survive a reader checking it. + +Bundle figures are measured, so they go stale on every merge. Re-measure with +`npm run build` before quoting them, and keep this paragraph and the README's +"Bundle sizes" table in step — they disagreed for a day while both claimed the +same measurement date. **The trigger to revisit is a subsystem growing, not a size threshold.** `app-compat.ts` is ~0.3 kB gzipped today (measured by unwiring it and rebuilding). diff --git a/README.md b/README.md index c270dd0..2d3dfbb 100644 --- a/README.md +++ b/README.md @@ -915,12 +915,12 @@ Built in, not configurable: ## Bundle sizes -Measured from a clean `npm run build` on 2026-09-11: +Measured from a clean `npm run build` on 2026-09-12: | File | Raw | Gzipped | |---|---|---| -| `dist/pusha.min.js` (UMD, prod) | 34.7 kB | **11.5 kB** | -| `dist/pusha.esm.js` (ESM, main entry) | 26.9 kB | 8.1 kB | +| `dist/pusha.min.js` (UMD, prod) | 35.3 kB | **11.7 kB** | +| `dist/pusha.esm.js` (ESM, main entry) | 27.2 kB | 8.2 kB | Grown from 9.1 kB gzipped in the first build — the analytics bridges, the standard cart event bridge, and the app-compatibility flags landed since. The UMD bundle is everything — navigation, prefetch cache, From 66de64a2b121ee8b0e924824b00f7062f4845a2e Mon Sep 17 00:00:00 2001 From: Ephriam Nagler Date: Sat, 12 Sep 2026 07:33:23 -0400 Subject: [PATCH 5/9] fix(islands): clear .is-revalidating on every path, and count what applied MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found in the first browser walk islands have ever had. The run reported `cached: true`, one revalidation event, `swapped 1 in 229ms` — and left `.is-revalidating` on a live element. The class was only ever removed because `wrapper.replaceWith()` destroyed the node carrying it. Every path that does not swap — no `#shopify-section-` wrapper in the document, an empty response, an island marker sitting outside the wrapper being replaced — stranded it on a live element permanently. CLAUDE.md promised "removed when the new HTML is in place", which was true by accident. A theme styling it as a dim or skeleton, which is what the README tells them to do, gets a region that never comes back. Removed explicitly now, for every island, on every path. The second half is why the walk looked clean. `swapped ${keys.length}` counted keys in the response, not nodes replaced, so it printed `swapped 1` for a run that may have touched nothing — the diagnostic that should have caught the stranded class certified the run instead. It now counts what applied, names what did not, and logs NO TARGET for a section id with no wrapper. `pjax:islands-revalidated` had the same flaw: `sectionIds` was what was requested. A listener re-initialising the regions it names was re-running against markup that never changed. It now carries what was applied, with `requestedIds` alongside. Whether the Horizon run hit the no-wrapper case or the outside-the-wrapper case is still unknown — the next run says, which it could not before. Both tests verified to fail without the fix. The pre-existing hash test's fixture has an island with no wrapper and never noticed. Surfaces updated together: README (event table + islands section), CLAUDE.md (visual contract), PATTERNS.md (the id must name a wrapper that exists). No audit rule or init behaviour changes. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8 --- CLAUDE.md | 2 +- README.md | 6 ++-- skill/PATTERNS.md | 9 +++++ src/islands.ts | 41 +++++++++++++++++++--- test/smoke.test.ts | 85 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 136 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e3a0b1f..73df774 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -288,7 +288,7 @@ The Shopify Section Rendering API lets you fetch any section's HTML in isolation - On navigations where prefetchConfig's `soft` TTL has elapsed but `hard` has not — page HTML is reused, islands fetched fresh - Never on uncached navigations — the page was just fetched, everything is current -**Visual contract**: while an island is revalidating, it gets a `.is-revalidating` class that themes can style for a subtle dim/skeleton. The class is removed when the new HTML is in place. +**Visual contract**: while an island is revalidating, it gets a `.is-revalidating` class that themes can style for a subtle dim/skeleton. The class is removed when the revalidation cycle ends — **on every path, including the ones that swap nothing**. It used to come off only because `wrapper.replaceWith()` destroyed the node carrying it, so an island whose `#shopify-section-` wrapper was absent kept the class on a live element forever and a themed dim never lifted. Found in a browser, 2026-09-12; the diagnostic that should have caught it counted keys in the response rather than nodes replaced, so it printed `swapped 1` for a run that swapped nothing. **Exported as `@mimeticthemes/pusha/islands` subpath** for explicit imports. Auto-imported by main entry when prefetch is enabled. diff --git a/README.md b/README.md index 2d3dfbb..7e63d56 100644 --- a/README.md +++ b/README.md @@ -709,7 +709,7 @@ Dispatched on `document`: |---|---|---| | `pjax:before-nav` | `{ url, event }` | Cancelable. `preventDefault()` on the CustomEvent falls through to a full nav. | | `pjax:content-swap` | `{ url, template, cached }` | Fires after `onAfterInit`. | -| `pjax:islands-revalidated` | `{ sectionIds }` | Fires after Section Rendering API revalidation. | +| `pjax:islands-revalidated` | `{ sectionIds, requestedIds }` | Fires after Section Rendering API revalidation. `sectionIds` is what was actually swapped into the page; `requestedIds` is what was asked for. They differ when the server renders a section this document has no `#shopify-section-` wrapper for. | | `cart:mutated` | `{ source, cart?, lastOperation? }` | **The theme dispatches this** after any cart change. Pusha listens and invalidates prefetch — and also dispatches it itself, with `source: 'shopify-standard-events'`, when a standard cart event settles. See [Cart](#cart). | ### Islands (Section Rendering API) @@ -723,7 +723,9 @@ For inventory- and price-sensitive regions inside templates that use long prefet
``` -After a cached navigation, Pusha fetches `?sections=section-id-1,section-id-2`, parses the JSON response, and hot-swaps the section markup. While revalidating, the island gets `.is-revalidating` for a subtle dim/skeleton. +After a cached navigation, Pusha fetches `?sections=section-id-1,section-id-2`, parses the JSON response, and hot-swaps the section markup. While revalidating, the island gets `.is-revalidating` for a subtle dim/skeleton; it is removed when the cycle ends, whether or not anything was swapped. + +The island marker and the `#shopify-section-` wrapper it names have to be in the same document. Shopify returns section HTML keyed by section id, and Pusha replaces `#shopify-section-`, so an island whose `data-section-id` has no matching wrapper is fetched and then has nowhere to go. Run with `debug: true` and the log says `NO TARGET` for exactly that case. ### Cart diff --git a/skill/PATTERNS.md b/skill/PATTERNS.md index 6b1c759..af5bfb7 100644 --- a/skill/PATTERNS.md +++ b/skill/PATTERNS.md @@ -673,6 +673,15 @@ shell region with it does nothing at all, silently. Use option 1 (derive the value in JS on `onAfterSwap`) or option 3 (`data-no-transition` to force a full load at that boundary) instead. +**If you do add `[data-island][data-section-id]` to a section inside the +container, the id must name a `#shopify-section-` wrapper that exists in the +same document.** Shopify keys the Section Rendering API response by section id +and Pusha replaces that wrapper, so an island naming an id with no wrapper — +a theme block, or a section rendered without one — is fetched and then has +nowhere to go. Run the port with `debug: true` and the log prints `NO TARGET` +for exactly that, plus `swapped /` so a partial apply is +visible. + **3. Mark as reload boundary.** For L-B auth flows specifically: add the relevant route to `data-no-transition` (the skill's bucket H/K escape hatch) so the link forces a full nav, refreshing the persistent shell's `customer.*` state. Pusha already excludes these from interception — the authority is `SHOPIFY_RESERVED` in `src/routes.ts`, shared by link interception and prefetch so the two cannot disagree. It currently covers `/checkout(s)`, the `/account/*` auth routes, `/customer_authentication/*`, `/password`, `/localization`, `/gift_card(s)`, the app proxy `/a/*`, and the GET routes that mutate cart state (`/cart/add`, `/cart/change`, `/cart/update`, `/cart/clear`, cart permalinks) plus `/discount/*`. Read the regex rather than trusting this list. diff --git a/src/islands.ts b/src/islands.ts index 0247b92..e071a3c 100644 --- a/src/islands.ts +++ b/src/islands.ts @@ -70,25 +70,58 @@ export async function revalidateIslands( return; } + const applied = new Set(); Object.entries(json).forEach(([sectionId, html]) => { const selector = `#shopify-section-${CSS.escape(sectionId)}`; - document.querySelectorAll(selector).forEach((wrapper) => { + const wrappers = document.querySelectorAll(selector); + if (wrappers.length === 0) { + // The server rendered the section but there is no `#shopify-section-` + // wrapper in this document to put it in. A theme block, a section the + // swap removed, or an id that never had a wrapper. Silence here is what + // made this look like a success. + dlog('islands', `NO TARGET for ${sectionId} — no #shopify-section-${sectionId} in the document`); + return; + } + wrappers.forEach((wrapper) => { hooks.onBeforeSwap?.(wrapper); const tmp = document.createElement('div'); tmp.innerHTML = html.trim(); const fresh = tmp.firstElementChild as HTMLElement | null; - if (!fresh) return; + if (!fresh) { + dlog('islands', `EMPTY response for ${sectionId} — keeping the current markup`); + return; + } wrapper.replaceWith(fresh); + applied.add(sectionId); hooks.onAfterSwap?.(fresh); }); }); + // Remove the class explicitly. It used to come off only because + // `wrapper.replaceWith()` destroyed the node carrying it, which meant every + // path that did NOT swap — no wrapper, empty response, an island marker that + // sits outside the wrapper being replaced — stranded `.is-revalidating` on a + // live element forever. A theme styling it as a dim or skeleton got a region + // that never came back. Removing from detached nodes too is harmless and + // keeps the rule one line instead of a liveness check. + islands.forEach(({ islandEl }) => islandEl.classList.remove('is-revalidating')); + const elapsed = Math.round((typeof performance !== 'undefined' ? performance.now() : Date.now()) - started); - dlog('islands', `swapped ${Object.keys(json).length} in ${elapsed}ms`); + const requested = Object.keys(json); + // Count what was APPLIED, not what came back. The old line reported the + // number of keys in the response, so "swapped 1" was printed whether or not a + // single node had been touched — the diagnostic that should have caught the + // stranded class instead certified the run as clean. + dlog('islands', `swapped ${applied.size}/${requested.length} in ${elapsed}ms`); + if (applied.size !== requested.length) { + dlog('islands', `NOT applied: ${requested.filter((id) => !applied.has(id)).join(', ')}`); + } document.dispatchEvent( - new CustomEvent('pjax:islands-revalidated', { detail: { sectionIds: Object.keys(json) } }), + new CustomEvent('pjax:islands-revalidated', { + detail: { sectionIds: Array.from(applied), requestedIds: requested }, + }), ); } diff --git a/test/smoke.test.ts b/test/smoke.test.ts index 6ba1af5..26c69e5 100644 --- a/test/smoke.test.ts +++ b/test/smoke.test.ts @@ -711,6 +711,91 @@ test('islands revalidate on a #hash URL', async () => { assert.equal(parsed.pathname, '/products/foo'); }); +test('islands: .is-revalidating is cleared even when nothing was swapped', async () => { + // Found in a browser. The class used to come off only because + // `wrapper.replaceWith()` destroyed the node carrying it, so any path that + // did not swap left it on a live element forever — and a theme styling it as + // a dim or skeleton got a region that never came back. + // + // This fixture has the island marker but no `#shopify-section-price` wrapper + // to swap into, which is the exact shape the pre-existing hash test already + // had without anyone noticing. + const islandHtml = '
£20
'; + (globalThis as Record).fetch = async (input: string | URL) => { + const url = String(input); + if (url.includes('sections=')) { + return new Response(JSON.stringify({ price: islandHtml }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + } + return new Response( + makePageHtml('product', '
£10
'), + { status: 200, headers: { 'Content-Type': 'text/html' } }, + ); + }; + + runtime.initRuntime({ prefetchConfig: { product: { soft: 1, hard: 60_000 } } }); + const prefetch = await import('../src/prefetch.ts'); + await prefetch.prefetchPage('/products/foo'); + document.querySelector('a[href="/products/foo"]')!.click(); + await new Promise((r) => setTimeout(r, 120)); + + assert.equal( + document.querySelectorAll('.is-revalidating').length, + 0, + 'no element is left dimmed after the cycle finishes', + ); +}); + +test('islands: the event reports what was applied, not what was requested', async () => { + // The old diagnostic counted keys in the response, so "swapped 1" printed + // whether or not a node had been touched. Same for the event's sectionIds. + // A listener re-initialising the regions it names would have been re-running + // against markup that never changed. + const detail: Array> = []; + document.addEventListener('pjax:islands-revalidated', (e) => { + detail.push((e as CustomEvent).detail); + }); + + (globalThis as Record).fetch = async (input: string | URL) => { + const url = String(input); + if (url.includes('sections=')) { + // Two sections come back; only `price` has somewhere to go. + return new Response( + JSON.stringify({ + price: '
£20
', + ghost: '
nowhere
', + }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ); + } + return new Response( + makePageHtml( + 'product', + '
£10
' + + '
no wrapper for me
', + ), + { status: 200, headers: { 'Content-Type': 'text/html' } }, + ); + }; + + runtime.initRuntime({ prefetchConfig: { product: { soft: 1, hard: 60_000 } } }); + const prefetch = await import('../src/prefetch.ts'); + await prefetch.prefetchPage('/products/foo'); + document.querySelector('a[href="/products/foo"]')!.click(); + await new Promise((r) => setTimeout(r, 120)); + + assert.equal(detail.length, 1, 'the event fires once'); + assert.deepEqual(detail[0].sectionIds, ['price'], 'only the section that actually swapped'); + assert.deepEqual( + (detail[0].requestedIds as string[]).sort(), + ['ghost', 'price'], + 'what was asked for is still reported, separately', + ); + assert.equal(document.querySelectorAll('.is-revalidating').length, 0, 'nothing left dimmed'); +}); + test('stylesheets in section bodies are synced, and only once', async () => { // Shopify's stylesheet_tag emits the link inside the section body, so a // head-only scan missed every section stylesheet and the first visit to each From 22179b82b6a090e0339760ebe2adb3ceac9a6ca9 Mon Sep 17 00:00:00 2001 From: Ephriam Nagler Date: Sat, 12 Sep 2026 07:38:09 -0400 Subject: [PATCH 6/9] fix(islands): name a response that does not answer the question MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first run with honest counting produced a confusing pair of lines: Pusha asked for `template--22360478679180__main` and the loop reported `NO TARGET for product`. Both true, and together they read like a theme markup problem — when the actual event is that the response was not keyed by the ids that were requested. Diagnostic only. What to DO about a mismatched response is not decided yet, because the cause is not known: this was measured on `shopify theme dev`, and the local proxy is already known to behave differently under load. Writing a fallback now would be guessing, and a wrong fallback here swaps the wrong markup into a live page. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8 --- src/islands.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/islands.ts b/src/islands.ts index e071a3c..249ef39 100644 --- a/src/islands.ts +++ b/src/islands.ts @@ -70,6 +70,23 @@ export async function revalidateIslands( return; } + // The response is supposed to be keyed by the ids we asked for. When it is + // not, every downstream symptom is confusing — the swap loop iterates keys + // that name no wrapper and reports "no target", which reads like a theme + // markup problem rather than a response that did not answer the question. + // Measured on `shopify theme dev`: asked for `template--__main`, got back + // a key of `product`. + const askedFor = islands.map((i) => i.sectionId); + const cameBack = Object.keys(json); + const missing = askedFor.filter((id) => !cameBack.includes(id)); + if (missing.length) { + dlog( + 'islands', + `RESPONSE MISMATCH — asked for [${askedFor.join(', ')}], got [${cameBack.join(', ')}]. ` + + `Missing: ${missing.join(', ')}. The server did not key the response by the requested section ids.`, + ); + } + const applied = new Set(); Object.entries(json).forEach(([sectionId, html]) => { const selector = `#shopify-section-${CSS.escape(sectionId)}`; From af0f715fad5db417186be21b7db2c435ac8aa6e8 Mon Sep 17 00:00:00 2001 From: Ephriam Nagler Date: Sat, 12 Sep 2026 07:44:31 -0400 Subject: [PATCH 7/9] fix(islands): the Accept header was asking for the wrong document MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Islands have never worked on a product page. Not on Horizon — on any theme, since the first release. `Accept: application/json` on a product URL triggers Shopify's product-JSON content negotiation, and it beats the query string: `?sections=` is ignored and the body is `{"product": {...}}`, status 200, content-type application/json. So the response parsed, the cycle completed, and the one key in it named no section. Every layer reported success. Measured with curl against the same URL — with the header, the product object; without it, the section HTML keyed by the requested id, wrapper included. Removing the header is the whole fix. The Section Rendering API keys off `?sections=` and never needed an Accept header. Three things had to be fixed before this was findable, which is the part worth keeping: the stranded `.is-revalidating`, the counter that reported response keys instead of applied nodes, and the mismatch line that said "asked for X, got Y". The first browser run reported `swapped 1` and looked clean. Islands are the reason to distrust a green jsdom suite — the fixture answers whatever the stub is told to answer, so a content-negotiation bug is invisible by construction. README needs no change: no config key, event, or contract moved. Recorded in CLAUDE.md under Islands, since the next person to add a header there needs to know why there isn't one. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8 --- CLAUDE.md | 12 ++++++++++++ src/islands.ts | 13 ++++++++++++- test/smoke.test.ts | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 73df774..624a1ec 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -290,6 +290,18 @@ The Shopify Section Rendering API lets you fetch any section's HTML in isolation **Visual contract**: while an island is revalidating, it gets a `.is-revalidating` class that themes can style for a subtle dim/skeleton. The class is removed when the revalidation cycle ends — **on every path, including the ones that swap nothing**. It used to come off only because `wrapper.replaceWith()` destroyed the node carrying it, so an island whose `#shopify-section-` wrapper was absent kept the class on a live element forever and a themed dim never lifted. Found in a browser, 2026-09-12; the diagnostic that should have caught it counted keys in the response rather than nodes replaced, so it printed `swapped 1` for a run that swapped nothing. +**Never send `Accept: application/json` on the sections request.** On a product +URL that header triggers Shopify's product-JSON content negotiation, which beats +the query string: `?sections=` is ignored and the response is `{"product": {…}}` +at status 200 with content-type `application/json`. Every check downstream +passes, the single key names no section, and nothing is swapped. Islands +therefore never worked on a product page — the case they exist for — from the +first release until 2026-09-12, and no test caught it because jsdom fixtures +answer whatever the stub is told to answer. Measured with curl on the same URL: +with the header, the product object; without it, the section HTML keyed by the +requested id. The Section Rendering API keys off `?sections=` and needs no +Accept header. + **Exported as `@mimeticthemes/pusha/islands` subpath** for explicit imports. Auto-imported by main entry when prefetch is enabled. ### Theme editor integration diff --git a/src/islands.ts b/src/islands.ts index 249ef39..bb412ca 100644 --- a/src/islands.ts +++ b/src/islands.ts @@ -57,9 +57,20 @@ export async function revalidateIslands( let json: Record; try { + // NO `Accept: application/json`. On a product URL that header triggers + // Shopify's product-JSON content negotiation, which wins over the query + // string: `?sections=` is ignored and the response is the PRODUCT object — + // `{"product": {...}}`, status 200, content-type application/json. Every + // downstream check passed and nothing was ever swapped, because the one key + // in the response named no section. Islands therefore never worked on a + // product page, on any theme, which is the case they exist for (price and + // availability). Measured with curl against the same URL, 2026-09-12: + // with the header, the product JSON; without it, the section HTML keyed by + // the requested id. The Section Rendering API keys off `?sections=` and + // needs no Accept header at all. const res = await fetch(url, { method: 'GET', - headers: { 'X-Requested-With': 'XMLHttpRequest', Accept: 'application/json' }, + headers: { 'X-Requested-With': 'XMLHttpRequest' }, credentials: 'same-origin', }); if (!res.ok) throw new Error(`sections=${res.status}`); diff --git a/test/smoke.test.ts b/test/smoke.test.ts index 26c69e5..6208c1a 100644 --- a/test/smoke.test.ts +++ b/test/smoke.test.ts @@ -711,6 +711,45 @@ test('islands revalidate on a #hash URL', async () => { assert.equal(parsed.pathname, '/products/foo'); }); +test('islands: the sections request sends no Accept: application/json', async () => { + // Measured in a browser and then with curl. On a product URL that header + // triggers Shopify's product-JSON content negotiation, which beats the query + // string: `?sections=` is ignored and the body is `{"product": {...}}` with + // status 200 and content-type application/json. Every check downstream + // passed, the single key named no section, and nothing was ever swapped — so + // islands never worked on a product page, which is the case they exist for. + const headersSeen: Array> = []; + (globalThis as Record).fetch = async (input: string | URL, init?: RequestInit) => { + const url = String(input); + if (url.includes('sections=')) { + headersSeen.push((init?.headers ?? {}) as Record); + return new Response(JSON.stringify({ price: '
£20
' }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + } + return new Response( + makePageHtml('product', '
£10
'), + { status: 200, headers: { 'Content-Type': 'text/html' } }, + ); + }; + + runtime.initRuntime({ prefetchConfig: { product: { soft: 1, hard: 60_000 } } }); + const prefetch = await import('../src/prefetch.ts'); + await prefetch.prefetchPage('/products/foo'); + document.querySelector('a[href="/products/foo"]')!.click(); + await new Promise((r) => setTimeout(r, 120)); + + assert.equal(headersSeen.length, 1, 'the sections request was made'); + const keys = Object.keys(headersSeen[0]).map((k) => k.toLowerCase()); + assert.ok(!keys.includes('accept'), `no Accept header — got ${JSON.stringify(headersSeen[0])}`); + assert.equal( + (headersSeen[0] as Record)['X-Requested-With'], + 'XMLHttpRequest', + 'the request is still identifiable as a Pusha fetch', + ); +}); + test('islands: .is-revalidating is cleared even when nothing was swapped', async () => { // Found in a browser. The class used to come off only because // `wrapper.replaceWith()` destroyed the node carrying it, so any path that From 7b599cc15f13908b533007c89d889b55fb312fe9 Mon Sep 17 00:00:00 2001 From: Ephriam Nagler Date: Sat, 12 Sep 2026 07:55:01 -0400 Subject: [PATCH 8/9] =?UTF-8?q?feat(audit):=20bucket=20R=20=E2=80=94=20isl?= =?UTF-8?q?and=20candidates?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit had no island coverage at all for classic OS 2.0 — `island` appeared twice in the CLI, both about new-Liquid partials. So nothing told a porting agent which regions were stale-prone, nobody marked any, and islands went a full release with no marker in any theme. That is why the Accept-header bug survived: there was nothing to exercise. R is bucket L's question asked about the swap container. They do not overlap — an L finding freezes because the shell is never swapped, an R finding goes stale because the page HTML came out of the prefetch cache. Sub-letters: A price, B availability/inventory, C variant state, D cart-derived (already covered by cart:mutated), E render-time values, plus `marker` for conformance of an existing data-island. **Candidates are `decide`, never `transform`.** Revalidating costs a request and re-runs the section's JS, so marking a region that does not actually change taxes every cached navigation. Whether a price is stale-prone is a question about the store, not the code, and the audit must not answer it. Broken markers ARE `transform` — those are mechanical, and each fails silently: - `data-island` with no `data-section-id` (the selector needs both) - a marker in the persistent shell (islands are collected from the swapped container only) - a snippet writing {{ section.id }} whose render sites pass no `section:` — {% render %} does not inherit scope, so the attribute renders empty That last one is verified, not assumed. The first version flagged every snippet reading section.id and reported the rig's correctly-wired island as dead. It now resolves the actual render sites and names the ones that miss it. Reporting on shape alone is the same error as suppressing on shape, inverted. `section` IS in scope in theme blocks — checked against a Horizon port, where 13 of 93 blocks read section.id and buy-buttons.liquid already writes data-section-id. I had assumed otherwise; the assumption was wrong and the check would have encoded it. New fixture covers all four marker cases against a correctly-wired control. Goldens regenerated: only the R section and summary line moved, one legitimate finding each in os2-sections and analytics-surface, no noise anywhere else. All four surfaces: BUCKET_RULES.R + report + JSON action mapping in the CLI, SKILL.md (batching guidance), PATTERNS.md (the four things that must hold, each of which fails silently), README and CLAUDE.md. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8 --- CLAUDE.md | 11 +- README.md | 2 +- bin/pusha.js | 249 +++++++++++++++++- skill/PATTERNS.md | 49 ++++ skill/SKILL.md | 10 +- test/audit-golden.test.ts | 4 + .../island-candidates/assets/theme.js | 3 + .../island-candidates/blocks/inventory.liquid | 3 + .../island-candidates/layout/theme.liquid | 11 + .../sections/main-product.liquid | 8 + .../island-candidates/snippets/no-id.liquid | 1 + .../snippets/price-badge.liquid | 3 + .../snippets/site-footer.liquid | 3 + .../snippets/stock-note.liquid | 3 + .../island-candidates/templates/index.json | 1 + test/golden/analytics-surface.txt | 10 +- test/golden/app-blocks-classic.txt | 5 + test/golden/app-edge-cases.txt | 5 + test/golden/app-surface-horizon.txt | 5 + test/golden/island-candidates.txt | 174 ++++++++++++ test/golden/new-liquid.txt | 5 + test/golden/os2-sections.txt | 10 +- test/golden/ported-theme.txt | 5 + test/golden/uppercase-tags.txt | 5 + 24 files changed, 578 insertions(+), 7 deletions(-) create mode 100644 test/fixtures/island-candidates/assets/theme.js create mode 100644 test/fixtures/island-candidates/blocks/inventory.liquid create mode 100644 test/fixtures/island-candidates/layout/theme.liquid create mode 100644 test/fixtures/island-candidates/sections/main-product.liquid create mode 100644 test/fixtures/island-candidates/snippets/no-id.liquid create mode 100644 test/fixtures/island-candidates/snippets/price-badge.liquid create mode 100644 test/fixtures/island-candidates/snippets/site-footer.liquid create mode 100644 test/fixtures/island-candidates/snippets/stock-note.liquid create mode 100644 test/fixtures/island-candidates/templates/index.json create mode 100644 test/golden/island-candidates.txt diff --git a/CLAUDE.md b/CLAUDE.md index 624a1ec..184cb2c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -118,7 +118,7 @@ This is the command that converts a curious reader into a user. It's also where #### `npx @mimeticthemes/pusha audit` -Runs the audit against the current directory (or path argument). Buckets A–H and K (per-script) plus the surface buckets: J (analytics), L (per-request Liquid in the shell), M (persistent-shell stateful UI), P (partials), X (theme app extensions). `--json` emits structured output for tools/agents. The CLI is the canonical audit implementation — there is no longer a parallel bash script. +Runs the audit against the current directory (or path argument). Buckets A–H and K (per-script) plus the surface buckets: J (analytics), L (per-request Liquid in the shell), M (persistent-shell stateful UI), P (partials), R (island candidates), X (theme app extensions). `--json` emits structured output for tools/agents. The CLI is the canonical audit implementation — there is no longer a parallel bash script. The authority on this list is `BUCKET_RULES` in `bin/pusha.js`; `skill/SKILL.md` restates it for the porting agent. All three drifted once already — X shipped in the CLI and the skill while this line still stopped at P. @@ -302,6 +302,15 @@ with the header, the product object; without it, the section HTML keyed by the requested id. The Section Rendering API keys off `?sections=` and needs no Accept header. +**The audit proposes islands; it never marks them.** Bucket R lists stale-prone +regions inside the container (price, availability, inventory, selected variant, +render-time values) and checks conformance of any `data-island` already present. +Candidates are `decide`, never `transform`: revalidating costs a request and +re-runs the section's JS, so whether a region is stale-prone enough to spend +that on is a question about the store. Broken markers — no `data-section-id`, a +marker in the shell, a snippet whose render sites pass no `section:` — are +`transform`, because those are mechanical and each one fails silently. + **Exported as `@mimeticthemes/pusha/islands` subpath** for explicit imports. Auto-imported by main entry when prefetch is enabled. ### Theme editor integration diff --git a/README.md b/README.md index 7e63d56..172c213 100644 --- a/README.md +++ b/README.md @@ -985,7 +985,7 @@ path to audit a different directory. `--claude` / `--cursor` / `--aider` to install the skill for that agent. Add `--global` to install into `~/` instead of the project. -The audit classifies every script in the theme by transformation difficulty (buckets A–H, plus K for portal-to-body custom elements needing `data-pusha-cleanup`), and then the buckets that cover a whole surface rather than a single script: J (analytics — coverage, payload conformance, placement, and raw pixels that bypass Customer Events), L (per-request Liquid frozen in the shell), M (persistent-shell stateful UI), P (`{% partial %}` regions), and X (theme app extensions — which installed app blocks and embeds sit inside the swap container, and which survive it). The `pusha` skill consumes this output to apply the wrappers; agents without the skill can act on the audit's prescriptive "Next steps" block directly. +The audit classifies every script in the theme by transformation difficulty (buckets A–H, plus K for portal-to-body custom elements needing `data-pusha-cleanup`), and then the buckets that cover a whole surface rather than a single script: J (analytics — coverage, payload conformance, placement, and raw pixels that bypass Customer Events), L (per-request Liquid frozen in the shell), M (persistent-shell stateful UI), P (`{% partial %}` regions), R (island candidates — stale-prone regions inside the container, and conformance of any `data-island` markers already there), and X (theme app extensions — which installed app blocks and embeds sit inside the swap container, and which survive it). The `pusha` skill consumes this output to apply the wrappers; agents without the skill can act on the audit's prescriptive "Next steps" block directly. A `create-pusha` scaffolder for new themes is on the roadmap, once Pusha is published. diff --git a/bin/pusha.js b/bin/pusha.js index 6baa12b..f186eed 100755 --- a/bin/pusha.js +++ b/bin/pusha.js @@ -1199,6 +1199,185 @@ function detectLiquidPersistentState(file, themePath) { return findings; } +// ─── Bucket R: island candidates ──────────────────────────────────────────── +// Stale-prone regions INSIDE the swap container. Bucket L is the same question +// asked about the persistent shell; R is the in-container answer, and the two +// do not overlap — L findings freeze because the shell is never swapped, R +// findings go stale because the page HTML came out of the prefetch cache. +// +// The remedy is opt-in markup, never inference. Pusha does not and must not +// guess which regions are stale-prone: revalidating a section costs a request +// and re-running its JS, so a wrong guess is a tax on every cached navigation. +// This bucket produces CANDIDATES for a human or agent to accept or decline. +// +// Ranks: `mark` — clear candidate, add the attributes. `ask` — plausible but +// the call depends on the merchant (a price that never changes is not stale). +// `ok` — already marked and correctly wired. +const R_PATTERNS = [ + // R-A price — the canonical island. Prefetch TTLs are minutes; a price + // change inside that window is served stale from cache. + { sub: 'A', rank: 'mark', re: /\|\s*money(_with_currency|_without_currency|_without_trailing_zeros)?\b/, what: 'money filter' }, + { sub: 'A', rank: 'mark', re: /\b(compare_at_price|price_varies|price_min|price_max)\b/, what: 'price comparison/range' }, + + // R-B availability — the one that costs a sale. A sold-out variant served + // from cache lets a buyer add to cart and fail at checkout. + { sub: 'B', rank: 'mark', re: /\b(available|sold_out)\b/, what: 'availability' }, + { sub: 'B', rank: 'mark', re: /\binventory_(quantity|policy|management)\b/, what: 'inventory.*' }, + + // R-C variant state — selected variant drives price and availability, so a + // stale variant object is both of the above at once. + { sub: 'C', rank: 'mark', re: /\bselected_or_first_available_variant\b/, what: 'selected_or_first_available_variant' }, + { sub: 'C', rank: 'ask', re: /\bcurrent_variant\b/, what: 'current_variant' }, + + // R-D cart-derived inside the container. Already covered by cart:mutated + // invalidating the cache, so this is a verify rather than a fix — listed + // because a theme that dispatches no cart:mutated has neither mechanism. + { sub: 'D', rank: 'ok', re: /\bcart\.(item_count|total_price|items_subtotal_price)\b/, what: 'cart.* in container' }, + + // R-E render-time values. `now` is the clearest case in the language: the + // value is wrong the moment it is cached. + { sub: 'E', rank: 'mark', re: /['"]now['"]\s*\|\s*date\b/, what: "'now' | date" }, +]; + +// A marker that cannot work. Each of these renders without error and silently +// never revalidates, which is the failure mode this bucket exists to prevent — +// islands shipped for a whole release with none of these checks and nobody +// noticed, because nothing throws. +function checkIslandMarkers(file, themePath, loc) { + const rel = relative(themePath, file); + const text = stripLiquidComments(readFileText(file)); + const lines = text.split('\n'); + const out = []; + for (let i = 0; i < lines.length; i++) { + const ln = lines[i]; + if (!/\bdata-island\b/.test(ln)) continue; + const line = i + 1; + const match = ln.trim().slice(0, 160); + + // The attributes may be spread over several lines of one tag, so look at a + // small window rather than the single line. + const windowText = lines.slice(Math.max(0, i - 6), i + 7).join('\n'); + const hasSectionId = /\bdata-section-id\s*=/.test(windowText); + + if (!hasSectionId) { + out.push({ file: rel, line, match, sub: 'marker', rank: 'gap', location: loc, + what: 'data-island with no data-section-id — findIslands matches [data-island][data-section-id], so this is inert' }); + continue; + } + if (loc === 'shell') { + out.push({ file: rel, line, match, sub: 'marker', rank: 'gap', location: loc, + what: 'data-island in the persistent shell — islands are collected from the swapped container only, so this never revalidates' }); + continue; + } + // `{% render %}` does not inherit the caller's scope. A snippet writing + // `{{ section.id }}` without being passed `section` renders an EMPTY id, + // and an empty data-section-id fails the attribute selector silently. + if (loc === 'include' && /\bdata-section-id\s*=\s*["']\{\{-?\s*section\.id/.test(windowText)) { + // Verify before reporting. The snippet is only broken if a caller fails + // to pass `section:` — a correctly-wired island would otherwise be + // reported as dead, which is the same error as suppressing on shape, + // inverted. + const snippetName = rel.replace(/^snippets\//, '').replace(/\.liquid$/, ''); + const bad = renderSitesMissingSection(themePath, snippetName); + if (bad.length) { + out.push({ file: rel, line, match, sub: 'marker', rank: 'gap', location: loc, + what: `snippet writes {{ section.id }} and {% render %} does not inherit scope — these render sites pass no \`section:\`, so the id renders empty there: ${bad.join(', ')}` }); + continue; + } + } + out.push({ file: rel, line, match, sub: 'marker', rank: 'ok', location: loc, + what: 'island marker present with a section id' }); + } + return out; +} + +function detectIslandCandidates(themePath, shellRelSet) { + const findings = []; + const dirs = ['sections', 'blocks', 'snippets', 'templates']; + for (const dir of dirs) { + const abs = join(themePath, dir); + if (!existsSync(abs)) continue; + for (const file of walkFiles(abs, ['.liquid'])) { + const rel = relative(themePath, file); + const loc = locationClass(rel, shellRelSet); + + // Marker conformance runs everywhere, including the shell — a marker in + // the shell is precisely one of the things worth reporting. + findings.push(...checkIslandMarkers(file, themePath, loc)); + + // Candidates are an in-container question only. Shell staleness is + // bucket L and has different remedies. + if (loc === 'shell') continue; + + const text = stripLiquidComments(readFileText(file)); + if (/\bdata-island\b/.test(text)) continue; // already marked — no candidate noise + const lines = text.split('\n'); + const seen = new Set(); + for (let i = 0; i < lines.length; i++) { + const ln = lines[i]; + for (const pat of R_PATTERNS) { + if (!pat.re.test(ln)) continue; + // One finding per file+sub. A product template mentions `money` + // fifteen times and they are all the same decision. + const key = `${rel}:${pat.sub}`; + if (seen.has(key)) continue; + seen.add(key); + findings.push({ + file: rel, + line: i + 1, + match: ln.trim().slice(0, 160), + sub: pat.sub, + rank: pat.rank, + location: loc, + what: pat.what, + sectionIdInScope: sectionIdResolvable(text, loc), + }); + } + } + } + } + return findings; +} + +// Can this file write a usable `data-section-id`? Sections and theme blocks get +// `section` from the renderer. Snippets do not — verified against a Horizon +// port, where 13 of 93 blocks read `section.id` happily while snippets have to +// be handed it at the render site. +function sectionIdResolvable(text, loc) { + if (loc === 'section' || loc === 'block') return true; + if (loc === 'include') return /\bsection\.id\b/.test(text) ? 'passed-in-required' : false; + return false; +} + +// Which render sites of this snippet fail to pass `section:`? Verified rather +// than assumed: a snippet reading `{{ section.id }}` is only broken if the +// render tag does not hand it `section`, and flagging every such snippet on +// shape alone reports a working island as broken. Returns the offending sites, +// so an empty array means every caller is correct. +function renderSitesMissingSection(themePath, snippetName) { + const missing = []; + for (const dir of ['sections', 'blocks', 'snippets', 'layout', 'templates']) { + const abs = join(themePath, dir); + if (!existsSync(abs)) continue; + for (const file of walkFiles(abs, ['.liquid'])) { + const text = stripLiquidComments(readFileText(file)); + // The whole tag, which may wrap across lines when it carries arguments. + const re = new RegExp( + `\\{%-?\\s*(?:render|include)\\s+['"]${snippetName}['"]([\\s\\S]*?)-?%\\}`, + 'g', + ); + let m; + while ((m = re.exec(text)) !== null) { + const args = m[1] ?? ''; + if (/\bsection\s*:/.test(args)) continue; + const line = text.slice(0, m.index).split('\n').length; + missing.push(`${relative(themePath, file)}:${line}`); + } + } + } + return missing; +} + // ─── Bucket M: persistent-shell stateful UI ───────────────────────────────── // Modals, drawers, overlays, and dropdowns that live OUTSIDE #MainContent and // were authored assuming a full reload would dismiss them. PJAX swaps don't @@ -1333,7 +1512,7 @@ function auditTheme(themePath, { useWhitelists = true } = {}) { if (!existsSync(dirs[name])) console.error(`! ${name}/ not found at ${dirs[name]} — skipping`); } - const findings = { A: [], B: [], C: [], D: [], E: [], F: [], G: [], H: [], J: [], K: [], L: [], M: [], P: [], X: [], unknown: [] }; + const findings = { A: [], B: [], C: [], D: [], E: [], F: [], G: [], H: [], J: [], K: [], L: [], M: [], P: [], R: [], X: [], unknown: [] }; // Each entry: { bucket, file, line?, match?, reason? } — the would-have-been // finding plus why it was suppressed. `files` keyed by Pusha-self filenames; // `G` keyed by the file-level Pusha.on* trigger; `H` keyed by line patterns. @@ -1536,6 +1715,12 @@ function auditTheme(themePath, { useWhitelists = true } = {}) { // location class so the report prescribes a paradigm-correct fix instead of a // blanket "wrap in sectionInits". const shellRelSet = new Set(shellFiles.map((f) => relative(themePath, f))); + + // R — island candidates. Runs after the shell is resolved because the whole + // question is "is this inside the swap container", and that is what the shell + // resolution answers. + findings.R = detectIslandCandidates(themePath, shellRelSet); + for (const f of findings.E) { let loc = locationClass(f.file, shellRelSet); if (loc === 'shell' && inlineScriptIsConfigOnly(join(themePath, f.file), f.line)) loc = 'head-config'; @@ -1638,6 +1823,11 @@ function auditTheme(themePath, { useWhitelists = true } = {}) { M_body_classes: findings.M.filter((x) => x.kind === 'body-class').length, P: findings.P.length, P_gaps: findings.P.filter((x) => x.dangling === 'undeclared').length, + R: findings.R.length, + R_mark: findings.R.filter((x) => x.rank === 'mark').length, + R_ask: findings.R.filter((x) => x.rank === 'ask').length, + R_gaps: findings.R.filter((x) => x.rank === 'gap').length, + R_marked: findings.R.filter((x) => x.sub === 'marker' && x.rank === 'ok').length, J: findings.J.length, J_gaps: findings.J.filter((x) => x.rank === 'gap').length, J_warns: findings.J.filter((x) => x.rank === 'warn').length, @@ -2130,6 +2320,7 @@ const BUCKET_RULES = { L: 'Per-request Liquid in the layout shell (layout/theme.liquid, section groups, transitively-rendered snippets) freezes on first load. Sub-letters mirror the request-scoped taxonomy: A=URL/template, B=customer, C=cart, D=locale, E=per-page object, F=personalization, G=time, H=app-injected. Rank: auto=URL-derivable in JS, ask=user decides (full-reload boundary or section refetch), ok=already handled by Pusha or theme convention.', M: 'Persistent-shell stateful UI — modals/drawers/overlays that lived outside #MainContent and were authored assuming a full reload would dismiss them. Three remediation options: (1) add `data-pusha-close-on-nav` to the root (Pusha strips `[open]` / sets `aria-expanded="false"` / removes body classes listed in `data-pusha-body-class-on-open`); (2) implement a `closeOnNav()` method on the custom element; (3) call `Pusha.onBeforeLeave(() => this.close())` manually. Cart drawers and persistent widgets simply omit the marker — opt-in is the safe default.', J: 'Analytics surface. NOTE: Pusha reaches the pixel sandbox only through PREFIXED CUSTOM events — the customEvents bridge publishes pusha:page_viewed plus prefixed copies of the page-type payloads, and those are delivered to custom pixels and app pixels. What is fenced is publishing under STANDARD names, which the storefront API rejects, so a standard page_viewed never arrives on a swap. Delivery is not consumption: a third-party app pixel subscribed to the standard vocabulary has no mapping for a prefixed name, so reviving it still needs a companion custom pixel that forwards the event (docs/analytics-companion-pixel.md, README "Analytics & tracking"). This bucket checks the theme-serialized