Open
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request adds automatic skill synchronisation at session start across all supported agents (codex, cursor, hermes, pi, and the base hook). A new ChangesSkilify Auto-Pull Feature
MCP Server Bundle Rebuild
Sequence Diagram(s)sequenceDiagram
participant Agent as Agent SessionStart
participant Hook as Hook Process
participant AutoPull as maybeAutoPull()
participant Config as loadConfig()
participant Query as DeeplakeApi Query
participant State as State File
Agent->>Hook: trigger session-start
Hook->>Hook: load credentials
Hook->>AutoPull: invoke maybeAutoPull()
AutoPull->>AutoPull: check HIVEMIND_AUTOPULL_DISABLED
AutoPull->>State: readLastRun()
AutoPull->>AutoPull: check interval throttle
alt throttled or disabled
AutoPull-->>Hook: {pulled:0, skipped:true}
else should run
AutoPull->>Config: load credentials
alt not logged in
AutoPull-->>Hook: {pulled:0, skipped:true}
else logged in
AutoPull->>Query: runPull(timeout:5s)
alt success
Query-->>AutoPull: {pulled:N, skipped:false}
AutoPull->>State: writeLastRun(now)
else timeout or error
Query-->>AutoPull: error
AutoPull-->>Hook: {pulled:0, skipped:true, reason}
end
end
end
Hook->>Hook: log autopull results
Hook->>Agent: continue session
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
mcp/bundle/server.js (1)
35-37: Consider adding a deterministic bundle check in CI.A “rebuild bundle and fail on diff” step would catch accidental artifact drift and keep
mcp/bundle/server.jsreproducible across environments.🤖 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 `@mcp/bundle/server.js` around lines 35 - 37, Add a CI job that rebuilds the bundle and fails if mcp/bundle/server.js differs from the committed file: add a script (e.g., "verify:bundle" or run the existing build that outputs mcp/bundle/server.js), run the build in CI, then diff the generated mcp/bundle/server.js against the repository copy (or run git status --porcelain) and exit non‑zero on any difference; reference the bundle file name "mcp/bundle/server.js" and the bundle build step (your project's build/bundle task) so the CI step deterministically regenerates the artifact and fails the pipeline on any drift.vitest.config.ts (1)
260-262: ⚡ Quick winHold
auto-pull.tsline coverage at 90 too.This new module is already reporting very high line coverage in the PR, but the threshold here only enforces
lines: 80. That weakens the regression guard more than the adjacent 90/70/90 settings suggest.Suggested change
- "src/skilify/auto-pull.ts": { statements: 90, branches: 70, functions: 90, lines: 80 }, + "src/skilify/auto-pull.ts": { statements: 90, branches: 70, functions: 90, lines: 90 },🤖 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 `@vitest.config.ts` around lines 260 - 262, The coverage threshold for "src/skilify/auto-pull.ts" is currently set to lines: 80 but should match the module's other high thresholds; update the coverage entry for the key "src/skilify/auto-pull.ts" in the vitest config to set lines: 90 (keeping statements: 90, branches: 70, functions: 90) so the file's line coverage threshold is enforced at 90.claude-code/tests/skilify-auto-pull.test.ts (1)
144-159: 💤 Low valueConsider using already-imported fs functions.
Lines 147 and 155 use
require("node:fs")inside the test body, butmkdirSyncandwriteFileSyncare not among the top-level imports (onlyexistsSync,mkdtempSync,readFileSync,rmSync). For consistency, consider addingmkdirSyncandwriteFileSyncto the top-level import and using them directly.♻️ Suggested refactor
-import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";Then in tests:
- const { mkdirSync, writeFileSync } = require("node:fs"); - mkdirSync(join(tmpHome, ".deeplake/state/skilify"), { recursive: true }); - writeFileSync(path, "not json"); + mkdirSync(join(tmpHome, ".deeplake/state/skilify"), { recursive: true }); + writeFileSync(path, "not json");🤖 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 `@claude-code/tests/skilify-auto-pull.test.ts` around lines 144 - 159, The tests call require("node:fs") to grab mkdirSync and writeFileSync inside the test bodies; instead add mkdirSync and writeFileSync to the top-level fs imports and use those imported functions in the two tests that call mkdirSync/writeFileSync (the tests referencing TIMESTAMP_REL, tmpHome and calling readLastRun()). Update the import list so mkdirSync and writeFileSync are available and replace the inline require calls with the top-level identifiers to keep imports consistent.
🤖 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.
Inline comments:
In `@pi/extension-source/hivemind.ts`:
- Around line 315-317: The regex used for name validation in hivemind.ts is
looser than the shared assertValidSkillName in src/skilify/skill-writer.ts,
causing mismatches; update the validation in hivemind.ts to use the same
kebab-case rule as assertValidSkillName (i.e., only lowercase letters/digits and
hyphen-separated segments) so skill names are accepted/rejected
consistently—locate the check with the current pattern
/^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$/ and replace it with the same pattern used by
assertValidSkillName to enforce lowercase, hyphen-separated names and segment
rules.
In `@src/hooks/codex/session-start.ts`:
- Around line 108-112: The code currently awaits maybeAutoPull() inside the
synchronous session-start hook which can block session emission; change it to
invoke maybeAutoPull() without awaiting so the auto-pull runs in background:
replace "const pullResult = await maybeAutoPull(); log(...)" with a
fire-and-forget call to maybeAutoPull() (e.g., maybeAutoPull().then(res =>
log(...)).catch(err => logger.warn(...)) or simply maybeAutoPull().catch(...))
so the hook remains synchronous and heavy fs/network work is moved to the
background; keep the original log behavior via the background promise resolution
and ensure any errors are swallowed or logged to avoid leaking into the fast
path (refer to maybeAutoPull and session-start-setup.js).
In `@src/hooks/cursor/session-start.ts`:
- Around line 168-172: The auto-pull currently invoked from SessionStart calls
maybeAutoPull() which hardcodes install="global" (resolving to
~/.claude/skills), causing pulled skills to land in Claude's directory and be
invisible to other agents; update the call site in SessionStart (where
maybeAutoPull() is invoked) to pass an agent-aware install target derived from
the current session context (e.g., agent identifier or session config) so
maybeAutoPull receives the correct install value for Cursor/Hermes/Codex, or
alternately gate the call so SessionStart only calls maybeAutoPull for Claude
sessions and document the limitation; reference maybeAutoPull and the
SessionStart invocation to locate and change the hardcoded install behavior.
---
Nitpick comments:
In `@claude-code/tests/skilify-auto-pull.test.ts`:
- Around line 144-159: The tests call require("node:fs") to grab mkdirSync and
writeFileSync inside the test bodies; instead add mkdirSync and writeFileSync to
the top-level fs imports and use those imported functions in the two tests that
call mkdirSync/writeFileSync (the tests referencing TIMESTAMP_REL, tmpHome and
calling readLastRun()). Update the import list so mkdirSync and writeFileSync
are available and replace the inline require calls with the top-level
identifiers to keep imports consistent.
In `@mcp/bundle/server.js`:
- Around line 35-37: Add a CI job that rebuilds the bundle and fails if
mcp/bundle/server.js differs from the committed file: add a script (e.g.,
"verify:bundle" or run the existing build that outputs mcp/bundle/server.js),
run the build in CI, then diff the generated mcp/bundle/server.js against the
repository copy (or run git status --porcelain) and exit non‑zero on any
difference; reference the bundle file name "mcp/bundle/server.js" and the bundle
build step (your project's build/bundle task) so the CI step deterministically
regenerates the artifact and fails the pipeline on any drift.
In `@vitest.config.ts`:
- Around line 260-262: The coverage threshold for "src/skilify/auto-pull.ts" is
currently set to lines: 80 but should match the module's other high thresholds;
update the coverage entry for the key "src/skilify/auto-pull.ts" in the vitest
config to set lines: 90 (keeping statements: 90, branches: 70, functions: 90) so
the file's line coverage threshold is enforced at 90.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5844273d-a191-4f24-8481-42a18b23d19e
📒 Files selected for processing (21)
README.mdbundle/cli.jsclaude-code/bundle/session-start.jsclaude-code/bundle/shell/deeplake-shell.jsclaude-code/tests/codex-session-start-hook.test.tsclaude-code/tests/skilify-auto-pull.test.tscodex/bundle/session-start.jscodex/bundle/shell/deeplake-shell.jscursor/bundle/session-start.jscursor/bundle/shell/deeplake-shell.jshermes/bundle/session-start.jshermes/bundle/shell/deeplake-shell.jsmcp/bundle/server.jspi/extension-source/hivemind.tssrc/cli/index.tssrc/hooks/codex/session-start.tssrc/hooks/cursor/session-start.tssrc/hooks/hermes/session-start.tssrc/hooks/session-start.tssrc/skilify/auto-pull.tsvitest.config.ts
Comment on lines
+108
to
+112
| // Auto-pull skills from all org users (5s timeout, throttled to 30min). | ||
| // See src/skilify/auto-pull.ts for the full opt-out story. maybeAutoPull | ||
| // never rejects — all errors are swallowed inside. | ||
| const pullResult = await maybeAutoPull(); | ||
| log(`autopull: pulled=${pullResult.pulled} skipped=${pullResult.skipped}`); |
There was a problem hiding this comment.
Keep the Codex fast path free of awaited auto-pull work.
This hook exists specifically to stay synchronous and push heavy work into session-start-setup.js. Awaiting maybeAutoPull() here adds fs/network work and can block context emission for up to the timeout window, which regresses SessionStart latency on the first run in each throttle interval.
Suggested direction
- const pullResult = await maybeAutoPull();
- log(`autopull: pulled=${pullResult.pulled} skipped=${pullResult.skipped}`);
+ // Run auto-pull from the detached setup path so the fast hook can return
+ // its context immediately.🤖 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 `@src/hooks/codex/session-start.ts` around lines 108 - 112, The code currently
awaits maybeAutoPull() inside the synchronous session-start hook which can block
session emission; change it to invoke maybeAutoPull() without awaiting so the
auto-pull runs in background: replace "const pullResult = await maybeAutoPull();
log(...)" with a fire-and-forget call to maybeAutoPull() (e.g.,
maybeAutoPull().then(res => log(...)).catch(err => logger.warn(...)) or simply
maybeAutoPull().catch(...)) so the hook remains synchronous and heavy fs/network
work is moved to the background; keep the original log behavior via the
background promise resolution and ensure any errors are swallowed or logged to
avoid leaking into the fast path (refer to maybeAutoPull and
session-start-setup.js).
Comment on lines
+168
to
+172
| // Auto-pull skills from all org users (5s timeout, throttled to 30min). | ||
| // See src/skilify/auto-pull.ts for the full opt-out story. maybeAutoPull | ||
| // never rejects — all errors are swallowed inside. | ||
| const pullResult = await maybeAutoPull(); | ||
| log(`autopull: pulled=${pullResult.pulled} skipped=${pullResult.skipped}`); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== auto-pull destination =="
rg -n -C2 'resolvePullDestination|maybeAutoPull\(|"\.claude", "skills"|~\/\.claude\/skills' src bundle/cli.js
echo
echo "== non-Claude skill/config surfaces =="
rg -n -C2 '\.hermes|\.agents|AGENTS\.md|SKILL\.md|session-start' src codex cursor hermes pi bundle/cli.js
echo
echo "== any non-Claude consumer of ~/.claude/skills =="
rg -n -C2 '\.claude[/"]\s*,\s*"skills"|~\/\.claude\/skills' src codex cursor hermes piRepository: activeloopai/hivemind
Length of output: 318
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Looking for auto-pull and destination logic =="
rg -n 'maybeAutoPull|resolvePullDestination|\.claude.*skills' src
echo ""
echo "== Non-Claude hook files =="
find . -type f -path '*/hooks/*' -name '*.ts' | head -20
echo ""
echo "== Checking for skill directory references =="
rg -n '\.claude|\.hermes|\.agents' src --type ts | grep -i skill || true
echo ""
echo "== Checking auto-pull.ts implementation =="
find . -type f -name '*auto-pull*'Repository: activeloopai/hivemind
Length of output: 7098
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Checking if Cursor/Hermes/Codex hooks load skills =="
rg -n 'readSkill|loadSkill|\.claude' src/hooks/cursor/session-start.ts src/hooks/hermes/session-start.ts src/hooks/codex/session-start.ts
echo ""
echo "== Check skill loading in skilify-worker (used by all agents) =="
rg -n 'readdir|\.claude|skillsDir|SKILLS' src/skilify/skilify-worker.ts | head -30
echo ""
echo "== Verify maybeAutoPull doesn't have agent-specific logic =="
cat -n src/skilify/auto-pull.ts | head -50Repository: activeloopai/hivemind
Length of output: 3312
Fix auto-pull destination for non-Claude agents.
This change calls maybeAutoPull() from Cursor's SessionStart, but the auto-pull function hardcodes install="global" which resolves to ~/.claude/skills. Cursor, Hermes, and Codex use their own skill directories (~/.agents/skills, ~/.hermes/skills, etc.) and never read from ~/.claude/skills. This means the auto-pull pulls skills that remain invisible to the calling agent—the feature silently fails to work outside Claude while still incurring I/O and network overhead.
The auto-pull logic needs to become agent-aware (pulling to the agent's own skill directory based on context), or the feature should be scoped to Claude-only with documentation updated to clarify the limitation.
🤖 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 `@src/hooks/cursor/session-start.ts` around lines 168 - 172, The auto-pull
currently invoked from SessionStart calls maybeAutoPull() which hardcodes
install="global" (resolving to ~/.claude/skills), causing pulled skills to land
in Claude's directory and be invisible to other agents; update the call site in
SessionStart (where maybeAutoPull() is invoked) to pass an agent-aware install
target derived from the current session context (e.g., agent identifier or
session config) so maybeAutoPull receives the correct install value for
Cursor/Hermes/Codex, or alternately gate the call so SessionStart only calls
maybeAutoPull for Claude sessions and document the limitation; reference
maybeAutoPull and the SessionStart invocation to locate and change the hardcoded
install behavior.
Every supported agent (Claude Code, Codex, Cursor, Hermes, pi) now
auto-runs the equivalent of `hivemind skilify pull --all-users --to
global` at the start of each session, so teammate-mined skills become
available without anyone having to run pull manually.
Design decisions:
- Cadence: throttled at the SessionStart entry point. The previous-run
timestamp is persisted at ~/.deeplake/state/skilify/autopull-last-run.json
via an atomic tmp+rename write (mirrors src/skilify/state.ts). Default
window is 30 minutes; configurable via HIVEMIND_AUTOPULL_INTERVAL_MIN
(0 forces every session, -1 disables). HIVEMIND_AUTOPULL_DISABLED=1 is
the hard opt-out.
- Failure mode: never block SessionStart. maybeAutoPull catches every
network / missing-table / auth error and resolves with skipped:true.
The hook callers do not wrap it in try/catch — the function is
documented as never-rejecting, which keeps the hooks' branch-coverage
numbers intact.
- Async vs sync: the pull runs synchronously (awaited) but is bounded by
a 5-second timeout via Promise.race. A slow Deeplake never freezes the
session.
- Scope: install=global, users=[], force=false. Global so all projects
benefit from the shared skill set. The last-run timestamp is only
written on a successful pull so a failed attempt is naturally retried
on the next SessionStart.
- First-run / not-logged-in: silent skip. No nag.
Wiring:
- New module src/skilify/auto-pull.ts encapsulates the throttle +
loadConfig + runPull + timestamp-persistence logic. It accepts
injected loadConfigFn, queryFn, nowMs, timeoutMs, install, cwd to
make tests fully hermetic at the network seam.
- Five hooks call maybeAutoPull() right after credential bootstrap and
before additionalContext is assembled:
src/hooks/session-start.ts (Claude Code)
src/hooks/codex/session-start.ts (Codex)
src/hooks/cursor/session-start.ts (Cursor)
src/hooks/hermes/session-start.ts (Hermes)
pi/extension-source/hivemind.ts (pi)
- The pi extension ships as raw .ts with zero non-builtin runtime
dependencies, so it cannot import from src/skilify/auto-pull.ts.
The throttle / write / pull logic is inlined into hivemind.ts under
autopullSkillsForPi(), matching the inlining pattern already used for
spawnPiSkilifyWorker and the deeplake fetch client. Schema and SQL
shape match buildPullSql + renderSkillFile in src/skilify/pull.ts.
Tests:
- claude-code/tests/skilify-auto-pull.test.ts (21 cases) covers:
- readIntervalMs: defaults, integer minutes, 0, negative, garbage,
empty
- writeLastRun + readLastRun round-trip incl. malformed JSON paths
- HIVEMIND_AUTOPULL_DISABLED=1 short-circuits before loadConfig
- HIVEMIND_AUTOPULL_INTERVAL_MIN=-1 short-circuits before loadConfig
- not-logged-in silent skip (loadConfig returns null)
- throttle: first call writes timestamp, second call inside window
is skipped (no SQL, no timestamp bump), call past the window runs
again, interval=0 forces every call
- failure paths: query throw → swallowed, timestamp NOT updated;
"table does not exist" → treated as empty, timestamp updated;
hanging query is bounded by the injected 100ms timeout and
completes well under 1s
- install=global default writes under ~/.claude/skills
All assertions go through the real maybeAutoPull → runPull pipeline
with a mocked QueryFn, per CLAUDE.md's "mock at the network seam"
rule. Filesystem effects are scoped to a per-test temp HOME so the
developer's real ~/.deeplake is untouched.
- claude-code/tests/codex-session-start-hook.test.ts adds a stub for
src/skilify/auto-pull.js so the codex hook test (which doesn't mock
src/config.js or src/deeplake-api.js) doesn't fire a real network
query during SessionStart. The other agents' tests already mock
loadConfig + DeeplakeApi so their auto-pull invocation is naturally
contained.
Coverage on src/skilify/auto-pull.ts: 98% lines / 100% functions /
87% branches. vitest.config.ts adds 80/70/90/80 thresholds for the
new file (matches the rest of the skilify suite).
Docs: README "Skills (skilify)" section gains an "Auto-pull at
SessionStart" sub-section with the env-var table; the CLI USAGE
block under `hivemind skilify pull` mentions the auto-pull behaviour
and the opt-out env vars.
Source rebase folded in the new pull layout (<name>--<author>/) from #111; this commit refreshes every shipped bundle to match.
efenocchi
force-pushed
the
feat/session-start-autopull-skills
branch
from
4f67104 to
9f1d3ed
Compare
Contributor
Coverage ReportScope: files changed in this PR. Enforced threshold: 90% per metric (per file via
File Coverage — 12 files changed
Generated for commit 81597c2. |
Pulled skills land canonically under ~/.claude/skills/<name>--<author>/.
Codex, Hermes, and pi read skills from ~/.agents/skills/, ~/.hermes/skills/,
and ~/.pi/agent/skills/ respectively, so they previously couldn't see
anything pulled at session start. After writing the canonical SKILL.md, the
pull now creates a symlink at <agent-root>/<name>--<author> -> canonical for
each detected non-Claude agent root, fanning the same SKILL.md out across
every agent installed on the box.
The fan-out is recorded per-entry in the manifest as `symlinks: string[]`,
making cleanup manifest-driven:
* `unpull` walks `entry.symlinks` and `lstat`-checks each path; symlinks
are unlinked, real files/dirs are left alone (never clobber user
content).
* Manifest-orphan prune (canonical dir gone) also unlinks the now-dangling
fan-out links.
* `runPull` runs `pruneOrphanedEntries()` at the start (skipped on
dry-run) so a user who `rm -rf`-ed a canonical dir by hand sees the
dangling symlinks cleaned up on their next pull.
Refused cases at fan-out time (skipped, not in `symlinks[]`):
* a non-symlink already exists at the link path (user content)
* `symlinkSync` errors (Windows non-developer mode, read-only fs).
Idempotent: next pull retries.
Project-install pulls do NOT fan out — symlinking <cwd>/.claude/skills
into ~/.agents/skills would leak project scope globally.
Cursor has no native skill discovery, so no candidate root.
Schema:
src/skilify/agent-roots.ts new — existence-based detection
src/skilify/manifest.ts + symlinks[] on PulledEntry,
+ unlinkSymlinks(),
+ pruneOrphanedEntries()
src/skilify/pull.ts + fanOutSymlinks(),
+ pruneOrphanedEntries() at runPull start,
+ symlinks recorded on each entry
src/skilify/unpull.ts unlink fan-out symlinks before drop
Coverage: +30 tests across 4 files (agent-roots detection, symlinks
field validation, manifest sweep, fan-out idempotency + clobber
refusal, unpull symlink cleanup, dry-run behaviour).
…rtions stable The cursor and hermes session-start hook tests both assert exact DeeplakeApi.query() call counts (typically 2: SELECT placeholder, INSERT placeholder). After PR #112 wired maybeAutoPull() into SessionStart, a third query (SELECT against `skills`) would fire whenever auto-pull wasn't throttled, breaking those assertions intermittently. The flake was load-order-dependent on whether ~/.deeplake/state/skilify/autopull-last-run.json existed with a recent timestamp — i.e. it depended on the developer's real machine state, not on the test inputs. CI runs from a clean HOME would always fail; local runs would pass once any prior test (or real session) had stamped the file. Fix: set HIVEMIND_AUTOPULL_DISABLED=1 in beforeEach and clear it in afterEach. The codex test was already insulated via direct maybeAutoPullMock; the Claude Code session-start.test.ts asserts hooks.json structure only and never runs the hook, so it's unaffected. The auto-pull module's behaviour is covered exhaustively in claude-code/tests/skilify-auto-pull.test.ts.
Pi's session_start used to inline ~165 LOC of auto-pull logic — a hand- maintained mirror of src/skilify/auto-pull.ts that wrote skills to ~/.claude/skills/<project_key>/<name>/, the pre-#111 layout. After the fan-out work landed (commit d59bc40) that inline copy was triply wrong: 1. Duplicate of the shared maybeAutoPull (drift risk). 2. Wrong layout — wrote <project_key>/<name>/, not <name>--<author>/, so pi-pulled skills wouldn't dedupe across the new manifest schema. 3. Bypassed the symlink fan-out, so pulled skills were invisible to pi itself (pi reads from ~/.pi/agent/skills + ~/.agents/skills). Pi can't import the shared TS module — pi/extension-source/hivemind.ts ships as raw .ts with zero non-builtin runtime deps so pi's runtime can compile + load it directly. Same constraint as wiki-worker and skilify-worker, which we already handle by bundling separately. Solution: bundle a tiny src/skilify/autopull-worker.ts that just calls maybeAutoPull() and exits. Pi's session_start runs it via spawnSync with a 6s wall-clock cap (the worker's internal timeout is 5s, so the extra second is for spawn overhead). Synchronous so freshly pulled skills are visible to pi before the first prompt — same UX as the in-process await in codex / cursor / hermes. Behaviour delta: + pi now writes to <name>--<author>/, with symlinks fanned out into ~/.agents/skills, ~/.hermes/skills, ~/.pi/agent/skills (via the shared detectAgentSkillsRoots logic). + Stale-entry sweep at runPull start applies to pi too. + Manifest writes go through recordPull, so unpull works for pi- pulled skills. - One process spawn per session_start (within the existing 30-min throttle). Net: -165 LOC of inline duplicate, +40 LOC of spawn wrapper + worker shim + installer wiring. Future fixes to maybeAutoPull (timeout, throttle, schema) automatically apply to pi without a parallel edit.
Three small follow-ups from the PR review:
* vitest.config.ts: bump src/skilify/auto-pull.ts line coverage
threshold from 80 to 90, matching the surrounding modules. The file
is currently at 95.23% line coverage, so the new floor doesn't
require any test additions.
* claude-code/tests/skilify-auto-pull.test.ts: drop the inline
require("node:fs") calls in the round-trip tests; mkdirSync /
writeFileSync are now part of the top-level fs import. Cosmetic.
* .github/workflows/ci.yml: extend the existing bundle-drift guard
beyond claude-code/bundle/ to every shipped bundle directory
(bundle/, codex/, cursor/, hermes/, mcp/, pi/). Catches the pi
autopull-worker.js, mcp/server.js, etc. drifting on a forgotten
rebuild — the same kind of regression that produced the previous
"regenerate bundles" follow-up commits in this branch.
bundle/cli.js refresh is the byproduct of the build that ran the
threshold check.
Collaborator
Author
Re: coderabbit suggestion to fire-and-forget
|
CI's "Verify bundle/ directories are up to date" job was failing on every commit since the rebase because the bundles were generated from this worktree, where node_modules resolves to the parent repository three directories up. esbuild embedded those resolved paths verbatim as bundle module markers (e.g. "// ../../../node_modules/@modelcontextprotocol/sdk/..."), while CI's fresh checkout produces "// node_modules/...". Bytes differed, diff failed. Rebuilding after `npm install` inside the worktree (local node_modules) produces the same paths CI does. Affects mcp/server.js (heaviest user of node_modules deps), bundle/cli.js, and the four shell bundles that pull in just-bash. No semantic code change — only the bundle module-marker comments shift from "../../../node_modules/..." to "node_modules/...". This is a one-time correction to align committed artefacts with what `npm run build` produces on a clean checkout. Future builds in this worktree (now that node_modules is local) will match.
Two correctness gaps discovered during e2e testing on a real machine:
1. detectAgentSkillsRoots used existsSync(<skills-dir>) directly. That
broke pi: pi's installer never creates ~/.pi/agent/skills/ (skills
are populated lazily by the user), so pi+hivemind installs would
silently skip pi from the fan-out target list and leave pi without
any pulled skills until the user happened to mkdir the dir.
Switched to marker-based detection: ~/.codex / ~/.hermes /
~/.pi/agent existence drives detection of the corresponding skills
root. As a side-benefit, pi-only-no-codex setups now also correctly
include ~/.agents/skills/ (pi co-consumes the agentskills.io shared
layout, but only codex's installer creates the dir).
2. Per-row fan-out only ran for action="wrote". A user installing a
NEW agent (codex / hermes / pi) AFTER having already pulled skills
would never see those existing skills propagated to the new agent
root, because the next pull's decideAction returns "skipped" for
already-up-to-date skills and the fan-out path is skipped with it.
Added backfillSymlinks(installRoot) that walks all manifest entries
and refreshes their fan-out symlinks to match currently-detected
agent roots. Idempotent: skips manifest writes when the resolved
set matches the recorded one (sameSorted comparison). Called from
runPull's tail (only on global, non-dry-run).
Verified live on the dev box:
* ~/.pi/agent/skills/ went from non-existent → 9 symlinks (one per
pulled skill), all pointing at the canonical ~/.claude/skills/...
* Manifest's symlinks[] field now contains all three agent paths
(~/.agents/, ~/.hermes/, ~/.pi/agent/) for every entry.
* Re-running autopull with all rows action="skipped" (already up to
date) still backfills any deleted symlinks correctly.
Tests: +2 in skilify-pull.test.ts (backfill happy path, orphan-prune
interaction); skilify-agent-roots.test.ts rewritten for marker-based
detection (9 cases including the explicit pi-detection regression
guard). All 2191 tests pass.
The throttle was a performance optimization for an axis that turned out
to be cheap. Inside `runPull` the file writes are already idempotent:
* SKILL.md write skipped when localVersion >= remoteVersion
* symlink fan-out is lstat-checked (no clobber, replaces only stale)
* manifest writes are sameSorted-skipped when nothing changed
So re-running every session is essentially free on disk; the only cost
is a single SQL round-trip plus a few existsSync syscalls. In return,
freshly-mined teammate skills now propagate within seconds rather than
waiting up to 30 minutes for the throttle window to expire.
Removed:
* `HIVEMIND_AUTOPULL_INTERVAL_MIN` env var (was: 0/-1/N minutes)
* `~/.deeplake/state/skilify/autopull-last-run.json` timestamp file
(no migration — the file is now orphan and harmless if it lingers)
* `readIntervalMs`, `readLastRun`, `writeLastRun` helpers in
auto-pull.ts (~70 LOC)
* `nowMs` injection point on AutoPullDeps (only used for throttle)
Kept:
* 5-second `withTimeout` race so a slow Deeplake never freezes
SessionStart
* `HIVEMIND_AUTOPULL_DISABLED=1` hard kill switch
* Silent skip on not-logged-in (no nag)
* Failure swallowing — maybeAutoPull never rejects
Test fallout:
* skilify-auto-pull.test.ts: dropped throttle/timestamp tests
(-13 cases), added a regression guard that two back-to-back calls
both run the query.
* session-start-hook.test.ts (Claude Code): added
HIVEMIND_AUTOPULL_DISABLED=1 in beforeEach. The test asserts exact
queryMock call counts on placeholder branching; without throttling
the auto-pull's own SQL would now leak into every test run, the
same way it did for cursor/hermes earlier in this branch.
Live verified:
* Two back-to-back invocations of the bundled worker both fire the
SQL query (527ms first call, 111ms second — second is fast because
the SDK reuses the connection but the query itself runs). No
timestamp file written either time.
After removing readIntervalMs / readLastRun / writeLastRun from auto-pull.ts, the file's function count fell to 7 — and one of those (the `(sql) => api.query(sql)` lambda built when deps.queryFn is not injected) was never exercised in tests. CI's 90% function threshold caught it: 6/7 = 85.71%. Stubbed DeeplakeApi at module load via vi.mock with a class form (arrow functions aren't constructable, so vi.fn().mockImplementation wouldn't work for `new DeeplakeApi(...)`). The new test calls maybeAutoPull WITHOUT queryFn injected and asserts the wrapped DeeplakeApi.query path is reached. auto-pull.ts coverage now: 100% statements / 100% functions / 100% lines / 78% branches (threshold 70%).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Every supported agent (Claude Code, Codex, Cursor, Hermes, pi) now auto-runs the equivalent of
hivemind skilify pull --all-users --to globalat the start of each session, so teammate-mined skills become available without anyone having to remember to run pull manually.Design decisions
~/.deeplake/state/skilify/autopull-last-run.jsonvia an atomic tmp+rename write (mirrorssrc/skilify/state.ts). Default window is 30 minutes; configurable viaHIVEMIND_AUTOPULL_INTERVAL_MIN(0forces every session,-1disables).HIVEMIND_AUTOPULL_DISABLED=1is the hard opt-out.maybeAutoPullcatches every network / missing-table / auth error and resolves withskipped: true. The hook callers do not wrap it in try/catch — the function is documented as never-rejecting, which keeps the hooks' branch-coverage numbers intact.Promise.race. A slow Deeplake never freezes the session.install: "global",users: [],force: false. Global so all projects benefit. The last-run timestamp is only written on a successful pull, so a failed attempt is naturally retried on the next SessionStart.What got wired
New module
src/skilify/auto-pull.tsencapsulates throttle +loadConfig+runPull+ timestamp persistence. It accepts injectedloadConfigFn,queryFn,nowMs,timeoutMs,install,cwdso tests are fully hermetic at the network seam.maybeAutoPull()is called from each of the five SessionStart hooks right after credential bootstrap and beforeadditionalContextis assembled:src/hooks/session-start.ts(Claude Code)src/hooks/codex/session-start.ts(Codex)src/hooks/cursor/session-start.ts(Cursor)src/hooks/hermes/session-start.ts(Hermes)pi/extension-source/hivemind.ts(pi — logic is inlined because pi ships as raw.tswith zero non-builtin runtime deps; mirrors the existingspawnPiSkilifyWorkerinlining pattern)Tests
claude-code/tests/skilify-auto-pull.test.ts(21 cases) covers:readIntervalMs: defaults, integer minutes,0, negative, garbage, emptywriteLastRun+readLastRunround-trip incl. malformed JSON pathsHIVEMIND_AUTOPULL_DISABLED=1short-circuits beforeloadConfigHIVEMIND_AUTOPULL_INTERVAL_MIN=-1short-circuits beforeloadConfigloadConfigreturns null)interval=0forces every callrelation "skills" does not exist→ treated as empty, timestamp updated; hanging query is bounded by an injected 100ms timeout and completes well under 1sinstall=globaldefault writes under~/.claude/skillsAll assertions go through the real
maybeAutoPull→runPullpipeline with a mockedQueryFn, perCLAUDE.md's "mock at the network seam" rule. Filesystem effects are scoped to a per-test tempHOMEso the developer's real~/.deeplakeis untouched.claude-code/tests/codex-session-start-hook.test.tsadds a stub forsrc/skilify/auto-pull.jsso the codex hook test (which doesn't mocksrc/config.jsorsrc/deeplake-api.js) doesn't fire a real network query during SessionStart. The other agents' tests already mockloadConfig+DeeplakeApi, so their auto-pull invocation is naturally contained.Coverage
src/skilify/auto-pull.ts: 98% lines / 100% functions / 87% branches.vitest.config.tsadds80/70/90/80thresholds for the new file (matches the rest of the skilify suite).Full suite: 2099 / 2099 passing, no threshold ERRORs.
Docs
hivemind skilify pullUSAGE block insrc/cli/index.tsmentions the auto-pull behaviour and the opt-out env vars.Test plan
src/skilify/and does not touch the files PR skilify: fix pull layout + add unpull with manifest-tracked installs #111 edits)~/.deeplake/state/skilify/autopull-last-run.jsonafter the first sessionautopull: throttled (...)in~/.deeplake/hook-debug.logwhenHIVEMIND_DEBUG=1)HIVEMIND_AUTOPULL_INTERVAL_MIN=0andHIVEMIND_AUTOPULL_DISABLED=1opt-outs.ts; verifyautopullSkillsForPiruns at session_start without breaking the existing capture path)Summary by CodeRabbit
New Features
HIVEMIND_AUTOPULL_INTERVAL_MINandHIVEMIND_AUTOPULL_DISABLEDenvironment variables.Documentation
Bug Fixes