diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index e1f705dea0e2..7c39e58f8880 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -1584,7 +1584,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { yield* runGit(repoDir, ["push", "-u", "upstream", "main"]); yield* runGit(repoDir, ["checkout", "-b", "develop"]); yield* runGit(repoDir, ["push", "-u", "upstream", "develop"]); - yield* runGit(remoteDir, ["symbolic-ref", "HEAD", "refs/heads/develop"]); + yield* runGit(remoteDir, ["--git-dir=.", "symbolic-ref", "HEAD", "refs/heads/develop"]); yield* runGit(repoDir, ["remote", "set-head", "upstream", "develop"]); const { manager } = yield* makeManager({ @@ -4198,9 +4198,13 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { ), ).toContain("?? .vercel/"); expect( - yield* runGit(remoteDir, ["log", "-1", "--pretty=%s", "feature/push-dirty"]).pipe( - Effect.map((output) => output.stdout.trim()), - ), + yield* runGit(remoteDir, [ + "--git-dir=.", + "log", + "-1", + "--pretty=%s", + "feature/push-dirty", + ]).pipe(Effect.map((output) => output.stdout.trim())), ).toBe("Push dirty branch"); }), ); @@ -4249,7 +4253,11 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { expect(result.pr.number).toBe(303); expect((yield* runGit(repoDir, ["status", "--porcelain"])).stdout).toBe(statusBefore); expect( - (yield* runGit(remoteDir, ["rev-parse", "feature/create-pr-only"])).stdout.trim(), + (yield* runGit(remoteDir, [ + "--git-dir=.", + "rev-parse", + "feature/create-pr-only", + ])).stdout.trim(), ).toBe(headBefore); expect( ghCalls.some((call) => @@ -5009,7 +5017,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { const remoteDir = yield* createBareRemote(); yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); yield* runGit(repoDir, ["push", "-u", "origin", "main"]); - yield* runGit(remoteDir, ["symbolic-ref", "HEAD", "refs/heads/main"]); + yield* runGit(remoteDir, ["--git-dir=.", "symbolic-ref", "HEAD", "refs/heads/main"]); const peerDir = yield* makeTempDir("t3code-git-peer-"); yield* runGit(peerDir, ["clone", remoteDir, "."]); diff --git a/apps/server/src/project/RepositoryIdentityResolver.ts b/apps/server/src/project/RepositoryIdentityResolver.ts index 57084dd179f3..dd94b6b5f086 100644 --- a/apps/server/src/project/RepositoryIdentityResolver.ts +++ b/apps/server/src/project/RepositoryIdentityResolver.ts @@ -11,6 +11,8 @@ import * as Exit from "effect/Exit"; import * as Layer from "effect/Layer"; import * as ProcessRunner from "../processRunner.ts"; +import { explicitBareRepositoryEnv } from "../vcs/VcsProcess.ts"; +import * as HostProcess from "@t3tools/shared/HostProcess"; const DEFAULT_REPOSITORY_IDENTITY_CACHE_CAPACITY = 512; // Background sweeps resolve every project each minute. A long TTL keeps them @@ -124,6 +126,7 @@ const resolveRepositoryIdentityCacheKey = Effect.fn("RepositoryIdentityResolver. .run({ command: "git", args: ["-C", cwd, "rev-parse", "--show-toplevel"], + env: explicitBareRepositoryEnv(process.env, yield* HostProcess.Platform), timeoutBehavior: "timedOutResult", }) .pipe(Effect.option); @@ -146,6 +149,7 @@ const resolveRepositoryIdentityFromCacheKey = Effect.fn( .run({ command: "git", args: ["-C", cacheKey, "remote", "-v"], + env: explicitBareRepositoryEnv(process.env, yield* HostProcess.Platform), timeoutBehavior: "timedOutResult", }) .pipe(Effect.option); diff --git a/apps/server/src/pullRequest/PullRequestReviewGuideService.ts b/apps/server/src/pullRequest/PullRequestReviewGuideService.ts index de3ee38c7aa2..8086541067fb 100644 --- a/apps/server/src/pullRequest/PullRequestReviewGuideService.ts +++ b/apps/server/src/pullRequest/PullRequestReviewGuideService.ts @@ -158,7 +158,7 @@ const make = Effect.gen(function* () { git.execute({ operation: "PullRequestReviewGuideService.fetch", cwd: workspaceRoot, - args: ["fetch", "--quiet", "--no-tags", remote, ...args], + args: ["fetch", "--quiet", "--no-tags", "--", remote, ...args], timeoutMs: 5 * 60_000, }); // The host names the base branch. A name git rejects, or one that would change the refspec diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index 88ff7b723884..21d947dcced8 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -7714,3 +7714,123 @@ it.effect.each([ assert.deepStrictEqual(calls, [expected]); }), ); + +it.effect("expands only files the change request changes, across every slice of its diff", () => + Effect.gen(function* () { + const read: Array = []; + const service = yield* makeService({ + projects: [ + project({ id: "p1", title: "t3code", workspaceRoot: "/a", repository: "pingdotgg/t3code" }), + ], + providers: [ + fakeProvider(SourceControlProviderKind.make("github"), { + getDiff: (input) => + Effect.succeed( + input.cursor === undefined + ? { + patch: [ + "diff --git a/src/old.ts b/src/new.ts", + "rename from src/old.ts", + "rename to src/new.ts", + "--- a/src/old.ts", + "+++ b/src/new.ts", + "", + ].join("\n"), + truncated: false, + nextCursor: "2", + } + : { + patch: [ + "diff --git a/gone.md b/gone.md", + "deleted file mode 100644", + "--- a/gone.md", + "+++ /dev/null", + "", + ].join("\n"), + truncated: false, + nextCursor: null, + }, + ), + getDiffFileContents: (input) => + Effect.sync(() => { + read.push(`${input.oldPath}->${input.newPath}`); + return { oldContents: "old", newContents: "new" }; + }), + }), + ], + }); + const ref = { projectId: "p1" as ProjectId, repository: "pingdotgg/t3code", number: 1 }; + + yield* service.diffFileContents({ + ...ref, + changeType: "rename-changed", + oldPath: "src/old.ts", + newPath: "src/new.ts", + }); + yield* service.diffFileContents({ + ...ref, + changeType: "deleted", + oldPath: "gone.md", + newPath: "gone.md", + }); + const outside = yield* service + .diffFileContents({ ...ref, changeType: "deleted", oldPath: ".env", newPath: ".env" }) + .pipe(Effect.flip); + const halfRename = yield* service + .diffFileContents({ + ...ref, + changeType: "change", + oldPath: "src/secret.ts", + newPath: "src/new.ts", + }) + .pipe(Effect.flip); + + assert.strictEqual(outside._tag, "PullRequestOperationError"); + assert.strictEqual(halfRename._tag, "PullRequestOperationError"); + assert.deepStrictEqual(read, ["src/old.ts->src/new.ts", "gone.md->gone.md"]); + }), +); + +it.effect("checks expanded files against a diff too large to cache without fetching it again", () => + Effect.gen(function* () { + let diffReads = 0; + const filler = `+${"x".repeat(300 * 1024)}`; + const service = yield* makeService({ + projects: [ + project({ id: "p1", title: "t3code", workspaceRoot: "/a", repository: "pingdotgg/t3code" }), + ], + providers: [ + fakeProvider(SourceControlProviderKind.make("github"), { + getDiff: () => + Effect.sync(() => { + diffReads += 1; + return { + patch: ["a.ts", "b.ts"] + .flatMap((name) => [ + `diff --git a/${name} b/${name}`, + `--- a/${name}`, + `+++ b/${name}`, + "@@ -0,0 +1 @@", + filler, + ]) + .join("\n"), + truncated: false, + nextCursor: null, + }; + }), + getDiffFileContents: () => Effect.succeed({ oldContents: "old", newContents: "new" }), + }), + ], + }); + const ref = { projectId: "p1" as ProjectId, repository: "pingdotgg/t3code", number: 1 }; + for (const name of ["a.ts", "b.ts"]) { + yield* service.diffFileContents({ + ...ref, + changeType: "change", + oldPath: name, + newPath: name, + }); + } + assert.strictEqual(diffReads, 1); + }), +); diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 5e871859586d..328e32526c1e 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -4,6 +4,7 @@ import { sourceControlRepositorySelector, } from "@t3tools/shared/sourceControl"; import { normalizeGitRemoteUrl } from "@t3tools/shared/git"; +import { parseDiffFilePaths } from "@t3tools/source-control-core/server/diffFileRevisions"; import * as Cache from "effect/Cache"; import * as Cause from "effect/Cause"; import * as Clock from "effect/Clock"; @@ -1717,7 +1718,7 @@ export const make = Effect.gen(function* () { ), ); - const diffFileContents: PullRequestService["Service"]["diffFileContents"] = (input) => + const readDiffFileContents: PullRequestService["Service"]["diffFileContents"] = (input) => requireProject(input).pipe( Effect.flatMap((project) => { const read = project.api.getDiffFileContents; @@ -3248,6 +3249,91 @@ export const make = Effect.gen(function* () { ); }); + /** + * The file names in each diff slice, keyed as the slice is. Names stay small when a patch is too + * large to cache, so expanding a file in a large change request does not fetch its diff again. + */ + const diffFiles = (() => { + const held = new Map< + string, + { + readonly at: number; + readonly files: ReadonlySet; + readonly nextCursor: string | null; + } + >(); + const ttlMs = Duration.toMillis(DIFF_CACHE_TTL); + const fileKey = (oldPath: string, newPath: string) => JSON.stringify([oldPath, newPath]); + const sliceKey = (input: PullRequestDiffInput) => + JSON.stringify([refCacheKey(input), input.cursor ?? null, input.commit ?? null]); + const read = (input: PullRequestDiffInput) => + Effect.gen(function* () { + const key = sliceKey(input); + const now = yield* Clock.currentTimeMillis; + const cached = held.get(key); + if (cached !== undefined && now - cached.at <= ttlMs) return cached; + const slice = yield* diff(input); + const entry = { + at: now, + files: new Set( + parseDiffFilePaths(slice.patch).map((file) => + fileKey(file.oldPath ?? file.newPath ?? "", file.newPath ?? file.oldPath ?? ""), + ), + ), + nextCursor: slice.nextCursor, + }; + held.delete(key); + if (held.size >= DIFF_CACHE_CAPACITY) { + const oldest = held.keys().next().value; + if (oldest !== undefined) held.delete(oldest); + } + held.set(key, entry); + return entry; + }); + return { read: credentialCached(read), fileKey }; + })(); + /** + * Whether the change request, or the one commit asked for, changes this file under these names. + * Walks the same slices the reader was shown, reading only their file names once held. + */ + const changesFile = (input: PullRequestDiffFileContentsInput) => { + const wanted = diffFiles.fileKey(input.oldPath, input.newPath); + const walk = (cursor: string | undefined): Effect.Effect => + diffFiles + .read({ + ...input, + ...(cursor === undefined ? {} : { cursor }), + ...(input.commit === undefined ? {} : { commit: input.commit }), + }) + .pipe( + Effect.flatMap((slice) => + slice.files.has(wanted) + ? Effect.succeed(true) + : slice.nextCursor === null + ? Effect.succeed(false) + : walk(slice.nextCursor), + ), + ); + return walk(undefined); + }; + // Only the files the change request itself changes: these are read with the project's own + // credential, and a reader who may look at the change request may not thereby read the rest of + // the repository. + const diffFileContents: PullRequestService["Service"]["diffFileContents"] = (input) => + changesFile(input).pipe( + Effect.flatMap((changed) => + changed + ? readDiffFileContents(input) + : Effect.fail( + new PullRequestOperationError({ + operation: "diffFileContents", + detail: "This file is not part of the change request.", + reason: "not-found", + }), + ), + ), + ); + return PullRequestService.of({ routing, routingIdentity, diff --git a/apps/server/src/sourceControl/githubStackRebase.test.ts b/apps/server/src/sourceControl/githubStackRebase.test.ts index 58c94ab53140..660521ebacb4 100644 --- a/apps/server/src/sourceControl/githubStackRebase.test.ts +++ b/apps/server/src/sourceControl/githubStackRebase.test.ts @@ -64,11 +64,11 @@ const setup = Effect.gen(function* () { yield* git(work, "add", "-A"); yield* git(work, "commit", "--quiet", "-m", "Docs"); yield* git(work, "push", "--quiet", "origin", "main", "feat/answer-42", "feat/answer-doc"); - const head = (branch: string) => git(remote, "rev-parse", `refs/heads/${branch}`); + const head = (branch: string) => git(remote, "--git-dir=.", "rev-parse", `refs/heads/${branch}`); return { remote, head, - git: (...args: string[]) => git(remote, ...args), + git: (...args: string[]) => git(remote, "--git-dir=.", ...args), layers: [ { number: 1, headBranch: "feat/answer-42", headSha: yield* head("feat/answer-42") }, { number: 2, headBranch: "feat/answer-doc", headSha: yield* head("feat/answer-doc") }, diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index fbc906218189..d68444867428 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -835,7 +835,7 @@ const make = Effect.gen(function* () { yield* cleanupGit(worktreePath, ["clean", "-ffdx"], "git clean"); yield* cleanupGit( project.workspaceRoot, - ["-c", "status.showUntrackedFiles=normal", "worktree", "remove", worktreePath], + ["-c", "status.showUntrackedFiles=normal", "worktree", "remove", "--", worktreePath], "git worktree remove", ); entries.push({ ...entry, outcome: "removed", reason: sentence(removalReason), bytes }); diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index ea3f4bb2c1af..6646b2e8aa56 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -38,6 +38,7 @@ import { windowsLongPathConfigEnv, } from "./GitVcsDriverCore.ts"; import * as GitVcsDriver from "./GitVcsDriver.ts"; +import { gitConfigEnv } from "./VcsProcess.ts"; const encodeGitCommandError = Schema.encodeEffect(Schema.fromJsonString(GitCommandError)); @@ -643,7 +644,7 @@ it.effect("invalidates a ref snapshot when a mutation fails after changing Git", if (!ChildProcess.isStandardCommand(command)) { return yield* Effect.die("expected a standard Git command"); } - if (command.args[0] === "branch" && command.args[1] === "feature/partial-failure") { + if (command.args[0] === "branch" && command.args.at(-1) === "feature/partial-failure") { const handle = yield* delegate.spawn(command); yield* handle.exitCode; return makeNonRepositoryHandle(); @@ -1037,7 +1038,7 @@ it.effect.each([ if (!ChildProcess.isStandardCommand(command)) return yield* Effect.die("expected Git command"); if (command.args[0] !== "fetch") return makeNonRepositoryHandle(); - assert.deepEqual(command.args, ["fetch", "--quiet", "origin"]); + assert.deepEqual(command.args, ["fetch", "--quiet", "--", "origin"]); assert.equal(command.options.env?.LC_ALL, "C"); assert.equal(command.options.env?.GIT_TERMINAL_PROMPT, "0"); yield* Ref.update(attempts, (count) => count + 1); @@ -1517,7 +1518,7 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { _tag: "GitCommandError", operation: "GitVcsDriver.removeWorktree", command: "git", - argumentCount: 5, + argumentCount: 6, cwd, }); assert.notProperty(error, "cause"); @@ -2118,6 +2119,26 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }), ); + it.effect("reads a base ref that looks like an option as a revision", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + const fileSystem = yield* FileSystem.FileSystem; + const output = (yield* Path.Path).join(yield* makeTmpDir(), "out"); + + yield* driver + .getReviewDiffFileContents( + makeReviewDiffFileContentsInput(cwd, { baseRef: `--output=${output}` }), + ) + .pipe(Effect.flip); + yield* driver.readRangeContext(cwd, `--output=${output}`).pipe(Effect.flip); + + assert.isFalse(yield* fileSystem.exists(`${output}:README.md`)); + assert.isFalse(yield* fileSystem.exists(`${output}..HEAD`)); + }), + ); + it.effect("attributes working-tree filesystem failures to the failing operation", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); @@ -2371,6 +2392,30 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }); describe("repository status", () => { + it.effect("does not open a project folder laid out as a bare repository", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + const fileSystem = yield* FileSystem.FileSystem; + const pathService = yield* Path.Path; + // Plain files a project save or a clone can hold. Git discovers them as a bare repository. + const planted = pathService.join(cwd, "planted"); + yield* writeTextFile(planted, "HEAD", "ref: refs/heads/main\n"); + yield* writeTextFile(planted, "objects/info/keep", ""); + yield* writeTextFile(planted, "refs/heads/keep", ""); + yield* writeTextFile( + planted, + "config", + '[core]\n\trepositoryformatversion = 0\n\tbare = false\n\tworktree = .\n\tfsmonitor = "echo ran >> fsmonitor-runs; false"\n', + ); + + yield* Effect.exit(driver.statusDetailsLocal(planted)); + assert.isFalse(yield* fileSystem.exists(pathService.join(planted, "fsmonitor-runs"))); + assert.isTrue((yield* driver.statusDetailsLocal(cwd)).isRepo); + }), + ); + it.effect("reads Changes totals with untracked files when requested", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); @@ -2947,6 +2992,71 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }), ); + it.effect("treats ref names that look like options as ref names", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + const fileSystem = yield* FileSystem.FileSystem; + const pathService = yield* Path.Path; + const remote = yield* makeTmpDir("git-remote-"); + yield* git(remote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "origin", remote]); + yield* git(cwd, ["push", "origin", "HEAD:refs/heads/feature", "HEAD:refs/heads/-M"]); + // Git refuses such branch names, but update-ref writes them, here tracking origin/feature. + yield* git(cwd, ["update-ref", "refs/heads/-f", "HEAD"]); + yield* git(cwd, ["config", "branch.-f.remote", "origin"]); + yield* git(cwd, ["config", "branch.-f.merge", "refs/heads/feature"]); + yield* git(cwd, ["branch", "keep"]); + yield* writeTextFile(cwd, "README.md", "uncommitted work\n"); + const branches = yield* driver.listLocalBranchNames(cwd); + const worktrees = yield* makeTmpDir("git-worktrees-"); + + yield* driver.switchRef({ cwd, refName: "-f" }).pipe(Effect.flip); + yield* driver.switchRef({ cwd, refName: "origin/feature" }).pipe(Effect.flip); + yield* driver.createRef({ cwd, refName: "--list" }).pipe(Effect.flip); + for (const [refName, newRefName] of [ + ["--detach", undefined], + ["-M", undefined], + ["-M", "renamed"], + ["-D", "keep"], + ["keep", "-M"], + ["keep", "-D"], + ] as const) { + yield* driver + .createWorktree({ + cwd, + path: pathService.join(worktrees, "wt"), + refName, + ...(newRefName ? { newRefName } : {}), + }) + .pipe(Effect.flip); + } + + assert.equal( + yield* fileSystem.readFileString(pathService.join(cwd, "README.md")), + "uncommitted work\n", + ); + assert.equal(yield* git(cwd, ["branch", "--show-current"]), initialBranch); + assert.deepEqual(yield* driver.listLocalBranchNames(cwd), branches); + assert.isFalse(yield* fileSystem.exists(pathService.join(worktrees, "wt"))); + + // A lone "-" is still the previous checkout. + yield* git(cwd, ["checkout", "-q", "keep"]); + yield* git(cwd, ["checkout", "-q", initialBranch]); + assert.equal((yield* driver.switchRef({ cwd, refName: "-" })).refName, "keep"); + const previous = yield* driver.createWorktree({ + cwd, + path: pathService.join(worktrees, "previous"), + refName: "-", + }); + assert.equal( + yield* git(previous.worktree.path, ["branch", "--show-current"]), + initialBranch, + ); + }), + ); + it.effect("still creates and reuses remote tracking branches and allows detached refs", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); @@ -3590,6 +3700,38 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }), ); + it.effect("fetches from a configured remote whose name looks like an option", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const remote = yield* makeTmpDir("git-remote-"); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + const fileSystem = yield* FileSystem.FileSystem; + // Remote names come from .git/config. Read as an option, this one runs `touch` in cwd. + const remoteName = "--upload-pack=touch${IFS}t3-marker;false"; + yield* git(remote, ["init", "--bare"]); + yield* git(cwd, [ + "push", + remote, + `HEAD:refs/heads/${initialBranch}`, + "HEAD:refs/pull/7/head", + ]); + yield* git(cwd, ["config", `remote.${remoteName}.url`, remote]); + + assert.equal(yield* driver.resolvePrimaryRemoteName(cwd), remoteName); + yield* driver.fetchRemoteTrackingBranch({ cwd, remoteName, remoteBranch: initialBranch }); + yield* driver.fetchRemote({ cwd, remoteName }); + const head = yield* driver.fetchPullRequestHeadCommit({ cwd, prNumber: 7 }); + + assert.isFalse(yield* fileSystem.exists((yield* Path.Path).join(cwd, "t3-marker"))); + assert.equal(head.commitSha, yield* git(cwd, ["rev-parse", "HEAD"])); + assert.equal( + yield* git(cwd, ["rev-parse", `refs/remotes/${remoteName}/${initialBranch}`]), + head.commitSha, + ); + }), + ); + it.effect("ensureRemote reuses an existing remote across ssh/https transport variants", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); @@ -3839,7 +3981,7 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { yield* git(remote, ["init", "--bare"]); yield* git(cwd, ["remote", "add", "origin", remote]); yield* git(cwd, ["push", "-u", "origin", initialBranch]); - yield* git(remote, ["symbolic-ref", "HEAD", `refs/heads/${initialBranch}`]); + yield* git(remote, ["--git-dir=.", "symbolic-ref", "HEAD", `refs/heads/${initialBranch}`]); const beforeFetch = yield* git(cwd, ["rev-parse", `refs/remotes/origin/${initialBranch}`]); yield* git(peer, ["clone", remote, "."]); @@ -4031,13 +4173,13 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { setUpstream: false, }); assert.equal( - yield* git(remote, ["log", "-1", "--pretty=%s", "main"]), + yield* git(remote, ["--git-dir=.", "log", "-1", "--pretty=%s", "main"]), "Add upstream update", ); const badBranch = yield* driver.execute({ operation: "GitVcsDriver.test.showBadRemoteBranch", cwd: remote, - args: ["show-ref", "--verify", "--quiet", "refs/heads/origin/main"], + args: ["--git-dir=.", "show-ref", "--verify", "--quiet", "refs/heads/origin/main"], allowNonZeroExit: true, timeoutMs: 10_000, }); @@ -4071,8 +4213,11 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { upstreamBranch: "origin/feature/x", setUpstream: true, }); - assert.equal(yield* git(remote, ["log", "-1", "--pretty=%s", "feature/x"]), "Add feature"); - assert.equal(yield* git(remote, ["rev-parse", "dev"]), devSha); + assert.equal( + yield* git(remote, ["--git-dir=.", "log", "-1", "--pretty=%s", "feature/x"]), + "Add feature", + ); + assert.equal(yield* git(remote, ["--git-dir=.", "rev-parse", "dev"]), devSha); assert.equal( yield* git(cwd, ["rev-parse", "--abbrev-ref", "@{upstream}"]), "origin/feature/x", @@ -4141,7 +4286,7 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { setUpstream: false, }); assert.equal( - yield* git(remote, ["log", "-1", "--pretty=%s", "effect-atom"]), + yield* git(remote, ["--git-dir=.", "log", "-1", "--pretty=%s", "effect-atom"]), "Add alias update", ); }), @@ -4169,13 +4314,13 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { setUpstream: true, }); assert.equal( - yield* git(publishRemote, ["log", "-1", "--pretty=%s", "main"]), + yield* git(publishRemote, ["--git-dir=.", "log", "-1", "--pretty=%s", "main"]), "initial commit", ); const originMain = yield* driver.execute({ operation: "GitVcsDriver.test.originMainMissing", cwd: originRemote, - args: ["show-ref", "--verify", "--quiet", "refs/heads/main"], + args: ["--git-dir=.", "show-ref", "--verify", "--quiet", "refs/heads/main"], allowNonZeroExit: true, timeoutMs: 10_000, }); @@ -4238,6 +4383,29 @@ describe("Windows long path configuration", () => { }); }); + it("extends the per-command count when inherited and per-command casings differ", () => { + assert.deepStrictEqual( + windowsLongPathConfigEnv("win32", { GIT_CONFIG_COUNT: "0", git_config_count: "1" }), + { + GIT_CONFIG_COUNT: "2", + git_config_count: "2", + GIT_CONFIG_KEY_1: "core.longpaths", + GIT_CONFIG_VALUE_1: "true", + }, + ); + }); + + it("reads only the exact count name outside Windows", () => { + assert.deepStrictEqual( + gitConfigEnv({ git_config_count: "3" }, "safe.bareRepository", "explicit", "linux"), + { + GIT_CONFIG_COUNT: "1", + GIT_CONFIG_KEY_0: "safe.bareRepository", + GIT_CONFIG_VALUE_0: "explicit", + }, + ); + }); + it("appends after inherited entries, parsing the count as Git does", () => { for (const [count, next] of [ [undefined, 0], diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index db56c68b48eb..2bb4477b17af 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -37,6 +37,7 @@ import { parseT3ProjectFile } from "@t3tools/shared/t3ProjectFile"; import { resolveProjectFileBackedSetting } from "@t3tools/shared/projectSettings"; import { gitCommandDuration, gitCommandsTotal, withMetrics } from "../observability/Metrics.ts"; import * as GitVcsDriver from "./GitVcsDriver.ts"; +import { explicitBareRepositoryEnv, gitConfigEnv } from "./VcsProcess.ts"; import { resolveWorktreesDirectory } from "../worktreesDirectory.ts"; import { parseRemoteNames, @@ -331,6 +332,24 @@ export function splitNullSeparatedGitStdoutPaths( return splitNullSeparatedPaths(result.stdout, result.stdoutTruncated); } +/** + * Refuses a ref name with a leading "-" before it reaches checkout or `worktree add`. Git refuses + * such branch and tag names, and both commands, or the `git branch` that `worktree add` runs, read + * one as an option. `--end-of-options` cannot guard them: Git before 2.44 reads it like `--`. + * A lone "-" is the previous checkout, which both commands read as a ref. + */ +const refuseOptionLikeRefName = (operation: string, cwd: string, refName: string | undefined) => + refName !== undefined && refName !== "-" && refName.startsWith("-") + ? Effect.fail( + new GitCommandError({ + operation, + command: "git", + cwd, + detail: "Ref names cannot start with '-'.", + }), + ) + : Effect.void; + function sanitizeRemoteName(value: string): string { const sanitized = value .trim() @@ -899,29 +918,8 @@ const collectOutput = Effect.fnUntraced(function* ( export const windowsLongPathConfigEnv = ( platform: NodeJS.Platform, env: NodeJS.ProcessEnv, -): NodeJS.ProcessEnv => { - if (platform !== "win32") { - return {}; - } - const countKey = - Object.keys(env).find((key) => key.toUpperCase() === "GIT_CONFIG_COUNT") ?? "GIT_CONFIG_COUNT"; - const inherited = env[countKey]; - if ( - inherited !== undefined && - inherited !== "" && - // Git parses the count with strtoul: leading whitespace, an optional sign - // (only -0 is non-negative), digits, and nothing after them. - /^[ \t\r\n\v\f]*(?:\+?\d+|-0+)/.exec(inherited)?.[0] !== inherited - ) { - return {}; - } - const count = inherited === undefined || inherited === "" ? 0 : Number.parseInt(inherited, 10); - return { - [countKey]: String(count + 1), - [`GIT_CONFIG_KEY_${count}`]: "core.longpaths", - [`GIT_CONFIG_VALUE_${count}`]: "true", - }; -}; +): NodeJS.ProcessEnv => + platform === "win32" ? gitConfigEnv(env, "core.longpaths", "true", platform) : {}; export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* () { const fileSystem = yield* FileSystem.FileSystem; @@ -954,7 +952,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* }), ), ); - const env = { + const baseEnv = { ...process.env, // Status polling runs beside the user's own git commands; without this, // `git status` takes index.lock to save its refreshed index. @@ -962,6 +960,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ...input.env, ...trace2Monitor.env, }; + const env = { ...baseEnv, ...explicitBareRepositoryEnv(baseEnv, hostPlatform) }; const spawnEnv = { ...env, ...windowsLongPathConfigEnv(hostPlatform, env) }; const resolved = yield* resolveSpawnCommand("git", [], { env: spawnEnv }); // A git.cmd wrapper would need cmd.exe, which cuts multi-line commit @@ -1270,7 +1269,16 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* return executeGit( "GitVcsDriver.fetchRemoteForStatus", fetchCwd, - ["--git-dir", gitCommonDir, "fetch", "--quiet", "--no-tags", "--no-auto-gc", remoteName], + [ + "--git-dir", + gitCommonDir, + "fetch", + "--quiet", + "--no-tags", + "--no-auto-gc", + "--", + remoteName, + ], { env: STATUS_UPSTREAM_REFRESH_ENV, fallbackErrorDetail: "Background Git fetch exited with a non-zero status.", @@ -1553,9 +1561,14 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ).pipe(Effect.map((result) => result.exitCode === 0)); const remoteExists: GitVcsDriver.GitVcsDriver["Service"]["remoteExists"] = (input) => - executeGit("GitVcsDriver.remoteExists", input.cwd, ["remote", "get-url", input.remoteName], { - allowNonZeroExit: true, - }).pipe(Effect.map((result) => result.exitCode === 0)); + executeGit( + "GitVcsDriver.remoteExists", + input.cwd, + ["remote", "get-url", "--", input.remoteName], + { + allowNonZeroExit: true, + }, + ).pipe(Effect.map((result) => result.exitCode === 0)); const originRemoteExists = (cwd: string): Effect.Effect => remoteExists({ cwd, remoteName: "origin" }); @@ -1647,6 +1660,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* yield* runGit("GitVcsDriver.ensureRemote.add", input.cwd, [ "remote", "add", + "--", remoteName, input.url, ]); @@ -1737,7 +1751,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const result = yield* executeGit( "GitVcsDriver.computeAheadCountAgainstBase", cwd, - ["rev-list", "--count", `${baseRef}..HEAD`], + ["rev-list", "--count", "--end-of-options", `${baseRef}..HEAD`], { allowNonZeroExit: true }, ); if (result.exitCode !== 0) { @@ -2337,7 +2351,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* yield* runGit( "GitVcsDriver.pushCurrentBranch.pushWithRequestedRemote", cwd, - ["push", "-u", requestedRemoteName, `HEAD:refs/heads/${publishBranch}`], + ["push", "-u", "--", requestedRemoteName, `HEAD:refs/heads/${publishBranch}`], { timeoutMs: null }, ); return { @@ -2402,7 +2416,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* yield* runGit( "GitVcsDriver.pushCurrentBranch.pushWithUpstream", cwd, - ["push", "-u", publishRemoteName, `HEAD:refs/heads/${publishBranch}`], + ["push", "-u", "--", publishRemoteName, `HEAD:refs/heads/${publishBranch}`], { timeoutMs: null }, ); return { @@ -2451,7 +2465,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* yield* runGit( "GitVcsDriver.pushCurrentBranch.pushOwnBranch", cwd, - ["push", "-u", remoteName, `HEAD:refs/heads/${publishBranch}`], + ["push", "-u", "--", remoteName, `HEAD:refs/heads/${publishBranch}`], { timeoutMs: null }, ); return { @@ -2465,7 +2479,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* yield* runGit( "GitVcsDriver.pushCurrentBranch.pushUpstream", cwd, - ["push", currentUpstream.remoteName, `HEAD:refs/heads/${currentUpstream.branchName}`], + ["push", "--", currentUpstream.remoteName, `HEAD:refs/heads/${currentUpstream.branchName}`], { timeoutMs: null }, ); return { @@ -2549,7 +2563,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* runGitStdoutWithOptions( "GitVcsDriver.readRangeContext.log", cwd, - ["log", "--oneline", commitRange], + ["log", "--oneline", "--end-of-options", commitRange], { maxOutputBytes: RANGE_COMMIT_SUMMARY_MAX_OUTPUT_BYTES, appendTruncationMarker: true, @@ -2558,7 +2572,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* runGitStdoutWithOptions( "GitVcsDriver.readRangeContext.diffStat", cwd, - ["diff", "--stat", diffRange], + ["diff", "--stat", "--end-of-options", diffRange], { maxOutputBytes: RANGE_DIFF_SUMMARY_MAX_OUTPUT_BYTES, appendTruncationMarker: true, @@ -2567,7 +2581,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* runGitStdoutWithOptions( "GitVcsDriver.readRangeContext.diffPatch", cwd, - ["diff", "--no-ext-diff", "--patch", "--minimal", diffRange], + ["diff", "--no-ext-diff", "--patch", "--minimal", "--end-of-options", diffRange], { maxOutputBytes: RANGE_DIFF_PATCH_MAX_OUTPUT_BYTES, appendTruncationMarker: true, @@ -2719,7 +2733,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ) : null); if (baseRef === null) return { baseRef, mergeBase: "HEAD" }; - const args = ["merge-base", baseRef, "HEAD"]; + const args = ["merge-base", "--end-of-options", baseRef, "HEAD"]; const result = yield* executeGit("GitVcsDriver.resolveReviewMergeBase", cwd, args, { allowNonZeroExit: true, }); @@ -2974,7 +2988,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const result = yield* executeGit( "GitVcsDriver.getReviewDiffFileContents.revision", input.cwd, - ["show", `${revision}:${relativePath}`], + ["show", "--end-of-options", `${revision}:${relativePath}`], { maxOutputBytes: REVIEW_DIFF_FILE_MAX_OUTPUT_BYTES }, ); if (result.stdout.includes("\0")) { @@ -3409,6 +3423,8 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const createWorktree: GitVcsDriver.GitVcsDriver["Service"]["createWorktree"] = Effect.fn( "createWorktree", )(function* (input, options) { + yield* refuseOptionLikeRefName("GitVcsDriver.createWorktree", input.cwd, input.refName); + yield* refuseOptionLikeRefName("GitVcsDriver.createWorktree", input.cwd, input.newRefName); const targetBranch = input.newRefName ?? input.refName; const sanitizedBranch = targetBranch.replace(/\//g, "-"); const repoName = path.basename(input.cwd); @@ -3431,8 +3447,8 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* worktreePath = path.join(parentDir, repoName, sanitizedBranch); } const args = input.newRefName - ? ["worktree", "add", "-b", input.newRefName, worktreePath, input.refName] - : ["worktree", "add", worktreePath, input.refName]; + ? ["worktree", "add", "-b", input.newRefName, "--", worktreePath, input.refName] + : ["worktree", "add", "--", worktreePath, input.refName]; const progress = options?.progress; const onCheckoutProgress = progress?.onCheckoutProgress; @@ -3561,6 +3577,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* "fetch", "--quiet", "--no-tags", + "--", remoteName, `+refs/pull/${input.prNumber}/head:refs/heads/${input.branch}`, ], @@ -3590,7 +3607,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* yield* executeGit( "GitVcsDriver.fetchPullRequestHeadCommit", input.cwd, - ["fetch", "--quiet", "--no-tags", remoteName, `refs/pull/${input.prNumber}/head`], + ["fetch", "--quiet", "--no-tags", "--", remoteName, `refs/pull/${input.prNumber}/head`], { fallbackErrorDetail: "git fetch pull request head failed", }, @@ -3661,7 +3678,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const fetchRemote: GitVcsDriver.GitVcsDriver["Service"]["fetchRemote"] = Effect.fn("fetchRemote")( function* (input) { - const args = ["fetch", "--quiet", input.remoteName]; + const args = ["fetch", "--quiet", "--", input.remoteName]; const options = { env: STATUS_UPSTREAM_REFRESH_ENV, fallbackErrorDetail: `git fetch ${input.remoteName} failed`, @@ -3762,6 +3779,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* "fetch", "--quiet", "--no-tags", + "--", input.remoteName, `+refs/heads/${input.remoteBranch}:refs/remotes/${input.remoteName}/${input.remoteBranch}`, ]); @@ -3772,8 +3790,8 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* "GitVcsDriver.fetchRemoteBranch.materialize", input.cwd, localBranchAlreadyExists - ? ["branch", "--force", input.localBranch, targetRef] - : ["branch", input.localBranch, targetRef], + ? ["branch", "--force", "--", input.localBranch, targetRef] + : ["branch", "--", input.localBranch, targetRef], ); }); @@ -3783,6 +3801,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* "fetch", "--quiet", "--no-tags", + "--", input.remoteName, `+refs/heads/${input.remoteBranch}:refs/remotes/${input.remoteName}/${input.remoteBranch}`, ]); @@ -3793,6 +3812,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* "branch", "--set-upstream-to", `${input.remoteName}/${input.remoteBranch}`, + "--", input.branch, ]); @@ -3805,7 +3825,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* if (input.force) { args.push("--force"); } - args.push(input.path); + args.push("--", input.path); const result = yield* executeGitWithStableDiagnostics( "GitVcsDriver.removeWorktree", input.cwd, @@ -3954,6 +3974,8 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ? ["checkout", localTrackingBranch] : ["checkout", input.refName]; + yield* refuseOptionLikeRefName("GitVcsDriver.switchRef", input.cwd, checkoutArgs.at(-1)); + // A stale ref must not turn into a path checkout that discards local edits. yield* executeGit("GitVcsDriver.switchRef.checkout", input.cwd, [...checkoutArgs, "--"], { timeoutMs: 10_000, @@ -3971,7 +3993,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const createRef: GitVcsDriver.GitVcsDriver["Service"]["createRef"] = Effect.fn("createRef")( function* (input) { - yield* executeGit("GitVcsDriver.createRef", input.cwd, ["branch", input.refName], { + yield* executeGit("GitVcsDriver.createRef", input.cwd, ["branch", "--", input.refName], { timeoutMs: 10_000, fallbackErrorDetail: "git branch create failed", }); diff --git a/apps/server/src/vcs/VcsProcess.test.ts b/apps/server/src/vcs/VcsProcess.test.ts index bbe95780a1f4..3e14efd78e89 100644 --- a/apps/server/src/vcs/VcsProcess.test.ts +++ b/apps/server/src/vcs/VcsProcess.test.ts @@ -7,7 +7,9 @@ import * as Effect from "effect/Effect"; import * as Exit from "effect/Exit"; import * as Cause from "effect/Cause"; import * as Fiber from "effect/Fiber"; +import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; +import * as Path from "effect/Path"; import * as Queue from "effect/Queue"; import * as Ref from "effect/Ref"; import * as Schema from "effect/Schema"; @@ -292,6 +294,32 @@ describe("VcsProcess.run", () => { }).pipe(provideLive), ); + it.effect("keeps git from opening a folder laid out as a bare repository", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-vcs-bare-" }); + yield* fileSystem.makeDirectory(path.join(cwd, "objects", "info"), { recursive: true }); + yield* fileSystem.makeDirectory(path.join(cwd, "refs", "heads"), { recursive: true }); + yield* fileSystem.writeFileString(path.join(cwd, "HEAD"), "ref: refs/heads/main\n"); + yield* fileSystem.writeFileString( + path.join(cwd, "config"), + '[core]\n\trepositoryformatversion = 0\n\tbare = false\n\tworktree = .\n\tfsmonitor = "echo ran >> fsmonitor-runs; false"\n', + ); + + const result = yield* run({ + operation: "test.bare-repository", + command: "git", + args: ["status"], + cwd, + allowNonZeroExit: true, + }); + + expect(result.exitCode).not.toBe(0); + expect(yield* fileSystem.exists(path.join(cwd, "fsmonitor-runs"))).toBe(false); + }).pipe(Effect.scoped, provideLive, Effect.provide(NodeServices.layer)), + ); + it.effect("writes stdin before waiting for exit", () => Effect.gen(function* () { const result = yield* run({ diff --git a/apps/server/src/vcs/VcsProcess.ts b/apps/server/src/vcs/VcsProcess.ts index 8c98419c3e4c..35694643753e 100644 --- a/apps/server/src/vcs/VcsProcess.ts +++ b/apps/server/src/vcs/VcsProcess.ts @@ -18,6 +18,8 @@ import { VcsProcessStdinWriteError, VcsProcessTimeoutError, } from "@t3tools/contracts"; +import * as HostProcess from "@t3tools/shared/HostProcess"; + import * as ProcessRunner from "../processRunner.ts"; export interface VcsProcessInput { @@ -61,6 +63,54 @@ const GITHUB_PROCESS_CONCURRENCY = 4; export const CHECKPOINT_CAPTURE_OPERATION = "GitVcsDriver.checkpoints.captureCheckpoint"; +/** + * Appends `key=value` to the `GIT_CONFIG_*` entries git reads as command-line + * config, so argv stays the same on every platform and nothing is written to + * the user's config. The entry goes after any inherited ones. Windows env names + * are case-insensitive (elsewhere git reads only `GIT_CONFIG_COUNT`), so there the + * count is found regardless of casing, read from + * the last spelling (a per-command value spread over process.env), and updated + * under every spelling so the child sees it whichever one Node keeps. A count + * git would reject is left alone so git still reports it. + */ +export const gitConfigEnv = ( + env: NodeJS.ProcessEnv, + key: string, + value: string, + platform: NodeJS.Platform, +): NodeJS.ProcessEnv => { + const countKeys = Object.keys(env).filter((name) => + platform === "win32" ? name.toUpperCase() === "GIT_CONFIG_COUNT" : name === "GIT_CONFIG_COUNT", + ); + const countKey = countKeys.at(-1) ?? "GIT_CONFIG_COUNT"; + const inherited = env[countKey]; + if ( + inherited !== undefined && + inherited !== "" && + // Git parses the count with strtoul: leading whitespace, an optional sign + // (only -0 is non-negative), digits, and nothing after them. + /^[ \t\r\n\v\f]*(?:\+?\d+|-0+)/.exec(inherited)?.[0] !== inherited + ) { + return {}; + } + const count = inherited === undefined || inherited === "" ? 0 : Number.parseInt(inherited, 10); + return { + ...Object.fromEntries([...countKeys, countKey].map((name) => [name, String(count + 1)])), + [`GIT_CONFIG_KEY_${count}`]: key, + [`GIT_CONFIG_VALUE_${count}`]: value, + }; +}; + +/** + * Git otherwise opens any folder holding `HEAD`, `objects/` and `refs/` as a + * bare repository and applies its `config`, including commands such as + * `core.fsmonitor`. Files saved in a project or checked out from a clone can + * form one, so git started by T3 (or by gh and friends) only opens a bare + * repository named with `--git-dir` or `GIT_DIR`. Git before 2.38 ignores it. + */ +export const explicitBareRepositoryEnv = (env: NodeJS.ProcessEnv, platform: NodeJS.Platform) => + gitConfigEnv(env, "safe.bareRepository", "explicit", platform); + const classifyNonZeroExit = (command: string, stderr: string): VcsProcessExitFailureKind => { const normalized = stderr.toLowerCase(); @@ -114,6 +164,7 @@ const isTransientGitExit = (stderr: string) => export const make = Effect.gen(function* () { const processRunner = yield* ProcessRunner.ProcessRunner; + const hostPlatform = yield* HostProcess.Platform; const vcsProcesses = yield* Semaphore.make(VCS_PROCESS_CONCURRENCY); const githubProcesses = yield* Semaphore.make(GITHUB_PROCESS_CONCURRENCY); @@ -133,7 +184,10 @@ export const make = Effect.gen(function* () { ...(input.spawnCwd !== undefined ? { spawnCwd: input.spawnCwd } : {}), ...(input.stdin !== undefined ? { stdin: input.stdin } : {}), ...(input.onStdoutChunk !== undefined ? { onStdoutChunk: input.onStdoutChunk } : {}), - ...(input.env !== undefined ? { env: input.env } : {}), + env: { + ...input.env, + ...explicitBareRepositoryEnv({ ...process.env, ...input.env }, hostPlatform), + }, timeout: input.timeoutMs ?? DEFAULT_TIMEOUT_MS, maxOutputBytes: input.maxOutputBytes ?? DEFAULT_MAX_OUTPUT_BYTES, outputMode: input.outputMode ?? "truncate", diff --git a/packages/source-control-core/src/server/diffFileRevisions.test.ts b/packages/source-control-core/src/server/diffFileRevisions.test.ts index cd1790adf8bb..7e79127810d5 100644 --- a/packages/source-control-core/src/server/diffFileRevisions.test.ts +++ b/packages/source-control-core/src/server/diffFileRevisions.test.ts @@ -1,6 +1,6 @@ import { assert, describe, it } from "@effect/vitest"; -import { parseDiffFileRevisions } from "./diffFileRevisions.ts"; +import { parseDiffFilePaths, parseDiffFileRevisions } from "./diffFileRevisions.ts"; function patchOf(...lines: ReadonlyArray): string { return `${lines.join("\n")}\n`; @@ -278,3 +278,36 @@ describe("parseDiffFileRevisions", () => { assert.deepStrictEqual([...revisions], [["one two", "eeeeeee"]]); }); }); + +describe("parseDiffFilePaths", () => { + it("names both sides of each file, with no side for an added or deleted one", () => { + const paths = parseDiffFilePaths( + [ + "diff --git a/src/a.ts b/src/a.ts", + "--- a/src/a.ts", + "+++ b/src/a.ts", + "@@ -1 +1 @@", + "--- a/not/a/header.ts", + "diff --git a/added.md b/added.md", + "new file mode 100644", + "--- /dev/null", + "+++ b/added.md", + "diff --git a/gone.md b/gone.md", + "deleted file mode 100644", + "--- a/gone.md", + "+++ /dev/null", + 'diff --git "a/tab\\tname" b/moved.ts', + 'rename from "tab\\tname"', + "rename to moved.ts", + "", + ].join("\n"), + ); + + assert.deepStrictEqual(paths, [ + { oldPath: "src/a.ts", newPath: "src/a.ts" }, + { oldPath: null, newPath: "added.md" }, + { oldPath: "gone.md", newPath: null }, + { oldPath: "tab\tname", newPath: "moved.ts" }, + ]); + }); +}); diff --git a/packages/source-control-core/src/server/diffFileRevisions.ts b/packages/source-control-core/src/server/diffFileRevisions.ts index b2f7e4498d01..2aa683e860c0 100644 --- a/packages/source-control-core/src/server/diffFileRevisions.ts +++ b/packages/source-control-core/src/server/diffFileRevisions.ts @@ -76,16 +76,14 @@ function headRevision(rest: string): string | null { return head.length === 0 ? null : head; } -export function parseDiffFileRevisions(patch: string): ReadonlyMap { - const revisions = new Map(); +function parseEntries(patch: string): ReadonlyArray> { + const entries: Array> = []; let entry: Entry | null = null; const close = () => { if (entry === null) return; - const path = entry.deleted ? entry.oldPath : (entry.newPath ?? entry.oldPath); - if (path !== null && path.length > 0 && entry.revision !== null) { - revisions.set(path, entry.revision); - } + const { inBody: _inBody, ...closed } = entry; + entries.push(closed); entry = null; }; @@ -116,5 +114,34 @@ export function parseDiffFileRevisions(patch: string): ReadonlyMap..` + * line: Bitbucket exposes no blob id for a file anywhere else. Keyed by the head's name, except + * for a deletion where only the old name exists. An entry with no `index` line (most often one + * Bitbucket excluded by pattern) is left out. + */ +export function parseDiffFileRevisions(patch: string): ReadonlyMap { + const revisions = new Map(); + for (const entry of parseEntries(patch)) { + const path = entry.deleted ? entry.oldPath : (entry.newPath ?? entry.oldPath); + if (path !== null && path.length > 0 && entry.revision !== null) { + revisions.set(path, entry.revision); + } + } return revisions; } + +/** + * The files a unified patch changes, as the name each side has. A side with no file (`/dev/null` + * for an added or a deleted file) is null; a rename is the only entry whose two names differ. + */ +export function parseDiffFilePaths( + patch: string, +): ReadonlyArray<{ readonly oldPath: string | null; readonly newPath: string | null }> { + return parseEntries(patch).flatMap(({ oldPath, newPath }) => + (oldPath ?? newPath) === null ? [] : [{ oldPath, newPath }], + ); +} diff --git a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts index 9bf81ee6c16d..957694c60ed8 100644 --- a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts +++ b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts @@ -451,6 +451,14 @@ export const make = Effect.gen(function* () { ], }); if (input.force) { + // The sha comes from the server and git would read anything but a commit as an option. + if (!/^[0-9a-f]{7,64}$/i.test(pull.head.sha)) + return yield* new ForgejoCli.ForgejoCliError({ + command: repo.command ?? "tea", + cwd: input.cwd, + detail: "Forgejo reported a pull request head that is not a commit.", + reason: "invalid-response", + }); // tea leaves an existing PR branch at its old tip. Keep dirty files safe // while bringing the selected branch to the PR revision we fetched. yield* process.run({ diff --git a/packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts b/packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts index e1e48092a5c6..d60ae1641f47 100644 --- a/packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts +++ b/packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts @@ -719,7 +719,7 @@ describe("GitHubSourceControlProvider.checkoutChangeRequest", () => { "fetchRemoteTrackingBranch", { cwd: "/repo", remoteName: "origin", remoteBranch: "feature/x" }, ], - ["execute", ["branch", "feature/x", "refs/remotes/origin/feature/x"]], + ["execute", ["branch", "--", "feature/x", "refs/remotes/origin/feature/x"]], ["switchRef", { cwd: "/repo", refName: "feature/x" }], [ "setBranchUpstream", diff --git a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts index 68b83d88098a..8c8aa06cd9a0 100644 --- a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts +++ b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts @@ -777,6 +777,7 @@ export const make = Effect.gen(function* () { "fetch", "--quiet", "--no-tags", + "--", yield* baseRemote, `refs/pull/${pullRequest.number}/head`, ]); @@ -791,7 +792,7 @@ export const make = Effect.gen(function* () { ); yield* Effect.gen(function* () { - if (!exists) yield* runGit(input.cwd, "branch", ["branch", localBranch, target.ref]); + if (!exists) yield* runGit(input.cwd, "branch", ["branch", "--", localBranch, target.ref]); yield* Effect.scoped(git.switchRef({ cwd: input.cwd, refName: localBranch })); if (exists) { yield* runGit( diff --git a/packages/source-control-gitlab/src/server/GitLabCli.test.ts b/packages/source-control-gitlab/src/server/GitLabCli.test.ts index 2756460f78f5..1b1e0b9d12a8 100644 --- a/packages/source-control-gitlab/src/server/GitLabCli.test.ts +++ b/packages/source-control-gitlab/src/server/GitLabCli.test.ts @@ -77,7 +77,7 @@ layer("GitLabCli.layer", (it) => { expect.objectContaining({ command: "glab", cwd: "/repo", - args: ["mr", "view", "42", "--output", "json"], + args: ["mr", "view", "--output", "json", "--", "42"], }), ); }), @@ -295,7 +295,7 @@ layer("GitLabCli.layer", (it) => { expect.objectContaining({ command: "glab", cwd: "/repo", - args: ["mr", "checkout", "42"], + args: ["mr", "checkout", "--", "42"], }), ); }), diff --git a/packages/source-control-gitlab/src/server/GitLabCli.ts b/packages/source-control-gitlab/src/server/GitLabCli.ts index 6845ee771001..00b900b8ab76 100644 --- a/packages/source-control-gitlab/src/server/GitLabCli.ts +++ b/packages/source-control-gitlab/src/server/GitLabCli.ts @@ -496,7 +496,7 @@ export const make = Effect.gen(function* () { executeMergeRequest({ cwd: input.cwd, reference: input.reference, - args: ["mr", "view", input.reference, "--output", "json"], + args: ["mr", "view", "--output", "json", "--", input.reference], }).pipe( Effect.map((result) => result.stdout.trim()), Effect.flatMap((raw) => @@ -652,7 +652,7 @@ export const make = Effect.gen(function* () { executeMergeRequest({ cwd: input.cwd, reference: input.reference, - args: ["mr", "checkout", input.reference], + args: ["mr", "checkout", "--", input.reference], }).pipe(Effect.asVoid), }); }); diff --git a/packages/ssh/src/command.test.ts b/packages/ssh/src/command.test.ts index d6da8b417f9c..28efc3c9487a 100644 --- a/packages/ssh/src/command.test.ts +++ b/packages/ssh/src/command.test.ts @@ -8,7 +8,7 @@ import * as Result from "effect/Result"; import * as Sink from "effect/Sink"; import * as Stream from "effect/Stream"; import * as TestClock from "effect/testing/TestClock"; -import { ChildProcessSpawner } from "effect/process"; +import { ChildProcess, ChildProcessSpawner } from "effect/process"; import { baseSshArgs, @@ -99,6 +99,29 @@ describe("ssh command", () => { }), ); + it.effect("passes an option-shaped alias to ssh as the destination", () => { + const spawned: Array> = []; + const spawner = ChildProcessSpawner.make((command) => { + if (ChildProcess.isStandardCommand(command)) spawned.push(command.args); + return Effect.succeed(makeFailedProcess({ stdout: "", stderr: "bad hostname\n" })); + }); + return runSshCommand( + { alias: "-oProxyCommand=false", hostname: "devbox", username: null, port: null }, + { remoteCommandArgs: ["sh", "-s"] }, + ).pipe( + Effect.flip, + Effect.map(() => { + assert.deepEqual(spawned[0]?.slice(-4), ["--", "-oProxyCommand=false", "sh", "-s"]); + }), + Effect.provide( + Layer.merge( + NodeServices.layer, + Layer.succeed(ChildProcessSpawner.ChildProcessSpawner, spawner), + ), + ), + ); + }); + // Remote servers store state under this key, so it must not change across releases. it.effect("derives a stable remote state key", () => Effect.gen(function* () { diff --git a/packages/ssh/src/command.ts b/packages/ssh/src/command.ts index e149908a9604..744146c6b47d 100644 --- a/packages/ssh/src/command.ts +++ b/packages/ssh/src/command.ts @@ -202,6 +202,7 @@ const runSshCommandInScope = Effect.fn("ssh/command.runSshCommand.inScope")(func batchMode: input.batchMode ?? (input.interactiveAuth ? "no" : "yes"), }), ...(input.preHostArgs ?? []), + "--", hostSpec, ...(input.remoteCommandArgs ?? []), ]; diff --git a/packages/ssh/src/tunnel.test.ts b/packages/ssh/src/tunnel.test.ts index ebdf12696ed8..701d4f87df40 100644 --- a/packages/ssh/src/tunnel.test.ts +++ b/packages/ssh/src/tunnel.test.ts @@ -507,7 +507,8 @@ describe("ssh tunnel scripts", () => { kills += 1; }); } - if (args.includes("--")) return makeSuccessfulProcess('{"remotePort":3773}\n'); + if (args.some((arg, index) => arg === "-s" && args[index + 1] === "--")) + return makeSuccessfulProcess('{"remotePort":3773}\n'); if (args.includes("sh")) stops += 1; return makeSuccessfulProcess(""); }), @@ -558,7 +559,7 @@ describe("ssh tunnel scripts", () => { tunnelKillCount += 1; }); } - if (args.includes("sh") && args.includes("--")) { + if (args.some((arg, index) => arg === "-s" && args[index + 1] === "--")) { return makeSuccessfulProcess('{"remotePort":3773}\n'); } if (args.includes("sh")) { @@ -602,6 +603,7 @@ describe("ssh tunnel scripts", () => { assert.include(firstTunnelArgs, "ControlMaster=no"); assert.include(firstTunnelArgs, "ControlPath=none"); assert.include(firstTunnelArgs, "ControlPersist=no"); + assert.deepEqual(firstTunnelArgs?.slice(-2), ["--", "julius@devbox"]); const disconnected = yield* Effect.result(manager.disconnectEnvironment(target)); if (mode === "failed stop") { @@ -679,7 +681,7 @@ describe("ssh tunnel scripts", () => { } return tunnel; } - if (args.includes("--")) { + if (args.some((arg, index) => arg === "-s" && args[index + 1] === "--")) { if (isTarget) { launches += 1; remoteRunning = true; diff --git a/packages/ssh/src/tunnel.ts b/packages/ssh/src/tunnel.ts index 355e5c66c0bf..e4e24ce9bba2 100644 --- a/packages/ssh/src/tunnel.ts +++ b/packages/ssh/src/tunnel.ts @@ -1180,6 +1180,7 @@ const startSshTunnel = Effect.fn("ssh/tunnel.startSshTunnel")(function* (input: "-N", "-L", `${input.localPort}:127.0.0.1:${input.remotePort}`, + "--", hostSpec, ]; const sshCommand = yield* resolveSshCommand;