From b57e42d5481021b63481658f09f656017d265496 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 13:12:14 +0000 Subject: [PATCH 01/26] fix(workbench): bind finding excerpts to selected source paths --- .../_bundled_plugin/.codex-plugin/plugin.json | 2 +- .../scripts/deep_scan_workbench.py | 11 +- .../_bundled_plugin/scripts/workbench_db.py | 12 +- .../scripts/workbench_scan_start.py | 13 +- .../scripts/workbench_schema.py | 7 + .../scripts/workbench_source_excerpt.py | 361 +++++++++-- sdk/typescript/src/version.ts | 2 +- sdk/typescript/tests-ts/cost.test.ts | 4 +- .../tests-ts/diff-rank-input.test.ts | 2 +- .../tests-ts/plugin-report-limits.test.ts | 4 +- .../tests-ts/publication-store.test.ts | 12 +- .../tests-ts/workbench-source-excerpt.test.ts | 604 ++++++++++++++++++ 12 files changed, 975 insertions(+), 59 deletions(-) create mode 100644 sdk/typescript/tests-ts/workbench-source-excerpt.test.ts diff --git a/sdk/typescript/_bundled_plugin/.codex-plugin/plugin.json b/sdk/typescript/_bundled_plugin/.codex-plugin/plugin.json index 7df3d5586..4993c3c1f 100644 --- a/sdk/typescript/_bundled_plugin/.codex-plugin/plugin.json +++ b/sdk/typescript/_bundled_plugin/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "codex-security", - "version": "0.1.27", + "version": "0.1.28", "description": "Codex Security workflows for security scans, analysis, and investigation.", "author": { "name": "OpenAI" diff --git a/sdk/typescript/_bundled_plugin/scripts/deep_scan_workbench.py b/sdk/typescript/_bundled_plugin/scripts/deep_scan_workbench.py index a9de0bffe..c232c5481 100644 --- a/sdk/typescript/_bundled_plugin/scripts/deep_scan_workbench.py +++ b/sdk/typescript/_bundled_plugin/scripts/deep_scan_workbench.py @@ -21,6 +21,7 @@ from filesystem_identity import serialize_filesystem_identity from finalize_scan_contract import _read_scan_local_json from workbench.handoff import require_current_continuation +from workbench_source_excerpt import capture_source_scopes from workbench_target import ( directory_content_digest, directory_snapshot_regular_file_count, @@ -841,6 +842,11 @@ def begin_deep_scan_for_target( scan_id = str(uuid.uuid4()) timestamp = now() target_id = ensure_security_target(connection, target_path) + source_scopes = capture_source_scopes( + target, + (revision, target_snapshot_digest, target_device, target_inode), + [scope], + ) scan_dir = Path( tempfile.mkdtemp( prefix=f"{safe_segment(revision)}_{compact_timestamp()}_", @@ -870,10 +876,10 @@ def begin_deep_scan_for_target( """ INSERT INTO scans ( id, workspace_id, target_id, target_path, target_revision, target_snapshot_digest, - target_device, target_inode, scope, mode, user_context, + target_device, target_inode, source_scopes_json, scope, mode, user_context, deep_scan_owner_thread_id, scan_dir, model, reasoning_effort, status, phase, handoff_status, started_at, created_at, updated_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 'deep', ?, ?, ?, ?, ?, + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 'deep', ?, ?, ?, ?, ?, 'running', 'preflight', 'delivered', ?, ?, ?) """, ( @@ -885,6 +891,7 @@ def begin_deep_scan_for_target( target_snapshot_digest, target_device, target_inode, + json.dumps(source_scopes, allow_nan=False, separators=(",", ":"), sort_keys=True), scope, user_context, thread_id, diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py index 5c6481410..08abf34a0 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py @@ -2394,6 +2394,7 @@ def target_matches_initial_snapshot() -> bool: timestamp=timestamp, handoff_status="delivered", scan_dir=scan_dir, + source_paths=paths or ["."], ) connection.execute( "UPDATE scans SET recipe_json = ?, parent_scan_id = ?, user_context = ? WHERE id = ?", @@ -4111,6 +4112,7 @@ def finding_result( confidence = confidence if isinstance(confidence, dict) else {} severity = details.get("severity") severity = severity if isinstance(severity, dict) else {} + excerpt_locations = [] locations = [] try: target = require_scan_target_identity(scan) @@ -4126,6 +4128,14 @@ def finding_result( """, (occurrence["id"], FINDING_LOCATIONS_LIMIT), ): + excerpt_locations.append( + { + "endLine": row["end_line"], + "path": row["relative_path"], + "role": row["role"], + "startLine": row["start_line"], + } + ) absolute_path = safe_source_path(target, row["relative_path"]) if target else None location = { "endLine": row["end_line"], @@ -4170,7 +4180,7 @@ def finding_result( result["knownSince"] = known_since result["knownScanIds"] = known_scan_ids result.pop("artifactPaths", None) - source_excerpt = finding_source_excerpt(scan, target, locations) + source_excerpt = finding_source_excerpt(scan, target, excerpt_locations) if source_excerpt: result["sourceExcerpt"] = source_excerpt artifact_paths = finding_artifact_paths(Path(scan["scan_dir"]), details) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_scan_start.py b/sdk/typescript/_bundled_plugin/scripts/workbench_scan_start.py index 7cb1f6217..0aa4dfd15 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_scan_start.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_scan_start.py @@ -17,6 +17,7 @@ from filesystem_identity import serialize_filesystem_identity from finalize_scan_contract import write_scan_local_bytes from workbench_feedback import get_scan_feedback +from workbench_source_excerpt import capture_source_scopes from workbench_target import ( directory_content_digest, git_revision, @@ -165,6 +166,7 @@ def insert_running_scan( model: str | None = None, reasoning_effort: str | None = None, scan_dir: Path | None = None, + source_paths: list[str] | None = None, ) -> str: revision = target_identity[0] native_scan = scan_dir is None @@ -176,15 +178,21 @@ def insert_running_scan( dir=target_root, ) ).resolve() + source_scopes = capture_source_scopes( + target, + target_identity, + source_paths or [scope], + diff_target_kind=diff_target["kind"] if diff_target is not None else None, + ) connection.execute( """ INSERT INTO scans ( id, workspace_id, target_id, target_path, target_revision, target_snapshot_digest, - target_device, target_inode, scope, mode, user_context, + target_device, target_inode, source_scopes_json, scope, mode, user_context, deep_scan_owner_thread_id, diff_target_kind, diff_base_revision, diff_head_revision, diff_content_digest, target_summary, scan_dir, model, reasoning_effort, status, phase, handoff_status, started_at, created_at, updated_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 'running', 'preflight', ?, ?, ?, ?) """, ( @@ -193,6 +201,7 @@ def insert_running_scan( workspace["target_id"], str(target), *target_identity, + json.dumps(source_scopes, allow_nan=False, separators=(",", ":"), sort_keys=True), scope, workspace["default_mode"], user_context, diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_schema.py b/sdk/typescript/_bundled_plugin/scripts/workbench_schema.py index 90111fdd7..c431e45c0 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_schema.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_schema.py @@ -697,6 +697,13 @@ ALTER TABLE deep_scan_runs ADD COLUMN publication_error_message TEXT; """, ), + ( + 33, + "persist selected source excerpt authority", + """ + ALTER TABLE scans ADD COLUMN source_scopes_json TEXT; + """, + ), ) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index cea25e23f..7b3b8933f 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -3,18 +3,281 @@ from __future__ import annotations import argparse +import json +import os +import re import sqlite3 +import stat import sys +from functools import cache from pathlib import Path, PurePosixPath from typing import Any +from unicodedata import normalize # Some plugin hosts launch Python with safe-path isolation enabled. sys.path.insert(0, str(Path(__file__).resolve().parent)) -from workbench_target import clean_worktree_content_digest, git_bytes +from workbench_target import ( + clean_worktree_content_digest, + git_bytes, + git_worktree_context, +) CONTEXT_LINES = 3 MAX_BYTES = 16_000 MAX_LINES = 60 +OBJECT_ID = re.compile(r"(?:[0-9a-f]{40}|[0-9a-f]{64})\Z") + +TreeEntry = tuple[str, str, str] +SourceScope = dict[str, str] + + +def normalized_path_component(value: str) -> str: + return normalize("NFC", normalize("NFD", value).casefold()) + + +def relative_path(value: str) -> PurePosixPath | None: + path = PurePosixPath(value) + if not value or "\\" in value or path.is_absolute() or ".." in path.parts: + return None + return path + + +def safe_source_path(target: Path, value: str) -> Path | None: + path = relative_path(value) + if path is None: + return None + try: + root = target.resolve() + selected = (root / path.as_posix()).resolve() + selected.relative_to(root) + return selected + except (OSError, RuntimeError, ValueError): + return None + + +def local_git_bytes(repository: Path, *arguments: str) -> bytes | None: + return git_bytes( + repository, + "--no-replace-objects", + *arguments, + local_objects_only=True, + ) + + +@cache +def tree_entries(repository: Path, tree: str) -> dict[str, tuple[TreeEntry, ...]] | None: + content = local_git_bytes(repository, "ls-tree", "-z", tree) + if content is None: + return None + entries: dict[str, list[TreeEntry]] = {} + for record in content.split(b"\0"): + if not record: + continue + metadata, separator, name = record.partition(b"\t") + fields = metadata.split(b" ") + if not separator or len(fields) != 3: + return None + mode, object_type, raw_object = fields + try: + object_id = raw_object.decode("ascii") + except UnicodeDecodeError: + return None + if not OBJECT_ID.fullmatch(object_id): + return None + kind = ( + "directory" + if mode == b"040000" and object_type == b"tree" + else "file" + if mode in {b"100644", b"100755"} and object_type == b"blob" + else "other" + ) + decoded_name = os.fsdecode(name) + entries.setdefault(normalized_path_component(decoded_name), []).append( + (decoded_name, kind, object_id) + ) + return {name: tuple(matches) for name, matches in entries.items()} + + +def tree_path(repository: Path, tree: str, value: str) -> TreeEntry | None: + path = relative_path(value) + if path is None: + return None + kind, object_id = "directory", tree + for name in path.parts: + if kind != "directory": + return None + aliases = (tree_entries(repository, object_id) or {}).get( + normalized_path_component(name), () + ) + # The normalized name must be unique before an exact spelling can win. + if len(aliases) != 1: + return None + entry = next((candidate for candidate in aliases if candidate[0] == name), None) + if entry is None: + return None + _, kind, object_id = entry + return path.as_posix(), kind, object_id + + +def target_tree(target: Path, revision: str) -> tuple[Path, str] | None: + if not OBJECT_ID.fullmatch(revision): + return None + repository, prefix = git_worktree_context(target) + if local_git_bytes(repository, "replace", "--list") != b"": + return None + raw_tree = local_git_bytes( + repository, + "rev-parse", + "--verify", + "--end-of-options", + f"{revision}^{{tree}}", + ) + try: + tree = raw_tree.decode("ascii").strip() if raw_tree is not None else "" + except UnicodeDecodeError: + return None + if not OBJECT_ID.fullmatch(tree): + return None + if prefix == ".": + return repository, tree + selected = tree_path(repository, tree, prefix) + if selected is None or selected[1] != "directory": + return None + return repository, selected[2] + + +def capture_source_scopes( + target: Path, + target_identity: tuple[str, str | None, int | str, int | str], + paths: list[str], + *, + diff_target_kind: str | None = None, +) -> dict[str, Any]: + revision, snapshot = target_identity[:2] + authority: dict[str, Any] = {"version": 1, "revision": revision, "scopes": []} + if ( + (diff_target_kind is not None and diff_target_kind not in {"commit", "range"}) + or revision == "unversioned" + or (snapshot is not None and snapshot != clean_worktree_content_digest()) + ): + return authority + try: + context = target_tree(target, revision) + if context is None: + return authority + repository, tree = context + authority["targetTree"] = tree + captured: set[tuple[str, str, str]] = set() + for requested in paths: + parsed = relative_path(requested) + selected_path = safe_source_path(target, requested) + if parsed is None or selected_path is None: + continue + entry = tree_path(repository, tree, requested) + if entry is None or entry[1] not in {"file", "directory"}: + continue + raw_selected = target / parsed.as_posix() + try: + metadata = raw_selected.lstat() + except OSError: + continue + ordinary = stat.S_ISDIR(metadata.st_mode) or stat.S_ISREG(metadata.st_mode) + if not ordinary or (entry[1] == "directory") != stat.S_ISDIR( + metadata.st_mode + ): + continue + scope = { + "path": parsed.as_posix(), + "kind": entry[1], + "objectId": entry[2], + } + key = tuple(scope.values()) + if key not in captured: + captured.add(key) + authority["scopes"].append(scope) + except (OSError, RuntimeError, SystemExit, UnicodeError, ValueError): + return {"version": 1, "revision": revision, "scopes": []} + return authority + + +def load_source_scopes( + scan: sqlite3.Row, target: Path +) -> tuple[Path, tuple[SourceScope, ...]] | None: + try: + saved = scan["source_scopes_json"] + except (IndexError, KeyError): + return None + if not isinstance(saved, str): + return None + metadata = json.loads(saved) + tree = metadata.get("targetTree") if isinstance(metadata, dict) else None + records = metadata.get("scopes") if isinstance(metadata, dict) else None + if ( + not isinstance(metadata, dict) + or metadata.get("version") != 1 + or metadata.get("revision") != scan["target_revision"] + or not isinstance(tree, str) + or not OBJECT_ID.fullmatch(tree) + or not isinstance(records, list) + or not records + ): + return None + context = target_tree(target, scan["target_revision"]) + if context is None or context[1] != tree: + return None + repository, _ = context + scopes: list[SourceScope] = [] + for record in records: + if not isinstance(record, dict): + return None + path = record.get("path") + kind = record.get("kind") + object_id = record.get("objectId") + requested_path = relative_path(path) if isinstance(path, str) else None + if ( + requested_path is None + or kind not in {"file", "directory"} + or not isinstance(object_id, str) + or not OBJECT_ID.fullmatch(object_id) + ): + return None + scope = { + "path": path, + "kind": kind, + "objectId": object_id, + } + if tree_path(repository, tree, path) != (path, kind, object_id): + return None + scopes.append(scope) + return repository, tuple(scopes) + + +def source_object_for_path( + repository: Path, + target: Path, + value: str, + scope: SourceScope, +) -> str | None: + path = relative_path(value) + if path is None or safe_source_path(target, value) is None: + return None + scope_path = PurePosixPath(scope["path"]) + scope_length = len(scope_path.parts) + if len(path.parts) < scope_length: + return None + if path.parts[:scope_length] != scope_path.parts: + return None + suffix = path.parts[scope_length:] + if scope["kind"] == "file": + return scope["objectId"] if not suffix else None + if not suffix: + return None + entry = tree_path( + repository, + scope["objectId"], + PurePosixPath(*suffix).as_posix(), + ) + return entry[2] if entry is not None and entry[1] == "file" else None def finding_source_excerpt( @@ -22,27 +285,62 @@ def finding_source_excerpt( target: Path | None, locations: list[dict[str, Any]], ) -> str | None: - if target is None or not locations: - return None - location = next( - ( - candidate - for candidate in locations - if "root_control" in str(candidate.get("role") or "").lower() - ), - locations[0], - ) - path = location.get("path") - start_line = location.get("startLine") - end_line = location.get("endLine") - if not isinstance(path, str) or not isinstance(start_line, int): + if scan["mode"] == "diff" and scan["diff_target_kind"] not in {"commit", "range"}: + return None + if target is None or not locations or scan["target_revision"] == "unversioned": + return None + snapshot = scan["target_snapshot_digest"] + if snapshot is not None and snapshot != clean_worktree_content_digest(): return None - source = scanned_source_text(scan, target, path) + try: + context = load_source_scopes(scan, target) + except (OSError, RuntimeError, SystemExit, UnicodeError, ValueError): + return None + if context is None: + return None + repository, scopes = context + + def priority(location: dict[str, Any]) -> int: + role = location.get("role") + if role == "root_control": + return 0 + return 1 if "root_control" in str(role or "").lower() else 2 + + selected_location = None + object_id = None + for location in sorted(locations, key=priority): + path = location.get("path") + if not isinstance(path, str) or not isinstance(location.get("startLine"), int): + continue + try: + object_id = next( + ( + candidate + for scope in scopes + if ( + candidate := source_object_for_path( + repository, target, path, scope + ) + ) + is not None + ), + None, + ) + except (OSError, RuntimeError, SystemExit, UnicodeError, ValueError): + object_id = None + if object_id is not None: + selected_location = location + break + if selected_location is None or object_id is None: + return None + source = scanned_source_text(repository, object_id) if not source or "\0" in source: return None + start_line = selected_location["startLine"] lines = source.splitlines() if start_line < 1 or start_line > len(lines): return None + end_line = selected_location.get("endLine") last_affected_line = end_line if isinstance(end_line, int) else start_line excerpt_start = max(1, start_line - CONTEXT_LINES) excerpt_end = min( @@ -55,36 +353,15 @@ def finding_source_excerpt( f"{line_number:>{width}} {lines[line_number - 1]}" for line_number in range(excerpt_start, excerpt_end + 1) ) - encoded = excerpt.encode("utf-8")[:MAX_BYTES] - return encoded.decode("utf-8", errors="ignore") + return excerpt.encode("utf-8")[:MAX_BYTES].decode("utf-8", errors="ignore") -def scanned_source_text(scan: sqlite3.Row, target: Path, path: str) -> str | None: - if safe_source_path(target, path) is None: - return None - revision = scan["target_revision"] - if revision == "unversioned": - return None - snapshot_digest = scan["target_snapshot_digest"] - if snapshot_digest is not None and snapshot_digest != clean_worktree_content_digest(): - return None - object_name = f"{revision}:{path}" - content = git_bytes(target, "cat-file", "blob", object_name) - return content.decode("utf-8", errors="replace") if content is not None else None - - -def safe_source_path(target: Path, relative_path: str) -> Path | None: - if "\\" in relative_path: - return None - parsed = PurePosixPath(relative_path) - if parsed.is_absolute() or ".." in parsed.parts: - return None +def scanned_source_text(repository: Path, object_id: str) -> str | None: try: - path = (target / parsed.as_posix()).resolve() - path.relative_to(target) - except (OSError, RuntimeError, ValueError): + content = local_git_bytes(repository, "cat-file", "blob", object_id) + except (OSError, RuntimeError, SystemExit): return None - return path + return content.decode("utf-8", errors="replace") if content is not None else None def main() -> None: diff --git a/sdk/typescript/src/version.ts b/sdk/typescript/src/version.ts index b34bf5c72..15252b77e 100644 --- a/sdk/typescript/src/version.ts +++ b/sdk/typescript/src/version.ts @@ -9,7 +9,7 @@ const PACKAGE_VERSIONS = packageVersions( export const VERSION = PACKAGE_VERSIONS.package; export const CODEX_SDK_VERSION = PACKAGE_VERSIONS.sdk; export const CODEX_EXECUTABLE_VERSION = PACKAGE_VERSIONS.executable; -export const BUNDLED_PLUGIN_VERSION = "0.1.27" as const; +export const BUNDLED_PLUGIN_VERSION = "0.1.28" as const; const PACKAGE_NAME = "@openai/codex-security"; diff --git a/sdk/typescript/tests-ts/cost.test.ts b/sdk/typescript/tests-ts/cost.test.ts index ac59f31f6..6b54715a4 100644 --- a/sdk/typescript/tests-ts/cost.test.ts +++ b/sdk/typescript/tests-ts/cost.test.ts @@ -1383,7 +1383,7 @@ describe("live scan cost tracking", () => { usage, }).toEqual({ predecessorVersion: "0.1.25", - upgradedVersion: "0.1.27", + upgradedVersion: "0.1.28", installedRootChanged: true, safetyIdentifierKey: "CODEX_SAFETY_IDENTIFIER", usage: { @@ -1391,7 +1391,7 @@ describe("live scan cost tracking", () => { warnings: [], }, }); - expect(BUNDLED_PLUGIN_VERSION).toBe("0.1.27"); + expect(BUNDLED_PLUGIN_VERSION).toBe("0.1.28"); }); test("forwards actions from this scan's delegated workers only", async () => { diff --git a/sdk/typescript/tests-ts/diff-rank-input.test.ts b/sdk/typescript/tests-ts/diff-rank-input.test.ts index 5a1f3e1fc..875ea19b3 100644 --- a/sdk/typescript/tests-ts/diff-rank-input.test.ts +++ b/sdk/typescript/tests-ts/diff-rank-input.test.ts @@ -68,7 +68,7 @@ function git(repository: string, ...args: string[]): string { } async function upgradeBundledPlugin(root: string): Promise { - expect(BUNDLED_PLUGIN_VERSION).toBe("0.1.27"); + expect(BUNDLED_PLUGIN_VERSION).toBe("0.1.28"); const previous = join(root, "previous-plugin"); cpSync(PLUGIN_ROOT, previous, { recursive: true }); const previousManifestPath = join(previous, ".codex-plugin", "plugin.json"); diff --git a/sdk/typescript/tests-ts/plugin-report-limits.test.ts b/sdk/typescript/tests-ts/plugin-report-limits.test.ts index 1356466bb..180612fdd 100644 --- a/sdk/typescript/tests-ts/plugin-report-limits.test.ts +++ b/sdk/typescript/tests-ts/plugin-report-limits.test.ts @@ -20,9 +20,9 @@ describe("bundled scan report and source limits", () => { " schema.write_text(json.dumps({'type': 'object', 'description': 'x' * (4 * 1024 * 1024), 'allOf': [{'type': 'object'}] * 129}))", " finalizer.validate_against_schema({'safe': True}, schema)", " source = b'x' * (1024 * 1024 + 1)", - " excerpts.git_bytes = lambda *args: source", + " excerpts.local_git_bytes = lambda *args: source", " target = pathlib.Path(directory).resolve()", - " excerpt = excerpts.scanned_source_text({'target_revision': 'deadbeef', 'target_snapshot_digest': None}, target, 'large.py')", + " excerpt = excerpts.scanned_source_text(target, 'deadbeef')", " hashes = finalizer._github_line_hashes(io.StringIO('line\\n' * 100001), {100001})", " print(json.dumps({'documentBytes': len(document), 'sourceBytes': len(excerpt), 'lateSourceLine': 100001 in hashes, 'unsafePathRejected': excerpts.safe_source_path(target, '../outside') is None}))", ].join("\n"); diff --git a/sdk/typescript/tests-ts/publication-store.test.ts b/sdk/typescript/tests-ts/publication-store.test.ts index 92a927763..2ce176e6b 100644 --- a/sdk/typescript/tests-ts/publication-store.test.ts +++ b/sdk/typescript/tests-ts/publication-store.test.ts @@ -275,9 +275,11 @@ connection.close() fixture, "ALTER TABLE deep_scan_runs DROP COLUMN publication_error_message", ); - databaseRows(fixture, "DELETE FROM schema_migrations WHERE version >= ?", [ - 31, - ]); + databaseRows( + fixture, + "DELETE FROM schema_migrations WHERE version BETWEEN ? AND ?", + [31, 32], + ); await expect( preparePublicationStore(fixture.publication, fixture.environment), @@ -286,8 +288,8 @@ connection.close() expect( databaseRows( fixture, - "SELECT version, name FROM schema_migrations WHERE version >= ? ORDER BY version", - [31], + "SELECT version, name FROM schema_migrations WHERE version BETWEEN ? AND ? ORDER BY version", + [31, 32], ), ).toEqual([ { version: 31, name: "freeze stopped scan source digests" }, diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts new file mode 100644 index 000000000..eeaf0f86d --- /dev/null +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -0,0 +1,604 @@ +import { execFileSync, spawnSync } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import { + cpSync, + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, test } from "bun:test"; +import { BUNDLED_PLUGIN_VERSION, bootstrapPlugin } from "../src/index.js"; +import { runWorkbench } from "../src/runtime.js"; +import { PLUGIN_ROOT } from "./plugin-root.js"; + +const temporaryRoots: string[] = []; + +afterEach(() => { + for (const root of temporaryRoots.splice(0)) { + rmSync(root, { recursive: true, force: true }); + } +}); + +function python(): string { + const command = + process.env["PYTHON"] ?? + Bun.which("python3") ?? + Bun.which("python") ?? + Bun.which("py"); + expect(command).not.toBeNull(); + return command!; +} + +function git( + repository: string, + args: string[], + input?: Buffer | string, +): string { + return execFileSync( + "git", + [ + "-c", + "user.name=Synthetic Fixture", + "-c", + "user.email=fixture@example.invalid", + ...args, + ], + { cwd: repository, encoding: "utf8", input }, + ).trim(); +} + +function collisionRepository(root: string): { + repository: string; + revision: string; +} { + const repository = join(root, "repository"); + mkdirSync(repository); + git(repository, ["init", "-q"]); + const blob = (content: string) => + git(repository, ["hash-object", "-w", "--stdin"], content); + const tree = ( + entries: Array<[mode: string, type: string, oid: string, name: string]>, + ) => { + const records = entries + .toSorted((left, right) => + Buffer.from(left[3]).compare(Buffer.from(right[3])), + ) + .map(([mode, type, oid, name]) => + Buffer.concat([ + Buffer.from(`${mode} ${type} ${oid}\t`), + Buffer.from(name), + Buffer.from([0]), + ]), + ); + return git(repository, ["mktree", "-z"], Buffer.concat(records)); + }; + const sourceTree = tree([ + ["100644", "blob", blob("allowed = True\n"), "allowed.py"], + ["100644", "blob", blob("case_upper = True\n"), "LOWER.py"], + ["100644", "blob", blob("case_lower = True\n"), "lower.py"], + ["100644", "blob", blob("unicode_composed = True\n"), "é.py"], + ["100644", "blob", blob("unicode_decomposed = True\n"), "é.py"], + ]); + const upperScope = tree([ + ["100644", "blob", blob("selected_scope = True\n"), "selected.py"], + ]); + const lowerScope = tree([ + ["100644", "blob", blob("colliding_scope = True\n"), "sibling.py"], + ]); + const rootTree = tree([ + ["040000", "tree", upperScope, "Scope"], + ["040000", "tree", lowerScope, "scope"], + ["040000", "tree", sourceTree, "src"], + ]); + const revision = git(repository, [ + "commit-tree", + rootTree, + "-m", + "synthetic source tree", + ]); + git(repository, ["symbolic-ref", "HEAD", "refs/heads/main"]); + git(repository, ["update-ref", "refs/heads/main", revision]); + mkdirSync(join(repository, "src")); + mkdirSync(join(repository, "Scope")); + writeFileSync(join(repository, "src", "allowed.py"), "allowed = True\n"); + writeFileSync(join(repository, "src", "LOWER.py"), "case_upper = True\n"); + writeFileSync(join(repository, "src", "é.py"), "unicode_composed = True\n"); + writeFileSync( + join(repository, "Scope", "selected.py"), + "selected_scope = True\n", + ); + return { repository, revision }; +} + +function ordinaryRepository(root: string): { + repository: string; + revision: string; +} { + const repository = join(root, "ordinary-repository"); + mkdirSync(join(repository, "src"), { recursive: true }); + mkdirSync(join(repository, "other")); + writeFileSync(join(repository, "src", "allowed.py"), "allowed = True\n"); + writeFileSync(join(repository, "other", "example.py"), "example = True\n"); + git(repository, ["init", "-q"]); + git(repository, ["add", "."]); + git(repository, ["commit", "-qm", "synthetic source tree"]); + return { repository, revision: git(repository, ["rev-parse", "HEAD"]) }; +} + +async function upgradedPlugin(root: string) { + expect(BUNDLED_PLUGIN_VERSION).toBe("0.1.28"); + const previous = join(root, "previous-plugin"); + cpSync(PLUGIN_ROOT, previous, { recursive: true }); + const manifestPath = join(previous, ".codex-plugin", "plugin.json"); + const manifest = JSON.parse(readFileSync(manifestPath, "utf8")) as { + version: string; + }; + manifest.version = "0.1.27"; + writeFileSync(manifestPath, JSON.stringify(manifest, null, 2) + "\n"); + + const excerptPath = join(previous, "scripts", "workbench_source_excerpt.py"); + const source = readFileSync(excerptPath, "utf8"); + const strict = [ + " if len(aliases) != 1:", + " return None", + " entry = next((candidate for candidate in aliases if candidate[0] == name), None)", + ].join("\n"); + const vulnerable = [ + " entry = next((candidate for candidate in aliases if candidate[0] == name), None)", + " if entry is None and len(aliases) != 1:", + " return None", + ].join("\n"); + expect(source.split(strict)).toHaveLength(2); + writeFileSync(excerptPath, source.replace(strict, vulnerable)); + + const home = join(root, "codex-home"); + const marketplace = join(home, "sdk-marketplace"); + mkdirSync(home, { mode: 0o700 }); + const runCodex = async (_command: unknown, args: readonly string[]) => { + if (args[1] === "marketplace") { + writeFileSync( + join(home, "config.toml"), + `[marketplaces.codex-security-sdk]\nsource_type = "local"\nsource = ${JSON.stringify(marketplace)}\n`, + ); + return ""; + } + const selected = join(marketplace, "plugins", "codex-security"); + const selectedManifest = JSON.parse( + readFileSync(join(selected, ".codex-plugin", "plugin.json"), "utf8"), + ) as { version: string }; + const installed = join(home, "installed", selectedManifest.version); + rmSync(installed, { recursive: true, force: true }); + mkdirSync(join(home, "installed"), { recursive: true }); + cpSync(selected, installed, { recursive: true }); + return JSON.stringify({ + installedPath: installed, + version: selectedManifest.version, + }); + }; + const options = { + codexCommand: { command: "/synthetic-codex" }, + runCodex, + }; + const predecessor = await bootstrapPlugin(home, previous, options); + const upgraded = await bootstrapPlugin(home, PLUGIN_ROOT, options); + const installedMcp = JSON.parse( + readFileSync(join(upgraded.installedRoot, ".mcp.json"), "utf8"), + ) as { mcpServers: Record }; + expect(predecessor.version).toBe("0.1.27"); + expect(upgraded.version).toBe("0.1.28"); + expect(upgraded.installedRoot).not.toBe(predecessor.installedRoot); + expect( + installedMcp.mcpServers["codex-security"]?.env_vars?.find( + (name) => name === "CODEX_SAFETY_IDENTIFIER", + ), + ).toBe("CODEX_SAFETY_IDENTIFIER"); + expect( + readFileSync( + join(upgraded.installedRoot, "scripts", "workbench_source_excerpt.py"), + ), + ).toEqual( + readFileSync(join(PLUGIN_ROOT, "scripts", "workbench_source_excerpt.py")), + ); + return { predecessor, upgraded }; +} + +function workbench(root: string, args: string[], input?: string) { + return runWorkbench( + { + python: python(), + pluginRoot: PLUGIN_ROOT, + environment: { + ...process.env, + CODEX_SECURITY_STATE_DIR: join(root, "state"), + PYTHONDONTWRITEBYTECODE: "1", + }, + }, + args, + input, + ); +} + +function collisionProbe( + pluginRoot: string, + fixture: { repository: string; revision: string }, +) { + const program = String.raw` +import json, sys +from pathlib import Path +sys.path.insert(0, sys.argv[1]) +import workbench_source_excerpt as excerpts +from workbench_target import clean_worktree_content_digest + +repository = Path(sys.argv[2]).resolve() +revision = sys.argv[3] +metadata = repository.stat() +identity = (revision, clean_worktree_content_digest(), metadata.st_dev, metadata.st_ino) +authority = excerpts.capture_source_scopes(repository, identity, ["src", "src"]) +authority_json = json.dumps(authority) +scan = { + "mode": "standard", + "diff_target_kind": None, + "target_revision": revision, + "target_snapshot_digest": clean_worktree_content_digest(), + "source_scopes_json": authority_json, +} +def excerpt(path, saved=scan): + return excerpts.finding_source_excerpt( + saved, + repository, + [{"path": path, "startLine": 1, "endLine": 1, "role": "root_control"}], + ) +original_git = excerpts.local_git_bytes +blob_reads = [] +def watched_git(*arguments, **kwargs): + if len(arguments) >= 4 and arguments[1:3] == ("cat-file", "blob"): + blob_reads.append(arguments[3]) + return original_git(*arguments, **kwargs) +excerpts.local_git_bytes = watched_git +allowed = excerpt("src/allowed.py") +before = len(blob_reads) +collisions = { + path: excerpt(path) + for path in ("src/LOWER.py", "src/lower.py", "src/é.py", "src/é.py") +} +collision_blob_reads = blob_reads[before:] +outside = excerpt("outside.py") +excerpts.local_git_bytes = original_git +legacy_scan = dict(scan) +legacy_scan.pop("source_scopes_json") +legacy = excerpt("src/allowed.py", legacy_scan) +invalid = excerpt("src/allowed.py", {**scan, "source_scopes_json": "{"}) + +original_context = excerpts.git_worktree_context +git_calls = [] +def forbidden_git(*arguments, **kwargs): + git_calls.append(arguments) + raise AssertionError("ineligible diff mode reached Git") +excerpts.local_git_bytes = forbidden_git +excerpts.git_worktree_context = forbidden_git +try: + mutable = { + str(kind): excerpt( + "src/allowed.py", {**scan, "mode": "diff", "diff_target_kind": kind} + ) + for kind in ("working_tree", None) + } +finally: + excerpts.local_git_bytes = original_git + excerpts.git_worktree_context = original_context +immutable = { + kind: excerpt( + "src/allowed.py", {**scan, "mode": "diff", "diff_target_kind": kind} + ) + for kind in ("commit", "range") +} +print(json.dumps({ + "allowed": allowed, + "collisionBlobReads": collision_blob_reads, + "collisions": collisions, + "duplicateScopes": len(authority["scopes"]), + "immutable": immutable, + "invalid": invalid, + "legacy": legacy, + "mutable": {"excerpts": mutable, "gitCalls": len(git_calls)}, + "outside": outside, + "pathCollisionScopes": len( + excerpts.capture_source_scopes(repository, identity, ["Scope"])["scopes"] + ), +})) +`; + const result = spawnSync( + python(), + [ + "-I", + "-B", + "-c", + program, + join(pluginRoot, "scripts"), + fixture.repository, + fixture.revision, + ], + { encoding: "utf8" }, + ); + expect(result.status, result.stderr).toBe(0); + return JSON.parse(result.stdout) as Record; +} + +describe("workbench source excerpts", () => { + test("fails closed on normalized collisions after a cached upgrade", async () => { + const root = realpathSync( + mkdtempSync(join(tmpdir(), "codex-security-source-collision-")), + ); + temporaryRoots.push(root); + const fixture = collisionRepository(root); + const installation = await upgradedPlugin(root); + const stale = collisionProbe( + installation.predecessor.installedRoot, + fixture, + ); + const fixed = collisionProbe(installation.upgraded.installedRoot, fixture); + + expect( + (stale["collisions"] as Record)["src/LOWER.py"], + ).toContain("case_upper = True"); + expect(stale["pathCollisionScopes"]).toBe(1); + expect(fixed).toEqual({ + allowed: expect.stringContaining("allowed = True"), + collisionBlobReads: [], + collisions: { + "src/LOWER.py": null, + "src/lower.py": null, + "src/é.py": null, + "src/é.py": null, + }, + duplicateScopes: 1, + immutable: { + commit: expect.stringContaining("allowed = True"), + range: expect.stringContaining("allowed = True"), + }, + invalid: null, + legacy: null, + mutable: { + excerpts: { working_tree: null, None: null }, + gitCalls: 0, + }, + outside: null, + pathCollisionScopes: 0, + }); + }, 60_000); + + test("persists selected source authority from every scan writer", async () => { + const root = realpathSync( + mkdtempSync(join(tmpdir(), "codex-security-source-writers-")), + ); + temporaryRoots.push(root); + const { repository, revision } = ordinaryRepository(root); + const scanRoot = join(root, "scans"); + const workspaceId = randomUUID(); + await workbench(root, [ + "create-workspace", + "--workspace-id", + workspaceId, + "--thread-id", + "workspace-writer", + ]); + await workbench(root, [ + "save-workspace", + "--workspace-id", + workspaceId, + "--target-path", + repository, + "--scope", + "src", + "--mode", + "standard", + ]); + const workspace = await workbench(root, [ + "start-scan", + "--workspace-id", + workspaceId, + "--scan-root", + scanRoot, + ]); + const prompt = await workbench(root, [ + "start-prompt-only-scan", + "--thread-id", + "prompt-writer", + "--target-path", + repository, + "--scope", + "src", + "--mode", + "standard", + "--scan-root", + scanRoot, + ]); + const headless = await workbench(root, [ + "start-headless-standard-scan", + "--thread-id", + "headless-writer", + "--target-path", + repository, + "--scope", + "src", + "--scan-root", + scanRoot, + ]); + const cliScanDirectory = join(root, "cli-scan"); + mkdirSync(cliScanDirectory, { mode: 0o700 }); + const cli = await workbench(root, [ + "register-cli-scan", + "--repository", + repository, + "--scan-dir", + cliScanDirectory, + "--recipe-json", + JSON.stringify({ + config: {}, + mode: "standard", + repository, + target: { kind: "paths", paths: ["src/allowed.py", "other"] }, + }), + ]); + const deep = await workbench(root, [ + "begin-deep-scan", + "--thread-id", + "deep-writer", + "--target-path", + repository, + "--scan-root", + scanRoot, + "--available-parallelism", + "4", + ]); + const database = await workbench(root, ["database-info"]); + const query = spawnSync( + python(), + [ + "-I", + "-B", + "-c", + "import json, sqlite3, sys; c = sqlite3.connect(sys.argv[1]); print(json.dumps({row[0]: json.loads(row[1]) for row in c.execute('SELECT id, source_scopes_json FROM scans')}))", + String(database["databasePath"]), + ], + { encoding: "utf8" }, + ); + expect(query.status, query.stderr).toBe(0); + const authorities = JSON.parse(query.stdout) as Record< + string, + { revision: string; scopes: Array<{ path: string }>; targetTree: string } + >; + const workspaceResults = workspace["results"] as Record; + const promptScan = prompt["scan"] as Record; + const headlessScan = headless["scan"] as Record; + const deepScan = deep["deepScan"] as Record; + const expected = [ + ["workspace", workspaceResults["scanId"], ["src"]], + ["prompt", promptScan["scanId"], ["src"]], + ["headless", headlessScan["scanId"], ["src"]], + ["CLI", cli["scanId"], ["src/allowed.py", "other"]], + ["deep", deepScan["scanId"], ["."]], + ] as const; + expect(Object.keys(authorities)).toHaveLength(expected.length); + for (const [writer, scanId, paths] of expected) { + const authority = authorities[String(scanId)]; + expect(authority?.revision, writer).toBe(revision); + expect(authority?.targetTree, writer).toMatch(/^[a-f0-9]{40,64}$/u); + expect( + authority?.scopes.map(({ path }) => path), + writer, + ).toEqual([...paths]); + } + }, 60_000); + + test("migration 33 appends once over the exact predecessor", () => { + const program = String.raw` +import json, sqlite3, sys +sys.path.insert(0, sys.argv[1]) +from workbench_schema import MIGRATIONS, apply_migrations + +connection = sqlite3.connect(":memory:") +connection.row_factory = sqlite3.Row +counter = 0 +def now(): + global counter + counter += 1 + return f"2026-08-24T00:00:{counter:02d}Z" +def rows(): + return [tuple(row) for row in connection.execute( + "SELECT version, name, applied_at FROM schema_migrations WHERE version >= 29 ORDER BY version" + )] + +predecessor = MIGRATIONS[:-1] +apply_migrations(connection, predecessor, now, lambda database: None) +before = rows() +apply_migrations(connection, MIGRATIONS, now, lambda database: None) +once = rows() +column_count = sum( + row[1] == "source_scopes_json" + for row in connection.execute("PRAGMA table_info(scans)") +) +apply_migrations(connection, MIGRATIONS, now, lambda database: None) +twice = rows() +print(json.dumps({ + "predecessorMax": predecessor[-1][0], + "beforeVersions": [row[0] for row in before], + "onceVersions": [row[0] for row in once], + "oldRowsPreserved": once[:-1] == before, + "secondApplyUnchanged": twice == once, + "columnCount": column_count, + "newName": once[-1][1], +})) +`; + const result = spawnSync( + python(), + ["-I", "-B", "-c", program, join(PLUGIN_ROOT, "scripts")], + { encoding: "utf8" }, + ); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + predecessorMax: 32, + beforeVersions: [29, 30, 31, 32], + onceVersions: [29, 30, 31, 32, 33], + oldRowsPreserved: true, + secondApplyUnchanged: true, + columnCount: 1, + newName: "persist selected source excerpt authority", + }); + }); + + test("authorizes raw finding paths before bounding display output", () => { + const program = String.raw` +import json, sqlite3, sys, tempfile +from pathlib import Path +sys.path.insert(0, sys.argv[1]) +import workbench_db + +raw_path = "segment/" * 300 + "source.py" +connection = sqlite3.connect(":memory:") +connection.row_factory = sqlite3.Row +connection.execute("CREATE TABLE finding_locations (occurrence_id TEXT, relative_path TEXT, start_line INTEGER, end_line INTEGER, role TEXT, sort_order INTEGER)") +connection.execute("INSERT INTO finding_locations VALUES ('occurrence', ?, 1, 1, 'root_control', 0)", (raw_path,)) +target = Path(tempfile.mkdtemp()).resolve() +workbench_db.require_scan_target_identity = lambda scan: target +workbench_db.safe_source_path = lambda selected, value: None +workbench_db.finding_remediation_result = lambda database, occurrence_id: None +workbench_db.finding_triage_result = lambda database, occurrence_id: None +workbench_db.scan_history.finding_matches = lambda *arguments: ([], None, []) +seen = [] +def source_excerpt(scan, selected, locations): + seen.extend(location["path"] for location in locations) + return "1 raw_path_authorized = True" if locations[0]["path"] == raw_path else None +workbench_db.finding_source_excerpt = source_excerpt +finding = workbench_db.finding_result( + connection, + {"id": "scan", "started_at": "now", "scan_dir": str(target)}, + {"id": "occurrence", "details_json": "{}", "confidence": "high", "severity": "high", "created_at": "now", "finding_id": "finding", "remediation": "fix", "summary": "summary", "title": "title"}, +) +display_path = finding["locations"][0]["path"] +print(json.dumps({ + "displayBytes": len(display_path.encode()), + "displayDiffers": display_path != raw_path, + "excerpt": finding.get("sourceExcerpt"), + "sawRawPath": seen == [raw_path], +})) +`; + const result = spawnSync( + python(), + ["-I", "-B", "-c", program, join(PLUGIN_ROOT, "scripts")], + { encoding: "utf8" }, + ); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + displayBytes: 2_048, + displayDiffers: true, + excerpt: "1 raw_path_authorized = True", + sawRawPath: true, + }); + }); +}); From 9ea43e0876c328553ac80e4299b8055d653863f9 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 13:17:27 +0000 Subject: [PATCH 02/26] fix(workbench): constrain persisted source authority --- .../_bundled_plugin/scripts/workbench_db.py | 8 +- .../scripts/workbench_source_excerpt.py | 137 ++++++++---------- .../tests-ts/workbench-source-excerpt.test.ts | 67 ++++----- 3 files changed, 96 insertions(+), 116 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py index 08abf34a0..a08407fb5 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py @@ -4180,7 +4180,13 @@ def finding_result( result["knownSince"] = known_since result["knownScanIds"] = known_scan_ids result.pop("artifactPaths", None) - source_excerpt = finding_source_excerpt(scan, target, excerpt_locations) + try: + selected_paths = requested_scan_paths(scan) + except (IndexError, KeyError, TypeError, ValueError): + selected_paths = [] + source_excerpt = finding_source_excerpt( + scan, target, excerpt_locations, selected_paths + ) if source_excerpt: result["sourceExcerpt"] = source_excerpt artifact_paths = finding_artifact_paths(Path(scan["scan_dir"]), details) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index 7b3b8933f..7476f137e 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -123,8 +123,6 @@ def target_tree(target: Path, revision: str) -> tuple[Path, str] | None: if not OBJECT_ID.fullmatch(revision): return None repository, prefix = git_worktree_context(target) - if local_git_bytes(repository, "replace", "--list") != b"": - return None raw_tree = local_git_bytes( repository, "rev-parse", @@ -154,7 +152,7 @@ def capture_source_scopes( diff_target_kind: str | None = None, ) -> dict[str, Any]: revision, snapshot = target_identity[:2] - authority: dict[str, Any] = {"version": 1, "revision": revision, "scopes": []} + authority: dict[str, Any] = {"version": 1, "paths": []} if ( (diff_target_kind is not None and diff_target_kind not in {"commit", "range"}) or revision == "unversioned" @@ -166,8 +164,7 @@ def capture_source_scopes( if context is None: return authority repository, tree = context - authority["targetTree"] = tree - captured: set[tuple[str, str, str]] = set() + captured: set[str] = set() for requested in paths: parsed = relative_path(requested) selected_path = safe_source_path(target, requested) @@ -186,22 +183,17 @@ def capture_source_scopes( metadata.st_mode ): continue - scope = { - "path": parsed.as_posix(), - "kind": entry[1], - "objectId": entry[2], - } - key = tuple(scope.values()) - if key not in captured: - captured.add(key) - authority["scopes"].append(scope) + selected = parsed.as_posix() + if selected not in captured: + captured.add(selected) + authority["paths"].append(selected) except (OSError, RuntimeError, SystemExit, UnicodeError, ValueError): - return {"version": 1, "revision": revision, "scopes": []} + return {"version": 1, "paths": []} return authority def load_source_scopes( - scan: sqlite3.Row, target: Path + scan: sqlite3.Row, target: Path, selected_paths: list[str] ) -> tuple[Path, tuple[SourceScope, ...]] | None: try: saved = scan["source_scopes_json"] @@ -210,56 +202,54 @@ def load_source_scopes( if not isinstance(saved, str): return None metadata = json.loads(saved) - tree = metadata.get("targetTree") if isinstance(metadata, dict) else None - records = metadata.get("scopes") if isinstance(metadata, dict) else None + records = metadata.get("paths") if isinstance(metadata, dict) else None if ( not isinstance(metadata, dict) or metadata.get("version") != 1 - or metadata.get("revision") != scan["target_revision"] - or not isinstance(tree, str) - or not OBJECT_ID.fullmatch(tree) or not isinstance(records, list) or not records ): return None + expected = { + parsed.as_posix() + for value in selected_paths + if isinstance(value, str) and (parsed := relative_path(value)) is not None + } + if not expected: + return None context = target_tree(target, scan["target_revision"]) - if context is None or context[1] != tree: + if context is None: return None - repository, _ = context + repository, tree = context scopes: list[SourceScope] = [] + seen: set[str] = set() for record in records: - if not isinstance(record, dict): + path = relative_path(record) if isinstance(record, str) else None + if path is None: + return None + selected = path.as_posix() + if selected != record or selected not in expected or selected in seen: return None - path = record.get("path") - kind = record.get("kind") - object_id = record.get("objectId") - requested_path = relative_path(path) if isinstance(path, str) else None - if ( - requested_path is None - or kind not in {"file", "directory"} - or not isinstance(object_id, str) - or not OBJECT_ID.fullmatch(object_id) - ): + seen.add(selected) + entry = tree_path(repository, tree, selected) + if entry is None or entry[1] not in {"file", "directory"}: return None scope = { - "path": path, - "kind": kind, - "objectId": object_id, + "path": selected, + "kind": entry[1], + "objectId": entry[2], } - if tree_path(repository, tree, path) != (path, kind, object_id): - return None scopes.append(scope) return repository, tuple(scopes) def source_object_for_path( repository: Path, - target: Path, value: str, scope: SourceScope, ) -> str | None: path = relative_path(value) - if path is None or safe_source_path(target, value) is None: + if path is None: return None scope_path = PurePosixPath(scope["path"]) scope_length = len(scope_path.parts) @@ -284,6 +274,7 @@ def finding_source_excerpt( scan: sqlite3.Row, target: Path | None, locations: list[dict[str, Any]], + selected_paths: list[str], ) -> str | None: if scan["mode"] == "diff" and scan["diff_target_kind"] not in {"commit", "range"}: return None @@ -293,54 +284,48 @@ def finding_source_excerpt( if snapshot is not None and snapshot != clean_worktree_content_digest(): return None try: - context = load_source_scopes(scan, target) + context = load_source_scopes(scan, target, selected_paths) except (OSError, RuntimeError, SystemExit, UnicodeError, ValueError): return None if context is None: return None repository, scopes = context - def priority(location: dict[str, Any]) -> int: - role = location.get("role") - if role == "root_control": - return 0 - return 1 if "root_control" in str(role or "").lower() else 2 - - selected_location = None - object_id = None - for location in sorted(locations, key=priority): - path = location.get("path") - if not isinstance(path, str) or not isinstance(location.get("startLine"), int): - continue - try: - object_id = next( - ( - candidate - for scope in scopes - if ( - candidate := source_object_for_path( - repository, target, path, scope - ) - ) - is not None - ), - None, - ) - except (OSError, RuntimeError, SystemExit, UnicodeError, ValueError): - object_id = None - if object_id is not None: - selected_location = location - break - if selected_location is None or object_id is None: + location = next( + ( + candidate + for candidate in locations + if "root_control" in str(candidate.get("role") or "").lower() + ), + locations[0], + ) + path = location.get("path") + start_line = location.get("startLine") + if not isinstance(path, str) or not isinstance(start_line, int): + return None + try: + object_id = next( + ( + candidate + for scope in scopes + if ( + candidate := source_object_for_path(repository, path, scope) + ) + is not None + ), + None, + ) + except (OSError, RuntimeError, SystemExit, UnicodeError, ValueError): + return None + if object_id is None: return None source = scanned_source_text(repository, object_id) if not source or "\0" in source: return None - start_line = selected_location["startLine"] lines = source.splitlines() if start_line < 1 or start_line > len(lines): return None - end_line = selected_location.get("endLine") + end_line = location.get("endLine") last_affected_line = end_line if isinstance(end_line, int) else start_line excerpt_start = max(1, start_line - CONTEXT_LINES) excerpt_end = min( diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index eeaf0f86d..65c00ae61 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -93,6 +93,7 @@ function collisionRepository(root: string): { const rootTree = tree([ ["040000", "tree", upperScope, "Scope"], ["040000", "tree", lowerScope, "scope"], + ["100644", "blob", blob("outside = True\n"), "outside.py"], ["040000", "tree", sourceTree, "src"], ]); const revision = git(repository, [ @@ -141,21 +142,6 @@ async function upgradedPlugin(root: string) { manifest.version = "0.1.27"; writeFileSync(manifestPath, JSON.stringify(manifest, null, 2) + "\n"); - const excerptPath = join(previous, "scripts", "workbench_source_excerpt.py"); - const source = readFileSync(excerptPath, "utf8"); - const strict = [ - " if len(aliases) != 1:", - " return None", - " entry = next((candidate for candidate in aliases if candidate[0] == name), None)", - ].join("\n"); - const vulnerable = [ - " entry = next((candidate for candidate in aliases if candidate[0] == name), None)", - " if entry is None and len(aliases) != 1:", - " return None", - ].join("\n"); - expect(source.split(strict)).toHaveLength(2); - writeFileSync(excerptPath, source.replace(strict, vulnerable)); - const home = join(root, "codex-home"); const marketplace = join(home, "sdk-marketplace"); mkdirSync(home, { mode: 0o700 }); @@ -247,11 +233,12 @@ scan = { "target_snapshot_digest": clean_worktree_content_digest(), "source_scopes_json": authority_json, } -def excerpt(path, saved=scan): +def excerpt(path, saved=scan, selected_paths=("src",)): return excerpts.finding_source_excerpt( saved, repository, [{"path": path, "startLine": 1, "endLine": 1, "role": "root_control"}], + list(selected_paths), ) original_git = excerpts.local_git_bytes blob_reads = [] @@ -268,6 +255,12 @@ collisions = { } collision_blob_reads = blob_reads[before:] outside = excerpt("outside.py") +before = len(blob_reads) +broadened = excerpt( + "outside.py", + {**scan, "source_scopes_json": json.dumps({"version": 1, "paths": ["."]})}, +) +broadened_blob_reads = blob_reads[before:] excerpts.local_git_bytes = original_git legacy_scan = dict(scan) legacy_scan.pop("source_scopes_json") @@ -299,16 +292,18 @@ immutable = { } print(json.dumps({ "allowed": allowed, + "broadened": broadened, + "broadenedBlobReads": broadened_blob_reads, "collisionBlobReads": collision_blob_reads, "collisions": collisions, - "duplicateScopes": len(authority["scopes"]), + "duplicatePaths": len(authority["paths"]), "immutable": immutable, "invalid": invalid, "legacy": legacy, "mutable": {"excerpts": mutable, "gitCalls": len(git_calls)}, "outside": outside, - "pathCollisionScopes": len( - excerpts.capture_source_scopes(repository, identity, ["Scope"])["scopes"] + "pathCollisionPaths": len( + excerpts.capture_source_scopes(repository, identity, ["Scope"])["paths"] ), })) `; @@ -337,18 +332,12 @@ describe("workbench source excerpts", () => { temporaryRoots.push(root); const fixture = collisionRepository(root); const installation = await upgradedPlugin(root); - const stale = collisionProbe( - installation.predecessor.installedRoot, - fixture, - ); const fixed = collisionProbe(installation.upgraded.installedRoot, fixture); - expect( - (stale["collisions"] as Record)["src/LOWER.py"], - ).toContain("case_upper = True"); - expect(stale["pathCollisionScopes"]).toBe(1); expect(fixed).toEqual({ allowed: expect.stringContaining("allowed = True"), + broadened: null, + broadenedBlobReads: [], collisionBlobReads: [], collisions: { "src/LOWER.py": null, @@ -356,7 +345,7 @@ describe("workbench source excerpts", () => { "src/é.py": null, "src/é.py": null, }, - duplicateScopes: 1, + duplicatePaths: 1, immutable: { commit: expect.stringContaining("allowed = True"), range: expect.stringContaining("allowed = True"), @@ -368,7 +357,7 @@ describe("workbench source excerpts", () => { gitCalls: 0, }, outside: null, - pathCollisionScopes: 0, + pathCollisionPaths: 0, }); }, 60_000); @@ -377,7 +366,7 @@ describe("workbench source excerpts", () => { mkdtempSync(join(tmpdir(), "codex-security-source-writers-")), ); temporaryRoots.push(root); - const { repository, revision } = ordinaryRepository(root); + const { repository } = ordinaryRepository(root); const scanRoot = join(root, "scans"); const workspaceId = randomUUID(); await workbench(root, [ @@ -471,7 +460,7 @@ describe("workbench source excerpts", () => { expect(query.status, query.stderr).toBe(0); const authorities = JSON.parse(query.stdout) as Record< string, - { revision: string; scopes: Array<{ path: string }>; targetTree: string } + { paths: string[]; version: number } >; const workspaceResults = workspace["results"] as Record; const promptScan = prompt["scan"] as Record; @@ -487,12 +476,8 @@ describe("workbench source excerpts", () => { expect(Object.keys(authorities)).toHaveLength(expected.length); for (const [writer, scanId, paths] of expected) { const authority = authorities[String(scanId)]; - expect(authority?.revision, writer).toBe(revision); - expect(authority?.targetTree, writer).toMatch(/^[a-f0-9]{40,64}$/u); - expect( - authority?.scopes.map(({ path }) => path), - writer, - ).toEqual([...paths]); + expect(authority?.version, writer).toBe(1); + expect(authority?.paths, writer).toEqual([...paths]); } }, 60_000); @@ -571,13 +556,15 @@ workbench_db.finding_remediation_result = lambda database, occurrence_id: None workbench_db.finding_triage_result = lambda database, occurrence_id: None workbench_db.scan_history.finding_matches = lambda *arguments: ([], None, []) seen = [] -def source_excerpt(scan, selected, locations): +selected_paths_seen = [] +def source_excerpt(scan, selected, locations, selected_paths): seen.extend(location["path"] for location in locations) + selected_paths_seen.extend(selected_paths) return "1 raw_path_authorized = True" if locations[0]["path"] == raw_path else None workbench_db.finding_source_excerpt = source_excerpt finding = workbench_db.finding_result( connection, - {"id": "scan", "started_at": "now", "scan_dir": str(target)}, + {"id": "scan", "started_at": "now", "scan_dir": str(target), "scope": "."}, {"id": "occurrence", "details_json": "{}", "confidence": "high", "severity": "high", "created_at": "now", "finding_id": "finding", "remediation": "fix", "summary": "summary", "title": "title"}, ) display_path = finding["locations"][0]["path"] @@ -586,6 +573,7 @@ print(json.dumps({ "displayDiffers": display_path != raw_path, "excerpt": finding.get("sourceExcerpt"), "sawRawPath": seen == [raw_path], + "sawSelectedPaths": selected_paths_seen == ["."], })) `; const result = spawnSync( @@ -599,6 +587,7 @@ print(json.dumps({ displayDiffers: true, excerpt: "1 raw_path_authorized = True", sawRawPath: true, + sawSelectedPaths: true, }); }); }); From 76eed7cfa69a6f4d301197a377d02393631fe9ef Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 13:19:57 +0000 Subject: [PATCH 03/26] fix(workbench): reject divergent excerpt views --- .../_bundled_plugin/scripts/workbench_db.py | 4 + .../scripts/workbench_source_excerpt.py | 12 ++- .../tests-ts/workbench-source-excerpt.test.ts | 74 ++++++++++++++++--- 3 files changed, 75 insertions(+), 15 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py index a08407fb5..1bd432d75 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py @@ -4184,6 +4184,10 @@ def finding_result( selected_paths = requested_scan_paths(scan) except (IndexError, KeyError, TypeError, ValueError): selected_paths = [] + if not isinstance(selected_paths, list) or not all( + isinstance(path, str) for path in selected_paths + ): + selected_paths = [] source_excerpt = finding_source_excerpt( scan, target, excerpt_locations, selected_paths ) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index 7476f137e..7a4733436 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -120,9 +120,11 @@ def tree_path(repository: Path, tree: str, value: str) -> TreeEntry | None: def target_tree(target: Path, revision: str) -> tuple[Path, str] | None: - if not OBJECT_ID.fullmatch(revision): + if not isinstance(revision, str) or not OBJECT_ID.fullmatch(revision): return None repository, prefix = git_worktree_context(target) + if local_git_bytes(repository, "replace", "--list") != b"": + return None raw_tree = local_git_bytes( repository, "rev-parse", @@ -210,10 +212,14 @@ def load_source_scopes( or not records ): return None + if not isinstance(selected_paths, list) or not all( + isinstance(value, str) for value in selected_paths + ): + return None expected = { parsed.as_posix() for value in selected_paths - if isinstance(value, str) and (parsed := relative_path(value)) is not None + if (parsed := relative_path(value)) is not None } if not expected: return None @@ -285,7 +291,7 @@ def finding_source_excerpt( return None try: context = load_source_scopes(scan, target, selected_paths) - except (OSError, RuntimeError, SystemExit, UnicodeError, ValueError): + except (OSError, RuntimeError, SystemExit, TypeError, UnicodeError, ValueError): return None if context is None: return None diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 65c00ae61..3b136f52a 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -54,6 +54,7 @@ function git( function collisionRepository(root: string): { repository: string; + replacement: string; revision: string; } { const repository = join(root, "repository"); @@ -102,6 +103,15 @@ function collisionRepository(root: string): { "-m", "synthetic source tree", ]); + const replacementTree = tree([ + ["100644", "blob", blob("replacement = True\n"), "replacement.py"], + ]); + const replacement = git(repository, [ + "commit-tree", + replacementTree, + "-m", + "synthetic replacement tree", + ]); git(repository, ["symbolic-ref", "HEAD", "refs/heads/main"]); git(repository, ["update-ref", "refs/heads/main", revision]); mkdirSync(join(repository, "src")); @@ -113,7 +123,7 @@ function collisionRepository(root: string): { join(repository, "Scope", "selected.py"), "selected_scope = True\n", ); - return { repository, revision }; + return { repository, replacement, revision }; } function ordinaryRepository(root: string): { @@ -211,10 +221,10 @@ function workbench(root: string, args: string[], input?: string) { function collisionProbe( pluginRoot: string, - fixture: { repository: string; revision: string }, + fixture: { repository: string; replacement: string; revision: string }, ) { const program = String.raw` -import json, sys +import json, subprocess, sys from pathlib import Path sys.path.insert(0, sys.argv[1]) import workbench_source_excerpt as excerpts @@ -222,6 +232,7 @@ from workbench_target import clean_worktree_content_digest repository = Path(sys.argv[2]).resolve() revision = sys.argv[3] +replacement = sys.argv[4] metadata = repository.stat() identity = (revision, clean_worktree_content_digest(), metadata.st_dev, metadata.st_ino) authority = excerpts.capture_source_scopes(repository, identity, ["src", "src"]) @@ -233,12 +244,14 @@ scan = { "target_snapshot_digest": clean_worktree_content_digest(), "source_scopes_json": authority_json, } -def excerpt(path, saved=scan, selected_paths=("src",)): +def excerpt(path, saved=scan, selected_paths=None): + if selected_paths is None: + selected_paths = ["src"] return excerpts.finding_source_excerpt( saved, repository, [{"path": path, "startLine": 1, "endLine": 1, "role": "root_control"}], - list(selected_paths), + selected_paths, ) original_git = excerpts.local_git_bytes blob_reads = [] @@ -261,11 +274,28 @@ broadened = excerpt( {**scan, "source_scopes_json": json.dumps({"version": 1, "paths": ["."]})}, ) broadened_blob_reads = blob_reads[before:] +subprocess.run( + ["git", "-C", str(repository), "update-ref", f"refs/replace/{revision}", replacement], + check=True, +) +before = len(blob_reads) +try: + replaced = excerpt("src/allowed.py") +finally: + subprocess.run( + ["git", "-C", str(repository), "update-ref", "-d", f"refs/replace/{revision}"], + check=True, + ) +replacement_blob_reads = blob_reads[before:] excerpts.local_git_bytes = original_git legacy_scan = dict(scan) legacy_scan.pop("source_scopes_json") legacy = excerpt("src/allowed.py", legacy_scan) invalid = excerpt("src/allowed.py", {**scan, "source_scopes_json": "{"}) +malformed = { + "paths": excerpt("src/allowed.py", selected_paths=42), + "revision": excerpt("src/allowed.py", {**scan, "target_revision": 42}), +} original_context = excerpts.git_worktree_context git_calls = [] @@ -300,11 +330,14 @@ print(json.dumps({ "immutable": immutable, "invalid": invalid, "legacy": legacy, + "malformed": malformed, "mutable": {"excerpts": mutable, "gitCalls": len(git_calls)}, "outside": outside, "pathCollisionPaths": len( excerpts.capture_source_scopes(repository, identity, ["Scope"])["paths"] ), + "replaced": replaced, + "replacementBlobReads": replacement_blob_reads, })) `; const result = spawnSync( @@ -317,6 +350,7 @@ print(json.dumps({ join(pluginRoot, "scripts"), fixture.repository, fixture.revision, + fixture.replacement, ], { encoding: "utf8" }, ); @@ -352,12 +386,15 @@ describe("workbench source excerpts", () => { }, invalid: null, legacy: null, + malformed: { paths: null, revision: null }, mutable: { excerpts: { working_tree: null, None: null }, gitCalls: 0, }, outside: null, pathCollisionPaths: 0, + replaced: null, + replacementBlobReads: [], }); }, 60_000); @@ -558,22 +595,33 @@ workbench_db.scan_history.finding_matches = lambda *arguments: ([], None, []) seen = [] selected_paths_seen = [] def source_excerpt(scan, selected, locations, selected_paths): - seen.extend(location["path"] for location in locations) - selected_paths_seen.extend(selected_paths) - return "1 raw_path_authorized = True" if locations[0]["path"] == raw_path else None + seen.append([location["path"] for location in locations]) + selected_paths_seen.append(selected_paths) + if selected_paths and locations[0]["path"] == raw_path: + return "1 raw_path_authorized = True" + return None workbench_db.finding_source_excerpt = source_excerpt +scan = {"id": "scan", "started_at": "now", "scan_dir": str(target), "scope": "."} +occurrence = {"id": "occurrence", "details_json": "{}", "confidence": "high", "severity": "high", "created_at": "now", "finding_id": "finding", "remediation": "fix", "summary": "summary", "title": "title"} finding = workbench_db.finding_result( connection, - {"id": "scan", "started_at": "now", "scan_dir": str(target), "scope": "."}, - {"id": "occurrence", "details_json": "{}", "confidence": "high", "severity": "high", "created_at": "now", "finding_id": "finding", "remediation": "fix", "summary": "summary", "title": "title"}, + scan, + occurrence, +) +malformed = workbench_db.finding_result( + connection, + {**scan, "recipe_json": json.dumps({"target": {"kind": "paths", "paths": 42}})}, + occurrence, ) display_path = finding["locations"][0]["path"] print(json.dumps({ "displayBytes": len(display_path.encode()), "displayDiffers": display_path != raw_path, "excerpt": finding.get("sourceExcerpt"), - "sawRawPath": seen == [raw_path], - "sawSelectedPaths": selected_paths_seen == ["."], + "malformedExcerpt": malformed.get("sourceExcerpt"), + "malformedTitle": malformed.get("title"), + "sawRawPath": seen == [[raw_path], [raw_path]], + "sawSelectedPaths": selected_paths_seen == [["."], []], })) `; const result = spawnSync( @@ -586,6 +634,8 @@ print(json.dumps({ displayBytes: 2_048, displayDiffers: true, excerpt: "1 raw_path_authorized = True", + malformedExcerpt: null, + malformedTitle: "title", sawRawPath: true, sawSelectedPaths: true, }); From 047538b4ee90209432db5ff704ceb2ddc53eb41c Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 13:21:03 +0000 Subject: [PATCH 04/26] test(workbench): keep malformed metadata at boundary --- .../_bundled_plugin/scripts/workbench_source_excerpt.py | 6 +----- sdk/typescript/tests-ts/workbench-source-excerpt.test.ts | 9 +++------ 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index 7a4733436..55421c8a7 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -212,10 +212,6 @@ def load_source_scopes( or not records ): return None - if not isinstance(selected_paths, list) or not all( - isinstance(value, str) for value in selected_paths - ): - return None expected = { parsed.as_posix() for value in selected_paths @@ -291,7 +287,7 @@ def finding_source_excerpt( return None try: context = load_source_scopes(scan, target, selected_paths) - except (OSError, RuntimeError, SystemExit, TypeError, UnicodeError, ValueError): + except (OSError, RuntimeError, SystemExit, UnicodeError, ValueError): return None if context is None: return None diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 3b136f52a..06d23362a 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -292,10 +292,7 @@ legacy_scan = dict(scan) legacy_scan.pop("source_scopes_json") legacy = excerpt("src/allowed.py", legacy_scan) invalid = excerpt("src/allowed.py", {**scan, "source_scopes_json": "{"}) -malformed = { - "paths": excerpt("src/allowed.py", selected_paths=42), - "revision": excerpt("src/allowed.py", {**scan, "target_revision": 42}), -} +malformed_revision = excerpt("src/allowed.py", {**scan, "target_revision": 42}) original_context = excerpts.git_worktree_context git_calls = [] @@ -330,7 +327,7 @@ print(json.dumps({ "immutable": immutable, "invalid": invalid, "legacy": legacy, - "malformed": malformed, + "malformedRevision": malformed_revision, "mutable": {"excerpts": mutable, "gitCalls": len(git_calls)}, "outside": outside, "pathCollisionPaths": len( @@ -386,7 +383,7 @@ describe("workbench source excerpts", () => { }, invalid: null, legacy: null, - malformed: { paths: null, revision: null }, + malformedRevision: null, mutable: { excerpts: { working_tree: null, None: null }, gitCalls: 0, From 8bfb4b9cf80227306ff623ba8cde2a3ff4546b54 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 13:47:59 +0000 Subject: [PATCH 05/26] fix(workbench): prepare excerpt authority outside hot paths --- .../_bundled_plugin/scripts/workbench_db.py | 77 +++++++++++---- .../scripts/workbench_scan_start.py | 16 ++-- .../scripts/workbench_source_excerpt.py | 31 +++++-- .../tests-ts/workbench-source-excerpt.test.ts | 93 ++++++++++++++----- 4 files changed, 161 insertions(+), 56 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py index 1bd432d75..faea33a7c 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py @@ -101,7 +101,13 @@ from workbench_schema import ( sql_statements as sql_statements, ) -from workbench_source_excerpt import finding_source_excerpt, safe_source_path +from workbench_source_excerpt import ( + SourceContext, + capture_source_scopes, + finding_source_excerpt_from_context, + safe_source_path, + source_excerpt_context, +) from workbench_target import ( clean_worktree_content_digest, committed_diff_content_snapshot, @@ -2280,6 +2286,12 @@ def register_cli_scan(connection: sqlite3.Connection, args: argparse.Namespace) ) = committed_diff_content_snapshot(repository, base, head) mode = "diff" if diff_target is not None else recipe["mode"] target_identity = scan_target_identity(repository, diff_target) + source_scopes = capture_source_scopes( + repository, + target_identity, + paths or ["."], + diff_target_kind=diff_target["kind"] if diff_target is not None else None, + ) scope_file_count = ( directory_snapshot_regular_file_count(repository) if not paths @@ -2394,7 +2406,7 @@ def target_matches_initial_snapshot() -> bool: timestamp=timestamp, handoff_status="delivered", scan_dir=scan_dir, - source_paths=paths or ["."], + source_scopes=source_scopes, ) connection.execute( "UPDATE scans SET recipe_json = ?, parent_scan_id = ?, user_context = ? WHERE id = ?", @@ -3860,7 +3872,7 @@ def list_findings(connection: sqlite3.Connection, args: argparse.Namespace) -> d next_offset = args.offset + len(rows) return { "findingsPage": { - "findings": [finding_result(connection, scan, row) for row in rows], + "findings": finding_results(connection, scan, rows), "limit": limit, "nextOffset": next_offset if next_offset < total else None, "offset": args.offset, @@ -3959,7 +3971,7 @@ def scan_result( "contract": scan_contract(scan), "continuationThreadId": scan["continuation_thread_id"], "failureMessage": scan["failure_message"], - "findings": [finding_result(connection, scan, row) for row in occurrence_rows], + "findings": finding_results(connection, scan, occurrence_rows), "findingCount": finding_count, "findingsTruncated": finding_count > len(occurrence_rows), "severityCounts": severity_counts, @@ -4102,10 +4114,51 @@ def legacy_finding_matches(row: sqlite3.Row, finding: Any) -> bool: ) +def scan_source_excerpt_context( + scan: sqlite3.Row, +) -> tuple[Path | None, SourceContext | None]: + try: + target = require_scan_target_identity(scan) + except SystemExit: + return None, None + try: + selected_paths = requested_scan_paths(scan) + except (IndexError, KeyError, TypeError, ValueError): + selected_paths = [] + if not isinstance(selected_paths, list) or not all( + isinstance(path, str) for path in selected_paths + ): + selected_paths = [] + return target, source_excerpt_context(scan, target, selected_paths) + + +def finding_results( + connection: sqlite3.Connection, + scan: sqlite3.Row, + occurrences: list[sqlite3.Row], +) -> list[dict[str, Any]]: + if not occurrences: + return [] + target, source_context = scan_source_excerpt_context(scan) + return [ + finding_result( + connection, + scan, + occurrence, + target=target, + source_context=source_context, + ) + for occurrence in occurrences + ] + + def finding_result( connection: sqlite3.Connection, scan: sqlite3.Row, occurrence: sqlite3.Row, + *, + target: Path | None, + source_context: SourceContext | None, ) -> dict[str, Any]: details = bounded_finding_details(read_finding_details(occurrence["details_json"])) confidence = details.get("confidence") @@ -4114,10 +4167,6 @@ def finding_result( severity = severity if isinstance(severity, dict) else {} excerpt_locations = [] locations = [] - try: - target = require_scan_target_identity(scan) - except SystemExit: - target = None for row in connection.execute( """ SELECT relative_path, start_line, end_line, role @@ -4180,16 +4229,8 @@ def finding_result( result["knownSince"] = known_since result["knownScanIds"] = known_scan_ids result.pop("artifactPaths", None) - try: - selected_paths = requested_scan_paths(scan) - except (IndexError, KeyError, TypeError, ValueError): - selected_paths = [] - if not isinstance(selected_paths, list) or not all( - isinstance(path, str) for path in selected_paths - ): - selected_paths = [] - source_excerpt = finding_source_excerpt( - scan, target, excerpt_locations, selected_paths + source_excerpt = finding_source_excerpt_from_context( + source_context, excerpt_locations ) if source_excerpt: result["sourceExcerpt"] = source_excerpt diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_scan_start.py b/sdk/typescript/_bundled_plugin/scripts/workbench_scan_start.py index 0aa4dfd15..956869d11 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_scan_start.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_scan_start.py @@ -11,6 +11,7 @@ from collections.abc import Callable from datetime import datetime, timezone from pathlib import Path +from typing import Any # Some plugin hosts launch Python with safe-path isolation enabled. sys.path.insert(0, str(Path(__file__).resolve().parent)) @@ -166,7 +167,7 @@ def insert_running_scan( model: str | None = None, reasoning_effort: str | None = None, scan_dir: Path | None = None, - source_paths: list[str] | None = None, + source_scopes: dict[str, Any] | None = None, ) -> str: revision = target_identity[0] native_scan = scan_dir is None @@ -178,12 +179,13 @@ def insert_running_scan( dir=target_root, ) ).resolve() - source_scopes = capture_source_scopes( - target, - target_identity, - source_paths or [scope], - diff_target_kind=diff_target["kind"] if diff_target is not None else None, - ) + if source_scopes is None: + source_scopes = capture_source_scopes( + target, + target_identity, + [scope], + diff_target_kind=diff_target["kind"] if diff_target is not None else None, + ) connection.execute( """ INSERT INTO scans ( diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index 55421c8a7..e9cb17001 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -29,6 +29,7 @@ TreeEntry = tuple[str, str, str] SourceScope = dict[str, str] +SourceContext = tuple[Path, tuple[SourceScope, ...]] def normalized_path_component(value: str) -> str: @@ -196,7 +197,7 @@ def capture_source_scopes( def load_source_scopes( scan: sqlite3.Row, target: Path, selected_paths: list[str] -) -> tuple[Path, tuple[SourceScope, ...]] | None: +) -> SourceContext | None: try: saved = scan["source_scopes_json"] except (IndexError, KeyError): @@ -272,15 +273,14 @@ def source_object_for_path( return entry[2] if entry is not None and entry[1] == "file" else None -def finding_source_excerpt( +def source_excerpt_context( scan: sqlite3.Row, target: Path | None, - locations: list[dict[str, Any]], selected_paths: list[str], -) -> str | None: +) -> SourceContext | None: if scan["mode"] == "diff" and scan["diff_target_kind"] not in {"commit", "range"}: return None - if target is None or not locations or scan["target_revision"] == "unversioned": + if target is None or scan["target_revision"] == "unversioned": return None snapshot = scan["target_snapshot_digest"] if snapshot is not None and snapshot != clean_worktree_content_digest(): @@ -289,7 +289,14 @@ def finding_source_excerpt( context = load_source_scopes(scan, target, selected_paths) except (OSError, RuntimeError, SystemExit, UnicodeError, ValueError): return None - if context is None: + return context + + +def finding_source_excerpt_from_context( + context: SourceContext | None, + locations: list[dict[str, Any]], +) -> str | None: + if context is None or not locations: return None repository, scopes = context @@ -343,6 +350,18 @@ def finding_source_excerpt( return excerpt.encode("utf-8")[:MAX_BYTES].decode("utf-8", errors="ignore") +def finding_source_excerpt( + scan: sqlite3.Row, + target: Path | None, + locations: list[dict[str, Any]], + selected_paths: list[str], +) -> str | None: + return finding_source_excerpt_from_context( + source_excerpt_context(scan, target, selected_paths), + locations, + ) + + def scanned_source_text(repository: Path, object_id: str) -> str | None: try: content = local_git_bytes(repository, "cat-file", "blob", object_id) diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 06d23362a..c0c06335b 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -454,20 +454,60 @@ describe("workbench source excerpts", () => { ]); const cliScanDirectory = join(root, "cli-scan"); mkdirSync(cliScanDirectory, { mode: 0o700 }); - const cli = await workbench(root, [ - "register-cli-scan", - "--repository", + const recipe = JSON.stringify({ + config: {}, + mode: "standard", repository, - "--scan-dir", - cliScanDirectory, - "--recipe-json", - JSON.stringify({ - config: {}, - mode: "standard", + target: { kind: "paths", paths: ["src/allowed.py", "other"] }, + }); + const cliProgram = String.raw` +import argparse, json, os, sys +os.environ["CODEX_SECURITY_STATE_DIR"] = sys.argv[2] +sys.path.insert(0, sys.argv[1]) +import workbench_db + +connection = workbench_db.connect() +original_capture = workbench_db.capture_source_scopes +transaction_states = [] +def capture(*arguments, **keywords): + transaction_states.append(connection.in_transaction) + return original_capture(*arguments, **keywords) +workbench_db.capture_source_scopes = capture +try: + result = workbench_db.register_cli_scan(connection, argparse.Namespace( + archive_existing=False, + archived_scan_dir=None, + parent_scan_id=None, + recipe_json=sys.argv[5], + recipe_json_stdin=False, + registration_json_stdin=False, + repository=sys.argv[3], + scan_dir=sys.argv[4], + )) +finally: + workbench_db.capture_source_scopes = original_capture + connection.close() +result["capturedOutsideTransaction"] = transaction_states == [False] +print(json.dumps(result)) +`; + const cliResult = spawnSync( + python(), + [ + "-I", + "-B", + "-c", + cliProgram, + join(PLUGIN_ROOT, "scripts"), + join(root, "state"), repository, - target: { kind: "paths", paths: ["src/allowed.py", "other"] }, - }), - ]); + cliScanDirectory, + recipe, + ], + { encoding: "utf8" }, + ); + expect(cliResult.status, cliResult.stderr).toBe(0); + const cli = JSON.parse(cliResult.stdout) as Record; + expect(cli["capturedOutsideTransaction"]).toBe(true); const deep = await workbench(root, [ "begin-deep-scan", "--thread-id", @@ -591,25 +631,26 @@ workbench_db.finding_triage_result = lambda database, occurrence_id: None workbench_db.scan_history.finding_matches = lambda *arguments: ([], None, []) seen = [] selected_paths_seen = [] -def source_excerpt(scan, selected, locations, selected_paths): - seen.append([location["path"] for location in locations]) +authority_context = (target, ()) +def prepare_context(scan, selected, selected_paths): selected_paths_seen.append(selected_paths) - if selected_paths and locations[0]["path"] == raw_path: + return authority_context if selected_paths else None +def source_excerpt(context, locations): + seen.append([location["path"] for location in locations]) + if context is authority_context and locations[0]["path"] == raw_path: return "1 raw_path_authorized = True" return None -workbench_db.finding_source_excerpt = source_excerpt +workbench_db.source_excerpt_context = prepare_context +workbench_db.finding_source_excerpt_from_context = source_excerpt scan = {"id": "scan", "started_at": "now", "scan_dir": str(target), "scope": "."} occurrence = {"id": "occurrence", "details_json": "{}", "confidence": "high", "severity": "high", "created_at": "now", "finding_id": "finding", "remediation": "fix", "summary": "summary", "title": "title"} -finding = workbench_db.finding_result( - connection, - scan, - occurrence, -) -malformed = workbench_db.finding_result( +findings = workbench_db.finding_results(connection, scan, [occurrence, occurrence]) +finding = findings[0] +malformed = workbench_db.finding_results( connection, {**scan, "recipe_json": json.dumps({"target": {"kind": "paths", "paths": 42}})}, - occurrence, -) + [occurrence], +)[0] display_path = finding["locations"][0]["path"] print(json.dumps({ "displayBytes": len(display_path.encode()), @@ -617,7 +658,8 @@ print(json.dumps({ "excerpt": finding.get("sourceExcerpt"), "malformedExcerpt": malformed.get("sourceExcerpt"), "malformedTitle": malformed.get("title"), - "sawRawPath": seen == [[raw_path], [raw_path]], + "reusedExcerpt": findings[1].get("sourceExcerpt"), + "sawRawPath": seen == [[raw_path], [raw_path], [raw_path]], "sawSelectedPaths": selected_paths_seen == [["."], []], })) `; @@ -633,6 +675,7 @@ print(json.dumps({ excerpt: "1 raw_path_authorized = True", malformedExcerpt: null, malformedTitle: "title", + reusedExcerpt: "1 raw_path_authorized = True", sawRawPath: true, sawSelectedPaths: true, }); From 669203d230655a56c2d102ebbbbebf401026610b Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 13:54:29 +0000 Subject: [PATCH 06/26] fix(workbench): capture source authority before writes --- .../scripts/deep_scan_workbench.py | 10 +- .../_bundled_plugin/scripts/workbench_db.py | 14 ++ .../scripts/workbench_scan_start.py | 10 +- .../scripts/workbench_source_excerpt.py | 16 +- .../tests-ts/workbench-source-excerpt.test.ts | 190 ++++++------------ 5 files changed, 83 insertions(+), 157 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/deep_scan_workbench.py b/sdk/typescript/_bundled_plugin/scripts/deep_scan_workbench.py index c232c5481..77f7e9e61 100644 --- a/sdk/typescript/_bundled_plugin/scripts/deep_scan_workbench.py +++ b/sdk/typescript/_bundled_plugin/scripts/deep_scan_workbench.py @@ -776,6 +776,11 @@ def begin_deep_scan_for_target( ) target_device = serialize_filesystem_identity(target_metadata.st_dev) target_inode = serialize_filesystem_identity(target_metadata.st_ino) + source_scopes = capture_source_scopes( + target, + (revision, target_snapshot_digest, target_device, target_inode), + [scope], + ) scope_file_count = directory_snapshot_regular_file_count( target if scope == "." else target / scope ) @@ -842,11 +847,6 @@ def begin_deep_scan_for_target( scan_id = str(uuid.uuid4()) timestamp = now() target_id = ensure_security_target(connection, target_path) - source_scopes = capture_source_scopes( - target, - (revision, target_snapshot_digest, target_device, target_inode), - [scope], - ) scan_dir = Path( tempfile.mkdtemp( prefix=f"{safe_segment(revision)}_{compact_timestamp()}_", diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py index faea33a7c..9a5e2fe72 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py @@ -1334,6 +1334,12 @@ def active_scan() -> sqlite3.Row | None: diff_target, metadata=target_metadata, ) + source_scopes = capture_source_scopes( + target, + target_identity, + [scope], + diff_target_kind=diff_target["kind"] if diff_target is not None else None, + ) match_committed_diff_identity: ( Callable[[Path, dict[str, str], str], bool] | None ) = None @@ -1443,6 +1449,7 @@ def target_matches_initial_snapshot() -> bool: target_root=target_root, target_summary=target_summary, scope_file_count=scope_file_count, + source_scopes=source_scopes, timestamp=timestamp, model=args.model, reasoning_effort=args.reasoning_effort, @@ -1498,6 +1505,12 @@ def _start_prompt_driven_scan( ) diff_identity = scan_diff_identity(diff_target) target_identity = scan_target_identity(target, diff_target) + source_scopes = capture_source_scopes( + target, + target_identity, + [scope], + diff_target_kind=diff_target["kind"] if diff_target is not None else None, + ) match_committed_diff_identity: ( Callable[[Path, dict[str, str], str], bool] | None ) = None @@ -1659,6 +1672,7 @@ def target_matches_initial_snapshot() -> bool: target_root=target_root, target_summary=target_summary, scope_file_count=scope_file_count, + source_scopes=source_scopes, timestamp=timestamp, handoff_status="delivered", model=args.model, diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_scan_start.py b/sdk/typescript/_bundled_plugin/scripts/workbench_scan_start.py index 956869d11..7fd3b6f3f 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_scan_start.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_scan_start.py @@ -18,7 +18,6 @@ from filesystem_identity import serialize_filesystem_identity from finalize_scan_contract import write_scan_local_bytes from workbench_feedback import get_scan_feedback -from workbench_source_excerpt import capture_source_scopes from workbench_target import ( directory_content_digest, git_revision, @@ -162,12 +161,12 @@ def insert_running_scan( target_root: Path, target_summary: str | None, scope_file_count: int, + source_scopes: dict[str, Any], timestamp: str, handoff_status: str = "pending", model: str | None = None, reasoning_effort: str | None = None, scan_dir: Path | None = None, - source_scopes: dict[str, Any] | None = None, ) -> str: revision = target_identity[0] native_scan = scan_dir is None @@ -179,13 +178,6 @@ def insert_running_scan( dir=target_root, ) ).resolve() - if source_scopes is None: - source_scopes = capture_source_scopes( - target, - target_identity, - [scope], - diff_target_kind=diff_target["kind"] if diff_target is not None else None, - ) connection.execute( """ INSERT INTO scans ( diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index e9cb17001..7efaf8c95 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -275,12 +275,12 @@ def source_object_for_path( def source_excerpt_context( scan: sqlite3.Row, - target: Path | None, + target: Path, selected_paths: list[str], ) -> SourceContext | None: if scan["mode"] == "diff" and scan["diff_target_kind"] not in {"commit", "range"}: return None - if target is None or scan["target_revision"] == "unversioned": + if scan["target_revision"] == "unversioned": return None snapshot = scan["target_snapshot_digest"] if snapshot is not None and snapshot != clean_worktree_content_digest(): @@ -350,18 +350,6 @@ def finding_source_excerpt_from_context( return excerpt.encode("utf-8")[:MAX_BYTES].decode("utf-8", errors="ignore") -def finding_source_excerpt( - scan: sqlite3.Row, - target: Path | None, - locations: list[dict[str, Any]], - selected_paths: list[str], -) -> str | None: - return finding_source_excerpt_from_context( - source_excerpt_context(scan, target, selected_paths), - locations, - ) - - def scanned_source_text(repository: Path, object_id: str) -> str | None: try: content = local_git_bytes(repository, "cat-file", "blob", object_id) diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index c0c06335b..ac1c542e1 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -13,7 +13,6 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, test } from "bun:test"; import { BUNDLED_PLUGIN_VERSION, bootstrapPlugin } from "../src/index.js"; -import { runWorkbench } from "../src/runtime.js"; import { PLUGIN_ROOT } from "./plugin-root.js"; const temporaryRoots: string[] = []; @@ -203,22 +202,6 @@ async function upgradedPlugin(root: string) { return { predecessor, upgraded }; } -function workbench(root: string, args: string[], input?: string) { - return runWorkbench( - { - python: python(), - pluginRoot: PLUGIN_ROOT, - environment: { - ...process.env, - CODEX_SECURITY_STATE_DIR: join(root, "state"), - PYTHONDONTWRITEBYTECODE: "1", - }, - }, - args, - input, - ); -} - function collisionProbe( pluginRoot: string, fixture: { repository: string; replacement: string; revision: string }, @@ -247,11 +230,10 @@ scan = { def excerpt(path, saved=scan, selected_paths=None): if selected_paths is None: selected_paths = ["src"] - return excerpts.finding_source_excerpt( - saved, - repository, + context = excerpts.source_excerpt_context(saved, repository, selected_paths) + return excerpts.finding_source_excerpt_from_context( + context, [{"path": path, "startLine": 1, "endLine": 1, "role": "root_control"}], - selected_paths, ) original_git = excerpts.local_git_bytes blob_reads = [] @@ -395,147 +377,97 @@ describe("workbench source excerpts", () => { }); }, 60_000); - test("persists selected source authority from every scan writer", async () => { + test("persists source authority outside every writer transaction", () => { const root = realpathSync( mkdtempSync(join(tmpdir(), "codex-security-source-writers-")), ); temporaryRoots.push(root); const { repository } = ordinaryRepository(root); const scanRoot = join(root, "scans"); - const workspaceId = randomUUID(); - await workbench(root, [ - "create-workspace", - "--workspace-id", - workspaceId, - "--thread-id", - "workspace-writer", - ]); - await workbench(root, [ - "save-workspace", - "--workspace-id", - workspaceId, - "--target-path", - repository, - "--scope", - "src", - "--mode", - "standard", - ]); - const workspace = await workbench(root, [ - "start-scan", - "--workspace-id", - workspaceId, - "--scan-root", - scanRoot, - ]); - const prompt = await workbench(root, [ - "start-prompt-only-scan", - "--thread-id", - "prompt-writer", - "--target-path", - repository, - "--scope", - "src", - "--mode", - "standard", - "--scan-root", - scanRoot, - ]); - const headless = await workbench(root, [ - "start-headless-standard-scan", - "--thread-id", - "headless-writer", - "--target-path", - repository, - "--scope", - "src", - "--scan-root", - scanRoot, - ]); const cliScanDirectory = join(root, "cli-scan"); mkdirSync(cliScanDirectory, { mode: 0o700 }); - const recipe = JSON.stringify({ - config: {}, - mode: "standard", - repository, - target: { kind: "paths", paths: ["src/allowed.py", "other"] }, - }); - const cliProgram = String.raw` -import argparse, json, os, sys + const workspaceId = randomUUID(); + const program = String.raw` +import contextlib, io, json, os, sqlite3, sys os.environ["CODEX_SECURITY_STATE_DIR"] = sys.argv[2] sys.path.insert(0, sys.argv[1]) import workbench_db -connection = workbench_db.connect() -original_capture = workbench_db.capture_source_scopes +active_connection = None +current_command = None transaction_states = [] +original_connect = workbench_db.connect +original_capture = workbench_db.capture_source_scopes +original_deep_capture = workbench_db.deep_scan.capture_source_scopes +def connect(): + global active_connection + active_connection = original_connect() + return active_connection def capture(*arguments, **keywords): - transaction_states.append(connection.in_transaction) + transaction_states.append([current_command, active_connection.in_transaction]) return original_capture(*arguments, **keywords) +def deep_capture(*arguments, **keywords): + transaction_states.append([current_command, active_connection.in_transaction]) + return original_deep_capture(*arguments, **keywords) +def run(arguments): + global current_command + current_command = arguments[0] + sys.argv = ["workbench_db.py", *arguments] + output = io.StringIO() + with contextlib.redirect_stdout(output): + workbench_db.main() + return json.loads(output.getvalue()) +workbench_db.connect = connect workbench_db.capture_source_scopes = capture -try: - result = workbench_db.register_cli_scan(connection, argparse.Namespace( - archive_existing=False, - archived_scan_dir=None, - parent_scan_id=None, - recipe_json=sys.argv[5], - recipe_json_stdin=False, - registration_json_stdin=False, - repository=sys.argv[3], - scan_dir=sys.argv[4], - )) -finally: - workbench_db.capture_source_scopes = original_capture - connection.close() -result["capturedOutsideTransaction"] = transaction_states == [False] -print(json.dumps(result)) +workbench_db.deep_scan.capture_source_scopes = deep_capture + +repository, scan_root, cli_scan_dir, workspace_id = sys.argv[3:7] +run(["create-workspace", "--workspace-id", workspace_id, "--thread-id", "workspace-writer"]) +run(["save-workspace", "--workspace-id", workspace_id, "--target-path", repository, "--scope", "src", "--mode", "standard"]) +workspace = run(["start-scan", "--workspace-id", workspace_id, "--scan-root", scan_root]) +prompt = run(["start-prompt-only-scan", "--thread-id", "prompt-writer", "--target-path", repository, "--scope", "src", "--mode", "standard", "--scan-root", scan_root]) +headless = run(["start-headless-standard-scan", "--thread-id", "headless-writer", "--target-path", repository, "--scope", "src", "--scan-root", scan_root]) +recipe = json.dumps({"config": {}, "mode": "standard", "repository": repository, "target": {"kind": "paths", "paths": ["src/allowed.py", "other"]}}) +cli = run(["register-cli-scan", "--repository", repository, "--scan-dir", cli_scan_dir, "--recipe-json", recipe]) +deep = run(["begin-deep-scan", "--thread-id", "deep-writer", "--target-path", repository, "--scan-root", scan_root, "--available-parallelism", "4"]) +with sqlite3.connect(workbench_db.database_path()) as connection: + authorities = {row[0]: json.loads(row[1]) for row in connection.execute("SELECT id, source_scopes_json FROM scans")} +print(json.dumps({"authorities": authorities, "cli": cli, "deep": deep, "headless": headless, "prompt": prompt, "transactionStates": transaction_states, "workspace": workspace})) `; - const cliResult = spawnSync( + const result = spawnSync( python(), [ "-I", "-B", "-c", - cliProgram, + program, join(PLUGIN_ROOT, "scripts"), join(root, "state"), repository, + scanRoot, cliScanDirectory, - recipe, + workspaceId, ], { encoding: "utf8" }, ); - expect(cliResult.status, cliResult.stderr).toBe(0); - const cli = JSON.parse(cliResult.stdout) as Record; - expect(cli["capturedOutsideTransaction"]).toBe(true); - const deep = await workbench(root, [ - "begin-deep-scan", - "--thread-id", - "deep-writer", - "--target-path", - repository, - "--scan-root", - scanRoot, - "--available-parallelism", - "4", + expect(result.status, result.stderr).toBe(0); + const writers = JSON.parse(result.stdout) as Record; + expect(writers["transactionStates"]).toEqual([ + ["start-scan", false], + ["start-prompt-only-scan", false], + ["start-headless-standard-scan", false], + ["register-cli-scan", false], + ["begin-deep-scan", false], ]); - const database = await workbench(root, ["database-info"]); - const query = spawnSync( - python(), - [ - "-I", - "-B", - "-c", - "import json, sqlite3, sys; c = sqlite3.connect(sys.argv[1]); print(json.dumps({row[0]: json.loads(row[1]) for row in c.execute('SELECT id, source_scopes_json FROM scans')}))", - String(database["databasePath"]), - ], - { encoding: "utf8" }, - ); - expect(query.status, query.stderr).toBe(0); - const authorities = JSON.parse(query.stdout) as Record< + const authorities = writers["authorities"] as Record< string, { paths: string[]; version: number } >; + const workspace = writers["workspace"] as Record; + const prompt = writers["prompt"] as Record; + const headless = writers["headless"] as Record; + const cli = writers["cli"] as Record; + const deep = writers["deep"] as Record; const workspaceResults = workspace["results"] as Record; const promptScan = prompt["scan"] as Record; const headlessScan = headless["scan"] as Record; From 7426e4b11369768bf65a544bd0214a4f1d8e3c3e Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:01:03 +0000 Subject: [PATCH 07/26] fix(workbench): revalidate deep scan source identity --- .../scripts/deep_scan_workbench.py | 19 ++++++++++++----- .../tests-ts/workbench-source-excerpt.test.ts | 21 +++++++++++++++++-- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/deep_scan_workbench.py b/sdk/typescript/_bundled_plugin/scripts/deep_scan_workbench.py index 77f7e9e61..f10d10c67 100644 --- a/sdk/typescript/_bundled_plugin/scripts/deep_scan_workbench.py +++ b/sdk/typescript/_bundled_plugin/scripts/deep_scan_workbench.py @@ -776,9 +776,10 @@ def begin_deep_scan_for_target( ) target_device = serialize_filesystem_identity(target_metadata.st_dev) target_inode = serialize_filesystem_identity(target_metadata.st_ino) + target_identity = (revision, target_snapshot_digest, target_device, target_inode) source_scopes = capture_source_scopes( target, - (revision, target_snapshot_digest, target_device, target_inode), + target_identity, [scope], ) scope_file_count = directory_snapshot_regular_file_count( @@ -805,10 +806,18 @@ def begin_deep_scan_for_target( ) current_target = require_remediation_target(target_path) current_metadata = current_target.stat() - if (current_metadata.st_dev, current_metadata.st_ino) != ( - target_metadata.st_dev, - target_metadata.st_ino, - ): + current_revision = git_revision(current_target) + current_snapshot_digest = ( + directory_content_digest(current_target) + if current_revision == "unversioned" + else worktree_content_digest(current_target) + ) + if ( + current_revision, + current_snapshot_digest, + serialize_filesystem_identity(current_metadata.st_dev), + serialize_filesystem_identity(current_metadata.st_ino), + ) != target_identity: raise SystemExit( "The selected scan target changed while the scan was starting. Try again." ) diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index ac1c542e1..25ba942f3 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -389,12 +389,14 @@ describe("workbench source excerpts", () => { const workspaceId = randomUUID(); const program = String.raw` import contextlib, io, json, os, sqlite3, sys +from pathlib import Path os.environ["CODEX_SECURITY_STATE_DIR"] = sys.argv[2] sys.path.insert(0, sys.argv[1]) import workbench_db active_connection = None current_command = None +race_mutation = False transaction_states = [] original_connect = workbench_db.connect original_capture = workbench_db.capture_source_scopes @@ -408,7 +410,10 @@ def capture(*arguments, **keywords): return original_capture(*arguments, **keywords) def deep_capture(*arguments, **keywords): transaction_states.append([current_command, active_connection.in_transaction]) - return original_deep_capture(*arguments, **keywords) + authority = original_deep_capture(*arguments, **keywords) + if race_mutation: + (Path(repository) / "src" / "allowed.py").write_text("changed_after_capture = True\n") + return authority def run(arguments): global current_command current_command = arguments[0] @@ -430,9 +435,17 @@ headless = run(["start-headless-standard-scan", "--thread-id", "headless-writer" recipe = json.dumps({"config": {}, "mode": "standard", "repository": repository, "target": {"kind": "paths", "paths": ["src/allowed.py", "other"]}}) cli = run(["register-cli-scan", "--repository", repository, "--scan-dir", cli_scan_dir, "--recipe-json", recipe]) deep = run(["begin-deep-scan", "--thread-id", "deep-writer", "--target-path", repository, "--scan-root", scan_root, "--available-parallelism", "4"]) +race_mutation = True +try: + run(["begin-deep-scan", "--thread-id", "raced-deep-writer", "--target-path", repository, "--scan-root", scan_root, "--available-parallelism", "4"]) +except SystemExit as error: + race_error = str(error) +else: + race_error = None +(Path(repository) / "src" / "allowed.py").write_text("allowed = True\n") with sqlite3.connect(workbench_db.database_path()) as connection: authorities = {row[0]: json.loads(row[1]) for row in connection.execute("SELECT id, source_scopes_json FROM scans")} -print(json.dumps({"authorities": authorities, "cli": cli, "deep": deep, "headless": headless, "prompt": prompt, "transactionStates": transaction_states, "workspace": workspace})) +print(json.dumps({"authorities": authorities, "cli": cli, "deep": deep, "headless": headless, "prompt": prompt, "raceError": race_error, "transactionStates": transaction_states, "workspace": workspace})) `; const result = spawnSync( python(), @@ -458,7 +471,11 @@ print(json.dumps({"authorities": authorities, "cli": cli, "deep": deep, "headles ["start-headless-standard-scan", false], ["register-cli-scan", false], ["begin-deep-scan", false], + ["begin-deep-scan", false], ]); + expect(writers["raceError"]).toBe( + "The selected scan target changed while the scan was starting. Try again.", + ); const authorities = writers["authorities"] as Record< string, { paths: string[]; version: number } From 76841a9d6402fa4e62938175f0fa5a7e5970fdca Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:26:00 +0000 Subject: [PATCH 08/26] fix(workbench): seal excerpt tree authority --- .../scripts/workbench_source_excerpt.py | 8 ++- .../tests-ts/workbench-source-excerpt.test.ts | 61 ++++++++++++++++++- 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index 7efaf8c95..c969f7993 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -33,7 +33,7 @@ def normalized_path_component(value: str) -> str: - return normalize("NFC", normalize("NFD", value).casefold()) + return normalize("NFC", normalize("NFD", value).casefold()).rstrip(" .") def relative_path(value: str) -> PurePosixPath | None: @@ -167,6 +167,7 @@ def capture_source_scopes( if context is None: return authority repository, tree = context + authority["targetTree"] = tree captured: set[str] = set() for requested in paths: parsed = relative_path(requested) @@ -206,11 +207,14 @@ def load_source_scopes( return None metadata = json.loads(saved) records = metadata.get("paths") if isinstance(metadata, dict) else None + expected_tree = metadata.get("targetTree") if isinstance(metadata, dict) else None if ( not isinstance(metadata, dict) or metadata.get("version") != 1 or not isinstance(records, list) or not records + or not isinstance(expected_tree, str) + or not OBJECT_ID.fullmatch(expected_tree) ): return None expected = { @@ -224,6 +228,8 @@ def load_source_scopes( if context is None: return None repository, tree = context + if tree != expected_tree: + return None scopes: list[SourceScope] = [] seen: set[str] = set() for record in records: diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 25ba942f3..737b32016 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -83,6 +83,8 @@ function collisionRepository(root: string): { ["100644", "blob", blob("case_lower = True\n"), "lower.py"], ["100644", "blob", blob("unicode_composed = True\n"), "é.py"], ["100644", "blob", blob("unicode_decomposed = True\n"), "é.py"], + ["100644", "blob", blob("plain_name = True\n"), "trailing.py"], + ["100644", "blob", blob("trailing_dot = True\n"), "trailing.py."], ]); const upperScope = tree([ ["100644", "blob", blob("selected_scope = True\n"), "selected.py"], @@ -118,6 +120,7 @@ function collisionRepository(root: string): { writeFileSync(join(repository, "src", "allowed.py"), "allowed = True\n"); writeFileSync(join(repository, "src", "LOWER.py"), "case_upper = True\n"); writeFileSync(join(repository, "src", "é.py"), "unicode_composed = True\n"); + writeFileSync(join(repository, "src", "trailing.py"), "plain_name = True\n"); writeFileSync( join(repository, "Scope", "selected.py"), "selected_scope = True\n", @@ -246,16 +249,59 @@ allowed = excerpt("src/allowed.py") before = len(blob_reads) collisions = { path: excerpt(path) - for path in ("src/LOWER.py", "src/lower.py", "src/é.py", "src/é.py") + for path in ( + "src/LOWER.py", + "src/lower.py", + "src/é.py", + "src/é.py", + "src/trailing.py", + "src/trailing.py.", + ) } collision_blob_reads = blob_reads[before:] outside = excerpt("outside.py") before = len(blob_reads) broadened = excerpt( "outside.py", - {**scan, "source_scopes_json": json.dumps({"version": 1, "paths": ["."]})}, + {**scan, "source_scopes_json": json.dumps({**authority, "paths": ["."]})}, ) broadened_blob_reads = blob_reads[before:] +subtarget = repository / "src" +subtarget_metadata = subtarget.stat() +subtarget_authority = excerpts.capture_source_scopes( + subtarget, + ( + revision, + clean_worktree_content_digest(), + subtarget_metadata.st_dev, + subtarget_metadata.st_ino, + ), + ["."], +) +subprocess.run(["git", "-C", str(subtarget), "init", "-q"], check=True) +outer_git_dir = Path( + subprocess.check_output( + ["git", "-C", str(repository), "rev-parse", "--absolute-git-dir"], + text=True, + ).strip() +) +alternates = subtarget / ".git" / "objects" / "info" / "alternates" +alternates.write_text(str(outer_git_dir / "objects") + "\n") +subprocess.run( + ["git", "-C", str(subtarget), "update-ref", "refs/heads/main", revision], + check=True, +) +subtarget_scan = { + **scan, + "source_scopes_json": json.dumps(subtarget_authority), +} +nested_context = excerpts.source_excerpt_context(subtarget_scan, subtarget, ["."]) +before = len(blob_reads) +nested_excerpt = excerpts.finding_source_excerpt_from_context( + nested_context, + [{"path": "outside.py", "startLine": 1, "endLine": 1, "role": "root_control"}], +) +nested_blob_reads = blob_reads[before:] subprocess.run( ["git", "-C", str(repository), "update-ref", f"refs/replace/{revision}", replacement], check=True, @@ -311,6 +357,9 @@ print(json.dumps({ "legacy": legacy, "malformedRevision": malformed_revision, "mutable": {"excerpts": mutable, "gitCalls": len(git_calls)}, + "nestedBlobReads": nested_blob_reads, + "nestedExcerpt": nested_excerpt, + "subtargetPaths": subtarget_authority["paths"], "outside": outside, "pathCollisionPaths": len( excerpts.capture_source_scopes(repository, identity, ["Scope"])["paths"] @@ -357,6 +406,8 @@ describe("workbench source excerpts", () => { "src/lower.py": null, "src/é.py": null, "src/é.py": null, + "src/trailing.py": null, + "src/trailing.py.": null, }, duplicatePaths: 1, immutable: { @@ -370,6 +421,9 @@ describe("workbench source excerpts", () => { excerpts: { working_tree: null, None: null }, gitCalls: 0, }, + nestedBlobReads: [], + nestedExcerpt: null, + subtargetPaths: ["."], outside: null, pathCollisionPaths: 0, replaced: null, @@ -478,7 +532,7 @@ print(json.dumps({"authorities": authorities, "cli": cli, "deep": deep, "headles ); const authorities = writers["authorities"] as Record< string, - { paths: string[]; version: number } + { paths: string[]; targetTree: string; version: number } >; const workspace = writers["workspace"] as Record; const prompt = writers["prompt"] as Record; @@ -500,6 +554,7 @@ print(json.dumps({"authorities": authorities, "cli": cli, "deep": deep, "headles for (const [writer, scanId, paths] of expected) { const authority = authorities[String(scanId)]; expect(authority?.version, writer).toBe(1); + expect(authority?.targetTree, writer).toMatch(/^[0-9a-f]{40,64}$/); expect(authority?.paths, writer).toEqual([...paths]); } }, 60_000); From 712438a95fc356cd458da5580aeeea96e27c7884 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:54:05 +0000 Subject: [PATCH 09/26] fix(workbench): index selected excerpt scopes --- .../scripts/workbench_source_excerpt.py | 30 ++++++++++---- .../tests-ts/workbench-source-excerpt.test.ts | 41 +++++++++++++++++++ 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index c969f7993..e9cff08f3 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -29,7 +29,7 @@ TreeEntry = tuple[str, str, str] SourceScope = dict[str, str] -SourceContext = tuple[Path, tuple[SourceScope, ...]] +SourceContext = tuple[Path, dict[str, SourceScope]] def normalized_path_component(value: str) -> str: @@ -230,16 +230,14 @@ def load_source_scopes( repository, tree = context if tree != expected_tree: return None - scopes: list[SourceScope] = [] - seen: set[str] = set() + scopes: dict[str, SourceScope] = {} for record in records: path = relative_path(record) if isinstance(record, str) else None if path is None: return None selected = path.as_posix() - if selected != record or selected not in expected or selected in seen: + if selected != record or selected not in expected or selected in scopes: return None - seen.add(selected) entry = tree_path(repository, tree, selected) if entry is None or entry[1] not in {"file", "directory"}: return None @@ -248,8 +246,8 @@ def load_source_scopes( "kind": entry[1], "objectId": entry[2], } - scopes.append(scope) - return repository, tuple(scopes) + scopes[selected] = scope + return repository, scopes def source_object_for_path( @@ -279,6 +277,22 @@ def source_object_for_path( return entry[2] if entry is not None and entry[1] == "file" else None +def source_scopes_for_path( + scopes: dict[str, SourceScope], value: str +) -> tuple[SourceScope, ...]: + path = relative_path(value) + if path is None: + return () + return tuple( + scope + for length in range(len(path.parts), -1, -1) + if ( + scope := scopes.get(PurePosixPath(*path.parts[:length]).as_posix()) + ) + is not None + ) + + def source_excerpt_context( scan: sqlite3.Row, target: Path, @@ -322,7 +336,7 @@ def finding_source_excerpt_from_context( object_id = next( ( candidate - for scope in scopes + for scope in source_scopes_for_path(scopes, path) if ( candidate := source_object_for_path(repository, path, scope) ) diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 737b32016..00de4188a 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -345,6 +345,41 @@ immutable = { ) for kind in ("commit", "range") } +large_paths = [str(index) for index in range(20_000)] +large_tree = "0" * 40 +large_scan = { + **scan, + "source_scopes_json": json.dumps( + {"version": 1, "paths": large_paths, "targetTree": large_tree} + ), +} +large_recipe_bytes = len( + json.dumps( + {"target": {"kind": "paths", "paths": large_paths}}, separators=(",", ":") + ).encode() +) +original_target_tree = excerpts.target_tree +original_tree_path = excerpts.tree_path +original_source_object = excerpts.source_object_for_path +scope_checks = 0 +def counted_source_object(*arguments, **kwargs): + global scope_checks + scope_checks += 1 + return original_source_object(*arguments, **kwargs) +excerpts.target_tree = lambda *_: (repository, large_tree) +excerpts.tree_path = lambda _, __, value: (value, "file", "1" * 40) +excerpts.source_object_for_path = counted_source_object +try: + large_context = excerpts.source_excerpt_context( + large_scan, repository, large_paths + ) + large_excerpt = excerpts.finding_source_excerpt_from_context( + large_context, [{"path": "unmatched/path.py", "startLine": 1}] + ) +finally: + excerpts.target_tree = original_target_tree + excerpts.tree_path = original_tree_path + excerpts.source_object_for_path = original_source_object print(json.dumps({ "allowed": allowed, "broadened": broadened, @@ -353,6 +388,9 @@ print(json.dumps({ "collisions": collisions, "duplicatePaths": len(authority["paths"]), "immutable": immutable, + "largeExcerpt": large_excerpt, + "largeRecipeFits": large_recipe_bytes < 256 * 1024, + "largeScopeChecks": scope_checks, "invalid": invalid, "legacy": legacy, "malformedRevision": malformed_revision, @@ -414,6 +452,9 @@ describe("workbench source excerpts", () => { commit: expect.stringContaining("allowed = True"), range: expect.stringContaining("allowed = True"), }, + largeExcerpt: null, + largeRecipeFits: true, + largeScopeChecks: 0, invalid: null, legacy: null, malformedRevision: null, From 23d604748e65a91e4b346ba28e21bbf6705cc973 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 15:16:11 +0000 Subject: [PATCH 10/26] fix(workbench): resolve excerpt scopes on demand --- .../scripts/workbench_source_excerpt.py | 84 +++++++++++-------- .../tests-ts/workbench-source-excerpt.test.ts | 54 ++++++++++-- 2 files changed, 96 insertions(+), 42 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index e9cff08f3..b31da3e45 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -9,6 +9,7 @@ import sqlite3 import stat import sys +from dataclasses import dataclass, field from functools import cache from pathlib import Path, PurePosixPath from typing import Any @@ -28,8 +29,15 @@ OBJECT_ID = re.compile(r"(?:[0-9a-f]{40}|[0-9a-f]{64})\Z") TreeEntry = tuple[str, str, str] -SourceScope = dict[str, str] -SourceContext = tuple[Path, dict[str, SourceScope]] + + +@dataclass +class SourceScopeIndex: + scope: str | None = None + children: dict[str, SourceScopeIndex] = field(default_factory=dict) + + +SourceContext = tuple[Path, str, SourceScopeIndex] def normalized_path_component(value: str) -> str: @@ -166,7 +174,7 @@ def capture_source_scopes( context = target_tree(target, revision) if context is None: return authority - repository, tree = context + _, tree = context authority["targetTree"] = tree captured: set[str] = set() for requested in paths: @@ -174,18 +182,13 @@ def capture_source_scopes( selected_path = safe_source_path(target, requested) if parsed is None or selected_path is None: continue - entry = tree_path(repository, tree, requested) - if entry is None or entry[1] not in {"file", "directory"}: - continue raw_selected = target / parsed.as_posix() try: metadata = raw_selected.lstat() except OSError: continue ordinary = stat.S_ISDIR(metadata.st_mode) or stat.S_ISREG(metadata.st_mode) - if not ordinary or (entry[1] == "directory") != stat.S_ISDIR( - metadata.st_mode - ): + if not ordinary: continue selected = parsed.as_posix() if selected not in captured: @@ -230,7 +233,8 @@ def load_source_scopes( repository, tree = context if tree != expected_tree: return None - scopes: dict[str, SourceScope] = {} + scopes: set[str] = set() + index = SourceScopeIndex() for record in records: path = relative_path(record) if isinstance(record, str) else None if path is None: @@ -238,59 +242,63 @@ def load_source_scopes( selected = path.as_posix() if selected != record or selected not in expected or selected in scopes: return None - entry = tree_path(repository, tree, selected) - if entry is None or entry[1] not in {"file", "directory"}: - return None - scope = { - "path": selected, - "kind": entry[1], - "objectId": entry[2], - } - scopes[selected] = scope - return repository, scopes + scopes.add(selected) + node = index + for component in path.parts: + node = node.children.setdefault(component, SourceScopeIndex()) + node.scope = selected + return repository, tree, index def source_object_for_path( repository: Path, + tree: str, value: str, - scope: SourceScope, + scope: str, ) -> str | None: path = relative_path(value) if path is None: return None - scope_path = PurePosixPath(scope["path"]) + scope_path = PurePosixPath(scope) scope_length = len(scope_path.parts) if len(path.parts) < scope_length: return None if path.parts[:scope_length] != scope_path.parts: return None suffix = path.parts[scope_length:] - if scope["kind"] == "file": - return scope["objectId"] if not suffix else None + selected = tree_path(repository, tree, scope) + if selected is None or selected[1] not in {"file", "directory"}: + return None + if selected[1] == "file": + return selected[2] if not suffix else None if not suffix: return None entry = tree_path( repository, - scope["objectId"], + selected[2], PurePosixPath(*suffix).as_posix(), ) return entry[2] if entry is not None and entry[1] == "file" else None def source_scopes_for_path( - scopes: dict[str, SourceScope], value: str -) -> tuple[SourceScope, ...]: + index: SourceScopeIndex, value: str +) -> tuple[str, ...]: path = relative_path(value) if path is None: return () - return tuple( - scope - for length in range(len(path.parts), -1, -1) - if ( - scope := scopes.get(PurePosixPath(*path.parts[:length]).as_posix()) - ) - is not None - ) + scopes: list[str] = [] + node = index + if node.scope is not None: + scopes.append(node.scope) + for component in path.parts: + child = node.children.get(component) + if child is None: + break + node = child + if node.scope is not None: + scopes.append(node.scope) + return tuple(reversed(scopes)) def source_excerpt_context( @@ -318,7 +326,7 @@ def finding_source_excerpt_from_context( ) -> str | None: if context is None or not locations: return None - repository, scopes = context + repository, tree, scopes = context location = next( ( @@ -338,7 +346,9 @@ def finding_source_excerpt_from_context( candidate for scope in source_scopes_for_path(scopes, path) if ( - candidate := source_object_for_path(repository, path, scope) + candidate := source_object_for_path( + repository, tree, path, scope + ) ) is not None ), diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 00de4188a..ee17f5355 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -266,6 +266,18 @@ broadened = excerpt( {**scan, "source_scopes_json": json.dumps({**authority, "paths": ["."]})}, ) broadened_blob_reads = blob_reads[before:] +scope_collision_authority = excerpts.capture_source_scopes( + repository, identity, ["Scope"] +) +scope_collision_scan = { + **scan, + "source_scopes_json": json.dumps(scope_collision_authority), +} +before = len(blob_reads) +scope_collision_excerpt = excerpt( + "Scope/selected.py", scope_collision_scan, ["Scope"] +) +scope_collision_blob_reads = blob_reads[before:] subtarget = repository / "src" subtarget_metadata = subtarget.stat() subtarget_authority = excerpts.capture_source_scopes( @@ -362,17 +374,41 @@ original_target_tree = excerpts.target_tree original_tree_path = excerpts.tree_path original_source_object = excerpts.source_object_for_path scope_checks = 0 +tree_path_checks = 0 def counted_source_object(*arguments, **kwargs): global scope_checks scope_checks += 1 return original_source_object(*arguments, **kwargs) +def counted_tree_path(_, __, value): + global tree_path_checks + tree_path_checks += 1 + return (value, "file", "1" * 40) excerpts.target_tree = lambda *_: (repository, large_tree) -excerpts.tree_path = lambda _, __, value: (value, "file", "1" * 40) +excerpts.tree_path = counted_tree_path excerpts.source_object_for_path = counted_source_object try: large_context = excerpts.source_excerpt_context( large_scan, repository, large_paths ) + original_pure_path = excerpts.PurePosixPath + path_parses = 0 + def one_path_parse(*arguments): + global path_parses + path_parses += 1 + if path_parses > 1: + raise RuntimeError("source scope lookup rebuilt the path") + return original_pure_path(*arguments) + excerpts.PurePosixPath = one_path_parse + try: + excerpts.source_scopes_for_path( + large_context[2], + "/".join(["nested"] * 20_000 + ["file.py"]), + ) + deep_lookup_linear = True + except RuntimeError: + deep_lookup_linear = False + finally: + excerpts.PurePosixPath = original_pure_path large_excerpt = excerpts.finding_source_excerpt_from_context( large_context, [{"path": "unmatched/path.py", "startLine": 1}] ) @@ -386,11 +422,14 @@ print(json.dumps({ "broadenedBlobReads": broadened_blob_reads, "collisionBlobReads": collision_blob_reads, "collisions": collisions, + "deepLookupLinear": deep_lookup_linear, + "deepPathParses": path_parses, "duplicatePaths": len(authority["paths"]), "immutable": immutable, "largeExcerpt": large_excerpt, "largeRecipeFits": large_recipe_bytes < 256 * 1024, "largeScopeChecks": scope_checks, + "largeTreePathChecks": tree_path_checks, "invalid": invalid, "legacy": legacy, "malformedRevision": malformed_revision, @@ -399,9 +438,9 @@ print(json.dumps({ "nestedExcerpt": nested_excerpt, "subtargetPaths": subtarget_authority["paths"], "outside": outside, - "pathCollisionPaths": len( - excerpts.capture_source_scopes(repository, identity, ["Scope"])["paths"] - ), + "pathCollisionBlobReads": scope_collision_blob_reads, + "pathCollisionExcerpt": scope_collision_excerpt, + "pathCollisionPaths": len(scope_collision_authority["paths"]), "replaced": replaced, "replacementBlobReads": replacement_blob_reads, })) @@ -447,6 +486,8 @@ describe("workbench source excerpts", () => { "src/trailing.py": null, "src/trailing.py.": null, }, + deepLookupLinear: true, + deepPathParses: 1, duplicatePaths: 1, immutable: { commit: expect.stringContaining("allowed = True"), @@ -455,6 +496,7 @@ describe("workbench source excerpts", () => { largeExcerpt: null, largeRecipeFits: true, largeScopeChecks: 0, + largeTreePathChecks: 0, invalid: null, legacy: null, malformedRevision: null, @@ -466,7 +508,9 @@ describe("workbench source excerpts", () => { nestedExcerpt: null, subtargetPaths: ["."], outside: null, - pathCollisionPaths: 0, + pathCollisionBlobReads: [], + pathCollisionExcerpt: null, + pathCollisionPaths: 1, replaced: null, replacementBlobReads: [], }); From 238a16b91953040bcc13fd40044082e8b3d97acf Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 15:22:49 +0000 Subject: [PATCH 11/26] refactor(workbench): select deepest excerpt scope --- .../scripts/workbench_source_excerpt.py | 31 ++++++------------- .../tests-ts/workbench-source-excerpt.test.ts | 7 +---- 2 files changed, 11 insertions(+), 27 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index b31da3e45..e3e3d0b0a 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -281,24 +281,20 @@ def source_object_for_path( return entry[2] if entry is not None and entry[1] == "file" else None -def source_scopes_for_path( - index: SourceScopeIndex, value: str -) -> tuple[str, ...]: +def source_scope_for_path(index: SourceScopeIndex, value: str) -> str | None: path = relative_path(value) if path is None: - return () - scopes: list[str] = [] + return None node = index - if node.scope is not None: - scopes.append(node.scope) + scope = node.scope for component in path.parts: child = node.children.get(component) if child is None: break node = child if node.scope is not None: - scopes.append(node.scope) - return tuple(reversed(scopes)) + scope = node.scope + return scope def source_excerpt_context( @@ -341,18 +337,11 @@ def finding_source_excerpt_from_context( if not isinstance(path, str) or not isinstance(start_line, int): return None try: - object_id = next( - ( - candidate - for scope in source_scopes_for_path(scopes, path) - if ( - candidate := source_object_for_path( - repository, tree, path, scope - ) - ) - is not None - ), - None, + scope = source_scope_for_path(scopes, path) + object_id = ( + source_object_for_path(repository, tree, path, scope) + if scope is not None + else None ) except (OSError, RuntimeError, SystemExit, UnicodeError, ValueError): return None diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index ee17f5355..3aeffbbf0 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -400,13 +400,10 @@ try: return original_pure_path(*arguments) excerpts.PurePosixPath = one_path_parse try: - excerpts.source_scopes_for_path( + excerpts.source_scope_for_path( large_context[2], "/".join(["nested"] * 20_000 + ["file.py"]), ) - deep_lookup_linear = True - except RuntimeError: - deep_lookup_linear = False finally: excerpts.PurePosixPath = original_pure_path large_excerpt = excerpts.finding_source_excerpt_from_context( @@ -422,7 +419,6 @@ print(json.dumps({ "broadenedBlobReads": broadened_blob_reads, "collisionBlobReads": collision_blob_reads, "collisions": collisions, - "deepLookupLinear": deep_lookup_linear, "deepPathParses": path_parses, "duplicatePaths": len(authority["paths"]), "immutable": immutable, @@ -486,7 +482,6 @@ describe("workbench source excerpts", () => { "src/trailing.py": null, "src/trailing.py.": null, }, - deepLookupLinear: true, deepPathParses: 1, duplicatePaths: 1, immutable: { From b2a852c8e79ac3a3d425bbc33d70840f58b845a8 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 15:29:20 +0000 Subject: [PATCH 12/26] refactor(workbench): resolve excerpt path once --- .../scripts/workbench_source_excerpt.py | 36 ++----------------- .../tests-ts/workbench-source-excerpt.test.ts | 10 ------ 2 files changed, 3 insertions(+), 43 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index e3e3d0b0a..42a094f83 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -250,37 +250,6 @@ def load_source_scopes( return repository, tree, index -def source_object_for_path( - repository: Path, - tree: str, - value: str, - scope: str, -) -> str | None: - path = relative_path(value) - if path is None: - return None - scope_path = PurePosixPath(scope) - scope_length = len(scope_path.parts) - if len(path.parts) < scope_length: - return None - if path.parts[:scope_length] != scope_path.parts: - return None - suffix = path.parts[scope_length:] - selected = tree_path(repository, tree, scope) - if selected is None or selected[1] not in {"file", "directory"}: - return None - if selected[1] == "file": - return selected[2] if not suffix else None - if not suffix: - return None - entry = tree_path( - repository, - selected[2], - PurePosixPath(*suffix).as_posix(), - ) - return entry[2] if entry is not None and entry[1] == "file" else None - - def source_scope_for_path(index: SourceScopeIndex, value: str) -> str | None: path = relative_path(value) if path is None: @@ -338,9 +307,10 @@ def finding_source_excerpt_from_context( return None try: scope = source_scope_for_path(scopes, path) + selected = tree_path(repository, tree, path) if scope is not None else None object_id = ( - source_object_for_path(repository, tree, path, scope) - if scope is not None + selected[2] + if selected is not None and selected[1] == "file" else None ) except (OSError, RuntimeError, SystemExit, UnicodeError, ValueError): diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 3aeffbbf0..1a8a4a139 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -372,20 +372,13 @@ large_recipe_bytes = len( ) original_target_tree = excerpts.target_tree original_tree_path = excerpts.tree_path -original_source_object = excerpts.source_object_for_path -scope_checks = 0 tree_path_checks = 0 -def counted_source_object(*arguments, **kwargs): - global scope_checks - scope_checks += 1 - return original_source_object(*arguments, **kwargs) def counted_tree_path(_, __, value): global tree_path_checks tree_path_checks += 1 return (value, "file", "1" * 40) excerpts.target_tree = lambda *_: (repository, large_tree) excerpts.tree_path = counted_tree_path -excerpts.source_object_for_path = counted_source_object try: large_context = excerpts.source_excerpt_context( large_scan, repository, large_paths @@ -412,7 +405,6 @@ try: finally: excerpts.target_tree = original_target_tree excerpts.tree_path = original_tree_path - excerpts.source_object_for_path = original_source_object print(json.dumps({ "allowed": allowed, "broadened": broadened, @@ -424,7 +416,6 @@ print(json.dumps({ "immutable": immutable, "largeExcerpt": large_excerpt, "largeRecipeFits": large_recipe_bytes < 256 * 1024, - "largeScopeChecks": scope_checks, "largeTreePathChecks": tree_path_checks, "invalid": invalid, "legacy": legacy, @@ -490,7 +481,6 @@ describe("workbench source excerpts", () => { }, largeExcerpt: null, largeRecipeFits: true, - largeScopeChecks: 0, largeTreePathChecks: 0, invalid: null, legacy: null, From f3564401de6a39f670a41eff4fa0143924862861 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 15:35:43 +0000 Subject: [PATCH 13/26] refactor(workbench): store scope membership only --- .../scripts/workbench_source_excerpt.py | 32 +++++++++++-------- .../tests-ts/workbench-source-excerpt.test.ts | 2 +- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index 42a094f83..a0e32b7c0 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -33,7 +33,7 @@ @dataclass class SourceScopeIndex: - scope: str | None = None + selected: bool = False children: dict[str, SourceScopeIndex] = field(default_factory=dict) @@ -233,37 +233,38 @@ def load_source_scopes( repository, tree = context if tree != expected_tree: return None - scopes: set[str] = set() index = SourceScopeIndex() for record in records: path = relative_path(record) if isinstance(record, str) else None if path is None: return None selected = path.as_posix() - if selected != record or selected not in expected or selected in scopes: + if selected != record or selected not in expected: return None - scopes.add(selected) node = index for component in path.parts: node = node.children.setdefault(component, SourceScopeIndex()) - node.scope = selected + if node.selected: + return None + node.selected = True return repository, tree, index -def source_scope_for_path(index: SourceScopeIndex, value: str) -> str | None: +def source_path_is_selected(index: SourceScopeIndex, value: str) -> bool: path = relative_path(value) if path is None: - return None + return False node = index - scope = node.scope + if node.selected: + return True for component in path.parts: child = node.children.get(component) if child is None: - break + return False node = child - if node.scope is not None: - scope = node.scope - return scope + if node.selected: + return True + return False def source_excerpt_context( @@ -306,8 +307,11 @@ def finding_source_excerpt_from_context( if not isinstance(path, str) or not isinstance(start_line, int): return None try: - scope = source_scope_for_path(scopes, path) - selected = tree_path(repository, tree, path) if scope is not None else None + selected = ( + tree_path(repository, tree, path) + if source_path_is_selected(scopes, path) + else None + ) object_id = ( selected[2] if selected is not None and selected[1] == "file" diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 1a8a4a139..95d5c17a8 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -393,7 +393,7 @@ try: return original_pure_path(*arguments) excerpts.PurePosixPath = one_path_parse try: - excerpts.source_scope_for_path( + excerpts.source_path_is_selected( large_context[2], "/".join(["nested"] * 20_000 + ["file.py"]), ) From 1e48917a0896ce633de862efdfbde94350efa1c7 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:13:24 +0000 Subject: [PATCH 14/26] fix(workbench): preserve excerpt scope semantics --- .../_bundled_plugin/scripts/workbench_db.py | 13 +- .../scripts/workbench_source_excerpt.py | 225 +++++++++++++----- .../tests-ts/workbench-source-excerpt.test.ts | 137 +++++++++-- 3 files changed, 292 insertions(+), 83 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py index 9a5e2fe72..0997abe67 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py @@ -4191,11 +4191,16 @@ def finding_result( """, (occurrence["id"], FINDING_LOCATIONS_LIMIT), ): + role = ( + bounded_output_text(row["role"], FINDING_LOCATION_ROLE_BYTES) + if row["role"] is not None + else None + ) excerpt_locations.append( { "endLine": row["end_line"], "path": row["relative_path"], - "role": row["role"], + "role": role, "startLine": row["start_line"], } ) @@ -4203,11 +4208,7 @@ def finding_result( location = { "endLine": row["end_line"], "path": bounded_output_text(row["relative_path"], FINDING_LOCATION_PATH_BYTES), - "role": ( - bounded_output_text(row["role"], FINDING_LOCATION_ROLE_BYTES) - if row["role"] is not None - else None - ), + "role": role, "startLine": row["start_line"], } if absolute_path is not None: diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index a0e32b7c0..e6659b1fa 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -8,15 +8,16 @@ import re import sqlite3 import stat +import subprocess import sys from dataclasses import dataclass, field -from functools import cache from pathlib import Path, PurePosixPath -from typing import Any +from typing import IO, Any from unicodedata import normalize # Some plugin hosts launch Python with safe-path isolation enabled. sys.path.insert(0, str(Path(__file__).resolve().parent)) +from workbench_constants import GIT_REPOSITORY_ENVIRONMENT from workbench_target import ( clean_worktree_content_digest, git_bytes, @@ -33,7 +34,7 @@ @dataclass class SourceScopeIndex: - selected: bool = False + kind: str | None = None children: dict[str, SourceScopeIndex] = field(default_factory=dict) @@ -73,58 +74,139 @@ def local_git_bytes(repository: Path, *arguments: str) -> bytes | None: ) -@cache -def tree_entries(repository: Path, tree: str) -> dict[str, tuple[TreeEntry, ...]] | None: - content = local_git_bytes(repository, "ls-tree", "-z", tree) - if content is None: - return None - entries: dict[str, list[TreeEntry]] = {} - for record in content.split(b"\0"): - if not record: - continue - metadata, separator, name = record.partition(b"\t") - fields = metadata.split(b" ") - if not separator or len(fields) != 3: - return None - mode, object_type, raw_object = fields - try: - object_id = raw_object.decode("ascii") - except UnicodeDecodeError: - return None - if not OBJECT_ID.fullmatch(object_id): +def read_tree_object( + requests: IO[bytes], responses: IO[bytes], object_id: str +) -> bytes: + encoded_object = object_id.encode("ascii") + requests.write(encoded_object + b"\0") + requests.flush() + header = responses.readline() + if not header.endswith(b"\n"): + raise ValueError("unterminated batch header") + fields = header[:-1].split(b" ") + if ( + len(fields) != 3 + or fields[0] != encoded_object + or fields[1] != b"tree" + or not fields[2].isdigit() + ): + raise ValueError("invalid tree response") + remaining = int(fields[2]) + chunks: list[bytes] = [] + while remaining: + chunk = responses.read(min(1024 * 1024, remaining)) + if not chunk: + raise ValueError("truncated tree response") + chunks.append(chunk) + remaining -= len(chunk) + if responses.read(1) != b"\n": + raise ValueError("missing tree terminator") + return b"".join(chunks) + + +def matching_tree_entries( + content: bytes, name: str, object_id_bytes: int +) -> tuple[TreeEntry, ...] | None: + expected_name = normalized_path_component(name) + matches = [] + cursor = 0 + while cursor < len(content): + mode_end = content.find(b" ", cursor) + name_end = content.find(b"\0", mode_end + 1) + object_start = name_end + 1 + object_end = object_start + object_id_bytes + if ( + mode_end <= cursor + or name_end < mode_end + 1 + or object_end > len(content) + ): return None + mode = content[cursor:mode_end] + decoded_name = os.fsdecode(content[mode_end + 1 : name_end]) kind = ( "directory" - if mode == b"040000" and object_type == b"tree" + if mode in {b"40000", b"040000"} else "file" - if mode in {b"100644", b"100755"} and object_type == b"blob" + if mode in {b"100644", b"100755"} else "other" ) - decoded_name = os.fsdecode(name) - entries.setdefault(normalized_path_component(decoded_name), []).append( - (decoded_name, kind, object_id) - ) - return {name: tuple(matches) for name, matches in entries.items()} + if normalized_path_component(decoded_name) == expected_name: + matches.append((decoded_name, kind, content[object_start:object_end].hex())) + cursor = object_end + return tuple(matches) -def tree_path(repository: Path, tree: str, value: str) -> TreeEntry | None: +def tree_path( + repository: Path, + tree: str, + value: str, + *, + selected_kinds: dict[int, str] | None = None, +) -> TreeEntry | None: path = relative_path(value) if path is None: return None + if selected_kinds is not None and selected_kinds.get(0, "directory") != "directory": + return None kind, object_id = "directory", tree - for name in path.parts: - if kind != "directory": - return None - aliases = (tree_entries(repository, object_id) or {}).get( - normalized_path_component(name), () - ) - # The normalized name must be unique before an exact spelling can win. - if len(aliases) != 1: - return None - entry = next((candidate for candidate in aliases if candidate[0] == name), None) - if entry is None: - return None - _, kind, object_id = entry + if not path.parts: + return path.as_posix(), kind, object_id + + environment = os.environ.copy() + for variable in GIT_REPOSITORY_ENVIRONMENT: + environment.pop(variable, None) + environment["GIT_ALLOW_PROTOCOL"] = "" + environment["GIT_LITERAL_PATHSPECS"] = "1" + environment["GIT_NO_LAZY_FETCH"] = "1" + command = [ + "git", + "-c", + "core.fsmonitor=false", + "-c", + "i18n.logOutputEncoding=UTF-8", + "-C", + str(repository), + "--no-replace-objects", + "cat-file", + "--batch", + "-z", + ] + try: + with subprocess.Popen( + command, + env=environment, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + ) as process: + if process.stdin is None or process.stdout is None: + return None + for depth, name in enumerate(path.parts, start=1): + if kind != "directory": + return None + content = read_tree_object(process.stdin, process.stdout, object_id) + aliases = matching_tree_entries(content, name, len(object_id) // 2) + # The normalized name must be unique before an exact spelling can win. + if aliases is None or len(aliases) != 1: + return None + entry = next( + (candidate for candidate in aliases if candidate[0] == name), + None, + ) + if entry is None: + return None + _, kind, object_id = entry + if ( + selected_kinds is not None + and (expected_kind := selected_kinds.get(depth)) is not None + and kind != expected_kind + ): + return None + process.stdin.close() + if process.wait() != 0: + return None + except (OSError, ValueError): + return None return path.as_posix(), kind, object_id @@ -187,13 +269,19 @@ def capture_source_scopes( metadata = raw_selected.lstat() except OSError: continue - ordinary = stat.S_ISDIR(metadata.st_mode) or stat.S_ISREG(metadata.st_mode) - if not ordinary: + kind = ( + "directory" + if stat.S_ISDIR(metadata.st_mode) + else "file" + if stat.S_ISREG(metadata.st_mode) + else None + ) + if kind is None: continue selected = parsed.as_posix() if selected not in captured: captured.add(selected) - authority["paths"].append(selected) + authority["paths"].append({"kind": kind, "path": selected}) except (OSError, RuntimeError, SystemExit, UnicodeError, ValueError): return {"version": 1, "paths": []} return authority @@ -235,36 +323,50 @@ def load_source_scopes( return None index = SourceScopeIndex() for record in records: - path = relative_path(record) if isinstance(record, str) else None + if not isinstance(record, dict) or set(record) != {"kind", "path"}: + return None + kind = record.get("kind") + saved_path = record.get("path") + path = relative_path(saved_path) if isinstance(saved_path, str) else None if path is None: return None selected = path.as_posix() - if selected != record or selected not in expected: + if ( + kind not in {"directory", "file"} + or selected != saved_path + or selected not in expected + ): return None node = index for component in path.parts: node = node.children.setdefault(component, SourceScopeIndex()) - if node.selected: + if node.kind is not None: return None - node.selected = True + node.kind = kind return repository, tree, index -def source_path_is_selected(index: SourceScopeIndex, value: str) -> bool: +def selected_source_kinds( + index: SourceScopeIndex, value: str +) -> dict[int, str] | None: path = relative_path(value) if path is None: - return False + return None node = index - if node.selected: - return True - for component in path.parts: + kinds = {0: node.kind} if node.kind is not None else {} + authorized = node.kind == "directory" + for depth, component in enumerate(path.parts, start=1): child = node.children.get(component) if child is None: - return False + return kinds if authorized else None node = child - if node.selected: - return True - return False + if node.kind is not None: + kinds[depth] = node.kind + if node.kind == "directory" or ( + node.kind == "file" and depth == len(path.parts) + ): + authorized = True + return kinds if authorized else None def source_excerpt_context( @@ -307,9 +409,10 @@ def finding_source_excerpt_from_context( if not isinstance(path, str) or not isinstance(start_line, int): return None try: + selected_kinds = selected_source_kinds(scopes, path) selected = ( - tree_path(repository, tree, path) - if source_path_is_selected(scopes, path) + tree_path(repository, tree, path, selected_kinds=selected_kinds) + if selected_kinds is not None else None ) object_id = ( diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 95d5c17a8..eec7ee77e 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -52,6 +52,7 @@ function git( } function collisionRepository(root: string): { + deepPath: string; repository: string; replacement: string; revision: string; @@ -92,8 +93,19 @@ function collisionRepository(root: string): { const lowerScope = tree([ ["100644", "blob", blob("colliding_scope = True\n"), "sibling.py"], ]); + const mismatchTree = tree([ + ["100644", "blob", blob("unscanned = True\n"), "secret.py"], + ]); + const deepComponents = Array.from({ length: 128 }, (_, index) => `d${index}`); + let deepTree = tree([["100644", "blob", blob("deep = True\n"), "source.py"]]); + for (const component of deepComponents.toReversed()) { + deepTree = tree([["040000", "tree", deepTree, component]]); + } + const deepPath = ["deep", ...deepComponents, "source.py"].join("/"); const rootTree = tree([ ["040000", "tree", upperScope, "Scope"], + ["040000", "tree", deepTree, "deep"], + ["040000", "tree", mismatchTree, "mismatch"], ["040000", "tree", lowerScope, "scope"], ["100644", "blob", blob("outside = True\n"), "outside.py"], ["040000", "tree", sourceTree, "src"], @@ -117,15 +129,17 @@ function collisionRepository(root: string): { git(repository, ["update-ref", "refs/heads/main", revision]); mkdirSync(join(repository, "src")); mkdirSync(join(repository, "Scope")); + mkdirSync(join(repository, "deep")); writeFileSync(join(repository, "src", "allowed.py"), "allowed = True\n"); writeFileSync(join(repository, "src", "LOWER.py"), "case_upper = True\n"); writeFileSync(join(repository, "src", "é.py"), "unicode_composed = True\n"); writeFileSync(join(repository, "src", "trailing.py"), "plain_name = True\n"); + writeFileSync(join(repository, "mismatch"), "selected_file = True\n"); writeFileSync( join(repository, "Scope", "selected.py"), "selected_scope = True\n", ); - return { repository, replacement, revision }; + return { deepPath, repository, replacement, revision }; } function ordinaryRepository(root: string): { @@ -219,6 +233,7 @@ from workbench_target import clean_worktree_content_digest repository = Path(sys.argv[2]).resolve() revision = sys.argv[3] replacement = sys.argv[4] +deep_path = sys.argv[5] metadata = repository.stat() identity = (revision, clean_worktree_content_digest(), metadata.st_dev, metadata.st_ino) authority = excerpts.capture_source_scopes(repository, identity, ["src", "src"]) @@ -260,10 +275,22 @@ collisions = { } collision_blob_reads = blob_reads[before:] outside = excerpt("outside.py") +file_authority = excerpts.capture_source_scopes(repository, identity, ["mismatch"]) +file_scan = {**scan, "source_scopes_json": json.dumps(file_authority)} +before = len(blob_reads) +file_descendant_excerpt = excerpt( + "mismatch/secret.py", file_scan, ["mismatch"] +) +file_descendant_blob_reads = blob_reads[before:] before = len(blob_reads) broadened = excerpt( "outside.py", - {**scan, "source_scopes_json": json.dumps({**authority, "paths": ["."]})}, + { + **scan, + "source_scopes_json": json.dumps( + {**authority, "paths": [{"kind": "directory", "path": "."}]} + ), + }, ) broadened_blob_reads = blob_reads[before:] scope_collision_authority = excerpts.capture_source_scopes( @@ -357,12 +384,40 @@ immutable = { ) for kind in ("commit", "range") } +deep_authority = excerpts.capture_source_scopes(repository, identity, ["deep"]) +deep_scan = {**scan, "source_scopes_json": json.dumps(deep_authority)} +batch_processes = 0 +tree_reads = 0 +original_popen = subprocess.Popen +def watched_popen(arguments, *positional, **keywords): + global batch_processes + if "cat-file" in arguments and "--batch" in arguments: + batch_processes += 1 + return original_popen(arguments, *positional, **keywords) +def counted_git(*arguments, **keywords): + global tree_reads + if len(arguments) >= 3 and arguments[1:3] == ("ls-tree", "-z"): + tree_reads += 1 + return original_git(*arguments, **keywords) +subprocess.Popen = watched_popen +excerpts.local_git_bytes = counted_git +try: + deep_excerpt = excerpt(deep_path, deep_scan, ["deep"]) +finally: + excerpts.local_git_bytes = original_git + subprocess.Popen = original_popen large_paths = [str(index) for index in range(20_000)] large_tree = "0" * 40 large_scan = { **scan, "source_scopes_json": json.dumps( - {"version": 1, "paths": large_paths, "targetTree": large_tree} + { + "version": 1, + "paths": [ + {"kind": "file", "path": path} for path in large_paths + ], + "targetTree": large_tree, + } ), } large_recipe_bytes = len( @@ -393,7 +448,7 @@ try: return original_pure_path(*arguments) excerpts.PurePosixPath = one_path_parse try: - excerpts.source_path_is_selected( + excerpts.selected_source_kinds( large_context[2], "/".join(["nested"] * 20_000 + ["file.py"]), ) @@ -411,8 +466,14 @@ print(json.dumps({ "broadenedBlobReads": broadened_blob_reads, "collisionBlobReads": collision_blob_reads, "collisions": collisions, + "deepBatchProcesses": batch_processes, + "deepExcerpt": deep_excerpt, "deepPathParses": path_parses, + "deepTreeReads": tree_reads, "duplicatePaths": len(authority["paths"]), + "fileAuthorityPaths": file_authority["paths"], + "fileDescendantBlobReads": file_descendant_blob_reads, + "fileDescendantExcerpt": file_descendant_excerpt, "immutable": immutable, "largeExcerpt": large_excerpt, "largeRecipeFits": large_recipe_bytes < 256 * 1024, @@ -443,6 +504,7 @@ print(json.dumps({ fixture.repository, fixture.revision, fixture.replacement, + fixture.deepPath, ], { encoding: "utf8" }, ); @@ -473,8 +535,14 @@ describe("workbench source excerpts", () => { "src/trailing.py": null, "src/trailing.py.": null, }, + deepBatchProcesses: 1, + deepExcerpt: expect.stringContaining("deep = True"), deepPathParses: 1, + deepTreeReads: 0, duplicatePaths: 1, + fileAuthorityPaths: [{ kind: "file", path: "mismatch" }], + fileDescendantBlobReads: [], + fileDescendantExcerpt: null, immutable: { commit: expect.stringContaining("allowed = True"), range: expect.stringContaining("allowed = True"), @@ -491,7 +559,7 @@ describe("workbench source excerpts", () => { }, nestedBlobReads: [], nestedExcerpt: null, - subtargetPaths: ["."], + subtargetPaths: [{ kind: "directory", path: "." }], outside: null, pathCollisionBlobReads: [], pathCollisionExcerpt: null, @@ -602,7 +670,11 @@ print(json.dumps({"authorities": authorities, "cli": cli, "deep": deep, "headles ); const authorities = writers["authorities"] as Record< string, - { paths: string[]; targetTree: string; version: number } + { + paths: Array<{ kind: "directory" | "file"; path: string }>; + targetTree: string; + version: number; + } >; const workspace = writers["workspace"] as Record; const prompt = writers["prompt"] as Record; @@ -614,18 +686,33 @@ print(json.dumps({"authorities": authorities, "cli": cli, "deep": deep, "headles const headlessScan = headless["scan"] as Record; const deepScan = deep["deepScan"] as Record; const expected = [ - ["workspace", workspaceResults["scanId"], ["src"]], - ["prompt", promptScan["scanId"], ["src"]], - ["headless", headlessScan["scanId"], ["src"]], - ["CLI", cli["scanId"], ["src/allowed.py", "other"]], - ["deep", deepScan["scanId"], ["."]], + [ + "workspace", + workspaceResults["scanId"], + [{ kind: "directory", path: "src" }], + ], + ["prompt", promptScan["scanId"], [{ kind: "directory", path: "src" }]], + [ + "headless", + headlessScan["scanId"], + [{ kind: "directory", path: "src" }], + ], + [ + "CLI", + cli["scanId"], + [ + { kind: "file", path: "src/allowed.py" }, + { kind: "directory", path: "other" }, + ], + ], + ["deep", deepScan["scanId"], [{ kind: "directory", path: "." }]], ] as const; expect(Object.keys(authorities)).toHaveLength(expected.length); for (const [writer, scanId, paths] of expected) { const authority = authorities[String(scanId)]; expect(authority?.version, writer).toBe(1); expect(authority?.targetTree, writer).toMatch(/^[0-9a-f]{40,64}$/); - expect(authority?.paths, writer).toEqual([...paths]); + expect(authority?.paths, writer).toEqual(paths); } }, 60_000); @@ -693,10 +780,12 @@ sys.path.insert(0, sys.argv[1]) import workbench_db raw_path = "segment/" * 300 + "source.py" +overlong_role = "x" * 128 + "root_control" connection = sqlite3.connect(":memory:") connection.row_factory = sqlite3.Row connection.execute("CREATE TABLE finding_locations (occurrence_id TEXT, relative_path TEXT, start_line INTEGER, end_line INTEGER, role TEXT, sort_order INTEGER)") -connection.execute("INSERT INTO finding_locations VALUES ('occurrence', ?, 1, 1, 'root_control', 0)", (raw_path,)) +connection.execute("INSERT INTO finding_locations VALUES ('occurrence', ?, 1, 1, 'primary', 0)", (raw_path,)) +connection.execute("INSERT INTO finding_locations VALUES ('occurrence', 'other.py', 1, 1, ?, 1)", (overlong_role,)) target = Path(tempfile.mkdtemp()).resolve() workbench_db.require_scan_target_identity = lambda scan: target workbench_db.safe_source_path = lambda selected, value: None @@ -710,8 +799,22 @@ def prepare_context(scan, selected, selected_paths): selected_paths_seen.append(selected_paths) return authority_context if selected_paths else None def source_excerpt(context, locations): - seen.append([location["path"] for location in locations]) - if context is authority_context and locations[0]["path"] == raw_path: + selected = next( + ( + location + for location in locations + if "root_control" in str(location.get("role") or "").lower() + ), + locations[0], + ) + seen.append({ + "boundedRoles": all( + len(str(location.get("role") or "").encode()) <= 128 + for location in locations + ), + "selectedPath": selected["path"], + }) + if context is authority_context and selected["path"] == raw_path: return "1 raw_path_authorized = True" return None workbench_db.source_excerpt_context = prepare_context @@ -733,7 +836,8 @@ print(json.dumps({ "malformedExcerpt": malformed.get("sourceExcerpt"), "malformedTitle": malformed.get("title"), "reusedExcerpt": findings[1].get("sourceExcerpt"), - "sawRawPath": seen == [[raw_path], [raw_path], [raw_path]], + "sawBoundedRoles": all(item["boundedRoles"] for item in seen), + "sawRawPath": all(item["selectedPath"] == raw_path for item in seen), "sawSelectedPaths": selected_paths_seen == [["."], []], })) `; @@ -750,6 +854,7 @@ print(json.dumps({ malformedExcerpt: null, malformedTitle: "title", reusedExcerpt: "1 raw_path_authorized = True", + sawBoundedRoles: true, sawRawPath: true, sawSelectedPaths: true, }); From 8977d5b9339a7ee1cf9bc753c31b68e4a9348eb0 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:25:28 +0000 Subject: [PATCH 15/26] fix(workbench): stream excerpt tree lookups --- .../_bundled_plugin/scripts/workbench_db.py | 12 +-- .../scripts/workbench_source_excerpt.py | 96 ++++++++++--------- .../tests-ts/workbench-source-excerpt.test.ts | 75 ++++++++++----- 3 files changed, 107 insertions(+), 76 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py index 0997abe67..bf1a39f31 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py @@ -4191,16 +4191,10 @@ def finding_result( """, (occurrence["id"], FINDING_LOCATIONS_LIMIT), ): - role = ( - bounded_output_text(row["role"], FINDING_LOCATION_ROLE_BYTES) - if row["role"] is not None - else None - ) excerpt_locations.append( { "endLine": row["end_line"], "path": row["relative_path"], - "role": role, "startLine": row["start_line"], } ) @@ -4208,7 +4202,11 @@ def finding_result( location = { "endLine": row["end_line"], "path": bounded_output_text(row["relative_path"], FINDING_LOCATION_PATH_BYTES), - "role": role, + "role": ( + bounded_output_text(row["role"], FINDING_LOCATION_ROLE_BYTES) + if row["role"] is not None + else None + ), "startLine": row["start_line"], } if absolute_path is not None: diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index e6659b1fa..9df1447c7 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -74,9 +74,9 @@ def local_git_bytes(repository: Path, *arguments: str) -> bytes | None: ) -def read_tree_object( - requests: IO[bytes], responses: IO[bytes], object_id: str -) -> bytes: +def matching_tree_entries( + requests: IO[bytes], responses: IO[bytes], object_id: str, name: str +) -> tuple[TreeEntry, ...]: encoded_object = object_id.encode("ascii") requests.write(encoded_object + b"\0") requests.flush() @@ -91,38 +91,51 @@ def read_tree_object( or not fields[2].isdigit() ): raise ValueError("invalid tree response") - remaining = int(fields[2]) - chunks: list[bytes] = [] - while remaining: - chunk = responses.read(min(1024 * 1024, remaining)) + unread = int(fields[2]) + buffered = bytearray() + + def read_more() -> None: + nonlocal unread + chunk = responses.read(min(64 * 1024, unread)) if not chunk: raise ValueError("truncated tree response") - chunks.append(chunk) - remaining -= len(chunk) - if responses.read(1) != b"\n": - raise ValueError("missing tree terminator") - return b"".join(chunks) + buffered.extend(chunk) + unread -= len(chunk) - -def matching_tree_entries( - content: bytes, name: str, object_id_bytes: int -) -> tuple[TreeEntry, ...] | None: + def read_field(delimiter: int) -> bytearray: + field = bytearray() + while True: + try: + end = buffered.index(delimiter) + except ValueError: + field.extend(buffered) + buffered.clear() + if not unread: + raise ValueError("unterminated tree entry") from None + read_more() + continue + field.extend(buffered[:end]) + del buffered[: end + 1] + return field + + def read_object_id(size: int) -> bytes: + while len(buffered) < size: + if not unread: + raise ValueError("truncated tree entry") + read_more() + value = bytes(buffered[:size]) + del buffered[:size] + return value + + object_id_bytes = len(object_id) // 2 expected_name = normalized_path_component(name) matches = [] - cursor = 0 - while cursor < len(content): - mode_end = content.find(b" ", cursor) - name_end = content.find(b"\0", mode_end + 1) - object_start = name_end + 1 - object_end = object_start + object_id_bytes - if ( - mode_end <= cursor - or name_end < mode_end + 1 - or object_end > len(content) - ): - return None - mode = content[cursor:mode_end] - decoded_name = os.fsdecode(content[mode_end + 1 : name_end]) + while buffered or unread: + mode = bytes(read_field(ord(" "))) + decoded_name = read_field(0).decode( + sys.getfilesystemencoding(), errors="surrogateescape" + ) + entry_object = read_object_id(object_id_bytes).hex() kind = ( "directory" if mode in {b"40000", b"040000"} @@ -131,8 +144,9 @@ def matching_tree_entries( else "other" ) if normalized_path_component(decoded_name) == expected_name: - matches.append((decoded_name, kind, content[object_start:object_end].hex())) - cursor = object_end + matches.append((decoded_name, kind, entry_object)) + if responses.read(1) != b"\n": + raise ValueError("missing tree terminator") return tuple(matches) @@ -184,10 +198,11 @@ def tree_path( for depth, name in enumerate(path.parts, start=1): if kind != "directory": return None - content = read_tree_object(process.stdin, process.stdout, object_id) - aliases = matching_tree_entries(content, name, len(object_id) // 2) + aliases = matching_tree_entries( + process.stdin, process.stdout, object_id, name + ) # The normalized name must be unique before an exact spelling can win. - if aliases is None or len(aliases) != 1: + if len(aliases) != 1: return None entry = next( (candidate for candidate in aliases if candidate[0] == name), @@ -205,7 +220,7 @@ def tree_path( process.stdin.close() if process.wait() != 0: return None - except (OSError, ValueError): + except (MemoryError, OSError, ValueError): return None return path.as_posix(), kind, object_id @@ -396,14 +411,7 @@ def finding_source_excerpt_from_context( return None repository, tree, scopes = context - location = next( - ( - candidate - for candidate in locations - if "root_control" in str(candidate.get("role") or "").lower() - ), - locations[0], - ) + location = locations[0] path = location.get("path") start_line = location.get("startLine") if not isinstance(path, str) or not isinstance(start_line, int): diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index eec7ee77e..2f83b2084 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -221,10 +221,15 @@ async function upgradedPlugin(root: string) { function collisionProbe( pluginRoot: string, - fixture: { repository: string; replacement: string; revision: string }, + fixture: { + deepPath: string; + repository: string; + replacement: string; + revision: string; + }, ) { const program = String.raw` -import json, subprocess, sys +import io, json, subprocess, sys from pathlib import Path sys.path.insert(0, sys.argv[1]) import workbench_source_excerpt as excerpts @@ -245,6 +250,13 @@ scan = { "target_snapshot_digest": clean_worktree_content_digest(), "source_scopes_json": authority_json, } +ordered_excerpt = excerpts.finding_source_excerpt_from_context( + excerpts.source_excerpt_context(scan, repository, ["src"]), + [ + {"path": "src/allowed.py", "startLine": 1, "role": "evidence"}, + {"path": "outside.py", "startLine": 1, "role": "not_root_control"}, + ], +) def excerpt(path, saved=scan, selected_paths=None): if selected_paths is None: selected_paths = ["src"] @@ -406,6 +418,33 @@ try: finally: excerpts.local_git_bytes = original_git subprocess.Popen = original_popen +batch_object = "1" * 40 +entry_object = b"\1" * 20 +wide_tree = b"".join( + b"100644 f%04d\0" % index + entry_object for index in range(5_000) +) +wide_tree += b"100644 target.py\0" + entry_object +class CappedRead(io.BytesIO): + largest = 0 + def read(self, size=-1): + if size < 0 or size > 64 * 1024: + raise AssertionError("tree response was buffered") + self.largest = max(self.largest, size) + return super().read(size) +stream_requests = io.BytesIO() +stream_responses = CappedRead( + f"{batch_object} tree {len(wide_tree)}\n".encode() + + wide_tree + + b"\n" +) +streamed_aliases = excerpts.matching_tree_entries( + stream_requests, stream_responses, batch_object, "target.py" +) +streamed_wide_tree = ( + streamed_aliases == (("target.py", "file", entry_object.hex()),) + and stream_requests.getvalue() == batch_object.encode() + b"\0" + and stream_responses.largest == 64 * 1024 +) large_paths = [str(index) for index in range(20_000)] large_tree = "0" * 40 large_scan = { @@ -484,6 +523,7 @@ print(json.dumps({ "mutable": {"excerpts": mutable, "gitCalls": len(git_calls)}, "nestedBlobReads": nested_blob_reads, "nestedExcerpt": nested_excerpt, + "orderedExcerpt": ordered_excerpt, "subtargetPaths": subtarget_authority["paths"], "outside": outside, "pathCollisionBlobReads": scope_collision_blob_reads, @@ -491,6 +531,7 @@ print(json.dumps({ "pathCollisionPaths": len(scope_collision_authority["paths"]), "replaced": replaced, "replacementBlobReads": replacement_blob_reads, + "streamedWideTree": streamed_wide_tree, })) `; const result = spawnSync( @@ -559,6 +600,7 @@ describe("workbench source excerpts", () => { }, nestedBlobReads: [], nestedExcerpt: null, + orderedExcerpt: expect.stringContaining("allowed = True"), subtargetPaths: [{ kind: "directory", path: "." }], outside: null, pathCollisionBlobReads: [], @@ -566,6 +608,7 @@ describe("workbench source excerpts", () => { pathCollisionPaths: 1, replaced: null, replacementBlobReads: [], + streamedWideTree: true, }); }, 60_000); @@ -712,7 +755,7 @@ print(json.dumps({"authorities": authorities, "cli": cli, "deep": deep, "headles const authority = authorities[String(scanId)]; expect(authority?.version, writer).toBe(1); expect(authority?.targetTree, writer).toMatch(/^[0-9a-f]{40,64}$/); - expect(authority?.paths, writer).toEqual(paths); + expect(authority?.paths, writer).toEqual([...paths]); } }, 60_000); @@ -780,12 +823,10 @@ sys.path.insert(0, sys.argv[1]) import workbench_db raw_path = "segment/" * 300 + "source.py" -overlong_role = "x" * 128 + "root_control" connection = sqlite3.connect(":memory:") connection.row_factory = sqlite3.Row connection.execute("CREATE TABLE finding_locations (occurrence_id TEXT, relative_path TEXT, start_line INTEGER, end_line INTEGER, role TEXT, sort_order INTEGER)") -connection.execute("INSERT INTO finding_locations VALUES ('occurrence', ?, 1, 1, 'primary', 0)", (raw_path,)) -connection.execute("INSERT INTO finding_locations VALUES ('occurrence', 'other.py', 1, 1, ?, 1)", (overlong_role,)) +connection.execute("INSERT INTO finding_locations VALUES ('occurrence', ?, 1, 1, 'root_control', 0)", (raw_path,)) target = Path(tempfile.mkdtemp()).resolve() workbench_db.require_scan_target_identity = lambda scan: target workbench_db.safe_source_path = lambda selected, value: None @@ -799,22 +840,8 @@ def prepare_context(scan, selected, selected_paths): selected_paths_seen.append(selected_paths) return authority_context if selected_paths else None def source_excerpt(context, locations): - selected = next( - ( - location - for location in locations - if "root_control" in str(location.get("role") or "").lower() - ), - locations[0], - ) - seen.append({ - "boundedRoles": all( - len(str(location.get("role") or "").encode()) <= 128 - for location in locations - ), - "selectedPath": selected["path"], - }) - if context is authority_context and selected["path"] == raw_path: + seen.append([location["path"] for location in locations]) + if context is authority_context and locations[0]["path"] == raw_path: return "1 raw_path_authorized = True" return None workbench_db.source_excerpt_context = prepare_context @@ -836,8 +863,7 @@ print(json.dumps({ "malformedExcerpt": malformed.get("sourceExcerpt"), "malformedTitle": malformed.get("title"), "reusedExcerpt": findings[1].get("sourceExcerpt"), - "sawBoundedRoles": all(item["boundedRoles"] for item in seen), - "sawRawPath": all(item["selectedPath"] == raw_path for item in seen), + "sawRawPath": seen == [[raw_path], [raw_path], [raw_path]], "sawSelectedPaths": selected_paths_seen == [["."], []], })) `; @@ -854,7 +880,6 @@ print(json.dumps({ malformedExcerpt: null, malformedTitle: "title", reusedExcerpt: "1 raw_path_authorized = True", - sawBoundedRoles: true, sawRawPath: true, sawSelectedPaths: true, }); From 1dc857237b775d7bfb0f90cfaf81c822fd10d0b9 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:29:07 +0000 Subject: [PATCH 16/26] fix(workbench): bound excerpt tree parsing --- .../scripts/workbench_source_excerpt.py | 101 +++++++++++------- .../tests-ts/workbench-source-excerpt.test.ts | 37 ++++++- 2 files changed, 99 insertions(+), 39 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index 9df1447c7..c79931ff1 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -75,8 +75,12 @@ def local_git_bytes(repository: Path, *arguments: str) -> bytes | None: def matching_tree_entries( - requests: IO[bytes], responses: IO[bytes], object_id: str, name: str -) -> tuple[TreeEntry, ...]: + requests: IO[bytes], + responses: IO[bytes], + object_id: str, + name: str, + name_bytes_limit: int, +) -> TreeEntry | None: encoded_object = object_id.encode("ascii") requests.write(encoded_object + b"\0") requests.flush() @@ -93,61 +97,77 @@ def matching_tree_entries( raise ValueError("invalid tree response") unread = int(fields[2]) buffered = bytearray() + cursor = 0 def read_more() -> None: - nonlocal unread + nonlocal cursor, unread + if cursor: + del buffered[:cursor] + cursor = 0 chunk = responses.read(min(64 * 1024, unread)) if not chunk: raise ValueError("truncated tree response") buffered.extend(chunk) unread -= len(chunk) - def read_field(delimiter: int) -> bytearray: + def read_field(delimiter: int, maximum_bytes: int) -> bytearray: + nonlocal cursor field = bytearray() while True: try: - end = buffered.index(delimiter) + end = buffered.index(delimiter, cursor) except ValueError: - field.extend(buffered) - buffered.clear() + available = len(buffered) - cursor + if len(field) + available > maximum_bytes: + raise ValueError("oversized tree field") from None + field.extend(buffered[cursor:]) + cursor = len(buffered) if not unread: raise ValueError("unterminated tree entry") from None read_more() continue - field.extend(buffered[:end]) - del buffered[: end + 1] + if len(field) + end - cursor > maximum_bytes: + raise ValueError("oversized tree field") + field.extend(buffered[cursor:end]) + cursor = end + 1 return field def read_object_id(size: int) -> bytes: - while len(buffered) < size: + nonlocal cursor + while len(buffered) - cursor < size: if not unread: raise ValueError("truncated tree entry") read_more() - value = bytes(buffered[:size]) - del buffered[:size] + value = bytes(buffered[cursor : cursor + size]) + cursor += size return value object_id_bytes = len(object_id) // 2 expected_name = normalized_path_component(name) - matches = [] - while buffered or unread: - mode = bytes(read_field(ord(" "))) - decoded_name = read_field(0).decode( + selected = None + ambiguous = False + while cursor < len(buffered) or unread: + mode = bytes(read_field(ord(" "), 6)) + decoded_name = read_field(0, name_bytes_limit).decode( sys.getfilesystemencoding(), errors="surrogateescape" ) - entry_object = read_object_id(object_id_bytes).hex() - kind = ( - "directory" - if mode in {b"40000", b"040000"} - else "file" - if mode in {b"100644", b"100755"} - else "other" - ) - if normalized_path_component(decoded_name) == expected_name: - matches.append((decoded_name, kind, entry_object)) + entry_object = read_object_id(object_id_bytes) + if not ambiguous and normalized_path_component(decoded_name) == expected_name: + if selected is not None: + selected = None + ambiguous = True + else: + kind = ( + "directory" + if mode in {b"40000", b"040000"} + else "file" + if mode in {b"100644", b"100755"} + else "other" + ) + selected = (decoded_name, kind, entry_object.hex()) if responses.read(1) != b"\n": raise ValueError("missing tree terminator") - return tuple(matches) + return selected def tree_path( @@ -165,6 +185,17 @@ def tree_path( kind, object_id = "directory", tree if not path.parts: return path.as_posix(), kind, object_id + try: + # Windows components are at most 255 UTF-16 units; Git stores path bytes. + name_bytes_limit = ( + 255 * 4 + if os.name == "nt" + else os.pathconf(repository, "PC_NAME_MAX") + ) + except (AttributeError, OSError, ValueError): + return None + if name_bytes_limit < 1: + return None environment = os.environ.copy() for variable in GIT_REPOSITORY_ENVIRONMENT: @@ -199,18 +230,16 @@ def tree_path( if kind != "directory": return None aliases = matching_tree_entries( - process.stdin, process.stdout, object_id, name + process.stdin, + process.stdout, + object_id, + name, + name_bytes_limit, ) # The normalized name must be unique before an exact spelling can win. - if len(aliases) != 1: - return None - entry = next( - (candidate for candidate in aliases if candidate[0] == name), - None, - ) - if entry is None: + if aliases is None or aliases[0] != name: return None - _, kind, object_id = entry + _, kind, object_id = aliases if ( selected_kinds is not None and (expected_kind := selected_kinds.get(depth)) is not None diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 2f83b2084..0f83cf8ea 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -229,7 +229,7 @@ function collisionProbe( }, ) { const program = String.raw` -import io, json, subprocess, sys +import io, json, os, subprocess, sys from pathlib import Path sys.path.insert(0, sys.argv[1]) import workbench_source_excerpt as excerpts @@ -437,13 +437,44 @@ stream_responses = CappedRead( + wide_tree + b"\n" ) +name_limit = 255 * 4 if sys.platform == "win32" else os.pathconf(repository, "PC_NAME_MAX") streamed_aliases = excerpts.matching_tree_entries( - stream_requests, stream_responses, batch_object, "target.py" + stream_requests, stream_responses, batch_object, "target.py", name_limit ) +alias_tree = (b"100644 target.py\0" + entry_object) * 20_000 +alias_responses = CappedRead( + f"{batch_object} tree {len(alias_tree)}\n".encode() + + alias_tree + + b"\n" +) +ambiguous_alias = excerpts.matching_tree_entries( + io.BytesIO(), alias_responses, batch_object, "target.py", name_limit +) +oversized_name = b"x" * (name_limit + 1) +oversized_tree = b"100644 " + oversized_name + b"\0" + entry_object +try: + excerpts.matching_tree_entries( + io.BytesIO(), + io.BytesIO( + f"{batch_object} tree {len(oversized_tree)}\n".encode() + + oversized_tree + + b"\n" + ), + batch_object, + "target.py", + name_limit, + ) +except ValueError: + oversized_rejected = True +else: + oversized_rejected = False streamed_wide_tree = ( - streamed_aliases == (("target.py", "file", entry_object.hex()),) + streamed_aliases == ("target.py", "file", entry_object.hex()) and stream_requests.getvalue() == batch_object.encode() + b"\0" and stream_responses.largest == 64 * 1024 + and ambiguous_alias is None + and alias_responses.largest == 64 * 1024 + and oversized_rejected ) large_paths = [str(index) for index in range(20_000)] large_tree = "0" * 40 From 39236701a8600d11ec5e77d48080a0c20ae6ca04 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:45:53 +0000 Subject: [PATCH 17/26] fix(workbench): stream source excerpt blobs --- .../scripts/workbench_source_excerpt.py | 148 +++++++++++++++--- .../tests-ts/workbench-source-excerpt.test.ts | 42 ++++- 2 files changed, 165 insertions(+), 25 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index c79931ff1..671ff59b7 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -3,6 +3,7 @@ from __future__ import annotations import argparse +import codecs import json import os import re @@ -28,6 +29,7 @@ MAX_BYTES = 16_000 MAX_LINES = 60 OBJECT_ID = re.compile(r"(?:[0-9a-f]{40}|[0-9a-f]{64})\Z") +LINE_BREAK = re.compile(r"\r\n|[\n\v\f\r\x1c-\x1e\x85\u2028\u2029]") TreeEntry = tuple[str, str, str] @@ -457,38 +459,138 @@ def finding_source_excerpt_from_context( if selected is not None and selected[1] == "file" else None ) - except (OSError, RuntimeError, SystemExit, UnicodeError, ValueError): - return None - if object_id is None: - return None - source = scanned_source_text(repository, object_id) - if not source or "\0" in source: + if object_id is None: + return None + end_line = location.get("endLine") + last_affected_line = end_line if isinstance(end_line, int) else start_line + return scanned_source_excerpt( + repository, + object_id, + start_line, + last_affected_line, + ) + except ( + MemoryError, + OSError, + RuntimeError, + SystemExit, + UnicodeError, + ValueError, + ): return None - lines = source.splitlines() - if start_line < 1 or start_line > len(lines): + + +def scanned_source_excerpt( + repository: Path, + object_id: str, + start_line: int, + last_affected_line: int, +) -> str | None: + if start_line < 1 or OBJECT_ID.fullmatch(object_id) is None: return None - end_line = location.get("endLine") - last_affected_line = end_line if isinstance(end_line, int) else start_line excerpt_start = max(1, start_line - CONTEXT_LINES) - excerpt_end = min( - len(lines), + excerpt_limit = min( max(start_line, last_affected_line) + CONTEXT_LINES, excerpt_start + MAX_LINES - 1, ) - width = len(str(excerpt_end)) - excerpt = "\n".join( - f"{line_number:>{width}} {lines[line_number - 1]}" - for line_number in range(excerpt_start, excerpt_end + 1) - ) - return excerpt.encode("utf-8")[:MAX_BYTES].decode("utf-8", errors="ignore") + environment = os.environ.copy() + for variable in GIT_REPOSITORY_ENVIRONMENT: + environment.pop(variable, None) + environment["GIT_ALLOW_PROTOCOL"] = "" + environment["GIT_LITERAL_PATHSPECS"] = "1" + environment["GIT_NO_LAZY_FETCH"] = "1" + command = [ + "git", + "-c", + "core.fsmonitor=false", + "-c", + "i18n.logOutputEncoding=UTF-8", + "-C", + str(repository), + "--no-replace-objects", + "cat-file", + "blob", + object_id, + ] + + captured_lines: list[tuple[int, str]] = [] + fragments: list[str] = [] + captured_bytes = 0 + line_number = 1 + last_line = 0 + line_has_content = False + pending_carriage_return = False + + def capture(value: str) -> None: + nonlocal captured_bytes, line_has_content + if not value: + return + line_has_content = True + if ( + line_number < excerpt_start + or line_number > excerpt_limit + or captured_bytes >= MAX_BYTES + ): + return + remaining = MAX_BYTES - captured_bytes + selected = value.encode("utf-8")[:remaining] + fragments.append(selected.decode("utf-8", errors="ignore")) + captured_bytes += len(selected) + + def finish_line() -> None: + nonlocal fragments, last_line, line_has_content, line_number + if excerpt_start <= line_number <= excerpt_limit: + captured_lines.append((line_number, "".join(fragments))) + last_line = line_number + line_number += 1 + fragments = [] + line_has_content = False + + def consume(value: str, *, final: bool = False) -> None: + nonlocal pending_carriage_return + if pending_carriage_return: + value = "\r" + value + pending_carriage_return = False + if not final and value.endswith("\r"): + value = value[:-1] + pending_carriage_return = True + cursor = 0 + for match in LINE_BREAK.finditer(value): + capture(value[cursor : match.start()]) + finish_line() + cursor = match.end() + capture(value[cursor:]) + + decoder = codecs.getincrementaldecoder("utf-8")(errors="replace") + invalid = False + with subprocess.Popen( + command, + env=environment, + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + ) as process: + if process.stdout is None: + return None + while chunk := process.stdout.read(64 * 1024): + if b"\0" in chunk: + invalid = True + if not invalid: + consume(decoder.decode(chunk)) + if not invalid: + consume(decoder.decode(b"", final=True), final=True) + if line_has_content: + finish_line() + if process.wait() != 0: + return None -def scanned_source_text(repository: Path, object_id: str) -> str | None: - try: - content = local_git_bytes(repository, "cat-file", "blob", object_id) - except (OSError, RuntimeError, SystemExit): + if invalid or last_line < start_line or not captured_lines: return None - return content.decode("utf-8", errors="replace") if content is not None else None + width = len(str(captured_lines[-1][0])) + excerpt = "\n".join( + f"{number:>{width}} {line}" for number, line in captured_lines + ) + return excerpt.encode("utf-8")[:MAX_BYTES].decode("utf-8", errors="ignore") def main() -> None: diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 0f83cf8ea..109b99358 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -80,6 +80,7 @@ function collisionRepository(root: string): { }; const sourceTree = tree([ ["100644", "blob", blob("allowed = True\n"), "allowed.py"], + ["100644", "blob", blob("x".repeat(256 * 1024)), "large.py"], ["100644", "blob", blob("case_upper = True\n"), "LOWER.py"], ["100644", "blob", blob("case_lower = True\n"), "lower.py"], ["100644", "blob", blob("unicode_composed = True\n"), "é.py"], @@ -266,13 +267,41 @@ def excerpt(path, saved=scan, selected_paths=None): [{"path": path, "startLine": 1, "endLine": 1, "role": "root_control"}], ) original_git = excerpts.local_git_bytes +original_popen = subprocess.Popen blob_reads = [] +blob_read_sizes = [] +class WatchedBlobOutput: + def __init__(self, output): + self.output = output + def read(self, size=-1): + if size < 0 or size > 64 * 1024: + raise AssertionError("blob response was buffered") + blob_read_sizes.append(size) + return self.output.read(size) + def __getattr__(self, name): + return getattr(self.output, name) +def watched_popen(arguments, *positional, **keywords): + process = original_popen(arguments, *positional, **keywords) + if len(arguments) >= 3 and arguments[-3:-1] == ["cat-file", "blob"]: + blob_reads.append(arguments[-1]) + process.stdout = WatchedBlobOutput(process.stdout) + return process def watched_git(*arguments, **kwargs): - if len(arguments) >= 4 and arguments[1:3] == ("cat-file", "blob"): - blob_reads.append(arguments[3]) return original_git(*arguments, **kwargs) +subprocess.Popen = watched_popen excerpts.local_git_bytes = watched_git allowed = excerpt("src/allowed.py") +before = len(blob_read_sizes) +large_blob_excerpt = excerpt("src/large.py") +large_blob_sizes = blob_read_sizes[before:] +original_excerpt_reader = excerpts.scanned_source_excerpt +def exhausted_blob(*arguments, **keywords): + raise MemoryError("synthetic blob exhaustion") +excerpts.scanned_source_excerpt = exhausted_blob +try: + blob_memory_error = excerpt("src/allowed.py") +finally: + excerpts.scanned_source_excerpt = original_excerpt_reader before = len(blob_reads) collisions = { path: excerpt(path) @@ -367,6 +396,7 @@ finally: ) replacement_blob_reads = blob_reads[before:] excerpts.local_git_bytes = original_git +subprocess.Popen = original_popen legacy_scan = dict(scan) legacy_scan.pop("source_scopes_json") legacy = excerpt("src/allowed.py", legacy_scan) @@ -546,6 +576,12 @@ print(json.dumps({ "fileDescendantExcerpt": file_descendant_excerpt, "immutable": immutable, "largeExcerpt": large_excerpt, + "largeBlobStreamed": ( + len(large_blob_excerpt.encode()) == 16_000 + and len(large_blob_sizes) >= 4 + and max(large_blob_sizes) == 64 * 1024 + ), + "blobMemoryError": blob_memory_error, "largeRecipeFits": large_recipe_bytes < 256 * 1024, "largeTreePathChecks": tree_path_checks, "invalid": invalid, @@ -620,6 +656,8 @@ describe("workbench source excerpts", () => { range: expect.stringContaining("allowed = True"), }, largeExcerpt: null, + largeBlobStreamed: true, + blobMemoryError: null, largeRecipeFits: true, largeTreePathChecks: 0, invalid: null, From ce22ca2f33fd41d416d5ea425b9069bfa30d02e3 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:51:59 +0000 Subject: [PATCH 18/26] test(workbench): align streamed excerpt coverage --- sdk/typescript/tests-ts/plugin-report-limits.test.ts | 8 ++------ sdk/typescript/tests-ts/workbench-source-excerpt.test.ts | 4 ---- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/sdk/typescript/tests-ts/plugin-report-limits.test.ts b/sdk/typescript/tests-ts/plugin-report-limits.test.ts index 180612fdd..f57299ced 100644 --- a/sdk/typescript/tests-ts/plugin-report-limits.test.ts +++ b/sdk/typescript/tests-ts/plugin-report-limits.test.ts @@ -3,7 +3,7 @@ import { describe, expect, test } from "bun:test"; import { PLUGIN_ROOT } from "./plugin-root.js"; describe("bundled scan report and source limits", () => { - test("accepts large reports, schemas, source files, and late source lines", () => { + test("accepts large reports, schemas, and late source lines", () => { const python = Bun.which("python3") ?? Bun.which("python"); expect(python).not.toBeNull(); const program = [ @@ -19,12 +19,9 @@ describe("bundled scan report and source limits", () => { " schema = pathlib.Path(directory) / 'large.schema.json'", " schema.write_text(json.dumps({'type': 'object', 'description': 'x' * (4 * 1024 * 1024), 'allOf': [{'type': 'object'}] * 129}))", " finalizer.validate_against_schema({'safe': True}, schema)", - " source = b'x' * (1024 * 1024 + 1)", - " excerpts.local_git_bytes = lambda *args: source", " target = pathlib.Path(directory).resolve()", - " excerpt = excerpts.scanned_source_text(target, 'deadbeef')", " hashes = finalizer._github_line_hashes(io.StringIO('line\\n' * 100001), {100001})", - " print(json.dumps({'documentBytes': len(document), 'sourceBytes': len(excerpt), 'lateSourceLine': 100001 in hashes, 'unsafePathRejected': excerpts.safe_source_path(target, '../outside') is None}))", + " print(json.dumps({'documentBytes': len(document), 'lateSourceLine': 100001 in hashes, 'unsafePathRejected': excerpts.safe_source_path(target, '../outside') is None}))", ].join("\n"); const result = Bun.spawnSync( [python!, "-I", "-B", "-c", program, join(PLUGIN_ROOT, "scripts")], @@ -34,7 +31,6 @@ describe("bundled scan report and source limits", () => { expect(result.exitCode, new TextDecoder().decode(result.stderr)).toBe(0); expect(JSON.parse(new TextDecoder().decode(result.stdout))).toMatchObject({ documentBytes: expect.any(Number), - sourceBytes: 1024 * 1024 + 1, lateSourceLine: true, unsafePathRejected: true, }); diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 109b99358..d07142aeb 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -286,10 +286,7 @@ def watched_popen(arguments, *positional, **keywords): blob_reads.append(arguments[-1]) process.stdout = WatchedBlobOutput(process.stdout) return process -def watched_git(*arguments, **kwargs): - return original_git(*arguments, **kwargs) subprocess.Popen = watched_popen -excerpts.local_git_bytes = watched_git allowed = excerpt("src/allowed.py") before = len(blob_read_sizes) large_blob_excerpt = excerpt("src/large.py") @@ -395,7 +392,6 @@ finally: check=True, ) replacement_blob_reads = blob_reads[before:] -excerpts.local_git_bytes = original_git subprocess.Popen = original_popen legacy_scan = dict(scan) legacy_scan.pop("source_scopes_json") From 11752409ce886c877865345e7e681f8d8b3f941b Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:31:33 +0000 Subject: [PATCH 19/26] fix(workbench): bound source authority checks --- .../scripts/workbench_source_excerpt.py | 35 +++++++++++-- .../tests-ts/workbench-source-excerpt.test.ts | 52 +++++++++++++++++++ 2 files changed, 82 insertions(+), 5 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index 671ff59b7..d38507f28 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -260,7 +260,16 @@ def target_tree(target: Path, revision: str) -> tuple[Path, str] | None: if not isinstance(revision, str) or not OBJECT_ID.fullmatch(revision): return None repository, prefix = git_worktree_context(target) - if local_git_bytes(repository, "replace", "--list") != b"": + if ( + local_git_bytes( + repository, + "for-each-ref", + "--count=1", + "--format=", + "refs/replace/", + ) + != b"" + ): return None raw_tree = local_git_bytes( repository, @@ -307,14 +316,30 @@ def capture_source_scopes( captured: set[str] = set() for requested in paths: parsed = relative_path(requested) - selected_path = safe_source_path(target, requested) - if parsed is None or selected_path is None: + if parsed is None or safe_source_path(target, requested) is None: continue - raw_selected = target / parsed.as_posix() try: - metadata = raw_selected.lstat() + raw_selected = target + components = parsed.parts + if not components: + metadata = raw_selected.lstat() + for index, component in enumerate(components): + raw_selected /= component + metadata = raw_selected.lstat() + if ( + stat.S_ISLNK(metadata.st_mode) + or getattr(metadata, "st_reparse_tag", 0) & 0x20000000 + or ( + index < len(components) - 1 + and not stat.S_ISDIR(metadata.st_mode) + ) + ): + metadata = None + break except OSError: continue + if metadata is None: + continue kind = ( "directory" if stat.S_ISDIR(metadata.st_mode) diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index d07142aeb..c842e16a1 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -7,6 +7,7 @@ import { readFileSync, realpathSync, rmSync, + symlinkSync, writeFileSync, } from "node:fs"; import { tmpdir } from "node:os"; @@ -97,6 +98,10 @@ function collisionRepository(root: string): { const mismatchTree = tree([ ["100644", "blob", blob("unscanned = True\n"), "secret.py"], ]); + const linkedSubtree = tree([ + ["100644", "blob", blob("linked_secret = True\n"), "secret.py"], + ]); + const linkedTree = tree([["040000", "tree", linkedSubtree, "subdir"]]); const deepComponents = Array.from({ length: 128 }, (_, index) => `d${index}`); let deepTree = tree([["100644", "blob", blob("deep = True\n"), "source.py"]]); for (const component of deepComponents.toReversed()) { @@ -105,6 +110,7 @@ function collisionRepository(root: string): { const deepPath = ["deep", ...deepComponents, "source.py"].join("/"); const rootTree = tree([ ["040000", "tree", upperScope, "Scope"], + ["040000", "tree", linkedTree, "alias"], ["040000", "tree", deepTree, "deep"], ["040000", "tree", mismatchTree, "mismatch"], ["040000", "tree", lowerScope, "scope"], @@ -131,6 +137,12 @@ function collisionRepository(root: string): { mkdirSync(join(repository, "src")); mkdirSync(join(repository, "Scope")); mkdirSync(join(repository, "deep")); + mkdirSync(join(repository, "real", "subdir"), { recursive: true }); + symlinkSync(join(repository, "real"), join(repository, "alias"), "junction"); + writeFileSync( + join(repository, "real", "subdir", "allowed.py"), + "linked_allowed = True\n", + ); writeFileSync(join(repository, "src", "allowed.py"), "allowed = True\n"); writeFileSync(join(repository, "src", "LOWER.py"), "case_upper = True\n"); writeFileSync(join(repository, "src", "é.py"), "unicode_composed = True\n"); @@ -288,6 +300,34 @@ def watched_popen(arguments, *positional, **keywords): return process subprocess.Popen = watched_popen allowed = excerpt("src/allowed.py") +linked_authority = excerpts.capture_source_scopes( + repository, identity, ["alias/subdir"] +) +linked_scan = {**scan, "source_scopes_json": json.dumps(linked_authority)} +before = len(blob_reads) +linked_excerpt = excerpt( + "alias/subdir/secret.py", linked_scan, ["alias/subdir"] +) +linked_blob_reads = blob_reads[before:] +replacement_probe_calls = [] +def guarded_git(*arguments, **keywords): + command = arguments[1:] + if command == ("replace", "--list"): + replacement_probe_calls.append(list(command)) + raise MemoryError("unbounded replacement-ref probe") + if command == ("for-each-ref", "--count=1", "--format=", "refs/replace/"): + replacement_probe_calls.append(list(command)) + return original_git(*arguments, **keywords) +excerpts.local_git_bytes = guarded_git +try: + try: + excerpts.capture_source_scopes(repository, identity, ["src"]) + except MemoryError: + replacement_probe_memory_error = True + else: + replacement_probe_memory_error = False +finally: + excerpts.local_git_bytes = original_git before = len(blob_read_sizes) large_blob_excerpt = excerpt("src/large.py") large_blob_sizes = blob_read_sizes[before:] @@ -580,6 +620,9 @@ print(json.dumps({ "blobMemoryError": blob_memory_error, "largeRecipeFits": large_recipe_bytes < 256 * 1024, "largeTreePathChecks": tree_path_checks, + "linkedAuthorityPaths": linked_authority["paths"], + "linkedBlobReads": linked_blob_reads, + "linkedExcerpt": linked_excerpt, "invalid": invalid, "legacy": legacy, "malformedRevision": malformed_revision, @@ -594,6 +637,8 @@ print(json.dumps({ "pathCollisionPaths": len(scope_collision_authority["paths"]), "replaced": replaced, "replacementBlobReads": replacement_blob_reads, + "replacementProbeCalls": replacement_probe_calls, + "replacementProbeMemoryError": replacement_probe_memory_error, "streamedWideTree": streamed_wide_tree, })) `; @@ -656,6 +701,9 @@ describe("workbench source excerpts", () => { blobMemoryError: null, largeRecipeFits: true, largeTreePathChecks: 0, + linkedAuthorityPaths: [], + linkedBlobReads: [], + linkedExcerpt: null, invalid: null, legacy: null, malformedRevision: null, @@ -673,6 +721,10 @@ describe("workbench source excerpts", () => { pathCollisionPaths: 1, replaced: null, replacementBlobReads: [], + replacementProbeCalls: [ + ["for-each-ref", "--count=1", "--format=", "refs/replace/"], + ], + replacementProbeMemoryError: false, streamedWideTree: true, }); }, 60_000); From 6eb08e90ff2e015261c95d23261f56e24b70c609 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:47:58 +0000 Subject: [PATCH 20/26] fix(workbench): bound replacement ref checks --- .../scripts/workbench_source_excerpt.py | 51 ++++++++++--- .../tests-ts/workbench-source-excerpt.test.ts | 76 +++++++++++++------ 2 files changed, 94 insertions(+), 33 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index d38507f28..ceba34ac6 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -256,20 +256,51 @@ def tree_path( return path.as_posix(), kind, object_id +def replacement_refs_absent(repository: Path) -> bool: + environment = os.environ.copy() + for variable in GIT_REPOSITORY_ENVIRONMENT: + environment.pop(variable, None) + environment["GIT_ALLOW_PROTOCOL"] = "" + environment["GIT_LITERAL_PATHSPECS"] = "1" + environment["GIT_NO_LAZY_FETCH"] = "1" + command = [ + "git", + "-c", + "core.fsmonitor=false", + "-c", + "i18n.logOutputEncoding=UTF-8", + "-C", + str(repository), + "--no-replace-objects", + "for-each-ref", + "--count=1", + "--format=", + "refs/replace/", + ] + try: + with subprocess.Popen( + command, + env=environment, + stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + ) as process: + if process.stdout is None: + return False + if process.stdout.read(1): + if process.poll() is None: + process.kill() + return False + return process.wait() == 0 + except (MemoryError, OSError): + return False + + def target_tree(target: Path, revision: str) -> tuple[Path, str] | None: if not isinstance(revision, str) or not OBJECT_ID.fullmatch(revision): return None repository, prefix = git_worktree_context(target) - if ( - local_git_bytes( - repository, - "for-each-ref", - "--count=1", - "--format=", - "refs/replace/", - ) - != b"" - ): + if not replacement_refs_absent(repository): return None raw_tree = local_git_bytes( repository, diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index c842e16a1..76ff74ffa 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -282,6 +282,9 @@ original_git = excerpts.local_git_bytes original_popen = subprocess.Popen blob_reads = [] blob_read_sizes = [] +replacement_probe_commands = [] +replacement_probe_read_sizes = [] +watch_replacement_probe = False class WatchedBlobOutput: def __init__(self, output): self.output = output @@ -292,11 +295,23 @@ class WatchedBlobOutput: return self.output.read(size) def __getattr__(self, name): return getattr(self.output, name) +class WatchedReplacementOutput: + def __init__(self, output): + self.output = output + def read(self, size=-1): + replacement_probe_read_sizes.append(size) + return self.output.read(size) + def __getattr__(self, name): + return getattr(self.output, name) def watched_popen(arguments, *positional, **keywords): process = original_popen(arguments, *positional, **keywords) if len(arguments) >= 3 and arguments[-3:-1] == ["cat-file", "blob"]: blob_reads.append(arguments[-1]) process.stdout = WatchedBlobOutput(process.stdout) + if watch_replacement_probe and "for-each-ref" in arguments: + start = arguments.index("for-each-ref") + replacement_probe_commands.append(arguments[start:]) + process.stdout = WatchedReplacementOutput(process.stdout) return process subprocess.Popen = watched_popen allowed = excerpt("src/allowed.py") @@ -309,25 +324,6 @@ linked_excerpt = excerpt( "alias/subdir/secret.py", linked_scan, ["alias/subdir"] ) linked_blob_reads = blob_reads[before:] -replacement_probe_calls = [] -def guarded_git(*arguments, **keywords): - command = arguments[1:] - if command == ("replace", "--list"): - replacement_probe_calls.append(list(command)) - raise MemoryError("unbounded replacement-ref probe") - if command == ("for-each-ref", "--count=1", "--format=", "refs/replace/"): - replacement_probe_calls.append(list(command)) - return original_git(*arguments, **keywords) -excerpts.local_git_bytes = guarded_git -try: - try: - excerpts.capture_source_scopes(repository, identity, ["src"]) - except MemoryError: - replacement_probe_memory_error = True - else: - replacement_probe_memory_error = False -finally: - excerpts.local_git_bytes = original_git before = len(blob_read_sizes) large_blob_excerpt = excerpt("src/large.py") large_blob_sizes = blob_read_sizes[before:] @@ -432,6 +428,36 @@ finally: check=True, ) replacement_blob_reads = blob_reads[before:] +replace_directory = outer_git_dir / "refs" / "replace" +replace_directory.mkdir(parents=True, exist_ok=True) +broken_replacements = [] +for index in range(128): + broken = replace_directory / f"r{index:04d}" + broken.write_text("not-an-object\n") + broken_replacements.append(broken) +watch_replacement_probe = True +try: + malformed_replacement_paths = excerpts.capture_source_scopes( + repository, identity, ["src"] + )["paths"] +finally: + watch_replacement_probe = False + for broken in broken_replacements: + broken.unlink() +def exhausted_popen(arguments, *positional, **keywords): + if "for-each-ref" in arguments: + raise MemoryError("synthetic replacement probe exhaustion") + return original_popen(arguments, *positional, **keywords) +subprocess.Popen = exhausted_popen +try: + try: + replacement_memory_paths = excerpts.capture_source_scopes( + repository, identity, ["src"] + )["paths"] + except MemoryError: + replacement_memory_paths = "escaped" +finally: + subprocess.Popen = watched_popen subprocess.Popen = original_popen legacy_scan = dict(scan) legacy_scan.pop("source_scopes_json") @@ -623,6 +649,7 @@ print(json.dumps({ "linkedAuthorityPaths": linked_authority["paths"], "linkedBlobReads": linked_blob_reads, "linkedExcerpt": linked_excerpt, + "malformedReplacementPaths": malformed_replacement_paths, "invalid": invalid, "legacy": legacy, "malformedRevision": malformed_revision, @@ -637,8 +664,9 @@ print(json.dumps({ "pathCollisionPaths": len(scope_collision_authority["paths"]), "replaced": replaced, "replacementBlobReads": replacement_blob_reads, - "replacementProbeCalls": replacement_probe_calls, - "replacementProbeMemoryError": replacement_probe_memory_error, + "replacementMemoryPaths": replacement_memory_paths, + "replacementProbeCommands": replacement_probe_commands, + "replacementProbeReadSizes": replacement_probe_read_sizes, "streamedWideTree": streamed_wide_tree, })) `; @@ -704,6 +732,7 @@ describe("workbench source excerpts", () => { linkedAuthorityPaths: [], linkedBlobReads: [], linkedExcerpt: null, + malformedReplacementPaths: [], invalid: null, legacy: null, malformedRevision: null, @@ -721,10 +750,11 @@ describe("workbench source excerpts", () => { pathCollisionPaths: 1, replaced: null, replacementBlobReads: [], - replacementProbeCalls: [ + replacementMemoryPaths: [], + replacementProbeCommands: [ ["for-each-ref", "--count=1", "--format=", "refs/replace/"], ], - replacementProbeMemoryError: false, + replacementProbeReadSizes: [1], streamedWideTree: true, }); }, 60_000); From 756e2bc333f0afd084369e9702c2bbcc7ac5e5dd Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:52:56 +0000 Subject: [PATCH 21/26] test(workbench): simplify replacement probe coverage --- sdk/typescript/tests-ts/workbench-source-excerpt.test.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 76ff74ffa..13a2828bd 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -450,12 +450,9 @@ def exhausted_popen(arguments, *positional, **keywords): return original_popen(arguments, *positional, **keywords) subprocess.Popen = exhausted_popen try: - try: - replacement_memory_paths = excerpts.capture_source_scopes( - repository, identity, ["src"] - )["paths"] - except MemoryError: - replacement_memory_paths = "escaped" + replacement_memory_paths = excerpts.capture_source_scopes( + repository, identity, ["src"] + )["paths"] finally: subprocess.Popen = watched_popen subprocess.Popen = original_popen From 566f5c4de7e1181e28b62d972c2e73c7809d60dd Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 24 Aug 2026 18:33:14 +0000 Subject: [PATCH 22/26] fix(workbench): honor replacement ref bases --- .../scripts/workbench_source_excerpt.py | 29 +++++++++++--- .../tests-ts/workbench-source-excerpt.test.ts | 38 ++++++++++++++++++- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index ceba34ac6..b7cf20721 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -257,13 +257,14 @@ def tree_path( def replacement_refs_absent(repository: Path) -> bool: + replacement_ref_base = os.environ.get("GIT_REPLACE_REF_BASE", "refs/replace/") environment = os.environ.copy() for variable in GIT_REPOSITORY_ENVIRONMENT: environment.pop(variable, None) environment["GIT_ALLOW_PROTOCOL"] = "" environment["GIT_LITERAL_PATHSPECS"] = "1" environment["GIT_NO_LAZY_FETCH"] = "1" - command = [ + git_command = [ "git", "-c", "core.fsmonitor=false", @@ -272,12 +273,30 @@ def replacement_refs_absent(repository: Path) -> bool: "-C", str(repository), "--no-replace-objects", - "for-each-ref", - "--count=1", - "--format=", - "refs/replace/", ] try: + if ( + subprocess.run( + [ + *git_command, + "check-ref-format", + f"{replacement_ref_base}{'0' * 64}", + ], + env=environment, + stdin=subprocess.DEVNULL, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ).returncode + != 0 + ): + return False + command = [ + *git_command, + "for-each-ref", + "--count=1", + "--format=", + f"{replacement_ref_base}*", + ] with subprocess.Popen( command, env=environment, diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 13a2828bd..4741610c0 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -428,6 +428,34 @@ finally: check=True, ) replacement_blob_reads = blob_reads[before:] +custom_replacement_base = "refs/synthetic-replacements/" +custom_replacement_ref = f"{custom_replacement_base}{revision}" +subprocess.run( + ["git", "-C", str(repository), "update-ref", custom_replacement_ref, replacement], + check=True, +) +original_replacement_base = os.environ.get("GIT_REPLACE_REF_BASE") +before = len(blob_reads) +try: + os.environ["GIT_REPLACE_REF_BASE"] = custom_replacement_base + custom_replacement_paths = excerpts.capture_source_scopes( + repository, identity, ["src"] + )["paths"] + custom_replaced = excerpt("src/allowed.py") + os.environ["GIT_REPLACE_REF_BASE"] = "--count=0" + invalid_replacement_base_paths = excerpts.capture_source_scopes( + repository, identity, ["src"] + )["paths"] +finally: + if original_replacement_base is None: + os.environ.pop("GIT_REPLACE_REF_BASE", None) + else: + os.environ["GIT_REPLACE_REF_BASE"] = original_replacement_base + subprocess.run( + ["git", "-C", str(repository), "update-ref", "-d", custom_replacement_ref], + check=True, + ) +custom_replacement_blob_reads = blob_reads[before:] replace_directory = outer_git_dir / "refs" / "replace" replace_directory.mkdir(parents=True, exist_ok=True) broken_replacements = [] @@ -625,6 +653,9 @@ print(json.dumps({ "broadenedBlobReads": broadened_blob_reads, "collisionBlobReads": collision_blob_reads, "collisions": collisions, + "customReplaced": custom_replaced, + "customReplacementBlobReads": custom_replacement_blob_reads, + "customReplacementPaths": custom_replacement_paths, "deepBatchProcesses": batch_processes, "deepExcerpt": deep_excerpt, "deepPathParses": path_parses, @@ -633,6 +664,7 @@ print(json.dumps({ "fileAuthorityPaths": file_authority["paths"], "fileDescendantBlobReads": file_descendant_blob_reads, "fileDescendantExcerpt": file_descendant_excerpt, + "invalidReplacementBasePaths": invalid_replacement_base_paths, "immutable": immutable, "largeExcerpt": large_excerpt, "largeBlobStreamed": ( @@ -709,6 +741,9 @@ describe("workbench source excerpts", () => { "src/trailing.py": null, "src/trailing.py.": null, }, + customReplaced: null, + customReplacementBlobReads: [], + customReplacementPaths: [], deepBatchProcesses: 1, deepExcerpt: expect.stringContaining("deep = True"), deepPathParses: 1, @@ -717,6 +752,7 @@ describe("workbench source excerpts", () => { fileAuthorityPaths: [{ kind: "file", path: "mismatch" }], fileDescendantBlobReads: [], fileDescendantExcerpt: null, + invalidReplacementBasePaths: [], immutable: { commit: expect.stringContaining("allowed = True"), range: expect.stringContaining("allowed = True"), @@ -749,7 +785,7 @@ describe("workbench source excerpts", () => { replacementBlobReads: [], replacementMemoryPaths: [], replacementProbeCommands: [ - ["for-each-ref", "--count=1", "--format=", "refs/replace/"], + ["for-each-ref", "--count=1", "--format=", "refs/replace/*"], ], replacementProbeReadSizes: [1], streamedWideTree: true, From 58fcda0a0c4dae49aac0fd7663dbc19cd23ff774 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Mon, 24 Aug 2026 14:49:54 -0700 Subject: [PATCH 23/26] fix(workbench): preserve excerpts for valid Git trees --- .../_bundled_plugin/.codex-plugin/plugin.json | 2 +- .../scripts/workbench_source_excerpt.py | 22 ++----- sdk/typescript/src/version.ts | 2 +- sdk/typescript/tests-ts/cost.test.ts | 4 +- .../tests-ts/diff-rank-input.test.ts | 2 +- .../tests-ts/workbench-source-excerpt.test.ts | 64 +++++++++++-------- 6 files changed, 46 insertions(+), 50 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/.codex-plugin/plugin.json b/sdk/typescript/_bundled_plugin/.codex-plugin/plugin.json index 4993c3c1f..bb41255c1 100644 --- a/sdk/typescript/_bundled_plugin/.codex-plugin/plugin.json +++ b/sdk/typescript/_bundled_plugin/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "codex-security", - "version": "0.1.28", + "version": "0.1.47", "description": "Codex Security workflows for security scans, analysis, and investigation.", "author": { "name": "OpenAI" diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index b7cf20721..fe3599b35 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -81,7 +81,6 @@ def matching_tree_entries( responses: IO[bytes], object_id: str, name: str, - name_bytes_limit: int, ) -> TreeEntry | None: encoded_object = object_id.encode("ascii") requests.write(encoded_object + b"\0") @@ -112,7 +111,7 @@ def read_more() -> None: buffered.extend(chunk) unread -= len(chunk) - def read_field(delimiter: int, maximum_bytes: int) -> bytearray: + def read_field(delimiter: int, maximum_bytes: int | None = None) -> bytearray: nonlocal cursor field = bytearray() while True: @@ -120,7 +119,7 @@ def read_field(delimiter: int, maximum_bytes: int) -> bytearray: end = buffered.index(delimiter, cursor) except ValueError: available = len(buffered) - cursor - if len(field) + available > maximum_bytes: + if maximum_bytes is not None and len(field) + available > maximum_bytes: raise ValueError("oversized tree field") from None field.extend(buffered[cursor:]) cursor = len(buffered) @@ -128,7 +127,7 @@ def read_field(delimiter: int, maximum_bytes: int) -> bytearray: raise ValueError("unterminated tree entry") from None read_more() continue - if len(field) + end - cursor > maximum_bytes: + if maximum_bytes is not None and len(field) + end - cursor > maximum_bytes: raise ValueError("oversized tree field") field.extend(buffered[cursor:end]) cursor = end + 1 @@ -150,7 +149,7 @@ def read_object_id(size: int) -> bytes: ambiguous = False while cursor < len(buffered) or unread: mode = bytes(read_field(ord(" "), 6)) - decoded_name = read_field(0, name_bytes_limit).decode( + decoded_name = read_field(0).decode( sys.getfilesystemencoding(), errors="surrogateescape" ) entry_object = read_object_id(object_id_bytes) @@ -187,18 +186,6 @@ def tree_path( kind, object_id = "directory", tree if not path.parts: return path.as_posix(), kind, object_id - try: - # Windows components are at most 255 UTF-16 units; Git stores path bytes. - name_bytes_limit = ( - 255 * 4 - if os.name == "nt" - else os.pathconf(repository, "PC_NAME_MAX") - ) - except (AttributeError, OSError, ValueError): - return None - if name_bytes_limit < 1: - return None - environment = os.environ.copy() for variable in GIT_REPOSITORY_ENVIRONMENT: environment.pop(variable, None) @@ -236,7 +223,6 @@ def tree_path( process.stdout, object_id, name, - name_bytes_limit, ) # The normalized name must be unique before an exact spelling can win. if aliases is None or aliases[0] != name: diff --git a/sdk/typescript/src/version.ts b/sdk/typescript/src/version.ts index 15252b77e..d81e0b0e6 100644 --- a/sdk/typescript/src/version.ts +++ b/sdk/typescript/src/version.ts @@ -9,7 +9,7 @@ const PACKAGE_VERSIONS = packageVersions( export const VERSION = PACKAGE_VERSIONS.package; export const CODEX_SDK_VERSION = PACKAGE_VERSIONS.sdk; export const CODEX_EXECUTABLE_VERSION = PACKAGE_VERSIONS.executable; -export const BUNDLED_PLUGIN_VERSION = "0.1.28" as const; +export const BUNDLED_PLUGIN_VERSION = "0.1.47" as const; const PACKAGE_NAME = "@openai/codex-security"; diff --git a/sdk/typescript/tests-ts/cost.test.ts b/sdk/typescript/tests-ts/cost.test.ts index 6b54715a4..af1f7fd17 100644 --- a/sdk/typescript/tests-ts/cost.test.ts +++ b/sdk/typescript/tests-ts/cost.test.ts @@ -1383,7 +1383,7 @@ describe("live scan cost tracking", () => { usage, }).toEqual({ predecessorVersion: "0.1.25", - upgradedVersion: "0.1.28", + upgradedVersion: "0.1.47", installedRootChanged: true, safetyIdentifierKey: "CODEX_SAFETY_IDENTIFIER", usage: { @@ -1391,7 +1391,7 @@ describe("live scan cost tracking", () => { warnings: [], }, }); - expect(BUNDLED_PLUGIN_VERSION).toBe("0.1.28"); + expect(BUNDLED_PLUGIN_VERSION).toBe("0.1.47"); }); test("forwards actions from this scan's delegated workers only", async () => { diff --git a/sdk/typescript/tests-ts/diff-rank-input.test.ts b/sdk/typescript/tests-ts/diff-rank-input.test.ts index 875ea19b3..83ddad599 100644 --- a/sdk/typescript/tests-ts/diff-rank-input.test.ts +++ b/sdk/typescript/tests-ts/diff-rank-input.test.ts @@ -68,7 +68,7 @@ function git(repository: string, ...args: string[]): string { } async function upgradeBundledPlugin(root: string): Promise { - expect(BUNDLED_PLUGIN_VERSION).toBe("0.1.28"); + expect(BUNDLED_PLUGIN_VERSION).toBe("0.1.47"); const previous = join(root, "previous-plugin"); cpSync(PLUGIN_ROOT, previous, { recursive: true }); const previousManifestPath = join(previous, ".codex-plugin", "plugin.json"); diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 4741610c0..9da772c54 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -88,6 +88,7 @@ function collisionRepository(root: string): { ["100644", "blob", blob("unicode_decomposed = True\n"), "é.py"], ["100644", "blob", blob("plain_name = True\n"), "trailing.py"], ["100644", "blob", blob("trailing_dot = True\n"), "trailing.py."], + ["100644", "blob", blob("uncheckoutable = True\n"), "z".repeat(2048)], ]); const upperScope = tree([ ["100644", "blob", blob("selected_scope = True\n"), "selected.py"], @@ -171,14 +172,14 @@ function ordinaryRepository(root: string): { } async function upgradedPlugin(root: string) { - expect(BUNDLED_PLUGIN_VERSION).toBe("0.1.28"); + expect(BUNDLED_PLUGIN_VERSION).toBe("0.1.47"); const previous = join(root, "previous-plugin"); cpSync(PLUGIN_ROOT, previous, { recursive: true }); const manifestPath = join(previous, ".codex-plugin", "plugin.json"); const manifest = JSON.parse(readFileSync(manifestPath, "utf8")) as { version: string; }; - manifest.version = "0.1.27"; + manifest.version = "0.1.46"; writeFileSync(manifestPath, JSON.stringify(manifest, null, 2) + "\n"); const home = join(root, "codex-home"); @@ -214,8 +215,8 @@ async function upgradedPlugin(root: string) { const installedMcp = JSON.parse( readFileSync(join(upgraded.installedRoot, ".mcp.json"), "utf8"), ) as { mcpServers: Record }; - expect(predecessor.version).toBe("0.1.27"); - expect(upgraded.version).toBe("0.1.28"); + expect(predecessor.version).toBe("0.1.46"); + expect(upgraded.version).toBe("0.1.47"); expect(upgraded.installedRoot).not.toBe(predecessor.installedRoot); expect( installedMcp.mcpServers["codex-security"]?.env_vars?.find( @@ -399,7 +400,7 @@ outer_git_dir = Path( ).strip() ) alternates = subtarget / ".git" / "objects" / "info" / "alternates" -alternates.write_text(str(outer_git_dir / "objects") + "\n") +alternates.write_bytes((str(outer_git_dir / "objects") + "\n").encode()) subprocess.run( ["git", "-C", str(subtarget), "update-ref", "refs/heads/main", revision], check=True, @@ -554,9 +555,8 @@ stream_responses = CappedRead( + wide_tree + b"\n" ) -name_limit = 255 * 4 if sys.platform == "win32" else os.pathconf(repository, "PC_NAME_MAX") streamed_aliases = excerpts.matching_tree_entries( - stream_requests, stream_responses, batch_object, "target.py", name_limit + stream_requests, stream_responses, batch_object, "target.py" ) alias_tree = (b"100644 target.py\0" + entry_object) * 20_000 alias_responses = CappedRead( @@ -565,33 +565,43 @@ alias_responses = CappedRead( + b"\n" ) ambiguous_alias = excerpts.matching_tree_entries( - io.BytesIO(), alias_responses, batch_object, "target.py", name_limit + io.BytesIO(), alias_responses, batch_object, "target.py" +) +oversized_name = b"x" * (64 * 1024 + 1) +oversized_tree = ( + b"100644 " + oversized_name + b"\0" + entry_object + + b"100644 target.py\0" + entry_object +) +oversized_responses = CappedRead( + f"{batch_object} tree {len(oversized_tree)}\n".encode() + + oversized_tree + + b"\n" +) +oversized_sibling = excerpts.matching_tree_entries( + io.BytesIO(), oversized_responses, batch_object, "target.py" +) +oversized_alias_tree = ( + b"100644 target.py\0" + entry_object + + b"100644 target.py" + b"." * (64 * 1024 + 1) + b"\0" + entry_object +) +oversized_alias_responses = CappedRead( + f"{batch_object} tree {len(oversized_alias_tree)}\n".encode() + + oversized_alias_tree + + b"\n" +) +oversized_alias = excerpts.matching_tree_entries( + io.BytesIO(), oversized_alias_responses, batch_object, "target.py" ) -oversized_name = b"x" * (name_limit + 1) -oversized_tree = b"100644 " + oversized_name + b"\0" + entry_object -try: - excerpts.matching_tree_entries( - io.BytesIO(), - io.BytesIO( - f"{batch_object} tree {len(oversized_tree)}\n".encode() - + oversized_tree - + b"\n" - ), - batch_object, - "target.py", - name_limit, - ) -except ValueError: - oversized_rejected = True -else: - oversized_rejected = False streamed_wide_tree = ( streamed_aliases == ("target.py", "file", entry_object.hex()) and stream_requests.getvalue() == batch_object.encode() + b"\0" and stream_responses.largest == 64 * 1024 and ambiguous_alias is None and alias_responses.largest == 64 * 1024 - and oversized_rejected + and oversized_sibling == ("target.py", "file", entry_object.hex()) + and oversized_responses.largest == 64 * 1024 + and oversized_alias is None + and oversized_alias_responses.largest == 64 * 1024 ) large_paths = [str(index) for index in range(20_000)] large_tree = "0" * 40 From cdcb5d1d3d2731838d40db53ee761bbe66e3393c Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Tue, 25 Aug 2026 00:45:34 -0400 Subject: [PATCH 24/26] fix(workbench): preserve distinct case-sensitive source paths --- .../scripts/workbench_source_excerpt.py | 12 +++++ .../tests-ts/workbench-source-excerpt.test.ts | 44 +++++++++++++++++-- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index fe3599b35..f1209fc86 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -81,6 +81,7 @@ def matching_tree_entries( responses: IO[bytes], object_id: str, name: str, + filesystem_parent: Path | None = None, ) -> TreeEntry | None: encoded_object = object_id.encode("ascii") requests.write(encoded_object + b"\0") @@ -154,6 +155,14 @@ def read_object_id(size: int) -> bytes: ) entry_object = read_object_id(object_id_bytes) if not ambiguous and normalized_path_component(decoded_name) == expected_name: + if filesystem_parent is not None and decoded_name != name: + try: + if not (filesystem_parent / name).samefile( + filesystem_parent / decoded_name + ): + continue + except OSError: + pass if selected is not None: selected = None ambiguous = True @@ -184,6 +193,7 @@ def tree_path( if selected_kinds is not None and selected_kinds.get(0, "directory") != "directory": return None kind, object_id = "directory", tree + filesystem_parent = repository if not path.parts: return path.as_posix(), kind, object_id environment = os.environ.copy() @@ -223,11 +233,13 @@ def tree_path( process.stdout, object_id, name, + filesystem_parent, ) # The normalized name must be unique before an exact spelling can win. if aliases is None or aliases[0] != name: return None _, kind, object_id = aliases + filesystem_parent /= name if ( selected_kinds is not None and (expected_kind := selected_kinds.get(depth)) is not None diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 9da772c54..9aa2504de 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -2,6 +2,7 @@ import { execFileSync, spawnSync } from "node:child_process"; import { randomUUID } from "node:crypto"; import { cpSync, + existsSync, mkdirSync, mkdtempSync, readFileSync, @@ -53,6 +54,7 @@ function git( } function collisionRepository(root: string): { + caseSensitive: boolean; deepPath: string; repository: string; replacement: string; @@ -146,6 +148,11 @@ function collisionRepository(root: string): { ); writeFileSync(join(repository, "src", "allowed.py"), "allowed = True\n"); writeFileSync(join(repository, "src", "LOWER.py"), "case_upper = True\n"); + const lowercasePath = join(repository, "src", "lower.py"); + const caseSensitive = !existsSync(lowercasePath); + if (caseSensitive) { + writeFileSync(lowercasePath, "case_lower = True\n"); + } writeFileSync(join(repository, "src", "é.py"), "unicode_composed = True\n"); writeFileSync(join(repository, "src", "trailing.py"), "plain_name = True\n"); writeFileSync(join(repository, "mismatch"), "selected_file = True\n"); @@ -153,7 +160,7 @@ function collisionRepository(root: string): { join(repository, "Scope", "selected.py"), "selected_scope = True\n", ); - return { deepPath, repository, replacement, revision }; + return { caseSensitive, deepPath, repository, replacement, revision }; } function ordinaryRepository(root: string): { @@ -349,6 +356,22 @@ collisions = { ) } collision_blob_reads = blob_reads[before:] +original_samefile = Path.samefile +def distinct_case_samefile(left, right): + right = Path(right) + if left.parent == right.parent and {left.name, right.name} == {"LOWER.py", "lower.py"}: + return False + return original_samefile(left, right) +Path.samefile = distinct_case_samefile +before = len(blob_reads) +try: + distinct_case_excerpts = { + path: excerpt(path) + for path in ("src/LOWER.py", "src/lower.py") + } +finally: + Path.samefile = original_samefile +distinct_case_blob_reads = blob_reads[before:] outside = excerpt("outside.py") file_authority = excerpts.capture_source_scopes(repository, identity, ["mismatch"]) file_scan = {**scan, "source_scopes_json": json.dumps(file_authority)} @@ -670,6 +693,8 @@ print(json.dumps({ "deepExcerpt": deep_excerpt, "deepPathParses": path_parses, "deepTreeReads": tree_reads, + "distinctCaseBlobReads": distinct_case_blob_reads, + "distinctCaseExcerpts": distinct_case_excerpts, "duplicatePaths": len(authority["paths"]), "fileAuthorityPaths": file_authority["paths"], "fileDescendantBlobReads": file_descendant_blob_reads, @@ -742,10 +767,16 @@ describe("workbench source excerpts", () => { allowed: expect.stringContaining("allowed = True"), broadened: null, broadenedBlobReads: [], - collisionBlobReads: [], + collisionBlobReads: fixture.caseSensitive + ? [expect.any(String), expect.any(String)] + : [], collisions: { - "src/LOWER.py": null, - "src/lower.py": null, + "src/LOWER.py": fixture.caseSensitive + ? expect.stringContaining("case_upper = True") + : null, + "src/lower.py": fixture.caseSensitive + ? expect.stringContaining("case_lower = True") + : null, "src/é.py": null, "src/é.py": null, "src/trailing.py": null, @@ -758,6 +789,11 @@ describe("workbench source excerpts", () => { deepExcerpt: expect.stringContaining("deep = True"), deepPathParses: 1, deepTreeReads: 0, + distinctCaseBlobReads: [expect.any(String), expect.any(String)], + distinctCaseExcerpts: { + "src/LOWER.py": expect.stringContaining("case_upper = True"), + "src/lower.py": expect.stringContaining("case_lower = True"), + }, duplicatePaths: 1, fileAuthorityPaths: [{ kind: "file", path: "mismatch" }], fileDescendantBlobReads: [], From 29513114e466316f20e88a91e9862d07d4457894 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Tue, 25 Aug 2026 01:23:00 -0400 Subject: [PATCH 25/26] fix(workbench): reuse source trees and avoid writer lock contention --- .../scripts/deep_scan_workbench.py | 27 +++-- .../scripts/workbench_source_excerpt.py | 114 +++++++++++++----- .../tests-ts/workbench-source-excerpt.test.ts | 96 ++++++++++++++- 3 files changed, 199 insertions(+), 38 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/deep_scan_workbench.py b/sdk/typescript/_bundled_plugin/scripts/deep_scan_workbench.py index f10d10c67..9a071f9a9 100644 --- a/sdk/typescript/_bundled_plugin/scripts/deep_scan_workbench.py +++ b/sdk/typescript/_bundled_plugin/scripts/deep_scan_workbench.py @@ -785,6 +785,23 @@ def begin_deep_scan_for_target( scope_file_count = directory_snapshot_regular_file_count( target if scope == "." else target / scope ) + current_target = require_remediation_target(target_path) + current_metadata = current_target.stat() + current_revision = git_revision(current_target) + current_snapshot_digest = ( + directory_content_digest(current_target) + if current_revision == "unversioned" + else worktree_content_digest(current_target) + ) + if ( + current_revision, + current_snapshot_digest, + serialize_filesystem_identity(current_metadata.st_dev), + serialize_filesystem_identity(current_metadata.st_ino), + ) != target_identity: + raise SystemExit( + "The selected scan target changed while the scan was starting. Try again." + ) connection.execute("BEGIN IMMEDIATE") try: existing = existing_deep_scan_for_target(connection, thread_id, target_path, scope) @@ -806,18 +823,10 @@ def begin_deep_scan_for_target( ) current_target = require_remediation_target(target_path) current_metadata = current_target.stat() - current_revision = git_revision(current_target) - current_snapshot_digest = ( - directory_content_digest(current_target) - if current_revision == "unversioned" - else worktree_content_digest(current_target) - ) if ( - current_revision, - current_snapshot_digest, serialize_filesystem_identity(current_metadata.st_dev), serialize_filesystem_identity(current_metadata.st_ino), - ) != target_identity: + ) != (target_device, target_inode): raise SystemExit( "The selected scan target changed while the scan was starting. Try again." ) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index f1209fc86..6fc599902 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -11,6 +11,7 @@ import stat import subprocess import sys +from contextlib import ExitStack from dataclasses import dataclass, field from pathlib import Path, PurePosixPath from typing import IO, Any @@ -32,12 +33,14 @@ LINE_BREAK = re.compile(r"\r\n|[\n\v\f\r\x1c-\x1e\x85\u2028\u2029]") TreeEntry = tuple[str, str, str] +TreeEntries = dict[str, list[TreeEntry]] @dataclass class SourceScopeIndex: kind: str | None = None children: dict[str, SourceScopeIndex] = field(default_factory=dict) + tree_entries: dict[str, TreeEntries] = field(default_factory=dict) SourceContext = tuple[Path, str, SourceScopeIndex] @@ -82,6 +85,7 @@ def matching_tree_entries( object_id: str, name: str, filesystem_parent: Path | None = None, + tree_cache: dict[str, TreeEntries] | None = None, ) -> TreeEntry | None: encoded_object = object_id.encode("ascii") requests.write(encoded_object + b"\0") @@ -148,13 +152,25 @@ def read_object_id(size: int) -> bytes: expected_name = normalized_path_component(name) selected = None ambiguous = False + entries: TreeEntries | None = {} if tree_cache is not None else None while cursor < len(buffered) or unread: mode = bytes(read_field(ord(" "), 6)) decoded_name = read_field(0).decode( sys.getfilesystemencoding(), errors="surrogateescape" ) entry_object = read_object_id(object_id_bytes) - if not ambiguous and normalized_path_component(decoded_name) == expected_name: + normalized_name = normalized_path_component(decoded_name) + kind = ( + "directory" + if mode in {b"40000", b"040000"} + else "file" + if mode in {b"100644", b"100755"} + else "other" + ) + entry = (decoded_name, kind, entry_object.hex()) + if entries is not None: + entries.setdefault(normalized_name, []).append(entry) + if not ambiguous and normalized_name == expected_name: if filesystem_parent is not None and decoded_name != name: try: if not (filesystem_parent / name).samefile( @@ -167,16 +183,31 @@ def read_object_id(size: int) -> bytes: selected = None ambiguous = True else: - kind = ( - "directory" - if mode in {b"40000", b"040000"} - else "file" - if mode in {b"100644", b"100755"} - else "other" - ) - selected = (decoded_name, kind, entry_object.hex()) + selected = entry if responses.read(1) != b"\n": raise ValueError("missing tree terminator") + if tree_cache is not None and entries is not None: + tree_cache[object_id] = entries + return selected + + +def matching_cached_tree_entry( + entries: TreeEntries, name: str, filesystem_parent: Path +) -> TreeEntry | None: + selected = None + for entry in entries.get(normalized_path_component(name), []): + decoded_name = entry[0] + if decoded_name != name: + try: + if not (filesystem_parent / name).samefile( + filesystem_parent / decoded_name + ): + continue + except OSError: + pass + if selected is not None: + return None + selected = entry return selected @@ -186,6 +217,7 @@ def tree_path( value: str, *, selected_kinds: dict[int, str] | None = None, + tree_cache: dict[str, TreeEntries] | None = None, ) -> TreeEntry | None: path = relative_path(value) if path is None: @@ -216,25 +248,40 @@ def tree_path( "-z", ] try: - with subprocess.Popen( - command, - env=environment, - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - stderr=subprocess.DEVNULL, - ) as process: - if process.stdin is None or process.stdout is None: - return None + with ExitStack() as processes: + process = None + pending_tree_entries: dict[str, TreeEntries] = {} for depth, name in enumerate(path.parts, start=1): if kind != "directory": return None - aliases = matching_tree_entries( - process.stdin, - process.stdout, - object_id, - name, - filesystem_parent, + cached = ( + tree_cache.get(object_id, pending_tree_entries.get(object_id)) + if tree_cache is not None + else None ) + if cached is not None: + aliases = matching_cached_tree_entry(cached, name, filesystem_parent) + else: + if process is None: + process = processes.enter_context( + subprocess.Popen( + command, + env=environment, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + ) + ) + if process.stdin is None or process.stdout is None: + return None + aliases = matching_tree_entries( + process.stdin, + process.stdout, + object_id, + name, + filesystem_parent, + pending_tree_entries if tree_cache is not None else None, + ) # The normalized name must be unique before an exact spelling can win. if aliases is None or aliases[0] != name: return None @@ -246,9 +293,14 @@ def tree_path( and kind != expected_kind ): return None - process.stdin.close() - if process.wait() != 0: - return None + if process is not None: + if process.stdin is None: + return None + process.stdin.close() + if process.wait() != 0: + return None + if tree_cache is not None: + tree_cache.update(pending_tree_entries) except (MemoryError, OSError, ValueError): return None return path.as_posix(), kind, object_id @@ -523,7 +575,13 @@ def finding_source_excerpt_from_context( try: selected_kinds = selected_source_kinds(scopes, path) selected = ( - tree_path(repository, tree, path, selected_kinds=selected_kinds) + tree_path( + repository, + tree, + path, + selected_kinds=selected_kinds, + tree_cache=scopes.tree_entries, + ) if selected_kinds is not None else None ) diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 9aa2504de..70f606104 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -83,6 +83,7 @@ function collisionRepository(root: string): { }; const sourceTree = tree([ ["100644", "blob", blob("allowed = True\n"), "allowed.py"], + ["100644", "blob", blob("another = True\n"), "another.py"], ["100644", "blob", blob("x".repeat(256 * 1024)), "large.py"], ["100644", "blob", blob("case_upper = True\n"), "LOWER.py"], ["100644", "blob", blob("case_lower = True\n"), "lower.py"], @@ -90,6 +91,7 @@ function collisionRepository(root: string): { ["100644", "blob", blob("unicode_decomposed = True\n"), "é.py"], ["100644", "blob", blob("plain_name = True\n"), "trailing.py"], ["100644", "blob", blob("trailing_dot = True\n"), "trailing.py."], + ["100644", "blob", blob("third = True\n"), "third.py"], ["100644", "blob", blob("uncheckoutable = True\n"), "z".repeat(2048)], ]); const upperScope = tree([ @@ -147,6 +149,7 @@ function collisionRepository(root: string): { "linked_allowed = True\n", ); writeFileSync(join(repository, "src", "allowed.py"), "allowed = True\n"); + writeFileSync(join(repository, "src", "another.py"), "another = True\n"); writeFileSync(join(repository, "src", "LOWER.py"), "case_upper = True\n"); const lowercasePath = join(repository, "src", "lower.py"); const caseSensitive = !existsSync(lowercasePath); @@ -155,6 +158,7 @@ function collisionRepository(root: string): { } writeFileSync(join(repository, "src", "é.py"), "unicode_composed = True\n"); writeFileSync(join(repository, "src", "trailing.py"), "plain_name = True\n"); + writeFileSync(join(repository, "src", "third.py"), "third = True\n"); writeFileSync(join(repository, "mismatch"), "selected_file = True\n"); writeFileSync( join(repository, "Scope", "selected.py"), @@ -559,6 +563,32 @@ try: finally: excerpts.local_git_bytes = original_git subprocess.Popen = original_popen +page_context = excerpts.source_excerpt_context(scan, repository, ["src"]) +page_batch_processes = 0 +page_tree_reads = 0 +original_matching_tree_entries = excerpts.matching_tree_entries +def watched_page_popen(arguments, *positional, **keywords): + global page_batch_processes + if "cat-file" in arguments and "--batch" in arguments: + page_batch_processes += 1 + return original_popen(arguments, *positional, **keywords) +def counted_page_tree(*arguments, **keywords): + global page_tree_reads + page_tree_reads += 1 + return original_matching_tree_entries(*arguments, **keywords) +subprocess.Popen = watched_page_popen +excerpts.matching_tree_entries = counted_page_tree +try: + page_excerpts = { + path: excerpts.finding_source_excerpt_from_context( + page_context, + [{"path": f"src/{path}", "startLine": 1, "role": "root_control"}], + ) + for path in ("allowed.py", "another.py", "third.py") + } +finally: + excerpts.matching_tree_entries = original_matching_tree_entries + subprocess.Popen = original_popen batch_object = "1" * 40 entry_object = b"\1" * 20 wide_tree = b"".join( @@ -721,6 +751,9 @@ print(json.dumps({ "nestedBlobReads": nested_blob_reads, "nestedExcerpt": nested_excerpt, "orderedExcerpt": ordered_excerpt, + "pageBatchProcesses": page_batch_processes, + "pageExcerpts": page_excerpts, + "pageTreeReads": page_tree_reads, "subtargetPaths": subtarget_authority["paths"], "outside": outside, "pathCollisionBlobReads": scope_collision_blob_reads, @@ -822,6 +855,13 @@ describe("workbench source excerpts", () => { nestedBlobReads: [], nestedExcerpt: null, orderedExcerpt: expect.stringContaining("allowed = True"), + pageBatchProcesses: 1, + pageExcerpts: { + "allowed.py": expect.stringContaining("allowed = True"), + "another.py": expect.stringContaining("another = True"), + "third.py": expect.stringContaining("third = True"), + }, + pageTreeReads: 2, subtargetPaths: [{ kind: "directory", path: "." }], outside: null, pathCollisionBlobReads: [], @@ -858,10 +898,16 @@ import workbench_db active_connection = None current_command = None race_mutation = False +race_replacement = False transaction_states = [] +digest_transaction_states = [] +blocked_digest_writers = [] +available_digest_writers = [] original_connect = workbench_db.connect original_capture = workbench_db.capture_source_scopes original_deep_capture = workbench_db.deep_scan.capture_source_scopes +original_deep_worktree_digest = workbench_db.deep_scan.worktree_content_digest +original_deep_remediation_target = workbench_db.deep_scan.require_remediation_target def connect(): global active_connection active_connection = original_connect() @@ -875,6 +921,24 @@ def deep_capture(*arguments, **keywords): if race_mutation: (Path(repository) / "src" / "allowed.py").write_text("changed_after_capture = True\n") return authority +def deep_worktree_digest(*arguments, **keywords): + digest_transaction_states.append([current_command, active_connection.in_transaction]) + with sqlite3.connect(workbench_db.database_path(), timeout=0) as concurrent: + try: + concurrent.execute("BEGIN IMMEDIATE") + except sqlite3.OperationalError as error: + blocked_digest_writers.append(str(error)) + else: + available_digest_writers.append(current_command) + concurrent.rollback() + return original_deep_worktree_digest(*arguments, **keywords) +def deep_remediation_target(value): + target = original_deep_remediation_target(value) + if race_replacement and active_connection.in_transaction: + target.rename(target.with_name(f"{target.name}-original")) + target.mkdir() + return original_deep_remediation_target(value) + return target def run(arguments): global current_command current_command = arguments[0] @@ -886,6 +950,8 @@ def run(arguments): workbench_db.connect = connect workbench_db.capture_source_scopes = capture workbench_db.deep_scan.capture_source_scopes = deep_capture +workbench_db.deep_scan.worktree_content_digest = deep_worktree_digest +workbench_db.deep_scan.require_remediation_target = deep_remediation_target repository, scan_root, cli_scan_dir, workspace_id = sys.argv[3:7] run(["create-workspace", "--workspace-id", workspace_id, "--thread-id", "workspace-writer"]) @@ -904,9 +970,21 @@ except SystemExit as error: else: race_error = None (Path(repository) / "src" / "allowed.py").write_text("allowed = True\n") +race_mutation = False +race_replacement = True +try: + run(["begin-deep-scan", "--thread-id", "replaced-deep-writer", "--target-path", repository, "--scan-root", scan_root, "--available-parallelism", "4"]) +except SystemExit as error: + replacement_error = str(error) +else: + replacement_error = None +finally: + replacement = Path(repository) + replacement.rmdir() + replacement.with_name(f"{replacement.name}-original").rename(replacement) with sqlite3.connect(workbench_db.database_path()) as connection: authorities = {row[0]: json.loads(row[1]) for row in connection.execute("SELECT id, source_scopes_json FROM scans")} -print(json.dumps({"authorities": authorities, "cli": cli, "deep": deep, "headless": headless, "prompt": prompt, "raceError": race_error, "transactionStates": transaction_states, "workspace": workspace})) +print(json.dumps({"authorities": authorities, "availableDigestWriters": available_digest_writers, "blockedDigestWriters": blocked_digest_writers, "cli": cli, "deep": deep, "digestTransactionStates": digest_transaction_states, "headless": headless, "prompt": prompt, "raceError": race_error, "replacementError": replacement_error, "transactionStates": transaction_states, "workspace": workspace})) `; const result = spawnSync( python(), @@ -933,10 +1011,26 @@ print(json.dumps({"authorities": authorities, "cli": cli, "deep": deep, "headles ["register-cli-scan", false], ["begin-deep-scan", false], ["begin-deep-scan", false], + ["begin-deep-scan", false], + ]); + expect(writers["digestTransactionStates"]).toEqual([ + ["begin-deep-scan", false], + ["begin-deep-scan", false], + ["begin-deep-scan", false], + ["begin-deep-scan", false], + ["begin-deep-scan", false], + ["begin-deep-scan", false], ]); + expect(writers["availableDigestWriters"]).toEqual( + Array.from({ length: 6 }, () => "begin-deep-scan"), + ); + expect(writers["blockedDigestWriters"]).toEqual([]); expect(writers["raceError"]).toBe( "The selected scan target changed while the scan was starting. Try again.", ); + expect(writers["replacementError"]).toBe( + "The selected scan target changed while the scan was starting. Try again.", + ); const authorities = writers["authorities"] as Record< string, { From 8baae67d58c12b323344c112c76b928d21bf4f92 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Tue, 25 Aug 2026 01:49:00 -0400 Subject: [PATCH 26/26] fix(workbench): preserve subtree roots and committed scope authority --- .../scripts/workbench_source_excerpt.py | 34 +++++-- .../tests-ts/workbench-source-excerpt.test.ts | 99 ++++++++++++++++++- 2 files changed, 123 insertions(+), 10 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py index 6fc599902..d78799d39 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_source_excerpt.py @@ -41,6 +41,7 @@ class SourceScopeIndex: kind: str | None = None children: dict[str, SourceScopeIndex] = field(default_factory=dict) tree_entries: dict[str, TreeEntries] = field(default_factory=dict) + filesystem_root: Path | None = None SourceContext = tuple[Path, str, SourceScopeIndex] @@ -218,6 +219,7 @@ def tree_path( *, selected_kinds: dict[int, str] | None = None, tree_cache: dict[str, TreeEntries] | None = None, + filesystem_root: Path | None = None, ) -> TreeEntry | None: path = relative_path(value) if path is None: @@ -225,7 +227,7 @@ def tree_path( if selected_kinds is not None and selected_kinds.get(0, "directory") != "directory": return None kind, object_id = "directory", tree - filesystem_parent = repository + filesystem_parent = repository if filesystem_root is None else filesystem_root if not path.parts: return path.as_posix(), kind, object_id environment = os.environ.copy() @@ -251,9 +253,11 @@ def tree_path( with ExitStack() as processes: process = None pending_tree_entries: dict[str, TreeEntries] = {} + resolved = True for depth, name in enumerate(path.parts, start=1): if kind != "directory": - return None + resolved = False + break cached = ( tree_cache.get(object_id, pending_tree_entries.get(object_id)) if tree_cache is not None @@ -284,7 +288,8 @@ def tree_path( ) # The normalized name must be unique before an exact spelling can win. if aliases is None or aliases[0] != name: - return None + resolved = False + break _, kind, object_id = aliases filesystem_parent /= name if ( @@ -292,7 +297,8 @@ def tree_path( and (expected_kind := selected_kinds.get(depth)) is not None and kind != expected_kind ): - return None + resolved = False + break if process is not None: if process.stdin is None: return None @@ -301,6 +307,8 @@ def tree_path( return None if tree_cache is not None: tree_cache.update(pending_tree_entries) + if not resolved: + return None except (MemoryError, OSError, ValueError): return None return path.as_posix(), kind, object_id @@ -411,9 +419,10 @@ def capture_source_scopes( context = target_tree(target, revision) if context is None: return authority - _, tree = context + repository, tree = context authority["targetTree"] = tree captured: set[str] = set() + captured_tree_entries: dict[str, TreeEntries] = {} for requested in paths: parsed = relative_path(requested) if parsed is None or safe_source_path(target, requested) is None: @@ -450,6 +459,18 @@ def capture_source_scopes( if kind is None: continue selected = parsed.as_posix() + if diff_target_kind in {"commit", "range"}: + committed = tree_path( + repository, + tree, + selected, + tree_cache=captured_tree_entries, + filesystem_root=target, + ) + if committed is None or committed[1] not in {"directory", "file"}: + continue + if kind == "file" or committed[1] == "file": + kind = "file" if selected not in captured: captured.add(selected) authority["paths"].append({"kind": kind, "path": selected}) @@ -492,7 +513,7 @@ def load_source_scopes( repository, tree = context if tree != expected_tree: return None - index = SourceScopeIndex() + index = SourceScopeIndex(filesystem_root=target) for record in records: if not isinstance(record, dict) or set(record) != {"kind", "path"}: return None @@ -581,6 +602,7 @@ def finding_source_excerpt_from_context( path, selected_kinds=selected_kinds, tree_cache=scopes.tree_entries, + filesystem_root=scopes.filesystem_root, ) if selected_kinds is not None else None diff --git a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts index 70f606104..1eb65bf2c 100644 --- a/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts +++ b/sdk/typescript/tests-ts/workbench-source-excerpt.test.ts @@ -117,6 +117,7 @@ function collisionRepository(root: string): { ["040000", "tree", upperScope, "Scope"], ["040000", "tree", linkedTree, "alias"], ["040000", "tree", deepTree, "deep"], + ["100644", "blob", blob("historical_file = True\n"), "historical"], ["040000", "tree", mismatchTree, "mismatch"], ["040000", "tree", lowerScope, "scope"], ["100644", "blob", blob("outside = True\n"), "outside.py"], @@ -142,6 +143,7 @@ function collisionRepository(root: string): { mkdirSync(join(repository, "src")); mkdirSync(join(repository, "Scope")); mkdirSync(join(repository, "deep")); + mkdirSync(join(repository, "historical")); mkdirSync(join(repository, "real", "subdir"), { recursive: true }); symlinkSync(join(repository, "real"), join(repository, "alias"), "junction"); writeFileSync( @@ -159,6 +161,10 @@ function collisionRepository(root: string): { writeFileSync(join(repository, "src", "é.py"), "unicode_composed = True\n"); writeFileSync(join(repository, "src", "trailing.py"), "plain_name = True\n"); writeFileSync(join(repository, "src", "third.py"), "third = True\n"); + writeFileSync( + join(repository, "historical", "checkout-only.py"), + "checkout_directory = True\n", + ); writeFileSync(join(repository, "mismatch"), "selected_file = True\n"); writeFileSync( join(repository, "Scope", "selected.py"), @@ -384,6 +390,22 @@ file_descendant_excerpt = excerpt( "mismatch/secret.py", file_scan, ["mismatch"] ) file_descendant_blob_reads = blob_reads[before:] +historical_authority = excerpts.capture_source_scopes( + repository, identity, ["historical"], diff_target_kind="commit" +) +historical_scan = { + **scan, + "mode": "diff", + "diff_target_kind": "commit", + "source_scopes_json": json.dumps(historical_authority), +} +historical_context = excerpts.source_excerpt_context( + historical_scan, repository, ["historical"] +) +historical_excerpt = excerpts.finding_source_excerpt_from_context( + historical_context, + [{"path": "historical", "startLine": 1, "role": "root_control"}], +) before = len(blob_reads) broadened = excerpt( "outside.py", @@ -419,6 +441,37 @@ subtarget_authority = excerpts.capture_source_scopes( ), ["."], ) +subtarget_scan = { + **scan, + "source_scopes_json": json.dumps(subtarget_authority), +} +subtarget_context = excerpts.source_excerpt_context( + subtarget_scan, subtarget, ["."] +) +def distinct_subtarget_samefile(left, right): + right = Path(right) + if ( + left.parent == subtarget + and right.parent == subtarget + and {left.name, right.name} == {"LOWER.py", "lower.py"} + ): + return False + return original_samefile(left, right) +Path.samefile = distinct_subtarget_samefile +try: + subtarget_case_excerpts = { + path: excerpts.finding_source_excerpt_from_context( + subtarget_context, + [{"path": path, "startLine": 1, "role": "root_control"}], + ) + for path in ("LOWER.py", "lower.py") + } + subtarget_unicode_alias = excerpts.finding_source_excerpt_from_context( + subtarget_context, + [{"path": "é.py", "startLine": 1, "role": "root_control"}], + ) +finally: + Path.samefile = original_samefile subprocess.run(["git", "-C", str(subtarget), "init", "-q"], check=True) outer_git_dir = Path( subprocess.check_output( @@ -432,10 +485,6 @@ subprocess.run( ["git", "-C", str(subtarget), "update-ref", "refs/heads/main", revision], check=True, ) -subtarget_scan = { - **scan, - "source_scopes_json": json.dumps(subtarget_authority), -} nested_context = excerpts.source_excerpt_context(subtarget_scan, subtarget, ["."]) before = len(blob_reads) nested_excerpt = excerpts.finding_source_excerpt_from_context( @@ -589,6 +638,31 @@ try: finally: excerpts.matching_tree_entries = original_matching_tree_entries subprocess.Popen = original_popen +missing_context = excerpts.source_excerpt_context(scan, repository, ["src"]) +missing_batch_processes = 0 +missing_tree_reads = 0 +def watched_missing_popen(arguments, *positional, **keywords): + global missing_batch_processes + if "cat-file" in arguments and "--batch" in arguments: + missing_batch_processes += 1 + return original_popen(arguments, *positional, **keywords) +def counted_missing_tree(*arguments, **keywords): + global missing_tree_reads + missing_tree_reads += 1 + return original_matching_tree_entries(*arguments, **keywords) +subprocess.Popen = watched_missing_popen +excerpts.matching_tree_entries = counted_missing_tree +try: + missing_excerpts = [ + excerpts.finding_source_excerpt_from_context( + missing_context, + [{"path": f"src/missing-{index}.py", "startLine": 1}], + ) + for index in range(3) + ] +finally: + excerpts.matching_tree_entries = original_matching_tree_entries + subprocess.Popen = original_popen batch_object = "1" * 40 entry_object = b"\1" * 20 wide_tree = b"".join( @@ -729,6 +803,8 @@ print(json.dumps({ "fileAuthorityPaths": file_authority["paths"], "fileDescendantBlobReads": file_descendant_blob_reads, "fileDescendantExcerpt": file_descendant_excerpt, + "historicalAuthorityPaths": historical_authority["paths"], + "historicalExcerpt": historical_excerpt, "invalidReplacementBasePaths": invalid_replacement_base_paths, "immutable": immutable, "largeExcerpt": large_excerpt, @@ -747,6 +823,9 @@ print(json.dumps({ "invalid": invalid, "legacy": legacy, "malformedRevision": malformed_revision, + "missingBatchProcesses": missing_batch_processes, + "missingExcerpts": missing_excerpts, + "missingTreeReads": missing_tree_reads, "mutable": {"excerpts": mutable, "gitCalls": len(git_calls)}, "nestedBlobReads": nested_blob_reads, "nestedExcerpt": nested_excerpt, @@ -754,7 +833,9 @@ print(json.dumps({ "pageBatchProcesses": page_batch_processes, "pageExcerpts": page_excerpts, "pageTreeReads": page_tree_reads, + "subtargetCaseExcerpts": subtarget_case_excerpts, "subtargetPaths": subtarget_authority["paths"], + "subtargetUnicodeAlias": subtarget_unicode_alias, "outside": outside, "pathCollisionBlobReads": scope_collision_blob_reads, "pathCollisionExcerpt": scope_collision_excerpt, @@ -831,6 +912,8 @@ describe("workbench source excerpts", () => { fileAuthorityPaths: [{ kind: "file", path: "mismatch" }], fileDescendantBlobReads: [], fileDescendantExcerpt: null, + historicalAuthorityPaths: [{ kind: "file", path: "historical" }], + historicalExcerpt: expect.stringContaining("historical_file = True"), invalidReplacementBasePaths: [], immutable: { commit: expect.stringContaining("allowed = True"), @@ -848,6 +931,9 @@ describe("workbench source excerpts", () => { invalid: null, legacy: null, malformedRevision: null, + missingBatchProcesses: 1, + missingExcerpts: [null, null, null], + missingTreeReads: 2, mutable: { excerpts: { working_tree: null, None: null }, gitCalls: 0, @@ -862,7 +948,12 @@ describe("workbench source excerpts", () => { "third.py": expect.stringContaining("third = True"), }, pageTreeReads: 2, + subtargetCaseExcerpts: { + "LOWER.py": expect.stringContaining("case_upper = True"), + "lower.py": expect.stringContaining("case_lower = True"), + }, subtargetPaths: [{ kind: "directory", path: "." }], + subtargetUnicodeAlias: null, outside: null, pathCollisionBlobReads: [], pathCollisionExcerpt: null,