diff --git a/src/lifecycle.ts b/src/lifecycle.ts index 26905bf..ff15080 100644 --- a/src/lifecycle.ts +++ b/src/lifecycle.ts @@ -1,8 +1,8 @@ import {spawn} from 'node:child_process'; import {closeSync, openSync} from 'node:fs'; -import {chmod, readFile, realpath, writeFile} from 'node:fs/promises'; +import {chmod, readdir, readFile, realpath, writeFile} from 'node:fs/promises'; import {platform} from 'node:os'; -import {dirname, join} from 'node:path'; +import {dirname, join, sep} from 'node:path'; import {stderr as processStderr, stdin as processStdin, stdout as processStdout} from 'node:process'; import {createInterface} from 'node:readline/promises'; import yaml from 'js-yaml'; @@ -32,7 +32,7 @@ import { repairStaleRecallIndex, summaryAutoGenerationDisabled, } from './index_repair.js'; -import {readSeedManifest} from './manifest.js'; +import {readSeedManifest, uriSegment} from './manifest.js'; import {removeMcpConfigs, removeMcpSnippets, resolveMcpClients, runMcpInstall} from './mcp.js'; import {maybeRunPostUpdateAfterRepair, readOpenVikingCliVersion} from './update.js'; import { @@ -74,8 +74,11 @@ import { httpGetText, isExecutable, isJsonObject, + isSummarySidecarUri, isTcpPortOpen, maybeRun, + memoryFrontmatterField, + memoryUriProjectSegment, parseJsonConfigObject, readFileIfExists, removePath, @@ -132,6 +135,7 @@ export async function collectDoctorChecks(config: RuntimeConfig, options: Doctor checks.push(...(await userAgentInstructionsChecks())); checks.push(await manifestCheck(config.manifestPath)); checks.push(await recallIndexFreshnessCheck(config)); + checks.push(await memoryProjectConsistencyCheck(config)); checks.push(await fileCheck(join(toolRoot(), '.threadnoteignore'), 'ignore file')); checks.push(await fileCheck(join(toolRoot(), 'config', 'ov.conf.template.json'), 'server config template')); checks.push(await fileCheck(join(toolRoot(), 'config', 'ovcli.conf.template.json'), 'cli config template')); @@ -946,6 +950,74 @@ async function recallIndexFreshnessCheck(config: RuntimeConfig): Promise { + const name = 'memory project consistency'; + const memoriesRoot = join( + config.agentContextHome, + 'data', + 'viking', + config.account, + 'user', + uriSegment(config.user), + 'memories', + ); + try { + let entries: string[]; + try { + entries = await readdir(memoriesRoot, {recursive: true}); + } catch { + return {name, status: 'ok', detail: 'no memories directory yet'}; + } + const mismatches: string[] = []; + let checked = 0; + for (const entry of entries) { + if (!entry.endsWith('.md') || isSummarySidecarUri(entry)) { + continue; + } + const uri = `viking://user/${uriSegment(config.user)}/memories/${entry.split(sep).join('/')}`; + const pathProject = memoryUriProjectSegment(uri); + if (!pathProject) { + continue; + } + let content: string; + try { + content = await readFile(join(memoriesRoot, entry), 'utf8'); + } catch { + // Removed mid-walk (concurrent forget/compact/archive) or transiently + // unreadable — skip this file rather than aborting the whole check. + continue; + } + checked += 1; + const frontProject = memoryFrontmatterField(content, 'project'); + if (frontProject && uriSegment(frontProject) !== pathProject) { + mismatches.push(`${uri} (frontmatter "${frontProject}" vs path "${pathProject}")`); + } + } + if (mismatches.length === 0) { + return {name, status: 'ok', detail: `${checked} project-scoped memories consistent`}; + } + const sample = mismatches.slice(0, 3).join('; '); + const extra = Math.max(0, mismatches.length - 3); + return { + name, + status: 'warn', + detail: + `${mismatches.length} memory(ies) whose frontmatter project differs from their storage path; ` + + `re-store under the correct project to fix: ${sample}${extra > 0 ? `, +${extra} more` : ''}`, + }; + } catch (err: unknown) { + return {name, status: 'warn', detail: errorMessage(err)}; + } +} + async function fileCheck(path: string, label: string): Promise { return (await exists(path)) ? {name: label, status: 'ok', detail: path} diff --git a/src/memory.ts b/src/memory.ts index e42df83..2fde99e 100644 --- a/src/memory.ts +++ b/src/memory.ts @@ -873,6 +873,24 @@ async function storeMemory(config: RuntimeConfig, options: StoreMemoryOptions): } } +/** + * Warn when an in-place shared replacement was asked to change the memory's + * project — that is fixed by the storage path, so the request is ignored to keep + * frontmatter and path consistent (the divergence the doctor check flags). The + * caller's value is normalized via `uriSegment` to match how the path segment + * was produced. Topic is left to the existing caller-wins behavior: it is not a + * consistency-checked field and the path-derived topic can be a raw multi-segment + * value (`a/b`) that must not be slugged into or persisted from here. + */ +function warnOnSharedProjectDrift(metadata: MemoryMetadata, inferred: {readonly project?: string} | undefined): void { + if (inferred?.project && metadata.project && uriSegment(metadata.project) !== inferred.project) { + console.log( + `WARN keeping shared memory project "${inferred.project}" from its storage path; ignoring requested "${metadata.project}". ` + + `To change a shared memory's project, forget it and store a new one under the new project.`, + ); + } +} + async function storeSharedMemoryReplacement( config: RuntimeConfig, ov: string, @@ -888,9 +906,16 @@ async function storeSharedMemoryReplacement( } const team = await resolveTeam(config, teamName); const inferred = sharedMemoryUriParts(config, targetUri); + // The file is updated in place at targetUri, so its frontmatter project must + // match the path it lives under; otherwise recall's project scoping and the + // doctor consistency check disagree with the file's real location. Prefer the + // path's project over a differing caller value and warn — changing a shared + // memory's project means relocating it (forget + store anew), not editing the + // frontmatter of the file at the old path. Topic keeps caller-wins semantics. + warnOnSharedProjectDrift(options.metadata, inferred); const metadata: MemoryMetadata = { ...options.metadata, - project: options.metadata.project ?? inferred?.project, + project: inferred?.project ?? options.metadata.project, topic: options.metadata.topic ?? inferred?.topic, }; const rawMemory = formatMemoryDocument(options.title, metadata, options.bodyText); diff --git a/src/utils.ts b/src/utils.ts index 6dd1aea..5dde26d 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -947,6 +947,49 @@ export function isExcludedRecallUri(uri: string): boolean { return isSummarySidecarUri(uri) || isAgentArtifactPackUri(uri); } +/** + * The project directory segment a memory URI is stored under, or undefined for + * kinds that carry no project (preferences, smoke), a directory node, or a + * non-memory URI. Shared team memories (`.../memories/shared//...`) are + * de-scoped to their underlying kind first, so both personal and shared layouts + * resolve the same way. Used by the doctor project-consistency check to compare + * a memory's storage location against its frontmatter `project`. + */ +export function memoryUriProjectSegment(uri: string): string | undefined { + const marker = '/memories/'; + const at = uri.indexOf(marker); + if (at < 0) { + return undefined; + } + let segments = stripAnchor(uri) + .slice(at + marker.length) + .split('/') + .filter(Boolean); + if (segments[0] === 'shared') { + segments = segments.slice(2); // drop "shared" and the team name + } + // durable/handoffs/incidents store ///; + // require a file after the project segment so directory nodes are ignored. + const [kind, , project, ...rest] = segments; + if ((kind === 'durable' || kind === 'handoffs' || kind === 'incidents') && project && rest.length > 0) { + return project; + } + return undefined; +} + +/** + * Read a single frontmatter field (e.g. `project`) from a memory document — the + * value after `: ` on its own line within the leading header block (the + * text before the first blank line). Returns undefined when the field is absent. + */ +export function memoryFrontmatterField(content: string, field: string): string | undefined { + const blankLine = content.indexOf('\n\n'); + const header = blankLine < 0 ? content : content.slice(0, blankLine); + // `[ \t]*` (not `\s*`) so the value cannot span into the next header line. + const match = header.match(new RegExp(`^${escapeRegExp(field)}:[ \\t]*(.+)$`, 'm')); + return match?.[1]?.trim() || undefined; +} + /** * Extract the matched resource URIs from `ov grep --output json` stdout, minus * summary sidecars. The CLI prints a `cmd: ...` banner before the JSON, so diff --git a/test/unit/lifecycle.doctor.test.ts b/test/unit/lifecycle.doctor.test.ts new file mode 100644 index 0000000..16524b8 --- /dev/null +++ b/test/unit/lifecycle.doctor.test.ts @@ -0,0 +1,107 @@ +import {mkdir, mkdtemp, rm, writeFile} from 'node:fs/promises'; +import {tmpdir} from 'node:os'; +import {dirname, join} from 'node:path'; +import {afterEach, describe, expect, it} from 'vitest'; +import {memoryProjectConsistencyCheck} from '../../src/lifecycle.js'; +import type {RuntimeConfig} from '../../src/types.js'; + +const homes: string[] = []; + +afterEach(async () => { + await Promise.all(homes.splice(0).map(home => rm(home, {force: true, recursive: true}))); +}); + +async function makeConfig(): Promise { + const home = await mkdtemp(join(tmpdir(), 'threadnote-doctor-')); + homes.push(home); + return { + account: 'local', + agentContextHome: home, + agentId: 'threadnote', + host: '127.0.0.1', + manifestPath: join(home, 'manifest.json'), + openVikingVersion: '0.0.0', + port: 1933, + user: 'denyskashkovskyi', + }; +} + +function memoriesDir(config: RuntimeConfig): string { + return join(config.agentContextHome, 'data', 'viking', config.account, 'user', config.user, 'memories'); +} + +async function writeMemory(root: string, relPath: string, project: string | undefined): Promise { + const full = join(root, relPath); + await mkdir(dirname(full), {recursive: true}); + const header = [ + 'MEMORY', + 'kind: durable', + 'status: active', + project ? `project: ${project}` : undefined, + 'topic: t', + '', + 'Body.', + ] + .filter((line): line is string => line !== undefined) + .join('\n'); + await writeFile(full, header); +} + +describe('memoryProjectConsistencyCheck', () => { + it('is ok before any memories exist', async () => { + const config = await makeConfig(); + const check = await memoryProjectConsistencyCheck(config); + expect(check.status).toBe('ok'); + expect(check.detail).toMatch(/no memories directory/); + }); + + it('flags a memory whose frontmatter project differs from its storage path', async () => { + const config = await makeConfig(); + const root = memoriesDir(config); + await writeMemory(root, 'durable/projects/mobile-native/consistent.md', 'mobile-native'); + await writeMemory(root, 'durable/projects/general/no-project.md', undefined); // absent project → not flagged + await writeMemory(root, 'preferences/coding-style.md', 'whatever'); // project-less kind → skipped + await writeMemory(root, 'shared/docs-desktop/durable/projects/coda/pagerduty.md', 'mobile-native'); // drift + + const check = await memoryProjectConsistencyCheck(config); + expect(check.status).toBe('warn'); + expect(check.detail).toContain('shared/docs-desktop/durable/projects/coda/pagerduty.md'); + expect(check.detail).toContain('frontmatter "mobile-native" vs path "coda"'); + expect(check.detail).toMatch(/^1 memory/); // exactly one divergence + }); + + it('is ok when every project-scoped memory matches its path', async () => { + const config = await makeConfig(); + const root = memoriesDir(config); + await writeMemory(root, 'durable/projects/mobile-native/a.md', 'mobile-native'); + await writeMemory(root, 'handoffs/active/threadnote/b.md', 'threadnote'); + await writeMemory(root, 'incidents/active/coda/c.md', 'coda'); + const check = await memoryProjectConsistencyCheck(config); + expect(check.status).toBe('ok'); + expect(check.detail).toMatch(/3 project-scoped memories consistent/); + }); + + it('skips non-.md files, summary sidecars, and unreadable entries without flagging', async () => { + const config = await makeConfig(); + const root = memoriesDir(config); + await writeMemory(root, 'durable/projects/coda/real.md', 'coda'); // the only real memory + await writeMemory(root, 'durable/projects/coda/notes.txt', 'mobile-native'); // not .md → skipped + await writeMemory(root, 'durable/projects/coda/.overview.md', 'mobile-native'); // sidecar → skipped + await mkdir(join(root, 'durable/projects/coda/isdir.md'), {recursive: true}); // dir → readFile throws, skipped + const check = await memoryProjectConsistencyCheck(config); + expect(check.status).toBe('ok'); + expect(check.detail).toMatch(/1 project-scoped memories consistent/); + }); + + it('caps the reported sample and notes the remainder', async () => { + const config = await makeConfig(); + const root = memoriesDir(config); + for (const project of ['a', 'b', 'c', 'd']) { + await writeMemory(root, `durable/projects/${project}/m.md`, 'wrong'); + } + const check = await memoryProjectConsistencyCheck(config); + expect(check.status).toBe('warn'); + expect(check.detail).toMatch(/^4 memory/); + expect(check.detail).toContain('+1 more'); + }); +}); diff --git a/test/unit/memory.shared-replace.test.ts b/test/unit/memory.shared-replace.test.ts index fb04927..e0479ef 100644 --- a/test/unit/memory.shared-replace.test.ts +++ b/test/unit/memory.shared-replace.test.ts @@ -104,6 +104,55 @@ describe('remember shared replacement', () => { expect(output).not.toContain('memories/durable/projects/mobile-native/auth.md --from-file'); }); + it('keeps the project from the storage path when the caller requests a different one', async () => { + const config = await makeRuntime(); + homes.push(config.agentContextHome); + const logs: string[] = []; + vi.spyOn(console, 'log').mockImplementation((...args: readonly unknown[]) => { + logs.push(args.map(String).join(' ')); + }); + + const sharedUri = 'viking://user/denyskashkovskyi/memories/shared/default/durable/projects/mobile-native/auth.md'; + await runRemember(config, { + dryRun: true, + kind: 'durable', + project: 'coda', // differs from the path project (mobile-native) + replace: sharedUri, + sourceAgentClient: 'codex', + text: 'Updated shared auth memory.', + }); + + const output = logs.join('\n'); + // Frontmatter tracks the path, not the differing request — no divergence. + expect(output).toContain('project: mobile-native'); + expect(output).not.toContain('project: coda'); + expect(output).toContain('keeping shared memory project "mobile-native"'); + expect(output).toContain('ignoring requested "coda"'); + }); + + it('does not warn when the caller project matches the storage path', async () => { + const config = await makeRuntime(); + homes.push(config.agentContextHome); + const logs: string[] = []; + vi.spyOn(console, 'log').mockImplementation((...args: readonly unknown[]) => { + logs.push(args.map(String).join(' ')); + }); + + const sharedUri = 'viking://user/denyskashkovskyi/memories/shared/default/durable/projects/mobile-native/auth.md'; + await runRemember(config, { + dryRun: true, + kind: 'durable', + project: 'mobile-native', // matches the path project → no drift + replace: sharedUri, + sourceAgentClient: 'codex', + text: 'Updated shared auth memory.', + }); + + const output = logs.join('\n'); + expect(output).toContain('project: mobile-native'); + expect(output).not.toContain('keeping shared memory project'); + }); + it('rejects non-durable shared replacements', async () => { const config = await makeRuntime(); homes.push(config.agentContextHome); diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts index 774c849..b37f74e 100644 --- a/test/unit/utils.test.ts +++ b/test/unit/utils.test.ts @@ -22,6 +22,8 @@ import { globToRegExp, grepUrisFromJson, isAgentArtifactPackUri, + memoryFrontmatterField, + memoryUriProjectSegment, mergeRecallHits, parseRecallHits, RECALL_LOW_CONFIDENCE_NOTE, @@ -1016,6 +1018,59 @@ describe('categoryForUri', () => { }); }); +describe('memoryUriProjectSegment', () => { + const u = (path: string): string => `viking://user/me/memories/${path}`; + it('extracts the project for personal durable/handoff/incident memories', () => { + expect(memoryUriProjectSegment(u('durable/projects/mobile-native/auth.md'))).toBe('mobile-native'); + expect(memoryUriProjectSegment(u('durable/archived/mobile-native/auth.md'))).toBe('mobile-native'); + expect(memoryUriProjectSegment(u('handoffs/active/threadnote/foo.md'))).toBe('threadnote'); + expect(memoryUriProjectSegment(u('incidents/active/coda/bar.md'))).toBe('coda'); + }); + + it('de-scopes shared team memories to the underlying project', () => { + expect(memoryUriProjectSegment(u('shared/docs-desktop/durable/projects/coda/pagerduty.md'))).toBe('coda'); + }); + + it('returns undefined for project-less kinds, directory nodes, and non-memory URIs', () => { + expect(memoryUriProjectSegment(u('preferences/coding-style.md'))).toBeUndefined(); + expect(memoryUriProjectSegment(u('preferences/archived/old.md'))).toBeUndefined(); + expect(memoryUriProjectSegment(u('smoke/active/probe.md'))).toBeUndefined(); + expect(memoryUriProjectSegment(u('durable/projects/mobile-native'))).toBeUndefined(); // dir node, no file + expect(memoryUriProjectSegment('viking://resources/repos/coda/README.md')).toBeUndefined(); + }); + + it('ignores a chunk anchor', () => { + expect(memoryUriProjectSegment(u('durable/projects/coda/x.md#chunk_0001'))).toBe('coda'); + }); +}); + +describe('memoryFrontmatterField', () => { + const doc = ['MEMORY', 'kind: durable', 'status: active', 'project: mobile-native', 'topic: auth', '', 'Body.'].join( + '\n', + ); + it('reads a header field from the leading block', () => { + expect(memoryFrontmatterField(doc, 'project')).toBe('mobile-native'); + expect(memoryFrontmatterField(doc, 'topic')).toBe('auth'); + }); + it('returns undefined for an absent field and does not read the body', () => { + expect(memoryFrontmatterField(doc, 'repo')).toBeUndefined(); + expect(memoryFrontmatterField('MEMORY\nkind: durable\n\nproject: not-a-header', 'project')).toBeUndefined(); + }); + + it('treats a bare (empty-value) field as absent', () => { + expect(memoryFrontmatterField('MEMORY\nkind: durable\nproject:\ntopic: t\n\nb', 'project')).toBeUndefined(); + expect(memoryFrontmatterField('MEMORY\nproject: \n\nb', 'project')).toBeUndefined(); + }); + + it('does not match a field that is only a prefix of another key', () => { + expect(memoryFrontmatterField('MEMORY\nproject_id: coda\n\nb', 'project')).toBeUndefined(); + }); + + it('trims a trailing-whitespace value (memories round-trip through git)', () => { + expect(memoryFrontmatterField('MEMORY\nproject: coda \n\nb', 'project')).toBe('coda'); + }); +}); + describe('isAgentArtifactPackUri', () => { it('flags pack machinery but not shared skills or plain memories', () => { expect(