From 777fcdf236562e2bb9f079cfef088c7c0531c0b8 Mon Sep 17 00:00:00 2001 From: XuanRui LI Date: Thu, 4 Jun 2026 13:12:58 +0800 Subject: [PATCH 1/4] Add WebHarbor reset and smoke verification script --- README.md | 11 + scripts/check_reset_smoke.py | 552 ++++++++++++++++++++++++++++++ scripts/test_check_reset_smoke.py | 278 +++++++++++++++ 3 files changed, 841 insertions(+) create mode 100644 scripts/check_reset_smoke.py create mode 100644 scripts/test_check_reset_smoke.py diff --git a/README.md b/README.md index a05d266a..2f194de6 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,17 @@ Any other improvement — bug fixes, UI polish, data enrichment, task suggestion | 📊 Contribution Track Sheet | [Google Sheet](https://docs.google.com/spreadsheets/d/1vZsrQjy9nJKze58fx4kbQtFi85NjVXIWCFyu3ShD7gk/edit?gid=0#gid=0) | | 📝 Contribution Request Form | [Google Form](https://forms.gle/ngcD1rzAfUEphNmRA) | +## Reset And Smoke Checks + +Use the repository reset/smoke checker to verify control-plane resets, homepage reachability, and local seed/runtime DB parity: + +```bash +python scripts/check_reset_smoke.py --site amazon +python scripts/check_reset_smoke.py --control-url http://localhost:8101 +python scripts/check_reset_smoke.py --json +python scripts/check_reset_smoke.py --strict +``` + ## Citation WebHarbor is initiated by UNC-Chapel Hill and Microsoft, with contributions from the broader community. If you have any questions, please contact us via `webharborcomm at gmail dot com` or `zhaoyang at cs dot unc dot edu`. diff --git a/scripts/check_reset_smoke.py b/scripts/check_reset_smoke.py new file mode 100644 index 00000000..74b6c039 --- /dev/null +++ b/scripts/check_reset_smoke.py @@ -0,0 +1,552 @@ +#!/usr/bin/env python3 +"""Check WebHarbor control-plane reset behavior, homepage smoke, and DB MD5s.""" + +from __future__ import annotations + +import argparse +import ast +import hashlib +import json +import re +import sys +from dataclasses import asdict, dataclass +from pathlib import Path +from typing import Any +from urllib.error import HTTPError, URLError +from urllib.parse import urlparse +from urllib.request import Request, urlopen + + +@dataclass +class Message: + severity: str + message: str + site: str | None = None + url: str | None = None + file: str | None = None + + +@dataclass +class ControlCheck: + url: str + status: str + http_status: int | None + detail: str + + +@dataclass +class SiteCheck: + site: str + port: int + homepage_url: str + reset_status: str + reset_http_status: int | None + reset_detail: str + home_status: str + home_http_status: int | None + home_detail: str + md5_status: str + md5_runtime_db: str | None + md5_seed_db: str | None + md5_runtime_hash: str | None + md5_seed_hash: str | None + md5_detail: str + + +@dataclass +class SmokeResult: + root: str + control_url: str + base_host: str + timeout: float + strict: bool + reset_all: bool + control_server: ControlCheck + sites_discovered: int + sites_checked: int + site_checks: list[SiteCheck] + errors: list[Message] + warnings: list[Message] + + @property + def exit_code(self) -> int: + if self.errors: + return 1 + if self.strict and self.warnings: + return 1 + return 0 + + def summary_counts(self) -> dict[str, int]: + def count(attr: str, value: str) -> int: + return sum(1 for site in self.site_checks if getattr(site, attr) == value) + + return { + "reset_pass": count("reset_status", "PASS"), + "reset_fail": count("reset_status", "FAIL"), + "reset_skip": count("reset_status", "SKIP"), + "home_pass": count("home_status", "PASS"), + "home_fail": count("home_status", "FAIL"), + "home_skip": count("home_status", "SKIP"), + "md5_pass": count("md5_status", "PASS"), + "md5_fail": count("md5_status", "FAIL"), + "md5_skip": count("md5_status", "SKIP"), + } + + def to_json_dict(self) -> dict[str, Any]: + return { + "summary": { + "root": self.root, + "control_url": self.control_url, + "base_host": self.base_host, + "timeout": self.timeout, + "strict": self.strict, + "reset_all": self.reset_all, + "sites_discovered": self.sites_discovered, + "sites_checked": self.sites_checked, + "errors": len(self.errors), + "warnings": len(self.warnings), + "exit_code": self.exit_code, + **self.summary_counts(), + }, + "control_server": asdict(self.control_server), + "sites": [asdict(check) for check in self.site_checks], + "errors": [asdict(item) for item in self.errors], + "warnings": [asdict(item) for item in self.warnings], + } + + +class Collector: + def __init__(self) -> None: + self.errors: list[Message] = [] + self.warnings: list[Message] = [] + + def error( + self, + message: str, + *, + site: str | None = None, + url: str | None = None, + file: str | None = None, + ) -> None: + self.errors.append(Message("ERROR", message, site=site, url=url, file=file)) + + def warn( + self, + message: str, + *, + site: str | None = None, + url: str | None = None, + file: str | None = None, + ) -> None: + self.warnings.append(Message("WARN", message, site=site, url=url, file=file)) + + +def parse_site_array(text: str, file_label: str) -> tuple[list[str], int]: + match = re.search(r"\bSITES\s*=\s*(\(.+?\)|\[.+?\])", text, re.DOTALL) + if not match: + raise ValueError(f"Could not parse SITES from {file_label}") + block = match.group(1) + if block.startswith("("): + sites = re.findall(r"[A-Za-z0-9_]+", block) + else: + sites = ast.literal_eval(block) + if not isinstance(sites, list): + raise ValueError(f"SITES is not a list in {file_label}") + base_match = re.search(r"\bBASE_PORT\s*=\s*(\d+)", text) + if not base_match: + raise ValueError(f"Could not parse BASE_PORT from {file_label}") + return sites, int(base_match.group(1)) + + +def build_port_map(sites: list[str], base_port: int) -> dict[str, int]: + return {site: base_port + index for index, site in enumerate(sites)} + + +def discover_sites(root: Path) -> dict[str, int]: + websyn_text = (root / "websyn_start.sh").read_text(encoding="utf-8", errors="replace") + control_text = (root / "control_server.py").read_text(encoding="utf-8", errors="replace") + websyn_sites, websyn_base = parse_site_array(websyn_text, "websyn_start.sh") + control_sites, control_base = parse_site_array(control_text, "control_server.py") + if websyn_sites != control_sites or websyn_base != control_base: + raise ValueError( + "websyn_start.sh and control_server.py registration lists are out of sync" + ) + return build_port_map(websyn_sites, websyn_base) + + +def md5_file(path: Path) -> str: + hasher = hashlib.md5() + with path.open("rb") as handle: + for chunk in iter(lambda: handle.read(1024 * 1024), b""): + hasher.update(chunk) + return hasher.hexdigest() + + +def resolve_db_pair(site_root: Path, site: str) -> tuple[Path | None, Path | None, str | None]: + runtime_dir = site_root / "instance" + seed_dir = site_root / "instance_seed" + if not runtime_dir.exists() or not seed_dir.exists(): + return None, None, "runtime or seed DB directory is missing locally" + + runtime_files = sorted(runtime_dir.glob("*.db")) + seed_files = sorted(seed_dir.glob("*.db")) + if not runtime_files or not seed_files: + return None, None, "runtime or seed DB files are missing locally" + + if len(runtime_files) == 1 and len(seed_files) == 1: + return runtime_files[0], seed_files[0], None + + runtime_by_name = {path.name: path for path in runtime_files} + seed_by_name = {path.name: path for path in seed_files} + shared_names = sorted(set(runtime_by_name) & set(seed_by_name)) + preferred_name = f"{site}.db" + if preferred_name in runtime_by_name and preferred_name in seed_by_name: + return runtime_by_name[preferred_name], seed_by_name[preferred_name], None + if len(shared_names) == 1: + name = shared_names[0] + return runtime_by_name[name], seed_by_name[name], None + return None, None, "could not infer a unique runtime/seed DB pair" + + +def http_request( + url: str, + *, + method: str = "GET", + timeout: float = 10.0, +) -> tuple[bool, int | None, str]: + request = Request(url, method=method) + try: + with urlopen(request, timeout=timeout) as response: + body = response.read(512) + detail = f"HTTP {response.status}" + if body: + detail += f" ({len(body)} byte(s) read)" + return True, response.status, detail + except HTTPError as exc: + return False, exc.code, f"HTTP {exc.code}: {exc.reason}" + except URLError as exc: + reason = getattr(exc, "reason", exc) + detail = str(reason) + if "Connection refused" in detail or "[WinError 10061]" in detail: + detail += " (server may not be running)" + return False, None, detail + except OSError as exc: + detail = str(exc) + if "Connection refused" in detail or "[WinError 10061]" in detail: + detail += " (server may not be running)" + return False, None, detail + + +def normalize_control_url(url: str) -> str: + parsed = urlparse(url) + if parsed.scheme and parsed.netloc: + return url.rstrip("/") + return f"http://{url.strip().rstrip('/')}" + + +def build_homepage_url(base_host: str, port: int) -> str: + host = base_host.strip().strip("/") + if "://" in host: + parsed = urlparse(host) + scheme = parsed.scheme or "http" + hostname = parsed.hostname or "localhost" + return f"{scheme}://{hostname}:{port}/" + return f"http://{host}:{port}/" + + +def check_control_health(control_url: str, timeout: float, collector: Collector) -> ControlCheck: + url = f"{control_url}/health" + ok, status_code, detail = http_request(url, timeout=timeout) + if ok: + return ControlCheck(url=url, status="PASS", http_status=status_code, detail=detail) + if status_code == 404: + collector.warn("control server health endpoint is missing; skipping health validation", url=url) + return ControlCheck(url=url, status="SKIP", http_status=status_code, detail=detail) + collector.error("control server health check failed", url=url) + return ControlCheck(url=url, status="FAIL", http_status=status_code, detail=detail) + + +def check_site( + root: Path, + site: str, + port: int, + *, + control_url: str, + base_host: str, + timeout: float, + collector: Collector, + use_reset_all: bool, + reset_all_ok: bool, +) -> SiteCheck: + homepage_url = build_homepage_url(base_host, port) + site_root = root / "sites" / site + + if use_reset_all: + if reset_all_ok: + reset_status = "PASS" + reset_code = 200 + reset_detail = "covered by successful /reset-all call" + else: + reset_status = "FAIL" + reset_code = None + reset_detail = "reset-all failed; per-site reset was not attempted" + collector.error("reset-all failed; site reset considered failed", site=site, url=f"{control_url}/reset-all") + else: + reset_url = f"{control_url}/reset/{site}" + ok, status_code, detail = http_request(reset_url, method="POST", timeout=timeout) + if ok: + reset_status = "PASS" + reset_code = status_code + reset_detail = detail + else: + reset_status = "FAIL" + reset_code = status_code + reset_detail = detail + collector.error("site reset request failed", site=site, url=reset_url) + + home_ok, home_status_code, home_detail = http_request(homepage_url, timeout=timeout) + if home_ok or (home_status_code is not None and 300 <= home_status_code < 400): + home_status = "PASS" + else: + home_status = "FAIL" + collector.error("homepage smoke check failed", site=site, url=homepage_url) + + runtime_db, seed_db, md5_skip_reason = resolve_db_pair(site_root, site) + if md5_skip_reason: + md5_status = "SKIP" + collector.warn(md5_skip_reason, site=site, file=str(site_root)) + runtime_path = None + seed_path = None + runtime_hash = None + seed_hash = None + md5_detail = md5_skip_reason + else: + assert runtime_db is not None and seed_db is not None + runtime_hash = md5_file(runtime_db) + seed_hash = md5_file(seed_db) + runtime_path = str(runtime_db) + seed_path = str(seed_db) + if runtime_hash == seed_hash: + md5_status = "PASS" + md5_detail = "runtime DB matches seed DB" + else: + md5_status = "FAIL" + md5_detail = "runtime DB MD5 differs from seed DB after reset" + collector.error(md5_detail, site=site, file=runtime_path) + + return SiteCheck( + site=site, + port=port, + homepage_url=homepage_url, + reset_status=reset_status, + reset_http_status=reset_code, + reset_detail=reset_detail, + home_status=home_status, + home_http_status=home_status_code, + home_detail=home_detail, + md5_status=md5_status, + md5_runtime_db=runtime_path, + md5_seed_db=seed_path, + md5_runtime_hash=runtime_hash, + md5_seed_hash=seed_hash, + md5_detail=md5_detail, + ) + + +def run_checks( + root: Path, + *, + site: str | None = None, + control_url: str = "http://localhost:8101", + base_host: str = "localhost", + timeout: float = 10.0, + strict: bool = False, + reset_all: bool = False, +) -> SmokeResult: + collector = Collector() + control_url = normalize_control_url(control_url) + site_map = discover_sites(root) + if site is not None: + if site not in site_map: + collector.error(f"unknown site '{site}'") + return SmokeResult( + root=str(root), + control_url=control_url, + base_host=base_host, + timeout=timeout, + strict=strict, + reset_all=reset_all, + control_server=ControlCheck( + url=f"{control_url}/health", + status="SKIP", + http_status=None, + detail="site lookup failed before control checks", + ), + sites_discovered=len(site_map), + sites_checked=0, + site_checks=[], + errors=collector.errors, + warnings=collector.warnings, + ) + filtered_sites = {site: site_map[site]} + else: + filtered_sites = site_map + + control = check_control_health(control_url, timeout, collector) + reset_all_ok = False + if reset_all: + reset_all_url = f"{control_url}/reset-all" + ok, status_code, detail = http_request(reset_all_url, method="POST", timeout=timeout) + if ok: + reset_all_ok = True + else: + collector.error("reset-all request failed", url=reset_all_url) + if status_code == 404: + collector.warn("control server does not expose /reset-all", url=reset_all_url) + + site_checks = [ + check_site( + root, + site_slug, + port, + control_url=control_url, + base_host=base_host, + timeout=timeout, + collector=collector, + use_reset_all=reset_all, + reset_all_ok=reset_all_ok, + ) + for site_slug, port in filtered_sites.items() + ] + + return SmokeResult( + root=str(root), + control_url=control_url, + base_host=base_host, + timeout=timeout, + strict=strict, + reset_all=reset_all, + control_server=control, + sites_discovered=len(site_map), + sites_checked=len(site_checks), + site_checks=site_checks, + errors=collector.errors, + warnings=collector.warnings, + ) + + +def render_human(result: SmokeResult) -> str: + counts = result.summary_counts() + lines = [ + f"Control URL: {result.control_url}", + f"Base host: {result.base_host}", + f"Sites discovered: {result.sites_discovered}", + f"Sites checked: {result.sites_checked}", + ( + "Reset pass/fail/skip: " + f"{counts['reset_pass']}/{counts['reset_fail']}/{counts['reset_skip']}" + ), + ( + "Homepage pass/fail/skip: " + f"{counts['home_pass']}/{counts['home_fail']}/{counts['home_skip']}" + ), + ( + "MD5 pass/fail/skip: " + f"{counts['md5_pass']}/{counts['md5_fail']}/{counts['md5_skip']}" + ), + f"Errors: {len(result.errors)} Warnings: {len(result.warnings)}", + ( + "Control health: " + f"{result.control_server.status} " + f"{result.control_server.http_status or ''} " + f"{result.control_server.detail}" + ).strip(), + "", + ] + + for site in result.site_checks: + lines.append( + ( + f"[{site.site}] port={site.port} " + f"reset={site.reset_status} home={site.home_status} md5={site.md5_status}" + ) + ) + lines.append(f" reset: {site.reset_detail}") + lines.append(f" home: {site.home_detail}") + lines.append(f" md5: {site.md5_detail}") + + findings = [*result.errors, *result.warnings] + if findings: + lines.append("") + for item in findings: + parts = [item.severity] + if item.site: + parts.append(f"site={item.site}") + if item.url: + parts.append(f"url={item.url}") + if item.file: + parts.append(f"file={item.file}") + parts.append(item.message) + lines.append(" | ".join(parts)) + + return "\n".join(lines) + + +def build_arg_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--site", help="Check only one registered site slug") + parser.add_argument( + "--control-url", + default="http://localhost:8101", + help="Control server base URL (default: http://localhost:8101)", + ) + parser.add_argument( + "--base-host", + default="localhost", + help="Hostname used to build per-site homepage URLs (default: localhost)", + ) + parser.add_argument( + "--timeout", + type=float, + default=10.0, + help="HTTP timeout in seconds for control and homepage checks", + ) + parser.add_argument("--strict", action="store_true", help="Treat warnings as failures") + parser.add_argument("--json", action="store_true", help="Print machine-readable JSON only") + parser.add_argument( + "--reset-all", + action="store_true", + help="Call POST /reset-all once instead of per-site POST /reset/", + ) + return parser + + +def main( + argv: list[str] | None = None, + *, + root: Path | None = None, + stdout: Any = None, +) -> int: + args = build_arg_parser().parse_args(argv) + target_root = root or Path(__file__).resolve().parents[1] + result = run_checks( + target_root, + site=args.site, + control_url=args.control_url, + base_host=args.base_host, + timeout=args.timeout, + strict=args.strict, + reset_all=args.reset_all, + ) + stream = stdout if stdout is not None else sys.stdout + if args.json: + json.dump(result.to_json_dict(), stream, indent=2, sort_keys=True) + stream.write("\n") + else: + stream.write(render_human(result)) + stream.write("\n") + return result.exit_code + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/test_check_reset_smoke.py b/scripts/test_check_reset_smoke.py new file mode 100644 index 00000000..973f6147 --- /dev/null +++ b/scripts/test_check_reset_smoke.py @@ -0,0 +1,278 @@ +#!/usr/bin/env python3 +"""Tests for scripts/check_reset_smoke.py.""" + +from __future__ import annotations + +import io +import json +import sys +import tempfile +import textwrap +import threading +import unittest +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +import check_reset_smoke as smoke # noqa: E402 + + +def write(path: Path, content: str) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(textwrap.dedent(content).lstrip("\n"), encoding="utf-8") + + +def build_repo( + root: Path, + *, + sites: list[str] | None = None, + base_port: int = 40000, + with_runtime_db: bool = True, + with_seed_db: bool = True, + runtime_content: bytes = b"seed", + seed_content: bytes = b"seed", +) -> None: + sites = sites or ["amazon"] + write( + root / "websyn_start.sh", + f""" + #!/bin/bash + SITES=({' '.join(sites)}) + BASE_PORT={base_port} + """, + ) + write( + root / "control_server.py", + f""" + SITES = {sites!r} + BASE_PORT = {base_port} + """, + ) + write( + root / "site_runner.py", + """ + from app import app + """, + ) + write( + root / "README.md", + """ + curl -X POST http://localhost:8101/reset/amazon + """, + ) + for site in sites: + site_root = root / "sites" / site + write(site_root / "app.py", "from flask import Flask\napp = Flask(__name__)\n") + write( + site_root / "tasks.jsonl", + json.dumps( + { + "web_name": site.title(), + "id": f"{site}--0", + "ques": "Find something", + "web": f"http://localhost:{base_port}/", + "upstream_url": f"https://{site}.example.com/", + } + ) + + "\n", + ) + if with_runtime_db: + runtime_dir = site_root / "instance" + runtime_dir.mkdir(parents=True, exist_ok=True) + (runtime_dir / f"{site}.db").write_bytes(runtime_content) + if with_seed_db: + seed_dir = site_root / "instance_seed" + seed_dir.mkdir(parents=True, exist_ok=True) + (seed_dir / f"{site}.db").write_bytes(seed_content) + + +class _SmokeHandler(BaseHTTPRequestHandler): + def do_GET(self) -> None: # noqa: N802 + if self.path == "/health": + body = json.dumps( + {"ok": True, "sites": {"amazon": {"alive": True, "port": self.server.server_port}}} + ).encode("utf-8") + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + return + if self.path == "/": + body = b"ok" + self.send_response(200) + self.send_header("Content-Type", "text/html") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + return + self.send_response(404) + self.end_headers() + + def do_POST(self) -> None: # noqa: N802 + if self.path in {"/reset/amazon", "/reset-all"}: + body = b'{"ready": true}' + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + return + self.send_response(404) + self.end_headers() + + def log_message(self, format: str, *args: object) -> None: # noqa: A003 + return + + +class SmokeServer: + def __init__(self) -> None: + self.server = ThreadingHTTPServer(("127.0.0.1", 0), _SmokeHandler) + self.thread = threading.Thread(target=self.server.serve_forever, daemon=True) + + @property + def port(self) -> int: + return self.server.server_port + + def __enter__(self) -> "SmokeServer": + self.thread.start() + return self + + def __exit__(self, exc_type, exc, tb) -> None: + self.server.shutdown() + self.server.server_close() + self.thread.join(timeout=5) + + +class CheckResetSmokeTests(unittest.TestCase): + def test_md5_match_passes(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + build_repo(root) + result = smoke.run_checks( + root, + site="amazon", + control_url="http://127.0.0.1:9", + base_host="127.0.0.1", + timeout=0.1, + ) + self.assertEqual(result.site_checks[0].md5_status, "PASS") + + def test_md5_mismatch_fails(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + build_repo(root, runtime_content=b"runtime", seed_content=b"seed") + result = smoke.run_checks( + root, + site="amazon", + control_url="http://127.0.0.1:9", + base_host="127.0.0.1", + timeout=0.1, + ) + self.assertEqual(result.site_checks[0].md5_status, "FAIL") + self.assertNotEqual(result.exit_code, 0) + + def test_missing_dbs_warn_not_error_by_default(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + with SmokeServer() as server: + root = Path(tmpdir) + build_repo( + root, + base_port=server.port, + with_runtime_db=False, + with_seed_db=False, + ) + result = smoke.run_checks( + root, + site="amazon", + control_url=f"http://127.0.0.1:{server.port}", + base_host="127.0.0.1", + timeout=2.0, + ) + self.assertEqual(result.site_checks[0].md5_status, "SKIP") + self.assertEqual(result.exit_code, 0) + self.assertTrue(any("DB" in warning.message for warning in result.warnings)) + + def test_site_filtering_and_unknown_site(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + build_repo(root, sites=["amazon", "apple"]) + filtered = smoke.run_checks( + root, + site="apple", + control_url="http://127.0.0.1:9", + base_host="127.0.0.1", + timeout=0.1, + ) + self.assertEqual(filtered.sites_checked, 1) + self.assertEqual(filtered.site_checks[0].site, "apple") + unknown = smoke.run_checks(root, site="amtrak") + self.assertEqual(unknown.exit_code, 1) + + def test_json_output_is_valid(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + build_repo(root) + buffer = io.StringIO() + exit_code = smoke.main( + ["--json", "--site", "amazon", "--control-url", "http://127.0.0.1:9", "--base-host", "127.0.0.1", "--timeout", "0.1"], + root=root, + stdout=buffer, + ) + payload = json.loads(buffer.getvalue()) + self.assertEqual(exit_code, 1) + self.assertIn("summary", payload) + self.assertIn("sites", payload) + self.assertIn("control_server", payload) + + def test_strict_mode_treats_warnings_as_failure(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + with SmokeServer() as server: + root = Path(tmpdir) + build_repo( + root, + base_port=server.port, + with_runtime_db=False, + with_seed_db=False, + ) + normal = smoke.run_checks( + root, + site="amazon", + control_url=f"http://127.0.0.1:{server.port}", + base_host="127.0.0.1", + timeout=2.0, + strict=False, + ) + strict = smoke.run_checks( + root, + site="amazon", + control_url=f"http://127.0.0.1:{server.port}", + base_host="127.0.0.1", + timeout=2.0, + strict=True, + ) + self.assertEqual(normal.strict, False) + self.assertEqual(normal.exit_code, 0) + self.assertEqual(strict.strict, True) + self.assertEqual(strict.exit_code, 1) + + def test_http_reset_and_homepage_pass(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + with SmokeServer() as server: + root = Path(tmpdir) + build_repo(root, base_port=server.port) + result = smoke.run_checks( + root, + site="amazon", + control_url=f"http://127.0.0.1:{server.port}", + base_host="127.0.0.1", + timeout=2.0, + ) + self.assertEqual(result.control_server.status, "PASS") + self.assertEqual(result.site_checks[0].reset_status, "PASS") + self.assertEqual(result.site_checks[0].home_status, "PASS") + self.assertEqual(result.site_checks[0].md5_status, "PASS") + + +if __name__ == "__main__": + unittest.main() From aeb5c0493700b64f96877616d98b831390edf0d5 Mon Sep 17 00:00:00 2001 From: JackJin Date: Sun, 13 Sep 2026 00:56:45 +0800 Subject: [PATCH 2/4] fix(check_reset_smoke): tie DB parity to the source it actually read Review of #47. The reset and homepage checks were correct; the DB parity check and the registry failure paths were not. DB parity hashed /sites//{instance,instance_seed}, but the control plane resets /opt/WebSyn//instance inside the deployment and the Dockerfile lays sites out with `COPY sites/ /opt/WebSyn/`. Under the workflow the README documents those are never the same files, so the verdict was independent of the environment it claimed to check: with the container stopped it still reported "runtime DB matches seed DB", and dirtying only the local checkout produced a failing "differs from seed DB after reset" against a container verified clean. - add --docker-container to hash under /opt/WebSyn/ inside the running deployment, and --db-root for a host deployment - report md5_source on every site result and in --json; a PASS now always names the DBs it read - with no source configured, SKIP instead of silently comparing the checkout, and drop the warning that made --strict fail a correct docker environment - do not report local parity when no reset succeeded, and reserve the "after reset" wording for a source that is the reset target - raise RegistryError for drift, missing and unparseable registries so they are structured findings instead of tracebacks, and name BASE_PORT mismatches specifically rather than as list drift - record a failed --reset-all once instead of once per registered site Tests: 11 added for the DB source contract, registry failures and --reset-all counting; the pre-existing md5 tests now run against a live control plane because a parity verdict requires a reset to be "after". 18 pass. Co-Authored-By: Claude Opus 5 --- README.md | 20 +- scripts/check_reset_smoke.py | 297 +++++++++++++++++++++++++----- scripts/test_check_reset_smoke.py | 255 ++++++++++++++++++++++--- 3 files changed, 500 insertions(+), 72 deletions(-) diff --git a/README.md b/README.md index 2f194de6..39bdc0b6 100644 --- a/README.md +++ b/README.md @@ -101,7 +101,8 @@ Any other improvement — bug fixes, UI polish, data enrichment, task suggestion ## Reset And Smoke Checks -Use the repository reset/smoke checker to verify control-plane resets, homepage reachability, and local seed/runtime DB parity: +Use the repository reset/smoke checker to verify control-plane resets, homepage +reachability, and runtime/seed DB parity: ```bash python scripts/check_reset_smoke.py --site amazon @@ -110,6 +111,23 @@ python scripts/check_reset_smoke.py --json python scripts/check_reset_smoke.py --strict ``` +Reset and homepage checks go over HTTP, so they work from anywhere that can reach the +control plane. The DB parity check has to read the files the control plane actually +resets — `/opt/WebSyn//instance` **inside the deployment**, which is not this +checkout when the environment runs in Docker. Point the checker at that source: + +```bash +# environment in a container (the usual case) +python scripts/check_reset_smoke.py --docker-container + +# sites deployed on this host +python scripts/check_reset_smoke.py --db-root /opt/WebSyn +``` + +Without one of those flags the DB check reports `SKIP` with source `none` rather than +comparing this checkout's files, and every result names the source it hashed +(`md5_source`), so a `PASS` always says which DBs it read. + ## Citation WebHarbor is initiated by UNC-Chapel Hill and Microsoft, with contributions from the broader community. If you have any questions, please contact us via `webharborcomm at gmail dot com` or `zhaoyang at cs dot unc dot edu`. diff --git a/scripts/check_reset_smoke.py b/scripts/check_reset_smoke.py index 74b6c039..c373ce36 100644 --- a/scripts/check_reset_smoke.py +++ b/scripts/check_reset_smoke.py @@ -1,5 +1,12 @@ #!/usr/bin/env python3 -"""Check WebHarbor control-plane reset behavior, homepage smoke, and DB MD5s.""" +"""Check WebHarbor control-plane reset behavior, homepage smoke, and DB MD5s. + +The DB parity check hashes whichever DB source is configured and always reports +which one it read (``md5_source``). The control plane resets +``/opt/WebSyn//instance`` inside the deployment, which is not the repository +checkout when the environment runs in Docker, so a parity verdict is only reported +against a source the caller actually pointed at. +""" from __future__ import annotations @@ -8,6 +15,8 @@ import hashlib import json import re +import shlex +import subprocess import sys from dataclasses import asdict, dataclass from pathlib import Path @@ -46,6 +55,7 @@ class SiteCheck: home_http_status: int | None home_detail: str md5_status: str + md5_source: str md5_runtime_db: str | None md5_seed_db: str | None md5_runtime_hash: str | None @@ -141,20 +151,24 @@ def warn( self.warnings.append(Message("WARN", message, site=site, url=url, file=file)) +class RegistryError(Exception): + """A site registry could not be read or the two registries disagree.""" + + def parse_site_array(text: str, file_label: str) -> tuple[list[str], int]: match = re.search(r"\bSITES\s*=\s*(\(.+?\)|\[.+?\])", text, re.DOTALL) if not match: - raise ValueError(f"Could not parse SITES from {file_label}") + raise RegistryError(f"Could not parse SITES from {file_label}") block = match.group(1) if block.startswith("("): sites = re.findall(r"[A-Za-z0-9_]+", block) else: sites = ast.literal_eval(block) if not isinstance(sites, list): - raise ValueError(f"SITES is not a list in {file_label}") + raise RegistryError(f"SITES is not a list in {file_label}") base_match = re.search(r"\bBASE_PORT\s*=\s*(\d+)", text) if not base_match: - raise ValueError(f"Could not parse BASE_PORT from {file_label}") + raise RegistryError(f"Could not parse BASE_PORT from {file_label}") return sites, int(base_match.group(1)) @@ -162,14 +176,43 @@ def build_port_map(sites: list[str], base_port: int) -> dict[str, int]: return {site: base_port + index for index, site in enumerate(sites)} +def _read_registry(root: Path, name: str) -> str: + try: + return (root / name).read_text(encoding="utf-8", errors="replace") + except FileNotFoundError: + raise RegistryError(f"{name} is missing from {root}") from None + except OSError as exc: + raise RegistryError(f"{name} could not be read: {exc}") from None + + def discover_sites(root: Path) -> dict[str, int]: - websyn_text = (root / "websyn_start.sh").read_text(encoding="utf-8", errors="replace") - control_text = (root / "control_server.py").read_text(encoding="utf-8", errors="replace") + """Return the registered site -> port map, or raise RegistryError. + + Registry drift is a finding this checker is meant to report, so every failure + here is raised as RegistryError and rendered as a structured error by main(). + """ + websyn_text = _read_registry(root, "websyn_start.sh") + control_text = _read_registry(root, "control_server.py") websyn_sites, websyn_base = parse_site_array(websyn_text, "websyn_start.sh") control_sites, control_base = parse_site_array(control_text, "control_server.py") - if websyn_sites != control_sites or websyn_base != control_base: - raise ValueError( - "websyn_start.sh and control_server.py registration lists are out of sync" + if websyn_sites != control_sites: + only_websyn = [s for s in websyn_sites if s not in control_sites] + only_control = [s for s in control_sites if s not in websyn_sites] + detail = [] + if only_websyn: + detail.append(f"only in websyn_start.sh: {', '.join(only_websyn)}") + if only_control: + detail.append(f"only in control_server.py: {', '.join(only_control)}") + if not detail: + detail.append("same sites in a different order") + raise RegistryError( + "websyn_start.sh and control_server.py registration lists are " + f"out of sync ({'; '.join(detail)})" + ) + if websyn_base != control_base: + raise RegistryError( + f"BASE_PORT differs between the registries: websyn_start.sh={websyn_base}, " + f"control_server.py={control_base}" ) return build_port_map(websyn_sites, websyn_base) @@ -208,6 +251,137 @@ def resolve_db_pair(site_root: Path, site: str) -> tuple[Path | None, Path | Non return None, None, "could not infer a unique runtime/seed DB pair" +DOCKER_SITE_ROOT = "/opt/WebSyn" + + +@dataclass +class DbCheck: + status: str + source: str + runtime_db: str | None + seed_db: str | None + runtime_hash: str | None + seed_hash: str | None + detail: str + + +def _pick_db(names: list[str], site: str) -> tuple[str | None, str | None]: + """Choose the one DB that represents a site, mirroring resolve_db_pair().""" + if len(names) == 1: + return names[0], None + preferred = f"{site}.db" + if preferred in names: + return preferred, None + return None, "could not infer a unique runtime/seed DB pair" + + +def docker_md5(container: str, dirs: list[str]) -> dict[str, str]: + """Hash the single *.db in each directory inside a running container.""" + hashes: dict[str, str] = {} + for directory in dirs: + cmd = ["docker", "exec", container, "sh", "-c", + f"md5sum {shlex.quote(directory)}/*.db"] + try: + proc = subprocess.run(cmd, capture_output=True, text=True, timeout=60) + except (OSError, subprocess.SubprocessError) as exc: + raise RuntimeError(f"docker exec failed: {exc}") from None + if proc.returncode != 0: + raise RuntimeError( + f"docker exec failed for {directory}: " + f"{proc.stderr.strip() or f'exit {proc.returncode}'}" + ) + entries: dict[str, str] = {} + for line in proc.stdout.splitlines(): + parts = line.split(None, 1) + if len(parts) == 2: + entries[Path(parts[1].strip()).name] = parts[0] + name, problem = _pick_db(sorted(entries), Path(directory).parent.name) + if problem or name is None: + raise RuntimeError(problem or "no *.db found") + hashes[directory] = entries[name] + return hashes + + +def check_db_parity( + site: str, + *, + repo_root: Path, + db_root: str | None, + docker_container: str | None, + container_hasher: Any, + collector: Collector, + reset_succeeded: bool, +) -> DbCheck: + """Compare a site's runtime DB against its seed in the configured source. + + The control plane resets DOCKER_SITE_ROOT//instance inside the deployment. + Only a source the caller pointed at is hashed, and the source is always reported, + so a PASS can never be read as a statement about an environment that was not read. + """ + if docker_container: + base = f"{DOCKER_SITE_ROOT}/{site}" + source = f"docker:{docker_container}:{base}" + runtime_dir, seed_dir = f"{base}/instance", f"{base}/instance_seed" + hasher = container_hasher or docker_md5 + try: + hashes = hasher(docker_container, [runtime_dir, seed_dir]) + except Exception as exc: # noqa: BLE001 - reported, never raised to the user + collector.error(f"could not hash DBs in container: {exc}", site=site) + return DbCheck("FAIL", source, runtime_dir, seed_dir, None, None, str(exc)) + runtime_hash, seed_hash = hashes.get(runtime_dir), hashes.get(seed_dir) + if runtime_hash and runtime_hash == seed_hash: + return DbCheck("PASS", source, runtime_dir, seed_dir, runtime_hash, + seed_hash, "runtime DB matches seed DB") + detail = ( + "runtime DB MD5 differs from seed DB after reset" if reset_succeeded + else "runtime DB MD5 differs from seed DB (no successful reset this run)" + ) + collector.error(detail, site=site, file=runtime_dir) + return DbCheck("FAIL", source, runtime_dir, seed_dir, runtime_hash, seed_hash, + detail) + + explicit = db_root is not None + base_dir = Path(db_root) if explicit else repo_root / "sites" + site_root = base_dir / site + runtime_db, seed_db, problem = resolve_db_pair(site_root, site) + if problem: + source = f"local:{site_root}" + if explicit: + detail = f"{problem} under --db-root {base_dir}" + collector.error(detail, site=site, file=str(site_root)) + return DbCheck("FAIL", source, None, None, None, None, detail) + if "missing locally" in problem: + # The documented docker workflow keeps instance/ inside the container, so + # its absence here is an expected configuration rather than a fault. + return DbCheck( + "SKIP", "none", None, None, None, None, + "no DB source configured; the control plane resets " + f"{DOCKER_SITE_ROOT}/{site}/instance inside the deployment. " + "Pass --docker-container or --db-root to check DB parity.", + ) + collector.warn(problem, site=site, file=str(site_root)) + return DbCheck("SKIP", source, None, None, None, None, problem) + + assert runtime_db is not None and seed_db is not None + source = f"local:{site_root}" + if not reset_succeeded: + # There is no reset to attribute a parity verdict to, and this checkout is not + # necessarily the reset target, so reporting PASS here would assert something + # that was never observed. + return DbCheck( + "SKIP", source, str(runtime_db), str(seed_db), None, None, + "no successful reset to verify; DB parity not evaluated", + ) + runtime_hash, seed_hash = md5_file(runtime_db), md5_file(seed_db) + if runtime_hash == seed_hash: + return DbCheck("PASS", source, str(runtime_db), str(seed_db), runtime_hash, + seed_hash, "runtime DB matches seed DB") + detail = "local runtime DB differs from local seed DB" + collector.error(detail, site=site, file=str(runtime_db)) + return DbCheck("FAIL", source, str(runtime_db), str(seed_db), runtime_hash, + seed_hash, detail) + + def http_request( url: str, *, @@ -277,9 +451,11 @@ def check_site( collector: Collector, use_reset_all: bool, reset_all_ok: bool, + db_root: str | None = None, + docker_container: str | None = None, + container_hasher: Any = None, ) -> SiteCheck: homepage_url = build_homepage_url(base_host, port) - site_root = root / "sites" / site if use_reset_all: if reset_all_ok: @@ -289,8 +465,9 @@ def check_site( else: reset_status = "FAIL" reset_code = None + # The failure is already recorded once against /reset-all itself; do not + # repeat it for every registered site. reset_detail = "reset-all failed; per-site reset was not attempted" - collector.error("reset-all failed; site reset considered failed", site=site, url=f"{control_url}/reset-all") else: reset_url = f"{control_url}/reset/{site}" ok, status_code, detail = http_request(reset_url, method="POST", timeout=timeout) @@ -311,28 +488,15 @@ def check_site( home_status = "FAIL" collector.error("homepage smoke check failed", site=site, url=homepage_url) - runtime_db, seed_db, md5_skip_reason = resolve_db_pair(site_root, site) - if md5_skip_reason: - md5_status = "SKIP" - collector.warn(md5_skip_reason, site=site, file=str(site_root)) - runtime_path = None - seed_path = None - runtime_hash = None - seed_hash = None - md5_detail = md5_skip_reason - else: - assert runtime_db is not None and seed_db is not None - runtime_hash = md5_file(runtime_db) - seed_hash = md5_file(seed_db) - runtime_path = str(runtime_db) - seed_path = str(seed_db) - if runtime_hash == seed_hash: - md5_status = "PASS" - md5_detail = "runtime DB matches seed DB" - else: - md5_status = "FAIL" - md5_detail = "runtime DB MD5 differs from seed DB after reset" - collector.error(md5_detail, site=site, file=runtime_path) + db = check_db_parity( + site, + repo_root=root, + db_root=db_root, + docker_container=docker_container, + container_hasher=container_hasher, + collector=collector, + reset_succeeded=reset_status == "PASS", + ) return SiteCheck( site=site, @@ -344,12 +508,13 @@ def check_site( home_status=home_status, home_http_status=home_status_code, home_detail=home_detail, - md5_status=md5_status, - md5_runtime_db=runtime_path, - md5_seed_db=seed_path, - md5_runtime_hash=runtime_hash, - md5_seed_hash=seed_hash, - md5_detail=md5_detail, + md5_status=db.status, + md5_source=db.source, + md5_runtime_db=db.runtime_db, + md5_seed_db=db.seed_db, + md5_runtime_hash=db.runtime_hash, + md5_seed_hash=db.seed_hash, + md5_detail=db.detail, ) @@ -362,10 +527,40 @@ def run_checks( timeout: float = 10.0, strict: bool = False, reset_all: bool = False, + db_root: str | None = None, + docker_container: str | None = None, + container_hasher: Any = None, ) -> SmokeResult: collector = Collector() control_url = normalize_control_url(control_url) - site_map = discover_sites(root) + + def empty(detail: str) -> SmokeResult: + return SmokeResult( + root=str(root), + control_url=control_url, + base_host=base_host, + timeout=timeout, + strict=strict, + reset_all=reset_all, + control_server=ControlCheck( + url=f"{control_url}/health", + status="SKIP", + http_status=None, + detail=detail, + ), + sites_discovered=0, + sites_checked=0, + site_checks=[], + errors=collector.errors, + warnings=collector.warnings, + ) + + try: + site_map = discover_sites(root) + except RegistryError as exc: + # Registry drift and unreadable registries are findings, not crashes. + collector.error(str(exc), file=str(root)) + return empty("site registry could not be resolved") if site is not None: if site not in site_map: collector.error(f"unknown site '{site}'") @@ -415,6 +610,9 @@ def run_checks( collector=collector, use_reset_all=reset_all, reset_all_ok=reset_all_ok, + db_root=db_root, + docker_container=docker_container, + container_hasher=container_hasher, ) for site_slug, port in filtered_sites.items() ] @@ -473,7 +671,7 @@ def render_human(result: SmokeResult) -> str: ) lines.append(f" reset: {site.reset_detail}") lines.append(f" home: {site.home_detail}") - lines.append(f" md5: {site.md5_detail}") + lines.append(f" md5: [{site.md5_source}] {site.md5_detail}") findings = [*result.errors, *result.warnings] if findings: @@ -511,6 +709,21 @@ def build_arg_parser() -> argparse.ArgumentParser: default=10.0, help="HTTP timeout in seconds for control and homepage checks", ) + parser.add_argument( + "--db-root", + help=( + "Deployment root holding /instance and /instance_seed. " + "Defaults to this checkout's sites/ directory, which is NOT what the " + "control plane resets when the environment runs in Docker." + ), + ) + parser.add_argument( + "--docker-container", + help=( + "Hash each site's DBs inside this running container under " + f"{DOCKER_SITE_ROOT}/, i.e. where the control plane actually resets them." + ), + ) parser.add_argument("--strict", action="store_true", help="Treat warnings as failures") parser.add_argument("--json", action="store_true", help="Print machine-readable JSON only") parser.add_argument( @@ -537,6 +750,8 @@ def main( timeout=args.timeout, strict=args.strict, reset_all=args.reset_all, + db_root=args.db_root, + docker_container=args.docker_container, ) stream = stdout if stdout is not None else sys.stdout if args.json: diff --git a/scripts/test_check_reset_smoke.py b/scripts/test_check_reset_smoke.py index 973f6147..f7ece8c1 100644 --- a/scripts/test_check_reset_smoke.py +++ b/scripts/test_check_reset_smoke.py @@ -146,33 +146,41 @@ def __exit__(self, exc_type, exc, tb) -> None: class CheckResetSmokeTests(unittest.TestCase): def test_md5_match_passes(self) -> None: + # A parity verdict needs a reset to be "after", so this runs against a live + # control plane rather than a dead port. with tempfile.TemporaryDirectory() as tmpdir: - root = Path(tmpdir) - build_repo(root) - result = smoke.run_checks( - root, - site="amazon", - control_url="http://127.0.0.1:9", - base_host="127.0.0.1", - timeout=0.1, - ) - self.assertEqual(result.site_checks[0].md5_status, "PASS") + with SmokeServer() as server: + root = Path(tmpdir) + build_repo(root, base_port=server.port) + result = smoke.run_checks( + root, + site="amazon", + control_url=f"http://127.0.0.1:{server.port}", + base_host="127.0.0.1", + timeout=2.0, + ) + self.assertEqual(result.site_checks[0].md5_status, "PASS") def test_md5_mismatch_fails(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: - root = Path(tmpdir) - build_repo(root, runtime_content=b"runtime", seed_content=b"seed") - result = smoke.run_checks( - root, - site="amazon", - control_url="http://127.0.0.1:9", - base_host="127.0.0.1", - timeout=0.1, - ) - self.assertEqual(result.site_checks[0].md5_status, "FAIL") - self.assertNotEqual(result.exit_code, 0) + with SmokeServer() as server: + root = Path(tmpdir) + build_repo(root, base_port=server.port, + runtime_content=b"runtime", seed_content=b"seed") + result = smoke.run_checks( + root, + site="amazon", + control_url=f"http://127.0.0.1:{server.port}", + base_host="127.0.0.1", + timeout=2.0, + ) + self.assertEqual(result.site_checks[0].md5_status, "FAIL") + self.assertNotEqual(result.exit_code, 0) - def test_missing_dbs_warn_not_error_by_default(self) -> None: + def test_missing_dbs_are_not_an_error_by_default(self) -> None: + """Original intent kept: absent local DBs must never be an error. The former + warning was dropped because the documented docker layout has no local + instance/, so warning on it made --strict fail a correct environment.""" with tempfile.TemporaryDirectory() as tmpdir: with SmokeServer() as server: root = Path(tmpdir) @@ -189,9 +197,12 @@ def test_missing_dbs_warn_not_error_by_default(self) -> None: base_host="127.0.0.1", timeout=2.0, ) - self.assertEqual(result.site_checks[0].md5_status, "SKIP") + check = result.site_checks[0] + self.assertEqual(check.md5_status, "SKIP") + self.assertEqual(check.md5_source, "none") self.assertEqual(result.exit_code, 0) - self.assertTrue(any("DB" in warning.message for warning in result.warnings)) + self.assertIn("no DB source configured", check.md5_detail) + self.assertIn("/opt/WebSyn/amazon/instance", check.md5_detail) def test_site_filtering_and_unknown_site(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: @@ -229,12 +240,13 @@ def test_strict_mode_treats_warnings_as_failure(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: with SmokeServer() as server: root = Path(tmpdir) - build_repo( - root, - base_port=server.port, - with_runtime_db=False, - with_seed_db=False, - ) + build_repo(root, base_port=server.port) + # Ambiguous DB pair is a genuine warning; --strict must escalate it. + for sub in ("instance", "instance_seed"): + d = root / "sites" / "amazon" / sub + (d / "extra.db").write_bytes((d / "amazon.db").read_bytes()) + (d / "amazon.db").unlink() + (d / "other.db").write_bytes(b"x") normal = smoke.run_checks( root, site="amazon", @@ -274,5 +286,188 @@ def test_http_reset_and_homepage_pass(self) -> None: self.assertEqual(result.site_checks[0].md5_status, "PASS") +class DbSourceTests(unittest.TestCase): + """The DB parity check must name what it hashed and never imply it observed + an environment it did not read.""" + + def test_result_reports_the_db_source_it_hashed(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + with SmokeServer() as server: + build_repo(root, base_port=server.port) + result = smoke.run_checks( + root, site="amazon", + control_url=f"http://127.0.0.1:{server.port}", + base_host="127.0.0.1", timeout=2.0, + ) + check = result.site_checks[0] + self.assertEqual(check.md5_status, "PASS") + self.assertTrue(check.md5_source.startswith("local:"), check.md5_source) + self.assertIn("md5_source", result.to_json_dict()["sites"][0]) + + def test_no_db_source_skips_without_warning(self) -> None: + """The documented docker layout has no local instance/. That is an expected + configuration, not a fault, so --strict must not fail on it.""" + with tempfile.TemporaryDirectory() as tmpdir: + with SmokeServer() as server: + root = Path(tmpdir) + build_repo(root, base_port=server.port, with_runtime_db=False, + with_seed_db=False) + result = smoke.run_checks( + root, site="amazon", + control_url=f"http://127.0.0.1:{server.port}", + base_host="127.0.0.1", timeout=2.0, strict=True, + ) + check = result.site_checks[0] + self.assertEqual(check.md5_status, "SKIP") + self.assertEqual(check.md5_source, "none") + self.assertEqual(result.warnings, []) + self.assertEqual(result.exit_code, 0) + + def test_explicit_db_root_with_missing_dirs_is_an_error(self) -> None: + """If the operator asked for the DB check, being unable to run it is a fault.""" + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + build_repo(root, with_runtime_db=False, with_seed_db=False) + result = smoke.run_checks( + root, site="amazon", control_url="http://127.0.0.1:9", + base_host="127.0.0.1", timeout=0.1, + db_root=str(root / "nowhere"), + ) + self.assertEqual(result.site_checks[0].md5_status, "FAIL") + self.assertNotEqual(result.exit_code, 0) + + def test_db_root_overrides_the_repo_checkout(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + with SmokeServer() as server: + build_repo(root, base_port=server.port, + runtime_content=b"stale", seed_content=b"seed") + deploy = root / "deployment" + for sub, content in (("instance", b"live"), ("instance_seed", b"live")): + d = deploy / "amazon" / sub + d.mkdir(parents=True) + (d / "amazon.db").write_bytes(content) + result = smoke.run_checks( + root, site="amazon", + control_url=f"http://127.0.0.1:{server.port}", + base_host="127.0.0.1", timeout=2.0, db_root=str(deploy), + ) + check = result.site_checks[0] + self.assertEqual(check.md5_status, "PASS") + self.assertIn("deployment", check.md5_source) + + def test_docker_container_source_hashes_inside_the_container(self) -> None: + calls = [] + + def fake_exec(container: str, paths: list[str]) -> dict[str, str]: + calls.append((container, list(paths))) + return {path: "deadbeef" for path in paths} + + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + build_repo(root, runtime_content=b"stale", seed_content=b"different") + result = smoke.run_checks( + root, site="amazon", control_url="http://127.0.0.1:9", + base_host="127.0.0.1", timeout=0.1, + docker_container="webharbor", container_hasher=fake_exec, + ) + check = result.site_checks[0] + self.assertEqual(check.md5_status, "PASS") + self.assertEqual(check.md5_source, "docker:webharbor:/opt/WebSyn/amazon") + self.assertEqual(calls[0][0], "webharbor") + + + def test_local_parity_is_not_reported_without_a_successful_reset(self) -> None: + """SC-04: with no environment running, a self-consistent local pair must not + be reported as a passing reset verification.""" + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + build_repo(root) + result = smoke.run_checks( + root, site="amazon", control_url="http://127.0.0.1:9", + base_host="127.0.0.1", timeout=0.1, + ) + check = result.site_checks[0] + self.assertEqual(check.reset_status, "FAIL") + self.assertEqual(check.md5_status, "SKIP") + self.assertIn("no successful reset", check.md5_detail) + + +class RegistryFailureTests(unittest.TestCase): + """Registry faults are the condition this checker exists to report; they must + come back as structured findings, never as an unhandled exception.""" + + def _run(self, root: Path): + buffer = io.StringIO() + code = smoke.main( + ["--control-url", "http://127.0.0.1:9", "--timeout", "0.1"], + root=root, stdout=buffer, + ) + return code, buffer.getvalue() + + def test_registry_drift_is_structured(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + build_repo(root, sites=["amazon"]) + write(root / "websyn_start.sh", """ + #!/bin/bash + SITES=(amazon ghost_site) + BASE_PORT=40000 + """) + code, out = self._run(root) + self.assertEqual(code, 1) + self.assertIn("out of sync", out) + self.assertIn("ghost_site", out) + + def test_missing_registry_file_is_structured(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + build_repo(root) + (root / "websyn_start.sh").unlink() + code, out = self._run(root) + self.assertEqual(code, 1) + self.assertIn("websyn_start.sh", out) + + def test_unparseable_sites_is_structured(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + build_repo(root) + write(root / "control_server.py", """ + SITES = dict(a=1) + BASE_PORT = 40000 + """) + code, out = self._run(root) + self.assertEqual(code, 1) + self.assertIn("Could not parse SITES", out) + + def test_base_port_mismatch_names_the_base_port(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + build_repo(root, sites=["amazon"]) + write(root / "websyn_start.sh", """ + #!/bin/bash + SITES=(amazon) + BASE_PORT=41000 + """) + code, out = self._run(root) + self.assertEqual(code, 1) + self.assertIn("BASE_PORT", out) + self.assertNotIn("registration lists are out of sync", out) + + +class ResetAllTests(unittest.TestCase): + def test_reset_all_failure_is_reported_once(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + build_repo(root, sites=["amazon", "apple"]) + result = smoke.run_checks( + root, control_url="http://127.0.0.1:9", base_host="127.0.0.1", + timeout=0.1, reset_all=True, + ) + reset_errors = [e for e in result.errors if "reset" in e.message] + self.assertEqual(len(reset_errors), 1, [e.message for e in reset_errors]) + + if __name__ == "__main__": unittest.main() From fda2266c6480dd2157cc7dad279ae7577e93cd56 Mon Sep 17 00:00:00 2001 From: JackJin Date: Sun, 13 Sep 2026 10:26:30 +0800 Subject: [PATCH 3/4] fix(check_reset_smoke): require the DB source to be asked for Follow-up to the previous commit, from an independent review of the frozen runs. The SKIP-with-source-none path was only reached when the checkout had no sites//instance directory. Any checkout that happened to carry one -- anyone who has run a site on the host, or extracted assets and booted locally -- silently fell back to comparing the checkout and issued a parity verdict labelled local:, contradicting the documented behaviour that a flagless run reports SKIP rather than comparing this checkout's files. Two concrete consequences, both reproduced against a live control plane: - a stale local DB failed a healthy deployment: reset=PASS, home=PASS, the deployment's own instance and instance_seed byte-identical, yet exit 1 on "local runtime DB differs from local seed DB" - a flagless --json run reported md5_status=PASS for every site without ever reading /opt/WebSyn -- the same false confidence the previous commit set out to remove, just narrowed to checkouts that have an instance/ directory A flagless run now always reports SKIP with source none. Under --db-root, a root that does not hold the site's DBs is an error (the requested check cannot run), while an undecidable DB pair stays a warning. --reset-all failures now carry the HTTP detail instead of discarding it. Tests: 2 added for the flagless contract; the tests that exercise the local source now pass --db-root explicitly. 20 pass. Co-Authored-By: Claude Opus 5 --- scripts/check_reset_smoke.py | 46 +++++++++++++++---------------- scripts/test_check_reset_smoke.py | 42 ++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 23 deletions(-) diff --git a/scripts/check_reset_smoke.py b/scripts/check_reset_smoke.py index c373ce36..c76ad06d 100644 --- a/scripts/check_reset_smoke.py +++ b/scripts/check_reset_smoke.py @@ -305,7 +305,6 @@ def docker_md5(container: str, dirs: list[str]) -> dict[str, str]: def check_db_parity( site: str, *, - repo_root: Path, db_root: str | None, docker_container: str | None, container_hasher: Any, @@ -340,34 +339,36 @@ def check_db_parity( return DbCheck("FAIL", source, runtime_dir, seed_dir, runtime_hash, seed_hash, detail) - explicit = db_root is not None - base_dir = Path(db_root) if explicit else repo_root / "sites" + if db_root is None: + # The DB source has to be asked for. Falling back to this checkout whenever it + # happens to carry an instance/ directory would produce a parity verdict -- + # green or red -- without ever reading the DBs the control plane resets. + return DbCheck( + "SKIP", "none", None, None, None, None, + "no DB source configured; the control plane resets " + f"{DOCKER_SITE_ROOT}/{site}/instance inside the deployment. " + "Pass --docker-container or --db-root to check DB parity.", + ) + + base_dir = Path(db_root) site_root = base_dir / site + source = f"local:{site_root}" runtime_db, seed_db, problem = resolve_db_pair(site_root, site) if problem: - source = f"local:{site_root}" - if explicit: - detail = f"{problem} under --db-root {base_dir}" + detail = f"{problem} under --db-root {base_dir}" + if "missing" in problem: + # The caller pointed at a root that does not hold this site's DBs, so the + # check they asked for cannot run at all. collector.error(detail, site=site, file=str(site_root)) return DbCheck("FAIL", source, None, None, None, None, detail) - if "missing locally" in problem: - # The documented docker workflow keeps instance/ inside the container, so - # its absence here is an expected configuration rather than a fault. - return DbCheck( - "SKIP", "none", None, None, None, None, - "no DB source configured; the control plane resets " - f"{DOCKER_SITE_ROOT}/{site}/instance inside the deployment. " - "Pass --docker-container or --db-root to check DB parity.", - ) - collector.warn(problem, site=site, file=str(site_root)) - return DbCheck("SKIP", source, None, None, None, None, problem) + # The root is plausible but this site's DB pair is undecidable; report it + # rather than guessing which file to compare. + collector.warn(detail, site=site, file=str(site_root)) + return DbCheck("SKIP", source, None, None, None, None, detail) assert runtime_db is not None and seed_db is not None - source = f"local:{site_root}" if not reset_succeeded: - # There is no reset to attribute a parity verdict to, and this checkout is not - # necessarily the reset target, so reporting PASS here would assert something - # that was never observed. + # Nothing to attribute a parity verdict to. return DbCheck( "SKIP", source, str(runtime_db), str(seed_db), None, None, "no successful reset to verify; DB parity not evaluated", @@ -490,7 +491,6 @@ def check_site( db = check_db_parity( site, - repo_root=root, db_root=db_root, docker_container=docker_container, container_hasher=container_hasher, @@ -595,7 +595,7 @@ def empty(detail: str) -> SmokeResult: if ok: reset_all_ok = True else: - collector.error("reset-all request failed", url=reset_all_url) + collector.error(f"reset-all request failed: {detail}", url=reset_all_url) if status_code == 404: collector.warn("control server does not expose /reset-all", url=reset_all_url) diff --git a/scripts/test_check_reset_smoke.py b/scripts/test_check_reset_smoke.py index f7ece8c1..da4d9158 100644 --- a/scripts/test_check_reset_smoke.py +++ b/scripts/test_check_reset_smoke.py @@ -158,6 +158,7 @@ def test_md5_match_passes(self) -> None: control_url=f"http://127.0.0.1:{server.port}", base_host="127.0.0.1", timeout=2.0, + db_root=str(root / "sites"), ) self.assertEqual(result.site_checks[0].md5_status, "PASS") @@ -173,6 +174,7 @@ def test_md5_mismatch_fails(self) -> None: control_url=f"http://127.0.0.1:{server.port}", base_host="127.0.0.1", timeout=2.0, + db_root=str(root / "sites"), ) self.assertEqual(result.site_checks[0].md5_status, "FAIL") self.assertNotEqual(result.exit_code, 0) @@ -254,6 +256,7 @@ def test_strict_mode_treats_warnings_as_failure(self) -> None: base_host="127.0.0.1", timeout=2.0, strict=False, + db_root=str(root / "sites"), ) strict = smoke.run_checks( root, @@ -262,6 +265,7 @@ def test_strict_mode_treats_warnings_as_failure(self) -> None: base_host="127.0.0.1", timeout=2.0, strict=True, + db_root=str(root / "sites"), ) self.assertEqual(normal.strict, False) self.assertEqual(normal.exit_code, 0) @@ -279,6 +283,7 @@ def test_http_reset_and_homepage_pass(self) -> None: control_url=f"http://127.0.0.1:{server.port}", base_host="127.0.0.1", timeout=2.0, + db_root=str(root / "sites"), ) self.assertEqual(result.control_server.status, "PASS") self.assertEqual(result.site_checks[0].reset_status, "PASS") @@ -299,6 +304,7 @@ def test_result_reports_the_db_source_it_hashed(self) -> None: root, site="amazon", control_url=f"http://127.0.0.1:{server.port}", base_host="127.0.0.1", timeout=2.0, + db_root=str(root / "sites"), ) check = result.site_checks[0] self.assertEqual(check.md5_status, "PASS") @@ -324,6 +330,41 @@ def test_no_db_source_skips_without_warning(self) -> None: self.assertEqual(result.warnings, []) self.assertEqual(result.exit_code, 0) + def test_flagless_run_skips_even_when_the_checkout_has_an_instance_dir(self) -> None: + """A checkout that happens to carry sites//instance must not turn a + flagless run into a parity verdict. The DB source has to be asked for, or the + utility produces a green result without reading what the control plane resets.""" + with tempfile.TemporaryDirectory() as tmpdir: + with SmokeServer() as server: + root = Path(tmpdir) + build_repo(root, base_port=server.port) # instance/ present and matching + result = smoke.run_checks( + root, site="amazon", + control_url=f"http://127.0.0.1:{server.port}", + base_host="127.0.0.1", timeout=2.0, + ) + check = result.site_checks[0] + self.assertEqual(check.reset_status, "PASS") + self.assertEqual(check.md5_status, "SKIP") + self.assertEqual(check.md5_source, "none") + self.assertIsNone(check.md5_runtime_hash) + self.assertEqual(result.exit_code, 0) + + def test_flagless_run_does_not_fail_on_a_stale_checkout(self) -> None: + """RS-C04: a stale local DB must not fail a healthy deployment.""" + with tempfile.TemporaryDirectory() as tmpdir: + with SmokeServer() as server: + root = Path(tmpdir) + build_repo(root, base_port=server.port, + runtime_content=b"stale", seed_content=b"seed") + result = smoke.run_checks( + root, site="amazon", + control_url=f"http://127.0.0.1:{server.port}", + base_host="127.0.0.1", timeout=2.0, + ) + self.assertEqual(result.site_checks[0].md5_status, "SKIP") + self.assertEqual(result.exit_code, 0) + def test_explicit_db_root_with_missing_dirs_is_an_error(self) -> None: """If the operator asked for the DB check, being unable to run it is a fault.""" with tempfile.TemporaryDirectory() as tmpdir: @@ -387,6 +428,7 @@ def test_local_parity_is_not_reported_without_a_successful_reset(self) -> None: result = smoke.run_checks( root, site="amazon", control_url="http://127.0.0.1:9", base_host="127.0.0.1", timeout=0.1, + db_root=str(root / "sites"), ) check = result.site_checks[0] self.assertEqual(check.reset_status, "FAIL") From f3a799df518292075361e2699bdcf2687f99421c Mon Sep 17 00:00:00 2001 From: JackJin Date: Sun, 13 Sep 2026 11:32:21 +0800 Subject: [PATCH 4/4] docs(check_reset_smoke): correct the --db-root help text The second independent review of the frozen runs flagged that the shipped contract contradicted itself: the README says a flagless run skips the DB check, while --db-root's help still claimed it "defaults to this checkout's sites/ directory". That default was removed in the previous commit, so anyone reading --help would believe the check runs against the checkout when it does not. Also drops the now-dead root parameter from check_site(), unused since the DB source stopped being derived from the repository root. No behaviour change; 20 tests pass. Co-Authored-By: Claude Opus 5 --- scripts/check_reset_smoke.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/scripts/check_reset_smoke.py b/scripts/check_reset_smoke.py index c76ad06d..de1b93e0 100644 --- a/scripts/check_reset_smoke.py +++ b/scripts/check_reset_smoke.py @@ -442,7 +442,6 @@ def check_control_health(control_url: str, timeout: float, collector: Collector) def check_site( - root: Path, site: str, port: int, *, @@ -601,7 +600,6 @@ def empty(detail: str) -> SmokeResult: site_checks = [ check_site( - root, site_slug, port, control_url=control_url, @@ -712,9 +710,10 @@ def build_arg_parser() -> argparse.ArgumentParser: parser.add_argument( "--db-root", help=( - "Deployment root holding /instance and /instance_seed. " - "Defaults to this checkout's sites/ directory, which is NOT what the " - "control plane resets when the environment runs in Docker." + "Deployment root holding /instance and /instance_seed, for " + "sites deployed on this host. There is no default: without this or " + "--docker-container the DB check is skipped, because this checkout is not " + "what the control plane resets when the environment runs in Docker." ), ) parser.add_argument(