Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions packages/storage/src/__tests__/fixtures/git-repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,36 @@ import { promisify } from 'node:util';

const execFileAsync = promisify(execFile);

export const BROKEN_GIT_SHAPES = [
'head-directory',
'head-garbage',
'gitfile-garbage-head',
'missing-objects-and-refs',
] as const;

export type BrokenGitShape = (typeof BROKEN_GIT_SHAPES)[number];

/** Writes structurally broken Git metadata (a `.git` entry or its target) into root. */
export async function createBrokenGitMetadata(root: string, shape: BrokenGitShape): Promise<void> {
switch (shape) {
case 'head-directory':
await mkdir(join(root, '.git', 'HEAD'), { recursive: true });
return;
case 'head-garbage':
await mkdir(join(root, '.git'), { recursive: true });
await writeFile(join(root, '.git', 'HEAD'), 'gk\n', 'utf8');
return;
case 'gitfile-garbage-head':
await writeFile(join(root, '.git'), 'gitdir: stub\n', 'utf8');
await mkdir(join(root, 'stub'), { recursive: true });
await writeFile(join(root, 'stub', 'HEAD'), 'gk\n', 'utf8');
return;
case 'missing-objects-and-refs':
await mkdir(join(root, '.git'), { recursive: true });
await writeFile(join(root, '.git', 'HEAD'), `ref: refs/heads/${'a'.repeat(40)}\n`, 'utf8');
}
}

export async function createGitRepositoryWithWorktree(
repository: string,
linkedWorktree: string,
Expand Down
82 changes: 81 additions & 1 deletion packages/storage/src/__tests__/project-catalog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ import {
resolveProjectLocation,
} from '../project-catalog.js';
import { createSessionStore } from '../session-store.js';
import { createGitRepositoryWithWorktree } from './fixtures/git-repository.js';
import {
BROKEN_GIT_SHAPES,
createBrokenGitMetadata,
createGitRepositoryWithWorktree,
} from './fixtures/git-repository.js';

const execFileAsync = promisify(execFile);
const trackedCatalogs = new Map<ProjectCatalog, string>();
Expand Down Expand Up @@ -99,6 +103,82 @@ test('a plain folder resolves without requiring the Git executable', async () =>
}
});

test('an incomplete enclosing .git directory does not turn a nested folder into a repository', async () => {
const base = await mkdtemp(join(tmpdir(), 'maka-project-folder-invalid-git-'));
try {
const folder = join(base, 'folder');
await mkdir(join(base, '.git', 'gk'), { recursive: true });
await mkdir(folder);

assert.deepEqual(await resolveProjectLocationWithoutGit(folder), {
canonicalPath: await realpath(folder),
identity: `folder:${await realpath(folder)}`,
kind: 'folder',
});
} finally {
await rm(base, { recursive: true, force: true });
}
});

test('broken ancestor Git metadata does not turn a nested folder into a repository', async () => {
const base = await mkdtemp(join(tmpdir(), 'maka-project-invalid-ancestor-'));
try {
for (const shape of BROKEN_GIT_SHAPES) {
const root = join(base, shape);
const folder = join(root, 'folder');
await mkdir(folder, { recursive: true });
await createBrokenGitMetadata(root, shape);

assert.deepEqual(
await resolveProjectLocation({ path: folder }),
{
canonicalPath: await realpath(folder),
identity: `folder:${await realpath(folder)}`,
kind: 'folder',
},
shape,
);
}
} finally {
await rm(base, { recursive: true, force: true });
}
});

test('a folder nested inside a repository resolves to that repository', async () => {
const base = await mkdtemp(join(tmpdir(), 'maka-project-nested-in-repo-'));
try {
const repository = join(base, 'repository');
const nested = join(repository, 'sub', 'dir');
await mkdir(nested, { recursive: true });
await execFileAsync('git', ['init', '--quiet'], { cwd: repository });

const resolved = await resolveProjectLocation({ path: nested });

assert.equal(resolved.kind, 'git');
assert.equal(resolved.git?.worktreeRoot, await realpath(repository));
} finally {
await rm(base, { recursive: true, force: true });
}
});

test('a folder nested inside a linked worktree resolves to that repository', async () => {
const base = await mkdtemp(join(tmpdir(), 'maka-project-nested-in-worktree-'));
try {
const repository = join(base, 'repository');
const linkedWorktree = join(base, 'linked');
await createGitRepositoryWithWorktree(repository, linkedWorktree, 'nested-linked');
const nested = join(linkedWorktree, 'nested');
await mkdir(nested);

const resolved = await resolveProjectLocation({ path: nested });

assert.equal(resolved.kind, 'git');
assert.equal(resolved.git?.isWorktree, true);
} finally {
await rm(base, { recursive: true, force: true });
}
});

test('a Git probe failure cannot persistently downgrade a repository to a folder', async () => {
const base = await mkdtemp(join(tmpdir(), 'maka-project-repository-no-git-'));
try {
Expand Down
56 changes: 55 additions & 1 deletion packages/storage/src/__tests__/workspace-identity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import assert from 'node:assert/strict';
import { execFile } from 'node:child_process';
import { access, chmod, mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises';
import { access, chmod, mkdir, mkdtemp, readFile, rm, symlink, writeFile } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { test } from 'node:test';
Expand All @@ -31,6 +31,7 @@ import {
WORKSPACE_MARKER_FILE,
WorkspaceIdentityError,
} from '../workspace-identity.js';
import { BROKEN_GIT_SHAPES, createBrokenGitMetadata } from './fixtures/git-repository.js';

const execFileAsync = promisify(execFile);

Expand Down Expand Up @@ -200,6 +201,38 @@ test('a full Git exclude does not grow when resolving workspace identity', async
}
});

test('a malformed ancestor .git directory does not block a workspace marker', async () => {
const base = await mkdtemp(join(tmpdir(), 'maka-workspace-git-malformed-ancestor-'));
try {
const workspace = join(base, 'workspace');
await mkdir(join(base, '.git', 'gk'), { recursive: true });
await mkdir(workspace);

await resolveWorkspaceIdentityWithoutGit(workspace);

await access(join(workspace, WORKSPACE_MARKER_FILE));
} finally {
await rm(base, { recursive: true, force: true });
}
});

test('broken ancestor Git metadata does not block a workspace marker', async () => {
const base = await mkdtemp(join(tmpdir(), 'maka-workspace-invalid-ancestor-'));
try {
for (const shape of BROKEN_GIT_SHAPES) {
const root = join(base, shape);
const workspace = join(root, 'workspace');
await mkdir(workspace, { recursive: true });
await createBrokenGitMetadata(root, shape);

await resolveWorkspaceIdentity({ path: workspace });
await access(join(workspace, WORKSPACE_MARKER_FILE));
}
} finally {
await rm(base, { recursive: true, force: true });
}
});

test('a malformed enclosing Git repository prevents publishing a new marker', async () => {
const workspace = await mkdtemp(join(tmpdir(), 'maka-workspace-git-malformed-'));
try {
Expand All @@ -216,6 +249,27 @@ test('a malformed enclosing Git repository prevents publishing a new marker', as
}
});

test('a dangling .git symlink in the workspace itself blocks marker publication', {
skip:
process.platform === 'win32'
? 'Windows symlink creation requires elevated privileges or Developer Mode'
: false,
}, async () => {
const workspace = await mkdtemp(join(tmpdir(), 'maka-workspace-git-dangling-'));
try {
await symlink(join(workspace, 'missing-target'), join(workspace, '.git'));

await assert.rejects(
() => resolveWorkspaceIdentity({ path: workspace }),
(error: unknown) =>
error instanceof WorkspaceIdentityError && error.code === 'workspace_io_failed',
);
await assert.rejects(access(join(workspace, WORKSPACE_MARKER_FILE)), { code: 'ENOENT' });
} finally {
await rm(workspace, { recursive: true, force: true });
}
});

test('a non-Git workspace resolves when the Git executable is unavailable', async () => {
const workspace = await mkdtemp(join(tmpdir(), 'maka-workspace-no-git-required-'));
try {
Expand Down
73 changes: 68 additions & 5 deletions packages/storage/src/git-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,84 @@
* under the License.
*/

import { lstat } from 'node:fs/promises';
import { join, parse } from 'node:path';
import { lstat, readFile, stat } from 'node:fs/promises';
import { join, parse, resolve } from 'node:path';

const GITDIR_PREFIX = 'gitdir: ';
const HEAD_REF_PREFIX = 'ref: ';
// HEAD holds a 40-char SHA-1 object id, or 64 chars for SHA-256 repositories.
const HEAD_OBJECT_ID = /^[0-9a-f]{40}(?:[0-9a-f]{24})?$/;

export async function hasEnclosingGitEntry(path: string): Promise<boolean> {
let current = path;
while (true) {
const gitPath = join(current, '.git');
try {
await lstat(join(current, '.git'));
return true;
// lstat does not follow symlinks, so a dangling `.git` symlink still
// counts as an existing entry. The selected directory fails closed
// downstream when its own Git metadata is damaged; an ancestor only
// counts when it is structurally valid.
const entry = await lstat(gitPath);
if (current === path) return true;
const gitStat = entry.isSymbolicLink() ? await stat(gitPath) : entry;
if (gitStat.isDirectory()) return isGitDirectory(gitPath);
if (gitStat.isFile()) {
const content = (await readFile(gitPath, 'utf8')).trim();
if (!content.startsWith(GITDIR_PREFIX)) return false;
const target = content.slice(GITDIR_PREFIX.length).trim();
if (!target) return false;
return isGitDirectory(resolve(current, target));
}
return false;
} catch (error) {
const code = (error as NodeJS.ErrnoException).code;
if (code !== 'ENOENT' && code !== 'ENOTDIR') throw error;
if (code !== 'ENOENT' && code !== 'ENOTDIR') return current === path;
}
const parent = parse(current).dir;
if (parent === current) return false;
current = parent;
}
}

/**
* Checks the minimum Git directory contract git-rev-parse relies on: a
* readable regular HEAD holding either a symref or an object id, plus the
* objects and refs directories. Anything else would make the downstream Git
* commands fail closed instead of being treated as an enclosing repository.
*/
async function isGitDirectory(gitDir: string): Promise<boolean> {
try {
// readFile fails closed on a missing, unreadable, or directory HEAD.
const head = (await readFile(join(gitDir, 'HEAD'), 'utf8')).trim();
const validHead = head.startsWith(HEAD_REF_PREFIX)
? head.slice(HEAD_REF_PREFIX.length).trim() !== ''

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] This accepts every non-empty ref: ... suffix, including strings that Git does not recognize as a repository HEAD. On this exact head, an ancestor containing .git/objects, .git/refs, and HEAD set to ref: gk passes isGitDirectory(), but git -C <nested> rev-parse ... exits 128. Through the production paths, resolveProjectLocation() then throws that Git failure and resolveWorkspaceIdentity() returns workspace_io_failed without publishing a marker. The malformed ancestor therefore still blocks the two workflows this PR is intended to recover. Please validate the symbolic ref shape and add this case to the shared broken-metadata fixture.

: HEAD_OBJECT_ID.test(head);
if (!validHead) return false;
} catch {
return false;
}
// Linked worktrees keep HEAD locally but share objects/refs with the
// common dir named by their commondir file.
return (
(await hasGitSubdirectory(gitDir, 'objects')) && (await hasGitSubdirectory(gitDir, 'refs'))
);
}

async function hasGitSubdirectory(gitDir: string, name: string): Promise<boolean> {
if (await isDirectory(join(gitDir, name))) return true;
try {
const commonDir = (await readFile(join(gitDir, 'commondir'), 'utf8')).trim();
return commonDir !== '' && (await isDirectory(resolve(gitDir, commonDir, name)));
} catch {
// No commondir file: a plain Git directory.
return false;
}
}

async function isDirectory(path: string): Promise<boolean> {
try {
return (await stat(path)).isDirectory();
} catch {
return false;
}
}