infra(app): deploy the frontend through a smoked candidate revision - #11207
Conversation
app/cloudbuild.yaml deployed straight onto live traffic while api/cloudbuild.yaml already had the candidate chain. This service is the one that carries the whole crawler path in app/nginx.conf — the $is_bot map, the `location =` bypasses, the @seo_proxy upstream — and that is the file whose breakage served every bot an HTTP 502 for four weeks in 2026 while humans, Plausible and CI all saw a healthy site. Until now a typo in it went live unchecked and the daily bot-serving monitor was the only net, a night later. The chain is now build -> push $BUILD_ID -> deploy --no-traffic --tag=candidate --revision-suffix=b$BUILD_ID -> smoke the candidate's tag URL -> update-traffic to exactly that revision -> push :latest. The smoke probes both halves of the split, only one of which a human ever sees: a browser UA must get <div id="root">, Googlebot must get the prerendered page. The marker is the canonical link — the SPA shell carries none at all, and its value names the route, so one grep proves both that the bot hop ran and that the right page came back. A deep route is probed alongside the home page, which covers the request URI reaching the upstream. robots.txt and llms.txt must come from the `location =` bypasses, and llms.txt must keep its utf-8 charset. Each probe fetches to a file instead of piping into `grep -q`: grep exits at the first match and SIGPIPEs curl, so the pipe form passes only because `bash -ceu` carries no pipefail. That also lets a failure name which probe failed and what it expected. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🟡 Changes recommended
The shared candidate tag has a race that can promote an untested revision, and several deployment guarantees are inaccurate.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a smoke-tested candidate rollout for the frontend Cloud Run service.
Changes:
- Deploys and probes a no-traffic candidate before promotion.
- Delays the
:latestpush until promotion. - Documents the deployment workflow.
File summaries
| File | Description |
|---|---|
app/cloudbuild.yaml |
Adds candidate deployment, smoke tests, and promotion. |
agentic/docs/project-guide.md |
Documents candidate rollouts. |
CHANGELOG.md |
Records the infrastructure change. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 5
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review findings on #11207: - The `candidate` tag is shared, so a concurrent build could move it between the check before the probes and the last probe — this build would then have smoked another revision while promoting its own. The smoke now re-asserts the tag after the probes. A competing build only ever tags its own revision and never ours back, so seeing our revision at both ends means every probe in between hit it. - The `:latest` comment claimed more than waitFor can deliver. It orders steps within one build; two overlapping deploys still race for the tag. What it does guarantee is that the tag never names an image that was not rolled out, and neither pipeline reads it — both deploy $BUILD_ID. - "mirrors api/cloudbuild.yaml step for step" was wrong in the header comment, the changelog and the project guide: the API still pushes :latest alongside the deploy and ends with get-url. All three now say "the same candidate-rollout pattern" and name the difference. - The project guide said a failed revision "never serves a request"; the smoke's own requests go to exactly that revision. It now says live traffic. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
Every bullet in the file cites its PR; this one was missing it. The entry also now mentions the post-probe candidate-tag re-assertion added in the review round. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
Review finding on #11207: the project guide claimed the candidate guarantee for both cloudbuild files, but api/cloudbuild.yaml checks the shared `candidate` tag only before its probes. Two overlapping API builds can therefore still promote a revision that was not the one smoked. The paragraph now says the guarantee holds unconditionally for the app and one-build-at-a-time for the API, and the "the chains differ" list carries the recheck as the second difference — with the note that the same four lines would close it on the API side. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
There was a problem hiding this comment.
🟡 Changes recommended
Reusing the candidate tag can smoke the previous revision while Cloud Run routing propagates.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Balanced
Review finding on #11207: both tag checks read status.traffic, which is the control-plane tag assignment, not proof of which revision answered a given request. Because `candidate` is reused across builds, a probe issued while a re-assignment is still propagating can reach the previous revision. Documented rather than closed, in the smoke step and in the project guide, with the two ways to close it named: a build-unique tag (tag URLs then accumulate on the service without bound) or a build id the served response itself carries. The residual is narrow — it needs the previous candidate to pass every probe too, so the worst case is promoting a revision believed smoked, not shipping a page known to be broken. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
There was a problem hiding this comment.
🔵 Needs a closer look
The production rollout path has not yet run, and one comment still overstates its revision-verification guarantee.
Review details
Suppressed comments (1)
app/cloudbuild.yaml:189
- This conclusion contradicts the control-plane limitation documented above: matching
status.trafficbefore and after only proves that the tag assignment did not change; it does not prove every HTTP probe reached this revision while routing propagated. Rephrase this so future maintainers do not mistake the recheck for serving-revision verification.
# while promoting its own. A competing build only ever tags its OWN
# revision and never ours back, so seeing our revision at both ends
# means every probe in between hit it.
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Balanced
CHANGELOG only; both sides add entries under Added and both are kept. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
The fragment convention landed on main in #11215 while this PR was open: a PR writes changelog.d/<slug>.md and leaves CHANGELOG.md alone. The one CHANGELOG.md edit that stays is not this PR's own entry but the correction of a PREVIOUS one — #11207's frontend bullet still says the API pushes :latest alongside the deploy, which is exactly what this PR changes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3
Measured, not assumed: with the correction in place, `tools.changelog check --base origin/main` answers "CHANGELOG.md [Unreleased] gained a bullet". The fragment gate compares bullet SETS, so editing an existing bullet is indistinguishable from adding one. So #11207's frontend entry keeps its now-stale parenthetical ("the API still pushes :latest alongside the deploy") for the moment. It needs either a skip-changelog-labelled touch or a pass at the release cut, where [Unreleased] is edited by hand anyway. Reported rather than forced. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3
…11212) `api/cloudbuild.yaml` invented the candidate rollout. `app/cloudbuild.yaml` then improved it in #11207, and the improvements never came back to the file they came from. Three of them do here — nothing else in the deploy changes. ## 1. `:latest` was published before the smoke `push-latest` waited on `build-image`. The tag therefore reached Artifact Registry before the candidate was even deployed, let alone probed: a build whose smoke failed still left `:latest` pointing at the image that failed it — and `:latest` is what a human reaches for when they want "the image that is running". It now waits on `promote`, exactly as the frontend's does. Still not a guarantee across concurrent builds (two overlapping deploys race for the tag), which is why the deploy step pulls `:$BUILD_ID` and never `:latest`; the comment says so at the step. ## 2. The `candidate` tag was checked once, not twice `candidate` is a shared tag. The smoke resolved it, asserted it named this build's revision, then probed — and a concurrent build moving the tag between that assertion and the last probe would leave this build promoting a revision it only *believed* it had smoked. The tag is now re-asserted after the probes. A competing build only ever tags its own revision and never ours back, so seeing our revision at both ends means every probe in between hit it. Same reasoning and same wording as `app/cloudbuild.yaml`. The API is the service where this matters most: its probes are the ones that touch Cloud SQL and the admin gate. ## 3. `curl … | grep -q` is a trap that had not sprung yet ``` curl -fsS $RETRY "${HDR[@]}" "$URL/libraries" | grep -q '"libraries"' ``` `grep -q` exits at the first match and SIGPIPEs curl. The pipeline only ever passed because `-ceu` carries no `pipefail` — the day someone adds `set -o pipefail` as a hardening pass, every deploy goes red for reasons that have nothing to do with the deploy. The probes now go through the same `expect` helper the frontend build uses: fetch to a file, `grep -qF`, and on failure say *which* probe failed and *what* was missing. `/health` keeps a bare variant (`expect_bare`) because reaching it **without** the origin header is precisely what its gate exemption has to prove. ## Verification No Cloud Build dry-run exists for this, so the script was checked out of band: - YAML parses; the bash step was extracted and expanded the way Cloud Build expands it (`$$` → `$`, substitutions resolved) — `bash -n` clean, no leftover `$$`. - The helpers were then run against a local HTTP server on three paths: happy path (`OK: /health`, `OK: /libraries`), a needle that is not in the body (`needle missing must fail (/libraries is missing: NOT_THERE)`, exit 1), and a 404 (`candidate did not serve /nope`, exit 1). - `HDR=()` expanded as `"${HDR[@]}"` under `set -u` was confirmed safe on bash 5.2 — the cloud-sdk builder's bash. The real proof is the next `deploy-api` build, which runs the whole chain. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3 --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
One line of `CHANGELOG.md`, so the `[Unreleased]` section describes one rollout state instead of two. #11207's frontend entry said: > …then `update-traffic` to exactly that revision (the chains are not identical — this one pushes `:latest` only after the promotion, **where the API still pushes it alongside the deploy**). #11212 is what changed that, so the sentence became false the moment it merged. Now: > …to exactly that revision (the chains **were** not identical **at the time** — this one **pushed** `:latest` only after the promotion, where the API still **pushed** it alongside the deploy; **the API caught up in #11212**). Past tense, and the closing PR named rather than a position: the release cut folds fragments in **above** the existing bullets, so "the entry above" would be wrong today and right after the cut, while `#11212` is true in both. ## Why `skip-changelog` rather than a fragment This corrects an existing entry; it is not a change worth a release-notes line of its own. And it *needs* the label, because the fragment gate compares bullet **sets** — an edited bullet is indistinguishable from an added one. Measured rather than assumed, when this same edit first rode along in #11212: ``` error: CHANGELOG.md [Unreleased] gained a bullet — it belongs in a fragment: - **The frontend deploys through a candidate revision instead of straigh… ``` That is the reason it was taken back out of #11212 and moved here. The `skip-changelog` label did not exist in this repository — #11215 documented it as the escape hatch without creating it — so it was created for this PR (grey, "No changelog fragment needed: this PR changes nothing worth a release-notes line"). The gate's own `if:` condition now has something to match. **Noted for later, not built here:** the gate could tell an *added* bullet from a *changed* one by treating the bold title as the bullet's identity, which would let a correction like this pass without a label. That is a sibling-pair change for both repos, deliberately out of scope tonight. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Why
api/cloudbuild.yamlalready deploys through a candidate revision (--no-traffic→ smoke →update-traffic, #10821).app/cloudbuild.yamldid not — it deployed straight onto live traffic.That is backwards, because the app service is the one carrying the whole crawler path in
app/nginx.conf: the$is_botmap, thelocation =bypasses for robots/llms/sitemap/og, the@seo_proxyupstream. That is the very file whose breakage served every bot an HTTP 502 for four weeks (2026-06-12 → 2026-07-09, aproxy_ssl_verify_depthdefault against a 4-deep Let's Encrypt chain) while humans, Plausible and CI all saw a healthy site. Until now a typo in it went live unchecked and the dailybot-serving-check.ymlwas the only net — a night later.The sibling repo kurrentschrift carries the same chain in its
app/cloudbuild.yaml; transferred here per the sibling rule.Every changed line
Header comment (new). Records the shared candidate-rollout pattern (and the two steps where the app and API chains differ) and why this service earns the same care as the API.
build-imagestep — unchanged args, gainsid: 'build-image'. Both tags are still built; only the push of:latestmoves.Push step — was
push --all-tags …/anyplot-app, now pushes only…:$BUILD_ID, withid: 'push-image'/waitFor: ['build-image']. A--all-tagspush before the rollout means:latestnames an image that may never have served a request.:latestgets its own step at the bottom, behindpromote.deploystep — every existing flag (--memory 512Mi,--cpu 1,--timeout 60,--min-instances 0,--max-instances 3,--port 8080, gen2,--cpu-throttling,--concurrency 15,--allow-unauthenticated) is unchanged, including the scale-to-zero decision from #10812. Three flags are added:--no-traffic— the revision goes up serving nobody.--tag=candidate— gives it a stable tag URL to probe.--revision-suffix=b$BUILD_ID— a deterministic revision name sopromotecan target exactly the revision this build smoked, never a concurrent build's newer one. Thebprefix is required: Cloud Run wants the suffix to start with a lowercase letter and$BUILD_IDis a UUID that usually starts with a digit.Plus
id: 'deploy'/waitFor: ['push-image'].smokestep (new). Resolves thecandidatetag's URL and asserts the tag points at this build's revision — before and again after the probes.candidateis a shared tag, so a concurrent build could move it mid-smoke and this build would then have smoked someone else's revision while promoting its own; a competing build only ever tags its OWN revision and never ours back, so seeing our revision at both ends means every probe in between hit it. Between them, six probes, every one retried (--retry 5 --retry-delay 5 --retry-all-errors) because atmin-instances 0the candidate is always cold and the crawler probes additionally wait on the first upstream call toapi.anyplot.ai:/<div id="root">/<link rel="canonical" href="https://anyplot.ai/" />$is_bot→@seo_proxyhop ran/scatter-basic…href="https://anyplot.ai/scatter-basic" />/robots.txtUser-agent: Bytespiderlocation =bypass, not the proxy/llms.txt# anyplot/llms.txtcharset=utf-8The canonical link is the prerender marker. anyplot has no committed prerender files and no marker comment, but the SPA shell (
app/index.html) carries no<link rel="canonical">at all, while everyapi/routers/seo.pypage emitshref="https://anyplot.ai{route}"— so onegrep -Fproves both "this is the prerendered page" and "it is the right route". Verified live on all twelve current bot routes (see below).promotestep (new).gcloud run services update-traffic --to-revisions=anyplot-app-b$BUILD_ID=100, never--to-latest(which could promote a concurrent build's unsmoked revision).push-lateststep (new).:latestmoves only after this build promoted, so the tag can no longer name an image that was never rolled out (which the old--all-tagspush before the deploy did). It is not a cross-build guarantee — two overlapping deploys still race for the tag — which is fine, because neither pipeline reads:latest: both deploy:$BUILD_ID.timeout: '1200s'(new). deploy → cold-start smoke → promote does not fit the default 10 min with a retrying probe. Matchesapi/cloudbuild.yaml.images:andoptions:are unchanged.Evidence
The smoke script was expanded through the Cloud Build substitution rules (
${_VAR}/$BUILTINreplaced,$$→ literal$applied last so an escape can never be re-read as a substitution), then syntax-checked and executed:bash -non the expanded script: clean.$$escapes): none. This matters —$request_uriappears in a comment inside the script and had to be written$$request_uri; an unescaped one fails the build before a single step runs, and YAML comments (which is where$is_botsits) are stripped before substitution and are safe.anyplot-app(reads only, no deploy, no writes):/,/plots,/specs,/libraries,/legal,/mcp,/about,/palette,/map,/stats,/scatter-basic,/scatter-basic/python/matplotlib— all200, all withhref="https://anyplot.ai{route}", none with<div id="root">.What the tag check does not prove
status.trafficis the control-plane tag assignment, not proof of which revision answered a given request.candidateis reused across builds, so a probe issued while a re-assignment is still propagating can reach the previous revision. Closing that needs either a build-unique tag (tag URLs then accumulate on the service without bound) or a build id the app serves in its own response (anapp/Dockerfile+ Vite change). Neither is taken here; both the smoke step's comment andagentic/docs/project-guide.mdnow say so. The residual is narrow and its worst case is mild — it needs the previous candidate to pass every probe too, so the failure would be "promoted a revision we believed we smoked", not "shipped a page we know is broken", and it is still strictly better than the no-smoke chain this replaces.Decisions taken here (routine, flagged for override)
grep -q.grep -qexits at the first match and SIGPIPEs curl, so the pipe form (whichapi/cloudbuild.yamland the kurrentschrift original both use) passes only becausebash -ceucarries nopipefail. Addingpipefail— a normal hardening instinct — would red every deploy. I hit exactly that while testing. The file form is correct under any shell options, checks curl's own exit status, and lets each failure name which probe failed and what it expected. This is the one place the file deliberately does not mirrorapi/cloudbuild.yamlbyte for byte; happy to align if you'd rather have strict parity.bot-serving-check.ymlhas a comment recording ten red nights from exactly that); the canonical is generated from the route and cannot drift.location /mechanism, so the deep route is not about coverage of the map — it is the only probe that would notice$request_urifailing to reach the upstream.api/cloudbuild.yamluntouched. Itspush-lateststill waits onbuild-imagerather thanpromote, so the API's:latestcan still name an image that never served. Left for the author / the parallelapi/work rather than taken here — see below.Findings for the author
api/cloudbuild.yaml'spush-latestsits behindbuild-image, notpromote. Same one-line fix as here (waitFor: ['promote']); I stayed out of that file to avoid colliding with the parallel work inapi/.api/cloudbuild.yamlhas the same shared-tag race this PR just closed — it resolvescandidateonce before its probes and never re-checks, so two overlapping API builds can promote a revision that was not the one smoked. The fix is the four lines added here; same reason for not applying it in that file.agentic/docs/project-guide.mdnow records the asymmetry explicitly rather than claiming the guarantee for both pipelines.--no-traffic/--tag=candidate/update-trafficon theanyplot-appservice happens on the first deploy after merge. Worth watching that one build.🤖 Generated with Claude Code
https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke