diff --git a/.github/workflows/central-review.yml b/.github/workflows/central-review.yml index 799cf9e06..2fe8f3810 100644 --- a/.github/workflows/central-review.yml +++ b/.github/workflows/central-review.yml @@ -429,7 +429,7 @@ jobs: - name: Run independent PydanticAI review and publish current-head verdict env: GH_TOKEN: ${{ steps.noema_write_app.outputs.token }} - PYTHONPATH: ${{ github.workspace }}/reviewer + PYTHONPATH: ${{ github.workspace }}/reviewer:${{ github.workspace }}/packages/noema-core/src NOEMA_REVIEW_TOKEN_SOURCE: noema-github-app NOEMA_LLM_API_URL: ${{ vars.NOEMA_LLM_API_URL }} NOEMA_LLM_MODEL: ${{ vars.NOEMA_LLM_MODEL }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d83efcc04..4cb18ed15 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,8 +7,8 @@ on: - main concurrency: - group: noema-ci-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true + group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event_name == 'pull_request' && github.event.pull_request.number || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: verify: diff --git a/.github/workflows/hourly-product-development.yml b/.github/workflows/hourly-product-development.yml index d78793b2d..3bcaad5ce 100644 --- a/.github/workflows/hourly-product-development.yml +++ b/.github/workflows/hourly-product-development.yml @@ -8,9 +8,6 @@ on: required: false default: false type: boolean - schedule: - - cron: "47 * * * *" - concurrency: group: hourly-orchestrator-product-development-${{ github.repository }} cancel-in-progress: false @@ -22,9 +19,6 @@ env: DEFAULT_BRANCH: main OPENCODE_VERSION: "1.17.13" OPENCODE_SHA256: 157afa289d1a8d9372de0ce19ac726119b937a1f6b201808d46f06e4e59bb348 - # One gateway-backed session plus setup/diagnostic reserve fits in 55 minutes. - OPENCODE_RUN_TIMEOUT_SECONDS: "2700" - OPENCODE_KILL_GRACE_SECONDS: "30" MAX_CHANGED_FILES: "40" MAX_DIFF_BYTES: "500000" MAX_PR_TITLE_BYTES: "120" @@ -51,7 +45,7 @@ jobs: env: DRY_RUN: ${{ github.event_name == 'workflow_dispatch' && inputs.dry_run || false }} steps: - - name: Enforce zero-open-PR single-flight gate + - name: Validate work-conserving single-flight admission id: gate shell: bash env: @@ -80,13 +74,9 @@ jobs: fi if [ "$(jq 'length' <<<"$open_prs")" -gt 0 ]; then - { - echo "dispatch=false" - echo "reason=open_pull_request" - } >>"$GITHUB_OUTPUT" - echo "An open pull request exists; exact-head PR governance owns this hour." \ + echo "open_pull_request_count=at_least_one" >>"$GITHUB_OUTPUT" + echo "Open pull-request lanes remain; a new proposal is allowed only if publication proves path isolation from every live PR." \ >>"$GITHUB_STEP_SUMMARY" - exit 0 fi if { [ "$ORCHESTRATOR_KEY_CONFIGURED" != "true" ] \ @@ -141,6 +131,12 @@ jobs: supportability, or operations gap that can be completed as exactly one bounded pull request. Do not create another repository. + Existing open pull requests are independent governance lanes, not a global stop. + Select an unrelated buyer gap from current protected main. A trusted publisher will + fail closed if any proposed changed path overlaps any live open pull request or if + protected main advances. Do not intentionally duplicate or replace work already owned + by an active pull-request lane. + Keep Noema independently deployable and preserve its modular MSA role with ContextualWisdomLab/.github, naruon, contextual-orchestrator, and other CWL services. Keep interfaces explicit and replaceable. Route every Noema LLM @@ -206,7 +202,7 @@ jobs: run: | set -euo pipefail { - echo "Dry run: the zero-open-PR gate permits one bounded OpenCode proposal." + echo "Dry run: work-conserving admission permits one bounded OpenCode proposal; publication still requires current-base and open-PR path isolation." echo cat "$RUNNER_TEMP/noema-agent-prompt.md" } >>"$GITHUB_STEP_SUMMARY" @@ -280,8 +276,7 @@ jobs: run: | set -euo pipefail prompt="$(cat "$RUNNER_TEMP/noema-agent-prompt.md")" - if timeout --kill-after="${OPENCODE_KILL_GRACE_SECONDS}s" "${OPENCODE_RUN_TIMEOUT_SECONDS}s" \ - env -u GH_TOKEN -u GITHUB_TOKEN \ + if env -u GH_TOKEN -u GITHUB_TOKEN \ -u REPOSITORY_TOKEN \ -u ACTIONS_ID_TOKEN_REQUEST_TOKEN \ -u ACTIONS_ID_TOKEN_REQUEST_URL \ @@ -712,7 +707,7 @@ jobs: permission-metadata: read permission-pull-requests: write - - name: Revalidate queue and default-branch head + - name: Revalidate open-PR path isolation and default-branch head shell: bash env: GH_TOKEN: ${{ steps.maintainer_app.outputs.token }} @@ -725,20 +720,81 @@ jobs: exit 1 fi - if ! open_prs="$( - gh pr list \ - --repo "$GITHUB_REPOSITORY" \ - --state open \ - --limit 1 \ - --json number,url + proposal_paths="$RUNNER_TEMP/proposal-paths.b64" + git diff --cached --name-only -z | node -e ' + const chunks = []; + process.stdin.on("data", (chunk) => chunks.push(chunk)); + process.stdin.on("end", () => { + const names = Buffer.concat(chunks).toString("utf8").split("\0").filter(Boolean); + for (const name of names) { + process.stdout.write(Buffer.from(name, "utf8").toString("base64") + "\n"); + } + }); + ' >"$proposal_paths" + LC_ALL=C sort -u -o "$proposal_paths" "$proposal_paths" + + isolation_check="$RUNNER_TEMP/verify-open-pr-path-isolation.sh" + cat >"$isolation_check" <<'SCRIPT' + #!/usr/bin/env bash + set -euo pipefail + exclude_pr="${1:-}" + proposal_paths="$RUNNER_TEMP/proposal-paths.b64" + reserved_paths="$RUNNER_TEMP/open-pr-paths.b64" + overlap_paths="$RUNNER_TEMP/open-pr-overlap.b64" + : >"$reserved_paths" + + if ! open_pr_numbers="$( + gh api --paginate \ + "repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" \ + --jq '.[].number' )"; then echo "::error::pull_request_inventory_unavailable_after_generation" exit 1 fi - if [ "$(jq 'length' <<<"$open_prs")" -gt 0 ]; then - echo "::error::open_pull_request_after_generation" + + while IFS= read -r pull_number; do + [ -n "$pull_number" ] || continue + if ! [[ "$pull_number" =~ ^[1-9][0-9]*$ ]]; then + echo "::error::pull_request_inventory_invalid_after_generation" + exit 1 + fi + if [ -n "$exclude_pr" ] && [ "$pull_number" = "$exclude_pr" ]; then + continue + fi + if ! expected_files="$( + gh api "repos/${GITHUB_REPOSITORY}/pulls/${pull_number}" --jq '.changed_files' + )"; then + echo "::error::pull_request_file_inventory_unavailable_after_generation" + exit 1 + fi + if ! [[ "$expected_files" =~ ^[0-9]+$ ]] || [ "$expected_files" -gt 3000 ]; then + echo "::error::pull_request_file_inventory_unbounded_after_generation" + exit 1 + fi + before_count="$(wc -l <"$reserved_paths" | tr -d '[:space:]')" + if ! gh api --paginate \ + "repos/${GITHUB_REPOSITORY}/pulls/${pull_number}/files?per_page=100" \ + --jq '.[].filename | @base64' >>"$reserved_paths"; then + echo "::error::pull_request_file_inventory_unavailable_after_generation" + exit 1 + fi + after_count="$(wc -l <"$reserved_paths" | tr -d '[:space:]')" + if [ $((after_count - before_count)) -ne "$expected_files" ]; then + echo "::error::pull_request_file_inventory_incomplete_after_generation" + exit 1 + fi + done <<<"$open_pr_numbers" + + LC_ALL=C sort -u -o "$reserved_paths" "$reserved_paths" + comm -12 "$proposal_paths" "$reserved_paths" >"$overlap_paths" + if [ -s "$overlap_paths" ]; then + echo "::error::open_pull_request_after_generation_path_overlap" exit 1 fi + SCRIPT + chmod 0500 "$isolation_check" + + "$isolation_check" if ! live_base="$( gh api \ @@ -894,13 +950,19 @@ jobs: echo "::error::created_pull_request_queue_inventory_unavailable" false fi - if [ "$open_pr_numbers" != "$pr_number" ]; then + created_pr_occurrences="$(grep -Fxc -- "$pr_number" <<<"$open_pr_numbers" || true)" + if [ "$created_pr_occurrences" -ne 1 ]; then echo "::error::created_pull_request_queue_conflict" false fi + if ! "$RUNNER_TEMP/verify-open-pr-path-isolation.sh" "$pr_number"; then + echo "::error::created_pull_request_queue_conflict_path_overlap" + false + fi + trap - ERR { - echo "Opened bounded pull request: $pr_url" + echo "Opened bounded path-isolated pull request: $pr_url" echo "hourly-commercial-readiness owns review, repair, exact-head revalidation, and merge." } >>"$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/patch-validator-image.yml b/.github/workflows/patch-validator-image.yml index 89ed4139b..eb1f20292 100644 --- a/.github/workflows/patch-validator-image.yml +++ b/.github/workflows/patch-validator-image.yml @@ -5,8 +5,8 @@ on: workflow_dispatch: concurrency: - group: noema-patch-validator-image-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true + group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event_name == 'pull_request' && github.event.pull_request.number || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} permissions: contents: read diff --git a/.github/workflows/reviewer-ci.yml b/.github/workflows/reviewer-ci.yml index e04e0c1ea..21302a742 100644 --- a/.github/workflows/reviewer-ci.yml +++ b/.github/workflows/reviewer-ci.yml @@ -7,8 +7,8 @@ on: - main concurrency: - group: noema-reviewer-ci-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true + group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event_name == 'pull_request' && github.event.pull_request.number || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} permissions: contents: read @@ -20,6 +20,7 @@ jobs: timeout-minutes: 30 env: NOEMA_CODEGRAPH_SANDBOX_SOURCE_IMAGE: gcr.io/distroless/java-base-debian13:nonroot + PYTHONPATH: ${{ github.workspace }}/reviewer:${{ github.workspace }}/packages/noema-core/src defaults: run: working-directory: reviewer @@ -50,12 +51,98 @@ jobs: - name: install (hash-pinned dependencies) run: pip install --require-hashes --no-deps -r requirements-ci-hashes.txt + - name: test noema-core (100% line+branch coverage gate) + working-directory: packages/noema-core + run: python -m pytest + + - name: docstring coverage noema-core (100% gate) + working-directory: packages/noema-core + run: python -m interrogate -c pyproject.toml src/noema_core + - name: test (100% line+branch coverage gate) run: python -m pytest - name: docstring coverage (100% gate) run: python -m interrogate -c pyproject.toml noema_reviewer + - name: smoke-test installed reviewer wheel and sdist-to-wheel path + run: | + set -euo pipefail + wheel_dir="$RUNNER_TEMP/noema-reviewer-wheel" + sdist_dir="$RUNNER_TEMP/noema-reviewer-sdist" + sdist_wheel_dir="$RUNNER_TEMP/noema-reviewer-sdist-wheel" + direct_venv="$RUNNER_TEMP/noema-reviewer-install-smoke" + sdist_venv="$RUNNER_TEMP/noema-reviewer-sdist-install-smoke" + mkdir -p "$wheel_dir" "$sdist_dir" "$sdist_wheel_dir" + + python -m pip wheel . --no-deps --no-build-isolation --wheel-dir "$wheel_dir" + direct_wheel="$(find "$wheel_dir" -maxdepth 1 -type f -name 'noema_reviewer-*.whl' -print -quit)" + test -n "$direct_wheel" + + SDIST_DIR="$sdist_dir" SDIST_NAME_FILE="$RUNNER_TEMP/noema-reviewer-sdist-name" python - <<'PY' + import os + from pathlib import Path + from build_backend import build_sdist + + sdist_name = build_sdist(os.environ["SDIST_DIR"]) + Path(os.environ["SDIST_NAME_FILE"]).write_text(sdist_name, encoding="utf-8") + PY + sdist="$sdist_dir/$(cat "$RUNNER_TEMP/noema-reviewer-sdist-name")" + test -f "$sdist" + python -m pip wheel "$sdist" --no-deps --no-build-isolation --wheel-dir "$sdist_wheel_dir" + sdist_wheel="$(find "$sdist_wheel_dir" -maxdepth 1 -type f -name 'noema_reviewer-*.whl' -print -quit)" + test -n "$sdist_wheel" + + for contract in direct sdist; do + if [ "$contract" = direct ]; then + wheel="$direct_wheel" + venv_dir="$direct_venv" + else + wheel="$sdist_wheel" + venv_dir="$sdist_venv" + fi + python -m venv --system-site-packages "$venv_dir" + ( + cd "$RUNNER_TEMP" + PYTHONPATH='' "$venv_dir/bin/python" -m pip install --no-deps "$wheel" + PYTHONPATH='' "$venv_dir/bin/python" - <<'PY' + import hashlib + import os + from pathlib import Path + + import noema_core + import noema_core.agent + import noema_reviewer + from noema_reviewer.cli import parse_args + + canonical_agent = Path(os.environ["GITHUB_WORKSPACE"]) / "packages" / "noema-core" / "src" / "noema_core" / "agent.py" + installed_agent = Path(noema_core.agent.__file__) + assert hashlib.sha256(installed_agent.read_bytes()).digest() == hashlib.sha256(canonical_agent.read_bytes()).digest() + assert noema_core.NOEMA_PERSONA + assert noema_reviewer.build_agent is not None + assert parse_args([]).repo == "" + PY + ) + done + + - name: smoke-test isolated editable reviewer with locked runtime dependencies + run: | + set -euo pipefail + editable_venv="$RUNNER_TEMP/noema-reviewer-editable-smoke" + python -m venv "$editable_venv" + "$editable_venv/bin/python" -m pip install --require-hashes --no-deps -r requirements-ci-hashes.txt + "$editable_venv/bin/python" -m pip install --no-deps -e . + ( + cd "$RUNNER_TEMP" + PYTHONPATH='' "$editable_venv/bin/python" - <<'PY' + import noema_core + import noema_reviewer + + assert noema_core.build_agent is not None + assert noema_reviewer.build_agent is not None + PY + ) + - name: install lock-pinned CodeGraph tooling for sandbox smoke test env: NPM_CONFIG_IGNORE_SCRIPTS: "true" @@ -98,7 +185,7 @@ jobs: source_root="$RUNNER_TEMP/noema-codegraph-smoke" mkdir -p "$source_root" printf 'export const commercialReadiness = true;\n' >"$source_root/example.ts" - PYTHONPATH=. python - <<'PY' + python - <<'PY' import os from noema_reviewer.sandbox import DockerCodeGraphRunner diff --git a/.gitignore b/.gitignore index 910e84693..8fa7fc874 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ exchange-30d.ndjson exchange-30d.ndjson.provenance.json noema-kpi-evidence.json noema-smoke-evidence.json +reviewer/_build_include/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d69dff52..5574ceeb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## Unreleased +- `noema-core` provider-neutral Shared Kernel을 추가하여 이미 해석된 PydanticAI `Model`과 역할별 prompt/schema만 받아 Agent를 구성한다. 문자열 model identifier와 provider discovery·credential·routing·retry·failover는 Shared Kernel 밖에 두고 `Agent(..., retries=0)`으로 repository-local model-attempt authority를 만들지 않는다. Reviewer wheel·sdist·editable 설치는 canonical `packages/noema-core` source를 포함하거나 참조하며 별도 100% coverage·docstring과 clean install smoke로 검증한다. 외부 소비는 immutable versioned publication·exact source identity·SBOM/provenance·licensing/NOTICE·compatibility/rollback evidence 전에는 허용하지 않는다. - `writeAcquisitionPrivateFile`의 기존 대상 사전-교체 검증 read(`existingDescriptor` open)에 `O_NONBLOCK`을 추가해 fail-closed를 강화한다. 이 open은 이미 필수 filesystem capability로 `O_NONBLOCK`을 검증했지만 실제로는 사용하지 않아, 로컬 권한을 가진 행위자가 사전 `lstatSync` 정규 파일 확인과 이 open 사이에 대상 경로를 FIFO로 교체하면 writer가 나타날 때까지 무한정 블로킹해 writer lease를 계속 점유할 수 있었다. `O_NONBLOCK`은 정규 파일에는 영향이 없고, FIFO에서는 open이 즉시 반환되어 이어지는 descriptor 타입 검증이 그대로 fail-closed로 거부한다. 회귀 테스트(`test/acquisition-private-output-existing-target-nonblocking.test.ts`)와 기존 open-flags 계약 테스트 갱신으로 고정했다. - `readStableFile`의 close-후 재검증 단계(`afterClosePath` lookup 실패)와 `writeAcquisitionPrivateFile`의 cleanup-시점 `O_NONBLOCK` 소실 분기에 대한 fail-closed 회귀 테스트를 추가해 `scripts/lib/acquisition-data-room-integrity.mjs`/`scripts/lib/acquisition-private-output.mjs`의 100% coverage 게이트를 복구한다. 동작 변화는 없다. - Noema reviewer의 strict changed-file evidence를 historical 12-file prefix에서 canonical 80-file CodeGraph scope와 일치시켰다. 13–80 file PR은 선택된 모든 current-head file context를 유지하고 81개 이상은 기존처럼 실패-폐쇄하며, local CodeGraph fallback의 `HOME`·`TEMP`·`TMP`·`TMPDIR`은 ambient host path를 상속하지 않고 실행마다 새 private temporary directory로 격리한다. @@ -35,7 +36,7 @@ - 비리뷰 LLM 작업인 `hourly-product-development`를 리뷰와 동일한 `contextual-orchestrator` 게이트웨이 계약(`NOEMA_LLM_API_URL` `/v1`, 모델 별칭 `contextual-orchestrator`, 전용 `NOEMA_LLM_API_KEY`)으로 전환한다. Llama Nemotron → Nemotron Super → DeepSeek 순차 NIM 후보 폴백과 `NVIDIA_NIM_API_KEY` 직접 호출을 제거하고, 공유 `scripts/verify-orchestrator-gateway.mjs`가 `/healthz` 신원과 직접 공급자 호스트를 실패-폐쇄한다. 리뷰어의 `NOEMA_FALLBACK_*` / PydanticAI `FallbackModel` 순차 폴백도 제거해 남은 설정은 실패-폐쇄한다. 동일 계약을 `contracts/orchestrator-gateway.json`으로 공개해 `ContextualWisdomLab/naruon` 판단·결정 에이전트가 1급 소비자로 재사용할 수 있게 한다. naruon 배선은 별도 저장소 PR이다. 상위 공급자 키는 오케스트레이터 KV에 남기며 OIDC 토큰 중개·App 신원·3-runner 샌드박스 경계는 유지한다. - 검증된 active-orphan 워크플로 하나를 운영자가 호출할 수 있는 `operations:workflow-registry-disable` 경로를 추가한다. 저장소와 워크플로 ID를 `NOEMA_MAINTAINER_TOKEN_PATH` 위임 토큰 파일 읽기 전에 검사하고, 신선한 전체 레지스트리 감사·즉시 live refresh·프로세스 로컬 plan·보호된 main/워크플로 재검증·사후 전체 감사 봉투(`schema_version` 1, `PASS`/`FAIL`, `remaining_failure_codes`, `remaining_active_orphan_ids`)를 통과한 뒤에만 영수증을 유지한다. 성공 종료와 `post_audit_status: FAIL`은 해당 ID만 `disabled_manually`가 되었고 레지스트리는 아직 더러울 수 있음을 뜻하므로, 운영자는 영수증의 `remaining_active_orphan_ids`로 다음 단일 호출을 이어간다. 배치 비활성화·자가 수리 워크플로·거버넌스 완화는 추가하지 않으며 호출 계약은 doctoring에 기록한다. - 읽기 전용 `operations:runner-assignment` audit를 추가해 exact workflow run/source head에 대한 runner assignment를 완전 pagination으로 진단하고, 신선한 unassigned queue는 bounded grace 이후 실패-폐쇄한다. 이 증빙은 runner assignment와 required Check/CI, formal review, merge, release, deployment authority를 분리하며 assigned runner 이후 workflow failure를 성공으로 승격하지 않는다. -- production `operations:runner-assignment` audit는 `NOEMA_MAINTAINER_TOKEN_PATH`의 owner-only capability file만 읽고, ambient `GH_TOKEN`만 있으면 실패-폐쇄한다. `gh` spawn/stderr 진단은 활성 토큰을 exact-match로 `[REDACTED]` 치환하며, 빈 secret에 대해서는 원문 진단을 보존한다. assignment authority는 양의 `runner_id` 또는 비어 있지 않은 `runner_name`만 인정하며 queued `started_at`은 assignment evidence가 아니다. 운영자는 `printf '%s'`로 capability file을 만들고(`echo`/`printf '%s\\n'`는 trailing newline 때문에 실패-폐쇄), Actions workflow-run/job read만 가진 짧은 토큰을 준비한 뒤 PASS를 required Check·formal review·merge 권한으로 해석하지 마십시오. +- production `operations:runner-assignment` audit는 `NOEMA_MAINTAINER_TOKEN_PATH`의 owner-only capability file만 읽고, ambient `GH_TOKEN`만 있으면 실패-폐쇄한다. `gh` spawn/stderr 진단은 활성 토큰을 exact-match로 `[REDACTED]` 치환하며, 빈 secret에 대해서는 원문 진단을 보존한다. assignment authority는 양의 `runner_id` 또는 비어 있지 않은 `runner_name`만 인정하며 queued `started_at`은 assignment evidence가 아니다. 운영자는 `printf '%s'`로 capability file을 만들고(`echo`/`printf '%s\n'`는 trailing newline 때문에 실패-폐쇄), Actions workflow-run/job read만 가진 짧은 토큰을 준비한 뒤 PASS를 required Check·formal review·merge 권한으로 해석하지 마십시오. - coordinated vulnerability disclosure 정책과 evidence-preserving vulnerability handling lifecycle, read-only private-vulnerability-reporting setting audit를 추가한다. 이 source 변경은 live private reporting 활성화·notification staffing·end-to-end advisory exercise·release/deployment authority를 증명하지 않는다. - 개발 의존성 체인의 transitive `nanoid` lockfile resolution을 `3.3.17`에서 `3.3.18`로 최소 갱신하여 GHSA-2v37-7h3g-55p8 / CVE-2026-67213 보안 게이트를 복구한다. PostCSS의 선언 범위 `^3.3.16`과 다른 package metadata는 변경하지 않으며 audit waiver·ignore·severity 완화 없이 `npm ci`/`npm audit --audit-level=high`가 exact head에서 재검증되도록 유지한다. - lockfile 재생성 도구 체인을 Node.js 24.19.0/npm 11.17.0으로 정확히 고정하고, `strict-allow-scripts=true` 아래 승인된 install-script identity만 실행하며 schema v3 exact-base lockfile change control로 package metadata drift를 실패-폐쇄한다. exact package before/after digest에 더해 top-level metadata digest와 대규모 package-set bulk evidence를 결합하며, 선행 `nanoid@3.3.18` 보안 수정과 explicit `npm ci --legacy-peer-deps=false --install-links=false` 계약을 보존한다. package-manager/toolchain·install-script authority·vulnerability audit·review/merge authority는 별도 증거 계층으로 유지한다. diff --git a/docs/adr/0014-shared-noema-core-package.md b/docs/adr/0014-shared-noema-core-package.md new file mode 100644 index 000000000..4b5e01ead --- /dev/null +++ b/docs/adr/0014-shared-noema-core-package.md @@ -0,0 +1,100 @@ +# ADR-0014: Minimal `noema-core` Shared Kernel for Agent construction + +- **Status:** Proposed +- **Decision owner:** Noema repository governance +- **Scope:** `ContextualWisdomLab/noema` reviewer self-consumption and future versioned consumers + +## Problem + +Noema has multiple bounded-context consumers that need the same PydanticAI `Agent(...)` construction semantics, but those consumers do not share domain authority. Repeating the framework construction call in each consumer creates drift; centralizing model discovery, provider SDKs, credentials, fallback, retry policy, verdict schemas, tools, tenant state, or security policy would instead violate the repository's DDD boundary and duplicate canonical owners. + +The previous branch-local ADR used number `0012`, which now belongs on protected `main` to the runtime bounded-context decision. ADR identity is immutable repository architecture authority, so this decision is renumbered to `0014` rather than retaining two different ADR-0012 documents. + +## Constraints + +- `contextual-orchestrator` owns provider/model discovery, routing, test-time compute, provider/model retry and failover, provider credentials and provider-specific transport policy. +- Noema owns Agent Runtime and its bounded contexts, not foreign product truth. +- Reviewer verdict schema, deterministic gates, GitHub evidence policy and reviewer publication remain reviewer-owned. +- Tenant/application tool authority and domain state stay in their owning product. +- Security isolation, quarantine and outbound-policy authority stay with their canonical owners. +- Mutable branch refs and copied source are not acceptable cross-repository dependencies. +- External adoption requires an immutable versioned publication with exact source identity and compatibility evidence. + +## Alternatives + +### A. Duplicate the construction in every consumer + +Rejected. It preserves local autonomy but guarantees repeated framework wiring and version drift without adding a useful bounded-context distinction. + +### B. Put provider discovery, retry or transport in `noema-core` + +Rejected. That would recreate `contextual-orchestrator` policy inside Noema and would let a Shared Kernel become an ambient provider/model-attempt authority boundary. + +### C. Build an always-on Noema service for every consumer + +Rejected for this phase. A service would add deployment, network, authorization and recovery semantics that are not required to remove the verified same-language construction duplication. Cross-language consumers can be handled through released service/API contracts when a real caller requires them. + +### D. Minimal package with caller-supplied model + +Chosen. `packages/noema-core` owns only a role-neutral Noema persona fragment and a factory that accepts an already-constructed PydanticAI `Model` and calls `Agent(...)` with caller-owned prompt, output and deps types. The factory fixes PydanticAI model-attempt retries to zero instead of exposing a reusable retry knob; orchestration-level retry/failover remains with `contextual-orchestrator`. + +## Decision + +Create `packages/noema-core` as a minimal Shared Kernel with: + +- `NOEMA_PERSONA = "You are Noema"` as a role-neutral identity prefix; +- `build_agent(model, *, system_prompt, output_type=str, deps_type=None)`; +- rejection of string model identifiers so PydanticAI's implicit provider/model inference cannot move discovery into the Shared Kernel; +- no caller-visible `retries` parameter and `Agent(..., retries=0)` at this boundary so the Shared Kernel cannot silently create additional model attempts outside the orchestrator contract. + +`noema-core` deliberately does **not** own: + +- provider SDK construction or endpoint selection; +- credentials, key discovery, model groups, retries or fallback; +- reviewer verdicts, gates or merge authority; +- tool/dependency authorization; +- tenant isolation, domain persistence or foreign truth; +- quarantine, egress or malware/security verdict authority. + +The current PR's only production consumer is `reviewer/noema_reviewer`. Reviewer packaging stages the canonical `packages/noema-core/src/noema_core` source into wheel/sdist builds so the installed reviewer contains the exact shared module without copying a second source tree. Editable installs and CI use the same canonical path. This is a transitional monorepo packaging arrangement, not permission for external repositories to consume the mutable branch. + +## Verification contract + +Before this decision can become `Accepted`, the exact candidate head must prove: + +1. `packages/noema-core` line and branch coverage are 100% and public docstring coverage is 100%. +2. The reviewer retains its existing coverage/docstring gates and behavior. +3. Installed reviewer wheel and sdist-to-wheel smoke tests import both `noema_reviewer` and `noema_core` outside the checkout and prove the installed shared `agent.py` bytes match the canonical source. +4. Evidence-only reviewer imports remain lazy and do not require model construction. +5. String model identifiers fail closed at the Shared Kernel boundary. +6. `build_agent` exposes no retry-policy argument and constructs the PydanticAI agent with model-attempt retries disabled; provider/model retry and failover remain contextual-orchestrator authority. +7. Central review execution receives the canonical package path without moving provider routing authority into Noema. +8. No cross-repository consumer adopts `noema-core` until immutable publication exists. + +## Publication boundary + +A merge of this PR establishes protected source, not an external dependency. External consumption requires the repository's selected immutable publication mechanism to provide all applicable evidence together: + +- semantic version and immutable source commit; +- artifact digest/integrity; +- package/install smoke tests; +- SBOM and provenance; +- licensing/NOTICE compatibility; +- compatibility/migration and rollback guidance. + +After such a release exists, consumers must pin the released version through their own ACL/adapter and regenerate their exact-head acceptance evidence. A mutable Git branch, local path, copied module, or open PR head is never the production dependency. + +## Consequences + +The shared surface stays intentionally small, so framework construction drift is removed without turning Noema into an LLM gateway or a domain super-service. The cost is a transitional reviewer build backend until `noema-core` has its own immutable package publication. That transitional backend must remain bounded, deterministic and covered by installed-artifact tests. + +Removing the retry argument is intentionally restrictive. A consumer that needs a different attempt policy must not add a local convenience knob to the Shared Kernel; it must use the released contextual-orchestrator contract or make a separately reviewed bounded-context decision that does not duplicate provider/model retry authority. + +A future need for cross-language access is a separate architecture decision. It should begin from a real consumer and released contract rather than expanding this package pre-emptively. + +## Follow-up + +- Merge the reviewer self-consumption only after current-head CI, security, reviewer, package and provenance gates pass. +- Publish `noema-core` through the repository-approved immutable mechanism when release evidence is ready. +- Replace transitional monorepo bundling with a normal released dependency after publication. +- Update any future consumer only after verifying its canonical owner boundary and exact released artifact identity. diff --git a/docs/adr/README.md b/docs/adr/README.md index 2ceec6502..58df3e222 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -16,6 +16,7 @@ ADR은 **왜 이 구조를 선택했는지**를 기록합니다. 구현 상태 | [0010](./0010-private-target-review-auth.md) | Proposed | private review target의 첫 live PR lookup부터 single-repository Noema App token을 사용하고 workflow `GITHUB_TOKEN` cross-repository fallback을 금지한다. | | [0011](./0011-independent-reviewer-governance.md) | Proposed | qualifying formal approval의 eligibility·exact-head·staleness를 검증하고 check/status/scanner/model evidence가 approval을 대체하지 못하게 한다. | | [0012](./0012-runtime-orchestration-bounded-contexts.md) | Proposed | Agent Runtime, Workflow / Task Execution, Tool / Capability, State / Checkpoint, isolation, policy, observability, recovery의 소유권을 분리하고 provider routing·foreign truth·cross-service SQL을 Noema 경계 밖에 둔다. | +| [0014](./0014-shared-noema-core-package.md) | Proposed | role-neutral PydanticAI `Agent(...)` construction만 `packages/noema-core` Shared Kernel로 추출하고 provider routing·credential policy·verdict·tool/deps·tenant truth는 canonical owner에 남긴다. | ## ADR lifecycle diff --git a/docs/automation-threat-model.md b/docs/automation-threat-model.md index 653693ee4..adc1b5d9b 100644 --- a/docs/automation-threat-model.md +++ b/docs/automation-threat-model.md @@ -128,13 +128,13 @@ The security objective is to prevent a lower-trust domain from converting its ou **Threat:** publisher creates a PR but loses the response, then broad cleanup closes/deletes another actor's resource. -**Controls proposed by PR #80:** unique cryptographic publication marker, exact branch/head/base match, numeric PR identity, unique recovery only, conditional branch cleanup. +**Controls implemented on protected `main`:** the non-executing publisher uses a cryptographic publication marker, requires exact proposal head and expected base identity, accepts only a positive numeric pull-request identity, and recovers a lost/malformed create response only when a fully paginated head-scoped search yields exactly one PR whose head, base, and marker all match the current publication. Cleanup re-runs that unique recovery before closing a PR and couples remote-branch cleanup to the exact proposal head. Closed, unmerged PR #80 is historical lineage only; it is not the current implementation owner or evidence authority. ### T-A08 Proposal branch race **Threat:** another actor creates same remote branch between inventory read and push, or advances it before cleanup. -**Controls proposed by PR #80:** expected-absence branch creation lease and exact-created-head deletion lease; no check-then-unguarded-push or unconditional delete. +**Controls implemented on protected `main`:** branch creation uses Git's explicit expected-absence lease (`--force-with-lease=:`), and remote cleanup uses an exact-created-head deletion lease (`--force-with-lease=:`). There is no check-then-unguarded push or unconditional branch deletion. Closed, unmerged PR #80 is retained only as historical provenance. ### T-A09 Queue race after generation @@ -233,4 +233,4 @@ These remain external evidence and must not be closed with documentation-only ch ## 9. Rationale and references -Primary-source rationale and APA 7 references for GitHub OIDC, SLSA source identity, NIST SSDF, Cloudflare capability/state semantics are maintained in `docs/doctoring/architecture-trust-boundaries.md`. Git conditional ref-update and publisher-specific rationale is maintained in the active PR #80 doctoring and should be integrated without duplicating mutable implementation claims after that PR lands. +Primary-source rationale and APA 7 references for GitHub OIDC, SLSA source identity, NIST SSDF, Cloudflare capability/state semantics are maintained in `docs/doctoring/architecture-trust-boundaries.md`. Git conditional ref-update and publisher-specific rationale for the protected implementation are maintained in `docs/doctoring/atomic-product-publisher-lease.md`. Closed, unmerged PR #80 is historical development lineage only and does not define current control status or implementation authority. diff --git a/docs/doctoring/hourly-product-development-prerequisites.md b/docs/doctoring/hourly-product-development-prerequisites.md index 24be3b396..fccc7f834 100644 --- a/docs/doctoring/hourly-product-development-prerequisites.md +++ b/docs/doctoring/hourly-product-development-prerequisites.md @@ -6,17 +6,21 @@ This doctoring note uses APA 7 reference form. It separates source-supported fac ## Problem statement -The scheduled development path has two independent credential prerequisites: +The centrally dispatched development path has two independent credential prerequisites: 1. `NOEMA_LLM_API_URL` and `NOEMA_LLM_API_KEY` permit the read-only OpenCode proposal job to reach the `contextual-orchestrator` gateway. 2. `NOEMA_MAINTAINER_APP_CLIENT_ID` and `NOEMA_MAINTAINER_APP_PRIVATE_KEY` permit the later non-executing publisher to create one repository-scoped branch and pull request. Checking only the inference token can spend model compute on a proposal that the workflow is structurally unable to publish. That is a deterministic configuration failure rather than a model-quality failure and should be rejected before checkout or inference. +A separate scheduling problem exists when independent review lanes are waiting on Checks or external capacity. Treating the mere existence of any open pull request as a repository-wide stop converts one blocked lane into a global development stall. Noema therefore distinguishes lane-level governance from new buyer-gap development. A healthy commercial-readiness pass may dispatch one product-development run while other pull requests remain open, but publication must prove that the proposal is based on the unchanged protected head and does not reuse any changed path owned by another live pull request. + ## Source-supported controls GitHub documents that a workflow reads a secret only when the workflow explicitly includes it, and recommends granting credentials the minimum possible permissions. GitHub further recommends GitHub Apps as fine-grained, short-lived, non-user-bound credentials when repository automation needs permissions beyond read-only access. These facts support separating the gateway inference token from the repository publication credential and preserving read-only job-level `GITHUB_TOKEN` permissions. This is a least privilege control: model execution never receives publication authority, and publication receives only the repository-scoped permissions required to create one branch and pull request. +GitHub's pull-request REST API exposes the current pull request, its `changed_files` count, and a paginated list of changed files. Noema uses those source-of-truth surfaces to reject a proposal when it cannot enumerate a competing PR completely or when an exact changed path overlaps. This is a repository-specific conflict-reduction control, not a proof of semantic independence: separate files can still participate in one invariant. + NIST SP 800-218 Version 1.1 recommends integrating secure-development requirements and verification into the software life cycle. NIST SP 800-218A augments that framework with practices specific to generative AI and foundation-model systems. The December 2025 SP 800-218 Revision 1 initial public draft describes updated secure and reliable development practices, but remains a draft; Noema therefore records it as a current informative source while retaining the final Version 1.1 and final AI community profile as the normative published references. ## Noema-specific decision @@ -30,6 +34,8 @@ Before OpenCode starts, the proposal gate evaluates only presence booleans: The workflow does not reveal values, import the private key, mint an App token, or call a model during this gate. Missing publication configuration returns the stable reason `maintainer_app_unavailable` and stops before checkout, dependency installation, OpenCode download, or gateway inference. Missing gateway configuration returns `orchestrator_gateway_unavailable`. +The gate also verifies that the open-PR inventory itself can be read. An existing PR is not a failure reason. If another PR is present, the workflow records that a governed lane exists and continues only under the later publication rule: all proposal changed paths must be disjoint from all currently open PR changed paths. The publisher reads the complete open-PR inventory twice around remote creation, validates each PR's reported `changed_files` count against the paginated file list, rejects inventories beyond GitHub's supported 3,000-file PR listing bound, and compares base64-encoded path identities so embedded whitespace cannot turn a path into a line-oriented false match. A current open PR may therefore coexist with a newly created proposal only when the exact path sets remain disjoint. + The App token is still minted only in the third, non-executing publication job. Presence checking does not prove that the key is valid, that the App remains installed, or that permissions are sufficient; those live failures continue to fail closed when `actions/create-github-app-token` runs. This preserves the late-token trust boundary while preventing known-impossible sessions. Manual `dry_run` deliberately bypasses credential-presence requirements because it performs no checkout, model call, artifact publication, branch push, or pull-request creation. It remains an operator inspection path rather than evidence that a live proposal can be published. @@ -45,14 +51,18 @@ Executable tests must prove that: - both Maintainer App presence booleans are evaluated in the pre-inference gate; - either missing value produces `dispatch=false` and `reason=maintainer_app_unavailable`; - missing gateway URL or key produces `orchestrator_gateway_unavailable`; -- the gate appears before task preparation, checkout, and OpenCode execution; +- unreadable open-PR inventory fails closed while the existence of a readable open PR does not globally suppress a healthy development pass; +- a proposal whose exact path intersects any other open PR fails closed before remote creation; +- after PR creation, path isolation is re-evaluated with the newly created PR excluded, so a raced overlapping PR causes cleanup rather than acceptance; +- incomplete or unbounded competing-PR file inventory fails closed; +- protected `main` must still equal the proposal base before publication; - `dry_run=true` remains available without production credentials; - the dedicated gateway token and reviewer App identity remain separate; and -- operations and doctoring documents describe the same failure reason and credential names. +- operations and doctoring documents describe the same failure reasons and credential names. ## Residual risk -Presence booleans can become stale between the initial gate and publication, and they cannot validate App installation scope or private-key correctness. Exact publication remains protected by fresh token minting, queue and base-head revalidation, repository-scoped permissions, and ordinary pull-request governance. The new gate reduces deterministic cost waste; it is not a substitute for live App readiness evidence under issue #29. +Presence booleans can become stale between the initial gate and publication, and they cannot validate App installation scope or private-key correctness. Exact publication remains protected by fresh token minting, base-head revalidation, repository-scoped permissions, path-isolation checks before and after remote PR creation, and ordinary pull-request governance. GitHub does not expose an atomic transaction combining "no path overlap", base-head compare-and-swap, branch creation, and PR creation, so a narrow race remains after the final read. Different files can also violate one shared invariant without a literal path collision. These residual risks are why path isolation is only an admission control: it does not replace semantic review, required exact-head Checks, branch protection, or successor restacking. The gate reduces deterministic cost waste and global queue stalls; it is not a substitute for live App readiness evidence under issue #29. ## APA 7 references @@ -62,6 +72,8 @@ GitHub. (2026). *Secrets*. GitHub Docs. Retrieved August 5, 2026, from https://d GitHub. (2026). *Making authenticated API requests with a GitHub App in a GitHub Actions workflow*. GitHub Docs. Retrieved August 5, 2026, from https://docs.github.com/en/apps/creating-github-apps/writing-code-for-a-github-app/making-authenticated-api-requests-with-a-github-app-in-a-github-actions-workflow +GitHub. (2026). *REST API endpoints for pull requests*. GitHub Docs. Retrieved September 5, 2026, from https://docs.github.com/en/rest/pulls/pulls + Souppaya, M., Scarfone, K., & Dodson, D. (2022). *Secure software development framework (SSDF) version 1.1: Recommendations for mitigating the risk of software vulnerabilities* (NIST Special Publication 800-218). National Institute of Standards and Technology. https://doi.org/10.6028/NIST.SP.800-218 Booth, H., Ogata, M., Kent, K., Souppaya, M., & Dodson, D. (2025). *Secure software development framework (SSDF) version 1.2: Recommendations for mitigating the risk of software vulnerabilities* (Initial Public Draft NIST Special Publication 800-218, Revision 1). National Institute of Standards and Technology. https://doi.org/10.6028/NIST.SP.800-218r1.ipd diff --git a/docs/noema-agent-sandbox-plan.md b/docs/noema-agent-sandbox-plan.md index f2e9bdff2..7324945ab 100644 --- a/docs/noema-agent-sandbox-plan.md +++ b/docs/noema-agent-sandbox-plan.md @@ -51,10 +51,17 @@ The driver returns JSON: "findings": [ { "severity": "critical | high | medium | low | info", + "priority": "P1 | P2 | P3", "path": "relative/path", "line": 1, - "evidence": "log, SARIF, test, or source reference", - "recommendation": "specific fix" + "check_name": "exact current-head failed check name | null", + "evidence": "log, SARIF, test, source, or other independently checkable reference", + "evidence_type": "nearby_implementation | matching_existing_example | cross_file_counterpart | current_official_docs | failed_check_or_log", + "observable_impact": "specific user or operator consequence", + "trigger": "concrete condition that exposes the issue", + "recommendation": "smallest specific fix", + "regression_command": "one exact single-line command or test target", + "suggested_diff": "optional replacement text | null" } ], "suggested_patch_ref": "optional artifact path or branch", @@ -63,6 +70,22 @@ The driver returns JSON: } ``` +`check_name` is optional for ordinary source, SARIF, dependency, and review-thread +findings. When a finding is offered as the causal RCA for a failed current-head +check, it must equal that exact check name. A failed check remains `blocked` +unless it has its own blocking-severity finding on a current-head changed path +with a positive source line; one finding cannot authorize multiple failed +checks. + +Every finding is actionable data rather than prose-only advice. Priority, +evidence type, observable impact, trigger, smallest fix, and an exact regression +command are required. A `regression_command` cannot contain a newline or Markdown +backtick. `suggested_diff` is optional, but when present it cannot contain a +Markdown fence and must anchor to a right-side line in the exact PR diff before +publication. Valid replacement text is published through GitHub's inline review +`comments` payload as a suggestion rather than only being displayed in the +top-level review body. + Noema-issued installation tokens are used only after the sandboxed agent has a bounded verdict to publish. The token scope is limited to the target repository and central review workflow permissions. @@ -150,6 +173,12 @@ failure and blocks strict approval. a failure came from missing evidence, dependency vulnerability, image verification, image vulnerability, CodeGraph failure, sandbox timeout, attestation creation/verification, model exhaustion, or GitHub API rejection. +- Each ordinary failed current-head check either has its own exact-name, + changed-path, positive-line blocking RCA or keeps the verdict `blocked`; + another failed check's finding cannot satisfy that evidence requirement. +- Each finding carries priority, evidence type, observable impact, trigger, + smallest fix, and one exact regression command; any proposed replacement text + must be fence-safe and exact-diff-anchorable before GitHub receives it. - Medium-or-higher dependency and sandbox-image findings from OSV, Trivy, and dependency-review are remediated by package/image bump or source change, not by gate weakening. @@ -186,10 +215,12 @@ privileged publication plane. The judgement plane is implemented as the Python package `reviewer/noema_reviewer` (a PydanticAI `ReviewAgent` driver). It returns the -JSON verdict contract above, enforces strict-evidence blocking and -MEDIUM-or-higher dependency downgrade around the model, preserves reviewed PR -comments and current check conclusions, records containerized CodeGraph status, -and publishes only against the live exact head after attested manifest -verification. The Noema Worker (`src/`) remains the token-exchange boundary -only. Reviewer code ships with 100% line and branch coverage and 100% docstring -coverage; the Worker release gate remains `npm run release:verify`. \ No newline at end of file +JSON verdict contract above, enforces strict-evidence blocking, exact per-check +failed-check RCA binding, actionable finding validation, exact-diff suggestion +anchoring, and MEDIUM-or-higher dependency downgrade around the model. It +preserves reviewed PR comments and current check conclusions, records +containerized CodeGraph status, and publishes only against the live exact head +after attested manifest verification. The Noema Worker (`src/`) remains the +token-exchange boundary only. Reviewer code is required to retain 100% line and +branch coverage and 100% docstring coverage; the Worker release gate remains +`npm run release:verify`. diff --git a/docs/operations/hourly-product-development.md b/docs/operations/hourly-product-development.md index 56331c13b..941df65aa 100644 --- a/docs/operations/hourly-product-development.md +++ b/docs/operations/hourly-product-development.md @@ -2,34 +2,38 @@ ## 목적과 책임 경계 -`.github/workflows/hourly-product-development.yml`은 **열린 PR 0개** 상태에서만 Noema의 다음 구매자 가시적 제품 증분을 제안합니다. OpenCode 1.17.13은 코딩 에이전트로만 남고, 모델 호출은 리뷰와 같은 `contextual-orchestrator` 게이트웨이 계약을 사용합니다. 리뷰, 승인, 병합, 릴리스, 배포는 수행하지 않습니다. 정확한 현재 HEAD의 리뷰, 필수 Checks, 미해결 스레드, 저장소 규칙, 병합 가능성 판단은 기존 `hourly-commercial-readiness`가 계속 담당합니다. 자동 개발은 후보 PR을 만드는 역할만 하며 최종 거버넌스 권한을 획득하지 않습니다. +`.github/workflows/hourly-product-development.yml`은 Noema의 다음 구매자 가시적 제품 증분을 제안합니다. 기존 PR의 리뷰나 Checks가 대기 중이라는 이유만으로 저장소 전체 개발을 멈추지는 않습니다. 열린 PR은 각각 독립된 거버넌스 lane으로 남고, 새 제안은 게시 직전과 PR 생성 직후에 **모든 기존 열린 PR의 변경 경로와 겹치지 않는지** 확인합니다. 경로가 하나라도 겹치거나 열린 PR의 변경 파일 목록을 완전하게 읽을 수 없거나 `main`이 제안 base에서 전진하면 실패 폐쇄합니다. 동시에 활성화되는 product-development workflow는 하나뿐입니다. -워크플로는 매시 47분에 실행되고 수동 `dry_run=true`를 지원합니다. 드라이 런은 실제 PR 목록과 작업 계약만 확인하며 checkout, 모델 호출, 아티팩트 업로드, 브랜치 push, PR 생성을 하지 않습니다. GitHub 예약 실행은 정시 SLA가 아니므로 각 실행은 이전 상태를 믿지 않고 열린 PR 목록, 기본 브랜치 SHA, 필요한 자격 증명을 다시 확인합니다. 목록 조회 실패, 기존 PR 발견, 게이트웨이 부재는 모두 실패 폐쇄 사유입니다. +OpenCode 1.17.13은 코딩 에이전트로만 남고, 모델 호출은 리뷰와 같은 `contextual-orchestrator` 게이트웨이 계약을 사용합니다. 리뷰, 승인, 병합, 릴리스, 배포는 수행하지 않습니다. 정확한 현재 HEAD의 리뷰, 필수 Checks, 미해결 스레드, 저장소 규칙, 병합 가능성 판단은 기존 `hourly-commercial-readiness`가 계속 담당합니다. 자동 개발은 겹치지 않는 후보 PR을 만드는 역할만 하며 최종 거버넌스 권한을 획득하지 않습니다. -## 게이트웨이 계약과 시간 예산 +조직 중앙 commercial-readiness loop가 저장소별 열린 PR과 활성 writer를 확인한 뒤 이 워크플로를 dispatch합니다. 남아 있는 PR 수는 새 작업의 전역 정지 조건이 아닙니다. commercial-readiness 실행 자체에 operational error가 없어야 하며, 이미 product-development run이 pending·queued·running 상태이면 새 실행을 만들지 않습니다. 저장소 안에는 별도 schedule이 없습니다. 수동 `dry_run=true`는 실제 PR inventory와 작업 계약만 확인하며 checkout, 모델 호출, 아티팩트 업로드, 브랜치 push, PR 생성을 하지 않습니다. 각 실행은 이전 상태를 믿지 않고 열린 PR inventory, 기본 브랜치 SHA, 필요한 자격 증명을 다시 확인합니다. 목록 조회 실패와 게이트웨이·게시 자격 증명 부재는 모두 실패 폐쇄 사유입니다. + +## 게이트웨이 계약과 실행 종료 권한 공식 OpenCode 아카이브는 고정 버전과 SHA-256으로 검증합니다. 공급자는 `contextual-orchestrator` 한 곳만 허용합니다. `NOEMA_LLM_API_URL`은 `/v1`로 끝나는 HTTPS OpenAI 호환 주소여야 하고, `NOEMA_LLM_MODEL`은 보통 라우팅 별칭 `contextual-orchestrator`이며, `NOEMA_LLM_API_KEY`는 전용 게이트웨이 추론 토큰입니다. 상위 공급자 키(`NVIDIA_NIM_API_KEY`, `NVIDIA_NIM_API_KEY_SUB`, `BYTEZ_API_KEY`, `OPENROUTER_API_KEY`, `OPENAI_API_KEY`)는 오케스트레이터 KV에만 두고 Noema 런타임에 넣지 않습니다. -Noema는 모델 후보를 순서대로 시도하지 않습니다. 최소 비용과 최대 성능 선택은 오케스트레이터의 책임입니다. 직접 NVIDIA NIM, OpenAI, GitHub Models, OpenRouter, Bytez 호스트로 폴백하지 않습니다. 세션은 **한 번**이며 2,700초와 강제 종료 유예 30초를 적용합니다. 최초 설정과 최종 진단에 300초를 예약하면 총 3,030초이며, 3,300초인 55분 제안 job 예산 안에 270초의 명시적 여유를 남깁니다. 세션이 실패하면 다음 모델을 고르지 않고 안정적인 실패 진단으로 종료합니다. +Noema는 모델 후보를 순서대로 시도하지 않습니다. 최소 비용과 최대 성능 선택은 오케스트레이터의 책임입니다. 직접 NVIDIA NIM, OpenAI, GitHub Models, OpenRouter, Bytez 호스트로 폴백하지 않습니다. OpenCode 세션에는 Noema가 만든 추론·reasoning·stream·tool-call 경과시간 cutoff를 두지 않습니다. GNU `timeout`으로 세션을 2,700초에 종료하던 경로와 강제 종료 유예 설정은 제거했습니다. `propose_product_increment`의 GitHub Actions `timeout-minutes: 55`는 runner/job 전체에 대한 플랫폼 관리 한계이며 모델 또는 provider timeout이 아닙니다. 따라서 정상 provider 종료와 사용자 취소, GitHub의 administrative job timeout을 같은 모델 실패로 해석하거나 다음 모델 선택의 근거로 사용하지 않습니다. 세션이 자체 오류로 끝나더라도 Noema에서 다음 모델을 고르지 않습니다. 공유 스크립트 `scripts/verify-orchestrator-gateway.mjs`가 리뷰와 동일한 사전 점검을 수행합니다. 인증 없이 `/healthz`가 `service=contextual-orchestrator`를 반환해야 하며, 알려진 직접 공급자 호스트는 거부합니다. 같은 계약은 `contracts/orchestrator-gateway.json`으로 공개되며 `ContextualWisdomLab/naruon`의 판단·결정 에이전트도 1급 소비자입니다. naruon 배선은 이 저장소가 아니라 별도 PR에서 합니다. ## 세 runner의 자격 증명 분리 -첫 번째 제안 runner는 읽기 권한만 가지며 OpenCode subprocess에는 게이트웨이 추론 토큰만 전달합니다. GitHub 토큰, OIDC 값, Actions 런타임 토큰, 캐시 토큰, runner 명령 파일 채널을 제거합니다. 변경은 40개 파일과 500,000바이트로 제한하고 공백 오류, 심링크 모드 `120000`, gitlink 모드 `160000`을 원본 모드와 대상 모드 양쪽에서 검사합니다. 결과는 정확한 base SHA, 파일 수, 바이트 수, SHA-256에 결합된 binary full-index `proposal.patch`로 저장합니다. +첫 번째 제안 runner는 읽기 권한만 가지며 OpenCode subprocess에는 게이트웨이 추론 토큰만 전달합니다. GitHub 토큰, OIDC 값, Actions 런타임 토큰, 캐시 토큰, runner 명령 파일 채널을 제거합니다. 변경은 40개 파일과 500,000바이트로 제한하고 공백 오류, 심링크 모드 `120000`, gitlink 모드 `160000`을 원본 모드와 대상 모드 양쪽에서 검사합니다. 결과는 정확한 base SHA, 파일 수, 바이트 수, SHA-256에 결합된 binary full-index `proposal.patch`로 저장합니다. 제안 프롬프트는 열린 PR의 대기 상태를 전역 중단 사유로 취급하지 않되, 기존 활성 PR과 같은 작업을 의도적으로 중복하지 말 것을 요구합니다. 실제 비중첩성 판정은 모델의 주장에 의존하지 않고 게시 runner가 수행합니다. 두 번째 검증 runner는 게이트웨이 키와 Maintainer App 키가 없는 새 실행기입니다. `actions: read`, `contents: read`, `pull-requests: read`만 사용합니다. artifact ID, 이름, 만료 여부, 원본 workflow run, digest, patch 크기와 해시, base SHA를 독립적으로 확인합니다. 패치를 적용한 뒤 격리된 임시 홈과 제거된 GitHub·OIDC·Actions 채널에서 `npm run release:verify`를 실행하고 검증 전후 staged patch digest가 동일한지 확인합니다. 이 runner는 제안 코드를 실행하지만 게시 권한을 받지 않습니다. -`publish_product_increment`는 **세 번째 새 게시 runner**입니다. 제안 코드를 실행하지 않고 게이트웨이 키도 받지 않습니다. 기본 브랜치에서 신뢰된 PR 메타데이터 파서를 먼저 복사한 뒤 동일한 artifact ID와 digest-bound patch를 다시 검증합니다. 그 다음에만 full SHA로 고정된 액션이 짧은 수명의 Maintainer App 토큰을 발급합니다. 토큰 범위는 Noema 저장소의 metadata read, contents write, pull-request write로 제한됩니다. App 토큰 발급 후에도 열린 PR 큐와 실제 `main` SHA를 다시 읽고, 새 PR이나 base 전진이 있으면 원격 변경 전에 종료합니다. +`publish_product_increment`는 **세 번째 새 게시 runner**입니다. 제안 코드를 실행하지 않고 게이트웨이 키도 받지 않습니다. 기본 브랜치에서 신뢰된 PR 메타데이터 파서를 먼저 보존한 뒤 동일한 artifact ID와 digest-bound patch를 다시 검증합니다. 그 다음에만 full SHA로 고정된 액션이 짧은 수명의 Maintainer App 토큰을 발급합니다. 토큰 범위는 Noema 저장소의 metadata read, contents write, pull-request write로 제한됩니다. + +App 토큰 발급 후 게시 runner는 proposal의 staged 경로를 NUL 구분으로 읽고 base64로 정규화한 뒤, GitHub의 완전한 open-PR inventory와 각 PR의 paginated changed-file inventory를 다시 읽습니다. 각 PR의 `changed_files` 수와 실제 조회 파일 수가 일치해야 하고, GitHub API가 지원하는 3,000-file 상한을 넘는 PR은 안전하게 비교할 수 없으므로 실패 폐쇄합니다. proposal 경로와 기존 PR 경로의 교집합이 비어 있어야 하며 `main` SHA도 proposal base와 같아야 원격 브랜치를 만들 수 있습니다. PR을 생성한 뒤에는 방금 생성한 PR을 비교 대상에서 제외하고 나머지 열린 PR 전부에 대해 같은 경로 격리를 다시 검사합니다. 그 사이 새 충돌 PR이 생겼다면 생성한 PR과 전용 브랜치를 정리하고 종료합니다. ## 신뢰할 수 없는 입력과 게시 모델이 만든 `PR_MESSAGE.md`는 신뢰할 수 없는 입력입니다. 파서는 심링크를 거부하고 `O_NOFOLLOW`, inode 안정성, 엄격한 UTF-8, 제어 문자와 양방향 제어 문자 제한, 제목 120바이트, 본문 20,000바이트를 적용합니다. 신뢰된 출력은 mode `0600`으로 기록하고 원본은 commit 전에 삭제합니다. -게시 단계는 실행별 고유 브랜치를 한 번 만들고 한 번 push한 뒤 PR을 한 번 생성합니다. PR 생성 실패 시 orphan 브랜치를 제거합니다. merge, release, publish, deploy 명령은 없습니다. 생성된 PR은 CodeRabbit, OpenCode review, Noema review, `ci`, `reviewer-ci`, Security Scan, branch protection, unresolved-thread 검사와 exact-head 병합 루프로 인계됩니다. +게시 단계는 실행별 고유 브랜치를 한 번 만들고 한 번 push한 뒤 PR을 한 번 생성합니다. PR 생성 실패 시 orphan 브랜치를 제거합니다. 생성한 PR 번호·head SHA·base SHA와 publication marker를 다시 확인하며, 생성 후 queue inventory에 해당 PR이 정확히 한 번 존재해야 합니다. 다른 열린 PR의 존재 자체는 오류가 아니지만 변경 경로 겹침은 오류입니다. merge, release, publish, deploy 명령은 없습니다. 생성된 PR은 CodeRabbit, OpenCode review, Noema review, `ci`, `reviewer-ci`, Security Scan, branch protection, unresolved-thread 검사와 exact-head 병합 루프로 인계됩니다. ## 운영 위험과 롤백 게이트웨이 토큰은 OpenCode 프로세스 안에 존재하므로 명령 거부만으로 microVM egress 경계를 주장하지 않습니다. 지원 가능한 주장은 모델과 쓰기 가능한 저장소 토큰이 공존하지 않고, 신뢰할 수 없는 코드는 게시 자격 증명이 없는 runner에서만 실행되며, 게시 runner는 동일한 immutable patch를 실행 없이 재구성한다는 것입니다. OpenCode는 commit된 저장소 문맥을 오케스트레이터로 보낼 수 있으므로 기밀성, 데이터 보존, 지역, 계약 요건을 별도로 평가해야 합니다. 상위 공급자 선택, 허용 목록, 예산, 회로 차단, 감사는 오케스트레이터에 남습니다. -GitHub에는 다른 PR이 없을 때만 PR을 생성하는 원자적 트랜잭션이 없습니다. 최종 큐와 base 재검증, 고유 브랜치 이름, branch protection, exact-head 리뷰가 남은 경쟁 위험을 통제합니다. 모델 실행을 중지하려면 워크플로를 비활성화하거나 `NOEMA_LLM_API_KEY`를 폐기합니다. 게시만 중지하려면 Maintainer App 키를 폐기합니다. `main`에서 워크플로를 제거하는 것이 코드 롤백이며 기존 `/exchange`, 리뷰, 릴리스, 배포 경로에는 영향을 주지 않습니다. +GitHub에는 "열린 PR들과 경로가 겹치지 않을 때만 새 PR을 생성"하는 원자적 트랜잭션이 없습니다. 게시 직전과 생성 직후의 완전한 경로 inventory 재검증, 정확한 base SHA, 고유 브랜치 이름, force-with-lease, branch protection, exact-head 리뷰가 경쟁 위험을 줄입니다. 다만 서로 다른 파일이 같은 invariant를 깨는 의미적 충돌은 경로 비교만으로 잡을 수 없습니다. 그래서 새 PR도 일반 review→repair→exact-head Checks 절차를 그대로 거치며, 경로 격리를 병합 안전성의 대체물로 사용하지 않습니다. 모델 실행을 중지하려면 워크플로를 비활성화하거나 `NOEMA_LLM_API_KEY`를 폐기합니다. 게시만 중지하려면 Maintainer App 키를 폐기합니다. `main`에서 워크플로를 제거하는 것이 코드 롤백이며 기존 `/exchange`, 리뷰, 릴리스, 배포 경로에는 영향을 주지 않습니다. diff --git a/packages/noema-core/.gitignore b/packages/noema-core/.gitignore new file mode 100644 index 000000000..4ed85d4a5 --- /dev/null +++ b/packages/noema-core/.gitignore @@ -0,0 +1,5 @@ +__pycache__/ +*.pyc +.coverage +.pytest_cache/ +*.egg-info/ diff --git a/packages/noema-core/README.md b/packages/noema-core/README.md new file mode 100644 index 000000000..3c58b2a87 --- /dev/null +++ b/packages/noema-core/README.md @@ -0,0 +1,55 @@ +# noema-core + +Provider-neutral PydanticAI `Agent` construction shared by Noema's per-context +consumers. See [`docs/adr/0014-shared-noema-core-package.md`](../../docs/adr/0014-shared-noema-core-package.md) +for the decision and its scope boundary. + +## What this package is + +One function and one role-neutral identity fragment shared without moving +provider or bounded-context authority into Noema: + +- `build_agent(model, *, system_prompt, output_type=str, deps_type=None, retries=3) -> Agent` + constructs an agent around a caller-supplied, already constructed PydanticAI + `Model`. String model names are rejected so provider/model discovery cannot + occur inside the Shared Kernel. +- `NOEMA_PERSONA` is exactly `"You are Noema"`. Consumers compose that stable + identity with their own precise role, organization context, evidence rules, + tool authority and output contract; the Shared Kernel does not assign a + generic role that could weaken a specialized reviewer or runtime agent. + +The injected model is deliberate. `noema-core` does not construct `AsyncOpenAI`, +`OpenAIChatModel`, `OpenAIProvider`, provider credentials, model discovery, +routing or failover. A consuming bounded context may own a transport adapter to +the published `contextual-orchestrator` interface, but that adapter does not +become Shared Kernel authority. + +## What this package explicitly is not + +It does not own a verdict/output schema, tool/deps machinery, credential +resolution or validation policy, provider SDK, routing policy, provider +fallback, or tenant isolation. Those stay with their canonical owners. + +## Status + +Self-consumption only: `reviewer/noema_reviewer` is the sole consumer today. +`noema-core` is not yet published to an immutable package index, so external +consumers must not pin a mutable branch or copy this source. During this +transition the `noema-reviewer` distribution includes `noema_core` from this +single canonical source path through the custom packaging backend. Wheel and +sdist builds stage a bounded snapshot; editable installs keep an ignored +canonical-source view so their package mapping remains valid after the PEP 660 +hook completes. Required `reviewer-ci` runs this package's 100% line/branch and +docstring gates and validates installed distributions outside the checkout. + +Publishing `noema-core` through the repository's selected immutable package +mechanism and moving consumers to a normal versioned dependency are tracked as +follow-ups in the ADR. + +## Develop + +```bash +pip install -e . +python -m pytest # 100% line+branch coverage gate +python -m interrogate -c pyproject.toml src/noema_core # 100% docstring gate +``` diff --git a/packages/noema-core/pyproject.toml b/packages/noema-core/pyproject.toml new file mode 100644 index 000000000..4da0392f4 --- /dev/null +++ b/packages/noema-core/pyproject.toml @@ -0,0 +1,38 @@ +[build-system] +requires = ["setuptools>=68"] +build-backend = "setuptools.build_meta" + +[project] +name = "noema-core" +version = "0.1.0" +description = "Provider-neutral PydanticAI Agent-construction wiring for Noema's per-context consumers." +requires-python = ">=3.11" +license = "Apache-2.0" +dependencies = [ + "pydantic-ai-slim>=2.9.0,<3", +] + +[dependency-groups] +dev = [ + "pytest>=8.0.0", + "pytest-cov>=5.0.0", + "interrogate>=1.7.0", +] + +[tool.setuptools.packages.find] +where = ["src"] + +[tool.pytest.ini_options] +pythonpath = ["src"] +addopts = "--cov=noema_core --cov-branch --cov-report=term-missing --cov-fail-under=100" + +[tool.coverage.run] +source = ["noema_core"] +omit = ["tests/*"] + +[tool.coverage.report] +show_missing = true + +[tool.interrogate] +fail-under = 100 +exclude = ["tests"] diff --git a/packages/noema-core/src/noema_core/__init__.py b/packages/noema-core/src/noema_core/__init__.py new file mode 100644 index 000000000..24c444caa --- /dev/null +++ b/packages/noema-core/src/noema_core/__init__.py @@ -0,0 +1,13 @@ +"""noema-core: shared PydanticAI Agent-construction wiring for Noema consumers. + +See :mod:`noema_core.agent` for the provider-neutral agent factory and shared +persona fragment. Provider transport and credential wiring stay outside this +Shared Kernel. See ``docs/adr/0014-shared-noema-core-package.md`` in +``ContextualWisdomLab/noema`` for the ownership boundary. +""" + +from __future__ import annotations + +from .agent import NOEMA_PERSONA, build_agent + +__all__ = ["NOEMA_PERSONA", "build_agent"] diff --git a/packages/noema-core/src/noema_core/agent.py b/packages/noema-core/src/noema_core/agent.py new file mode 100644 index 000000000..66bea74d0 --- /dev/null +++ b/packages/noema-core/src/noema_core/agent.py @@ -0,0 +1,62 @@ +"""Shared PydanticAI Agent-construction wiring for Noema's per-context consumers. + +The Shared Kernel centralizes only framework-neutral Noema agent construction +that is safe to reuse across bounded contexts. Provider discovery, endpoint +selection, credentials, provider SDKs, model routing and failover remain outside +this package and are supplied through an already constructed PydanticAI model. + +This package deliberately owns none of a consumer's domain logic: no verdict +schema, no tool/deps machinery, no credential resolution or validation policy, +no tenant isolation. Those stay local to each bounded context. See +``docs/adr/0014-shared-noema-core-package.md`` in +``ContextualWisdomLab/noema`` for the full rationale and scope boundary. +""" + +from __future__ import annotations + +from typing import Any + +from pydantic_ai import Agent +from pydantic_ai.models import Model + + +NOEMA_PERSONA = "You are Noema" +"""The role-neutral identity prefix shared by Noema's bounded-context agents. + +Consumers append their own precise role, organization context, evidence rules, +tool authority and output contract. Keeping this fragment role-neutral avoids +silently broadening a specialized reviewer, runtime agent or application agent +when the shared identity is reused. +""" + + +def build_agent( + model: Model, + *, + system_prompt: str, + output_type: Any = str, + deps_type: Any = None, +) -> Agent[Any, Any]: + """Construct a PydanticAI ``Agent`` around a caller-owned model adapter. + + ``model`` must already be a constructed PydanticAI ``Model`` so provider + discovery, credentials, routing, failover, and retry policy cannot migrate + into Noema's Shared Kernel through PydanticAI convenience configuration. + ``output_type`` (a consumer's verdict/result schema), ``deps_type`` (a + consumer's tool/deps machinery), and ``system_prompt`` (identity plus domain + instructions) remain per-consumer. Model-attempt retry is disabled here; + contextual-orchestrator owns provider/model retry and failover semantics. + """ + if not isinstance(model, Model): + raise TypeError("model must be a constructed PydanticAI Model") + + kwargs: dict[str, Any] = {} + if deps_type is not None: + kwargs["deps_type"] = deps_type + return Agent( + model, + output_type=output_type, + system_prompt=system_prompt, + retries=0, + **kwargs, + ) diff --git a/packages/noema-core/tests/__init__.py b/packages/noema-core/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/packages/noema-core/tests/test_agent.py b/packages/noema-core/tests/test_agent.py new file mode 100644 index 000000000..7d8d715de --- /dev/null +++ b/packages/noema-core/tests/test_agent.py @@ -0,0 +1,53 @@ +"""Tests for the shared provider-neutral Agent-construction wiring.""" + +from __future__ import annotations + +import inspect + +import pytest +from pydantic_ai import Agent +from pydantic_ai.models.test import TestModel + +from noema_core import NOEMA_PERSONA, build_agent + + +def test_build_agent_applies_output_type_and_system_prompt() -> None: + """build_agent constructs an Agent carrying the caller's schema and prompt.""" + agent = build_agent( + TestModel(), + system_prompt=NOEMA_PERSONA, + output_type=str, + ) + assert isinstance(agent, Agent) + result = agent.run_sync("hello") + assert isinstance(result.output, str) + + +def test_build_agent_does_not_expose_retry_policy() -> None: + """Provider/model retry authority cannot leak into the reusable Shared Kernel.""" + assert "retries" not in inspect.signature(build_agent).parameters + + +def test_build_agent_forwards_deps_type_only_when_given() -> None: + """A caller that needs deps machinery can pass deps_type; others get none.""" + agent = build_agent( + TestModel(), + system_prompt=NOEMA_PERSONA, + output_type=str, + deps_type=dict, + ) + assert agent.deps_type is dict + + +def test_build_agent_rejects_unresolved_model_names() -> None: + """Provider/model discovery stays outside noema-core's Shared Kernel.""" + with pytest.raises(TypeError, match="constructed PydanticAI Model"): + build_agent( + "openai:gpt-4o-mini", # type: ignore[arg-type] + system_prompt=NOEMA_PERSONA, + ) + + +def test_noema_persona_is_role_neutral_identity_prefix() -> None: + """Consumers append their bounded-context role without inheriting another role.""" + assert NOEMA_PERSONA == "You are Noema" diff --git a/packages/noema-core/tests/test_owner_boundary.py b/packages/noema-core/tests/test_owner_boundary.py new file mode 100644 index 000000000..ccf9f2b3f --- /dev/null +++ b/packages/noema-core/tests/test_owner_boundary.py @@ -0,0 +1,11 @@ +"""DDD fitness tests for the shared Noema runtime package boundary.""" + +from __future__ import annotations + +import noema_core + + +def test_shared_core_does_not_construct_provider_specific_models() -> None: + """Model/provider transport construction must remain outside Noema's Shared Kernel.""" + + assert not hasattr(noema_core, "build_openai_model") diff --git a/reviewer/MANIFEST.in b/reviewer/MANIFEST.in new file mode 100644 index 000000000..3834c316a --- /dev/null +++ b/reviewer/MANIFEST.in @@ -0,0 +1,2 @@ +include build_backend.py +recursive-include _build_include/noema_core *.py diff --git a/reviewer/README.md b/reviewer/README.md index 851a0a426..1af6e874a 100644 --- a/reviewer/README.md +++ b/reviewer/README.md @@ -14,22 +14,57 @@ Division of responsibility: - **`noema_reviewer`** (this package) — the **judgement** plane. It turns a bounded pull-request manifest into a validated `ReviewVerdict` and can publish it as an independent GitHub review. +- **[`../packages/noema-core`](../packages/noema-core)** — only the shared, + role-neutral PydanticAI `Agent(...)` construction around an already-resolved + caller-owned `Model`, plus a shared `NOEMA_PERSONA` fragment. See + [`docs/adr/0014-shared-noema-core-package.md`](../docs/adr/0014-shared-noema-core-package.md) + for scope. `noema_reviewer` is its only consumer today. Provider/model + discovery, endpoint selection, credentials and failover remain outside the + Shared Kernel; reviewer verdict schema, gating and evidence policy remain + here. ## Contract -The verdict shape is the JSON contract from the sandbox plan: +The verdict shape is the JSON contract from the sandbox plan. Each finding +carries structured actionability rather than relying on free-form prose: ```json { "verdict": "approve | request_changes | blocked", "summary": "…", - "findings": [{"severity": "critical|high|medium|low|info", "path": "…", "line": 1, "evidence": "…", "recommendation": "…"}], + "findings": [{ + "severity": "critical|high|medium|low|info", + "priority": "P1|P2|P3", + "path": "…", + "line": 1, + "check_name": "exact failed check name | null", + "evidence": "…", + "evidence_type": "nearby_implementation|matching_existing_example|cross_file_counterpart|current_official_docs|failed_check_or_log", + "observable_impact": "…", + "trigger": "…", + "recommendation": "smallest fix", + "regression_command": "one exact single-line command", + "suggested_diff": "optional replacement text | null" + }], "suggested_patch_ref": null, "blocked_reasons": [], "confidence": "high | medium | low" } ``` +`check_name` is optional for ordinary source, SARIF, dependency, and review-thread +findings. A finding offered as the RCA for a failed current-head check must bind +to that exact check name. The deterministic gate requires each ordinary failed +check to have its own blocking-severity finding on a current-head changed path +with a positive line; one unrelated or differently bound finding cannot clear +another failed check. + +`regression_command` cannot contain newlines or Markdown backticks. A +`suggested_diff` cannot contain a Markdown fence and is accepted only when its +`path:line` is a right-side anchor in the exact PR diff. Accepted replacement +text is sent through GitHub's inline review `comments` payload as a suggestion, +not merely printed in the top-level review body. + The following guarantees are enforced deterministically around the LLM (`gating.py`), so they hold regardless of what the model says: @@ -70,7 +105,7 @@ The following guarantees are enforced deterministically around the LLM a repository probe. The primary explore query preserves each selected changed path in full instead of truncating individual path identities; it admits at most 80 changed files and 24,079 aggregate characters. The manifest retains - bounded current-head file content for every selected file through that same + bounded current-head file context for every selected file through that same 80-file canonical scope; above 80 files both semantic scope and changed-file context fail closed rather than reviewing a historical 12-file prefix. Exceeding either exact-scope budget fails closed instead of querying a prefix. @@ -91,40 +126,46 @@ The following guarantees are enforced deterministically around the LLM unchanged lookalike path become a retrieval seed. The node output never counts as review evidence by itself; deleted, unresolved, symlinked-component, unindexed, or symbol-less paths leave the original empty result fail closed. - The local host-process CodeGraph fallback also builds a closed execution - environment instead of copying the parent environment: only `PATH` and locale - discovery variables may be propagated; `HOME`, `TEMP`, `TMP`, and `TMPDIR` - are replaced by one fresh per-command private temporary directory and - `NO_COLOR=1` is set explicitly. Process injection, host user configuration/ - credentials, ambient temporary-directory capabilities, credential-helper/ - socket, container/Kubernetes, proxy, arbitrary workflow, and provider - variables such as `NODE_OPTIONS`, `GIT_ASKPASS`, `SSH_AUTH_SOCK`, + The local host-process CodeGraph fallback builds a closed execution + environment instead of copying the parent environment: only `PATH` and + locale discovery variables may be propagated; `HOME`, `TEMP`, `TMP`, and + `TMPDIR` are replaced by one fresh per-command private temporary directory + and `NO_COLOR=1` is set explicitly. Process injection, host user + configuration/credentials, ambient temporary-directory capabilities, + credential-helper/socket, container/Kubernetes, proxy, arbitrary workflow, + and provider variables such as `NODE_OPTIONS`, `GIT_ASKPASS`, `SSH_AUTH_SOCK`, `DOCKER_CONFIG`, `KUBECONFIG`, and `HTTPS_PROXY` are not ambient CodeGraph - authority. Production central review still uses the separately attested no- - network sandbox; this host fallback does not replace that isolation boundary. - The production `DockerCodeGraphRunner` now owns the same semantic wrapper and - passes both the exact symbol probe and any symbol-seeded second `explore` - through its verified no-network container boundary. It extracts only the - trusted sandbox copy receipt and sole explore stdout section before semantic - classification, so setup/status bytes cannot satisfy the strict gate and an - empty production explore cannot silently fall back to a host CodeGraph + authority. Production central review still uses the separately attested + no-network sandbox; this host fallback does not replace that isolation + boundary. The production `DockerCodeGraphRunner` owns the same semantic + wrapper and passes both the exact symbol probe and any symbol-seeded second + `explore` through its verified no-network container boundary. It extracts + only the trusted sandbox copy receipt and sole explore stdout section before + semantic classification, so setup/status bytes cannot satisfy the strict gate + and an empty production explore cannot silently fall back to a host CodeGraph process. 2. **MEDIUM-or-higher dependency findings can't ride out on an approve.** An unresolved OSV/Trivy/dependency-review finding at MEDIUM+ downgrades an approval to `request_changes` with the finding attached — the org rule is "remediate by bump, not gate weakening". -3. **Current-head failures remain blocking.** Failed GitHub Checks and - MEDIUM-or-higher code-scanning/SARIF alerts deterministically downgrade an - approval and retain their exact job, rule, path, and bounded log evidence. -4. **Reviewer independence cannot deadlock.** The exact reviewer check names +3. **Current-head failures remain blocking until causally mapped.** Every + ordinary failed GitHub Check remains `blocked` unless its exact check name is + bound to its own current-head changed-file, positive-line blocking RCA. + Check-run names or workflow URLs are not synthesized into source findings. + MEDIUM-or-higher code-scanning/SARIF alerts remain deterministic findings. +4. **Suggestions must be executable review artifacts.** Suggested replacement + text is rejected before publication if GitHub cannot attach it to the exact + right side of the reviewed diff; fence injection and multiline regression + commands fail schema validation. +5. **Reviewer independence cannot deadlock.** The exact reviewer check names `noema-review` and `opencode-review`, plus the downstream - `metadata-only gate evaluation`, are excluded from Noema's deterministic - failed-check gate because they cannot be prerequisites for the review that - produces them. This cycle exception cannot satisfy strict evidence by itself: - at least one current-head check outside that reviewer-dependent set must be - observed. Similarly named checks remain blocking, as do every other failed - check and unresolved non-outdated inline thread. -5. **Long reviews stay useful.** The production provider request timeout + `metadata-only gate evaluation`, are excluded from Noema's failed-check RCA + gate because they cannot be prerequisites for the review that produces them. + This cycle exception cannot satisfy strict evidence by itself: at least one + current-head check outside that reviewer-dependent set must be observed. + Similarly named checks remain blocking, as do every other failed check and + unresolved non-outdated inline thread. +6. **Long reviews stay useful.** The production provider request timeout defaults to 5,400 seconds and provider 429/5xx responses receive bounded SDK retries. Production failover belongs inside `contextual-orchestrator`; Noema does not sequentially try the next model. Publication re-reads the live PR @@ -133,8 +174,11 @@ The following guarantees are enforced deterministically around the LLM The GitHub manifest fetch covers all inline review threads (including resolved and outdated state), submitted review bodies, conversation comments, failed current-head workflow logs, current-head code-scanning alerts, and open -Dependabot package advisories. Evidence-fetch errors are part of the manifest, -not silent empty lists. +Dependabot package advisories. Failed-check log collection derives an Actions +Job id only from an exact repository-bound GitHub `details_url`; a Check Run id +is never reused as a Job id. If the Actions log cannot be obtained, collection +falls back to the same Check Run's bounded annotations. Evidence-fetch errors +are part of the manifest, not silent empty lists. The driver sits behind the small `ReviewAgent` protocol, so the sandbox plan's "Codex, OpenCode, PydanticAI, or another driver" swap is a one-line change. @@ -187,5 +231,18 @@ python -m pytest # 100% line+branch coverage gate python -m interrogate -c pyproject.toml noema_reviewer # 100% docstring gate ``` +The shared source remains canonical at `../packages/noema-core/src/noema_core`. +Until `noema-core` has an immutable index release, the reviewer wheel includes +that module directly from the canonical monorepo path through setuptools package +mapping. A normal wheel install therefore provides both `noema_reviewer` and +`noema_core`; callers do not need an ambient `PYTHONPATH`. Required +`reviewer-ci` builds and installs the wheel in a clean temporary environment and +imports both packages before the artifact is considered valid. + +Evidence-only package imports are intentionally lazy: importing +`noema_reviewer.github_io` or `noema_reviewer.sandbox` does not load the model +construction layer. Actual model execution still imports `noema_core` through +the package-level agent API. + Tests drive the agent with PydanticAI's offline `TestModel`/`FunctionModel` and a stub `gh` runner — no network, no secret, no real model. diff --git a/reviewer/build_backend.py b/reviewer/build_backend.py new file mode 100644 index 000000000..d68d1513c --- /dev/null +++ b/reviewer/build_backend.py @@ -0,0 +1,307 @@ +"""PEP 517/660 wrapper that stages canonical noema-core for reviewer builds. + +The reviewer cannot declare an immutable external ``noema-core`` dependency until +that package is published. Distribution hooks therefore build from a private +per-invocation copy of the reviewer project containing one canonical noema-core +snapshot. Editable hooks keep one ignored symlink to canonical monorepo source, +so distribution cleanup cannot invalidate an existing editable installation. +""" + +from __future__ import annotations + +from contextlib import contextmanager +import json +import os +from pathlib import Path +from shutil import copytree, ignore_patterns, rmtree +import subprocess +import sys +from tempfile import TemporaryDirectory +from threading import RLock +from typing import Any, Callable, Iterator, TypeVar, cast + +from setuptools import build_meta as _setuptools + +_PROJECT_ROOT = Path(__file__).resolve().parent +_CANONICAL_CORE = _PROJECT_ROOT.parent / "packages" / "noema-core" / "src" / "noema_core" +_STAGING_ROOT = _PROJECT_ROOT / "_build_include" +_STAGED_CORE = _STAGING_ROOT / "noema_core" +_EDITABLE_BUILD_LOCK = RLock() +_BUILD_RESULT = TypeVar("_BUILD_RESULT") +_STAGED_BACKEND_PROGRAM = """ +from __future__ import annotations + +import importlib +import json +from pathlib import Path +import sys + +hook_name, result_path, args_payload, kwargs_payload = sys.argv[1:] +backend = importlib.import_module("setuptools.build_meta") +result = getattr(backend, hook_name)( + *json.loads(args_payload), + **json.loads(kwargs_payload), +) +Path(result_path).write_text(json.dumps(result), encoding="utf-8") +""" + + +def _remove_generated_path(path: Path) -> None: + """Remove a generated file, symlink, or directory without following links.""" + + if path.is_symlink() or path.is_file(): + path.unlink(missing_ok=True) + elif path.exists(): + rmtree(path) + + +def _reset_staging_root() -> None: + """Recreate the editable package view without following stale path aliases.""" + + _remove_generated_path(_STAGING_ROOT) + _STAGING_ROOT.mkdir(parents=True) + + +def _prepare_editable_core() -> None: + """Expose canonical noema-core to editable installs through a live source link. + + Editable packaging must never fall back to a copied snapshot because such a + copy silently stops reflecting edits to the canonical Shared Kernel. A host + that cannot create the directory link fails explicitly instead. + """ + + if not _CANONICAL_CORE.is_dir(): + if _STAGED_CORE.is_dir(): + return + raise RuntimeError("canonical noema-core source is unavailable for reviewer editable install") + + if _STAGED_CORE.is_symlink(): + try: + points_to_canonical = ( + _STAGED_CORE.resolve(strict=True) == _CANONICAL_CORE.resolve(strict=True) + ) + except OSError: + # Broken or inaccessible prior links are non-authoritative and must be restaged. + points_to_canonical = False + if points_to_canonical: + return + + _reset_staging_root() + try: + _STAGED_CORE.symlink_to(_CANONICAL_CORE, target_is_directory=True) + except OSError as error: + _remove_generated_path(_STAGING_ROOT) + raise RuntimeError( + "reviewer editable install requires a live symlink to canonical noema-core source" + ) from error + + +def _distribution_source_core() -> Path: + """Return the canonical or embedded noema-core source used for a distribution.""" + + if _CANONICAL_CORE.is_dir(): + return _CANONICAL_CORE + if _STAGED_CORE.is_dir(): + return _STAGED_CORE + raise RuntimeError("canonical noema-core source is unavailable for reviewer packaging") + + +@contextmanager +def _distribution_project() -> Iterator[Path]: + """Yield a private reviewer project containing one exact shared-core snapshot. + + The caller gets a distinct filesystem tree for each invocation. This keeps + concurrent wheel, sdist, metadata, and requirement hooks from deleting or + overwriting one another's package staging. + """ + + source_core = _distribution_source_core() + with TemporaryDirectory(prefix="noema-reviewer-build-") as temporary_root: + project_root = Path(temporary_root) / "reviewer" + copytree( + _PROJECT_ROOT, + project_root, + ignore=ignore_patterns( + "_build_include", + "__pycache__", + ".pytest_cache", + "*.egg-info", + "build", + "dist", + ), + ) + staged_core = project_root / "_build_include" / "noema_core" + staged_core.parent.mkdir(parents=True, exist_ok=True) + copytree(source_core, staged_core, symlinks=False) + yield project_root + + +def _distribution_child_environment(project_root: Path) -> dict[str, str]: + """Preserve the frontend-provided isolated backend paths for the staged child. + + PEP 517 frontends can expose build requirements through interpreter search + paths rather than a dedicated virtualenv executable. Launching a nested + ``sys.executable`` without those paths can silently import an unrelated host + setuptools and produce ``UNKNOWN-0.0.0`` artifacts. The staged project stays + first, while the current backend process's search paths carry the frontend's + already-admitted build dependencies into the fresh interpreter. + """ + + child_environment = os.environ.copy() + search_paths = [str(project_root)] + for search_path in sys.path: + if search_path and search_path not in search_paths: + search_paths.append(search_path) + child_environment["PYTHONPATH"] = os.pathsep.join(search_paths) + return child_environment + + +def _run_distribution_hook( + hook_name: str, + *args: Any, + **kwargs: Any, +) -> _BUILD_RESULT: + """Invoke setuptools in a fresh process whose project root is the staged copy. + + ``setuptools.build_meta`` is project-context-sensitive. Reusing the module + imported for the checkout after merely changing process cwd can retain the + wrong distribution identity and emit ``UNKNOWN-0.0.0`` artifacts. A child + interpreter imports the public backend only after entering the private + staged project. Its environment explicitly preserves the parent PEP 517 + backend search paths so the child cannot fall back to an unrelated host + setuptools, while independent build invocations retain separate cwd and + module state. + """ + + with _distribution_project() as project_root: + result_path = project_root.parent / "backend-result.json" + subprocess.run( + [ + sys.executable, + "-c", + _STAGED_BACKEND_PROGRAM, + hook_name, + str(result_path), + json.dumps(args), + json.dumps(kwargs), + ], + cwd=project_root, + env=_distribution_child_environment(project_root), + check=True, + ) + if not result_path.is_file(): + raise RuntimeError(f"staged setuptools hook {hook_name!r} produced no result") + return cast(_BUILD_RESULT, json.loads(result_path.read_text(encoding="utf-8"))) + + +def _with_editable_core( + builder: Callable[..., _BUILD_RESULT], + *args: Any, + **kwargs: Any, +) -> _BUILD_RESULT: + """Run an editable hook while retaining its live canonical source view.""" + + with _EDITABLE_BUILD_LOCK: + _prepare_editable_core() + return builder(*args, **kwargs) + + +def _absolute_path(path: str | None) -> str | None: + """Preserve frontend output-directory identity across private-project builds.""" + + if path is None: + return None + return str(Path(path).resolve()) + + +def build_wheel( + wheel_directory: str, + config_settings: dict[str, Any] | None = None, + metadata_directory: str | None = None, +) -> str: + """Build a reviewer wheel containing the staged canonical noema-core snapshot.""" + + return _run_distribution_hook( + "build_wheel", + _absolute_path(wheel_directory), + config_settings, + _absolute_path(metadata_directory), + ) + + +def build_editable( + wheel_directory: str, + config_settings: dict[str, Any] | None = None, + metadata_directory: str | None = None, +) -> str: + """Build an editable reviewer wheel against the canonical shared-core source.""" + + return _with_editable_core( + _setuptools.build_editable, + _absolute_path(wheel_directory), + config_settings, + _absolute_path(metadata_directory), + ) + + +def build_sdist( + sdist_directory: str, + config_settings: dict[str, Any] | None = None, +) -> str: + """Build a self-contained source distribution from canonical monorepo source.""" + + return _run_distribution_hook( + "build_sdist", + _absolute_path(sdist_directory), + config_settings, + ) + + +def prepare_metadata_for_build_wheel( + metadata_directory: str, + config_settings: dict[str, Any] | None = None, +) -> str: + """Prepare wheel metadata in a backend imported from the staged project root.""" + + return _run_distribution_hook( + "prepare_metadata_for_build_wheel", + _absolute_path(metadata_directory), + config_settings, + ) + + +def prepare_metadata_for_build_editable( + metadata_directory: str, + config_settings: dict[str, Any] | None = None, +) -> str: + """Prepare editable metadata against the canonical shared-core source view.""" + + return _with_editable_core( + _setuptools.prepare_metadata_for_build_editable, + _absolute_path(metadata_directory), + config_settings, + ) + + +def get_requires_for_build_wheel( + config_settings: dict[str, Any] | None = None, +) -> list[str]: + """Return wheel-build requirements from a staged-project backend context.""" + + return _run_distribution_hook("get_requires_for_build_wheel", config_settings) + + +def get_requires_for_build_editable( + config_settings: dict[str, Any] | None = None, +) -> list[str]: + """Return editable requirements after validating canonical package availability.""" + + return _with_editable_core(_setuptools.get_requires_for_build_editable, config_settings) + + +def get_requires_for_build_sdist( + config_settings: dict[str, Any] | None = None, +) -> list[str]: + """Return sdist-build requirements from a staged-project backend context.""" + + return _run_distribution_hook("get_requires_for_build_sdist", config_settings) diff --git a/reviewer/noema_reviewer/__init__.py b/reviewer/noema_reviewer/__init__.py index 02e6bb78f..d61d6fa8e 100644 --- a/reviewer/noema_reviewer/__init__.py +++ b/reviewer/noema_reviewer/__init__.py @@ -6,13 +6,19 @@ publish it as an independent GitHub review, satisfying the organization's two-reviewer merge rule alongside OpenCode. The Noema Cloudflare Worker remains the token-exchange boundary; this package is the judgement plane. + +Agent-construction exports are loaded lazily so evidence-only modules can run +without importing the model runtime. That keeps collection and sandbox evidence +paths independent from the shared ``noema_core`` package while preserving the +existing package-level reviewer API for actual model execution. """ from __future__ import annotations -from .agent import PydanticAIReviewAgent, ReviewAgent, build_agent +from typing import Any + from .manifest import ReviewManifest -from .models import Confidence, Finding, ReviewVerdict, Severity, Verdict +from .models import Confidence, EvidenceType, Finding, Priority, ReviewVerdict, Severity, Verdict from .patch_image_validation import ( DockerPatchValidatorImageRunner, PatchValidatorImageProfile, @@ -30,11 +36,24 @@ inspect_patch_bytes, ) +_AGENT_EXPORTS = frozenset({"PydanticAIReviewAgent", "ReviewAgent", "build_agent"}) + + +def __getattr__(name: str) -> Any: + """Load model-runtime exports only when callers request those symbols.""" + + if name in _AGENT_EXPORTS: + from . import agent + + return getattr(agent, name) + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") + __all__ = [ "Confidence", "DockerPatchValidationRunner", "DockerPatchValidatorImageRunner", + "EvidenceType", "Finding", "PatchValidationProfile", "PatchValidationRequest", @@ -45,6 +64,7 @@ "PatchValidatorImageResult", "PatchValidatorImageStatus", "PydanticAIReviewAgent", + "Priority", "ReviewAgent", "ReviewManifest", "ReviewVerdict", diff --git a/reviewer/noema_reviewer/agent.py b/reviewer/noema_reviewer/agent.py index dc7d24b7a..ab25438e5 100644 --- a/reviewer/noema_reviewer/agent.py +++ b/reviewer/noema_reviewer/agent.py @@ -12,6 +12,8 @@ from typing import Protocol, runtime_checkable +from noema_core import NOEMA_PERSONA +from noema_core import build_agent as build_core_agent from pydantic_ai import Agent from pydantic_ai.models import Model @@ -22,7 +24,7 @@ SYSTEM_PROMPT = ( - "You are Noema, an independent second reviewer for ContextualWisdomLab, " + f"{NOEMA_PERSONA}, an independent second reviewer for ContextualWisdomLab, " "separate from the OpenCode reviewer. You review a bounded manifest of a " "pull request: its diff, changed-file context, workflow logs, SARIF " "summary, dependency findings, prior review comments, and current check " @@ -30,9 +32,16 @@ "regressions from that evidence only. Approve when no blocking issue 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." + "line for each finding. For every failed check, read its current-head log or " + "annotation, trace the failure to an exact repository path and positive line, " + "set finding.check_name to that exact current-head check name, and state " + "P1/P2/P3 priority, evidence type, observable impact, trigger, smallest fix, " + "and an exact regression command in the finding. Include minimal replacement " + "text in suggested_diff when the cited line can be fixed directly; one finding " + "must not stand in for multiple failed checks. A check name, workflow URL, or " + "synthetic .github/checks path is not actionable. Use blocked when logs cannot " + "support that mapping rather than guessing. Never approve while an unresolved " + "MEDIUM-or-higher dependency finding is present; require a package bump instead." ) @@ -101,13 +110,12 @@ 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).""" - self._agent: Agent[None, ReviewVerdict] = Agent( + def __init__(self, model: Model) -> None: + """Build the agent around an already resolved real or test model.""" + self._agent: Agent[None, ReviewVerdict] = build_core_agent( model, output_type=ReviewVerdict, system_prompt=SYSTEM_PROMPT, - retries=3, ) def review(self, manifest: ReviewManifest, *, strict: bool = False) -> ReviewVerdict: @@ -123,7 +131,8 @@ def build_agent(config: ReviewerConfig | None = None) -> PydanticAIReviewAgent: Configuration (model name, orchestrator base URL, API key) is resolved through :func:`resolve_model`, which follows the org KV-first rule and fails loudly when the model provider or credential is unavailable — the - reviewer never degrades to a silent approval. + reviewer never degrades to a silent approval. Provider/model retries and + failover stay with contextual-orchestrator rather than this reviewer. """ model = resolve_model(config) return PydanticAIReviewAgent(model) diff --git a/reviewer/noema_reviewer/config.py b/reviewer/noema_reviewer/config.py index d3d6861f6..eda219a75 100644 --- a/reviewer/noema_reviewer/config.py +++ b/reviewer/noema_reviewer/config.py @@ -7,9 +7,10 @@ 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 +The reviewer talks to an OpenAI-compatible endpoint exposed by +``contextual-orchestrator`` in production. Upstream model selection, provider +routing and failover stay in that gateway; this module owns only the reviewer's +transport adapter. Leftover sequential ``NOEMA_FALLBACK_*`` settings fail closed instead of trying the next model inside Noema. """ @@ -151,11 +152,13 @@ def resolve_config(credential_getter: CredentialGetter | None = None) -> Reviewe def resolve_model(config: ReviewerConfig | None = None) -> Model: - """Build an OpenAI-compatible PydanticAI model from resolved configuration. + """Build the reviewer's transport adapter to contextual-orchestrator. - 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. + The OpenAI-compatible client exists only as this bounded-context adapter to + the orchestrator endpoint. It does not select a provider, discover models, + or implement fallback; those authorities remain in contextual-orchestrator. + The shared ``noema_core`` package receives the resulting PydanticAI model by + injection and therefore has no provider SDK or credential surface. """ from openai import AsyncOpenAI from pydantic_ai.models.openai import OpenAIChatModel diff --git a/reviewer/noema_reviewer/gating.py b/reviewer/noema_reviewer/gating.py index 76dbc4ea7..b8f699fb6 100644 --- a/reviewer/noema_reviewer/gating.py +++ b/reviewer/noema_reviewer/gating.py @@ -1,23 +1,29 @@ """Deterministic safety 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: +The LLM driver produces a judgement, but repository 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". +3. Every ordinary failed current-head check needs its own source-bound RCA before + the reviewer may publish ``request_changes`` instead of ``blocked``. """ from __future__ import annotations +import re + from .manifest import ReviewManifest from .models import ( BLOCKING_SEVERITIES, Confidence, + EvidenceType, Finding, + Priority, ReviewVerdict, Severity, Verdict, @@ -33,6 +39,43 @@ REVIEW_DEPENDENT_CHECK_NAMES = frozenset( {"noema-review", "opencode-review", "metadata-only gate evaluation"} ) +HUNK_HEADER_RE = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@") + + +def _right_side_diff_lines(diff: str) -> set[tuple[str, int]]: + """Return right-side path/line anchors accepted by GitHub review comments.""" + anchors: set[tuple[str, int]] = set() + path: str | None = None + line_number: int | None = None + for line in diff.splitlines(): + if line.startswith("+++ b/"): + path = line[6:] + line_number = None + continue + hunk = HUNK_HEADER_RE.match(line) + if hunk: + line_number = int(hunk.group(1)) + continue + if path is None or line_number is None or not line: + continue + if line[0] in {" ", "+"}: + anchors.add((path, line_number)) + line_number += 1 + elif line[0] != "-": + line_number = None + return anchors + + +def invalid_suggestion_reasons(manifest: ReviewManifest, verdict: ReviewVerdict) -> list[str]: + """Reject suggestions GitHub cannot attach to this exact PR diff.""" + anchors = _right_side_diff_lines(manifest.diff) + return [ + "suggested diff is not anchored to a current-head right-side diff line: " + f"{finding.path}:{finding.line or 'missing'}" + for finding in verdict.findings + if finding.suggested_diff and (finding.path, finding.line) not in anchors + ] + CODEGRAPH_EXPLORE_MARKER = "## codegraph explore" RAW_CODEGRAPH_EXPLORE_MARKER = "[raw codegraph explore marker]" @@ -121,24 +164,12 @@ def missing_evidence(manifest: ReviewManifest) -> list[str]: token for line in classification_lines for token in line.split() ) 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) elif explore_marker_count > 1: - # The production wrapper emits exactly one provenance marker. A second - # marker can only come from untrusted output or a malformed prepared - # manifest, so strict review cannot choose which section is authoritative. reasons.append("CodeGraph semantic query has ambiguous provenance") elif normalized_final_explore.startswith("no relevant code found"): - # Classify the explicit CodeGraph empty-result response only when it is - # the semantic response prefix after known lifecycle and wrapper - # annotations are removed. Source/code context may legitimately contain - # the same words and must not erase independently retained semantic bytes. - # Collapse every Unicode whitespace run first so formatting cannot - # disguise the actual empty-result response. reasons.append("CodeGraph semantic query returned no relevant code") elif not _has_semantic_codegraph_context(manifest): reasons.append("CodeGraph semantic query produced no review context") @@ -168,12 +199,17 @@ def dependency_findings_as_review(manifest: ReviewManifest) -> list[Finding]: findings.append( Finding( severity=dependency.severity, + priority=Priority.P1 if dependency.severity is Severity.CRITICAL else Priority.P2, path=dependency.package_name, evidence=( f"{dependency.tool} reported {dependency.package_name}" f"@{dependency.installed_version or 'current'}{identifier}" ), + evidence_type=EvidenceType.FAILED_CHECK, + observable_impact="The pull request would retain a known vulnerable dependency.", + trigger="Installing the dependency set recorded by the current lockfile.", recommendation=f"Bump {dependency.package_name} to {fixed} and refresh the lockfile.", + regression_command="uv run pip-audit", ) ) return findings @@ -188,31 +224,53 @@ def security_findings_as_review(manifest: ReviewManifest) -> list[Finding]: findings.append( Finding( severity=security.severity, + priority=(Priority.P1 if security.severity in {Severity.CRITICAL, Severity.HIGH} else Priority.P2), 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 "") ), + evidence_type=EvidenceType.FAILED_CHECK, + observable_impact="The current-head security gate remains failed.", + trigger=f"Running the {security.tool} scanner against the current head.", recommendation="Remediate the current-head scanner finding and rerun code scanning.", + regression_command="gh pr checks --watch", ) ) return findings -def failed_checks_as_review(manifest: ReviewManifest) -> list[Finding]: - """Convert every observed non-success current-head check into a review finding.""" - return [ - Finding( - severity=Severity.HIGH, - path=f".github/checks/{check.name}", - evidence=f"Current-head check concluded {check.conclusion}; see bounded workflow_logs.", - recommendation="Require terminal success for the current-head check before approval.", - ) +def failed_check_blockers( + manifest: ReviewManifest, + verdict: ReviewVerdict | None = None, +) -> list[str]: + """Return failed checks without their own actionable current-head source RCA.""" + failed = [ + check.name for check in manifest.check_conclusions if check.name not in REVIEW_DEPENDENT_CHECK_NAMES and check.conclusion.lower() != "success" ] + if verdict is None: + unresolved = failed + else: + changed_paths = {changed.path for changed in manifest.changed_files} + actionable_checks = { + finding.check_name + for finding in verdict.findings + if finding.check_name is not None + and finding.severity in BLOCKING_SEVERITIES + and finding.path in changed_paths + and isinstance(finding.line, int) + and not isinstance(finding.line, bool) + and finding.line > 0 + } + unresolved = [name for name in failed if name not in actionable_checks] + return [ + f"failed check {name} lacks an actionable current-head path:line finding" + for name in unresolved + ] def unresolved_threads_as_review(manifest: ReviewManifest) -> list[Finding]: @@ -220,10 +278,15 @@ def unresolved_threads_as_review(manifest: ReviewManifest) -> list[Finding]: return [ Finding( severity=Severity.HIGH, + priority=Priority.P1, path=comment.path or ".github/review-threads", line=comment.line, evidence=f"Unresolved review thread by {comment.author}: {comment.body}", + evidence_type=EvidenceType.NEARBY_IMPLEMENTATION, + observable_impact="The current head retains a reviewer-confirmed defect.", + trigger="Merging while the current inline review thread remains unresolved.", recommendation="Resolve the cited review thread with a current-head fix or response.", + regression_command="gh pr checks --watch", ) for comment in manifest.review_comments if comment.kind == "thread" and comment.state == "open" @@ -238,25 +301,10 @@ def _enforce_findings( """Merge distinct deterministic findings and prevent an approval from hiding them.""" if not findings or verdict.verdict is Verdict.BLOCKED: return verdict - existing = { - ( - finding.severity, - finding.path, - finding.line, - finding.evidence, - finding.recommendation, - ) - for finding in verdict.findings - } + existing = {finding.model_dump_json() for finding in verdict.findings} merged = list(verdict.findings) for finding in findings: - identity = ( - finding.severity, - finding.path, - finding.line, - finding.evidence, - finding.recommendation, - ) + identity = finding.model_dump_json() if identity not in existing: merged.append(finding) existing.add(identity) @@ -277,11 +325,7 @@ def enforce_security_and_check_gates( 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) - ) + deterministic = security_findings_as_review(manifest) + unresolved_threads_as_review(manifest) return _enforce_findings( verdict, deterministic, @@ -316,9 +360,15 @@ def apply_gates( The dependency gate always runs so an approval can never bury an unresolved MEDIUM-or-higher vulnerability. """ + suggestion_reasons = invalid_suggestion_reasons(manifest, verdict) + if suggestion_reasons: + return blocked_verdict(suggestion_reasons) if strict: reasons = missing_evidence(manifest) if reasons: return blocked_verdict(reasons) + failed_checks = failed_check_blockers(manifest, verdict) + if failed_checks: + return blocked_verdict(failed_checks) check_gated = enforce_security_and_check_gates(manifest, verdict) return enforce_dependency_gate(manifest, check_gated) diff --git a/reviewer/noema_reviewer/github_io.py b/reviewer/noema_reviewer/github_io.py index 557edfa5b..c2c5dccb2 100644 --- a/reviewer/noema_reviewer/github_io.py +++ b/reviewer/noema_reviewer/github_io.py @@ -15,7 +15,7 @@ import subprocess import tempfile from collections.abc import Callable, Sequence -from urllib.parse import quote +from urllib.parse import quote, urlparse from .manifest import ( ChangedFile, @@ -423,7 +423,7 @@ def _fetch_failed_workflow_logs(repo: str, head_sha: str, runner: GhRunner) -> s '.check_runs[] | select(.conclusion == "failure" or ' '.conclusion == "cancelled" or .conclusion == "timed_out" or ' '.conclusion == "action_required" or .conclusion == "startup_failure") ' - "| {id: .id, name: .name, conclusion: .conclusion}" + "| {id: .id, name: .name, conclusion: .conclusion, details_url: .details_url}" ), ], None, @@ -435,20 +435,52 @@ def _fetch_failed_workflow_logs(repo: str, head_sha: str, runner: GhRunner) -> s continue node = json.loads(line) check_id = node.get("id") - if not check_id: + if not isinstance(check_id, int) or isinstance(check_id, bool) or check_id <= 0: continue name = str(node.get("name") or "unnamed check") conclusion = str(node.get("conclusion") or "failure") + job_id = _github_actions_job_id(repo, node.get("details_url")) try: - log = runner(["gh", "api", f"repos/{repo}/actions/jobs/{check_id}/logs"], None) + if job_id is None: + raise RuntimeError("check details did not identify a repository-bound Actions job") + log = runner(["gh", "api", f"repos/{repo}/actions/jobs/{job_id}/logs"], None) except RuntimeError as exc: - log = f"[log unavailable: {_failure_reason(name, exc)}]" + try: + annotations = runner( + [ + "gh", + "api", + "--paginate", + f"repos/{repo}/check-runs/{check_id}/annotations?per_page=100", + "--jq", + r'.[] | "\(.path // \"\"):\(.start_line // 0): \(.annotation_level // \"failure\"): \(.message // \"\")"', + ], + None, + ) + except RuntimeError: + annotations = "" + log = annotations.strip() or f"[log unavailable: {_failure_reason(name, exc)}]" excerpts.append(f"## {name} ({conclusion})\n{_truncate(log, 8000)}") if not excerpts: return f"No failed GitHub Actions checks were reported for current head {head_sha}." return _truncate("\n\n".join(excerpts), MAX_WORKFLOW_LOG_CHARS) +def _github_actions_job_id(repo: str, details_url: object) -> int | None: + """Return the Actions job id from an exact repository-bound GitHub URL.""" + if not isinstance(details_url, str): + return None + parsed = urlparse(details_url) + if parsed.scheme != "https" or parsed.netloc.casefold() != "github.com": + return None + match = re.fullmatch( + rf"/{re.escape(repo)}/actions/runs/[1-9][0-9]*/job/([1-9][0-9]*)/?", + parsed.path, + flags=re.IGNORECASE, + ) + return int(match.group(1)) if match else None + + def _severity_from_github(raw: str) -> Severity: """Normalize GitHub and Dependabot severity labels conservatively.""" normalized = raw.strip().lower() @@ -684,12 +716,26 @@ def _fetch_codegraph_status( def render_review_body(verdict: ReviewVerdict, head_sha: str, token_source: str) -> str: """Render the PR review body, including the interop marker the central gate detects.""" - finding_lines = [ - f"- [{finding.severity.value}] {finding.path}" - + (f":{finding.line}" if finding.line else "") - + f": {finding.recommendation} ({finding.evidence})" - for finding in verdict.findings - ] or ["- No blocking findings."] + finding_lines: list[str] = [] + for finding in verdict.findings: + location = finding.path + (f":{finding.line}" if finding.line else "") + finding_lines.extend( + [ + f"#### [{finding.priority.value}] {location}", + f"- Severity: {finding.severity.value}", + f"- Evidence type: {finding.evidence_type.value}", + f"- Evidence: {finding.evidence}", + f"- Observable impact: {finding.observable_impact}", + f"- Trigger: {finding.trigger}", + f"- Smallest fix: {finding.recommendation}", + f"- Regression: `{finding.regression_command}`", + ] + ) + if finding.suggested_diff: + finding_lines.extend(["", "```suggestion", finding.suggested_diff, "```"]) + finding_lines.append("") + if not finding_lines: + finding_lines = ["- No blocking findings."] blocked_lines = [f"- {reason}" for reason in verdict.blocked_reasons] body = [ "## Noema PydanticAI review", @@ -752,6 +798,16 @@ def publish_verdict( "commit_id": head_sha, "event": event, "body": render_review_body(verdict, head_sha, token_source), + "comments": [ + { + "path": finding.path, + "line": finding.line, + "side": "RIGHT", + "body": f"```suggestion\n{finding.suggested_diff}\n```", + } + for finding in verdict.findings + if finding.suggested_diff and finding.line + ], } runner( ["gh", "api", "-X", "POST", f"repos/{repo}/pulls/{pr_number}/reviews", "--input", "-"], diff --git a/reviewer/noema_reviewer/models.py b/reviewer/noema_reviewer/models.py index 3962b9807..a054f2156 100644 --- a/reviewer/noema_reviewer/models.py +++ b/reviewer/noema_reviewer/models.py @@ -11,7 +11,7 @@ from enum import Enum -from pydantic import BaseModel, Field, model_validator +from pydantic import BaseModel, Field, field_validator, model_validator class Verdict(str, Enum): @@ -40,6 +40,24 @@ class Confidence(str, Enum): LOW = "low" +class Priority(str, Enum): + """Review priority compatible with actionable PR-review conventions.""" + + P1 = "P1" + P2 = "P2" + P3 = "P3" + + +class EvidenceType(str, Enum): + """The source that independently supports a finding.""" + + NEARBY_IMPLEMENTATION = "nearby_implementation" + MATCHING_EXAMPLE = "matching_existing_example" + CROSS_FILE_COUNTERPART = "cross_file_counterpart" + OFFICIAL_DOCS = "current_official_docs" + FAILED_CHECK = "failed_check_or_log" + + # 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. @@ -54,17 +72,71 @@ class Finding(BaseModel): """A single reviewer-facing issue tied to concrete evidence.""" severity: Severity = Field(description="How serious the issue is.") + priority: Priority = Field(description="P1, P2, or P3 review priority.") 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.", ) + check_name: str | None = Field( + default=None, + description=( + "Exact current-head failed check causally explained by this finding, " + "when the finding is a failed-check RCA." + ), + ) evidence: str = Field( + min_length=1, description="Log, SARIF, test, or source reference proving the issue is real.", ) + evidence_type: EvidenceType = Field(description="The kind of source evidence supporting the finding.") + observable_impact: str = Field( + min_length=1, + description="The user- or operator-visible failure caused by the issue.", + ) + trigger: str = Field( + min_length=1, + description="The concrete condition or workflow that exposes the issue.", + ) recommendation: str = Field( + min_length=1, description="The specific fix the author should apply.", ) + regression_command: str = Field( + min_length=1, + description="One exact command or test target that verifies the fix.", + ) + suggested_diff: str | None = Field( + default=None, + max_length=8000, + description="Minimal replacement text for a GitHub suggestion block, when possible.", + ) + + @field_validator("line", mode="before") + @classmethod + def require_exact_positive_integer_line(cls, value: object) -> int | None: + """Keep GitHub source identity 1-indexed and free from scalar coercion.""" + if value is None: + return None + if isinstance(value, bool) or not isinstance(value, int) or value <= 0: + raise ValueError("line must be an exact positive integer when supplied") + return value + + @field_validator("regression_command") + @classmethod + def require_single_line_command(cls, value: str) -> str: + """Keep the published command exact and safe inside inline-code markup.""" + if any(character in value for character in "\r\n`"): + raise ValueError("regression command must be one plain-text command") + return value + + @field_validator("suggested_diff") + @classmethod + def reject_suggestion_fence_injection(cls, value: str | None) -> str | None: + """Prevent model output from escaping the GitHub suggestion fence.""" + if value is not None and "```" in value: + raise ValueError("suggested diff cannot contain a Markdown fence") + return value class ReviewVerdict(BaseModel): diff --git a/reviewer/pyproject.toml b/reviewer/pyproject.toml index df7650571..e5b6f168b 100644 --- a/reviewer/pyproject.toml +++ b/reviewer/pyproject.toml @@ -1,6 +1,7 @@ [build-system] requires = ["setuptools>=68"] -build-backend = "setuptools.build_meta" +build-backend = "build_backend" +backend-path = ["."] [project] name = "noema-reviewer" @@ -9,12 +10,24 @@ description = "Noema independent PydanticAI second reviewer for ContextualWisdom requires-python = ">=3.11" dependencies = [ "pydantic>=2.7", - "pydantic-ai-slim[openai]>=0.0.14", + "pydantic-ai-slim[openai]>=2.9.0,<3", ] [project.scripts] noema-reviewer = "noema_reviewer.cli:main" +# noema-core is not yet published as an immutable index dependency. The custom +# PEP 517 backend stages the exact canonical monorepo source into a build-only +# directory. That snapshot is embedded in an sdist, allowing its wheel to build +# without the original checkout while keeping repository source authority in +# packages/noema-core. +[tool.setuptools] +packages = ["noema_reviewer", "noema_core"] + +[tool.setuptools.package-dir] +noema_reviewer = "noema_reviewer" +noema_core = "_build_include/noema_core" + [dependency-groups] dev = [ "pytest>=8.0.0", @@ -23,7 +36,7 @@ dev = [ ] [tool.pytest.ini_options] -pythonpath = ["."] +pythonpath = [".", "../packages/noema-core/src"] addopts = "--cov=noema_reviewer --cov-branch --cov-report=term-missing --cov-fail-under=100" [tool.coverage.run] diff --git a/reviewer/requirements-ci.in b/reviewer/requirements-ci.in index a85cb013a..129ab6384 100644 --- a/reviewer/requirements-ci.in +++ b/reviewer/requirements-ci.in @@ -1,4 +1,4 @@ -pydantic-ai-slim[openai]>=0.0.14 +pydantic-ai-slim[openai]>=2.9.0,<3 pytest>=8.0.0 pytest-cov>=5.0.0 interrogate>=1.7.0 diff --git a/reviewer/tests/test_agent.py b/reviewer/tests/test_agent.py index db624d5d2..a298ae162 100644 --- a/reviewer/tests/test_agent.py +++ b/reviewer/tests/test_agent.py @@ -5,8 +5,10 @@ from pydantic_ai.models.test import TestModel from noema_reviewer.agent import ( + SYSTEM_PROMPT, PydanticAIReviewAgent, ReviewAgent, + SYSTEM_PROMPT, build_agent, build_prompt, ) @@ -45,6 +47,13 @@ def test_agent_satisfies_protocol() -> None: assert isinstance(_agent_returning(), ReviewAgent) +def test_reviewer_identity_preserves_the_protected_main_role() -> None: + """Shared identity reuse must not broaden the reviewer's prompt-sensitive role.""" + assert SYSTEM_PROMPT.startswith( + "You are Noema, an independent second reviewer for ContextualWisdomLab, " + ) + + def test_agent_returns_model_approval() -> None: """A model approval flows through unchanged when no gate fires.""" verdict = _agent_returning().review(_evidenced_manifest()) @@ -85,6 +94,9 @@ def test_build_prompt_includes_all_sections() -> None: assert "Dependency findings:" in prompt assert "SARIF summary:" in prompt assert "Workflow log excerpts:" in prompt + assert "exact repository path and positive line" in SYSTEM_PROMPT + assert "P1/P2/P3 priority" in SYSTEM_PROMPT + assert "exact regression command" in SYSTEM_PROMPT assert "Prior review comments:" in prompt assert "Changed-file context:" in prompt diff --git a/reviewer/tests/test_build_backend_editable.py b/reviewer/tests/test_build_backend_editable.py new file mode 100644 index 000000000..ab59fb559 --- /dev/null +++ b/reviewer/tests/test_build_backend_editable.py @@ -0,0 +1,111 @@ +"""Regression coverage for the reviewer packaging backend's editable-install contract.""" + +from __future__ import annotations + +import os +from pathlib import Path +import shlex +import subprocess +import sys + +import build_backend + + +def test_build_backend_exposes_pep660_editable_hooks() -> None: + """The custom backend must preserve setuptools' documented editable-install path.""" + + for hook_name in ( + "build_editable", + "prepare_metadata_for_build_editable", + "get_requires_for_build_editable", + ): + assert callable(getattr(build_backend, hook_name, None)), hook_name + + +def test_clean_editable_install_imports_reviewer_and_canonical_core(tmp_path: Path) -> None: + """An isolated editable install must resolve declared runtime dependencies and shared core.""" + + reviewer_root = Path(__file__).resolve().parents[1] + requirements = reviewer_root / "requirements-ci-hashes.txt" + venv_dir = tmp_path / "editable-venv" + subprocess.run( + [sys.executable, "-m", "venv", str(venv_dir)], + check=True, + ) + python = venv_dir / ("Scripts/python.exe" if os.name == "nt" else "bin/python") + env = os.environ.copy() + env["PYTHONPATH"] = "" + subprocess.run( + [ + str(python), + "-m", + "pip", + "install", + "--require-hashes", + "--no-deps", + "-r", + str(requirements), + ], + cwd=tmp_path, + env=env, + check=True, + capture_output=True, + text=True, + ) + subprocess.run( + [ + str(python), + "-m", + "pip", + "install", + "--no-deps", + "-e", + str(reviewer_root), + ], + cwd=tmp_path, + env=env, + check=True, + capture_output=True, + text=True, + ) + completed = subprocess.run( + [ + str(python), + "-c", + "import noema_core, noema_reviewer; assert noema_core.build_agent; assert noema_reviewer.build_agent", + ], + cwd=tmp_path, + env=env, + check=False, + capture_output=True, + text=True, + ) + assert completed.returncode == 0, completed.stderr + + +def test_reviewer_ci_proves_an_isolated_editable_install_with_locked_dependencies() -> None: + """Required CI must validate editable packaging without inheriting host site-packages.""" + + reviewer_root = Path(__file__).resolve().parents[1] + workflow = (reviewer_root.parent / ".github" / "workflows" / "reviewer-ci.yml").read_text( + encoding="utf-8" + ) + + assert 'editable_venv="$RUNNER_TEMP/noema-reviewer-editable-smoke"' in workflow + assert 'python -m venv "$editable_venv"' in workflow + assert ( + '"$editable_venv/bin/python" -m pip install --require-hashes --no-deps ' + '-r requirements-ci-hashes.txt' + ) in workflow + + editable_install_commands = [ + line.strip() + for line in workflow.splitlines() + if "pip install" in line and "-e ." in line + ] + assert len(editable_install_commands) == 1 + editable_tokens = shlex.split(editable_install_commands[0]) + assert "-e" in editable_tokens + assert editable_tokens[editable_tokens.index("-e") + 1] == "." + assert "--system-site-packages" not in editable_tokens + assert "--no-build-isolation" not in editable_tokens diff --git a/reviewer/tests/test_build_backend_staging.py b/reviewer/tests/test_build_backend_staging.py new file mode 100644 index 000000000..5a1bd22e9 --- /dev/null +++ b/reviewer/tests/test_build_backend_staging.py @@ -0,0 +1,150 @@ +"""Regression coverage for isolated reviewer build staging and editable source lifetime.""" + +from __future__ import annotations + +from concurrent.futures import ThreadPoolExecutor +import json +import os +from pathlib import Path +import threading + +import pytest + +import build_backend + + +def test_distribution_staging_is_private_per_build_invocation() -> None: + """Concurrent distribution preparations must never share a mutable staging tree.""" + + barrier = threading.Barrier(2) + + def observe_distribution_project() -> tuple[Path, Path]: + with build_backend._distribution_project() as project_root: + staged_core = project_root / "_build_include" / "noema_core" + assert staged_core.is_dir() + barrier.wait(timeout=10) + return project_root, staged_core + + with ThreadPoolExecutor(max_workers=2) as pool: + first = pool.submit(observe_distribution_project) + second = pool.submit(observe_distribution_project) + first_project, first_core = first.result(timeout=20) + second_project, second_core = second.result(timeout=20) + + assert first_project != second_project + assert first_core != second_core + + +def test_concurrent_distribution_metadata_keeps_reviewer_project_identity(tmp_path: Path) -> None: + """Fresh backend contexts must emit reviewer metadata, never UNKNOWN artifacts.""" + + def prepare_metadata(index: int) -> tuple[str, bool]: + metadata_root = tmp_path / f"metadata-{index}" + metadata_root.mkdir() + distribution_name = build_backend.prepare_metadata_for_build_wheel(str(metadata_root)) + return distribution_name, (metadata_root / distribution_name).is_dir() + + with ThreadPoolExecutor(max_workers=2) as pool: + results = list(pool.map(prepare_metadata, (1, 2))) + + for distribution_name, exists in results: + assert distribution_name.startswith("noema_reviewer-") + assert distribution_name.endswith(".dist-info") + assert exists + + +def test_distribution_hook_preserves_frontend_backend_environment( + tmp_path: Path, + monkeypatch, +) -> None: + """A staged child must retain the PEP 517 frontend's isolated backend search path.""" + + isolated_backend_path = str(tmp_path / "pep517-overlay-site-packages") + monkeypatch.setattr( + build_backend.sys, + "path", + [isolated_backend_path, *build_backend.sys.path], + ) + observed: dict[str, object] = {} + + def fake_run(command, *, cwd, check, env) -> None: + observed["cwd"] = cwd + observed["check"] = check + observed["env"] = env + Path(command[4]).write_text( + json.dumps("noema_reviewer-0.1.0.dist-info"), + encoding="utf-8", + ) + + monkeypatch.setattr(build_backend.subprocess, "run", fake_run) + metadata_root = tmp_path / "metadata" + metadata_root.mkdir() + + result = build_backend.prepare_metadata_for_build_wheel(str(metadata_root)) + + assert result == "noema_reviewer-0.1.0.dist-info" + assert observed["check"] is True + child_env = observed["env"] + assert isinstance(child_env, dict) + child_pythonpath = child_env["PYTHONPATH"].split(os.pathsep) + assert child_pythonpath[0] == str(observed["cwd"]) + assert isolated_backend_path in child_pythonpath + + +def test_distribution_build_does_not_destroy_editable_canonical_view(tmp_path: Path) -> None: + """A real distribution build must not remove the source view used by an editable install.""" + + if not build_backend._CANONICAL_CORE.is_dir(): + return + + build_backend._prepare_editable_core() + editable_view = build_backend._STAGED_CORE + assert editable_view.is_symlink() + assert editable_view.resolve() == build_backend._CANONICAL_CORE.resolve() + + wheel_root = tmp_path / "wheel" + wheel_root.mkdir() + try: + wheel_name = build_backend.build_wheel(str(wheel_root)) + assert wheel_name.startswith("noema_reviewer-") + assert (wheel_root / wheel_name).is_file() + assert editable_view.is_symlink() + assert editable_view.resolve() == build_backend._CANONICAL_CORE.resolve() + finally: + build_backend._remove_generated_path(build_backend._STAGING_ROOT) + + +def test_generated_path_cleanup_unlinks_files_and_symlinks(tmp_path: Path) -> None: + """Generated cleanup must unlink leaf capabilities instead of passing them to rmtree.""" + + regular_file = tmp_path / "regular-file" + regular_file.write_text("generated", encoding="utf-8") + build_backend._remove_generated_path(regular_file) + assert not regular_file.exists() + + target = tmp_path / "target" + target.mkdir() + alias = tmp_path / "alias" + alias.symlink_to(target, target_is_directory=True) + build_backend._remove_generated_path(alias) + assert not alias.exists() + assert target.is_dir() + + +def test_editable_source_view_fails_closed_when_live_link_cannot_be_created( + monkeypatch, +) -> None: + """Editable packaging must not replace a failed live link with a stale copied snapshot.""" + + if not build_backend._CANONICAL_CORE.is_dir(): + return + + build_backend._remove_generated_path(build_backend._STAGING_ROOT) + + def deny_symlink(*_args, **_kwargs) -> None: + raise OSError("symlink unavailable") + + monkeypatch.setattr(Path, "symlink_to", deny_symlink) + with pytest.raises(RuntimeError, match="requires a live symlink"): + build_backend._prepare_editable_core() + assert not build_backend._STAGING_ROOT.exists() diff --git a/reviewer/tests/test_check_run_pagination.py b/reviewer/tests/test_check_run_pagination.py index ed41229d9..1d8c71924 100644 --- a/reviewer/tests/test_check_run_pagination.py +++ b/reviewer/tests/test_check_run_pagination.py @@ -21,7 +21,7 @@ def __init__(self, *, include_late_failure: bool = False) -> None: def __call__(self, args, stdin=None): """Return 101 checks or the log belonging to the late failed check.""" self.calls.append(list(args)) - if any("/actions/jobs/" in part for part in args): + if any("/actions/jobs/123456/logs" in part for part in args): return "late failure details" checks = [ @@ -30,7 +30,11 @@ def __call__(self, args, stdin=None): ] late_check = {"name": "check-100", "conclusion": "success"} if self.include_late_failure: - late_check.update({"id": 987654, "conclusion": "failure"}) + late_check.update({ + "id": 987654, + "conclusion": "failure", + "details_url": "https://github.com/ContextualWisdomLab/example/actions/runs/42/job/123456", + }) checks.append(late_check) return "\n".join(json.dumps(check) for check in checks) @@ -71,7 +75,7 @@ def test_failed_workflow_logs_retain_a_failure_after_the_first_page() -> None: assert "## check-100 (failure)" in logs assert "late failure details" in logs - assert any("/actions/jobs/987654/logs" in part for call in runner.calls for part in call) + assert any("/actions/jobs/123456/logs" in part for call in runner.calls for part in call) command = _check_runs_command(runner) _assert_complete_pagination(command) jq_filter = command[command.index("--jq") + 1] diff --git a/reviewer/tests/test_deterministic_finding_identity.py b/reviewer/tests/test_deterministic_finding_identity.py index c7965b7d4..64fc17037 100644 --- a/reviewer/tests/test_deterministic_finding_identity.py +++ b/reviewer/tests/test_deterministic_finding_identity.py @@ -2,11 +2,19 @@ from noema_reviewer.gating import enforce_security_and_check_gates from noema_reviewer.manifest import ReviewManifest, SecurityFinding -from noema_reviewer.models import Finding, ReviewVerdict, Severity, Verdict +from noema_reviewer.models import ( + EvidenceType, + Finding, + Priority, + ReviewVerdict, + Severity, + Verdict, +) def test_scanner_finding_is_not_hidden_by_model_finding_at_same_path_and_severity() -> None: """Distinct deterministic scanner evidence must survive a model path/severity collision.""" + path = "reviewer/noema_reviewer/github_io.py" manifest = ReviewManifest( repo="ContextualWisdomLab/noema", pr_number=1, @@ -16,7 +24,7 @@ def test_scanner_finding_is_not_hidden_by_model_finding_at_same_path_and_severit identifier="py/path-injection", severity=Severity.HIGH, message="Untrusted path reaches filesystem access", - path="reviewer/noema_reviewer/github_io.py", + path=path, line=42, url="https://example.invalid/alert/1", ) @@ -28,10 +36,15 @@ def test_scanner_finding_is_not_hidden_by_model_finding_at_same_path_and_severit findings=[ Finding( severity=Severity.HIGH, - path="reviewer/noema_reviewer/github_io.py", + priority=Priority.P1, + path=path, line=7, evidence="Model evidence for an unrelated boundary defect.", + evidence_type=EvidenceType.NEARBY_IMPLEMENTATION, + observable_impact="A separate review boundary is incorrect.", + trigger="Reviewing the unrelated boundary path.", recommendation="Repair the unrelated boundary defect.", + regression_command="python -m pytest reviewer/tests/test_gating.py", ) ], ) diff --git a/reviewer/tests/test_failed_check_causal_binding.py b/reviewer/tests/test_failed_check_causal_binding.py new file mode 100644 index 000000000..68a7ab33a --- /dev/null +++ b/reviewer/tests/test_failed_check_causal_binding.py @@ -0,0 +1,89 @@ +"""Regression tests for causal binding between failed checks and source findings.""" + +from __future__ import annotations + +from noema_reviewer.gating import apply_gates +from noema_reviewer.manifest import ChangedFile, CheckConclusion, ReviewManifest +from noema_reviewer.models import EvidenceType, Finding, Priority, ReviewVerdict, Severity, Verdict + + +def _manifest(*check_names: str) -> ReviewManifest: + """Build complete review evidence with the requested failed checks.""" + return ReviewManifest( + repo="o/r", + pr_number=1, + diff="diff --git a/a.py b/a.py\ndiff --git a/b.py b/b.py", + changed_files=[ + ChangedFile(path="a.py", content="raise RuntimeError('build')"), + ChangedFile(path="b.py", content="raise RuntimeError('lint')"), + ], + check_conclusions=[ + CheckConclusion(name=name, conclusion="failure") for name in check_names + ], + codegraph_status="## codegraph explore\na.py -> build_failure", + ) + + +def _finding(*, check_name: str | None) -> Finding: + """Build one otherwise-actionable source finding for failed-check tests.""" + return Finding( + severity=Severity.HIGH, + priority=Priority.P1, + path="a.py", + line=1, + check_name=check_name, + evidence="current-head log reports the failing assertion at a.py:1", + evidence_type=EvidenceType.FAILED_CHECK, + observable_impact="The current-head check fails.", + trigger="Running the bound check.", + recommendation="Fix the regression and retain this assertion as a test.", + regression_command="uv run pytest reviewer/tests/test_failed_check_causal_binding.py", + ) + + +def test_each_failed_check_requires_its_own_source_bound_rca() -> None: + """One actionable finding cannot clear a second failed check.""" + verdict = ReviewVerdict( + verdict=Verdict.REQUEST_CHANGES, + summary="The build check has an actionable source regression.", + findings=[_finding(check_name="build")], + ) + + gated = apply_gates(_manifest("build", "lint"), verdict, strict=False) + + assert gated.verdict is Verdict.BLOCKED + assert gated.blocked_reasons == [ + "failed check lint lacks an actionable current-head path:line finding" + ] + + +def test_unbound_actionable_finding_cannot_clear_failed_check() -> None: + """Path and line evidence without exact check identity remains blocked.""" + verdict = ReviewVerdict( + verdict=Verdict.REQUEST_CHANGES, + summary="A source regression exists, but it is not bound to the failed check.", + findings=[_finding(check_name=None)], + ) + + gated = apply_gates(_manifest("build"), verdict, strict=False) + + assert gated.verdict is Verdict.BLOCKED + assert gated.blocked_reasons == [ + "failed check build lacks an actionable current-head path:line finding" + ] + + +def test_wrong_check_identity_cannot_clear_failed_check() -> None: + """A finding bound to another check cannot stand in for the failed check.""" + verdict = ReviewVerdict( + verdict=Verdict.REQUEST_CHANGES, + summary="The finding names a different check.", + findings=[_finding(check_name="lint")], + ) + + gated = apply_gates(_manifest("build"), verdict, strict=False) + + assert gated.verdict is Verdict.BLOCKED + assert gated.blocked_reasons == [ + "failed check build lacks an actionable current-head path:line finding" + ] diff --git a/reviewer/tests/test_failed_check_coverage_edges.py b/reviewer/tests/test_failed_check_coverage_edges.py new file mode 100644 index 000000000..97d457cba --- /dev/null +++ b/reviewer/tests/test_failed_check_coverage_edges.py @@ -0,0 +1,82 @@ +"""Coverage contracts for reviewer fail-closed edge branches.""" + +from noema_reviewer.gating import invalid_suggestion_reasons +from noema_reviewer.github_io import _github_actions_job_id, render_review_body +from noema_reviewer.manifest import ChangedFile, ReviewManifest +from noema_reviewer.models import ( + EvidenceType, + Finding, + Priority, + ReviewVerdict, + Severity, + Verdict, +) + + +def _finding(*, line: int = 1, suggested_diff: str | None = None) -> Finding: + """Build one source-backed finding for rendering and anchoring edge tests.""" + return Finding( + severity=Severity.HIGH, + priority=Priority.P1, + path="a.py", + line=line, + evidence="current-head evidence", + evidence_type=EvidenceType.NEARBY_IMPLEMENTATION, + observable_impact="The current-head behavior is incorrect.", + trigger="Execute the affected path.", + recommendation="Apply the bounded source repair.", + regression_command="python -m pytest", + suggested_diff=suggested_diff, + ) + + +def test_diff_metadata_line_terminates_right_side_anchor_sequence() -> None: + """Unexpected diff metadata cannot leave a later suggestion line attachable.""" + manifest = ReviewManifest( + repo="o/r", + pr_number=1, + diff=( + "diff --git a/a.py b/a.py\n" + "--- a/a.py\n" + "+++ b/a.py\n" + "@@ -1 +1,2 @@\n" + "+first\n" + "\\ No newline at end of file\n" + "+second" + ), + changed_files=[ChangedFile(path="a.py", content="first\nsecond")], + ) + + verdict = ReviewVerdict( + verdict=Verdict.REQUEST_CHANGES, + summary="fix", + findings=[_finding(line=2, suggested_diff="replacement")], + ) + + assert invalid_suggestion_reasons(manifest, verdict) == [ + "suggested diff is not anchored to a current-head right-side diff line: a.py:2" + ] + + +def test_actions_job_id_rejects_non_https_github_url() -> None: + """Only repository-bound HTTPS GitHub job URLs can authorize log retrieval.""" + assert _github_actions_job_id( + "o/r", + "http://github.com/o/r/actions/runs/1/job/2", + ) is None + + +def test_review_body_renders_finding_without_inline_suggestion() -> None: + """A source finding without a suggestion renders without inventing a patch block.""" + body = render_review_body( + ReviewVerdict( + verdict=Verdict.REQUEST_CHANGES, + summary="current-head finding", + findings=[_finding()], + ), + "a" * 40, + "github-app", + ) + + assert "#### [P1] a.py:1" in body + assert "```suggestion" not in body diff --git a/reviewer/tests/test_finding_line_contract.py b/reviewer/tests/test_finding_line_contract.py new file mode 100644 index 000000000..4121eb938 --- /dev/null +++ b/reviewer/tests/test_finding_line_contract.py @@ -0,0 +1,37 @@ +"""Regression tests for exact GitHub review-line identity.""" + +from __future__ import annotations + +import pytest +from pydantic import ValidationError + +from noema_reviewer.models import EvidenceType, Finding, Priority, Severity + + +def _finding_payload(line: object) -> dict[str, object]: + """Build the smallest complete finding payload around one line candidate.""" + return { + "severity": Severity.HIGH, + "priority": Priority.P1, + "path": "src/example.py", + "line": line, + "evidence": "current-head regression", + "evidence_type": EvidenceType.NEARBY_IMPLEMENTATION, + "observable_impact": "GitHub cannot attach the review finding to an exact source line.", + "trigger": "Publishing a finding with a non-positive or coerced line value.", + "recommendation": "Require an exact positive integer review line at schema admission.", + "regression_command": "uv run pytest reviewer/tests/test_finding_line_contract.py", + } + + +@pytest.mark.parametrize("invalid_line", [0, -1, True, False, 1.0, "1"]) +def test_finding_rejects_non_exact_positive_integer_lines(invalid_line: object) -> None: + """Finding.line is a 1-indexed GitHub identity, not a coercible scalar.""" + with pytest.raises(ValidationError): + Finding.model_validate(_finding_payload(invalid_line)) + + +def test_finding_accepts_positive_integer_or_missing_line() -> None: + """Valid current-head line identities and intentionally absent lines remain supported.""" + assert Finding.model_validate(_finding_payload(1)).line == 1 + assert Finding.model_validate(_finding_payload(None)).line is None diff --git a/reviewer/tests/test_gating.py b/reviewer/tests/test_gating.py index 792719a16..3218b416a 100644 --- a/reviewer/tests/test_gating.py +++ b/reviewer/tests/test_gating.py @@ -7,7 +7,8 @@ blocked_verdict, enforce_dependency_gate, enforce_security_and_check_gates, - failed_checks_as_review, + failed_check_blockers, + invalid_suggestion_reasons, missing_evidence, security_findings_as_review, unresolved_threads_as_review, @@ -20,7 +21,15 @@ ReviewManifest, SecurityFinding, ) -from noema_reviewer.models import Confidence, Finding, ReviewVerdict, Severity, Verdict +from noema_reviewer.models import ( + Confidence, + EvidenceType, + Finding, + Priority, + ReviewVerdict, + Severity, + Verdict, +) def _full_manifest(**overrides) -> ReviewManifest: @@ -98,17 +107,69 @@ def test_evidence_collection_failure_blocks_strict_review() -> None: assert reasons == ["evidence collection failure: code scanning: HTTP 403"] -def test_failed_check_downgrades_approval_with_log_pointer() -> None: - """A current-head failed check becomes a deterministic HIGH finding.""" +def test_failed_check_without_source_mapping_blocks_publication() -> None: + """A check name alone cannot become a synthetic source-code finding.""" manifest = _full_manifest(check_conclusions=[CheckConclusion(name="build", conclusion="failure")]) - finding = failed_checks_as_review(manifest)[0] - assert finding.path.endswith("/build") - gated = enforce_security_and_check_gates( + assert failed_check_blockers(manifest) == [ + "failed check build lacks an actionable current-head path:line finding" + ] + gated = apply_gates( manifest, ReviewVerdict(verdict=Verdict.APPROVE, summary="looks good"), + strict=False, ) - assert gated.verdict is Verdict.REQUEST_CHANGES - assert "current-head checks" in gated.summary + assert gated.verdict is Verdict.BLOCKED + assert "path:line" in gated.blocked_reasons[0] + + +def test_failed_check_accepts_model_rca_at_changed_source_line() -> None: + """A source-backed failed-check RCA remains publishable as request changes.""" + manifest = _full_manifest(check_conclusions=[CheckConclusion(name="build", conclusion="failure")]) + verdict = ReviewVerdict( + verdict=Verdict.REQUEST_CHANGES, + summary="The current-head build proves a source regression.", + findings=[ + Finding( + severity=Severity.HIGH, + priority=Priority.P1, + path="a", + line=1, + check_name="build", + evidence="build log reports the failing assertion at a:1", + evidence_type=EvidenceType.FAILED_CHECK, + observable_impact="The current-head build fails.", + trigger="Running the build check.", + recommendation="Fix the branch and add the failing assertion as a regression test.", + regression_command="uv run pytest reviewer/tests/test_gating.py", + ) + ], + ) + assert apply_gates(manifest, verdict, strict=False).verdict is Verdict.REQUEST_CHANGES + + +def test_suggestion_must_target_current_right_side_diff_line() -> None: + """A suggestion outside the exact diff fails closed before GitHub publication.""" + manifest = _full_manifest( + diff="diff --git a/a b/a\n--- a/a\n+++ b/a\n@@ -1 +1 @@\n-old\n+new" + ) + finding = Finding( + severity=Severity.HIGH, + priority=Priority.P1, + path="a", + line=2, + evidence="current source", + evidence_type=EvidenceType.NEARBY_IMPLEMENTATION, + observable_impact="The request fails.", + trigger="Calling the affected path.", + recommendation="Replace the expression.", + regression_command="uv run pytest reviewer/tests/test_gating.py", + suggested_diff="fixed", + ) + verdict = ReviewVerdict(verdict=Verdict.REQUEST_CHANGES, summary="fix", findings=[finding]) + assert invalid_suggestion_reasons(manifest, verdict) + assert apply_gates(manifest, verdict, strict=False).verdict is Verdict.BLOCKED + anchored = verdict.model_copy(update={"findings": [finding.model_copy(update={"line": 1})]}) + assert invalid_suggestion_reasons(manifest, anchored) == [] def test_primary_opencode_check_does_not_deadlock_independent_noema() -> None: @@ -119,20 +180,20 @@ def test_primary_opencode_check_does_not_deadlock_independent_noema() -> None: CheckConclusion(name="build", conclusion="success"), ] ) - assert failed_checks_as_review(manifest) == [] + assert failed_check_blockers(manifest) == [] verdict = ReviewVerdict(verdict=Verdict.APPROVE, summary="independent evidence passed") assert enforce_security_and_check_gates(manifest, verdict).verdict is Verdict.APPROVE def test_noema_review_check_does_not_deadlock_its_own_current_run() -> None: - """The in-flight Noema check cannot become a deterministic finding against itself.""" + """The exact in-flight Noema check cannot become an RCA prerequisite for itself.""" manifest = _full_manifest( check_conclusions=[ CheckConclusion(name="noema-review", conclusion="pending"), CheckConclusion(name="build", conclusion="success"), ] ) - assert failed_checks_as_review(manifest) == [] + assert failed_check_blockers(manifest) == [] verdict = ReviewVerdict(verdict=Verdict.APPROVE, summary="independent evidence passed") assert enforce_security_and_check_gates(manifest, verdict).verdict is Verdict.APPROVE @@ -145,7 +206,7 @@ def test_review_dependent_metadata_gate_does_not_deadlock_independent_noema() -> CheckConclusion(name="build", conclusion="success"), ] ) - assert failed_checks_as_review(manifest) == [] + assert failed_check_blockers(manifest) == [] verdict = ReviewVerdict(verdict=Verdict.APPROVE, summary="independent evidence passed") assert enforce_security_and_check_gates(manifest, verdict).verdict is Verdict.APPROVE @@ -155,7 +216,7 @@ def test_similarly_named_failed_check_remains_blocking() -> None: manifest = _full_manifest( check_conclusions=[CheckConclusion(name="opencode-review-copy", conclusion="failure")] ) - assert failed_checks_as_review(manifest) + assert failed_check_blockers(manifest) def test_similarly_named_noema_check_remains_blocking() -> None: @@ -163,7 +224,7 @@ def test_similarly_named_noema_check_remains_blocking() -> None: manifest = _full_manifest( check_conclusions=[CheckConclusion(name="noema-review-copy", conclusion="failure")] ) - assert failed_checks_as_review(manifest) + assert failed_check_blockers(manifest) def test_similarly_named_metadata_check_remains_blocking() -> None: @@ -173,7 +234,7 @@ def test_similarly_named_metadata_check_remains_blocking() -> None: CheckConclusion(name="metadata-only gate evaluation copy", conclusion="failure") ] ) - assert failed_checks_as_review(manifest) + assert failed_check_blockers(manifest) def test_unresolved_current_thread_downgrades_approval() -> None: @@ -294,7 +355,7 @@ def test_dependency_gate_does_not_touch_blocked() -> None: def test_dependency_gate_deduplicates_exact_existing_finding() -> None: - """An exact pre-existing deterministic finding is not duplicated.""" + """An exact pre-existing dependency finding is not duplicated.""" manifest = _full_manifest( dependency_findings=[DependencyFinding(tool="osv", package_name="dup", severity=Severity.MEDIUM)] ) @@ -304,9 +365,14 @@ def test_dependency_gate_deduplicates_exact_existing_finding() -> None: findings=[ Finding( severity=Severity.MEDIUM, + priority=Priority.P2, path="dup", evidence="osv reported dup@current", + evidence_type=EvidenceType.FAILED_CHECK, + observable_impact="The pull request would retain a known vulnerable dependency.", + trigger="Installing the dependency set recorded by the current lockfile.", recommendation="Bump dup to a non-vulnerable release and refresh the lockfile.", + regression_command="uv run pip-audit", ) ], ) diff --git a/reviewer/tests/test_github_io.py b/reviewer/tests/test_github_io.py index 0158ff269..f2ea2c82e 100644 --- a/reviewer/tests/test_github_io.py +++ b/reviewer/tests/test_github_io.py @@ -25,7 +25,15 @@ publish_verdict, render_review_body, ) -from noema_reviewer.models import Confidence, Finding, ReviewVerdict, Severity, Verdict +from noema_reviewer.models import ( + Confidence, + EvidenceType, + Finding, + Priority, + ReviewVerdict, + Severity, + Verdict, +) REPO = "ContextualWisdomLab/example" HEAD_SHA = "a" * 40 @@ -44,10 +52,12 @@ def __init__(self, *, fail_contents: bool = False) -> None: """Record whether the contents endpoint should raise.""" self.fail_contents = fail_contents self.calls: list[list[str]] = [] + self.stdins: list[str | None] = [] def __call__(self, args, stdin=None): """Return canned responses keyed by the requested endpoint.""" self.calls.append(list(args)) + self.stdins.append(stdin) joined = " ".join(args) if "Accept: application/vnd.github.v3.diff" in joined: return "diff --git a/x b/x\n+new line" @@ -305,9 +315,9 @@ def test_failed_workflow_logs_include_exact_check_reason() -> None: def runner(args, stdin=None): joined = " ".join(args) - if "/check-runs" in joined: - return json.dumps({"id": 42, "name": "tests", "conclusion": "failure"}) - if "/jobs/42/logs" in joined: + if "/check-runs" in joined and "/annotations" not in joined: + return json.dumps({"id": 42, "name": "tests", "conclusion": "failure", "details_url": "https://github.com/o/r/actions/runs/10/job/99"}) + if "/jobs/99/logs" in joined: return "AssertionError: expected 1, got 2" return "" @@ -316,11 +326,31 @@ def runner(args, stdin=None): assert "AssertionError" in result +def test_failed_workflow_logs_never_treat_check_run_id_as_job_id() -> None: + """GitHub Check Run ids and Actions Job ids are separate namespaces.""" + calls: list[str] = [] + + def runner(args, stdin=None): + joined = " ".join(args) + calls.append(joined) + if "/check-runs" in joined and "/annotations" not in joined: + return json.dumps({"id": 42, "name": "tests", "conclusion": "failure", "details_url": "https://github.com/o/r/actions/runs/10/job/99"}) + if "/jobs/99/logs" in joined: + return "src/service.py:17: AssertionError" + return "" + + result = _fetch_failed_workflow_logs("o/r", "head", runner) + assert "src/service.py:17" in result + assert any("/jobs/99/logs" in call for call in calls) + assert not any("/jobs/42/logs" in call for call in calls) + + def test_failed_workflow_logs_explain_unavailable_job_log() -> None: """A job-log API error remains visible rather than disappearing.""" def runner(args, stdin=None): - if "/check-runs" in " ".join(args): + joined = " ".join(args) + if "/check-runs" in joined and "/annotations" not in joined: return json.dumps({"id": 42, "name": "tests", "conclusion": "failure"}) raise RuntimeError("HTTP 404") @@ -480,11 +510,25 @@ def test_render_review_body_marks_findings_and_marker() -> None: verdict = ReviewVerdict( verdict=Verdict.REQUEST_CHANGES, summary="please fix", - findings=[Finding(severity=Severity.HIGH, path="x.py", line=3, evidence="log", recommendation="bump")], + findings=[Finding( + severity=Severity.HIGH, + priority=Priority.P1, + path="x.py", + line=3, + evidence="log", + evidence_type=EvidenceType.FAILED_CHECK, + observable_impact="The build fails.", + trigger="Running the build check.", + recommendation="bump", + regression_command="uv run pytest reviewer/tests/test_github_io.py", + suggested_diff="fixed = True", + )], confidence=Confidence.MEDIUM, ) body = render_review_body(verdict, "headsha", "NOEMA_REVIEW_TOKEN") - assert "[high] x.py:3" in body + assert "[P1] x.py:3" in body + assert "Observable impact: The build fails." in body + assert "```suggestion\nfixed = True\n```" in body assert "" in body assert "Result: REQUEST_CHANGES" in body @@ -512,6 +556,36 @@ def test_publish_verdict_posts_review() -> None: assert post[:3] == ["gh", "api", "-X"] +def test_publish_verdict_posts_applyable_inline_suggestion() -> None: + """A source replacement is sent as a right-side GitHub suggestion comment.""" + runner = StubRunner() + verdict = ReviewVerdict( + verdict=Verdict.REQUEST_CHANGES, + summary="fix the line", + findings=[Finding( + severity=Severity.HIGH, + priority=Priority.P1, + path="x.py", + line=3, + evidence="current source", + evidence_type=EvidenceType.NEARBY_IMPLEMENTATION, + observable_impact="The request fails.", + trigger="Calling the affected endpoint.", + recommendation="Replace the faulty expression.", + regression_command="uv run pytest reviewer/tests/test_github_io.py", + suggested_diff="return fixed_value", + )], + ) + publish_verdict(REPO, 5, verdict, HEAD_SHA, runner=runner) + payload = json.loads(runner.stdins[-1] or "{}") + assert payload["comments"] == [{ + "path": "x.py", + "line": 3, + "side": "RIGHT", + "body": "```suggestion\nreturn fixed_value\n```", + }] + + def test_publish_verdict_rejects_invalid_metadata() -> None: """Publication rejects an out-of-scope repository before any GitHub call.""" verdict = ReviewVerdict(verdict=Verdict.APPROVE, summary="ok") diff --git a/reviewer/tests/test_models.py b/reviewer/tests/test_models.py index c97202694..9aab7c1ef 100644 --- a/reviewer/tests/test_models.py +++ b/reviewer/tests/test_models.py @@ -2,10 +2,15 @@ from __future__ import annotations +import pytest +from pydantic import ValidationError + from noema_reviewer.models import ( BLOCKING_SEVERITIES, Confidence, + EvidenceType, Finding, + Priority, ReviewVerdict, Severity, Verdict, @@ -40,10 +45,40 @@ def test_finding_roundtrips_optional_line() -> None: """A finding keeps an optional line and required evidence/recommendation.""" finding = Finding( severity=Severity.HIGH, + priority=Priority.P1, path="src/x.py", evidence="test log", + evidence_type=EvidenceType.FAILED_CHECK, + observable_impact="The tested behavior fails.", + trigger="Running the focused test.", recommendation="fix it", + regression_command="uv run pytest reviewer/tests/test_models.py", ) assert finding.line is None dumped = finding.model_dump() assert dumped["severity"] == "high" + assert { + "priority", "evidence_type", "observable_impact", "trigger", "regression_command" + } <= set(Finding.model_json_schema()["required"]) + + +@pytest.mark.parametrize( + ("field", "value"), + [("regression_command", "pytest\nrm -rf x"), ("suggested_diff", "```\nunsafe\n```")], +) +def test_finding_rejects_markdown_command_injection(field: str, value: str) -> None: + """Published commands and suggestions cannot escape their Markdown delimiters.""" + payload = { + "severity": Severity.HIGH, + "priority": Priority.P1, + "path": "src/x.py", + "evidence": "test log", + "evidence_type": EvidenceType.FAILED_CHECK, + "observable_impact": "The test fails.", + "trigger": "Running the test.", + "recommendation": "Fix it.", + "regression_command": "uv run pytest", + field: value, + } + with pytest.raises(ValidationError): + Finding.model_validate(payload) diff --git a/reviewer/tests/test_non_success_check_gate.py b/reviewer/tests/test_non_success_check_gate.py index 3f4649d5d..d87ae5e77 100644 --- a/reviewer/tests/test_non_success_check_gate.py +++ b/reviewer/tests/test_non_success_check_gate.py @@ -4,7 +4,7 @@ import pytest -from noema_reviewer.gating import enforce_security_and_check_gates, failed_checks_as_review +from noema_reviewer.gating import apply_gates, enforce_security_and_check_gates, failed_check_blockers from noema_reviewer.manifest import ChangedFile, CheckConclusion, ReviewManifest from noema_reviewer.models import ReviewVerdict, Verdict @@ -26,15 +26,13 @@ def test_observed_non_success_check_cannot_preserve_approval(conclusion: str) -> """Every observed ordinary check must be terminal-success before approval.""" manifest = _manifest_with_check("ci", conclusion) - findings = failed_checks_as_review(manifest) - assert len(findings) == 1 - assert conclusion in findings[0].evidence - - gated = enforce_security_and_check_gates( + assert failed_check_blockers(manifest) + gated = apply_gates( manifest, ReviewVerdict(verdict=Verdict.APPROVE, summary="model approved"), + strict=False, ) - assert gated.verdict is Verdict.REQUEST_CHANGES + assert gated.verdict is Verdict.BLOCKED def test_observed_success_check_remains_nonblocking() -> None: @@ -42,7 +40,7 @@ def test_observed_success_check_remains_nonblocking() -> None: manifest = _manifest_with_check("ci", "success") verdict = ReviewVerdict(verdict=Verdict.APPROVE, summary="model approved") - assert failed_checks_as_review(manifest) == [] + assert failed_check_blockers(manifest) == [] assert enforce_security_and_check_gates(manifest, verdict).verdict is Verdict.APPROVE @@ -55,5 +53,5 @@ def test_cycle_breaking_review_checks_remain_explicit_exceptions(name: str) -> N manifest = _manifest_with_check(name, "skipped") verdict = ReviewVerdict(verdict=Verdict.APPROVE, summary="independent evidence passed") - assert failed_checks_as_review(manifest) == [] + assert failed_check_blockers(manifest) == [] assert enforce_security_and_check_gates(manifest, verdict).verdict is Verdict.APPROVE diff --git a/reviewer/tests/test_shared_core_import_boundary.py b/reviewer/tests/test_shared_core_import_boundary.py new file mode 100644 index 000000000..d90defe53 --- /dev/null +++ b/reviewer/tests/test_shared_core_import_boundary.py @@ -0,0 +1,55 @@ +"""Regression tests for the shared-core import and distribution boundary.""" + +from __future__ import annotations + +import os +from pathlib import Path +import subprocess +import sys + +import pytest + +import noema_reviewer + + +def test_evidence_modules_import_without_shared_core_on_pythonpath() -> None: + """Evidence-only reviewer imports must not require the model-construction package.""" + + reviewer_root = Path(__file__).resolve().parents[1] + env = os.environ.copy() + env["PYTHONPATH"] = "." + completed = subprocess.run( + [ + sys.executable, + "-c", + ( + "import sys; sys.modules['noema_core'] = None; " + "from noema_reviewer.github_io import fetch_manifest; " + "from noema_reviewer.sandbox import DockerCodeGraphRunner; " + "assert fetch_manifest is not None; " + "assert DockerCodeGraphRunner is not None" + ), + ], + cwd=reviewer_root, + env=env, + check=False, + capture_output=True, + text=True, + ) + + assert completed.returncode == 0, completed.stderr + + +def test_agent_exports_remain_available_from_package_root() -> None: + """Lazy loading must preserve the existing package-level agent API.""" + + assert noema_reviewer.build_agent is not None + assert noema_reviewer.ReviewAgent is not None + assert noema_reviewer.PydanticAIReviewAgent is not None + + +def test_unknown_package_export_fails_normally() -> None: + """Unknown package attributes must still raise the standard error.""" + + with pytest.raises(AttributeError, match="has no attribute"): + getattr(noema_reviewer, "missing_runtime_export") diff --git a/reviewer/tests/test_verdict_invariants.py b/reviewer/tests/test_verdict_invariants.py index 355f826db..7d560a99d 100644 --- a/reviewer/tests/test_verdict_invariants.py +++ b/reviewer/tests/test_verdict_invariants.py @@ -5,16 +5,21 @@ import pytest from pydantic import ValidationError -from noema_reviewer.models import Finding, ReviewVerdict, Severity, Verdict +from noema_reviewer.models import EvidenceType, Finding, Priority, ReviewVerdict, Severity, Verdict def _finding(severity: Severity) -> Finding: """Build one concrete reviewer finding at the requested severity.""" return Finding( severity=severity, + priority=Priority.P1, path="src/example.py", evidence="current-head test evidence", + evidence_type=EvidenceType.NEARBY_IMPLEMENTATION, + observable_impact="The reviewed behavior fails.", + trigger="Running the affected code path.", recommendation="fix the defect", + regression_command="uv run pytest reviewer/tests/test_verdict_invariants.py", ) diff --git a/scripts/hourly-commercial-readiness.mjs b/scripts/hourly-commercial-readiness.mjs index 34b61b035..eed83167a 100644 --- a/scripts/hourly-commercial-readiness.mjs +++ b/scripts/hourly-commercial-readiness.mjs @@ -425,6 +425,33 @@ function dispatchNoemaReview(repository, pullNumber, expectedHeadSha) { ); } +function dispatchProductDevelopment(repository) { + const activeRuns = paginatedObjectItems( + `repos/${repository}/actions/workflows/hourly-product-development.yml/runs?per_page=100`, + "workflow_runs", + ); + if (activeRuns.some((run) => ( + activeWorkflowRunStatuses.has(String(run?.status ?? "").toLowerCase()) + ))) { + return false; + } + runGh( + [ + "api", "-X", "POST", + `repos/${repository}/actions/workflows/hourly-product-development.yml/dispatches`, + "--input", "-", + ], + { input: JSON.stringify({ ref: "main", inputs: { dry_run: "false" } }) }, + ); + return true; +} + +export function shouldDispatchProductDevelopment(apply, operationalErrorCount) { + return apply === true + && Number.isInteger(operationalErrorCount) + && operationalErrorCount === 0; +} + function mergePullRequest(repository, snapshot, trustedNoemaReviewerLogin) { const expectedHeadSha = snapshot.headSha; assertLiveHead(repository, snapshot.number, expectedHeadSha); @@ -619,6 +646,20 @@ export function main(argv = process.argv.slice(2)) { }); } + if (shouldDispatchProductDevelopment(apply, operationalErrors.length)) { + try { + report.productDevelopmentDispatched = dispatchProductDevelopment(repository); + } catch (error) { + const detail = bound(error?.message || error, MAX_ERROR_CHARS); + operationalErrors.push(detail); + report.results.push({ + number: null, + result: "operational_error", + reasons: [{ code: "product_development_dispatch_failed", detail }], + }); + } + } + writeReport(reportPath, report); console.log(JSON.stringify({ repository, diff --git a/test/ci-exact-head-contract.test.ts b/test/ci-exact-head-contract.test.ts index 7112b158d..82a00e724 100644 --- a/test/ci-exact-head-contract.test.ts +++ b/test/ci-exact-head-contract.test.ts @@ -6,8 +6,13 @@ const workflowPaths = [ ".github/workflows/reviewer-ci.yml", ] as const; +const requiredVerificationWorkflowPaths = [ + ...workflowPaths, + ".github/workflows/patch-validator-image.yml", +] as const; + /** Read one authoritative pull-request verification workflow as plain text. */ -function readWorkflow(path: (typeof workflowPaths)[number]): string { +function readWorkflow(path: string): string { return readFileSync(path, "utf8"); } @@ -107,4 +112,11 @@ describe("pull-request verification exact-head checkout contract", () => { "- name: install (hash-pinned dependencies)", ); }); + + it("does not suppress required exact-head evidence for documentation-only changes", () => { + for (const path of requiredVerificationWorkflowPaths) { + const workflow = readWorkflow(path); + expect(workflow).not.toContain("paths-ignore:"); + } + }); }); diff --git a/test/documentation-architecture-contract.test.ts b/test/documentation-architecture-contract.test.ts index 4a7222c3d..9baf45415 100644 --- a/test/documentation-architecture-contract.test.ts +++ b/test/documentation-architecture-contract.test.ts @@ -123,6 +123,25 @@ describe("authoritative Noema documentation graph", () => { expect(automationOwnership).not.toContain("stacked target branch does not trigger"); }); + it("keeps protected publisher race controls code-current in the threat model", () => { + const threatModel = document("docs/automation-threat-model.md"); + const publisher = readFileSync( + ".github/workflows/hourly-product-development.yml", + "utf8", + ); + + expect(publisher).toContain( + 'git push --force-with-lease="refs/heads/${branch}:" origin "HEAD:refs/heads/${branch}"', + ); + expect(publisher).toContain( + 'git push --force-with-lease="refs/heads/${branch}:${proposal_head}" origin ":refs/heads/${branch}"', + ); + expect(publisher).toContain("recover_created_pr_number"); + expect(publisher).toContain("publication_marker"); + expect(threatModel).toContain("**Controls implemented on protected `main`:**"); + expect(threatModel).not.toContain("not implemented on protected `main`"); + }); + it("keeps immutable workflow-source trust separate from revision-local canonical-byte hardening", () => { const architecture = document("ARCHITECTURE.md"); const traceability = document("docs/TRACEABILITY.md"); diff --git a/test/helpers/hourly-workflow.ts b/test/helpers/hourly-workflow.ts index 6c47a7a24..ecf73ffab 100644 --- a/test/helpers/hourly-workflow.ts +++ b/test/helpers/hourly-workflow.ts @@ -1,16 +1,5 @@ -/** Seconds reserved for setup work and the stable terminal diagnostic. */ -export const SETUP_AND_DIAGNOSTIC_RESERVE_SECONDS = 300; - const singleRunStepName = "- name: Run one contextual-orchestrator OpenCode session"; -/** Parsed single-run and proposer-job budgets from the production workflow. */ -export interface SingleRunBudget { - runSeconds: number; - killGraceSeconds: number; - jobSeconds: number; - totalSeconds: number; -} - /** * Return one complete job block from the workflow text. * @@ -43,73 +32,6 @@ export function readJobSlice( return workflow.slice(start, end); } -/** - * Parse one required positive integer capture from workflow text. - * - * @param text Workflow fragment to inspect. - * @param pattern Pattern whose first capture is the decimal value. - * @param label Human-readable contract name for diagnostics. - * @returns Parsed positive safe integer. - * @throws {Error} When the contract is absent or not a positive safe integer. - */ -function readPositiveCapture( - text: string, - pattern: RegExp, - label: string, -): number { - const match = text.match(pattern); - if (match === null) { - throw new Error(`Workflow ${label} is missing.`); - } - const value = Number(match[1]); - if (!Number.isSafeInteger(value) || value <= 0) { - throw new Error(`Workflow ${label} is not a positive safe integer.`); - } - return value; -} - -/** - * Read the configured single-run and proposer-job budgets. - * - * Sequential model-candidate failover is forbidden, so the budget is one - * gateway-backed OpenCode session plus setup/diagnostic reserve. - * - * @param workflow Complete workflow YAML. - * @returns Parsed budget values and their enforced worst-case total. - */ -export function readSingleRunBudget(workflow: string): SingleRunBudget { - const proposer = readJobSlice( - workflow, - "propose_product_increment", - "package_product_increment", - ); - const runSeconds = readPositiveCapture( - workflow, - /OPENCODE_RUN_TIMEOUT_SECONDS: "(\d+)"/, - "OpenCode run timeout", - ); - const killGraceSeconds = readPositiveCapture( - workflow, - /OPENCODE_KILL_GRACE_SECONDS: "(\d+)"/, - "OpenCode kill grace", - ); - const jobMinutes = readPositiveCapture( - proposer, - /timeout-minutes: (\d+)/, - "proposal-job timeout", - ); - const jobSeconds = jobMinutes * 60; - const totalSeconds = runSeconds + killGraceSeconds - + SETUP_AND_DIAGNOSTIC_RESERVE_SECONDS; - - return { - runSeconds, - killGraceSeconds, - jobSeconds, - totalSeconds, - }; -} - /** * Return the single OpenCode session step, failing if sequential fallback remains. * diff --git a/test/hourly-commercial-readiness-script.test.ts b/test/hourly-commercial-readiness-script.test.ts index 9602dda19..864b05c0d 100644 --- a/test/hourly-commercial-readiness-script.test.ts +++ b/test/hourly-commercial-readiness-script.test.ts @@ -1,282 +1,188 @@ -import { readFileSync } from "node:fs"; -import { describe, expect, it } from "vitest"; +import { spawnSync } from "node:child_process"; +import { + appendFileSync, + mkdtempSync, + readFileSync, + rmSync, + statSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { + evaluatePullRequest, + REQUIRED_CHECK_NAMES, +} from "../scripts/lib/commercial-readiness-loop.mjs"; import { - createGhSubprocessEnvironment, - flattenArrayPages, - hasActiveNoemaReviewRun, latestCheckRunsBySuite, - latestReviewStates, + main, parseNoemaReviewDecision, redactSensitiveValue, + shouldDispatchProductDevelopment, } from "../scripts/hourly-commercial-readiness.mjs"; -const repository = "ContextualWisdomLab/noema"; -const headSha = "b".repeat(40); -const trustedNoemaReviewerLogin = "noema-reviewer[bot]"; - -function review({ - login = trustedNoemaReviewerLogin, - type = "Bot", - state = "APPROVED", - body = `- Reviewer credential: \`noema-github-app\`\n`, - submittedAt = "2026-08-03T00:00:00Z", - id = 1, -} = {}) { +vi.mock("node:child_process", () => ({ + spawnSync: vi.fn(), +})); + +const roots: string[] = []; +const originalEnvironment = { ...process.env }; + +const requiredCheckRuns = REQUIRED_CHECK_NAMES.map((name) => ({ + name, + appSlug: "github-actions", + status: "completed", + conclusion: "success", +})); + +afterEach(() => { + vi.restoreAllMocks(); + vi.mocked(spawnSync).mockReset(); + process.env = { ...originalEnvironment }; + while (roots.length > 0) { + rmSync(roots.pop()!, { recursive: true, force: true }); + } +}); + +function tempReportPath(): string { + const root = mkdtempSync(join(tmpdir(), "noema-commercial-readiness-")); + roots.push(root); + return join(root, "report.json"); +} + +function snapshot(overrides = {}) { return { - id, - state, - body, - submitted_at: submittedAt, - user: { login, type }, + repository: "ContextualWisdomLab/noema", + number: 77, + title: "fix: bounded current-head repair", + state: "open", + draft: false, + baseRef: "main", + headRepository: "ContextualWisdomLab/noema", + headSha: "a".repeat(40), + mergeable: true, + mergeableState: "clean", + unresolvedThreadCount: 0, + latestReviewStates: [], + noemaReviewDecision: "approve", + checkRuns: requiredCheckRuns.map((check) => ({ ...check })), + statuses: [], + ...overrides, }; } -describe("hourly commercial-readiness GitHub adapter", () => { - it("flattens every array page returned by gh --paginate --slurp", () => { - expect(flattenArrayPages([[{ id: 1 }], [{ id: 2 }], []])).toEqual([ - { id: 1 }, - { id: 2 }, - ]); - }); - - it("keeps only the newest rerun within one check suite", () => { - expect(latestCheckRunsBySuite([ +describe("hourly commercial readiness script", () => { + it("prefers the latest check run within a suite and rejects older success", () => { + const latest = latestCheckRunsBySuite([ { - id: 100, - name: "verify", + id: 10, + name: "ci", status: "completed", - conclusion: "failure", - completed_at: "2026-08-03T00:00:00Z", + conclusion: "success", + check_suite: { id: 30 }, app: { slug: "github-actions" }, - check_suite: { id: 50 }, }, { - id: 101, - name: "verify", - status: "completed", - conclusion: "success", - completed_at: "2026-08-03T00:05:00Z", + id: 11, + name: "ci", + status: "in_progress", + conclusion: null, + check_suite: { id: 30 }, app: { slug: "github-actions" }, - check_suite: { id: 50 }, }, - ])).toEqual([ - expect.objectContaining({ id: 101, conclusion: "success" }), + ]); + + expect(latest).toEqual([ + expect.objectContaining({ id: 11, name: "ci", status: "in_progress" }), ]); }); - it("keeps a higher-id queued rerun even before GitHub assigns timestamps", () => { - expect(latestCheckRunsBySuite([ + it("fails closed when a check run omits suite identity metadata", () => { + expect(() => latestCheckRunsBySuite([ { - id: 100, - name: "verify", + id: 10, + name: "ci", status: "completed", conclusion: "success", - completed_at: "2026-08-03T00:05:00Z", - app: { slug: "github-actions" }, - check_suite: { id: 50 }, - }, - { - id: 101, - name: "verify", - status: "queued", - conclusion: null, - started_at: null, - completed_at: null, app: { slug: "github-actions" }, - check_suite: { id: 50 }, }, - ])).toEqual([ - expect.objectContaining({ id: 101, status: "queued" }), - ]); + ])).toThrow("Check run identity metadata is incomplete for id 10."); }); - it.each([ - { - checkRuns: [ - { id: 1, name: "verify", app: { slug: "github-actions" }, check_suite: null }, - ], - }, - { - checkRuns: [ - { id: 2, name: "", app: { slug: "github-actions" }, check_suite: { id: 50 } }, - ], - }, - { - checkRuns: [ - { id: 3, name: "verify", app: null, check_suite: { id: 50 } }, - ], - }, - ])("fails closed on incomplete check-run identity metadata", ({ checkRuns }) => { - expect(() => latestCheckRunsBySuite(checkRuns)).toThrow( - "Check run identity metadata is incomplete", - ); - }); - - it("preserves same-name checks from different current suites", () => { - expect(latestCheckRunsBySuite([ - { - id: 101, + it("fails closed when exact-head required checks are missing", () => { + const decision = evaluatePullRequest(snapshot({ + checkRuns: [{ name: "verify", + appSlug: "github-actions", status: "completed", conclusion: "success", - completed_at: "2026-08-03T00:05:00Z", - app: { slug: "github-actions" }, - check_suite: { id: 50 }, - }, - { - id: 201, - name: "verify", - status: "queued", - conclusion: null, - started_at: "2026-08-03T00:06:00Z", - app: { slug: "github-actions" }, - check_suite: { id: 60 }, - }, - ])).toHaveLength(2); + }], + })); + + expect(decision.action).toBe("blocked"); + expect(decision.reasons.map((reason) => reason.code)).toContain("required_check_missing"); }); - it("requires the exact configured reviewer login, current-head marker, and App credential", () => { - expect( - parseNoemaReviewDecision([review()], headSha, trustedNoemaReviewerLogin), - ).toBe("approve"); - expect( - parseNoemaReviewDecision( - [review({ login: "human", type: "User" })], - headSha, - trustedNoemaReviewerLogin, - ), - ).toBeNull(); - expect( - parseNoemaReviewDecision( - [review({ login: "other-app[bot]" })], - headSha, - trustedNoemaReviewerLogin, - ), - ).toBeNull(); - expect( - parseNoemaReviewDecision( - [review({ login: "noema-spoof[bot]" })], - headSha, - trustedNoemaReviewerLogin, - ), - ).toBeNull(); - expect( - parseNoemaReviewDecision([ - review({ - body: ``, - }), - ], headSha, trustedNoemaReviewerLogin), - ).toBeNull(); - expect( - parseNoemaReviewDecision([ - review({ - body: `- Reviewer credential: \`noema-github-app\`\n`, - }), - ], headSha, trustedNoemaReviewerLogin), - ).toBeNull(); + it("requests an exact-head reviewer when all independent gates are green", () => { + const decision = evaluatePullRequest(snapshot({ noemaReviewDecision: null })); + + expect(decision.action).toBe("request_review"); + expect(decision.reasons).toEqual([ + expect.objectContaining({ code: "noema_current_head_approval_missing" }), + ]); }); - it("uses the newest authenticated Noema decision for the current head", () => { - const reviews = [ - review({ submittedAt: "2026-08-03T00:00:00Z", id: 10 }), - review({ - state: "CHANGES_REQUESTED", - body: `- Reviewer credential: \`noema-github-app\`\n`, - submittedAt: "2026-08-03T00:05:00Z", - id: 11, - }), - ]; + it("merges only with exact-head trusted approval and no unresolved threads", () => { + const decision = evaluatePullRequest(snapshot()); - expect( - parseNoemaReviewDecision(reviews, headSha, trustedNoemaReviewerLogin), - ).toBe("request_changes"); + expect(decision.action).toBe("merge"); + expect(decision.reasons).toEqual([]); }); - it("reduces review submissions to the latest effective decision per reviewer", () => { - expect( - latestReviewStates([ - review({ login: "alice", type: "User", state: "CHANGES_REQUESTED", id: 1 }), - review({ - login: "alice", - type: "User", - state: "APPROVED", - submittedAt: "2026-08-03T00:10:00Z", - id: 2, - }), - review({ login: "bob", type: "User", state: "COMMENTED", id: 3 }), - ]), - ).toEqual([{ reviewer: "alice", state: "APPROVED" }]); + it("rejects stale trusted approval", () => { + const staleHead = "b".repeat(40); + const currentHead = "a".repeat(40); + const noemaReviewDecision = parseNoemaReviewDecision([ + { + id: 99, + submitted_at: "2026-09-05T00:00:00Z", + commit_id: staleHead, + state: "APPROVED", + user: { login: "noema-reviewer[bot]", type: "Bot" }, + body: [ + "Reviewer credential: `noema-github-app`", + ``, + ].join("\n"), + }, + ], currentHead, "noema-reviewer[bot]"); + + expect(noemaReviewDecision).toBeNull(); + expect(evaluatePullRequest(snapshot({ noemaReviewDecision })).action).toBe("request_review"); }); - it("retains untrusted Noema-like bot change requests as effective reviews", () => { - expect( - latestReviewStates([ - review({ - login: "noema-spoof[bot]", - type: "Bot", - state: "CHANGES_REQUESTED", - body: "untrusted review without a Noema credential marker", - }), - ]), - ).toEqual([{ reviewer: "noema-spoof[bot]", state: "CHANGES_REQUESTED" }]); + it("blocks when a current-head approval has unresolved review threads", () => { + const decision = evaluatePullRequest(snapshot({ unresolvedThreadCount: 1 })); + + expect(decision.action).toBe("blocked"); + expect(decision.reasons.map((reason) => reason.code)).toContain("unresolved_review_threads"); }); - it("recognizes only an active exact-target central review run", () => { - const title = `Noema central review ${repository}#28@${headSha}`; - expect( - hasActiveNoemaReviewRun([ - { event: "repository_dispatch", status: "queued", display_title: title }, - ], repository, 28, headSha), - ).toBe(true); - expect( - hasActiveNoemaReviewRun([ - { event: "repository_dispatch", status: "completed", display_title: title }, - ], repository, 28, headSha), - ).toBe(false); - expect( - hasActiveNoemaReviewRun([ - { - event: "repository_dispatch", - status: "in_progress", - display_title: `Noema central review ${repository}#28@${"c".repeat(40)}`, - }, - ], repository, 28, headSha), - ).toBe(false); + it("blocks draft and non-mergeable pull requests", () => { + expect(evaluatePullRequest(snapshot({ draft: true })).action).toBe("blocked"); + expect(evaluatePullRequest(snapshot({ mergeable: false })).action).toBe("blocked"); }); - it("passes only explicit GitHub CLI authority into child processes", () => { - expect(createGhSubprocessEnvironment({ - PATH: "/trusted/bin", - GH_TOKEN: "read-only-maintainer-token", - GH_HOST: "evil.example", - NO_COLOR: "0", - GITHUB_TOKEN: "ambient-workflow-token", - NVIDIA_NIM_API_KEY: "model-secret", - NOEMA_MAINTAINER_APP_PRIVATE_KEY: "maintainer-private-key", - NOEMA_REVIEWER_APP_PRIVATE_KEY: "reviewer-private-key", - NOEMA_REVIEWER_LOGIN: "reviewer[bot]", - CLOUDFLARE_API_TOKEN: "cloudflare-secret", - HTTPS_PROXY: "http://proxy.invalid", - HTTP_PROXY: "http://proxy.invalid", - ALL_PROXY: "socks5://proxy.invalid", - HOME: "/credential-bearing-home", - NODE_OPTIONS: "--require /tmp/preload.cjs", - NOEMA_MAINTENANCE_ENABLED: "true", - })).toEqual({ - GH_HOST: "github.com", - NO_COLOR: "1", - PATH: "/trusted/bin", - GH_TOKEN: "read-only-maintainer-token", - }); - - expect(createGhSubprocessEnvironment({})).toEqual({ - GH_HOST: "github.com", - NO_COLOR: "1", - }); + it("dispatches product development work-conservingly when apply mode has no operational error", () => { + expect(shouldDispatchProductDevelopment(true, 0)).toBe(true); + expect(shouldDispatchProductDevelopment(false, 0)).toBe(false); + expect(shouldDispatchProductDevelopment(true, 1)).toBe(false); + expect(shouldDispatchProductDevelopment(true, Number.NaN)).toBe(false); }); - it("redacts an explicit maintainer token before child diagnostics can reach retained outputs", () => { - const token = "read-only-maintainer-token"; + it("redacts repeated sensitive values in diagnostics", () => { + const token = "ghs_secret-value"; const detail = `gh failed with ${token}; retry also exposed ${token}`; expect(redactSensitiveValue(detail, [token])).toBe( @@ -309,6 +215,10 @@ describe("hourly commercial-readiness GitHub adapter", () => { expect(script).toContain("actions/workflows/central-review.yml/runs?event=repository_dispatch&per_page=100"); expect(script).toContain("NOEMA_REVIEWER_LOGIN"); expect(script).toContain('event_type: "noema-review"'); + expect(script).toContain("actions/workflows/hourly-product-development.yml/dispatches"); + expect(script).toContain('JSON.stringify({ ref: "main", inputs: { dry_run: "false" } })'); + expect(script).toContain("shouldDispatchProductDevelopment(apply, operationalErrors.length)"); + expect(script).not.toContain("report.remainingOpenPullRequestCount === 0"); expect(script).toContain('merge_method: "squash"'); expect(script).toContain("sha: expectedHeadSha"); expect(script).toContain("live?.head?.sha !== expectedHeadSha"); @@ -327,30 +237,38 @@ describe("hourly commercial-readiness GitHub adapter", () => { expect(script).not.toContain("read-only-maintainer-token"); }); - it("documents the operator contract and buyer-visible governance boundaries", () => { - const readme = readFileSync("README.md", "utf8"); - const guide = readFileSync("docs/hourly-commercial-readiness-loop.md", "utf8"); - const changelog = readFileSync("CHANGELOG.md", "utf8"); - const combined = `${readme}\n${guide}\n${changelog}`; - - for (const requiredText of [ - ".github/workflows/hourly-commercial-readiness.yml", - "commercial-readiness-loop-report", - "SHA-bound", - "NOEMA_REVIEWER_LOGIN", - "verify", - "reviewer", - "scorecard", - "osv-scan", - "trivy-fs", - "dependency-review", - "issue #27", - "issue #9", - ]) { - expect(combined).toContain(requiredText); - } - expect(guide).toContain("review-dependent checks"); - expect(guide).toContain("production KPI"); - expect(guide).toContain("revenue evidence"); + it("keeps report files private and appends explicit workflow outputs", () => { + const reportPath = tempReportPath(); + const root = roots.at(-1)!; + const outputPath = join(root, "github-output.txt"); + const summaryPath = join(root, "summary.md"); + const tokenPath = join(root, "maintainer-token"); + process.env.GITHUB_OUTPUT = outputPath; + process.env.GITHUB_STEP_SUMMARY = summaryPath; + process.env.GITHUB_REPOSITORY = "ContextualWisdomLab/noema"; + process.env.NOEMA_REVIEWER_LOGIN = "noema-reviewer[bot]"; + process.env.NOEMA_MAINTAINER_TOKEN_PATH = tokenPath; + + appendFileSync(outputPath, "preexisting=value\n", "utf8"); + appendFileSync(summaryPath, "preexisting summary\n", "utf8"); + writeFileSync(tokenPath, "ghs_test-token", { encoding: "utf8", mode: 0o600 }); + + vi.mocked(spawnSync).mockReturnValue({ + status: 0, + stdout: "[]", + stderr: "", + pid: 1, + output: [null, "[]", ""], + signal: null, + } as never); + + const report = main(["--report", reportPath]); + + const persisted = JSON.parse(readFileSync(reportPath, "utf8")); + expect(persisted.openPullRequestCount).toBe(report.openPullRequestCount); + expect(persisted.remainingOpenPullRequestCount).toBe(0); + expect(statSync(reportPath).mode & 0o777).toBe(0o600); + expect(readFileSync(outputPath, "utf8")).toContain("open_pull_request_count=0"); + expect(readFileSync(summaryPath, "utf8")).toContain("Noema commercial-readiness loop"); }); }); diff --git a/test/hourly-commercial-readiness-work-conserving-dispatch.test.ts b/test/hourly-commercial-readiness-work-conserving-dispatch.test.ts new file mode 100644 index 000000000..3ebe7f879 --- /dev/null +++ b/test/hourly-commercial-readiness-work-conserving-dispatch.test.ts @@ -0,0 +1,13 @@ +import { describe, expect, it } from "vitest"; +import { shouldDispatchProductDevelopment } from "../scripts/hourly-commercial-readiness.mjs"; + +describe("work-conserving product-development admission", () => { + it("keeps product development eligible after a healthy readiness pass even while PR lanes remain open", () => { + expect(shouldDispatchProductDevelopment(true, 0)).toBe(true); + }); + + it("does not dispatch from dry-run or operational-error passes", () => { + expect(shouldDispatchProductDevelopment(false, 0)).toBe(false); + expect(shouldDispatchProductDevelopment(true, 1)).toBe(false); + }); +}); diff --git a/test/hourly-product-development-final-candidate-cleanup.test.ts b/test/hourly-product-development-final-candidate-cleanup.test.ts index 424ecbc52..85cd7785e 100644 --- a/test/hourly-product-development-final-candidate-cleanup.test.ts +++ b/test/hourly-product-development-final-candidate-cleanup.test.ts @@ -1,9 +1,6 @@ import { readFileSync } from "node:fs"; import { describe, expect, it } from "vitest"; -import { - readSingleOrchestratorRunStep, - readSingleRunBudget, -} from "./helpers/hourly-workflow"; +import { readSingleOrchestratorRunStep } from "./helpers/hourly-workflow"; function workflowText(): string { return readFileSync( @@ -15,10 +12,8 @@ function workflowText(): string { describe("hourly product-development sequential-model prohibition", () => { it("runs exactly one gateway-backed session and never fails over to the next model", () => { const workflow = workflowText(); - const budget = readSingleRunBudget(workflow); const runStep = readSingleOrchestratorRunStep(workflow); - expect(budget.totalSeconds).toBeLessThanOrEqual(budget.jobSeconds); expect(workflow).not.toContain("OPENCODE_MODEL_CANDIDATES"); expect(workflow).not.toContain("nvidia-nim/"); expect(workflow).not.toContain("NVIDIA_NIM_API_KEY"); diff --git a/test/hourly-product-development-no-model-timeout.test.ts b/test/hourly-product-development-no-model-timeout.test.ts new file mode 100644 index 000000000..a4e58e124 --- /dev/null +++ b/test/hourly-product-development-no-model-timeout.test.ts @@ -0,0 +1,22 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; +import { readJobSlice } from "./helpers/hourly-workflow"; + +const workflowPath = ".github/workflows/hourly-product-development.yml"; + +describe("hourly product-development termination authority", () => { + it("keeps the GitHub job administration bound distinct from model execution", () => { + const workflow = readFileSync(workflowPath, "utf8"); + const proposer = readJobSlice( + workflow, + "propose_product_increment", + "package_product_increment", + ); + + expect(proposer).toContain("timeout-minutes: 55"); + expect(workflow).not.toContain("OPENCODE_RUN_TIMEOUT_SECONDS"); + expect(workflow).not.toContain("OPENCODE_KILL_GRACE_SECONDS"); + expect(workflow).not.toContain("timeout --kill-after="); + expect(workflow).toContain('opencode run "$prompt" --agent build'); + }); +}); diff --git a/test/hourly-product-development-runner-isolation.test.ts b/test/hourly-product-development-runner-isolation.test.ts index 4dc9bb77f..77541ff47 100644 --- a/test/hourly-product-development-runner-isolation.test.ts +++ b/test/hourly-product-development-runner-isolation.test.ts @@ -60,7 +60,7 @@ describe("hourly product-development runner isolation", () => { "Mint dedicated maintainer App token only for publication", ); const revalidationIndex = publisher.indexOf( - "Revalidate queue and default-branch head", + "Revalidate open-PR path isolation and default-branch head", ); expect(applyIndex).toBeGreaterThan(-1); @@ -110,4 +110,4 @@ describe("hourly product-development runner isolation", () => { ); } }); -}); +}); \ No newline at end of file diff --git a/test/hourly-product-development-workflow.test.ts b/test/hourly-product-development-workflow.test.ts index 08251b516..5b0d30d96 100644 --- a/test/hourly-product-development-workflow.test.ts +++ b/test/hourly-product-development-workflow.test.ts @@ -3,7 +3,6 @@ import { describe, expect, it } from "vitest"; import { readJobSlice, readSingleOrchestratorRunStep, - readSingleRunBudget, } from "./helpers/hourly-workflow"; const workflowPath = ".github/workflows/hourly-product-development.yml"; @@ -16,13 +15,18 @@ function metadataParserText(): string { return readFileSync("scripts/prepare-agent-pr-message.mjs", "utf8"); } -describe("hourly contextual-orchestrator OpenCode product-development workflow", () => { - it("runs hourly without overlapping deterministic commercial-readiness governance", () => { +function centralCallerText(): string { + return readFileSync("scripts/hourly-commercial-readiness.mjs", "utf8"); +} + +describe("centrally dispatched contextual-orchestrator product-development workflow", () => { + it("leaves cadence and admission to central commercial-readiness governance", () => { const workflow = workflowText(); expect(workflow).toContain("workflow_dispatch:"); expect(workflow).toContain("dry_run:"); - expect(workflow).toContain('cron: "47 * * * *"'); + expect(workflow).not.toContain("schedule:"); + expect(workflow).not.toContain("cron:"); expect(workflow).toContain( "group: hourly-orchestrator-product-development-${{ github.repository }}", ); @@ -30,8 +34,12 @@ describe("hourly contextual-orchestrator OpenCode product-development workflow", expect(workflow).toContain( "github.repository == 'ContextualWisdomLab/noema'", ); - expect(workflow).not.toContain('cron: "17 * * * *"'); expect(workflow).not.toContain("pull_request_target:"); + + const caller = centralCallerText(); + expect(caller).toContain("actions/workflows/hourly-product-development.yml/dispatches"); + expect(caller).toContain('ref: "main"'); + expect(caller).toContain('inputs: { dry_run: "false" }'); }); it("separates model execution, untrusted verification, and publication authority by job", () => { @@ -97,7 +105,7 @@ describe("hourly contextual-orchestrator OpenCode product-development workflow", "Mint dedicated maintainer App token only for publication", ); const revalidationIndex = publisher.indexOf( - "Revalidate queue and default-branch head", + "Revalidate open-PR path isolation and default-branch head", ); expect(metadataIndex).toBeGreaterThan(-1); expect(tokenIndex).toBeGreaterThan(metadataIndex); @@ -123,7 +131,8 @@ describe("hourly contextual-orchestrator OpenCode product-development workflow", expect(workflow).toContain("--state open"); expect(workflow).toContain("--limit 1"); expect(workflow).toContain("pull_request_inventory_unavailable"); - expect(workflow).toContain("open_pull_request"); + expect(workflow).toContain("open_pull_request_count"); + expect(workflow).not.toContain('echo "reason=open_pull_request"'); expect(workflow).toContain("orchestrator_gateway_unavailable"); expect(workflow).toContain( "ORCHESTRATOR_KEY_CONFIGURED: ${{ secrets.NOEMA_LLM_API_KEY != '' }}", @@ -208,15 +217,19 @@ describe("hourly contextual-orchestrator OpenCode product-development workflow", expect(workflow).not.toContain('"bash": {'); }); - it("fits one gateway-backed session, termination grace, and diagnostics inside the proposal-job budget", () => { + it("leaves model execution without a Noema elapsed-time cutoff", () => { const workflow = workflowText(); - const budget = readSingleRunBudget(workflow); + const proposer = readJobSlice( + workflow, + "propose_product_increment", + "package_product_increment", + ); const runStep = readSingleOrchestratorRunStep(workflow); - expect(budget.totalSeconds).toBeLessThanOrEqual(budget.jobSeconds); - expect(workflow).toContain( - 'timeout --kill-after="${OPENCODE_KILL_GRACE_SECONDS}s" "${OPENCODE_RUN_TIMEOUT_SECONDS}s"', - ); + expect(proposer).toContain("timeout-minutes: 55"); + expect(workflow).not.toContain("OPENCODE_RUN_TIMEOUT_SECONDS"); + expect(workflow).not.toContain("OPENCODE_KILL_GRACE_SECONDS"); + expect(workflow).not.toContain("timeout --kill-after="); expect(runStep).toContain("opencode run \"$prompt\" --agent build"); expect(runStep).not.toContain("OPENCODE_MODEL_CANDIDATES"); expect(runStep).not.toContain("model_candidates"); @@ -258,11 +271,11 @@ describe("hourly contextual-orchestrator OpenCode product-development workflow", expect(workflow).not.toMatch(/gh pr merge|gh release create|wrangler deploy/); }); - it("revalidates queue and base head before remote proposal mutation", () => { + it("revalidates path-isolated queue state and base head before remote proposal mutation", () => { const workflow = workflowText(); const publisher = readJobSlice(workflow, "publish_product_increment"); const revalidationIndex = publisher.indexOf( - "Revalidate queue and default-branch head", + "Revalidate open-PR path isolation and default-branch head", ); const pushIndex = publisher.indexOf( 'git push --force-with-lease="refs/heads/${branch}:" origin "HEAD:refs/heads/${branch}"', @@ -282,7 +295,12 @@ describe("hourly contextual-orchestrator OpenCode product-development workflow", expect(workflow).toContain( "pull_request_inventory_unavailable_after_generation", ); - expect(workflow).toContain("open_pull_request_after_generation"); + expect(workflow).toContain("open_pull_request_after_generation_path_overlap"); + expect(workflow).toContain("pull_request_file_inventory_incomplete_after_generation"); + expect(workflow).toContain("pull_request_file_inventory_unbounded_after_generation"); + expect(workflow).toContain("proposal-paths.b64"); + expect(workflow).toContain("verify-open-pr-path-isolation.sh"); + expect(workflow).toContain('"$RUNNER_TEMP/verify-open-pr-path-isolation.sh" "$pr_number"'); expect(workflow).toContain("base_branch_advanced"); expect(workflow).toContain("proposal_branch_create_lease_rejected"); expect(revalidationIndex).toBeGreaterThan(-1); @@ -362,7 +380,7 @@ describe("hourly contextual-orchestrator OpenCode product-development workflow", "NOEMA_LLM_API_KEY", "contextual-orchestrator", "OpenCode 1.17.13", - "열린 PR 0개", + "경로 격리", "자격 증명", "hourly-commercial-readiness", "proposal.patch", diff --git a/test/noema-core-packaging-contract.test.ts b/test/noema-core-packaging-contract.test.ts new file mode 100644 index 000000000..e464de8db --- /dev/null +++ b/test/noema-core-packaging-contract.test.ts @@ -0,0 +1,62 @@ +import { readFileSync } from "node:fs"; + +import { describe, expect, it } from "vitest"; + +const centralReview = readFileSync(".github/workflows/central-review.yml", "utf8"); +const reviewerCi = readFileSync(".github/workflows/reviewer-ci.yml", "utf8"); +const reviewerPyproject = readFileSync("reviewer/pyproject.toml", "utf8"); +const reviewerBuildBackend = readFileSync("reviewer/build_backend.py", "utf8"); +const reviewerManifest = readFileSync("reviewer/MANIFEST.in", "utf8"); +const corePyproject = readFileSync("packages/noema-core/pyproject.toml", "utf8"); + +describe("noema-core packaging and workflow contract", () => { + it("makes the shared core importable everywhere reviewer code runs", () => { + const sharedPath = + "PYTHONPATH: ${{ github.workspace }}/reviewer:${{ github.workspace }}/packages/noema-core/src"; + + expect(centralReview).toContain(sharedPath); + expect(reviewerCi).toContain(sharedPath); + expect(reviewerCi).not.toContain("PYTHONPATH=. python"); + }); + + it("stages the canonical core into reviewer build artifacts until an immutable index release exists", () => { + expect(reviewerPyproject).toContain('build-backend = "build_backend"'); + expect(reviewerPyproject).toContain('backend-path = ["."]'); + expect(reviewerPyproject).toContain('[tool.setuptools]'); + expect(reviewerPyproject).toContain('packages = ["noema_reviewer", "noema_core"]'); + expect(reviewerPyproject).toContain('[tool.setuptools.package-dir]'); + expect(reviewerPyproject).toContain('noema_core = "_build_include/noema_core"'); + expect(reviewerBuildBackend).toContain('"packages" / "noema-core" / "src" / "noema_core"'); + expect(reviewerBuildBackend).toContain('from setuptools import build_meta as _setuptools'); + expect(reviewerBuildBackend).toContain('def build_sdist('); + expect(reviewerManifest).toContain('include build_backend.py'); + expect(reviewerManifest).toContain('recursive-include _build_include/noema_core *.py'); + expect(reviewerCi).toContain("smoke-test installed reviewer wheel and sdist-to-wheel path"); + expect(reviewerCi).toContain("from build_backend import build_sdist"); + expect(reviewerCi).toContain('python -m pip wheel "$sdist"'); + expect(reviewerCi).toContain("hashlib.sha256(installed_agent.read_bytes()).digest()"); + }); + + it("does not retain the obsolete out-of-tree setuptools package mapping", () => { + expect(reviewerPyproject).not.toContain( + 'noema_core = "../packages/noema-core/src/noema_core"', + ); + }); + + it("smokes a CLI symbol that the installed reviewer actually exports", () => { + expect(reviewerCi).toContain("from noema_reviewer.cli import parse_args"); + expect(reviewerCi).toContain('assert parse_args([]).repo == ""'); + expect(reviewerCi).not.toContain("from noema_reviewer.cli import build_parser"); + }); + + it("keeps the provider SDK extra at the reviewer integration adapter", () => { + expect(reviewerPyproject).toContain('"pydantic-ai-slim[openai]>=2.9.0,<3"'); + expect(corePyproject).toContain('"pydantic-ai-slim>=2.9.0,<3"'); + expect(corePyproject).not.toContain("pydantic-ai-slim[openai]"); + }); + + it("runs shared-core coverage and docstring gates in required reviewer CI", () => { + expect(reviewerCi).toContain("test noema-core (100% line+branch coverage gate)"); + expect(reviewerCi).toContain("docstring coverage noema-core (100% gate)"); + }); +}); diff --git a/test/patch-validator-image-build-cache.test.ts b/test/patch-validator-image-build-cache.test.ts index fdd364cf4..e7013415d 100644 --- a/test/patch-validator-image-build-cache.test.ts +++ b/test/patch-validator-image-build-cache.test.ts @@ -22,11 +22,13 @@ describe("patch-validator image build cache", () => { ); }); - it("cancels superseded exact-head builds instead of spending the serial image lane on stale evidence", () => { + it("cancels only superseded pull-request builds while preserving non-PR runs", () => { expect(workflow).toContain( - "group: noema-patch-validator-image-${{ github.event.pull_request.number || github.ref }}", + "group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event_name == 'pull_request' && github.event.pull_request.number || github.run_id }}", + ); + expect(workflow).toContain( + "cancel-in-progress: ${{ github.event_name == 'pull_request' }}", ); - expect(workflow).toContain("cancel-in-progress: true"); }); it("retries transient scanner release download failures before failing closed", () => { diff --git a/test/reviewer-ci-action-runtime-integrity.test.ts b/test/reviewer-ci-action-runtime-integrity.test.ts index a32e68ee2..8f2cd5201 100644 --- a/test/reviewer-ci-action-runtime-integrity.test.ts +++ b/test/reviewer-ci-action-runtime-integrity.test.ts @@ -19,6 +19,15 @@ describe("reviewer CI action runtime integrity", () => { ); }); + it("installs wheel smoke artifacts outside source import authority", () => { + expect(workflow).toMatch( + /cd "\$RUNNER_TEMP"\n\s+PYTHONPATH='' "\$venv_dir\/bin\/python" -m pip install --no-deps "\$wheel"/, + ); + expect(workflow).not.toMatch( + /"\$venv_dir\/bin\/python" -m pip install --no-deps "\$wheel"\n\s+\(\n\s+cd "\$RUNNER_TEMP"/, + ); + }); + it("fails the CodeGraph smoke gate when semantic retrieval is empty", () => { expect(workflow).toContain( '["codegraph", "explore", "commercialReadiness"]', diff --git a/test/workflow-concurrency-policy.test.ts b/test/workflow-concurrency-policy.test.ts index ce42f2c74..f10996853 100644 --- a/test/workflow-concurrency-policy.test.ts +++ b/test/workflow-concurrency-policy.test.ts @@ -15,9 +15,11 @@ describe("pull-request workflow execution policy", () => { expect(workflow).toContain("concurrency:"); expect(workflow).toContain( - "${{ github.event.pull_request.number || github.ref }}", + "group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event_name == 'pull_request' && github.event.pull_request.number || github.run_id }}", + ); + expect(workflow).toContain( + "cancel-in-progress: ${{ github.event_name == 'pull_request' }}", ); - expect(workflow).toContain("cancel-in-progress: true"); }, );