diff --git a/CHANGELOG.md b/CHANGELOG.md index cb700e6f..1cc69abb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,8 @@ per-file diff commands. ### Fixed +- **Framework version checks compare marker values using Git-compatible paths on every platform** (#578, `tools/check_framework_version.py`) - quoting or spacing an unchanged version no longer exempts an accompanying guidance edit, hardening a latent bypass without changing legitimate version bumps. + - **`/reset documents` clears nested and hidden user entries while preserving scaffolds** - cleanup retains tracked `.gitkeep` files and the README, with the complete inventory shown before confirmation (#580). - **`jobindex-search` examples follow the search CLI contract** - the posted-today example includes a required query, unsupported flags are removed, and regression checks accept LF and CRLF checkouts (#579). diff --git a/tests/test_check_framework_version.py b/tests/test_check_framework_version.py index 54ee6b09..6e48eb1c 100644 --- a/tests/test_check_framework_version.py +++ b/tests/test_check_framework_version.py @@ -18,7 +18,7 @@ import sys import tempfile import unittest import unittest.mock -from pathlib import Path +from pathlib import Path, PureWindowsPath REPO_ROOT = Path(__file__).resolve().parent.parent SCRIPT = REPO_ROOT / "tools" / "check_framework_version.py" @@ -96,6 +96,26 @@ class FrameworkVersionGateTests(CheckerRepoFixture): self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + def test_same_version_reformatted_does_not_count_as_a_bump(self): + for marker in ['framework_version: "1.0.0"', "framework_version: 1.0.0"]: + with self.subTest(marker=marker): + self.framework_file.write_text( + FRONTMATTER.replace("framework_version: 1.0.0", marker) + + BODY + "\nChanged guidance with the same version.\n", + encoding="utf-8", + ) + result = self.run_checker() + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("modified without bumping 'framework_version'", result.stdout) + + def test_reformatting_version_alone_remains_allowed(self): + self.framework_file.write_text( + FRONTMATTER.replace("framework_version: 1.0.0", 'framework_version: "1.0.0"') + BODY, + encoding="utf-8", + ) + result = self.run_checker() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + def test_file_without_version_marker_fails(self): (self.skill_dir / "02-unmarked.md").write_text( "# No frontmatter at all\n", encoding="utf-8" @@ -106,6 +126,31 @@ class FrameworkVersionGateTests(CheckerRepoFixture): self.assertEqual(result.returncode, 1, result.stdout + result.stderr) self.assertIn("missing 'framework_version'", result.stdout) +class WindowsGitPathTests(unittest.TestCase): + def test_diff_and_show_use_posix_paths_for_windows_checkout(self): + import importlib.util + + spec = importlib.util.spec_from_file_location("framework_windows_paths", SCRIPT) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + root = PureWindowsPath("C:/checkout") + profile = root / ".claude/skills/job-application-assistant/01-test-profile.md" + relative = ".claude/skills/job-application-assistant/01-test-profile.md" + diff = '-framework_version: 1.0.0\n+framework_version: "1.0.0"\n-Old guidance\n+New guidance\n' + with unittest.mock.patch.object(module, "ROOT", root), \ + unittest.mock.patch.object(module, "run_git", side_effect=[ + (0, diff, ""), (0, FRONTMATTER + BODY, "") + ]) as run_git, \ + unittest.mock.patch.object(module, "parse_frontmatter", return_value={ + "framework_version": "1.0.0" + }): + self.assertTrue(module.has_non_trivial_changes(profile, "base")) + self.assertEqual(run_git.call_args_list, [ + unittest.mock.call(["diff", "-U0", "base", "--", relative]), + unittest.mock.call(["show", f"base:{relative}"]), + ]) + + class RunGitEncodingTests(unittest.TestCase): """run_git() must decode `git diff` output as UTF-8 explicitly, not via whatever `subprocess.run(text=True)` falls back to on the host locale. diff --git a/tools/check_framework_version.py b/tools/check_framework_version.py index 01bf250d..e00e3897 100755 --- a/tools/check_framework_version.py +++ b/tools/check_framework_version.py @@ -62,7 +62,9 @@ def get_base_commit() -> str | None: def parse_frontmatter(path: Path) -> dict: if not path.exists(): return {} - text = path.read_text(encoding="utf-8") + return parse_frontmatter_text(path.read_text(encoding="utf-8")) + +def parse_frontmatter_text(text: str) -> dict: if not text.startswith("---\n"): return {} end = text.find("\n---", 4) @@ -79,7 +81,7 @@ def parse_frontmatter(path: Path) -> dict: def has_non_trivial_changes(file_path: Path, base_commit: str) -> bool: # Get diff of the file from base_commit to HEAD - rel_path = str(file_path.relative_to(ROOT)) + rel_path = file_path.relative_to(ROOT).as_posix() rc, stdout, stderr = run_git(["diff", "-U0", base_commit, "--", rel_path]) if rc != 0: # If diff fails (e.g. file is new/untracked), it's a change @@ -107,9 +109,15 @@ def has_non_trivial_changes(file_path: Path, base_commit: str) -> bool: continue meaningful_changes += 1 - # If the version key itself was modified, we don't fail, regardless of other changes + # Editing a version line is not necessarily a bump: YAML quoting and + # whitespace can change while the parsed marker stays the same. if version_changed: - return False + rc, base_text, _ = run_git(["show", f"{base_commit}:{rel_path}"]) + if rc != 0 or ( + parse_frontmatter_text(base_text).get("framework_version") + != parse_frontmatter(file_path).get("framework_version") + ): + return False # If there are meaningful changes but the version was not changed return meaningful_changes > 0 @@ -119,7 +127,7 @@ def main() -> int: # 1. Lint: Check that all framework files have framework_version in frontmatter for path in FRAMEWORK_FILES: - rel_path = str(path.relative_to(ROOT)) + rel_path = path.relative_to(ROOT).as_posix() fm = parse_frontmatter(path) if "framework_version" not in fm: errors.append(f"{rel_path}: missing 'framework_version' in frontmatter") @@ -129,7 +137,7 @@ def main() -> int: if base_commit: print(f"Comparing HEAD against base commit: {base_commit}") for path in FRAMEWORK_FILES: - rel_path = str(path.relative_to(ROOT)) + rel_path = path.relative_to(ROOT).as_posix() if "framework_version" not in parse_frontmatter(path): # Skip checking changes if it doesn't even have frontmatter (already reported above) continue