Skip to content

fix(release): wait before the final checksum validation - #2779

Draft
Nitnek wants to merge 1 commit into
NotRexed:mainfrom
Nitnek:fix/release-final-delay-20261112
Draft

Nitnek wants to merge 1 commit into
NotRexed:mainfrom
Nitnek:fix/release-final-delay-20261112

Conversation

@Nitnek

@Nitnek Nitnek commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

Release jobs can exhaust the checksum-replacement budget without checking whether their last upload has become visible. This adds one delayed, read-only final validation rather than another replacement.

What changes

  • Preserve the existing loop with at most five logical replacement calls.
  • If it has not converged, wait five seconds after the last replacement, recompute checksums from a fresh asset listing and compare the exact remote SHA256SUMS bytes once more.
  • Do not perform a sixth replacement. Existing internal upload retries are unchanged: five logical replacements can still involve up to fifteen checksum POST attempts.
  • Keep persistent inconsistency, failed final listings and failed output copies as failures.
  • Add a deterministic regression that makes visibility advance after waits, not after a count of reads, and checks the five-replacement/five-wait bound.

Fresh local validation

Base: 51da0464b4cf0c7f56b81ebd6bb309325835e175 (current main at qualification).
Head: 90c63184c52fb1780c578390e8ba8c343f4f0b5c.
Linux, Python 3.14.9, isolated -I -B invocations; no real release API calls.

  • bash -n scripts/release_files.sh: exit 0.
  • New temporal regression on that exact base's production script, with only the candidate test fixture overlaid: RED, 1 test, exit 1, expected SHA256SUMS still differs from the files after 5 uploads.
  • Same regression on the candidate: GREEN, 1 test, exit 0.
  • python -I -B tests/test_release_scripts.py -v: the first candidate run failed only in the existing concurrent-nightly test; the unchanged follow-up passed 34/34, exit 0.
  • python -I -B tests/run_all.py --strict --jobs 1 release_scripts: the first run failed on the same concurrent-nightly test; the unchanged follow-up passed 1/1 checks, no skips, exit 0.
  • Production and test files are byte-identical to the independently reviewed snapshot, transplanted without logical adjustment. Only scripts/release_files.sh and tests/test_release_scripts.py differ from the base.

Existing intermittent failure, kept visible

This is not an all-green history. In this fresh qualification, the original, clean base's release suite failed 1 of 33 tests in test_nightly_two_runs_at_once_end_at_the_newer_commit. The candidate's first direct suite failed 1 of 34 tests in the same test; its first targeted runner also failed there (both ordering subcases), with the same checksum-exhaustion assertion and HTTP 404 diagnostics.

An isolated replay on the exact base production with the byte-identical candidate fixture reproduced that assertion immediately. The other exercised release scripts were also byte-identical. A further interleaved baseline replay passed, then the unchanged candidate runner passed. Complete stdout/stderr, native exits and commit/interpreter/file identities are retained for every attempt. Subprocesses mentioned by ResourceWarnings were checked afterwards; no active fixture child remained. This patch does not claim to solve the broader concurrent-nightly race.

Prior review and limits

The prior independent review was on base 467805032c70d21cb1e5e633faf7eb556b02935f; both changed files on current main were unchanged from that base. Its separate, uncommitted diagnostic probes covered twelve synthetic scenarios, including persistent invisible/divergent checksums, final-listing and checksum-read errors, asset changes during the final wait, nested retries, output-copy failure and fast paths. Those are historical bounded review evidence, not twelve new tests run on current main or added to this PR.

Assisted-by: GPT-5.6-terra and GPT-6.1-sol
Co-Authored-By: GPT-5.6-terra noreply@openai.com
Co-Authored-By: GPT-6.1-sol noreply@openai.com

After the final replacement, allow visibility to advance before
recomputing assets and comparing SHA256SUMS one last time. Keep
the five logical replacement limit and existing transport retries.

Assisted-by: GPT-5.6-terra and GPT-6.1-sol
Co-Authored-By: GPT-5.6-terra <noreply@openai.com>
Co-Authored-By: GPT-6.1-sol <noreply@openai.com>
hckrman101 pushed a commit that referenced this pull request Nov 12, 2026
…is read again (flaky nightly test) (#2822)

`tests/test_release_scripts.py`'s
`test_nightly_two_runs_at_once_end_at_the_newer_commit` fails now and
then on the ubuntu part 2 check (klimovich008 saw it on #2708 and #2732
today), and the same race can fail a real nightly when the Linux and
Windows runs finish together: `release_files.sh finish` gives up with
"SHA256SUMS still differs from the files after 5 uploads".

## Cause

From the failing runs' logs (e.g. run 43237869157): after `nightly:
moved forward ...` and its uploads, the run printed `gh: Not Found (HTTP
404)` three times, then the five-uploads error. `uploaded()` lists the
release's files, then reads SHA256SUMS by its id; the other platform's
run replaces SHA256SUMS (a new id) between those two calls, the read
404s, and `|| true` turned that into "no SHA256SUMS". So the run uploads
it again, which replaces it under the other run's read, and the two runs
ping-pong until one has used its five uploads (in the tests `sleep` is a
no-op, so the runs retry in step).

## Change

- `uploaded()` lists and reads again (up to three times) when the read
fails, so a file replaced meanwhile is read under its new id instead of
counting as missing. A read that keeps failing still answers nothing, as
before.
- The stand-in gh can replay that race (`race: {"SHA256SUMS": n}`: the
next n reads of it find it replaced under a new id), and
`test_sums_replaced_between_listing_and_read_are_read_again` checks that
`check` passes and `finish` uploads nothing through two such
replacements. On main's script `check` fails (the empty read) and
`finish` uploads SHA256SUMS again.

Overlap: #2779 (Nitnek, draft) adds one more check after the fifth
upload in `finish()`; this PR does not touch `finish()` and the two
merge in either order. Both together: this one stops the ping-pong,
#2779 catches a last upload not yet visible. #2626 changes other lines
of `finish()`.

## Tested

- `python -I -B tests/run_all.py` on Windows 11 (419 passed; 5 failed,
the same 5 as main on this PC: compiler headers and Python modules
here). `test_release_scripts.py` is skipped on Windows (it needs `fcntl`
and `jq`), so its run is this PR's Linux CI.
- `bash -n scripts/release_files.sh`.
- Not tested: the flake's own rate. It is a race; the new test pins the
mechanism deterministically, and the ubuntu part 2 check on later PRs is
where it would show again.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: breakthetargets <81792782+breakthetargets@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@Nitnek

Nitnek commented Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

Compatibility follow-up against main 4cf785a0b143bb34026506c9a3fe4f4bb93c797d and this Draft's unchanged head 90c63184c52fb1780c578390e8ba8c343f4f0b5c. I checked the already-merged #2822 instead of implementing a competing fix.

The two changes address different observations:

I ran both existing deterministic public regression tests under Linux, with the fixture gh storing only SYNTHETIC state. The fixture assertions and production inputs were kept unchanged except for the explicitly described local composition:

Production source Replaced-asset-ID test from current main Last-upload-visibility test from #2779
main 4cf785a0 PASS / exit 0 expected FAIL / exit 1
original #2779 90c63184 expected FAIL / exit 1 (it lacks #2822) already covered by this PR's original qualification
private composition: current main + this PR's exact final-read block PASS / exit 0 PASS / exit 0

Those are five fresh test invocations, including two deliberate negative controls—not “five green tests”. The combined temporal test still asserts five logical checksum uploads and five simulated waits. The replaced-ID test asserts that its two replacements are exhausted and check/finish do not upload another checksum unnecessarily.

The composition retained main's uploaded() byte-for-byte and added only the existing #2779 final-read block to finish(). Source hashes were verified before/after, Bash syntax checks passed, and complete script outputs/mock call-state records were retained before fixture cleanup. No branch/head/rebase was changed publicly.

This is bounded compatibility evidence using two different fixture versions, not a whole-suite run, a real GitHub release/API test, hosted CI, or a measured flake rate for the concurrent-nightly test. The Draft status remains appropriate. A future integration should retain #2822 and its deterministic regression, rather than treating these changes as alternatives.

AI-assisted analysis: Hermes Agent (OpenAI GPT-6.1-sol).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant