Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions .claude/guardrails.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,43 @@ instead of trusting a "done" in chat. A runbook that shipped this way belongs
in the owning README, so the next round starts from a reviewed text rather
than a fresh improvisation.

## After every merge, check that the site renders

Owner rule, 2026-10-22, after an outage nothing caught.

**What happened.** A Dependabot batch merged nine PRs in an afternoon, one
lock file at a time, each with green checks. One of them, vite 8.0.16 to
8.1.6 (#12225), deployed at 17:55 UTC. Rolldown 1.1.5 put its CJS-interop
runtime helper into the app's `theme` chunk, which imports `mui`, while
`mui` imports the helper back. The chunk cycle left the helper undefined, the
bundle threw `TypeError: t is not a function` at module load, and every page
on anyplot.ai rendered blank. Every PR check passed, including "Build app
image and smoke the origin gate", which proves nginx serves the shell but
never runs the JavaScript. The deploy was green too. The owner noticed about
45 minutes later; a cloud session found the cause and shipped vite 8.3.5
(#12235), which emits the helper as its own `rolldown-runtime` chunk.

**The check.** `scripts/site-smoke.mjs` loads `/`, `/plots` and
`/scatter-basic` in headless Chromium and fails on a status other than 200,
an uncaught page error, or a `#root` with next to no text; it also checks
`api.anyplot.ai/health`. It takes about two seconds. The recipe is in
`/open-pr` §4. Run against a build of the broken commit, it fails exactly as
the outage did; against a healthy one it passes, with the CORS error of a
foreign origin reported as a warning only.

**Two layers.** The `app-image` job of `ci-image.yml` runs the same script
against the built container before the merge, so a bump that breaks the boot
fails its PR. It runs when the container's inputs change (`package.json`,
`yarn.lock`, `vite.config.ts` and the like), not for `app/src` edits, and it
cannot see what only production has: the CDN, the origin gate armed, the real
data. The post-merge run covers those, and every merge that never builds the
container.

**Why after every merge.** A merge that touches no `app/` file can still
break the site through the API, a Cloudflare rule, or a data change, and the
run costs seconds. A `FAIL` stops further merges until the site renders,
because each extra merge widens the set of suspects and the revert.

## Modify repo files only with the Edit/Write tools

Appending with `>>` counts — appending at the end of a file is exactly the
Expand Down
38 changes: 37 additions & 1 deletion .claude/skills/open-pr/SKILL.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
name: open-pr
description: Take a finished change from diff to an open, green, review-clean PR — run the matching verify skills and local CI gates first, then commit, push and open the PR via /pull_request, watch the pipeline AND the Copilot review, fix sensible findings, resolve the review threads, and watch any Cloud Build deploy after merge. Detects local vs. cloud GitHub tooling (gh vs. GitHub MCP). Never merges unless the user explicitly authorizes it in this session. Use when asked to open or create a PR, ship a change, or finish up a change.
description: Take a finished change from diff to an open, green, review-clean PR — run the matching verify skills and local CI gates first, then commit, push and open the PR via /pull_request, watch the pipeline AND the Copilot review, fix sensible findings, resolve the review threads, and after a merge watch the Cloud Build deploys and check in a browser that the live site still renders (scripts/site-smoke.mjs). Detects local vs. cloud GitHub tooling (gh vs. GitHub MCP). Never merges unless the user explicitly authorizes it in this session. Use when asked to open or create a PR, ship a change, or finish up a change.
---

# Open a PR (ship a change)
Expand Down Expand Up @@ -316,6 +316,42 @@ while both deploys are already done (2026-08-28, #10808 follow-through:
a 20-minute poll on the global list never saw the builds). Match the
`SHORT_SHA` column against the merge commit before trusting a row.

**Then check that the site renders — after every merge, whatever it
touched.** A green PR and a green deploy do not prove the bundle boots:
on 2026-10-22 vite 8.1.6 (#12225) passed every check and both, and every
page on anyplot.ai stayed blank for about 45 minutes until #12235. Once
the merge's deploys have finished (the watcher above shows them
`SUCCESS`; right after the merge when it triggered none), run:

```bash
SCRATCH=<the session scratchpad>
[ -d "$SCRATCH/node_modules/playwright-core" ] || \
(cd "$SCRATCH" && PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1 npm i --silent playwright-core@1.64.1)
NODE_PATH="$SCRATCH/node_modules" node scripts/site-smoke.mjs
```

It loads `/`, `/plots` and `/scatter-basic` in headless Chromium,
checks `api.anyplot.ai/health`, and prints one `OK` or `FAIL` line per
check, with the entry bundle (`bundle=index-*.js`) that answered; it
never downloads a browser. A failed route counts too: the app's error
boundary ("Something went wrong") keeps the masthead and footer, so
the script looks for its text, not only for an empty page.

Without `gcloud` (a cloud session) the deploy cannot be watched, and
elapsed time proves nothing: Cloud Build may still be waiting for an
earlier deploy. For a merge that touches `app/`, run the script once
before the merge and wait until `bundle=` changes; until it does, or
when you cannot tell, report the post-merge check as unconfirmed
rather than passed.

- **Exit 1 is an incident.** Tell the owner at once with the failing
lines and every merge since the last passing run, propose reverting
the suspect merge (a revert PR is the fastest rollback), and land no
further merge until the site renders again.
- **Exit 2** means no browser was found, so nothing was checked: say
so, then load `/` and a spec page with the Playwright MCP and read
its console errors instead.

## Gotchas

- **`isOutdated` ≠ `isResolved`.** A fix-push can outdate a Copilot
Expand Down
22 changes: 22 additions & 0 deletions .github/workflows/ci-image.yml
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,28 @@ jobs:
want "armed, no secret: empty header too" "$(code -H 'X-Origin-Secret;' localhost:8083/)" 403
want "armed, no secret: /_health exempt" "$(code localhost:8083/_health)" 200

# Served is not booted. On 2026-10-22 a Vite minor bump (#12225) passed
# every step above and blanked every page in production: Rolldown put its
# runtime helper into a chunk cycle, and the bundle threw `TypeError: t is
# not a function` at module load (fixed by #12235). The runner's Google
# Chrome loads the gate-off container's home page and fails on an
# uncaught page error or an empty #root; scripts/site-smoke.mjs is the
# same check the post-merge rule runs against production. No API check:
# the page's data calls go to production, whose CORS does not know this
# origin, so only the boot is asserted.
- name: Browser boot (the bundle renders)
if: steps.check.outputs.should_build == 'true'
env:
SITE: http://localhost:8081
API: none
CHROME_PATH: /usr/bin/google-chrome
MIN_TEXT: "50"
PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: "1"
run: |
set -euo pipefail
npm install --no-save --no-audit --no-fund --silent --prefix "$RUNNER_TEMP/smoke" playwright-core@1.64.1
NODE_PATH="$RUNNER_TEMP/smoke/node_modules" node scripts/site-smoke.mjs /

- name: Container logs on failure
if: failure() && steps.check.outputs.should_build == 'true'
run: |
Expand Down
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ A companion guide `.github/copilot-instructions.md` carries the shared rules for
- **External-system writes need explicit, named authorization** - Merging or closing PRs/issues this session did not create, bulk merges, and label changes on others' PRs are blocked by the permission classifier unless the user named that action; a generic "ok, sounds good" authorizes nothing. The same discipline covers every prod-touching action in interactive sessions — Cloud SQL writes/DDL, GCS production-folder changes, Secret Manager access, Cloud Build config: name the exact action, resource, and id, and ask before acting (the automated `spec-*`/`impl-*` workflows write these by design and are exempt). For any change to `.claude/settings*.json`, use the built-in `/update-config` skill (a Claude Code harness skill, not a repo command) immediately — direct writes are blocked as self-modification and retrying variants just burns round trips.
- **Never echo secret values into the transcript** - Verify secrets by exit code or metadata, never by printing them; never create a Secret Manager version via `echo` (the trailing newline corrupts the value).
- **Snapshot before destructive prod operations** - Before anything that can overwrite or delete shared prod DB data or GCS production objects (bulk UPDATE/DELETE, a data-rewriting migration, bulk GCS overwrite/delete): take a timestamped backup first (`pg_dump` to a new directory outside the working tree, `gsutil cp` to a backup prefix), sanity-check it (row/object counts — a silent empty snapshot is worse than none because it looks like safety), and never write into, delete, or rename an existing snapshot.
- **After every merge, check that the site renders** - Yours, Dependabot's, or one the owner asked for: once the merge's deploys finish, run `scripts/site-smoke.mjs` (recipe in `/open-pr` §4); a `FAIL` is an incident, reported at once with the suspect merge and a revert proposal, and nothing else merges until the site renders. A green PR plus a green deploy blanked anyplot.ai for 45 minutes on 2026-10-22 (#12225).
- **Long-running work reports progress proactively** - During pipeline babysitting, bulk operations, or extended planning, post a one-line status roughly every 10 minutes and flag a suspected stall immediately with evidence; never let the user be the one to ask "still running?". Before starting a multi-item queue, confirm its extent and stop-point with the user.
- **Manual user tasks go to Todoist** - Whenever a session identifies a step only the user can or should do (adding `approved` labels, merge authorization, console/billing/DNS actions, secret rotations), create a task in the user's Todoist project **Anyplot** (via the Todoist MCP tools) naming the concrete action and a context link (PR/issue/run URL) — instead of leaving it buried in a chat reply. Interactive sessions only; GitHub Actions workflows have no MCP access.
- **Fix small build/test blockers directly, even when out of scope** - If a typecheck error, failing test, lint failure, or other small pipeline blocker shows up while working on something unrelated (incl. things the current PR was not meant to touch), fix it in the same PR or a tiny follow-up — never leave it parked under "out of scope". A latent `tsc` error that doesn't surface locally will silently break the next Cloud Build, which deploys nothing new and leaves production stale even though every PR check looks green (this is exactly how PR #6961's frontend fixes never reached anyplot.ai — the unrelated `prism/r` TS7016 from #6944 was deferred, then blocked the next `yarn build`). The bar: if the fix is < ~20 lines and obviously correct, just do it; if it would expand scope meaningfully, ask first rather than deferring silently.
Expand Down
19 changes: 15 additions & 4 deletions agentic/commands/dependabot.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@ known GitHub quirks. Every gotcha below cost real session round-trips.
- Auto-merge is the mechanism of choice: `gh pr merge <num> --auto --squash` — the PR merges
itself when checks pass; no polling needed.
- Grouped bumps (npm-minor, python-minor, actions) are one PR each — treat like any other.
- **One lock-file PR at a time, and every `app/` bump checked in the browser.** Of the PRs that
touch the same lock file (`uv.lock`, `app/yarn.lock`), only one has auto-merge at a time: several
squash-merged within a minute have duplicated package blocks in `uv.lock`. After an `app/` PR
merges, wait for its `deploy-app` build and run the site smoke (`/open-pr` skill §4,
`scripts/site-smoke.mjs`) before the next `app/` PR gets auto-merge. On 2026-10-22 vite 8.1.6
(#12225) passed every check, blanked every page, and stayed unnoticed for about 45 minutes
inside a batch. Comment `@dependabot rebase` on the next PR of a chain before you enable its
auto-merge, and expect Dependabot to close and regroup a PR now and then ("updatable in another
way"); list the PRs again when it does.
- Per the changelog contract, individual Dependabot bumps get NO `CHANGELOG.md` entries —
they are aggregated into one **Dependencies:** bullet at release time (`/release` step 2).

Expand Down Expand Up @@ -40,11 +49,13 @@ known GitHub quirks. Every gotcha below cost real session round-trips.
3. A dep bump breaking tests/config (e.g. a major with changed exports): consult **Context7**
for the library's current API (CLAUDE.md MCP tools section) — do not grep `node_modules`
`.d.ts` files. Fix on the Dependabot branch or note the PR for manual handling.
4. When all PRs have auto-merge set, **stop — do not leave a background monitor running.**
Auto-merge needs no supervision, and orphaned monitors fire notifications into later
sessions. Report the list with each PR's end state instead.
4. When the last PR has merged and its deploys finished, run the site smoke once more
(`/open-pr` skill §4). A failure is an incident: report it at once with the merges since the
last passing run. Then **stop — do not leave a background monitor running.** Orphaned
monitors fire notifications into later sessions. Report the list with each PR's end state
instead.

## Report

Per PR: number, title, action taken (auto-merge set / updated+re-triggered / needs manual
attention + why).
attention + why), and the site smoke's result after the last merge.
10 changes: 10 additions & 0 deletions changelog.d/site-smoke-after-merge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
### Added

- **A browser check that the site renders, before and after every merge.**
`scripts/site-smoke.mjs` loads the home page, `/plots` and a spec page in
headless Chromium and fails on a JavaScript error or an empty page. The
app-image CI job runs it against the built container, so a dependency bump
that breaks the boot fails its PR, and a new rule in `CLAUDE.md` (with the
recipe in the `open-pr` skill and the Dependabot batch command) runs it
against production after every merge. On 2026-10-22 a vite bump passed
every check and left every page blank for about 45 minutes. (#12236)
1 change: 1 addition & 0 deletions docs/reference/repository.md
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ anyplot/
│ └── cloudflare/ # Source for the apex Worker + its rollout notes
│
├── scripts/ # One-time and manual scripts
│ ├── site-smoke.mjs # Does the site render? After every merge, and in CI
│ ├── evaluate-plot.py # Manual plot evaluation
│ ├── regenerate-thumbnails.py # Image processing
│ ├── backfill_review_metadata.py # One-time migration
Expand Down
Loading
Loading