fix(strix): coalesce push scans per protected branch instead of one group per run id

.github/workflows/strix.yml

@@ -44,12 +44,15 @@ on:
     # them, so the same doc/image-only decision is enforced by the
     # changed-scope job below. The run-name
     # includes the PR number and head SHA for status grouping, while the
-    # concurrency group is scoped per repository and event class to prevent
-    # shared-provider key rate-limit storms. Strix runs intentionally do not
-    # cancel in progress because a pre-job cancellation leaves no scanner log to
-    # review. GitHub keeps one active and one pending run per group; the merge
-    # scheduler re-dispatches exact-head evidence when a pending run is
-    # superseded. For PRs the merge scheduler manages, same-head Strix evidence
+    # concurrency group is scoped per repository AND pull request (native and
+    # dispatch PR runs share one group), or per protected branch for push
+    # events, to prevent shared-provider key rate-limit storms. That group
+    # runs with cancel-in-progress: true, so a newer head of the same PR or
+    # branch retires the older run: cancellation is a supersede signal, never
+    # passing evidence, and the merge scheduler still requires exact-head
+    # evidence before it will act. schedule and PR-less repository_dispatch
+    # keep a unique run id and so are never cancelled by a sibling.
+    # For PRs the merge scheduler manages, same-head Strix evidence
     # is still forced at merge time via repository_dispatch (which paths-ignore
     # does not affect), so merged code never loses evidence.
     paths-ignore:
@@ -77,12 +80,36 @@ on:
 concurrency:
   # Workflow-level admission is required: job-level groups are never evaluated
   # while the whole run is queued behind the organization job ceiling.
+  # Push scans coalesce per protected branch: a newer head of the same branch
+  # supersedes the older scan exactly as a newer PR head does. A push scan
+  # covers the whole tree (STRIX_TARGET_PATH is './' outside PR scope) and
+  # publishes no 'strix' commit status, so the newest head's scan is a
+  # complete scan OF THE CURRENT TREE -- not a record of every earlier
+  # commit's findings: code that entered and left main between two heads, and
+  # findings a retired run never uploaded, are absent from it. With the run-id
+  # fallback every main push was its own group and nothing ever retired a
+  # superseded main scan: on 2026-09-05 nine push/main runs were outstanding
+  # at once in this repository against a 10-30 minute normal scan -- five
+  # holding runner slots under the shared 60-job ceiling (running for up to
+  # two hours) and four still queued, which occupy no slot until a runner is
+  # assigned. schedule and repository_dispatch without a PR number keep a
+  # unique run id.
+  # Coverage trade, measured the same day: main moved 50 times in 24 hours
+  # (median gap 8.4 min, mean 26.5 min, bursty), so with cancel-in-progress
+  # only the final head of each merge burst completes a scan -- one completed
+  # main scan per quiet window, not one per push. That is the intended
+  # exchange: a cancelled push scan gives up its own report, and the scan that
+  # replaces it re-covers the current tree rather than that commit's history;
+  # a per-commit evidence-retention guarantee would need its own preservation
+  # contract. The weekly schedule scan is the floor.
   group: >-
     strix-security-scan-${{
     github.event.pull_request.base.repo.full_name ||
     github.event.client_payload.target_repository || github.repository }}-${{
     github.event.pull_request.number ||
-    github.event.client_payload.pr_number || github.run_id }}
+    github.event.client_payload.pr_number ||
+    (github.event_name == 'push' && format('push-{0}', github.ref_name)) ||
+    github.run_id }}
   cancel-in-progress: true
 
 # Scorecard Token-Permissions (alert #43): keep the workflow-level token

docs/doctoring/startup-failure-and-strix-concurrency-20260904.md

@@ -43,6 +43,35 @@ another. Workflow-level concurrency was deliberately not used because GitHub
 applies it before any live-head admission job can run and does not guarantee
 concurrency ordering.
 
+**Amendment (2026-09-05).** "nor one another" no longer holds for `push`
+events on the same branch. Measured at 14:27Z in `.github`: nine `push`/`main`
+Strix runs were outstanding at once — five holding runner slots under the
+shared 60-job ceiling (jobs started 12:31-14:25Z, one already past two hours)
+and four more waiting in the queue behind them, which occupy no slot until a
+runner is assigned — against a 10-30 minute normal scan. The run-id fallback
+in the workflow-level group made every main push its own group, so no newer
+main head ever retired an older scan. The workflow-level group now scopes
+`push` events as `push-<ref_name>`: a newer head of the same protected branch
+supersedes the older scan exactly as a newer PR head does. What a retired scan
+gives up is its own report, not the gate's inputs: a push scan covers the whole
+tree (`STRIX_TARGET_PATH` is `./` outside PR scope) and publishes no `strix`
+commit status, so the newest head's scan is a complete scan *of the current
+tree*. It is not a record of every earlier commit: code that entered and left
+`main` between two heads, and findings a retired run never uploaded, are absent
+from the newest report, and report collection preserves only runs that reach
+it. A per-commit evidence-retention guarantee would need a separate,
+verifiable preservation contract; this change does not provide one. `schedule`
+and PR-less `repository_dispatch` runs still receive a unique run id. The
+`pr_number=${GITHUB_RUN_ID}` admission output is unchanged.
+
+Tradeoff, stated so a later reader of the security dashboard is not
+surprised: with `main` moving roughly every 30 minutes against a 10-30 minute
+scan, "main is scanned after every merge" becomes "the latest `main` is
+scanned once merging pauses for at least one scan duration". During a merge
+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.
+
 ## Verification
 
 - `python -m pytest -q tests/test_pr_review_merge_scheduler.py -k 'startup_failures or startup_failure'`

scripts/ci/test_strix_quick_gate.sh

@@ -203,6 +203,7 @@ assert_strix_workflow_pr_trigger_hardened() {
 	assert_file_contains "$workflow_file" "needs: [changed-scope, admit-current-head]" "strix provider queue waits for live-head admission"
 	assert_file_contains "$workflow_file" 'strix-security-scan-${{' "strix workflow coalesces by repository and PR before job admission"
 	assert_file_not_contains "$workflow_file" 'strix-security-scan-${{ needs.admit-current-head.outputs.target_repository }}-${{' "strix concurrency is not delayed until job admission"
+	assert_file_contains "$workflow_file" "format('push-{0}', github.ref_name)" "strix push scans coalesce per protected branch instead of one group per run id"
 	assert_file_contains "$workflow_file" "cancel-superseded-pr-runs:" "strix workflow runs superseded-head cleanup outside the provider scan queue"
 	assert_file_not_contains "$workflow_file" "format('closed-pr-{0}-{1}'" "strix cleanup does not need a second concurrency queue"
 	assert_file_contains "$workflow_file" 'echo "pr_number=${GITHUB_RUN_ID}"' "strix workflow preserves independent push and schedule evidence"

tests/test_required_workflow_queue_contract.py

@@ -33,6 +33,31 @@ def workflow_text(name: str) -> str:
 )
 
 
+def _strip_yaml_inline_comment(text: str) -> str:
+    """Drop a YAML inline comment from one scalar line.
+
+    YAML opens a comment at ``#`` only when it starts the line or follows
+    whitespace, and never inside a quoted scalar, so a bare ``split("#")``
+    would truncate a legitimate value that merely contains the character.
+    """
+    index = 0
+    quote = ""
+    while index < len(text):
+        char = text[index]
+        if quote:
+            if quote == '"' and char == "\\":
+                index += 2
+                continue
+            if char == quote:
+                quote = ""
+        elif char in "\"'":
+            quote = char
+        elif char == "#" and (index == 0 or text[index - 1] in " \t"):
+            return text[:index]
+        index += 1
+    return text
+
+
 def workflow_level_concurrency_group(workflow: str) -> str:
     """Return only the workflow-level ``concurrency.group`` value, comments removed.
 
@@ -56,7 +81,7 @@ def workflow_level_concurrency_group(workflow: str) -> str:
         if not collecting:
             if re.match(r"^\s*group:", line):
                 collecting = True
-                value.append(line.split("group:", 1)[1])
+                value.append(_strip_yaml_inline_comment(line.split("group:", 1)[1]))
             continue
         if re.match(r"^\s*[A-Za-z][\w-]*:", line):
             break
@@ -437,6 +462,66 @@ def test_concurrency_group_slice_ignores_the_comment_that_documents_it() -> None
     assert "opencode-review-dispatch-${{ github.repository }}" in group_value
 
 
+def test_concurrency_group_slice_ignores_an_inline_comment_on_the_key() -> None:
+    """An inline comment beside a plain-scalar key must not satisfy the contract.
+
+    The full-line negative control above does not cover this shape. YAML allows a
+    comment on the key's own line, so a change collapsing the group to the
+    repository alone could keep the documented expressions one space away and
+    leave every substring assertion green.
+    """
+    inline = textwrap.dedent(
+        """\
+        concurrency:
+          group: opencode-review-dispatch-${{ github.repository }} # ${{ github.event.client_payload.pr_number || github.run_id }}
+          cancel-in-progress: true
+        permissions:
+          contents: read
+        """
+    )
+    group_value = workflow_level_concurrency_group(inline)
+
+    assert "opencode-review-dispatch-${{ github.repository }}" in group_value
+    assert "github.event.client_payload.pr_number" not in group_value
+    assert "github.run_id" not in group_value
+
+
+def test_concurrency_group_slice_keeps_a_hash_that_is_not_a_comment() -> None:
+    """Stripping must follow YAML's rules rather than cutting at every ``#``.
+
+    Two shapes would be corrupted by a naive ``split("#")``: a quoted scalar
+    containing the character, and a folded block body, where ``#`` is literal
+    content and never opens a comment. Only the key's own line is stripped.
+    """
+    quoted = textwrap.dedent(
+        """\
+        concurrency:
+          group: "release-#42-${{ github.repository }}"
+          cancel-in-progress: true
+        permissions:
+          contents: read
+        """
+    )
+    assert "release-#42-${{ github.repository }}" in workflow_level_concurrency_group(
+        quoted
+    )
+
+    folded = textwrap.dedent(
+        """\
+        concurrency:
+          group: >-
+            release-${{ github.repository }}-#${{
+            github.run_id }}
+          cancel-in-progress: true
+        permissions:
+          contents: read
+        """
+    )
+    folded_value = workflow_level_concurrency_group(folded)
+    assert "#${{" in folded_value
+    assert "github.run_id" in folded_value
+
+
 def test_concurrency_group_slice_reads_a_folded_multi_line_key() -> None:
     """The real key is a folded block, so the slice must join its continuation lines."""
     folded = textwrap.dedent(
@@ -701,6 +786,18 @@ def test_strix_serializes_provider_evidence_per_repository_and_pr() -> None:
     superseded runs before runner admission, including runs still blocked by
     the organization-wide job ceiling. Native and dispatched evidence share
     one group; non-PR events use a unique run id.
+
+    2026-09-05: push events are scoped per protected branch (``push-<ref>``)
+    instead of a unique run id. Measured that morning in this repository:
+    nine ``push``/``main`` Strix runs were outstanding at once (five running
+    for up to two hours, four queued) against a 10-30 minute normal scan,
+    because the run-id fallback made every main push its own group and
+    nothing ever retired a superseded main scan. A push scan covers the whole
+    tree (``STRIX_TARGET_PATH`` is ``./`` outside PR scope) and publishes no
+    ``strix`` commit status, so the newest head's scan is a complete scan of
+    the current tree (not a record of every earlier commit's findings).
+    ``schedule`` and ``repository_dispatch`` without a PR number keep a
+    unique run id.
     """
     workflow = workflow_text("strix.yml")
     concurrency_contract = workflow.split("concurrency:", 1)[1].split(
@@ -719,6 +816,14 @@ def test_strix_serializes_provider_evidence_per_repository_and_pr() -> None:
     assert "github.event.pull_request.number" in group_value
     assert "github.event.client_payload.pr_number" in group_value
     assert "github.run_id" in group_value
+    # Asserted against group_value, not the whole concurrency block: #1970 made
+    # these keys immune to comment leakage, and this file's own prose now
+    # discusses the push clause at length, so the comment text would otherwise
+    # satisfy the assertion whether or not the expression survived.
+    assert (
+        "(github.event_name == 'push' && format('push-{0}', github.ref_name)) ||"
+        in group_value
+    )
     assert "github.event.pull_request.head.sha" not in concurrency_contract
     assert "github.event.client_payload.pr_head_sha" not in concurrency_contract
     assert "cancel-in-progress: true" in concurrency_contract