diff --git a/CHANGELOG.md b/CHANGELOG.md index 310f21856..95bcec4e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ 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)). +- `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/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/docs/guide/knowledge.md b/docs/guide/knowledge.md index c1191ee61..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 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`. `--namespace ""` writes to the shared root `learnings/`. To stop the prompt, set this in `teamai.yaml`: 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/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/docs/guide/zh-CN/knowledge.md b/docs/guide/zh-CN/knowledge.md index f39b349b0..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` ้‡Œๅฃฐๆ˜Ž็š„ learnings namespaceใ€‚ +`--namespace` ๅฟ…้กปๆ˜ฏๅฝ“ๅ‰ scope ็š„้กน็›ฎๅœจ `manifest/projects.yaml` ้‡Œใ€ๆˆ–ๅ…ถ่ง’่‰ฒๅœจ `manifest/roles.yaml` ้‡Œๅฃฐๆ˜Ž็š„ learnings namespaceใ€‚`--namespace ""` ๅ†™ๅ…ฅๅ…ฑไบซๆ น็›ฎๅฝ• `learnings/`ใ€‚ ไธๆƒณ่ฆ่ฟ™ๆกๆ้†’ๆ—ถ๏ผŒๅœจ `teamai.yaml` ้‡Œๅ†™๏ผš 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 b0277fa9b..b576cec4a 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.). 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 ## pull @@ -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/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/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__/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/__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__/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.'); + }); +}); diff --git a/src/__tests__/placement-records.test.ts b/src/__tests__/placement-records.test.ts index 1b83a93c4..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 <name> IS the team file @@ -60,6 +60,60 @@ 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 NonNullable<State['placements']>, + 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': { path: 'docs/ops/guide.md', blob } }); + 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 blob = run(['hash-object', 'docs/ops/guide.md']); + 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': { path: 'docs/ops/guide.md', blob } }, + 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__/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/__tests__/push-all-resources-namespaces.test.ts b/src/__tests__/push-all-resources-namespaces.test.ts new file mode 100644 index 000000000..b7615a4bb --- /dev/null +++ b/src/__tests__/push-all-resources-namespaces.test.ts @@ -0,0 +1,505 @@ +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'; +import { + declaredNamespaceFor, + isAtSharedRoot, + isPlaceableType, + NAMESPACE_AXIS, + PLACEABLE_TYPES, + placedResourcePath, + resolveProjectNamespace, + withNamespace, +} from '../push-namespaces.js'; +import type { ProjectsManifest } from '../projects.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[] = []; + +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: 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 }, + // 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 }, + // A scorer is scorers/<name>.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 }, + { 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 }, + { 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); + } + + 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(); + + // 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', () => { + 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.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'); + 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], 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'); + }); +}); + +/** + * 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' | 'env', authorPath: string, placedPath: string) { + 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', ...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<void> => { + 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); + }; + + 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 [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'); + + 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([]); + }); + + 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 scan(); + expect(edited).toHaveLength(1); + 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 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 () => { + 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); + 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 () => { + 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); + 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([]); + + 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/__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/__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); 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__/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/__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/<dir>/ 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> = {}): 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/__tests__/review-branch.test.ts b/src/__tests__/review-branch.test.ts index 1a53911c9..191a08131 100644 --- a/src/__tests__/review-branch.test.ts +++ b/src/__tests__/review-branch.test.ts @@ -48,6 +48,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<void> { const dir = path.join(repoDir, ...relativeDir.split('/')); await fse.ensureDir(dir); @@ -172,6 +188,22 @@ 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'), 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 === 'workflows/backend/deploy.yaml' + && /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'])); @@ -213,6 +245,145 @@ 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\.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'])); + 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\.md" 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/__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 <teamaiHome>/env (that path is the dir โ†’ EISDIR). It must diff --git a/src/__tests__/workflow-cache-namespaces.test.ts b/src/__tests__/workflow-cache-namespaces.test.ts new file mode 100644 index 000000000..dc017722f --- /dev/null +++ b/src/__tests__/workflow-cache-namespaces.test.ts @@ -0,0 +1,245 @@ +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('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/<ns>/<name>.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/__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/__tests__/workflow-records.test.ts b/src/__tests__/workflow-records.test.ts index b8c79b439..932f32788 100644 --- a/src/__tests__/workflow-records.test.ts +++ b/src/__tests__/workflow-records.test.ts @@ -203,7 +203,7 @@ 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', 'bad_workspace'] satisfies Array<WorkflowDiagnostic['code']>; + const diagnostics = ['unknown_key', 'missing_needs', 'false_if', 'bad_template', 'dotted_name', 'undeclared_output', 'undeclared_end', 'bad_loop', 'bad_workspace', 'workflow_namespace_conflict', 'prompt_namespace_conflict'] satisfies Array<WorkflowDiagnostic['code']>; 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/contribute.ts b/src/contribute.ts index d42650956..5373ba054 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<string> { - 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,13 +192,11 @@ 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 && + // 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 dc614de0c..38aaf9ed7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -190,10 +190,11 @@ program .description('Push local resources to team repo') .option('--all', 'Push all without confirmation') .option('--skill <path>', 'Push a specific skill by path (e.g., ~/.claude/skills/hai/my-skill or skills/hai_dev/my-skill)') - .option('--role <id>', 'Namespace for new skills, rules and agents (skills/<id>/, rules/<id>/, agents/<id>/). ' + .option('--role <id>', 'Namespace for every new resource type (skills/<id>/, rules/<id>/, docs/<id>/, env/<id>/, 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 <id>', "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 <name>', 'Push to this destination branch instead of a generated teamai/push branch') .action(async (cmdOpts) => { const globalOpts = program.opts() as GlobalOptions; @@ -1236,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) => { 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<HandDeclaredResourceType, z.ZodOptional<z.ZodArray<z.ZodType<string>>>>; /** 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<void> { 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 8717f4f59..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: @@ -18,7 +19,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/<id>/`, `learnings/<id>/`) just @@ -199,11 +202,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; @@ -231,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<LocalConfig['repo'], 'localPath'> } & Partial<Pick<LocalConfig, 'projects' | 'primaryRole' | 'additionalRoles'>>, ): Promise<string[]> { - 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<string[]> => { + 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<string[]> => { + 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()])]; } /** @@ -280,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/push-namespaces.ts b/src/push-namespaces.ts index 470a54a80..077140c2b 100644 --- a/src/push-namespaces.ts +++ b/src/push-namespaces.ts @@ -12,20 +12,49 @@ * 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 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 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']; +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): type is PlaceableType { - return (PLACEABLE_TYPES as readonly ResourceType[]).includes(type); +export function isPlaceableType(type: ResourceType | string): type is PlaceableType { + return (PLACEABLE_TYPES as readonly string[]).includes(type); } /** @@ -38,7 +67,36 @@ export const NAMESPACE_AXIS = { skills: 'skills', rules: 'knowledge', agents: 'agents', -} as const satisfies Record<PlaceableType, ProjectResourceType>; + 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<PlaceableType, AllProjectResourceType>; + +/** + * 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/<ns>/topic/note.md` in each active + * namespace and falls back to the shared `prompts/topic/note.md`. + */ +export const NESTED_PLACEABLE_TYPES: ReadonlySet<PlaceableType> = new Set<PlaceableType>(['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 @@ -48,17 +106,51 @@ export const NAMESPACE_AXIS = { * behind. */ export function isAtSharedRoot(item: ResourceItem): boolean { + if (item.namespace) return false; + // 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/prompts path, if any. */ +export function declaredNamespaceFor( + item: ResourceItem, + declaredNamespaces: readonly string[], +): string | 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; +} + /** * 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, 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; +} + +/** + * 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 withNamespace(relativePath: string, namespace: string): string { - const [root, ...rest] = relativePath.split('/'); - return [root, namespace, ...rest].join('/'); +export function namespaceOfPlacedPath(type: PlaceableType, placedPath: string): string | undefined { + return placedPath.split('/')[rootSegmentsFor(type)]; } /** @@ -77,6 +169,30 @@ export type NamespaceResolution = | { ok: true; namespace: string } | { ok: false; message: string }; +/** + * The namespace `--role <ns>` 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 <id>` places `type` in, read from that type's own * axis in `manifest/projects.yaml`. @@ -97,7 +213,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 c55a56072..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, @@ -18,24 +18,32 @@ 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'; 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 { - isAtSharedRoot, isPlaceableType, NAMESPACE_AXIS, PLACEABLE_TYPES, placedResourcePath, - resolveProjectNamespace, skillNamespacePath, withNamespace, type PlaceableType, + declaredNamespaceFor, isAtSharedRoot, isNestedPlaceableType, isPlaceableType, NAMESPACE_AXIS, PLACEABLE_TYPES, placedResourcePath, + 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'; 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<PlaceableType> = 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 @@ -108,7 +116,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` @@ -230,18 +238,17 @@ async function resolveNamespaceForNew( options: { role?: string; project?: string; silent?: boolean }, localConfig: LocalConfig, projectsManifest: ProjectsManifest | null, + roleNamespaces: readonly NamespaceEntry[] | null, ): Promise<NewResourceDestination> { - 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; @@ -546,13 +553,15 @@ function reuseRecordedDestinations(groups: PushGroup[]): Set<ResourceItem> { 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); + item.relativePath = withNamespace(item.relativePath, ns, rootSegmentsFor(item.type)); } } } @@ -576,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<ResourceItem>; }): Promise<boolean> { - 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 @@ -615,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); @@ -633,7 +644,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 @@ -656,7 +667,14 @@ 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 + // 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}`); } @@ -1037,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, @@ -1109,10 +1255,10 @@ 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.filter((t) => !LOCAL_TOOL_TYPES.has(t)).map((t) => getHandler(t).scanLocalForPush(teamConfig, localConfig)), + ); + repoScan.catch(() => {}); const [committed, status] = await Promise.all([ workingContent === null ? null : getFileContentAtRev(repoPath, 'HEAD', 'teamai.yaml'), git.status(), @@ -1124,8 +1270,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, @@ -1137,8 +1282,16 @@ async function pushCore( deleted: status.deleted, renamed: status.renamed, })); - for (const item of await envScan) { + 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 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); @@ -1237,6 +1390,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 @@ -1374,7 +1548,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 @@ -1384,15 +1558,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; @@ -1443,6 +1615,41 @@ async function pushCore( } } + // 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<ReturnType<typeof resolveResourceNamespaces>> = null; + try { + resolvedNamespaces = await resolveResourceNamespaces(localConfig); + } catch (e) { + log.error(`${(e as Error).message}`); + process.exitCode = 2; + return; + } + if (resolvedNamespaces) { + const declaredByType: Partial<Record<PlaceableType, string[]>> = { + docs: [ + ...(resolvedNamespaces.activeNamespaces.docs ?? []), + ...resolvedNamespaces.inactiveDocsNamespaces, + ], + wiki: [ + ...(resolvedNamespaces.activeNamespaces.wiki ?? []), + ...resolvedNamespaces.inactiveWikiNamespaces, + ], + prompts: [ + ...(resolvedNamespaces.activeNamespaces.prompts ?? []), + ...resolvedNamespaces.inactivePromptsNamespaces, + ], + }; + for (const item of fullScan) { + if (!isPlaceableType(item.type) || !isNestedPlaceableType(item.type)) 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 @@ -1458,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; @@ -1470,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 @@ -1653,7 +1866,12 @@ async function pushCore( process.exitCode = 2; return; } - skillsDestination = options.role; + const resolved = resolveRoleNamespace(roleNamespaces, options.role, 'skills'); + if (resolved.ok) { + skillsDestination = resolved.namespace; + } else { + skillsDestinationError = resolved.message; + } } else if (options.project && projectsManifest && allItems.some(isRelocatedByFlag)) { const resolved = resolveProjectNamespace(projectsManifest, options.project, 'skills'); if (resolved.ok) { @@ -1861,11 +2079,12 @@ 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)); 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'); @@ -1887,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 @@ -1907,7 +2127,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 โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ @@ -1957,7 +2177,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) { @@ -2091,3 +2317,4 @@ async function pushTeamConfigOnly( return; } } + diff --git a/src/resource-namespaces.ts b/src/resource-namespaces.ts index 0fb4263f2..f60430a49 100644 --- a/src/resource-namespaces.ts +++ b/src/resource-namespaces.ts @@ -40,6 +40,8 @@ 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)), + inactiveScorersNamespaces: inAll(resolved.map((one) => one.inactiveScorersNamespaces)), }; } @@ -166,7 +168,34 @@ 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/<dir>/ 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<string>([ + ...(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)); + + // Scorers follow the prompts rule. Nothing reads scorers/<ns>/ on pull yet + // (#1085); this is the set that reader will withhold. + const declaredScorersNamespaces = new Set<string>([ + ...(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, + }; } /** 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<ResourceItem[]> { - // Docs are managed directly in team repo - return []; + async scanLocalForPush(teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise<ResourceItem[]> { + return scanRepoResourceForPush('docs', 'docs', teamConfig, localConfig); } async scanTeamForPull(_teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise<ResourceItem[]> { @@ -640,8 +640,8 @@ export class DocsHandler extends ResourceHandler { return (await listDocFiles(sourcePath)).length; } - async pushItem(_item: ResourceItem, _teamConfig: TeamaiConfig, _localConfig: LocalConfig): Promise<void> { - // No-op + async pushItem(item: ResourceItem, _teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise<void> { + await pushRepoResourceItem(item, localConfig); } /** diff --git a/src/resources/env.ts b/src/resources/env.ts index 5dd3e98e0..d8b10ffc1 100644 --- a/src/resources/env.ts +++ b/src/resources/env.ts @@ -3,7 +3,9 @@ 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 { 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'; @@ -285,8 +287,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. */ @@ -339,11 +341,26 @@ 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. - 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; + if (!baseExists) { + // 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; + } + } + items.push(envPushItem(relativePath, activeEnv, baseExists ? 'modified' : 'new')); } return items; } @@ -353,17 +370,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<string> | null = null; + let untracked: Set<string> = 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 +397,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; } @@ -389,20 +416,28 @@ export class EnvHandler extends ResourceHandler { } async pushItem(item: ResourceItem, _teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise<void> { - // 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/<ns>/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 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(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/<ns>/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/resources/hooks.ts b/src/resources/hooks.ts index f726230c3..ed22e50a4 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, @@ -258,9 +259,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<ResourceItem[]> { - return []; + async scanLocalForPush(teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise<ResourceItem[]> { + return scanRepoResourceForPush('hooks', 'hooks', teamConfig, localConfig); } async scanTeamForPull(_teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise<ResourceItem[]> { @@ -269,8 +269,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<void> { - // No-op โ€” hooks.yaml is edited directly in the repo; push.ts handles git commit. + async pushItem(item: ResourceItem, _teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise<void> { + await pushRepoResourceItem(item, localConfig); } async pullItem(_item: ResourceItem, _teamConfig: TeamaiConfig, _localConfig: LocalConfig): Promise<void> { diff --git a/src/resources/index.ts b/src/resources/index.ts index ba7bf297e..f94b89225 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<ResourceType, ResourceHandler> = { @@ -16,6 +17,13 @@ const handlers: Record<ResourceType, ResourceHandler> = { 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', 'teamwiki/evidence/code'), + 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<ResourceItem[]> { - return []; + async scanLocalForPush(teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise<ResourceItem[]> { + return scanRepoResourceForPush('mcp', 'mcp', teamConfig, localConfig); } /** @@ -156,8 +153,8 @@ export class McpHandler extends ResourceHandler { return items; } - async pushItem(): Promise<void> { - // Servers live in a single mcp.yaml committed directly; nothing per-item to copy. + async pushItem(item: ResourceItem, _teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise<void> { + await pushRepoResourceItem(item, localConfig); } async pullItem(): Promise<void> { diff --git a/src/resources/repo-resource.ts b/src/resources/repo-resource.ts new file mode 100644 index 000000000..af0052ce2 --- /dev/null +++ b/src/resources/repo-resource.ts @@ -0,0 +1,235 @@ +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 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<string[]> { + 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 []; + } +} + +/** + * 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<State['placements']>, +): Promise<ResourceItem | 'delivered' | null> { + 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. Merge your edit into ${record.path}, keeping the team's change there, then 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, + _teamConfig: TeamaiConfig, + localConfig: LocalConfig, + fileFilter?: (file: string) => boolean, +): Promise<ResourceItem[]> { + 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[] = []; + // 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}`; + const atBaseline = await pathExists(baseFile); + if (atBaseline && await fileContentEqual(activeFile, baseFile)) continue; + if (!atBaseline) { + const placed = await placedCopy(type, rel, relativePath, activeFile, localConfig, placements); + if (placed === 'delivered') continue; + if (placed) { + items.push(placed); + continue; + } + } + items.push({ + name: rel, + type, + sourcePath: activeFile, + relativePath, + status: atBaseline ? 'modified' : 'new', + }); + } + return items; + } + + const repoPath = localConfig.repo.localPath; + const targetDir = path.join(repoPath, dir); + if (!await pathExists(targetDir)) return []; + + let changedFiles: Set<string> | null = null; + let untrackedFiles: Set<string> = 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, + localConfig: LocalConfig, +): Promise<void> { + const dest = path.resolve(localConfig.repo.localPath, ...item.relativePath.split('/')); + const originalSource = path.resolve(item.sourcePath); + if (originalSource !== dest) { + await ensureDir(path.dirname(dest)); + // 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 }); + } + } +} + +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<ResourceItem[]> { + return scanRepoResourceForPush(this.type, this.dir, teamConfig, localConfig, this.fileFilter); + } + + async scanTeamForPull( + _teamConfig: TeamaiConfig, + localConfig: LocalConfig, + ): Promise<ResourceItem[]> { + 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<void> { + await pushRepoResourceItem(item, localConfig); + } + + async pullItem( + _item: ResourceItem, + _teamConfig: TeamaiConfig, + _localConfig: LocalConfig, + ): Promise<void> { + // Pull synchronization is orchestrated by high-level sync routines in pull.ts and workflow cache. + } + + async removeItem( + name: string, + _teamConfig: TeamaiConfig, + localConfig: LocalConfig, + ): Promise<string[]> { + 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 1946ef55f..c96a4089e 100644 --- a/src/review-branch.ts +++ b/src/review-branch.ts @@ -28,12 +28,14 @@ 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 { NAMESPACE_AXIS, type PlaceableType } from './push-namespaces.js'; +import { ALL_PROJECT_RESOURCE_TYPES, loadProjectsManifest, type AllProjectResourceType, type ProjectsManifest } from './projects.js'; +import { + isNestedPlaceableType, NAMESPACE_AXIS, PLACEABLE_TYPES, rootSegmentsFor, 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 { 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'; import { LocalConfigSchema, type GlobalOptions, type LocalConfig, type ResourceItem, type TeamaiConfig } from './types.js'; import { splitFrontmatter } from './utils/frontmatter.js'; @@ -71,7 +73,7 @@ export interface BranchReview { findings: Finding[]; } -const AXES: readonly ProjectResourceType[] = ['knowledge', 'skills', 'learnings', 'agents']; +const AXES: readonly AllProjectResourceType[] = ALL_PROJECT_RESOURCE_TYPES; // โ”€โ”€โ”€ Resource model โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ @@ -96,54 +98,144 @@ function stripExtension(segment: string): string { * them (`<root>/<ns>/<name>` is namespaced, `<root>/<name>` is shared). The * handlers' own `name` is not used: for rules it carries the namespace segment. */ -function parseRelativePath(relativePath: string): { name: string; namespace?: string } { - const rest = relativePath.split('/').slice(1); +function parseRelativePath( + type: PlaceableType, + relativePath: string, + index: ManifestIndex, +): { name: string; namespace?: string } { + const rootSegs = rootSegmentsFor(type); + const all = relativePath.split('/'); + const rest = all.slice(rootSegs); if (rest.length <= 1) { - return { name: stripExtension(rest[0] ?? '') }; + const only = rest[0] ?? ''; + return { name: isNestedPlaceableType(type) ? only : stripExtension(only) }; } - return { - name: stripExtension(rest[rest.length - 1] ?? ''), - namespace: rest[0], - }; + const namespace = rest[0]; + // 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 (isNestedPlaceableType(type)) { + if (index.declares(NAMESPACE_AXIS[type], namespace!)) { + return { name: rest.slice(1).join('/'), namespace }; + } + return { name: rest.join('/') }; + } + // A first segment under the other roots is a namespace by construction + // (`skills/<ns>/<name>`), 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. */ -async function scanResources(repoDir: string): Promise<ReviewResource[]> { - // `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<ReviewResource[]> { 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 rootRel = repoRootFor(type); + const root = path.join(repoDir, ...rootRel.split('/')); + for (const file of await listRepoFilesRecursive(root)) { + if (type === 'workflows' && !file.endsWith('.yaml')) continue; + if (type === 'hooks') { + // Hooks are reviewed by entry, the same identity pull's hooks reader + // delivers by: hooks/a/hooks.yaml and hooks/b/hooks.yaml are two files + // that may define disjoint ids, so the file name is not the identity. + const absolutePath = path.join(root, ...file.split('/')); + const relativePath = `${rootRel}/${file}`; + const segments = file.split('/'); + const namespace = segments.length > 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: `${rootRel}/${file}`, + }, + }); + } + } return items.map(({ type, item }) => { - const { name, namespace } = parseRelativePath(item.relativePath); + // Handler-provided names (MCP server names, skill names) are authoritative; + // parseRelativePath would overwrite them with the filename stem (e.g. 'mcp' + // from mcp/<ns>/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. 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, + 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 }; }); } +/** + * The repo-relative directory a resource type lives under. It is the type name + * everywhere except wiki, whose authoritative root is `teamwiki/evidence/code` + * (three segments, as `rootSegmentsFor` counts), not `wiki/`. + */ +function repoRootFor(type: PlaceableType): string { + return type === 'wiki' ? 'teamwiki/evidence/code' : type; +} + // โ”€โ”€โ”€ Manifest index โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ /** 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; + /** True when no manifest exists at all โ€” every on-disk namespace is valid. */ + manifestFree: boolean; } function buildManifestIndex(roles: RolesManifest | null, projects: ProjectsManifest | null): ManifestIndex { - const listsByAxis = new Map<ProjectResourceType, string[][]>(); + const listsByAxis = new Map<AllProjectResourceType, string[][]>(); for (const axis of AXES) listsByAxis.set(axis, []); - const collect = (resources: Partial<Record<ProjectResourceType, string[]>>): void => { + const collect = (resources: Record<string, unknown> | 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); @@ -164,6 +256,7 @@ function buildManifestIndex(roles: RolesManifest | null, projects: ProjectsManif }); }, anyOn: (axis) => listsByAxis.get(axis)!.length > 0, + manifestFree: !roles && !projects, }; } @@ -210,7 +303,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', @@ -243,12 +338,16 @@ async function namingFindings(resources: ReviewResource[], readText: (p: string) message: `Namespace "${resource.namespace}" in ${resource.relativePath} is not a usable path segment: ${NAMESPACE_RULE}.`, }); } - if (!isSafeNamespaceSegment(resource.name)) { + // A name may be a multi-segment sub-path (docs and wiki keep their + // bundle-relative path below the root or namespace, e.g. 'api/guide'), so + // each segment is validated on its own rather than the joined string. + const badSegment = resource.name.split('/').find((segment) => !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); @@ -380,6 +479,9 @@ const MAX_SCANNED_FILE_BYTES = 512 * 1024; async function leakFindings(resources: ReviewResource[], readText: (p: string) => Promise<string | null>): Promise<Finding[]> { 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<string>(); for (const resource of resources) { const files = resource.type === 'skills' ? (await listFilesRecursive(resource.absolutePath)).map((rel) => ({ @@ -389,6 +491,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; @@ -463,7 +567,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[] = [ @@ -483,8 +587,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 = ['skills', 'rules', 'agents', 'manifest', 'workflows', 'prompts', 'scorers']; +/** + * 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 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<typeof RoleSchema>; export type RolesManifest = z.infer<typeof RolesManifestSchema>; /** - * 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<RoleResourceType | 'learnings', string[]> // 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<string>(); const out: string[] = []; for (const role of resolvedRoles) { - for (const namespace of role.resources[type] ?? []) { + for (const namespace of (role.resources as Record<string, string[] | undefined>)[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<void> { 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/types.ts b/src/types.ts index b42a5b062..8b66f368f 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 @@ -1001,6 +1008,10 @@ export type RecordedAgentModel = z.infer<typeof RecordedAgentModelSchema>; /** `RecordedAgentModel` by agent stem, then tool. */ export type AgentModelRecords = Record<string, Record<string, RecordedAgentModel>>; +/** 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<typeof PlacementRecordSchema>; + export const StateSchema = z.object({ lastPush: z.string().nullable().default(null), lastPull: z.string().nullable().default(null), @@ -1109,6 +1120,26 @@ 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 `<type>:<path the author edits>`, e.g. + * `{ "docs:docs/guide.md": { "path": "docs/ops/guide.md", "blob": "<git blob id>" } }`. + * `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. 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.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 @@ -1177,9 +1208,21 @@ export interface TagsConfig { rules: Record<string, string[]>; } -// โ”€โ”€โ”€ 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'; @@ -1192,6 +1235,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 { @@ -1526,7 +1571,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 = '<!-- [teamai:rules:start] -->'; export const TEAMAI_RULES_END = '<!-- [teamai:rules:end] -->'; diff --git a/src/utils/pending-push.ts b/src/utils/pending-push.ts index 28a4d4681..00bef8c76 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,22 @@ 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 `<root>/<ns>/`, 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.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 +321,7 @@ function sharedRootPaths(root: 'rules' | 'agents', name: string): string[] { */ export async function reconcilePlacementRecords( repoPath: string, - state: Pick<State, 'placedRules' | 'placedAgents' | 'pendingPushes' | 'placementsCheckedAt' | 'retiredPlacedAgents'>, + state: Pick<State, 'placedRules' | 'placedAgents' | 'pendingPushes' | 'placementsCheckedAt' | 'retiredPlacedAgents' | 'placements'>, tip?: string, deliversEveryNamespace: () => Promise<boolean> = async () => false, ): Promise<boolean> { @@ -330,12 +349,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 +397,20 @@ 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]: { path: item.relativePath, blob: item.blob } }; + 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 // the path โ€” the blob stays in history, so the check above would still @@ -429,11 +465,43 @@ 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: NonNullable<State['placements']> = {}; + let dropped = false; + for (const [key, recorded] of Object.entries(state.placements)) { + if (!await exists(recorded.path)) { + log.debug(`Dropping placement record ${key} โ†’ ${recorded.path}: 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.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; + } + kept[key] = recorded; + } + 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 // 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; diff --git a/src/workflow/cache.ts b/src/workflow/cache.ts index 90961c8c2..7c454912b 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,133 @@ export async function syncWorkflowDefinitions( // Asked on the first valid file: a team without workflows starts no git here. let reachable: Promise<ReachableRevision | undefined> | undefined; 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 + } + + // 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 + // 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 + // workflows/ tree with prompts/<ns>/ 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<ReturnType<typeof resolveResourceNamespaces>> = null; + if (hasNamespaced) { + try { + resourceNamespaces = await resolveResourceNamespaces(config); + } catch { + // 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<ReturnType<typeof resolveResourceNamespaces>>; + } + } + 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<string>(); + 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] + ))), + ]; + + const byStem = new Map<string, Array<{ stem: string; relativePath: string; full: string; namespace: string | null }>>(); + 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) { + 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, effectiveActiveWorkflows); + 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<ReturnType<typeof fs.lstat>>; try { stat = await fs.lstat(full); @@ -202,7 +329,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 +339,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, effectiveActivePrompts); if (!resolved.ok) { checked.push({ stem, relativePath, diagnostics: resolved.diagnostics }); continue; @@ -423,6 +549,7 @@ async function resolvePromptFiles( definition: WorkflowDefinition, locations: Record<string, { line: number; column: number }>, workflowFile: string, + activePromptsNamespaces: readonly string[] | null = null, ): Promise< | { ok: true; definition: WorkflowDefinition; sources: Array<{ path: string; content: string }> } | { ok: false; diagnostics: WorkflowDiagnostic[] } @@ -439,8 +566,45 @@ 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); + // Namespace lookup works for any leaf depth โ€” a nested path like + // prompts/topic/note.md resolves to prompts/<ns>/topic/note.md + // just as a flat prompts/note.md resolves to prompts/<ns>/note.md. + // + // A leaf whose first segment is already an active namespace is explicit + // and resolved as written: prepending another namespace would look up + // prompts/<other>/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; + }), + )).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); @@ -460,7 +624,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 d3ba579c9..b1d0d61df 100644 --- a/src/workflow/types.ts +++ b/src/workflow/types.ts @@ -173,7 +173,7 @@ export interface WorkflowDiagnostic { * `bad_session`: a `session.from` pull rejects; the kept copy would resume a step the author did not name. * `bad_workspace`: `workspace` is not read-only, new, or continue; the kept copy would run the step in the wrong place. */ - code?: 'unknown_key' | 'missing_needs' | 'false_if' | 'bad_template' | 'dotted_name' | 'undeclared_output' | 'undeclared_end' | 'bad_loop' | 'bad_session' | 'bad_workspace'; + code?: 'unknown_key' | 'missing_needs' | 'false_if' | 'bad_template' | 'dotted_name' | 'undeclared_output' | 'undeclared_end' | 'bad_loop' | 'bad_session' | 'bad_workspace' | 'workflow_namespace_conflict' | 'prompt_namespace_conflict'; } export type StepOutcome = 'success' | 'failed' | 'released'; 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<readonly Lo export async function deliveredLearningsNamespaces(config: LocalConfig): Promise<string[]> { const namespaces = new Set<string>(); 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); } }