diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index ffa84855409..f4add44a958 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -1881,6 +1881,16 @@ jobs: ${{ needs.route.outputs.do_review == 'true' }} runs-on: 'ubuntu-latest' timeout-minutes: 15 + # A forced scan can write the same status comment as review-address. + # Share its per-PR lock so neither writer can erase the other's state. + # Keep the predicate as narrow as the job's own `if:` — concurrency is + # evaluated BEFORE it, so without the do_review conjunct a dispatch with + # `phase: issue` + `pr_number: N` (route emits pr_number unconditionally) + # would park this skipped job in that PR's shared slot behind a 300-minute + # address round, stalling the issue phase that `needs` it. + concurrency: + group: "qwen-pr-head-write-${{ needs.route.outputs.do_review == 'true' && needs.route.outputs.pr_number || github.run_id }}" + cancel-in-progress: false outputs: targets: '${{ steps.scan.outputs.targets }}' has_targets: '${{ steps.scan.outputs.has_targets }}' @@ -1907,6 +1917,186 @@ jobs: } WORKDIR="$(mktemp -d)" + read_forced_pr_meta() { + local attempt meta + for attempt in 1 2 3; do + if meta="$(gh pr view "${FORCED_PR}" --repo "${REPO}" \ + --json number,state,author,headRefName,isCrossRepository,baseRefName,labels,maintainerCanModify 2> /dev/null)" \ + && jq -e 'type == "object" + and (.number | type == "number") + and (.state | type == "string") + and (.author.login | type == "string") + and (.headRefName | type == "string") + and (.baseRefName | type == "string") + and (.isCrossRepository | type == "boolean") + and (.labels | type == "array") + and (.maintainerCanModify | type == "boolean")' > /dev/null <<< "${meta}"; then + printf '%s' "${meta}" + return 0 + fi + echo "::warning::Forced PR #${FORCED_PR} metadata lookup failed (attempt ${attempt}/3)" >&2 + [[ "${attempt}" -lt 3 ]] && sleep "${attempt}" + done + return 1 + } + + # 'none' and HTTP 404 are DEFINITIVE answers, not lookup failures. + # GitHub returns 200 with permission 'none' for logins that exist but + # hold nothing here (bot-type logins such as dependabot[bot], and org + # logins), and 404 for logins that do not exist or are empty. Both + # mean "no write access" — the routine rejection this gate is for. + # Retrying them would burn 3 API calls plus back-off per candidate per + # scheduled tick, forever, and strand the caller on + # 'permission_lookup_failed': a red forced run (exit 1) whose blocked + # comment promises "a later scheduled scan will retry" — a retry that + # can never succeed — while the actionable "grant the fork author + # write access" guidance behind author_permission_* stays unreachable. + # Only genuinely transient answers (5xx, network, auth) retry. + read_live_permission() { + local login="$1" attempt permission err result='' + # An empty login can only 404; skip the call and answer terminally. + if [[ -z "${login}" ]]; then + printf 'none' + return 0 + fi + err="$(mktemp)" + for attempt in 1 2 3; do + if permission="$(gh api "repos/${REPO}/collaborators/${login}/permission" --jq '.permission // ""' 2> "${err}")" \ + && [[ "${permission}" =~ ^(admin|maintain|write|triage|read|none)$ ]]; then + result="${permission}" + break + fi + if grep -q 'HTTP 404' "${err}"; then + result='none' + break + fi + # Surface gh's own diagnosis instead of discarding it: a rate + # limit, an expired PAT and a 5xx all look identical otherwise. + echo "::warning::Permission lookup failed for ${login} (attempt ${attempt}/3): $(tr '\n' ' ' < "${err}")" >&2 + [[ "${attempt}" -lt 3 ]] && sleep "${attempt}" + done + rm -f "${err}" + [[ -n "${result}" ]] || return 1 + printf '%s' "${result}" + } + + forced_admission_reason() { + jq -r --arg ab "${AUTOFIX_BOT}" --arg take "${TAKEOVER_LABEL}" --arg skip "${SKIP_LABEL}" ' + if (.state // "") != "OPEN" then "not_open" + elif (.baseRefName // "") != "main" then "wrong_base" + elif ([.labels[]?.name] | index($skip) != null) then "skip_label" + elif ((((.author.login // "") == $ab) or ([.labels[]?.name] | index($take) != null)) | not) then "unmanaged_author" + elif (.isCrossRepository == true) and (.maintainerCanModify != true) then "maintainer_edits_disabled" + elif (((.isCrossRepository == true) or (.isCrossRepository == false)) | not) then "cross_repo_state_missing" + else "eligible" + end' + } + + report_forced_takeover_blocked() { + local reason="$1" actor status_ids status_id body attempt status_lookup_ok next_en next_zh err + [[ "${DRY_RUN}" == 'true' ]] && return 0 + [[ "$(jq -r --arg ab "${AUTOFIX_BOT}" --arg take "${TAKEOVER_LABEL}" ' + ((.author.login // "") == $ab) or ([.labels[]?.name] | index($take) != null) + ' <<< "${META}")" == 'true' ]] || return 0 + case "${reason}" in + permission_lookup_failed|author_permission_*|maintainer_edits_disabled|cross_repo_state_missing) ;; + *) return 0 ;; + esac + actor='' + for attempt in 1 2 3; do + if actor="$(gh api user --jq '.login' 2> /dev/null)" && [[ -n "${actor}" ]]; then + break + fi + echo "::warning::PAT identity lookup failed (attempt ${attempt}/3)" >&2 + [[ "${attempt}" -lt 3 ]] && sleep "${attempt}" + done + if [[ "${actor}" != "${AUTOFIX_BOT}" ]]; then + echo "::warning::Blocked takeover status skipped: PAT authenticates as '${actor:-unknown}'" >&2 + return 1 + fi + if [[ "${reason}" == 'maintainer_edits_disabled' ]]; then + next_en='Re-enable maintainer edits on the fork PR to resume takeover.' + next_zh='请在 fork PR 上重新允许 maintainer edits，以恢复 takeover。' + elif [[ "${reason}" == author_permission_* ]]; then + next_en='Grant the fork author write access, or remove the autofix/takeover label, to resume takeover.' + next_zh='请授予 fork 作者 write 权限，或移除 autofix/takeover 标签，以恢复 takeover。' + else + next_en='A later scheduled scan will retry without advancing the feedback watermark.' + next_zh='后续定时扫描会重试，本次不会推进反馈水位。' + fi + # Every other status writer resolves its run link from + # github.server_url / GITHUB_SERVER_URL. Hardcoding github.com here + # would make the one link this message exists to surface the only + # broken one on a GHES or proxied host. + body="$(printf '\n\n⛔ **AutoFix blocked** — takeover admission stopped at `%s`, so no work was started. [View run](%s/%s/actions/runs/%s). %s\n\n
\n中文说明\n\n⛔ **AutoFix 已阻塞** —— takeover 准入停在 `%s`，因此本轮未开始处理。[查看运行](%s/%s/actions/runs/%s)。%s\n\n
' \ + "${reason}" "${GITHUB_SERVER_URL}" "${REPO}" "${GITHUB_RUN_ID}" "${next_en}" \ + "${reason}" "${GITHUB_SERVER_URL}" "${REPO}" "${GITHUB_RUN_ID}" "${next_zh}")" + status_ids='' + status_lookup_ok=false + err="$(mktemp)" + # Same filter as the sibling upsert in 'Post autofix status comment', + # including its two guards: `// ""` so a single comment with a null + # body cannot abort the whole program (jq exits 5, all three + # attempts fail, and the run reds out WITHOUT posting the very + # status it exists to post), and --arg so a repo-configured + # AUTOFIX_BOT_LOGIN containing " or \ is a mismatch instead of a jq + # parse error. Stays an inline id stream into `tail -1` — it never + # lands in a WORKDIR json file, so the WORKDIR page normalizer + # (add-with-empty-default) must NOT be applied here: it would wrap + # the id stream in an array and break the tail-1 consumer. + # pipefail is set LOCALLY here rather than relied on: this `if` + # must test gh's status, not jq's. A gh failure carrying an HTTP + # status prints the error body to stdout, so jq errors out and the + # retry fires — but a CONNECTION-level failure (TCP reset, TLS + # abort, DNS blip) leaves stdout EMPTY, and `jq -rs` then prints + # nothing and exits 0. Without pipefail that reads as success on + # nothing read: status_lookup_ok=true, the empty id takes the + # writer down the "no status comment yet" branch, and it posts a + # DUPLICATE ⛔ blocked comment beside the stale ✅ one — the exact + # two-status state this function exists to prevent — on a green + # run. `defaults.run.shell: bash` already gives every step in this + # file `-eo pipefail`, so this is redundant today; it is also the + # only guard that survives that default changing or this helper + # being lifted into a step that sets its own options. + for attempt in 1 2 3; do + if status_ids="$(set -o pipefail; gh api "repos/${REPO}/issues/${FORCED_PR}/comments" --paginate 2> "${err}" | + jq -rs --arg ab "${AUTOFIX_BOT}" --arg m '' \ + '.[][] | select((.user.login // "") == $ab) | select((.body // "") | contains($m)) | .id')"; then + status_lookup_ok=true + break + fi + # Surface gh's own diagnosis instead of discarding it, exactly as + # read_live_permission does: a rate limit, an expired PAT and a + # 5xx are indistinguishable from 'attempt 3/3' alone. + echo "::warning::Takeover status lookup failed for #${FORCED_PR} (attempt ${attempt}/3): $(tr '\n' ' ' < "${err}")" >&2 + [[ "${attempt}" -lt 3 ]] && sleep "${attempt}" + done + rm -f "${err}" + if [[ "${status_lookup_ok}" != 'true' ]]; then + echo "::warning::Failed to read takeover status comments for #${FORCED_PR}" >&2 + return 1 + fi + status_id="$(tail -1 <<< "${status_ids}")" + if [[ -n "${status_id}" ]]; then + for attempt in 1 2 3; do + if gh api --method PATCH "repos/${REPO}/issues/comments/${status_id}" -f body="${body}" > /dev/null; then + return 0 + fi + echo "::warning::Failed to update blocked takeover status for #${FORCED_PR} (attempt ${attempt}/3)" >&2 + [[ "${attempt}" -lt 3 ]] && sleep "${attempt}" + done + else + for attempt in 1 2 3; do + if gh pr comment "${FORCED_PR}" --repo "${REPO}" --body "${body}" > /dev/null; then + return 0 + fi + echo "::warning::Failed to post blocked takeover status for #${FORCED_PR} (attempt ${attempt}/3)" >&2 + [[ "${attempt}" -lt 3 ]] && sleep "${attempt}" + done + fi + return 1 + } + # Candidate PRs: open, same-repo, targeting main, and either # authored by the dev-bot or opted in via TAKEOVER_LABEL. A PR # carrying SKIP_LABEL is excluded everywhere — skip wins over @@ -1916,8 +2106,12 @@ jobs: # jq's // treats false as empty, so that form is false for EVERY # input and silently green-no-op'd all forced dispatches. if [[ -n "${FORCED_PR}" ]]; then - META="$(gh pr view "${FORCED_PR}" --repo "${REPO}" \ - --json number,state,author,headRefName,isCrossRepository,baseRefName,labels,maintainerCanModify 2> /dev/null || echo '{}')" + if ! META="$(read_forced_pr_meta)"; then + echo "::error::Forced PR #${FORCED_PR} admission blocked: metadata_fetch_failed" + echo "targets=[]" >> "${GITHUB_OUTPUT}" + echo "has_targets=false" >> "${GITHUB_OUTPUT}" + exit 1 + fi # Same admission as the scheduled scan below. In-repo PRs fail # CLOSED on a missing isCrossRepository field (`.isCrossRepository # == false`, never a `// true | not` default — jq's // treats false @@ -1927,34 +2121,40 @@ jobs: # gate runs in the shell case just below, mirroring the scan's # per-candidate permission call) so the real-time route's fork # pickup is not silently discarded here. - OK="$(jq -r --arg ab "${AUTOFIX_BOT}" --arg take "${TAKEOVER_LABEL}" --arg skip "${SKIP_LABEL}" \ - '(((.state // "") == "OPEN") - and (((.author.login // "") == $ab) or ([.labels[]?.name] | index($take) != null)) - and ([.labels[]?.name] | index($skip) | not) - and ((.baseRefName // "") == "main") - and (if (.isCrossRepository == true) - then (.maintainerCanModify == true) - else (.isCrossRepository == false) - end))' <<< "${META}")" + ADMISSION_REASON="$(forced_admission_reason <<< "${META}")" # Fork only: the author must hold write+ RIGHT NOW (the same # live-privilege rule the scan applies per candidate and # review-address re-checks before pushing). In-repo PRs are gated # by author/label alone. - if [[ "${OK}" == 'true' && "$(jq -r '.isCrossRepository == true' <<< "${META}")" == 'true' ]]; then + if [[ "${ADMISSION_REASON}" == 'eligible' && "$(jq -r '.isCrossRepository == true' <<< "${META}")" == 'true' ]]; then FORK_AUTHOR="$(jq -r '.author.login // ""' <<< "${META}")" - FPERM="$(gh api "repos/${REPO}/collaborators/${FORK_AUTHOR}/permission" --jq '.permission // ""' 2> /dev/null || echo '')" + if ! FPERM="$(read_live_permission "${FORK_AUTHOR}")"; then + ADMISSION_REASON='permission_lookup_failed' + report_forced_takeover_blocked "${ADMISSION_REASON}" \ + || echo "::error::Forced PR #${FORCED_PR} blocked status update failed" + echo "::error::Forced PR #${FORCED_PR} admission blocked: ${ADMISSION_REASON}" + echo "targets=[]" >> "${GITHUB_OUTPUT}" + echo "has_targets=false" >> "${GITHUB_OUTPUT}" + exit 1 + fi case "${FPERM}" in admin|maintain|write) echo "🌿 forced fork PR #${FORCED_PR} admitted (author ${FORK_AUTHOR}=${FPERM})" ;; *) - echo "🧭 forced fork PR #${FORCED_PR} rejected: author ${FORK_AUTHOR} permission='${FPERM:-none}' below write" - OK='false' + ADMISSION_REASON="author_permission_${FPERM:-none}" + echo "🧭 forced fork PR #${FORCED_PR} rejected: ${ADMISSION_REASON}" ;; esac fi - if [[ "${OK}" != "true" ]]; then - echo "❌ #${FORCED_PR} is not an open main-targeting PR owned by ${AUTOFIX_BOT} or labeled ${TAKEOVER_LABEL} (or it carries ${SKIP_LABEL}); a fork PR additionally needs maintainer edits allowed and a live write+ author" + if [[ "${ADMISSION_REASON}" != 'eligible' ]]; then + if ! report_forced_takeover_blocked "${ADMISSION_REASON}"; then + echo "::error::Forced PR #${FORCED_PR} blocked status update failed" + echo "targets=[]" >> "${GITHUB_OUTPUT}" + echo "has_targets=false" >> "${GITHUB_OUTPUT}" + exit 1 + fi + echo "❌ Forced PR #${FORCED_PR} rejected: ${ADMISSION_REASON}" echo "targets=[]" >> "${GITHUB_OUTPUT}" echo "has_targets=false" >> "${GITHUB_OUTPUT}" exit 0 @@ -2000,7 +2200,11 @@ jobs: # candidates alone exhaust the inspection budget. while IFS=$'\t' read -r FPR FAUTHOR; do [[ -z "${FPR}" ]] && continue - FPERM="$(gh api "repos/${REPO}/collaborators/${FAUTHOR}/permission" --jq '.permission // ""' 2> /dev/null || echo '')" + if ! FPERM="$(read_live_permission "${FAUTHOR}")"; then + echo "::warning::Fork takeover candidate #${FPR} blocked: permission_lookup_failed" + fleet_row "${FPR}" 'blocked' 'permission_lookup_failed' + continue + fi case "${FPERM}" in admin|maintain|write) echo "🌿 fork takeover candidate #${FPR} admitted (author ${FAUTHOR}=${FPERM})" @@ -2008,6 +2212,7 @@ jobs: ;; *) echo "🧭 fork takeover candidate #${FPR} skipped: author ${FAUTHOR} permission='${FPERM:-none}' below write" + fleet_row "${FPR}" 'blocked' "author_permission_${FPERM:-none}" ;; esac done < <(jq -rs --arg skip "${SKIP_LABEL}" ' diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index e6c215fed2c..8a12d144a9a 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -323,7 +323,8 @@ describe('qwen-autofix workflow', () => { expect(authRounds).toBeLessThan(strictRounds); expect(workflow).toContain("MAX_OPEN_AUTOFIX_PRS: '5'"); expect(reviewScanJob).toContain('isCrossRepository'); - expect(reviewScanJob).toContain('not an open main-targeting PR'); + expect(reviewScanJob).toContain('Forced PR #${FORCED_PR} rejected:'); + expect(reviewScanJob).toContain('forced_admission_reason'); // Candidates fail CLOSED on the fork field, matching the forced path // and the NOTE that documents the jq // false trap. expect(reviewScanJob).toContain('select(.isCrossRepository == false)'); @@ -2576,9 +2577,14 @@ describe('qwen-autofix workflow', () => { // forces a deliberate test update, however it is spaced or line-wrapped: // bump this count AND pipe the new site through the normalizer (bumping // the count below too) — bumping this pin alone leaves toBe(9) green. - expect(workflow.split('--paginate').length - 1).toBe(13); + expect(workflow.split('--paginate').length - 1).toBe(14); // scan ic + pr-events + ic re-fetch + scan rv/rc + prepare rv/rc/ic + - // report COMMENTS_JSON fallback = nine normalized fetch sites. + // report COMMENTS_JSON fallback = nine normalized fetch sites. The + // blocked-takeover status lookup is deliberately NOT among them: like the + // sibling STATUS_ID read, it consumes the page stream inline via + // `--jq ... | .id` into `tail -1` and never lands in a WORKDIR json file, + // so piping it through `jq -s 'add // []'` would wrap the id stream in an + // array and break the tail-1 consumer. expect(workflow.split("jq -s 'add // []'").length - 1).toBe(9); // Empty-input semantics: a total gh failure feeds the fallback an EMPTY // stream, where the normalizer filter must yield '[]' and not 'null' — @@ -3381,33 +3387,33 @@ describe('qwen-autofix workflow', () => { }); it('behaviorally validates forced targets against author, takeover, and skip', () => { - // Extract the forced-PR OK predicate VERBATIM and replay it: the bot's + // Extract the forced-PR classifier VERBATIM and replay it: the bot's // own PRs pass; a human PR passes only with the takeover label; skip // vetoes even a takeover-labeled PR; closed PRs never pass. A fork PR // passes the structural predicate only with maintainer edits allowed — the // live write+ author gate is a shell step below (asserted separately), // mirroring the scheduled scan's per-candidate fork admission. - const okProgram = reviewScanJob.match( - /OK="\$\(jq -r --arg ab "\$\{AUTOFIX_BOT\}" --arg take "\$\{TAKEOVER_LABEL\}" --arg skip "\$\{SKIP_LABEL\}" \\\n\s+'([\s\S]*?)'/, + const classifier = reviewScanJob.match( + /(forced_admission_reason\(\) \{[\s\S]*?\n {10}\})/, )?.[1]; - expect(okProgram).toBeTruthy(); - const ok = (meta) => + expect(classifier).toBeTruthy(); + const reason = (meta) => execFileSync( - 'jq', + 'bash', [ - '-r', - '--arg', - 'ab', - 'qwen-code-dev-bot', - '--arg', - 'take', - 'autofix/takeover', - '--arg', - 'skip', - 'autofix/skip', - okProgram, + '-c', + `${classifier.replace(/\n {10}/g, '\n')}\nforced_admission_reason`, ], - { encoding: 'utf8', input: JSON.stringify(meta) }, + { + encoding: 'utf8', + input: JSON.stringify(meta), + env: { + ...process.env, + AUTOFIX_BOT: 'qwen-code-dev-bot', + TAKEOVER_LABEL: 'autofix/takeover', + SKIP_LABEL: 'autofix/skip', + }, + }, ).trim(); const meta = (author, labels = [], extra = {}) => ({ state: 'OPEN', @@ -3417,46 +3423,51 @@ describe('qwen-autofix workflow', () => { labels: labels.map((name) => ({ name })), ...extra, }); - expect(ok(meta('qwen-code-dev-bot'))).toBe('true'); - expect(ok(meta('human', ['autofix/takeover']))).toBe('true'); - expect(ok(meta('human'))).toBe('false'); - expect(ok(meta('human', ['autofix/takeover', 'autofix/skip']))).toBe( - 'false', + expect(reason(meta('qwen-code-dev-bot'))).toBe('eligible'); + expect(reason(meta('human', ['autofix/takeover']))).toBe('eligible'); + expect(reason(meta('human'))).toBe('unmanaged_author'); + expect(reason(meta('human', ['autofix/takeover', 'autofix/skip']))).toBe( + 'skip_label', ); - expect(ok(meta('qwen-code-dev-bot', ['autofix/skip']))).toBe('false'); - expect(ok(meta('human', ['autofix/takeover'], { state: 'CLOSED' }))).toBe( - 'false', + expect(reason(meta('qwen-code-dev-bot', ['autofix/skip']))).toBe( + 'skip_label', ); + expect( + reason(meta('human', ['autofix/takeover'], { state: 'CLOSED' })), + ).toBe('not_open'); + expect( + reason(meta('human', ['autofix/takeover'], { baseRefName: 'next' })), + ).toBe('wrong_base'); // Fork PRs: admitted structurally only when maintainer edits are allowed // (the bot's own fork or a takeover-labelled fork). The live write+ author // check is the shell gate asserted below; without allow-edits a fork still // fails closed here. expect( - ok( + reason( meta('human', ['autofix/takeover'], { isCrossRepository: true, maintainerCanModify: true, }), ), - ).toBe('true'); + ).toBe('eligible'); expect( - ok( + reason( meta('qwen-code-dev-bot', [], { isCrossRepository: true, maintainerCanModify: true, }), ), - ).toBe('true'); + ).toBe('eligible'); expect( - ok(meta('human', ['autofix/takeover'], { isCrossRepository: true })), - ).toBe('false'); + reason(meta('human', ['autofix/takeover'], { isCrossRepository: true })), + ).toBe('maintainer_edits_disabled'); // A missing isCrossRepository fails CLOSED. This case is why the // predicate reads `.isCrossRepository == false`: jq's // treats false as // empty, so the previous `(.isCrossRepository // true) | not` was false // for EVERY input and silently green-no-op'd all forced dispatches. const missing = meta('qwen-code-dev-bot'); delete missing.isCrossRepository; - expect(ok(missing)).toBe('false'); + expect(reason(missing)).toBe('cross_repo_state_missing'); expect(reviewScanJob).toContain('.isCrossRepository == false'); expect(reviewScanJob).not.toContain('(.isCrossRepository // true) | not'); // The forced path queries maintainerCanModify and re-checks a fork author's @@ -3467,7 +3478,756 @@ describe('qwen-autofix workflow', () => { ); expect(reviewScanJob).toContain('forced fork PR #${FORCED_PR} admitted'); expect(reviewScanJob).toContain( - 'gh api "repos/${REPO}/collaborators/${FORK_AUTHOR}/permission"', + 'gh api "repos/${REPO}/collaborators/${login}/permission"', + ); + }); + + it('recovers transient forced-target reads and reports terminal takeover blocks', () => { + const readMeta = reviewScanJob.match( + /(read_forced_pr_meta\(\) \{[\s\S]*?\n {10}\})/, + )?.[1]; + const readPermission = reviewScanJob.match( + /(read_live_permission\(\) \{[\s\S]*?\n {10}\})/, + )?.[1]; + const reportBlocked = reviewScanJob.match( + /(report_forced_takeover_blocked\(\) \{[\s\S]*?\n {10}\})/, + )?.[1]; + expect(readMeta).toBeTruthy(); + expect(readPermission).toBeTruthy(); + expect(reportBlocked).toBeTruthy(); + + const runReader = (reader, command, successOutput) => { + const dir = mkdtempSync(join(tmpdir(), 'autofix-admission-')); + try { + writeFileSync( + join(dir, 'gh'), + `#!/bin/bash\ncount_file='${dir}/count'\ncount=0\n[[ -f "$count_file" ]] && count="$(cat "$count_file")"\ncount=$((count + 1))\nprintf '%s' "$count" > "$count_file"\nif [[ "$count" -eq 1 ]]; then exit 1; fi\nprintf '%s' '${successOutput}'\n`, + ); + chmodSync(join(dir, 'gh'), 0o755); + const result = spawnSync( + 'bash', + [ + '-c', + // Production shell options (defaults.run.shell: bash → `bash + // --noprofile --norc -eo pipefail`), with the call made through + // `|| exit $?` so errexit is suspended inside the helper exactly + // as the `if !` call sites suspend it. + [ + 'set -eo pipefail', + 'sleep() { :; }', + reader.replace(/\n {10}/g, '\n'), + `${command} || exit $?`, + ].join('\n'), + ], + { + env: { + ...process.env, + PATH: `${dir}:${process.env.PATH}`, + REPO: 'QwenLM/qwen-code', + FORCED_PR: '8320', + }, + encoding: 'utf8', + }, + ); + return result; + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }; + + const meta = JSON.stringify({ + number: 8320, + state: 'OPEN', + author: { login: 'qqqys' }, + headRefName: 'topic', + baseRefName: 'main', + isCrossRepository: true, + labels: [{ name: 'autofix/takeover' }], + maintainerCanModify: true, + }); + const metaResult = runReader(readMeta, 'read_forced_pr_meta', meta); + expect(metaResult.status).toBe(0); + expect(metaResult.stdout).toBe(meta); + const permissionResult = runReader( + readPermission, + 'read_live_permission qqqys', + 'write', + ); + expect(permissionResult.status).toBe(0); + expect(permissionResult.stdout).toBe('write'); + + const failingDir = mkdtempSync(join(tmpdir(), 'autofix-admission-fail-')); + try { + writeFileSync(join(failingDir, 'gh'), '#!/bin/bash\nexit 1\n'); + chmodSync(join(failingDir, 'gh'), 0o755); + const failedPermission = spawnSync( + 'bash', + [ + '-c', + [ + 'set -eo pipefail', + 'sleep() { :; }', + readPermission.replace(/\n {10}/g, '\n'), + 'read_live_permission qqqys || exit $?', + ].join('\n'), + ], + { + env: { + ...process.env, + PATH: `${failingDir}:${process.env.PATH}`, + REPO: 'QwenLM/qwen-code', + }, + encoding: 'utf8', + }, + ); + expect(failedPermission.status).toBe(1); + expect(failedPermission.stderr).toContain('(attempt 3/3)'); + + const failedMeta = spawnSync( + 'bash', + [ + '-c', + [ + 'set -eo pipefail', + 'sleep() { :; }', + readMeta.replace(/\n {10}/g, '\n'), + 'read_forced_pr_meta || exit $?', + ].join('\n'), + ], + { + env: { + ...process.env, + PATH: `${failingDir}:${process.env.PATH}`, + REPO: 'QwenLM/qwen-code', + FORCED_PR: '8320', + }, + encoding: 'utf8', + }, + ); + expect(failedMeta.status).toBe(1); + expect(failedMeta.stderr).toContain('(attempt 3/3)'); + } finally { + rmSync(failingDir, { recursive: true, force: true }); + } + + const reporterDir = mkdtempSync(join(tmpdir(), 'autofix-reporter-')); + try { + const callsFile = join(reporterDir, 'calls'); + writeFileSync( + join(reporterDir, 'gh'), + `#!/bin/bash +printf '%q ' "$@" >> '${callsFile}' +printf '\n' >> '${callsFile}' +if [[ "$1 $2" == 'api user' ]]; then + if [[ "\${FAIL_ACTOR_ONCE:-false}" == 'true' && ! -f '${reporterDir}/actor-failed' ]]; then + printf '1' > '${reporterDir}/actor-failed' + exit 1 + fi + printf '%s' "\${STUB_ACTOR:-qwen-code-dev-bot}" + exit 0 +fi +if [[ "$1 $2" == 'api repos/QwenLM/qwen-code/issues/8320/comments' ]]; then + # CONNECTION-level failure: nothing on stdout at all. This is the shape that + # needs pipefail — a downstream \`jq -rs\` reads empty input, prints nothing + # and exits 0, so without it the caller cannot tell this from success. + if [[ "\${FAIL_STATUS_LOOKUP:-false}" == 'true' ]]; then + printf 'gh: Server Error (HTTP 502)\\n' >&2 + exit 1 + fi + # HTTP-level failure: gh puts the error BODY on stdout, so jq chokes on it + # and fails on its own. This path never depended on pipefail; pinned so the + # two halves stay distinguishable. + if [[ "\${FAIL_STATUS_LOOKUP_HTTP:-false}" == 'true' ]]; then + printf '%s' '{"message":"Server Error"}' + printf 'gh: Server Error (HTTP 502)\\n' >&2 + exit 1 + fi + [[ "\${NO_STATUS_MARKER:-false}" == 'true' ]] && { printf '%s' '[]'; exit 0; } + # A realistic page: gh emits the comment objects, not bare ids, and a + # deleted-body comment really does arrive as "body": null. + printf '%s' "\${STATUS_PAGE}" + exit 0 +fi +if [[ "$1 $2 $3" == 'api --method PATCH' ]]; then + if [[ "\${FAIL_PATCH_ONCE:-false}" == 'true' && ! -f '${reporterDir}/patch-failed' ]]; then + printf '1' > '${reporterDir}/patch-failed' + exit 1 + fi + exit 0 +fi +if [[ "$1 $2" == 'pr comment' ]]; then + if [[ "\${FAIL_COMMENT_ONCE:-false}" == 'true' && ! -f '${reporterDir}/comment-failed' ]]; then + printf '1' > '${reporterDir}/comment-failed' + exit 1 + fi + exit 0 +fi +exit 1 +`, + ); + chmodSync(join(reporterDir, 'gh'), 0o755); + // One page carrying a null-bodied comment alongside the real status + // comment: the shape the production filter must survive. + const statusPage = JSON.stringify([ + { id: 1, user: { login: 'qwen-code-dev-bot' }, body: null }, + { id: 2, user: { login: 'wenshao' }, body: 'looks good' }, + { + id: 123, + user: { login: 'qwen-code-dev-bot' }, + body: '\n\n🔄 working', + }, + ]); + const runReporter = ( + extraEnv = {}, + reason = 'permission_lookup_failed', + shellOpts = 'set -eo pipefail', + ) => + spawnSync( + 'bash', + [ + '-c', + // Production runs this block under `bash --noprofile --norc -eo + // pipefail` (defaults.run.shell: bash, pinned below), and every + // call site is an `if !` / `||` context, which suspends errexit + // inside the call. Reproduce BOTH halves: the harness sets -eo + // pipefail and calls the function through `|| exit $?`, exactly as + // the gate does. `shellOpts` is overridable so one case can drop + // the ambient pipefail and prove the helper carries its own. + [ + shellOpts, + 'sleep() { :; }', + reportBlocked.replace(/\n {10}/g, '\n'), + `report_forced_takeover_blocked ${reason} || exit $?`, + ].join('\n'), + ], + { + env: { + ...process.env, + PATH: `${reporterDir}:${process.env.PATH}`, + REPO: 'QwenLM/qwen-code', + FORCED_PR: '8320', + DRY_RUN: 'false', + AUTOFIX_BOT: 'qwen-code-dev-bot', + TAKEOVER_LABEL: 'autofix/takeover', + GITHUB_RUN_ID: '30778039590', + GITHUB_SERVER_URL: 'https://ghes.example.com', + STATUS_PAGE: statusPage, + META: meta, + ...extraEnv, + }, + encoding: 'utf8', + }, + ); + const reporter = runReporter(); + expect({ status: reporter.status, stderr: reporter.stderr }).toEqual({ + status: 0, + stderr: '', + }); + const calls = readFileSync(callsFile, 'utf8'); + // Picking 123 out of a page whose FIRST bot comment has `body: null` is + // the whole point: without the `// ""` guard jq aborts the program + // (rc=5), gh exits non-zero, all three attempts fail, and the run reds + // out without ever posting the blocked status it exists to post. + expect(calls).toContain('repos/QwenLM/qwen-code/issues/comments/123'); + expect(calls).toContain('autofix-status'); + expect(calls).toContain('AutoFix blocked'); + expect(calls).toContain('permission_lookup_failed'); + expect(calls).toContain('A later scheduled scan will retry'); + // The run link resolves from GITHUB_SERVER_URL like every other status + // writer; a hardcoded github.com is the one broken link on GHES. + expect(calls).toContain( + 'https://ghes.example.com/QwenLM/qwen-code/actions/runs/30778039590', + ); + expect(calls).not.toContain('https://github.com/QwenLM'); + + writeFileSync(callsFile, ''); + const transientActorReporter = runReporter({ FAIL_ACTOR_ONCE: 'true' }); + expect(transientActorReporter.status).toBe(0); + expect(readFileSync(callsFile, 'utf8').match(/api user/g)).toHaveLength( + 2, + ); + + writeFileSync(callsFile, ''); + const transientPatchReporter = runReporter({ FAIL_PATCH_ONCE: 'true' }); + expect(transientPatchReporter.status).toBe(0); + expect( + readFileSync(callsFile, 'utf8').match(/api --method PATCH/g), + ).toHaveLength(2); + + writeFileSync(callsFile, ''); + const transientCommentReporter = runReporter({ + NO_STATUS_MARKER: 'true', + FAIL_COMMENT_ONCE: 'true', + }); + expect(transientCommentReporter.status).toBe(0); + expect(readFileSync(callsFile, 'utf8').match(/pr comment/g)).toHaveLength( + 2, + ); + + writeFileSync(callsFile, ''); + const maintainerEditsReporter = runReporter( + {}, + 'maintainer_edits_disabled', + ); + expect(maintainerEditsReporter.status).toBe(0); + const maintainerEditsCalls = readFileSync(callsFile, 'utf8'); + expect(maintainerEditsCalls).toContain( + 'Re-enable maintainer edits on the fork PR to resume takeover', + ); + expect(maintainerEditsCalls).not.toContain( + 'A later scheduled scan will retry', + ); + + writeFileSync(callsFile, ''); + const authorPermissionReporter = runReporter( + {}, + 'author_permission_read', + ); + expect(authorPermissionReporter.status).toBe(0); + const authorPermissionCalls = readFileSync(callsFile, 'utf8'); + expect(authorPermissionCalls).toContain( + 'Grant the fork author write access', + ); + expect(authorPermissionCalls).toContain( + 'remove the autofix/takeover label', + ); + expect(authorPermissionCalls).not.toContain( + 'A later scheduled scan will retry', + ); + + writeFileSync(callsFile, ''); + const botManagedMeta = JSON.stringify({ + ...JSON.parse(meta), + author: { login: 'qwen-code-dev-bot' }, + labels: [], + }); + const botManagedReporter = runReporter( + { META: botManagedMeta }, + 'maintainer_edits_disabled', + ); + expect(botManagedReporter.status).toBe(0); + expect(readFileSync(callsFile, 'utf8')).toContain('AutoFix blocked'); + + writeFileSync(callsFile, ''); + const failedReporter = runReporter({ FAIL_STATUS_LOOKUP: 'true' }); + expect(failedReporter.status).toBe(1); + // Warnings go to stderr like the two reader helpers, so the reporter is + // safe to wrap in $( ) and its warnings never pollute a captured value. + expect(failedReporter.stdout).toBe(''); + expect(failedReporter.stderr).toContain('(attempt 3/3)'); + expect(failedReporter.stderr).toContain( + 'Failed to read takeover status comments', + ); + // gh's own diagnosis rides along instead of going to /dev/null — the + // rule this same block states for read_live_permission. + expect(failedReporter.stderr).toContain('HTTP 502'); + // Nothing was read, so nothing may be written: a "post a new one" here + // would be the duplicate ⛔ comment beside the stale ✅ one. + expect(readFileSync(callsFile, 'utf8')).not.toContain('AutoFix blocked'); + + // Same connection-level failure with the ambient pipefail REMOVED. The + // status read is the one `if` in this helper that tests a PIPELINE, and + // `jq -rs` turns gh's empty stdout into a silent exit 0 — so without a + // local `set -o pipefail` the loop breaks on attempt 1, status_lookup_ok + // goes true on nothing read, and the empty id routes the writer to the + // "no status comment yet" branch: a DUPLICATE blocked comment, run green + // at exit 0. defaults.run.shell (pinned below) makes production pipefail + // today; this case is what keeps the helper correct without it. + writeFileSync(callsFile, ''); + const failedNoPipefail = runReporter( + { FAIL_STATUS_LOOKUP: 'true' }, + 'permission_lookup_failed', + 'set -e', + ); + expect(failedNoPipefail.status).toBe(1); + expect(failedNoPipefail.stderr).toContain('(attempt 3/3)'); + expect(failedNoPipefail.stderr).toContain( + 'Failed to read takeover status comments', + ); + const noPipefailCalls = readFileSync(callsFile, 'utf8'); + expect(noPipefailCalls.match(/issues\/8320\/comments/g)).toHaveLength(3); + expect(noPipefailCalls).not.toContain('AutoFix blocked'); + + // The HTTP-status half of the same failure, also without ambient + // pipefail: gh writes the error body to stdout, jq chokes on it and + // fails by itself. This path was already correct — pinned so a future + // "simplification" cannot conclude the pipefail above is what carries it + // and drop it. + writeFileSync(callsFile, ''); + const failedHttpNoPipefail = runReporter( + { FAIL_STATUS_LOOKUP_HTTP: 'true' }, + 'permission_lookup_failed', + 'set -e', + ); + expect(failedHttpNoPipefail.status).toBe(1); + expect(failedHttpNoPipefail.stderr).toContain('(attempt 3/3)'); + expect( + readFileSync(callsFile, 'utf8').match(/issues\/8320\/comments/g), + ).toHaveLength(3); + + // 'The PAT is not the bot' is the riskiest new branch and had no + // coverage in either direction. It is still a hard stop (return 1), so + // the mismatch cannot be mistaken for a delivered status comment. + writeFileSync(callsFile, ''); + const wrongActorReporter = runReporter({ STUB_ACTOR: 'some-human' }); + expect(wrongActorReporter.status).toBe(1); + expect(wrongActorReporter.stderr).toContain( + "PAT authenticates as 'some-human'", + ); + expect(readFileSync(callsFile, 'utf8')).not.toContain('AutoFix blocked'); + } finally { + rmSync(reporterDir, { recursive: true, force: true }); + } + + expect( + reviewScanJob.match( + /report_forced_takeover_blocked "\$\{ADMISSION_REASON\}"/g, + ), + ).toHaveLength(2); + expect(reviewScanJob).toContain('metadata_fetch_failed'); + expect(reviewScanJob).toContain('permission_lookup_failed'); + expect(reviewScanJob).toContain( + 'fleet_row "${FPR}" \'blocked\' "author_permission_${FPERM:-none}"', + ); + expect(reviewScanJob).toContain('report_forced_takeover_blocked'); + expect(reviewScanJob).toContain(''); + expect(reviewScanJob).toContain('AutoFix blocked'); + expect(reviewScanJob).toContain('exit 1'); + + // The status read is the only `if` in this helper testing a PIPELINE, so + // it carries pipefail itself instead of inheriting it. Pinned textually + // because the behavioural case above can only observe its ABSENCE by + // dropping the ambient option — a reader of the YAML alone would not see + // why one command substitution differs from its neighbours. + expect(reviewScanJob).toContain( + 'if status_ids="$(set -o pipefail; gh api "repos/${REPO}/issues/${FORCED_PR}/comments" --paginate', + ); + // And the ambient half: `shell: bash` is what expands to `bash --noprofile + // --norc -eo pipefail`, which every other gh|jq pipeline in this file (the + // scan's `| jq -s 'add // []'` writers) relies on WITHOUT saying so. Drop + // this default and those go silently green on empty input; the harnesses + // above would keep passing, since they set the option themselves. + expect(workflow).toMatch(/\ndefaults:\n {2}run:\n {4}shell: 'bash'\n/); + + const runBlock = reviewScanJob.match(/run: \|-\n([\s\S]*)$/)?.[1]; + expect(runBlock).toBeTruthy(); + const syntax = spawnSync('bash', ['-n'], { + encoding: 'utf8', + input: runBlock.replace(/^ {10}/gm, ''), + }); + expect({ status: syntax.status, stderr: syntax.stderr }).toEqual({ + status: 0, + stderr: '', + }); + }); + + it('answers terminal permission states without retrying them', () => { + const readPermission = reviewScanJob.match( + /(read_live_permission\(\) \{[\s\S]*?\n {10}\})/, + )?.[1]; + expect(readPermission).toBeTruthy(); + // 'none' MUST be in the accepted set. Without it the `case *)` arms that + // render author_permission_${FPERM:-none} are unreachable dead code and + // every bot/org author falls through to permission_lookup_failed instead. + expect(readPermission).toContain( + '^(admin|maintain|write|triage|read|none)$', + ); + + const runPermission = (ghBody, login = 'qqqys') => { + const dir = mkdtempSync(join(tmpdir(), 'autofix-perm-')); + try { + const callsFile = join(dir, 'calls'); + writeFileSync( + join(dir, 'gh'), + `#!/bin/bash\nprintf 'call\\n' >> '${callsFile}'\n${ghBody}\n`, + ); + chmodSync(join(dir, 'gh'), 0o755); + const result = spawnSync( + 'bash', + [ + '-c', + [ + 'set -eo pipefail', + 'sleep() { :; }', + readPermission.replace(/\n {10}/g, '\n'), + `read_live_permission '${login}' || exit $?`, + ].join('\n'), + ], + { + env: { + ...process.env, + PATH: `${dir}:${process.env.PATH}`, + REPO: 'QwenLM/qwen-code', + }, + encoding: 'utf8', + }, + ); + const calls = existsSync(callsFile) + ? readFileSync(callsFile, 'utf8').split('\n').filter(Boolean).length + : 0; + return { + status: result.status, + stdout: result.stdout, + stderr: result.stderr, + calls, + }; + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }; + + // Bot-type logins (dependabot[bot], github-actions[bot], renovate[bot]) + // and org logins answer HTTP 200 with permission 'none' — a definitive + // "holds nothing here", settled in ONE call. + expect(runPermission("printf '%s' 'none'\nexit 0")).toMatchObject({ + status: 0, + stdout: 'none', + calls: 1, + }); + // A login that does not exist answers HTTP 404 — equally definitive. + expect( + runPermission("printf 'gh: Not Found (HTTP 404)\\n' >&2\nexit 1"), + ).toMatchObject({ status: 0, stdout: 'none', calls: 1 }); + // An empty login can only ever 404, so it is answered without a call. + expect(runPermission("printf '%s' 'write'\nexit 0", '')).toMatchObject({ + status: 0, + stdout: 'none', + calls: 0, + }); + // Every real grant level passes through verbatim, one call each. + for (const permission of ['admin', 'maintain', 'write', 'triage', 'read']) { + expect( + runPermission(`printf '%s' '${permission}'\nexit 0`), + ).toMatchObject({ status: 0, stdout: permission, calls: 1 }); + } + // A genuinely transient answer still burns the full retry budget and + // reports failure: collapsing 5xx into 'none' would silently reject an + // author who actually holds write. + const transient = runPermission( + "printf 'gh: Server Error (HTTP 502)\\n' >&2\nexit 1", + ); + expect(transient.status).toBe(1); + expect(transient.calls).toBe(3); + expect(transient.stderr).toContain('(attempt 3/3)'); + // gh's own diagnosis rides along instead of being discarded to /dev/null — + // a rate limit, an expired PAT and a 5xx are indistinguishable without it. + expect(transient.stderr).toContain('HTTP 502'); + }); + + it('wires forced admission end to end: reader, classifier, permission gate, reporter', () => { + // The four pieces are unit-pinned above; this runs the ACTUAL gate that + // joins them, so a rewiring (wrong reason string, a gate that exits 1 on a + // routine rejection, a reporter that never sees the reason) fails here. + const readMeta = reviewScanJob.match( + /(read_forced_pr_meta\(\) \{[\s\S]*?\n {10}\})/, + )?.[1]; + const readPermission = reviewScanJob.match( + /(read_live_permission\(\) \{[\s\S]*?\n {10}\})/, + )?.[1]; + const classifier = reviewScanJob.match( + /(forced_admission_reason\(\) \{[\s\S]*?\n {10}\})/, + )?.[1]; + const reportBlocked = reviewScanJob.match( + /(report_forced_takeover_blocked\(\) \{[\s\S]*?\n {10}\})/, + )?.[1]; + const gate = reviewScanJob.match( + /(if ! META="\$\(read_forced_pr_meta\)"; then[\s\S]*?\n {14}exit 0\n {12}fi)/, + )?.[1]; + expect(readMeta).toBeTruthy(); + expect(readPermission).toBeTruthy(); + expect(classifier).toBeTruthy(); + expect(reportBlocked).toBeTruthy(); + expect(gate).toBeTruthy(); + + const forkMeta = JSON.stringify({ + number: 8320, + state: 'OPEN', + author: { login: 'renovate[bot]' }, + headRefName: 'topic', + baseRefName: 'main', + isCrossRepository: true, + labels: [{ name: 'autofix/takeover' }], + maintainerCanModify: true, + }); + + const inRepoMeta = JSON.stringify({ + ...JSON.parse(forkMeta), + author: { login: 'qqqys' }, + isCrossRepository: false, + }); + const statusPage = JSON.stringify([ + { + id: 123, + user: { login: 'qwen-code-dev-bot' }, + body: '\n\n🔄 working', + }, + ]); + + // permission 'transient' makes every collaborator lookup answer HTTP 502, + // the only input that reaches permission_lookup_failed. + const runGate = (permission, metaJson = forkMeta) => { + const dir = mkdtempSync(join(tmpdir(), 'autofix-wiring-')); + try { + const callsFile = join(dir, 'calls'); + const outputFile = join(dir, 'github-output'); + writeFileSync(outputFile, ''); + writeFileSync( + join(dir, 'gh'), + `#!/bin/bash +printf '%q ' "$@" >> '${callsFile}' +printf '\\n' >> '${callsFile}' +case "$1 $2" in + 'pr view') printf '%s' '${metaJson}'; exit 0 ;; + 'api user') printf '%s' 'qwen-code-dev-bot'; exit 0 ;; +esac +case "$2" in + *collaborators/*/permission) + if [[ '${permission}' == 'transient' ]]; then + printf 'gh: Server Error (HTTP 502)\\n' >&2 + exit 1 + fi + printf '%s' '${permission}'; exit 0 ;; + */issues/8320/comments) printf '%s' '${statusPage}'; exit 0 ;; +esac +[[ "$1 $2 $3" == 'api --method PATCH' ]] && exit 0 +exit 1 +`, + ); + chmodSync(join(dir, 'gh'), 0o755); + const result = spawnSync( + 'bash', + [ + '-c', + [ + // Production shell options: `bash --noprofile --norc -eo + // pipefail` (defaults.run.shell: bash). The gate runs at top + // level there, so errexit is live for it here too. + 'set -eo pipefail', + 'sleep() { :; }', + readMeta.replace(/\n {10}/g, '\n'), + readPermission.replace(/\n {10}/g, '\n'), + classifier.replace(/\n {10}/g, '\n'), + reportBlocked.replace(/\n {10}/g, '\n'), + gate.replace(/\n {12}/g, '\n'), + 'echo "ADMITTED:${ADMISSION_REASON}"', + ].join('\n'), + ], + { + env: { + ...process.env, + PATH: `${dir}:${process.env.PATH}`, + REPO: 'QwenLM/qwen-code', + FORCED_PR: '8320', + DRY_RUN: 'false', + AUTOFIX_BOT: 'qwen-code-dev-bot', + TAKEOVER_LABEL: 'autofix/takeover', + SKIP_LABEL: 'autofix/skip', + GITHUB_RUN_ID: '30778039590', + GITHUB_SERVER_URL: 'https://ghes.example.com', + GITHUB_OUTPUT: outputFile, + }, + encoding: 'utf8', + }, + ); + return { + status: result.status, + stdout: result.stdout, + stderr: result.stderr, + calls: readFileSync(callsFile, 'utf8'), + output: readFileSync(outputFile, 'utf8'), + }; + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }; + + // A fork author holding nothing is a ROUTINE rejection: green run, the + // reason names the actual permission, and the blocked comment carries the + // remedy that can actually clear it. Before 'none' was a terminal answer + // this exited 1 and promised a scheduled retry that could never succeed. + const rejected = runGate('none'); + expect(rejected.status).toBe(0); + expect(rejected.stdout).toContain('rejected: author_permission_none'); + expect(rejected.stdout).not.toContain('permission_lookup_failed'); + expect(rejected.output).toContain('has_targets=false'); + expect(rejected.calls).toContain('Grant the fork author write access'); + // The gate never reaches the 'ADMITTED' echo — it exits inside the block. + expect(rejected.stdout).not.toContain('ADMITTED:'); + + // A write-holding fork author falls through the gate as eligible. + const admitted = runGate('write'); + expect(admitted.status).toBe(0); + expect(admitted.stdout).toContain('admitted (author renovate[bot]=write)'); + expect(admitted.stdout).toContain('ADMITTED:eligible'); + expect(admitted.calls).not.toContain('--method'); + + // In-repo PRs are gated by author/label ALONE — the live-permission call + // is fork-only. Without this case, deleting the `isCrossRepository == true` + // conjunct keeps the whole suite green while an in-repo takeover PR whose + // author was demoted gets rejected as author_permission_read. + const inRepo = runGate('read', inRepoMeta); + expect(inRepo.status).toBe(0); + expect(inRepo.stdout).toContain('ADMITTED:eligible'); + expect(inRepo.calls).not.toContain('collaborators'); + expect(inRepo.calls).not.toContain('AutoFix blocked'); + + // A genuinely transient permission answer is the ONE path that still reds + // the run, and it must post the blocked status before it does. Without + // this case, flipping that `exit 1` to `exit 0` ships green and restores + // the silent-success failure mode this PR exists to remove. + const lookupFailed = runGate('transient'); + expect(lookupFailed.status).toBe(1); + expect(lookupFailed.stdout).not.toContain('ADMITTED:'); + expect(lookupFailed.calls).toContain('permission_lookup_failed'); + expect(lookupFailed.calls).toContain('A later scheduled scan will retry'); + expect(lookupFailed.calls).toContain( + 'repos/QwenLM/qwen-code/issues/comments/123', + ); + }); + + it('serializes forced status writes with the matching address job', () => { + // The prefix is a LITERAL on both sides: job-level `concurrency` cannot + // read the `env` context, so the two jobs cannot share a constant. Compare + // the extracted prefixes instead of pinning two independent literals — + // renaming one side alone silently re-opens the lost-update race on the + // status comment, and nothing else in the suite would notice. + // Either quote style: the scan side is double-quoted because its + // expression embeds `'true'`, which Prettier will not leave escaped. + const groupOf = (text) => + text.match(/\n {4}concurrency:\n {6}group: ['"]([a-z-]+?)-\$\{\{/)?.[1]; + const scanLock = groupOf(reviewScanJob); + const addressLock = groupOf(reviewAddressJob); + expect(scanLock).toBeTruthy(); + expect(scanLock).toBe(addressLock); + + // Each side must still key the group on the PR number — a shared prefix + // with a per-run suffix would serialise nothing. + expect(reviewScanJob).toContain( + `group: "${scanLock}-\${{ needs.route.outputs.do_review == 'true' && needs.route.outputs.pr_number || github.run_id }}"`, + ); + expect(reviewAddressJob).toContain( + `group: '${addressLock}-\${{ matrix.target.pr }}'`, + ); + expect(reviewScanJob).toContain('cancel-in-progress: false'); + expect(reviewAddressJob).toContain('cancel-in-progress: false'); + }); + + // GitHub evaluates concurrency BEFORE the job `if`, so a predicate broader + // than the job's own condition lets a run that will only skip take the + // shared per-PR slot. `route` emits pr_number unconditionally, so without + // the do_review conjunct a `phase: issue` dispatch carrying pr_number: N + // queues this skipped job behind PR N's in-flight address round — and + // `issue-autofix` needs review-scan, so the dispatched issue phase idles + // with only a "queued" badge to explain it. + it('keeps the forced-scan lock as narrow as the job condition', () => { + const groupLine = reviewScanJob.match(/\n {6}group: ['"].*['"]/)?.[0] ?? ''; + expect(groupLine).toContain("needs.route.outputs.do_review == 'true'"); + // A skipped run must fall to a per-run group, never the shared per-PR one. + expect(groupLine).toContain('github.run_id'); + // The gate the group must mirror, matched on the job's own `if:` block so + // this stays a comparison and not a restatement of the group line. + expect(reviewScanJob).toContain( + "if: |-\n ${{ needs.route.outputs.do_review == 'true' }}", ); }); @@ -3680,7 +4440,7 @@ describe('qwen-autofix workflow', () => { /- name: 'Scan for PRs with new feedback'[\s\S]*?(?=\n[ ]{6}- name: )/, )?.[0] ?? ''; expect(reviewScanStep).toContain('isCrossRepository'); - expect(reviewScanStep).toContain('(.baseRefName // "") == "main"'); + expect(reviewScanStep).toContain('(.baseRefName // "") != "main"'); expect(reviewScanStep).toContain('--base main'); // review-address must check out trusted base, not PR merge ref. expect(workflow).toContain("'Checkout trusted base'");