Skip to content

ci: cross-platform matrix + paths-ignore + concurrency #556

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rohitg00 merged 5 commits into from
May 20, 2026
Merged

ci: cross-platform matrix + paths-ignore + concurrency #556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eabcaf9
ci: cross-platform matrix + paths-ignore + concurrency (lift from iii)
rohitg00 May 19, 2026
21d710b
ci(windows): force bash shell so build script's POSIX idioms work
rohitg00 May 20, 2026
776831f
ci(windows): point npm script-shell at git-bash before build
rohitg00 May 20, 2026
cfabff7
ci: drop windows-latest from matrix (obsidian-export hardcoded POSIX …
rohitg00 May 20, 2026
4e47b84
test(fs-watcher): bump waits to 1500ms + describe retry for macos fse…
rohitg00 May 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 44 additions & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,62 @@
name: CI

# `paths-ignore` keeps doc-only / website / README / CHANGELOG churn from
# burning runner minutes. Source / config / workflow changes always run.
# `workflow_dispatch` gives a manual re-run button for flake debugging.
on:
push:
branches: [main]
paths-ignore:
- "README.md"
- "CHANGELOG.md"
- "AGENTS.md"
- "ROADMAP.md"
- "website/**"
- "docs/**"
- "assets/**"
- "deploy/**/README.md"
- "**/*.md"
- "**/*.mdx"
pull_request:
branches: [main]
paths-ignore:
- "README.md"
- "CHANGELOG.md"
- "AGENTS.md"
- "ROADMAP.md"
- "website/**"
- "docs/**"
- "assets/**"
- "deploy/**/README.md"
- "**/*.md"
- "**/*.mdx"
workflow_dispatch:

# Cancel in-flight PR runs when a force-push lands. Keep push runs to
# protect against partial state on main.
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}

jobs:
test:
runs-on: ubuntu-latest
runs-on: ${{ matrix.os }}
strategy:
# Don't bail the whole matrix on one cell's failure — we want to
# see whether the same failure reproduces across OSes (e.g.
# whether a flake is platform-specific or universal).
fail-fast: false
matrix:
# Windows held back: test/obsidian-export.test.ts has hardcoded
# POSIX paths (`/tmp/...`) that fail on D:\ drive runners.
# src/functions/obsidian-export.ts needs os.tmpdir() + path.join
# rework before Windows can be added back. Tracked as follow-up.
os: [ubuntu-latest, macos-latest]
node-version: [20, 22]
steps:
- uses: actions/checkout@v6
with:
persist-credentials: false
- uses: actions/setup-node@v6
Comment on lines 57 to 60
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 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 -20

Repository: rohitg00/agentmemory

Length of output: 398

🏁 Script executed:

# Look for CONTRIBUTING, security, or policy files
fd -i "contributing\|security\|policy" -e md -e txt

Repository: 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 -20

Repository: 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:

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.

Suggested change
- 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.

with:
node-version: ${{ matrix.node-version }}
Expand Down
10 changes: 5 additions & 5 deletions test/fs-watcher.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ function wait(ms: number): Promise<void> {
return new Promise((r) => setTimeout(r, ms));
}

describe("FilesystemWatcher", () => {
describe("FilesystemWatcher", { retry: 2 }, () => {
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 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:

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.

let root: string;
const originalFetch = globalThis.fetch;
let captured: Array<{ url: string; body: unknown; headers: Record<string, string> }>;
Expand Down Expand Up @@ -49,7 +49,7 @@ describe("FilesystemWatcher", () => {
w.start();
try {
writeFileSync(join(root, "notes.md"), "hello world\n");
await wait(800);
await wait(1500);
expect(captured.length).toBeGreaterThanOrEqual(1);
const obs = captured[captured.length - 1];
expect(obs.url).toBe("http://localhost:3111/agentmemory/observe");
Expand Down Expand Up @@ -87,7 +87,7 @@ describe("FilesystemWatcher", () => {
w.start();
try {
unlinkSync(join(root, "old.md"));
await wait(800);
await wait(1500);
const deletes = captured.filter(
(c) => (c.body as { data: { changeKind: string } }).data?.changeKind === "file_delete",
);
Expand Down Expand Up @@ -116,7 +116,7 @@ describe("FilesystemWatcher", () => {
w.start();
try {
writeFileSync(join(root, "node_modules", "ignored.js"), "x");
await wait(800);
await wait(1500);
const matches = captured.filter((c) =>
(c.body as { data: { files: string[] } }).data?.files?.some((f) => f.includes("ignored.js")),
);
Expand All @@ -136,7 +136,7 @@ describe("FilesystemWatcher", () => {
w.start();
try {
writeFileSync(join(root, "secret.md"), "bearer test\n");
await wait(800);
await wait(1500);
expect(captured.length).toBeGreaterThanOrEqual(1);
const headers = captured[captured.length - 1].headers as Record<string, string>;
expect(headers.authorization).toBe("Bearer shhh");
Expand Down