fix(review): fail closed before every Noema skip path - #1198
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCodeQL merge preview, 현재 HEAD 리뷰 게이트, PR 스케줄러, coverage 빌드, VCS 라이선스 검증을 갱신했습니다. 관련 계약 테스트와 운영 문서도 변경했습니다. Changes리뷰 제어 무결성
Estimated code review effort: 5 (Critical) | ~120 minutes Merge Risk: 🟠 High · up to This PR changes privileged review scheduling and security validation, but newer actionable pull requests can be starved by the processing cap, and the current head still lacks required exact-head approval and terminal checks while workflow-integrity and license-policy concerns remain unresolved. These risks can delay required reviews or allow insufficiently validated automation changes, so the PR should not merge until they are fixed or explicitly accepted. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 70.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 114 functions across 16 files. (10 skipped: 9 unsupported, 1 too large.) ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Exact-head local verification (observed at head 081e9eb, base 731af58):
Hosted Checks remain queued/pending and no exact-head approval is present; this PR remains on the normal protected path and was not merged or bypassed. |
|
@opencode-agent @cwl-noema-review Current-head review request for central security PR #1198. Exact head: 081e9eb Read the complete one-file hash-locked dependency diff. Verify that pip is upgraded to a fixed release, both hashes match the release artifact, no unrelated dependencies changed, and the lock remains compatible with the repository's install/audit contract. Confirm the PYSEC-2026-3721 pip-audit failure is resolved without weakening the security gate. Return an exact-head substantive verdict with path:line evidence; do not reuse predecessor evidence. |
|
Exact-head security verification at 081e9eb: the one-file lock diff updates pip 26.1.2 to 26.2.1 with two replacement hashes. In a project-local CPython 3.12 uv environment, |
|
Exact-head revalidation after stack-only child merge: current head 7005e5c, base main 731af58. The tree includes the pip 26.2.1 lock repair plus the stack-only merge of #1168. Local verification on this exact tree: 1295 tests and 16 subtests passed; actionlint, Python compileall, bash -n for both changed shell scripts, git diff --check, 28-package --require-hashes installation in CPython 3.12, and pip-audit hard gate all passed; pip-audit reported No known vulnerabilities found. Live PR remains OPEN/MERGEABLE/BLOCKED; exact-head policy is in progress, remaining hosted required lanes are queued, and no independent approval exists. No bypass or force push. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent @cwl-noema-review Exact current HEAD e44bdef. This is the stack root: the pip 26.2.1 security lock repair remains together with the already-merged #1178 contextual-orchestrator hourly review-repair child. The child was preserved through normal inverse/reapply commits; no history rewrite or force push was used. Exact-head verification: the stack-specific caller test passed (4 tests); Python compileall and git diff --check passed. Re-run the full current-head review and security evidence across the complete stack. Do not reuse predecessor-head status or review evidence. |
Revert the unrelated pip-audit lock/input policy commit from the Strix owner branch. The canonical dependency-only security repair remains in PR #1198; this branch continues to own only the Strix fail-closed contract.
Exact-head security verification
The owner-scope repair restored the dependency-only tree without force-push. The diff remains limited to pip Exact-head local evidence:
The repository-wide interrogate result is 99.9% because the unchanged base already has one undocumented Hosted required Checks for |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Exact-head lock regeneration evidence
The lock was not hand-edited: the exact recorded uv generation contract reproduces the current pip block, and the hash-enforced install path accepts it. No unrelated lock refresh is being introduced because this PR intentionally changes only the vulnerable pip pin. Disposition: resolved; no further source change is required for this finding. |
seonghobae
changed the title
|
@opencode-agent @cwl-noema-review Review-only request for exact current HEAD e44bdef. Verify the pip 26.2.1 lock repair and the already-reviewed #1178 hourly caller as one six-file stack-root proposal to main. Confirm hash integrity, explicit credential scope, schedule contract, and exact-head Checks; do not reuse predecessor evidence. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Exact-head verification update
Current six-file diff and local evidence
Hosted state and decision
Decision: |
Exact-head verification update
|
|
Fresh independent reproduction of the Noema fail-open branch on #1270 exact head
This is a second current owner-PR reproduction of the exact false-green #1198 repairs. It is non-passing evidence until the protected-main Noema workflow exits non-zero whenever the primary exact-head verdict is absent, skipped, stale, COMMENTED, or otherwise non-qualifying. No consumer or #1270 workaround is appropriate. |
…hestrator Devin's review on PR #933 found the vision-input exclusion added in 94e3b9a was incomplete on three points: 1. Runtime free pool remained unfiltered: `_auto_discover_runtime_agents` (`--auto-discover-model-agents`) and `provider_bootstrap`'s `_active_agent_from_discovered` (used by `bootstrap_provider_runtime` and, through it, `provider_catalog_bootstrap.bootstrap_provider_catalog_runtime`) both tag an agent `cost:free` from raw price evidence alone and never consulted the new exclusion, so NVIDIA NIM's free `meta/llama-3.2-90b-vision-instruct` could still reach a live, blindly-selectable `cost:free` agent through either path -- reproducing the original Strix tool-calling incident (ContextualWisdomLab/.github#1198). 2. `free_discovered_models()` conflated price-based inventory with serving-pool eligibility, so `--free-only`, `free_tier_count`, and the free-tier data-privacy totals silently undercounted a model that is genuinely free-priced but unfit for blind serving. 3. (Evaluated, not adopted) Devin also suggested narrowing the exclusion to spare a model that "also supports text as a standalone input". Rejected against this repo's own incident evidence: the incident model itself declares both `text` and `image` per Models.dev, so that narrowing would have silently re-admitted the exact model this fix is about. Kept the conservative "any declared non-text input modality disqualifies" reading, documented with a fixture for all three modality shapes (text-only, vision-only, text+image). Fix: - `free_discovered_models()` is pure price-based inventory again. - New `model_discovery.general_free_serving_candidates()` carries the modality-based exclusion for composing a blind free pool, wired into the `discover-models` CLI report as `general_free_serving_count` alongside the restored `free_tier_count`. - The actual enforcement is now `TaskOrchestrator._is_free_agent`, a single choke point every `orchestrator/free` selection path shares: an agent whose tags declare a non-text `input:<modality>` is never treated as free-pool eligible there, regardless of which code built it or how old that agent-pool row is (protects durable pool-store rows written before this exclusion existed, and any future pool-construction path). `cost:free` keeps meaning "honest zero price" everywhere else, preserving `provider_catalog_store.py`'s durable `is_free` round trip (see `test_serving_tags_preserve_only_explicit_free_and_modality_evidence` and `test_last_known_good_restores_free_and_modality_evidence`, both left unchanged). - `review_gateway.py` audited: its agents never carry a `cost:free` tag (tags are fully replaced with `("review",)`) and no price is registered, so it was never reachable through this bug; no change needed there. Also brought `interrogate --fail-under=100` back to green: it was already failing at 97.9% on the unmodified base commit (94e3b9a) from six pre-existing undocumented `fuzz/*.py` harness functions, unrelated to this incident; added their docstrings. TDD: new/relocated tests fail against the pre-fix source (verified by temporarily restoring it) and pass after the fix -- `test_auto_discovery_activates_a_free_vision_model_but_free_pool_excludes_it`, `test_active_agent_from_discovered_free_vision_model_is_not_free_pool_eligible`, `test_free_discovered_models_still_counts_a_free_vision_only_input_model`, `test_general_free_serving_candidates_modality_shapes`, and the relocated `test_general_free_serving_candidates_excludes_a_free_vision_only_input_model`. Verified: python -m pytest tests -q -> 2774 passed, 1 skipped (2770 passed, 1 skipped on the unmodified base commit; net +4 tests, zero regressions). interrogate (fail-under 100) -> 100.0%. python tests/test_conventions.py passes (new function name is valid two-or-more-word snake_case). git diff --check clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
|
Fresh independent exact-head reproduction of the Noema fail-open boundary:
Acceptance remains fail-closed: missing primary current-head approval must make the required Noema result explicitly non-passing. Do not count either successful check as review evidence. Canonical owner head was re-fetched immediately before this comment as |
|
Fresh exact-head operational evidence (2026-08-31 KST) reproduces the remaining Noema fail-open on two independent central heads:
Both heads lacked a qualifying exact-current-head primary OpenCode approval. Provider preflight/readiness does not make the absence of the prerequisite verdict passing evidence. Acceptance remains: this path must terminate non-zero / explicitly non-passing, while a qualifying current-head OpenCode approval may continue into the secondary review. These are central contract reproductions; no consumer workaround is appropriate. |
…1270) Central PR merge scheduler now refuses direct-merge and auto-merge until GitHub reports reviewDecision APPROVED plus a non-author, non-OpenCode formal approval bound to the exact live head; disarms stale auto-merge otherwise. Repairs check-run chronology (check-suite creation time as the ordering signal via a shared check_run_recency_key/_newest_check_run_per_identity helper, so a canceled-before-start or queued rerun cannot lose to an older completed run merely because startedAt is null -- this fix now also covers strix_evidence_state's latest_check_run_attempts path, not just failed_status_checks) and re-validates approval immediately before every merge-authorizing mutation (closes a TOCTOU window where a same-head approval revoked mid-invocation could still authorize merge). Eight+ rounds of reproduce-before/confirm-after TDD fixes across this PR's lifetime, each independently verified by Devin Review and CodeRabbit with all findings resolved; full suite passing with 100% coverage and docstrings on scripts/ci. Merge commit 98aa3a1 resolves a real conflict against main (from #1433) where two PRs independently fixed the same check-run dedup logic differently -- unified on the correct implementation rather than picking one side. Bypass-merged (as a merge commit, not squash, to preserve the resolved-conflict history) per explicit user authorization: exact-head Security Scan, SAST, CodeQL, OSV, SBOM, Scorecard, Secret Scan, and Strix are terminal success; the required OpenCode/Noema formal-verdict dispatch has not completed for reasons independent of this PR's own correctness (a separate, already-diagnosed central review-gate defect tracked on #1198), and this scheduler hardening is foundational for every other PR's merge safety.
|
Consumer acceptance canary for the existing review-gate repair, from read-only |
|
Fresh exact-head Noema false-green reproduction (2026-08-31 UTC) on canonical canary #1437 head
This is the same central contract owned by #1198, not a #1437 source defect or a consumer workaround. The acceptance boundary remains: Draft review dispatch may be review-only, but an intentional skip/absent exact-head primary verdict must conclude non-passing; Noema must never translate |
Restack #1198 onto protected main while preserving current-main scheduler authority. Missing exact-current-head OpenCode approval now returns non-passing before Draft, prior-Noema, or primary-token skip paths.
seonghobae
changed the title
|
@opencode-agent review Please review exact head |
|
Exact-head operational RED is now reproduced on
This is the precise false-green repaired by the two-file PR delta. Hosted Noema GREEN cannot exist until protected main integrates the source; predecessor/default-branch execution is authoritative RED, not passing evidence. Security Scan, SAST, CodeQL, OSV, SBOM, Python Security, Secret Scan, and Scorecard completed successfully on the exact head. Strix was cancelled and remains non-passing. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ee (#933) * fix(discovery): exclude vision-input-only models from orchestrator/free ContextualWisdomLab/.github PR #1198's required Strix Security Scan check failed (run 33325907333, job 99295892400): 3 independent scan attempts each hit NVIDIA NIM's meta/llama-3.2-90b-vision-instruct via orchestrator/free and got an identical HTTP 400 invalid_request_error ("Adjust the request parameters and retry"), exhausting the whole free pool against this one agent. Root cause: this model is zero-priced (Models.dev reports cost 0/0) and passes every existing chat-capability check (text output modality, no disqualifying model-id token), so free_discovered_models() admitted it into the general-purpose free pool that Strix's tool-calling requests route through. Models.dev's own tool_call field claims true for this exact model (verified live against models.dev/api.json), so that field cannot gate this; its declared input modality (text + image) is the only honest catalog evidence that distinguishes it from an ordinary text-only free worker. Two alternate fixes were considered and rejected against this repo's own tested contracts: - Loosening ModelClient's cross-provider failover to retry a plain HTTP 400 on a different free candidate is explicitly foreclosed by test_non_transient_error_is_not_replayed and test_virtual_passthrough_keeps_non_size_tool_errors_sticky (tests/test_passthrough_provider_failover.py): "caller errors fail closed instead of duplicating a request across providers." - The circuit breaker never records a sticky (non-failover-eligible) rejection at all, but recording it would not have prevented this incident either: each Strix attempt is a fresh sidecar process/gateway instance (in-memory circuit state), and the observed failure is the very first request each attempt makes. Fix: free_discovered_models() now excludes a free model that declares a non-text input modality from the general-purpose free pool. This is pool-composition, not per-request retry: the model stays fully discovered and price-evidenced (available to a pool that explicitly wants a vision/multimodal capability) and is only withheld from the capability-blind orchestrator/free default. Scoped to the free selector specifically (not general chat-candidate eligibility) so a genuinely tool-capable paid multimodal model elsewhere is unaffected. Known limitation: NVIDIA NIM's own /v1/models listing carries no tool/function-calling capability metadata at all, and Models.dev's tool_call field is unreliable at the per-deployment granularity (proven wrong here). If NIM's remaining free text-only catalog also turns out to lack genuine tool-calling support for some models, broader free-tier discovery (OpenRouter/Bytez/OpenAI free-tier offerings, today evidence-only or never free) is a follow-up, not something this change can fabricate evidence for. TDD: tests/test_model_discovery.py::test_free_discovered_models_excludes_a_free_vision_only_input_model reproduces the bug against a DiscoveredModel fixture shaped like the broken NIM agent (fails pre-fix, passes post-fix); a text-only free model and a free model with no modality evidence at all remain eligible. Verified: python -m pytest tests -q -> 2770 passed, 1 skipped. interrogate (fail-under 100) -> 100.0%. python tests/test_self_check.py and the naming-convention/chat-capability/provider-bootstrap check scripts named in README.md all pass individually too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX * fix(discovery): centralize free-serving-pool modality gate in TaskOrchestrator Devin's review on PR #933 found the vision-input exclusion added in 94e3b9a was incomplete on three points: 1. Runtime free pool remained unfiltered: `_auto_discover_runtime_agents` (`--auto-discover-model-agents`) and `provider_bootstrap`'s `_active_agent_from_discovered` (used by `bootstrap_provider_runtime` and, through it, `provider_catalog_bootstrap.bootstrap_provider_catalog_runtime`) both tag an agent `cost:free` from raw price evidence alone and never consulted the new exclusion, so NVIDIA NIM's free `meta/llama-3.2-90b-vision-instruct` could still reach a live, blindly-selectable `cost:free` agent through either path -- reproducing the original Strix tool-calling incident (ContextualWisdomLab/.github#1198). 2. `free_discovered_models()` conflated price-based inventory with serving-pool eligibility, so `--free-only`, `free_tier_count`, and the free-tier data-privacy totals silently undercounted a model that is genuinely free-priced but unfit for blind serving. 3. (Evaluated, not adopted) Devin also suggested narrowing the exclusion to spare a model that "also supports text as a standalone input". Rejected against this repo's own incident evidence: the incident model itself declares both `text` and `image` per Models.dev, so that narrowing would have silently re-admitted the exact model this fix is about. Kept the conservative "any declared non-text input modality disqualifies" reading, documented with a fixture for all three modality shapes (text-only, vision-only, text+image). Fix: - `free_discovered_models()` is pure price-based inventory again. - New `model_discovery.general_free_serving_candidates()` carries the modality-based exclusion for composing a blind free pool, wired into the `discover-models` CLI report as `general_free_serving_count` alongside the restored `free_tier_count`. - The actual enforcement is now `TaskOrchestrator._is_free_agent`, a single choke point every `orchestrator/free` selection path shares: an agent whose tags declare a non-text `input:<modality>` is never treated as free-pool eligible there, regardless of which code built it or how old that agent-pool row is (protects durable pool-store rows written before this exclusion existed, and any future pool-construction path). `cost:free` keeps meaning "honest zero price" everywhere else, preserving `provider_catalog_store.py`'s durable `is_free` round trip (see `test_serving_tags_preserve_only_explicit_free_and_modality_evidence` and `test_last_known_good_restores_free_and_modality_evidence`, both left unchanged). - `review_gateway.py` audited: its agents never carry a `cost:free` tag (tags are fully replaced with `("review",)`) and no price is registered, so it was never reachable through this bug; no change needed there. Also brought `interrogate --fail-under=100` back to green: it was already failing at 97.9% on the unmodified base commit (94e3b9a) from six pre-existing undocumented `fuzz/*.py` harness functions, unrelated to this incident; added their docstrings. TDD: new/relocated tests fail against the pre-fix source (verified by temporarily restoring it) and pass after the fix -- `test_auto_discovery_activates_a_free_vision_model_but_free_pool_excludes_it`, `test_active_agent_from_discovered_free_vision_model_is_not_free_pool_eligible`, `test_free_discovered_models_still_counts_a_free_vision_only_input_model`, `test_general_free_serving_candidates_modality_shapes`, and the relocated `test_general_free_serving_candidates_excludes_a_free_vision_only_input_model`. Verified: python -m pytest tests -q -> 2774 passed, 1 skipped (2770 passed, 1 skipped on the unmodified base commit; net +4 tests, zero regressions). interrogate (fail-under 100) -> 100.0%. python tests/test_conventions.py passes (new function name is valid two-or-more-word snake_case). git diff --check clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX * fix(discovery): scope the blind-free-pool modality gate to general chat only Devin's second review round on PR #933 (against 5648567) found the modality exclusion had overshot: TaskOrchestrator._is_free_agent had grown a non-text-input check meant only for the capability-blind general chat pool, but it also backed every capability-scoped free route (_capability_agents -> /v1/audio/transcriptions, /v1/videos, image, speech, rerank) and server.py's _require_pool_model. A free transcription agent naturally carries an input:audio tag and a free image/video agent an input:image tag -- exactly the modality its own capability-scoped free route is asking for, not a surprise -- so the shared predicate made those genuinely free agents unreachable through their own free route. Fix: split the predicate. - _is_free_agent reverts to plain, modality-blind price evidence (used by _capability_agents/_ranked_agents when chat_only=False, and by server.py's capability-scoped _require_pool_model branch). - New _is_general_free_agent = _is_free_agent(agent) and not _agent_requires_non_text_input(agent) is the stricter, general-chat-only variant, now used at every blind general-chat FREE_MODEL call site (proxy_completion, _orchestrated_provider_completion, route_once, conduct, _ranked_agents when chat_only=True, list_openai_models's advertising check, and server.py's capability-agnostic _require_pool_model branch). Also addressed the review's two informational notes: - "Duplicate serving policies can drift": extracted the actual "what counts as non-text" classification into chat_capability.requires_non_text_input, a single shared predicate both model_discovery._requires_non_text_input (DiscoveredModel.input_modalities) and orchestrator._agent_requires_non_text_input (an agent's input:<modality> tags) now delegate to, so the two representations of the same catalog evidence cannot diverge independently. New cross-consistency test: test_discovery_and_orchestrator_modality_eligibility_cannot_drift. - "Serving count uses a different population": documented in __main__.py that free_tier_count and general_free_serving_count are deliberately both computed over the complete `discovered` population regardless of --free-only, matching each other's established convention (not a new inconsistency). TDD: new tests fail against 5648567 (verified by temporarily inserting them against that commit) and pass after this fix -- test_free_virtual_model_selects_a_free_agent_whose_own_capability_needs_non_text_input (orchestrator._capability_agents path), test_require_pool_model_serves_capability_free_route_despite_non_text_input (server._require_pool_model path), plus updated assertions on the two existing regression tests from the first round confirming _is_free_agent now returns True (capability-reachable) while _is_general_free_agent stays False (blind-chat-excluded) for the same agent. Verified: python -m pytest tests -q -> 2775 passed, 1 skipped (net +1 test over the previous round's 2774; zero regressions). interrogate -> 100.0%. python tests/test_conventions.py passes. git diff --check clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX * fix(discovery): require routability in general_free_serving_candidates Devin's review pass on PR #933 after efd44f6 found two issues: 1. general_free_serving_candidates() admitted evidence-only and non-chat-capable zero-cost text-input catalog rows, overcounting general_free_serving_count with models that could never actually become a serving agent. Now also requires is_routable_discovered_model -- the same predicate _auto_discover_runtime_agents and provider_bootstrap already require before promoting a discovered row to an ordinary chat agent. New regression test test_general_free_serving_candidates_excludes_unroutable_free_models (an evidence-only free text model and a free embedding-only model) fails red pre-fix. 2. A real, deterministic CI failure on efd44f6 itself (GitHub Actions "Full unit and contract suite" job 99313736725): test_discovery_and_orchestrator_modality_eligibility_cannot_drift built ModelAgent fixtures using hyphenated ids straight from provider model ids (e.g. "text-only-model", "meta/llama-3.2-90b-vision-instruct"), which fail this repo's require_object_name two-or-more-word snake_case convention. Fixed by deriving a compliant id (casefold + translate "/.-" to "_") distinct from the `model` field under test. Also documents both this round's fix and the prior capability-route modality-scoping round in CHANGELOG.md (neither had an entry yet). Verified: python -m pytest tests -q -> 2779 passed, 1 skipped, 0 failed (with fast-mlsirm installed via git+https, working around this sandbox's plain-tarball-download 403 that is unrelated to the fix); interrogate -> 100%; tests/test_conventions.py -> ok; git diff --check -> clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX --------- Co-authored-by: Claude <noreply@anthropic.com>
|
@opencode-agent review Please review exact current head |
Merge-conflict triage: genuine architectural divergence, not attempting a guessed resolutionAttempted the standard
This means Per this repo's own standard for this exact situation (see Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com Generated by Claude Code |
Two conflicted files, four blocks. The branch's delta splits into two independent parts with opposite verdicts. PART A - paginated review evidence: KEPT (49 of the branch's 52 added script lines). main still fetches reviews through GraphQL 'reviews(last: 100)' (noema_review_gate.py:324), so a review event older than the last 100 is invisible to existing_noema_review() - main's only consumer of pr['reviews']['nodes'] - and Noema re-reviews a PR it already reviewed. fetch_complete_reviews() and its fetch_pr wiring merged cleanly outside every conflict block and are unchanged here. PART B - fail-closed guard before Noema skip paths: DROPPED as superseded (3 of 52 lines). a643d8a 'fix: make Noema review independent (#1477)' deleted all four helpers this guard is built on. Verified org-wide, not just in this file: current_primary_approval, has_current_changes_requested, has_unresolved_threads and blocking_checks each return 0 hits across all of origin/main (positive control: require_expected_head, 16 hits). main closed the same hole by removing the skip paths rather than guarding them, and made the primary-actor case raise RuntimeError where the branch only returned 1 - strictly stronger than this PR's fix. The branch's own test also called the pre-#1477 signature (inspect_and_review missing required 'expected_head'), so it was superseded twice over. Block verdicts: - noema_review_gate.py x2 -> main's inspect_and_review structure. The branch's guard has no surviving call target. - test file, block 1 -> main's test_existing_noema_review_matches_ actor_and_head, plus the branch's two Part A tests appended at EOF (the conflict boundary cut through the middle of main's function, so they cannot be inserted at the block site). - test file, block 2 -> main's parametrized cases; the branch's used make_pr shapes for deleted behaviour. - test_noema_never_skips_success_without_current_head_primary_approval merged cleanly outside the blocks and was removed: it is Part B. One assertion was translated, not dropped: test_fetch_pr_keeps_exact_head_approval_older_than_one_hundred_reviews asserted through the deleted current_primary_approval(). It now asserts main's live path directly - len(pr['reviews']['nodes']) == 101 and nodes[0] is the pre-page-boundary approval - which is the test's stated purpose and keeps its detection power. Evidence: - uvx ruff check --select F821 scripts/ci tests: All checks passed - full suite, branch head b8f4adb (unmerged): 2099 passed, 0 failed - full suite, this merge: 2899 passed, 0 failed (failure-name diff empty both ways, denominators 2099 and 2899) - negative control: deleting the fetch_pr REST wiring makes the kept pagination test fail by name -> it detects the regression it claims to - coverage: TOTAL 100%, noema_review_gate.py 860/860 100% - interrogate: PASSED (minimum 100.0%) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
main 병합 완료 — 이 PR의 델타가 둘로 갈라졌습니다 (
|
| 심볼 | main 전체 hits |
|---|---|
current_primary_approval |
0 |
has_current_changes_requested |
0 |
has_unresolved_threads |
0 |
blocking_checks |
0 |
(포지티브 컨트롤) require_expected_head |
16 |
main은 같은 구멍을 가드를 붙이는 대신 스킵 경로 자체를 없애서 막았고, 1차 리뷰어 액터 케이스는 return 0 스킵에서 raise RuntimeError로 바뀌었습니다 — 이 PR의 return 1보다 강합니다. 게다가 이 PR의 테스트는 #1477 이전 시그니처를 호출합니다(inspect_and_review() missing 1 required positional argument: 'expected_head'). 이중으로 대체된 상태입니다.
블록별 판정
| 블록 | 판정 |
|---|---|
noema_review_gate.py ×2 |
main의 inspect_and_review 구조 채택. 브랜치 가드는 호출 대상이 남아 있지 않습니다. |
| 테스트 블록1 | main의 test_existing_noema_review_matches_actor_and_head + 브랜치의 Part A 테스트 2개를 파일 말미에 추가. 충돌 경계가 main 함수 중간을 지나가서 원위치 삽입은 함수를 깨뜨립니다. |
| 테스트 블록2 | main의 파라미터 케이스 채택. 브랜치 케이스는 삭제된 동작의 make_pr 형태였습니다. |
test_noema_never_skips_success_without_current_head_primary_approval |
충돌 밖에서 병합되어 남아 있었고 제거했습니다 — Part B입니다. |
단언 하나는 버리지 않고 번역했습니다
test_fetch_pr_keeps_exact_head_approval_older_than_one_hundred_reviews가 삭제된 current_primary_approval()을 통해 단언하고 있었습니다. main의 실제 경로로 바꿨습니다 — len(pr["reviews"]["nodes"]) == 101과 nodes[0]이 페이지 경계 이전의 승인인지. 테스트가 스스로 밝힌 목적("must not lose a valid approval behind GitHub's review page size") 그대로이고 탐지력도 유지합니다.
검증
uvx ruff check --select F821 scripts/ci tests— All checks passed- 전체 스위트, 병합 전 브랜치 헤드
b8f4adb5: 2099 passed / 0 failed - 전체 스위트, 이 병합: 2899 passed / 0 failed (실패 이름 차집합 양방향 공집합, 분모 2099·2899)
- 네거티브 컨트롤:
fetch_pr의 REST 배선을 지우면 보존한 페이지네이션 테스트가 이름을 찍고 실패 — 주장하는 회귀를 실제로 탐지합니다 coverageTOTAL 100%,noema_review_gate.py860/860 100%interrogatePASSED (minimum 100.0%)
fast-forward 푸시이며 force push는 없습니다.
확인 요청: 이 PR의 제목은 Part B(fail closed before every Noema skip path)인데 실질 내용은 이제 Part A입니다. 제목을 페이지네이션 수정에 맞게 바꾸시는 편이 리뷰어에게 정확합니다.
🤖 Generated with Claude Code
2 participants
Summary
main@4b115bdad2b3682f36639c4f978fd0e976a83ff0; current-main scheduler, workflow, security, and contextual-orchestrator behavior remains authoritative.Current identity
b8f4adb5a54f561fc019d0856a43f56a195351411216d855f9cbda27a29dbd35a7ea2e481275a2a8main@4b115bdad2b3682f36639c4f978fd0e976a83ff00 behind / 49 aheadscripts/ci/noema_review_gate.pyandtests/test_noema_review_gate.pyTDD evidence
RED on protected-main source:
4 failed5a51a6124bfce2cfc73199b490ff684f230b0be3used protected workflow4b115bda, loggedCurrent head does not have a primary OpenCode approval; Noema review skipped., and still concluded GitHub-success.GREEN on the exact replacement tree:
24 passed2,099 passed · 1 skipped · 21 subtests10,466/10,4664,172/4,172100%1216d855...Acceptance gate
Do not merge from source evidence alone. Require terminal exact-head workflows, zero unresolved actionable threads, and a qualifying exact-head formal review under live protection. After protected-main integration, unchanged consumer canaries must prove that GitHub-success can no longer coexist with an absent exact-current-head primary verdict.
All predecessor workflow results are historical and non-passing for this head. Exact-head hosted security, coverage, Strix, OpenCode, and Noema evidence is regenerating and remains non-passing until terminal.