Implement buildPackageFilters() with exhaustive unit tests #169

New issue

Closed

opened 2026-09-08 00:02:10 +00:00 by david · 1 comment

david commented

2026-09-08 00:02:10 +00:00

Owner

Summary

Add a pure function buildPackageFilters({ rgOk, fjState }) to scripts/local-install.mjs that translates detected tool state into the pi settings.json package-filter shape (or null when no filtering is needed). This is the core mapping logic that the settings.json rewrite step will apply.

Background

pi's package system supports an object filter form in settings.json for narrowing what a registered package loads:

{
  "source": "/abs/path/to/repo",
  "extensions": ["!extensions/pr-comments/src/index.ts"],
  "skills": ["!skills/forgejo-cli"]
}

!pattern excludes matching paths; omitting a key loads all of that type. This is documented in pi's packages documentation under "Package Filtering".

Given the tool-state object { rgOk, fjState } (produced by preflight() — see the fj/rg detection-helpers and preflight-refactor steps), the installer needs to build the exact filter object to apply to this repo's single local-path package registration:

fjState === "absent" → exclude extensions/pr-comments/src/index.ts and skills/forgejo-cli.
!rgOk → exclude extensions/rg/index.ts.
fjState === "unauthenticated" or "ok" → do NOT exclude pr-comments/forgejo-cli (unauthenticated is warn-only, not a skip trigger).
When nothing needs excluding, no filter object should be applied at all (the package entry stays a plain string source).

Depends on: #167 (Add fj/rg tool-detection helpers (rgAvailable, detectFjState) with unit tests) and #168 (Refactor preflight() to skip/warn for missing fj/rg instead of hard-failing) — both establish the { rgOk, fjState } tool-state contract this function consumes. This function's own unit tests construct { rgOk, fjState } objects directly (not via preflight()), so implementation can proceed in parallel if needed, but the contract shape must match.

Implementation Details

Add to scripts/local-install.mjs:

/**
 * Build the settings.json package-entry filters for the given tool state.
 * Returns `null` when no filtering is needed (both tools fully usable, or
 * fj merely unauthenticated) — callers should leave the package entry as a
 * plain string source in that case.
 */
export function buildPackageFilters({ rgOk, fjState }) {
  const extensions = [];
  if (fjState === "absent") extensions.push("!extensions/pr-comments/src/index.ts");
  if (!rgOk) extensions.push("!extensions/rg/index.ts");

  const skills = [];
  if (fjState === "absent") skills.push("!skills/forgejo-cli");

  if (extensions.length === 0 && skills.length === 0) return null;
  return { extensions, skills };
}

Notes:

Construction order matters for test determinism and readability: the pr-comments exclusion is always pushed before the rg exclusion in the extensions array when both apply.
unauthenticated and ok are treated identically by this function (neither triggers exclusion) — only "absent" triggers the fj-related exclusions.
This function does not touch the filesystem or settings.json at all — it's a pure mapping, fully unit-testable in isolation without any shim/PATH setup.

Acceptance Criteria

buildPackageFilters({ rgOk: true, fjState: "ok" }) returns null.
buildPackageFilters({ rgOk: true, fjState: "unauthenticated" }) returns null (unauthenticated is not a filter trigger).
buildPackageFilters({ rgOk: false, fjState: "ok" }) returns { extensions: ["!extensions/rg/index.ts"], skills: [] }.
buildPackageFilters({ rgOk: false, fjState: "unauthenticated" }) returns { extensions: ["!extensions/rg/index.ts"], skills: [] }.
buildPackageFilters({ rgOk: true, fjState: "absent" }) returns { extensions: ["!extensions/pr-comments/src/index.ts"], skills: ["!skills/forgejo-cli"] }.
buildPackageFilters({ rgOk: false, fjState: "absent" }) returns { extensions: ["!extensions/pr-comments/src/index.ts", "!extensions/rg/index.ts"], skills: ["!skills/forgejo-cli"] } (pr-comments exclusion listed before rg exclusion).
The function is exported and has no side effects (no filesystem/process access).

Test Plan

Unit test each of the six cells in the acceptance criteria above via assert.deepEqual.
npm test — all green, including these new pure-function tests (no PATH shims or temp directories needed for this specific test group).

## Summary Add a pure function `buildPackageFilters({ rgOk, fjState })` to `scripts/local-install.mjs` that translates detected tool state into the pi settings.json package-filter shape (or `null` when no filtering is needed). This is the core mapping logic that the settings.json rewrite step will apply. ## Background pi's package system supports an object filter form in `settings.json` for narrowing what a registered package loads: ```json { "source": "/abs/path/to/repo", "extensions": ["!extensions/pr-comments/src/index.ts"], "skills": ["!skills/forgejo-cli"] } ``` `!pattern` excludes matching paths; omitting a key loads all of that type. This is documented in pi's packages documentation under "Package Filtering". Given the tool-state object `{ rgOk, fjState }` (produced by `preflight()` — see the fj/rg detection-helpers and preflight-refactor steps), the installer needs to build the exact filter object to apply to this repo's single local-path package registration: - `fjState === "absent"` → exclude `extensions/pr-comments/src/index.ts` and `skills/forgejo-cli`. - `!rgOk` → exclude `extensions/rg/index.ts`. - `fjState === "unauthenticated"` or `"ok"` → do NOT exclude pr-comments/forgejo-cli (unauthenticated is warn-only, not a skip trigger). - When nothing needs excluding, no filter object should be applied at all (the package entry stays a plain string source). **Depends on:** #167 (Add fj/rg tool-detection helpers (rgAvailable, detectFjState) with unit tests) and #168 (Refactor preflight() to skip/warn for missing fj/rg instead of hard-failing) — both establish the `{ rgOk, fjState }` tool-state contract this function consumes. This function's own unit tests construct `{ rgOk, fjState }` objects directly (not via `preflight()`), so implementation can proceed in parallel if needed, but the contract shape must match. ## Implementation Details Add to `scripts/local-install.mjs`: ```js /** * Build the settings.json package-entry filters for the given tool state. * Returns `null` when no filtering is needed (both tools fully usable, or * fj merely unauthenticated) — callers should leave the package entry as a * plain string source in that case. */ export function buildPackageFilters({ rgOk, fjState }) { const extensions = []; if (fjState === "absent") extensions.push("!extensions/pr-comments/src/index.ts"); if (!rgOk) extensions.push("!extensions/rg/index.ts"); const skills = []; if (fjState === "absent") skills.push("!skills/forgejo-cli"); if (extensions.length === 0 && skills.length === 0) return null; return { extensions, skills }; } ``` Notes: - Construction order matters for test determinism and readability: the `pr-comments` exclusion is always pushed before the `rg` exclusion in the `extensions` array when both apply. - `unauthenticated` and `ok` are treated identically by this function (neither triggers exclusion) — only `"absent"` triggers the fj-related exclusions. - This function does not touch the filesystem or settings.json at all — it's a pure mapping, fully unit-testable in isolation without any shim/PATH setup. ## Acceptance Criteria - [ ] `buildPackageFilters({ rgOk: true, fjState: "ok" })` returns `null`. - [ ] `buildPackageFilters({ rgOk: true, fjState: "unauthenticated" })` returns `null` (unauthenticated is not a filter trigger). - [ ] `buildPackageFilters({ rgOk: false, fjState: "ok" })` returns `{ extensions: ["!extensions/rg/index.ts"], skills: [] }`. - [ ] `buildPackageFilters({ rgOk: false, fjState: "unauthenticated" })` returns `{ extensions: ["!extensions/rg/index.ts"], skills: [] }`. - [ ] `buildPackageFilters({ rgOk: true, fjState: "absent" })` returns `{ extensions: ["!extensions/pr-comments/src/index.ts"], skills: ["!skills/forgejo-cli"] }`. - [ ] `buildPackageFilters({ rgOk: false, fjState: "absent" })` returns `{ extensions: ["!extensions/pr-comments/src/index.ts", "!extensions/rg/index.ts"], skills: ["!skills/forgejo-cli"] }` (pr-comments exclusion listed before rg exclusion). - [ ] The function is exported and has no side effects (no filesystem/process access). ## Test Plan - Unit test each of the six cells in the acceptance criteria above via `assert.deepEqual`. - `npm test` — all green, including these new pure-function tests (no PATH shims or temp directories needed for this specific test group).

david added this to the Install Tool Detection: M3 - Settings.json filtering milestone

2026-09-08 00:02:10 +00:00

david added the

enhancement

label

2026-09-08 00:02:10 +00:00

david referenced this issue

2026-09-08 01:39:11 +00:00

168: Refactor preflight() to skip/warn for missing fj/rg instead of hard-failing #177

david referenced this issue

2026-09-08 02:00:41 +00:00

169: Implement buildPackageFilters() with exhaustive unit tests #178