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

New issue

Closed

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

david commented

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

Owner

Summary

Replace scripts/local-install.mjs's hardcoded post-install reminder text with a dynamic summary that reflects which extensions/skills actually installed vs. were skipped, based on toolState. Also do a final cleanup pass confirming no dead fj/rg hard-fail code paths remain.

Background

scripts/local-install.mjs's reminder() currently prints a fixed string regardless of what was actually installed:

function reminder() {
  console.log("\nDone. Run /reload in pi to pick up the changes.");
  console.log("Verify: pi list  →  /skills (expect 12 skills, 5 extensions incl. grillme)");
}

Since this change (across the earlier steps in this milestone group) can now skip pr-comments, rg, and/or the forgejo-cli skill depending on detected tool availability, this hardcoded text is misleading whenever anything is skipped — telling the user to expect a full install when it wasn't. main() already threads toolState ({ rgOk, fjState }) through to reminder() from the preflight-refactor step (even before this step consumed it meaningfully).

Depends on: #168 (Refactor preflight() to skip/warn for missing fj/rg instead of hard-failing) — provides toolState and the main() → reminder(toolState) call site.

Implementation Details

Replace reminder():

const ALL_EXTENSIONS = ["rg", "postgres", "pr-comments", "forgejo", "grillme", "victorialogs"];
const ALL_SKILLS_COUNT = 13; // keep in sync with `ls skills | wc -l` at implementation time

function reminder(toolState = { rgOk: true, fjState: "ok" }) {
  const skippedExtensions = [];
  if (!toolState.rgOk) skippedExtensions.push("rg");
  if (toolState.fjState === "absent") skippedExtensions.push("pr-comments");
  const skippedSkillsCount = toolState.fjState === "absent" ? 1 : 0;

  const extensionCount = ALL_EXTENSIONS.length - skippedExtensions.length;
  const skillCount = ALL_SKILLS_COUNT - skippedSkillsCount;

  console.log("\nDone. Run /reload in pi to pick up the changes.");
  const skippedNote = skippedExtensions.length
    ? ` (skipped: ${skippedExtensions.join(", ")}${
        toolState.fjState === "absent" ? ", forgejo-cli skill" : ""
      })`
    : "";
  console.log(
    `Verify: pi list  →  /skills (expect ${skillCount} skills, ${extensionCount} ` +
      `extensions incl. grillme${skippedNote})`
  );
}

Notes:

ALL_SKILLS_COUNT must be verified against the actual current skill count at implementation time (ls skills | wc -l) rather than assumed — update the constant and its comment if the repo has gained/lost skills since this issue was drafted.
ALL_EXTENSIONS must be verified against package.json's pi.extensions array length/contents at implementation time.
Tests should assert on the presence of specific substrings (e.g. "skipped: rg", the computed skill/extension counts) rather than the entire string verbatim, so future copy tweaks don't require lockstep test rewrites — but the counts must be exactly correct.
toolState.fjState === "unauthenticated" must NOT reduce the reported counts or add a skip note — only "absent" (for fj) and !rgOk (for rg) count as skips.

Cleanup pass

Once this and the preceding milestones' steps are merged, do a final grep across scripts/local-install.mjs for:

Any remaining fail(...) call whose message mentions rg or fj — there should be none.
Any leftover unused imports/helpers from the old hard-fail implementation.

This is a verification pass, not expected to require code changes if the prior steps were implemented correctly — but call out explicitly in the PR description if anything was found and removed.

Acceptance Criteria

reminder({ rgOk: true, fjState: "ok" }) (or called with no arguments) prints the full skill/extension counts with no "skipped:" note.
reminder({ rgOk: false, fjState: "ok" }) prints one fewer extension than the full count, the same skill count, and a "skipped: rg" note.
reminder({ rgOk: true, fjState: "absent" }) prints one fewer extension, one fewer skill, and a note mentioning both "pr-comments" and "forgejo-cli".
reminder({ rgOk: false, fjState: "absent" }) prints two fewer extensions, one fewer skill, and a note mentioning "rg", "pr-comments", and "forgejo-cli".
reminder({ rgOk: true, fjState: "unauthenticated" }) prints the same output as the fully-available case (no skip note, full counts) — unauthenticated must not affect the reminder.
A repo-wide check confirms no fail(...) call remains referencing rg or fj in scripts/local-install.mjs.
main()'s call site (reminder(toolState)) passes the real toolState object, not a stubbed/default value, on every invocation path.

Test Plan

Unit test each of the five reminder() scenarios in the acceptance criteria, capturing console.log output and asserting on count and substring presence.
grep -n "fail(" scripts/local-install.mjs (or equivalent) manually reviewed to confirm no rg/fj-referencing hard-fail call remains — note the result in the PR description.
npm test — all green, including the updated "direct invocation exits 0 and prints the Step-4 reminder on the happy path" end-to-end test (its exact reminder-text assertion will need updating to match the new dynamic format — same meaning, i.e. full counts, but the literal string differs from the old hardcoded text).

## Summary Replace `scripts/local-install.mjs`'s hardcoded post-install reminder text with a dynamic summary that reflects which extensions/skills actually installed vs. were skipped, based on `toolState`. Also do a final cleanup pass confirming no dead `fj`/`rg` hard-fail code paths remain. ## Background `scripts/local-install.mjs`'s `reminder()` currently prints a fixed string regardless of what was actually installed: ```js function reminder() { console.log("\nDone. Run /reload in pi to pick up the changes."); console.log("Verify: pi list → /skills (expect 12 skills, 5 extensions incl. grillme)"); } ``` Since this change (across the earlier steps in this milestone group) can now skip `pr-comments`, `rg`, and/or the `forgejo-cli` skill depending on detected tool availability, this hardcoded text is misleading whenever anything is skipped — telling the user to expect a full install when it wasn't. `main()` already threads `toolState` (`{ rgOk, fjState }`) through to `reminder()` from the preflight-refactor step (even before this step consumed it meaningfully). **Depends on:** #168 (Refactor preflight() to skip/warn for missing fj/rg instead of hard-failing) — provides `toolState` and the `main()` → `reminder(toolState)` call site. ## Implementation Details Replace `reminder()`: ```js const ALL_EXTENSIONS = ["rg", "postgres", "pr-comments", "forgejo", "grillme", "victorialogs"]; const ALL_SKILLS_COUNT = 13; // keep in sync with `ls skills | wc -l` at implementation time function reminder(toolState = { rgOk: true, fjState: "ok" }) { const skippedExtensions = []; if (!toolState.rgOk) skippedExtensions.push("rg"); if (toolState.fjState === "absent") skippedExtensions.push("pr-comments"); const skippedSkillsCount = toolState.fjState === "absent" ? 1 : 0; const extensionCount = ALL_EXTENSIONS.length - skippedExtensions.length; const skillCount = ALL_SKILLS_COUNT - skippedSkillsCount; console.log("\nDone. Run /reload in pi to pick up the changes."); const skippedNote = skippedExtensions.length ? ` (skipped: ${skippedExtensions.join(", ")}${ toolState.fjState === "absent" ? ", forgejo-cli skill" : "" })` : ""; console.log( `Verify: pi list → /skills (expect ${skillCount} skills, ${extensionCount} ` + `extensions incl. grillme${skippedNote})` ); } ``` Notes: - `ALL_SKILLS_COUNT` must be verified against the actual current skill count at implementation time (`ls skills | wc -l`) rather than assumed — update the constant and its comment if the repo has gained/lost skills since this issue was drafted. - `ALL_EXTENSIONS` must be verified against `package.json`'s `pi.extensions` array length/contents at implementation time. - Tests should assert on the presence of specific substrings (e.g. `"skipped: rg"`, the computed skill/extension counts) rather than the entire string verbatim, so future copy tweaks don't require lockstep test rewrites — but the *counts* must be exactly correct. - `toolState.fjState === "unauthenticated"` must NOT reduce the reported counts or add a skip note — only `"absent"` (for fj) and `!rgOk` (for rg) count as skips. ### Cleanup pass Once this and the preceding milestones' steps are merged, do a final grep across `scripts/local-install.mjs` for: - Any remaining `fail(...)` call whose message mentions `rg` or `fj` — there should be none. - Any leftover unused imports/helpers from the old hard-fail implementation. This is a verification pass, not expected to require code changes if the prior steps were implemented correctly — but call out explicitly in the PR description if anything was found and removed. ## Acceptance Criteria - [ ] `reminder({ rgOk: true, fjState: "ok" })` (or called with no arguments) prints the full skill/extension counts with no "skipped:" note. - [ ] `reminder({ rgOk: false, fjState: "ok" })` prints one fewer extension than the full count, the same skill count, and a "skipped: rg" note. - [ ] `reminder({ rgOk: true, fjState: "absent" })` prints one fewer extension, one fewer skill, and a note mentioning both "pr-comments" and "forgejo-cli". - [ ] `reminder({ rgOk: false, fjState: "absent" })` prints two fewer extensions, one fewer skill, and a note mentioning "rg", "pr-comments", and "forgejo-cli". - [ ] `reminder({ rgOk: true, fjState: "unauthenticated" })` prints the same output as the fully-available case (no skip note, full counts) — unauthenticated must not affect the reminder. - [ ] A repo-wide check confirms no `fail(...)` call remains referencing `rg` or `fj` in `scripts/local-install.mjs`. - [ ] `main()`'s call site (`reminder(toolState)`) passes the real `toolState` object, not a stubbed/default value, on every invocation path. ## Test Plan - Unit test each of the five `reminder()` scenarios in the acceptance criteria, capturing `console.log` output and asserting on count and substring presence. - `grep -n "fail(" scripts/local-install.mjs` (or equivalent) manually reviewed to confirm no `rg`/`fj`-referencing hard-fail call remains — note the result in the PR description. - `npm test` — all green, including the updated `"direct invocation exits 0 and prints the Step-4 reminder on the happy path"` end-to-end test (its exact reminder-text assertion will need updating to match the new dynamic format — same meaning, i.e. full counts, but the literal string differs from the old hardcoded text).

david added this to the Install Tool Detection: M4 - Dynamic reminder & cleanup milestone

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

david added the

enhancement

label

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

david referenced this issue

2026-09-08 00:05:18 +00:00

Add end-to-end main() tests for the fj/rg tool-state matrix #172

david referenced this issue

2026-09-08 03:05:36 +00:00

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