Skip to content

Fix the npm read-after-write race I introduced in the publish gate - #196

Merged
brentrager merged 1 commit into
mainfrom
fix/lag-gate-race
Aug 20, 2026
Merged

Fix the npm read-after-write race I introduced in the publish gate#196
brentrager merged 1 commit into
mainfrom
fix/lag-gate-race

Conversation

@brentrager

Copy link
Copy Markdown
Contributor

My bug, from #194, caught in production one release later

#194 replaced if: steps.changesets.outputs.published == 'true' with "does npm already serve this version", so a mid-run failure could no longer silently strand four registries. It reintroduced the same failure through a different door.

npm registry reads are eventually consistent. npm view @smooai/logger@4.5.4 moments after a successful publish still 404s. So on release run 32407821367:

Target version: 4.5.4
npm does not have 4.5.4 yet — nothing to backfill.

…all four steps skipped and the run went green. npm view @smooai/logger version returns 4.5.4 now, but the damage is done:

registry version
npm 4.5.4
PyPI missing
crates.io missing
Go tag missing
NuGet missing

Exactly the class of defect #194 set out to kill, caused by the fix for it. Reporting it plainly because a green run that publishes nothing is precisely the thing nobody notices.

Fix

Two independent signals count as "released", because neither alone is sufficient:

signal why it's needed
steps.changesets.outputs.published == 'true' authoritative the instant it happens, no registry read involved — this is the normal path, and the one the race broke
npm already serves the version the backfill path: what lets a re-run catch up registries an earlier run left behind, which was the entire point of #194

Only when neither holds do the four steps skip — correctly, because that is a run that merely opened the version PR, where package.json is bumped to something nothing has published and the workspace is sitting on an unmerged branch. (That case is real; it's why #194 gained the npm gate at all, and it worked: run 32407587838 logged npm does not have 4.5.4 yet and correctly published nothing.)

Deliberately no changeset

package.json stays at 4.5.4, so merging this takes the backfill path and ships 4.5.4 to the four registries it's missing from. A changeset would bump to 4.5.5 and strand 4.5.4 permanently.

This also serves as the live test of the backfill path against a real drift.

Verification

All three release states, by hand:

just-published (npm may still 404)     -> PROCEED (published this run)
backfill: npm serves it                -> PROCEED (npm serves it — backfill)
version PR only, unreleased            -> SKIP ALL FOUR

pnpm format:check clean; check-versions: OK (all manifests at 4.5.4).

🤖 Generated with Claude Code

https://claude.ai/code/session_0152bbE1veqfG1SVJdyLCBxC

#194 made each registry publish gate on "does npm already serve this version",
to stop a mid-run failure silently stranding four registries. It reintroduced
the same failure through a different door: npm registry READS are eventually
consistent, so `npm view @smooai/logger@4.5.4` moments after a successful
publish still 404s. The gate concluded "not released", skipped PyPI, crates.io,
the Go tag and NuGet, and the run went green — leaving 4.5.4 on npm alone.

Two independent signals now count as released, because neither alone is enough:

  - changesets published it in THIS run (steps.changesets.outputs.published).
    Authoritative the instant it happens, no registry read involved. This is the
    normal path and the one the race broke.
  - npm already serves it. The backfill path, which is what lets a re-run catch
    up registries an earlier run left behind — the whole point of #194.

Only when neither holds do the four steps skip, which is correct: that is a run
that merely opened the version PR, where package.json is bumped to something
nothing has published and the workspace sits on an unmerged branch.

Deliberately NO changeset. With package.json staying at 4.5.4, merging this
takes the backfill path and ships 4.5.4 to the four registries it is missing
from. A changeset would bump to 4.5.5 and strand 4.5.4 permanently.

Verified all three states by hand: published-this-run -> proceed even while npm
404s; npm-serves-it -> proceed; unreleased 9.9.9 -> skip all four.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0152bbE1veqfG1SVJdyLCBxC
@changeset-bot

changeset-bot Bot commented Aug 20, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: edd47ad

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@brentrager

Copy link
Copy Markdown
Contributor Author

Correction to the causal claim in #194, isolated after that PR merged — the observation was right, the explanation was wrong, and the risk is higher than I said.

#194 stated Go "refuses to cache" parity_corpus_test.go because it reads ../parity-corpus.json from outside the package dir. The non-caching was real and the #183 positive control was valid, but that is not why.

Isolated in a scratch module:

fixture location re-run after mutating it
outside the package, inside the module (cached) FAIL — content tracked, cache invalidates correctly
outside the module (cached) ok (cached)stale green

The trigger is the module boundary, not the package boundary. Logger's Go module root is go/ and the corpus sits at the repo root, so it is on the dangerous side of that line.

What actually saved it: the corpus test builds a real logger, which writes .smooai-logs/ to disk, and a Go test that writes files is uncacheable. Incidental, and any refactor that stubs out file rotation would silently remove it.

So -count=1 in go:test is load-bearing here, not belt-and-braces. No code change needed — the flag already landed in #194 — but the reasoning in that commit message should not be trusted by the next person, and "I never saw (cached)" is not evidence of safety. Verify by mutation.

@brentrager
brentrager merged commit f15a3e8 into main Aug 20, 2026
1 check passed
@brentrager
brentrager deleted the fix/lag-gate-race branch August 20, 2026 19:23
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