ci(autofix): recover from generated-artifact CI gates and stop silent stalls

.github/scripts/check-settings-schema.sh

@@ -0,0 +1,40 @@
+#!/usr/bin/env bash
+# Settings-schema freshness gate, shared by the qwen-autofix verify steps
+# (.github/workflows/qwen-autofix.yml) so the two gates cannot drift apart.
+#
+# Mirrors CI's "Check settings schema is up-to-date" step EXACTLY: regenerate,
+# then fail if the committed artifact changed. Uses regenerate +
+# `git status --porcelain` (NOT the generator's --check, which was reverted
+# from main by #7031 — after merge this runs against main's generator, which
+# ignores args and would make --check fail-open). Stale schemas are invisible
+# to build/typecheck/lint/vitest.
+#
+# On failure: prints the diff, restores the schema file, writes
+# `outcome=failed` to $GITHUB_OUTPUT (when set, matching the calling step's
+# contract), and exits 1.
+set -uo pipefail
+
+SCHEMA_FILE='packages/vscode-ide-companion/schemas/settings.schema.json'
+
+fail() {
+  if [[ -n "${GITHUB_OUTPUT:-}" ]]; then
+    echo "outcome=failed" >> "${GITHUB_OUTPUT}"
+  fi
+  exit 1
+}
+
+# Guard the generator itself: if it CRASHES (e.g. a type error the agent
+# introduced in the schema source), a caller running under set -eo pipefail
+# would abort before outcome=failed is written, leaving OUTCOME unset. Handle
+# it here so the failure is explicit, not inferred from job.status.
+if ! npm run generate:settings-schema; then
+  echo "❌ Settings schema generator failed to run."
+  fail
+fi
+
+if [[ -n "$(git status --porcelain "${SCHEMA_FILE}")" ]]; then
+  echo "❌ ${SCHEMA_FILE} is out of date. Run: npm run generate:settings-schema"
+  git --no-pager diff -- "${SCHEMA_FILE}" || true
+  git checkout -- "${SCHEMA_FILE}" || true
+  fail
+fi

.github/workflows/qwen-autofix.yml

@@ -717,6 +717,7 @@ jobs:
                 "run_shell_command(npm run typecheck)",
                 "run_shell_command(npm run lint)",
                 "run_shell_command(npx vitest)",
+                "run_shell_command(npm run generate:settings-schema)",
                 "run_shell_command(pwd)"
               ],
               "tools": {
@@ -782,6 +783,12 @@ jobs:
           npm run typecheck
           npm run lint
 
+          # Settings-schema freshness gate, shared with the triage-and-address
+          # verify step so the two copies cannot drift (rationale + the
+          # generator crash guard live in the script). On failure it writes
+          # outcome=failed to GITHUB_OUTPUT and exits 1.
+          bash .github/scripts/check-settings-schema.sh
+
           # Run changed/related tests for the packages this fix touches.
           # --changed follows the import graph so transitive breakage is caught.
           # Full regression is covered by regular CI on the PR after the push.
@@ -979,46 +986,92 @@ jobs:
               "${WORKDIR}/bot-prs.json")"
           fi
 
+          # Pending-check staleness bound (invariant across candidate PRs, computed
+          # once): ignore a check stuck far past any legitimate runtime. The bound
+          # must sit ABOVE real check durations here — review-pr can take ~50m and
+          # review-address is capped at 120m — so an active run keeps blocking and
+          # is never aged out mid-flight (which would enqueue the PR against a live
+          # check and double-process the feedback).
+          PENDING_STALE_MIN=240
+          PENDING_CUTOFF="$(date -u -d "${PENDING_STALE_MIN} minutes ago" +%Y-%m-%dT%H:%M:%SZ)"
           TARGETS='[]'
           for PR in ${CANDIDATES}; do
-            BRANCH="$(gh pr view "${PR}" --repo "${REPO}" --json headRefName --jq '.headRefName')"
+            # One PR fetch for the branch name, check rollup, and creation time (the
+            # watermark floor below) — avoids extra round-trips per candidate PR.
+            PR_META="$(gh pr view "${PR}" --repo "${REPO}" \
+              --json headRefName,statusCheckRollup,createdAt 2> /dev/null || echo '{}')"
+            BRANCH="$(jq -r '.headRefName // ""' <<< "${PR_META}")"
+            if [[ -z "${BRANCH}" ]]; then
+              # Metadata fetch failed (transient API error / rate limit). Skip rather
+              # than fall through with an empty branch, which would make the address
+              # job fail at `git checkout -B "" origin/` and post a misleading "could
+              # not start evaluation" handoff. Retried on the next scan. (This also
+              # means CREATED_WM below is only reached with a populated PR_META.)
+              echo "⚠️ #${PR}: could not fetch PR metadata (API error); skipping until next scan"
+              continue
+            fi
             # Extract issue number: autofix/issue-<N> → N; otherwise use PR number.
             if [[ "${BRANCH}" == "${BRANCH_PREFIX}"* ]]; then
               ISSUE="${BRANCH#"${BRANCH_PREFIX}"}"
             else
               ISSUE="${PR}"
             fi
-            HEAD_SHA="$(gh api "repos/${REPO}/pulls/${PR}" --jq '.head.sha')"
-            CHECKS_JSON="$(gh pr view "${PR}" --repo "${REPO}" \
-              --json statusCheckRollup --jq '.statusCheckRollup // []' 2> /dev/null || echo '[]')"
-            HAS_PENDING_CHECKS="$(jq -r '
+            CHECKS_JSON="$(jq -c '.statusCheckRollup // []' <<< "${PR_META}")"
+            # startedAt is the only staleness clock: a check blocks only if it
+            # started within the bound; one with no startedAt (queued, not yet
+            # running) is not blocking (the next scan re-checks once it starts).
+            HAS_PENDING_CHECKS="$(jq -r --arg cut "${PENDING_CUTOFF}" '
               [ .[]
                 | select((.status // .state // "") | IN("QUEUED", "IN_PROGRESS", "PENDING", "WAITING", "REQUESTED"))
-                | select(((.workflowName // "") != "Qwen Autofix") or (((.name // "") | startswith("review-address")))) ]
+                | select(((.workflowName // "") != "Qwen Autofix") or (((.name // "") | startswith("review-address"))))
+                | select((.startedAt // $cut) > $cut) ]
               | length > 0
             ' <<< "${CHECKS_JSON}")"
             if [[ "${HAS_PENDING_CHECKS}" == "true" ]]; then
-              echo "⏳ #${PR}: PR has pending checks; skipping until the current verification finishes"
+              echo "⏳ #${PR}: active checks in flight; skipping until they finish (only checks stuck >${PENDING_STALE_MIN}m past their start are treated as dead and ignored)"
               continue
             fi
-            # Push watermark: the PR's last push. Feedback older than this was in
-            # front of the agent on a previous round.
-            PUSH_WM="$(gh api "repos/${REPO}/commits/${HEAD_SHA}" --jq '.commit.committer.date')"
+            # Pre-first-eval floor: the PR's IMMUTABLE creation time. Feedback
+            # cannot predate the PR, and unlike the head commit date this never
+            # advances when the branch is synced with main ("Update branch"/base
+            # merge), so an early base-sync merge cannot bury a comment made before
+            # the first eval. If the metadata query failed (empty), fall back to an
+            # EMPTY floor — over-inclusive (evaluates all feedback once, then the
+            # first eval writes a marker) but never buries. NEVER fall back to the
+            # mutable head commit date: a base-sync HEAD would recreate the burial.
+            CREATED_WM="$(jq -r '.createdAt // ""' <<< "${PR_META}")"
 
             gh api "repos/${REPO}/issues/${PR}/comments" --paginate > "${WORKDIR}/ic.json"
             # Eval markers the bot left after a previous evaluation carry the
             # newest feedback timestamp it already considered, plus the round.
             # Only our own comments are trusted, so a spoofed marker is ignored.
+            # NOTE: this regex is POSITIONAL — group .[0]=ts, .[2]=round — and must
+            # match the marker string emitted at every write site verbatim (search
+            # `autofix-eval ts=`: the push/report success, noop, and handoff steps).
+            # Inserting or reordering a field here or at any write site silently
+            # corrupts round tracking; keep the `ts= acted= round=` order in lockstep.
             MARKERS="$(jq -c --arg ab "${AUTOFIX_BOT}" '
               [ .[] | select((.user.login // "") == $ab) | (.body // "")
                 | [ scan("<!-- autofix-eval ts=([^ ]+) acted=([^ ]+) round=([0-9]+) -->") ] | .[]
                 | {ts: .[0], round: (.[2] | tonumber)} ]' "${WORKDIR}/ic.json")"
             EVAL_WM="$(jq -r 'map(.ts) | max // ""' <<< "${MARKERS}")"
-            ROUND="$(jq -r '(sort_by(.ts) | last | .round) // 0' <<< "${MARKERS}")"
-
-            # Effective watermark = the later of the last push and the last eval.
-            EFF_WM="${PUSH_WM}"
-            if [[ -n "${EVAL_WM}" && "${EVAL_WM}" > "${EFF_WM}" ]]; then EFF_WM="${EVAL_WM}"; fi
+            # Highest round across markers, not last-by-ts: a terminal handoff
+            # marker (round = MAX_ROUNDS) must make the scan skip regardless of its
+            # timestamp, and normal rounds increase monotonically anyway.
+            ROUND="$(jq -r 'map(.round) | max // 0' <<< "${MARKERS}")"
+
+            # Effective watermark = what the agent has actually evaluated (its last
+            # eval marker's newest-feedback timestamp), NOT the last push. A bot
+            # fix always writes a marker, so a real fix advances this; a base-sync
+            # "Merge branch 'main'" push (or any commit that did not evaluate
+            # feedback) does NOT, so it can never bury unaddressed maintainer
+            # comments under the watermark. Before the first evaluation there is no
+            # marker, so fall back to the PR creation floor.
+            if [[ -n "${EVAL_WM}" ]]; then
+              EFF_WM="${EVAL_WM}"
+            else
+              EFF_WM="${CREATED_WM}"
+            fi
 
             if [[ "${ROUND}" -ge "${MAX_ROUNDS}" ]]; then
               echo "🚧 #${PR}: hit MAX_ROUNDS (${ROUND}/${MAX_ROUNDS}) — leaving for a human"
@@ -1319,14 +1372,20 @@ jobs:
               | select((.conclusion // .state // "") | IN("FAILURE", "FAILED", "ERROR", "TIMED_OUT", "ACTION_REQUIRED", "CANCELLED"))
               | select(((.workflowName // "") != "Qwen Autofix") or (((.name // "") | startswith("review-address"))))
               | select((.completedAt // .updatedAt // "") > $wm)
-              | "- \((.workflowName // "external check") | gsub("[^A-Za-z0-9 _./()-]"; "") | .[0:80]): \(.conclusion // .state // "?")"' \
+              | "- \(((.name // .workflowName) // "external check") | gsub("[^A-Za-z0-9 _./()-]"; "") | .[0:80]): \(.conclusion // .state // "?")"' \
               "${WORKDIR}/checks.json"
           } > "${WORKDIR}/feedback.md"
           echo '--- feedback.md ---'
           cat "${WORKDIR}/feedback.md"
 
       - name: 'Triage and address'
         id: 'address'
+        # Bound the agent well below the 120-minute job timeout so a runaway agent
+        # fails THIS step (not the whole job), leaving the always() verify and
+        # report steps time to run and post a handoff. A job-level timeout would
+        # cancel those steps too and leave the loop silent. 80 leaves ~40 minutes
+        # of headroom for setup (install/build) plus verify + report.
+        timeout-minutes: 80
         env:
           PR: '${{ env.PR }}'
           ISSUE: '${{ env.ISSUE }}'
@@ -1360,6 +1419,7 @@ jobs:
                 "run_shell_command(npm run typecheck)",
                 "run_shell_command(npm run lint)",
                 "run_shell_command(npx vitest)",
+                "run_shell_command(npm run generate:settings-schema)",
                 "run_shell_command(pwd)"
               ],
               "tools": {
@@ -1405,6 +1465,17 @@ jobs:
 
           git checkout "${BRANCH}"
 
+          # Settings-schema freshness is a STRUCTURAL guard, checked BEFORE the
+          # no-op/unchanged return: on a stale-schema PR the agent can wrongly
+          # write no-action.md, and without this the no-op path would report the
+          # feedback as evaluated (acted=false) while CI stays red — the exact bug
+          # this PR fixes. So it runs on EVERY path. The gate is shared with the
+          # issue-fix verify step (rationale + the generator crash guard live in
+          # the script); the write is on a tracked file compared by `git status`,
+          # not the commit-level no-op git-diff below, and it is restored on
+          # failure. On failure it writes outcome=failed and exits 1.
+          bash .github/scripts/check-settings-schema.sh
+
           if git diff --quiet "origin/${BRANCH}...${BRANCH}"; then
             # No new commit. That is only legitimate as a deliberate no-action.
             if [[ -s "${WORKDIR}/no-action.md" ]]; then
@@ -1561,6 +1632,7 @@ jobs:
           DRY_RUN: '${{ needs.route.outputs.dry_run }}'
           GITHUB_TOKEN: '${{ secrets.CI_DEV_BOT_PAT }}'
           NEWEST: '${{ steps.prepare.outputs.newest }}'
+          JOB_STATUS: '${{ job.status }}'
         run: |-
           SUFFIX=''
           [[ "${DRY_RUN}" == "true" ]] && SUFFIX=' (dry-run, nothing pushed)'
@@ -1579,7 +1651,25 @@ jobs:
             done
           } >> "${GITHUB_STEP_SUMMARY}"
 
-          if [[ "${DRY_RUN}" != "true" && "${OUTCOME:-unknown}" == "failed" && -n "${NEWEST:-}" && -n "${GITHUB_TOKEN:-}" && -s "${WORKDIR}/handoff.md" ]]; then
+          # Leave a visible handoff + eval marker when the address did NOT publish a
+          # result — a verify failure, or an agent/infra crash or timeout before the
+          # verify gate ran. Without it the loop goes SILENT (no comment, no marker)
+          # and the next scan re-targets the same feedback forever.
+          #
+          # SUPPRESS entirely once "Push and report" already handled this run
+          # (OUTCOME fixed or noop). That step is also always()-gated and runs even
+          # if a LATER always() step (e.g. artifact upload) fails the job; without
+          # this guard, such a late failure would flip JOB_STATUS to failure and
+          # post a contradictory acted=false handoff on top of the published fix.
+          # (A genuine push failure leaves OUTCOME=fixed but writes no marker, so
+          # the next scan simply retries — it does not need a handoff here.)
+          POST_HANDOFF=false
+          if [[ "${DRY_RUN}" != "true" && -n "${GITHUB_TOKEN:-}" && "${OUTCOME:-unknown}" != "fixed" && "${OUTCOME:-unknown}" != "noop" ]]; then
+            if [[ "${OUTCOME:-unknown}" == "failed" || "${JOB_STATUS:-}" != "success" ]]; then
+              POST_HANDOFF=true
+            fi
+          fi
+          if [[ "${POST_HANDOFF}" == "true" ]]; then
             api_error_file="$(mktemp)"
             if ! bot_actor="$(GH_TOKEN="${GITHUB_TOKEN}" gh api user --jq '.login' 2>"${api_error_file}")"; then
               api_error="$(tr '\r\n' ' ' < "${api_error_file}")"
@@ -1593,18 +1683,66 @@ jobs:
               echo "::error::CI_DEV_BOT_PAT authenticates as ${bot_actor}; expected ${AUTOFIX_BOT}."
               exit 1
             fi
+            # Attach the most actionable agent output. failure.md first (its
+            # diagnosis; run-agent.mjs wraps it in a generic handoff.md, so prefer
+            # failure.md). Then the agent's SUCCESS outputs: on the OUTCOME=failed
+            # path where the agent committed a fix but a post-agent verify gate then
+            # failed (most notably the schema-freshness gate), only
+            # address-summary.md/no-action.md exist and "Push and report" is
+            # skipped, so this handoff is their only route to the PR — otherwise the
+            # comment would wrongly say "crashed or timed out". Generic notice only
+            # if none exist.
+            DETAIL_FILE=''
+            for f in failure.md handoff.md address-summary.md no-action.md; do
+              if [[ -s "${WORKDIR}/${f}" ]]; then DETAIL_FILE="${WORKDIR}/${f}"; break; fi
+            done
+            # If feedback was actually read (prepare ran), stamp its newest ts so
+            # the watermark advances and the same feedback is not re-selected next
+            # scan. If the crash happened before prepare, NEWEST is empty and the
+            # watermark cannot advance — mark the round terminal (MAX_ROUNDS) so the
+            # scan's max-round guard skips this PR instead of re-handing-off every
+            # tick, without pretending the unread feedback was evaluated. The final
+            # sentinel guards a cascading API failure that left WATERMARK empty too:
+            # an empty ts= would not match the scan's `ts=([^ ]+)` regex, so the
+            # terminal marker would be ignored and the PR re-handed-off. A far-future
+            # ISO-8601 date is used (not a bare word) so it is both non-empty AND
+            # sorts above any real timestamp in EVAL_WM's max, belt-and-suspenders
+            # with the terminal round.
+            MARK_TS="${NEWEST:-${WATERMARK:-9999-12-31T23:59:59Z}}"
+            if [[ -n "${NEWEST:-}" ]]; then
+              MARK_ROUND="$(( ROUND + 1 ))"
+              HEADLINE="🤖 Could not address the latest feedback automatically (round ${MARK_ROUND}/${MAX_ROUNDS}). A human should take over this PR."
+            else
+              # Crashed/timed out before reading the feedback: mark terminal so the
+              # scan skips (it can't advance the watermark without a read), but say
+              # so plainly — do NOT imply MAX_ROUNDS attempts were made when zero
+              # rounds happened. Because the marker is terminal, the max-round guard
+              # skips this PR on EVERY future scan, including a forced dispatch, so
+              # the headline must state the real recovery (delete the marker) rather
+              # than promise a re-trigger that the guard would ignore.
+              MARK_ROUND="${MAX_ROUNDS}"
+              HEADLINE="🤖 AutoFix could not start evaluation — it crashed or timed out before reading the feedback, so no fix was attempted. This PR is now marked terminal and future scans (including forced dispatch) will skip it. To recover: delete this bot's terminal \`autofix-eval\` marker comment, then re-trigger if the failure looked transient."
+            fi
             {
-              echo "🤖 Could not address the latest review feedback automatically."
+              echo "${HEADLINE}"
               echo
-              echo "The feedback was evaluated, but AutoFix failed before producing a verified commit. A human should take over this PR."
-              echo
-              echo "Run: ${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}"
+              if [[ -n "${DETAIL_FILE}" ]]; then
+                echo "**What I found before stopping:**"
+                # -c drops any partial multi-byte sequence a byte-level head -c may
+                # have split, so the comment body stays valid UTF-8. iconv -c still
+                # EXITS 1 when it discards a byte, which under this shell's
+                # `set -eo pipefail` would abort the step and skip the marker + gh
+                # pr comment below — the exact silent stall this block prevents — so
+                # `|| true` keeps the (already-emitted) cleaned text and continues.
+                head -c 1500 "${DETAIL_FILE}" | iconv -f utf-8 -t utf-8 -c | sed 's/<!--[^>]*-->//g' || true
+              else
+                echo "AutoFix failed before producing a verified commit (the run crashed or timed out before it could explain why)."
+              fi
               echo
-              echo "**Failure:**"
-              head -c 1500 "${WORKDIR}/failure.md" | sed 's/<!--[^>]*-->//g'
               echo
+              echo "Run log: ${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}"
               echo
-              echo "<!-- autofix-eval ts=${NEWEST} acted=false round=${ROUND} -->"
+              echo "<!-- autofix-eval ts=${MARK_TS} acted=false round=${MARK_ROUND} -->"
             } > "${WORKDIR}/report.md"
             gh pr comment "${PR}" --repo "${REPO}" --body-file "${WORKDIR}/report.md" || echo "::warning::Failed to post handoff comment on PR #${PR}"
           fi