fix(cli): suppress OpenSkills fallback when Tier-1 already has the skill

[naorsabag]

Summary

• Suppresses the OpenSkills fallback hint introduced in feat(cli): OpenSkills fallback when no Tier-1 client detected #116 when a Tier-1 client is detected and already has the skill (the re-run case).
• Extracts the gate into a pure shouldPrintOpenSkillsFallback(results) predicate; 5 new direct unit tests cover the matrix.
• Addresses CodeRabbit's review comment on feat(cli): OpenSkills fallback when no Tier-1 client detected #116.

Why

With the merged gate installed === 0 && wouldInstall === 0, a user re-running init after a prior successful install saw self-contradicting output:

claude-code  skipped  already installed
...
No Tier-1 client received the skill. For Codex CLI / Gemini CLI / ...

The skill is on their machine — just not from this invocation.

Behavior matrix

Scenario	Hint fires?
No Tier-1 client detected at all	✅ yes
Only advisory (Continue.dev) detected	✅ yes
Re-run: Tier-1 already installed	❌ no
Fresh install: at least one Tier-1 installed	❌ no
Dry-run: at least one Tier-1 would install	❌ no
Failed Tier-1 install	n/a (early-exits with EXIT_CONFLICT before this check)

Test plan

• npx vitest run __tests__/init.test.ts → 25/25 pass (20 prior + 5 new on the predicate).
• Smoke: pre-seeded $HOME/.claude/skills/openhop/ → claude-code skipped (already installed) and no fallback line.
• Smoke: empty $HOME → fallback line still printed; exit 4.
• CI green on this PR.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved the init command to more accurately determine when to display the OpenSkills fallback hint, ensuring it only appears when no Tier-1 client receives a skill installation.

• Tests

  • Added comprehensive test coverage for fallback message display logic across detection, dry-run, and reinstall scenarios.

[coderabbitai]

Walkthrough

This PR extracts the logic for determining when to print the OpenSkills fallback message into a reusable, exported predicate function. The new shouldPrintOpenSkillsFallback function evaluates installation results and returns whether any Tier-1 client received the skill, accounting for advisory results and detection skips. The init command is updated to use this predicate, replacing earlier inline condition logic.

Changes

OpenSkills Fallback Predicate

Layer / File(s)	Summary
Predicate Implementation packages/cli/src/init.ts	Adds exported shouldPrintOpenSkillsFallback(results) function that returns true when no Tier-1 client received the skill, treating non-advisory results as Tier-1 receipts and excluding "skipped: not detected" from counting as a receipt.
Integration with registerInit packages/cli/src/init.ts	Updates the non-JSON fallback-hint condition in registerInit to call shouldPrintOpenSkillsFallback(results) instead of checking installed/would-install counts.
Tests and Export packages/cli/__tests__/init.test.ts	Imports the new predicate and adds unit tests validating that fallback prints when no Tier-1 clients are detected or only an advisory client is detected, and does not print when Tier-1 is already installed, installed during the run, or would-install on dry-run.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• naorsabag/openhop#116: Implements the original OpenSkills fallback message logic that this PR refactors by extracting the decision predicate into a testable function.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: suppressing the OpenSkills fallback message when a Tier-1 client already has the skill, which is the primary fix addressed in this pull request.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/init-fallback-suppress-on-reinstall

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/cli/src/init.ts (1)

273-276: ⚡ Quick win

Decouple fallback gating from user-facing reason text.

Line 275 keys behavior off the literal 'not detected'; wording changes can silently break fallback decisions. Prefer a shared internal reason constant/code for machine checks.

Proposed refactor

+const REASON_NOT_DETECTED = 'not detected' as const
+
 export function runInit(
   opts: InitOptions,
   clients: ClientSpec[],
   sourceSkill: string | null,
   fs: FsLike = realFs
 ): { results: InstallResult[]; sourceMissing: boolean } {
@@
     if (!fs.existsSync(c.detectDir())) {
-      results.push({ client: c.id, status: 'skipped', reason: 'not detected' })
+      results.push({ client: c.id, status: 'skipped', reason: REASON_NOT_DETECTED })
       continue
     }
@@
 export function shouldPrintOpenSkillsFallback(results: InstallResult[]): boolean {
   const hasTier1ReceivedSkill = results.some(
-    (r) => r.status !== 'advisory' && !(r.status === 'skipped' && r.reason === 'not detected')
+    (r) =>
+      r.status !== 'advisory' &&
+      !(r.status === 'skipped' && r.reason === REASON_NOT_DETECTED)
   )
   return !hasTier1ReceivedSkill
 }
@@
 const detectedCount = results.filter(
-  (r) => !(r.status === 'skipped' && r.reason === 'not detected')
+  (r) => !(r.status === 'skipped' && r.reason === REASON_NOT_DETECTED)
 ).length

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/cli/src/init.ts` around lines 273 - 276, The fallback gating in
shouldPrintOpenSkillsFallback uses the literal string 'not detected' from
InstallResult.reason which couples logic to user-facing text; introduce a
machine-only enum or constant (e.g., InternalSkipReason { NotDetected =
'NOT_DETECTED' }) and update shouldPrintOpenSkillsFallback to check r.reason ===
InternalSkipReason.NotDetected (or the new constant) instead of the literal, and
then change all code sites that set InstallResult.reason for machine decisions
(e.g., where skips are created) to assign the new internal code while keeping or
mapping to the existing user-facing message for display.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/cli/src/init.ts`:
- Around line 273-276: The fallback gating in shouldPrintOpenSkillsFallback uses
the literal string 'not detected' from InstallResult.reason which couples logic
to user-facing text; introduce a machine-only enum or constant (e.g.,
InternalSkipReason { NotDetected = 'NOT_DETECTED' }) and update
shouldPrintOpenSkillsFallback to check r.reason ===
InternalSkipReason.NotDetected (or the new constant) instead of the literal, and
then change all code sites that set InstallResult.reason for machine decisions
(e.g., where skips are created) to assign the new internal code while keeping or
mapping to the existing user-facing message for display.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ed71e01a-ccf3-4239-a13d-fda17ed4153c

📥 Commits

Reviewing files that changed from the base of the PR and between f12e7d9 and 2cfd5bc.

📒 Files selected for processing (2)

• packages/cli/__tests__/init.test.ts
• packages/cli/src/init.ts