diff --git a/docs/usage-guide.md b/docs/usage-guide.md index d16b5ba08..7e505697e 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -324,7 +324,11 @@ explicit `--project` skips the picker. as before; existing flat `learnings/*.md` stay shared with everyone (zero migration). - **`teamai contribute`** lands a learning under the active project's subdirectory - when exactly one project is active, otherwise at the shared root. + when exactly one project is active, otherwise at the shared root. When several + learnings namespaces are active, `--namespace ` files it under one of them — + restricted to the namespaces this directory reads, so a learning never lands + where its author's `recall` would not find it. `teamai projects list` shows the + default destination and the accepted namespaces. `manifest/projects.yaml` example: diff --git a/docs/usage-guide.zh-CN.md b/docs/usage-guide.zh-CN.md index 498229d9e..35f52f2e3 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -280,7 +280,9 @@ cd ~/work/billing && teamai init --project billing - **向后兼容。** 没有 `manifest/projects.yaml` 的仓库行为与之前完全一致;现存扁平 的 `learnings/*.md` 继续对所有人共享(零迁移)。 - **`teamai contribute`** 在恰好激活一个项目时,把经验落到该项目子目录,否则落到 - 共享的根目录。 + 共享的根目录。激活了多个 learnings 命名空间时,`--namespace ` 可以把经验落到 + 其中一个——只接受本目录会读取的命名空间,经验不会落到作者 `recall` 不到的地方。 + `teamai projects list` 会显示默认落点与可选命名空间。 `manifest/projects.yaml` 示例: diff --git a/skill-data/core/references/commands.md b/skill-data/core/references/commands.md index 0cc5ed3eb..f522cc7c7 100644 --- a/skill-data/core/references/commands.md +++ b/skill-data/core/references/commands.md @@ -300,6 +300,7 @@ Generated: do not edit by hand. Regenerate with - `--title ` — Title for the contribution document - `--session-id <id>` — Session ID for dedup tracking - `--scope <scope>` — Target scope: user or project + - `--namespace <ns>` — File the learning under this learnings namespace; must be one this directory reads (`teamai projects list` shows them) ## recall diff --git a/skill-data/share/SKILL.md b/skill-data/share/SKILL.md index 12fb76c1f..af24eda5d 100644 --- a/skill-data/share/SKILL.md +++ b/skill-data/share/SKILL.md @@ -31,6 +31,9 @@ URLs, paths and code identifiers stay as they are. - Pitfalls and things to watch out for 3. **Save it**: write the document to a temporary file 4. **Push it to the team**: run `teamai contribute --file <path> --title "<title>"` + — if `teamai projects list` shows several learnings namespaces for this + directory, pass `--namespace <ns>` (one it lists) to file the learning under + one of them instead of the shared root ## Document Template diff --git a/src/__tests__/contribute-namespace.test.ts b/src/__tests__/contribute-namespace.test.ts new file mode 100644 index 000000000..a64420c5b --- /dev/null +++ b/src/__tests__/contribute-namespace.test.ts @@ -0,0 +1,216 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import os from 'node:os'; + +// ─── contribute --namespace (#916) ────────────────────────── +// A directory that reads several learnings namespaces gets a way to choose +// one for a contribution, instead of always landing in the shared root. + +function writeManifest(repoDir: string, learnings: Record<string, string[]>): void { + const projects = Object.entries(learnings) + .map(([id, ns]) => ` - id: ${id}\n resources: { learnings: [${ns.join(', ')}] }`) + .join('\n'); + fs.mkdirSync(path.join(repoDir, 'manifest'), { recursive: true }); + fs.writeFileSync( + path.join(repoDir, 'manifest', 'projects.yaml'), + `version: 1\nprojects:\n${projects}\n`, + 'utf-8', + ); +} + +describe('resolveLearningsDestination (#916)', () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-contribute-dest-')); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + function localConfigFor(projects: string[]) { + return { + repo: { localPath: tmpDir }, + username: 'testuser', + projects, + } as never; + } + + it('routes to the single active namespace without a flag', async () => { + writeManifest(tmpDir, { 'svc-a': ['svc-a'] }); + const { resolveLearningsDestination } = await import('../contribute.js'); + const dest = await resolveLearningsDestination(localConfigFor(['svc-a'])); + expect(dest).toEqual({ subdir: 'svc-a', namespaces: ['svc-a'] }); + }); + + it('falls back to the shared root when several namespaces are active', async () => { + writeManifest(tmpDir, { 'svc-a': ['svc-a'], payments: ['payments'] }); + const { resolveLearningsDestination } = await import('../contribute.js'); + const dest = await resolveLearningsDestination(localConfigFor(['svc-a', 'payments'])); + expect(dest).toEqual({ subdir: '', namespaces: ['svc-a', 'payments'] }); + }); + + it('routes a --namespace flag to the requested namespace', async () => { + writeManifest(tmpDir, { 'svc-a': ['svc-a'], payments: ['payments'] }); + const { resolveLearningsDestination } = await import('../contribute.js'); + const dest = await resolveLearningsDestination(localConfigFor(['svc-a', 'payments']), 'payments'); + expect(dest).toEqual({ subdir: 'payments', namespaces: ['svc-a', 'payments'] }); + }); + + it('accepts a namespace that differs from the project id', async () => { + writeManifest(tmpDir, { alpha: ['alpha-notes'] }); + const { resolveLearningsDestination } = await import('../contribute.js'); + const dest = await resolveLearningsDestination(localConfigFor(['alpha']), 'alpha-notes'); + expect(dest).toEqual({ subdir: 'alpha-notes', namespaces: ['alpha-notes'] }); + }); + + it('refuses a namespace this directory does not read, listing the valid ones', async () => { + writeManifest(tmpDir, { 'svc-a': ['svc-a'], payments: ['payments'] }); + const { resolveLearningsDestination } = await import('../contribute.js'); + await expect( + resolveLearningsDestination(localConfigFor(['svc-a', 'payments']), 'nope'), + ).rejects.toThrow('Unknown learnings namespace "nope". Valid namespaces: svc-a, payments'); + }); + + it('refuses any flag value when no namespace is active', async () => { + writeManifest(tmpDir, { 'svc-a': ['svc-a'] }); + const { resolveLearningsDestination } = await import('../contribute.js'); + await expect( + resolveLearningsDestination(localConfigFor([]), 'svc-a'), + ).rejects.toThrow('reads no learnings namespace'); + }); +}); + +describe('contribute --namespace (#916)', () => { + let tmpDir: string; + let repoDir: string; + const originalHome = process.env.HOME; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-contribute-ns-')); + process.env.HOME = tmpDir; + // One level in: the queue sits NEXT TO the repo checkout, and a repo at + // the tmpDir root would put it in the shared OS temp dir. + repoDir = path.join(tmpDir, 'repo'); + writeManifest(repoDir, { 'svc-a': ['svc-a'], payments: ['payments'] }); + }); + + afterEach(() => { + process.env.HOME = originalHome; + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + function mockConfig(projects: string[]) { + // The pure-function tests above import contribute.js against the real + // config; drop the registry so this import re-resolves it to the mock. + vi.resetModules(); + vi.doMock('../config.js', () => ({ + requireInit: vi.fn().mockResolvedValue({ + localConfig: { + repo: { localPath: repoDir, remote: 'https://example.com/team/repo.git', kind: 'git' }, + username: 'testuser', + projects, + scope: 'user', + }, + teamConfig: {}, + }), + detectProjectConfig: vi.fn().mockResolvedValue(null), + })); + // The queue write checks the install's own config still names this install + // as the queue's owner; without it every write is refused as stale. + fs.mkdirSync(path.join(tmpDir, '.teamai'), { recursive: true }); + fs.writeFileSync( + path.join(tmpDir, '.teamai', 'config.yaml'), + `repo:\n localPath: ${repoDir}\n remote: https://example.com/team/repo.git\n kind: git\nusername: testuser\nprojects: ${JSON.stringify(projects)}\n`, + 'utf-8', + ); + } + + function contentFile(): string { + const file = path.join(tmpDir, 'notes.md'); + fs.writeFileSync(file, '# Session Notes\nA payment routing insight.', 'utf-8'); + return file; + } + + // The queue sits next to the repo checkout, not inside it. + function queuedFiles(): string[] { + const queueDir = path.join(path.dirname(repoDir), 'pending-learnings'); + if (!fs.existsSync(queueDir)) return []; + return fs.readdirSync(queueDir, { recursive: true }) as string[]; + } + + function mockPublishNothing(): void { + vi.doMock('../utils/learnings-publish.js', () => ({ + publishQueuedLearnings: vi.fn().mockResolvedValue({ + published: [], lastError: null, refused: false, installChanged: null, + }), + })); + } + + it('files a learning under the requested namespace', async () => { + mockConfig(['svc-a', 'payments']); + mockPublishNothing(); + + const { contribute } = await import('../contribute.js'); + await contribute({ file: contentFile(), title: 'Payments insight', namespace: 'payments' }); + + expect(queuedFiles().some((f) => f.includes(`payments${path.sep}`))).toBe(true); + vi.doUnmock('../config.js'); + vi.doUnmock('../utils/learnings-publish.js'); + }); + + it('refuses an unknown namespace without queueing anything', async () => { + mockConfig(['svc-a', 'payments']); + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + const { contribute } = await import('../contribute.js'); + await contribute({ file: contentFile(), namespace: 'nope' }); + + // log.error prefixes its symbol; the message is the second argument. + expect(errorSpy).toHaveBeenCalledWith( + expect.anything(), + expect.stringContaining('Unknown learnings namespace "nope". Valid namespaces: svc-a, payments'), + ); + expect(queuedFiles()).toEqual([]); + + errorSpy.mockRestore(); + logSpy.mockRestore(); + vi.doUnmock('../config.js'); + }); + + it('hints at the namespaces when several are active and no flag is passed', async () => { + mockConfig(['svc-a', 'payments']); + mockPublishNothing(); + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + const { contribute } = await import('../contribute.js'); + await contribute({ file: contentFile() }); + + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining('several learnings namespaces (svc-a, payments)'), + ); + + logSpy.mockRestore(); + vi.doUnmock('../config.js'); + vi.doUnmock('../utils/learnings-publish.js'); + }); + + it('shows the namespace in the dry-run path', async () => { + mockConfig(['svc-a', 'payments']); + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + const { contribute } = await import('../contribute.js'); + await contribute({ + file: contentFile(), title: 'Payments insight', namespace: 'payments', dryRun: true, + }); + + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining('[dry-run] Would push: learnings/payments/')); + expect(queuedFiles()).toEqual([]); + + logSpy.mockRestore(); + vi.doUnmock('../config.js'); + }); +}); diff --git a/src/__tests__/projects-cmd.test.ts b/src/__tests__/projects-cmd.test.ts index b7eb521b0..da3667bf7 100644 --- a/src/__tests__/projects-cmd.test.ts +++ b/src/__tests__/projects-cmd.test.ts @@ -32,7 +32,7 @@ const logMocks = vi.hoisted(() => ({ })); vi.mock('../utils/logger.js', () => ({ log: logMocks })); -import { projectsAdd, projectsUpdate, projectsRemove } from '../projects-cmd.js'; +import { projectsAdd, projectsUpdate, projectsRemove, projectsList } from '../projects-cmd.js'; import { autoDetectInit } from '../config.js'; describe('projects add / update / remove (#756)', () => { @@ -262,3 +262,59 @@ describe('projects add / update / remove (#756)', () => { }); }); }); + +describe('projects list destination (#916)', () => { + let repoDir: string; + const logLines = () => logSpy.mock.calls.map((c) => String(c[0])).join('\n'); + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + beforeEach(async () => { + repoDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-projects-list-')); + vi.clearAllMocks(); + }); + + afterEach(async () => { + await fse.remove(repoDir); + }); + + async function mockActive(projects: string[]): Promise<void> { + vi.mocked(autoDetectInit).mockResolvedValue({ + localConfig: { + repo: { localPath: repoDir, remote: 'https://github.com/team/repo.git' }, + username: 'member', + projects, + scope: 'user', + }, + } as unknown as Awaited<ReturnType<typeof autoDetectInit>>); + await fse.ensureDir(path.join(repoDir, 'manifest')); + await fse.writeFile(path.join(repoDir, 'manifest', 'projects.yaml'), YAML.stringify({ + version: 1, + projects: [ + { id: 'svc-a', resources: { learnings: ['svc-a'] } }, + { id: 'payments', resources: { learnings: ['payments'] } }, + ], + })); + } + + it('names the single active namespace as the contribute destination', async () => { + await mockActive(['svc-a']); + await projectsList({}); + expect(logLines()).toContain('Contribute destination for this directory: learnings/svc-a/'); + }); + + it('shows the shared root and the --namespace escape hatch for several', async () => { + await mockActive(['svc-a', 'payments']); + await projectsList({}); + const lines = logLines(); + expect(lines).toContain('Contribute destination for this directory: learnings/ (shared root)'); + expect(lines).toContain( + 'Namespaces read here: svc-a, payments — pass `teamai contribute --namespace <ns>` to file under one.', + ); + }); + + it('shows the shared root when no namespace is active', async () => { + await mockActive([]); + await projectsList({}); + expect(logLines()).toContain('Contribute destination for this directory: learnings/ (shared root)'); + }); +}); diff --git a/src/contribute.ts b/src/contribute.ts index 952b01193..46438025a 100644 --- a/src/contribute.ts +++ b/src/contribute.ts @@ -14,6 +14,17 @@ import { isSafeNamespaceSegment } from './manifest-schema.js'; import type { GlobalOptions, LocalConfig } from './types.js'; import { getProjectSearchIndexPath, isSelfMode } from './types.js'; +/** + * Where a contribution lands, and every learnings namespace the directory + * reads — the same resolution `contribute` routes by and hints from. + */ +export interface LearningsDestination { + /** The learnings-relative subdirectory: a namespace name, or '' for the shared root. */ + subdir: string; + /** All namespaces the active projects read; `--namespace` must be one of them. */ + namespaces: string[]; +} + /** * Decide which learnings subdirectory a contribution lands in — resolved from * the manifest's `resources.learnings`, the SAME mapping `pull` indexes by (NOT @@ -24,15 +35,34 @@ import { getProjectSearchIndexPath, isSelfMode } from './types.js'; * - Zero (no project, or the active projects declare no learnings namespace) → * the shared root (empty string). * - Multiple active learnings namespaces → the shared root, because the - * contribution's ownership is ambiguous; a member on several projects can still - * target one explicitly by contributing from that project's directory. This - * favors the safe default (visible to all) over silently guessing a namespace. + * contribution's ownership is ambiguous; `--namespace` targets one of them + * explicitly, restricted to what this directory reads so a learning never + * lands somewhere its author's `recall` would not find it (#916). This favors + * the safe default (visible to all) over silently guessing a namespace. */ -export async function resolveLearningsSubdir(localConfig: LocalConfig): Promise<string> { +export async function resolveLearningsDestination( + localConfig: LocalConfig, + requestedNamespace?: string, +): Promise<LearningsDestination> { const namespaces = await resolveActiveLearningsNamespaces( localConfig.repo.localPath, localConfig.projects ?? [], ); + + if (requestedNamespace !== undefined) { + // Refuse an unknown namespace rather than fall back to the shared root: the + // caller asked for a specific destination, and landing elsewhere would hide + // the mistake while still publishing the learning. + if (!namespaces.includes(requestedNamespace)) { + throw new Error( + namespaces.length === 0 + ? 'This directory reads no learnings namespace, so --namespace has nothing to target.' + : `Unknown learnings namespace "${requestedNamespace}". Valid namespaces: ${namespaces.join(', ')}`, + ); + } + return { subdir: requestedNamespace, namespaces }; + } + const sub = namespaces.length === 1 ? namespaces[0] : ''; // Defense-in-depth: the namespace is a path component here. It is validated at // the manifest boundary, but refuse anything that isn't a safe single segment @@ -40,7 +70,12 @@ export async function resolveLearningsSubdir(localConfig: LocalConfig): Promise< if (sub && !isSafeNamespaceSegment(sub)) { throw new Error(`Invalid learnings namespace "${sub}": must not contain path separators or '..'`); } - return sub; + return { subdir: sub, namespaces }; +} + +/** The subdirectory alone — for callers that only route, never hint. */ +export async function resolveLearningsSubdir(localConfig: LocalConfig): Promise<string> { + return (await resolveLearningsDestination(localConfig)).subdir; } /** @@ -146,7 +181,7 @@ export function generateFilename(title?: string): string { * dealt with. */ export async function contribute( - options: GlobalOptions & { file?: string; title?: string; sessionId?: string; scope?: string }, + options: GlobalOptions & { file?: string; title?: string; sessionId?: string; scope?: string; namespace?: string }, ): Promise<void> { // Validate file if (!options.file) { @@ -189,9 +224,23 @@ export async function contribute( const filename = generateFilename(options.title); // Route into an active-project subdir when there is exactly one, else the - // shared root. `relPath` is the learnings-relative path used everywhere. - const learningsSubdir = await resolveLearningsSubdir(localConfig); - const relPath = learningsSubdir ? path.posix.join(learningsSubdir, filename) : filename; + // shared root; --namespace targets one of the namespaces this directory + // reads (#916). `relPath` is the learnings-relative path used everywhere. + let destination: LearningsDestination; + try { + destination = await resolveLearningsDestination(localConfig, options.namespace); + } catch (e) { + log.error((e as Error).message); + log.info('Run `teamai projects list` to see the learnings namespaces this directory reads.'); + return; + } + if (destination.subdir === '' && destination.namespaces.length > 1) { + log.info( + `This directory reads several learnings namespaces (${destination.namespaces.join(', ')}); ` + + 'contributing to the shared root. Pass --namespace <ns> to file this under one of them.', + ); + } + const relPath = destination.subdir ? path.posix.join(destination.subdir, filename) : filename; if (options.dryRun) { log.info(`[dry-run] Would push: learnings/${relPath} (${content.length} bytes)`); diff --git a/src/index.ts b/src/index.ts index 0371f3bbc..f99f1daba 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1086,6 +1086,7 @@ program .option('--title <title>', 'Title for the contribution document') .option('--session-id <id>', 'Session ID for dedup tracking') .option('--scope <scope>', 'Target scope: user or project') + .option('--namespace <ns>', 'File the learning under this learnings namespace; must be one this directory reads (`teamai projects list` shows them)') .action(async (cmdOpts) => { const globalOpts = program.opts() as GlobalOptions; const { contribute } = await import('./contribute.js'); diff --git a/src/projects-cmd.ts b/src/projects-cmd.ts index 1bc05055b..f5158d810 100644 --- a/src/projects-cmd.ts +++ b/src/projects-cmd.ts @@ -15,6 +15,7 @@ import { listProjectIds, unknownProjectMessage, PROJECT_RESOURCE_TYPES, + resolveActiveLearningsNamespaces, } from './projects.js'; import type { ProjectsManifest, TeamProject } from './projects.js'; import { pullLatest, runManifestEdit, pushManifestChange } from './manifest-edit.js'; @@ -76,6 +77,18 @@ export async function projectsList(_options: GlobalOptions): Promise<void> { } else { console.log('No active projects in this directory. Run `teamai projects set <id>` to set them.'); } + + // Where `contribute` files learnings from this directory: the single active + // namespace, or the shared root when none or several are active — several is + // exactly the case `--namespace` resolves (#916). + const learningsNamespaces = await resolveActiveLearningsNamespaces(repoPath, active); + const destination = learningsNamespaces.length === 1 + ? `learnings/${learningsNamespaces[0]}/` + : 'learnings/ (shared root)'; + console.log(`Contribute destination for this directory: ${destination}`); + if (learningsNamespaces.length > 1) { + console.log(`Namespaces read here: ${learningsNamespaces.join(', ')} — pass \`teamai contribute --namespace <ns>\` to file under one.`); + } } // ─── projects set ───────────────────────────────────────