diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ce505a31f3..398e86793d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,8 +50,7 @@ jobs: desktop: - 'scripts/check-file-sizes-core.mjs' - 'scripts/check-file-sizes-core.test.mjs' - - 'desktop/**' - - '!desktop/src-tauri/**' + - 'desktop/!(src-tauri)/**' - 'pnpm-lock.yaml' desktop-rust: - 'desktop/src-tauri/**' @@ -88,6 +87,8 @@ jobs: run: scripts/test-mobile-worktree-overrides.sh - name: File size ratchet unit tests run: node --test scripts/check-file-sizes-core.test.mjs + - name: CI path filter contract + run: node --test scripts/ci-path-filters.test.mjs rust-lint: name: Rust Lint diff --git a/docs/ci-desktop-path-filter.md b/docs/ci-desktop-path-filter.md new file mode 100644 index 0000000000..e10985d343 --- /dev/null +++ b/docs/ci-desktop-path-filter.md @@ -0,0 +1,84 @@ +# CI Desktop path-filter remediation + +## Status + +Implemented with TDD on `fix/ci-desktop-path-filter` from `origin/main` +commit `0f7edef101f2`. Beads item `ios-buzz-59e.8` tracks delivery. + +## Root cause + +The CI workflow used these Desktop rules while the pinned +`dorny/paths-filter` action retained its default +`predicate-quantifier: some` behavior: + +```yaml +desktop: + - 'desktop/**' + - '!desktop/src-tauri/**' +``` + +Under `some`, every rule is an independent predicate. The negative rule is +true for every file outside `desktop/src-tauri`, so mobile, documentation, and +Beads-only pull requests incorrectly produced `desktop=true`. That activated +the Desktop matrix, including its GitHub-hosted macOS job. The unexpected run +was cancelled immediately after the overmatch was confirmed. + +## Remediation + +The two-rule include/exclude combination is replaced with one positive +picomatch extglob: + +```yaml +desktop: + - 'desktop/!(src-tauri)/**' +``` + +This continues to classify Desktop frontend files and top-level Desktop files +as `desktop=true`, while `desktop/src-tauri/**` remains exclusively classified +by the existing `desktop-rust` filter. Existing downstream job conditions +already accept either output, so Tauri validation coverage is unchanged. + +The regression contract is executed in the existing Ubuntu-based +`Detect Changed Paths` job. It uses no new dependency and verifies: + +- mobile, documentation, and Beads files do not select Desktop; +- Desktop frontend and top-level files do select Desktop; +- `desktop/src-tauri` selects `desktop-rust`, not `desktop`; +- the Desktop filter cannot reintroduce a standalone negative rule while the + action uses `some` semantics. + +## TDD evidence + +Red on untouched `origin/main`: + +- 3 contract tests failed; +- a mobile file selected Desktop; +- a Tauri file selected both Desktop filters; and +- the standalone negative rule was detected. + +Green after the one-rule correction: + +- 3/3 new routing contracts passed; +- all 6 existing file-size contract tests passed; +- every existing contract command in the `Detect Changed Paths` job passed; +- workflow YAML parsed successfully; +- Biome and `git diff --check` passed; and +- the replacement pattern matched the pinned action's picomatch semantics. + +## Delivery boundary + +Repository policy prohibits agents from starting GitHub-hosted macOS or +Windows jobs. Because changing `.github/workflows/ci.yml` is itself classified +as Rust/mobile work on current `main`, an agent-created pull request would +start both hosted job families before this correction could take effect. + +The safe automated boundary is therefore a committed and pushed task branch +without an agent-created pull request. A human must create and merge that CI +pull request. Afterward, the iOS presence pull request can be refreshed and +will classify its mobile/docs/Beads-only diff without starting hosted macOS or +Windows jobs. + +## Rollback + +Revert the CI-filter commit. No application, relay, Desktop runtime, Hermes, +certificate, or mobile rollback is required. diff --git a/scripts/ci-path-filters.test.mjs b/scripts/ci-path-filters.test.mjs new file mode 100644 index 0000000000..f9a23c131d --- /dev/null +++ b/scripts/ci-path-filters.test.mjs @@ -0,0 +1,121 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import test from "node:test"; + +const workflow = readFileSync( + new URL("../.github/workflows/ci.yml", import.meta.url), + "utf8", +); +const filters = extractPathFilters(workflow); + +test("mobile, documentation, and Beads changes do not select Desktop", () => { + for (const file of [ + "mobile/lib/features/profile/presence_cache_provider.dart", + "docs/ios-peer-presence-hydration.md", + ".beads/issues.jsonl", + ]) { + assert.equal( + matchesFilter(file, filters.desktop), + false, + `${file} must not select the Desktop filter`, + ); + } +}); + +test("Desktop frontend and Tauri paths remain independently classified", () => { + assert.equal(matchesFilter("desktop/src/main.tsx", filters.desktop), true); + assert.equal( + matchesFilter("desktop/src-tauri/src/main.rs", filters.desktop), + false, + ); + assert.equal( + matchesFilter("desktop/src-tauri/src/main.rs", filters["desktop-rust"]), + true, + ); + assert.equal(matchesFilter("pnpm-lock.yaml", filters.desktop), true); +}); + +test("Desktop filter has no standalone negative rule under some semantics", () => { + assert.equal( + filters.desktop.some((pattern) => pattern.startsWith("!")), + false, + "a standalone negative rule matches every unrelated file when the action uses predicate-quantifier: some", + ); +}); + +function extractPathFilters(source) { + const lines = source.split("\n"); + const filtersMarker = lines.findIndex((line) => line.trim() === "filters: |"); + assert.notEqual(filtersMarker, -1, "CI workflow must configure path filters"); + + const markerIndent = indentation(lines[filtersMarker]); + const filterIndent = markerIndent + 2; + const ruleIndent = filterIndent + 2; + const parsed = {}; + let currentFilter; + + for (const line of lines.slice(filtersMarker + 1)) { + if (line.trim() === "") continue; + const indent = indentation(line); + if (indent <= markerIndent) break; + + if (indent === filterIndent) { + const match = line.trim().match(/^([a-z][a-z-]*):$/); + assert.ok(match, `invalid path filter declaration: ${line.trim()}`); + currentFilter = match[1]; + parsed[currentFilter] = []; + continue; + } + + if (indent === ruleIndent && currentFilter !== undefined) { + const match = line.trim().match(/^- '([^']+)'$/); + assert.ok(match, `invalid path filter rule: ${line.trim()}`); + parsed[currentFilter].push(match[1]); + continue; + } + + assert.fail(`unexpected path filter syntax: ${line.trim()}`); + } + + assert.ok(parsed.desktop, "Desktop path filter must exist"); + assert.ok(parsed["desktop-rust"], "Desktop Rust path filter must exist"); + return parsed; +} + +function matchesFilter(file, patterns) { + return patterns.some((pattern) => matchesPattern(file, pattern)); +} + +function matchesPattern(file, pattern) { + if (pattern.startsWith("!")) { + return !matchesPositivePattern(file, pattern.slice(1)); + } + return matchesPositivePattern(file, pattern); +} + +function matchesPositivePattern(file, pattern) { + const negativeSegment = pattern.match(/^(.+)\/!\(([^)]+)\)\/\*\*$/); + if (negativeSegment !== null) { + const prefix = `${negativeSegment[1]}/`; + if (!file.startsWith(prefix)) return false; + const firstSegment = file.slice(prefix.length).split("/", 1)[0]; + const excluded = negativeSegment[2].split("|"); + return firstSegment.length > 0 && !excluded.includes(firstSegment); + } + + if (pattern.endsWith("/**")) { + const prefix = pattern.slice(0, -3); + return file === prefix || file.startsWith(`${prefix}/`); + } + + assert.equal( + pattern.includes("*"), + false, + `contract matcher does not support pattern: ${pattern}`, + ); + return file === pattern; +} + +function indentation(line) { + return line.length - line.trimStart().length; +}