ci(packaging): make the dormant Homebrew cask job visible, and add the AppStream metadata Flathub needs - #340
Merged
Conversation
The tap configuration lived in the job-level `if:`, so an unconfigured job resolved to `skipped` — which is green. Neither HOMEBREW_TAP_OWNER nor HOMEBREW_TAP_REPO has ever existed on this repository, so the cask has never been published once, on any release, with every run reporting success. Same failure as #148 for winget, and fixed the same way: the check moves into a step that names each missing piece in a warning. Manual replay also gains the tag guard aur-publish.yml already has. workflow_dispatch takes free text and the prerelease filter only covers the release event, so replaying v1.9.4-rc.2 would have published an RC as the stable cask. Refs #335
Flathub will not take a MetaInfo file written by the packager, so the file has to exist upstream before any manifest can be submitted. It is also missing where it would already help: the shipped 1.9.2 deb installs a .desktop file and nine icon sizes and no /usr/share/metainfo/ at all, so GNOME Software and Discover have nothing to show but an icon. Component ID is com.getopenscreen.OpenScreen, not the Electron appId com.etiennelescot.openscreen: Flathub requires the ID to map to a domain the project controls, getopenscreen.com is that domain, and the Electron appId decides the userData path of every existing install. Nothing consumes the file yet, so CI validating it is the only thing between an edit here and a rejected submission months later. Caught its own first bug that way, in a container: an XML comment may not contain a double hyphen. Refs #335
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe change adds OpenScreen AppStream metadata and offline CI validation. It also updates the Homebrew cask workflow to validate configuration and stable tags before publishing, and documents the related CI and release requirements. ChangesAppStream metadata validation
Homebrew release workflow
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
13 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.github/workflows/ci.yml (1)
69-76: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRestrict the token for this validation-only job.
This job only reads repository content and runs
appstreamcli. Setpermissions: contents: read. Setpersist-credentials: falseonactions/checkoutso later steps cannot use a persisted token.Proposed change
appstream: name: AppStream metadata runs-on: ubuntu-latest + permissions: + contents: read steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@v7 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 69 - 76, Restrict the appstream job by adding job-level permissions with contents read, and configure its actions/checkout step with persist-credentials set to false. Keep the existing checkout and appstream installation behavior unchanged.Source: Linters/SAST tools
.github/workflows/update-homebrew-cask.yml (1)
172-175: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin
actions/checkoutto an immutable commit.Line [175] passes
HOMEBREW_TAP_TOKENto the action. Replace the mutable@v7reference at Line [172] with the reviewed full commit SHA. GitHub recommends full-length SHA pinning for immutable action execution. (docs.github.com)Proposed fix
- uses: actions/checkout@v7 + uses: actions/checkout@<full-40-character-commit-sha> # v7.0.0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/update-homebrew-cask.yml around lines 172 - 175, Update the actions/checkout reference in the checkout step to use the reviewed full commit SHA instead of the mutable `@v7` tag, while preserving the existing repository and HOMEBREW_TAP_TOKEN inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/update-homebrew-cask.yml:
- Around line 73-76: Update the TAG validation condition in the workflow to
require each major, minor, and patch component to be either 0 or a nonzero digit
followed by digits, rejecting values such as v01.02.03 before deriving VERSION.
Preserve acceptance of stable vMAJOR.MINOR.PATCH tags without leading zeroes.
- Around line 44-50: Update the configuration check before setting
configured=true to require TAP_REPO to follow the documented homebrew-* naming
convention, while preserving the existing owner and token validations. Ensure
nonconforming repository names remain configured=false and use the existing
warning path.
In `@technical-documentation/engineering/release-and-secrets.md`:
- Around line 152-153: Update the release artifact guidance in the documented
cask-publishing note to distinguish credentialed CI outputs (signed, notarized,
stapled, and validated DMGs) from outputs produced without Apple credentials
(ad-hoc-signed and unnotarized). Avoid treating mac.notarize: false as the final
artifact state, and only prescribe the xattr quarantine-removal step for
unnotarized artifacts while preserving the existing discovery and brew upgrade
context.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 69-76: Restrict the appstream job by adding job-level permissions
with contents read, and configure its actions/checkout step with
persist-credentials set to false. Keep the existing checkout and appstream
installation behavior unchanged.
In @.github/workflows/update-homebrew-cask.yml:
- Around line 172-175: Update the actions/checkout reference in the checkout
step to use the reviewed full commit SHA instead of the mutable `@v7` tag, while
preserving the existing repository and HOMEBREW_TAP_TOKEN inputs.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 00a7bea4-0d4b-41a3-8afb-abe48e2de604
📒 Files selected for processing (5)
.github/workflows/ci.yml.github/workflows/update-homebrew-cask.ymlbuild/com.getopenscreen.OpenScreen.metainfo.xmltechnical-documentation/engineering/ci-workflows.mdtechnical-documentation/engineering/release-and-secrets.md
The configuration step accepted any non-empty HOMEBREW_TAP_REPO, so `openscreen-tap` set configured=true — and the job would then clone, commit and push a cask to it successfully. Green run, cask published, nothing installable: `brew tap getopenscreen/openscreen` expands to `getopenscreen/homebrew-openscreen`, which is a different repository. That is the same shape as the bug this workflow change exists to fix. The step already names the homebrew-<something> rule in its warning, and the release documentation already spells out that a wrongly-named tap "would be checked out and pushed to successfully and still be untappable" — the check just never applied the rule it was describing. Now it does. `homebrew-?*` rather than the prefix alone, so a repository named the bare `homebrew-` is refused too. A name with a capital H is refused as well, which Homebrew would in fact resolve; the cost of that strictness is a warning naming the convention, not a silent mispublish.
The cask note read `mac.notarize: false` out of electron-builder.json5 and reported it as the state of the shipped artifact. That field only says electron-builder does not notarize; build.yml does it afterwards by hand — `Sign DMG`, `Notarize DMG`, `Staple notarization ticket`, `Validate stapled DMG`, all four gated on the Apple credentials being present, on every tag including pre-releases. So the paragraph contradicted its own file: sixty-seven lines above, the Apple signing section already says every tag signs, notarizes, staples and validates, and falls back to an ad-hoc signature only when a value is missing. Both states are now named, and the quarantine step is attached to the one that actually needs it.
8 tasks
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.
Summary
Two of the four Homebrew checkboxes in #335, plus the upstream prerequisite the Flathub half turns out to need first.
1. The Homebrew cask job no longer reports an unpublished cask as a green release.
The tap configuration lived in the job-level
if:, so an unconfigured job resolved toskipped— which is green. NeitherHOMEBREW_TAP_OWNERnorHOMEBREW_TAP_REPOhad ever existed on this repository, so the cask had never been published once, on any release, with every run reporting success. Exactly #148's failure, fixed the same waypublish-winget.ymlwas: the check moves into a step that names each missing piece in a::warning::, and the remaining steps gate on its output the wayaur-publish.ymlalready does.Manual replay also gains the tag guard
aur-publish.ymlhas.workflow_dispatchtakes free text and theprereleasefilter only covers thereleaseevent, so replayingv1.9.4-rc.2would have published an RC as the cask and handed it to everyone on stable viabrew upgrade. Homebrew accepts that version string happily; only this check refuses it.2. Upstream AppStream metadata (
build/com.getopenscreen.OpenScreen.metainfo.xml), validated in CI.Flathub will not accept a MetaInfo file written by the packager — it has to exist upstream — so no manifest can be submitted until this file lands. It is also missing where it would already help: the shipped 1.9.2 deb installs a
.desktopfile and nine icon sizes and no/usr/share/metainfo/at all, so GNOME Software and Discover have nothing to show but an icon.The component ID is
com.getopenscreen.OpenScreen, deliberately not the ElectronappIdcom.etiennelescot.openscreen. Flathub requires the ID to map to a domain the project controls;getopenscreen.comis that domain,etiennelescot.comis not one anyone here owns, and the ElectronappIddecides the userData path of every existing install.Nothing consumes the file yet, so the new
appstreamCI job is the only thing standing between a broken edit and a submission rejected for it months later.Done outside the diff
getopenscreen/homebrew-openscreenand seeded it with a README.HOMEBREW_TAP_OWNER=getopenscreenandHOMEBREW_TAP_REPO=homebrew-openscreen.Still open, and deliberately not in this PR
HOMEBREW_TAP_TOKENhas to be created by a human — contents write on the tap. Until it exists the job runs and warns, which is the point of the change. Then dispatch the workflow onv1.9.2and verifybrew install --cask getopenscreen/openscreen/openscreen..debis out. For this app that means compiling the Rust compositor, the C PipeWire helper, ffmpeg with its renamed symbols and the whisper.cpp/Vulkan STT stack offline inside the sandbox. That is its own project, filed separately rather than guessed at here.Related issue
Refs #335
Type of change
Release impact
Desktop impact
Screenshots / video
Not applicable — no UI change.
Testing
No native helper code is touched, so no manual smoke test applies. Everything asserted below was run, not assumed.
ubuntu:24.04container (appstreamcli1.0.2), which is what the new CI job runs. It failed the first attempt onxml-markup-invalid— an XML comment may not contain a double hyphen, and mine named theappstreamcliflags — and passes after the fix. XML well-formedness re-checked locally afterwards.js-yaml, and the resulting step gating was read back: one always-run reporting step inupdate-homebrew-cask.yml, seven gated onsteps.config.outputs.configured.configuredoutput and warning text in each.v1.9.2,v10.20.30(accept) andv1.9.4-rc.2,1.9.2,v1.9, an empty tag and a quote-injection attempt (refuse).Openscreen-Linux-1.9.2.debwas downloaded and unpacked, confirming/usr/share/applications/openscreen.desktopwithStartupWMClass=Openscreen, nine icon sizes, and no/usr/share/metainfo/. That is where the metainfo's<launchable>value comes from.node scripts/check-docs.mjs→ OK (22 files).The one thing not verified here is the
appstreamjob on a real runner; it runs on this PR.Summary by CodeRabbit
New Features
Documentation
Chores