perf(review): bound verification label scans - #1555
Open
seonghobae wants to merge 464 commits into
Open
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+977
to
+990
| end = len(text) | ||
| for candidate in APPROVAL_VERIFICATION_LABELS: | ||
| if candidate == label: | ||
| continue | ||
| index = text.find(candidate, start, end) | ||
| while index != -1: | ||
| if ( | ||
| candidate == "coverage:" | ||
| and text[max(0, index - 10) : index] == "docstring " | ||
| ): | ||
| index = text.find(candidate, index + len(candidate), end) | ||
| continue | ||
| end = min(end, index) | ||
| break |
Contributor
There was a problem hiding this comment.
📝 Info: Bounded scans preserve section selection
The evolving end cannot hide a winning boundary. Later matches outside it cannot beat the current minimum, while coverage: suffix collisions remain excluded.
Was this helpful? React with 👍 or 👎 to provide feedback.
QUEUE_SATURATION_CHICKEN_EGG: exact-head Devin/CodeRabbit review is clean, all inline findings are resolved, and remaining hosted admission evidence is queued behind the saturated Actions fleet. This change collapses 18 scheduled caller sources into one reviewed scheduler and carries the protected-main-only one-shot registry retirement needed to disable their persistent workflow identities.
QUEUE_SATURATION_CHICKEN_EGG: exact head 23908fa was mechanically mergeable with one docs-only file, no review threads, no CHANGES_REQUESTED or security findings, while exact-head required/security workflows and required-workflow bootstrap were queued in a 1,593-run central Actions backlog. Bypass is limited to those queue-bound admission blockers; no failing gate or substantive finding was bypassed.
QUEUE_SATURATION_CHICKEN_EGG: exact head 42612f4 is mechanically mergeable and changes only the one-shot registry-retirement runner, its focused regression contract, and doctoring. The sole reviewed source finding was corrected on this exact head; there are no CHANGES_REQUESTED reviews. Required/security workflows remain queued in the saturated standard-runner control plane that this change is needed to escape. The active ruleset permits OrganizationAdmin bypass. This merge bypasses only queue-bound admission evidence so the protected-main migration can run on ubuntu-slim and retire obsolete workflow identities; it does not bypass a failing test, security finding, merge conflict, or substantive unresolved implementation objection.
) QUEUE_SATURATION_CHICKEN_EGG: exact head 265b5fa is mechanically mergeable, 0 behind current protected main at verification, and changes only the registry-retirement one-shot, its focused regression contract, and doctoring. Protected-main run 33596622523/job 100141255712 proved #1684 escaped the saturated runner lane but failed before mutation because the complete workflow registry returned zero matches for the first deleted legacy path. The repair treats zero legacy matches as terminally absent, exactly one as disable/read-back, duplicates as fail-closed, while replacement and self remain exact-one. Independent execution of the exact shell logic proved absent->success+self-disable, duplicate->fail-before-self-disable, and visible-active->disable+read-back; focused contract/YAML parsing also passed. There are no exact-head submitted reviews, CHANGES_REQUESTED reviews, or inline review comments, and no observed exact-head failing check; required/security workflows are queued in the same central capacity condition this migration must relieve. Active ruleset 17921150 permits OrganizationAdmin bypass. This merge bypasses only queue-bound admission evidence; it does not bypass a failing test, security finding, conflict, or substantive unresolved review objection.
QUEUE_SATURATION_CHICKEN_EGG: exact head f9a54bb is mechanically mergeable, 0 behind protected main 6958918, and changes only post-success lifecycle cleanup: delete the disabled registry-retirement one-shot and its migration-only fixture, remove dead quality-CI watch/compile entries, and preserve the doctoring record. Live protected-main run 33597034283 / job 100142454414 completed SUCCESS, enumerated the complete paginated workflow registry, proved all 18 deleted legacy caller identities absent, revalidated the consolidated replacement active, then disabled and read-backed the one-shot identity 348089470; the hosted-evidence review finding was answered with that proof and both inline threads are resolved. Exact-head PR workflow inventory has no observed failing run; security/quality workflows remain queued under repository-wide Actions saturation (1,780 queued versus 5 in-progress at verification). Active ruleset 17921150 authorizes OrganizationAdmin bypass. This merge bypasses queue-bound admission only, not a failing test, security finding, CHANGES_REQUESTED review, provenance defect, conflict, or unrelated policy defect.
…re (#1697) QUEUE_SATURATION_CHICKEN_EGG: exact head 9965863 is mechanically mergeable, independently reviewed with no findings, and all protected admission runs are queued under central Actions saturation. Preserve live-head validation while retiring superseded draft/ready dispatches without red-X noise.
…ust transport-failure count (#1707) The "Fail closed without a current-head OpenCode verdict" step's polling loop was bounded only by max_poll_transport_failures (consecutive gh api transport failures) with no total wall-clock cap of its own. When a review dispatch never produces a verdict while every individual gh api call keeps succeeding, the loop polled forever, holding a live GitHub Actions runner for up to the platform's 360-minute default job timeout. Confirmed live in production: multiple "Required OpenCode Review"/"Strix Security Scan" runs stuck in this exact step for 7-20 hours (e.g. run 33509949967 on bandscope#1115 stuck 1190+ minutes), consuming enough of the org's shared Actions concurrent-job capacity to stall required-review dispatch for essentially every other open PR (thousands of queued runs across .github, contextual-orchestrator, naruon, and other repos). Adds a 3-hour (10800s) wall-clock deadline check at the top of each poll iteration -- comfortably above this org's own documented "accommodate over 2 hours per model" allowance (docs/product-goal-directive.md §8) so a legitimately slow model is never falsely failed, but well short of GitHub's 360-minute job default so a runner is reliably released. This bounds how long the CI job waits for a verdict; it does not cap the model's own reasoning/streaming time, which remains governed entirely upstream. Verified directly: extracted the real step body via PyYAML (matching this file's own existing test extraction pattern) and executed it against a stubbed gh CLI with bash 5. A never-resolving verdict now exits cleanly with a clear diagnostic at exactly the deadline instead of hanging; a verdict posted immediately still succeeds normally and is unaffected by the new check. This is an emergency direct fix authorized by the repository owner given the ongoing org-wide capacity incident (a genuine chicken-and-egg situation: this fix's own required review cannot complete because the system it fixes is what is broken). Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
scan-pr-queue in pr-review-merge-scheduler.yml had no job-level timeout-minutes, so a stuck run (rate-limited GitHub API, a hung gh invocation) falls back to GitHub's 360-minute platform default. Live evidence (2026-09-02, 69 queued runs for this workflow, several schedule/push/workflow_run runs queued for hours) shows this contributing to the org-wide Actions capacity incident, alongside the sibling opencode-review.yml unbounded verdict-polling fix. Bound it to 30 minutes: shorter than org-queue-sweep's existing timeout-minutes: 60 precedent, since scan-pr-queue only scans this one repository's PR queue (paginated GraphQL reads, page size 25, plus at most one review dispatch and one branch update per run) rather than walking every target repository in the organization. Left cancel-in-progress as-is for the workflow_run/push/schedule paths: cancelling scan-pr-queue mid-mutation (mid-merge, mid-branch-update) risks leaving a PR/branch partially updated, and there is no evidence in hand that this is safe. The new timeout bound alone converts an unbounded job into a bounded-but-serial one, which is the minimal safe fix. Adds test_scan_pr_queue_has_a_bounded_runtime asserting the job declares timeout-minutes in (1, 45] and strictly less than org-queue-sweep's 60. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…#1704) org-queue-sweep explicitly excludes ContextualWisdomLab/.github from its target list, so scan-pr-queue's own cron is the sole periodic fallback for this repository's PR queue, and for any required check (Security Scan, SAST Semgrep) with no workflow_run listener anywhere in this file. Deleting it would leave this repository strictly worse off than every sibling repo, which still gets the hourly org-sweep. Apply the same lever #1630 already used for org-queue-sweep: lengthen the cron from */30 (every 30 min) to hourly, offset to "30 * * * *" so it doesn't collide with org-queue-sweep's "0 * * * *" tick. Document why the entry exists and why it cannot simply be removed, the way the adjoining org-queue-sweep cron already documents its own rationale. Add a cadence-contract test asserting the new schedule, and refresh a stale docstring in test_required_workflow_queue_contract.py that referred to "the separate 30-minute scan". Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
… (#1710) * fix(tests): match live-head-moved regression to #1697's intentional reorder #1697 (commit 5c561a6) reordered opencode-review.yml's live-state checks so closed/draft admission runs before the head-SHA-match check, and exits 0 instead of 1 for an open, ready PR whose live head has moved. A draft PR whose live head has moved is therefore exempted by the draft check first — the head-moved branch is now unreachable while still draft. test_opencode_live_draft_state_regression.py's test_draft_exemption_fails_closed_when_live_head_moved still asserted the pre-#1697 behavior (returncode 1, "head moved while validating live" in stdout) for exactly that input shape, so it fails on current main. Update it to assert the actual current behavior (returncode 0, exempted via the draft-check message), matching the equivalent direct-production-step coverage #1697 already added in test_opencode_required_verdict_regression.py. Confirmed via a clean origin/main worktree that the regression pre-dates this change and is not introduced by it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 * fix(tests): stub sleep in OpenCode poll regression tests, salvage #1706 Two existing tests extract the real "Fail closed without a current-head OpenCode verdict" step's bash and run it against a fake gh, but never stubbed `sleep` -- driving the transport-failure retry path to its 3-failure threshold performed two genuine 60s sleeps per affected test run (confirmed directly: this exact gap made a 2-test run exceed a 120s timeout). Both now stub `sleep` alongside the existing fake `gh`, matching the pattern already used in test_opencode_poll_self_retirement.py: tests/test_opencode_required_verdict_regression.py::test_fail_closed_step_still_polls_for_a_non_draft_pr tests/test_opencode_live_draft_state_regression.py::test_stale_draft_verdict_event_does_not_exempt_live_ready_pr Also fixes test_opencode_poll_self_retirement.py, which was silently broken on current main: #1707's wall-clock-deadline fix to opencode-review.yml added a `poll_deadline_epoch` reference at the top of the poll loop, but this file's `_run_poll_loop` harness never declared that variable before splicing in the now-changed real loop body, so 7 of its tests failed with an empty gh-calls.log (the script aborted under `set -u` before making any call). Adds the missing `poll_deadline_epoch` line and an injectable fake `date` (extending the existing fake-gh/fake-sleep/fake-timeout harness) to prove the wall-clock deadline logic itself: the loop fails closed with the new diagnostic once the deadline is exceeded even when every gh call keeps succeeding (the exact zombie scenario the fix targets), a fast verdict is unaffected, and the production shape keeps both bounds distinct and additive. No test sleeps for real time. Full affected suite (73 tests) verified green in ~16s; the full project suite (2582 passed, 1 skipped, 21 subtests) runs in ~116s with 100% coverage and 100% docstrings, matching #1706's own claimed 236.65s -> 112.76s improvement. This is a same-file-conflict-driven successor to #1706, which also included this exact test-file delta. #1706 additionally touched .github/workflows/opencode-review.yml with the wall-clock-deadline logic itself -- that exact fix already landed separately as #1707 (bypass-merged during the org-wide capacity incident, before #1706 finished), which is why #1706 is now DIRTY/CONFLICTING against main through no fault of its own test-file changes. This PR carries only the still-valid, non-redundant test-suite-hang fix forward; #1706 is being closed in favor of this PR. Branched from and includes #1705 (fix/live-draft-regression-test-1697, a different in-flight fix to the same tests/test_opencode_live_draft_state_regression.py file, addressing an unrelated draft-head-moved logic question) to avoid a second same-file conflict. If #1705 merges to main independently before this PR, this PR's identical carried-forward hunk should merge as a no-op; if this PR merges first, #1705 should rebase onto main afterward. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
Preserve the quality-gate close-retirement delta while integrating protected main 445f6be. The intervening #1710 changes only OpenCode regression tests and do not overlap PR #1708's workflow, doctoring, or runtime-budget contract. Two-parent reconciliation preserves both histories without force-push or destructive rebase.
…ose-retirement fix(actions): retire queued quality CI when a PR closes
) inspect_pr() calls cancel_stale_pr_runs() unconditionally for every non-draft PR before any eligibility gate, and other call sites (active_review_run_refs, dispatch_strix_evidence's busy check) ask the identical unfiltered (repo, ("queued", "in_progress")) question again -- all against the one repository a scheduler invocation ever targets, with zero caching anywhere in the file. At the default MAX_PRS=100 this reissued the same repository-wide, paginated gh api .../actions/runs fetch well over a hundred times per run. Memoize active_workflow_runs's result keyed on the full (repo, statuses, event, created, head_sha) call shape for one main() invocation, with explicit cache invalidation immediately after the four places that mutate GitHub Actions run state (force_cancel_workflow_runs, rerun_actions_job, dispatch_opencode_review, dispatch_strix_evidence) so a later read in the same run never replays a pre-mutation snapshot -- a blind never-invalidated cache would let a just-cancelled run still look "busy," or let a same-invocation dispatch go undetected by the repository-wide single-concurrency dispatch guard. The four pre-existing ThreadPoolExecutor sites and the correctly sequential per-PR mutation-budget loop are untouched; this is a duplicate-read cache fix, not a parallelization of anything with ordering dependencies. Adds 4 tests proving identical results, a genuine wall-clock improvement (artificial per-call delay stub), cache-key correctness across distinct call shapes, and invalidation-on-mutation correctness; updates 2 existing call-index assertions that shifted because a busy-check read is now a cache hit; adds an autouse fixture isolating the new module-global cache between tests. 100% coverage/docstrings on scripts/ci preserved. See docs/adr/0022 for the full record, including why this stays Python (I/O-bound gh CLI/GraphQL+REST glue, not a CPU-bound path Rust would meaningfully speed up). Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…#1712) force_cancel_workflow_runs() returns a {run_id: failure_reason} dict for cancellation calls GitHub actually rejects, but cancel_stale_pr_runs(), cancel_stale_opencode_runs() (via force_cancel_workflow_run_refs()), and dispatch_strix_evidence()'s busy-run exclusion set all discarded that return value and treated every requested run_id as gone. A rejected cancellation could therefore let a duplicate review dispatch alongside a run that was, in fact, still active. force_cancel_workflow_run_refs() now returns only the refs GitHub actually cancelled; its three callers (cancel_stale_opencode_runs, dispatch_opencode_review, dispatch_strix_evidence) and the sibling direct caller cancel_stale_pr_runs() use that corrected result instead of assuming success. Discovered mid-flight during PR #1669's development (the naruon headRefOid cancellation fix) and intentionally scoped out of that PR to keep its diff to the bug it was opened for; landing fresh here. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
#1669) * fix(scheduler): never let a falsy headRefOid cancel every run for a PR stale_pr_run_ids() and active_review_run_refs() in scripts/ci/pr_review_merge_scheduler.py computed the PR's expected current head as str(pr.get("headRefOid") or "").lower(), unlike every other head-comparison call site in this file, which validates via validate_git_sha(). A falsy headRefOid (missing/None/empty -- plausible on a PR inspected moments after it opens) silently coerced to "", which never equals a real run head_sha, so every active run for that PR -- including one for its true, unchanged current head -- was misclassified as stale and force-cancelled with no further check by cancel_stale_pr_runs() / cancel_stale_opencode_runs(). This is called unconditionally from inspect_pr() by both the per-PR scan-pr-queue job and the hourly org-queue-sweep job, so it is an org-wide exposure, not repo-specific. This reproduces the 2026-09-02 incident where naruon PR #1528's Strix run (33581213829) was cancelled while it was the PR's sole, still-current head. It is the same bug class docs/doctoring/queue-hygiene-live-ref-race.md already fixed for the sibling bash "Queue hygiene" cancellation path (which revalidates every candidate via revalidate_queue_cancellation.sh before cancelling) -- that fix never touched this earlier-running, revalidation-free path in the same inspect_pr() pass. Both functions now fail safe on a falsy headRefOid: log a warning and return no stale/cancellable runs for that PR, instead of treating an unresolved head as "matches nothing, so everything is stale." Adds three regression tests reproducing the incident with the real run id, head SHA, and PR number, each failing against the pre-fix code and passing after the guard. Full suite: 2603 passed, 1 skipped; 100% coverage and docstrings hold on scripts/ci. Doctoring note: docs/doctoring/scheduler-stale-headrefoid-cancellation.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(scheduler): stage live-head cancellation race repair * ci(scheduler): run bounded live-head cancellation repair * fix(scheduler): repair exact-head cancellation race findings * fix(scheduler): run exact-head live cancellation repair v2 * fix(scheduler): adapt legacy synthetic SHA fixtures * fix(scheduler): publish validated repair with branch token * test(scheduler): preserve docstring gate in generated race repair * ci(scheduler): verify v4 live-head race repair * fix(ci): isolate PR1669 writer credential to publish step * docs: make stale-cancellation incident references linkable * ci: retrigger exact-head PR1669 guarded repair * test(scheduler): cover live cancellation revalidation branches * fix(actions): publish guarded scheduler repair with branch token * fix(actions): isolate scheduler repair write authority * fix(scheduler): make PR1669 repair current-main aware * fix(actions): run current-main-aware PR1669 publisher * test(scheduler): cover parallel live stale-run cancellation * test(actions): rerun PR1669 with parallel cancellation coverage * test(scheduler): refresh legacy cancellation fixtures * ci(pr1669): verify refreshed legacy fixtures * test(pr1669): close live revalidation coverage gaps * test(pr1669): cover direct revalidation fail-closed branch * test(scheduler): cover stale OpenCode cancellation owner path * refactor(pr1669): retire dead pre-revalidation batch helper * fix(scheduler): cover exact-head cancellation path * test(scheduler): align PR1669 fixtures with live revalidation * ci(scheduler): run PR1669 v8 fixture repair * fix(ci): provision PR1669 publisher verification toolchain * fix(ci): publish PR1669 successor with scoped workflow token * ci: preserve PR1669 successor check triggering * test(scheduler): prove draft retry cancellation race * ci: include PR1669 draft-review RED repair * fix(ci): strip PR1669 regression EOF whitespace * fix(ci): align PR1669 draft cancellation fixture * fix(ci): make PR1669 fixture reconciliation indentation-safe * ci(scheduler): coalesce PR1669 repair onto one runner * ci(scheduler): publish verified one-shot repair with scoped token * fix(scheduler): revalidate live state before cancellation * test(scheduler): preserve failed stale-run cancellations * test(scheduler): cover dispatch cancellation failure * ci(scheduler): repair failed-cancellation result semantics * ci: add one-shot failed-cancellation repair * ci: retire weaker PR1669 repair writer * ci: repair PR1669 cancellation-result fixtures * test(scheduler): make central run revalidation credential RED * repair(scheduler): bind stale-run reads to causal credential * test(pr1669): reproduce the naruon headRefOid incident directly Add real, non-vacuous regression tests to tests/test_pr1669_cancel_stale_opencode_runs.py for the exact incident PR #1669 exists to fix: ContextualWisdomLab/naruon PR #1528's Strix run 33581213829 (head cf472cf77fb93325858f485a22e967449d7c387a) was force-cancelled while it was the PR's sole, unchanged current head, because stale_pr_run_ids() and active_review_run_refs() computed the expected head as str(pr.get("headRefOid") or "").lower() instead of validating it. The branch's tip (c06b625) already carries the real fix -- both functions now validate the snapshot headRefOid via validate_git_sha() and fail safe (empty result) when it is missing or malformed, matching the idiom used elsewhere in this file -- and the six temp_pr1669_*/-_temp_pr1669_* debris files from an earlier abandoned self-repair attempt are already gone from this branch. What was missing was direct regression coverage naming the incident: the prior test file only covered cancel_stale_opencode_runs()'s revalidation plumbing, and the PR description claimed three tests (test_stale_pr_run_ids_preserves_current_head_run_when_head_ref_oid_missing, test_active_review_run_refs_preserves_current_head_run_when_head_ref_oid_missing, test_cancel_stale_pr_runs_issues_no_cancel_call_when_head_ref_oid_missing) that did not exist anywhere in the repository. Each new test uses the real naruon PR #1528 / run 33581213829 identifiers, was verified by temporarily reverting the validate_git_sha() guard to fail against the pre-fix code, and passes against the current fix. Full suite: 2604 passed, 1 skipped, 21 subtests passed. 100% coverage on scripts/ci. 100% docstring coverage (interrogate). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ci(pr1669): remove a second, out-of-scope self-repair attempt While rebasing onto this branch's latest remote tip, a second in-flight self-repair attempt (unrelated to the headRefOid bug this PR fixes) was found already pushed here: a push-triggered, contents:write one-shot workflow (.github/workflows/_temp_pr1669_failed_cancel_result_repair.yml) plus three failing regression tests for a different bug -- a rejected force_cancel_workflow_runs() API call being silently reported as a successful cancellation. That bug looks real, but it is not the headRefOid bug this PR exists to fix, and the still-armed workflow (watching pushes to this exact branch, touching tests/test_pr1669_cancel_stale_opencode_runs.py among its trigger paths) would have reacted to this very push and self-modified the branch again. Remove both rather than let another self-modifying generator loop run here or leave unlanded RED tests behind; the finding has been routed to a separate, properly scoped follow-up instead of being fixed or left dangling in this PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ci(pr1669): remove a third, out-of-scope RED test file tests/test_pr1669_central_run_revalidation_credential.py (added by a concurrent, still-running self-repair pass) asserts that _fresh_active_run_for_cancellation() must read a central repository_dispatch run through the dispatch credential (gh_api_json_via_dispatch_token) rather than the target-repo read credential (gh_api_json). One of its two tests currently fails against this file's actual implementation, which always uses gh_api_json() regardless of which repository is being read -- a real-looking credential-boundary gap, but in the defense-in-depth revalidate-before-cancel machinery, not the headRefOid bug this PR exists to fix, and no corresponding code change had landed for it (diff against c06b625 confirms scripts/ci/pr_review_merge_scheduler.py is unchanged). Removed for the same reason as the prior cleanup commit: keep this PR's diff scoped to the headRefOid fix it was opened for, and genuinely green, rather than carrying an unlanded RED test for an unrelated, still-being- worked-on finding. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
#1883) The admission-controller feature burst (#1859-#1869) shipped review_admission_controller.py, pr_review_merge_scheduler_core.py's SchedulerAdmissionGate, and (separately, pre-existing) a coverage gap in audit_codeql_default_setup_rollout.py without full test coverage or docstrings, breaking the required 100% coverage/docstring gate for every PR in this repository regardless of that PR's own diff. The original fix for this landed on .github#1871, which was later closed in favor of narrower successors (#1877 for the stale schedule oracles, #1879 for HTTP error response bodies) -- but the coverage and docstring portion of #1871's delta was dropped in that narrowing and never reached main. This PR recovers exactly that portion from #1871's still-present branch (fix/hourly-review-repair-callers-cron- format-drift) and completes it: - review_admission_controller.py: 85% -> 100% coverage (new tests/test_review_admission_controller.py), 14 missing docstrings added across its WorkerBoundary/AdmissionRequest/RequestRecord/ DispatchLease/ControllerState/DispatchPlan dataclasses and methods. - audit_codeql_default_setup_rollout.py: 79% -> 100% coverage (new tests/test_codeql_default_setup_rollout.py), 2 missing docstrings added (parse_args, main). - pr_review_merge_scheduler_core.py's SchedulerAdmissionGate: 3 missing docstrings added (__init__ and its two nested closures, lease/reconcile_state). Additionally closed pr_review_merge_scheduler_core.py's own separate, longer-standing coverage gap (98% -> 100%, unrelated to the admission-controller work) discovered while verifying this fix would actually bring main to a green gate rather than a differently-shaped 99%: the durable admission gate's own bounded-budget/stale-head branches across every dispatch call site (9 "admission_deferred" checks across post_update_branch_followup/dispatch_draft_review_only/ inspect_pr, plus dispatch_strix_evidence's own two "admission_deferred"/ "stale_head" pairs), reconcile()'s live-head-moved and still-running branches, rotating_pr_window's/dispatch_draft_review_only's/the workflow-run classifier's/the empty-PR-close path's/main()'s own --admission-state-path wiring's remaining gaps, and two untestable package-import fallback lines marked `# pragma: no cover - package import path` matching this file's established convention for that exact pattern. Full local triad: 2875 passed, 1 skipped; coverage 100%; interrogate 100%. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…e constraint (#1907) Lands this session's durable know-how into the repo per the org convention that durable knowledge belongs in the repo, not private agent memory. Scope was divided with the four concurrent peer sessions so no two write the same area; this covers only what this session actually got wrong and corrected. AGENTS.md gains a "Verification discipline" section: check organization-wide before calling an item unstarted, read a library's own source (not its README) before any negative capability claim, treat a peer's restatement as one check rather than two, and prefer a different model family for adversarial review. CLAUDE.md gains two gotchas. The first is load-bearing and was nearly recorded backwards: because every agent session shares one GitHub identity, sessions cannot approve each other's PRs, and merge_approval_block_reason fails closed without a non-author formal APPROVED review on the exact current head -- so a verification comment documents evidence but cannot unblock a merge. The second covers actions/runs?status=completed being a misleading sample during queue churn. A read-only Codex pass reviewed this text before commit and caught the approval claim stated backwards, plus several overstatements; the section recommending cross-family review is itself a product of that catch. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Applied fixes from three independent critique lenses run against the draft: - Narrowed "a red required check" to the suite gates this procedure actually diagnoses; Semgrep/CodeQL/Strix/Scorecard are a different diagnosis. - Replaced the absence claim about push-to-main full-suite runs with the checkable one: those workflows exist but are paths-filtered, so a pairing broken outside their declared paths lands with no full-suite run. - Cut point-in-time merge-velocity figures, which read as false during any quiet period, in favour of the mechanism they were illustrating. - Replaced the CI-inventory assertion about markdown linting with the durable in-repo fact: no test parses fenced blocks. - Made the content-hash pin discoverable via grep instead of enumerated, and warned that most forty-hex literals under tests/ are commit/action pins that hash-object would corrupt. - Stated the interrogate rule first and the configuration second, since the config is a line a future PR can flip. Added the two-dot/three-dot diff rule: gh pr diff is already three-dot, so the mass-revert illusion belongs to two-dot git diff, not to gh. Verified: python3 -m pytest tests -q -> 2883 passed, 1 skipped, 21 subtests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Counted them rather than characterising from memory: of 17 distinct forty-hex literals across 14 test files, exactly one is a blob pin. The rest are pinned action SHAs, a vendored-revision pin, synthetic fixture heads, and assertions that a SHA appears in a document. "Commit or action pins" undersold that variety; naming the categories lets an agent recognise which constant it is holding before reaching for hash-object. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
) * docs(agents): document how to verify a "superseded" closure claim "repair, don't close" is already a standing convention, but the *verification method* was never written down -- so a closure whose comment reads convincingly could discard real, tested delta. Records the commands that actually caught one (#1661, closed as "superseded by protected main" while a ~450-line concurrency fix and 3 of 4 cited doctoring docs were provably absent from main): git grep -l "<symbol>" origin/main --, git show origin/main:<path>, and git diff --stat origin/main <head> because a long-lived branch's title records what it was opened for, not what it now contains. Narrowing a PR into successors is the same claim and needs the same proof. Also records why cancel-in-progress: true is safe on a PR-scoped group once an admission job re-verifies the live head -- the SHA-suffix and cancel-in-progress: false workarounds are then unnecessary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(agents): correct the closure-verification commands after cross-model audit An independent read-only Codex audit of the first draft found seven factual problems. Corrected here rather than shipping guidance that would mislead: - Dropped the cancel-in-progress bullet entirely. It claimed an in-workflow admission job makes cancel-in-progress: true safe, but workflow-level concurrency cancels before any job runs, so no job can precede it. It also contradicted the existing "put concurrency at workflow scope" guidance, and the SHA-suffix point was already covered two bullets above. - git diff now uses three-dot (origin/main...<head>). Two-dot reports main's own newer commits as phantom deletions by the PR -- precisely wrong for the stale PR this section is about. - git grep gains -F (it is a regex otherwise), and no longer claims absence proves the delta is missing: a successor may have renamed or restructured it, and a match does not prove behaviour was inherited. - git show now says the path exists *now*; a non-zero exit does not mean the content never landed, since it may have landed and later been deleted. - The closure conditions are quoted from the repo's actual documented rule in docs/org-required-workflow-rollout.md instead of an invented four-item list. - Dropped an unnamed "several PRs" claim, and qualified the reopen advice: inconclusive evidence is not disproven succession. Adds the ancestry caveat: git merge-base --is-ancestor answers a different question, and is unreliable in both directions here because this repository mixes squash merges with real merge commits (measured: 153 single-parent vs 47 two-parent over 200 commits). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…itical path (#1910) opencode-review.yml chained five jobs in series: required-workflow-bootstrap -> admit-current-head -> coverage-source-tree -> coverage-evidence -> opencode-review-target. The middle two exist solely to hold branch-protection contexts; each one's entire body is a single `echo`, and neither declares `outputs:`, so both `needs:` edges through them ordered work without carrying any data. Ordering is not free. A job is not created until its `needs:` complete, so under a saturated queue every link waits out the whole queue again. Measured on naruon#1528 (run 33581213805), where each job's created_at equals the previous job's completed_at: required-workflow-bootstrap waited 7h57m, ran 4s coverage-source-tree waited 9h40m, ran 4s coverage-evidence waited 13h01m, ran 5s opencode-review waited 12h13m That is ~22h41m of queue time spent to print two sentences, with the actual review held behind it. Both edges are removed and the two context holders now depend on admit-current-head directly, dropping serial depth from 5 to 3 and the number of queue waits from 4 to 2. Safety, verified rather than assumed: - coverage-evidence had no `if:` and relied entirely on transitive skipping through coverage-source-tree. Its admission gate is now stated explicitly, so an unadmitted head still skips it. Dropping the edge without this would have run a required context on unadmitted heads. - opencode-review-target never reads coverage-evidence at runtime; the only reference was the `needs:` line itself. The consumer of that context is opencode-review-dispatch.yml via scripts/ci/opencode_coverage_identity.py, which resolves it against the check-runs API on its own schedule. - No test asserts this ordering. scripts/ci/test_strix_quick_gate.sh:1203 names both jobs but as set membership, not sequence. - Branch protection evaluates required contexts independently; all of required-workflow-bootstrap, coverage-evidence and opencode-review still report. Scope is deliberately limited to opencode-review.yml. opencode-review-dispatch.yml has jobs of the same two names whose edge is a real data dependency -- its coverage-source-tree uploads the materialized PR merge tree and its coverage-evidence downloads it -- and must not be parallelized. Credit to peer session review for catching that name collision, and to a Codex audit for finding the echo-only jobs in the first place. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…pline (#1909) Two rules from mistakes this session actually made and corrected, per the per-session lane split agreed with the other concurrent sessions (peer 3 took verification discipline in #1907; peer 2 has gate/merge mechanics; host 1 has close-time diff comparison and noema concurrency; host 2 has CI failure diagnosis). - Narrowing a PR does not carry its delta. #1871 was closed in favor of #1877 plus #1879; both successors were green, but neither carried the coverage/docstring delta, leaving main's required 100% gate broken until #1883 recovered it. "Each piece works" and "the pieces together cover the original's scope" are different questions. - Compare content, not ancestry. main mixes squash and merge commits (last 200: 153 single-parent, 47 two-parent, counted directly), so `git merge-base --is-ancestor` gives false negatives for squashed deltas and false positives for reverted merge-commit deltas. - Never endorse a timeout or retry constant on a model-invocation path without reading docs/product-goal-directive.md section 8, which accepts more than two hours per model and states speed is not a core consideration. #1889/#1890/#1892 each capped a model step at 900s on real multi-hour-hang evidence and were all reverted (#1891, #1895). Every PR number, the section-8 quotes, the parent-count distribution, and the 100% gate values were verified against the repository directly. An earlier draft of the timeout bullet cited a section number that does not exist and attributed a sentence to that file which appears only in #1891's PR body; both were caught by grepping rather than trusting the summary that introduced them, and that failure is recorded in the text. Full suite: 2883 passed, 1 skipped. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…1904) * docs(items15-17): measure Detect changed scope gate-job runner waste Took jobs-per-PR as the metric for the 60-job ceiling complaint and measured a real baseline: one completed .github PR head produced 57 check runs across 2 attempts (~28/attempt), with "Detect changed scope" the most repeated job name at 5 per attempt. The obvious reading -- 5 duplicate gates, 5 wasted runners -- is wrong, and this entry records the corrected version. Whether a gate is waste depends on its consumer count: security-scan.yml amortizes one gate across 4 gated jobs (legitimate; self-gating would trade 1 runner for 4 redundant API calls), while sast-semgrep.yml and strix.yml each gate exactly one consumer, so each burns two runner allocations where one suffices. Real opportunity: 2 runner slots per PR, org-wide (both are ruleset-required workflows dispatched into ~74 repos). Records the load-bearing constraint any fix must preserve -- the ruleset ignores on: filters, so the job-level gate cannot become a trigger-level skip. Not fixed here: these are live org-wide required workflows and the org CI cannot complete runs at all right now, so the edit belongs in its own PR that can actually be validated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(items15-17): two echo-only jobs sit serially on the review critical path Extends the gate-job measurement with a second, larger finding. A peer session's read-only Codex pass spotted that opencode-review.yml's coverage-source-tree job does nothing but echo a string; verified here against origin/main, which shows the problem is bigger than one job. opencode-review.yml chains five jobs serially -- bootstrap -> admit-current-head -> coverage-source-tree -> coverage-evidence -> opencode-review-target -- and two of those links only print a sentence. A job is created only after its needs: predecessor finishes, so under queue saturation each link pays a full fresh queue wait. Quantified with this session's own item-13 audit data for naruon#1528 (run 33581213805): the two echo-only links waited ~9h40m and ~13h1m respectively, contributing roughly 22h41m of pure queue latency to one PR while holding the actual review behind them. Both jobs are load-bearing as reported branch-protection contexts and cannot simply be deleted, but their needs: edges are ordering, not data dependency -- neither produces an output the next consumes. Records the parallelization option and explicitly flags it as needing a check that nothing depends on these contexts completing in order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(items15-17): close the order-dependency question, add the skip guard A peer session independently re-pulled run 33581213805 and confirmed the serialization mechanism rather than inferring it: each job's created_at is exactly its predecessor's completed_at, so a job is not queued until its needs: predecessor finishes. Execution was 4 and 5 seconds against 9h40m and 13h1m of waiting. Closes the order-dependency question this entry left open: no test asserts the needs: chain order, the merge scheduler reads only a context name and its exact-head conclusion (CANONICAL_CHECK_NAME), and neither job declares outputs. Adds a safety condition the first draft missed: coverage-evidence has no if: of its own and is skipped only transitively via coverage-source-tree's admission guard, so cutting that edge without moving the guard would let a required context run on an unadmitted head. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(items15-17): warn that two files define these job names, one unsafe Two sessions independently reasoned about "the coverage jobs" without checking that the name resolves to two different jobs in two workflow files. opencode-review.yml (required, pull_request_target) holds the echo-only placeholders this entry analyses. opencode-review-dispatch.yml (privileged, repository_dispatch) defines jobs with the same names that do the real work: coverage-source-tree materializes the PR merge tree and uploads it as an artifact, coverage-evidence downloads that artifact and runs with a 300 minute timeout. There the edge is a hard data dependency, and cutting it would break coverage measurement outright. Caught by opening scripts/ci/test_strix_quick_gate.sh, whose assertions describe coverage-source-tree as materializing and uploading a merge tree -- contradicting "it only echoes" and exposing the second file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(items15-17): record the implementation and cross-family confirmation #1910 implements the fix, correctly scoped to opencode-review.yml only: five serial links to three, queue waits per PR from four to two, with the explicit admission if: carried onto coverage-evidence and coverage-evidence dropped from opencode-review-target's needs after confirming that job never reads the context at runtime. Adds the cross-family (Codex) reproduction of all three points, including the artifact name this record had not cited (opencode-coverage-source). Records the implementing session's own honest note: their change was safe because they scoped it narrowly, not because they had checked for the name collision. That generalizes better than the specific fix -- a job name is unique only within one workflow file, and the same name in another file can carry the opposite safety property. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…works (#1868) The .github-side follow-up this entry named as still-needed (call_llm not reading the HTTPError response body, so it couldn't surface served_model) shipped in #1831 (merged), hardened by #1835 and #1850. Found incidentally while handling an unrelated Autofix event on PR #1757: a fresh gateway failure now logs phase=response_error and a real model name (served_model=google/gemma-4-31b-it) instead of the old unknown/connecting pair. The underlying gateway instability (502 after 284.7s) is still a separate, open, recurring problem -- but the telemetry gap that made every prior instance of it undiagnosable is closed. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: opencode-agent[bot] <219766164+opencode-agent[bot]@users.noreply.github.com>
…merge-mechanics docs(agents): record test-gate regression and stale-PR merge mechanics
사용자가 명시한 Actions chicken-and-egg 복구 예외를 이 CI 수정에만 적용한다. 기존 main의 CI-only fixture 실패를 제거하고 같은 단일 품질 workflow에서 scheduler 변경의 trigger/selector 누락을 복구한다. 운영 코드, 의존성, 권한, 보안 검사와 보호 규칙은 변경하지 않는다. Exact head f7f11af: 일반/CI 각 2890 passed, 1 LLVM-host skip, 21 subtests. 지정 CI coverage 658 statements/222 branches 100%, 문서화 100%, actionlint/ShellCheck 통과. 독립 소스 검토 후 두 지적을 반영했다. Hosted required Checks는 대기 상태이며 GitHub 승인 완료나 일반 보호 병합으로 주장하지 않는다. #1899/#1900은 실제 main 채택 후 자체 exact-head 검증을 다시 수행해야 한다. 전체 41개 목표의 완료 증거가 아니다. Co-Authored-By: Codex <noreply@openai.com>
…all three consumers (#1929) (#1932) * fix(dispatch): accept a list of trusted dispatcher identities Two trusted workflows send the opencode-review repository_dispatch: opencode-review.yml through the OpenCode GitHub App (sender opencode-agent[bot], introduced by #1497) and pr-review-merge-scheduler.yml through its own token chain (sender github-actions[bot]). The authorization gate in opencode-review-dispatch.yml compared both actor and sender against a single-valued variable that still names only github-actions[bot], so every app-token dispatch has failed at the first job -- 9611 failures to 466 successes over the workflow's lifetime, and no open PR holds a successful review on its current head (#1929). Parse ALLOWED_DISPATCH_ACTOR as a comma-separated list, exactly as the adjacent ALLOWED_DISPATCH_TARGETS block already does. Semantics are preserved otherwise: actor and sender must both equal the SAME listed identity (a dispatch whose actor and sender are two different listed identities is still rejected), and an empty allowlist admits nothing. This change does not alter the variable. Which identities belong on the list is an authorization decision for the repository owner; this only makes the gate able to express more than one. A single-valued variable keeps working unchanged. Contract test extended in tests/test_opencode_agent_contract.py: both identities pass with a listed allowlist (whitespace around commas tolerated), an unlisted identity is rejected, and mismatched actor/sender is rejected. Negative control: the extended test fails against the unmodified gate on origin/main. REVIEW_DISPATCH_BLOB_SHA recomputed via git hash-object. Verified: 2890 passed, coverage 100%, interrogate 100%. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(dispatch): parse the actor allowlist identically in all three consumers vars.OPENCODE_REPOSITORY_DISPATCH_ACTOR is read by three workflows, and the first commit widened only one of them: opencode-review-dispatch.yml:127 covered by the previous commit codeql-scan-dispatch.yml:155 byte-identical gate, was still exact-match pr-review-fix-scheduler.yml:156 same three conditions, different error line Left as-is, codeql-scan-dispatch would keep rejecting the App identity once #1925's toJSON fix lets it reach line 155, and the scheduler would too. Three consumers of one variable with two parsers is the next drift, so all three now run the same comma-separated parse with the same semantics: actor and sender must both equal the SAME listed identity, empty list admits nothing, single value unchanged. The scheduler keeps its own error line. Tests extended in place for both: a listed identity passes (whitespace around commas tolerated), an unlisted one is rejected, and actor/sender that are two different listed identities are rejected. Negative control: both extended tests fail against the unmodified gates on origin/main. The codeql helper creates tmp_path/bin, so each invocation gets its own subdirectory. No blob-SHA pin references either newly edited workflow. No open PR touches either gate: #1926 covers codeql-scan-dispatch 146-152 and #1741 covers pr-review-fix-scheduler 210-217. Verified: 2891 passed, coverage 100%, interrogate 100%. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
* fix(codeql): serialise the dispatched scan matrix with toJSON codeql-pr.yml sends client_payload.matrix as an array, and the dispatch handler assigned it straight into env:, where a value must be a scalar. GitHub rejects the step with "A sequence was not expected", so the step dies before running any of its script and the dispatched scan is skipped. The handler has 0 successes against 136 failures since #1776 added it. The validate step already reads the value through jq and checks `type == "array" and length == 1`, so JSON text is what it was written to consume; no consumer changes. Dropping the `|| ''` fallback is safe because an absent matrix yields the string "null", which fails the same array check and reaches the existing error path. Neither yaml.safe_load nor actionlint 1.7.12 reports this file as invalid -- it is an Actions template rule rather than YAML syntax, so only GitHub's validator rejects it and no local gate catches the class. The added string contract test is therefore the only guard that runs before a dispatch does. Refs #1925 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(codeql): the matrix env: failure is step-scoped, not a whole-file rejection The validate-dispatch job does get a runner and its first steps run; GitHub rejects only the step whose env: receives the array, when that env: is evaluated. Docstring wording corrected to match the observed job timeline. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit f90c23b3c0386e22528cc1ab8680c4a31fa630b9) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: opencode-agent[bot] <219766164+opencode-agent[bot]@users.noreply.github.com>
…cks are in flight (#1937) A scheduler run that executes after a 2h+ queue wait finds its scanned PR behind main and merges main into the head before any review is dispatched (inspect_pr, pre-review path). That push cancels every queued check on the old head (22/28 on #1926, 21/30 on #1484) and requeues the PR at the back, so under a saturated queue no head ever finishes its checks: 76 of the 77 PRs merged since 2026-09-04 had 0/12 required contexts satisfied at merge. has_in_flight_check_runs() reuses latest_check_runs()/running_check_state(); the pre-review path now decides "wait" while any newest current-head check run is still queued or running. No age cap on purpose: a cap would restart the loop. The post-approval update path is unchanged (main is strict=true). CLAUDE.md described only the post-approval update; it now names both paths. Refs #1935 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…1939) build_zdr_prioritized_catalog sorted eligible routes by (cost, ZDR, provider, model) and filled the bounded catalog in that order, taking up to account_cap per account. With the sidecar's ORCHESTRATOR_CATALOG_ ACCOUNT_CAP=8 and ORCHESTRATOR_CATALOG_LIMIT=12 the fill took 8 nvidia_nim + 4 nvidia_nim_sub and stopped before the alphabetically last account: noema-review run 33969842312 admitted 62 free routes across three accounts (free_account_diversity 3) and served 12 NVIDIA routes, of which runtime preflight kept 2, so a stalled NVIDIA endpoint had no other account to fail over to (contextual-orchestrator#1045). Keep the sort; group the sorted rows by (cost, ZDR) tier and fill each tier round-robin across provider accounts until limit, honouring account_cap. Tier order, cap, limit and discovery-order independence are unchanged; the same input now yields 4 + 4 + 4. The launcher's evidence_only filter (#1476) is not the cause on the current pin (2e414d15 includes contextual-orchestrator#949), so that PR stays a complementary hardening. Tests: three new cases (interleave within tier; ZDR tier still first; exhausted accounts hand turns over) were RED against the old loop (3 failed) and are GREEN now. Gate on this tree: 2896 passed, coverage 100%, interrogate 100%. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…ew sidecar's stderr (#1943) The review sidecar never configured its orchestrator process's logging, so it ran at Python's default WARNING while contextual_orchestrator logs every provider attempt, classified failure, backoff and circuit event at DEBUG. A 3122 s noema-review 502 on 2026-09-05 (.github#1940) could only be attributed by reading source: six preflight-ready routes, two retry layers (TaskOrchestrator.tool_retry_attempts=1 over ModelClient max_retries=2 at a 90 s per-recv timeout), about 548 s per hop. The launcher now calls the vendored debug_logging.configure_logging before serving, DEBUG by default and overridable through ORCHESTRATOR_SIDECAR_LOG_LEVEL, and gives every root handler a timestamped format so per-hop durations can be read off the trace. At the vendored pin no DEBUG site logs a prompt, payload or response body; the only free-text field (provider_attempt_failed's error_message) is kept out of CI evidence by the sidecar's allow-list sanitizer, which a companion change extends to admit these templates and upload the file. Tests: level default, override, level-and-format application, invalid level -> SystemExit, and a source-order contract that logging is configured before credential work; all five fail against main's launcher. Gate on this tree: 2901 passed, coverage 100%, interrogate 100% (the final commit differs from the gated tree by a docstring only; the touched test module and interrogate were re-run on the final text). Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…rcuit events (#1945) * fix(sidecar): let the stream sanitizer pass orchestrator route and circuit events Every provider_*/circuit_* line from the orchestrator was folded into omitted_unstructured_lines, so even the provider_exhausted WARNING that fires today never reached an artifact. Admit those templates field by field against bounded charsets, cut provider_attempt_failed before its free-text error_message, and accept both the default and the sidecar formatter log prefixes (keeping the timestamp for durations). Companion to #1943 and #1944. Refs #1935, #1939 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(sidecar): accept float circuit counters and pin the real formatter output The orchestrator's circuit counters are floats (failures 0.0 += 1.0, circuit_reset_seconds 30.0), so the lines that reach stderr say failures=2.0 / reset_seconds=30.0; the integer-only pattern rejected both circuit_failure and circuit_opened. Found by rendering the templates through the sidecar's logging.Formatter, which the new test now does for all ten. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…erdict phase fails (#1944) A failed noema-review run left artifacts=0, so a 3122 s walk across six ready routes ending in HTTP 502 (run 33981136873) was diagnosable only from the caller's one-line summary. Ship the sanitized sidecar stderr and the preflight report on failure, using the same pinned upload-artifact and the same file Strix already publishes in strix-reports. Refs #1935, #1939 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Four conflicts, including one modify/delete, plus one order-dependent defect this merge surfaced. THE PERF CHANGE IS ALREADY ON MAIN. The label_section conflict in opencode_review_normalize_output.py is the same algorithm on both sides - main's copy differs only by naming the index 'idx' and carrying a Bolt comment - so main's side was taken and nothing is lost. MODIFY/DELETE, relocated rather than dropped: main deleted .github/workflows/organization-commercial-readiness-loop-quality-ci.yml in ec4521f 'ci(actions): consolidate commercial readiness quality', folding it into agent-review-runtime-quality-ci.yml. This branch had modified the deleted file to cover the two modules its split creates. Dropping that with the file would have left the coordinator ungated: loop.py is now a 54-line facade over a 892-line core plus a 274-line contract module, and the surviving gate measured only 'scripts/ci/organization_commercial_readiness_loop.py'. The same four extensions were applied to the consolidated workflow - path trigger, suite selection case, coverage --include glob, compileall list - and verified by running that job's own command: core 386/386, ddd_contract 195/195, facade 30/30, TOTAL 100%. Three contract tests pinned the literal include path and were updated to the glob with the reason in a comment (test_workflow_and_doctoring_contracts, test_commercial_readiness_suite_is_ selected_and_conditionally_executed, test_organization_loop_contract_moves_to_ agent_quality_job). Negative control: restoring the literal include makes test_workflow_and_doctoring_contracts fail by name. DEFECT FOUND AND FIXED - facade import identity: test_organization_commercial_readiness_loop_facade.py passed alone and failed in the full suite. Bisected to tests/test_codeql_default_setup_rollout.py, which prepends scripts/ci to sys.path and imports the module under its bare top-level name. Probed the resulting state directly: both sys.modules['scripts.ci.organization_commercial_readiness_loop'] and sys.modules['organization_commercial_readiness_loop'] are the SAME object and its __spec__.name is the top-level one, so runpy.run_module on the package name raises 'loader for organization_commercial_readiness_loop cannot handle scripts.ci.organization_commercial_readiness_loop'. The facade restamped _core's identity unconditionally on every import, letting the second public name overwrite the first. Guarded so only the first adopter stamps it - the same fix this repository already applied for this shape in #1545. Verified in both orders. This is the branch's facade meeting main's larger suite: the branch head passes because main's trigger test is not in its tree. Evidence: - uvx ruff check --select F821 scripts/ci tests: All checks passed - full suite, branch head 0def341 (unmerged): 2243 passed, 0 failed - full suite, this merge: 2922 passed, 0 failed - coverage: TOTAL 100%; interrogate: PASSED (minimum 100.0%) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
main 병합 완료 (
|
This was referenced Sep 6, 2026
Merged
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head mergeability evidence and changed-file flow before approval, then found merge conflicts on the affected path.
Findings
1. HIGH Merge Conflict Guidance - Resolve the PR branch against the latest base branch
- Problem: GitHub reports mergeStateStatus
DIRTYfor this pull request. - Root cause: Branch
perf/normalize-label-section-boundscannot be merged cleanly intofix/hourly-ddd-development-contract-20260901; the changed-file flow below shows which review/runtime path is blocked by the conflict. - Fix: Merge or rebase the latest
fix/hourly-ddd-development-contract-20260901intoperf/normalize-label-section-bounds, resolve conflict markers in the PR branch, rerun the focused checks, and push the same branch. - Repair commands:
gh pr checkout 1555 --repo ContextualWisdomLab/.github
git fetch origin fix/hourly-ddd-development-contract-20260901
git merge --no-ff origin/fix/hourly-ddd-development-contract-20260901 # or: git rebase origin/fix/hourly-ddd-development-contract-20260901
git status --short
# resolve files, then git add <resolved-files>
# merge path: git commit
# rebase path: git rebase --continue
git push origin HEAD:perf/normalize-label-section-bounds
# rebase path only: git push --force-with-lease origin HEAD:perf/normalize-label-section-bounds- Regression test: Keep OpenCode approval gated on mergeability so model-output failures cannot approve a conflicted PR.
Merge Conflict Evidence Map
flowchart LR
Evidence["OpenCode evidence"] --> Review["Current PR review path"]
Review --> Verify["Required checks"]
- Result: REQUEST_CHANGES
- Reason: mergeStateStatus is
DIRTY; mergeable isCONFLICTING. - Head SHA:
96d2600e2342da0fc725a0c2d43db725927a5592 - Workflow run: 33999842423
- Workflow attempt: 1
Changed-File Evidence Map
flowchart LR
Evidence["OpenCode evidence"] --> Review["Current PR review path"]
Review --> Verify["Required checks"]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Outcome
Bound each verification-label candidate search to the earliest section endpoint already found. This preserves duplicate-label and
docstring coverage:suffix handling while avoiding scans beyond a boundary that cannot affect the result.Scope
This clean salvage contains only:
label_sectioncandidate-bound search optimizationIt intentionally excludes the stale Noema, scheduler, workflow, product-gap, and global Bolt-rule changes mixed into #1522.
Dependency
This is a stacked PR whose base is
fix/hourly-ddd-development-contract-20260901(PR #1545). Do not merge it before #1545. After #1545 reaches protectedmain, retarget this PR tomainand incorporate any newly advanced default-branch history by a normal merge, never a force push.Protected
mainadvanced to5686de41660d51a7a7f22b8840dfa6ccfe5ff3f1while this stack was being prepared; it is intentionally not mixed into this focused stacked diff before #1545 is updated/merged.Verification
git diff --check: passA separate deterministic audit also compared the optimized and prior implementations over 1,000 generated texts across all 23 labels with identical results. A long-text microbenchmark measured 9.587 seconds before and 1.208 seconds after for the same workload.