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

New issue

Closed

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

david commented

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

Owner

Summary

Change scripts/local-install.mjs's preflight() so a missing rg binary or an unusable fj no longer abort the whole install. Instead, preflight() prints an appropriate message and returns a tool-state object that later steps use to decide what to skip.

Background

scripts/local-install.mjs's preflight() currently contains two hard-fail blocks:

if (!commandAvailable("rg")) {
  fail("rg binary not found. Install ripgrep, e.g. `sudo apt install ripgrep` (needed by the rg extension).");
}
...
const fjAuth = run("fj", ["auth", "list"]);
if (!ok(fjAuth) || (fjAuth.stdout ?? "").trim() === "") {
  fail("fj CLI missing or not authenticated. Install fj and log in: `fj auth login` (needed by the pr-comments extension).");
}

fail(message) prints [install:local] Aborted: <message> and calls process.exit(1), terminating the entire install — including unrelated extensions like postgres, forgejo, grillme, and victorialogs that have nothing to do with rg or fj.

The desired behavior (per the agreed plan):

Condition	New behavior
`rg` absent from PATH	Skip the `rg` extension only; print a skip message; do not abort
`fj` absent from PATH	Skip the `pr-comments` extension and `forgejo-cli` skill only; print a skip message; do not abort
`fj` present but `fj auth list` fails/empty (unauthenticated)	Warn only; still install `pr-comments` + `forgejo-cli` normally; do not abort
`fj` present and authenticated	No warning, no skip (unchanged)

This does not touch the extensions/forgejo extension (only needs FORGEJO_TOKEN, already warn-only and untouched) or any other extension/skill.

Depends on: #167 (Add fj/rg tool-detection helpers (rgAvailable, detectFjState) with unit tests) — this step consumes rgAvailable() and detectFjState() from that step.

Implementation Details

Remove the two hard-fail blocks shown above from preflight(). Replace them with non-aborting branches using the detection helpers, and have preflight() return a { rgOk, fjState } object:

export function preflight(cwd) {
  const env = process.env;

  // ...unchanged hard-fail checks: pi CLI, managed-clone, git-checkout,
  // origin match, branch === main, clean working tree...

  const rgOk = rgAvailable();
  if (!rgOk) {
    console.error(
      "[install:local] rg binary not found on PATH — skipping the rg extension. " +
        "Install ripgrep, e.g. `sudo apt install ripgrep`, and re-run " +
        "`npm run install:local` to include it."
    );
  }

  // FORGEJO_TOKEN check stays exactly as-is (unrelated, already warn-only):
  if (!hasConfigValue("FORGEJO_TOKEN", env)) {
    console.error(/* existing unchanged warning */);
  }

  const fjState = detectFjState();
  if (fjState === "absent") {
    console.error(
      "[install:local] fj CLI not found on PATH — skipping the pr-comments extension " +
        "and the forgejo-cli skill (both require fj). Install fj and re-run " +
        "`npm run install:local` to include them."
    );
  } else if (fjState === "unauthenticated") {
    console.error(
      "[install:local] Warning: fj CLI is installed but not authenticated " +
        "(`fj auth list` failed). pr-comments and forgejo-cli will still install, but " +
        "fj-based operations will fail until you run `fj auth login`."
    );
  }

  return { rgOk, fjState };
}

Update main() to capture and use the return value:

export function main(cwd = process.cwd(), options = {}) {
  ...
  const toolState = preflight(cwd);
  console.log("[install:local] Preflight checks passed.");
  ...
  dedupeAndRegister(cwd, { ...options, toolState }); // toolState threading only —
                                                      // actual filter logic is a
                                                      // separate, dependent step
  reminder(toolState); // dynamic reminder is a separate, dependent step;
                        // for now just pass toolState through, even if reminder()
                        // doesn't yet use it (avoid breaking the call signature twice)
}

Note: this issue only changes preflight()'s control flow and main()'s threading of the return value. It does NOT implement the settings.json filter rewrite in dedupeAndRegister() or the dynamic reminder() output — those are separate, dependent steps that will consume toolState. If dedupeAndRegister() and reminder() don't yet accept a second/toolState argument, add the parameter now as an unused pass-through so this step's main() change doesn't break, and leave a // TODO: consumed by <next milestone> comment.

Acceptance Criteria

preflight() no longer calls fail() for a missing rg binary.
preflight() no longer calls fail() for a missing or unauthenticated fj.
When rg is absent, preflight() prints a message containing "rg binary not found on PATH" and "skipping the rg extension", and does not abort.
When fj is absent, preflight() prints a message containing "fj CLI not found on PATH" and mentions both "pr-comments" and "forgejo-cli", and does not abort.
When fj is present but unauthenticated, preflight() prints a message containing "not authenticated" and "fj auth login", and does not abort — and does NOT print the "skipping" message.
When both fj and rg are fully available, preflight() prints neither warning and returns { rgOk: true, fjState: "ok" }.
preflight() returns { rgOk, fjState } matching the detected state in all cases.
All pre-existing hard-fail checks (pi CLI, managed clone, git checkout, origin, branch, dirty tree) are unchanged and still abort exactly as before.
main() captures preflight()'s return value and passes it through to dedupeAndRegister() and reminder() (even if those functions don't yet use it meaningfully).

Test Plan

Replace the existing test "preflight aborts when rg is missing" with a test asserting aborted === false, the new message substrings appear in stderr, and the returned rgOk === false.
Replace the existing tests "preflight aborts when fj auth fails" and "preflight aborts when fj reports no authenticated accounts" with tests asserting aborted === false, the new unauthenticated-warning substrings appear, and fjState === "unauthenticated".
Add a new test: fj absent (via withFj: false) → aborted === false, skip message present, fjState === "absent".
Add a new test: both fj and rg absent simultaneously → aborted === false, both skip messages present, { rgOk: false, fjState: "absent" }.
Add a new test: both tools fully available → aborted === false, no fj/rg-related warnings in stderr, { rgOk: true, fjState: "ok" }.
Confirm existing tests for the untouched hard-fail checks (pi missing, managed clone, wrong repo/branch, dirty tree, FORGEJO_TOKEN warn-only) all still pass unmodified.
npm test — all green.

## Summary Change `scripts/local-install.mjs`'s `preflight()` so a missing `rg` binary or an unusable `fj` no longer abort the whole install. Instead, `preflight()` prints an appropriate message and returns a tool-state object that later steps use to decide what to skip. ## Background `scripts/local-install.mjs`'s `preflight()` currently contains two hard-fail blocks: ```js if (!commandAvailable("rg")) { fail("rg binary not found. Install ripgrep, e.g. `sudo apt install ripgrep` (needed by the rg extension)."); } ... const fjAuth = run("fj", ["auth", "list"]); if (!ok(fjAuth) || (fjAuth.stdout ?? "").trim() === "") { fail("fj CLI missing or not authenticated. Install fj and log in: `fj auth login` (needed by the pr-comments extension)."); } ``` `fail(message)` prints `[install:local] Aborted: <message>` and calls `process.exit(1)`, terminating the entire install — including unrelated extensions like `postgres`, `forgejo`, `grillme`, and `victorialogs` that have nothing to do with `rg` or `fj`. The desired behavior (per the agreed plan): | Condition | New behavior | |---|---| | `rg` absent from PATH | Skip the `rg` extension only; print a skip message; do not abort | | `fj` absent from PATH | Skip the `pr-comments` extension and `forgejo-cli` skill only; print a skip message; do not abort | | `fj` present but `fj auth list` fails/empty (unauthenticated) | Warn only; still install `pr-comments` + `forgejo-cli` normally; do not abort | | `fj` present and authenticated | No warning, no skip (unchanged) | This does not touch the `extensions/forgejo` extension (only needs `FORGEJO_TOKEN`, already warn-only and untouched) or any other extension/skill. **Depends on:** #167 (Add fj/rg tool-detection helpers (rgAvailable, detectFjState) with unit tests) — this step consumes `rgAvailable()` and `detectFjState()` from that step. ## Implementation Details Remove the two hard-fail blocks shown above from `preflight()`. Replace them with non-aborting branches using the detection helpers, and have `preflight()` return a `{ rgOk, fjState }` object: ```js export function preflight(cwd) { const env = process.env; // ...unchanged hard-fail checks: pi CLI, managed-clone, git-checkout, // origin match, branch === main, clean working tree... const rgOk = rgAvailable(); if (!rgOk) { console.error( "[install:local] rg binary not found on PATH — skipping the rg extension. " + "Install ripgrep, e.g. `sudo apt install ripgrep`, and re-run " + "`npm run install:local` to include it." ); } // FORGEJO_TOKEN check stays exactly as-is (unrelated, already warn-only): if (!hasConfigValue("FORGEJO_TOKEN", env)) { console.error(/* existing unchanged warning */); } const fjState = detectFjState(); if (fjState === "absent") { console.error( "[install:local] fj CLI not found on PATH — skipping the pr-comments extension " + "and the forgejo-cli skill (both require fj). Install fj and re-run " + "`npm run install:local` to include them." ); } else if (fjState === "unauthenticated") { console.error( "[install:local] Warning: fj CLI is installed but not authenticated " + "(`fj auth list` failed). pr-comments and forgejo-cli will still install, but " + "fj-based operations will fail until you run `fj auth login`." ); } return { rgOk, fjState }; } ``` Update `main()` to capture and use the return value: ```js export function main(cwd = process.cwd(), options = {}) { ... const toolState = preflight(cwd); console.log("[install:local] Preflight checks passed."); ... dedupeAndRegister(cwd, { ...options, toolState }); // toolState threading only — // actual filter logic is a // separate, dependent step reminder(toolState); // dynamic reminder is a separate, dependent step; // for now just pass toolState through, even if reminder() // doesn't yet use it (avoid breaking the call signature twice) } ``` Note: this issue only changes `preflight()`'s control flow and `main()`'s threading of the return value. It does NOT implement the settings.json filter rewrite in `dedupeAndRegister()` or the dynamic `reminder()` output — those are separate, dependent steps that will consume `toolState`. If `dedupeAndRegister()` and `reminder()` don't yet accept a second/`toolState` argument, add the parameter now as an unused pass-through so this step's `main()` change doesn't break, and leave a `// TODO: consumed by <next milestone>` comment. ## Acceptance Criteria - [ ] `preflight()` no longer calls `fail()` for a missing `rg` binary. - [ ] `preflight()` no longer calls `fail()` for a missing or unauthenticated `fj`. - [ ] When `rg` is absent, `preflight()` prints a message containing "rg binary not found on PATH" and "skipping the rg extension", and does not abort. - [ ] When `fj` is absent, `preflight()` prints a message containing "fj CLI not found on PATH" and mentions both "pr-comments" and "forgejo-cli", and does not abort. - [ ] When `fj` is present but unauthenticated, `preflight()` prints a message containing "not authenticated" and "fj auth login", and does not abort — and does NOT print the "skipping" message. - [ ] When both `fj` and `rg` are fully available, `preflight()` prints neither warning and returns `{ rgOk: true, fjState: "ok" }`. - [ ] `preflight()` returns `{ rgOk, fjState }` matching the detected state in all cases. - [ ] All pre-existing hard-fail checks (pi CLI, managed clone, git checkout, origin, branch, dirty tree) are unchanged and still abort exactly as before. - [ ] `main()` captures `preflight()`'s return value and passes it through to `dedupeAndRegister()` and `reminder()` (even if those functions don't yet use it meaningfully). ## Test Plan - Replace the existing test `"preflight aborts when rg is missing"` with a test asserting `aborted === false`, the new message substrings appear in stderr, and the returned `rgOk === false`. - Replace the existing tests `"preflight aborts when fj auth fails"` and `"preflight aborts when fj reports no authenticated accounts"` with tests asserting `aborted === false`, the new unauthenticated-warning substrings appear, and `fjState === "unauthenticated"`. - Add a new test: fj absent (via `withFj: false`) → `aborted === false`, skip message present, `fjState === "absent"`. - Add a new test: both `fj` and `rg` absent simultaneously → `aborted === false`, both skip messages present, `{ rgOk: false, fjState: "absent" }`. - Add a new test: both tools fully available → `aborted === false`, no fj/rg-related warnings in stderr, `{ rgOk: true, fjState: "ok" }`. - Confirm existing tests for the untouched hard-fail checks (pi missing, managed clone, wrong repo/branch, dirty tree, `FORGEJO_TOKEN` warn-only) all still pass unmodified. - `npm test` — all green.

david added this to the Install Tool Detection: M2 - Preflight refactor milestone

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

david added the

enhancement

label

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

david referenced this issue

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

Implement buildPackageFilters() with exhaustive unit tests #169

david referenced this issue

2026-09-08 00:04:13 +00:00

Make reminder() report dynamic install/skip counts; remove dead fj/rg fail() code #171