ci: cross-platform matrix + paths-ignore + concurrency#556
Conversation
Three patterns lifted from iii-hq/iii/.github: 1. **OS matrix** — Linux + Windows + macOS, both Node 20 + 22. 6 cells, ~3min each, ~18min wall time. Direct test against the class of bug #487 caught: hooks crashing on Windows usernames with spaces. Pre-merge Linux-only CI meant that bug landed in main + a release. fail-fast: false so a flake on one cell doesn't mask whether the same failure reproduces elsewhere. 2. **paths-ignore** — skip CI runs on README / CHANGELOG / docs / website / assets / .md / .mdx pushes. ~half the runner minutes back on doc-only churn. Source / config / workflow changes always run. 3. **concurrency + cancel-in-progress** — PR force-pushes cancel in-flight runs instead of piling them up. Push to main protected (concurrency group still scoped to ref, no cancel for main pushes). Plus minor hardening: persist-credentials: false on the checkout step so the GITHUB_TOKEN doesn't land in .git/config. What was NOT lifted (rationale per plan): - Per-package reusable workflows (Rust/Python/Homebrew — non-TS). - License-header check (no per-file Apache banners in agentmemory). - CLA bot (defer until external PR volume justifies friction). - tsc --noEmit lint job (codebase has ~10 pre-existing type errors tsdown skips; gating CI on those would block every PR until fixed; tracked as separate cleanup). - Smoke test (`agentmemory demo + livez`) — defer to its own PR with its own validation cycle. - Codecov badge — defer until baseline is set.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCI now ignores doc/markdown-only pushes/PRs to main, adds manual dispatch and PR concurrency cancellation, expands the ChangesCI Workflow Enhancement
FilesystemWatcher tests
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub
participant Workflow
participant MatrixRunner
participant Checkout
participant TestSteps
Developer->>GitHub: push / pull_request / workflow_dispatch
GitHub->>Workflow: start workflow
Workflow->>Workflow: evaluate `paths-ignore` filters
Workflow->>Workflow: evaluate `concurrency` for PRs
Workflow->>MatrixRunner: spawn matrix jobs (ubuntu/macos × Node20/22)
MatrixRunner->>Checkout: actions/checkout (persist-credentials: false)
MatrixRunner->>TestSteps: run tests (fail-fast: false)
TestSteps-->>MatrixRunner: results
MatrixRunner-->>Workflow: job complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
56-58: ⚡ Quick winConsider pinning GitHub Actions to commit SHA for stronger supply-chain integrity.
Lines 56 and 59 use mutable tags (
actions/checkout@v6andactions/setup-node@v6). While Dependabot manages updates, committing to full commit SHAs provides stronger protection against tag mutation attacks. This would require manually resolving the action SHAs (e.g.,actions/checkout@abc1234567...) and periodic updates via Dependabot PRs.🤖 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 56 - 58, Replace the mutable action tags used in the workflow (specifically the uses entries "actions/checkout@v6" and "actions/setup-node@v6") with their corresponding full commit SHAs (e.g., "actions/checkout@<full-sha>" and "actions/setup-node@<full-sha>"), updating the YAML to reference those SHAs and periodically bump them via Dependabot or manual review to keep them current.
🤖 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.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 56-58: Replace the mutable action tags used in the workflow
(specifically the uses entries "actions/checkout@v6" and
"actions/setup-node@v6") with their corresponding full commit SHAs (e.g.,
"actions/checkout@<full-sha>" and "actions/setup-node@<full-sha>"), updating the
YAML to reference those SHAs and periodically bump them via Dependabot or manual
review to keep them current.
rohitg00
changed the title
Windows runners default to cmd.exe for npm run scripts; the build script uses POSIX patterns the build script's exit codes (`cp ... 2>/dev/null || true`, `mkdir -p`) that cmd doesn't parse. ubuntu + macos already use bash by default so this is Windows-only behaviour change. Alternative: rewrite the build script in Node. Bigger lift, not minimal.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/ci.yml:
- Around line 56-59: Replace mutable tag references actions/checkout@v6 and
actions/setup-node@v6 with their corresponding full immutable commit SHAs;
locate the workflow steps that use "uses: actions/checkout@v6" and "uses:
actions/setup-node@v6" and update each to the full-length commit SHA for the
specific release you want to pin, then verify the SHAs match the upstream
repository commits and run the workflow to confirm no breakage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| - uses: actions/checkout@v6 | ||
| with: | ||
| persist-credentials: false | ||
| - uses: actions/setup-node@v6 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, read the CI workflow file to verify lines 56-59
cat -n .github/workflows/ci.yml | sed -n '50,65p'Repository: rohitg00/agentmemory
Length of output: 913
🏁 Script executed:
# Search for any documented policies or security guidelines
fd -e md -e txt | xargs grep -l "pin\|action\|commit.*sha" 2>/dev/null | head -20Repository: rohitg00/agentmemory
Length of output: 398
🏁 Script executed:
# Look for CONTRIBUTING, security, or policy files
fd -i "contributing\|security\|policy" -e md -e txtRepository: rohitg00/agentmemory
Length of output: 46
🏁 Script executed:
# Check CONTRIBUTING.md for any policy about GitHub Actions pinning
cat CONTRIBUTING.md | grep -i -A5 -B5 "action\|pin\|commit"Repository: rohitg00/agentmemory
Length of output: 2527
🏁 Script executed:
# Also check README.md and other key docs
grep -r "action.*pin\|pin.*action\|commit.*sha" . --include="*.md" 2>/dev/null | head -20Repository: rohitg00/agentmemory
Length of output: 4506
🌐 Web query:
GitHub Actions version pinning best practices security supply chain
💡 Result:
To secure your supply chain against malicious updates or compromised maintainer accounts, the industry-standard best practice for GitHub Actions is to pin all actions to their full-length commit SHA [1][2][3]. Why Pinning to SHA is Critical GitHub Actions referenced by tags (e.g., @v4) or branches are mutable, meaning they can be updated or moved by the repository owner at any time [4][5]. If an action's repository is compromised, an attacker can force-push new code to an existing tag, causing all workflows using that tag to automatically execute the malicious code [6][7][5]. Pinning to a full-length commit SHA ensures that your workflow always executes the exact, immutable code you have audited and trusted [4][3]. Best Practices for Implementation 1. Use Full-Length Commit SHAs: Always use the full 40-character commit SHA rather than a short hash [3]. 2. Include Version Comments: To maintain readability and track which version the SHA corresponds to, append the version tag as a comment in your workflow file [1][2]: uses: actions/checkout@b4ffde6 # v4.1.1 3. Enforce via Policy: Organizations can enforce this practice at the enterprise or organization level by configuring GitHub Actions policies to require full-length commit SHAs for all actions [6][3]. 4. Automate Updates: Use Dependabot to manage your pinned actions [8]. When configured for GitHub Actions, Dependabot will monitor for updates and open pull requests to bump your pinned SHAs to newer, secure versions [8][9]. 5. Be Aware of Transitive Dependencies: Pinning your direct action dependencies does not automatically pin the actions they call internally [10]. Be cautious when using complex actions that pull in many transitive dependencies, as these can also be targets for supply chain attacks [10]. Additional Security Considerations - Trust and Verification: If you must use tags for convenience, only do so for actions from highly trusted, verified creators [3]. - Monitor for Compromise: Even with pinning, stay informed about security advisories for the actions you use [4][11]. - Platform Controls: Leverage GitHub's evolving security features, such as immutable releases (which prevent tags from being moved) and organization-level execution policies, to further harden your CI/CD environment [6][12][5].
Citations:
- 1: Pin all GitHub Actions to immutable SHA commits for supply chain security github/gh-aw#5691
- 2: Pin GitHub Actions to commit SHAs in release workflow github/gh-aw#10021
- 3: https://docs.github.com/en/actions/reference/security/secure-use
- 4: https://www.stepsecurity.io/blog/pinning-github-actions-for-enhanced-security-a-complete-guide
- 5: https://dev.to/kanywst/hacking-github-from-tag-rewrites-to-dangling-commits-where-the-git-protocol-trusts-you-without-2o4h
- 6: https://github.blog/changelog/2025-08-15-github-actions-policy-now-supports-blocking-and-sha-pinning-actions/
- 7: https://gbhackers.com/compromised-github-action/
- 8: https://docs.github.com/en/code-security/how-tos/secure-your-supply-chain/secure-your-dependencies/keeping-your-actions-up-to-date-with-dependabot?learn=dependency_version_updates
- 9: Add option for GH actions to prefer SHA pins dependabot/dependabot-core#7913
- 10: https://www.aikido.dev/blog/checklist-github-actions
- 11: https://dev.to/martinhicks/filing-my-first-security-advisory-1i3f
- 12: https://dev.to/gitguardian/the-future-of-github-actions-security-and-what-you-can-do-right-now-32nj
Pin GitHub Actions to immutable commit SHAs.
Line 56 and Line 59 use mutable major tags (@v6). These should be pinned to full-length commit SHAs to prevent supply-chain attacks; if the upstream repository is compromised, tag-based references will execute malicious code pushed to that tag.
Suggested change
- - uses: actions/checkout@v6
+ - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
with:
persist-credentials: false
- - uses: actions/setup-node@v6
+ - uses: actions/setup-node@1a4442caab129478d0a1be582e52490f7e012c96 # v4.0.1
with:
node-version: ${{ matrix.node-version }}📝 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.
| - uses: actions/checkout@v6 | |
| with: | |
| persist-credentials: false | |
| - uses: actions/setup-node@v6 | |
| - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 | |
| with: | |
| persist-credentials: false | |
| - uses: actions/setup-node@1a4442caab129478d0a1be582e52490f7e012c96 # v4.0.1 |
🧰 Tools
🪛 zizmor (1.25.2)
[error] 56-56: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 59-59: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 56 - 59, Replace mutable tag
references actions/checkout@v6 and actions/setup-node@v6 with their
corresponding full immutable commit SHAs; locate the workflow steps that use
"uses: actions/checkout@v6" and "uses: actions/setup-node@v6" and update each to
the full-length commit SHA for the specific release you want to pin, then verify
the SHAs match the upstream repository commits and run the workflow to confirm
no breakage.
`shell: bash` on the step only sets the shell for the step's own runner; `npm run` still spawns its inner script via npm's `script-shell` config, which defaults to cmd.exe on Windows. Configure npm to use Git-Bash (preinstalled on GitHub-hosted Windows runners) so `npm run build` and `npm run test` execute the build script the same way ubuntu + macos do. Step is gated on `runner.os == 'Windows'` so it's a no-op on the other matrix cells.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
56-56:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDuplicate: Pin actions to commit SHAs for supply-chain security.
This issue was flagged in a previous review and remains unaddressed. The mutable
@v6tags should be replaced with immutable commit SHAs to prevent supply-chain attacks if upstream repositories are compromised.Run this script to find the correct commit SHAs for the v6 releases:
#!/bin/bash # Find the latest commit SHA for actions/checkout@v6 and actions/setup-node@v6 echo "=== actions/checkout v6 ===" gh api repos/actions/checkout/git/ref/tags/v6 --jq '.object.sha' 2>/dev/null || echo "Tag lookup failed; trying releases..." gh api repos/actions/checkout/releases/latest --jq '"\(.tag_name): \(.target_commitish)"' echo -e "\n=== actions/setup-node v6 ===" gh api repos/actions/setup-node/git/ref/tags/v6 --jq '.object.sha' 2>/dev/null || echo "Tag lookup failed; trying releases..." gh api repos/actions/setup-node/releases/latest --jq '"\(.tag_name): \(.target_commitish)"'Also applies to: 59-59
🤖 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 at line 56, Replace the mutable action tags with immutable commit SHAs: change the uses entries that reference actions/checkout@v6 and actions/setup-node@v6 to the corresponding commit SHA for the v6 tag (pin to the full 40-char commit) so the workflow uses a specific commit instead of the mutable `@v6` tag; locate the uses lines for "actions/checkout@v6" and "actions/setup-node@v6" in the CI YAML and replace the tag with the exact commit SHA returned by the GitHub API or release metadata.
🤖 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.
Duplicate comments:
In @.github/workflows/ci.yml:
- Line 56: Replace the mutable action tags with immutable commit SHAs: change
the uses entries that reference actions/checkout@v6 and actions/setup-node@v6 to
the corresponding commit SHA for the v6 tag (pin to the full 40-char commit) so
the workflow uses a specific commit instead of the mutable `@v6` tag; locate the
uses lines for "actions/checkout@v6" and "actions/setup-node@v6" in the CI YAML
and replace the tag with the exact commit SHA returned by the GitHub API or
release metadata.
…paths) Windows runners fail on test/obsidian-export.test.ts because the test + src hardcode `/tmp/...` POSIX paths that don't resolve on the D:\ drive Windows uses. Fixing it cleanly requires reworking src/functions/obsidian-export.ts to use os.tmpdir() + path.join, which is a separate scope. Drop windows from the matrix for now. Ship ubuntu + macos coverage (real darwin/linux divergence catch) and file a follow-up to make obsidian-export cross-platform so Windows can be added back.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/fs-watcher.test.ts`:
- Line 15: Remove the invalid options object passed to describe — change the
line using describe("FilesystemWatcher", { retry: 2 }, ...) to a normal
describe("FilesystemWatcher", ...) call, and configure retries in your Vitest
config instead (add/adjust test.retry or test.retry.count in vitest.config.ts to
2, or use the detailed test.retry object if needed); this keeps the test suite
signature clean and ensures retries are applied via Vitest configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| } | ||
|
|
||
| describe("FilesystemWatcher", () => { | ||
| describe("FilesystemWatcher", { retry: 2 }, () => { |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Vitest 4.1.6 describe retry configuration syntax
💡 Result:
Vitest 4.1.6 “retry” configuration syntax (for test retries) is: 1) As a number (retries all failures) - In vitest config, set test.retry to a number meaning “how many times to retry the test if it fails”. Default is 0. [1] Example: export default defineConfig({ test: { retry: 3, }, }) [2][1] 2) As an object (fine-grained control; available since Vitest 4.1) The object supports these fields: [2][1] - count?: number The number of times to retry the test if it fails. Default is 0. [2][1] - delay?: number Delay in milliseconds between retry attempts. Default is 0. [2][1] - condition?: RegExp | ((error: TestError) => boolean) Controls which failures should trigger a retry: [2][1] - If RegExp, it is tested against the error message. [1] - If function, it’s called with the TestError; return true to retry. [1] Example (RegExp condition): export default defineConfig({ test: { retry: { count: 2, condition: /ECONNREFUSED|ETIMEDOUT/i, }, }, }) [3] Example (CLI flags mentioned in the retry feature commit; same options map to config fields): - --retry= - --retry - --retry.count - --retry.delay - --retry.condition [3] Notes on the function condition: - Vitest docs note that functions can only be used in test files, not in vitest.config.ts, because configuration is serialized when passed to worker threads. [2][1] So, in Vitest 4.1.6, the retry configuration is either: - test: { retry: } or - test: { retry: { count?: , delay?: , condition?: <RegExp|function> } } [1]
Citations:
- 1: https://vitest.dev/api/test
- 2: https://github.com/vitest-dev/vitest/blob/v4.1.4/docs/api/test.md
- 3: vitest-dev/vitest@9e4cfd2
Remove invalid retry syntax and configure retries in vitest.config.ts instead.
The { retry: 2 } options object as a parameter to describe() is not valid Vitest 4.1.6 syntax. Vitest 4.1.6 only supports retry configuration through the config file, either globally (test: { retry: 2 }) or with fine-grained control (test: { retry: { count: 2, delay: 0, condition: <RegExp|function> } }). The current syntax has no effect and will not add retry behavior to the test suite. Move the retry configuration to vitest.config.ts.
🤖 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 `@test/fs-watcher.test.ts` at line 15, Remove the invalid options object passed
to describe — change the line using describe("FilesystemWatcher", { retry: 2 },
...) to a normal describe("FilesystemWatcher", ...) call, and configure retries
in your Vitest config instead (add/adjust test.retry or test.retry.count in
vitest.config.ts to 2, or use the detailed test.retry object if needed); this
keeps the test suite signature clean and ensures retries are applied via Vitest
configuration.
1 participant
Summary
Three CI patterns added to make
ci.ymlcarry more weight. All pay rent on every run.What changed
1. Cross-platform OS matrix (ubuntu + macos)
4 cells, ~3min each. Catches darwin/linux divergence that Linux-only CI misses.
Windows held back —
test/obsidian-export.test.tshardcodes POSIX/tmp/...paths that fail on Windows runners (D:\tmp\...).src/functions/obsidian-export.tsneedsos.tmpdir()+path.joinrework before Windows can be added back. Will file as follow-up.fail-fast: falseso a flake on one cell doesn't mask whether the same failure reproduces elsewhere.2.
paths-ignorefor doc-only churnSkip CI runs on README / CHANGELOG / docs / website / assets /
.md/.mdxpushes. ~half the runner minutes back on doc-only churn. Source / config / workflow changes always run.3.
concurrencywith PR-only cancelPR force-pushes cancel in-flight runs. Push to main keeps protection (concurrency group scoped to ref, no cancel for main pushes).
4. Minor hardening
persist-credentials: falseonactions/checkout@v6— GITHUB_TOKEN doesn't land in.git/config.Verification
Branch CI itself proves it:
Follow-up (separate PR)
Rework
src/functions/obsidian-export.tsto useos.tmpdir()+path.joinso Windows can be added to the matrix.Summary by CodeRabbit
Chores
Tests