ci(packaging): make the dormant Homebrew cask job visible, and add the AppStream metadata Flathub needs - #340
ci(packaging): make the dormant Homebrew cask job visible, and add the AppStream metadata Flathub needs#340EtienneLescot wants to merge 2 commits into
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
📝 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 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
| if [[ -n "$TAP_OWNER" && -n "$TAP_REPO" && "$HAS_TOKEN" == "true" ]]; then | ||
| echo "configured=true" >> "$GITHUB_OUTPUT" | ||
| echo "Tap: ${TAP_OWNER}/${TAP_REPO}, cask ${CASK_NAME}." | ||
| exit 0 | ||
| fi | ||
| echo "configured=false" >> "$GITHUB_OUTPUT" | ||
| echo "::warning title=Homebrew cask not updated::No cask was published. Needs (1) the repository variable HOMEBREW_TAP_OWNER, currently ${TAP_OWNER:-UNSET}; (2) HOMEBREW_TAP_REPO, currently ${TAP_REPO:-UNSET}; (3) the secret HOMEBREW_TAP_TOKEN, currently $([[ "$HAS_TOKEN" == "true" ]] && echo set || echo UNSET), with contents write on that repository; and (4) the tap repository itself, which must be named homebrew-<something> for Homebrew to recognise it. See https://github.com/getopenscreen/openscreen/issues/335" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- workflow files ---'
git ls-files '.github/workflows/update-homebrew-cask.yml' 'technical-documentation/engineering/release-and-secrets.md'
printf '%s\n' '--- workflow size and relevant lines ---'
wc -l .github/workflows/update-homebrew-cask.yml
cat -n .github/workflows/update-homebrew-cask.yml | sed -n '1,120p'
printf '%s\n' '--- documentation references ---'
rg -n -C 5 'HOMEBREW_TAP|homebrew-|brew install|cask' technical-documentation/engineering/release-and-secrets.md
printf '%s\n' '--- related workflow/configuration references ---'
rg -n -C 3 'TAP_REPO|TAP_OWNER|configured|update-homebrew-cask|homebrew-' .github technical-documentation --glob '*.yml' --glob '*.yaml' --glob '*.md' || trueRepository: getopenscreen/openscreen
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- downstream checkout and push path ---'
cat -n .github/workflows/update-homebrew-cask.yml | sed -n '165,247p'
printf '%s\n' '--- predicate behavior ---'
bash <<'BASH'
set -euo pipefail
check_current() {
local TAP_OWNER=$1 TAP_REPO=$2 HAS_TOKEN=true
[[ -n "$TAP_OWNER" && -n "$TAP_REPO" && "$HAS_TOKEN" == "true" ]]
}
check_proposed() {
local TAP_OWNER=$1 TAP_REPO=$2 HAS_TOKEN=true
[[ -n "$TAP_OWNER" && "$TAP_REPO" == homebrew-* && "$TAP_REPO" != "homebrew-" && "$HAS_TOKEN" == "true" ]]
}
for repo in '' 'openscreen-tap' 'homebrew-' 'homebrew-openscreen' 'homebrew-foo.bar' 'Homebrew-openscreen'; do
current=false
proposed=false
check_current getopenscreen "$repo" && current=true || :
check_proposed getopenscreen "$repo" && proposed=true || :
printf 'repo=%q current=%s proposed=%s\n' "$repo" "$current" "$proposed"
done
BASHRepository: getopenscreen/openscreen
Length of output: 4291
🌐 Web query:
Official Homebrew documentation for tap repository naming: confirm whether the short brew tap USER/REPOform requires a GitHub repository namedhomebrew-REPO, and whether brew install --cask USER/REPO/CASK depends on that mapping.
💡 Result:
The short-form brew tap USER/REPO command is a shortcut specifically designed for GitHub-hosted repositories that follow the homebrew-REPO naming convention [1][2][3]. When you run brew tap USER/REPO, Homebrew automatically expands this to https://github.com/USER/homebrew-REPO [4][2][3]. While this prefix is required to use the shortcut [1][4], you are not restricted to this naming convention if you use the two-argument form (brew tap USER/REPO URL), which allows you to tap any repository from any source by providing the full URL explicitly [1][2][3]. Regarding brew install --cask USER/REPO/CASK, this command does not strictly depend on the homebrew- prefix for the tap repository name itself; rather, it depends on the presence of the tap identified as USER/REPO [5][6][7]. If you have tapped a repository using a custom name (e.g., brew tap USER/MY-TOOLS http://example.com/repo.git), you can still install a cask from it using the USER/MY-TOOLS/CASK syntax, provided the tap is installed and contains the requested cask in a Casks/ directory [5][6]. The homebrew- prefix is purely a convention to enable the brew tap shortcut functionality on GitHub [1][8][5].
Citations:
- 1: https://docs.brew.sh/Taps
- 2: https://github.com/Homebrew/brew/blob/master/docs/Manpage.md
- 3: https://github.com/Homebrew/brew/blob/HEAD/Library/Homebrew/cmd/tap.rb
- 4: https://github.com/Homebrew/brew/blob/master/docs/Taps.md
- 5: https://docs.brew.sh/How-to-Create-and-Maintain-a-Tap
- 6: https://github.com/Homebrew/homebrew-cask/blob/main/USAGE.md
- 7: https://docs.brew.sh/Tap-Trust
- 8: https://github.com/Homebrew/brew/blob/main/docs/How-to-Create-and-Maintain-a-Tap.md
Validate the tap name before setting configured=true.
The check accepts any non-empty HOMEBREW_TAP_REPO, so openscreen-tap can be updated even though brew install --cask getopenscreen/openscreen/openscreen resolves getopenscreen/homebrew-openscreen. Enforce the documented homebrew-* naming rule, or document and test an explicit tap URL for custom names.
Proposed fix
- if [[ -n "$TAP_OWNER" && -n "$TAP_REPO" && "$HAS_TOKEN" == "true" ]]; then
+ if [[ -n "$TAP_OWNER" && "$TAP_REPO" == homebrew-* && "$TAP_REPO" != "homebrew-" && "$HAS_TOKEN" == "true" ]]; then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ -n "$TAP_OWNER" && -n "$TAP_REPO" && "$HAS_TOKEN" == "true" ]]; then | |
| echo "configured=true" >> "$GITHUB_OUTPUT" | |
| echo "Tap: ${TAP_OWNER}/${TAP_REPO}, cask ${CASK_NAME}." | |
| exit 0 | |
| fi | |
| echo "configured=false" >> "$GITHUB_OUTPUT" | |
| echo "::warning title=Homebrew cask not updated::No cask was published. Needs (1) the repository variable HOMEBREW_TAP_OWNER, currently ${TAP_OWNER:-UNSET}; (2) HOMEBREW_TAP_REPO, currently ${TAP_REPO:-UNSET}; (3) the secret HOMEBREW_TAP_TOKEN, currently $([[ "$HAS_TOKEN" == "true" ]] && echo set || echo UNSET), with contents write on that repository; and (4) the tap repository itself, which must be named homebrew-<something> for Homebrew to recognise it. See https://github.com/getopenscreen/openscreen/issues/335" | |
| if [[ -n "$TAP_OWNER" && "$TAP_REPO" == homebrew-* && "$TAP_REPO" != "homebrew-" && "$HAS_TOKEN" == "true" ]]; then | |
| echo "configured=true" >> "$GITHUB_OUTPUT" | |
| echo "Tap: ${TAP_OWNER}/${TAP_REPO}, cask ${CASK_NAME}." | |
| exit 0 | |
| fi | |
| echo "configured=false" >> "$GITHUB_OUTPUT" | |
| echo "::warning title=Homebrew cask not updated::No cask was published. Needs (1) the repository variable HOMEBREW_TAP_OWNER, currently ${TAP_OWNER:-UNSET}; (2) HOMEBREW_TAP_REPO, currently ${TAP_REPO:-UNSET}; (3) the secret HOMEBREW_TAP_TOKEN, currently $([[ "$HAS_TOKEN" == "true" ]] && echo set || echo UNSET), with contents write on that repository; and (4) the tap repository itself, which must be named homebrew-<something> for Homebrew to recognise it. See https://github.com/getopenscreen/openscreen/issues/335" |
🤖 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 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.
| if [[ ! "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then | ||
| echo "::error::Refusing tag '$TAG' — expected a stable vMAJOR.MINOR.PATCH tag" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- workflow context ---'
cat -n .github/workflows/update-homebrew-cask.yml | sed -n '55,90p'
printf '%s\n' '--- regex behavior ---'
python3 - <<'PY'
import re
current = re.compile(r'^v[0-9]+\.[0-9]+\.[0-9]+$')
proposed = re.compile(r'^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)$')
tags = [
"v0.0.0",
"v1.2.3",
"v01.02.03",
"v1.02.3",
"v1.2.03",
"v00.1.2",
"v1.2.3-beta",
"1.2.3",
"v1.2",
]
for tag in tags:
print(f"{tag}: current={bool(current.fullmatch(tag))}, proposed={bool(proposed.fullmatch(tag))}")
PYRepository: getopenscreen/openscreen
Length of output: 2465
Reject leading-zero version components.
The expression accepts tags such as v01.02.03, which violates Semantic Versioning. Reject leading zeroes before deriving VERSION.
🤖 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 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.
| Note what it would publish before turning it on: the two DMGs attached to the release, which are unsigned and un-notarized (`mac.notarize: false`). A cask does not change that — `brew install --cask` runs the same Gatekeeper path as a manual download, so users still need the `xattr -rd com.apple.quarantine` step the README documents. What the tap buys is discovery and `brew upgrade`, not trust. | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Document both DMG signing states.
Line [152] conflates mac.notarize: false with the final release artifact state. The packaging contract in technical-documentation/engineering/build-and-packaging.md states that release CI signs, notarizes, staples, and validates DMGs when Apple credentials are present. Missing credentials produce ad-hoc-signed, unnotarized artifacts. This wording tells users to remove quarantine even for notarized artifacts and misstates the security state.
Proposed fix
-Note what it would publish before turning it on: the two DMGs attached to the release, which are unsigned and un-notarized (`mac.notarize: false`). A cask does not change that — `brew install --cask` runs the same Gatekeeper path as a manual download, so users still need the `xattr -rd com.apple.quarantine` step the README documents. What the tap buys is discovery and `brew upgrade`, not trust.
+Note what it would publish before turning it on: the two DMGs attached to the release. When Apple credentials are present, release CI signs, notarizes, staples, and validates them. When credentials are missing, release CI produces ad-hoc-signed, unnotarized artifacts. A cask does not change this behavior; only the latter case requires the `xattr -rd com.apple.quarantine` step documented in the README. What the tap buys is discovery and `brew upgrade`, not trust.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Note what it would publish before turning it on: the two DMGs attached to the release, which are unsigned and un-notarized (`mac.notarize: false`). A cask does not change that — `brew install --cask` runs the same Gatekeeper path as a manual download, so users still need the `xattr -rd com.apple.quarantine` step the README documents. What the tap buys is discovery and `brew upgrade`, not trust. | |
| Note what it would publish before turning it on: the two DMGs attached to the release. When Apple credentials are present, release CI signs, notarizes, staples, and validates them. When credentials are missing, release CI produces ad-hoc-signed, unnotarized artifacts. A cask does not change this behavior; only the latter case requires the `xattr -rd com.apple.quarantine` step documented in the README. What the tap buys is discovery and `brew upgrade`, not trust. |
🤖 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 `@technical-documentation/engineering/release-and-secrets.md` around lines 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.
1 participant
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