diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 667a2f8..d2c4809 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -118,3 +118,6 @@ jobs: - name: Template integrity gate run: python scripts/check-templates.py + + - name: Theme contract gate + run: python scripts/check-theme-contract.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a42a0ed..c8286ed 100644 --- a/CHANGELOG.md +++ b/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 + +- Added a theme contract so a Spark-derived store theme can prove it still carries the platform integration points Spark declares. `theme-contract.json` lists them (today: `{% pixels %}` in `layouts/base.html`, required since 1.3.0, with the reason the gate prints on failure) and `scripts/check-theme-contract.py` asserts them against a working copy (`--root`) or a live theme (`--store` + `--theme-id`, reading the store admin API). This exists because a derived theme never updates from this repo and the resulting failure is silent: the storefront renders, apps stay installed and enabled, and only the events go missing. A theme can also stop satisfying the contract without anyone editing it, when an older working copy is republished over the active theme, so the live mode matters as much as the local one. The gate reads more than the tag: it masks comments, so a commented-out tag does not satisfy the contract, and it fails a child template that overrides the block without the tag, which a plain text search would pass. `pixels` was also added to `REQUIRED_BASE_BLOCKS` in `scripts/check-templates.py`, so dropping the block from Spark itself fails CI. Run it with `make contract`; it is part of `make verify-theme` and runs in CI against Spark's own copy. + ## 1.4.1 - 2026-09-09 - Fixed custom Pages rendering empty titles, breadcrumbs, and content because the page template referenced `flatpage` instead of the platform-provided `page` object. This also restores page content in Account Only mode. diff --git a/Makefile b/Makefile index b7db538..fa35d77 100644 --- a/Makefile +++ b/Makefile @@ -6,7 +6,7 @@ COMPAT = python3 scripts/sass-compat.py TAILWIND_VERSION = v4.2.2 CSS_INPUT = css/input.css -.PHONY: dev build css css-drift css-check verify-theme test watch push release install-tailwind +.PHONY: dev build css css-drift css-check contract verify-theme test watch push release install-tailwind # Run both Tailwind watcher and ntk watcher in parallel dev: @@ -45,8 +45,14 @@ push: css-check test: python3 -m unittest discover -s tests +# Assert the platform integration points in theme-contract.json. +# Point it at a live theme before or after a push: +# make contract THEME_ARGS="--store https://x.29next.store --theme-id 68" +contract: + python3 scripts/check-theme-contract.py $(THEME_ARGS) + # Full pre-upload verification for generated theme artifacts. -verify-theme: css-check test +verify-theme: css-check test contract @echo "Theme verification complete." # Watch Tailwind only (useful when running ntk watch separately) diff --git a/docs/theme-contract.md b/docs/theme-contract.md new file mode 100644 index 0000000..2e4304f --- /dev/null +++ b/docs/theme-contract.md @@ -0,0 +1,73 @@ +# Theme contract + +A store theme derived from Spark never updates from this repo. When Spark gains a +required platform integration point, nothing tells the derived theme, and the +failure that follows is silent: the storefront renders, apps stay installed and +enabled in the dashboard, and only the events go missing. + +`theme-contract.json` declares those integration points. `scripts/check-theme-contract.py` +asserts them against a theme. + +## What is in the contract + +| id | file | must contain | since | +|---|---|---|---| +| `pixels` | `layouts/base.html` | `{% pixels %}` | 1.3.0 | + +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. + +## Checking a theme + +A working copy, before you push it: + +```bash +python3 scripts/check-theme-contract.py --root path/to/theme +``` + +A live theme, which is the check that matters: + +```bash +NTK_APIKEY= python3 scripts/check-theme-contract.py \ + --store https://.29next.store --theme-id +``` + +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"`. + +## Why the live check is the one that matters + +A store carries several theme copies. Republishing an old one silently undoes a +patch applied to the active theme, so a theme can satisfy the contract one week +and stop satisfying it the next without anyone editing it. Checking a working +copy proves what you are about to push; checking the live theme proves what the +store is actually serving. + +Run it against every theme on the store, not only the active one. A copy without +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: + +- `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 + plain text search would pass. +- `since` — the Spark version that introduced the requirement. +- `verify_on_storefront` — an expression to confirm the rendered result. + +Keep the contract small. It is for integration points whose absence is invisible, +not for style or structure. + +## Verifying on a storefront + +Check the published storefront, never the Theme Editor preview. The preview does +not render the tracker frames, so it shows this fault whether or not the theme +actually has it. + +```js +document.getElementsByName('customer_event_iframe').length > 0 +``` + +Read `/pixels/customer-events//` to see which app each frame belongs to. diff --git a/scripts/check-templates.py b/scripts/check-templates.py index f68ed00..bdbe895 100644 --- a/scripts/check-templates.py +++ b/scripts/check-templates.py @@ -50,6 +50,7 @@ "content_wrapper", "footer", "side_cart", + "pixels", "custom_css", "platform_compatibility", "preview_indicator", diff --git a/scripts/check-theme-contract.py b/scripts/check-theme-contract.py new file mode 100755 index 0000000..883de6a --- /dev/null +++ b/scripts/check-theme-contract.py @@ -0,0 +1,255 @@ +#!/usr/bin/env python3 +"""Assert a theme still carries the platform 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 +storefront renders, the dashboard shows the app installed and enabled, and only +the events go missing. Nothing surfaces it until someone looks. + +Two modes: + + check-theme-contract.py # a working copy, before push + check-theme-contract.py --store ... --theme-id N # a live theme, after it + +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. +""" + +import argparse +import importlib.util +import json +import os +import re +import sys +import urllib.error +import urllib.request +from pathlib import Path + + +CONTRACT_FILENAME = "theme-contract.json" +# The contract ships with Spark, beside this script. A derived theme being +# checked does not carry one, and in remote mode there is no local theme at all. +DEFAULT_CONTRACT = Path(__file__).resolve().parents[1] / CONTRACT_FILENAME +TEMPLATE_DIRECTORIES = ("layouts", "templates", "partials") +REQUEST_TIMEOUT = 30 + + +def load_masking(): + """Reuse check-templates.py's comment masking rather than restating it. + + The module name has a hyphen, so it cannot be imported normally. Masking + matters here: a required tag sitting inside {# ... #} is commented out and + must not satisfy the contract. + """ + path = Path(__file__).with_name("check-templates.py") + spec = importlib.util.spec_from_file_location("spark_check_templates", path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module.mask_ignored_regions + + +def load_contract(path): + with open(path, encoding="utf-8") as handle: + contract = json.load(handle) + + requirements = contract.get("requirements") + if not isinstance(requirements, list) or not requirements: + raise ValueError(f"{path}: 'requirements' must be a non-empty list") + + for requirement in requirements: + for field in ("id", "file", "must_contain", "why"): + if not requirement.get(field): + raise ValueError( + f"{path}: requirement {requirement.get('id', '?')!r} " + f"is missing {field!r}" + ) + + return contract + + +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 + # page, so the text search alone would pass a broken theme. + return re.compile( + r"{%\s*block\s+" + re.escape(block_name) + r"\s*%}" + r"(?P.*?)" + r"{%\s*endblock(?:\s+" + re.escape(block_name) + r")?\s*%}", + re.DOTALL, + ) + + +def check_sources(sources, contract, mask): + """Check {path: text} against the contract. Returns a list of failures.""" + failures = [] + + for requirement in contract["requirements"]: + target = requirement["file"] + needle = requirement["must_contain"] + text = sources.get(target) + + if text is None: + failures.append( + (requirement, f"{target} is missing from the theme") + ) + continue + + if needle not in mask(text): + failures.append( + (requirement, f"{target} does not contain {needle}") + ) + continue + + block_name = requirement.get("block") + if not block_name: + continue + + pattern = block_override_re(block_name) + for path, other in sorted(sources.items()): + if path == target: + continue + for match in pattern.finditer(mask(other)): + if needle not in match.group("body"): + failures.append( + ( + requirement, + f"{path} overrides block {block_name!r} without " + f"{needle}, which removes it from every page that " + "template renders", + ) + ) + + return failures + + +def read_local_sources(root): + sources = {} + for directory in TEMPLATE_DIRECTORIES: + for path in sorted((root / directory).rglob("*.html")): + if path.is_file(): + key = path.relative_to(root).as_posix() + sources[key] = path.read_text(encoding="utf-8") + return sources + + +def read_remote_sources(store, theme_id, apikey): + """Fetch a live theme's templates from the store admin API. + + The API's ?name= filter is ignored and returns the whole list, so the + filtering happens here. + """ + url = f"{store.rstrip('/')}/api/admin/themes/{theme_id}/templates/" + request = urllib.request.Request( + url, headers={"Authorization": f"Bearer {apikey}"} + ) + + with urllib.request.urlopen(request, timeout=REQUEST_TIMEOUT) as response: + payload = json.loads(response.read().decode("utf-8")) + + entries = payload if isinstance(payload, list) else payload.get("results", []) + sources = {} + for entry in entries: + name = entry.get("name", "") + if name.endswith(".html") and entry.get("content") is not None: + sources[name] = entry["content"] + return sources + + +def report(failures, subject): + if not failures: + print(f"Theme contract gate passed: {subject}.") + return 0 + + print( + f"Theme contract gate failed for {subject} " + f"with {len(failures)} violation(s):", + file=sys.stderr, + ) + for requirement, detail in failures: + print(f"\n- [{requirement['id']}] {detail}", file=sys.stderr) + print(f" Why it matters: {requirement['why']}", file=sys.stderr) + since = requirement.get("since") + if since: + print(f" Required since Spark {since}.", file=sys.stderr) + verify = requirement.get("verify_on_storefront") + if verify: + print( + " Confirm on the published storefront (never the Theme " + f"Editor preview): {verify}", + file=sys.stderr, + ) + return 1 + + +def parse_args(argv): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--root", default=".", help="theme directory to check (default: .)" + ) + parser.add_argument( + "--contract", + default=None, + help=f"contract file (default: Spark's own {CONTRACT_FILENAME})", + ) + parser.add_argument("--store", help="store URL, e.g. https://x.29next.store") + parser.add_argument("--theme-id", help="theme id to check on that store") + parser.add_argument( + "--apikey", + default=os.environ.get("NTK_APIKEY"), + help="store API key (default: $NTK_APIKEY)", + ) + return parser.parse_args(argv) + + +def main(argv=None): + args = parse_args(argv) + root = Path(args.root) + contract_path = Path(args.contract) if args.contract else DEFAULT_CONTRACT + + try: + contract = load_contract(contract_path) + except (OSError, ValueError, json.JSONDecodeError) as error: + print(f"Theme contract gate failed: {error}", file=sys.stderr) + return 1 + + remote = bool(args.store or args.theme_id) + if remote: + if not (args.store and args.theme_id and args.apikey): + print( + "Theme contract gate failed: --store, --theme-id and an API " + "key (--apikey or $NTK_APIKEY) are all required to check a " + "live theme.", + file=sys.stderr, + ) + return 1 + try: + sources = read_remote_sources(args.store, args.theme_id, args.apikey) + except (urllib.error.URLError, json.JSONDecodeError, OSError) as error: + print( + f"Theme contract gate failed: could not read theme " + f"{args.theme_id} from {args.store} ({error})", + file=sys.stderr, + ) + return 1 + subject = f"theme {args.theme_id} on {args.store}" + else: + try: + sources = read_local_sources(root) + except OSError as error: + print(f"Theme contract gate failed: {error}", file=sys.stderr) + return 1 + subject = f"{len(sources)} template file(s) under {root}" + + if not sources: + print( + "Theme contract gate failed: no template files were found, so " + "nothing was actually checked.", + file=sys.stderr, + ) + return 1 + + return report(check_sources(sources, contract, load_masking()), subject) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_theme_contract.py b/tests/test_theme_contract.py new file mode 100644 index 0000000..0a5dbbf --- /dev/null +++ b/tests/test_theme_contract.py @@ -0,0 +1,159 @@ +import json +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +SCRIPTS = ROOT / "scripts" +CONTRACT = ROOT / "theme-contract.json" + +BASE_WITH_PIXELS = """ + {% block side_cart %}{% include 'partials/side_cart.html' %}{% endblock side_cart %} + + {% block pixels %} + {% pixels %} + {% endblock pixels %} + + {% block scripts %}{% endblock scripts %} + +""" + + +def run_checker(*arguments, cwd=ROOT): + return subprocess.run( + [sys.executable, str(SCRIPTS / "check-theme-contract.py"), *map(str, arguments)], + cwd=cwd, + capture_output=True, + text=True, + check=False, + ) + + +def write_theme(root, base_html, extra=None): + """Lay out a minimal derived theme: base layout plus one of each directory.""" + (root / "layouts").mkdir(parents=True, exist_ok=True) + (root / "templates").mkdir(parents=True, exist_ok=True) + (root / "partials").mkdir(parents=True, exist_ok=True) + (root / "layouts" / "base.html").write_text(base_html, encoding="utf-8") + (root / "templates" / "index.html").write_text( + (extra or "

index

"), encoding="utf-8" + ) + (root / "partials" / "side_cart.html").write_text("
", encoding="utf-8") + + +class ContractFileTests(unittest.TestCase): + def test_contract_declares_the_pixels_requirement(self): + contract = json.loads(CONTRACT.read_text(encoding="utf-8")) + ids = {requirement["id"] for requirement in contract["requirements"]} + + self.assertIn("pixels", ids) + + def test_every_requirement_carries_an_explanation(self): + # The failure message is the whole point: whoever trips this gate is + # usually not the person who knows what the tag does. + contract = json.loads(CONTRACT.read_text(encoding="utf-8")) + + for requirement in contract["requirements"]: + for field in ("id", "file", "must_contain", "why"): + self.assertTrue(requirement.get(field), requirement) + + +class ThemeContractGateTests(unittest.TestCase): + def test_spark_itself_satisfies_its_own_contract(self): + result = run_checker() + + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("Theme contract gate passed", result.stdout) + + def test_missing_tag_fails_with_the_reason(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + write_theme(root, "{% block scripts %}{% endblock %}") + + result = run_checker("--root", root, "--contract", CONTRACT) + + self.assertEqual(result.returncode, 1) + self.assertIn("does not contain {% pixels %}", result.stderr) + self.assertIn("Why it matters", result.stderr) + + def test_commented_out_tag_does_not_satisfy_the_contract(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + write_theme(root, "{# {% pixels %} #}") + + result = run_checker("--root", root, "--contract", CONTRACT) + + self.assertEqual(result.returncode, 1) + self.assertIn("does not contain {% pixels %}", result.stderr) + + def test_child_override_that_drops_the_tag_fails(self): + # The tag is present in the base layout, so a text search passes while + # every page this template renders is missing the frames. + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + write_theme( + root, + BASE_WITH_PIXELS, + extra="{% block pixels %}{% endblock pixels %}", + ) + + result = run_checker("--root", root, "--contract", CONTRACT) + + self.assertEqual(result.returncode, 1) + self.assertIn("overrides block 'pixels'", result.stderr) + + def test_child_override_that_keeps_the_tag_passes(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + write_theme( + root, + BASE_WITH_PIXELS, + extra="{% block pixels %}{% pixels %}{% endblock pixels %}", + ) + + result = run_checker("--root", root, "--contract", CONTRACT) + + self.assertEqual(result.returncode, 0, result.stderr) + + def test_empty_theme_fails_rather_than_reporting_success(self): + # A gate that passes when it read nothing is worse than no gate. + with tempfile.TemporaryDirectory() as tmp: + result = run_checker("--root", tmp, "--contract", CONTRACT) + + self.assertEqual(result.returncode, 1) + self.assertIn("nothing was actually checked", result.stderr) + + def test_remote_mode_requires_credentials(self): + result = run_checker( + "--store", "https://example.29next.store", "--theme-id", "1", + "--apikey", "", + ) + + self.assertEqual(result.returncode, 1) + self.assertIn("required to check a live theme", result.stderr) + + def test_default_contract_is_sparks_own_not_the_checked_theme(self): + # A derived theme carries no contract of its own, and remote mode has + # no local theme at all. Defaulting to the theme directory made the + # gate unusable in exactly the case it exists for. + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + write_theme(root, "{% block scripts %}{% endblock %}") + + result = run_checker("--root", root) + + self.assertEqual(result.returncode, 1) + self.assertIn("does not contain {% pixels %}", result.stderr) + + def test_unreadable_contract_fails(self): + result = run_checker("--contract", ROOT / "does-not-exist.json") + + self.assertEqual(result.returncode, 1) + self.assertIn("Theme contract gate failed", result.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/theme-contract.json b/theme-contract.json new file mode 100644 index 0000000..6f06eef --- /dev/null +++ b/theme-contract.json @@ -0,0 +1,15 @@ +{ + "contract_version": 1, + "description": "Platform integration points every Spark-derived theme must keep. A derived theme renders correctly without these and fails silently, so they are asserted rather than left to review.", + "requirements": [ + { + "id": "pixels", + "file": "layouts/base.html", + "must_contain": "{% pixels %}", + "block": "pixels", + "since": "1.3.0", + "why": "App event trackers (GA4, Google Tag Manager, Klaviyo, Taboola) subscribe through the iframes this tag renders. Without it the storefront renders correctly, apps stay installed and enabled, and no storefront event is ever emitted. Checkout is unaffected, which is what keeps the fault invisible.", + "verify_on_storefront": "document.getElementsByName('customer_event_iframe').length > 0" + } + ] +}