diff --git a/.github/workflows/house-rules.yml b/.github/workflows/house-rules.yml index 667f275..c80a9fe 100644 --- a/.github/workflows/house-rules.yml +++ b/.github/workflows/house-rules.yml @@ -1,7 +1,7 @@ name: House rules # Fails unless AGENTS.md copies the shared rules from craft-contrib unchanged, at a commit that is -# on main. An intact older copy warns and does not fail. Callers run it beside their gate: +# on main. An intact older copy warns only when its normalized standards file changed on main, and does not fail. Callers run it beside their gate: # # house-rules: # uses: storytold/craft-common-actions/.github/workflows/house-rules.yml@main diff --git a/README.md b/README.md index 7d2a810..498b0a9 100644 --- a/README.md +++ b/README.md @@ -662,7 +662,8 @@ Fails unless `AGENTS.md` carries the shared house rules from the commit it was copied from and the SHA-256 of the bytes inside it. The check loads `standards/house-rules-core.md` (and `standards/house-rules-rust.md` when `rust` is true) at that commit, requires the commit to be on `main`, and compares the normalized bytes. An edited block, or -a commit that is not on `main`, fails. An intact older commit warns. A source that cannot be read +a commit that is not on `main`, fails. An intact older commit warns only when its normalized standards file changed on `main`. +If the file is unchanged, there is no warning. A source that cannot be read fails as "could not check". `rust` defaults to true; a repository that is not Rust passes `false`. The caller is only the workflow call. The reusable workflow checks the app out and checks @@ -703,7 +704,7 @@ Individual actions are also available for composing custom pipelines: - `actions/setup-windows-signing`: Exports the shared Authenticode signing script (`CRAFT_SIGN_SCRIPT`) for the app's packaging step (see "Windows code signing"). - `actions/ast-grep`: Installs a pinned ast-grep, runs the caller's rule tests and scans with its `sgconfig.yml` (see `ast-grep.yml`). - `actions/check-git-sources`: The check `git-sources.yml` runs, for a job that has already checked out the app; `allowed-prefix` changes where git dependencies may come from (default `git+https://github.com/storytold/`). -- `actions/house-rules`: The check `house-rules.yml` runs. `source` is a checkout of craft-contrib with `main` history; `require-rust` asks for the Rust block as well as the core block. +- `actions/house-rules`: The check `house-rules.yml` runs. `source` is a checkout of craft-contrib with `main` history (refresh `origin/main` before checking; the local `main` ref is used only when `origin/main` is absent, never `HEAD`); `require-rust` asks for the Rust block as well as the core block. ## Storytold Organization Hosting diff --git a/actions/house-rules/action.yml b/actions/house-rules/action.yml index 1da65e8..9ccf6aa 100644 --- a/actions/house-rules/action.yml +++ b/actions/house-rules/action.yml @@ -1,7 +1,7 @@ name: Check house rules description: >- Fail unless the AGENTS.md house-rules blocks match craft-contrib at the commit they name, - and that commit is on main. An intact older commit warns. + and that commit is on main. An intact older commit warns only when the normalized standards file changed on main. inputs: working-directory: diff --git a/actions/house-rules/house_rules.py b/actions/house-rules/house_rules.py index 2e5c1b1..af430f8 100644 --- a/actions/house-rules/house_rules.py +++ b/actions/house-rules/house_rules.py @@ -4,7 +4,7 @@ The bytes inside a block are the normalized standards file. The recorded sha256 must match those bytes, and those bytes must match the file at the recorded commit on craft-contrib main. A digest rewritten to suit an edit does not pass. -An intact older commit warns. A missing source is "could not check". +An intact older commit warns only when the normalized standards file changed on main. A missing source is "could not check". """ from __future__ import annotations @@ -25,6 +25,7 @@ START = re.compile( r"(?:\r\n|\r|\n)" ) END = "" +MARKER = re.compile(r"")): + with self.subTest(text=text): + target = self.app / "AGENTS.md" + target.write_bytes(text.encode()) + self.assertTrue(self.check(False)[0]) + with self.assertRaises(ValueError): + self.write(False) + self.assertEqual(target.read_bytes(), text.encode()) + + def test_duplicate_and_unknown_blocks_fail_without_writing(self): + core = house_rules.render("core", self.second, "Core two.\n") + for text in (core + core, core + house_rules.render("other", self.second, "Other.\n")): + with self.subTest(text=text): + target = self.app / "AGENTS.md" + target.write_bytes(text.encode()) + self.assertTrue(self.check(False)[0]) + with self.assertRaises(ValueError): + self.write(False) + self.assertEqual(target.read_bytes(), text.encode()) + + def test_writer_uses_main_even_with_unmerged_head(self): + git(self.source, "checkout", "-b", "side") + self.commit_standards("Side core.\n", "Side rust.\n") + self.write() + text = (self.app / "AGENTS.md").read_text() + self.assertIn(self.second, text) + self.assertIn("Core two.", text) + self.assertNotIn("Side core.", text) + self.assertEqual(self.check(), ([], [])) + + def test_origin_main_takes_priority_over_local_main(self): + git(self.source, "update-ref", "refs/remotes/origin/main", self.first) + self.write() + text = (self.app / "AGENTS.md").read_text() + self.assertIn(self.first, text) + self.assertIn("Core one.", text) + self.assertEqual(self.check(), ([], [])) + self.assertEqual(house_rules.revision_state(str(self.source), self.second), "not-on-main") + + def test_missing_main_fails_closed_even_with_valid_head(self): + self.write() + target = self.app / "AGENTS.md" + original = target.read_bytes() + git(self.source, "branch", "-m", "side") + self.assertTrue(any("could not check" in item for item in self.check()[0])) + with self.assertRaises(RuntimeError): + self.write() + self.assertEqual(target.read_bytes(), original) + + def test_detached_head_uses_named_main(self): + git(self.source, "checkout", "--detach", self.first) + self.write() + self.assertIn(self.second, (self.app / "AGENTS.md").read_text()) + self.assertEqual(self.check(), ([], [])) + + def test_ancestry_errors_are_not_reported_as_non_ancestry(self): + real_git = house_rules.git + def git_error(source, args): + if args[0] == "merge-base": + return subprocess.CompletedProcess(args, 128, "", "missing intermediate commit") + return real_git(source, args) + with mock.patch.object(house_rules, "git", side_effect=git_error): + self.assertEqual(house_rules.revision_state(str(self.source), self.first), "missing") + failures, _ = house_rules.check_file( + house_rules.render("core", self.first, "Core one.\n"), self.source, False) + self.assertTrue(any("could not check" in item for item in failures)) + + def test_git_uses_explicit_utf8_and_reports_invalid_output(self): + with mock.patch.object(house_rules.subprocess, "run") as run: + house_rules.git(str(self.source), ["status"]) + self.assertEqual(run.call_args.kwargs["encoding"], "utf-8") + error = UnicodeDecodeError("utf-8", b"\xff", 0, 1, "invalid") + with mock.patch.object(house_rules.subprocess, "run", side_effect=error): + self.assertNotEqual(house_rules.git(str(self.source), ["status"]).returncode, 0) + + def test_non_ascii_standards_round_trip(self): + standards = self.source / "standards" + (standards / "house-rules-core.md").write_bytes("Core café.\n".encode("utf-8")) + git(self.source, "add", "standards") + git(self.source, "commit", "-m", "Unicode standards") + self.write() + self.assertIn("café".encode("utf-8"), (self.app / "AGENTS.md").read_bytes()) + self.assertEqual(self.check(), ([], [])) + + class HouseRulesWiringTests(unittest.TestCase): def test_the_action_passes_its_inputs_through_the_environment(self): action = yaml.load((ROOT / "actions/house-rules/action.yml").read_text(), Loader=yaml.BaseLoader)