Skip to content

Wire shared getLatestVersionCached into CC and Codex session-start hooks#70

Closed
kaghni wants to merge 1 commit intomainfrom
feat/cache-version-check-cc-codex
Closed

Wire shared getLatestVersionCached into CC and Codex session-start hooks#70
kaghni wants to merge 1 commit intomainfrom
feat/cache-version-check-cc-codex

Conversation

@kaghni
Copy link
Copy Markdown
Collaborator

@kaghni kaghni commented Apr 21, 2026

Summary

Completes the version-check caching work started in PR #69 (which wired openclaw). The cached helper getLatestVersionCached has existed in src/hooks/version-check.ts for a while — written, exported, fully unit-tested — but was never wired into production. All three Claude Code and Codex session-start hooks were still calling the uncached getLatestVersion(), meaning each session fires a raw.githubusercontent.com request.

This PR switches them to the cached version. Same helper that PR #69 uses for openclaw.

What changes

  • src/hooks/session-start.ts — CC session-start
  • src/hooks/session-start-setup.ts — CC async setup
  • src/hooks/codex/session-start-setup.ts — codex async setup

All three now call:

const latest = await getLatestVersionCached({
  url: GITHUB_RAW_PKG,
  timeoutMs: 3000,
  cachePath: process.env.HIVEMIND_VERSION_CACHE_PATH || undefined,
});
  • Caches at ~/.deeplake/.version-check.json with a 1h TTL
  • Falls back to the cached value on fetch failure (network hiccups don't blank out the check)
  • Honors HIVEMIND_VERSION_CACHE_PATH env override for test isolation

GITHUB_RAW_PKG is now exported from src/utils/version-check.ts so callers reference the canonical URL.

Test isolation

Previously, the session-start tests mocked global fetch but let the cache helper write to the user's real ~/.deeplake/.version-check.json. Between tests, cached state would bleed into fetch-mock assertions. This PR:

  • Gives each test a unique cache path via mkdtempSync + HIVEMIND_VERSION_CACHE_PATH
  • Cleans up the tmp dir in afterEach

Test plan

  • npm run typecheck clean
  • npm run build — all bundles rebuilt
  • npm test — 918/918 passing across 41 files

Follow-up

After merge, the full caching story lines up across all three hosts:

Network-destinations disclosure (in the README and SKILL.md) can then honestly say "contacts raw.githubusercontent.com at most once per hour."

🤖 Generated with Claude Code

…ck (1h cache)

The cached helper has existed in src/hooks/version-check.ts since it was
written, with full test coverage, but was never wired into production.
All three session-start hooks (CC session-start, CC session-start-setup,
codex session-start-setup) were still calling the uncached
getLatestVersion() — meaning they hit raw.githubusercontent.com on every
session start.

Now all three use getLatestVersionCached, which caches the fetched
'latest' version at ~/.deeplake/.version-check.json with a 1h TTL
(falls back to cached value on fetch failure). Reduces per-host
per-day GitHub fetches from once-per-session to at most once-per-hour,
matches the openclaw plugin's wiring in PR #69, and eliminates the
'we hit GitHub on every session' disclosure gap.

Test isolation: hooks now respect process.env.HIVEMIND_VERSION_CACHE_PATH
if set. Session-start and session-start-setup tests use mkdtempSync to
give each test a unique cache path, so the cache doesn't bleed between
tests (or into the user's real ~/.deeplake/.version-check.json).

Exported GITHUB_RAW_PKG from src/utils/version-check.ts so call sites
pass the URL to the cached helper explicitly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude Bot commented Apr 21, 2026

Claude finished @kaghni's task in 2m 35s —— View job


Review

  • Read PR diff and changed files
  • Analyze for bugs, security issues, correctness, and test coverage

One issue found and posted as an inline comment: the catch block in getLatestVersionCached (and the !res.ok branch) write checkedAt: nowMs to cache on any fetch failure, resetting the full 1h TTL. A transient network error on first install caches null for an hour, silently suppressing update checks. Fix is in src/hooks/version-check.ts — skip the write on error and just return the stale value.

Everything else looks clean: test isolation with per-test tmp dirs is correct, HIVEMIND_VERSION_CACHE_PATH wiring is consistent, and the || undefined fallback pattern is correct.

@github-actions
Copy link
Copy Markdown
Contributor

Coverage Report

Scope: files changed in this PR. Enforced threshold: 90% per metric (per file via vitest.config.ts).

Status Category Percentage Covered / Total
🟢 Lines 95.33% (🎯 90%) 204 / 214
🟢 Statements 94.51% (🎯 90%) 224 / 237
🟢 Functions 94.12% (🎯 90%) 16 / 17
🔴 Branches 83.33% (🎯 90%) 100 / 120
File Coverage — 4 files changed
File Stmts Branches Functions Lines
src/hooks/codex/session-start-setup.ts 🟢 98.5% 🔴 84.4% 🟢 100.0% 🟢 98.3%
src/hooks/session-start-setup.ts 🟢 98.0% 🔴 86.4% 🟢 100.0% 🟢 97.8%
src/hooks/session-start.ts 🟢 98.9% 🟢 91.3% 🟢 100.0% 🟢 98.8%
src/utils/version-check.ts 🔴 67.7% 🔴 60.0% 🔴 75.0% 🔴 72.0%

Generated for commit 2e10e34.

Comment on lines +510 to +518
} catch {
const latest = stale?.latest ?? null;
writeVersionCache({
checkedAt: nowMs,
latest,
url: opts.url
}, cachePath);
return latest;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Writing checkedAt: nowMs in the catch block resets the full 1h TTL on any transient fetch failure (timeout, DNS hiccup). A brand-new install with a brief network blip will cache null and skip version checks for an entire hour. The same issue exists in the !res.ok branch (line 503) — a non-200 response also silently extends the TTL.

Skip the cache write on error and just return the stale value; retrying next session is preferable to a guaranteed 1h blackout. Fix should be applied in src/hooks/version-check.ts (the bundle is generated):

Suggested change
} catch {
const latest = stale?.latest ?? null;
writeVersionCache({
checkedAt: nowMs,
latest,
url: opts.url
}, cachePath);
return latest;
}
} catch {
return stale?.latest ?? null;
}

@kaghni kaghni marked this pull request as draft April 22, 2026 00:37
@kaghni
Copy link
Copy Markdown
Collaborator Author

kaghni commented Apr 22, 2026

Closing without merge. The per-session fetch is the current and preferred behavior. Caching was the bot-suggested alternative to the disclosure fix; now that openclaw's docs (PR #69) accurately name both network destinations, the caching mitigation isn't needed.

Context: the getLatestVersionCached helper reached main via PR #64 (stacked on Davit's still-open PR #61) but was never wired into production call sites. PR #61 is the right venue for anyone revisiting that design choice.

@kaghni kaghni closed this Apr 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant