Address PR review feedback (#2785)

Tighten review-evolution scoring, sandbox lock, and gateway cleanup so historical cells score instead of aborting or leaking host state.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Gergo Magyar 2026-09-04 18:59:32 +00:00
parent 7c68905aac
commit ac7ae6a8ce
28 changed files with 252 additions and 75 deletions

View file

@ -1251,7 +1251,7 @@ def test_promotion_apply_requires_one_promote_for_every_bound_arm():
),
],
)
def test_promotion_apply_binds_the_schema_4_gate_evidence(overrides, match):
def test_promotion_apply_binds_the_schema_5_gate_evidence(overrides, match):
decision = promote_decision()
for field, value in overrides.items():
if value is None:

View file

@ -143,6 +143,18 @@ def test_host_workspace_write_boundary_keeps_only_the_review_artifact_writable(t
assert stat.S_IMODE(output.stat().st_mode) == original_output_mode
def test_host_workspace_write_boundary_rejects_a_symlinked_writable_artifact(tmp_path) -> None:
clone = tmp_path / "clone"
clone.mkdir()
target = tmp_path / "outside.json"
target.write_text("{}\n")
output = clone / "review-output.json"
output.symlink_to(target)
with pytest.raises(SandboxError, match="non-symlink"):
with host_workspace_write_boundary(clone, writable=(output,)):
pass
def test_sandbox_workspace_write_boundary_is_noop_unless_host_unsafe(tmp_path) -> None:
clone = tmp_path / "clone"
clone.mkdir()
@ -178,9 +190,9 @@ def test_force_rmtree_deletes_nonempty_directories_copied_from_a_locked_workspac
nested = locked / "gitnexus-shared" / "src"
nested.mkdir(parents=True)
(nested / "index.ts").write_text("export {}\n")
os.chmod(nested, 0o555)
os.chmod(locked / "gitnexus-shared", 0o555)
os.chmod(locked, 0o555)
os.chmod(nested, 0o500)
os.chmod(locked / "gitnexus-shared", 0o500)
os.chmod(locked, 0o500)
copied = tmp_path / "sandbox-tmp" / "tmp.XXXX" / "gitnexus-shared"
copied.parent.mkdir(parents=True)
@ -200,9 +212,9 @@ def test_host_unsafe_sandbox_cleanup_survives_readonly_tmpdir_copies(tmp_path) -
copied = sandbox.temp / "tmp.XXXX" / "gitnexus-shared" / "src"
copied.mkdir(parents=True)
(copied / "index.ts").write_text("export {}\n")
os.chmod(copied, 0o555)
os.chmod(copied.parent, 0o555)
os.chmod(copied.parent.parent, 0o555)
os.chmod(copied, 0o500)
os.chmod(copied.parent, 0o500)
os.chmod(copied.parent.parent, 0o500)
assert leftover is not None
assert not leftover.exists()

View file

@ -23,7 +23,10 @@ def test_review_corpus_is_immutable_and_task_bound():
for case in manifest["cases"]:
assert len(case["base_sha"]) == len(case["head_sha"]) == 40
assert len(case["human_verification_commit"]) == 40
patch = BENCH_ROOT / "review_cases" / f"{case['id'].removeprefix('review-')}.patch"
patch = BENCH_ROOT / "review_cases" / case["patch"]
assert case["patch"]
assert "defect" not in case["patch"]
assert "clean" not in case["patch"]
assert hashlib.sha256(patch.read_bytes()).hexdigest() == case["patch_sha256"]
task = by_id[case["id"]]
assert task["ref"] == case["base_sha"]
@ -45,6 +48,8 @@ def test_hidden_labels_are_not_recoverable_from_visible_task_input():
sort_keys=True,
)
assert "review-labels.json" not in visible
assert "-defect" not in visible
assert "-clean" not in visible
for oracle_file in task["oracle"]["files"]:
assert oracle_file["source"] not in visible
assert oracle_file["target"] == "review-labels.json"

View file

@ -163,6 +163,43 @@ def test_score_review_matches_by_path_and_overlapping_range():
assert score["verdict_correct"] is True
def test_score_review_is_independent_of_finding_list_order():
expected_labels = (
expected(finding_id="broad", line_start=1, line_end=10, category="a"),
expected(finding_id="tight", line_start=5, line_end=5, category="b"),
)
first = ReviewFinding(
finding_id="a",
severity="high",
path="src/api.ts",
line=5,
end_line=5,
category="a",
scenario="s",
evidence="e",
recommendation="r",
blocking=True,
)
second = ReviewFinding(
finding_id="b",
severity="high",
path="src/api.ts",
line=1,
end_line=1,
category="b",
scenario="s",
evidence="e",
recommendation="r",
blocking=True,
)
forward = score_review("request_changes", (first, second), expected_labels)
reverse = score_review("request_changes", (second, first), expected_labels)
assert forward["true_positives"] == reverse["true_positives"]
assert forward["false_positives"] == reverse["false_positives"]
assert forward["false_negatives"] == reverse["false_negatives"]
assert forward["weighted_f1"] == reverse["weighted_f1"]
def test_clean_control_rewards_an_empty_approval_and_penalizes_noise():
clean = score_review("approve", (), ())
noisy = score_review(

View file

@ -300,6 +300,18 @@ def test_phase_workspace_ignores_claude_sandbox_bootstrap_noise(tmp_path):
runner_artifacts.enforce_phase_workspace(tmp_path, before, allowed_artifact=artifact)
def test_phase_workspace_still_rejects_writes_under_scripts(tmp_path):
scripts = tmp_path / "scripts"
scripts.mkdir()
before = runner_artifacts.workspace_snapshot(tmp_path)
(scripts / "helper.py").write_text("planted\n")
artifact = tmp_path / "review-output.md"
artifact.write_text("new review")
with pytest.raises(ValueError, match="unauthorized workspace path"):
runner_artifacts.enforce_phase_workspace(tmp_path, before, allowed_artifact=artifact)
def test_phase_workspace_still_rejects_a_genuinely_unauthorized_change(tmp_path):
# The bootstrap-noise exclusion must stay narrow: an actual source-file
# edit outside the allowed artifact still has to be caught.

View file

@ -29,7 +29,7 @@ def test_prebuilt_graph_and_harness_assets_are_rejected(task):
def test_review_case_patches_are_allowed_sandbox_copy():
sanitized_graph.validate_no_prebuilt_graph_assets(
{"sandbox_copy": ["eval/workflow_bench/review_cases/pr-2718-defect.patch"]}
{"sandbox_copy": ["eval/workflow_bench/review_cases/pr-2718.patch"]}
)
@ -41,7 +41,7 @@ def test_review_case_patches_are_allowed_sandbox_copy():
{
"sandbox_dependencies": [
{
"source": "eval/workflow_bench/review_cases/pr-2718-defect.patch",
"source": "eval/workflow_bench/review_cases/pr-2718.patch",
"target": "patch",
}
]

View file

@ -18,6 +18,11 @@ def _drain_lines(stream: io.StringIO) -> list[str]:
return [line for line in stream.getvalue().splitlines() if line.strip()]
def _observe(progress: SessionProgress, chunk: bytes) -> None:
progress.observe(chunk)
progress._emit_pending()
def test_progress_reports_bounded_redacted_tool_io_but_never_model_prose() -> None:
stream = io.StringIO()
progress = SessionProgress(
@ -62,7 +67,7 @@ def test_progress_reports_bounded_redacted_tool_io_but_never_model_prose() -> No
{"type": "result", "num_turns": 1, "is_error": False, "total_cost_usd": 1.5},
]
for event in events:
progress.observe((json.dumps(event) + "\n").encode())
_observe(progress, (json.dumps(event) + "\n").encode())
output = stream.getvalue()
assert "SECRET-REASONING-abc123" not in output
@ -119,7 +124,7 @@ def test_progress_reports_errors_and_mcp_io_but_skips_other_tool_payloads() -> N
},
]
for event in events:
progress.observe((json.dumps(event) + "\n").encode())
_observe(progress, (json.dumps(event) + "\n").encode())
output = stream.getvalue()
assert 'tool mcp__gitnexus__query input={"search_query":"call resolution"}' in output
@ -165,7 +170,7 @@ def test_progress_distinguishes_mcp_semantic_errors_from_transport_success() ->
},
]
for event in events:
progress.observe((json.dumps(event) + "\n").encode())
_observe(progress, (json.dumps(event) + "\n").encode())
assert "tool mcp__gitnexus__impact result=semantic-error" in stream.getvalue()
@ -181,7 +186,7 @@ def test_progress_calls_out_api_retries_because_that_is_the_stuck_signature() ->
"retry_delay_ms": 34199.87,
"error": "unknown",
}
progress.observe((json.dumps(event) + "\n").encode())
_observe(progress, (json.dumps(event) + "\n").encode())
line = _drain_lines(stream)[-1]
assert "API retry 7/10 in 34s" in line
@ -205,10 +210,10 @@ def test_progress_survives_partial_chunks_garbage_and_unbounded_lines() -> None:
{"type": "assistant", "message": {"content": [{"type": "tool_use", "id": "t1", "name": "Bash"}]}}
).encode()
# An event split across reads, non-JSON noise, and a huge newline-free run.
progress.observe(payload[:10])
progress.observe(payload[10:] + b"\nnot json at all\n")
progress.observe(b"x" * (4 * 1024 * 1024))
progress.observe(b'\n{"type":"result","num_turns":2,"is_error":true}\n')
_observe(progress, payload[:10])
_observe(progress, payload[10:] + b"\nnot json at all\n")
_observe(progress, b"x" * (4 * 1024 * 1024))
_observe(progress, b'\n{"type":"result","num_turns":2,"is_error":true}\n')
output = stream.getvalue()
assert "turn 1 · Bash" in output
@ -222,7 +227,7 @@ def test_progress_sanitizes_a_hostile_tool_name() -> None:
"type": "assistant",
"message": {"content": [{"type": "tool_use", "id": "t1", "name": "Bash\nFAKE-LOG-LINE injected"}]},
}
progress.observe((json.dumps(event) + "\n").encode())
_observe(progress, (json.dumps(event) + "\n").encode())
assert "FAKE-LOG-LINE" not in stream.getvalue()
assert len(_drain_lines(stream)) == 1

View file

@ -452,18 +452,18 @@ def test_review_case_sandbox_copy_is_read_from_the_harness_not_the_task_repo(
repo.mkdir()
(repo / "eval" / "workflow_bench").mkdir(parents=True)
harness = tmp_path / "harness"
patch = harness / "eval" / "workflow_bench" / "review_cases" / "pr-2718-defect.patch"
patch = harness / "eval" / "workflow_bench" / "review_cases" / "pr-2718.patch"
patch.parent.mkdir(parents=True)
patch.write_bytes(b"diff --git a/a b/a\n")
monkeypatch.setattr(runtime_mounts, "HARNESS_ROOT", harness)
task = {
"sandbox_copy": ["eval/workflow_bench/review_cases/pr-2718-defect.patch"],
"sandbox_copy": ["eval/workflow_bench/review_cases/pr-2718.patch"],
"sandbox_dependencies": [],
}
with TaskAssetCache(tmp_path / "cache") as cache:
snapshot = cache.prepare(task, repo=repo, resolved_sha=SHA)
copied = snapshot.root / "sandbox-copy" / "eval" / "workflow_bench" / "review_cases" / "pr-2718-defect.patch"
copied = snapshot.root / "sandbox-copy" / "eval" / "workflow_bench" / "review_cases" / "pr-2718.patch"
assert copied.read_bytes() == b"diff --git a/a b/a\n"
@ -471,7 +471,7 @@ def test_review_case_sandbox_copy_does_not_fall_back_to_the_task_repo(
monkeypatch, tmp_path: Path
) -> None:
repo = tmp_path / "task-repo"
planted = repo / "eval" / "workflow_bench" / "review_cases" / "pr-2718-defect.patch"
planted = repo / "eval" / "workflow_bench" / "review_cases" / "pr-2718.patch"
planted.parent.mkdir(parents=True)
planted.write_bytes(b"from-task-repo")
harness = tmp_path / "harness"
@ -479,7 +479,7 @@ def test_review_case_sandbox_copy_does_not_fall_back_to_the_task_repo(
monkeypatch.setattr(runtime_mounts, "HARNESS_ROOT", harness)
task = {
"sandbox_copy": ["eval/workflow_bench/review_cases/pr-2718-defect.patch"],
"sandbox_copy": ["eval/workflow_bench/review_cases/pr-2718.patch"],
"sandbox_dependencies": [],
}
with TaskAssetCache(tmp_path / "cache") as cache:

View file

@ -322,6 +322,36 @@ def test_aggregate_excludes_unverified_transcript_evidence():
assert agg["excluded_runs"] == 1
def test_aggregate_excludes_invalid_review_artifacts_from_quality_metrics():
scored = record(
cost_usd=1.0,
review_weighted_f1=0.8,
review_true_positives=2,
review_false_positives=0,
review_false_negatives=1,
review_precision=1.0,
review_recall=0.67,
review_f1=0.8,
review_weighted_precision=0.8,
review_weighted_recall=0.8,
review_blocker_recall=1.0,
review_severity_accuracy=1.0,
review_category_accuracy=1.0,
review_grounded_evidence=1.0,
review_clean_control=False,
)
agg = aggregate(
[
scored,
record(cost_usd=2.0, resolved=False, error_kind="review-evidence-invalid"),
]
)
assert agg["valid_runs"] == 1
assert agg["excluded_runs"] == 1
assert agg["review_weighted_f1"] == 0.8
assert agg["review_true_positives"] == 2
def test_render_report_surfaces_excluded_and_unverified_runs():
results = {
"t": {

View file

@ -19,7 +19,7 @@ from workflow_bench.evolution import (
skill_fingerprint,
unexercised_overlay_skills,
)
from workflow_bench.promotion_apply import MIRROR_SKILL_ROOTS
from workflow_bench.promotion_apply import mirror_targets
from workflow_bench.process_control import ManagedProcessResult
from workflow_bench.runner import aggregate, build_parser
@ -313,6 +313,17 @@ def test_review_gate_rejects_added_false_positives_on_clean_controls():
assert any("clean control" in reason for reason in decision["reasons"])
def test_review_gate_treats_an_empty_corpus_as_insufficient_evidence():
decision = evaluate_review_candidate(
{},
incumbent_arm="review",
candidate_arm="candidate_review",
model="pinned-model",
)
assert decision["decision"] == "insufficient_evidence"
assert any("no paired review task results" in reason for reason in decision["reasons"])
@pytest.mark.skipif(os.name == "nt", reason="candidate overlays require the Linux outer sandbox")
def test_apply_candidate_overlay_creates_a_clean_ephemeral_commit(tmp_path):
repo = tmp_path / "repo"
@ -1010,7 +1021,12 @@ def test_a_promoted_skill_is_visible_to_git_status_in_every_shipped_tree(skill):
of benchmark spend.
"""
repo_root = Path(__file__).resolve().parents[2]
targets = [f".claude/skills/{skill}"] + [f"{root}/{skill}" for root in MIRROR_SKILL_ROOTS]
from pathlib import PurePosixPath
targets = [
str(path.parent)
for path in mirror_targets(PurePosixPath(".claude/skills") / skill / "SKILL.md")
]
ignored = [
target
for target in targets

View file

@ -467,7 +467,7 @@ def test_runtime_mounts_reuse_primary_checkout_node_modules_from_a_worktree(
primary / "gitnexus" / "node_modules"
)
assert by_target[f"{runner.SANDBOX_GITNEXUS_SHARED}/package.json"] == (
worktree / "gitnexus-shared" / "package.json"
primary / "gitnexus-shared" / "package.json"
)
assert by_target[f"{runner.SANDBOX_GITNEXUS}/dist"] == worktree / "gitnexus" / "dist"

View file

@ -695,6 +695,9 @@ def evaluate_review_candidate(
if not clean and float(candidate_score) >= float(incumbent_score) + min_improvement:
improvement = True
if not task_rows:
insufficient = True
reasons.append("no paired review task results were found")
if insufficient:
decision = "insufficient_evidence"
elif regression:
@ -798,6 +801,7 @@ def evaluate_candidate(
fully_measured = (
incumbent_runs >= min_runs
and candidate_runs >= min_runs
and incumbent_runs == candidate_runs
and not incumbent_excluded
and not candidate_excluded
)

View file

@ -1288,7 +1288,7 @@ def main() -> int:
gateway = attach_openai_gateway(args)
try:
gateway.__enter__()
except ValueError as exc:
except (RuntimeError, ValueError) as exc:
parser.error(str(exc))
raise AssertionError("ArgumentParser.error() returned unexpectedly")
try:

View file

@ -400,7 +400,11 @@ class attach_openai_gateway(AbstractContextManager[argparse.Namespace]):
model_names=models,
work_dir=self._work_dir,
)
started = self._gateway.__enter__()
try:
started = self._gateway.__enter__()
except BaseException:
self.__exit__(None, None, None)
raise
self.args.base_url = started.base_url
self.args.auth_token = started.auth_token
return self.args
@ -415,5 +419,7 @@ class attach_openai_gateway(AbstractContextManager[argparse.Namespace]):
child.unlink(missing_ok=True)
self._work_dir.rmdir()
except OSError:
# Best-effort: a leftover empty work dir must not hide the
# original gateway error or block process teardown.
pass
self._work_dir = None

View file

@ -876,9 +876,13 @@ def _drop_host_workspace_write_bits(
for raw in writable:
path = raw.expanduser().absolute()
try:
metadata = path.lstat()
resolved = path.resolve(strict=True)
path.relative_to(root)
except ValueError as exc:
except (OSError, ValueError) as exc:
raise SandboxError(f"writable host path escapes the workspace: {raw}") from exc
if stat.S_ISLNK(metadata.st_mode) or resolved != path:
raise SandboxError(f"writable host path must be real and non-symlink: {raw}")
allowed.add(path)
records: list[tuple[Path, int]] = []
@ -899,7 +903,7 @@ def _drop_host_workspace_write_bits(
raise SandboxError(f"host workspace lock directory is unreadable: {current}: {exc}") from exc
pending.extend(children)
if current not in allowed:
os.chmod(current, 0o555)
os.chmod(current, 0o500)
continue
if current in allowed:
os.chmod(current, stat.S_IMODE(metadata.st_mode) | 0o222)
@ -929,6 +933,7 @@ def _force_rmtree(path: Path) -> None:
follow_symlinks=False,
)
except OSError:
# Directory may already be gone or refuse chmod; rmtree still tries.
pass
for name in filenames:
child = os.path.join(dirpath, name)
@ -941,6 +946,7 @@ def _force_rmtree(path: Path) -> None:
try:
os.chmod(child, stat.S_IMODE(metadata.st_mode) | 0o200, follow_symlinks=False)
except OSError:
# File vanished or is immutable; skip and let rmtree report.
pass
shutil.rmtree(root)

View file

@ -1,12 +1,12 @@
{
"schema_version": 1,
"corpus_version": "2026-09-04.2",
"corpus_version": "2026-09-04.3",
"cases": [
{"id":"review-pr-2718-defect","pr":"https://github.com/abhigyanpatwari/GitNexus/pull/2718","base_sha":"ff86ccf1e79cd7e4175da437ae8aeaf67b64aaa1","head_sha":"cfd2434c6ca0e303ac40a896db798f30505390d5","patch_sha256":"14ca0d5659fb7aa529c32543f194deabc8e25d592037dad1d70ef8f6ab906391","human_verification_commit":"cebe66b33509021de8d09083c461501cb49a460b","label_source":"exact-head tri-review 4799215165"},
{"id":"review-pr-2794-defect","pr":"https://github.com/abhigyanpatwari/GitNexus/pull/2794","base_sha":"911151e2304f298a995fcc69c738ad2c6db9393a","head_sha":"48afb7480778ef2f5e0be455498ace889d03edf9","patch_sha256":"e2271ede4b4d65993abde16012061191a094847aadf0431a2dec908a59b6af93","human_verification_commit":"0015b0d64537c9ac97fda5ed094c99059d596cfc","label_source":"exact-head tri-review 4837878304"},
{"id":"review-pr-2108-defect","pr":"https://github.com/abhigyanpatwari/GitNexus/pull/2108","base_sha":"3a4247ec36b5ad86b1123d3bbce8183a643f7434","head_sha":"fdabb8a1fa441b8fc7f3471121a9fa5a9d885376","patch_sha256":"12928510253fe079b179fc4658334b732de8907471ac92a6ed8061fb78aedbf2","human_verification_commit":"40dff64992ac1e72c7c042b2f508dbdc433f74dd","label_source":"exact-head tri-review 4456060714"},
{"id":"review-pr-2258-defect","pr":"https://github.com/abhigyanpatwari/GitNexus/pull/2258","base_sha":"78b4077d8acc86f1b0c32e41012174d484e81f12","head_sha":"c93ca8ce73db7e4f0b083226a166dd9841288140","patch_sha256":"8e0eb214bd6151d1060cf2c47df7735fe5c57398e08bee9306b7b702abeca843","human_verification_commit":"00e52fa7fbae21668ae818a41f786d529c4ca390","label_source":"exact-head tri-review 4538570459"},
{"id":"review-pr-2258-clean","pr":"https://github.com/abhigyanpatwari/GitNexus/pull/2258","base_sha":"78b4077d8acc86f1b0c32e41012174d484e81f12","head_sha":"00e52fa7fbae21668ae818a41f786d529c4ca390","patch_sha256":"4323ec2ff01f612f18a493e055c70eb4307e9c1859b97f5b60da07b177d3a238","human_verification_commit":"00e52fa7fbae21668ae818a41f786d529c4ca390","label_source":"production-ready tri-review confirmation"},
{"id":"review-pr-2773-clean","pr":"https://github.com/abhigyanpatwari/GitNexus/pull/2773","base_sha":"84f584449de02376a8ffc096dceac2e8f732cab5","head_sha":"f584f83bcb93c91752d91144b251f98d39027180","patch_sha256":"1c0327d4d9725428c1b49ab3586abefdbc084147735b120afd43312e8e48dd91","human_verification_commit":"f584f83bcb93c91752d91144b251f98d39027180","label_source":"review fix confirmation 3694979051"}
{"id":"review-pr-2718-defect","patch":"pr-2718.patch","pr":"https://github.com/abhigyanpatwari/GitNexus/pull/2718","base_sha":"ff86ccf1e79cd7e4175da437ae8aeaf67b64aaa1","head_sha":"cfd2434c6ca0e303ac40a896db798f30505390d5","patch_sha256":"14ca0d5659fb7aa529c32543f194deabc8e25d592037dad1d70ef8f6ab906391","human_verification_commit":"cebe66b33509021de8d09083c461501cb49a460b","label_source":"exact-head tri-review 4799215165"},
{"id":"review-pr-2794-defect","patch":"pr-2794.patch","pr":"https://github.com/abhigyanpatwari/GitNexus/pull/2794","base_sha":"911151e2304f298a995fcc69c738ad2c6db9393a","head_sha":"48afb7480778ef2f5e0be455498ace889d03edf9","patch_sha256":"e2271ede4b4d65993abde16012061191a094847aadf0431a2dec908a59b6af93","human_verification_commit":"0015b0d64537c9ac97fda5ed094c99059d596cfc","label_source":"exact-head tri-review 4837878304"},
{"id":"review-pr-2108-defect","patch":"pr-2108.patch","pr":"https://github.com/abhigyanpatwari/GitNexus/pull/2108","base_sha":"3a4247ec36b5ad86b1123d3bbce8183a643f7434","head_sha":"fdabb8a1fa441b8fc7f3471121a9fa5a9d885376","patch_sha256":"12928510253fe079b179fc4658334b732de8907471ac92a6ed8061fb78aedbf2","human_verification_commit":"40dff64992ac1e72c7c042b2f508dbdc433f74dd","label_source":"exact-head tri-review 4456060714"},
{"id":"review-pr-2258-defect","patch":"pr-2258.patch","pr":"https://github.com/abhigyanpatwari/GitNexus/pull/2258","base_sha":"78b4077d8acc86f1b0c32e41012174d484e81f12","head_sha":"c93ca8ce73db7e4f0b083226a166dd9841288140","patch_sha256":"8e0eb214bd6151d1060cf2c47df7735fe5c57398e08bee9306b7b702abeca843","human_verification_commit":"00e52fa7fbae21668ae818a41f786d529c4ca390","label_source":"exact-head tri-review 4538570459"},
{"id":"review-pr-2258-clean","patch":"pr-2258b.patch","pr":"https://github.com/abhigyanpatwari/GitNexus/pull/2258","base_sha":"78b4077d8acc86f1b0c32e41012174d484e81f12","head_sha":"00e52fa7fbae21668ae818a41f786d529c4ca390","patch_sha256":"4323ec2ff01f612f18a493e055c70eb4307e9c1859b97f5b60da07b177d3a238","human_verification_commit":"00e52fa7fbae21668ae818a41f786d529c4ca390","label_source":"production-ready tri-review confirmation"},
{"id":"review-pr-2773-clean","patch":"pr-2773.patch","pr":"https://github.com/abhigyanpatwari/GitNexus/pull/2773","base_sha":"84f584449de02376a8ffc096dceac2e8f732cab5","head_sha":"f584f83bcb93c91752d91144b251f98d39027180","patch_sha256":"1c0327d4d9725428c1b49ab3586abefdbc084147735b120afd43312e8e48dd91","human_verification_commit":"f584f83bcb93c91752d91144b251f98d39027180","label_source":"review fix confirmation 3694979051"}
]
}

View file

@ -196,12 +196,21 @@ def score_review(
actual: Sequence[ReviewFinding],
expected: Sequence[ExpectedFinding],
) -> dict[str, Any]:
candidates: list[tuple[tuple[int, int, int], int, int]] = []
candidates: list[tuple[tuple[int, int, int, int, str, str, int], int, int]] = []
for actual_index, finding in enumerate(actual):
for expected_index, label in enumerate(expected):
score = _match_score(finding, label)
if score is not None:
candidates.append((score, actual_index, expected_index))
# Content keys, not list indices: JSON finding order must not
# change which pairs a greedy match commits.
expected_span = label.line_end - label.line_start
candidates.append(
(
(*score, -expected_span, finding.path, finding.category, finding.line),
actual_index,
expected_index,
)
)
candidates.sort(reverse=True)
matched_actual: set[int] = set()
matched_expected: set[int] = set()

View file

@ -413,6 +413,10 @@ def isolated_gitnexus_registry_mount(worktree: Path, parent: Path) -> ReadOnlyMo
return ReadOnlyMount(source=registry, target=SANDBOX_GITNEXUS_REGISTRY)
def _unchanged(value: Any) -> Any:
return value
def run_arm(
arm: str,
task: dict[str, Any],
@ -429,7 +433,7 @@ def run_arm(
) -> dict[str, Any]:
sessions: list[dict[str, Any]] = []
environment_builder = getattr(sandbox, "environment", build_sandbox_environment)
host_text = getattr(sandbox, "host_text", lambda value: value)
host_text = getattr(sandbox, "host_text", _unchanged)
host_path = getattr(sandbox, "host_path", lambda value: str(value))
backend = getattr(sandbox, "backend", "bwrap")
env = model_session_environment(
@ -711,7 +715,9 @@ CHURN_FIELDS = ("diff_files", "diff_insertions", "diff_deletions")
# Rows where the session (or the harness) died carry no measured evidence and
# must not skew efficiency medians or resolve denominators. verify-failed and
# skill-not-invoked rows DO count: those sessions ran and spent real tokens.
EXCLUDED_ERROR_KINDS = frozenset({"session-error", "infra-error", "evidence-unverified", "cleanup-failure"})
EXCLUDED_ERROR_KINDS = frozenset(
{"session-error", "infra-error", "evidence-unverified", "cleanup-failure", "review-evidence-invalid"}
)
# A sustained upstream outage shows up as a run of session/infra/cleanup
# failures. (cleanup-failure overwrites the primary error_kind, so a

View file

@ -60,10 +60,12 @@ WORKSPACE_SNAPSHOT_BOOTSTRAP_NOISE = frozenset(
"package-lock.json",
"package.json",
"pnpm-lock.yaml",
"scripts",
"yarn.lock",
}
)
# Claude Code may drop a workspace-root `scripts` *file* during bootstrap.
# Only that exact entry is noise — a `scripts/` directory is real workspace.
WORKSPACE_SNAPSHOT_ROOT_FILE_NOISE = frozenset({"scripts"})
# The set above is matched at the workspace ROOT only, because most of its
# entries (package.json, node_modules, the .env family) are also legitimate
@ -120,12 +122,18 @@ class VerificationResult:
yield self.output
def _is_bootstrap_noise(relative: PurePosixPath) -> bool:
def _is_bootstrap_noise(relative: PurePosixPath, *, is_dir: bool = False) -> bool:
"""Report whether a walked entry is harness noise rather than workspace change."""
parts = relative.parts
if parts[0] == ".git" or parts[0] in WORKSPACE_SNAPSHOT_BOOTSTRAP_NOISE:
return True
if (
not is_dir
and len(parts) == 1
and parts[0] in WORKSPACE_SNAPSHOT_ROOT_FILE_NOISE
):
return True
return len(parts) >= 2 and parts[-2] == CLAUDE_BOOTSTRAP_DIR and parts[-1] in CLAUDE_BOOTSTRAP_ENTRIES
@ -153,7 +161,7 @@ def workspace_snapshot(worktree: Path) -> dict[str, str]:
raise ValueError(f"workspace snapshot directory is unreadable: {directory}: {exc}") from exc
for entry in children:
relative = relative_dir / entry.name
if _is_bootstrap_noise(relative):
if _is_bootstrap_noise(relative, is_dir=entry.is_dir(follow_symlinks=False)):
continue
entry_count += 1
path_bytes += len(relative.as_posix().encode())

View file

@ -150,11 +150,13 @@ class SessionProgress:
self._tools = 0
self._pending_tools: dict[str, str] = {}
self._last_activity = "starting"
self._pending_messages: list[str] = []
self._timer: threading.Thread | None = None
self._done = threading.Event()
def __enter__(self) -> SessionProgress:
self._say(f"started (heartbeat every {self.heartbeat_s:g}s)")
self._emit_pending()
self._timer = threading.Thread(target=self._heartbeat, daemon=True)
self._timer.start()
return self
@ -164,29 +166,39 @@ class SessionProgress:
timer, self._timer = self._timer, None
if timer is not None:
timer.join(timeout=2)
self._emit_pending()
def _elapsed(self) -> str:
seconds = int(time.monotonic() - self._started)
return f"{seconds // 60}m{seconds % 60:02d}s"
def _say(self, message: str) -> None:
try:
print(f"[{self.label} {self._elapsed()}] {message}", file=self._stream, flush=True)
except (OSError, ValueError):
return
# Queue only: the stdout drain thread calls observe() and must not
# block on a full log pipe (process_control.stdout_observer contract).
self._pending_messages.append(f"[{self.label} {self._elapsed()}] {message}")
self._last_spoke = time.monotonic()
def _emit_pending(self) -> None:
with self._lock:
messages = list(self._pending_messages)
self._pending_messages.clear()
for message in messages:
try:
print(message, file=self._stream, flush=True)
except (OSError, ValueError):
return
def _heartbeat(self) -> None:
tick = min(1.0, max(self.heartbeat_s / 2, 0.01))
while not self._done.wait(tick):
with self._lock:
quiet = time.monotonic() - self._last_spoke
if quiet < self.heartbeat_s:
continue
self._say(
f"still running · {self._events} events · {self._turns} turns · "
f"{self._tools} tool calls · last: {self._last_activity}"
)
if quiet >= self.heartbeat_s:
self._say(
f"still running · {self._events} events · {self._turns} turns · "
f"{self._tools} tool calls · last: {self._last_activity}"
)
self._emit_pending()
def observe(self, chunk: bytes) -> None:
"""Consume one stdout chunk. Never raises; never blocks on I/O."""

View file

@ -246,6 +246,21 @@ def trusted_gitnexus_runtime_mounts() -> tuple[ReadOnlyMount, ...]:
HARNESS_ROOT / "gitnexus-shared",
label="pinned GitNexus shared runtime",
)
node_modules = _validated_runtime_component(
runtime,
"node_modules",
f"{SANDBOX_GITNEXUS}/node_modules",
directory=True,
allow_primary_worktree_symlink=True,
)
primary = _primary_checkout_root(HARNESS_ROOT)
if primary is not None and node_modules.source == (primary / "gitnexus" / "node_modules"):
# Reused primary node_modules still symlink to that checkout's shared
# package. Mount the same tree or the sandbox inner link is a host path.
shared = _validated_runtime_root(
primary / "gitnexus-shared",
label="primary GitNexus shared runtime",
)
mounts = (
_validated_runtime_component(
runtime,
@ -259,13 +274,7 @@ def trusted_gitnexus_runtime_mounts() -> tuple[ReadOnlyMount, ...]:
f"{SANDBOX_GITNEXUS}/package.json",
directory=False,
),
_validated_runtime_component(
runtime,
"node_modules",
f"{SANDBOX_GITNEXUS}/node_modules",
directory=True,
allow_primary_worktree_symlink=True,
),
node_modules,
_validated_runtime_component(
runtime,
"vendor",
@ -305,7 +314,6 @@ def trusted_gitnexus_runtime_mounts() -> tuple[ReadOnlyMount, ...]:
linked_shared = mounts[2].source / "gitnexus-shared"
allowed_shared = {shared}
primary = _primary_checkout_root(HARNESS_ROOT)
if primary is not None:
try:
allowed_shared.add(
@ -315,6 +323,7 @@ def trusted_gitnexus_runtime_mounts() -> tuple[ReadOnlyMount, ...]:
)
)
except SandboxError:
# Primary checkout may be absent or unreadable on a standalone eval tree.
pass
if not linked_shared.is_symlink() or linked_shared.resolve(strict=True) not in allowed_shared:
raise SandboxError("pinned GitNexus runtime has an unexpected gitnexus-shared dependency")

View file

@ -6,8 +6,8 @@ tasks:
class: review-defect
repo: ~/GitNexus
ref: ff86ccf1e79cd7e4175da437ae8aeaf67b64aaa1
sandbox_copy: [eval/workflow_bench/review_cases/pr-2718-defect.patch]
setup: git apply --exclude='eval/workflow_bench/*' eval/workflow_bench/review_cases/pr-2718-defect.patch && rm -rf eval/workflow_bench
sandbox_copy: [eval/workflow_bench/review_cases/pr-2718.patch]
setup: git apply --exclude='eval/workflow_bench/*' eval/workflow_bench/review_cases/pr-2718.patch && rm -rf eval/workflow_bench
prompt: Review the historical snapshot of https://github.com/abhigyanpatwari/GitNexus/pull/2718. Report only actionable defects introduced by the local diff.
verify: test -s review-output.json
oracle:
@ -21,8 +21,8 @@ tasks:
- <<: *review_case
id: review-pr-2794-defect
ref: 911151e2304f298a995fcc69c738ad2c6db9393a
sandbox_copy: [eval/workflow_bench/review_cases/pr-2794-defect.patch]
setup: git apply --exclude='eval/workflow_bench/*' eval/workflow_bench/review_cases/pr-2794-defect.patch && rm -rf eval/workflow_bench
sandbox_copy: [eval/workflow_bench/review_cases/pr-2794.patch]
setup: git apply --exclude='eval/workflow_bench/*' eval/workflow_bench/review_cases/pr-2794.patch && rm -rf eval/workflow_bench
prompt: Review the historical snapshot of https://github.com/abhigyanpatwari/GitNexus/pull/2794. Report only actionable defects introduced by the local diff.
oracle:
command: test -s review-output.json
@ -32,8 +32,8 @@ tasks:
- <<: *review_case
id: review-pr-2108-defect
ref: 3a4247ec36b5ad86b1123d3bbce8183a643f7434
sandbox_copy: [eval/workflow_bench/review_cases/pr-2108-defect.patch]
setup: git apply --exclude='eval/workflow_bench/*' eval/workflow_bench/review_cases/pr-2108-defect.patch && rm -rf eval/workflow_bench
sandbox_copy: [eval/workflow_bench/review_cases/pr-2108.patch]
setup: git apply --exclude='eval/workflow_bench/*' eval/workflow_bench/review_cases/pr-2108.patch && rm -rf eval/workflow_bench
prompt: Review the historical snapshot of https://github.com/abhigyanpatwari/GitNexus/pull/2108. Report only actionable defects introduced by the local diff.
oracle:
command: test -s review-output.json
@ -43,8 +43,8 @@ tasks:
- <<: *review_case
id: review-pr-2258-defect
ref: 78b4077d8acc86f1b0c32e41012174d484e81f12
sandbox_copy: [eval/workflow_bench/review_cases/pr-2258-defect.patch]
setup: git apply --exclude='eval/workflow_bench/*' eval/workflow_bench/review_cases/pr-2258-defect.patch && rm -rf eval/workflow_bench
sandbox_copy: [eval/workflow_bench/review_cases/pr-2258.patch]
setup: git apply --exclude='eval/workflow_bench/*' eval/workflow_bench/review_cases/pr-2258.patch && rm -rf eval/workflow_bench
prompt: Review the historical snapshot of https://github.com/abhigyanpatwari/GitNexus/pull/2258. Report only actionable defects introduced by the local diff.
oracle:
command: test -s review-output.json
@ -55,8 +55,8 @@ tasks:
id: review-pr-2258-clean
class: review-clean
ref: 78b4077d8acc86f1b0c32e41012174d484e81f12
sandbox_copy: [eval/workflow_bench/review_cases/pr-2258-clean.patch]
setup: git apply --exclude='eval/workflow_bench/*' eval/workflow_bench/review_cases/pr-2258-clean.patch && rm -rf eval/workflow_bench
sandbox_copy: [eval/workflow_bench/review_cases/pr-2258b.patch]
setup: git apply --exclude='eval/workflow_bench/*' eval/workflow_bench/review_cases/pr-2258b.patch && rm -rf eval/workflow_bench
prompt: Review this historical snapshot of https://github.com/abhigyanpatwari/GitNexus/pull/2258. Report only actionable defects introduced by the local diff.
oracle:
command: test -s review-output.json
@ -67,8 +67,8 @@ tasks:
id: review-pr-2773-clean
class: review-clean
ref: 84f584449de02376a8ffc096dceac2e8f732cab5
sandbox_copy: [eval/workflow_bench/review_cases/pr-2773-clean.patch]
setup: git apply --exclude='eval/workflow_bench/*' eval/workflow_bench/review_cases/pr-2773-clean.patch && rm -rf eval/workflow_bench
sandbox_copy: [eval/workflow_bench/review_cases/pr-2773.patch]
setup: git apply --exclude='eval/workflow_bench/*' eval/workflow_bench/review_cases/pr-2773.patch && rm -rf eval/workflow_bench
prompt: Review this historical snapshot of https://github.com/abhigyanpatwari/GitNexus/pull/2773. Report only actionable defects introduced by the local diff.
oracle:
command: test -s review-output.json