From 289a47ade9a1e2965b007ade46812fe87016fd2d Mon Sep 17 00:00:00 2001 From: Michael Libio Date: Sat, 30 May 2026 00:04:19 +0200 Subject: [PATCH 1/2] Skill preflight: PATH check, SKILL.md preflight docs, daemon error coverage (closes #1229) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - SKILL.md: adds Preflight Check section with explicit `nodespace --version` + `nodespace diagnostics` instructions before multi-step operations, and a failure→recovery table covering "command not found" and "daemon not running" - installer.ts: adds `isNodespaceBinaryOnPath()` (exported) that probes the binary via `execFileSync('nodespace', ['--version'])` without requiring a daemon; `install()` calls it and writes a loud stderr warning when the binary is absent at install time - installer.test.ts: 4 new tests for `isNodespaceBinaryOnPath` and the stderr warning path `nodespace --version` was already daemon-free (clap handles it before Cli::parse() returns); all subcommands already emit actionable daemon-down messages via the three connect() helpers — no Rust changes needed. Co-Authored-By: Claude --- packages/skill/SKILL.md | 25 ++++++++- packages/skill/src/installer.ts | 24 +++++++++ packages/skill/src/tests/installer.test.ts | 60 +++++++++++++++++++++- 3 files changed, 106 insertions(+), 3 deletions(-) diff --git a/packages/skill/SKILL.md b/packages/skill/SKILL.md index 8485fc1c1..fe1eb8ef2 100644 --- a/packages/skill/SKILL.md +++ b/packages/skill/SKILL.md @@ -2,6 +2,25 @@ NodeSpace is a local-first knowledge graph that stores notes, tasks, and structured data on your machine. Use it to persist information across sessions, build personal knowledge bases, and retrieve context from previous work. +## Preflight Check + +**Before starting any multi-step NodeSpace operation**, run these two commands to confirm the tooling is present and healthy: + +```bash +nodespace --version +nodespace diagnostics +``` + +Run this preflight once per session or task, not before every individual command. + +### Failure recovery + +| Symptom | Cause | Recovery | +|---------|-------|----------| +| `command not found: nodespace` | CLI not installed or not on `$PATH` | Tell the user: NodeSpace CLI is not installed. They need to install it (e.g. via the NodeSpace DMG or `cargo install nodespace-cli`). Do not proceed. | +| `Could not connect to nodespaced` | Daemon not running | Surface the CLI's own message to the user: start the daemon with `nodespaced`. Do not retry automatically — wait for confirmation. | +| `diagnostics` shows entries in `errors` | Database issues | Report the specific error messages to the user before continuing. | + ## When to Use NodeSpace - **Store notes or findings**: Save research, decisions, or summaries you'll want later @@ -13,7 +32,7 @@ NodeSpace is a local-first knowledge graph that stores notes, tasks, and structu NodeSpace daemon must be running. The `nodespace` CLI communicates with `nodespaced` over a Unix socket. If the daemon is not running, CLI commands will fail with a connection error. -Start the daemon: `nodespace daemon start` (or it starts automatically on login if installed via DMG). +Start the daemon: `nodespaced` (or it starts automatically on login if installed via DMG). ## CLI Reference @@ -123,7 +142,9 @@ nodespace node create --type note --content "Decision: use REST not GraphQL" --p ### Build a knowledge graph session ```bash -# At session start: search for relevant context +# At session start: preflight check, then search for relevant context +nodespace --version +nodespace diagnostics nodespace node search --query "previous work on this codebase" # During session: save discoveries diff --git a/packages/skill/src/installer.ts b/packages/skill/src/installer.ts index 3585d37c8..5c24b6fb7 100644 --- a/packages/skill/src/installer.ts +++ b/packages/skill/src/installer.ts @@ -1,6 +1,7 @@ import { existsSync, mkdirSync, copyFileSync, rmSync, readdirSync } from 'node:fs'; import { join, dirname, basename } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { execFileSync } from 'node:child_process'; import { AGENTS } from './agents.js'; import type { AgentName, InstallResult, UninstallResult } from './types.js'; @@ -14,7 +15,30 @@ function detectAgents(): AgentName[] { .map(agent => agent.name); } +/** + * Check whether `nodespace` resolves on $PATH by running `nodespace --version`. + * Returns true if the binary is found and exits 0, false otherwise. + * Safe to call without a running daemon — `--version` is handled by clap + * before any socket connection is attempted. + */ +export function isNodespaceBinaryOnPath(): boolean { + try { + execFileSync('nodespace', ['--version'], { stdio: 'ignore' }); + return true; + } catch { + return false; + } +} + export function install(targetAgents?: AgentName[], packageRoot = PACKAGE_ROOT): InstallResult[] { + if (!isNodespaceBinaryOnPath()) { + process.stderr.write( + 'WARNING: `nodespace` is not on $PATH. The skill will be installed, but the CLI\n' + + 'must be installed and on $PATH before agents can use NodeSpace.\n' + + 'Install it via the NodeSpace DMG or `cargo install nodespace-cli`.\n', + ); + } + const detected = targetAgents ?? detectAgents(); const results: InstallResult[] = []; diff --git a/packages/skill/src/tests/installer.test.ts b/packages/skill/src/tests/installer.test.ts index f9ed775bc..46a60c7eb 100644 --- a/packages/skill/src/tests/installer.test.ts +++ b/packages/skill/src/tests/installer.test.ts @@ -11,7 +11,7 @@ vi.mock('node:os', async (importOriginal) => { return { ...actual, homedir: () => TMP }; }); -const { install, uninstall } = await import('../installer.js'); +const { install, uninstall, isNodespaceBinaryOnPath } = await import('../installer.js'); const { AGENTS } = await import('../agents.js'); const SKILL_MD_CONTENT = '# NodeSpace Skill\nTest content'; @@ -180,3 +180,61 @@ describe('uninstall', () => { } }); }); + +describe('isNodespaceBinaryOnPath', () => { + it('returns true when nodespace --version exits 0', () => { + // nodespace is actually built and on PATH in dev — exercise the real binary. + // In CI where the binary is absent this test is skipped via the env guard below. + // We verify the function's contract via the "returns false" case (always testable). + const result = isNodespaceBinaryOnPath(); + // The function must return a boolean — value depends on whether the binary is installed. + expect(typeof result).toBe('boolean'); + }); + + it('returns false when execFileSync throws (binary not found)', async () => { + // Patch child_process on the installer module's live reference by re-importing + // after hoisting the mock. vi.doMock + resetModules lets us control this. + vi.resetModules(); + vi.doMock('node:child_process', () => ({ + execFileSync: () => { throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); }, + })); + const { isNodespaceBinaryOnPath: check } = await import('../installer.js'); + expect(check()).toBe(false); + vi.doUnmock('node:child_process'); + vi.resetModules(); + }); +}); + +describe('install PATH warning', () => { + it('writes a warning to stderr when nodespace is not on PATH', async () => { + vi.resetModules(); + vi.doMock('node:child_process', () => ({ + execFileSync: () => { throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); }, + })); + const { install: freshInstall } = await import('../installer.js'); + + const stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + freshInstall([], FAKE_PKG_ROOT); + expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining('nodespace` is not on $PATH')); + + stderrSpy.mockRestore(); + vi.doUnmock('node:child_process'); + vi.resetModules(); + }); + + it('does not write a warning when nodespace is on PATH', async () => { + vi.resetModules(); + vi.doMock('node:child_process', () => ({ + execFileSync: () => Buffer.from('nodespace 0.1.0\n'), + })); + const { install: freshInstall } = await import('../installer.js'); + + const stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + freshInstall([], FAKE_PKG_ROOT); + expect(stderrSpy).not.toHaveBeenCalled(); + + stderrSpy.mockRestore(); + vi.doUnmock('node:child_process'); + vi.resetModules(); + }); +}); From c0d76802f06543296aaca7145c4b6db38c58bf9a Mon Sep 17 00:00:00 2001 From: Michael Libio Date: Sat, 30 May 2026 00:22:06 +0200 Subject: [PATCH 2/2] Address review: fix vacuous test, export isNodespaceBinaryOnPath, add execFileSync timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace typeof-only assertion with a deterministic mock-based test for the happy path (execFileSync exits 0 → returns true) - Export isNodespaceBinaryOnPath from index.ts so programmatic consumers don't need to reach into internal module paths - Add timeout: 3000 to execFileSync call so a hung wrapper binary can't block install() indefinitely Skipped: aligning SKILL.md symptom column to full CLI output — the prefix "Could not connect to nodespaced" is sufficient for agent pattern matching and the full string includes a machine-specific path that would vary per user. Co-Authored-By: Claude --- packages/skill/src/index.ts | 2 +- packages/skill/src/installer.ts | 2 +- packages/skill/src/tests/installer.test.ts | 16 +++++++++------- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/skill/src/index.ts b/packages/skill/src/index.ts index b6cd19f3b..c2747f761 100644 --- a/packages/skill/src/index.ts +++ b/packages/skill/src/index.ts @@ -1,3 +1,3 @@ -export { install, uninstall } from './installer.js'; +export { install, uninstall, isNodespaceBinaryOnPath } from './installer.js'; export type { AgentName, AgentConfig, InstallResult, UninstallResult } from './types.js'; export { AGENTS } from './agents.js'; diff --git a/packages/skill/src/installer.ts b/packages/skill/src/installer.ts index 5c24b6fb7..a29ad9c72 100644 --- a/packages/skill/src/installer.ts +++ b/packages/skill/src/installer.ts @@ -23,7 +23,7 @@ function detectAgents(): AgentName[] { */ export function isNodespaceBinaryOnPath(): boolean { try { - execFileSync('nodespace', ['--version'], { stdio: 'ignore' }); + execFileSync('nodespace', ['--version'], { stdio: 'ignore', timeout: 3000 }); return true; } catch { return false; diff --git a/packages/skill/src/tests/installer.test.ts b/packages/skill/src/tests/installer.test.ts index 46a60c7eb..c447f4dd1 100644 --- a/packages/skill/src/tests/installer.test.ts +++ b/packages/skill/src/tests/installer.test.ts @@ -182,13 +182,15 @@ describe('uninstall', () => { }); describe('isNodespaceBinaryOnPath', () => { - it('returns true when nodespace --version exits 0', () => { - // nodespace is actually built and on PATH in dev — exercise the real binary. - // In CI where the binary is absent this test is skipped via the env guard below. - // We verify the function's contract via the "returns false" case (always testable). - const result = isNodespaceBinaryOnPath(); - // The function must return a boolean — value depends on whether the binary is installed. - expect(typeof result).toBe('boolean'); + it('returns true when execFileSync exits 0', async () => { + vi.resetModules(); + vi.doMock('node:child_process', () => ({ + execFileSync: () => Buffer.from('nodespace 0.1.0\n'), + })); + const { isNodespaceBinaryOnPath: check } = await import('../installer.js'); + expect(check()).toBe(true); + vi.doUnmock('node:child_process'); + vi.resetModules(); }); it('returns false when execFileSync throws (binary not found)', async () => {