Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 46 additions & 26 deletions .github/workflows/strix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,19 @@ concurrency:
github.event.pull_request.number ||
github.event.client_payload.pr_number ||
(github.event_name == 'push' && format('push-{0}', github.ref_name)) ||
github.run_id }}
cancel-in-progress: true
github.run_id }}-${{
github.event.action == 'closed' && github.run_id ||
github.event_name == 'push' && 'protected-ref' ||
github.event.pull_request.head.sha ||
github.event.client_payload.pr_head_sha || github.run_id }}
# Draft/Ready and duplicate admission events can share one exact head. Do
# not let those lifecycle events destroy an executing scanner verdict. The
# metadata-only cleanup job below remains the cancellation owner for a
# verified superseded head or closed pull request. Closed events use their
# unique run id above so cleanup cannot queue behind the scan it must stop.
# Only a newer protected-branch push cancels in-progress work; PR lifecycle
# events use exact-head identities and never stop same-head provider work.
cancel-in-progress: ${{ github.event_name == 'push' }}

# Scorecard Token-Permissions (alert #43): keep the workflow-level token
# read-only and scope same-repo status publication to the Strix scan job.
Expand Down Expand Up @@ -242,8 +253,10 @@ jobs:

cancel-superseded-pr-runs:
if: >-
github.event_name == 'pull_request_target' &&
(github.event.action == 'synchronize' || github.event.action == 'converted_to_draft' || github.event.action == 'closed')
(github.event_name == 'pull_request_target' &&
(github.event.action == 'synchronize' || github.event.action == 'closed')) ||
(github.event_name == 'repository_dispatch' &&
github.event.client_payload.pr_number != '')
# Idempotent per PR: a fresh sweep re-verifies live state (live_target_matches
# below) before selecting or cancelling anything, so it fully subsumes
# whatever an older, not-yet-run instance would have done. cancel-in-progress
Expand All @@ -257,7 +270,9 @@ jobs:
concurrency:
group: >-
cancel-superseded-pr-runs-${{
github.event.client_payload.target_repository ||
github.event.pull_request.base.repo.full_name || github.repository }}-${{
github.event.client_payload.pr_number ||
github.event.pull_request.number || github.run_id }}
cancel-in-progress: true
runs-on: ubuntu-24.04
Expand All @@ -277,10 +292,11 @@ jobs:
pull-requests: read
env:
GH_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || github.token }}
TARGET_REPOSITORY: ${{ github.event.pull_request.base.repo.full_name || github.repository }}
TARGET_PR_NUMBER: ${{ github.event.pull_request.number }}
TARGET_PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
PR_ACTION: ${{ github.event.action }}
RUN_REPOSITORY: ${{ github.repository }}
TARGET_REPOSITORY: ${{ github.event.client_payload.target_repository || github.event.pull_request.base.repo.full_name || github.repository }}
TARGET_PR_NUMBER: ${{ github.event.client_payload.pr_number || github.event.pull_request.number }}
TARGET_PR_HEAD_SHA: ${{ github.event.client_payload.pr_head_sha || github.event.pull_request.head.sha }}
PR_ACTION: ${{ github.event_name == 'repository_dispatch' && 'synchronize' || github.event.action }}
CURRENT_RUN_ID: ${{ github.run_id }}
steps:
- name: Cancel queued and running scans for superseded or inactive pull requests
Expand All @@ -289,18 +305,16 @@ jobs:
set -euo pipefail

live_target_matches() {
local live_pr_json live_state live_draft live_head
local live_pr_json live_state live_head
if ! live_pr_json="$(gh api "repos/${TARGET_REPOSITORY}/pulls/${TARGET_PR_NUMBER}" 2>/tmp/strix-cleanup-gh-error)"; then
echo "::warning::Strix cleanup could not verify the live pull request; leaving runs unchanged."
sed 's/^/ /' /tmp/strix-cleanup-gh-error >&2 || true
return 1
fi
live_state="$(jq -r '.state // ""' <<<"$live_pr_json")"
live_draft="$(jq -r '.draft // false' <<<"$live_pr_json")"
live_head="$(jq -r '.head.sha // ""' <<<"$live_pr_json")"
[ "$live_head" = "$TARGET_PR_HEAD_SHA" ] && {
{ [ "$PR_ACTION" = "closed" ] && [ "$live_state" = "closed" ]; } ||
{ [ "$PR_ACTION" = "converted_to_draft" ] && [ "$live_state" = "open" ] && [ "$live_draft" = "true" ]; } ||
{ [ "$PR_ACTION" = "synchronize" ] && [ "$live_state" = "open" ]; }
}
}
Expand All @@ -311,7 +325,7 @@ jobs:
echo "::notice::Strix cleanup target changed before run selection; leaving runs unchanged."
return 0
fi
local runs_url="repos/${TARGET_REPOSITORY}/actions/runs?status=${status}&per_page=100"
local runs_url="repos/${RUN_REPOSITORY}/actions/runs?status=${status}&per_page=100"
local runs_json
if ! runs_json="$(gh api --paginate "$runs_url" 2>/tmp/strix-cleanup-gh-error)"; then
echo "::warning::Strix cleanup could not inspect ${TARGET_REPOSITORY}; leaving runs unchanged."
Expand All @@ -320,25 +334,26 @@ jobs:
fi
local run_ids
if ! run_ids="$(jq -r --arg pr "$TARGET_PR_NUMBER" --arg head_sha "$TARGET_PR_HEAD_SHA" \
--arg action "$PR_ACTION" --arg repo "$TARGET_REPOSITORY" --arg current "$CURRENT_RUN_ID" '
--arg action "$PR_ACTION" --arg repo "$TARGET_REPOSITORY" --arg run_repo "$RUN_REPOSITORY" \
--arg current "$CURRENT_RUN_ID" '
.workflow_runs[]
| select((.id | tostring) != $current)
| select(.name == "Strix Security Scan")
| select(.event == "pull_request_target")
| select(.event == "pull_request_target" or .event == "repository_dispatch")
| (($run_repo | ascii_downcase) == ($repo | ascii_downcase)) as $metadata_is_target_repository
| ((.display_title // "") | startswith("Strix Security Scan " + $repo + "#" + $pr + "@")) as $title_matches
| ((.pull_requests // []) | any((.number | tostring) == $pr)) as $metadata_matches
| ($metadata_is_target_repository and ((.pull_requests // []) | any((.number | tostring) == $pr))) as $metadata_matches
| select($title_matches or $metadata_matches)
| ((.display_title // "") | endswith("@" + $head_sha)) as $title_is_current
| ((.pull_requests // []) | any(
| ($metadata_is_target_repository and ((.pull_requests // []) | any(
((.number | tostring) == $pr)
and ((.head.sha // "") | ascii_downcase) == ($head_sha | ascii_downcase)
)) as $metadata_is_current
| ((.pull_requests // []) | any(
))) as $metadata_is_current
| ($metadata_is_target_repository and ((.pull_requests // []) | any(
((.number | tostring) == $pr) and ((.head.sha // "") != "")
)) as $metadata_has_head
))) as $metadata_has_head
| select(
$action == "closed"
or $action == "converted_to_draft"
or (($title_matches or $metadata_has_head) and (($title_is_current or $metadata_is_current) | not))
)
| .id
Expand All @@ -352,11 +367,11 @@ jobs:
echo "::notice::Strix cleanup target changed before cancellation; leaving runs unchanged."
return 0
fi
if gh api --method POST "repos/${TARGET_REPOSITORY}/actions/runs/${run_id}/cancel" >/dev/null 2>/tmp/strix-cleanup-cancel-error ||
gh api --method POST "repos/${TARGET_REPOSITORY}/actions/runs/${run_id}/force-cancel" >/dev/null 2>>/tmp/strix-cleanup-cancel-error; then
echo "Cancelled obsolete Strix run ${run_id} in ${TARGET_REPOSITORY} for PR #${TARGET_PR_NUMBER}."
if gh api --method POST "repos/${RUN_REPOSITORY}/actions/runs/${run_id}/cancel" >/dev/null 2>/tmp/strix-cleanup-cancel-error ||
gh api --method POST "repos/${RUN_REPOSITORY}/actions/runs/${run_id}/force-cancel" >/dev/null 2>>/tmp/strix-cleanup-cancel-error; then
echo "Cancelled obsolete Strix run ${run_id} in ${RUN_REPOSITORY} for ${TARGET_REPOSITORY} PR #${TARGET_PR_NUMBER}."
else
echo "::warning::Strix cleanup could not cancel run ${run_id} in ${TARGET_REPOSITORY}; it may have finished or the credential lacks Actions write access."
echo "::warning::Strix cleanup could not cancel run ${run_id} in ${RUN_REPOSITORY}; it may have finished or the credential lacks Actions write access."
sed 's/^/ /' /tmp/strix-cleanup-cancel-error >&2 || true
fi
done <<<"$run_ids"
Expand All @@ -367,8 +382,13 @@ jobs:
done

strix:
needs: [changed-scope, admit-current-head]
if: needs.changed-scope.outputs.code == 'true' && needs.admit-current-head.outputs.admitted == 'true'
needs: [changed-scope, admit-current-head, cancel-superseded-pr-runs]
if: >-
always() && !cancelled() &&
needs.changed-scope.outputs.code == 'true' &&
needs.admit-current-head.outputs.admitted == 'true' &&
(needs.cancel-superseded-pr-runs.result == 'success' ||
needs.cancel-superseded-pr-runs.result == 'skipped')
# Large, actively-growing repositories (e.g. contextual-orchestrator) can
# legitimately require well over two hours to scan -- this org's own
# standing operating directive accepts that central OpenCode/Strix/Noema
Expand Down
23 changes: 23 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,26 @@
### Strix reruns bind the exact PR base as well as the head

- The scheduler now rejects a failed Strix job whose native pull-request
association belongs to an older base SHA, and revalidates both live base and
head immediately before the rerun mutation. Retargeting an unchanged head can
no longer replay an old-base scanner job as current evidence.

### Strix preserves PR evidence and retires superseded push scans

- Workflow-level `cancel-in-progress` is true only for `push`; Draft/Ready and
duplicate same-head PR admission events cannot destroy an executing scanner
verdict. The PR group is exact-head scoped, so a synchronized new head can start
its metadata-only superseded-run cleanup without waiting behind the old scan;
its provider job now waits for that cleanup to finish. A closed event uses its
unique run id for the same reason. Draft transitions preserve the current scan,
while the live-revalidated cleanup job covers both native and dispatched PR
runs and cancels only verified superseded heads or a closed pull request.
Native PR metadata is accepted only when the run and target repositories match,
preventing same-number cross-repository cancellation. No provider deadline or
merge-gate relaxation was added. This repairs the cancellation pattern
seen in runs `34068478185`, `34067942252`, and PR #1999 run `34067362987`,
while preserving #1938's protected-ref push coalescing and cancellation.

### Failed-check finding names the Strix sandbox instead of the gateway

- `opencode-review-dispatch.yml`'s `emit_strix_provider_failure_finding` rendered one fixed finding for every `STRIX_PROVIDER_UNAVAILABLE` line, whose Root cause read "The contextual-orchestrator gateway or its discovered provider pool was unavailable for this run". `#1953` had just given the Strix sandbox bootstrap failure its own second verdict token (`STRIX_SANDBOX_UNAVAILABLE`) precisely because that attribution is wrong for it -- the sandbox container never reaches its Caido proxy, so the run dies before the gateway serves anything -- and this consumer re-applied the wrong attribution one step downstream, into the review findings and the failure census. The emitter now branches on the second token: a sandbox verdict gets a finding that names Strix's sandbox, says the verdict does not name the gateway, and tells the reader not to change gateway or provider configuration on its strength. A `STRIX_PROVIDER_UNAVAILABLE` line without the token keeps its existing text verbatim, so the gateway class has no regression surface. No test covered this finding text at all before (`gateway or its discovered provider pool` matched nothing under `tests/`); `tests/test_opencode_dispatch_strix_sandbox_finding.py` now runs the production emitter from the published run block and pins both directions plus the no-signal case. Refs #1953, #1935.
Expand Down
48 changes: 48 additions & 0 deletions docs/doctoring/scheduler-explicit-open-live-dispatch-guard.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Scheduler의 명시적 OPEN·현재 head 확인

## 원인과 범위

#1902의 후속 조사에서 CodeQL 복구 primitive보다 먼저 고칠 공통 결함을 확인했다.
`4bf80b99b6908c0323ac406d7e30e8346e09a50d`의
`scripts/ci/pr_review_merge_scheduler_core.py`는 GraphQL 공통 PR fragment에서
`state`를 요청하지 않았고, REST PR 정규화에서도 그 필드를 보존하지 않았다.
그런데 `live_dispatch_head_matches`는 누락되거나 빈 state를 OPEN으로 취급했다.
단일 PR 조회는 닫힌 PR도 반환하므로 head가 그대로면 닫힌 PR을 허용할 수 있었다.
또한 양쪽 head를 빈 문자열로 대체해 비교했으므로 빈 값끼리도 일치했다.

영향 범위는 OpenCode repository dispatch, Strix의 기존 job rerun,
Strix repository dispatch 직전의 공통 guard다. 이번 수정은 이 세 경로의
새 실행 요청을 막는 조건만 다룬다. 앞서 수행되는 stale-run cleanup의 순서나
cancellation 정책은 바꾸지 않는다.

## 수정

- GraphQL 공통 fragment가 PR state를 실제로 요청한다.
- REST fallback은 원본 state를 대문자로 보존하고, 누락은 빈 값으로 남긴다.
- guard는 정확히 한 PR, 명시적 `OPEN`, 양쪽의 문자열 타입 40자리 hex SHA,
대소문자를 제외한 동일 head를 모두 요구한다.
- 기존 정상 fixture는 `OPEN`을 명시한다. 누락 사례를 정상 fixture로 대체하지 않는다.

토큰, 권한, trigger, queue, concurrency, dispatch payload는 변경하지 않았다.
CodeQL primitive도 추가하지 않았다. 조회 직후 PR 상태가 바뀔 수 있는 경쟁 조건과
중복 전송의 원자성은 여전히 미해결이며, 이 guard는 exact-once 보장이 아니다.
Cross-repo target callback의 Actions-write 권한도 별도 미해결 조건이다.

## 회귀 검증

`tests/test_scheduler_live_dispatch_guard.py`는 실제 guard와 세 caller를 실행하고
외부 API 및 실행 요청만 대체한다. 누락·빈 값·CLOSED·MERGED·UNKNOWN은 dispatch와
rerun에 도달하지 않아야 하며, OPEN의 정상 경로는 계속 도달해야 한다.
별도 사례가 빈 값, 잘못된 길이, 비-hex, 비문자열, 서로 다른 SHA를 거부하고
GraphQL 실제 query와 REST fallback의 state 전달을 확인한다.

Production 수정 전 새 회귀는 17 failed / 19 passed였다. 이후 실제 live head만
잘못된 사례 두 건도 추가했다. 최종 관련 5파일은 `-W error`를 적용해 정상 환경에서
380 passed, `GITHUB_ACTIONS=true` 환경에서도 380 passed를 확인했다.
검증 명령은 다음과 같다.

```sh
python -m pytest -q -W error tests/test_scheduler_live_dispatch_guard.py tests/test_pr_review_merge_scheduler.py tests/test_strix_rerun_job_selection.py tests/test_repository_branch_coverage_review_schedulers.py tests/test_pr_review_fix_scheduler_rest_workflow_identity.py
```

로컬 회귀 통과는 실제 GitHub dispatch, protected merge, 대상 job 복구의 증거가 아니다.
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,16 @@ burst each new head cancels the previous scan; the burst's final head is
scanned, and the weekly full-tree `schedule` scan (unique run id, never
cancelled) is the floor under a sustained burst.

**Amendment (2026-09-07).** The workflow group now carries two independent
identities. Pull-request work includes the exact head, while `closed` uses a
run-unique suffix; with cancellation disabled for PR events, Ready, Draft, and
same-head dispatch admission preserve an executing verdict, while a new head
or closed cleanup does not wait behind it. Push work remains grouped by
protected ref and is the only event class with `cancel-in-progress` authority.
The replacement provider waits for live-revalidated cleanup to finish, and
that cleanup enumerates both native and dispatched PR runs. No elapsed-time
condition can cancel provider work.

## Verification

- `python -m pytest -q tests/test_pr_review_merge_scheduler.py -k 'startup_failures or startup_failure'`
Expand Down
Loading
Loading