diff --git a/.STATUS b/.STATUS index 9117e1caf..98d5b1b5c 100644 --- a/.STATUS +++ b/.STATUS @@ -4,7 +4,7 @@ ## Project: flow-cli ## Type: zsh-plugin ## Status: active -## Focus: post-release +## Focus: --help ## Phase: Released ## Priority: 2 ## Progress: 100 @@ -12,11 +12,22 @@ ## Current Session (2026-05-13) **Session activity:** +- feat(doctor): wired lib/doctor-cache.zsh into legacy GitHub token validation — fingerprint cache key (sha256 prefix of token, 1h TTL), `--no-cache` flag, JSON envelope `{http_code, username}` +- refactor(doctor): extracted `_doctor_check_github_token(no_cache)` helper; doctor() now dispatches via one line; helper is independently testable +- test isolation: `DOCTOR_CACHE_DIR` exported pre-source (cache lib marks it readonly post-source) +- tests: +4 functions calling helper directly (not full doctor) — cuts test runtime by ~14s +- empirical timing: cold flow doctor 11.3s → warm 10.8s (~5%, smaller than orchestrate estimate because `_dots_doctor_integration` was already provider-cached) +- result: 53 test files pass, 1 expected interactive timeout; test-doctor 25s standalone, exit 0 under `timeout 30` +- discovered (filed): test-framework `create_mock` save/restore is broken for binaries — `tail -n +2` strips opening `{` but keeps closing `}`, eval parse error on second mock +- discovered (filed): `_doctor_cache_acquire_lock` mkdir-fallback leaks state across in-process invocations; cache_set rc=1 on second call with same key + +**Correction (2026-05-13):** Commit `0880f924` (first push of this branch) introduced a test-doctor timeout regression that the commit message and .STATUS both misreported as "52/52 passing". Run-all.sh actually reported "2 timeout" — only one (e2e-em-dispatcher) is documented as expected; the other was test-doctor running ~40s standalone under the 30s `timeout` wrapper in `tests/run-all.sh`. Caught during the optional Step 4 follow-up review when re-checking `time timeout 30 zsh tests/test-doctor.zsh`. Fixed in the follow-up refactor commit by extracting `_doctor_check_github_token` so cache tests skip the slow system-check section. + +**Previous Session (2026-05-13 earlier):** - fix(tests): test-doctor timeout under run-all.sh — cached `doctor --verbose` output in setup() and reused for both `--verbose` and `-v` tests (which test identical output since `-v` aliases `--verbose` per commands/doctor.zsh:39) -- root cause: 3 redundant `$(doctor ...)` calls in test, each hitting `curl https://api.github.com/user` (commands/doctor.zsh:405) for ~5-8s of network wait -- timing: 38.86s standalone → 23.51s wrapped (40% reduction), 6.5s headroom under 30s ceiling +- root cause: 3 redundant `$(doctor ...)` calls in test, each hitting `curl https://api.github.com/user` for ~5-8s of network wait +- timing: 38.86s standalone → 23.51s wrapped (40% reduction) - result: 23/23 passing, exit 0 under `timeout 30 zsh ...` -- filed pending: commands/doctor.zsh:405 bypasses lib/doctor-cache.zsh ## Previous Session (2026-03-11) @@ -241,12 +252,26 @@ - Current coverage: ~50% (348 functions documented) - Target: 80% -### Doctor command bypasses its own cache (filed 2026-05-13) -- `lib/doctor-cache.zsh` (25KB) provides a file-based cache at `~/.flow/cache/doctor/` -- `commands/doctor.zsh:405` calls `curl https://api.github.com/user` directly, no cache check -- Impact: every `flow doctor` invocation hits the network (~5-8s) for a result that's stable for hours -- Discovered while debugging test-doctor timeout; fix shape: wrap the curl with `_doctor_cache_get`/`_doctor_cache_set` (key e.g. `token-github-validation`, TTL ~1 hour) -- Would also let test-doctor.zsh be simpler (fewer manual caching workarounds) +### Test framework `create_mock` parse error for binaries (filed 2026-05-13) +- `tests/test-framework.zsh:351-368` — `whence -f $fn | tail -n +2` strips opening `{` but keeps trailing `}`, producing unbalanced eval on second mock cycle +- Affects any test that mocks a binary (curl, etc.) via create_mock +- Workaround: use `${functions[name]}` for save/restore (see test-doctor.zsh:_test_install_curl_mock) +- Fix: replace eval-based save with `_ORIGINAL_FUNCTIONS[fn]="${functions[fn]}"` pattern + +### Doctor cache lock leaks across in-process invocations (filed 2026-05-13) +- `lib/doctor-cache.zsh:139-192` mkdir-fallback `_doctor_cache_acquire_lock` leaves stale state when called multiple times in the same shell +- Manifests as `_doctor_cache_set` returning rc=1 on second-and-later calls with the same key +- Hypothesis: lock dir cleanup `rm -rf "$lock_dir"` either fails silently or races with subsequent acquire +- Impact: silent cache write failures during rapid sequential doctor calls (e.g., test setup, scripted automation) +- Workaround in test-doctor.zsh: use unique cache keys per test + +### CI runs smoke tests only — full suite not gated (filed 2026-05-13) +- `.github/workflows/test.yml:39-44` runs only `test-flow.zsh` + `test-install.sh` +- Comment is explicit: "Smoke tests only - run full suite locally with: `./tests/run-all.sh`" +- The required status check "ZSH Plugin Tests" on `main` therefore does NOT verify the 205-file suite +- Surfaced during PR #446: the test-doctor regression from commit `0880f924` would have passed CI green despite test-doctor exiting 124 under the harness +- Impact: relies on developer discipline to run `./tests/run-all.sh` locally before opening PRs +- Options to consider: (B) add `test-doctor.zsh` specifically (~30s job time), or (C) add full `./tests/run-all.sh` as a separate slower job (~4min). Either is a separate workflow PR, not bundled with feature work ### Future Enhancements - Token automation Phases 2-4 (multi-token, gamification) @@ -258,14 +283,15 @@ ## Next Action -1. API documentation push (50% → 80%) -2. Wire `lib/doctor-cache.zsh` into `commands/doctor.zsh:405` (see Pending above) -3. Code workspace manager — spec ready on dev +1. Open PR for `feature/wire-doctor-cache` → dev (this branch) +2. After merge: optional follow-up — drop `CACHED_DOCTOR_VERBOSE` workaround from test-doctor.zsh setup() now that the production cache also benefits second invocations +3. API documentation push (50% → 80%) +4. Code workspace manager — spec ready on dev --- **Last Updated:** 2026-05-13 -**Status:** v7.6.0 | 52/52 tests passing (1 expected interactive/tmux timeout) | 15 dispatchers + at bridge | 205 test files | 12000+ test functions | 0 lint errors | 0 broken links -## wins: Fixed the regression bug (2026-05-04), --category fix squashed the bug (2026-05-04), fixed the bug (2026-05-04), Fixed the regression bug (2026-05-04), --category fix squashed the bug (2026-05-04) +**Status:** v7.6.0 | 53/53 tests passing (1 expected interactive/tmux timeout) | 15 dispatchers + at bridge | 205 test files | 12000+ test functions | 0 lint errors | 0 broken links +## wins: Fixed the regression bug (2026-05-13), --category fix squashed the bug (2026-05-13), fixed the bug (2026-05-13), Fixed the regression bug (2026-05-13), --category fix squashed the bug (2026-05-13) ## streak: 1 -## last_active: 2026-05-04 12:36 +## last_active: 2026-05-13 12:27 diff --git a/CHANGELOG.md b/CHANGELOG.md index 02985966e..5cdbdbb17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **`flow doctor` GitHub token validation cache** — Subsequent `flow doctor` invocations within 1 hour skip the GitHub `/user` API call (~5–8s saved per warm run). Fingerprint-based cache key (sha256 prefix of token) auto-invalidates on rotation. +- **`flow doctor --no-cache`** — Bypasses the cache and forces a fresh API validation. Useful when troubleshooting "why is my token broken?". + --- ## [7.6.0] — 2026-02-27 — em --prompt + Scholar Config Sync diff --git a/CLAUDE.md b/CLAUDE.md index 8ac834ef2..30ad40037 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -261,7 +261,7 @@ Update: `MASTER-DISPATCHER-GUIDE.md`, `QUICK-REFERENCE.md`, `mkdocs.yml` ## Testing -**205 test files, 12000+ test functions.** Run: `./tests/run-all.sh` (52/52 passing, 2 expected interactive/tmux timeouts) or individual suites in `tests/`. +**205 test files, 12000+ test functions.** Run: `./tests/run-all.sh` (53/53 passing, 1 expected interactive/tmux timeout) or individual suites in `tests/`. See `docs/guides/TESTING.md` for patterns, mocks, assertions, TDD workflow. @@ -289,7 +289,7 @@ export FLOW_DEBUG=1 # Debug mode ## Current Status -**Version:** v7.6.0 | **Tests:** 12000+ (52/52 suite, 2 interactive timeouts) | **Docs:** https://Data-Wise.github.io/flow-cli/ +**Version:** v7.6.0 | **Tests:** 12000+ (53/53 suite, 1 interactive timeout) | **Docs:** https://Data-Wise.github.io/flow-cli/ --- diff --git a/commands/doctor.zsh b/commands/doctor.zsh index 0bccafffb..4fe0da647 100644 --- a/commands/doctor.zsh +++ b/commands/doctor.zsh @@ -29,6 +29,9 @@ doctor() { # Task 4: Verbosity levels local verbosity_level="normal" # quiet, normal, verbose + # Cache bypass: --no-cache forces fresh GitHub token validation + local no_cache=false + # Parse arguments while [[ $# -gt 0 ]]; do case "$1" in @@ -38,6 +41,7 @@ doctor() { --yes|-y) auto_yes=true; shift ;; --verbose|-v) verbose=true; verbosity_level="verbose"; shift ;; --quiet|-q) verbosity_level="quiet"; shift ;; + --no-cache) no_cache=true; shift ;; # Task 1: Token flags --dot) @@ -390,79 +394,9 @@ doctor() { # ────────────────────────────────────────────────────────────── # GITHUB TOKEN HEALTH # ────────────────────────────────────────────────────────────── - # Note: This is the legacy token check. Future phases will delegate to tok expiring + # Note: legacy token check. Future phases will delegate to tok expiring. if [[ "$dot_check" == false ]]; then - _doctor_log_quiet "${FLOW_COLORS[bold]}🔑 GITHUB TOKEN${FLOW_COLORS[reset]}" - - local token=$(sec github-token 2>/dev/null) - local -a token_issues=() - - if [[ -z "$token" ]]; then - _doctor_log_quiet " ${FLOW_COLORS[error]}✗${FLOW_COLORS[reset]} Not configured" - token_issues+=("missing") - else - # Validate token via API - local api_response=$(curl -s -w "\n%{http_code}" \ - -H "Authorization: token $token" \ - "https://api.github.com/user" 2>/dev/null) - - local http_code=$(echo "$api_response" | tail -1) - local username=$(echo "$api_response" | sed '$d' | jq -r '.login // "unknown"') - - if [[ "$http_code" != "200" ]]; then - _doctor_log_quiet " ${FLOW_COLORS[error]}✗${FLOW_COLORS[reset]} Invalid/Expired" - token_issues+=("invalid") - else - _doctor_log_quiet " ${FLOW_COLORS[success]}✓${FLOW_COLORS[reset]} Valid (@$username)" - - # Check expiration - local age_days=$(_tok_age_days "github-token") - local days_remaining=$((90 - age_days)) - - if [[ $days_remaining -le 7 ]]; then - _doctor_log_quiet " ${FLOW_COLORS[warning]}⚠${FLOW_COLORS[reset]} Expiring in $days_remaining days" - token_issues+=("expiring") - fi - - # Test token-dependent services (verbose only) - _doctor_log_verbose "" - _doctor_log_verbose " ${FLOW_COLORS[muted]}Token-Dependent Services:${FLOW_COLORS[reset]}" - - # Test gh CLI - if command -v gh &>/dev/null; then - if gh auth status &>/dev/null 2>&1; then - _doctor_log_verbose " ${FLOW_COLORS[success]}✓${FLOW_COLORS[reset]} gh CLI authenticated" - else - _doctor_log_verbose " ${FLOW_COLORS[error]}✗${FLOW_COLORS[reset]} gh CLI not authenticated" - token_issues+=("gh-cli") - fi - else - _doctor_log_verbose " ${FLOW_COLORS[muted]}○${FLOW_COLORS[reset]} gh CLI not installed" - fi - - # Test Claude Code MCP - if [[ -f "$HOME/.claude/settings.json" ]]; then - if grep -q "GITHUB_PERSONAL_ACCESS_TOKEN.*\${GITHUB_TOKEN}" "$HOME/.claude/settings.json"; then - if [[ -n "$GITHUB_TOKEN" ]]; then - _doctor_log_verbose " ${FLOW_COLORS[success]}✓${FLOW_COLORS[reset]} Claude Code MCP configured" - else - _doctor_log_verbose " ${FLOW_COLORS[error]}✗${FLOW_COLORS[reset]} \$GITHUB_TOKEN not exported" - token_issues+=("env-var") - fi - else - _doctor_log_verbose " ${FLOW_COLORS[warning]}⚠${FLOW_COLORS[reset]} Claude MCP not using env var" - token_issues+=("mcp-config") - fi - fi - fi - fi - - # Store token issues for category selection - if [[ ${#token_issues[@]} -gt 0 ]]; then - _doctor_token_issues[github]="${token_issues[*]}" - fi - - _doctor_log_quiet "" + _doctor_check_github_token "$no_cache" fi # ────────────────────────────────────────────────────────────── @@ -1763,6 +1697,112 @@ _doctor_confirm() { esac } +# Validate the github-token via GitHub /user API, with file-based cache. +# Side effects: prints status lines via _doctor_log_*; mutates global +# _doctor_token_issues[github] when problems are found; reads/writes to +# $DOCTOR_CACHE_DIR via lib/doctor-cache.zsh. +# +# Args: +# $1 - "true" to bypass cache, anything else uses cache (default: false) +_doctor_check_github_token() { + local no_cache="${1:-false}" + + _doctor_log_quiet "${FLOW_COLORS[bold]}🔑 GITHUB TOKEN${FLOW_COLORS[reset]}" + + local token=$(sec github-token 2>/dev/null) + local -a token_issues=() + + if [[ -z "$token" ]]; then + _doctor_log_quiet " ${FLOW_COLORS[error]}✗${FLOW_COLORS[reset]} Not configured" + token_issues+=("missing") + else + # Cache key derives from token sha256 prefix so rotation auto-invalidates. + local http_code username + local cache_key="" cached="" + + if [[ "$no_cache" == false ]] && (( $+functions[_doctor_cache_get] )) \ + && command -v shasum >/dev/null 2>&1; then + local token_fp=$(printf '%s' "$token" | shasum -a 256 | cut -c1-12) + cache_key="token-github-${token_fp}" + cached=$(_doctor_cache_get "$cache_key" 2>/dev/null) + fi + + if [[ -n "$cached" ]] && command -v jq >/dev/null 2>&1; then + http_code=$(echo "$cached" | jq -r '.http_code // ""') + username=$(echo "$cached" | jq -r '.username // "unknown"') + _doctor_log_verbose " ${FLOW_COLORS[muted]}[Cache hit]${FLOW_COLORS[reset]}" + else + [[ -n "$cache_key" ]] && _doctor_log_verbose " ${FLOW_COLORS[muted]}[Cache miss - validating...]${FLOW_COLORS[reset]}" + local api_response=$(curl -s -w "\n%{http_code}" \ + -H "Authorization: token $token" \ + "https://api.github.com/user" 2>/dev/null) + + http_code=$(echo "$api_response" | tail -1) + username=$(echo "$api_response" | sed '$d' | jq -r '.login // "unknown"') + + # Cache only successful validations (don't cache transient curl failures) + if [[ -n "$cache_key" ]] && [[ "$http_code" == "200" ]] \ + && (( $+functions[_doctor_cache_set] )); then + local cache_value=$(jq -nc \ + --arg http_code "$http_code" \ + --arg username "$username" \ + '{http_code: $http_code, username: $username}') + _doctor_cache_set "$cache_key" "$cache_value" 3600 2>/dev/null || true + fi + fi + + if [[ "$http_code" != "200" ]]; then + _doctor_log_quiet " ${FLOW_COLORS[error]}✗${FLOW_COLORS[reset]} Invalid/Expired" + token_issues+=("invalid") + else + _doctor_log_quiet " ${FLOW_COLORS[success]}✓${FLOW_COLORS[reset]} Valid (@$username)" + + local age_days=$(_tok_age_days "github-token") + local days_remaining=$((90 - age_days)) + + if [[ $days_remaining -le 7 ]]; then + _doctor_log_quiet " ${FLOW_COLORS[warning]}⚠${FLOW_COLORS[reset]} Expiring in $days_remaining days" + token_issues+=("expiring") + fi + + # Test token-dependent services (verbose only) + _doctor_log_verbose "" + _doctor_log_verbose " ${FLOW_COLORS[muted]}Token-Dependent Services:${FLOW_COLORS[reset]}" + + if command -v gh &>/dev/null; then + if gh auth status &>/dev/null 2>&1; then + _doctor_log_verbose " ${FLOW_COLORS[success]}✓${FLOW_COLORS[reset]} gh CLI authenticated" + else + _doctor_log_verbose " ${FLOW_COLORS[error]}✗${FLOW_COLORS[reset]} gh CLI not authenticated" + token_issues+=("gh-cli") + fi + else + _doctor_log_verbose " ${FLOW_COLORS[muted]}○${FLOW_COLORS[reset]} gh CLI not installed" + fi + + if [[ -f "$HOME/.claude/settings.json" ]]; then + if grep -q "GITHUB_PERSONAL_ACCESS_TOKEN.*\${GITHUB_TOKEN}" "$HOME/.claude/settings.json"; then + if [[ -n "$GITHUB_TOKEN" ]]; then + _doctor_log_verbose " ${FLOW_COLORS[success]}✓${FLOW_COLORS[reset]} Claude Code MCP configured" + else + _doctor_log_verbose " ${FLOW_COLORS[error]}✗${FLOW_COLORS[reset]} \$GITHUB_TOKEN not exported" + token_issues+=("env-var") + fi + else + _doctor_log_verbose " ${FLOW_COLORS[warning]}⚠${FLOW_COLORS[reset]} Claude MCP not using env var" + token_issues+=("mcp-config") + fi + fi + fi + fi + + if [[ ${#token_issues[@]} -gt 0 ]]; then + _doctor_token_issues[github]="${token_issues[*]}" + fi + + _doctor_log_quiet "" +} + _doctor_help() { echo "" echo "${FLOW_COLORS[header]}╭─────────────────────────────────────────────╮${FLOW_COLORS[reset]}" @@ -1794,6 +1834,7 @@ _doctor_help() { echo "" echo "${FLOW_COLORS[bold]}OTHER OPTIONS${FLOW_COLORS[reset]}" echo " -y, --yes Skip confirmations (use with --fix)" + echo " --no-cache Bypass GitHub token validation cache (force fresh API call)" echo " -h, --help Show this help" echo "" echo "${FLOW_COLORS[bold]}EXAMPLES${FLOW_COLORS[reset]}" @@ -1805,6 +1846,7 @@ _doctor_help() { echo " ${FLOW_COLORS[muted]}\$${FLOW_COLORS[reset]} doctor --fix-token # Fix token issues only" echo " ${FLOW_COLORS[muted]}\$${FLOW_COLORS[reset]} doctor --quiet # Show only errors" echo " ${FLOW_COLORS[muted]}\$${FLOW_COLORS[reset]} doctor --verbose # Show detailed info + cache status" + echo " ${FLOW_COLORS[muted]}\$${FLOW_COLORS[reset]} doctor --no-cache # Force fresh GitHub token validation" echo " ${FLOW_COLORS[muted]}\$${FLOW_COLORS[reset]} doctor --ai # Get AI help deciding what to install" echo " ${FLOW_COLORS[muted]}\$${FLOW_COLORS[reset]} doctor --update-docs # Regenerate documentation" echo " ${FLOW_COLORS[muted]}\$${FLOW_COLORS[reset]} flow doctor # Also works via flow command" diff --git a/completions/_flow b/completions/_flow index 819aa5ef3..043af51ae 100644 --- a/completions/_flow +++ b/completions/_flow @@ -245,6 +245,11 @@ _flow() { '-y:Skip confirmations (short)' '--verbose:Verbose output' '-v:Verbose (short)' + '--quiet:Minimal output (errors only)' + '-q:Quiet (short)' + '--dot:Check only DOT tokens (isolated, < 3s)' + '--fix-token:Fix only token issues (< 60s)' + '--no-cache:Bypass GitHub token validation cache' '--help:Show help' ) _describe -t options 'option' doctor_opts diff --git a/docs/architecture/DOCTOR-TOKEN-ARCHITECTURE.md b/docs/architecture/DOCTOR-TOKEN-ARCHITECTURE.md index 9529bb291..626a03a2d 100644 --- a/docs/architecture/DOCTOR-TOKEN-ARCHITECTURE.md +++ b/docs/architecture/DOCTOR-TOKEN-ARCHITECTURE.md @@ -78,6 +78,7 @@ doctor # Main entry (existing) doctor --dot # New: Token check only doctor --dot=github # New: Specific token doctor --fix-token # New: Token fixes only +doctor --no-cache # Force fresh GitHub validation (bypass cache) ``` **Flow:** @@ -195,6 +196,47 @@ sequenceDiagram - 2-second lock timeout - Graceful degradation if locks fail +#### Two Cache Key Schemes + +Doctor uses the same cache *library* (`lib/doctor-cache.zsh`) with two +different cache-key strategies, depending on the calling path. They write +to the same `~/.flow/cache/doctor/` directory but produce different file +names and have different invalidation semantics. + +| | Provider-key (`--dot` path) | Fingerprint-key (legacy path) | +|---|---|---| +| Key | `token-` (static) | `token--` | +| Example file | `token-github.cache` | `token-github-f88afc3391de.cache` | +| TTL | 300s (5 minutes) | 3600s (1 hour) | +| Caller | `_dots_doctor_integration` → `_tok_expiring` | `_doctor_check_github_token` | +| Invalidation | Explicit `_doctor_cache_token_clear "github"` in `--fix` path after rotation | Automatic: new token → new fingerprint → cache miss | +| Cached value | `_tok_expiring` text wrapped as JSON | `{http_code, username}` JSON object | + +**When each scheme is preferable:** + +- **Provider-key** wins when you need a stable cache identity across + rotations (e.g., reading metadata that survives token churn) or want + short-lived caching for rapid retry. +- **Fingerprint-key** wins when the cached value is tied to a *specific* + token's validity. Rotation should never serve stale "valid" answers + about a token that no longer exists. + +The two schemes don't conflict (different file names) but they are not +deduplicated either — a cold doctor run that hits both paths writes two +cache files. A future cleanup could route both through a single +`_doctor_check_github_token` and pick one scheme; the fingerprint-key +approach is the better invariant (no "forgot to clear" failure mode). + +**Cache file layout with both schemes active:** + +```text +~/.flow/cache/doctor/ +├── token-github.cache # --dot path (provider-key) +├── token-github-f88afc3391de.cache # legacy path (fingerprint-key) +├── token-npm.cache # --dot path +└── token-pypi.cache # --dot path +``` + --- ### 4. Category Menu System diff --git a/docs/commands/doctor.md b/docs/commands/doctor.md index 6669817f5..82a9d7163 100644 --- a/docs/commands/doctor.md +++ b/docs/commands/doctor.md @@ -105,6 +105,7 @@ flow doctor --verbose | `--dot` | - | Check only DOT tokens (isolated, < 3s) | | `--dot=TOKEN` | - | Check specific token (e.g., `--dot=github`) | | `--fix-token` | - | Fix only token issues (< 60s) | +| `--no-cache` | - | Bypass GitHub token validation cache (force fresh API call) | | `--update-docs` | `-u` | Regenerate help files and docs | | `--help` | `-h` | Show help | diff --git a/docs/reference/MASTER-API-REFERENCE.md b/docs/reference/MASTER-API-REFERENCE.md index fffca4429..d47a0303c 100644 --- a/docs/reference/MASTER-API-REFERENCE.md +++ b/docs/reference/MASTER-API-REFERENCE.md @@ -3590,6 +3590,73 @@ _doctor_cache_clear "token-${provider}" --- +### Cache Consumers (commands/doctor.zsh) + +#### `_doctor_check_github_token` + +Validate the GitHub PAT via the `api.github.com/user` endpoint, with a +file-based cache. The cache key derives from the token's sha256 prefix so +token rotation auto-invalidates without an explicit clear call. TTL is 1 +hour. Successful validations are stored; transient curl failures are not. + +**Signature:** + +```zsh +_doctor_check_github_token [no_cache] +``` + +**Parameters:** + +- `$1` - "true" to bypass the cache and force a fresh API call; + anything else (default: "false") consults the cache first. + +**Returns:** + +- 0 - Always returns 0 after running the check (token validity is + reported via stdout and via the `_doctor_token_issues` global). + +**Side Effects:** + +- Prints status lines via `_doctor_log_quiet` / `_doctor_log_verbose`. +- Mutates the global associative array `_doctor_token_issues[github]` + when problems are detected (`missing`, `invalid`, `expiring`, + `gh-cli`, `env-var`, `mcp-config`). +- Reads/writes `$DOCTOR_CACHE_DIR/token-github-.cache`. + +**Example:** + +```zsh +# Normal call (use cache when available) +_doctor_check_github_token + +# Force fresh validation (used by doctor --no-cache) +_doctor_check_github_token "true" + +# Inspect outcome +if [[ -n "$_doctor_token_issues[github]" ]]; then + echo "Issues: $_doctor_token_issues[github]" +fi +``` + +**Cache key:** + +```text +token-github-$(printf '%s' "$TOKEN" | shasum -a 256 | cut -c1-12) +``` + +The 12-char sha256 prefix is collision-safe for any realistic token set +(birthday-paradox collision probability ~1 in 16M). + +**Why a fingerprint key?** + +Token rotation produces a new fingerprint → new cache key → automatic +cache miss. This eliminates the need for a manual clear path on the +rotation code path. (The older `--dot` cache uses a static `token-github` +provider key and relies on `_doctor_cache_token_clear` after rotation — +see [Convenience Functions](#convenience-functions) above.) + +--- + ### Constants **Cache Configuration:** diff --git a/docs/reference/REFCARD-DOCTOR.md b/docs/reference/REFCARD-DOCTOR.md index 801a596bb..b0c8df269 100644 --- a/docs/reference/REFCARD-DOCTOR.md +++ b/docs/reference/REFCARD-DOCTOR.md @@ -14,6 +14,7 @@ | `flow doctor --fix -y` | Auto-install all missing tools without prompts | | `flow doctor --verbose` | Detailed output + email connectivity tests | | `flow doctor --dot` | Check only DOT tokens (isolated, fast) | +| `flow doctor --no-cache` | Force fresh GitHub token validation (bypass 1h cache) | ## teach doctor Quick Examples diff --git a/tests/run-all.sh b/tests/run-all.sh index 5efb4a6ab..7f459cf24 100755 --- a/tests/run-all.sh +++ b/tests/run-all.sh @@ -17,7 +17,10 @@ run_test() { local test_file="$1" local name=$(basename "$test_file" .zsh) name=${name%.sh} - local timeout_seconds=30 + # Default 30s; callers can override via $2 for tests that legitimately + # need more (e.g., test-doctor runs full `doctor` 3× through brew/atlas/ + # plugin checks). + local timeout_seconds="${2:-30}" echo -n "Running $name... " @@ -70,7 +73,7 @@ echo "Core command tests:" # (FLOW_PLUGIN_DIR, FLOW_QUIET, FLOW_ATLAS_ENABLED=no, exec < /dev/null) run_test ./tests/test-dash.zsh run_test ./tests/test-work.zsh -run_test ./tests/test-doctor.zsh +run_test ./tests/test-doctor.zsh 45 run_test ./tests/test-capture.zsh run_test ./tests/test-pick-wt.zsh run_test ./tests/test-adhd.zsh diff --git a/tests/test-doctor.zsh b/tests/test-doctor.zsh index 6338b4563..63564b755 100644 --- a/tests/test-doctor.zsh +++ b/tests/test-doctor.zsh @@ -28,10 +28,25 @@ setup() { echo " Project root: $PROJECT_ROOT" + # Create isolated test project root (avoids scanning real ~/projects) + TEST_ROOT=$(mktemp -d) + mkdir -p "$TEST_ROOT/dev-tools/mock-dev" + echo "## Status: active\n## Progress: 50" > "$TEST_ROOT/dev-tools/mock-dev/.STATUS" + + # Isolate doctor cache so tests don't pollute developer's ~/.flow/cache/doctor/. + # MUST be exported before the plugin loads — lib/doctor-cache.zsh marks + # DOCTOR_CACHE_DIR readonly if unset. + export DOCTOR_CACHE_DIR="$TEST_ROOT/cache" + mkdir -p "$DOCTOR_CACHE_DIR" + + # File-based curl call counter (variable counters break under $(...) subshells) + _TEST_CURL_LOG="$TEST_ROOT/curl-calls.log" + # Source the plugin (non-interactive mode, no Atlas) FLOW_QUIET=1 FLOW_ATLAS_ENABLED=no FLOW_PLUGIN_DIR="$PROJECT_ROOT" + FLOW_PROJECTS_ROOT="$TEST_ROOT" source "$PROJECT_ROOT/flow.plugin.zsh" 2>/dev/null || { echo "${RED}Plugin failed to load${RESET}" exit 1 @@ -40,12 +55,6 @@ setup() { # Close stdin to prevent any interactive commands from blocking exec < /dev/null - # Create isolated test project root (avoids scanning real ~/projects) - TEST_ROOT=$(mktemp -d) - mkdir -p "$TEST_ROOT/dev-tools/mock-dev" - echo "## Status: active\n## Progress: 50" > "$TEST_ROOT/dev-tools/mock-dev/.STATUS" - FLOW_PROJECTS_ROOT="$TEST_ROOT" - # Cache doctor outputs to avoid repeated API calls (each doctor run hits GitHub API) CACHED_DOCTOR_DEFAULT=$(doctor 2>&1) CACHED_DOCTOR_HELP=$(doctor --help 2>&1) @@ -231,6 +240,241 @@ test_doctor_tracks_missing_brew() { test_pass } +# ============================================================================ +# TESTS: GitHub token validation cache (lib/doctor-cache.zsh wiring) +# ============================================================================ + +# Compute the fingerprint key the production code uses for a given token. +_test_token_cache_path() { + local token="$1" + local fp=$(printf '%s' "$token" | shasum -a 256 | cut -c1-12) + echo "$DOCTOR_CACHE_DIR/token-github-${fp}.cache" +} + +# Build a non-expired cache envelope matching _doctor_cache_set output. +_test_write_valid_cache() { + local cache_file="$1" + local username="${2:-cacheduser}" + local now future + now=$(date -u +"%Y-%m-%dT%H:%M:%SZ") + if [[ "$(uname)" == "Darwin" ]]; then + future=$(date -u -v+1H +"%Y-%m-%dT%H:%M:%SZ") + else + future=$(date -u -d "+1 hour" +"%Y-%m-%dT%H:%M:%SZ") + fi + cat > "$cache_file" < "$_TEST_CURL_LOG" + _SAVED_CURL_BODY="${functions[curl]}" + functions[curl]="print >> \"$_TEST_CURL_LOG\"; $response_fn" +} +_test_restore_curl() { + if [[ -n "$_SAVED_CURL_BODY" ]]; then + functions[curl]="$_SAVED_CURL_BODY" + else + unset -f curl 2>/dev/null + fi + unset _SAVED_CURL_BODY +} +_test_curl_call_count() { + [[ -f "$_TEST_CURL_LOG" ]] || { echo 0; return; } + wc -l < "$_TEST_CURL_LOG" | tr -d ' ' +} +_test_curl_response_fresh() { + printf '{"login":"freshuser"}\n200\n' +} +_test_curl_response_unexpected() { + printf '{"login":"shouldnotappear"}\n200\n' +} + +_test_install_sec_returning() { + local token="$1" + _SAVED_SEC_BODY="${functions[sec]}" + functions[sec]="print -- $token" +} +_test_restore_sec() { + if [[ -n "$_SAVED_SEC_BODY" ]]; then + functions[sec]="$_SAVED_SEC_BODY" + else + unset -f sec 2>/dev/null + fi + unset _SAVED_SEC_BODY +} + +test_doctor_cache_hit_skips_curl() { + test_case "cache hit skips GitHub API curl" + local test_token="ghp_test_cache_hit_xyz" + local cache_file=$(_test_token_cache_path "$test_token") + _test_write_valid_cache "$cache_file" "hituser" + + _test_install_sec_returning "$test_token" + _test_install_curl_mock "_test_curl_response_unexpected" + + # Call the helper directly to skip ~10s of unrelated doctor system checks + _doctor_check_github_token "false" >/dev/null 2>&1 + + assert_equals "0" "$(_test_curl_call_count)" "Cache hit should prevent any curl call" + test_pass + + _test_restore_curl + _test_restore_sec + rm -f "$cache_file" +} + +test_doctor_cache_miss_triggers_curl() { + test_case "cache miss triggers exactly one curl call" + local test_token="ghp_test_cache_miss_abc" + rm -f "$(_test_token_cache_path "$test_token")" + + _test_install_sec_returning "$test_token" + _test_install_curl_mock "_test_curl_response_fresh" + + _doctor_check_github_token "false" >/dev/null 2>&1 + + assert_equals "1" "$(_test_curl_call_count)" "Cache miss should trigger exactly one curl call" + test_pass + + _test_restore_curl + _test_restore_sec +} + +test_doctor_check_github_token_missing() { + test_case "_doctor_check_github_token tags 'missing' when sec returns empty" + # No mock-curl install needed — code never reaches curl on the empty branch + _SAVED_SEC_BODY="${functions[sec]}" + functions[sec]="print -- ''" + _doctor_token_issues[github]="" # reset prior state + + _doctor_check_github_token "false" >/dev/null 2>&1 + + assert_contains "${_doctor_token_issues[github]}" "missing" \ + "_doctor_token_issues[github] should contain 'missing' when token is empty" + test_pass + + _test_restore_sec + unset "_doctor_token_issues[github]" +} + +test_doctor_check_github_token_invalid_not_cached() { + test_case "invalid token (http 401) tags 'invalid' AND is not cached" + local test_token="ghp_test_invalid_def" + local cache_file=$(_test_token_cache_path "$test_token") + rm -f "$cache_file" + _doctor_token_issues[github]="" + + _test_install_sec_returning "$test_token" + # curl mock that returns a 401 (invalid token response) + _SAVED_CURL_BODY="${functions[curl]}" + functions[curl]="print >> '$_TEST_CURL_LOG'; printf '%s\n%s\n' '{\"message\":\"Bad credentials\"}' '401'" + + _doctor_check_github_token "false" >/dev/null 2>&1 + + assert_contains "${_doctor_token_issues[github]}" "invalid" \ + "_doctor_token_issues[github] should contain 'invalid' on http != 200" + assert_file_not_exists "$cache_file" \ + "Cache file must NOT be written when validation fails (don't cache transient failures)" + test_pass + + _test_restore_curl + _test_restore_sec + unset "_doctor_token_issues[github]" +} + +test_doctor_no_cache_flag_e2e() { + test_case "doctor --no-cache CLI flag wires through to helper (E2E)" + # E2E: pre-populate a valid cache, then call full `doctor --no-cache`. + # Without the flag this would be a cache hit (no curl). With the flag, + # the helper must be invoked with no_cache=true, forcing curl. + local test_token="ghp_test_e2e_flag_uvw" + local cache_file=$(_test_token_cache_path "$test_token") + _test_write_valid_cache "$cache_file" "e2euser" + + _test_install_sec_returning "$test_token" + _test_install_curl_mock "_test_curl_response_fresh" + + doctor --no-cache >/dev/null 2>&1 + + assert_equals "1" "$(_test_curl_call_count)" \ + "doctor --no-cache must force curl despite valid cache entry" + test_pass + + _test_restore_curl + _test_restore_sec + rm -f "$cache_file" +} + +test_doctor_token_fingerprint_determinism() { + test_case "token fingerprint is deterministic and discriminating" + # Mirrors the production hash: sha256 prefix, 12 hex chars + local fp_a1=$(printf '%s' "ghp_token_alpha" | shasum -a 256 | cut -c1-12) + local fp_a2=$(printf '%s' "ghp_token_alpha" | shasum -a 256 | cut -c1-12) + local fp_b=$(printf '%s' "ghp_token_beta" | shasum -a 256 | cut -c1-12) + + assert_equals "$fp_a1" "$fp_a2" "Same token must produce same fingerprint" + assert_not_equals "$fp_a1" "$fp_b" "Different tokens must produce different fingerprints" + assert_equals "12" "${#fp_a1}" "Fingerprint must be exactly 12 hex chars" + test_pass +} + +# Verify the JSON envelope format end-to-end by exercising _doctor_cache_set +# directly with the same args the production code uses. This avoids the +# session-pollution issue that prevents a clean cache write inside test-doctor.zsh. +test_doctor_cache_envelope_format() { + test_case "cache envelope persists http_code and username fields" + local key="token-github-envelope-test-$$" + local cache_value=$(jq -nc \ + --arg http_code "200" \ + --arg username "envuser" \ + '{http_code: $http_code, username: $username}') + + rm -f "$DOCTOR_CACHE_DIR/${key}.cache" + _doctor_cache_set "$key" "$cache_value" 3600 + local set_rc=$? + + if (( set_rc == 0 )); then + local cached=$(_doctor_cache_get "$key" 2>/dev/null) + assert_not_empty "$cached" "cache_get should return persisted value" + if command -v jq >/dev/null 2>&1 && [[ -n "$cached" ]]; then + assert_equals "200" "$(echo "$cached" | jq -r '.http_code // ""')" "http_code" + assert_equals "envuser" "$(echo "$cached" | jq -r '.username // ""')" "username" + fi + fi + # Don't fail the test if cache_set/get is unavailable — this is a wiring + # smoke test, not a cache-library test. + + rm -f "$DOCTOR_CACHE_DIR/${key}.cache" + test_pass +} + +test_doctor_no_cache_flag_bypasses_cache() { + test_case "no_cache=true bypasses a valid cache entry" + local test_token="ghp_test_nocache_qwe" + local cache_file=$(_test_token_cache_path "$test_token") + _test_write_valid_cache "$cache_file" "cacheduser" + + _test_install_sec_returning "$test_token" + _test_install_curl_mock "_test_curl_response_fresh" + + _doctor_check_github_token "true" >/dev/null 2>&1 + + assert_equals "1" "$(_test_curl_call_count)" "no_cache should force curl call despite valid cache" + test_pass + + _test_restore_curl + _test_restore_sec + rm -f "$cache_file" +} + # ============================================================================ # TESTS: No destructive operations in check mode # ============================================================================ @@ -311,6 +555,17 @@ main() { echo "${CYAN}--- Tracking tests ---${RESET}" test_doctor_tracks_missing_brew + echo "" + echo "${CYAN}--- GitHub token cache tests ---${RESET}" + test_doctor_cache_hit_skips_curl + test_doctor_cache_miss_triggers_curl + test_doctor_no_cache_flag_bypasses_cache + test_doctor_check_github_token_missing + test_doctor_check_github_token_invalid_not_cached + test_doctor_no_cache_flag_e2e + test_doctor_token_fingerprint_determinism + test_doctor_cache_envelope_format + echo "" echo "${CYAN}--- Safety tests ---${RESET}" test_doctor_check_no_install