From d8d6e2a97547d388e74091f9d407beef5d73b079 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 21 Oct 2026 23:59:52 -0700 Subject: [PATCH 1/8] merge-compile-guard tests: only a compile that finished red refuses (red) The guard's GitHub reads timed out under load on cosmic-gray and refused #23660 and #23371 for hours with a SHIP receipt at the head and a green compile. These tests expect a pending compile, an unreadable GitHub, a merge the guard can't name and guards it can't load to warn and let the merge through. Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a1602e-bc09-7990-8c75-5c412a66b114&host=Air-Blue --- tests/test_guard_hooks_from_base.py | 23 ++++-- tests/test_merge_compile_guard.py | 105 +++++++++++++++++++++------- 2 files changed, 96 insertions(+), 32 deletions(-) diff --git a/tests/test_guard_hooks_from_base.py b/tests/test_guard_hooks_from_base.py index da0416841a16..7860064e5e26 100644 --- a/tests/test_guard_hooks_from_base.py +++ b/tests/test_guard_hooks_from_base.py @@ -126,21 +126,34 @@ def test_a_stale_origin_is_fetched_before_a_merge(self): self.assertEqual(code, 0, err) self.assertEqual(out["hookSpecificOutput"]["permissionDecisionReason"], "merge guard base v1") - def test_a_merge_whose_guards_cannot_be_read_is_refused(self): - """The merge guard fails closed (it refuses when it can't read GitHub), so the hook does too: a merge it can't - judge is denied, never let through with only a stderr line.""" + def test_a_merge_whose_guards_cannot_be_read_goes_ahead_with_a_warning(self): + """The merge guard refuses only a compile that finished red; a merge it can't judge, because GitHub or the + guards could not be read, goes ahead with a warning the agent and the person both see (2026-10-21: the + guard's timed-out reads held #23660 and #23371 for hours with every gate met).""" old = self.old_base() git(self.lane, "remote", "set-url", "origin", str(self.tmp / "gone.git")) git(self.lane, "update-ref", "refs/remotes/origin/feat-cmux-next", old) code, out, err = self.hook("gh pr merge 5 --squash") self.assertEqual(code, 0, err) - self.assertEqual(out["hookSpecificOutput"]["permissionDecision"], "deny") - self.assertIn("merge guards did not run", out["hookSpecificOutput"]["permissionDecisionReason"]) + self.assertNotIn("permissionDecision", out["hookSpecificOutput"]) + self.assertIn("merge guards did not run", out["hookSpecificOutput"]["additionalContext"]) + self.assertEqual(out["systemMessage"], out["hookSpecificOutput"]["additionalContext"]) # The update-branch warning only warns: unreadable, it does not block. code, out, err = self.hook("gh api -X PUT repos/manaflow-ai/cmux/pulls/5/update-branch") self.assertIsNone(out) self.assertNotEqual(code, 2) + def test_a_base_without_the_guards_lets_a_merge_through_with_a_warning(self): + """guards-from-base.sh itself is on the base but the guards are not (`git archive` fails).""" + git(self.seed, "rm", "-q", "scripts/ci/merge_compile_guard.py") + git(self.seed, "commit", "-qm", "the guard leaves the base") + git(self.seed, "push", "-q", "origin", "feat-cmux-next") + code, out, err = self.hook("gh pr merge 5 --squash") + self.assertEqual(code, 0, err) + self.assertNotIn("permissionDecision", out["hookSpecificOutput"]) + self.assertIn("cannot read the guards", out["systemMessage"]) + self.assertEqual(out["systemMessage"], out["hookSpecificOutput"]["additionalContext"]) + def old_base(self) -> str: """A commit on the base from before the launcher existed.""" old = self.tmp / "old" diff --git a/tests/test_merge_compile_guard.py b/tests/test_merge_compile_guard.py index c995f8728284..77ae6eb382cb 100644 --- a/tests/test_merge_compile_guard.py +++ b/tests/test_merge_compile_guard.py @@ -1,7 +1,10 @@ #!/usr/bin/env python3 -"""merge_compile_guard: a hand merge of a feat-cmux-next PR that changes Swift waits for its Release compile. +"""merge_compile_guard: a hand merge of a feat-cmux-next PR that changes Swift is refused only once its compile +finished red; a compile still pending, and the guard's own slow or failed read, are warnings. -On 2026-10-21 (UTC) #22835, #21480 and #21487 were merged with `gh pr merge` while their compile was pending. +On 2026-10-21 (UTC) #22835, #21480 and #21487 were merged with `gh pr merge` while their compile was pending. Later +that day, under load on cosmic-gray, the guard's GitHub reads timed out and refused #23660 and #23371 for hours with +every gate met (Leo: "IF THE MERGE HOOK IS WRONG THEN JUST FUCKING MERGE"). """ from __future__ import annotations @@ -44,6 +47,21 @@ def test_a_swift_pr_merges_only_once_its_compile_passed(self): # A rerun that passed after a red counts. self.assertEqual(guard.verdict(pr(), SWIFT, [check(COMPILE, conclusion="failure"), check(COMPILE)]), "") + def test_only_a_compile_that_finished_red_refuses(self): + """Everything else is a Hold: the hook says it and the merge goes ahead (HQ's landing gate does the same).""" + for checks in ([], [check(COMPILE, "queued")], [check(COMPILE, "in_progress")], + [check("cmux-next path routing", "in_progress")], [check(COMPILE, conclusion="cancelled")], + [check(guard.FLEET_COMPILE, conclusion="cancelled")]): + with self.subTest(checks=checks): + self.assertIsInstance(guard.verdict(pr(), SWIFT, checks), guard.Hold) + for checks in ([check(COMPILE, conclusion="failure")], [check(COMPILE, conclusion="timed_out")], + [check(guard.FLEET_COMPILE, conclusion="failure")], + [check(COMPILE, conclusion="failure"), check(COMPILE, "queued")]): + with self.subTest(checks=checks): + why = guard.verdict(pr(), SWIFT, checks) + self.assertTrue(why) + self.assertNotIsInstance(why, guard.Hold) + def test_a_compile_not_created_yet_waits_while_cmux_next_routes_and_places(self): for job in guard.ROUTING: with self.subTest(job=job): @@ -164,7 +182,7 @@ def test_the_fleet_compile_check_refusal_reads_whole_with_several_or_oddly_named ([base + "Find and Replace.swift"], f"({base}Find and Replace.swift)")): why = guard.verdict(pr(), swift, []) self.assertTrue(why.startswith(f"#7 changes Swift {what} and {HEAD[:12]} has no compile result yet."), why) - self.assertTrue(why.endswith(f"({HEAD[:12]} has no checks yet; merge once its {COMPILE} passes.)"), why) + self.assertTrue(why.endswith(f"({HEAD[:12]} has no checks yet.)"), why) self.assertEqual(why.count("cmux-ci compile-check"), 1, why) def test_a_refusal_names_the_cause_of_its_own_read_not_an_earlier_one(self): @@ -446,29 +464,42 @@ def test_other_repositories_are_not_guarded(self): reader = lambda *a: self.fail("read another repo") # noqa: E731 self.assertEqual(guard.refusal("gh pr merge 662 -R teamleaderleo/leo-workspace --squash", None, reader), "") - def test_a_merge_in_a_checkout_it_cannot_read_is_refused_unless_it_names_the_repo(self): + def test_a_merge_in_a_checkout_it_cannot_read_is_a_warning_unless_it_names_the_repo(self): for command in ('ssh host "cd wt && gh pr merge 5"', 'cd "$WT" && gh pr merge 5'): with self.subTest(command=command): self.assertEqual(guard.merge_targets(command, "/tmp")[0][2], guard.REMOTE) - self.assertIn("can't read", guard.refusal(command, "/tmp", lambda *a: self.fail("read"))) + why = guard.refusal(command, "/tmp", lambda *a: self.fail("read")) + self.assertIn("can't read", why) + self.assertIsInstance(why, guard.Hold) self.assertEqual(guard.refusal('ssh host "gh pr merge 5 -R manaflow-ai/cmux"', "/tmp", lambda *a: ""), "") # Unquoted, ssh joins the words into the remote command. self.assertEqual(guard.merge_targets("ssh -p 22 cmux16 gh pr merge 5 -R manaflow-ai/cmux", "/tmp"), [("manaflow-ai/cmux", "5", guard.REMOTE)]) - def test_a_merge_whose_checkout_cannot_be_read_is_refused(self): + def test_a_merge_whose_checkout_cannot_be_read_is_a_warning(self): # Out of time (or no git): the checkout's repository is unknown, so the merge can't be judged. spent = guard.STARTED try: guard.STARTED = guard.time.monotonic() - guard.BUDGET - self.assertIn("can't tell which repository", guard.refusal("gh pr merge 5 --squash", "/tmp", - lambda *a: self.fail("read"))) + why = guard.refusal("gh pr merge 5 --squash", "/tmp", lambda *a: self.fail("read")) + self.assertIn("can't tell which repository", why) + self.assertIsInstance(why, guard.Hold) finally: guard.STARTED = spent - def test_a_merge_it_cannot_name_is_refused(self): + def test_a_merge_it_cannot_name_is_a_warning(self): why = guard.refusal('gh pr merge "$PR" -R manaflow-ai/cmux', None, lambda *a: self.fail("read")) self.assertIn("can't name", why) + self.assertIsInstance(why, guard.Hold) + + def test_a_warning_never_hides_a_red_compile_later_in_the_command(self): + red = "#5 changes Swift (A.swift) and its compile finished failure on cccccccccccc." + why = guard.refusal('gh pr merge "$PR" -R manaflow-ai/cmux; gh pr merge 5 -R manaflow-ai/cmux', None, + lambda *a: red) + self.assertEqual(why, f"merge-compile-guard: {red}") + self.assertNotIsInstance(why, guard.Hold) + held = guard.refusal("gh pr merge 5 -R manaflow-ai/cmux", None, lambda *a: guard.Hold("still running")) + self.assertIsInstance(held, guard.Hold) def fake_gh(bin_dir: Path, answers: dict[str, str], log: Path, throttle: float = 0, sleep: float = 0) -> None: @@ -497,6 +528,21 @@ def fake_gh(bin_dir: Path, answers: dict[str, str], log: Path, throttle: float = class Hook(unittest.TestCase): + @staticmethod + def denial(out) -> str | None: + """The hook's refusal, or None when the command goes ahead (with or without a warning).""" + decision = (out or {}).get("hookSpecificOutput") or {} + return decision.get("permissionDecisionReason") if decision.get("permissionDecision") == "deny" else None + + def warning(self, out) -> str: + """What the hook says about a merge it lets through: shown to the person and added to the agent's context.""" + self.assertIsNotNone(out) + self.assertIsNone(self.denial(out), out) + self.assertNotIn("permissionDecision", out["hookSpecificOutput"]) + self.assertEqual(out["systemMessage"], out["hookSpecificOutput"]["additionalContext"]) + self.assertIn("merging anyway", out["systemMessage"]) + return out["systemMessage"] + def run_hook(self, command: str, compile_state: tuple[str, str | None] | None, statuses=None, fleet_runs=None, other_statuses=None, on_base="ahead", head=HEAD, cmux_ci_jobs=(), cmux_ci_receipts=None, gh_throttle=0, gh_sleep=0, env_extra=None): @@ -542,11 +588,16 @@ def run_hook(self, command: str, compile_state: tuple[str, str | None] | None, s calls = log.read_text().splitlines() if log.exists() else [] return (json.loads(done.stdout) if done.stdout.strip() else None), calls - def test_a_merge_with_the_compile_pending_is_refused(self): + def test_a_merge_with_the_compile_pending_goes_ahead_with_a_warning(self): out, _ = self.run_hook("gh pr merge 22835 -R manaflow-ai/cmux --squash", ("in_progress", None)) - decision = out["hookSpecificOutput"] - self.assertEqual(decision["permissionDecision"], "deny") - self.assertIn("#22835 changes Swift", decision["permissionDecisionReason"]) + why = self.warning(out) + self.assertIn("#22835 changes Swift", why) + self.assertIn("still running", why) + + def test_a_merge_after_a_red_compile_is_refused(self): + out, _ = self.run_hook("gh pr merge 22835 -R manaflow-ai/cmux --squash", ("completed", "failure")) + self.assertIn("#22835 changes Swift", self.denial(out)) + self.assertIn("finished failure", self.denial(out)) def test_a_merge_after_a_green_compile_goes_ahead(self): out, calls = self.run_hook("gh pr merge 22835 -R manaflow-ai/cmux --squash", ("completed", "success")) @@ -556,7 +607,7 @@ def test_a_merge_after_a_green_compile_goes_ahead(self): def test_a_bare_merge_checks_the_checkouts_branch(self): """gh refuses `pr view -R` without a selector, so the branch is looked up in the checkout.""" out, calls = self.run_hook("gh pr merge --squash 2>&1", ("in_progress", None)) - self.assertIn("#22835 changes Swift", out["hookSpecificOutput"]["permissionDecisionReason"]) + self.assertIn("#22835 changes Swift", self.warning(out)) view = [json.loads(c) for c in calls if json.loads(c)[:2] == ["pr", "view"]] self.assertEqual(view, [["pr", "view", "--json", "number"]]) out, _ = self.run_hook("gh pr merge --squash", ("completed", "success")) @@ -571,17 +622,18 @@ def test_the_gh_guards_repeat_throttle_does_not_slow_the_guard(self): self.assertIsNone(out) self.assertLess(time.monotonic() - started, 10) - def test_a_slow_github_refuses_and_says_it_timed_out(self): + def test_a_slow_github_lets_the_merge_through_and_says_it_timed_out(self): + """The guard's own read timing out is no evidence against the PR (#23660 and #23371, 2026-10-21).""" out, _ = self.run_hook("gh pr merge 22835 -R manaflow-ai/cmux --squash", ("completed", "success"), gh_sleep=5, env_extra={"CMUX_MERGE_GUARD_GH_TIMEOUT": "1"}) - why = out["hookSpecificOutput"]["permissionDecisionReason"] + why = self.warning(out) self.assertIn("could not read #22835 from GitHub", why) self.assertIn("did not answer within 1 s", why) self.assertNotIn("rate limit", why) - def test_an_unreadable_github_refuses(self): + def test_an_unreadable_github_lets_the_merge_through(self): out, _ = self.run_hook("gh pr merge 1 -R manaflow-ai/cmux --squash", None) - self.assertIn("could not read #1 from GitHub", out["hookSpecificOutput"]["permissionDecisionReason"]) + self.assertIn("could not read #1 from GitHub", self.warning(out)) MERGE = "gh pr merge 22835 -R manaflow-ai/cmux --squash" @@ -594,15 +646,15 @@ def test_a_fleet_pass_on_a_different_sha_does_not_count(self): # The pass was posted on another commit (an older head): the head has no status. out, _ = self.run_hook(self.MERGE, ("queued", None), statuses=[], other_statuses=[status()], fleet_runs={99: fleet_run(sha="d" * 40)}) - self.assertIn("still running", out["hookSpecificOutput"]["permissionDecisionReason"]) + self.assertIn("still running", self.warning(out)) # A status on the head that links a run which compiled another sha. out, _ = self.run_hook(self.MERGE, ("queued", None), statuses=[status()], fleet_runs={99: fleet_run(sha="d" * 40)}) - self.assertIn("still running", out["hookSpecificOutput"]["permissionDecisionReason"]) + self.assertIn("still running", self.warning(out)) # A run that names only a prefix of the head. out, _ = self.run_hook(self.MERGE, ("queued", None), statuses=[status()], fleet_runs={99: fleet_run(sha=HEAD[:12])}) - self.assertIn("still running", out["hookSpecificOutput"]["permissionDecisionReason"]) + self.assertIn("still running", self.warning(out)) def test_a_self_posted_or_unverified_fleet_status_does_not_count(self): cases = { @@ -621,15 +673,14 @@ def test_a_self_posted_or_unverified_fleet_status_does_not_count(self): for name, (statuses, fleet_runs) in cases.items(): with self.subTest(name): out, _ = self.run_hook(self.MERGE, ("queued", None), statuses=statuses, fleet_runs=fleet_runs) - self.assertIsNotNone(out, name) - self.assertEqual(out["hookSpecificOutput"]["permissionDecision"], "deny") + self.assertIn("no compile result yet", self.warning(out), name) def test_a_run_whose_workflow_commit_left_the_branch_does_not_count(self): for on_base in ("diverged", "behind", None): with self.subTest(on_base=on_base): out, _ = self.run_hook(self.MERGE, ("queued", None), statuses=[status()], fleet_runs={99: fleet_run()}, on_base=on_base) - self.assertEqual(out["hookSpecificOutput"]["permissionDecision"], "deny") + self.assertIn("no compile result yet", self.warning(out)) def test_every_attempt_of_the_compile_is_read(self): out, calls = self.run_hook(self.MERGE, ("queued", None), statuses=[status()], fleet_runs={99: fleet_run()}) @@ -638,7 +689,7 @@ def test_every_attempt_of_the_compile_is_read(self): def test_a_fleet_pass_does_not_let_a_red_release_compile_merge(self): out, calls = self.run_hook(self.MERGE, ("completed", "failure"), statuses=[status()], fleet_runs={99: fleet_run()}) - self.assertIn("finished failure", out["hookSpecificOutput"]["permissionDecisionReason"]) + self.assertIn("finished failure", self.denial(out)) self.assertFalse(any("statuses" in " ".join(json.loads(c)) for c in calls)) def test_a_newer_unverified_status_does_not_hide_a_verified_pass(self): @@ -656,13 +707,13 @@ def test_the_23606_cmux_ci_pass_lets_its_queued_compile_merge_without_a_dispatch def test_a_cmux_ci_pass_on_another_head_refuses_and_says_what_to_run(self): out, _ = self.run_hook(self.MERGE, ("queued", None), statuses=[], cmux_ci_jobs=[job()]) - why = out["hookSpecificOutput"]["permissionDecisionReason"] + why = self.warning(out) self.assertIn("still running", why) self.assertIn(f"cmux-ci compile-check --ref {HEAD} --app", why) def test_a_cmux_ci_pass_does_not_let_a_red_release_compile_merge(self): out, calls = self.run_hook(self.MERGE, ("completed", "failure"), head=HEAD_23606, cmux_ci_jobs=[job()]) - self.assertIn("finished failure", out["hookSpecificOutput"]["permissionDecisionReason"]) + self.assertIn("finished failure", self.denial(out)) self.assertNotIn(["cmux-ci", "status"], [json.loads(c) for c in calls]) def test_an_unreachable_controller_falls_back_to_the_fleet_status(self): From 89b2e8ecd1abb44feb1f2747dcb270604ea6c841 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 22 Oct 2026 00:03:40 -0700 Subject: [PATCH 2/8] merge-compile-guard: only a compile that finished red refuses; the rest warns and merges Under load on cosmic-gray (2026-10-21) the guard's GitHub reads timed out and refused #23660 and #23371 for hours with a SHIP receipt at the head and a green compile. Leo: "IF THE MERGE HOOK IS WRONG THEN JUST FUCKING MERGE". HQ's landing gate (require-cmux-guards.py, cmuxterm-hq#2976) already refuses only a missing SHIP, a failed compile, --admin and an unpinned --auto; this repo hook now agrees with it. - A compile still waiting, a GitHub read that failed or ran out of time, a merge the guard can't name and a checkout it can't read are a Hold: a systemMessage plus additionalContext, no permissionDecision, and the merge goes ahead. - A red compile still refuses, and a Hold earlier in the command never hides a red later in it. - guards-from-base.sh and the settings hook warn instead of refusing when they can't load the guards from the base. Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a1602e-bc09-7990-8c75-5c412a66b114&host=Air-Blue --- .claude/settings.json | 2 +- scripts/ci/guards-from-base.sh | 12 +++-- scripts/ci/merge_compile_guard.py | 88 ++++++++++++++++++++----------- tests/test_merge_compile_guard.py | 2 +- 4 files changed, 65 insertions(+), 39 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index d652670b30f7..61fda6a87ad4 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "command": "bash -c 'i=$(cat); case $i in *\"pr merge\"*|*\"/merge\"*) m=1 ;; *update-branch*) m=0 ;; *) exit 0 ;; esac; d=${CLAUDE_PROJECT_DIR:-.}; if [ \"$m\" = 1 ] && command -v timeout >/dev/null 2>&1; then timeout 10 git -C \"$d\" fetch --quiet --no-tags origin feat-cmux-next 2>/dev/null; fi; f=$(mktemp) || exit 1; trap \"rm -f -- \\\"$f\\\"\" EXIT; if ! git -C \"$d\" show origin/feat-cmux-next:scripts/ci/guards-from-base.sh > \"$f\" 2>/dev/null; then why=\"cmux guards: cannot read scripts/ci/guards-from-base.sh at origin/feat-cmux-next, so the merge guards did not run; fetch origin feat-cmux-next and retry\"; if [ \"$m\" = 1 ]; then printf \"{\\\"hookSpecificOutput\\\": {\\\"hookEventName\\\": \\\"PreToolUse\\\", \\\"permissionDecision\\\": \\\"deny\\\", \\\"permissionDecisionReason\\\": \\\"%s\\\"}}\\n\" \"$why\"; exit 0; fi; echo \"$why\" >&2; exit 1; fi; printf %s \"$i\" | bash \"$f\" \"$d\"'", + "command": "bash -c 'i=$(cat); case $i in *\"pr merge\"*|*\"/merge\"*) m=1 ;; *update-branch*) m=0 ;; *) exit 0 ;; esac; d=${CLAUDE_PROJECT_DIR:-.}; if [ \"$m\" = 1 ] && command -v timeout >/dev/null 2>&1; then timeout 10 git -C \"$d\" fetch --quiet --no-tags origin feat-cmux-next 2>/dev/null; fi; f=$(mktemp) || exit 1; trap \"rm -f -- \\\"$f\\\"\" EXIT; if ! git -C \"$d\" show origin/feat-cmux-next:scripts/ci/guards-from-base.sh > \"$f\" 2>/dev/null; then why=\"cmux guards: cannot read scripts/ci/guards-from-base.sh at origin/feat-cmux-next, so the merge guards did not run; fetch origin feat-cmux-next and retry\"; if [ \"$m\" = 1 ]; then printf \"{\\\"systemMessage\\\": \\\"%s; merging anyway\\\", \\\"hookSpecificOutput\\\": {\\\"hookEventName\\\": \\\"PreToolUse\\\", \\\"additionalContext\\\": \\\"%s; merging anyway\\\"}}\\n\" \"$why\" \"$why\"; exit 0; fi; echo \"$why\" >&2; exit 1; fi; printf %s \"$i\" | bash \"$f\" \"$d\"'", "timeout": 120 } ] diff --git a/scripts/ci/guards-from-base.sh b/scripts/ci/guards-from-base.sh index 19cd408ad460..83ce8df4d7b4 100755 --- a/scripts/ci/guards-from-base.sh +++ b/scripts/ci/guards-from-base.sh @@ -9,7 +9,8 @@ # before its Release compile passed; # - base-merge-warning-hook.sh (base_merge_warning.py): warns before an update-branch cancels CI. # The first that answers (a refusal or a note) is the answer; the hook JSON goes to stdout. A merge -# whose guards can't be read from the base is refused. +# whose guards can't be read from the base goes ahead with a warning: only a compile that finished red +# refuses (merge_compile_guard.py). # # Usage: printf '%s' "$payload" | bash guards-from-base.sh CHECKOUT set -uo pipefail @@ -17,12 +18,13 @@ dir="${1:-.}" input=$(&2 diff --git a/scripts/ci/merge_compile_guard.py b/scripts/ci/merge_compile_guard.py index 84fd555edc94..fee9d8f0dadf 100755 --- a/scripts/ci/merge_compile_guard.py +++ b/scripts/ci/merge_compile_guard.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Refuse merging a feat-cmux-next pull request that changes Swift before its head's Release compile passed. +"""Refuse merging a feat-cmux-next pull request that changes Swift once its head's compile finished red. A Claude Code PreToolUse(Bash) hook (`merge_compile_guard.py --claude-hook`, reading the hook's JSON on stdin). On 2026-10-21 (UTC) #22835, #21480 and #21487 were merged by hand with `gh pr merge` while their @@ -8,14 +8,18 @@ It refuses `gh pr merge [N|#N|URL|BRANCH]` (with or without --auto) and `gh api .../pulls/N/merge` when the PR is into feat-cmux-next, changes a Swift (or C header or source, module map, xcconfig, -Xcode project or package manifest) file, and its head's `cmux-next Release compile (Xcode 26)`: +Xcode project or package manifest) file, and its head's `cmux-next Release compile (Xcode 26)` finished +red with no passing rerun. -- is queued or running, or not created yet while cmux-next is still routing and placing the head - (or the head has no checks at all yet); -- or finished red or cancelled with no passing rerun; +Only that refuses (Leo, 2026-10-21: "IF THE MERGE HOOK IS WRONG THEN JUST FUCKING MERGE"; HQ's landing gate, +tools/agent-guards/require-cmux-guards.py, does the same). Everything else is a Hold: the hook says it to the +person and the agent, and the merge goes ahead. A Hold is a compile still waiting (queued, running, not created +yet while cmux-next routes and places the head, no checks at all yet, or every attempt cancelled, which gives +no verdict), a GitHub read that failed or ran out of time, and a merge the guard can't name or whose +checkout it can't read. Under load on cosmic-gray that day the guard's reads timed out and refused #23660 +and #23371 for hours with a SHIP receipt at the head and a green compile. -unless that compile is still waiting (queued, running, not created yet, or every attempt cancelled, which -gives no verdict) and the build fleet compiled the exact head, read from either record of it: +A waiting compile says nothing when the build fleet compiled the exact head, read from either record of it: - the build fleet's controller (`cmux-ci status`) holds a done `cmux-ci compile-check --ref --app` job from a verified submitter, with exactly the command and pool that client submits (the worker runs @@ -30,7 +34,7 @@ A same-repository PR compiles its head on the build fleet (`cmux-next app compile (build fleet)`) and runs the Release compile only when that gave no verdict: while that job runs, the Release compile does not -exist yet, which is a wait, and its red is a refusal like a red Release compile. +exist yet, which is a Hold, and its red is a refusal like a red Release compile. A Release compile that finished red (any conclusion but success, skipped, neutral or cancelled) stays a refusal, even with a rerun queued: the fleet compiles on Linux with the pool @@ -39,8 +43,8 @@ A compile the router skipped (iOS, packages outside the CmuxNext graph, forks) or a head cmux-next never ran on does not apply, and neither does a PR that changes no Swift. The command is read the -way the shell splits it: quoted text and heredoc bodies are data, never a merge. When GitHub can't be -read for a merge it has to check, it refuses; the merge itself needs GitHub too. +way the shell splits it: quoted text and heredoc bodies are data, never a merge. A Hold never hides a +red: every merge in the command is judged, and any red refuses. """ from __future__ import annotations @@ -117,7 +121,7 @@ def run(args: list[str], cwd: str | None = None) -> str | None: left = BUDGET - (time.monotonic() - STARTED) if left < 1: FAILED[:] = [f"the guard's {BUDGET} s for GitHub reads was spent"] - return None # out of time: an unread answer refuses, or counts as no fleet pass + return None # out of time: an unread answer is a Hold, or counts as no fleet pass timeout = min(TIMEOUT, left) try: done = subprocess.run(args, capture_output=True, text=True, timeout=timeout, cwd=cwd, env=env) @@ -323,8 +327,12 @@ def gh_lines(args: list[str], cwd: str | None): return None +class Hold(str): + """Why the guard would have held a merge it found no red for: the hook warns and the merge goes ahead.""" + + def verdict(pr: dict, files: list[str], checks: list[dict], fleet_passed: bool = False) -> str: - """"" when the merge may go ahead, else why it is refused.""" + """"" when the merge goes ahead silently, a Hold when it goes ahead with a warning, else why it is refused.""" if pr.get("base") != BASE: return "" swift = [f for f in files if changes_swift(f)] @@ -338,9 +346,9 @@ def verdict(pr: dict, files: list[str], checks: list[dict], fleet_passed: bool = # No compile result on the head yet: the default path is the fleet's dev-build workers (COMMON.md), not # waiting on the Mac Release compile. what = changes_what(pr, swift) - return (f"{what} and {pr['head'][:12]} has no compile result yet. Compile-check it on the fleet dev-build " - f"workers: `cmux-ci compile-check --ref {pr['head']} --app`, then merge again; the guard reads that job. " - f"({why.removeprefix(what + ' and ')})") + return Hold(f"{what} and {pr['head'][:12]} has no compile result yet. To compile it now, run " + f"`cmux-ci compile-check --ref {pr['head']} --app` on the fleet dev-build workers. " + f"({why.removeprefix(what + ' and ')})") def compile_wait(checks: list[dict]) -> bool: @@ -373,11 +381,11 @@ def compile_refusal(pr: dict, swift: list[str], checks: list[dict]) -> str: head = pr["head"][:12] what = changes_what(pr, swift) if not checks: - return f"{what} and {head} has no checks yet; merge once its {COMPILE} passes." + return f"{what} and {head} has no checks yet, so no {COMPILE} either." if any(c.get("status") in RUNNING for c in named): - return f"{what} and its {COMPILE} is still running on {head}; merge once it passes." + return f"{what} and its {COMPILE} is still running on {head}." if not named and any(c.get("name") in ROUTING and c.get("status") in RUNNING for c in checks): - return f"{what} and cmux-next has not created its {COMPILE} on {head} yet; merge once it passes." + return f"{what} and cmux-next has not created its {COMPILE} on {head} yet." ran = [c for c in named if c.get("conclusion") not in NOT_RUN] if ran and not any(c.get("conclusion") == "success" for c in ran): return (f"{what} and its {COMPILE} finished {ran[0].get('conclusion')} on {head}. " @@ -516,40 +524,51 @@ def on_base(repo: str, sha: str, cwd: str | None) -> bool: def refusal(command: str, cwd: str | None, reader=read_pr) -> str: + """The first red among the command's merges; else the first Hold; else "".""" + held = "" for repo, selector, where in merge_targets(command, cwd): if repo is None and where is REMOTE: - return ("merge-compile-guard: this merges a pull request in a checkout the guard can't read (over " - "ssh or after `cd $VAR`); run `gh pr merge N -R OWNER/REPO` so its compile is checked.") + held = held or Hold("merge-compile-guard: this merges a pull request in a checkout the guard can't read " + "(over ssh or after `cd $VAR`), so its compile was not checked; `gh pr merge N -R " + "OWNER/REPO` lets the guard check it.") + continue repo = repo or checkout_repo(where) if repo is None: - return ("merge-compile-guard: can't tell which repository this merges in (its checkout's origin " - "could not be read); run `gh pr merge N -R OWNER/REPO` so its compile is checked.") + held = held or Hold("merge-compile-guard: can't tell which repository this merges in (its checkout's " + "origin could not be read), so its compile was not checked; `gh pr merge N -R " + "OWNER/REPO` lets the guard check it.") + continue if repo != GUARDED_REPO: continue where = None if where is REMOTE else where if selector == UNKNOWN: - return ("merge-compile-guard: this merges a manaflow-ai/cmux pull request the guard can't name (a shell " - "variable or a GraphQL mutation); run `gh pr merge N` with its number so its compile is checked.") + held = held or Hold("merge-compile-guard: this merges a manaflow-ai/cmux pull request the guard can't " + "name (a shell variable or a GraphQL mutation), so its compile was not checked; " + "`gh pr merge N` with its number lets the guard check it.") + continue if selector is None or not selector.isdigit(): # gh refuses `pr view -R` without a selector: the checkout's branch is looked up in the checkout. args = [selector, "-R", repo] if selector else [] found = gh_json(["pr", "view", *args, "--json", "number"], where) number = (found or {}).get("number") if not number: - return (f"merge-compile-guard: could not tell which pull request `gh pr merge {selector or ''}` merges; " - f"name it by number (gh pr merge N).") + held = held or Hold(f"merge-compile-guard: could not tell which pull request `gh pr merge " + f"{selector or ''}` merges, so its compile was not checked.") + continue else: number = int(selector) try: why = reader(repo, number, where) except Unreadable: cause = FAILED[0] if FAILED else "GitHub gave no readable answer" - return (f"merge-compile-guard: could not read #{number} from GitHub ({cause}), so it can't tell whether " - f"its Swift compile passed. Retry the merge; if GitHub stays slow, check `glaeda-gh budget` and the " - f"host's load.") - if why: + held = held or Hold(f"merge-compile-guard: could not read #{number} from GitHub ({cause}), so it can't " + f"tell whether its Swift compile passed.") + continue + if isinstance(why, Hold): + held = held or Hold(f"merge-compile-guard: {why}") + elif why: return f"merge-compile-guard: {why}" - return "" + return held def claude_hook(stdin) -> int: @@ -559,7 +578,12 @@ def claude_hook(stdin) -> int: return 0 command = ((payload.get("tool_input") or {}).get("command") or "") why = refusal(command, payload.get("cwd")) - if why: + if isinstance(why, Hold): + # No permissionDecision: the command goes through the normal permission flow. + text = f"{why} Only a compile that finished red refuses, so merging anyway." + json.dump({"systemMessage": text, "hookSpecificOutput": {"hookEventName": "PreToolUse", + "additionalContext": text}}, sys.stdout) + elif why: json.dump({"hookSpecificOutput": {"hookEventName": "PreToolUse", "permissionDecision": "deny", "permissionDecisionReason": why}}, sys.stdout) return 0 diff --git a/tests/test_merge_compile_guard.py b/tests/test_merge_compile_guard.py index 77ae6eb382cb..0e11f2fe7735 100644 --- a/tests/test_merge_compile_guard.py +++ b/tests/test_merge_compile_guard.py @@ -182,7 +182,7 @@ def test_the_fleet_compile_check_refusal_reads_whole_with_several_or_oddly_named ([base + "Find and Replace.swift"], f"({base}Find and Replace.swift)")): why = guard.verdict(pr(), swift, []) self.assertTrue(why.startswith(f"#7 changes Swift {what} and {HEAD[:12]} has no compile result yet."), why) - self.assertTrue(why.endswith(f"({HEAD[:12]} has no checks yet.)"), why) + self.assertTrue(why.endswith(f"({HEAD[:12]} has no checks yet, so no {COMPILE} either.)"), why) self.assertEqual(why.count("cmux-ci compile-check"), 1, why) def test_a_refusal_names_the_cause_of_its_own_read_not_an_earlier_one(self): From 3b19f092067398b5d4a28e5b5b8ccb876e7497ee Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 22 Oct 2026 01:31:35 -0700 Subject: [PATCH 3/8] merge-compile-guard tests: a red fleet compile, a crashing guard and a hidden update-branch warning (red) From the Codex review of #24265: a failed build-fleet compile next to a queued Release compile read as a wait; a guard exiting 2 blocks the tool call; a merge warning short-circuited the update-branch warning. Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a1602e-bc09-7990-8c75-5c412a66b114&host=Air-Blue --- tests/test_guard_hooks_from_base.py | 49 +++++++++++++++++++++++++++++ tests/test_merge_compile_guard.py | 17 ++++++++++ 2 files changed, 66 insertions(+) diff --git a/tests/test_guard_hooks_from_base.py b/tests/test_guard_hooks_from_base.py index 7860064e5e26..7c3ceffa617c 100644 --- a/tests/test_guard_hooks_from_base.py +++ b/tests/test_guard_hooks_from_base.py @@ -25,6 +25,12 @@ json.dump({"hookSpecificOutput": {"hookEventName": "PreToolUse", "permissionDecision": "deny", "permissionDecisionReason": "merge guard %s"}}, sys.stdout) """ +# A stand-in guard that lets the merge through with a warning, as the real guard does for a pending compile. +HOLD = """import json, sys +text = "merge guard %s holds nothing; merging anyway" +json.dump({"systemMessage": text, "hookSpecificOutput": {"hookEventName": "PreToolUse", "additionalContext": text}}, + sys.stdout) +""" WARNING = """import json, sys print(json.dumps({"hookSpecificOutput": {"hookEventName": "PreToolUse", "additionalContext": "base merge warning %s"}})) """ @@ -154,6 +160,49 @@ def test_a_base_without_the_guards_lets_a_merge_through_with_a_warning(self): self.assertIn("cannot read the guards", out["systemMessage"]) self.assertEqual(out["systemMessage"], out["hookSpecificOutput"]["additionalContext"]) + def land_on_base(self, path: str, text: str) -> None: + (self.seed / path).write_text(text) + git(self.seed, "commit", "-qam", f"the base changes {path}") + git(self.seed, "push", "-q", "origin", "feat-cmux-next") + + def test_a_guard_that_crashes_never_blocks_a_merge(self): + """Codex review of #24265: a hook that exits 2 (a bash syntax error does) blocks the tool call in Claude + Code, though no compile finished red. Every failure of a guard is a warning.""" + for broken in ("if then fi\n", "exit 2\n", "exit 3\n"): + with self.subTest(hook=broken): + self.land_on_base("scripts/ci/merge-compile-guard-hook.sh", broken) + code, out, err = self.hook("gh pr merge 5 --squash") + self.assertNotEqual(code, 2, err) + if out is not None: + self.assertNotIn("permissionDecision", out["hookSpecificOutput"]) + self.assertIn("merging anyway", out["systemMessage"]) + self.land_on_base("scripts/ci/base-merge-warning-hook.sh", "exit 2\n") + code, out, err = self.hook("gh api -X PUT repos/manaflow-ai/cmux/pulls/5/update-branch") + self.assertNotEqual(code, 2, err) + + def test_the_settings_hook_never_exits_2_when_the_launcher_itself_breaks(self): + self.land_on_base("scripts/ci/guards-from-base.sh", "exit 2\n") + code, out, err = self.hook("gh pr merge 5 --squash") + self.assertNotEqual(code, 2, err) + + def test_a_merge_warning_does_not_hide_the_update_branch_warning(self): + """Codex review of #24265: `gh pr merge ...; gh api .../update-branch` used to be refused whole; now the merge + may go ahead with a warning, and the update-branch warning must still be said.""" + self.land_on_base("scripts/ci/merge_compile_guard.py", HOLD % "base v1") + code, out, err = self.hook("gh pr merge 5 --squash; gh api -X PUT repos/manaflow-ai/cmux/pulls/5/update-branch") + self.assertEqual(code, 0, err) + context = out["hookSpecificOutput"]["additionalContext"] + self.assertIn("merge guard base v1 holds nothing", context) + self.assertIn("base merge warning base v1", context) + self.assertNotIn("permissionDecision", out["hookSpecificOutput"]) + self.assertIn("merge guard base v1 holds nothing", out["systemMessage"]) + + def test_a_refusal_still_wins_over_a_warning(self): + code, out, err = self.hook("gh pr merge 5 --squash; gh api -X PUT repos/manaflow-ai/cmux/pulls/5/update-branch") + self.assertEqual(code, 0, err) + self.assertEqual(out["hookSpecificOutput"]["permissionDecision"], "deny") + self.assertEqual(out["hookSpecificOutput"]["permissionDecisionReason"], "merge guard base v1") + def old_base(self) -> str: """A commit on the base from before the launcher existed.""" old = self.tmp / "old" diff --git a/tests/test_merge_compile_guard.py b/tests/test_merge_compile_guard.py index 0e11f2fe7735..250d946a20ad 100644 --- a/tests/test_merge_compile_guard.py +++ b/tests/test_merge_compile_guard.py @@ -62,6 +62,23 @@ def test_only_a_compile_that_finished_red_refuses(self): self.assertTrue(why) self.assertNotIsInstance(why, guard.Hold) + def test_a_red_fleet_compile_refuses_even_with_a_release_compile_queued(self): + """Codex review of #24265: the red check read only Release attempts, so a failed build-fleet compile next to + a queued Release compile read as a wait and became a warning, and a fleet pass on the head could silence it.""" + fleet = guard.FLEET_COMPILE + for runs in ([check(fleet, conclusion="failure"), check(COMPILE, "queued")], + [check(fleet, conclusion="failure"), check(COMPILE, "in_progress")], + [check(fleet, conclusion="timed_out"), check("cmux-next path routing", "in_progress")]): + with self.subTest(runs=runs): + self.assertFalse(guard.compile_wait(runs)) + for fleet_passed in (False, True): + why = guard.verdict(pr(), SWIFT, runs, fleet_passed=fleet_passed) + self.assertIn(f"{fleet} finished", why) + self.assertNotIsInstance(why, guard.Hold) + # A passing rerun of either compile clears it. + self.assertEqual(guard.verdict(pr(), SWIFT, [check(fleet, conclusion="failure"), check(fleet)]), "") + self.assertEqual(guard.verdict(pr(), SWIFT, [check(fleet, conclusion="failure"), check(COMPILE)]), "") + def test_a_compile_not_created_yet_waits_while_cmux_next_routes_and_places(self): for job in guard.ROUTING: with self.subTest(job=job): From f86daab2a7ddda1960ea944cccf3ed6073ad9c8d Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 22 Oct 2026 01:33:20 -0700 Subject: [PATCH 4/8] merge-compile-guard: a red fleet compile refuses beside a queued Release compile; a guard's failure and a merge warning never block or hide From the Codex review of #24265: - fleet_red(): a build-fleet compile that finished red with no passing attempt (of it or of the Release compile) refuses before any pending state, and compile_wait() no longer reads it as a wait, so neither a queued Release compile nor a fleet pass turns it into a warning. - guards-from-base.sh runs both guards. A refusal is the answer at once; otherwise every warning is joined into one answer, so a merge's warning never hides the update-branch warning of the same command. A guard that exits non-zero is a warning, never the exit 2 that blocks a tool call. - The settings hook exits 1 (a non-blocking error) when the launcher itself fails. Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a1602e-bc09-7990-8c75-5c412a66b114&host=Air-Blue --- .claude/settings.json | 2 +- scripts/ci/guards-from-base.sh | 45 ++++++++++++++++++++++++++----- scripts/ci/merge_compile_guard.py | 19 +++++++++---- 3 files changed, 54 insertions(+), 12 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index 61fda6a87ad4..f2969dd80af9 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "command": "bash -c 'i=$(cat); case $i in *\"pr merge\"*|*\"/merge\"*) m=1 ;; *update-branch*) m=0 ;; *) exit 0 ;; esac; d=${CLAUDE_PROJECT_DIR:-.}; if [ \"$m\" = 1 ] && command -v timeout >/dev/null 2>&1; then timeout 10 git -C \"$d\" fetch --quiet --no-tags origin feat-cmux-next 2>/dev/null; fi; f=$(mktemp) || exit 1; trap \"rm -f -- \\\"$f\\\"\" EXIT; if ! git -C \"$d\" show origin/feat-cmux-next:scripts/ci/guards-from-base.sh > \"$f\" 2>/dev/null; then why=\"cmux guards: cannot read scripts/ci/guards-from-base.sh at origin/feat-cmux-next, so the merge guards did not run; fetch origin feat-cmux-next and retry\"; if [ \"$m\" = 1 ]; then printf \"{\\\"systemMessage\\\": \\\"%s; merging anyway\\\", \\\"hookSpecificOutput\\\": {\\\"hookEventName\\\": \\\"PreToolUse\\\", \\\"additionalContext\\\": \\\"%s; merging anyway\\\"}}\\n\" \"$why\" \"$why\"; exit 0; fi; echo \"$why\" >&2; exit 1; fi; printf %s \"$i\" | bash \"$f\" \"$d\"'", + "command": "bash -c 'i=$(cat); case $i in *\"pr merge\"*|*\"/merge\"*) m=1 ;; *update-branch*) m=0 ;; *) exit 0 ;; esac; d=${CLAUDE_PROJECT_DIR:-.}; if [ \"$m\" = 1 ] && command -v timeout >/dev/null 2>&1; then timeout 10 git -C \"$d\" fetch --quiet --no-tags origin feat-cmux-next 2>/dev/null; fi; f=$(mktemp) || exit 1; trap \"rm -f -- \\\"$f\\\"\" EXIT; if ! git -C \"$d\" show origin/feat-cmux-next:scripts/ci/guards-from-base.sh > \"$f\" 2>/dev/null; then why=\"cmux guards: cannot read scripts/ci/guards-from-base.sh at origin/feat-cmux-next, so the merge guards did not run; fetch origin feat-cmux-next and retry\"; if [ \"$m\" = 1 ]; then printf \"{\\\"systemMessage\\\": \\\"%s; merging anyway\\\", \\\"hookSpecificOutput\\\": {\\\"hookEventName\\\": \\\"PreToolUse\\\", \\\"additionalContext\\\": \\\"%s; merging anyway\\\"}}\\n\" \"$why\" \"$why\"; exit 0; fi; echo \"$why\" >&2; exit 1; fi; printf %s \"$i\" | bash \"$f\" \"$d\" || exit 1'", "timeout": 120 } ] diff --git a/scripts/ci/guards-from-base.sh b/scripts/ci/guards-from-base.sh index 83ce8df4d7b4..744d8ac6eb97 100755 --- a/scripts/ci/guards-from-base.sh +++ b/scripts/ci/guards-from-base.sh @@ -8,9 +8,9 @@ # - merge-compile-guard-hook.sh (merge_compile_guard.py): refuses a hand merge of a Swift PR # before its Release compile passed; # - base-merge-warning-hook.sh (base_merge_warning.py): warns before an update-branch cancels CI. -# The first that answers (a refusal or a note) is the answer; the hook JSON goes to stdout. A merge -# whose guards can't be read from the base goes ahead with a warning: only a compile that finished red -# refuses (merge_compile_guard.py). +# A refusal from either is the answer; otherwise their warnings are joined into one answer on stdout. A +# merge whose guards can't be read from the base, or whose guard fails, goes ahead with a warning: only a +# compile that finished red refuses (merge_compile_guard.py). # # Usage: printf '%s' "$payload" | bash guards-from-base.sh CHECKOUT set -uo pipefail @@ -37,12 +37,45 @@ trap 'rm -rf "$work"' EXIT git -C "$dir" archive "$base" scripts/ci/merge-compile-guard-hook.sh scripts/ci/merge_compile_guard.py \ scripts/ci/base-merge-warning-hook.sh scripts/ci/base_merge_warning.py | tar -x -C "$work" \ || cannot "cmux guards: cannot read the guards at origin/feat-cmux-next ${base:0:12}" +# Each guard runs: a refusal from either is the answer at once. Otherwise their warnings are joined, so a +# merge's warning never hides the update-branch warning of the same command. A guard that crashes or exits +# non-zero (exit 2 would block the tool call in Claude Code) is only a warning: no guard blocks on its own failure. +said=() for hook in merge-compile-guard-hook.sh base-merge-warning-hook.sh; do - out="$(printf '%s' "$input" | bash "$work/scripts/ci/$hook")" + out="$(printf '%s' "$input" | bash "$work/scripts/ci/$hook" 2>/dev/null)" status=$? - if [[ -n "$out" || $status -ne 0 ]]; then + if ((status != 0)); then + out="cmux guards: $hook exited $status, so it judged nothing" + fi + [[ -n "$out" ]] || continue + if [[ $out == *'"permissionDecision": "deny"'* || $out == *'"permissionDecision":"deny"'* ]]; then printf '%s\n' "$out" - exit "$status" + exit 0 fi + said+=("$out") done +((${#said[@]})) || exit 0 +# One answer from every warning: a guard's JSON gives its systemMessage and additionalContext, any other text +# is a note. The person sees the systemMessage, the agent the additionalContext. +python3 -I -c 'import json, sys +shown, context = [], [] +for out in sys.argv[2:]: + try: + answer = json.loads(out) + spec = answer.get("hookSpecificOutput") or {} + except (ValueError, AttributeError): + answer, spec = {}, {} + if not spec and not answer.get("systemMessage"): + note = out + ("; merging anyway" if sys.argv[1] == "1" else "") + shown.append(note) + context.append(note) + continue + if answer.get("systemMessage"): + shown.append(answer["systemMessage"]) + if spec.get("additionalContext"): + context.append(spec["additionalContext"]) +result = {"hookSpecificOutput": {"hookEventName": "PreToolUse", "additionalContext": "\n".join(context)}} +if shown: + result["systemMessage"] = "\n".join(shown) +json.dump(result, sys.stdout)' "$merge" "${said[@]}" || true exit 0 diff --git a/scripts/ci/merge_compile_guard.py b/scripts/ci/merge_compile_guard.py index fee9d8f0dadf..127b9aa2039a 100755 --- a/scripts/ci/merge_compile_guard.py +++ b/scripts/ci/merge_compile_guard.py @@ -358,12 +358,22 @@ def compile_wait(checks: list[dict]) -> bool: finished = [c.get("conclusion") for c in named if c.get("status") == "completed"] if any(c not in NO_VERDICT and c != "success" for c in finished) and "success" not in finished: return False # an attempt finished red: a rerun queued after it is no wait (every attempt is read) + if fleet_red(checks): + return False # the build-fleet compile finished red: the Release compile queued beside it is no wait return (not checks or any(c.get("status") in RUNNING for c in named) or (not named and any(c.get("name") in ROUTING and c.get("status") in RUNNING for c in checks)) or (len(finished) == len(named) and set(finished) - NOT_RUN == {"cancelled"}) or fleet_stopped(checks)) +def fleet_red(checks: list[dict]) -> str: + """The red conclusion of the PR's build-fleet compile when no attempt of it, and no Release compile, passed.""" + fleet = [c.get("conclusion") for c in checks if c.get("name") == FLEET_COMPILE and c.get("status") == "completed"] + red = [c for c in fleet if c not in NO_VERDICT and c != "success"] + passed = "success" in fleet or any(c.get("name") == COMPILE and c.get("conclusion") == "success" for c in checks) + return red[0] if red and not passed else "" + + def fleet_stopped(checks: list[dict]) -> bool: """The PR's build-fleet compile was cancelled with no pass and no Release compile ran: no verdict at all (a run cancelled mid-compile also skips the Release compile).""" @@ -382,6 +392,10 @@ def compile_refusal(pr: dict, swift: list[str], checks: list[dict]) -> str: what = changes_what(pr, swift) if not checks: return f"{what} and {head} has no checks yet, so no {COMPILE} either." + red = fleet_red(checks) + if red: + return (f"{what} and its {FLEET_COMPILE} finished {red} on {head}. " + f"Fix it, or merge the base in when the base has the fix, and merge once it passes.") if any(c.get("status") in RUNNING for c in named): return f"{what} and its {COMPILE} is still running on {head}." if not named and any(c.get("name") in ROUTING and c.get("status") in RUNNING for c in checks): @@ -390,11 +404,6 @@ def compile_refusal(pr: dict, swift: list[str], checks: list[dict]) -> str: if ran and not any(c.get("conclusion") == "success" for c in ran): return (f"{what} and its {COMPILE} finished {ran[0].get('conclusion')} on {head}. " f"Fix it, or merge the base in when the base has the fix, and merge once it passes.") - fleet = [c.get("conclusion") for c in checks if c.get("name") == FLEET_COMPILE and c.get("status") == "completed"] - red = [c for c in fleet if c not in NO_VERDICT and c != "success"] - if red and "success" not in fleet and not any(c.get("conclusion") == "success" for c in ran): - return (f"{what} and its {FLEET_COMPILE} finished {red[0]} on {head}. " - f"Fix it, or merge the base in when the base has the fix, and merge once it passes.") if fleet_stopped(checks): return f"{what} and its {FLEET_COMPILE} was cancelled on {head}, so nothing compiled it; rerun it." return "" From b13d026a350642f2423c5d0b00e772bcfd228577 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 22 Oct 2026 01:40:12 -0700 Subject: [PATCH 5/8] guards-from-base tests: a refusal in any JSON spacing still refuses (red) Codex review of #24265, round 2. Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a1602e-bc09-7990-8c75-5c412a66b114&host=Air-Blue --- tests/test_guard_hooks_from_base.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_guard_hooks_from_base.py b/tests/test_guard_hooks_from_base.py index 7c3ceffa617c..644173b0e551 100644 --- a/tests/test_guard_hooks_from_base.py +++ b/tests/test_guard_hooks_from_base.py @@ -203,6 +203,19 @@ def test_a_refusal_still_wins_over_a_warning(self): self.assertEqual(out["hookSpecificOutput"]["permissionDecision"], "deny") self.assertEqual(out["hookSpecificOutput"]["permissionDecisionReason"], "merge guard base v1") + def test_a_refusal_in_any_json_spacing_still_refuses(self): + """Codex review of #24265 (round 2): the loop matched `"permissionDecision": "deny"` as text, so a valid + refusal written with other spacing was merged into a warning and the red compile's refusal was lost.""" + spaced = GUARD.replace("sys.stdout)", "sys.stdout, separators=(',', ': '), indent=2)") + self.land_on_base("scripts/ci/merge_compile_guard.py", spaced % "base v1") + for command in ("gh pr merge 5 --squash", + "gh pr merge 5 --squash; gh api -X PUT repos/manaflow-ai/cmux/pulls/5/update-branch"): + with self.subTest(command=command): + code, out, err = self.hook(command) + self.assertEqual(code, 0, err) + self.assertEqual(out["hookSpecificOutput"]["permissionDecision"], "deny") + self.assertEqual(out["hookSpecificOutput"]["permissionDecisionReason"], "merge guard base v1") + def old_base(self) -> str: """A commit on the base from before the launcher existed.""" old = self.tmp / "old" From 02026aa35b3d78a99d8eee8c6a96fdcf670a131e Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 22 Oct 2026 01:40:13 -0700 Subject: [PATCH 6/8] guards-from-base: read a guard's refusal as JSON, not as text The loop matched "permissionDecision": "deny" literally, so a valid refusal written with other spacing was merged into a warning and lost (Codex review of #24265, round 2). The merger now parses each answer and passes a refusal through as it is before joining warnings. Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a1602e-bc09-7990-8c75-5c412a66b114&host=Air-Blue --- scripts/ci/guards-from-base.sh | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/scripts/ci/guards-from-base.sh b/scripts/ci/guards-from-base.sh index 744d8ac6eb97..405b56ace2ac 100755 --- a/scripts/ci/guards-from-base.sh +++ b/scripts/ci/guards-from-base.sh @@ -37,8 +37,8 @@ trap 'rm -rf "$work"' EXIT git -C "$dir" archive "$base" scripts/ci/merge-compile-guard-hook.sh scripts/ci/merge_compile_guard.py \ scripts/ci/base-merge-warning-hook.sh scripts/ci/base_merge_warning.py | tar -x -C "$work" \ || cannot "cmux guards: cannot read the guards at origin/feat-cmux-next ${base:0:12}" -# Each guard runs: a refusal from either is the answer at once. Otherwise their warnings are joined, so a -# merge's warning never hides the update-branch warning of the same command. A guard that crashes or exits +# Each guard runs: a refusal from either is the answer. Otherwise their warnings are joined, so a merge's +# warning never hides the update-branch warning of the same command. A guard that crashes or exits # non-zero (exit 2 would block the tool call in Claude Code) is only a warning: no guard blocks on its own failure. said=() for hook in merge-compile-guard-hook.sh base-merge-warning-hook.sh; do @@ -47,17 +47,21 @@ for hook in merge-compile-guard-hook.sh base-merge-warning-hook.sh; do if ((status != 0)); then out="cmux guards: $hook exited $status, so it judged nothing" fi - [[ -n "$out" ]] || continue - if [[ $out == *'"permissionDecision": "deny"'* || $out == *'"permissionDecision":"deny"'* ]]; then - printf '%s\n' "$out" - exit 0 - fi - said+=("$out") + [[ -n "$out" ]] && said+=("$out") done ((${#said[@]})) || exit 0 -# One answer from every warning: a guard's JSON gives its systemMessage and additionalContext, any other text -# is a note. The person sees the systemMessage, the agent the additionalContext. +# One answer: a guard's refusal (read as JSON, whatever its spacing) as it is; else every warning joined. A +# guard's JSON gives its systemMessage and additionalContext, any other text is a note. The person sees the +# systemMessage, the agent the additionalContext. python3 -I -c 'import json, sys +for out in sys.argv[2:]: + try: + spec = json.loads(out).get("hookSpecificOutput") or {} + except (ValueError, AttributeError): + continue + if isinstance(spec, dict) and spec.get("permissionDecision") == "deny": + sys.stdout.write(out + "\n") + sys.exit(0) shown, context = [], [] for out in sys.argv[2:]: try: From 1f1e479730a9298c0552101e9d66760dd0cdea5f Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 22 Oct 2026 01:47:03 -0700 Subject: [PATCH 7/8] guards-from-base tests: a refusal never waits for the next guard (red) Codex review of #24265, round 3. Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a1602e-bc09-7990-8c75-5c412a66b114&host=Air-Blue --- tests/test_guard_hooks_from_base.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_guard_hooks_from_base.py b/tests/test_guard_hooks_from_base.py index 644173b0e551..afe629167df6 100644 --- a/tests/test_guard_hooks_from_base.py +++ b/tests/test_guard_hooks_from_base.py @@ -13,6 +13,7 @@ import shutil import subprocess import tempfile +import time import unittest from pathlib import Path from unittest import mock @@ -216,6 +217,16 @@ def test_a_refusal_in_any_json_spacing_still_refuses(self): self.assertEqual(out["hookSpecificOutput"]["permissionDecision"], "deny") self.assertEqual(out["hookSpecificOutput"]["permissionDecisionReason"], "merge guard base v1") + def test_a_refusal_never_waits_for_the_next_guard(self): + """Codex review of #24265 (round 3): both guards ran before the refusal was read, so a slow update-branch + warning could run the hook past its timeout with the refusal unprinted, and the merge went through.""" + self.land_on_base("scripts/ci/base-merge-warning-hook.sh", "sleep 30\n") + started = time.monotonic() + code, out, err = self.hook("gh pr merge 5 --squash; gh api -X PUT repos/manaflow-ai/cmux/pulls/5/update-branch") + self.assertLess(time.monotonic() - started, 20) + self.assertEqual(code, 0, err) + self.assertEqual(out["hookSpecificOutput"]["permissionDecision"], "deny") + def old_base(self) -> str: """A commit on the base from before the launcher existed.""" old = self.tmp / "old" From beeca0b0986d167452bfed787ea03f32ba7dca66 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 22 Oct 2026 01:47:56 -0700 Subject: [PATCH 8/8] guards-from-base: a guard's refusal is the answer the moment it is given Both guards ran before any refusal was read, so a slow update-branch warning could run the hook past its 120 s timeout with the refusal unprinted, and the merge went through (Codex review of #24265, round 3). Each answer is read as JSON right after its guard; a refusal is printed and the loop ends. Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a1602e-bc09-7990-8c75-5c412a66b114&host=Air-Blue --- scripts/ci/guards-from-base.sh | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/scripts/ci/guards-from-base.sh b/scripts/ci/guards-from-base.sh index 405b56ace2ac..dc53a4ce3fab 100755 --- a/scripts/ci/guards-from-base.sh +++ b/scripts/ci/guards-from-base.sh @@ -47,21 +47,24 @@ for hook in merge-compile-guard-hook.sh base-merge-warning-hook.sh; do if ((status != 0)); then out="cmux guards: $hook exited $status, so it judged nothing" fi - [[ -n "$out" ]] && said+=("$out") + [[ -n "$out" ]] || continue + # A refusal is the answer the moment its guard gives it, read as JSON whatever its spacing: the next + # guard could run the hook past its timeout with the refusal unprinted. + if python3 -I -c 'import json, sys +try: + spec = json.loads(sys.argv[1]).get("hookSpecificOutput") or {} +except (ValueError, AttributeError): + sys.exit(1) +sys.exit(0 if isinstance(spec, dict) and spec.get("permissionDecision") == "deny" else 1)' "$out"; then + printf '%s\n' "$out" + exit 0 + fi + said+=("$out") done ((${#said[@]})) || exit 0 -# One answer: a guard's refusal (read as JSON, whatever its spacing) as it is; else every warning joined. A -# guard's JSON gives its systemMessage and additionalContext, any other text is a note. The person sees the -# systemMessage, the agent the additionalContext. +# No refusal: one answer from every warning. A guard's JSON gives its systemMessage and additionalContext, +# any other text is a note. The person sees the systemMessage, the agent the additionalContext. python3 -I -c 'import json, sys -for out in sys.argv[2:]: - try: - spec = json.loads(out).get("hookSpecificOutput") or {} - except (ValueError, AttributeError): - continue - if isinstance(spec, dict) and spec.get("permissionDecision") == "deny": - sys.stdout.write(out + "\n") - sys.exit(0) shown, context = [], [] for out in sys.argv[2:]: try: