ci(prs): separate validation readiness from description checks (#5939)

* ci(prs): separate validation readiness from description checks

* fix(ci): harden PR readiness state

---------

Co-authored-by: Alexandre Teixeira <alexandremagteixeira@gmail.com>

.env.example

@@ -159,6 +159,21 @@ SEARXNG_INSTANCE=http://localhost:8080
 # Local HTTP setups may use the callback URL inferred by the application.
 # GOOGLE_OAUTH_REDIRECT_URI=https://your-domain.com/api/email/oauth/google/callback
 
+# Origin the MCP OAuth callback is sent back to, for remote (Streamable HTTP)
+# MCP servers that register it dynamically. Defaults to http://localhost:$APP_PORT,
+# which is right only when you reach Odysseus directly on that port. Set it for
+# HTTPS, reverse-proxy, hosted, and Docker installs — inside the container the
+# app always listens on 7000 and cannot see the host port map, so the default is
+# wrong there whenever APP_PORT is not 7000.
+#
+# Not for Google MCP servers. Those use Desktop App credentials, and Google only
+# accepts loopback redirect URIs for that client type, so a public origin here is
+# rejected with redirect_uri_mismatch. Leave it unset for a Google-only install:
+# the loopback default is what Google wants, and remote users finish through the
+# paste-back page, which never has to load the redirect.
+# https://developers.google.com/identity/protocols/oauth2/native-app
+# OAUTH_REDIRECT_BASE_URL=https://your-domain.com
+
 # ============================================================
 # Misc
 # ============================================================

.github/pull_request_template.md

@@ -28,6 +28,7 @@ Fixes #
 - [ ] This PR targets `dev`
 - [ ] My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
 - [ ] I actually ran the app (`docker compose up` or `uvicorn app:app`) and verified the change works end-to-end. Type-checks and unit tests are not enough.
+- [ ] I did not run the app/runtime validation and stated that gap in **How to Test**. Leave this unchecked when the app-run box above is checked.
 
 ## How to Test
 

.github/scripts/check-pr-description.js

@@ -21,11 +21,11 @@ module.exports = async ({ github, context, core }) => {
     return strip(m?.[0].replace(new RegExp(`#+\\s+${heading}`, 'i'), '') ?? '');
   }
 
-  const problems = [];
+  const descriptionProblems = [];
 
   // 1. Summary must be filled in.
   if (section('Summary').length < 20) {
-    problems.push('**Summary** is empty or too short — describe what changed and why.');
+    descriptionProblems.push('**Summary** is empty or too short — describe what changed and why.');
   }
 
   // 2. Linked Issue must reference a real issue. Accept a bare #NNN, a closing
@@ -34,18 +34,18 @@ module.exports = async ({ github, context, core }) => {
   const linkedSection = section('Linked Issue');
   const hasIssueRef = /#\d+\b/.test(linkedSection) || /\/issues\/\d+/.test(linkedSection);
   if (!linkedSection || !hasIssueRef) {
-    problems.push('**Linked Issue** — add a reference like `Fixes #NNN`, a bare `#NNN`, or a link to the issue.');
+    descriptionProblems.push('**Linked Issue** — add a reference like `Fixes #NNN`, a bare `#NNN`, or a link to the issue.');
   }
 
   // 3. At least one Type of Change box must be checked.
   const typeBlock = body.match(/##\s+Type of Change[\s\S]*?(?=\n##\s|$)/i)?.[0] ?? '';
   if (!/- \[x\]/i.test(typeBlock)) {
-    problems.push('**Type of Change** — check at least one box.');
+    descriptionProblems.push('**Type of Change** — check at least one box.');
   }
 
   // 4. Duplicate-search checklist item must be checked.
   if (!/- \[x\] I searched/i.test(body)) {
-    problems.push('**Checklist** — check the duplicate-search box to confirm you searched existing issues and PRs.');
+    descriptionProblems.push('**Checklist** — check the duplicate-search box to confirm you searched existing issues and PRs.');
   }
 
   // 5. How to Test must contain enough real detail for a reviewer to act on.
@@ -53,7 +53,79 @@ module.exports = async ({ github, context, core }) => {
   //    code block — so we only require non-trivial content, not a specific shape.
   const howTo = section('How to Test');
   if (howTo.length < 30) {
-    problems.push('**How to Test** — explain how a reviewer can verify this change. Numbered steps, the commands you ran, or a short code block all work — give a sentence or two of real detail (not just "tested locally").');
+    descriptionProblems.push('**How to Test** — explain how a reviewer can verify this change. Numbered steps, the commands you ran, or a short code block all work — give a sentence or two of real detail (not just "tested locally").');
+  }
+
+  // Classify paths from GitHub's API. This workflow runs in the privileged base
+  // context, so it must never check out or execute code from the PR branch.
+  const changedFiles = await github.paginate(github.rest.pulls.listFiles, {
+    owner, repo, pull_number: prNum, per_page: 100,
+  });
+  const changedPaths = changedFiles.map(file => file.filename);
+
+  function isUiSensitivePath(filename) {
+    const path = filename.toLowerCase();
+    return path.startsWith('static/')
+      || path.startsWith('templates/')
+      || /\.(?:html?|css|svg)$/.test(path);
+  }
+
+  function isDocsOnlyPath(filename) {
+    const path = filename.toLowerCase();
+    return /\.(?:md|mdx|rst|adoc|txt)$/.test(path)
+      || (path.startsWith('docs/') && !isUiSensitivePath(path));
+  }
+
+  function isRuntimeSensitivePath(filename) {
+    const path = filename.toLowerCase();
+    if (isUiSensitivePath(path)) return false;
+    if (path.startsWith('tests/') || path.startsWith('.github/')) return false;
+    return /^(?:app\.py|routes\/|services\/|src\/|core\/|mcp_servers\/|scripts\/|docker\/)/.test(path)
+      || /^(?:dockerfile|docker-compose.*\.ya?ml|requirements(?:-optional)?\.txt|pyproject\.toml|setup\.py)$/.test(path)
+      || /\.(?:py|sh|ps1|bat)$/.test(path);
+  }
+
+  let classification = 'tooling';
+  if (changedPaths.some(isUiSensitivePath)) {
+    classification = 'UI-sensitive';
+  } else if (changedPaths.some(isRuntimeSensitivePath)) {
+    classification = 'backend/runtime';
+  } else if (changedPaths.length > 0 && changedPaths.every(isDocsOnlyPath)) {
+    classification = 'docs-only';
+  }
+
+  const appRan = /- \[x\]\s+I actually ran the app\b/i.test(body);
+  const appNotRun = /- \[x\]\s+I did not run the app\/runtime validation\b/i.test(body);
+  const screenshotChecked = /- \[x\]\s+\*\*Screenshot or short clip\*\*/i.test(body);
+  const screenshotSection = section('Screenshots / clips');
+  const hasVisualEvidence = /!\[[^\]]*\]\([^)]+\)|<(?:img|video|source)\b[^>]*(?:src|href)=|https?:\/\/[^\s)]+/i.test(screenshotSection);
+  const evidenceGaps = [];
+  let needsRuntimeValidation = false;
+  let needsVisualEvidence = false;
+
+  if (classification === 'backend/runtime' || classification === 'UI-sensitive') {
+    if (appRan && appNotRun) {
+      needsRuntimeValidation = true;
+      evidenceGaps.push('The app-run and explicit not-run boxes are both checked. Select the one state that is true.');
+    } else if (!appRan) {
+      needsRuntimeValidation = true;
+      if (appNotRun) {
+        evidenceGaps.push('The author explicitly reports that app/runtime validation was not performed.');
+      } else {
+        evidenceGaps.push('App/runtime validation is not author-attested. Check the run box only after running it, or check the explicit not-run box and describe the gap.');
+      }
+    }
+  }
+
+  if (classification === 'UI-sensitive') {
+    if (!screenshotChecked) {
+      needsVisualEvidence = true;
+      evidenceGaps.push('The screenshot/clip checkbox is not checked for this UI-sensitive change.');
+    }
+    if (!hasVisualEvidence) {
+      needsVisualEvidence = true;
+      evidenceGaps.push('The Screenshots / clips section does not contain an actual attachment or link.');
+    }
   }
 
   // ── Comment ──────────────────────────────────────────────────────────────
@@ -62,22 +134,43 @@ module.exports = async ({ github, context, core }) => {
   });
   const existing = comments.find(c => (c.body ?? '').includes(MARKER));
 
-  if (problems.length === 0) {
+  if (descriptionProblems.length === 0 && evidenceGaps.length === 0) {
     if (existing) {
       await github.rest.issues.deleteComment({ owner, repo, comment_id: existing.id });
     }
   } else {
-    const commentBody = [
-      MARKER,
-      '⚠️ **PR description — action needed**',
-      '',
-      'The following required sections are missing or incomplete. Please update the PR description to address them:',
-      '',
-      problems.map(p => `- ${p}`).join('\n'),
+    const commentLines = [MARKER];
+    if (descriptionProblems.length > 0) {
+      commentLines.push(
+        '⚠️ **PR description — action needed**',
+        '',
+        'The following required sections are missing or incomplete. Please update the PR description to address them:',
+        '',
+        descriptionProblems.map(problem => `- ${problem}`).join('\n'),
+      );
+    } else {
+      commentLines.push(
+        '⚠️ **PR description is complete; validation evidence is still outstanding**',
+        '',
+        `Changed-file classification: **${classification}**.`,
+      );
+    }
+    if (evidenceGaps.length > 0) {
+      commentLines.push(
+        '',
+        '**Author-reported runtime / visual state**',
+        '',
+        evidenceGaps.map(gap => `- ${gap}`).join('\n'),
+        '',
+        'Checkboxes are author attestations. GitHub Actions results remain the execution evidence for CI; this check does not prove that a local command ran.',
+      );
+    }
+    commentLines.push(
       '',
       '---',
-      '_This comment is deleted automatically once all sections are complete._',
-    ].join('\n');
+      '_This comment updates automatically when the description or changed files change._',
+    );
+    const commentBody = commentLines.join('\n');
 
     if (existing) {
       await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body: commentBody });
@@ -97,34 +190,47 @@ module.exports = async ({ github, context, core }) => {
       return true;
     } catch (e) {
       if (e.status === 404) return false;
+      if (e.status === 403) {
+        core.warning(`Could not inspect label "${name}" — token lacks label read access; skipping.`);
+        return false;
+      }
       throw e;
     }
   }
 
-  async function swapLabel(num, add, remove) {
-    if (await labelExists(add)) {
+  async function setLabel(name, wanted) {
+    if (wanted && await labelExists(name)) {
       try {
-        await github.rest.issues.addLabels({ owner, repo, issue_number: num, labels: [add] });
+        await github.rest.issues.addLabels({ owner, repo, issue_number: prNum, labels: [name] });
       } catch (e) {
         // Fail soft on a token that can't write labels so a label permission
         // problem never masks the actual description verdict.
-        if (e.status !== 403) throw e;
-        core.warning(`Could not add "${add}" — token lacks label write here; skipping.`);
+        if (e.status !== 403 && e.status !== 404) throw e;
+        core.warning(`Could not add "${name}" — label is unavailable or the token lacks label write access; skipping.`);
       }
+    } else if (wanted) {
+      core.warning(`Label "${name}" does not exist in the repo — skipping. Create it once to enable labelling.`);
     } else {
-      core.warning(`Label "${add}" does not exist in the repo — skipping. Create it once to enable labelling.`);
-    }
-    try {
-      await github.rest.issues.removeLabel({ owner, repo, issue_number: num, name: remove });
-    } catch (e) {
-      if (e.status !== 404 && e.status !== 410 && e.status !== 403) throw e;
+      try {
+        await github.rest.issues.removeLabel({ owner, repo, issue_number: prNum, name });
+      } catch (e) {
+        if (e.status !== 404 && e.status !== 410 && e.status !== 403) throw e;
+      }
     }
   }
 
-  if (problems.length === 0) {
-    await swapLabel(prNum, 'ready for review', 'needs work');
-  } else {
-    await swapLabel(prNum, 'needs work', 'ready for review');
-    core.setFailed(`PR description has ${problems.length} issue(s) — see bot comment for details.`);
+  const descriptionComplete = descriptionProblems.length === 0;
+  const evidenceComplete = evidenceGaps.length === 0;
+  const isDraft = Boolean(context.payload.pull_request.draft);
+  await setLabel(
+    'ready for review',
+    descriptionComplete && evidenceComplete && !isDraft,
+  );
+  await setLabel('needs work', !descriptionComplete);
+  await setLabel('needs runtime validation', needsRuntimeValidation);
+  await setLabel('needs visual evidence', needsVisualEvidence);
+
+  if (!descriptionComplete) {
+    core.setFailed(`PR description has ${descriptionProblems.length} issue(s) — see bot comment for details.`);
   }
 };

.github/workflows/pr-description-check.yml

@@ -5,7 +5,11 @@ on:
   # works on fork PRs. Safe here: the checkout pins to the base branch (no fork
   # code runs) and the scripts only read context.payload and call the GitHub API.
   pull_request_target:  # zizmor: ignore[dangerous-triggers]
-    types: [opened, edited, synchronize, reopened, ready_for_review]
+    types: [opened, edited, synchronize, reopened, ready_for_review, converted_to_draft]
+
+concurrency:
+  group: pr-description-${{ github.event.pull_request.number }}
+  cancel-in-progress: true
 
 # Default-deny at the workflow level; each job opts into only the scopes it needs.
 # Note: modifying a PR's labels/comments needs pull-requests:write even though the
@@ -59,12 +63,14 @@ jobs:
 
   check-mergeable:
     name: Flag unmergeable PRs
+    needs: check-description
     runs-on: ubuntu-latest
     permissions:
       pull-requests: write
       issues: write
-    # Skip bots: they open PRs programmatically and have their own process.
-    if: github.event.pull_request.user.type != 'Bot'
+    # Run after description validation failures, but never from an obsolete
+    # workflow run canceled by a newer PR event.
+    if: ${{ !cancelled() && github.event.pull_request.user.type != 'Bot' }}
     steps:
       - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3  # v9.0.0
         with:

build-macos-app.sh

@@ -73,6 +73,10 @@ cat > "$APP/Contents/MacOS/$APP_NAME.tmpl" <<'LAUNCHER'
 INSTALL_DIR="__INSTALL_DIR__"
 PORT="__PORT__"
 URL="http://127.0.0.1:${PORT}"
+# uvicorn is started with --port below, but APP_PORT is what the app itself
+# reads when it needs to build a URL for this instance (internal_api_base(),
+# companion pairing, the MCP OAuth callback), so export it as well.
+export APP_PORT="$PORT"
 export PATH="/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:$PATH"
 
 UVICORN="$INSTALL_DIR/venv/bin/uvicorn"

docker-compose.gpu-amd.yml

@@ -75,6 +75,11 @@ services:
       - GOOGLE_OAUTH_CLIENT_ID=${GOOGLE_OAUTH_CLIENT_ID:-}
       - GOOGLE_OAUTH_CLIENT_SECRET=${GOOGLE_OAUTH_CLIENT_SECRET:-}
       - GOOGLE_OAUTH_REDIRECT_URI=${GOOGLE_OAUTH_REDIRECT_URI:-}
+      # Externally reachable origin for MCP OAuth callbacks. The container
+      # always listens on 7000 and cannot see the host port map above, so
+      # remote MCP OAuth needs this set whenever the browser reaches
+      # Odysseus on anything other than http://localhost:7000.
+      - OAUTH_REDIRECT_BASE_URL=${OAUTH_REDIRECT_BASE_URL:-}
       - TAVILY_API_KEY=${TAVILY_API_KEY:-}
       - SERPER_API_KEY=${SERPER_API_KEY:-}
       # PUID / PGID — the user/group the container drops to before

docker-compose.gpu-nvidia.yml

@@ -74,6 +74,11 @@ services:
       - GOOGLE_OAUTH_CLIENT_ID=${GOOGLE_OAUTH_CLIENT_ID:-}
       - GOOGLE_OAUTH_CLIENT_SECRET=${GOOGLE_OAUTH_CLIENT_SECRET:-}
       - GOOGLE_OAUTH_REDIRECT_URI=${GOOGLE_OAUTH_REDIRECT_URI:-}
+      # Externally reachable origin for MCP OAuth callbacks. The container
+      # always listens on 7000 and cannot see the host port map above, so
+      # remote MCP OAuth needs this set whenever the browser reaches
+      # Odysseus on anything other than http://localhost:7000.
+      - OAUTH_REDIRECT_BASE_URL=${OAUTH_REDIRECT_BASE_URL:-}
       - TAVILY_API_KEY=${TAVILY_API_KEY:-}
       - SERPER_API_KEY=${SERPER_API_KEY:-}
       # PUID / PGID — the user/group the container drops to before

docker-compose.yml

@@ -63,6 +63,11 @@ services:
       - GOOGLE_OAUTH_CLIENT_ID=${GOOGLE_OAUTH_CLIENT_ID:-}
       - GOOGLE_OAUTH_CLIENT_SECRET=${GOOGLE_OAUTH_CLIENT_SECRET:-}
       - GOOGLE_OAUTH_REDIRECT_URI=${GOOGLE_OAUTH_REDIRECT_URI:-}
+      # Externally reachable origin for MCP OAuth callbacks. The container
+      # always listens on 7000 and cannot see the host port map above, so
+      # remote MCP OAuth needs this set whenever the browser reaches
+      # Odysseus on anything other than http://localhost:7000.
+      - OAUTH_REDIRECT_BASE_URL=${OAUTH_REDIRECT_BASE_URL:-}
       - TAVILY_API_KEY=${TAVILY_API_KEY:-}
       - SERPER_API_KEY=${SERPER_API_KEY:-}
       # PUID / PGID — the user/group the container drops to before

launch-windows.ps1

@@ -163,6 +163,10 @@ if (Test-Path $cudaBase) {
 }
 
 # 7. Start the server (use `python -m uvicorn` - bare `uvicorn` may not be on PATH)
+# -Port only reaches uvicorn as a flag. Everything that builds a URL for this
+# instance - internal_api_base(), companion pairing, the MCP OAuth callback -
+# reads APP_PORT, so set it too or they all assume 7000.
+$env:APP_PORT = $Port
 Write-Step ("Starting Odysseus at http://{0}:{1}" -f $BindHost, $Port)
 Write-Host "Press Ctrl+C to stop."
 Write-Host ""

routes/mcp/mcp_routes.py

@@ -475,7 +475,7 @@ def setup_mcp_routes(mcp_manager: McpManager):
                 return RedirectResponse(auth_url)
             else:
                 # Remote device — show paste-back page
-                return HTMLResponse(_oauth_authorize_page(auth_url, server_id, host, redirect_uri))
+                return HTMLResponse(_oauth_authorize_page(auth_url, server_id, redirect_uri))
         finally:
             db.close()
 
@@ -612,15 +612,13 @@ def setup_mcp_routes(mcp_manager: McpManager):
 def _oauth_authorize_page(
     auth_url: str,
     server_id: str,
-    host: str,
-    redirect_uri: str = "http://localhost:7000/api/mcp/oauth/callback",
+    redirect_uri: str,
 ) -> str:
     """Page with Google sign-in link and URL paste-back form for remote access."""
-    # Escape values interpolated into the page: `host` comes from the request
-    # Host header and `server_id` from the OAuth state — neither is trusted.
+    # Escape values interpolated into the page: `server_id` comes from the OAuth
+    # state and is not trusted.
     auth_url = html.escape(auth_url, quote=True)
     server_id = html.escape(server_id, quote=True)
-    host = html.escape(host, quote=True)
     redirect_uri = html.escape(redirect_uri, quote=True)
     return f"""<!DOCTYPE html>
 <html><head>
@@ -664,7 +662,15 @@ def _oauth_authorize_page(
   </div>
   <a class="auth-link" href="{auth_url}" target="_blank" rel="noopener">Sign in with Google</a>
   <div class="divider"></div>
-  <form method="POST" action="http://{host}/api/mcp/oauth/exchange/{server_id}">
+  <!-- Relative action: the browser resolves it against the origin this page was
+       served from, so the form follows the user through any proxy without the
+       app having to know the scheme or the host. An absolute http:// action is
+       blocked as mixed content on exactly the HTTPS deployments that need
+       paste-back, and request.url.scheme cannot be trusted to spot them —
+       uvicorn only honours X-Forwarded-Proto from a peer in
+       --forwarded-allow-ips, which defaults to 127.0.0.1 and excludes a proxy
+       arriving over the Docker bridge. -->
+  <form method="POST" action="/api/mcp/oauth/exchange/{server_id}">
     <p>Paste the URL from your browser after signing in:</p>
     <input type="text" name="callback_url" placeholder="{redirect_uri}?code=..." required>
     <br><button type="submit">Connect</button>

src/mcp_oauth.py

@@ -15,18 +15,32 @@ from urllib.parse import urlparse, parse_qs
 
 logger = logging.getLogger(__name__)
 
+
+def _resolve_redirect_base() -> str:
+    """Origin the browser is sent back to after authorizing.
+
+    Falls back to the port the app binds natively (APP_PORT, read the same way
+    by app.py and launcher.py) rather than a fixed 7000: the macOS launcher
+    defaults to 7860, and a callback on the wrong port reaches nothing. The
+    hostname stays `localhost` rather than internal_api_base()'s 127.0.0.1 —
+    this URI is registered with the authorization server (via DCR, or by hand
+    for Google clients), so changing the host invalidates registrations that
+    already exist.
+    """
+    return (
+        os.environ.get("OAUTH_REDIRECT_BASE_URL")
+        or os.environ.get("APP_PUBLIC_URL")
+        or f"http://localhost:{os.environ.get('APP_PORT', '7000')}"
+    ).rstrip("/")
+
+
 # OAuth redirect URI registered with every authorization server via DCR. Loopback
 # is allowed for native/desktop clients (RFC 8252); remote users finish via the
-# paste-back flow. Deployments not reachable at http://localhost:7000 (custom
-# port, reverse proxy, or public domain) must set OAUTH_REDIRECT_BASE_URL (or
-# APP_PUBLIC_URL) to their externally reachable origin so the redirect lands back
-# on Odysseus. APP_PORT is intentionally not used: it is only the Docker host
-# port-map; the app always listens on 7000 inside the container.
-_REDIRECT_BASE = (
-    os.environ.get("OAUTH_REDIRECT_BASE_URL")
-    or os.environ.get("APP_PUBLIC_URL")
-    or "http://localhost:7000"
-).rstrip("/")
+# paste-back flow. Deployments whose externally reachable origin differs from the
+# port Odysseus binds — reverse proxy, public domain, or Docker, whose host port
+# map is invisible inside the container — must set OAUTH_REDIRECT_BASE_URL (or
+# APP_PUBLIC_URL), otherwise the redirect never lands back on Odysseus.
+_REDIRECT_BASE = _resolve_redirect_base()
 REDIRECT_URI = f"{_REDIRECT_BASE}/api/mcp/oauth/callback"
 
 # How long the background connect waits for the user to authorize before giving up.

start-macos.sh

@@ -34,6 +34,10 @@ fi
 # values (APP_PORT / APP_BIND), then built-in defaults.
 PORT="${ODYSSEUS_PORT:-${APP_PORT:-7860}}"   # 7860, not 7000 — macOS AirPlay Receiver holds 7000.
 HOST="${ODYSSEUS_HOST:-${APP_BIND:-127.0.0.1}}" # Set APP_BIND=0.0.0.0 in .env for LAN/Tailscale access.
+# The port only reaches uvicorn as a flag, so export it too: everything that
+# builds a URL for this instance — internal_api_base(), the companion pairing
+# code, the MCP OAuth callback — reads APP_PORT and would otherwise assume 7000.
+export APP_PORT="$PORT"
 PROBE_HOST="$HOST"
 if [ "$PROBE_HOST" = "0.0.0.0" ] || [ "$PROBE_HOST" = "::" ]; then
     PROBE_HOST="127.0.0.1"

tests/test_mcp_oauth.py

@@ -130,3 +130,137 @@ def test_update_recovers_from_non_dict_oauth_tokens():
     srv, storage = _fake_storage('["stale", "data"]')
     storage._update("tokens", {"access_token": "new"})
     assert json.loads(srv.oauth_tokens) == {"tokens": {"access_token": "new"}}
+
+
+# ── Callback origin ───────────────────────────────────────────────
+#
+# The redirect URI is registered with the authorization server (dynamically for
+# remote MCP servers, by hand for Google ones) and the browser is sent to it
+# after authorizing. It is resolved once, outside any request, so it cannot be
+# derived from the request the way the email OAuth routes derive theirs — an
+# operator-supplied origin is the only thing that can be right behind a proxy.
+# What the default can get right is the port, which the app knows.
+
+_REDIRECT_ENV = ("OAUTH_REDIRECT_BASE_URL", "APP_PUBLIC_URL", "APP_PORT")
+
+
+def _resolve_base(monkeypatch, **env):
+    for key in _REDIRECT_ENV:
+        monkeypatch.delenv(key, raising=False)
+    for key, value in env.items():
+        monkeypatch.setenv(key, value)
+    return mcp_oauth._resolve_redirect_base()
+
+
+def test_redirect_base_defaults_to_the_bound_port(monkeypatch):
+    # The macOS launcher serves on 7860 because AirPlay Receiver holds 7000; a
+    # callback pinned to 7000 lands on AirPlay instead of Odysseus.
+    assert _resolve_base(monkeypatch, APP_PORT="7860") == "http://localhost:7860"
+
+
+def test_redirect_base_keeps_7000_when_app_port_is_unset(monkeypatch):
+    assert _resolve_base(monkeypatch) == "http://localhost:7000"
+
+
+def test_redirect_base_prefers_the_explicit_origin(monkeypatch):
+    # Only an operator-supplied origin can be right behind a TLS proxy, so it
+    # outranks the derived default — and its trailing slash is trimmed.
+    resolved = _resolve_base(
+        monkeypatch, OAUTH_REDIRECT_BASE_URL="https://odysseus.example/", APP_PORT="7860"
+    )
+    assert resolved == "https://odysseus.example"
+
+
+def test_redirect_base_accepts_app_public_url_as_the_alias(monkeypatch):
+    assert (
+        _resolve_base(monkeypatch, APP_PUBLIC_URL="https://public.example", APP_PORT="7860")
+        == "https://public.example"
+    )
+
+
+# ── Paste-back form origin ────────────────────────────────────────
+
+def _authorize_page():
+    from routes.mcp.mcp_routes import _oauth_authorize_page
+
+    return _oauth_authorize_page(
+        "https://accounts.google.com/o/oauth2/v2/auth?state=srv-1",
+        "srv-1",
+        "https://odysseus.example.com/api/mcp/oauth/callback",
+    )
+
+
+def test_paste_back_form_action_is_relative():
+    # Remote users finish the flow by pasting the callback URL into this form,
+    # so it has to post back to the origin they are on. An absolute action
+    # cannot: an http:// one is mixed content on an HTTPS page and gets blocked,
+    # and the app cannot reliably tell that it is behind TLS, because uvicorn
+    # only honours X-Forwarded-Proto from a peer inside --forwarded-allow-ips
+    # (default 127.0.0.1, which a proxy on the Docker bridge is not). A relative
+    # action is resolved by the browser and is right in every one of those cases.
+    page = _authorize_page()
+    assert 'action="/api/mcp/oauth/exchange/srv-1"' in page
+    assert 'action="http' not in page
+
+
+# ── Docker configurability ────────────────────────────────────────
+#
+# The override above is the only fix available to a Docker install: the
+# container always listens on 7000 and cannot see the host port map, so the
+# derived default cannot be right there. Compose has to forward the variable
+# or the escape hatch does not exist.
+
+_COMPOSE_FILES = (
+    "docker-compose.yml",
+    "docker-compose.gpu-nvidia.yml",
+    "docker-compose.gpu-amd.yml",
+)
+
+
+def _repo_root():
+    from pathlib import Path
+
+    return Path(__file__).resolve().parent.parent
+
+
+def test_redirect_base_override_is_forwarded_into_the_container():
+    import yaml
+
+    for name in _COMPOSE_FILES:
+        path = _repo_root() / name
+        compose = yaml.safe_load(path.read_text(encoding="utf-8"))
+        environment = set(compose["services"]["odysseus"]["environment"])
+        assert "OAUTH_REDIRECT_BASE_URL=${OAUTH_REDIRECT_BASE_URL:-}" in environment, name
+
+
+def test_redirect_base_override_is_documented():
+    import pytest
+
+    env_example = _repo_root() / ".env.example"
+    if not env_example.exists():
+        pytest.skip("this checkout does not include the optional .env.example file")
+    assert "# OAUTH_REDIRECT_BASE_URL=" in env_example.read_text(encoding="utf-8")
+
+
+# ── Launcher port propagation ─────────────────────────────────────
+#
+# The derived default is only as good as APP_PORT, and every launcher hands the
+# port to uvicorn as a command-line flag, which the app cannot read back. Each
+# one has to put the same value in the environment or the callback falls back to
+# 7000 — which is the macOS-launcher-on-7860 case this whole change is about.
+# internal_api_base() and companion pairing read APP_PORT too, so they go wrong
+# in the same way.
+
+_LAUNCHERS = (
+    # file, the export, the uvicorn flag it has to agree with
+    ("start-macos.sh", 'export APP_PORT="$PORT"', '--port "$PORT"'),
+    ("build-macos-app.sh", 'export APP_PORT="$PORT"', '--port "$PORT"'),
+    ("launch-windows.ps1", "$env:APP_PORT = $Port", "--port $Port"),
+)
+
+
+def test_launchers_export_the_port_they_serve_on():
+    for name, export, uvicorn_flag in _LAUNCHERS:
+        text = (_repo_root() / name).read_text(encoding="utf-8")
+        assert uvicorn_flag in text, f"{name}: launcher no longer passes {uvicorn_flag}"
+        assert export in text, f"{name}: serves on a port the app cannot read back"

tests/test_pr_description_check.py

@@ -0,0 +1,313 @@
+"""Regression coverage for PR description and validation-state checks."""
+
+import json
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+
+_REPO = Path(__file__).resolve().parent.parent
+_CHECKER = _REPO / ".github" / "scripts" / "check-pr-description.js"
+_WORKFLOW = _REPO / ".github" / "workflows" / "pr-description-check.yml"
+pytestmark = pytest.mark.skipif(not shutil.which("node"), reason="node not on PATH")
+
+
+def _body(*, app_ran=False, app_not_run=False, screenshot=False, media=""):
+    return f"""## Summary
+
+This focused change has enough concrete summary detail for the checker.
+
+## Linked Issue
+
+Fixes #5934
+
+## Type of Change
+
+- [x] CI / tooling / configuration
+
+## Checklist
+
+- [x] I searched open issues and open PRs.
+- [{'x' if app_ran else ' '}] I actually ran the app and verified the change works end-to-end.
+- [{'x' if app_not_run else ' '}] I did not run the app/runtime validation and stated that gap in How to Test.
+
+## How to Test
+
+Run the focused checker regression tests and inspect their exact assertions.
+
+## Visual / UI changes
+
+- [{'x' if screenshot else ' '}] **Screenshot or short clip** of the running change.
+
+### Screenshots / clips
+
+{media}
+"""
+
+
+def _run_checker(files, body, *, missing_labels=(), draft=False):
+    harness = r"""
+const checkPrDescription = require(process.argv[1]);
+const input = JSON.parse(process.argv[2]);
+const calls = [];
+const listFiles = async () => {};
+const listComments = async () => {};
+
+const github = {
+  paginate: async (method) => {
+    if (method === listFiles) return input.files.map(filename => ({ filename }));
+    if (method === listComments) return [];
+    throw new Error('unexpected paginated method');
+  },
+  rest: {
+    pulls: { listFiles },
+    issues: {
+      listComments,
+      getLabel: async ({ name }) => {
+        if (input.missingLabels.includes(name)) {
+          const error = new Error(`missing label: ${name}`);
+          error.status = 404;
+          throw error;
+        }
+        return { data: { name } };
+      },
+      addLabels: async (params) => calls.push({ method: 'addLabels', params }),
+      removeLabel: async (params) => calls.push({ method: 'removeLabel', params }),
+      createComment: async (params) => calls.push({ method: 'createComment', params }),
+      updateComment: async (params) => calls.push({ method: 'updateComment', params }),
+      deleteComment: async (params) => calls.push({ method: 'deleteComment', params }),
+    },
+  },
+};
+const context = {
+  payload: {
+    pull_request: {
+      number: 42,
+      body: input.body,
+      draft: input.draft,
+    },
+  },
+  repo: { owner: 'odysseus-dev', repo: 'odysseus' },
+};
+const core = {
+  warning: (message) => calls.push({ method: 'warning', message }),
+  setFailed: (message) => calls.push({ method: 'setFailed', message }),
+};
+
+checkPrDescription({ github, context, core })
+  .then(() => process.stdout.write(JSON.stringify(calls)))
+  .catch((error) => {
+    console.error(error);
+    process.exitCode = 1;
+  });
+"""
+    payload = json.dumps(
+        {
+            "files": files,
+            "body": body,
+            "missingLabels": list(missing_labels),
+            "draft": draft,
+        }
+    )
+    proc = subprocess.run(
+        ["node", "-e", harness, str(_CHECKER), payload],
+        capture_output=True,
+        text=True,
+        cwd=str(_REPO),
+        timeout=30,
+    )
+    assert proc.returncode == 0, proc.stderr
+    return json.loads(proc.stdout)
+
+
+def _added_labels(calls):
+    return {
+        call["params"]["labels"][0]
+        for call in calls
+        if call["method"] == "addLabels"
+    }
+
+
+def _comment(calls):
+    comments = [call for call in calls if call["method"] == "createComment"]
+    return comments[0]["params"]["body"] if comments else ""
+
+
+@pytest.mark.parametrize(
+    ("files", "body"),
+    [
+        (["README.md"], _body()),
+        (["routes/example.py"], _body(app_ran=True)),
+        (
+            ["static/js/example.js"],
+            _body(
+                app_ran=True,
+                screenshot=True,
+                media="https://github.com/user-attachments/assets/example",
+            ),
+        ),
+    ],
+)
+def test_complete_expected_state_is_ready(files, body):
+    calls = _run_checker(files, body)
+
+    assert _added_labels(calls) == {"ready for review"}
+    assert not _comment(calls)
+    assert not any(call["method"] == "setFailed" for call in calls)
+
+
+def test_ui_checkbox_without_media_still_needs_visual_evidence():
+    calls = _run_checker(
+        ["static/js/example.js"],
+        _body(app_ran=True, screenshot=True),
+    )
+
+    assert _added_labels(calls) == {"needs visual evidence"}
+    assert "does not contain an actual attachment or link" in _comment(calls)
+    assert not any(call["method"] == "setFailed" for call in calls)
+
+
+def test_explicit_not_run_is_honest_but_not_ready():
+    calls = _run_checker(
+        ["services/example.py"],
+        _body(app_not_run=True),
+    )
+
+    assert _added_labels(calls) == {"needs runtime validation"}
+    assert "explicitly reports that app/runtime validation was not performed" in _comment(calls)
+    assert "does not prove that a local command ran" in _comment(calls)
+    assert not any(call["method"] == "setFailed" for call in calls)
+
+
+def test_missing_validation_label_fails_soft():
+    calls = _run_checker(
+        ["services/example.py"],
+        _body(app_not_run=True),
+        missing_labels=("needs runtime validation",),
+    )
+
+    assert "needs runtime validation" not in _added_labels(calls)
+    assert any(
+        call["method"] == "warning"
+        and 'Label "needs runtime validation" does not exist' in call["message"]
+        for call in calls
+    )
+    assert not any(call["method"] == "setFailed" for call in calls)
+
+
+@pytest.mark.parametrize(
+    "filename",
+    ("requirements.txt", "requirements-optional.txt"),
+)
+def test_requirement_manifests_require_runtime_validation(filename):
+    calls = _run_checker(
+        [filename],
+        _body(app_not_run=True),
+    )
+
+    assert _added_labels(calls) == {"needs runtime validation"}
+    assert not any(
+        call["method"] == "addLabels"
+        and call["params"]["labels"] == ["ready for review"]
+        for call in calls
+    )
+    assert any(
+        call["method"] == "removeLabel"
+        and call["params"]["name"] == "ready for review"
+        for call in calls
+    )
+    assert "Changed-file classification: **backend/runtime**." in _comment(calls)
+
+
+def test_old_template_runtime_pr_is_not_ready_without_attestation():
+    calls = _run_checker(
+        ["routes/example.py"],
+        _body(),
+    )
+
+    assert _added_labels(calls) == {"needs runtime validation"}
+    assert "App/runtime validation is not author-attested" in _comment(calls)
+    assert not any(call["method"] == "setFailed" for call in calls)
+
+
+def test_conflicting_runtime_attestations_are_not_ready():
+    calls = _run_checker(
+        ["services/example.py"],
+        _body(app_ran=True, app_not_run=True),
+    )
+
+    assert _added_labels(calls) == {"needs runtime validation"}
+    assert "both checked" in _comment(calls)
+    assert not any(
+        call["method"] == "addLabels"
+        and call["params"]["labels"] == ["ready for review"]
+        for call in calls
+    )
+
+
+def test_structurally_invalid_description_still_fails_hard_gate():
+    calls = _run_checker(
+        ["README.md"],
+        "## Summary\nshort\n",
+    )
+
+    assert any(call["method"] == "setFailed" for call in calls)
+    assert "PR description" in _comment(calls)
+    assert "needs work" in _added_labels(calls)
+    assert "ready for review" not in _added_labels(calls)
+
+
+def test_github_workflow_only_change_does_not_require_app_runtime():
+    calls = _run_checker(
+        [".github/workflows/example.yml"],
+        _body(),
+    )
+
+    assert _added_labels(calls) == {"ready for review"}
+    assert not _comment(calls)
+    assert not any(call["method"] == "setFailed" for call in calls)
+
+
+def test_draft_pr_never_receives_ready_for_review():
+    calls = _run_checker(
+        ["README.md"],
+        _body(),
+        draft=True,
+    )
+
+    assert "ready for review" not in _added_labels(calls)
+    assert any(
+        call["method"] == "removeLabel"
+        and call["params"]["name"] == "ready for review"
+        for call in calls
+    )
+    assert not any(call["method"] == "setFailed" for call in calls)
+
+
+def test_workflow_serializes_readiness_before_mergeability():
+    workflow = _WORKFLOW.read_text()
+
+    assert (
+        "types: [opened, edited, synchronize, reopened, ready_for_review, "
+        "converted_to_draft]"
+    ) in workflow
+    assert "group: pr-description-${{ github.event.pull_request.number }}" in workflow
+    assert "cancel-in-progress: true" in workflow
+
+    mergeable = workflow.split("  check-mergeable:", 1)[1]
+    assert "needs: check-description" in mergeable
+    assert (
+        "if: ${{ !cancelled() && github.event.pull_request.user.type != 'Bot' }}"
+        in mergeable
+    )
+
+
+def test_privileged_pr_workflow_executes_only_base_code():
+    workflow = _WORKFLOW.read_text()
+
+    assert "pull_request_target:" in workflow
+    assert "ref: ${{ github.base_ref }}" in workflow
+    assert "persist-credentials: false" in workflow
+    assert "github.event.pull_request.head" not in workflow

tests/test_security_regressions.py

@@ -1027,9 +1027,13 @@ def test_session_html_export_escapes_name():
 def test_mcp_oauth_page_escapes_reflected_values():
     src = Path(__file__).resolve().parents[1] / "routes" / "mcp" / "mcp_routes.py"
     text = src.read_text()
-    body = text.split("def _oauth_authorize_page(", 1)[1].split("return f", 1)[0]
-    for var in ("auth_url", "server_id", "host", "redirect_uri"):
+    page = text.split("def _oauth_authorize_page(", 1)[1].split("def _oauth_result_page", 1)[0]
+    body = page.split("return f", 1)[0]
+    for var in ("auth_url", "server_id", "redirect_uri"):
         assert f"{var} = html.escape({var}" in body, var
+    # The Host header is no longer reflected at all: the paste-back form posts to
+    # a relative action, so there is nothing to escape and nothing to smuggle.
+    assert "{host}" not in page
 
 
 def _import_mcp_routes():