Fix detail page error handling and improve loading states

.dockerignore

@@ -1,9 +1,16 @@
 frontend/coverage
 frontend/dist
 frontend/node_modules
+frontend/e2e/test-results
+frontend/e2e/playwright-report
+frontend/e2e/blob-report
+frontend/e2e/.auth
+**/*.tsbuildinfo
 frontend/ssl
 
 **/*.log
 **/*.env
+**/.env.*
+!**/.env.example
 **/.DS_Store
 **/Thumbs.db

.github/scripts/codegen-inputs-changed.sh

@@ -0,0 +1,57 @@
+#!/usr/bin/env bash
+# Decide whether a run has to re-check that frontend/src/graphql/types.ts is fresh.
+# Echoes `changed=true|false`. Split out of ci.yml so the range selection is testable
+# (frontend/e2e/ci-codegen-gate.unit.test.ts drives it over real git fixtures).
+#
+# Usage: codegen-inputs-changed.sh <event_name> <before_sha> <base_sha> <head_sha>
+set -uo pipefail
+
+EVENT="${1:-}"
+BEFORE="${2:-}"
+BASE_SHA="${3:-}"
+HEAD_SHA="${4:-}"
+
+# On pull_request, `before` is the PR's PREVIOUS HEAD, so a follow-up push diffs a range
+# that no longer contains the commit that edited the schema — the gate would skip itself
+# on every push after the first. The PR's base is the range that always spans the change.
+if [ "$EVENT" = "pull_request" ]; then
+    base="$BASE_SHA"
+else
+    base="$BEFORE"
+fi
+
+# When the range can't be resolved (new branch, force-push, tag), check anyway.
+# stdout is consumed verbatim as a workflow output, so diagnostics go to stderr.
+if [ -z "$base" ] || [ "$base" = "0000000000000000000000000000000000000000" ] \
+    || ! git cat-file -e "$base^{commit}" 2>/dev/null; then
+    echo "reason=unresolvable-base" >&2
+    echo "changed=true"
+    exit 0
+fi
+
+# The generated file is in the list too: a push that edits only it would otherwise skip
+# the check, and the drift surfaces later on someone else's unrelated codegen push.
+INPUTS=(
+    backend/pkg/graph/schema.graphqls
+    frontend/graphql-schema.graphql
+    frontend/graphql-codegen.ts
+    frontend/pnpm-lock.yaml
+    frontend/src/graphql/types.ts
+)
+
+if ! files=$(git diff --name-only "$base" "$HEAD_SHA"); then
+    echo "reason=diff-failed" >&2
+    echo "changed=true"
+    exit 0
+fi
+
+for input in "${INPUTS[@]}"; do
+    case $'\n'"$files"$'\n' in
+        *$'\n'"$input"$'\n'*)
+            echo "changed=true"
+            exit 0
+            ;;
+    esac
+done
+
+echo "changed=false"

.github/workflows/ci.yml

@@ -1,13 +1,23 @@
 name: Docker build and push
 
 on:
+  # pull_request so the lint/codegen/test gate runs on fork PRs too: a fork's commits never push to
+  # this repo, so a push-only trigger let a fork edit the schema without regenerating types.ts (or
+  # ship a lint/test failure) and merge green, since e2e.yml's tsc build cannot see stale codegen.
+  # lint-and-test uses no secrets and is fork-safe; docker-build stays gated to main/tags below.
+  pull_request:
+    types: [opened, synchronize, reopened]
   push:
     branches:
-      - "**"
+      - main
     tags:
       - "v[0-9]+.[0-9]+.[0-9]+"
   workflow_dispatch:
 
+concurrency:
+  group: ci-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
 permissions:
   contents: read
 
@@ -32,7 +42,7 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v6
         with:
-          go-version: '1.24'
+          go-version: "1.24"
           cache: true
           cache-dependency-path: backend/go.sum
 
@@ -47,8 +57,8 @@ jobs:
         uses: actions/setup-node@v6
         with:
           node-version-file: frontend/.nvmrc
-          cache: 'pnpm'
-          cache-dependency-path: 'frontend/pnpm-lock.yaml'
+          cache: "pnpm"
+          cache-dependency-path: "frontend/pnpm-lock.yaml"
 
       # Frontend lint and test
       - name: Frontend - Install dependencies
@@ -70,21 +80,14 @@ jobs:
       # Only when a codegen input moved. The lockfile is included on purpose: a codegen
       # version bump can change the output, and skipping it there would let types.ts go
       # stale and fail the next schema push for something its author did not do.
-      # When the range can't be resolved (new branch, force-push, tag), check anyway.
       - name: Frontend - Detect GraphQL codegen input changes
         id: codegen-inputs
         run: |
-          base='${{ github.event.before }}'
-          if [ -z "$base" ] || [ "$base" = "0000000000000000000000000000000000000000" ] \
-             || ! git cat-file -e "$base^{commit}" 2>/dev/null; then
-            echo "changed=true" >> "$GITHUB_OUTPUT"
-            exit 0
-          fi
-          if git diff --name-only "$base" "${{ github.sha }}" | grep -qE '^(backend/pkg/graph/schema\.graphqls|frontend/graphql-schema\.graphql|frontend/graphql-codegen\.ts|frontend/pnpm-lock\.yaml)$'; then
-            echo "changed=true" >> "$GITHUB_OUTPUT"
-          else
-            echo "changed=false" >> "$GITHUB_OUTPUT"
-          fi
+          .github/scripts/codegen-inputs-changed.sh \
+            '${{ github.event_name }}' \
+            '${{ github.event.before }}' \
+            '${{ github.event.pull_request.base.sha }}' \
+            '${{ github.sha }}' >> "$GITHUB_OUTPUT"
 
       # The app ships the operations compiled into src/graphql/types.ts; regenerate and
       # diff so it can't drift from graphql-schema.graphql (the codegen input the stand's
@@ -133,17 +136,17 @@ jobs:
           PACKAGE_VER=${LATEST_TAG#v}
           CURRENT_COMMIT=$(git rev-parse HEAD)
           TAG_COMMIT=$(git rev-list -n 1 "$LATEST_TAG" 2>/dev/null || echo "")
-          
+
           if [ "$CURRENT_COMMIT" != "$TAG_COMMIT" ]; then
             PACKAGE_REV=$(git rev-parse --short HEAD)
           else
             PACKAGE_REV=""
           fi
-          
+
           LDFLAGS="-X pentagi/pkg/version.PackageName=pentagi -X pentagi/pkg/version.PackageVer=${PACKAGE_VER} -X pentagi/pkg/version.PackageRev=${PACKAGE_REV}"
-          
+
           echo "Building with version: ${PACKAGE_VER}${PACKAGE_REV:+-$PACKAGE_REV}"
-          
+
           # Build for AMD64
           GOOS=linux GOARCH=amd64 go build -trimpath -ldflags "$LDFLAGS" -o /tmp/pentagi-amd64 ./cmd/pentagi
           echo "✓ Successfully built for linux/amd64"
@@ -179,13 +182,13 @@ jobs:
           # Get latest tag version (without 'v' prefix)
           LATEST_TAG=$(git describe --tags --abbrev=0 2>/dev/null || echo "v0.0.0")
           VERSION=${LATEST_TAG#v}
-          
+
           # Get current commit hash
           CURRENT_COMMIT=$(git rev-parse HEAD)
-          
+
           # Get commit hash of the latest tag
           TAG_COMMIT=$(git rev-list -n 1 "$LATEST_TAG" 2>/dev/null || echo "")
-          
+
           # Set revision only if current commit differs from tag commit
           if [ "$CURRENT_COMMIT" != "$TAG_COMMIT" ]; then
             PACKAGE_REV=$(git rev-parse --short HEAD)
@@ -205,7 +208,7 @@ jobs:
             echo "patch=${VERSION}" >> $GITHUB_OUTPUT
             echo "  Docker tags: latest, ${major}, ${major}.${minor}, ${VERSION}"
           fi
-          
+
           echo "version=${VERSION}" >> $GITHUB_OUTPUT
 
       - name: Generate Docker metadata

.github/workflows/e2e-report.yml

@@ -28,6 +28,7 @@ jobs:
     timeout-minutes: 5
     steps:
       - name: Download results
+        id: results
         continue-on-error: true
         uses: actions/download-artifact@v4
         with:
@@ -55,15 +56,25 @@ jobs:
             const fs = require('fs');
             const run = context.payload.workflow_run;
 
-            // workflow_run.pull_requests is empty for fork PRs — resolve by head SHA.
-            let prNumber = run.pull_requests[0]?.number;
+            // Match the PR whose head is still this run's — one commit can belong to several PRs, so
+            // taking the first association (either source) could hand the report to a stale or
+            // foreign one. Same filter on both paths: workflow_run.pull_requests (empty for forks)
+            // and the head-SHA lookup that covers them.
+            // workflow_run.pull_requests carries head.repo.name but no full_name, unlike the API
+            // lookup — enforce the repo only when it is present, so the primary path still matches.
+            const isThisRun = (pr) =>
+              pr.head?.sha === run.head_sha &&
+              pr.head?.ref === run.head_branch &&
+              (pr.head?.repo?.full_name == null || pr.head.repo.full_name === run.head_repository?.full_name);
+
+            let prNumber = run.pull_requests.find(isThisRun)?.number;
             if (!prNumber) {
               const { data: prs } = await github.rest.repos.listPullRequestsAssociatedWithCommit({
                 owner: context.repo.owner,
                 repo: context.repo.repo,
                 commit_sha: run.head_sha,
               });
-              prNumber = prs[0]?.number;
+              prNumber = prs.find(isThisRun)?.number;
             }
             if (!prNumber) {
               core.info(`No PR found for ${run.head_sha}; skipping comment.`);
@@ -94,32 +105,55 @@ jobs:
               return;
             }
 
-            let statsLine = 'The run produced no results file (it likely failed before the tests started).';
+            // e2e.yml uploads results.json with `if-no-files-found: ignore`, so a run that never
+            // wrote one publishes no artifact at all: a failed download means the file is absent,
+            // a successful download with unreadable stats means it is corrupt.
+            const resultsDownloaded = '${{ steps.results.outcome }}' === 'success';
+
+            let statsLine = resultsDownloaded
+              ? 'The results file could not be parsed, so no test counts are available.'
+              : 'No results artifact was produced for this run.';
+            let statsRead = false;
             let noFailures = false;
             try {
               const report = JSON.parse(fs.readFileSync('results/results.json', 'utf8'));
               const n = (value) => Number(value) || 0;
               const { expected, unexpected, flaky, skipped } = report.stats ?? {};
               statsLine = `**${n(expected)} passed**, ${n(unexpected)} failed, ${n(flaky)} flaky, ${n(skipped)} skipped.`;
               noFailures = n(unexpected) === 0;
+              statsRead = true;
             } catch (error) {
               core.info(`results.json unavailable: ${error}`);
             }
 
-            // Both halves are required: results.json is written before an abort can be
-            // observed, so a run killed by the global timeout still reports zero failures.
-            // When the jobs API was unreachable, fall back to results.json alone.
-            const mockPassed = noFailures && (jobsFetched ? mockJob?.conclusion === 'success' : true);
+            // The job conclusion is load-bearing, not a nicety: results.json is written before a
+            // global-timeout abort can be observed (its un-run tests land in `skipped`, so
+            // `unexpected` stays 0), and only the job's own conclusion reflects the abort. So a
+            // green needs BOTH zero failures AND a successful job; whenever either source is
+            // unreadable the verdict is unknown — never a green, and never a red.
+            const jobFailed = jobsFetched && mockJob?.conclusion !== 'success';
+            const mockStatus = jobFailed || (statsRead && !noFailures)
+              ? 'fail'
+              : statsRead && jobsFetched
+                ? 'pass'
+                : 'unknown';
 
             // Key the advisory on the e2e-visual-diffs artifact (uploaded only when the snapshot
             // step itself failed), not on the visual job's conclusion — a tag-guard, install or
             // build failure would otherwise read as "snapshots differ".
             const visualDiffers = '${{ steps.visualdiffs.outcome }}' === 'success';
 
-            const conclusion = !mockPassed ? '❌' : visualDiffers ? '⚠️' : '✅';
+            const conclusion = mockStatus === 'fail' ? '❌' : mockStatus === 'unknown' || visualDiffers ? '⚠️' : '✅';
             const marker = '<!-- e2e-report -->';
-            const advisoryLine = visualDiffers
-              ? '⚠️ **Visual snapshots differ** — advisory only, does not block merge. Download the `e2e-visual-diffs` artifact to compare expected / actual / diff.'
+            const unconfirmedReason = !jobsFetched && !statsRead
+              ? 'neither the workflow jobs API nor `results.json` could be read'
+              : !jobsFetched
+              ? 'the workflow jobs API was unreachable, so this reflects `results.json` alone, which cannot distinguish a clean pass from a run killed mid-flight'
+              : '`results.json` could not be read, so this reflects the job conclusion alone and no test counts were checked';
+            const advisoryLine = mockStatus === 'unknown'
+              ? `⚠️ **Result unconfirmed** — ${unconfirmedReason}. Check the run before merging.`
+              : visualDiffers
+              ? '⚠️ **Visual snapshots differ** — advisory only, does not block merge. Download the `e2e-visual-diffs` artifact to compare expected / actual / diff (a missing baseline uploads actual only).'
               : null;
             const body = [
               marker,

.github/workflows/e2e-stand.yml

@@ -10,27 +10,26 @@ on:
   pull_request:
     types: [labeled]
   workflow_dispatch:
-    inputs:
-      tier:
-        description: "Run the stand tier"
-        type: choice
-        options:
-          - stand
-          - all
-        default: stand
 
 permissions:
   contents: read
 
+# Keyed on the label too: concurrency is evaluated for the whole run before the job's `if`, so a run
+# started by an unrelated label would share this group and cancel an approved stand run mid-flight.
 concurrency:
-  group: e2e-stand-${{ github.event.pull_request.number || github.run_id }}
+  group: e2e-stand-${{ github.event.pull_request.number || github.run_id }}-${{ github.event.label.name }}
   cancel-in-progress: ${{ github.event_name == 'pull_request' }}
 
 jobs:
   e2e-stand:
+    # GitHub withholds secrets from fork `pull_request` runs: without the fork check a labelled fork
+    # PR holds reviewers for an environment approval and then fails on empty credentials. Assert repo
+    # identity rather than negating `head.repo.fork`: a deleted fork sends `head.repo: null`, and
+    # GitHub coerces null and false alike to 0, so the negation lets that PR through.
     if: >
       github.event_name == 'workflow_dispatch' ||
-      contains(github.event.pull_request.labels.*.name, 'e2e:stand')
+      (github.event.label.name == 'e2e:stand' &&
+       github.event.pull_request.head.repo.full_name == github.repository)
     runs-on: ubuntu-latest
     timeout-minutes: 30
     environment: e2e-stand
@@ -78,11 +77,41 @@ jobs:
           E2E_USER: ${{ secrets.E2E_STAND_USER }}
           E2E_PASSWORD: ${{ secrets.E2E_STAND_PASSWORD }}
 
-      # Traces, videos and screenshots are all disabled on this tier — they would
-      # embed the live session cookie, the stand URL or the login into a
-      # public-repo artifact. results.json carries none of those.
-      - name: Upload report
+      # Traces, videos and screenshots are disabled on this tier. results.json still
+      # embeds the stand URL (baseURL in navigation/locator error messages) and the
+      # user (in locator text) on a failing run, so redact the secrets before the
+      # public-repo upload — GitHub masks secrets in logs, never in artifacts.
+      - name: Redact secrets from report
+        id: redact
         if: always()
+        working-directory: frontend
+        run: |
+          node -e '
+            const fs = require("fs");
+            const f = "e2e/test-results/results.json";
+            if (!fs.existsSync(f)) process.exit(0);
+            let s = fs.readFileSync(f, "utf8");
+            // Derived forms too, not just the raw value: auth.setup.ts builds a locator name from
+            // the regex-escaped user, so a failed login writes `user@host\.com` into the error —
+            // and since this redacts the JSON text, that backslash is itself encoded as `\\`.
+            const regexEscaped = (v) => v.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
+            const jsonEncoded = (v) => JSON.stringify(v).slice(1, -1);
+            for (const v of [process.env.E2E_BASE_URL, process.env.E2E_USER, process.env.E2E_PASSWORD]) {
+              if (!v) continue;
+              const forms = [v, regexEscaped(v), encodeURIComponent(v)];
+              for (const form of new Set([...forms, ...forms.map(jsonEncoded)])) {
+                s = s.split(form).join("<redacted>");
+              }
+            }
+            fs.writeFileSync(f, s);
+          '
+        env:
+          E2E_BASE_URL: ${{ secrets.E2E_STAND_URL }}
+          E2E_USER: ${{ secrets.E2E_STAND_USER }}
+          E2E_PASSWORD: ${{ secrets.E2E_STAND_PASSWORD }}
+
+      - name: Upload report
+        if: always() && steps.redact.outcome == 'success'
         uses: actions/upload-artifact@v4
         with:
           name: e2e-stand-report