Repository navigation
fix(release): publish Sparkle minimum system version as an item element - #1004
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
PR SummaryMedium Risk Overview
Reviewed by Cursor Bugbot for commit d013b59. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Codex review: needs maintainer review before merge. Reviewed October 7, 2026, 6:45 PM ET / 22:45 UTC. ClawSweeper reviewWhat this changesThe PR moves the minimum macOS version into Sparkle’s supported update-item element and updates the feed, release validator, regression tests, and documentation. Merge readiness✅ Ready for maintainer review This repair remains necessary: current main and v4.9.0 still use the enclosure attribute Sparkle ignores. The patch matches the pinned Sparkle parser contract, preserves other feed metadata, and has no actionable correctness findings. Priority: P2 Review scores
Verification
How this fits togetherPeekaboo’s release tooling generates the update feed consumed by Sparkle in the macOS app. Sparkle uses each update’s minimum macOS version to decide whether that update is compatible with the user’s system. flowchart TD
A[Release artifact metadata] --> B[Appcast generator]
B --> C[Release validation]
C --> D[Published update feed]
D --> E[Sparkle compatibility check]
F[User macOS version] --> E
E --> G[Eligible updates]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep the generator, validator, and published feed aligned with Sparkle’s item-element contract while preserving existing version requirements and signed artifact metadata. Do we have a high-confidence way to reproduce the issue? Yes, source inspection establishes the mechanism: current main emits an enclosure attribute while the pinned Sparkle parser reads the item element. No live macOS update-selection reproduction was executed. Is this the best way to solve the issue? Yes. Correcting placement and its validation together is a narrow repair of the existing Sparkle contract, and the feed comparison confirms unrelated release metadata is preserved. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 60b541e8f340. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
Summary
Sparkle reads an update's minimum macOS version only from the
<item>child element<sparkle:minimumSystemVersion>. Its parser (SUAppcastItem.m) takes the value from the item's child-element dictionary; the enclosure's attributes live in a separate dictionary that never feeds it. Peekaboo's appcast generator wrote the value as an<enclosure>attribute, and the release validator checked that attribute. Sparkle silently ignored it, so all 45 4.x and late-3.x feed entries carried a minimum-version constraint that never applied.Today this is harmless, because every Peekaboo build that reads the feed already requires macOS 15. It would have broken the next time the minimum moves: older systems would be offered an update they cannot run.
scripts/update-appcast-entry.mjsnow emits<sparkle:minimumSystemVersion>as an item child before<enclosure>.validateAppcastchecks that element, so an entry with only the attribute, or with the wrong value, now fails. All other emitted bytes are unchanged. The release driver's use ofvalidateAppcastkeeps its strict semantics.appcast.xml: every enclosure-attribute entry (45, all15.0, including 4.9.0) moves to the element form, placed directly before<enclosure>. The two 3.9.x entries already in element form are byte-identical. With only the minimum-version fields stripped, the old and new feeds are byte-identical.docs/RELEASING.mddocuments the element requirement, andCHANGELOG.mdgets one Unreleased bullet.Found during the 4.9.0 release-branch merge review.
Tests
node scripts/test-update-appcast-entry.mjs: covers the generated element, no enclosure attribute, rejection of a wrong value or an attribute-only entry, and no attribute form in the checked-in feed.node scripts/test-release-driver-contract.mjsbash scripts/test-release-binary-reuse.shnode --test tests/release-preflight-contract.test.mjs(28/28)node scripts/docs-lint.mjsxmllint --noout appcast.xmlmain: byte-identical outside the moved minimum-version fields.