diff --git a/.github/actions/noema-review/two_phase.py b/.github/actions/noema-review/two_phase.py new file mode 100644 index 0000000000..1cab5aa411 --- /dev/null +++ b/.github/actions/noema-review/two_phase.py @@ -0,0 +1,262 @@ +#!/usr/bin/env python3 +"""Prepare and publish Noema verdicts across short-lived reviewer credentials. + +The model phase can legitimately outlive a one-hour GitHub App installation +credential. This trusted helper therefore seals the already validated model +verdict to a runner-local file, then a later workflow step reopens that file +only after the reviewer credential has been refreshed. Publication always +re-fetches the live pull request and verifies its exact head and base before +submitting any review evidence. +""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import stat +import sys +from pathlib import Path +from typing import Any + +ROOT = Path(__file__).resolve().parents[3] +if str(ROOT) not in sys.path: + sys.path.insert(0, str(ROOT)) + +from scripts.ci import noema_review_gate as gate # noqa: E402 + +ENVELOPE_SCHEMA_VERSION = 1 +MAX_ENVELOPE_BYTES = 2 * 1024 * 1024 + + +def _canonical_head(value: str) -> str: + """Return one canonical lowercase Git SHA or fail closed.""" + head = value.strip().lower() + if not re.fullmatch(r"[0-9a-f]{40}", head): + raise RuntimeError("Noema two-phase handoff requires a canonical 40-character Git SHA") + return head + + +def _canonical_base(pull_request: dict[str, Any]) -> str: + """Return the exact base commit that defined the reviewed diff/context.""" + base = str(pull_request.get("baseRefOid") or "").strip().lower() + if not re.fullmatch(r"[0-9a-f]{40}", base): + raise RuntimeError("Noema two-phase handoff requires a canonical 40-character base SHA") + return base + + +def _reviewer_actor() -> str: + """Return a verified independent reviewer actor for the active token.""" + actor = gate.current_actor() + if not actor: + raise RuntimeError("Noema reviewer identity could not be verified") + if actor in gate.PRIMARY_REVIEW_AUTHORS: + raise RuntimeError( + f"Current token actor {actor!r} is already a primary review actor; " + "Noema requires an independent reviewer credential." + ) + return actor + + +def _write_envelope(path: Path, payload: dict[str, Any]) -> None: + """Create one private, non-following runner-local verdict envelope.""" + encoded = (json.dumps(payload, separators=(",", ":"), sort_keys=True) + "\n").encode("utf-8") + if len(encoded) > MAX_ENVELOPE_BYTES: + raise RuntimeError("Noema verdict envelope exceeds the bounded handoff size") + flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL + if hasattr(os, "O_NOFOLLOW"): + flags |= os.O_NOFOLLOW + fd = os.open(path, flags, 0o600) + try: + file_stat = os.fstat(fd) + if not stat.S_ISREG(file_stat.st_mode) or file_stat.st_nlink != 1: + raise RuntimeError("Noema verdict envelope target is not a private regular file") + view = memoryview(encoded) + written = 0 + while written < len(view): + count = os.write(fd, view[written:]) + if count <= 0: + raise RuntimeError("Noema verdict envelope write made no forward progress") + written += count + os.fsync(fd) + except BaseException: + os.close(fd) + path.unlink(missing_ok=True) + raise + else: + os.close(fd) + + +def _read_envelope(path: Path) -> dict[str, Any]: + """Read and validate one sealed runner-local verdict envelope.""" + flags = os.O_RDONLY + if hasattr(os, "O_NOFOLLOW"): + flags |= os.O_NOFOLLOW + try: + fd = os.open(path, flags) + except OSError as exc: + raise RuntimeError("Noema verdict envelope is unavailable for publication") from exc + try: + file_stat = os.fstat(fd) + if not stat.S_ISREG(file_stat.st_mode) or file_stat.st_nlink != 1: + raise RuntimeError("Noema verdict envelope is not a regular single-link file") + if file_stat.st_mode & 0o077: + raise RuntimeError("Noema verdict envelope permissions are broader than owner-only") + if file_stat.st_size <= 0 or file_stat.st_size > MAX_ENVELOPE_BYTES: + raise RuntimeError("Noema verdict envelope size is outside the bounded contract") + chunks: list[bytes] = [] + remaining = MAX_ENVELOPE_BYTES + 1 + while remaining > 0: + chunk = os.read(fd, min(65536, remaining)) + if not chunk: + break + chunks.append(chunk) + remaining -= len(chunk) + raw = b"".join(chunks) + if len(raw) > MAX_ENVELOPE_BYTES: + raise RuntimeError("Noema verdict envelope exceeded the bounded read limit") + finally: + os.close(fd) + try: + payload = json.loads(raw.decode("utf-8")) + except (UnicodeDecodeError, json.JSONDecodeError) as exc: + raise RuntimeError("Noema verdict envelope is malformed") from exc + if not isinstance(payload, dict): + raise RuntimeError("Noema verdict envelope root must be an object") + return payload + + +def prepare_verdict(repo: str, number: int, expected_head: str, path: Path) -> int: + """Run model review and seal its verdict without publishing GitHub evidence.""" + expected = _canonical_head(expected_head) + pull_request = gate.fetch_pr(repo, number) + try: + gate.require_expected_head(pull_request, expected) + except RuntimeError: + print("Pull request is closed or stale; Noema verdict preparation skipped.") + return 0 + expected_base = _canonical_base(pull_request) + actor = _reviewer_actor() + if pull_request.get("isDraft"): + print("PR is draft; Noema verdict preparation skipped.") + return 0 + if gate.existing_noema_review(pull_request, actor): + print("Current head already has a Noema review; verdict preparation skipped.") + return 0 + + diff, truncated = gate.fetch_diff(repo, number) + changed_files = gate.fetch_changed_files(repo, number) + changed_paths = tuple(file_path for file_path, _status in changed_files) + review_context = gate.build_review_context(repo, number, pull_request, changed_files) + try: + verdict = gate.call_llm( + repo, + number, + pull_request, + diff, + truncated, + expected, + review_context, + changed_paths, + ) + except gate.StaleHeadDuringRepairRetryError: + print("Pull request head changed during model repair retry; verdict was not sealed.") + return 0 + + _write_envelope( + path, + { + "schema_version": ENVELOPE_SCHEMA_VERSION, + "repository": repo, + "pull_request_number": number, + "expected_head": expected, + "expected_base": expected_base, + "verdict": verdict, + }, + ) + print( + f"Prepared Noema verdict for {repo}#{number} at head {expected} / base {expected_base}; " + "publication is deferred." + ) + return 0 + + +def publish_verdict(repo: str, number: int, expected_head: str, path: Path) -> int: + """Publish a prepared verdict only with fresh exact-head/base reviewer authority.""" + expected = _canonical_head(expected_head) + try: + payload = _read_envelope(path) + required_keys = { + "schema_version", + "repository", + "pull_request_number", + "expected_head", + "expected_base", + "verdict", + } + if set(payload) != required_keys: + raise RuntimeError("Noema verdict envelope fields do not match the trusted schema") + if payload["schema_version"] != ENVELOPE_SCHEMA_VERSION: + raise RuntimeError("Noema verdict envelope schema version is unsupported") + if payload["repository"] != repo or payload["pull_request_number"] != number: + raise RuntimeError("Noema verdict envelope target identity does not match publication") + if payload["expected_head"] != expected: + raise RuntimeError("Noema verdict envelope head does not match publication") + expected_base = str(payload["expected_base"]).strip().lower() + if not re.fullmatch(r"[0-9a-f]{40}", expected_base): + raise RuntimeError("Noema verdict envelope base does not contain a canonical Git SHA") + verdict = payload["verdict"] + if not isinstance(verdict, dict): + raise RuntimeError("Noema verdict envelope verdict must be an object") + + current_pull_request = gate.fetch_pr(repo, number) + try: + gate.require_expected_head(current_pull_request, expected) + except RuntimeError: + print("Pull request closed or advanced after model review; prepared verdict was not published.") + return 0 + if _canonical_base(current_pull_request) != expected_base: + print("Pull request base advanced after model review; stale prepared verdict was not published.") + return 0 + actor = _reviewer_actor() + if current_pull_request.get("isDraft"): + print("PR became draft after model review; prepared verdict was not published.") + return 0 + if gate.existing_noema_review(current_pull_request, actor): + print("Current head already has a Noema review; duplicate publication skipped.") + return 0 + gate.submit_review(repo, number, current_pull_request, actor, verdict) + return 0 + finally: + path.unlink(missing_ok=True) + + +def parse_args(argv: list[str]) -> argparse.Namespace: + """Parse the trusted two-phase handoff command line.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--repo", required=True) + parser.add_argument("--pr-number", required=True, type=int) + parser.add_argument("--expected-head", required=True) + modes = parser.add_mutually_exclusive_group(required=True) + modes.add_argument("--prepare-verdict-file", type=Path) + modes.add_argument("--publish-verdict-file", type=Path) + return parser.parse_args(argv) + + +def main(argv: list[str]) -> int: + """Execute the selected prepare or publication phase.""" + args = parse_args(argv) + if args.pr_number <= 0: + raise SystemExit("--pr-number must be positive") + if args.prepare_verdict_file is not None: + return prepare_verdict(args.repo, args.pr_number, args.expected_head, args.prepare_verdict_file) + return publish_verdict(args.repo, args.pr_number, args.expected_head, args.publish_verdict_file) + + +if __name__ == "__main__": + try: + raise SystemExit(main(sys.argv[1:])) + except RuntimeError as exc: + print(f"::error::{exc}", file=sys.stderr) + raise SystemExit(1) from exc diff --git a/.github/workflows/noema-review.yml b/.github/workflows/noema-review.yml index 794c94569f..6b2e3fcede 100644 --- a/.github/workflows/noema-review.yml +++ b/.github/workflows/noema-review.yml @@ -552,8 +552,9 @@ jobs: set -euo pipefail bash "$GITHUB_WORKSPACE/scripts/ci/contextual_orchestrator_review_sidecar.sh" - - name: Run Noema LLM review and submit verdict + - name: Prepare Noema model verdict if: env.PR_NUMBER != '' + id: noema_prepare env: GH_TOKEN: ${{ secrets.NOEMA_REVIEW_TOKEN || steps.noema_github_app_token.outputs.token || steps.noema_oidc_token.outputs.token }} NOEMA_REVIEW_TOKEN_SOURCE: ${{ steps.noema_credential.outputs.source == 'pat' && 'noema-review-pat' || steps.noema_credential.outputs.source == 'github-app' && 'noema-review-github-app' || 'noema-review-app-oidc' }} @@ -563,10 +564,11 @@ jobs: set -euo pipefail if [ -z "${PR_NUMBER:-}" ]; then echo "No pull request number was available for this event; skipping." + echo "prepared=false" >>"$GITHUB_OUTPUT" exit 0 fi if [ -z "${GH_TOKEN:-}" ]; then - echo "::error::Noema reviewer credential selection succeeded but no token was minted; review cannot submit a verdict." + echo "::error::Noema reviewer credential selection succeeded but no token was minted; review cannot prepare a verdict." exit 1 fi if [ -z "${CONTEXTUAL_ORCHESTRATOR_BASE_URL:-}" ] || [ -z "${CONTEXTUAL_ORCHESTRATOR_TOKEN_FILE:-}" ]; then @@ -578,7 +580,50 @@ jobs: export NOEMA_LLM_MODEL="orchestrator/free" export NOEMA_LLM_API_KEY="${CONTEXTUAL_ORCHESTRATOR_TOKEN}" export NOEMA_LLM_VIA_ORCHESTRATOR=1 - python3 -m scripts.ci.noema_review_gate \ - --repo "$TARGET_REPOSITORY" \ - --pr-number "$PR_NUMBER" \ - --expected-head "$EXPECTED_HEAD_SHA" + verdict_file="${RUNNER_TEMP}/noema-verdict-envelope.json" + rm -f "$verdict_file" + python3 "$GITHUB_WORKSPACE/.github/actions/noema-review/two_phase.py" --repo "$TARGET_REPOSITORY" --pr-number "$PR_NUMBER" --expected-head "$EXPECTED_HEAD_SHA" --prepare-verdict-file "$verdict_file" + if [ -f "$verdict_file" ]; then + echo "prepared=true" >>"$GITHUB_OUTPUT" + else + echo "prepared=false" >>"$GITHUB_OUTPUT" + echo "::notice::Noema model phase produced no publishable envelope; publication is skipped." + fi + + - name: Refresh repository-scoped Noema GitHub App token for publication + if: env.PR_NUMBER != '' && steps.noema_prepare.outputs.prepared == 'true' && steps.noema_credential.outputs.source == 'github-app' + id: noema_github_app_publication_token + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + with: + client-id: ${{ vars.NOEMA_GITHUB_APP_CLIENT_ID }} + private-key: ${{ secrets.NOEMA_GITHUB_APP_PRIVATE_KEY }} + owner: ContextualWisdomLab + repositories: ${{ steps.noema_credential.outputs.repository }} + permission-actions: read + permission-checks: read + permission-contents: read + permission-metadata: read + permission-pull-requests: write + permission-security-events: read + permission-statuses: read + permission-vulnerability-alerts: read + + - name: Publish prepared Noema verdict on the exact live head + if: env.PR_NUMBER != '' && steps.noema_prepare.outputs.prepared == 'true' + env: + GH_TOKEN: ${{ steps.noema_credential.outputs.source == 'pat' && secrets.NOEMA_REVIEW_TOKEN || steps.noema_credential.outputs.source == 'github-app' && steps.noema_github_app_publication_token.outputs.token || steps.noema_credential.outputs.source == 'oidc' && steps.noema_oidc_token.outputs.token || '' }} + NOEMA_REVIEW_TOKEN_SOURCE: ${{ steps.noema_credential.outputs.source == 'pat' && 'noema-review-pat' || steps.noema_credential.outputs.source == 'github-app' && 'noema-review-github-app-refresh' || steps.noema_credential.outputs.source == 'oidc' && 'noema-review-app-oidc' || '' }} + NOEMA_REVIEW_ACTOR: ${{ steps.noema_github_app_publication_token.outputs['app-slug'] && format('{0}[bot]', steps.noema_github_app_publication_token.outputs['app-slug']) || '' }} + NOEMA_REVIEW_INSTALLATION_ID: ${{ steps.noema_github_app_publication_token.outputs['installation-id'] }} + run: | + set -euo pipefail + if [ -z "${GH_TOKEN:-}" ]; then + echo "::error::Noema publication has no credential for the explicitly selected reviewer source; refusing any GITHUB_TOKEN or author fallback." + exit 1 + fi + verdict_file="${RUNNER_TEMP}/noema-verdict-envelope.json" + if [ ! -f "$verdict_file" ]; then + echo "::error::Noema prepared-verdict output claimed success but its private envelope is missing." + exit 1 + fi + python3 "$GITHUB_WORKSPACE/.github/actions/noema-review/two_phase.py" --repo "$TARGET_REPOSITORY" --pr-number "$PR_NUMBER" --expected-head "$EXPECTED_HEAD_SHA" --publish-verdict-file "$verdict_file" diff --git a/.github/workflows/noema-token-lifetime-quality-ci.yml b/.github/workflows/noema-token-lifetime-quality-ci.yml new file mode 100644 index 0000000000..3de8f18ab3 --- /dev/null +++ b/.github/workflows/noema-token-lifetime-quality-ci.yml @@ -0,0 +1,36 @@ +name: Noema Reviewer Token Lifetime CI + +on: + pull_request: + paths: + - .github/workflows/noema-review.yml + - .github/actions/noema-review/two_phase.py + - tests/test_noema_reviewer_token_lifetime.py + - tests/test_noema_two_phase_handoff.py + - docs/doctoring/noema-review-token-lifetime.md + - docs/product-technical-gap-baseline.md + - CHANGELOG.md + - requirements-opencode-review-ci-hashes.txt + - .github/workflows/noema-token-lifetime-quality-ci.yml + +permissions: + contents: read + +jobs: + noema-reviewer-token-lifetime: + runs-on: ubuntu-24.04 + timeout-minutes: 20 + steps: + - name: Checkout exact source + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + - name: Install pinned review CI dependencies + run: >- + python3 -m pip install --disable-pip-version-check --require-hashes --only-binary=:all: -r requirements-opencode-review-ci-hashes.txt + - name: Verify token-lifetime handoff contracts + run: | + set -euo pipefail + PYTHONPATH=. python3 -m pytest -q tests/test_noema_reviewer_token_lifetime.py tests/test_noema_two_phase_handoff.py + python3 -m compileall -q .github/actions/noema-review/two_phase.py tests/test_noema_reviewer_token_lifetime.py tests/test_noema_two_phase_handoff.py + git diff --check diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f0680a91d..8f980f794d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- **Refresh Noema reviewer App authority after long model work (`#1616`).** A real `naruon#1497` review outlived its repository-scoped GitHub App installation token and failed the next exact-head GitHub operation with HTTP 401. The trusted workflow now prepares the validated verdict into a private runner-local envelope, remints the same least-privilege repository-scoped App authority after model work, independently re-fetches exact live head/reviewer identity, and only then publishes. Skipped preparation creates no envelope, predecessor App tokens cannot authorize publication, PAT/OIDC remain explicit fail-closed sources, malformed handoffs are cleaned up, and executable plus step-scoped regressions cover stale-head, identity, alias, workflow wiring, and migration of legacy broader-suite contracts away from the retired single-process reviewer path. - Fix `existing_noema_review()` treating a "legacy" Noema review (one posted before `NOEMA_REVIEW_FOOTER_MARKER` existed) as proof the current head was already reviewed. `noema_review_handoff.py`'s `noema_review_state()` can never recognize such a review as a diff --git a/docs/doctoring/noema-review-token-lifetime.md b/docs/doctoring/noema-review-token-lifetime.md new file mode 100644 index 0000000000..5346333ee2 --- /dev/null +++ b/docs/doctoring/noema-review-token-lifetime.md @@ -0,0 +1,21 @@ +# Noema reviewer credential lifetime + +## Incident and root cause + +On 2026-09-01, trusted central Noema review for `ContextualWisdomLab/naruon#1497@152d1998c4e8024be9dc7026c8789d343c884fd0` minted the repository-scoped `cwl-noema-review` GitHub App installation token before model work. Contextual-orchestrator review then exceeded the installation-token lifetime; the first later GitHub operation failed HTTP 401 and cleanup independently reported token expiry. Repository-owned deterministic checks on that Naruon head were otherwise green. The defect is in the central reviewer credential lifecycle, not Naruon product code. + +## Closed operating contract + +Noema separates model verdict preparation from GitHub publication. Preparation remains bound to the trigger's canonical exact head and the exact base commit that defined the reviewed diff/context, and stores only a bounded, owner-only, single-link runner-local envelope. If preparation intentionally skips because the PR is stale, draft, or already reviewed, the workflow emits `prepared=false` and performs no publication. + +For the GitHub App path, a second repository-scoped installation token is minted only after model work and only when a publishable envelope exists. Publication never reuses the predecessor App token, never falls back to `github.token` or the PR author, and independently re-fetches the live PR/head/base and reviewer actor before submitting evidence. A base-branch advance with an unchanged PR head invalidates the prepared verdict because the changed-file diff and review context may have changed; such predecessor-base evidence is consumed without publication. PAT and OIDC remain explicit sources: publication uses only the selected source and fails closed if it is absent; this repair does not silently convert those paths to another authority. + +The envelope is deleted after every publication attempt, including malformed-envelope read validation failures. Executable regressions cover preparation-without-publication, exact-head/base/actor rebinding, stale heads, base drift with an unchanged head, draft skip behavior, cleanup, and hard-link alias rejection. Step-scoped workflow regressions prove that the second App mint sits between preparation and publication and that publication references the fresh token. + +## Verification and downstream replay + +Focused CI runs the token-lifetime and two-phase handoff regressions with hash-pinned review dependencies whenever the workflow/helper/contracts change. After protected-main merge, replay unchanged `naruon#1497@152d1998c4e8024be9dc7026c8789d343c884fd0`: Required Noema Review must finish with current-head-and-base schema-valid review evidence or a typed review-unavailable result, never opaque expired-token 401 and never stale-head/base publication. A pre-merge run does not prove the merged workflow-source path and is not promoted to release evidence. + +### Regression-suite migration + +The two-phase migration also updates pre-existing executable workflow contracts to target the `Prepare Noema model verdict` step and the explicit prepare/publish helper invocations. This prevents a green focused gate from coexisting with stale broader-suite expectations for the retired single-process command or step name. diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 6a2bf678d4..7ba1d7cd41 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -2562,3 +2562,17 @@ Zhang, S., Yu, Y., Li, Y., Zhao, W., Yang, Y., Zhang, Y., & Liu, T. (2025). *Con Xu, J., Sun, Q., Schwendeman, P., Nielsen, S., Cetin, E., & Tang, Y. (2026). *TRINITY: An evolved LLM coordinator* [Preprint]. arXiv. https://doi.org/10.48550/arXiv.2512.04695 Higgins, S. S., Crepalde, N., & Fernandes, L. (2021). Segmented multiplexity: A research agenda for multiplexity beyond the average. *PLOS ONE, 16*(9), e0257527. https://doi.org/10.1371/journal.pone.0257527 + + +## Noema reviewer credential-lifetime delta — 2026-09-01 + +**Observed gap.** `ContextualWisdomLab/naruon#1497@152d1998c4e8024be9dc7026c8789d343c884fd0` demonstrated a control-plane latency/authority defect: a repository-scoped `cwl-noema-review` GitHub App token minted before contextual-orchestrator model work expired before the next GitHub operation, producing HTTP 401 even though repository-owned deterministic checks were otherwise successful. This is a central `.github` reviewer-lifecycle gap, not a Naruon product failure. + +**Owner-side closure in #1616.** The Noema workflow now treats model preparation and GitHub publication as separate trust phases. A bounded private envelope carries only the model verdict; the GitHub App path remints the same repository-scoped least-privilege authority after model work, and publication independently verifies repository, PR number, canonical exact head, live PR state, draft state, independent reviewer actor, and duplicate-current-head review state before submission. No predecessor-head evidence or predecessor App credential is accepted as publication authority. PAT/OIDC remain explicit sources and there is no `github.token` or author fallback. + +**Executable evidence.** `tests/test_noema_reviewer_token_lifetime.py` binds the production workflow step graph to prepare → fresh App mint → publish with exact-head arguments and source-specific credentials. `tests/test_noema_two_phase_handoff.py` executes the helper against controlled gate doubles and proves no preparation-side publication, fresh-head/actor rebinding, stale-head non-publication, draft skip behavior, cleanup on malformed handoff, and hard-link alias rejection. `.github/workflows/noema-token-lifetime-quality-ci.yml` runs these contracts with hash-pinned dependencies on every relevant seam. + + +**Regression-suite consistency.** Legacy broader-suite assertions that still named the retired single-process Noema step/module are migrated to the two-phase prepare/publish contract, including step-scoped helper and envelope-argument evidence. This closes the false-GREEN gap where focused token-lifetime CI could pass while unchanged broader contracts described an impossible execution path. + +**Residual external verification.** After this central change reaches protected `main`, replay Required Noema Review for unchanged `naruon#1497@152d1998c4e8024be9dc7026c8789d343c884fd0`. Closure evidence requires a current-head schema-valid review or typed review-unavailable outcome without expired-token 401; a pre-merge run cannot prove the merged workflow-source path and is not promoted to release evidence. diff --git a/tests/test_noema_orchestrator_workflow_contract.py b/tests/test_noema_orchestrator_workflow_contract.py index 5355a8ca89..3f6116caf4 100644 --- a/tests/test_noema_orchestrator_workflow_contract.py +++ b/tests/test_noema_orchestrator_workflow_contract.py @@ -172,8 +172,13 @@ def test_noema_review_credentials_and_llm_use_orchestrator_free() -> None: assert "OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }}" in workflow assert "OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}" in workflow assert 'export NOEMA_LLM_MODEL="orchestrator/free"' in workflow - assert "python3 -m scripts.ci.noema_review_gate" in workflow - assert "python3 scripts/ci/noema_review_gate.py" not in workflow + prepare = workflow_step(workflow, "Prepare Noema model verdict") + publish = workflow_step(workflow, "Publish prepared Noema verdict on the exact live head") + assert '.github/actions/noema-review/two_phase.py' in prepare + assert '--prepare-verdict-file "$verdict_file"' in prepare + assert '.github/actions/noema-review/two_phase.py' in publish + assert '--publish-verdict-file "$verdict_file"' in publish + assert "python3 -m scripts.ci.noema_review_gate" not in workflow assert ( "contextual-orchestrator review sidecar must be provisioned before Noema LLM review." in workflow @@ -339,7 +344,7 @@ def test_strix_gateway_default_and_noema_sidecar_fail_closed(tmp_path: Path) -> noema_script = textwrap.dedent( workflow_step( workflow_text("noema-review.yml"), - "Run Noema LLM review and submit verdict", + "Prepare Noema model verdict", ).split(" run: |\n", 1)[1] ) noema_env = { diff --git a/tests/test_noema_reviewer_token_lifetime.py b/tests/test_noema_reviewer_token_lifetime.py new file mode 100644 index 0000000000..8057a23435 --- /dev/null +++ b/tests/test_noema_reviewer_token_lifetime.py @@ -0,0 +1,65 @@ +"""Regression contract for Noema reviewer credential lifetime.""" + +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +WORKFLOW = ROOT / ".github" / "workflows" / "noema-review.yml" +APP_TOKEN_ACTION = ( + "uses: actions/create-github-app-token@" + "bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0" +) + + +def _step_block(text: str, name: str) -> str: + """Return one exact named workflow step without borrowing sibling evidence.""" + marker = f" - name: {name}\n" + start = text.index(marker) + next_step = text.find("\n - name: ", start + len(marker)) + return text[start:] if next_step < 0 else text[start:next_step] + + +def test_noema_remints_repository_scoped_app_token_after_model_before_publication() -> None: + """A long model call must not publish with its predecessor App token.""" + workflow = WORKFLOW.read_text(encoding="utf-8") + prepare = _step_block(workflow, "Prepare Noema model verdict") + refresh = _step_block(workflow, "Refresh repository-scoped Noema GitHub App token for publication") + publish = _step_block(workflow, "Publish prepared Noema verdict on the exact live head") + + assert APP_TOKEN_ACTION in refresh + assert "--prepare-verdict-file" in prepare + assert "--publish-verdict-file" in publish + assert '--expected-head "$EXPECTED_HEAD_SHA"' in prepare + assert '--expected-head "$EXPECTED_HEAD_SHA"' in publish + assert 'export NOEMA_LLM_MODEL="orchestrator/free"' in prepare + assert "steps.noema_prepare.outputs.prepared == 'true'" in refresh + assert "steps.noema_credential.outputs.source == 'github-app'" in refresh + assert "steps.noema_prepare.outputs.prepared == 'true'" in publish + + +def test_publication_step_uses_fresh_app_token_without_authority_fallback() -> None: + """Publication selects the refreshed App token and fails closed for unknown sources.""" + workflow = WORKFLOW.read_text(encoding="utf-8") + refresh = _step_block(workflow, "Refresh repository-scoped Noema GitHub App token for publication") + publish = _step_block(workflow, "Publish prepared Noema verdict on the exact live head") + + assert "owner: ContextualWisdomLab" in refresh + assert "repositories: ${{ steps.noema_credential.outputs.repository }}" in refresh + assert "permission-pull-requests: write" in refresh + assert "permission-contents: read" in refresh + assert "permission-actions: read" in refresh + assert "steps.noema_github_app_publication_token.outputs.token" in publish + assert "steps.noema_github_app_token.outputs.token" not in publish + assert "secrets.NOEMA_REVIEW_TOKEN" in publish + assert "steps.noema_oidc_token.outputs.token" in publish + assert "github.token" not in publish + assert "refusing any GITHUB_TOKEN or author fallback" in publish + + +def test_prepare_and_publish_are_the_only_model_verdict_execution_path() -> None: + """The old single-process review path must not survive beside the handoff.""" + workflow = WORKFLOW.read_text(encoding="utf-8") + assert "Run Noema LLM review and submit verdict" not in workflow + assert "python3 -m scripts.ci.noema_review_gate" not in workflow + assert workflow.count("--prepare-verdict-file") == 1 + assert workflow.count("--publish-verdict-file") == 1 diff --git a/tests/test_noema_two_phase_handoff.py b/tests/test_noema_two_phase_handoff.py new file mode 100644 index 0000000000..992522be7b --- /dev/null +++ b/tests/test_noema_two_phase_handoff.py @@ -0,0 +1,193 @@ +"""Executable regressions for the Noema two-phase reviewer handoff.""" + +from __future__ import annotations + +import importlib.util +import os +from pathlib import Path +from types import ModuleType + +import pytest + + +ROOT = Path(__file__).resolve().parents[1] +MODULE_PATH = ROOT / ".github" / "actions" / "noema-review" / "two_phase.py" +HEAD = "a" * 40 +BASE = "b" * 40 + + +def _load_module() -> ModuleType: + spec = importlib.util.spec_from_file_location("noema_two_phase_under_test", MODULE_PATH) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def _patch_live_gate(monkeypatch: pytest.MonkeyPatch, module: ModuleType) -> None: + monkeypatch.setattr( + module.gate, + "fetch_pr", + lambda _repo, _number: { + "isDraft": False, + "headRefOid": HEAD, + "baseRefOid": BASE, + }, + ) + monkeypatch.setattr(module.gate, "require_expected_head", lambda _pr, _head: None) + monkeypatch.setattr(module.gate, "current_actor", lambda: "cwl-noema-review[bot]") + monkeypatch.setattr(module.gate, "PRIMARY_REVIEW_AUTHORS", frozenset({"seonghobae"})) + monkeypatch.setattr(module.gate, "existing_noema_review", lambda _pr, _actor: False) + + +def test_prepare_seals_validated_verdict_without_publishing(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Preparation performs model work but cannot submit GitHub review evidence.""" + module = _load_module() + _patch_live_gate(monkeypatch, module) + monkeypatch.setattr(module.gate, "fetch_diff", lambda _repo, _number: ("diff", False)) + monkeypatch.setattr(module.gate, "fetch_changed_files", lambda _repo, _number: [("src/a.py", "MODIFIED")]) + monkeypatch.setattr(module.gate, "build_review_context", lambda *_args: "context") + verdict = {"decision": "approve", "summary": "bounded"} + monkeypatch.setattr(module.gate, "call_llm", lambda *_args: verdict) + monkeypatch.setattr(module.gate, "submit_review", lambda *_args: pytest.fail("preparation must never publish")) + envelope = tmp_path / "verdict.json" + + assert module.prepare_verdict("ContextualWisdomLab/example", 7, HEAD, envelope) == 0 + payload = module._read_envelope(envelope) + assert payload["verdict"] == verdict + assert payload["expected_base"] == BASE + + +def test_publish_refetches_exact_head_and_base_with_fresh_actor_and_removes_envelope(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Publication rebinds repository/head/base/actor and consumes the private handoff.""" + module = _load_module() + _patch_live_gate(monkeypatch, module) + envelope = tmp_path / "verdict.json" + verdict = {"decision": "approve", "summary": "bounded"} + module._write_envelope(envelope, { + "schema_version": module.ENVELOPE_SCHEMA_VERSION, + "repository": "ContextualWisdomLab/example", + "pull_request_number": 7, + "expected_head": HEAD, + "expected_base": BASE, + "verdict": verdict, + }) + submitted: list[tuple[object, ...]] = [] + monkeypatch.setattr(module.gate, "submit_review", lambda *args: submitted.append(args)) + + assert module.publish_verdict("ContextualWisdomLab/example", 7, HEAD, envelope) == 0 + assert len(submitted) == 1 + assert submitted[0][0:2] == ("ContextualWisdomLab/example", 7) + assert submitted[0][3] == "cwl-noema-review[bot]" + assert submitted[0][4] == verdict + assert not envelope.exists() + + +def test_publish_rejects_stale_head_and_never_submits(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """A moved head invalidates predecessor model evidence before publication.""" + module = _load_module() + monkeypatch.setattr( + module.gate, + "fetch_pr", + lambda _repo, _number: { + "isDraft": False, + "headRefOid": "c" * 40, + "baseRefOid": BASE, + }, + ) + + def stale(_pr: object, _head: str) -> None: + raise RuntimeError("stale") + + monkeypatch.setattr(module.gate, "require_expected_head", stale) + monkeypatch.setattr(module.gate, "submit_review", lambda *_args: pytest.fail("stale evidence must not publish")) + envelope = tmp_path / "verdict.json" + module._write_envelope(envelope, { + "schema_version": module.ENVELOPE_SCHEMA_VERSION, + "repository": "ContextualWisdomLab/example", + "pull_request_number": 7, + "expected_head": HEAD, + "expected_base": BASE, + "verdict": {"decision": "approve"}, + }) + + assert module.publish_verdict("ContextualWisdomLab/example", 7, HEAD, envelope) == 0 + assert not envelope.exists() + + +def test_publish_rejects_base_drift_with_unchanged_head(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """A moved base invalidates the prepared diff/context even when the head is unchanged.""" + module = _load_module() + _patch_live_gate(monkeypatch, module) + monkeypatch.setattr( + module.gate, + "fetch_pr", + lambda _repo, _number: { + "isDraft": False, + "headRefOid": HEAD, + "baseRefOid": "c" * 40, + }, + ) + envelope = tmp_path / "verdict.json" + module._write_envelope(envelope, { + "schema_version": module.ENVELOPE_SCHEMA_VERSION, + "repository": "ContextualWisdomLab/example", + "pull_request_number": 7, + "expected_head": HEAD, + "expected_base": BASE, + "verdict": {"decision": "approve", "summary": "stale base"}, + }) + monkeypatch.setattr(module.gate, "submit_review", lambda *_args: pytest.fail("base-drifted evidence must not publish")) + + assert module.publish_verdict("ContextualWisdomLab/example", 7, HEAD, envelope) == 0 + assert not envelope.exists() + + +def test_prepare_skip_creates_no_publishable_envelope(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Draft skip semantics stay non-failing and cannot fabricate evidence.""" + module = _load_module() + monkeypatch.setattr( + module.gate, + "fetch_pr", + lambda _repo, _number: { + "isDraft": True, + "headRefOid": HEAD, + "baseRefOid": BASE, + }, + ) + monkeypatch.setattr(module.gate, "require_expected_head", lambda _pr, _head: None) + monkeypatch.setattr(module.gate, "current_actor", lambda: "cwl-noema-review[bot]") + monkeypatch.setattr(module.gate, "PRIMARY_REVIEW_AUTHORS", frozenset({"seonghobae"})) + monkeypatch.setattr(module.gate, "existing_noema_review", lambda _pr, _actor: False) + monkeypatch.setattr(module.gate, "call_llm", lambda *_args: pytest.fail("draft must not call the model")) + envelope = tmp_path / "verdict.json" + + assert module.prepare_verdict("ContextualWisdomLab/example", 7, HEAD, envelope) == 0 + assert not envelope.exists() + + +def test_publish_cleans_untrusted_envelope_even_when_read_validation_fails(tmp_path: Path) -> None: + """Malformed handoff state cannot linger after a failed publication attempt.""" + module = _load_module() + envelope = tmp_path / "verdict.json" + envelope.write_text("{}\n", encoding="utf-8") + os.chmod(envelope, 0o644) + + with pytest.raises(RuntimeError, match="permissions"): + module.publish_verdict("ContextualWisdomLab/example", 7, HEAD, envelope) + assert not envelope.exists() + + +def test_reader_rejects_hardlinked_aliases(tmp_path: Path) -> None: + """A caller-owned alias cannot mutate the supposedly private handoff file.""" + module = _load_module() + envelope = tmp_path / "verdict.json" + alias = tmp_path / "alias.json" + module._write_envelope(envelope, {"schema_version": module.ENVELOPE_SCHEMA_VERSION}) + os.link(envelope, alias) + try: + with pytest.raises(RuntimeError, match="single-link"): + module._read_envelope(envelope) + finally: + envelope.unlink(missing_ok=True) + alias.unlink(missing_ok=True) diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index a5079daa67..9823c417c1 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -770,7 +770,7 @@ def test_strix_gateway_default_and_noema_sidecar_fail_closed( noema_script = textwrap.dedent( workflow_step( workflow_text("noema-review.yml"), - "Run Noema LLM review and submit verdict", + "Prepare Noema model verdict", ).split(" run: |\n", 1)[1] ) noema_env = {