Conversation
OpenWrt 24.10+ defaults to APK as the package manager on many targets. Users on these builds get "v2 package format error" when installing .ipk files with `apk add`. This builds both formats per architecture so both opkg and apk users are supported.
Allows triggering the full build matrix on PRs by adding a "run-build" label. Builds also re-trigger on push if the label is present. PR builds upload artifacts instead of attaching to a release.
When the APK build is not found, the package_name output is empty, causing the upload path to resolve to /tmp/ which fails. Split uploads into conditional steps gated on find_apk.outputs.found.
Extract a prepare job that builds dist/ once and shares it via artifact instead of running Node.js setup + pnpm install + build in all 8 matrix jobs. Cache the OpenWrt SDKs (pinned to 24.10.5) so subsequent runs skip the ~200MB download per architecture. Version is computed once and passed as a job output.
CONFIG_USE_APK is marked BROKEN in the 24.10 SDK and the apk host binary is absent, so it cannot produce apk packages. Split into two parallel job matrices: 24.10.5 SDK produces ipk for opkg devices, 25.12.0 SDK (which ships apk as default) produces apk for devices running apk.
CONFIG_USE_APK is BROKEN in the 24.10 SDK. Use separate SDK versions: 24.10.5 for ipk (opkg devices), 25.12.0 for apk (apk devices). Both matrices run in parallel after the shared prepare job.
Cache SDKs and deduplicate build assets across matrix
…k-package-format # Conflicts: # .github/workflows/build-release.yml
- Replace unstyled btn-icon/btn-edit/btn-delete classes with existing action-btn-sm theme - Add action-btn-sm.danger hover style for delete buttons - Start login-view hidden to prevent flash while async auth check runs
APK mkpkg rejects versions with hyphens in the base (0.0.0-pr8). Use dot-separated format (0.0.0.8) which is valid for both ipk and apk.
UI: Fix table action buttons and login flash
Add OpenWrt 25.12+ apk install commands alongside existing opkg instructions. Update architecture list to use actual package suffixes.
Build both ipk and apk packages for OpenWrt 24.10+
📝 WalkthroughWalkthroughRestructures the GitHub Actions pipeline into a prepare job that builds assets and determines package version, plus dependent build-ipk and build-apk jobs that reuse cached OpenWrt SDKs per target/arch, build versioned packages, and upload artifacts or create releases; updates README, CSS, index visibility, and action-button rendering. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Prepare as prepare job
participant Dist as dist artifact
participant BuildIPK as build-ipk job
participant BuildAPK as build-apk job
participant SDK as OpenWrt SDK cache
participant Release as GitHub Release / Artifacts
GH->>Prepare: trigger (push/tag or labeled PR)
Prepare->>GH: checkout, setup node/pnpm, install, build minified assets
Prepare->>Prepare: determine version (tag or PR-based)
Prepare->>Dist: upload dist artifact (includes version)
GH->>BuildIPK: start when prepare succeeds
BuildIPK->>Dist: download dist artifact
BuildIPK->>SDK: restore or download/extract SDK (per target/arch)
BuildIPK->>BuildIPK: copy package files, update Makefile version, build IPK
BuildIPK->>Release: upload IPK artifact or attach to Release
GH->>BuildAPK: start when prepare succeeds
BuildAPK->>Dist: download dist artifact
BuildAPK->>SDK: restore or download/extract SDK (per target/arch)
BuildAPK->>BuildAPK: copy package files, update Makefile version, build APK
BuildAPK->>Release: upload APK artifact or attach to Release
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 (1)
.github/workflows/build-release.yml (1)
59-61: Considerfail-fast: falsefor both architecture matrices.GitHub Actions defaults matrix
fail-fasttotrue, so one broken target cancels the remaining builds in that matrix. For a multi-arch release workflow, that usually means fewer salvageable artifacts and less diagnostic signal from the same run. (docs.github.com)Suggested change
strategy: + fail-fast: false matrix: include:Also applies to: 165-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-release.yml around lines 59 - 61, The matrix strategy is currently using the default fail-fast behavior which cancels remaining jobs on the first failure; update the workflow's strategy blocks that define the matrix (the YAML blocks containing "strategy:" and "matrix:" with "include:") to set fail-fast: false so each matrix runs all jobs even if one fails—apply this change to both matrix definitions in the file (the two "strategy: matrix:" sections).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-release.yml:
- Around line 97-103: The SDK cache step ("Cache SDK" with id cache-sdk using
actions/cache@v4 and matrix.sdk_name) currently persists the entire SDK
including bin/packages which can leave old build outputs; modify the workflow to
(1) clean the SDK's prior build outputs (remove or empty bin/packages) before
building so cached stale packages can't be reused, and (2) change the "Find and
rename" logic to avoid head -n 1 by matching the exact expected filename (or
using a deterministic pattern with sort/most-recent selection) when selecting
the package to upload; alternatively restrict the cache path to exclude
bin/packages (or explicitly cache only source artifacts) to prevent packaging
artifacts from being cached.
- Around line 6-7: The workflow grants top-level write permissions to contents
during pull_request runs, exposing a write-scoped token; update the job
definitions for build-ipk and build-apk to add job-level permissions that
restrict token scope during PRs (e.g., set permissions.contents to read by
default and only enable write when github.event_name == 'release' or move write
to a separate release-publish job); modify the build-ipk and build-apk job
blocks to include a permissions stanza that conditionally sets contents to read
or to write only for release events so PR runs receive a read-only token.
In `@moci/index.html`:
- Line 16: The login page is unintentionally unreachable because both
`#login-view` and `#main-view` start with the hidden class; change the initial state
so at least the login UI can be shown immediately (remove the hidden class from
`#login-view` in index.html) or add a dedicated loading element instead of hiding
both screens, and update the session flow in moci/js/core.js (functions
autoLogin(), login(), and showLoginView()) to toggle the new loading state and
only hide/show `#main-view/`#login-view after session validation completes; ensure
fetch/login error paths call showLoginView() so users can manually sign in if
autoLogin fails.
---
Nitpick comments:
In @.github/workflows/build-release.yml:
- Around line 59-61: The matrix strategy is currently using the default
fail-fast behavior which cancels remaining jobs on the first failure; update the
workflow's strategy blocks that define the matrix (the YAML blocks containing
"strategy:" and "matrix:" with "include:") to set fail-fast: false so each
matrix runs all jobs even if one fails—apply this change to both matrix
definitions in the file (the two "strategy: matrix:" sections).
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: df9a1b50-4723-4271-8a3d-96330b3d1ab1
📒 Files selected for processing (6)
.github/workflows/build-release.ymlREADME.mdmoci/app.cssmoci/app.jsmoci/index.htmlmoci/js/core.js
| pull_request: | ||
| types: [labeled, synchronize] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat .github/workflows/build-release.ymlRepository: HudsonGraeme/MoCI
Length of output: 9362
🌐 Web query:
GitHub GITHUB_TOKEN permissions pull request same repository forked PR write access
💡 Result:
By default, the GITHUB_TOKEN in GitHub Actions workflows triggered by a pull_request event from a forked repository has read-only permissions to the base repository. This is a security feature to prevent malicious forks from writing to the target repo using workflows. Write access with GITHUB_TOKEN for forked PRs requires one of these: 1. Use the pull_request_target event trigger instead of pull_request. This grants read/write permissions to GITHUB_TOKEN (configurable via the permissions key) even for forks, and workflows run from the base branch. However, it has security risks: do not checkout the PR head sha or run untrusted code, as it can access secrets. See GitHub security guides for safe usage. 2. For private repositories, enable "Send write tokens to workflows from pull requests" in repo Actions settings (under Fork pull request workflows in private repositories). This allows GITHUB_TOKEN write permissions for pull_request events from forks, but pair with "Require approval for fork pull request workflows" for safety. GITHUB_TOKEN cannot write to the fork repository itself from the base repo workflow; it only has permissions scoped to the base repo. Permissions can be set in the workflow YAML with the 'permissions' key, e.g.: permissions: contents: write pull-requests: write Repo settings also control default GITHUB_TOKEN permissions (read-only or read/write).
Citations:
- 1: https://docs.github.com/actions/writing-workflows/choosing-what-your-workflow-does/controlling-permissions-for-github_token
- 2: https://stackoverflow.com/questions/75744849/permission-to-repo-denied-to-github-actions-for-pr-coming-from-a-fork-only
- 3: https://stackoverflow.com/questions/75655770/github-actions-requiring-secrets-on-a-fork-origin-pr
- 4: https://github.com/orgs/community/discussions/26904
- 5: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository
- 6: Warn about the limitations of GitHub tokens actions/labeler#491
- 7: Clarify why this needs pull_request_target, rather than
pull_requestactions/labeler#121 - 8: https://shreyapohekar.com/blogs/the-right-way-to-handle-permissions-in-github-actions-a-practical-guide-to-staying-secure/
- 9: https://stackoverflow.com/questions/69499645/how-to-securely-allow-github-actions-to-check-pr-and-post-results-in-comment
Restrict token scope for pull request jobs.
The workflow uses pull_request triggers with top-level permissions: contents: write. For same-repository PRs, this write scope is not automatically downgraded (unlike forked PRs). While the current code properly gates release uploads with if: github.event_name == 'release', the write-scoped token remains available in the job environment during PR runs, creating unnecessary exposure.
Add job-level permissions to restrict token scope for PR execution:
Example fix
build-ipk:
permissions:
contents: ${{ github.event_name == 'release' && 'write' || 'read' }}Or simplify by setting read for matrix jobs and keeping write only in a separate release-publishing step.
Also applies to: build-apk job (same concern applies)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-release.yml around lines 6 - 7, The workflow grants
top-level write permissions to contents during pull_request runs, exposing a
write-scoped token; update the job definitions for build-ipk and build-apk to
add job-level permissions that restrict token scope during PRs (e.g., set
permissions.contents to read by default and only enable write when
github.event_name == 'release' or move write to a separate release-publish job);
modify the build-ipk and build-apk job blocks to include a permissions stanza
that conditionally sets contents to read or to write only for release events so
PR runs receive a read-only token.
| - name: Cache SDK | ||
| id: cache-sdk | ||
| uses: actions/cache@v4 | ||
| with: | ||
| version: 9 | ||
|
|
||
| - name: Install Node dependencies | ||
| run: pnpm install --frozen-lockfile | ||
|
|
||
| - name: Build minified assets | ||
| run: pnpm run build | ||
| path: ${{ matrix.sdk_name }} | ||
| key: sdk-${{ matrix.sdk_name }} | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd "$(git rev-parse --show-toplevel)" && find . -name "build-release.yml" -type fRepository: HudsonGraeme/MoCI
Length of output: 98
🏁 Script executed:
cd "$(git rev-parse --show-toplevel)" && cat -n .github/workflows/build-release.yml | head -n 260Repository: HudsonGraeme/MoCI
Length of output: 10976
The SDK cache can publish stale packages.
The cache step persists the entire SDK directory, including bin/packages/. When a cached SDK is reused, old build outputs remain. The "Find and rename" steps use head -n 1 to select the first matching file, which is non-deterministic and can pick an older package if multiple versions exist. This risks uploading a stale package to the release instead of the current build output.
Clean prior package outputs before each build and use exact filename matching:
Suggested hardening
- name: Build ipk package
working-directory: ${{ matrix.sdk_name }}
run: |
+ find bin/packages -name 'moci_*.ipk' -delete
make defconfig
make package/moci/compile V=s
- name: Find and rename ipk
id: find_ipk
working-directory: ${{ matrix.sdk_name }}
run: |
+ VERSION="${{ needs.prepare.outputs.version }}"
- IPK_FILE=$(find bin/packages -name "moci_*.ipk" | head -n 1)
+ IPK_FILE=$(find bin/packages -name "moci_${VERSION}-r1_${{ matrix.arch }}.ipk" -print -quit)
if [ -z "$IPK_FILE" ]; then
echo "Error: ipk package not found!"
exit 1
fi
- VERSION="${{ needs.prepare.outputs.version }}"
NEW_NAME="moci_${VERSION}-r1_${{ matrix.arch }}.ipk"
- name: Build apk package
working-directory: ${{ matrix.sdk_name }}
run: |
+ find bin/packages -name 'moci-*.apk' -delete
make defconfig
make package/moci/compile V=s
- name: Find and rename apk
id: find_apk
working-directory: ${{ matrix.sdk_name }}
run: |
+ VERSION="${{ needs.prepare.outputs.version }}"
- APK_FILE=$(find bin/packages -name "moci*.apk" | head -n 1)
+ APK_FILE=$(find bin/packages -name "moci-${VERSION}-r1.apk" -print -quit)
if [ -z "$APK_FILE" ]; then
echo "Error: apk package not found!"
exit 1
fi
- VERSION="${{ needs.prepare.outputs.version }}"
NEW_NAME="moci_${VERSION}-r1_${{ matrix.arch }}.apk"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-release.yml around lines 97 - 103, The SDK cache
step ("Cache SDK" with id cache-sdk using actions/cache@v4 and matrix.sdk_name)
currently persists the entire SDK including bin/packages which can leave old
build outputs; modify the workflow to (1) clean the SDK's prior build outputs
(remove or empty bin/packages) before building so cached stale packages can't be
reused, and (2) change the "Find and rename" logic to avoid head -n 1 by
matching the exact expected filename (or using a deterministic pattern with
sort/most-recent selection) when selecting the package to upload; alternatively
restrict the cache path to exclude bin/packages (or explicitly cache only source
artifacts) to prevent packaging artifacts from being cached.
| <body> | ||
| <div id="app"> | ||
| <div id="login-view" class="view"> | ||
| <div id="login-view" class="view hidden"> |
There was a problem hiding this comment.
Don't start with both views hidden.
Line 16 makes the login screen invisible until moci/js/core.js:80-99 finishes session validation / auto-login and showLoginView() in moci/js/core.js:331-334 removes hidden. Because #main-view is already hidden and autoLogin() eventually calls login(), which still uses a plain fetch, a stale session or hanging /ubus request now leaves the user on a blank page with no way to log in manually.
🛠 Minimal fix
- <div id="login-view" class="view hidden">
+ <div id="login-view" class="view">If the goal is to avoid a login flash for valid sessions, add a dedicated loading state instead of hiding both screens.
📝 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.
| <div id="login-view" class="view hidden"> | |
| <div id="login-view" class="view"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@moci/index.html` at line 16, The login page is unintentionally unreachable
because both `#login-view` and `#main-view` start with the hidden class; change the
initial state so at least the login UI can be shown immediately (remove the
hidden class from `#login-view` in index.html) or add a dedicated loading element
instead of hiding both screens, and update the session flow in moci/js/core.js
(functions autoLogin(), login(), and showLoginView()) to toggle the new loading
state and only hide/show `#main-view/`#login-view after session validation
completes; ensure fetch/login error paths call showLoginView() so users can
manually sign in if autoLogin fails.
- Restrict top-level permissions to read, grant write only at job level where release uploads need it - Clean bin/packages before building to prevent stale cached artifacts from being picked up by find - Set fail-fast: false on both matrix strategies so all arch builds complete independently
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/build-release.yml (1)
59-60:⚠️ Potential issue | 🟠 MajorRestrict write-scoped
GITHUB_TOKENduring PR builds.Line 59 and Line 169 still grant
contents: writefor the full job, including pull_request runs. Even with release-only upload steps, PR job steps still execute with a write-scoped token. Limit write scope to a dedicated release-publish job (or otherwise ensure build jobs run with read-only permissions).For GitHub Actions pull_request events from the same repository, if a job sets permissions.contents: write, does GITHUB_TOKEN keep write access for all steps in that job even when the release upload step is gated by if: github.event_name == 'release'?Also applies to: 169-170
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-release.yml around lines 59 - 60, The workflow grants a write-scoped GITHUB_TOKEN via the permissions: contents: write setting for jobs that run on PRs; change the default job permissions to contents: read (or remove write) so pull_request builds run with read-only token, and move contents: write to only the dedicated release-publish job (the job that performs release uploads and is gated with if: github.event_name == 'release') so only that job has write scope; update the workflow to set permissions: contents: write inside the release-publish job definition and ensure all other jobs use contents: read.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/build-release.yml:
- Around line 59-60: The workflow grants a write-scoped GITHUB_TOKEN via the
permissions: contents: write setting for jobs that run on PRs; change the
default job permissions to contents: read (or remove write) so pull_request
builds run with read-only token, and move contents: write to only the dedicated
release-publish job (the job that performs release uploads and is gated with if:
github.event_name == 'release') so only that job has write scope; update the
workflow to set permissions: contents: write inside the release-publish job
definition and ensure all other jobs use contents: read.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 30ff874a-16a1-4f5a-8921-97e155437446
📒 Files selected for processing (1)
.github/workflows/build-release.yml
1 participant
Summary by CodeRabbit
New Features
Documentation
Style
Chores