From 15a9a3cd189d78264fcd85c6e1a691780336cb2e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 08:17:59 +0900 Subject: [PATCH 1/8] fix(noema): skip sidecar for ineligible reviews Reuse the exact-head, base, independent actor, Draft, and existing-review eligibility checks before provisioning the contextual-orchestrator sidecar. Re-run the same checks immediately before model work so state changes still fail closed. Refs #1992 Co-authored-by: Codex Signed-off-by: Seongho Bae --- .github/actions/noema-review/two_phase.py | 43 ++++++- .github/workflows/noema-review.yml | 31 ++++- CHANGELOG.md | 3 + docs/doctoring/noema-orchestrator-free-zdr.md | 18 +++ ...st_noema_orchestrator_workflow_contract.py | 28 +++++ tests/test_noema_two_phase_handoff.py | 114 ++++++++++++++++++ 6 files changed, 230 insertions(+), 7 deletions(-) diff --git a/.github/actions/noema-review/two_phase.py b/.github/actions/noema-review/two_phase.py index 2815d7a050..b4030621fe 100755 --- a/.github/actions/noema-review/two_phase.py +++ b/.github/actions/noema-review/two_phase.py @@ -145,23 +145,55 @@ def _read_envelope(path: Path) -> dict[str, Any]: return payload -def prepare_verdict(repo: str, number: int, expected_head: str, path: Path) -> int: - """Run model review and seal its verdict without publishing GitHub evidence.""" +def _model_work_eligibility( + repo: str, + number: int, + expected_head: str, +) -> tuple[str, dict[str, Any], str, str] | None: + """Return the validated review identity when this head still needs model work.""" expected = _canonical_head(expected_head) pull_request = gate.fetch_pr(repo, number) try: gate.require_expected_head(pull_request, expected) except RuntimeError: print("Pull request is closed or stale; Noema verdict preparation skipped.") - return 0 + return None expected_base = _canonical_base(pull_request) actor = _reviewer_actor() if pull_request.get("isDraft"): print("PR is draft; Noema verdict preparation skipped.") - return 0 + return None if gate.existing_noema_review(pull_request, actor): print("Current head already has a Noema review; verdict preparation skipped.") + return None + return expected, pull_request, expected_base, actor + + +def admit_model_work(repo: str, number: int, expected_head: str, path: Path) -> int: + """Record whether the current review needs the expensive model sidecar.""" + eligibility = _model_work_eligibility(repo, number, expected_head) + if eligibility is None: + return 0 + expected, _pull_request, expected_base, _actor = eligibility + _write_envelope( + path, + { + "schema_version": ENVELOPE_SCHEMA_VERSION, + "repository": repo, + "pull_request_number": number, + "expected_head": expected, + "expected_base": expected_base, + }, + ) + return 0 + + +def prepare_verdict(repo: str, number: int, expected_head: str, path: Path) -> int: + """Run model review and seal its verdict without publishing GitHub evidence.""" + eligibility = _model_work_eligibility(repo, number, expected_head) + if eligibility is None: return 0 + expected, pull_request, expected_base, _actor = eligibility diff, truncated = gate.fetch_diff(repo, number) changed_files = gate.fetch_changed_files(repo, number) @@ -253,6 +285,7 @@ def parse_args(argv: list[str]) -> argparse.Namespace: parser.add_argument("--pr-number", required=True, type=int) parser.add_argument("--expected-head", required=True) modes = parser.add_mutually_exclusive_group(required=True) + modes.add_argument("--admit-model-file", type=Path) modes.add_argument("--prepare-verdict-file", type=Path) modes.add_argument("--publish-verdict-file", type=Path) return parser.parse_args(argv) @@ -263,6 +296,8 @@ def main(argv: list[str]) -> int: args = parse_args(argv) if args.pr_number <= 0: raise SystemExit("--pr-number must be positive") + if args.admit_model_file is not None: + return admit_model_work(args.repo, args.pr_number, args.expected_head, args.admit_model_file) if args.prepare_verdict_file is not None: return prepare_verdict(args.repo, args.pr_number, args.expected_head, args.prepare_verdict_file) return publish_verdict(args.repo, args.pr_number, args.expected_head, args.publish_verdict_file) diff --git a/.github/workflows/noema-review.yml b/.github/workflows/noema-review.yml index f8ab55c896..54544d9636 100644 --- a/.github/workflows/noema-review.yml +++ b/.github/workflows/noema-review.yml @@ -599,8 +599,33 @@ jobs: exit 1 fi - - name: Resolve Noema target repository visibility + - name: Admit Noema model work if: env.PR_NUMBER != '' + id: noema_model_admission + env: + GH_TOKEN: ${{ secrets.NOEMA_REVIEW_TOKEN || steps.noema_github_app_token.outputs.token || steps.noema_oidc_token.outputs.token }} + NOEMA_REVIEW_TOKEN_SOURCE: ${{ steps.noema_credential.outputs.source == 'pat' && 'noema-review-pat' || steps.noema_credential.outputs.source == 'github-app' && 'noema-review-github-app' || 'noema-review-app-oidc' }} + NOEMA_REVIEW_ACTOR: ${{ steps.noema_github_app_token.outputs['app-slug'] && format('{0}[bot]', steps.noema_github_app_token.outputs['app-slug']) || '' }} + NOEMA_REVIEW_INSTALLATION_ID: ${{ steps.noema_github_app_token.outputs['installation-id'] }} + run: | + set -euo pipefail + admission_file="${RUNNER_TEMP}/noema-model-admission.json" + rm -f "$admission_file" + python3 "$GITHUB_WORKSPACE/.github/actions/noema-review/two_phase.py" \ + --repo "$TARGET_REPOSITORY" \ + --pr-number "$PR_NUMBER" \ + --expected-head "$EXPECTED_HEAD_SHA" \ + --admit-model-file "$admission_file" + if [ -f "$admission_file" ]; then + rm -f "$admission_file" + echo "admitted=true" >>"$GITHUB_OUTPUT" + else + echo "admitted=false" >>"$GITHUB_OUTPUT" + echo "::notice::Noema model work is unnecessary for the current pull request state." + fi + + - name: Resolve Noema target repository visibility + if: env.PR_NUMBER != '' && steps.noema_model_admission.outputs.admitted == 'true' id: target_visibility env: GH_TOKEN: ${{ secrets.NOEMA_REVIEW_TOKEN || steps.noema_github_app_token.outputs.token || steps.noema_oidc_token.outputs.token }} @@ -638,7 +663,7 @@ jobs: esac - name: Provision contextual-orchestrator review sidecar - if: env.PR_NUMBER != '' + if: env.PR_NUMBER != '' && steps.noema_model_admission.outputs.admitted == 'true' env: BYTEZ_API_KEY: ${{ secrets.BYTEZ_API_KEY }} NVIDIA_NIM_API_KEY: ${{ secrets.NVIDIA_NIM_API_KEY }} @@ -651,7 +676,7 @@ jobs: bash "$GITHUB_WORKSPACE/scripts/ci/contextual_orchestrator_review_sidecar.sh" - name: Prepare Noema model verdict - if: env.PR_NUMBER != '' + if: env.PR_NUMBER != '' && steps.noema_model_admission.outputs.admitted == 'true' id: noema_prepare env: GH_TOKEN: ${{ secrets.NOEMA_REVIEW_TOKEN || steps.noema_github_app_token.outputs.token || steps.noema_oidc_token.outputs.token }} diff --git a/CHANGELOG.md b/CHANGELOG.md index bf192f6a9e..be36826cf2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,9 @@ - Raised `hourly-review-repair.yml`'s discovery ceiling from 50 to 200 while rotating deterministic 50-PR deep-inspection windows by hourly run number. The scheduler hydrates only the selected window and stops immediately after its single dispatch, preserving access to newer PRs without quadrupling expensive review/check/comment work. See `docs/doctoring/hourly-review-repair-single-file-consolidation.md`'s 2026-09-03 follow-up. ## [Unreleased] +- Reuse Noema's exact-head, base, reviewer-identity, Draft, and existing-review + eligibility checks before provisioning its model sidecar, while retaining + the same checks immediately before model work. Refs #1992. - Include merge-scheduler entrypoint, core, and regression-test changes in the existing runtime-quality workflow's trigger and suite selector. Scheduler workflow edits retain queue checks and also select the full review-repair diff --git a/docs/doctoring/noema-orchestrator-free-zdr.md b/docs/doctoring/noema-orchestrator-free-zdr.md index ec1add661a..fee9f4ae8c 100644 --- a/docs/doctoring/noema-orchestrator-free-zdr.md +++ b/docs/doctoring/noema-orchestrator-free-zdr.md @@ -82,3 +82,21 @@ is not Noema review evidence. For GitHub App credentials, reviewer identity is bound to the pinned token mint action's app slug and numeric installation ID. PAT and OIDC credentials continue to resolve their actor through GitHub's authenticated API. + +## Draft 조기 판정 + +`.github` run `34045630637`의 Noema job은 sidecar 준비를 시작한 뒤 +706.63초가 지나서야 Draft 상태를 확인하고 모델 작업을 건너뛰었다. 이 +실행에서 불필요한 sidecar 시작은 1회였다. Issue #1992는 이 값을 관측 +baseline으로 추적한다. + +모델 작업 admission은 reviewer credential을 만든 뒤, repository visibility +조회와 sidecar 준비보다 먼저 실행한다. 판정은 `two_phase.py`의 기존 순서인 +exact head, base, 독립 reviewer actor, Draft, 현재 head의 기존 Noema review를 +그대로 공유한다. 실제 verdict 준비도 같은 판정을 다시 실행하므로 admission +뒤 상태 변경을 신뢰하지 않는다. Noema는 계속 OpenCode 승인과 독립적으로 +실행된다. + +로컬 회귀는 Draft와 기존 review에서 sidecar admission marker가 생기지 않는 +것을 확인한다. 새 exact-head hosted 실행에서 불필요한 sidecar 시작이 0회인지 +확인하기 전에는 runtime 개선이 완료됐다고 보지 않는다. Refs #1992. diff --git a/tests/test_noema_orchestrator_workflow_contract.py b/tests/test_noema_orchestrator_workflow_contract.py index 628fa3cbc1..a6c7d3a53b 100644 --- a/tests/test_noema_orchestrator_workflow_contract.py +++ b/tests/test_noema_orchestrator_workflow_contract.py @@ -13,6 +13,14 @@ from tests.test_required_workflow_queue_contract import workflow_step, workflow_text +def workflow_step_condition(workflow: str, name: str) -> str: + """Return the executable one-line condition from one named workflow step.""" + step = workflow_step(workflow, name) + match = re.search(r"(?m)^ if:\s*(.+?)\s*$", step) + assert match is not None, f"workflow step {name!r} has no one-line condition" + return match.group(1) + + def test_noema_close_cleanup_selects_only_the_closed_pr_across_shared_display_titles( tmp_path: Path, ) -> None: @@ -196,7 +204,22 @@ def test_noema_review_credentials_and_llm_use_orchestrator_free() -> None: assert "OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}" in workflow assert 'export NOEMA_LLM_MODEL="orchestrator/free"' in workflow prepare = workflow_step(workflow, "Prepare Noema model verdict") + admission = workflow_step(workflow, "Admit Noema model work") publish = workflow_step(workflow, "Publish prepared Noema verdict on the exact live head") + assert '--admit-model-file "$admission_file"' in admission + assert "id: noema_model_admission" in admission + admission_condition = ( + "env.PR_NUMBER != '' && steps.noema_model_admission.outputs.admitted == 'true'" + ) + assert workflow_step_condition( + workflow, "Resolve Noema target repository visibility" + ) == admission_condition + assert workflow_step_condition( + workflow, "Provision contextual-orchestrator review sidecar" + ) == admission_condition + assert workflow_step_condition( + workflow, "Prepare Noema model verdict" + ) == admission_condition assert '.github/actions/noema-review/two_phase.py' in prepare assert '--prepare-verdict-file "$verdict_file"' in prepare assert '.github/actions/noema-review/two_phase.py' in publish @@ -211,6 +234,11 @@ def test_noema_review_credentials_and_llm_use_orchestrator_free() -> None: assert "Noema app token is unavailable; review skipped." not in workflow assert "COPILOT_GITHUB_TOKEN" not in workflow assert "secrets: inherit" not in workflow + validate_index = workflow.index(" - name: Validate current pull request head\n") + admission_index = workflow.index(" - name: Admit Noema model work\n") + visibility_index = workflow.index(" - name: Resolve Noema target repository visibility\n") + sidecar_index = workflow.index(" - name: Provision contextual-orchestrator review sidecar\n") + assert validate_index < admission_index < visibility_index < sidecar_index def _expected_head_from_workflow_run_event(event: dict) -> str: diff --git a/tests/test_noema_two_phase_handoff.py b/tests/test_noema_two_phase_handoff.py index 992522be7b..f44bde2dc2 100644 --- a/tests/test_noema_two_phase_handoff.py +++ b/tests/test_noema_two_phase_handoff.py @@ -166,6 +166,120 @@ def test_prepare_skip_creates_no_publishable_envelope(tmp_path: Path, monkeypatc assert not envelope.exists() +@pytest.mark.parametrize("skip_kind", ["closed_or_stale", "draft", "existing_review"]) +def test_model_admission_skips_ineligible_review_before_sidecar( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + skip_kind: str, +) -> None: + """The shared prepare predicate must decline model work without fabricating admission.""" + module = _load_module() + _patch_live_gate(monkeypatch, module) + if skip_kind == "closed_or_stale": + monkeypatch.setattr( + module.gate, + "require_expected_head", + lambda _pr, _head: (_ for _ in ()).throw(RuntimeError("closed or stale")), + ) + elif skip_kind == "draft": + monkeypatch.setattr( + module.gate, + "fetch_pr", + lambda _repo, _number: { + "isDraft": True, + "headRefOid": HEAD, + "baseRefOid": BASE, + }, + ) + else: + monkeypatch.setattr(module.gate, "existing_noema_review", lambda _pr, _actor: True) + marker = tmp_path / "model-admission.json" + + assert module.admit_model_work("ContextualWisdomLab/example", 7, HEAD, marker) == 0 + assert not marker.exists() + + +def test_model_admission_reuses_prepare_identity_checks( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Eligible model work is admitted only after the current prepare identity checks.""" + module = _load_module() + _patch_live_gate(monkeypatch, module) + marker = tmp_path / "model-admission.json" + + assert module.admit_model_work("ContextualWisdomLab/example", 7, HEAD, marker) == 0 + assert module._read_envelope(marker) == { + "expected_base": BASE, + "expected_head": HEAD, + "pull_request_number": 7, + "repository": "ContextualWisdomLab/example", + "schema_version": module.ENVELOPE_SCHEMA_VERSION, + } + + +@pytest.mark.parametrize("invalid_identity", ["head", "base", "actor"]) +def test_model_admission_fails_closed_before_skipping_draft( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + invalid_identity: str, +) -> None: + """Draft status cannot bypass the prepare path's head, base, or actor checks.""" + module = _load_module() + monkeypatch.setattr( + module.gate, + "fetch_pr", + lambda _repo, _number: { + "isDraft": True, + "headRefOid": HEAD, + "baseRefOid": "short" if invalid_identity == "base" else BASE, + }, + ) + monkeypatch.setattr(module.gate, "PRIMARY_REVIEW_AUTHORS", frozenset({"seonghobae"})) + monkeypatch.setattr( + module.gate, + "current_actor", + lambda: "" if invalid_identity == "actor" else "cwl-noema-review[bot]", + ) + monkeypatch.setattr(module.gate, "require_expected_head", lambda _pr, _head: None) + marker = tmp_path / "model-admission.json" + + expected_head = "short" if invalid_identity == "head" else HEAD + with pytest.raises(RuntimeError): + module.admit_model_work("ContextualWisdomLab/example", 7, expected_head, marker) + assert not marker.exists() + + +def test_prepare_rechecks_eligibility_after_admission( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A PR becoming Draft after admission still blocks the model call.""" + module = _load_module() + states = iter((False, True)) + monkeypatch.setattr( + module.gate, + "fetch_pr", + lambda _repo, _number: { + "isDraft": next(states), + "headRefOid": HEAD, + "baseRefOid": BASE, + }, + ) + monkeypatch.setattr(module.gate, "require_expected_head", lambda _pr, _head: None) + monkeypatch.setattr(module.gate, "current_actor", lambda: "cwl-noema-review[bot]") + monkeypatch.setattr(module.gate, "PRIMARY_REVIEW_AUTHORS", frozenset({"seonghobae"})) + monkeypatch.setattr(module.gate, "existing_noema_review", lambda _pr, _actor: False) + monkeypatch.setattr(module.gate, "call_llm", lambda *_args: pytest.fail("Draft must not call the model")) + marker = tmp_path / "model-admission.json" + envelope = tmp_path / "verdict.json" + + assert module.admit_model_work("ContextualWisdomLab/example", 7, HEAD, marker) == 0 + assert marker.exists() + assert module.prepare_verdict("ContextualWisdomLab/example", 7, HEAD, envelope) == 0 + assert not envelope.exists() + + def test_publish_cleans_untrusted_envelope_even_when_read_validation_fails(tmp_path: Path) -> None: """Malformed handoff state cannot linger after a failed publication attempt.""" module = _load_module() From f2325e634531dcd6015e5d7dba168d8815fad42d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 08:36:57 +0900 Subject: [PATCH 2/8] fix(noema): consolidate strict head admission Remove the duplicate REST head-validation step and make the shared two-phase eligibility check explicit: admission fails closed for closed or stale targets while verdict preparation retains its established successful skip. Refs #1992 Co-authored-by: Codex Signed-off-by: Seongho Bae --- .github/actions/noema-review/two_phase.py | 18 ++++- .github/workflows/noema-review.yml | 19 ----- docs/doctoring/noema-orchestrator-free-zdr.md | 6 ++ ...st_noema_orchestrator_workflow_contract.py | 12 ++- tests/test_noema_two_phase_handoff.py | 77 ++++++++++++++++--- 5 files changed, 100 insertions(+), 32 deletions(-) diff --git a/.github/actions/noema-review/two_phase.py b/.github/actions/noema-review/two_phase.py index b4030621fe..3381f67a79 100755 --- a/.github/actions/noema-review/two_phase.py +++ b/.github/actions/noema-review/two_phase.py @@ -149,6 +149,8 @@ def _model_work_eligibility( repo: str, number: int, expected_head: str, + *, + skip_closed_or_stale: bool, ) -> tuple[str, dict[str, Any], str, str] | None: """Return the validated review identity when this head still needs model work.""" expected = _canonical_head(expected_head) @@ -156,6 +158,8 @@ def _model_work_eligibility( try: gate.require_expected_head(pull_request, expected) except RuntimeError: + if not skip_closed_or_stale: + raise print("Pull request is closed or stale; Noema verdict preparation skipped.") return None expected_base = _canonical_base(pull_request) @@ -171,7 +175,12 @@ def _model_work_eligibility( def admit_model_work(repo: str, number: int, expected_head: str, path: Path) -> int: """Record whether the current review needs the expensive model sidecar.""" - eligibility = _model_work_eligibility(repo, number, expected_head) + eligibility = _model_work_eligibility( + repo, + number, + expected_head, + skip_closed_or_stale=False, + ) if eligibility is None: return 0 expected, _pull_request, expected_base, _actor = eligibility @@ -190,7 +199,12 @@ def admit_model_work(repo: str, number: int, expected_head: str, path: Path) -> def prepare_verdict(repo: str, number: int, expected_head: str, path: Path) -> int: """Run model review and seal its verdict without publishing GitHub evidence.""" - eligibility = _model_work_eligibility(repo, number, expected_head) + eligibility = _model_work_eligibility( + repo, + number, + expected_head, + skip_closed_or_stale=True, + ) if eligibility is None: return 0 expected, pull_request, expected_base, _actor = eligibility diff --git a/.github/workflows/noema-review.yml b/.github/workflows/noema-review.yml index 54544d9636..ac315ff4d3 100644 --- a/.github/workflows/noema-review.yml +++ b/.github/workflows/noema-review.yml @@ -580,25 +580,6 @@ jobs: echo "::add-mask::$app_token" echo "token=$app_token" >>"$GITHUB_OUTPUT" - - name: Validate current pull request head - if: env.PR_NUMBER != '' - env: - GH_TOKEN: ${{ secrets.NOEMA_REVIEW_TOKEN || steps.noema_github_app_token.outputs.token || steps.noema_oidc_token.outputs.token }} - run: | - set -euo pipefail - if ! [[ "$EXPECTED_HEAD_SHA" =~ ^[0-9a-fA-F]{40}$ ]]; then - echo "::error::Noema expected head must be a full commit SHA." - exit 1 - fi - pull_request_json="$(gh api "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}")" - live_state="$(jq -r '.state // empty' <<<"$pull_request_json")" - live_head_sha="$(jq -r '.head.sha // empty' <<<"$pull_request_json")" - if [ "$live_state" != "open" ] || [ "${live_head_sha,,}" != "${EXPECTED_HEAD_SHA,,}" ]; then - printf '::error::Noema review target is closed or stale. expected head=%s; live state=%s head=%s.\n' \ - "$EXPECTED_HEAD_SHA" "${live_state:-missing}" "${live_head_sha:-missing}" - exit 1 - fi - - name: Admit Noema model work if: env.PR_NUMBER != '' id: noema_model_admission diff --git a/docs/doctoring/noema-orchestrator-free-zdr.md b/docs/doctoring/noema-orchestrator-free-zdr.md index fee9f4ae8c..05999ac27f 100644 --- a/docs/doctoring/noema-orchestrator-free-zdr.md +++ b/docs/doctoring/noema-orchestrator-free-zdr.md @@ -97,6 +97,12 @@ exact head, base, 독립 reviewer actor, Draft, 현재 head의 기존 Noema revi 뒤 상태 변경을 신뢰하지 않는다. Noema는 계속 OpenCode 승인과 독립적으로 실행된다. +Admission은 closed/stale target을 실패 처리하고 실제 verdict 준비는 기존처럼 +성공적으로 건너뛴다. 공유 판정의 명시적 keyword-only flag가 두 의미를 +분리한다. 이 GraphQL 판정이 같은 reviewer token으로 head/state까지 확인하므로 +바로 앞의 중복 REST 검증 step은 제거했다. 실행마다 REST 호출 1회가 줄지만, +eligible 경로의 추가 GraphQL 조회가 REST와 비용이 같다고 간주하지 않는다. + 로컬 회귀는 Draft와 기존 review에서 sidecar admission marker가 생기지 않는 것을 확인한다. 새 exact-head hosted 실행에서 불필요한 sidecar 시작이 0회인지 확인하기 전에는 runtime 개선이 완료됐다고 보지 않는다. Refs #1992. diff --git a/tests/test_noema_orchestrator_workflow_contract.py b/tests/test_noema_orchestrator_workflow_contract.py index a6c7d3a53b..60acbaf078 100644 --- a/tests/test_noema_orchestrator_workflow_contract.py +++ b/tests/test_noema_orchestrator_workflow_contract.py @@ -208,6 +208,11 @@ def test_noema_review_credentials_and_llm_use_orchestrator_free() -> None: publish = workflow_step(workflow, "Publish prepared Noema verdict on the exact live head") assert '--admit-model-file "$admission_file"' in admission assert "id: noema_model_admission" in admission + assert " - name: Validate current pull request head\n" not in workflow + assert ( + "GH_TOKEN: ${{ secrets.NOEMA_REVIEW_TOKEN || steps.noema_github_app_token.outputs.token || steps.noema_oidc_token.outputs.token }}" + in admission + ) admission_condition = ( "env.PR_NUMBER != '' && steps.noema_model_admission.outputs.admitted == 'true'" ) @@ -234,11 +239,14 @@ def test_noema_review_credentials_and_llm_use_orchestrator_free() -> None: assert "Noema app token is unavailable; review skipped." not in workflow assert "COPILOT_GITHUB_TOKEN" not in workflow assert "secrets: inherit" not in workflow - validate_index = workflow.index(" - name: Validate current pull request head\n") + credential_index = workflow.index(" - name: Select fail-closed Noema reviewer credential\n") + app_token_index = workflow.index(" - name: Mint repository-scoped Noema GitHub App token\n") + oidc_token_index = workflow.index(" - name: Exchange Noema app token through OIDC\n") admission_index = workflow.index(" - name: Admit Noema model work\n") visibility_index = workflow.index(" - name: Resolve Noema target repository visibility\n") sidecar_index = workflow.index(" - name: Provision contextual-orchestrator review sidecar\n") - assert validate_index < admission_index < visibility_index < sidecar_index + assert credential_index < app_token_index < oidc_token_index < admission_index + assert admission_index < visibility_index < sidecar_index def _expected_head_from_workflow_run_event(event: dict) -> str: diff --git a/tests/test_noema_two_phase_handoff.py b/tests/test_noema_two_phase_handoff.py index f44bde2dc2..18c3f68879 100644 --- a/tests/test_noema_two_phase_handoff.py +++ b/tests/test_noema_two_phase_handoff.py @@ -151,6 +151,7 @@ def test_prepare_skip_creates_no_publishable_envelope(tmp_path: Path, monkeypatc "fetch_pr", lambda _repo, _number: { "isDraft": True, + "state": "OPEN", "headRefOid": HEAD, "baseRefOid": BASE, }, @@ -166,7 +167,7 @@ def test_prepare_skip_creates_no_publishable_envelope(tmp_path: Path, monkeypatc assert not envelope.exists() -@pytest.mark.parametrize("skip_kind", ["closed_or_stale", "draft", "existing_review"]) +@pytest.mark.parametrize("skip_kind", ["draft", "existing_review"]) def test_model_admission_skips_ineligible_review_before_sidecar( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, @@ -175,13 +176,7 @@ def test_model_admission_skips_ineligible_review_before_sidecar( """The shared prepare predicate must decline model work without fabricating admission.""" module = _load_module() _patch_live_gate(monkeypatch, module) - if skip_kind == "closed_or_stale": - monkeypatch.setattr( - module.gate, - "require_expected_head", - lambda _pr, _head: (_ for _ in ()).throw(RuntimeError("closed or stale")), - ) - elif skip_kind == "draft": + if skip_kind == "draft": monkeypatch.setattr( module.gate, "fetch_pr", @@ -199,6 +194,70 @@ def test_model_admission_skips_ineligible_review_before_sidecar( assert not marker.exists() +@pytest.mark.parametrize( + "pull_request", + [ + {"isDraft": False, "state": "CLOSED", "headRefOid": HEAD, "baseRefOid": BASE}, + {"isDraft": False, "state": "OPEN", "headRefOid": "c" * 40, "baseRefOid": BASE}, + ], +) +def test_model_admission_fails_closed_for_inactive_target( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + pull_request: dict[str, object], +) -> None: + """Admission uses the real exact-head validator and rejects closed or stale work.""" + module = _load_module() + monkeypatch.setattr(module.gate, "fetch_pr", lambda _repo, _number: pull_request) + marker = tmp_path / "model-admission.json" + + with pytest.raises(RuntimeError, match="closed or its head changed"): + module.admit_model_work("ContextualWisdomLab/example", 7, HEAD, marker) + assert not marker.exists() + + +@pytest.mark.parametrize( + "pull_request", + [ + {"isDraft": False, "state": "CLOSED", "headRefOid": HEAD, "baseRefOid": BASE}, + {"isDraft": False, "state": "OPEN", "headRefOid": "c" * 40, "baseRefOid": BASE}, + ], +) +def test_prepare_keeps_closed_or_stale_as_a_successful_skip( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + pull_request: dict[str, object], +) -> None: + """The model phase retains its established successful stale-target retirement.""" + module = _load_module() + monkeypatch.setattr(module.gate, "fetch_pr", lambda _repo, _number: pull_request) + envelope = tmp_path / "verdict.json" + + assert module.prepare_verdict("ContextualWisdomLab/example", 7, HEAD, envelope) == 0 + assert not envelope.exists() + + +def test_model_admission_propagates_pull_request_api_failure( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """GitHub lookup failures cannot become successful admission skips.""" + module = _load_module() + monkeypatch.setattr( + module.gate, + "fetch_pr", + lambda _repo, _number: (_ for _ in ()).throw(RuntimeError("GitHub unavailable")), + ) + + with pytest.raises(RuntimeError, match="GitHub unavailable"): + module.admit_model_work( + "ContextualWisdomLab/example", + 7, + HEAD, + tmp_path / "model-admission.json", + ) + + def test_model_admission_reuses_prepare_identity_checks( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, @@ -231,6 +290,7 @@ def test_model_admission_fails_closed_before_skipping_draft( "fetch_pr", lambda _repo, _number: { "isDraft": True, + "state": "OPEN", "headRefOid": HEAD, "baseRefOid": "short" if invalid_identity == "base" else BASE, }, @@ -241,7 +301,6 @@ def test_model_admission_fails_closed_before_skipping_draft( "current_actor", lambda: "" if invalid_identity == "actor" else "cwl-noema-review[bot]", ) - monkeypatch.setattr(module.gate, "require_expected_head", lambda _pr, _head: None) marker = tmp_path / "model-admission.json" expected_head = "short" if invalid_identity == "head" else HEAD From e6d013193d538d91643d9124085836d293562413 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 11:29:54 +0900 Subject: [PATCH 3/8] fix(noema): reuse verified live repository visibility --- .github/actions/noema-review/two_phase.py | 11 ++++- .github/workflows/noema-review.yml | 22 ++------- scripts/ci/noema_review_gate.py | 1 + ...st_noema_orchestrator_workflow_contract.py | 30 ++++++++++--- tests/test_noema_two_phase_handoff.py | 45 +++++++++++++++++++ 5 files changed, 83 insertions(+), 26 deletions(-) diff --git a/.github/actions/noema-review/two_phase.py b/.github/actions/noema-review/two_phase.py index 3381f67a79..dc5af93d12 100755 --- a/.github/actions/noema-review/two_phase.py +++ b/.github/actions/noema-review/two_phase.py @@ -183,7 +183,15 @@ def admit_model_work(repo: str, number: int, expected_head: str, path: Path) -> ) if eligibility is None: return 0 - expected, _pull_request, expected_base, _actor = eligibility + expected, pull_request, expected_base, _actor = eligibility + repository = pull_request.get("repository") + if ( + not isinstance(repository, dict) + or not isinstance(repository.get("nameWithOwner"), str) + or repository["nameWithOwner"].casefold() != repo.casefold() + or repository.get("visibility") not in ("PUBLIC", "PRIVATE", "INTERNAL") + ): + raise RuntimeError("Noema repository visibility could not be verified") _write_envelope( path, { @@ -192,6 +200,7 @@ def admit_model_work(repo: str, number: int, expected_head: str, path: Path) -> "pull_request_number": number, "expected_head": expected, "expected_base": expected_base, + "repository_visibility": repository["visibility"].lower(), }, ) return 0 diff --git a/.github/workflows/noema-review.yml b/.github/workflows/noema-review.yml index ac315ff4d3..de420357da 100644 --- a/.github/workflows/noema-review.yml +++ b/.github/workflows/noema-review.yml @@ -598,6 +598,8 @@ jobs: --expected-head "$EXPECTED_HEAD_SHA" \ --admit-model-file "$admission_file" if [ -f "$admission_file" ]; then + visibility="$(jq -er '.repository_visibility | select(. == "public" or . == "private" or . == "internal")' "$admission_file")" + echo "repository_visibility=$visibility" >>"$GITHUB_OUTPUT" rm -f "$admission_file" echo "admitted=true" >>"$GITHUB_OUTPUT" else @@ -609,26 +611,10 @@ jobs: if: env.PR_NUMBER != '' && steps.noema_model_admission.outputs.admitted == 'true' id: target_visibility env: - GH_TOKEN: ${{ secrets.NOEMA_REVIEW_TOKEN || steps.noema_github_app_token.outputs.token || steps.noema_oidc_token.outputs.token }} + REPOSITORY_VISIBILITY: ${{ steps.noema_model_admission.outputs.repository_visibility }} run: | set -euo pipefail - if [ -z "${GH_TOKEN:-}" ]; then - echo "::error::Noema target visibility cannot be resolved without the selected repository-scoped reviewer token." - exit 1 - fi - visibility="" - for target_visibility_attempt in 1 2 3 4 5 6; do - if visibility="$( - gh api "/repos/${TARGET_REPOSITORY}" --jq '.visibility // (if .private then "private" else "public" end)' - )"; then - break - fi - visibility="" - if [ "$target_visibility_attempt" -lt 6 ]; then - echo "Repository visibility lookup failed (attempt ${target_visibility_attempt}/6), possibly a transient GitHub API rate limit; retrying after backoff." >&2 - sleep "$(( target_visibility_attempt * 5 ))" - fi - done + visibility="${REPOSITORY_VISIBILITY:-}" case "$visibility" in private|internal) echo "require_zdr=true" >>"$GITHUB_OUTPUT" diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 5ab7e830f3..2c2c0a7056 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -299,6 +299,7 @@ def graphql(query: str, **fields: str | int) -> dict[str, Any]: query($owner: String!, $name: String!, $number: Int!) { repository(owner: $owner, name: $name) { pullRequest(number: $number) { + repository { nameWithOwner visibility } number title body diff --git a/tests/test_noema_orchestrator_workflow_contract.py b/tests/test_noema_orchestrator_workflow_contract.py index 60acbaf078..1c3279931e 100644 --- a/tests/test_noema_orchestrator_workflow_contract.py +++ b/tests/test_noema_orchestrator_workflow_contract.py @@ -383,18 +383,34 @@ def test_stale_trigger_step_still_rejects_a_genuinely_different_head( assert "Noema trigger is stale" in result.stdout -def test_noema_visibility_lookup_retries_transient_api_failures() -> None: - """Bound transient GitHub API failures without weakening visibility validation.""" +def test_noema_visibility_reuses_admission_without_lookup(tmp_path: Path) -> None: + """Live admission controls ZDR without duplicate API calls or retry sleeps.""" workflow = workflow_text("noema-review.yml") start = workflow.index(" - name: Resolve Noema target repository visibility") end = workflow.index(" - name: Provision contextual-orchestrator review sidecar", start) visibility_step = workflow[start:end] - assert "for target_visibility_attempt in 1 2 3 4 5 6; do" in visibility_step - assert 'if visibility="$(' in visibility_step - assert 'sleep "$(( target_visibility_attempt * 5 ))"' in visibility_step - assert "possibly a transient GitHub API rate limit; retrying after backoff." in visibility_step - assert "case \"$visibility\" in" in visibility_step + assert "steps.noema_model_admission.outputs.repository_visibility" in visibility_step + assert "gh api" not in visibility_step + assert "sleep " not in visibility_step + admission = workflow_step(workflow, "Admit Noema model work") + assert ".repository_visibility | select(" in admission + assert 'echo "repository_visibility=$visibility"' in admission + script = textwrap.dedent(visibility_step.split(" run: |\n", 1)[1]) + for index, visibility in enumerate(("public", "private", "internal", "", "unknown", "PUBLIC")): + output = tmp_path / f"output-{index}" + result = subprocess.run( + [shutil.which("bash") or "/bin/bash", "-c", script], + capture_output=True, text=True, check=False, + env={**os.environ, "REPOSITORY_VISIBILITY": visibility, + "EVENT_REPOSITORY_VISIBILITY": "public", "GITHUB_OUTPUT": str(output)}, + ) + if visibility in ("public", "private", "internal"): + assert result.returncode == 0, result.stderr + assert output.read_text() == f"require_zdr={'false' if visibility == 'public' else 'true'}\n" + else: + assert result.returncode != 0 + assert not output.exists() def test_strix_gateway_default_and_noema_sidecar_fail_closed(tmp_path: Path) -> None: diff --git a/tests/test_noema_two_phase_handoff.py b/tests/test_noema_two_phase_handoff.py index 18c3f68879..45250d86b0 100644 --- a/tests/test_noema_two_phase_handoff.py +++ b/tests/test_noema_two_phase_handoff.py @@ -32,6 +32,7 @@ def _patch_live_gate(monkeypatch: pytest.MonkeyPatch, module: ModuleType) -> Non "isDraft": False, "headRefOid": HEAD, "baseRefOid": BASE, + "repository": {"nameWithOwner": "ContextualWisdomLab/example", "visibility": "PUBLIC"}, }, ) monkeypatch.setattr(module.gate, "require_expected_head", lambda _pr, _head: None) @@ -274,9 +275,52 @@ def test_model_admission_reuses_prepare_identity_checks( "pull_request_number": 7, "repository": "ContextualWisdomLab/example", "schema_version": module.ENVELOPE_SCHEMA_VERSION, + "repository_visibility": "public", } +@pytest.mark.parametrize("visibility", ["PUBLIC", "PRIVATE", "INTERNAL"]) +def test_admission_reuses_live_repository_visibility(tmp_path, monkeypatch, visibility): + """One existing PR query binds privacy; queued event metadata is not authority.""" + module = _load_module() + monkeypatch.setattr(module.gate, "current_actor", lambda: "cwl-noema-review[bot]") + monkeypatch.setattr(module.gate, "existing_noema_review", lambda *_: False) + monkeypatch.setenv("EVENT_REPOSITORY_VISIBILITY", "PUBLIC") + calls = [] + + def graphql(query, **kwargs): + calls.append(kwargs) + assert "repository { nameWithOwner visibility }" in query + return {"data": {"repository": {"pullRequest": { + "state": "OPEN", "isDraft": False, "headRefOid": HEAD, "baseRefOid": BASE, + "repository": {"nameWithOwner": "contextualwisdomlab/EXAMPLE", "visibility": visibility}, + }}}} + + monkeypatch.setattr(module.gate, "graphql", graphql) + marker = tmp_path / "admission.json" + module.admit_model_work("ContextualWisdomLab/example", 7, HEAD, marker) + assert len(calls) == 1 + assert module._read_envelope(marker)["repository_visibility"] == visibility.lower() + + +@pytest.mark.parametrize("repository", [None, {}, "public", + {"nameWithOwner": "ContextualWisdomLab/other", "visibility": "PUBLIC"}, + {"nameWithOwner": "ContextualWisdomLab/example", "visibility": "UNKNOWN"}, + {"nameWithOwner": "ContextualWisdomLab/example", "visibility": None}, +]) +def test_admission_rejects_unverified_repository_visibility(tmp_path, monkeypatch, repository): + """Unknown privacy or a different repository cannot admit a model request.""" + module = _load_module() + _patch_live_gate(monkeypatch, module) + monkeypatch.setattr(module.gate, "fetch_pr", lambda *_: { + "isDraft": False, "headRefOid": HEAD, "baseRefOid": BASE, "repository": repository, + }) + marker = tmp_path / "admission.json" + with pytest.raises(RuntimeError, match="repository visibility"): + module.admit_model_work("ContextualWisdomLab/example", 7, HEAD, marker) + assert not marker.exists() + + @pytest.mark.parametrize("invalid_identity", ["head", "base", "actor"]) def test_model_admission_fails_closed_before_skipping_draft( tmp_path: Path, @@ -323,6 +367,7 @@ def test_prepare_rechecks_eligibility_after_admission( "isDraft": next(states), "headRefOid": HEAD, "baseRefOid": BASE, + "repository": {"nameWithOwner": "ContextualWisdomLab/example", "visibility": "PUBLIC"}, }, ) monkeypatch.setattr(module.gate, "require_expected_head", lambda _pr, _head: None) From 9f08ed08594f17d66f802c556c0573f6d3fae21a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 11:37:33 +0900 Subject: [PATCH 4/8] test(noema): follow shared exact-head admission contract Co-Authored-By: Codex Signed-off-by: Seongho Bae --- tests/test_required_workflow_queue_contract.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 4ab09b1d0c..84afa190fa 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -1078,7 +1078,9 @@ def test_noema_triggers_preserve_standalone_pull_request_review() -> None: assert re.search(r"(?m)^concurrency:", workflow) assert not re.search(r"(?m)^ concurrency:", workflow) assert "needs.admit-current-head.outputs.admitted == 'true'" in noema_job - assert '[ "${live_head_sha,,}" != "${EXPECTED_HEAD_SHA,,}" ]' in workflow + admission = workflow_step(workflow, "Admit Noema model work") + assert '--expected-head "$EXPECTED_HEAD_SHA"' in admission + assert '--admit-model-file "$admission_file"' in admission def test_noema_review_credentials_and_orchestrator_configuration_fail_closed() -> None: From 3170932b726dc0e7be5ba83a12d9a533ac40d543 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 14:13:45 +0900 Subject: [PATCH 5/8] =?UTF-8?q?fix(ci):=20Noema=EC=99=80=20=ED=81=90=20?= =?UTF-8?q?=EA=B3=84=EC=95=BD=EC=9D=98=20=EC=84=A0=ED=83=9D=20=EB=B0=8F=20?= =?UTF-8?q?=EC=8B=A4=ED=96=89=20=EB=88=84=EB=9D=BD=20=EB=B3=B4=EC=99=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Seongho Bae --- .../agent-review-runtime-quality-ci.yml | 14 ++++++- AGENTS.md | 5 +++ ...nt_review_runtime_quality_consolidation.py | 37 +++++++++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/.github/workflows/agent-review-runtime-quality-ci.yml b/.github/workflows/agent-review-runtime-quality-ci.yml index 3680da8778..01b6231142 100644 --- a/.github/workflows/agent-review-runtime-quality-ci.yml +++ b/.github/workflows/agent-review-runtime-quality-ci.yml @@ -7,6 +7,10 @@ on: - ".github/workflows/agent-review-runtime-quality-ci.yml" - ".github/workflows/noema-review.yml" - ".github/actions/noema-review/two_phase.py" + - "scripts/ci/noema_review_gate.py" + - "tests/test_noema_review_gate.py" + - "tests/test_noema_orchestrator_workflow_contract.py" + - "tests/test_required_workflow_queue_contract.py" - "tests/test_noema_reviewer_token_lifetime.py" - "tests/test_noema_two_phase_handoff.py" - "tests/test_noema_refreshed_app_identity.py" @@ -175,6 +179,9 @@ jobs: ;; .github/workflows/noema-review.yml|\ .github/actions/noema-review/two_phase.py|\ + scripts/ci/noema_review_gate.py|\ + tests/test_noema_review_gate.py|\ + tests/test_noema_orchestrator_workflow_contract.py|\ tests/test_noema_reviewer_token_lifetime.py|\ tests/test_noema_two_phase_handoff.py|\ tests/test_noema_refreshed_app_identity.py|\ @@ -210,7 +217,8 @@ jobs: queue_suite=true review_repair_suite=true ;; - scripts/ci/current_head_run_coalescer.py) + scripts/ci/current_head_run_coalescer.py|\ + tests/test_required_workflow_queue_contract.py) queue_suite=true ;; .github/workflows/pr-review-fix-scheduler.yml|\ @@ -328,6 +336,8 @@ jobs: run: | set -euo pipefail PYTHONPATH=. python -m pytest -q \ + tests/test_noema_review_gate.py \ + tests/test_noema_orchestrator_workflow_contract.py \ tests/test_noema_reviewer_token_lifetime.py \ tests/test_noema_two_phase_handoff.py \ tests/test_noema_refreshed_app_identity.py \ @@ -374,7 +384,7 @@ jobs: if: steps.affected_suites.outputs.queue == 'true' run: | set -euo pipefail - python -m pytest -q tests/test_current_head_coalescer_self_cancellation.py + python -m pytest -q tests/test_current_head_coalescer_self_cancellation.py tests/test_required_workflow_queue_contract.py python -m compileall -q tests/test_current_head_coalescer_self_cancellation.py - name: Verify scheduler and contextual-orchestrator review-repair contracts diff --git a/AGENTS.md b/AGENTS.md index e955f8b36a..dba8c49740 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -212,3 +212,8 @@ them alone proves succession. variable in CI, so a failure class exists that cannot reproduce locally. Before calling a scheduler change clean, run the affected tests both ways, including `GITHUB_ACTIONS=true python3 -m pytest `. +- For consolidated CI, verify all three links: changed-path trigger, actual shell suite + selection, and the selected pytest command. A green job does not cover a changed + contract merely because its filename appears elsewhere in the workflow. Reuse the + existing single-runner job and execute its selector in regression tests; record the + hosted test count separately from a local full-suite result. diff --git a/tests/test_agent_review_runtime_quality_consolidation.py b/tests/test_agent_review_runtime_quality_consolidation.py index 4592cfd166..e225dc0c04 100644 --- a/tests/test_agent_review_runtime_quality_consolidation.py +++ b/tests/test_agent_review_runtime_quality_consolidation.py @@ -207,6 +207,43 @@ def test_commercial_readiness_suite_is_selected_and_conditionally_executed() -> assert "--fail-under=100" in workflow +@pytest.mark.parametrize( + ("changed_path", "suite", "test_path"), + ( + ("scripts/ci/noema_review_gate.py", "noema", "tests/test_noema_review_gate.py"), + ("tests/test_noema_review_gate.py", "noema", "tests/test_noema_review_gate.py"), + ("tests/test_noema_orchestrator_workflow_contract.py", "noema", "tests/test_noema_orchestrator_workflow_contract.py"), + ("tests/test_required_workflow_queue_contract.py", "queue", "tests/test_required_workflow_queue_contract.py"), + ), +) +def test_admission_changes_select_and_execute_owned_contracts( + changed_path: str, suite: str, test_path: str +) -> None: + """A passing selected job must actually execute the changed gate contracts.""" + workflow = _workflow_text() + trigger = workflow.split("on:\n", 1)[1].split("\nconcurrency:\n", 1)[0] + assert f' - "{changed_path}"' in trigger + selector = workflow.split(' case "$changed_path" in\n', 1)[1].split( + " esac", 1 + )[0] + result = subprocess.run( + ["bash", "-euo", "pipefail", "-c", + 'read -r changed_path\nnoema_suite=false\nqueue_suite=false\n' + 'case "$changed_path" in\n' + selector + + 'esac\nprintf "%s" "$' + suite + '_suite"'], + input=changed_path + "\n", text=True, capture_output=True, check=True, + ) + assert result.stdout == "true" + assert result.stderr == "" + selected_step = workflow.split( + f"if: steps.affected_suites.outputs.{suite} == 'true'\n", 1 + )[1].split("\n - name:", 1)[0] + pytest_command = selected_step.split("python -m pytest -q", 1)[1].split( + "python -m compileall", 1 + )[0] + assert test_path in pytest_command + + def test_exact_artifact_suite_preserves_version_and_quality_contracts() -> None: """Compile on Python 3.10 before running full Python 3.14 evidence.""" From a216b526e2bc8d0279870eac1099cec34176b6f4 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 14:16:44 +0900 Subject: [PATCH 6/8] =?UTF-8?q?fix(ci):=20=ED=81=90=20=EC=9E=90=EC=B2=B4?= =?UTF-8?q?=20=ED=9A=8C=EA=B7=80=EC=9D=98=20=EC=8B=A4=ED=96=89=20=EC=A1=B0?= =?UTF-8?q?=EA=B1=B4=EA=B3=BC=20=EC=84=A0=ED=83=9D=20=EB=B2=94=EC=9C=84=20?= =?UTF-8?q?=EA=B2=80=EC=A6=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Seongho Bae --- .github/workflows/agent-review-runtime-quality-ci.yml | 2 ++ tests/test_agent_review_runtime_quality_consolidation.py | 5 +++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/agent-review-runtime-quality-ci.yml b/.github/workflows/agent-review-runtime-quality-ci.yml index 01b6231142..81276e1bbe 100644 --- a/.github/workflows/agent-review-runtime-quality-ci.yml +++ b/.github/workflows/agent-review-runtime-quality-ci.yml @@ -11,6 +11,7 @@ on: - "tests/test_noema_review_gate.py" - "tests/test_noema_orchestrator_workflow_contract.py" - "tests/test_required_workflow_queue_contract.py" + - "tests/test_current_head_coalescer_self_cancellation.py" - "tests/test_noema_reviewer_token_lifetime.py" - "tests/test_noema_two_phase_handoff.py" - "tests/test_noema_refreshed_app_identity.py" @@ -218,6 +219,7 @@ jobs: review_repair_suite=true ;; scripts/ci/current_head_run_coalescer.py|\ + tests/test_current_head_coalescer_self_cancellation.py|\ tests/test_required_workflow_queue_contract.py) queue_suite=true ;; diff --git a/tests/test_agent_review_runtime_quality_consolidation.py b/tests/test_agent_review_runtime_quality_consolidation.py index e225dc0c04..34c6e79628 100644 --- a/tests/test_agent_review_runtime_quality_consolidation.py +++ b/tests/test_agent_review_runtime_quality_consolidation.py @@ -214,6 +214,7 @@ def test_commercial_readiness_suite_is_selected_and_conditionally_executed() -> ("tests/test_noema_review_gate.py", "noema", "tests/test_noema_review_gate.py"), ("tests/test_noema_orchestrator_workflow_contract.py", "noema", "tests/test_noema_orchestrator_workflow_contract.py"), ("tests/test_required_workflow_queue_contract.py", "queue", "tests/test_required_workflow_queue_contract.py"), + ("tests/test_current_head_coalescer_self_cancellation.py", "queue", "tests/test_current_head_coalescer_self_cancellation.py"), ), ) def test_admission_changes_select_and_execute_owned_contracts( @@ -230,10 +231,10 @@ def test_admission_changes_select_and_execute_owned_contracts( ["bash", "-euo", "pipefail", "-c", 'read -r changed_path\nnoema_suite=false\nqueue_suite=false\n' 'case "$changed_path" in\n' + selector - + 'esac\nprintf "%s" "$' + suite + '_suite"'], + + 'esac\nprintf "%s,%s" "$noema_suite" "$queue_suite"'], input=changed_path + "\n", text=True, capture_output=True, check=True, ) - assert result.stdout == "true" + assert result.stdout == ("true,false" if suite == "noema" else "false,true") assert result.stderr == "" selected_step = workflow.split( f"if: steps.affected_suites.outputs.{suite} == 'true'\n", 1 From 32bb4f62f46c7cb57e56fdff6db702d2a12eb4aa Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 14:20:37 +0900 Subject: [PATCH 7/8] =?UTF-8?q?fix(ci):=20=EB=B3=80=EA=B2=BD=20=ED=8C=8C?= =?UTF-8?q?=EC=9D=BC=20=EC=A1=B0=ED=9A=8C=20=EC=8B=A4=ED=8C=A8=EB=A5=BC=20?= =?UTF-8?q?=EA=B2=80=EC=82=AC=20=EC=83=9D=EB=9E=B5=EC=9C=BC=EB=A1=9C=20?= =?UTF-8?q?=EC=B2=98=EB=A6=AC=ED=95=98=EC=A7=80=20=EC=95=8A=EA=B8=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Seongho Bae --- .../agent-review-runtime-quality-ci.yml | 3 ++- AGENTS.md | 4 ++++ ...nt_review_runtime_quality_consolidation.py | 22 +++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/.github/workflows/agent-review-runtime-quality-ci.yml b/.github/workflows/agent-review-runtime-quality-ci.yml index 81276e1bbe..6c5168a318 100644 --- a/.github/workflows/agent-review-runtime-quality-ci.yml +++ b/.github/workflows/agent-review-runtime-quality-ci.yml @@ -159,6 +159,7 @@ jobs: commercial_readiness_suite=false exact_artifact_suite=false + changed_paths="$(git diff --name-only "$BASE_SHA...$HEAD_SHA")" while IFS= read -r changed_path; do case "$changed_path" in .github/workflows/agent-review-runtime-quality-ci.yml) @@ -296,7 +297,7 @@ jobs: exact_artifact_suite=true ;; esac - done < <(git diff --name-only "$BASE_SHA...$HEAD_SHA") + done <<<"$changed_paths" { echo "noema=$noema_suite" diff --git a/AGENTS.md b/AGENTS.md index dba8c49740..92f41f7579 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -217,3 +217,7 @@ them alone proves succession. contract merely because its filename appears elsewhere in the workflow. Reuse the existing single-runner job and execute its selector in regression tests; record the hosted test count separately from a local full-suite result. +- Capture changed-file discovery before iterating it. Bash process substitution + can hide a failed `git diff` despite `set -e`; do not publish all-false suite + outputs when the base cannot be read. Execute the real selector with an invalid + base in its regression test and require failure before any selection output. diff --git a/tests/test_agent_review_runtime_quality_consolidation.py b/tests/test_agent_review_runtime_quality_consolidation.py index 34c6e79628..11b60fea44 100644 --- a/tests/test_agent_review_runtime_quality_consolidation.py +++ b/tests/test_agent_review_runtime_quality_consolidation.py @@ -6,8 +6,10 @@ workflow_level_cancels_in_progress, ) +import os import re import subprocess +import textwrap from pathlib import Path import pytest @@ -139,6 +141,26 @@ def test_exact_head_is_verified_before_selected_suites_run() -> None: assert 'git diff --name-only "$BASE_SHA...$HEAD_SHA"' in selector +def test_unreadable_base_fails_before_publishing_suite_selection(tmp_path: Path) -> None: + """A failed git diff must not turn every affected suite into a clean skip.""" + step = _workflow_text().split("- name: Select affected contract suites", 1)[1].split( + " - name: Install exact hash-verified base dependencies", 1 + )[0] + script = textwrap.dedent(step.split(" run: |\n", 1)[1]) + head = subprocess.check_output( + ["git", "rev-parse", "HEAD"], cwd=REPOSITORY_ROOT, text=True + ).strip() + output = tmp_path / "suite_outputs" + result = subprocess.run( + ["bash", "-euo", "pipefail", "-c", script], cwd=REPOSITORY_ROOT, + env={**os.environ, "BASE_SHA": "0" * 40, "HEAD_SHA": head, + "GITHUB_OUTPUT": str(output)}, + text=True, capture_output=True, check=False, + ) + assert result.returncode != 0 + assert not output.exists() + + def test_review_repair_suite_is_selected_and_conditionally_executed() -> None: """Run review-repair contracts only when their owned paths change.""" From 66854e8286af2f469e8cb7c02ca62b08fd4fea35 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 14:35:09 +0900 Subject: [PATCH 8/8] =?UTF-8?q?fix(noema):=20=EC=8B=A4=ED=96=89=20?= =?UTF-8?q?=EB=8B=A8=EA=B3=84=EC=97=90=20=EB=A7=9E=EB=8A=94=20=EC=83=9D?= =?UTF-8?q?=EB=9E=B5=20=EB=A1=9C=EA=B7=B8=20=EA=B8=B0=EB=A1=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Seongho Bae --- .github/actions/noema-review/two_phase.py | 9 ++++++--- tests/test_noema_two_phase_handoff.py | 10 +++++++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/.github/actions/noema-review/two_phase.py b/.github/actions/noema-review/two_phase.py index dc5af93d12..fb13a96268 100755 --- a/.github/actions/noema-review/two_phase.py +++ b/.github/actions/noema-review/two_phase.py @@ -151,6 +151,7 @@ def _model_work_eligibility( expected_head: str, *, skip_closed_or_stale: bool, + phase: str, ) -> tuple[str, dict[str, Any], str, str] | None: """Return the validated review identity when this head still needs model work.""" expected = _canonical_head(expected_head) @@ -160,15 +161,15 @@ def _model_work_eligibility( except RuntimeError: if not skip_closed_or_stale: raise - print("Pull request is closed or stale; Noema verdict preparation skipped.") + print(f"Pull request is closed or stale; Noema {phase} skipped.") return None expected_base = _canonical_base(pull_request) actor = _reviewer_actor() if pull_request.get("isDraft"): - print("PR is draft; Noema verdict preparation skipped.") + print(f"PR is draft; Noema {phase} skipped.") return None if gate.existing_noema_review(pull_request, actor): - print("Current head already has a Noema review; verdict preparation skipped.") + print(f"Current head already has a Noema review; Noema {phase} skipped.") return None return expected, pull_request, expected_base, actor @@ -180,6 +181,7 @@ def admit_model_work(repo: str, number: int, expected_head: str, path: Path) -> number, expected_head, skip_closed_or_stale=False, + phase="model admission", ) if eligibility is None: return 0 @@ -213,6 +215,7 @@ def prepare_verdict(repo: str, number: int, expected_head: str, path: Path) -> i number, expected_head, skip_closed_or_stale=True, + phase="verdict preparation", ) if eligibility is None: return 0 diff --git a/tests/test_noema_two_phase_handoff.py b/tests/test_noema_two_phase_handoff.py index 45250d86b0..4ea995c641 100644 --- a/tests/test_noema_two_phase_handoff.py +++ b/tests/test_noema_two_phase_handoff.py @@ -169,10 +169,17 @@ def test_prepare_skip_creates_no_publishable_envelope(tmp_path: Path, monkeypatc @pytest.mark.parametrize("skip_kind", ["draft", "existing_review"]) +@pytest.mark.parametrize( + ("operation", "phase"), + [("admit_model_work", "model admission"), ("prepare_verdict", "verdict preparation")], +) def test_model_admission_skips_ineligible_review_before_sidecar( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, skip_kind: str, + operation: str, + phase: str, + capsys: pytest.CaptureFixture[str], ) -> None: """The shared prepare predicate must decline model work without fabricating admission.""" module = _load_module() @@ -191,8 +198,9 @@ def test_model_admission_skips_ineligible_review_before_sidecar( monkeypatch.setattr(module.gate, "existing_noema_review", lambda _pr, _actor: True) marker = tmp_path / "model-admission.json" - assert module.admit_model_work("ContextualWisdomLab/example", 7, HEAD, marker) == 0 + assert getattr(module, operation)("ContextualWisdomLab/example", 7, HEAD, marker) == 0 assert not marker.exists() + assert f"{phase} skipped." in capsys.readouterr().out @pytest.mark.parametrize(