fix(ci): keep the review workflow under the expression-length limit

.github/workflows/qwen-code-pr-review.yml

@@ -748,6 +748,16 @@ jobs:
           # the next on the reused self-hosted workspace (reset in "Clean stale
           # agent state"). Must match the QWEN_HOME computed there.
           QWEN_HOME: '${{ runner.temp }}/qwen-home'
+          # KEEP THE `run` BODY BELOW FREE OF `${{ }}`. A run block containing
+          # one is evaluated as a single expression template, capped at 21000
+          # characters — and this script is far past that. Every value it needs
+          # from the workflow context is passed as an environment variable, so
+          # the body is plain bash the runner never templates. See the
+          # workflow-expression-length test in
+          # scripts/tests/qwen-pr-review-workflow.test.js.
+          EVENT_NAME: '${{ github.event_name }}'
+          EVENT_HEAD_SHA: '${{ github.event.pull_request.head.sha }}'
+          MAX_TIMEOUT_MINUTES_VAR: '${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}'
         run: |-
           set -euo pipefail
           fail() {
@@ -960,7 +970,7 @@ jobs:
           if [ "$TIMEOUT_MINUTES" -le 5 ]; then
             fail "timeout_minutes must be greater than 5"
           fi
-          MAX_TIMEOUT_MINUTES="${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}"
+          MAX_TIMEOUT_MINUTES="$MAX_TIMEOUT_MINUTES_VAR"
           if [ "$TIMEOUT_MINUTES" -gt "$MAX_TIMEOUT_MINUTES" ]; then
             fail "timeout_minutes must not exceed ${MAX_TIMEOUT_MINUTES} minutes"
           fi
@@ -989,7 +999,7 @@ jobs:
               if [ "$PR_SIZE_LINES" -le 300 ]; then
                 EFFECTIVE_TIMEOUT_MINUTES=180
               else
-                EFFECTIVE_TIMEOUT_MINUTES="${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}"
+                EFFECTIVE_TIMEOUT_MINUTES="$MAX_TIMEOUT_MINUTES_VAR"
               fi
               echo "PR #${PR_NUMBER} changed ${PR_SIZE_LINES} lines; auto timeout ${EFFECTIVE_TIMEOUT_MINUTES} minutes."
             else
@@ -1056,8 +1066,7 @@ jobs:
             exit 0
           fi
           EXPECTED_HEAD_SHA="$CURRENT_HEAD_SHA"
-          if [ "${{ github.event_name }}" = "pull_request_target" ]; then
-            EVENT_HEAD_SHA="${{ github.event.pull_request.head.sha }}"
+          if [ "$EVENT_NAME" = "pull_request_target" ]; then
             if [ "$CURRENT_HEAD_SHA" != "$EVENT_HEAD_SHA" ]; then
               echo "Skipping stale review run: event head ${EVENT_HEAD_SHA} is no longer current (current head ${CURRENT_HEAD_SHA})." | tee -a "$GITHUB_STEP_SUMMARY"
               exit 0

scripts/tests/qwen-pr-review-workflow.test.js

@@ -2161,3 +2161,49 @@ describe('upstream-timeout headroom (PR 8507 incident)', () => {
     );
   });
 });
+
+describe('workflow expression length', () => {
+  // A `run:` body containing `${{ }}` is evaluated as ONE expression template,
+  // and GitHub caps a single expression at 21000 characters. Blowing that cap
+  // does not fail a job — it makes the whole workflow file *invalid*, so no
+  // event triggers it at all and no run is even created for the ones that
+  // matter. That is how every automatic review and every `@qwen-code /review`
+  // in this repository silently stopped for ~12h on 2026-08-07: #8648 pushed
+  // the "Run review" body from 17705 to 22282 characters, and from that merge
+  // onward the only runs left were startup failures reading
+  // `Invalid workflow file: … (Line: 751, Col: 14): Exceeded max expression
+  // length 21000` (e.g. run 31239579253). CI stayed green the whole time — no
+  // test covered this, which is why it is covered here.
+  const LIMIT = 21000;
+  const dir = '.github/workflows';
+  const files = readdirSync(dir).filter((f) => /\.ya?ml$/.test(f));
+
+  it('keeps every templated run block under the limit', () => {
+    expect(files.length).toBeGreaterThan(0);
+    const over = [];
+    for (const file of files) {
+      const doc = parse(readFileSync(join(dir, file), 'utf8'));
+      for (const [jobId, job] of Object.entries(doc?.jobs ?? {})) {
+        for (const step of job?.steps ?? []) {
+          const body = step?.run;
+          if (typeof body !== 'string' || !body.includes('${{')) continue;
+          if (body.length > LIMIT) {
+            over.push(
+              `${file} › ${jobId} › ${step.name}: ${body.length} chars`,
+            );
+          }
+        }
+      }
+    }
+    expect(over).toEqual([]);
+  });
+
+  it('keeps the review script free of ${{ }} so its length cannot break it', () => {
+    // This one body is ~24000 characters — already past the limit — so it stays
+    // valid only while nothing templates it. Every context value it needs is
+    // passed through the step's `env:` instead. A single `${{ }}` added back
+    // here takes the entire workflow down, which the test above would also
+    // catch; this asserts the actual invariant a contributor has to preserve.
+    expect(runReviewStep()).not.toContain('${{');
+  });
+});

scripts/tests/qwen-resolve-workflow.test.js

@@ -356,8 +356,13 @@ describe('qwen resolve workflow', () => {
     expect(contextStep).toContain('timeout=*)');
     expect(contextStep).toContain('TIMEOUT_MINUTES="${token#timeout=}"');
     expect(runStep).toContain('if [ "${#TIMEOUT_MINUTES}" -gt 3 ]; then');
+    // The cap still comes from the repository variable, but reaches the script
+    // through the step's env: the run body must stay free of `${{ }}` or the
+    // whole workflow exceeds the 21000-character expression limit and becomes
+    // invalid. Both halves are asserted so neither can drift alone.
+    expect(runStep).toContain('MAX_TIMEOUT_MINUTES="$MAX_TIMEOUT_MINUTES_VAR"');
     expect(runStep).toContain(
-      'MAX_TIMEOUT_MINUTES="${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}"',
+      "MAX_TIMEOUT_MINUTES_VAR: '${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}'",
     );
     expect(runStep).toContain(
       'if [ "$TIMEOUT_MINUTES" -gt "$MAX_TIMEOUT_MINUTES" ]; then',
@@ -403,7 +408,10 @@ describe('qwen resolve workflow', () => {
     expect(sizeGuardArm).toContain('if [ "$PR_SIZE_LINES" -le 300 ]; then');
     expect(runStep).toContain('EFFECTIVE_TIMEOUT_MINUTES=180');
     expect(runStep).toContain(
-      'EFFECTIVE_TIMEOUT_MINUTES="${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}"',
+      'EFFECTIVE_TIMEOUT_MINUTES="$MAX_TIMEOUT_MINUTES_VAR"',
+    );
+    expect(runStep).toContain(
+      "MAX_TIMEOUT_MINUTES_VAR: '${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}'",
     );
     // Slice the small-PR arm so a swap of the two assignments between the
     // branches fails: unordered containment keeps both texts present.
@@ -416,7 +424,7 @@ describe('qwen resolve workflow', () => {
       runStep.indexOf('else', smallPrStart),
     );
     expect(smallPrArm).toContain('EFFECTIVE_TIMEOUT_MINUTES=180');
-    expect(smallPrArm).not.toContain('vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES');
+    expect(smallPrArm).not.toContain('MAX_TIMEOUT_MINUTES_VAR');
     expect(runStep).not.toContain('EFFECTIVE_TIMEOUT_MINUTES=210');
     expect(runStep).toContain(
       'echo "effective_timeout_minutes=$EFFECTIVE_TIMEOUT_MINUTES"',
@@ -532,15 +540,26 @@ describe('qwen resolve workflow', () => {
 
   it('skips stale automatic review runs before invoking qwen', () => {
     const runStep = step(reviewJob, 'Run review');
+    const staleHeadStart = runStep.indexOf(
+      'if [ "$EVENT_NAME" = "pull_request_target" ]; then',
+    );
+    // Without this, a reworded guard makes `indexOf` return -1 and the slice
+    // below silently degrades instead of failing.
+    expect(staleHeadStart).toBeGreaterThan(-1);
     const staleHeadCheck = runStep.slice(
-      runStep.indexOf(
-        'if [ "${{ github.event_name }}" = "pull_request_target" ]; then',
-      ),
+      staleHeadStart,
       runStep.indexOf('PROMPT="/review ${REVIEW_URL}"'),
     );
 
+    // Both context values arrive as step env so the run body carries no
+    // `${{ }}` — see the expression-length test in
+    // qwen-pr-review-workflow.test.js for why that is load-bearing.
+    expect(runStep).toContain("EVENT_NAME: '${{ github.event_name }}'");
+    expect(runStep).toContain(
+      "EVENT_HEAD_SHA: '${{ github.event.pull_request.head.sha }}'",
+    );
     expect(staleHeadCheck).toContain(
-      'EVENT_HEAD_SHA="${{ github.event.pull_request.head.sha }}"',
+      'if [ "$CURRENT_HEAD_SHA" != "$EVENT_HEAD_SHA" ]; then',
     );
     expect(runStep).toContain(
       'PR_DATA="$(gh pr view "$PR_NUMBER" --repo "$REPO" --json state,headRefOid --jq \'[.state, .headRefOid] | @tsv\')"',