mirror of
https://github.com/MadsLorentzen/ai-job-search.git
synced 2026-11-02 13:12:50 +00:00
fix(framework-version): distinguish marker changes from YAML formatting (#578)
* fix(framework-version): distinguish marker changes from YAML formatting * fix(framework-version): normalize Windows paths for Git base reads
This commit is contained in:
1 parent
d590aa1b10
commit
176b675080
3 files changed
+62
-7
No files matched your search
@@ -68,6 +68,8 @@ per-file diff commands.
|
|||||||
|
|
||||||
### Fixed
|
### 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).
|
- **`/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).
|
- **`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).
|
||||||
|
|||||||
@@ -18,7 +18,7 @@ import sys
|
|||||||
import tempfile
|
import tempfile
|
||||||
import unittest
|
import unittest
|
||||||
import unittest.mock
|
import unittest.mock
|
||||||
from pathlib import Path
|
from pathlib import Path, PureWindowsPath
|
||||||
|
|
||||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||||
SCRIPT = REPO_ROOT / "tools" / "check_framework_version.py"
|
SCRIPT = REPO_ROOT / "tools" / "check_framework_version.py"
|
||||||
@@ -96,6 +96,26 @@ class FrameworkVersionGateTests(CheckerRepoFixture):
|
|||||||
|
|
||||||
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
|
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):
|
def test_file_without_version_marker_fails(self):
|
||||||
(self.skill_dir / "02-unmarked.md").write_text(
|
(self.skill_dir / "02-unmarked.md").write_text(
|
||||||
"# No frontmatter at all\n", encoding="utf-8"
|
"# 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.assertEqual(result.returncode, 1, result.stdout + result.stderr)
|
||||||
self.assertIn("missing 'framework_version'", result.stdout)
|
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):
|
class RunGitEncodingTests(unittest.TestCase):
|
||||||
"""run_git() must decode `git diff` output as UTF-8 explicitly, not via
|
"""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.
|
whatever `subprocess.run(text=True)` falls back to on the host locale.
|
||||||
|
|||||||
@@ -62,7 +62,9 @@ def get_base_commit() -> str | None:
|
|||||||
def parse_frontmatter(path: Path) -> dict:
|
def parse_frontmatter(path: Path) -> dict:
|
||||||
if not path.exists():
|
if not path.exists():
|
||||||
return {}
|
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"):
|
if not text.startswith("---\n"):
|
||||||
return {}
|
return {}
|
||||||
end = text.find("\n---", 4)
|
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:
|
def has_non_trivial_changes(file_path: Path, base_commit: str) -> bool:
|
||||||
# Get diff of the file from base_commit to HEAD
|
# 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])
|
rc, stdout, stderr = run_git(["diff", "-U0", base_commit, "--", rel_path])
|
||||||
if rc != 0:
|
if rc != 0:
|
||||||
# If diff fails (e.g. file is new/untracked), it's a change
|
# 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
|
continue
|
||||||
meaningful_changes += 1
|
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:
|
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
|
# If there are meaningful changes but the version was not changed
|
||||||
return meaningful_changes > 0
|
return meaningful_changes > 0
|
||||||
@@ -119,7 +127,7 @@ def main() -> int:
|
|||||||
|
|
||||||
# 1. Lint: Check that all framework files have framework_version in frontmatter
|
# 1. Lint: Check that all framework files have framework_version in frontmatter
|
||||||
for path in FRAMEWORK_FILES:
|
for path in FRAMEWORK_FILES:
|
||||||
rel_path = str(path.relative_to(ROOT))
|
rel_path = path.relative_to(ROOT).as_posix()
|
||||||
fm = parse_frontmatter(path)
|
fm = parse_frontmatter(path)
|
||||||
if "framework_version" not in fm:
|
if "framework_version" not in fm:
|
||||||
errors.append(f"{rel_path}: missing 'framework_version' in frontmatter")
|
errors.append(f"{rel_path}: missing 'framework_version' in frontmatter")
|
||||||
@@ -129,7 +137,7 @@ def main() -> int:
|
|||||||
if base_commit:
|
if base_commit:
|
||||||
print(f"Comparing HEAD against base commit: {base_commit}")
|
print(f"Comparing HEAD against base commit: {base_commit}")
|
||||||
for path in FRAMEWORK_FILES:
|
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):
|
if "framework_version" not in parse_frontmatter(path):
|
||||||
# Skip checking changes if it doesn't even have frontmatter (already reported above)
|
# Skip checking changes if it doesn't even have frontmatter (already reported above)
|
||||||
continue
|
continue
|
||||||
|
|||||||
Reference in new issue
Block a user