Repository navigation
Add interactive review for large changes #571
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+14,241
−54
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
c9e5a27
Check Change-Ids and review evidence in hooks
jserv 2ff31e8
Add the local interactive review workbench
jserv 2e35fbe
Declare reviews in pull requests; guard agents
jserv dd6bda9
Read diffs by content and draw them clearly
jserv e56bbf2
Fix review findings in the diff and answer checks
jserv 1a6b169
Guard the merge queue and unify Dependabot
jserv 8b5c73d
Wake the agent when the contributor confirms
jserv 2be8899
Close guard bypasses and stale blocks
jserv 97c9918
Add CODEOWNERS for the review gate
jserv File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| { | ||
| "hooks": { | ||
| "PreToolUse": [ | ||
| { | ||
| "matcher": "Bash", | ||
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "python3 \"$CLAUDE_PROJECT_DIR/scripts/agent-guard.py\"" | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,226 @@ | ||
| #!/usr/bin/env python3 | ||
| """Collect parent-relative commit sizes from a local PR range without network access.""" | ||
|
|
||
| import argparse | ||
| import json | ||
| import os | ||
| from pathlib import Path | ||
| import shutil | ||
| import subprocess | ||
| import sys | ||
| import tempfile | ||
| from contextlib import contextmanager | ||
|
|
||
|
|
||
| def git(*args): | ||
| return subprocess.check_output( | ||
| ["git", "--no-lazy-fetch", *args], | ||
| stderr=subprocess.PIPE, | ||
| env={**os.environ, "GIT_TERMINAL_PROMPT": "0", "GIT_NO_REPLACE_OBJECTS": "1"}, | ||
| ) | ||
|
|
||
|
|
||
| @contextmanager | ||
| def canonical(source, worktree=False): | ||
| objects = Path(git("rev-parse", "--git-path", "objects").decode().strip()).resolve() | ||
| env = {k: v for k, v in os.environ.items() if not k.startswith("GIT_")} | ||
| env.update( | ||
| GIT_CONFIG_NOSYSTEM="1", | ||
| GIT_CONFIG_GLOBAL=os.devnull, | ||
| GIT_ATTR_NOSYSTEM="1", | ||
| GIT_NO_REPLACE_OBJECTS="1", | ||
| GIT_TERMINAL_PROMPT="0", | ||
| ) | ||
| with tempfile.TemporaryDirectory(prefix="codetrial-diff-") as directory: | ||
| subprocess.run( | ||
| ["git", "init", "--bare", "--quiet", "--template=", directory], | ||
| env=env, | ||
| check=True, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, | ||
| ) | ||
| context = Path(directory) | ||
| (context / "objects/info/alternates").write_text( | ||
| json.dumps(str(objects), ensure_ascii=False) + "\n" | ||
| ) | ||
| prefix = [ | ||
| "git", | ||
| "--no-lazy-fetch", | ||
| "--git-dir=" + directory, | ||
| "--attr-source=" + source, | ||
| "-c", | ||
| "core.attributesFile=" + os.devnull, | ||
| ] | ||
| if worktree: | ||
| top = git("rev-parse", "--show-toplevel").decode().strip() | ||
| index = Path( | ||
| git("rev-parse", "--git-path", "index").decode().strip() | ||
| ).resolve() | ||
| if index.exists(): | ||
| shutil.copy2(index, context / "index") | ||
| shared_path = git("rev-parse", "--shared-index-path").decode().strip() | ||
| if shared_path: | ||
| shutil.copy2(shared_path, context / Path(shared_path).name) | ||
| for shared in index.parent.glob("sharedindex.*"): | ||
| shutil.copy2(shared, context / shared.name) | ||
| (context / "HEAD").write_text(source + "\n") | ||
| prefix.extend(["--work-tree=" + top, "-c", "core.bare=false"]) | ||
|
|
||
| def run(*args): | ||
| return subprocess.check_output( | ||
| [*prefix, *args], env=env, stderr=subprocess.PIPE | ||
| ) | ||
|
|
||
| yield run | ||
|
|
||
|
|
||
| def diff(base, head=None, *, numstat=False, attributes=None): | ||
| # `attributes` pins which commit's .gitattributes decide binary and -diff. | ||
| # A change between two commits is read under the base's, so a branch cannot | ||
| # hide what it adds from the reviewer, or from the count, by marking it | ||
| # -diff; the working tree has no base to pin and keeps its own. | ||
| source = head or git("rev-parse", "HEAD").decode().strip() | ||
| with canonical( | ||
| attributes or (base if head else source), worktree=head is None | ||
| ) as run: | ||
| flags = [ | ||
| "--no-color", | ||
| "--no-ext-diff", | ||
| "--no-textconv", | ||
| "--no-relative", | ||
| "--src-prefix=a/", | ||
| "--dst-prefix=b/", | ||
| "--full-index", | ||
| "--diff-algorithm=myers", | ||
| "--no-indent-heuristic", | ||
| ] | ||
| flags += ( | ||
| ["--numstat", "--no-renames", "-z"] | ||
| if numstat | ||
| else ["--find-renames=50%", "--unified=3"] | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
Resolved
|
||
| ) | ||
| return run("diff", *flags, base, *([head] if head else []), "--") | ||
|
|
||
|
|
||
| def resolve(ref): | ||
| return ( | ||
| git("rev-parse", "--verify", "--end-of-options", ref + "^{commit}") | ||
| .decode() | ||
| .strip() | ||
| ) | ||
|
|
||
|
|
||
| def collect(base, head): | ||
| if git("rev-parse", "--is-shallow-repository").strip() == b"true": | ||
| raise ValueError("Fetch complete history before measuring commit sizes") | ||
| grafts = Path(git("rev-parse", "--git-path", "info/grafts").decode().strip()) | ||
| if grafts.exists() and grafts.read_bytes().strip(): | ||
| raise ValueError("Remove local history grafts before measuring commit sizes") | ||
| head = resolve(head) | ||
| base = resolve(base) | ||
| ancestor = git("merge-base", base, head).decode().strip() | ||
| commits = [] | ||
| complete = True | ||
| for sha in ( | ||
| git("rev-list", "--reverse", ancestor + ".." + head).decode().splitlines() | ||
| ): | ||
| parents = git("rev-list", "--parents", "-n", "1", sha).decode().split() | ||
| comparison = [parents[1], sha] if len(parents) > 1 else ["--root", sha] | ||
| with canonical(sha) as run: | ||
| if len(parents) == 3: | ||
| # A clean integration (a merge of upstream main, a merge queue) | ||
| # adds nothing of its own; size only what it changes beyond the | ||
| # clean merge, so incoming commits are not counted as this one's. | ||
| try: | ||
| clean = run("merge-tree", "--write-tree", *parents[1:]).split( | ||
| b"\n" | ||
| )[0] | ||
| comparison = [clean.decode(), sha] | ||
| except subprocess.CalledProcessError: | ||
| pass | ||
| raw = run( | ||
| "diff-tree", | ||
| "--no-commit-id", | ||
| "--no-ext-diff", | ||
| "--no-textconv", | ||
| "-r", | ||
| "--numstat", | ||
| "--no-renames", | ||
| "-z", | ||
| *comparison, | ||
| ) | ||
| files = [] | ||
| for record in raw.split(b"\0"): | ||
| if not record: | ||
| continue | ||
| added, deleted, path = record.split(b"\t", 2) | ||
| files.append( | ||
| { | ||
| "path": os.fsdecode(path), | ||
| "additions": int(added) if added != b"-" else None, | ||
| "deletions": int(deleted) if deleted != b"-" else None, | ||
| } | ||
| ) | ||
| known = all(f[k] is not None for f in files for k in ("additions", "deletions")) | ||
| complete = complete and known | ||
| commits.append( | ||
| { | ||
| "sha": sha, | ||
| "files": files, | ||
| "known_lines": sum( | ||
| (f["additions"] or 0) + (f["deletions"] or 0) for f in files | ||
| ), | ||
| "complete": known, | ||
| } | ||
| ) | ||
| return {"head": head, "base": ancestor, "commits": commits, "complete": complete} | ||
|
|
||
|
|
||
| def main(): | ||
| parser = argparse.ArgumentParser(description=__doc__) | ||
| parser.add_argument("--base") | ||
| parser.add_argument("--diff", help="Local bundle comparison: base or base...HEAD") | ||
| parser.add_argument("--numstat", action="store_true") | ||
| parser.add_argument("--head", default="HEAD") | ||
| args = parser.parse_args() | ||
| try: | ||
| if args.diff: | ||
| if args.diff.endswith("...HEAD"): | ||
| head = resolve("HEAD") | ||
| base = git("merge-base", args.diff[:-7], head).decode().strip() | ||
| raw = diff(base, head, numstat=args.numstat) | ||
| else: | ||
| base = ( | ||
| git( | ||
| "rev-parse", | ||
| "--verify", | ||
| "--end-of-options", | ||
| args.diff + "^{commit}", | ||
| ) | ||
| .decode() | ||
| .strip() | ||
| ) | ||
| raw = diff(base, numstat=args.numstat) | ||
|
|
||
| sys.stdout.buffer.write(raw) | ||
| return 0 | ||
| if not args.base: | ||
| parser.error("--base or --diff is required") | ||
| result = collect(args.base, args.head) | ||
| except (subprocess.CalledProcessError, ValueError, OSError): | ||
| print( | ||
| json.dumps( | ||
| { | ||
| "commits": [], | ||
| "complete": False, | ||
| "error": "Cannot inspect the full local commit range; use a checkout with its base and head", | ||
| } | ||
| ) | ||
| ) | ||
| return 1 | ||
| print(json.dumps(result)) | ||
| return 0 | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| raise SystemExit(main()) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.