Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
262 changes: 262 additions & 0 deletions .github/actions/noema-review/two_phase.py
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -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:
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
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.")
Comment on lines +219 to +220

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Base updates preserve stale approvals

If the base advances after _canonical_base, the review still publishes and records only the unchanged head. Merge gates can accept stale evidence.

Prompt for agents
The exact-base check in .github/actions/noema-review/two_phase.py:publish_verdict is only a pre-submit snapshot. A base update can race the subsequent submit_review call, and any base update after publication leaves a review whose body and machine-readable footer identify only the head SHA. Existing review consumers therefore cannot prove that a review matches the current base. Extend the review evidence contract to include the reviewed base SHA and update every merge/review-evidence consumer to compare it with the live PR base. Also handle a base change between the final fetch and submission so a raced publication cannot remain valid evidence, using the repository's established review dismissal or invalidation mechanism as appropriate.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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
57 changes: 51 additions & 6 deletions .github/workflows/noema-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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' }}
Expand All @@ -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
Expand All @@ -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"
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
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"
36 changes: 36 additions & 0 deletions .github/workflows/noema-token-lifetime-quality-ci.yml
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading