From e4a11f8120fc11c0bcf331e6553f390aa5e03559 Mon Sep 17 00:00:00 2001 From: midagedev Date: Thu, 15 Oct 2026 05:22:51 +0900 Subject: [PATCH 01/10] tools: refuse a claude that does not answer --version, and skip pre-checks when asked A binary on PATH that fails claude --version is a broken install: the run ends 75 and names it, before a review. --no-pre-checks skips the text checks and leaves that probe in place. The self-test does not call the installed claude. --- tools/review/local.py | 78 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 73 insertions(+), 5 deletions(-) diff --git a/tools/review/local.py b/tools/review/local.py index c3d6866ec..d00b23cd1 100644 --- a/tools/review/local.py +++ b/tools/review/local.py @@ -2,7 +2,7 @@ """Shingler's reviewer on this machine, before a push: the PR review's own instructions, catalog and ledger, run through the base tree's tools/review/run-claude.sh under the caller's own Claude login (`just pre-review`). - tools/review/local.py [--base REF] [--full] [--minutes N] [--body FILE] + tools/review/local.py [--base REF] [--full] [--minutes N] [--body FILE] [--no-pre-checks] tools/review/local.py --self-test What it reviews is the committed HEAD, what a push sends; a dirty tree is named and left out. The trees come from @@ -21,14 +21,16 @@ Before the reviewer, the text-only checks a push needs and nobody runs by hand run on PR_TREE when the branch touches their inputs (PRE_CHECKS: tools/check-levers.sh, about 10 s, for tools/, the justfile, .cargo/ or crates/); a red one -ends the run 1 by name, before the reviewer's cost. The failure it blocks: a lever or marker name that only a -hand-run check-levers caught. +ends the run 1 by name, before the reviewer's cost. --no-pre-checks skips them. The failure it blocks: a lever or +marker name that only a hand-run check-levers caught. + +`claude --version` runs first on a real review. A binary that is on PATH and does not answer is a broken install. Prints the verdict, the must-fix titles and where the output is (~/.cache/bloomery/pre-review/-/). A CLEAN review also prints one line for the pull request body, with the full HEAD sha. A base that lacks commits origin/main has is behind it and is refused by name before the reviewer runs. A missing origin/main is not that case. A failed ancestry walk stops 70. -Exit: 0 CLEAN; 1 MUST_FIX or a red pre-check; 75 no claude CLI, or a role past --minutes (default 10); 64 usage, +Exit: 0 CLEAN; 1 MUST_FIX or a red pre-check; 75 no claude CLI, a broken install, or a role past --minutes (default 10); 64 usage, or a base behind origin/main; 65 output post.py refused or a review with no verdict; 70 a context step or a git command that failed (named). The failure it blocks: a pull request whose first Shingler review is a MUST_FIX, each fold another 10-27 minute round. @@ -335,6 +337,17 @@ def run_role(role, d, minutes, say): return rc +def claude_ok(): + """`claude --version` must answer. On PATH and silent or red is a broken install, not a missing binary.""" + try: + p = subprocess.run(["claude", "--version"], capture_output=True, text=True, + stdin=subprocess.DEVNULL, timeout=15) + except (OSError, subprocess.TimeoutExpired) as e: + raise Stop(75, f"pre-review: claude --version failed ({type(e).__name__}): 설치 깨짐; nothing was reviewed") from e + if p.returncode != 0: + raise Stop(75, f"pre-review: claude --version failed (rc {p.returncode}): 설치 깨짐; nothing was reviewed") + + def pre_checks(pr_tree, paths, say): """PRE_CHECKS whose prefixes a changed path starts with, on PR_TREE; the first red one stops the run 1, named.""" for script, prefixes in PRE_CHECKS: @@ -358,6 +371,8 @@ def must_fix_titles(body): def review(args, stubbed=False, say=lambda m: print(m, file=sys.stderr)): if not stubbed and not shutil.which("claude"): raise Stop(75, "pre-review: no claude CLI on PATH; nothing was reviewed") + if not stubbed: + claude_ok() try: fetched = subprocess.run(["git", "fetch", "-q", "origin"], capture_output=True, timeout=NET_S).returncode == 0 except subprocess.TimeoutExpired: @@ -385,7 +400,10 @@ def review(args, stubbed=False, say=lambda m: print(m, file=sys.stderr)): archive(head, os.path.join(d, "pr"), cwd=os.getcwd()) except RecipeError as e: raise Stop(70, f"pre-review: {e}") from e - pre_checks(os.path.join(d, "pr"), git("diff", "--name-only", "-z", f"{base_sha}...{head}").split("\0"), say) + if getattr(args, "no_pre_checks", False): + say("pre-review: pre-checks skipped (--no-pre-checks)") + else: + pre_checks(os.path.join(d, "pr"), git("diff", "--name-only", "-z", f"{base_sha}...{head}").split("\0"), say) for x in ("ctx", "out", "trace"): os.makedirs(os.path.join(d, x)) try: @@ -545,6 +563,34 @@ def broken(root, pins): open(lev, "w").write("exit 0\n") pre_checks(t, ["crates/gpu/src/lib.rs"], said.append) expect(len(said) == 1 and said[0].startswith("pre-review: tools/check-levers.sh ok"), f"a green one is printed: {said}") + # claude --version: a binary that exits red is a broken install (75), and the message names it + with tempfile.TemporaryDirectory() as t: + fake = os.path.join(t, "claude") + open(fake, "w").write("#!/bin/sh\necho broken >&2\nexit 1\n") + os.chmod(fake, 0o755) + saved_path = os.environ.get("PATH") + os.environ["PATH"] = t + os.pathsep + (saved_path or "") + try: + try: + claude_ok() + got = None + except Stop as e: + got = e + expect(got is not None and got.code == 75 and "설치 깨짐" in got.msg, + f"a claude that fails --version ends 75 naming a broken install: {got and (got.code, got.msg)}") + open(fake, "w").write("#!/bin/sh\necho 2.0.0\nexit 0\n") + os.chmod(fake, 0o755) + try: + claude_ok() + ok_ver = True + except Stop as e: + ok_ver = False + expect(ok_ver, "a claude that answers --version is a healthy install") + finally: + if saved_path is None: + os.environ.pop("PATH", None) + else: + os.environ["PATH"] = saved_path # gh's paginated answer is several JSON documents: every page is read; a credential lookup that times out is the # named empty-ledger fallback, not a traceback with tempfile.TemporaryDirectory() as t: @@ -703,6 +749,7 @@ def go(text, sleep=0, minutes=10): os.unlink(pidf) wrapper = (f"import shutil, sys\nsys.path.insert(0, {HERE!r})\nimport local as L\nreal = shutil.which\n" "shutil.which = lambda n, *a: '/x' if n == 'claude' else real(n, *a)\n" + "L.claude_ok = lambda: None\n" "L.gh_json = lambda *a: None\nL.device_dirs = lambda b, o: open(o, 'w').write('crates/gpu\\n')\n" "L.read_ledger = lambda b, o: (open(o, 'w').close(), 'v0')[1]\n" "sys.exit(L.main(['--base', 'basebr', '--minutes', '5']))\n") @@ -724,6 +771,26 @@ def go(text, sleep=0, minutes=10): expect(ended and not alive, f"SIGTERM to local.py ends it and the role's process group: ended {ended}, child alive {alive}") if alive: os.kill(child, signal.SIGKILL) + # --no-pre-checks: a red text check on this head does not run, and the review still ends + open(os.path.join(repo, "tools/check-levers.sh"), "w").write("echo 'check-levers: SOME_NAME is no row'; exit 3\n") + sh("git", "add", "-A") + sh("git", "commit", "-qm", "topic: a red levers check") + a = argparse.Namespace(base="basebr", full=False, minutes=10, body=None, no_pre_checks=False) + try: + rc, msg = review(a, stubbed=True, say=lambda m: None), "" + except Stop as e: + rc, msg = e.code, e.msg + expect(rc == 1 and "tools/check-levers.sh is red" in msg, + f"a red pre-check still stops the run when the flag is off: {rc} {msg}") + runner("**Verdict: CLEAN**: fine\n\n### Checked\nall") + a.no_pre_checks = True + skipped = [] + try: + rc = review(a, stubbed=True, say=skipped.append) + except Stop as e: + rc = e.code + expect(rc == 0 and any("pre-checks skipped" in m for m in skipped), + f"--no-pre-checks skips a red pre-check and the review ends 0: {rc} {skipped}") finally: os.chdir(cwd) g.update(saved) @@ -744,6 +811,7 @@ def main(argv): ap.add_argument("--full", action="store_true", help="the validator too, as the workflow runs it") ap.add_argument("--minutes", type=float, default=10, help="the bound per role (default 10)") ap.add_argument("--body", help="a file with the pull request body the reviewer reads (default: the open PR's, else none)") + ap.add_argument("--no-pre-checks", action="store_true", help="skip the text-only checks before the reviewer") try: args = ap.parse_args(argv) except SystemExit as e: From 3d4f9a8d55e23aa3740e90d13d3f8968d9588779 Mon Sep 17 00:00:00 2001 From: midagedev Date: Thu, 15 Oct 2026 06:13:14 +0900 Subject: [PATCH 02/10] tools: two pre-review slots, a device-dir cache, context time after the checks A third pre-review waits on a flock slot. device-dirs.txt is reused for a base sha. The summary's context seconds start once the pre-checks have finished. --- tools/review/local.py | 106 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 101 insertions(+), 5 deletions(-) diff --git a/tools/review/local.py b/tools/review/local.py index d00b23cd1..8fd77cea6 100644 --- a/tools/review/local.py +++ b/tools/review/local.py @@ -24,6 +24,9 @@ ends the run 1 by name, before the reviewer's cost. --no-pre-checks skips them. The failure it blocks: a lever or marker name that only a hand-run check-levers caught. +Two pre-reviews run at once; a third waits on a flock slot. device-dirs.txt is cached by the base sha, and the +summary's context time starts after the pre-checks. + `claude --version` runs first on a real review. A binary that is on PATH and does not answer is a broken install. Prints the verdict, the must-fix titles and where the output is (~/.cache/bloomery/pre-review/-/). @@ -185,7 +188,7 @@ def build_ctx(ctx, base_tree, base_sha, head, args, say): f.write("".join(f"#{i['number']} {i['title']}\n" for i in issues)) else: say("pre-review: gh could not list the bot's open issues; open-issues.txt is empty") - device_dirs(base_tree, os.path.join(ctx, "device-dirs.txt")) + device_dirs(base_tree, os.path.join(ctx, "device-dirs.txt"), base_sha) ledger = read_ledger(base_tree, os.path.join(ctx, "ledger.md")) if ledger is None: say("pre-review: the slag ledger could not be read (gh); ledger.md is empty") @@ -207,11 +210,34 @@ def build_ctx(ctx, base_tree, base_sha, head, args, say): return meta -def device_dirs(base_tree, out): +def device_dirs(base_tree, out, base_sha=None): + """post.py device, copied from the cache for this base sha when that file is already there.""" + cache = base_sha and os.path.expanduser(f"~/.cache/bloomery-review/device-{base_sha}") + if cache and os.path.isfile(cache): + shutil.copy(cache, out) + return p = subprocess.run([sys.executable, os.path.join(base_tree, "tools/review/post.py"), "device", "--root", base_tree, "--out", out], capture_output=True, text=True) if p.returncode: raise Stop(70, f"pre-review: post.py device failed: {p.stderr.strip()[:200]}") + if cache: + os.makedirs(os.path.dirname(cache), exist_ok=True) + shutil.copy(out, cache) + + +def review_slot(): + """One of two flock slots. A third pre-review blocks on the first until a holder finishes.""" + d = os.path.expanduser("~/.cache/bloomery-review") + os.makedirs(d, exist_ok=True) + fds = [open(os.path.join(d, f"slot{i}"), "a") for i in (0, 1)] + for f in fds: + try: + fcntl.flock(f, fcntl.LOCK_EX | fcntl.LOCK_NB) + return f + except BlockingIOError: + continue + fcntl.flock(fds[0], fcntl.LOCK_EX) + return fds[0] def read_ledger(base_tree, out): @@ -369,6 +395,7 @@ def must_fix_titles(body): def review(args, stubbed=False, say=lambda m: print(m, file=sys.stderr)): + slot = review_slot() # held until this review returns if not stubbed and not shutil.which("claude"): raise Stop(75, "pre-review: no claude CLI on PATH; nothing was reviewed") if not stubbed: @@ -404,11 +431,12 @@ def review(args, stubbed=False, say=lambda m: print(m, file=sys.stderr)): say("pre-review: pre-checks skipped (--no-pre-checks)") else: pre_checks(os.path.join(d, "pr"), git("diff", "--name-only", "-z", f"{base_sha}...{head}").split("\0"), say) + t_pre = time.time() for x in ("ctx", "out", "trace"): os.makedirs(os.path.join(d, x)) try: meta = build_ctx(os.path.join(d, "ctx"), os.path.join(d, "base"), base_sha, head, args, say) - t_ctx = time.time() - t0 + t_ctx = time.time() - t_pre roles = ("reviewer", "validator") if args.full else ("reviewer",) for role in roles: rc = run_role(role, d, args.minutes, say) @@ -618,6 +646,62 @@ def broken(root, pins): globals()["NET_S"] = saved_net for k, v in saved_env.items(): os.environ.pop(k, None) if v is None else os.environ.__setitem__(k, v) + # two pre-reviews at once: a third waits until one slot frees + with tempfile.TemporaryDirectory() as t: + home = os.environ.get("HOME") + os.environ["HOME"] = t + held = [] + try: + held = [review_slot(), review_slot()] + seen = [] + + def third(): + review_slot() + seen.append(1) + + th = threading.Thread(target=third) + th.start() + time.sleep(0.4) + waited = seen == [] + held.pop(0).close() + th.join(5) + expect(waited and seen == [1] and not th.is_alive(), + f"a third pre-review waits for a slot: waited {waited}, ran {seen}, alive {th.is_alive()}") + finally: + for f in held: + f.close() + if home is None: + os.environ.pop("HOME", None) + else: + os.environ["HOME"] = home + # device dirs are cached by the base sha: the second call at one sha does not run post.py again + with tempfile.TemporaryDirectory() as t: + home = os.environ.get("HOME") + os.environ["HOME"] = t + ran = [] + real_run = subprocess.run + + def wrapped(argv, *a, **k): + if len(argv) > 1 and str(argv[1]).endswith("post.py"): + ran.append(1) + open(argv[argv.index("--out") + 1], "w").write("crates/gpu/\n") + return subprocess.CompletedProcess(argv, 0, "", "") + return real_run(argv, *a, **k) + + subprocess.run = wrapped + try: + outs = [os.path.join(t, n) for n in ("a", "b", "c")] + device_dirs(t, outs[0], "sha-a") + device_dirs(t, outs[1], "sha-a") + device_dirs(t, outs[2], "sha-b") + expect(ran == [1, 1] and open(outs[1]).read() == "crates/gpu/\n", + f"device dirs run once per base sha: ran {len(ran)}, second {open(outs[1]).read()!r}") + finally: + subprocess.run = real_run + if home is None: + os.environ.pop("HOME", None) + else: + os.environ["HOME"] = home # a scratch repo end to end with a stub runner: CLEAN 0, MUST_FIX 1, a slow runner 75, no verdict 65 with tempfile.TemporaryDirectory() as t: repo = os.path.join(t, "r") @@ -655,7 +739,7 @@ def runner(text, sleep=0): saved = {k: g[k] for k in ("gh_json", "device_dirs", "read_ledger")} # no network and no cargo here: gh answers nothing, the device list and the ledger are fixed g["gh_json"] = lambda *a: None - g["device_dirs"] = lambda base_tree, out: open(out, "w").write("crates/gpu\n") + g["device_dirs"] = lambda base_tree, out, base_sha=None: open(out, "w").write("crates/gpu\n") def fake_ledger(base_tree, out): open(out, "w").close() return "v0" @@ -750,7 +834,7 @@ def go(text, sleep=0, minutes=10): wrapper = (f"import shutil, sys\nsys.path.insert(0, {HERE!r})\nimport local as L\nreal = shutil.which\n" "shutil.which = lambda n, *a: '/x' if n == 'claude' else real(n, *a)\n" "L.claude_ok = lambda: None\n" - "L.gh_json = lambda *a: None\nL.device_dirs = lambda b, o: open(o, 'w').write('crates/gpu\\n')\n" + "L.gh_json = lambda *a: None\nL.device_dirs = lambda b, o, s=None: open(o, 'w').write('crates/gpu\\n')\n" "L.read_ledger = lambda b, o: (open(o, 'w').close(), 'v0')[1]\n" "sys.exit(L.main(['--base', 'basebr', '--minutes', '5']))\n") proc = subprocess.Popen([sys.executable, "-c", wrapper], cwd=repo, stdout=subprocess.PIPE, stderr=subprocess.PIPE) @@ -791,6 +875,18 @@ def go(text, sleep=0, minutes=10): rc = e.code expect(rc == 0 and any("pre-checks skipped" in m for m in skipped), f"--no-pre-checks skips a red pre-check and the review ends 0: {rc} {skipped}") + # context seconds start after the pre-checks: a slow check is not part of them + open(os.path.join(repo, "tools/check-levers.sh"), "w").write("sleep 1.2\nexit 0\n") + sh("git", "add", "-A") + sh("git", "commit", "-qm", "topic: a slow pre-check") + runner("**Verdict: CLEAN**: fine\n\n### Checked\nall") + buf = io.StringIO() + a = argparse.Namespace(base="basebr", full=False, minutes=10, body=None, no_pre_checks=False) + with contextlib.redirect_stdout(buf): + rc = review(a, stubbed=True, say=lambda m: None) + ctx_s = re.search(r"context (\d+) s", buf.getvalue()) + expect(rc == 0 and ctx_s is not None and int(ctx_s.group(1)) == 0, + f"context time starts after pre-checks: {buf.getvalue()!r}") finally: os.chdir(cwd) g.update(saved) From 6420f2090dd1aaf4858212ec36d7f423e04459da Mon Sep 17 00:00:00 2001 From: midagedev Date: Thu, 15 Oct 2026 06:33:26 +0900 Subject: [PATCH 03/10] tools: a waiting pre-review takes whichever slot frees The third run polls both flock slots. The device-dir cache is published with replace, so a reader does not see a short file. The local-check table names cargo, which post.py device runs. --- docs/contrib/local-check.md | 2 +- tools/review/local.py | 39 ++++++++++++++++++++++++------------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/docs/contrib/local-check.md b/docs/contrib/local-check.md index 1ff4adfff..71b9a9623 100644 --- a/docs/contrib/local-check.md +++ b/docs/contrib/local-check.md @@ -31,7 +31,7 @@ runs of the same binary scatter on one machine. *DSpark* is V4.1's draft model f | Mac | `just mac-static [BASE]` | formatting, clippy and the build shapes `BASE`'s change touches check as x86_64 Linux, and every check script passes; no test or gate runs | the environment file the header of `tools/mac-check.sh` describes (section 2) | | x86_64 or aarch64 Linux, with or without a card | `just contrib-check --tier t1` | T0, then the tests of every crate that reaches no card | the CUDA headers, libclang, numpy and PIL (section 2) | | Linux with an NVIDIA card | `BLOOMERY_BOX_LOCAL=1 BLOOMERY_TIER=fixture just gate-`, for each gate of your list | that gate passes on your card at the fixture tier | an environment file, the fixtures and the reference sets (sections 3 and 4) | -| Any machine with the `claude` CLI logged in | `just pre-review` before each push of a pull request branch | Shingler's reviewer, on your committed HEAD with the pull request review's own instructions, finds no must-fix (exit 0) and prints one pull-request body line with the full HEAD sha; exit 1 lists must-fix items; a base behind origin/main is refused by name (64) | `git`, `gh` for an incremental review of an open pull request and the slag ledger, a Claude login | +| Any machine with the `claude` CLI logged in | `just pre-review` before each push of a pull request branch | Shingler's reviewer, on your committed HEAD with the pull request review's own instructions, finds no must-fix (exit 0) and prints one pull-request body line with the full HEAD sha; exit 1 lists must-fix items; a base behind origin/main is refused by name (64) | `git`, `gh` for an incremental review of an open pull request and the slag ledger, `cargo` (`post.py device` runs `recipes.py pure-crates`), a Claude login | Paste what each command printed into the pull request: for `contrib-check`, its header block, its unit lines and its `DONE` line; for a gate, its summary lines and its exit code. Name the machine as `CONTRIBUTING.md` asks ("Pull requests diff --git a/tools/review/local.py b/tools/review/local.py index 8fd77cea6..1ceb0453f 100644 --- a/tools/review/local.py +++ b/tools/review/local.py @@ -222,22 +222,24 @@ def device_dirs(base_tree, out, base_sha=None): raise Stop(70, f"pre-review: post.py device failed: {p.stderr.strip()[:200]}") if cache: os.makedirs(os.path.dirname(cache), exist_ok=True) - shutil.copy(out, cache) + tmp = f"{cache}.{os.getpid()}" + shutil.copy(out, tmp) + os.replace(tmp, cache) def review_slot(): - """One of two flock slots. A third pre-review blocks on the first until a holder finishes.""" + """One of two flock slots. A third pre-review polls both and takes the first that frees.""" d = os.path.expanduser("~/.cache/bloomery-review") os.makedirs(d, exist_ok=True) fds = [open(os.path.join(d, f"slot{i}"), "a") for i in (0, 1)] - for f in fds: - try: - fcntl.flock(f, fcntl.LOCK_EX | fcntl.LOCK_NB) - return f - except BlockingIOError: - continue - fcntl.flock(fds[0], fcntl.LOCK_EX) - return fds[0] + while True: + for f in fds: + try: + fcntl.flock(f, fcntl.LOCK_EX | fcntl.LOCK_NB) + return f + except BlockingIOError: + continue + time.sleep(0.05) def read_ledger(base_tree, out): @@ -663,10 +665,10 @@ def third(): th.start() time.sleep(0.4) waited = seen == [] - held.pop(0).close() + held.pop().close() # slot 1 frees; slot 0 stays held th.join(5) expect(waited and seen == [1] and not th.is_alive(), - f"a third pre-review waits for a slot: waited {waited}, ran {seen}, alive {th.is_alive()}") + f"a third pre-review takes the free slot, not only slot 0: waited {waited}, ran {seen}, alive {th.is_alive()}") finally: for f in held: f.close() @@ -689,14 +691,23 @@ def wrapped(argv, *a, **k): return real_run(argv, *a, **k) subprocess.run = wrapped + real_replace = os.replace try: outs = [os.path.join(t, n) for n in ("a", "b", "c")] + published = [] + + def wrap_replace(src, dst): + published.append(open(src).read()) + return real_replace(src, dst) + + os.replace = wrap_replace device_dirs(t, outs[0], "sha-a") device_dirs(t, outs[1], "sha-a") device_dirs(t, outs[2], "sha-b") - expect(ran == [1, 1] and open(outs[1]).read() == "crates/gpu/\n", - f"device dirs run once per base sha: ran {len(ran)}, second {open(outs[1]).read()!r}") + expect(ran == [1, 1] and open(outs[1]).read() == "crates/gpu/\n" and published == ["crates/gpu/\n", "crates/gpu/\n"], + f"device dirs run once per base sha and replace the cache whole: ran {len(ran)}, published {published}") finally: + os.replace = real_replace subprocess.run = real_run if home is None: os.environ.pop("HOME", None) From 4f7f338144b4ab1ce47a91b1ac2df3cb5bb4172f Mon Sep 17 00:00:00 2001 From: midagedev Date: Thu, 15 Oct 2026 07:41:38 +0900 Subject: [PATCH 04/10] tools: post.py device runs under the stable toolchain cargo metadata in the base tree would otherwise install the pinned nightly. The self-test row is red when that env is missing. --- tools/review/local.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/review/local.py b/tools/review/local.py index 1ceb0453f..776d5c5a9 100644 --- a/tools/review/local.py +++ b/tools/review/local.py @@ -216,8 +216,10 @@ def device_dirs(base_tree, out, base_sha=None): if cache and os.path.isfile(cache): shutil.copy(cache, out) return + # review.yml pins stable: cargo metadata must not install the nightly the base tree names. + env = dict(os.environ, RUSTUP_TOOLCHAIN="stable") p = subprocess.run([sys.executable, os.path.join(base_tree, "tools/review/post.py"), "device", "--root", base_tree, - "--out", out], capture_output=True, text=True) + "--out", out], capture_output=True, text=True, env=env) if p.returncode: raise Stop(70, f"pre-review: post.py device failed: {p.stderr.strip()[:200]}") if cache: @@ -681,11 +683,13 @@ def third(): home = os.environ.get("HOME") os.environ["HOME"] = t ran = [] + envs = [] real_run = subprocess.run def wrapped(argv, *a, **k): if len(argv) > 1 and str(argv[1]).endswith("post.py"): ran.append(1) + envs.append((k.get("env") or {}).get("RUSTUP_TOOLCHAIN")) open(argv[argv.index("--out") + 1], "w").write("crates/gpu/\n") return subprocess.CompletedProcess(argv, 0, "", "") return real_run(argv, *a, **k) @@ -706,6 +710,8 @@ def wrap_replace(src, dst): device_dirs(t, outs[2], "sha-b") expect(ran == [1, 1] and open(outs[1]).read() == "crates/gpu/\n" and published == ["crates/gpu/\n", "crates/gpu/\n"], f"device dirs run once per base sha and replace the cache whole: ran {len(ran)}, published {published}") + expect(envs == ["stable", "stable"], + f"post.py device runs under RUSTUP_TOOLCHAIN=stable: {envs}") finally: os.replace = real_replace subprocess.run = real_run From ba46eb44c52459dc6fc1be0afecdbc2c28de181d Mon Sep 17 00:00:00 2001 From: midagedev Date: Thu, 15 Oct 2026 09:26:57 +0900 Subject: [PATCH 05/10] tools/review/local.py: the older self-test namespaces carry no_pre_checks, so the flag is read directly Co-Authored-By: Claude Opus 5.5 --- tools/review/local.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/review/local.py b/tools/review/local.py index 776d5c5a9..f582d7c6c 100644 --- a/tools/review/local.py +++ b/tools/review/local.py @@ -431,7 +431,7 @@ def review(args, stubbed=False, say=lambda m: print(m, file=sys.stderr)): archive(head, os.path.join(d, "pr"), cwd=os.getcwd()) except RecipeError as e: raise Stop(70, f"pre-review: {e}") from e - if getattr(args, "no_pre_checks", False): + if args.no_pre_checks: say("pre-review: pre-checks skipped (--no-pre-checks)") else: pre_checks(os.path.join(d, "pr"), git("diff", "--name-only", "-z", f"{base_sha}...{head}").split("\0"), say) @@ -769,7 +769,7 @@ def fake_ledger(base_tree, out): sh("git", "update-ref", "refs/remotes/origin/main", "basebr") try: def go(text, sleep=0, minutes=10): - a = argparse.Namespace(base="basebr", full=False, minutes=minutes, body=None) + a = argparse.Namespace(base="basebr", full=False, minutes=minutes, body=None, no_pre_checks=False) msgs = [] try: runner(text, sleep) @@ -821,7 +821,7 @@ def go(text, sleep=0, minutes=10): f"a MUST_FIX review ends 1 and prints no PR-body line (got {rc}): {buf.getvalue()!r}") pidf = os.path.join(t, "child.pid") open(stub, "w").write(f"#!/usr/bin/env bash\n( sleep 30 & echo $! > {pidf}; wait )\n") - a = argparse.Namespace(base="basebr", full=False, minutes=0.03, body=None) + a = argparse.Namespace(base="basebr", full=False, minutes=0.03, body=None, no_pre_checks=False) try: rc, msg = review(a, stubbed=True, say=lambda m: None), "" except Stop as e: @@ -835,7 +835,7 @@ def go(text, sleep=0, minutes=10): f"a role past --minutes ends 75 named and its whole process group is gone: rc {rc}, child alive {alive}") # --full: a validator that ends 0 and leaves the review as the reviewer wrote it is no validation runner("**Verdict: CLEAN**: fine\n\n### Checked\nall") - a = argparse.Namespace(base="basebr", full=True, minutes=10, body=None) + a = argparse.Namespace(base="basebr", full=True, minutes=10, body=None, no_pre_checks=False) try: rc, msg = review(a, stubbed=True, say=lambda m: None), "" except Stop as e: From 751d45d2129780e6f98c913ab58a7653fd2738c3 Mon Sep 17 00:00:00 2001 From: midagedev Date: Thu, 15 Oct 2026 10:21:17 +0900 Subject: [PATCH 06/10] tools/review/local.py: time the context with a fake clock, and leave the toolchain to the repo The timing self-test reads a fake clock. Pre-check time is 10s on that clock and context time is 1s, so the boundary does not depend on a sleep. Moving the context sample to before the pre-checks reports 11s and the test goes red. post.py device inherits the caller's environment. rustup then reads rust-toolchain.toml. install.sh installs only that channel. --- tools/review/local.py | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/tools/review/local.py b/tools/review/local.py index f582d7c6c..43835608c 100644 --- a/tools/review/local.py +++ b/tools/review/local.py @@ -216,10 +216,9 @@ def device_dirs(base_tree, out, base_sha=None): if cache and os.path.isfile(cache): shutil.copy(cache, out) return - # review.yml pins stable: cargo metadata must not install the nightly the base tree names. - env = dict(os.environ, RUSTUP_TOOLCHAIN="stable") + # No toolchain override: rustup reads rust-toolchain.toml. install.sh installs only that channel, not stable. p = subprocess.run([sys.executable, os.path.join(base_tree, "tools/review/post.py"), "device", "--root", base_tree, - "--out", out], capture_output=True, text=True, env=env) + "--out", out], capture_output=True, text=True) if p.returncode: raise Stop(70, f"pre-review: post.py device failed: {p.stderr.strip()[:200]}") if cache: @@ -710,8 +709,8 @@ def wrap_replace(src, dst): device_dirs(t, outs[2], "sha-b") expect(ran == [1, 1] and open(outs[1]).read() == "crates/gpu/\n" and published == ["crates/gpu/\n", "crates/gpu/\n"], f"device dirs run once per base sha and replace the cache whole: ran {len(ran)}, published {published}") - expect(envs == ["stable", "stable"], - f"post.py device runs under RUSTUP_TOOLCHAIN=stable: {envs}") + expect(envs == [None, None], + f"post.py device does not force RUSTUP_TOOLCHAIN: {envs}") finally: os.replace = real_replace subprocess.run = real_run @@ -892,17 +891,23 @@ def go(text, sleep=0, minutes=10): rc = e.code expect(rc == 0 and any("pre-checks skipped" in m for m in skipped), f"--no-pre-checks skips a red pre-check and the review ends 0: {rc} {skipped}") - # context seconds start after the pre-checks: a slow check is not part of them - open(os.path.join(repo, "tools/check-levers.sh"), "w").write("sleep 1.2\nexit 0\n") + # context seconds start after the pre-checks: the clock, not a sleep, places that boundary + from types import SimpleNamespace + from unittest.mock import patch + + open(os.path.join(repo, "tools/check-levers.sh"), "w").write("exit 0\n") sh("git", "add", "-A") - sh("git", "commit", "-qm", "topic: a slow pre-check") + sh("git", "commit", "-qm", "topic: a timing pre-check") runner("**Verdict: CLEAN**: fine\n\n### Checked\nall") buf = io.StringIO() - a = argparse.Namespace(base="basebr", full=False, minutes=10, body=None, no_pre_checks=False) - with contextlib.redirect_stdout(buf): + a = argparse.Namespace(base="basebr", full=False, minutes=10, + body=None, no_pre_checks=False) + ticks = iter((0, 0, 10, 10, 11, 11)) + fake_time = SimpleNamespace(time=lambda: next(ticks)) + with patch.dict(g, time=fake_time), contextlib.redirect_stdout(buf): rc = review(a, stubbed=True, say=lambda m: None) ctx_s = re.search(r"context (\d+) s", buf.getvalue()) - expect(rc == 0 and ctx_s is not None and int(ctx_s.group(1)) == 0, + expect(rc == 0 and ctx_s is not None and int(ctx_s.group(1)) == 1, f"context time starts after pre-checks: {buf.getvalue()!r}") finally: os.chdir(cwd) From 855d7a343fc39ca5ae7cb326f78631f07579bd16 Mon Sep 17 00:00:00 2001 From: midagedev Date: Thu, 15 Oct 2026 10:30:13 +0900 Subject: [PATCH 07/10] tools/review/local.py: device_dirs requires the base sha A call without a sha skipped the cache and still wrote the device list. The only caller passes the sha. The argument is required, and the test stubs take it the same way. --- tools/review/local.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/tools/review/local.py b/tools/review/local.py index 43835608c..dbededdc8 100644 --- a/tools/review/local.py +++ b/tools/review/local.py @@ -210,10 +210,10 @@ def build_ctx(ctx, base_tree, base_sha, head, args, say): return meta -def device_dirs(base_tree, out, base_sha=None): +def device_dirs(base_tree, out, base_sha): """post.py device, copied from the cache for this base sha when that file is already there.""" - cache = base_sha and os.path.expanduser(f"~/.cache/bloomery-review/device-{base_sha}") - if cache and os.path.isfile(cache): + cache = os.path.expanduser(f"~/.cache/bloomery-review/device-{base_sha}") + if os.path.isfile(cache): shutil.copy(cache, out) return # No toolchain override: rustup reads rust-toolchain.toml. install.sh installs only that channel, not stable. @@ -221,11 +221,10 @@ def device_dirs(base_tree, out, base_sha=None): "--out", out], capture_output=True, text=True) if p.returncode: raise Stop(70, f"pre-review: post.py device failed: {p.stderr.strip()[:200]}") - if cache: - os.makedirs(os.path.dirname(cache), exist_ok=True) - tmp = f"{cache}.{os.getpid()}" - shutil.copy(out, tmp) - os.replace(tmp, cache) + os.makedirs(os.path.dirname(cache), exist_ok=True) + tmp = f"{cache}.{os.getpid()}" + shutil.copy(out, tmp) + os.replace(tmp, cache) def review_slot(): @@ -755,7 +754,7 @@ def runner(text, sleep=0): saved = {k: g[k] for k in ("gh_json", "device_dirs", "read_ledger")} # no network and no cargo here: gh answers nothing, the device list and the ledger are fixed g["gh_json"] = lambda *a: None - g["device_dirs"] = lambda base_tree, out, base_sha=None: open(out, "w").write("crates/gpu\n") + g["device_dirs"] = lambda base_tree, out, base_sha: open(out, "w").write("crates/gpu\n") def fake_ledger(base_tree, out): open(out, "w").close() return "v0" @@ -850,7 +849,7 @@ def go(text, sleep=0, minutes=10): wrapper = (f"import shutil, sys\nsys.path.insert(0, {HERE!r})\nimport local as L\nreal = shutil.which\n" "shutil.which = lambda n, *a: '/x' if n == 'claude' else real(n, *a)\n" "L.claude_ok = lambda: None\n" - "L.gh_json = lambda *a: None\nL.device_dirs = lambda b, o, s=None: open(o, 'w').write('crates/gpu\\n')\n" + "L.gh_json = lambda *a: None\nL.device_dirs = lambda b, o, s: open(o, 'w').write('crates/gpu\\n')\n" "L.read_ledger = lambda b, o: (open(o, 'w').close(), 'v0')[1]\n" "sys.exit(L.main(['--base', 'basebr', '--minutes', '5']))\n") proc = subprocess.Popen([sys.executable, "-c", wrapper], cwd=repo, stdout=subprocess.PIPE, stderr=subprocess.PIPE) From e84cf9b6f3dbe43d2eab94f11ac32fb304cd4a7e Mon Sep 17 00:00:00 2001 From: midagedev Date: Thu, 15 Oct 2026 10:44:52 +0900 Subject: [PATCH 08/10] tools/review/local.py: a stubbed review takes no flock slot The SIGTERM child sets HOME to its temp directory, so that run does not take the caller's slots. --- tools/review/local.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/review/local.py b/tools/review/local.py index dbededdc8..7b5565de5 100644 --- a/tools/review/local.py +++ b/tools/review/local.py @@ -397,7 +397,7 @@ def must_fix_titles(body): def review(args, stubbed=False, say=lambda m: print(m, file=sys.stderr)): - slot = review_slot() # held until this review returns + slot = None if stubbed else review_slot() # held until this review returns if not stubbed and not shutil.which("claude"): raise Stop(75, "pre-review: no claude CLI on PATH; nothing was reviewed") if not stubbed: @@ -852,7 +852,8 @@ def go(text, sleep=0, minutes=10): "L.gh_json = lambda *a: None\nL.device_dirs = lambda b, o, s: open(o, 'w').write('crates/gpu\\n')\n" "L.read_ledger = lambda b, o: (open(o, 'w').close(), 'v0')[1]\n" "sys.exit(L.main(['--base', 'basebr', '--minutes', '5']))\n") - proc = subprocess.Popen([sys.executable, "-c", wrapper], cwd=repo, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + proc = subprocess.Popen([sys.executable, "-c", wrapper], cwd=repo, stdout=subprocess.PIPE, stderr=subprocess.PIPE, + env={**os.environ, "HOME": t}) for _ in range(200): if os.path.exists(pidf) and open(pidf).read().strip(): break From f65b82a259da0498bec06bf16d3fe6616b215fd0 Mon Sep 17 00:00:00 2001 From: midagedev Date: Thu, 15 Oct 2026 10:49:26 +0900 Subject: [PATCH 09/10] tools/review/local.py: name a busy slot wait, and say broken install in English A third pre-review prints one line before it polls. claude --version's exit 75 says broken install. --- tools/review/local.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/tools/review/local.py b/tools/review/local.py index 7b5565de5..6f790f51c 100644 --- a/tools/review/local.py +++ b/tools/review/local.py @@ -227,11 +227,12 @@ def device_dirs(base_tree, out, base_sha): os.replace(tmp, cache) -def review_slot(): +def review_slot(say=lambda m: None): """One of two flock slots. A third pre-review polls both and takes the first that frees.""" d = os.path.expanduser("~/.cache/bloomery-review") os.makedirs(d, exist_ok=True) fds = [open(os.path.join(d, f"slot{i}"), "a") for i in (0, 1)] + said = False while True: for f in fds: try: @@ -239,6 +240,9 @@ def review_slot(): return f except BlockingIOError: continue + if not said: + say("pre-review: both review slots are busy; waiting") + said = True time.sleep(0.05) @@ -371,9 +375,9 @@ def claude_ok(): p = subprocess.run(["claude", "--version"], capture_output=True, text=True, stdin=subprocess.DEVNULL, timeout=15) except (OSError, subprocess.TimeoutExpired) as e: - raise Stop(75, f"pre-review: claude --version failed ({type(e).__name__}): 설치 깨짐; nothing was reviewed") from e + raise Stop(75, f"pre-review: claude --version failed ({type(e).__name__}): broken install; nothing was reviewed") from e if p.returncode != 0: - raise Stop(75, f"pre-review: claude --version failed (rc {p.returncode}): 설치 깨짐; nothing was reviewed") + raise Stop(75, f"pre-review: claude --version failed (rc {p.returncode}): broken install; nothing was reviewed") def pre_checks(pr_tree, paths, say): @@ -397,7 +401,7 @@ def must_fix_titles(body): def review(args, stubbed=False, say=lambda m: print(m, file=sys.stderr)): - slot = None if stubbed else review_slot() # held until this review returns + slot = None if stubbed else review_slot(say) # held until this review returns if not stubbed and not shutil.which("claude"): raise Stop(75, "pre-review: no claude CLI on PATH; nothing was reviewed") if not stubbed: @@ -606,7 +610,7 @@ def broken(root, pins): got = None except Stop as e: got = e - expect(got is not None and got.code == 75 and "설치 깨짐" in got.msg, + expect(got is not None and got.code == 75 and "broken install" in got.msg, f"a claude that fails --version ends 75 naming a broken install: {got and (got.code, got.msg)}") open(fake, "w").write("#!/bin/sh\necho 2.0.0\nexit 0\n") os.chmod(fake, 0o755) @@ -656,9 +660,10 @@ def broken(root, pins): try: held = [review_slot(), review_slot()] seen = [] + said = [] def third(): - review_slot() + review_slot(said.append) seen.append(1) th = threading.Thread(target=third) @@ -667,8 +672,10 @@ def third(): waited = seen == [] held.pop().close() # slot 1 frees; slot 0 stays held th.join(5) - expect(waited and seen == [1] and not th.is_alive(), - f"a third pre-review takes the free slot, not only slot 0: waited {waited}, ran {seen}, alive {th.is_alive()}") + expect(waited and seen == [1] and not th.is_alive() + and said == ["pre-review: both review slots are busy; waiting"], + f"a third pre-review takes the free slot, not only slot 0, and says it is waiting: " + f"waited {waited}, ran {seen}, alive {th.is_alive()}, said {said}") finally: for f in held: f.close() From f23040c06e649bf2822f0b197164edc6c1532a1a Mon Sep 17 00:00:00 2001 From: midagedev Date: Thu, 15 Oct 2026 11:55:21 +0900 Subject: [PATCH 10/10] tools/review/local.py: a claude --version that prints nothing is a broken install A binary that exits 0 with empty or whitespace-only output used to pass the health check. review() then built the context and exited 70 on the empty transcript. It is exit 75, the same as a red --version, before any of that work. Self-test: python3 tools/review/local.py --self-test, 0 failed. The empty and whitespace stubs were red before this change (claude_ok returned). --- tools/review/local.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tools/review/local.py b/tools/review/local.py index 6f790f51c..c6ecfaaca 100644 --- a/tools/review/local.py +++ b/tools/review/local.py @@ -376,7 +376,7 @@ def claude_ok(): stdin=subprocess.DEVNULL, timeout=15) except (OSError, subprocess.TimeoutExpired) as e: raise Stop(75, f"pre-review: claude --version failed ({type(e).__name__}): broken install; nothing was reviewed") from e - if p.returncode != 0: + if p.returncode != 0 or not (p.stdout + p.stderr).strip(): raise Stop(75, f"pre-review: claude --version failed (rc {p.returncode}): broken install; nothing was reviewed") @@ -620,6 +620,16 @@ def broken(root, pins): except Stop as e: ok_ver = False expect(ok_ver, "a claude that answers --version is a healthy install") + for body in ("#!/bin/sh\nexit 0\n", "#!/bin/sh\nprintf ' \\n\\n'\nexit 0\n"): + open(fake, "w").write(body) + os.chmod(fake, 0o755) + try: + claude_ok() + got = None + except Stop as e: + got = e + expect(got is not None and got.code == 75 and "broken install" in got.msg, + f"a claude that exits 0 with no version text ends 75 naming a broken install: {got and (got.code, got.msg)}") finally: if saved_path is None: os.environ.pop("PATH", None)