diff --git a/docs/contrib/local-check.md b/docs/contrib/local-check.md index d123142e7..1ff4adfff 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); exit 1 lists them | `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, 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/justfile b/justfile index 27f5c4d6a..76d2fd6ab 100644 --- a/justfile +++ b/justfile @@ -116,7 +116,9 @@ contrib-check *ARGS: # Before a push: Shingler's reviewer on this machine, on the committed HEAD against origin/main, under your own Claude # login (`--full` adds the validator, `--minutes N` bounds a role, default 10). Exit 0 CLEAN, 1 MUST_FIX, 75 no verdict. -# Nothing is posted; the pull request's own review stays the review of record. The owner is tools/review/local.py. +# A base behind origin/main is refused by name (64) before the reviewer runs. A CLEAN run prints one pull-request body +# line with the full HEAD sha. Nothing is posted; the pull request's own review stays the review of record. The owner +# is tools/review/local.py. pre-review *ARGS: python3 tools/review/local.py {{ARGS}} diff --git a/tools/review/local.py b/tools/review/local.py index 1977d3512..c3d6866ec 100644 --- a/tools/review/local.py +++ b/tools/review/local.py @@ -25,8 +25,12 @@ hand-run check-levers caught. Prints the verdict, the must-fix titles and where the output is (~/.cache/bloomery/pre-review/-/). -Exit: 0 CLEAN; 1 MUST_FIX or a red pre-check; 75 no claude CLI, or a role past --minutes (default 10); 64 usage; 65 output post.py refused -or a review with no verdict; 70 a context step that failed (named). +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, +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. """ @@ -80,6 +84,12 @@ def git(*args): return p.stdout +def git_ok(*args): + """stdout of a git call, or None when git refuses it. A clone with no origin/main is not refused for that.""" + p = subprocess.run(["git", *args], capture_output=True, text=True) + return p.stdout.strip() if p.returncode == 0 else None + + def gh_json(*args): """gh's JSON answer, or None when gh cannot answer (no gh, no login, no network).""" if not shutil.which("gh"): @@ -356,6 +366,13 @@ def review(args, stubbed=False, say=lambda m: print(m, file=sys.stderr)): say(f"pre-review: git fetch origin failed or took over {NET_S} s; {args.base} is as this clone last saw it") head = git("rev-parse", "HEAD").strip() base_sha = git("rev-parse", args.base).strip() + main_sha = git_ok("rev-parse", "--verify", "origin/main") + if main_sha and base_sha != main_sha: + behind = git("rev-list", "--count", f"{base_sha}..{main_sha}").strip() + if behind != "0": + noun = "commit" if behind == "1" else "commits" + raise Stop(64, f"pre-review: base {args.base} ({base_sha[:8]}) is behind origin/main ({main_sha[:8]}): " + f"origin/main has {behind} {noun} the base does not") if git("status", "--porcelain", "--untracked-files=no").strip(): say("pre-review: the tree has uncommitted changes; what is reviewed is the committed HEAD, what a push sends") branch = re.sub(r"[^A-Za-z0-9._-]", "_", git("rev-parse", "--abbrev-ref", "HEAD").strip()) @@ -407,6 +424,8 @@ def review(args, stubbed=False, say=lambda m: print(m, file=sys.stderr)): for t in titles[:12]: print(f" must fix: {t}") print(f" review: {keep}/out/review.md") + if m.group(1) == "CLEAN": + print(f"pre-review: PR body: Pre-review CLEAN at {head}.") return 1 if m.group(1) == "MUST_FIX" else 0 @@ -599,6 +618,8 @@ def fake_ledger(base_tree, out): home = os.environ.get("HOME") os.environ["HOME"] = os.path.join(t, "home") os.environ["PRE_REVIEW_TEST_STUB"] = stub + # origin/main at the base: the branch's base is not behind it. A later case moves it ahead. + 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) @@ -614,14 +635,43 @@ def go(text, sleep=0, minutes=10): rc, _ = go("**Verdict: CLEAN**: fine\n\n### Checked\nall") expect(rc == 0, f"a CLEAN review ends 0 (got {rc})") expect("$" not in buf.getvalue(), f"accounting that is not available is not printed as a cost: {buf.getvalue()!r}") + head_sha = git("rev-parse", "HEAD").strip() + expect(f"pre-review: PR body: Pre-review CLEAN at {head_sha}." in buf.getvalue().splitlines(), + f"a CLEAN review prints one PR-body line with the full HEAD sha: {buf.getvalue()!r}") + sh("git", "checkout", "-q", "main") + open(os.path.join(repo, "m.txt"), "w").write("m\n") + sh("git", "add", "m.txt") + sh("git", "commit", "-qm", "main moves") + sh("git", "update-ref", "refs/remotes/origin/main", "HEAD") + sh("git", "checkout", "-q", "topic") + rc, msgs = go("**Verdict: CLEAN**: should not run\n\n### Checked\nall") + expect(rc == 64 and any("is behind origin/main" in m for m in msgs), + f"a base behind origin/main is refused by name before the review: {rc} {msgs}") + sh("git", "update-ref", "refs/remotes/origin/main", "basebr") + # A commit whose parent object is gone: rev-parse still names it, rev-list cannot walk it. + tree = subprocess.run(["git", "rev-parse", "HEAD^{tree}"], cwd=repo, capture_output=True, + check=True, text=True).stdout.strip() + parent = subprocess.run(["git", "commit-tree", tree, "-m", "parent"], cwd=repo, + capture_output=True, check=True, text=True).stdout.strip() + child = subprocess.run(["git", "commit-tree", tree, "-p", parent, "-m", "child"], cwd=repo, + capture_output=True, check=True, text=True).stdout.strip() + os.remove(os.path.join(repo, ".git", "objects", parent[:2], parent[2:])) + sh("git", "update-ref", "refs/remotes/origin/main", child) + rc, msgs = go("**Verdict: CLEAN**: should not run\n\n### Checked\nall") + expect(rc == 70 and any("rev-list" in m for m in msgs), + f"a failed ancestry walk stops 70: {rc} {msgs}") + sh("git", "update-ref", "refs/remotes/origin/main", "basebr") keep = os.path.join(t, "home/.cache/bloomery/pre-review/topic-" + git("rev-parse", "HEAD").strip()[:8]) ctx_files = set(os.listdir(os.path.join(keep, "ctx"))) expect(ctx_files == set(CTX_FILES), f"the kept ctx holds the workflow's files: {sorted(ctx_files)}") pj = json.load(open(os.path.join(keep, "ctx/pr.json"))) expect(tuple(pj) == PR_KEYS and pj["mode"] == "full" and pj["run"] == "local", f"pr.json: {pj}") expect(open(os.path.join(keep, "ctx/files.txt")).read() == "modified\t+1\t-0\ta.txt\n", "files.txt of the branch") - rc, _ = go("**Verdict: MUST_FIX**: no\n\n### Must fix\n#### 1. Broken\n") - expect(rc == 1, f"a MUST_FIX review ends 1 (got {rc})") + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + rc, _ = go("**Verdict: MUST_FIX**: no\n\n### Must fix\n#### 1. Broken\n") + expect(rc == 1 and "PR body:" not in buf.getvalue(), + 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)