Skip to content

fix(release): publish Sparkle minimum system version as an item element - #1004

Merged
steipete merged 1 commit into
mainfrom
fix/appcast-minimum-system-version-element
Oct 8, 2026
Merged

steipete merged 1 commit into
mainfrom
fix/appcast-minimum-system-version-element

Conversation

@steipete

@steipete steipete commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

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.mjs now emits <sparkle:minimumSystemVersion> as an item child before <enclosure>. validateAppcast checks 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 of validateAppcast keeps its strict semantics.
  • appcast.xml: every enclosure-attribute entry (45, all 15.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.md documents the element requirement, and CHANGELOG.md gets 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.mjs
  • bash scripts/test-release-binary-reuse.sh
  • node --test tests/release-preflight-contract.test.mjs (28/28)
  • node scripts/docs-lint.mjs
  • xmllint --noout appcast.xml
  • Feed stripping comparison against main: byte-identical outside the moved minimum-version fields.
  • Codex autoreview (P0–P2): scoped-clean.

@clawsweeper

clawsweeper Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@cursor

cursor Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches the macOS auto-update feed and release validation; incorrect placement would silently break minimum-OS gating on the next version bump, though behavior is unchanged today and tests enforce the new shape.

Overview
Sparkle minimum macOS version is now emitted and validated as an <item> child <sparkle:minimumSystemVersion> instead of an ignored <enclosure> attribute, so future minimum-OS bumps actually gate updates.

update-appcast-entry.mjs writes the element before <enclosure> and validateAppcast requires it (attribute-only or missing values fail). The checked-in appcast.xml is migrated for all affected entries (still 15.0). Release docs and tests are updated to match, including assertions that the feed has no enclosure-attribute form.

Reviewed by Cursor Bugbot for commit d013b59. Bugbot is set up for automated code reviews on this repo. Configure here.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Oct 7, 2026
@clawsweeper

clawsweeper Bot commented Oct 7, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed October 7, 2026, 6:45 PM ET / 22:45 UTC.

ClawSweeper review

What this changes

The 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
Reviewed head: d013b59c010caa6ac0b7ade3653b81d7b7336c1a

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused contract repair with appropriate regression coverage and verified preservation of unrelated feed metadata.
Proof confidence 🌊 off-meta tidepool Not applicable: The COLLABORATOR author is exempt from ordinary contributor runtime proof. Source inspection confirms the generator-to-Sparkle contract and feed preservation; reported tests are supplemental, and no user-stored data contract changes.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The COLLABORATOR author is exempt from ordinary contributor runtime proof. Source inspection confirms the generator-to-Sparkle contract and feed preservation; reported tests are supplemental, and no user-stored data contract changes.
Evidence reviewed 8 items Pinned introduced change: The verified main-to-head delta changes the generator’s minimum-version placement and the validator’s lookup together. XML escaping, download URLs, build numbers, archive lengths, and signature handling remain intact.
Affirmative Sparkle dependency boundary: The Mac package pins sparkle-project/Sparkle 2.10.0 at eef1a539a373c1f1a320624b1130fc5de7b2e100. The app’s updater uses Sparkle, and its Info.plist points to this repository’s main-branch appcast.
Pinned consumer contract: At the pinned dependency revision, SUAppcastItem reads minimumSystemVersion from the item dictionary at line 644; enclosure metadata is read from a separate dictionary. This independently supports the PR’s placement correction.
Findings None None.
Security None None.

How this fits together

Peekaboo’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]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Feed compatibility fields 45 entries corrected; 47 total; 2 already-correct entries preserved The migration changes only minimum-version placement and preserves all other feed bytes.

Technical review

Best 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.

Labels

Label changes:

  • add P2: This bounded release-tooling repair prevents incorrect update eligibility after a future minimum-macOS increase; current requirements remain 15.0.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The COLLABORATOR author is exempt from ordinary contributor runtime proof. Source inspection confirms the generator-to-Sparkle contract and feed preservation; reported tests are supplemental, and no user-stored data contract changes.

Label justifications:

  • P2: This bounded release-tooling repair prevents incorrect update eligibility after a future minimum-macOS increase; current requirements remain 15.0.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The COLLABORATOR author is exempt from ordinary contributor runtime proof. Source inspection confirms the generator-to-Sparkle contract and feed preservation; reported tests are supplemental, and no user-stored data contract changes.

Evidence

What I checked:

  • Pinned introduced change: The verified main-to-head delta changes the generator’s minimum-version placement and the validator’s lookup together. XML escaping, download URLs, build numbers, archive lengths, and signature handling remain intact. (scripts/update-appcast-entry.mjs:87, d013b59c010c)
  • Affirmative Sparkle dependency boundary: The Mac package pins sparkle-project/Sparkle 2.10.0 at eef1a539a373c1f1a320624b1130fc5de7b2e100. The app’s updater uses Sparkle, and its Info.plist points to this repository’s main-branch appcast. (Apps/Mac/Package.resolved:25, d013b59c010c)
  • Pinned consumer contract: At the pinned dependency revision, SUAppcastItem reads minimumSystemVersion from the item dictionary at line 644; enclosure metadata is read from a separate dictionary. This independently supports the PR’s placement correction. (Sparkle/SUAppcastItem.m:644, eef1a539a373)
  • Complete feed migration comparison: Read-only parsing of the pinned before/after blobs found 47 items in both feeds, 45 old enclosure attributes, 47 resulting minimum-version elements containing 15.0, and zero resulting minimum-version attributes. Removing only the moved fields makes the feeds byte-identical; the two already-correct items remain byte-identical. (appcast.xml:13, d013b59c010c)
  • Regression coverage and release integration: The inspected tests assert element placement, reject missing/wrong/attribute-only minimum versions, and query the generated XML with a namespace-aware consumer. The existing release driver calls the same validateAppcast function. The PR body reports passing release-contract checks and XML validation; this review did not execute tests or target code. (scripts/test-update-appcast-entry.mjs:71, d013b59c010c)
  • Current-main and release necessity: The pinned main blob and the v4.9.0 generator still emit and validate sparkle:minimumSystemVersion as an enclosure attribute. The requested correction is therefore absent from both inspected versions. (scripts/update-appcast-entry.mjs:87, 7020aedd3620)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • rudycelekli: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete merged commit c02c269 into main Oct 8, 2026
15 checks passed
@steipete
steipete deleted the fix/appcast-minimum-system-version-element branch October 8, 2026 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant