Skip to content

Commit d1caefc

Browse files
Kewtonclaude
andauthored
fix(sync): リポジトリ消滅時に stale worktree 行を剪定する (#1349) (#1390)
リポジトリのディレクトリ削除・非git化で scanWorktrees() が git exit 128 に より [] を返すと、そのリポジトリはスキャン結果から消え、syncWorktreesToDB() の per-repo 剪定にも到達しない。結果として worktree 行が DB に残置し、サイド バーに幽霊行として残る。syncWorktreesToDB() のグローバル early-return は「全 リポジトリが空のとき」しか効かないため、健全なリポジトリに混じった単一の消滅 リポジトリは永久に回収されなかった。 対応: - worktrees.ts に pruneStaleRepositoryWorktrees(db, liveWorktrees) を追加。 DB に worktree 行を持つ全 repository_path を getRepositories() で列挙し、 当該スキャンで worktree を産まなかったもののうち、ディレクトリ(または .git)が実際に消えている場合のみ行を CASCADE 削除する。 - repositoryExistsOnDisk() を追加。dir と .git の双方が存在するときのみ 「実在」と判定し、それ以外(EACCES 等の不確定な読み取りも含む)は剪定 しない側に倒す。ディレクトリが実在するのに git がエラーを返しただけの ケース(ロック中・ネットワークドライブの一時不可視等)で幽霊行と誤判定 して履歴を破壊しないため。 - 呼び出しは全リポジトリを対象とする POST /api/repositories/sync のみ。 設計判断(Issue 本文との差異): Issue は syncWorktreesToDB() 自体の改修を 想定していたが、同関数は scan/restore 経路(scanWorktrees の単一リポジトリ 結果を渡す)からも呼ばれる。ここに全 DB リポジトリを走査する剪定を組み込むと、 単一リポジトリのスキャンが対象外リポジトリの行まで剪定してしまい、「同一 リポジトリ内のみ削除」という既存不変条件(および回帰テスト)を破る。そのため 剪定は独立関数として切り出し、全体を掌握する全体スキャン経路のみが呼ぶ構成に した。行キーは #1347 の正規化前提を踏襲し、getRepositories() が返す path.resolve() 済みの repository_path を DB・ファイルシステム双方で一貫使用。 回帰テスト: pruneStaleRepositoryWorktrees / repositoryExistsOnDisk に対し、 実 temp ディレクトリ+実 SQLite(FK ON)で、ディレクトリ削除→剪定・非git化 →剪定・実在時は空スキャンでも非剪定・消滅リポジトリのみ剪定して健全リポジトリ は不変・スキャンに現れたリポジトリはFS状態に依らず生存、を検証。 品質ゲート: lint / tsc / build / CI=true npm run test:unit いずれも exit 0 (unit 10079 passed)。 Claude-Session: https://claude.ai/code/session_013uPu2nRJSA3fWZhYEYYYEY Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 707dcfa commit d1caefc

4 files changed

Lines changed: 270 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111

12+
- fix(sync): **リポジトリ消滅時に stale worktree 行を剪定する** (#1349)。リポジトリのディレクトリ削除・非git化で `scanWorktrees` が git exit 128 により `[]` を返すと、そのリポジトリはスキャン結果から消え、`syncWorktreesToDB` の per-repo 剪定にも到達しないため worktree 行が DB に残置し、サイドバーに幽霊行として残っていた。`syncWorktreesToDB` のグローバル early-return は「**全**リポジトリが空のとき」しか効かず、健全なリポジトリに混じった単一の消滅リポジトリは永久に回収されなかった。全体スキャンの照合ステップとして `pruneStaleRepositoryWorktrees(db, liveWorktrees)` を追加。DB に worktree 行を持つ全 `repository_path` を列挙し、当該スキャンで worktree を産まなかったもののうち**ディレクトリ(または `.git`)が実際に消えている**場合のみ行を CASCADE 削除する。ディレクトリが実在するのに git がエラーを返しただけのケース(ロック中・ネットワークドライブの一時不可視等)は剪定しない側に倒し、一過性の失敗が履歴を破壊しないようにした(`repositoryExistsOnDisk`)。呼び出しは全リポジトリを対象とする `POST /api/repositories/sync` のみ。単一リポジトリを扱う scan/restore 経路は対象外リポジトリへの権限を持たないため実行しない。行キーは #1347 の正規化前提を踏襲し、`getRepositories` が返す `path.resolve()` 済みの `repository_path` を DB・ファイルシステム双方で一貫して用いる
1213
- fix(cli): **API レスポンスをシェイプ検証し版照会で版ずれを警告する** (#1357)。CLI の `ApiClient.get<T>/post<T>/patch<T>` は受信 JSON を `as T` で無検証キャストしており、古いデーモンが返す形の違うレスポンスを黙って受け入れていた。特に `fetchAgentInstances()` の `worktree.agentInstances ?? []` は、roster API を持たない旧デーモンでフィールド欠落を**無音の空配列**に丸め、ユーザーには「インスタンスが無い」ように見えてしまっていた。対策として `assertResponseShape()`(必須フィールドの存在チェック)を追加し、`fetchAgentInstances()` は `agentInstances` の**欠落**(=旧デーモン)を「サーバーが古い可能性」を示す `ApiError` として明示化する(現行デーモンは常に同フィールドを含め、空配列は「インスタンス無し」として正しく通す)。あわせて #1359 で信頼できる実行時版になった `GET /api/app/update-check` の `currentVersion` を読む `fetchDaemonVersion()` と、CLI 自身の版(`readPackageVersion()`)と食い違えば stderr に警告する `warnIfVersionSkew()` を追加(advisory・非 throw、版不明時は沈黙)。変更は `src/cli/utils/api-client.ts` と `src/cli/utils/agent-instances.ts` のみ。
1314
- fix(cli): **quickstart で稼働デーモンと CLI の版を照合し警告する** (#1337)。`npx commandmate@latest` を叩いてもデーモンが稼働中なら `quickstart.ts` の `ensureServerRunning` は再起動せず既存 URL を返すだけで、公開済みの修正が黙って利用者に届かなかった。しかも `npx …@latest --version` は取得したての CLI 版を表示するため、利用者は「最新で動いている」と誤認する。#1354 でデーモンが状態ファイルに記録するようになった版(`getStatus().version`)と、`readPackageVersion()` が返す CLI 版を照合し、食い違う場合に**稼働中サーバーは更新されない旨と `commandmate update` への導線を警告表示**する(`status` コマンドの版照合と同じ仕組み)。自動再起動は稼働中セッションを巻き込むため行わず、既定は警告に留める。版を記録しない旧デーモンや CLI 版が解決できない場合は従来どおり無警告で URL を返す。担当は `quickstart.ts` のみ(`api-client.ts` は触らない)
1415
- fix(api): **UNIQUE 制約違反を事前検証し 409 で返す(schedules 同名 / memos position)** (#1351)。2つの POST 経路が DB の UNIQUE 制約を事前チェックせず生 INSERT していたため、衝突が**原因不明の 500** として表面化していた。① `POST /api/worktrees/:id/schedules` は `scheduled_executions` の `UNIQUE(worktree_id, name)` を検査せず、同名スケジュールの作成が catch の一律 500 に落ちていた。② `POST /api/worktrees/:id/memos` は position を**明示指定した分岐**だけ空きチェックを通っておらず(自動採番分岐のみ `usedPositions` を見ていた)、`worktree_memos` の `UNIQUE(worktree_id, position)` 違反がやはり 500 になっていた(`createMemo` は素の INSERT)。両経路に INSERT 前の重複検証を追加し、schedules は `409 DUPLICATE_NAME`、memos は `409 DUPLICATE_POSITION` を明示的に返す。あわせて INSERT 側でも UNIQUE 違反を捕捉してエラーコードに変換する多重防御を入れた(`createMemo` は `src/lib/external-apps/db.ts` の `ExternalAppDbError('DUPLICATE')` に倣い `MemoDbError('DUPLICATE_POSITION')` を throw、schedules 経路は catch 内で `UNIQUE constraint` を検出)。事前チェックと INSERT の間の競合(同名/同 position を掴んだ並行リクエスト)でも 500 に戻らない。制約はいずれも worktree スコープのため、別 worktree での同名・同 position は従来どおり許可される。FOREIGN KEY 等その他の DB エラーは従来どおり素通しで 500 のまま

src/app/api/repositories/sync/route.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import { NextResponse } from 'next/server';
99
import { getDbInstance } from '@/lib/db/db-instance';
10-
import { getRepositoryPaths, scanMultipleRepositories } from '@/lib/git/worktrees';
10+
import { getRepositoryPaths, scanMultipleRepositories, pruneStaleRepositoryWorktrees } from '@/lib/git/worktrees';
1111
import { registerAndFilterRepositories, getAllRepositories } from '@/lib/db/db-repository';
1212
import { syncWorktreesAndCleanup } from '@/lib/session-cleanup';
1313
import { createLogger } from '@/lib/logger';
@@ -48,6 +48,13 @@ export async function POST() {
4848
// Issue #526: Sync to database and clean up sessions for deleted worktrees
4949
const { syncResult, cleanupWarnings } = await syncWorktreesAndCleanup(db, allWorktrees);
5050

51+
// Issue #1349: a deleted / de-gitified repository scans to [] (git exit 128),
52+
// so its worktree rows never reach syncWorktreesToDB's per-repo prune and
53+
// linger as ghost rows in the sidebar. Reconcile them here — on the global
54+
// sync, which is the only caller with authority over every repository —
55+
// deleting rows only for repositories whose directory is genuinely gone.
56+
const staleDeletedIds = pruneStaleRepositoryWorktrees(db, allWorktrees);
57+
5158
// Get unique repository count
5259
const uniqueRepos = new Set(allWorktrees.map(wt => wt.repositoryPath));
5360

@@ -58,7 +65,7 @@ export async function POST() {
5865
worktreeCount: allWorktrees.length,
5966
repositoryCount: uniqueRepos.size,
6067
repositories: Array.from(uniqueRepos),
61-
deletedCount: syncResult.deletedIds.length,
68+
deletedCount: syncResult.deletedIds.length + staleDeletedIds.length,
6269
cleanupWarnings,
6370
},
6471
{ status: 200 }

src/lib/git/worktrees.ts

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,16 @@
55

66
import { exec } from 'child_process';
77
import { promisify } from 'util';
8+
import fs from 'fs';
89
import path from 'path';
910
import type { Worktree } from '@/types/models';
1011
import type Database from 'better-sqlite3';
11-
import { upsertWorktree, getWorktreesByRepository, deleteWorktreesByIds } from '@/lib/db';
12+
import {
13+
upsertWorktree,
14+
getWorktreesByRepository,
15+
deleteWorktreesByIds,
16+
getRepositories,
17+
} from '@/lib/db';
1218
import { createLogger } from '@/lib/logger';
1319

1420
const logger = createLogger('worktrees');
@@ -347,3 +353,96 @@ export function syncWorktreesToDB(
347353

348354
return { deletedIds: allDeletedIds, upsertedCount };
349355
}
356+
357+
/**
358+
* Whether a repository still exists on disk as a git working tree.
359+
*
360+
* Returns `false` only when the directory itself is gone OR its `.git` entry is
361+
* missing (a de-gitified directory). Every other state — including a present
362+
* directory whose `.git` is intact — is reported as "still there", so a
363+
* transient git failure (a locked index, a momentarily-unavailable network
364+
* drive whose mount point is still present, …) never satisfies the prune
365+
* condition. Issue #1349: fall on the side of NOT pruning whenever the on-disk
366+
* state is inconclusive.
367+
*
368+
* @param repoPath - Canonical repository path (as stored in
369+
* `worktrees.repository_path`, i.e. already `path.resolve()`d by
370+
* {@link scanWorktrees}; see #1347)
371+
* @returns `true` if the repository still looks present on disk
372+
*/
373+
export function repositoryExistsOnDisk(repoPath: string): boolean {
374+
if (!repoPath) return false;
375+
try {
376+
return fs.existsSync(repoPath) && fs.existsSync(path.join(repoPath, '.git'));
377+
} catch {
378+
// Inconclusive filesystem read (e.g. EACCES): be conservative and keep rows.
379+
return true;
380+
}
381+
}
382+
383+
/**
384+
* Prune DB worktree rows for repositories that have vanished from disk.
385+
*
386+
* Issue #1349: when a repository directory is deleted or de-gitified,
387+
* {@link scanWorktrees} returns `[]` (git exit 128), so the repository silently
388+
* drops out of the scan and {@link syncWorktreesToDB} never reaches its per-repo
389+
* prune for it — the rows linger as ghost entries in the sidebar. The global
390+
* early-return in `syncWorktreesToDB` only fires when *every* repository is
391+
* empty, so a single vanished repository among healthy ones is never cleaned up.
392+
*
393+
* This runs as the global-sync reconciliation step: it enumerates every
394+
* `repository_path` that still owns worktree rows and, for those that produced
395+
* no worktrees in the current scan, deletes the rows **only when the repository
396+
* directory (or its `.git`) is genuinely gone** (see
397+
* {@link repositoryExistsOnDisk}). A repository that still exists but merely
398+
* errored in git is left untouched, so a transient failure — a network drive
399+
* blip, a locked repo — never destroys history.
400+
*
401+
* Intended for the full-scan endpoint (`POST /api/repositories/sync`) only.
402+
* Single-repository callers (scan/restore) must NOT run this: they hold no
403+
* authority over repositories outside their own scan, and running it there would
404+
* prune repositories that merely weren't part of that request.
405+
*
406+
* Row keys stay consistent with #1347: {@link getRepositories} returns the
407+
* stored (already `path.resolve()`d) `repository_path`, and every subsequent DB
408+
* and filesystem lookup uses that same canonical value.
409+
*
410+
* @param db - Database instance
411+
* @param liveWorktrees - Worktrees produced by the current scan; their
412+
* repositories are known-present and therefore skipped
413+
* @returns IDs of worktrees deleted for vanished repositories
414+
*/
415+
export function pruneStaleRepositoryWorktrees(
416+
db: Database.Database,
417+
liveWorktrees: Worktree[]
418+
): string[] {
419+
// Repositories that produced at least one worktree this scan are alive by
420+
// definition (git listed them), so they are never candidates for pruning.
421+
const liveRepoPaths = new Set(
422+
liveWorktrees
423+
.map(wt => wt.repositoryPath)
424+
.filter((p): p is string => !!p)
425+
);
426+
427+
const deletedIds: string[] = [];
428+
429+
for (const repo of getRepositories(db)) {
430+
const repoPath = repo.path;
431+
if (!repoPath) continue;
432+
if (liveRepoPaths.has(repoPath)) continue;
433+
// Conservative guard: keep rows unless the directory/.git is truly gone.
434+
if (repositoryExistsOnDisk(repoPath)) continue;
435+
436+
const ids = getWorktreesByRepository(db, repoPath).map(row => row.id);
437+
if (ids.length === 0) continue;
438+
439+
const result = deleteWorktreesByIds(db, ids);
440+
deletedIds.push(...ids);
441+
logger.info('worktree:stale-repo-pruned', {
442+
repoPath,
443+
deletedCount: result.deletedCount,
444+
});
445+
}
446+
447+
return deletedIds;
448+
}

tests/unit/lib/worktrees-sync.test.ts

Lines changed: 160 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
*/
1616

1717
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
18+
import fs from 'fs';
19+
import os from 'os';
20+
import path from 'path';
1821
import Database from 'better-sqlite3';
1922
import {
2023
upsertWorktree,
@@ -23,7 +26,11 @@ import {
2326
migrateWorktreeIdPreservingChildren,
2427
} from '@/lib/db';
2528
import { runMigrations } from '@/lib/db/db-migrations';
26-
import { syncWorktreesToDB } from '@/lib/git/worktrees';
29+
import {
30+
syncWorktreesToDB,
31+
pruneStaleRepositoryWorktrees,
32+
repositoryExistsOnDisk,
33+
} from '@/lib/git/worktrees';
2734
import type { Worktree } from '@/types/models';
2835

2936
const REPO_PATH = '/repos/anvil';
@@ -313,3 +320,155 @@ describe('Issue #1151: branch-switch history preservation', () => {
313320
});
314321
});
315322
});
323+
324+
/**
325+
* Issue #1349: a repository whose directory is deleted or de-gitified makes
326+
* `scanWorktrees` return `[]` (git exit 128). The repository then never appears
327+
* in the scan handed to `syncWorktreesToDB`, so its worktree rows are never
328+
* pruned and survive forever as ghost rows in the sidebar. `syncWorktreesToDB`'s
329+
* global early-return only fires when *every* repository is empty, so a single
330+
* vanished repository among healthy ones is never reconciled.
331+
*
332+
* `pruneStaleRepositoryWorktrees` is the global-sync reconciliation step. These
333+
* tests exercise it against a real in-memory SQLite database plus real temp
334+
* directories, asserting that:
335+
* - a vanished directory (or a de-gitified one) is pruned (CASCADE),
336+
* - a directory that still exists is NEVER pruned even when the scan is empty
337+
* (transient git error / network-drive blip → keep, do not destroy history),
338+
* - a repository present in the current scan is treated as alive regardless of
339+
* the filesystem, and other repositories are left untouched.
340+
*/
341+
describe('Issue #1349: pruneStaleRepositoryWorktrees', () => {
342+
let db: Database.Database;
343+
const tempDirs: string[] = [];
344+
345+
beforeEach(() => {
346+
db = new Database(':memory:');
347+
runMigrations(db);
348+
// Mirror production (src/lib/db/db-instance.ts): enforce FK so CASCADE fires.
349+
db.pragma('foreign_keys = ON');
350+
});
351+
352+
afterEach(() => {
353+
db.close();
354+
for (const dir of tempDirs.splice(0)) {
355+
fs.rmSync(dir, { recursive: true, force: true });
356+
}
357+
});
358+
359+
/** Create a real temp repo directory (optionally with a `.git` entry). */
360+
function makeRepoDir(withGit = true): string {
361+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cm-1349-'));
362+
if (withGit) fs.mkdirSync(path.join(dir, '.git'));
363+
tempDirs.push(dir);
364+
return dir;
365+
}
366+
367+
function repoWorktree(repoPath: string, id: string, branch: string): Worktree {
368+
return {
369+
id,
370+
name: branch,
371+
branch,
372+
path: repoPath,
373+
repositoryPath: repoPath,
374+
repositoryName: path.basename(repoPath),
375+
};
376+
}
377+
378+
describe('repositoryExistsOnDisk', () => {
379+
it('is true only when both the directory and its .git exist', () => {
380+
const withGit = makeRepoDir(true);
381+
const noGit = makeRepoDir(false);
382+
383+
expect(repositoryExistsOnDisk(withGit)).toBe(true);
384+
expect(repositoryExistsOnDisk(noGit)).toBe(false); // de-gitified
385+
expect(repositoryExistsOnDisk('/definitely/not/here/cm-1349')).toBe(false);
386+
expect(repositoryExistsOnDisk('')).toBe(false);
387+
});
388+
});
389+
390+
it('prunes worktree rows (CASCADE) when the repository directory is deleted', () => {
391+
const repo = makeRepoDir();
392+
syncWorktreesToDB(db, [repoWorktree(repo, 'r-main', 'main')]);
393+
seedChildData(db, 'r-main');
394+
expect(getWorktreeById(db, 'r-main')).not.toBeNull();
395+
expect(totalChildRows(db)).toBe(CHILD_TABLES.length);
396+
397+
// Directory deleted → next global sync scans it to [] (absent from live set).
398+
fs.rmSync(repo, { recursive: true, force: true });
399+
const deleted = pruneStaleRepositoryWorktrees(db, []);
400+
401+
expect(deleted).toContain('r-main');
402+
expect(getWorktreeById(db, 'r-main')).toBeNull();
403+
// Child data is CASCADE-deleted with the row.
404+
expect(countChildData(db, 'r-main').chat_messages).toBe(0);
405+
expect(totalChildRows(db)).toBe(0);
406+
});
407+
408+
it('prunes when the directory exists but .git was removed (de-gitified)', () => {
409+
const repo = makeRepoDir();
410+
syncWorktreesToDB(db, [repoWorktree(repo, 'r-main', 'main')]);
411+
412+
fs.rmSync(path.join(repo, '.git'), { recursive: true, force: true });
413+
const deleted = pruneStaleRepositoryWorktrees(db, []);
414+
415+
expect(deleted).toContain('r-main');
416+
expect(getWorktreeById(db, 'r-main')).toBeNull();
417+
});
418+
419+
it('does NOT prune when the directory + .git still exist but the scan is empty', () => {
420+
// Conservative guard: a present repository that merely errored in git (or a
421+
// transiently-invisible network drive whose mount point is still present)
422+
// must never be pruned.
423+
const repo = makeRepoDir();
424+
syncWorktreesToDB(db, [repoWorktree(repo, 'r-main', 'main')]);
425+
seedChildData(db, 'r-main');
426+
427+
const deleted = pruneStaleRepositoryWorktrees(db, []);
428+
429+
expect(deleted).toEqual([]);
430+
expect(getWorktreeById(db, 'r-main')).not.toBeNull();
431+
expect(totalChildRows(db)).toBe(CHILD_TABLES.length);
432+
});
433+
434+
it('prunes only the vanished repository, leaving live repositories untouched', () => {
435+
const gone = makeRepoDir();
436+
const alive = makeRepoDir();
437+
syncWorktreesToDB(db, [
438+
repoWorktree(gone, 'gone-main', 'main'),
439+
repoWorktree(alive, 'alive-main', 'main'),
440+
]);
441+
seedChildData(db, 'gone-main');
442+
seedChildData(db, 'alive-main');
443+
444+
fs.rmSync(gone, { recursive: true, force: true });
445+
// Current scan reports only the alive repository.
446+
const deleted = pruneStaleRepositoryWorktrees(db, [
447+
repoWorktree(alive, 'alive-main', 'main'),
448+
]);
449+
450+
expect(deleted).toEqual(['gone-main']);
451+
expect(getWorktreeById(db, 'gone-main')).toBeNull();
452+
expect(getWorktreeById(db, 'alive-main')).not.toBeNull();
453+
expect(countChildData(db, 'alive-main').chat_messages).toBe(1);
454+
});
455+
456+
it('treats a repository present in the scan as alive even if its dir check would fail', () => {
457+
// A repo that produced worktrees was listed by git and is alive by
458+
// definition; the scan result wins over a racing filesystem read.
459+
const repo = makeRepoDir();
460+
syncWorktreesToDB(db, [repoWorktree(repo, 'r-main', 'main')]);
461+
462+
fs.rmSync(repo, { recursive: true, force: true });
463+
const deleted = pruneStaleRepositoryWorktrees(db, [
464+
repoWorktree(repo, 'r-main', 'main'),
465+
]);
466+
467+
expect(deleted).toEqual([]);
468+
expect(getWorktreeById(db, 'r-main')).not.toBeNull();
469+
});
470+
471+
it('returns an empty array when there is nothing to prune', () => {
472+
expect(pruneStaleRepositoryWorktrees(db, [])).toEqual([]);
473+
});
474+
});

0 commit comments

Comments
 (0)