diff --git a/src/cli.ts b/src/cli.ts index 00d9cbb..27a2c4c 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -17,10 +17,15 @@ const program = new Command(); program .name('agentage') - .description('The agentage CLI') + .description('The offline-first terminal client for agentage Memory') .version(VERSION) .option('--no-daemon', 'run memory verbs in-process instead of via the daemon'); +program.addHelpText( + 'after', + '\nNew here? Run: agentage vault add --local, or agentage setup to connect an account.' +); + program.hook('preAction', () => { if (program.opts().daemon === false) disableDaemon(); }); diff --git a/src/commands/memory.test.ts b/src/commands/memory.test.ts index 05e9c74..2b92cc3 100644 --- a/src/commands/memory.test.ts +++ b/src/commands/memory.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { MemoryClient } from '../lib/memory-client.js'; +import { translateEngineMessage } from '../lib/memory-client.js'; import { runDelete, runEdit, runList, runRead, runSearch, runWrite } from './memory.js'; const client = (): MemoryClient => ({ @@ -110,4 +111,60 @@ describe('memory command wiring', () => { expect(c.delete).toHaveBeenCalledWith('a.md', { vault: undefined }); expect(logs.join()).toContain('git history'); }); + + it('edit with no --old/--new/--body errors instead of a silent no-op', async () => { + const c = client(); + await expect(runEdit('a.md', {}, c)).rejects.toThrow( + 'specify --old/--new for a replacement or --body to overwrite' + ); + expect(c.edit).not.toHaveBeenCalled(); + }); + + it('write rejects invalid --frontmatter JSON with a friendly hint', async () => { + const c = client(); + await expect(runWrite('a.md', { body: 'x', frontmatter: '{bad' }, c)).rejects.toThrow( + /--frontmatter must be a JSON object, e\.g\. '\{"tags":\["x"\]\}'/ + ); + expect(c.write).not.toHaveBeenCalled(); + }); + + it('write rejects non-object --frontmatter JSON (array/scalar)', async () => { + const c = client(); + await expect(runWrite('a.md', { body: 'x', frontmatter: '[1,2]' }, c)).rejects.toThrow( + /must be a JSON object/ + ); + await expect(runWrite('a.md', { body: 'x', frontmatter: '42' }, c)).rejects.toThrow( + /must be a JSON object/ + ); + expect(c.write).not.toHaveBeenCalled(); + }); + + it('write with no --body on a TTY errors instead of blocking on stdin', async () => { + const c = client(); + const original = process.stdin.isTTY; + Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); + try { + await expect(runWrite('a.md', {}, c)).rejects.toThrow( + 'provide --body, or pipe content on stdin' + ); + } finally { + Object.defineProperty(process.stdin, 'isTTY', { value: original, configurable: true }); + } + expect(c.write).not.toHaveBeenCalled(); + }); +}); + +describe('engine error translation', () => { + it('rewrites the memory__list MCP vocabulary to a CLI command', () => { + const engine = + 'Unknown vault "@nosuchvault". Use memory__list with no folder to see available vaults.'; + const out = translateEngineMessage(engine); + expect(out).not.toContain('memory__list'); + expect(out).toContain('Run `agentage vault list` to see available vaults.'); + expect(out).toContain('Unknown vault "@nosuchvault".'); + }); + + it('passes through messages with no known engine pattern unchanged', () => { + expect(translateEngineMessage('not found: a.md')).toBe('not found: a.md'); + }); }); diff --git a/src/commands/memory.ts b/src/commands/memory.ts index 015f89b..8800005 100644 --- a/src/commands/memory.ts +++ b/src/commands/memory.ts @@ -3,7 +3,11 @@ import { type Command } from 'commander'; import { type TreeEntry } from '@agentage/memory-core'; import { ensureDaemon } from '../lib/daemon-client.js'; import { daemonDisabled } from '../lib/daemon-pref.js'; -import { createDirectClient, type MemoryClient } from '../lib/memory-client.js'; +import { + createDirectClient, + translateEngineMessage, + type MemoryClient, +} from '../lib/memory-client.js'; import { loadVaultsConfig } from '../lib/vaults.js'; // Default engine path (DO3/DO4): the daemon - single writer, autostarted - when reachable; the @@ -21,6 +25,21 @@ const readStdin = async (): Promise => { return Buffer.concat(chunks).toString('utf-8'); }; +// --frontmatter must be a JSON object; a raw parser error or a non-object value is unfriendly. +const parseFrontmatter = (raw?: string): Record | undefined => { + if (!raw) return undefined; + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch (err) { + const detail = err instanceof Error ? err.message : String(err); + throw new Error(`--frontmatter must be a JSON object, e.g. '{"tags":["x"]}': ${detail}`); + } + if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) + throw new Error(`--frontmatter must be a JSON object, e.g. '{"tags":["x"]}'`); + return parsed as Record; +}; + const emit = (json: boolean, data: unknown, human: () => void): void => { if (json) console.log(JSON.stringify(data, null, 2)); else human(); @@ -67,10 +86,11 @@ export const runWrite = async ( client?: MemoryClient ): Promise => { const c = client ?? (await resolveClient()); - const body = opts.body !== undefined && opts.body !== '-' ? opts.body : await readStdin(); - const frontmatter = opts.frontmatter - ? (JSON.parse(opts.frontmatter) as Record) - : undefined; + const inlineBody = opts.body !== undefined && opts.body !== '-'; + if (!inlineBody && process.stdin.isTTY) + throw new Error('provide --body, or pipe content on stdin'); + const body = inlineBody ? opts.body! : await readStdin(); + const frontmatter = parseFrontmatter(opts.frontmatter); const out = await c.write(ref, body, { vault: opts.vault, frontmatter }); emit(opts.json ?? false, out, () => console.log(chalk.green(`Wrote ${out.path}`))); }; @@ -81,6 +101,8 @@ export const runEdit = async ( client?: MemoryClient ): Promise => { const c = client ?? (await resolveClient()); + if (opts.old === undefined && opts.body === undefined) + throw new Error('specify --old/--new for a replacement or --body to overwrite'); const op = opts.old !== undefined ? { mode: 'str_replace' as const, old_str: opts.old, new_str: opts.new ?? '' } @@ -117,7 +139,8 @@ export const runDelete = async ( const guard = (fn: () => Promise): Promise => fn().catch((err: unknown) => { - console.error(chalk.red(err instanceof Error ? err.message : String(err))); + const message = translateEngineMessage(err instanceof Error ? err.message : String(err)); + console.error(chalk.red(message)); process.exitCode = 1; }); diff --git a/src/commands/vault.ts b/src/commands/vault.ts index ce3b7c7..5abc2d9 100644 --- a/src/commands/vault.ts +++ b/src/commands/vault.ts @@ -112,7 +112,7 @@ export const runVaultList = (opts: { json?: boolean }, deps: VaultDeps = default return; } if (names.length === 0) { - deps.log('No vaults registered. Add one with `agentage vault add `.'); + deps.log('No vaults registered. Add one with `agentage vault add --local`.'); return; } for (const name of names) { diff --git a/src/lib/file-lock.test.ts b/src/lib/file-lock.test.ts index 005b955..49922a1 100644 --- a/src/lib/file-lock.test.ts +++ b/src/lib/file-lock.test.ts @@ -1,5 +1,13 @@ import { execFile } from 'node:child_process'; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { + chmodSync, + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { pathToFileURL } from 'node:url'; @@ -42,6 +50,41 @@ describe('file lock', () => { releaseFileLock(target); }); + // chmod-based unwritability does not apply to root, so skip these there. + const asRoot = typeof process.getuid === 'function' && process.getuid() === 0; + + it.skipIf(asRoot)('fails fast on a permission error instead of treating it as contention', () => { + // An unwritable lock dir: writeFileSync({flag:'wx'}) throws EACCES/EPERM, which never clears. + const roDir = join(dir, 'readonly'); + mkdirSync(roDir); + const roTarget = join(roDir, 'data.json'); + chmodSync(roDir, 0o500); + try { + expect(() => acquireFileLock(roTarget)).toThrow(/permission denied/); + } finally { + chmodSync(roDir, 0o700); + } + }); + + it.skipIf(asRoot)( + 'withFileLock surfaces a permission error immediately, never spinning MAX_WAIT_MS', + async () => { + const roDir = join(dir, 'readonly2'); + mkdirSync(roDir); + const roTarget = join(roDir, 'data.json'); + chmodSync(roDir, 0o500); + const started = Date.now(); + try { + await expect(withFileLock(roTarget, () => 'unreachable')).rejects.toThrow( + /permission denied/ + ); + expect(Date.now() - started).toBeLessThan(2_000); + } finally { + chmodSync(roDir, 0o700); + } + } + ); + it('releases idempotently', () => { acquireFileLock(target); releaseFileLock(target); diff --git a/src/lib/file-lock.ts b/src/lib/file-lock.ts index 465ca4a..da398a4 100644 --- a/src/lib/file-lock.ts +++ b/src/lib/file-lock.ts @@ -64,7 +64,18 @@ export const acquireFileLock = (target: string, now: number = Date.now()): boole try { writeFileSync(lockPath(target), `${process.pid} ${now}`, { flag: 'wx' }); return true; - } catch { + } catch (err) { + // Only EEXIST means contention. A permission/read-only error never clears, so failing fast + // beats burning the full MAX_WAIT_MS before reporting a generic "could not acquire lock". + const code = (err as NodeJS.ErrnoException).code; + if (code !== 'EEXIST') { + const path = lockPath(target); + throw new Error( + code === 'EACCES' || code === 'EPERM' || code === 'EROFS' + ? `permission denied: ${path}` + : `could not write lock file ${path}${code ? ` (${code})` : ''}` + ); + } const held = heldAt(lockPath(target)); if (held !== null && now - held < LOCK_TTL_MS) return false; if (!takeOverStale(target, now)) return false; diff --git a/src/lib/memory-client.ts b/src/lib/memory-client.ts index 35a139b..7dc3d73 100644 --- a/src/lib/memory-client.ts +++ b/src/lib/memory-client.ts @@ -19,6 +19,21 @@ export interface DeleteResult { deleted: boolean; } +// Engine (memory-core) messages leak MCP tool vocabulary (`memory__list`, ...) unfit for a terminal. +// Rewrite the known patterns to CLI vocabulary at this seam; do not fork the engine. +const ENGINE_MESSAGE_MAP: ReadonlyArray = [ + [ + /Use memory__list with no folder to see available vaults\.?/g, + 'Run `agentage vault list` to see available vaults.', + ], +]; + +export const translateEngineMessage = (message: string): string => + ENGINE_MESSAGE_MAP.reduce( + (m, [pattern, replacement]) => m.replace(pattern, replacement), + message + ); + export interface VerbOptions { vault?: string; }