ci: address review findings across the status-comment pipeline

Fixes for the 10 open review threads:

- ucb-build-links: URL_RE now requires /builds/<digits> under the Unity hosts, mirroring build.py's producer check — query-string-only paths (open-redirect bait) no longer validate.
- pr-comment-artifact-url: artifacts list fetched with per_page=100 so unity_build_info_* cannot fall off page 1 and fake build-ran=false; the PAT expiry probe moved to gh api HEAD /rate_limit (token via env, never argv), gained continue-on-error + a 15s ceiling + a loud warning when the expiration header is absent; the dispatch-failed marker now fires on outcome != success so upstream failures (Find latest release, section upserts) no longer leave the performance section silent.
- pr-comment-test-failures: slowest/failed test names render inside inline code with backticks/pipes stripped — markdown-shaped names read as text instead of first-party links/images.
- test.yml: duration values are clamped to finite floats (float() admits nan/inf/1e999), keeping the timings JSON valid for the consumer's jq.
- visual-regression: the two comment-writing jobs drop to contents:read + pull-requests:write. Narrowing secrets:inherit needs run-visual-suite.yml to declare workflow_call secrets first (it currently instructs callers to use inherit), so that part stays.
- upsert-ci-status: fence-existence check is whole-line (grep -qxF) matching the awk matchers, so an embedded marker in a body line can no longer wedge a section; file-passed bodies are capped at 20k chars with a visible truncation note (GitHub's 65536 ceiling is shared by all sections); SECTION is validated against the fence set and fails fast; duplicate-comment GC is skipped for NO_CREATE callers; NO_CREATE waits one round before falling back to a standalone comment.

Committed via API because repository rules require verified signatures.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: eordano

.github/actions/ci-status-comment/upsert-ci-status.sh

@@ -33,8 +33,24 @@ set -euo pipefail
 #                       find it, spawning duplicates).
 if [ -n "${SECTION_BODY_FILE:-}" ]; then
   SECTION_BODY="$(cat "$SECTION_BODY_FILE")"
+  # GitHub caps an issue comment at 65536 chars across every section; keep one
+  # writer from consuming the whole budget and failing an unrelated section's
+  # PATCH with an opaque 422. Truncation is fine for a status section that
+  # already links out to the full report.
+  if [ "${#SECTION_BODY}" -gt 20000 ]; then
+    echo "::warning::Section body is ${#SECTION_BODY} chars; truncating to 20000."
+    SECTION_BODY="${SECTION_BODY:0:20000}"$'\n\n'"_…truncated; see the linked run for the full report._"
+  fi
 fi
 
+# Fail fast on a section name outside the fence set — an unknown name would
+# append a dead fence to the shared comment and then wedge the survive check
+# for 5 attempts, burning ~15 API calls per write from then on.
+case "${SECTION:-}" in
+  build|lint|tests|performance|automation) ;;
+  *) echo "::error::Unknown section '${SECTION:-}'."; exit 2 ;;
+esac
+
 MARKER="<!-- ci-status -->"
 HEADER="### 🚦 CI Status"
 BOT="github-actions[bot]"
@@ -114,12 +130,23 @@ for attempt in 1 2 3 4 5; do
   COMMENT_ID="${IDS[0]:-}"
 
   if [ -z "$COMMENT_ID" ] && [ -n "${NO_CREATE:-}" ]; then
-    echo "No unified CI status comment exists and NO_CREATE is set; leaving creation to the repo's own workflows."
-    exit 3
+    # Lose one round before falling back: an external caller often lands here
+    # seconds before the build workflow seeds the comment, and the standalone
+    # fallback it would post instead is noise that never collapses.
+    if [ "$attempt" -ge 2 ]; then
+      echo "No unified CI status comment exists and NO_CREATE is set; leaving creation to the repo's own workflows."
+      exit 3
+    fi
+    echo "No unified CI status comment yet (attempt $attempt); waiting for the repo's own workflows to seed it."
+    sleep $((attempt * 2))
+    continue
   fi
 
-  # Collapse accidental duplicates from a create race: keep the oldest, drop the rest.
-  if [ "${#IDS[@]}" -gt 1 ]; then
+  # Collapse accidental duplicates from a create race: keep the oldest, drop the
+  # rest. Skipped for external callers — comment GC belongs to this repo's own
+  # workflows, which run often enough to clean up within minutes, and a misfire
+  # under a foreign token would delete evidence with nothing logged.
+  if [ "${#IDS[@]}" -gt 1 ] && [ -z "${NO_CREATE:-}" ]; then
     for extra in "${IDS[@]:1}"; do
       echo "Deleting duplicate CI status comment $extra."
       gh api -X DELETE "/repos/$REPO/issues/comments/$extra" >/dev/null || true
@@ -138,7 +165,11 @@ for attempt in 1 2 3 4 5; do
   # instead of resetting the whole comment and wiping the other sections' state.
   if [ -z "$CURRENT_BODY" ]; then
     CURRENT_BODY="$(skeleton)"
-  elif ! grep -qF "$START" <<< "$CURRENT_BODY"; then
+  # -x: whole-line, matching replace_section/extract_section's $0==s exactly. A
+  # substring hit on a marker embedded in a body line (which the strip filter
+  # deliberately lets through) would skip fence creation here while the awk
+  # matchers see nothing — leaving the section permanently unwritable.
+  elif ! grep -qxF "$START" <<< "$CURRENT_BODY"; then
     CURRENT_BODY="$CURRENT_BODY"$'\n\n'"$(wrap_section "$SECTION" "$(section_default "$SECTION")")"
   fi
 

.github/actions/ucb-build-links/action.yml

@@ -41,7 +41,10 @@ runs:
         # The info files come out of the PR-controlled build workflow, so treat them as
         # untrusted input: accept only a numeric build id and a Unity dashboard URL with
         # a conservative charset before letting them anywhere near a comment body.
-        URL_RE='^https://(cloud\.unity\.com|developer\.cloud\.unity3d\.com|dashboard\.unity3d\.com)/[A-Za-z0-9./_%~?=&#-]*$'
+        # Mirrors the producer's '/builds/<id>' requirement (build.py) so the two
+        # validators agree, and pins the id to digits — a query-string-only path
+        # under a Unity host (open-redirect bait) no longer passes.
+        URL_RE='^https://(cloud\.unity\.com|developer\.cloud\.unity3d\.com|dashboard\.unity3d\.com)/[A-Za-z0-9./_%~?=&#-]*/builds/[0-9]+[A-Za-z0-9./_%~?=&#-]*$'
         parse_info() {
           local target="$1"
           local dir="ucb_info_${target}"

.github/workflows/pr-comment-artifact-url.yml

@@ -88,7 +88,11 @@ jobs:
           # uploaded as soon as the build id is known, so a build that failed before
           # producing player artifacts still posts a failure comment (with the Unity
           # Cloud link) instead of leaving the comment stuck on "Pending".
-          NAMES=$(gh api "/repos/$OWNER/$REPO/actions/runs/$RUN_ID/artifacts" \
+          # per_page=100: the default page holds 30 and a two-target run already
+          # uploads ~a dozen artifacts — unity_build_info_* falling off page 1
+          # would read as build-ran=false, the exact stuck-on-Pending bug the
+          # flag exists to prevent.
+          NAMES=$(gh api "/repos/$OWNER/$REPO/actions/runs/$RUN_ID/artifacts?per_page=100" \
             --jq '[.artifacts[].name]')
           PLAYER_COUNT=$(jq 'map(select(startswith("Decentraland_"))) | length' <<< "$NAMES")
           INFO_COUNT=$(jq 'map(select(startswith("unity_build_info_"))) | length' <<< "$NAMES")
@@ -308,13 +312,20 @@ jobs:
       # expiry canary — the 2026-06..08 outage was exactly this token expiring
       # with no warning anywhere a human looks.
       - name: Probe performance PAT expiry
+        # Purely decorative — its whole output is one optional sentence, so it
+        # must never gate the dispatched/dispatch-failed section writes below.
         if: steps.perf_dispatch.outcome == 'success'
+        continue-on-error: true
         env:
           PAT: ${{ secrets.PERFORMANCE_TESTING_PAT }}
         run: |
           set -euo pipefail
-          exp=$(curl -sI -H "Authorization: Bearer $PAT" https://api.github.com/repos/decentraland/performance-testing \
-            | tr -d '\r' | grep -i '^github-authentication-token-expiration:' | cut -d' ' -f2- || true)
+          # The expiry header rides every authenticated call; /rate_limit spends
+          # no quota and names no repo that could drift. gh reads the token from
+          # the environment, keeping it out of any process's argv. head -1 keeps
+          # $GITHUB_ENV single-line even if the response ever repeats the header.
+          exp=$(GH_TOKEN="$PAT" timeout 15 gh api --include --method HEAD /rate_limit 2>/dev/null \
+            | tr -d '\r' | grep -i '^github-authentication-token-expiration:' | head -1 | cut -d' ' -f2- || true)
           msg=""
           if [ -n "$exp" ]; then
             exp_s=$(date -d "$exp" +%s 2>/dev/null || echo 0)
@@ -325,6 +336,8 @@ jobs:
                 msg="⚠️ \`PERFORMANCE_TESTING_PAT\` expires in **$days days** ($exp) — rotate it before benchmark dispatches start failing."
               fi
             fi
+          else
+            echo "::warning::No github-authentication-token-expiration header returned — PAT expiry cannot be monitored."
           fi
           echo "PAT_EXPIRY_WARNING=$msg" >> "$GITHUB_ENV"
 
@@ -349,8 +362,12 @@ jobs:
       # cause — it broke silently from 2026-06-15 to 2026-08-13) turns this job
       # red but leaves no trace on the PR; surface it in the performance section
       # so it cannot go unnoticed for weeks again.
+      # outcome != 'success' rather than == 'failure': anything failing between
+      # the checkout and the dispatch (Find latest release, the build-section
+      # upsert) skips perf_dispatch, and a skipped dispatch is the same silence
+      # on the PR as a failed one.
       - name: Mark performance section as dispatch-failed
-        if: failure() && steps.perf_dispatch.outcome == 'failure'
+        if: failure() && steps.perf_dispatch.outcome != 'success'
         uses: ./.github/actions/ci-status-comment
         with:
           pr-number: ${{ needs.pre-validation.outputs.pr-number }}
@@ -359,7 +376,7 @@ jobs:
           body: |-
             [![Performance](https://img.shields.io/badge/Performance-Dispatch%20failed-ff0000?logo=speedtest&logoColor=white&style=for-the-badge)](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }})
 
-            ❌ Could not dispatch the bare-metal benchmark — see the [step log](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}). If it says "Repository not found, OR token has insufficient permissions", the `PERFORMANCE_TESTING_PAT` secret has expired and needs to be rotated.
+            ❌ Could not dispatch the bare-metal benchmark (the job failed before or during the dispatch) — see the [step log](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}). If it says "Repository not found, OR token has insufficient permissions", the `PERFORMANCE_TESTING_PAT` secret has expired and needs to be rotated.
 
   comment-failed:
     needs: [pre-validation, check-build-ran]

.github/workflows/pr-comment-test-failures.yml

@@ -118,19 +118,22 @@ jobs:
 
             # Suite duration + slowest tests come from the same untrusted artifact:
             # duration must be numeric before it reaches awk, and slowest entries are
-            # type-checked in jq with names flattened to a single line.
+            # type-checked in jq with names flattened to a single line. Names render
+            # inside inline code (backticks/pipes stripped) so a name shaped like
+            # markdown — a link, an <img>, a </details> — reads as text instead of
+            # rendering as first-party comment furniture.
             d=$(jq -r '.duration // empty' "$file")
             if [[ "$d" =~ ^[0-9]+(\.[0-9]+)?$ ]]; then dur=$(fmt_secs "$d"); else dur="—"; fi
             slow=$(jq -r --arg mode "$mode" \
-              '.slowest[]? | select((.seconds|type=="number") and (.name|type=="string")) | "- [\($mode)] \(.seconds)s  \(.name | gsub("[\r\n]"; " "))"' \
+              '.slowest[]? | select((.seconds|type=="number") and (.name|type=="string")) | "- [\($mode)] \(.seconds)s  `\(.name | gsub("[\r\n`|]"; " "))`"' \
               "$file" 2>/dev/null || true)
             [ -n "$slow" ] && slowest_list="$slowest_list$slow"$'\n'
 
             if [ "$f" -gt 0 ]; then
               if [ "$status" = "passed" ]; then status=failed; fi
               total_failed=$((total_failed + f))
               rows="$rows| $disp_cell | ❌ $f failed | $p | $f | $s | $dur |"$'\n'
-              names=$(jq -r --arg mode "$mode" '.failed[] | "- [\($mode)] \(. | gsub("[\r\n]"; " "))"' "$file")
+              names=$(jq -r --arg mode "$mode" '.failed[] | "- [\($mode)] `\(. | gsub("[\r\n`|]"; " "))`"' "$file")
               failed_list="$failed_list$names"$'\n'
             else
               rows="$rows| $disp_cell | ✅ Passed | $p | 0 | $s | $dur |"$'\n'

.github/workflows/test.yml

@@ -805,7 +805,7 @@ jobs:
           TEST_MODE: ${{ matrix.testMode }}
         run: |
           python3 - <<'PY'
-          import glob, json, os, xml.etree.ElementTree as ET
+          import glob, json, math, os, xml.etree.ElementTree as ET
 
           artifacts_path = os.environ["ARTIFACTS_PATH"]
           test_mode = os.environ["TEST_MODE"]
@@ -831,6 +831,11 @@ jobs:
                       seconds = float(test_case.get("duration") or 0)
                   except ValueError:
                       seconds = 0.0
+                  # float() admits nan/inf/1e999 without raising, and json.dump
+                  # would then emit bare NaN/Infinity — invalid JSON that aborts
+                  # the consumer's very first jq read of this file.
+                  if not math.isfinite(seconds):
+                      seconds = 0.0
                   duration += seconds
                   timings.append((seconds, test_case.get("fullname") or test_case.get("name")))
                   if case_result == "Passed":

.github/workflows/visual-regression.yml

@@ -171,6 +171,11 @@ jobs:
     needs: resolve
     if: needs.resolve.outputs.authorized == 'true' && needs.resolve.outputs.pr_number != ''
     runs-on: ubuntu-latest
+    # This job only writes the status comment; the workflow-level contents:write
+    # ceiling exists for the reusable suite call, not for it.
+    permissions:
+      contents: read
+      pull-requests: write
     steps:
       - name: Checkout CI status action
         uses: actions/checkout@v6
@@ -215,6 +220,11 @@ jobs:
     needs: [resolve, run-suite]
     if: always() && needs.resolve.outputs.authorized == 'true' && needs.resolve.outputs.pr_number != ''
     runs-on: ubuntu-latest
+    # This job only writes the status comment; the workflow-level contents:write
+    # ceiling exists for the reusable suite call, not for it.
+    permissions:
+      contents: read
+      pull-requests: write
     steps:
       - name: Checkout CI status action
         uses: actions/checkout@v6