diff --git a/CHANGELOG.md b/CHANGELOG.md index fba2c6da..a0b7d2e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ All notable changes to this project will be documented in this file. See [standa ### 🐛 Bug Fixes - `teamai init` no longer hangs without a terminal. When the provider had no session it spawned `gh auth login --web` (or `gf auth login`, `cnb login`) with inherited stdio and waited for a browser device flow that nobody could complete, about five minutes for GitHub, then exited with the provider's error and no hint of the missing credential. Each login now refuses up front when the run is not interactive and names the credential to prepare (`GITHUB_TOKEN` / `GH_TOKEN`, `CNB_TOKEN`, or for TGit a prior `gf auth login`, since a `TGIT_TOKEN` PAT is REST-API-only and cannot clone). A run is non-interactive when stdin is not a TTY or when `CI` or `TEAMAI_NONINTERACTIVE` is set, so an agent sandbox with a pseudo-terminal can declare itself unattended, and every prompt in the CLI follows the same rule. `git` also runs with its prompts closed in that case — `GIT_TERMINAL_PROMPT=0`, `GIT_ASKPASS=echo` and `GCM_INTERACTIVE=never`, each only where the caller set nothing — so a missing clone credential fails at once instead of waiting on a terminal prompt or an askpass or credential-manager dialog. `ssh` keeps its own settings: its batch flag is only reachable through `GIT_SSH_COMMAND`, which would override each repository's `core.sshCommand` (for [#711](https://github.com/Tencent/teamai-cli/issues/711)). +- `teamai members list` and `teamai projects members` read the roster registered before the reports switch, so a team upgrading past the orphan-branch split no longer sees "No team members registered" while its `members/` still lives on the default branch. The default-branch copy becomes a read-only inherited root, the way learnings' already was: listed in union with the `teamai-reports` copy, with the branch copy winning when the same file exists on both; nothing is copied or deleted, and a cold `members list` still does not publish the reports branch. Member registration merges against the inherited copy too, so a re-init keeps the original `registeredAt` and projects. Fixes [#735](https://github.com/Tencent/teamai-cli/issues/735). - Cache GC now rejects partial integers such as `12abc`, decimals, zero and unsafe integers for `--max-bytes` and `--stale-days` before deleting anything. An invalid `TEAMAI_CACHE_MAX_BYTES` value falls back to the default 5 GB limit instead of using a numeric prefix. - Usage reporting scopes sessions by path on Windows too. The project/user scope filter compared an event's `cwd` against `projectRoot` with a hard-coded `/` separator, so on Windows only a session started in the project root itself matched: every session started in a subdirectory was dropped from the project team's report and counted in the user scope's instead, which is the isolation the usage guide promises. Windows paths are also compared case-insensitively, so a drive letter or a directory name spelled with different case in the two sources no longer leaks a project session into the user scope. POSIX paths keep their own rules: case-sensitive, and a `\` in a filename stays part of the name. - `teamai tags subscribe` and `teamai tags unsubscribe` now invalidate the pull revision cache, as `teamai skill exclude` already does, so the next `teamai pull` applies the new subscriptions instead of reporting "Already synced" when the team repo has not changed. diff --git a/docs/usage-guide.md b/docs/usage-guide.md index 0718a285..5fbe1bf6 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -144,7 +144,7 @@ Resulting directory structure: └── reports-wt/ # checkout of the `teamai-reports` orphan branch ``` -Independent git clones use the same split as single-repo mode: `members/` `sessions/` `votes/` `stats/` go to the `teamai-reports` orphan branch and `learnings/` goes to `teamai-learnings` (both checkouts sit **beside** the clone, not inside it). Knowledge (`skills/` `rules/` `docs/` `teamai.yaml`) stays on the default branch, reached by pull request. Report files and learnings already on `main` are left in place: reports are ignored from then on, learnings keep being read. +Independent git clones use the same split as single-repo mode: `members/` `sessions/` `votes/` `stats/` go to the `teamai-reports` orphan branch and `learnings/` goes to `teamai-learnings` (both checkouts sit **beside** the clone, not inside it). Knowledge (`skills/` `rules/` `docs/` `teamai.yaml`) stays on the default branch, reached by pull request. Report files and learnings already on `main` are left in place: `members/` keeps being read from the default-branch copy as a read-only inherited root (nothing copied or deleted; when the same file exists on both, the branch copy wins), other reports are ignored from then on, and learnings keep being read. In both modes, commands that only read reports (`members`, `digest`, `projects members`, `stats`, `viz`) never create or push the `teamai-reports` branch. `teamai pull` refreshes the reports checkout from `origin` before it rebuilds the search index (vote hotness) and skill recommendations. Report writers (session save `--push`, Stop-hook votes, member registration, auto-report) merge into origin's latest copy of the member's file first, so the same member reporting from two machines does not lose a session, vote, or stats entry. diff --git a/docs/usage-guide.zh-CN.md b/docs/usage-guide.zh-CN.md index 5aa56d40..5115e758 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -143,7 +143,7 @@ teamai init https://github.com/yourorg/yourrepo └── reports-wt/ # `teamai-reports` 孤儿分支的检出 ``` -独立 git clone 与单仓模式使用同一套拆分:`members/` `sessions/` `votes/` `stats/` 写到 `teamai-reports` 孤儿分支,`learnings/` 写到 `teamai-learnings`(两个检出目录都在 clone **旁边**,不嵌在 clone 里)。知识资产(`skills/` `rules/` `docs/` `teamai.yaml`)仍在默认分支,通过 PR 写入。默认分支上已有的上报文件与 learnings 都留在原地:上报数据从此被忽略,learnings 仍会被读取。 +独立 git clone 与单仓模式使用同一套拆分:`members/` `sessions/` `votes/` `stats/` 写到 `teamai-reports` 孤儿分支,`learnings/` 写到 `teamai-learnings`(两个检出目录都在 clone **旁边**,不嵌在 clone 里)。知识资产(`skills/` `rules/` `docs/` `teamai.yaml`)仍在默认分支,通过 PR 写入。默认分支上已有的上报文件与 learnings 都留在原地:`members/` 仍会从默认分支副本读取(只读继承根,不复制也不删除;同一文件两处都有时以分支副本为准),其余上报数据从此被忽略,learnings 仍会被读取。 两种模式下,只读取上报数据的命令(`members`、`digest`、`projects members`、`stats`、`viz`)都不会创建或推送 `teamai-reports` 分支。`teamai pull` 在重建检索索引(投票热度)和技能推荐之前,会先从 `origin` 刷新上报检出。写入上报(`session save --push`、Stop hook 投票、成员注册、自动上报)会先合并 `origin` 上该成员文件的最新副本,因此同一成员在两台机器上报时不会丢掉会话、投票或统计。 diff --git a/src/__tests__/e2e/members-legacy-roster.test.ts b/src/__tests__/e2e/members-legacy-roster.test.ts new file mode 100644 index 00000000..ffb2459d --- /dev/null +++ b/src/__tests__/e2e/members-legacy-roster.test.ts @@ -0,0 +1,179 @@ +/** + * Built-CLI repro for #735: a team whose roster was committed to the default + * branch by a pre-#489 CLI loses every member from `members list` once the CLI + * switches to the teamai-reports orphan branch. The default-branch copy must + * stay readable as an inherited root (the learnings pattern, #485): listed, + * never copied, never published. + * + * Real CLI, real git: a local bare origin, an isolated HOME. No mocks — the + * bug only surfaces through the real members-list wiring. + */ +import { afterEach, 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 YAML from 'yaml'; + +const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../../..'); +const cli = path.join(root, 'dist/index.js'); +let sandbox: string; + +function git(args: string[], cwd: string, env: NodeJS.ProcessEnv): string { + return execFileSync('git', args, { cwd, env, encoding: 'utf8', windowsHide: true }); +} + +function machineEnv(home: string): NodeJS.ProcessEnv { + return { + ...process.env, + HOME: home, + USERPROFILE: home, + XDG_CONFIG_HOME: path.join(home, '.config'), + GIT_CONFIG_GLOBAL: path.join(home, '.gitconfig'), + GIT_CONFIG_NOSYSTEM: '1', + GIT_AUTHOR_NAME: 'member', + GIT_AUTHOR_EMAIL: 'member@example.invalid', + GIT_COMMITTER_NAME: 'member', + GIT_COMMITTER_EMAIL: 'member@example.invalid', + GIT_TERMINAL_PROMPT: '0', + FORCE_COLOR: '0', + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'protocol.file.allow', + GIT_CONFIG_VALUE_0: 'always', + }; +} + +function runCli(home: string, args: string[], cwd: string): Promise<{ code: number | null; output: string }> { + const env = machineEnv(home); + return new Promise((resolve, reject) => { + const child = spawn(process.execPath, [cli, ...args], { + cwd, + env, + windowsHide: true, + stdio: ['ignore', 'pipe', 'pipe'], + }); + let output = ''; + const timer = setTimeout(() => { + child.kill(); + reject(new Error(`CLI timed out\n${output}`)); + }, 45_000); + const capture = (data: Buffer) => { output += data.toString(); }; + child.stdout.on('data', capture); + child.stderr.on('data', capture); + child.on('error', (error) => { clearTimeout(timer); reject(error); }); + child.on('close', (code) => { + clearTimeout(timer); + resolve({ code, output }); + }); + }); +} + +async function originBranches(origin: string, env: NodeJS.ProcessEnv): Promise { + return (await execFileSync('git', ['for-each-ref', '--format=%(refname:short)', 'refs/heads'], { + cwd: origin, env, encoding: 'utf8', windowsHide: true, + })).split('\n').filter(Boolean); +} + +afterEach(() => { + if (sandbox && path.dirname(sandbox) === os.tmpdir() && path.basename(sandbox).startsWith('teamai-members-')) { + fs.rmSync(sandbox, { recursive: true, force: true }); + } +}); + +describe('real CLI inherited member roster (#735)', () => { + it('lists the pre-switch roster from the default branch, then the union once reports exist', async () => { + if (!fs.existsSync(cli)) { + throw new Error(`CLI binary not found at ${cli}. Run "npm run build" first.`); + } + + sandbox = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-members-')); + const origin = path.join(sandbox, 'origin.git'); + const seed = path.join(sandbox, 'seed'); + const home = path.join(sandbox, 'home-alice'); + const env = machineEnv(home); + + // A pre-#489 team: knowledge AND the roster live on the default branch. + fs.mkdirSync(path.join(seed, 'members'), { recursive: true }); + fs.writeFileSync(path.join(seed, 'teamai.yaml'), YAML.stringify({ + team: 'acme', + repo: origin, + provider: 'git', + usageReport: false, + })); + fs.writeFileSync(path.join(seed, 'members', 'carol.yaml'), YAML.stringify({ + username: 'carol', + displayName: 'Carol Pre', + registeredAt: '2025-01-01T00:00:00.000Z', + })); + fs.writeFileSync(path.join(seed, 'members', 'dan.yaml'), YAML.stringify({ + username: 'dan', + registeredAt: '2025-01-02T00:00:00.000Z', + })); + git(['init', '-q', '-b', 'main'], seed, env); + git(['add', '.'], seed, env); + git(['commit', '-q', '-m', 'fixture'], seed, env); + git(['clone', '-q', '--bare', seed, origin], sandbox, env); + + // Alice's machine: clone + local config, no reports branch anywhere yet. + const clone = path.join(home, '.teamai', 'team-repo'); + fs.mkdirSync(path.join(home, '.claude'), { recursive: true }); + fs.writeFileSync( + path.join(home, '.gitconfig'), + ['[user]', '\tname = alice', '\temail = alice@example.invalid', '[protocol "file"]', '\tallow = always', ''].join('\n'), + ); + git(['clone', '-q', origin, clone], sandbox, env); + git(['config', 'user.name', 'alice'], clone, machineEnv(home)); + git(['config', 'user.email', 'alice@example.invalid'], clone, machineEnv(home)); + git(['config', 'protocol.file.allow', 'always'], clone, machineEnv(home)); + fs.writeFileSync(path.join(home, '.teamai', 'config.yaml'), YAML.stringify({ + repo: { localPath: clone, remote: origin, kind: 'git' }, + username: 'alice', + scope: 'user', + updatePolicy: 'skip', + enabledAgents: ['claude'], + additionalRoles: [], + })); + + // The #735 repro: the roster exists on main, the reports branch does not. + const list1 = await runCli(home, ['members', 'list'], sandbox); + expect(list1.code, list1.output).toBe(0); + expect(list1.output).toContain('Team members (2)'); + expect(list1.output).toContain('carol'); + expect(list1.output).toContain('Carol Pre'); + expect(list1.output).toContain('dan'); + // Read-only: listing must not publish a reports branch... + expect(await originBranches(origin, env)).toEqual(['main']); + // ...and must not copy the roster into the reports worktree. + const wt = path.join(home, '.teamai', 'reports-wt'); + expect(fs.existsSync(path.join(wt, 'members', 'carol.yaml'))).toBe(false); + expect(fs.existsSync(path.join(clone, 'members', 'carol.yaml'))).toBe(true); + + // A teammate registers with the new CLI: origin grows a teamai-reports + // branch carrying bob plus a newer copy of carol than the one on main. + const reports = path.join(sandbox, 'reports-seed'); + fs.mkdirSync(path.join(reports, 'members'), { recursive: true }); + fs.writeFileSync(path.join(reports, 'members', 'bob.yaml'), YAML.stringify({ + username: 'bob', + registeredAt: '2025-06-01T00:00:00.000Z', + })); + fs.writeFileSync(path.join(reports, 'members', 'carol.yaml'), YAML.stringify({ + username: 'carol', + displayName: 'Carol Branch', + registeredAt: '2025-01-01T00:00:00.000Z', + })); + git(['init', '-q', '-b', 'teamai-reports'], reports, env); + git(['add', '.'], reports, env); + git(['commit', '-q', '-m', 'register bob'], reports, env); + git(['push', '-q', origin, 'teamai-reports'], reports, env); + + // Alice now sees the union, with the reports-branch copy winning for carol. + const list2 = await runCli(home, ['members', 'list'], sandbox); + expect(list2.code, list2.output).toBe(0); + expect(list2.output).toContain('Team members (3)'); + expect(list2.output).toContain('bob'); + expect(list2.output).toContain('dan'); + expect(list2.output).toContain('Carol Branch'); + expect(list2.output).not.toContain('Carol Pre'); + }, 90_000); +}); diff --git a/src/__tests__/git-kind-reports.test.ts b/src/__tests__/git-kind-reports.test.ts index 3edc24c9..9e4106e2 100644 --- a/src/__tests__/git-kind-reports.test.ts +++ b/src/__tests__/git-kind-reports.test.ts @@ -2,7 +2,7 @@ * Real-git coverage for independent clones writing reports to teamai-reports. * No mocks of the units under test. */ -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; @@ -12,6 +12,7 @@ import { getReportsDir, REPORTS_WORKTREE_DIRNAME, type LocalConfig } from '../ty import { commitAndPushReports, ensureReportsWorktree, refreshReportsWorktree, updateReports } from '../utils/reports-branch.js'; import { pushRepoDirectly } from '../utils/git.js'; import { reportUsageToTeam } from '../team-push.js'; +import { listMembers } from '../members.js'; let tmp: string; let originalHome: string; @@ -123,7 +124,7 @@ describe('git-kind reports branch', () => { expect(fs.existsSync(path.join(clone, 'stats', 'alice.yaml'))).toBe(false); }); - it('ignores leftover default-branch members after the switch and does not copy or delete them', async () => { + it('does not copy or delete leftover default-branch members when writing reports', async () => { const { origin, clone } = await seedBareOrigin(); const leftover = path.join(clone, 'members', 'stale.yaml'); fs.mkdirSync(path.dirname(leftover), { recursive: true }); @@ -430,6 +431,96 @@ describe('git-kind reports: refresh before reading (#557)', () => { }); }); +describe('git-kind reports: inherited member root (#735)', () => { + let consoleSpy: ReturnType; + + beforeEach(() => { + consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(() => { + consoleSpy.mockRestore(); + }); + + /** The machine-local config a `teamai init` wrote; the roster predates the switch. */ + function writeLocalConfig(clone: string, origin: string, username: string): void { + // The seed's bare `team:` line lacks the required `repo` key; give the + // clone a teamai.yaml the config loader accepts. + fs.writeFileSync(path.join(clone, 'teamai.yaml'), `team: acme\nrepo: ${origin}\nprovider: git\n`); + const home = process.env.HOME!; + fs.mkdirSync(path.join(home, '.teamai'), { recursive: true }); + fs.writeFileSync( + path.join(home, '.teamai', 'config.yaml'), + `repo:\n localPath: ${clone}\n remote: ${origin}\n kind: git\nusername: ${username}\nscope: user\nupdatePolicy: skip\nenabledAgents: [claude]\nadditionalRoles: []\n`, + ); + } + + /** Commit a pre-switch roster file on the clone's default branch. */ + async function commitLegacyRoster(clone: string, username: string): Promise { + const cloneGit = simpleGit(clone); + fs.mkdirSync(path.join(clone, 'members'), { recursive: true }); + fs.writeFileSync( + path.join(clone, 'members', `${username}.yaml`), + `username: ${username}\nregisteredAt: 2025-01-01T00:00:00.000Z\n`, + ); + await cloneGit.add([`members/${username}.yaml`]); + await cloneGit.commit('pre-switch roster'); + } + + it('lists pre-switch members from the clone without copying or publishing anything', async () => { + const { origin, clone } = await seedBareOrigin(); + await commitLegacyRoster(clone, 'carol'); + writeLocalConfig(clone, origin, 'carol'); + + await listMembers({}); + + const allOutput = consoleSpy.mock.calls.map((c) => c[0]).join('\n'); + expect(allOutput).toContain('Team members (1)'); + expect(allOutput).toContain('carol'); + // Read-only cold start: the reports branch is not published, the legacy + // file is not copied into the worktree, and the clone copy is untouched. + expect(await originHasReportsBranch(origin)).toBe(false); + const wt = path.join(tmp, REPORTS_WORKTREE_DIRNAME); + expect(fs.existsSync(path.join(wt, 'members', 'carol.yaml'))).toBe(false); + expect( + fs.readFileSync(path.join(clone, 'members', 'carol.yaml'), 'utf-8'), + ).toContain('username: carol'); + }); + + it('lists the union of pre-switch and post-switch members', async () => { + const { origin, clone } = await seedBareOrigin(); + await commitLegacyRoster(clone, 'carol'); + writeLocalConfig(clone, origin, 'carol'); + + const cfg = gitConfig(clone, origin); + const wt = await ensureReportsWorktree(cfg); + fs.mkdirSync(path.join(wt, 'members'), { recursive: true }); + fs.writeFileSync( + path.join(wt, 'members', 'alice.yaml'), + 'username: alice\nregisteredAt: 2025-06-01T00:00:00.000Z\n', + ); + await commitAndPushReports(cfg, '[teamai] Register member: alice', ['members/']); + + await listMembers({}); + + const allOutput = consoleSpy.mock.calls.map((c) => c[0]).join('\n'); + expect(allOutput).toContain('Team members (2)'); + expect(allOutput).toContain('carol'); + expect(allOutput).toContain('alice'); + // Same member on both roots: the reports-branch copy wins. + fs.writeFileSync( + path.join(wt, 'members', 'carol.yaml'), + 'username: carol\ndisplayName: Carol (branch)\nregisteredAt: 2025-06-02T00:00:00.000Z\n', + ); + await commitAndPushReports(cfg, '[teamai] Update member roster: carol', ['members/carol.yaml']); + + await listMembers({}); + const output2 = consoleSpy.mock.calls.map((c) => c[0]).join('\n'); + expect(output2).toContain('Team members (2)'); + expect(output2).toContain('Carol (branch)'); + }); +}); + describe('self-mode reports: shared stash', () => { it('does not drop a pre-existing business-worktree stash whose message contains autostash', async () => { const { origin, clone } = await seedBareOrigin(); diff --git a/src/__tests__/members.test.ts b/src/__tests__/members.test.ts index b2448d8d..ad812fbb 100644 --- a/src/__tests__/members.test.ts +++ b/src/__tests__/members.test.ts @@ -42,7 +42,7 @@ vi.mock('../utils/logger.js', () => ({ })), })); -import { getMemberConfig, listMembers, mergeMemberConfig } from '../members.js'; +import { getMemberConfig, listMembers, memberReadRoots, mergeMemberConfig, readMemberConfig } from '../members.js'; import { requireInit } from '../config.js'; import { log } from '../utils/logger.js'; @@ -173,7 +173,19 @@ describe('listMembers', () => { await fse.remove(tmpDir); }); - it('should show "No team members registered" when members dir is empty', async () => { + it('should show "No team members registered" when no root has member files', async () => { + mockRequireInit(cloneDir); + + await listMembers({}); + + expect(log.info).toHaveBeenCalledWith('No team members registered'); + expect(consoleSpy).not.toHaveBeenCalled(); + // Listing is read-only: a cold start must not publish the reports branch. + expect(reportsMocks.refreshReportsWorktree).toHaveBeenCalledWith(expect.anything(), { pushIfCreated: false }); + expect(reportsMocks.ensureReportsWorktree).toHaveBeenCalledWith(expect.anything(), { pushIfCreated: false }); + }); + + it('lists members registered before the reports switch from the default-branch clone', async () => { mockRequireInit(cloneDir); await fse.writeFile( path.join(cloneDir, 'members', 'stale.yaml'), @@ -186,8 +198,10 @@ describe('listMembers', () => { await listMembers({}); - expect(log.info).toHaveBeenCalledWith('No team members registered'); - expect(consoleSpy).not.toHaveBeenCalled(); + const allOutput = consoleSpy.mock.calls.map((c) => c[0]).join('\n'); + expect(allOutput).toContain('stale'); + expect(allOutput).toContain('Leftover on clone'); + expect(allOutput).toContain('Team members (1)'); // Listing is read-only: a cold start must not publish the reports branch. expect(reportsMocks.refreshReportsWorktree).toHaveBeenCalledWith(expect.anything(), { pushIfCreated: false }); expect(reportsMocks.ensureReportsWorktree).toHaveBeenCalledWith(expect.anything(), { pushIfCreated: false }); @@ -313,7 +327,7 @@ describe('listMembers', () => { expect(allOutput).toContain('legacy'); }); - it('does not list leftover members YAML on the default-branch clone', async () => { + it('lists the union of reports-branch and default-branch members', async () => { await writeReportsMember('alice.yaml', { username: 'alice', displayName: 'Alice Chen', @@ -334,9 +348,98 @@ describe('listMembers', () => { const allOutput = consoleSpy.mock.calls.map((c) => c[0]).join('\n'); expect(allOutput).toContain('alice'); + expect(allOutput).toContain('stale'); + expect(allOutput).toContain('Team members (2)'); + }); + + it('prefers the reports-branch copy when both roots have the same member file', async () => { + await writeReportsMember('alice.yaml', { + username: 'alice', + displayName: 'Alice (branch)', + registeredAt: '2025-06-01T00:00:00.000Z', + }); + await fse.writeFile( + path.join(cloneDir, 'members', 'alice.yaml'), + YAML.stringify({ + username: 'alice', + displayName: 'Alice (pre-switch clone)', + registeredAt: '2025-01-01T00:00:00.000Z', + }), + ); + + mockRequireInit(cloneDir, 'alice'); + + await listMembers({ verbose: true }); + + const allOutput = consoleSpy.mock.calls.map((c) => c[0]).join('\n'); + expect(allOutput).toContain('Alice (branch)'); + expect(allOutput).not.toContain('Alice (pre-switch clone)'); expect(allOutput).toContain('Team members (1)'); - expect(allOutput).not.toContain('stale'); - expect(allOutput).not.toContain('Leftover on clone'); + expect(allOutput).toContain('registered: 2025-06-01T00:00:00.000Z'); + }); +}); + +describe('memberReadRoots', () => { + it('adds the default-branch clone as an inherited root behind the primary', () => { + const localConfig = { + repo: { localPath: '/data/team-repo', remote: 'https://git.example.com/team/repo.git' }, + username: 'alice', + } as never; + expect(memberReadRoots('/data/reports-wt', localConfig)).toEqual(['/data/reports-wt', '/data/team-repo']); + }); + + it('returns a single root when the primary is the clone itself (HTTP path)', () => { + const localConfig = { + repo: { localPath: '/data/team-repo', remote: 'https://git.example.com/team/repo.git', kind: 'http' }, + username: 'alice', + } as never; + expect(memberReadRoots('/data/team-repo', localConfig)).toEqual(['/data/team-repo']); + }); +}); + +describe('readMemberConfig', () => { + let tmpDir: string; + let primaryRoot: string; + let inheritedRoot: string; + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-test-')); + primaryRoot = path.join(tmpDir, 'reports-wt'); + inheritedRoot = path.join(tmpDir, 'team-repo'); + await fse.ensureDir(path.join(primaryRoot, 'members')); + await fse.ensureDir(path.join(inheritedRoot, 'members')); + }); + + afterEach(async () => { + await fse.remove(tmpDir); + }); + + it('falls back to the inherited root when the primary has no copy', async () => { + await fse.writeFile( + path.join(inheritedRoot, 'members', 'bob.yaml'), + YAML.stringify({ username: 'bob', registeredAt: '2025-01-01T00:00:00.000Z' }), + ); + + const result = await readMemberConfig([primaryRoot, inheritedRoot], 'bob'); + expect(result?.username).toBe('bob'); + }); + + it('prefers the primary root copy on conflict', async () => { + await fse.writeFile( + path.join(primaryRoot, 'members', 'bob.yaml'), + YAML.stringify({ username: 'bob', registeredAt: '2025-06-01T00:00:00.000Z' }), + ); + await fse.writeFile( + path.join(inheritedRoot, 'members', 'bob.yaml'), + YAML.stringify({ username: 'bob', registeredAt: '2025-01-01T00:00:00.000Z' }), + ); + + const result = await readMemberConfig([primaryRoot, inheritedRoot], 'bob'); + expect(result?.registeredAt).toBe('2025-06-01T00:00:00.000Z'); + }); + + it('returns null when no root has the member', async () => { + expect(await readMemberConfig([primaryRoot, inheritedRoot], 'nobody')).toBeNull(); }); }); diff --git a/src/__tests__/projects-members.test.ts b/src/__tests__/projects-members.test.ts new file mode 100644 index 00000000..2c4a608a --- /dev/null +++ b/src/__tests__/projects-members.test.ts @@ -0,0 +1,120 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import path from 'node:path'; +import os from 'node:os'; +import fse from 'fs-extra'; +import YAML from 'yaml'; + +vi.mock('../config.js', () => ({ + autoDetectInit: vi.fn(), +})); + +vi.mock('../utils/git.js', () => ({ + pullRepo: vi.fn().mockResolvedValue('Already up to date.'), + isDedicatedRepoRoot: vi.fn().mockResolvedValue(true), +})); + +const reportsMocks = vi.hoisted(() => ({ + ensureReportsWorktree: vi.fn(), + refreshReportsWorktree: vi.fn().mockResolvedValue(undefined), +})); +vi.mock('../utils/reports-branch.js', () => ({ + ensureReportsWorktree: (...args: unknown[]) => reportsMocks.ensureReportsWorktree(...args), + refreshReportsWorktree: (...args: unknown[]) => reportsMocks.refreshReportsWorktree(...args), +})); + +vi.mock('../utils/logger.js', () => ({ + log: { + info: vi.fn(), + success: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + dim: vi.fn(), + }, +})); + +import { projectsMembers } from '../projects-cmd.js'; +import { autoDetectInit } from '../config.js'; + +describe('projectsMembers: inherited member root (#735)', () => { + let tmpDir: string; + let cloneDir: string; + let reportsDir: string; + let consoleSpy: ReturnType; + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-test-')); + cloneDir = path.join(tmpDir, 'team-repo'); + reportsDir = path.join(tmpDir, 'reports-wt'); + await fse.ensureDir(path.join(cloneDir, 'members')); + await fse.ensureDir(path.join(reportsDir, 'members')); + // The projects manifest is knowledge on the default-branch clone. + await fse.ensureDir(path.join(cloneDir, 'manifest')); + await fse.writeFile( + path.join(cloneDir, 'manifest', 'projects.yaml'), + YAML.stringify({ version: 1, projects: [{ id: 'checkout', resources: {} }] }), + ); + reportsMocks.ensureReportsWorktree.mockReset().mockResolvedValue(reportsDir); + reportsMocks.refreshReportsWorktree.mockReset().mockResolvedValue(undefined); + consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + vi.mocked(autoDetectInit).mockResolvedValue({ + localConfig: { + repo: { localPath: cloneDir, remote: 'https://git.woa.com/team/repo.git' }, + username: 'alice', + updatePolicy: 'auto', + additionalRoles: [], + scope: 'user', + }, + teamConfig: { + team: 'test', + description: '', + repo: 'https://git.woa.com/team/repo.git', + provider: 'tgit' as const, + reviewers: [], + sharing: { skills: {}, rules: { enforced: [] }, docs: { localDir: '' }, env: { injectShellProfile: true } }, + toolPaths: {}, + }, + }); + }); + + afterEach(async () => { + consoleSpy.mockRestore(); + await fse.remove(tmpDir); + }); + + it('finds project members registered before the reports switch', async () => { + await fse.writeFile( + path.join(cloneDir, 'members', 'carol.yaml'), + YAML.stringify({ username: 'carol', registeredAt: '2025-01-01T00:00:00.000Z', projects: ['checkout'] }), + ); + + await projectsMembers('checkout', {}); + + const allOutput = consoleSpy.mock.calls.map((c) => c[0]).join('\n'); + expect(allOutput).toContain('Members of project "checkout" (1)'); + expect(allOutput).toContain('carol'); + }); + + it('lists the union of inherited and reports-branch members, branch copy winning', async () => { + await fse.writeFile( + path.join(cloneDir, 'members', 'carol.yaml'), + YAML.stringify({ username: 'carol', registeredAt: '2025-01-01T00:00:00.000Z', projects: ['checkout'] }), + ); + // carol also re-registered on the reports branch but left the project. + await fse.writeFile( + path.join(reportsDir, 'members', 'carol.yaml'), + YAML.stringify({ username: 'carol', registeredAt: '2025-06-01T00:00:00.000Z' }), + ); + await fse.writeFile( + path.join(reportsDir, 'members', 'dan.yaml'), + YAML.stringify({ username: 'dan', registeredAt: '2025-06-02T00:00:00.000Z', projects: ['checkout'] }), + ); + + await projectsMembers('checkout', {}); + + const allOutput = consoleSpy.mock.calls.map((c) => c[0]).join('\n'); + expect(allOutput).toContain('Members of project "checkout" (1)'); + expect(allOutput).toContain('dan'); + expect(allOutput).not.toContain('carol'); + }); +}); diff --git a/src/bootstrap.ts b/src/bootstrap.ts index 885a7655..90c6b7b5 100644 --- a/src/bootstrap.ts +++ b/src/bootstrap.ts @@ -26,6 +26,7 @@ import { readFileSafe, pathExists, ensureDir, writeFile } from './utils/fs.js'; import { getRemoteUrl } from './utils/git.js'; import { log } from './utils/logger.js'; import { acquireLock, releaseLock } from './update.js'; +import { getMemberConfig, mergeMemberConfig } from './members.js'; export type BootstrapResult = 'bootstrapped' | 'already' | 'skip'; @@ -214,11 +215,13 @@ export async function bootstrapSelfRepo( await ensureDir(memberDir); const memberPath = path.join(memberDir, `${username}.yaml`); if (await pathExists(memberPath)) return null; - await writeFile(memberPath, YAML.stringify({ - username, - displayName: username, - registeredAt: new Date().toISOString(), - })); + // Absorb the member's pre-switch file from the clone (inherited root): + // its displayName/registeredAt/projects survive the re-registration. + const inherited = await getMemberConfig(localConfig.repo.localPath, username); + const config = inherited + ? mergeMemberConfig(inherited, { username }).config + : { username, displayName: username, registeredAt: new Date().toISOString() }; + await writeFile(memberPath, YAML.stringify(config)); return { files: ['members/'], message: `[teamai] Register member: ${username}` }; }); } catch (e) { diff --git a/src/init.ts b/src/init.ts index 92831d2e..2684d284 100644 --- a/src/init.ts +++ b/src/init.ts @@ -22,7 +22,7 @@ import { import { getUserHome } from './utils/home.js'; import { describeRoles, listRoleIds, loadRolesManifest } from './roles.js'; import { loadProjectsManifest, listProjectIds } from './projects.js'; -import { getMemberConfig, mergeMemberConfig } from './members.js'; +import { memberReadRoots, readMemberConfig, mergeMemberConfig } from './members.js'; import { askQuestion, askConfirmation, askSelection, closePrompt, isInteractive } from './utils/prompt.js'; import { normalizeAgentList, @@ -985,7 +985,7 @@ export async function initSelfRepo(options: GlobalOptions & { await ensureDir(memberDir); const memberPath = path.join(memberDir, `${username}.yaml`); isNewSelfMember = !await pathExists(memberPath); - const existingSelfMember = await getMemberConfig(wt, username); + const existingSelfMember = await readMemberConfig(memberReadRoots(wt, localConfig), username); const merged = mergeMemberConfig(existingSelfMember, { username, projects: localConfig.projects, @@ -1442,7 +1442,8 @@ export async function init(options: GlobalOptions & { } // Step 5: member roster on the teamai-reports orphan branch (never the - // default branch). Leftover members/ on the clone is ignored. + // default branch). The clone's leftover members/ is a read-only inherited + // root: the merge below absorbs the member's pre-switch file. let isNewMember = true; if (!options.dryRun) { try { @@ -1454,7 +1455,7 @@ export async function init(options: GlobalOptions & { await ensureDir(memberDir); const memberPath = path.join(memberDir, `${username}.yaml`); isNewMember = !await pathExists(memberPath); - const existingMember = await getMemberConfig(wt, username); + const existingMember = await readMemberConfig(memberReadRoots(wt, reportsConfig), username); const merged = mergeMemberConfig(existingMember, { username, projects: resolvedProjects, diff --git a/src/members.ts b/src/members.ts index 8acd9ec1..1c3771a4 100644 --- a/src/members.ts +++ b/src/members.ts @@ -5,7 +5,7 @@ import { readFileSafe, listFiles } from './utils/fs.js'; import { pullRepo } from './utils/git.js'; import { log } from './utils/logger.js'; import { MemberConfigSchema } from './types.js'; -import type { GlobalOptions, MemberConfig } from './types.js'; +import type { GlobalOptions, LocalConfig, MemberConfig } from './types.js'; /** * Read a specific member's config from the repo. @@ -22,6 +22,31 @@ export async function getMemberConfig(repoPath: string, username: string): Promi } } +/** + * Read roots for member files, highest precedence first. The primary root is + * the teamai-reports worktree (the clone itself for HTTP repos); the + * default-branch clone is an inherited root for members registered before + * reports moved to their own branch (#489) — read the way learnings' inherited + * root is (#485): forever, with nothing copied out of it or deleted from it. + */ +export function memberReadRoots(primary: string, localConfig: LocalConfig): string[] { + if (primary === localConfig.repo.localPath) return [primary]; + return [primary, localConfig.repo.localPath]; +} + +/** + * Read a member's config across read roots. The first root that yields one + * wins, so a copy that moved to the reports branch supersedes its inherited + * original. + */ +export async function readMemberConfig(roots: string[], username: string): Promise { + for (const root of roots) { + const config = await getMemberConfig(root, username); + if (config) return config; + } + return null; +} + /** * Merge a member's roster entry with newly-active role/projects, returning the * updated config and whether anything changed. Projects use **append + dedupe** @@ -66,8 +91,9 @@ export async function listMembers(options: GlobalOptions): Promise { const localConfig = projectConfig ?? (await requireInit()).localConfig; // Members live on the teamai-reports orphan branch for non-HTTP repos; read - // them from the reports worktree (refreshed from origin). Leftover members/ - // on the default-branch clone is ignored. HTTP keeps the clone/API path. + // them from the reports worktree (refreshed from origin). Members registered + // before the switch still live on the default-branch clone, so it stays a + // read-only inherited root. HTTP keeps the clone/API path. // Listing is read-only: never publish a missing reports branch. let repoPath: string; const { usesBranchWorktree } = await import('./types.js'); @@ -80,21 +106,30 @@ export async function listMembers(options: GlobalOptions): Promise { await pullRepo(repoPath); } - const membersDir = path.join(repoPath, 'members'); - const files = await listFiles(membersDir); - const yamlFiles = files.filter((f) => f.endsWith('.yaml') || f.endsWith('.yml')); + // Union across read roots; the first root that has a file supplies its + // bytes, so a copy on the reports branch supersedes the inherited one. + const memberFiles: Array<{ file: string; root: string }> = []; + const listed = new Set(); + for (const root of memberReadRoots(repoPath, localConfig)) { + for (const file of await listFiles(path.join(root, 'members'))) { + if (!file.endsWith('.yaml') && !file.endsWith('.yml')) continue; + if (listed.has(file)) continue; + listed.add(file); + memberFiles.push({ file, root }); + } + } - if (yamlFiles.length === 0) { + if (memberFiles.length === 0) { log.info('No team members registered'); return; } console.log(''); - console.log(`Team members (${yamlFiles.length}):`); + console.log(`Team members (${memberFiles.length}):`); console.log(''); - for (const file of yamlFiles) { - const content = await readFileSafe(path.join(membersDir, file)); + for (const { file, root } of memberFiles) { + const content = await readFileSafe(path.join(root, 'members', file)); if (!content) continue; try { const raw = YAML.parse(content); diff --git a/src/projects-cmd.ts b/src/projects-cmd.ts index 3ba23a2d..144a752b 100644 --- a/src/projects-cmd.ts +++ b/src/projects-cmd.ts @@ -17,6 +17,7 @@ import { readFileSafe, listFiles } from './utils/fs.js'; import { pullRepo } from './utils/git.js'; import { log } from './utils/logger.js'; import { MemberConfigSchema } from './types.js'; +import { memberReadRoots } from './members.js'; import type { GlobalOptions } from './types.js'; function parseIds(input: string[]): string[] { @@ -131,8 +132,8 @@ export async function projectsMembers( const { localConfig } = await autoDetectInit(); // Members live on the teamai-reports orphan branch for non-HTTP repos; the - // projects manifest is knowledge on the default branch. Split the two roots - // so leftover clone members/ is ignored and projects.yaml is still found. + // projects manifest is knowledge on the default branch. Split the two roots: + // the clone's members/ is a read-only inherited root for pre-switch files. const knowledgePath = localConfig.repo.localPath; let membersRoot = knowledgePath; const { usesBranchWorktree } = await import('./types.js'); @@ -151,21 +152,26 @@ export async function projectsMembers( // Continue anyway — the roster may still record historical membership. } - const membersDir = path.join(membersRoot, 'members'); - const files = (await listFiles(membersDir)).filter((f) => f.endsWith('.yaml') || f.endsWith('.yml')); - + // Union across read roots; the first root that has a file supplies its + // bytes, so a copy on the reports branch supersedes the inherited one. const members: string[] = []; - for (const file of files) { - const content = await readFileSafe(path.join(membersDir, file)); - if (!content) continue; - try { - const member = MemberConfigSchema.parse(YAML.parse(content)); - if ((member.projects ?? []).includes(projectId)) { - const display = member.displayName ? ` — ${member.displayName}` : ''; - members.push(`${member.username}${display}`); + const listed = new Set(); + for (const root of memberReadRoots(membersRoot, localConfig)) { + const files = (await listFiles(path.join(root, 'members'))).filter((f) => f.endsWith('.yaml') || f.endsWith('.yml')); + for (const file of files) { + if (listed.has(file)) continue; + listed.add(file); + const content = await readFileSafe(path.join(root, 'members', file)); + if (!content) continue; + try { + const member = MemberConfigSchema.parse(YAML.parse(content)); + if ((member.projects ?? []).includes(projectId)) { + const display = member.displayName ? ` — ${member.displayName}` : ''; + members.push(`${member.username}${display}`); + } + } catch { + // Skip invalid member files silently (listMembers already warns on `members`). } - } catch { - // Skip invalid member files silently (listMembers already warns on `members`). } }