diff --git a/.github/workflows/context-target-schema-compatibility-probe.yml b/.github/workflows/context-target-schema-compatibility-probe.yml new file mode 100644 index 00000000..17bba4df --- /dev/null +++ b/.github/workflows/context-target-schema-compatibility-probe.yml @@ -0,0 +1,122 @@ +name: Context target schema compatibility probe + +on: + workflow_dispatch: + inputs: + run_probe: + description: Send exactly one request per fixed schema variant to the configured LLM + type: boolean + required: true + default: false + +permissions: + contents: read + +concurrency: + group: context-target-schema-compatibility-probe + cancel-in-progress: false + +jobs: + probe: + if: >- + github.event_name == 'workflow_dispatch' && + github.repository == 'groktopus/codereview' && + github.ref == 'refs/heads/main' && + inputs.run_probe == true + runs-on: ubuntu-latest + timeout-minutes: 15 + permissions: + contents: read + steps: + - name: Check out the runner at the immutable dispatch revision + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.sha }} + fetch-depth: 0 + persist-credentials: false + + - name: Verify source dispatch context + env: + ACTUAL_REPOSITORY: ${{ github.repository }} + ACTUAL_REF: ${{ github.ref }} + ACTUAL_EVENT: ${{ github.event_name }} + EXPECTED_SHA: ${{ github.sha }} + run: | + python3 - <<'PY' + import os + import re + import subprocess + + if os.environ.get("ACTUAL_REPOSITORY") != "groktopus/codereview": + raise SystemExit("probe_repository_invalid") + if os.environ.get("ACTUAL_REF") != "refs/heads/main" or os.environ.get("ACTUAL_EVENT") != "workflow_dispatch": + raise SystemExit("probe_dispatch_context_invalid") + expected = os.environ.get("EXPECTED_SHA", "") + if not re.fullmatch(r"[0-9a-f]{40}", expected): + raise SystemExit("probe_source_sha_invalid") + result = subprocess.run(["git", "rev-parse", "HEAD"], capture_output=True, text=True, timeout=10, check=False) + if result.returncode or result.stdout.strip() != expected: + raise SystemExit("probe_source_sha_mismatch") + PY + + - name: Acquire fixed PR305 Git objects without checking out PR files + env: + GIT_TERMINAL_PROMPT: "0" + GIT_LFS_SKIP_SMUDGE: "1" + GIT_CONFIG_NOSYSTEM: "1" + GIT_CONFIG_GLOBAL: /dev/null + run: | + set -euo pipefail + target_repo="$RUNNER_TEMP/pr305-source.git" + git init --bare "$target_repo" + git -C "$target_repo" config core.hooksPath /dev/null + git -C "$target_repo" remote add origin https://github.com/magnus919/SlopSearX.git + timeout 120s git -C "$target_repo" -c http.lowSpeedLimit=1000 -c http.lowSpeedTime=30 fetch \ + --no-tags --no-recurse-submodules --filter=blob:none --depth=10000 origin \ + 00accc58a42eaa470e12831498b572cab2483981 \ + 90165dd65be3595006171d7abef34a082c90a71d + git -C "$target_repo" cat-file -e '00accc58a42eaa470e12831498b572cab2483981^{commit}' + git -C "$target_repo" cat-file -e '90165dd65be3595006171d7abef34a082c90a71d^{commit}' + test "$(git -C "$target_repo" merge-base 00accc58a42eaa470e12831498b572cab2483981 90165dd65be3595006171d7abef34a082c90a71d)" = \ + "00accc58a42eaa470e12831498b572cab2483981" + + - name: Set up Python + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + with: + python-version: "3.13" + + - name: Install the fixed review harness source + run: python -m pip install --disable-pip-version-check . + + - name: Prepare the exact fresh task pair without provider calls + env: + TARGET_SOURCE_DIR: ${{ runner.temp }}/pr305-source.git + LLM_MODEL: ${{ secrets.LLM_MODEL }} + RUNNER_TEMP: ${{ runner.temp }} + GIT_TERMINAL_PROMPT: "0" + GIT_LFS_SKIP_SMUDGE: "1" + GIT_CONFIG_NOSYSTEM: "1" + GIT_CONFIG_GLOBAL: /dev/null + run: python scripts/run_context_target_schema_probe.py --prepare-only + + - name: Compare the fixed old and candidate schema encodings + env: + TARGET_SOURCE_DIR: ${{ runner.temp }}/pr305-source.git + LLM_BASE_URL: ${{ secrets.LLM_BASE_URL }} + LLM_MODEL: ${{ secrets.LLM_MODEL }} + LLM_API_KEY: ${{ secrets.LLM_API_KEY }} + RUNNER_TEMP: ${{ runner.temp }} + GIT_TERMINAL_PROMPT: "0" + GIT_LFS_SKIP_SMUDGE: "1" + GIT_CONFIG_NOSYSTEM: "1" + GIT_CONFIG_GLOBAL: /dev/null + run: python scripts/run_context_target_schema_probe.py + + - name: Upload only the bounded probe summary + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: context-target-schema-compatibility-${{ github.run_id }} + path: ${{ runner.temp }}/context-target-schema-probe-summary.json + if-no-files-found: ignore + retention-days: 30 diff --git a/pyproject.toml b/pyproject.toml index cf571f7f..0a6c3d43 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,7 +24,7 @@ where = ["src"] [project.optional-dependencies] github-app = ["PyJWT[crypto]>=2.10,<3"] -dev = ["build>=1.2,<2", "pytest>=8", "ruff>=0.9", "setuptools>=68", "PyJWT[crypto]>=2.10,<3", "PyYAML>=6,<7"] +dev = ["build>=1.2,<2", "pytest>=8", "ruff>=0.9", "setuptools>=68", "PyJWT[crypto]>=2.10,<3", "PyYAML>=6,<7", "jsonschema>=4.18,<5"] [tool.pytest.ini_options] testpaths = ["tests"] diff --git a/scripts/run_context_target_schema_probe.py b/scripts/run_context_target_schema_probe.py new file mode 100644 index 00000000..feb14b81 --- /dev/null +++ b/scripts/run_context_target_schema_probe.py @@ -0,0 +1,443 @@ +#!/usr/bin/env python3 +"""Compare two fixed target-schema encodings against one frozen PR305 task. + +The workflow is manually gated and supplies only the configured LLM endpoint, +model, and credential. This script has no caller-controlled repository, commit, +profile, request, or URL inputs and makes at most two provider attempts. +""" + +from __future__ import annotations + +import copy +import hashlib +import io +import json +import os +import re +import subprocess +import sys +import tarfile +import tempfile +from pathlib import Path +from urllib.parse import urlsplit + +from pr_review_harness import contracts +from pr_review_harness.engine import _evidence_for, _hash, effective_task_input_ceiling, prepare_plan_tasks +from pr_review_harness.planner import plan_review +from pr_review_harness.providers import OpenAIProvider, ProviderError +from pr_review_harness.snapshot import collect_snapshot + +REPOSITORY = "groktopus/codereview" +TARGET_REPOSITORY = "magnus919/SlopSearX" +BASE_SHA = "00accc58a42eaa470e12831498b572cab2483981" +HEAD_SHA = "90165dd65be3595006171d7abef34a082c90a71d" +PROFILE_SHA256 = "28725e806a87812386ff67f5c6e63320cc9f3f2b9c1a79c6942c7cf1be004dac" +LIMITS_SHA256 = "ec191dcde1672b4f86aac24ee6412752cd9a08871d1aba1b332d11341b6fd2a6" +TASK_ID = "task-0151e9e9cad39a3b:chunk-1" +TASK_INPUT_SHA256 = "5e9d85b07e220c49103a13d2e20ef77e5e9c8da0f392e9db84b46260b9f94b73" +MANIFEST_SHA256 = "6b342ead0b67b0ab9df1c100f249d12621d2d4894507dbc1ee82d1284c79f3e0" +LEGACY_PROVIDER_COMMIT = "b075e50db01154cbd996a5549afef62f065df2d9" +LEGACY_PROVIDER_FILE_SHA256 = "2044a9fed9fcbcab4846b842df3b725043f92ac37c85a05850976449de8ad4c8" +UNIT_IDS = [ + "unit-9ec6727d1355643feb4e", + "unit-d857a6647fc0642fe459", + "unit-a375830d2b400d606450", +] +TARGET_LENS = "maintainability" +HISTORICAL_OLD_ERROR = {"status": 400, "response_bytes": 211, + "response_sha256": "63770a1ce1eb574d5791e070af9a4caf18754328f806e076e7d2cac6ae0ccffc"} +PROFILE_PATH = "profiles/slopsearx-v17-context-target-manifest-candidate.json" +LIMITS_PATH = "profiles/ordinary-review-limits-v3.json" +OUTPUT_NAME = "context-target-schema-probe-summary.json" +PREPARATION_NAME = "context-target-schema-probe-preparation.json" + + +def _legacy_request_from_archived_provider( + root: Path, task: dict, evidence: list[dict], limits: dict, model: str +) -> bytes: + """Run the actual archived provider implementation against the fresh task.""" + provider_path = "src/pr_review_harness/providers.py" + file_hash = subprocess.run( + ["git", "-C", str(root), "show", f"{LEGACY_PROVIDER_COMMIT}:{provider_path}"], + capture_output=True, + timeout=20, + check=True, + ).stdout + if hashlib.sha256(file_hash).hexdigest() != LEGACY_PROVIDER_FILE_SHA256: + raise ValueError("probe_legacy_provider_source_mismatch") + with tempfile.TemporaryDirectory(prefix="context-target-legacy-provider-") as temp_name: + temp = Path(temp_name) + archive = subprocess.run( + ["git", "-C", str(root), "archive", LEGACY_PROVIDER_COMMIT, "src/pr_review_harness"], + capture_output=True, + timeout=30, + check=True, + ).stdout + with tarfile.open(fileobj=io.BytesIO(archive), mode="r:") as tar: + tar.extractall(temp, filter="data") + child = r""" +import json, sys +from pr_review_harness.providers import OpenAIProvider +payload = json.load(sys.stdin) +provider = OpenAIProvider({ + "kind": "openai_compatible", "provider_id": "schema-compatibility-probe", + "base_url": "https://offline.invalid/v1", "model": payload["model"], + "api_key_env": "PROBE_UNUSED_KEY", "max_request_bytes": payload["limits"]["max_input_bytes_per_task"], + "max_response_bytes": payload["limits"]["max_output_bytes_per_task"], + "max_output_tokens": payload["limits"]["max_output_tokens"], "timeout_seconds": 30, +}) +system, user, schema = provider._review_parts(payload["task"], payload["evidence"]) +sys.stdout.write(provider._request_bytes(system, user, schema, payload["limits"]).decode("utf-8")) +""" + env = dict(os.environ) + env["PYTHONPATH"] = str(temp / "src") + env.pop("LLM_API_KEY", None) + completed = subprocess.run( + [sys.executable, "-c", child], + input=json.dumps( + {"task": task, "evidence": evidence, "limits": limits, "model": model}, + separators=(",", ":"), + ), + capture_output=True, + text=True, + timeout=30, + check=False, + env=env, + ) + if completed.returncode: + raise ValueError("probe_legacy_provider_prepare_failed") + return completed.stdout.encode("utf-8") + + +def _request_schema_pair(root: Path, provider: OpenAIProvider, task: dict, evidence: list[dict], limits: dict): + system, user, candidate_schema = provider._review_parts(task, evidence) + candidate_body = provider._request_bytes(system, user, candidate_schema, limits) + legacy_body = _legacy_request_from_archived_provider(root, task, evidence, limits, provider.model) + + legacy_doc, candidate_doc = json.loads(legacy_body), json.loads(candidate_body) + legacy_schema = copy.deepcopy(legacy_doc["response_format"]["json_schema"]) + legacy_target = legacy_doc["response_format"]["json_schema"]["schema"]["properties"] + candidate_target = candidate_doc["response_format"]["json_schema"]["schema"]["properties"] + legacy_target["context_gap_proposals"]["items"]["properties"]["target"] = None + candidate_target["context_gap_proposals"]["items"]["properties"]["target"] = None + if legacy_doc != candidate_doc: + raise ValueError("probe_request_diff_exceeds_target_schema") + return system, user, legacy_schema, candidate_schema, legacy_body, candidate_body + + +def _run_variant(provider, *, system, user, schema, limits, body) -> dict: + try: + _parsed, metadata = provider._call( + system=system, + user=user, + schema=schema, + limits=limits, + contract_version=contracts.SPECIALIST_V4, + serialized_request_bytes=body, + ) + exchange = metadata.get("local_http_exchange", {}) + return { + "state": "HTTP_RESPONSE", + "http_status": exchange.get("http_status"), + "request_attempted": exchange.get("request_attempted") is True, + "request_bytes": exchange.get("request_bytes"), + "request_sha256": exchange.get("request_sha256"), + "response_bytes": exchange.get("response_bytes"), + "response_sha256": exchange.get("response_sha256"), + "elapsed_ms": exchange.get("elapsed_ms"), + } + except ProviderError as exc: + exchange = exc.meta.get("local_http_exchange", {}) if isinstance(exc.meta, dict) else {} + code = exc.code if isinstance(exc.code, str) and re.fullmatch(r"[a-z0-9_]{1,100}", exc.code) else "provider_error" + return { + "state": "PROVIDER_ERROR", + "error_code": code, + "http_status": exchange.get("http_status"), + "request_attempted": exchange.get("request_attempted") is True, + "request_bytes": exchange.get("request_bytes"), + "request_sha256": exchange.get("request_sha256"), + "response_bytes": exchange.get("response_bytes"), + "response_sha256": exchange.get("response_sha256"), + "elapsed_ms": exchange.get("elapsed_ms"), + } + except Exception as exc: + metadata = getattr(exc, "meta", {}) + exchange = metadata.get("local_http_exchange", {}) if isinstance(metadata, dict) else {} + return { + "state": "LOCAL_ERROR", + "error_code": "unexpected_probe_error", + "http_status": exchange.get("http_status"), + "request_attempted": exchange.get("request_attempted") is True, + "request_bytes": exchange.get("request_bytes"), + "request_sha256": exchange.get("request_sha256"), + "response_bytes": exchange.get("response_bytes"), + "response_sha256": exchange.get("response_sha256"), + "elapsed_ms": exchange.get("elapsed_ms"), + } + + +def _require_runtime_context(prepare_only: bool = False) -> Path: + if not prepare_only and ( + os.environ.get("GITHUB_EVENT_NAME") != "workflow_dispatch" + or os.environ.get("GITHUB_REPOSITORY") != REPOSITORY + or os.environ.get("GITHUB_REF") != "refs/heads/main" + or not re.fullmatch(r"[0-9a-f]{40}", os.environ.get("GITHUB_SHA", "")) + ): + raise ValueError("probe_dispatch_context_invalid") + root = Path(__file__).resolve().parents[1] + if hashlib.sha256((root / PROFILE_PATH).read_bytes()).hexdigest() != PROFILE_SHA256: + raise ValueError("probe_profile_hash_mismatch") + if hashlib.sha256((root / LIMITS_PATH).read_bytes()).hexdigest() != LIMITS_SHA256: + raise ValueError("probe_limits_hash_mismatch") + source = Path(os.environ.get("TARGET_SOURCE_DIR", "")) + if not source.is_dir(): + raise ValueError("probe_target_source_missing") + bare = subprocess.run( + ["git", "-C", str(source), "rev-parse", "--is-bare-repository"], + capture_output=True, + text=True, + timeout=10, + check=False, + ) + if bare.returncode or bare.stdout.strip() != "true": + raise ValueError("probe_target_source_must_be_bare") + for revision in (BASE_SHA, HEAD_SHA): + result = subprocess.run( + ["git", "-C", str(source), "cat-file", "-e", f"{revision}^{{commit}}"], + capture_output=True, + timeout=10, + check=False, + ) + if result.returncode: + raise ValueError("probe_target_revision_missing") + merge_base = subprocess.run( + ["git", "-C", str(source), "merge-base", BASE_SHA, HEAD_SHA], + capture_output=True, + text=True, + timeout=10, + check=False, + ) + if merge_base.returncode or merge_base.stdout.strip() != BASE_SHA: + raise ValueError("probe_target_comparison_base_mismatch") + return root + + +def _provider_config_from_environment(limits: dict) -> dict: + base_url = os.environ.get("LLM_BASE_URL", "") + model = os.environ.get("LLM_MODEL", "") + api_key = os.environ.get("LLM_API_KEY", "") + try: + parsed = urlsplit(base_url) + _ = parsed.port + except ValueError: + raise ValueError("probe_provider_configuration_invalid") from None + if ( + not base_url + or len(base_url.encode("utf-8")) > 2048 + or base_url != base_url.strip() + or parsed.scheme != "https" + or not parsed.hostname + or parsed.username + or parsed.password + or parsed.query + or parsed.fragment + or parsed.path.rstrip("/").endswith("/chat/completions") + or any(ord(char) < 32 or ord(char) == 127 for char in base_url) + or not model + or len(model.encode("utf-8")) > 256 + or model != model.strip() + or any(ord(char) < 32 or ord(char) == 127 for char in model) + or not api_key + or len(api_key.encode("utf-8")) > 8192 + or api_key != api_key.strip() + or any(ord(char) < 32 or ord(char) == 127 for char in api_key) + ): + raise ValueError("probe_provider_configuration_invalid") + return { + "kind": "openai_compatible", + "provider_id": "schema-compatibility-probe", + "base_url": base_url.rstrip("/"), + "model": model, + "api_key_env": "LLM_API_KEY", + "max_request_bytes": limits["max_input_bytes_per_task"], + "max_response_bytes": limits["max_output_bytes_per_task"], + "max_output_tokens": limits["max_output_tokens"], + "timeout_seconds": min(limits["deadline_seconds"], 30), + } + + +def _offline_provider_config(limits: dict) -> dict: + """Provider shape for planning/serialization only; it cannot dispatch.""" + model = os.environ.get("LLM_MODEL") + if not model and os.environ.get("GITHUB_EVENT_NAME") == "workflow_dispatch": + raise ValueError("probe_provider_configuration_missing") + model = model or "offline-prepare-only" + if ( + not model + or len(model.encode("utf-8")) > 256 + or model != model.strip() + or any(ord(char) < 32 or ord(char) == 127 for char in model) + ): + raise ValueError("probe_provider_configuration_invalid") + return { + "kind": "openai_compatible", + "provider_id": "schema-compatibility-probe-offline-prepare", + "base_url": "https://offline.invalid/v1", + "model": model, + "api_key_env": "PROBE_UNUSED_KEY", + "max_request_bytes": limits["max_input_bytes_per_task"], + "max_response_bytes": limits["max_output_bytes_per_task"], + "max_output_tokens": limits["max_output_tokens"], + "timeout_seconds": 30, + } + + +def run_probe(prepare_only: bool = False) -> dict: + root = _require_runtime_context(prepare_only) + profile = json.loads((root / PROFILE_PATH).read_text(encoding="utf-8")) + limits = json.loads((root / LIMITS_PATH).read_text(encoding="utf-8")) + provider = OpenAIProvider( + _offline_provider_config(limits) if prepare_only else _provider_config_from_environment(limits) + ) + provider.preflight("SPECIALIST_FINDINGS") + source = os.environ["TARGET_SOURCE_DIR"] + snapshot = collect_snapshot(source, BASE_SHA, HEAD_SHA, profile, limits, pr_diff=True) + plan = plan_review(snapshot, profile, "FOCUSED") + prepared, _skipped = prepare_plan_tasks(snapshot, plan, profile, limits, provider) + scoped = [ + row for row in prepared + if row.get("unit_ids") == UNIT_IDS + and row.get("obligation_ids") + and all(f":lens:{TARGET_LENS}" in obligation for obligation in row["obligation_ids"]) + ] + if len(scoped) != 1: + raise ValueError("probe_fixed_scope_not_unique") + task = scoped[0] + manifest = task.get("context_target_manifest") + if not isinstance(manifest, dict): + raise ValueError("probe_manifest_missing") + evidence = _evidence_for(task, snapshot, effective_task_input_ceiling(limits, provider)) + task_input_hash = _hash({"evidence": evidence, "context_target_manifest": manifest}) + system, user, old_schema, new_schema, old_body, new_body = _request_schema_pair( + root, provider, task, evidence, limits + ) + if max(len(old_body), len(new_body)) > limits["max_input_bytes_per_task"]: + raise ValueError("probe_request_exceeds_fixed_input_cap") + + if not prepare_only: + preparation_path = Path(os.environ.get("RUNNER_TEMP", "/tmp")) / PREPARATION_NAME + try: + preparation = json.loads(preparation_path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + raise ValueError("probe_fresh_preparation_missing") from None + expected_fresh_binding = { + "base_sha": BASE_SHA, + "head_sha": HEAD_SHA, + "snapshot_id": snapshot.get("snapshot_id"), + "profile_sha256": PROFILE_SHA256, + "limits_sha256": LIMITS_SHA256, + "task_id": task.get("task_id"), + "task_input_sha256": task_input_hash, + "manifest_sha256": manifest.get("manifest_sha256"), + "probe_source_sha": os.environ.get("GITHUB_SHA"), + "variants": { + "whole_object_enum": { + "request_body_bytes": len(old_body), + "request_body_sha256": hashlib.sha256(old_body).hexdigest(), + }, + "nested_anyof_by_kind": { + "request_body_bytes": len(new_body), + "request_body_sha256": hashlib.sha256(new_body).hexdigest(), + }, + }, + } + if not isinstance(preparation, dict) or preparation.get("state") != "PREPARED_ONLY": + raise ValueError("probe_fresh_preparation_invalid") + for key, value in expected_fresh_binding.items(): + actual = preparation.get(key) + if key == "variants": + actual = { + variant: { + field: preparation.get("variants", {}).get(variant, {}).get(field) + for field in fields + } + for variant, fields in value.items() + } + if actual != value: + raise ValueError("probe_fresh_preparation_mismatch") + + if prepare_only: + old_result = {"state": "NOT_CALLED"} + new_result = {"state": "NOT_CALLED"} + else: + old_result = _run_variant( + provider, system=system, user=user, schema=old_schema, limits=limits, body=old_body + ) + new_result = _run_variant( + provider, system=system, user=user, schema=new_schema, limits=limits, body=new_body + ) + return { + "contract_version": "context-target-schema-compatibility-probe.v1", + "state": "PREPARED_ONLY" if prepare_only else "COMPLETE", + "target_repository": TARGET_REPOSITORY, + "base_sha": BASE_SHA, + "head_sha": HEAD_SHA, + "probe_source_sha": os.environ.get("GITHUB_SHA") if re.fullmatch(r"[0-9a-f]{40}", os.environ.get("GITHUB_SHA", "")) else None, + "snapshot_id": snapshot.get("snapshot_id"), + "profile_sha256": PROFILE_SHA256, + "limits_sha256": LIMITS_SHA256, + "task_id": task.get("task_id"), + "task_input_sha256": task_input_hash, + "manifest_sha256": manifest.get("manifest_sha256"), + "unit_ids": UNIT_IDS, + "lens": TARGET_LENS, + "historical_task_match": task.get("task_id") == TASK_ID, + "historical_manifest_match": manifest.get("manifest_sha256") == MANIFEST_SHA256, + "historical_task_input_match": task_input_hash == TASK_INPUT_SHA256, + "legacy_provider_source_commit": LEGACY_PROVIDER_COMMIT, + "legacy_provider_file_sha256": LEGACY_PROVIDER_FILE_SHA256, + "retry_limit": 0, + "provider_call_limit": 2, + "request_diff_scope": "context_gap_proposals.items.properties.target only", + "variants": { + "whole_object_enum": { + **old_result, + "request_body_bytes": len(old_body), + "request_body_sha256": hashlib.sha256(old_body).hexdigest(), + }, + "nested_anyof_by_kind": { + **new_result, + "request_body_bytes": len(new_body), + "request_body_sha256": hashlib.sha256(new_body).hexdigest(), + }, + }, + "historical_old_error_signature": HISTORICAL_OLD_ERROR, + } + + +def main() -> int: + prepare_only = sys.argv[1:] == ["--prepare-only"] + if sys.argv[1:] and not prepare_only: + print(json.dumps({"state": "REJECTED", "error_code": "probe_arguments_forbidden"}, separators=(",", ":"))) + return 2 + output = Path(os.environ.get("RUNNER_TEMP", "/tmp")) / ( + PREPARATION_NAME if prepare_only else OUTPUT_NAME + ) + try: + required = () if prepare_only else ("LLM_BASE_URL", "LLM_MODEL", "LLM_API_KEY") + if any(not os.environ.get(name) for name in required): + raise ValueError("probe_provider_configuration_missing") + result = run_probe(prepare_only) + exit_code = 0 + except Exception as exc: + code = str(exc) if isinstance(exc, ValueError) and re.fullmatch(r"[a-z0-9_]{1,100}", str(exc)) else "probe_failed" + result = {"contract_version": "context-target-schema-compatibility-probe.v1", "state": "NOT_RUN", "error_code": code} + exit_code = 1 + output.write_text(json.dumps(result, sort_keys=True, separators=(",", ":")) + "\n", encoding="utf-8") + os.chmod(output, 0o600) + print(json.dumps({"summary_path": str(output), "state": result.get("state")}, separators=(",", ":"))) + return exit_code + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/pr_review_harness/providers.py b/src/pr_review_harness/providers.py index 40481cc3..a3c5e9ba 100644 --- a/src/pr_review_harness/providers.py +++ b/src/pr_review_harness/providers.py @@ -932,8 +932,42 @@ def _review_parts(self, task: dict, evidence: list[dict]) -> tuple[str, dict, di except ValueError as exc: raise ProviderError("invalid_context_target_manifest") from exc target_pairs = list( - { (choice["kind"], choice["value"]) for choice in choices } + {(choice["kind"], choice["value"]) for choice in choices} ) + if target_pairs: + target_schema = { + "anyOf": [ + { + "type": "object", + "additionalProperties": False, + "required": ["kind", "value"], + "properties": { + "kind": {"type": "string", "enum": [kind]}, + "value": { + "type": "string", + "enum": sorted(value for pair_kind, value in target_pairs if pair_kind == kind), + }, + }, + } + for kind in sorted({kind for kind, _ in target_pairs}) + ] + } + elif manifest is not None: + # Empty opted-in manifests are valid, but cannot yield a target. + # Keep the nested item schema itself valid and make the array + # uninhabitable with maxItems=0 instead of contradictory string bounds. + target_schema = { + "type": "object", + "additionalProperties": False, + "required": ["kind", "value"], + "properties": { + "kind": {"type": "string", "enum": ["unit"]}, + "value": {"type": "string", "minLength": 1, "maxLength": 2000}, + }, + } + else: + # Preserve the legacy v4 schema and serialized request exactly. + target_schema = None if context_followup is not None and not input_v2: raise ProviderError("context_followup_requires_specialist_input_v2") evidence_ids = sorted( @@ -1029,6 +1063,7 @@ def _review_parts(self, task: dict, evidence: list[dict]) -> tuple[str, dict, di }, "context_gap_proposals": { "type": "array", + **({"maxItems": 0} if manifest is not None and not target_pairs else {}), "items": { "type": "object", "additionalProperties": False, @@ -1054,38 +1089,19 @@ def _review_parts(self, task: dict, evidence: list[dict]) -> tuple[str, dict, di "other", ], }, - "target": { + "target": target_schema if target_schema is not None else { "type": "object", "additionalProperties": False, "required": ["kind", "value"], - **( - { - "enum": [ - {"kind": kind, "value": value} - for kind, value in sorted(target_pairs) - ] - } - if target_pairs - else {} - ), "properties": { "kind": { "type": "string", - "enum": ( - sorted({kind for kind, _ in target_pairs}) - if target_pairs - else (["unit"] if manifest is not None else ["unit", "path", "symbol"]) - ), + "enum": ["unit", "path", "symbol"], }, "value": { "type": "string", "minLength": 1, "maxLength": 2000, - **( - {"enum": sorted({value for _, value in target_pairs})} - if target_pairs - else ({"maxLength": 0} if manifest is not None else {}) - ), }, }, }, diff --git a/tests/test_context_target_manifest.py b/tests/test_context_target_manifest.py index 1a2762a9..2cf37fb2 100644 --- a/tests/test_context_target_manifest.py +++ b/tests/test_context_target_manifest.py @@ -1,9 +1,11 @@ from __future__ import annotations import copy +import hashlib import json from pathlib import Path +import jsonschema import pytest import pr_review_harness.engine as engine_module @@ -159,15 +161,13 @@ def test_provider_v4_schema_enumerates_opt_in_targets_but_default_stays_legacy() evidence = [{"evidence_id": "ev-1"}] system, user, schema = provider._review_parts(task, evidence) target = schema["schema"]["properties"]["context_gap_proposals"]["items"]["properties"]["target"] - assert target["enum"] == [ - {"kind": "path", "value": "engines/exa.py"}, - {"kind": "path", "value": "tests/test_exa.py"}, - {"kind": "unit", "value": "unit-test"}, - ] - jsonschema = pytest.importorskip("jsonschema") + jsonschema.Draft202012Validator.check_schema(schema["schema"]) validator = jsonschema.Draft202012Validator(target) assert validator.is_valid({"kind": "path", "value": "engines/exa.py"}) + assert validator.is_valid({"kind": "path", "value": "tests/test_exa.py"}) + assert validator.is_valid({"kind": "unit", "value": "unit-test"}) assert not validator.is_valid({"kind": "unit", "value": "engines/exa.py"}) + assert not validator.is_valid({"kind": "path", "value": "unit-test"}) assert "choose only one exact {kind,value} pair" in system assert user["task"]["context_target_manifest"] == task["context_target_manifest"] @@ -190,3 +190,41 @@ def test_provider_v4_schema_enumerates_opt_in_targets_but_default_stays_legacy() assert provider.review_input_bytes(task, evidence, limits) > provider.review_input_bytes( default_task, evidence, limits ) + legacy_body = provider.serialize_review_request(default_task, evidence, limits) + assert len(legacy_body) == 8050 + assert hashlib.sha256(legacy_body).hexdigest() == "f443fa38c082303824cec8b393e842c70739a00b4e4100283daa66b64775b9a0" + + +def test_empty_target_manifest_uses_valid_uninhabitable_gap_schema(): + provider = OpenAIProvider( + { + "kind": "openai_compatible", + "base_url": "https://provider.invalid/v1", + "model": "fixture-model", + "api_key_env": "UNUSED_TEST_CREDENTIAL_REFERENCE", + "max_request_bytes": 100_000, + } + ) + snapshot = _snapshot() + snapshot["inventory"] = [] + snapshot["evidence"] = {} + task = { + "task_id": "task-empty", + "unit_ids": ["unit-test"], + "evidence_ids": ["ev-1"], + "request_input_contract": "specialist-input.v2", + "unit_evidence_bindings": [ + {"unit_id": "unit-test", "binding_status": "VERIFIED", "evidence_ids": ["ev-1"]} + ], + "context_target_manifest": build_context_target_manifest( + {"task_id": "task-empty", "unit_ids": ["unit-test"]}, snapshot, _profile() + ), + } + assert task["context_target_manifest"]["choices"] == [] + system, _, schema = provider._review_parts(task, [{"evidence_id": "ev-1"}]) + proposal_schema = schema["schema"]["properties"]["context_gap_proposals"] + assert proposal_schema["maxItems"] == 0 + jsonschema.Draft202012Validator.check_schema(schema["schema"]) + assert jsonschema.Draft202012Validator(proposal_schema).is_valid([]) + assert not jsonschema.Draft202012Validator(proposal_schema).is_valid([{}]) + assert "no available context-gap targets" in system diff --git a/tests/test_context_target_schema_probe.py b/tests/test_context_target_schema_probe.py new file mode 100644 index 00000000..6c7e9f4b --- /dev/null +++ b/tests/test_context_target_schema_probe.py @@ -0,0 +1,257 @@ +from __future__ import annotations + +import copy +import hashlib +import importlib.util +import json +from pathlib import Path + +import jsonschema +import pytest + +from pr_review_harness.context_targets import build_context_target_manifest, validate_context_target_manifest +from pr_review_harness.providers import OpenAIProvider, ProviderError + +_SCRIPT_PATH = Path(__file__).parents[1] / "scripts/run_context_target_schema_probe.py" +_SPEC = importlib.util.spec_from_file_location("context_target_schema_probe", _SCRIPT_PATH) +assert _SPEC is not None and _SPEC.loader is not None +_PROBE = importlib.util.module_from_spec(_SPEC) +_SPEC.loader.exec_module(_PROBE) +_request_schema_pair = _PROBE._request_schema_pair +_provider_config_from_environment = _PROBE._provider_config_from_environment +_run_variant = _PROBE._run_variant + + +def _profile(): + return { + "version": "probe-fixture", + "retrieval_context_patterns": ["engines/*.py"], + "trusted_policy_paths": ["AGENTS.md"], + "retrieval_revisions": {"implementation": "head"}, + "context_target_manifest": { + "version": "context-target-manifest.v1", + "max_entries": 8, + "max_bytes": 16_384, + }, + } + + +def _fixture(): + base, head = "a" * 40, "b" * 40 + snapshot = { + "snapshot_id": "snap-probe", + "base_sha": base, + "head_sha": head, + "inventory": [ + {"unit_id": "unit-engine", "path": "engines/exa.py"}, + {"unit_id": "unit-policy", "path": "AGENTS.md"}, + ], + "evidence": { + "ev-engine": { + "source_kind": "head_file", + "path": "engines/exa.py", + "source_revision": head, + "source_object_id": "c" * 40, + "source_object_format": "sha1", + "trust": "untrusted_pr_content", + } + }, + } + task = { + "task_id": "probe-task", + "task_kind": "SPECIALIST_FINDINGS", + "unit_ids": ["unit-engine"], + "evidence_ids": ["ev-engine"], + "request_input_contract": "specialist-input.v2", + "unit_evidence_bindings": [ + {"unit_id": "unit-engine", "binding_status": "VERIFIED", "evidence_ids": ["ev-engine"]} + ], + } + task["context_target_manifest"] = build_context_target_manifest(task, snapshot, _profile()) + evidence = [{"evidence_id": "ev-engine", "content": "def call(): pass\n"}] + return task, evidence + + +def test_probe_requests_differ_only_at_target_schema_and_fit_the_fixed_cap(monkeypatch): + provider = OpenAIProvider( + { + "kind": "openai_compatible", + "base_url": "https://provider.invalid/v1", + "model": "fixture-model", + "api_key_env": "UNUSED_TEST_CREDENTIAL_REFERENCE", + "max_request_bytes": 96_000, + } + ) + limits = { + "max_input_bytes_per_task": 96_000, + "max_output_bytes_per_task": 16_000, + "max_output_tokens": 1_800, + "deadline_seconds": 30, + } + task, evidence = _fixture() + def archived_control(_root, source_task, source_evidence, source_limits, model): + system, user, schema = provider._review_parts(source_task, source_evidence) + legacy = copy.deepcopy(schema) + pairs = sorted({(choice["kind"], choice["value"]) for choice in validate_context_target_manifest(source_task["context_target_manifest"])}) + target = legacy["schema"]["properties"]["context_gap_proposals"]["items"]["properties"] + target["target"] = { + "type": "object", + "additionalProperties": False, + "required": ["kind", "value"], + "enum": [{"kind": kind, "value": value} for kind, value in pairs], + "properties": { + "kind": {"type": "string", "enum": sorted({kind for kind, _ in pairs})}, + "value": { + "type": "string", + "minLength": 1, + "maxLength": 2000, + "enum": sorted({value for _, value in pairs}), + }, + }, + } + return provider._request_bytes(system, user, legacy, source_limits) + + monkeypatch.setattr(_PROBE, "_legacy_request_from_archived_provider", archived_control) + _system, _user, old_schema, new_schema, old_body, new_body = _request_schema_pair( + Path("/unused-by-fixture"), provider, task, evidence, limits + ) + old_target = old_schema["schema"]["properties"]["context_gap_proposals"]["items"]["properties"]["target"] + new_target = new_schema["schema"]["properties"]["context_gap_proposals"]["items"]["properties"]["target"] + jsonschema.Draft202012Validator.check_schema(old_schema["schema"]) + jsonschema.Draft202012Validator.check_schema(new_schema["schema"]) + assert "enum" in old_target + assert "anyOf" in new_target + assert len(old_body) <= 96_000 and len(new_body) <= 96_000 + assert hashlib.sha256(old_body).digest() != hashlib.sha256(new_body).digest() + + +def test_probe_error_receipt_drops_untrusted_text_and_keeps_only_safe_fields(): + provider = OpenAIProvider( + { + "kind": "openai_compatible", + "base_url": "https://provider.invalid/v1", + "model": "fixture-model", + "api_key_env": "UNUSED_TEST_CREDENTIAL_REFERENCE", + } + ) + + def fail(**_kwargs): + raise ProviderError( + "http_status_400", + meta={ + "error_text": "do not echo this or sk-secret", + "local_http_exchange": { + "http_status": 400, + "request_attempted": True, + "request_bytes": 100, + "request_sha256": "a" * 64, + "response_bytes": 211, + "response_sha256": "b" * 64, + "elapsed_ms": 12.5, + }, + }, + ) + + provider._call = fail + result = _run_variant( + provider, + system="unused", + user={}, + schema={}, + limits={}, + body=b"request", + ) + serialized = json.dumps(result) + assert result["state"] == "PROVIDER_ERROR" + assert result["http_status"] == 400 + assert result["response_sha256"] == "b" * 64 + assert "do not echo" not in serialized + assert "sk-secret" not in serialized + + def fail_after_attempt(**_kwargs): + error = RuntimeError("private failure text") + error.meta = { + "local_http_exchange": { + "http_status": 502, + "request_attempted": True, + "request_bytes": 100, + "request_sha256": "c" * 64, + } + } + raise error + + provider._call = fail_after_attempt + result = _run_variant( + provider, + system="unused", + user={}, + schema={}, + limits={}, + body=b"request", + ) + assert result["state"] == "LOCAL_ERROR" + assert result["http_status"] == 502 + assert result["request_attempted"] is True + assert "private failure text" not in json.dumps(result) + + +@pytest.mark.parametrize( + "base_url", + [ + "http://provider.example/v1", + "https://user:password@provider.example/v1", + "https://provider.example/v1?token=secret", + "https://provider.example/v1/chat/completions", + ], +) +def test_probe_rejects_unsafe_or_non_api_root_endpoints(monkeypatch, base_url): + monkeypatch.setenv("LLM_BASE_URL", base_url) + monkeypatch.setenv("LLM_MODEL", "fixture-model") + monkeypatch.setenv("LLM_API_KEY", "fixture-key") + with pytest.raises(ValueError, match="probe_provider_configuration_invalid"): + _provider_config_from_environment( + { + "max_input_bytes_per_task": 96_000, + "max_output_bytes_per_task": 16_000, + "max_output_tokens": 1_800, + "deadline_seconds": 30, + } + ) + + +def test_probe_config_keeps_credential_as_environment_reference(monkeypatch): + monkeypatch.setenv("LLM_BASE_URL", "https://provider.example/v1") + monkeypatch.setenv("LLM_MODEL", "fixture-model") + monkeypatch.setenv("LLM_API_KEY", "fixture-key") + config = _provider_config_from_environment( + { + "max_input_bytes_per_task": 96_000, + "max_output_bytes_per_task": 16_000, + "max_output_tokens": 1_800, + "deadline_seconds": 30, + } + ) + assert config["api_key_env"] == "LLM_API_KEY" + assert "fixture-key" not in repr(config) + + +def test_probe_workflow_is_manual_main_only_read_only_and_disabled_by_default(): + path = Path(__file__).parents[1] / ".github/workflows/context-target-schema-compatibility-probe.yml" + text = path.read_text(encoding="utf-8") + assert "workflow_dispatch:" in text + assert "default: false" in text + assert "github.repository == 'groktopus/codereview'" in text + assert "github.ref == 'refs/heads/main'" in text + assert "git init --bare" in text + assert "--filter=blob:none" in text + assert "timeout 120s" in text + assert "90165dd65be3595006171d7abef34a082c90a71d" in text + assert "--prepare-only" in text + assert text.index("--prepare-only") < text.index("Compare the fixed old and candidate schema encodings") + assert "path: target-source" not in text + assert "permissions:\n contents: read" in text + assert "LLM_BASE_URL: ${{ secrets.LLM_BASE_URL }}" in text + assert "LLM_MODEL: ${{ secrets.LLM_MODEL }}" in text + assert "LLM_API_KEY: ${{ secrets.LLM_API_KEY }}" in text + assert "JEV_API_KEY" not in text + assert "pulls: write" not in text