diff --git a/CHANGELOG.md b/CHANGELOG.md index 75a4109c9d..ae7549aed0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,9 @@ - Raised `hourly-review-repair.yml`'s discovery ceiling from 50 to 200 while rotating deterministic 50-PR deep-inspection windows by hourly run number. The scheduler hydrates only the selected window and stops immediately after its single dispatch, preserving access to newer PRs without quadrupling expensive review/check/comment work. See `docs/doctoring/hourly-review-repair-single-file-consolidation.md`'s 2026-09-03 follow-up. ## [Unreleased] +- Failed Noema reviews retain the original network failure even when response + cleanup also fails, while process cancellation still stops the review. Direct + redirect-rejection tests now release their responses explicitly. - Include merge-scheduler entrypoint, core, and regression-test changes in the existing runtime-quality workflow's trigger and suite selector. Scheduler workflow edits retain queue checks and also select the full review-repair @@ -151,6 +154,9 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- **Keep Noema's strict output schema and deterministic probe validator identical (#1641).** Each structured probe now declares its closed `probe_kind` together with the exact required `class_evidence` witness roles and source receipt fields. Nested `anyOf` variants preserve strict OpenAI-compatible required/additional-property semantics, so a realistic verdict cannot be rejected merely because the outbound schema and local admission contract disagree. The single-request invalid-location regression now reaches and asserts the intended changed-side rejection instead of passing on an earlier status mismatch. +- **Require source-bound observed defect classes in Noema formal reviews (#1641).** Canonical changed-line coordinates now reject JSON booleans, material reviews must cover distinct classes from the executable external-finding corpus, and class witnesses bind to exact changed-side source text (including lexical-shape-independent blank/non-ASCII lines) with non-vacuous causal observations. A single parser now owns both source text and coordinates; bounded truncation drops the incomplete line instead of synthesizing a changed-line marker, so genuine source equal to the old marker remains reviewable. The prompt explicitly attacks workflow-event authority plus mutable-alias, TOCTOU, identity, oracle, contract, authority, dependency-context, coercion, and state-machine failure shapes without fabricating benchmark claims. +- **Fail closed on fabricated Noema execution and external-source provenance (#1641).** Model-authored claims that runtime behavior, command output, toolchain help, or authoritative external documentation confirmed a conclusion now require an out-of-band typed receipt and an exact receipt citation. The isolated reviewer may still reason from changed source and recommend toolchain-specific verification; it cannot present that recommendation as executed evidence. This regression is grounded in `ConceptWeave#35@a31ae0c2`, where review `5120903874` claimed Cargo runtime/documentation confirmation although required Noema run `33938445009` executed no Cargo or documentation lookup step. - **Pin `opencode-review-dispatch.yml` off the starved floating `ubuntu-latest` image.** The 2026-09-01 floating-image fix (see that entry below) pinned `strix.yml`, `opencode-review.yml`, and `noema-review.yml` -- the three required-check @@ -1458,3 +1464,5 @@ Semantic Versioning where the repository publishes a release. - Added an organization-owned reusable exact-artifact SBOM attestation boundary that validates inert six-file wheel/sdist evidence, binds CycloneDX 1.7 predicates to exact SHA-256 subjects, signs through least-privilege GitHub artifact attestations, and exports online and offline verification bundles. - Hardened exact-artifact SBOM verification with strict finite RFC 8259 JSON, integer CycloneDX document versions, deterministic UUIDv5 subject identities, exact filename properties and single SHA-256 root bindings, environment-only shell input transfer, pinned Ubuntu 24.04 quality runners, and checksum-sealed beginner-readable offline evidence. The decision record now cites Bray (2017) so NaN and Infinity cannot be treated as sealed SBOM numbers. - Recorded the org control-plane architecture, including exact-artifact SBOM attestation, so agents reconstruct the signing trust boundary from the repo instead of private memory. + +- Noema review evidence now uses exact class-and-field claim roles and source excerpts instead of a fixed English causal-word heuristic, preserving non-ASCII and symbol-only review evidence without treating keywords as proof. diff --git a/docs/doctoring/noema-observed-defect-corpus-current-main.md b/docs/doctoring/noema-observed-defect-corpus-current-main.md new file mode 100644 index 0000000000..efef8db6a5 --- /dev/null +++ b/docs/doctoring/noema-observed-defect-corpus-current-main.md @@ -0,0 +1,27 @@ +# Noema observed-defect review corpus + +The trusted Noema review gate treats externally demonstrated review misses as executable regression evidence, not as benchmark claims. Material source/test reviews must exercise at least two distinct observed defect classes and every admitted class witness remains bound to an exact changed-side source coordinate. + +The current closed taxonomy is: `mutable_alias`, `time_of_check_time_of_use`, `execution_identity`, `coercion_boundary`, `test_oracle`, `cross_contract`, `authority_boundary`, `dependency_context`, and `state_machine_race`. Each class has class-specific witness keys. Witness values are `{path,line,side,source_excerpt,claim_role,observation}` records bound to the probe location. `source_excerpt` must equal the exact changed-side line, and `observation` must quote the exact source line (or ``) plus a causal/behavioral relation beyond taxonomy labels; ASCII token shape is not admission authority; repeated or differently worded generic labels do not satisfy the deterministic validator. + +The outbound strict structured-output schema and the local validator share that same closed contract. Every probe is one nested `anyOf` variant that correlates a single `probe_kind` with exactly its required `class_evidence` keys; every witness field is required and unknown fields are rejected. Only the containing `adversarial_validation` value is nullable for a non-formal comment. This follows the strict structured-output rule that object properties are required (nullable when truly optional) and prevents the gateway from accepting a probe shape that deterministic admission must reject. + +The model is explicitly asked to attack mutable/immutability escapes, changing getters/TOCTOU, request or tenant identity confusion, weak/vacuous oracles, cross-contract contradictions, authority overreach, missing causal dependency context, and reliability/security state-machine races. A falsified hypothesis is valid evidence and must not be promoted into a finding merely to satisfy taxonomy diversity. For CI/automation changes, the review prompt also requires checking whether the mutation credential can create the downstream events/checks the state machine depends on. + +JSON booleans are rejected as line coordinates even though Python considers `True == 1`: changed-line evidence requires `type(line) is int` and a positive value. Production review calls always provide the complete changed-path manifest, which activates the observed taxonomy; direct validator unit tests may omit that manifest to exercise lower-level generic schema boundaries independently. + +This repair is a narrow current-main successor to the heavily diverged PR #1589 evidence lineage. It does not copy CodeRabbitAI or Devin wording and makes no superiority claim. + +Exact-head follow-up removes synthetic bounded-diff omission lines from the diff grammar entirely: truncation drops the incomplete final line and carries the separate `truncated` control flag. A genuine source line equal to the historical marker remains admissible, as do short identifiers, symbol-only lines, blank changed lines, and non-ASCII source through exact string equality rather than lexical guessing. Coordinates and source text now come from one parser so future diff fixes cannot desynchronize their trust boundaries. + +The exact-head structural follow-up removes the fixed English relation-word list. Formal evidence now carries a schema-derived `claim_role` for each defect-class witness, while the deterministic gate verifies exact source identity, canonical coordinates, role identity, and distinct observations. Semantic causal adequacy remains a reviewer/evaluation responsibility; the validator does not pretend English keyword presence proves causality. + +Workflow-local bootstrap or generated commits are not accepted as final review/check proof merely because their source transaction verified locally. The merge candidate must be a workflow-starting successor writer head produced through ordinary owner-side mutation, with the required review and quality checks observed on that exact unchanged head before merge. + +Failed HTTP responses belong to the requesting transport. After bounded telemetry +extraction, close the response there; a secondary cleanup exception must not replace +the original typed transport failure. Process cancellation still propagates. Tests +that invoke a redirect handler directly own the resulting HTTPError and must close +it themselves rather than relying on garbage collection. Run the Noema regression +tests with `-W error`; tests in other HTTP consumers do not become passing evidence +merely because this transport was repaired. diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 1cc9e20313..113373a73e 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -2626,6 +2626,27 @@ Higgins, S. S., Crepalde, N., & Fernandes, L. (2021). Segmented multiplexity: A **Residual.** This closes the specific floating-image contribution from these three central workflows; it does not by itself guarantee the organization-wide Actions queue is fully drained, since other repositories' own workflows and any remaining unpinned central workflows may still request the floating image. Worth a follow-up sweep across the rest of `.github/workflows/` and sibling-repo workflows if queuing persists after this lands. + +### 2026-09-02 — Noema observed-defect false-negative corpus (#1641) + +- **Verified gap:** protected current main admitted Noema adversarial evidence by count/prose identity and compared model line coordinates with Python integers without excluding booleans. Thus `true` could alias line `1`, and two differently worded probes could satisfy material-change diversity without proving distinct observed defect shapes. +- **Repair:** exact changed-side coordinates now require canonical positive integers; production review verdicts use a closed observed-defect taxonomy with class-specific source-bound witnesses whose exact `source_excerpt` must match the cited changed line and whose observation must quote that exact source (or ``) plus causal behavior without ASCII/token-shape heuristics. Material changes require distinct classes, and the prompt explicitly checks workflow-starting mutation credentials before relying on downstream required checks. +- **Regression evidence:** `tests/test_noema_observed_defect_corpus_current_main.py` is committed before the causal production change and covers boolean aliasing, malformed/unknown class labels, duplicate-class diversity, witness/source binding, a valid multi-class verdict, and rendered prompt coverage. +- **Authority boundary:** no reviewer, provider, routing, merge, or repository-write authority is widened. The taxonomy is evaluation/admission evidence only. + +- **Noema exact-source follow-up (PR #1641):** bounded truncation no longer synthesizes a +/- omission line; it drops the incomplete line and carries the separate `truncated` flag. Genuine source equal to the historical marker remains admissible. One parser now emits both changed coordinates and exact source text, while short, symbol-only, blank, and non-ASCII changed lines use exact equality and arbitrary source-adjacent words do not satisfy causal evidence. + +- **Noema structural-causality follow-up (PR #1641):** removed fixed English relation-word admission. Each class witness now carries an exact schema-derived `claim_role` plus exact changed-line source text; deterministic validation stays language-neutral and semantic causality is tested through reviewer/evaluation regressions rather than guessed from keywords. + +- **Noema strict-schema parity follow-up (PR #1641):** the outbound response schema now correlates every observed `probe_kind` with the exact required class-witness object that production validates. A realistic verdict is applied to both contracts in one regression, and invalid changed-line telemetry reaches the intended coordinate rejection before asserting the one-request boundary. + +### 2026-09-05 — Noema executed-evidence provenance boundary (#1641) + +- **Observed RED:** `ConceptualWisdomLab/ConceptWeave#35@a31ae0c2df920f2794f7ddb456795b04797ab472` received CHANGES_REQUESTED review `5120903874`, which stated that Cargo CLI documentation and runtime behavior confirmed `cargo generate-lockfile --locked` was unsupported. Required Noema run `33938445009`, job `101256294197`, used trusted workflow source `8272e4f95c253ab067592460cc9288581bf3a422`; its model phase invoked only the isolated Noema gateway client. No Cargo command, help lookup, or official-document retrieval step executed. Cargo 1.98.0's actual help is contrary evidence, but this central repair does not hard-code a Cargo verdict or remove the consumer lockfile guard. +- **Causal boundary:** exact changed-line and observed-defect-class validation proves that a model response is structurally reviewable; it does not prove that prose describing runtime or external documentation was observed. The trusted gate now inspects only model-authored evidence fields and rejects claims of executed/toolchain behavior or authoritative external sources unless an out-of-band typed receipt ID is supplied and cited in the same statement. The current workflow supplies no such receipts. Source-only reasoning and explicit verification directions remain admissible. +- **Fail-closed preservation:** a missing, wrong-type, or uncited receipt produces no usable verdict. Self-approval, blanket warning suppression, toolchain assumptions, and hard-coded consumer approval are not introduced. Future command/document preprocessors must bind receipt type and ID outside model-controlled context before enabling those claim classes. +- **Scope:** this is central reviewer-evidence provenance only. ConceptWeave source and PR state remain read-only to this owner; the prior CHANGES_REQUESTED review is not dismissed or converted to approval by this change. + ## 2026-09-02 GitHub Actions review sidecar pool pinned to `orchestrator/free`; `auto` removed as an accepted value **Problem.** `scripts/ci/contextual_orchestrator_review_sidecar.sh` — the script every central required review workflow (Strix, OpenCode Review, Noema Review, the PR-review autofix sidecar) provisions to talk to `contextual-orchestrator` — read an operator-settable `CONTEXTUAL_ORCHESTRATOR_POOL` environment variable, defaulted it to `free`, and validated it against exactly two accepted values: `free` or `auto` (`case "$orchestrator_pool" in free|auto) ...`). `auto` is a real, load-bearing value one layer down: `scripts/ci/contextual_orchestrator_review_launcher.py --pool auto` admits *priced* discovered routes as a fallback stage once the free pool is exhausted (`build_zdr_prioritized_catalog(..., pool="auto")`), by design, for callers that want that behavior. Nothing in this repository's own review-provisioning code path currently sets `CONTEXTUAL_ORCHESTRATOR_POOL=auto` — the only workflow that sets the variable at all, `strix.yml`, sets it to `free`; every other central review workflow simply relies on the script's own `:-free` default — so this was not a live incident, it was an unaudited, structurally-reachable escape hatch: a future edit to any of the four workflows above, or a manually-triggered `workflow_dispatch` with a custom env override, could set `CONTEXTUAL_ORCHESTRATOR_POOL=auto` and the sidecar would accept it silently, with no cost ceiling, no budget/authorization gate, and no reviewer visibility that priced models were now in scope for a required check. diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 5ab7e830f3..586187d541 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -20,6 +20,7 @@ import urllib.parse import urllib.request from collections.abc import Sequence +from contextlib import suppress from typing import Any from scripts.ci.opencode_review_normalize_output import changed_file_is_material @@ -61,6 +62,34 @@ MAX_ALLOWED_LOCATIONS_JSON_BYTES = 32 * 1024 MAX_HTTP_ERROR_BODY_BYTES = 16 * 1024 DIFF_HUNK_RE = re.compile(r"^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@") +OBSERVED_REVIEW_PROBE_KINDS = frozenset( + { + "mutable_alias", + "time_of_check_time_of_use", + "execution_identity", + "coercion_boundary", + "test_oracle", + "cross_contract", + "authority_boundary", + "dependency_context", + "state_machine_race", + } +) +OBSERVED_REVIEW_PROBE_EVIDENCE_FIELDS: dict[str, tuple[str, ...]] = { + "mutable_alias": ("alias_origin", "mutation_attempt", "post_validation_observation"), + "time_of_check_time_of_use": ("check_observation", "intervening_change", "use_observation"), + "execution_identity": ("incoming_identity", "retained_identity", "mismatch_guard"), + "coercion_boundary": ("raw_value", "conversion_path", "canonicality_guard"), + "test_oracle": ("assertion_under_test", "negative_control", "distinguishing_observation"), + "cross_contract": ("first_contract", "second_contract", "contradiction_or_alignment"), + "authority_boundary": ("component_authority", "external_authority", "enforcement_boundary"), + "dependency_context": ("dependency", "omitted_or_included_context", "causal_effect"), + "state_machine_race": ("initial_state", "event_order", "invariant_observation"), +} +OBSERVED_REVIEW_PROBE_CLAIM_ROLES: dict[str, dict[str, str]] = { + kind: {field: f"{kind}:{field}" for field in fields} + for kind, fields in OBSERVED_REVIEW_PROBE_EVIDENCE_FIELDS.items() +} SAFE_MODEL_IDENTIFIER_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._:/@+-]{0,199}$") ORCHESTRATOR_LOOPBACK_HOSTS = frozenset({"127.0.0.1", "::1"}) @@ -105,27 +134,71 @@ }, "required": ["path", "line", "side", "analysis"], } -_NOEMA_PROBE_SCHEMA: dict[str, Any] = { - "type": "object", - "additionalProperties": False, - "properties": { +_NOEMA_PROBE_BASE_PROPERTIES: dict[str, Any] = { + "path": {"type": "string"}, + "line": {"type": "integer"}, + "side": {"type": "string", "enum": ["LEFT", "RIGHT"]}, + "hypothesis": {"type": "string"}, + "attack_or_counterexample": {"type": "string"}, + "evidence": {"type": "string"}, + "outcome": {"type": "string", "enum": ["falsified", "confirmed"]}, +} + + +def _noema_class_evidence_witness_schema(claim_role: str) -> dict[str, Any]: + """Return one strict changed-source witness schema for a taxonomy role.""" + properties = { "path": {"type": "string"}, "line": {"type": "integer"}, "side": {"type": "string", "enum": ["LEFT", "RIGHT"]}, - "hypothesis": {"type": "string"}, - "attack_or_counterexample": {"type": "string"}, - "evidence": {"type": "string"}, - "outcome": {"type": "string", "enum": ["falsified", "confirmed"]}, - }, - "required": [ - "path", - "line", - "side", - "hypothesis", - "attack_or_counterexample", - "evidence", - "outcome", - ], + "source_excerpt": {"type": "string"}, + "claim_role": {"type": "string", "enum": [claim_role]}, + "observation": {"type": "string"}, + } + return { + "type": "object", + "additionalProperties": False, + "properties": properties, + "required": list(properties), + } + + +def _noema_observed_probe_schema(probe_kind: str) -> dict[str, Any]: + """Return a strict probe variant correlated with its exact evidence roles.""" + evidence_properties = { + field: _noema_class_evidence_witness_schema( + OBSERVED_REVIEW_PROBE_CLAIM_ROLES[probe_kind][field] + ) + for field in OBSERVED_REVIEW_PROBE_EVIDENCE_FIELDS[probe_kind] + } + properties = { + **_NOEMA_PROBE_BASE_PROPERTIES, + "probe_kind": {"type": "string", "enum": [probe_kind]}, + "class_evidence": { + "type": "object", + "additionalProperties": False, + "properties": evidence_properties, + "required": list(evidence_properties), + }, + } + return { + "type": "object", + "additionalProperties": False, + "properties": properties, + "required": list(properties), + } + + +# Keep the kind and its exact class-evidence key set in the same ``anyOf`` +# branch. Independent enums would let the gateway admit a mismatched pair that +# the deterministic validator must reject. All formal probe fields are required +# rather than nullable; only their containing adversarial-validation object may +# be null for a non-formal comment verdict. +_NOEMA_PROBE_SCHEMA: dict[str, Any] = { + "anyOf": [ + _noema_observed_probe_schema(probe_kind) + for probe_kind in sorted(OBSERVED_REVIEW_PROBE_KINDS) + ] } _NOEMA_FINDING_SCHEMA: dict[str, Any] = { "type": "object", @@ -139,51 +212,101 @@ }, "required": ["severity", "file", "line", "side", "message"], } +def _noema_adversarial_validation_schema( + required_probes: int, + *, + status: str | None, +) -> dict[str, Any]: + """Build one strict validation receipt, optionally pinning its status.""" + status_schema: dict[str, Any] = {"type": "string", "enum": ["passed", "failed"]} + if status is not None: + status_schema["enum"] = [status] + return { + "type": "object", + "additionalProperties": False, + "properties": { + "status": status_schema, + "residual_risk": {"type": "string"}, + "probes": { + "type": "array", + "minItems": required_probes, + "items": _NOEMA_PROBE_SCHEMA, + }, + }, + "required": ["status", "residual_risk", "probes"], + } + + +def _noema_verdict_variant_schema( + required_probes: int, + *, + decision: str, + status: str | None, +) -> dict[str, Any]: + """Build one decision-correlated verdict branch below the root object.""" + validation_schema = _noema_adversarial_validation_schema( + required_probes, + status=status, + ) + if decision == "comment": + validation_schema = {"anyOf": [validation_schema, {"type": "null"}]} + properties = { + "decision": {"type": "string", "enum": [decision]}, + "summary": {"type": "string"}, + "reviewed_lines": { + "type": ["array", "null"], + "items": _NOEMA_REVIEWED_LINE_SCHEMA, + }, + "adversarial_validation": validation_schema, + "findings": {"type": "array", "items": _NOEMA_FINDING_SCHEMA}, + } + return { + "type": "object", + "additionalProperties": False, + "properties": properties, + "required": list(properties), + } + + def _noema_verdict_json_schema(required_probes: int) -> dict[str, Any]: - """Build the verdict JSON Schema with this request's exact probe floor. + """Build the wrapped verdict JSON Schema with this request's probe floor. ``required_probes`` must come from ``_required_probe_count(diff, changed_paths)`` -- the same call ``validate_substantive_verdict`` uses -- so the gateway-enforced structural floor and the Python-side backstop - can never silently diverge. The static per-field schemas above are safe - to share by reference here since nothing in this module mutates them. + can never silently diverge. The probe schema also correlates each closed + taxonomy kind with the exact witness roles enforced by the local validator. """ + # OpenAI Structured Outputs requires the root to be an object and does not + # support ``if``/``then``/``else``. Put the decision variants in the + # required nested ``verdict`` property so the gateway can reject an + # approve/failed or request_changes/passed contradiction before returning + # it to the deterministic local validator. return { "type": "object", "additionalProperties": False, "properties": { - "decision": { - "type": "string", - "enum": ["approve", "request_changes", "comment"], - }, - "summary": {"type": "string"}, - "reviewed_lines": { - "type": ["array", "null"], - "items": _NOEMA_REVIEWED_LINE_SCHEMA, - }, - "adversarial_validation": { - "type": ["object", "null"], - "additionalProperties": False, - "properties": { - "status": {"type": "string", "enum": ["passed", "failed"]}, - "residual_risk": {"type": "string"}, - "probes": { - "type": "array", - "minItems": required_probes, - "items": _NOEMA_PROBE_SCHEMA, - }, - }, - "required": ["status", "residual_risk", "probes"], - }, - "findings": {"type": "array", "items": _NOEMA_FINDING_SCHEMA}, + "verdict": { + "anyOf": [ + _noema_verdict_variant_schema( + required_probes, + decision="approve", + status="passed", + ), + _noema_verdict_variant_schema( + required_probes, + decision="request_changes", + status="failed", + ), + _noema_verdict_variant_schema( + required_probes, + decision="comment", + status=None, + ), + ] + } }, - "required": [ - "decision", - "summary", - "reviewed_lines", - "adversarial_validation", - "findings", - ], + "required": ["verdict"], } @@ -199,6 +322,15 @@ def _noema_verdict_response_format(required_probes: int) -> dict[str, Any]: } +def _unwrap_noema_verdict(envelope: dict[str, Any]) -> dict[str, Any]: + """Return the sole structured verdict while preserving the local contract.""" + if set(envelope) != {"verdict"} or not isinstance(envelope.get("verdict"), dict): + raise NoemaModelOutputError( + "Noema LLM response must contain exactly one structured verdict object" + ) + return envelope["verdict"] + + class NoemaModelOutputError(RuntimeError): """Raised when untrusted model output violates the trusted verdict contract.""" @@ -207,6 +339,89 @@ class NoemaTransportError(RuntimeError): """Raised when the bounded review transport cannot produce usable evidence.""" +TRUSTED_PROVENANCE_CITATION_RE = re.compile( + r"\[receipt:(?P[A-Za-z0-9][A-Za-z0-9._-]{0,79})\]" +) +EXECUTED_EVIDENCE_CLAIM_RE = re.compile( + r"(?i)\b(?:runtime(?:\s+behavior)?|command(?:\s+(?:output|execution))?|" + r"(?:cli|toolchain)\s+(?:help|output|execution))\s+" + r"(?:confirms?|confirmed|shows?|shown|demonstrates?|demonstrated|proves?|proved|" + r"returns?|returned|passes?|passed|fails?|failed|accepts?|accepted|rejects?|rejected)\b" +) +EXTERNAL_SOURCE_CLAIM_RE = re.compile( + r"(?i)\b(?:official|upstream|vendor|external)\s+" + r"(?:(?:[A-Za-z0-9._+-]+)\s+){0,3}" + r"(?:documentation|docs?|reference|manual|help)\b[^.\n]{0,240}\b" + r"(?:confirms?|confirmed|shows?|shown|states?|stated|documents?|documented|" + r"agrees?|agreed|supports?|supported|rejects?|rejected)\b" +) + + +def _model_evidence_statements(verdict: dict[str, Any]) -> list[str]: + """Return only model-authored prose that can assert review evidence.""" + statements: list[str] = [] + + def append(value: Any) -> None: + if isinstance(value, str) and value.strip(): + statements.append(value) + + append(verdict.get("summary")) + for reviewed in verdict.get("reviewed_lines") or []: + if isinstance(reviewed, dict): + append(reviewed.get("analysis")) + validation = verdict.get("adversarial_validation") + if isinstance(validation, dict): + append(validation.get("residual_risk")) + for probe in validation.get("probes") or []: + if not isinstance(probe, dict): + continue + for field in ("hypothesis", "attack_or_counterexample", "evidence"): + append(probe.get(field)) + class_evidence = probe.get("class_evidence") + if isinstance(class_evidence, dict): + for witness in class_evidence.values(): + if isinstance(witness, dict): + append(witness.get("observation")) + for finding in verdict.get("findings") or []: + if isinstance(finding, dict): + append(finding.get("message")) + return statements + + +def validate_evidence_provenance( + verdict: dict[str, Any], + *, + trusted_execution_receipt_ids: Sequence[str] = (), + trusted_source_receipt_ids: Sequence[str] = (), +) -> None: + """Reject claims of executed or external evidence without a typed receipt. + + Noema is isolated from command execution and network access. A model may + reason from the supplied source and recommend a verification command, but + it must not turn that recommendation into purported observed evidence. + Future trusted preprocessors can authorize a claim only by supplying a + typed receipt ID out-of-band and requiring the model to cite that ID in the + same evidence statement. + """ + for statement in _model_evidence_statements(verdict): + cited_ids = { + match.group("receipt_id") + for match in TRUSTED_PROVENANCE_CITATION_RE.finditer(statement) + } + if EXECUTED_EVIDENCE_CLAIM_RE.search(statement) and not ( + cited_ids & set(trusted_execution_receipt_ids) + ): + raise NoemaModelOutputError( + "Noema evidence provenance requires a trusted execution receipt" + ) + if EXTERNAL_SOURCE_CLAIM_RE.search(statement) and not ( + cited_ids & set(trusted_source_receipt_ids) + ): + raise NoemaModelOutputError( + "Noema evidence provenance requires a trusted external-source receipt" + ) + + def _stable_failure_diagnostic(exc: BaseException) -> str: """Return actionable trusted diagnostics without reflecting model values.""" @@ -231,6 +446,7 @@ def _stable_failure_diagnostic(exc: BaseException) -> str: "Noema adversarial probe ", "Noema approve ", "Noema request_changes ", + "Noema evidence provenance ", ) if message.startswith(trusted_prefixes): return message @@ -472,29 +688,25 @@ def current_actor() -> str: def fetch_diff(repo: str, number: int) -> tuple[str, bool]: - """Fetch the PR diff and truncate it to the bounded LLM prompt size.""" + """Fetch the PR diff and truncate before an incomplete final line.""" diff = run(["gh", "api", f"repos/{repo}/pulls/{number}", "-H", "Accept: application/vnd.github.v3.diff"]) truncated = len(diff) > MAX_DIFF_CHARS if truncated: - marker = "[overlong changed line content omitted]" - bounded = diff[: MAX_DIFF_CHARS - len(marker) - 2] - complete, separator, partial = bounded.rpartition("\n") + bounded = diff[:MAX_DIFF_CHARS] + complete, separator, _partial = bounded.rpartition("\n") if not separator: - return diff[:MAX_DIFF_CHARS], truncated - last_hunk = max(complete.rfind("\n@@"), 0 if complete.startswith("@@") else -1) - last_file = max(complete.rfind("\ndiff --git "), 0 if complete.startswith("diff --git ") else -1) - inside_hunk = last_hunk > last_file - if partial.startswith(("+", "-")) and ( - inside_hunk or not partial.startswith(("+++", "---")) - ): - complete += f"\n{partial[0]}{marker}" + return bounded, truncated + # Do not synthesize a +/- line: such a marker is indistinguishable + # from genuine source with the same text and can become false exact + # changed-line evidence. The explicit ``truncated`` flag tells the + # model and deterministic gate that the bounded diff is incomplete. diff = complete return diff, truncated -def changed_diff_locations(diff: str) -> set[tuple[str, int, str]]: - """Return exact LEFT/RIGHT changed-line locations from a unified diff.""" - locations: set[tuple[str, int, str]] = set() +def changed_diff_line_texts(diff: str) -> dict[tuple[str, int, str], str]: + """Return exact changed-side source text from one unified-diff parser.""" + texts: dict[tuple[str, int, str], str] = {} old_path = new_path = "" old_line = new_line = 0 in_hunk = False @@ -520,18 +732,23 @@ def changed_diff_locations(diff: str) -> set[tuple[str, int, str]]: continue if raw_line.startswith("+"): if not new_path: - return set() - locations.add((new_path, new_line, "RIGHT")) + return {} + texts[(new_path, new_line, "RIGHT")] = raw_line[1:] new_line += 1 elif raw_line.startswith("-"): if not old_path: - return set() - locations.add((old_path, old_line, "LEFT")) + return {} + texts[(old_path, old_line, "LEFT")] = raw_line[1:] old_line += 1 else: old_line += 1 new_line += 1 - return locations + return texts + + +def changed_diff_locations(diff: str) -> set[tuple[str, int, str]]: + """Return coordinates from the exact-source parser to prevent drift.""" + return set(changed_diff_line_texts(diff)) def parse_diff_path(raw: str, prefix: str) -> str: @@ -548,6 +765,102 @@ def parse_diff_path(raw: str, prefix: str) -> str: return value.removeprefix(prefix) +def _canonical_changed_location(record: dict[str, Any], label: str) -> tuple[str, int, str]: + """Return a canonical changed-side location without bool/int coercion.""" + path_value = record.get("path") + line_value = record.get("line") + side_value = record.get("side") + if not isinstance(path_value, str) or not path_value.strip(): + raise NoemaModelOutputError(f"{label} requires a canonical changed-side path") + if type(line_value) is not int or line_value <= 0: + raise NoemaModelOutputError(f"{label} requires a canonical positive integer line") + if side_value not in {"LEFT", "RIGHT"}: + raise NoemaModelOutputError(f"{label} requires canonical LEFT/RIGHT side") + return (path_value, line_value, side_value) + + +def _validate_observed_probe_class_evidence( + probe: dict[str, Any], + probe_kind: str, + index: int, + location: tuple[str, int, str], + diff: str, +) -> None: + """Require defect-class witnesses to bind to the probe's exact changed line.""" + class_evidence = probe.get("class_evidence") + required_fields = OBSERVED_REVIEW_PROBE_EVIDENCE_FIELDS[probe_kind] + if not isinstance(class_evidence, dict) or set(class_evidence) != set(required_fields): + expected = ", ".join(required_fields) + raise NoemaModelOutputError( + f"Noema adversarial probe {index} class_evidence for {probe_kind} " + f"must contain exactly: {expected}" + ) + normalized_observations: list[str] = [] + source_texts = changed_diff_line_texts(diff) + for field in required_fields: + source_ref = class_evidence.get(field) + if not isinstance(source_ref, dict) or set(source_ref) != { + "path", + "line", + "side", + "source_excerpt", + "claim_role", + "observation", + }: + raise NoemaModelOutputError( + f"Noema adversarial probe {index} class_evidence.{field} requires " + "path, line, side, exact source_excerpt, class-specific claim_role, and non-empty observation" + ) + source_location = _canonical_changed_location( + source_ref, f"Noema adversarial probe {index} class_evidence.{field}" + ) + if source_location != location: + raise NoemaModelOutputError( + f"Noema adversarial probe {index} class_evidence.{field} must bind to " + "the probe location" + ) + expected_excerpt = source_texts.get(source_location) + source_excerpt = source_ref.get("source_excerpt") + if ( + not isinstance(source_excerpt, str) + or expected_excerpt is None + or source_excerpt != expected_excerpt + ): + raise NoemaModelOutputError( + f"Noema adversarial probe {index} class_evidence.{field} requires the " + "exact changed-line source_excerpt" + ) + observation = source_ref.get("observation") + if not isinstance(observation, str) or not observation.strip(): + raise NoemaModelOutputError( + f"Noema adversarial probe {index} class_evidence.{field} requires a " + "non-empty observation" + ) + if len(observation) > MAX_THREAD_BODY_CHARS: + raise NoemaModelOutputError( + f"Noema adversarial probe {index} class_evidence.{field} observation " + f"exceeds {MAX_THREAD_BODY_CHARS} characters" + ) + source_marker = source_excerpt if source_excerpt.strip() else "" + if source_marker not in observation: + raise NoemaModelOutputError( + f"Noema adversarial probe {index} class_evidence.{field} observation " + "must quote the exact source_excerpt (or for a blank line)" + ) + expected_claim_role = OBSERVED_REVIEW_PROBE_CLAIM_ROLES[probe_kind][field] + claim_role = source_ref.get("claim_role") + if claim_role != expected_claim_role: + raise NoemaModelOutputError( + f"Noema adversarial probe {index} class_evidence.{field} claim_role " + f"must be {expected_claim_role!r}" + ) + normalized_observations.append(observation.strip().casefold()) + if len(set(normalized_observations)) != len(normalized_observations): + raise NoemaModelOutputError( + f"Noema adversarial probe {index} requires distinct class-specific observations" + ) + + def _required_probe_count(diff: str, changed_paths: Sequence[str] = ()) -> int: """Return the minimum adversarial-probe count a formal verdict must carry. @@ -610,7 +923,7 @@ def validate_substantive_verdict( entry = _entry_ordinal(position, reviewed_total) if not isinstance(reviewed, dict): raise NoemaModelOutputError(f"Noema reviewed line {entry} must be an object") - location = (reviewed.get("path"), reviewed.get("line"), reviewed.get("side")) + location = _canonical_changed_location(reviewed, f"Noema reviewed line {entry}") if location not in locations: path, line, side = location raise NoemaModelOutputError( @@ -641,12 +954,14 @@ def validate_substantive_verdict( confirmed: set[tuple[str, int, str]] = set() identities: set[tuple[Any, ...]] = set() + probe_kinds: set[str] = set() + enforce_observed_taxonomy = bool(changed_paths) probes_total = len(probes) for position, probe in enumerate(probes, start=1): entry = _entry_ordinal(position, probes_total) if not isinstance(probe, dict): raise NoemaModelOutputError(f"Noema adversarial probe {entry} must be an object") - location = (probe.get("path"), probe.get("line"), probe.get("side")) + location = _canonical_changed_location(probe, f"Noema adversarial probe {entry}") if location not in locations: path, line, side = location raise NoemaModelOutputError( @@ -663,6 +978,14 @@ def validate_substantive_verdict( raise NoemaModelOutputError( f"Noema adversarial probe {entry} outcome must be falsified or confirmed" ) + if enforce_observed_taxonomy: + probe_kind = probe.get("probe_kind") + if not isinstance(probe_kind, str) or probe_kind not in OBSERVED_REVIEW_PROBE_KINDS: + raise NoemaModelOutputError( + f"Noema adversarial probe {entry} requires probe_kind from the observed defect taxonomy" + ) + _validate_observed_probe_class_evidence(probe, probe_kind, position, location, diff) + probe_kinds.add(probe_kind) identity = ( *location, probe["hypothesis"].strip().casefold(), @@ -674,6 +997,11 @@ def validate_substantive_verdict( if outcome == "confirmed": confirmed.add((str(probe["path"]), int(probe["line"]), str(probe["side"]))) + if enforce_observed_taxonomy and len(probe_kinds) < required_probes: + raise NoemaModelOutputError( + f"Noema {decision} requires at least {required_probes} distinct probe_kind values" + ) + if decision == "approve" and confirmed: raise NoemaModelOutputError("Noema approve cannot contain a confirmed adversarial probe") if decision == "request_changes": @@ -1546,11 +1874,20 @@ def call_llm( "You are Noema, an independent pull request reviewer for ContextualWisdomLab.", "Review the PR diff plus the additional changed-file and review-thread context for correctness, security, maintainability, and behavioral regressions.", "Return only JSON with the declared response_format schema.", - "Every formal verdict must cite exact changed-side lines. APPROVE requires falsifying concrete regression hypotheses; source or test changes require at least two distinct probes and other changes require at least one. REQUEST_CHANGES requires a confirmed probe at a finding location.", + "Every formal verdict must cite exact changed-side lines. APPROVE requires falsifying concrete regression hypotheses; material source or test changes require at least two distinct probe_kind values and other changes require at least one. REQUEST_CHANGES requires a confirmed probe at a finding location.", "Use only path, line, and side tuples listed in the bounded allowed-locations JSON below. If it is truncated, omit a formal verdict for any location not listed instead of guessing.", f"Allowed changed-side locations: {allowed_locations_json}", f"Location shape example: {json.dumps(location_example, separators=(',', ':'))}", + "Observed defect taxonomy and required source-bound class_evidence keys: " + + json.dumps( + {kind: list(fields) for kind, fields in OBSERVED_REVIEW_PROBE_EVIDENCE_FIELDS.items()}, + sort_keys=True, + separators=(",", ":"), + ), + "Every class_evidence witness must include path, line, side, source_excerpt, claim_role, and observation. source_excerpt must be the exact cited changed-side line, including an empty string for a blank line; an overlong-line omission marker is never source evidence. claim_role is the exact class-and-field role emitted by the schema. The observation must quote that exact source_excerpt (or ) and explain the claimed behavior. The deterministic gate validates source identity and the structural role; it deliberately does not guess causality from an English relation-word list.", + "Actively attack mutable alias/immutability escapes, time-of-check/time-of-use or changing-getter behavior, execution/tenant/request identity confusion, coercion boundaries, weak or vacuous test oracles, cross-file/cross-document contract contradictions, internal-vs-external authority overreach, missing causal dependency context, and security/reliability state-machine races. For automation or CI that mutates a branch or source and then relies on later events, verify that the mutation uses a workflow-starting credential/actor and that downstream required checks can actually be created on the successor head. Distinguish confirmed defects from falsified hypotheses; do not manufacture findings to satisfy the taxonomy.", "Use request_changes only for blocking, concrete issues. A generic no-issues statement is not review evidence.", + "You cannot execute commands or access external documentation in this review. Do not claim that runtime behavior, command output, help text, or external documentation confirmed a conclusion unless the additional context contains a trusted receipt and your evidence cites its exact [receipt:]. No trusted receipts are supplied by this workflow today. State source reasoning and verification directions as such.", f"Repository: {repo}", f"PR: #{number}", f"Title: {pr.get('title') or ''}", @@ -1594,7 +1931,7 @@ def call_llm( raw = decode_llm_response_body(raw_bytes) served_model = _extract_served_model(raw) content = extract_llm_message_content(raw) - verdict = extract_json_object(content) + verdict = _unwrap_noema_verdict(extract_json_object(content)) active_phase = "validating" decision = str(verdict.get("decision") or "").strip().lower() if decision not in {"approve", "request_changes", "comment"}: @@ -1632,11 +1969,20 @@ def call_llm( "Noema LLM request_changes response did not contain a substantive finding" ) validate_substantive_verdict(verdict, diff, changed_paths) + validate_evidence_provenance(verdict) except (RuntimeError, urllib.error.URLError, http.client.HTTPException, OSError) as exc: gateway_telemetry: dict[str, str | int] = {} if isinstance(exc, urllib.error.HTTPError): active_phase = "response_error" - gateway_telemetry = _extract_http_error_telemetry(exc) + try: + gateway_telemetry = _extract_http_error_telemetry(exc) + finally: + # HTTPError owns its response body. Telemetry reads only a + # bounded allowlisted prefix, then this caller must release the + # socket/file even when decoding or schema inspection fails. + # Keep the primary transport error; process cancellation still propagates. + with suppress(Exception): + exc.close() model_value = gateway_telemetry.get("served_model") served_model = model_value if isinstance(model_value, str) else None elapsed = time.monotonic() - attempt_started @@ -1707,7 +2053,7 @@ def format_review_evidence(verdict: dict[str, Any]) -> list[str]: for probe in (validation.get("probes") or [])[:20]: if isinstance(probe, dict): lines.append( - f"- `{probe.get('path')}:{probe.get('line')} ({probe.get('side')})` " + f"- [{probe.get('probe_kind') or 'legacy'}] `{probe.get('path')}:{probe.get('line')} ({probe.get('side')})` " f"{probe.get('outcome')}: {str(probe.get('hypothesis') or '').strip()} — " f"{str(probe.get('evidence') or '').strip()}" ) diff --git a/tests/conftest.py b/tests/conftest.py index 6f0c91d00f..c3b50c68e4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -21,6 +21,26 @@ def clear_trusted_uv_process_caches() -> Iterator[None]: opener_cache_clear() +@pytest.fixture(autouse=True) +def isolate_noema_repair_deadline_from_external_dns(request: pytest.FixtureRequest, monkeypatch: pytest.MonkeyPatch) -> None: + """Keep the repair-deadline unit test about wall time, not external DNS latency. + + ``call_llm`` deliberately resolves configured public hosts as part of its + SSRF guard. The deadline regression replaces the HTTP opener but used to + leave that DNS lookup live, so a cold/slow resolver could consume several + seconds before the synthetic slow-read path even began and make the + otherwise-correct 50 ms process-timer assertion fail nondeterministically. + Other Noema SSRF tests retain the real resolver/mocked resolver behavior; + only this single unit test gets a no-op URL guard because URL admission is + outside the behavior it is asserting. + """ + if request.node.name != "test_total_repair_wall_clock_deadline_interrupts_slow_read": + return + from scripts.ci import noema_review_gate as gate + + monkeypatch.setattr(gate, "reject_private_llm_url", lambda _url: None) + + class FakeHttpResponse: """Expose bounded context-managed reads from one deterministic final URL.""" diff --git a/tests/test_noema_class_evidence_observation_contract.py b/tests/test_noema_class_evidence_observation_contract.py new file mode 100644 index 0000000000..c840fa9c98 --- /dev/null +++ b/tests/test_noema_class_evidence_observation_contract.py @@ -0,0 +1,471 @@ +"""Regression tests for class-specific Noema probe observation evidence.""" + +from __future__ import annotations + +import pytest + +from scripts.ci import noema_review_gate as noema + + +DIFF = """diff --git a/src/tool.py b/src/tool.py +--- a/src/tool.py ++++ b/src/tool.py +@@ -1 +1 @@ +-old = 1 ++new = 1 +""" + + +def _assert_matches_declared_schema(value: object, schema: dict[str, object]) -> None: + """Apply the strict-output JSON Schema subset used by the Noema contract.""" + variants = schema.get("anyOf") + if isinstance(variants, list): + failures: list[str] = [] + for variant in variants: + try: + _assert_matches_declared_schema(value, variant) + except AssertionError as exc: + failures.append(str(exc)) + else: + return + raise AssertionError("no anyOf variant admitted the verdict: " + "; ".join(failures)) + + expected_type = schema.get("type") + allowed_types = expected_type if isinstance(expected_type, list) else [expected_type] + if value is None: + actual_type = "null" + elif isinstance(value, dict): + actual_type = "object" + elif isinstance(value, list): + actual_type = "array" + elif type(value) is int: + actual_type = "integer" + elif isinstance(value, str): + actual_type = "string" + else: + actual_type = type(value).__name__ + assert actual_type in allowed_types, f"expected {allowed_types}, got {actual_type}" + + if "enum" in schema: + assert value in schema["enum"] + if actual_type == "object": + properties = schema.get("properties") + assert isinstance(properties, dict) + required = schema.get("required") + assert isinstance(required, list) + assert set(required) == set(properties), "strict objects require every property" + assert set(value) == set(properties), "required/additional properties diverged" + for key, child_schema in properties.items(): + _assert_matches_declared_schema(value[key], child_schema) + elif actual_type == "array": + assert len(value) >= int(schema.get("minItems", 0)) + for item in value: + _assert_matches_declared_schema(item, schema["items"]) + + +def _assert_strict_object_contract(schema: dict[str, object]) -> None: + """Require every nested object variant to use the strict SDK shape.""" + variants = schema.get("anyOf") + if isinstance(variants, list): + for variant in variants: + _assert_strict_object_contract(variant) + return + schema_type = schema.get("type") + allowed_types = schema_type if isinstance(schema_type, list) else [schema_type] + if "object" in allowed_types: + properties = schema.get("properties") + assert isinstance(properties, dict) + assert schema.get("additionalProperties") is False + assert set(schema.get("required", [])) == set(properties) + for child_schema in properties.values(): + _assert_strict_object_contract(child_schema) + if "array" in allowed_types: + _assert_strict_object_contract(schema["items"]) + + +def _location() -> dict[str, object]: + """Return the single exact changed-side location used by this fixture.""" + return {"path": "src/tool.py", "line": 1, "side": "RIGHT"} + + +def _class_evidence( + kind: str, + *, + observations: bool, + repeated: bool = False, + source_excerpt: bool = False, + generic_but_different: bool = False, +) -> dict[str, object]: + """Build class evidence spanning the intentionally weak and hardened schemas.""" + evidence: dict[str, object] = {} + for index, field in enumerate(noema.OBSERVED_REVIEW_PROBE_EVIDENCE_FIELDS[kind], start=1): + witness = _location() + witness["claim_role"] = noema.OBSERVED_REVIEW_PROBE_CLAIM_ROLES[kind][field] + if observations: + if repeated: + witness["observation"] = "new = 1 is the same repeated source observation." + elif generic_but_different: + witness["observation"] = ( + f"Generic {field.replace('_', ' ')} concern appears in this area." + ) + else: + witness["observation"] = ( + f"new = 1 is exact source evidence for structured witness {index}: {field}." + ) + if source_excerpt: + witness["source_excerpt"] = "new = 1" + evidence[field] = witness + return evidence + + +def _probe( + kind: str, + *, + observations: bool, + repeated: bool = False, + source_excerpt: bool = False, + generic_but_different: bool = False, +) -> dict[str, object]: + """Build one adversarial probe for the requested observed defect class.""" + return { + **_location(), + "probe_kind": kind, + "class_evidence": _class_evidence( + kind, + observations=observations, + repeated=repeated, + source_excerpt=source_excerpt, + generic_but_different=generic_but_different, + ), + "hypothesis": f"Generic hypothesis relabeled as {kind}.", + "attack_or_counterexample": f"Generic attack relabeled as {kind}.", + "evidence": f"Probe evidence for {kind}.", + "outcome": "falsified", + } + + +def _verdict( + *, + observations: bool, + repeated: bool = False, + source_excerpt: bool = False, + generic_but_different: bool = False, +) -> dict[str, object]: + """Build an otherwise-valid approval verdict with two distinct class labels.""" + return { + "decision": "approve", + "summary": "Two observed defect classes were attacked.", + "findings": [], + "reviewed_lines": [{**_location(), "analysis": "Reviewed exact changed line."}], + "adversarial_validation": { + "status": "passed", + "residual_risk": "Unit fixture does not exercise an external runtime.", + "probes": [ + _probe( + "mutable_alias", + observations=observations, + repeated=repeated, + source_excerpt=source_excerpt, + generic_but_different=generic_but_different, + ), + _probe( + "time_of_check_time_of_use", + observations=observations, + repeated=repeated, + source_excerpt=source_excerpt, + generic_but_different=generic_but_different, + ), + ], + }, + } + + +def test_location_only_class_evidence_cannot_relabel_generic_probes() -> None: + """Different taxonomy labels cannot make coordinate-only generic probes substantive.""" + with pytest.raises(noema.NoemaModelOutputError, match="non-empty observation"): + noema.validate_substantive_verdict(_verdict(observations=False), DIFF, ["src/tool.py"]) + + +def test_class_evidence_requires_exact_observed_field_set() -> None: + """A defect-class label cannot omit one of its schema-defined witness roles.""" + verdict = _verdict(observations=True, source_excerpt=True) + verdict["adversarial_validation"]["probes"][0]["class_evidence"].pop("mutation_attempt") + + with pytest.raises(noema.NoemaModelOutputError, match="must contain exactly"): + noema.validate_substantive_verdict(verdict, DIFF, ["src/tool.py"]) + + +def test_empty_class_observation_is_rejected() -> None: + """Exact source coordinates and excerpts do not substitute for an observation.""" + verdict = _verdict(observations=True, source_excerpt=True) + verdict["adversarial_validation"]["probes"][0]["class_evidence"]["mutation_attempt"][ + "observation" + ] = "" + + with pytest.raises(noema.NoemaModelOutputError, match="non-empty observation"): + noema.validate_substantive_verdict(verdict, DIFF, ["src/tool.py"]) + + +def test_repeated_generic_observations_do_not_satisfy_class_specific_witnesses() -> None: + """A probe must provide distinct observations for its class-specific witness fields.""" + with pytest.raises(noema.NoemaModelOutputError, match="distinct class-specific observations"): + noema.validate_substantive_verdict( + _verdict(observations=True, repeated=True, source_excerpt=True), + DIFF, + ["src/tool.py"], + ) + + +def test_differently_worded_generic_observations_without_source_signal_are_rejected() -> None: + """Unique prose labels are not evidence unless they name concrete changed-source content.""" + with pytest.raises(noema.NoemaModelOutputError, match="quote the exact source_excerpt"): + noema.validate_substantive_verdict( + _verdict( + observations=True, + source_excerpt=True, + generic_but_different=True, + ), + DIFF, + ["src/tool.py"], + ) + + +def test_fabricated_source_excerpt_is_rejected() -> None: + """A model cannot attach a plausible observation to source text absent from the cited line.""" + verdict = _verdict(observations=True, source_excerpt=True) + verdict["adversarial_validation"]["probes"][0]["class_evidence"]["mutation_attempt"][ + "source_excerpt" + ] = "fabricated = 2" + + with pytest.raises(noema.NoemaModelOutputError, match="exact changed-line source_excerpt"): + noema.validate_substantive_verdict(verdict, DIFF, ["src/tool.py"]) + + +def test_invented_claim_role_cannot_replace_class_specific_evidence() -> None: + """Free-form labels cannot substitute for the schema's exact class-and-field role.""" + verdict = _verdict(observations=True, source_excerpt=True) + verdict["adversarial_validation"]["probes"][0]["class_evidence"]["mutation_attempt"][ + "claim_role" + ] = "banana" + + with pytest.raises(noema.NoemaModelOutputError, match="claim_role must be"): + noema.validate_substantive_verdict(verdict, DIFF, ["src/tool.py"]) + + +def test_distinct_source_bound_class_observations_are_accepted() -> None: + """Concrete source-backed observations preserve an otherwise-valid multi-class verdict.""" + noema.validate_substantive_verdict( + _verdict(observations=True, source_excerpt=True), + DIFF, + ["src/tool.py"], + ) + + +def test_outbound_strict_schema_and_local_validator_admit_the_same_verdict() -> None: + """The exact structured-output receipt cannot contradict local admission.""" + verdict = _verdict(observations=True, source_excerpt=True) + schema = noema._noema_verdict_json_schema(required_probes=2) + + _assert_matches_declared_schema({"verdict": verdict}, schema) + noema.validate_substantive_verdict(verdict, DIFF, ["src/tool.py"]) + + +@pytest.mark.parametrize("envelope", [{}, {"verdict": []}, {"verdict": {}, "extra": {}}]) +def test_local_unwrap_rejects_noncanonical_gateway_envelopes( + envelope: dict[str, object], +) -> None: + """Missing, non-object, and extra envelope fields remain fail closed.""" + with pytest.raises(noema.NoemaModelOutputError, match="exactly one structured verdict"): + noema._unwrap_noema_verdict(envelope) + + +@pytest.mark.parametrize( + ("decision", "valid_status", "invalid_status"), + [ + ("approve", "passed", "failed"), + ("request_changes", "failed", "passed"), + ], +) +def test_outbound_schema_rejects_decision_status_pairs_rejected_locally( + decision: str, + valid_status: str, + invalid_status: str, +) -> None: + """The gateway schema and local semantic backstop reject the same state flips.""" + verdict = _verdict(observations=True, source_excerpt=True) + verdict["decision"] = decision + verdict["adversarial_validation"]["status"] = valid_status + if decision == "request_changes": + verdict["adversarial_validation"]["probes"][0]["outcome"] = "confirmed" + verdict["findings"] = [ + { + "severity": "high", + "file": "src/tool.py", + "line": 1, + "side": "RIGHT", + "message": "Confirmed defect at the changed line.", + } + ] + + schema = noema._noema_verdict_json_schema(required_probes=2) + _assert_matches_declared_schema({"verdict": verdict}, schema) + noema.validate_substantive_verdict(verdict, DIFF, ["src/tool.py"]) + + verdict["adversarial_validation"]["status"] = invalid_status + with pytest.raises(AssertionError, match="no anyOf variant admitted"): + _assert_matches_declared_schema({"verdict": verdict}, schema) + with pytest.raises(noema.NoemaModelOutputError, match=f"status={valid_status}"): + noema.validate_substantive_verdict(verdict, DIFF, ["src/tool.py"]) + + +def test_every_outbound_probe_variant_is_strict_and_taxonomy_complete() -> None: + """Nested unions remain SDK-compatible and cover the closed local taxonomy.""" + response_format = noema._noema_verdict_response_format(required_probes=2) + assert response_format["json_schema"]["strict"] is True + schema = response_format["json_schema"]["schema"] + assert schema["type"] == "object" + _assert_strict_object_contract(schema) + + verdict_variants = schema["properties"]["verdict"]["anyOf"] + approve_schema = next( + variant + for variant in verdict_variants + if variant["properties"]["decision"]["enum"] == ["approve"] + ) + probe_variants = approve_schema["properties"]["adversarial_validation"]["properties"][ + "probes" + ]["items"]["anyOf"] + assert { + variant["properties"]["probe_kind"]["enum"][0] + for variant in probe_variants + } == noema.OBSERVED_REVIEW_PROBE_KINDS + assert approve_schema["properties"]["adversarial_validation"]["type"] == "object" + assert all( + variant["properties"]["class_evidence"]["type"] == "object" + for variant in probe_variants + ) + + +@pytest.mark.parametrize( + ("record", "message"), + [ + ({"path": "", "line": 1, "side": "RIGHT"}, "canonical changed-side path"), + ({"path": "src/tool.py", "line": True, "side": "RIGHT"}, "canonical positive integer line"), + ({"path": "src/tool.py", "line": 0, "side": "RIGHT"}, "canonical positive integer line"), + ({"path": "src/tool.py", "line": 1, "side": "right"}, "canonical LEFT/RIGHT side"), + ], +) +def test_canonical_changed_location_rejects_noncanonical_coordinates( + record: dict[str, object], message: str +) -> None: + """Canonical source coordinates reject empty paths, bool/int aliases, and invalid sides.""" + with pytest.raises(noema.NoemaModelOutputError, match=message): + noema._canonical_changed_location(record, "fixture") + + +def test_changed_diff_line_texts_covers_context_markers_and_no_newline_marker() -> None: + """A genuine marker-shaped source line remains exact review evidence.""" + diff = """diff --git a/src/tool.py b/src/tool.py +--- a/src/tool.py ++++ b/src/tool.py +@@ -1,3 +1,3 @@ + context +-[overlong changed line content omitted] ++[overlong changed line content omitted] +-old ++new +\\ No newline at end of file +""" + assert noema.changed_diff_line_texts(diff) == { + ("src/tool.py", 2, "LEFT"): "[overlong changed line content omitted]", + ("src/tool.py", 2, "RIGHT"): "[overlong changed line content omitted]", + ("src/tool.py", 3, "LEFT"): "old", + ("src/tool.py", 3, "RIGHT"): "new", + } + assert noema.changed_diff_locations(diff) == set(noema.changed_diff_line_texts(diff)) + + +def test_changed_diff_line_texts_fails_closed_when_hunk_paths_are_missing() -> None: + """A hunk without its canonical file headers cannot manufacture source evidence.""" + assert noema.changed_diff_line_texts("@@ -1 +1 @@\n+new\n") == {} + assert noema.changed_diff_line_texts("@@ -1 +1 @@\n-old\n") == {} + + +def test_changed_diff_line_texts_handles_dev_null_addition() -> None: + """New files may have an empty old path while their RIGHT-side source remains exact.""" + diff = """diff --git a/new.py b/new.py +--- /dev/null ++++ b/new.py +@@ -0,0 +1 @@ ++value = 1 +""" + assert noema.changed_diff_line_texts(diff) == {("new.py", 1, "RIGHT"): "value = 1"} + + +def test_blank_changed_source_uses_explicit_blank_marker() -> None: + """A blank changed line remains admissible through exact equality and the explicit marker.""" + diff = """diff --git a/src/tool.py b/src/tool.py +--- a/src/tool.py ++++ b/src/tool.py +@@ -1 +1 @@ +-old = 1 ++ +""" + verdict = _verdict(observations=True, source_excerpt=True) + for probe in verdict["adversarial_validation"]["probes"]: + for field, witness in probe["class_evidence"].items(): + witness["source_excerpt"] = "" + witness["observation"] = f" is exact source evidence for {probe['probe_kind']}:{field}." + noema.validate_substantive_verdict(verdict, diff, ["src/tool.py"]) + + +def test_whitespace_only_changed_source_uses_explicit_blank_marker() -> None: + """Whitespace-only source cannot satisfy evidence through incidental prose spaces.""" + spaces = " " + diff = f"""diff --git a/src/tool.py b/src/tool.py +--- a/src/tool.py ++++ b/src/tool.py +@@ -1 +1 @@ +-old = 1 ++{spaces} +""" + verdict = _verdict(observations=True, source_excerpt=True) + for probe in verdict["adversarial_validation"]["probes"]: + for field, witness in probe["class_evidence"].items(): + witness["source_excerpt"] = spaces + witness["observation"] = f"ordinary prose space is not evidence for {probe['probe_kind']}:{field}." + + with pytest.raises(noema.NoemaModelOutputError, match=r"must quote the exact source_excerpt"): + noema.validate_substantive_verdict(verdict, diff, ["src/tool.py"]) + + for probe in verdict["adversarial_validation"]["probes"]: + for field, witness in probe["class_evidence"].items(): + witness["observation"] = f" is exact source evidence for {probe['probe_kind']}:{field}." + noema.validate_substantive_verdict(verdict, diff, ["src/tool.py"]) + + +def test_literal_omission_marker_source_remains_reviewable() -> None: + """Literal source text must not alias synthetic prompt-truncation metadata.""" + marker = "[overlong changed line content omitted]" + diff = f"""diff --git a/src/tool.py b/src/tool.py +--- a/src/tool.py ++++ b/src/tool.py +@@ -1 +1 @@ +-old = 1 ++{marker} +""" + verdict = _verdict(observations=True, source_excerpt=True) + for probe in verdict["adversarial_validation"]["probes"]: + for field, witness in probe["class_evidence"].items(): + witness["source_excerpt"] = marker + witness["observation"] = f"{marker} is exact source evidence for {probe['probe_kind']}:{field}." + noema.validate_substantive_verdict(verdict, diff, ["src/tool.py"]) + + +def test_overlong_class_observation_is_rejected_before_semantic_admission() -> None: + """Bounded review evidence refuses oversized witness prose without weakening source checks.""" + verdict = _verdict(observations=True, source_excerpt=True) + witness = verdict["adversarial_validation"]["probes"][0]["class_evidence"]["mutation_attempt"] + witness["observation"] = "x" * (noema.MAX_THREAD_BODY_CHARS + 1) + with pytest.raises(noema.NoemaModelOutputError, match="exceeds"): + noema.validate_substantive_verdict(verdict, DIFF, ["src/tool.py"]) diff --git a/tests/test_noema_observed_defect_corpus_current_main.py b/tests/test_noema_observed_defect_corpus_current_main.py new file mode 100644 index 0000000000..76857a635d --- /dev/null +++ b/tests/test_noema_observed_defect_corpus_current_main.py @@ -0,0 +1,353 @@ +"""Executable regressions for observed Noema review false-negative shapes. + +These cases are grounded in externally demonstrated review findings rather than +claims of benchmark superiority. They keep the trusted review admission layer +honest about exact source coordinates and require the model prompt/validator to +attack more than one high-value defect class on material changes. +""" + +from __future__ import annotations + +import json + +import pytest + +from scripts.ci import noema_review_gate as noema + + +DIFF = """diff --git a/src/tool.py b/src/tool.py +--- a/src/tool.py ++++ b/src/tool.py +@@ -1 +1 @@ +-old = 1 ++new = 1 +""" + + +def _source_ref() -> dict[str, object]: + return {"path": "src/tool.py", "line": 1, "side": "RIGHT"} + + +def _class_evidence(kind: str) -> dict[str, dict[str, object]]: + return { + field: { + **_source_ref(), + "source_excerpt": "new = 1", + "claim_role": noema.OBSERVED_REVIEW_PROBE_CLAIM_ROLES[kind][field], + "observation": ( + f"new = 1 is exact source evidence for structured role {index}: {field}." + ), + } + for index, field in enumerate( + noema.OBSERVED_REVIEW_PROBE_EVIDENCE_FIELDS[kind], + start=1, + ) + } + + +def _probe(kind: str, *, hypothesis: str) -> dict[str, object]: + return { + **_source_ref(), + "probe_kind": kind, + "class_evidence": _class_evidence(kind), + "hypothesis": hypothesis, + "attack_or_counterexample": f"Attack {kind} at the exact changed line.", + "evidence": f"Observed source-bound evidence for {kind}.", + "outcome": "falsified", + } + + +def _verdict() -> dict[str, object]: + return { + "decision": "approve", + "summary": "Two independently classified defect shapes were attacked.", + "findings": [], + "reviewed_lines": [{**_source_ref(), "analysis": "Reviewed the exact changed line."}], + "adversarial_validation": { + "status": "passed", + "residual_risk": "No runtime integration exercise was available in this unit fixture.", + "probes": [ + _probe("mutable_alias", hypothesis="Caller-owned mutable state may escape validation."), + _probe( + "time_of_check_time_of_use", + hypothesis="A changing getter may differ between validation and use.", + ), + ], + }, + } + + +@pytest.mark.parametrize("container", [True, False]) +def test_boolean_reviewed_line_cannot_alias_integer_coordinate(container: bool) -> None: + verdict = _verdict() + verdict["reviewed_lines"][0]["line"] = container + + with pytest.raises(noema.NoemaModelOutputError, match="canonical positive integer line"): + noema.validate_substantive_verdict(verdict, DIFF, ["src/tool.py"]) + + +@pytest.mark.parametrize("container", [True, False]) +def test_boolean_probe_line_cannot_alias_integer_coordinate(container: bool) -> None: + verdict = _verdict() + verdict["adversarial_validation"]["probes"][0]["line"] = container + + with pytest.raises(noema.NoemaModelOutputError, match="canonical positive integer line"): + noema.validate_substantive_verdict(verdict, DIFF, ["src/tool.py"]) + + +def test_material_review_requires_distinct_observed_defect_classes() -> None: + verdict = _verdict() + verdict["adversarial_validation"]["probes"] = [ + _probe("mutable_alias", hypothesis="First mutable-alias wording."), + _probe("mutable_alias", hypothesis="Different prose, same defect shape."), + ] + + with pytest.raises(noema.NoemaModelOutputError, match="distinct probe_kind"): + noema.validate_substantive_verdict(verdict, DIFF, ["src/tool.py"]) + + +@pytest.mark.parametrize("probe_kind", [[], {}, "unknown_shape"]) +def test_probe_kind_fails_closed_on_malformed_or_unknown_values(probe_kind: object) -> None: + verdict = _verdict() + verdict["adversarial_validation"]["probes"][0]["probe_kind"] = probe_kind + + with pytest.raises(noema.NoemaModelOutputError, match="observed defect taxonomy"): + noema.validate_substantive_verdict(verdict, DIFF, ["src/tool.py"]) + + +def test_class_evidence_must_be_source_bound_to_the_probe_location() -> None: + verdict = _verdict() + probe = verdict["adversarial_validation"]["probes"][0] + probe["class_evidence"]["mutation_attempt"] = { + "path": "src/tool.py", + "line": 1, + "side": "LEFT", + "source_excerpt": "old = 1", + "claim_role": noema.OBSERVED_REVIEW_PROBE_CLAIM_ROLES["mutable_alias"]["mutation_attempt"], + "observation": "old = 1 is exact source evidence for the mutation-attempt role.", + } + + with pytest.raises(noema.NoemaModelOutputError, match="must bind to the probe location"): + noema.validate_substantive_verdict(verdict, DIFF, ["src/tool.py"]) + + +def test_valid_observed_defect_taxonomy_verdict_is_accepted() -> None: + noema.validate_substantive_verdict(_verdict(), DIFF, ["src/tool.py"]) + + +def test_unreceipted_runtime_and_official_documentation_claims_fail_closed() -> None: + verdict = _verdict() + verdict["decision"] = "request_changes" + verdict["summary"] = ( + "Cargo CLI documentation and runtime behavior confirm that --locked is unsupported." + ) + validation = verdict["adversarial_validation"] + validation["status"] = "failed" + validation["probes"][0]["outcome"] = "confirmed" + validation["probes"][0]["evidence"] = ( + "Official Cargo documentation confirms the command rejects --locked." + ) + verdict["findings"] = [ + { + "severity": "medium", + "file": "src/tool.py", + "line": 1, + "side": "RIGHT", + "message": "Runtime behavior confirms an unexpected-argument failure.", + } + ] + + with pytest.raises(noema.NoemaModelOutputError, match=r"trusted .*receipt"): + noema.validate_evidence_provenance(verdict) + + +@pytest.mark.parametrize( + "unreceipted_claim", + [ + ( + "--locked is not a valid invocation: --locked is not accepted by the " + "generate-lockfile subcommand. This will always fail, breaking the workflow." + ), + "The generate-lockfile subcommand rejects --locked, so this workflow cannot succeed.", + ], + ids=["concept35-original", "concept35-synonym"], +) +def test_unreceipted_external_behavior_claims_fail_closed( + unreceipted_claim: str, +) -> None: + """Original external-behavior prose must not bypass receipt provenance.""" + verdict = _verdict() + verdict["decision"] = "request_changes" + verdict["summary"] = unreceipted_claim + validation = verdict["adversarial_validation"] + validation["status"] = "failed" + validation["probes"][0]["outcome"] = "confirmed" + validation["probes"][0]["evidence"] = ( + "The exact changed line was selected for a source-bound behavior hypothesis." + ) + verdict["findings"] = [ + { + "severity": "medium", + "file": "src/tool.py", + "line": 1, + "side": "RIGHT", + "message": unreceipted_claim, + } + ] + + # Keep the reproducer causal: substantive/source binding already succeeds; + # only the missing trusted provenance must reject the verdict. + noema.validate_substantive_verdict(verdict, DIFF, ["src/tool.py"]) + with pytest.raises(noema.NoemaModelOutputError, match=r"trusted .*receipt"): + noema.validate_evidence_provenance(verdict) + + +def test_source_reasoning_and_verification_direction_do_not_claim_execution() -> None: + verdict = _verdict() + verdict["summary"] = "The changed source invokes cargo generate-lockfile --locked." + verdict["reviewed_lines"][0]["analysis"] = ( + "Verify this source-level hypothesis against the repository-pinned Cargo help output." + ) + + noema.validate_evidence_provenance(verdict) + + +def test_trusted_receipt_must_be_typed_and_explicitly_cited() -> None: + verdict = _verdict() + verdict["summary"] = "Runtime behavior confirms the guard passes [receipt:cargo-help-1]." + + with pytest.raises(noema.NoemaModelOutputError, match=r"trusted .*receipt"): + noema.validate_evidence_provenance( + verdict, + trusted_source_receipt_ids={"cargo-help-1"}, + ) + + noema.validate_evidence_provenance( + verdict, + trusted_execution_receipt_ids={"cargo-help-1"}, + ) + + verdict["summary"] = ( + "Official Cargo documentation confirms the option is supported " + "[receipt:cargo-docs-1]." + ) + with pytest.raises(noema.NoemaModelOutputError, match=r"trusted .*receipt"): + noema.validate_evidence_provenance( + verdict, + trusted_execution_receipt_ids={"cargo-docs-1"}, + ) + noema.validate_evidence_provenance( + verdict, + trusted_source_receipt_ids={"cargo-docs-1"}, + ) + + +def test_evidence_statement_collection_ignores_non_prose_container_values() -> None: + """Malformed optional containers cannot become provenance claim text.""" + verdict = { + "summary": " ", + "reviewed_lines": [None, {"analysis": "reviewed source"}], + "adversarial_validation": { + "residual_risk": None, + "probes": [ + None, + { + "hypothesis": "hypothesis", + "attack_or_counterexample": "attack", + "evidence": "evidence", + "class_evidence": None, + }, + { + "hypothesis": "second hypothesis", + "attack_or_counterexample": "second attack", + "evidence": "second evidence", + "class_evidence": { + "malformed": None, + "valid": {"observation": "observed source"}, + }, + }, + ], + }, + "findings": [None, {"message": "finding"}], + } + + assert noema._model_evidence_statements(verdict) == [ + "reviewed source", + "hypothesis", + "attack", + "evidence", + "second hypothesis", + "second attack", + "second evidence", + "observed source", + "finding", + ] + + +def test_noema_prompt_names_every_observed_defect_class(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example.test/v1/chat/completions") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "test-key") + monkeypatch.setattr(noema, "reject_private_llm_url", lambda _url: None) + monkeypatch.setattr(noema, "validate_substantive_verdict", lambda *_args: None) + monkeypatch.setattr( + noema, + "fetch_pr", + lambda _repo, _number: {"state": "OPEN", "headRefOid": "a" * 40}, + ) + seen: dict[str, object] = {} + + class Response: + def __enter__(self): + return self + + def __exit__(self, *_args): + return False + + def read(self): + payload = { + "choices": [ + { + "message": { + "content": json.dumps( + { + "verdict": { + "decision": "comment", + "summary": "ok", + "findings": [], + } + } + ) + } + } + ] + } + return json.dumps(payload).encode("utf-8") + + class Opener: + def open(self, request, timeout=None): + seen["request"] = json.loads(request.data.decode("utf-8")) + return Response() + + monkeypatch.setattr(noema.urllib.request, "build_opener", lambda *_args: Opener()) + pr = {"title": "fixture", "headRefOid": "a" * 40} + + noema.call_llm( + "owner/repo", + 7, + pr, + DIFF, + False, + "a" * 40, + changed_paths=["src/tool.py"], + ) + + prompt = seen["request"]["messages"][1]["content"] + for probe_kind in noema.OBSERVED_REVIEW_PROBE_KINDS: + assert probe_kind in prompt + assert "class_evidence" in prompt + assert "exact changed-side" in prompt + assert "source_excerpt" in prompt + assert "workflow-starting credential" in prompt + assert "downstream required checks" in prompt + assert "cannot execute commands or access external documentation" in prompt + assert "[receipt:]" in prompt diff --git a/tests/test_noema_repair_attempt_telemetry.py b/tests/test_noema_repair_attempt_telemetry.py index d90084a0a5..7b92ad2452 100644 --- a/tests/test_noema_repair_attempt_telemetry.py +++ b/tests/test_noema_repair_attempt_telemetry.py @@ -27,6 +27,27 @@ def _verdict() -> dict: "residual_risk": "No additional risk identified.", "probes": [{ "path": "README.md", "line": 1, "side": "RIGHT", + "probe_kind": "test_oracle", + "class_evidence": { + "assertion_under_test": { + "path": "README.md", "line": 1, "side": "RIGHT", + "source_excerpt": "new", + "claim_role": gate.OBSERVED_REVIEW_PROBE_CLAIM_ROLES["test_oracle"]["assertion_under_test"], + "observation": "The exact source `new` is the behavior claimed by this fixture.", + }, + "negative_control": { + "path": "README.md", "line": 1, "side": "RIGHT", + "source_excerpt": "new", + "claim_role": gate.OBSERVED_REVIEW_PROBE_CLAIM_ROLES["test_oracle"]["negative_control"], + "observation": "The exact source `new` is inspected by the fixture oracle.", + }, + "distinguishing_observation": { + "path": "README.md", "line": 1, "side": "RIGHT", + "source_excerpt": "new", + "claim_role": gate.OBSERVED_REVIEW_PROBE_CLAIM_ROLES["test_oracle"]["distinguishing_observation"], + "observation": "The exact source `new` would expose a mismatched fixture result.", + }, + }, "hypothesis": "The replacement could be wrong.", "attack_or_counterexample": "Inspect the exact changed line.", "evidence": "The new value is present at the cited line.", @@ -57,7 +78,12 @@ def open_response(_opener, request, **kwargs): def test_success_uses_one_request_and_one_phase_annotation(monkeypatch, capsys) -> None: - raw = json.dumps({"model": "provider/model", "choices": [{"message": {"content": json.dumps(_verdict())}}]}).encode() + raw = json.dumps( + { + "model": "provider/model", + "choices": [{"message": {"content": json.dumps({"verdict": _verdict()})}}], + } + ).encode() requests = _configure(monkeypatch, raw) verdict = gate.call_llm("owner/repo", 7, {"title": "t", "headRefOid": "a" * 40}, DIFF, False, "a" * 40, changed_paths=("README.md",)) assert verdict["decision"] == "approve" @@ -205,6 +231,12 @@ def test_malformed_verdict_json_is_not_retried(monkeypatch) -> None: def test_rejected_changed_line_verdict_is_not_retried(monkeypatch) -> None: verdict = _verdict() verdict["decision"] = "request_changes" + verdict["adversarial_validation"]["status"] = "failed" + probe = verdict["adversarial_validation"]["probes"][0] + probe["line"] = 99 + probe["outcome"] = "confirmed" + for witness in probe["class_evidence"].values(): + witness["line"] = 99 verdict["findings"] = [{ "severity": "high", "file": "README.md", @@ -212,8 +244,16 @@ def test_rejected_changed_line_verdict_is_not_retried(monkeypatch) -> None: "side": "RIGHT", "message": "Outside the changed hunk.", }] - raw = json.dumps({"model": "provider/model", "choices": [{"message": {"content": json.dumps(verdict)}}]}).encode() + raw = json.dumps( + { + "model": "provider/model", + "choices": [{"message": {"content": json.dumps({"verdict": verdict})}}], + } + ).encode() calls, kwargs = _invoke_once(monkeypatch, raw=raw) - with pytest.raises(gate.NoemaModelOutputError, match="caller attempts=1"): + with pytest.raises( + gate.NoemaModelOutputError, + match=r"adversarial probe entry 1/1 .*line=99.*not an exact changed-side line.*caller attempts=1", + ): gate.call_llm(**kwargs) assert len(calls) == 1 diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 5fa23dec53..5c44090de5 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -16,6 +16,11 @@ from scripts.ci import noema_review_gate as noema +def _structured_verdict(verdict: dict[str, object]) -> str: + """Encode the exact nested strict-output envelope used by the gateway.""" + return json.dumps({"verdict": verdict}) + + def test_gitleaks_ignore_is_exactly_scoped_to_superseded_uuid_fixture(): entries = { line @@ -870,16 +875,16 @@ def app_identity(args, **kwargs): monkeypatch.setattr(noema, "run", lambda *args, **kwargs: source) diff, truncated = noema.fetch_diff("owner/repo", 1) assert truncated - assert diff.endswith("+[overlong changed line content omitted]") - assert ("a.py", 1, "RIGHT") in noema.changed_diff_locations(diff) + assert "[overlong changed line content omitted]" not in diff + assert ("a.py", 1, "RIGHT") not in noema.changed_diff_locations(diff) assert len(diff) <= noema.MAX_DIFF_CHARS source = "diff --git a/a.py b/a.py\n--- a/a.py\n+++ b/a.py\n@@ -0,0 +1 @@\n+++" + "x" * noema.MAX_DIFF_CHARS monkeypatch.setattr(noema, "run", lambda *args, **kwargs: source) diff, truncated = noema.fetch_diff("owner/repo", 1) assert truncated - assert diff.endswith("+[overlong changed line content omitted]") - assert ("a.py", 1, "RIGHT") in noema.changed_diff_locations(diff) + assert "[overlong changed line content omitted]" not in diff + assert ("a.py", 1, "RIGHT") not in noema.changed_diff_locations(diff) assert noema.extract_json_object('{"decision":"approve"}') == {"decision": "approve"} assert noema.extract_json_object('prefix {"decision":"comment"} suffix') == {"decision": "comment"} @@ -1421,7 +1426,7 @@ def fake_urlopen(request, timeout): "choices": [ { "message": { - "content": json.dumps( + "content": _structured_verdict( { "decision": "approve", "summary": "ok", @@ -1453,7 +1458,9 @@ def open(self, request, timeout=None): assert "extra review context" in seen["body"]["messages"][1]["content"] def fake_urlopen_defer(request, timeout=None): - return FakeResponse({"choices": [{"message": {"content": '{"decision":"defer"}'}}]}) + return FakeResponse( + {"choices": [{"message": {"content": _structured_verdict({"decision": "defer"})}}]} + ) monkeypatch.setattr( noema.urllib.request, @@ -1536,7 +1543,7 @@ def __exit__(self, *args): def read(self): return json.dumps( - {"choices": [{"message": {"content": json.dumps(verdict)}}]} + {"choices": [{"message": {"content": _structured_verdict(verdict)}}]} ).encode() class Opener: @@ -1639,6 +1646,72 @@ def open(self, request): assert secret not in diagnostic +def test_call_llm_closes_http_error_response_after_bounded_telemetry(monkeypatch): + """The one borrowed HTTP error body is always closed after typed extraction.""" + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example.test/chat") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "secret") + body = io.BytesIO( + json.dumps( + { + "error": { + "detail": { + "model": "orchestrator/free", + "terminal_reason": "eligible_candidates_exhausted", + } + } + } + ).encode() + ) + error = noema.urllib.error.HTTPError( + "https://llm.example.test/chat", 502, "Bad Gateway", {}, body + ) + + class Opener: + def open(self, _request): + raise error + + monkeypatch.setattr(noema.urllib.request, "build_opener", lambda *_args: Opener()) + + with pytest.raises(noema.NoemaTransportError): + noema.call_llm("owner/repo", 1, make_pr(), "diff", False, "head") + + assert error.closed + assert body.closed + + +@pytest.mark.parametrize("close_error_type", [OSError, ValueError, RuntimeError, KeyboardInterrupt]) +def test_call_llm_preserves_typed_transport_failure_when_http_error_close_fails( + monkeypatch, close_error_type, +): + """A cleanup error cannot mask the original bounded gateway failure.""" + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example.test/chat") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "secret") + + class CloseFailsHTTPError(noema.urllib.error.HTTPError): + def close(self): + super().close() + raise close_error_type("cleanup failed") + + error = CloseFailsHTTPError( + "https://llm.example.test/chat", + 502, + "Bad Gateway", + {}, + io.BytesIO(b'{}'), + ) + + class Opener: + def open(self, _request): + raise error + + monkeypatch.setattr(noema.urllib.request, "build_opener", lambda *_args: Opener()) + + expected_error = KeyboardInterrupt if close_error_type is KeyboardInterrupt else noema.NoemaTransportError + expected_message = "cleanup failed" if close_error_type is KeyboardInterrupt else "HTTP Error 502" + with pytest.raises(expected_error, match=expected_message): + noema.call_llm("owner/repo", 1, make_pr(), "diff", False, "head") + + @pytest.mark.parametrize( "body", [ @@ -1743,7 +1816,7 @@ def test_noema_redirect_handler_rejects_redirects(): handler = noema.NoRedirectHandler() request = noema.urllib.request.Request("https://llm.example.test/chat") - with pytest.raises(noema.urllib.error.HTTPError): + with pytest.raises(noema.urllib.error.HTTPError) as error_info: handler.redirect_request( request, fp=None, @@ -1752,6 +1825,7 @@ def test_noema_redirect_handler_rejects_redirects(): headers={}, newurl="http://169.254.169.254/latest/meta-data/", ) + error_info.value.close() def test_call_llm_rejects_control_character_scheme_evasion(monkeypatch): @@ -2061,7 +2135,17 @@ def __exit__(self, *args): return None def read(self): - return json.dumps({"choices": [{"message": {"content": '{"decision":"approve"}'}}]}).encode() + return json.dumps( + { + "choices": [ + { + "message": { + "content": _structured_verdict({"decision": "approve"}) + } + } + ] + } + ).encode() monkeypatch.setattr(noema.urllib.request.OpenerDirector, "open", lambda *args, **kwargs: Response()) monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr()) @@ -2115,7 +2199,9 @@ def __exit__(self, *args): return None def read(self): - return json.dumps({"choices": [{"message": {"content": json.dumps(verdict)}}]}).encode() + return json.dumps( + {"choices": [{"message": {"content": _structured_verdict(verdict)}}]} + ).encode() monkeypatch.setattr(noema.urllib.request.OpenerDirector, "open", lambda *args, **kwargs: Response()) monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr()) @@ -2149,7 +2235,9 @@ def __exit__(self, *args): return None def read(self): - return json.dumps({"choices": [{"message": {"content": json.dumps(verdict)}}]}).encode() + return json.dumps( + {"choices": [{"message": {"content": _structured_verdict(verdict)}}]} + ).encode() monkeypatch.setattr(noema.urllib.request.OpenerDirector, "open", lambda *args, **kwargs: Response()) monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr()) @@ -2170,7 +2258,9 @@ def __exit__(self, *args): return None def read(self): - return json.dumps({"choices": [{"message": {"content": json.dumps(verdict)}}]}).encode() + return json.dumps( + {"choices": [{"message": {"content": _structured_verdict(verdict)}}]} + ).encode() monkeypatch.setattr(noema.urllib.request.OpenerDirector, "open", lambda *args, **kwargs: Response()) monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr()) diff --git a/tests/test_noema_review_orchestrator_ssrf.py b/tests/test_noema_review_orchestrator_ssrf.py index cfe7f3c10d..1941ac5dbe 100644 --- a/tests/test_noema_review_orchestrator_ssrf.py +++ b/tests/test_noema_review_orchestrator_ssrf.py @@ -127,7 +127,25 @@ def test_call_llm_allows_matching_orchestrator_sidecar_loopback(monkeypatch): def fake_urlopen(request, timeout): seen["url"] = request.full_url seen["model"] = json.loads(request.data.decode("utf-8"))["model"] - return FakeResponse({"choices": [{"message": {"content": '{\"decision\":\"approve\",\"summary\":\"ok\",\"findings\":[]}'}}]}) + return FakeResponse( + { + "choices": [ + { + "message": { + "content": json.dumps( + { + "verdict": { + "decision": "approve", + "summary": "ok", + "findings": [], + } + } + ) + } + } + ] + } + ) class FakeOpener: def __init__(self, call_func): diff --git a/tests/test_repository_branch_coverage_review_schedulers.py b/tests/test_repository_branch_coverage_review_schedulers.py index 04defb2d3f..91ed565827 100644 --- a/tests/test_repository_branch_coverage_review_schedulers.py +++ b/tests/test_repository_branch_coverage_review_schedulers.py @@ -48,9 +48,11 @@ def read(self) -> bytes: "message": { "content": json.dumps( { - "decision": "approve", - "summary": "clean", - "findings": [], + "verdict": { + "decision": "approve", + "summary": "clean", + "findings": [], + } } ) }