diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ccac65..5269b67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ Notable changes to Agent Code Guard are recorded here. ## Unreleased +### Added + +- Opt-in `guards.loc.ratchetAt: "review"` support freezes source-controlled LOC + allowances beginning above the effective review threshold, while omitted or + explicit `"fail"` preserves the existing failure-only lifecycle and output. + ### Fixed - Known per-file syntax and provider failures now produce blocking structured diff --git a/README.md b/README.md index ec0905d..7059232 100644 --- a/README.md +++ b/README.md @@ -228,6 +228,9 @@ Built-in defaults require no configuration. Configure a project only for a concrete policy reason; see the [configuration guide](https://github.com/stef-k/agent-code-guard/blob/main/docs/configuration.md). The LOC baseline is an explicit adoption tool for established legacy repositories, not an ordinary-use requirement or a way to silence findings. +Its default `ratchetAt: "fail"` policy freezes files above `failAt`; choose +`ratchetAt: "review"` only when every established file above `warnAt` must be +non-increasing while ordinary findings in that range remain REVIEW. ## Trust, CI, and platform support diff --git a/docs/configuration.md b/docs/configuration.md index 40d5e41..6c50f06 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -30,7 +30,7 @@ The six guard keys and their production defaults are: | Key | Default | | --- | --- | -| `loc` | `warnAt: 400`, `failAt: 600` | +| `loc` | `warnAt: 400`, `failAt: 600`, `ratchetAt: "fail"` | | `callableSize` | `enabled: true`, `reviewAt: 80` | | `nesting` | `enabled: true`, `reviewAt: 4` | | `cyclomaticComplexity` | `enabled: true`, `reviewAt: 15` | @@ -42,7 +42,7 @@ passes. The five `reviewAt` guards are REVIEW-only. Only `loc` can FAIL. Set `enabled` to `false` to disable a guard. For REVIEW-only guards, `reviewAt` must be a positive integer when enabled. LOC supports its established options, -including `enabled`, `warnAt`, `failAt`, line-count settings, extension policy, +including `enabled`, `warnAt`, `failAt`, `ratchetAt`, line-count settings, extension policy, allowed large files, and path-specific overrides. Example with one deliberate threshold change: @@ -104,9 +104,11 @@ applicable guards. ## Source-controlled LOC ratchet -The legacy-adoption ratchet is not a configuration property. Its only location -is `.agent-tools/code-guard.loc-baseline.json` at the owning analysis root, and -its complete version-1 schema is: +The allowance data lives only in `.agent-tools/code-guard.loc-baseline.json` at +the owning analysis root. `guards.loc.ratchetAt` controls how that unchanged +version-1 data is interpreted: omitted or `"fail"` records and enforces files +above effective `failAt`; `"review"` starts above effective `warnAt`. Its complete +persisted schema is: ```json { @@ -128,7 +130,8 @@ duplicate, absolute, or unsafe paths fail closed. Writers use UTF-8, two-space indentation, LF endings, and a final newline. `--create-loc-baseline` records only selected, applicable files currently -strictly above their effective `failAt`, after exclusions, line-count options, +strictly above their effective policy threshold (`failAt` for `fail`, `warnAt` +for `review`), after exclusions, line-count options, CLI thresholds, and the last matching override. `allowedLargeFiles` entries are not recorded. `--update-loc-baseline` can only lower or remove existing entries within its positional bounds; it cannot add or increase one. Normal analysis @@ -139,7 +142,10 @@ project threshold differs, and `allowedLargeFiles` for a reviewed static exemption. The ratchet instead preserves an exact legacy maximum and rejects growth; overlap with `allowedLargeFiles` is invalid. Manual ratchet edits require normal source-control review. This facility is for established repositories, -not new projects, which should meet LOC policy directly. +not new projects, which should meet LOC policy directly. Choose `fail` to freeze +only existing hard failures; choose `review` when established review-level files +must also be non-increasing. Never change the policy, thresholds, exclusions, +exemptions, or stored allowances merely to silence growth. ## Fail-closed validation diff --git a/docs/usage.md b/docs/usage.md index 85da3cf..6d4b3d8 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -155,8 +155,10 @@ paths that no longer exist are ignored. ## Legacy LOC adoption ratchet -Established repositories may adopt LOC policy without accepting further growth -in files that already exceed their effective failure threshold. Create the +Established repositories may adopt LOC policy without accepting further growth. +The default `guards.loc.ratchetAt: "fail"` records files above effective +`failAt`. Use `"review"` when existing files above effective `warnAt` may remain +but must not grow. Create the canonical source-controlled ratchet over an intentionally bounded scope: ```bash @@ -168,11 +170,12 @@ git diff --cached The file is always `/.agent-tools/code-guard.loc-baseline.json`, where the root is the enclosing Git top-level or, outside Git, the resolved invocation directory. -Normal analysis reads it automatically and never writes it. A file at its exact -allowance, or reduced while still above ordinary `failAt`, becomes `REVIEW` -with the human label `RATCHET`. Growth above the allowance remains `FAIL`. Once -the count is at or below `failAt`, ordinary PASS/REVIEW behavior applies and -the ratchet is reported as no longer needed. +Normal analysis reads it automatically and never writes it. Under `fail`, a file +at its allowance, or reduced while still above `failAt`, becomes grandfathered +`REVIEW`. Under `review`, a file within its allowance keeps ordinary `REVIEW` +between `warnAt` and `failAt`, and is grandfathered only above `failAt`. Growth +above either allowance is `FAIL`. At or below the selected policy threshold, +the entry is reported as no longer needed. After reducing or deleting legacy code, explicitly lower and prune entries: @@ -182,7 +185,7 @@ code-guard src/legacy --update-loc-baseline Update bounds come only from the positional paths. Update lowers existing allowances, removes entries that are missing, excluded, inapplicable, or no -longer above `failAt`, and leaves entries outside the bounds unchanged. It +longer above the selected policy threshold, and leaves entries outside the bounds unchanged. It never adds a newly oversized path or raises an allowance; any attempted growth aborts the entire update. A rename is an old-path deletion plus a new, ungrandfathered destination—Git history is not consulted. Manual JSON edits @@ -197,6 +200,10 @@ This workflow is only for adopting established legacy repositories. New projects, including Agent Code Guard itself, should meet policy directly and must not create a ratchet baseline. +Do not switch between `fail` and `review`, raise thresholds, add exclusions or +exemptions, or edit allowances merely to silence a growth failure. Those are +source-controlled policy changes and require their own substantive justification. + ## Results and exit codes - `PASS` means no special action and exits `0`. diff --git a/examples/code-guard.config.json b/examples/code-guard.config.json index 61a2092..5c14d55 100644 --- a/examples/code-guard.config.json +++ b/examples/code-guard.config.json @@ -1,3 +1,8 @@ { - "version": 1 + "version": 1, + "guards": { + "loc": { + "ratchetAt": "fail" + } + } } diff --git a/skills/code-guard/references/loc-policy.md b/skills/code-guard/references/loc-policy.md index 540f4ef..1836b9f 100644 --- a/skills/code-guard/references/loc-policy.md +++ b/skills/code-guard/references/loc-policy.md @@ -27,6 +27,15 @@ Existing `allowedLargeFiles` entries may be honored with their configured reason Do not infer approval from inconvenience, historical size, a nearby exemption, time pressure, or a request to finish the coding task. +## Source-controlled ratchets + +An established repository may use `guards.loc.ratchetAt: "fail"` (the default) +to freeze only files already above `failAt`, or `"review"` to freeze every +recorded file above `warnAt` while retaining ordinary REVIEW semantics within +its allowance. Growth above a recorded allowance is always FAIL. Never switch +the policy, raise thresholds, add exclusions or exemptions, or increase/remove +allowances merely to silence growth. + ## Do not game LOC Project formatting conventions take priority. Never combine independent statements, compress control flow or expressions unusually, minify handwritten code, remove useful comments/structure, or fight the formatter merely to lower physical LOC. diff --git a/src/agent_code_guard/config_validation.py b/src/agent_code_guard/config_validation.py index 461a9dc..f01e691 100644 --- a/src/agent_code_guard/config_validation.py +++ b/src/agent_code_guard/config_validation.py @@ -26,6 +26,7 @@ REVIEW_GUARD_KEYS = {"enabled", "reviewAt"} LOC_KEYS = { "enabled", + "ratchetAt", "warnAt", "failAt", "countBlankLines", diff --git a/src/agent_code_guard/guards/loc.py b/src/agent_code_guard/guards/loc.py index 1736bad..225b846 100644 --- a/src/agent_code_guard/guards/loc.py +++ b/src/agent_code_guard/guards/loc.py @@ -64,6 +64,7 @@ class Config: exclude: list[str] allowed_large_files: list[AllowedLargeFile] overrides: list[ThresholdOverride] + ratchet_at: str = "fail" def load_config(args: argparse.Namespace) -> Config: @@ -89,6 +90,9 @@ def load_config(args: argparse.Namespace) -> Config: enabled = data.get("enabled", True) if not isinstance(enabled, bool): raise ValueError("guards.loc.enabled must be a boolean") + ratchet_at = data.get("ratchetAt", "fail") + if not isinstance(ratchet_at, str) or ratchet_at not in {"fail", "review"}: + raise ValueError("guards.loc.ratchetAt must be 'fail' or 'review'") warn_at = args.warn if args.warn is not None else data.get("warnAt", DEFAULT_WARN_AT) fail_at = args.fail if args.fail is not None else data.get("failAt", DEFAULT_FAIL_AT) @@ -114,6 +118,7 @@ def load_config(args: argparse.Namespace) -> Config: False if args.ignore_comment_lines else count_comments, include_extensions, exclude, parse_allowed_large_files(data.get("allowedLargeFiles", [])), parse_overrides(data.get("overrides", [])), + ratchet_at, ) @@ -201,6 +206,20 @@ def evaluate( ratchet_status = None if allowed and counted > warn_at: native_status, state, reason = "exempt", "pass", allowed.reason + elif config.ratchet_at == "review" and baseline_loc is not None: + if counted > baseline_loc: + native_status, state = "ratchetExceeded", "fail" + reason = f"LOC grew above source-controlled allowance {baseline_loc}." + ratchet_status = "exceeded" + elif counted > fail_at: + native_status, state, reason = "grandfathered", "review", None + ratchet_status = "within" + elif counted > warn_at: + native_status, state, reason = "warn", "review", None + ratchet_status = "within" + else: + native_status, state, reason = "ok", "pass", None + ratchet_status = "notNeeded" elif counted > fail_at: if baseline_loc is not None and counted <= baseline_loc: native_status, state, reason = "grandfathered", "review", None diff --git a/src/agent_code_guard/loc_baseline.py b/src/agent_code_guard/loc_baseline.py index 7caec42..3e18d22 100644 --- a/src/agent_code_guard/loc_baseline.py +++ b/src/agent_code_guard/loc_baseline.py @@ -126,8 +126,9 @@ def create(root: Path, files: tuple[Path, ...], config: loc.Config) -> int: if any(matches_path_glob(relative, item.path) for item in config.allowed_large_files): continue counted = loc.count_loc(path, config) - _, fail_at, _ = loc.effective_thresholds(relative, config) - if counted > fail_at: + warn_at, fail_at, _ = loc.effective_thresholds(relative, config) + threshold = warn_at if config.ratchet_at == "review" else fail_at + if counted > threshold: entries[relative] = counted content = serialize(entries) created_directory = not target.parent.exists() @@ -178,8 +179,9 @@ def update( raise ValueError( f"LOC baseline update would increase allowance for {relative}: {allowance} to {counted}" ) - _, fail_at, _ = loc.effective_thresholds(relative, config) - if counted <= fail_at: + warn_at, fail_at, _ = loc.effective_thresholds(relative, config) + threshold = warn_at if config.ratchet_at == "review" else fail_at + if counted <= threshold: proposed.pop(relative) removed += 1 elif counted < allowance: diff --git a/tests/test_config_validation.py b/tests/test_config_validation.py index f8dac52..fb505ef 100644 --- a/tests/test_config_validation.py +++ b/tests/test_config_validation.py @@ -71,6 +71,7 @@ def test_valid_configuration_contracts_continue_to_load(self) -> None: "guards": { "loc": { "enabled": True, + "ratchetAt": "fail", "warnAt": 10, "failAt": 20, "countBlankLines": False, diff --git a/tests/test_loc_baseline.py b/tests/test_loc_baseline.py index 78e547a..79afb6a 100644 --- a/tests/test_loc_baseline.py +++ b/tests/test_loc_baseline.py @@ -13,6 +13,107 @@ class LocBaselineTests(CodeGuardTestCase): + def test_review_ratchet_public_lifecycle_and_outputs(self) -> None: + with tempfile.TemporaryDirectory() as temp: + root = Path(temp) + init_git(root) + write_lines(root / "legacy.py", 5) + write_lines(root / "equal.py", 3) + config = write_config(root, {"warnAt": 3, "failAt": 7, "ratchetAt": "review"}) + + created = self.run_guard(root, ".", "--config", str(config), "--create-loc-baseline") + self.assertEqual((created.returncode, created.stdout), (0, ( + "Created LOC baseline: .agent-tools/code-guard.loc-baseline.json (1 entries).\n" + ))) + baseline = root / BASELINE + self.assertEqual( + json.loads(baseline.read_text(encoding="utf-8"))["loc"]["files"], + [{"path": "legacy.py", "allowedLoc": 5}], + ) + + unchanged = self.run_guard(root, ".", "--config", str(config), "--json") + by_path = {item["path"]: item for item in self.findings(unchanged)} + self.assertEqual( + (by_path["legacy.py"]["nativeStatus"], by_path["legacy.py"]["state"], + by_path["legacy.py"]["ratchetStatus"]), + ("warn", "review", "within"), + ) + self.assertEqual(by_path["equal.py"]["nativeStatus"], "ok") + + write_lines(root / "legacy.py", 4) + reduced = self.run_guard(root, ".", "--config", str(config), "--json") + self.assertEqual(self.findings(reduced)[1]["state"], "review") + lowered = self.run_guard(root, ".", "--config", str(config), "--update-loc-baseline") + self.assertIn("(1 lowered, 0 removed, 0 unchanged)", lowered.stdout) + + write_lines(root / "legacy.py", 6) + before_growth = baseline.read_bytes() + before_mtime = baseline.stat().st_mtime_ns + for ci in (False, True): + args = ("--ci",) if ci else () + growth = self.run_guard(root, ".", "--config", str(config), *args, "--json") + self.assertEqual(growth.returncode, 2) + finding = next(item for item in self.findings(growth) if item["path"] == "legacy.py") + self.assertEqual( + (finding["nativeStatus"], finding["state"], finding["ratchetStatus"], + finding["baselineLoc"], finding["reason"]), + ("ratchetExceeded", "fail", "exceeded", 4, + "LOC grew above source-controlled allowance 4."), + ) + self.assertEqual(self.read_json(growth)["requiredPolicies"], ["loc"]) + human = self.run_guard(root, ".", "--config", str(config)) + self.assertIn("LOC grew above source-controlled allowance 4.", human.stdout) + rejected = self.run_guard(root, ".", "--config", str(config), "--update-loc-baseline") + self.assertEqual(rejected.returncode, 3) + self.assertEqual((baseline.read_bytes(), baseline.stat().st_mtime_ns), (before_growth, before_mtime)) + + write_lines(root / "legacy.py", 3) + write_lines(root / "new.py", 5) + full = self.run_guard(root, ".", "--config", str(config), "--json") + by_path = {item["path"]: item for item in self.findings(full)} + self.assertEqual( + (by_path["legacy.py"]["nativeStatus"], by_path["legacy.py"]["state"], + by_path["legacy.py"]["ratchetStatus"]), + ("ok", "pass", "notNeeded"), + ) + self.assertEqual( + (by_path["new.py"]["nativeStatus"], by_path["new.py"]["state"]), + ("warn", "review"), + ) + compact = self.run_guard( + root, ".", "--config", str(config), "--json", "--json-mode", "compact", + ) + self.assertNotIn("legacy.py", {item["path"] for item in self.findings(compact)}) + self.assertIn("new.py", {item["path"] for item in self.findings(compact)}) + pruned = self.run_guard(root, ".", "--config", str(config), "--update-loc-baseline") + self.assertIn("(0 lowered, 1 removed, 0 unchanged)", pruned.stdout) + self.assertEqual(json.loads(baseline.read_text(encoding="utf-8"))["loc"]["files"], []) + + def test_review_ratchet_uses_last_matching_override_for_lifecycle(self) -> None: + with tempfile.TemporaryDirectory() as temp: + root = Path(temp) + init_git(root) + write_lines(root / "special.py", 5) + config = write_config(root, { + "warnAt": 10, "failAt": 20, "ratchetAt": "review", + "overrides": [ + {"match": ["*.py"], "warnAt": 4, "failAt": 8}, + {"match": ["special.py"], "warnAt": 3, "failAt": 6}, + ], + }) + self.assertEqual( + self.run_guard(root, ".", "--config", str(config), "--create-loc-baseline").returncode, + 0, + ) + finding = self.findings(self.run_guard(root, ".", "--config", str(config), "--json"))[0] + self.assertEqual((finding["overrideIndex"], finding["warnAt"], finding["failAt"]), (1, 3, 6)) + write_lines(root / "special.py", 3) + self.assertEqual( + self.run_guard(root, ".", "--config", str(config), "--update-loc-baseline").returncode, + 0, + ) + self.assertEqual(json.loads((root / BASELINE).read_text(encoding="utf-8"))["loc"]["files"], []) + def test_normal_analysis_ratchet_states_and_output_contract(self) -> None: with tempfile.TemporaryDirectory() as temp: root = Path(temp) diff --git a/tests/test_provider_failure_isolation.py b/tests/test_provider_failure_isolation.py index c291de2..b358dd9 100644 --- a/tests/test_provider_failure_isolation.py +++ b/tests/test_provider_failure_isolation.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json import tempfile from pathlib import Path @@ -10,6 +11,30 @@ class ProviderFailureIsolationLifecycleTests(CodeGuardTestCase): + def test_compact_incomplete_run_retains_loc_ratchet_failure(self) -> None: + with tempfile.TemporaryDirectory() as temp: + root = Path(temp) + (root / "broken.py").write_text("def broken(:\nvalue = 1\nvalue = 2\nvalue = 3\n", encoding="utf-8") + config = write_config(root, {"warnAt": 2, "failAt": 10, "ratchetAt": "review"}) + baseline = root / ".agent-tools" / "code-guard.loc-baseline.json" + baseline.parent.mkdir() + baseline.write_text(json.dumps({ + "version": 1, + "loc": {"files": [{"path": "broken.py", "allowedLoc": 3}]}, + }, indent=2) + "\n", encoding="utf-8") + for ci in (False, True): + args = ("--ci",) if ci else () + result = self.run_guard( + root, "broken.py", "--config", str(config), *args, + "--json", "--json-mode", "compact", + ) + self.assertEqual(result.returncode, 3) + data = self.read_json(result) + self.assertEqual((data["overall"], data["completedOverall"]), ("incomplete", "fail")) + self.assertEqual(data["guards"]["loc"]["complete"], True) + self.assertEqual(data["requiredPolicies"], ["loc"]) + self.assertEqual(data["guards"]["loc"]["findings"][0]["ratchetStatus"], "exceeded") + def create_mixed_selection(self, root: Path) -> Path: (root / "valid.py").write_text( "def classify(value):\n" diff --git a/tests/test_runner_config_validation.py b/tests/test_runner_config_validation.py index d9bed5b..91c8450 100644 --- a/tests/test_runner_config_validation.py +++ b/tests/test_runner_config_validation.py @@ -8,6 +8,20 @@ class ConfigurationValidationTests(CodeGuardTestCase): + def test_ratchet_at_accepts_only_exact_policy_strings_and_defaults_to_fail(self) -> None: + with tempfile.TemporaryDirectory() as temp: + root = Path(temp) + write_lines(root / "sample.py", 4) + for value in ("FAIL", "Review", "other", True, 1, None): + with self.subTest(value=value): + config = write_config(root, {"warnAt": 3, "failAt": 6, "ratchetAt": value}) + result = self.run_guard(root, ".", "--config", str(config), "--json") + self.assertEqual(result.returncode, 3) + self.assertIn("guards.loc.ratchetAt must be 'fail' or 'review'", self.read_json(result)["error"]) + for value in ("fail", "review"): + config = write_config(root, {"warnAt": 3, "failAt": 6, "ratchetAt": value}) + self.assertEqual(self.run_guard(root, ".", "--config", str(config), "--json").returncode, 1) + def test_global_thresholds_require_positive_json_integers(self) -> None: invalid_values = ["3", True, 3.0] with tempfile.TemporaryDirectory() as temp: