diff --git a/.gitattributes b/.gitattributes index ea6f5acdbc..0385221a0a 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1 +1,2 @@ scripts/release/update-homebrew-tap.sh linguist-language=Shell +docs/specs/spec-lint-exceptions.tsv merge=union diff --git a/docs/specs/guide/structure-and-ownership.md b/docs/specs/guide/structure-and-ownership.md index 80da88c5bd..3dd3296cec 100644 --- a/docs/specs/guide/structure-and-ownership.md +++ b/docs/specs/guide/structure-and-ownership.md @@ -111,7 +111,7 @@ capability. ## Context limits -The specification linter reads the limits from `docs/specs/spec-lint.json`. +The specification linter reads the default limits from `docs/specs/spec-lint.json`. The default limits are: - System index: 12 KiB. @@ -124,7 +124,9 @@ The default limits are: Split a file before it reaches its limit. Split by capability, lifecycle, or contract boundary. Do not split by arbitrary line ranges. -An oversized legacy file can have a frozen ceiling in the linter configuration. +An oversized legacy file can have a frozen ceiling registered in +`docs/specs/spec-lint-exceptions.tsv` (one `pathsize` record per line). +The path must identify a regular legacy Markdown file under `docs/specs`. The ceiling permits migration work but does not permit growth. Lower the ceiling in the same change whenever the file shrinks. Remove the exception after the file falls below the default limit. diff --git a/docs/specs/spec-lint-exceptions.tsv b/docs/specs/spec-lint-exceptions.tsv new file mode 100644 index 0000000000..6361fd7967 --- /dev/null +++ b/docs/specs/spec-lint-exceptions.tsv @@ -0,0 +1,4 @@ +docs/specs/office-task-content-editing/spec.md 84103 +docs/specs/task-cost-ledger/spec.md 91931 +docs/specs/task-delivery-ledger/spec.md 157207 +docs/specs/workflow-on-enter-action-dispatch/spec.md 116827 diff --git a/docs/specs/spec-lint.json b/docs/specs/spec-lint.json index a459fd12ec..d5a008060f 100644 --- a/docs/specs/spec-lint.json +++ b/docs/specs/spec-lint.json @@ -8,11 +8,5 @@ "system-design": 32768, "system-index": 12288, "template": 8192 - }, - "legacy_size_exceptions": { - "docs/specs/office-task-content-editing/spec.md": 84103, - "docs/specs/task-cost-ledger/spec.md": 91931, - "docs/specs/task-delivery-ledger/spec.md": 157207, - "docs/specs/workflow-on-enter-action-dispatch/spec.md": 116827 } } diff --git a/scripts/lint-spec-files.py b/scripts/lint-spec-files.py index 7c65800306..6b56c57eb2 100644 --- a/scripts/lint-spec-files.py +++ b/scripts/lint-spec-files.py @@ -22,6 +22,7 @@ VALID_REQUIREMENT_STATUSES = {"draft", "active", "deprecated"} VALID_DESIGN_STATUSES = {"draft", "current", "superseded"} VALID_SYSTEM_STATUSES = {"draft", "active", "retired"} VALID_MIGRATION_STATUSES = {"in_progress", "complete"} +SIZE_EXCEPTIONS_PATH = "docs/specs/spec-lint-exceptions.tsv" REQ_ID_PATTERN = r"REQ-[A-Z0-9]+(?:-[A-Z0-9]+)*-\d{3}" AC_ID_PATTERN = r"AC-[A-Z0-9]+(?:-[A-Z0-9]+)*-\d{3}\.\d+" REQ_HEADING = re.compile(rf"^###\s+(?P{REQ_ID_PATTERN}):\s+\S") @@ -33,13 +34,23 @@ def load_config(root: Path) -> dict: return json.loads(path.read_text(encoding="utf-8")) -def lint_specs(root: Path, config: dict | None = None) -> list[Violation]: +def lint_specs( + root: Path, + config: dict | None = None, + exceptions: dict[str, int] | None = None, +) -> list[Violation]: config = config or load_config(root) validate_config(config) - previous_config = load_previous_config(root) + exceptions_provided_by_caller = exceptions is not None + exception_violations: list[Violation] = [] + if exceptions is None: + exceptions, exception_violations = load_size_exceptions(root) + previous_exceptions = ( + None if exceptions_provided_by_caller else load_previous_size_exceptions(root) + ) specs_root = root / "docs/specs" if not specs_root.exists(): - return [] + return exception_violations paths = sorted(path for path in specs_root.rglob("*.md") if path.is_file()) migrated_systems = set() @@ -69,7 +80,7 @@ def lint_specs(root: Path, config: dict | None = None) -> list[Violation]: for path in paths: relative = path.relative_to(root) kind, system = classify_path(relative) - violations.extend(check_size(path, relative, kind, config)) + violations.extend(check_size(path, relative, kind, config, exceptions)) inside = relative.relative_to("docs/specs").parts if inside and inside[0] in migrated_systems and kind == "legacy": violations.append( @@ -196,14 +207,20 @@ def lint_specs(root: Path, config: dict | None = None) -> list[Violation]: ) ) - violations.extend(check_size_exception_catalog(root, config)) - violations.extend(check_legacy_size_ratchet(root, config, previous_config)) + violations.extend(check_size_exception_catalog(root, config, exceptions)) + violations.extend(check_legacy_size_ratchet(root, exceptions, previous_exceptions)) + violations.extend(exception_violations) return sorted(violations, key=lambda item: (item.path.as_posix(), item.line, item.rule)) def validate_config(config: dict) -> None: if config.get("version") != 1: raise ValueError("specification lint configuration must use version 1") + if "legacy_size_exceptions" in config: + raise ValueError( + "specification lint configuration must not contain legacy_size_exceptions; " + f"register exceptions in {SIZE_EXCEPTIONS_PATH} instead" + ) required_limits = { "guide", "legacy", @@ -224,8 +241,57 @@ def validate_config(config: dict) -> None: raise ValueError("specification size limits must be positive integers") -def load_previous_config(root: Path) -> dict | None: - """Load the nearest prior config so legacy ceilings cannot increase silently.""" +def parse_size_exceptions(text: str, source: Path) -> tuple[dict[str, int], list[Violation]]: + """Parse `pathsize` records, one per line, flagging malformed or duplicate lines.""" + exceptions: dict[str, int] = {} + violations: list[Violation] = [] + for line_number, raw_line in enumerate(text.splitlines(), start=1): + if not raw_line.strip(): + continue + parts = raw_line.split("\t") + if len(parts) != 2 or not parts[0] or not re.fullmatch(r"[0-9]+", parts[1]): + violations.append( + Violation( + "malformed-size-exception", + source, + line_number, + "expected `pathsize` with an ASCII decimal size", + ) + ) + continue + relative_text, size_text = parts + if relative_text in exceptions: + violations.append( + Violation( + "duplicate-size-exception", + source, + line_number, + f"{relative_text} already has a frozen ceiling of {exceptions[relative_text]} bytes", + ) + ) + continue + exceptions[relative_text] = int(size_text) + return exceptions, violations + + +def load_size_exceptions(root: Path) -> tuple[dict[str, int], list[Violation]]: + path = root / SIZE_EXCEPTIONS_PATH + if not path.exists() and not path.is_symlink(): + return {}, [] + if not is_safe_regular_file(path, root): + return {}, [ + Violation( + "invalid-size-exception-catalog", + path, + 1, + "size exception catalog must be a regular file inside the repository", + ) + ] + return parse_size_exceptions(path.read_text(encoding="utf-8"), path) + + +def load_previous_size_exceptions(root: Path) -> dict[str, int] | None: + """Load the nearest prior sidecar so legacy ceilings cannot increase silently.""" try: parents_output = subprocess.run( ["git", "rev-list", "--parents", "-n", "1", "HEAD"], @@ -255,18 +321,17 @@ def load_previous_config(root: Path) -> dict | None: for revision in candidates: try: - config_text = subprocess.run( - ["git", "show", f"{revision}:docs/specs/spec-lint.json"], + exceptions_text = subprocess.run( + ["git", "show", f"{revision}:{SIZE_EXCEPTIONS_PATH}"], cwd=root, check=True, capture_output=True, text=True, ).stdout - previous_config = json.loads(config_text) - except (OSError, subprocess.CalledProcessError, json.JSONDecodeError): + except (OSError, subprocess.CalledProcessError): continue - if isinstance(previous_config, dict): - return previous_config + exceptions, _ = parse_size_exceptions(exceptions_text, root / SIZE_EXCEPTIONS_PATH) + return exceptions return None @@ -294,12 +359,35 @@ def classify_path(relative: Path) -> tuple[str, str | None]: return "legacy", None -def check_size(path: Path, relative: Path, kind: str, config: dict) -> list[Violation]: +def is_safe_regular_file(path: Path, root: Path) -> bool: + if path.is_symlink() or not path.is_file(): + return False + try: + path.resolve(strict=True).relative_to(root.resolve()) + except (OSError, ValueError): + return False + return True + + +def is_canonical_legacy_path(relative_text: str, relative: Path) -> bool: + return ( + "\x00" not in relative_text + and not relative.is_absolute() + and relative.as_posix() == relative_text + and relative.parts[:2] == ("docs", "specs") + and relative.suffix == ".md" + and all(part not in {".", ".."} for part in relative.parts) + ) + + +def check_size( + path: Path, relative: Path, kind: str, config: dict, exceptions: dict[str, int] +) -> list[Violation]: if kind == "unknown": return [] size = path.stat().st_size default_limit = config["limits"][kind] - exception = config.get("legacy_size_exceptions", {}).get(relative.as_posix()) + exception = exceptions.get(relative.as_posix()) limit = exception if kind == "legacy" and exception is not None else default_limit if size <= limit: return [] @@ -314,11 +402,24 @@ def check_size(path: Path, relative: Path, kind: str, config: dict) -> list[Viol ] -def check_size_exception_catalog(root: Path, config: dict) -> list[Violation]: +def check_size_exception_catalog( + root: Path, config: dict, exceptions: dict[str, int] +) -> list[Violation]: violations = [] default_limit = config["limits"]["legacy"] - for relative_text, ceiling in config.get("legacy_size_exceptions", {}).items(): + for relative_text, ceiling in exceptions.items(): relative = Path(relative_text) + if not is_canonical_legacy_path(relative_text, relative): + violations.append( + Violation( + "invalid-size-exception", + root / SIZE_EXCEPTIONS_PATH, + 1, + "size exceptions must reference canonical legacy Markdown files under " + f"docs/specs (got `{relative_text}`)", + ) + ) + continue path = root / relative if not path.exists(): violations.append( @@ -330,6 +431,16 @@ def check_size_exception_catalog(root: Path, config: dict) -> list[Violation]: ) ) continue + if not is_safe_regular_file(path, root / "docs/specs"): + violations.append( + Violation( + "invalid-size-exception", + path, + 1, + "size exceptions must reference regular files inside docs/specs", + ) + ) + continue kind, _ = classify_path(relative) if kind != "legacy": violations.append( @@ -364,25 +475,27 @@ def check_size_exception_catalog(root: Path, config: dict) -> list[Violation]: def check_legacy_size_ratchet( - root: Path, config: dict, previous_config: dict | None + root: Path, exceptions: dict[str, int], previous_exceptions: dict[str, int] | None ) -> list[Violation]: - if previous_config is None: - return [] - previous_exceptions = previous_config.get("legacy_size_exceptions", {}) - current_exceptions = config.get("legacy_size_exceptions", {}) - if not isinstance(previous_exceptions, dict) or not isinstance(current_exceptions, dict): + if previous_exceptions is None: return [] violations = [] for relative_text, previous_ceiling in previous_exceptions.items(): - current_ceiling = current_exceptions.get(relative_text) - if not isinstance(previous_ceiling, int) or not isinstance(current_ceiling, int): + current_ceiling = exceptions.get(relative_text) + if current_ceiling is None: continue if current_ceiling > previous_ceiling: + relative = Path(relative_text) + violation_path = ( + root / relative + if is_canonical_legacy_path(relative_text, relative) + else root / SIZE_EXCEPTIONS_PATH + ) violations.append( Violation( "legacy-size-ratchet", - root / relative_text, + violation_path, 1, f"frozen ceiling increased from {previous_ceiling} to {current_ceiling}; lower it or migrate the file", ) diff --git a/scripts/lint-spec-files.test.py b/scripts/lint-spec-files.test.py index ebfb77569c..0ce542bdea 100644 --- a/scripts/lint-spec-files.test.py +++ b/scripts/lint-spec-files.test.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 import importlib.util +import subprocess import tempfile import unittest from pathlib import Path @@ -34,7 +35,6 @@ class SpecLinterTest(unittest.TestCase): "system-index": 200, "template": 100, }, - "legacy_size_exceptions": {}, } def write(self, relative: str, content: str) -> Path: @@ -43,6 +43,15 @@ class SpecLinterTest(unittest.TestCase): path.write_text(content, encoding="utf-8") return path + def git(self, *args: str) -> None: + subprocess.run( + ["git", *args], + cwd=self.root, + check=True, + capture_output=True, + text=True, + ) + def valid_requirement(self) -> str: return """--- status: active @@ -103,31 +112,31 @@ migration: in_progress def test_freezes_an_oversized_legacy_file_at_its_recorded_size(self) -> None: path = self.write("docs/specs/legacy/spec.md", "x" * 12) self.config["limits"]["legacy"] = 10 - self.config["legacy_size_exceptions"] = {"docs/specs/legacy/spec.md": 12} + exceptions = {"docs/specs/legacy/spec.md": 12} linter = load_linter() - self.assertEqual(linter.lint_specs(self.root, self.config), []) + self.assertEqual(linter.lint_specs(self.root, self.config, exceptions), []) path.write_text("x" * 13, encoding="utf-8") - violations = linter.lint_specs(self.root, self.config) + violations = linter.lint_specs(self.root, self.config, exceptions) self.assertIn("file-size", self.rules(violations)) self.assertIn("frozen ceiling 12", violations[0].message) def test_reports_a_stale_legacy_size_exception(self) -> None: self.write("docs/specs/legacy/spec.md", "small") self.config["limits"]["legacy"] = 10 - self.config["legacy_size_exceptions"] = {"docs/specs/legacy/spec.md": 12} + exceptions = {"docs/specs/legacy/spec.md": 12} - violations = load_linter().lint_specs(self.root, self.config) + violations = load_linter().lint_specs(self.root, self.config, exceptions) self.assertEqual(self.rules(violations), ["stale-size-exception"]) def test_requires_the_legacy_ceiling_to_follow_every_size_reduction(self) -> None: self.write("docs/specs/legacy/spec.md", "x" * 11) self.config["limits"]["legacy"] = 10 - self.config["legacy_size_exceptions"] = {"docs/specs/legacy/spec.md": 12} + exceptions = {"docs/specs/legacy/spec.md": 12} - violations = load_linter().lint_specs(self.root, self.config) + violations = load_linter().lint_specs(self.root, self.config, exceptions) self.assertEqual(self.rules(violations), ["stale-size-exception"]) self.assertIn("Lower the frozen ceiling to 11", violations[0].message) @@ -135,30 +144,141 @@ migration: in_progress def test_reports_a_redundant_legacy_size_exception(self) -> None: self.write("docs/specs/legacy/spec.md", "x" * 8) self.config["limits"]["legacy"] = 10 - self.config["legacy_size_exceptions"] = {"docs/specs/legacy/spec.md": 8} + exceptions = {"docs/specs/legacy/spec.md": 8} - violations = load_linter().lint_specs(self.root, self.config) + violations = load_linter().lint_specs(self.root, self.config, exceptions) self.assertEqual(self.rules(violations), ["stale-size-exception"]) self.assertIn("at or below the default limit", violations[0].message) def test_rejects_a_raised_legacy_size_ceiling(self) -> None: - path = self.write("docs/specs/legacy/spec.md", "x" * 13) - previous_config = { - **self.config, - "legacy_size_exceptions": {"docs/specs/legacy/spec.md": 12}, - } - current_config = { - **self.config, - "legacy_size_exceptions": {"docs/specs/legacy/spec.md": 13}, - } + previous_exceptions = {"docs/specs/legacy/spec.md": 12} + current_exceptions = {"docs/specs/legacy/spec.md": 13} violations = load_linter().check_legacy_size_ratchet( - self.root, current_config, previous_config + self.root, current_exceptions, previous_exceptions ) self.assertEqual(self.rules(violations), ["legacy-size-ratchet"]) - self.assertEqual(violations[0].path, path) + self.assertEqual(violations[0].path, self.root / "docs/specs/legacy/spec.md") + + def test_rejects_a_raised_absolute_ceiling_without_crashing(self) -> None: + previous_exceptions = {"/etc/passwd": 100} + current_exceptions = {"/etc/passwd": 200} + + violations = load_linter().check_legacy_size_ratchet( + self.root, current_exceptions, previous_exceptions + ) + + self.assertEqual(self.rules(violations), ["legacy-size-ratchet"]) + # main() calls violation.path.relative_to(root) on every violation; a + # noncanonical (e.g. absolute) historical path must not produce a + # violation path that raises there. + violations[0].path.relative_to(self.root) + + def test_loads_the_previous_ceiling_from_git_history_and_flags_a_raise(self) -> None: + self.config["limits"]["legacy"] = 10 + self.git("init") + self.git("config", "user.email", "test@example.com") + self.git("config", "user.name", "Spec Lint Test") + self.write("docs/specs/spec-lint-exceptions.tsv", "docs/specs/legacy/spec.md\t12\n") + self.write("docs/specs/legacy/spec.md", "x" * 12) + self.git("add", "-A") + self.git("commit", "-m", "freeze the legacy ceiling at 12") + self.write("README.md", "placeholder\n") + self.git("add", "-A") + self.git("commit", "-m", "unrelated follow-up commit") + + self.write("docs/specs/spec-lint-exceptions.tsv", "docs/specs/legacy/spec.md\t13\n") + self.write("docs/specs/legacy/spec.md", "x" * 13) + + violations = load_linter().lint_specs(self.root, self.config) + + self.assertIn("legacy-size-ratchet", self.rules(violations)) + + def test_loads_a_valid_sidecar_from_disk_and_enforces_its_ceiling(self) -> None: + self.write("docs/specs/spec-lint-exceptions.tsv", "docs/specs/legacy/spec.md\t12\n") + path = self.write("docs/specs/legacy/spec.md", "x" * 12) + self.config["limits"]["legacy"] = 10 + + self.assertEqual(load_linter().lint_specs(self.root, self.config), []) + + path.write_text("x" * 13, encoding="utf-8") + violations = load_linter().lint_specs(self.root, self.config) + self.assertIn("file-size", self.rules(violations)) + self.assertIn("frozen ceiling 12", violations[0].message) + + def test_rejects_a_symlinked_size_exception_sidecar(self) -> None: + outside = tempfile.TemporaryDirectory() + self.addCleanup(outside.cleanup) + target = Path(outside.name) / "not-a-sidecar.txt" + target.write_text("not a sidecar\n", encoding="utf-8") + sidecar = self.root / "docs/specs/spec-lint-exceptions.tsv" + sidecar.parent.mkdir(parents=True, exist_ok=True) + sidecar.symlink_to(target) + + violations = load_linter().lint_specs(self.root, self.config) + + self.assertEqual(self.rules(violations), ["invalid-size-exception-catalog"]) + + def test_rejects_noncanonical_size_exception_targets(self) -> None: + self.write( + "docs/specs/spec-lint-exceptions.tsv", + "other/specs/not-a-spec.md\t100\n" + "docs/specs/legacy/not-a-spec.txt\t100\n", + ) + self.write("other/specs/not-a-spec.md", "x") + self.write("docs/specs/legacy/not-a-spec.txt", "x") + + violations = load_linter().lint_specs(self.root, self.config) + + self.assertEqual(self.rules(violations), ["invalid-size-exception", "invalid-size-exception"]) + + def test_rejects_an_absolute_size_exception_target_without_crashing(self) -> None: + self.write("docs/specs/spec-lint-exceptions.tsv", "/etc/passwd\t100\n") + + violations = load_linter().lint_specs(self.root, self.config) + + self.assertEqual(self.rules(violations), ["invalid-size-exception"]) + # main() calls violation.path.relative_to(root) on every violation; an + # absolute (or otherwise out-of-tree) target must not produce a violation + # path that raises there. + for violation in violations: + violation.path.relative_to(self.root) + + def test_flags_a_malformed_size_exception_line(self) -> None: + self.write("docs/specs/spec-lint-exceptions.tsv", "docs/specs/legacy/spec.md\n") + self.write("docs/specs/legacy/spec.md", "x" * 5) + + violations = load_linter().lint_specs(self.root, self.config) + + self.assertIn("malformed-size-exception", self.rules(violations)) + + def test_flags_a_non_ascii_digit_as_a_malformed_size_exception_line(self) -> None: + self.write("docs/specs/spec-lint-exceptions.tsv", "docs/specs/legacy/spec.md\t²\n") + self.write("docs/specs/legacy/spec.md", "x" * 5) + + violations = load_linter().lint_specs(self.root, self.config) + + self.assertIn("malformed-size-exception", self.rules(violations)) + + def test_flags_a_duplicate_size_exception_path(self) -> None: + self.config["limits"]["legacy"] = 11 + self.write( + "docs/specs/spec-lint-exceptions.tsv", + "docs/specs/legacy/spec.md\t12\ndocs/specs/legacy/spec.md\t13\n", + ) + self.write("docs/specs/legacy/spec.md", "x" * 12) + + violations = load_linter().lint_specs(self.root, self.config) + + self.assertEqual(self.rules(violations), ["duplicate-size-exception"]) + + def test_rejects_a_residual_legacy_size_exceptions_key_in_config(self) -> None: + self.config["legacy_size_exceptions"] = {} + + with self.assertRaisesRegex(ValueError, "legacy_size_exceptions"): + load_linter().lint_specs(self.root, self.config) def test_rejects_a_system_artifact_without_a_system_index(self) -> None: self.write("docs/specs/task-system/requirements/dependencies.md", self.valid_requirement())