Skip to content

Correct the record: zero-copy promotion is already unblocked - #267

Merged
brad-richardson merged 2 commits into
mainfrom
feat/v2-aware-slice-delete-guard
Aug 7, 2026
Merged

Correct the record: zero-copy promotion is already unblocked#267
brad-richardson merged 2 commits into
mainfrom
feat/v2-aware-slice-delete-guard

Conversation

@brad-richardson

Copy link
Copy Markdown
Owner

The state doc says item 1's precondition is "two changes, not one" and concludes "until both land, keep release_slice_version EMPTY". That was true when written and is now stale — and it's blocking a decision worth ~158 GiB and ~123 minutes on every promotion.

Both halves already exist

  1. A v2-aware unreferenced checkscripts/v2_retention_guard.py (8e4e0fc) resolves v2/catalog.jsonv2/releases/{build}/release.json → every family source and retained external operation_sources. It scans recursively rather than enumerating fields (enumerating is precisely how the v1 guard came to miss the v2 chain) and fails closed on a missing release document, a byte-mismatched one, an unreadable catalog, or a partial target.
  2. A phase that can match a slice prefix — phase 3 emits a bare <prefix>/ target for each ORPHAN_PREFIXES entry with no version-format restriction, and the guard accepts slice-YYYY-MM-DD.N as a bucket-root prefix. r2-cleanup.yml fetches the whole chain before any delete and calls the guard in both bucket-root delete phases.

What this PR adds

I started building the v2-aware guard before checking whether one existed — it did, in a better form than the one I was writing. The durable fix for that is a test, not a note: the guard only protects what calls it, so this adds contract tests asserting both delete phases consult it, that the chain is fetched before any delete, and that a slice prefix is a legal phase-3 target. Plus the state-doc correction in three places.

What remains

A decision, not engineering. The slice smoke already promotes both layouts on every change and asserts byte-identical published routing.json and family manifests (0bf477e); Monaco measured 21 objects copied under the construction layout against 0 copied / 21 prepositioned under the release layout.

Full suite: 1,812 passed, 2 skipped.

🤖 Generated with Claude Code

brad-richardson and others added 2 commits August 7, 2026 03:36
The state doc says item 1's precondition is "two changes, not one" and
concludes "until both land, keep `release_slice_version` EMPTY". That was
true when written and is now stale, and it is actively blocking a decision:
the forward promotion it gates copies ~158 GiB and takes ~123 minutes.

Both halves exist. `scripts/v2_retention_guard.py` (8e4e0fc) resolves the v2
chain -- catalog to release documents to every family source and retained
external operation source -- scanning recursively rather than enumerating
fields, because enumerating is exactly how the v1 guard came to miss the v2
chain. It fails closed on a missing release document, a byte-mismatched one,
an unreadable catalog, or a partial target. And phase 3 already emits a bare
`<prefix>/` target for anything in ORPHAN_PREFIXES with no version-format
restriction, while the guard accepts `slice-YYYY-MM-DD.N` as a bucket-root
prefix.

I went to build the v2-aware guard before checking whether it existed, and it
did, in a better form than the one I was writing. The lasting fix for that is
a test rather than a note: the guard only protects what calls it, so this
adds contract tests asserting both bucket-root delete phases consult it, that
the whole chain is fetched before any delete, and that a slice prefix is a
legal phase-3 target.

What remains before turning `release_slice_version` on is a decision, not
engineering. The slice smoke already promotes both layouts on every change
and asserts byte-identical published routing and family manifests; Monaco
measured 21 objects copied against 0 copied / 21 prepositioned.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
release-slice-families.yml's cleanup job deleted
s3://geocoder-shards/${SLICE_VERSION}/ --recursive behind nothing but a
shape regex -- the exact shape zero-copy promotion publishes live
serving objects into. It is always(), so it ran even when preflight
failed and every build job was skipped, and its one reference check
walks the v1 catalog, which reports 'not referenced' for a live
promoted slice by construction.

It now fetches the v2 chain and calls the same guard r2-cleanup does,
before deleting.

The two wiring tests were substring counts, which is decoration: they
stayed green under || true on the guard call, continue-on-error on its
step, reordering the guard after the delete, and adding a fresh
unguarded delete. Replaced with structural checks over the parsed step
order, and mutation-checked against all four.

Also corrects this section's claim that the chain is fetched 'before any
delete' -- phases 1 and 2 delete first, and are sound instead because
they pin their targets below the bucket root.
@brad-richardson

Copy link
Copy Markdown
Owner Author

Adversarial review found a real one, and it inverted the conclusion

I had this PR framed as "the precondition is already met, nothing to build." A subagent review tasked with refuting that found the delete surface is wider than r2-cleanup.yml.

F1 — release-slice-families.yml deletes exactly the shape zero-copy publishes into, unguarded. Its cleanup job runs aws s3 rm "s3://geocoder-shards/${SLICE_VERSION}/" --recursive behind nothing but a shape regex — and that regex is precisely release_slice_version's validated format. Three things compound it:

  • the job is always(), so it runs when preflight failed and every build job was skipped;
  • its one reference check, probe_catalog_excludes_slice.sh, walks the v1 catalog's child links, so it reports "not referenced" for a fully live promoted slice — always. It is reassurance, not a gate;
  • flipping release_slice_version is what moves ~45 GiB (Places) / ~114 GiB (Addresses) of serving objects into that namespace, from a construction prefix no delete path can name.

Verified in the code, not taken on report. Fixed here: the job now fetches the same v2 chain and calls the same guard before deleting.

F3 — my own overclaim. I wrote that r2-cleanup.yml "fetches the whole v2 chain before any delete." It does not: the fetch is at line 379, phase 1's RM at 329, phase 2's at 366. Those phases are sound for a different reason — they pin targets to */staging/ and ^staging/global-v2/[0-9a-f]{64}/$, neither of which can be a bucket root. Doc corrected, and the distinction is now what the test encodes.

F4 — both my new tests were tautologies. The reviewer built four safety-breaking edits that left them green. Replaced with structural checks over the parsed step order, and mutation-checked against all four:

mutation old tests new tests
|| true on the guard call pass fail
continue-on-error on its step pass fail
guard reordered after the delete pass fail
guard step deleted outright pass fail

The \|\| true case needed a second pass — the guard invocation spans backslash continuations, so appending to its last line evaded a per-line scan. Continuations are joined first now.

Recorded, not fixed (in the state doc): the pre-promotion window where a finished-but-unpromoted slice reads as unreferenced and the create-only claim file is not consulted; phase 3 needing a workflow edit to target a slice; phase 3 lacking phase 5's cache-TTL wait; and rebuild-r2-shards.yml's retention prune still being v1-only.

Full suite: 1,816 passed, 2 skipped.

@brad-richardson
brad-richardson merged commit c6df24b into main Aug 7, 2026
9 checks passed
@brad-richardson
brad-richardson deleted the feat/v2-aware-slice-delete-guard branch August 7, 2026 04:04
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.

1 participant