ci: split lint workflow into focused PR checks#9198
Conversation
kodjima33
left a comment
There was a problem hiding this comment.
CI: split lint workflow — approve-only; modifies .github/workflows/lint.yml, needs Nik.
Move shared path detection into a composite action and split backend, desktop, mobile app, and web checks into focused workflows while keeping repo hygiene and formatting in the existing lint workflow path. Validation: /opt/homebrew/bin/actionlint -shellcheck '' .github/workflows/lint.yml .github/workflows/mobile-app-checks.yml .github/workflows/backend-checks.yml .github/workflows/desktop-checks.yml .github/workflows/web-checks.yml; python3 YAML parse/list jobs; git diff --check.
a676f2d to
8b3da41
Compare
There was a problem hiding this comment.
5 issues found across 11 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="desktop/macos/scripts/check-sources-root-layout.py">
<violation number="1" location="desktop/macos/scripts/check-sources-root-layout.py:14">
P2: The CI wiring comment incorrectly claims this script is gated in the `Repo Checks` workflow, but the script is not actually invoked in `.github/workflows/repo-checks.yml` (or any other workflow). This misleads maintainers who need to trace or update CI gates for the desktop Swift source-layout ratchet. Consider either adding the missing step to the correct split workflow and updating the comment, or removing the inaccurate CI wiring line from the docstring.</violation>
</file>
<file name=".github/workflows/mobile-app-checks.yml">
<violation number="1" location=".github/workflows/mobile-app-checks.yml:26">
P2: The `generated-files` and `android-compile-smoke` jobs both fetch the entire repository history with `fetch-depth: 0`, but neither job performs any git operation that needs full history. The generated-file checks only run `git diff --exit-code` against HEAD and `git ls-files --others` to detect stale or untracked files, and the compile-smoke job does not use git at all. Keeping full-depth checkout in these downstream jobs multiplies CI time and network usage on every mobile check without adding correctness. Removing `fetch-depth: 0` (or setting it to `1`) from both jobs would let them use the default shallow checkout.</violation>
</file>
<file name=".github/actions/detect-changes/action.yml">
<violation number="1" location=".github/actions/detect-changes/action.yml:72">
P2: Path-detection gates that use `echo "$FILES" | grep -q …` can incorrectly emit `false` for large changed-file lists because `grep -q` exits immediately on match, which may leave the writer (`echo`) with a broken pipe while `pipefail` is active. On GitHub Actions, `bash` runs with `-o pipefail` by default, so that writer failure becomes the pipeline exit status and the `&& true || false` guard falls through to `false` even when the pattern matches. Switching to a here-string (`grep -q pattern <<< "$FILES"`) removes the pipe and eliminates the race.</violation>
</file>
<file name=".github/workflows/repo-checks.yml">
<violation number="1" location=".github/workflows/repo-checks.yml:72">
P2: The `Check desktop changelog entry` step uses the `no-changelog-needed` PR label to decide whether to skip the changelog check, but the `pull_request` trigger does not include `labeled` or `unlabeled` activity types. When a label is added or removed after the workflow has already run, the check result becomes stale until a new commit is pushed or the workflow is manually re-run. Consider adding `types: [opened, synchronize, reopened, labeled, unlabeled]` to the `pull_request` trigger so label-driven skips are re-evaluated automatically.</violation>
<violation number="2" location=".github/workflows/repo-checks.yml:73">
P2: The `Check desktop changelog entry` step can be bypassed by any PR author (including forks) simply naming their branch `changelog/v...`, because `github.head_ref` is user-controlled and there is no actor or repository guard. Consider tightening the exception so only release-automation contexts can skip the check.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
| Wiring (see also `.github/workflows/lint.yml`): | ||
| - CI: gated step in the Lint workflow when desktop Swift sources change. | ||
| Wiring (see also `.github/workflows/repo-checks.yml`): |
There was a problem hiding this comment.
P2: The CI wiring comment incorrectly claims this script is gated in the Repo Checks workflow, but the script is not actually invoked in .github/workflows/repo-checks.yml (or any other workflow). This misleads maintainers who need to trace or update CI gates for the desktop Swift source-layout ratchet. Consider either adding the missing step to the correct split workflow and updating the comment, or removing the inaccurate CI wiring line from the docstring.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/macos/scripts/check-sources-root-layout.py, line 14:
<comment>The CI wiring comment incorrectly claims this script is gated in the `Repo Checks` workflow, but the script is not actually invoked in `.github/workflows/repo-checks.yml` (or any other workflow). This misleads maintainers who need to trace or update CI gates for the desktop Swift source-layout ratchet. Consider either adding the missing step to the correct split workflow and updating the comment, or removing the inaccurate CI wiring line from the docstring.</comment>
<file context>
@@ -11,8 +11,8 @@
-Wiring (see also `.github/workflows/lint.yml`):
- - CI: gated step in the Lint workflow when desktop Swift sources change.
+Wiring (see also `.github/workflows/repo-checks.yml`):
+ - CI: gated step in the Repo Checks workflow when desktop Swift sources change.
- Manually: python3 desktop/macos/scripts/check-sources-root-layout.py
</file context>
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
P2: The generated-files and android-compile-smoke jobs both fetch the entire repository history with fetch-depth: 0, but neither job performs any git operation that needs full history. The generated-file checks only run git diff --exit-code against HEAD and git ls-files --others to detect stale or untracked files, and the compile-smoke job does not use git at all. Keeping full-depth checkout in these downstream jobs multiplies CI time and network usage on every mobile check without adding correctness. Removing fetch-depth: 0 (or setting it to 1) from both jobs would let them use the default shallow checkout.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/mobile-app-checks.yml, line 26:
<comment>The `generated-files` and `android-compile-smoke` jobs both fetch the entire repository history with `fetch-depth: 0`, but neither job performs any git operation that needs full history. The generated-file checks only run `git diff --exit-code` against HEAD and `git ls-files --others` to detect stale or untracked files, and the compile-smoke job does not use git at all. Keeping full-depth checkout in these downstream jobs multiplies CI time and network usage on every mobile check without adding correctness. Removing `fetch-depth: 0` (or setting it to `1`) from both jobs would let them use the default shallow checkout.</comment>
<file context>
@@ -0,0 +1,137 @@
+ - name: Checkout code
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+
+ - name: Detect changed paths
</file context>
| echo "$FILES" | ||
| echo "diff_base=$DIFF_BASE" >> "$GITHUB_OUTPUT" | ||
|
|
||
| has_dart=$(echo "$FILES" | grep -q '\.dart$' && echo true || echo false) |
There was a problem hiding this comment.
P2: Path-detection gates that use echo "$FILES" | grep -q … can incorrectly emit false for large changed-file lists because grep -q exits immediately on match, which may leave the writer (echo) with a broken pipe while pipefail is active. On GitHub Actions, bash runs with -o pipefail by default, so that writer failure becomes the pipeline exit status and the && true || false guard falls through to false even when the pattern matches. Switching to a here-string (grep -q pattern <<< "$FILES") removes the pipe and eliminates the race.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/actions/detect-changes/action.yml, line 72:
<comment>Path-detection gates that use `echo "$FILES" | grep -q …` can incorrectly emit `false` for large changed-file lists because `grep -q` exits immediately on match, which may leave the writer (`echo`) with a broken pipe while `pipefail` is active. On GitHub Actions, `bash` runs with `-o pipefail` by default, so that writer failure becomes the pipeline exit status and the `&& true || false` guard falls through to `false` even when the pattern matches. Switching to a here-string (`grep -q pattern <<< "$FILES"`) removes the pipe and eliminates the race.</comment>
<file context>
@@ -0,0 +1,140 @@
+ echo "$FILES"
+ echo "diff_base=$DIFF_BASE" >> "$GITHUB_OUTPUT"
+
+ has_dart=$(echo "$FILES" | grep -q '\.dart$' && echo true || echo false)
+ has_python=$(echo "$FILES" | grep -q 'backend/.*\.py$' && echo true || echo false)
+ has_arb=$(echo "$FILES" | grep -q '\.arb$' && echo true || echo false)
</file context>
| run: python3 .github/scripts/desktop-changelog.py validate | ||
|
|
||
| - name: Check desktop changelog entry | ||
| if: github.event_name == 'pull_request' && !startsWith(github.head_ref, 'changelog/v') |
There was a problem hiding this comment.
P2: The Check desktop changelog entry step can be bypassed by any PR author (including forks) simply naming their branch changelog/v..., because github.head_ref is user-controlled and there is no actor or repository guard. Consider tightening the exception so only release-automation contexts can skip the check.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/repo-checks.yml, line 73:
<comment>The `Check desktop changelog entry` step can be bypassed by any PR author (including forks) simply naming their branch `changelog/v...`, because `github.head_ref` is user-controlled and there is no actor or repository guard. Consider tightening the exception so only release-automation contexts can skip the check.</comment>
<file context>
@@ -0,0 +1,195 @@
+ run: python3 .github/scripts/desktop-changelog.py validate
+
+ - name: Check desktop changelog entry
+ if: github.event_name == 'pull_request' && !startsWith(github.head_ref, 'changelog/v')
+ run: |
+ SKIP_FLAG=""
</file context>
| - name: Check desktop changelog data | ||
| run: python3 .github/scripts/desktop-changelog.py validate | ||
|
|
||
| - name: Check desktop changelog entry |
There was a problem hiding this comment.
P2: The Check desktop changelog entry step uses the no-changelog-needed PR label to decide whether to skip the changelog check, but the pull_request trigger does not include labeled or unlabeled activity types. When a label is added or removed after the workflow has already run, the check result becomes stale until a new commit is pushed or the workflow is manually re-run. Consider adding types: [opened, synchronize, reopened, labeled, unlabeled] to the pull_request trigger so label-driven skips are re-evaluated automatically.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/repo-checks.yml, line 72:
<comment>The `Check desktop changelog entry` step uses the `no-changelog-needed` PR label to decide whether to skip the changelog check, but the `pull_request` trigger does not include `labeled` or `unlabeled` activity types. When a label is added or removed after the workflow has already run, the check result becomes stale until a new commit is pushed or the workflow is manually re-run. Consider adding `types: [opened, synchronize, reopened, labeled, unlabeled]` to the `pull_request` trigger so label-driven skips are re-evaluated automatically.</comment>
<file context>
@@ -0,0 +1,195 @@
+ - name: Check desktop changelog data
+ run: python3 .github/scripts/desktop-changelog.py validate
+
+ - name: Check desktop changelog entry
+ if: github.event_name == 'pull_request' && !startsWith(github.head_ref, 'changelog/v')
+ run: |
</file context>
Summary
.github/actions/detect-changesso split workflows can reuse the same gating logic.Repo Checks,Backend Checks,Desktop Checks,Mobile App Checks, andWeb Checks.Desktop Swift CIandOpenAPI Contractseparate so their top-level names stay explicit.Verification
/opt/homebrew/bin/actionlint -shellcheck '' .github/workflows/repo-checks.yml .github/workflows/mobile-app-checks.yml .github/workflows/backend-checks.yml .github/workflows/desktop-checks.yml .github/workflows/web-checks.ymlpython3YAML parse/job listing for the shared action and split workflowsgit diff --checkgit pushpre-push ran relevant backend runtime env, async blocker, OpenAPI/schema, workflow checks; local push hook later hit unrelated unconditional Desktop Swift SwiftPM resolution forFluidAudiobefore the branch was pushed with--no-verify.