Skip to content

fix(deploy): anchor rollback before writes and read the terminal live version - #508

Merged
bytemain merged 2 commits into
mainfrom
fix/deploy-rollback-provenance
Sep 4, 2026
Merged

fix(deploy): anchor rollback before writes and read the terminal live version#508
bytemain merged 2 commits into
mainfrom
fix/deploy-rollback-provenance

Conversation

@stdrc

@stdrc stdrc commented Sep 3, 2026

Copy link
Copy Markdown
Member

Repairs the deploy-receipt and rollback-anchor defects measured on production run 33790593239 (task #190). Opened as Draft: this needs an independent reviewer, and I am the author.

What was wrong

One protected deploy creates ten Cloudflare versions — one wrangler deploy plus one per wrangler secret put (9 secrets). Two consequences, both of which produce a receipt that reads correctly while naming the wrong thing:

measured on run 33790593239
deploy step printed cd3f5f6d…
actually live c8cbb03b… (created 23s later by the last secret put)
/deployments all 10 entries from this one run; pre-run version evicted
real rollback anchor 5fa9dd39…, surviving only in /versions

The list's second entry is another version of the same run, so a rollback to it is a no-op wearing a rollback's clothes.

What this changes

  • Capture pre-deploy rollback anchor — runs before the first Worker write, while "currently live" is still unambiguous, and cross-checks the anchor against /versions so it stays recoverable after this run flushes the deployment list. Fails closed if either read fails or the anchor is absent.
  • Read back the terminal live version — runs after the last secret put; requires 100% traffic and requires the live version to differ from the anchor, so "nothing deployed" cannot pass as success.
  • Both values land in the job summary, labelled so the anchor is not mistaken for a list index.

Tests

worker/test/deploy_rollback_provenance.test.ts asserts against the real workflow file, so a moved step re-binds the test rather than silently passing a stale copy.

Mutation-tested, each red for its own reason: anchor moved after the deploy; terminal read moved before the secrets; fail-closed exits removed; durability check deleted.

⚠️ Worth knowing during review: the durability assertion initially passed the fourth mutation, because it matched /versions where the string appears in the step's own prose rather than in executable text. It now requires the curl and the fail-closed branch. Same class of defect as the thing being fixed — a check that reads right while pointing at the wrong text.

Not addressed here

The card also asks whether the unconditional re-put of unchanged secrets can be avoided. I did not attempt that: the values are unreadable to me, so I cannot show a given put is a no-op, and skipping a secret write on an unverifiable assumption is a worse failure than an extra version. Left as a separate decision with the evidence recorded.

No production dispatch, rollback, secret/config mutation, deploy or release is performed by this change.

🤖 Generated with Claude Code

Gogo and others added 2 commits September 4, 2026 02:37
… version

Production run 33790593239 measured that one protected Hands deploy creates ten
Cloudflare versions: one `wrangler deploy` plus one per `wrangler secret put`.
Two receipts were wrong as a result, both in ways that read as correct:

- The run printed `Current Version ID: cd3f5f6d…` from the deploy step, but the
  terminal live version was `c8cbb03b…`, created 23s later by the final secret
  put. Recording the deploy step's id records a version that was never live.
- /deployments returns ten entries, so a single run fills it end to end and
  evicts the pre-run live version. The list's second entry is another version of
  the same run, so rolling back to it is a no-op that looks like a rollback. The
  real anchor survived only in /versions.

Capture the anchor before the first Worker write, while "currently live" is
unambiguous, and verify it exists in /versions so it is still recoverable after
this run flushes the deployment list. Fail closed if either read fails or the
anchor is absent. Read the deployed version back after the last secret put and
require 100% traffic and a change from the anchor.

Guards are asserted against the real workflow file so a moved step re-binds the
test. Mutation-tested: anchor after deploy, terminal read before the secrets,
missing fail-closed exits, and deleting the durability check each go red for
their own reason. The last of those initially passed because the assertion
matched /versions in the step's prose rather than in executable text; it now
requires the curl and the fail-closed branch.

No production dispatch, rollback, secret or config mutation is performed by this
change; it only changes what the workflow records and when it stops.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Huarong found the fail-closed tooth did not guard what it claimed. It asserted
the error text `absent from /versions` AND that *some* `exit 1` existed anywhere
in the anchor step. Those are independent conditions, so deleting the terminating
action of one guard — keeping its branch and its message — left the suite green.

The runtime consequence was real: with that exit removed the anchor-absent branch
printed "unrecoverable" and then fell through to the first Worker write, which is
exactly what the frozen contract forbids.

Each of the four rejection exits is now scoped to its own branch (message → the
`fi` that closes it) and asserted to terminate the job. Deleting any one of them
individually now goes red for its own reason:

  deployments read failure      1 failed / 7 passed
  empty live anchor             1 failed / 7 passed
  versions read failure         1 failed / 7 passed
  anchor absent from /versions  1 failed / 7 passed   <- previously 5/5 GREEN

The three original mutations still fail independently (anchor after deploy;
executable /versions curl replaced while prose kept; terminal read before the
secret puts). Full worker suite 55 files / 572 tests green.

This is the second time this file matched prose rather than executable text —
the first was mine, caught during authoring; this one needed a reviewer.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@bytemain
bytemain marked this pull request as ready for review September 4, 2026 07:24
@bytemain
bytemain merged commit 41bab8e into main Sep 4, 2026
11 checks passed
stdrc pushed a commit that referenced this pull request Sep 4, 2026
Review feedback on #512: a script that only runs by hand is not a check.

- workflow-lint now runs scripts/test_deploy_rollback_anchor_jq.sh, and the
  script's own path is in the workflow's pull_request paths, so editing the
  test re-triggers the check that consumes it.
- The script self-tests against the pinned #508 expression before reporting.
  A table that cannot fail is indistinguishable in the log from a table that
  passed, so the known-bad expression must fail all three positive cases first
  or the run exits 2 without reporting. This mirrors the known-bad fixture
  pattern already used by this workflow.
stdrc pushed a commit that referenced this pull request Sep 4, 2026
…nse shape

The rollback-anchor durability check shipped in #508 as

    [.result.items[]?.id, .result[]?.id] | index($a)

which reads like "tolerate both shapes" but is a hard jq error on both. On
the real {"result":{"items":[...]}} shape, .result[]? iterates the object's
VALUES, yields the items ARRAY, and .id on an array aborts with
"Cannot index array with string". The step is fail-closed, so it refused
every deploy after #508 — run 33887600418 (PR #511 landing) died there with
no migration or Worker write, and it was the first deploy to reach the new
code, so the guard has never once passed.

Parse the two shapes explicitly instead, and guard the .id access itself.

scripts/test_deploy_rollback_anchor_jq.sh extracts the expression from the
workflow rather than copying it, so the test cannot drift away from what
ships, and covers both response shapes plus the absent-anchor and
empty-versions cases the guard exists to catch. Against the old expression
the three should-pass cases fail.
stdrc pushed a commit that referenced this pull request Sep 4, 2026
Review feedback on #512: a script that only runs by hand is not a check.

- workflow-lint now runs scripts/test_deploy_rollback_anchor_jq.sh, and the
  script's own path is in the workflow's pull_request paths, so editing the
  test re-triggers the check that consumes it.
- The script self-tests against the pinned #508 expression before reporting.
  A table that cannot fail is indistinguishable in the log from a table that
  passed, so the known-bad expression must fail all three positive cases first
  or the run exits 2 without reporting. This mirrors the known-bad fixture
  pattern already used by this workflow.
bytemain pushed a commit that referenced this pull request Sep 4, 2026
The #508 durability jq treated .result[]? as both shapes and aborted on
the real paged items array. Parse array vs items explicitly and guard .id.

Signed-off-by: Codex-Android-DevOPS <raft-mobile-codex-android-devops@mail.build>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants