diff --git a/.github/workflows/deploy-pages-input-security-ci.yml b/.github/workflows/deploy-pages-input-security-ci.yml new file mode 100644 index 0000000000..e3618432da --- /dev/null +++ b/.github/workflows/deploy-pages-input-security-ci.yml @@ -0,0 +1,46 @@ +name: Deploy Pages Input Security CI + +on: + pull_request: + branches: [main] + paths: + - ".github/workflows/deploy-pages.yml" + - ".github/workflows/deploy-pages-input-security-ci.yml" + - "tests/test_deploy_pages_input_shell_boundary.py" + +permissions: + contents: read + +concurrency: + group: deploy-pages-input-security-${{ github.repository }}-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + pages_input_shell_boundary: + name: pages-input-shell-boundary + runs-on: ubuntu-24.04 + timeout-minutes: 5 + steps: + - name: Harden runner + uses: step-security/harden-runner@bf7454d06d71f1098171f2acdf0cd4708d7b5920 # v2.20.0 + with: + egress-policy: audit + + - name: Checkout exact pull request head + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ github.event.pull_request.head.sha }} + fetch-depth: 1 + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: "3.14" + + - name: Verify exact-head Pages shell-input boundary + shell: bash --noprofile --norc -e -o pipefail {0} + run: | + test "$(git rev-parse HEAD)" = "${{ github.event.pull_request.head.sha }}" + python -m unittest -q tests/test_deploy_pages_input_shell_boundary.py + python -m compileall -q tests/test_deploy_pages_input_shell_boundary.py diff --git a/.github/workflows/deploy-pages.yml b/.github/workflows/deploy-pages.yml index f86b614022..a799281f93 100644 --- a/.github/workflows/deploy-pages.yml +++ b/.github/workflows/deploy-pages.yml @@ -100,13 +100,21 @@ jobs: fi fi + # Caller inputs reach the shell through env, never through ${{ }} + # interpolation into the script body: a project name containing shell + # metacharacters would otherwise execute here. Same defect class that + # Semgrep's run-shell-injection rule flags elsewhere in this repo. - name: Summary if: always() + env: + PROJECT_NAME: ${{ inputs.project_name }} + BUILD_DIR: ${{ inputs.build_dir }} + CUSTOM_DOMAIN: ${{ inputs.custom_domain }} run: | { echo "## Cloudflare Pages deploy" echo "" - echo "- **Project:** \`${{ inputs.project_name }}\`" - echo "- **Build dir:** \`${{ inputs.build_dir }}\`" - echo "- **Custom domain:** \`${{ inputs.custom_domain || '(none)' }}\`" + echo "- **Project:** \`${PROJECT_NAME}\`" + echo "- **Build dir:** \`${BUILD_DIR}\`" + echo "- **Custom domain:** \`${CUSTOM_DOMAIN:-(none)}\`" } >> "$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index d98a72e605..3815b07353 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -32,12 +32,14 @@ on: review_dispatch_limit: description: OpenCode/Strix review dispatch budget per scheduler run (-1 dispatches every eligible current-head review) required: false - default: "1" + type: string + admission_dispatch_budget: + description: Review-admission dispatch budget per scheduler run (-1 disables the per-run admission cap) + required: false type: string branch_update_limit: description: Branch update budget per scheduler run (-1 updates every eligible outdated branch) required: false - default: "1" type: string enable_auto_merge: description: Enable auto-merge for current-head approved PRs @@ -132,9 +134,9 @@ jobs: PROJECT_FLOW_INPUT: ${{ github.event.client_payload.project_flow || inputs.project_flow || vars.PROJECT_FLOW || '' }} PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number || github.event.client_payload.pr_number || inputs.pr_number || '' }} TRIGGER_REVIEWS: ${{ github.event_name == 'schedule' || github.event_name == 'push' || github.event_name == 'pull_request_target' || github.event_name == 'pull_request_review' || (github.event_name == 'repository_dispatch' && github.event.client_payload.trigger_reviews != false) || inputs.trigger_reviews == true }} - REVIEW_DISPATCH_LIMIT_INPUT: ${{ github.event.client_payload.review_dispatch_limit || inputs.review_dispatch_limit || vars.REVIEW_DISPATCH_LIMIT || '1' }} - REVIEW_ADMISSION_DISPATCH_BUDGET: ${{ vars.REVIEW_ADMISSION_DISPATCH_BUDGET || '1' }} - BRANCH_UPDATE_LIMIT_INPUT: ${{ github.event.client_payload.branch_update_limit || inputs.branch_update_limit || vars.BRANCH_UPDATE_LIMIT || '1' }} + REVIEW_DISPATCH_LIMIT_INPUT: ${{ format('{0}', github.event.client_payload.review_dispatch_limit) || inputs.review_dispatch_limit || vars.REVIEW_DISPATCH_LIMIT || '' }} + REVIEW_ADMISSION_DISPATCH_BUDGET: ${{ format('{0}', github.event.client_payload.admission_dispatch_budget) || inputs.admission_dispatch_budget || vars.REVIEW_ADMISSION_DISPATCH_BUDGET || '' }} + BRANCH_UPDATE_LIMIT_INPUT: ${{ format('{0}', github.event.client_payload.branch_update_limit) || inputs.branch_update_limit || vars.BRANCH_UPDATE_LIMIT || '' }} ENABLE_AUTO_MERGE: ${{ github.event_name == 'schedule' || github.event_name == 'push' || github.event_name == 'pull_request_target' || (github.event_name == 'repository_dispatch' && github.event.client_payload.enable_auto_merge != false) || inputs.enable_auto_merge == true }} MERGE_MODE: ${{ github.event.client_payload.merge_mode || inputs.merge_mode || vars.PR_MERGE_MODE || 'direct_or_auto' }} UPDATE_BRANCHES: ${{ github.event_name == 'schedule' || github.event_name == 'push' || github.event_name == 'pull_request_target' || (github.event_name == 'repository_dispatch' && github.event.client_payload.update_branches != false) || inputs.update_branches == true }} @@ -527,12 +529,24 @@ jobs: fi review_dispatch_limit="$REVIEW_DISPATCH_LIMIT_INPUT" if [ -z "$review_dispatch_limit" ]; then - review_dispatch_limit="-1" + echo "::error::REVIEW_DISPATCH_LIMIT must be explicitly configured" >&2 + exit 1 fi branch_update_limit="$BRANCH_UPDATE_LIMIT_INPUT" if [ -z "$branch_update_limit" ]; then - branch_update_limit="1" + echo "::error::BRANCH_UPDATE_LIMIT must be explicitly configured" >&2 + exit 1 + fi + admission_dispatch_budget="$REVIEW_ADMISSION_DISPATCH_BUDGET" + if [ -z "$admission_dispatch_budget" ]; then + echo "::error::REVIEW_ADMISSION_DISPATCH_BUDGET must be explicitly configured" >&2 + exit 1 fi + printf 'scheduler_effective_limits event=%s review_dispatch=%s branch_update=%s admission=%s\n' \ + "${GITHUB_EVENT_NAME}" \ + "$review_dispatch_limit" \ + "$branch_update_limit" \ + "$admission_dispatch_budget" args=( --repo "$TARGET_REPOSITORY" --base-branch "$TARGET_DEFAULT_BRANCH" @@ -541,7 +555,7 @@ jobs: --review-workflow "Required OpenCode Review" --review-dispatch-limit "$review_dispatch_limit" --admission-state-path "${RUNNER_TEMP}/review-admission/state.json" - --admission-dispatch-budget "$REVIEW_ADMISSION_DISPATCH_BUDGET" + --admission-dispatch-budget "$admission_dispatch_budget" --admission-sequence "$GITHUB_RUN_ID" --branch-update-limit "$branch_update_limit" --stale-opencode-minutes "$STALE_OPENCODE_MINUTES" diff --git a/CHANGELOG.md b/CHANGELOG.md index 34281625cb..a5f05476a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,15 @@ +### Merge-scheduler preserves explicit zero and unlimited mutation budgets + +- Repository-dispatch payloads for review dispatch, admission dispatch, and branch update now stringify the supplied value before fallback selection. GitHub Actions treats numeric `0` as falsy, so the prior `payload || input || repository variable` expression could replace an explicit zero with a positive repository variable and authorize mutation the caller denied. The admission controllers also preserve explicit `-1` unlimited authority while rejecting values below `-1`. Contract coverage pins numeric-zero and unlimited behavior at the workflow and owner modules. Refs #2267. + +### Strix isolated fixtures preserve the complete evidence-binding runtime + +- Runtime Quality runs `35445211402` (`#2272`) and `35448837045` (`#2109`) failed with the same first causal error: isolated Strix fixtures copied the gate and model helpers but omitted `strix_evidence_binding.py`. The first attempted repair then truncated the 13,138-line shell contract, its Python regression, CHANGELOG, and product-gap baseline. This ordinary-forward repair restores those four authorities, adopts protected `main` as a second parent, and adds the binder beside the model helper in all 25 isolated fixture runtimes. A source-first regression now requires the complete 25/25 runtime closure. + +### SAST successor restores lost Pages evidence and inherits redirect authority + +- `.github#2272` was briefly force-moved from `4967d66f` to sibling `1ca50644`, dropping the dedicated Pages caller-input security workflow and its executable regression. Before this repair published, a second concurrent rewrite produced `e0b6e70f` with `4967d66f` restored as an ancestor. Ordinary merge `3923b196` keeps that complete current lineage as first parent and stacks the canonical GitHub REST redirect-authority successor `.github#2279@9c19c6e` as second parent. The resulting Draft preserves the Pages `env` shell boundary, its exact-head hosted test, both initial-origin regressions, and the production no-redirect opener/source/tests without another Force Push, scanner suppression, or gate weakening. + ### Noema transport capacity schedules a bounded continuation re-dispatch - After gateway failover, HTTP 429/5xx no longer end only as a permanent required-check failure with `caller attempts=1`. ADR-0031 classifies that class as `provider_capacity_unavailable`, keeps the single gateway request per job, surfaces `provider_attempt_count` from the orchestrator error envelope, and authorizes at most two same-head `repository_dispatch` retries after a capped `Retry-After` or deterministic 60–180 s jitter. Review is never skipped. Refs #2165. @@ -96,6 +108,7 @@ - Raised `hourly-review-repair.yml`'s discovery ceiling from 50 to 200 while rotating deterministic 50-PR deep-inspection windows by hourly run number. The scheduler hydrates only the selected window and stops immediately after its single dispatch, preserving access to newer PRs without quadrupling expensive review/check/comment work. See `docs/doctoring/hourly-review-repair-single-file-consolidation.md`'s 2026-09-03 follow-up. ## [Unreleased] +- **Fail closed when scheduler mutation budgets have no operator authority.** Merge-scheduler recovery keeps its structured outdated-head taxonomy and update/dispatch fallthrough, but no longer derives review, branch-update, or admission limits from one observed backlog. Each budget must be supplied explicitly by dispatch/input/repository variable; absent authority stops before PR mutation. - **Bind GitHub REST redirect evidence to both production opener chains.** `.github#2279` now feeds a synthetic same-authority 302 through the CodeQL identity and Strix evidence clients' real module-level openers, proving the redirect target is never contacted and the bearer header is never forwarded. Removing `_RejectRedirects` from either opener makes the contract fail on the forbidden second request. Four stale Strix HTTP/transport/JSON fixtures now patch that same production seam; direct handler unit cases and standalone CodeQL materialization remain unchanged. - **Define an evidence-backed repository README quality standard.** Added `docs/repository-readme-quality-standard.md` as the shared review contract for product-first structure, code-current onboarding, authority boundaries, durable quality signals, and repository/source/dependency license due diligence. Product repositories continue to own their own README prose; the standard is linked from the root documentation map and does not centralize or generate product claims. - Include merge-scheduler entrypoint, core, and regression-test changes in diff --git a/CLAUDE.md b/CLAUDE.md index 7dde78f5d2..4da24c6a2d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,7 +51,10 @@ carry an independent proof and source-line digest; it never invents observed results. The scheduler updates a PR branch in two cases: after approval, when no current-head check has failed and GitHub reports the PR as behind; and before review dispatch, when the PR is behind and no current-head check is still queued or running (an in-flight check is evidence the update would -discard; see #1935). The mechanical merge scheduler itself never synthesizes a fix: it gives `DIRTY`/`CONFLICTING` +discard; see #1935). Daily `schedule` recovery is the exception: it may update (or, if the update +budget is exhausted, dispatch review on) an outdated OpenCode-needing head despite in-flight checks, +with a loud warning, so recovery is not inert under queue saturation (see +`docs/doctoring/schedule-recovery-inflight-update-bypass-20260918.md`). The mechanical merge scheduler itself never synthesizes a fix: it gives `DIRTY`/`CONFLICTING` PRs repair guidance. A separate edit-capable autofix flow (`scripts/ci/pr_review_fix_scheduler.py` → `.github/workflows/pr-review-autofix.yml`) may, for an approved same-repository-head PR, merge the base into the head and resolve the conflict markers; the diff --git a/docs/doctoring/merge-scheduler-review-dispatch-budget-20260918.md b/docs/doctoring/merge-scheduler-review-dispatch-budget-20260918.md new file mode 100644 index 0000000000..2329e08495 --- /dev/null +++ b/docs/doctoring/merge-scheduler-review-dispatch-budget-20260918.md @@ -0,0 +1,65 @@ +# Doctoring record: merge-scheduler `REVIEW_DISPATCH_LIMIT` throughput shaping (2026-09-18) + +- **Date:** 2026-09-18 +- **Subject:** Cap per-run OpenCode/Strix review dispatch budget so the merge + scheduler shapes throughput under the org Actions plan ceiling, without + disabling review dispatch. +- **Decision record:** operational var change plus fail-closed validation. The + measured repo variable remains the authority; unset or blank has no invented fallback. +- **PR:** this commit's pull request. + +## What changed (operational + durable) + +| Lever | Before | After | +|---|---|---| +| Repo var `REVIEW_DISPATCH_LIMIT` | **4** | **1** (lead set at `2026-09-18T06:19:47Z` for immediate effect) | +| `workflow_call` input `review_dispatch_limit` default | `"1"` | none; caller or repo variable must provide authority | +| Shell empty fallback in `pr-review-merge-scheduler.yml` | `-1` (unlimited) | fail closed with an explicit configuration error | +| Explicit input/var value `-1` | unlimited | still unlimited when set deliberately | + +The same authority rule applies to this scheduler's sibling mutation budgets: +`REVIEW_ADMISSION_DISPATCH_BUDGET` and `BRANCH_UPDATE_LIMIT` must come from an +explicit dispatch payload, reusable-workflow input, or repository variable. +They have no inferred trigger-specific fallback. `ORG_SWEEP_REVIEW_DISPATCH_LIMIT` +and fix-scheduler `MAX_DISPATCHES` remain separate operational authorities and +are not reused as fallbacks for this scheduler. + +## Why 4 → 1 (not disable) + +Each ruleset-injected merge-scheduler run can fan out up to `REVIEW_DISPATCH_LIMIT` +AI review dispatches (OpenCode / Strix / related). At **4**, concurrent scheduler +runs across repositories multiply that fan-out against an org concurrent-job +ceiling of roughly **60** (`docs/doctoring/actions-plan-concurrency-ceiling-20260903.md`). + +Live queue snapshot before the var change (path +`~/.local/orca-watchdog/queue-before-061947.json`, measured +`2026-09-18T10:20:32Z` UTC — note the filename marks the earlier operational +cutover `061947Z`): + +| Signal | Value | +|---|---| +| Org `queued` (sample) | ≈390 | +| Org `in_progress` | ≈17 | +| `.github` eligible non-draft unapproved proxy | ≈31 | +| Queued OpenCode Dispatch | ≈118 | +| Then-current `REVIEW_DISPATCH_LIMIT` | 4 | + +This is **throughput shaping**, not a kill switch: reviews still dispatch up to +the explicitly configured current-head budget. Work continues; only the +configured per-run burst width changes. `cancel-in-progress` concurrency +is already correct and was not touched. No age-based cancel. If the configured +variable is absent, dispatch stops with a configuration error rather than +silently choosing a rule-of-thumb budget. + +## Reversibility + +Raise the repo variable (or pass explicit `workflow_call` / +`repository_dispatch` values) to restore wider fan-out. Explicit **`-1`** +retains each command's documented unlimited behavior. Leaving any of the three +mutation authorities unset or blank now fails closed; it does not infer **1**, +**8/20/8**, unlimited, or any other decision-affecting budget. + +## Out of scope + +Trigger-narrowing and `ready_for_review` / synchronize deferral for AI-review +workflows are a separate thread and must not be reopened here. diff --git a/docs/doctoring/schedule-recovery-inflight-update-bypass-20260918.md b/docs/doctoring/schedule-recovery-inflight-update-bypass-20260918.md new file mode 100644 index 0000000000..217bc48d6b --- /dev/null +++ b/docs/doctoring/schedule-recovery-inflight-update-bypass-20260918.md @@ -0,0 +1,65 @@ +# Doctoring record: schedule recovery bypasses #1935 in-flight hold (2026-09-18) + +- **Date:** 2026-09-18 +- **Subject:** Daily merge-scheduler recovery (`GITHUB_EVENT_NAME=schedule`) was + inert under Actions queue saturation: OpenCode-needing heads were behind, so + review could not dispatch, and `#1935`'s in-flight check hold blocked + `update_branch`, so recovery neither updated nor dispatched — yet exited + success. Same class as `fmls#2006`. +- **Holds:** ContextualWisdomLab/.github#2267 (fail-loud taxonomy + effective + limits) until this dispatch path is proven; do not merge on taxonomy alone. + +## Measured evidence (cron run 35202348887, 2026-09-17) + +| Observation | Evidence | +|---|---| +| Reviews were triggered | Log: `TRIGGER_REVIEWS: true` | +| Budgets were non-zero | `REVIEW_DISPATCH_LIMIT_INPUT: 4`, `REVIEW_ADMISSION_DISPATCH_BUDGET: 1` | +| Local schedule scan ran | 100 decisions; not a targeted-dispatch reject | +| OpenCode-needing heads blocked on freshness | PR #834 `update_branch` (no in-flight); PRs #1198, #1215, #1238, #1519 `wait` with "outdated before review dispatch, but current-head checks are still queued or running" | +| Counts | `update_branch=1`, `wait=21`, **no `review_dispatch` key** (`dispatched=0`) | +| Contrast when head is current | Prior cron 35076102529: `PR #1519: review_dispatch: ... OpenCode dispatched` | + +Root cause is a **definition / pre-dispatch filter mismatch**, not a zero budget +or a false `TRIGGER_REVIEWS`. The five OpenCode-needing heads were all outdated +before review dispatch; four were soft-held by `#1935`. + +## Repair (smaller policy change) + +On `GITHUB_EVENT_NAME=schedule` only, when an OpenCode-needing head is behind +and the only blocker is `#1935`'s in-flight check hold: + +1. **Prefer `update_branch`** despite queued/running current-head checks, with a + loud `::warning` citing the `#1935` tradeoff (discard in-flight evidence so + daily recovery is not inert). +2. If the branch-update budget is already exhausted on that schedule tick, + **fall through to `review_dispatch` / `security_dispatch`** on the behind + head with an explicit warning, rather than soft-idle. + +Event-driven paths (`pull_request_target`, `workflow_run`, …) keep the `#1935` +hold unchanged. + +Companion observability (same PR #2267): `scheduler_effective_limits` prints +the live review-dispatch / branch-update / admission values every run; +`classify_review_recovery` / `emit_review_recovery_signal` fail loud when +schedule recovery finds outdated OpenCode-needing heads and still produces +neither update nor dispatch. + +The measured backlog does not itself authorize a numeric mutation budget. +PR #2270 is integrated as the single-writer authority contract: review +dispatch, branch update, and review admission each require an explicit +dispatch payload, reusable-workflow input, or repository variable. Missing or +blank authority fails before the scheduler mutates a pull request. The observed +`5` review-needing and approximately `18` outdated heads remain operational +evidence, not a rule that rounds itself into `8/20/8` or an event default. + +## Audit trail + +- Cron logs for `35202348887` and `35076102529` (Daily Review Recovery). +- `#1935` hold rationale in `CHANGELOG.md` / `inspect_pr` comment. +- Implementation: `scripts/ci/pr_review_merge_scheduler_core.py` schedule + branch of the outdated-before-review path; tests in + `tests/test_pr_review_merge_scheduler.py`. +- Budget authority: `docs/doctoring/merge-scheduler-review-dispatch-budget-20260918.md` + and the workflow contract tests. Operators configure all three mutation + budgets before using this recovery path. diff --git a/docs/doctoring/strix-evidence-binding-2159-2168.md b/docs/doctoring/strix-evidence-binding-2159-2168.md index 2e6151a8ce..1dc9230dab 100644 --- a/docs/doctoring/strix-evidence-binding-2159-2168.md +++ b/docs/doctoring/strix-evidence-binding-2159-2168.md @@ -44,7 +44,37 @@ apply_patch-miss RED fixtures. Gate wiring is pinned by fail-closed evidence binder; do not restore false PR-delta attribution or false remediation claims. +## Fixture runtime closure RCA (2026-09-20) + +Agent Review Runtime Quality run `35445211402`, job `105902856459`, checked out +`.github#2272@cd3b41b8`; run `35448837045`, job `105912348418`, later reproduced +the same failure on `.github#2109@db84349c`; and run `35448347210`, job +`105911090486`, reproduced it on `.github#2267@3eb0a5c2` with 527 cascades. +In all three logs the first causal +message is `ERROR: Strix evidence binder is missing`. The shell self-test copied +`strix_quick_gate.sh` and `strix_model_utils.sh` into isolated repositories but +not the binder the gate executes, so ordinary success, retry, provider-failure, +scope, and remediation fixtures collapsed into hundreds of exit-code and output +assertions. + +The first attempted repair was not valid evidence. Commit `857e7882` cut +`tests/test_strix_evidence_binding.py` at the token `exce`; `89cee557` replaced +the 13,138-line shell contract with 675 lines; and `1eb03c7a` deleted 4,176 +lines from CHANGELOG and the product-gap authority. The claimed `37 passed` +could not be reproduced from that exact tree because the Python file did not +compile. Those commits remain in ancestry for auditability and are restored +ordinary-forward after adopting protected `main`; no force update or destructive +rebase is used. + +The corrected RED is `tests/test_strix_fixture_runtime_closure.py`: the broken +head had zero of the 25 model-helper fixture copies and failed `0 == 25`; after +restoring the complete harness it proved the precise residual defect, 25 model +helpers versus zero binders. GREEN adds the binder alongside each model helper, +leaving production gate behavior unchanged. Hosted acceptance and downstream +adoption remain separate current-head gates. + ## References - ContextualWisdomLab/.github#2159 - ContextualWisdomLab/.github#2168 +- ContextualWisdomLab/.github#2272 diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index c617e3ad73..b7a1c92102 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -7,10 +7,17 @@ 이 문서는 제품·기술·운영 Gap을 현재 문서와 현재 GitHub 상태에 묶어 두는 기준선이다. 새 작업은 먼저 이 문서의 Gap ID를 PR 설명과 테스트 증거에 연결하고, PR의 정확한 exact HEAD·Checks·리뷰를 다시 수집한 뒤 구현한다. 표의 상태는 작성 시점의 관측값이므로, 병합 판단에는 재사용하지 않는다. 이 인벤토리는 스냅샷이며 merge authorization이 아니다. +### 2026-09-19 exact-head incident delta + +| Gap ID | 상태 | exact-head evidence | causal owner / next gate | +|---|---|---|---| +| CONTROL-SCHEDULER-ZERO-BUDGET-01 | **Proposed — source RED/GREEN on `.github#2267@f275c57c2`; hosted acceptance pending** | Current-head review thread `PRRT_kwDOS_C14s6j-5oP` proved that GitHub Actions evaluates numeric `0` as falsy, so each `repository_dispatch` mutation budget could fall through to a positive repository variable. Concurrent RED `c8704966d` covers numeric-zero and explicit `-1`; GREEN `f275c57c2` repairs the workflow and both admission owners. | Canonical owners are `.github/workflows/pr-review-merge-scheduler.yml`, `scripts/ci/pr_review_merge_scheduler_core.py`, and `scripts/ci/review_admission_controller.py`. Numeric zero forbids mutation, `-1` remains explicit unlimited authority, values below `-1` fail closed, and fresh exact-head Checks plus independent approval remain required before merge. | + ### 2026-09-13 current-head incident delta | Gap ID | 상태 | exact-head evidence | causal owner / next gate | |---|---|---|---| +| CONTROL-STRIX-FIXTURE-RUNTIME-01 | **RED reproduced on three exact heads; corrected source repair pending hosted evidence** | `.github#2272@cd3b41b8` Runtime Quality run `35445211402`, job `105902856459`; `.github#2109@db84349c` run `35448837045`, job `105912348418`; and `.github#2267@3eb0a5c2` run `35448347210`, job `105911090486` all first fail because isolated fixtures omit `scripts/ci/strix_evidence_binding.py`, followed by hundreds of exit-code/assertion cascades (`#2267`: 527 failures). The first `#2272` repair commits `857e7882`, `89cee557`, and `1eb03c7a` instead truncated four authority files and did not establish their stated 37-pass evidence. | Canonical owner remains `.github#2272`. The corrected ordinary-forward lane restores all four authorities, preserves protected `main`, and copies the binder beside the model helper in each of 25 fixture runtimes. `.github#2267` adopts exact owner head `4e8829f5` as an ordinary second parent while preserving its scheduler delta. Acceptance requires the new source-first 25/25 contract, Python compile, binder suite, complete shell harness classification, and fresh exact-head hosted Runtime Quality; affected downstream heads such as `#2109` must likewise ordinary-adopt the repaired owner. | | CONTROL-OPENCODE-VCS-PYROOT-01 | **Source repaired on `main` (#2123 `ebc69a401`); image-path helper extracted + offline-proven under #2157 follow-up; hosted consumer step-#17 link still required to close the issue** | `ContextualWisdomLab/contextual-orchestrator#1149@684cf28f`의 중앙 [OpenCode run 34701472466](https://github.com/ContextualWisdomLab/.github/actions/runs/34701472466) `coverage-evidence` job `103574547257`은 PR 코드를 실행하기 전에 immutable `ContextualWisdomLab/fast-mlsirm@09f762d`의 `python/fast_mlsirm` import root를 찾지 못해 종료했다. 같은 head의 제품 테스트는 `3602 passed, 2 skipped`, native CodeQL·fuzz·SBOM·SAST·Strix는 성공했다. | `.github`의 `opencode-review-dispatch.yml`이 root/`src/`만 허용한 계약 drift를 소유했다. #2123이 `python/` candidates를 추가해 `main`에 병합했고, #2157 follow-up은 동일 로직을 `scripts/ci/resolve_opencode_base_vcs_import_root.sh`로 추출해 `tests/test_opencode_vcs_python_source_root_contract.py` fixture로 증명한다. Issue #2157 종료는 post-`ebc69a401` consumer `coverage-evidence`가 docker step #17을 통과한 job id를 문서에 링크한 뒤에만 한다. | ## 1. 근거와 범위 @@ -101,6 +108,7 @@ flowchart LR | G-15 | 첨부파일 처리 경계가 제품별로 다르고, 1MB 상한은 업무 데이터와 맞지 않으며 미지원 MIME/컨테이너가 parser registry에서 명시적으로 pending/quarantine 되는지 확인되지 않았다. 현재 20MB 초과 파일 가능성과 PDF/HWP/HWPX·이미지·압축파일의 parse/sidecar 흐름을 하나의 exact contract로 묶지 못했다 | 큰 업무 첨부를 거부하거나 파싱 실패를 조용히 잃으면 고객의 메일·문서 업무가 중단된다 | naruon/newsdom-api 소유 PR에서 streaming upload, configurable bounded limit above 20MB, MIME sniffing, parser capability registry, quarantine/retry, source-position provenance, and ADR를 추가하고 size/unsupported-type/zip-bomb tests를 required evidence로 만든다 | | G-16 | Required Pingora policy treated a changed documentation PNG screenshot as UTF-8 runtime evidence | Valid UI evidence blocked otherwise valid product PRs before policy evaluation | This branch verifies bounded PNG magic before exemption while runtime paths and malformed assets continue to fail closed; protected-main delivery remains the release gate | | G-17 | `.github#2279` blocked authenticated GitHub REST redirects in source, but redirect tests invoked `_RejectRedirects` directly and four Strix transport fixtures still patched the removed `urlopen` seam | A future opener-composition regression could forward a bearer token on a 3xx while redirect tests stayed green; Strix error mapping could fail before exercising production | Proposed `57477289ebec5631b0c48f0bc419f336dbe19deb` sends all four synthetic redirect classes through both real module-level openers; `663ffac390d27ab21daa58b91b624d3f00dce7de` moves every Strix fixture to the production opener; `9c19c6e00eafc028068719ab482282c1256f8893` adds malformed-authority coverage and records the owner evidence. Mutation RED proves the default opener contacts a second same-authority URL with the bearer header. The focused suite passes twice (`87 passed` normal and `GITHUB_ACTIONS=true`) with 100% statement/branch coverage on both affected modules. Exact-head hosted security and independent review remain required | +| G-18 | `.github#2267` converted one queue observation into implicit event/schedule mutation budgets (`1` and `8/20/8`) while `.github#2270` already owned the fail-closed budget-authority contract | Unmeasured defaults can over-admit review work during saturation or silently under-admit recovery, and two PRs become competing writers for the same scheduler policy | Proposed integration retains #2267's structured recovery taxonomy and update/dispatch fallthrough while merging #2270's RED→GREEN authority lineage. Review dispatch, branch update, and admission now require explicit dispatch/input/repository-variable authority; missing values fail closed. Exact-head hosted checks and independent review remain required | ## 4. 열린 PR live inventory @@ -3424,3 +3432,15 @@ alone -- it is a documented multi-PR hot-file collision zone. Contract: **Action.** Exact `57477289ebec5631b0c48f0bc419f336dbe19deb` adds a dependency-free synthetic-302 transport to `tests/test_github_api_url_boundary.py`. For both actual production openers, the case drives a canonical bearer request through the real HTTPS open/response chain, requires the typed HTTP-302 failure mapping, and proves transport receives exactly one original request; lookalike HTTPS, HTTP, `file:`, and same-authority redirect targets never receive a second request or bearer. Exact `e0b0b4d4fff5b6ea88236a1e91dcd7dbb3be09b5` repairs the doctoring claim so direct-handler coverage is not mislabeled as production-chain proof. **Evidence / remaining condition.** The standalone fixture mechanism was executed locally against Python stdlib and produced one canonical request followed by terminal HTTP 302 for every hostile target. This is mechanism evidence, not repository acceptance. Final authority requires focused/full exact-tree GREEN, fresh exact-head Security/SAST/Python Security/CodeQL/runtime-quality checks, no unresolved actionable review, ordinary protected-main integration, and downstream consumer validation. No scanner suppression, redirect allowlist widening, provider fallback, workflow gate weakening, or credential-boundary change is included. + +## 2026-09-19 SAST successor stack and forced-update carryover + +**Status:** Proposed on `ContextualWisdomLab/.github#2272`; exact-head hosted checks, zero actionable review findings, and qualifying independent approval remain mandatory. + +**Context Map / owner.** The central `.github` CI bounded context owns both the reusable Pages deployment shell boundary and the shared GitHub REST clients. `.github#2279` is the canonical owner lane for GitHub API authority/redirect behavior; `.github#2272` owns the Pages caller-input SAST repair and composes the released owner delta rather than copying an alternate transport implementation. + +**Gap.** The `#2272` head branch moved from `4967d66f303bde675080466e359e75c260a91e06` to sibling `1ca50644a8b3d155b125a5cf24aadeea7cb40a0a`, temporarily losing `.github/workflows/deploy-pages-input-security-ci.yml` and `tests/test_deploy_pages_input_shell_boundary.py`. A concurrent rewrite then restored `4967d66f...` as an ancestor at current `e0b6e70f8c8ea87648af2fc2d34dd43ffa625beb`, but that lineage still retained initial URL admission without `#2279`'s authenticated redirect containment, leaving its live review thread valid. + +**Action.** Ordinary merge `3923b196daf48f38759b42cd20a70e994ccb7935` retains current `#2272@e0b6e70f...` as first parent, including the restored `4967d66f...` Pages evidence, and integrates canonical owner `#2279@9c19c6e00eafc028068719ab482282c1256f8893` as second parent. The merge selects the stricter exact-authority parser and production no-redirect opener while preserving all Pages workflow/test deltas and the sibling origin-pin tests. + +**Evidence / remaining condition.** The stack graph is explicit and lossless; no predecessor was closed. This branch must independently pass the Pages workflow contract, GitHub authority/redirect suites, full repository tests, Python Security, Security Scan, SAST Semgrep, CodeQL PR, Runtime Quality, and current-head independent review. Predecessor checks and `#2279` receipts do not transfer. No Force Push, destructive rebase, synthetic status, scanner suppression, bypass, or source-neutral wake commit is authorized. diff --git a/scripts/ci/codeql_ghas_configuration_identity.py b/scripts/ci/codeql_ghas_configuration_identity.py index 53e00c41c6..f6c0a64da4 100644 --- a/scripts/ci/codeql_ghas_configuration_identity.py +++ b/scripts/ci/codeql_ghas_configuration_identity.py @@ -199,7 +199,8 @@ def _request_json(url: str, *, token: str, timeout_seconds: int) -> Any: with _GITHUB_API_OPENER.open(request, timeout=timeout_seconds) as response: payload = response.read().decode("utf-8") except urllib.error.HTTPError as exc: - body = exc.read().decode("utf-8", errors="replace")[-400:] + with exc: + body = exc.read().decode("utf-8", errors="replace")[-400:] raise ConfigurationIdentityError( f"GitHub API GET failed with HTTP {exc.code}: {body}" ) from exc diff --git a/scripts/ci/pr_review_merge_scheduler_core.py b/scripts/ci/pr_review_merge_scheduler_core.py index 4489ee62a3..a70edd258f 100644 --- a/scripts/ci/pr_review_merge_scheduler_core.py +++ b/scripts/ci/pr_review_merge_scheduler_core.py @@ -50,8 +50,8 @@ def __init__(self, state_path: Path, *, sequence: int, dispatch_budget: int) -> """Bind this gate to one durable state file, run sequence, and worker budget.""" if sequence < 1: raise ValueError("admission sequence must be positive") - if dispatch_budget < 0: - raise ValueError("admission dispatch budget must not be negative") + if dispatch_budget < -1: + raise ValueError("admission dispatch budget must be -1 or greater") self.state_path = Path(state_path) self.sequence = sequence self.dispatch_budget = dispatch_budget @@ -410,6 +410,7 @@ class Decision: action: str reason: str notes: tuple[str, ...] = () + review_recovery_class: str = "" RESOLVE_REVIEW_THREAD_MUTATION = """\ @@ -553,9 +554,10 @@ def decision_payload( dry_run: bool, base_branch: str, project_flow: str, + recovery: dict[str, int] | None = None, ) -> dict[str, Any]: """Return the machine-readable scheduler decision contract.""" - return { + payload: dict[str, Any] = { "schema_version": "pr-review-merge-scheduler/v2", "base_branch": base_branch, "dry_run": dry_run, @@ -564,6 +566,9 @@ def decision_payload( "project_flow": project_flow, "decisions": [decision_contract_entry(decision) for decision in decisions], } + if recovery is not None: + payload["recovery"] = recovery + return payload def decision_contract_entry(decision: Decision) -> dict[str, Any]: @@ -579,6 +584,8 @@ def decision_contract_entry(decision: Decision) -> dict[str, Any]: entry["guidance"] = guidance if decision.notes: entry["notes"] = list(decision.notes) + if decision.review_recovery_class: + entry["review_recovery_class"] = decision.review_recovery_class return entry @@ -2168,7 +2175,13 @@ def with_outdated_thread_cleanup_note(decision: Decision, count: int, *, dry_run f"{verb} {count} outdated review thread(s) before active unresolved-thread checks; " "outdated diff comments are not current-head review blockers." ) - return Decision(decision.pr, decision.action, decision.reason, (*decision.notes, note)) + return Decision( + decision.pr, + decision.action, + decision.reason, + (*decision.notes, note), + decision.review_recovery_class, + ) def review_author_login(review: dict[str, Any]) -> str: @@ -4454,6 +4467,7 @@ def inspect_pr( pr, dry_run=dry_run, ) + review_recovery_class = "" def finish(decision: Decision) -> Decision: """Attach obsolete review cleanup evidence to the final decision.""" @@ -4473,6 +4487,7 @@ def finish(decision: Decision) -> Decision: decision.action, decision.reason, (*decision.notes, note), + decision.review_recovery_class, ) approval_note = stale_approval_cleanup_note( stale_approval_cleanup_count, @@ -4485,12 +4500,20 @@ def finish(decision: Decision) -> Decision: decision.action, decision.reason, (*decision.notes, approval_note), + decision.review_recovery_class, ) return decision def decide(action: str, reason: str) -> Decision: """Create a decision after applying shared cleanup notes.""" - return finish(Decision(number, action, reason)) + return finish( + Decision( + number, + action, + reason, + review_recovery_class=review_recovery_class, + ) + ) def revalidate_before_merge() -> Decision | None: """Return a blocking decision if a fresh re-check just revoked approval. @@ -4542,6 +4565,7 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio f"{freshness_reason}; branch update requested with {mutation_token_label()} " f"inside GitHub Actions as {mutation_actor_label()}{suffix}", (followup_note,) if followup_note else (), + review_recovery_class, ) return finish(decision) @@ -4958,6 +4982,7 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio ) if behind_by and trigger_reviews: + review_recovery_class = "outdated_before_review" if not update_branches: return decide("wait", "current head has no OpenCode approval; branch update disabled before review dispatch") if not can_update_pr_head(repo, pr): @@ -4972,13 +4997,27 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio # current head and requeue the pull request behind them. Under a # saturated runner queue the PR's own delayed scheduler run does # this on every execution, so no head ever finishes its checks - # (#1935). Deliberately no age cap: a check that never finishes - # keeps the head where it is instead of restarting that loop. - return decide( - "wait", - "current head has no OpenCode approval; branch is outdated before review dispatch, " - "but current-head checks are still queued or running; holding the update so their " - "evidence is not discarded", + # (#1935). Deliberately no age cap on event-driven runs: a check + # that never finishes keeps the head where it is instead of + # restarting that loop. + # + # Daily schedule recovery is the exception. Soft-waiting forever + # under queue saturation left OpenCode-needing heads with neither + # update nor dispatch (cron 35202348887 / same class as fmls#2006). + # Prefer a loud update that discards in-flight checks over inert + # success when the only blocker is the #1935 hold. + if os.environ.get("GITHUB_EVENT_NAME") != "schedule": + return decide( + "wait", + "current head has no OpenCode approval; branch is outdated before review dispatch, " + "but current-head checks are still queued or running; holding the update so their " + "evidence is not discarded", + ) + print( + "::warning::Schedule recovery bypasses #1935 in-flight check hold for " + f"PR #{number}: updating outdated OpenCode-needing head despite " + "queued/running current-head checks so daily recovery is not inert.", + file=sys.stderr, ) if merge_state == "BEHIND": freshness_reason = "current head has no OpenCode approval; branch is outdated before review dispatch" @@ -4988,9 +5027,19 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio f"base branch is {behind_by} commit(s) ahead before review dispatch even though " f"GitHub mergeability is {merge_state}" ) - return request_branch_update(freshness_reason) + if branch_update_allowed or os.environ.get("GITHUB_EVENT_NAME") != "schedule": + return request_branch_update(freshness_reason) + # Schedule recovery with an exhausted update budget: still attempt + # review_dispatch on the behind head rather than soft-idle. Event + # paths keep failing closed at request_branch_update's limit wait. + print( + "::warning::Schedule recovery: branch update budget exhausted for " + f"PR #{number}; allowing review_dispatch on outdated OpenCode-needing " + "head so daily recovery yields non-zero update_or_dispatch.", + file=sys.stderr, + ) - if merge_state == "UNKNOWN": + if merge_state == "UNKNOWN" and review_recovery_class != "outdated_before_review": if pr.get("autoMergeRequest"): return finish( disable_auto_merge_decision( @@ -5154,12 +5203,15 @@ def print_summary( for decision in decisions: counts[decision.action] = counts.get(decision.action, 0) + 1 print(f"PR #{decision.pr}: {decision.action}: {decision.reason}") + recovery = classify_review_recovery(decisions) + print(f"scheduler_recovery_taxonomy {json.dumps(recovery, sort_keys=True)}") write_actions_summary( decisions, counts=counts, dry_run=dry_run, base_branch=base_branch, project_flow=project_flow, + recovery=recovery, ) print( json.dumps( @@ -5169,12 +5221,112 @@ def print_summary( dry_run=dry_run, base_branch=base_branch, project_flow=project_flow, + recovery=recovery, ), sort_keys=True, ) ) +def classify_review_recovery(decisions: Sequence[Decision]) -> dict[str, int]: + """Count review-recovery classes so schedule idle cannot look like success. + + ``update_before_review`` heads need a branch update before they are + review-dispatch eligible. Counting them as "eligible for dispatch" hides + why a recovery run can report OpenCode-needing work and still dispatch + zero reviews. + """ + recovery = { + "review_dispatch": 0, + "security_dispatch": 0, + "update_before_review": 0, + "update_before_review_inflight_hold": 0, + "dispatch_limit_reached": 0, + "admission_exhausted": 0, + "opencode_already_active": 0, + "dispatch_coalescing": 0, + } + for decision in decisions: + reason = decision.reason or "" + if decision.action == "review_dispatch": + recovery["review_dispatch"] += 1 + elif decision.action == "security_dispatch": + recovery["security_dispatch"] += 1 + if ( + decision.review_recovery_class == "outdated_before_review" + or "outdated before review dispatch" in reason + ): + if "queued or running" in reason: + recovery["update_before_review_inflight_hold"] += 1 + else: + recovery["update_before_review"] += 1 + if "review dispatch limit reached" in reason: + recovery["dispatch_limit_reached"] += 1 + if "bounded admission budget is exhausted" in reason: + recovery["admission_exhausted"] += 1 + if "workflow run is already active" in reason: + recovery["opencode_already_active"] += 1 + if "coalescing window" in reason: + recovery["dispatch_coalescing"] += 1 + return recovery + + +def emit_review_recovery_signal( + decisions: Sequence[Decision], + *, + trigger_reviews: bool, +) -> int: + """Fail closed when dispatch-eligible recovery work is silently skipped. + + Returns a process exit code: ``1`` when review-dispatch-eligible heads were + present but none dispatched (effective budget zero), or when a schedule + recovery run finds OpenCode-needing outdated heads and makes no update and + no dispatch. Otherwise returns ``0``, emitting a warning on schedule when + outdated-before-review heads explain a zero-dispatch recovery tick. + """ + if not trigger_reviews: + return 0 + recovery = classify_review_recovery(decisions) + dispatched = recovery["review_dispatch"] + recovery["security_dispatch"] + if recovery["dispatch_limit_reached"] > 0 and dispatched == 0: + print( + "::error::Scheduler found review-dispatch-eligible heads but dispatched " + f"none (dispatch_limit_reached={recovery['dispatch_limit_reached']}). " + "Effective review-dispatch budget resolved to zero.", + file=sys.stderr, + ) + return 1 + event_name = os.environ.get("GITHUB_EVENT_NAME", "") + outdated = ( + recovery["update_before_review"] + recovery["update_before_review_inflight_hold"] + ) + updates = sum(1 for decision in decisions if decision.action in {"update_branch", "restamp_head"}) + if ( + event_name == "schedule" + and outdated > 0 + and dispatched == 0 + and updates == 0 + and recovery["opencode_already_active"] == 0 + ): + print( + "::error::Schedule recovery found OpenCode-needing outdated heads but " + "made no branch update and no review dispatch (silent idle recovery).", + file=sys.stderr, + ) + return 1 + if event_name == "schedule" and outdated > 0 and dispatched == 0: + print( + "::warning::Schedule recovery: " + f"{outdated} OpenCode-needing head(s) were outdated-before-review " + f"(inflight_hold={recovery['update_before_review_inflight_hold']}, " + f"update_branch={updates}, review_dispatch={dispatched}). " + "Review dispatch runs only after the head is current; zero " + "review_dispatch on this tick is not a clean no-op.", + file=sys.stderr, + ) + return 0 + + def markdown_cell(value: object) -> str: """Escape a value for a compact GitHub Actions summary table cell.""" return str(value).replace("|", "\\|").replace("\n", "
") @@ -5193,6 +5345,7 @@ def write_actions_summary( dry_run: bool, base_branch: str, project_flow: str, + recovery: dict[str, int] | None = None, ) -> None: """Append scheduler decisions to the GitHub Actions step summary.""" summary_path = os.environ.get("GITHUB_STEP_SUMMARY") @@ -5207,10 +5360,16 @@ def write_actions_summary( f"- Dry run: `{str(dry_run).lower()}`", f"- Inspected PRs: `{len(decisions)}`", f"- Actions: `{json.dumps(counts, sort_keys=True)}`", - "", - "| PR | Action | Reason |", - "| ---: | --- | --- |", ] + if recovery is not None: + lines.append(f"- Recovery taxonomy: `{json.dumps(recovery, sort_keys=True)}`") + lines.extend( + [ + "", + "| PR | Action | Reason |", + "| ---: | --- | --- |", + ] + ) lines.extend( f"| #{decision.pr} | {markdown_cell(decision.action)} | {markdown_cell(decision.reason)} |" for decision in decisions @@ -6230,7 +6389,7 @@ def parse_args(argv: list[str]) -> argparse.Namespace: "--admission-dispatch-budget", type=int, default=int(os.environ.get("REVIEW_ADMISSION_DISPATCH_BUDGET", "1")), - help="Maximum leased review workers across this scheduler execution", + help="Maximum leased review workers across this scheduler execution; -1 means unlimited", ) parser.add_argument( "--admission-sequence", @@ -6290,8 +6449,8 @@ def main(argv: list[str]) -> int: raise SystemExit("--pr-number must not be negative") if args.review_dispatch_limit < -1: raise SystemExit("--review-dispatch-limit must be -1 or greater") - if args.admission_dispatch_budget < 0: - raise SystemExit("--admission-dispatch-budget must not be negative") + if args.admission_dispatch_budget < -1: + raise SystemExit("--admission-dispatch-budget must be -1 or greater") if args.admission_sequence < 1: raise SystemExit("--admission-sequence must be positive") if args.stacked_review_dispatch_limit is not None and args.stacked_review_dispatch_limit < -1: @@ -6404,7 +6563,7 @@ def main(argv: list[str]) -> int: project_flow=args.project_flow, ) _ACTIVE_ADMISSION_GATE = None - return 0 + return emit_review_recovery_signal(decisions, trigger_reviews=args.trigger_reviews) if __name__ == "__main__": # pragma: no cover diff --git a/scripts/ci/review_admission_controller.py b/scripts/ci/review_admission_controller.py index dab99d6ae1..baf13d1df0 100644 --- a/scripts/ci/review_admission_controller.py +++ b/scripts/ci/review_admission_controller.py @@ -314,9 +314,9 @@ def plan_dispatches( live_heads: Mapping[tuple[str, int], str], dispatch_budget: int, ) -> DispatchPlan: - """Apply requests and lease at most ``dispatch_budget`` independent workers.""" - if dispatch_budget < 0: - raise ValueError("dispatch budget must not be negative") + """Apply requests and lease workers, with ``-1`` as the explicit unlimited value.""" + if dispatch_budget < -1: + raise ValueError("dispatch budget must be -1 or greater") records = dict(state.records) latest = dict(state.latest_sequences) rejections: dict[str, str] = {} @@ -366,10 +366,14 @@ def plan_dispatches( ), ) dispatches = [] - available_budget = max( - 0, - dispatch_budget - - sum(record.status == "dispatched" for record in records.values()), + available_budget = ( + len(queued) + if dispatch_budget == -1 + else max( + 0, + dispatch_budget + - sum(record.status == "dispatched" for record in records.values()), + ) ) for record in queued: if len(dispatches) >= available_budget: diff --git a/scripts/ci/strix_evidence_binding.py b/scripts/ci/strix_evidence_binding.py index 7319040df2..9b1d28b4bd 100644 --- a/scripts/ci/strix_evidence_binding.py +++ b/scripts/ci/strix_evidence_binding.py @@ -305,6 +305,7 @@ def default_github_opener(url: str, token: str) -> Any: with _GITHUB_API_OPENER.open(request, timeout=30) as response: payload = response.read() except HTTPError as exc: + exc.close() raise EvidenceBindingError( f"GitHub changed-file request failed with HTTP {exc.code}" ) from exc diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 150b9102b3..60b623deba 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -1588,7 +1588,10 @@ assert_pr_review_merge_scheduler_uses_github_actions_bot_token() { assert_file_contains "$workflow_file" "github.event_name == 'pull_request_target' || (github.event_name == 'repository_dispatch' && github.event.client_payload.update_branches != false) || inputs.update_branches == true" "scheduler enables branch updates after native PR events or an explicit dispatch" assert_file_contains "$workflow_file" "review_dispatch_limit:" "scheduler exposes a bounded review dispatch budget" assert_file_contains "$workflow_file" "REVIEW_DISPATCH_LIMIT_INPUT" "scheduler forwards the review dispatch budget to the canonical script" - assert_file_contains "$workflow_file" 'review_dispatch_limit="-1"' "scheduler dispatches every eligible same-head review or Strix evidence job immediately unless an explicit budget overrides it" + assert_file_contains "$workflow_file" 'REVIEW_DISPATCH_LIMIT must be explicitly configured' "scheduler fails closed when no review dispatch authority is configured" + assert_file_contains "$workflow_file" 'BRANCH_UPDATE_LIMIT must be explicitly configured' "scheduler fails closed when no branch-update authority is configured" + assert_file_contains "$workflow_file" 'REVIEW_ADMISSION_DISPATCH_BUDGET must be explicitly configured' "scheduler fails closed when no admission authority is configured" + assert_file_not_contains "$workflow_file" 'review_dispatch_limit="1"' "scheduler does not invent a one-dispatch fallback" assert_file_not_contains "$workflow_file" 'review_dispatch_limit="0"' "scheduler must not silently suppress eligible review dispatches on base-branch push events" assert_file_contains "$workflow_file" "--review-dispatch-limit" "scheduler passes the dispatch budget to the canonical script" assert_file_contains "$workflow_file" "branch_update_limit:" "scheduler exposes a bounded branch-update budget" @@ -3296,6 +3299,7 @@ run_gate_case() { local gate_under_test="$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$GATE_SCRIPT" "$gate_under_test" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$gate_under_test" local fake_strix="$bin_dir/strix" local path_hijack_log="$tmp_dir/path-hijack.log" @@ -7026,6 +7030,7 @@ run_pull_request_target_head_scope_case() { mkdir -p "$bin_dir" "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" local fake_strix="$bin_dir/strix" @@ -7174,6 +7179,7 @@ run_pull_request_target_plaintext_runner_token_fails_closed_case() { mkdir -p "$bin_dir" "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" local fake_strix="$bin_dir/strix" @@ -7296,6 +7302,7 @@ run_pull_request_target_bounded_head_context_scope_case() { mkdir -p "$bin_dir" "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" local fake_strix="$bin_dir/strix" @@ -7401,6 +7408,7 @@ run_pull_request_target_changed_context_scope_uses_pr_head_case() { mkdir -p "$bin_dir" "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" local fake_strix="$bin_dir/strix" @@ -7580,6 +7588,7 @@ run_pull_request_target_changed_backend_context_scope_case() { mkdir -p "$bin_dir" "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" local fake_strix="$bin_dir/strix" @@ -7839,6 +7848,7 @@ run_pull_request_target_frontend_email_context_scope_case() { mkdir -p "$bin_dir" "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" local fake_strix="$bin_dir/strix" @@ -8029,6 +8039,7 @@ run_pull_request_target_shallow_head_merge_base_fallback_case() { cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" local fake_strix="$bin_dir/strix" @@ -8144,6 +8155,7 @@ run_pull_request_target_aborts_on_pr_head_blob_failure_case() { mkdir -p "$bin_dir" "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" local real_git @@ -8268,6 +8280,7 @@ run_pull_request_target_rejects_invalid_sha_case() { mkdir -p "$bin_dir" "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" local fake_strix="$bin_dir/strix" @@ -8361,6 +8374,7 @@ run_pull_request_target_irregular_head_entry_fails_closed_case() { mkdir -p "$bin_dir" "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" local fake_strix="$bin_dir/strix" @@ -8444,6 +8458,7 @@ run_pull_request_target_gitlink_is_explicitly_skipped_case() { mkdir -p "$bin_dir" "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" local fake_strix="$bin_dir/strix" @@ -8526,6 +8541,7 @@ run_full_head_scope_skips_gitlink_case() { mkdir -p "$bin_dir" "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" local fake_strix="$bin_dir/strix" @@ -8640,6 +8656,7 @@ run_pull_request_target_rejects_unsafe_changed_path_case() { mkdir -p "$bin_dir" "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" local fake_strix="$bin_dir/strix" @@ -8732,6 +8749,7 @@ run_timeout_cleanup_case() { mkdir -p "$bin_dir" "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" local fake_strix="$bin_dir/strix" local child_pid_file="$tmp_dir/child.pid" @@ -8814,6 +8832,7 @@ run_vertex_model_ignores_untrusted_llm_api_base_file_case() { mkdir -p "$repo_root_dir/scripts/ci" "$allowed_input_dir" "$outside_dir" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cat >"$fake_strix" <<'EOF' @@ -8866,6 +8885,7 @@ run_total_timeout_case() { mkdir -p "$bin_dir" "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" local fake_strix="$bin_dir/strix" local output_log="$tmp_dir/output.log" @@ -9193,6 +9213,7 @@ run_llm_api_base_file_outside_input_root_fails_closed_case() { mkdir -p "$repo_root_dir/scripts/ci" "$allowed_input_dir" "$outside_dir" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cat >"$fake_strix" <<'EOF' @@ -9248,6 +9269,7 @@ run_pr_scoped_llm_api_base_file_config_failure_exits_2_case() { mkdir -p "$repo_root_dir/scripts/ci" "$repo_root_dir/src" "$allowed_input_dir" "$outside_dir" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" printf '%s\n' 'print("one")' >"$repo_root_dir/src/one.py" printf '%s\n' 'print("two")' >"$repo_root_dir/src/two.py" @@ -9309,6 +9331,7 @@ run_required_input_file_outside_input_root_fails_closed_case() { mkdir -p "$repo_root_dir/scripts/ci" "$allowed_input_dir" "$outside_dir" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cat >"$fake_strix" <<'EOF' @@ -9379,6 +9402,7 @@ run_input_file_root_override_takes_precedence_over_runner_temp_case() { mkdir -p "$repo_root_dir/scripts/ci" "$explicit_input_root" "$inherited_runner_temp" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cat >"$fake_strix" <<'EOF' @@ -9433,6 +9457,7 @@ run_stale_report_case() { mkdir -p "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" mkdir -p "$stale_report_dir" @@ -9488,6 +9513,7 @@ run_symlink_report_case() { mkdir -p "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" mkdir -p "$external_report_dir" "$repo_root_dir/strix_runs" @@ -9544,6 +9570,7 @@ run_unsafe_target_path_case() { mkdir -p "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cat >"$fake_strix" <<'EOF' @@ -9592,6 +9619,7 @@ run_absolute_outside_target_path_case() { mkdir -p "$bin_dir" "$repo_root_dir/src" "$repo_root_dir/scripts/ci" cp "$GATE_SCRIPT" "$repo_root_dir/scripts/ci/strix_quick_gate.sh" cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh" "$repo_root_dir/scripts/ci/strix_model_utils.sh" + cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py" "$repo_root_dir/scripts/ci/strix_evidence_binding.py" chmod +x "$repo_root_dir/scripts/ci/strix_quick_gate.sh" local fake_strix="$bin_dir/strix" local call_log="$tmp_dir/calls.log" diff --git a/tests/test_codeql_ghas_configuration_identity.py b/tests/test_codeql_ghas_configuration_identity.py index 817cd56497..7728dbc99c 100644 --- a/tests/test_codeql_ghas_configuration_identity.py +++ b/tests/test_codeql_ghas_configuration_identity.py @@ -495,3 +495,25 @@ def test_list_codeql_analyses_rejects_non_list_payload(monkeypatch): monkeypatch.setattr(identity, "_request_json", lambda url, token, timeout_seconds: {"ok": True}) with pytest.raises(identity.ConfigurationIdentityError): identity.list_codeql_analyses("ContextualWisdomLab/wardnet", token="opaque") + + +def test_request_json_refuses_a_non_github_api_url(): + """The opener is pinned to https://api.github.com before the request is built. + + `_request_json` takes its URL as a plain string. Every caller builds an + api.github.com URL, but the function is what has to enforce it -- an + unexpected caller must not be able to make it fetch another host or another + scheme. The lookalike host matters as much as the scheme: a prefix check + would accept `api.github.com.evil.example`. + """ + assert ( + identity._require_github_api_url("https://api.github.com/repos/o/r") + == "https://api.github.com/repos/o/r" + ) + for rejected in ( + "http://api.github.com/repos/o/r", + "https://api.github.com.evil.example/repos/o/r", + "file:///etc/passwd", + ): + with pytest.raises(identity.ConfigurationIdentityError): + identity._require_github_api_url(rejected) diff --git a/tests/test_deploy_pages_input_shell_boundary.py b/tests/test_deploy_pages_input_shell_boundary.py new file mode 100644 index 0000000000..5583614ef3 --- /dev/null +++ b/tests/test_deploy_pages_input_shell_boundary.py @@ -0,0 +1,97 @@ +"""Executable shell-boundary contract for the reusable Pages deployment workflow.""" + +from __future__ import annotations + +import re +import unittest +from pathlib import Path + + +WORKFLOW_PATH = Path(__file__).parents[1] / ".github" / "workflows" / "deploy-pages.yml" +CALLER_INPUT_EXPRESSIONS = { + "PROJECT_NAME": "${{ inputs.project_name }}", + "BUILD_DIR": "${{ inputs.build_dir }}", + "CUSTOM_DOMAIN": "${{ inputs.custom_domain }}", +} + + +def _indented_blocks(text: str, key: str) -> tuple[str, ...]: + """Return literal/folded YAML blocks for ``key`` without requiring a YAML parser.""" + + lines = text.splitlines() + blocks: list[str] = [] + start_re = re.compile(rf"^(?P\s*){re.escape(key)}:\s*[|>][-+]?\s*$") + index = 0 + while index < len(lines): + match = start_re.match(lines[index]) + if match is None: + index += 1 + continue + base_indent = len(match.group("indent")) + index += 1 + body: list[str] = [] + while index < len(lines): + line = lines[index] + if line.strip() and len(line) - len(line.lstrip()) <= base_indent: + break + body.append(line) + index += 1 + blocks.append("\n".join(body)) + return tuple(blocks) + + +def _named_step(text: str, name: str) -> str: + """Return one workflow step block identified by its exact ``name`` field.""" + + lines = text.splitlines() + marker = f"- name: {name}" + for index, line in enumerate(lines): + if line.strip() != marker: + continue + step_indent = len(line) - len(line.lstrip()) + block = [line] + for next_line in lines[index + 1 :]: + if ( + next_line.strip().startswith("- name:") + and len(next_line) - len(next_line.lstrip()) == step_indent + ): + break + block.append(next_line) + return "\n".join(block) + raise AssertionError(f"workflow step not found: {name}") + + +class DeployPagesInputShellBoundaryTests(unittest.TestCase): + """Pin caller-controlled reusable-workflow inputs outside shell source text.""" + + @classmethod + def setUpClass(cls) -> None: + """Read the workflow once from the exact checked-out source tree.""" + + cls.workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + + def test_caller_inputs_never_interpolate_directly_into_run_scripts(self) -> None: + """Caller-controlled values must cross into shell scripts only through env.""" + + run_blocks = _indented_blocks(self.workflow, "run") + self.assertTrue(run_blocks, "deploy-pages.yml must contain executable run blocks") + for run_script in run_blocks: + for expression in CALLER_INPUT_EXPRESSIONS.values(): + self.assertNotIn(expression, run_script) + + def test_summary_binds_caller_inputs_through_environment(self) -> None: + """The summary step consumes caller values from named environment variables.""" + + summary = _named_step(self.workflow, "Summary") + for variable, expression in CALLER_INPUT_EXPRESSIONS.items(): + self.assertRegex( + summary, + rf"(?m)^\s+{re.escape(variable)}:\s+{re.escape(expression)}\s*$", + ) + self.assertIn("${PROJECT_NAME}", summary) + self.assertIn("${BUILD_DIR}", summary) + self.assertIn("${CUSTOM_DOMAIN:-(none)}", summary) + + +if __name__ == "__main__": # pragma: no cover - CI uses unittest discovery directly. + unittest.main() diff --git a/tests/test_github_api_url_boundary.py b/tests/test_github_api_url_boundary.py index a9050584fd..87f454f203 100644 --- a/tests/test_github_api_url_boundary.py +++ b/tests/test_github_api_url_boundary.py @@ -264,6 +264,34 @@ def test_published_lineage_guard_rejects_unreachable_g17_evidence() -> None: _assert_g17_evidence_is_published(mutated) +def test_published_lineage_guard_rejects_resolvable_nonancestor_g17_evidence( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A resolvable G-17 commit outside current ancestry must fail closed.""" + evidence_sha = "1" * 40 + commands: list[list[str]] = [] + + def fake_run(command: list[str], **_kwargs: Any) -> subprocess.CompletedProcess[str]: + """Resolve the object while rejecting only its published ancestry.""" + commands.append(command) + return subprocess.CompletedProcess( + command, + 0 if command[1] == "cat-file" else 1, + stdout="", + stderr="", + ) + + monkeypatch.setattr(subprocess, "run", fake_run) + + with pytest.raises(AssertionError, match="current HEAD ancestry"): + _assert_g17_evidence_is_published(f"| G-17 | `{evidence_sha}` |") + + assert commands == [ + ["git", "cat-file", "-e", f"{evidence_sha}^{{commit}}"], + ["git", "merge-base", "--is-ancestor", evidence_sha, "HEAD"], + ] + + def test_doctoring_qualifies_foreign_semgrep_revision_owner() -> None: """Foreign evidence must identify its repository instead of resembling a local SHA.""" doctoring = Path( diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index 5a41cb7cdc..283c1b892d 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -2400,8 +2400,13 @@ def test_merge_scheduler_uses_escalating_mutation_credentials(): in workflow ) assert "SCHEDULER_MUTATION_TOKEN_SOURCE" in workflow - assert 'default: "1"' in workflow - assert 'review_dispatch_limit="-1"' in workflow + review_input = workflow.split("review_dispatch_limit:", 1)[1].split( + "admission_dispatch_budget:", 1 + )[0] + assert 'default: "1"' not in review_input + assert "REVIEW_DISPATCH_LIMIT must be explicitly configured" in workflow + assert "BRANCH_UPDATE_LIMIT must be explicitly configured" in workflow + assert "REVIEW_ADMISSION_DISPATCH_BUDGET must be explicitly configured" in workflow assert "branch_update_limit:" in workflow assert "BRANCH_UPDATE_LIMIT_INPUT" in workflow assert '--branch-update-limit "$branch_update_limit"' in workflow diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 4b8715d361..f7a01a045d 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -6925,6 +6925,94 @@ def fail(_args, stdin=None): assert "Resource not accessible by integration" in capsys.readouterr().out + + +def test_classify_review_recovery_separates_outdated_from_dispatch(): + """Outdated-before-review heads are not review-dispatch eligible.""" + decisions = [ + sched.Decision( + 1, + "update_branch", + "current head has no OpenCode approval; branch is outdated before review dispatch; branch update requested", + ), + sched.Decision( + 2, + "wait", + "current head has no OpenCode approval; branch is outdated before review dispatch, " + "but current-head checks are still queued or running; holding the update", + ), + sched.Decision( + 3, + "review_dispatch", + "current head has completed Strix evidence; same-head OpenCode dispatched", + ), + sched.Decision( + 4, + "wait", + "current head has completed Strix evidence; review dispatch limit reached", + ), + ] + recovery = sched.classify_review_recovery(decisions) + assert recovery["update_before_review"] == 1 + assert recovery["update_before_review_inflight_hold"] == 1 + assert recovery["review_dispatch"] == 1 + assert recovery["dispatch_limit_reached"] == 1 + + +def test_emit_review_recovery_signal_errors_when_limit_reached_with_zero_dispatch(capsys, monkeypatch): + """Effective budget zero must not exit clean after finding dispatch-eligible work.""" + monkeypatch.delenv("GITHUB_EVENT_NAME", raising=False) + decisions = [ + sched.Decision( + 4, + "wait", + "current head has completed Strix evidence; review dispatch limit reached", + ) + ] + assert sched.emit_review_recovery_signal(decisions, trigger_reviews=True) == 1 + err = capsys.readouterr().err + assert "dispatched none" in err + assert sched.emit_review_recovery_signal(decisions, trigger_reviews=False) == 0 + + +def test_emit_schedule_recovery_warns_when_outdated_explains_zero_dispatch(capsys, monkeypatch): + """Schedule zero-dispatch with an update is a warning, not a clean silent success.""" + monkeypatch.setenv("GITHUB_EVENT_NAME", "schedule") + decisions = [ + sched.Decision( + 1, + "update_branch", + "current head has no OpenCode approval; branch is outdated before review dispatch; updated", + ), + sched.Decision( + 2, + "wait", + "current head has no OpenCode approval; branch is outdated before review dispatch, " + "but current-head checks are still queued or running; holding the update", + ), + ] + assert sched.emit_review_recovery_signal(decisions, trigger_reviews=True) == 0 + err = capsys.readouterr().err + assert "outdated-before-review" in err + + +def test_emit_schedule_recovery_errors_when_idle_with_only_inflight_holds(capsys, monkeypatch): + """Schedule recovery that neither updates nor dispatches must not exit clean.""" + monkeypatch.setenv("GITHUB_EVENT_NAME", "schedule") + decisions = [ + sched.Decision( + 1198, + "wait", + "current head has no OpenCode approval; branch is outdated before review dispatch, " + "but current-head checks are still queued or running; holding the update so their " + "evidence is not discarded", + ), + ] + assert sched.emit_review_recovery_signal(decisions, trigger_reviews=True) == 1 + err = capsys.readouterr().err + assert "silent idle recovery" in err + + def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys): monkeypatch.setenv("SCHEDULER_MUTATION_TOKEN_SOURCE", "github-token") summary_path = tmp_path / "summary.md" @@ -9581,8 +9669,8 @@ def test_main_rejects_invalid_review_dispatch_limit(): ) -def test_main_rejects_negative_admission_dispatch_budget(): - with pytest.raises(SystemExit, match="--admission-dispatch-budget must not be negative"): +def test_main_rejects_admission_dispatch_budget_below_unlimited_sentinel(): + with pytest.raises(SystemExit, match="--admission-dispatch-budget must be -1 or greater"): sched.main( [ "--repo", @@ -9592,11 +9680,31 @@ def test_main_rejects_negative_admission_dispatch_budget(): "--project-flow", "github-flow", "--admission-dispatch-budget", - "-1", + "-2", ] ) +def test_main_accepts_unlimited_admission_dispatch_budget(monkeypatch, tmp_path): + """The explicit -1 operator value reaches the admission gate unchanged.""" + monkeypatch.setattr(sched, "fetch_open_prs", lambda *_args: []) + + assert sched.main( + [ + "--repo", + "owner/repo", + "--base-branch", + "main", + "--project-flow", + "github-flow", + "--admission-state-path", + str(tmp_path / "admission.json"), + "--admission-dispatch-budget", + "-1", + ] + ) == 0 + + def test_main_rejects_non_positive_admission_sequence(): with pytest.raises(SystemExit, match="--admission-sequence must be positive"): sched.main( @@ -10915,8 +11023,21 @@ def test_admission_gate_rejects_invalid_sequence_and_budget(tmp_path): state_path = tmp_path / "admission.json" with pytest.raises(ValueError, match="admission sequence must be positive"): sched.SchedulerAdmissionGate(state_path, sequence=0, dispatch_budget=1) - with pytest.raises(ValueError, match="admission dispatch budget must not be negative"): - sched.SchedulerAdmissionGate(state_path, sequence=1, dispatch_budget=-1) + with pytest.raises(ValueError, match="admission dispatch budget must be -1 or greater"): + sched.SchedulerAdmissionGate(state_path, sequence=1, dispatch_budget=-2) + + +def test_admission_gate_unlimited_budget_dispatches_all_workers(tmp_path): + """The explicit -1 budget leases every eligible independent worker.""" + gate = sched.SchedulerAdmissionGate( + tmp_path / "admission.json", sequence=76, dispatch_budget=-1 + ) + pr = make_pr(number=7, headRefOid="a" * 40) + + assert all( + gate.admit(component, "ContextualWisdomLab/example", pr) + for component in ("opencode", "noema", "strix") + ) def test_bounded_admission_persists_leases_and_completes_only_current_head( @@ -11072,7 +11193,7 @@ def test_reconcile_releases_strix_lease_when_no_run_was_created(tmp_path): assert record.status == "stale" -def test_inspect_pr_holds_pre_review_update_while_current_head_checks_run(): +def test_inspect_pr_holds_pre_review_update_while_current_head_checks_run(monkeypatch): """A behind, unreviewed head keeps its queued checks instead of being updated (#1935). Under a saturated queue the PR's own delayed scheduler run used to merge @@ -11080,8 +11201,11 @@ def test_inspect_pr_holds_pre_review_update_while_current_head_checks_run(): check on the old head and requeueing the PR behind them. The hold has no age cap on purpose: a check that never finishes keeps the head in place rather than restarting that loop, and the update resumes as soon as every - newest check run has a terminal status. + newest check run has a terminal status. Daily ``schedule`` recovery is the + deliberate exception — see + ``test_inspect_pr_schedule_bypasses_inflight_hold_for_recovery``. """ + monkeypatch.delenv("GITHUB_EVENT_NAME", raising=False) def behind_with(nodes): return make_pr( @@ -11117,3 +11241,122 @@ def behind_with(nodes): assert "checks are still queued or running" not in resumed.reason assert sched.has_in_flight_check_runs(behind_with([])) is False + + +def test_inspect_pr_schedule_bypasses_inflight_hold_for_recovery(monkeypatch, capsys): + """Daily schedule recovery updates outdated OpenCode-needing heads despite #1935.""" + monkeypatch.setenv("GITHUB_EVENT_NAME", "schedule") + pr = make_pr( + mergeStateStatus="BEHIND", + statusCheckRollup={ + "contexts": { + "nodes": [ + { + "__typename": "CheckRun", + "name": "trivy-fs", + "status": "QUEUED", + "conclusion": None, + }, + { + "__typename": "CheckRun", + "name": "scan-pr-queue", + "status": "IN_PROGRESS", + "conclusion": None, + }, + ] + } + }, + ) + + decision = inspect(pr) + + assert decision.action == "update_branch" + assert "outdated before review dispatch" in decision.reason + assert "checks are still queued or running" not in decision.reason + err = capsys.readouterr().err + assert "bypasses #1935" in err + assert "daily recovery is not inert" in err + + +def test_inspect_pr_schedule_dispatches_when_update_budget_exhausted(monkeypatch, capsys): + """Schedule recovery still dispatches when the branch-update budget is spent.""" + monkeypatch.setenv("GITHUB_EVENT_NAME", "schedule") + dispatched = [] + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, pr, dry_run: dispatched.append((repo, workflow, pr["headRefOid"])) + or "dispatched", + ) + monkeypatch.setattr(sched, "repository_dispatch_wait_reason", lambda *_args: None) + pr = make_pr( + mergeStateStatus="BEHIND", + compareBehindBy=3, + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + ) + + decision = inspect(pr, branch_update_allowed=False, branch_update_limit=0) + + assert decision.action == "review_dispatch" + assert "same-head OpenCode dispatched" in decision.reason + assert dispatched == [("owner/repo", "OpenCode Review", "head")] + err = capsys.readouterr().err + assert "branch update budget exhausted" in err + assert "allowing review_dispatch on outdated" in err + + +def test_schedule_recovery_dispatches_when_compare_proves_unknown_head_outdated( + monkeypatch, + capsys, +): + """A compare-proven outdated head must not stop at UNKNOWN mergeability.""" + monkeypatch.setenv("GITHUB_EVENT_NAME", "schedule") + dispatched = [] + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, pr, dry_run: dispatched.append((repo, workflow, pr["headRefOid"])) + or "dispatched", + ) + monkeypatch.setattr(sched, "repository_dispatch_wait_reason", lambda *_args: None) + pr = make_pr( + mergeStateStatus="UNKNOWN", + restMergeableState="UNKNOWN", + compareStatus="behind", + compareBehindBy=3, + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + ) + + decision = inspect(pr, branch_update_allowed=False, branch_update_limit=0) + + assert decision.action == "review_dispatch" + assert decision.review_recovery_class == "outdated_before_review" + assert dispatched == [("owner/repo", "OpenCode Review", "head")] + err = capsys.readouterr().err + assert "branch update budget exhausted" in err + + +def test_classify_review_recovery_uses_structured_outdated_state(): + """Preserve recovery classification when a later wait reason replaces freshness prose.""" + decisions = [ + sched.Decision( + 1, + "wait", + "bounded admission budget is exhausted", + review_recovery_class="outdated_before_review", + ), + sched.Decision(2, "security_dispatch", "same-head Strix dispatched"), + sched.Decision(3, "wait", "same-head OpenCode workflow run is already active"), + sched.Decision(4, "wait", "current head is within the push-burst coalescing window"), + ] + + recovery = sched.classify_review_recovery(decisions) + + assert recovery["update_before_review"] == 1 + assert recovery["admission_exhausted"] == 1 + assert recovery["security_dispatch"] == 1 + assert recovery["opencode_already_active"] == 1 + assert recovery["dispatch_coalescing"] == 1 + assert sched.decision_contract_entry(decisions[0])["review_recovery_class"] == ( + "outdated_before_review" + ) diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 87277d45f5..c867ebf541 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -146,12 +146,16 @@ def workflow_step(workflow: str, name: str) -> str: return workflow[start:end] -def test_merge_scheduler_dispatches_one_review_by_default() -> None: - """Keep the default scheduler dispatch bounded to one review.""" +def test_merge_scheduler_requires_an_explicit_review_dispatch_limit() -> None: + """Do not invent a review-dispatch budget when no authority supplied one.""" workflow = workflow_text("pr-review-merge-scheduler.yml") - assert workflow.count('default: "1"') >= 2 - assert "vars.REVIEW_DISPATCH_LIMIT || '1'" in workflow + review_input = workflow.split("review_dispatch_limit:", 1)[1].split( + "admission_dispatch_budget:", 1 + )[0] + assert 'default: "1"' not in review_input + assert "vars.REVIEW_DISPATCH_LIMIT || '1'" not in workflow + assert "vars.REVIEW_DISPATCH_LIMIT || ''" in workflow assert "SCHEDULER_ALLOW_CROSS_REPO_REPOSITORY_DISPATCH" in workflow assert ( "secrets.PR_REVIEW_MERGE_TOKEN != '' || secrets.OPENCODE_APPROVE_TOKEN != ''" @@ -159,6 +163,51 @@ def test_merge_scheduler_dispatches_one_review_by_default() -> None: ) +def test_merge_scheduler_empty_review_dispatch_limit_fails_closed() -> None: + """An absent dispatch budget must stop instead of choosing a magic limit.""" + workflow = workflow_text("pr-review-merge-scheduler.yml") + run_step = workflow_step(workflow, "Inspect PR review and merge queue") + + assert "vars.REVIEW_DISPATCH_LIMIT || ''" in workflow + assert ( + '-1 dispatches every eligible current-head review' in workflow + ), "explicit -1 unlimited must remain documented on the input" + assert 'review_dispatch_limit="$REVIEW_DISPATCH_LIMIT_INPUT"' in run_step + assert ( + 'if [ -z "$review_dispatch_limit" ]; then\n' + ' echo "::error::REVIEW_DISPATCH_LIMIT must be explicitly configured" >&2\n' + " exit 1\n" + " fi" + ) in run_step + assert 'review_dispatch_limit="1"' not in run_step + assert 'review_dispatch_limit="-1"' not in run_step + + +def test_merge_scheduler_other_empty_mutation_budgets_fail_closed() -> None: + """Branch updates and admission must also require operator authority.""" + workflow = workflow_text("pr-review-merge-scheduler.yml") + run_step = workflow_step(workflow, "Inspect PR review and merge queue") + + assert "vars.BRANCH_UPDATE_LIMIT || ''" in workflow + assert "vars.REVIEW_ADMISSION_DISPATCH_BUDGET || ''" in workflow + assert "BRANCH_UPDATE_LIMIT must be explicitly configured" in run_step + assert "REVIEW_ADMISSION_DISPATCH_BUDGET must be explicitly configured" in run_step + assert "default_branch_update_limit" not in run_step + assert "default_admission_dispatch_budget" not in run_step + + +def test_merge_scheduler_preserves_numeric_zero_repository_dispatch_budgets() -> None: + """A numeric zero payload remains authoritative instead of falling through.""" + workflow = workflow_text("pr-review-merge-scheduler.yml") + + for fragment in ( + "REVIEW_DISPATCH_LIMIT_INPUT: ${{ format('{0}', github.event.client_payload.review_dispatch_limit) ||", + "REVIEW_ADMISSION_DISPATCH_BUDGET: ${{ format('{0}', github.event.client_payload.admission_dispatch_budget) ||", + "BRANCH_UPDATE_LIMIT_INPUT: ${{ format('{0}', github.event.client_payload.branch_update_limit) ||", + ): + assert fragment in workflow + + def test_scheduler_uses_bounded_run_state_without_cache_lock_claims() -> None: """Keep each run bounded without treating immutable cache snapshots as locks.""" workflow = workflow_text("pr-review-merge-scheduler.yml") diff --git a/tests/test_review_admission_controller.py b/tests/test_review_admission_controller.py index ce83f13918..1b2a729746 100644 --- a/tests/test_review_admission_controller.py +++ b/tests/test_review_admission_controller.py @@ -82,6 +82,24 @@ def test_controller_is_idempotent_bounded_and_rejects_stale_or_out_of_order() -> assert delayed.rejections[request("opencode", HEAD_3, 1).identity] == "out_of_order" +def test_controller_unlimited_budget_dispatches_every_eligible_worker() -> None: + """The explicit -1 operator value removes only the per-run admission cap.""" + requests = [request(component) for component in ("opencode", "noema", "strix")] + + plan = plan_dispatches( + ControllerState.empty(), + requests, + live_heads={(requests[0].repository, requests[0].pull_request): HEAD_2}, + dispatch_budget=-1, + ) + + assert [lease.request.component for lease in plan.dispatches] == [ + "opencode", + "noema", + "strix", + ] + + def test_worker_boundaries_remain_separate_and_publish_requires_live_head_cas() -> None: assert ADMISSION_PERMISSIONS == ("contents: read", "pull-requests: read") assert set(WORKER_BOUNDARIES) == {"opencode", "noema", "strix"} @@ -395,8 +413,8 @@ def test_state_file_rejects_corruption_symlinks_and_nonregular_paths(tmp_path) - def test_update_and_dispatch_reject_invalid_transitions(tmp_path) -> None: with pytest.raises(TypeError, match="must return ControllerState"): update_state_file(tmp_path / "state.json", lambda state: object()) - with pytest.raises(ValueError, match="budget must not be negative"): - plan_dispatches(ControllerState.empty(), [], live_heads={}, dispatch_budget=-1) + with pytest.raises(ValueError, match="budget must be -1 or greater"): + plan_dispatches(ControllerState.empty(), [], live_heads={}, dispatch_budget=-2) item = request("opencode", HEAD_2, 2) lease = DispatchLease(item, WORKER_BOUNDARIES["opencode"]) diff --git a/tests/test_strix_evidence_binding.py b/tests/test_strix_evidence_binding.py index 90a5454ecb..d460744b6b 100644 --- a/tests/test_strix_evidence_binding.py +++ b/tests/test_strix_evidence_binding.py @@ -969,3 +969,38 @@ def test_workspace_missing_root_returns_false(tmp_path: Path) -> None: missing = tmp_path / "missing-root" assert binding.workspace_contains_expected_diff(missing, "a.py", "body") is False + + +def test_default_github_opener_refuses_a_non_github_origin() -> None: + """The opener takes a string, so it must pin the origin itself. + + Without this, an unexpected caller could make it fetch any scheme or host, + including file:// or an internal address. Semgrep's dynamic-urllib audit + rule is what surfaced the gap. + """ + import importlib.util + import sys + from pathlib import Path + + spec = importlib.util.spec_from_file_location( + "strix_evidence_binding", Path("scripts/ci/strix_evidence_binding.py") + ) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + sys.modules["strix_evidence_binding"] = module + spec.loader.exec_module(module) + + assert ( + module._require_github_api_url("https://api.github.com/repos/o/r") + == "https://api.github.com/repos/o/r" + ) + for rejected in ( + "http://api.github.com/repos/o/r", + "https://api.github.com.evil.example/repos/o/r", + "file:///etc/passwd", + ): + try: + module._require_github_api_url(rejected) + except module.EvidenceBindingError: + continue + raise AssertionError(f"{rejected} was not rejected") diff --git a/tests/test_strix_fixture_runtime_closure.py b/tests/test_strix_fixture_runtime_closure.py new file mode 100644 index 0000000000..be69b0120d --- /dev/null +++ b/tests/test_strix_fixture_runtime_closure.py @@ -0,0 +1,17 @@ +"""Regression contract for isolated Strix fixture runtime dependencies.""" + +from pathlib import Path + + +SELF_TEST_PATH = Path("scripts/ci/test_strix_quick_gate.sh") +MODEL_UTILS_COPY = 'cp "$REPO_ROOT/scripts/ci/strix_model_utils.sh"' +EVIDENCE_BINDER_COPY = 'cp "$REPO_ROOT/scripts/ci/strix_evidence_binding.py"' + + +def test_every_isolated_strix_fixture_copies_the_evidence_binder() -> None: + """Each of the 25 gate fixtures must carry every production runtime helper.""" + + self_test = SELF_TEST_PATH.read_text(encoding="utf-8") + + assert self_test.count(MODEL_UTILS_COPY) == 25 + assert self_test.count(EVIDENCE_BINDER_COPY) == 25