Repository navigation
ci: ramp production from Actions, and stop trusting an empty variable - #351
Conversation
Wire-size diff vs merge base
Brotli q11, the quality the edge serves. Every number here is deterministic: Client assetsNo change. 19 files, 68.07 KiB Brotli total. PagesNo change. 48 files, 492.15 KiB Brotli total. Worker bundleNo change. 276.59 KiB gzip. Dictionary deltas (
|
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
aadhar-sh | 1afac19 | Aug 12 2026, 09:10 PM |
Four release steps ran on a workstation or not at all. Three of them move to the
repo here.
RAMP. `.github/workflows/ramp.yml` is new, and CLAUDE.md has described it since
2026-08-06 without it ever existing: no such workflow in any commit, environments
`copilot`/`production` rather than the canary/full pair, and no write token. What
landed that day was the `release-guard.mjs` rewrite alone, which asks whether a
process can authenticate instead of whether it is CI. That is what makes this
possible; the pipeline it cleared the way for is what was missing.
It splits at the human rather than removing them:
canary 10% production-canary, no reviewer, runs on its own
full 50% -> 100% production-full, REQUIRED REVIEWER, waits for a click
verify none no environment, no credential, runs dcz:check
A merge reaches a tenth of traffic by itself and stops. Approving `full` is the
same decision as before, minus the terminal.
Three parts of it are load-bearing:
- It checks out `production`, never `main`. The ramp writes the D1 changelog by
diffing the checked-out checkpoints.json against D1, so the tree has to be
the one that was built. This is gotcha 24 solved rather than moved: CI cannot
ramp from a stale tree, nor from one running ahead of what is serving.
- `full` refuses a version the canary never saw. Approval is asynchronous and
Workers Builds uploads a version for every push, so re-resolving the newest
build after an approval could put traffic on something nothing canaried,
silently, with every downstream check passing because a version that built
returns 200s.
- Every step runs through `pnpm run`. deploy-promote.mjs shells out to `npx
wrangler`, and npx resolves whatever it finds: measured in a tree with no
node_modules, `npx wrangler --version` answered 4.105.0 from its own cache
while this repo pins 4.120.0.
Both environments restrict deployments to `main`, which is not tidiness:
`workflow_dispatch` can target any branch carrying the workflow file, so without
a branch policy anyone able to push a branch could ship a modified ramp.yml and
read the token out of it.
CHECKPOINTS. `checkpoints:check` was deliberately unwired ("NOT wired into the PR
job by default", its own header) because it needs a D1 read. It needs no NEW
credential: D1:Read is already one of the six scopes on CLOUDFLARE_API_TOKEN.
Fatal on drift, which is safe because the script already tells a STAGED release
apart from a rewrite, so a PR staging v187 passes. Tiered on the token the way
infra:check is, since failing a fork PR for not holding a secret is a worse check
than no check.
OG CARDS. `og-cards.yml` bakes the cards on a runner and opens a PR.
Dispatch-only, never scheduled: the cards capture production so data-driven demos
render populated, and the photo grid, live counters and routing prober are not
deterministic between runs, so a nightly job would propose a PR of changed PNGs
every night that meant nothing. dictionary-roll.yml can be nightly because its
output is a pure function of what production serves; this is not.
AN EMPTY VARIABLE, found on the way. ci.yml passed
`CLOUDFLARE_ACCOUNT_ID: ${{ vars.CLOUDFLARE_ACCOUNT_ID }}` to infra:check and NO
repository variable of that name has ever existed (`gh variable list` returns
nothing), so it expanded to "" on every run. It reads
scripts/print-account-id.mjs now, which cannot be silently empty and cannot drift
from the account the deploy targets, because it IS that string.
This is HARDENING, not a repair, and the difference is worth stating because the
first version of this message got it wrong. checkApi() falls back to GET
/accounts and only degrades when the count is not exactly 1; the scoped CI token
sees exactly one, so the account tier was asserting normally. Measured: `infra
ok: 44 checks passed, 1 skipped or advisory` both before and after, with the one
advisory being the documented CodeQL default-setup note and no `token sees N
accounts` warning in either run. The two-account fact belongs to the owner's
interactive wrangler login, which is a DIFFERENT credential from the CI token,
and conflating them is what produced the wrong claim.
What the helper is genuinely required for is the ramp: release-guard.mjs returns
an error unless CLOUDFLARE_ACCOUNT_ID is set in CI, unconditionally, so the
canary and full jobs cannot authenticate without it.
What did NOT need building, checked rather than assumed: md twins are build
output regenerated every build, with checkTwinFacts pinning the hand-written ones
in both directions; photos already have photo-pipeline.yml plus photos:check in
CI; dictionaries already have dictionary-roll.yml.
Environments are created but NOT declared in infra.json, which leaves the
required reviewer on the honour system. Asserting them in check-infra.mjs is the
follow-up and is deliberately not bundled with the change that created them.
244 contract tests, typecheck, build and pages:check all pass; checkpoints:check
passes against live D1 (122 released, newest v186).
e9b7629 to
ba91cc3
Compare
Four spots interpolated an expression directly into a `run:` block. A ${{ }}
expansion is substituted into the script TEXT before bash parses it, so a value
carrying shell metacharacters executes. `inputs.base` in og-cards.yml is the one
that matters: a URL somebody types into the dispatch form.
Dispatching needs write access, so this is hygiene rather than a hole, and the
other three values were already constrained (a hex prefix the canary matched with
a regex, and a literal true/false). Fixed all four anyway, because the safe shape
should be the habit rather than a judgement call per value.
Found by reading the diff, not by the security check: github-advanced-security
failed on this PR for the eighth recorded time with `CAPIError: 400 The requested
model is not supported`, so Copilot's agent crashed before reading anything (see
gotcha 27). `Analyze (actions)` passed and flagged none of this.
On the two bot commentsNeither needs a change, and one of them is good news.
On
|
Three more occurrences on 2026-08-12, all the same CAPIError: 400, taking it to
nine recorded. One landed on a commit whose entire diff moved ${{ }} expressions
out of run: blocks and into env:, which strictly reduces attack surface — so a red
mark here carries no information about the diff even in the direction it claims to.
Those three cost three separate investigations in one session, which is the actual
bill for a check that gates nothing. So the note now ends with the short loop:
confirm the run id is new, grep for CAPIError, stop. Do not re-read the diff, and
do not push to appease it. Names the settings-side fix for when the noise stops
being worth it, and records that CodeQL and Analyze (actions) are the checks doing
the real work.
Also records what Analyze (actions) does NOT catch, since it passed on the
${{ inputs.base }} interpolation fixed in a76ea18: the CodeQL Actions analyzer
treats workflow_dispatch inputs as trusted, so that injection class is on the
author to read for.
gotcha 29 says the repo moved off npx because npx FETCHES what it cannot resolve
locally, which is a hole in a repo that pins wrangler exactly and enforces one
version on every PR. It also says "all 30 call sites" were converted. Eight were
not:
scripts/check-checkpoints.mjs d1 execute
scripts/deploy-promote.mjs the RAMP
scripts/lens-webmcp.mjs browser create + close
scripts/perf-budget.mjs deploy --dry-run, check startup
scripts/perf-snapshot.mjs deploy --dry-run
scripts/release-status.mjs versions list
Each spawned execFile("npx", ["wrangler", ...]), so no grep for `npx wrangler` as
a phrase could find them: the command is assembled from array elements and only
the word "npx" appears as a token. Same blind spot the holding/ rename hit from
the other direction, and the rule that generalises is to grep for "npx" quoted
rather than npx <binary> as a phrase.
deploy-promote.mjs was the one that mattered: the wrangler moving production
traffic depended on how the ramp was invoked. Under `pnpm run` it got the pin;
from a tree with no node_modules it got whatever npx had cached, measured at
4.105.0 against a pinned 4.120.0.
pnpm exec, not pnpm dlx. dlx is npx's true twin and would fetch from the registry
and ignore the pin, which is the hole rather than the fix; wrangler is a pinned
devDependency, so exec is correct. The three npx strings in infra.json and its
mirrors still stand on purpose: they RECORD dashboard values, and editing them
alone fails infra:check on drift it invented itself.
Exercised rather than assumed, since a bad spawn surfaces as a missing binary at
the worst possible moment: checkpoints:check reads D1 (122 released, newest v186),
perf-budget runs both wrangler calls and reports hard checks green, release-status
lists versions. 244 tests and the build pass.
Two comments I wrote hours ago went stale with this and are corrected: ramp.yml
and CLAUDE.md both explained using `pnpm run` as a guardrail against npx, which is
now consistency rather than protection.
This repo has said `Workers Scripts:Edit` + `D1:Edit` "and nothing else" since 2026-08-06, and since no such token existed until now the list was never checked against a running ramp. Derived from what deploy-promote.mjs actually executes: Account Workers Scripts Edit versions list, deployments status, versions deploy Account D1 Edit SELECT shipped vnums, INSERT the changelog row Account Account Settings Read wrangler resolves the account User User Details Read wrangler identifies itself non-interactively User Memberships Read same The bottom three are wrangler's own identity calls rather than anything the ramp asks for, which is why a two-scope list read as complete. Narrower than the token Workers Builds generates for itself in one place worth recording: no Workers Routes, no KV, no R2. A ramp shifts traffic between versions that already exist and never uploads one. Also documents the control (CI=1 matters, or release-guard ignores the token and falls back to the interactive login, so the check tests the wrong credential) and the one failure that is silent: the changelog INSERT runs only after 100% and is caught rather than fatal, so a missing D1:Edit costs a changelog gap on a ramp that reports success. checkpoints:check is the backstop and goes red on the next PR.
gotcha 27 said the fix for the noise was in GitHub's settings and named Copilot Autofix. Measured: the owner turned it off and the very next push failed identically, 12th recorded occurrence. The toggle did land, and the log shows it changed something real: COPILOT_AGENT_ONLINE_EVALUATION_DISABLED: true [skills] session=github/code-scanning enabled=false source=disabled and the job ran 29s instead of ~80s. It then crashed on the same CAPIError: 400 regardless. So the setting is not the off switch, and which setting is (if any) is now recorded as UNKNOWN rather than guessed at a second time. This is the second wrong claim I have had to walk back in this note, both from asserting a cause without measuring it. The note now says what is known: the check configures nothing in this repo, it gates nothing, CodeQL and Analyze (actions) do the real scanning and stay green.
The scope table listed five permissions as though equally established. Two are not: User Details:Read and Memberships:Read were inferred from the token Workers Builds generates for itself, not from anything deploy-promote.mjs is known to call, and CLOUDFLARE_ACCOUNT_ID is passed explicitly so wrangler need not enumerate anything. Marked SPECULATIVE, with the guidance to grant the three Account rows and run the control instead of collecting all five. Also records why they look absent from the dashboard: User-scoped permissions exist only on USER-owned tokens (My Profile, API Tokens). An account-owned token created from Manage Account does not offer the category at all, which reads like a missing option and is a token-type difference.
Verified with a real token: CI=1 deploy:promote --dry-run resolved a target, printed the current split, and skipped two non-production versions by alias, on Workers Scripts:Edit + D1:Edit + Account Settings:Read alone. So the two User rows come out. They were inferred from the token Workers Builds generates for itself rather than from anything the ramp calls, they are unnecessary because CLOUDFLARE_ACCOUNT_ID removes the account-enumeration call they would have served, and they are unavailable on an account-owned token — so listing them sent somebody hunting for a dashboard category that was never there. Records what the control does NOT prove, since it only reads: a token holding Workers Scripts:Read would satisfy it too. versions deploy and the D1 INSERT are first exercised on a real ramp, and the D1 one is caught rather than fatal. Third correction to this token note in one session, all three from asserting a cause before measuring it.
Renaming or moving a page turned every page LINKING to it into a 404, and nothing in this repo noticed. routes:check sweeps the routes it is TOLD about, which is the forward direction. Invariant #1 asserts the Worker's routes reach run_worker_first. Neither one reads an href. scripts/lib/link-integrity.mjs is the resolver, run as a build invariant over the MINIFIED tree, since those are the bytes a visitor gets and the minifier rewrites asset refs to their hashed /a/ form. COMPLETE rather than scoped to the diff, because it costs nothing to be: 2645 refs across 48 documents in ~45ms. A diff-scoped version would be more code and would miss the case that matters most, where the moved page is not in the diff and its dependents are. No second copy of the route table. The inputs are exactly what invariant #1 already extracts (the ROUTES keys, run_worker_first's globs) plus the surface registry and the staged tree, so a route added anywhere is understood here with no matching edit. Two findings from building it, both now load-bearing: run_worker_first cannot be the resolver on its own. It answers whether the Worker SEES a request, not whether a path SERVES a page, and it carries /garage/* and /lwe/* — the namespaces holding most of the site's pages. Measured on a deliberately broken ref, a glob-only resolver called /garage/renamed-page fine. So a namespace that already holds registered surfaces is GOVERNED by the registry and the glob buys it nothing there. The governed set is derived from site-manifest.json, so adding a section governs it with no edit. The scanner has to be quote-aware. minify-html unquotes attributes, so the served bytes carry href=/coffee far more often than href="/coffee", and the first draft read 33 refs where there are 2645 and passed with a straight face. Third naive scanner this repo's minified output has caught. Tested for teeth rather than agreement, since the tree currently has zero dangling refs and a test asserting only "the real site passes" would survive the resolver being reduced to () => true. Three unit tests cover accept, reject and the scanner; end to end, injecting <a href=/garage/renamed-page> into www/index.html fails the build with the path and the referring document named. Non-goals recorded in CLAUDE.md: prose mentions of a path (this repo discusses deliberately deleted routes, so that would false-fire on its own history), the Run palette and nav fences (generated from the registry, asserted by invariant #8), and off-origin links. 247 tests, build, pages:check, typecheck and 130 routes all pass.
…verse (#416) This gotcha claimed twice that the Copilot check fails on every diff, and one of those claims went in a few hours ago as the general lesson. It is wrong. The run always fires; whether it REQUESTS A MODEL depends on the diff. The job prints a file exclusion list on every run, covering js, json, mjs, ts, py, rs, yaml, yml, html, xml, go, java, c/cpp, rb and more. `.md`, `.sh` and `.css` are NOT on it. So a docs diff survives the filter and gets a `claude-opus-4.6` request, while a pure JavaScript diff is filtered to nothing and the run exits clean. Both paths print `Sessions disabled: not supported for code scanning yet`, which this session initially read as the discriminator and is not; the discriminating line is `Creating copilot-sdk session with model`, which only the failing path reaches. Correlates 12 of 12: the five runs of 2026-08-15 (#399 jpg/json and #402 js/mjs passed; #398, #400 and #401 all carried md and failed), plus all seven earlier failures checked backwards. #313 is the "CSS-only" diff this gotcha already singles out, and css is not on the list either. #351's note here called its entire diff a `${{ }}` move in workflow files; it touched md and mjs too, and the md is what drew the request. The prose-only argument is unchanged in conclusion and inverted in mechanism: a docs PR reddens this because markdown is one of the few things the detector still looks at, rather than because the agent choked on prose. The entitlement root cause is untouched. Records the method failure too, since it is the transferable part. Nine entries were built by pattern-matching outcomes across PRs without once reading which files each diff touched, and "fails on everything" was an artifact of that sample. One uncollected column explained the whole shape. Co-authored-by: Aadharsh Pannirselvam <19518661+oddharsh@users.noreply.github.com>
Four release steps ran on a workstation or not at all. Three move to the repo here.
The ramp
.github/workflows/ramp.ymlis new, and CLAUDE.md has described it since2026-08-06 without it ever existing: no such workflow in any commit, environments
copilot/productionrather than the canary/full pair, and no write token. Whatlanded that day was the
release-guard.mjsrewrite alone, which asks whether aprocess can authenticate rather than whether it is CI. That rewrite is what makes
this possible; the pipeline it was clearing the way for is what was missing.
It splits at the human instead of removing them:
canaryproduction-canaryfullproduction-fullverifydcz:checkA merge reaches a tenth of traffic by itself and stops. Approving
fullin theActions tab is the same decision you were making at a terminal. A workstation ramp
keeps working unchanged, and rollback stays a workstation command.
Three parts are load-bearing rather than incidental:
production, nevermain. The ramp writes the D1 changelog bydiffing the checked-out
checkpoints.jsonagainst D1, so the tree has to be theone that was built and uploaded. That is gotcha 24 solved rather than relocated:
CI cannot ramp from a stale tree, and cannot ramp from one running ahead of
what is serving either.
fullrefuses a version the canary never saw. Approval is asynchronous andWorkers Builds uploads a version for every push, so re-resolving the newest
production build after an approval could put traffic on something nothing
canaried, silently, with every downstream check passing because a version that
built returns 200s.
pnpm run.deploy-promote.mjsshells out tonpx wrangler, and npx resolves whatever it can find: measured in a tree with nonode_modules,npx wrangler --versionanswered 4.105.0 from its own cachewhile this repo pins 4.120.0. Under
pnpm run,node_modules/.bincomesfirst.
Both environments restrict deployments to
main, which is not tidiness.workflow_dispatchcan target any branch carrying the workflow file, so without abranch policy anyone able to push a branch could ship a modified
ramp.ymlandread the token out of it.
checkpoints:check
Deliberately unwired until now ("NOT wired into the PR job by default", its own
header) because it needs a D1 read. It needs no new credential:
D1:Readisalready one of the six scopes on
CLOUDFLARE_API_TOKEN.Fatal on drift, which is safe because the script already tells a staged release
apart from a rewrite: rows above D1's newest that form a contiguous tail report as
pending. So a PR staging v187 passes, and only a row rewritten below D1's newest,
or a broken sequence, fails. Tiered on the token the way
infra:checkis, sincefailing a fork PR for not holding a secret is a worse check than no check.
og-cards
og-cards.ymlbakes the cards on a runner and opens a PR, with anonlyinputmapping to
OG_ONLY.Dispatch-only, never scheduled. The cards capture production so data-driven
demos render populated, and the photo grid, live counters and routing prober are
not deterministic between runs, so a nightly job would open a PR of changed PNGs
every night that meant nothing.
dictionary-roll.ymlcan be nightly because itsoutput is a pure function of what production serves; this is not that.
An empty variable, found on the way
ci.ymlpassedCLOUDFLARE_ACCOUNT_ID: ${{ vars.CLOUDFLARE_ACCOUNT_ID }}toinfra:check, and no repository variable of that name has ever existed (gh variable listreturns nothing), so it expanded to""on every run. It readsscripts/print-account-id.mjsnow.This is hardening, not a repair, and an earlier version of this PR body claimed
otherwise.
checkApi()falls back toGET /accountsand only degrades when thecount is not exactly 1; the scoped CI token sees exactly one, so the account tier
was asserting normally. Measured on either side of this change:
infra ok: 44 checks passed, 1 skipped or advisory, with the one advisory being the documentedCodeQL default-setup note and no
token sees N accountswarning in either run.The two-account fact belongs to the owner's interactive wrangler login, which is a
different credential from the CI token, and conflating them is what produced the
wrong claim.
What the helper buys here: a value that cannot be silently empty, cannot drift
from the account the deploy targets (it is that string), and one fewer API call.
Where it is genuinely required is the ramp —
release-guard.mjsreturns anerror unless
CLOUDFLARE_ACCOUNT_IDis set in CI, unconditionally, so the canaryand full jobs cannot authenticate without it.
What did not need building
Checked rather than assumed:
checkTwinFacts()pinning the hand-written ones against the Worker in both directions.
photo-pipeline.ymlplusphotos:checkin CI.dictionary-roll.yml.Before merging
You have to mint the token; I can't. Create a Cloudflare API token scoped to
Workers Scripts:Edit+D1:Editand nothing more, then add it asCLOUDFLARE_API_TOKEN_RAMPon both environments (Settings, Environments) —never as a repo secret. Until it exists the
canaryjob fails at the firstwrangler call, having moved nothing.
The two environments are already created and configured:
production-fullhas youas required reviewer, both are restricted to
main.One gap left on purpose. The environments are not declared in
infra.json, sodeleting the required reviewer would remove the gate silently, which is exactly the
failure the ruleset declarations exist to catch. Asserting
environmentsincheck-infra.mjsis the follow-up, kept out of the change that created them.244 contract tests, typecheck, build and
pages:checkpass;checkpoints:checkpasses against live D1 (122 released, newest v186).