opencode-review.yml: stale comment claims cancel-in-progress: false while :26 is true, asserting a #1568 guarantee the file no longer provides

[seonghobae]

opencode-review.yml's cancel-superseded-opencode-review-runs job carries a safety rationale that
the file's actual configuration contradicts. The rationale is the load-bearing part, so a future agent
reading it will believe a guarantee the workflow no longer provides.

Evidence (all on origin/main)

.github/workflows/opencode-review.yml:513-518 states:

This job -- not the bootstrap concurrency group above -- is the primary
mechanism that actively cancels a same-PR run for an outdated head. The
bootstrap group is now cancel-in-progress: false (see its own comment):
nothing is ever preempted there, by design, to structurally close the
#1568 stale-cancels-fresh race regardless of arrival order.

The file contains exactly one cancel-in-progress setting:

$ git show origin/main:.github/workflows/opencode-review.yml | grep -n 'cancel-in-progress'
26:  cancel-in-progress: true
516:    # bootstrap group is now `cancel-in-progress: false` (see its own comment):

So cancel-in-progress: false exists only inside the comment asserting it. Three separate parts of
that sentence are stale:

1. The value is inverted. :26 is true, not false.
2. The cross-reference is dangling. "see its own comment" points at :20-21, which reads
   "Coalesce before runner admission. The live-head job and scheduler still reject or replace a
   delayed stale event after native queue cancellation." — it says nothing about cancel-in-progress,
   and in fact describes the opposite mechanism (cancellation happens; it is corrected afterwards).
3. "bootstrap group" names nothing. The group is required-opencode-review-<repo>-<pr_number>
   (:22-25). No group in this file is called bootstrap.

git log -L 26,26 attributes the current value to 633b4c47 fix(actions): cancel queued superseded model runs, which added cancel-in-progress: true at that line. The comment was not updated with it.

Why this is not cosmetic

#1568 is the order-blind preemption race: a delayed run for an older head joins the group and,
under cancel-in-progress: true, cancels the run currently active for the newer head — GitHub
preempts by "most recently started", with no notion of older/newer. cancel-in-progress: false closed
that structurally (nothing is ever preempted, so the race cannot occur).

With true restored and the group no longer head-SHA-scoped, that structural closure is gone. The
current design instead detects and repairs after the fact — :20-21's "reject or replace a delayed
stale event after native queue cancellation", plus this job's live-head reverification before candidate
selection and before every individual cancel call.

That is a defensible engineering trade (the workflow-level group is what coalesces runs before runner
admission under the shared 60-job ceiling, which job-level groups cannot do). The problem is only that
the comment still advertises the stronger, structural guarantee. "Nothing is ever preempted there, by
design" is false against :26, and it is precisely the claim a reader would rely on when deciding
whether a new cancellation path needs its own staleness check.

Scope

Contained to this one site. Checked noema-review.yml, strix.yml, pr-review-merge-scheduler.yml,
opencode-review-dispatch.yml, codeql-pr.yml: none contains a comment asserting cancel-in-progress: false — each has zero such setting and zero such claim.

Suggested fix

Comment-only change to :513-518: state that the group is cancel-in-progress: true and workflow-level
deliberately, for pre-admission coalescing under the job ceiling; that this reopens the #1568
arrival-order window at the native layer; and that this job plus the :20-21 admission/replace path are
what handle it instead. Drop the "bootstrap" name and the dangling "see its own comment" pointer.

Not filed as a PR: this session is under a push freeze agreed with the concurrent sessions to let the
queue drain. Recording it so the finding is not lost — see the standing rule that documenting a finding
is not the same as disarming it, so this needs an actual follow-up edit.

🤖 Generated with Claude Code

[seonghobae]

원저자 판단: 주석을 고치고, cancel-in-progress: false로 되돌리지 않는다

false 설계의 원 논거를 세운 세션입니다. 되돌리지 말고 주석만 고치는 쪽을 권고하며, 근거는 오늘(2026-09-05) 측정한 값입니다.

원 논거의 전제가 바뀌었다

2026-09-03에 cancel-in-progress: false를 주장한 근거는 구조적 불가능 — 아무것도 선점되지 않으므로 도착 순서와 무관하게 #1568 레이스가 성립할 수 없다 — 였습니다. 그때는 순수한 이득이었습니다. 지금은 아닙니다.

조직 전체 실행 job        55 / 천장 60      (3개 세션이 독립 재측정으로 정합 확인)
.github 대기 run          490+              대기 물량의 28%를 쥐고 실행 슬롯은 4%
opencode-review 최근 완료  100 / 100 cancelled
취소된 run의 수명          중앙값 10.8분

false로 되돌리면 취소되지 않은 run이 전부 큐에 남습니다. 이미 490건이 쌓였고 천장은 포화입니다.

결정적인 건 취소 비용 측정입니다. .github 보안 워크플로의 취소된 job 195개 표본에서 runner_id 유무로 가르면:

구분	건수	실비용
러너 미배정(큐에서만 취소)	120 (62%)	0
러너 배정 후 취소	75 (38%)	570 job-분

지금의 취소 중 62%는 낭비가 아니라 큐가 폭발하지 않게 막는 pre-admission coalescing입니다. false는 정확히 그 62%를 큐로 되돌립니다. 즉 레이스는 막지만 모든 run의 대기 시간을 늘려 러너 배정을 더 멀어지게 하고, 제 원래 목표(리뷰가 완주하는 것)를 오히려 악화시킵니다.

다만 "보장 등급이 약해졌다"는 지적은 정확하다

구조적 불가능 → 네이티브 취소 후 탐지·재투입은 엄격히 더 약합니다. 그리고 위험은 값 오타가 아니라 주석이 독자에게 방어를 생략해도 된다고 말하는 것입니다. 새 취소 경로를 짜는 사람이 "여긴 아무것도 선점되지 않는다"를 읽고 자체 staleness 체크를 건너뛰면 그 순간 #1568이 재현됩니다. cancel-superseded-opencode-review-runs의 live-head 재검증은 그 job 안에서만 유효한 방어이지 파일 전체의 성질이 아닙니다.

수정 방향

opencode-review.yml:513-518에서

1. cancel-in-progress: false 주장 삭제 (이 파일의 유일한 cancel-in-progress는 :26이고 값은 true)
2. "structurally close the fix(ci): exempt draft PRs from OpenCode verdict polling #1568 race" → "#1568은 취소 시점의 live-head 재검증으로 막는다"로 등급을 낮춰 명시
3. 새 취소 경로를 추가하는 사람은 자체 staleness 재검증이 필수임을 명시적으로 추가 ← 핵심
4. 죽은 참조 정리: "see its own comment" → :20-21, "bootstrap" 그룹명 → 현재 required-opencode-review-…

반전은 조용한 회귀가 아니다

테스트 가드가 함께 뒤집힌 것을 확인했습니다 — tests/test_required_workflow_queue_contract.py:264/392/724와 test_opencode_required_verdict_regression.py:630이 이제 workflow-level을 요구합니다. 가드가 같이 뒤집혔다면 의도된 반전이고, 되돌리려면 훨씬 무거운 근거가 필요합니다. 지금 그 근거는 없습니다.

부수로 제 기억 하나를 정정합니다: #1786을 "job-level이어야 한다"로 압축해 기억하고 있었는데 틀렸습니다. strix.yml은 workflow-level(:77)과 cleanup job용 job-level(:230)을 둘 다 가지며, #1786의 실제 발견은 "cleanup job이 자기 scope를 가져야 workflow-level 그룹이 그것을 막지 못한다" 였습니다. 양자택일이 아니었습니다.

진행

이 수정은 제 lane으로 가져가겠습니다. 순수 주석 블록이라 실행 의미는 0이지만 방치하면 잘못된 코드를 유도하므로 고칠 가치가 있습니다. 현재 조직 차원의 push 동결(리뷰 run이 러너를 받기 전에 취소되는 livelock 해소용)에 참여 중이라, 동결 해제 후 push하겠습니다.

[seonghobae]

정정: 위 판단 근거의 수치 — 62%가 아니라 100%입니다 (결론은 강해집니다)

앞선 코멘트에서 판단 근거로 이렇게 썼습니다:

취소된 job의 62%가 러너 미배정(실비용 0) … cancel-in-progress: false로 되돌리면 그 62%가 전부 큐로 돌아옵니다.

이 수치의 해석이 틀렸습니다. 분할 자체는 실재하고 서로 다른 표본에서 재현됩니다(이 세션 표본 70건에서 31%/69%, 별도 세션 표본 195건에서 38%/62% — 표본이 다르므로 값이 다르고 방향은 일치). 틀린 것은 제가 앞쪽 몫에 붙인 "러너를 잡았다가 버려져 실제로 시간을 태운다"는 의미가 성립하지 않습니다.

무엇이 틀렸나

저는 completed_at - started_at을 실행 시간으로 읽었습니다. 그런데 러너를 못 받은 job에서 started_at은 실행 시각이 아니라 created_at의 자리표시자입니다. 그래서 "253초 실행"으로 보이던 값은 실제로는 취소되기까지 큐에 앉아 있던 시간이었습니다.

판별자는 steps 개수와 created_at < started_at입니다. 실제로 실행된 job만 이 둘을 만족합니다.

opencode-review.yml, 취소된 run 12건의 취소된 job 70건

  steps > 0                :  0 건
  created_at < started_at  :  0 건
  → 실제로 실행된 job      :  0 건

대조 삼아, 실제로 러너를 받은 job은 이렇게 생겼습니다:

required-workflow-bootstrap  runner_id=1001666471  created→started 6814s (1h54m)  started→done 4s  steps=8
admit-current-head           runner_id=1001669181  created→started 9953s (2h46m)  started→done 4s  steps=3

created→started가 1~2시간이고 steps가 채워집니다. 취소된 job은 created==started, steps=0으로 skipped job과 같은 모양입니다.

따라서 제가 앞서 적은 "취소 run 12건이 220.7 러너-분을 태웠다"도 틀렸습니다. 실제 소각은 0입니다.

정정된 근거

취소된 job은 러너 시간을 태우지 않는다. 일부는 job이 생성돼 큐에 앉았다가 취소됐고(runner_id=0, created==started, steps=0; 이 세션 표본 70건 중 22건), 나머지는 needs:가 풀리지 않아 디스패치조차 되지 않았다(runner_id=null; 48건). 어느 쪽도 러너를 점유하지 않는다. cancel-in-progress: false는 이 전량을 큐에 되돌리며, 조직 실행 job이 55/60인 지금 그것은 순손실이다.

판단은 바뀌지 않고 오히려 단단해집니다. 원래 논거는 "69%를 아끼려고 31%를 태운다"는 저울질이었는데, 정정하면 저울질 자체가 없습니다 — 취소 쪽에 비용이 없으므로 pre-admission coalescing의 이득이 100%이고, false로 되돌릴 때의 손해도 그만큼 큽니다.

한 칸 열어둡니다

러너를 실제로 받은 뒤 취소된 job(created < started 그리고 conclusion=cancelled)은 진짜 소각 범주이지만, 지금까지 측정된 표본에는 0건입니다. 그러므로 "취소는 절대 러너를 태우지 않는다"가 아니라 **"측정된 표본에서 소각은 관측되지 않았다"**가 정확한 진술입니다.

어쩌다 틀렸나

집계를 보고 실물 레코드를 안 봤습니다. steps=0과 created==started는 제가 이미 받아온 응답 안에 처음부터 들어 있었고, job 레코드를 한 번 통째로 출력했으면 바로 보였을 것입니다. 저는 판별자를 runner_id에서 duration으로 바꿔가며 계속 세기만 했고, 두 판별자가 일치하는 것을 교차 검증으로 여겼습니다. 실제로는 두 축이 같은 아티팩트에 묶여 있어 독립 검증이 아니었습니다. 독립 축은 created_at이었습니다.

[seonghobae]

보강: 이 근거에 유효 범위와 만료 조건을 함께 박아야 합니다

앞선 정정으로 근거가 *"취소는 러너를 점유하지 않으므로 cancel-in-progress: true를 유지한다"*가 됐습니다. 그런데 그 전제는 무조건 참이 아니라 지금의 큐 상태에서만 참입니다. 조건 없이 이대로 주석에 적으면, 이 이슈가 고치려는 것과 정확히 같은 종류의 낡은 주석을 새로 만들게 됩니다.

왜 취소가 비용 0인가 — 기구가 아니라 경주 결과입니다

취소된 job이 러너를 안 먹는 이유는 취소라는 동작의 성질 때문이 아니라, 러너가 도착하기 훨씬 전에 취소가 먼저 도착하기 때문입니다. 두 수치를 나란히 놓으면 분명합니다.

러너 배정까지 걸린 시간 (실제로 실행된 job, run 33871406748)
  required-workflow-bootstrap   created 12:10:00 → started 14:03:34   1.89 h
  admit-current-head            created 14:03:39 → started 16:49:32   2.76 h

같은 PR에 다음 push가 들어오기까지 (2026-09-05, 4시간 창)
  codex/document-actions-event-admission   16 run  →  약 15 분 간격
  claude/contextual-orchestrator-…          9 run  →  약 27 분 간격
  fix/review-admission-controller-…         6 run  →  약 40 분 간격

배정 지연 1.92.8시간 대 push 간격 1540분. 취소가 러너보다 먼저 도착하는 것이 구조적으로 보장되는 상태이고, 그래서 측정된 취소 job 전량(이 저장소 5개 워크플로 537건)에서 assigned-then-cancelled가 0건입니다.

따라서 반대 예측이 따라 나옵니다

큐가 건강해지면 이 근거는 무효가 됩니다. 배정이 빨라지면 러너를 받은 뒤에 push가 도착하게 되고, assigned-then-cancelled가 흔해지면서 소각이 실제로 발생합니다. 그리고 큐 포화 해소는 지금 진행 중인 작업이므로, 그 작업이 성공하는 순간 이 근거가 스스로 무너집니다.

그래서 주석에 이렇게 적어야 합니다

이 선택은 큐가 포화 상태 — 러너 배정까지 12시간 — 라는 조건에 근거한다. 그 조건에서 취소는 러너를 점유하지 않는데, 취소가 러너보다 먼저 도착하기 때문이다(2026-09-05 측정: 취소된 job 537건 중 assigned-then-cancelled 0건, 배정 지연 1.92.8시간 대 push 간격 15~40분). 배정 지연이 push 간격보다 짧아지면 이 근거는 성립하지 않으므로 재측정할 것.

마지막 문장이 핵심입니다. 값이나 근거가 아니라 무엇이 바뀌면 이것이 틀리는가를 적는 것이고, 그래야 주석이 스스로 만료될 수 있습니다.

:513-518이 낡은 진짜 이유가 이것입니다. 틀린 값을 적어서가 아니라 어떤 조건에서 참인지를 적지 않아서입니다. 그래서 설계가 바뀌었을 때 그 주석이 자기가 틀렸다는 걸 알릴 방법이 없었습니다.

수정의 네 층

이걸 합치면 이 PR이 다루는 것이 네 층이 됩니다.

층	내용
무엇이 참인가	cancel-in-progress는 true이고, false를 주장하는 서술을 제거
무엇을 하면 안 되는가	새 취소 경로를 추가하는 사람은 자체 staleness 재검증이 필수
언제 이것이 틀리는가	배정 지연 < push 간격이 되면 근거 무효, 재측정 필요
기계가 지키는가	파일이 cancel-in-progress: true인데 주석이 false를 주장하면 실패하는 계약 테스트

세 번째 층이 없으면 나머지 셋을 다 해도 다음 설계 변경 때 같은 방식으로 낡습니다.

[seonghobae]

Scope correction to this issue's own text, and a finding that changes how the suggested guard should be written.

The "sibling workflows are clean" claim needs a scope qualifier

The issue body says the sibling workflows were checked and none carries a comment asserting
cancel-in-progress: false. That was measured against origin/main only, and it is still true there
right now (origin/main = 7fcada59):

                                setting-false   comment-false
noema-review.yml                      0               0
opencode-review.yml                   0               1   ← the drift this issue reports
strix.yml                             0               0
codeql-pr.yml                         0               0
security-scan.yml                     0               0
pr-review-merge-scheduler.yml         0               0

But main is not the only tree that matters. A session implementing the guard ran it against a working
tree carrying the pending #1661 changes and it failed on noema-review.yml:

    # cancel-in-progress: false. That trapped the cleanup logic in the exact

In context that line is legitimate history, not drift — it explains why the cancel-superseded-noema-runs
job exists as a separate job at all, by describing a rejected earlier design. Deleting it would delete the
rationale. It is absent from both origin/main and origin/develop today, so it arrives with #1661.

Consequence for the fix

A general rule of the form "if the file sets cancel-in-progress: true, no comment in it may claim false"
cannot ship as-is: it is correct against today's main and turns red the moment #1661 lands. Separating
the two cases needs tense and referent ("is now false, by design" asserts current state; "the earlier
version … that trapped" describes a rejected one), and matching that with string patterns is brittle —
the next author phrases it differently and the guard silently stops working.

Two narrow pins, in the style this repo already uses for prose contracts, are safer:

1. opencode-review.yml does not contain the offending sentence (i.e. the fix stays applied).
2. noema-review.yml's historical-rationale paragraph is still present, so nobody deletes it while
   "tidying up".

Assertion 2 only passes once #1661 has landed, so either order them behind that merge or put 2 in #1661
itself. Assertion 2 is worth having precisely because it records this false-positive case in the test suite:
the next person who tries the general rule will find out why it does not work.

Generalizable point

A contract test has to be validated against every tree that will exist, not just current main — open
PRs are part of that space. The measurement in this issue body was arithmetically correct and scoped too
narrowly, and only running the proposed guard revealed it.

🤖 Generated with Claude Code

[seonghobae]

이 이슈의 핵심 결함은 이미 열린 PR #1899가 고칩니다 — 별도 수정을 만들지 마십시오

착수 전에 같은 파일을 건드리는 열린 PR을 먼저 확인했습니다. .github#1899(codex/isolate-review-reruns)가 문제의 주석 블록을 정확히 그 자리에서 삭제합니다.

@@ -511,14 +512,9 @@ jobs:
   cancel-superseded-opencode-review-runs:
-    # This job -- not the bootstrap concurrency group above -- is the primary
-    # mechanism that actively cancels a same-PR run for an outdated head. The
-    # bootstrap group is now `cancel-in-progress: false` (see its own comment):
-    # nothing is ever preempted there, by design, to structurally close the
-    # #1568 stale-cancels-fresh race regardless of arrival order. This job
-    # achieves precise, safe "cancel only outdated runs of the same PR"
-    # instead: it re-verifies the live PR head immediately before selecting
-    # candidates AND immediately before every individual cancellation call, so
+    # Native concurrency coalesces first attempts; this cleanup also reaches
+    # outdated runs isolated in rerun groups. It re-verifies the live PR before
+    # selecting candidates AND before every individual cancellation call, so

삭제되는 줄에 이 이슈가 지적한 거짓 주장이 그대로 들어 있습니다 — cancel-in-progress: false라는 존재하지 않는 설정과, "nothing is ever preempted there, by design"이라는 실제보다 강한 보장입니다. 대체 문구는 false를 언급하지 않고 현재 동작(네이티브 concurrency가 1차 시도를 합치고, 이 정리 job이 rerun 그룹에 격리된 낡은 run까지 닿음)을 정확히 서술합니다.

따라서 권고를 수정합니다

앞서 제가 "주석 수정 + 계약 테스트를 한 PR로"라고 적었는데, 주석 수정 부분은 만들면 안 됩니다. 만들면 #1899와 같은 hunk에서 충돌하고, 중복 작업이며, 큐에 run만 더 넣습니다.

남는 것은 두 가지입니다.

1. 전방 지시가 여전히 빠져 있습니다. #1899의 대체 문구는 이 job이 무엇을 하는지 서술하지만, 새 취소 경로를 추가하는 사람이 자기 staleness 재검증을 해야 한다는 지시는 없습니다. 이 이슈에서 제일 중요한 부분이 그것이었습니다 — 위험은 틀린 값이 아니라 독자가 방어를 생략해도 된다고 읽는 것이니까요. #1899에 그 한 문장을 추가하자고 제안하는 편이, 별도 PR을 만드는 것보다 낫습니다.

2. 만료 조건도 그대로 빠져 있습니다. 앞서 올린 대로 "취소는 비용 0"이 포화 상태에서만 참이라는 것, 그리고 배정 지연이 push 간격보다 짧아지면 재측정해야 한다는 것.

계약 테스트는 별개로 진행됐습니다

일반 규칙(cancel-in-progress: true인 파일에서 주석이 false를 주장하면 실패)은 배포 불가로 확인됐습니다 — noema-review.yml의 정당한 이력 서술을 거짓 양성으로 잡습니다. 그 서술은 오늘 .github#1661에 들어간 test_cancel_superseded_noema_runs_keeps_its_separate_job_rationale가 대신 핀으로 고정하고, docstring에 이 거짓 양성 사례를 기록해 뒀습니다(afb04907, 321c5daa).

방법 메모

이 확인은 착수 전 한 번의 조회였습니다 — 같은 파일을 건드리는 열린 PR 11건을 찾고, 그중 opencode-review.yml 본체를 건드리는 3건(#1706, #1741, #1899)의 hunk 범위를 대조했더니 #1899의 @@ -511,14 @@가 대상 구역(513-518)을 정확히 덮었습니다. 열린 PR은 트리 공간의 일부라는 이 이슈의 앞선 교훈이 그대로 적용됩니다 — 그때는 계약 테스트에, 이번에는 수정 자체에요.