fix: parity-hardening (bounded retry + previous-tag floor/history)#2
Conversation
- bounded 5xx retry around read-only forensic API calls in validate + resolve - previous-tag minAppVersion floor (must not decrease) and versions.json full-history equality, in release-contract.mjs (validate-previous-tag) with tests, wired into the release build job and inline into release-validate
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70b91caa07
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const previousMinAppVersion = assertMinAppVersion( | ||
| previousManifest.minAppVersion, | ||
| "previous tag", | ||
| ); | ||
| const baseMinAppVersion = assertMinAppVersion(baseManifest.minAppVersion, "base"); |
There was a problem hiding this comment.
Validate previous tag metadata before trusting its floor
On the release.yml build path this CLI is the only previous-tag history check, but it trusts previousManifest.minAppVersion without first verifying that the previous tag's own metadata is synchronized, e.g. that previousVersions[previousManifest.version] matches it. If the previous tag is moved or otherwise points at a commit where manifest.json has a lowered floor while versions.json is unchanged, the history equality below still passes and the no-decrease comparison is made against the tampered lower value; the supported manual workflow_dispatch recovery path for release.yml does not run the inline release-validate metadata check. Validate the previous tag's version/history mapping before using its floor.
Useful? React with 👍 / 👎.
Parity-hardening for the shared release pipeline
Closes two defense-in-depth gaps the PodNotes parity audit (chhoumann/PodNotes#289)
found in the shared reusables, before the remaining three repos migrate.
1. Bounded retry around read-only forensic API calls
A transient 5xx from the GitHub API must not abandon a release the maintainer has
already merged. Added a small
retryRead(thunk)helper (up to four attempts, onlyon HTTP 5xx, linear backoff) and wrapped the read-only calls in
release-validate(the merge-time forensics) and
release.yml'sresolvejob. Writes - therecovery-branch
createRefand thecreateWorkflowDispatch- are deliberatelyNOT retried.
2. Previous-tag minAppVersion floor + full-history equality
minAppVersionandversions.jsononly change in lockstep with a release, so bothinvariants are really about previous-tag vs base:
minAppVersionmay only rise, never drop, from theprevious release to the base (and both must be stable semver).
versions.jsonis append-only and only a release PR editsit, so between two releases it must be byte-identical.
Both live in
release-contract.mjsasvalidatePreviousTagHistory(newvalidate-previous-tagCLI command) with unit tests, plus a stable-semver +no-decrease floor in
validateVersionFiles. Enforced at both gates: the releasebuildjob (extracts the previous tag'smanifest.json+versions.jsonand runsthe command;
fetch-tags: trueguarantees the tag is present) and inline inrelease-validateat merge time.Note (no action)
The merge-subject
(#PR)suffix becoming mandatory is accepted asstricter-not-weaker, per the audit.
Validation
pnpm test: 43/43 (was 38; +5 covering the floor and previous-tag checks, npm+pnpm).validate-previous-tag(happy path, lowered floor, rewritten history).actionlintclean on all workflows; pyyaml parses everything; no em-dashes.