mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-03 02:21:44 +00:00
fix(storage): never share a graph from a checkout that hides files (#3374)
Trigger: a sparse checkout, a skip-worktree/assume-unchanged entry, or an uninitialized submodule leaves `git status` clean, and the run's indexCoverage.dirtyPaths drops paths with no file hash, so publishSharedGraph published a graph missing those files as the commit graph. seedSharedSlot then copied the seed's lastCommit into the new pointer slot, so the next analyze of a sibling hit the up-to-date fast path without hashing. Fix: add isWorkingTreePristine (storage/git.ts): the unfiltered listWorkingTreeDirtyPaths must be empty (null fails closed) and every gitlink in the index must have a checked-out `.git`. publishSharedGraph requires it instead of isWorkingTreeDirty. seedSharedSlot keeps the seed's lastCommit only when the seed is at HEAD and the checkout is pristine, so a clean sibling still fast-paths onto the shared graph; any other seeded pointer gets an empty lastCommit, like seedFromLocalIndex, and its next run hash-diffs, then re-points at the commit graph on publish. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
416af2a8f6
commit
42c39b5c82
4 changed files with 219 additions and 6 deletions
|
|
@ -20,7 +20,7 @@ import { existsSync, constants as fsConstants } from 'fs';
|
|||
import fs from 'fs/promises';
|
||||
import path from 'path';
|
||||
import { acquireIndexLock, requireExclusiveIndexLock } from '../storage/index-lock.js';
|
||||
import { commitDistanceToHead, getRemoteUrl, isWorkingTreeDirty } from '../storage/git.js';
|
||||
import { commitDistanceToHead, getRemoteUrl, isWorkingTreePristine } from '../storage/git.js';
|
||||
import {
|
||||
canonicalizePath,
|
||||
findRegistryEntryByRepoPath,
|
||||
|
|
@ -168,7 +168,10 @@ const listCommitGraphs = async (layout: SharedStoreLayout): Promise<CommitGraph[
|
|||
* ponytail: one `git` call pair per commit graph; fine for tens of graphs,
|
||||
* batch through `git rev-list` if stores grow to hundreds.
|
||||
*/
|
||||
const pickSeed = (repoPath: string, graphs: CommitGraph[]): CommitGraph | null => {
|
||||
const pickSeed = (
|
||||
repoPath: string,
|
||||
graphs: CommitGraph[],
|
||||
): { graph: CommitGraph; distance: number } | null => {
|
||||
let best: { graph: CommitGraph; distance: number } | null = null;
|
||||
for (const graph of graphs) {
|
||||
const distance = commitDistanceToHead(repoPath, graph.commit);
|
||||
|
|
@ -179,7 +182,7 @@ const pickSeed = (repoPath: string, graphs: CommitGraph[]): CommitGraph | null =
|
|||
(distance === best.distance && graph.meta.indexedAt > best.graph.meta.indexedAt);
|
||||
if (better) best = { graph, distance };
|
||||
}
|
||||
return best?.graph ?? null;
|
||||
return best;
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
@ -251,7 +254,12 @@ export const seedSharedSlot = async (
|
|||
log: Log,
|
||||
): Promise<void> => {
|
||||
if (await loadMeta(layout.checkoutSlot)) return;
|
||||
const seed = pickSeed(repoPath, await listCommitGraphs(layout));
|
||||
const picked = pickSeed(repoPath, await listCommitGraphs(layout));
|
||||
const seed = picked?.graph;
|
||||
// A commit graph matches its commit exactly, so a checkout showing exactly
|
||||
// that commit is up to date. Any other checkout gets an empty lastCommit,
|
||||
// like seedFromLocalIndex, so its next run hash-diffs.
|
||||
const upToDate = picked?.distance === 0 && isWorkingTreePristine(repoPath);
|
||||
// Record the pointer under the publish lock, where reclaim counts
|
||||
// references, so the graph cannot be deleted between the pick and the save.
|
||||
const pointed =
|
||||
|
|
@ -265,6 +273,7 @@ export const seedSharedSlot = async (
|
|||
repoPath,
|
||||
storagePath: layout.checkoutSlot,
|
||||
graphPath: graph,
|
||||
lastCommit: upToDate ? seed.meta.lastCommit : '',
|
||||
};
|
||||
delete meta.incrementalInProgress;
|
||||
await saveMeta(layout.checkoutSlot, meta);
|
||||
|
|
@ -361,7 +370,8 @@ export const publishSharedGraph = async (
|
|||
meta.lastCommit === currentCommit &&
|
||||
!meta.incrementalInProgress &&
|
||||
builtClean &&
|
||||
!isWorkingTreeDirty(repoPath);
|
||||
// A sparse or partial checkout builds a graph missing the files it hides.
|
||||
isWorkingTreePristine(repoPath);
|
||||
// Every pointer change and the reclaim that follows run under one publish
|
||||
// lock, so a concurrent reclaim never sees a half-recorded reference.
|
||||
await withStoreLock(layout, 'publish', async () => {
|
||||
|
|
|
|||
|
|
@ -121,6 +121,33 @@ export const listWorkingTreeDirtyPaths = (repoPath: string): string[] | null =>
|
|||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* True when the working tree shows exactly the committed tree: nothing dirty
|
||||
* or untracked, no path hidden by skip-worktree or assume-unchanged (which is
|
||||
* how a sparse checkout leaves files out), and every gitlink checked out as a
|
||||
* submodule. `git status` stays clean in all three hidden cases. False on any
|
||||
* git failure.
|
||||
*/
|
||||
export const isWorkingTreePristine = (repoPath: string): boolean => {
|
||||
if (listWorkingTreeDirtyPaths(repoPath)?.length !== 0) return false;
|
||||
try {
|
||||
const out = execFileSync('git', ['ls-files', '--stage', '-z', '--'], {
|
||||
cwd: repoPath,
|
||||
windowsHide: true,
|
||||
...gitPathListExec,
|
||||
});
|
||||
for (const record of out.split('\0')) {
|
||||
// `<mode> <object> <stage>\t<path>`; mode 160000 is a gitlink.
|
||||
if (!record.startsWith('160000 ')) continue;
|
||||
const rel = record.slice(record.indexOf('\t') + 1);
|
||||
if (!existsSync(path.join(repoPath, rel, '.git'))) return false;
|
||||
}
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Snapshot, per candidate file, whether it is safe for `selfCommitContextFiles`
|
||||
* to auto-commit — call this BEFORE `analyze` writes AGENTS.md/CLAUDE.md.
|
||||
|
|
|
|||
|
|
@ -3,7 +3,12 @@ import { existsSync } from 'fs';
|
|||
import fs from 'fs/promises';
|
||||
import path from 'path';
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { featureKeyOf, publishSharedGraph } from '../../src/core/shared-store-analyze.js';
|
||||
import {
|
||||
ensurePrivateSharedGraph,
|
||||
featureKeyOf,
|
||||
publishSharedGraph,
|
||||
seedSharedSlot,
|
||||
} from '../../src/core/shared-store-analyze.js';
|
||||
import {
|
||||
getStoragePaths,
|
||||
listRegisteredRepos,
|
||||
|
|
@ -333,4 +338,56 @@ describe('publishSharedGraph race (#3352)', () => {
|
|||
expect(await listCommitDirs(layoutOf(main))).toEqual([]);
|
||||
expect(existsSync(path.join(layoutOf(main).checkoutSlot, 'lbug'))).toBe(true);
|
||||
});
|
||||
|
||||
// #3374: `git status` is clean in a sparse checkout, but the graph lacks the
|
||||
// files the checkout leaves out.
|
||||
it('keeps a checkout that hides committed files private', async () => {
|
||||
const { checkouts, head } = await setup();
|
||||
const [main] = checkouts;
|
||||
git(main, 'update-index', '--skip-worktree', '--', 'a.ts');
|
||||
await fs.rm(path.join(main, 'a.ts'));
|
||||
await publishSharedGraph(layoutOf(main), main, head, () => {});
|
||||
expect(await listCommitDirs(layoutOf(main))).toEqual([]);
|
||||
expect(existsSync(path.join(layoutOf(main).checkoutSlot, 'lbug'))).toBe(true);
|
||||
});
|
||||
|
||||
const seedFreshSlot = async (checkout: string): Promise<RepoMeta | null> => {
|
||||
const layout = layoutOf(checkout);
|
||||
await fs.rm(layout.checkoutSlot, { recursive: true, force: true });
|
||||
await seedSharedSlot(layout, checkout, () => {});
|
||||
return loadMeta(layout.checkoutSlot);
|
||||
};
|
||||
|
||||
it('seeds a pristine checkout at the graph commit as up to date', async () => {
|
||||
const { checkouts, head } = await setup();
|
||||
const [main, wt] = checkouts;
|
||||
await publishSharedGraph(layoutOf(main), main, head, () => {});
|
||||
const seeded = await seedFreshSlot(wt);
|
||||
expect(seeded?.graphPath).toBe((await loadMeta(layoutOf(main).checkoutSlot))?.graphPath);
|
||||
expect(seeded?.lastCommit).toBe(head);
|
||||
});
|
||||
|
||||
it('seeds a checkout that hides committed files without a commit, then re-points', async () => {
|
||||
const { checkouts, head } = await setup();
|
||||
const [main, wt] = checkouts;
|
||||
await publishSharedGraph(layoutOf(main), main, head, () => {});
|
||||
const shared = (await loadMeta(layoutOf(main).checkoutSlot))?.graphPath;
|
||||
git(wt, 'update-index', '--skip-worktree', '--', 'a.ts');
|
||||
const seeded = await seedFreshSlot(wt);
|
||||
expect(seeded?.graphPath).toBe(shared);
|
||||
// An empty lastCommit sends the next analyze through the file-hash diff.
|
||||
expect(seeded?.lastCommit).toBe('');
|
||||
|
||||
// That analyze copies the graph, finds nothing to change, and stamps HEAD;
|
||||
// once the checkout shows every file again, publish drops the copy.
|
||||
const slot = layoutOf(wt).checkoutSlot;
|
||||
expect(await ensurePrivateSharedGraph(slot, () => {})).toBe(true);
|
||||
const copied = await loadMeta(slot);
|
||||
expect(copied).not.toBeNull();
|
||||
await saveMeta(slot, { ...(copied as RepoMeta), lastCommit: head });
|
||||
git(wt, 'update-index', '--no-skip-worktree', '--', 'a.ts');
|
||||
await publishSharedGraph(layoutOf(wt), wt, head, () => {});
|
||||
expect((await loadMeta(slot))?.graphPath).toBe(shared);
|
||||
expect(existsSync(path.join(slot, 'lbug'))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1135,3 +1135,122 @@ describe('listWorkingTreeDirtyPaths', () => {
|
|||
},
|
||||
);
|
||||
});
|
||||
|
||||
// ─── isWorkingTreePristine ────────────────────────────────────────────────
|
||||
//
|
||||
// The shared-store publish gate (#3374): a graph built from a working tree
|
||||
// that hides committed content (sparse checkout, index bits, an uninitialized
|
||||
// submodule) must never become the commit graph other checkouts reuse.
|
||||
|
||||
/** A repo with one committed file, `a.ts`. */
|
||||
function makeCommittedRepo(): string {
|
||||
const repo = makeIsolatedGitRepo();
|
||||
fs.writeFileSync(path.join(repo, 'a.ts'), 'export const a = 1;');
|
||||
fs.mkdirSync(path.join(repo, 'lib'));
|
||||
fs.writeFileSync(path.join(repo, 'lib', 'b.ts'), 'export const b = 1;');
|
||||
execFileSync(gitExecutable, ['add', '-A'], { cwd: repo, stdio: 'ignore' });
|
||||
execFileSync(gitExecutable, ['commit', '-q', '-m', 'init'], { cwd: repo, stdio: 'ignore' });
|
||||
return repo;
|
||||
}
|
||||
|
||||
const gitIn = (repo: string, ...args: string[]): string =>
|
||||
execFileSync(gitExecutable, args, { cwd: repo, encoding: 'utf8' }).trim();
|
||||
|
||||
describe('isWorkingTreePristine', () => {
|
||||
it('is true for a clean checkout, ignoring GitNexus-managed writes', async () => {
|
||||
const { isWorkingTreePristine } = await import('../../src/storage/git.js');
|
||||
const repo = makeCommittedRepo();
|
||||
try {
|
||||
fs.mkdirSync(path.join(repo, '.gitnexus'));
|
||||
fs.writeFileSync(path.join(repo, '.gitnexus', 'meta.json'), '{}');
|
||||
fs.writeFileSync(path.join(repo, 'AGENTS.md'), 'x');
|
||||
|
||||
expect(isWorkingTreePristine(repo)).toBe(true);
|
||||
} finally {
|
||||
fs.rmSync(repo, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('is false with an untracked source file', async () => {
|
||||
const { isWorkingTreePristine } = await import('../../src/storage/git.js');
|
||||
const repo = makeCommittedRepo();
|
||||
try {
|
||||
fs.writeFileSync(path.join(repo, 'new.ts'), 'export const n = 1;');
|
||||
|
||||
expect(isWorkingTreePristine(repo)).toBe(false);
|
||||
} finally {
|
||||
fs.rmSync(repo, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it.each(['--assume-unchanged', '--skip-worktree'])(
|
||||
'is false when git update-index %s hides a path, even with unchanged content',
|
||||
async (flag) => {
|
||||
const { isWorkingTreePristine } = await import('../../src/storage/git.js');
|
||||
const repo = makeCommittedRepo();
|
||||
try {
|
||||
gitIn(repo, 'update-index', flag, '--', 'a.ts');
|
||||
|
||||
expect(isWorkingTreePristine(repo)).toBe(false);
|
||||
} finally {
|
||||
fs.rmSync(repo, { recursive: true, force: true });
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it('is false in a sparse checkout that leaves committed files out', async () => {
|
||||
const { isWorkingTreePristine } = await import('../../src/storage/git.js');
|
||||
const repo = makeCommittedRepo();
|
||||
try {
|
||||
gitIn(repo, 'sparse-checkout', 'set', '--no-cone', '/a.ts');
|
||||
|
||||
expect(fs.existsSync(path.join(repo, 'lib', 'b.ts'))).toBe(false);
|
||||
expect(isWorkingTreePristine(repo)).toBe(false);
|
||||
} finally {
|
||||
fs.rmSync(repo, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('is false with a registered but uninitialized submodule', async () => {
|
||||
const { isWorkingTreePristine } = await import('../../src/storage/git.js');
|
||||
const repo = makeCommittedRepo();
|
||||
try {
|
||||
const head = gitIn(repo, 'rev-parse', 'HEAD');
|
||||
gitIn(repo, 'update-index', '--add', '--cacheinfo', `160000,${head},vendor/dep`);
|
||||
gitIn(repo, 'commit', '-q', '-m', 'add gitlink');
|
||||
|
||||
expect(isWorkingTreePristine(repo)).toBe(false);
|
||||
} finally {
|
||||
fs.rmSync(repo, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('is true with a checked-out submodule', async () => {
|
||||
const { isWorkingTreePristine } = await import('../../src/storage/git.js');
|
||||
const repo = makeCommittedRepo();
|
||||
try {
|
||||
const sub = path.join(repo, 'vendor', 'dep');
|
||||
fs.mkdirSync(sub, { recursive: true });
|
||||
gitIn(sub, 'init', '-q');
|
||||
fs.writeFileSync(path.join(sub, 'c.ts'), 'export const c = 1;');
|
||||
gitIn(sub, 'add', '-A');
|
||||
gitIn(sub, '-c', 'user.name=t', '-c', 'user.email=t@t', 'commit', '-q', '-m', 'sub');
|
||||
gitIn(repo, 'add', 'vendor/dep');
|
||||
gitIn(repo, 'commit', '-q', '-m', 'add submodule');
|
||||
|
||||
expect(isWorkingTreePristine(repo)).toBe(true);
|
||||
} finally {
|
||||
fs.rmSync(repo, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('is false (fails closed) outside a git repository', async () => {
|
||||
const { isWorkingTreePristine } = await import('../../src/storage/git.js');
|
||||
const dir = makeIsolatedTempDir('gn-nongit-pristine-');
|
||||
try {
|
||||
expect(isWorkingTreePristine(dir)).toBe(false);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue