#21116: fix(skills): don't cache negative binary lookups

Fixes #21094 ## Summary - **Problem**: `hasBinary()` cached negative results (binary not found), which became stale after `brew install` succeeded - **Why it matters**: Users saw "Install" button persist even after successful installation, requiring manual cache clear or restart - **What changed**: Removed `hasBinaryCache.set(bin, false)` — only positive results are cached now - **What did NOT change**: Positive result caching unchanged; function signature unchanged; no config/API changes ## Change Type (select all) - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Docs - [ ] Security hardening - [ ] Chore/infra ## Scope (select all touched areas) - [ ] Gateway / orchestration - [x] Skills / tool execution - [ ] Auth / tokens - [ ] Memory / storage - [ ] Integrations - [ ] API / contracts - [x] UI / DX - [ ] CI/CD / infra ## Linked Issue/PR - Closes #21094 - Related: N/A ## User-visible / Behavior Changes **Before**: After installing a skill's binary via dashboard, refreshing the page showed "Install" button again (stale cache). **After**: Dashboard immediately reflects actual binary state after installation. No restart or cache clear needed. **Impact**: Skills dashboard now correctly shows "eligible" status after brew installs. ## Security Impact (required) - New permissions/capabilities? **No** - Secrets/tokens handling changed? **No** - New/changed network calls? **No** - Command/tool execution surface changed? **No** - Data access scope changed? **No** ## Repro + Verification ### Environment - OS: macOS 26.2 (Apple Silicon - arm64) - Runtime/container: Homebrew at `/opt/homebrew/`, Node v22.14.0 - Model/provider: N/A (skill installation bug) - Integration/channel: Skills dashboard - Relevant config: Default skill installation with brew prerequisites ### Steps 1. Open Skills dashboard 2. Find skill requiring brew binary (e.g., `wacli`, `gog`, `gifgrep`) 3. Observe "Install [skill] (brew)" button with status "blocked - Missing: bin:[name]" 4. Click install button → observe "Installed" status 5. Refresh page 6. **Before fix**: Shows "Install" button again ❌ 7. **After fix**: Shows skill as "eligible" ✅ ### Expected After successful brew install, dashboard refresh should show skill as eligible/installed. ### Actual Dashboard showed "Install" button again because `hasBinary()` returned stale cached `false`. ## Evidence - [x] Code logic verified (one-line change) - [x] Root cause identified in issue description - [x] Fix aligns with suggested solution in #21094 **Code change**: ```diff - hasBinaryCache.set(bin, false); + // Don't cache negative results - they become stale after brew install ``` ## Human Verification (required) **Verified scenarios**: - Code compiles without errors (`pnpm format` passed on changed file) - Logic review: positive lookups cached (fast), negative lookups not cached (correct after install) - One-line change with clear rationale **Edge cases checked**: - Positive results still cached (performance maintained) - PATH change still triggers cache clear (existing behavior preserved) - Windows PATHEXT handling untouched **What I did NOT verify**: - Full end-to-end dashboard test with actual brew install (requires production setup) - Load testing for performance impact (negligible - see Risks section) ## Compatibility / Migration - Backward compatible? **Yes** - Config/env changes? **No** - Migration needed? **No** No breaking changes. Existing cached positive results remain valid. Negative results simply aren't cached anymore. ## Failure Recovery (if this breaks) **How to disable/revert**: ```bash git revert <this-commit> # or restore line: hasBinaryCache.set(bin, false); ``` **Files to restore**: `src/shared/config-eval.ts` (line 147) **Known bad symptoms**: - Dashboard shows "Install" button after successful installation (same as before) - Users would need to restart gateway to clear cache (original issue) ## Risks and Mitigations **Risk**: Slight performance overhead from re-checking missing binaries on each `skills.status` call - **Mitigation**: - Negative lookups are NOT the hot path (binaries either exist or are rarely checked) - Cost is a few `fs.accessSync` calls across PATH dirs (~microseconds) - Positive lookups remain cached (hot path unaffected) - Trade-off: microseconds of overhead vs. always-correct dashboard state **Risk**: None (no other risks identified) <!-- greptile_comment --> <h3>Greptile Summary</h3> Removed caching of negative binary lookup results in `hasBinary()`. Previously, when a binary wasn't found, the function cached `false`, which became stale after successful installation via `brew install`. Now only positive results (binary found) are cached, ensuring the skills dashboard immediately reflects actual binary availability after installation. <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with no risk - Single-line fix with clear rationale addressing a specific bug. The change preserves performance for the hot path (positive lookups remain cached) while fixing stale cache issues for negative lookups. Cache invalidation logic on PATH changes remains intact. No breaking changes to function signature or behavior. - No files require special attention _{Last reviewed commit: 11642bf} <!-- greptile_other_comments_section --> _{(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!} <!-- /greptile_comment -->