fix(linux): declare xdg-desktop-portal and name its backend; ci(winget): stop skipping in silence - #329
Merged
EtienneLescot merged 4 commits intoAug 10, 2026
Conversation
Split out of #324 as #328, where it had buried a different bug: two Linux users get `A portal frontend implementing org.freedesktop.portal.ScreenCast was not found` and no recording. Nothing declares xdg-desktop-portal — not deb, not rpm, not pacman — although every Linux capture goes through it, X11 included, since the helper has no other path. Same class as the three sonames 1.9.2 chased, hidden by the same accident: desktop metapackages pull the portal, so it is present on every machine anyone tested on, and absent on a minimal install or a hand-assembled session. What makes it worse is that the guard 1.9.2 added is structurally blind to it. verify-linux-package.sh asks ld.so whether every shipped ELF resolves; a portal is a D-Bus service and appears in no DT_NEEDED entry, so no amount of loader interrogation will ever reach it. The check is not weak here, it is looking at the wrong layer — and that is worth writing down, because the next dependency of this shape will be invisible to it too. Declaring it is only half, and the half that cannot stand alone: the frontend routes, and the ScreenCast implementation comes from a backend that depends on the desktop (-gnome, -kde, -hyprland, -wlr, -gtk). Choosing one in a depends list would be wrong on half the machines. The other half is the error text, and it is not a log line. PortalError::message() becomes the IPC error in handlers.ts and ends up in a toast, so the raw zbus sentence reached the user verbatim and read as an application fault to the only person able to fix it. portal_unavailable now names the five backends, the way d3d_linux::diagnose names Mesa. It is also the only half that covers the AppImage, which declares nothing and cannot. Package names NOT verified in a container: no container engine would start on this machine. verify-linux-package.sh is what settles it, by installing into a bare image where a name that does not exist fails the whole transaction — which is exactly how http-parser was caught on pacman, on that job's first run.
#148 has been open since 2026-07-24 on "the workflow is broken". It was not. It was gated on `vars.WINGET_IDENTIFIER != ''`, a repository variable that had never been set. A skipped job is green, so the last eight releases — v1.8.0 through v1.9.2, RCs included — all reported success while publishing nothing, and no run, no summary and no check ever said otherwise. Same family as the dead zsync glob in build.yml: a guard that quietly does nothing guards nothing. So the configuration test leaves the job-level `if` and becomes a step that says what is missing. The job now always starts on a non-prerelease and either publishes, or annotates the run with which of the two pieces it lacks. `secrets` is not a context any `if` can read — not at job level, not at step level; only `env` can. Hence HAS_TOKEN as a boolean-as-string, which exposes the token's presence without ever exposing its value. WINGET_IDENTIFIER is now set to OpenScreen.OpenScreen, the identifier #148 proposed. WINGET_ACC_TOKEN is still absent and is the remaining blocker: minting a personal access token is the repo owner's to do, not something CI can arrange for itself. Until it exists this publishes nothing — the difference is that it now says so. Costs one runner minute per release. Buys "did not publish" being visible.
CodeRabbit caught it on #329: adding `xdg-desktop-portal` shifted every "les N dernières" in electron-builder.json5, and those counts are load-bearing — they are how a future editor knows which entries are electron-builder's default and must not be removed. The rpm one was already stale before this branch, still claiming two project-specific entries where there were five. A count that has to be revised on every append will not be, so the comments now name the packages. Defaults are given as a range against real first and last entries; ours are listed by name. While here, the verification note is now true rather than pending. The dispatched build (run 31391020354) put all three packages through `verify-linux-package.sh`, and each answered `ok install: declared depends resolved` in a bare ubuntu:22.04, fedora:40 and archlinux:latest — so `xdg-desktop-portal` is confirmed to be the right name on the three distros, which the commit adding it could only promise. The doc gains the same precision about what that check does and does not cover: it cannot tell you the portal is missing from the list, since no D-Bus service appears in a DT_NEEDED entry, but it does prove a name you put there exists.
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds ChangesLinux portal support
Winget publishing
Estimated code review effort: 3 (Moderate) | ~20 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 |
![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: 2
🤖 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 `@electron-builder.json5`:
- Around line 206-207: Update the dependency-list comments in
electron-builder.json5 to match the current Debian, Arch, and RPM arrays after
adding xdg-desktop-portal; replace stale positional counts with explicit package
names where possible, or correct all counts and positions to reflect the final
list ordering.
In `@electron/native/pipewire-capture/src/portal.rs`:
- Around line 141-166: Update the ScreenCast operations in
`available_cursor_modes()` and `create_session()`/`select_sources()` so service-
or interface-unavailable `ServiceUnknown`/`NoReply` errors are passed through
`portal_unavailable()`. Preserve `failed(...)` for all unrelated portal or D-Bus
errors, and apply the mapping at each relevant call rather than only in
`Screencast::new()`.
🪄 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: 2a850b63-de45-456b-b1a5-ec8255e03af4
📒 Files selected for processing (4)
.github/workflows/publish-winget.ymlelectron-builder.json5electron/native/pipewire-capture/src/portal.rstechnical-documentation/engineering/build-and-packaging.md
Both are documented upstream and neither is guessable from the workflow.
A fine-grained PAT does not work — winget-releaser#172. The token has to be a
classic one with `public_repo`. Someone reading "a PAT with public_repo scope"
in 2027 will mint the kind GitHub now offers by default, and lose a release
finding out.
And the action refuses to run at all until one version of the package already
exists in winget-pkgs: it writes the next manifest by copying the previous one,
so it cannot author the first. Its own pre-flight says so ("Please add atleast
one version of the package before using this action"), which is loud but arrives
only after the token exists and the release has been cut. The first submission is
manual, `wingetcreate new`, and the automation takes over from the second.
Also names the fork requirement, since `fork-user` defaults to the repository
owner and therefore expects getopenscreen/winget-pkgs to exist.
EtienneLescot
added a commit
that referenced
this pull request
Aug 10, 2026
#329 moves the configuration test out of the job-level if, so "gates on vars.WINGET_IDENTIFIER" stops being true the moment both land. Describe what has to be true instead of the mechanism that checks it, which also lets the paragraph name the two prerequisites nobody could guess: the PAT must be classic, and the action refuses to author a package that does not already exist in winget-pkgs.
EtienneLescot
added a commit
that referenced
this pull request
Aug 10, 2026
#329 moves the configuration test out of the job-level if, so "gates on vars.WINGET_IDENTIFIER" stops being true the moment both land. Describe what has to be true instead of the mechanism that checks it, which also lets the paragraph name the two prerequisites nobody could guess: the PAT must be classic, and the action refuses to author a package that does not already exist in winget-pkgs.
EtienneLescot
added a commit
that referenced
this pull request
Aug 10, 2026
CodeRabbit, correctly, on my own paragraph. After #329 the job is not off: it starts on every stable release and either publishes or annotates the run. What is unfinished is the publishing, not the workflow, and describing it as "never run" would be false the moment both land. Keeps the history, which is the part worth remembering: the silent skip is why #148 sat open through eight green releases.
EtienneLescot
added a commit
that referenced
this pull request
Aug 10, 2026
CodeRabbit caught it on #329: adding `xdg-desktop-portal` shifted every "les N dernières" in electron-builder.json5, and those counts are load-bearing — they are how a future editor knows which entries are electron-builder's default and must not be removed. The rpm one was already stale before this branch, still claiming two project-specific entries where there were five. A count that has to be revised on every append will not be, so the comments now name the packages. Defaults are given as a range against real first and last entries; ours are listed by name. While here, the verification note is now true rather than pending. The dispatched build (run 31391020354) put all three packages through `verify-linux-package.sh`, and each answered `ok install: declared depends resolved` in a bare ubuntu:22.04, fedora:40 and archlinux:latest — so `xdg-desktop-portal` is confirmed to be the right name on the three distros, which the commit adding it could only promise. The doc gains the same precision about what that check does and does not cover: it cannot tell you the portal is missing from the list, since no D-Bus service appears in a DT_NEEDED entry, but it does prove a name you put there exists.
EtienneLescot
added a commit
that referenced
this pull request
Aug 10, 2026
#329 moves the configuration test out of the job-level if, so "gates on vars.WINGET_IDENTIFIER" stops being true the moment both land. Describe what has to be true instead of the mechanism that checks it, which also lets the paragraph name the two prerequisites nobody could guess: the PAT must be classic, and the action refuses to author a package that does not already exist in winget-pkgs.
EtienneLescot
added a commit
that referenced
this pull request
Aug 10, 2026
CodeRabbit, correctly, on my own paragraph. After #329 the job is not off: it starts on every stable release and either publishes or annotates the run. What is unfinished is the publishing, not the workflow, and describing it as "never run" would be false the moment both land. Keeps the history, which is the part worth remembering: the silent skip is why #148 sat open through eight green releases.
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 independent packaging fixes found while triaging the Windows/Linux issue backlog against what 1.9.1 and 1.9.2 actually shipped.
fix(linux)— the portal (#328, split out of #324)Nothing declares
xdg-desktop-portal, on any of the three formats, although every Linux capture goes through it — X11 included, the helper has no other path. Two users hit it in #324 and got the raw zbus sentence in a toast.Same class as the three sonames 1.9.2 chased, and hidden by the same accident: desktop metapackages pull the portal, so it is present on every machine anyone tested on. What is new is that
verify-linux-package.shcannot ever see this one — it asksld.so, and a portal is a D-Bus service that appears in noDT_NEEDEDentry. The check is not weak here, it is at the wrong layer.Both halves are needed:
dependsgainsxdg-desktop-portalon deb/rpm/pacman, which makes the constraint explicit rather than inherited by accident;portal_unavailablenames the desktop-specific backend to install (-gnome,-kde,-hyprland,-wlr,-gtk), because the frontend only routes and nodependslist can pick a backend without being wrong on half the machines. This is also the only half that reaches the AppImage, which declares nothing.Not verified in a container — no engine would start on the machine this was written on.
verify-linux-package.shin CI is what settles the package names, by installing into a bare image where a bad name fails the whole transaction. That is howhttp-parserwas caught on pacman on the job's first run, so the negative case is known to work.ci(winget)— the silent skip (#148)The workflow was never broken. It was gated on
vars.WINGET_IDENTIFIER != '', a variable that had never been set, so the job resolved toskippedon the last eight releases — all green, none publishing anything. Same family as the dead zsync glob: a guard that quietly does nothing guards nothing.The configuration test moves out of the job-level
ifinto a step that annotates the run with which piece is missing.WINGET_IDENTIFIERis now set toOpenScreen.OpenScreen;WINGET_ACC_TOKENis still absent and remains the owner's to mint.Also done outside this branch
status: pending releaselabel removed from all 26 closed issues carrying it.Refs #148, #324, #328.
Summary by CodeRabbit
Bug Fixes
Packaging
xdg-desktop-portalas a Linux runtime requirement for screen capture.Documentation