fix(ci): allow 90 minutes for ECS runner version resolution

.github/workflows/update-ecs-runner-qwen.yml

@@ -25,7 +25,7 @@ jobs:
     name: 'Resolve version'
     if: "${{ github.repository == 'QwenLM/qwen-code' }}"
     runs-on: 'ubuntu-latest'
-    timeout-minutes: 30
+    timeout-minutes: 100
     outputs:
       version: '${{ steps.version.outputs.version }}'
     steps:
@@ -36,12 +36,16 @@ jobs:
           # npm publishes asynchronously: `npm publish --provenance` returns
           # "Your package is being processed and may take a few minutes to
           # become available", and release.yml dispatches this workflow as soon
-          # as it returns. For v0.22.3 the gap was ~16 minutes (published
-          # 17:14Z, resolvable 17:30Z), so the single un-retried `npm view` this
-          # step used to run 404'd on 3 of the 4 pools and left the fleet split
-          # across two CLI versions until a maintainer noticed. Wait the
-          # registry out rather than losing the race.
-          RESOLVE_TIMEOUT_SECONDS: '1500'
+          # as it returns. The gap grows with the package size and varies
+          # widely: v0.22.3 took ~16 minutes, and v0.23.0 (a ~100 MB main
+          # package) took 55m22s measured end to end (npm accepted the upload
+          # 10:58:30Z, the registry served it 11:53:52Z). The previous 1500s
+          # budget gave up long before that and the run failed with the
+          # package still in flight, leaving the fleet on the old version.
+          # Budget ~1 hour for a main-package release and treat anything past
+          # 90 minutes as anomalous. The wait polls every 30s, so fast
+          # publishes still resolve immediately.
+          RESOLVE_TIMEOUT_SECONDS: '5400'
           RESOLVE_INTERVAL_SECONDS: '30'
         run: |-
           set -euo pipefail
@@ -97,9 +101,22 @@ jobs:
             # package dir (ENOTEMPTY). Clear trash left by crashed installs
             # and retry through the seconds-long race window.
             PKG_DIR='/usr/local/lib/node_modules/@qwen-code'
+            # --prefix pins the install to /usr/local: on the hk-4/hk-5
+            # machines root's npm global prefix is a custom Node directory
+            # (/usr/local/lib/nodejs/node-v22.23.2-linux-x64), so without
+            # it the update landed there while the pool kept resolving the
+            # stale /usr/local/bin/qwen -> /usr/local/lib/node_modules copy.
+            # Some pool machines have no passwordless sudo for the runner
+            # user; fall back to installing as that user, which works when
+            # it owns the prefix.
+            if sudo -n true 2>/dev/null; then
+              INSTALL=(sudo env -u NPM_CONFIG_PREFIX)
+            else
+              INSTALL=(env)
+            fi
             for attempt in 1 2 3; do
               sudo rm -rf "${PKG_DIR}"/.qwen-code-* 2>/dev/null || true
-              if sudo env -u NPM_CONFIG_PREFIX npm install -g --registry=https://registry.npmjs.org "@qwen-code/qwen-code@${VERSION}"; then
+              if "${INSTALL[@]}" npm install -g --prefix /usr/local --registry=https://registry.npmjs.org "@qwen-code/qwen-code@${VERSION}"; then
                 exit 0
               fi
               if [[ "${attempt}" -lt 3 ]]; then
@@ -118,9 +135,38 @@ jobs:
         run: |-
           set -euo pipefail
           qwen_path="$(command -v qwen)"
-          actual="$("${qwen_path}" --version)"
+          # Tolerate a broken entrypoint: a crashed install can leave
+          # `command -v qwen` resolving to a binary whose `--version` exits
+          # non-zero, and under `set -e` a failing command substitution
+          # feeding a bare assignment aborts the step right here — before
+          # the mismatch diagnostics below can print, for exactly the
+          # failure class they exist for. The version test at the end
+          # still fails the step either way. The capture stays stdout-only:
+          # a stderr byte would fold into the strict-matched version and
+          # fail a healthy install on one runtime warning.
+          actual="$("${qwen_path}" --version)" || actual="(qwen --version failed, exit $?)"
           echo "qwen path: ${qwen_path}"
           echo "qwen version: ${actual}"
+          if [[ "${actual}" != "${VERSION}" ]]; then
+            # Diagnose where the stale binary comes from before failing:
+            # the install step's npm may write to a different prefix than
+            # the one `command -v` resolves on this pool. The probes must
+            # work on the pools without passwordless sudo — the same pools
+            # the install fallback serves — where an interactive `sudo`
+            # blocks on a password prompt until the job timeout kills the
+            # step, so they run as the runner user or with `sudo -n`, and
+            # the runner-user variants see what the fallback installed.
+            echo "--- diagnostics ---"
+            echo "readlink: $(readlink -f "${qwen_path}" 2>/dev/null || echo '(not a link)')"
+            ls -la /usr/local/bin/qwen /usr/bin/qwen 2>/dev/null || true
+            echo "npm prefix (user): $(npm prefix -g 2>/dev/null || true)"
+            echo "npm prefix (sudo): $(sudo -n env -u NPM_CONFIG_PREFIX npm prefix -g 2>/dev/null || true)"
+            cat /usr/local/lib/node_modules/@qwen-code/qwen-code/package.json 2>/dev/null | grep '"version"' || true
+            sudo -n cat /usr/local/lib/node_modules/@qwen-code/qwen-code/package.json 2>/dev/null | grep '"version"' || true
+            ls -la /usr/local/lib/node_modules/@qwen-code/ 2>/dev/null || true
+            sudo -n ls /usr/local/lib/node_modules/@qwen-code/ 2>/dev/null || true
+            echo "--- end diagnostics ---"
+          fi
           test "${actual}" = "${VERSION}"
 
   # A failed fleet update is otherwise invisible. `qwen-code-pr-review.yml` and

scripts/tests/update-ecs-runner-qwen-workflow.test.js

@@ -112,7 +112,67 @@ const DEFAULT_JOBS = jobsFixture({
 describe('ECS runner qwen update workflow', () => {
   it('installs without the selected runner npm prefix', () => {
     expect(workflow).toContain('cd "${RUNNER_TEMP:?}"');
-    expect(workflow).toContain('sudo env -u NPM_CONFIG_PREFIX npm install -g');
+    expect(workflow).toContain('INSTALL=(sudo env -u NPM_CONFIG_PREFIX)');
+    expect(workflow).toContain(
+      '"${INSTALL[@]}" npm install -g --prefix /usr/local',
+    );
+  });
+
+  it('keeps the verify diagnostics usable without passwordless sudo', () => {
+    // The diagnostics exist for the pools where the install step fell back
+    // to the runner user; on those pools an interactive `sudo` blocks on a
+    // password prompt until the job timeout kills the step, so every sudo
+    // probe must be non-interactive and the package dir must also be
+    // probed as the runner user sees it.
+    const verify = stepBody('Verify version');
+    expect(verify).toContain('npm prefix (user): $(npm prefix -g');
+    expect(verify).toMatch(
+      /^\s*cat \/usr\/local\/lib\/node_modules\/@qwen-code\/qwen-code\/package\.json/m,
+    );
+    // Assert the class documented above — every sudo probe must be
+    // non-interactive — instead of enumerating the verbs used today: a bare
+    // `sudo` with any other verb passes an enumeration and blocks on a
+    // password prompt until the job timeout on exactly the pools these
+    // diagnostics exist for. The probe comment mentions `sudo` in prose, so
+    // the comment lines stay out of the check.
+    const verifyCode = verify
+      .split('\n')
+      .filter((line) => !line.trim().startsWith('#'))
+      .join('\n');
+    expect(verifyCode).not.toMatch(/sudo\s+(?!-n\b)/);
+  });
+
+  it('captures the installed version tolerantly', () => {
+    // Under `set -e`, a failing `--version` feeding a bare assignment
+    // aborts the step before the mismatch branch — and the diagnostics
+    // this step exists to print — ever runs. The capture stays stdout-only:
+    // any stderr byte would fold into the strict-matched version and fail a
+    // healthy install on one runtime warning.
+    expect(stepBody('Verify version')).toContain(
+      'actual="$("${qwen_path}" --version)" || actual=',
+    );
+  });
+
+  it('keeps the resolve job timeout above the shipped wait budget', () => {
+    // The replay harness injects its own timeout values, so nothing else
+    // guards the shipped budget: a merge resolution that lowered the job
+    // timeout below the wait would let GitHub kill the poll while the
+    // registry is still propagating, silently restoring the v0.23.0
+    // failure this PR fixes.
+    const resolveJob = workflow.slice(
+      workflow.indexOf('\n  resolve:'),
+      workflow.indexOf('\n  update:'),
+    );
+    expect(resolveJob).toContain("RESOLVE_TIMEOUT_SECONDS: '5400'");
+    const timeoutMinutes = Number(
+      resolveJob.match(/timeout-minutes: (\d+)/)?.[1] ?? 0,
+    );
+    const budgetSeconds = Number(
+      resolveJob.match(/RESOLVE_TIMEOUT_SECONDS: '(\d+)'/)?.[1] ?? 0,
+    );
+    expect(budgetSeconds).toBe(5400);
+    // The job must outlive the whole wait plus the final poll's npm call.
+    expect(timeoutMinutes * 60).toBeGreaterThanOrEqual(budgetSeconds + 600);
   });
 
   it('runs only when this workflow changes on main', () => {
@@ -290,6 +350,50 @@ function runResolve({ failures = 0, version = '0.22.3', env = {} } = {}) {
   }
 }
 
+// Runs the 'Verify version' step body against a stubbed `qwen` whose
+// `--version` prints `output` (stdout), optionally `stderrOutput` (stderr),
+// and exits with `exitCode`.
+function runVerify({
+  output = '',
+  stderrOutput = '',
+  exitCode = 0,
+  target = '0.22.3',
+} = {}) {
+  const dir = mkdtempSync(join(tmpdir(), 'ecs-verify-'));
+  try {
+    const qwenStub = join(dir, 'qwen');
+    const stubLines = ['#!/usr/bin/env bash'];
+    if (stderrOutput) {
+      stubLines.push(`echo '${stderrOutput}' >&2`);
+    }
+    if (output) {
+      stubLines.push(`echo '${output}'`);
+    }
+    stubLines.push(`exit ${exitCode}`);
+    writeFileSync(qwenStub, stubLines.join('\n'), { mode: 0o755 });
+    chmodSync(qwenStub, 0o755);
+
+    const script = join(dir, 'verify.sh');
+    writeFileSync(script, stepBody('Verify version'));
+
+    const result = spawnSync('bash', [script], {
+      encoding: 'utf8',
+      env: {
+        ...process.env,
+        PATH: `${dir}:${process.env.PATH ?? ''}`,
+        VERSION: target,
+      },
+    });
+    return {
+      status: result.status,
+      stdout: result.stdout ?? '',
+      stderr: result.stderr ?? '',
+    };
+  } finally {
+    rmSync(dir, { recursive: true, force: true });
+  }
+}
+
 // Runs .github/scripts/ecs-fleet-update-failure-issue.sh against a stubbed
 // `gh`. The stub applies the script's real `--jq` filter with real jq, so the
 // pool-naming expression is exercised rather than mocked away, and it honours
@@ -418,6 +522,48 @@ describe.skipIf(!replayable)('ECS runner qwen update replay', () => {
     expect(resolved.output.trim()).toBe('version=0.22.3');
   });
 
+  it('verifies a healthy install without diagnostics', () => {
+    const verified = runVerify({ output: '0.22.3', target: '0.22.3' });
+    expect(verified.status).toBe(0);
+    expect(verified.stdout).toContain('qwen version: 0.22.3');
+    expect(verified.stdout).not.toContain('--- diagnostics ---');
+  });
+
+  it('ignores stderr noise on a successful --version', () => {
+    // The capture is stdout-only: one stderr line during `--version` (a Node
+    // runtime warning, a future startup notice) must not fail a healthy
+    // install on every pool and file a stale-fleet issue against a correctly
+    // updated fleet.
+    const verified = runVerify({
+      output: '0.22.3',
+      stderrOutput: '(node:1234) ExperimentalWarning: some future warning',
+      target: '0.22.3',
+    });
+    expect(verified.status).toBe(0);
+    expect(verified.stdout).toContain('qwen version: 0.22.3');
+    expect(verified.stdout).not.toContain('--- diagnostics ---');
+  });
+
+  it('prints the diagnostics when the installed qwen is stale', () => {
+    const verified = runVerify({ output: '0.22.2', target: '0.22.3' });
+    expect(verified.status).toBe(1);
+    expect(verified.stdout).toContain('--- diagnostics ---');
+    expect(verified.stdout).toContain('--- end diagnostics ---');
+  });
+
+  it('prints the diagnostics when the installed qwen cannot run at all', () => {
+    // A crashed install can leave `command -v qwen` resolving to a broken
+    // entrypoint whose `--version` exits non-zero; the tolerant capture is
+    // what keeps the step failing at the version test with diagnostics
+    // instead of dying at the bare assignment under `set -e`.
+    const verified = runVerify({ exitCode: 127, target: '0.22.3' });
+    expect(verified.status).toBe(1);
+    expect(verified.stdout).toContain(
+      'qwen version: (qwen --version failed, exit 127)',
+    );
+    expect(verified.stdout).toContain('--- diagnostics ---');
+  });
+
   it('files an issue naming the pools left on the old CLI', () => {
     const reported = runReport({ openIssues: [] });
     expect(reported.status).toBe(0);