Add "lessons/patterns/startup-hooks-must-check-venv-cache-path-first"
@@ -0,0 +1,96 @@
|
||||
# Startup Hooks Must Check Venv Cache Path First
|
||||
|
||||
## Context
|
||||
- **Date:** 2026-02-02
|
||||
- **Time Wasted:** 4+ hours debugging across 3 devices
|
||||
- **Severity:** Critical - blocks all MCP functionality
|
||||
- **Components:** SessionStart hooks, MCP server venvs
|
||||
|
||||
## Problem
|
||||
|
||||
SessionStart hooks in `data-platform` and `pr-review` plugins were reporting "MCP venv missing" even though venvs existed and MCP servers worked fine.
|
||||
|
||||
The hooks checked the WRONG path:
|
||||
```bash
|
||||
# WRONG - checks marketplace directory (gets wiped on updates)
|
||||
VENV_PATH="$MARKETPLACE_ROOT/mcp-servers/data-platform/.venv/bin/python"
|
||||
```
|
||||
|
||||
But venvs actually live in the CACHE directory (survives updates):
|
||||
```bash
|
||||
# CORRECT - cache location
|
||||
~/.cache/claude-mcp-venvs/leo-claude-mktplace/{server}/.venv/
|
||||
```
|
||||
|
||||
## Root Cause
|
||||
|
||||
The `run.sh` scripts correctly check cache first, then local. But the startup hooks were written later and copied the wrong pattern - checking only the marketplace path.
|
||||
|
||||
**Inconsistency between:**
|
||||
- `mcp-servers/*/run.sh` - checks cache first ✓
|
||||
- `plugins/*/hooks/startup-check.sh` - only checked marketplace ✗
|
||||
|
||||
## Solution
|
||||
|
||||
Update startup hooks to match run.sh pattern:
|
||||
|
||||
```bash
|
||||
# Check cache first (preferred), then local
|
||||
CACHE_VENV="$HOME/.cache/claude-mcp-venvs/leo-claude-mktplace/{server}/.venv/bin/python"
|
||||
LOCAL_VENV="$MARKETPLACE_ROOT/mcp-servers/{server}/.venv/bin/python"
|
||||
|
||||
if [[ -f "$CACHE_VENV" ]]; then
|
||||
VENV_PATH="$CACHE_VENV"
|
||||
elif [[ -f "$LOCAL_VENV" ]]; then
|
||||
VENV_PATH="$LOCAL_VENV"
|
||||
else
|
||||
echo "$PREFIX MCP venv missing - run /initial-setup or setup.sh"
|
||||
exit 0
|
||||
fi
|
||||
```
|
||||
|
||||
## Prevention
|
||||
|
||||
### 1. Canonical Venv Path Pattern
|
||||
Add to `docs/CANONICAL-PATHS.md`:
|
||||
```
|
||||
MCP Server Venvs (ALWAYS check in this order):
|
||||
1. Cache: ~/.cache/claude-mcp-venvs/leo-claude-mktplace/{server}/.venv/
|
||||
2. Local: {marketplace}/mcp-servers/{server}/.venv/
|
||||
```
|
||||
|
||||
### 2. Pre-Commit Check
|
||||
Add validation script that ensures all hooks checking for venvs use the cache-first pattern.
|
||||
|
||||
### 3. Code Review Checklist
|
||||
When reviewing hooks that check venv paths:
|
||||
- [ ] Does it check cache path FIRST?
|
||||
- [ ] Does it match the pattern in run.sh?
|
||||
- [ ] Is the server name correct?
|
||||
|
||||
## Files Changed
|
||||
|
||||
1. `plugins/data-platform/hooks/startup-check.sh` - Fixed path checking
|
||||
2. `plugins/pr-review/hooks/startup-check.sh` - Fixed path checking
|
||||
|
||||
## Why This Matters
|
||||
|
||||
- Venvs in marketplace get DELETED on every update/reinstall
|
||||
- Cache directory SURVIVES updates - that's its entire purpose
|
||||
- False "venv missing" errors cause hours of debugging
|
||||
- User wastes time running setup scripts that don't fix anything
|
||||
|
||||
## Tags for Discovery
|
||||
|
||||
Search terms that should find this lesson:
|
||||
- "venv missing"
|
||||
- "MCP venv"
|
||||
- "startup hook"
|
||||
- "SessionStart"
|
||||
- "cache path"
|
||||
- "marketplace path"
|
||||
- "run.sh"
|
||||
|
||||
|
||||
---
|
||||
**Tags:** venv, mcp, hooks, startup, cache, debugging, critical, path-mismatch
|
||||
Reference in New Issue
Block a user