Address PR review feedback (#2785)

Close follow-up holes in host write locks, preview redaction, runtime mounts, and review matching.

Co-authored-by: Cursor <cursoragent@cursor.com>

eval/tests/test_evolve.py

@@ -576,6 +576,7 @@ def test_run_proposer_hides_the_hidden_harness_and_keeps_the_full_tool_surface(m
     transcript_projects.mkdir()
     captured: dict[str, object] = {}
     sanitized: list[Path] = []
+    events: list[str] = []
 
     def fake_make_worktree(_repo, _ref, destination):
         clone = destination / "clone"
@@ -583,6 +584,7 @@ def test_run_proposer_hides_the_hidden_harness_and_keeps_the_full_tool_surface(m
         return clone
 
     def fake_sanitize(clone):
+        events.append("sanitize")
         sanitized.append(clone)
         return "0" * 40
 
@@ -597,6 +599,7 @@ def test_run_proposer_hides_the_hidden_harness_and_keeps_the_full_tool_surface(m
 
     @contextmanager
     def fake_prepare_sandbox(**_kwargs):
+        events.append("prepare")
         yield FakeSandbox()
 
     def fake_run_claude(*_args, **kwargs):
@@ -622,6 +625,7 @@ def test_run_proposer_hides_the_hidden_harness_and_keeps_the_full_tool_surface(m
     assert record["ok"] is False
     # Sanitization has to happen on the clone the session actually runs in,
     # and before the sandbox is prepared around it.
+    assert events[:2] == ["sanitize", "prepare"]
     assert [clone.name for clone in sanitized] == ["clone"]
     # Not --bare: bare ignores --tools and would cost the proposer Grep/Glob.
     assert captured.get("bare", False) is False

eval/tests/test_proposer_sandbox.py

@@ -143,6 +143,25 @@ 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_keeps_files_under_an_allowed_directory(tmp_path) -> None:
+    clone = tmp_path / "clone"
+    artifacts = clone / "artifacts"
+    artifacts.mkdir(parents=True)
+    existing = artifacts / "review-output.json"
+    existing.write_text("{}\n")
+    (clone / "src").mkdir()
+    locked = clone / "src" / "source.ts"
+    locked.write_text("trusted\n")
+
+    with host_workspace_write_boundary(clone, writable=(artifacts,)):
+        existing.write_text('{"schema_version":1}\n')
+        with pytest.raises(OSError):
+            locked.write_text("tampered\n")
+
+    assert existing.read_text() == '{"schema_version":1}\n'
+    assert locked.read_text() == "trusted\n"
+
+
 def test_host_workspace_write_boundary_rejects_a_symlinked_writable_artifact(tmp_path) -> None:
     clone = tmp_path / "clone"
     clone.mkdir()

eval/tests/test_review_scoring.py

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

eval/tests/test_runner_hardening.py

@@ -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_records_a_root_scripts_symlink(tmp_path):
+    before = runner_artifacts.workspace_snapshot(tmp_path)
+    target = tmp_path / "helper.py"
+    target.write_text("planted\n")
+    (tmp_path / "scripts").symlink_to(target)
+    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_writes_under_scripts(tmp_path):
     scripts = tmp_path / "scripts"
     scripts.mkdir()

eval/tests/test_session_progress.py

@@ -233,6 +233,26 @@ def test_progress_sanitizes_a_hostile_tool_name() -> None:
     assert len(_drain_lines(stream)) == 1
 
 
+def test_progress_redacts_non_ascii_secrets_before_json_escaping() -> None:
+    stream = io.StringIO()
+    secret = "tokén-密码"
+    progress = SessionProgress("flow", stream=stream, heartbeat_s=3600, secrets=(secret,))
+    event = {
+        "type": "assistant",
+        "message": {
+            "content": [
+                {"type": "tool_use", "id": "t1", "name": "Grep", "input": {"token": secret}},
+            ]
+        },
+    }
+    _observe(progress, (json.dumps(event, ensure_ascii=False) + "\n").encode())
+
+    output = stream.getvalue()
+    assert secret not in output
+    assert json.dumps(secret)[1:-1] not in output
+    assert "[REDACTED]" in output
+
+
 def test_cell_failure_detail_line_explains_why_a_cell_failed() -> None:
     from workflow_bench.runner import cell_failure_detail_line
 

eval/tests/test_workflow_bench_sessions.py

@@ -472,6 +472,31 @@ def test_runtime_mounts_reuse_primary_checkout_node_modules_from_a_worktree(
     assert by_target[f"{runner.SANDBOX_GITNEXUS}/dist"] == worktree / "gitnexus" / "dist"
 
 
+def test_runtime_mounts_reuse_primary_shared_when_only_the_inner_link_points_there(
+    monkeypatch, tmp_path
+) -> None:
+    primary = tmp_path / "primary"
+    worktree = tmp_path / "worktree"
+    _install_pinned_runtime(primary)
+    (primary / ".git" / "worktrees" / "wt").mkdir(parents=True)
+    _install_pinned_runtime(worktree)
+    linked = worktree / "gitnexus" / "node_modules" / "gitnexus-shared"
+    linked.unlink()
+    linked.symlink_to(primary / "gitnexus-shared", target_is_directory=True)
+    (worktree / ".git").write_text(f"gitdir: {primary / '.git' / 'worktrees' / 'wt'}\n")
+    monkeypatch.setattr(runtime_mounts, "HARNESS_ROOT", worktree)
+
+    mounts = runner.trusted_gitnexus_runtime_mounts()
+    by_target = {mount.target: mount.source for mount in mounts}
+
+    assert by_target[f"{runner.SANDBOX_GITNEXUS}/node_modules"] == (
+        worktree / "gitnexus" / "node_modules"
+    )
+    assert by_target[f"{runner.SANDBOX_GITNEXUS_SHARED}/package.json"] == (
+        primary / "gitnexus-shared" / "package.json"
+    )
+
+
 def test_runtime_mounts_reject_a_node_modules_symlink_outside_the_primary_checkout(
     monkeypatch, tmp_path
 ) -> None:

eval/workflow_bench/proposer_sandbox.py

@@ -897,13 +897,15 @@ def _drop_host_workspace_write_bits(
         if stat.S_ISLNK(metadata.st_mode):
             continue
         if stat.S_ISDIR(metadata.st_mode):
+            if current in allowed:
+                # Match bwrap: a writable directory bind keeps its children writable.
+                continue
             try:
                 children = [Path(entry.path) for entry in os.scandir(current)]
             except OSError as exc:
                 raise SandboxError(f"host workspace lock directory is unreadable: {current}: {exc}") from exc
             pending.extend(children)
-            if current not in allowed:
-                os.chmod(current, 0o500)
+            os.chmod(current, 0o500)
             continue
         if current in allowed:
             os.chmod(current, stat.S_IMODE(metadata.st_mode) | 0o222)

eval/workflow_bench/review_scoring.py

@@ -191,6 +191,69 @@ def _match_score(actual: ReviewFinding, expected: ExpectedFinding) -> tuple[int,
     return category, severity, -distance
 
 
+def _greedy_pairs(
+    candidates: list[tuple[tuple[int, int, int, int, str, str, int], int, int]],
+) -> list[tuple[int, int]]:
+    matched_actual: set[int] = set()
+    matched_expected: set[int] = set()
+    pairs: list[tuple[int, int]] = []
+    for _score, actual_index, expected_index in candidates:
+        if actual_index in matched_actual or expected_index in matched_expected:
+            continue
+        matched_actual.add(actual_index)
+        matched_expected.add(expected_index)
+        pairs.append((actual_index, expected_index))
+    return pairs
+
+
+def _assign_pairs(
+    candidates: list[tuple[tuple[int, int, int, int, str, str, int], int, int]],
+) -> list[tuple[int, int]]:
+    """Maximum-cardinality assignment; remaining ties follow candidate rank."""
+
+    if not candidates:
+        return []
+    actual_ids = sorted({actual_index for _score, actual_index, _expected_index in candidates})
+    expected_ids = sorted({expected_index for _score, _actual_index, expected_index in candidates})
+    if len(actual_ids) > 16 or len(expected_ids) > 16:
+        return _greedy_pairs(candidates)
+
+    actual_pos = {actual_index: index for index, actual_index in enumerate(actual_ids)}
+    expected_pos = {expected_index: index for index, expected_index in enumerate(expected_ids)}
+    edges: dict[int, list[tuple[tuple[int, int, int, int, str, str, int], int]]] = {}
+    for score, actual_index, expected_index in candidates:
+        edges.setdefault(actual_pos[actual_index], []).append((score, expected_pos[expected_index]))
+
+    memo: dict[tuple[int, int], tuple[int, tuple, tuple[tuple[int, int], ...]]] = {}
+
+    def search(index: int, mask: int) -> tuple[int, tuple, tuple[tuple[int, int], ...]]:
+        key = (index, mask)
+        cached = memo.get(key)
+        if cached is not None:
+            return cached
+        if index == len(actual_ids):
+            empty: tuple[int, tuple, tuple[tuple[int, int], ...]] = (0, (), ())
+            memo[key] = empty
+            return empty
+        best = search(index + 1, mask)
+        for score, expected_pos_index in edges.get(index, ()):
+            bit = 1 << expected_pos_index
+            if mask & bit:
+                continue
+            card, scores, pairs = search(index + 1, mask | bit)
+            candidate = (
+                card + 1,
+                (score, *scores),
+                ((actual_ids[index], expected_ids[expected_pos_index]), *pairs),
+            )
+            if candidate[0] > best[0] or (candidate[0] == best[0] and candidate[1] > best[1]):
+                best = candidate
+        memo[key] = best
+        return best
+
+    return list(search(0, 0)[2])
+
+
 def score_review(
     verdict: str,
     actual: Sequence[ReviewFinding],
@@ -212,15 +275,9 @@ def score_review(
                     )
                 )
     candidates.sort(reverse=True)
-    matched_actual: set[int] = set()
-    matched_expected: set[int] = set()
-    pairs: list[tuple[int, int]] = []
-    for _score, actual_index, expected_index in candidates:
-        if actual_index in matched_actual or expected_index in matched_expected:
-            continue
-        matched_actual.add(actual_index)
-        matched_expected.add(expected_index)
-        pairs.append((actual_index, expected_index))
+    pairs = _assign_pairs(candidates)
+    matched_actual = {actual_index for actual_index, _expected_index in pairs}
+    matched_expected = {expected_index for _actual_index, expected_index in pairs}
 
     tp = len(pairs)
     fp = len(actual) - tp

eval/workflow_bench/runner_artifacts.py

@@ -122,14 +122,20 @@ class VerificationResult:
         yield self.output
 
 
-def _is_bootstrap_noise(relative: PurePosixPath, *, is_dir: bool = False) -> bool:
+def _is_bootstrap_noise(
+    relative: PurePosixPath,
+    *,
+    is_dir: bool = False,
+    is_file: 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
+        is_file
+        and not is_dir
         and len(parts) == 1
         and parts[0] in WORKSPACE_SNAPSHOT_ROOT_FILE_NOISE
     ):
@@ -161,7 +167,11 @@ 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, is_dir=entry.is_dir(follow_symlinks=False)):
+            if _is_bootstrap_noise(
+                relative,
+                is_dir=entry.is_dir(follow_symlinks=False),
+                is_file=entry.is_file(follow_symlinks=False),
+            ):
                 continue
             entry_count += 1
             path_bytes += len(relative.as_posix().encode())

eval/workflow_bench/runner_sessions.py

@@ -71,9 +71,9 @@ def _tool_preview(value: Any, secrets: Sequence[str]) -> str:
     """Render one bounded, redacted, single-line tool payload preview."""
 
     try:
-        raw = json.dumps(value, ensure_ascii=True, separators=(",", ":"), default=str)
+        raw = json.dumps(value, ensure_ascii=False, separators=(",", ":"), default=str)
     except (TypeError, ValueError):
-        raw = json.dumps(str(value), ensure_ascii=True)
+        raw = json.dumps(str(value), ensure_ascii=False)
     redacted = redact_text(raw, secrets)
     if len(redacted) <= MAX_TOOL_PREVIEW_CHARS:
         return redacted

eval/workflow_bench/runtime_mounts.py

@@ -254,13 +254,30 @@ def trusted_gitnexus_runtime_mounts() -> tuple[ReadOnlyMount, ...]:
         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",
-        )
+    if primary is not None:
+        try:
+            primary_shared = _validated_runtime_root(
+                primary / "gitnexus-shared",
+                label="primary GitNexus shared runtime",
+            )
+        except SandboxError:
+            primary_shared = None
+        else:
+            reuse_primary_shared = node_modules.source == (primary / "gitnexus" / "node_modules")
+            if not reuse_primary_shared:
+                linked_shared = node_modules.source / "gitnexus-shared"
+                try:
+                    reuse_primary_shared = (
+                        linked_shared.is_symlink()
+                        and linked_shared.resolve(strict=True) == primary_shared
+                    )
+                except OSError:
+                    reuse_primary_shared = False
+            if reuse_primary_shared:
+                # Reused primary node_modules (or its inner gitnexus-shared
+                # link) still points at that checkout. Mount the same tree or
+                # the sandbox inner link is a host path.
+                shared = primary_shared
     mounts = (
         _validated_runtime_component(
             runtime,