fix: move spec-lint size exceptions to a merge-friendly sidecar (#3207)

Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
Henry
2026-09-01 07:19:03 +00:00
committed by GitHub
parent 095f7e2d1a
commit 9d5c8d308b
6 changed files with 290 additions and 56 deletions
+140 -27
View File
@@ -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<id>{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 `path<TAB>size` 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 `path<TAB>size` 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",
)
+141 -21
View File
@@ -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())