From dfad7cc219843f28a9fd7a083e7645ffaa2f4131 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=87=A1=E8=BE=9E?= Date: Mon, 27 Jul 2026 22:38:30 +0800 Subject: [PATCH] =?UTF-8?q?fix(repo):=20=E9=81=BF=E5=85=8D=E9=BB=98?= =?UTF-8?q?=E8=AE=A4=E7=9B=AE=E5=BD=95=E6=A8=A1=E5=BC=8F=E6=89=AB=E6=8F=8F?= =?UTF-8?q?=E7=94=A8=E6=88=B7=E4=B8=BB=E7=9B=AE=E5=BD=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/core/command-handler.ts | 41 ++++++++++++++-------------- src/core/session-manager.ts | 19 ++++++++++--- src/services/project-scanner.ts | 18 +++++++++---- test/command-handler.test.ts | 7 +++-- test/repo-selection.test.ts | 28 ++++++++++++++++++- test/session-manager-scan.test.ts | 45 +++++++++++++++++++++++++++++++ 6 files changed, 124 insertions(+), 34 deletions(-) diff --git a/src/core/command-handler.ts b/src/core/command-handler.ts index d188c1489..2e22143f7 100644 --- a/src/core/command-handler.ts +++ b/src/core/command-handler.ts @@ -205,20 +205,19 @@ export { validateWorkingDir }; * 1. Build candidate absolute paths — absolute / `~` taken as-is; relative or * bare names resolved against each scan dir, then the daemon cwd (mirrors * how the card's project list is rooted). - * 2. Prefer a candidate matching a scanned git project (carries a branch label). - * 3. For a bare name, also match a scanned project by basename (covers projects - * nested deeper than the scan-dir top level). - * 4. Fall back to any existing directory — lenient like `/cd`, whose trust model - * is "owner explicitly chose a dir"; the CLI already runs with full FS access. + * 2. Return the first directly existing candidate, describing its git ref + * without scanning unrelated roots. This is lenient like `/cd`, whose trust + * model is "owner explicitly chose a dir"; the CLI already runs with full + * FS access. + * 3. Only for a bare name that did not directly resolve, scan projects and + * match by basename (covers projects nested deeper than the scan-dir top + * level). * Returns null when nothing resolves to an existing directory. */ export function resolveRepoSelection( repoArg: string, scanDirs: string[], ): { path: string; displayName: string } | null { - const existingScanDirs = scanDirs.filter((d) => existsSync(d)); - const projects = existingScanDirs.length > 0 ? scanMultipleProjects(existingScanDirs) : []; - const isExplicitPath = repoArg.startsWith('/') || repoArg.startsWith('~') || @@ -233,18 +232,9 @@ export function resolveRepoSelection( candidates.push(resolve(expandHome(repoArg))); // daemon-cwd fallback (matches /cd) } - // 1) Exact scanned-project match — preferred, gives the "name (branch)" label. - for (const cand of candidates) { - const proj = projects.find((p) => resolve(p.path) === cand); - if (proj) return { path: proj.path, displayName: `${proj.name} (${proj.branch})` }; - } - // 2) Bare name → match a scanned project by basename. - if (!isExplicitPath) { - const byName = projects.find((p) => p.name === repoArg); - if (byName) return { path: byName.path, displayName: `${byName.name} (${byName.branch})` }; - } - // 3) Lenient fallback: any existing directory. Label it with a git ref when - // it's a repo (covers explicit paths outside the scan roots), else basename. + // Direct candidates must win before any recursive scan. Besides avoiding + // unnecessary traversal (especially a legacy HOME fallback), describing just + // the selected directory preserves the same "name (branch)" label for repos. for (const cand of candidates) { try { if (!statSync(cand).isDirectory()) continue; @@ -256,6 +246,17 @@ export function resolveRepoSelection( ? { path: cand, displayName: `${desc.name} (${desc.branch})` } : { path: cand, displayName: basename(cand) }; } + + // Explicit and relative paths have no basename-search semantics: when their + // concrete candidates do not exist, a recursive project scan cannot resolve + // them. Bare names alone may refer to a repo nested below a scan root. + if (isExplicitPath) return null; + + const existingScanDirs = scanDirs.filter((d) => existsSync(d)); + const projects = existingScanDirs.length > 0 ? scanMultipleProjects(existingScanDirs) : []; + const byName = projects.find((p) => p.name === repoArg); + if (byName) return { path: byName.path, displayName: `${byName.name} (${byName.branch})` }; + return null; } diff --git a/src/core/session-manager.ts b/src/core/session-manager.ts index 29d4b4deb..059085fff 100644 --- a/src/core/session-manager.ts +++ b/src/core/session-manager.ts @@ -222,9 +222,13 @@ export function getProjectScanDirs(ds?: DaemonSession): string[] { if (ds?.larkAppId) { const bot = getBot(ds.larkAppId); const dirs = new Set(); - const workingDirs = bot.config.workingDirs?.length - ? bot.config.workingDirs - : parseWorkingDirList(bot.config.workingDir ?? '~'); + const configuredMultiDirs = parseWorkingDirList(bot.config.workingDirs); + const configuredLegacyDirs = parseWorkingDirList(bot.config.workingDir); + const workingDirs = configuredMultiDirs.length > 0 + ? configuredMultiDirs + : configuredLegacyDirs.length > 0 + ? configuredLegacyDirs + : [effectiveDefaultWorkingDir(bot.config) ?? '~']; for (const wd of workingDirs) { dirs.add(expandHome(wd)); } @@ -235,7 +239,14 @@ export function getProjectScanDirs(ds?: DaemonSession): string[] { } // Fallback to global config const dirs = new Set(); - for (const wd of config.daemon.workingDirs) { + const configuredMultiDirs = parseWorkingDirList(config.daemon.workingDirs); + const configuredLegacyDirs = parseWorkingDirList(config.daemon.workingDir); + const workingDirs = configuredMultiDirs.length > 0 + ? configuredMultiDirs + : configuredLegacyDirs.length > 0 + ? configuredLegacyDirs + : ['~']; + for (const wd of workingDirs) { dirs.add(expandHome(wd)); } if (ds?.workingDir) { diff --git a/src/services/project-scanner.ts b/src/services/project-scanner.ts index 461fd7b64..eaadbd479 100644 --- a/src/services/project-scanner.ts +++ b/src/services/project-scanner.ts @@ -37,14 +37,22 @@ export interface ProjectScanOptions { } /** - * Describe a single directory as a project: its basename + current git ref, - * or null if the directory isn't a git repo/worktree. Used by `/repo ` - * to label an explicitly given path that may sit outside the scanned roots - * (the card's project list, by contrast, only covers the scan dirs). + * Describe a single directory as a project: the main worktree basename + + * current git ref, or null if the directory isn't a git repo/worktree. Using + * the main worktree name preserves the picker label for an explicitly selected + * linked worktree without recursively scanning the configured roots first. */ export function describeProjectDir(dir: string): { name: string; branch: string } | null { if (!isValidGitMarker(dir)) return null; - return { name: basename(dir), branch: getGitRef(dir) }; + const worktreeList = runGit('worktree list --porcelain', dir); + const mainWorktree = worktreeList + ?.split('\n') + .find(line => line.startsWith('worktree ')) + ?.slice('worktree '.length); + return { + name: mainWorktree ? basename(mainWorktree) : basename(dir), + branch: getGitRef(dir), + }; } /** `rev-parse --abbrev-ref HEAD` returns the literal string `HEAD` when diff --git a/test/command-handler.test.ts b/test/command-handler.test.ts index 4e03bc9a6..e1c543bba 100644 --- a/test/command-handler.test.ts +++ b/test/command-handler.test.ts @@ -484,7 +484,7 @@ import { existsSync, statSync, readFileSync, mkdirSync, mkdtempSync, rmSync, wri import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { codexHome } from '../src/services/codex-paths.js'; -import { scanMultipleProjects } from '../src/services/project-scanner.js'; +import { scanMultipleProjects, describeProjectDir } from '../src/services/project-scanner.js'; import { readGlobalConfig, repoPickerScanOptions } from '../src/global-config.js'; import { createRepoWorktree, pushWorktreeBranch } from '../src/services/git-worktree.js'; import { discoverAdoptableSessions } from '../src/core/session-discovery.js'; @@ -2025,14 +2025,13 @@ describe('handleCommand', () => { it('should resolve a first-level project name and switch repo (mid-session)', async () => { vi.mocked(existsSync).mockReturnValue(true); - vi.mocked(scanMultipleProjects).mockReturnValue([ - { name: 'payments', path: '/home/testuser/payments', branch: 'main' }, - ]); + vi.mocked(describeProjectDir).mockReturnValueOnce({ name: 'payments', branch: 'main' }); const ds = makeDaemonSession({ pendingRepo: false, repoCardMessageId: 'om_card' }); const deps = makeDeps(ds); await handleCommand('/repo', ROOT_ID, makeLarkMessage('/repo payments'), deps, LARK_APP_ID); + expect(scanMultipleProjects).not.toHaveBeenCalled(); expect(ds.workingDir).toBe('/home/testuser/payments'); expect(sessionStore.createSession).toHaveBeenCalledWith( CHAT_ID, ROOT_ID, 'payments (main)', 'group', undefined, diff --git a/test/repo-selection.test.ts b/test/repo-selection.test.ts index 5469d5bc2..2d4dae3e5 100644 --- a/test/repo-selection.test.ts +++ b/test/repo-selection.test.ts @@ -3,12 +3,13 @@ * which lets a user skip the Lark repo-selection card by naming a path * (absolute/relative) or a first-level project name under a scan dir. */ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { execSync } from 'node:child_process'; import { mkdtempSync, mkdirSync, rmSync, realpathSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; import { resolveRepoSelection } from '../src/core/command-handler.js'; +import { logger } from '../src/utils/logger.js'; function gitInit(dir: string, branch = 'main'): void { execSync(`git init -q -b ${branch} "${dir}"`, { stdio: 'pipe' }); @@ -44,6 +45,20 @@ describe('resolveRepoSelection', () => { expect(r!.displayName).toBe('botmux (main)'); }); + it('does not scan unrelated roots when a candidate directory exists directly', () => { + const direct = join(scanDir, 'tmp'); + mkdirSync(direct); + const scanLog = vi.spyOn(logger, 'info').mockImplementation(() => {}); + + try { + const r = resolveRepoSelection('tmp', [scanDir]); + expect(r).toEqual({ path: direct, displayName: 'tmp' }); + expect(scanLog).not.toHaveBeenCalledWith(expect.stringContaining('Scanned ')); + } finally { + scanLog.mockRestore(); + } + }); + it('resolves an absolute path to an existing git repo', () => { const repo = join(scanDir, 'proj'); mkdirSync(repo); @@ -89,6 +104,17 @@ describe('resolveRepoSelection', () => { expect(r!.displayName).toBe('app (main)'); }); + it('still scans for a nested project matched by basename', () => { + const repo = join(scanDir, 'teams', 'platform', 'deep-app'); + mkdirSync(repo, { recursive: true }); + gitInit(repo, 'main'); + + const r = resolveRepoSelection('deep-app', [scanDir]); + expect(r).not.toBeNull(); + expect(realpathSync(r!.path)).toBe(repo); + expect(r!.displayName).toBe('deep-app (main)'); + }); + it('falls back to a plain (non-git) directory with a basename label', () => { const plain = join(scanDir, 'plaindir'); mkdirSync(plain); diff --git a/test/session-manager-scan.test.ts b/test/session-manager-scan.test.ts index b9f556cc6..5b48ffa6e 100644 --- a/test/session-manager-scan.test.ts +++ b/test/session-manager-scan.test.ts @@ -13,6 +13,10 @@ const mockGetBot = vi.fn(); vi.mock('../src/bot-registry.js', () => ({ getBot: (id: string) => mockGetBot(id), getAllBots: () => [], + effectiveDefaultWorkingDir: (cfg: any) => + cfg?.defaultWorkingDir + || (cfg?.defaultOncall?.enabled ? cfg.defaultOncall.workingDir : undefined) + || undefined, })); vi.mock('../src/config.js', () => ({ @@ -43,6 +47,39 @@ describe('getProjectScanDir (single)', () => { }); describe('getProjectScanDirs (multi)', () => { + it('uses defaultWorkingDir instead of implicitly scanning HOME', () => { + mockGetBot.mockReturnValue({ config: { defaultWorkingDir: '~/Code' } }); + expect(getProjectScanDirs({ larkAppId: 'a1' } as any)).toEqual([`${HOME}/Code`]); + }); + + it('keeps workingDirs → workingDir → effective default priority', () => { + mockGetBot.mockReturnValue({ + config: { + workingDirs: ['/repos/one', '/repos/two'], + workingDir: '/legacy/root', + defaultWorkingDir: '/default/root', + }, + }); + expect(getProjectScanDirs({ larkAppId: 'a1' } as any)).toEqual(['/repos/one', '/repos/two']); + + mockGetBot.mockReturnValue({ + config: { + workingDir: '/legacy/root', + defaultWorkingDir: '/default/root', + }, + }); + expect(getProjectScanDirs({ larkAppId: 'a1' } as any)).toEqual(['/legacy/root']); + }); + + it('uses the enabled defaultOncall dir as the effective default', () => { + mockGetBot.mockReturnValue({ + config: { + defaultOncall: { enabled: true, workingDir: '/oncall/root' }, + }, + }); + expect(getProjectScanDirs({ larkAppId: 'a1' } as any)).toEqual(['/oncall/root']); + }); + it('scans each configured workingDir from itself, not the parent', () => { mockGetBot.mockReturnValue({ config: { workingDir: '/repos/foo' } }); expect(getProjectScanDirs({ larkAppId: 'a1' } as any)).toEqual(['/repos/foo']); @@ -65,6 +102,14 @@ describe('getProjectScanDirs (multi)', () => { expect(dirs).not.toContain('/repos'); // never the parent }); + it('deduplicates the session workingDir after home expansion', () => { + mockGetBot.mockReturnValue({ config: { defaultWorkingDir: '~/Code' } }); + expect(getProjectScanDirs({ + larkAppId: 'a1', + workingDir: `${HOME}/Code`, + } as any)).toEqual([`${HOME}/Code`]); + }); + it('falls back to global config workingDirs rooted at themselves (no bot)', () => { expect(getProjectScanDirs(undefined)).toEqual(['/global/repos']); });