Skip to content
Draft
Show file tree
Hide file tree
Changes from 2 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
13 changes: 12 additions & 1 deletion .github/workflows/opencode-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -565,11 +565,22 @@ jobs:
return 0
fi
local run_ids
# Workflow identity comes from `.path`, not `.name` -- the same
# defect strix.yml's cleanup job carried. This workflow declares
# `run-name:`, so a run's `name` is the rendered title
# "Required OpenCode Review <repo>#<pr>@<sha>", never the bare
# workflow name, and the former `.name ==` equality selected
# nothing: sampled 2026-09-07, 0 of 100 runs carried the bare name
# and 100 of 100 carried the rendered one. `.path` is the signal
# noema-review.yml already adopted here, stable for both native and
# ruleset-injected runs; the `startswith` check keeps the second,
# independent signal the equality was reaching for.
if ! run_ids="$(jq -r --arg pr "$TARGET_PR_NUMBER" --arg head_sha "$TARGET_PR_HEAD_SHA" \
--arg repo "$TARGET_REPOSITORY" --arg current "$CURRENT_RUN_ID" '
.workflow_runs[]
| select((.id | tostring) != $current)
| select(.name == "Required OpenCode Review")
| select(.path == ".github/workflows/opencode-review.yml")
| select((.name // "") | startswith("Required OpenCode Review"))
| select(.event == "pull_request_target")
| ((.display_title // "") | startswith("Required OpenCode Review " + $repo + "#" + $pr + "@")) as $title_matches
| ((.pull_requests // []) | any((.number | tostring) == $pr)) as $metadata_matches
Expand Down
23 changes: 22 additions & 1 deletion .github/workflows/strix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -292,11 +292,32 @@ jobs:
return 0
fi
local run_ids
# Workflow identity comes from `.path`, not `.name`. This workflow
# declares `run-name:`, and GitHub reports the *rendered* run-name
# in a run's `name` -- "Strix Security Scan <repo>#<pr>@<sha>",
# never the bare workflow name. The former `.name ==` equality
# therefore matched nothing and this whole cleanup job was a silent
# no-op: sampled 2026-09-07, 0 of 100 strix runs carried the bare
# name while 100 of 100 carried the rendered one. `.path` is the
# signal noema-review.yml already adopted for this same defect, and
# it holds in both contexts -- verified the same day as
# ".github/workflows/strix.yml" on native `.github` runs and on the
# 9 ruleset-injected runs in bandscope, which carries no local
# strix.yml. The `startswith` name check is kept as the second,
# independent signal the original equality was reaching for.
#
# The workflow-level `concurrency` group above hides this on the
# `synchronize` path (it cancels the previous head's run itself),
# so what was actually lost is every case no successor run
# supersedes: `closed` and `converted_to_draft` left their in-flight
# scans running to completion, holding admission slots under the
# shared 60-job ceiling for a PR nobody is waiting on.
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" '
.workflow_runs[]
| select((.id | tostring) != $current)
| select(.name == "Strix Security Scan")
| select(.path == ".github/workflows/strix.yml")
| select((.name // "") | startswith("Strix Security Scan"))
| select(.event == "pull_request_target")
| ((.display_title // "") | startswith("Strix Security Scan " + $repo + "#" + $pr + "@")) as $title_matches
| ((.pull_requests // []) | any((.number | tostring) == $pr)) as $metadata_matches
Expand Down
46 changes: 40 additions & 6 deletions scripts/ci/pr_review_merge_scheduler_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -3175,6 +3175,41 @@ def workflow_run_mentions_pr(run_data: dict[str, Any], pr_number: int) -> bool:
return any(pr.get("number") == pr_number for pr in run_data.get("pull_requests") or [])


def run_name_identifies_workflow(run_name: str, *workflow_names: str) -> bool:
"""Return whether a run's reported ``name`` identifies one of these workflows.

GitHub reports the *rendered* ``run-name:`` in a run's ``name`` field, not
the workflow's declared ``name:``. Every central review workflow here
declares one, so ``name`` arrives as ``"<declared name> <repo>#<pr>@<sha>"``
and an equality test against the declared name matches nothing in
production at all: sampled 2026-09-07, 0 of 100 ``strix.yml`` runs and 0 of
100 ``opencode-review.yml`` runs carried the bare form while 100 of 100
carried the rendered one.

Both forms are accepted because both occur. The bare name is what a
workflow declaring no ``run-name:`` sends, and what GitHub can fall back to
for an organization-required-workflow run materialized in a sibling
repository (recorded on ``noema-review.yml``'s cleanup job).

The space this requires after a candidate is a word boundary, and that is
the whole of what it buys: a workflow named "Strix Security Scanner" does
not answer for the candidate "Strix Security Scan". It deliberately does
*not* separate a longer workflow name that begins with the candidate and a
space -- a hypothetical "Strix Security Scan Extended" would be accepted.
Two things make that safe rather than latent. No central workflow name
prefixes another (verified 0 of 35 on 2026-09-07, and pinned by
:mod:`tests.test_stale_run_cleanup_workflow_identity`), and every call site
pins identity a second time -- by ``display_title`` prefix or by the run's
``path`` -- so this predicate narrows a candidate set rather than deciding
identity alone. Accepting a prefix is also required, not merely tolerated:
callers pass short aliases ("Strix") for the same workflow on purpose.
"""
return any(
run_name == candidate or run_name.startswith(f"{candidate} ")
for candidate in workflow_names
)


def stale_pr_run_ids(
repo: str,
pr: dict[str, Any],
Expand All @@ -3195,7 +3230,9 @@ def stale_pr_run_ids(
number = int(pr["number"])
stale: list[str] = []
for run_data in active_workflow_runs(repo, statuses):
if workflow is not None and run_data.get("name") != workflow:
if workflow is not None and not run_name_identifies_workflow(
str(run_data.get("name") or ""), workflow
):
continue
if str(run_data.get("head_sha") or "").lower() == head:
continue
Expand Down Expand Up @@ -3266,10 +3303,7 @@ def active_review_run_refs(
# never cancelled. Sampled 2026-09-07: 100 of 100
# opencode-review-dispatch runs carry the rendered form, 0 bare.
# Accept it -- the workflow name, then a space, then the suffix.
if not any(
run_name == candidate or run_name.startswith(f"{candidate} ")
for candidate in (workflow, *workflow_aliases)
):
if not run_name_identifies_workflow(run_name, workflow, *workflow_aliases):
continue
run_id = run_data.get("id")
if not run_id:
Expand Down Expand Up @@ -3797,7 +3831,7 @@ def dispatch_strix_evidence(repo: str, workflow: str, pr: dict[str, Any], *, dry
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
and run_name_identifies_workflow(str(run_data.get("name") or ""), workflow)
and run_data.get("event") == "repository_dispatch"
and str(run_data.get("display_title") or "").startswith(
f"Strix Security Scan {target_repo}#"
Expand Down
24 changes: 20 additions & 4 deletions tests/test_opencode_required_verdict_regression.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,20 +215,31 @@ def _cleanup_run(
*,
run_id: int,
head_sha: str = HEAD,
name: str = "Required OpenCode Review",
name: str | None = None,
path: str = ".github/workflows/opencode-review.yml",
event: str = "pull_request_target",
display_title: str | None = None,
pr_number: int = 1437,
) -> dict[str, object]:
"""Build one synthetic workflow-run record for the cleanup filter."""
"""Build one synthetic workflow-run record for the cleanup filter.

``name`` defaults to the same string as ``display_title`` because that is
what GitHub sends: this workflow declares ``run-name:``, and a run's
``name`` is the rendered result, never the declared workflow name. The
earlier default paired a bare ``name`` with a rendered ``display_title``, a
combination the API cannot produce, which is how the selector's former
``.name ==`` equality passed here while matching 0 of 100 live runs.
``path`` carries the workflow identity the selector now reads.
"""
title = (
display_title
if display_title is not None
else f"Required OpenCode Review ContextualWisdomLab/example#{pr_number}@{head_sha}"
)
return {
"id": run_id,
"name": name,
"name": title if name is None else name,
"path": path,
"event": event,
"display_title": title,
"pull_requests": [{"number": pr_number, "head": {"sha": head_sha}}],
Expand Down Expand Up @@ -261,7 +272,12 @@ def test_cleanup_excludes_a_different_pull_request() -> None:

def test_cleanup_excludes_a_differently_named_or_triggered_run() -> None:
"""A same-PR run for another workflow or trigger is left untouched."""
other_workflow = _cleanup_run(run_id=1, head_sha="b" * 40, name="Strix Security Scan")
other_workflow = _cleanup_run(
run_id=1,
head_sha="b" * 40,
name="Strix Security Scan",
path=".github/workflows/strix.yml",
)
other_event = _cleanup_run(run_id=2, head_sha="b" * 40, event="workflow_dispatch")
assert (
cleanup_candidate_run_ids([other_workflow, other_event], current_run_id="999")
Expand Down
7 changes: 6 additions & 1 deletion tests/test_pr_review_merge_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -6079,9 +6079,14 @@ def fake_run(args, stdin=None):

def test_dispatch_strix_waits_for_active_target_repository_run(monkeypatch, capsys):
calls = []
# The run name is the *rendered* ``run-name:``, identical to
# ``display_title`` -- the only shape GitHub actually sends for a workflow
# that declares one. The previous fixture paired a bare ``name`` with a
# rendered ``display_title``, a combination GitHub cannot produce, so this
# test passed while the production predicate matched 0 of 100 live runs.
active_run = {
"id": 9350,
"name": "Strix Security Scan",
"name": f"Strix Security Scan owner/repo#2@{'c' * 40}",
"event": "repository_dispatch",
"display_title": f"Strix Security Scan owner/repo#2@{'c' * 40}",
"pull_requests": [],
Expand Down
18 changes: 12 additions & 6 deletions tests/test_required_workflow_queue_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -781,13 +781,19 @@ def test_strix_cleanup_uses_pr_metadata_when_custom_title_is_absent() -> None:
marker = '--arg action "$PR_ACTION" --arg repo "$TARGET_REPOSITORY" --arg current "$CURRENT_RUN_ID" \'\n'
start = workflow.index(marker) + len(marker)
end = workflow.index('\n \' <<<"$runs_json"', start)
# Every row carries ``path``: the selector reads workflow identity there
# now, because a run's ``name`` is the rendered ``run-name:`` and the old
# ``.name ==`` equality matched 0 of 100 live runs. The bare ``name`` here
# is deliberate and still accepted -- it is the required-workflow-ruleset
# shape this test exists for, where GitHub renders no run-name at all.
strix_path = ".github/workflows/strix.yml"
runs = {
"workflow_runs": [
{"id": 1, "name": "Strix Security Scan", "event": "pull_request_target", "pull_requests": [{"number": 7, "head": {"sha": "old"}}]},
{"id": 2, "name": "Strix Security Scan", "event": "pull_request_target", "pull_requests": [{"number": 7, "head": {"sha": "current"}}]},
{"id": 3, "name": "Strix Security Scan", "event": "pull_request_target", "pull_requests": [{"number": 7}]},
{"id": 4, "name": "Strix Security Scan", "event": "pull_request_target", "display_title": "Strix Security Scan owner/repo#7@old", "pull_requests": [{"number": 7, "head": {"sha": "current"}}]},
{"id": 5, "name": "Strix Security Scan", "event": "pull_request_target", "pull_requests": [{"number": 8, "head": {"sha": "old"}}]},
{"id": 1, "name": "Strix Security Scan", "path": strix_path, "event": "pull_request_target", "pull_requests": [{"number": 7, "head": {"sha": "old"}}]},
{"id": 2, "name": "Strix Security Scan", "path": strix_path, "event": "pull_request_target", "pull_requests": [{"number": 7, "head": {"sha": "current"}}]},
{"id": 3, "name": "Strix Security Scan", "path": strix_path, "event": "pull_request_target", "pull_requests": [{"number": 7}]},
{"id": 4, "name": "Strix Security Scan", "path": strix_path, "event": "pull_request_target", "display_title": "Strix Security Scan owner/repo#7@old", "pull_requests": [{"number": 7, "head": {"sha": "current"}}]},
{"id": 5, "name": "Strix Security Scan", "path": strix_path, "event": "pull_request_target", "pull_requests": [{"number": 8, "head": {"sha": "old"}}]},
]
}
result = subprocess.run(
Expand Down Expand Up @@ -836,7 +842,7 @@ def _run_strix_cleanup(
exit 0
fi
if [[ "$*" == *"actions/runs?status=queued"* ]]; then
printf '%s\n' '{"workflow_runs":[{"id":100,"name":"Strix Security Scan","event":"pull_request_target","pull_requests":[{"number":7,"head":{"sha":"old"}}]}]}'
printf '%s\n' '{"workflow_runs":[{"id":100,"name":"Strix Security Scan","path":".github/workflows/strix.yml","event":"pull_request_target","pull_requests":[{"number":7,"head":{"sha":"old"}}]}]}'
exit 0
fi
if [[ "$*" == *"actions/runs?status="* ]]; then
Expand Down
Loading
Loading