diff --git a/.github/workflows/qwen-autofix-fork-bridge.yml b/.github/workflows/qwen-autofix-fork-bridge.yml new file mode 100644 index 00000000000..f864fd32be3 --- /dev/null +++ b/.github/workflows/qwen-autofix-fork-bridge.yml @@ -0,0 +1,180 @@ +name: 'Qwen Autofix Fork Bridge' + +# The base-context half of the bridge described in +# `qwen-autofix-fork-signal.yml`. A `workflow_run` run is created on the +# DEFAULT BRANCH with the repository's own token and secrets — exactly what the +# fork-PR review lane is denied — so this is where the review scan can actually +# be started for a fork PR. +# +# It never checks out or executes pull request code, and it holds no PAT. Its +# only write is a `workflow_dispatch` of `qwen-autofix.yml` naming ONE PR +# number, using the run's own `actions: write` token (the same lever Fleet +# Shepherd already pulls). The dispatched run re-derives admission from live +# API state — open, targeting main, authored by the bot or carrying the +# takeover label, and for a fork a live write+ author plus maintainer edits +# allowed — so the number only says WHEN to look, never who may be touched. +# +# Which PR was reviewed — and by whom — rides the signal's run-name, which +# GitHub surfaces on the workflow_run event as `display_title` (a fork cannot +# forge it: fork-triggered runs execute the signal FILE from the base branch, +# and that file supplies the run-name). A head SHA alone cannot carry the +# binding: it authenticates the revision, not which PR generated the review, +# and two open fork PRs can share one head commit — resolving by head only +# would dispatch every PR sharing it on a review submitted to one. +# `workflow_run.head_sha` still matters as the freshness gate: for a +# `pull_request_review` run it IS the reviewed PR's head SHA (verified: run +# 31152873061's head_sha equals PR #8436's headRefOid), and a push landing +# between the review and this run leaves the bound PR no longer carrying that +# head — the bridge no-ops green instead of reading the race as a security +# event, and the scheduled scan remains the backstop for the stale review. +# (`workflow_run.pull_requests` is EMPTY for fork PRs — measured — so it +# cannot supply the binding.) + +on: + workflow_run: + workflows: + - 'Qwen Autofix Fork Signal' + types: + - 'completed' + +# The scan side honors the `source=fork-bridge` marker (staying quiet at the +# round cap) only after verifying provenance against a SUCCESSFUL run of THIS +# workflow naming the PR — propagate the signal's title into the run title so +# that check has something to match. +run-name: 'fork-bridge: ${{ github.event.workflow_run.display_title }}' + +permissions: + actions: 'write' + # The reviewer live-permission re-check reads + # `repos/{repo}/collaborators/{user}/permission` — the same call route + # makes with `contents: read`. + contents: 'read' + pull-requests: 'read' + +defaults: + run: + shell: 'bash' + +# Keyed on the conclusion AS WELL AS the reviewed head. Every signal run — +# including ones that gated themselves out as `skipped` and ones `cancelled` +# by the signal's own concurrency — fires `workflow_run: completed`, so +# without the conclusion in the key a skipped bridge run would enter the same +# group as the real one, cancel it, and then skip its own job: repeating an +# ungated review could hold the bridge off a fork PR indefinitely. With it, +# only genuine `success` signals share a group, where two reviews landing on +# the same commit want one scan and the newer signal supersedes the older. +# Duplicates past this point are harmless anyway — review-scan holds a per-PR +# lock and advances a watermark — so collapsing the burst is an economy, not +# a correctness gate. +concurrency: + group: 'qwen-autofix-fork-bridge-${{ github.event.workflow_run.conclusion }}-${{ github.event.workflow_run.head_sha }}' + cancel-in-progress: true + +env: + # Mirrors qwen-autofix.yml's AUTOFIX_BOT / REVIEW_BOT / TAKEOVER_LABEL / + # SKIP_LABEL; pinned to them by + # scripts/tests/qwen-autofix-fork-bridge-workflow.test.js. + AUTOFIX_BOT: "${{ vars.AUTOFIX_BOT_LOGIN || 'qwen-code-dev-bot' }}" + REVIEW_BOT: 'qwen-code-ci-bot' + TAKEOVER_LABEL: 'autofix/takeover' + SKIP_LABEL: 'autofix/skip' + +jobs: + dispatch: + name: 'Dispatch the autofix review lane' + # A signal that gated itself out (in-repo PR, untrusted review, unmanaged + # PR) completes as `skipped`, never `success`. + if: |- + ${{ github.repository == 'QwenLM/qwen-code' + && github.event.workflow_run.conclusion == 'success' }} + runs-on: 'ubuntu-latest' + timeout-minutes: 10 + env: + REPO: '${{ github.repository }}' + # The head commit of the run that produced the signal — for a + # `pull_request_review` run the reviewed PR's head SHA; the freshness + # gate the resolved PR is checked against. + SIGNAL_HEAD_SHA: '${{ github.event.workflow_run.head_sha }}' + # The signal's run-name — the GitHub-owned binding that names the + # reviewed PR and the reviewer. + SIGNAL_TITLE: '${{ github.event.workflow_run.display_title }}' + GITHUB_TOKEN: '${{ github.token }}' + steps: + - name: 'Resolve the signalled PR and dispatch the scan' + run: |- + # The signal encodes the reviewed PR and the reviewer in its + # run-name (a base-branch file the fork cannot edit); GitHub + # surfaces it as the workflow_run's display_title. That binding — + # not the head SHA — says WHICH PR the review targeted. + TITLE_RE='^fork-signal: PR ([0-9]+) reviewed by (.+)$' + if [[ ! "${SIGNAL_TITLE}" =~ ${TITLE_RE} ]]; then + echo "::error::could not parse the PR binding from the signal run title '${SIGNAL_TITLE}' — signal and bridge are out of contract" + exit 1 + fi + SIGNAL_PR="${BASH_REMATCH[1]}" + SIGNAL_REVIEWER="${BASH_REMATCH[2]}" + + # The signal gate trusts the reviewer's author_association, but + # MEMBER/COLLABORATOR does not guarantee LIVE write permission — + # the direct pull_request_review lane rejects a below-write + # reviewer on exactly this event. Re-check live here (the + # review-bot exception mirrors route's), so a read/triage + # collaborator cannot signal the secret-bearing lane. A rejected + # reviewer is a green no-op: the direct lane declined the same + # review, and the scheduled scan remains the backstop. + if [[ "${SIGNAL_REVIEWER}" != "${REVIEW_BOT}" ]]; then + REVIEWER_PERM="$(gh api "repos/${REPO}/collaborators/${SIGNAL_REVIEWER}/permission" --jq '.permission // ""' 2> /dev/null || echo '')" + case "${REVIEWER_PERM}" in + admin|maintain|write) : ;; + *) + echo "🧭 reviewer '${SIGNAL_REVIEWER}' permission='${REVIEWER_PERM:-none}' is below write — the direct lane would decline this review, so nothing is dispatched" + exit 0 + ;; + esac + fi + + # Live re-read of the bound PR: consent can move between the review + # and the dispatch. These filters mirror route's fork admission + # plus SKIP-wins-everywhere — cheap early-outs, not a security + # boundary: review-scan re-derives all of it. They exist so a PR + # that closed, retargeted, moved its head, or withdrew consent + # between the review and now costs one API read instead of a whole + # dispatched run, and so the log says which it was. + if ! PR_JSON="$(gh pr view "${SIGNAL_PR}" --repo "${REPO}" \ + --json number,author,labels,headRefOid,isCrossRepository,maintainerCanModify,state,baseRefName 2> /dev/null)"; then + echo "::error::could not read PR #${SIGNAL_PR} to resolve the signal" + exit 1 + fi + + ELIGIBLE="$(jq -r --arg sha "${SIGNAL_HEAD_SHA}" --arg bot "${AUTOFIX_BOT}" \ + --arg take "${TAKEOVER_LABEL}" --arg skip "${SKIP_LABEL}" ' + select(.state == "OPEN" and .baseRefName == "main") + | select(.headRefOid == $sha and .isCrossRepository == true and .maintainerCanModify == true) + | select((.author.login // "") == $bot or (([.labels[]?.name] | index($take)) != null)) + | select(([.labels[]?.name] | index($skip)) == null) + | .number' <<< "${PR_JSON}")" + if [[ -z "${ELIGIBLE}" ]]; then + echo "🧭 signalled PR #${SIGNAL_PR} no longer carries head ${SIGNAL_HEAD_SHA} as an eligible fork PR (head moved, PR closed, or consent withdrawn) — the scheduled scan remains the backstop" + exit 0 + fi + + for attempt in 1 2 3; do + if gh workflow run qwen-autofix.yml --repo "${REPO}" --ref main \ + -f pr_number="${ELIGIBLE}" -f source=fork-bridge; then + echo "🚀 dispatched the autofix review lane for fork PR #${ELIGIBLE}" + { + echo "### 🌉 AutoFix fork bridge" + echo + echo "Dispatched the review lane for [#${ELIGIBLE}](${GITHUB_SERVER_URL}/${REPO}/pull/${ELIGIBLE}) — a fork PR whose own review event carries no repository secrets." + } >> "${GITHUB_STEP_SUMMARY}" + exit 0 + fi + echo "::warning::dispatch for #${ELIGIBLE} failed (attempt ${attempt}/3)" >&2 + if [[ "${attempt}" -lt 3 ]]; then sleep "${attempt}"; fi + done + # Red on purpose when a dispatch never landed. The scheduled scan + # still reaches the PR eventually, but a bridge that cannot + # dispatch is broken in a way no later tick repairs, and silence + # here would look identical to a working bridge with nothing to do. + echo "::error::could not dispatch the autofix review lane for resolved PR #${ELIGIBLE}" + exit 1 diff --git a/.github/workflows/qwen-autofix-fork-signal.yml b/.github/workflows/qwen-autofix-fork-signal.yml new file mode 100644 index 00000000000..2f9ef163ccf --- /dev/null +++ b/.github/workflows/qwen-autofix-fork-signal.yml @@ -0,0 +1,103 @@ +name: 'Qwen Autofix Fork Signal' + +# GitHub hands NO repository secrets to a workflow run tied to a pull request +# whose head lives in a fork — the run header reads `Secret source: None` — so +# `secrets.CI_DEV_BOT_PAT` arrives empty and the review lane in +# `qwen-autofix.yml` cannot authenticate from it. #8671 (merged, 63a99c2c11) +# already changed the direct lane to match: route's decide script declines +# fork `pull_request_review` events inline, and review-scan fails fast at the +# top on an empty PAT — so fork reviews no longer redden the direct lane, but +# they also get no real-time pickup there: only the scheduled scan, running +# in repo context, ever engages a fork PR. +# +# This is the credential-free half of the standard `workflow_run` bridge that +# closes that gap. It records nothing — its only job is to complete `success` +# EXACTLY for the reviews the direct lane cannot serve, so the base-context +# half (`qwen-autofix-fork-bridge.yml`) can start the real scan where the +# secrets and `actions: write` do exist. Which PR was reviewed, and by whom, +# rides this run's `run-name` below — GitHub surfaces it on the +# `workflow_run` event as `display_title` — and the bridge re-checks the +# reviewed head against the PR's live head as a freshness gate. +# +# Fork-triggered runs execute the workflow file from the BASE branch, not the +# PR head — verified on run 31152873061, whose PR branch predates a main-only +# change to `qwen-autofix.yml` and which still executed the NEW file — so a +# fork cannot edit this gate or forge the run-name binding, and every +# conjunct is re-checked downstream anyway: the bridge re-reads live PR state +# (including the reviewer's LIVE write permission) and review-scan re-derives +# admission, so this gate decides WHEN, never whether. +# +# Deliberately powerless: `permissions: {}`, no checkout, and no repository +# code executed, so a fork-triggered run holds nothing worth stealing. For the +# same reason it stays on the hosted pool — fork-triggered work never goes on +# the persistent self-hosted runners, whose workspaces outlive the job. + +on: + pull_request_review: + types: + - 'submitted' + +# The bridge's PR binding and the reviewer identity ride this run-name: +# GitHub surfaces it on the `workflow_run` event as `display_title`, and this +# FILE — which a fork cannot edit — supplies it. The format is load-bearing +# and pinned by scripts/tests/qwen-autofix-fork-bridge-workflow.test.js. +run-name: 'fork-signal: PR ${{ github.event.pull_request.number }} reviewed by ${{ github.event.review.user.login }}' + +permissions: {} + +defaults: + run: + shell: 'bash' + +# Trusted reviews coalesce PER PR (two reviews seconds apart signal once); +# everything else gets a run-unique group. The group is entered BEFORE the job +# `if:` runs, so without the trust split an arbitrary reviewer's submission +# would cancel a queued or in-flight trusted signal and then skip its own job +# — silently dropping the maintainer's review, the exact gap this bridge +# exists to close. Mirrors route's trust-keyed pattern; the literals mirror +# qwen-autofix.yml's TRUSTED_ASSOC / REVIEW_BOT and are pinned to them by +# scripts/tests/qwen-autofix-fork-bridge-workflow.test.js. +concurrency: + group: >- + ${{ (contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.review.author_association) || github.event.review.user.login == 'qwen-code-ci-bot') && format('qwen-autofix-fork-signal-pr-{0}', github.event.pull_request.number) || format('qwen-autofix-fork-signal-{0}', github.run_id) }} + cancel-in-progress: true + +jobs: + signal: + name: 'Signal the reviewed fork PR' + # In-repo PRs keep their secrets and reach the review lane directly, so + # signalling them would only dispatch a duplicate scan. The remaining + # conjuncts MIRROR route's fork admission (qwen-autofix.yml's + # pull_request_review path): maintainer edits on, and the PR bot-authored + # or takeover-labeled — so an ordinary contributor's fork PR burns no + # signal/bridge/dispatched runs per review, and a takeover PR whose + # author turned maintainer edits off is never pushed into the credentialed + # lane. SKIP wins here as it wins everywhere: a skip-labeled PR's reviews + # would otherwise burn a signal + bridge run and a PR read before being + # discarded, repeating known-useless work on every review. The + # association list mirrors TRUSTED_ASSOC, the review-bot login mirrors + # REVIEW_BOT, and the two labels mirror TAKEOVER_LABEL / SKIP_LABEL — all + # pinned by the fork-bridge workflow test. The association is a WHEN + # filter only: MEMBER/COLLABORATOR does not guarantee LIVE write + # permission, so the bridge re-checks the reviewer live before + # dispatching. + if: |- + ${{ github.repository == 'QwenLM/qwen-code' + && github.event.pull_request.head.repo.full_name != github.repository + && github.event.pull_request.base.ref == 'main' + && github.event.pull_request.state == 'open' + && github.event.pull_request.maintainer_can_modify == true + && (github.event.pull_request.user.login == (vars.AUTOFIX_BOT_LOGIN || 'qwen-code-dev-bot') + || contains(toJSON(github.event.pull_request.labels.*.name), '"autofix/takeover"')) + && !contains(toJSON(github.event.pull_request.labels.*.name), '"autofix/skip"') + && (contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.review.author_association) + || github.event.review.user.login == 'qwen-code-ci-bot') }} + runs-on: 'ubuntu-latest' + timeout-minutes: 5 + steps: + - name: 'Complete so the bridge fires' + env: + PR_NUMBER: '${{ github.event.pull_request.number }}' + run: |- + echo "🔔 signalling fork PR #${PR_NUMBER} for the autofix review lane" + echo "The bridge reads the PR number and reviewer from this run's title and re-checks the head SHA; nothing is uploaded." diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index d00f33d28ec..cc36acd8cec 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -83,6 +83,10 @@ on: required: false type: 'boolean' default: false + source: + description: 'Dispatch-origin marker (fork-bridge = the fork-review bridge); routing metadata only — never changes what may be touched, and its one behavioral effect (staying quiet at the round cap) is honored only after the scan verifies a matching fork-bridge run' + required: false + type: 'string' defaults: run: @@ -237,7 +241,12 @@ jobs: # apart route once — the old shared group's one useful side effect, # kept, without letting events on OTHER PRs cancel this one) # • issue events coalesce PER issue - # • dispatches are unique per run and are never cancelled + # • dispatches are unique per run and are never cancelled — + # fork-bridge dispatches included: `source` is a public + # workflow_dispatch input, so no dispatch may claim a trusted + # per-PR coalescing group by asserting an origin; fork-review + # bursts coalesce upstream instead (the signal per PR, the bridge + # per conclusion+head) # The old single shared cancel-in-progress group let ANY newer event kill # pending full scans while route jobs sat queued behind runner backlog — # observed as hours of scan starvation during review-event storms. @@ -261,7 +270,10 @@ jobs: # a burst of trusted command comments coalesces to at most two runs # with latest-intent semantics, never touching review routes; # issues → per-issue; anything else (dispatch) → unique per run_id, - # never cancelled. + # never cancelled. A fork-bridge dispatch is still a dispatch here: + # `source` is a public input any manual dispatch can set, so keying a + # cancellable per-PR group on it would let a manual dispatch cancel a + # queued one (and vice versa). group: >- ${{ github.event_name == 'schedule' && 'qwen-autofix-route-cron' || (github.event_name == 'pull_request_review' && (contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.review.author_association) || github.event.review.user.login == 'qwen-code-ci-bot') && format('qwen-autofix-route-pr-{0}', github.event.pull_request.number)) || (github.event_name == 'pull_request' && github.event.label.name == 'autofix/takeover' && format('qwen-autofix-route-label-{0}', github.event.pull_request.number)) || (github.event_name == 'issue_comment' && contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.comment.author_association) && format('qwen-autofix-route-cmd-{0}', github.event.issue.number)) || (github.event_name == 'issues' && format('qwen-autofix-route-issue-{0}', github.event.issue.number)) || format('qwen-autofix-route-{0}', github.run_id) }} cancel-in-progress: |- @@ -1902,6 +1914,7 @@ jobs: FORCED_PR: '${{ needs.route.outputs.pr_number }}' DRY_RUN: '${{ needs.route.outputs.dry_run }}' EVENT_NAME: '${{ github.event_name }}' + DISPATCH_SOURCE: "${{ github.event_name == 'workflow_dispatch' && inputs.source || '' }}" run: |- # Every lane that reaches this scan is supposed to hold the PAT: # route now declines the one event GitHub is known to run without @@ -2707,8 +2720,36 @@ jobs: # #7836, so review submissions stay covered by the # once-per-window pause notice below. No dedup on the dispatch # itself: the shepherd sends at most one per head, and a human - # asking twice deserves two answers. - if [[ -n "${FORCED_PR}" && "${FORCED_PR}" == "${PR}" && "${EVENT_NAME}" == 'workflow_dispatch' ]]; then + # asking twice deserves two answers. fork-bridge dispatches are + # the one dispatch-shaped exception: they are fork-PR reviews + # laundered into dispatch form (a fork's review event carries no + # secrets), not an explicit human/shepherd dispatch — answering + # each one loudly would post one refusal per review on a capped + # fork PR, the exact #7836 spam this gate exists to prevent. + # But `source` is a public workflow_dispatch input any manual + # dispatch can set, so the silence is honored ONLY on positive + # proof of origin: a recent SUCCESSFUL fork-bridge run whose + # title names this exact PR (the bridge propagates the signal's + # run-name into its own title — both base-branch files, not + # fork-forgeable). The window is generous because route backlog + # can queue a dispatch for hours; the PR match, not the window, + # is what proves origin. Unverified → answered like any + # explicit dispatch. + FORK_BRIDGE_VERIFIED=false + if [[ "${DISPATCH_SOURCE}" == 'fork-bridge' ]]; then + BRIDGE_CUTOFF="$(date -u -d '360 minutes ago' +%Y-%m-%dT%H:%M:%SZ)" + if BRIDGE_RUNS="$(gh run list --repo "${REPO}" --workflow qwen-autofix-fork-bridge.yml --limit 20 --json conclusion,createdAt,displayTitle 2> /dev/null)" \ + && jq -e --arg pr "${PR}" --arg cutoff "${BRIDGE_CUTOFF}" ' + [ .[] + | select(.conclusion == "success" and ((.createdAt // "") >= $cutoff)) + | select((.displayTitle // "") | startswith("fork-bridge: fork-signal: PR \($pr) reviewed by ")) ] + | length > 0' <<< "${BRIDGE_RUNS}" > /dev/null; then + FORK_BRIDGE_VERIFIED=true + else + echo "::warning::fork-bridge provenance unverified for #${PR} — answering this dispatch like an explicit one" + fi + fi + if [[ -n "${FORCED_PR}" && "${FORCED_PR}" == "${PR}" && "${EVENT_NAME}" == 'workflow_dispatch' && "${FORK_BRIDGE_VERIFIED}" != 'true' ]]; then if [[ "${DRY_RUN}" == "true" ]]; then echo "🧪 DRY-RUN: would post cap-refused notice on #${PR}" else diff --git a/.github/workflows/qwen-fleet-shepherd.yml b/.github/workflows/qwen-fleet-shepherd.yml index e2df10c8837..b646f2f1569 100644 --- a/.github/workflows/qwen-fleet-shepherd.yml +++ b/.github/workflows/qwen-fleet-shepherd.yml @@ -215,17 +215,42 @@ jobs: # Identify OUR run by id for next tick's in-flight attribution. # Safe correlation window: nothing else in this tick has # dispatched yet (conflict dispatches happen later in the - # walk), so the newest dispatch created since T0 is ours. If it - # never appears, run=none is recorded and the failure mode is a - # duplicate scan, not starvation. + # walk), so a dispatch created since T0 is ours — provided it + # is the ONLY one. The fork-review bridge also dispatches + # autonomously at arbitrary times, and GitHub may expose a + # foreign dispatch before our own run appears in the list. + # So: ANY poll seeing two or more candidates records run=none + # (the window is ambiguous — never attribute a guess), and a + # single candidate is accepted only as the sole candidate + # across two consecutive polls — the FIRST singleton might be + # the foreign one, and our run arriving at the next poll flips + # the window to ambiguous instead of being attributed on top + # of it. Either fallback is the documented one: a duplicate + # scan, never starvation. if [[ "${DRY_RUN}" != "true" ]]; then for _ in 1 2 3 4 5; do sleep 2 - LIVENESS_RUN_OUT="$(env GITHUB_TOKEN="${ACTIONS_TOKEN}" gh run list --repo "${REPO}" --workflow qwen-autofix.yml \ + CANDIDATES="$(env GITHUB_TOKEN="${ACTIONS_TOKEN}" gh run list --repo "${REPO}" --workflow qwen-autofix.yml \ --event workflow_dispatch --limit 5 --json databaseId,createdAt 2> /dev/null \ - | jq -r --arg t0 "${DISPATCH_T0}" '[ .[] | select(.createdAt >= $t0) ] | first | .databaseId // "" | tostring' 2> /dev/null || true)" - if [[ -n "${LIVENESS_RUN_OUT}" && "${LIVENESS_RUN_OUT}" != "null" ]]; then break; fi + | jq -c --arg t0 "${DISPATCH_T0}" '[ .[] | select(.createdAt >= $t0) ]' 2> /dev/null || true)" + COUNT="$(jq 'length' <<< "${CANDIDATES}" 2> /dev/null || echo 0)" + if [[ "${COUNT}" =~ ^[0-9]+$ ]] && [[ "${COUNT}" -gt 1 ]]; then + echo "🫀 ${COUNT} dispatches since ${DISPATCH_T0} — attribution ambiguous (fork bridge?), recording run=none" + ATTRIBUTED='' + break + fi + SEEN_ID='' + if [[ "${COUNT}" == "1" ]]; then + SEEN_ID="$(jq -r '.[0].databaseId // "" | tostring' <<< "${CANDIDATES}")" + fi + if [[ -n "${SEEN_ID}" && "${SEEN_ID}" == "${PREV_SEEN:-}" ]]; then + ATTRIBUTED="${SEEN_ID}" + else + ATTRIBUTED='' + fi + PREV_SEEN="${SEEN_ID}" done + LIVENESS_RUN_OUT="${ATTRIBUTED:-}" fi fi fi diff --git a/scripts/tests/qwen-autofix-fork-bridge-workflow.test.js b/scripts/tests/qwen-autofix-fork-bridge-workflow.test.js new file mode 100644 index 00000000000..d3135a3ecf7 --- /dev/null +++ b/scripts/tests/qwen-autofix-fork-bridge-workflow.test.js @@ -0,0 +1,579 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { spawnSync } from 'node:child_process'; +import { + chmodSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { parse } from 'yaml'; + +const signalPath = '.github/workflows/qwen-autofix-fork-signal.yml'; +const bridgePath = '.github/workflows/qwen-autofix-fork-bridge.yml'; +const autofixPath = '.github/workflows/qwen-autofix.yml'; +const shepherdPath = '.github/workflows/qwen-fleet-shepherd.yml'; +const signalText = readFileSync(signalPath, 'utf8'); +const bridgeText = readFileSync(bridgePath, 'utf8'); +const autofixText = readFileSync(autofixPath, 'utf8'); +const shepherdText = readFileSync(shepherdPath, 'utf8'); +const signal = parse(signalText); +const bridge = parse(bridgeText); +const autofix = parse(autofixText); + +const signalJob = signal.jobs.signal; +const bridgeJob = bridge.jobs.dispatch; +const bridgeScript = bridgeJob.steps[0].run; +const routeConcurrency = autofix.jobs.route.concurrency; + +// Identity literals live in qwen-autofix.yml's top-level env; the fork bridge +// must mirror them, not fork its own copies (see the pinning test below). +const REVIEW_BOT = autofix.env.REVIEW_BOT; +const TRUSTED_ASSOC = JSON.parse(autofix.env.TRUSTED_ASSOC); +const TAKEOVER_LABEL = autofix.env.TAKEOVER_LABEL; +const SKIP_LABEL = autofix.env.SKIP_LABEL; +const BOT_FALLBACK = autofix.env.AUTOFIX_BOT.match( + /vars\.AUTOFIX_BOT_LOGIN \|\| '([^']+)'/, +)?.[1]; +const TRUSTED_ASSOC_LITERAL = `["${TRUSTED_ASSOC.join('", "')}"]`; + +// The run-name formats are load-bearing cross-file contracts: the signal +// encodes the reviewed PR and reviewer for the bridge, and the bridge +// propagates the signal's title so the scan can verify fork-bridge +// provenance. Keep these in lockstep with the workflow files. +const SIGNAL_RUN_NAME = `fork-signal: PR \${{ github.event.pull_request.number }} reviewed by \${{ github.event.review.user.login }}`; +const BRIDGE_RUN_NAME = + 'fork-bridge: ${{ github.event.workflow_run.display_title }}'; + +// Does the file actually READ a secret, as opposed to naming one in prose? +// Both workflows explain themselves by referring to `secrets.CI_DEV_BOT_PAT`, +// so only an expression counts — and the expression scan must survive braces +// INSIDE the expression (`format('{0}', secrets.X)` or a fromJSON object +// literal), which a `[^}]*` body cannot cross. Scan each `${{ … }}` +// expression with a pass that skips single-quoted string literals (with `''` +// escapes) when locating the closing `}}`, then test the body. +const expressionsIn = (text) => { + const expressions = []; + let start = text.indexOf('${{'); + while (start !== -1) { + let i = start + 3; + let inString = false; + let end = -1; + while (i < text.length) { + const ch = text[i]; + if (inString) { + if (ch === "'") { + if (text[i + 1] === "'") { + i += 2; + continue; + } + inString = false; + } + } else if (ch === "'") { + inString = true; + } else if (ch === '}' && text[i + 1] === '}') { + end = i; + break; + } + i += 1; + } + if (end === -1) break; + expressions.push(text.slice(start + 3, end)); + start = text.indexOf('${{', end + 2); + } + return expressions; +}; +const readsASecret = (text) => + expressionsIn(text).some((expression) => /\bsecrets\./.test(expression)); + +describe('qwen autofix fork bridge', () => { + it('keeps the trigger name and dispatch target wired together', () => { + // Cross-FILE contracts that no single file can be read to verify. Each + // one, broken, leaves both files individually valid and the bridge + // permanently silent — the exact failure this suite exists to catch. + + // `workflow_run` matches on the triggering workflow's `name:`, not its + // path, so renaming the signal workflow decouples the two silently. + expect(bridge.on.workflow_run.workflows).toEqual([signal.name]); + expect(bridge.on.workflow_run.types).toEqual(['completed']); + + // The signal side of the trigger contract: `pull_request_review` with + // type `submitted`. Flipping the type (say to `dismissed`) leaves both + // files individually valid and silently kills real-time fork-PR pickup. + expect(Object.keys(signal.on)).toEqual(['pull_request_review']); + expect(signal.on.pull_request_review.types).toEqual(['submitted']); + + // The dispatch target must exist and accept the inputs the bridge + // passes — INCLUDING their types: the bridge sends `-f source=…` as a + // string field, which GitHub rejects against a boolean input (every + // dispatch failing after three retries), and shepherd/manual dispatches + // omit both inputs, so neither may become required. + expect(bridgeScript).toContain('gh workflow run qwen-autofix.yml'); + expect(bridgeScript).toContain('-f pr_number='); + expect(bridgeScript).toContain('-f source=fork-bridge'); + expect(Object.keys(autofix.on.workflow_dispatch.inputs)).toEqual( + expect.arrayContaining(['pr_number', 'source']), + ); + expect(autofix.on.workflow_dispatch.inputs.source.type).toBe('string'); + expect(autofix.on.workflow_dispatch.inputs.source.required).toBe(false); + expect(autofix.on.workflow_dispatch.inputs.pr_number.type).toBe('string'); + expect(autofix.on.workflow_dispatch.inputs.pr_number.required).toBe(false); + + // The PR binding rides the signal's run-name (surfaced on the + // workflow_run event as display_title) — there is NO artifact channel. + // A resurrected one would reintroduce a binding that read a benign + // concurrent push as a forgery. + expect(signalText).not.toContain('upload-artifact'); + expect(signalText).not.toContain('pr-number.txt'); + expect(bridgeScript).not.toContain('gh run download'); + expect(signal['run-name']).toBe(SIGNAL_RUN_NAME); + expect(bridge['run-name']).toBe(BRIDGE_RUN_NAME); + expect(bridgeJob.env.SIGNAL_TITLE).toBe( + '${{ github.event.workflow_run.display_title }}', + ); + // The head SHA binding — the freshness gate the resolved PR is checked + // against — and its parse format are pinned too: rebinding either to + // `head_branch` silently resolves nothing forever (SHA-vs-branch + // comparison), and a format change on either side breaks the parse. + expect(bridgeJob.env.SIGNAL_HEAD_SHA).toBe( + '${{ github.event.workflow_run.head_sha }}', + ); + expect(bridgeScript).toContain( + "TITLE_RE='^fork-signal: PR ([0-9]+) reviewed by (.+)$'", + ); + // The bound PR is re-read live and re-checked against the reviewed + // head — never enumerated-and-guessed. + expect(bridgeScript).toContain('gh pr view "${SIGNAL_PR}"'); + expect(bridgeScript).toContain( + '--json number,author,labels,headRefOid,isCrossRepository,maintainerCanModify,state,baseRefName', + ); + expect(bridgeScript).toContain('headRefOid == $sha'); + expect(bridgeScript).toContain( + '.state == "OPEN" and .baseRefName == "main"', + ); + }); + + it('gives the fork-triggered half nothing worth stealing', () => { + // The signal is the ONLY half a fork PR's event can reach. GitHub already + // withholds secrets there, but the token is not withheld — so the + // permissions are emptied explicitly, and nothing in it may read a secret + // or run repository code. + expect(signal.permissions).toEqual({}); + // Matched as an EXPRESSION, not as a substring: both files discuss + // `secrets.CI_DEV_BOT_PAT` in prose to explain why they exist, and a + // grep-shaped assertion would either fail on that prose or be deleted. + expect(readsASecret(signalText)).toBe(false); + expect(signalText).not.toContain('actions/checkout'); + // Never the persistent pool: a self-hosted workspace outlives the job, + // and fork-triggered work must not land in one. + // Pinned as the resolved value, not as the absence of the word + // "self-hosted" — the comment above the field explains the rule and + // would fail a substring check. + expect(signalJob['runs-on']).toBe('ubuntu-latest'); + + // The bridge holds real power (`actions: write` dispatches the lane that + // pushes commits), so it must not ALSO hold the PAT — that combination in + // one run is what makes a pwn-request worth attempting. The exact scope + // set is pinned so future creep trips the test that polices it: + // `contents: read` is not decorative — the reviewer live-permission + // re-check reads the collaborators API, exactly as route does under the + // same scope. + expect(bridge.permissions).toEqual({ + actions: 'write', + contents: 'read', + 'pull-requests': 'read', + }); + expect(readsASecret(bridgeText)).toBe(false); + expect(bridgeText).not.toContain('actions/checkout'); + + // The detector itself must not be defeated by braces inside the + // expression — proven payloads that a `[^}]*`-shaped scan lets through. + expect( + readsASecret('LEAKED: "${{ format(\'{0}\', secrets.CI_DEV_BOT_PAT) }}"'), + ).toBe(true); + expect( + readsASecret('${{ fromJSON(\'{"a":1}\').a == secrets.CI_DEV_BOT_PAT }}'), + ).toBe(true); + // Plain reads are still caught, and prose stays clean. + expect(readsASecret('X: "${{ secrets.CI_DEV_BOT_PAT }}"')).toBe(true); + expect(readsASecret('prose about secrets.CI_DEV_BOT_PAT here')).toBe(false); + }); + + it('signals only the PRs the direct lane cannot serve', () => { + // The whole gate is pinned VERBATIM: its connectives and branch order + // are load-bearing (a flipped `||`/`&&` silently degrades fork reviews + // to the throttled cron backstop or lets arbitrary PRs through), and a + // substring pin cannot see them. Built from the pinned constants so a + // change in qwen-autofix.yml fails HERE. + expect(signalJob.if).toBe( + [ + "${{ github.repository == 'QwenLM/qwen-code'", + '&& github.event.pull_request.head.repo.full_name != github.repository', + "&& github.event.pull_request.base.ref == 'main'", + "&& github.event.pull_request.state == 'open'", + '&& github.event.pull_request.maintainer_can_modify == true', + `&& (github.event.pull_request.user.login == (vars.AUTOFIX_BOT_LOGIN || '${BOT_FALLBACK}')`, + `|| contains(toJSON(github.event.pull_request.labels.*.name), '"${TAKEOVER_LABEL}"'))`, + `&& !contains(toJSON(github.event.pull_request.labels.*.name), '"${SKIP_LABEL}"')`, + `&& (contains(fromJSON('${TRUSTED_ASSOC_LITERAL}'), github.event.review.author_association)`, + `|| github.event.review.user.login == '${REVIEW_BOT}') }}`, + ].join('\n'), + ); + + // The scope guard keeps the chain inside this repository — pinned for + // the bridge job as well. + expect(bridgeJob.if).toBe( + [ + "${{ github.repository == 'QwenLM/qwen-code'", + "&& github.event.workflow_run.conclusion == 'success' }}", + ].join('\n'), + ); + + // The bridge re-checks admission against LIVE state — consent can move + // between the review and the dispatch — and SKIP wins everywhere. The + // reviewer identity is re-checked LIVE too: association alone does not + // guarantee write permission. + expect(bridge.env.TAKEOVER_LABEL).toBe(TAKEOVER_LABEL); + expect(bridge.env.SKIP_LABEL).toBe(SKIP_LABEL); + expect(bridge.env.AUTOFIX_BOT).toBe(autofix.env.AUTOFIX_BOT); + expect(bridge.env.REVIEW_BOT).toBe(REVIEW_BOT); + expect(bridgeScript).toContain('maintainerCanModify == true'); + expect(bridgeScript).toContain('index($take)'); + expect(bridgeScript).toContain('index($skip)'); + expect(bridgeScript).toContain( + 'gh api "repos/${REPO}/collaborators/${SIGNAL_REVIEWER}/permission"', + ); + expect(bridgeScript).toContain( + 'if [[ "${SIGNAL_REVIEWER}" != "${REVIEW_BOT}" ]]; then', + ); + }); + + it('keeps every identity literal pinned to qwen-autofix.yml', () => { + // The fork bridge cannot import qwen-autofix.yml's env, so the copies it + // must make are pinned HERE: if the review-bot identity, the trust list, + // the labels, or the bot login ever change there, this test fails + // instead of the signal silently stopping to fire. + + // The review-bot login appears in BOTH the job gate and the trust-keyed + // concurrency group; every occurrence must equal REVIEW_BOT. + const botComparisons = + signalText.match(/github\.event\.review\.user\.login == '([^']+)'/g) ?? + []; + expect(botComparisons.length).toBeGreaterThanOrEqual(2); + for (const comparison of botComparisons) { + expect(comparison).toBe( + `github.event.review.user.login == '${REVIEW_BOT}'`, + ); + } + + // Every fromJSON'd association list (job gate + concurrency group) must + // equal TRUSTED_ASSOC. + const assocLiterals = [ + ...signalText.matchAll(/fromJSON\('(\[[^\]]*\])'\)/g), + ].map((m) => JSON.parse(m[1])); + expect(assocLiterals.length).toBeGreaterThanOrEqual(2); + for (const list of assocLiterals) { + expect(list).toEqual(TRUSTED_ASSOC); + } + + // The takeover and skip labels in the gate are quoted as JSON-array + // ELEMENTS so a longer label sharing the prefix cannot match. + expect(signalJob.if).toContain(`'"${TAKEOVER_LABEL}"'`); + expect(signalJob.if).toContain(`'"${SKIP_LABEL}"'`); + }); + + it('isolates gated-out runs so they cannot cancel the real ones', () => { + // Signal: concurrency is evaluated BEFORE the job `if:`, so an untrusted + // review would otherwise cancel a queued or in-flight trusted signal and + // then skip its own job — silently dropping the maintainer's review. + // Trusted reviews coalesce per PR; everything else gets a run-unique + // group, mirroring route's pattern. Pinned VERBATIM: the `&&`/`||` glue + // between the trust clause and the two formats is exactly what routes a + // review to one group or the other, and substring pins cannot see a + // flipped connective. + expect(signal.concurrency.group).toBe( + "${{ (contains(fromJSON('" + + TRUSTED_ASSOC_LITERAL + + "'), github.event.review.author_association) || github.event.review.user.login == '" + + REVIEW_BOT + + "') && format('qwen-autofix-fork-signal-pr-{0}', github.event.pull_request.number) || format('qwen-autofix-fork-signal-{0}', github.run_id) }}", + ); + expect(signal.concurrency['cancel-in-progress']).toBe(true); + + // Bridge: EVERY signal run fires `workflow_run: completed` — skipped and + // cancelled ones included. Without the conclusion in the group key, a + // skipped bridge run would cancel the in-flight real one and then skip + // its own job; repeating the ungated review could hold the bridge off a + // fork PR indefinitely. + expect(bridge.concurrency.group).toBe( + 'qwen-autofix-fork-bridge-${{ github.event.workflow_run.conclusion }}-${{ github.event.workflow_run.head_sha }}', + ); + expect(bridge.concurrency['cancel-in-progress']).toBe(true); + }); + + it('keeps bridged dispatches run-unique and wires source for provenance', () => { + // `source` is a public workflow_dispatch input, so no dispatch may earn + // trusted per-PR coalescing by asserting an origin: route keeps EVERY + // dispatch in a run-unique never-cancelled group, and fork-review bursts + // coalesce upstream instead (the signal per PR, the bridge per + // conclusion+head). The marker's one remaining effect — staying quiet at + // the round cap — is honored by review-scan only after verifying + // provenance (pinned behaviorally in qwen-autofix-workflow.test.js). + expect(routeConcurrency.group).not.toContain('inputs.source'); + expect(routeConcurrency.group).not.toContain('forkbridge'); + expect(routeConcurrency['cancel-in-progress']).toBe( + "${{ github.event_name != 'workflow_dispatch' }}", + ); + + // The wiring the scan's provenance check relies on: the marker reaches + // review-scan as DISPATCH_SOURCE, and the bridge names the PR in its run + // title so the scan has something GitHub-owned to match. + expect(autofixText).toContain( + "DISPATCH_SOURCE: \"${{ github.event_name == 'workflow_dispatch' && inputs.source || '' }}\"", + ); + }); + + it('keeps the shepherd attribution honest about foreign dispatches', () => { + // The shepherd correlates its own liveness dispatch by polling for + // dispatches created since T0. The bridge dispatches autonomously at + // arbitrary times, and GitHub may expose a foreign dispatch before the + // shepherd's own run appears — so an ambiguous window records run=none, + // and a singleton is accepted only once it survives a second consecutive + // poll (never on first sight). Behavioral proof lives in the shepherd + // suite; this pins the mechanism's presence across the file boundary. + expect(shepherdText).toContain('DISPATCH_T0='); + expect(shepherdText).toContain('--event workflow_dispatch --limit 5'); + expect(shepherdText).toContain('[ .[] | select(.createdAt >= $t0) ]'); + expect(shepherdText).toContain('if [[ "${COUNT}" == "1" ]]; then'); + expect(shepherdText).toContain('attribution ambiguous'); + expect(shepherdText).toContain('two consecutive polls'); + }); + + it('documents what #8671 actually changed', () => { + // The signal header used to cite route's `secretless` output — a + // mechanism #8671 never shipped (it declined fork reviews inline in + // route and added the empty-PAT fail-fast instead, recording the + // route-output design as the rejected alternative). The header must say + // what actually happened, and nothing may pin the wrong word again. + expect(signalText).not.toContain('secretless'); + expect(signalText).not.toContain('REQUIRES #8671'); + expect(signalText).toContain('63a99c2c11'); + expect(signalText).toContain( + "route's decide script declines\n# fork `pull_request_review` events inline", + ); + expect(signalText).toContain( + 'review-scan fails fast at the\n# top on an empty PAT', + ); + }); + + it('resolves the bound PR and dispatches it alone', () => { + // The review's PR number and reviewer ride the signal's run-name — a + // base-branch file the fork cannot edit — so a head SHA shared by two + // open fork PRs dispatches only the reviewed one, and the reviewer's + // LIVE permission is re-checked before the secret-bearing lane starts. + // Replayed against a stub API. + const managedForkPr = (over = {}) => ({ + number: 8436, + author: { login: BOT_FALLBACK }, + labels: [], + headRefOid: 'b8ac04ef', + isCrossRepository: true, + maintainerCanModify: true, + state: 'OPEN', + baseRefName: 'main', + ...over, + }); + const runBridge = ({ + signalTitle = 'fork-signal: PR 8436 reviewed by a-maintainer', + signalHeadSha = 'b8ac04ef', + viewed = managedForkPr(), + viewFails = false, + reviewerPermission = 'write', + permissionLookupFails = false, + dispatchFails = 0, + }) => { + const dir = mkdtempSync(join(tmpdir(), 'fork-bridge-')); + try { + const callsFile = join(dir, 'calls'); + const summaryFile = join(dir, 'summary'); + writeFileSync(callsFile, ''); + writeFileSync(summaryFile, ''); + writeFileSync( + join(dir, 'gh'), + `#!/bin/bash +printf '%q ' "$@" >> '${callsFile}' +printf '\\n' >> '${callsFile}' +if [[ "$1" == 'api' ]]; then + ${permissionLookupFails ? 'exit 1' : ''} + printf '%s' '${reviewerPermission}' + exit 0 +fi +if [[ "$1" == 'pr' && "$2" == 'view' ]]; then + ${viewFails ? 'exit 1' : ''} + printf '%s' '${JSON.stringify(viewed).replace(/'/g, "'\\''")}' + exit 0 +fi +if [[ "$1" == 'workflow' && "$2" == 'run' ]]; then + n="$(cat '${dir}/dispatches' 2>/dev/null || echo 0)" + n=$(( n + 1 )); printf '%s' "$n" > '${dir}/dispatches' + [[ "$n" -le ${dispatchFails} ]] && exit 1 + exit 0 +fi +exit 1 +`, + ); + chmodSync(join(dir, 'gh'), 0o755); + const result = spawnSync( + 'bash', + [ + '-c', + // Production shell for a `run:` step under `defaults.run.shell: + // bash` — errexit and pipefail both live. + ['set -eo pipefail', 'sleep() { :; }', bridgeScript].join('\n'), + ], + { + env: { + ...process.env, + PATH: `${dir}:${process.env.PATH}`, + REPO: 'QwenLM/qwen-code', + SIGNAL_HEAD_SHA: signalHeadSha, + SIGNAL_TITLE: signalTitle, + GITHUB_TOKEN: 'stub', + GITHUB_SERVER_URL: 'https://github.com', + GITHUB_STEP_SUMMARY: summaryFile, + AUTOFIX_BOT: BOT_FALLBACK, + REVIEW_BOT, + TAKEOVER_LABEL, + SKIP_LABEL, + }, + encoding: 'utf8', + }, + ); + return { + status: result.status, + stdout: result.stdout, + stderr: result.stderr, + calls: readFileSync(callsFile, 'utf8'), + summary: readFileSync(summaryFile, 'utf8'), + }; + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }; + + // Happy path: a trusted reviewer, and the bound PR still carries the + // reviewed head and is still managed. + const ok = runBridge({}); + expect({ status: ok.status, stderr: ok.stderr }).toEqual({ + status: 0, + stderr: '', + }); + expect(ok.calls).toContain('collaborators/a-maintainer/permission'); + expect(ok.calls).toContain('pr_number=8436'); + expect(ok.calls).toContain('source=fork-bridge'); + expect(ok.calls.match(/workflow run/g) ?? []).toHaveLength(1); + expect(ok.stdout).toContain('dispatched the autofix review lane'); + expect(ok.summary).toContain('#8436'); + + // The review bot keeps its route-mirrored exception: dispatched without + // any collaborator-permission lookup. + const botReview = runBridge({ + signalTitle: `fork-signal: PR 8436 reviewed by ${REVIEW_BOT}`, + }); + expect(botReview.status).toBe(0); + expect(botReview.calls).toContain('pr_number=8436'); + expect(botReview.calls).not.toContain('collaborators/'); + + // A bot-suffixed reviewer (org-member app) parses and dispatches too — + // the capture must not choke on the brackets. + const botSuffix = runBridge({ + signalTitle: 'fork-signal: PR 8436 reviewed by some-app[bot]', + }); + expect(botSuffix.status).toBe(0); + expect(botSuffix.calls).toContain('pr_number=8436'); + + // A below-write reviewer is declined LIVE, exactly as the direct lane + // declines the same review; a failed lookup fails the same way closed. + for (const over of [ + { reviewerPermission: 'triage' }, + { reviewerPermission: 'read' }, + { reviewerPermission: '' }, + { permissionLookupFails: true }, + ]) { + const declined = runBridge(over); + expect(declined.status).toBe(0); + expect(declined.stdout).toContain('below write'); + expect(declined.calls).not.toContain('workflow run'); + } + + // An unparseable run title is a broken signal/bridge contract — red, + // never a silent no-op or a guessed dispatch. + const unparseable = runBridge({ signalTitle: 'some unexpected title' }); + expect(unparseable.status).toBe(1); + expect(unparseable.stdout).toContain('could not parse the PR binding'); + expect(unparseable.calls).not.toContain('workflow run'); + + // A benign concurrent push: the head moved between the review and the + // bridge. Green no-op — NOT a red run reading the race as a forgery — + // and nothing is dispatched on a stale head. + const moved = runBridge({ signalHeadSha: 'deadbeef' }); + expect(moved.status).toBe(0); + expect(moved.stdout).toContain('no longer carries head'); + expect(moved.calls).not.toContain('workflow run'); + + // Admission re-derived from live state: each withdrawn consent is a + // green no-op, never a dispatched run. + for (const over of [ + { maintainerCanModify: false }, // edits turned off since the review + { isCrossRepository: false }, // retargeted/converted to in-repo + { + author: { login: 'a-contributor' }, + }, // neither bot-authored nor takeover-labeled + { labels: [{ name: SKIP_LABEL }, { name: TAKEOVER_LABEL }] }, // skip wins + { state: 'CLOSED' }, // closed since the review + { baseRefName: 'release' }, // retargeted away from main + ]) { + const withdrawn = runBridge({ viewed: managedForkPr(over) }); + expect(withdrawn.status).toBe(0); + expect(withdrawn.calls).not.toContain('workflow run'); + } + + // A takeover-labeled human PR with edits on IS dispatched — to the + // bound number only, even though the head could be shared. + const takeover = runBridge({ + signalTitle: 'fork-signal: PR 900 reviewed by a-maintainer', + viewed: managedForkPr({ + number: 900, + author: { login: 'a-contributor' }, + labels: [{ name: TAKEOVER_LABEL }], + }), + }); + expect(takeover.status).toBe(0); + expect(takeover.calls).toContain('pr_number=900'); + expect(takeover.calls.match(/pr_number=/g) ?? []).toHaveLength(1); + + // An unreadable bound PR is red: the bridge cannot tell a signal it + // should have served from one it should not, and staying silent would + // look exactly like a healthy bridge with nothing to do. + expect(runBridge({ viewFails: true }).status).toBe(1); + + // A transient dispatch failure retries; a persistent one reds the run + // after three attempts instead of dropping the review on the floor. + const flaky = runBridge({ dispatchFails: 1 }); + expect(flaky.status).toBe(0); + expect(flaky.stdout).toContain('dispatched the autofix review lane'); + expect(flaky.calls.match(/workflow run/g) ?? []).toHaveLength(2); + const dead = runBridge({ dispatchFails: 9 }); + expect(dead.status).toBe(1); + expect(dead.stderr).toContain('(attempt 3/3)'); + expect(dead.calls.match(/workflow run/g) ?? []).toHaveLength(3); + }); +}); diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index a78a4973914..638f6dd8fd6 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -1842,7 +1842,11 @@ describe('qwen-autofix workflow', () => { // Per-TARGET keys: cron ticks coalesce with each other; review events // coalesce per PR (near-simultaneous reviews on one PR route once, without // events on OTHER PRs cancelling this one); issue events per issue; - // dispatches unique and never cancelled. + // dispatches unique and never cancelled — fork-bridge dispatches + // included: `source` is a public workflow_dispatch input, so no dispatch + // may claim trusted per-PR coalescing by asserting an origin; fork-review + // bursts coalesce upstream instead (the signal per PR, the bridge per + // conclusion+head). expect(routeJob).toContain("'qwen-autofix-route-cron'"); expect(routeJob).toContain( "format('qwen-autofix-route-pr-{0}', github.event.pull_request.number)", @@ -1853,6 +1857,12 @@ describe('qwen-autofix workflow', () => { expect(routeJob).toContain( "format('qwen-autofix-route-{0}', github.run_id)", ); + // The public `source` marker must not buy any dispatch a shared group or + // cancellation rights: pin the forkbridge branch OUT (order-blind + // substring pins cannot see a re-added disjunct before the run-id + // fallback, but an absent one cannot hide). + expect(routeJob).not.toContain('forkbridge'); + expect(routeJob).not.toContain("inputs.source == 'fork-bridge'"); expect(routeJob).toContain( "cancel-in-progress: |-\n ${{ github.event_name != 'workflow_dispatch' }}", ); @@ -2780,9 +2790,27 @@ describe('qwen-autofix workflow', () => { // spammed 7 refusals on #7836 — those stay covered by the // once-per-window pause notice. No dedup on the dispatch itself: the // shepherd sends at most one per head, and a human asking twice - // deserves two answers. + // deserves two answers. fork-bridge dispatches are the one + // dispatch-shaped exception — they are fork-PR reviews laundered into + // dispatch form, so answering each one loudly would post one refusal + // per review on a capped fork PR (the exact #7836 spam this gate + // prevents). But `source` is a public workflow_dispatch input a manual + // dispatch can set, so the silence is honored ONLY on positive proof of + // origin: a recent SUCCESSFUL fork-bridge run whose title names this + // exact PR. Unverified markers are answered like explicit dispatches. expect(reviewScanJob).toContain( - 'if [[ -n "${FORCED_PR}" && "${FORCED_PR}" == "${PR}" && "${EVENT_NAME}" == \'workflow_dispatch\' ]]; then', + "DISPATCH_SOURCE: \"${{ github.event_name == 'workflow_dispatch' && inputs.source || '' }}\"", + ); + expect(reviewScanJob).toContain('FORK_BRIDGE_VERIFIED=false'); + expect(reviewScanJob).toContain( + '--workflow qwen-autofix-fork-bridge.yml --limit 20 --json conclusion,createdAt,displayTitle', + ); + expect(reviewScanJob).toContain( + 'startswith("fork-bridge: fork-signal: PR \\($pr) reviewed by ")', + ); + expect(reviewScanJob).toContain('fork-bridge provenance unverified'); + expect(reviewScanJob).toContain( + 'if [[ -n "${FORCED_PR}" && "${FORCED_PR}" == "${PR}" && "${EVENT_NAME}" == \'workflow_dispatch\' && "${FORK_BRIDGE_VERIFIED}" != \'true\' ]]; then', ); expect(reviewScanJob).toContain(''); expect(reviewScanJob).toContain('Dispatch refused'); @@ -2790,25 +2818,76 @@ describe('qwen-autofix workflow', () => { expect(reviewScanJob).toContain( 'cap-refused notice skipped: PAT authenticates as', ); + // Provenance is a GitHub-records check: success conclusion, a title + // naming the exact PR (no prefix collisions), and a recent createdAt. + // Replay the extracted jq program VERBATIM so a dropped predicate fails + // the test, not just a substring. + const provenanceJq = reviewScanJob.match( + /jq -e --arg pr "\$\{PR\}" --arg cutoff "\$\{BRIDGE_CUTOFF\}" '([\s\S]*?)' <<< "\$\{BRIDGE_RUNS\}"/, + )?.[1]; + expect(provenanceJq).toBeTruthy(); + const verifies = (runs, pr = '7836', cutoff = '2026-08-07T04:00:00Z') => + spawnSync( + 'jq', + ['-e', '--arg', 'pr', pr, '--arg', 'cutoff', cutoff, provenanceJq], + { + input: JSON.stringify(runs), + encoding: 'utf8', + }, + ).status === 0; + const bridgeRun = (displayTitle, over = {}) => ({ + conclusion: 'success', + createdAt: '2026-08-07T10:00:00Z', + displayTitle, + ...over, + }); + const MATCHING = 'fork-bridge: fork-signal: PR 7836 reviewed by wenshao'; + expect(verifies([bridgeRun(MATCHING)])).toBe(true); + // A different PR's bridge run proves nothing. + expect( + verifies([ + bridgeRun('fork-bridge: fork-signal: PR 999 reviewed by wenshao'), + ]), + ).toBe(false); + // No prefix collision: PR 78366 is not PR 7836. + expect( + verifies([ + bridgeRun('fork-bridge: fork-signal: PR 78366 reviewed by wenshao'), + ]), + ).toBe(false); + // Only SUCCESSFUL bridge runs count, and only recent ones. + expect(verifies([bridgeRun(MATCHING, { conclusion: 'failure' })])).toBe( + false, + ); + expect( + verifies([bridgeRun(MATCHING, { createdAt: '2026-08-07T03:00:00Z' })]), + ).toBe(false); + expect(verifies([])).toBe(false); // The loud refusal is gated on workflow_dispatch (FORCED_PR is also set - // for trusted review submissions). Replay the guard VERBATIM so a - // dropped EVENT_NAME condition fails the test, not just a substring: a - // dispatch is answered, a review submission is left to the pause notice. + // for trusted review submissions) and EXCLUDES only PROVENANCE-VERIFIED + // fork-bridge dispatches. Replay the guard VERBATIM so a dropped + // condition fails the test, not just a substring: an explicit dispatch + // is answered, a verified fork-bridge dispatch stays silent, and a + // fork-bridge MARKER without verification is answered like an explicit + // dispatch (the replay cannot call the API — verification state rides + // FORK_BRIDGE_VERIFIED, set by the jq program replayed above). const refusedGuard = reviewScanJob.match( - /(if \[\[ -n "\$\{FORCED_PR\}" && "\$\{FORCED_PR\}" == "\$\{PR\}" && "\$\{EVENT_NAME\}" == 'workflow_dispatch' \]\]; then)/, + /(if \[\[ -n "\$\{FORCED_PR\}" && "\$\{FORCED_PR\}" == "\$\{PR\}" && "\$\{EVENT_NAME\}" == 'workflow_dispatch' && "\$\{FORK_BRIDGE_VERIFIED\}" != 'true' \]\]; then)/, )?.[1]; expect(refusedGuard).toBeTruthy(); - const refuses = (eventName) => + const refuses = (eventName, forkBridgeVerified = 'false') => execFileSync('bash', ['-c', `${refusedGuard}\necho REFUSED\nfi`], { env: { ...process.env, FORCED_PR: '7836', PR: '7836', EVENT_NAME: eventName, + FORK_BRIDGE_VERIFIED: forkBridgeVerified, }, encoding: 'utf8', }).trim(); expect(refuses('workflow_dispatch')).toContain('REFUSED'); + expect(refuses('workflow_dispatch', 'true')).not.toContain('REFUSED'); expect(refuses('pull_request_review')).not.toContain('REFUSED'); // The standard-mode pause and the refusal both point at the actual // recovery command as a printf ARG (the takeover variant keeps its diff --git a/scripts/tests/qwen-fleet-shepherd-workflow.test.js b/scripts/tests/qwen-fleet-shepherd-workflow.test.js index 79ba4966e01..dbaca67e030 100644 --- a/scripts/tests/qwen-fleet-shepherd-workflow.test.js +++ b/scripts/tests/qwen-fleet-shepherd-workflow.test.js @@ -271,6 +271,127 @@ describe('fleet shepherd workflow', () => { ); }); + it('behaviorally proves liveness attribution refuses an ambiguous dispatch window', () => { + // The fork-review bridge dispatches autonomously at arbitrary times, so + // the "a dispatch since T0 is ours" window can hold TWO dispatches — + // and GitHub may expose a foreign dispatch before our own run appears. + // Extract the correlation loop VERBATIM (drift fails the test) and + // replay it with a PATH-stubbed gh: a singleton is attributed only once + // it is the sole candidate across two consecutive polls, an ambiguous + // window records run=none instead of tracking a foreign run, and + // dispatches older than T0 do not count. + expect(workflow).toContain('if [[ "${COUNT}" == "1" ]]; then'); + expect(workflow).toContain('attribution ambiguous'); + expect(workflow).toContain('two consecutive polls'); + const loop = workflow.match( + /(for _ in 1 2 3 4 5; do\n[\s\S]*?\n {16}done\n {16}LIVENESS_RUN_OUT="\$\{ATTRIBUTED:-\}")/, + )?.[1]; + expect(loop).toBeTruthy(); + const replay = (ghScript) => { + const dir = mkdtempSync(join(tmpdir(), 'shepherd-corr-')); + try { + writeFileSync(join(dir, 'gh'), `#!/bin/bash\n${ghScript}\n`); + chmodSync(join(dir, 'gh'), 0o755); + // The loop logs an ambiguity note to stdout; the VALUE under test + // is the recorded run id, printed last. + return execFileSync( + 'bash', + [ + '-c', + `set -eo pipefail +sleep() { :; } +${loop.replace(/\n {16}/g, '\n')} +printf '%s' "${'$'}{LIVENESS_RUN_OUT:-}"`, + ], + { + env: { + ...process.env, + PATH: `${dir}:${process.env.PATH}`, + ACTIONS_TOKEN: 'x', + REPO: 'QwenLM/qwen-code', + DISPATCH_T0: '2026-08-07T08:00:00Z', + DRY_RUN: 'false', + }, + encoding: 'utf8', + }, + ) + .split('\n') + .at(-1); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }; + const staticStub = (dispatches) => + `if [[ "$1" == 'run' && "$2" == 'list' ]]; then + printf '%s' '${JSON.stringify(dispatches)}' + exit 0 +fi +exit 1`; + const run = (databaseId, createdAt) => ({ databaseId, createdAt }); + // Exactly one dispatch in the window, stable across the polls → ours, + // attributed by id. + expect(replay(staticStub([run(900001, '2026-08-07T08:00:05Z')]))).toBe( + '900001', + ); + // Two dispatches in the window (the bridge raced us) → run=none; the + // heuristic must not attribute a run it cannot prove is its own. + expect( + replay( + staticStub([ + run(900001, '2026-08-07T08:00:05Z'), + run(900002, '2026-08-07T08:00:06Z'), + ]), + ), + ).toBe(''); + // Only dispatches older than T0 → nothing attributed. + expect(replay(staticStub([run(899999, '2026-08-07T07:59:00Z')]))).toBe(''); + // Nothing at all → the polling loop exhausts and records nothing. + expect(replay(staticStub([]))).toBe(''); + + // STATEFUL stubs: a static stub cannot tell single-poll from five-poll, + // so loop mutants escape it. These sequences change per call (the last + // line repeats), proving the retry-until-appears semantics and the + // stabilization window. + const statefulStub = (responses) => { + const dir = mkdtempSync(join(tmpdir(), 'shepherd-corr-seq-')); + try { + writeFileSync(join(dir, 'responses'), responses.join('\n')); + const script = `if [[ "$1" == 'run' && "$2" == 'list' ]]; then + n="$(cat '${dir}/polls' 2>/dev/null || echo 0)" + n=$(( n + 1 )); printf '%s' "$n" > '${dir}/polls' + line="$(sed -n "$((${responses.length} < n ? ${responses.length} : n))p" '${dir}/responses')" + printf '%s' "$line" + exit 0 +fi +exit 1`; + return replay(script); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }; + // First poll empty, our run appears on the second → still attributed. + // A loop that broke after the first empty poll would record nothing. + expect( + statefulStub([ + '[]', + JSON.stringify([run(900001, '2026-08-07T08:00:05Z')]), + ]), + ).toBe('900001'); + // The filed defect: a FOREIGN dispatch is visible before our own run. + // Accepting the first singleton would attribute the foreign id; the + // stabilization window sees two candidates on the next poll and records + // run=none instead. + expect( + statefulStub([ + JSON.stringify([run(900002, '2026-08-07T08:00:05Z')]), + JSON.stringify([ + run(900002, '2026-08-07T08:00:05Z'), + run(900001, '2026-08-07T08:00:07Z'), + ]), + ]), + ).toBe(''); + }); + it('behaviorally proves a failed jobs read yields unknown busy-state, not an empty busy-set', () => { // Extract the busy-set walk VERBATIM (drift fails the test) and replay it // with a PATH-stubbed gh: one live run whose jobs read fails must flip