diff --git a/reviewer/noema_reviewer/__init__.py b/reviewer/noema_reviewer/__init__.py index 02e6bb78f..3aa572663 100644 --- a/reviewer/noema_reviewer/__init__.py +++ b/reviewer/noema_reviewer/__init__.py @@ -12,7 +12,7 @@ from .agent import PydanticAIReviewAgent, ReviewAgent, build_agent from .manifest import ReviewManifest -from .models import Confidence, Finding, ReviewVerdict, Severity, Verdict +from .models import Finding, ReviewVerdict, Severity, Verdict from .patch_image_validation import ( DockerPatchValidatorImageRunner, PatchValidatorImageProfile, @@ -32,7 +32,6 @@ __all__ = [ - "Confidence", "DockerPatchValidationRunner", "DockerPatchValidatorImageRunner", "Finding", diff --git a/reviewer/noema_reviewer/agent.py b/reviewer/noema_reviewer/agent.py index dc7d24b7a..7df4e1544 100644 --- a/reviewer/noema_reviewer/agent.py +++ b/reviewer/noema_reviewer/agent.py @@ -27,12 +27,12 @@ "pull request: its diff, changed-file context, workflow logs, SARIF " "summary, dependency findings, prior review comments, and current check " "conclusions. Judge correctness, security, maintainability, and behavioral " - "regressions from that evidence only. Approve when no blocking issue is " + "regressions from that evidence only. Approve only when no unresolved finding is " "supported by the evidence. Use request_changes only for concrete, " "evidence-backed blocking issues, and cite the log, SARIF, test, or source " "line for each finding. Use blocked when required evidence is missing rather " - "than guessing. Never approve while an unresolved MEDIUM-or-higher " - "dependency finding is present; require a package bump instead." + "than guessing. Never approve while any unresolved dependency or scanner " + "finding is present; require remediation evidence instead." ) @@ -101,13 +101,15 @@ def build_prompt(manifest: ReviewManifest) -> str: class PydanticAIReviewAgent: """A ``ReviewAgent`` backed by a PydanticAI ``Agent`` with a typed verdict.""" - def __init__(self, model: Model | str) -> None: - """Build the agent around an injected model (a real model or a test model).""" + def __init__(self, model: Model | str, *, zdr_only: bool = False) -> None: + """Build the agent without local retries and with exact request privacy policy.""" + model_settings = {"extra_body": {"zdr_only": True}} if zdr_only else None self._agent: Agent[None, ReviewVerdict] = Agent( model, output_type=ReviewVerdict, system_prompt=SYSTEM_PROMPT, - retries=3, + retries=0, + model_settings=model_settings, ) def review(self, manifest: ReviewManifest, *, strict: bool = False) -> ReviewVerdict: @@ -125,5 +127,8 @@ def build_agent(config: ReviewerConfig | None = None) -> PydanticAIReviewAgent: fails loudly when the model provider or credential is unavailable — the reviewer never degrades to a silent approval. """ - model = resolve_model(config) - return PydanticAIReviewAgent(model) + from .config import resolve_config + + resolved = config or resolve_config() + model = resolve_model(resolved) + return PydanticAIReviewAgent(model, zdr_only=resolved.zdr_only) diff --git a/reviewer/noema_reviewer/config.py b/reviewer/noema_reviewer/config.py index d3d6861f6..50137d2bf 100644 --- a/reviewer/noema_reviewer/config.py +++ b/reviewer/noema_reviewer/config.py @@ -1,16 +1,11 @@ -"""Reviewer model/credential resolution. - -Per the repo ``AGENTS.md`` rule, secrets are not read ad hoc from the process -environment at runtime: they come from a KV / credential registry. This module -centralises that read into one place. A ``credential_getter`` (the KV) is the -source of truth; the process environment is only the bootstrap *transport* the -CI step uses to hand secrets to the KV, so the env fallback is explicit and -documented rather than scattered ``os.getenv`` reads. - -The reviewer talks to an OpenAI-compatible endpoint (the -``contextual-orchestrator`` gateway in production). Upstream model selection -stays in that gateway; leftover sequential ``NOEMA_FALLBACK_*`` settings fail -closed instead of trying the next model inside Noema. +"""Reviewer model/credential resolution for the exact orchestrator/free pool. + +Secrets are resolved from the credential registry with environment variables as +bootstrap transport only. Noema does not allocate provider retries, wall-clock +model attempt budgets, candidate fallbacks, or a second routing policy. Those +choices belong to ContextualWisdomLab/contextual-orchestrator. Private-target +privacy is a trusted workflow-derived boolean and is forwarded as request-level +``zdr_only`` evidence rather than inferred from model/provider names. """ from __future__ import annotations @@ -25,17 +20,21 @@ CredentialGetter = Callable[[str], str | None] _LOOPBACK_MODEL_HOSTS = frozenset({"localhost", "127.0.0.1", "::1"}) +_FREE_ROUTING_ALIAS = "orchestrator/free" +_LEGACY_ATTEMPT_CONTROLS = ( + "NOEMA_LLM_REQUEST_TIMEOUT_SECONDS", + "NOEMA_LLM_MAX_RETRIES", +) @dataclass(frozen=True) class ReviewerConfig: - """Resolved settings for a production review agent.""" + """Resolved settings for one production review request.""" model_name: str base_url: str api_key: str - request_timeout_seconds: float = 5400.0 - max_retries: int = 1 + zdr_only: bool = False def _read(name: str, credential_getter: CredentialGetter | None) -> str: @@ -47,39 +46,32 @@ def _read(name: str, credential_getter: CredentialGetter | None) -> str: return (os.environ.get(name) or "").strip() -def _bounded_int( - name: str, - default: int, - minimum: int, - maximum: int, - credential_getter: CredentialGetter | None, -) -> int: - """Read a bounded integer setting and fail with a non-secret reason.""" - raw = _read(name, credential_getter) - if not raw: - return default - try: - value = int(raw) - except ValueError as exc: - raise RuntimeError(f"{name} must be an integer") from exc - if not minimum <= value <= maximum: - raise RuntimeError(f"{name} must be between {minimum} and {maximum}") - return value +def _read_zdr_policy(credential_getter: CredentialGetter | None) -> bool: + """Parse the trusted workflow-derived request privacy policy exactly.""" + raw = _read("NOEMA_LLM_ZDR_ONLY", credential_getter) + if raw in ("", "false"): + return False + if raw == "true": + return True + raise RuntimeError("NOEMA_LLM_ZDR_ONLY must be exactly true or false") -def _require_single_routing_alias(name: str, value: str) -> None: - """Reject sequential candidate lists and direct-provider model prefixes.""" - if any(character.isspace() for character in value) or "," in value: +def _reject_legacy_attempt_controls(credential_getter: CredentialGetter | None) -> None: + """Fail closed if Noema-local model timeout/retry allocation is reintroduced.""" + configured = [name for name in _LEGACY_ATTEMPT_CONTROLS if _read(name, credential_getter)] + if configured: raise RuntimeError( - f"{name} must be one routing alias; sequential model candidates are not allowed" - ) - if value.startswith(("nvidia-nim/", "openai/", "github-models/")): - raise RuntimeError( - f"{name} must be the contextual-orchestrator routing alias, " - "not a direct provider model" + ", ".join(configured) + + " is not allowed; model attempt allocation belongs to contextual-orchestrator" ) +def _require_single_routing_alias(name: str, value: str) -> None: + """Require the exact governed free-pool alias and reject local routing choices.""" + if value != _FREE_ROUTING_ALIAS: + raise RuntimeError(f"{name} must equal {_FREE_ROUTING_ALIAS}") + + def _require_safe_model_endpoint(name: str, value: str) -> None: """Reject credential-bearing model endpoints that use unsafe remote transport.""" try: @@ -95,20 +87,12 @@ def _require_safe_model_endpoint(name: str, value: str) -> None: def resolve_config(credential_getter: CredentialGetter | None = None) -> ReviewerConfig: - """Resolve reviewer configuration from the KV getter or env transport. - - Raises: - RuntimeError: when the model name, base URL, or API key is not - configured, so a misconfiguration fails loudly instead of letting - the reviewer silently skip its verdict. - """ + """Resolve the exact free-pool reviewer configuration and fail closed on drift.""" model_name = _read("NOEMA_LLM_MODEL", credential_getter) base_url = _read("NOEMA_LLM_API_URL", credential_getter) api_key = _read("NOEMA_LLM_API_KEY", credential_getter) - request_timeout_seconds = _bounded_int( - "NOEMA_LLM_REQUEST_TIMEOUT_SECONDS", 5400, 60, 7200, credential_getter - ) - max_retries = _bounded_int("NOEMA_LLM_MAX_RETRIES", 1, 0, 8, credential_getter) + _reject_legacy_attempt_controls(credential_getter) + zdr_only = _read_zdr_policy(credential_getter) leftover_fallback = [ name for name in ( @@ -137,7 +121,7 @@ def resolve_config(credential_getter: CredentialGetter | None = None) -> Reviewe raise RuntimeError( "Noema sequential model fallback is not allowed; unset " + ", ".join(leftover_fallback) - + ". contextual-orchestrator selects min-cost / max-performance." + + ". contextual-orchestrator owns routing." ) _require_single_routing_alias("NOEMA_LLM_MODEL", model_name) _require_safe_model_endpoint("NOEMA_LLM_API_URL", base_url) @@ -145,18 +129,12 @@ def resolve_config(credential_getter: CredentialGetter | None = None) -> Reviewe model_name=model_name, base_url=base_url, api_key=api_key, - request_timeout_seconds=float(request_timeout_seconds), - max_retries=max_retries, + zdr_only=zdr_only, ) def resolve_model(config: ReviewerConfig | None = None) -> Model: - """Build an OpenAI-compatible PydanticAI model from resolved configuration. - - The reviewer routes every model call through an OpenAI-compatible endpoint - (the ``contextual-orchestrator`` gateway in production), so the OpenAI - provider is a required dependency rather than an optional extra. - """ + """Build one OpenAI-compatible gateway model with no Noema-local retries/time budget.""" from openai import AsyncOpenAI from pydantic_ai.models.openai import OpenAIChatModel from pydantic_ai.providers.openai import OpenAIProvider @@ -168,8 +146,8 @@ def resolve_model(config: ReviewerConfig | None = None) -> Model: client = AsyncOpenAI( base_url=resolved.base_url, api_key=resolved.api_key, - timeout=resolved.request_timeout_seconds, - max_retries=resolved.max_retries, + timeout=None, + max_retries=0, ) return OpenAIChatModel( resolved.model_name, diff --git a/reviewer/noema_reviewer/gating.py b/reviewer/noema_reviewer/gating.py index 59f0b750e..d3c202888 100644 --- a/reviewer/noema_reviewer/gating.py +++ b/reviewer/noema_reviewer/gating.py @@ -1,37 +1,17 @@ -"""Deterministic safety gates applied around the LLM review. +"""Fail-closed evidence gates applied around the LLM review. -The LLM driver produces a judgement, but two guarantees from the sandbox plan's -Acceptance Criteria must hold regardless of what the model says, so they are -enforced here in plain, testable code rather than trusted to the prompt: - -1. Manual **strict** runs fail (``blocked``) when required evidence is missing, - naming exactly what was missing — never a silent pass. -2. An unresolved MEDIUM-or-higher dependency finding can never ride out on an - ``approve``; it is downgraded to ``request_changes`` with the finding - attached, because the org rule is "remediate by bump, not gate weakening". +Severity remains source metadata. Noema does not invent a severity threshold: +any unresolved current-head dependency, scanner, check, or review-thread finding +prevents approval. Missing strict-mode evidence remains a blocked outcome. """ from __future__ import annotations from .manifest import ReviewManifest -from .models import ( - BLOCKING_SEVERITIES, - Confidence, - Finding, - ReviewVerdict, - Severity, - Verdict, -) - - -# Noema is an independent reviewer. Treating the primary OpenCode review check -# as a deterministic finding would make each reviewer wait on the other and -# deadlock the two-reviewer rule. The metadata-only gate is also downstream of -# review evidence, so it cannot be used as evidence against an independent -# review. Every other observed current-head check must be terminal-success. -REVIEW_DEPENDENT_CHECK_NAMES = frozenset( - {"opencode-review", "metadata-only gate evaluation"} -) +from .models import Finding, ReviewVerdict, Severity, Verdict + + +REVIEW_DEPENDENT_CHECK_NAMES = frozenset({"opencode-review", "metadata-only gate evaluation"}) def missing_evidence(manifest: ReviewManifest) -> list[str]: @@ -47,9 +27,6 @@ def missing_evidence(manifest: ReviewManifest) -> list[str]: reasons.append("missing current GitHub check conclusions") codegraph_status = manifest.codegraph_status.strip() if not codegraph_status: - # A blank/whitespace status is not evidence; treat it as missing so a - # malformed artifact cannot pass strict mode silently (mirrors the diff - # check above and the field's own "not supplied" default semantics). reasons.append("missing CodeGraph evidence") elif codegraph_status.lower().startswith("unavailable"): reasons.append(manifest.codegraph_status) @@ -58,22 +35,20 @@ def missing_evidence(manifest: ReviewManifest) -> list[str]: def blocked_verdict(reasons: list[str]) -> ReviewVerdict: - """Build a ``blocked`` verdict that names every missing input.""" + """Build a blocked verdict that names every missing input.""" return ReviewVerdict( verdict=Verdict.BLOCKED, - summary=( - "Noema could not reach a decision because required review evidence " - "was missing; see blocked_reasons." - ), + summary="Noema could not reach a decision because required review evidence was missing; see blocked_reasons.", blocked_reasons=reasons, - confidence=Confidence.HIGH, ) def dependency_findings_as_review(manifest: ReviewManifest) -> list[Finding]: - """Convert unresolved blocking dependency findings into review findings.""" + """Convert every unresolved dependency finding into a review finding.""" findings: list[Finding] = [] - for dependency in manifest.unresolved_dependency_findings(BLOCKING_SEVERITIES): + for dependency in manifest.dependency_findings: + if dependency.resolved: + continue fixed = dependency.fixed_version or "a non-vulnerable release" identifier = f" ({dependency.identifier})" if dependency.identifier else "" findings.append( @@ -91,24 +66,20 @@ def dependency_findings_as_review(manifest: ReviewManifest) -> list[Finding]: def security_findings_as_review(manifest: ReviewManifest) -> list[Finding]: - """Convert current-head MEDIUM+ SARIF findings into review findings.""" - findings: list[Finding] = [] - for security in manifest.security_findings: - if security.severity not in BLOCKING_SEVERITIES: - continue - findings.append( - Finding( - severity=security.severity, - path=security.path or ".github/code-scanning", - line=security.line, - evidence=( - f"{security.tool} reported {security.identifier}: {security.message}" - + (f" ({security.url})" if security.url else "") - ), - recommendation="Remediate the current-head scanner finding and rerun code scanning.", - ) + """Convert every current-head scanner finding into a review finding.""" + return [ + Finding( + severity=security.severity, + path=security.path or ".github/code-scanning", + line=security.line, + evidence=( + f"{security.tool} reported {security.identifier}: {security.message}" + + (f" ({security.url})" if security.url else "") + ), + recommendation="Remediate the current-head scanner finding and rerun code scanning.", ) - return findings + for security in manifest.security_findings + ] def failed_checks_as_review(manifest: ReviewManifest) -> list[Finding]: @@ -121,8 +92,7 @@ def failed_checks_as_review(manifest: ReviewManifest) -> list[Finding]: recommendation="Require terminal success for the current-head check before approval.", ) for check in manifest.check_conclusions - if check.name not in REVIEW_DEPENDENT_CHECK_NAMES - and check.conclusion.lower() != "success" + if check.name not in REVIEW_DEPENDENT_CHECK_NAMES and check.conclusion.lower() != "success" ] @@ -141,11 +111,7 @@ def unresolved_threads_as_review(manifest: ReviewManifest) -> list[Finding]: ] -def _enforce_findings( - verdict: ReviewVerdict, - findings: list[Finding], - summary_prefix: str, -) -> ReviewVerdict: +def _enforce_findings(verdict: ReviewVerdict, findings: list[Finding], summary_prefix: str) -> ReviewVerdict: """Merge deterministic findings and prevent an approval from hiding them.""" if not findings or verdict.verdict is Verdict.BLOCKED: return verdict @@ -157,62 +123,32 @@ def _enforce_findings( summary = verdict.summary if verdict.verdict is Verdict.APPROVE: summary = summary_prefix + summary - return verdict.model_copy( - update={ - "verdict": Verdict.REQUEST_CHANGES, - "findings": merged, - "summary": summary, - } - ) + return verdict.model_copy(update={"verdict": Verdict.REQUEST_CHANGES, "findings": merged, "summary": summary}) -def enforce_security_and_check_gates( - manifest: ReviewManifest, - verdict: ReviewVerdict, -) -> ReviewVerdict: - """Block approvals on current-head non-success checks or MEDIUM+ SARIF findings.""" - deterministic = ( - failed_checks_as_review(manifest) - + security_findings_as_review(manifest) - + unresolved_threads_as_review(manifest) - ) +def enforce_security_and_check_gates(manifest: ReviewManifest, verdict: ReviewVerdict) -> ReviewVerdict: + """Prevent approval while any current-head check/scanner/thread finding remains.""" + deterministic = failed_checks_as_review(manifest) + security_findings_as_review(manifest) + unresolved_threads_as_review(manifest) return _enforce_findings( verdict, deterministic, - "Downgraded to request_changes: current-head checks or MEDIUM-or-higher " - "code-scanning findings require remediation. ", + "Downgraded to request_changes: current-head checks, scanner findings, or unresolved threads require remediation. ", ) -def enforce_dependency_gate( - manifest: ReviewManifest, - verdict: ReviewVerdict, -) -> ReviewVerdict: - """Downgrade an approval that ignores unresolved MEDIUM+ dependency findings.""" - dependency_findings = dependency_findings_as_review(manifest) +def enforce_dependency_gate(manifest: ReviewManifest, verdict: ReviewVerdict) -> ReviewVerdict: + """Prevent approval while any unresolved dependency finding remains.""" return _enforce_findings( verdict, - dependency_findings, - "Downgraded to request_changes: unresolved MEDIUM-or-higher dependency " - "finding(s) must be remediated by package bump before approval. ", + dependency_findings_as_review(manifest), + "Downgraded to request_changes: unresolved dependency finding(s) require remediation. ", ) -def apply_gates( - manifest: ReviewManifest, - verdict: ReviewVerdict, - *, - strict: bool, -) -> ReviewVerdict: - """Apply the evidence and dependency gates to a driver's raw verdict. - - In strict mode, missing evidence short-circuits to a ``blocked`` verdict. - The dependency gate always runs so an approval can never bury an unresolved - MEDIUM-or-higher vulnerability. - """ +def apply_gates(manifest: ReviewManifest, verdict: ReviewVerdict, *, strict: bool) -> ReviewVerdict: + """Apply fail-closed evidence, scanner/check/thread, and dependency gates.""" if strict: reasons = missing_evidence(manifest) if reasons: return blocked_verdict(reasons) - check_gated = enforce_security_and_check_gates(manifest, verdict) - return enforce_dependency_gate(manifest, check_gated) + return enforce_dependency_gate(manifest, enforce_security_and_check_gates(manifest, verdict)) diff --git a/reviewer/noema_reviewer/models.py b/reviewer/noema_reviewer/models.py index 3962b9807..eeec74abc 100644 --- a/reviewer/noema_reviewer/models.py +++ b/reviewer/noema_reviewer/models.py @@ -1,11 +1,4 @@ -"""Structured review-verdict schema for the Noema second reviewer. - -The shapes here are the wire contract documented in -``docs/noema-agent-sandbox-plan.md`` ("The driver returns JSON"). Keeping them -as Pydantic models lets the PydanticAI agent emit a validated object directly -and lets every consumer (the central ``.github`` review gate, tests, and any -future sandbox plane) share one source of truth. -""" +"""Structured review-verdict schema for the Noema second reviewer.""" from __future__ import annotations @@ -23,7 +16,7 @@ class Verdict(str, Enum): class Severity(str, Enum): - """Finding severity ordered from most to least serious.""" + """Scanner/reviewer severity metadata, never a local admission cutoff.""" CRITICAL = "critical" HIGH = "high" @@ -32,39 +25,14 @@ class Severity(str, Enum): INFO = "info" -class Confidence(str, Enum): - """Calibrated confidence the reviewer attaches to its verdict.""" - - HIGH = "high" - MEDIUM = "medium" - LOW = "low" - - -# Severities at or above which an unresolved dependency finding must block an -# approval (the org rule: remediate MEDIUM-or-higher by bump, never by gate -# weakening). Ordered worst-first for deterministic comparisons. -BLOCKING_SEVERITIES: tuple[Severity, ...] = ( - Severity.CRITICAL, - Severity.HIGH, - Severity.MEDIUM, -) - - class Finding(BaseModel): """A single reviewer-facing issue tied to concrete evidence.""" - severity: Severity = Field(description="How serious the issue is.") + severity: Severity = Field(description="Source-provided severity metadata.") path: str = Field(description="Repository-relative path the issue lives in.") - line: int | None = Field( - default=None, - description="1-indexed line the issue anchors to, when known.", - ) - evidence: str = Field( - description="Log, SARIF, test, or source reference proving the issue is real.", - ) - recommendation: str = Field( - description="The specific fix the author should apply.", - ) + line: int | None = Field(default=None, description="1-indexed line when known.") + evidence: str = Field(description="Evidence proving the issue is real.") + recommendation: str = Field(description="The specific remediation to apply.") class ReviewVerdict(BaseModel): @@ -72,32 +40,19 @@ class ReviewVerdict(BaseModel): verdict: Verdict = Field(description="The terminal outcome of the review.") summary: str = Field(description="Short reviewer-facing summary.") - findings: list[Finding] = Field( - default_factory=list, - description="Concrete, evidence-backed findings.", - ) - suggested_patch_ref: str | None = Field( - default=None, - description="Optional artifact path or branch holding a suggested patch.", - ) - blocked_reasons: list[str] = Field( - default_factory=list, - description="Missing required log/SARIF/review context that blocked a decision.", - ) - confidence: Confidence = Field( - default=Confidence.MEDIUM, - description="Calibrated confidence in the verdict.", - ) + findings: list[Finding] = Field(default_factory=list) + suggested_patch_ref: str | None = Field(default=None) + blocked_reasons: list[str] = Field(default_factory=list) @model_validator(mode="after") def validate_approval_invariants(self) -> "ReviewVerdict": - """Reject approval states that still contain deterministic blockers.""" + """Reject approval states that retain any unresolved finding or evidence gap.""" if self.verdict is not Verdict.APPROVE: return self if self.blocked_reasons: raise ValueError("approval verdict cannot contain blocked reasons") - if any(finding.severity in BLOCKING_SEVERITIES for finding in self.findings): - raise ValueError("approval verdict cannot contain blocking findings") + if self.findings: + raise ValueError("approval verdict cannot contain findings") return self def is_approval(self) -> bool: diff --git a/reviewer/tests/test_no_heuristic_review_contract.py b/reviewer/tests/test_no_heuristic_review_contract.py new file mode 100644 index 000000000..b972c9376 --- /dev/null +++ b/reviewer/tests/test_no_heuristic_review_contract.py @@ -0,0 +1,127 @@ +"""Executable no-heuristics contracts for Noema review execution.""" + +from __future__ import annotations + +import pytest + +from noema_reviewer.config import ReviewerConfig, resolve_config +from noema_reviewer.gating import ( + dependency_findings_as_review, + security_findings_as_review, +) +from noema_reviewer.manifest import DependencyFinding, ReviewManifest, SecurityFinding +from noema_reviewer.models import Finding, ReviewVerdict, Severity, Verdict + + +def _kv(values: dict[str, str]): + """Build the credential getter used by production configuration tests.""" + return lambda name: values.get(name) + + +def _base_values(**extra: str) -> dict[str, str]: + """Return a complete exact free-pool gateway configuration.""" + values = { + "NOEMA_LLM_MODEL": "orchestrator/free", + "NOEMA_LLM_API_URL": "https://orchestrator.example/v1", + "NOEMA_LLM_API_KEY": "gateway-token", + } + values.update(extra) + return values + + +def test_exact_free_pool_and_private_zdr_are_first_class_configuration() -> None: + """The reviewer preserves exact free-pool identity and trusted ZDR policy.""" + config = resolve_config(_kv(_base_values(NOEMA_LLM_ZDR_ONLY="true"))) + assert config == ReviewerConfig( + model_name="orchestrator/free", + base_url="https://orchestrator.example/v1", + api_key="gateway-token", + zdr_only=True, + ) + + +def test_generic_orchestrator_alias_is_not_an_accepted_production_pool() -> None: + """A generic gateway alias cannot silently select a non-free pool.""" + with pytest.raises(RuntimeError, match="orchestrator/free"): + resolve_config( + _kv( + { + "NOEMA_LLM_MODEL": "contextual-orchestrator", + "NOEMA_LLM_API_URL": "https://orchestrator.example/v1", + "NOEMA_LLM_API_KEY": "gateway-token", + } + ) + ) + + +@pytest.mark.parametrize( + "legacy_control", + ("NOEMA_LLM_REQUEST_TIMEOUT_SECONDS", "NOEMA_LLM_MAX_RETRIES"), +) +def test_repository_authored_model_attempt_controls_fail_closed(legacy_control: str) -> None: + """Noema cannot reintroduce hand-selected model timeout/retry allocation.""" + with pytest.raises(RuntimeError, match=legacy_control): + resolve_config(_kv(_base_values(**{legacy_control: "1"}))) + + +def test_approval_cannot_carry_any_unresolved_finding_regardless_of_label() -> None: + """Severity labels remain evidence metadata, never a local admission threshold.""" + with pytest.raises(ValueError, match="approval verdict cannot contain findings"): + ReviewVerdict( + verdict=Verdict.APPROVE, + summary="not admissible", + findings=[ + Finding( + severity=Severity.LOW, + path="src/a.py", + evidence="scanner finding", + recommendation="remediate the finding", + ) + ], + ) + + +def test_all_unresolved_dependency_findings_are_review_findings() -> None: + """No local MEDIUM cutoff may hide a lower-labelled unresolved dependency finding.""" + manifest = ReviewManifest( + repo="ContextualWisdomLab/example", + pr_number=1, + dependency_findings=[ + DependencyFinding( + tool="scanner", + identifier="CVE-example", + package_name="example-package", + installed_version="1.0.0", + fixed_version="1.0.1", + severity=Severity.LOW, + resolved=False, + ) + ], + ) + findings = dependency_findings_as_review(manifest) + assert [finding.path for finding in findings] == ["example-package"] + + +def test_all_current_security_findings_are_review_findings() -> None: + """No local MEDIUM cutoff may suppress a current scanner finding.""" + manifest = ReviewManifest( + repo="ContextualWisdomLab/example", + pr_number=1, + security_findings=[ + SecurityFinding( + tool="scanner", + identifier="rule-example", + severity=Severity.LOW, + message="observed issue", + path="src/a.py", + ) + ], + ) + findings = security_findings_as_review(manifest) + assert [finding.path for finding in findings] == ["src/a.py"] + + +def test_review_verdict_has_no_uncalibrated_confidence_field() -> None: + """Noema does not publish categorical uncertainty without a calibration model.""" + verdict = ReviewVerdict(verdict=Verdict.BLOCKED, summary="missing evidence") + assert "confidence" not in verdict.model_dump() diff --git a/test/no-heuristic-orchestrator-free-workflows.test.ts b/test/no-heuristic-orchestrator-free-workflows.test.ts new file mode 100644 index 000000000..c5e0148b2 --- /dev/null +++ b/test/no-heuristic-orchestrator-free-workflows.test.ts @@ -0,0 +1,80 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +function source(path: string): string { + return readFileSync(path, "utf8"); +} + +function jobSlice(workflow: string, job: string, nextJob?: string): string { + const start = workflow.indexOf(` ${job}:`); + if (start < 0) throw new Error(`missing workflow job ${job}`); + if (!nextJob) return workflow.slice(start); + const end = workflow.indexOf(` ${nextJob}:`, start + 1); + if (end < 0) throw new Error(`missing workflow job ${nextJob}`); + return workflow.slice(start, end); +} + +describe("Noema no-heuristics orchestrator contract", () => { + it("pins every GitHub Actions LLM path to orchestrator/free", () => { + const gateway = source("scripts/lib/orchestrator-gateway.mjs"); + const review = source(".github/workflows/central-review.yml"); + const hourly = source(".github/workflows/hourly-product-development.yml"); + + expect(gateway).toContain('const DEFAULT_ROUTING_ALIAS = "orchestrator/free";'); + expect(review).toContain("NOEMA_LLM_MODEL: orchestrator/free"); + expect(hourly).toContain("NOEMA_LLM_MODEL: orchestrator/free"); + expect(review).not.toContain("NOEMA_LLM_MODEL: ${{ vars.NOEMA_LLM_MODEL }}"); + expect(hourly).not.toContain("NOEMA_LLM_MODEL: ${{ vars.NOEMA_LLM_MODEL }}"); + }); + + it("does not allocate model attempts with repository-authored timeout or retry counts", () => { + const gateway = source("scripts/lib/orchestrator-gateway.mjs"); + const review = source(".github/workflows/central-review.yml"); + const hourly = source(".github/workflows/hourly-product-development.yml"); + const config = source("reviewer/noema_reviewer/config.py"); + const agent = source("reviewer/noema_reviewer/agent.py"); + const publish = jobSlice(review, "publish_review"); + const proposer = jobSlice(hourly, "propose_product_increment", "package_product_increment"); + + expect(gateway).not.toContain("HEALTH_TIMEOUT_MS"); + expect(publish).not.toContain("NOEMA_LLM_REQUEST_TIMEOUT_SECONDS"); + expect(publish).not.toContain("NOEMA_LLM_MAX_RETRIES"); + expect(publish).not.toContain("timeout-minutes:"); + expect(proposer).not.toContain("OPENCODE_RUN_TIMEOUT_SECONDS"); + expect(proposer).not.toContain("OPENCODE_KILL_GRACE_SECONDS"); + expect(proposer).not.toContain("timeout --kill-after"); + expect(proposer).not.toContain("timeout-minutes:"); + expect(config).not.toContain("request_timeout_seconds"); + expect(config).not.toContain("NOEMA_LLM_REQUEST_TIMEOUT_SECONDS"); + expect(config).not.toContain("NOEMA_LLM_MAX_RETRIES"); + expect(agent).toContain("retries=0"); + expect(agent).not.toContain("retries=3"); + }); + + it("derives private-target ZDR from live repository visibility and never from a caller override", () => { + const review = source(".github/workflows/central-review.yml"); + const hourly = source(".github/workflows/hourly-product-development.yml"); + const agent = source("reviewer/noema_reviewer/agent.py"); + + expect(review).toContain('gh api "repos/${TARGET_REPOSITORY}" --jq .visibility'); + expect(review).toContain("NOEMA_LLM_ZDR_ONLY=true"); + expect(hourly).toContain('gh api "repos/${GITHUB_REPOSITORY}" --jq .visibility'); + expect(hourly).toContain("NOEMA_LLM_ZDR_ONLY=true"); + expect(review).not.toContain("vars.NOEMA_LLM_ZDR_ONLY"); + expect(hourly).not.toContain("vars.NOEMA_LLM_ZDR_ONLY"); + expect(agent).toContain('"extra_body": {"zdr_only": True}'); + }); + + it("does not use a locally selected severity cutoff or uncalibrated confidence estimate as review authority", () => { + const models = source("reviewer/noema_reviewer/models.py"); + const gating = source("reviewer/noema_reviewer/gating.py"); + const review = source(".github/workflows/central-review.yml"); + + expect(models).not.toContain("BLOCKING_SEVERITIES"); + expect(models).not.toContain("class Confidence"); + expect(models).not.toContain("confidence:"); + expect(gating).not.toContain("MEDIUM-or-higher"); + expect(review).not.toContain("--severity MEDIUM,HIGH,CRITICAL"); + expect(review).not.toContain("confidence}'"); + }); +}); diff --git a/test/no-temporary-self-modifying-writer.test.ts b/test/no-temporary-self-modifying-writer.test.ts new file mode 100644 index 000000000..769cfa19c --- /dev/null +++ b/test/no-temporary-self-modifying-writer.test.ts @@ -0,0 +1,21 @@ +import { existsSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; + +const repositoryRoot = process.cwd(); + +const temporaryWriterArtifacts = [ + ".github/source-fix-no-heuristic-orchestrator-free.trigger", + ".github/workflows/source-fix-no-heuristic-orchestrator-free.yml", + "scripts/source_fix_no_heuristic_orchestrator_free.py", +] as const; + +describe("Noema writer lease", () => { + it("forbids temporary self-modifying source-fix writers", () => { + const present = temporaryWriterArtifacts.filter((path) => + existsSync(join(repositoryRoot, path)), + ); + + expect(present).toEqual([]); + }); +});