From 9bb41383e37901601dac9182eb794bec1e826373 Mon Sep 17 00:00:00 2001 From: noah Date: Sun, 26 Jul 2026 21:57:03 -0400 Subject: [PATCH] fix(shared): stop reporting "not a worktree" when .git cannot be read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveGitWorktreePath swallowed every filesystem failure. `stat` ran under Effect.option, so any error became None and the walk continued upward; readFileString ran under orElseSucceed(() => ""), so any read error produced an empty string that parsed as "no gitdir pointer". In both cases the function answered `undefined` — not a worktree. That answer is not harmless here. resolveWorktreeT3Home's own docstring says it deliberately does not require `.t3` to exist because "falling back because it is missing would send callers at the shared home", and dev-runner then falls through to an ambient T3CODE_HOME. So an EACCES on a `.git` file, a transient EBUSY, or an unreachable network mount would quietly point a worktree dev server at ~/.t3 and the installed app's live userdata database — exactly the outcome this module exists to prevent. Distinguish absence from failure: only a NotFound reason means "no .git here, keep walking". Everything else propagates, so the error channel becomes PlatformError rather than never. Once stat has established the file exists, a read failure is a fault rather than an answer, so readFileString propagates too. The trade is deliberate: a dev server now refuses to start on an unreadable .git instead of starting against the wrong database. Failing loudly is recoverable; silently opening production state is not. Tests: the fixture named "unreadable-git-file" wrote a perfectly readable file containing "not a gitdir pointer" — that is the *unparseable* path, and the genuinely unreadable one had no coverage at all. Rename it accordingly and add a real chmod 000 case, skipped where the mode proves nothing (root, and Windows, which has no equivalent). Both new tests fail against the previous implementation and pass against this one. Also cover paths that existed only by inference: relative gitdir pointers (git >= 2.48 with worktree.useRelativePaths), Windows gitdir pointers — the sole reason replaceAll("\\", "/") exists — the segments.length >= 3 boundary, a repository nested inside a worktree, and resolveWorktreeT3Home's negative case. Rename the "bare" fixture to "no-repo": it creates no .git at all, which made it confusing next to "bare-repo-worktree", which genuinely is bare. --- packages/shared/src/devHome.test.ts | 107 ++++++++++++++++++++++++++-- packages/shared/src/devHome.ts | 37 ++++++++-- 2 files changed, 131 insertions(+), 13 deletions(-) diff --git a/packages/shared/src/devHome.test.ts b/packages/shared/src/devHome.test.ts index 812c34e3cce..1978b5a1209 100644 --- a/packages/shared/src/devHome.test.ts +++ b/packages/shared/src/devHome.test.ts @@ -8,15 +8,27 @@ import * as Effect from "effect/Effect"; import { resolveGitWorktreePath, resolveWorktreeT3Home } from "./devHome.ts"; +/** + * Denying read on the `.git` file only proves anything for a user the mode + * applies to. Root ignores it, and Windows has no equivalent, so the + * unreadable case is skipped rather than silently passing there. + */ +const canDenyReads = typeof process.getuid === "function" && process.getuid() !== 0; + const makeRepo = ( kind: | "worktree" | "checkout" - | "bare" + | "no-repo" | "submodule" + | "unparseable-git-file" | "unreadable-git-file" | "bare-repo-worktree" - | "custom-common-dir-worktree", + | "custom-common-dir-worktree" + | "relative-worktree" + | "windows-worktree" + | "shallow-gitdir" + | "nested-checkout", ) => Effect.acquireRelease( Effect.sync(() => { @@ -29,18 +41,45 @@ const makeRepo = ( } else if (kind === "custom-common-dir-worktree") { // $GIT_COMMON_DIR need not be named `.git` at all. NodeFS.writeFileSync(NodePath.join(root, ".git"), "gitdir: /srv/store/worktrees/x\n"); + } else if (kind === "relative-worktree") { + // git >= 2.48 with `worktree.useRelativePaths` writes a relative pointer. + NodeFS.writeFileSync(NodePath.join(root, ".git"), "gitdir: ../.git/worktrees/x\n"); + } else if (kind === "windows-worktree") { + NodeFS.writeFileSync(NodePath.join(root, ".git"), "gitdir: C:\\repo\\.git\\worktrees\\x\n"); + } else if (kind === "shallow-gitdir") { + // Nothing precedes `worktrees`, so there is no common dir to speak of. + NodeFS.writeFileSync(NodePath.join(root, ".git"), "gitdir: worktrees/x\n"); } else if (kind === "submodule") { NodeFS.writeFileSync(NodePath.join(root, ".git"), "gitdir: ../.git/modules/sub\n"); - } else if (kind === "unreadable-git-file") { + } else if (kind === "unparseable-git-file") { NodeFS.writeFileSync(NodePath.join(root, ".git"), "not a gitdir pointer\n"); + } else if (kind === "unreadable-git-file") { + const gitPath = NodePath.join(root, ".git"); + NodeFS.writeFileSync(gitPath, "gitdir: /elsewhere/.git/worktrees/x\n"); + NodeFS.chmodSync(gitPath, 0o000); } else if (kind === "checkout") { NodeFS.mkdirSync(NodePath.join(root, ".git")); } const nested = NodePath.join(root, "apps", "web", "src"); NodeFS.mkdirSync(nested, { recursive: true }); + if (kind === "nested-checkout") { + // A worktree that contains a second, unrelated repository. Walking up + // from inside the inner one must stop there, not adopt the outer root. + NodeFS.writeFileSync(NodePath.join(root, ".git"), "gitdir: /elsewhere/.git/worktrees/x\n"); + NodeFS.mkdirSync(NodePath.join(root, "apps", "web", ".git")); + } return { root, nested }; }), - ({ root }) => Effect.sync(() => NodeFS.rmSync(root, { recursive: true, force: true })), + ({ root }) => + Effect.sync(() => { + // Restore read permission first: a 0o000 file is still removable, but + // only because the directory is writable — do not depend on that. + const gitPath = NodePath.join(root, ".git"); + if (NodeFS.existsSync(gitPath) && NodeFS.statSync(gitPath).isFile()) { + NodeFS.chmodSync(gitPath, 0o600); + } + NodeFS.rmSync(root, { recursive: true, force: true }); + }), ); describe("resolveGitWorktreePath", () => { @@ -60,7 +99,7 @@ describe("resolveGitWorktreePath", () => { it.effect("reports a directory outside a repository", () => Effect.gen(function* () { - const { nested } = yield* makeRepo("bare"); + const { nested } = yield* makeRepo("no-repo"); assert.equal(yield* resolveGitWorktreePath(nested), undefined); }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); @@ -74,7 +113,7 @@ describe("resolveGitWorktreePath", () => { it.effect("reports a .git file without a usable gitdir pointer", () => Effect.gen(function* () { - const { nested } = yield* makeRepo("unreadable-git-file"); + const { nested } = yield* makeRepo("unparseable-git-file"); assert.equal(yield* resolveGitWorktreePath(nested), undefined); }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); @@ -92,6 +131,47 @@ describe("resolveGitWorktreePath", () => { assert.equal(yield* resolveGitWorktreePath(nested), NodePath.resolve(root)); }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); + + it.effect("finds a worktree from a relative gitdir pointer", () => + Effect.gen(function* () { + const { root, nested } = yield* makeRepo("relative-worktree"); + assert.equal(yield* resolveGitWorktreePath(nested), NodePath.resolve(root)); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); + + it.effect("finds a worktree from a Windows gitdir pointer", () => + Effect.gen(function* () { + const { root, nested } = yield* makeRepo("windows-worktree"); + assert.equal(yield* resolveGitWorktreePath(nested), NodePath.resolve(root)); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); + + it.effect("rejects a gitdir with nothing before `worktrees`", () => + Effect.gen(function* () { + const { nested } = yield* makeRepo("shallow-gitdir"); + assert.equal(yield* resolveGitWorktreePath(nested), undefined); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); + + it.effect("stops at a repository nested inside a worktree", () => + Effect.gen(function* () { + const { nested } = yield* makeRepo("nested-checkout"); + assert.equal(yield* resolveGitWorktreePath(nested), undefined); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); + + it.effect.skipIf(!canDenyReads)( + "fails instead of reporting `not a worktree` when .git cannot be read", + () => + Effect.gen(function* () { + const { nested } = yield* makeRepo("unreadable-git-file"); + // The whole point: an unreadable pointer is not an answer. Reporting + // `undefined` would send the caller at the shared home and its live + // database, which is the outcome this module exists to prevent. + const error = yield* Effect.flip(resolveGitWorktreePath(nested)); + assert.notEqual(error.reason._tag, "NotFound"); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); }); describe("resolveWorktreeT3Home", () => { @@ -103,4 +183,19 @@ describe("resolveWorktreeT3Home", () => { assert.isFalse(NodeFS.existsSync(home ?? "")); }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); + + it.effect("answers with undefined outside a linked worktree", () => + Effect.gen(function* () { + const { nested } = yield* makeRepo("checkout"); + assert.equal(yield* resolveWorktreeT3Home(nested), undefined); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); + + it.effect.skipIf(!canDenyReads)("propagates an unreadable .git rather than falling back", () => + Effect.gen(function* () { + const { nested } = yield* makeRepo("unreadable-git-file"); + const error = yield* Effect.flip(resolveWorktreeT3Home(nested)); + assert.notEqual(error.reason._tag, "NotFound"); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); }); diff --git a/packages/shared/src/devHome.ts b/packages/shared/src/devHome.ts index 1fde5409f5c..29384c0826c 100644 --- a/packages/shared/src/devHome.ts +++ b/packages/shared/src/devHome.ts @@ -12,6 +12,18 @@ import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Option from "effect/Option"; import * as Path from "effect/Path"; +import * as PlatformError from "effect/PlatformError"; + +/** + * A missing `.git` is the ordinary answer to "is this a worktree root?" — it + * means keep walking up. Any other failure is not evidence of absence: an + * unreadable file, a busy descriptor or a disconnected network mount all say + * "unknown", and answering "not a worktree" to unknown is precisely the + * outcome this module exists to prevent, because the caller then falls through + * to the shared home and its live database. Only absence is absence; the rest + * propagate. + */ +const isAbsent = (error: PlatformError.PlatformError): boolean => error.reason._tag === "NotFound"; /** * A `.git` file points at the real git directory. A linked worktree's lives at @@ -55,7 +67,11 @@ const pointsAtLinkedWorktree = (gitFileContents: string, path: Path.Path): boole */ export const resolveGitWorktreePath = ( cwd: string, -): Effect.Effect => +): Effect.Effect< + string | undefined, + PlatformError.PlatformError, + FileSystem.FileSystem | Path.Path +> => Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; const path = yield* Path.Path; @@ -63,7 +79,10 @@ export const resolveGitWorktreePath = ( let directory = path.resolve(cwd); for (;;) { const gitPath = path.join(directory, ".git"); - const info = yield* fileSystem.stat(gitPath).pipe(Effect.option); + const info = yield* fileSystem.stat(gitPath).pipe( + Effect.map(Option.some), + Effect.catchIf(isAbsent, () => Effect.succeed(Option.none())), + ); if (Option.isSome(info)) { // A directory means the main checkout. Stop either way: nesting one // repository inside another does not make the outer one this root. @@ -71,10 +90,10 @@ export const resolveGitWorktreePath = ( return undefined; } // A submodule also has a `.git` file, but it is not a worktree of this - // repository and gets no worktree-local home. - const contents = yield* fileSystem - .readFileString(gitPath) - .pipe(Effect.orElseSucceed(() => "")); + // repository and gets no worktree-local home. `stat` has already + // established the file is there, so a read failure here is a real + // fault rather than an answer — let it surface. + const contents = yield* fileSystem.readFileString(gitPath); return pointsAtLinkedWorktree(contents, path) ? directory : undefined; } const parent = path.dirname(directory); @@ -92,7 +111,11 @@ export const resolveGitWorktreePath = ( */ export const resolveWorktreeT3Home = ( cwd: string, -): Effect.Effect => +): Effect.Effect< + string | undefined, + PlatformError.PlatformError, + FileSystem.FileSystem | Path.Path +> => Effect.gen(function* () { const worktreePath = yield* resolveGitWorktreePath(cwd); if (worktreePath === undefined) {