Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/__tests__/hooks-cmd.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,17 @@ describe('hooksInject', () => {
expect(mockedReconcile).toHaveBeenCalledTimes(2);
expect(mockedReconcile).toHaveBeenNthCalledWith(1, mockTeamConfig.toolPaths, '/path/to/project', TEAM_DEFS, expect.any(String), { builtinOverride: undefined });
expect(mockedReconcile).toHaveBeenNthCalledWith(2, mockTeamConfig.toolPaths, '/home/testuser', TEAM_DEFS, expect.any(String), { builtinOverride: undefined });

// #85: the user-home target must be reconciled against the USER's own
// manifest, not the project's — otherwise `pull`'s per-scope reconcile
// (which always uses each scope's own manifest) diverges from `inject`,
// causing duplicate injection / wrongful cleanup of the shared file.
const projectManifestPath = mockedReconcile.mock.calls[0][3] as string;
const userManifestPath = mockedReconcile.mock.calls[1][3] as string;
expect(userManifestPath).not.toBe(projectManifestPath);
expect(projectManifestPath).toContain('/path/to/project');
expect(userManifestPath).toContain('/home/testuser');
expect(userManifestPath).not.toContain('/path/to/project');
});
});

Expand Down Expand Up @@ -248,6 +259,12 @@ describe('hooksRemove', () => {
restoreHome();
}
expect(mockedReconcile).toHaveBeenCalledTimes(2);

// #85: same per-scope manifest requirement as `hooksInject`.
const projectManifestPath = mockedReconcile.mock.calls[0][3] as string;
const userManifestPath = mockedReconcile.mock.calls[1][3] as string;
expect(userManifestPath).not.toBe(projectManifestPath);
expect(userManifestPath).not.toContain('/path/to/project');
});

it('does not duplicate when HOME equals projectRoot', async () => {
Expand Down
253 changes: 253 additions & 0 deletions src/__tests__/scope-isolation-e2e-issue85.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,253 @@
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { spawn, execSync } from 'node:child_process';
import path from 'node:path';
import fs from 'node:fs';
import os from 'node:os';
import { fileURLToPath } from 'node:url';

// ─── Issue #85 end-to-end: remaining scope-isolation gaps ──────────────
//
// #73/#77 already fixed recall's dual-scope merge and #91 fixed auto-recall's
// upvote scope (see scope-isolation-e2e.test.ts). This file drives the real
// CLI binary against offline git fixtures to cover the four gaps that were
// still open after those landed:
// 1. `hooks inject`/`hooks remove` must track the user-home copy under the
// USER's own manifest, not the project's (else duplicate injection /
// wrongful cleanup of the shared tool settings file).
// 2. `tags subscribe`/`unsubscribe` must write to the active scope's
// config.yaml, not always ~/.teamai/config.yaml.
// 3. `contribute` must make a new learning immediately recallable, without
// requiring a separate `pull` to rebuild the index.
// 4. A project config.yaml missing `projectRoot` (pre-migration / hand
// edited) must still resolve to the project directory rather than
// silently degrading to the user's home.

const __dirname = path.dirname(fileURLToPath(import.meta.url));
const ROOT = path.resolve(__dirname, '..', '..');
const CLI = path.join(ROOT, 'dist', 'index.js');

interface RunResult {
code: number | null;
output: string;
}

function runCLI(args: string[], env: Record<string, string>, cwd: string): Promise<RunResult> {
return new Promise((resolve) => {
const child = spawn('node', [CLI, ...args], {
env: { ...process.env, FORCE_COLOR: '0', ...env },
stdio: ['pipe', 'pipe', 'pipe'],
cwd,
});
let out = '';
child.stdout.on('data', (d: Buffer) => { out += d.toString(); });
child.stderr.on('data', (d: Buffer) => { out += d.toString(); });
child.stdin.end();
child.on('close', (code) => resolve({ code, output: out }));
});
}

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',
};

function git(cmd: string, cwd: string): void {
execSync(`git ${cmd}`, { cwd, stdio: 'pipe', env: { ...process.env, ...GIT_ENV } });
}

const TEAM_YAML = [
'team: e2e-team',
'repo: https://example.com/e2e.git',
'provider: tgit',
'toolPaths:',
' claude:',
' skills: .claude/skills',
' rules: .claude/rules',
' settings: .claude/settings.json',
].join('\n');

const HOOKS_YAML = [
'hooks:',
' - id: e2e-marker-hook',
' description: e2e marker hook',
' event: Stop',
' command: echo teamai-e2e-hook-marker',
].join('\n');

/** Create a bare-ish pushable git remote fixture with teamai.yaml + hooks.yaml. */
function makeRemote(dir: string): void {
fs.mkdirSync(path.join(dir, 'learnings'), { recursive: true });
fs.mkdirSync(path.join(dir, 'hooks'), { recursive: true });
fs.writeFileSync(path.join(dir, 'teamai.yaml'), TEAM_YAML);
fs.writeFileSync(path.join(dir, 'hooks', 'hooks.yaml'), HOOKS_YAML);
git('init -q', dir);
// Allow `git push` to update the checked-out branch directly (contribute()
// pushes straight to master with no separate bare remote in this fixture).
git('config receive.denyCurrentBranch updateInstead', dir);
git('add -A', dir);
git('commit -q -m init', dir);
}

describe('issue #85 remaining scope-isolation gaps (e2e)', () => {
let sandbox: string;
let homeDir: string;
let projectRoot: string;
let userConfigPath: string;
let userConfigBefore: string;

beforeAll(() => {
if (!fs.existsSync(CLI)) {
throw new Error(`CLI binary not found at ${CLI}. Run "npm run build" first.`);

Check failure on line 102 in src/__tests__/scope-isolation-e2e-issue85.test.ts

View workflow job for this annotation

GitHub Actions / Lint & Test (Node 22 on macos-latest)

src/__tests__/scope-isolation-e2e-issue85.test.ts > issue #85 remaining scope-isolation gaps (e2e)

Error: CLI binary not found at /Users/runner/work/teamai-cli/teamai-cli/dist/index.js. Run "npm run build" first. ❯ src/__tests__/scope-isolation-e2e-issue85.test.ts:102:13

Check failure on line 102 in src/__tests__/scope-isolation-e2e-issue85.test.ts

View workflow job for this annotation

GitHub Actions / Lint & Test (Node 22 on ubuntu-latest)

src/__tests__/scope-isolation-e2e-issue85.test.ts > issue #85 remaining scope-isolation gaps (e2e)

Error: CLI binary not found at /home/runner/work/teamai-cli/teamai-cli/dist/index.js. Run "npm run build" first. ❯ src/__tests__/scope-isolation-e2e-issue85.test.ts:102:13

Check failure on line 102 in src/__tests__/scope-isolation-e2e-issue85.test.ts

View workflow job for this annotation

GitHub Actions / Lint & Test (Node 20 on macos-latest)

src/__tests__/scope-isolation-e2e-issue85.test.ts > issue #85 remaining scope-isolation gaps (e2e)

Error: CLI binary not found at /Users/runner/work/teamai-cli/teamai-cli/dist/index.js. Run "npm run build" first. ❯ src/__tests__/scope-isolation-e2e-issue85.test.ts:102:13

Check failure on line 102 in src/__tests__/scope-isolation-e2e-issue85.test.ts

View workflow job for this annotation

GitHub Actions / Lint & Test (Node 20 on ubuntu-latest)

src/__tests__/scope-isolation-e2e-issue85.test.ts > issue #85 remaining scope-isolation gaps (e2e)

Error: CLI binary not found at /home/runner/work/teamai-cli/teamai-cli/dist/index.js. Run "npm run build" first. ❯ src/__tests__/scope-isolation-e2e-issue85.test.ts:102:13
}

sandbox = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-issue85-e2e-'));
homeDir = path.join(sandbox, 'home');
projectRoot = path.join(sandbox, 'proj');
fs.mkdirSync(homeDir, { recursive: true });
fs.mkdirSync(projectRoot, { recursive: true });
fs.mkdirSync(path.join(homeDir, '.claude', 'skills'), { recursive: true });
fs.mkdirSync(path.join(projectRoot, '.claude', 'skills'), { recursive: true });

// ── User-scope fixture (present only so we can prove project-scope
// commands do NOT write into it, except where #44 intentionally
// mirrors hooks into the user's home). ──
const userRemote = path.join(sandbox, 'user-remote');
makeRemote(userRemote);
const userLocal = path.join(homeDir, '.teamai', 'team-repo');
git(`clone -q "${userRemote}" "${userLocal}"`, sandbox);
userConfigPath = path.join(homeDir, '.teamai', 'config.yaml');
fs.writeFileSync(
userConfigPath,
[
'repo:',
` localPath: ${userLocal}`,
` remote: ${userRemote}`,
'username: ci-user',
'updatePolicy: auto',
'scope: user',
].join('\n'),
);
userConfigBefore = fs.readFileSync(userConfigPath, 'utf-8');

// ── Project-scope fixture. Deliberately omit `projectRoot:` from the
// on-disk config to exercise the detectProjectConfig() backfill
// (item 4) through every test below. ──
const projectRemote = path.join(sandbox, 'proj-remote');
makeRemote(projectRemote);
const projectLocal = path.join(projectRoot, '.teamai', 'team-repo');
git(`clone -q "${projectRemote}" "${projectLocal}"`, sandbox);
fs.writeFileSync(
path.join(projectRoot, '.teamai', 'config.yaml'),
[
'repo:',
` localPath: ${projectLocal}`,
` remote: ${projectRemote}`,
'username: ci-proj',
'updatePolicy: auto',
'scope: project',
].join('\n'),
);
}, 60_000);

afterAll(() => {
if (sandbox) fs.rmSync(sandbox, { recursive: true, force: true });
});

describe('projectRoot backfill (item 4)', () => {
it('recall resolves the project scope even though config.yaml has no projectRoot field', async () => {
const res = await runCLI(['recall', 'anything'], { HOME: homeDir }, projectRoot);
expect(res.code, res.output).toBe(0);
expect(res.output).not.toContain('not initialized');
});
});

describe('hooks inject/remove manifest scoping (item 1)', () => {
it('inject creates BOTH a project manifest and a user manifest, each tracking its own directory', async () => {
const res = await runCLI(['hooks', 'inject'], { HOME: homeDir }, projectRoot);
expect(res.code, res.output).toBe(0);

const projectManifestPath = path.join(projectRoot, '.teamai', 'managed-hooks.json');
const userManifestPath = path.join(homeDir, '.teamai', 'managed-hooks.json');
expect(fs.existsSync(projectManifestPath)).toBe(true);
expect(fs.existsSync(userManifestPath)).toBe(true);

const projectManifest = JSON.parse(fs.readFileSync(projectManifestPath, 'utf-8'));
const userManifest = JSON.parse(fs.readFileSync(userManifestPath, 'utf-8'));
expect(projectManifest.claude?.[0]?.command).toContain('teamai-e2e-hook-marker');
expect(userManifest.claude?.[0]?.command).toContain('teamai-e2e-hook-marker');

// Both settings files actually received the hook (the #44 behavior of
// also writing into the user's home dir must still work).
const projectSettings = fs.readFileSync(path.join(projectRoot, '.claude', 'settings.json'), 'utf-8');
const userSettings = fs.readFileSync(path.join(homeDir, '.claude', 'settings.json'), 'utf-8');
expect(projectSettings).toContain('teamai-e2e-hook-marker');
expect(userSettings).toContain('teamai-e2e-hook-marker');
});

it('re-running inject is idempotent — no duplicate hook entries in the user settings file', async () => {
const before = fs.readFileSync(path.join(homeDir, '.claude', 'settings.json'), 'utf-8');
const beforeCount = before.split('teamai-e2e-hook-marker').length - 1;

const res = await runCLI(['hooks', 'inject'], { HOME: homeDir }, projectRoot);
expect(res.code, res.output).toBe(0);

const after = fs.readFileSync(path.join(homeDir, '.claude', 'settings.json'), 'utf-8');
const afterCount = after.split('teamai-e2e-hook-marker').length - 1;
expect(afterCount).toBe(beforeCount);
});

it('remove cleans up both the project and user copies', async () => {
const res = await runCLI(['hooks', 'remove'], { HOME: homeDir }, projectRoot);
expect(res.code, res.output).toBe(0);

const projectSettings = fs.readFileSync(path.join(projectRoot, '.claude', 'settings.json'), 'utf-8');
const userSettings = fs.readFileSync(path.join(homeDir, '.claude', 'settings.json'), 'utf-8');
expect(projectSettings).not.toContain('teamai-e2e-hook-marker');
expect(userSettings).not.toContain('teamai-e2e-hook-marker');
});
});

describe('tags subscribe/unsubscribe scope isolation (item 2)', () => {
it('subscribe writes to the project config, leaving the user config untouched', async () => {
const res = await runCLI(['tags', 'subscribe', 'hai'], { HOME: homeDir }, projectRoot);
expect(res.code, res.output).toBe(0);

const projectConfig = fs.readFileSync(path.join(projectRoot, '.teamai', 'config.yaml'), 'utf-8');
expect(projectConfig).toContain('hai');

expect(fs.readFileSync(userConfigPath, 'utf-8')).toBe(userConfigBefore);
});

it('unsubscribe removes it again from the project config', async () => {
const res = await runCLI(['tags', 'unsubscribe', 'hai'], { HOME: homeDir }, projectRoot);
expect(res.code, res.output).toBe(0);

const projectConfig = fs.readFileSync(path.join(projectRoot, '.teamai', 'config.yaml'), 'utf-8');
expect(projectConfig).not.toContain('subscribedTags');
expect(fs.readFileSync(userConfigPath, 'utf-8')).toBe(userConfigBefore);
});
});

describe('contribute rebuilds the index immediately (item 3)', () => {
it('a newly contributed learning is recallable without a separate `pull`', async () => {
const docPath = path.join(sandbox, 'new-learning.md');
fs.writeFileSync(
docPath,
'---\ntitle: "Zzyzx Contribution Marker"\nauthor: ci\ndate: 2026-07-01\ntags: [e2e]\n---\n\nUnique marker content for the e2e contribute test: Zzyzx Contribution Marker.\n',
);

const res = await runCLI(
['contribute', '--file', docPath, '--title', 'Zzyzx Contribution Marker'],
{ HOME: homeDir },
projectRoot,
);
expect(res.code, res.output).toBe(0);

const recallRes = await runCLI(['recall', 'Zzyzx Contribution Marker'], { HOME: homeDir }, projectRoot);
expect(recallRes.code, recallRes.output).toBe(0);
expect(recallRes.output).toContain('Zzyzx Contribution Marker');
}, 20_000);
});
});
9 changes: 4 additions & 5 deletions src/__tests__/scope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,15 @@ scope: 'project',
expect(resolveBaseDir(config)).toBe('/Users/testuser/my-project');
});

it('should fallback to HOME if project scope without projectRoot', () => {
it('should throw instead of silently falling back to HOME if project scope without projectRoot (#85)', () => {
const config: LocalConfig = {
repo: { localPath: '/tmp/repo', remote: 'https://example.com' },
username: 'test',
updatePolicy: 'auto',
additionalRoles: [],
scope: 'project',
};
expect(resolveBaseDir(config)).toBe('/Users/testuser');
expect(() => resolveBaseDir(config)).toThrow(/projectRoot is missing/);
});
});

Expand All @@ -138,9 +138,8 @@ describe('getTeamaiHome', () => {
expect(result).toBe('/Users/test/proj/.teamai');
});

it('should fallback to ~/.teamai if project scope without projectRoot', () => {
const result = getTeamaiHome('project');
expect(result).toBe(`${process.env.HOME}/.teamai`);
it('should throw instead of silently falling back to ~/.teamai if project scope without projectRoot (#85)', () => {
expect(() => getTeamaiHome('project')).toThrow(/projectRoot is missing/);
});
});

Expand Down
17 changes: 14 additions & 3 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,15 @@ export async function loadLocalConfigForScope(
try {
const raw = YAML.parse(content);
const parsed = LocalConfigSchema.parse(raw);
return await migrateLegacyRoleConfig(parsed, configPath);
// Config files written before `projectRoot` was added to the schema (or
// hand-edited) may be missing it. We already know the project root — it's
// the directory this config was loaded for — so backfill it instead of
// letting getTeamaiHome()/resolveBaseDir() silently fall back to the user
// home directory later (#85).
const withProjectRoot = scope === 'project' && projectRoot && !parsed.projectRoot
? { ...parsed, projectRoot }
: parsed;
return await migrateLegacyRoleConfig(withProjectRoot, configPath);
} catch (e) {
log.error(`Invalid ${scope} config at ${configPath}: ${(e as Error).message}`);
return null;
Expand Down Expand Up @@ -187,8 +195,11 @@ export async function detectProjectConfig(cwd?: string): Promise<LocalConfig | n
try {
const raw = YAML.parse(content);
const config = LocalConfigSchema.parse(raw);
if (config.scope === 'project') return config;
return null;
if (config.scope !== 'project') return null;
// Backfill projectRoot from the directory we actually found the config
// in, so callers never see scope === 'project' with projectRoot missing
// (see loadLocalConfigForScope for the same backfill) (#85).
return config.projectRoot ? config : { ...config, projectRoot: dir };
} catch {
return null;
}
Expand Down
Loading
Loading