diff --git a/src/api/list.test.ts b/src/api/list.test.ts index dfa2ed5..46b445f 100644 --- a/src/api/list.test.ts +++ b/src/api/list.test.ts @@ -14,6 +14,13 @@ vi.mock('../lib/github.js', () => ({ isGhInstalled: vi.fn(), })); +vi.mock('../lib/config.js', () => ({ + loadConfig: vi.fn(() => ({ + baseBranch: 'main', + worktreePattern: 'pr{number}.{slug}', + })), +})); + vi.mock('../lib/lswt/index.js', () => ({ gatherWorktreeInfo: vi.fn(), createDefaultDeps: vi.fn(() => ({})), diff --git a/src/api/list.ts b/src/api/list.ts index 5b0ba4e..0808d28 100644 --- a/src/api/list.ts +++ b/src/api/list.ts @@ -18,6 +18,7 @@ import { } from '../lib/json-output.js'; import * as git from '../lib/git.js'; import * as github from '../lib/github.js'; +import { loadConfig } from '../lib/config.js'; /** * Information about a single worktree @@ -116,11 +117,14 @@ export async function listWorktrees( } // Build options for the lib function + const config = loadConfig(repoRoot); const listOptions: ListOptions = { json: true, // We always want structured output verbose: true, // Include all info showStatus: effectiveShowStatus, interactive: false, // Never interactive for API + worktreePattern: config.worktreePattern, + baseBranch: config.baseBranch, }; // Gather worktree info diff --git a/src/cli/lswt.ts b/src/cli/lswt.ts index 79e7bcd..f01693a 100644 --- a/src/cli/lswt.ts +++ b/src/cli/lswt.ts @@ -104,6 +104,7 @@ async function main(): Promise { // Load config for worktree pattern const config = loadConfig(repoRoot); options.worktreePattern = config.worktreePattern; + options.baseBranch = config.baseBranch; // Gather worktree info const deps = createDefaultDeps(); diff --git a/src/cli/wt/interactive-menu.test.ts b/src/cli/wt/interactive-menu.test.ts index 64d6049..a1eae2f 100644 --- a/src/cli/wt/interactive-menu.test.ts +++ b/src/cli/wt/interactive-menu.test.ts @@ -193,7 +193,13 @@ describe('Interactive Menu Flows', () => { expect(gatherWorktreeInfo).toHaveBeenCalledWith( '/mock/repo', - { verbose: false, json: false, showStatus: false }, + { + verbose: false, + json: false, + showStatus: false, + worktreePattern: '{repo}.pr{number}', + baseBranch: 'main', + }, expect.anything() ); expect(runInteractiveMode).toHaveBeenCalled(); diff --git a/src/cli/wt/interactive-menu.ts b/src/cli/wt/interactive-menu.ts index f0a81ec..d59a2db 100644 --- a/src/cli/wt/interactive-menu.ts +++ b/src/cli/wt/interactive-menu.ts @@ -260,10 +260,17 @@ async function handleListWorktrees(): Promise { console.log(); try { const repoRoot = git.getRepoRoot(); + const config = loadConfig(repoRoot); const deps = createLswtDeps(); const worktrees = await gatherWorktreeInfo( repoRoot, - { verbose: false, json: false, showStatus: false }, + { + verbose: false, + json: false, + showStatus: false, + worktreePattern: config.worktreePattern, + baseBranch: config.baseBranch, + }, deps ); // Run interactive mode (same as standalone lswt with no args in TTY) diff --git a/src/cli/wt/list.ts b/src/cli/wt/list.ts index a29efd0..1a0a0a1 100644 --- a/src/cli/wt/list.ts +++ b/src/cli/wt/list.ts @@ -128,6 +128,7 @@ export const listCommand: CommandModule = { // Pass worktreePattern through to extractPrNumber options.worktreePattern = config.worktreePattern; + options.baseBranch = config.baseBranch; // Gather worktree info const deps = createDefaultDeps(); diff --git a/src/integration/worktree-layout.integration.test.ts b/src/integration/worktree-layout.integration.test.ts index dcb4a5d..d1bb51b 100644 --- a/src/integration/worktree-layout.integration.test.ts +++ b/src/integration/worktree-layout.integration.test.ts @@ -4,7 +4,16 @@ import fs from 'fs'; import path from 'path'; import os from 'os'; import * as git from '../lib/git.js'; -import { getDefaultConfig, generateWorktreePath } from '../lib/config.js'; +import { + getDefaultConfig, + getConfigPath, + generateWorktreePath, + loadConfigWithValidation, +} from '../lib/config.js'; +import { + gatherWorktreeInfo, + createDefaultDeps as createLswtDeps, +} from '../lib/lswt/worktree-info.js'; /** * Integration tests for worktree layout anchoring against a REAL bare-repository @@ -21,6 +30,7 @@ function normalizePath(p: string): string { describe('worktree layout anchoring integration', () => { let tempDir: string; + let seedRepo: string; let container: string; let mainWorktree: string; let prWorktree: string; @@ -30,18 +40,22 @@ describe('worktree layout anchoring integration', () => { // Seed a normal repo with one commit, then clone it bare — the standard // way to set up a .bare/ + worktrees layout. - const seed = path.join(tempDir, 'seed'); - fs.mkdirSync(seed); - execSync('git init -q -b main', { cwd: seed }); - execSync('git config user.email test@test.com', { cwd: seed }); - execSync('git config user.name Test', { cwd: seed }); - fs.writeFileSync(path.join(seed, 'README.md'), 'seed\n'); - execSync('git add README.md', { cwd: seed }); - execSync('git commit -q -m initial', { cwd: seed }); + seedRepo = path.join(tempDir, 'seed'); + fs.mkdirSync(seedRepo); + execSync('git init -q -b main', { cwd: seedRepo }); + execSync('git config user.email test@test.com', { cwd: seedRepo }); + execSync('git config user.name Test', { cwd: seedRepo }); + fs.writeFileSync(path.join(seedRepo, 'README.md'), 'seed\n'); + fs.writeFileSync( + path.join(seedRepo, '.worktreerc'), + JSON.stringify({ worktreeParent: '.worktrees', worktreePattern: 'pr{number}.{slug}' }) + ); + execSync('git add README.md .worktreerc', { cwd: seedRepo }); + execSync('git commit -q -m initial', { cwd: seedRepo }); container = path.join(tempDir, 'container'); fs.mkdirSync(container); - execSync(`git clone --bare -q "${seed}" "${path.join(container, '.bare')}"`); + execSync(`git clone --bare -q "${seedRepo}" "${path.join(container, '.bare')}"`); mainWorktree = path.join(container, 'main'); execSync(`git worktree add -q "${mainWorktree}" main`, { @@ -53,6 +67,10 @@ describe('worktree layout anchoring integration', () => { execSync(`git worktree add -q -b feat/existing-feature "${prWorktree}" main`, { cwd: path.join(container, '.bare'), }); + fs.writeFileSync( + path.join(mainWorktree, '.worktreerc.local'), + JSON.stringify({ worktreeParent: '../pr' }) + ); }); afterAll(() => { @@ -72,6 +90,123 @@ describe('worktree layout anchoring integration', () => { expect(normalizePath(git.getMainWorktreeRoot(prWorktree))).toBe(normalizePath(container)); }); + it('finds the canonical main checkout from a linked pr worktree', () => { + expect(normalizePath(git.getMainWorktree(prWorktree)?.path ?? '')).toBe( + normalizePath(mainWorktree) + ); + }); + + it('loads the main checkout local override when invoked from a linked worktree', () => { + const result = loadConfigWithValidation(prWorktree); + + expect(result.config.worktreeParent).toBe('../pr'); + expect(normalizePath(result.configPath ?? '')).toBe( + normalizePath(path.join(mainWorktree, '.worktreerc.local')) + ); + }); + + it('selects the canonical local override for config operations from a linked worktree', () => { + expect(normalizePath(getConfigPath(prWorktree) ?? '')).toBe( + normalizePath(path.join(mainWorktree, '.worktreerc.local')) + ); + }); + + it('resolves a canonical local override under the nested workspace container', () => { + const config = loadConfigWithValidation(prWorktree).config; + const result = generateWorktreePath( + config, + prWorktree, + 'container', + 2600, + 'feat/new-feature', + git.getMainWorktreeRoot(prWorktree) + ); + + expect(normalizePath(result)).toBe( + normalizePath(path.join(container, 'pr', 'pr2600.new-feature')) + ); + }); + + it('lists canonical main correctly when invoked from a linked worktree', async () => { + const config = loadConfigWithValidation(prWorktree).config; + const result = await gatherWorktreeInfo( + prWorktree, + { + showStatus: false, + json: true, + verbose: true, + worktreePattern: 'pr{number}.{slug}', + baseBranch: config.baseBranch, + }, + createLswtDeps() + ); + + expect( + result.find((worktree) => normalizePath(worktree.path) === normalizePath(mainWorktree))?.type + ).toBe('main'); + expect( + result.find((worktree) => normalizePath(worktree.path) === normalizePath(prWorktree))?.type + ).toBe('pr'); + }); + + it('bootstraps a local-only non-default base branch with a custom bare directory', async () => { + const developContainer = path.join(tempDir, 'develop-container'); + const bareRepo = path.join(developContainer, 'repository.git'); + const developWorktree = path.join(developContainer, 'develop'); + const featureWorktree = path.join(developContainer, 'pr', 'pr2.feature'); + fs.mkdirSync(developContainer); + execSync(`git clone --bare -q "${seedRepo}" "${bareRepo}"`); + execSync('git update-ref refs/heads/develop refs/heads/main', { cwd: bareRepo }); + execSync('git symbolic-ref HEAD refs/heads/develop', { cwd: bareRepo }); + execSync('git update-ref -d refs/heads/main', { cwd: bareRepo }); + execSync(`git worktree add -q "${developWorktree}" develop`, { cwd: bareRepo }); + fs.mkdirSync(path.dirname(featureWorktree)); + execSync(`git worktree add -q -b feat/feature "${featureWorktree}" develop`, { + cwd: bareRepo, + }); + fs.writeFileSync( + path.join(developWorktree, '.worktreerc.local'), + JSON.stringify({ baseBranch: 'develop', worktreeParent: '../pr' }) + ); + + const loaded = loadConfigWithValidation(featureWorktree); + const worktrees = await gatherWorktreeInfo( + featureWorktree, + { + showStatus: false, + json: true, + verbose: true, + worktreePattern: loaded.config.worktreePattern, + baseBranch: loaded.config.baseBranch, + }, + createLswtDeps() + ); + const generatedPath = generateWorktreePath( + loaded.config, + featureWorktree, + 'develop-container', + 2601, + 'feat/another-feature', + git.getMainWorktreeRoot(featureWorktree) + ); + + expect(loaded.config.baseBranch).toBe('develop'); + expect(loaded.config.worktreeParent).toBe('../pr'); + expect(normalizePath(loaded.configPath ?? '')).toBe( + normalizePath(path.join(developWorktree, '.worktreerc.local')) + ); + expect(normalizePath(git.getMainWorktree(featureWorktree, 'develop')?.path ?? '')).toBe( + normalizePath(developWorktree) + ); + expect( + worktrees.find((worktree) => normalizePath(worktree.path) === normalizePath(developWorktree)) + ?.type + ).toBe('main'); + expect(normalizePath(generatedPath)).toBe( + normalizePath(path.join(developContainer, 'pr', 'pr2601.another-feature')) + ); + }); + it('places a new pr worktree under the container, invoked from main', () => { const config = { ...getDefaultConfig(), diff --git a/src/lib/config-editor.test.ts b/src/lib/config-editor.test.ts index 7f18fe7..9454d2e 100644 --- a/src/lib/config-editor.test.ts +++ b/src/lib/config-editor.test.ts @@ -97,6 +97,7 @@ describe('config-editor', () => { configPath: null, validation: null, }); + (config.getConfigPath as ReturnType).mockReturnValueOnce(null); (prompts.promptChoice as ReturnType).mockResolvedValueOnce('__exit__'); await runConfigEditor('/repo'); @@ -252,30 +253,46 @@ describe('config-editor', () => { it('sets boolean value directly', async () => { const result = await quickEditConfig('/repo', 'draftPr', 'true'); - expect(config.saveConfig).toHaveBeenCalledWith('/repo', { draftPr: true }); + expect(config.saveConfig).toHaveBeenCalledWith( + '/repo', + { draftPr: true }, + { configPath: '/repo/.worktreerc' } + ); expect(result.saved).toBe(true); }); it('sets boolean false with "false"', async () => { const result = await quickEditConfig('/repo', 'draftPr', 'false'); - expect(config.saveConfig).toHaveBeenCalledWith('/repo', { draftPr: false }); + expect(config.saveConfig).toHaveBeenCalledWith( + '/repo', + { draftPr: false }, + { configPath: '/repo/.worktreerc' } + ); expect(result.saved).toBe(true); }); it('sets boolean true with "1"', async () => { const result = await quickEditConfig('/repo', 'draftPr', '1'); - expect(config.saveConfig).toHaveBeenCalledWith('/repo', { draftPr: true }); + expect(config.saveConfig).toHaveBeenCalledWith( + '/repo', + { draftPr: true }, + { configPath: '/repo/.worktreerc' } + ); expect(result.saved).toBe(true); }); it('sets number value directly', async () => { const result = await quickEditConfig('/repo', 'hookDefaults.timeout', '60000'); - expect(config.saveConfig).toHaveBeenCalledWith('/repo', { - hookDefaults: { timeout: 60000 }, - }); + expect(config.saveConfig).toHaveBeenCalledWith( + '/repo', + { + hookDefaults: { timeout: 60000 }, + }, + { configPath: '/repo/.worktreerc' } + ); expect(result.saved).toBe(true); }); @@ -289,16 +306,24 @@ describe('config-editor', () => { it('sets array value from comma-separated string', async () => { const result = await quickEditConfig('/repo', 'sharedRepos', 'repo1,repo2,repo3'); - expect(config.saveConfig).toHaveBeenCalledWith('/repo', { - sharedRepos: ['repo1', 'repo2', 'repo3'], - }); + expect(config.saveConfig).toHaveBeenCalledWith( + '/repo', + { + sharedRepos: ['repo1', 'repo2', 'repo3'], + }, + { configPath: '/repo/.worktreerc' } + ); expect(result.saved).toBe(true); }); it('sets string value directly', async () => { const result = await quickEditConfig('/repo', 'baseBranch', 'develop'); - expect(config.saveConfig).toHaveBeenCalledWith('/repo', { baseBranch: 'develop' }); + expect(config.saveConfig).toHaveBeenCalledWith( + '/repo', + { baseBranch: 'develop' }, + { configPath: '/repo/.worktreerc' } + ); expect(result.saved).toBe(true); }); @@ -341,18 +366,40 @@ describe('config-editor', () => { it('sets enum value directly', async () => { const result = await quickEditConfig('/repo', 'preferredEditor', 'cursor'); - expect(config.saveConfig).toHaveBeenCalledWith('/repo', { preferredEditor: 'cursor' }); + expect(config.saveConfig).toHaveBeenCalledWith( + '/repo', + { preferredEditor: 'cursor' }, + { configPath: '/repo/.worktreerc' } + ); expect(result.saved).toBe(true); }); it('sets nested AI config value', async () => { const result = await quickEditConfig('/repo', 'ai.provider', 'claude'); - expect(config.saveConfig).toHaveBeenCalledWith('/repo', { - ai: { provider: 'claude' }, - }); + expect(config.saveConfig).toHaveBeenCalledWith( + '/repo', + { + ai: { provider: 'claude' }, + }, + { configPath: '/repo/.worktreerc' } + ); expect(result.saved).toBe(true); }); + + it('saves to the canonical active config path', async () => { + (config.getConfigPath as ReturnType).mockReturnValueOnce( + '/workspace/repo/main/.worktreerc.local' + ); + + await quickEditConfig('/workspace/repo/pr/pr42.feature', 'baseBranch', 'develop'); + + expect(config.saveConfig).toHaveBeenCalledWith( + '/workspace/repo/pr/pr42.feature', + { baseBranch: 'develop' }, + { configPath: '/workspace/repo/main/.worktreerc.local' } + ); + }); }); describe('editProperty via runConfigEditor', () => { @@ -639,7 +686,8 @@ describe('config-editor', () => { expect.objectContaining({ baseBranch: 'develop', branchPrefix: 'fix', - }) + }), + { configPath: '/repo/.worktreerc' } ); }); }); diff --git a/src/lib/config-editor.ts b/src/lib/config-editor.ts index a4d7d5d..d49cb72 100644 --- a/src/lib/config-editor.ts +++ b/src/lib/config-editor.ts @@ -15,6 +15,7 @@ import { green, dim, cyan, yellow, red, bold } from './colors.js'; import { loadConfigWithValidation, saveConfig, + getConfigPath, getDefaultConfig, type WorktreeConfig, type ResolvedConfig, @@ -491,13 +492,10 @@ export async function runConfigEditor(repoRoot: string): Promise { const { config: currentConfig } = loadConfigWithValidation(repoRoot, { warnOnErrors: false }); + const configPath = getConfigPath(repoRoot); // Find the property definition let property: ConfigProperty | undefined; @@ -877,7 +880,9 @@ export async function quickEditConfig( // Save try { - const result = saveConfig(repoRoot, modifiedConfig); + const result = saveConfig(repoRoot, modifiedConfig, { + configPath: configPath ?? undefined, + }); console.log(green(`Saved to ${result.configPath}`)); return { saved: true, configPath: result.configPath }; } catch (error) { diff --git a/src/lib/config.test.ts b/src/lib/config.test.ts index f3cc4f5..3cd693a 100644 --- a/src/lib/config.test.ts +++ b/src/lib/config.test.ts @@ -6,6 +6,7 @@ import { loadConfig, generateBranchNameAsync, generatePRContentAsync, + resolveRelativeWorktreeParent, } from './config.js'; import * as path from 'path'; import * as fs from 'fs'; @@ -366,6 +367,18 @@ describe('config', () => { '/home/user/repos/myproject/.worktrees/pr42.fix-login-bug' ); }); + + it('clamps leading traversal inside a bare-container anchor', () => { + expect(normalizePath(resolveRelativeWorktreeParent('/workspace/repo', '../pr', true))).toBe( + '/workspace/repo/pr' + ); + }); + + it('clamps embedded traversal inside a bare-container anchor', () => { + expect( + normalizePath(resolveRelativeWorktreeParent('/workspace/repo', 'pr/../../outside', true)) + ).toBe('/workspace/repo/outside'); + }); }); describe('loadConfig', () => { diff --git a/src/lib/config.ts b/src/lib/config.ts index 7a20a0d..6157d8b 100644 --- a/src/lib/config.ts +++ b/src/lib/config.ts @@ -29,6 +29,7 @@ import { } from './global-config.js'; import { logger } from './logger.js'; import { printStatus } from './ui/index.js'; +import * as git from './git.js'; /** * Hook execution defaults configuration @@ -489,6 +490,41 @@ function loadSingleConfigFile( } } +/** + * Find the checkout that owns repository-local configuration. + * + * Conventional repositories always use their primary checkout. Bare-container + * layouts normally use the configured base-branch checkout; when that branch is + * declared only in a local config, a unique self-describing checkout bootstraps + * discovery without falling back to an arbitrary path-sorted worktree. + */ +function findCanonicalLocalConfigRoot(repoRoot: string, baseBranch: string): string { + try { + if (git.isBareContainerLayout(repoRoot)) { + const selfDescribingRoots = git + .listWorktrees(repoRoot) + .filter((worktree) => !worktree.isBare && worktree.branch !== null) + .filter((worktree) => { + const localPath = findLocalConfigFile(worktree.path); + if (!localPath) { + return false; + } + const localSource = loadSingleConfigFile(localPath, 'local', false); + return localSource?.config.baseBranch === worktree.branch; + }) + .map((worktree) => worktree.path); + + if (selfDescribingRoots.length === 1) { + return selfDescribingRoots[0]; + } + } + + return git.getMainWorktree(repoRoot, baseBranch)?.path ?? repoRoot; + } catch { + return repoRoot; + } +} + /** * Load configuration with full validation result * Implements three-tier hierarchy: defaults ← global ← repo ← local @@ -534,8 +570,15 @@ export function loadConfigWithValidation( } } - // 3. Load local config (highest priority) - const localConfigPath = findLocalConfigFile(repoRoot); + // 3. Load local config (highest priority). A gitignored local config exists + // only in one checkout, so commands invoked from another linked worktree + // must read it from the canonical base-branch checkout. + const baseBranch = + [...sources].reverse().find((source) => source.config.baseBranch !== undefined)?.config + .baseBranch ?? defaults.baseBranch; + const localConfigRoot = findCanonicalLocalConfigRoot(repoRoot, baseBranch); + + const localConfigPath = findLocalConfigFile(localConfigRoot); if (localConfigPath) { const localSource = loadSingleConfigFile(localConfigPath, 'local', validate); if (localSource) { @@ -648,7 +691,7 @@ function mergeConfigs(base: ResolvedConfig, override: WorktreeConfig): ResolvedC export function saveConfig( repoRoot: string, config: WorktreeConfig, - options: { validate?: boolean } = {} + options: { validate?: boolean; configPath?: string } = {} ): { configPath: string; validation: ValidationResult | null } { const { validate = true } = options; @@ -662,7 +705,7 @@ export function saveConfig( } // Find existing config or use default name - let configPath = findRepoConfigFile(repoRoot); + let configPath = options.configPath ?? findRepoConfigFile(repoRoot); if (!configPath) { configPath = path.join(repoRoot, CONFIG_FILE_NAMES[0]); // Use .worktreerc } @@ -730,15 +773,12 @@ function deepMergeConfigs(base: WorktreeConfig, override: WorktreeConfig): Workt * Returns the highest priority config that exists (local > repo) */ export function getConfigPath(repoRoot: string): string | null { - // Check local config first (highest priority) - const localPath = findLocalConfigFile(repoRoot); - if (localPath) return localPath; - - // Then check repo config - const repoPath = findRepoConfigFile(repoRoot); - if (repoPath) return repoPath; - - return null; + const result = loadConfigWithValidation(repoRoot, { + validate: false, + warnOnErrors: false, + }); + const repositorySources = result.sources.filter((source) => source.level !== 'global'); + return repositorySources.at(-1)?.path ?? null; } /** @@ -804,12 +844,58 @@ export function generateWorktreePath( } else { const anchor = config.worktreeParentAnchor === 'repo-root' ? repoRoot : (mainWorktreeRoot ?? repoRoot); - parentDir = path.resolve(anchor, config.worktreeParent); + const containWithinAnchor = + config.worktreeParentAnchor !== 'repo-root' && git.isBareContainerLayout(repoRoot); + parentDir = resolveRelativeWorktreeParent(anchor, config.worktreeParent, containWithinAnchor); } return path.join(parentDir, pattern); } +/** + * Resolve a relative worktree parent without silently escaping a bare-repository + * container. This keeps legacy checkout-relative overrides such as `../pr` + * compatible after `main-worktree` anchoring moved to the container root. + */ +export function resolveRelativeWorktreeParent( + anchor: string, + worktreeParent: string, + containWithinAnchor = false +): string { + const resolved = path.resolve(anchor, worktreeParent); + + if (!containWithinAnchor || isWithin(anchor, resolved)) { + return resolved; + } + + const containedSegments: string[] = []; + for (const segment of path.normalize(worktreeParent).split(path.sep)) { + if (!segment || segment === '.') { + continue; + } + if (segment === '..') { + containedSegments.pop(); + continue; + } + containedSegments.push(segment); + } + const containedParent = containedSegments.join(path.sep); + const contained = path.resolve(anchor, containedParent); + + logger.warn( + `worktreeParent "${worktreeParent}" resolves outside the bare-repository container ` + + `"${anchor}"; using "${contained}" instead. Update the relative parent or use an ` + + `absolute path / worktreeParentAnchor: "repo-root" to place worktrees outside it.` + ); + + return contained; +} + +function isWithin(root: string, candidate: string): boolean { + const relative = path.relative(root, candidate); + return relative === '' || (!relative.startsWith('..') && !path.isAbsolute(relative)); +} + /** * Generate branch name from description (synchronous, rule-based) */ diff --git a/src/lib/git.test.ts b/src/lib/git.test.ts index 6d1c6a0..776e442 100644 --- a/src/lib/git.test.ts +++ b/src/lib/git.test.ts @@ -334,6 +334,81 @@ describe('git', () => { }); }); + describe('getMainWorktree', () => { + it('preserves the primary checkout in a conventional repository', () => { + mockSpawnSync.mockReturnValue( + mockSpawnSuccess( + 'worktree /workspace/repo\n' + + 'HEAD abc123\n' + + 'branch refs/heads/feat/current\n' + + '\n' + + 'worktree /workspace/linked-main\n' + + 'HEAD def456\n' + + 'branch refs/heads/main\n' + + '\n' + ) + ); + + expect(git.getMainWorktree('/workspace/repo')?.path).toBe('/workspace/repo'); + }); + + it('selects the base-branch checkout in a bare-container layout', () => { + mockSpawnSync.mockReturnValue( + mockSpawnSuccess( + 'worktree /workspace/repo/.bare\n' + + 'bare\n' + + '\n' + + 'worktree /workspace/repo/agents/first\n' + + 'HEAD abc123\n' + + 'branch refs/heads/agent/first\n' + + '\n' + + 'worktree /workspace/repo/main\n' + + 'HEAD def456\n' + + 'branch refs/heads/main\n' + + '\n' + ) + ); + + expect(git.getMainWorktree('/workspace/repo/pr/pr42.feature')?.path).toBe( + '/workspace/repo/main' + ); + }); + + it('honors a configured non-main base branch', () => { + mockSpawnSync.mockReturnValue( + mockSpawnSuccess( + 'worktree /workspace/repo/.bare\n' + + 'bare\n' + + '\n' + + 'worktree /workspace/repo/develop\n' + + 'HEAD abc123\n' + + 'branch refs/heads/develop\n' + + '\n' + ) + ); + + expect(git.getMainWorktree('/workspace/repo/pr/pr42.feature', 'develop')?.path).toBe( + '/workspace/repo/develop' + ); + }); + + it('does not select an arbitrary checkout when the bare base branch is absent', () => { + mockSpawnSync.mockReturnValue( + mockSpawnSuccess( + 'worktree /workspace/repo/repository.git\n' + + 'bare\n' + + '\n' + + 'worktree /workspace/repo/agents/first\n' + + 'HEAD abc123\n' + + 'branch refs/heads/agent/first\n' + + '\n' + ) + ); + + expect(git.getMainWorktree('/workspace/repo/agents/first')).toBeNull(); + }); + }); + describe('getCommitRelationship', () => { it('returns same when HEAD equals base', () => { mockSpawnSync @@ -662,6 +737,12 @@ describe('git', () => { expect(git.isBareContainerLayout(path.join(containerPath, 'main'))).toBe(true); }); + it('returns true for a bare repository with a custom directory name', () => { + const containerPath = path.join('/home', 'chris', 'workspace', 'repo'); + mockSpawnSync.mockReturnValue(mockSpawnSuccess(path.join(containerPath, 'repository.git'))); + expect(git.isBareContainerLayout(path.join(containerPath, 'develop'))).toBe(true); + }); + it('returns false when git-common-dir lookup fails', () => { mockSpawnSync.mockReturnValue(mockSpawnFailure('not a git repository')); expect(git.isBareContainerLayout()).toBe(false); diff --git a/src/lib/git.ts b/src/lib/git.ts index 94fd0bd..ca90613 100644 --- a/src/lib/git.ts +++ b/src/lib/git.ts @@ -423,9 +423,26 @@ export function listWorktrees(cwd?: string): Worktree[] { /** * Find the main worktree */ -export function getMainWorktree(cwd?: string): Worktree | null { +export function getMainWorktree( + cwd?: string, + baseBranch: string = DEFAULT_BASE_BRANCH +): Worktree | null { const worktrees = listWorktrees(cwd); - return worktrees.find((w) => w.isMain && !w.isBare) || null; + const primaryWorktree = worktrees.find((w) => w.isMain && !w.isBare); + + // In a conventional repository git lists the primary checkout first and + // listWorktrees marks it as main, and it stays canonical even when another + // linked worktree has the configured base branch checked out. + if (!worktrees[0]?.isBare) { + return primaryWorktree ?? null; + } + + // A bare-container layout has no primary checkout entry: the bare repository + // is first and every checkout is linked. The canonical checkout is therefore + // the one carrying the configured base branch. Do not fall back to the first + // path-sorted checkout when that branch is absent; callers can then bootstrap + // discovery from repository-local configuration without choosing arbitrarily. + return worktrees.find((w) => !w.isBare && w.branch === baseBranch) ?? null; } /** @@ -433,9 +450,11 @@ export function getMainWorktree(cwd?: string): Worktree | null { */ export function isWorktree(cwd?: string): boolean { const repoRoot = getRepoRoot(cwd); - const worktrees = listWorktrees(cwd); - const current = worktrees.find((w) => path.normalize(w.path) === path.normalize(repoRoot)); - return current ? !current.isMain : false; + const mainWorktree = getMainWorktree(cwd); + if (!mainWorktree) { + return false; + } + return path.normalize(mainWorktree.path) !== path.normalize(repoRoot); } /** diff --git a/src/lib/lswt/action-executors.test.ts b/src/lib/lswt/action-executors.test.ts index 97f0ac6..24e12fa 100644 --- a/src/lib/lswt/action-executors.test.ts +++ b/src/lib/lswt/action-executors.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import path from 'path'; import { executeAction, createDefaultExecutorDeps, @@ -26,7 +27,9 @@ vi.mock('../github.js', () => ({ vi.mock('../git.js', () => ({ getRepoRoot: vi.fn(), removeWorktree: vi.fn(), + getMainWorktree: vi.fn(), getMainWorktreeRoot: vi.fn(), + isBareContainerLayout: vi.fn(() => false), addWorktree: vi.fn(), deleteBranch: vi.fn(), exec: vi.fn(), @@ -1368,7 +1371,7 @@ describe('lswt/action-executors', () => { }); it('returns error when repo root cannot be found', async () => { - vi.mocked(git.getMainWorktreeRoot).mockReturnValue(null as unknown as string); + vi.mocked(git.getMainWorktree).mockReturnValue(null); const worktree = makeWorktree({ type: 'remote_pr', @@ -1391,6 +1394,15 @@ describe('lswt/action-executors', () => { it('successfully creates worktree for remote PR', async () => { const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + vi.mocked(git.getMainWorktree).mockReturnValue({ + path: '/home/user/repo', + branch: 'main', + commit: 'abc123', + isMain: true, + isBare: false, + isLocked: false, + isPrunable: false, + }); vi.mocked(git.getMainWorktreeRoot).mockReturnValue('/home/user/repo'); vi.mocked(git.addWorktree).mockImplementation(() => {}); // Mock git.exec to return empty string (git fetch succeeds) @@ -1421,6 +1433,15 @@ describe('lswt/action-executors', () => { it('handles git fetch failure', async () => { const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + vi.mocked(git.getMainWorktree).mockReturnValue({ + path: '/home/user/repo', + branch: 'main', + commit: 'abc123', + isMain: true, + isBare: false, + isLocked: false, + isPrunable: false, + }); vi.mocked(git.getMainWorktreeRoot).mockReturnValue('/home/user/repo'); // Mock git.exec to throw (git fetch fails) vi.mocked(git.exec).mockImplementation(() => { @@ -1446,6 +1467,47 @@ describe('lswt/action-executors', () => { expect(result.message).toContain('Failed to checkout PR'); consoleSpy.mockRestore(); }); + + it('uses the configured canonical checkout for git and the container for placement', async () => { + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + vi.mocked(git.getMainWorktree).mockReturnValue({ + path: '/workspace/repo/develop', + branch: 'develop', + commit: 'abc123', + isMain: false, + isBare: false, + isLocked: false, + isPrunable: false, + }); + vi.mocked(git.getMainWorktreeRoot).mockReturnValue('/workspace/repo'); + vi.mocked(git.exec).mockReturnValue(''); + + const worktree = makeWorktree({ + type: 'remote_pr', + prNumber: 42, + branch: 'feat/remote-feature', + }); + const config = makeConfig({ + baseBranch: 'develop', + worktreeParent: 'pr', + worktreePattern: 'pr{number}.{slug}', + }); + + const result = await executeAction('checkout_pr', worktree, makeEnv(), config, makeDeps()); + + expect(result.success).toBe(true); + expect(git.getMainWorktree).toHaveBeenCalledWith(undefined, 'develop'); + expect(git.exec).toHaveBeenCalledWith( + ['fetch', 'origin', 'feat/remote-feature:feat/remote-feature'], + expect.objectContaining({ cwd: '/workspace/repo/develop' }) + ); + expect(git.addWorktree).toHaveBeenCalledWith( + path.resolve('/workspace/repo/pr/pr42.remote-feature'), + 'feat/remote-feature', + expect.objectContaining({ cwd: '/workspace/repo/develop' }) + ); + consoleSpy.mockRestore(); + }); }); describe('show_details action for remote_pr', () => { diff --git a/src/lib/lswt/action-executors.ts b/src/lib/lswt/action-executors.ts index 3692d73..cb34a47 100644 --- a/src/lib/lswt/action-executors.ts +++ b/src/lib/lswt/action-executors.ts @@ -609,19 +609,30 @@ async function checkoutPr( } try { - // Get repo root and name - const repoRoot = git.getMainWorktreeRoot(); - if (!repoRoot) { + const fullConfig = { ...getDefaultConfig(), ...config }; + + // Git mutations need a real checkout as cwd, while relative worktree + // placement uses the stable main-worktree/container anchor. + const mainWorktree = git.getMainWorktree(undefined, fullConfig.baseBranch); + if (!mainWorktree) { return { success: false, message: 'Could not find repository root', }; } - const repoName = path.basename(repoRoot); + const repoRoot = mainWorktree.path; + const mainWorktreeRoot = git.getMainWorktreeRoot(repoRoot); + const repoName = path.basename(mainWorktreeRoot); // Generate worktree path using config - const fullConfig = { ...getDefaultConfig(), ...config }; - const worktreePath = generateWorktreePath(fullConfig, repoRoot, repoName, prNumber, branch); + const worktreePath = generateWorktreePath( + fullConfig, + repoRoot, + repoName, + prNumber, + branch, + mainWorktreeRoot + ); console.log(colors.dim('\nFetching PR branch...')); diff --git a/src/lib/lswt/types.ts b/src/lib/lswt/types.ts index e4f3603..a040427 100644 --- a/src/lib/lswt/types.ts +++ b/src/lib/lswt/types.ts @@ -17,6 +17,8 @@ export interface ListOptions { noColor?: boolean; /** Worktree naming pattern for PR number extraction */ worktreePattern?: string; + /** Configured canonical base branch for main-worktree discovery */ + baseBranch?: string; } /** diff --git a/src/lib/lswt/worktree-info.test.ts b/src/lib/lswt/worktree-info.test.ts index 13462d3..e8c58f4 100644 --- a/src/lib/lswt/worktree-info.test.ts +++ b/src/lib/lswt/worktree-info.test.ts @@ -6,6 +6,7 @@ import type { ListOptions } from './types.js'; // Mock git vi.mock('../git.js', () => ({ listWorktrees: vi.fn(), + getMainWorktree: vi.fn(), getStatusOutput: vi.fn(), })); @@ -61,6 +62,48 @@ describe('lswt/worktree-info', () => { expect(result[0].name).toBe('repo'); }); + it('identifies the canonical main checkout when invoked from a linked worktree', async () => { + const canonicalMain = makeWorktree({ path: '/workspace/repo/main', branch: 'main' }); + const invokingPr = makeWorktree({ + path: '/workspace/repo/pr/pr42.feature', + branch: 'feat/feature', + isMain: false, + }); + const deps = makeDeps({ + listWorktrees: () => [invokingPr, canonicalMain], + getMainWorktree: () => canonicalMain, + }); + + const result = await gatherWorktreeInfo( + invokingPr.path, + { ...defaultOptions, worktreePattern: 'pr{number}.{slug}' }, + deps + ); + + expect(result.find((worktree) => worktree.path === canonicalMain.path)?.type).toBe('main'); + expect(result.find((worktree) => worktree.path === invokingPr.path)?.type).toBe('pr'); + }); + + it('passes the configured base branch when identifying the canonical checkout', async () => { + const canonicalDevelop = makeWorktree({ + path: '/workspace/repo/develop', + branch: 'develop', + }); + const getMainWorktree = vi.fn(() => canonicalDevelop); + const deps = makeDeps({ + listWorktrees: () => [canonicalDevelop], + getMainWorktree, + }); + + await gatherWorktreeInfo( + '/workspace/repo/pr/pr42.feature', + { ...defaultOptions, baseBranch: 'develop' }, + deps + ); + + expect(getMainWorktree).toHaveBeenCalledWith('/workspace/repo/pr/pr42.feature', 'develop'); + }); + it('identifies PR worktree from path pattern', async () => { const worktrees = [ makeWorktree({ path: '/home/user/repo', branch: 'main' }), @@ -480,9 +523,11 @@ describe('lswt/worktree-info', () => { const deps = createDefaultDeps(); expect(deps).toHaveProperty('listWorktrees'); + expect(deps).toHaveProperty('getMainWorktree'); expect(deps).toHaveProperty('hasUncommittedChanges'); expect(deps).toHaveProperty('getPrInfo'); expect(typeof deps.listWorktrees).toBe('function'); + expect(typeof deps.getMainWorktree).toBe('function'); expect(typeof deps.hasUncommittedChanges).toBe('function'); expect(typeof deps.getPrInfo).toBe('function'); }); @@ -500,6 +545,30 @@ describe('lswt/worktree-info', () => { }); }); + describe('getMainWorktree', () => { + it('calls git.getMainWorktree with provided cwd', () => { + const mainWorktree = { + path: '/workspace/repo/main', + branch: 'main', + commit: 'abc', + isMain: false, + isBare: false, + isLocked: false, + isPrunable: false, + }; + vi.mocked(git.getMainWorktree).mockReturnValue(mainWorktree); + + const deps = createDefaultDeps(); + expect(deps.getMainWorktree?.('/workspace/repo/pr/pr42.feature', 'develop')).toEqual( + mainWorktree + ); + expect(git.getMainWorktree).toHaveBeenCalledWith( + '/workspace/repo/pr/pr42.feature', + 'develop' + ); + }); + }); + describe('hasUncommittedChanges', () => { it('returns true when git status has output', () => { vi.mocked(git.getStatusOutput).mockReturnValue(' M file.txt\n'); diff --git a/src/lib/lswt/worktree-info.ts b/src/lib/lswt/worktree-info.ts index c536a76..d9f7c7b 100644 --- a/src/lib/lswt/worktree-info.ts +++ b/src/lib/lswt/worktree-info.ts @@ -34,6 +34,7 @@ export interface RemotePrInfo { */ export interface GatherDeps { listWorktrees: (cwd?: string) => Worktree[]; + getMainWorktree?: (cwd?: string, baseBranch?: string) => Worktree | null; hasUncommittedChanges: (worktreePath: string) => boolean; getPrInfo: (prNumber: number) => Promise; listOpenPrs: () => Promise; @@ -49,12 +50,13 @@ export async function gatherWorktreeInfo( deps: GatherDeps ): Promise { const worktrees = deps.listWorktrees(repoRoot); + const mainWorktreePath = deps.getMainWorktree?.(repoRoot, options.baseBranch)?.path ?? repoRoot; const result: WorktreeDisplay[] = []; for (const wt of worktrees) { const name = path.basename(wt.path); const prNumber = extractPrNumber(wt.path, { worktreePattern: options.worktreePattern }); - const isMain = isMainWorktree(wt.path, repoRoot); + const isMain = isMainWorktree(wt.path, mainWorktreePath); const hasChanges = deps.hasUncommittedChanges(wt.path); let type: WorktreeDisplay['type']; @@ -139,6 +141,7 @@ async function gatherRemotePrs( export function createDefaultDeps(): GatherDeps { return { listWorktrees: (cwd?: string) => git.listWorktrees(cwd), + getMainWorktree: (cwd?: string, baseBranch?: string) => git.getMainWorktree(cwd, baseBranch), hasUncommittedChanges: (worktreePath: string): boolean => { try {