fix(upgrade): use generated-patch from-version + close output fd synchronously - #1327
Merged
Conversation
Contributor
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d40b920. Configure here.
| if (streamError === undefined) { | ||
| throw endErr; | ||
| } | ||
| log.debug(`writer.end failed after a stream error: ${String(endErr)}`); |
Contributor
There was a problem hiding this comment.
Stream error masked by close
Low Severity
When decompression fails and closeSync also fails, writeError is thrown first and the original streamError is discarded. The comment says the stream error is preferred, and the previous createWriteStream path did prefer it. Callers can then see a close failure instead of the real gzip/stream root cause.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit d40b920. Configure here.
| PREV_TAG="$PREV_TAG_NIGHTLY" | ||
| else | ||
| PREV_TAG="$PREV_TAG_RELEASE" | ||
| fi |
Contributor
There was a problem hiding this comment.
Unreachable release tag wiring
Low Severity
prev-release-tag is recorded by generate-patches and selected in the publish-nightly else branch, but that job only runs on main, so the release path never executes. The stable from-version plumbing added here is dead code and can give false confidence that release-branch patches get the threaded annotation.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit d40b920. Configure here.
…hronously Two related fixes for the self-upgrade flow: 1) CI workflow annotation mismatch (Bug #1). The `generate-patches` and `publish-nightly` jobs independently recomputed `PREV_TAG` from `oras repo tags`. They could — and did — disagree: for the `patch-0.40.0-dev.1785511994` image, generate-patches generated bytes from `0.41.0-dev.1785504113` (the semver-previous) but publish-nightly stamped `from-version: 0.40.0-dev.1785506255` in the annotation. `binpatch` verifies only the final output SHA, so this silently bypassed the user's expected upgrade path and produced SHA mismatch on apply. Pass `PREV_TAG` from generate-patches to publish-nightly via job outputs (`prev-tag` / `prev-release-tag`) so the annotation always matches the actual source. 2) ETXTBSY on full-download fallback (Bug #2). `streamDecompressToFile` used `fs.createWriteStream` + `await writer.end()` — Node resolves the `end` callback on the `'finish'` event (data flushed), but the kernel fd release is async. A subsequent `spawn(tempPath)` then failed with ETXTBSY ('text file busy'). The `fs.writeFile` raw fallback path didn't exhibit the bug because it closes the fd synchronously. Replaced with `fs.openSync` + `fs.writeSync` + `fs.closeSync` so the fd is released before this function returns. Also drops the stale 'Bun.file().writer() workaround for Bun event-loop GC bug' docstring — project migrated to Node, the referenced Bun issue no longer applies.
BYK
force-pushed
the
fix/upgrade-annotation-and-fd-leak
branch
from
July 31, 2026 18:55
d40b920 to
93cf412
Compare
BYK
added a commit
that referenced
this pull request
Aug 1, 2026
…eries (#1329) ## Problem `generate-patches` was picking the wrong `PREV_TAG` for 0.40 nightly builds, causing patches to be generated from a 0.41 source and stamped with `from-version: 0.41.x-dev.Y` — useless for any user on the 0.40 series. Their chain resolution then fails (their version is chronologically before 0.41) and the upgrade falls back to full-download every time. ## Repro on the live registry ``` ghcr.io/getsentry/cli:patch-0.40.0-dev.1785524715 from-version: 0.41.0-dev.1785504113 ← should be 0.40.0-dev.1785522378 ghcr.io/getsentry/cli:patch-0.40.0-dev.1785526951 from-version: 0.41.0-dev.1785504113 ← should be 0.40.0-dev.1785524715 ``` ## Root cause The PREV_TAG loop in `generate-patches`: ```bash TAGS=$(oras repo tags "${REPO}" 2>/dev/null | grep '^nightly-[0-9]' | sort -V || echo "") for tag in $TAGS; do if [ "$tag" = "nightly-${VERSION}" ]; then break; fi PREV_TAG="$tag" done ``` `sort -V` orders `0.41.0-dev.X` after every `0.40.0-dev.Y` (because `0.41 > 0.40` lexicographically). At the time `generate-patches` runs, the current build's tag hasn't been pushed yet — so the `if [ "$tag" = "nightly-${VERSION}" ]` break never triggers. The loop walks past every 0.40 tag and onto the 0.41 tag, and PREV_TAG becomes the 0.41 nightly. ## Fix Filter `TAGS` to only those in the same `major.minor` series as the current build before iterating. `MAJOR_MINOR` is parsed from VERSION (`echo "${VERSION}" | cut -d. -f1,2` → e.g. `0.40`). For 0.40 builds the loop only sees `nightly-0.40.0-dev.*` tags; for 0.41 builds only `nightly-0.41.0-dev.*`. The first build of a new series (no prior tags in the same series) falls through with `PREV_TAG=""` and the existing HAS_PREV skip handles it cleanly. ## Retroactive remediation (follow-up) The two mis-annotated patches above have correct bytes but wrong annotations. Re-stamping them with the correct `from-version` will let existing 0.40-series users chain through them to the latest. Tracked as a separate follow-up — they're already on the registry, just need the annotation re-pushed. ## Relationship to PR #1327 PR #1327 prevents annotation drift between `generate-patches` and `publish-nightly` (they now agree via job outputs). This fix prevents the initial PREV_TAG selection from being wrong in the first place. Together they make the CI pipeline produce correct annotations end-to-end. ## Verification - Local simulation: `MAJOR_MINOR` filter + iteration over same-series tags returns `nightly-0.40.0-dev.1785522378` for VERSION=`0.40.0-dev.1785524715` vs. the current broken `nightly-0.41.0-dev.1785504113`. - Workflow YAML parses cleanly. - Will re-verify against a real build after merge (next nightly).
BYK
added a commit
to BYK/binpatch
that referenced
this pull request
Aug 1, 2026
Closes #40. The `publish-ghcr` mode re-derived the previous version from the registry independently of the `generate-ghcr` step that actually downloaded the source binary and ran bsdiff. If the registry state changed between the two steps, the two values could disagree and `binpatch` consumers would silently hit SHA mismatch (the library only verifies the final output SHA, not the source binary identity — the annotation is a chain pointer, not a content hash). ## Fix Add a new `from-version` input on `publish-ghcr`. When set, the annotation on the pushed patch manifest uses THIS value verbatim. When empty (back-compat), the old registry re-derive runs and prints a hint suggesting the caller thread the value from `generate-ghcr`. ## Consumer wiring ```yaml - uses: BYK/binpatch/action@vX.Y.Z id: gen with: { mode: generate-ghcr, ... } - uses: BYK/binpatch/action@vX.Y.Z with: mode: publish-ghcr from-version: ${{ steps.gen.outputs.from-version }} ``` ## Files - action/action.yml: add `from-version` input, thread to push_patches via env, use input when set else re-derive - action/README.md: document the new input and the gen→publish wiring ## Relationship to other fixes - #37 — fd-close fix in applyReaderToFile (released as 0.4.0) - getsentry/cli#1329 — same-series filter in custom workflow (now merged) - getsentry/cli#1327 — CI workflow annotation threading (now merged) Combined, the chain-side fixes (binpatch fd-close + getsentry/cli CI workflow) and this action-side fix mean the consumer can stop running its own hand-rolled patch generation/publish and consume the binpatch action directly. ## Verification - pnpm test: 58 passed - pnpm build: clean - pnpm typecheck: clean - pnpm lint: (no script)
BYK
added a commit
that referenced
this pull request
Aug 1, 2026
…lish (#1330) Replaces the hand-rolled ~250 lines of `generate-patches` and `publish-nightly` bash in `.github/workflows/ci.yml` with the `BYK/binpatch/action@v0.4.1` composite action. The action handles: - ORAS CLI install + GHCR login - Previous-nightly discovery (with the same-series filter that fixes the cross-major nightly tag confusion) - zig-bsdiff invocation + patch size gate (50% ratio) - Binary + versioned-tag + patch manifest push with `from-version` annotation threaded via action outputs This eliminates the divergence bug we hit twice: - The PREV_TAG recomputation divergence between generate-patches and publish-nightly (PR #1327 fixed this in the hand-rolled shell; the action solves it generically by exposing `generate-ghcr.outputs.from-version` and consuming it as a `publish-ghcr` input — see BYK/binpatch#40) - The cross-major nightly tag confusion (PR #1329 fixed this in the hand-rolled shell; the action's `validateChainStep` does the equivalent check at chain-resolution time) Also bumps the `binpatch` runtime dep from `^0.3.1` to `^0.4.1` for the new `openSync`/`writeSync`/`closeSync` output path (which avoids the ETXTBSY race when chaining into a `spawn(tempPath)`). ## Files - `.github/workflows/ci.yml`: -220 lines of hand-rolled shell, +10 lines of action wiring - `packages/cli/package.json`: binpatch ^0.3.1 → ^0.4.1 - `pnpm-lock.yaml`: regenerated ## Diff ``` .github/workflows/ci.yml | 347 +++++++------- packages/cli/package.json | 2 +- pnpm-lock.yaml | 10 +- ``` ## Verification - `node -e 'require("yaml").parse(...)'` parses cleanly - `pnpm --filter sentry run typecheck`: clean - `pnpm --filter sentry run lint`: clean - Lint passes (no shell-script lint script; YAML is the only surface) --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two related fixes for the self-upgrade flow on nightly and stable channels.
1. CI workflow annotation mismatch (SHA mismatch on delta upgrade)
generate-patchesandpublish-nightlyindependently recomputedPREV_TAGfromoras repo tags. They could — and did — disagree: forthe published
patch-0.40.0-dev.1785511994image,generate-patchesgenerated bytes against
0.41.0-dev.1785504113(the semver-previous insort -Vorder), butpublish-nightlystampedfrom-version: 0.40.0-dev.1785506255in the annotation.binpatchverifies only the final output SHA, so this silently bypassed the user's
expected upgrade path and produced a SHA mismatch on apply — the
patched binary's SHA didn't match the annotation, so the delta failed
and the upgrade fell back to full-download.
Fix: thread
PREV_TAGthrough as job outputs.generate-patchesdeclares
outputs.prev-tag(nightly main) andoutputs.prev-release-tag(stable release branches) and writes them via the step IDs that already
compute the tag.
publish-nightlyreads the value via${{ needs.generate-patches.outputs.prev-tag }}and strips theappropriate prefix (
nightly-for main,vfor release tags — bashparameter expansion is a no-op when the prefix doesn't match).
Verified by inspecting the published
:patch-0.40.0-dev.1785511994manifest on GHCR:
from-versionannotation was0.40.0-dev.1785506255but the patch bytes apply correctly only tothe
0.41.0-dev.1785504113binary. After the fix, the annotationwill match the actual source for every future build.
2. ETXTBSY on full-download fallback
streamDecompressToFile(the .gz streaming path used for nightlymanifest blobs and GitHub Release .gz assets) used
fs.createWriteStream+await writer.end(). Node resolves theendcallback on the'finish'event (data flushed) but theunderlying fd release is async — a subsequent
spawn(tempPath)thenintermittently failed with
ETXTBSY("text file busy"). Thefs.writeFileraw fallback path didn't exhibit the bug because itcloses the fd synchronously before returning, which is why direct
full-version downloads worked but the delta fallback didn't.
Fix: replace
createWriteStreamwithfs.openSync+ manualwriteSync+fs.closeSyncso the fd is released before thefunction returns. Same pattern as the binpatch fix in
BYK/binpatch#37.
Also drops the stale 'Bun.file().writer() workaround for Bun
event-loop GC bug' docstring — the project migrated to Node, the
referenced Bun issue no longer applies.
Verification
pnpm --filter sentry run typecheck— cleanpnpm --filter sentry run test:unit— 8754 passed | 15 skippedsentry cli upgrade nightlyon/home/byk/.local/bin/sentry(currently at0.40.0-dev.1785506255$) reproduced both errors before the fix; will re-verify against the merged workflow once a new nightly with the correctedfrom-version` lands.Files
.github/workflows/ci.yml—generate-patchesdeclaresoutputs.prev-tag/prev-release-tag, writes them viaset-prev-tag/set-prev-release-tagstep IDs;publish-nightlyreads from outputs instead of recomputing.packages/cli/src/lib/upgrade.ts—streamDecompressToFilerewritten with synchronous fd close.