ci(bot-check): give the monitor an alarm path and derive what it asserts

[MarkusNeusinger]

Why

bot-serving-check.yml is the alarm for a failure mode humans cannot see: crawlers get an error page or the empty SPA shell while the site looks healthy, Plausible shows normal traffic and CI is green. It exists because exactly that ran for four weeks in 2026 undetected.

But the monitor had no alarm path (issues: write = 0). It could only turn a tab red — and its own header records what that is worth: the check "went red for ten consecutive days without anyone noticing". The cause of those ten days was the second half of this PR: the expectations were hard-coded literals, the home page copy changed, and the alarm became one nobody could trust. A monitor that cries wolf in a tab nobody reads is not a monitor.

The sibling repo kurrentschrift has both halves already; transferred here per the sibling rule.

What changes

1. The alarm path (permissions: issues: write, two new steps)

• if: failure() → looks for an open issue titled Bot serving check is red. If it exists, comment Still red: <run url> — so a week-long outage is one thread, not seven issues. If not, open it (bug + infrastructure labels) with a body that names the two causes worth separating first: the serving path actually broke, or the deployed pages are simply behind the repo (a merged change not yet deployed reads as a mismatch).
• if: success() → comment Green again: <run url> and close it.
• The title travels into jq as env.ALARM_TITLE, never as interpolated syntax.

2. Derived expectations instead of hand-written literals

• Routes come from the repo: every @router.get("/seo-proxy/…") in api/routers/seo.py, read off the parsed AST rather than grepped — a decorator in single quotes, wrapped over two lines, carrying kwargs or passing its path as path= is silently skipped by a regex, and a short list that looks like coverage is the exact failure this file exists to stop making. A router.get whose path is not a string literal at all is counted and reported rather than skipped: a route this cannot see is one it cannot promise to cover. That is 10 pages today — /, /plots, /specs, /libraries, /legal, /mcp, /about, /palette, /map, /stats — where the file previously probed only /. A bot page added in seo.py is swept the moment it lands. The parameterised decorators cannot be swept generically, so their count is returned too and checked against the three this file probes by hand — a fourth added to seo.py fails the run instead of going unnoticed.
• The per-route assertion is the canonical link, <link rel="canonical" href="https://anyplot.ai{route}" />. It is generated from the route, so no copy change can make it stale, and it proves both halves at once: the SPA shell (app/index.html) carries no canonical at all, so a match means the bot hop ran, and the href names the route, so it means the right page came back.
• The spec title comes from plots/scatter-basic/specification.yaml (title: Basic Scatter Plot), decoded with yaml.safe_load and then html.escaped — two decodings, both mirroring what the page actually does with the value. Not the source line, because a quoted title is valid YAML and already occurs here (plots/heatmap-chromagram); and escaped, because api/routers/seo.py:429 escapes the title before it reaches <title>. Either omission is a false alarm against a perfectly healthy page — the same failure this rewrite exists to remove, arriving through another door. Asserted as the prefix <title>Basic Scatter Plot. The suffix is site copy, and the implementation page inserts the library display name — neither is this monitor's to pin. That kills the last four stale-able literals (the spec page, the ClaudeBot probe, and the 15-UA loop).
• The implementation page /scatter-basic/python/matplotlib is asserted on its canonical rather than a title prefix: a three-segment route is precisely the one that would still look right with the hub page served in its place.
• The middle tier /{spec}/{language} — the third parameterised route, consolidated onto the hub — now has a probe too: it must 301 to /{spec}, and a target still carrying /seo-proxy is called out explicitly, because that is the redirect loop its own docstring records (nginx re-prefixes the Location, the request comes back to the same route, and Googlebot logged 48 "Redirect error" URLs before the target was sanitised).
• scatter-basic is now a $SPEC variable, so the trailing-slash and human-control probes follow it too.

3. Two watchdogs against a silent no-op. A checkout that produced nothing, a parse error, or a refactor that moves those decorators out of reach would otherwise let the whole sweep pass by checking zero routes: swept < 1 is an error, as is a spec file with no title:. And the parameterised-route count must stay at 3, the number this file probes individually.

4. The alarm is fenced. A concurrency group (bot-serving-check-${{ github.ref }}, cancel-in-progress: true) — a manual dispatch during the nightly run, which can take up to the timeout, would otherwise have two runs mutating the same issue: two failures could open it twice, and an older run finishing last could close a newer failure or reopen an alarm after a newer success. And both issue steps run only on the default branch: workflow_dispatch can pick any branch carrying this file, and a branch experimenting with the derived expectations must not be able to raise, or silently close, a repository-wide incident. Off main the checks still run and still red the job.

5. Timeout recomputed by the file's own formula. 36 check() calls × 90 s (--retry 2 → 3 attempts × --max-time 30) ≈ 54 min, plus five non-retried probes at 30 s → 62 (was 46 for 27 calls). The comment says so and names the route sweep as the thing that grows it.

A checkout step is added (fetch-depth 1) — the derivation needs the repo present before the first request goes out.

Evidence

actionlint 1.7.7 on the changed file: clean (exit 0). bash -n on the extracted 190-line script: clean.

The route derivation, run against api/routers/seo.py at this commit:

$ grep -oE '@router\.get\("/seo-proxy/[^"{]*"\)' api/routers/seo.py \
    | sed -E 's|@router\.get\("/seo-proxy||; s|"\)$||' | sed 's|^$|/|' | sort -u
/  /about  /legal  /libraries  /map  /mcp  /palette  /plots  /specs  /stats

Ten routes; the file's @router.get("/seo-proxy/{spec_id}"), /{spec_id}/{language} and /{spec_id}/{language}/{library} are counted as parameterised, not swept, and the count guard is satisfied at 3.

The AST extraction beats the regex it replaced, measured against a synthetic seo.py carrying four static decorators (plain, single-quoted, wrapped over two lines, with kwargs) and four parameterised ones:

=== old grep found ===
/
=== new AST extraction ===
swept 4 derived bot route(s)
::error::api/routers/seo.py has 4 parameterised /seo-proxy route(s); this monitor probes 3 by hand — add a probe for the new one and update this guard

1 of 4 static routes before, 4 of 4 now, and the count guard fires on the extra templated route. A second file, carrying the path= keyword form and a path taken from a module constant:

swept 2 derived bot route(s)                    (the keyword one included)
::error::… has 4 parameterised /seo-proxy route(s); this monitor probes 3 by hand …
::error::1 router.get decorator(s) … carry a path this monitor cannot resolve

The whole check step, run locally against the Cloud Run origin (reads only — 36 checks + 5 probes, no writes anywhere):

OK: …/ (Googlebot)          OK: …/about      OK: …/legal     OK: …/libraries
OK: …/map                   OK: …/mcp        OK: …/palette   OK: …/plots
OK: …/specs                 OK: …/stats
swept 10 derived bot route(s)
expecting the scatter-basic pages to be titled: Basic Scatter Plot
OK: …/scatter-basic (Googlebot)
OK: …/scatter-basic/python/matplotlib (Twitterbot/1.0)
OK: …/scatter-basic (ClaudeBot)
… 15 user-directed fetcher UAs, all OK …
OK: …/this-spec-does-not-exist   OK: …/robots.txt   OK: …/sitemap.xml
OK: og-image.png served as image to a preview bot
OK: .well-known/llms.txt -> …/llms.txt
OK: …/llms.txt (Googlebot)  OK: …/llms.txt (ChatGPT-User)  OK: …/llms-full.txt
OK: llms.txt content-type: text/plain; charset=utf-8
OK: /scatter-basic/python -> 301 …/scatter-basic   (the consolidated middle tier)
OK: trailing slash -> …/scatter-basic
OK: …/ (human)              OK: …/scatter-basic (human)
EXIT=0

Both title decodings matter, on the two specs the review named — checked against the live page:

sed:    ["Music Chromagram (Pitch Class Distribution over Time)"]
parser: [Music Chromagram (Pitch Class Distribution over Time)]

yaml only : "Mohr's Circle for Stress Analysis"
escaped   : 'Mohr's Circle for Stress Analysis'
served    : <title>Mohr&#x27;s Circle for Stress Analysis | anyplot.ai</title>

escaped needle present in the served page: True
raw needle present in the served page:     False

The watchdog, proven to bite. Same script run against a tree with an empty seo.py and a specification.yaml without a title:

::error::no bot routes found in api/routers/seo.py — the sweep asserted nothing
::error::plots/scatter-basic/specification.yaml carries no title — nothing to assert against
EXIT=1

(The title guard then also fails every page probe, on purpose: it substitutes a sentinel rather than an empty needle, which grep -F "" would match against anything.)

Decisions taken here (routine, flagged for override)

• The canonical link rather than a title as the per-route marker. anyplot has no committed prerender files, so the kurrentschrift original's "read the expected title out of app/prerender/*.html" has no direct counterpart. The canonical is the equivalent generated-not-authored property, and it is strictly better at telling routes apart.
• The 15-UA list stays hand-written. Deriving it from the ~* patterns of the $is_bot map in app/nginx.conf is not mechanical (a regex fragment is not a UA string), and the list is a policy statement, not a copy of something. Only its assertion moved to the derived title.
• Sweeping all 10 static bot routes rather than keeping one probe. It is the point of the transfer — a page added to seo.py that nginx or the SPA does not route is exactly the invisible failure this monitor is for — and it costs 9 more checks, which is what raised the timeout ceiling to 62.
• Labels bug + infrastructure on the alarm issue; both already exist in this repo.
• The concurrency group stays ref-scoped, not constant. Only default-branch runs can mutate the issue (the branch gate above), and those all share one ref and therefore one group; a constant group would instead let an unrelated branch test cancel the nightly production run.

Findings for the author

1. The first real exercise of the alarm path is a failing scheduled run; opening/closing an issue cannot be dry-run from a PR branch (the schedule trigger only runs on main). The two steps are byte-for-byte the kurrentschrift ones, which have run in production there.
2. /scatter-basic is now referenced through $SPEC in six places. If that spec is ever retired, the monitor fails loudly on it rather than silently skipping — which is the right direction, but worth knowing.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke

[Copilot]

🟡 Changes recommended

Alarm-state races and the untested language-level redirect must be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Hardens the bot-serving monitor with repository-derived assertions and issue-based failure alerts.

Changes:

• Derives monitored routes, canonical links, and spec titles.
• Opens, updates, and closes an alarm issue.
• Updates timeout accounting and documentation.

File summaries

File	Review
docs/workflows/overview.md	Documents the enhanced monitor; no issues found.
CHANGELOG.md	Nit: Add the pull-request reference to the new entry.
agentic/docs/project-guide.md	Updates the workflow reference; no issues found.
.github/workflows/bot-serving-check.yml	Critical: Serialize overlapping runs so stale results cannot corrupt alarm state or create duplicate issues. Moderate: Probe /$SPEC/python, verify its 301 target, and update timeout accounting. Nit: Correct the parameterized-route comment if that redirect remains intentionally unprobed.

Review details

Suppressed comments (4)

.github/workflows/bot-serving-check.yml:262

• workflow_dispatch can run this workflow from a feature branch, so a branch whose derived expectations are not deployed can open or update the production alarm. Restrict issue mutations to the default branch; branch dispatches can still run the monitor without changing shared incident state.

        if: failure()

.github/workflows/bot-serving-check.yml:303

• A successful workflow_dispatch from any feature branch can close an alarm raised by the scheduled default-branch monitor. Apply the same default-branch guard here so only production-context runs can stand down the incident.

        if: success()

.github/workflows/bot-serving-check.yml:102

• This source currently has three parameterized decorators, including /{spec_id}/{language} at api/routers/seo.py:1306. The comment incorrectly says there are two and implies every excluded route is probed below; clarify that the language-level route is an intentionally unprobed redirect.

          # go stale. (The two parameterised decorators, /{spec_id} and
          # /{spec_id}/{language}/{library}, are excluded by the `[^"{]` class
          # and probed separately below.)

CHANGELOG.md:46

• This new changelog bullet is missing its pull-request reference. Repository changelog entries carry their PR number (for example, the neighboring entries at lines 57 and 71), so append this PR's (#…) reference before merging.

  recomputed to 62 min by the file's own formula (36 checks x 90 s + four non-retried
  probes).

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The workflow has unresolved reliability and false-alarm risks, and the generated artifact should be removed.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

.github/workflows/bot-serving-check.yml:127

• Each request added to this /seo-proxy sweep emits a bot_fetch event (api/main.py:203-205), but docs/reference/plausible.md:297 still tells operators to expect about 25 requests from this daily run. The nine additional static routes plus the new language redirect materially change that dashboard baseline; update the analytics reference with the recomputed count.

          while IFS= read -r route; do
            [ -n "$route" ] || continue
            swept=$((swept + 1))
            check "$GOOGLEBOT" "$ORIGIN$route" "<link rel=\"canonical\" href=\"https://anyplot.ai$route\" />"

.github/workflows/bot-serving-check.yml:332

• This success path also runs for manually dispatched non-default branches, so a successful PR-branch check can close an alarm raised by the scheduled default-branch monitor. Restrict the stand-down action to the default branch so only the production monitor controls its issue.

        if: success()

• Files reviewed: 4/4 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The workflow has unresolved moderate issues affecting alarm correctness and monitoring reliability.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

.github/workflows/bot-serving-check.yml:168

• This probe checks only %{redirect_url}, so a 302, 307, or 308 to the expected target passes even though the documented contract is specifically a permanent 301. Capture %{http_code} together with the target and reject every status other than 301.

          # refactor that moves the decorators somewhere this cannot see would
          # otherwise pass the whole sweep by checking zero routes.

.github/workflows/bot-serving-check.yml:122

• The regex recognizes only a single-line decorator whose path is the sole argument. Adding a valid static route with another decorator argument or formatting the call across lines silently omits that route while swept remains nonzero, so the watchdog does not protect the stated “every route” invariant. Derive the literals with Python's AST (or another parser of the route registry) rather than matching one source-code layout.

          # a fourth one added to seo.py fails the run instead of going
          # unnoticed.
          #

.github/workflows/bot-serving-check.yml:291

• workflow_dispatch can run this workflow from a feature ref, where checkout derives expectations from code that is not deployed to the production origin. Because this alarm step is unguarded (and the concurrency group is ref-scoped), such a run can open or append to the production alarm even when production is healthy. Restrict issue mutation to the default branch.

          esac

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Route and title derivation can silently miss or misinterpret valid source forms, causing coverage gaps or false incidents.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The derived title check can falsely fail on valid HTML-sensitive characters.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

CHANGELOG.md:40

• Replace the relative word “today”; changelog prose is historical and the repository documentation rules require absolute, non-rotting time references. Removing the snapshot count also keeps this entry accurate after more routes are added.

  its path as `path=` is not silently skipped the way a regex skips it, and a path that is

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Both alarm paths must only mutate workflow-owned issues.

Review details

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

.github/workflows/bot-serving-check.yml:395

• The title-only lookup lets any pre-existing issue with this title suppress creation of the workflow-owned alarm. That issue may lack the alarm labels or be unrelated, so failures would be appended to the wrong thread. Include author in the JSON and only reuse an issue created by github-actions[bot].

This issue also appears on line 394 of the same file.
.github/workflows/bot-serving-check.yml:434

• This title-only lookup can comment on and close an unrelated issue created by a user who chose the same title. Restrict the match to the workflow bot so the success path only mutates alarms that this workflow owns.

This issue also appears on line 433 of the same file.

.github/workflows/bot-serving-check.yml:395

• This lookup examines only the first 100 open issues. If the alarm issue falls outside that window during a long outage, the next failure creates a duplicate instead of appending to the existing incident. Narrow the list server-side by title before applying the exact jq check.

        run: |
          set -euo pipefail

.github/workflows/bot-serving-check.yml:434

• The same 100-issue window can hide an older open alarm, causing a recovery run to report that there is nothing to close while the incident remains open. Search by title server-side, then retain the exact-title filter.

          ALARM_TITLE: Bot serving check is red
          RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Route classification can misinterpret unrelated paths and trigger false production alarms.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

.github/workflows/bot-serving-check.yml:154

• The prefix test also accepts unrelated paths such as /seo-proxy-health; removeprefix then derives -health, so the monitor probes $ORIGIN-health and raises a false production alarm. Match the /seo-proxy path segment boundary before classifying routes.

.github/workflows/bot-serving-check.yml:61

• The expanded sweep makes the analytics runbook stale: docs/reference/plausible.md:297-298 still says this workflow emits about 25 crawler requests, but the new accounting yields 38 crawler-UA requests (33 retried check() calls after excluding the three human-UA calls, plus five non-retried probes). Update that diagnostic baseline so operators do not treat a healthy run as inflated bot traffic.

    # 36 check() calls (10 derived bot routes + spec page + impl page +
    # ClaudeBot + 15 crawler UAs + 404 + robots + sitemap + 3 llms + 2 human
    # controls) x (--retry 2 -> up to 3 attempts x --max-time 30) can reach
    # ~54 min worst-case, plus five non-retried probes (llms.txt charset,
    # trailing slash, og-image, .well-known redirect, the /{spec}/{language}
    # 301 — 30s each); 62 leaves

docs/workflows/overview.md:180

• This description is inaccurate: the monitor intentionally accepts a 404 for the nonexistent spec and a 301 for the language route, so it does not fail on every non-200 response. Describe it as failing on unexpected statuses or content instead.

| `bot-serving-check.yml` | Daily synthetic monitor: curls the Cloud Run origin with crawler UAs and fails on non-200 or a page that is not the prerendered one. Routes are derived from the `@router.get("/seo-proxy/…")` decorators in `api/routers/seo.py` and the expected spec title from `plots/<spec>/specification.yaml`, so no literal here can go stale. A failure opens (or comments on) the fixed-title issue **Bot serving check is red**; the next green run closes it — the bot→seo-proxy path is invisible to human traffic and needs its own alarm |

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The workflow has an unresolved critical credential-exposure issue and requires human review.

Review details

Suppressed comments (1)

.github/workflows/bot-serving-check.yml:56

• This increased request set leaves docs/reference/plausible.md:297-298 stale: it still tells operators to expect about 25 monitor events at 06:23 UTC. The new static-route sweep alone adds nine crawler requests, and the middle-tier probe adds another, so that documented diagnostic baseline can now mislead bot_fetch troubleshooting. Recompute and update it in this PR.

    # 36 check() calls (10 derived bot routes + spec page + impl page +

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced