From 9d2a9f632835e89f4d0f4dc15f8c727129a10e23 Mon Sep 17 00:00:00 2001 From: Kaden Schutt <151092359+Kaden-Schutt@users.noreply.github.com> Date: Sat, 5 Sep 2026 01:02:44 +0000 Subject: [PATCH] fix(hw-gate): merge past a generated-crate-map 409 instead of holding Every rung of the 2026-09-04 ladder hit the same 409 on the staging merge: `crates/*/map.md` carries a `` block that both branches regenerate, so any two PRs touching the same crate conflict there while their real code merges cleanly. #689, #690, #691, #686, #687, #688 and #682 all needed the same three manual steps -- merge staging in, regenerate the block with scripts/check-crate-maps.py, merge -- six of them tonight. A gate that decides merge-staging and then holds on a generated file is asking a human to run a script, which is not review. On a 409 the decide phase now retries locally: merge staging into the PR head, and if the conflicted set is generated maps only, re-run check-crate-maps.py for those crates, commit, and merge the result. The retry is deliberately narrow, because auto-resolving conflicts is exactly where a gate can do damage: - if ANY conflicted path is not a `map.md`, it declines and the hold stands with the offending paths named -- a real code conflict must reach a human - it regenerates rather than picking a side, so the committed block is what the tree actually generates, not whichever branch won - a failed regeneration, a git error, or a timeout all decline rather than force Test: `test_generated_map_retry_refuses_real_code_conflicts` builds a real repo with a conflicting `.rs` and asserts the retry returns no merge SHA and names the file. The guard is the part worth pinning; the happy path is exercised by the ladder itself. 122/122 hw-gate tests pass. --- scripts/hw-gate/review.py | 89 +++++++++++++++++++++++++--- scripts/hw-gate/tests/test_review.py | 33 +++++++++++ 2 files changed, 115 insertions(+), 7 deletions(-) diff --git a/scripts/hw-gate/review.py b/scripts/hw-gate/review.py index 3a9fda187..836c2a331 100755 --- a/scripts/hw-gate/review.py +++ b/scripts/hw-gate/review.py @@ -1265,6 +1265,67 @@ def _has_hold(r: str) -> bool: # decide phase (fable) # --------------------------------------------------------------------------- +def _resolve_generated_map_conflict(repo: str, checkout: str, staging: str, head: str, commit_msg: str) -> tuple[str | None, str]: + """Retry a 409 staging merge when the only conflicts are generated maps. + + Every rung on the 2026-09-04 ladder hit the same 409: `crates/*/map.md` + carries a `` block that both branches + regenerate, so any two PRs touching the same crate conflict there while + their real code merges cleanly. Six merges were unblocked by hand with + exactly the steps below. + + The retry is deliberately narrow. It merges locally, and if ANY conflicted + path is not a `map.md`, it gives up and leaves the hold in place: a real + code conflict must reach a human. For the generated files it re-runs + scripts/check-crate-maps.py rather than picking a side, so the committed + block is what the tree actually generates. + + Returns (merge_sha, note). `merge_sha` is None when the caller should hold. + """ + def git(*args: str, check: bool = True) -> str: + r = subprocess.run(["git", "-C", checkout, *args], capture_output=True, text=True, timeout=300) + if check and r.returncode != 0: + raise ReviewError(f"git {' '.join(args)}: {r.stderr.strip() or r.stdout.strip()}") + return r.stdout + + try: + git("fetch", "--quiet", "origin", staging, head) + git("checkout", "--quiet", "--detach", head) + merge = subprocess.run( + ["git", "-C", checkout, "merge", "--no-edit", "-m", f"Merge {staging} into PR head for staging", f"origin/{staging}"], + capture_output=True, text=True, timeout=600, + ) + if merge.returncode != 0: + conflicts = [p for p in git("diff", "--name-only", "--diff-filter=U").split() if p] + if not conflicts: + return None, f"local merge failed with no conflicted paths: {merge.stderr.strip()[:300]}" + non_generated = [p for p in conflicts if not p.endswith("/map.md")] + if non_generated: + return None, f"real code conflicts, not just generated maps: {', '.join(non_generated[:6])}" + crates = sorted({p.split("/")[1] for p in conflicts if p.startswith("crates/")}) + git("checkout", "--ours", "--", *conflicts) + regen = subprocess.run( + [sys.executable, "scripts/check-crate-maps.py", *crates], + cwd=checkout, capture_output=True, text=True, timeout=600, + ) + if regen.returncode not in (0, 1): + return None, f"check-crate-maps.py failed for {crates}: {regen.stderr.strip()[:200]}" + git("add", "--", *conflicts) + git("commit", "--quiet", "--no-edit") + merged_head = git("rev-parse", "HEAD").strip() + git("push", "--quiet", "origin", f"HEAD:refs/heads/{head[:12]}-staging-merge") + out = _gh(["api", f"repos/{repo}/merges", "-f", f"base={staging}", "-f", f"head={merged_head}", + "-f", f"commit_message={commit_msg} (generated crate maps regenerated)"]) + try: + resp = json.loads(out) if out.strip() else {} + sha = resp.get("sha") if isinstance(resp, dict) else None + except Exception: + sha = out.strip() or None + return (sha or merged_head), f"generated maps regenerated for {', '.join(crates) if merge.returncode != 0 else 'none'}" + except (ReviewError, subprocess.TimeoutExpired, OSError) as exc: + return None, f"generated-map retry failed: {exc}" + + def _run_decide(args) -> int: # Load select try: @@ -1546,14 +1607,28 @@ def _has_hold(r: str) -> bool: merged = {"base": staging, "head": args.head, "merge_sha": merge_sha} except ReviewError as e: err_msg = str(e) - merged = {"base": staging, "head": args.head, "merge_sha": None, "error": err_msg} is_409 = "409" in err_msg or "already" in err_msg.lower() or "conflict" in err_msg.lower() - decision_final = "hold" - hard.append("staging_merge_failed" if not is_409 else "staging_merge_conflict") - announcement_extra = ( - f" Fable decided merge-staging, but merging into `{staging}` failed" - f"{' with a conflict (409)' if is_409 else ''}: {err_msg}. Holding for a human." - ) + retry_sha, retry_note = (None, "") + if is_409: + retry_sha, retry_note = _resolve_generated_map_conflict( + args.repo, args.checkout, staging, args.head, commit_msg + ) + if retry_sha: + merged = {"base": staging, "head": args.head, "merge_sha": retry_sha, "retry": retry_note} + announcement_extra = ( + f" The first merge into `{staging}` hit a 409 on generated crate maps only;" + f" merged after regenerating them ({retry_note})." + ) + else: + merged = {"base": staging, "head": args.head, "merge_sha": None, "error": err_msg, + "retry": retry_note or None} + decision_final = "hold" + hard.append("staging_merge_failed" if not is_409 else "staging_merge_conflict") + announcement_extra = ( + f" Fable decided merge-staging, but merging into `{staging}` failed" + f"{' with a conflict (409)' if is_409 else ''}: {err_msg}." + f"{' Retry: ' + retry_note + '.' if retry_note else ''} Holding for a human." + ) announcement = "" if isinstance(decision, dict): announcement = decision.get("announcement", "") diff --git a/scripts/hw-gate/tests/test_review.py b/scripts/hw-gate/tests/test_review.py index d3a661dc8..3d53703b7 100644 --- a/scripts/hw-gate/tests/test_review.py +++ b/scripts/hw-gate/tests/test_review.py @@ -897,3 +897,36 @@ def test_decision_records_the_commit_it_judged(): assert data["base"] == base, data.get("base") + + +def test_generated_map_retry_refuses_real_code_conflicts(tmp_path): + """The 409 retry must be narrow. + + Every rung on the 2026-09-04 ladder hit a 409 on `crates/*/map.md`, whose + `` block both branches regenerate; six merges + were unblocked by hand. Automating that is only safe if a genuine code + conflict still stops at a human, so this asserts the guard rather than the + happy path: a conflicted .rs makes the retry decline. + """ + import subprocess as sp + repo = tmp_path / "r" + repo.mkdir() + sp.run(["git", "init", "-q", "-b", "main", str(repo)], check=True) + sp.run(["git", "-C", str(repo), "config", "user.email", "t@t"], check=True) + sp.run(["git", "-C", str(repo), "config", "user.name", "t"], check=True) + (repo / "a.rs").write_text("fn main() {}\n") + sp.run(["git", "-C", str(repo), "add", "-A"], check=True) + sp.run(["git", "-C", str(repo), "commit", "-qm", "base"], check=True) + base = sp.run(["git", "-C", str(repo), "rev-parse", "HEAD"], capture_output=True, text=True).stdout.strip() + sp.run(["git", "-C", str(repo), "checkout", "-q", "-b", "other"], check=True) + (repo / "a.rs").write_text("fn main() { other() }\n") + sp.run(["git", "-C", str(repo), "commit", "-aqm", "other"], check=True) + sp.run(["git", "-C", str(repo), "checkout", "-q", base], check=True) + sp.run(["git", "-C", str(repo), "checkout", "-q", "-b", "head"], check=True) + (repo / "a.rs").write_text("fn main() { head() }\n") + sp.run(["git", "-C", str(repo), "commit", "-aqm", "head"], check=True) + # `origin` points at itself so the helper's fetch resolves without a network + sp.run(["git", "-C", str(repo), "remote", "add", "origin", str(repo)], check=True) + sha, note = review._resolve_generated_map_conflict("o/r", str(repo), "other", "head", "msg") + assert sha is None, "a conflicted .rs must not be auto-merged" + assert "a.rs" in note or "real code conflicts" in note, note