From 03ea6a259b6b3bd79ca79b293c747d6a77d9ba5a Mon Sep 17 00:00:00 2001 From: levineam Date: Fri, 14 Aug 2026 14:34:55 -0400 Subject: [PATCH] fix(secondbrain): use canonical vault resolver for onboarding --- .../scripts/detect-vault.js | 65 ++++------------ .../tests/detect-vault.test.js | 74 +++++++++++++++++++ runtimes/hermes/skills/jarvos/SKILL.md | 8 +- 3 files changed, 93 insertions(+), 54 deletions(-) create mode 100644 modules/jarvos-secondbrain/tests/detect-vault.test.js diff --git a/modules/jarvos-secondbrain/scripts/detect-vault.js b/modules/jarvos-secondbrain/scripts/detect-vault.js index 178bc927..ed928434 100755 --- a/modules/jarvos-secondbrain/scripts/detect-vault.js +++ b/modules/jarvos-secondbrain/scripts/detect-vault.js @@ -20,60 +20,21 @@ 'use strict'; -const os = require('os'); const path = require('path'); const fs = require('fs'); +const { + discoverConfigPath, + resolveConfig, +} = require('../bridge/config/src/resolve-config'); -// ── Path resolution (mirrors jarvos-paths.js without requiring it) ───────── - -const DEFAULT_CLAWD_DIR = path.join(os.homedir(), 'clawd'); -const DEFAULT_VAULT_DIR = path.join(os.homedir(), 'Documents', 'Vault v3'); - -function expandTilde(p) { - if (typeof p === 'string' && p.startsWith('~/')) { - return path.join(os.homedir(), p.slice(2)); - } - return p; -} - -function loadJarvosConfig() { - const clawdDir = expandTilde( - process.env.JARVOS_CLAWD_DIR || process.env.CLAWD_DIR || DEFAULT_CLAWD_DIR - ); - const configPath = path.join(clawdDir, 'jarvos.config.json'); - let cfg = {}; - let configExists = false; - if (fs.existsSync(configPath)) { - configExists = true; - try { - cfg = JSON.parse(fs.readFileSync(configPath, 'utf8')); - } catch { - // unparseable — treat as empty - } - } - return { cfg, configPath, configExists }; -} +// ── Path resolution ──────────────────────────────────────────── function resolveVaultPaths() { - const { cfg, configPath, configExists } = loadJarvosConfig(); - - // Env vars take precedence over config file. - const vault = - expandTilde(process.env.JARVOS_VAULT_DIR) || - expandTilde(cfg.paths?.vault) || - DEFAULT_VAULT_DIR; - - const journal = - expandTilde(process.env.JARVOS_JOURNAL_DIR) || - expandTilde(process.env.JOURNAL_DIR) || - expandTilde(cfg.paths?.journal) || - path.join(vault, 'Journal'); - - const notes = - expandTilde(process.env.JARVOS_NOTES_DIR) || - expandTilde(process.env.VAULT_NOTES_DIR) || - expandTilde(cfg.paths?.notes) || - path.join(vault, 'Notes'); + // Keep onboarding on the same fail-closed resolver used by vault mutations. + // In particular, do not duplicate defaults or stale-vault guardrails here. + const configPath = discoverConfigPath(); + const configExists = fs.existsSync(configPath); + const { paths: { vault, journal, notes } } = resolveConfig(); return { vault, journal, notes, configPath, configExists }; } @@ -130,8 +91,13 @@ function main() { const jsonMode = args.includes('--json'); const { vault, journal, notes, configPath, configExists } = resolveVaultPaths(); + const vaultExists = fs.existsSync(vault); if (jsonMode) { + if (!vaultExists) { + console.error(`Resolved vault directory does not exist on disk: ${vault}`); + process.exit(2); + } process.stdout.write( JSON.stringify({ vault, journal, notes, configPath, configExists }, null, 2) + '\n' ); @@ -151,7 +117,6 @@ function main() { console.log(''); // Vault existence check - const vaultExists = fs.existsSync(vault); if (!vaultExists) { console.log(` ✗ Resolved vault directory does not exist on disk: ${vault}`); console.log(' Create it, or update jarvos.config.json / JARVOS_VAULT_DIR to point at your vault.'); diff --git a/modules/jarvos-secondbrain/tests/detect-vault.test.js b/modules/jarvos-secondbrain/tests/detect-vault.test.js new file mode 100644 index 00000000..85c46e3a --- /dev/null +++ b/modules/jarvos-secondbrain/tests/detect-vault.test.js @@ -0,0 +1,74 @@ +'use strict'; + +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { spawnSync } = require('node:child_process'); +const test = require('node:test'); + +const script = path.resolve(__dirname, '../scripts/detect-vault.js'); + +function runDetector(home, extraEnv = {}) { + const env = { ...process.env, HOME: home, ...extraEnv }; + for (const key of [ + 'CLAWD_DIR', + 'JARVOS_CLAWD_DIR', + 'JARVOS_CONFIG_FILE', + 'JARVOS_CONFIG_PATH', + 'JARVOS_JOURNAL_DIR', + 'JARVOS_NOTES_DIR', + 'JARVOS_VAULT_DIR', + 'JOURNAL_DIR', + 'VAULT_NOTES_DIR', + 'XDG_CONFIG_HOME', + ]) delete env[key]; + Object.assign(env, extraEnv); + return spawnSync(process.execPath, [script, '--json'], { encoding: 'utf8', env }); +} + +test('detect-vault uses the canonical shared resolver', (t) => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'detect-vault-')); + t.after(() => fs.rmSync(home, { recursive: true, force: true })); + const canonical = path.join(home, 'Vaults', 'Vault v3'); + fs.mkdirSync(canonical, { recursive: true }); + + const result = runDetector(home); + + assert.equal(result.status, 0, result.stderr); + assert.deepEqual(JSON.parse(result.stdout), { + vault: canonical, + journal: path.join(canonical, 'Journal'), + notes: path.join(canonical, 'Notes'), + configPath: path.join(home, 'clawd', 'jarvos.config.json'), + configExists: false, + }); +}); + +test('detect-vault JSON mode does not emit a nonexistent vault as usable', (t) => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'detect-vault-')); + t.after(() => fs.rmSync(home, { recursive: true, force: true })); + + const result = runDetector(home); + + assert.equal(result.status, 2); + assert.equal(result.stdout, ''); + assert.match(result.stderr, /does not exist on disk/); +}); + +test('detect-vault rejects an explicitly configured stale Documents vault', (t) => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'detect-vault-')); + t.after(() => fs.rmSync(home, { recursive: true, force: true })); + fs.mkdirSync(path.join(home, 'Vaults', 'Vault v3'), { recursive: true }); + fs.mkdirSync(path.join(home, 'clawd'), { recursive: true }); + fs.writeFileSync( + path.join(home, 'clawd', 'jarvos.config.json'), + JSON.stringify({ paths: { vault: '~/Documents/Vault v3' } }), + ); + + const result = runDetector(home); + + assert.equal(result.status, 1); + assert.equal(result.stdout, ''); + assert.match(result.stderr, /Refusing to use stale vault path/); +}); diff --git a/runtimes/hermes/skills/jarvos/SKILL.md b/runtimes/hermes/skills/jarvos/SKILL.md index 4987b1a9..e81cff62 100644 --- a/runtimes/hermes/skills/jarvos/SKILL.md +++ b/runtimes/hermes/skills/jarvos/SKILL.md @@ -95,13 +95,13 @@ paths in this order: 3. `~/.jarvos/config.json` `paths.journal` / `paths.notes` 4. Vault root from `JARVOS_VAULT_DIR` or `jarvos.config.json` `paths.vault`, with `Journal` / `Notes` appended -5. Default `~/Documents/Vault v3/Journal` and `~/Documents/Vault v3/Notes` +5. Canonical default `~/Vaults/Vault v3/Journal` and `~/Vaults/Vault v3/Notes` **If the user already uses OpenClaw with jarvOS**, they have a secondbrain vault configured. Hermes should use the **same vault** — not a separate one. ### To confirm vault config (ask the user once, then remember): -- Vault root: `$JARVOS_VAULT_DIR` or `~/Documents/Vault v3` +- Vault root: `$JARVOS_VAULT_DIR` or `~/Vaults/Vault v3` - Journal dir: `$JARVOS_JOURNAL_DIR`, `$JOURNAL_DIR`, or `/Journal` - Notes dir: `$JARVOS_NOTES_DIR`, `$VAULT_NOTES_DIR`, or `/Notes` @@ -119,8 +119,8 @@ configured. Hermes should use the **same vault** — not a separate one. ### Pitfall: do NOT invent a new vault path or journal file The whole point of shared-vault onboarding is that every runtime (OpenClaw, Hermes, -and any future runtime) uses one vault. If you're unsure, default to -`~/Documents/Vault v3` and ask the user to confirm rather than creating a new path. +and any future runtime) uses one vault. If you're unsure, run `detect-vault.js` +and ask the user to confirm its validated path rather than creating a new path. Do not create guessed daily journal files under `Notes/`; canonical journals live at `Journal/YYYY-MM-DD.md`.