From 2db6fc6529ed465ad282eef24539b0adc4d98fc6 Mon Sep 17 00:00:00 2001 From: legend91019 <15853153143@163.com> Date: Mon, 19 Oct 2026 21:31:24 +0800 Subject: [PATCH 01/28] feat(push): place all distributed resources with project and role namespaces (#1101) --- .../push-all-resources-namespaces.test.ts | 289 ++++++++++++++++++ src/__tests__/push-namespaces.test.ts | 49 ++- .../workflow-cache-namespaces.test.ts | 212 +++++++++++++ src/manifest-schema.ts | 15 +- src/projects.ts | 4 +- src/push-namespaces.ts | 58 +++- src/push.ts | 28 +- src/resources/docs.ts | 12 +- src/resources/hooks.ts | 10 +- src/resources/index.ts | 13 +- src/resources/mcp.ts | 13 +- src/resources/repo-resource.ts | 178 +++++++++++ src/review-branch.ts | 19 +- src/types.ts | 37 ++- src/workflow/cache.ts | 123 +++++++- src/workflow/types.ts | 2 +- 16 files changed, 993 insertions(+), 69 deletions(-) create mode 100644 src/__tests__/push-all-resources-namespaces.test.ts create mode 100644 src/__tests__/workflow-cache-namespaces.test.ts create mode 100644 src/resources/repo-resource.ts diff --git a/src/__tests__/push-all-resources-namespaces.test.ts b/src/__tests__/push-all-resources-namespaces.test.ts new file mode 100644 index 000000000..337fc96e2 --- /dev/null +++ b/src/__tests__/push-all-resources-namespaces.test.ts @@ -0,0 +1,289 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { + isAtSharedRoot, + isPlaceableType, + NAMESPACE_AXIS, + PLACEABLE_TYPES, + placedResourcePath, + resolveProjectNamespace, + withNamespace, +} from '../push-namespaces.js'; +import type { ProjectsManifest } from '../projects.js'; +import type { LocalConfig, ResourceItem } from '../types.js'; +import { scanRepoResourceForPush, pushRepoResourceItem } from '../resources/repo-resource.js'; +import { getHandler } from '../resources/index.js'; + +const dirs: string[] = []; + +function makeTmpDir(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-push-all-ns-')); + dirs.push(dir); + return dir; +} + +afterEach(() => { + for (const d of dirs.splice(0)) { + fs.rmSync(d, { recursive: true, force: true }); + } +}); + +describe('all 14 placeable resource types and namespace axis mapping', () => { + it('covers all 14 distributed resource types in PLACEABLE_TYPES', () => { + const expected = [ + 'skills', + 'rules', + 'agents', + 'docs', + 'env', + 'hooks', + 'mcp', + 'models', + 'claudemd', + 'learnings', + 'wiki', + 'workflows', + 'prompts', + 'scorers', + ]; + expect([...PLACEABLE_TYPES].sort()).toEqual(expected.sort()); + for (const t of expected) { + expect(isPlaceableType(t)).toBe(true); + } + }); + + it('maps each placeable type to a valid manifest axis', () => { + expect(NAMESPACE_AXIS.skills).toBe('skills'); + expect(NAMESPACE_AXIS.rules).toBe('knowledge'); + expect(NAMESPACE_AXIS.agents).toBe('agents'); + expect(NAMESPACE_AXIS.docs).toBe('docs'); + expect(NAMESPACE_AXIS.env).toBe('env'); + expect(NAMESPACE_AXIS.hooks).toBe('hooks'); + expect(NAMESPACE_AXIS.mcp).toBe('mcp'); + expect(NAMESPACE_AXIS.models).toBe('models'); + expect(NAMESPACE_AXIS.claudemd).toBe('knowledge'); + expect(NAMESPACE_AXIS.learnings).toBe('learnings'); + expect(NAMESPACE_AXIS.wiki).toBe('wiki'); + expect(NAMESPACE_AXIS.workflows).toBe('workflows'); + expect(NAMESPACE_AXIS.prompts).toBe('prompts'); + expect(NAMESPACE_AXIS.scorers).toBe('scorers'); + }); +}); + +describe('isAtSharedRoot and withNamespace across all resource types', () => { + it('detects shared root for items without namespace segments', () => { + const sampleItems: Array<{ type: (typeof PLACEABLE_TYPES)[number]; path: string; shared: boolean }> = [ + { type: 'docs', path: 'docs/guide.md', shared: true }, + { type: 'docs', path: 'docs/team-a/guide.md', shared: false }, + { type: 'workflows', path: 'workflows/ci.yaml', shared: true }, + { type: 'workflows', path: 'workflows/team-a/ci.yaml', shared: false }, + { type: 'prompts', path: 'prompts/review.md', shared: true }, + { type: 'prompts', path: 'prompts/team-a/review.md', shared: false }, + { type: 'scorers', path: 'scorers/accuracy.py', shared: true }, + { type: 'scorers', path: 'scorers/team-a/accuracy.py', shared: false }, + { type: 'models', path: 'models/config.json', shared: true }, + { type: 'models', path: 'models/team-a/config.json', shared: false }, + { type: 'wiki', path: 'wiki/home.md', shared: true }, + { type: 'wiki', path: 'wiki/team-a/home.md', shared: false }, + { type: 'hooks', path: 'hooks/pre-commit.json', shared: true }, + { type: 'hooks', path: 'hooks/team-a/pre-commit.json', shared: false }, + { type: 'mcp', path: 'mcp/servers.json', shared: true }, + { type: 'mcp', path: 'mcp/team-a/servers.json', shared: false }, + { type: 'env', path: 'env/default.env', shared: true }, + { type: 'env', path: 'env/team-a/default.env', shared: false }, + ]; + + for (const { type, path: relPath, shared } of sampleItems) { + const item: ResourceItem = { + name: path.basename(relPath), + type, + sourcePath: `/local/${relPath}`, + relativePath: relPath, + }; + expect(isAtSharedRoot(item)).toBe(shared); + } + }); + + it('correctly inserts namespace segments for various resource formats', () => { + expect(withNamespace('docs/readme.md', 'core')).toBe('docs/core/readme.md'); + expect(withNamespace('workflows/build.yaml', 'ci')).toBe('workflows/ci/build.yaml'); + expect(withNamespace('prompts/codegen.txt', 'ai')).toBe('prompts/ai/codegen.txt'); + expect(withNamespace('scorers/metric.py', 'eval')).toBe('scorers/eval/metric.py'); + expect(withNamespace('models/deepseek.json', 'llm')).toBe('models/llm/deepseek.json'); + expect(withNamespace('wiki/architecture.md', 'docs')).toBe('wiki/docs/architecture.md'); + expect(withNamespace('hooks/pre-push.sh', 'git')).toBe('hooks/git/pre-push.sh'); + expect(withNamespace('mcp/postgres.json', 'db')).toBe('mcp/db/postgres.json'); + expect(withNamespace('env/prod.env', 'infra')).toBe('env/infra/prod.env'); + }); +}); + +describe('resolveProjectNamespace behavior across newly added axes', () => { + const manifest: ProjectsManifest = { + version: 1, + projects: [ + { + id: 'infra-project', + name: 'Infrastructure', + description: 'Infra project description', + resources: { + docs: ['infra-docs'], + workflows: ['infra-wf'], + prompts: ['infra-prompts'], + scorers: ['infra-scorers'], + env: ['infra-env'], + hooks: ['infra-hooks'], + mcp: ['infra-mcp'], + models: ['infra-models'], + knowledge: ['infra-know'], + skills: ['infra-skills'], + learnings: ['infra-learnings'], + wiki: ['infra-wiki'], + agents: ['infra-agents'], + }, + }, + { + id: 'empty-project', + name: 'Empty', + description: 'Empty project description', + resources: { + knowledge: [], + skills: [], + learnings: [], + agents: [], + }, + }, + { + id: 'multi-docs-project', + name: 'Multi Docs', + description: 'Multi docs description', + resources: { + knowledge: [], + skills: [], + learnings: [], + agents: [], + docs: ['docs-a', 'docs-b'], + }, + }, + ], + }; + + it('resolves namespace when single namespace declared on axis', () => { + expect(resolveProjectNamespace(manifest, 'infra-project', 'docs')).toEqual({ + ok: true, + namespace: 'infra-docs', + }); + expect(resolveProjectNamespace(manifest, 'infra-project', 'workflows')).toEqual({ + ok: true, + namespace: 'infra-wf', + }); + expect(resolveProjectNamespace(manifest, 'infra-project', 'prompts')).toEqual({ + ok: true, + namespace: 'infra-prompts', + }); + expect(resolveProjectNamespace(manifest, 'infra-project', 'scorers')).toEqual({ + ok: true, + namespace: 'infra-scorers', + }); + }); + + it('fails with clear error when axis has no namespaces declared', () => { + const res = resolveProjectNamespace(manifest, 'empty-project', 'workflows'); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.message).toContain('workflows'); + expect(res.message).toContain('empty-project'); + } + }); + + it('fails with clear error when axis has multiple namespaces declared', () => { + const res = resolveProjectNamespace(manifest, 'multi-docs-project', 'docs'); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.message).toContain('docs'); + expect(res.message).toContain('docs-a, docs-b'); + expect(res.message).toContain('--role'); + } + }); +}); + +describe('placedResourcePath lookup for rules and agents', () => { + it('looks up destination correctly when recorded', () => { + const ruleRecords = { + 'my-rule': 'rules/eng/my-rule.md', + }; + const agentRecords = { + 'my-agent': 'agents/eng/my-agent.yaml', + }; + expect(placedResourcePath(ruleRecords, 'rules', 'my-rule')).toBe('rules/eng/my-rule.md'); + expect(placedResourcePath(agentRecords, 'agents', 'my-agent')).toBe('agents/eng/my-agent.yaml'); + }); + + it('rejects malicious or escaping destinations', () => { + const records = { + 'my-rule': 'rules/../../etc/passwd', + 'other-rule': 'other/ci/my-rule.md', + }; + expect(placedResourcePath(records, 'rules', 'my-rule')).toBeNull(); + expect(placedResourcePath(records, 'rules', 'other-rule')).toBeNull(); + }); +}); + +describe('repo resource handler push and scan', () => { + it('handlers are registered for all new distributed resource types in getHandler()', () => { + const types = [ + 'docs', + 'hooks', + 'mcp', + 'models', + 'claudemd', + 'learnings', + 'wiki', + 'workflows', + 'prompts', + 'scorers', + ] as const; + + for (const t of types) { + const handler = getHandler(t); + expect(handler).toBeDefined(); + expect(typeof handler.scanLocalForPush).toBe('function'); + expect(typeof handler.pushItem).toBe('function'); + } + }); + + it('scanRepoResourceForPush and pushRepoResourceItem work with self-mode structure', async () => { + const root = makeTmpDir(); + const projectRoot = path.join(root, 'project'); + const repoLocal = path.join(root, 'team-repo'); + + fs.mkdirSync(projectRoot, { recursive: true }); + fs.mkdirSync(repoLocal, { recursive: true }); + + const localConfig: LocalConfig = { + repo: { localPath: repoLocal, remote: 'local', kind: 'self' }, + username: 'alice', + scope: 'project', + projectRoot, + additionalRoles: [], + }; + + // Create a new workflow in project .teamai/workflows/ops/test.yaml + const activeDir = path.join(projectRoot, '.teamai', 'workflows', 'ops'); + fs.mkdirSync(activeDir, { recursive: true }); + fs.writeFileSync(path.join(activeDir, 'test.yaml'), 'name: test\n'); + + const scanned = await scanRepoResourceForPush('workflows', 'workflows', {} as any, localConfig); + expect(scanned.length).toBe(1); + expect(scanned[0].name).toBe('ops/test.yaml'); + expect(scanned[0].relativePath).toBe('workflows/ops/test.yaml'); + expect(scanned[0].status).toBe('new'); + + // Now push it + await pushRepoResourceItem(scanned[0], {} as any, localConfig); + const destFile = path.join(repoLocal, 'workflows', 'ops', 'test.yaml'); + expect(fs.existsSync(destFile)).toBe(true); + expect(fs.readFileSync(destFile, 'utf8')).toBe('name: test\n'); + }); +}); diff --git a/src/__tests__/push-namespaces.test.ts b/src/__tests__/push-namespaces.test.ts index 5d887875f..528ef21b4 100644 --- a/src/__tests__/push-namespaces.test.ts +++ b/src/__tests__/push-namespaces.test.ts @@ -30,14 +30,17 @@ describe('isAtSharedRoot', () => { }); describe('isPlaceableType', () => { - it('accepts the namespaced types and rejects the rest', () => { - expect(isPlaceableType('skills')).toBe(true); - expect(isPlaceableType('rules')).toBe(true); - expect(isPlaceableType('agents')).toBe(true); - // env is pushable but never namespaced; docs and hooks are not placed either. - expect(isPlaceableType('env')).toBe(false); - expect(isPlaceableType('docs')).toBe(false); - expect(isPlaceableType('hooks')).toBe(false); + it('accepts all 14 placeable resource types and rejects invalid ones', () => { + const validTypes = [ + 'skills', 'rules', 'agents', 'docs', 'env', 'hooks', 'mcp', + 'models', 'claudemd', 'learnings', 'wiki', 'workflows', 'prompts', 'scorers', + ]; + for (const t of validTypes) { + expect(isPlaceableType(t)).toBe(true); + } + expect(isPlaceableType('unknown')).toBe(false); + expect(isPlaceableType('')).toBe(false); + expect(isPlaceableType('config')).toBe(false); }); }); @@ -45,6 +48,9 @@ describe('withNamespace', () => { it('inserts the namespace after the resource root, keeping the basename', () => { expect(withNamespace('rules/my-rule.md', 'knowledge-ns')).toBe('rules/knowledge-ns/my-rule.md'); expect(withNamespace('skills/my-skill', 'skills-ns')).toBe('skills/skills-ns/my-skill'); + expect(withNamespace('docs/guide.md', 'docs-ns')).toBe('docs/docs-ns/guide.md'); + expect(withNamespace('workflows/ci.yaml', 'wf-ns')).toBe('workflows/wf-ns/ci.yaml'); + expect(withNamespace('prompts/system.md', 'pr-ns')).toBe('prompts/pr-ns/system.md'); }); it('keeps an agent extension the caller does not know about', () => { @@ -62,7 +68,22 @@ const manifest: ProjectsManifest = { name: 'Front App', description: '', // Deliberately all different: the axis a type resolves from is the point. - resources: { knowledge: ['fe-know'], skills: ['fe-skills'], learnings: [], agents: ['fe-agents'] }, + resources: { + knowledge: ['fe-know'], + skills: ['fe-skills'], + learnings: ['fe-learnings'], + agents: ['fe-agents'], + docs: ['fe-docs'], + env: ['fe-env'], + hooks: ['fe-hooks'], + mcp: ['fe-mcp'], + models: ['fe-models'], + claudemd: ['fe-claudemd'], + wiki: ['fe-wiki'], + workflows: ['fe-workflows'], + prompts: ['fe-prompts'], + scorers: ['fe-scorers'], + }, }, { id: 'bare', @@ -91,6 +112,16 @@ describe('resolveProjectNamespace', () => { expect(resolveProjectNamespace(manifest, 'front-app', 'rules')).toEqual({ ok: true, namespace: 'fe-know' }); expect(resolveProjectNamespace(manifest, 'front-app', 'skills')).toEqual({ ok: true, namespace: 'fe-skills' }); expect(resolveProjectNamespace(manifest, 'front-app', 'agents')).toEqual({ ok: true, namespace: 'fe-agents' }); + expect(resolveProjectNamespace(manifest, 'front-app', 'docs')).toEqual({ ok: true, namespace: 'fe-docs' }); + expect(resolveProjectNamespace(manifest, 'front-app', 'env')).toEqual({ ok: true, namespace: 'fe-env' }); + expect(resolveProjectNamespace(manifest, 'front-app', 'hooks')).toEqual({ ok: true, namespace: 'fe-hooks' }); + expect(resolveProjectNamespace(manifest, 'front-app', 'mcp')).toEqual({ ok: true, namespace: 'fe-mcp' }); + expect(resolveProjectNamespace(manifest, 'front-app', 'models')).toEqual({ ok: true, namespace: 'fe-models' }); + expect(resolveProjectNamespace(manifest, 'front-app', 'claudemd')).toEqual({ ok: true, namespace: 'fe-know' }); + expect(resolveProjectNamespace(manifest, 'front-app', 'wiki')).toEqual({ ok: true, namespace: 'fe-wiki' }); + expect(resolveProjectNamespace(manifest, 'front-app', 'workflows')).toEqual({ ok: true, namespace: 'fe-workflows' }); + expect(resolveProjectNamespace(manifest, 'front-app', 'prompts')).toEqual({ ok: true, namespace: 'fe-prompts' }); + expect(resolveProjectNamespace(manifest, 'front-app', 'scorers')).toEqual({ ok: true, namespace: 'fe-scorers' }); }); it('fails naming the type and the axis when the project declares none', () => { diff --git a/src/__tests__/workflow-cache-namespaces.test.ts b/src/__tests__/workflow-cache-namespaces.test.ts new file mode 100644 index 000000000..925c741ed --- /dev/null +++ b/src/__tests__/workflow-cache-namespaces.test.ts @@ -0,0 +1,212 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { execFileSync } from 'node:child_process'; +import { afterEach, describe, expect, it } from 'vitest'; +import type { LocalConfig } from '../types.js'; +import { loadCachedWorkflow, syncWorkflowDefinitions } from '../workflow/cache.js'; + +const dirs: string[] = []; + +function testConfig(): LocalConfig { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-wf-ns-test-')); + dirs.push(root); + const repo = path.join(root, 'team-repo'); + fs.mkdirSync(path.join(repo, 'workflows'), { recursive: true }); + fs.mkdirSync(path.join(repo, 'prompts'), { recursive: true }); + return { + repo: { localPath: repo, remote: 'local', kind: 'self' }, + username: 'tester', + scope: 'project', + additionalRoles: [], + projectRoot: root, + dataHome: path.join(root, 'data'), + }; +} + +function git(cwd: string, ...args: string[]): string { + return execFileSync('git', args, { cwd, encoding: 'utf8' }).trim(); +} + +function initializeGit(repoDir: string): (msg: string) => string { + git(repoDir, 'init', '--initial-branch=main'); + git(repoDir, 'config', 'user.name', 'TeamAI Test'); + git(repoDir, 'config', 'user.email', 'test@example.com'); + return (msg: string) => { + git(repoDir, 'add', '.'); + git(repoDir, 'commit', '--allow-empty', '-m', msg); + return git(repoDir, 'rev-parse', 'HEAD'); + }; +} + +afterEach(() => { + for (const dir of dirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +const WORKFLOW_A = `schemaVersion: v1alpha1 +name: deploy +description: Deploy from namespace A. +steps: + - id: step-a + command: echo "deploying A" +`; + +const WORKFLOW_B = `schemaVersion: v1alpha1 +name: deploy +description: Deploy from namespace B. +steps: + - id: step-b + command: echo "deploying B" +`; + +const SHARED_WORKFLOW = `schemaVersion: v1alpha1 +name: deploy +description: Deploy from shared root. +steps: + - id: step-shared + command: echo "deploying shared" +`; + +describe('workflow and prompt namespaces in cache', () => { + it('discovers and loads namespaced workflows from workflows//.yaml', async () => { + const config = testConfig(); + const repo = config.repo.localPath; + const commit = initializeGit(repo); + + // Setup active namespaces manifest + fs.mkdirSync(path.join(repo, 'manifest'), { recursive: true }); + fs.writeFileSync( + path.join(repo, 'manifest', 'projects.yaml'), + `version: 1 +projects: + - id: infra + name: Infra + resources: + workflows: [ops] +` + ); + config.projects = ['infra']; + + // Write namespaced workflow under workflows/ops/deploy.yaml + fs.mkdirSync(path.join(repo, 'workflows', 'ops'), { recursive: true }); + fs.writeFileSync(path.join(repo, 'workflows', 'ops', 'deploy.yaml'), WORKFLOW_A); + + commit('add ops deploy workflow'); + + const result = await syncWorkflowDefinitions(config); + expect(result.invalid).toEqual([]); + + const loaded = await loadCachedWorkflow(config, 'deploy'); + expect(loaded).not.toBeNull(); + expect(loaded?.definition.description).toBe('Deploy from namespace A.'); + }); + + it('namespaced workflow takes precedence over shared root workflow when namespace is active', async () => { + const config = testConfig(); + const repo = config.repo.localPath; + const commit = initializeGit(repo); + + // Setup active namespaces + fs.mkdirSync(path.join(repo, 'manifest'), { recursive: true }); + fs.writeFileSync( + path.join(repo, 'manifest', 'projects.yaml'), + `version: 1 +projects: + - id: infra + name: Infra + resources: + workflows: [ops] +` + ); + config.projects = ['infra']; + + // Write shared workflow at workflows/deploy.yaml + fs.writeFileSync(path.join(repo, 'workflows', 'deploy.yaml'), SHARED_WORKFLOW); + // Write namespaced workflow at workflows/ops/deploy.yaml + fs.mkdirSync(path.join(repo, 'workflows', 'ops'), { recursive: true }); + fs.writeFileSync(path.join(repo, 'workflows', 'ops', 'deploy.yaml'), WORKFLOW_A); + + commit('add shared and ops workflows'); + + const result = await syncWorkflowDefinitions(config); + expect(result.invalid).toEqual([]); + + const loaded = await loadCachedWorkflow(config, 'deploy'); + expect(loaded).not.toBeNull(); + // Namespaced version wins over root shared version + expect(loaded?.definition.description).toBe('Deploy from namespace A.'); + }); + + it('drops workflow and records diagnostic when two active namespaces conflict', async () => { + const config = testConfig(); + const repo = config.repo.localPath; + const commit = initializeGit(repo); + + // Setup two active namespaces declaring different namespaces for workflows + fs.mkdirSync(path.join(repo, 'manifest'), { recursive: true }); + fs.writeFileSync( + path.join(repo, 'manifest', 'projects.yaml'), + `version: 1 +projects: + - id: infra + name: Infra + resources: + workflows: [ops, devops] +` + ); + config.projects = ['infra']; + + // Write workflows/ops/deploy.yaml and workflows/devops/deploy.yaml + fs.mkdirSync(path.join(repo, 'workflows', 'ops'), { recursive: true }); + fs.writeFileSync(path.join(repo, 'workflows', 'ops', 'deploy.yaml'), WORKFLOW_A); + fs.mkdirSync(path.join(repo, 'workflows', 'devops'), { recursive: true }); + fs.writeFileSync(path.join(repo, 'workflows', 'devops', 'deploy.yaml'), WORKFLOW_B); + + commit('add conflicting workflows'); + + const result = await syncWorkflowDefinitions(config); + // Should produce diagnostic for conflict + expect(result.invalid.length).toBeGreaterThan(0); + const conflictDiag = result.invalid.find((d) => d.code === 'workflow_namespace_conflict'); + expect(conflictDiag).toBeDefined(); + expect(conflictDiag?.message).toContain('deploy'); + + // Conflicted workflow must be dropped from cache + const loaded = await loadCachedWorkflow(config, 'deploy'); + expect(loaded).toBeNull(); + }); + + it('ignores workflows in namespaces that are not active', async () => { + const config = testConfig(); + const repo = config.repo.localPath; + const commit = initializeGit(repo); + + // Only "team-x" is active + fs.mkdirSync(path.join(repo, 'manifest'), { recursive: true }); + fs.writeFileSync( + path.join(repo, 'manifest', 'projects.yaml'), + `version: 1 +projects: + - id: team-x + name: Team X + resources: + workflows: [ns-x] +` + ); + config.projects = ['team-x']; + + // Write workflow in inactive namespace "ns-other" + fs.mkdirSync(path.join(repo, 'workflows', 'ns-other'), { recursive: true }); + fs.writeFileSync(path.join(repo, 'workflows', 'ns-other', 'deploy.yaml'), WORKFLOW_B); + + commit('add inactive workflow'); + + const result = await syncWorkflowDefinitions(config); + expect(result.invalid).toEqual([]); + + const loaded = await loadCachedWorkflow(config, 'deploy'); + expect(loaded).toBeNull(); + }); +}); diff --git a/src/manifest-schema.ts b/src/manifest-schema.ts index 6b6af1304..d46c37fe1 100644 --- a/src/manifest-schema.ts +++ b/src/manifest-schema.ts @@ -71,7 +71,17 @@ export const NamespaceSegmentSchema = z.string().min(1).refine(isSafeNamespaceSe * writes back carries one only when an admin declared it. A new type is one * entry here plus one line in the shape below. */ -export const HAND_DECLARED_RESOURCE_TYPES = ['env', 'hooks', 'mcp', 'models', 'docs', 'wiki'] as const; +export const HAND_DECLARED_RESOURCE_TYPES = [ + 'env', + 'hooks', + 'mcp', + 'models', + 'docs', + 'wiki', + 'workflows', + 'prompts', + 'scorers', +] as const; export type HandDeclaredResourceType = typeof HAND_DECLARED_RESOURCE_TYPES[number]; @@ -105,6 +115,9 @@ export const HandDeclaredNamespacesShape = { // project id — a team picks one it already uses so declaring it needs no // new convention. wiki: OptionalNamespaceList, + workflows: OptionalNamespaceList, + prompts: OptionalNamespaceList, + scorers: OptionalNamespaceList, } satisfies Record>>>; /** diff --git a/src/projects.ts b/src/projects.ts index 8717f4f59..a32e489d8 100644 --- a/src/projects.ts +++ b/src/projects.ts @@ -18,7 +18,9 @@ export const PROJECT_RESOURCE_TYPES = ['knowledge', 'skills', 'learnings', 'agen export type ProjectResourceType = typeof PROJECT_RESOURCE_TYPES[number]; /** Every type a project can namespace, the hand-declared ones included. */ -const ALL_PROJECT_RESOURCE_TYPES = [...PROJECT_RESOURCE_TYPES, ...HAND_DECLARED_RESOURCE_TYPES] as const; +export const ALL_PROJECT_RESOURCE_TYPES = [...PROJECT_RESOURCE_TYPES, ...HAND_DECLARED_RESOURCE_TYPES] as const; + +export type AllProjectResourceType = typeof ALL_PROJECT_RESOURCE_TYPES[number]; /** * A project id becomes a path component (`skills//`, `learnings//`) just diff --git a/src/push-namespaces.ts b/src/push-namespaces.ts index 470a54a80..49b1ac61c 100644 --- a/src/push-namespaces.ts +++ b/src/push-namespaces.ts @@ -15,17 +15,46 @@ import { isSafeNamespaceSegment, NAMESPACE_RULE } from './manifest-schema.js'; import { findProject, unknownProjectMessage, - type ProjectResourceType, type ProjectsManifest, + type AllProjectResourceType, type ProjectsManifest, } from './projects.js'; import type { ResourceItem, ResourceType } from './types.js'; /** Resource types `pull` namespaces, and push therefore has to place. */ -export type PlaceableType = 'skills' | 'rules' | 'agents'; - -export const PLACEABLE_TYPES: readonly PlaceableType[] = ['skills', 'rules', 'agents']; - -export function isPlaceableType(type: ResourceType): type is PlaceableType { - return (PLACEABLE_TYPES as readonly ResourceType[]).includes(type); +export type PlaceableType = + | 'skills' + | 'rules' + | 'agents' + | 'docs' + | 'env' + | 'hooks' + | 'mcp' + | 'models' + | 'claudemd' + | 'learnings' + | 'wiki' + | 'workflows' + | 'prompts' + | 'scorers'; + +export const PLACEABLE_TYPES: readonly PlaceableType[] = [ + 'skills', + 'rules', + 'agents', + 'docs', + 'env', + 'hooks', + 'mcp', + 'models', + 'claudemd', + 'learnings', + 'wiki', + 'workflows', + 'prompts', + 'scorers', +]; + +export function isPlaceableType(type: ResourceType | string): type is PlaceableType { + return (PLACEABLE_TYPES as readonly string[]).includes(type); } /** @@ -38,7 +67,18 @@ export const NAMESPACE_AXIS = { skills: 'skills', rules: 'knowledge', agents: 'agents', -} as const satisfies Record; + docs: 'docs', + env: 'env', + hooks: 'hooks', + mcp: 'mcp', + models: 'models', + claudemd: 'knowledge', + learnings: 'learnings', + wiki: 'wiki', + workflows: 'workflows', + prompts: 'prompts', + scorers: 'scorers', +} as const satisfies Record; /** * True when `item` would be written to the shared root — no namespace segment @@ -97,7 +137,7 @@ export function resolveProjectNamespace( } const axis = NAMESPACE_AXIS[type]; - const namespaces = project.resources[axis]; + const namespaces = project.resources[axis] ?? []; if (namespaces.length === 0) { return { ok: false, diff --git a/src/push.ts b/src/push.ts index 9dcbc1e9c..c5dff6256 100644 --- a/src/push.ts +++ b/src/push.ts @@ -108,7 +108,7 @@ async function namespaceCandidates( additionalRoles: localConfig.additionalRoles ?? [], }); const axis = NAMESPACE_AXIS[type]; - const candidates = namespaces[axis]; + const candidates = namespaces[axis] ?? []; // A namespace is ONE directory under the resource root. `--role` and the // projects manifest are both checked for that; the roles manifest was // not, so `foo/bar` went through and pushed an agent to a depth `pull` @@ -655,8 +655,12 @@ async function placeNewResources(args: { process.exitCode = 2; return false; } + const originalRelPath = item.relativePath; item.namespace = destination.namespace; item.relativePath = placedAt; + if (originalRelPath !== placedAt) { + item.supersedes = originalRelPath; + } // The silent widening in #649 was the real damage: say where it went. log.info(`[${type}] ${item.name} → ${item.relativePath}`); } @@ -1103,10 +1107,16 @@ async function pushCore( const yamlPath = path.join(repoPath, 'teamai.yaml'); const workingContent = await readFileSafe(yamlPath); // Nothing below writes before these reads finish, so they run together: - // the env scan, the clone's status, and what HEAD holds of teamai.yaml. - const envScan = getHandler('env').scanLocalForPush(teamConfig, localConfig); - // Awaited after the status: a status failure is the one reported, as before. - envScan.catch(() => {}); + const repoScan = Promise.all( + PLACEABLE_TYPES.map(async (t) => { + try { + return await getHandler(t).scanLocalForPush(teamConfig, localConfig); + } catch { + return []; + } + }), + ); + repoScan.catch(() => {}); const [committed, status] = await Promise.all([ workingContent === null ? null : getFileContentAtRev(repoPath, 'HEAD', 'teamai.yaml'), git.status(), @@ -1118,8 +1128,7 @@ async function pushCore( if (pendingTeamConfig !== null && await hasGitModeChange(git, 'teamai.yaml')) { modeChangedPaths.add('teamai.yaml'); } - // `env add` edits env files in the clone and leaves the commit to push: - // capture what the env scan will push, to restore it after the refresh + // Capture what repo scans will push, to restore after the refresh // below as teamai.yaml is (#881). A mode change is not captured, so it // stays dirty and stops the push. So does any index state: the snapshot // holds only the working copy, and reset --hard would drop a staged, @@ -1131,7 +1140,8 @@ async function pushCore( deleted: status.deleted, renamed: status.renamed, })); - for (const item of await envScan) { + const scannedGroups = await repoScan; + for (const item of scannedGroups.flat()) { if (indexedPaths.has(item.relativePath)) continue; const snapshot = await readFileSnapshot(item.sourcePath); if (snapshot !== null && !await hasGitModeChange(git, item.relativePath)) { @@ -1368,7 +1378,7 @@ async function pushCore( // Scan for pushable resources first, then resolve namespace for new skills only. // Skills already carry the destination scanLocalForPush decided: a modified // one its team skill's, a new one awaiting review its open PR's. - const pushableTypes: ResourceType[] = ['skills', 'rules', 'env', 'agents']; + const pushableTypes: ResourceType[] = [...PLACEABLE_TYPES]; const fullScan: ResourceItem[] = []; // Agents are the one type whose SCAN needs the destination: it has to tell diff --git a/src/resources/docs.ts b/src/resources/docs.ts index e9cc00fb0..511a5d3e5 100644 --- a/src/resources/docs.ts +++ b/src/resources/docs.ts @@ -9,8 +9,9 @@ import { caseFoldKey } from '../manifest-schema.js'; import { resolveResourceNamespaces } from '../resource-namespaces.js'; import { describeKeptEntry, describeMembersDirLeft, isLink, isTeamaiCopy, isTeamaiSkillCopy } from './delivered-copies.js'; import { blobIdOf, historicalVersions, type HistoricalVersion } from '../utils/team-history.js'; -import type { DeliveryRecorder } from '../git-exclude-delivered.js'; import { gitTracks, realFilePath, updateFileLocked, withOwnerLines } from '../git-exclude.js'; +import type { DeliveryRecorder } from '../git-exclude-delivered.js'; +import { pushRepoResourceItem, scanRepoResourceForPush } from './repo-resource.js'; /** * The single directory the team docs bundle is copied into. In project scope a @@ -618,9 +619,8 @@ async function withdrawInactiveNamespaces(desired: DesiredDocs, localDocsDir: st export class DocsHandler extends ResourceHandler { readonly type = 'docs' as const; - async scanLocalForPush(_teamConfig: TeamaiConfig, _localConfig: LocalConfig): Promise { - // Docs are managed directly in team repo - return []; + async scanLocalForPush(teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise { + return scanRepoResourceForPush('docs', 'docs', teamConfig, localConfig); } async scanTeamForPull(_teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise { @@ -640,8 +640,8 @@ export class DocsHandler extends ResourceHandler { return (await listDocFiles(sourcePath)).length; } - async pushItem(_item: ResourceItem, _teamConfig: TeamaiConfig, _localConfig: LocalConfig): Promise { - // No-op + async pushItem(item: ResourceItem, _teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise { + await pushRepoResourceItem(item, localConfig); } /** diff --git a/src/resources/hooks.ts b/src/resources/hooks.ts index def3b51a3..d3f82321b 100644 --- a/src/resources/hooks.ts +++ b/src/resources/hooks.ts @@ -6,6 +6,7 @@ import type { ResourceItem, TeamaiConfig, LocalConfig, HookDef } from '../types. import { TEAMAI_CUSTOM_HOOK_PREFIX, areTeamHooksDisabled, getHooksSharing } from '../types.js'; import { pathExists } from '../utils/fs.js'; import { log } from '../utils/logger.js'; +import { pushRepoResourceItem, scanRepoResourceForPush } from './repo-resource.js'; import { entryFilePath, missingTopLevelKeyReason, readEntryFileText, reportEntryResolution, resolveEntriesFor, unknownEntryKeys, type EntryReader, type EntryResolution, @@ -234,9 +235,8 @@ export async function resolveTeamHooks( export class HooksHandler extends ResourceHandler { readonly type = 'hooks' as const; - /** Never reverse-push from local settings (avoids confusion with built-in A hooks). */ - async scanLocalForPush(_teamConfig: TeamaiConfig, _localConfig: LocalConfig): Promise { - return []; + async scanLocalForPush(teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise { + return scanRepoResourceForPush('hooks', 'hooks', teamConfig, localConfig); } async scanTeamForPull(_teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise { @@ -245,8 +245,8 @@ export class HooksHandler extends ResourceHandler { return [{ name: 'hooks.yaml', type: 'hooks', sourcePath: yamlPath, relativePath: 'hooks/hooks.yaml' }]; } - async pushItem(_item: ResourceItem, _teamConfig: TeamaiConfig, _localConfig: LocalConfig): Promise { - // No-op — hooks.yaml is edited directly in the repo; push.ts handles git commit. + async pushItem(item: ResourceItem, _teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise { + await pushRepoResourceItem(item, localConfig); } async pullItem(_item: ResourceItem, _teamConfig: TeamaiConfig, _localConfig: LocalConfig): Promise { diff --git a/src/resources/index.ts b/src/resources/index.ts index ba7bf297e..9e151ea75 100644 --- a/src/resources/index.ts +++ b/src/resources/index.ts @@ -6,6 +6,7 @@ import { EnvHandler } from './env.js'; import { AgentsHandler } from './agents.js'; import { HooksHandler } from './hooks.js'; import { McpHandler } from './mcp.js'; +import { RepoResourceHandler } from './repo-resource.js'; import type { ResourceType } from '../types.js'; const handlers: Record = { @@ -16,6 +17,13 @@ const handlers: Record = { agents: new AgentsHandler(), hooks: new HooksHandler(), mcp: new McpHandler(), + models: new RepoResourceHandler('models', 'models'), + claudemd: new RepoResourceHandler('claudemd', 'claudemd'), + learnings: new RepoResourceHandler('learnings', 'learnings'), + wiki: new RepoResourceHandler('wiki', 'wiki'), + workflows: new RepoResourceHandler('workflows', 'workflows', (f) => f.endsWith('.yaml')), + prompts: new RepoResourceHandler('prompts', 'prompts'), + scorers: new RepoResourceHandler('scorers', 'scorers'), }; export function getHandler(type: ResourceType): ResourceHandler { @@ -26,4 +34,7 @@ export function getAllHandlers(): ResourceHandler[] { return Object.values(handlers); } -export { SkillsHandler, RulesHandler, DocsHandler, EnvHandler, AgentsHandler, HooksHandler, McpHandler }; +export { + SkillsHandler, RulesHandler, DocsHandler, EnvHandler, AgentsHandler, HooksHandler, McpHandler, + RepoResourceHandler, +}; diff --git a/src/resources/mcp.ts b/src/resources/mcp.ts index 67dbbf346..cfb77ecde 100644 --- a/src/resources/mcp.ts +++ b/src/resources/mcp.ts @@ -4,6 +4,7 @@ import { ResourceHandler } from './base.js'; import type { ResourceItem, TeamaiConfig, LocalConfig, McpServerDef } from '../types.js'; import { readFileSafe, writeFile } from '../utils/fs.js'; import { log } from '../utils/logger.js'; +import { pushRepoResourceItem, scanRepoResourceForPush } from './repo-resource.js'; import { entryFileAbsolutePath, entryFilePath, listEntryFiles, missingTopLevelKeyReason, readEntryFileText, unknownEntryKeys, type EntryReader, @@ -120,12 +121,8 @@ export function teamMcpToDef(s: TeamMcpServer): McpServerDef { export class McpHandler extends ResourceHandler { readonly type = 'mcp' as const; - /** - * MCP servers are contributed by editing mcp/mcp.yaml directly (same as hooks), - * so there is nothing to discover on the local side for push. - */ - async scanLocalForPush(): Promise { - return []; + async scanLocalForPush(teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise { + return scanRepoResourceForPush('mcp', 'mcp', teamConfig, localConfig); } /** @@ -156,8 +153,8 @@ export class McpHandler extends ResourceHandler { return items; } - async pushItem(): Promise { - // Servers live in a single mcp.yaml committed directly; nothing per-item to copy. + async pushItem(item: ResourceItem, _teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise { + await pushRepoResourceItem(item, localConfig); } async pullItem(): Promise { diff --git a/src/resources/repo-resource.ts b/src/resources/repo-resource.ts new file mode 100644 index 000000000..410fc400c --- /dev/null +++ b/src/resources/repo-resource.ts @@ -0,0 +1,178 @@ +import path from 'node:path'; +import fs from 'node:fs/promises'; +import { ResourceHandler } from './base.js'; +import { isSelfMode, type LocalConfig, type ResourceItem, type ResourceType, type TeamaiConfig } from '../types.js'; +import { ensureDir, fileContentEqual, pathExists, readFileSafe, writeFile } from '../utils/fs.js'; + +export async function listRepoFilesRecursive(dir: string, baseDir = dir): Promise { + try { + const entries = await fs.readdir(dir, { withFileTypes: true }); + const files: string[] = []; + for (const entry of entries) { + if (entry.name.startsWith('.')) continue; + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + const nested = await listRepoFilesRecursive(full, baseDir); + files.push(...nested); + } else if (entry.isFile()) { + files.push(path.relative(baseDir, full).split(path.sep).join('/')); + } + } + return files; + } catch { + return []; + } +} + +export async function scanRepoResourceForPush( + type: ResourceType, + dir: string, + _teamConfig: TeamaiConfig, + localConfig: LocalConfig, + fileFilter?: (file: string) => boolean, +): Promise { + if (isSelfMode(localConfig) && localConfig.projectRoot) { + const activeRoot = path.join(localConfig.projectRoot, '.teamai', dir); + const baselineRoot = path.join(localConfig.repo.localPath, dir); + if (!await pathExists(activeRoot)) return []; + + const items: ResourceItem[] = []; + const files = await listRepoFilesRecursive(activeRoot); + for (const rel of files) { + if (fileFilter && !fileFilter(rel)) continue; + const activeFile = path.join(activeRoot, ...rel.split('/')); + const baseFile = path.join(baselineRoot, ...rel.split('/')); + const relativePath = `${dir}/${rel}`; + if (await pathExists(baseFile) && await fileContentEqual(activeFile, baseFile)) continue; + const status = await pathExists(baseFile) ? 'modified' : 'new'; + items.push({ + name: rel, + type, + sourcePath: activeFile, + relativePath, + status, + }); + } + return items; + } + + const repoPath = localConfig.repo.localPath; + const targetDir = path.join(repoPath, dir); + if (!await pathExists(targetDir)) return []; + + let changedFiles: Set | null = null; + let untrackedFiles: Set = new Set(); + try { + const { execFile } = await import('node:child_process'); + const { promisify } = await import('node:util'); + const execFileAsync = promisify(execFile); + + const [changedRes, untrackedRes] = await Promise.all([ + execFileAsync('git', ['ls-files', '-z', '--modified', '--others', '--exclude-standard', '--', dir], { cwd: repoPath }), + execFileAsync('git', ['ls-files', '-z', '--others', '--exclude-standard', '--', dir], { cwd: repoPath }), + ]); + changedFiles = new Set(changedRes.stdout.split('\0').filter(Boolean)); + untrackedFiles = new Set(untrackedRes.stdout.split('\0').filter(Boolean)); + } catch { + changedFiles = null; + } + + const allFiles = await listRepoFilesRecursive(targetDir); + const items: ResourceItem[] = []; + for (const rel of allFiles) { + if (fileFilter && !fileFilter(rel)) continue; + const relativePath = `${dir}/${rel}`; + if (changedFiles && !changedFiles.has(relativePath)) continue; + const sourcePath = path.join(targetDir, ...rel.split('/')); + const status = untrackedFiles.has(relativePath) || !changedFiles ? 'new' : 'modified'; + items.push({ + name: rel, + type, + sourcePath, + relativePath, + status, + }); + } + return items; +} + +export async function pushRepoResourceItem( + item: ResourceItem, + localConfigOrTeamConfig: TeamaiConfig | LocalConfig, + maybeLocalConfig?: LocalConfig, +): Promise { + const localConfig = maybeLocalConfig ?? (localConfigOrTeamConfig as LocalConfig); + const dest = path.resolve(localConfig.repo.localPath, ...item.relativePath.split('/')); + const originalSource = path.resolve(item.sourcePath); + if (originalSource !== dest) { + await ensureDir(path.dirname(dest)); + const content = await readFileSafe(originalSource); + if (content !== null) { + await writeFile(dest, content); + } + const repoRoot = path.resolve(localConfig.repo.localPath); + if (originalSource.startsWith(repoRoot + path.sep)) { + await fs.rm(originalSource, { force: true }); + } + } +} + +export class RepoResourceHandler extends ResourceHandler { + constructor( + readonly type: ResourceType, + readonly dir: string = type, + readonly fileFilter?: (file: string) => boolean, + ) { + super(); + } + + async scanLocalForPush( + teamConfig: TeamaiConfig, + localConfig: LocalConfig, + ): Promise { + return scanRepoResourceForPush(this.type, this.dir, teamConfig, localConfig, this.fileFilter); + } + + async scanTeamForPull( + _teamConfig: TeamaiConfig, + localConfig: LocalConfig, + ): Promise { + const dirPath = path.join(localConfig.repo.localPath, this.dir); + if (!await pathExists(dirPath)) return []; + const files = await listRepoFilesRecursive(dirPath); + return files + .filter((file) => !this.fileFilter || this.fileFilter(file)) + .map((file) => ({ + name: file, + type: this.type, + sourcePath: path.join(dirPath, ...file.split('/')), + relativePath: `${this.dir}/${file}`, + })); + } + + async pushItem( + item: ResourceItem, + _teamConfig: TeamaiConfig, + localConfig: LocalConfig, + ): Promise { + await pushRepoResourceItem(item, localConfig); + } + + async pullItem( + _item: ResourceItem, + _teamConfig: TeamaiConfig, + _localConfig: LocalConfig, + ): Promise { + // Pull synchronization is orchestrated by high-level sync routines in pull.ts and workflow cache. + } + + async removeItem( + name: string, + _teamConfig: TeamaiConfig, + localConfig: LocalConfig, + ): Promise { + const target = path.resolve(localConfig.repo.localPath, this.dir, ...name.split('/')); + await fs.rm(target, { force: true }); + return [name]; + } +} diff --git a/src/review-branch.ts b/src/review-branch.ts index 785d511f9..98dec8b05 100644 --- a/src/review-branch.ts +++ b/src/review-branch.ts @@ -26,7 +26,7 @@ import fse from 'fs-extra'; import { resolveConfigForDir } from './config.js'; import { caseFoldKey, isSafeNamespaceSegment, NAMESPACE_RULE } from './manifest-schema.js'; -import { loadProjectsManifest, type ProjectResourceType, type ProjectsManifest } from './projects.js'; +import { ALL_PROJECT_RESOURCE_TYPES, loadProjectsManifest, type AllProjectResourceType, type ProjectsManifest } from './projects.js'; import { NAMESPACE_AXIS, type PlaceableType } from './push-namespaces.js'; import { parseAgentYaml } from './resources/agent-format.js'; import { AgentsHandler } from './resources/agents.js'; @@ -67,7 +67,7 @@ export interface BranchReview { findings: Finding[]; } -const AXES: readonly ProjectResourceType[] = ['knowledge', 'skills', 'learnings', 'agents']; +const AXES: readonly AllProjectResourceType[] = ALL_PROJECT_RESOURCE_TYPES; // ─── Resource model ────────────────────────────────────────────────────────── @@ -125,21 +125,24 @@ async function scanResources(repoDir: string): Promise { /** The namespace lists a branch's roles and projects declare, per axis. */ interface ManifestIndex { /** True when any role or project declares `namespace` on `axis`. */ - declares(axis: ProjectResourceType, namespace: string): boolean; + declares(axis: AllProjectResourceType, namespace: string): boolean; /** True when any single role or project declares both namespaces on `axis`. */ - coActive(axis: ProjectResourceType, a: string, b: string): boolean; + coActive(axis: AllProjectResourceType, a: string, b: string): boolean; /** True when any role or project declares any namespace on `axis`. */ - anyOn(axis: ProjectResourceType): boolean; + anyOn(axis: AllProjectResourceType): boolean; } function buildManifestIndex(roles: RolesManifest | null, projects: ProjectsManifest | null): ManifestIndex { - const listsByAxis = new Map(); + const listsByAxis = new Map(); for (const axis of AXES) listsByAxis.set(axis, []); - const collect = (resources: Partial>): void => { + const collect = (resources: Record | undefined): void => { + if (!resources) return; for (const axis of AXES) { const list = resources[axis]; - if (list && list.length > 0) listsByAxis.get(axis)!.push(list); + if (Array.isArray(list) && list.length > 0) { + listsByAxis.get(axis)!.push(list as string[]); + } } }; for (const role of roles?.roles ?? []) collect(role.resources); diff --git a/src/types.ts b/src/types.ts index 3b43b1c96..72b2118de 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1144,9 +1144,21 @@ export interface TagsConfig { rules: Record; } -// ─── Resource types ───────────────────────────────────── - -export type ResourceType = 'skills' | 'rules' | 'docs' | 'env' | 'agents' | 'hooks' | 'mcp'; +export type ResourceType = + | 'skills' + | 'rules' + | 'docs' + | 'env' + | 'agents' + | 'hooks' + | 'mcp' + | 'models' + | 'claudemd' + | 'learnings' + | 'wiki' + | 'workflows' + | 'prompts' + | 'scorers'; export type ResourceItemStatus = 'new' | 'modified'; @@ -1159,6 +1171,8 @@ export interface ResourceItem { namespace?: string; /** A kept skill with a recorded origin: `--role`/`--project` never relocate it (#1020). */ fromInactiveNamespace?: true; + /** When placed into a namespace, the original relative path superseded by this item. */ + supersedes?: string; } export interface ResourceDiff { @@ -1485,7 +1499,22 @@ export function getUpdateLockPath(): string { return path.join(getTeamaiHomeDir(), '.update-lock'); } -export const RESOURCE_TYPES: ResourceType[] = ['skills', 'rules', 'docs', 'env', 'agents', 'hooks', 'mcp']; +export const RESOURCE_TYPES: ResourceType[] = [ + 'skills', + 'rules', + 'docs', + 'env', + 'agents', + 'hooks', + 'mcp', + 'models', + 'claudemd', + 'learnings', + 'wiki', + 'workflows', + 'prompts', + 'scorers', +]; export const TEAMAI_RULES_START = ''; export const TEAMAI_RULES_END = ''; diff --git a/src/workflow/cache.ts b/src/workflow/cache.ts index 78c828840..3e1921bcc 100644 --- a/src/workflow/cache.ts +++ b/src/workflow/cache.ts @@ -2,7 +2,10 @@ import { createHash } from 'node:crypto'; import fs from 'node:fs/promises'; import path from 'node:path'; import { log } from '../utils/logger.js'; -import { readFileSafe, remove, writeJsonAtomic } from '../utils/fs.js'; +import { pathExists, readFileSafe, remove, writeJsonAtomic } from '../utils/fs.js'; +import { isSafeNamespaceSegment } from '../manifest-schema.js'; +import { resolveResourceNamespaces } from '../resource-namespaces.js'; +import { resolveNamespacedItems, type NamespaceCandidate } from '../namespace-resolver.js'; import { getBusinessRoot, getDataHome, @@ -191,9 +194,85 @@ export async function syncWorkflowDefinitions( const knowledgeRoot = getKnowledgeDir(config); const reachable = await remotelyReachableRevision(knowledgeRoot); const checked: Checked[] = []; + const rawCandidates: Array<{ stem: string; relativePath: string; full: string; namespace: string | null }> = []; for (const name of await workflowFileNames(dir)) { - const relativePath = `workflows/${name}`; - const full = path.join(dir, name); + rawCandidates.push({ + stem: name.replace(/\.yaml$/, ''), + relativePath: `workflows/${name}`, + full: path.join(dir, name), + namespace: null, + }); + } + + try { + const entries = await fs.readdir(dir, { withFileTypes: true }); + for (const entry of entries) { + if (!entry.isDirectory() || entry.isSymbolicLink() || entry.name.startsWith('.')) continue; + if (!isSafeNamespaceSegment(entry.name)) continue; + const ns = entry.name; + const nsDir = path.join(dir, ns); + for (const name of await workflowFileNames(nsDir)) { + rawCandidates.push({ + stem: name.replace(/\.yaml$/, ''), + relativePath: `workflows/${ns}/${name}`, + full: path.join(nsDir, name), + namespace: ns, + }); + } + } + } catch { + // dir missing or unreadable + } + + const resourceNamespaces = await resolveResourceNamespaces(config); + const activeWorkflows = resourceNamespaces?.activeNamespaces.workflows ?? (resourceNamespaces ? [] : null); + const activePrompts = resourceNamespaces?.activeNamespaces.prompts ?? (resourceNamespaces ? [] : null); + + const byStem = new Map>(); + for (const cand of rawCandidates) { + const group = byStem.get(cand.stem) ?? []; + group.push(cand); + byStem.set(cand.stem, group); + } + + const toCheck: Array<{ stem: string; relativePath: string; full: string }> = []; + + for (const [stem, group] of byStem) { + if (activeWorkflows === null) { + if (group[0]) toCheck.push(group[0]); + continue; + } + + const namespaceCandidatesList: NamespaceCandidate<{ stem: string; relativePath: string; full: string }>[] = group.map((c) => ({ + name: c.stem, + source: c.relativePath, + namespace: c.namespace, + value: c, + })); + + const resolution = resolveNamespacedItems(namespaceCandidatesList, activeWorkflows); + if (resolution.kind === 'conflict') { + checked.push({ + stem, + relativePath: resolution.first.source, + diagnostics: [{ + file: resolution.first.source, + line: 1, + column: 1, + field: '(root)', + code: 'workflow_namespace_conflict', + message: `Workflow "${stem}" is defined in multiple active namespaces (${resolution.first.source} and ${resolution.second.source}); conflicting workflows have no runnable version`, + }], + }); + continue; + } + + if (resolution.items.length > 0) { + toCheck.push(resolution.items[0].value); + } + } + + for (const { stem, relativePath, full } of toCheck) { let stat: Awaited>; try { stat = await fs.lstat(full); @@ -202,7 +281,6 @@ export async function syncWorkflowDefinitions( } // A symlink is not a workflow file, even when the target is a YAML file outside this directory. if (stat.isSymbolicLink() || !stat.isFile()) continue; - const stem = name.replace(/\.yaml$/, ''); const content = await readFileSafe(full); if (content === null) { checked.push({ stem, relativePath, diagnostics: [{ file: relativePath, line: 1, column: 1, field: '(root)', message: 'workflow file could not be read' }] }); @@ -213,7 +291,7 @@ export async function syncWorkflowDefinitions( checked.push({ stem, relativePath, diagnostics: parsed.diagnostics }); continue; } - const resolved = await resolvePromptFiles(config, parsed.definition, parsed.promptFileLocations, relativePath); + const resolved = await resolvePromptFiles(config, parsed.definition, parsed.promptFileLocations, relativePath, activePrompts); if (!resolved.ok) { checked.push({ stem, relativePath, diagnostics: resolved.diagnostics }); continue; @@ -420,6 +498,7 @@ async function resolvePromptFiles( definition: WorkflowDefinition, locations: Record, workflowFile: string, + activePromptsNamespaces: readonly string[] | null = null, ): Promise< | { ok: true; definition: WorkflowDefinition; sources: Array<{ path: string; content: string }> } | { ok: false; diagnostics: WorkflowDiagnostic[] } @@ -436,8 +515,38 @@ async function resolvePromptFiles( continue; } if (!step.promptFile) continue; - const target = path.resolve(root, step.promptFile); + + let promptPath = step.promptFile; const location = locations[field] ?? { line: 1, column: 1 }; + + const promptRelative = promptPath.replace(/^[\\/]/, ''); + if (promptRelative.startsWith('prompts/') && activePromptsNamespaces && activePromptsNamespaces.length > 0) { + const leaf = promptRelative.slice('prompts/'.length); + if (!leaf.includes('/')) { + const found = (await Promise.all( + activePromptsNamespaces.map(async (ns) => { + const candidate = path.join(root, 'prompts', ns, leaf); + return (await pathExists(candidate)) ? `prompts/${ns}/${leaf}` : null; + }), + )).filter((p): p is string => p !== null); + + if (found.length === 1) { + promptPath = found[0]; + } else if (found.length > 1) { + diagnostics.push({ + file: workflowFile, + line: location.line, + column: location.column, + field: `${field}.prompt_file`, + code: 'prompt_namespace_conflict', + message: `prompt_file "${step.promptFile}" is ambiguous between active namespaces (${found.join(', ')}); fix the conflict or specify the prompt path explicitly`, + }); + continue; + } + } + } + + const target = path.resolve(root, promptPath); let prompt: string | null = null; try { assertWithinRoot(path.join(root, 'prompts'), target); @@ -457,7 +566,7 @@ async function resolvePromptFiles( continue; } diagnostics.push(...promptFileDiagnostics(workflowFile, definition, [...trail, index], prompt, location)); - sources.push({ path: step.promptFile, content: prompt }); + sources.push({ path: promptPath, content: prompt }); steps[index] = { ...step, prompt }; } return steps; diff --git a/src/workflow/types.ts b/src/workflow/types.ts index 5d2bf29c2..69baa7b8a 100644 --- a/src/workflow/types.ts +++ b/src/workflow/types.ts @@ -164,7 +164,7 @@ export interface WorkflowDiagnostic { * `bad_loop`: a `loop`, `break` or `for_each` pull rejects; the kept copy would run once the steps the author wrote * to repeat or to run per item, or run on past the point the author wrote to stop at. */ - code?: 'unknown_key' | 'missing_needs' | 'false_if' | 'bad_template' | 'dotted_name' | 'undeclared_output' | 'undeclared_end' | 'bad_loop'; + code?: 'unknown_key' | 'missing_needs' | 'false_if' | 'bad_template' | 'dotted_name' | 'undeclared_output' | 'undeclared_end' | 'bad_loop' | 'workflow_namespace_conflict' | 'prompt_namespace_conflict'; } export type StepOutcome = 'success' | 'failed' | 'released'; From fd2c98b2e484cd6780296ae56af3dac579208f94 Mon Sep 17 00:00:00 2001 From: legend91019 <15853153143@163.com> Date: Tue, 20 Oct 2026 15:22:29 +0800 Subject: [PATCH 02/28] fix(push): harden namespace routing and review (#1101) --- .../push-all-resources-namespaces.test.ts | 22 +++++++- src/__tests__/review-branch.test.ts | 27 +++++++++ .../workflow-cache-namespaces.test.ts | 33 +++++++++++ src/__tests__/workflow-records.test.ts | 6 +- src/push-namespaces.ts | 14 +++++ src/push.ts | 25 ++++++++- src/review-branch.ts | 56 +++++++++++++------ src/workflow/cache.ts | 12 ++-- 8 files changed, 167 insertions(+), 28 deletions(-) diff --git a/src/__tests__/push-all-resources-namespaces.test.ts b/src/__tests__/push-all-resources-namespaces.test.ts index 337fc96e2..75260c2c9 100644 --- a/src/__tests__/push-all-resources-namespaces.test.ts +++ b/src/__tests__/push-all-resources-namespaces.test.ts @@ -3,6 +3,7 @@ import os from 'node:os'; import path from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; import { + declaredNamespaceFor, isAtSharedRoot, isPlaceableType, NAMESPACE_AXIS, @@ -76,7 +77,7 @@ describe('isAtSharedRoot and withNamespace across all resource types', () => { it('detects shared root for items without namespace segments', () => { const sampleItems: Array<{ type: (typeof PLACEABLE_TYPES)[number]; path: string; shared: boolean }> = [ { type: 'docs', path: 'docs/guide.md', shared: true }, - { type: 'docs', path: 'docs/team-a/guide.md', shared: false }, + { type: 'docs', path: 'docs/team-a/guide.md', shared: true }, { type: 'workflows', path: 'workflows/ci.yaml', shared: true }, { type: 'workflows', path: 'workflows/team-a/ci.yaml', shared: false }, { type: 'prompts', path: 'prompts/review.md', shared: true }, @@ -86,7 +87,7 @@ describe('isAtSharedRoot and withNamespace across all resource types', () => { { type: 'models', path: 'models/config.json', shared: true }, { type: 'models', path: 'models/team-a/config.json', shared: false }, { type: 'wiki', path: 'wiki/home.md', shared: true }, - { type: 'wiki', path: 'wiki/team-a/home.md', shared: false }, + { type: 'wiki', path: 'wiki/team-a/home.md', shared: true }, { type: 'hooks', path: 'hooks/pre-commit.json', shared: true }, { type: 'hooks', path: 'hooks/team-a/pre-commit.json', shared: false }, { type: 'mcp', path: 'mcp/servers.json', shared: true }, @@ -104,6 +105,23 @@ describe('isAtSharedRoot and withNamespace across all resource types', () => { }; expect(isAtSharedRoot(item)).toBe(shared); } + + expect(isAtSharedRoot({ + name: 'guide.md', + type: 'docs', + sourcePath: '/local/docs/team-a/guide.md', + relativePath: 'docs/team-a/guide.md', + namespace: 'team-a', + })).toBe(false); + + const nestedDoc: ResourceItem = { + name: 'guide.md', + type: 'docs', + sourcePath: '/local/docs/team-a/guides/guide.md', + relativePath: 'docs/team-a/guides/guide.md', + }; + expect(declaredNamespaceFor(nestedDoc, ['team-a'])).toBe('team-a'); + expect(declaredNamespaceFor(nestedDoc, ['other-team'])).toBeUndefined(); }); it('correctly inserts namespace segments for various resource formats', () => { diff --git a/src/__tests__/review-branch.test.ts b/src/__tests__/review-branch.test.ts index 3f226ed4d..3480b818b 100644 --- a/src/__tests__/review-branch.test.ts +++ b/src/__tests__/review-branch.test.ts @@ -104,6 +104,33 @@ describe('reviewBranchDir — read-only health checks for a pushed branch (#776) expect(findings(review).some((f) => f.check === 'structure' && /namespace "backend"/.test(f.message))).toBe(true); }); + it('reviews newly placeable resource types for undeclared namespaces', async () => { + await fse.ensureDir(path.join(repoDir, 'manifest')); + await fse.writeFile(path.join(repoDir, 'manifest', 'roles.yaml'), [ + 'version: 1', + 'roles:', + ' - id: dev', + ' description: dev', + ' resources:', + ' knowledge: []', + ' skills: []', + ' agents: []', + ' prompts: [frontend]', + '', + ].join('\n')); + await fse.ensureDir(path.join(repoDir, 'prompts', 'backend')); + await fse.writeFile(path.join(repoDir, 'prompts', 'backend', 'review.md'), 'Review this change.\n'); + + const review = await reviewBranchDir(BRANCH, repoDir); + + expect(review.ok).toBe(false); + expect(findings(review).some((f) => ( + f.check === 'structure' + && f.path === 'prompts/backend/review.md' + && /namespace "backend"/.test(f.message) + ))).toBe(true); + }); + it('fails the same resource name in two namespaces that can be active together', async () => { await fse.ensureDir(path.join(repoDir, 'manifest')); await fse.writeFile(path.join(repoDir, 'manifest', 'roles.yaml'), rolesYaml(['frontend', 'backend'])); diff --git a/src/__tests__/workflow-cache-namespaces.test.ts b/src/__tests__/workflow-cache-namespaces.test.ts index 925c741ed..dc017722f 100644 --- a/src/__tests__/workflow-cache-namespaces.test.ts +++ b/src/__tests__/workflow-cache-namespaces.test.ts @@ -70,6 +70,39 @@ steps: `; describe('workflow and prompt namespaces in cache', () => { + it('lets a namespaced workflow override the shared root in legacy unfiltered mode', async () => { + const config = testConfig(); + const repo = config.repo.localPath; + const commit = initializeGit(repo); + + fs.writeFileSync(path.join(repo, 'workflows', 'deploy.yaml'), SHARED_WORKFLOW); + fs.mkdirSync(path.join(repo, 'workflows', 'ops'), { recursive: true }); + fs.writeFileSync(path.join(repo, 'workflows', 'ops', 'deploy.yaml'), WORKFLOW_A); + commit('add legacy shared and namespaced workflows'); + + const result = await syncWorkflowDefinitions(config); + expect(result.invalid).toEqual([]); + expect((await loadCachedWorkflow(config, 'deploy'))?.definition.description) + .toBe('Deploy from namespace A.'); + }); + + it('drops conflicting namespaced workflows in legacy unfiltered mode', async () => { + const config = testConfig(); + const repo = config.repo.localPath; + const commit = initializeGit(repo); + + fs.mkdirSync(path.join(repo, 'workflows', 'ops'), { recursive: true }); + fs.writeFileSync(path.join(repo, 'workflows', 'ops', 'deploy.yaml'), WORKFLOW_A); + fs.mkdirSync(path.join(repo, 'workflows', 'devops'), { recursive: true }); + fs.writeFileSync(path.join(repo, 'workflows', 'devops', 'deploy.yaml'), WORKFLOW_B); + commit('add conflicting legacy namespaced workflows'); + + const result = await syncWorkflowDefinitions(config); + expect(result.invalid.some((diagnostic) => diagnostic.code === 'workflow_namespace_conflict')) + .toBe(true); + expect(await loadCachedWorkflow(config, 'deploy')).toBeNull(); + }); + it('discovers and loads namespaced workflows from workflows//.yaml', async () => { const config = testConfig(); const repo = config.repo.localPath; diff --git a/src/__tests__/workflow-records.test.ts b/src/__tests__/workflow-records.test.ts index 628827b65..8a2998726 100644 --- a/src/__tests__/workflow-records.test.ts +++ b/src/__tests__/workflow-records.test.ts @@ -203,7 +203,11 @@ describe('#1083 workflow run records', () => { it('publishes the failure code of every step failure teamai writes, and only codes it can name', () => { // Codes a `code: '…'` literal in src/workflow writes that never become a run's failure. - const diagnostics = ['unknown_key', 'missing_needs', 'false_if', 'bad_template', 'dotted_name', 'undeclared_output', 'undeclared_end', 'bad_loop'] satisfies Array; + const diagnostics = [ + 'unknown_key', 'missing_needs', 'false_if', 'bad_template', 'dotted_name', + 'undeclared_output', 'undeclared_end', 'bad_loop', + 'workflow_namespace_conflict', 'prompt_namespace_conflict', + ] satisfies Array; const answers = ['invalid_json', 'driver_running', 'invalid_run', 'repeated_response', 'agent_in_path', 'stale_response', 'retried', 'end_reached', 'repeated_report', 'wrong_attempt', 'lease_cancelled', 'interrupted', 'run_unreadable', 'driver_stuck', 'busy']; const source = path.join(path.dirname(fileURLToPath(import.meta.url)), '..', 'workflow'); const written = new Set(fs.readdirSync(source).filter((file) => file.endsWith('.ts')) diff --git a/src/push-namespaces.ts b/src/push-namespaces.ts index 49b1ac61c..a3c418359 100644 --- a/src/push-namespaces.ts +++ b/src/push-namespaces.ts @@ -88,9 +88,23 @@ export const NAMESPACE_AXIS = { * behind. */ export function isAtSharedRoot(item: ResourceItem): boolean { + if (item.namespace) return false; + // A first directory under docs/ or wiki/ is only a namespace when a manifest + // declares it. Undeclared directories are ordinary shared content (#1101). + if (item.type === 'docs' || item.type === 'wiki') return true; return item.relativePath.split('/').length === 2; } +/** Return the declared namespace carried by a docs/wiki path, if any. */ +export function declaredNamespaceFor( + item: ResourceItem, + declaredNamespaces: readonly string[], +): string | undefined { + if (item.type !== 'docs' && item.type !== 'wiki') return undefined; + const candidate = item.relativePath.split('/')[1]; + return candidate && declaredNamespaces.includes(candidate) ? candidate : undefined; +} + /** * Insert `namespace` after the resource root. Only the directory changes, so * the basename — `.yaml` or a legacy `.md` for agents, a bare directory name diff --git a/src/push.ts b/src/push.ts index c5dff6256..5f6d36aa9 100644 --- a/src/push.ts +++ b/src/push.ts @@ -18,7 +18,7 @@ import { getUserHome } from './utils/home.js'; import { getHandler } from './resources/index.js'; import { createSkillPushItem, scanTeamRepoNamespaces } from './resources/skills.js'; import { AgentsHandler } from './resources/agents.js'; -import { deliversEveryNamespace } from './resource-namespaces.js'; +import { deliversEveryNamespace, resolveResourceNamespaces } from './resource-namespaces.js'; import type { GlobalOptions, PendingPushItem, ResourceItem, ResourceType, LocalConfig, TeamaiConfig, State, } from './types.js'; @@ -29,7 +29,7 @@ import { loadRolesManifest, resolveRoleResourceNamespaces, RolesManifestNotFound import { loadProjectsManifest, resolveProjectResourceNamespaces, type ProjectsManifest } from './projects.js'; import { isSafeNamespaceSegment, NAMESPACE_RULE, fallbackNamespaceError } from './manifest-schema.js'; import { - isAtSharedRoot, isPlaceableType, NAMESPACE_AXIS, PLACEABLE_TYPES, placedResourcePath, + declaredNamespaceFor, isAtSharedRoot, isPlaceableType, NAMESPACE_AXIS, PLACEABLE_TYPES, placedResourcePath, resolveProjectNamespace, skillNamespacePath, withNamespace, type PlaceableType, } from './push-namespaces.js'; import { askQuestion, askSelection, isInteractive } from './utils/prompt.js'; @@ -1447,6 +1447,27 @@ async function pushCore( } } + // docs/ and wiki/ allow ordinary nested shared content. Only a first path + // segment declared by a role or project is a namespace, so annotate those + // items before the placement filter decides whether they are already scoped. + const resolvedNamespaces = await resolveResourceNamespaces(localConfig); + if (resolvedNamespaces) { + const declaredByType = { + docs: [ + ...(resolvedNamespaces.activeNamespaces.docs ?? []), + ...resolvedNamespaces.inactiveDocsNamespaces, + ], + wiki: [ + ...(resolvedNamespaces.activeNamespaces.wiki ?? []), + ...resolvedNamespaces.inactiveWikiNamespaces, + ], + }; + for (const item of fullScan) { + if (item.type !== 'docs' && item.type !== 'wiki') continue; + item.namespace ??= declaredNamespaceFor(item, declaredByType[item.type]); + } + } + // A project that cannot answer for agents is reported below for an agent the // scan itself dropped — skipped as "no active source" with `needsDestination` // set. That one never reaches the listing, so deferring its error until the diff --git a/src/review-branch.ts b/src/review-branch.ts index 98dec8b05..5cdbcd62b 100644 --- a/src/review-branch.ts +++ b/src/review-branch.ts @@ -27,11 +27,10 @@ import fse from 'fs-extra'; import { resolveConfigForDir } from './config.js'; import { caseFoldKey, isSafeNamespaceSegment, NAMESPACE_RULE } from './manifest-schema.js'; import { ALL_PROJECT_RESOURCE_TYPES, loadProjectsManifest, type AllProjectResourceType, type ProjectsManifest } from './projects.js'; -import { NAMESPACE_AXIS, type PlaceableType } from './push-namespaces.js'; +import { NAMESPACE_AXIS, PLACEABLE_TYPES, type PlaceableType } from './push-namespaces.js'; import { parseAgentYaml } from './resources/agent-format.js'; -import { AgentsHandler } from './resources/agents.js'; -import { RulesHandler } from './resources/rules.js'; -import { SkillsHandler } from './resources/skills.js'; +import { getHandler } from './resources/index.js'; +import { listRepoFilesRecursive } from './resources/repo-resource.js'; import { loadRolesManifestIfPresent, type RolesManifest } from './roles.js'; import type { GlobalOptions, LocalConfig, ResourceItem, TeamaiConfig } from './types.js'; import { splitFrontmatter } from './utils/frontmatter.js'; @@ -92,30 +91,53 @@ function stripExtension(segment: string): string { * them (`//` is namespaced, `/` is shared). The * handlers' own `name` is not used: for rules it carries the namespace segment. */ -function parseRelativePath(relativePath: string): { name: string; namespace?: string } { +function parseRelativePath( + type: PlaceableType, + relativePath: string, + index: ManifestIndex, +): { name: string; namespace?: string } { const rest = relativePath.split('/').slice(1); if (rest.length <= 1) { return { name: stripExtension(rest[0] ?? '') }; } + const namespace = rest[0]; + if ((type === 'docs' || type === 'wiki') && !index.declares(NAMESPACE_AXIS[type], namespace!)) { + return { name: stripExtension(rest[rest.length - 1] ?? '') }; + } return { name: stripExtension(rest[rest.length - 1] ?? ''), - namespace: rest[0], + namespace, }; } /** Enumerate the resources on the branch through the same scanners pull uses. */ -async function scanResources(repoDir: string): Promise { - // `scanTeamForPull` reads only `repo.localPath` for skills, rules and agents; - // the rest of the configs are placeholders, as in the handlers' own tests. +async function scanResources(repoDir: string, index: ManifestIndex): Promise { const localConfig = { repo: { localPath: repoDir } } as LocalConfig; const teamConfig = {} as TeamaiConfig; - const items: Array<{ type: PlaceableType; item: ResourceItem }> = [ - ...(await new SkillsHandler().scanTeamForPull(teamConfig, localConfig)).map((item) => ({ type: 'skills' as const, item })), - ...(await new RulesHandler().scanTeamForPull(teamConfig, localConfig)).map((item) => ({ type: 'rules' as const, item })), - ...(await new AgentsHandler().scanTeamForPull(teamConfig, localConfig)).map((item) => ({ type: 'agents' as const, item })), - ]; + const handlerTypes = ['skills', 'rules', 'agents', 'mcp'] as const; + const items: Array<{ type: PlaceableType; item: ResourceItem }> = []; + for (const type of handlerTypes) { + items.push(...(await getHandler(type).scanTeamForPull(teamConfig, localConfig)) + .map((item) => ({ type, item }))); + } + for (const type of PLACEABLE_TYPES) { + if ((handlerTypes as readonly PlaceableType[]).includes(type)) continue; + const root = path.join(repoDir, type); + for (const file of await listRepoFilesRecursive(root)) { + if (type === 'workflows' && !file.endsWith('.yaml')) continue; + items.push({ + type, + item: { + type, + name: file, + sourcePath: path.join(root, ...file.split('/')), + relativePath: `${type}/${file}`, + }, + }); + } + } return items.map(({ type, item }) => { - const { name, namespace } = parseRelativePath(item.relativePath); + const { name, namespace } = parseRelativePath(type, item.relativePath, index); return { type, name, namespace, relativePath: item.relativePath, absolutePath: item.sourcePath }; }); } @@ -426,7 +448,7 @@ export async function reviewBranchDir(branch: string, repoDir: string): Promise< loadProjectsManifest(repoDir), ]); const index = buildManifestIndex(roles, projects); - const resources = await scanResources(repoDir); + const resources = await scanResources(repoDir, index); const readText = makeTextReader(); const findings: Finding[] = [ @@ -446,7 +468,7 @@ export async function reviewBranchDir(branch: string, repoDir: string): Promise< // ─── Read-only branch reader ───────────────────────────────────────────────── /** The repo roots read out of the branch; everything else is irrelevant here. */ -const READ_ROOTS = ['skills', 'rules', 'agents', 'manifest']; +const READ_ROOTS = [...PLACEABLE_TYPES, 'manifest']; /** * The ref to read `branch` from: the name itself when it resolves locally, else diff --git a/src/workflow/cache.ts b/src/workflow/cache.ts index 3e1921bcc..b2adbd26f 100644 --- a/src/workflow/cache.ts +++ b/src/workflow/cache.ts @@ -227,6 +227,11 @@ export async function syncWorkflowDefinitions( const resourceNamespaces = await resolveResourceNamespaces(config); const activeWorkflows = resourceNamespaces?.activeNamespaces.workflows ?? (resourceNamespaces ? [] : null); const activePrompts = resourceNamespaces?.activeNamespaces.prompts ?? (resourceNamespaces ? [] : null); + const effectiveActiveWorkflows = activeWorkflows ?? [ + ...new Set(rawCandidates.flatMap((candidate) => ( + candidate.namespace === null ? [] : [candidate.namespace] + ))), + ]; const byStem = new Map>(); for (const cand of rawCandidates) { @@ -238,11 +243,6 @@ export async function syncWorkflowDefinitions( const toCheck: Array<{ stem: string; relativePath: string; full: string }> = []; for (const [stem, group] of byStem) { - if (activeWorkflows === null) { - if (group[0]) toCheck.push(group[0]); - continue; - } - const namespaceCandidatesList: NamespaceCandidate<{ stem: string; relativePath: string; full: string }>[] = group.map((c) => ({ name: c.stem, source: c.relativePath, @@ -250,7 +250,7 @@ export async function syncWorkflowDefinitions( value: c, })); - const resolution = resolveNamespacedItems(namespaceCandidatesList, activeWorkflows); + const resolution = resolveNamespacedItems(namespaceCandidatesList, effectiveActiveWorkflows); if (resolution.kind === 'conflict') { checked.push({ stem, From b48ef1f2522ec5ee5eb7b595304fca167e2e9d1b Mon Sep 17 00:00:00 2001 From: legend91019 <15853153143@163.com> Date: Wed, 21 Oct 2026 16:52:02 +0800 Subject: [PATCH 03/28] fix(push): read manifests after pull and guard capture/restore paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three CI failures rooted in the same family of pre-pull / path-confusion bugs introduced with the namespace placement work: 1. syncWorkflowDefinitions unconditionally resolved resource namespaces (reading projects.yaml and roles.yaml) from validateWorkflows, which runs BEFORE pushCore refreshes the clone. A stale manifest mapped new resources to the wrong namespace (#649 review). Now the manifest is only read when namespaced workflow candidates exist, wrapped in try/catch so an unreadable manifest degrades to on-disk namespaces. 2. resolveResourceNamespaces at the docs/wiki annotation site had no error handling: a manifest that exists but cannot be read produced an unhandled rejection instead of the graceful exitCode=2 stop every other manifest-error path uses. 3. The pre-pull snapshot capture was widened from env-only to every placeable type, but restorePendingEnvFiles writes to clone/. For project-local sources (tool-format agents, rules, skills) that wrote tool-format bytes over the clone's canonical files, so the next scan reported "cannot read or parse canonical agent YAML" and dropped rules entirely. Now only sources inside the team repo clone are captured — the ones resetToCleanMaster can destroy. --- src/push.ts | 19 ++++++++++++++++++- src/workflow/cache.ts | 16 +++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/push.ts b/src/push.ts index 5f6d36aa9..84aca64e4 100644 --- a/src/push.ts +++ b/src/push.ts @@ -1141,8 +1141,15 @@ async function pushCore( renamed: status.renamed, })); const scannedGroups = await repoScan; + const cloneRoot = path.resolve(localConfig.repo.localPath); for (const item of scannedGroups.flat()) { if (indexedPaths.has(item.relativePath)) continue; + // Only capture items whose source lives inside the team repo clone — + // those are the ones `resetToCleanMaster` can destroy. A project-local + // source (a tool-format agent, rule or skill) is untouched by the + // refresh, and restoring its bytes under `relativePath` would write + // tool-format content over the clone's canonical file. + if (!path.resolve(item.sourcePath).startsWith(cloneRoot + path.sep)) continue; const snapshot = await readFileSnapshot(item.sourcePath); if (snapshot !== null && !await hasGitModeChange(git, item.relativePath)) { pendingEnvFiles.set(item.relativePath, snapshot); @@ -1450,7 +1457,17 @@ async function pushCore( // docs/ and wiki/ allow ordinary nested shared content. Only a first path // segment declared by a role or project is a namespace, so annotate those // items before the placement filter decides whether they are already scoped. - const resolvedNamespaces = await resolveResourceNamespaces(localConfig); + // A manifest that exists but cannot be read must stop the push: every + // placement below would fall through to the shared root, the widening #649 + // is about — and nobody asked for it. + let resolvedNamespaces: Awaited> = null; + try { + resolvedNamespaces = await resolveResourceNamespaces(localConfig); + } catch (e) { + log.error(`${(e as Error).message}`); + process.exitCode = 2; + return; + } if (resolvedNamespaces) { const declaredByType = { docs: [ diff --git a/src/workflow/cache.ts b/src/workflow/cache.ts index b2adbd26f..b86c5c54e 100644 --- a/src/workflow/cache.ts +++ b/src/workflow/cache.ts @@ -224,7 +224,21 @@ export async function syncWorkflowDefinitions( // dir missing or unreadable } - const resourceNamespaces = await resolveResourceNamespaces(config); + // Resolve namespaces only when namespaced candidates exist. A root-only + // tree needs no manifest read, and an unconditional read here would fire + // before the push's pull refreshes the clone — the stale mapping the #649 + // review forbids. A manifest that cannot be read leaves the resolution + // null, which falls back to the set of namespaces found on disk. + const hasNamespaced = rawCandidates.some((c) => c.namespace !== null); + let resourceNamespaces: Awaited> = null; + if (hasNamespaced) { + try { + resourceNamespaces = await resolveResourceNamespaces(config); + } catch { + // The push path reports unreadable manifests properly; here a null + // resolution just means every on-disk namespace is treated as active. + } + } const activeWorkflows = resourceNamespaces?.activeNamespaces.workflows ?? (resourceNamespaces ? [] : null); const activePrompts = resourceNamespaces?.activeNamespaces.prompts ?? (resourceNamespaces ? [] : null); const effectiveActiveWorkflows = activeWorkflows ?? [ From 7961eb0cc42bc576f9025bb3232859e940512ba1 Mon Sep 17 00:00:00 2001 From: legend91019 <15853153143@163.com> Date: Wed, 21 Oct 2026 17:11:23 +0800 Subject: [PATCH 04/28] chore: trigger CI From 514e6c0f9193b8c58e58cb3896b86032e9896012 Mon Sep 17 00:00:00 2001 From: legend91019 <15853153143@163.com> Date: Wed, 21 Oct 2026 17:20:17 +0800 Subject: [PATCH 05/28] chore: nudge CI trigger --- src/push.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/push.ts b/src/push.ts index 84aca64e4..124c6f446 100644 --- a/src/push.ts +++ b/src/push.ts @@ -2133,3 +2133,4 @@ async function pushTeamConfigOnly( return; } } + From 91209e54deb58bb1cd6194252fb47fac83394dbd Mon Sep 17 00:00:00 2001 From: legend91019 <15853153143@163.com> Date: Wed, 21 Oct 2026 17:52:06 +0800 Subject: [PATCH 06/28] fix(push): do not stage pre-placement paths as superseded files placeNewResources marked the pre-placement root path as supersedes when moving a new resource into a namespace. pushRepoBranch then staged that path with git add, but the root file was never in the clone, so git aborted with 'pathspec did not match any files'. supersedes is for pushItem retiring a tracked file during a format rename (agent .md to .yaml); a placement from the shared root to a namespace is not a rename. --- src/push.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/push.ts b/src/push.ts index 4bd35be11..b88404125 100644 --- a/src/push.ts +++ b/src/push.ts @@ -655,12 +655,12 @@ async function placeNewResources(args: { process.exitCode = 2; return false; } - const originalRelPath = item.relativePath; item.namespace = destination.namespace; item.relativePath = placedAt; - if (originalRelPath !== placedAt) { - item.supersedes = originalRelPath; - } + // Do NOT mark the pre-placement path as `supersedes`: the original + // root file was never in the clone, so staging its path would fail + // with "pathspec did not match". `supersedes` is for `pushItem` + // retiring a tracked file during a format rename (agent .md ↔ .yaml). // The silent widening in #649 was the real damage: say where it went. log.info(`[${type}] ${item.name} → ${item.relativePath}`); } From 93809cd9c492406a09f918ed34d1e9cd01e532c4 Mon Sep 17 00:00:00 2001 From: legend91019 <15853153143@163.com> Date: Wed, 21 Oct 2026 22:08:25 +0800 Subject: [PATCH 07/28] =?UTF-8?q?fix:=20address=20review=20findings=20?= =?UTF-8?q?=E2=80=94=20binary=20copy,=20prompt=20namespaces,=20env=20place?= =?UTF-8?q?ment,=20role=20learnings,=20wiki=20source,=20and=20P2s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1 fixes: - repo-resource: use fs.copyFile byte copy instead of UTF-8 text I/O to prevent binary doc corruption - workflow/cache: resolve prompt namespaces independently of workflow layout; support nested prompt paths - env: add status field to envPushItem so placement routes env/env.yaml to env//env.yaml - roles: include learnings in role namespace resolution (#1101: 'Learnings should support --project and --role') - wiki: point handler at teamwiki/evidence/code/ (authoritative source per manifest-schema.ts:112) - push: filter LOCAL_TOOL_TYPES from pre-refresh scan to suppress spurious warnings P2 fixes: - push-namespaces: add rootSegmentsFor() and withNamespace(rootSegments) for wiki's multi-segment root - review-branch: fix nested path identity collision (use full sub-path as name); preserve handler-provided names - review-branch: skip undeclared-namespace check in manifest-free teams - --role: warn when not declared in manifest/roles.yaml - status: count models/claudemd/learnings/wiki/workflows/prompts/scorers - push: simplify pushRepoResourceItem signature to LocalConfig only - docs: update distribution.md and --role/--project help text - push: tighten pre-refresh snapshot to only capture clone-backed items --- docs/guide/distribution.md | 2 +- .../push-all-resources-namespaces.test.ts | 2 +- src/index.ts | 4 +- src/projects.ts | 10 +-- src/push-namespaces.ts | 18 +++-- src/push.ts | 48 ++++++++++--- src/resources/env.ts | 39 ++++++---- src/resources/index.ts | 2 +- src/resources/repo-resource.ts | 14 ++-- src/review-branch.ts | 35 ++++++--- src/roles.ts | 26 +++---- src/status.ts | 11 ++- src/workflow/cache.ts | 71 ++++++++++++------- 13 files changed, 185 insertions(+), 97 deletions(-) diff --git a/docs/guide/distribution.md b/docs/guide/distribution.md index 99310d876..f4a86d344 100644 --- a/docs/guide/distribution.md +++ b/docs/guide/distribution.md @@ -101,7 +101,7 @@ sharing: .teamai/mcp/mcp.yaml ``` -`teamai push` sends skills and rules. Commit `docs/`, `hooks/hooks.yaml`, `mcp/mcp.yaml`, and `models/models.yaml` yourself. +`teamai push` sends every distributed resource type — skills, rules, agents, docs, env, hooks, mcp, models, claudemd, learnings, wiki, workflows, prompts, and scorers — with `--project` and `--role` namespace placement. ### Git permissions diff --git a/src/__tests__/push-all-resources-namespaces.test.ts b/src/__tests__/push-all-resources-namespaces.test.ts index 75260c2c9..1bf1defbd 100644 --- a/src/__tests__/push-all-resources-namespaces.test.ts +++ b/src/__tests__/push-all-resources-namespaces.test.ts @@ -299,7 +299,7 @@ describe('repo resource handler push and scan', () => { expect(scanned[0].status).toBe('new'); // Now push it - await pushRepoResourceItem(scanned[0], {} as any, localConfig); + await pushRepoResourceItem(scanned[0], localConfig); const destFile = path.join(repoLocal, 'workflows', 'ops', 'test.yaml'); expect(fs.existsSync(destFile)).toBe(true); expect(fs.readFileSync(destFile, 'utf8')).toBe('name: test\n'); diff --git a/src/index.ts b/src/index.ts index 3e0831ee1..08bde80a9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -170,10 +170,10 @@ program .description('Push local resources to team repo') .option('--all', 'Push all without confirmation') .option('--skill ', 'Push a specific skill by path (e.g., ~/.claude/skills/hai/my-skill or skills/hai_dev/my-skill)') - .option('--role ', 'Namespace for new skills, rules and agents (skills//, rules//, agents//). ' + .option('--role ', 'Namespace for every new resource type (skills//, rules//, docs//, env//, etc.). ' + 'Without --role or --project, the namespaces of your role and active projects decide') .option('--project ', "Target a project: each new resource goes to that project's namespace for its own type " - + '— skills, knowledge for rules, agents (from manifest/projects.yaml)') + + '— skills, knowledge for rules, agents, learnings, docs, env, hooks, mcp, models, and more (from manifest/projects.yaml)') .option('--branch ', 'Push to this destination branch instead of a generated teamai/push branch') .action(async (cmdOpts) => { const globalOpts = program.opts() as GlobalOptions; diff --git a/src/projects.ts b/src/projects.ts index a32e489d8..e09f65d8f 100644 --- a/src/projects.ts +++ b/src/projects.ts @@ -201,11 +201,11 @@ function getProjectOrThrow(manifest: ProjectsManifest, projectId: string): TeamP * Resolve the resource namespaces contributed by the given active projects, as a * deduped union across all three resource types (knowledge/skills/learnings). * - * This is the ONLY source of learnings namespaces. Roles never contribute them. - * The caller unions the result with `resolveRoleResourceNamespaces(...)` on the - * knowledge/skills axes; there is no priority override between the two dimensions - * (same-named resources across a role and a project namespace are an admin-side - * duplicate error, not a runtime precedence decision). + * Both projects and roles contribute learnings namespaces (#1101). The caller + * unions the result with `resolveRoleResourceNamespaces(...)` on every axis; + * there is no priority override between the two dimensions (same-named resources + * across a role and a project namespace are an admin-side duplicate error, not a + * runtime precedence decision). */ export function resolveProjectResourceNamespaces(input: { manifest: ProjectsManifest; diff --git a/src/push-namespaces.ts b/src/push-namespaces.ts index a3c418359..06f77d10a 100644 --- a/src/push-namespaces.ts +++ b/src/push-namespaces.ts @@ -101,7 +101,8 @@ export function declaredNamespaceFor( declaredNamespaces: readonly string[], ): string | undefined { if (item.type !== 'docs' && item.type !== 'wiki') return undefined; - const candidate = item.relativePath.split('/')[1]; + const segments = item.relativePath.split('/'); + const candidate = segments[rootSegmentsFor(item.type)]; return candidate && declaredNamespaces.includes(candidate) ? candidate : undefined; } @@ -109,10 +110,19 @@ export function declaredNamespaceFor( * Insert `namespace` after the resource root. Only the directory changes, so * the basename — `.yaml` or a legacy `.md` for agents, a bare directory name * for skills — survives without the caller knowing which it has. + * + * `rootSegments` is how many leading path segments form the root before the + * namespace insertion point: 1 for `docs/`, `skills/` etc., 3 for wiki's + * `teamwiki/evidence/code/`. */ -export function withNamespace(relativePath: string, namespace: string): string { - const [root, ...rest] = relativePath.split('/'); - return [root, namespace, ...rest].join('/'); +export function withNamespace(relativePath: string, namespace: string, rootSegments = 1): string { + const parts = relativePath.split('/'); + return [...parts.slice(0, rootSegments), namespace, ...parts.slice(rootSegments)].join('/'); +} + +/** Leading path segments before the namespace slot, per resource type. */ +export function rootSegmentsFor(type: PlaceableType): number { + return type === 'wiki' ? 3 : 1; } /** diff --git a/src/push.ts b/src/push.ts index b88404125..89fc3786b 100644 --- a/src/push.ts +++ b/src/push.ts @@ -30,12 +30,20 @@ import { loadProjectsManifest, resolveProjectResourceNamespaces, type ProjectsMa import { isSafeNamespaceSegment, NAMESPACE_RULE, fallbackNamespaceError } from './manifest-schema.js'; import { declaredNamespaceFor, isAtSharedRoot, isPlaceableType, NAMESPACE_AXIS, PLACEABLE_TYPES, placedResourcePath, - resolveProjectNamespace, skillNamespacePath, withNamespace, type PlaceableType, + resolveProjectNamespace, rootSegmentsFor, skillNamespacePath, withNamespace, type PlaceableType, } from './push-namespaces.js'; import { askQuestion, askSelection, isInteractive } from './utils/prompt.js'; import { pathExists, pruneEmptyDirs, readFileSafe, writeFile } from './utils/fs.js'; import { brokenTeamProfileFiles } from './models/profile.js'; +/** + * Types whose `scanLocalForPush` source is the tool's own file (outside the + * clone), not a clone-backed path. The pre-refresh snapshot must exclude them: + * scanning them fires spurious warnings (e.g. "Skipped b-skill") and their + * source is untouched by `resetToCleanMaster` anyway. + */ +const LOCAL_TOOL_TYPES: ReadonlySet = new Set(['skills', 'rules', 'agents']); + /** * Filter a list of repo-root-relative paths (e.g. "rules/", ".codebuddy-plugin/") down to * those that actually exist on disk. `git add` throws `pathspec did not match @@ -552,7 +560,7 @@ function reuseRecordedDestinations(groups: PushGroup[]): Set { // A rule or agent carries its own path from the scanner, and that path // is authoritative when it already names a namespace (#654). item.namespace = ns; - item.relativePath = withNamespace(item.relativePath, ns); + item.relativePath = withNamespace(item.relativePath, ns, rootSegmentsFor(item.type)); } } } @@ -633,7 +641,7 @@ async function placeNewResources(args: { continue; case 'namespace': for (const item of newAtRoot) { - const placedAt = withNamespace(item.relativePath, destination.namespace); + const placedAt = withNamespace(item.relativePath, destination.namespace, rootSegmentsFor(type)); // This resource is NEW here, so nothing of ours is at that path yet. // Anything already there is somebody else's, and `pushItem` writes // rather than merges: placing on top of it would replace their work @@ -1114,7 +1122,7 @@ async function pushCore( const workingContent = await readFileSafe(yamlPath); // Nothing below writes before these reads finish, so they run together: const repoScan = Promise.all( - PLACEABLE_TYPES.map(async (t) => { + PLACEABLE_TYPES.filter((t) => !LOCAL_TOOL_TYPES.has(t)).map(async (t) => { try { return await getHandler(t).scanLocalForPush(teamConfig, localConfig); } catch { @@ -1150,12 +1158,12 @@ async function pushCore( const cloneRoot = path.resolve(localConfig.repo.localPath); for (const item of scannedGroups.flat()) { if (indexedPaths.has(item.relativePath)) continue; - // Only capture items whose source lives inside the team repo clone — - // those are the ones `resetToCleanMaster` can destroy. A project-local - // source (a tool-format agent, rule or skill) is untouched by the - // refresh, and restoring its bytes under `relativePath` would write - // tool-format content over the clone's canonical file. - if (!path.resolve(item.sourcePath).startsWith(cloneRoot + path.sep)) continue; + // Only capture items whose source is the clone file at its relativePath + // — the ones `resetToCleanMaster` can destroy. A project-local source + // (a tool-format agent, rule or skill) is untouched by the refresh, and + // restoring its bytes under `relativePath` would write tool-format + // content over the clone's canonical file. + if (path.resolve(item.sourcePath) !== path.resolve(cloneRoot, ...item.relativePath.split('/'))) continue; const snapshot = await readFileSnapshot(item.sourcePath); if (snapshot !== null && !await hasGitModeChange(git, item.relativePath)) { pendingEnvFiles.set(item.relativePath, snapshot); @@ -1701,6 +1709,26 @@ async function pushCore( process.exitCode = 2; return; } + // Warn when the role names no namespace any manifest declares: pull would + // ignore the pushed paths. A manifest-free team is exempt — every on-disk + // namespace is valid there. + try { + const manifest = await loadRolesManifest(localConfig.repo.localPath); + const declared = new Set(); + for (const role of manifest.roles) { + for (const list of Object.values(role.resources) as (string[] | undefined)[]) { + for (const ns of list ?? []) declared.add(ns); + } + } + if (declared.size > 0 && !declared.has(options.role)) { + log.warn( + `--role ${options.role} is not declared in manifest/roles.yaml for any resource axis; ` + + 'pull will not deliver resources placed there. Declare it or pick a known namespace.', + ); + } + } catch { + // Manifest missing or unreadable: the push path reports that separately. + } skillsDestination = options.role; } else if (options.project && projectsManifest && allItems.some(isRelocatedByFlag)) { const resolved = resolveProjectNamespace(projectsManifest, options.project, 'skills'); diff --git a/src/resources/env.ts b/src/resources/env.ts index 5dd3e98e0..a8cf0e71f 100644 --- a/src/resources/env.ts +++ b/src/resources/env.ts @@ -3,7 +3,7 @@ import { readFile, realpath } from 'node:fs/promises'; import { z } from 'zod'; import YAML from 'yaml'; import { ResourceHandler } from './base.js'; -import type { ResourceItem, TeamaiConfig, LocalConfig } from '../types.js'; +import type { ResourceItem, ResourceItemStatus, TeamaiConfig, LocalConfig } from '../types.js'; import { TEAMAI_ENV_START, TEAMAI_ENV_END, getDataHome, getEnvBackupPath, getTeamaiHome, isSelfMode } from '../types.js'; import { pathExists, readFileIfExists, readFileSafe, writeFile, ensureDir, fileContentEqual, remove, symlinkTarget } from '../utils/fs.js'; import { log } from '../utils/logger.js'; @@ -285,8 +285,8 @@ function parseEnvYamlDocument(raw: unknown, label: string): EnvYamlRead { return { ok: true, variables: parsed.data.variables }; } -function envPushItem(relativePath: string, sourcePath: string): ResourceItem { - return { name: relativePath.slice('env/'.length), type: 'env', sourcePath, relativePath }; +function envPushItem(relativePath: string, sourcePath: string, status: ResourceItemStatus = 'modified'): ResourceItem { + return { name: relativePath.slice('env/'.length), type: 'env', sourcePath, relativePath, status }; } /** A `source` or `.` command naming a file called env.sh. */ @@ -342,8 +342,9 @@ export class EnvHandler extends ResourceHandler { for (const { relativePath, absolutePath: activeEnv } of await listEnvFiles(activeRoot)) { const baseEnv = path.join(localConfig.repo.localPath, ...relativePath.split('/')); // Not in the baseline → new; present but different → modified; equal → skip. - if (await pathExists(baseEnv) && await fileContentEqual(activeEnv, baseEnv)) continue; - items.push(envPushItem(relativePath, activeEnv)); + const baseExists = await pathExists(baseEnv); + if (baseExists && await fileContentEqual(activeEnv, baseEnv)) continue; + items.push(envPushItem(relativePath, activeEnv, baseExists ? 'modified' : 'new')); } return items; } @@ -353,17 +354,26 @@ export class EnvHandler extends ResourceHandler { const { promisify } = await import('node:util'); const execFileAsync = promisify(execFile); - // Modified and untracked files under env/, in one call. A file git cannot - // report on (no repository) is treated as changed, as before. `-z` keeps a + // Modified and untracked files under env/. A file git cannot report on + // (no repository) is treated as changed, as before. `-z` keeps a // non-ASCII path unquoted, so it matches the name on disk. let changed: Set | null = null; + let untracked: Set = new Set(); try { - const { stdout } = await execFileAsync( - 'git', - ['ls-files', '-z', '--modified', '--others', '--exclude-standard', '--', 'env'], - { cwd: repoPath }, - ); - changed = new Set(stdout.split('\0').filter(Boolean)); + const [changedRes, untrackedRes] = await Promise.all([ + execFileAsync( + 'git', + ['ls-files', '-z', '--modified', '--others', '--exclude-standard', '--', 'env'], + { cwd: repoPath }, + ), + execFileAsync( + 'git', + ['ls-files', '-z', '--others', '--exclude-standard', '--', 'env'], + { cwd: repoPath }, + ), + ]); + changed = new Set(changedRes.stdout.split('\0').filter(Boolean)); + untracked = new Set(untrackedRes.stdout.split('\0').filter(Boolean)); } catch { changed = null; } @@ -371,7 +381,8 @@ export class EnvHandler extends ResourceHandler { const items: ResourceItem[] = []; for (const { relativePath, absolutePath } of await listEnvFiles(repoPath)) { if (changed && !changed.has(relativePath)) continue; - items.push(envPushItem(relativePath, absolutePath)); + const status = untracked.has(relativePath) || !changed ? 'new' : 'modified'; + items.push(envPushItem(relativePath, absolutePath, status)); } return items; } diff --git a/src/resources/index.ts b/src/resources/index.ts index 9e151ea75..f94b89225 100644 --- a/src/resources/index.ts +++ b/src/resources/index.ts @@ -20,7 +20,7 @@ const handlers: Record = { models: new RepoResourceHandler('models', 'models'), claudemd: new RepoResourceHandler('claudemd', 'claudemd'), learnings: new RepoResourceHandler('learnings', 'learnings'), - wiki: new RepoResourceHandler('wiki', 'wiki'), + wiki: new RepoResourceHandler('wiki', 'teamwiki/evidence/code'), workflows: new RepoResourceHandler('workflows', 'workflows', (f) => f.endsWith('.yaml')), prompts: new RepoResourceHandler('prompts', 'prompts'), scorers: new RepoResourceHandler('scorers', 'scorers'), diff --git a/src/resources/repo-resource.ts b/src/resources/repo-resource.ts index 410fc400c..cd00cb5ea 100644 --- a/src/resources/repo-resource.ts +++ b/src/resources/repo-resource.ts @@ -2,7 +2,7 @@ import path from 'node:path'; import fs from 'node:fs/promises'; import { ResourceHandler } from './base.js'; import { isSelfMode, type LocalConfig, type ResourceItem, type ResourceType, type TeamaiConfig } from '../types.js'; -import { ensureDir, fileContentEqual, pathExists, readFileSafe, writeFile } from '../utils/fs.js'; +import { ensureDir, fileContentEqual, pathExists } from '../utils/fs.js'; export async function listRepoFilesRecursive(dir: string, baseDir = dir): Promise { try { @@ -98,18 +98,16 @@ export async function scanRepoResourceForPush( export async function pushRepoResourceItem( item: ResourceItem, - localConfigOrTeamConfig: TeamaiConfig | LocalConfig, - maybeLocalConfig?: LocalConfig, + localConfig: LocalConfig, ): Promise { - const localConfig = maybeLocalConfig ?? (localConfigOrTeamConfig as LocalConfig); const dest = path.resolve(localConfig.repo.localPath, ...item.relativePath.split('/')); const originalSource = path.resolve(item.sourcePath); if (originalSource !== dest) { await ensureDir(path.dirname(dest)); - const content = await readFileSafe(originalSource); - if (content !== null) { - await writeFile(dest, content); - } + // Byte-level copy: a PNG or other binary under docs/ must survive intact. + // readFileSafe/writeFile are UTF-8 text I/O and corrupt non-text bytes + // (e.g. 0x89 → U+FFFD → 0xEF 0xBF 0xBD on round-trip). + await fs.copyFile(originalSource, dest); const repoRoot = path.resolve(localConfig.repo.localPath); if (originalSource.startsWith(repoRoot + path.sep)) { await fs.rm(originalSource, { force: true }); diff --git a/src/review-branch.ts b/src/review-branch.ts index 5cdbcd62b..821725bef 100644 --- a/src/review-branch.ts +++ b/src/review-branch.ts @@ -27,7 +27,7 @@ import fse from 'fs-extra'; import { resolveConfigForDir } from './config.js'; import { caseFoldKey, isSafeNamespaceSegment, NAMESPACE_RULE } from './manifest-schema.js'; import { ALL_PROJECT_RESOURCE_TYPES, loadProjectsManifest, type AllProjectResourceType, type ProjectsManifest } from './projects.js'; -import { NAMESPACE_AXIS, PLACEABLE_TYPES, type PlaceableType } from './push-namespaces.js'; +import { NAMESPACE_AXIS, PLACEABLE_TYPES, rootSegmentsFor, type PlaceableType } from './push-namespaces.js'; import { parseAgentYaml } from './resources/agent-format.js'; import { getHandler } from './resources/index.js'; import { listRepoFilesRecursive } from './resources/repo-resource.js'; @@ -96,18 +96,20 @@ function parseRelativePath( relativePath: string, index: ManifestIndex, ): { name: string; namespace?: string } { - const rest = relativePath.split('/').slice(1); + const rootSegs = rootSegmentsFor(type); + const all = relativePath.split('/'); + const rest = all.slice(rootSegs); if (rest.length <= 1) { return { name: stripExtension(rest[0] ?? '') }; } const namespace = rest[0]; + // Keep the full sub-path as the name so nested docs/wiki files in different + // directories don't collide on their basename alone. + const name = stripExtension(rest.slice(1).join('/')); if ((type === 'docs' || type === 'wiki') && !index.declares(NAMESPACE_AXIS[type], namespace!)) { - return { name: stripExtension(rest[rest.length - 1] ?? '') }; + return { name: stripExtension(rest.join('/')) }; } - return { - name: stripExtension(rest[rest.length - 1] ?? ''), - namespace, - }; + return { name, namespace }; } /** Enumerate the resources on the branch through the same scanners pull uses. */ @@ -137,6 +139,18 @@ async function scanResources(repoDir: string, index: ManifestIndex): Promise { + // Handler-provided names (MCP server names, skill names) are authoritative; + // parseRelativePath would overwrite them with the filename stem (e.g. 'mcp' + // from mcp//mcp.yaml), collapsing distinct servers into one identity. + if ((handlerTypes as readonly PlaceableType[]).includes(type)) { + return { + type, + name: item.name, + namespace: item.namespace, + relativePath: item.relativePath, + absolutePath: item.sourcePath, + }; + } const { name, namespace } = parseRelativePath(type, item.relativePath, index); return { type, name, namespace, relativePath: item.relativePath, absolutePath: item.sourcePath }; }); @@ -152,6 +166,8 @@ interface ManifestIndex { coActive(axis: AllProjectResourceType, a: string, b: string): boolean; /** True when any role or project declares any namespace on `axis`. */ anyOn(axis: AllProjectResourceType): boolean; + /** True when no manifest exists at all — every on-disk namespace is valid. */ + manifestFree: boolean; } function buildManifestIndex(roles: RolesManifest | null, projects: ProjectsManifest | null): ManifestIndex { @@ -185,6 +201,7 @@ function buildManifestIndex(roles: RolesManifest | null, projects: ProjectsManif }); }, anyOn: (axis) => listsByAxis.get(axis)!.length > 0, + manifestFree: !roles && !projects, }; } @@ -231,7 +248,9 @@ async function structureFindings(resources: ReviewResource[], index: ManifestInd } if (resource.namespace) { const axis = NAMESPACE_AXIS[resource.type]; - if (!index.declares(axis, resource.namespace)) { + // A manifest-free team has no declarations to check against — every + // on-disk namespace is a valid pull source. + if (!index.manifestFree && !index.declares(axis, resource.namespace)) { findings.push({ level: 'fail', check: 'structure', diff --git a/src/roles.ts b/src/roles.ts index 2ce3ba9c1..d1d55fa41 100644 --- a/src/roles.ts +++ b/src/roles.ts @@ -19,10 +19,7 @@ const RoleResourceNamespacesSchema = z.object({ // Optional: a role without `agents` receives root-level agents only, which // is what every manifest written before this key existed already got. agents: z.array(NamespaceSegmentSchema).default([]), - // learnings is accepted for backward compatibility but ignored at runtime. - // All learnings are shared flat across the entire team (no namespace isolation). - // It never becomes a directory here, so it stays a plain string: holding an old - // manifest to the namespace rule would reject it over a field nothing reads. + // learnings namespaces: roles and projects both contribute them (#1101). learnings: z.array(z.string()).optional(), // env, hooks, mcp, models, docs: optional, see HAND_DECLARED_RESOURCE_TYPES. ...HandDeclaredNamespacesShape, @@ -46,11 +43,9 @@ export type TeamRole = z.infer; export type RolesManifest = z.infer; /** - * Active resource namespaces after resolving roles ∪ projects. `learnings` is - * always present but only projects ever populate it (roles leave it empty — see - * the note on RoleResourceNamespacesSchema). Kept as a superset of the role - * resource types so role and project resolutions share one shape and can be - * unioned directly. + * Active resource namespaces after resolving roles ∪ projects. Both dimensions + * contribute `learnings` (#1101). Kept as a superset of the role resource types + * so role and project resolutions share one shape and can be unioned directly. */ export type ResourceNamespaces = Record // Optional so a resolution built before a type existed (or a test double of @@ -104,7 +99,7 @@ function validateManifestShape(raw: unknown): RolesManifest { /** Every namespace a roles manifest puts to use, with the role that declares it. */ export function roleNamespaceEntries(manifest: RolesManifest): NamespaceEntry[] { return manifest.roles.flatMap((role) => - NAMESPACED_RESOURCE_TYPES.flatMap((type) => + [...NAMESPACED_RESOURCE_TYPES, 'learnings' as const].flatMap((type) => (role.resources[type] ?? []).map((namespace) => ({ type, namespace, owner: `role ${role.id}` })), ), ); @@ -209,16 +204,17 @@ export function resolveRoleResourceNamespaces(input: { ...input.additionalRoles.map((roleId) => getRoleOrThrow(input.manifest, roleId)), ]; - // Roles never contribute learnings namespaces; only projects do. Kept empty - // so the shape matches project resolution for a clean union at the call site. - // A hand-declared type (env, hooks, mcp, models, docs) is absent until one is active. + // Roles and projects both contribute learnings namespaces (#1101 comment: + // "Learnings should support --project and --role namespaces just like the + // other resources"). A hand-declared type (env, hooks, mcp, models, docs) is + // absent until one is active. const namespaces: ResourceNamespaces = { knowledge: [], skills: [], learnings: [], agents: [] }; - for (const type of NAMESPACED_RESOURCE_TYPES) { + for (const type of [...NAMESPACED_RESOURCE_TYPES, 'learnings' as const]) { const seen = new Set(); const out: string[] = []; for (const role of resolvedRoles) { - for (const namespace of role.resources[type] ?? []) { + for (const namespace of (role.resources as Record)[type] ?? []) { if (seen.has(namespace)) continue; seen.add(namespace); out.push(namespace); diff --git a/src/status.ts b/src/status.ts index 3bd9ded5b..f763c13b9 100644 --- a/src/status.ts +++ b/src/status.ts @@ -6,7 +6,7 @@ import { teamRefNotice } from './team-ref.js'; import { getDefaultBranch, getHeadRev, getRepoStatus } from './utils/git.js'; import { assertSafeResourceName } from './utils/path-safety.js'; import { log } from './utils/logger.js'; -import { getAllHandlers } from './resources/index.js'; +import { getAllHandlers, getHandler } from './resources/index.js'; import { listDirs, listFilesRecursive, pathExists, readFileSafe } from './utils/fs.js'; import { SkillsHandler } from './resources/skills.js'; import { DocsHandler } from './resources/docs.js'; @@ -131,6 +131,15 @@ export async function status(options: GlobalOptions): Promise { count('hooks', (await resolveTeamHookEntries(localConfig)).resolution); count('mcp', await resolveEntriesFor(mcpEntryReader, localConfig)); + // Repo-backed types: count what's in the team repo tree. + for (const type of ['models', 'claudemd', 'learnings', 'wiki', 'workflows', 'prompts', 'scorers'] as const) { + try { + counts[type] = (await getHandler(type).scanTeamForPull(teamConfig, localConfig)).length; + } catch { + counts[type] = 0; + } + } + for (const type of RESOURCE_TYPES) { console.log(` ${type}: ${counts[type] ?? 0}${origins[type] ?? ''}`); } diff --git a/src/workflow/cache.ts b/src/workflow/cache.ts index 8773fcc29..f593a3d1b 100644 --- a/src/workflow/cache.ts +++ b/src/workflow/cache.ts @@ -224,12 +224,28 @@ export async function syncWorkflowDefinitions( // dir missing or unreadable } - // Resolve namespaces only when namespaced candidates exist. A root-only - // tree needs no manifest read, and an unconditional read here would fire - // before the push's pull refreshes the clone — the stale mapping the #649 - // review forbids. A manifest that cannot be read leaves the resolution - // null, which falls back to the set of namespaces found on disk. - const hasNamespaced = rawCandidates.some((c) => c.namespace !== null); + // Resolve namespaces when namespaced candidates exist on disk — workflows + // OR prompts. A root-only tree needs no manifest read, and an unconditional + // read here would fire before the push's pull refreshes the clone — the + // stale mapping the #649 review forbids. A manifest that cannot be read + // leaves the resolution null, which falls back to the set of namespaces + // found on disk. + let hasNamespaced = rawCandidates.some((c) => c.namespace !== null); + if (!hasNamespaced) { + // Prompt namespaces are independent of workflow layout: a root-only + // workflows/ tree with prompts// still needs resolution. + try { + const promptsDir = path.join(knowledgeRoot, 'prompts'); + for (const entry of await fs.readdir(promptsDir, { withFileTypes: true })) { + if (entry.isDirectory() && !entry.isSymbolicLink() && isSafeNamespaceSegment(entry.name)) { + hasNamespaced = true; + break; + } + } + } catch { + // prompts dir missing or unreadable + } + } let resourceNamespaces: Awaited> = null; if (hasNamespaced) { try { @@ -539,27 +555,28 @@ async function resolvePromptFiles( const promptRelative = promptPath.replace(/^[\\/]/, ''); if (promptRelative.startsWith('prompts/') && activePromptsNamespaces && activePromptsNamespaces.length > 0) { const leaf = promptRelative.slice('prompts/'.length); - if (!leaf.includes('/')) { - const found = (await Promise.all( - activePromptsNamespaces.map(async (ns) => { - const candidate = path.join(root, 'prompts', ns, leaf); - return (await pathExists(candidate)) ? `prompts/${ns}/${leaf}` : null; - }), - )).filter((p): p is string => p !== null); - - if (found.length === 1) { - promptPath = found[0]; - } else if (found.length > 1) { - diagnostics.push({ - file: workflowFile, - line: location.line, - column: location.column, - field: `${field}.prompt_file`, - code: 'prompt_namespace_conflict', - message: `prompt_file "${step.promptFile}" is ambiguous between active namespaces (${found.join(', ')}); fix the conflict or specify the prompt path explicitly`, - }); - continue; - } + // Namespace lookup works for any leaf depth — a nested path like + // prompts/topic/note.md resolves to prompts//topic/note.md + // just as a flat prompts/note.md resolves to prompts//note.md. + const found = (await Promise.all( + activePromptsNamespaces.map(async (ns) => { + const candidate = path.join(root, 'prompts', ns, ...leaf.split('/')); + return (await pathExists(candidate)) ? `prompts/${ns}/${leaf}` : null; + }), + )).filter((p): p is string => p !== null); + + if (found.length === 1) { + promptPath = found[0]; + } else if (found.length > 1) { + diagnostics.push({ + file: workflowFile, + line: location.line, + column: location.column, + field: `${field}.prompt_file`, + code: 'prompt_namespace_conflict', + message: `prompt_file "${step.promptFile}" is ambiguous between active namespaces (${found.join(', ')}); fix the conflict or specify the prompt path explicitly`, + }); + continue; } } From d77e8f07b6bf84309ab3198a106debfbfc60ef14 Mon Sep 17 00:00:00 2001 From: legend91019 <15853153143@163.com> Date: Wed, 21 Oct 2026 23:09:28 +0800 Subject: [PATCH 08/28] fix: snapshot key mismatch and manifest-error namespace fallback - push: track prePlacementPath so the snapshot delete finds the entry keyed by the root path after placement rewrites item.relativePath - workflow/cache: a manifest that cannot be read now resolves to an empty namespace set (conservative) instead of falling back to 'every namespace active', which cached inactive workflows as runnable - commands.md: regenerate snapshot for updated --role/--project help text --- skill-data/core/references/commands.md | 4 ++-- src/push.ts | 11 ++++++++++- src/workflow/cache.ts | 7 +++++-- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/skill-data/core/references/commands.md b/skill-data/core/references/commands.md index b0277fa9b..0bbbd79df 100644 --- a/skill-data/core/references/commands.md +++ b/skill-data/core/references/commands.md @@ -36,8 +36,8 @@ Generated: do not edit by hand. Regenerate with - `teamai push` — Push local resources to team repo - `--all` — Push all without confirmation - `--skill ` — Push a specific skill by path (e.g., ~/.claude/skills/hai/my-skill or skills/hai_dev/my-skill) - - `--role ` — Namespace for new skills, rules and agents (skills//, rules//, agents//). Without --role or --project, the namespaces of your role and active projects decide - - `--project ` — Target a project: each new resource goes to that project's namespace for its own type — skills, knowledge for rules, agents (from manifest/projects.yaml) + - `--role ` — Namespace for every new resource type (skills//, rules//, docs//, env//, etc.). Without --role or --project, the namespaces of your role and active projects decide + - `--project ` — Target a project: each new resource goes to that project's namespace for its own type — skills, knowledge for rules, agents, learnings, docs, env, hooks, mcp, models, and more (from manifest/projects.yaml) - `--branch ` — Push to this destination branch instead of a generated teamai/push branch ## pull diff --git a/src/push.ts b/src/push.ts index 89fc3786b..286e7a7b6 100644 --- a/src/push.ts +++ b/src/push.ts @@ -664,6 +664,9 @@ async function placeNewResources(args: { return false; } item.namespace = destination.namespace; + // Save the pre-placement path so the snapshot delete (pushGroup + // cleanup) can find the entry keyed by the root path. + (item as ResourceItem & { prePlacementPath?: string }).prePlacementPath = item.relativePath; item.relativePath = placedAt; // Do NOT mark the pre-placement path as `supersedes`: the original // root file was never in the clone, so staging its path would fail @@ -2033,7 +2036,13 @@ async function pushCore( // retrying its PR also takes before it reports pushed), so the rest go back // whatever the outcome; rewriting a file still in place changes nothing. if (outcome === 'pushed' || outcome === 'pr-failed') { - for (const item of group.items) pendingEnvFiles.delete(item.relativePath); + for (const item of group.items) { + // The snapshot was keyed by the pre-placement path; placement rewrote + // item.relativePath to the namespaced destination, so look up both. + pendingEnvFiles.delete(item.relativePath); + const prePath = (item as ResourceItem & { prePlacementPath?: string }).prePlacementPath; + if (prePath) pendingEnvFiles.delete(prePath); + } } const lost = await restorePendingEnvFiles(); if (outcome === 'failed' || lost.length > 0) { diff --git a/src/workflow/cache.ts b/src/workflow/cache.ts index f593a3d1b..243ddca90 100644 --- a/src/workflow/cache.ts +++ b/src/workflow/cache.ts @@ -251,8 +251,11 @@ export async function syncWorkflowDefinitions( try { resourceNamespaces = await resolveResourceNamespaces(config); } catch { - // The push path reports unreadable manifests properly; here a null - // resolution just means every on-disk namespace is treated as active. + // A manifest exists but cannot be read: be conservative. Treating + // every on-disk namespace as active (the null fallback) would cache + // an inactive namespace's workflow as runnable for this member. + // An empty resolution means "no namespace is active". + resourceNamespaces = { activeNamespaces: {} } as Awaited>; } } const activeWorkflows = resourceNamespaces?.activeNamespaces.workflows ?? (resourceNamespaces ? [] : null); From 07af57dc96970f558a25211304ef15266eed0195 Mon Sep 17 00:00:00 2001 From: legend91019 <15853153143@163.com> Date: Thu, 22 Oct 2026 08:46:19 +0800 Subject: [PATCH 09/28] =?UTF-8?q?fix:=20address=20second=20re-review=20?= =?UTF-8?q?=E2=80=94=20reuse-path=20snapshot,=20env=20copy,=20manifest-fre?= =?UTF-8?q?e=20prompts,=20nested=20names,=20rule=20names?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1 fixes from re-review of d77e8f0: - push: reuseRecordedDestinations saves prePlacementPath before rewriting relativePath so the snapshot delete finds the root-path entry - env: pushItem copies file whenever source != destination (not just self mode), so env/env.yaml -> env//env.yaml actually writes the file - workflow/cache: manifest-free teams discover prompt namespaces from disk so prompts//deploy.md resolves for prompts/deploy.md - review-branch: parseRelativePath uses basename (not full sub-path) so nested names like 'api/guide' don't fail isSafeNamespaceSegment - rules: scanTeamForPull extracts name and namespace separately, so rules/ops/foo.md -> name 'foo', ns 'ops' instead of name 'ops/foo' P3: - workflow/cache: update stale comment about manifest-error fallback --- src/push.ts | 2 ++ src/resources/env.ts | 25 +++++++++++-------------- src/resources/rules.ts | 20 ++++++++++++++------ src/review-branch.ts | 6 +++--- src/workflow/cache.ts | 21 ++++++++++++++++++--- 5 files changed, 48 insertions(+), 26 deletions(-) diff --git a/src/push.ts b/src/push.ts index 286e7a7b6..1cce1a5c1 100644 --- a/src/push.ts +++ b/src/push.ts @@ -554,11 +554,13 @@ function reuseRecordedDestinations(groups: PushGroup[]): Set { if (item.type === 'skills') { // A skill's path is derived from its name, so the recorded namespace // replaces whatever a --role/--project override wrote above. + (item as ResourceItem & { prePlacementPath?: string }).prePlacementPath = item.relativePath; item.namespace = ns; item.relativePath = ns ? skillNamespacePath(ns, item.name) : `skills/${item.name}`; } else if (ns && isAtSharedRoot(item)) { // A rule or agent carries its own path from the scanner, and that path // is authoritative when it already names a namespace (#654). + (item as ResourceItem & { prePlacementPath?: string }).prePlacementPath = item.relativePath; item.namespace = ns; item.relativePath = withNamespace(item.relativePath, ns, rootSegmentsFor(item.type)); } diff --git a/src/resources/env.ts b/src/resources/env.ts index a8cf0e71f..755b2b8c7 100644 --- a/src/resources/env.ts +++ b/src/resources/env.ts @@ -400,21 +400,18 @@ export class EnvHandler extends ResourceHandler { } async pushItem(item: ResourceItem, _teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise { - // Non-self modes: env files already live in the repo dir; push.ts stages - // each selected one by its path — nothing to copy. + // The source path may differ from the destination when placement rewrote + // `relativePath` (e.g. env/env.yaml → env//env.yaml). Copy the bytes so + // the staged path actually exists in the clone before push.ts commits it. // - // Single-repo mode: the source is the ACTIVE tree's .teamai/env/ file, but - // the commit happens in the knowledge worktree (localConfig.repo.localPath). - // Copy the active copy into the worktree so the PR actually carries the change; - // otherwise staging that path would commit the stale baseline. (Guarded on the - // paths differing so non-self stays a no-op.) - if (isSelfMode(localConfig)) { - const dest = path.join(localConfig.repo.localPath, ...item.relativePath.split('/')); - if (item.sourcePath !== dest) { - await ensureDir(path.dirname(dest)); - const content = await readFileSafe(item.sourcePath); - if (content !== null) await writeFile(dest, content); - } + // Self mode: the source is the ACTIVE tree's .teamai/env/ file, but the + // commit happens in the knowledge worktree (localConfig.repo.localPath). + // Non-self: the source is the clone, but placement may have moved the path. + const dest = path.join(localConfig.repo.localPath, ...item.relativePath.split('/')); + if (item.sourcePath !== dest) { + await ensureDir(path.dirname(dest)); + const content = await readFileSafe(item.sourcePath); + if (content !== null) await writeFile(dest, content); } } diff --git a/src/resources/rules.ts b/src/resources/rules.ts index 242a785a3..9e9a53103 100644 --- a/src/resources/rules.ts +++ b/src/resources/rules.ts @@ -234,12 +234,20 @@ export class RulesHandler extends ResourceHandler { // teamai-context is the file pull writes the team instructions to; a team // rule of that name would land on it (#945). pullAllRules names it. .filter((f) => f.endsWith('.md') && f !== `${TEAMAI_CONTEXT_RULE_NAME}.md`) - .map((f) => ({ - name: f.replace(/\.md$/, ''), - type: 'rules' as const, - sourcePath: path.join(rulesDir, f), - relativePath: `rules/${f}`, - })); + .map((f) => { + // `rules//.md` is namespaced; `rules/.md` is shared. + // Extract name and namespace so review sees clean identities. + const segments = f.replace(/\.md$/, '').split('/'); + const namespace = segments.length > 1 ? segments[0] : undefined; + const name = segments.length > 1 ? segments.slice(1).join('/') : segments[0]; + return { + name, + type: 'rules' as const, + sourcePath: path.join(rulesDir, f), + relativePath: `rules/${f}`, + ...(namespace ? { namespace } : {}), + }; + }); } async pushItem(item: ResourceItem, teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise { diff --git a/src/review-branch.ts b/src/review-branch.ts index 821725bef..a37b4dcf5 100644 --- a/src/review-branch.ts +++ b/src/review-branch.ts @@ -103,9 +103,9 @@ function parseRelativePath( return { name: stripExtension(rest[0] ?? '') }; } const namespace = rest[0]; - // Keep the full sub-path as the name so nested docs/wiki files in different - // directories don't collide on their basename alone. - const name = stripExtension(rest.slice(1).join('/')); + // Use the basename as the name: a name must be a single safe path segment. + // Full sub-paths (e.g. 'api/guide') fail isSafeNamespaceSegment on the '/'. + const name = stripExtension(rest[rest.length - 1] ?? ''); if ((type === 'docs' || type === 'wiki') && !index.declares(NAMESPACE_AXIS[type], namespace!)) { return { name: stripExtension(rest.join('/')) }; } diff --git a/src/workflow/cache.ts b/src/workflow/cache.ts index 243ddca90..8d9c53520 100644 --- a/src/workflow/cache.ts +++ b/src/workflow/cache.ts @@ -228,8 +228,8 @@ export async function syncWorkflowDefinitions( // OR prompts. A root-only tree needs no manifest read, and an unconditional // read here would fire before the push's pull refreshes the clone — the // stale mapping the #649 review forbids. A manifest that cannot be read - // leaves the resolution null, which falls back to the set of namespaces - // found on disk. + // resolves to an empty namespace set (conservative), while a manifest-free + // team discovers prompt namespaces from disk below. let hasNamespaced = rawCandidates.some((c) => c.namespace !== null); if (!hasNamespaced) { // Prompt namespaces are independent of workflow layout: a root-only @@ -260,6 +260,21 @@ export async function syncWorkflowDefinitions( } const activeWorkflows = resourceNamespaces?.activeNamespaces.workflows ?? (resourceNamespaces ? [] : null); const activePrompts = resourceNamespaces?.activeNamespaces.prompts ?? (resourceNamespaces ? [] : null); + // Manifest-free: discover prompt namespaces from disk so a namespaced + // prompt directory is still found even without a roles/projects manifest. + let effectiveActivePrompts: readonly string[] | null = activePrompts; + if (effectiveActivePrompts === null) { + const discovered = new Set(); + try { + const promptsDir = path.join(knowledgeRoot, 'prompts'); + for (const entry of await fs.readdir(promptsDir, { withFileTypes: true })) { + if (entry.isDirectory() && !entry.isSymbolicLink() && isSafeNamespaceSegment(entry.name)) { + discovered.add(entry.name); + } + } + } catch { /* prompts dir missing */ } + effectiveActivePrompts = discovered.size > 0 ? [...discovered] : null; + } const effectiveActiveWorkflows = activeWorkflows ?? [ ...new Set(rawCandidates.flatMap((candidate) => ( candidate.namespace === null ? [] : [candidate.namespace] @@ -324,7 +339,7 @@ export async function syncWorkflowDefinitions( checked.push({ stem, relativePath, diagnostics: parsed.diagnostics }); continue; } - const resolved = await resolvePromptFiles(config, parsed.definition, parsed.promptFileLocations, relativePath, activePrompts); + const resolved = await resolvePromptFiles(config, parsed.definition, parsed.promptFileLocations, relativePath, effectiveActivePrompts); if (!resolved.ok) { checked.push({ stem, relativePath, diagnostics: resolved.diagnostics }); continue; From 9909453329f7a9f8aec66d02418a11ab2f6362c5 Mon Sep 17 00:00:00 2001 From: legend91019 <15853153143@163.com> Date: Thu, 22 Oct 2026 10:00:32 +0800 Subject: [PATCH 10/28] fix: revert scanTeamForPull change, fix rule names in review-branch instead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rules.ts scanTeamForPull change (extracting name/namespace from the path) broke pull and uninstall behavior — CI failures in uninstall.test.ts and workspace-children.test.ts showed extra rules/fe/*.md globs and rules/platform/ops.md delivery. Revert rules.ts to its original form and instead handle the name/namespace extraction in review-branch.ts scanResources, where rules items now go through parseRelativePath like other non-handler-authoritative types. --- src/resources/rules.ts | 20 ++++++-------------- src/review-branch.ts | 5 ++++- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/resources/rules.ts b/src/resources/rules.ts index 9e9a53103..242a785a3 100644 --- a/src/resources/rules.ts +++ b/src/resources/rules.ts @@ -234,20 +234,12 @@ export class RulesHandler extends ResourceHandler { // teamai-context is the file pull writes the team instructions to; a team // rule of that name would land on it (#945). pullAllRules names it. .filter((f) => f.endsWith('.md') && f !== `${TEAMAI_CONTEXT_RULE_NAME}.md`) - .map((f) => { - // `rules//.md` is namespaced; `rules/.md` is shared. - // Extract name and namespace so review sees clean identities. - const segments = f.replace(/\.md$/, '').split('/'); - const namespace = segments.length > 1 ? segments[0] : undefined; - const name = segments.length > 1 ? segments.slice(1).join('/') : segments[0]; - return { - name, - type: 'rules' as const, - sourcePath: path.join(rulesDir, f), - relativePath: `rules/${f}`, - ...(namespace ? { namespace } : {}), - }; - }); + .map((f) => ({ + name: f.replace(/\.md$/, ''), + type: 'rules' as const, + sourcePath: path.join(rulesDir, f), + relativePath: `rules/${f}`, + })); } async pushItem(item: ResourceItem, teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise { diff --git a/src/review-branch.ts b/src/review-branch.ts index a37b4dcf5..d666003d8 100644 --- a/src/review-branch.ts +++ b/src/review-branch.ts @@ -142,7 +142,10 @@ async function scanResources(repoDir: string, index: ManifestIndex): Promise/mcp.yaml), collapsing distinct servers into one identity. - if ((handlerTypes as readonly PlaceableType[]).includes(type)) { + // Rules are an exception: the handler's name carries the namespace prefix + // (e.g. 'ops/foo' for rules/ops/foo.md), so we parse name/namespace from + // the path instead. + if ((handlerTypes as readonly PlaceableType[]).includes(type) && type !== 'rules') { return { type, name: item.name, From 95f37a3fa0bfb0ceb0620a2e56e5a001affc0a5a Mon Sep 17 00:00:00 2001 From: legend91019 <15853153143@163.com> Date: Thu, 22 Oct 2026 22:41:01 +0800 Subject: [PATCH 11/28] fix(review): entry identities for hooks, wiki authoritative root, per-segment doc names - review-branch scans wiki at teamwiki/evidence/code (where the handler and pull read it) instead of wiki/, and READ_ROOTS materializes the same root, so wiki pages no longer bypass branch review - hooks are reviewed by their entry ids (pull's hooks reader identity), not by file name, so co-active hooks/a/hooks.yaml and hooks/b/hooks.yaml with disjoint ids are not duplicates; unreadable or entry-free files keep a file identity so naming and leak checks still see them - docs and wiki keep their bundle-relative sub-path as the resource name (below the namespace when declared, below the root otherwise), and namingFindings validates each path segment on its own, so shared nested docs like docs/api/guide.md pass review while co-active docs/a/api/guide.md and docs/b/ui/guide.md stay two resources - leakFindings checks a file once even when several resources share it - update the Chinese distribution guide to match the push behavior change --- docs/guide/zh-CN/distribution.md | 2 +- src/__tests__/review-branch.test.ts | 127 ++++++++++++++++++++++++++++ src/review-branch.ts | 85 ++++++++++++++++--- 3 files changed, 200 insertions(+), 14 deletions(-) diff --git a/docs/guide/zh-CN/distribution.md b/docs/guide/zh-CN/distribution.md index 511daa5eb..246fa40f8 100644 --- a/docs/guide/zh-CN/distribution.md +++ b/docs/guide/zh-CN/distribution.md @@ -101,7 +101,7 @@ sharing: .teamai/mcp/mcp.yaml ``` -skill 和 rule 用 `teamai push`。`docs/`、`hooks/hooks.yaml`、`mcp/mcp.yaml`、`models/models.yaml` 自己 commit。 +`teamai push` 发送所有分布式资源类型——skills、rules、agents、docs、env、hooks、mcp、models、claudemd、learnings、wiki、workflows、prompts、scorers——并按 `--project` 和 `--role` 做命名空间放置。 ### Git 权限 diff --git a/src/__tests__/review-branch.test.ts b/src/__tests__/review-branch.test.ts index 3480b818b..dffb3912e 100644 --- a/src/__tests__/review-branch.test.ts +++ b/src/__tests__/review-branch.test.ts @@ -38,6 +38,22 @@ function rolesYaml(namespaces: string[]): string { ].join('\n'); } +/** A roles manifest whose dev role declares `namespaces` on `axis`. */ +function resourcesYaml(axis: string, namespaces: string[]): string { + return [ + 'version: 1', + 'roles:', + ' - id: dev', + ' description: dev', + ' resources:', + ' knowledge: []', + ' skills: []', + ' agents: []', + ` ${axis}: [${namespaces.join(', ')}]`, + '', + ].join('\n'); +} + async function writeSkill(repoDir: string, relativeDir: string, body: string): Promise { const dir = path.join(repoDir, ...relativeDir.split('/')); await fse.ensureDir(dir); @@ -172,6 +188,117 @@ describe('reviewBranchDir — read-only health checks for a pushed branch (#776) expect(findings(review).some((f) => f.check === 'duplicates')).toBe(false); }); + it('passes a nested doc whose first directory is not a declared namespace', async () => { + await fse.ensureDir(path.join(repoDir, 'manifest')); + await fse.writeFile(path.join(repoDir, 'manifest', 'roles.yaml'), resourcesYaml('prompts', ['frontend'])); + await fse.ensureDir(path.join(repoDir, 'docs', 'api')); + await fse.writeFile(path.join(repoDir, 'docs', 'api', 'guide.md'), '# API guide\n'); + + const review = await reviewBranchDir(BRANCH, repoDir); + + expect(review.ok).toBe(true); + expect(findings(review)).toEqual([]); + }); + + it('keeps distinct nested doc sub-paths in co-active namespaces from conflicting', async () => { + await fse.ensureDir(path.join(repoDir, 'manifest')); + await fse.writeFile(path.join(repoDir, 'manifest', 'roles.yaml'), resourcesYaml('docs', ['a', 'b'])); + await fse.ensureDir(path.join(repoDir, 'docs', 'a', 'api')); + await fse.writeFile(path.join(repoDir, 'docs', 'a', 'api', 'guide.md'), '# API guide\n'); + await fse.ensureDir(path.join(repoDir, 'docs', 'b', 'ui')); + await fse.writeFile(path.join(repoDir, 'docs', 'b', 'ui', 'guide.md'), '# UI guide\n'); + + const review = await reviewBranchDir(BRANCH, repoDir); + + expect(review.ok).toBe(true); + expect(findings(review)).toEqual([]); + }); + + it('fails the same nested doc sub-path in two co-active namespaces', async () => { + await fse.ensureDir(path.join(repoDir, 'manifest')); + await fse.writeFile(path.join(repoDir, 'manifest', 'roles.yaml'), resourcesYaml('docs', ['a', 'b'])); + await fse.ensureDir(path.join(repoDir, 'docs', 'a', 'api')); + await fse.writeFile(path.join(repoDir, 'docs', 'a', 'api', 'guide.md'), '# API guide\n'); + await fse.ensureDir(path.join(repoDir, 'docs', 'b', 'api')); + await fse.writeFile(path.join(repoDir, 'docs', 'b', 'api', 'guide.md'), '# other guide\n'); + + const review = await reviewBranchDir(BRANCH, repoDir); + + expect(review.ok).toBe(false); + expect(findings(review).some((f) => ( + f.check === 'duplicates' && /"api\/guide" is defined in both/.test(f.message) + ))).toBe(true); + }); + + it('reviews hooks by their entry ids, not their file names', async () => { + await fse.ensureDir(path.join(repoDir, 'manifest')); + await fse.writeFile(path.join(repoDir, 'manifest', 'roles.yaml'), resourcesYaml('hooks', ['a', 'b'])); + for (const ns of ['a', 'b']) await fse.ensureDir(path.join(repoDir, 'hooks', ns)); + await fse.writeFile(path.join(repoDir, 'hooks', 'a', 'hooks.yaml'), [ + 'hooks:', + ' - id: deploy', + ' description: deploy helper', + ' event: PreToolUse', + ' command: ~/.teamai/team-scripts/deploy.sh', + '', + ].join('\n')); + await fse.writeFile(path.join(repoDir, 'hooks', 'b', 'hooks.yaml'), [ + 'hooks:', + ' - id: notify', + ' description: notify helper', + ' event: PostToolUse', + ' command: ~/.teamai/team-scripts/notify.sh', + '', + ].join('\n')); + + const review = await reviewBranchDir(BRANCH, repoDir); + + expect(review.ok).toBe(true); + expect(findings(review)).toEqual([]); + }); + + it('fails the same hook id in two co-active namespaces', async () => { + await fse.ensureDir(path.join(repoDir, 'manifest')); + await fse.writeFile(path.join(repoDir, 'manifest', 'roles.yaml'), resourcesYaml('hooks', ['a', 'b'])); + for (const ns of ['a', 'b']) await fse.ensureDir(path.join(repoDir, 'hooks', ns)); + const hook = [ + 'hooks:', + ' - id: deploy', + ' description: deploy helper', + ' event: PreToolUse', + ' command: ~/.teamai/team-scripts/deploy.sh', + '', + ].join('\n'); + await fse.writeFile(path.join(repoDir, 'hooks', 'a', 'hooks.yaml'), hook); + await fse.writeFile(path.join(repoDir, 'hooks', 'b', 'hooks.yaml'), hook); + + const review = await reviewBranchDir(BRANCH, repoDir); + + expect(review.ok).toBe(false); + expect(findings(review).some((f) => ( + f.check === 'duplicates' && /"deploy" is defined in both/.test(f.message) + ))).toBe(true); + }); + + it('reviews wiki pages at teamwiki/evidence/code, not the wiki/ name', async () => { + await fse.ensureDir(path.join(repoDir, 'manifest')); + await fse.writeFile(path.join(repoDir, 'manifest', 'roles.yaml'), resourcesYaml('wiki', ['a', 'b'])); + for (const ns of ['a', 'b']) await fse.ensureDir(path.join(repoDir, 'teamwiki', 'evidence', 'code', ns)); + await fse.writeFile(path.join(repoDir, 'teamwiki', 'evidence', 'code', 'a', 'page.md'), '# page a\n'); + await fse.writeFile(path.join(repoDir, 'teamwiki', 'evidence', 'code', 'b', 'page.md'), '# page b\n'); + // A stray file under the old wiki/ root must not be reviewed at all. + await fse.ensureDir(path.join(repoDir, 'wiki')); + await fse.writeFile(path.join(repoDir, 'wiki', 'stray.md'), 'password: hunter2\n'); + + const review = await reviewBranchDir(BRANCH, repoDir); + + expect(review.ok).toBe(false); + expect(findings(review).some((f) => ( + f.check === 'duplicates' && /"page" is defined in both/.test(f.message) + ))).toBe(true); + expect(findings(review).some((f) => f.path?.startsWith('wiki/'))).toBe(false); + }); + it('fails when a skill frontmatter name does not match its directory', async () => { await fse.ensureDir(path.join(repoDir, 'manifest')); await fse.writeFile(path.join(repoDir, 'manifest', 'roles.yaml'), rolesYaml(['frontend'])); diff --git a/src/review-branch.ts b/src/review-branch.ts index d666003d8..0a9da07f9 100644 --- a/src/review-branch.ts +++ b/src/review-branch.ts @@ -29,6 +29,7 @@ import { caseFoldKey, isSafeNamespaceSegment, NAMESPACE_RULE } from './manifest- import { ALL_PROJECT_RESOURCE_TYPES, loadProjectsManifest, type AllProjectResourceType, type ProjectsManifest } from './projects.js'; import { NAMESPACE_AXIS, PLACEABLE_TYPES, rootSegmentsFor, type PlaceableType } from './push-namespaces.js'; import { parseAgentYaml } from './resources/agent-format.js'; +import { hooksEntryReader } from './resources/hooks.js'; import { getHandler } from './resources/index.js'; import { listRepoFilesRecursive } from './resources/repo-resource.js'; import { loadRolesManifestIfPresent, type RolesManifest } from './roles.js'; @@ -103,13 +104,20 @@ function parseRelativePath( return { name: stripExtension(rest[0] ?? '') }; } const namespace = rest[0]; - // Use the basename as the name: a name must be a single safe path segment. - // Full sub-paths (e.g. 'api/guide') fail isSafeNamespaceSegment on the '/'. - const name = stripExtension(rest[rest.length - 1] ?? ''); - if ((type === 'docs' || type === 'wiki') && !index.declares(NAMESPACE_AXIS[type], namespace!)) { + // docs and wiki nest freely below their root and their namespaces, so their + // identity is the whole bundle-relative sub-path, never the bare basename: + // co-active docs/a/api/guide.md and docs/b/ui/guide.md are two resources, not + // a duplicate "guide". The name may contain '/', which namingFindings + // validates one segment at a time. + if (type === 'docs' || type === 'wiki') { + if (index.declares(NAMESPACE_AXIS[type], namespace!)) { + return { name: stripExtension(rest.slice(1).join('/')), namespace }; + } return { name: stripExtension(rest.join('/')) }; } - return { name, namespace }; + // A first segment under the other roots is a namespace by construction + // (`skills//`), so the name is the basename below it. + return { name: stripExtension(rest[rest.length - 1] ?? ''), namespace }; } /** Enumerate the resources on the branch through the same scanners pull uses. */ @@ -124,16 +132,43 @@ async function scanResources(repoDir: string, index: ManifestIndex): Promise 1 ? segments[0] : undefined; + const fileEntry = (name: string) => ({ + type, + item: { type, name, namespace, sourcePath: absolutePath, relativePath }, + }); + if (path.posix.basename(file) === 'hooks.yaml') { + const read = await hooksEntryReader.read(absolutePath, relativePath); + if (read !== null && read.ok && read.entries.length > 0) { + for (const hook of read.entries) { + items.push(fileEntry(hook.id)); + } + continue; + } + } + // Unreadable, entry-free, or non-canonical files keep a file identity, + // so the naming and leak checks still see them. + items.push(fileEntry(stripExtension(segments[segments.length - 1] ?? ''))); + continue; + } items.push({ type, item: { type, name: file, sourcePath: path.join(root, ...file.split('/')), - relativePath: `${type}/${file}`, + relativePath: `${rootRel}/${file}`, }, }); } @@ -144,8 +179,9 @@ async function scanResources(repoDir: string, index: ManifestIndex): Promise/mcp.yaml), collapsing distinct servers into one identity. // Rules are an exception: the handler's name carries the namespace prefix // (e.g. 'ops/foo' for rules/ops/foo.md), so we parse name/namespace from - // the path instead. - if ((handlerTypes as readonly PlaceableType[]).includes(type) && type !== 'rules') { + // the path instead. Hook entry ids are authoritative too — they are what + // pull delivers by (the scan above already set them with the namespace). + if (((handlerTypes as readonly PlaceableType[]).includes(type) && type !== 'rules') || type === 'hooks') { return { type, name: item.name, @@ -159,6 +195,15 @@ async function scanResources(repoDir: string, index: ManifestIndex): Promise !isSafeNamespaceSegment(segment)); + if (badSegment !== undefined) { findings.push({ level: 'fail', check: 'naming', path: resource.relativePath, - message: `Resource name "${resource.name}" in ${resource.relativePath} is not a usable path segment: ${NAMESPACE_RULE}.`, + message: `Resource name "${resource.name}" in ${resource.relativePath} has a segment ("${badSegment}") that is not usable: ${NAMESPACE_RULE}.`, }); } const declaredName = await frontmatterName(resource, readText); @@ -423,6 +472,9 @@ const MAX_SCANNED_FILE_BYTES = 512 * 1024; async function leakFindings(resources: ReviewResource[], readText: (p: string) => Promise): Promise { const findings: Finding[] = []; + // Several resources can share one file (every hook entry in a hooks.yaml is + // its own resource); its contents are checked once, not once per entry. + const seenFiles = new Set(); for (const resource of resources) { const files = resource.type === 'skills' ? (await listFilesRecursive(resource.absolutePath)).map((rel) => ({ @@ -432,6 +484,8 @@ async function leakFindings(resources: ReviewResource[], readText: (p: string) = : [{ relativePath: resource.relativePath, absolutePath: resource.absolutePath }]; for (const file of files) { + if (seenFiles.has(file.absolutePath)) continue; + seenFiles.add(file.absolutePath); const content = await readText(file.absolutePath); if (content === null || content.length > MAX_SCANNED_FILE_BYTES || content.includes('\0')) continue; @@ -489,8 +543,13 @@ export async function reviewBranchDir(branch: string, repoDir: string): Promise< // ─── Read-only branch reader ───────────────────────────────────────────────── -/** The repo roots read out of the branch; everything else is irrelevant here. */ -const READ_ROOTS = [...PLACEABLE_TYPES, 'manifest']; +/** + * The repo roots read out of the branch; everything else is irrelevant here. + * Wiki is read from its authoritative root `teamwiki/evidence/code`, matching + * where the resource handler and pull actually read it — reading `wiki/` would + * let everything under the real root bypass review. + */ +const READ_ROOTS = [...PLACEABLE_TYPES.map(repoRootFor), 'manifest']; /** * The ref to read `branch` from: the name itself when it resolves locally, else From db6b03bb99740aea7e39e500d1d5e08209577be2 Mon Sep 17 00:00:00 2001 From: legend91019 <15853153143@163.com> Date: Thu, 22 Oct 2026 23:11:10 +0800 Subject: [PATCH 12/28] fix(review): nested prompts are shared paths, docs identities keep extensions - prompts join docs and wiki as nested types: a first directory is a namespace only when a manifest declares it, so a workflow's prompt_file: prompts/topic/note.md stays a shared prompt instead of failing review as an undeclared namespace 'topic' - docs/wiki/prompt identities keep the file extension, so docs/a/api/guide.md and docs/b/api/guide.png are distinct resources and no longer report a false duplicate - move the undeclared-namespace assertion onto workflows, which still treats its first directory as a namespace --- src/__tests__/review-branch.test.ts | 51 +++++++++++++++++++---------- src/review-branch.ts | 33 ++++++++++++++----- 2 files changed, 58 insertions(+), 26 deletions(-) diff --git a/src/__tests__/review-branch.test.ts b/src/__tests__/review-branch.test.ts index e1b3a7e13..191a08131 100644 --- a/src/__tests__/review-branch.test.ts +++ b/src/__tests__/review-branch.test.ts @@ -190,27 +190,16 @@ describe('reviewBranchDir — read-only health checks for a pushed branch (#776) it('reviews newly placeable resource types for undeclared namespaces', async () => { await fse.ensureDir(path.join(repoDir, 'manifest')); - await fse.writeFile(path.join(repoDir, 'manifest', 'roles.yaml'), [ - 'version: 1', - 'roles:', - ' - id: dev', - ' description: dev', - ' resources:', - ' knowledge: []', - ' skills: []', - ' agents: []', - ' prompts: [frontend]', - '', - ].join('\n')); - await fse.ensureDir(path.join(repoDir, 'prompts', 'backend')); - await fse.writeFile(path.join(repoDir, 'prompts', 'backend', 'review.md'), 'Review this change.\n'); + await fse.writeFile(path.join(repoDir, 'manifest', 'roles.yaml'), resourcesYaml('workflows', ['frontend'])); + await fse.ensureDir(path.join(repoDir, 'workflows', 'backend')); + await fse.writeFile(path.join(repoDir, 'workflows', 'backend', 'deploy.yaml'), 'name: deploy\n'); const review = await reviewBranchDir(BRANCH, repoDir); expect(review.ok).toBe(false); expect(findings(review).some((f) => ( f.check === 'structure' - && f.path === 'prompts/backend/review.md' + && f.path === 'workflows/backend/deploy.yaml' && /namespace "backend"/.test(f.message) ))).toBe(true); }); @@ -294,10 +283,38 @@ describe('reviewBranchDir — read-only health checks for a pushed branch (#776) expect(review.ok).toBe(false); expect(findings(review).some((f) => ( - f.check === 'duplicates' && /"api\/guide" is defined in both/.test(f.message) + f.check === 'duplicates' && /"api\/guide\.md" is defined in both/.test(f.message) ))).toBe(true); }); + it('passes a nested prompt whose first directory is not a declared namespace', async () => { + await fse.ensureDir(path.join(repoDir, 'manifest')); + await fse.writeFile(path.join(repoDir, 'manifest', 'roles.yaml'), resourcesYaml('prompts', ['ops'])); + await fse.ensureDir(path.join(repoDir, 'prompts', 'topic')); + await fse.writeFile(path.join(repoDir, 'prompts', 'topic', 'note.md'), 'Review the change.\n'); + + const review = await reviewBranchDir(BRANCH, repoDir); + + // Delivered to everyone as a shared nested prompt, so it is context, not a failure. + expect(review.ok).toBe(true); + expect(review.counts.fail).toBe(0); + expect(findings(review).some((f) => f.check === 'naming' && /"topic\/note\.md"/.test(f.message))).toBe(false); + }); + + it('does not flag nested docs that differ only by extension', async () => { + await fse.ensureDir(path.join(repoDir, 'manifest')); + await fse.writeFile(path.join(repoDir, 'manifest', 'roles.yaml'), resourcesYaml('docs', ['a', 'b'])); + await fse.ensureDir(path.join(repoDir, 'docs', 'a', 'api')); + await fse.writeFile(path.join(repoDir, 'docs', 'a', 'api', 'guide.md'), '# API guide\n'); + await fse.ensureDir(path.join(repoDir, 'docs', 'b', 'api')); + await fse.writeFile(path.join(repoDir, 'docs', 'b', 'api', 'guide.png'), 'PNG placeholder\n'); + + const review = await reviewBranchDir(BRANCH, repoDir); + + expect(review.ok).toBe(true); + expect(findings(review).some((f) => f.check === 'duplicates')).toBe(false); + }); + it('reviews hooks by their entry ids, not their file names', async () => { await fse.ensureDir(path.join(repoDir, 'manifest')); await fse.writeFile(path.join(repoDir, 'manifest', 'roles.yaml'), resourcesYaml('hooks', ['a', 'b'])); @@ -362,7 +379,7 @@ describe('reviewBranchDir — read-only health checks for a pushed branch (#776) expect(review.ok).toBe(false); expect(findings(review).some((f) => ( - f.check === 'duplicates' && /"page" is defined in both/.test(f.message) + f.check === 'duplicates' && /"page\.md" is defined in both/.test(f.message) ))).toBe(true); expect(findings(review).some((f) => f.path?.startsWith('wiki/'))).toBe(false); }); diff --git a/src/review-branch.ts b/src/review-branch.ts index 643c1524d..25101274c 100644 --- a/src/review-branch.ts +++ b/src/review-branch.ts @@ -91,6 +91,20 @@ function stripExtension(segment: string): string { return segment.replace(/\.(md|markdown|yaml|yml|toml|json)$/i, ''); } +/** + * Types whose subtree nests freely below the root. For these a first directory + * is a namespace only when a manifest declares it, and the whole sub-path — + * extension included — is the resource identity. + * + * - docs and wiki are bundles of pages: `docs/api/guide.md` is a shared page + * under an undeclared `api` directory, not a page in namespace `api`. + * - prompts resolve the same way at runtime: a workflow's `prompt_file: + * prompts/topic/note.md` looks for `prompts//topic/note.md` in each + * active namespace and falls back to the shared `prompts/topic/note.md`, so + * a nested shared prompt is valid. + */ +const NESTED_TYPES: ReadonlySet = new Set(['docs', 'wiki', 'prompts']); + /** * Name and namespace from a repo-relative resource path, the same way pull reads * them (`//` is namespaced, `/` is shared). The @@ -105,19 +119,20 @@ function parseRelativePath( const all = relativePath.split('/'); const rest = all.slice(rootSegs); if (rest.length <= 1) { - return { name: stripExtension(rest[0] ?? '') }; + const only = rest[0] ?? ''; + return { name: NESTED_TYPES.has(type) ? only : stripExtension(only) }; } const namespace = rest[0]; - // docs and wiki nest freely below their root and their namespaces, so their - // identity is the whole bundle-relative sub-path, never the bare basename: - // co-active docs/a/api/guide.md and docs/b/ui/guide.md are two resources, not - // a duplicate "guide". The name may contain '/', which namingFindings - // validates one segment at a time. - if (type === 'docs' || type === 'wiki') { + // Nested types keep the whole bundle-relative sub-path as the identity, never + // the bare basename: co-active docs/a/api/guide.md and docs/b/ui/guide.md are + // two resources, not a duplicate "guide", and the extension stays because + // docs/a/api/guide.md and docs/b/api/guide.png are distinct files. The name + // may contain '/', which namingFindings validates one segment at a time. + if (NESTED_TYPES.has(type)) { if (index.declares(NAMESPACE_AXIS[type], namespace!)) { - return { name: stripExtension(rest.slice(1).join('/')), namespace }; + return { name: rest.slice(1).join('/'), namespace }; } - return { name: stripExtension(rest.join('/')) }; + return { name: rest.join('/') }; } // A first segment under the other roots is a namespace by construction // (`skills//`), so the name is the basename below it. From 3a61393cbfcfc5a39ee720d6a3d3e9c2750e9ec4 Mon Sep 17 00:00:00 2001 From: legend91019 <15853153143@163.com> Date: Fri, 23 Oct 2026 00:07:39 +0800 Subject: [PATCH 13/28] fix(push): place nested prompts, and remember where a repo-backed placement landed - prompts join docs and wiki as nested types on the push side too: a first directory is a namespace only when a role or project declares it, so a new prompts/topic/note.md is shared content (isAtSharedRoot) that a placement still moves to prompts//topic/note.md, the path a workflow resolves at runtime. resolveResourceNamespaces now reports the declared-but-inactive prompts namespaces that decision needs - remember where push placed a repo-backed resource (docs, wiki, prompts, workflows, scorers, models, env, hooks, claudemd, learnings) in state.placements, keyed by the path the author still edits, and record it only once the placement's blob is on the default branch (the same proof placedRules uses, so a PR closed unmerged leaves nothing behind) - a self-mode scan now compares that author copy against the destination instead of reading it as brand new: unchanged means already delivered, an edit is reported as that destination's modification, so the next push --all --role no longer collides with the file it placed --- src/__tests__/placement-records.test.ts | 23 ++++++ .../push-all-resources-namespaces.test.ts | 67 ++++++++++++++++- src/push-namespaces.ts | 29 ++++++-- src/push.ts | 24 ++++--- src/resource-namespaces.ts | 14 +++- src/resources/repo-resource.ts | 37 +++++++++- src/review-branch.ts | 22 ++---- src/types.ts | 21 ++++++ src/utils/pending-push.ts | 71 ++++++++++++++++--- 9 files changed, 262 insertions(+), 46 deletions(-) diff --git a/src/__tests__/placement-records.test.ts b/src/__tests__/placement-records.test.ts index 1b83a93c4..0a587725c 100644 --- a/src/__tests__/placement-records.test.ts +++ b/src/__tests__/placement-records.test.ts @@ -60,6 +60,29 @@ describe('reconcilePlacementRecords', () => { expect(state.placedRules).toEqual({ 'my-rule': 'rules/fe/my-rule.md' }); }); + it('records a repo-backed placement by the path the author still edits', async () => { + // docs, wiki, prompts and the other repo-backed types have no placedRules / + // placedAgents field: the record is keyed by the shared-root path the author + // keeps editing, so a self-mode scan can compare it with the destination. + const blob = await land('docs/ops/guide.md', 'x'); + const state = { + placedRules: {}, placedAgents: {}, placements: {} as Record, + pendingPushes: [pending([{ + type: 'docs', + name: 'guide.md', + relativePath: 'docs/ops/guide.md', + namespace: 'ops', + sourcePath: 'docs/guide.md', + placed: true, + blob, + }])], + }; + + expect(await reconcilePlacementRecords(repoPath, state)).toBe(true); + expect(state.placements).toEqual({ 'docs:docs/guide.md': 'docs/ops/guide.md' }); + expect(state.pendingPushes[0]?.items[0]?.placed).toBe(false); + }); + it('spends a placement with no blob instead of recording it because the path exists', async () => { // Nothing tells its landing from another member creating the path after // this PR closed unmerged (#649 review). diff --git a/src/__tests__/push-all-resources-namespaces.test.ts b/src/__tests__/push-all-resources-namespaces.test.ts index 1bf1defbd..a1eb723ab 100644 --- a/src/__tests__/push-all-resources-namespaces.test.ts +++ b/src/__tests__/push-all-resources-namespaces.test.ts @@ -16,6 +16,7 @@ import type { ProjectsManifest } from '../projects.js'; import type { LocalConfig, ResourceItem } from '../types.js'; import { scanRepoResourceForPush, pushRepoResourceItem } from '../resources/repo-resource.js'; import { getHandler } from '../resources/index.js'; +import { loadStateForScope, saveStateForScope } from '../config.js'; const dirs: string[] = []; @@ -81,7 +82,11 @@ describe('isAtSharedRoot and withNamespace across all resource types', () => { { type: 'workflows', path: 'workflows/ci.yaml', shared: true }, { type: 'workflows', path: 'workflows/team-a/ci.yaml', shared: false }, { type: 'prompts', path: 'prompts/review.md', shared: true }, - { type: 'prompts', path: 'prompts/team-a/review.md', shared: false }, + // An undeclared prompts/ directory is shared content, like docs/ and + // wiki/: a workflow resolves a nested prompt below the root when no + // namespace holds it. + { type: 'prompts', path: 'prompts/team-a/review.md', shared: true }, + { type: 'prompts', path: 'prompts/topic/note.md', shared: true }, { type: 'scorers', path: 'scorers/accuracy.py', shared: true }, { type: 'scorers', path: 'scorers/team-a/accuracy.py', shared: false }, { type: 'models', path: 'models/config.json', shared: true }, @@ -122,6 +127,24 @@ describe('isAtSharedRoot and withNamespace across all resource types', () => { }; expect(declaredNamespaceFor(nestedDoc, ['team-a'])).toBe('team-a'); expect(declaredNamespaceFor(nestedDoc, ['other-team'])).toBeUndefined(); + + // prompts follow the same rule: only a declared first segment is a namespace. + const nestedPrompt: ResourceItem = { + name: 'topic/note.md', + type: 'prompts', + sourcePath: '/local/prompts/topic/note.md', + relativePath: 'prompts/topic/note.md', + }; + expect(isAtSharedRoot(nestedPrompt)).toBe(true); + expect(declaredNamespaceFor(nestedPrompt, ['ops'])).toBeUndefined(); + + const namespacedPrompt: ResourceItem = { + name: 'note.md', + type: 'prompts', + sourcePath: '/local/prompts/ops/note.md', + relativePath: 'prompts/ops/note.md', + }; + expect(declaredNamespaceFor(namespacedPrompt, ['ops'])).toBe('ops'); }); it('correctly inserts namespace segments for various resource formats', () => { @@ -304,4 +327,46 @@ describe('repo resource handler push and scan', () => { expect(fs.existsSync(destFile)).toBe(true); expect(fs.readFileSync(destFile, 'utf8')).toBe('name: test\n'); }); + + it('compares a root-authored file against the destination an earlier push placed it at', async () => { + const root = makeTmpDir(); + const projectRoot = path.join(root, 'project'); + const repoLocal = path.join(root, 'team-repo'); + const dataHome = path.join(root, 'data'); + const localConfig: LocalConfig = { + repo: { localPath: repoLocal, remote: 'local', kind: 'self' }, + username: 'alice', + scope: 'project', + projectRoot, + dataHome, + additionalRoles: [], + }; + + // The author still edits .teamai/docs/guide.md; the team file it became is + // docs/ops/guide.md, and state.placements remembers the two are one file. + const activeFile = path.join(projectRoot, '.teamai', 'docs', 'guide.md'); + fs.mkdirSync(path.dirname(activeFile), { recursive: true }); + fs.writeFileSync(activeFile, 'v1\n'); + const destFile = path.join(repoLocal, 'docs', 'ops', 'guide.md'); + fs.mkdirSync(path.dirname(destFile), { recursive: true }); + fs.writeFileSync(destFile, 'v1\n'); + const state = await loadStateForScope(localConfig); + state.placements = { 'docs:docs/guide.md': 'docs/ops/guide.md' }; + await saveStateForScope(state, localConfig); + + // Unchanged: already delivered, so it is not a new resource again. + expect(await scanRepoResourceForPush('docs', 'docs', {} as never, localConfig)).toEqual([]); + + // Edited: the edit belongs on the placed file, not on a second copy that + // would collide with it, so the scan reports it as that path's modification. + fs.writeFileSync(activeFile, 'v2\n'); + const edited = await scanRepoResourceForPush('docs', 'docs', {} as never, localConfig); + expect(edited).toHaveLength(1); + expect(edited[0]).toMatchObject({ + name: 'guide.md', + relativePath: 'docs/ops/guide.md', + namespace: 'ops', + status: 'modified', + }); + }); }); diff --git a/src/push-namespaces.ts b/src/push-namespaces.ts index 06f77d10a..4d806da9a 100644 --- a/src/push-namespaces.ts +++ b/src/push-namespaces.ts @@ -80,6 +80,24 @@ export const NAMESPACE_AXIS = { scorers: 'scorers', } as const satisfies Record; +/** + * Types whose subtree nests freely below the root. For these a first directory + * is a namespace only when a role or project declares it, and anything else is + * ordinary shared content that a placement may still relocate: + * + * - docs and wiki are bundles of pages, so `docs/api/guide.md` is a shared page + * under an undeclared `api`, not a page in namespace `api` (#1101). + * - prompts resolve the same way at runtime: a workflow's `prompt_file: + * prompts/topic/note.md` looks for `prompts//topic/note.md` in each active + * namespace and falls back to the shared `prompts/topic/note.md`. + */ +export const NESTED_PLACEABLE_TYPES: ReadonlySet = new Set(['docs', 'wiki', 'prompts']); + +/** True for the types in `NESTED_PLACEABLE_TYPES`. */ +export function isNestedPlaceableType(type: PlaceableType): boolean { + return NESTED_PLACEABLE_TYPES.has(type); +} + /** * True when `item` would be written to the shared root — no namespace segment * between the resource root and the resource itself. Those are the only items @@ -89,18 +107,19 @@ export const NAMESPACE_AXIS = { */ export function isAtSharedRoot(item: ResourceItem): boolean { if (item.namespace) return false; - // A first directory under docs/ or wiki/ is only a namespace when a manifest - // declares it. Undeclared directories are ordinary shared content (#1101). - if (item.type === 'docs' || item.type === 'wiki') return true; + // A first directory under docs/, wiki/ or prompts/ is only a namespace when a + // manifest declares it. Undeclared directories are ordinary shared content + // (#1101), which a placement still moves into the chosen namespace. + if (isNestedPlaceableType(item.type)) return true; return item.relativePath.split('/').length === 2; } -/** Return the declared namespace carried by a docs/wiki path, if any. */ +/** Return the declared namespace carried by a docs/wiki/prompts path, if any. */ export function declaredNamespaceFor( item: ResourceItem, declaredNamespaces: readonly string[], ): string | undefined { - if (item.type !== 'docs' && item.type !== 'wiki') return undefined; + if (!isNestedPlaceableType(item.type)) return undefined; const segments = item.relativePath.split('/'); const candidate = segments[rootSegmentsFor(item.type)]; return candidate && declaredNamespaces.includes(candidate) ? candidate : undefined; diff --git a/src/push.ts b/src/push.ts index 1cce1a5c1..a156d2528 100644 --- a/src/push.ts +++ b/src/push.ts @@ -29,7 +29,7 @@ import { loadRolesManifest, resolveRoleResourceNamespaces, RolesManifestNotFound import { loadProjectsManifest, resolveProjectResourceNamespaces, type ProjectsManifest } from './projects.js'; import { isSafeNamespaceSegment, NAMESPACE_RULE, fallbackNamespaceError } from './manifest-schema.js'; import { - declaredNamespaceFor, isAtSharedRoot, isPlaceableType, NAMESPACE_AXIS, PLACEABLE_TYPES, placedResourcePath, + declaredNamespaceFor, isAtSharedRoot, isNestedPlaceableType, isPlaceableType, NAMESPACE_AXIS, PLACEABLE_TYPES, placedResourcePath, resolveProjectNamespace, rootSegmentsFor, skillNamespacePath, withNamespace, type PlaceableType, } from './push-namespaces.js'; import { askQuestion, askSelection, isInteractive } from './utils/prompt.js'; @@ -1473,12 +1473,12 @@ async function pushCore( } } - // docs/ and wiki/ allow ordinary nested shared content. Only a first path - // segment declared by a role or project is a namespace, so annotate those - // items before the placement filter decides whether they are already scoped. - // A manifest that exists but cannot be read must stop the push: every - // placement below would fall through to the shared root, the widening #649 - // is about — and nobody asked for it. + // docs/, wiki/ and prompts/ allow ordinary nested shared content. Only a + // first path segment declared by a role or project is a namespace, so + // annotate those items before the placement filter decides whether they are + // already scoped. A manifest that exists but cannot be read must stop the + // push: every placement below would fall through to the shared root, the + // widening #649 is about — and nobody asked for it. let resolvedNamespaces: Awaited> = null; try { resolvedNamespaces = await resolveResourceNamespaces(localConfig); @@ -1488,7 +1488,7 @@ async function pushCore( return; } if (resolvedNamespaces) { - const declaredByType = { + const declaredByType: Partial> = { docs: [ ...(resolvedNamespaces.activeNamespaces.docs ?? []), ...resolvedNamespaces.inactiveDocsNamespaces, @@ -1497,10 +1497,14 @@ async function pushCore( ...(resolvedNamespaces.activeNamespaces.wiki ?? []), ...resolvedNamespaces.inactiveWikiNamespaces, ], + prompts: [ + ...(resolvedNamespaces.activeNamespaces.prompts ?? []), + ...resolvedNamespaces.inactivePromptsNamespaces, + ], }; for (const item of fullScan) { - if (item.type !== 'docs' && item.type !== 'wiki') continue; - item.namespace ??= declaredNamespaceFor(item, declaredByType[item.type]); + if (!isPlaceableType(item.type) || !isNestedPlaceableType(item.type)) continue; + item.namespace ??= declaredNamespaceFor(item, declaredByType[item.type] ?? []); } } diff --git a/src/resource-namespaces.ts b/src/resource-namespaces.ts index 0fb4263f2..63963ed64 100644 --- a/src/resource-namespaces.ts +++ b/src/resource-namespaces.ts @@ -40,6 +40,7 @@ export async function resolveResourceNamespaces(localConfig: LocalConfig): Promi allSkillNamespaces: first.allSkillNamespaces, inactiveDocsNamespaces: inAll(resolved.map((one) => one.inactiveDocsNamespaces)), inactiveWikiNamespaces: inAll(resolved.map((one) => one.inactiveWikiNamespaces)), + inactivePromptsNamespaces: inAll(resolved.map((one) => one.inactivePromptsNamespaces)), }; } @@ -166,7 +167,18 @@ async function resolveOwnResourceNamespaces(localConfig: LocalConfig) { const activeWiki = new Set(activeNamespaces.wiki ?? []); const inactiveWikiNamespaces = [...declaredWikiNamespaces].filter((namespace) => !activeWiki.has(namespace)); - return { activeNamespaces, allSkillNamespaces, inactiveDocsNamespaces, inactiveWikiNamespaces }; + // Prompts follow the same declared-vs-active rule as docs and wiki: a + // prompts// anyone lists under `resources.prompts` is a namespace, and a + // workflow resolves a nested prompt below it; every other directory is shared + // content, which a placement may still move into a namespace. + const declaredPromptsNamespaces = new Set([ + ...(rolesManifest?.roles ?? []).flatMap((role) => role.resources.prompts ?? []), + ...(projectsManifest?.projects ?? []).flatMap((project) => project.resources.prompts ?? []), + ]); + const activePrompts = new Set(activeNamespaces.prompts ?? []); + const inactivePromptsNamespaces = [...declaredPromptsNamespaces].filter((namespace) => !activePrompts.has(namespace)); + + return { activeNamespaces, allSkillNamespaces, inactiveDocsNamespaces, inactiveWikiNamespaces, inactivePromptsNamespaces }; } /** diff --git a/src/resources/repo-resource.ts b/src/resources/repo-resource.ts index cd00cb5ea..b06e6f566 100644 --- a/src/resources/repo-resource.ts +++ b/src/resources/repo-resource.ts @@ -1,9 +1,16 @@ import path from 'node:path'; import fs from 'node:fs/promises'; import { ResourceHandler } from './base.js'; +import { loadStateForScope } from '../config.js'; import { isSelfMode, type LocalConfig, type ResourceItem, type ResourceType, type TeamaiConfig } from '../types.js'; +import { rootSegmentsFor, type PlaceableType } from '../push-namespaces.js'; import { ensureDir, fileContentEqual, pathExists } from '../utils/fs.js'; +/** The namespace a placed path carries, from its position below the root. */ +function namespaceOfPlacedPath(type: PlaceableType, placedPath: string): string | undefined { + return placedPath.split('/')[rootSegmentsFor(type)]; +} + export async function listRepoFilesRecursive(dir: string, baseDir = dir): Promise { try { const entries = await fs.readdir(dir, { withFileTypes: true }); @@ -37,20 +44,44 @@ export async function scanRepoResourceForPush( if (!await pathExists(activeRoot)) return []; const items: ResourceItem[] = []; + // Where an earlier push placed a resource this machine still edits at its + // shared-root path (`state.placements`, recorded once the PR landed). The + // author's copy never moves, so without the record every later scan reads + // it as brand new and collides with the placed copy it cannot see. + const placements = (await loadStateForScope(localConfig)).placements ?? {}; const files = await listRepoFilesRecursive(activeRoot); for (const rel of files) { if (fileFilter && !fileFilter(rel)) continue; const activeFile = path.join(activeRoot, ...rel.split('/')); const baseFile = path.join(baselineRoot, ...rel.split('/')); const relativePath = `${dir}/${rel}`; - if (await pathExists(baseFile) && await fileContentEqual(activeFile, baseFile)) continue; - const status = await pathExists(baseFile) ? 'modified' : 'new'; + const atBaseline = await pathExists(baseFile); + if (atBaseline && await fileContentEqual(activeFile, baseFile)) continue; + if (!atBaseline) { + const placedPath = placements[`${type}:${relativePath}`]; + const placedFile = placedPath ? path.join(localConfig.repo.localPath, ...placedPath.split('/')) : null; + if (placedPath && placedFile && await pathExists(placedFile)) { + // Compare the copy the author edits against the file that landed in + // its place: unchanged means already delivered, and an edit belongs + // on that file rather than on a second copy at the shared root. + if (await fileContentEqual(activeFile, placedFile)) continue; + items.push({ + name: rel, + type, + sourcePath: activeFile, + relativePath: placedPath, + namespace: namespaceOfPlacedPath(type as PlaceableType, placedPath), + status: 'modified', + }); + continue; + } + } items.push({ name: rel, type, sourcePath: activeFile, relativePath, - status, + status: atBaseline ? 'modified' : 'new', }); } return items; diff --git a/src/review-branch.ts b/src/review-branch.ts index 25101274c..c96a4089e 100644 --- a/src/review-branch.ts +++ b/src/review-branch.ts @@ -29,7 +29,9 @@ import fse from 'fs-extra'; import { resolveConfigForDir } from './config.js'; import { caseFoldKey, isSafeNamespaceSegment, NAMESPACE_RULE } from './manifest-schema.js'; import { ALL_PROJECT_RESOURCE_TYPES, loadProjectsManifest, type AllProjectResourceType, type ProjectsManifest } from './projects.js'; -import { NAMESPACE_AXIS, PLACEABLE_TYPES, rootSegmentsFor, type PlaceableType } from './push-namespaces.js'; +import { + isNestedPlaceableType, NAMESPACE_AXIS, PLACEABLE_TYPES, rootSegmentsFor, type PlaceableType, +} from './push-namespaces.js'; import { parseAgentYaml } from './resources/agent-format.js'; import { hooksEntryReader } from './resources/hooks.js'; import { getHandler } from './resources/index.js'; @@ -91,20 +93,6 @@ function stripExtension(segment: string): string { return segment.replace(/\.(md|markdown|yaml|yml|toml|json)$/i, ''); } -/** - * Types whose subtree nests freely below the root. For these a first directory - * is a namespace only when a manifest declares it, and the whole sub-path — - * extension included — is the resource identity. - * - * - docs and wiki are bundles of pages: `docs/api/guide.md` is a shared page - * under an undeclared `api` directory, not a page in namespace `api`. - * - prompts resolve the same way at runtime: a workflow's `prompt_file: - * prompts/topic/note.md` looks for `prompts//topic/note.md` in each - * active namespace and falls back to the shared `prompts/topic/note.md`, so - * a nested shared prompt is valid. - */ -const NESTED_TYPES: ReadonlySet = new Set(['docs', 'wiki', 'prompts']); - /** * Name and namespace from a repo-relative resource path, the same way pull reads * them (`//` is namespaced, `/` is shared). The @@ -120,7 +108,7 @@ function parseRelativePath( const rest = all.slice(rootSegs); if (rest.length <= 1) { const only = rest[0] ?? ''; - return { name: NESTED_TYPES.has(type) ? only : stripExtension(only) }; + return { name: isNestedPlaceableType(type) ? only : stripExtension(only) }; } const namespace = rest[0]; // Nested types keep the whole bundle-relative sub-path as the identity, never @@ -128,7 +116,7 @@ function parseRelativePath( // two resources, not a duplicate "guide", and the extension stays because // docs/a/api/guide.md and docs/b/api/guide.png are distinct files. The name // may contain '/', which namingFindings validates one segment at a time. - if (NESTED_TYPES.has(type)) { + if (isNestedPlaceableType(type)) { if (index.declares(NAMESPACE_AXIS[type], namespace!)) { return { name: rest.slice(1).join('/'), namespace }; } diff --git a/src/types.ts b/src/types.ts index ff456021f..123733fa9 100644 --- a/src/types.ts +++ b/src/types.ts @@ -947,6 +947,13 @@ export const PendingPushItemSchema = z.object({ * closed unmerged leaves no record behind, branch deleted or not. */ placed: z.boolean().optional(), + /** + * For a placement, the path the author still edits — the shared-root form + * `relativePath` was derived from, e.g. "docs/guide.md" for a destination of + * "docs/ops/guide.md". A self-mode scan compares that file against its placed + * counterpart once the record lands (`state.placements`). + */ + sourcePath: z.string().optional(), /** * Git blob id of the file this push wrote at `relativePath`, for a placed * item. Landing is proven by that blob appearing in the default branch's @@ -1109,6 +1116,20 @@ export const StateSchema = z.object({ * `placedRules`. */ placedAgents: z.record(z.string(), z.string()).optional(), + /** + * Where push placed a repo-backed resource (docs, wiki, prompts, workflows, + * scorers, models, env, hooks, claudemd, learnings) that the author still + * edits at its shared-root path, keyed `:`, e.g. + * `{ "docs:docs/guide.md": "docs/ops/guide.md" }`. Placement writes the team + * copy under `//` and leaves the author's own file at the root, so + * without this record the next self-mode scan reads that file as brand new, + * sends it to the same destination, and fails on the copy that is already + * there — once the PR has merged, the destination is exactly the file the + * scan never looks at. Recorded only after the placement landed, in + * `reconcilePlacementRecords`, on the same blob-in-history proof as + * `placedRules`, so a PR closed unmerged leaves no record behind. + */ + placements: z.record(z.string(), z.string()).optional(), /** * Default-branch commit the placement records were last checked against. A * record whose file was deleted after it is dropped even if something is at diff --git a/src/utils/pending-push.ts b/src/utils/pending-push.ts index 28a4d4681..8b4d11656 100644 --- a/src/utils/pending-push.ts +++ b/src/utils/pending-push.ts @@ -215,15 +215,18 @@ export async function toPendingItems(items: ResourceItem[], repoPath: string): P // A placement is marked only with the blob that can prove it landed: // without one, reconcile would have nothing but the path existing, which // another member's later file at that path also satisfies (#649 review). - const blob = isPlacement(i) ? await hashObject(repoPath, i.relativePath) : null; - if (isPlacement(i) && !blob) { + const placed = isPlacement(i) || isRepoPlacement(i); + const blob = placed ? await hashObject(repoPath, i.relativePath) : null; + if (placed && !blob) { log.warn(`[${i.type}] ${i.name}: could not hash ${i.relativePath}, so this machine will not treat it as its placement once merged.`); } + const sourcePath = (i as ResourceItem & { prePlacementPath?: string }).prePlacementPath; out.push({ type: i.type, name: i.name, relativePath: i.relativePath, namespace: i.namespace, + ...(sourcePath ? { sourcePath } : {}), ...(blob ? { placed: true, blob } : {}), }); } @@ -255,6 +258,21 @@ export function isPlacement(item: ResourceItem): boolean { return item.type === 'agents'; } +/** + * Whether pushing `item` PLACED a repo-backed resource (docs, wiki, prompts, + * workflows, ...): a new file the scan found at its shared-root path, which + * `pushItem` then wrote under `//`. Unlike a rule or agent, the author + * keeps editing the shared-root file, so the record has to name that path too + * (`sourcePath`) for a self-mode scan to compare against the destination. + */ +function isRepoPlacement(item: ResourceItem): boolean { + if (item.status !== 'new' || !item.namespace) return false; + // Skills, rules and agents keep their own records (placedRules/placedAgents, + // or a path derived from the skill name), so they are not keyed this way. + if (item.type === 'skills' || item.type === 'rules' || item.type === 'agents') return false; + return 'prePlacementPath' in item && typeof item.prePlacementPath === 'string'; +} + /** The shared-root file(s) that, if present, mean `name` is not ours to redirect. */ function sharedRootPaths(root: 'rules' | 'agents', name: string): string[] { return root === 'rules' ? [`rules/${name}.md`] : [`agents/${name}.yaml`, `agents/${name}.md`]; @@ -302,7 +320,7 @@ function sharedRootPaths(root: 'rules' | 'agents', name: string): string[] { */ export async function reconcilePlacementRecords( repoPath: string, - state: Pick, + state: Pick, tip?: string, deliversEveryNamespace: () => Promise = async () => false, ): Promise { @@ -330,12 +348,24 @@ export async function reconcilePlacementRecords( for (const item of entry.items) { if (!item.placed) continue; const field = fieldFor(item.type); - if (!field) continue; + // A repo-backed placement (docs, wiki, prompts, ...) has no + // placedRules/placedAgents field: it is recorded by the path the author + // still edits, so a self-mode scan can compare that file against the + // destination instead of reading it as a brand-new resource. Without the + // recorded source path there is nothing to key it by, so the mark is + // spent rather than guessed at. + const placementKey = field || !item.sourcePath ? null : `${item.type}:${item.sourcePath}`; + if (!field && !placementKey) { + item.placed = false; + delete item.blob; + changed = true; + continue; + } // Without the blob push wrote, landing cannot be told from another // member creating the same path after this PR closed unmerged, so the // mark is spent rather than recorded on the path existing (#649 review). if (!item.blob) { - log.debug(`Not recording placement ${field}.${item.name}: no blob to prove it landed`); + log.debug(`Not recording placement ${field ?? placementKey}.${item.name}: no blob to prove it landed`); item.placed = false; changed = true; continue; @@ -366,15 +396,19 @@ export async function reconcilePlacementRecords( // after this placement landed: what is there now was recreated by // someone else, and the placement is spent without a record. if (entry.base && await pathDeletedSince(repoPath, entry.base, item.relativePath, history) === true) { - log.debug(`Not recording placement ${field}.${item.name}: ${item.relativePath} was deleted after it landed`); + log.debug(`Not recording placement ${field ?? placementKey}.${item.name}: ${item.relativePath} was deleted after it landed`); item.placed = false; delete item.blob; changed = true; continue; } - log.debug(`Recording placement ${field}.${item.name} → ${item.relativePath}: landed on the default branch`); - state[field] = { ...state[field], [item.name]: item.relativePath }; - recordedNow.add(`${field}:${item.name}`); + log.debug(`Recording placement ${field ?? placementKey}.${item.name} → ${item.relativePath}: landed on the default branch`); + if (field) { + state[field] = { ...state[field], [item.name]: item.relativePath }; + recordedNow.add(`${field}:${item.name}`); + } else if (placementKey) { + state.placements = { ...state.placements, [placementKey]: item.relativePath }; + } // Consumed: a placement is recorded once. Left marked, it would record // again after the team deleted the file and another member recreated // the path — the blob stays in history, so the check above would still @@ -429,6 +463,25 @@ export async function reconcilePlacementRecords( state[field] = kept; } + // A repo-backed placement whose destination is gone from the default branch + // is dropped: the team deleted that file, so the author's root copy is a new + // resource again. Kept, it would compare the copy against a path nobody has + // and hide a genuine edit. + if (state.placements) { + const kept: Record = {}; + let dropped = false; + for (const [key, recorded] of Object.entries(state.placements)) { + if (await exists(recorded)) { + kept[key] = recorded; + } else { + log.debug(`Dropping placement record ${key} → ${recorded}: gone from the default branch`); + dropped = true; + changed = true; + } + } + if (dropped) state.placements = kept; + } + // The revision the surviving records were checked against, so the next run // sees a deletion that happened in between even if the path is back by then. // With no record left there is nothing it could vouch for, and kept it would From 161016ddfba6d59b535a113f0604def02e7e6c61 Mon Sep 17 00:00:00 2001 From: legend91019 <15853153143@163.com> Date: Fri, 23 Oct 2026 00:21:37 +0800 Subject: [PATCH 14/28] fix(push): reuse a placement record in env scans, and expire one whose file was deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - EnvHandler's self-mode scan reads state.placements too: an env file an earlier push placed under env// is compared against that destination, so the unchanged .teamai/env/env.yaml is not new again (and does not collide with the file it produced), and an edit is reported on the placed path - a repo-backed placement record is dropped when its destination was deleted since the last checkpoint, even if the path exists again: what is there now is somebody else's, and following the record would route this author's edits onto it. Same rule and same checkpoint as placedRules/placedAgents, which now stays while any record — including these — survives - share namespaceOfPlacedPath in push-namespaces so both scans read the namespace a record's destination carries the same way --- src/__tests__/placement-records.test.ts | 30 ++++++++++++++++ .../push-all-resources-namespaces.test.ts | 35 +++++++++++++++++++ src/push-namespaces.ts | 9 +++++ src/resources/env.ts | 21 +++++++++++ src/resources/repo-resource.ts | 7 +--- src/utils/pending-push.ts | 22 +++++++++--- 6 files changed, 114 insertions(+), 10 deletions(-) diff --git a/src/__tests__/placement-records.test.ts b/src/__tests__/placement-records.test.ts index 0a587725c..1b1993f4c 100644 --- a/src/__tests__/placement-records.test.ts +++ b/src/__tests__/placement-records.test.ts @@ -83,6 +83,36 @@ describe('reconcilePlacementRecords', () => { expect(state.pendingPushes[0]?.items[0]?.placed).toBe(false); }); + it('drops a repo-backed record whose destination was deleted since the last check', async () => { + // A removal that merged, then another member's file at the same path: the + // record must not route this author's edits onto that file. + const run = (args: string[]) => execFileSync('git', args, { cwd: repoPath, encoding: 'utf8', env: { + ...process.env, GIT_AUTHOR_NAME: 'T', GIT_AUTHOR_EMAIL: 't@t', GIT_COMMITTER_NAME: 'T', GIT_COMMITTER_EMAIL: 't@t', + } }).trim(); + const commit = (message: string): string => { + run(['add', '-A']); + run(['commit', '-q', '-m', message]); + return run(['rev-parse', 'HEAD']); + }; + run(['init', '-q', '-b', 'main']); + await fse.outputFile(path.join(repoPath, 'docs/ops/guide.md'), 'ours'); + const checkedAt = commit('landed'); + await fse.remove(path.join(repoPath, 'docs/ops/guide.md')); + commit('removed'); + await fse.outputFile(path.join(repoPath, 'docs/ops/guide.md'), 'somebody else\'s'); + commit('recreated'); + + const state = { + placedRules: {}, placedAgents: {}, + placements: { 'docs:docs/guide.md': 'docs/ops/guide.md' }, + placementsCheckedAt: checkedAt, + pendingPushes: [], + }; + + expect(await reconcilePlacementRecords(repoPath, state)).toBe(true); + expect(state.placements).toEqual({}); + }); + it('spends a placement with no blob instead of recording it because the path exists', async () => { // Nothing tells its landing from another member creating the path after // this PR closed unmerged (#649 review). diff --git a/src/__tests__/push-all-resources-namespaces.test.ts b/src/__tests__/push-all-resources-namespaces.test.ts index a1eb723ab..45a6181c8 100644 --- a/src/__tests__/push-all-resources-namespaces.test.ts +++ b/src/__tests__/push-all-resources-namespaces.test.ts @@ -369,4 +369,39 @@ describe('repo resource handler push and scan', () => { status: 'modified', }); }); + + it('compares a root-authored env file against the destination an earlier push placed it at', async () => { + const root = makeTmpDir(); + const projectRoot = path.join(root, 'project'); + const repoLocal = path.join(root, 'team-repo'); + const localConfig: LocalConfig = { + repo: { localPath: repoLocal, remote: 'local', kind: 'self' }, + username: 'alice', + scope: 'project', + projectRoot, + dataHome: path.join(root, 'data'), + additionalRoles: [], + }; + + const activeFile = path.join(projectRoot, '.teamai', 'env', 'env.yaml'); + fs.mkdirSync(path.dirname(activeFile), { recursive: true }); + fs.writeFileSync(activeFile, 'TEAM_TOKEN: one\n'); + const destFile = path.join(repoLocal, 'env', 'ops', 'env.yaml'); + fs.mkdirSync(path.dirname(destFile), { recursive: true }); + fs.writeFileSync(destFile, 'TEAM_TOKEN: one\n'); + const state = await loadStateForScope(localConfig); + state.placements = { 'env:env/env.yaml': 'env/ops/env.yaml' }; + await saveStateForScope(state, localConfig); + + expect(await getHandler('env').scanLocalForPush({} as never, localConfig)).toEqual([]); + + fs.writeFileSync(activeFile, 'TEAM_TOKEN: two\n'); + const edited = await getHandler('env').scanLocalForPush({} as never, localConfig); + expect(edited).toHaveLength(1); + expect(edited[0]).toMatchObject({ + relativePath: 'env/ops/env.yaml', + namespace: 'ops', + status: 'modified', + }); + }); }); diff --git a/src/push-namespaces.ts b/src/push-namespaces.ts index 4d806da9a..b9e1365d1 100644 --- a/src/push-namespaces.ts +++ b/src/push-namespaces.ts @@ -144,6 +144,15 @@ export function rootSegmentsFor(type: PlaceableType): number { return type === 'wiki' ? 3 : 1; } +/** + * The namespace a placed path carries, read from the slot `withNamespace` + * inserted: `docs/ops/guide.md` → `ops`, `teamwiki/evidence/code/ops/page.md` → + * `ops`. Undefined when that segment is missing (a malformed record). + */ +export function namespaceOfPlacedPath(type: PlaceableType, placedPath: string): string | undefined { + return placedPath.split('/')[rootSegmentsFor(type)]; +} + /** * Where a skill lands in `namespace`. Built from the root rather than through * `withNamespace`, because a skill's team path is derived from its name alone diff --git a/src/resources/env.ts b/src/resources/env.ts index 755b2b8c7..e2ef5df70 100644 --- a/src/resources/env.ts +++ b/src/resources/env.ts @@ -7,6 +7,8 @@ import type { ResourceItem, ResourceItemStatus, TeamaiConfig, LocalConfig } from import { TEAMAI_ENV_START, TEAMAI_ENV_END, getDataHome, getEnvBackupPath, getTeamaiHome, isSelfMode } from '../types.js'; import { pathExists, readFileIfExists, readFileSafe, writeFile, ensureDir, fileContentEqual, remove, symlinkTarget } from '../utils/fs.js'; import { log } from '../utils/logger.js'; +import { loadStateForScope } from '../config.js'; +import { namespaceOfPlacedPath } from '../push-namespaces.js'; import { envShExportsPath, envShMarker, isEnvShMarker, recordEnvShExports } from '../env-sh-exports.js'; import { ENV_KEY_RE, envNamePattern, envNamePatternKey, envTable, sameEnvName } from './env-key.js'; import { SECRETS_LAYOUT } from './secrets.js'; @@ -339,11 +341,30 @@ export class EnvHandler extends ResourceHandler { if (isSelfMode(localConfig) && localConfig.projectRoot) { const activeRoot = path.join(localConfig.projectRoot, '.teamai'); const items: ResourceItem[] = []; + // Where an earlier push placed a file this machine still edits at + // .teamai/env/, recorded once that placement landed (`state.placements`). + // Without it the unchanged original reads as new on every run and the + // placement collides with the copy it already wrote. + const placements = (await loadStateForScope(localConfig)).placements ?? {}; for (const { relativePath, absolutePath: activeEnv } of await listEnvFiles(activeRoot)) { const baseEnv = path.join(localConfig.repo.localPath, ...relativePath.split('/')); // Not in the baseline → new; present but different → modified; equal → skip. const baseExists = await pathExists(baseEnv); if (baseExists && await fileContentEqual(activeEnv, baseEnv)) continue; + if (!baseExists) { + const placedPath = placements[`env:${relativePath}`]; + const placedFile = placedPath ? path.join(localConfig.repo.localPath, ...placedPath.split('/')) : null; + if (placedPath && placedFile && await pathExists(placedFile)) { + // Unchanged: already delivered. Edited: the edit belongs on the + // file that landed in its place, not on a second copy at the root. + if (await fileContentEqual(activeEnv, placedFile)) continue; + items.push({ + ...envPushItem(placedPath, activeEnv, 'modified'), + namespace: namespaceOfPlacedPath('env', placedPath), + }); + continue; + } + } items.push(envPushItem(relativePath, activeEnv, baseExists ? 'modified' : 'new')); } return items; diff --git a/src/resources/repo-resource.ts b/src/resources/repo-resource.ts index b06e6f566..bddf0692a 100644 --- a/src/resources/repo-resource.ts +++ b/src/resources/repo-resource.ts @@ -3,14 +3,9 @@ import fs from 'node:fs/promises'; import { ResourceHandler } from './base.js'; import { loadStateForScope } from '../config.js'; import { isSelfMode, type LocalConfig, type ResourceItem, type ResourceType, type TeamaiConfig } from '../types.js'; -import { rootSegmentsFor, type PlaceableType } from '../push-namespaces.js'; +import { namespaceOfPlacedPath, type PlaceableType } from '../push-namespaces.js'; import { ensureDir, fileContentEqual, pathExists } from '../utils/fs.js'; -/** The namespace a placed path carries, from its position below the root. */ -function namespaceOfPlacedPath(type: PlaceableType, placedPath: string): string | undefined { - return placedPath.split('/')[rootSegmentsFor(type)]; -} - export async function listRepoFilesRecursive(dir: string, baseDir = dir): Promise { try { const entries = await fs.readdir(dir, { withFileTypes: true }); diff --git a/src/utils/pending-push.ts b/src/utils/pending-push.ts index 8b4d11656..697716b56 100644 --- a/src/utils/pending-push.ts +++ b/src/utils/pending-push.ts @@ -408,6 +408,7 @@ export async function reconcilePlacementRecords( recordedNow.add(`${field}:${item.name}`); } else if (placementKey) { state.placements = { ...state.placements, [placementKey]: item.relativePath }; + recordedNow.add(placementKey); } // Consumed: a placement is recorded once. Left marked, it would record // again after the team deleted the file and another member recreated @@ -471,13 +472,25 @@ export async function reconcilePlacementRecords( const kept: Record = {}; let dropped = false; for (const [key, recorded] of Object.entries(state.placements)) { - if (await exists(recorded)) { - kept[key] = recorded; - } else { + if (!await exists(recorded)) { log.debug(`Dropping placement record ${key} → ${recorded}: gone from the default branch`); dropped = true; changed = true; + continue; + } + // Deleted and recreated between two checks — a removal that merged, then + // another member's file at the same path — is not this author's: following + // the record would route their edits onto that file. Same rule as the + // placedRules/placedAgents records below, and the same reason + // `placementsCheckedAt` is kept. + if (checkedAt && !recordedNow.has(key) + && await pathDeletedSince(repoPath, checkedAt, recorded, history) === true) { + log.debug(`Dropping placement record ${key} → ${recorded}: deleted from the default branch since the last check`); + dropped = true; + changed = true; + continue; } + kept[key] = recorded; } if (dropped) state.placements = kept; } @@ -486,7 +499,8 @@ export async function reconcilePlacementRecords( // sees a deletion that happened in between even if the path is back by then. // With no record left there is nothing it could vouch for, and kept it would // later read a deletion from before a new record existed as that record's. - if (Object.keys(state.placedRules ?? {}).length + Object.keys(state.placedAgents ?? {}).length > 0) { + if (Object.keys(state.placedRules ?? {}).length + Object.keys(state.placedAgents ?? {}).length + + Object.keys(state.placements ?? {}).length > 0) { const head = await getHeadCommit(repoPath, history); if (head && head !== state.placementsCheckedAt) { state.placementsCheckedAt = head; From bb5919906c012b8bb1abe002ec94bc890d9c4d04 Mon Sep 17 00:00:00 2001 From: legend91019 <15853153143@163.com> Date: Fri, 23 Oct 2026 10:16:47 +0800 Subject: [PATCH 15/28] fix(push,workflow): retire a placed env original, keep explicit prompt namespaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - EnvHandler.pushItem retires the clone-local original once a placement has copied it, as pushRepoResourceItem does: a leftover untracked env/env.yaml was rediscovered as new by the next push, which then aborted because the placed env//env.yaml already existed. A self-mode source lives in the active .teamai tree, outside the clone, and is kept — that is the copy the author still edits - resolvePromptFiles resolves a prompt_file whose first segment is already an active namespace as written: prompts/ops/note.md no longer looks up prompts//ops/note.md, which loaded a different file whenever both existed --- src/__tests__/self-mode-env-agents.test.ts | 38 ++++++++++++++++++++++ src/__tests__/workflow-cache.test.ts | 23 +++++++++++++ src/resources/env.ts | 19 ++++++++--- src/workflow/cache.ts | 8 ++++- 4 files changed, 83 insertions(+), 5 deletions(-) diff --git a/src/__tests__/self-mode-env-agents.test.ts b/src/__tests__/self-mode-env-agents.test.ts index 1136fd64c..883d32b1d 100644 --- a/src/__tests__/self-mode-env-agents.test.ts +++ b/src/__tests__/self-mode-env-agents.test.ts @@ -117,6 +117,44 @@ describe('single-repo mode: env + agents direct .teamai scan', () => { expect(await fse.readFile(path.join(worktreeTeamai, 'env', 'secrets.yaml'), 'utf8')).toContain('GITHUB_TOKEN'); }); + it('env: retires the clone-local original after a placement copies it', async () => { + // Non-self push: the source IS inside the clone (a team clone's env/env.yaml) + // and placement moved it to env/ops/env.yaml. The leftover original stays + // untracked, is rediscovered as new by the next push, and that push aborts + // because the placed copy already exists — so it is retired after the copy, + // the same way pushRepoResourceItem handles a placed repo resource. + const repoRoot = path.join(tmp, 'team-clone'); + const plainConfig: LocalConfig = { + repo: { localPath: repoRoot, remote: 'https://github.com/acme/team.git', kind: 'git' }, + username: 'alice', + scope: 'project', + additionalRoles: [], + }; + const original = path.join(repoRoot, 'env', 'env.yaml'); + await fse.outputFile(original, 'variables:\n - key: A\n value: "1"\n'); + await new EnvHandler().pushItem( + { name: 'env.yaml', type: 'env', sourcePath: original, relativePath: 'env/ops/env.yaml' }, + teamConfig, + plainConfig, + ); + expect(await fse.readFile(path.join(repoRoot, 'env', 'ops', 'env.yaml'), 'utf8')).toContain('key: A'); + expect(await fse.pathExists(original)).toBe(false); + }); + + it('env: keeps a self-mode source outside the clone after a placement copies it', async () => { + // The active tree's .teamai/env/env.yaml is what the author keeps editing; + // only a clone-local original is retired. + const active = path.join(bizRoot, '.teamai', 'env', 'env.yaml'); + await fse.writeFile(active, 'variables:\n - key: B\n value: "2"\n'); + await new EnvHandler().pushItem( + { name: 'env.yaml', type: 'env', sourcePath: active, relativePath: 'env/ops/env.yaml' }, + teamConfig, + localConfig, + ); + expect(await fse.readFile(path.join(worktreeTeamai, 'env', 'ops', 'env.yaml'), 'utf8')).toContain('key: B'); + expect(await fse.pathExists(active)).toBe(true); + }); + // Regression: a teammate who clones a self-mode repo has .teamai/env/ as a // committed DIRECTORY (holding env.yaml). pullItem must NOT try to write its // KEY=value backup at /env (that path is the dir → EISDIR). It must diff --git a/src/__tests__/workflow-cache.test.ts b/src/__tests__/workflow-cache.test.ts index 268c3cc09..8208f89ce 100644 --- a/src/__tests__/workflow-cache.test.ts +++ b/src/__tests__/workflow-cache.test.ts @@ -507,6 +507,29 @@ steps: expect((await loadCachedWorkflow(config, 'build'))?.definition.steps[0]?.prompt).toBe('second prompt'); }); + it('resolves an already namespace-qualified prompt path as written', async () => { + const config = testConfig(); + // Two active namespaces, and prompts/dev/ops/note.md exists — the shape that + // used to make an explicit prompts/ops/note.md load the wrong file: the path + // was prefixed with every active namespace, and prompts/dev/ops/note.md won. + fs.mkdirSync(path.join(config.repo.localPath, 'prompts', 'ops'), { recursive: true }); + fs.mkdirSync(path.join(config.repo.localPath, 'prompts', 'dev', 'ops'), { recursive: true }); + fs.writeFileSync(path.join(config.repo.localPath, 'prompts', 'ops', 'note.md'), 'ops prompt'); + fs.writeFileSync(path.join(config.repo.localPath, 'prompts', 'dev', 'ops', 'note.md'), 'dev prompt'); + writeWorkflow(config, 'build', `schemaVersion: v1alpha1 +name: build +description: Review the project. +steps: + - id: review + prompt_file: prompts/ops/note.md +`); + + await syncWorkflowDefinitions(config); + + const cached = await loadCachedWorkflow(config, 'build'); + expect(cached?.definition.steps[0]).toMatchObject({ prompt: 'ops prompt' }); + }); + it('rejects symlinked prompt files that escape the team repo', async () => { const config = testConfig(); const outside = path.join(path.dirname(config.repo.localPath), 'outside-prompt.md'); diff --git a/src/resources/env.ts b/src/resources/env.ts index e2ef5df70..6cff8b98c 100644 --- a/src/resources/env.ts +++ b/src/resources/env.ts @@ -428,11 +428,22 @@ export class EnvHandler extends ResourceHandler { // Self mode: the source is the ACTIVE tree's .teamai/env/ file, but the // commit happens in the knowledge worktree (localConfig.repo.localPath). // Non-self: the source is the clone, but placement may have moved the path. - const dest = path.join(localConfig.repo.localPath, ...item.relativePath.split('/')); - if (item.sourcePath !== dest) { + const repoRoot = path.resolve(localConfig.repo.localPath); + const dest = path.resolve(repoRoot, ...item.relativePath.split('/')); + const original = path.resolve(item.sourcePath); + if (original !== dest) { await ensureDir(path.dirname(dest)); - const content = await readFileSafe(item.sourcePath); - if (content !== null) await writeFile(dest, content); + const content = await readFileSafe(original); + if (content !== null) { + await writeFile(dest, content); + // Retire the clone-local original, as pushRepoResourceItem does: the + // placement moved the file, and a leftover env/env.yaml stays untracked + // and is rediscovered as new by the next push, which then aborts + // because the placed env//env.yaml already exists. In self mode the + // source is the ACTIVE tree's .teamai/env file, outside the clone, and + // must stay: that is the copy the author keeps editing. + if (original.startsWith(repoRoot + path.sep)) await remove(original); + } } } diff --git a/src/workflow/cache.ts b/src/workflow/cache.ts index 8d9c53520..7c454912b 100644 --- a/src/workflow/cache.ts +++ b/src/workflow/cache.ts @@ -576,7 +576,13 @@ async function resolvePromptFiles( // Namespace lookup works for any leaf depth — a nested path like // prompts/topic/note.md resolves to prompts//topic/note.md // just as a flat prompts/note.md resolves to prompts//note.md. - const found = (await Promise.all( + // + // A leaf whose first segment is already an active namespace is explicit + // and resolved as written: prepending another namespace would look up + // prompts//ops/note.md and load that file instead. + const lead = leaf.split('/')[0]; + const alreadyNamespaced = lead !== undefined && activePromptsNamespaces.includes(lead); + const found = alreadyNamespaced ? [] : (await Promise.all( activePromptsNamespaces.map(async (ns) => { const candidate = path.join(root, 'prompts', ns, ...leaf.split('/')); return (await pathExists(candidate)) ? `prompts/${ns}/${leaf}` : null; From 7e6dd4da7ad59965ecbf627818ca92aca30ddc3a Mon Sep 17 00:00:00 2001 From: legend91019 <15853153143@163.com> Date: Fri, 23 Oct 2026 10:34:47 +0800 Subject: [PATCH 16/28] test(push): use .md scorer names in the placement samples A scorer is scorers/.md (frontmatter plus a prompt), and review --branch fails any other file under scorers/ but README.md (#1085), so the sample items name .md files rather than .py. --- src/__tests__/push-all-resources-namespaces.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/__tests__/push-all-resources-namespaces.test.ts b/src/__tests__/push-all-resources-namespaces.test.ts index 45a6181c8..cecaf064a 100644 --- a/src/__tests__/push-all-resources-namespaces.test.ts +++ b/src/__tests__/push-all-resources-namespaces.test.ts @@ -87,8 +87,10 @@ describe('isAtSharedRoot and withNamespace across all resource types', () => { // namespace holds it. { type: 'prompts', path: 'prompts/team-a/review.md', shared: true }, { type: 'prompts', path: 'prompts/topic/note.md', shared: true }, - { type: 'scorers', path: 'scorers/accuracy.py', shared: true }, - { type: 'scorers', path: 'scorers/team-a/accuracy.py', shared: false }, + // A scorer is scorers/.md (frontmatter plus a prompt), never .py: + // review --branch fails any other file under scorers/ but README.md. + { type: 'scorers', path: 'scorers/accuracy.md', shared: true }, + { type: 'scorers', path: 'scorers/team-a/accuracy.md', shared: false }, { type: 'models', path: 'models/config.json', shared: true }, { type: 'models', path: 'models/team-a/config.json', shared: false }, { type: 'wiki', path: 'wiki/home.md', shared: true }, @@ -151,7 +153,7 @@ describe('isAtSharedRoot and withNamespace across all resource types', () => { expect(withNamespace('docs/readme.md', 'core')).toBe('docs/core/readme.md'); expect(withNamespace('workflows/build.yaml', 'ci')).toBe('workflows/ci/build.yaml'); expect(withNamespace('prompts/codegen.txt', 'ai')).toBe('prompts/ai/codegen.txt'); - expect(withNamespace('scorers/metric.py', 'eval')).toBe('scorers/eval/metric.py'); + expect(withNamespace('scorers/metric.md', 'eval')).toBe('scorers/eval/metric.md'); expect(withNamespace('models/deepseek.json', 'llm')).toBe('models/llm/deepseek.json'); expect(withNamespace('wiki/architecture.md', 'docs')).toBe('wiki/docs/architecture.md'); expect(withNamespace('hooks/pre-push.sh', 'git')).toBe('hooks/git/pre-push.sh'); From c15069a6927eb52055baf2be63b9373fe0b8a9f7 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 22 Oct 2026 21:54:14 +0200 Subject: [PATCH 17/28] fix(push): compare a placed copy with what its push delivered A placement record held only the destination path, so the self-mode scan compared the author's root copy with the placed file as it is now. Once a teammate changed that file, an untouched copy read as an edit and was pushed over their change, with no warning. Keep the delivered blob with the record and compare the author's copy with it: equal is already delivered, different is the author's edit, which goes to the placed file unless that file changed too, in which case the scan stops and names the file to edit. An edit that lands refreshes the record. A record holding only the path, as earlier builds wrote, is dropped. --- src/__tests__/placement-records.test.ts | 9 +- .../push-all-resources-namespaces.test.ts | 133 ++++++++++++++---- src/resources/repo-resource.ts | 65 ++++++--- src/types.ts | 30 ++-- src/utils/pending-push.ts | 21 +-- 5 files changed, 193 insertions(+), 65 deletions(-) diff --git a/src/__tests__/placement-records.test.ts b/src/__tests__/placement-records.test.ts index 1b1993f4c..25188e08a 100644 --- a/src/__tests__/placement-records.test.ts +++ b/src/__tests__/placement-records.test.ts @@ -15,7 +15,7 @@ vi.mock('../utils/git.js', async () => ({ import { execFileSync } from 'node:child_process'; import { reconcilePlacementRecords, isPlacement } from '../utils/pending-push.js'; -import type { PendingPush, ResourceItem } from '../types.js'; +import type { PendingPush, ResourceItem, State } from '../types.js'; /** * A placement record says "the author's root copy of IS the team file @@ -66,7 +66,7 @@ describe('reconcilePlacementRecords', () => { // keeps editing, so a self-mode scan can compare it with the destination. const blob = await land('docs/ops/guide.md', 'x'); const state = { - placedRules: {}, placedAgents: {}, placements: {} as Record, + placedRules: {}, placedAgents: {}, placements: {} as NonNullable, pendingPushes: [pending([{ type: 'docs', name: 'guide.md', @@ -79,7 +79,7 @@ describe('reconcilePlacementRecords', () => { }; expect(await reconcilePlacementRecords(repoPath, state)).toBe(true); - expect(state.placements).toEqual({ 'docs:docs/guide.md': 'docs/ops/guide.md' }); + expect(state.placements).toEqual({ 'docs:docs/guide.md': { path: 'docs/ops/guide.md', blob } }); expect(state.pendingPushes[0]?.items[0]?.placed).toBe(false); }); @@ -96,6 +96,7 @@ describe('reconcilePlacementRecords', () => { }; run(['init', '-q', '-b', 'main']); await fse.outputFile(path.join(repoPath, 'docs/ops/guide.md'), 'ours'); + const blob = run(['hash-object', 'docs/ops/guide.md']); const checkedAt = commit('landed'); await fse.remove(path.join(repoPath, 'docs/ops/guide.md')); commit('removed'); @@ -104,7 +105,7 @@ describe('reconcilePlacementRecords', () => { const state = { placedRules: {}, placedAgents: {}, - placements: { 'docs:docs/guide.md': 'docs/ops/guide.md' }, + placements: { 'docs:docs/guide.md': { path: 'docs/ops/guide.md', blob } }, placementsCheckedAt: checkedAt, pendingPushes: [], }; diff --git a/src/__tests__/push-all-resources-namespaces.test.ts b/src/__tests__/push-all-resources-namespaces.test.ts index cecaf064a..5e8bed74a 100644 --- a/src/__tests__/push-all-resources-namespaces.test.ts +++ b/src/__tests__/push-all-resources-namespaces.test.ts @@ -1,4 +1,5 @@ import fs from 'node:fs'; +import { execFileSync } from 'node:child_process'; import os from 'node:os'; import path from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; @@ -13,10 +14,11 @@ import { withNamespace, } from '../push-namespaces.js'; import type { ProjectsManifest } from '../projects.js'; -import type { LocalConfig, ResourceItem } from '../types.js'; +import { getStatePath, type LocalConfig, type ResourceItem } from '../types.js'; import { scanRepoResourceForPush, pushRepoResourceItem } from '../resources/repo-resource.js'; import { getHandler } from '../resources/index.js'; import { loadStateForScope, saveStateForScope } from '../config.js'; +import { reconcilePlacementRecords, toPendingItems } from '../utils/pending-push.js'; const dirs: string[] = []; @@ -329,47 +331,127 @@ describe('repo resource handler push and scan', () => { expect(fs.existsSync(destFile)).toBe(true); expect(fs.readFileSync(destFile, 'utf8')).toBe('name: test\n'); }); +}); - it('compares a root-authored file against the destination an earlier push placed it at', async () => { +/** + * Self mode: the author keeps editing the shared-root copy (`.teamai/docs/guide.md`) + * after push placed it at `docs/ops/guide.md` and the PR merged. The record of + * that landing says what was delivered, so the scan can tell the author's edit + * from a teammate's later change to the placed file. + */ +describe('a placed copy the author still edits, against what its push delivered', () => { + const run = (cwd: string, args: string[]): string => execFileSync('git', args, { + cwd, + encoding: 'utf8', + env: { ...process.env, GIT_AUTHOR_NAME: 'T', GIT_AUTHOR_EMAIL: 't@t', GIT_COMMITTER_NAME: 'T', GIT_COMMITTER_EMAIL: 't@t' }, + }).trim(); + + async function placedCopy(type: 'docs', authorPath: string, placedPath: string) { const root = makeTmpDir(); const projectRoot = path.join(root, 'project'); const repoLocal = path.join(root, 'team-repo'); - const dataHome = path.join(root, 'data'); const localConfig: LocalConfig = { repo: { localPath: repoLocal, remote: 'local', kind: 'self' }, username: 'alice', scope: 'project', projectRoot, - dataHome, + dataHome: path.join(root, 'data'), additionalRoles: [], }; + const activeFile = path.join(projectRoot, '.teamai', ...authorPath.split('/')); + const placedFile = path.join(repoLocal, ...placedPath.split('/')); + const commit = (file: string, content: string | null, message: string): void => { + if (content === null) fs.rmSync(file); + else { + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, content); + } + run(repoLocal, ['add', '-A']); + run(repoLocal, ['commit', '-q', '-m', message]); + }; + const scan = () => getHandler(type).scanLocalForPush({} as never, localConfig); + /** Merge `items` as a push branch built on the current default branch, then reconcile as pull does. */ + const land = async (items: ResourceItem[]): Promise => { + const base = run(repoLocal, ['rev-parse', 'HEAD']); + for (const item of items) await getHandler(type).pushItem(item, {} as never, localConfig); + const pendingItems = await toPendingItems(items, repoLocal); + run(repoLocal, ['add', '-A']); + run(repoLocal, ['commit', '-q', '-m', 'merge the push']); + const state = await loadStateForScope(localConfig); + state.pendingPushes = [{ branch: 'teamai/push/alice/1', prUrl: null, createdAt: '2026-01-01T00:00:00.000Z', base, items: pendingItems }]; + await reconcilePlacementRecords(repoLocal, state); + await saveStateForScope(state, localConfig); + }; - // The author still edits .teamai/docs/guide.md; the team file it became is - // docs/ops/guide.md, and state.placements remembers the two are one file. - const activeFile = path.join(projectRoot, '.teamai', 'docs', 'guide.md'); + fs.mkdirSync(repoLocal, { recursive: true }); + run(repoLocal, ['init', '-q', '-b', 'main']); + commit(path.join(repoLocal, 'README.md'), 'team\n', 'seed'); fs.mkdirSync(path.dirname(activeFile), { recursive: true }); fs.writeFileSync(activeFile, 'v1\n'); - const destFile = path.join(repoLocal, 'docs', 'ops', 'guide.md'); - fs.mkdirSync(path.dirname(destFile), { recursive: true }); - fs.writeFileSync(destFile, 'v1\n'); - const state = await loadStateForScope(localConfig); - state.placements = { 'docs:docs/guide.md': 'docs/ops/guide.md' }; - await saveStateForScope(state, localConfig); + const [placed] = await scan(); + expect(placed).toMatchObject({ relativePath: authorPath, status: 'new' }); + // What placeNewResources does for --role ops. + Object.assign(placed, { namespace: 'ops', prePlacementPath: authorPath, relativePath: placedPath }); + await land([placed]); + expect(fs.readFileSync(placedFile, 'utf8')).toBe('v1\n'); + return { activeFile, placedFile, commit, scan, land, localConfig }; + } + + it('has nothing to push for an unchanged copy', async () => { + const { scan } = await placedCopy('docs', 'docs/guide.md', 'docs/ops/guide.md'); + expect(await scan()).toEqual([]); + }); + + it('does not push an unchanged copy over a teammate\'s later fix of the placed file', async () => { + const { placedFile, commit, scan } = await placedCopy('docs', 'docs/guide.md', 'docs/ops/guide.md'); + commit(placedFile, 'v2 (teammate fix)\n', 'teammate fixes the placed file'); - // Unchanged: already delivered, so it is not a new resource again. - expect(await scanRepoResourceForPush('docs', 'docs', {} as never, localConfig)).toEqual([]); + expect(await scan()).toEqual([]); + }); + + it('does not push an unchanged copy over a file someone re-created at the placed path', async () => { + const { placedFile, commit, scan } = await placedCopy('docs', 'docs/guide.md', 'docs/ops/guide.md'); + commit(placedFile, null, 'team deletes the placed file'); + commit(placedFile, 'somebody else\'s guide\n', 'another member creates the path'); + + expect(await scan()).toEqual([]); + }); - // Edited: the edit belongs on the placed file, not on a second copy that - // would collide with it, so the scan reports it as that path's modification. + it('sends an edit to the placed file', async () => { + const { activeFile, scan } = await placedCopy('docs', 'docs/guide.md', 'docs/ops/guide.md'); fs.writeFileSync(activeFile, 'v2\n'); - const edited = await scanRepoResourceForPush('docs', 'docs', {} as never, localConfig); + + const edited = await scan(); expect(edited).toHaveLength(1); - expect(edited[0]).toMatchObject({ - name: 'guide.md', - relativePath: 'docs/ops/guide.md', - namespace: 'ops', - status: 'modified', - }); + expect(edited[0]).toMatchObject({ relativePath: 'docs/ops/guide.md', namespace: 'ops', status: 'modified' }); + }); + + it('has nothing to push once the edit it sent has landed', async () => { + const { activeFile, scan, land } = await placedCopy('docs', 'docs/guide.md', 'docs/ops/guide.md'); + fs.writeFileSync(activeFile, 'v2\n'); + await land(await scan()); + + expect(await scan()).toEqual([]); + }); + + it('stops on an edit when the placed file also changed since, naming the file to edit instead', async () => { + const { activeFile, placedFile, commit, scan } = await placedCopy('docs', 'docs/guide.md', 'docs/ops/guide.md'); + commit(placedFile, 'v2 (teammate fix)\n', 'teammate fixes the placed file'); + fs.writeFileSync(activeFile, 'v1 plus my edit\n'); + + await expect(scan()).rejects.toThrow(/docs\/ops\/guide\.md/); + }); + + it('reads a record that holds only the path, as earlier builds wrote, as no record', async () => { + const { scan, localConfig } = await placedCopy('docs', 'docs/guide.md', 'docs/ops/guide.md'); + const statePath = getStatePath(localConfig); + const raw = JSON.parse(fs.readFileSync(statePath, 'utf8')); + raw.placements = { 'docs:docs/guide.md': 'docs/ops/guide.md' }; + fs.writeFileSync(statePath, JSON.stringify(raw)); + + // Without the delivered blob an unchanged copy cannot be told from an + // edit, so it is new again and placement refuses the occupied path. + expect(await scan()).toMatchObject([{ relativePath: 'docs/guide.md', status: 'new' }]); }); it('compares a root-authored env file against the destination an earlier push placed it at', async () => { @@ -392,7 +474,8 @@ describe('repo resource handler push and scan', () => { fs.mkdirSync(path.dirname(destFile), { recursive: true }); fs.writeFileSync(destFile, 'TEAM_TOKEN: one\n'); const state = await loadStateForScope(localConfig); - state.placements = { 'env:env/env.yaml': 'env/ops/env.yaml' }; + const blob = execFileSync('git', ['hash-object', '--', destFile], { encoding: 'utf8' }).trim(); + state.placements = { 'env:env/env.yaml': { path: 'env/ops/env.yaml', blob } }; await saveStateForScope(state, localConfig); expect(await getHandler('env').scanLocalForPush({} as never, localConfig)).toEqual([]); diff --git a/src/resources/repo-resource.ts b/src/resources/repo-resource.ts index bddf0692a..1366f9eb4 100644 --- a/src/resources/repo-resource.ts +++ b/src/resources/repo-resource.ts @@ -2,9 +2,10 @@ import path from 'node:path'; import fs from 'node:fs/promises'; import { ResourceHandler } from './base.js'; import { loadStateForScope } from '../config.js'; -import { isSelfMode, type LocalConfig, type ResourceItem, type ResourceType, type TeamaiConfig } from '../types.js'; +import { isSelfMode, type LocalConfig, type ResourceItem, type ResourceType, type State, type TeamaiConfig } from '../types.js'; import { namespaceOfPlacedPath, type PlaceableType } from '../push-namespaces.js'; import { ensureDir, fileContentEqual, pathExists } from '../utils/fs.js'; +import { hashObject } from '../utils/git.js'; export async function listRepoFilesRecursive(dir: string, baseDir = dir): Promise { try { @@ -26,6 +27,49 @@ export async function listRepoFilesRecursive(dir: string, baseDir = dir): Promis } } +/** + * The author's shared-root copy of a resource an earlier push placed elsewhere, + * judged against what that push delivered (`state.placements`). Unchanged is + * `'delivered'`, whatever the team did to the placed file since. An edit goes + * to the placed file, unless that file changed too: pushing would then + * overwrite the other change, so the scan stops. Null when no record applies. + */ +export async function placedCopy( + type: ResourceType, + name: string, + authorPath: string, + activeFile: string, + localConfig: LocalConfig, + placements: NonNullable, +): Promise { + const record = placements[`${type}:${authorPath}`]; + if (!record) return null; + const repoPath = localConfig.repo.localPath; + const placedFile = path.join(repoPath, ...record.path.split('/')); + if (!await pathExists(placedFile)) return null; + const [mine, theirs] = await Promise.all([hashObject(repoPath, activeFile), hashObject(repoPath, placedFile)]); + if (!mine || !theirs) { + throw new Error(`git could not read ${authorPath} or ${record.path} to compare them with what your push delivered`); + } + if (mine === record.blob) return 'delivered'; + if (theirs !== record.blob) { + throw new Error( + `${record.path} changed on the team repo after your ${authorPath} was placed there, and ${authorPath} ` + + `changed too, so pushing it would overwrite that change. Move your edit into ${record.path} and delete ${authorPath}`, + ); + } + const item: ResourceItem & { prePlacementPath: string } = { + name, + type, + sourcePath: activeFile, + relativePath: record.path, + namespace: namespaceOfPlacedPath(type as PlaceableType, record.path), + status: 'modified', + prePlacementPath: authorPath, + }; + return item; +} + export async function scanRepoResourceForPush( type: ResourceType, dir: string, @@ -53,21 +97,10 @@ export async function scanRepoResourceForPush( const atBaseline = await pathExists(baseFile); if (atBaseline && await fileContentEqual(activeFile, baseFile)) continue; if (!atBaseline) { - const placedPath = placements[`${type}:${relativePath}`]; - const placedFile = placedPath ? path.join(localConfig.repo.localPath, ...placedPath.split('/')) : null; - if (placedPath && placedFile && await pathExists(placedFile)) { - // Compare the copy the author edits against the file that landed in - // its place: unchanged means already delivered, and an edit belongs - // on that file rather than on a second copy at the shared root. - if (await fileContentEqual(activeFile, placedFile)) continue; - items.push({ - name: rel, - type, - sourcePath: activeFile, - relativePath: placedPath, - namespace: namespaceOfPlacedPath(type as PlaceableType, placedPath), - status: 'modified', - }); + const placed = await placedCopy(type, rel, relativePath, activeFile, localConfig, placements); + if (placed === 'delivered') continue; + if (placed) { + items.push(placed); continue; } } diff --git a/src/types.ts b/src/types.ts index 123733fa9..8b66f368f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1008,6 +1008,10 @@ export type RecordedAgentModel = z.infer; /** `RecordedAgentModel` by agent stem, then tool. */ export type AgentModelRecords = Record>; +/** A repo-backed placement that landed: the placed path and the blob push delivered there. */ +const PlacementRecordSchema = z.object({ path: z.string(), blob: z.string() }); +export type PlacementRecord = z.infer; + export const StateSchema = z.object({ lastPush: z.string().nullable().default(null), lastPull: z.string().nullable().default(null), @@ -1118,18 +1122,24 @@ export const StateSchema = z.object({ placedAgents: z.record(z.string(), z.string()).optional(), /** * Where push placed a repo-backed resource (docs, wiki, prompts, workflows, - * scorers, models, env, hooks, claudemd, learnings) that the author still - * edits at its shared-root path, keyed `:`, e.g. - * `{ "docs:docs/guide.md": "docs/ops/guide.md" }`. Placement writes the team - * copy under `//` and leaves the author's own file at the root, so - * without this record the next self-mode scan reads that file as brand new, - * sends it to the same destination, and fails on the copy that is already - * there — once the PR has merged, the destination is exactly the file the - * scan never looks at. Recorded only after the placement landed, in + * scorers, models, env, hooks, claudemd, learnings) that the author still edits at + * its shared-root path, keyed `:`, e.g. + * `{ "docs:docs/guide.md": { "path": "docs/ops/guide.md", "blob": "" } }`. + * `blob` is what that push delivered. A self-mode scan compares the author's + * copy with it, not with the placed file: equal means already delivered, + * whatever the team did to the placed file since, and different is the + * author's edit. Recorded only after the placement landed, in * `reconcilePlacementRecords`, on the same blob-in-history proof as - * `placedRules`, so a PR closed unmerged leaves no record behind. + * `placedRules`, so a PR closed unmerged leaves no record behind. An entry + * holding only the path, as earlier builds wrote, is dropped: without the + * blob nothing tells an unchanged copy from an edit. */ - placements: z.record(z.string(), z.string()).optional(), + placements: z.record(z.string(), z.unknown()) + .transform((records) => Object.fromEntries(Object.entries(records).flatMap(([key, record]) => { + const parsed = PlacementRecordSchema.safeParse(record); + return parsed.success ? [[key, parsed.data] as const] : []; + }))) + .optional(), /** * Default-branch commit the placement records were last checked against. A * record whose file was deleted after it is dropped even if something is at diff --git a/src/utils/pending-push.ts b/src/utils/pending-push.ts index 697716b56..00bef8c76 100644 --- a/src/utils/pending-push.ts +++ b/src/utils/pending-push.ts @@ -261,12 +261,13 @@ export function isPlacement(item: ResourceItem): boolean { /** * Whether pushing `item` PLACED a repo-backed resource (docs, wiki, prompts, * workflows, ...): a new file the scan found at its shared-root path, which - * `pushItem` then wrote under `//`. Unlike a rule or agent, the author - * keeps editing the shared-root file, so the record has to name that path too - * (`sourcePath`) for a self-mode scan to compare against the destination. + * `pushItem` then wrote under `//`, or an edit of that file the scan + * reached through its record. Unlike a rule or agent, the author keeps editing + * the shared-root file, so the record has to name that path too (`sourcePath`) + * and what was delivered (`blob`) for a self-mode scan to compare against. */ function isRepoPlacement(item: ResourceItem): boolean { - if (item.status !== 'new' || !item.namespace) return false; + if (!item.namespace) return false; // Skills, rules and agents keep their own records (placedRules/placedAgents, // or a path derived from the skill name), so they are not keyed this way. if (item.type === 'skills' || item.type === 'rules' || item.type === 'agents') return false; @@ -407,7 +408,7 @@ export async function reconcilePlacementRecords( state[field] = { ...state[field], [item.name]: item.relativePath }; recordedNow.add(`${field}:${item.name}`); } else if (placementKey) { - state.placements = { ...state.placements, [placementKey]: item.relativePath }; + state.placements = { ...state.placements, [placementKey]: { path: item.relativePath, blob: item.blob } }; recordedNow.add(placementKey); } // Consumed: a placement is recorded once. Left marked, it would record @@ -469,11 +470,11 @@ export async function reconcilePlacementRecords( // resource again. Kept, it would compare the copy against a path nobody has // and hide a genuine edit. if (state.placements) { - const kept: Record = {}; + const kept: NonNullable = {}; let dropped = false; for (const [key, recorded] of Object.entries(state.placements)) { - if (!await exists(recorded)) { - log.debug(`Dropping placement record ${key} → ${recorded}: gone from the default branch`); + if (!await exists(recorded.path)) { + log.debug(`Dropping placement record ${key} → ${recorded.path}: gone from the default branch`); dropped = true; changed = true; continue; @@ -484,8 +485,8 @@ export async function reconcilePlacementRecords( // placedRules/placedAgents records below, and the same reason // `placementsCheckedAt` is kept. if (checkedAt && !recordedNow.has(key) - && await pathDeletedSince(repoPath, checkedAt, recorded, history) === true) { - log.debug(`Dropping placement record ${key} → ${recorded}: deleted from the default branch since the last check`); + && await pathDeletedSince(repoPath, checkedAt, recorded.path, history) === true) { + log.debug(`Dropping placement record ${key} → ${recorded.path}: deleted from the default branch since the last check`); dropped = true; changed = true; continue; From 87bb24bf2c76557de3ec6792341ca263f72fd735 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 22 Oct 2026 21:55:05 +0200 Subject: [PATCH 18/28] fix(push): judge a placed env file against what its push delivered The self-mode env scan compared an unchanged .teamai/env copy with the placed file as it is now. After a teammate fixed env/ops/env.yaml on the team repo, the author's old copy read as an edit and the next push carried it over the fix. Use the same placed-copy check as the other repo-backed scans, which compares against the blob the push delivered. --- .../push-all-resources-namespaces.test.ts | 13 ++++++++++++- src/resources/env.ts | 19 +++++++------------ 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/__tests__/push-all-resources-namespaces.test.ts b/src/__tests__/push-all-resources-namespaces.test.ts index 5e8bed74a..f5ad9c47d 100644 --- a/src/__tests__/push-all-resources-namespaces.test.ts +++ b/src/__tests__/push-all-resources-namespaces.test.ts @@ -346,7 +346,7 @@ describe('a placed copy the author still edits, against what its push delivered' env: { ...process.env, GIT_AUTHOR_NAME: 'T', GIT_AUTHOR_EMAIL: 't@t', GIT_COMMITTER_NAME: 'T', GIT_COMMITTER_EMAIL: 't@t' }, }).trim(); - async function placedCopy(type: 'docs', authorPath: string, placedPath: string) { + async function placedCopy(type: 'docs' | 'env', authorPath: string, placedPath: string) { const root = makeTmpDir(); const projectRoot = path.join(root, 'project'); const repoLocal = path.join(root, 'team-repo'); @@ -442,6 +442,17 @@ describe('a placed copy the author still edits, against what its push delivered' await expect(scan()).rejects.toThrow(/docs\/ops\/guide\.md/); }); + it('treats a placed env file the same way', async () => { + const { activeFile, placedFile, commit, scan } = await placedCopy('env', 'env/env.yaml', 'env/ops/env.yaml'); + expect(await scan()).toEqual([]); + + commit(placedFile, 'v2 (teammate fix)\n', 'teammate fixes the placed env file'); + expect(await scan()).toEqual([]); + + fs.writeFileSync(activeFile, 'v1 plus my edit\n'); + await expect(scan()).rejects.toThrow(/env\/ops\/env\.yaml/); + }); + it('reads a record that holds only the path, as earlier builds wrote, as no record', async () => { const { scan, localConfig } = await placedCopy('docs', 'docs/guide.md', 'docs/ops/guide.md'); const statePath = getStatePath(localConfig); diff --git a/src/resources/env.ts b/src/resources/env.ts index 6cff8b98c..d8b10ffc1 100644 --- a/src/resources/env.ts +++ b/src/resources/env.ts @@ -3,12 +3,12 @@ import { readFile, realpath } from 'node:fs/promises'; import { z } from 'zod'; import YAML from 'yaml'; import { ResourceHandler } from './base.js'; +import { placedCopy } from './repo-resource.js'; +import { loadStateForScope } from '../config.js'; import type { ResourceItem, ResourceItemStatus, TeamaiConfig, LocalConfig } from '../types.js'; import { TEAMAI_ENV_START, TEAMAI_ENV_END, getDataHome, getEnvBackupPath, getTeamaiHome, isSelfMode } from '../types.js'; import { pathExists, readFileIfExists, readFileSafe, writeFile, ensureDir, fileContentEqual, remove, symlinkTarget } from '../utils/fs.js'; import { log } from '../utils/logger.js'; -import { loadStateForScope } from '../config.js'; -import { namespaceOfPlacedPath } from '../push-namespaces.js'; import { envShExportsPath, envShMarker, isEnvShMarker, recordEnvShExports } from '../env-sh-exports.js'; import { ENV_KEY_RE, envNamePattern, envNamePatternKey, envTable, sameEnvName } from './env-key.js'; import { SECRETS_LAYOUT } from './secrets.js'; @@ -352,16 +352,11 @@ export class EnvHandler extends ResourceHandler { const baseExists = await pathExists(baseEnv); if (baseExists && await fileContentEqual(activeEnv, baseEnv)) continue; if (!baseExists) { - const placedPath = placements[`env:${relativePath}`]; - const placedFile = placedPath ? path.join(localConfig.repo.localPath, ...placedPath.split('/')) : null; - if (placedPath && placedFile && await pathExists(placedFile)) { - // Unchanged: already delivered. Edited: the edit belongs on the - // file that landed in its place, not on a second copy at the root. - if (await fileContentEqual(activeEnv, placedFile)) continue; - items.push({ - ...envPushItem(placedPath, activeEnv, 'modified'), - namespace: namespaceOfPlacedPath('env', placedPath), - }); + // An earlier push may have placed this file under a namespace. + const placed = await placedCopy('env', relativePath.slice('env/'.length), relativePath, activeEnv, localConfig, placements); + if (placed === 'delivered') continue; + if (placed) { + items.push(placed); continue; } } From 4c1d3bdbfecffa7cffb000f8f4e1e86882a01fe9 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 22 Oct 2026 22:04:49 +0200 Subject: [PATCH 19/28] fix(push): require --role to name a declared namespace --role was taken as a literal namespace on every axis. In a team with manifest/roles.yaml or manifest/projects.yaml, a namespace no role or project declares for a type is one pull delivers to nobody, yet push wrote rules//, docs//, workflows// and the rest there after a warning that checked roles.yaml only. Resolve --role per type against the namespaces both manifests declare for that type's axis, the way --project resolves against its project, and stop with exit 2 otherwise. A team with neither manifest still takes any namespace. The push-role unit fixtures now declare the namespaces their tests pass. --- docs/guide/sharing.md | 2 +- docs/guide/zh-CN/sharing.md | 2 +- skill-data/core/references/commands.md | 2 +- .../e2e/push-active-namespaces.test.ts | 34 +++++++ src/__tests__/push-role.test.ts | 9 +- src/index.ts | 1 + src/push-namespaces.ts | 26 ++++- src/push.ts | 97 ++++++++++--------- 8 files changed, 120 insertions(+), 53 deletions(-) diff --git a/docs/guide/sharing.md b/docs/guide/sharing.md index 62416eedd..53aaba5c9 100644 --- a/docs/guide/sharing.md +++ b/docs/guide/sharing.md @@ -23,7 +23,7 @@ teamai push teamai push --role pm ``` -`push` fills in `name` and `description` when the frontmatter is missing. A new skill goes to `skills///`, where `` comes from your role or the active projects. Several choices ask which one. `--role` picks one. +`push` fills in `name` and `description` when the frontmatter is missing. A new skill goes to `skills///`, where `` comes from your role or the active projects. Several choices ask which one. `--role` picks one. When the team has `manifest/roles.yaml` or `manifest/projects.yaml`, a role or project there must declare it. ## Rules diff --git a/docs/guide/zh-CN/sharing.md b/docs/guide/zh-CN/sharing.md index a95f2d34f..b4dd5ff71 100644 --- a/docs/guide/zh-CN/sharing.md +++ b/docs/guide/zh-CN/sharing.md @@ -23,7 +23,7 @@ teamai push teamai push --role pm ``` -没有 frontmatter 时，`push` 会补上 `name` 和 `description`。新 skill 写到你的角色或当前项目给出的 `skills///`。有多个时会问。`--role` 指定一个。 +没有 frontmatter 时，`push` 会补上 `name` 和 `description`。新 skill 写到你的角色或当前项目给出的 `skills///`。有多个时会问。`--role` 指定一个。团队有 `manifest/roles.yaml` 或 `manifest/projects.yaml` 时，它必须由其中某个角色或项目声明。 ## Rules diff --git a/skill-data/core/references/commands.md b/skill-data/core/references/commands.md index 0bbbd79df..1af6d1e1e 100644 --- a/skill-data/core/references/commands.md +++ b/skill-data/core/references/commands.md @@ -36,7 +36,7 @@ Generated: do not edit by hand. Regenerate with - `teamai push` — Push local resources to team repo - `--all` — Push all without confirmation - `--skill ` — Push a specific skill by path (e.g., ~/.claude/skills/hai/my-skill or skills/hai_dev/my-skill) - - `--role ` — Namespace for every new resource type (skills//, rules//, docs//, env//, etc.). Without --role or --project, the namespaces of your role and active projects decide + - `--role ` — Namespace for every new resource type (skills//, rules//, docs//, env//, etc.). When the team has manifest/roles.yaml or manifest/projects.yaml, a role or project there must declare it for each type it places. Without --role or --project, the namespaces of your role and active projects decide - `--project ` — Target a project: each new resource goes to that project's namespace for its own type — skills, knowledge for rules, agents, learnings, docs, env, hooks, mcp, models, and more (from manifest/projects.yaml) - `--branch ` — Push to this destination branch instead of a generated teamai/push branch diff --git a/src/__tests__/e2e/push-active-namespaces.test.ts b/src/__tests__/e2e/push-active-namespaces.test.ts index bd2fd912b..3bd14d99b 100644 --- a/src/__tests__/e2e/push-active-namespaces.test.ts +++ b/src/__tests__/e2e/push-active-namespaces.test.ts @@ -1095,6 +1095,40 @@ describe('push places a new resource by the active projects (#1021)', () => { }); }); +describe('push --role places only where a role or project declares the namespace for that type (#1101)', () => { + it('pushes nothing for a namespace no role or project declares', () => { + writeFile(path.join(project, '.claude', 'rules', 'new-rule.md'), '# new rule\n'); + + const pushed = run(['push', '--all', '--role', 'rogue']); + + expect(pushed.code, pushed.output).toBe(2); + expect(pushed.output).toContain('--role rogue'); + expect(pushedFiles()).toEqual([]); + }); + + it('pushes nothing for a new skill under a namespace no role or project declares', () => { + writeFile(path.join(project, '.claude', 'skills', 'new-skill', 'SKILL.md'), skillMd('new-skill')); + + const pushed = run(['push', '--all', '--role', 'rogue']); + + expect(pushed.code, pushed.output).toBe(2); + expect(pushed.output).toContain('--role rogue'); + expect(pushedFiles()).toEqual([]); + }); + + it('pushes nothing for a namespace declared for other types only', () => { + // platform declares skills, knowledge and learnings, not docs. + writeFile(path.join(project, '.teamai', 'team-repo', 'docs', 'guide.md'), '# guide\n'); + + const pushed = run(['push', '--all', '--role', 'platform']); + + expect(pushed.code, pushed.output).toBe(2); + expect(pushed.output).toContain('--role platform'); + expect(pushed.output).toContain('docs'); + expect(pushedFiles()).toEqual([]); + }); +}); + describe('push stops when the active projects cannot be resolved', () => { const unresolved: [string, () => void, string][] = [ ['the projects manifest is gone', () => commitOnTeam('manifest/projects.yaml', null), 'no manifest/projects.yaml'], diff --git a/src/__tests__/push-role.test.ts b/src/__tests__/push-role.test.ts index 758e69b35..757dd4678 100644 --- a/src/__tests__/push-role.test.ts +++ b/src/__tests__/push-role.test.ts @@ -235,12 +235,14 @@ describe('push namespace routing', () => { availableUpdate: null, }); mockSaveStateForScope.mockResolvedValue(undefined); - // Default manifest: role "hai" has namespaces [common, hai] + // Default manifest: role "hai" has namespaces [common, hai]. Every + // namespace a test names with --role is one a role declares (#1101). mockLoadRolesManifest.mockResolvedValue({ version: 1, roles: [ { id: 'hai', description: 'HyperAI', resources: { knowledge: ['common', 'hai'], skills: ['common', 'hai'], agents: [] } }, { id: 'pm', description: 'Product Manager', resources: { knowledge: ['common', 'pm'], skills: ['common', 'pm'], agents: [] } }, + { id: 'backend', description: 'Backend', resources: { knowledge: [], skills: ['backend'], agents: [] } }, ], }); readlineAnswer = '1'; @@ -526,8 +528,7 @@ describe('push namespace routing', () => { pushItem: vi.fn(), }); - // --role "unknown" → used directly as namespace, no manifest validation - // (validation happens downstream in pushItem) + // --role "unknown" names no declared namespace, but nothing here needs placing. await push({ all: true, role: 'unknown' }); // No items to push, so pushRepoBranch should not be called @@ -1046,6 +1047,8 @@ describe('push namespace routing for rules and agents', () => { version: 1, roles: [ { id: 'hai', description: 'HyperAI', resources: { knowledge: ['common', 'hai'], skills: ['common', 'hai'], agents: [] } }, + // The namespaces the tests below name with --role (#1101). + { id: 'pm', description: 'Product Manager', resources: { knowledge: ['pm', 'fe-know'], skills: ['pm'], agents: ['pm', 'be-agents'] } }, ], }); readlineAnswer = '1'; diff --git a/src/index.ts b/src/index.ts index 0d5b5b8aa..de2f114c9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -191,6 +191,7 @@ program .option('--all', 'Push all without confirmation') .option('--skill ', 'Push a specific skill by path (e.g., ~/.claude/skills/hai/my-skill or skills/hai_dev/my-skill)') .option('--role ', 'Namespace for every new resource type (skills//, rules//, docs//, env//, etc.). ' + + 'When the team has manifest/roles.yaml or manifest/projects.yaml, a role or project there must declare it for each type it places. ' + 'Without --role or --project, the namespaces of your role and active projects decide') .option('--project ', "Target a project: each new resource goes to that project's namespace for its own type " + '— skills, knowledge for rules, agents, learnings, docs, env, hooks, mcp, models, and more (from manifest/projects.yaml)') diff --git a/src/push-namespaces.ts b/src/push-namespaces.ts index b9e1365d1..077140c2b 100644 --- a/src/push-namespaces.ts +++ b/src/push-namespaces.ts @@ -12,7 +12,7 @@ * reached the whole team (issue #649). It lives here so every type answers the * same question the same way, and so the answer can be tested without a repo. */ -import { isSafeNamespaceSegment, NAMESPACE_RULE } from './manifest-schema.js'; +import { isSafeNamespaceSegment, NAMESPACE_RULE, type NamespaceEntry } from './manifest-schema.js'; import { findProject, unknownProjectMessage, type AllProjectResourceType, type ProjectsManifest, @@ -169,6 +169,30 @@ export type NamespaceResolution = | { ok: true; namespace: string } | { ok: false; message: string }; +/** + * The namespace `--role ` places `type` in. A team with a roles or projects + * manifest delivers a namespace only to the members of a role or project that + * declares it for the type's axis, so any other namespace would reach nobody; + * `declared` is every namespace those manifests declare. A team with neither + * manifest (`declared` null) delivers every namespace, so any one goes. + */ +export function resolveRoleNamespace( + declared: readonly NamespaceEntry[] | null, + namespace: string, + type: PlaceableType, +): NamespaceResolution { + if (declared === null) return { ok: true, namespace }; + const axis = NAMESPACE_AXIS[type]; + const onAxis = [...new Set(declared.filter((entry) => entry.type === axis).map((entry) => entry.namespace))]; + if (onAxis.includes(namespace)) return { ok: true, namespace }; + return { + ok: false, + message: `--role ${namespace} cannot take new ${type}: no role or project declares "${namespace}" as a ${axis} ` + + 'namespace in manifest/roles.yaml or manifest/projects.yaml, so pull would deliver it to nobody. ' + + (onAxis.length > 0 ? `Pass one that is declared (${onAxis.join(', ')}), or declare it.` : `Declare it for ${axis} first.`), + }; +} + /** * The namespace `--project ` places `type` in, read from that type's own * axis in `manifest/projects.yaml`. diff --git a/src/push.ts b/src/push.ts index a156d2528..157956720 100644 --- a/src/push.ts +++ b/src/push.ts @@ -25,12 +25,12 @@ import type { import { getDataHome, SELF_KNOWLEDGE_SCAN_KEY, SYNC_LOCK_FILENAME } from './types.js'; import { acquireLock, releaseLock } from './update.js'; import { assertSafeResourceName, resolveReal } from './utils/path-safety.js'; -import { loadRolesManifest, resolveRoleResourceNamespaces, RolesManifestNotFoundError, type ResourceNamespaces } from './roles.js'; -import { loadProjectsManifest, resolveProjectResourceNamespaces, type ProjectsManifest } from './projects.js'; -import { isSafeNamespaceSegment, NAMESPACE_RULE, fallbackNamespaceError } from './manifest-schema.js'; +import { loadRolesManifest, resolveRoleResourceNamespaces, roleNamespaceEntries, RolesManifestNotFoundError, type ResourceNamespaces } from './roles.js'; +import { loadProjectsManifest, projectNamespaceEntries, resolveProjectResourceNamespaces, type ProjectsManifest } from './projects.js'; +import { isSafeNamespaceSegment, NAMESPACE_RULE, fallbackNamespaceError, type NamespaceEntry } from './manifest-schema.js'; import { declaredNamespaceFor, isAtSharedRoot, isNestedPlaceableType, isPlaceableType, NAMESPACE_AXIS, PLACEABLE_TYPES, placedResourcePath, - resolveProjectNamespace, rootSegmentsFor, skillNamespacePath, withNamespace, type PlaceableType, + resolveProjectNamespace, resolveRoleNamespace, rootSegmentsFor, skillNamespacePath, withNamespace, type PlaceableType, } from './push-namespaces.js'; import { askQuestion, askSelection, isInteractive } from './utils/prompt.js'; import { pathExists, pruneEmptyDirs, readFileSafe, writeFile } from './utils/fs.js'; @@ -238,18 +238,17 @@ async function resolveNamespaceForNew( options: { role?: string; project?: string; silent?: boolean }, localConfig: LocalConfig, projectsManifest: ProjectsManifest | null, + roleNamespaces: readonly NamespaceEntry[] | null, ): Promise { - if (options.project && projectsManifest) { - const resolved = resolveProjectNamespace(projectsManifest, options.project, type); - return resolved.ok - ? { kind: 'namespace', namespace: resolved.namespace } - : { kind: 'unresolvable', message: resolved.message }; + const flagged = options.project && projectsManifest + ? resolveProjectNamespace(projectsManifest, options.project, type) + : options.role ? resolveRoleNamespace(roleNamespaces, options.role, type) : null; + if (flagged) { + return flagged.ok + ? { kind: 'namespace', namespace: flagged.namespace } + : { kind: 'unresolvable', message: flagged.message }; } - // An explicit --role is a literal namespace on every axis (already checked - // for path traversal before selection). - if (options.role) return { kind: 'namespace', namespace: options.role }; - const resolution = await namespaceCandidates(type, localConfig); if (!resolution.ok) return { kind: 'unresolvable', message: resolution.message }; const { candidates } = resolution; @@ -586,13 +585,15 @@ async function placeNewResources(args: { options: { role?: string; project?: string; silent?: boolean }; localConfig: LocalConfig; projectsManifest: ProjectsManifest | null; + /** What `--role` may name: every namespace the manifests declare, null with neither manifest. */ + roleNamespaces: readonly NamespaceEntry[] | null; skillsDestinationError?: string; /** The clone could not be refreshed this run; its manifests may be stale. */ teamRepoStale?: boolean; /** Resources whose open PR already decided where they go. */ settled: ReadonlySet; }): Promise { - const { items, options, localConfig, projectsManifest, skillsDestinationError, teamRepoStale, settled } = args; + const { items, options, localConfig, projectsManifest, roleNamespaces, skillsDestinationError, teamRepoStale, settled } = args; // A project that declares no skills namespace only blocks the push once a // skill the flag relocates is actually selected, so a rule can still go out @@ -625,7 +626,7 @@ async function placeNewResources(args: { return false; } - const destination = await resolveNamespaceForNew(type, options, localConfig, projectsManifest); + const destination = await resolveNamespaceForNew(type, options, localConfig, projectsManifest, roleNamespaces); switch (destination.kind) { case 'unresolvable': log.error(destination.message); @@ -1267,6 +1268,27 @@ async function pushCore( } } + // --role names a namespace directly, so check it against what the manifests + // declare, as --project is: a namespace no role or project declares for a + // type is one pull delivers to nobody. + let roleNamespaces: NamespaceEntry[] | null = null; + if (options.role) { + try { + const roles = await loadRolesManifest(localConfig.repo.localPath).catch((e: unknown) => { + if (e instanceof RolesManifestNotFoundError) return null; + throw e; + }); + const projects = await loadProjectsManifest(localConfig.repo.localPath); + roleNamespaces = roles || projects + ? [...roles ? roleNamespaceEntries(roles) : [], ...projects ? projectNamespaceEntries(projects) : []] + : null; + } catch (e) { + log.error(`Cannot check where --role ${options.role} may place resources: ${(e as Error).message}`); + process.exitCode = 2; + return; + } + } + // Every path below reads the active projects' namespaces: the skills scan to // tell this directory's team skills from another project's of the same name, // `--skill`, and the placement of a new rule, skill or agent. Without them @@ -1414,15 +1436,13 @@ async function pushCore( // no candidate here and is rejected with exit 2 before anything is pushed. let requestedAgentsNamespace: string | undefined; let agentsDestinationError: string | undefined; - if (options.role) { - requestedAgentsNamespace = options.role; - } else if (options.project && projectsManifest) { - const resolved = resolveProjectNamespace(projectsManifest, options.project, 'agents'); - if (resolved.ok) { - requestedAgentsNamespace = resolved.namespace; - } else { - agentsDestinationError = resolved.message; - } + const flaggedAgents = options.project && projectsManifest + ? resolveProjectNamespace(projectsManifest, options.project, 'agents') + : options.role ? resolveRoleNamespace(roleNamespaces, options.role, 'agents') : null; + if (flaggedAgents?.ok) { + requestedAgentsNamespace = flaggedAgents.namespace; + } else if (flaggedAgents) { + agentsDestinationError = flaggedAgents.message; } // The skills namespace the flags name: it picks among a name's several open PRs. let requestedSkillsNamespace: string | undefined; @@ -1718,27 +1738,12 @@ async function pushCore( process.exitCode = 2; return; } - // Warn when the role names no namespace any manifest declares: pull would - // ignore the pushed paths. A manifest-free team is exempt — every on-disk - // namespace is valid there. - try { - const manifest = await loadRolesManifest(localConfig.repo.localPath); - const declared = new Set(); - for (const role of manifest.roles) { - for (const list of Object.values(role.resources) as (string[] | undefined)[]) { - for (const ns of list ?? []) declared.add(ns); - } - } - if (declared.size > 0 && !declared.has(options.role)) { - log.warn( - `--role ${options.role} is not declared in manifest/roles.yaml for any resource axis; ` - + 'pull will not deliver resources placed there. Declare it or pick a known namespace.', - ); - } - } catch { - // Manifest missing or unreadable: the push path reports that separately. + const resolved = resolveRoleNamespace(roleNamespaces, options.role, 'skills'); + if (resolved.ok) { + skillsDestination = resolved.namespace; + } else { + skillsDestinationError = resolved.message; } - skillsDestination = options.role; } else if (options.project && projectsManifest && allItems.some(isRelocatedByFlag)) { const resolved = resolveProjectNamespace(projectsManifest, options.project, 'skills'); if (resolved.ok) { @@ -1950,7 +1955,7 @@ async function pushCore( // destination first, then placement for whatever is still at the root. const settled = reuseRecordedDestinations(planPushGroups(allItems, reusablePending)); const placed = await placeNewResources({ - items: allItems, options, localConfig, projectsManifest, skillsDestinationError, teamRepoStale, settled, + items: allItems, options, localConfig, projectsManifest, roleNamespaces, skillsDestinationError, teamRepoStale, settled, }); if (!placed) return; log.info('Dry run — no changes made'); @@ -1992,7 +1997,7 @@ async function pushCore( // ── Step 4: Place NEW root-level resources in a namespace (after selection) ─ if (!await placeNewResources({ - items: selectedItems, options, localConfig, projectsManifest, skillsDestinationError, teamRepoStale, settled, + items: selectedItems, options, localConfig, projectsManifest, roleNamespaces, skillsDestinationError, teamRepoStale, settled, })) return; // ── Step 5: Push each group — one branch/PR per group ────────────── From f683be13b691454f06571fe336120bd1971a67b5 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 22 Oct 2026 22:11:30 +0200 Subject: [PATCH 20/28] fix(contribute): use the learnings namespaces of the member's roles Roles now declare learnings namespaces, and pull delivers learnings// to the role's members, but contribute still resolved learnings from active projects only. A role-only member's contribution went to the shared root, and --namespace rejected the role's own namespace. Resolve the active learnings namespaces from roles and projects, in the order pull merges them, in the one function contribute, recall, import and `projects list` read. --- docs/guide/knowledge.md | 2 +- docs/guide/zh-CN/knowledge.md | 2 +- skill-data/setup/references/manage-admin.md | 2 +- src/__tests__/contribute.test.ts | 35 ++++++++++++ src/__tests__/projects.test.ts | 8 +-- src/contribute.ts | 12 +--- src/projects-cmd.ts | 2 +- src/projects.ts | 61 +++++++++++++-------- src/workspace-children.ts | 2 +- 9 files changed, 86 insertions(+), 40 deletions(-) diff --git a/docs/guide/knowledge.md b/docs/guide/knowledge.md index c1191ee61..856908fdf 100644 --- a/docs/guide/knowledge.md +++ b/docs/guide/knowledge.md @@ -25,7 +25,7 @@ teamai contribute --file /tmp/session.md teamai contribute --file /tmp/session.md --namespace payments ``` -`--namespace` must be a learnings namespace this scope declares in `manifest/projects.yaml`. +`--namespace` must be a learnings namespace this scope's projects declare in `manifest/projects.yaml`, or its roles in `manifest/roles.yaml`. To stop the prompt, set this in `teamai.yaml`: diff --git a/docs/guide/zh-CN/knowledge.md b/docs/guide/zh-CN/knowledge.md index f39b349b0..7b3203b70 100644 --- a/docs/guide/zh-CN/knowledge.md +++ b/docs/guide/zh-CN/knowledge.md @@ -25,7 +25,7 @@ teamai contribute --file /tmp/session.md teamai contribute --file /tmp/session.md --namespace payments ``` -`--namespace` 必须是当前 scope 在 `manifest/projects.yaml` 里声明的 learnings namespace。 +`--namespace` 必须是当前 scope 的项目在 `manifest/projects.yaml` 里、或其角色在 `manifest/roles.yaml` 里声明的 learnings namespace。 不想要这条提醒时，在 `teamai.yaml` 里写： diff --git a/skill-data/setup/references/manage-admin.md b/skill-data/setup/references/manage-admin.md index 6df9d2fcd..4405e73a5 100644 --- a/skill-data/setup/references/manage-admin.md +++ b/skill-data/setup/references/manage-admin.md @@ -162,7 +162,7 @@ for the real reset fallback. Removal refuses when that refresh also fails. Every namespace that names a directory — `knowledge`, `skills`, `agents`, `env`, `hooks`, `mcp`, `models`, `docs` and `wiki` in either manifest, and `learnings` in `projects.yaml` (a role's `learnings:` is -ignored and unchecked) — must be a single path segment: no `/`, `\`, `:` or control character, no trailing +not checked) — must be a single path segment: no `/`, `\`, `:` or control character, no trailing `.` or space, and not a Windows device name (`CON`, `NUL`, `COM1`, …). `team-codebase` cannot be a `docs` namespace (`docs/team-codebase/` is the legacy codebase output). Two namespaces of one resource type may not differ only by case, across both diff --git a/src/__tests__/contribute.test.ts b/src/__tests__/contribute.test.ts index b8e3a1690..81e91f5f8 100644 --- a/src/__tests__/contribute.test.ts +++ b/src/__tests__/contribute.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import fs from 'node:fs'; import path from 'node:path'; import os from 'node:os'; +import type { LocalConfig } from '../types.js'; // ─── contribute.ts tests ──────────────────────────────────── // These test the contribute() function in isolation, @@ -111,3 +112,37 @@ describe('contribute', () => { vi.doUnmock('../config.js'); }); }); + +describe('the learnings namespace of a member whose role declares one (#1101)', () => { + it('is where a contribution lands and one --namespace accepts, as pull delivers it to that role', async () => { + const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-contribute-role-')); + fs.mkdirSync(path.join(repo, 'manifest'), { recursive: true }); + fs.writeFileSync(path.join(repo, 'manifest', 'roles.yaml'), [ + 'version: 1', + 'roles:', + ' - id: ops', + ' description: ops', + ' resources: { knowledge: [], skills: [], agents: [], learnings: [ops-notes] }', + '', + ].join('\n')); + const localConfig: LocalConfig = { + repo: { localPath: repo, remote: 'local' }, + username: 'alice', + scope: 'project', + primaryRole: 'ops', + additionalRoles: [], + projects: [], + }; + try { + const { resolveLearningsSubdir } = await import('../contribute.js'); + const { deliveredLearningsNamespaces } = await import('../workspace-children.js'); + const { resolveResourceNamespaces } = await import('../resource-namespaces.js'); + + expect((await resolveResourceNamespaces(localConfig))?.activeNamespaces.learnings).toEqual(['ops-notes']); + expect(await resolveLearningsSubdir(localConfig)).toBe('ops-notes'); + expect(await deliveredLearningsNamespaces(localConfig)).toEqual(['ops-notes']); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + } + }); +}); diff --git a/src/__tests__/projects.test.ts b/src/__tests__/projects.test.ts index 7e20fdbe3..bf89d82d1 100644 --- a/src/__tests__/projects.test.ts +++ b/src/__tests__/projects.test.ts @@ -517,7 +517,7 @@ projects: learnings: [alpha-notes] `); try { - expect(await resolveActiveLearningsNamespaces(repoDir, ['alpha'])).toEqual(['alpha-notes']); + expect(await resolveActiveLearningsNamespaces({ repo: { localPath: repoDir }, projects: ['alpha'] })).toEqual(['alpha-notes']); } finally { rmSync(repoDir, { recursive: true, force: true }); } @@ -533,9 +533,9 @@ projects: skills: [beta] `); try { - expect(await resolveActiveLearningsNamespaces(noManifest, ['x'])).toEqual([]); - expect(await resolveActiveLearningsNamespaces(noLearnings, [])).toEqual([]); - expect(await resolveActiveLearningsNamespaces(noLearnings, ['beta'])).toEqual([]); + expect(await resolveActiveLearningsNamespaces({ repo: { localPath: noManifest }, projects: ['x'] })).toEqual([]); + expect(await resolveActiveLearningsNamespaces({ repo: { localPath: noLearnings }, projects: [] })).toEqual([]); + expect(await resolveActiveLearningsNamespaces({ repo: { localPath: noLearnings }, projects: ['beta'] })).toEqual([]); } finally { rmSync(noManifest, { recursive: true, force: true }); rmSync(noLearnings, { recursive: true, force: true }); diff --git a/src/contribute.ts b/src/contribute.ts index d42650956..6c84ffa28 100644 --- a/src/contribute.ts +++ b/src/contribute.ts @@ -22,17 +22,14 @@ import { getProjectSearchIndexPath, isSelfMode, parseInstallScope } from './type * the manifest. * * - Exactly one active learnings namespace → that namespace's subdir (isolated). - * - Zero (no project, or the active projects declare no learnings namespace) → + * - Zero (no role or active project declares a learnings namespace) → * the shared root (empty string). * - Multiple active learnings namespaces → the shared root, because the * contribution's ownership is ambiguous. `contribute --namespace` lets the * member choose one of these namespaces without changing this default. */ export async function resolveLearningsSubdir(localConfig: LocalConfig): Promise { - const namespaces = await resolveActiveLearningsNamespaces( - localConfig.repo.localPath, - localConfig.projects ?? [], - ); + const namespaces = await resolveActiveLearningsNamespaces(localConfig); 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 @@ -195,10 +192,7 @@ export async function contribute( const filename = generateFilename(options.title); // The own selection picks the default; --namespace accepts any namespace this // scope reads, a workspace's children's included (#913). - const ownNamespaces = await resolveActiveLearningsNamespaces( - localConfig.repo.localPath, - localConfig.projects ?? [], - ); + const ownNamespaces = await resolveActiveLearningsNamespaces(localConfig); const namespaces = await deliveredLearningsNamespaces(localConfig); // Validate before saving the queue, creating worktrees or rebuilding indexes. if (options.namespace !== undefined && diff --git a/src/projects-cmd.ts b/src/projects-cmd.ts index 04653c359..622c3b576 100644 --- a/src/projects-cmd.ts +++ b/src/projects-cmd.ts @@ -47,7 +47,7 @@ export async function projectsList(_options: GlobalOptions): Promise { const manifest = await loadProjectsManifest(repoPath); // As contribute: the own selection picks the default, and a workspace accepts // its children's namespaces too (#913). - const ownNamespaces = await resolveActiveLearningsNamespaces(repoPath, localConfig.projects ?? []); + const ownNamespaces = await resolveActiveLearningsNamespaces(localConfig); const namespaces = await deliveredLearningsNamespaces(localConfig); const destination = ownNamespaces.length === 1 ? `learnings/${ownNamespaces[0]}/` : 'learnings/ (shared root)'; const contributionTarget = `Contributes to: ${destination} --namespace accepts: ${namespaces.join(', ') || '(none)'}`; diff --git a/src/projects.ts b/src/projects.ts index e09f65d8f..48a61fbe9 100644 --- a/src/projects.ts +++ b/src/projects.ts @@ -6,7 +6,8 @@ import { NamespaceSegmentSchema, parseManifest, readManifestFile, assertNoCaseAliasedNamespaces, warnUnknownResourceKeys, HAND_DECLARED_RESOURCE_TYPES, HandDeclaredNamespacesShape, type NamespaceEntry, parseManifestOnce, } from './manifest-schema.js'; -import type { ResourceNamespaces } from './roles.js'; +import { loadRolesManifest, resolveRoleResourceNamespaces, type ResourceNamespaces } from './roles.js'; +import type { LocalConfig } from './types.js'; /** * Project resource types. Unlike roles, `learnings` is an ACTIVE dimension here: @@ -233,30 +234,47 @@ export function resolveProjectResourceNamespaces(input: { } /** - * Resolve the active **learnings** namespaces for a directory, from the manifest - * — the SAME source `pull` uses. This is the canonical mapping from active - * project ids to learnings subdirectories: a project's learnings namespace is - * `resources.learnings`, which the schema allows to differ from the project id - * (e.g. project `alpha` → `learnings: [alpha-notes]`). `contribute` must route - * and index through this, not through the raw project id, or its landing point - * and post-contribute index diverge from what `pull` syncs. + * Resolve the active **learnings** namespaces for a directory, from the + * manifests — the SAME sources `pull` uses: the member's roles first, then the + * active projects, as `mergeNamespaces` orders them. This is the canonical + * mapping to learnings subdirectories: a learnings namespace is + * `resources.learnings`, which the schema allows to differ from the role or + * project id (e.g. project `alpha` → `learnings: [alpha-notes]`). `contribute` + * must route and index through this, not through the raw id, or its landing + * point and post-contribute index diverge from what `pull` syncs. * - * Returns `[]` when there is no manifest, no active project, or the active - * projects declare no learnings namespace (→ contribution lands at the shared root). + * Returns `[]` when there is no manifest, no role or active project, or none of + * them declares a learnings namespace (→ contribution lands at the shared root). */ export async function resolveActiveLearningsNamespaces( - repoPath: string, - activeProjects: string[], + config: { repo: Pick } & Partial>, ): Promise { - if (activeProjects.length === 0) return []; - const manifest = await loadProjectsManifest(repoPath); - if (!manifest) return []; - try { - return resolveProjectResourceNamespaces({ manifest, activeProjects }).learnings; - } catch { - // Unknown active project id, etc. — degrade to shared root rather than throw. - return []; - } + const repoPath = config.repo.localPath; + const activeProjects = config.projects ?? []; + // An unknown role or project id, or a manifest that does not parse, degrades + // to the shared root rather than throw. + const fromRoles = async (): Promise => { + if (!config.primaryRole) return []; + try { + const manifest = await loadRolesManifest(repoPath); + return resolveRoleResourceNamespaces({ + manifest, primaryRole: config.primaryRole, additionalRoles: config.additionalRoles ?? [], + }).learnings; + } catch { + return []; + } + }; + const fromProjects = async (): Promise => { + if (activeProjects.length === 0) return []; + const manifest = await loadProjectsManifest(repoPath); + if (!manifest) return []; + try { + return resolveProjectResourceNamespaces({ manifest, activeProjects }).learnings; + } catch { + return []; + } + }; + return [...new Set([...await fromRoles(), ...await fromProjects()])]; } /** @@ -282,7 +300,6 @@ export function mergeNamespaces( const merged: ResourceNamespaces = { knowledge: dedupe(roleNamespaces.knowledge, projectNamespaces.knowledge), skills: dedupe(roleNamespaces.skills, projectNamespaces.skills), - // Roles never contribute learnings; this is effectively the project set. learnings: dedupe(roleNamespaces.learnings, projectNamespaces.learnings), agents: dedupe(roleNamespaces.agents, projectNamespaces.agents), }; diff --git a/src/workspace-children.ts b/src/workspace-children.ts index 28f4bf5f0..8389ee8d9 100644 --- a/src/workspace-children.ts +++ b/src/workspace-children.ts @@ -163,7 +163,7 @@ export async function deliveredMembers(config: LocalConfig): Promise { const namespaces = new Set(); for (const member of await deliveredMembers(config) ?? [config]) { - for (const namespace of await resolveActiveLearningsNamespaces(config.repo.localPath, member.projects ?? [])) { + for (const namespace of await resolveActiveLearningsNamespaces({ ...member, repo: config.repo })) { namespaces.add(namespace); } } From 634e6eeeeffb4248f09f94fb8eaa4bdf2755988a Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 22 Oct 2026 22:49:21 +0200 Subject: [PATCH 21/28] fix(push): let a scan error before the refresh reach the pull failure path The pre-refresh scan swallowed a handler's error and treated that type as having no changes, so the clone was refreshed without knowing what the type held. Let the error propagate to the same catch the env scan uses. --- src/__tests__/push-env.test.ts | 11 +++++++++++ src/push.ts | 8 +------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/__tests__/push-env.test.ts b/src/__tests__/push-env.test.ts index e6d113dcb..4b28caeed 100644 --- a/src/__tests__/push-env.test.ts +++ b/src/__tests__/push-env.test.ts @@ -188,6 +188,17 @@ describe('push publishes the env files env add leaves in a standalone clone (#88 expect(fs.readFileSync(path.join(teamRepo, 'env', 'env.yaml'), 'utf8')).toContain('value: changed'); }); + it('reports a scan that fails before the refresh as a failed pull, not as a refresh that succeeded', async () => { + const { push } = await import('../push.js'); + const { getHandler } = await import('../resources/index.js'); + // Fails only the pre-refresh scan, the first call, so a later scan cannot report it. + vi.spyOn(getHandler('docs'), 'scanLocalForPush').mockRejectedValueOnce(new Error('docs scan exploded')); + + await push({ all: true }); + + expect(stderrOutput() + errorOutput()).toContain('Pull failed: docs scan exploded'); + }); + it('still refuses when another path is dirty, and keeps the env edit', async () => { const { envAdd } = await import('../env-commands.js'); const { push } = await import('../push.js'); diff --git a/src/push.ts b/src/push.ts index 157956720..2be6b43c5 100644 --- a/src/push.ts +++ b/src/push.ts @@ -1128,13 +1128,7 @@ async function pushCore( const workingContent = await readFileSafe(yamlPath); // Nothing below writes before these reads finish, so they run together: const repoScan = Promise.all( - PLACEABLE_TYPES.filter((t) => !LOCAL_TOOL_TYPES.has(t)).map(async (t) => { - try { - return await getHandler(t).scanLocalForPush(teamConfig, localConfig); - } catch { - return []; - } - }), + PLACEABLE_TYPES.filter((t) => !LOCAL_TOOL_TYPES.has(t)).map((t) => getHandler(t).scanLocalForPush(teamConfig, localConfig)), ); repoScan.catch(() => {}); const [committed, status] = await Promise.all([ From ba895487c017f119ce560ee3693b6621c303957b Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 22 Oct 2026 22:49:40 +0200 Subject: [PATCH 22/28] fix(push): say to merge a conflicting edit into the placed file The conflict message read as if the author's edit should replace the team's change in the placed file. Say the edit is merged into that file, keeping the team's change, before the shared-root copy is deleted. --- src/__tests__/push-all-resources-namespaces.test.ts | 4 +++- src/resources/repo-resource.ts | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/__tests__/push-all-resources-namespaces.test.ts b/src/__tests__/push-all-resources-namespaces.test.ts index f5ad9c47d..b7615a4bb 100644 --- a/src/__tests__/push-all-resources-namespaces.test.ts +++ b/src/__tests__/push-all-resources-namespaces.test.ts @@ -439,7 +439,9 @@ describe('a placed copy the author still edits, against what its push delivered' commit(placedFile, 'v2 (teammate fix)\n', 'teammate fixes the placed file'); fs.writeFileSync(activeFile, 'v1 plus my edit\n'); - await expect(scan()).rejects.toThrow(/docs\/ops\/guide\.md/); + await expect(scan()).rejects.toThrow( + /docs\/ops\/guide\.md changed on the team repo.*Merge your edit into docs\/ops\/guide\.md, keeping the team's change there, then delete docs\/guide\.md/, + ); }); it('treats a placed env file the same way', async () => { diff --git a/src/resources/repo-resource.ts b/src/resources/repo-resource.ts index 1366f9eb4..af0052ce2 100644 --- a/src/resources/repo-resource.ts +++ b/src/resources/repo-resource.ts @@ -55,7 +55,7 @@ export async function placedCopy( if (theirs !== record.blob) { throw new Error( `${record.path} changed on the team repo after your ${authorPath} was placed there, and ${authorPath} ` - + `changed too, so pushing it would overwrite that change. Move your edit into ${record.path} and delete ${authorPath}`, + + `changed too, so pushing it would overwrite that change. Merge your edit into ${record.path}, keeping the team's change there, then delete ${authorPath}`, ); } const item: ResourceItem & { prePlacementPath: string } = { From 2ced2a9ab6586b62166c0b3c4bb41b2873ac4a8e Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 22 Oct 2026 22:52:47 +0200 Subject: [PATCH 23/28] docs(changelog): note the --role namespace check --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 310f21856..ce7a34909 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ All notable changes to this project will be documented in this file. See [standa ### Changed - `teamai init` no longer pulls team skills, rules, agents or MCP before it exits. It still saves config and installs hooks. In project scope it creates a tool directory only for a tool named with `--agent` or picked in a terminal; otherwise that directory appears when the tool starts a session, and that session start pulls. `teamai pull` still does not create a missing tool directory. A tool that reads rules and MCP once, before its SessionStart hook, sees them from the following session (for [#1065](https://github.com/Tencent/teamai-cli/issues/1065)). +- `teamai push --role ` fails with exit 2 when no role or project declares `` for that type, as `--project` already does. It used to write the files under `` anyway, where pull delivers them to nobody. A team with neither manifest still accepts any namespace (for [#1101](https://github.com/Tencent/teamai-cli/issues/1101)). ### 💥 Breaking Changes From 0a2723c8a180beba5584121d4830c40b87781728 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 22 Oct 2026 22:53:44 +0200 Subject: [PATCH 24/28] feat(namespaces): resolve inactive scorers namespaces like prompts Pull has no scorers reader yet (#1085). Resolve the scorers namespaces a member does not select, in the same shape prompts use, so that reader can withhold them. --- .../resource-namespaces-scorers.test.ts | 59 +++++++++++++++++++ src/resource-namespaces.ts | 19 +++++- 2 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 src/__tests__/resource-namespaces-scorers.test.ts diff --git a/src/__tests__/resource-namespaces-scorers.test.ts b/src/__tests__/resource-namespaces-scorers.test.ts new file mode 100644 index 000000000..4856eb3cb --- /dev/null +++ b/src/__tests__/resource-namespaces-scorers.test.ts @@ -0,0 +1,59 @@ +/** + * Scorer namespaces resolve the way prompts do: a scorers// that any role + * or project lists under `resources.scorers` is a namespace, active only for the + * members who select it. Pull has no scorers reader yet (#1085); this is the + * shape that reader will consume. + */ +import { describe, expect, it } from 'vitest'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { resolveResourceNamespaces } from '../resource-namespaces.js'; +import type { LocalConfig } from '../types.js'; + +function repoWith(roles: string, projects: string): string { + const repoDir = mkdtempSync(path.join(os.tmpdir(), 'teamai-ns-scorers-')); + mkdirSync(path.join(repoDir, 'manifest'), { recursive: true }); + writeFileSync(path.join(repoDir, 'manifest', 'roles.yaml'), roles, 'utf-8'); + writeFileSync(path.join(repoDir, 'manifest', 'projects.yaml'), projects, 'utf-8'); + return repoDir; +} + +function localConfig(repoDir: string, overrides: Partial = {}): LocalConfig { + return { + repo: { localPath: repoDir, remote: 'https://github.com/acme/team.git' }, + username: 'e2e', + additionalRoles: [], + ...overrides, + } as LocalConfig; +} + +describe('resolveResourceNamespaces: scorers namespaces', () => { + const NO_SCORER_ROLE = 'version: 1\nroles:\n - id: other\n resources: { knowledge: [], skills: [] }\n'; + const PROJECTS = 'version: 1\nprojects:\n' + + ' - id: svc-a\n name: Svc A\n resources: { scorers: [svc-a] }\n' + + ' - id: svc-b\n name: Svc B\n resources: { scorers: [svc-b] }\n'; + + it('withholds a declared scorers namespace no active role or project selects', async () => { + const repoDir = repoWith(NO_SCORER_ROLE, PROJECTS); + try { + const resolved = await resolveResourceNamespaces(localConfig(repoDir, { projects: ['svc-a'] })); + expect(resolved?.inactiveScorersNamespaces).toEqual(['svc-b']); + expect(resolved?.activeNamespaces.scorers).toEqual(['svc-a']); + } finally { + rmSync(repoDir, { recursive: true, force: true }); + } + }); + + it('activates a scorers namespace a role declares for the member who holds it', async () => { + const roles = 'version: 1\nroles:\n - id: ops\n resources: { knowledge: [], skills: [], scorers: [ops-eval] }\n'; + const repoDir = repoWith(roles, 'version: 1\nprojects: []\n'); + try { + const resolved = await resolveResourceNamespaces(localConfig(repoDir, { primaryRole: 'ops', projects: [] })); + expect(resolved?.activeNamespaces.scorers).toEqual(['ops-eval']); + expect(resolved?.inactiveScorersNamespaces).toEqual([]); + } finally { + rmSync(repoDir, { recursive: true, force: true }); + } + }); +}); diff --git a/src/resource-namespaces.ts b/src/resource-namespaces.ts index 63963ed64..f60430a49 100644 --- a/src/resource-namespaces.ts +++ b/src/resource-namespaces.ts @@ -41,6 +41,7 @@ export async function resolveResourceNamespaces(localConfig: LocalConfig): Promi inactiveDocsNamespaces: inAll(resolved.map((one) => one.inactiveDocsNamespaces)), inactiveWikiNamespaces: inAll(resolved.map((one) => one.inactiveWikiNamespaces)), inactivePromptsNamespaces: inAll(resolved.map((one) => one.inactivePromptsNamespaces)), + inactiveScorersNamespaces: inAll(resolved.map((one) => one.inactiveScorersNamespaces)), }; } @@ -178,7 +179,23 @@ async function resolveOwnResourceNamespaces(localConfig: LocalConfig) { const activePrompts = new Set(activeNamespaces.prompts ?? []); const inactivePromptsNamespaces = [...declaredPromptsNamespaces].filter((namespace) => !activePrompts.has(namespace)); - return { activeNamespaces, allSkillNamespaces, inactiveDocsNamespaces, inactiveWikiNamespaces, inactivePromptsNamespaces }; + // Scorers follow the prompts rule. Nothing reads scorers// on pull yet + // (#1085); this is the set that reader will withhold. + const declaredScorersNamespaces = new Set([ + ...(rolesManifest?.roles ?? []).flatMap((role) => role.resources.scorers ?? []), + ...(projectsManifest?.projects ?? []).flatMap((project) => project.resources.scorers ?? []), + ]); + const activeScorers = new Set(activeNamespaces.scorers ?? []); + const inactiveScorersNamespaces = [...declaredScorersNamespaces].filter((namespace) => !activeScorers.has(namespace)); + + return { + activeNamespaces, + allSkillNamespaces, + inactiveDocsNamespaces, + inactiveWikiNamespaces, + inactivePromptsNamespaces, + inactiveScorersNamespaces, + }; } /** From bdd76d6ae21b96a91804016529c0f0223c8696ea Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Fri, 23 Oct 2026 06:26:24 +0200 Subject: [PATCH 25/28] feat(contribute): accept an empty --namespace as the shared root A member whose roles or project declare one learnings namespace contributed there by default, and --namespace only accepted declared namespaces, so that member could not write to the shared root. A member with several namespaces reaches it by default. --namespace "" now names the shared root for every member. --- CHANGELOG.md | 1 + docs/guide/knowledge.md | 2 +- docs/guide/zh-CN/knowledge.md | 2 +- skill-data/core/references/commands.md | 2 +- src/__tests__/e2e/contribute-namespace.test.ts | 11 ++++++++++- src/contribute.ts | 3 ++- src/index.ts | 2 +- 7 files changed, 17 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce7a34909..9c8a219e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ All notable changes to this project will be documented in this file. See [standa - `teamai init` no longer pulls team skills, rules, agents or MCP before it exits. It still saves config and installs hooks. In project scope it creates a tool directory only for a tool named with `--agent` or picked in a terminal; otherwise that directory appears when the tool starts a session, and that session start pulls. `teamai pull` still does not create a missing tool directory. A tool that reads rules and MCP once, before its SessionStart hook, sees them from the following session (for [#1065](https://github.com/Tencent/teamai-cli/issues/1065)). - `teamai push --role ` fails with exit 2 when no role or project declares `` for that type, as `--project` already does. It used to write the files under `` anyway, where pull delivers them to nobody. A team with neither manifest still accepts any namespace (for [#1101](https://github.com/Tencent/teamai-cli/issues/1101)). +- `teamai contribute --namespace ""` writes to the shared root `learnings/`, also for a member whose default is a namespace. It used to be rejected, so such a member could not reach the shared root (for [#1101](https://github.com/Tencent/teamai-cli/issues/1101)). ### 💥 Breaking Changes diff --git a/docs/guide/knowledge.md b/docs/guide/knowledge.md index 856908fdf..864a731ba 100644 --- a/docs/guide/knowledge.md +++ b/docs/guide/knowledge.md @@ -25,7 +25,7 @@ teamai contribute --file /tmp/session.md teamai contribute --file /tmp/session.md --namespace payments ``` -`--namespace` must be a learnings namespace this scope's projects declare in `manifest/projects.yaml`, or its roles in `manifest/roles.yaml`. +`--namespace` must be a learnings namespace this scope's projects declare in `manifest/projects.yaml`, or its roles in `manifest/roles.yaml`. `--namespace ""` writes to the shared root `learnings/`. To stop the prompt, set this in `teamai.yaml`: diff --git a/docs/guide/zh-CN/knowledge.md b/docs/guide/zh-CN/knowledge.md index 7b3203b70..1e52348f5 100644 --- a/docs/guide/zh-CN/knowledge.md +++ b/docs/guide/zh-CN/knowledge.md @@ -25,7 +25,7 @@ teamai contribute --file /tmp/session.md teamai contribute --file /tmp/session.md --namespace payments ``` -`--namespace` 必须是当前 scope 的项目在 `manifest/projects.yaml` 里、或其角色在 `manifest/roles.yaml` 里声明的 learnings namespace。 +`--namespace` 必须是当前 scope 的项目在 `manifest/projects.yaml` 里、或其角色在 `manifest/roles.yaml` 里声明的 learnings namespace。`--namespace ""` 写入共享根目录 `learnings/`。 不想要这条提醒时，在 `teamai.yaml` 里写： diff --git a/skill-data/core/references/commands.md b/skill-data/core/references/commands.md index 1af6d1e1e..b576cec4a 100644 --- a/skill-data/core/references/commands.md +++ b/skill-data/core/references/commands.md @@ -370,7 +370,7 @@ Generated: do not edit by hand. Regenerate with - `teamai contribute` — Contribute session knowledge to team repo; projects list shows the default destination and allowed namespaces - `--file ` — Path to the contribution document - `--title ` — Title for the contribution document - - `--namespace <ns>` — Write to an active learnings namespace (default: the only active namespace, or the shared root when none or several are active) + - `--namespace <ns>` — Write to an active learnings namespace, or to the shared root with "" (default: the only active namespace, or the shared root when none or several are active) - `--session-id <id>` — Session ID for dedup tracking - `--scope <scope>` — Target scope: user or workspace diff --git a/src/__tests__/e2e/contribute-namespace.test.ts b/src/__tests__/e2e/contribute-namespace.test.ts index 030d1c9ed..48fa5defd 100644 --- a/src/__tests__/e2e/contribute-namespace.test.ts +++ b/src/__tests__/e2e/contribute-namespace.test.ts @@ -132,6 +132,15 @@ describe('contribute --namespace through the built CLI (#916)', () => { if (projects[0] === 'svc-a') expect(result.output).toContain('pass --namespace <ns>: svc-a, payments'); }); + it('publishes to the shared root with an empty --namespace when the default is a namespace', () => { + setProjects(['alpha']); + const result = run(['contribute', '--file', note, '--title', 'narwhal', '--namespace', '']); + expect(result.code, result.output).toBe(0); + expect(result.output).toContain('Contributed: learnings/narwhal-'); + const files = git(['ls-tree', '-r', '--name-only', 'teamai-learnings'], origin).split('\n'); + expect(files.filter((f) => f.endsWith('.md'))).toEqual([expect.stringMatching(/^learnings\/narwhal-/)]); + }); + it.each(['project', 'user'])('previews the selected %s namespace without filesystem writes', (scope) => { if (scope === 'user') { const config = YAML.parse(fs.readFileSync(configPath, 'utf8')); @@ -145,7 +154,7 @@ describe('contribute --namespace through the built CLI (#916)', () => { expect(snapshot()).toEqual(before); }); - it.each(['alpha-notes', '../escape', 'a/b', 'a\\b', ''])('rejects the unavailable namespace %j before writing', (namespace) => { + it.each(['alpha-notes', '../escape', 'a/b', 'a\\b'])('rejects the unavailable namespace %j before writing', (namespace) => { const before = snapshot(sandbox, true); const result = run(['contribute', '--file', note, '--namespace', namespace]); expect(result.code, result.output).toBe(1); diff --git a/src/contribute.ts b/src/contribute.ts index 6c84ffa28..5373ba054 100644 --- a/src/contribute.ts +++ b/src/contribute.ts @@ -195,7 +195,8 @@ export async function contribute( const ownNamespaces = await resolveActiveLearningsNamespaces(localConfig); const namespaces = await deliveredLearningsNamespaces(localConfig); // Validate before saving the queue, creating worktrees or rebuilding indexes. - if (options.namespace !== undefined && + // An empty --namespace names the shared root, which every member may write. + if (options.namespace !== undefined && options.namespace !== '' && (!isSafeNamespaceSegment(options.namespace) || !namespaces.includes(options.namespace))) { log.error(`Cannot contribute to namespace "${options.namespace}". Allowed learnings namespaces: ${namespaces.join(', ') || '(none)'}.`); process.exitCode = 1; diff --git a/src/index.ts b/src/index.ts index de2f114c9..38aaf9ed7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1237,7 +1237,7 @@ program .description('Contribute session knowledge to team repo; projects list shows the default destination and allowed namespaces') .option('--file <path>', 'Path to the contribution document') .option('--title <title>', 'Title for the contribution document') - .option('--namespace <ns>', 'Write to an active learnings namespace (default: the only active namespace, or the shared root when none or several are active)') + .option('--namespace <ns>', 'Write to an active learnings namespace, or to the shared root with "" (default: the only active namespace, or the shared root when none or several are active)') .option('--session-id <id>', 'Session ID for dedup tracking') .option('--scope <scope>', 'Target scope: user or workspace') .action(async (cmdOpts) => { From 1debe9402f7d73e5dbe713c153e4386612d4b09e Mon Sep 17 00:00:00 2001 From: Saul Moro <saul.moro.gomez@gmail.com> Date: Fri, 23 Oct 2026 06:54:48 +0200 Subject: [PATCH 26/28] test(push): push only the rule where --role names a knowledge namespace Five placement-record cases pushed a rule, a skill and an agent with --role be-know, which the roles manifest declares only for knowledge. Since --role has to name a declared namespace for each type it places, that push stops with exit 2. The cases are about rule records, so they push the rule alone. --- src/__tests__/push-namespace-e2e.test.ts | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/__tests__/push-namespace-e2e.test.ts b/src/__tests__/push-namespace-e2e.test.ts index 676b0e771..032e57d8d 100644 --- a/src/__tests__/push-namespace-e2e.test.ts +++ b/src/__tests__/push-namespace-e2e.test.ts @@ -185,9 +185,18 @@ function makeFixture(options: { } /** Author a new rule, skill and agent at each tool directory's root. */ -function writeLocalResources(fixture: Fixture, ruleBody = '# Rule v1\n'): void { +/** + * Only the rule: `--role be-know` names the backend role's knowledge namespace, + * which no role declares for skills or agents, so a push carrying those fails. + */ +function writeLocalRule(fixture: Fixture, ruleBody = '# Rule v1\n'): void { const { projectRoot, agent } = fixture; fs.writeFileSync(path.join(projectRoot, `.${agent}/rules`, 'my-rule.md'), ruleBody); +} + +function writeLocalResources(fixture: Fixture, ruleBody = '# Rule v1\n'): void { + const { projectRoot, agent } = fixture; + writeLocalRule(fixture, ruleBody); fs.mkdirSync(path.join(projectRoot, `.${agent}/skills`, 'my-skill'), { recursive: true }); fs.writeFileSync( path.join(projectRoot, `.${agent}/skills/my-skill`, 'SKILL.md'), @@ -501,7 +510,7 @@ describe('push places new rules and agents in a namespace (issue #649)', () => { it('pull updates the author\'s root copy of a placed rule instead of writing a second one', async () => { const fixture = track(makeFixture({ agent: 'claude', provider: 'git' })); - writeLocalResources(fixture); + writeLocalRule(fixture); // be-know is the backend role's knowledge namespace, so it IS active here // and pull delivers the rule — which used to land at rules/be-know/ beside // the author's own copy at the root, the same rule twice for a tool that @@ -520,7 +529,7 @@ describe('push places new rules and agents in a namespace (issue #649)', () => { it('pull keeps a member\'s own file at the namespaced path beside the author\'s root copy, and removes teamai\'s (#993)', async () => { const fixture = track(makeFixture({ agent: 'claude', provider: 'git' })); - writeLocalResources(fixture); + writeLocalRule(fixture); await runCLI(['push', '--role', 'be-know', '--all'], fixture.projectRoot, fixture.home); mergeBranch(fixture, branchFiles(fixture).branch); const namespaced = path.join(fixture.projectRoot, '.claude/rules', 'be-know', 'my-rule.md'); @@ -626,7 +635,7 @@ describe('push places new rules and agents in a namespace (issue #649)', () => { it('pull drops the placement record of a rule the team has since deleted', async () => { const fixture = track(makeFixture({ agent: 'claude', provider: 'git' })); - writeLocalResources(fixture); + writeLocalRule(fixture); await runCLI(['push', '--role', 'be-know', '--all'], fixture.projectRoot, fixture.home); mergeBranch(fixture, branchFiles(fixture).branch); const landed = await runCLI(['pull', '--force'], fixture.projectRoot, fixture.home); @@ -652,7 +661,7 @@ describe('push places new rules and agents in a namespace (issue #649)', () => { it('never records a placement whose PR was closed without merging, even with the branch kept', async () => { const fixture = track(makeFixture({ agent: 'claude', provider: 'git' })); - writeLocalResources(fixture); + writeLocalRule(fixture); await runCLI(['push', '--role', 'be-know', '--all'], fixture.projectRoot, fixture.home); expect(branchFiles(fixture).files).toContain('rules/be-know/my-rule.md'); @@ -676,7 +685,7 @@ describe('push places new rules and agents in a namespace (issue #649)', () => { // its name here, as an active namespace's rule would (#707): the author's // root copy still stands for rules/be-know/my-rule.md. const fixture = track(makeFixture({ agent: 'claude', provider: 'git' })); - writeLocalResources(fixture); + writeLocalRule(fixture); await runCLI(['push', '--role', 'be-know', '--all'], fixture.projectRoot, fixture.home); mergeBranch(fixture, branchFiles(fixture).branch); await runCLI(['pull', '--force'], fixture.projectRoot, fixture.home); From 2c0e9406875d78670111e2d17096ee518a2ef39b Mon Sep 17 00:00:00 2001 From: Saul Moro <saul.moro.gomez@gmail.com> Date: Fri, 23 Oct 2026 09:10:59 +0200 Subject: [PATCH 27/28] test(push): keep a teammate's change from a self-mode checkout behind main Single-repo push compares the member's .teamai/ with a fresh origin/<default>, so on a checkout behind it a file a teammate changed reads as the member's edit and the branch carries the old content back. Cover a file only main changed, a file both changed, a namespaced destination, a format change that deletes a file main changed, a tool-directory copy edited on top of main, a shallow clone, a checkout with no shared history, and a stop for a file the push does not send. A genuine edit of a stale .teamai/rules copy now stops instead of overwriting the teammate's update. --- .../e2e/push-sync-followups-823.test.ts | 9 +- .../e2e/self-mode-push-behind.test.ts | 308 ++++++++++++++++++ 2 files changed, 313 insertions(+), 4 deletions(-) create mode 100644 src/__tests__/e2e/self-mode-push-behind.test.ts diff --git a/src/__tests__/e2e/push-sync-followups-823.test.ts b/src/__tests__/e2e/push-sync-followups-823.test.ts index 3e5f23e20..9f93e555e 100644 --- a/src/__tests__/e2e/push-sync-followups-823.test.ts +++ b/src/__tests__/e2e/push-sync-followups-823.test.ts @@ -192,14 +192,15 @@ describe('pre-push sync in single-repo mode (#823 items 2 and 10)', () => { expect(fs.readFileSync(activeRule(), 'utf8')).toBe(R1); }); - it('still lists a genuine edit of a stale .teamai/rules copy as modified (#823 item 10)', async () => { + it('refuses a genuine edit of a stale .teamai/rules copy rather than push it over the teammate\'s update (#823 item 10)', async () => { await run(['pull']); teammateLandsOnMain('.teamai/rules/team-rule.md', R2); fs.writeFileSync(activeRule(), `${R1}\nA local edit.\n`); - const push = await run(['--dry-run', 'push']); - expect(push).toContain('[rules] team-rule (modified)'); - expect(push).not.toContain(HELD); + const push = await runCLI(['--dry-run', 'push'], projectRoot, home); + expect(push.code, push.output).toBe(2); + expect(push.output).toContain('.teamai/rules/team-rule.md changed both here and on origin/main'); + expect(push.output).not.toContain(HELD); }); it('holds a stale .teamai/skills copy on a branch behind a teammate\'s update (#823 item 10)', async () => { diff --git a/src/__tests__/e2e/self-mode-push-behind.test.ts b/src/__tests__/e2e/self-mode-push-behind.test.ts new file mode 100644 index 000000000..9d819a655 --- /dev/null +++ b/src/__tests__/e2e/self-mode-push-behind.test.ts @@ -0,0 +1,308 @@ +/** + * E2E: in single-repo mode push compares the member's `.teamai/` with a fresh + * checkout of the default branch. A checkout behind that branch holds the + * older version of every team file a teammate changed since, and push must + * neither offer it as the member's edit nor push it over the teammate's. + */ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { execFileSync, spawn } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { projectSlug } from '../../utils/partition.js'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const ROOT = path.resolve(__dirname, '..', '..', '..'); +const CLI = path.join(ROOT, 'dist', 'index.js'); + +const GIT_ENV = { + GIT_AUTHOR_NAME: 'TeamAI CI', + GIT_AUTHOR_EMAIL: 'ci@teamai.test', + GIT_COMMITTER_NAME: 'TeamAI CI', + GIT_COMMITTER_EMAIL: 'ci@teamai.test', +}; + +interface RunResult { + code: number | null; + output: string; +} + +function runCLI(args: string[], cwd: string, home: string): Promise<RunResult> { + return new Promise((resolve) => { + const child = spawn('node', [CLI, ...args], { + cwd, + env: { ...process.env, ...GIT_ENV, HOME: home, FORCE_COLOR: '0' }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + let output = ''; + child.stdout.on('data', (data: Buffer) => { output += data.toString(); }); + child.stderr.on('data', (data: Buffer) => { output += data.toString(); }); + child.on('close', (code) => resolve({ code, output })); + }); +} + +function git(args: string[], cwd: string): string { + return execFileSync('git', args, { cwd, encoding: 'utf8', env: { ...process.env, ...GIT_ENV } }).trim(); +} + +describe('single-repo push from a checkout behind the default branch', () => { + let sandbox: string; + let home: string; + let projectRoot: string; + let teammate: string; + let remote: string; + + const README = '# Shared guide\n'; + const ENV = 'variables:\n - key: API_BASE\n value: https://old.example.com\n'; + const TEAMMATE_README = `${README}\nA teammate's fix.\n`; + const TEAMMATE_ENV = 'variables:\n - key: API_BASE\n value: https://new.example.com\n'; + const active = (file: string) => path.join(projectRoot, ...file.split('/')); + + beforeEach(() => { + if (!fs.existsSync(CLI)) throw new Error(`CLI binary not found at ${CLI}. Run "npm run build" first.`); + sandbox = fs.realpathSync.native(fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-self-behind-e2e-'))); + home = path.join(sandbox, 'home'); + projectRoot = path.join(sandbox, 'project'); + teammate = path.join(sandbox, 'teammate'); + remote = path.join(sandbox, 'origin.git'); + + fs.mkdirSync(home, { recursive: true }); + fs.mkdirSync(path.join(projectRoot, '.teamai', 'docs'), { recursive: true }); + fs.mkdirSync(path.join(projectRoot, '.teamai', 'env'), { recursive: true }); + fs.writeFileSync(path.join(projectRoot, '.teamai', 'teamai.yaml'), [ + 'team: self-behind-e2e', + 'repo: https://git.example.com/team/project.git', + 'provider: git', + 'mode: self', + 'usageReport: false', + '', + ].join('\n')); + fs.writeFileSync(path.join(projectRoot, '.teamai', '.gitignore'), 'config.yaml\nstate.json\n'); + fs.writeFileSync(active('.teamai/docs/README.md'), README); + fs.writeFileSync(active('.teamai/env/env.yaml'), ENV); + git(['init', '-q', '-b', 'main'], projectRoot); + git(['add', '-A'], projectRoot); + git(['commit', '-q', '-m', 'project'], projectRoot); + git(['clone', '-q', '--bare', projectRoot, remote], sandbox); + git(['remote', 'add', 'origin', remote], projectRoot); + git(['fetch', '-q', 'origin'], projectRoot); + git(['clone', '-q', remote, teammate], sandbox); + + const partition = path.join(home, '.teamai', 'projects', projectSlug(projectRoot)); + fs.mkdirSync(partition, { recursive: true }); + fs.writeFileSync(path.join(partition, 'anchor'), `${projectRoot}\n`); + fs.writeFileSync(path.join(partition, 'config.yaml'), [ + 'repo:', + ' kind: self', + ` localPath: ${path.join(projectRoot, '.teamai')}`, + ` remote: ${remote}`, + ` businessRepoRoot: ${projectRoot}`, + 'username: ci-self-behind', + 'updatePolicy: skip', + 'scope: project', + `projectRoot: ${projectRoot}`, + 'enabledAgents: [claude]', + '', + ].join('\n')); + }); + + afterEach(() => { + if (sandbox) fs.rmSync(sandbox, { recursive: true, force: true }); + }); + + /** A teammate lands these files on the default branch; the member does not merge it. */ + const teammateLandsOnMain = (files: Record<string, string>): void => { + git(['pull', '-q', 'origin', 'main'], teammate); + for (const [file, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(teammate, file)), { recursive: true }); + fs.writeFileSync(path.join(teammate, file), content); + } + git(['add', '-A'], teammate); + git(['commit', '-q', '-m', 'teammate fixes'], teammate); + git(['push', '-q', 'origin', 'main'], teammate); + }; + const pushBranches = (): string[] => + git(['for-each-ref', '--format=%(refname:short)', 'refs/heads/teamai/push/'], remote).split('\n').filter(Boolean); + /** The team merges the member's only push branch; the member takes it with git and `teamai pull`. */ + const mergePushAndPull = async (): Promise<void> => { + const [branch] = pushBranches(); + git(['pull', '-q', 'origin', 'main'], teammate); + git(['fetch', '-q', 'origin', branch], teammate); + git(['merge', '-q', '--no-ff', '-m', `merge ${branch}`, 'FETCH_HEAD'], teammate); + git(['push', '-q', 'origin', 'main', `:${branch}`], teammate); + git(['pull', '-q', '--ff-only', 'origin', 'main'], projectRoot); + expect((await runCLI(['pull'], projectRoot, home)).code).toBe(0); + }; + + it('pushes only the member\'s new file and keeps what a teammate changed on the default branch', async () => { + expect((await runCLI(['pull'], projectRoot, home)).code).toBe(0); + teammateLandsOnMain({ '.teamai/docs/README.md': TEAMMATE_README, '.teamai/env/env.yaml': TEAMMATE_ENV }); + fs.writeFileSync(active('.teamai/docs/handbook.md'), '# Handbook\n'); + + const push = await runCLI(['push', '--all'], projectRoot, home); + + // A generic git host takes the branch but no pull request, which exits 1. + expect(push.code, push.output).toBe(1); + expect(push.output).toContain('Pushed branch'); + expect(push.output).not.toContain('README.md (modified)'); + expect(push.output).not.toContain('env.yaml (modified)'); + expect(push.output).toContain('.teamai/docs/README.md'); + expect(push.output).toContain('git pull'); + const branches = pushBranches(); + expect(branches).toHaveLength(1); + expect(git(['diff', '--name-only', 'main', branches[0]], remote).split('\n')).toEqual(['.teamai/docs/handbook.md']); + expect(git(['show', `${branches[0]}:.teamai/docs/README.md`], remote)).toBe(TEAMMATE_README.trim()); + expect(fs.readFileSync(active('.teamai/docs/README.md'), 'utf8')).toBe(README); + }); + + it('refuses with exit 2, pushing nothing, when the member and a teammate both changed a team file', async () => { + expect((await runCLI(['pull'], projectRoot, home)).code).toBe(0); + teammateLandsOnMain({ '.teamai/docs/README.md': TEAMMATE_README }); + fs.writeFileSync(active('.teamai/docs/README.md'), `${README}\nThe member's edit.\n`); + fs.writeFileSync(active('.teamai/docs/handbook.md'), '# Handbook\n'); + + const push = await runCLI(['push', '--all'], projectRoot, home); + + expect(push.code, push.output).toBe(2); + expect(push.output).toContain('.teamai/docs/README.md'); + expect(push.output).toContain('git pull'); + expect(pushBranches()).toEqual([]); + expect(git(['show', 'main:.teamai/docs/README.md'], remote)).toBe(TEAMMATE_README.trim()); + }); + + it('pushes a selected skill when a file it leaves out changed both here and on the default branch', async () => { + expect((await runCLI(['pull'], projectRoot, home)).code).toBe(0); + teammateLandsOnMain({ '.teamai/env/env.yaml': TEAMMATE_ENV }); + fs.writeFileSync(active('.teamai/env/env.yaml'), 'variables:\n - key: API_BASE\n value: https://mine.example.com\n'); + const skill = active('.teamai/skills/new-skill'); + fs.mkdirSync(skill, { recursive: true }); + fs.writeFileSync(path.join(skill, 'SKILL.md'), '---\nname: new-skill\ndescription: A new skill\n---\n\nBody.\n'); + + const push = await runCLI(['push', '--skill', skill], projectRoot, home); + + expect(push.code, push.output).toBe(1); + expect(push.output).toContain('Pushed branch'); + const branches = pushBranches(); + expect(branches).toHaveLength(1); + const pushed = git(['diff', '--name-only', 'main', branches[0]], remote).split('\n'); + expect(pushed).toContain('.teamai/skills/new-skill/SKILL.md'); + expect(pushed.filter((file) => !file.startsWith('.teamai/skills/new-skill/'))).toEqual([]); + }); + + it('refuses with exit 2 when this checkout lacks the history to tell who changed a team file', async () => { + git(['checkout', '-q', '-b', 'feature'], projectRoot); + fs.writeFileSync(path.join(projectRoot, 'business.txt'), 'feature work\n'); + git(['add', '-A'], projectRoot); + git(['commit', '-q', '-m', 'feature work'], projectRoot); + git(['push', '-q', 'origin', 'feature'], projectRoot); + teammateLandsOnMain({ '.teamai/docs/README.md': TEAMMATE_README }); + fs.rmSync(projectRoot, { recursive: true, force: true }); + git(['clone', '-q', '--depth=1', '--branch', 'feature', '--no-single-branch', `file://${remote}`, projectRoot], sandbox); + fs.writeFileSync(active('.teamai/docs/handbook.md'), '# Handbook\n'); + + const push = await runCLI(['push', '--all'], projectRoot, home); + + expect(push.code, push.output).toBe(2); + expect(push.output).toContain('.teamai/docs/README.md'); + expect(push.output).toContain('git fetch --unshallow origin'); + expect(pushBranches()).toEqual([]); + + git(['fetch', '-q', '--unshallow', 'origin'], projectRoot); + const retry = await runCLI(['push', '--all'], projectRoot, home); + + expect(retry.code, retry.output).toBe(1); + const branches = pushBranches(); + expect(branches).toHaveLength(1); + expect(git(['diff', '--name-only', 'main', branches[0]], remote).split('\n')).toEqual(['.teamai/docs/handbook.md']); + }); + + it('refuses with exit 2, without suggesting an unshallow fetch, when a full checkout shares no history with the default branch', async () => { + git(['checkout', '-q', '--orphan', 'rewritten'], teammate); + fs.writeFileSync(path.join(teammate, '.teamai', 'docs', 'README.md'), TEAMMATE_README); + git(['add', '-A'], teammate); + git(['commit', '-q', '-m', 'rewritten history'], teammate); + git(['push', '-q', '--force', 'origin', 'rewritten:main'], teammate); + fs.writeFileSync(active('.teamai/docs/handbook.md'), '# Handbook\n'); + + const push = await runCLI(['push', '--all'], projectRoot, home); + + expect(push.code, push.output).toBe(2); + expect(push.output).toContain('.teamai/docs/README.md'); + expect(push.output).toContain('shares no history with origin/main'); + expect(push.output).not.toContain('--unshallow'); + expect(pushBranches()).toEqual([]); + }); + + it('refuses with exit 2 when a teammate changed the namespaced file a root rule is pushed to', async () => { + expect((await runCLI(['pull'], projectRoot, home)).code).toBe(0); + const RULE = '# Ops rule\n'; + fs.mkdirSync(active('.teamai/rules'), { recursive: true }); + fs.writeFileSync(active('.teamai/rules/ops-rule.md'), RULE); + const placed = await runCLI(['push', '--all', '--role', 'ops'], projectRoot, home); + expect(placed.output).toContain('Pushed branch'); + await mergePushAndPull(); + expect(fs.readFileSync(active('.teamai/rules/ops/ops-rule.md'), 'utf8')).toBe(RULE); + + teammateLandsOnMain({ '.teamai/rules/ops/ops-rule.md': `${RULE}\nA teammate's line.\n` }); + fs.writeFileSync(active('.teamai/rules/ops-rule.md'), `${RULE}\nThe member's line.\n`); + + const push = await runCLI(['push', '--all', '--role', 'ops'], projectRoot, home); + + expect(push.code, push.output).toBe(2); + expect(push.output).toContain('.teamai/rules/ops-rule.md'); + expect(pushBranches()).toEqual([]); + expect(git(['show', 'main:.teamai/rules/ops/ops-rule.md'], remote)).toContain('A teammate\'s line.'); + }); + + it('pushes an edit made on a tool copy that push already brought up to the default branch', async () => { + fs.mkdirSync(path.join(home, '.claude'), { recursive: true }); + fs.mkdirSync(active('.claude'), { recursive: true }); + const RULE = '# Foo rule\n'; + teammateLandsOnMain({ '.teamai/rules/foo.md': RULE }); + git(['pull', '-q', '--ff-only', 'origin', 'main'], projectRoot); + expect((await runCLI(['pull'], projectRoot, home)).code).toBe(0); + teammateLandsOnMain({ '.teamai/rules/foo.md': `${RULE}\nTeammate line.\n` }); + expect((await runCLI(['push', '--all', '--dry-run'], projectRoot, home)).code).toBe(0); + expect(fs.readFileSync(active('.claude/rules/foo.md'), 'utf8')).toContain('Teammate line.'); + fs.appendFileSync(active('.claude/rules/foo.md'), '\nMember line.\n'); + + const push = await runCLI(['push', '--all'], projectRoot, home); + + expect(push.code, push.output).toBe(1); + const branches = pushBranches(); + expect(branches).toHaveLength(1); + const pushed = git(['show', `${branches[0]}:.teamai/rules/foo.md`], remote); + expect(pushed).toContain('Teammate line.'); + expect(pushed).toContain('Member line.'); + }); + + it('refuses with exit 2 when the agent file a format change removes changed on the default branch', async () => { + expect((await runCLI(['pull'], projectRoot, home)).code).toBe(0); + const AGENT = '---\nname: sample\ndescription: A sample agent\n---\n\nSample instructions.\n'; + fs.mkdirSync(active('.teamai/agents'), { recursive: true }); + fs.writeFileSync(active('.teamai/agents/sample.md'), AGENT); + const placed = await runCLI(['push', '--all', '--role', 'ops'], projectRoot, home); + expect(placed.output).toContain('Pushed branch'); + await mergePushAndPull(); + expect(fs.readFileSync(active('.teamai/agents/ops/sample.md'), 'utf8')).toBe(AGENT); + + teammateLandsOnMain({ '.teamai/agents/ops/sample.md': `${AGENT}\nTeammate safety instructions.\n` }); + fs.rmSync(active('.teamai/agents/sample.md')); + fs.writeFileSync( + active('.teamai/agents/sample.yaml'), + 'name: sample\ndescription: A sample agent\ninstructions: Member replacement instructions.\n', + ); + + const push = await runCLI(['push', '--all', '--role', 'ops'], projectRoot, home); + + expect(push.code, push.output).toBe(2); + expect(push.output).toContain( + '.teamai/agents/sample.yaml (pushed to .teamai/agents/ops/sample.yaml) would delete .teamai/agents/ops/sample.md, ' + + 'which changed on origin/main since this checkout branched from it.', + ); + expect(push.output).toContain('carry that change into .teamai/agents/sample.yaml'); + expect(pushBranches()).toEqual([]); + expect(git(['show', 'main:.teamai/agents/ops/sample.md'], remote)).toContain('Teammate safety instructions.'); + }); +}); From 6c4bb84c56a8c4cfe497c01ed6383167df16a4c7 Mon Sep 17 00:00:00 2001 From: Saul Moro <saul.moro.gomez@gmail.com> Date: Fri, 23 Oct 2026 09:10:59 +0200 Subject: [PATCH 28/28] fix(push): leave out team files main changed under a self-mode checkout behind it For each item sourced from .teamai/, compare the member's file with the merge-base and the tip of the default branch, at its source, its destination and any file the push deletes. A file only the default branch changed is left out with a line saying git pull brings it in. A file both sides changed, a file the push would delete, or one that cannot be classified for lack of shared history stops the push with exit 2 before anything is written, naming the files and the remedy. The stop applies only to items going out. Tool-directory copies keep their own checks: the pre-push sync moves them to the default branch's version while .teamai/ stays behind. --- CHANGELOG.md | 1 + .../core/references/contribute-member.md | 4 + src/push.ts | 140 +++++++++++++++++- 3 files changed, 143 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c8a219e2..95bcec4e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ All notable changes to this project will be documented in this file. See [standa - `teamai init` no longer pulls team skills, rules, agents or MCP before it exits. It still saves config and installs hooks. In project scope it creates a tool directory only for a tool named with `--agent` or picked in a terminal; otherwise that directory appears when the tool starts a session, and that session start pulls. `teamai pull` still does not create a missing tool directory. A tool that reads rules and MCP once, before its SessionStart hook, sees them from the following session (for [#1065](https://github.com/Tencent/teamai-cli/issues/1065)). - `teamai push --role <ns>` fails with exit 2 when no role or project declares `<ns>` for that type, as `--project` already does. It used to write the files under `<ns>` anyway, where pull delivers them to nobody. A team with neither manifest still accepts any namespace (for [#1101](https://github.com/Tencent/teamai-cli/issues/1101)). - `teamai contribute --namespace ""` writes to the shared root `learnings/`, also for a member whose default is a namespace. It used to be rejected, so such a member could not reach the shared root (for [#1101](https://github.com/Tencent/teamai-cli/issues/1101)). +- In single-repo mode, `teamai push` from a branch behind the default branch leaves out a team file under `.teamai/` that the default branch changed since and the member did not, and names it. It used to push the member's older copy over the teammate's change. When the member changed that file too, push stops with exit 2 and names it; merge the default branch and push again (for [#1101](https://github.com/Tencent/teamai-cli/issues/1101)). ### 💥 Breaking Changes diff --git a/skill-data/core/references/contribute-member.md b/skill-data/core/references/contribute-member.md index e7e847430..14bbbc771 100644 --- a/skill-data/core/references/contribute-member.md +++ b/skill-data/core/references/contribute-member.md @@ -120,6 +120,10 @@ The doc lands in the team's `learnings/` and appears for teammates on their next pushing it would revert a teammate's update. To publish an edit of it, bring the current version in first (`git fetch origin && git merge origin/<default>`, or copy the team's current file over it), redo the edit on top, and push again. + Any other team file under `.teamai/` that the default branch changed since + your branch left it is left out of the push when you did not edit it. A file + you edited too, a rule included but not a skill's file, stops the push with + exit 2; merge the default branch and push again. ## After contributing diff --git a/src/push.ts b/src/push.ts index 2be6b43c5..9c8f558f2 100644 --- a/src/push.ts +++ b/src/push.ts @@ -5,7 +5,7 @@ import { autoDetectInit, loadStateForScope, saveStateForScope } from './config.j import { assertNotReadOnly } from './read-only.js'; import { createGit, pullRepo, pushRepoBranch, checkoutMaster, generateBranchName, unusedBranchName, - resetToCleanMaster, isDedicatedRepoRoot, getDefaultBranch, getFileContentAtRev, getHeadCommit, + resetToCleanMaster, isDedicatedRepoRoot, getDefaultBranch, getFileContentAtRev, getHeadCommit, hashObject, } from './utils/git.js'; import { reconcilePlacementRecords, dropClosedPendingPushes, findPendingForItem, isRecordedItem, partiallySelectedEntries, pendingNamespaceFor, planPushGroups, recordedNamespace, @@ -1055,6 +1055,134 @@ async function pendingSelfTeamConfig(localConfig: LocalConfig): Promise<string | return committed === null || committed.toString() !== activeConfig ? activeConfig : null; } +type DefaultBranchChanges = + | { history: 'complete'; untouched: ResourceItem[]; edited: ResourceItem[]; retiring: ResourceItem[] } + | { history: 'missing'; shallow: boolean; unclassified: ResourceItem[] }; + +/** + * The self-mode push items sourced from `.teamai/` whose source, destination or + * retired file the default branch changed since this checkout branched from it, + * and which the checkout does not already hold at the branch's version: + * `untouched` when the checkout still holds the version it branched from, + * `edited` when it changed the file too, `retiring` when the push would delete + * a file the branch changed. Self-mode scans compare the checkout with the default + * branch, so on a checkout behind it both read as this member's edit, and + * pushing either reverts the other side. Without a common ancestor (a shallow + * clone, or rewritten history), what differs from its own HEAD is + * `unclassified`. A tool-directory copy is left alone: the pre-push sync can + * move it to the default branch's version while `.teamai/` stays behind, so + * where this checkout branched says nothing about it. + */ +async function itemsChangedOnDefaultBranch( + localConfig: LocalConfig, + memberRoot: string, + items: readonly ResourceItem[], +): Promise<DefaultBranchChanges> { + const teamDir = path.join(memberRoot, '.teamai'); + const teamFile = (relativePath: string): string => path.join(teamDir, ...relativePath.split('/')); + const teamItems = items.filter((item) => path.resolve(item.sourcePath).startsWith(teamDir + path.sep)); + const git = createGit(memberRoot); + const missing = async (unclassified: ResourceItem[]): Promise<DefaultBranchChanges> => ({ + history: 'missing', + shallow: (await git.raw(['rev-parse', '--is-shallow-repository'])).trim() === 'true', + unclassified, + }); + const defaultTip = await getHeadCommit(localConfig.repo.businessRepoRoot ?? localConfig.repo.localPath); + if (!defaultTip) return missing(teamItems); + const branchedFrom = (await git.raw(['merge-base', 'HEAD', defaultTip]).catch(() => '')).trim(); + const raw = await git.raw([ + 'diff', '--raw', '-z', '--no-renames', '--no-abbrev', '--relative', branchedFrom || 'HEAD', defaultTip, '--', '.teamai/', + ]); + const blobs = new Map<string, { atBase: string | null; atTip: string | null }>(); + const fields = raw.split('\0'); + const blobOrNull = (sha: string | undefined): string | null => (!sha || /^0+$/.test(sha) ? null : sha); + for (let i = 0; i + 1 < fields.length; i += 2) { + const [, , atBase, atTip] = fields[i].split(' '); + blobs.set(path.join(memberRoot, ...fields[i + 1].split('/')), { atBase: blobOrNull(atBase), atTip: blobOrNull(atTip) }); + } + const untouched: ResourceItem[] = []; + const edited: ResourceItem[] = []; + const retiring: ResourceItem[] = []; + for (const item of teamItems) { + const retired = supersededPathOf(item); + if (retired && blobs.get(teamFile(retired))?.atTip) { + retiring.push(item); + continue; + } + const changes = [...new Set([path.resolve(item.sourcePath), teamFile(item.relativePath)])] + .flatMap((file) => blobs.get(file) ?? []); + if (changes.length === 0) continue; + const memberBlob = await hashObject(memberRoot, item.sourcePath); + const behind = changes.filter((change) => change.atTip !== memberBlob); + if (behind.length === 0) continue; + (behind.every((change) => change.atBase === memberBlob) ? untouched : edited).push(item); + } + return branchedFrom + ? { history: 'complete', untouched, edited, retiring } + : missing([...untouched, ...edited, ...retiring]); +} + +/** + * Where the default branch moved under a self-mode checkout: the items to + * leave out of the push (announced here), and a check that stops the push, + * exit 2, when an item going out would overwrite or delete that branch's change. + */ +async function defaultBranchGuard( + localConfig: LocalConfig, + memberRoot: string, + items: readonly ResourceItem[], +): Promise<{ leftOut: readonly ResourceItem[]; stops: (going: readonly ResourceItem[]) => Promise<boolean> }> { + const changes = await itemsChangedOnDefaultBranch(localConfig, memberRoot, items); + const sourceOf = (item: ResourceItem): string => path.relative(memberRoot, item.sourcePath).split(path.sep).join('/'); + const named = (list: readonly ResourceItem[]): string => list.map((item) => { + const source = sourceOf(item); + const destination = `.teamai/${item.relativePath}`; + return source === destination ? source : `${source} (pushed to ${destination})`; + }).join(', '); + const leftOut = changes.history === 'complete' ? changes.untouched : []; + if (leftOut.length > 0) { + const defaultBranch = await getDefaultBranch(memberRoot); + log.info( + `Left out ${named(leftOut)}: changed on origin/${defaultBranch} since ` + + `this checkout branched from it, and not edited here. \`git pull origin ${defaultBranch}\` brings the change in.`, + ); + } + + // Like the destination errors in pushCore, this stops the push only for an + // item that is going out: a file left out by --skill, a filter or the + // selection must not hold back the rest. + const stops = async (going: readonly ResourceItem[]): Promise<boolean> => { + const goingOut = (list: readonly ResourceItem[]): ResourceItem[] => list.filter((item) => going.includes(item)); + const edited = goingOut(changes.history === 'complete' ? changes.edited : changes.unclassified); + const retiring = goingOut(changes.history === 'complete' ? changes.retiring : []); + if (edited.length === 0 && retiring.length === 0) return false; + const defaultBranch = await getDefaultBranch(memberRoot); + if (changes.history === 'missing') { + log.error(changes.shallow + ? `${named(edited)} differs from origin/${defaultBranch}, and this checkout lacks the history to tell ` + + 'whether that is your edit or a change on that branch, so pushing could overwrite a teammate\'s change. ' + + 'Nothing was pushed. Run `git fetch --unshallow origin`, then push again.' + : `${named(edited)} differs from origin/${defaultBranch}, and this checkout shares no history with ` + + `origin/${defaultBranch} to tell whether that is your edit or a change on that branch, so pushing could ` + + 'overwrite a teammate\'s change. Nothing was pushed. ' + + `Merge or rebase onto origin/${defaultBranch}, then push again.`); + } + if (changes.history === 'complete' && edited.length > 0) { + log.error(`${named(edited)} changed both here and on origin/${defaultBranch} since this checkout branched ` + + 'from it, so pushing would overwrite that change. Nothing was pushed. ' + + `Merge origin/${defaultBranch} (\`git pull origin ${defaultBranch}\`), resolve any conflict, then push again.`); + } + for (const item of retiring) { + log.error(`${named([item])} would delete .teamai/${supersededPathOf(item)}, which changed on ` + + `origin/${defaultBranch} since this checkout branched from it. Nothing was pushed. ` + + `Run \`git pull origin ${defaultBranch}\`, carry that change into ${sourceOf(item)}, then push again.`); + } + process.exitCode = 2; + return true; + }; + return { leftOut, stops }; +} + async function pushCore( localConfig: LocalConfig, teamConfig: TeamaiConfig, @@ -1537,7 +1665,7 @@ async function pushCore( // Preserve blocked items in the full scan so their pending PR records survive. // Exclude them before selection and grouping: pushItem cannot write their paths. - const allItems = fullScan.filter((item) => { + const listed = fullScan.filter((item) => { if ('skipReason' in item && typeof item.skipReason === 'string' && item.skipReason) { log.warn(`[${item.type}] Skipped ${item.name}: ${item.skipReason}`); return false; @@ -1549,6 +1677,12 @@ async function pushCore( return true; }); + const memberRoot = selfMode ? localConfig.projectRoot : undefined; + const branchGuard = memberRoot + ? await defaultBranchGuard(localConfig, memberRoot, listed) + : { leftOut: [], stops: async () => false }; + const allItems = listed.filter((item) => !branchGuard.leftOut.includes(item)); + // Such an agent is skipped like any other, and a skipped agent does not // block the rest of the push: a stale copy of an agent from a dropped role // must not stop an unrelated rule going out (#649 review). The error is the @@ -1945,6 +2079,7 @@ async function pushCore( // before this would let a dry run call a push viable that the real command // refuses — and say nothing about who the new resources reach. if (options.dryRun) { + if (await branchGuard.stops(allItems)) return; // Same two steps, same order as a real run: an open PR's recorded // destination first, then placement for whatever is still at the root. const settled = reuseRecordedDestinations(planPushGroups(allItems, reusablePending)); @@ -1971,6 +2106,7 @@ async function pushCore( } selectedItems = indices.map((i) => allItems[i]); } + if (await branchGuard.stops(selectedItems)) return; // ── Step 3b: Split the selection into per-PR groups ──────────────── // Resources that belong to an open PR are pushed by force-pushing that PR's