Scope theme-contract requirements: pixels binds the fleet, runtime hooks bind Spark

CHANGELOG.md

@@ -6,6 +6,10 @@ Spark follows human-readable release notes rather than a package-manager version
 
 The GitHub release body is a summary, not a copy of the changelog section. Write one sentence framing the release, then a `### Highlights` list of at most five bullets, then a link to `CHANGELOG.md` at the release tag for the full record. A changelog entry stays as long as the change needs it to be, but the release page is scanned rather than read, so pasting a long entry into it produces notes nobody can follow. That is what happened to 1.3.0 and 1.4.0, both since rewritten. Use the 1.2.0 release as the reference format.
 
+## Unreleased
+
+- Every `theme-contract.json` requirement now carries a required `scope`, and `scripts/check-theme-contract.py` enforces it. `fleet` binds every Spark-derived store theme and is what a live check (`--store` + `--theme-id`) enforces by default; `spark` binds Spark's own working copy and is what a local check (`--root`, CI, `make contract`) enforces by default, on top of the fleet rules. `--scope fleet|spark` overrides either default, and the gate's output names the scope and how many requirements ran. `pixels` is the only fleet rule; the four runtime hooks #66 added in 1.5.0 without a changelog line (`cart-badge`, `mobile-nav-toggle`, `mobile-nav`, `cart-drawer`; the 1.5.0 entry below still describes the contract as `pixels` only) are scoped to Spark. The first fleet sweep against 1.5.0 reported nine of thirteen live Spark themes failing `mobile-nav` on the file rule alone, while four of those storefronts served `#mobile-nav` from another file: derived themes are forks that may carry a hook elsewhere, and a file-location rule against the fleet reports forks, not faults. Decided 2026-09-21. `docs/theme-contract.md` lists all five rules with their scope, the rule for promoting one to `fleet`, and the invocation for each case: a fork's working copy (`--root ../fork --scope fleet`), a derived live theme (default), and Spark's own copy on a dev store (`--scope spark`, since a live check otherwise assumes a derived theme).
+
 ## 1.5.0 - 2026-09-21
 
 - Product cards in grids now render for a product that cannot be bought instead of disappearing. The new Theme Setting `product_card_sold_out_style` (Product Pages > Product Cards) chooses `badge` (default: the card plus a localized Sold out badge in place of the Sale badge), `muted` (badge plus reduced opacity), or `hide` (the previous behaviour). A store whose data file predates the key falls back to `badge` without a `settings_data.json` push. `docs/extending-spark.md` gained a "Settings Data Safety On Live Stores" section: never push `configs/settings_data.json` to a live store by default, prefer template fallbacks so no data push is needed, and when keys must land on a live store pull first and merge onto the live file; `CLAUDE.md` and `docs/theme-settings-partials.md` point at it.

Makefile

@@ -45,9 +45,13 @@ push: css-check
 test:
 	python3 -m unittest discover -s tests
 
-# Assert the platform integration points in theme-contract.json.
+# Assert theme-contract.json: the platform integration points every derived theme
+# keeps (fleet scope) plus, on Spark's own copy, the runtime hooks theme.js
+# resolves (spark scope). A live check defaults to the fleet scope; add
+# --scope spark when the live theme is Spark itself.
 # Point it at a live theme before or after a push:
-#   make contract THEME_ARGS="--store https://x.29next.store --theme-id 68"
+#   make contract THEME_ARGS="--store https://x.29next.store --theme-id 68"               # a derived theme
+#   make contract THEME_ARGS="--store https://dev.29next.store --theme-id 68 --scope spark"  # Spark itself
 contract:
 	python3 scripts/check-theme-contract.py $(THEME_ARGS)
 

TODOS.md

@@ -2,6 +2,24 @@
 
 ## Open
 
+### Theme contract: mask HTML comments in the needle search
+**Priority:** P1
+**Effort:** S
+**What:** `scripts/check-theme-contract.py` masks only DTL comments (`{# #}`, `{% comment %}`, `{% verbatim %}`) before searching for a requirement's `must_contain`. A live theme carrying `<!-- {% pixels %} -->` passes the `pixels` rule while the browser discards the rendered tracker iframes, so the storefront emits no events. Wrap the checker's mask with an HTML-comment mask (checker-side only; `check-templates.py`'s masking has other consumers) and add the negative test. `{% if False %}{% pixels %}{% endif %}` also passes and is not text-fixable; document it under "Verifying on a storefront".
+**Why:** `pixels` is now the only fleet-scoped rule, so this is the whole fleet sweep's blind spot. Surfaced by the adversarial pass on the contract-scope PR, 2026-09-21.
+
+### Theme contract: harden the live check's transport
+**Priority:** P2
+**Effort:** S
+**What:** `read_remote_sources` uses the default `urllib` opener, so a 30x from the store forwards the `Authorization: Bearer` header to the redirect target, and `--store` accepts any URL scheme. Assert `https://` and install a non-following redirect handler. Also confirm the templates endpoint is unpaginated at the largest theme size (the checker reads `results` and never follows `next`), and quote remote template names in violation lines so a name containing a newline cannot fabricate a second `- [` line.
+**Why:** Pre-existing, but the unattended fleet sweep runs this against every store twice a week with a real admin key.
+
+### Fleet sweep: pass `--scope fleet` explicitly and record the applied count (next-mind)
+**Priority:** P2
+**Effort:** S
+**What:** `next-mind/scripts/spark_fleet_check.py` invokes the checker with no `--scope`, discards stdout (where the "N of M requirement(s)" line goes), and turns any non-violation exit-1 (scope refusal, traceback on a malformed API entry) into a `failing_active` row. Pass `--scope fleet`, capture the applied count into the fleet JSON and ledger, and distinguish infrastructure errors from violations (a separate exit code from the checker would help).
+**Why:** The sweep now depends on an implicit default that this repo changed under it; the ledger should say which rules ran.
+
 ### Preview mode placeholder suppression
 **Priority:** P2
 **Effort:** S (once platform variable identified)

docs/theme-contract.md

@@ -10,30 +10,59 @@ asserts them against a theme.
 
 ## What is in the contract
 
-| id | file | must contain | since |
-|---|---|---|---|
-| `pixels` | `layouts/base.html` | `{% pixels %}` | 1.3.0 |
+| id | scope | file | must contain | since |
+|---|---|---|---|---|
+| `pixels` | fleet | `layouts/base.html` | `{% pixels %}` | 1.3.0 |
+| `cart-badge` | spark | `partials/header.html` | `id="cart-badge"` | 1.1.0 |
+| `mobile-nav-toggle` | spark | `partials/header.html` | `data-toggle="mobile-nav"` | 1.0.0 |
+| `mobile-nav` | spark | `partials/mobile_menu.html` | `id="mobile-nav"` | 1.0.0 |
+| `cart-drawer` | spark | `partials/side_cart.html` | `</spark-cart-drawer>` | 1.1.1 |
 
 Each requirement carries a `why`, which the gate prints on failure. Whoever trips
 it is usually not the person who knows what the tag does.
 
+## Two scopes
+
+Every requirement names who it binds.
+
+- **`fleet`** — every Spark-derived store theme. This is the contract in the sense
+  above: a platform integration point whose absence is invisible on the storefront.
+  The live check enforces only these by default, and so does the fleet sweep that
+  runs against every store. Today that is `pixels` alone.
+- **`spark`** — Spark's own working copy. The runtime hooks `theme.js` resolves by
+  id or attribute belong here. Removing one from Spark passes every other gate and
+  fails only in the browser, so CI asserts them — but a derived theme is a fork, not
+  an install behind on a version. It may carry the same hook in a different file or
+  render the surface another way, and a file-location rule against the fleet reports
+  forks, not faults. Measured 2026-09-21: four live stores served `#mobile-nav` without
+  a `partials/mobile_menu.html`.
+
+A working copy is checked at the `spark` scope by default (it is Spark's CI gate,
+and a fork's author can opt down with `--scope fleet`). A live theme is checked at the
+`fleet` scope by default; `--scope spark` opts a live theme into the full set. The
+gate's output names the scope and how many requirements ran, so a pass is never
+mistaken for a pass against rules that were not applied.
+
 ## Checking a theme
 
 A working copy, before you push it:
 
 ```bash
-python3 scripts/check-theme-contract.py --root path/to/theme
+python3 scripts/check-theme-contract.py --root path/to/theme                # Spark's own copy: all rules
+python3 scripts/check-theme-contract.py --root path/to/fork --scope fleet   # a fork: fleet rules only
 ```
 
 A live theme, which is the check that matters:
 
 ```bash
 NTK_APIKEY=<store key> python3 scripts/check-theme-contract.py \
-    --store https://<store>.29next.store --theme-id <id>
+    --store https://<store>.29next.store --theme-id <id>                # a derived theme: fleet rules
+NTK_APIKEY=<store key> python3 scripts/check-theme-contract.py \
+    --store https://<dev-store>.29next.store --theme-id <id> --scope spark   # Spark's own copy on a dev store
 ```
 
-Spark's own copy runs in CI and through `make verify-theme`. Point it at another
-theme with `make contract THEME_ARGS="--root ../my-theme"`.
+Spark's own copy runs in CI and through `make verify-theme`. Point it at a fork
+with `make contract THEME_ARGS="--root ../my-theme --scope fleet"`.
 
 ## Why the live check is the one that matters
 
@@ -48,8 +77,11 @@ the block is a regression waiting for the next promote.
 
 ## Adding a requirement
 
-Add an entry to `theme-contract.json` with `id`, `file`, `must_contain`, and a
-`why` written for someone who has not read this repo. Optional fields:
+Add an entry to `theme-contract.json` with `id`, `scope`, `file`, `must_contain`,
+and a `why` written for someone who has not read this repo. `scope` is `fleet` or
+`spark` and is required: a rule that does not say who it binds is a load error, not
+a default. Promote a rule to `fleet` only when the platform, not Spark's own JS,
+depends on it — that is what makes its absence invisible on a fork. Optional fields:
 
 - `block` — the name of the base-layout block the tag lives in. The gate then
   also fails a child template that overrides that block without the tag, which a

scripts/check-theme-contract.py

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-"""Assert a theme still carries the platform integration points Spark declares.
+"""Assert a theme still carries the integration points Spark declares.
 
 Spark-derived store themes never update from this repo, so a fix that lands
 here does not reach them. The failures this gate targets are silent: the
@@ -13,6 +13,14 @@
 
 The remote mode is the one that matters. A store carries several theme copies,
 and republishing an old one silently undoes a patch applied to the active theme.
+
+Every requirement carries a scope. `fleet` binds every Spark-derived theme and
+is what the live check enforces by default; `spark` binds Spark's own working
+copy only, because derived themes are forks that may carry the same hook in a
+different file. Local mode checks both by default (it is Spark's CI gate);
+`--scope` overrides either default. A live check of Spark's own copy (a dev
+store, not a fork) therefore needs `--scope spark` to run the same rules the
+working-copy check did.
 """
 
 import argparse
@@ -32,6 +40,12 @@
 DEFAULT_CONTRACT = Path(__file__).resolve().parents[1] / CONTRACT_FILENAME
 TEMPLATE_DIRECTORIES = ("layouts", "templates", "partials")
 REQUEST_TIMEOUT = 30
+# A fleet requirement binds every Spark-derived theme; a spark requirement binds
+# only Spark's own working copy. Nothing else is a valid scope, and a requirement
+# without one is a load error rather than a silent default.
+SCOPE_FLEET = "fleet"
+SCOPE_SPARK = "spark"
+SCOPES = (SCOPE_FLEET, SCOPE_SPARK)
 
 
 def load_masking():
@@ -57,16 +71,34 @@ def load_contract(path):
         raise ValueError(f"{path}: 'requirements' must be a non-empty list")
 
     for requirement in requirements:
-        for field in ("id", "file", "must_contain", "why"):
+        for field in ("id", "file", "must_contain", "why", "scope"):
             if not requirement.get(field):
                 raise ValueError(
                     f"{path}: requirement {requirement.get('id', '?')!r} "
                     f"is missing {field!r}"
                 )
+        if requirement["scope"] not in SCOPES:
+            raise ValueError(
+                f"{path}: requirement {requirement['id']!r} has scope "
+                f"{requirement['scope']!r}; expected one of {', '.join(SCOPES)}"
+            )
 
     return contract
 
 
+def select_requirements(contract, scope):
+    """Return the requirements a check at `scope` enforces.
+
+    `fleet` keeps only fleet requirements. `spark` keeps everything: Spark's own
+    copy must satisfy the fleet rules too, since it is what the fleet derives from.
+    """
+    if scope not in SCOPES:
+        raise ValueError(f"unknown scope {scope!r}; expected one of {', '.join(SCOPES)}")
+    if scope == SCOPE_SPARK:
+        return list(contract["requirements"])
+    return [r for r in contract["requirements"] if r["scope"] == SCOPE_FLEET]
+
+
 def block_override_re(block_name):
     # A child template may override the block and drop the tag inside it. The
     # tag is then present in the base layout and absent from every rendered
@@ -79,11 +111,11 @@ def block_override_re(block_name):
     )
 
 
-def check_sources(sources, contract, mask):
-    """Check {path: text} against the contract. Returns a list of failures."""
+def check_sources(sources, requirements, mask):
+    """Check {path: text} against the requirements. Returns a list of failures."""
     failures = []
 
-    for requirement in contract["requirements"]:
+    for requirement in requirements:
         target = requirement["file"]
         needle = requirement["must_contain"]
         text = sources.get(target)
@@ -155,13 +187,14 @@ def read_remote_sources(store, theme_id, apikey):
     return sources
 
 
-def report(failures, subject):
+def report(failures, subject, scope, checked, total):
+    applied = f"{scope} scope, {checked} of {total} requirement(s)"
     if not failures:
-        print(f"Theme contract gate passed: {subject}.")
+        print(f"Theme contract gate passed: {subject} ({applied}).")
         return 0
 
     print(
-        f"Theme contract gate failed for {subject} "
+        f"Theme contract gate failed for {subject} ({applied}) "
         f"with {len(failures)} violation(s):",
         file=sys.stderr,
     )
@@ -198,6 +231,16 @@ def parse_args(argv):
         default=os.environ.get("NTK_APIKEY"),
         help="store API key (default: $NTK_APIKEY)",
     )
+    parser.add_argument(
+        "--scope",
+        choices=SCOPES,
+        default=None,
+        help=(
+            "which requirements to enforce: 'fleet' (every Spark-derived theme) "
+            "or 'spark' (Spark's own copy: fleet rules plus its runtime hooks). "
+            "Default: 'fleet' for a live theme, 'spark' for a working copy."
+        ),
+    )
     return parser.parse_args(argv)
 
 
@@ -213,6 +256,18 @@ def main(argv=None):
         return 1
 
     remote = bool(args.store or args.theme_id)
+    # A live theme is a derived copy unless the caller says otherwise, so the
+    # live default is the fleet scope; the working-copy default is Spark's own
+    # gate. `--scope` overrides either way.
+    scope = args.scope or (SCOPE_FLEET if remote else SCOPE_SPARK)
+    requirements = select_requirements(contract, scope)
+    if not requirements:
+        print(
+            f"Theme contract gate failed: no requirement carries scope "
+            f"{scope!r}, so nothing would be checked.",
+            file=sys.stderr,
+        )
+        return 1
     if remote:
         if not (args.store and args.theme_id and args.apikey):
             print(
@@ -248,7 +303,13 @@ def main(argv=None):
         )
         return 1
 
-    return report(check_sources(sources, contract, load_masking()), subject)
+    return report(
+        check_sources(sources, requirements, load_masking()),
+        subject,
+        scope,
+        len(requirements),
+        len(contract["requirements"]),
+    )
 
 
 if __name__ == "__main__":