From 97ac55214f9c3743990cb4109927a7551fcfffa5 Mon Sep 17 00:00:00 2001 From: yudongyouqing <142670774+yudongyouqing@users.noreply.github.com> Date: Sun, 4 Oct 2026 23:31:18 +0800 Subject: [PATCH] feat(contribute): target a learnings namespace with --namespace (#916) A directory reading several learnings namespaces always files into the shared root, so a team sharing a group namespace (e.g. payments) across projects loses each project's own namespace as a write target. contribute --namespace picks one, restricted to the namespaces this directory reads so a learning never lands where its author's recall would not find it; unknown values are refused with the valid list. Without the flag the routing is unchanged, and the ambiguous case now hints at the namespaces. projects list names the default destination and the accepted namespaces. Co-Authored-By: Claude Code --- docs/usage-guide.md | 6 +- docs/usage-guide.zh-CN.md | 4 +- skill-data/core/references/commands.md | 1 + skill-data/share/SKILL.md | 3 + src/__tests__/contribute-namespace.test.ts | 216 +++++++++++++++++++++ src/__tests__/projects-cmd.test.ts | 58 +++++- src/contribute.ts | 67 ++++++- src/index.ts | 1 + src/projects-cmd.ts | 13 ++ 9 files changed, 357 insertions(+), 12 deletions(-) create mode 100644 src/__tests__/contribute-namespace.test.ts 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 ───────────────────────────────────────