diff --git a/CHANGELOG.md b/CHANGELOG.md index f709edb..187a217 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ Notable changes to Agent Code Guard are recorded here. ## Unreleased -- Reuse one immutable invocation context for configuration and canonical selected-file identities across all guards, and use shared source line indexes for constant-time syntax location mapping. +- Reuse one immutable invocation context for configuration and canonical selected-file identities during ordinary multi-guard analysis, and use shared source line indexes for constant-time syntax location mapping. - Add a reproducible, non-CI Wayfarer benchmark harness for LOC-only, syntax-only, normal, and profiled scans. ## 0.3.0 - 2026-08-28 diff --git a/docs/usage.md b/docs/usage.md index 688ff3c..1292ce8 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -325,23 +325,20 @@ export, and platform activation. ## Performance benchmarking Performance changes can be measured against the fixed Wayfarer workload with -`tools/benchmark-wayfarer.ps1`. The caller supplies a disposable checkout at +`tools/benchmark_wayfarer.py`. The caller supplies a disposable checkout at commit `679ddae9717bf78681a2cfbf794f687127b23b5d`, its exact project config, and an output directory outside that checkout: -```powershell -.\tools\benchmark-wayfarer.ps1 ` - -WayfarerPath C:\bench\Wayfarer ` - -ConfigPath C:\bench\wayfarer-code-guard.config.json ` - -OutputDirectory C:\bench\results\after ` - -InstallationMode "editable wheel from issue 122 branch" +```console +python tools/benchmark_wayfarer.py --wayfarer-path C:\bench\Wayfarer --config-path C:\bench\wayfarer-code-guard.config.json --output-directory C:\bench\results\after --installation-mode "editable wheel from issue 122 branch" ``` The script validates the source commit, records Python and Code Guard versions, the configuration hash, exact commands, three fresh sequential warm-process samples and medians for LOC-only, syntax-only, and normal six-guard scans, plus -a normal-run cProfile file. It compares complete Git status before and after the -run and fails if analysis creates repository metadata. It never clones or writes -to the target checkout and is intentionally not a CI test. Run the same script -and installation mode against the before and after revisions, retaining both -result directories for comparison. +a normal-run cProfile file. It requires successful complete Git-status checks +before and after the run, and fails if verification fails or analysis creates +repository metadata. It rejects output paths equal to or beneath the target, +never clones or writes to the target checkout, and is intentionally not a CI +test. Run the same script and installation mode against the before and after +revisions, retaining both result directories for comparison. diff --git a/src/agent_code_guard/code_guard.py b/src/agent_code_guard/code_guard.py index ac604d0..27de2a4 100644 --- a/src/agent_code_guard/code_guard.py +++ b/src/agent_code_guard/code_guard.py @@ -339,9 +339,23 @@ def run_analysis( if baseline is not None: loc_baseline.validate_paths(context.root, baseline) loc_baseline.validate_overlap(baseline, loc_config) + error = "baseline analysis scope is outside analysis root" + try: + current_root = context.root.resolve(strict=True) + except OSError as exc: + raise ValueError(f"{error}: {context.root}") from exc for selected in context.selected_files: - if not selected.physical_path.is_file() or not selected.physical_path.is_relative_to(context.root): - raise ValueError(f"baseline analysis scope is outside analysis root: {selected.physical_path}") + try: + current_path = selected.physical_path.resolve(strict=True) + valid = ( + not selected.physical_path.is_symlink() + and current_path.is_file() + and current_path.is_relative_to(current_root) + ) + except OSError: + valid = False + if not valid: + raise ValueError(f"{error}: {selected.physical_path}") baseline = dict(baseline) for target in linked_targets or set(): baseline.pop(target.relative_to(context.root).as_posix(), None) diff --git a/tests/test_benchmark_wayfarer.py b/tests/test_benchmark_wayfarer.py new file mode 100644 index 0000000..2d7c8a3 --- /dev/null +++ b/tests/test_benchmark_wayfarer.py @@ -0,0 +1,76 @@ +from __future__ import annotations + +import subprocess +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +from tools import benchmark_wayfarer + + +class BenchmarkWayfarerTests(unittest.TestCase): + def test_target_output_is_rejected_before_directory_creation(self) -> None: + with tempfile.TemporaryDirectory() as value: + target = Path(value) + config = target.parent / "benchmark-config.json" + config.write_text("{}", encoding="utf-8") + + with patch.object(Path, "mkdir", side_effect=AssertionError("output directory created")): + with self.assertRaisesRegex(ValueError, "outside the disposable Wayfarer checkout"): + benchmark_wayfarer.main([ + "--wayfarer-path", str(target), + "--config-path", str(config), + "--output-directory", str(target), + ]) + + def test_external_and_similarly_prefixed_sibling_outputs_are_allowed(self) -> None: + with tempfile.TemporaryDirectory() as value: + parent = Path(value) + target = parent / "Wayfarer" + target.mkdir() + + for output in (parent / "results", parent / "Wayfarer-other"): + self.assertEqual( + benchmark_wayfarer.validate_output_directory(target, output), + output.absolute(), + ) + + def test_descendant_output_is_rejected(self) -> None: + with tempfile.TemporaryDirectory() as value: + target = Path(value) / "Wayfarer" + target.mkdir() + + with self.assertRaisesRegex(ValueError, "outside the disposable Wayfarer checkout"): + benchmark_wayfarer.validate_output_directory(target, target / "results") + + def test_external_symlink_into_target_is_rejected(self) -> None: + with tempfile.TemporaryDirectory() as value: + parent = Path(value) + target = parent / "Wayfarer" + sink = target / "sink" + sink.mkdir(parents=True) + link = parent / "external-link" + try: + link.symlink_to(sink, target_is_directory=True) + except OSError as exc: + self.skipTest(f"directory symlink creation is unavailable: {exc}") + + with self.assertRaisesRegex(ValueError, "outside the disposable Wayfarer checkout"): + benchmark_wayfarer.validate_output_directory(target, link / "results") + + def test_failed_pre_run_git_status_is_rejected(self) -> None: + failed = subprocess.CompletedProcess([], 1, stdout="possibly clean", stderr="failure") + with patch.object(benchmark_wayfarer.subprocess, "run", return_value=failed): + with self.assertRaisesRegex(RuntimeError, "pre-run Git status verification failed"): + benchmark_wayfarer.git_status(Path("checkout"), "pre-run") + + def test_failed_post_run_git_status_is_rejected(self) -> None: + failed = subprocess.CompletedProcess([], 1, stdout="", stderr="failure") + with patch.object(benchmark_wayfarer.subprocess, "run", return_value=failed): + with self.assertRaisesRegex(RuntimeError, "post-run Git status verification failed"): + benchmark_wayfarer.git_status(Path("checkout"), "post-run") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_shared_invocation.py b/tests/test_shared_invocation.py index e515540..a3f28fc 100644 --- a/tests/test_shared_invocation.py +++ b/tests/test_shared_invocation.py @@ -4,6 +4,7 @@ from types import SimpleNamespace from unittest.mock import patch +from agent_code_guard import code_guard from agent_code_guard.analysis.provider import TreeSitterProvider from agent_code_guard.analysis.regions import executable_regions from agent_code_guard.analysis.facts import AnalysisFacts, FileFacts @@ -88,6 +89,42 @@ def __iter__(self): self.assertEqual(facts.reporting_path_for(Path("file-99.py")), "src/file-99.py") self.assertEqual(files.iterations, construction_iterations) + def test_loaded_baseline_rejects_selected_path_swapped_to_external_symlink(self): + with tempfile.TemporaryDirectory() as value, tempfile.TemporaryDirectory() as outside_value: + root = Path(value) + source = root / "sample.py" + outside = Path(outside_value) / "outside.py" + source.write_text("inside = 1\n", encoding="utf-8") + outside.write_text("outside = 1\n", encoding="utf-8") + args = code_guard.parser().parse_args(["."]) + with patch("agent_code_guard.file_selection.find_repo_root", return_value=None): + context = resolve_invocation(args, root, {}) + source.unlink() + try: + source.symlink_to(outside) + except OSError as exc: + self.skipTest(f"symlink creation is unavailable: {exc}") + + with patch.object(code_guard.loc, "run", side_effect=AssertionError("outside file analyzed")): + with self.assertRaisesRegex(ValueError, "baseline analysis scope is outside analysis root"): + code_guard.run_analysis( + context, args, baseline_override={}, baseline_loaded=True, + ) + + def test_loaded_baseline_accepts_unchanged_selected_file(self): + with tempfile.TemporaryDirectory() as value: + root = Path(value) + (root / "sample.py").write_text("inside = 1\n", encoding="utf-8") + args = code_guard.parser().parse_args(["."]) + with patch("agent_code_guard.file_selection.find_repo_root", return_value=None): + context = resolve_invocation(args, root, {}) + + completed = code_guard.run_analysis( + context, args, baseline_override={}, baseline_loaded=True, + ) + + self.assertEqual(completed.results[0].findings[0].path, "sample.py") + if __name__ == "__main__": unittest.main() diff --git a/tools/benchmark-wayfarer.ps1 b/tools/benchmark-wayfarer.ps1 deleted file mode 100644 index 4eea1e0..0000000 --- a/tools/benchmark-wayfarer.ps1 +++ /dev/null @@ -1,107 +0,0 @@ -param( - [Parameter(Mandatory = $true)][string]$WayfarerPath, - [Parameter(Mandatory = $true)][string]$ConfigPath, - [Parameter(Mandatory = $true)][string]$OutputDirectory, - [string]$Python = "python", - [string]$InstallationMode = "installed distribution" -) - -$ErrorActionPreference = "Stop" -$expectedCommit = "679ddae9717bf78681a2cfbf794f687127b23b5d" -$target = (Resolve-Path -LiteralPath $WayfarerPath).Path -$config = (Resolve-Path -LiteralPath $ConfigPath).Path -$output = [IO.Path]::GetFullPath($OutputDirectory) -if ($output.StartsWith($target + [IO.Path]::DirectorySeparatorChar, [StringComparison]::OrdinalIgnoreCase)) { - throw "OutputDirectory must be outside the disposable Wayfarer checkout." -} -$commit = (& git -C $target rev-parse HEAD).Trim() -if ($LASTEXITCODE -ne 0 -or $commit -ne $expectedCommit) { - throw "Wayfarer must be checked out at $expectedCommit; found $commit." -} -$before = (& git -C $target status --porcelain=v1 --untracked-files=all) -join "`n" -New-Item -ItemType Directory -Force -Path $output | Out-Null - -$base = Get-Content -LiteralPath $config -Raw | ConvertFrom-Json -AsHashtable -$guardNames = @("loc", "callableSize", "nesting", "cyclomaticComplexity", "markdownDocumentSize", "markdownSectionSize") -if ($base -isnot [hashtable] -or $base.guards -isnot [hashtable]) { - throw "Normal benchmark configuration must contain a guards object." -} -$configuredGuardNames = @($base.guards.Keys | Sort-Object) -if (Compare-Object -ReferenceObject @($guardNames | Sort-Object) -DifferenceObject $configuredGuardNames) { - throw "Normal benchmark configuration must contain exactly the six shipped guard sections." -} -foreach ($guard in $guardNames) { - if ($base.guards[$guard] -isnot [hashtable] -or $base.guards[$guard].enabled -ne $true) { - throw "Normal benchmark configuration must explicitly enable guards.$guard." - } -} -function New-Variant([string]$name, [hashtable]$enabled) { - $copy = $base | ConvertTo-Json -Depth 100 | ConvertFrom-Json -AsHashtable - foreach ($guard in $guardNames) { - $copy.guards[$guard].enabled = [bool]$enabled[$guard] - } - $path = Join-Path $output "$name.config.json" - $copy | ConvertTo-Json -Depth 100 | Set-Content -LiteralPath $path -Encoding utf8 - return $path -} -$locOnly = New-Variant "loc-only" @{ loc=$true; callableSize=$false; nesting=$false; cyclomaticComplexity=$false; markdownDocumentSize=$false; markdownSectionSize=$false } -$syntaxOnly = New-Variant "syntax-only" @{ loc=$false; callableSize=$true; nesting=$true; cyclomaticComplexity=$true; markdownDocumentSize=$false; markdownSectionSize=$false } - -$metadata = [ordered]@{ - recordedAtUtc = [DateTime]::UtcNow.ToString("o") - sourceCommit = $commit - configuration = $config - configurationSha256 = (Get-FileHash -LiteralPath $config -Algorithm SHA256).Hash - python = (& $Python --version 2>&1) -join " " - codeGuard = (& $Python -m agent_code_guard.code_guard --version 2>&1) -join " " - installationMode = $InstallationMode - workingDirectory = $target - samples = [ordered]@{} -} -function Measure-Variant([string]$name, [string]$variantConfig) { - $samples = @() - $warmupStdout = Join-Path $output "$name.warmup.json" - $warmupStderr = Join-Path $output "$name.warmup.stderr.txt" - $warmup = Start-Process -FilePath $Python -ArgumentList @( - "-m", "agent_code_guard.code_guard", ".", "--config", $variantConfig, "--json", "--ci" - ) -WorkingDirectory $target -Wait -PassThru -NoNewWindow -RedirectStandardOutput $warmupStdout -RedirectStandardError $warmupStderr - if ($warmup.ExitCode -ne 0) { throw "$name warmup failed with exit $($warmup.ExitCode)." } - for ($sample = 1; $sample -le 3; $sample++) { - $stdout = Join-Path $output "$name.sample-$sample.json" - $stderr = Join-Path $output "$name.sample-$sample.stderr.txt" - $watch = [Diagnostics.Stopwatch]::StartNew() - $process = Start-Process -FilePath $Python -ArgumentList @( - "-m", "agent_code_guard.code_guard", ".", "--config", $variantConfig, "--json", "--ci" - ) -WorkingDirectory $target -Wait -PassThru -NoNewWindow -RedirectStandardOutput $stdout -RedirectStandardError $stderr - $watch.Stop() - if ($process.ExitCode -ne 0) { throw "$name sample $sample failed with exit $($process.ExitCode)." } - $samples += [ordered]@{ sample=$sample; seconds=$watch.Elapsed.TotalSeconds; exitCode=$process.ExitCode; stdout=$stdout; stderr=$stderr } - } - $ordered = @($samples.seconds | Sort-Object) - $metadata.samples[$name] = [ordered]@{ - command="$Python -m agent_code_guard.code_guard . --config `"$variantConfig`" --json --ci" - warmup=[ordered]@{ exitCode=$warmup.ExitCode; stdout=$warmupStdout; stderr=$warmupStderr } - runs=$samples - medianSeconds=$ordered[1] - } -} -Measure-Variant "loc-only" $locOnly -Measure-Variant "syntax-only" $syntaxOnly -Measure-Variant "normal" $config - -$profile = Join-Path $output "normal.cprofile" -$profileStdout = Join-Path $output "normal.profile.json" -$profileStderr = Join-Path $output "normal.profile.stderr.txt" -$profileProcess = Start-Process -FilePath $Python -ArgumentList @( - "-m", "cProfile", "-o", $profile, "-m", "agent_code_guard.code_guard", ".", "--config", $config, "--json", "--ci" -) -WorkingDirectory $target -Wait -PassThru -NoNewWindow -RedirectStandardOutput $profileStdout -RedirectStandardError $profileStderr -if ($profileProcess.ExitCode -ne 0) { throw "Normal profile failed with exit $($profileProcess.ExitCode)." } -$metadata.profile = [ordered]@{ command="$Python -m cProfile -o `"$profile`" -m agent_code_guard.code_guard . --config `"$config`" --json --ci"; exitCode=$profileProcess.ExitCode; output=$profile; stdout=$profileStdout; stderr=$profileStderr } - -$after = (& git -C $target status --porcelain=v1 --untracked-files=all) -join "`n" -$metadata.targetStatusBefore = $before -$metadata.targetStatusAfter = $after -$metadata.normalAnalysisCreatedRepositoryMetadata = ($before -ne $after) -$metadata | ConvertTo-Json -Depth 20 | Set-Content -LiteralPath (Join-Path $output "benchmark-results.json") -Encoding utf8 -if ($before -ne $after) { throw "Benchmark changed the target checkout; inspect benchmark-results.json." } -Write-Output (Join-Path $output "benchmark-results.json") diff --git a/tools/benchmark_wayfarer.py b/tools/benchmark_wayfarer.py new file mode 100644 index 0000000..fa1259a --- /dev/null +++ b/tools/benchmark_wayfarer.py @@ -0,0 +1,198 @@ +"""Reproducible, non-CI benchmark for the fixed Wayfarer workload.""" + +from __future__ import annotations + +import argparse +import copy +import hashlib +import json +import os +import subprocess +import time +from datetime import datetime, timezone +from pathlib import Path + + +EXPECTED_COMMIT = "679ddae9717bf78681a2cfbf794f687127b23b5d" +GUARD_NAMES = ( + "loc", + "callableSize", + "nesting", + "cyclomaticComplexity", + "markdownDocumentSize", + "markdownSectionSize", +) + + +def validate_output_directory(target: Path, output: Path) -> Path: + """Return a normalized output path only when it is outside the target.""" + output_path = Path(os.path.abspath(output)) + target_path = target.resolve(strict=True) + + def is_contained(candidate: Path) -> bool: + target_text = os.path.normcase(str(target_path)) + candidate_text = os.path.normcase(str(candidate)) + try: + return os.path.commonpath((target_text, candidate_text)) == target_text + except ValueError: + return False + + if is_contained(output_path) or is_contained(output_path.resolve(strict=False)): + raise ValueError("OutputDirectory must be outside the disposable Wayfarer checkout.") + return output_path + + +def git_status(target: Path, phase: str) -> str: + result = subprocess.run( + ["git", "-C", str(target), "status", "--porcelain=v1", "--untracked-files=all"], + text=True, + capture_output=True, + check=False, + ) + if result.returncode != 0: + detail = result.stderr.strip() + message = f"{phase} Git status verification failed" + raise RuntimeError(f"{message}: {detail}" if detail else message) + return result.stdout.rstrip("\r\n") + + +def checked_output(command: list[str], *, cwd: Path | None = None) -> str: + result = subprocess.run(command, cwd=cwd, text=True, capture_output=True, check=False) + if result.returncode != 0: + detail = result.stderr.strip() + rendered = subprocess.list2cmdline(command) + raise RuntimeError(f"command failed with exit {result.returncode}: {rendered}{': ' + detail if detail else ''}") + return (result.stdout or result.stderr).strip() + + +def write_json(path: Path, value: object) -> None: + path.write_text(json.dumps(value, indent=2) + "\n", encoding="utf-8") + + +def run_redirected(command: list[str], cwd: Path, stdout: Path, stderr: Path) -> tuple[int, float]: + started = time.perf_counter() + with stdout.open("w", encoding="utf-8") as stdout_file, stderr.open("w", encoding="utf-8") as stderr_file: + result = subprocess.run(command, cwd=cwd, stdout=stdout_file, stderr=stderr_file, check=False) + return result.returncode, time.perf_counter() - started + + +def command_text(command: list[str]) -> str: + return subprocess.list2cmdline(command) + + +def parser() -> argparse.ArgumentParser: + result = argparse.ArgumentParser(description=__doc__) + result.add_argument("--wayfarer-path", required=True) + result.add_argument("--config-path", required=True) + result.add_argument("--output-directory", required=True) + result.add_argument("--python", default="python") + result.add_argument("--installation-mode", default="installed distribution") + return result + + +def main(arguments: list[str] | None = None) -> int: + args = parser().parse_args(arguments) + target = Path(args.wayfarer_path).resolve(strict=True) + config = Path(args.config_path).resolve(strict=True) + output = validate_output_directory(target, Path(args.output_directory)) + + commit = checked_output(["git", "-C", str(target), "rev-parse", "HEAD"]) + if commit != EXPECTED_COMMIT: + raise RuntimeError(f"Wayfarer must be checked out at {EXPECTED_COMMIT}; found {commit}.") + before = git_status(target, "pre-run") + output.mkdir(parents=True, exist_ok=True) + + base = json.loads(config.read_text(encoding="utf-8")) + guards = base.get("guards") if isinstance(base, dict) else None + if not isinstance(guards, dict): + raise ValueError("Normal benchmark configuration must contain a guards object.") + if set(guards) != set(GUARD_NAMES): + raise ValueError("Normal benchmark configuration must contain exactly the six shipped guard sections.") + for guard in GUARD_NAMES: + if not isinstance(guards[guard], dict) or guards[guard].get("enabled") is not True: + raise ValueError(f"Normal benchmark configuration must explicitly enable guards.{guard}.") + + def variant(name: str, enabled: set[str]) -> Path: + document = copy.deepcopy(base) + for guard in GUARD_NAMES: + document["guards"][guard]["enabled"] = guard in enabled + path = output / f"{name}.config.json" + write_json(path, document) + return path + + loc_only = variant("loc-only", {"loc"}) + syntax_only = variant("syntax-only", {"callableSize", "nesting", "cyclomaticComplexity"}) + metadata: dict[str, object] = { + "recordedAtUtc": datetime.now(timezone.utc).isoformat().replace("+00:00", "Z"), + "sourceCommit": commit, + "configuration": str(config), + "configurationSha256": hashlib.sha256(config.read_bytes()).hexdigest().upper(), + "python": checked_output([args.python, "--version"]), + "codeGuard": checked_output([args.python, "-m", "agent_code_guard.code_guard", "--version"]), + "installationMode": args.installation_mode, + "workingDirectory": str(target), + "samples": {}, + } + + def measure(name: str, variant_config: Path) -> None: + command = [ + args.python, "-m", "agent_code_guard.code_guard", ".", + "--config", str(variant_config), "--json", "--ci", + ] + warmup_stdout = output / f"{name}.warmup.json" + warmup_stderr = output / f"{name}.warmup.stderr.txt" + warmup_exit, _ = run_redirected(command, target, warmup_stdout, warmup_stderr) + if warmup_exit != 0: + raise RuntimeError(f"{name} warmup failed with exit {warmup_exit}.") + samples = [] + for sample in range(1, 4): + stdout = output / f"{name}.sample-{sample}.json" + stderr = output / f"{name}.sample-{sample}.stderr.txt" + exit_code, seconds = run_redirected(command, target, stdout, stderr) + if exit_code != 0: + raise RuntimeError(f"{name} sample {sample} failed with exit {exit_code}.") + samples.append({ + "sample": sample, "seconds": seconds, "exitCode": exit_code, + "stdout": str(stdout), "stderr": str(stderr), + }) + ordered = sorted(sample["seconds"] for sample in samples) + metadata["samples"][name] = { + "command": command_text(command), + "warmup": {"exitCode": warmup_exit, "stdout": str(warmup_stdout), "stderr": str(warmup_stderr)}, + "runs": samples, + "medianSeconds": ordered[1], + } + + measure("loc-only", loc_only) + measure("syntax-only", syntax_only) + measure("normal", config) + + profile = output / "normal.cprofile" + profile_stdout = output / "normal.profile.json" + profile_stderr = output / "normal.profile.stderr.txt" + profile_command = [ + args.python, "-m", "cProfile", "-o", str(profile), "-m", + "agent_code_guard.code_guard", ".", "--config", str(config), "--json", "--ci", + ] + profile_exit, _ = run_redirected(profile_command, target, profile_stdout, profile_stderr) + if profile_exit != 0: + raise RuntimeError(f"Normal profile failed with exit {profile_exit}.") + metadata["profile"] = { + "command": command_text(profile_command), "exitCode": profile_exit, + "output": str(profile), "stdout": str(profile_stdout), "stderr": str(profile_stderr), + } + + after = git_status(target, "post-run") + metadata["targetStatusBefore"] = before + metadata["targetStatusAfter"] = after + metadata["normalAnalysisCreatedRepositoryMetadata"] = before != after + results = output / "benchmark-results.json" + write_json(results, metadata) + if before != after: + raise RuntimeError("Benchmark changed the target checkout; inspect benchmark-results.json.") + print(results) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main())