Strix run-name is an unenforced parse contract: any suffix makes the scheduler preserve every stale run and stop dispatching

While triaging PR #1009 I found a coupling that would disable Strix dispatch for every open PR in the organization if that branch merged as written. Filing separately because the defect is a latent fragility in main, independent of whether #1009 ever lands.

The coupling

.github/workflows/strix.yml sets a run-name, and scripts/ci/pr_review_merge_scheduler_core.py parses that name back out to recover the reviewed head SHA:

# pr_review_merge_scheduler_core.py:3487-3493
prefixes = tuple(f"{title} {repo}#{number}@" for title in sorted(titles, key=len, reverse=True))
prefix = next((c for c in prefixes if display_title.startswith(c)), None)
if prefix is None:
    raise ValueError("repository_dispatch run has no trusted target identity")
return validate_git_sha(display_title.removeprefix(prefix)).lower()

The prefix ends at @, so everything after @ must be a bare 40-hex SHA and nothing else. That contract lives entirely in the string format — nothing enforces it, and nothing in strix.yml mentions it.

Why it matters

PR #1009 appends a merge-state suffix to that same run-name:

… #<n>@${{ …pr_head_sha… }}:${{ github.event.client_payload.merge_state || 'open' }}

Executed against origin/main's real function:

Strix Security Scan org/repo#7@aaaa…aaaa        → OK
Strix Security Scan org/repo#7@aaaa…aaaa:open   → ValueError: invalid git sha: 'aaaa…aaaa:open'

_review_run_still_superseded (core:3506-3517) catches ValueError, prints ::warning::Preserving review run … failed closed, and returns False — meaning not superseded. Failing closed is right in isolation; the consequence is not. A stale Strix run can then never be proven stale, so it is preserved, folded into the current run refs, and dispatch_strix_evidence answers already_running rather than dispatching. Every open PR in every target repository, from one string edit in a different file.

The class of defect

This is the shape already catalogued as silently-inactive required checks: a guard that looks fully configured while a narrower condition quietly never matches. The novelty is the trust boundary being a display string — the producer (strix.yml) and the consumer (the scheduler core) have no shared constant, no test spanning both, and no comment on either side naming the other.

Two cheap fixes, not exclusive:

Make the parse tolerant. validate_git_sha(display_title.removeprefix(prefix).split(":", 1)[0]) — a suffix stops being fatal, and the reviewed head is still recovered exactly.
Pin the contract from both ends. One test asserting strix.yml's run-name ends with the head-SHA expression and that _review_run_target_head accepts what that template produces. Today each side is tested only against its own idea of the format.

A third, worth considering separately: the except (…ValueError…) in _review_run_still_superseded cannot distinguish "the API told us something inconsistent" (preserve — correct) from "we cannot parse our own run-name" (a bug in us). The second deserves a distinct, louder signal than a ::warning:: that scrolls past.

Status

Not fixed here — the parse is in the scheduler core, which several sessions are actively changing, and the run-name is in a required workflow. Recorded so the fix is deliberate rather than a side effect of someone else's merge. No repository state was changed while finding this; PR #1009 was left byte-identical.

🤖 Generated with Claude Code

Independently verified on main at f2f91b80 (a second reading, not a relay):

Producer: .github/workflows/strix.yml:2-5 — run-name: Strix Security Scan <target_repository>#<pr_number>@<…>.
Consumer: scripts/ci/pr_review_merge_scheduler_core.py:3486-3493 — builds f"{title} {repo}#{number}@" prefixes, strips the matching one from display_title, and passes the remainder to validate_git_sha; anything after @ that is not a bare SHA raises ValueError.
Catch: :3511-3516 — except (KeyError, RuntimeError, TypeError, ValueError) → ::warning::Preserving review run … live stale-run revalidation failed closed → return False.
Coupling test: none found — the only run_name occurrences under tests/ are runpy.run_path(..., run_name="__main__").

So the contract lives only in the string format, and a producer-side change (as #1009's branch makes, appending :open) fails closed into "preserve", which is the state that blocks dispatch_strix_evidence with already_running. Fix shape I would back: one shared constant for the separator/format used by both sides, one round-trip test that renders the run-name template and parses it, and a split of that except so a ValueError from our own format is loud rather than preserved. Not touching the core while other sessions have it open.

같은 뿌리가 이 파일에 넷입니다 — 개별 수정이면 다섯 번째가 나옵니다

다른 세션이 already_running 이 프로덕션에서 한 번도 발동한 적이 없다는 것을 찾았고(#1983), 소스에서 확인했습니다. 원인이 이 이슈와 같습니다.

core.py:3257   if run_name != workflow and run_name not in workflow_aliases: continue

name 정확 일치인데, 그 매처가 찾는 세 워크플로가 전부 run-name: 을 답니다(opencode-review.yml·opencode-review-dispatch.yml·strix.yml 각 1회). run-name 이 있으면 GitHub 은 name 에 렌더된 제목을 넣습니다:

"Strix Security Scan ContextualWisdomLab/.github#1984@71bda03067a"

그래서 그 비교는 참이 될 수 없습니다.

전수 조사

run.name / display_title 을 안정 식별자로 쓰는 지점입니다.

위치	형태
`3200`	`run_data.get("name") != workflow` — 정확 일치
`3257`	`run_name != workflow and not in aliases` — 정확 일치 (#1983)
`3263·3268·3273`	`display_title.startswith(prefix)` → `removeprefix`
`3354·3356·3361`	동일 형태
`3488·3492·3495`	동일 형태 (이 이슈)
`3802·3804`	`name == workflow` 및 `display_title.startswith(...)`

대안 필드는 이미 쓰이고 있습니다 — run_data.get("path") 3회, get("workflow_id") 1회. 그리고 한 곳은 이미 올바른 형태입니다:

# core.py:3062
workflow_key = str(run.get("workflow_id") or run.get("path") or run.get("name") or "")

제안

path 또는 workflow_id 기반으로 옮기면 3200·3257·3802 가 함께 닫히고 이 이슈의 절반도 닫힙니다. 개별 수정으로 가면 같은 부류의 다섯 번째가 나옵니다.

범위를 넓히면 리뷰가 무거워지므로 판단은 소유자 몫입니다. 최소한 각 수정의 커밋 메시지에 "같은 부류가 N 곳" 만 남겨도 다음 사람이 전수를 다시 찾지 않아도 됩니다.

🤖 Generated with Claude Code

The same root cause silently disables a policy boundary in the REST fallback

Following up on the run.name/display_title-as-identifier survey above. One of the sites I listed —
pr_review_merge_scheduler_core.py:1250 — turns out to be materially worse than the display_title
prefix-parse sites, because it defeats a fail-closed guard rather than mis-parsing a title.

What the code promises

fetch_workflow_names_by_check_suite_rest (:1224) builds {check_suite_id: workflow_name} from REST
run.name and feeds it to rest_check_node (:1273-1276), which reshapes it into GraphQL's
checkSuite.workflowRun.workflow.name. Its docstring states the contract:

allowing the REST fallback to retain the same workflow-level policy boundary as the GraphQL path.

What the two APIs actually return

Joined on check_suite_id for .github head ab457b69, all nine workflows on that head:

check suite	workflow	GraphQL `workflow.name`	REST `run.name`
92255861868	`opencode-review.yml`	`Required OpenCode Review`	`Required OpenCode Review ContextualWisdomLab/.github#834@ab457b69…`	diverges
92255861982	`noema-review.yml`	`Required Noema Review`	`… ContextualWisdomLab/.github#834@ab457b69…`	diverges
92255862141	`strix.yml`	`Strix Security Scan`	`… ContextualWisdomLab/.github#834@ab457b69…`	diverges
92255861906	`pr-review-merge-scheduler.yml`	`Required PR Review Merge Scheduler`	identical	same
92255865985 &c.	`sast-semgrep`, `python-security`, `security-scan`, `codeql-pr`, `agent-review-runtime-quality-ci`	—	identical	same

The split is exactly run-name: presence — the three workflows that declare one all diverge, the six
that don't are all identical. GraphQL returns the Workflow object's static name:; REST returns the
rendered run title.

What that does to the consumers

They compare the value by equality against static names:

is_strix_context (:1661) — workflow_name in {"Strix Security Scan", "Strix"} is False, and
the second clause node.get("name") == "strix" and workflow_name in {None, REST_UNKNOWN_GITHUB_ACTIONS_WORKFLOW}
is also False because the name is present. A genuine Strix check run is not recognised as Strix.
coverage-evidence (:2243) — workflow.get("name") == "Required OpenCode Review" is False.
is_opencode_check_run (:1636) survives, but only incidentally: its first clause matches the
check-run job name opencode-review before the workflow identity is consulted.

The sharp part is the sentinel. REST_UNKNOWN_GITHUB_ACTIONS_WORKFLOW (:341, set at :1274) exists so
that unknown workflow identity fails closed instead of passing as a source failure — and :1663 explicitly
honours it. But it is keyed on the name being absent. This failure mode is the name being present and
wrong, so the guard never engages. The safety net is indexed on absence while the actual fault is
contamination.

The repo already knows the rendering rule

scripts/ci/opencode_coverage_identity.py:143-147 reconstructs the rendered title directly —
f"{DISPATCH_WORKFLOW_NAME} {target_repo}#{pr_number}@{head_sha}" — and accepts either the bare name or
that title. The format matches the live REST strings above character for character. So this is not
undocumented GitHub behaviour; the handling exists one file over and core.py does not use it.

Credit: core.py:1250 was flagged as suspicious by peer 1 and the sibling-file precedent was found by
host 1; the join above is my verification of both.

Not verified

How often the REST path (rest_pr_node, :1301) is taken versus GraphQL in production. I have not measured
that, so I am not claiming a current production impact rate. Worth noting the direction, though: a
fallback is normally reached when the primary is degraded, so the policy boundary relaxes precisely during
an incident.

Suggested fix shape

Prefer path / workflow_id for workflow identity, which core.py:3062 already does
(workflow_key = str(run.get("workflow_id") or run.get("path") or run.get("name") or "")). Where a name
must be used, reuse the opencode_coverage_identity approach and accept both the bare and rendered forms
rather than adding another prefix-strip.

Follow-up: the REST fallback's trigger set, since I left reachability open above

I said I had not measured when the REST path is taken. I still have no production rate, but the trigger
set itself is readable, and it changes the severity picture in both directions.

Both entry points guard identically — fetch_open_prs (:1472-1479) and fetch_pr (:1500-1505):

except RuntimeError as exc:
    if github_resource_inaccessible(exc) or is_transient_github_api_error(exc):
        return fetch_open_prs_rest(...)   # <- contaminated workflow identity from here on
    raise

github_resource_inaccessible (:1133) is not an incident condition. It matches
"Resource not accessible by integration" — a token-scope state. If the scheduler ever runs under an
installation that cannot read the GraphQL fields, REST is not a fallback, it is the steady-state path, and
the policy boundary is relaxed continuously rather than during a blip.

is_transient_github_api_error (:958) fires on HTTP 500/502/503/504, connection reset|refused|timed out, gateway timeout, i/o timeout, stream error, service unavailable, temporary failure,
timeout, unexpected end of JSON input, and any json.JSONDecodeError (:960-961) — so a truncated
response also routes here.

And one correction to the obvious guess: rate limiting does not trigger it. "API rate limit exceeded" is matched by RATE_LIMIT_DIAGNOSTIC_RE (:954) and deliberately kept out of
TRANSIENT_GITHUB_API_ERRORS (comment at :951-953); gh_graphql waits and retries it internally, and a
rate-limit failure that survives the retries hits raise, not the REST path. Given how much shared-bucket
contention this org has, that was the first mechanism I expected to matter here, and it doesn't. Worth
stating so nobody re-derives it the wrong way.

So the accurate framing is: the contaminated-identity path is entered when GitHub's GraphQL endpoint is
erroring or answering with malformed JSON, or whenever the integration lacks GraphQL read scope. The
first is bounded and correlates with incidents; the second is unbounded and silent. I have measured
neither's frequency in production.

Correction: I gave the wrong direction for the coverage-evidence consumer

In my earlier comment
I wrote that on the REST path "coverage-evidence (:2243) — workflow.get("name") == "Required OpenCode Review" is False", listing it alongside is_strix_context as a case of evidence being lost. That is
backwards. host 1 reproduced all three consumers on live pre-fix payloads and caught it.

The predicate is negative — is_non_authoritative_coverage_check_run — and its consumer keeps a
check precisely when it returns False:

def coverage_evidence_indices(check_runs):
    return [
        index
        for index, node in enumerate(check_runs)
        if (node.get("name") or "").lower() == "coverage-evidence"
        and not is_non_authoritative_coverage_check_run(node)
    ]

So a contaminated workflow name does not withhold coverage evidence there. It admits central
metadata-only evidence that the GraphQL path correctly rejects:

#1978 head 5dad3fe8   coverage-evidence
  is_non_authoritative_coverage_check_run(REST)    = False
  is_non_authoritative_coverage_check_run(GraphQL) = True
  coverage_evidence_indices(REST)    = [0]   <- admitted as authoritative
  coverage_evidence_indices(GraphQL) = []    <- correctly excluded

The three consumers therefore fail in three different directions, not two:

consumer	REST path	direction
`is_strix_context`	`False` for a real `strix` check run	fail-closed, evidence lost
`is_opencode_check_run`	still `True`	unaffected — its first clause matches the check-run's own job name first
`is_non_authoritative_coverage_check_run`	`False`, so the check is kept	fail-open, evidence wrongly admitted

Reached only when SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY is set; how often that holds in production
is not measured.

How I got it wrong: I read the returned expression and inferred the function's meaning from it,
without reading the function's name or its caller. return workflow.get("name") == "Required OpenCode Review" reads like an identity test; it is the body of a predicate that answers the opposite question.
An expression does not tell you the polarity of the decision it feeds.

host 1 also sharpened the is_strix_context finding. Its rescue clause requires
workflow_name in {None, REST_UNKNOWN_GITHUB_ACTIONS_WORKFLOW} — so the clause meant to save a check
run whose workflow is unknown carries the same absence assumption the sentinel does. Contamination
defeats both designed safety nets at once, not just the sentinel.

#1986 fixes all three at the single point they share; its regression test for this case is now
written in the fail-open direction so the severity reads correctly.

A third site survives both fixes: `dispatch_strix_evidence`'s per-repository guard is dead

Auditing the rest of this class after #1986, one site is still comparing a rendered run name to a
declared workflow name, and it is neither what #1983 fixed nor what #1986 fixes.

pr_review_merge_scheduler_core.py:3885-3895:

busy_refs = [
    (dispatch_repo, str(run_data["id"]))
    for run_data in active_workflow_runs(dispatch_repo)
    if run_data.get("id")
    and str(run_data["id"]) not in cancelled_ids
    and run_data.get("name") == workflow            # <- declared name
    and run_data.get("event") == "repository_dispatch"
    and str(run_data.get("display_title") or "").startswith(
        f"Strix Security Scan {target_repo}#"
    )
]

workflow here is security_workflow, whose CLI default is "Strix Security Scan" (:6175). The runs
it filters are repository_dispatch runs, whose name is rendered through run-name:.

Measured on every Strix dispatch run in the last 7 days — 7 of 7:

path    .github/workflows/strix.yml
name    Strix Security Scan ContextualWisdomLab/.github#1965@1004874f…
name == "Strix Security Scan"  ->  False

The predicate never holds, so busy_refs is always empty and the if busy_refs: branch below it can
never run. The dispatch path is live: dispatch_strix_evidence posts event_type: "strix-scan", and
those 7 runs are its receivers.

Not the same site as #1983. That PR (74224b20) changed only active_review_run_refs
(@@ -3254,7 +3254,24 @@), adding run_name == candidate or run_name.startswith(f"{candidate} ").
Line 3890 still carries a bare ==; git blame puts its last change at 269e5bd9, before that PR.
The irony is local: the immediately preceding block in the same function (:3863-3869) already routes
through the alias-aware helper, so one function contains both the fixed and the unfixed form.

What it does and does not affect. The same-head case is covered — active_review_run_refs catches
active runs on the current head and returns early at :3874. What is lost is the broader guard: this
one skips dispatch when the dispatch repository has any active Strix run for the target repository,
across pull requests. So the failure is missing serialization per target repository, not duplicate
dispatch for one head.

That distinction matters because it does not contradict the earlier census finding that 35
in-progress Strix runs mapped to 35 unique (repo, head_sha) pairs with zero duplicates. Those are
different questions; this guard was never the thing that produced that zero.

Limits. The 7 sampled runs were completed, while the guard scans
active_workflow_runs(..., statuses=("queued", "in_progress")). name is bound when the run is created
and does not vary with status, so the conclusion carries — but I did not observe the predicate against a
live queued run. I also have not measured how often two pull requests of one target repository are
dispatched concurrently, so I am not claiming an observed cost.

One more trap worth recording: the most recent 100 repository_dispatch runs in this repository contain
zero Strix entries — CodeQL Scan Dispatch alone fills 80 of them. Reading that as "this path is
unused" would have been a false negative. The 7 runs only appear once the query is widened by date.

Follow-up rather than a change now: #1986 is in flight on this same file, so a second branch would
conflict. I will take this after it merges unless someone else has it.

Corroborated at 100/100, and the false negative has a named mechanism

peer 1 reproduced the :3890 finding independently with a larger sample — strix.yml,
event=repository_dispatch, since 2026-08-31: 100 runs, 100 rendered names, 0 bare. So the dead
predicate is confirmed on 107 runs between us, not 7.

They also identified why my first query returned zero, and it is worth recording because it is a
sampling artifact rather than a date-window problem. Both queries below run against the same
repository, same event filter, same moment:

repos/{org}/{repo}/actions/runs?event=repository_dispatch&per_page=100
    -> strix runs: 0 / 100      (CodeQL Scan Dispatch fills 80 of the page)

repos/{org}/{repo}/actions/workflows/strix.yml/runs?event=repository_dispatch&per_page=100
    -> strix runs: 100 / 100

A repository-level listing samples across all workflows, so a high-frequency workflow crowds the others
off any bounded page — invisibly, because the page is full and nothing reports truncation. Widening by
date recovers some rows; querying the workflow endpoint removes the bias instead of fighting it.

One further note from peer 1 on scope, which matters for anyone reading #1983 as having closed this
class: that PR's reasoning counted the call sites of active_review_run_refs and fixed both.
dispatch_strix_evidence calls that helper and separately carries its own inline == comparison,
so counting callers did not reach it.

Navigation Menu

Repository navigation

Strix run-name is an unenforced parse contract: any suffix makes the scheduler preserve every stale run and stop dispatching #1941

The coupling

Why it matters

The class of defect

Status

같은 뿌리가 이 파일에 넷입니다 — 개별 수정이면 다섯 번째가 나옵니다

전수 조사

제안

The same root cause silently disables a policy boundary in the REST fallback

What the code promises

What the two APIs actually return

What that does to the consumers

The repo already knows the rendering rule

Not verified

Suggested fix shape

Follow-up: the REST fallback's trigger set, since I left reachability open above

Correction: I gave the wrong direction for the coverage-evidence consumer

A third site survives both fixes: `dispatch_strix_evidence`'s per-repository guard is dead

Corroborated at 100/100, and the false negative has a named mechanism

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Strix run-name is an unenforced parse contract: any suffix makes the scheduler preserve every stale run and stop dispatching #1941

Description

The coupling

Why it matters

The class of defect

Status

Activity

seonghobae commented on Sep 5, 2026

seonghobae commented on Sep 6, 2026

같은 뿌리가 이 파일에 넷입니다 — 개별 수정이면 다섯 번째가 나옵니다

전수 조사

제안

seonghobae commented on Sep 6, 2026

The same root cause silently disables a policy boundary in the REST fallback

What the code promises

What the two APIs actually return

What that does to the consumers

The repo already knows the rendering rule

Not verified

Suggested fix shape

seonghobae commented on Sep 6, 2026

Follow-up: the REST fallback's trigger set, since I left reachability open above

seonghobae commented on Sep 6, 2026

Correction: I gave the wrong direction for the coverage-evidence consumer

seonghobae commented on Sep 6, 2026

A third site survives both fixes: dispatch_strix_evidence's per-repository guard is dead

seonghobae commented on Sep 6, 2026

Corroborated at 100/100, and the false negative has a named mechanism

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

A third site survives both fixes: `dispatch_strix_evidence`'s per-repository guard is dead