chore: Publish to winget-pkgs on release - #117
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThe CI pipeline gets extended with WinGet publication support. A new reusable workflow handles publishing desktop releases to WinGet, triggered conditionally by the CI system when WinGet-related files change. The main CI orchestrates detection and job execution using a changes-detection action. Changes
Sequence DiagramsequenceDiagram
participant CI as CI Workflow
participant CD as Changes Detection
participant WG as WinGet Job
participant LRI as Latest Release Info
participant Komac as michidk/run-komac
CI->>CD: Detect changes (winget-reusable.yml, latest-release-info)
CD-->>CI: Output winget flag (true/false)
CI->>WG: Trigger job (if winget=true)
WG->>LRI: Fetch latest release information
LRI-->>WG: Return TAG_NAME, URL, VERSION
WG->>Komac: Sync fork & update hrzlgnm.mdns-tui-browser
Komac-->>WG: Publish complete
WG->>Komac: Cleanup merged branches
Komac-->>WG: Cleanup done
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/ci.yml:
- Around line 122-127: The winget job currently runs on any PR that touches
winget files and can publish using secrets.WINGET_TOKEN; fix by adding the same
release-tag guard used by aur (e.g., require github.event.release.tag_name in
the job-level if for the winget job that uses
.github/workflows/winget-reusable.yml) so the job only runs on real releases,
and additionally add defensive gating inside winget-reusable.yml (mirror
aur-reusable.yml’s conditional checks around publish/update steps) to ensure
publishing steps never execute during PRs.
In @.github/workflows/winget-reusable.yml:
- Around line 27-35: The run block is vulnerable because it interpolates
expressions directly into the shell; change it to read TAG_NAME and VERSION from
environment variables instead of embedding `${{
steps.release-info.outputs.tagName }}` and `${{
steps.release-info.outputs.version }}` in the script: set env: TAG_NAME: ${{
steps.release-info.outputs.tagName }} and VERSION: ${{
steps.release-info.outputs.version }} on the step, then inside the run use the
shell-safe variables TAG_NAME and VERSION (referencing the existing URL
construction and echo lines that currently use TAG_NAME, VERSION and
steps.release-info.outputs) so the shell never parses raw workflow expressions.
🧹 Nitpick comments (2)
.github/workflows/winget-reusable.yml (2)
47-54:cleanupjob runs in parallel withpublish— is that intentional?The
cleanupjob has noneeds: publish, so both jobs run concurrently. Ifpublishcreates a PR via komac andcleanupis busy deleting merged branches at the same time, you probably won't hit a race condition because cleanup targets already-merged branches. But the ordering is worth thinking about for two seconds. If you're confident these are independent — fine, ship it. If cleanup should logically happen after publish, add the dependency.♻️ If cleanup should follow publish
cleanup: name: 🧹 branches + needs: publish runs-on: ubuntu-latest
37-45: Use environment variable for token instead of command-line argument.The
michidk/run-komacaction supports passing the token viaKOMAC_TOKENenvironment variable, which is the documented pattern. Passing--token=${{ secrets.WINGET_TOKEN }}via args means anyone with process access can see it in/proc/<pid>/cmdline. On an ephemeral GitHub Actions runner it's probably fine, but there's no reason not to do it the right way when it's literally supported:- uses: michidk/run-komac@9b27eadc6e9235c252444a437d246c139da2f57f # v2.1.0 env: KOMAC_TOKEN: ${{ secrets.WINGET_TOKEN }} with: args: "sync-fork"Do this for both steps (
sync-forkandupdate).
Signed-off-by: hrzlgnm <hrzlgnm@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: hrzlgnm <hrzlgnm@users.noreply.github.com>
Summary by CodeRabbit