chore(release): 0.2.0 bump, pipx-primary docs, customer-first release notes (#349 PR4)

[caohy1988]

Final code increment before the tracing-v0.2.0 tag — hardened across fifteen release-review rounds, every finding resolved with execution-proof.

Contents

• Version: producers 0.1.0 → 0.2.0; tag pipeline verifies version match AND exact protected-main-head provenance before minting credentials.
• Docs flip: pipx/no-checkout primary; PyPI-safe absolute links; customer-first release-notes template with strict executable token-fill (tests execute the customer bash).
• Pipeline integrity:
  • Split identities (stager: staging-only; promoter: public-only, WIF-bound to the release-promote environment subject); environments tag-restricted; admin-only tag ruleset; all actions SHA-pinned in both workflows; crane pinned + checksummed + --password-stdin.
  • Hermetic self-test (PR CI + release + local): pinned emulator on a private bridge network, version/factory/multi-stage-hygiene assertions, bad-token 401, authenticated protobuf export with proven publication; tested image ID re-asserted before push.
  • Supply chain: digest-pinned base declared as ONE consumed ARG (validated: exactly two ${PYTHON_BASE} stages — literal/alternate-variable stages refused, with executed negatives); hash-locked runtime deps + build backend + the generator itself; bounded fixed-point convergence in fresh hash-locked containers with two distinct preserved diagnostics on exhaustion; signal-safe transactional copyback (six-position TERM-injection matrix proves all-old-or-all-new; per-invocation backup tracking so stale backups from an interrupted previous run are never restored); three-lock CI drift check; multi-stage image with zero build tools at runtime.
  • Reconciliation anchored to the immutable build artifact: eleven-state contract defined once as constants (returns, dispatch, docs — exact bidirectional equality with mutation negatives), complete PyPI and TestPyPI success schemas (explicit-404-only absence, boolean yanked, strict hex digests, empty-release burn semantics per PyPI filename-reuse rules), absence-first recovery, release visibility as an orthogonal dimension (premature-publication containment wraps every non-complete state, indeterminate included).
  • Snapshot-bound publication: drafts discovered via authenticated List Releases (the by-tag endpoint returns only published releases); publication operates on the release ID with asset digests AND canonical title/body/prerelease verified immediately before and after a JSON --input PATCH (gh raw fields do not read @files); Boolean-schema draft flags enforced end to end; never repository Latest (only an explicit no-Latest 404 tolerated); immutable releases are a verified pre-publication prerequisite — the policy read uses a dedicated GitHub App installation token with only Administration:read (the job GITHUB_TOKEN cannot carry it), and a release published while the setting was off is burned, never "repaired".
  • Asset trust boundary: finalize validates release asset metadata via a PR-tested planner (well-formed ids/names/sizes, no duplicates, no unsafe names) and downloads ONLY the four expected hardcoded names within a size cap, by asset ID; unexpected/oversized assets become empty placeholders that preserve the tested reconciliation classification without unbounded downloads.
  • Rerun safety: run_attempt identities, artifact identity from the producing job, idempotent digest-checked promotion, byte-validated guard states (absent/exact/deviating per index — an exact TestPyPI publication with a missing draft recreates it from the preserved original-attempt artifact; deviating bytes are an explicit burn), rerun-safe pre-upload gates on both indexes (absent/satisfied/conflict/indeterminate; exact local publish set enforced), fail-closed lookups with transport-error diagnostics and CDN cache-busting. The workflow never deletes a release automatically (no conditional delete exists — a stale draft blocks with manual verify-and-delete instructions).
  • 546 producer tests (reconciler + guard + publish + index-gate + asset-planner + workflow-contract + generator-guard suites), including cross-module recovery-consistency regressions and workflow-level wiring pins (the Administration:read App token is minted only for complete && draft).

One-time setup required before the first release: enable immutable releases (Settings → General → Releases) and configure the policy-read GitHub App (BQAA_RELEASE_POLICY_APP_ID variable + BQAA_RELEASE_POLICY_APP_PRIVATE_KEY secret) — see producers/RELEASING.md.

Deliberate deviation: prevent_self_review stays off (single-maintainer repo — it would deadlock every approval).

Part of #349.

[caohy1988]

Self-review gate: changes required before merge/tag. The current CI checks are green, but the release review found three inherited pre-tag blockers plus two out-of-hunk documentation gaps.

1. [P0] Public-registry approval bypass. The unprotected build-image job and protected promote job impersonate the same bqaa-release-publisher service account. Live IAM read-back shows that account can write both bqaa-staging and public bqaa, while WIF trusts any workflow in this repository. Split staging and promotion service accounts, grant the stager only staging write, and bind the public promoter to the release-promote environment subject.
2. [P0] Tag provenance is unenforced. Any tracing-v* tag triggers the workflow; verification checks only tag text versus pyproject.toml, not that the SHA is on protected main. Only main currently has a ruleset. Add a tag ruleset restricting creation/update/deletion and a main-ancestry check before credentials are minted.
3. [P1] GitHub release publication is premature. promote publishes the draft before the public-default smoke and before the protected PyPI upload. Keep the release draft, publish PyPI after approval, then publish the GitHub release in a rerunnable finalization job.
4. [P1] The primary no-checkout guide still verifies through a checkout. deploy/otlp_receiver/README.md:145 uses PYTHONPATH=producers/src python3 -m ... verify. Replace it with the installed bqaa-otel verify command and shell variables.
5. [P1] The lifecycle runbook also dry-runs teardown. producers/RELEASING.md:90 omits --confirm, so the approval evidence can pass without deleting or existence-verifying resources.

Inline comments cover the PR-local renderer, generated notes/latest-release behavior, cleanup commands, token substitution, and published links.

[caohy1988]

[P2] Add a PR-time renderer test and remove the duplicated compatibility source. This tag-only inline renderer is not executed by Producers CI, so placeholder or str.format drift first fails after the release tag is pushed. Extract it into a checked-in script, test a representative render with no placeholders left, and source the Codex version from the canonical compatibility metadata instead of repeating 0.142.5 here.

[caohy1988]

[P1] Generic generated notes are wrong for the repository-wide tag stream. A live releases/generate-notes preview for tracing-v0.2.0 selected v0.4.0 as the base and included unrelated dashboard, Dependabot, skill-lab, and demo PRs. Disable generic notes and add a curated tracing changelog covering #316, #324, #317, #340, #342, and #343, or use a tracing-only filtered generator.

[caohy1988]

[P1] Preserve the SDK release as the repository Latest release. softprops/action-gh-release leaves make_latest at the GitHub API default, which is true for a newly published release. This tracing 0.2.0 release would displace the SDK v0.4.0 release. Set make_latest: false and preserve it when the draft is finalized.

[caohy1988]

[P1] This is only a dry run, not clean removal. The CLI returns success without deleting or probing anything unless --confirm is present. Show both commands: this preview first, then the same teardown with --confirm so customers actually remove resources and receive existence verification.

[caohy1988]

[P1] This relative link breaks in the PyPI long description. producers/README.md is the package metadata README, so ../deploy/... is no longer rooted in the GitHub source tree when rendered on PyPI. Use an absolute GitHub URL to the primary deployment guide.

[caohy1988]

[P1] Add the token retrieval and replacement step before distribution. Bootstrap writes literal <token> placeholders, and Codex does not expand environment variables in headers. As written, an admin can distribute configs that always receive 401s. Show the gcloud secrets versions access command and require replacing the placeholder before rollout.

[caohy1988]

[P1] The advertised cleanup never executes. Without --confirm, teardown only prints its plan and returns success; it performs no deletion or post-delete verification. Keep this as the preview, then show an explicit second command with --confirm.

[caohy1988]

[P1] This release-page link resolves to a 404. GitHub leaves the relative href unchanged, so from /releases/tag/tracing-v0.2.0 it resolves to /BigQuery-Agent-Analytics-SDK/demo/hero_story/, not a repository tree URL. Render an absolute tag-specific URL such as https://github.com/GoogleCloudPlatform/BigQuery-Agent-Analytics-SDK/tree/tracing-v{version}/demo/hero_story.

[caohy1988]

Fresh full review — changes required before merge/tag

I re-reviewed head 240c1c6 from scratch, including live IAM, WIF/environment protections, Artifact Registry immutability, GitHub release settings, rerun behavior, the clean release-test dependency set, renderer output, and issue #349. Current CI is green, but the following findings remain.

P1 — release blockers

1. Promoter cannot read private staging. Live IAM gives bqaa-release-promoter writer on public bqaa, but no role on bqaa-staging and no project-level grant. crane copy therefore cannot pull its source. Grant the promoter roles/artifactregistry.reader on staging.
2. Claude config retains <token>. Bootstrap emits the placeholder in both codex.config.toml and claude-code.managed-settings.json; the notes edit only Codex and then tell admins to distribute both. Replace both, fail on an empty token, and assert neither file retains the placeholder.
3. The release test silently skips the documented protobuf path. pip install -e ".[dev]" omits opentelemetry-proto; the only protobuf receiver test uses pytest.importorskip. A clean reproduction is 389 passed, 5 skipped, with the fifth skip being protobuf—not 390/4. Install .[dev,receiver], make that import mandatory, and send a real protobuf request through the built container.
4. Reruns reuse immutable identities. github.run_id is unchanged across attempts, while the live staging repository enforces immutable tags; release-tracing-dist is also reused. Include github.run_attempt in the candidate tag and artifact identity, propagate the exact artifact name, and make public promotion idempotently accept only an existing matching digest.
5. A full rerun can mutate an already-published GitHub release. Immutable releases are live-verified disabled. softprops/action-gh-release reuses an existing published release and defaults to overwriting its body/assets. Enable immutable releases, fail if the release is already public, and set overwrite_files: false.
6. Credentialed third-party actions use mutable refs. GCP auth, GitHub Release, checkout/setup/download/upload actions, and PyPI publication use movable tags/branches; repository SHA-pinning enforcement is disabled. Pin all actions to reviewed full SHAs and enable enforcement.
7. The TestPyPI lifecycle install mixes indexes unsafely. pip gives --index-url and --extra-index-url no priority. Install dependencies from PyPI, download the exact TestPyPI candidate with --no-deps, verify its SHA256, then install that local artifact with --no-deps.
8. Partial PyPI publication can strand release assets in a draft. finalize is skipped whenever the publish action fails, even if one distribution already became public. Add an always() reconciliation job that queries the exact PyPI file set and four GitHub assets, finalizes only on complete state, and emits the yank/version-burn recovery for partial state.

P2 — hardening and contract corrections

9. Scope the promoter WIF identity to job_workflow_ref plus the tracing tag/ref, not only the reusable environment subject; also restrict the environment to tracing tags and prevent self-review.
10. Define PROJECT, DATASET, and the receiver URL before the customer commands. Bootstrap prints a URL but does not export $URL.
11. Remove the duplicate public image coordinate from the renderer; pass the workflow's authoritative $PUBLIC_IMAGE through a validated CLI argument.
12. The ancestor check accepts stale or reverted protected-main commits. Require the exact approved release commit or an equivalently protected release ref, not merely any historical ancestor.
13. Update the workflow header/job name, RELEASING.md, PR description, and #349: they still describe generic notes or promote publishing the release and omit the actual finalization/reconciliation state.

Issue #349 gate

Keep #349 open. Besides these code findings, the trusted publishers/2FA, security artifacts and package-level hardening guidance, real lifecycle evidence, post-release smoke, public-pull proof, source-build proof, and second owner remain outstanding.

Because this PR is authored by the reviewing account, GitHub does not allow a formal REQUEST_CHANGES state; this comment is the equivalent merge-gate verdict.

[caohy1988]

[P1] The promoter cannot read the source repository. Live IAM gives this SA roles/artifactregistry.writer only on public bqaa; private bqaa-staging has only the stager writer, and the promoter has no project-level role. The following crane copy must pull the staging manifest/blobs and will fail. Grant this SA roles/artifactregistry.reader on bqaa-staging while retaining public-only write.

[caohy1988]

[P2] The WIF binding is environment-scoped, not workflow/ref-scoped. Any workflow in the repository can request the release-promote environment and receive this subject after approval or admin bypass. Map and condition job_workflow_ref plus the tracing tag/ref, restrict the environment deployment policy to tracing-v*, and enable self-review prevention.

[caohy1988]

[P1] This fills only the Codex artifact. bootstrap --source claude-code,codex also writes <token> into claude-code.managed-settings.json, and the next instruction distributes that unchanged file, producing Authorization: Bearer <token> and 401s. Fail unless TOKEN is non-empty, replace both files, and assert neither still contains the placeholder.

[caohy1988]

[P2] is-ancestor proves historical membership, not the approved release state. A stale or later-reverted commit remains an ancestor of protected main forever. Require the tag to equal the explicitly approved release commit/current protected release ref, or record and verify an equivalent immutable release-commit approval.

[caohy1988]

[P1] Guard reruns from mutating an already-published release. Immutable releases are currently disabled, and softprops/action-gh-release defaults overwrite_files to true. On a full rerun it finds the published release, preserves draft: false, overwrites the body, and deletes/reuploads matching assets before TestPyPI fails. Enable immutable releases, fail early when the tag already has a public release, and set overwrite_files: false.

[caohy1988]

[P1] Pin credentialed actions to full commit SHAs. This movable @v2 reference executes with id-token: write; the PyPI publisher and GitHub Release action are also movable, and live repository settings do not require SHA pinning. Pin every action in this release workflow and enable repository SHA enforcement.

[caohy1988]

[P1] Reconcile external PyPI state even when publication reports failure. Twine can upload one distribution and fail on the next; default needs behavior then skips this job, leaving a public wheel/sdist subset while the plugin and checksums remain draft-only. Add an if: always() reconciliation job that verifies the exact PyPI file set and release assets before publishing or emitting a yank/version-burn recovery.

[caohy1988]

[P2] These variables are undefined in the primary guide. The deployment examples use literal my-proj/agent_analytics, then verification switches to $PROJECT, $DATASET, and $URL; bootstrap prints the receiver URL but does not export it. Define/export all three or derive URL explicitly before this command.

[caohy1988]

[P2] This creates a second source of truth for the public coordinate. Injection and promotion use workflow PUBLIC_IMAGE, while release notes use this constant; either can drift and the renderer test will bless its own copy. Add a validated --public-image input and pass ${PUBLIC_IMAGE} from the workflow.

[caohy1988]

[P2] This test can pass while the executable lifecycle is still broken. It checks disconnected substrings, so it missed that token replacement covers only Codex and that $URL is never defined. Assert the complete token-fill commands for both generated files, the non-empty/no-placeholder guard, URL initialization, and the exact confirmed teardown command.

[caohy1988]

Fresh full review at 952f079: changes required before merge/tag

The 20 PR checks are green, but they do not execute the tag-only release workflow. Fresh local verification produced 392 passing tests with four expected live-E2E skips, while the exact no-ADC container startup failed with DefaultCredentialsError.

P1 release blockers

1. The pre-push HTTP self-test cannot boot without ADC or a Pub/Sub emulator.
2. Downstream-only reruns increment github.run_attempt and request an artifact the successful build job never uploaded.
3. A full rerun can update the draft body while retaining same-named assets from an earlier attempt.
4. Finalization treats two PyPI URLs and four GitHub assets as proof of identity, so stale or unrelated bytes can publish.
5. The published-release lookup fails open on API, authentication, network, or parsing errors.

P2 follow-ups

6. The self-test proves only bad-token rejection, not successful authenticated protobuf decoding/publication.
7. PR CI still installs .[dev], leaving the receiver/protobuf path tag-only.
8. The TestPyPI checksum step prints a digest instead of mechanically comparing it, and the later verification section still mixes indexes.
9. The rendered token-fill shell can continue after missing/unreadable file errors; its tests inspect strings rather than execute the lifecycle snippet.
10. The primary deployment guide still uses undefined PROJECT, DATASET, and URL.
11. Workflow/runbook metadata still describes the old publication contract and retains an unnecessary contents: write permission on promote.
12. The released container remains non-reproducible because python:3.12-slim and pip-resolved runtime/build dependencies are mutable.

The live split IAM/WIF, environment/tag protections, action SHA pins, exact-main provenance, and accepted single-maintainer self-review policy were rechecked and are not blockers in this round. Because the authenticated account authored this PR, GitHub cannot record a formal REQUEST_CHANGES; this COMMENTED review is the equivalent merge-gate verdict.

[caohy1988]

[P1] Make this self-test boot without production credentials. The container starts before google-github-actions/auth and receives neither ADC nor PUBSUB_EMULATOR_HOST. make_app() eagerly constructs PubSubPublisher(), whose PublisherClient() raises DefaultCredentialsError before gunicorn can serve the request; I reproduced that failure at this exact head. Start a Pub/Sub emulator and point the container at it, inject a credential-free publisher for the test, or defer publisher construction until after authentication.

[caohy1988]

[P2] A 401-only probe does not exercise the advertised OTLP path. Even after the startup failure is fixed, a reject-all implementation can pass this test without protobuf decoding or Pub/Sub publication ever working. Keep the wrong-token 401 assertion, then send a valid authenticated protobuf payload against an emulator/stub and require the expected successful response and publish.

[caohy1988]

[P1] Consume the artifact identity emitted by the producing job. GitHub increments github.run_attempt when only a failed downstream job is rerun, but the already-successful build job is not recreated. That retry asks for release-tracing-dist-a2 although only ...-a1 exists, so the documented recovery path fails before publication. Export the actual artifact name or ID from build and consume that output in every downloader instead of reconstructing it from the consumer's current attempt.

[caohy1988]

[P1] Fail closed when release state is indeterminate. Redirecting errors and using the lookup only as an if condition makes authentication, rate-limit, network, API, and parsing failures indistinguishable from an absent/draft release. If the API recovers for the following action, an already-published release can still be modified. Capture the response/status, allow only an explicit 404 or a confirmed draft, and fail every other state.

[caohy1988]

[P1] overwrite_files: false can preserve a mixed-attempt draft. On a full rerun, the pinned release action updates the existing draft body but skips same-named assets. The body can therefore describe the new image digest while the wheel, sdist, plugin, and checksums remain from the previous attempt. Reject a pre-existing draft unless every asset name/hash and rendered digest match the current immutable candidate, or resume exclusively from the original candidate manifest.

[caohy1988]

[P1] Counts are not artifact identity. Because finalize runs after failed/skipped publication too, any version with two PyPI URLs and any draft with four assets is marked complete. Verify the exact expected wheel/sdist/plugin/checksum filenames, compare PyPI and downloaded release-asset SHA-256 values with the current candidate manifest, reject yanked files, and revalidate the public image/body digest before making the release visible. A failed upload is idempotent only when byte identity is proven.

[caohy1988]

[P2] Make the TestPyPI integrity gate mechanical. This command only prints a checksum; it never compares it with the draft release's SHA256SUMS, and /tmp/gate may contain stale or multiple wheels. Use a fresh temporary directory, download the exact expected filename plus the checksum manifest, run sha256sum -c (or an equivalent helper), and install only that verified file. Also remove the later mixed --index-url/--extra-index-url verification recipe so there is one safe path.

[caohy1988]

[P2] File errors can bypass this token-fill guard. sed ... && rm and grep ... && abort place failing commands in conditional lists; a missing or unreadable artifact can return an error and the snippet can continue toward distribution. Require both files up front, run with strict shell error handling, perform replacement without exposing the token in argv where practical, and use an explicit if grep -q ...; then abort; fi per file. Execute missing-file and surviving-placeholder cases in tests instead of checking only substrings.

[caohy1988]

[P2] The primary no-checkout path still leaves these variables undefined. Earlier commands use literal my-proj and agent_analytics, and bootstrap prints the receiver URL without exporting $URL. Copying this block expands PROJECT, DATASET, and URL to empty strings. Define/export them before the deployment sequence or use the same concrete values and derive the Cloud Run URL here.

[caohy1988]

[P2] Finish updating the release contract metadata and permissions. The earlier workflow header still says the draft is published by promote, promote is still named Promote image + publish release, and RELEASING.md still claims generate_release_notes: true. Rename/update those descriptions, and remove contents: write from promote because only finalize now needs release-write permission.

[caohy1988]

Round-4 fresh review at ef56f45: changes required before merge/tag

Fresh verification: all 20 PR checks pass; local suite is 395 passed with four expected live-E2E skips; the exact image rebuild produced bad-token 401, authenticated protobuf 200, package version 0.2.0, and both factories importable. Live split IAM/WIF, tag policies, required reviewers, and immutable staging/public tags also read back correctly.

P1 release blockers

1. Finalization trusts the mutable draft's own manifest, does not require every expected file to appear in that manifest, and does not reject extra GitHub/PyPI files.
2. The id-token: write image job installs/imports mutable host packages after starting the tested container but before pushing its tag, so host code can retag an untested image.
3. The mutable Pub/Sub emulator image shares host networking and can spoof both receiver responses.
4. Emulator readiness has no bound, curl timeout, cleanup trap, or job timeout.
5. Promotion treats every crane digest failure as an absent public tag and enters the mutating copy path.

P2 follow-ups

6. The rewritten image gate dropped packaged-version and consumer-factory checks, and the Docker self-test still is not PR-executed.
7. Token-fill leaves both secret-bearing configuration files at the normal 0644 mode.
8. The crane access token remains in a process argument instead of stdin.
9. Image and package build dependencies remain mutable; pip freeze records drift but does not prevent it.
10. Workflow synopsis, runbook candidate-tag/self-test description, PR body, and contributor install command remain stale.
11. There is no .dockerignore; the local review sent a 920 MB context.
12. The PyPI long-description README retains broken relative repository links, and the manual plugin fallback still uses VERSION=0.1.0.

The documented single-maintainer self-review choice was treated as an accepted residual rather than re-counted as a blocker. Because the authenticated account authored the PR, GitHub cannot record a formal REQUEST_CHANGES; this COMMENTED review is the equivalent merge-gate verdict.

[caohy1988]

[P1] Anchor reconciliation to the immutable build artifact and require exact sets. This checks only entries that SHA256SUMS happens to list. I reproduced a present plugin omitted from the manifest while sha256sum -c still exited 0. The later PyPI loop likewise checks the expected wheel/sdist but accepts extra distributions, and the release download accepts extra assets. Because the manifest, plugin, and body all come from the same mutable draft, replacing them together is self-consistent but not current-build identity. Propagate the upload-artifact ID/digest from build, download that artifact in finalize, require exact manifest/release/PyPI filename sets, compare bytes and the freshly rendered body to that anchor, and add hermetic state-matrix tests.

[caohy1988]

[P1] Do not install or import mutable host code between testing and push. This job already has id-token: write, and the receiver container was started from the candidate tag before this unpinned install/import. Compromised package code can request the job's OIDC token, invoke the Docker-capable host, and retag the candidate while the already-running original container still returns 200; the later push then publishes untested bytes. Generate the payload inside the built image or use a checked-in binary fixture, and verify the local image ID immediately before push.

[caohy1988]

[P1] Pin and isolate the emulator used as the release oracle. google-cloud-cli:emulators is a mutable tag and runs on the host network before the receiver starts. A compromised image can bind port 18080 itself, return the expected 401/200 responses, and hide a receiver container that failed to boot. Pin the emulator by digest, put emulator and receiver on a private bridge network, and expose only the receiver port to the host.

[caohy1988]

[P1] Bound readiness and clean up every exit path. If the emulator starts but never becomes healthy, this loop runs until GitHub's multi-hour job limit. Failures before the explicit docker rm also leave both containers running for the rest of the job. Use finite retries with curl connection/request timeouts, print emulator logs on failure, install a cleanup trap immediately after container creation, and set timeout-minutes on the job.

[caohy1988]

[P1] Fail closed on indeterminate registry state. Every crane digest failure, including authentication, rate limiting, registry errors, and network loss, enters the crane copy branch as if the tag were explicitly absent. Live immutable tags currently prevent an overwrite, but this still turns a read failure into a mutation attempt and breaks idempotent recovery. Capture stderr/status, copy only for an explicit manifest-not-found response, and fail every other state. Also send the access token through --password-stdin instead of the process argument vector.

[caohy1988]

[P2] Restore the complete image contract and execute it before tags. The previous gate asserted installed package version and imported both Cloud Run factories; this rewrite tests only the receiver role. I independently verified the current image is 0.2.0 and both factories import, but those properties can regress without failing this workflow step, and the full Docker path remains tag-only. Restore the metadata/factory assertions, probe the consumer role where practical, extract the self-test into a checked-in script, and run it from PR CI as well as the release workflow.

[caohy1988]

[P2] Restrict the files before writing the bearer token. The generated artifacts use the normal umask; executing this block locally left both token-bearing files at 0644, readable by other users on a multi-user administration host. Set umask 077 before generation or chmod 600 both files before replacement, and assert the modes in the executed-snippet test.

[caohy1988]

[P2] pip freeze audits dependency drift after it happens; it does not prevent it. The base image is now digest-pinned, but the build backend and every receiver dependency are still resolved from mutable lower bounds. A compromised or incompatible upstream release changes production bytes at the same source commit. Build reviewed hash-locked constraints, install with --require-hashes, and similarly lock the wheel/sdist build toolchain.

[caohy1988]

[P2] Finish synchronizing the release contract documentation. The earlier synopsis still says the self-test checks package version/factory imports and that promote publishes the draft. RELEASING.md still documents <version>-candidate.<run_id> instead of <run_id>.<run_attempt>, and the PR body still describes the pre-round-3 401-only/count-based implementation. Update these together, plus the contributor install command to .[dev,receiver], so the runbook matches the code operators will execute.

[caohy1988]

[P1] The new image self-test is not selected for its own inputs. Both workflow path lists omit .dockerignore and deploy/otlp_receiver/**, so a Dockerfile, dependency-lock, or self-test-only PR skips Producers CI and the next execution is at release-tag time. Add those inputs to both push.paths and pull_request.paths; include .github/workflows/release-tracing.yml too if changes to the shared release invocation should exercise this gate.

[caohy1988]

[P1] Fail closed on every reconciliation state. reconcile_release.py can return invalid-anchor, but this dispatcher handles only complete, unpublished, and partial. I reproduced state=invalid-anchor falling through with exit code 0, which can leave PyPI public, the GitHub release draft, and the release run green. Add an explicit invalid-anchor failure plus a *) catch-all that exits nonzero, and test the state-to-workflow mapping.

[caohy1988]

[P1] HTTP 200 does not prove a successfully decoded export. The receiver can return 200 with published=0, and per-record decode failures can return 200 with dead_lettered>0; this curl discards the JSON body and checks only the status. I reproduced ReceiverResult(status=200, published=0, dead_lettered=0). Capture and parse the response body, then require published > 0 and dead_lettered == 0 before emitting SELFTEST_IMAGE_ID.

[caohy1988]

[P1] The runtime hash lock does not cover the PEP 517 build backend. pip install --no-deps /src/producers still creates an isolated build environment and downloads the unpinned hatchling declared in pyproject.toml; the successful CI log shows Installing build dependencies after the hash-locked install. Hash-lock hatchling and its build dependencies, install them explicitly, then install the local package with --no-build-isolation --no-deps.

[caohy1988]

[P1] The immutable artifact anchor is still produced by mutable PyPI code. This job installs build without a version or hash, python -m build resolves an unpinned isolated hatchling, and the later wheel install resolves runtime dependencies before the plugin tarball is created. A compromised upstream package can therefore alter the bytes that become the trusted anchor. Use a reviewed hash-locked build toolchain with python -m build --no-isolation, and install the built wheel with --no-deps for the metadata step.

[caohy1988]

[P2] Lookup failures are being classified as publication absence. Any GitHub release download failure becomes unpublished, while any PyPI 404, authentication error, rate limit, 5xx, malformed response, or network failure becomes {} and receives the same recovery advice. This stays fail-closed, but it sends the operator down the wrong recovery path. Distinguish an explicit 404 from an indeterminate lookup and fail with surface-specific diagnostics.

[caohy1988]

Note: the round-11 fix commit was amended from 2f6d1af to d2ab512 (pyink formatting, and requirements.lock regenerated via regen-locks.sh for the upstream google-auth 2.55.2→2.56.0 release that was failing the drift check). Content of the four fixes is unchanged.

[caohy1988]

Round-6 fresh review at 861c8d4: changes required before merge/tag

The 21 visible checks are green, and fresh local verification is 409 passed with four expected live-E2E skips. The locked image self-test proves a real decoded publication. Four release-integrity blockers still remain:

1. Finalization anchors assets but publishes the mutable draft body unchanged.
2. The hash-locked trusted-anchor build command is still tag-only; PR CI exercises a different build path.
3. Malformed PyPI success responses are converted into confirmed absence.
4. A missing GitHub draft short-circuits the PyPI lookup and hides a possible cross-surface partial state.

P2 follow-ups are inline: the missing build-toolchain input, transport-error diagnostics, the stale locking comment, and the build toolchain/source retained in the runtime image.

Release metadata also needs one synchronized cleanup: the PR description still says two rounds and 26 findings, claims eight renderer tests, and describes the older 401-only gate; the workflow synopsis still says promote publishes the draft. Refresh those before merge so the permanent release record matches this head.

Separate pre-tag note from live read-back: Artifact Registry vulnerability scanning is disabled. Issue #349 asks for scan evidence if practical, so enable it or explicitly record the deferral before closing the release issue.

Because the authenticated account authored this PR, GitHub cannot record a formal REQUEST_CHANGES review. This COMMENTED review is the equivalent merge-gate verdict.

[caohy1988]

[P1] Re-anchor the customer-facing release body at finalization. Asset reconciliation never reads the draft body, and this command publishes whatever body exists after the manual approval pause. A contents-write principal or an accidental draft edit can replace the documented commands or digest while every artifact check still reports complete. Re-render release_body.md here from the protected checkout using VERSION, PUBLIC_IMAGE, and the digest extracted from the anchor wheel, then publish atomically with gh release edit --notes-file release_body.md --draft=false --latest=false. Reset other mutable release metadata here as appropriate.

[caohy1988]

[P1] Execute this exact trusted-anchor build path in PR CI. The Producers build job still installs mutable build and runs an isolated python -m build; the Docker gate installs this lock but never invokes the build frontend. A lock or backend incompatibility can therefore merge green and first fail after the release tag is pushed. Make Producers CI install build-requirements.lock with --require-hashes, run python -m build --no-isolation, and install the wheel with --no-deps, matching this job.

[caohy1988]

[P1] Do not convert a malformed HTTP-200 response into confirmed absence. Truncated or invalid PyPI JSON lands here, becomes an empty object, and is classified as unpublished even though only an explicit 404 proves absence. That can give rerun advice after a partial upload instead of treating the lookup as indeterminate. Let decoding fail nonzero or introduce an explicit invalid-response state, and add CLI-level tests for malformed, truncated, and unexpected success bodies.

[caohy1988]

[P1] Missing GitHub release is not the same state as unpublished on PyPI. This exits before querying PyPI and feeds dispatch a state whose recovery says the PyPI upload did not happen. If the draft is deleted after a complete or partial PyPI upload, finalization hides the actual cross-surface partial state and recommends the wrong operation. Classify GitHub-release existence and PyPI state independently, then dispatch a distinct missing-release or combined partial state.

[caohy1988]

[P2] Check in the lock input used by this regeneration command. build-toolchain.in does not exist at the repository root or under producers, so the reviewed lock cannot be regenerated from repository state. Add the exact input file and make the command copy-pastable with its real path, preferably through a checked-in script that also pins the base-image digest used to compile it.

[caohy1988]

[P2] Report PyPI transport failures explicitly. This assignment runs under set -e, so a DNS, TLS, or connection failure exits before the HTTP-state branch; curl -s also suppresses the reason. Wrap the request in an explicit transport-error conditional, use curl error output plus bounded connect/request timeouts, and reserve the following branch for actual HTTP 200 and 404 responses.

[caohy1988]

[P2] Update this comment to the contract the Dockerfile now enforces. Runtime dependencies and the PEP 517 backend are both installed from hash-locked files below, so calling that work a follow-up and describing freeze as the control is now false. Document the digest-pinned base plus the runtime/build lock pair instead.

[caohy1988]

[P2] Keep the build toolchain out of the shipped runtime image. This single-stage install leaves build, Hatchling, their dependencies, and the copied producers source/tests/scripts in the production image even though only the installed receiver package is needed at runtime. Use a pinned builder stage to build the wheel, then install only the runtime lock and wheel into a clean pinned final stage.

[caohy1988]

Round-7 fresh review at 406db09: changes required before merge/tag

Fresh verification: all required checks pass; the local producer suite is 414 passed, 4 expected live-E2E skips; and the image CI log proves bad-token rejection plus a real authenticated protobuf decode and Pub/Sub publication. Live required-reviewer environments, split IAM/WIF, the tag ruleset, and Container Scanning also read back correctly.

P1 release blockers

1. The GitHub-release-missing branch classifies every PyPI HTTP 200 as missing-release without validating the anchor or parsing the response. Malformed or empty success responses can therefore produce destructive yank/version-burn advice instead of an indeterminate or unpublished state.
2. The canonical lock-regeneration script downloads and executes an unpinned latest pip-tools with the repository mounted read-write. The installed artifacts are hash-locked, but their generator is still a mutable supply-chain input with source-tree write access.

P2 follow-ups

3. Object-shaped invalid PyPI schemas such as {"urls": null} and {"urls": [{}]} crash reconciliation instead of returning invalid-response. The workflow fails closed, but loses the declared diagnostic and recovery semantics.
4. PR CI does not execute the complete publish-time body re-anchor command path.
5. The image self-test does not assert the new multi-stage security property that build tools and the source tree are absent from the runtime image.
6. The PR description says 24 reconciliation tests, while this exact head collects 19. The reconciler module's state list also omits invalid-response and missing-release.

Operationally, the PR is currently BEHIND current main; the active main ruleset requires strict up-to-date status checks. Update the branch and rerun CI after resolving the findings.

Because the authenticated account authored this PR, GitHub cannot record a formal REQUEST_CHANGES review. This COMMENTED review is the equivalent merge-gate verdict.

[caohy1988]

[P1] Do not classify an unvalidated PyPI HTTP 200 as a cross-surface partial. When the GitHub release is missing, this branch bypasses both immutable-anchor validation and JSON/schema parsing. A truncated success body or even {"urls": []} becomes missing-release, which emits destructive yank/version-burn advice. Always validate the anchor and parse the PyPI response before combining surface states; move this branch into the tested reconciler and cover the release-present/missing × PyPI-404/empty/files/malformed × anchor-valid/invalid matrix.

[caohy1988]

[P1] The lock generator itself is an unlocked supply-chain input. This downloads and executes the latest pip-tools from PyPI while /src is a read-write mount of the repository. A compromised resolver can rewrite the locks or any source/workflow file before review. Pin and hash pip-tools plus its dependencies, mount repository inputs read-only, generate into a separate writable temporary directory, then compare/copy only the two expected lock outputs; add a CI lock-drift check using that exact generator.

[caohy1988]

[P2] Validate object-shaped PyPI schemas before iterating. Fresh reproduction shows {"urls": null} raises TypeError and {"urls": [{}]} raises KeyError. This fails closed, but it bypasses the declared invalid-response state and its retry-first recovery advice. Require urls to be a list of objects with string filenames and valid digest objects, reject duplicates, and return invalid-response for every schema violation.

[caohy1988]

[P2] Execute this complete publish-time re-anchor path in PR tests. Wheel extraction and rendering are tested separately, but no test composes the exact anchor-wheel lookup, renderer CLI, output file, and gh release edit --notes-file ... --draft=false --latest=false command. A quoting/path regression first appears after PyPI publication. Extract a small helper and test it with a fake anchor wheel plus a stubbed gh invocation.

[caohy1988]

[P2] Assert the multi-stage image property, not only runtime behavior. This proves version, factories, HTTP decoding, and publication, but it still passes if a later Dockerfile edit copies /build/producers or installs build/hatchling in the final stage. Add assertions that the source/build tree and build distributions are absent while the tracing package and required runtime dependencies remain installed.

[caohy1988]

Round-7 fresh review at 50268fc: changes required before merge/tag

Fresh verification: the live PR head and base are exact, the branch is no longer behind, all visible CI checks pass, the producer suite is 427 passed with 4 expected live-E2E skips, and the focused release suite is 43 passed. Two release-state correctness blockers remain; the inline comments include concrete fixes.

P1 release blockers

1. PyPI schema validation still accepts malformed HTTP 200 bodies as trusted states. In particular, correct filenames/digests with yanked omitted can reach complete.
2. Release-asset validation runs before confirmed PyPI absence, so a broken draft with no PyPI files emits yank/version-burn advice, while a missing draft with no PyPI files emits advice to keep a draft that does not exist.

P2 follow-ups

3. pip-tools.in is not consumed or included in lock convergence, so the generator bootstrap input can drift without failing CI.
4. regen-locks.sh depends on the caller's working directory, cleans up only after success, and can replace one output before discovering a problem with the other.
5. Update the permanent PR description before merge. It still says six rounds, 24 reconciler tests, and that all actions are SHA-pinned. This head is round 7, collects 29 reconciler tests, and Producers CI still uses version tags for several actions. Either pin those actions too or qualify the statement as release-workflow-only.

Because the authenticated account authored this PR, GitHub cannot record a formal REQUEST_CHANGES review. This COMMENTED review is the equivalent merge-gate verdict.

[caohy1988]

[P1] Require the complete PyPI success schema before trusting this response. pypi.get("urls", []) makes an HTTP 200 {} indistinguishable from the workflow's explicit 404 marker; the digest check accepts any 64 characters; and entry.get("yanked") later treats missing/null yanked as false. I reproduced correct filenames and SHA-256 values with yanked omitted reaching complete.

Suggested fix: represent HTTP 404 with a separate --pypi-missing input instead of {}. For every HTTP 200, require urls to exist and be a list, require each SHA-256 value to fully match [0-9a-f]{64}, and require yanked to exist with Boolean type. Add negative tests for missing urls, missing/null/string yanked, non-hex/uppercase/wrong-length digests, and assert that none can publish or trigger destructive recovery.

[caohy1988]

[P1] Classify confirmed PyPI absence before choosing release recovery advice. With a partial/broken draft and zero PyPI files, this asset check returns partial, whose dispatch tells the operator to yank PyPI and burn the version even though nothing was published. With no draft and no PyPI files, the current unpublished dispatch instead says to keep a draft that does not exist.

Suggested fix: distinguish at least missing-all (no release, no PyPI), draft-invalid (release exists but assets differ, no PyPI), and unpublished (exact draft, no PyPI). Reserve partial plus yank/version-burn advice for states where validated PyPI files actually exist. Extend the Cartesian state tests to assert both the state and the exact recovery action/message, not only classification.

[caohy1988]

[P2] Include the generator bootstrap lock in the convergence contract. The exact generator installs from pip-tools.lock, but regen-locks.sh never consumes pip-tools.in and this diff checks only the two downstream locks. A change to pip-tools.in with a stale generator lock therefore passes unchanged.

Suggested fix: add a pinned bootstrap/convergence step that regenerates pip-tools.lock from pip-tools.in, then regenerates the runtime/build locks with that environment and diffs all three files. If pip-tools.lock is intentionally a manually reviewed trust root, remove the unused .in file and document/test that lifecycle explicitly instead of advertising automatic convergence.

[caohy1988]

[P2] Make regeneration independent of the caller and safe on failure. The script mounts $PWD, writes back through relative paths, removes $OUT only after success, and copies the two lock outputs one at a time without first validating both. Running it outside the repository root or failing between copies can leave leaked scratch data or a mixed lock pair.

Suggested fix: derive the repository root from ${BASH_SOURCE[0]} (or git -C that directory), install an EXIT cleanup trap immediately after mktemp, verify both generated outputs exist and are non-empty before touching the worktree, then stage and replace the reviewed pair only after all validation succeeds. Also source the base-image reference from one canonical checked-in location so the Dockerfile and generator cannot drift independently.

[caohy1988]

Round-9 fresh review at 644b08a: changes required before merge/tag

Fresh exact-head verification: all live CI checks pass; the producer suite is 435 passed with 4 expected live-E2E skips; the focused release suite is 51 passed; and the reconciler collects 37 tests. The omitted-yanked, strict-digest, explicit --pypi-missing, action-pinning, and absence-first draft fixes are present. One release-state blocker remains, plus four P2 hardening items. Concrete fixes are inline.

P1 release blocker

An HTTP-200 PyPI release with urls: [] is still classified as confirmed absence. PyPI can retain a release record after every file is deleted, and deleted filenames cannot be reused, so the current same-version rerun advice is wrong. This needs a distinct burned-version/no-yank recovery state.

P2 follow-ups

1. The three-lock generator is not one-run convergent when the generator version changes.
2. The Dockerfile still duplicates the supposedly canonical base reference while the parser reads only the first.
3. Copyback is sequential rather than all-or-nothing despite the comment.
4. Reconciler state documentation still describes the older six-state/{}-marker model.

Because the authenticated account authored this PR, GitHub cannot record a formal REQUEST_CHANGES state. This COMMENTED review is the equivalent merge-gate verdict.

[caohy1988]

[P1] Do not collapse an HTTP-200 empty release into the explicit-404 absence state. pypi_absent = not urls makes {"urls": []} produce unpublished or missing-all, both of which can advise a same-version rerun. PyPI can retain a release after its files are deleted, and PyPI forbids reusing deleted filenames, so that rerun will fail even though there is currently nothing to yank. The test at test_reconcile_release.py:365 currently locks in this unsafe equivalence.

Suggested fix: make pypi is None the sole confirmed-absence predicate. For a non-None HTTP-200 response whose validated urls list is empty, return a distinct empty-release state. Its dispatch should say: no files exist to yank, but the version is burned; bump, rebuild, and cut a new tag. Add release-present and release-missing tests asserting that this state contains neither same-version rerun advice nor a yank instruction.

References: https://docs.pypi.org/api/json/ shows releases with empty file lists; https://pypi.org/help/#file-name-reuse documents that deleted filenames cannot be reused.

[caohy1988]

[P2] Generate downstream locks with the newly resolved generator, not the old installed one. This container installs the checked-in pip-tools.lock, writes a potentially newer candidate pip-tools.lock, then immediately runs both downstream compilations with the already-installed old toolchain. A generator/backend update therefore is not one-run convergent and can give the downstream locks provenance that differs from the new generator lock.

Suggested fix: split regeneration into two phases. Phase 1 installs the reviewed checked-in lock only to produce a candidate generator lock. Phase 2 starts a fresh container/environment, installs that candidate with --require-hashes, regenerates the candidate generator lock again plus both downstream locks, and requires the second generator lock to be byte-identical before any copyback. CI should execute this exact two-phase path and diff all three outputs.

[caohy1988]

[P2] The base image is still duplicated, and this parser silently trusts only the first copy. The Dockerfile repeats the full base reference for the builder and runtime stages. grep -m1 reads only the builder FROM, so the runtime stage can drift while the generator continues claiming a single canonical source.

Suggested fix: define one global Dockerfile value, for example ARG PYTHON_BASE=python:3.12-slim@sha256:..., and use FROM ${PYTHON_BASE} for both stages; make this script parse that single ARG. If keeping literal FROM lines, parse every matching reference and fail unless the complete set contains exactly one identical value. Add a shell test that intentionally makes the second stage differ and requires regeneration to fail.

[caohy1988]

[P2] The copyback still is not all-or-nothing. Validating all generated files first prevents generation-time partial updates, but these three cp calls remain sequential. A permission, disk, interruption, or later-copy failure after the first replacement leaves the worktree with a mixed lock generation, exactly what the comment says cannot happen.

Suggested fix: stage destination-local temporary files, back up all three originals, and install a rollback trap covering ERR, INT, and TERM. Replace the set only after staging succeeds; if any replacement fails, restore every original before exiting. Add a test with an injected second-copy failure proving all three destination hashes remain unchanged.

[caohy1988]

[P2] Synchronize the public state contract with the implementation. This module still documents six states and the old broad partial/unpublished meanings, omitting missing-all and draft-invalid; the CLI comment near line 304 also still says {} is the explicit-404 marker even though --pypi-missing replaced it. Operators debugging finalize will read the wrong recovery model.

Suggested fix: after adding the HTTP-200 empty-release state, document all nine states here with their exact recovery semantics and replace the stale {} comment with the --pypi-missing/None representation. Prefer a single state-to-action mapping used by dispatch() plus a test asserting its keys equal the documented/produced state set, so the vocabulary cannot drift again.

[caohy1988]

[P2] Validate the Dockerfile FROM consumers, not merely matching literals.

This guard only proves that one Python digest literal appears somewhere. It never proves that the actual stages consume PYTHON_BASE. I replaced both stages with FROM ubuntu:latest while leaving the pinned ARG in place; this parser returned success and selected the unused Python digest. The current divergent-literal test misses every base that does not match its narrow Python regular expression.

Potential fix: parse exactly one global ARG PYTHON_BASE=, require the expected two FROM instructions to use ${PYTHON_BASE} (with only the optional builder alias), and reject every literal or alternate-variable stage. Add negatives for an unpinned FROM, an unrelated literal base, a second variable, missing or duplicate ARG declarations, and too few or too many stages.

[caohy1988]

[P2] Enforce the exact produced, dispatched, and documented state vocabulary.

The current head happens to produce the same nine strings that KNOWN_STATES contains, but the claimed one-mapping contract is not what this test proves. reconcile() still returns independent string literals, the module table is handwritten, and this assertion is one-way substring matching. A removed row can still pass when its word appears elsewhere in the prose, and an obsolete extra documented state is never rejected.

Potential fix: use shared state constants or structured StateSpec metadata for every reconciliation return and dispatch key. Parse exact state-table row tokens, then assert equality in both directions among produced states, dispatch keys, and documented rows. Include a mutation-style negative demonstrating that a stale extra row and a produced typo both fail.

[caohy1988]

[P2] Make a genuine generator disagreement recoverable.

If phase 2 differs from the phase-1 candidate, this branch exits before copyback and the EXIT trap deletes both generated locks. A rerun starts from the same unchanged checked-in seed, produces the same candidate, and reaches the same mismatch, so the current rerun-after-review advice is a deterministic dead end.

Potential fix: iterate in fresh hash-locked containers with a small explicit bound: install seed N, generate candidate N+1, and stop only when two consecutive resolution bodies match. Then start one final fresh environment from the converged lock to generate the downstream locks. On bounded exhaustion, preserve the last two diagnostic locks and print their location instead of deleting the only evidence. Add a stubbed-generator test that requires more than two phases and another that proves bounded non-convergence fails with usable diagnostics.

[caohy1988]

[P1] Make the copyback commit boundary signal-safe.

The new rollback covers staging failures, but it remains armed while backups are deleted one at a time, and the INT/TERM handlers return instead of terminating. I reproduced a TERM immediately after the first backup deletion: the command exited 0 with pip-tools.lock new while requirements.lock and build-requirements.lock were restored old. That violates the all-or-nothing guarantee and can leave a reviewed lock set with mixed provenance.

Potential fix: keep rollback armed only until all three destination renames succeed. At that point mark the transaction committed and disarm ERR/INT/TERM before best-effort backup cleanup. Use distinct signal handlers that roll back and exit nonzero, for example 130 for INT and 143 for TERM. Extend the executed test to inject failure or a signal after every rename and during every backup-cleanup position, asserting that the result is always all-old or all-new and that a signal never exits 0.

[caohy1988]

Round 10 verdict: changes requested.

GitHub does not permit this account to submit a formal request-changes review on its own PR, so this comment records the same merge gate.

I posted four inline findings with reproductions and potential fixes. The signal-window lock corruption is merge-blocking: a TERM during backup cleanup can leave one new lock and two old locks while the script exits 0.

[P2] Refresh the PR description with the current contract. The body still says eight review rounds, eight exhaustive states, and 37 reconciler tests. Exact head 31fa026 contains the round-9 commit, nine reconciliation states, 41 reconciler tests, and four generator-guard tests.

Potential fix: after resolving this review, update the description to the final review-round count and exact test/state totals. Replace the current one-run-convergent and transactional wording with the implemented bounded fixed-point and signal-safe commit semantics, so the public claim matches the executable guarantees.

Inline findings:

• P1 signal-safe transaction boundary: #356 (comment)
• P2 recoverable convergence: #356 (comment)
• P2 actual FROM-consumer validation: #356 (comment)
• P2 exact state-contract enforcement: #356 (comment)

Verification at this head: 443 producer tests passed with 4 skips; focused release suite 59 passed; ShellCheck, bash syntax, Python compilation, and diff checks passed; all GitHub checks are green. Those checks do not cover the reproduced cleanup signal window.

[caohy1988]

Fresh full review at ae916b7: changes required before merge/tag

I posted two P1 release blockers and two P2 correctness findings inline. The strongest blocker is the full-rerun path: it deletes a recoverable draft and rebuilds a different immutable artifact anchor after PyPI may already contain the original bytes.

Verification on the exact head: 451 passed, 4 expected live-E2E skips; 67 focused release tests passed; ShellCheck, syntax, compile, formatting, current PR checks, live IAM/WIF, registry immutability, and environment/tag protections were rechecked.

Because the authenticated account authored this PR, GitHub cannot record a formal REQUEST_CHANGES state. This COMMENTED review is the equivalent merge-gate verdict.

[caohy1988]

[P1] Preserve the existing release state before a full rerun.

This deletes every draft before checking PyPI or TestPyPI. If PyPI accepted the original wheel/sdist and only finalize failed transiently, a full rerun deletes the matching draft and builds a new anchor. I reproduced the failure mode: identical source and injected image digest built with different checkout timestamps produced different wheel and sdist SHA-256 hashes. Finalization then compares valid PyPI bytes against the replacement anchor and emits partial/yank/version-burn recovery.

Fix: never auto-delete an existing draft, or permit deletion only after both indexes return explicit 404s. Otherwise preserve the draft and instruct operators to rerun failed jobs from the original workflow attempt. Add a regression for draft present + PyPI complete + full rerun.

[caohy1988]

Fixed in 2f6d1af. The delete/preserve/fail decision now lives in PR-tested producers/scripts/guard_existing_release.py: a stale draft is deleted ONLY when PyPI and TestPyPI both return an explicit 404 for the version (indeterminate lookups fail before reaching the guard). Once either index holds files, the draft is preserved with instructions to rerun the FAILED jobs from the original workflow attempt; a published release always fails. The requested regression is test_draft_present_and_pypi_complete_full_rerun_preserves_draft, plus a full release×index matrix invariant test. RELEASING.md documents the original-attempt rerun path.

[caohy1988]

[P1] Reconcile release visibility metadata, not only downloaded assets.

gh release download succeeds for a published release too, but the reconciler receives no draft/visibility state. If the draft is accidentally published during an approval pause while PyPI remains absent, this path classifies the exact assets as unpublished and tells the operator to keep "the draft" even though incomplete artifacts are already customer-visible. The final edit also does not reassert title or prerelease=false.

Fix: fetch release metadata through the API, pass it into reconciliation, add an explicit premature-publication state, and reassert the canonical title, tag, prerelease=false, latest=false, and body at publication.

[caohy1988]

Fixed in 2f6d1af. finalize now fetches the release METADATA via the API before downloading assets and passes --release-published into the reconciler. Every determinate non-complete state on a published release becomes the new premature-publication state (re-draft immediately; the underlying state survives in the detail line) — the reviewer scenario (published during the approval pause + PyPI absent) is test_published_release_with_prod_absent_is_premature. Indeterminate states (invalid-anchor/invalid-response) keep retry-first advice. Publication now reasserts the canonical title (--title "Tracing <tag>"), body, --prerelease=false, and --latest=false in publish_release_body.py, with test assertions.

[caohy1988]

[P2] Include TestPyPI in cross-surface reconciliation.

The TestPyPI uploader can publish one distribution before failing on the next, which burns that version. finalize queries only production PyPI, so production 404 plus an exact draft becomes unpublished and recommends rerunning publication even though the failed TestPyPI prerequisite cannot succeed at the same version.

Fix: query and validate the exact TestPyPI filename/digest set alongside PyPI and GitHub, then make its partial/complete/absent states part of the tested recovery matrix.

[caohy1988]

Fixed in 2f6d1af. finalize now queries TestPyPI with the same fail-closed HTTP handling (explicit 404 = absent, 200 = schema-validated body, anything else = indeterminate error) and the reconciler validates the exact TestPyPI filename/digest set against the anchor. The reviewer scenario — production 404 + exact draft + partial TestPyPI — is the new testpypi-partial state: version burned there, bump + re-tag, never same-version rerun advice (test_prod_absent_testpypi_partial_is_burned_not_unpublished). The absent/complete/partial matrix is tested, including: pruned TestPyPI files never block a complete production release, while a digest conflict on a present file does fail closed (the full-lifecycle gate installed different bytes).

[caohy1988]

[P2] Preserve two distinct convergence diagnostics.

On the final mismatch this overwrites seed.lock with candidate.lock; the exhaustion branch then copies both names, so the promised "last two locks" are byte-identical. The stubbed non-convergence reproduction saved seed.lock = v3-unique and candidate.lock = v3-unique, eliminating the diagnostic delta.

Fix: preserve the pre-overwrite seed as a separate previous lock and copy that plus the final candidate on exhaustion. Extend the test to assert the diagnostic contents differ and represent the last two phases.

[caohy1988]

Fixed in 2f6d1af. The loop now copies the phase input to previous-seed.lock before reseeding, and exhaustion preserves previous-seed.lock + candidate.lock — the final phase's input and output, i.e. the last two distinct resolutions whose diff is the non-convergence. test_bounded_exhaustion_preserves_diagnostics now asserts the two files differ and carry exactly the phase-2 and phase-3 outputs of the stub (v2-unique/v3-unique); a new first-phase-exhaustion test covers the checked-in-seed edge.

[caohy1988]

Fresh re-review of d2ab512 after the four prior findings were addressed. Those fixes are present and the local/CI verification is green, but the end-to-end release path still has four blocking correctness/security issues plus four P2 recovery/transaction issues. Inline comments contain the reproductions and requested fixes.

Verification: exact PR head/base reviewed; 479 producer tests passed (4 optional-dependency skips); 84 focused release tests passed; pyink/isort, compile checks, shellcheck, workflow parsing, and current CI passed. Live GitHub/GCP controls were also rechecked.

Requesting changes because the normal finalize path currently cannot discover the draft release, and the rerun path can override the new guard's correct recovery decision.

[caohy1988]

P1 — the normal release path cannot discover this draft. GitHub documents GET /repos/{owner}/{repo}/releases/tags/{tag} as getting a published release, so a draft is returned as 404 here. The same lookup is used by finalize; it therefore classifies the valid draft as missing and can never publish the normal successful release. Please use authenticated List Releases, filter the exact tag, retain the release ID, and use ID-based asset operations. Docs: https://docs.github.com/en/rest/releases/releases?apiVersion=2026-03-10#get-a-release-by-tag-name

[caohy1988]

Fixed in 3782959. Both the guard and finalize now discover the release via authenticated List Releases (gh api --paginate .../releases) filtered on the exact tag — never the by-tag endpoint. More than one release/draft sharing the tag fails closed as ambiguous. The release ID is retained (exported to the publish step), and finalize downloads assets BY ASSET ID (.../releases/assets/{id} with Accept: application/octet-stream). Verified live against this repo: the list pipeline finds tracing-v0.1.0 with its ID, and a by-ID asset download reproduced the API's digest byte-for-byte.

[caohy1988]

P1 — finalize still runs after the new release guard deliberately fails. It does not directly need or gate on github-release. On a full rerun, the guard correctly preserves the original draft and fails, the publication jobs skip, but always() lets finalize reconcile that original draft against the newly rebuilt, byte-different anchor. That overrides the guard's correct "rerun the original attempt" advice with invalid-draft/yank/burn guidance. Please make github-release a direct need and skip this reconciliation when its guard fails; recovery must use the original attempt's anchor.

[caohy1988]

Fixed in 3782959. finalize now has github-release as a direct need and its if: requires needs.github-release.result == 'success' alongside verify/build. On a full rerun where the guard deliberately fails, publication jobs skip AND finalize skips — the guard's "rerun the failed jobs from the original attempt" advice stays authoritative, and reconciliation only ever runs against the anchor of the attempt that created the draft.

[caohy1988]

P1 — publication is not bound to the release snapshot that was reconciled. Assets are downloaded and verified earlier, but this publishes later by mutable tag. A contents:write actor can replace the draft assets between those steps, and the repository's live immutable-releases setting is currently disabled, so published assets and SHA256SUMS remain replaceable afterward. Please enable/assert immutable releases, retain the exact release and asset IDs plus digest inventory, and revalidate immediately before/after publishing the exact release. Premature-publication recovery will also need to account for immutable releases being non-redraftable. Docs: https://docs.github.com/en/code-security/concepts/supply-chain-security/immutable-releases

[caohy1988]

Fixed in 3782959. publish_release_body.py is now snapshot-bound and ID-based: it fetches the exact release ID that was reconciled, verifies the complete asset inventory (names + the API digest field against sha256 of the ANCHOR bytes, including SHA256SUMS) immediately BEFORE the edit, publishes via PATCH /releases/{id} (reasserting tag/title/body/prerelease/make_latest), re-fetches and re-verifies the same snapshot AFTER, and asserts the published release has immutable: true — failing closed with instructions until the repository setting is enabled (documented as required one-time setup in the workflow header and RELEASING.md; it cannot be enabled from a PR). Premature-publication recovery now covers non-redraftable immutable releases (burn + corrected follow-up). An already-published, immutable, snapshot-identical release is an idempotent success without an edit. All paths are unit-tested with a stubbed gh (pre/post tamper, extra/missing assets, mutable publication, idempotent rerun).

[caohy1988]

P1 — stale-draft deletion has a destructive TOCTOU window. Once draft discovery is fixed, the release can become published between the initial lookup, the two index requests, and this tag-based delete. PyPI also documents that its APIs are CDN-cached, so one 404 is not necessarily a fresh absence proof. The safest fix is to remove automatic deletion. If it remains automated, retain the exact release ID, re-fetch that ID immediately before deletion, require the same object still has draft=true, and use a suitably fresh absence check. https://docs.pypi.org/api/

[caohy1988]

Fixed in 3782959. Deletion remains automated but is now TOCTOU-guarded per your conditions: the guard retains the release ID from discovery, re-fetches that exact ID immediately before deletion, requires the same object to still be draft=true on the same tag (any change aborts), and deletes by ID — never by tag. Freshness: every index lookup (guard, finalize, and the new pre-upload gates) appends a unique run-scoped cache-busting query string so the CDN cannot serve a cached 404 as absence proof (per https://docs.pypi.org/api/).

[caohy1988]

P2 — the production-complete branch accepts extra and yanked TestPyPI files. This loop inspects only expected_pypi & set(tp_urls). I reproduced both an unexpected platform wheel and a yanked expected wheel returning complete, contrary to the module/test contract and potentially allowing the lifecycle gate to install different bytes. Missing expected files can remain tolerated for pruning, but every present filename should be expected, non-yanked, schema-valid, and digest-identical. Please add both regressions.

[caohy1988]

Fixed in 3782959. The production-complete branch now iterates over every PRESENT TestPyPI file: an unexpected filename, a yanked file, or a digest mismatch each return partial; only MISSING files stay tolerated (pruning). Both of your reproductions are regressions: test_testpypi_extra_file_with_prod_complete_is_partial (unexpected platform wheel) and test_testpypi_yanked_file_with_prod_complete_is_partial (yanked expected wheel).

[caohy1988]

P2 — published visibility is not orthogonal to the underlying state. invalid-anchor and invalid-response are excluded from this wrapper, so a published release plus malformed index response gets retry-only advice and remains public. Separately, the workflow's public-image mismatch overwrites the state with plain partial, also losing the published signal. Please preserve visibility independently: every published, non-complete outcome should emit immediate containment guidance plus the underlying recovery classification.

[caohy1988]

Fixed in 3782959. Visibility is now orthogonal: EVERY non-complete state — including invalid-anchor and invalid-response — becomes premature-publication when the release is publicly visible, with the underlying classification and its recovery preserved verbatim in the detail line (containment guidance first, then the underlying recovery). The workflow's public-image digest-mismatch override also keeps the published signal: it emits premature-publication instead of plain partial when the release is published. Regressions: test_indeterminate_states_are_wrapped_too, plus the workflow branch.

[caohy1988]

P2 — complete TestPyPI plus a failed uploader job has no viable recovery path. If TestPyPI accepted both files but the action lost the response/failed afterward, this returns unpublished and dispatch says to rerun failed jobs. The same-version uploader rerun then fails because those files already exist (skip-existing is not enabled), so promotion cannot resume. Please let an exact verified TestPyPI publication satisfy the failed upload stage, or provide an explicit non-reupload recovery path and cover the lost-response case.

[caohy1988]

Fixed in 3782959. New PR-tested scripts/check_index_publication.py runs as a pre-upload gate in BOTH publish jobs: absent (fresh explicit 404) → proceed with the upload; an EXACT verified existing publication (same filename set, nothing yanked, digests identical to the local dist files) → the stage is satisfied and the upload step is skipped; anything else → the version is burned there, fail closed. skip-existing stays disabled, so the byte-identity contract is unchanged — the lost-response rerun now converges instead of dying on "version already exists" (test_exact_existing_publication_is_satisfied), and promotion can resume.

[caohy1988]

P2 — rollback can restore a stale backup from a previous run. A post-commit interruption may strand .bqaa-bak files. If a later run fails before overwriting every destination's backup, this loop restores stale backups for destinations the current transaction never touched, mixing lock generations. Please use per-run backup names or track and restore only destinations actually backed up in this invocation, with a two-run interruption regression.

[caohy1988]

Fixed in 3782959. The staging loop records each destination it backs up in a BACKED_UP array, and rollback restores ONLY those — a stale .bqaa-bak stranded by a previous run's interrupted post-commit cleanup is never restored for a destination the current transaction didn't touch (it's left for forensics; the next successful run's cleanup removes it). The requested two-run interruption regression is test_stale_backup_from_a_previous_run_is_never_restored: run 1 commits with TERM during cleanup, run 2 fails after backing up only the first destination, and all three locks must remain run-1's generation.

[caohy1988]

Round 13 fresh review of 3782959. I found three P1 release blockers and three P2 findings. Local verification is green (508 passed, 4 skipped; pyink/isort, shellcheck, bash syntax, workflow YAML, compileall, and diff checks are clean), but the findings below are runtime/API-semantic gaps. The Producers CI workflow is currently queued rather than failing.

[caohy1988]

[P1] The pre-delete re-fetch does not close the check/delete race. The GET at lines 421-426 and this unconditional DELETE are separate requests. A maintainer or API client can publish after the GET reports draft=true but before this request, so the workflow deletes the now-public release by ID. Workflow concurrency does not serialize external release edits; if release immutability is enabled, publishing also protects the tag name from reuse even after release deletion. GitHub exposes no condition in this call tying DELETE to the fetched draft snapshot. Please remove automatic draft deletion or add an exclusive operational control that makes publication impossible across this interval; a second read narrows the race but does not eliminate it.

[caohy1988]

Fixed in ac106d2 by taking your safest option: automatic deletion is REMOVED entirely. The guard's (draft, 404, 404) cell now fails the job with instructions to manually verify the release is still an unpublished draft, delete it, and re-run the job — no GET/DELETE pair remains in the workflow. Regression: test_stale_draft_requires_manual_deletion_never_automated, plus a matrix invariant that the ONLY proceeding state is release-absent + both indexes 404 and that GuardAction no longer even has a deletion field.

[caohy1988]

[P1] This sends the literal string @release_body.md, not the rendered notes. gh api -f/--raw-field adds a static string; only -F/--field interprets a leading @ as a filename. I reproduced the exact semantics with gh api markdown -f text=@producers/scripts/release_notes_template.md -f mode=gfm, which returned <p>@producers/scripts/release_notes_template.md</p>. The test at test_publish_release_body.py:159 currently locks in the faulty argv shape. Use -F body=@... or pass a JSON body containing the rendered text, and assert the transmitted value equals the rendered notes.

[caohy1988]

Fixed in ac106d2. Confirmed your reproduction independently (gh api markdown -f text=@file → <p>@file</p>; -F reads the file). The publish edit now sends a JSON --input payload built by publish_release_body.py containing the rendered notes plus tag/name/draft/prerelease/make_latest — no =@ field syntax anywhere. test_payload_carries_rendered_notes_not_a_filename asserts the transmitted body equals the rendered notes (and contains the pinned image reference), and test_happy_path_publishes_by_id_with_json_payload locks in the full payload shape.

[caohy1988]

[P1] Immutability is checked only after the release has already been made public, and the documented recovery cannot repair it. The PATCH at lines 210-230 includes draft=false; only afterward does this branch reject a mutable publication. The live repository endpoint currently returns {"enabled":false,"enforced_by_owner":false}, so this sequence exposes customer-visible, replaceable artifacts and then fails. GitHub also documents that enabling release immutability applies only to future releases, so RELEASING.md lines 204-208 cannot fix the already-published release by merely enabling the setting and rerunning this same PATCH. Make an attested immutable-release policy a prerequisite before tagging or before publication (for example, a protected admin-read credential or enforced organization policy), retain this postcondition as defense in depth, and correct the recovery guidance.

[caohy1988]

Fixed in ac106d2. Immutability is now a PREREQUISITE: before anything becomes public, publish_release_body.py reads GET /repos/{repo}/immutable-releases and refuses to publish while enabled is false — the release stays an unpublished draft and the error instructs enabling the setting then re-running finalize (which is then correct, because our release publishes AFTER the setting is on). A release found already published-but-mutable is classified as burned with delete/yank/bump/re-tag guidance — never "enable and re-run" (RELEASING.md corrected accordingly, including a new troubleshooting entry for that state). The post-publish immutable: true assert is retained as defense in depth. Tests: test_disabled_immutable_setting_blocks_before_anything_is_public (asserts nothing else is even fetched), test_published_mutable_release_is_burned_not_repaired, test_unreadable_immutable_setting_fails_closed.

[caohy1988]

[P2] The idempotent and post-publish paths never verify the canonical editable metadata. verify_snapshot() checks only ID, tag, and asset state/digests, then this branch returns success for any published immutable release. GitHub immutable releases still allow title and release-note edits, and the post-PATCH check has the same omission, so the literal-body defect above or later drift in name, body, prerelease, or Latest status is accepted forever. Render the expected body before this branch, compare all canonical metadata on both fetched snapshots, and repair the fields GitHub permits editing or fail with exact remediation.

[caohy1988]

Fixed in ac106d2. The canonical body and title are rendered BEFORE any branch, and canonical_problems() (title, normalized rendered body, prerelease flag) now runs on both the idempotent snapshot and the post-publish snapshot alongside the asset-digest check. On a published immutable release, title/body drift is REPAIRED with a metadata-only PATCH (the edits GitHub permits) and re-verified for convergence; prerelease drift fails as unrepairable with exact remediation. The release must also not be the repository's Latest (checked via releases/latest when readable). Tests: test_editable_metadata_drift_is_repaired, test_repair_that_does_not_converge_fails, test_prerelease_drift_is_unrepairable, test_post_publish_canonical_drift_is_detected, test_becoming_repository_latest_fails.

[caohy1988]

[P2] A malformed HTTP-200 response is mislabeled as a proven version burn. This schema-failure path and the JSON parse path at lines 120-121 both return conflict, whose contract tells the operator to bump and re-tag. A transient malformed API/CDN response proves neither absence nor incompatible published files; the final reconciler correctly models the equivalent condition separately as retryable invalid-response. Add an indeterminate or invalid-response status that still exits nonzero but instructs a clean refetch, and reserve conflict for validated file-set, yank, or digest mismatches.

[caohy1988]

Fixed in ac106d2. The gate gains an indeterminate status: malformed HTTP-200 bodies, schema violations, and unparseable JSON exit nonzero with explicit "refetch the index and re-run before taking any recovery action" advice; a missing local dist file is likewise indeterminate (investigate the artifact). conflict — and its bump/re-tag contract — is now reserved for VALIDATED file-set, yank, or digest deviations, matching the reconciler's invalid-response modeling. Tests: test_invalid_schema_is_indeterminate_not_a_burn, test_missing_local_distribution_is_indeterminate, test_unparseable_body_is_indeterminate.

[caohy1988]

[P2] Finalize downloads every release asset before validating the expected set. This iterates attacker- or operator-controlled release metadata and streams all assets to disk; exact-set rejection happens only later in reconcile_release.py. One unexpected large asset or many extras can exhaust runner disk or the 15-minute job timeout before the intended recovery classification, and every rerun repeats the download. Validate metadata first against exactly the four expected hardcoded names, reject duplicate/invalid IDs and unreasonable sizes, then download only those four asset IDs to hardcoded destinations and keep the existing byte verification afterward.

[caohy1988]

Fixed in ac106d2. Finalize now validates asset METADATA before any byte is streamed: malformed entries (non-numeric ids, empty/non-string names, negative sizes), duplicate names, and names outside [A-Za-z0-9._+-] fail closed with no download. Only the four expected hardcoded names are downloaded — by asset ID, to hardcoded destinations — and only when the reported size is within a 100 MiB cap. Unexpected or oversized assets are never streamed; they become EMPTY placeholder files so the tested reconciler still sees the exact-set / byte-identity mismatch and emits its existing partial/draft-invalid classification instead of the job dying on disk or timeout. Reruns therefore never repeat an unbounded download.

[caohy1988]

Round 14 fresh review of exact head ac106d2: 1 P1 and 7 P2 findings. I re-read the full 5,275-line PR diff plus the round-13 delta, rechecked the live PR head and all 23 checks, ran the producer suite locally (517 passed, 4 skipped), and reran formatting, shellcheck, compile, YAML-parse, and diff-hygiene checks. The green PR checks do not exercise the tag-only GitHub credential boundary that causes the P1 below.

I also discarded one adversarial checkout-permission candidate after verifying this is a public repository (public checkout works without contents: read), so it is not included.

The PR description itself is now stale: it still says ten rounds, a nine-state contract, old test counts, and automated stale-draft deletion. Please refresh it after addressing the inline findings.

[caohy1988]

[P1] The new immutable-policy preflight cannot authenticate with this job token. publish_release_body.py:243 calls GET /repos/{owner}/{repo}/immutable-releases, but GitHub requires repository Administration: read for that endpoint (endpoint contract). This job grants and passes only the workflow GITHUB_TOKEN with contents: write; Administration is not one of the permissions a workflow can grant that token (available GITHUB_TOKEN permissions). Therefore enabling immutable releases does not unblock finalize: the helper's first API call still fails and every complete release remains a draft. The unit fake always returns enabled=true, so all 517 tests miss the credential boundary. Use a narrowly scoped GitHub App installation token with Administration-read for this policy read (GitHub's prescribed fallback); keep the standard GITHUB_TOKEN for release reads/PATCHes rather than broadening the admin token.

[caohy1988]

Fixed in f83011a. finalize mints a GitHub App installation token via actions/create-github-app-token (SHA-pinned v3.2.0) with permission-administration: read, only when state == complete (non-publishing paths never need it, so a missing App configuration cannot break classification-only runs). publish_release_body.py takes --admin-token-env and routes ONLY the immutable-releases read through that credential; every release read/PATCH keeps the job GITHUB_TOKEN. The unit fakes now enforce the boundary from both sides — the job-token fake refuses the policy path and the admin fake refuses everything else — plus regressions for a failing/absent admin credential (TestCredentialBoundary). One-time App setup (BQAA_RELEASE_POLICY_APP_ID variable + BQAA_RELEASE_POLICY_APP_PRIVATE_KEY secret, App installed with only Administration:read) is documented in RELEASING.md and the workflow header.

[caohy1988]

[P2] Extra local distributions bypass the precheck and are uploaded irreversibly. check() verifies that the expected wheel and sdist exist, but never requires the local directory's file set to equal those two names. After the workflow strips the plugin and SHA256SUMS, gh-action-pypi-publish uploads every distribution remaining in the default dist/ directory (action contract). I reproduced the checker returning absent with a third local wheel present. A future build change that emits an extra platform wheel can therefore publish it to TestPyPI/PyPI; finalize only then rejects the extra anchor entry, after the version is burned. Require the local publish set to be exactly wheel+sdist before either absent or satisfied, or copy exactly those files into a fresh publish directory and point both the checker and action at it.

[caohy1988]

Fixed in f83011a. check() now requires the local publish directory's file set to be EXACTLY wheel + sdist before returning either absent or satisfied; any extra local distribution returns indeterminate (exit nonzero, upload step never runs) with detail naming the extras — reproducing your third-wheel scenario is test_extra_local_distribution_blocks_the_upload, asserted for both the 404 and 200 index paths.

[caohy1988]

[P2] An exact TestPyPI publication plus a missing GitHub release gets an impossible recovery. Reproduction: release_dir=None, production 404, and a complete digest-identical TestPyPI response returns missing-all, whose dispatch says to restart github-release from the original attempt. But guard_existing_release.decide('absent', 'absent', 'present') rejects that same state as a cross-surface partial and says not to rebuild; I executed both paths and got the contradictory messages. A full rebuild cannot help because timestamps change the bytes, and the guard does not validate whether TestPyPI matches the preserved anchor. Either let the original-attempt guard validate exact TestPyPI bytes against dist/ and recreate the draft, or classify this as burned and give one consistent recovery. Add a cross-module regression covering reconciler output followed by guard behavior.

[caohy1988]

Fixed in f83011a with your first option: the guard's index states are now BYTE-VALIDATED. The github-release job copies wheel+sdist into a gate directory and runs check_index_publication.py against each present index, so decide() sees absent | exact | deviating. Release-missing + production-404 + TestPyPI-exact now PROCEEDS to recreate the draft from the preserved original-attempt artifact (the upload stage is then satisfied without re-upload) — exactly the reconciler's missing-all recovery — while a full rerun's rebuilt bytes naturally classify as deviating and get an explicit burn. Cross-module regressions (TestCrossModuleConsistency) execute reconciler output followed by guard behavior for the missing-all/exact, missing-release/prod-exact, and testpypi-partial/deviating states and assert the recoveries agree.

[caohy1988]

[P2] The promised Latest assertion fails open on every API error. _fetch_json() collapses auth, rate-limit, transport, and malformed responses into None, and this branch deliberately returns success; test_unreadable_latest_is_noted_not_fatal locks that in. make_latest=false constrains this PATCH only—it does not prove an already-published release was not later made Latest—so an idempotent rerun can report canonical success while the tracing release is actually Latest. Preserve HTTP/error classification, tolerate only the explicit no-Latest 404 case, and fail closed on 401/403/429/5xx/transport/unparseable responses.

[caohy1988]

Fixed in f83011a. run_gh now returns (rc, stdout, stderr) and _latest_problem classifies: only an explicit HTTP 404 (no Latest exists — ours certainly is not it) passes; 401/403/429/5xx/transport failures and unparseable bodies all FAIL with "refusing to assume" messaging, on the idempotent path too. test_unreadable_latest_is_noted_not_fatal is replaced by TestLatestAssertion covering the 404-pass, each error class, unparseable bodies, and the idempotent rerun.

[caohy1988]

[P2] Checking the repository setting before the release state masks an already-public mutable release with false draft recovery. This branch returns before fetching the release and always says it is still an unpublished draft. I reproduced setting={enabled:false} plus a matching draft=false, immutable=false snapshot: only the setting call runs and the helper tells the operator to enable and rerun, instead of issuing the documented burn/containment guidance. That can leave a customer-visible mutable release exposed while the operator follows the wrong action; only the next run, after enabling the setting, discovers the burn. Fetch and classify the exact release first: a published mutable object must fail immediately with burn guidance, while the setting remains a mandatory precondition only for the draft=true publication path.

[caohy1988]

Fixed in f83011a. The exact release object is fetched and classified FIRST; the policy setting now gates only the draft-publication path. Your reproduction — setting disabled + a published mutable snapshot — now fails immediately with the burn/containment guidance and never reaches the policy read (test_published_mutable_release_gets_burn_guidance_even_with_setting_off asserts the admin credential is never called and that no "still an unpublished draft" advice is emitted).

[caohy1988]

[P2] A missing or malformed draft field falls through to publication. verify_snapshot() validates ID, tag, and assets but not visibility, and this condition only special-cases literal False; None, a missing field, or a string all proceed to the PATCH with draft=false. That contradicts this helper's fail-closed snapshot contract. Require draft to be a Boolean, allow only True into the publish branch and False into the published branch, and add malformed-response regressions. The workflow's earlier .draft classification should make the same schema check rather than treating every non-false value as a draft.

[caohy1988]

Fixed in f83011a. verify_snapshot() requires draft to be a Boolean, so a missing/None/string flag is a snapshot problem that refuses publication before any PATCH; the publish branch is reached only via literal True/False. Regressions: TestDraftFlagSchema (None, "false", "true", 0, and a missing field). Both workflow jq classifications (guard discovery and finalize) now make the same schema check — .draft | type == "boolean" — and fail as indeterminate instead of treating non-false values as drafts.

[caohy1988]

[P2] The round-13 asset trust boundary is tag-only inline shell with no executable coverage. These lines implement malformed-metadata rejection, duplicate/unsafe-name handling, the 100 MiB cap, the allowlist, placeholder semantics, and by-ID downloads, but no PR test executes the block or references MAX_ASSET_BYTES, releases/assets, or its error strings. The current unit tests begin only after files already exist in a release directory, so a regression here first runs while cutting a real tag. Extract asset planning into a PR-tested helper (or execute a marked shell section with stubbed gh/jq) and cover malformed metadata, duplicate/unsafe names, oversized expected assets, unexpected assets, empty sets, and a denied asset GET; assert only the four bounded expected IDs can be downloaded.

[caohy1988]

Fixed in f83011a. The trust logic is extracted into PR-tested scripts/plan_asset_downloads.py: it validates metadata (positive integer ids — booleans rejected, non-empty safe names with '.'/'..' excluded, non-negative integer sizes, duplicate rejection) and emits a deterministic plan — download <id> <name> only for the four expected hardcoded names within the size cap, placeholder <name> for everything else. The workflow consumes the plan verbatim. Tests cover malformed metadata (10 shapes), duplicate/unsafe names, oversized expected assets, unexpected assets, the empty set, the CLI contract, and the core assertion that downloads ⊆ the bounded expected set. A denied asset GET remains covered by set -euo pipefail propagation in the consuming loop (verified in bash directly in round 12).

[caohy1988]

[P2] This still documents automatic stale-draft deletion, directly contradicting the round-13 behavior below. Lines 63-69 say the draft is deleted when both indexes return 404, while lines 73-77 and guard_existing_release.py correctly say the workflow never deletes a release and requires manual verify-and-delete. Rewrite this paragraph so both the normal flow and troubleshooting contract say the guard blocks and an operator may manually delete only after both explicit 404s.

[caohy1988]

Fixed in f83011a. The RELEASING.md item-4 paragraph is rewritten: the guard never deletes; it classifies drafts via the authenticated release list and byte-validates each index (absent/exact/deviating), a stale draft with both indexes absent BLOCKS with manual verify-and-delete instructions, exact-index states route to original-attempt reruns (or draft recreation when only TestPyPI holds the exact bytes), and deviating bytes are a burn. The normal-flow paragraph and the troubleshooting section now state the same contract. The stale PR description is also refreshed (14 rounds, 11-state contract, 542 tests, no automated deletion, App-token + immutable-releases one-time setup).

[caohy1988]

Round 15 fresh review: one P2 finding after a full diff read and local verification (542 passed, 4 skipped; format/import/static workflow checks clean).

[caohy1988]

[P2] Do not mint the policy token for an already-published idempotent rerun. complete is also the reconciler result for an exact release with release_published=True (test_published_release_with_complete_state_stays_complete), while the helper deliberately returns from the published/immutable path before its policy read and test_published_immutable_canonical_is_a_no_op asserts zero admin calls. This condition nevertheless runs the App-token action first. As a result, missing or rotated App credentials fail an otherwise idempotent rerun before the helper can recognize the finalized release (and can mask the helper’s burn guidance for an exact-but-public mutable release); it also contradicts RELEASING.md’s promise that the token is minted only when publication is about to happen. Export the matched release visibility/needs_policy_token from reconciliation and gate this step on complete && draft, then cover that workflow-level condition with a regression.

[caohy1988]

Fixed in e11179a. The finalize reconcile step now exports RELEASE_IS_DRAFT (default 0; flipped to 1 only by the Boolean-checked draft classification), and the mint step is gated on env.state == 'complete' && env.RELEASE_IS_DRAFT == '1' — the already-published idempotent rerun and the exact-but-public mutable burn path never touch the App credentials, so missing/rotated keys cannot break them or mask the burn guidance. The requested workflow-level regression is the new test_release_workflow_contract.py: it pins the compound condition on the App-token step itself, the visibility export (default-first ordering, =1 write inside the Boolean draft branch), the ADMIN_GH_TOKEN wiring into the publish helper, and that exactly one mint exists with no other bare state == 'complete' guard that could re-widen the condition. RELEASING.md's token-mint promise updated to match.

[caohy1988]

Final fresh review at e11179a: no blocking code findings.

I re-read the full diff against d49ecd6 and re-audited the release state machine, credential boundary, rerun behavior, asset/download trust boundary, shell/API failure modes, documentation, and tests. The round-15 fix is correct: RELEASE_IS_DRAFT defaults to 0, is set to 1 only for a Boolean-confirmed draft, and the Administration:read App token is minted only for complete && draft. The already-published immutable no-op and published-mutable burn paths do not require or touch the App credential.

Verification:

• local full producer suite: 546 passed, 4 skipped
• focused workflow/publish contract suite: 35 passed
• pyink + isort clean
• workflow YAML and all extracted run: blocks parse
• shellcheck/bash syntax, compileall, and git diff --check clean
• live CI: 23/23 terminal and expected (22 success; conditional zizmor-output skip)

One non-blocking bookkeeping item: the PR description still says fourteen review rounds and 542 tests; current reality is fifteen rounds and 546 tests.