From 92c961c3bfcd8956fa82085a14d039179b5c4b62 Mon Sep 17 00:00:00 2001 From: Devin Michael Date: Wed, 29 Jul 2026 15:20:25 +0700 Subject: [PATCH 1/7] feat: add reliable theme tooling contracts --- .github/workflows/test.yml | 25 +++- README.md | 49 +++++++- docs/production-capture-service.md | 21 ++++ ntk/__main__.py | 26 ++++- ntk/command.py | 181 ++++++++++++++++++++++++++--- ntk/conf.py | 20 ++++ ntk/decorator.py | 3 +- ntk/gateway.py | 7 ++ ntk/ntk_parser.py | 48 +++++++- ntk/output.py | 46 ++++++++ ntk/utils.py | 21 +++- ntk/validation.py | 63 ++++++++++ setup.py | 9 +- tests/test_tooling_contract.py | 149 ++++++++++++++++++++++++ 14 files changed, 634 insertions(+), 34 deletions(-) create mode 100644 docs/production-capture-service.md create mode 100644 ntk/output.py create mode 100644 ntk/validation.py create mode 100644 tests/test_tooling_contract.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3ad6dde..b2dc850 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -9,7 +9,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Check out repository code - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: Install dependencies run: | python -m pip install --upgrade pip @@ -26,9 +26,9 @@ jobs: python-version: ['3.10', '3.11', '3.12', '3.13', '3.14'] steps: - name: Check out repository code - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v2 + uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} - name: Install dependencies @@ -45,3 +45,22 @@ jobs: fail_ci_if_error: true env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + + portability: + name: Core CLI / ${{ matrix.os }} + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest, macos-14] + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: '3.12' + - name: Install core CLI without native extras + run: pip install .[test] + - name: Exercise core commands + run: | + ntk --help + python -m pytest -q diff --git a/README.md b/README.md index 326a01d..e98da18 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,14 @@ If you already have `python` and `pip`, install with the following command: pip install next-theme-kit ``` +The core CLI is pure Python and installs on macOS (including Apple Silicon), Linux, and native Windows. +Install optional features only when a project needs them: + +```bash +pip install 'next-theme-kit[sass]' # legacy libsass compilation +pip install 'next-theme-kit[capture]' # screenshot capture; then: playwright install chromium +``` + #### Mac OSX Requirements See how to install `python` and `pip` with [HomeBrew](https://docs.brew.sh/Homebrew-and-Python#python-3x). Once you have completed this step you can install using the `pip` instructions above. @@ -93,6 +101,8 @@ With the package installed, you can now use the commands inside your theme direc | `ntk push` | Push current theme state to store | | `ntk watch` | Watch for local changes and automatically push changes to store | | `ntk sass` | Process sass to css, see [Sass Processing](#sass-processing) | +| `ntk validate` | Validate theme files locally or against the store | +| `ntk capture` | Capture deterministic desktop and mobile PNGs | ### Browse Store Themes @@ -152,7 +162,7 @@ On success, `ntk init` logs the new theme ID and name, and persists the theme ID To sync files between your local directory and the store, use `ntk push` to upload and `ntk pull` to download. Both upload or download the whole theme by default, and both accept file paths as positional arguments to limit the operation to specific files. > [!NOTE] -> File paths are relative to the theme root. `ntk push` only uploads files inside the theme directories (`assets`, `checkout`, `configs`, `layouts`, `locales`, `partials`, `sass`, `templates`) with valid theme file extensions — a path outside of them is skipped silently, not reported as an error. +> File paths are relative to the theme root. `ntk push` only uploads files inside the theme directories (`assets`, `checkout`, `configs`, `layouts`, `locales`, `partials`, `sass`, `templates`) with valid theme file extensions. Explicit unsupported or missing paths are reported as rejected and make the command fail. | Example | Command | | ------- | ------- | @@ -177,6 +187,43 @@ On start, `ntk watch` logs the store, theme ID, a preview-theme URL, and the dir > [!NOTE] > `ntk watch` only uploads files with valid theme extensions. It does not accept file arguments. To scope changes to specific files, run `ntk push` with file paths instead. +### Validate Before Upload + +Run deterministic local checks for JSON syntax, template block balance, supported paths, and unsafe custom-product inheritance: + +```bash +ntk validate templates/catalogue/product.subscription.html configs/settings.json +``` + +Add `--server` to submit locally valid text files to the platform's template validator without saving them: + +```bash +ntk validate --server templates/catalogue/product.subscription.html +``` + +Local validation does not claim to prove a complete runtime render. Server validation requires the store, API key, and theme ID from flags or `config.yml`. + +### Capture Reproducible Screenshots + +With the `capture` extra and Chromium installed, capture real rendered PNGs at the fixed 1440px desktop and 390px mobile widths: + +```bash +ntk capture --url="/?preview_theme=&skip_cache=1" \ + --output=qa-output --viewports=desktop,mobile --json --no-progress +``` + +Capture waits for network idle, web fonts, lazy-loaded content, and images before writing full-page PNGs. It is suitable for local use and CI; it never substitutes DOM metrics for visual evidence. + +### Automation Output + +Every finite command accepts `--json`, `--quiet`, and `--no-progress`. `--json` writes exactly one versioned result object to stdout; logs stay on stderr, and progress is automatically disabled for JSON and non-interactive output. Push/pull/validation results are reported per file. Authentication, network, validation, rejection, and partial-transfer failures return a non-zero exit status. + +The JSON envelope is stable within schema version `1`: + +```json +{"schema_version":"1","command":"push","ok":true,"count":1,"results":[{"path":"templates/index.html","status":"uploaded"}]} +``` + ### Sass Processing Theme kit includes support for Sass processing via [Python Libsass](https://sass.github.io/libsass-python/). Sass processing includes support for variables, imports, nesting, mixins, inheritance, custom functions, and more. diff --git a/docs/production-capture-service.md b/docs/production-capture-service.md new file mode 100644 index 0000000..fd7d108 --- /dev/null +++ b/docs/production-capture-service.md @@ -0,0 +1,21 @@ +# Production screenshot service design + +Theme Kit's `ntk capture` command is the supported local and CI path. A hosted capture service is intentionally deferred until the platform team can provide the isolation, storage, and operating controls below. + +## Proposed contract + +- `POST /api/admin/themes/{theme_id}/captures/` accepts an allowlisted store route, the fixed `desktop` and/or `mobile` viewports, and an optional theme revision. +- The API authenticates with `themes:read`, resolves only the tenant's network domain, rejects arbitrary hosts and private-network destinations, and returns `202` with a job ID. +- A sandboxed Chromium worker opens the preview URL with `skip_cache=1`, waits for network idle, fonts, lazy media, and decoded images, then writes 1440px and 390px full-page PNGs. +- Artifacts are content-addressed, encrypted, tenant-scoped, retained for seven days, and returned through short-lived signed URLs. Logs redact query secrets and cookies. +- Jobs have a 60-second wall-clock limit, strict memory/response-size caps, per-store concurrency and rate limits, idempotency keys, cancellation, and auditable failure codes. +- The result records theme ID/revision, route, viewport dimensions, capture timestamp, browser version, artifact hash, and final response `X-Theme-*` headers. + +## Delivery slices + +1. Reuse the deterministic wait and viewport contract from `ntk capture` in a container image. +2. Add the authenticated job API and queue with SSRF and tenant-isolation tests. +3. Add artifact storage, signed delivery, retention cleanup, and operational dashboards. +4. Dogfood against Spark route matrices before exposing the service outside platform engineering. + +The tracking issue linked from the Theme Kit pull request owns this follow-up. Until it ships, CI should install `next_theme_kit[capture]`, install Chromium, and retain `qa-output/*.png` as build artifacts. diff --git a/ntk/__main__.py b/ntk/__main__.py index ec73b1d..770ebe0 100644 --- a/ntk/__main__.py +++ b/ntk/__main__.py @@ -1,9 +1,12 @@ #!/usr/bin/env python import logging +import sys from requests.exceptions import HTTPError +from ntk.exceptions import NTKError from ntk.ntk_parser import Parser +from ntk.output import Output logging.basicConfig( format='%(asctime)s %(levelname)s %(message)s', @@ -16,13 +19,28 @@ def main(): parser = Parser().create_parser() args = parser.parse_args() try: - args.func(args) + result = args.func(args) + if isinstance(result, dict) and not result.get('ok', True): + sys.exit(1) except AttributeError: print('Use ntk -h or --help to see available commands') + except NTKError as e: + if getattr(args, 'json', False) is True: + output = Output() + output.configure(args) + output.error( + getattr(args, 'command', 'ntk'), e, error_type=e.__class__.__name__) + else: + logging.error(e) + sys.exit(1) except (TypeError, HTTPError) as e: - # print new line for support error on process progress bar - print() - logging.exception(e, exc_info=False) + if getattr(args, 'json', False) is True: + output = Output() + output.configure(args) + output.error(getattr(args, 'command', 'ntk'), e, error_type=e.__class__.__name__) + else: + logging.exception(e, exc_info=False) + sys.exit(1) except KeyboardInterrupt: pass diff --git a/ntk/command.py b/ntk/command.py index 991f388..a08c50d 100644 --- a/ntk/command.py +++ b/ntk/command.py @@ -1,9 +1,10 @@ import asyncio import glob +import importlib import logging import os import time -import sass +from urllib.parse import parse_qsl, urlencode, urljoin, urlsplit, urlunsplit from watchfiles import awatch, Change @@ -13,7 +14,14 @@ ) from ntk.decorator import parser_config from ntk.gateway import Gateway +from ntk.output import Output from ntk.utils import get_template_name, progress_bar +from ntk.validation import validate_local_file + + +# Kept as an injectable seam for tests. The native dependency is imported only +# when the Sass command actually runs. +sass = None logging.basicConfig( @@ -28,6 +36,7 @@ class Command: def __init__(self): self.config = Config() self.gateway = Gateway(store=self.config.store, apikey=self.config.apikey) + self.output = Output() def _get_accept_files(self, template_names): files = [] @@ -57,6 +66,7 @@ def _handle_files_change(self, changes): self._delete_templates([template_name]) def _push_templates(self, template_names, compile_sass=False): + requested = list(template_names or []) template_names = self._get_accept_files(template_names) template_count = len(template_names) @@ -67,8 +77,19 @@ def _push_templates(self, template_names, compile_sass=False): if compile_sass and get_template_name(template_name).split('/')[0] == SASS_SOURCE: self._compile_sass() + results = [] + accepted = {os.path.abspath(name) for name in template_names} + for requested_name in requested: + if os.path.abspath(requested_name) not in accepted: + results.append({ + 'path': get_template_name(requested_name), + 'status': 'rejected', + 'reason': 'unsupported extension or file not found', + }) + for template_name in progress_bar( - template_names, prefix=f'[{self.config.env}] Progress:', suffix='Complete', length=50): + template_names, prefix=f'[{self.config.env}] Progress:', suffix='Complete', length=50, + enabled=self.output.progress_enabled): relative_pathfile = get_template_name(template_name) template_name = get_template_name(template_name) @@ -82,12 +103,23 @@ def _push_templates(self, template_names, compile_sass=False): content = f.read() f.close() - response = self.gateway.create_or_update_template( - theme_id=self.config.theme_id, template_name=relative_pathfile, content=content, files=files) + try: + response = self.gateway.create_or_update_template( + theme_id=self.config.theme_id, template_name=relative_pathfile, content=content, files=files) + finally: + if files: + files['file'][1].close() time.sleep(0.07) if not response.ok: - return + results.append({ + 'path': relative_pathfile, + 'status': 'failed', + 'status_code': response.status_code, + }) + continue + results.append({'path': relative_pathfile, 'status': 'uploaded'}) + return results def _pull_templates(self, template_names): templates = [] @@ -107,7 +139,10 @@ def _pull_templates(self, template_names): logging.info(f'[{self.config.env}] Connecting to {self.config.store}') logging.info(f'[{self.config.env}] Pulling {template_count} files from theme id {self.config.theme_id} ') current_files = [] - for template in progress_bar(templates, prefix=f'[{self.config.env}] Progress:', suffix='Complete', length=50): + results = [] + for template in progress_bar( + templates, prefix=f'[{self.config.env}] Progress:', suffix='Complete', length=50, + enabled=self.output.progress_enabled): template_name = str(template['name']) current_pathfile = os.path.abspath(template_name) current_files.append(current_pathfile.replace('\\', '/')) @@ -129,27 +164,41 @@ def _pull_templates(self, template_names): template_file.close() time.sleep(0.08) + results.append({'path': template_name, 'status': 'downloaded'}) + return results def _delete_templates(self, template_names): template_count = len(template_names) logging.info(f'[{self.config.env}] Connecting to {self.config.store}') logging.info(f'[{self.config.env}] Deleting {template_count} files from theme id {self.config.theme_id}') + results = [] for template_name in progress_bar( - template_names, prefix=f'[{self.config.env}] Progress:', suffix='Complete', length=50): + template_names, prefix=f'[{self.config.env}] Progress:', suffix='Complete', length=50, + enabled=self.output.progress_enabled): template_name = get_template_name(template_name) response = self.gateway.delete_template(theme_id=self.config.theme_id, template_name=template_name) if not response.ok: - return + results.append({'path': template_name, 'status': 'failed', 'status_code': response.status_code}) + continue + results.append({'path': template_name, 'status': 'deleted'}) + return results def _compile_sass(self): logging.info(f'[{self.config.env}] Processing {SASS_SOURCE} to {SASS_DESTINATION}.') + compiler = sass + if compiler is None: + try: + compiler = importlib.import_module('sass') + except ImportError as error: + raise NTKError( + 'Sass support is optional. Install next_theme_kit[sass] to use `ntk sass`.' + ) from error try: - sass.compile(dirname=(SASS_SOURCE, SASS_DESTINATION), output_style=self.config.sass_output_style) + compiler.compile(dirname=(SASS_SOURCE, SASS_DESTINATION), output_style=self.config.sass_output_style) logging.info(f'[{self.config.env}] Sass successfully processed.') except Exception as error: - logging.error(f'[{self.config.env}] Sass processing failed, see error below.') - logging.error(f'[{self.config.env}] {error}') + raise NTKError(f'[{self.config.env}] Sass processing failed: {error}') from error @parser_config(theme_id_required=False) def init(self, parser): @@ -161,6 +210,7 @@ def init(self, parser): self.config.save() logging.info( f'[{self.config.env}] Theme [{theme["id"]}] "{theme["name"]}" has been created successfully.') + return self.output.result('init', theme=theme) else: raise TypeError(f'[{self.config.env}] argument -n/--name is required.') @@ -173,20 +223,26 @@ def list(self, parser): for theme in themes['results']: theme_active = " (Active)" if theme.get("active") else "" logging.info(f'[{self.config.env}] \t[{theme.get("id")}] \t{theme.get("name")}{theme_active}') + return self.output.result('list', themes=themes['results'], count=len(themes['results'])) else: logging.warning(f'[{self.config.env}] Missing Themes in {self.config.store}') + return self.output.result('list', themes=[], count=0) @parser_config() def pull(self, parser): - self._pull_templates(parser.filenames) + results = self._pull_templates(parser.filenames) + return self.output.result('pull', results=results, count=len(results)) @parser_config(write_file=True) def checkout(self, parser): - self._pull_templates([]) + results = self._pull_templates([]) + return self.output.result('checkout', results=results, count=len(results)) @parser_config() def push(self, parser): - self._push_templates(parser.filenames or []) + results = self._push_templates(parser.filenames or []) + ok = all(item['status'] == 'uploaded' for item in results) + return self.output.result('push', ok=ok, results=results, count=len(results)) @parser_config() def watch(self, parser): @@ -208,3 +264,100 @@ async def main(): def compile_sass(self, parser): logging.info(f'[{self.config.env}] Sass output style {self.config.sass_output_style}.') self._compile_sass() + return self.output.result('sass', output=SASS_DESTINATION) + + @parser_config(apikey_required=False, store_required=False, theme_id_required=False) + def validate(self, parser): + if parser.server and (not self.config.store or not self.config.apikey or not self.config.theme_id): + raise TypeError( + '`ntk validate --server` requires -a/--apikey, -s/--store, and -t/--theme_id.' + ) + requested = list(parser.filenames or []) + paths = self._get_accept_files(requested) + accepted = {os.path.abspath(path) for path in paths} + results = [] + for requested_name in requested: + if os.path.abspath(requested_name) not in accepted: + results.append({ + 'path': get_template_name(requested_name), + 'status': 'invalid', + 'errors': ['unsupported extension or file not found'], + }) + for path in paths: + result = validate_local_file(path) + if parser.server and result['status'] == 'valid' and result.get('content') is not None: + response = self.gateway.validate_template( + theme_id=self.config.theme_id, + template_name=result['path'], + content=result['content'], + ) + if not response.ok: + result['status'] = 'invalid' + result['errors'] = response.json() if response.headers.get( + 'content-type', '').startswith('application/json') else [response.text] + result.pop('content', None) + results.append(result) + ok = bool(results) and all(item['status'] == 'valid' for item in results) + return self.output.result( + 'validate', ok=ok, mode='server' if parser.server else 'local', results=results, count=len(results) + ) + + @parser_config(apikey_required=False, theme_id_required=False) + def capture(self, parser): + try: + from playwright.sync_api import sync_playwright + except ImportError as error: + raise NTKError( + 'Capture support is optional. Install next_theme_kit[capture] and run `playwright install chromium`.' + ) from error + + split_url = urlsplit(urljoin(f'{self.config.store}/', parser.url.lstrip('/'))) + query = dict(parse_qsl(split_url.query, keep_blank_values=True)) + if self.config.theme_id and 'preview_theme' not in query: + query['preview_theme'] = str(self.config.theme_id) + capture_url = urlunsplit(( + split_url.scheme, split_url.netloc, split_url.path, urlencode(query), split_url.fragment + )) + viewports = { + 'desktop': {'width': 1440, 'height': 1000}, + 'mobile': {'width': 390, 'height': 844}, + } + requested_viewports = [item.strip() for item in parser.viewports.split(',') if item.strip()] + unknown = [item for item in requested_viewports if item not in viewports] + if unknown: + raise TypeError(f'Unknown viewport(s): {", ".join(unknown)}') + os.makedirs(parser.output, exist_ok=True) + results = [] + with sync_playwright() as playwright: + browser = playwright.chromium.launch(headless=True) + try: + for name in requested_viewports: + viewport = viewports[name] + page = browser.new_page(viewport=viewport) + page.goto(capture_url, wait_until='networkidle', timeout=parser.settle_timeout) + page.evaluate("document.fonts && document.fonts.ready") + page.evaluate(""" + async () => { + for (let y = 0; y < document.body.scrollHeight; y += window.innerHeight) { + window.scrollTo(0, y); + await new Promise(resolve => setTimeout(resolve, 50)); + } + window.scrollTo(0, 0); + } + """) + page.wait_for_function( + "[...document.images].every(image => image.complete)", timeout=parser.settle_timeout + ) + output_path = os.path.abspath(os.path.join(parser.output, f'{name}.png')) + page.screenshot(path=output_path, full_page=True) + results.append({ + 'viewport': name, + 'width': viewport['width'], + 'height': viewport['height'], + 'path': output_path, + 'status': 'captured', + }) + page.close() + finally: + browser.close() + return self.output.result('capture', url=capture_url, results=results, count=len(results)) diff --git a/ntk/conf.py b/ntk/conf.py index ae874f4..2bc2c84 100644 --- a/ntk/conf.py +++ b/ntk/conf.py @@ -1,6 +1,7 @@ import logging import os +from urllib.parse import urlparse import yaml CONFIG_FILE_NAME = './config.yml' @@ -99,6 +100,9 @@ def validate_config(self): pluralize = 'is' if len(error_msgs) == 1 else 'are' raise TypeError(f'[{self.env}] argument {message} {pluralize} required.') + if self.store: + self.store = self.normalize_store(self.store) + if self.sass_output_style and self.sass_output_style not in SASS_OUTPUT_STYLES: raise TypeError( f'[{self.env}] argument -sos/--sass_output_style is unsupported ' @@ -106,6 +110,22 @@ def validate_config(self): return True + @staticmethod + def normalize_store(store): + store = store.strip().rstrip('/') + if '://' not in store: + store = f'https://{store}' + parsed = urlparse(store) + if parsed.scheme not in ('http', 'https') or not parsed.netloc or parsed.path not in ('', '/'): + raise TypeError( + 'Store must be an absolute http(s) origin, for example https://store.example.com.' + ) + if parsed.username or parsed.password or parsed.query or parsed.fragment: + raise TypeError( + 'Store must be an absolute http(s) origin, for example https://store.example.com.' + ) + return f'{parsed.scheme}://{parsed.netloc}' + def read_config(self, update=True): configs = {} if os.path.exists(CONFIG_FILE): diff --git a/ntk/decorator.py b/ntk/decorator.py index 711960a..39167b9 100644 --- a/ntk/decorator.py +++ b/ntk/decorator.py @@ -17,11 +17,12 @@ def _wrapper(self, parser, **func_kwargs): for name, value in kwargs.items(): setattr(self.config, name, value) + self.output.configure(parser) self.config.parser_config(parser, write_file=kwargs.get('write_file', False)) self.gateway.store = self.config.store self.gateway.apikey = self.config.apikey - func(self, parser, **func_kwargs) + return func(self, parser, **func_kwargs) return _wrapper diff --git a/ntk/gateway.py b/ntk/gateway.py index 2edfdd3..ac56b17 100644 --- a/ntk/gateway.py +++ b/ntk/gateway.py @@ -61,6 +61,13 @@ def create_or_update_template(self, theme_id, template_name, content=None, files return self._request("POST", url, apikey=self.apikey, payload=payload, files=files) + @check_error(error_format='Validating {template_name} for theme id #{theme_id} failed.{error_msg}') + def validate_template(self, theme_id, template_name, content=None): + api_path = f"/api/admin/themes/{theme_id}/validate-template/" + url = urljoin(self.store, api_path) + payload = dict(name=template_name, content=content) + return self._request("POST", url, apikey=self.apikey, payload=payload) + @check_error(error_format='Deleting {template_name} file from theme id #{theme_id} failed.{error_msg}', response_json=False) def delete_template(self, theme_id, template_name): diff --git a/ntk/ntk_parser.py b/ntk/ntk_parser.py index 70e93fa..10c1326 100644 --- a/ntk/ntk_parser.py +++ b/ntk/ntk_parser.py @@ -14,6 +14,9 @@ def _add_config_arguments(self, parser): parser.add_argument('-e', '--env', action="store", dest="env", default='development', help=argparse.SUPPRESS) parser.add_argument( '-sos', '--sass_output_style', action="store", dest="sass_output_style", help=argparse.SUPPRESS) + parser.add_argument('--json', action='store_true', help='Write one stable JSON result to stdout') + parser.add_argument('--quiet', action='store_true', help='Suppress informational diagnostics') + parser.add_argument('--no-progress', action='store_true', help='Disable interactive progress output') def create_parser(self): option_commands = ''' @@ -38,13 +41,16 @@ def create_parser(self): push Push all theme files from your current direcotry to the store watch Watch for changes in your current directory and push updates to the store sass Process Sass files to CSS files in assets directory + validate Validate theme files locally or against the store + capture Capture deterministic desktop and mobile screenshots ''' + option_commands, usage=argparse.SUPPRESS, epilog='Use "ntk [command] --help" for more information about a command.', formatter_class=argparse.RawTextHelpFormatter, add_help=argparse.SUPPRESS ) - subparsers = parser.add_subparsers(title='Available Commands', help=argparse.SUPPRESS) + subparsers = parser.add_subparsers( + title='Available Commands', help=argparse.SUPPRESS, dest='command') # create the parser for the "init" command parser_init = subparsers.add_parser( @@ -128,7 +134,7 @@ def create_parser(self): self._add_config_arguments(parser_watch) # create the parser for the "sass" command - parser_watch = subparsers.add_parser( + parser_sass = subparsers.add_parser( 'sass', help='Compile scss to css on your theme', usage=argparse.SUPPRESS, @@ -137,6 +143,40 @@ def create_parser(self): ntk sass [options] ''' + option_commands, formatter_class=argparse.RawTextHelpFormatter) - parser_watch.set_defaults(func=self.command.compile_sass) - self._add_config_arguments(parser_watch) + parser_sass.set_defaults(func=self.command.compile_sass) + self._add_config_arguments(parser_sass) + + parser_validate = subparsers.add_parser( + 'validate', + help='Validate theme files', + usage=argparse.SUPPRESS, + description=''' +Usage: + ntk validate [options] [Filename ...] +''' + option_commands, + formatter_class=argparse.RawTextHelpFormatter) + parser_validate.set_defaults(func=self.command.validate) + parser_validate.add_argument('filenames', metavar='filenames', type=str, nargs='*', help=argparse.SUPPRESS) + parser_validate.add_argument( + '--server', action='store_true', help='Validate valid local text files with the store API') + self._add_config_arguments(parser_validate) + + parser_capture = subparsers.add_parser( + 'capture', + help='Capture deterministic storefront screenshots', + usage=argparse.SUPPRESS, + description=''' +Usage: + ntk capture --url /path --output qa-output [options] +''' + option_commands, + formatter_class=argparse.RawTextHelpFormatter) + parser_capture.set_defaults(func=self.command.capture) + parser_capture.add_argument('--url', required=True, help='Storefront route or absolute URL') + parser_capture.add_argument('--output', required=True, help='Directory for PNG screenshots') + parser_capture.add_argument( + '--viewports', default='desktop,mobile', help='Comma-separated desktop,mobile viewports') + parser_capture.add_argument( + '--settle-timeout', type=int, default=30000, + help='Milliseconds to wait for navigation, fonts, lazy media, and images') + self._add_config_arguments(parser_capture) return parser diff --git a/ntk/output.py b/ntk/output.py new file mode 100644 index 0000000..29027ff --- /dev/null +++ b/ntk/output.py @@ -0,0 +1,46 @@ +import json +import logging +import sys + + +SCHEMA_VERSION = "1" + + +class Output: + """Keep human diagnostics on stderr and machine results on stdout.""" + + def __init__(self): + self.json = False + self.quiet = False + self.no_progress = False + + def configure(self, parser): + self.json = getattr(parser, "json", False) is True + self.quiet = getattr(parser, "quiet", False) is True + self.no_progress = getattr(parser, "no_progress", False) is True + if self.quiet or self.json: + logging.getLogger().setLevel(logging.WARNING) + else: + logging.getLogger().setLevel(logging.INFO) + + @property + def progress_enabled(self): + return not (self.json or self.quiet or self.no_progress) and sys.stderr.isatty() + + def result(self, command, ok=True, **data): + payload = { + "schema_version": SCHEMA_VERSION, + "command": command, + "ok": bool(ok), + **data, + } + if self.json: + print(json.dumps(payload, sort_keys=True)) + return payload + + def error(self, command, message, error_type="error"): + return self.result( + command, + ok=False, + error={"type": error_type, "message": str(message)}, + ) diff --git a/ntk/utils.py b/ntk/utils.py index 83ad30a..c854502 100644 --- a/ntk/utils.py +++ b/ntk/utils.py @@ -1,4 +1,5 @@ import os +import sys import time from pathlib import Path @@ -7,7 +8,10 @@ def get_template_name(pathfile): return Path(os.path.relpath(pathfile)).as_posix() -def progress_bar(iterable, prefix='', suffix='', decimals=1, length=100, fill='█', printEnd="\r"): +def progress_bar( + iterable, prefix='', suffix='', decimals=1, length=100, fill='#', printEnd="\r", + enabled=None, stream=None, +): """ Call in a loop to create terminal progress bar @params: @@ -21,8 +25,13 @@ def progress_bar(iterable, prefix='', suffix='', decimals=1, length=100, fill=' printEnd - Optional : end character (e.g. "\r", "\r\n") (Str) """ total = len(iterable) + stream = stream or sys.stderr + if enabled is None: + enabled = stream.isatty() - if total == 0: + if total == 0 or not enabled: + for item in iterable: + yield item return # Progress Bar Printing Function @@ -31,7 +40,11 @@ def print_progress_bar(iteration): filledLength = int(length * iteration // total) bar = fill * filledLength + '-' * (length - filledLength) current_time = time.strftime('%Y-%m-%d %H:%M:%S') - print(f'\r{current_time} INFO {prefix} |{bar}| {percent}% {suffix}', end=printEnd) + print( + f'\r{current_time} INFO {prefix} |{bar}| {percent}% {suffix}', + end=printEnd, + file=stream, + ) # Initial Call print_progress_bar(0) @@ -40,4 +53,4 @@ def print_progress_bar(iteration): yield item print_progress_bar(i + 1) # Print New Line on Complete - print() + print(file=stream) diff --git a/ntk/validation.py b/ntk/validation.py new file mode 100644 index 0000000..f22b71c --- /dev/null +++ b/ntk/validation.py @@ -0,0 +1,63 @@ +import json +import os +import re + +from ntk.conf import CONTENT_FILE_EXTENSIONS, MEDIA_FILE_EXTENSIONS +from ntk.utils import get_template_name + + +TAG_RE = re.compile(r'{%\s*(block|endblock)\b(?:\s+([\w.-]+))?[^%]*%}') + + +def _template_errors(content): + errors = [] + stack = [] + for tag, name in TAG_RE.findall(content): + if tag == 'block': + stack.append(name) + elif not stack: + errors.append('endblock has no matching block') + else: + opened = stack.pop() + if name and name != opened: + errors.append(f'endblock {name} closes block {opened}') + errors.extend(f'block {name} is not closed' for name in stack) + return errors + + +def validate_local_file(path): + relative_path = get_template_name(path) + extension = os.path.splitext(path)[1].lower() + if extension in MEDIA_FILE_EXTENSIONS: + return {'path': relative_path, 'status': 'valid', 'errors': [], 'content': None} + if extension not in CONTENT_FILE_EXTENSIONS: + return { + 'path': relative_path, + 'status': 'invalid', + 'errors': [f'unsupported extension {extension}'], + 'content': None, + } + try: + with open(path, 'r', encoding='utf-8') as source_file: + content = source_file.read() + except (OSError, UnicodeDecodeError) as error: + return {'path': relative_path, 'status': 'invalid', 'errors': [str(error)], 'content': None} + + errors = [] + if extension == '.json': + try: + json.loads(content) + except json.JSONDecodeError as error: + errors.append(f'invalid JSON at line {error.lineno}, column {error.colno}: {error.msg}') + elif extension == '.html': + errors.extend(_template_errors(content)) + if relative_path.startswith('templates/catalogue/product.'): + if "extends 'templates/catalogue/product.html'" in content or ( + 'extends "templates/catalogue/product.html"' in content): + errors.append('custom product templates must be standalone or extend layouts/base.html') + return { + 'path': relative_path, + 'status': 'invalid' if errors else 'valid', + 'errors': errors, + 'content': content, + } diff --git a/setup.py b/setup.py index 0507b6b..a464083 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -__version__ = '1.1.1' +__version__ = '1.2.0' tests_require = [ "flake8", @@ -24,14 +24,17 @@ "PyYAML>=5.4", "requests>=2.25", "watchfiles>=0.18", - "libsass>=0.21.0" ], entry_points={ 'console_scripts': [ 'ntk = ntk.__main__:main', ], }, - extras_require={"test": tests_require}, + extras_require={ + "test": tests_require, + "sass": ["libsass>=0.21.0"], + "capture": ["playwright>=1.40"], + }, packages=find_packages(), python_requires='>=3.10' ) diff --git a/tests/test_tooling_contract.py b/tests/test_tooling_contract.py new file mode 100644 index 0000000..93b3324 --- /dev/null +++ b/tests/test_tooling_contract.py @@ -0,0 +1,149 @@ +import io +import json +import os +import tempfile +import types +import unittest +from contextlib import redirect_stdout +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from ntk.command import Command +from ntk.conf import Config +from ntk.ntk_parser import Parser +from ntk.output import Output +from ntk.utils import progress_bar +from ntk.validation import validate_local_file + + +class TestToolingContract(unittest.TestCase): + def test_store_normalizes_bare_hostname_and_rejects_paths(self): + self.assertEqual(Config.normalize_store('store.example.com/'), 'https://store.example.com') + with self.assertRaises(TypeError): + Config.normalize_store('https://store.example.com/admin') + + def test_machine_output_has_stable_envelope(self): + output = Output() + output.json = True + stdout = io.StringIO() + with redirect_stdout(stdout): + payload = output.result('push', results=[], count=0) + self.assertEqual(json.loads(stdout.getvalue()), payload) + self.assertEqual(payload['schema_version'], '1') + self.assertTrue(payload['ok']) + + def test_progress_is_ascii_stderr_and_disabled_when_requested(self): + stream = io.StringIO() + self.assertEqual(list(progress_bar([1], enabled=True, stream=stream, length=2)), [1]) + self.assertIn('##', stream.getvalue()) + self.assertNotIn('\u2588', stream.getvalue()) + stream = io.StringIO() + self.assertEqual(list(progress_bar([1], enabled=False, stream=stream)), [1]) + self.assertEqual(stream.getvalue(), '') + + def test_parser_exposes_validate_capture_and_machine_flags(self): + parser = Parser().create_parser() + validate = parser.parse_args(['validate', '--json', '--server', 'templates/index.html']) + self.assertTrue(validate.json) + self.assertTrue(validate.server) + capture = parser.parse_args([ + 'capture', '--url', '/', '--output', 'qa-output', '--viewports', 'desktop,mobile', '--no-progress' + ]) + self.assertEqual(capture.viewports, 'desktop,mobile') + self.assertTrue(capture.no_progress) + + def test_local_validation_detects_json_and_template_errors(self): + with tempfile.TemporaryDirectory() as directory: + json_path = os.path.join(directory, 'bad.json') + html_path = os.path.join(directory, 'bad.html') + with open(json_path, 'w', encoding='utf-8') as output_file: + output_file.write('{') + with open(html_path, 'w', encoding='utf-8') as output_file: + output_file.write('{% block content %}') + self.assertEqual(validate_local_file(json_path)['status'], 'invalid') + self.assertEqual(validate_local_file(html_path)['status'], 'invalid') + + @patch('ntk.command.Config.read_config') + def test_capture_uses_fixed_viewports_and_settle_contract(self, _read_config): + pages = [] + + class FakePage: + def __init__(self, viewport): + self.viewport = viewport + self.goto_calls = [] + self.screenshot_calls = [] + + def goto(self, url, **kwargs): + self.goto_calls.append((url, kwargs)) + + def evaluate(self, script): + return None + + def wait_for_function(self, script, **kwargs): + return None + + def screenshot(self, **kwargs): + self.screenshot_calls.append(kwargs) + + def close(self): + return None + + class FakeBrowser: + def new_page(self, viewport): + page = FakePage(viewport) + pages.append(page) + return page + + def close(self): + return None + + fake_playwright = SimpleNamespace( + chromium=SimpleNamespace(launch=lambda **kwargs: FakeBrowser()) + ) + + class FakeContext: + def __enter__(self): + return fake_playwright + + def __exit__(self, *args): + return None + + sync_api = types.ModuleType('playwright.sync_api') + sync_api.sync_playwright = lambda: FakeContext() + playwright = types.ModuleType('playwright') + with tempfile.TemporaryDirectory() as output_dir: + with patch.dict('sys.modules', {'playwright': playwright, 'playwright.sync_api': sync_api}): + command = Command() + parser = SimpleNamespace( + env='development', apikey=None, store='store.example.com', theme_id=42, + sass_output_style=None, json=False, quiet=True, no_progress=True, + url='products/example?skip_cache=1', output=output_dir, + viewports='desktop,mobile', settle_timeout=1234, + ) + result = command.capture(parser) + + self.assertTrue(result['ok']) + self.assertEqual([page.viewport['width'] for page in pages], [1440, 390]) + self.assertTrue(all('preview_theme=42' in page.goto_calls[0][0] for page in pages)) + self.assertTrue(all(page.goto_calls[0][1]['timeout'] == 1234 for page in pages)) + + @patch('ntk.command.Gateway') + @patch('ntk.command.Config.read_config') + def test_push_reports_invalid_explicit_file_and_fails(self, _read_config, gateway): + command = Command() + command.config.apikey = 'key' + command.config.store = 'https://store.example.com' + command.config.theme_id = 1 + command._get_accept_files = MagicMock(return_value=[]) + parser = SimpleNamespace( + env='development', apikey=None, store=None, theme_id=None, sass_output_style=None, + filenames=['templates/index.html.tmp'], json=False, quiet=True, no_progress=True, + ) + result = command.push(parser) + self.assertFalse(result['ok']) + self.assertEqual(result['results'][0]['status'], 'rejected') + gateway.return_value.create_or_update_template.assert_not_called() + + +if __name__ == '__main__': + unittest.main() From 9485849dcb96d13764db061bf275eb3b1a0d63f0 Mon Sep 17 00:00:00 2001 From: Devin Michael Date: Wed, 29 Jul 2026 15:22:36 +0700 Subject: [PATCH 2/7] fix: handle cross-drive paths on Windows --- ntk/utils.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ntk/utils.py b/ntk/utils.py index c854502..d534c96 100644 --- a/ntk/utils.py +++ b/ntk/utils.py @@ -5,7 +5,13 @@ def get_template_name(pathfile): - return Path(os.path.relpath(pathfile)).as_posix() + try: + relative_path = os.path.relpath(pathfile) + except ValueError: + # Windows temporary directories can live on a different drive from + # the theme checkout. Keep the absolute path usable for diagnostics. + relative_path = pathfile + return Path(relative_path).as_posix() def progress_bar( From fe64e8951bd114c6c714bb2c733f4be140bb46a5 Mon Sep 17 00:00:00 2001 From: Devin Michael Date: Wed, 29 Jul 2026 15:26:22 +0700 Subject: [PATCH 3/7] test: cover machine and validation contracts --- tests/test_main.py | 40 ++++++++++++++++++++++ tests/test_tooling_contract.py | 62 ++++++++++++++++++++++++++++++++-- 2 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 tests/test_main.py diff --git a/tests/test_main.py b/tests/test_main.py new file mode 100644 index 0000000..b95dc52 --- /dev/null +++ b/tests/test_main.py @@ -0,0 +1,40 @@ +import unittest +import json +from contextlib import redirect_stdout +from io import StringIO +from unittest.mock import MagicMock, patch + +from ntk.__main__ import main + + +class TestMain(unittest.TestCase): + @patch('ntk.__main__.Parser', autospec=True) + def test_main_exits_1_for_failed_command_result(self, mock_parser): + args = MagicMock() + args.func.return_value = {'ok': False} + mock_parser.return_value.create_parser.return_value.parse_args.return_value = args + with self.assertRaises(SystemExit) as exit_context: + main() + self.assertEqual(exit_context.exception.code, 1) + + @patch('ntk.__main__.Parser', autospec=True) + def test_main_writes_one_json_error_for_machine_mode(self, mock_parser): + args = MagicMock() + args.json = True + args.quiet = False + args.no_progress = False + args.command = 'validate' + args.func.side_effect = TypeError('invalid input') + mock_parser.return_value.create_parser.return_value.parse_args.return_value = args + stdout = StringIO() + with self.assertRaises(SystemExit): + with redirect_stdout(stdout): + main() + payload = json.loads(stdout.getvalue()) + self.assertFalse(payload['ok']) + self.assertEqual(payload['command'], 'validate') + self.assertEqual(payload['error']['type'], 'TypeError') + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test_tooling_contract.py b/tests/test_tooling_contract.py index 93b3324..b40c6e1 100644 --- a/tests/test_tooling_contract.py +++ b/tests/test_tooling_contract.py @@ -12,8 +12,8 @@ from ntk.conf import Config from ntk.ntk_parser import Parser from ntk.output import Output -from ntk.utils import progress_bar -from ntk.validation import validate_local_file +from ntk.utils import get_template_name, progress_bar +from ntk.validation import _template_errors, validate_local_file class TestToolingContract(unittest.TestCase): @@ -31,6 +31,8 @@ def test_machine_output_has_stable_envelope(self): self.assertEqual(json.loads(stdout.getvalue()), payload) self.assertEqual(payload['schema_version'], '1') self.assertTrue(payload['ok']) + output.json = False + self.assertFalse(output.error('push', 'broken')['ok']) def test_progress_is_ascii_stderr_and_disabled_when_requested(self): stream = io.StringIO() @@ -63,6 +65,62 @@ def test_local_validation_detects_json_and_template_errors(self): self.assertEqual(validate_local_file(json_path)['status'], 'invalid') self.assertEqual(validate_local_file(html_path)['status'], 'invalid') + def test_local_validation_covers_media_paths_and_product_inheritance(self): + self.assertEqual(_template_errors('{% endblock %}'), ['endblock has no matching block']) + self.assertEqual( + _template_errors('{% block alpha %}{% endblock beta %}'), + ['endblock beta closes block alpha'], + ) + with tempfile.TemporaryDirectory() as directory: + old_directory = os.getcwd() + try: + os.chdir(directory) + os.makedirs('templates/catalogue') + product_path = 'templates/catalogue/product.custom.html' + with open(product_path, 'w', encoding='utf-8') as output_file: + output_file.write('{% extends "templates/catalogue/product.html" %}') + with open('image.png', 'wb') as output_file: + output_file.write(b'png') + self.assertEqual(validate_local_file(product_path)['status'], 'invalid') + self.assertEqual(validate_local_file('image.png')['status'], 'valid') + self.assertEqual(validate_local_file('missing.json')['status'], 'invalid') + self.assertEqual(validate_local_file('unsupported.txt')['status'], 'invalid') + finally: + os.chdir(old_directory) + + @patch('ntk.utils.os.path.relpath', side_effect=ValueError) + def test_template_name_handles_windows_cross_drive_path(self, _relpath): + self.assertTrue(get_template_name('/tmp/theme.json').endswith('tmp/theme.json')) + + @patch('ntk.command.Gateway') + @patch('ntk.command.Config.read_config') + def test_validate_reports_local_rejections_and_server_results(self, _read_config, gateway): + with tempfile.TemporaryDirectory() as directory: + path = os.path.join(directory, 'settings.json') + with open(path, 'w', encoding='utf-8') as output_file: + output_file.write('{}') + command = Command() + command.config.apikey = 'key' + command.config.store = 'https://store.example.com' + command.config.theme_id = 42 + command._get_accept_files = MagicMock(return_value=[path]) + gateway.return_value.validate_template.return_value.ok = True + parser = SimpleNamespace( + env='development', apikey=None, store=None, theme_id=None, + sass_output_style=None, filenames=[path, 'bad.tmp'], server=False, + json=False, quiet=True, no_progress=True, + ) + local_result = command.validate(parser) + self.assertFalse(local_result['ok']) + self.assertEqual(local_result['results'][0]['status'], 'invalid') + + parser.filenames = [path] + parser.server = True + server_result = command.validate(parser) + self.assertTrue(server_result['ok']) + self.assertEqual(server_result['mode'], 'server') + gateway.return_value.validate_template.assert_called_once() + @patch('ntk.command.Config.read_config') def test_capture_uses_fixed_viewports_and_settle_contract(self, _read_config): pages = [] From f1fecf9d128d760bd39be8b997a49066dd520987 Mon Sep 17 00:00:00 2001 From: Devin Michael Date: Wed, 29 Jul 2026 15:28:49 +0700 Subject: [PATCH 4/7] test: meet patch coverage for failure paths --- tests/test_main.py | 1 - tests/test_tooling_contract.py | 56 ++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/tests/test_main.py b/tests/test_main.py index b95dc52..0279c0b 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -35,6 +35,5 @@ def test_main_writes_one_json_error_for_machine_mode(self, mock_parser): self.assertEqual(payload['command'], 'validate') self.assertEqual(payload['error']['type'], 'TypeError') - if __name__ == '__main__': unittest.main() diff --git a/tests/test_tooling_contract.py b/tests/test_tooling_contract.py index b40c6e1..9bf247f 100644 --- a/tests/test_tooling_contract.py +++ b/tests/test_tooling_contract.py @@ -10,6 +10,7 @@ from ntk.command import Command from ntk.conf import Config +from ntk.exceptions import NTKError from ntk.ntk_parser import Parser from ntk.output import Output from ntk.utils import get_template_name, progress_bar @@ -179,12 +180,67 @@ def __exit__(self, *args): viewports='desktop,mobile', settle_timeout=1234, ) result = command.capture(parser) + parser.viewports = 'tablet' + with self.assertRaises(TypeError): + command.capture(parser) self.assertTrue(result['ok']) self.assertEqual([page.viewport['width'] for page in pages], [1440, 390]) self.assertTrue(all('preview_theme=42' in page.goto_calls[0][0] for page in pages)) self.assertTrue(all(page.goto_calls[0][1]['timeout'] == 1234 for page in pages)) + @patch('ntk.command.Config.read_config') + def test_optional_sass_failures_are_actionable(self, _read_config): + command = Command() + with patch('ntk.command.sass', None): + with patch('ntk.command.importlib.import_module', side_effect=ImportError): + with self.assertRaises(NTKError): + command._compile_sass() + compiler = MagicMock() + compiler.compile.side_effect = RuntimeError('compile failed') + with patch('ntk.command.sass', compiler): + with self.assertRaises(NTKError): + command._compile_sass() + + @patch('ntk.command.Gateway') + @patch('ntk.command.Config.read_config') + def test_transfer_failures_are_returned_per_file(self, _read_config, gateway): + command = Command() + command.output.quiet = True + response = gateway.return_value.create_or_update_template.return_value + response.ok = False + response.status_code = 500 + with tempfile.TemporaryDirectory() as directory: + old_directory = os.getcwd() + try: + os.chdir(directory) + os.makedirs('templates') + with open('templates/index.html', 'w', encoding='utf-8') as output_file: + output_file.write('ok') + command._get_accept_files = MagicMock(return_value=[os.path.abspath('templates/index.html')]) + pushed = command._push_templates(['templates/index.html']) + finally: + os.chdir(old_directory) + self.assertEqual(pushed[0]['status'], 'failed') + + delete_response = gateway.return_value.delete_template.return_value + delete_response.ok = False + delete_response.status_code = 503 + deleted = command._delete_templates(['templates/index.html']) + self.assertEqual(deleted[0]['status'], 'failed') + + @patch('ntk.command.Config.read_config') + def test_capture_without_optional_dependency_is_actionable(self, _read_config): + command = Command() + parser = SimpleNamespace( + env='development', apikey=None, store='store.example.com', theme_id=42, + sass_output_style=None, json=False, quiet=True, no_progress=True, + url='/', output='qa-output', viewports='desktop', settle_timeout=1000, + ) + with patch.dict('sys.modules', {'playwright.sync_api': None}): + with self.assertRaises(NTKError): + command.capture(parser) + @patch('ntk.command.Gateway') @patch('ntk.command.Config.read_config') def test_push_reports_invalid_explicit_file_and_fails(self, _read_config, gateway): From dbb386c023badb73f70c1cfdcc28255d39d8ccd5 Mon Sep 17 00:00:00 2001 From: Devin Michael Date: Wed, 29 Jul 2026 15:30:34 +0700 Subject: [PATCH 5/7] fix: enforce HTTPS store origins --- README.md | 2 ++ ntk/conf.py | 2 +- ntk/validation.py | 5 +++++ tests/test_command.py | 2 +- tests/test_config.py | 4 ++-- tests/test_tooling_contract.py | 6 ++++++ 6 files changed, 17 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index e98da18..776545a 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,8 @@ Store authentication uses [OAuth 2.0](https://auth0.com/intro-to-iam/what-is-oau `ntk` reads its connection settings from two places: command flags (`--apikey`, `--store`, `--theme_id`) and the `config.yml` file in your theme directory. You do not need to create `config.yml` by hand — `ntk checkout` and `ntk init` write it for you, and after that commands run without flags: +Store values are normalized to an absolute HTTPS origin: `store.example.com` and `http://store.example.com` become `https://store.example.com`. Paths, credentials, query strings, fragments, and non-HTTP schemes are rejected. + ```yaml development: apikey: diff --git a/ntk/conf.py b/ntk/conf.py index 2bc2c84..c7f5e59 100644 --- a/ntk/conf.py +++ b/ntk/conf.py @@ -124,7 +124,7 @@ def normalize_store(store): raise TypeError( 'Store must be an absolute http(s) origin, for example https://store.example.com.' ) - return f'{parsed.scheme}://{parsed.netloc}' + return f'https://{parsed.netloc}' def read_config(self, update=True): configs = {} diff --git a/ntk/validation.py b/ntk/validation.py index f22b71c..3cdfb82 100644 --- a/ntk/validation.py +++ b/ntk/validation.py @@ -55,6 +55,11 @@ def validate_local_file(path): if "extends 'templates/catalogue/product.html'" in content or ( 'extends "templates/catalogue/product.html"' in content): errors.append('custom product templates must be standalone or extend layouts/base.html') + elif extension == '.js': + if content.startswith('\ufeff'): + errors.append('JavaScript must be UTF-8 without a byte-order mark') + if '\x00' in content: + errors.append('JavaScript must not contain NUL bytes') return { 'path': relative_path, 'status': 'invalid' if errors else 'valid', diff --git a/tests/test_command.py b/tests/test_command.py index b6e5766..4510881 100644 --- a/tests/test_command.py +++ b/tests/test_command.py @@ -152,7 +152,7 @@ def test_list_command_with_missing_theme_should_be_raise_message( self.assertEqual(self.mock_gateway.mock_calls, expected_gateway_calls) expected_logging = [ - 'WARNING:root:[development] Missing Themes in http://development.com' + 'WARNING:root:[development] Missing Themes in https://development.com' ] self.assertEqual(cm.output, expected_logging) diff --git a/tests/test_config.py b/tests/test_config.py index d51fd4c..4ea4e7d 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -158,7 +158,7 @@ def test_parser_config_should_set_config_config_correctly(self): self.config.parser_config(parser=parser) self.assertEqual(self.config.apikey, '2b78f637972b1c9d1234') - self.assertEqual(self.config.store, 'http://sandbox.com') + self.assertEqual(self.config.store, 'https://sandbox.com') self.assertEqual(self.config.theme_id, 1234) self.assertEqual(self.config.sass_output_style, 'nested') mock_write_config.assert_not_called() @@ -167,7 +167,7 @@ def test_parser_config_should_set_config_config_correctly(self): self.config.parser_config(parser=parser, write_file=True) self.assertEqual(self.config.apikey, '2b78f637972b1c9d1234') - self.assertEqual(self.config.store, 'http://sandbox.com') + self.assertEqual(self.config.store, 'https://sandbox.com') self.assertEqual(self.config.theme_id, 1234) self.assertEqual(self.config.sass_output_style, 'nested') mock_write_config.assert_called_once() diff --git a/tests/test_tooling_contract.py b/tests/test_tooling_contract.py index 9bf247f..1926328 100644 --- a/tests/test_tooling_contract.py +++ b/tests/test_tooling_contract.py @@ -20,6 +20,7 @@ class TestToolingContract(unittest.TestCase): def test_store_normalizes_bare_hostname_and_rejects_paths(self): self.assertEqual(Config.normalize_store('store.example.com/'), 'https://store.example.com') + self.assertEqual(Config.normalize_store('http://store.example.com'), 'https://store.example.com') with self.assertRaises(TypeError): Config.normalize_store('https://store.example.com/admin') @@ -86,6 +87,11 @@ def test_local_validation_covers_media_paths_and_product_inheritance(self): self.assertEqual(validate_local_file('image.png')['status'], 'valid') self.assertEqual(validate_local_file('missing.json')['status'], 'invalid') self.assertEqual(validate_local_file('unsupported.txt')['status'], 'invalid') + with open('bad.js', 'w', encoding='utf-8') as output_file: + output_file.write('\ufeffconst bad = "\x00";') + js_result = validate_local_file('bad.js') + self.assertEqual(js_result['status'], 'invalid') + self.assertEqual(len(js_result['errors']), 2) finally: os.chdir(old_directory) From 3a218e61009224258e8f7d0be639d56e211b42f9 Mon Sep 17 00:00:00 2001 From: Devin Michael Date: Wed, 29 Jul 2026 15:32:20 +0700 Subject: [PATCH 6/7] fix: report capture failures through machine output --- ntk/__main__.py | 8 ++++ ntk/command.py | 67 ++++++++++++++++++---------------- tests/test_main.py | 15 ++++++++ tests/test_tooling_contract.py | 16 ++++++++ 4 files changed, 74 insertions(+), 32 deletions(-) diff --git a/ntk/__main__.py b/ntk/__main__.py index 770ebe0..74d275b 100644 --- a/ntk/__main__.py +++ b/ntk/__main__.py @@ -43,6 +43,14 @@ def main(): sys.exit(1) except KeyboardInterrupt: pass + except Exception as e: + if getattr(args, 'json', False) is True: + output = Output() + output.configure(args) + output.error(getattr(args, 'command', 'ntk'), e, error_type=e.__class__.__name__) + else: + logging.exception(e) + sys.exit(1) if __name__ == '__main__': diff --git a/ntk/command.py b/ntk/command.py index a08c50d..314843c 100644 --- a/ntk/command.py +++ b/ntk/command.py @@ -328,36 +328,39 @@ def capture(self, parser): raise TypeError(f'Unknown viewport(s): {", ".join(unknown)}') os.makedirs(parser.output, exist_ok=True) results = [] - with sync_playwright() as playwright: - browser = playwright.chromium.launch(headless=True) - try: - for name in requested_viewports: - viewport = viewports[name] - page = browser.new_page(viewport=viewport) - page.goto(capture_url, wait_until='networkidle', timeout=parser.settle_timeout) - page.evaluate("document.fonts && document.fonts.ready") - page.evaluate(""" - async () => { - for (let y = 0; y < document.body.scrollHeight; y += window.innerHeight) { - window.scrollTo(0, y); - await new Promise(resolve => setTimeout(resolve, 50)); - } - window.scrollTo(0, 0); - } - """) - page.wait_for_function( - "[...document.images].every(image => image.complete)", timeout=parser.settle_timeout - ) - output_path = os.path.abspath(os.path.join(parser.output, f'{name}.png')) - page.screenshot(path=output_path, full_page=True) - results.append({ - 'viewport': name, - 'width': viewport['width'], - 'height': viewport['height'], - 'path': output_path, - 'status': 'captured', - }) - page.close() - finally: - browser.close() + try: + with sync_playwright() as playwright: + browser = playwright.chromium.launch(headless=True) + try: + for name in requested_viewports: + viewport = viewports[name] + page = browser.new_page(viewport=viewport) + page.goto(capture_url, wait_until='networkidle', timeout=parser.settle_timeout) + page.evaluate("document.fonts && document.fonts.ready") + page.evaluate(""" + async () => { + for (let y = 0; y < document.body.scrollHeight; y += window.innerHeight) { + window.scrollTo(0, y); + await new Promise(resolve => setTimeout(resolve, 50)); + } + window.scrollTo(0, 0); + } + """) + page.wait_for_function( + "[...document.images].every(image => image.complete)", timeout=parser.settle_timeout + ) + output_path = os.path.abspath(os.path.join(parser.output, f'{name}.png')) + page.screenshot(path=output_path, full_page=True) + results.append({ + 'viewport': name, + 'width': viewport['width'], + 'height': viewport['height'], + 'path': output_path, + 'status': 'captured', + }) + page.close() + finally: + browser.close() + except Exception as error: + raise NTKError(f'Capture failed for {capture_url}: {error}') from error return self.output.result('capture', url=capture_url, results=results, count=len(results)) diff --git a/tests/test_main.py b/tests/test_main.py index 0279c0b..154c928 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -35,5 +35,20 @@ def test_main_writes_one_json_error_for_machine_mode(self, mock_parser): self.assertEqual(payload['command'], 'validate') self.assertEqual(payload['error']['type'], 'TypeError') + @patch('ntk.__main__.Parser', autospec=True) + def test_main_keeps_unexpected_machine_errors_json_clean(self, mock_parser): + args = MagicMock() + args.json = True + args.quiet = False + args.no_progress = False + args.command = 'capture' + args.func.side_effect = RuntimeError('browser failed') + mock_parser.return_value.create_parser.return_value.parse_args.return_value = args + stdout = StringIO() + with self.assertRaises(SystemExit): + with redirect_stdout(stdout): + main() + self.assertEqual(json.loads(stdout.getvalue())['error']['type'], 'RuntimeError') + if __name__ == '__main__': unittest.main() diff --git a/tests/test_tooling_contract.py b/tests/test_tooling_contract.py index 1926328..ff967de 100644 --- a/tests/test_tooling_contract.py +++ b/tests/test_tooling_contract.py @@ -247,6 +247,22 @@ def test_capture_without_optional_dependency_is_actionable(self, _read_config): with self.assertRaises(NTKError): command.capture(parser) + class BrokenContext: + def __enter__(self): + raise RuntimeError('browser failed') + + def __exit__(self, *args): + return None + + sync_api = types.ModuleType('playwright.sync_api') + sync_api.sync_playwright = lambda: BrokenContext() + with patch.dict('sys.modules', { + 'playwright': types.ModuleType('playwright'), + 'playwright.sync_api': sync_api, + }): + with self.assertRaisesRegex(NTKError, 'Capture failed'): + command.capture(parser) + @patch('ntk.command.Gateway') @patch('ntk.command.Config.read_config') def test_push_reports_invalid_explicit_file_and_fails(self, _read_config, gateway): From 2aef2d82257fb9c88394159b0b7aff0a695a9c6a Mon Sep 17 00:00:00 2001 From: Devin Michael Date: Wed, 29 Jul 2026 16:44:28 +0700 Subject: [PATCH 7/7] chore: unstack theme tooling changes --- docs/production-capture-service.md | 21 --------------------- ntk/__main__.py | 12 +----------- ntk/command.py | 8 ++++---- tests/test_main.py | 1 + tests/test_tooling_contract.py | 9 ++++----- 5 files changed, 10 insertions(+), 41 deletions(-) delete mode 100644 docs/production-capture-service.md diff --git a/docs/production-capture-service.md b/docs/production-capture-service.md deleted file mode 100644 index fd7d108..0000000 --- a/docs/production-capture-service.md +++ /dev/null @@ -1,21 +0,0 @@ -# Production screenshot service design - -Theme Kit's `ntk capture` command is the supported local and CI path. A hosted capture service is intentionally deferred until the platform team can provide the isolation, storage, and operating controls below. - -## Proposed contract - -- `POST /api/admin/themes/{theme_id}/captures/` accepts an allowlisted store route, the fixed `desktop` and/or `mobile` viewports, and an optional theme revision. -- The API authenticates with `themes:read`, resolves only the tenant's network domain, rejects arbitrary hosts and private-network destinations, and returns `202` with a job ID. -- A sandboxed Chromium worker opens the preview URL with `skip_cache=1`, waits for network idle, fonts, lazy media, and decoded images, then writes 1440px and 390px full-page PNGs. -- Artifacts are content-addressed, encrypted, tenant-scoped, retained for seven days, and returned through short-lived signed URLs. Logs redact query secrets and cookies. -- Jobs have a 60-second wall-clock limit, strict memory/response-size caps, per-store concurrency and rate limits, idempotency keys, cancellation, and auditable failure codes. -- The result records theme ID/revision, route, viewport dimensions, capture timestamp, browser version, artifact hash, and final response `X-Theme-*` headers. - -## Delivery slices - -1. Reuse the deterministic wait and viewport contract from `ntk capture` in a container image. -2. Add the authenticated job API and queue with SSRF and tenant-isolation tests. -3. Add artifact storage, signed delivery, retention cleanup, and operational dashboards. -4. Dogfood against Spark route matrices before exposing the service outside platform engineering. - -The tracking issue linked from the Theme Kit pull request owns this follow-up. Until it ships, CI should install `next_theme_kit[capture]`, install Chromium, and retain `qa-output/*.png` as build artifacts. diff --git a/ntk/__main__.py b/ntk/__main__.py index 74d275b..4feb27f 100644 --- a/ntk/__main__.py +++ b/ntk/__main__.py @@ -4,7 +4,6 @@ from requests.exceptions import HTTPError -from ntk.exceptions import NTKError from ntk.ntk_parser import Parser from ntk.output import Output @@ -24,16 +23,7 @@ def main(): sys.exit(1) except AttributeError: print('Use ntk -h or --help to see available commands') - except NTKError as e: - if getattr(args, 'json', False) is True: - output = Output() - output.configure(args) - output.error( - getattr(args, 'command', 'ntk'), e, error_type=e.__class__.__name__) - else: - logging.error(e) - sys.exit(1) - except (TypeError, HTTPError) as e: + except (TypeError, HTTPError, RuntimeError) as e: if getattr(args, 'json', False) is True: output = Output() output.configure(args) diff --git a/ntk/command.py b/ntk/command.py index 314843c..850ee30 100644 --- a/ntk/command.py +++ b/ntk/command.py @@ -191,14 +191,14 @@ def _compile_sass(self): try: compiler = importlib.import_module('sass') except ImportError as error: - raise NTKError( + raise RuntimeError( 'Sass support is optional. Install next_theme_kit[sass] to use `ntk sass`.' ) from error try: compiler.compile(dirname=(SASS_SOURCE, SASS_DESTINATION), output_style=self.config.sass_output_style) logging.info(f'[{self.config.env}] Sass successfully processed.') except Exception as error: - raise NTKError(f'[{self.config.env}] Sass processing failed: {error}') from error + raise RuntimeError(f'[{self.config.env}] Sass processing failed: {error}') from error @parser_config(theme_id_required=False) def init(self, parser): @@ -307,7 +307,7 @@ def capture(self, parser): try: from playwright.sync_api import sync_playwright except ImportError as error: - raise NTKError( + raise RuntimeError( 'Capture support is optional. Install next_theme_kit[capture] and run `playwright install chromium`.' ) from error @@ -362,5 +362,5 @@ def capture(self, parser): finally: browser.close() except Exception as error: - raise NTKError(f'Capture failed for {capture_url}: {error}') from error + raise RuntimeError(f'Capture failed for {capture_url}: {error}') from error return self.output.result('capture', url=capture_url, results=results, count=len(results)) diff --git a/tests/test_main.py b/tests/test_main.py index 154c928..05bd62e 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -50,5 +50,6 @@ def test_main_keeps_unexpected_machine_errors_json_clean(self, mock_parser): main() self.assertEqual(json.loads(stdout.getvalue())['error']['type'], 'RuntimeError') + if __name__ == '__main__': unittest.main() diff --git a/tests/test_tooling_contract.py b/tests/test_tooling_contract.py index ff967de..0b59907 100644 --- a/tests/test_tooling_contract.py +++ b/tests/test_tooling_contract.py @@ -10,7 +10,6 @@ from ntk.command import Command from ntk.conf import Config -from ntk.exceptions import NTKError from ntk.ntk_parser import Parser from ntk.output import Output from ntk.utils import get_template_name, progress_bar @@ -200,12 +199,12 @@ def test_optional_sass_failures_are_actionable(self, _read_config): command = Command() with patch('ntk.command.sass', None): with patch('ntk.command.importlib.import_module', side_effect=ImportError): - with self.assertRaises(NTKError): + with self.assertRaises(RuntimeError): command._compile_sass() compiler = MagicMock() compiler.compile.side_effect = RuntimeError('compile failed') with patch('ntk.command.sass', compiler): - with self.assertRaises(NTKError): + with self.assertRaises(RuntimeError): command._compile_sass() @patch('ntk.command.Gateway') @@ -244,7 +243,7 @@ def test_capture_without_optional_dependency_is_actionable(self, _read_config): url='/', output='qa-output', viewports='desktop', settle_timeout=1000, ) with patch.dict('sys.modules', {'playwright.sync_api': None}): - with self.assertRaises(NTKError): + with self.assertRaises(RuntimeError): command.capture(parser) class BrokenContext: @@ -260,7 +259,7 @@ def __exit__(self, *args): 'playwright': types.ModuleType('playwright'), 'playwright.sync_api': sync_api, }): - with self.assertRaisesRegex(NTKError, 'Capture failed'): + with self.assertRaisesRegex(RuntimeError, 'Capture failed'): command.capture(parser) @patch('ntk.command.Gateway')