From 82739eeaaaee4b2b4200b5b2242b740bcf5a8317 Mon Sep 17 00:00:00 2001 From: Devin Michael <110886466+next-devin@users.noreply.github.com> Date: Mon, 21 Sep 2026 22:21:48 +0700 Subject: [PATCH 1/3] feat: scope theme-contract requirements to the fleet or to Spark's own copy Every requirement in theme-contract.json now carries a required `scope`. `fleet` binds every Spark-derived store theme; `spark` binds Spark's own working copy only. check-theme-contract.py validates the field, enforces only fleet rules by default for a live theme (--store/--theme-id) and all rules by default for a working copy (--root), and takes --scope to override either. The gate's output names the scope and how many requirements ran, and a scope that selects nothing refuses rather than passing. `pixels` is the only fleet rule. The four runtime hooks #66 added (cart-badge, mobile-nav-toggle, mobile-nav, cart-drawer) are scoped to Spark: the first fleet sweep reported nine of thirteen live themes failing mobile-nav on the file rule while four of them served #mobile-nav from another file. Derived themes are forks; a file-location rule against the fleet reports forks, not faults. Tests cover both defaults, both overrides, the load errors, the empty selection, the block-override rule at fleet scope, and the exact invocation the next-mind fleet sweep makes, against a stub admin API that rejects the wrong path or key. --- scripts/check-theme-contract.py | 79 ++++++- tests/test_theme_contract.py | 364 +++++++++++++++++++++++++++++++- theme-contract.json | 7 +- 3 files changed, 436 insertions(+), 14 deletions(-) diff --git a/scripts/check-theme-contract.py b/scripts/check-theme-contract.py index 883de6a..93fdc51 100755 --- a/scripts/check-theme-contract.py +++ b/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__": diff --git a/tests/test_theme_contract.py b/tests/test_theme_contract.py index 1c77446..4ae7d3a 100644 --- a/tests/test_theme_contract.py +++ b/tests/test_theme_contract.py @@ -1,7 +1,10 @@ +import contextlib +import http.server import json import subprocess import sys import tempfile +import threading import unittest from pathlib import Path @@ -10,6 +13,8 @@ SCRIPTS = ROOT / "scripts" CONTRACT = ROOT / "theme-contract.json" +BASE_WITHOUT_PIXELS = "{% block scripts %}{% endblock %}" + BASE_WITH_PIXELS = """ {% block side_cart %}{% include 'partials/side_cart.html' %}{% endblock side_cart %} @@ -22,13 +27,14 @@ """ -def run_checker(*arguments, cwd=ROOT): +def run_checker(*arguments, cwd=ROOT, env=None): return subprocess.run( [sys.executable, str(SCRIPTS / "check-theme-contract.py"), *map(str, arguments)], cwd=cwd, capture_output=True, text=True, check=False, + env=env, ) @@ -82,9 +88,358 @@ def test_every_requirement_carries_an_explanation(self): contract = json.loads(CONTRACT.read_text(encoding="utf-8")) for requirement in contract["requirements"]: - for field in ("id", "file", "must_contain", "why"): + for field in ("id", "file", "must_contain", "why", "scope"): self.assertTrue(requirement.get(field), requirement) + def test_only_pixels_binds_the_fleet(self): + # Decided 2026-09-21: the runtime hooks are Spark's own gate. Derived + # themes are forks that carry the same hook in a different file (four + # live stores served #mobile-nav without partials/mobile_menu.html), so + # a file-location rule against the fleet reports forks, not faults. + contract = json.loads(CONTRACT.read_text(encoding="utf-8")) + by_scope = {} + for requirement in contract["requirements"]: + by_scope.setdefault(requirement["scope"], set()).add(requirement["id"]) + + self.assertEqual(by_scope["fleet"], {"pixels"}) + self.assertEqual( + by_scope["spark"], + {"cart-badge", "mobile-nav-toggle", "mobile-nav", "cart-drawer"}, + ) + + def test_docs_table_lists_every_requirement_with_its_scope(self): + # docs/theme-contract.md is where a fork author learns which rules bind + # them; a rule added to the JSON without a row there is undocumented. + contract = json.loads(CONTRACT.read_text(encoding="utf-8")) + docs = (ROOT / "docs" / "theme-contract.md").read_text(encoding="utf-8") + + for requirement in contract["requirements"]: + row = f"| `{requirement['id']}` | {requirement['scope']} | `{requirement['file']}` |" + self.assertIn(row, docs, requirement["id"]) + + +def write_contract(root, requirements): + path = root / "contract.json" + path.write_text(json.dumps({"requirements": requirements}), encoding="utf-8") + return path + + +def requirement(**overrides): + """A well-formed fleet requirement; a test overrides only what it varies.""" + return { + "id": "pixels", + "file": "layouts/base.html", + "must_contain": "{% pixels %}", + "why": "events", + "scope": "fleet", + **overrides, + } + + +STUB_THEME_ID = "34" +STUB_APIKEY = "k" + + +@contextlib.contextmanager +def serve_theme(templates): + """Stand up a stub store admin API serving one theme's templates. + + Yields the store's base URL and shuts the server down on exit. Like the + real API it serves /api/admin/themes//templates/ and ignores the query + string; unlike a permissive stub it answers 404 to any other path and 401 + without the Bearer key, so a regression in the checker's URL or header + surfaces as "could not read theme" rather than a pass. + """ + payload = json.dumps( + [{"name": name, "content": content} for name, content in templates.items()] + ).encode("utf-8") + templates_path = f"/api/admin/themes/{STUB_THEME_ID}/templates/" + + class Handler(http.server.BaseHTTPRequestHandler): + def do_GET(self): + if self.path.split("?", 1)[0] != templates_path: + self.send_error(404) + return + if self.headers.get("Authorization") != f"Bearer {STUB_APIKEY}": + self.send_error(401) + return + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(payload))) + self.end_headers() + self.wfile.write(payload) + + def log_message(self, *args): + pass + + server = http.server.HTTPServer(("127.0.0.1", 0), Handler) + # serve_forever polls at 0.5s by default and shutdown() waits for the next + # tick, which put half a second of idle wait on every remote-mode test. + threading.Thread( + target=server.serve_forever, kwargs={"poll_interval": 0.01}, daemon=True + ).start() + try: + yield f"http://127.0.0.1:{server.server_address[1]}" + finally: + server.shutdown() + server.server_close() + + +# A fork with the tracker block and nothing else Spark's runtime hooks expect: +# the mobile menu lives inline in the header, as measured on live stores. +FORK_WITHOUT_SPARK_HOOKS = { + "layouts/base.html": BASE_WITH_PIXELS, + "partials/header.html": "", +} + + +class ContractScopeTests(unittest.TestCase): + def test_requirement_without_scope_is_a_load_error(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + write_theme(root, BASE_WITH_PIXELS) + contract = write_contract(root, [ + {k: v for k, v in requirement().items() if k != "scope"}, + ]) + + result = run_checker("--root", root, "--contract", contract) + + self.assertEqual(result.returncode, 1) + self.assertIn("is missing 'scope'", result.stderr) + + def test_unknown_scope_is_a_load_error(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + write_theme(root, BASE_WITH_PIXELS) + contract = write_contract(root, [ + requirement(scope="everywhere"), + ]) + + result = run_checker("--root", root, "--contract", contract) + + self.assertEqual(result.returncode, 1) + self.assertIn("has scope 'everywhere'", result.stderr) + + def test_working_copy_defaults_to_the_spark_scope(self): + # Local mode is Spark's own CI gate, so every rule applies. + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + write_theme(root, BASE_WITH_PIXELS) + (root / "partials" / "mobile_menu.html").unlink() + + result = run_checker("--root", root, "--contract", CONTRACT) + + self.assertEqual(result.returncode, 1) + self.assertIn("[mobile-nav] partials/mobile_menu.html is missing", result.stderr) + self.assertIn("spark scope, 5 of 5 requirement(s)", result.stderr) + + def test_fleet_scope_on_a_working_copy_checks_only_fleet_rules(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + write_theme(root, BASE_WITH_PIXELS) + (root / "partials" / "mobile_menu.html").unlink() + + result = run_checker( + "--root", root, "--contract", CONTRACT, "--scope", "fleet" + ) + + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("fleet scope, 1 of 5 requirement(s)", result.stdout) + + def test_fleet_scope_still_fails_a_missing_fleet_rule(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + write_theme(root, BASE_WITHOUT_PIXELS) + + result = run_checker( + "--root", root, "--contract", CONTRACT, "--scope", "fleet" + ) + + self.assertEqual(result.returncode, 1) + self.assertIn("does not contain {% pixels %}", result.stderr) + + def test_live_theme_defaults_to_the_fleet_scope(self): + # The measured case: a derived theme with the tracker block and none of + # Spark's runtime-hook files. The live check must pass it. + with serve_theme(FORK_WITHOUT_SPARK_HOOKS) as base_url: + result = run_checker( + "--store", base_url, "--theme-id", STUB_THEME_ID, "--apikey", STUB_APIKEY, + "--contract", CONTRACT, + ) + + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("theme 34 on", result.stdout) + self.assertIn("fleet scope, 1 of 5 requirement(s)", result.stdout) + + def test_live_theme_can_opt_into_the_spark_scope(self): + with serve_theme(FORK_WITHOUT_SPARK_HOOKS) as base_url: + result = run_checker( + "--store", base_url, "--theme-id", STUB_THEME_ID, "--apikey", STUB_APIKEY, + "--contract", CONTRACT, "--scope", "spark", + ) + + self.assertEqual(result.returncode, 1) + self.assertIn("[mobile-nav] partials/mobile_menu.html is missing", result.stderr) + self.assertIn("[cart-drawer]", result.stderr) + + def test_live_theme_missing_the_fleet_rule_still_fails(self): + with serve_theme({ + "layouts/base.html": BASE_WITHOUT_PIXELS, + }) as base_url: + result = run_checker( + "--store", base_url, "--theme-id", STUB_THEME_ID, "--apikey", STUB_APIKEY, + "--contract", CONTRACT, + ) + + self.assertEqual(result.returncode, 1) + self.assertIn("[pixels] layouts/base.html does not contain {% pixels %}", result.stderr) + + def test_scope_with_no_requirements_fails_rather_than_passing(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + write_theme(root, BASE_WITH_PIXELS) + contract = write_contract(root, [ + requirement(scope="spark"), + ]) + + result = run_checker( + "--root", root, "--contract", contract, "--scope", "fleet" + ) + + self.assertEqual(result.returncode, 1) + self.assertIn("no requirement carries scope 'fleet'", result.stderr) + + def test_empty_scope_is_a_load_error_not_a_default(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + write_theme(root, BASE_WITH_PIXELS) + contract = write_contract(root, [ + requirement(scope=""), + ]) + + result = run_checker("--root", root, "--contract", contract) + + self.assertEqual(result.returncode, 1) + self.assertIn("is missing 'scope'", result.stderr) + + def test_unknown_scope_error_names_the_valid_scopes(self): + # Scopes are exact: "Fleet" is not "fleet", and the message must say + # what would have been accepted. + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + write_theme(root, BASE_WITH_PIXELS) + contract = write_contract(root, [ + requirement(scope="Fleet"), + ]) + + result = run_checker("--root", root, "--contract", contract) + + self.assertEqual(result.returncode, 1) + self.assertIn("has scope 'Fleet'; expected one of fleet, spark", result.stderr) + + def test_scope_flag_rejects_values_outside_the_two_scopes(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + write_theme(root, BASE_WITH_PIXELS) + + result = run_checker( + "--root", root, "--contract", CONTRACT, "--scope", "everywhere" + ) + + self.assertEqual(result.returncode, 2) + self.assertIn("invalid choice: 'everywhere'", result.stderr) + + def test_spark_scope_on_a_working_copy_matches_the_default(self): + # The override that restates the local default must behave identically. + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + write_theme(root, BASE_WITH_PIXELS) + (root / "partials" / "mobile_menu.html").unlink() + + result = run_checker( + "--root", root, "--contract", CONTRACT, "--scope", "spark" + ) + + self.assertEqual(result.returncode, 1) + self.assertIn("[mobile-nav] partials/mobile_menu.html is missing", result.stderr) + self.assertIn("spark scope, 5 of 5 requirement(s)", result.stderr) + + def test_fleet_scope_on_a_live_theme_matches_the_default(self): + with serve_theme(FORK_WITHOUT_SPARK_HOOKS) as base_url: + result = run_checker( + "--store", base_url, "--theme-id", STUB_THEME_ID, "--apikey", STUB_APIKEY, + "--contract", CONTRACT, "--scope", "fleet", + ) + + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("fleet scope, 1 of 5 requirement(s)", result.stdout) + + def test_fleet_sweep_invocation_uses_env_key_and_sparks_own_contract(self): + # The next-mind fleet sweep runs exactly this: --store and --theme-id, + # the key in $NTK_APIKEY, no --contract and no --scope. It must land on + # Spark's shipped contract at the fleet scope and pass a fork. + with serve_theme(FORK_WITHOUT_SPARK_HOOKS) as base_url: + result = run_checker( + "--store", base_url, "--theme-id", STUB_THEME_ID, + env={"NTK_APIKEY": STUB_APIKEY, "PATH": "/usr/bin:/bin"}, + ) + + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("fleet scope, 1 of 5 requirement(s)", result.stdout) + + def test_fleet_sweep_sees_only_fleet_violations_on_stderr(self): + # The sweep parses stderr lines starting "- [" as violations. A theme + # missing every Spark file must report the one fleet rule, not five. + with serve_theme({ + "layouts/base.html": BASE_WITHOUT_PIXELS, + }) as base_url: + result = run_checker( + "--store", base_url, "--theme-id", STUB_THEME_ID, "--apikey", STUB_APIKEY, + "--contract", CONTRACT, + ) + + violations = [ + line.strip() for line in result.stderr.splitlines() + if line.strip().startswith("- [") + ] + self.assertEqual(result.returncode, 1) + self.assertEqual(len(violations), 1, result.stderr) + self.assertTrue(violations[0].startswith("- [pixels] "), violations) + self.assertIn("(fleet scope, 1 of 5 requirement(s)) with 1 violation(s)", result.stderr) + self.assertNotIn("[cart-drawer]", result.stderr) + + def test_live_theme_child_override_dropping_pixels_fails_at_fleet_scope(self): + # The block-override rule rides on the requirement list, so it must + # still fire for the fleet rule when the spark rules are filtered out. + with serve_theme({ + "layouts/base.html": BASE_WITH_PIXELS, + "templates/index.html": "{% block pixels %}{% endblock pixels %}", + }) as base_url: + result = run_checker( + "--store", base_url, "--theme-id", STUB_THEME_ID, "--apikey", STUB_APIKEY, + "--contract", CONTRACT, + ) + + self.assertEqual(result.returncode, 1) + self.assertIn("[pixels] templates/index.html overrides block 'pixels'", result.stderr) + + def test_live_theme_with_no_fleet_rules_refuses_before_reading_the_store(self): + # Nothing listens on this port. If the scope check ran after the fetch, + # the failure would be a connection error instead. + with tempfile.TemporaryDirectory() as tmp: + contract = write_contract(Path(tmp), [ + requirement(scope="spark"), + ]) + + result = run_checker( + "--store", "http://127.0.0.1:9", "--theme-id", STUB_THEME_ID, + "--apikey", STUB_APIKEY, "--contract", contract, + ) + + self.assertEqual(result.returncode, 1) + self.assertIn("no requirement carries scope 'fleet'", result.stderr) + self.assertNotIn("could not read theme", result.stderr) + class ThemeContractGateTests(unittest.TestCase): def test_spark_itself_satisfies_its_own_contract(self): @@ -92,11 +447,12 @@ def test_spark_itself_satisfies_its_own_contract(self): self.assertEqual(result.returncode, 0, result.stderr) self.assertIn("Theme contract gate passed", result.stdout) + self.assertIn("spark scope, 5 of 5 requirement(s)", 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 %}") + write_theme(root, BASE_WITHOUT_PIXELS) result = run_checker("--root", root, "--contract", CONTRACT) @@ -181,7 +537,7 @@ def test_default_contract_is_sparks_own_not_the_checked_theme(self): # gate unusable in exactly the case it exists for. with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) - write_theme(root, "{% block scripts %}{% endblock %}") + write_theme(root, BASE_WITHOUT_PIXELS) result = run_checker("--root", root) diff --git a/theme-contract.json b/theme-contract.json index de0cc04..99dded7 100644 --- a/theme-contract.json +++ b/theme-contract.json @@ -1,9 +1,10 @@ { "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.", + "description": "Platform integration points and runtime hooks Spark declares. Each requirement carries a scope: 'fleet' binds every Spark-derived store theme and is what the live check and the fleet sweep enforce; 'spark' binds Spark's own working copy only, because derived themes are forks that may carry the same hook in a different file or render the surface differently. A derived theme renders correctly without a fleet requirement and fails silently, so those are asserted rather than left to review.", "requirements": [ { "id": "pixels", + "scope": "fleet", "file": "layouts/base.html", "must_contain": "{% pixels %}", "since": "1.3.0", @@ -13,6 +14,7 @@ }, { "id": "cart-badge", + "scope": "spark", "file": "partials/header.html", "must_contain": "id=\"cart-badge\"", "since": "1.1.0", @@ -21,6 +23,7 @@ }, { "id": "mobile-nav-toggle", + "scope": "spark", "file": "partials/header.html", "must_contain": "data-toggle=\"mobile-nav\"", "since": "1.0.0", @@ -29,6 +32,7 @@ }, { "id": "mobile-nav", + "scope": "spark", "file": "partials/mobile_menu.html", "must_contain": "id=\"mobile-nav\"", "since": "1.0.0", @@ -37,6 +41,7 @@ }, { "id": "cart-drawer", + "scope": "spark", "file": "partials/side_cart.html", "must_contain": "", "since": "1.1.1", From 0b6c37245cc11192f54a50183b571ee859412f44 Mon Sep 17 00:00:00 2001 From: Devin Michael <110886466+next-devin@users.noreply.github.com> Date: Mon, 21 Sep 2026 22:21:48 +0700 Subject: [PATCH 2/3] docs: document the two contract scopes and the invocation for each case theme-contract.md lists all five rules with their scope, explains why the runtime hooks bind Spark only, states the rule for promoting one to fleet, and shows the command for a fork's working copy, a derived live theme, and Spark's own copy on a dev store. The Makefile contract target's comment says the same. --- Makefile | 8 +++++-- docs/theme-contract.md | 50 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index fa35d77..68cebe3 100644 --- a/Makefile +++ b/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) diff --git a/docs/theme-contract.md b/docs/theme-contract.md index 2e4304f..75a3f59 100644 --- a/docs/theme-contract.md +++ b/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` | `` | 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= python3 scripts/check-theme-contract.py \ - --store https://.29next.store --theme-id + --store https://.29next.store --theme-id # a derived theme: fleet rules +NTK_APIKEY= python3 scripts/check-theme-contract.py \ + --store https://.29next.store --theme-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 From 291b8f40067d0f2d0d2303a7e0dc8eee92034762 Mon Sep 17 00:00:00 2001 From: Devin Michael <110886466+next-devin@users.noreply.github.com> Date: Mon, 21 Sep 2026 22:21:48 +0700 Subject: [PATCH 3/3] chore: changelog entry and follow-up TODOs for contract scoping Unreleased entry for the scope field. TODOs for the three findings the pre-landing adversarial pass surfaced and this change does not take on: HTML-comment masking in the needle search, live-check transport hardening, and the fleet sweep passing --scope fleet explicitly. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 4 ++++ TODOS.md | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21e83b8..bac44f8 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 + +- 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. diff --git a/TODOS.md b/TODOS.md index 60027f6..cd9b800 100644 --- a/TODOS.md +++ b/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 `` 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)