diff --git a/CHANGELOG.md b/CHANGELOG.md index 75a4109c9d..114cfc6fec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,14 @@ - 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] +- Preserve the gateway's bounded error classification in failed Noema review + diagnostics, helping maintainers select the relevant investigation without + exposing free-form response bodies. Missing classifications remain unknown; + this does not resolve the historical gateway failure. See PR #1898 and + `docs/doctoring/noema-repair-attempt-telemetry.md`. +- 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 +159,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 +1469,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/doctoring/noema-repair-attempt-telemetry.md b/docs/doctoring/noema-repair-attempt-telemetry.md index ee4d681a59..c89176724c 100644 --- a/docs/doctoring/noema-repair-attempt-telemetry.md +++ b/docs/doctoring/noema-repair-attempt-telemetry.md @@ -14,7 +14,7 @@ The later review established a second ownership error: `contextual-orchestrator` Noema now sends exactly one structured-output request to the configured gateway. GitHub Actions fixes the model alias to `orchestrator/free`; the caller declares no provider, paid fallback, sampling temperature, or fixed inference timeout. `contextual-orchestrator` owns provider discovery, capability routing, structured-output repair, failover, and upstream completion. The repository remains responsible for deterministic local validation and exact-head publication. -Every gateway call emits exactly one passive Actions annotation. Success and failure annotations include caller attempt count, elapsed duration, active phase (`connecting`, `reading`, `decoding`, or `validating`), and a best-effort serving-model identifier. Serving-model text is secret-scrubbed, control-character-normalized, UTF-8 printable, and bounded before it can reach an annotation. Raw model output is never logged. +Every gateway call emits exactly one passive Actions annotation. Success and failure annotations include caller attempt count, elapsed duration, active phase (`connecting`, `reading`, `decoding`, `validating`, or `response_error`), and a best-effort serving-model identifier. The identifier validator trims exterior whitespace, then accepts only 1–200 ASCII characters in its restricted identifier alphabet. Invalid text is omitted, not repaired. This is format and length validation, not arbitrary secret detection; the gateway must supply non-sensitive identifiers. Raw model output is never logged. The local trailing-comma parser remains a deterministic syntax transform only. It may remove a genuine trailing comma after a complete JSON value, but missing-value forms such as `[,]`, `{,}`, `[1,,]`, and `{"a":,}` remain invalid. The transform emits no second attempt-level annotation and never bypasses semantic verdict validation. @@ -32,3 +32,47 @@ If the gateway cannot produce a valid structured verdict, Noema fails closed aft ## Verification The permanent contract test forbids `NOEMA_REPAIR_DEADLINE_SECONDS`, `_repair_wall_clock_deadline`, `NoemaRepairDeadlineExceeded`, `signal.setitimer`, retry-only parameters/recursion, and caller-specified `temperature`. Focused regressions prove one request on success and failure, one annotation per attempt, safe serving-model telemetry, strict missing-value rejection, accepted genuine trailing commas, and preserved exact changed-line diagnostics. + +## 2026-09-05 follow-up: retain the gateway's error classification + +Status: proposed consumer repair in [central PR #1898](https://github.com/ContextualWisdomLab/.github/pull/1898), not protected delivery or a resolved provider incident. This extends the evidence/control-plane requirement and G-02/G-03; ADR-0003's single-request ownership is unchanged. + +### Observed failure and source trace + +[Naruon #1244's failed job](https://github.com/ContextualWisdomLab/naruon/actions/runs/33933793278/job/101247827882) at head `50351e8cacc65b4124ba2145e00d41aeceef0775` reported HTTP 502, one caller attempt, `duration=1469.1s`, `phase=response_error`, and `served_model=deepseek-ai/deepseek-v4-flash-0731`. It did not preserve an error code or failure kind. The exception label `Noema gateway transport failed` therefore does not establish a network failure, nor does it establish structured-output exhaustion. That historical cause remains unknown. + +Protected contextual-orchestrator source at `a080297d2546bb61e89520d637cabc202db331ec` already maps `ProviderResponseError` to HTTP 502 and the literal `invalid_structured_output` in [`server.py:7978`](https://github.com/ContextualWisdomLab/contextual-orchestrator/blob/a080297d2546bb61e89520d637cabc202db331ec/contextual_orchestrator/server.py#L7978). [`_send_error`](https://github.com/ContextualWisdomLab/contextual-orchestrator/blob/a080297d2546bb61e89520d637cabc202db331ec/contextual_orchestrator/server.py#L8189) adds a request identifier; [`_error_payload`](https://github.com/ContextualWisdomLab/contextual-orchestrator/blob/a080297d2546bb61e89520d637cabc202db331ec/contextual_orchestrator/server.py#L686) places the classification in canonical `error.code`. This path has no `failure_kind`, model, or attempt list. Those source facts identify an observable envelope, not the cause of the earlier Naruon run or an immutable release. + +The original #1898 delta at `df0f735f42adbb44d45f3c3a4e503e400b47ed79` retains optional `error.detail.failure_kind`, but still discards `error.code`. Waiting only for contextual-orchestrator [#1004](https://github.com/ContextualWisdomLab/contextual-orchestrator/pull/1004), whose proposed `36133c8ab85d44fc4be2356edbdd56d9fc09f0d8` adds `structured_output_exhausted`, would leave the existing protected-source envelope unclassified. Noema must not infer that kind from a status code, copy owner repair logic, or consume the proposed branch as a released dependency. + +### Chosen repair and security boundary + +The existing bounded reader and formatter now preserve both independent fields: `error.code` as `error_code`, and `error.detail.failure_kind` as `failure_kind`, when present and valid. The consumer repair is [commit `49ae54e789c0a6951b3212e2182e4c64d0348a81`](https://github.com/ContextualWisdomLab/.github/commit/49ae54e789c0a6951b3212e2182e4c64d0348a81). Both the single failure annotation and the raised diagnostic use the same extracted receipt. Missing fields remain absent; the failure still fails. No request, retry, provider selection, credential, timeout, or verdict-approval rule changes. + +The reader requires canonical mapping envelopes including `error.detail`, reads at most 16 KiB plus one oversize-detection byte, and rejects malformed or oversized bodies. Each new field reuses the existing identifier validator; neither free-form messages, request identifiers, arbitrary detail, nor flattened compatibility aliases are logged. Embedded CR/LF, terminal escape sequences, surrogates, delimiter injection, non-string values, and overlength identifiers cannot enter the new fields. A syntactically valid secret placed in an allowlisted field would not be detected by this validator: non-sensitive canonical classifications remain a producer obligation. + +This follows OWASP's advice to define log field types and lengths, validate data crossing trust zones, prevent log injection, and exclude credentials and sensitive payloads. It does not claim that a regular expression supplies complete redaction (OWASP Foundation, n.d.). + +### Reproduction, integration, and remaining delivery gates + +The existing failed-call regression covers 17 values for each independent field, plus those 17 values with the sparse current gateway envelope: 51 cases. It verifies annotation/exception output, absent sibling fields, one request/annotation, and exclusion of unrelated payload text. The sparse cases have a request identifier and compatibility aliases but no model, attempts, terminal reason, or failure kind. They address an independent static review's missing-fixture finding; they must preserve the code, report an unknown model, and exclude request-ID/message text. Unit-only HTTP doubles replace the external gateway; parsing, formatting, and `call_llm` execute normally. + +Removing the original four `failure_kind` lines produced 3 failures; restoring them produced 138 focused passes. Adding canonical-code assertions to the old implementation reproduced 4 failures and 30 passes, including the actual protected-source code. Before the sparse-fixture extension, the consumer fix plus focused Noema edge coverage and the environment regression produced **159 passed, zero failures/skips**. With the sparse fixture added, removing just the four new code-extraction/formatting lines reproduced **8 failed / 43 passed**; restoring the implementation produced **51 passed**. These are chronological receipts, not totals for the final candidate. + +The first broader run had **2897 passed, one skipped, 21 subtests passed, and one failure**: the task-local uv environment lacked `pip`, needed by `test_materialized_bounded_include_is_resolvable_by_pip`. The project already declares `pip==26.2.1`; installing that exact declared tool fixed the test without changing source, skipping it, or modifying a shared environment. The environment combines hash-locked review requirements and that separately declared pip pin; it is not claimed as an entirely hash-locked clean install. + +The original PR delta and validation commit were preserved by ordinary merges. Protected main `f250638827f8252b0d9e5cb2601f4d333f96162f` (merged prerequisite #1922) is integrated at `719c91b1f678de6da3029b8f5920d6a245520e2e`. A preliminary normal run returned **2923 passed, one skipped, 21 subtests passed** before the sparse-fixture follow-up. The existing LLVM 19 admission test was skipped because its reviewed tools are absent on this macOS host; that path remains unverified, not passed. The separate maintainer exception reported for #1922 is not authorization to bypass #1898's gates. Full normal and `GITHUB_ACTIONS=true` verification must finish on the final integrated candidate, followed by fresh current-head hosted checks and qualifying independent review. Capture exact head/base and final command results in #1898; do not transfer old-head passes. + +Run from the isolated repository root: + +```sh +.venv/bin/python -m pytest -q -W error tests/test_noema_review_gate.py tests/test_noema_model_output_edge_coverage.py +PATH="$PWD/.venv/bin:$PATH" .venv/bin/python -m pytest tests -q -W error -rs +GITHUB_ACTIONS=true PATH="$PWD/.venv/bin:$PATH" .venv/bin/python -m pytest tests -q -W error -rs +``` + +After protected delivery, confirm a real consumer run uses the exact released central revision and gateway contract. If it fails, retain the returned classification and investigate that owner path. Do not reroute to a paid model, repeat an active model request, weaken semantic validation, or declare the historical 502 repaired from unit evidence. Product runtime, real PostgreSQL, browser, release, and deployed-gateway verification are not covered by this consumer diagnostic test. + +### Reference + +OWASP Foundation. (n.d.). *Logging cheat sheet*. OWASP Cheat Sheet Series. Retrieved September 5, 2026, from https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 1cc9e20313..18974fdda3 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -1,5 +1,19 @@ # Product and Technical Gap Baseline +## 2026-09-05 Noema failure classification follow-up — G-02 / G-03 + +This dated follow-up does not refresh the historical inventory below or authorize a merge. Central [#1898](https://github.com/ContextualWisdomLab/.github/pull/1898) remains a proposed repair. Its consumer integration `719c91b1f678de6da3029b8f5920d6a245520e2e` preserves the original `df0f735f42adbb44d45f3c3a4e503e400b47ed79` delta and includes protected main `f250638827f8252b0d9e5cb2601f4d333f96162f` by normal merge. + +- **Product need:** a failed required review must give maintainers a usable next investigation while protecting customer/source content. This supports the PRD-05 quality-first routing outcome and evidence/control-plane TRD; it does not demonstrate retrieval, calendar, connector, or UI acceptance. +- **Observed gap:** Naruon #1244 at `50351e8cacc65b4124ba2145e00d41aeceef0775` failed in [job 101247827882](https://github.com/ContextualWisdomLab/naruon/actions/runs/33933793278/job/101247827882) after 1469.1 seconds and one request. Phase/model telemetry exists, but no preserved classification proves its cause. A transport exception label is not network-failure evidence. +- **Source contract:** protected contextual-orchestrator `a080297d2546bb61e89520d637cabc202db331ec` already returns canonical `error.code=invalid_structured_output` for a structured-response error, without `failure_kind`. Noema previously dropped that code. The proposed #1004 exhaustion kind is separate and remains unreleased evidence. +- **Action and ownership:** #1898 preserves canonical `error.code` and optional `error.detail.failure_kind` with the existing bounded identifier validator in both failure diagnostics. `.github` owns the consumer receipt; contextual-orchestrator owns classification, model validation/repair, discovery, and routing. No extra model request, mutable owner dependency, raw response logging, approval relaxation, or timeout is added. +- **Verification:** canonical-code RED reproduced 4 failed / 30 passed; repaired focused Noema/edge/environment checks returned 159 passed. The earlier full run's missing declared pip tool was repaired in the isolated environment. Integrated full normal/CI runs, hosted required checks, independent review, protected delivery, and an exact-revision consumer run remain separate gates. This entry does not mark G-02 or G-03 closed. + +The [doctoring record](doctoring/noema-repair-attempt-telemetry.md#2026-09-05-follow-up-retain-the-gateways-error-classification) contains exact producer/API links, the rejected alternatives, logging-security limits, APA 7 reference, executable commands, and the remaining owner investigation. The existing ownership flow is unchanged: workflow → gateway-owned routing/validation → one bounded receipt → deterministic local validation → exact-head publication. + +## Historical baseline + 작성 기준일: **2026-08-26 10:35 KST** 대상: **ContextualWisdomLab/.github** 중앙 거버넌스·자동화 레포지터리와 이를 소비하는 naruon 생태계 현재 보호된 `main`: `826b92394c63deb6981c3a8d16a724d71f85a0d7` @@ -2626,6 +2640,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..dc3d939008 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": @@ -1299,7 +1627,7 @@ def decode_llm_response_body(raw_bytes: bytes) -> str: def _extract_served_model(raw: str) -> str | None: - """Return a bounded, scrubbed, single-line UTF-8-printable serving model id.""" + """Return a bounded, format-checked ASCII serving-model identifier.""" try: data = json.loads(raw) except (json.JSONDecodeError, TypeError, ValueError): @@ -1310,7 +1638,7 @@ def _extract_served_model(raw: str) -> str | None: def _safe_model_identifier(value: Any) -> str | None: - """Accept only a conservative, bounded model identifier safe for public logs.""" + """Validate a bounded ASCII identifier's format, without detecting secrets.""" if not isinstance(value, str): return None candidate = value.strip() @@ -1323,8 +1651,8 @@ def _extract_http_error_telemetry(exc: urllib.error.HTTPError) -> dict[str, str """Read bounded, allowlisted gateway failure telemetry without raw diagnostics. The response body is never returned or logged. Only the canonical - ``error.detail`` receipt fields are allowed; malformed, oversized, or - unexpected envelopes fail closed to no telemetry. + ``error.code`` and allowlisted ``error.detail`` fields are allowed. + Malformed, oversized, or unexpected envelopes fail closed to no telemetry. """ try: raw_bytes = exc.read(MAX_HTTP_ERROR_BODY_BYTES + 1) @@ -1346,10 +1674,16 @@ def _extract_http_error_telemetry(exc: urllib.error.HTTPError) -> dict[str, str return {} telemetry: dict[str, str | int] = {} model = _safe_model_identifier(detail.get("model")) + error_code = _safe_model_identifier(error.get("code")) + failure_kind = _safe_model_identifier(detail.get("failure_kind")) terminal_reason = _safe_model_identifier(detail.get("terminal_reason")) attempts = detail.get("attempts") if model is not None: telemetry["served_model"] = model + if error_code is not None: + telemetry["error_code"] = error_code + if failure_kind is not None: + telemetry["failure_kind"] = failure_kind if terminal_reason is not None: telemetry["terminal_reason"] = terminal_reason if isinstance(attempts, list) and attempts and len(attempts) <= 64: @@ -1383,6 +1717,8 @@ def _format_gateway_error_telemetry(telemetry: dict[str, str | int]) -> str: "upstream_phase", "attempt_number", "upstream_status", + "error_code", + "failure_kind", "terminal_reason", ) return " ".join( @@ -1546,11 +1882,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 +1939,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 +1977,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 +2061,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..91edcf3f0a --- /dev/null +++ b/tests/test_noema_observed_defect_corpus_current_main.py @@ -0,0 +1,312 @@ +"""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) + + +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..918753668e 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -1,6 +1,5 @@ import base64 import hashlib -import http.client import io import json import os @@ -16,6 +15,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 +874,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 +1425,7 @@ def fake_urlopen(request, timeout): "choices": [ { "message": { - "content": json.dumps( + "content": _structured_verdict( { "decision": "approve", "summary": "ok", @@ -1453,7 +1457,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 +1542,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: @@ -1587,34 +1593,76 @@ def test_allowed_locations_json_truncates_at_the_byte_budget(): assert 0 < len(envelope["locations"]) < len(locations) -def test_call_llm_reports_only_safe_model_from_bounded_http_error(monkeypatch, capsys): - """A gateway HTTP error exposes only its canonical safe model identifier.""" +@pytest.mark.parametrize( + ("receipt_field", "with_attempt_details"), + [("failure_kind", True), ("error_code", True), ("error_code", False)], +) +@pytest.mark.parametrize( + ("field_value", "expected_value"), + [ + pytest.param("structured_output_exhausted", "structured_output_exhausted", id="canonical"), + pytest.param("invalid_structured_output", "invalid_structured_output", id="protected-code"), + pytest.param(" upstream_error ", "upstream_error", id="trimmed"), + pytest.param("x" * 200, "x" * 200, id="maximum-length"), + pytest.param("x" * 201, None, id="overlength"), + pytest.param(None, None, id="null"), + pytest.param(True, None, id="boolean"), + pytest.param(1, None, id="integer"), + pytest.param({}, None, id="object"), + pytest.param([], None, id="array"), + pytest.param("", None, id="empty"), + pytest.param(" ", None, id="whitespace"), + pytest.param("upstream\n::error::injected", None, id="newline-command"), + pytest.param("upstream\rerror", None, id="carriage-return"), + pytest.param("upstream\x1b[31m", None, id="terminal-escape"), + pytest.param("upstream\ud800", None, id="surrogate"), + pytest.param("upstream=secret", None, id="field-injection"), + ], +) +def test_call_llm_reports_only_safe_model_from_bounded_http_error( + monkeypatch, capsys, receipt_field, with_attempt_details, field_value, expected_value +): + """A failed gateway call emits only bounded scalar receipt fields on both surfaces.""" monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example.test/chat") monkeypatch.setenv("NOEMA_LLM_API_KEY", "secret") secret = "never-print-this-error-detail" - body = json.dumps( - { - "error": { - "detail": { - "model": "github_models/deepseek-v3", - "terminal_reason": "eligible_candidates_exhausted", - "attempts": [{ - "provider_name": "nvidia_nim", - "phase": "connecting", - "attempt_number": 2, - "provider_status": 503, - "secret": secret, - }], + payload = { + "error": { + "detail": { + "model": "github_models/deepseek-v3", + "terminal_reason": "eligible_candidates_exhausted", + "attempts": [{ + "provider_name": "nvidia_nim", + "phase": "connecting", + "attempt_number": 2, + "provider_status": 503, "secret": secret, - }, - "message": secret, + }], + "secret": secret, }, - "arbitrary": secret, - } - ).encode() + "message": secret, + }, + "arbitrary": secret, + } + if not with_attempt_details: + # Mirror _send_error/_error_payload: no model, attempts, or failure kind. + payload["error"]["detail"] = {"request_id": secret} + payload.update( + error_code=field_value, + error_message=secret, + error_detail=payload["error"]["detail"], + ) + if receipt_field == "error_code": + # Current protected gateway errors have a code even without a failure kind. + payload["error"]["code"] = field_value + else: + payload["error"]["detail"]["failure_kind"] = field_value + body = json.dumps(payload).encode() + requests_seen = [] class Opener: def open(self, request): + requests_seen.append(request) raise noema.urllib.error.HTTPError( request.full_url, 502, "Bad Gateway", {}, io.BytesIO(body) ) @@ -1626,19 +1674,103 @@ def open(self, request): output = capsys.readouterr().out diagnostic = str(exc_info.value) + assert len(requests_seen) == 1 assert "phase=response_error" in output - assert "served_model=github_models/deepseek-v3" in output + serving_model = "github_models/deepseek-v3" if with_attempt_details else "unknown" + assert f"served_model={serving_model}" in output assert "phase=response_error" in diagnostic - assert "served_model=github_models/deepseek-v3" in diagnostic - assert "provider_name=nvidia_nim" in output - assert "upstream_phase=connecting" in output - assert "attempt_number=2" in output - assert "upstream_status=503" in output - assert "terminal_reason=eligible_candidates_exhausted" in output + assert f"served_model={serving_model}" in diagnostic + for receipt_fragment in ( + "provider_name=nvidia_nim", + "upstream_phase=connecting", + "attempt_number=2", + "upstream_status=503", + "terminal_reason=eligible_candidates_exhausted", + ): + assert (receipt_fragment in output) is with_attempt_details + assert (receipt_fragment in diagnostic) is with_attempt_details + if expected_value is None: + assert f"{receipt_field}=" not in output + assert f"{receipt_field}=" not in diagnostic + else: + assert f"{receipt_field}={expected_value}" in output + assert f"{receipt_field}={expected_value}" in diagnostic + absent_field = "failure_kind" if receipt_field == "error_code" else "error_code" + assert f"{absent_field}=" not in output + assert f"{absent_field}=" not in diagnostic + assert output.count("::warning::") == 1 + assert "::error::injected" not in output + assert "::error::injected" not in diagnostic assert secret not in output 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 +1875,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 +1884,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 +2194,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 +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()) @@ -2149,7 +2294,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 +2317,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": [], + } } ) }