Add fj/rg tool-detection helpers (rgAvailable, detectFjState) with unit tests #167

New issue

Closed

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

david commented

2026-09-08 00:00:27 +00:00

Owner

Summary

Add two small detection helpers to scripts/local-install.mjs — rgAvailable() and detectFjState() — that report whether the optional rg (ripgrep) and fj (Forgejo CLI) binaries are usable, without aborting the process. These replace the current hard-fail probes as pure status-reporting functions that later steps (preflight refactor, settings.json filtering) will consume.

Background

scripts/local-install.mjs is the repo's npm run install:local installer (see docs/IMPLEMENTATION_PLAN.md for the original script design). Its preflight() function currently hard-aborts the entire install if rg is missing from PATH, or if fj auth list fails/returns empty output. This is the first of a multi-step change to make the installer degrade gracefully instead: missing rg should only disable the rg extension, and missing/unauthenticated fj should only disable the pr-comments extension and forgejo-cli skill — everything else should still install.

This step only adds the detection functions themselves (no behavior change to preflight() yet — that's a separate, dependent step).

The existing commandAvailable(cmd) helper (checks cmd is on PATH via command -v / where) is already implemented in scripts/local-install.mjs and should be reused, not reimplemented.

Implementation Details

In scripts/local-install.mjs, near the existing commandAvailable() helper, add:

/** True when `rg` is present on PATH. */
export function rgAvailable() {
  return commandAvailable("rg");
}

/**
 * Detect fj usability.
 * Returns one of: "absent" | "unauthenticated" | "ok".
 *  - "absent": `fj` is not on PATH (commandAvailable("fj") === false).
 *  - "unauthenticated": `fj` is on PATH but `fj auth list` fails or returns
 *    no output (mirrors today's existing preflight check's exact condition).
 *  - "ok": `fj` is on PATH and `fj auth list` succeeds with output.
 */
export function detectFjState() {
  if (!commandAvailable("fj")) return "absent";
  const fjAuth = run("fj", ["auth", "list"]);
  if (!ok(fjAuth) || (fjAuth.stdout ?? "").trim() === "") return "unauthenticated";
  return "ok";
}

Notes:

run() and ok() are existing internal helpers in the file (run wraps spawnSync; ok checks !result.error && result.status === 0) — reuse them exactly as today's fj auth list preflight check does.
Both functions must be exported (export function ...) so they're directly importable by the test file, matching the existing export style used for commandAvailable-adjacent helpers like isInsideManagedClone.
Do not modify preflight() in this step — it still calls its own inline fj auth list / rg checks and hard-fails exactly as today. That refactor is a separate, dependent step.

Test harness change needed for `detectFjState()` tests

scripts/local-install.test.mjs's makeShimDir() helper currently always writes an fj shim into the generated PATH directory (there is no way to omit it, unlike withRg/withPi which already support this). Add a withFj = true option:

function makeShimDir({
  ...,
  withFj = true, // NEW
  ...
} = {}) {
  ...
  if (withFj) {
    writeExecutableShim(dir, "fj", fjBody);
  }
  ...
  // extend the existing restricted-PATH condition:
  if (!withRg || !withPi || !withFj) {
    symlinkSync("/bin/sh", join(dir, "sh"));
  }
}

When withFj: false, the caller must also restrict PATH to the shim dir alone (mirroring how withRg: false / withPi: false are handled today in runPreflightAndCaptureAbort), so commandAvailable("fj") actually returns false in the test rather than falling through to a real fj on the developer's machine.

Acceptance Criteria

rgAvailable() is exported from scripts/local-install.mjs and returns true/false based on commandAvailable("rg").
detectFjState() is exported from scripts/local-install.mjs and returns exactly one of "absent", "unauthenticated", or "ok".
makeShimDir() in scripts/local-install.test.mjs supports a withFj option (default true) that, when false, omits the fj shim and is included in the PATH-restriction condition.
Unit test: detectFjState() returns "absent" when fj is not on PATH (via withFj: false).
Unit test: detectFjState() returns "unauthenticated" when fj auth list fails (via the existing fjAuthFails: true shim flag).
Unit test: detectFjState() returns "unauthenticated" when fj auth list returns no accounts (via the existing fjAuthEmpty: true shim flag).
Unit test: detectFjState() returns "ok" when fj is present and fj auth list succeeds with output (default shim).
Unit test: rgAvailable() returns false when rg is absent (via the existing withRg: false shim flag) and true by default.
preflight()'s existing behavior (hard-fail on missing rg/fj) is unchanged by this step — no existing tests should need to change.

Test Plan

npm test (runs node --test scripts/local-install.test.mjs) — all existing tests still pass unmodified, plus the new detection-helper tests pass.
node --test scripts/local-install.test.mjs run directly — same result.
Manually verify detectFjState() returns "ok" on a machine with fj installed and authenticated, by temporarily adding a one-off console.log(detectFjState()) call (or a throwaway test) and removing it before commit.

## Summary Add two small detection helpers to `scripts/local-install.mjs` — `rgAvailable()` and `detectFjState()` — that report whether the optional `rg` (ripgrep) and `fj` (Forgejo CLI) binaries are usable, without aborting the process. These replace the current hard-fail probes as pure status-reporting functions that later steps (preflight refactor, settings.json filtering) will consume. ## Background `scripts/local-install.mjs` is the repo's `npm run install:local` installer (see `docs/IMPLEMENTATION_PLAN.md` for the original script design). Its `preflight()` function currently hard-aborts the entire install if `rg` is missing from PATH, or if `fj auth list` fails/returns empty output. This is the first of a multi-step change to make the installer degrade gracefully instead: missing `rg` should only disable the `rg` extension, and missing/unauthenticated `fj` should only disable the `pr-comments` extension and `forgejo-cli` skill — everything else should still install. This step only adds the detection functions themselves (no behavior change to `preflight()` yet — that's a separate, dependent step). The existing `commandAvailable(cmd)` helper (checks `cmd` is on PATH via `command -v` / `where`) is already implemented in `scripts/local-install.mjs` and should be reused, not reimplemented. ## Implementation Details In `scripts/local-install.mjs`, near the existing `commandAvailable()` helper, add: ```js /** True when `rg` is present on PATH. */ export function rgAvailable() { return commandAvailable("rg"); } /** * Detect fj usability. * Returns one of: "absent" | "unauthenticated" | "ok". * - "absent": `fj` is not on PATH (commandAvailable("fj") === false). * - "unauthenticated": `fj` is on PATH but `fj auth list` fails or returns * no output (mirrors today's existing preflight check's exact condition). * - "ok": `fj` is on PATH and `fj auth list` succeeds with output. */ export function detectFjState() { if (!commandAvailable("fj")) return "absent"; const fjAuth = run("fj", ["auth", "list"]); if (!ok(fjAuth) || (fjAuth.stdout ?? "").trim() === "") return "unauthenticated"; return "ok"; } ``` Notes: - `run()` and `ok()` are existing internal helpers in the file (`run` wraps `spawnSync`; `ok` checks `!result.error && result.status === 0`) — reuse them exactly as today's `fj auth list` preflight check does. - Both functions must be exported (`export function ...`) so they're directly importable by the test file, matching the existing export style used for `commandAvailable`-adjacent helpers like `isInsideManagedClone`. - Do not modify `preflight()` in this step — it still calls its own inline `fj auth list` / `rg` checks and hard-fails exactly as today. That refactor is a separate, dependent step. ### Test harness change needed for `detectFjState()` tests `scripts/local-install.test.mjs`'s `makeShimDir()` helper currently always writes an `fj` shim into the generated PATH directory (there is no way to omit it, unlike `withRg`/`withPi` which already support this). Add a `withFj = true` option: ```js function makeShimDir({ ..., withFj = true, // NEW ... } = {}) { ... if (withFj) { writeExecutableShim(dir, "fj", fjBody); } ... // extend the existing restricted-PATH condition: if (!withRg || !withPi || !withFj) { symlinkSync("/bin/sh", join(dir, "sh")); } } ``` When `withFj: false`, the caller must also restrict `PATH` to the shim dir alone (mirroring how `withRg: false` / `withPi: false` are handled today in `runPreflightAndCaptureAbort`), so `commandAvailable("fj")` actually returns `false` in the test rather than falling through to a real `fj` on the developer's machine. ## Acceptance Criteria - [ ] `rgAvailable()` is exported from `scripts/local-install.mjs` and returns `true`/`false` based on `commandAvailable("rg")`. - [ ] `detectFjState()` is exported from `scripts/local-install.mjs` and returns exactly one of `"absent"`, `"unauthenticated"`, or `"ok"`. - [ ] `makeShimDir()` in `scripts/local-install.test.mjs` supports a `withFj` option (default `true`) that, when `false`, omits the `fj` shim and is included in the PATH-restriction condition. - [ ] Unit test: `detectFjState()` returns `"absent"` when `fj` is not on PATH (via `withFj: false`). - [ ] Unit test: `detectFjState()` returns `"unauthenticated"` when `fj auth list` fails (via the existing `fjAuthFails: true` shim flag). - [ ] Unit test: `detectFjState()` returns `"unauthenticated"` when `fj auth list` returns no accounts (via the existing `fjAuthEmpty: true` shim flag). - [ ] Unit test: `detectFjState()` returns `"ok"` when `fj` is present and `fj auth list` succeeds with output (default shim). - [ ] Unit test: `rgAvailable()` returns `false` when `rg` is absent (via the existing `withRg: false` shim flag) and `true` by default. - [ ] `preflight()`'s existing behavior (hard-fail on missing rg/fj) is unchanged by this step — no existing tests should need to change. ## Test Plan - `npm test` (runs `node --test scripts/local-install.test.mjs`) — all existing tests still pass unmodified, plus the new detection-helper tests pass. - `node --test scripts/local-install.test.mjs` run directly — same result. - Manually verify `detectFjState()` returns `"ok"` on a machine with `fj` installed and authenticated, by temporarily adding a one-off `console.log(detectFjState())` call (or a throwaway test) and removing it before commit.

david added this to the Install Tool Detection: M1 - Detection helpers milestone

2026-09-08 00:00:27 +00:00

david added the

enhancement

label

2026-09-08 00:00:27 +00:00

david referenced this issue

2026-09-08 00:01:43 +00:00

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

david referenced this issue

2026-09-08 00:03:14 +00:00

Implement buildPackageFilters() with exhaustive unit tests #169