Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
22 changes: 15 additions & 7 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 }}
Comment thread
seonghobae marked this conversation as resolved.
Outdated
# 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 @@ -243,7 +254,7 @@ 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.action == 'synchronize' || github.event.action == 'closed')
# 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 Down Expand Up @@ -289,18 +300,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 Down Expand Up @@ -338,7 +347,6 @@ jobs:
)) 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 Down
20 changes: 20 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,23 @@
### 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;
a closed event uses its unique run id for the same reason. Draft transitions
preserve the current scan, while the live-revalidated cleanup job cancels
only verified superseded heads or a closed pull request. 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,14 @@ 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.
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
76 changes: 76 additions & 0 deletions docs/doctoring/strix-rerun-job-identity-binding.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# Strix 재실행 대상 job의 신원 결합

## 확인한 결함

현재 main `ee5567f7b15f0441a61ec2435415603b9518f1c6`과 #1902의
`951d0ecd1b5398a9eac293a13bba220a6528df24`에서 Strix 재실행 선택 경로를 비교했다.
관련 core 차이는 이전 OPEN-state 수리뿐이었다. 이번 작업은 951 위에서 진행하며
main을 merge하거나 다른 세션의 workflow 변경을 덮어쓰지 않았다.

기존 선택기는 check의 details URL에서 job ID만 추출했다. 직전 live guard는
PR snapshot이 최신인지 확인했지만, 선택한 job이 그 PR head를 스캔했는지는
확인하지 않았다. 로컬 mock-only 회귀에서 실제 caller와 rerun wrapper를 실행한
결과 8 failed / 2 passed였다. 실패 사례는 job/run 조회 없이 mock POST에 도달했다.
실제 GitHub 위조 요청이나 job 재실행을 실행한 결과가 아니다.

## 최소 수리와 보류 조건

기존 selector와 API 조회 helper를 유지하고 Strix rerun 분기에 검증 하나를 추가했다.
GraphQL과 REST 정규화는 selected check의 database ID를 보존한다.

- selected check URL은 같은 repo의 정확한 run/job을 지정해야 한다.
- 실제 job의 ID, run ID, 이름, 완료 상태와 재실행 가능한 실패 결론을 확인한다.
현재 허용 결론은 failure, cancelled, timed_out이다. 다른 결론은 자동 재실행을 보류한다.
- job이 가리키는 실제 check ID가 selected check와 같아야 한다. Check publisher는
github-actions여야 하며 check suite와 run의 연결도 일치해야 한다.
- 실제 run과 workflow 조회는 같은 repo의 `.github/workflows/strix.yml`,
`Strix Security Scan` 이름을 확인한다.
- pull_request_target은 정확히 하나의 PR association, base/head repository,
association의 PR base/head SHA, event에서 생성한 정확한 run-name이 모두 일치해야 한다.
job/run의 top-level head_sha가 base SHA인 정상 사례를 허용한다. 이 필드를
PR head로 간주하지 않는다. 누락되거나 상충하는 repository 식별자는 거부한다.
- repository_dispatch는 제어 코드의 실행 SHA만으로 target head를 증명할 수 없다.
이 경로에는 인증된 target receipt를 소비하는 계약이 없으므로, 제목이 맞더라도
자동 재실행을 보류한다. push 등 다른 event도 새로 허용하지 않는다.
- 검증 전후 live PR의 base/head SHA를 다시 확인한다. 같은 head가 다른 base로
retarget된 경우에도 과거 run을 재사용하지 않는다. API 실패나 불완전한 metadata는
`identity_unverified`로 보류하고 새 dispatch로 우회하지 않는다. 세 상위 caller도
이를 실행 완료가 아닌 wait로 보고한다.

## 검증과 한계

`tests/test_strix_job_binding.py`는 실제 REST 정규화, selector, live guard,
dispatch caller, actor 검사, rerun wrapper를 실행한다. 외부 명령은 모두 mock 경계에서
차단한다. 정상 대조군은 top-level base SHA와 PR head SHA, REST repository URL 형식을
포함한다. 음성 사례는 stale·상충·누락·다른 repo/workflow/publisher/event·API 실패 및
검증 중 base/head 이동을 포함한다. 정상 사례는 네 metadata GET과 단일 mock POST를 요구한다.

기존 state-only, 명령형식, sibling 선택 테스트 세 곳은 각자의 검증 대상을 유지하도록
새 guard만 국소적으로 대체했다. 신원 결합 자체는 별도 회귀에서 실제 구현을 사용한다.

권한, 토큰 선택, actor allowlist, queue, concurrency, CodeQL primitive는 변경하지 않았다.
조회와 POST 사이의 원자성, cross-repo callback 권한, hosted 복구는 해결했다고 주장하지
않는다. 신뢰할 provenance가 없는 역사적 run은 자동 복구가 보류될 수 있다.

## Draft/Ready 수명주기 증거 보존

2026-09-07의 current-head 재검토에서 별도의 실행 전 취소 결함을 확인했다. PR #1706의
Strix run `34068478185`와 PR #1150의 run `34067942252`는 같은 head에서 실행 중이었지만,
Draft/Ready 상태 전환이 PR 단위 workflow concurrency group에 다시 들어오자 provider
실행 중 취소되었다. replacement run은 queue에만 남았고 terminal Strix verdict와
publisher evidence는 생성되지 않았다. PR #1999 자체의 run `34067362987`도 `Run Strix
(quick)` 단계에서 취소되고 publisher job이 취소되어 같은 실패 형태를 재현했다.

Workflow-level `cancel-in-progress`는 `push`에서만 참이고 PR concurrency group은 exact
head까지 포함한다. Ready와 Draft는 같은 head group을 공유하므로 실행 중인 증거를 무효화하지
않고, 새 head의 `synchronize`는 이전 head group 뒤에 대기하지 않는다. `closed` event는
고유 run id group을 사용해 종료 대상 scan 뒤에 막히지 않고 metadata-only
`cancel-superseded-pr-runs` job을 실행한다. 이 job은 live PR을 재조회하고 각 mutation 직전
head와 상태를 다시 검증하므로, `synchronize`의 이전 head와 실제 closed PR만 취소한다.
같은 protected ref의 새 push만 superseded push scan을 취소한다. Provider 실행에는
elapsed-time cancellation을 추가하지 않았다.

회귀 계약은 workflow-level non-cancellation을 직접 파싱하고, 기존 subprocess fixture로
head가 전진한 뒤에는 취소하지 않음, selection 뒤 재검증 실패 시 mutation하지 않음,
검증된 Draft 전환에서는 current scan을 보존하고 closed event는 독립 group에서 cleanup을
실행함을 함께 증명한다.
Loading
Loading