diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 436d137a90..d715113ea0 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -174,5 +174,6 @@ than attaching private fixtures, snapshot identifiers, raw output or infrastruct - [ ] Neither the diff nor this PR body/comments/attachments disclose private state, credentials, raw traces or verifier output, internal links, or local machine paths (including `.loopx/`, `.codex/goals/`, and live `ACTIVE_GOAL_STATE.md`). - [ ] I did not duplicate maintainer-owned benchmark work unless a maintainer split out a public issue for it. - [ ] I kept the change scoped to the linked issue/task. +- [ ] Existing English/Chinese RFC pairs (including ledger entries) both have a diff, and the local premerge check `python3 scripts/check_rfc_bilingual.py --base origin/main --head HEAD` passes, or no paired RFC changed. - [ ] I completed the visual evidence section for UI changes, or marked UI impact `none`. - [ ] Every commit includes a DCO `Signed-off-by` trailer (`git commit -s`). diff --git a/AGENTS.md b/AGENTS.md index 8cd0f3805e..dc116e95ee 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -117,6 +117,26 @@ for the full DCO policy. Only skip this worktree/PR gate when the user explicitly says the change is local-only and must not be proposed for the repository. +### Bilingual RFC Diff Gate + +For an existing English/Chinese RFC pair under `docs/architecture/rfcs/`, +including its ledger entries, both files must change in the same PR. Check the +staged files before committing: + +```bash +python3 scripts/check_rfc_bilingual.py --base origin/main --staged +``` + +The staged check includes earlier branch commits. Before merge, run +`python3 scripts/check_rfc_bilingual.py --base origin/main --head HEAD` against +the PR's actual target branch and exact reviewed head. The source-checkout +`uv run --extra test loopx canary premerge --from-git-diff` runs this as a direct +blocking check, including in the quick tier. A failure holds merge even when CI +waiting is disabled or admin bypass is authorized. It checks the committed diff; +an unstaged translation cannot satisfy it. The check uses the merge base, covers +paired deletion/renaming, and leaves legacy single-language files alone. Review +the actual translation; paired diffs do not certify semantic equivalence. + For non-trivial repository changes, especially anything that touches benchmark adapters, smoke tests, public docs, or commit/push workflows, use the `git-split-commit-pr` workflow before staging: diff --git a/docs/architecture/rfcs/README.md b/docs/architecture/rfcs/README.md index 0a1bef16cc..cdfc5738c0 100644 --- a/docs/architecture/rfcs/README.md +++ b/docs/architecture/rfcs/README.md @@ -96,6 +96,20 @@ failure leaves the generated files untouched. successor reciprocally names that predecessor in `Supersedes / closes`. +## Bilingual changes + +Existing English/Chinese RFC pairs and their ledger entries must both have a +diff in the same PR. Before committing, run +`python3 scripts/check_rfc_bilingual.py --base origin/main --staged` (including +earlier branch commits). Before merge, run +`python3 scripts/check_rfc_bilingual.py --base origin/main --head HEAD`, using +the PR's actual target branch and reviewed head. Source-checkout +`uv run --extra test loopx canary premerge --from-git-diff` includes this direct +blocking check in every tier. CI waiting and admin bypass do not waive it. +Only committed changes satisfy the merge check. It compares from the merge base, +includes paired deletion/renaming, and leaves legacy single-language files alone. +Paired diffs do not certify translation accuracy. + ## Overall Product and Delivery Roadmap - [LoopX Overall Roadmap v0](loopx-overall-roadmap-v0.md) diff --git a/examples/canary/premerge-validation-gate-smoke.py b/examples/canary/premerge-validation-gate-smoke.py index 03e16ab254..ca4a2adb19 100644 --- a/examples/canary/premerge-validation-gate-smoke.py +++ b/examples/canary/premerge-validation-gate-smoke.py @@ -295,7 +295,11 @@ def assert_cli_premerge_reports_progress_by_default() -> None: assert payload["gate"]["status"] == "no_changes", payload assert payload["validation_summary"]["selected_check_count"] == 0, payload assert "[loopx canary] premerge start:" in completed.stderr, completed.stderr - assert "[loopx canary] start direct_checks 1/3:" in completed.stderr, completed.stderr + assert "[loopx canary] start direct_checks 1/" in completed.stderr, completed.stderr + assert any( + check["id"] == "rfc_bilingual_diff" and check["status"] == "passed" + for check in payload["direct_checks"] + ), payload assert "[loopx canary] start catalog_canaries" not in completed.stderr, completed.stderr assert "[loopx canary] premerge done:" in completed.stderr, completed.stderr diff --git a/loopx/canary/premerge.py b/loopx/canary/premerge.py index a0f2d93ae8..44106003ba 100644 --- a/loopx/canary/premerge.py +++ b/loopx/canary/premerge.py @@ -493,6 +493,17 @@ def _diff_hygiene_checks( "checks unstaged changes not yet committed", ), ] + rfc_checker = "scripts/check_rfc_bilingual.py" + checker_registered = (repo_root / rfc_checker).is_file() or subprocess.run( + ["git", "cat-file", "-e", f"{base}:{rfc_checker}"], + cwd=repo_root, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False, + ).returncode == 0 + if checker_registered: + specs.append(( + "rfc_bilingual_diff", + [sys.executable, rfc_checker, "--base", base, "--head", "HEAD"], + "requires both existing RFC mirrors in the committed PR diff before merge", + )) checks: list[dict[str, Any]] = [] for index, (check_id, argv, reason) in enumerate(specs, start=1): checks.append( diff --git a/scripts/check_rfc_bilingual.py b/scripts/check_rfc_bilingual.py new file mode 100644 index 0000000000..6759591e6a --- /dev/null +++ b/scripts/check_rfc_bilingual.py @@ -0,0 +1,80 @@ +"""Premerge check for both existing RFC language mirrors in the same PR diff.""" + +from __future__ import annotations + +import argparse +import os +import subprocess +import sys + +RFC_ROOT = "docs/architecture/rfcs/" + + +def git_paths(*args: str) -> set[str]: + raw = subprocess.check_output(["git", *args, "-z", "--", RFC_ROOT]) + return {os.fsdecode(path) for path in raw.split(b"\0") if path} + + +def revision(ref: str) -> str: + return subprocess.check_output( + ["git", "rev-parse", "--verify", "--end-of-options", f"{ref}^{{commit}}"], + text=True, + ).strip() + + +def missing_mirrors(changed: set[str], existing: set[str]) -> list[tuple[str, str]]: + missing = [] + for path in sorted(changed): + if not path.startswith(RFC_ROOT) or not path.endswith(".md"): + continue + mirror = ( + path.removesuffix(".zh-CN.md") + ".md" + if path.endswith(".zh-CN.md") else path.removesuffix(".md") + ".zh-CN.md" + ) + if mirror in existing and mirror not in changed: + missing.append((path, mirror)) + return missing + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--staged", action="store_true", help="check the index; --base includes earlier branch commits") + parser.add_argument("--base", help="target-branch commit; compare from the merge base") + parser.add_argument("--head", help="exact proposed commit, required with --base") + args = parser.parse_args() + if args.staged and args.head: + parser.error("--head cannot be combined with --staged") + if not args.staged and not (args.base and args.head): + parser.error("supply --staged or both --base and --head") + try: + if args.staged: + base = revision("HEAD") + if args.base: + base = subprocess.check_output( + ["git", "merge-base", revision(args.base), base], text=True, + ).strip() + changed = git_paths("diff", "--cached", "--name-only", "--no-renames", base) + after = git_paths("ls-files", "--cached") + else: + head = revision(args.head) + base = subprocess.check_output( + ["git", "merge-base", revision(args.base), head], text=True, + ).strip() + changed = git_paths("diff", "--name-only", "--no-renames", base, head) + after = git_paths("ls-tree", "-r", "--name-only", head) + before = git_paths("ls-tree", "-r", "--name-only", base) + except subprocess.CalledProcessError: + print("Cannot read the RFC diff; fetch the exact base/head and retry.", file=sys.stderr) + return 1 + missing = missing_mirrors(changed, before | after) + if missing: + print("RFC bilingual diff check failed:", file=sys.stderr) + for path, mirror in missing: + print(f" {path}: include a matching change to {mirror}", file=sys.stderr) + return 1 + print("RFC bilingual diff check passed.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/skills/loopx-pr-merge/SKILL.md b/skills/loopx-pr-merge/SKILL.md index 63485d8421..d7c24a6796 100644 --- a/skills/loopx-pr-merge/SKILL.md +++ b/skills/loopx-pr-merge/SKILL.md @@ -28,7 +28,13 @@ authorized: a diff read, green CI, or pull-request metadata is not a substitute. surface. 2. Apply `completion_gate` literally, and re-read the remote head immediately before the decision. -3. Immediately before the merge, run `loopx --format json pr-review --goal-id +3. For this repository, run `python3 scripts/check_rfc_bilingual.py --base + origin/main --head HEAD` from the exact reviewed checkout before merge; + substitute the PR's actual target branch and verify local HEAD matches the + remote reviewed head. The source-checkout `loopx canary premerge` includes + the same direct blocking check. A failed check holds merge regardless of CI + waiting or authorized admin bypass. Working-tree changes cannot satisfy it. +4. Immediately before the merge, run `loopx --format json pr-review --goal-id GOAL --repo OWNER/REPO --check-merge-readiness NUMBER@HEAD_OID`. Merge only when it returns `ready=true` for that unchanged head; this records the compact Goal readiness observation consumed by future review queues. diff --git a/tests/canary/test_rfc_bilingual.py b/tests/canary/test_rfc_bilingual.py new file mode 100644 index 0000000000..dc6126feae --- /dev/null +++ b/tests/canary/test_rfc_bilingual.py @@ -0,0 +1,252 @@ +"""Exercise the bilingual gate through real staged and committed Git diffs.""" + +from pathlib import Path +import json +import shutil +import subprocess +import sys +import tempfile +import unittest + +SCRIPT = Path(__file__).resolve().parents[2] / "scripts/check_rfc_bilingual.py" +ROOT = "docs/architecture/rfcs/" +EN, ZH = ROOT + "alpha.md", ROOT + "alpha.zh-CN.md" + + +class GitFixture(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.repo = Path(self.temp.name) + self.git("init", "-q") + self.git("config", "user.name", "Fixture") + self.git("config", "user.email", "fixture@example.invalid") + self.git("config", "core.hooksPath", "/dev/null") + for path in (EN, ZH, ROOT + "legacy.md"): + self.write(path, "Original\n") + self.commit() + self.base = self.git("rev-parse", "HEAD").strip() + + def git(self, *args): + return subprocess.check_output(["git", *args], cwd=self.repo, text=True) + + def write(self, path, text="Changed\n"): + target = self.repo / path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(text, encoding="utf-8") + + def commit(self): + # This is a disposable synthetic repository, never the source checkout. + self.git("add", "--all") + self.git("commit", "-q", "-m", "fixture") + + def check(self, *args): + return subprocess.run( + [sys.executable, str(SCRIPT), *args], cwd=self.repo, + capture_output=True, text=True, check=False, + ) + + def branch_check(self): + return self.check("--base", self.base, "--head", "HEAD") + + +class BilingualDiffTest(GitFixture): + def test_english_only_fails_with_the_exact_missing_path(self): + self.write(EN) + self.commit() + result = self.branch_check() + self.assertEqual(result.returncode, 1) + self.assertIn(ZH, result.stderr) + + def test_chinese_only_fails(self): + self.write(ZH) + self.commit() + self.assertEqual(self.branch_check().returncode, 1) + + def test_pair_can_be_changed_in_separate_commits(self): + self.write(EN) + self.commit() + self.write(ZH) + self.git("add", "--", ZH) + self.assertEqual(self.check("--base", self.base, "--staged").returncode, 0) + self.commit() + self.assertEqual(self.branch_check().returncode, 0) + + def test_only_staged_changes_satisfy_the_commit_check(self): + self.write(EN) + self.git("add", "--", EN) + self.write(ZH) + self.assertEqual(self.check("--staged").returncode, 1) + self.git("add", "--", ZH) + self.assertEqual(self.check("--staged").returncode, 0) + + def test_target_branch_change_does_not_satisfy_the_pr_pair(self): + self.git("checkout", "-q", "-b", "feature") + self.write(EN) + self.commit() + feature = self.git("rev-parse", "HEAD").strip() + self.git("checkout", "-q", "-b", "target", self.base) + self.write(ZH) + self.commit() + result = self.check("--base", "HEAD", "--head", feature) + self.assertEqual(result.returncode, 1) + self.assertIn(ZH, result.stderr) + + def test_deletion_requires_both_mirrors(self): + (self.repo / EN).unlink() + self.commit() + self.assertEqual(self.branch_check().returncode, 1) + (self.repo / ZH).unlink() + self.commit() + self.assertEqual(self.branch_check().returncode, 0) + + def test_rename_requires_both_mirrors(self): + self.git("mv", EN, ROOT + "renamed.md") + self.commit() + self.assertEqual(self.branch_check().returncode, 1) + self.git("mv", ZH, ROOT + "renamed.zh-CN.md") + self.commit() + self.assertEqual(self.branch_check().returncode, 0) + + def test_ledger_and_non_ascii_paths_are_covered(self): + en = ROOT + "ledger/alpha/双语 entry.md" + zh = ROOT + "ledger/alpha/双语 entry.zh-CN.md" + self.write(en) + self.write(zh) + self.commit() + self.base = self.git("rev-parse", "HEAD").strip() + self.write(en, "Revised\n") + self.commit() + result = self.branch_check() + self.assertEqual(result.returncode, 1) + self.assertIn(zh, result.stderr) + + def test_legacy_single_language_and_unrelated_changes_pass(self): + self.write(ROOT + "legacy.md") + self.write("docs/ordinary.md") + self.commit() + self.assertEqual(self.branch_check().returncode, 0) + + def test_new_pair_passes_and_partial_new_mirror_fails(self): + self.write(ROOT + "new.md") + self.write(ROOT + "new.zh-CN.md") + self.commit() + self.assertEqual(self.branch_check().returncode, 0) + self.write(ROOT + "legacy.zh-CN.md") + self.commit() + self.assertEqual(self.branch_check().returncode, 1) + + def test_missing_revision_fails_closed(self): + self.assertEqual(self.check("--base", "absent", "--head", "HEAD").returncode, 1) + + def test_staged_head_argument_is_rejected(self): + self.assertEqual(self.check("--staged", "--head", "HEAD").returncode, 2) + + +class PremergeTest(GitFixture): + def setUp(self): + super().setUp() + from loopx.canary.planner import DEFAULT_CATALOG_PATH + + checker = self.repo / "scripts/check_rfc_bilingual.py" + checker.parent.mkdir() + shutil.copyfile(SCRIPT, checker) + examples = self.repo / "examples" + examples.mkdir() + (examples / "fixture-smoke.py").write_text("print('fixture')\n") + self.write("fixture.txt", "Synthetic premerge validation input\n") + catalog = self.repo / "docs/concepts/interaction-pattern-catalog.md" + catalog.parent.mkdir() + shutil.copyfile(DEFAULT_CATALOG_PATH, catalog) + self.commit() + self.base = self.git("rev-parse", "HEAD").strip() + + def premerge_cli(self): + # This small repository supplies an explicit validation selector rather + # than claiming it contains LoopX's full semantic-canary source tree. + # RFC pairing must still inspect its complete real committed Git diff. + result = subprocess.run( + [sys.executable, "-m", "loopx.cli", "--format", "json", "canary", "premerge", + "--changed-file", "fixture.txt", "--git-diff-base", self.base, + "--tier", "quick", "--no-progress"], + cwd=self.repo, capture_output=True, text=True, check=False, + ) + self.assertTrue(result.stdout, result.stderr) + return result, json.loads(result.stdout) + + def test_real_premerge_cli_rejects_a_single_language_even_in_quick_tier(self): + self.write(EN) + self.commit() + result, payload = self.premerge_cli() + check = next(c for c in payload["direct_checks"] if c["id"] == "rfc_bilingual_diff") + self.assertEqual(result.returncode, 1) + self.assertFalse(payload["gate"]["merge_gate_passed"]) + self.assertEqual(check["status"], "failed") + self.assertIn(ZH, check["stderr_tail"]) + + def test_real_premerge_cli_accepts_a_committed_pair(self): + self.write(EN) + self.write(ZH) + self.commit() + result, payload = self.premerge_cli() + self.assertEqual(result.returncode, 0, result.stderr + result.stdout) + self.assertTrue(payload["gate"]["merge_gate_passed"]) + check = next(c for c in payload["direct_checks"] if c["id"] == "rfc_bilingual_diff") + self.assertEqual(check["status"], "passed") + + def test_omitted_change_selector_cannot_skip_the_direct_gate(self): + from loopx.canary.premerge import build_premerge_validation_gate + + self.write(EN) + self.commit() + payload = build_premerge_validation_gate( + repo_root=self.repo, changed_files=[], base_ref=self.base, tier="quick", execute=True, + ) + self.assertFalse(payload["ok"]) + self.assertFalse(payload["gate"]["merge_gate_passed"]) + self.assertEqual(payload["gate"]["direct_failure_count"], 1) + + def test_uncommitted_translation_does_not_satisfy_the_merge_gate(self): + self.write(EN) + self.commit() + self.write(ZH) + result, payload = self.premerge_cli() + self.assertEqual(result.returncode, 1) + self.assertFalse(payload["gate"]["merge_gate_passed"]) + + def test_deleting_a_registered_checker_fails_instead_of_skipping(self): + (self.repo / "scripts/check_rfc_bilingual.py").unlink() + self.write(EN) + self.commit() + result, payload = self.premerge_cli() + self.assertEqual(result.returncode, 1) + check = next(c for c in payload["direct_checks"] if c["id"] == "rfc_bilingual_diff") + self.assertEqual(check["status"], "failed") + + def test_preview_does_not_claim_a_passed_merge_gate(self): + from loopx.canary.premerge import build_premerge_validation_gate + + payload = build_premerge_validation_gate( + repo_root=self.repo, changed_files=[EN], base_ref=self.base, tier="quick", execute=False, + ) + check = next(c for c in payload["direct_checks"] if c["id"] == "rfc_bilingual_diff") + self.assertEqual(check["status"], "ready") + self.assertFalse(payload["gate"]["merge_gate_passed"]) + + +class UnregisteredRepositoryTest(GitFixture): + def test_other_repositories_retain_their_original_direct_checks(self): + from loopx.canary.premerge import build_premerge_validation_gate + + payload = build_premerge_validation_gate( + repo_root=self.repo, changed_files=[], base_ref=self.base, tier="quick", execute=True, + ) + self.assertEqual( + [c["id"] for c in payload["direct_checks"]], + ["diff_check_committed", "diff_check_staged", "diff_check_unstaged"], + ) + self.assertTrue(payload["ok"]) + + +if __name__ == "__main__": + unittest.main()