From 04f8675624949cd61cb2c46b21caf56f796f8df5 Mon Sep 17 00:00:00 2001 From: "to.watanabe" Date: Fri, 20 Mar 2026 16:02:10 +0900 Subject: [PATCH] fix(clv2): use -e instead of -d for .git check in detect-project.sh In git worktrees, .git is a file (not a directory) containing a gitdir pointer. The -d test fails for worktree checkouts, causing project detection to fall through to the "global" fallback. Changing to -e (exists) handles both regular repos and worktrees correctly. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../scripts/detect-project.sh | 2 +- tests/hooks/detect-project-worktree.test.js | 239 ++++++++++++++++++ 2 files changed, 240 insertions(+), 1 deletion(-) create mode 100644 tests/hooks/detect-project-worktree.test.js diff --git a/skills/continuous-learning-v2/scripts/detect-project.sh b/skills/continuous-learning-v2/scripts/detect-project.sh index ed1988c5..47b1e363 100755 --- a/skills/continuous-learning-v2/scripts/detect-project.sh +++ b/skills/continuous-learning-v2/scripts/detect-project.sh @@ -85,7 +85,7 @@ _clv2_detect_project() { # fall back to path hash (machine-specific but still useful) local remote_url="" if command -v git &>/dev/null; then - if [ "$source_hint" = "git" ] || [ -d "${project_root}/.git" ]; then + if [ "$source_hint" = "git" ] || [ -e "${project_root}/.git" ]; then remote_url=$(git -C "$project_root" remote get-url origin 2>/dev/null || true) fi fi diff --git a/tests/hooks/detect-project-worktree.test.js b/tests/hooks/detect-project-worktree.test.js new file mode 100644 index 00000000..41c86372 --- /dev/null +++ b/tests/hooks/detect-project-worktree.test.js @@ -0,0 +1,239 @@ +/** + * Tests for worktree project-ID mismatch fix + * + * Validates that detect-project.sh uses -e (not -d) for .git existence + * checks, so that git worktrees (where .git is a file) are detected + * correctly. + * + * Run with: node tests/hooks/detect-project-worktree.test.js + */ + +const assert = require('assert'); +const path = require('path'); +const fs = require('fs'); +const os = require('os'); +const { execSync } = require('child_process'); + +let passed = 0; +let failed = 0; + +function test(name, fn) { + try { + fn(); + console.log(` \u2713 ${name}`); + passed++; + } catch (err) { + console.log(` \u2717 ${name}`); + console.log(` Error: ${err.message}`); + failed++; + } +} + +function createTempDir() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-worktree-test-')); +} + +function cleanupDir(dir) { + try { + fs.rmSync(dir, { recursive: true, force: true }); + } catch { + // ignore cleanup errors + } +} + +const repoRoot = path.resolve(__dirname, '..', '..'); +const detectProjectPath = path.join( + repoRoot, + 'skills', + 'continuous-learning-v2', + 'scripts', + 'detect-project.sh' +); + +console.log('\n=== Worktree Project-ID Mismatch Tests ===\n'); + +// ────────────────────────────────────────────────────── +// Group 1: Content checks on detect-project.sh +// ────────────────────────────────────────────────────── + +console.log('--- Content checks on detect-project.sh ---'); + +test('uses -e (not -d) for .git existence check', () => { + const content = fs.readFileSync(detectProjectPath, 'utf8'); + assert.ok( + content.includes('[ -e "${project_root}/.git" ]'), + 'detect-project.sh should use -e for .git check' + ); + assert.ok( + !content.includes('[ -d "${project_root}/.git" ]'), + 'detect-project.sh should NOT use -d for .git check' + ); +}); + +test('has command -v git fallback check', () => { + const content = fs.readFileSync(detectProjectPath, 'utf8'); + assert.ok( + content.includes('command -v git'), + 'detect-project.sh should check for git availability with command -v' + ); +}); + +test('uses git -C for safe directory operations', () => { + const content = fs.readFileSync(detectProjectPath, 'utf8'); + assert.ok( + content.includes('git -C'), + 'detect-project.sh should use git -C for directory-scoped operations' + ); +}); + +// ────────────────────────────────────────────────────── +// Group 2: Behavior test — -e vs -d +// ────────────────────────────────────────────────────── + +console.log('\n--- Behavior test: -e vs -d ---'); + +const behaviorDir = createTempDir(); + +test('[ -d ] returns true for .git directory', () => { + const dir = path.join(behaviorDir, 'test-d-dir'); + fs.mkdirSync(dir, { recursive: true }); + fs.mkdirSync(path.join(dir, '.git')); + const result = execSync(`bash -c '[ -d "${dir}/.git" ] && echo yes || echo no'`).toString().trim(); + assert.strictEqual(result, 'yes'); +}); + +test('[ -d ] returns false for .git file', () => { + const dir = path.join(behaviorDir, 'test-d-file'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, '.git'), 'gitdir: /some/path\n'); + const result = execSync(`bash -c '[ -d "${dir}/.git" ] && echo yes || echo no'`).toString().trim(); + assert.strictEqual(result, 'no'); +}); + +test('[ -e ] returns true for .git directory', () => { + const dir = path.join(behaviorDir, 'test-e-dir'); + fs.mkdirSync(dir, { recursive: true }); + fs.mkdirSync(path.join(dir, '.git')); + const result = execSync(`bash -c '[ -e "${dir}/.git" ] && echo yes || echo no'`).toString().trim(); + assert.strictEqual(result, 'yes'); +}); + +test('[ -e ] returns true for .git file', () => { + const dir = path.join(behaviorDir, 'test-e-file'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, '.git'), 'gitdir: /some/path\n'); + const result = execSync(`bash -c '[ -e "${dir}/.git" ] && echo yes || echo no'`).toString().trim(); + assert.strictEqual(result, 'yes'); +}); + +test('[ -e ] returns false when .git does not exist', () => { + const dir = path.join(behaviorDir, 'test-e-none'); + fs.mkdirSync(dir, { recursive: true }); + const result = execSync(`bash -c '[ -e "${dir}/.git" ] && echo yes || echo no'`).toString().trim(); + assert.strictEqual(result, 'no'); +}); + +cleanupDir(behaviorDir); + +// ────────────────────────────────────────────────────── +// Group 3: E2E test — detect-project.sh with worktree .git file +// ────────────────────────────────────────────────────── + +console.log('\n--- E2E: detect-project.sh with worktree .git file ---'); + +test('detect-project.sh sets PROJECT_NAME and non-global PROJECT_ID for worktree', () => { + const testDir = createTempDir(); + + try { + // Create a "main" repo with git init so we have real git structures + const mainRepo = path.join(testDir, 'main-repo'); + fs.mkdirSync(mainRepo, { recursive: true }); + execSync('git init', { cwd: mainRepo, stdio: 'pipe' }); + execSync('git commit --allow-empty -m "init"', { + cwd: mainRepo, + stdio: 'pipe', + env: { + ...process.env, + GIT_AUTHOR_NAME: 'Test', + GIT_AUTHOR_EMAIL: 'test@test.com', + GIT_COMMITTER_NAME: 'Test', + GIT_COMMITTER_EMAIL: 'test@test.com' + } + }); + + // Create a worktree-like directory with .git as a file + const worktreeDir = path.join(testDir, 'my-worktree'); + fs.mkdirSync(worktreeDir, { recursive: true }); + + // Set up the worktree directory structure in the main repo + const worktreesDir = path.join(mainRepo, '.git', 'worktrees', 'my-worktree'); + fs.mkdirSync(worktreesDir, { recursive: true }); + + // Create the gitdir file and commondir in the worktree metadata + const mainGitDir = path.join(mainRepo, '.git'); + fs.writeFileSync( + path.join(worktreesDir, 'commondir'), + '../..\n' + ); + fs.writeFileSync( + path.join(worktreesDir, 'HEAD'), + fs.readFileSync(path.join(mainGitDir, 'HEAD'), 'utf8') + ); + + // Write .git file in the worktree directory (this is what git worktree creates) + fs.writeFileSync( + path.join(worktreeDir, '.git'), + `gitdir: ${worktreesDir}\n` + ); + + // Source detect-project.sh from the worktree directory and capture results + const script = ` + export CLAUDE_PROJECT_DIR="${worktreeDir}" + export HOME="${testDir}" + source "${detectProjectPath}" + echo "PROJECT_NAME=\${PROJECT_NAME}" + echo "PROJECT_ID=\${PROJECT_ID}" + `; + + const result = execSync(`bash -c '${script.replace(/'/g, "'\\''")}'`, { + cwd: worktreeDir, + timeout: 10000, + env: { + ...process.env, + HOME: testDir, + CLAUDE_PROJECT_DIR: worktreeDir + } + }).toString(); + + const lines = result.trim().split('\n'); + const vars = {}; + for (const line of lines) { + const match = line.match(/^(PROJECT_NAME|PROJECT_ID)=(.*)$/); + if (match) { + vars[match[1]] = match[2]; + } + } + + assert.ok( + vars.PROJECT_NAME && vars.PROJECT_NAME.length > 0, + `PROJECT_NAME should be set, got: "${vars.PROJECT_NAME || ''}"` + ); + assert.ok( + vars.PROJECT_ID && vars.PROJECT_ID !== 'global', + `PROJECT_ID should not be "global", got: "${vars.PROJECT_ID || ''}"` + ); + } finally { + cleanupDir(testDir); + } +}); + +// ────────────────────────────────────────────────────── +// Summary +// ────────────────────────────────────────────────────── + +console.log('\n=== Test Results ==='); +console.log(`Passed: ${passed}`); +console.log(`Failed: ${failed}`); +console.log(`Total: ${passed + failed}\n`); + +process.exit(failed > 0 ? 1 : 0);