diff --git a/src/__tests__/pull-scope-isolation.test.ts b/src/__tests__/pull-scope-isolation.test.ts index 22ff56c..5b379ee 100644 --- a/src/__tests__/pull-scope-isolation.test.ts +++ b/src/__tests__/pull-scope-isolation.test.ts @@ -268,4 +268,51 @@ describe('pull scope isolation (issue #73)', () => { expect(pullSources).toHaveBeenCalledTimes(1); expect(vi.mocked(pullSources).mock.calls[0][0]).toMatchObject({ scope: 'user' }); }); + + it('user mode self-repo: reportUsageToTeam receives selfConfig so business repo is never reset', async () => { + const businessRoot = path.join(tmpDir, 'business-repo'); + const selfRepoPath = path.join(businessRoot, '.teamai'); + const selfUserConfig: LocalConfig = { + repo: { + localPath: selfRepoPath, + remote: 'https://git.woa.com/test/self-repo.git', + kind: 'self', + businessRepoRoot: businessRoot, + }, + username: 'selfuser', + updatePolicy: 'auto', + additionalRoles: [], + scope: 'user', + }; + + vi.mocked(detectProjectConfig).mockResolvedValue(null); + vi.mocked(loadLocalConfigForScope).mockResolvedValue(selfUserConfig); + + await pull({ silent: true }); + + expect(reportUsageToTeam).toHaveBeenCalledWith( + selfRepoPath, + 'selfuser', + expect.objectContaining({ + selfConfig: expect.objectContaining({ + repo: expect.objectContaining({ kind: 'self' }), + }), + }), + ); + }); + + it('project mode http-repo: never calls reportUsageToTeam (data-loss guard)', async () => { + const httpProjectConfig: LocalConfig = { + ...projectConfig, + repo: { ...projectConfig.repo, kind: 'http' as const }, + }; + + vi.mocked(detectProjectConfig).mockResolvedValue(httpProjectConfig); + + await pull({ silent: true }); + + // HTTP-kind repo: kind !== 'http' guard filters out both report targets, + // so targets is empty and the business repo's team-repo dir is never reset. + expect(reportUsageToTeam).not.toHaveBeenCalled(); + }); }); diff --git a/src/__tests__/push-role.test.ts b/src/__tests__/push-role.test.ts index e556652..0fc57e0 100644 --- a/src/__tests__/push-role.test.ts +++ b/src/__tests__/push-role.test.ts @@ -60,6 +60,7 @@ vi.mock('../utils/git.js', () => ({ checkoutMaster: (...args: unknown[]) => mockCheckoutMaster(...args), generateBranchName: (...args: unknown[]) => mockGenerateBranchName(...args), resetToCleanMaster: (...args: unknown[]) => mockResetToCleanMaster(...args), + isDedicatedRepoRoot: vi.fn().mockResolvedValue(true), })); vi.mock('../roles.js', async () => { diff --git a/src/__tests__/self-mode-no-business-reset.test.ts b/src/__tests__/self-mode-no-business-reset.test.ts new file mode 100644 index 0000000..745bb5a --- /dev/null +++ b/src/__tests__/self-mode-no-business-reset.test.ts @@ -0,0 +1,122 @@ +/** + * E2E (real git, NO mocks): proves reportUsageToTeam never wipes a self-mode + * business repo working tree, and still resets a genuine git-mode cache clone. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import os from 'node:os'; +import { simpleGit } from 'simple-git'; + +import { reportUsageToTeam } from '../team-push.js'; +import type { LocalConfig } from '../types.js'; + +let tmp: string; +let originalHome: string; + +beforeEach(() => { + tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-e2e-')); + originalHome = process.env.HOME ?? ''; + process.env.HOME = path.join(tmp, 'home'); + fs.mkdirSync(process.env.HOME, { recursive: true }); +}); + +afterEach(() => { + process.env.HOME = originalHome; + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +async function makeRepo(dir: string, branch = 'master'): Promise { + fs.mkdirSync(dir, { recursive: true }); + const git = simpleGit(dir); + await git.init(); + await git.addConfig('user.email', 't@t.com'); + await git.addConfig('user.name', 't'); + await git.checkoutLocalBranch(branch); // default branch = master, like the bug report + fs.writeFileSync(path.join(dir, 'app.js'), 'console.log(1)\n'); + await git.add('.'); + await git.commit('init'); +} + +describe('E2E self-mode: business repo working tree is never reset', () => { + it('self mode: uncommitted change + non-master branch survive reportUsageToTeam', async () => { + const businessRoot = path.join(tmp, 'business'); + await makeRepo(businessRoot); + const git = simpleGit(businessRoot); + + // User is on a feature branch with uncommitted work (the scenario that was lost). + await git.checkoutLocalBranch('feature/wip'); + fs.writeFileSync(path.join(businessRoot, 'app.js'), 'console.log("MY UNCOMMITTED WORK")\n'); + + const teamaiDir = path.join(businessRoot, '.teamai'); + fs.mkdirSync(teamaiDir, { recursive: true }); + + // self-mode user-scope config: localPath = /.teamai + const cfg: LocalConfig = { + repo: { localPath: teamaiDir, remote: '', kind: 'self', businessRepoRoot: businessRoot }, + username: 'me', + scope: 'user', + additionalRoles: [], + } as unknown as LocalConfig; + + // The bug path: pull passes selfConfig now (fix 1). Even without it, the + // isDedicatedRoot guard (fix 2) must protect the tree — test the guard by + // NOT passing selfConfig, forcing the else branch. + await reportUsageToTeam(teamaiDir, 'me', { skipTruncate: true }); + + // Assert the user's working tree is untouched. + const branch = (await git.revparse(['--abbrev-ref', 'HEAD'])).trim(); + const content = fs.readFileSync(path.join(businessRoot, 'app.js'), 'utf-8'); + expect(branch).toBe('feature/wip'); + expect(content).toContain('MY UNCOMMITTED WORK'); + }); + + it('git mode: a real dedicated cache clone IS still reset (no regression)', async () => { + // A dedicated cache clone: repoPath IS the git top level. + // Use 'main' as default branch so resetToCleanMaster's fallback checkout matches. + const cacheRoot = path.join(tmp, 'cache'); + await makeRepo(cacheRoot, 'main'); + const git = simpleGit(cacheRoot); + + // Simulate stale dirty state in the cache. + fs.writeFileSync(path.join(cacheRoot, 'app.js'), 'garbage\n'); + + await reportUsageToTeam(cacheRoot, 'me', { skipTruncate: true }); + + // reset --hard should have discarded the dirty change in the cache clone. + const content = fs.readFileSync(path.join(cacheRoot, 'app.js'), 'utf-8'); + expect(content).toBe('console.log(1)\n'); + }); + + it('project scope (git kind): business repo is NOT reset when team-repo dir lacks its own .git', async () => { + // Reproduces the v0.19.0 data-loss path outside self mode: in project scope the + // team repo lives at /.teamai/team-repo. If that dir has no dedicated + // .git (e.g. clone never completed), git commands there bubble up to the business + // repo's .git — so an unguarded reset --hard + checkout would wipe the user's tree. + const businessRoot = path.join(tmp, 'project'); + await makeRepo(businessRoot); + const git = simpleGit(businessRoot); + + // User is on a feature branch with uncommitted work. + await git.checkoutLocalBranch('feature/wip'); + fs.writeFileSync(path.join(businessRoot, 'app.js'), 'console.log("MY UNCOMMITTED WORK")\n'); + + // team-repo dir exists but is a plain directory (no dedicated .git of its own). + const teamRepoDir = path.join(businessRoot, '.teamai', 'team-repo'); + fs.mkdirSync(teamRepoDir, { recursive: true }); + + // git-mode report to the in-business-repo path. The isDedicatedRoot guard must + // detect that show-toplevel resolves to the business root, not teamRepoDir, and bail. + const logBefore = await git.log(); + await reportUsageToTeam(teamRepoDir, 'me', { skipTruncate: true }); + + // The user's working tree and branch must be intact. + const branch = (await git.revparse(['--abbrev-ref', 'HEAD'])).trim(); + const content = fs.readFileSync(path.join(businessRoot, 'app.js'), 'utf-8'); + expect(branch).toBe('feature/wip'); + expect(content).toContain('MY UNCOMMITTED WORK'); + // Guard must also prevent a spurious stats commit from landing on the user's branch. + const logAfter = await git.log(); + expect(logAfter.total).toBe(logBefore.total); + }); +}); diff --git a/src/__tests__/team-push-interventions.test.ts b/src/__tests__/team-push-interventions.test.ts index fe1610d..8492c2b 100644 --- a/src/__tests__/team-push-interventions.test.ts +++ b/src/__tests__/team-push-interventions.test.ts @@ -12,6 +12,7 @@ vi.mock('../utils/git.js', () => ({ pushRepoDirectly: (...args: unknown[]) => pushRepoDirectly(...args), pullRepo: vi.fn().mockResolvedValue(undefined), resetToCleanMaster: vi.fn().mockResolvedValue(undefined), + isDedicatedRepoRoot: vi.fn().mockResolvedValue(true), })); vi.mock('../utils/logger.js', () => ({ log: { info: vi.fn(), success: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, diff --git a/src/pull.ts b/src/pull.ts index 81a7e70..17c1e8a 100644 --- a/src/pull.ts +++ b/src/pull.ts @@ -1229,7 +1229,7 @@ export async function pull(options: GlobalOptions): Promise { const { reportUsageToTeam } = await import('./team-push.js'); const { truncateUsageAfterReport, readUsageEvents } = await import('./usage-tracker.js'); const targets: Array<{ repoPath: string; username: string; opts: { skipTruncate: true; projectRoot?: string; excludeProjectRoots?: string[]; selfConfig?: LocalConfig } }> = []; - if (projectConfig) { + if (projectConfig && projectConfig.repo.kind !== 'http') { targets.push({ repoPath: projectConfig.repo.localPath, username: projectConfig.username, @@ -1245,7 +1245,13 @@ export async function pull(options: GlobalOptions): Promise { targets.push({ repoPath: activeUserConfig.repo.localPath, username: activeUserConfig.username, - opts: { skipTruncate: true, excludeProjectRoots: projectConfig?.projectRoot ? [projectConfig.projectRoot] : [] }, + opts: { + skipTruncate: true, + excludeProjectRoots: projectConfig?.projectRoot ? [projectConfig.projectRoot] : [], + // Self mode routes stats/votes to the teamai-reports orphan branch — + // never reset/pull the business repo working tree. + ...(activeUserConfig.repo.kind === 'self' ? { selfConfig: activeUserConfig } : {}), + }, }); } diff --git a/src/push.ts b/src/push.ts index 586a505..96cdc99 100644 --- a/src/push.ts +++ b/src/push.ts @@ -1,7 +1,10 @@ import path from 'node:path'; import { autoDetectInit, loadStateForScope, saveStateForScope } from './config.js'; import { assertNotReadOnly } from './read-only.js'; -import { createGit, pullRepo, pushRepoBranch, checkoutMaster, generateBranchName, resetToCleanMaster, getDefaultBranch } from './utils/git.js'; +import { + createGit, pullRepo, pushRepoBranch, checkoutMaster, generateBranchName, + resetToCleanMaster, isDedicatedRepoRoot, getDefaultBranch, +} from './utils/git.js'; import { syncTeamUpdatesToLocal } from './utils/pre-push-sync.js'; import { getProvider } from './providers/index.js'; import { log, spinner } from './utils/logger.js'; @@ -167,6 +170,16 @@ async function pushCore( try { const repoPath = localConfig.repo.localPath; const git = createGit(repoPath); + if (!(await isDedicatedRepoRoot(repoPath))) { + // repoPath is not its own git root (e.g. a project-scope team repo dir with no + // dedicated .git that resolves to the business repo). Every step below — reset + // --hard, checkout, and pushRepoBranch — would act on the enclosing business + // repo and wipe the user's working tree. Abort the whole push with a clear + // message rather than silently doing nothing or damaging their repo. + pullSpin.fail('Cannot push: team repo path is not a dedicated git root. ' + + 'Run `teamai init` to re-clone the team repo before pushing.'); + return; + } await resetToCleanMaster(git, repoPath); await pullRepo(repoPath); pullSpin.succeed('Up to date'); diff --git a/src/team-push.ts b/src/team-push.ts index fe0c120..1d77b27 100644 --- a/src/team-push.ts +++ b/src/team-push.ts @@ -3,7 +3,7 @@ import path from 'node:path'; import { readUsageEvents, truncateUsageAfterReport } from './usage-tracker.js'; import { aggregateUsage } from './stats.js'; import { readEvents, aggregateSessionMetrics } from './dashboard-collector.js'; -import { createGit, pushRepoDirectly, pullRepo, resetToCleanMaster } from './utils/git.js'; +import { createGit, pushRepoDirectly, pullRepo, resetToCleanMaster, isDedicatedRepoRoot } from './utils/git.js'; import { withTimeout } from './utils/async.js'; import { writeFile, readFileSafe, ensureDir, pathExists, readJson, writeJson } from './utils/fs.js'; import { log } from './utils/logger.js'; @@ -345,9 +345,24 @@ export async function reportUsageToTeam( const { ensureReportsWorktree } = await import('./utils/reports-branch.js'); writeRoot = await ensureReportsWorktree(selfConfig); } else { - // Reset any dirty/conflicted state and ensure we're on the default branch before pulling. - // Same pattern as push.ts — the team repo is a cache, safe to discard local state. + // The team repo is a disposable cache clone here — safe to discard local state + // and reset to the default branch before pulling (same pattern as push.ts). + // + // Defense-in-depth: this whole else-branch assumes repoPath is a dedicated clone + // ROOT with its own .git. If it is not the git top level, git commands here bubble + // up to the nearest enclosing .git and act on the USER'S BUSINESS REPO instead — + // reset --hard wipes their uncommitted work and checkout switches them off their + // branch. Two known ways repoPath ends up inside the business repo: + // - self mode: localPath is `/.teamai` + // - project scope: localPath is `/.teamai/team-repo`, and when that + // dir has no dedicated .git (clone missing/incomplete) it resolves to the + // business repo root. + // In either case bail out: there is no safe cache root to report into. const git = createGit(repoPath); + if (!(await isDedicatedRepoRoot(repoPath))) { + log.debug(`Skipping report: ${repoPath} is not a dedicated team-repo root (safety guard)`); + return; + } await resetToCleanMaster(git, repoPath); await pullRepo(repoPath); } diff --git a/src/utils/git.ts b/src/utils/git.ts index 4e401a5..ddca857 100644 --- a/src/utils/git.ts +++ b/src/utils/git.ts @@ -1,4 +1,5 @@ import fs from 'node:fs'; +import { realpath } from 'node:fs/promises'; import path from 'node:path'; import fse from 'fs-extra'; import simpleGit, { type SimpleGit } from 'simple-git'; @@ -398,6 +399,42 @@ export function generateBranchName(username: string): string { return `teamai/push/${username}/${timestamp}`; } +/** + * Check whether repoPath is a dedicated git repository root of its own — i.e. safe + * to run destructive maintenance (reset --hard, checkout) against as a disposable + * cache clone. + * + * Returns true ONLY when repoPath resolves to its own git top level. Returns false + * whenever that cannot be positively confirmed, so callers must bail out (skip + * reset/pull) on false: if repoPath is a subdirectory of the user's business repo + * (e.g. `/.teamai/team-repo` with no dedicated .git), git commands + * bubble up to the business repo and would wipe the user's working tree. + * + * @param repoPath - Absolute path expected to be a dedicated clone root. + * @returns True if repoPath is its own git top level; false if unconfirmed/unsafe. + */ +export async function isDedicatedRepoRoot(repoPath: string): Promise { + const git = createGit(repoPath); + let toplevel: string; + try { + toplevel = (await git.revparse(['--show-toplevel'])).trim(); + } catch { + // Not inside a git repository at all — there is no enclosing repo to damage, + // so treat repoPath as a plain dedicated dir (historical behavior). fail-open. + return true; + } + try { + // revparse succeeded: repoPath is inside SOME git repo. Confirm it is repoPath's + // OWN root, not an enclosing business repo. Resolve symlinks on both sides first + // (macOS /tmp -> /private/tmp) so path comparison is not fooled by a symlinked + // prefix. If realpath itself fails, we cannot confirm safety → fail-closed. + const [realTop, realRepo] = await Promise.all([realpath(toplevel), realpath(repoPath)]); + return realTop === realRepo; + } catch { + return false; + } +} + /** * Reset the team repo to a clean default-branch state. *