Skip to content

Commit e20a374

Browse files
tyulyukovtyulyukov
authored
Merge pull request #61 from tyulyukov/marcode/port-upstream-provider-git-fixes
fix(provider): port upstream git and session management fixes
2 parents 33ef2dc + e134fe2 commit e20a374

40 files changed

Lines changed: 1658 additions & 325 deletions

‎apps/server/src/git/Layers/GitCore.test.ts‎

Lines changed: 82 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -828,7 +828,7 @@ it.layer(TestLayer)("git integration", (it) => {
828828
}),
829829
);
830830

831-
it.effect("shares upstream refreshes across worktrees that use the same git common dir", () =>
831+
it.effect("coalesces upstream refreshes across sibling worktrees on the same remote", () =>
832832
Effect.gen(function* () {
833833
const ok = (stdout = "") =>
834834
Effect.succeed({
@@ -847,7 +847,9 @@ it.layer(TestLayer)("git integration", (it) => {
847847
input.args[2] === "--symbolic-full-name" &&
848848
input.args[3] === "@{upstream}"
849849
) {
850-
return ok("origin/main\n");
850+
return ok(
851+
input.cwd === "/repo/worktrees/pr-123" ? "origin/feature/pr-123\n" : "origin/main\n",
852+
);
851853
}
852854
if (input.args[0] === "remote") {
853855
return ok("origin\n");
@@ -858,10 +860,22 @@ it.layer(TestLayer)("git integration", (it) => {
858860
if (input.args[0] === "--git-dir" && input.args[2] === "fetch") {
859861
fetchCount += 1;
860862
expect(input.cwd).toBe("/repo");
863+
expect(input.args).toEqual([
864+
"--git-dir",
865+
"/repo/.git",
866+
"fetch",
867+
"--quiet",
868+
"--no-tags",
869+
"origin",
870+
]);
861871
return ok();
862872
}
863873
if (input.operation === "GitCore.statusDetails.status") {
864-
return ok("# branch.head main\n# branch.upstream origin/main\n# branch.ab +0 -0\n");
874+
return ok(
875+
input.cwd === "/repo/worktrees/pr-123"
876+
? "# branch.head feature/pr-123\n# branch.upstream origin/feature/pr-123\n# branch.ab +0 -0\n"
877+
: "# branch.head main\n# branch.upstream origin/main\n# branch.ab +0 -0\n",
878+
);
865879
}
866880
if (
867881
input.operation === "GitCore.statusDetails.unstagedNumstat" ||
@@ -888,70 +902,80 @@ it.layer(TestLayer)("git integration", (it) => {
888902
}),
889903
);
890904

891-
it.effect("briefly backs off failed upstream refreshes across sibling worktrees", () =>
892-
Effect.gen(function* () {
893-
const ok = (stdout = "") =>
894-
Effect.succeed({
895-
code: 0,
896-
stdout,
897-
stderr: "",
898-
stdoutTruncated: false,
899-
stderrTruncated: false,
900-
});
905+
it.effect(
906+
"briefly backs off failed upstream refreshes across sibling worktrees on one remote",
907+
() =>
908+
Effect.gen(function* () {
909+
const ok = (stdout = "") =>
910+
Effect.succeed({
911+
code: 0,
912+
stdout,
913+
stderr: "",
914+
stdoutTruncated: false,
915+
stderrTruncated: false,
916+
});
901917

902-
let fetchCount = 0;
903-
const core = yield* makeIsolatedGitCore((input) => {
904-
if (
905-
input.args[0] === "rev-parse" &&
906-
input.args[1] === "--abbrev-ref" &&
907-
input.args[2] === "--symbolic-full-name" &&
908-
input.args[3] === "@{upstream}"
909-
) {
910-
return ok("origin/main\n");
911-
}
912-
if (input.args[0] === "remote") {
913-
return ok("origin\n");
914-
}
915-
if (input.args[0] === "rev-parse" && input.args[1] === "--git-common-dir") {
916-
return ok("/repo/.git\n");
917-
}
918-
if (input.args[0] === "--git-dir" && input.args[2] === "fetch") {
919-
fetchCount += 1;
918+
let fetchCount = 0;
919+
const core = yield* makeIsolatedGitCore((input) => {
920+
if (
921+
input.args[0] === "rev-parse" &&
922+
input.args[1] === "--abbrev-ref" &&
923+
input.args[2] === "--symbolic-full-name" &&
924+
input.args[3] === "@{upstream}"
925+
) {
926+
return ok(
927+
input.cwd === "/repo/worktrees/pr-123"
928+
? "origin/feature/pr-123\n"
929+
: "origin/main\n",
930+
);
931+
}
932+
if (input.args[0] === "remote") {
933+
return ok("origin\n");
934+
}
935+
if (input.args[0] === "rev-parse" && input.args[1] === "--git-common-dir") {
936+
return ok("/repo/.git\n");
937+
}
938+
if (input.args[0] === "--git-dir" && input.args[2] === "fetch") {
939+
fetchCount += 1;
940+
return Effect.fail(
941+
new GitCommandError({
942+
operation: input.operation,
943+
command: `git ${input.args.join(" ")}`,
944+
cwd: input.cwd,
945+
detail: "simulated fetch timeout",
946+
}),
947+
);
948+
}
949+
if (input.operation === "GitCore.statusDetails.status") {
950+
return ok(
951+
input.cwd === "/repo/worktrees/pr-123"
952+
? "# branch.head feature/pr-123\n# branch.upstream origin/feature/pr-123\n# branch.ab +0 -0\n"
953+
: "# branch.head main\n# branch.upstream origin/main\n# branch.ab +0 -0\n",
954+
);
955+
}
956+
if (
957+
input.operation === "GitCore.statusDetails.unstagedNumstat" ||
958+
input.operation === "GitCore.statusDetails.stagedNumstat"
959+
) {
960+
return ok();
961+
}
962+
if (input.operation === "GitCore.statusDetails.defaultRef") {
963+
return ok("refs/remotes/origin/main\n");
964+
}
920965
return Effect.fail(
921966
new GitCommandError({
922967
operation: input.operation,
923968
command: `git ${input.args.join(" ")}`,
924969
cwd: input.cwd,
925-
detail: "simulated fetch timeout",
970+
detail: "Unexpected git command in refresh failure cooldown test.",
926971
}),
927972
);
928-
}
929-
if (input.operation === "GitCore.statusDetails.status") {
930-
return ok("# branch.head main\n# branch.upstream origin/main\n# branch.ab +0 -0\n");
931-
}
932-
if (
933-
input.operation === "GitCore.statusDetails.unstagedNumstat" ||
934-
input.operation === "GitCore.statusDetails.stagedNumstat"
935-
) {
936-
return ok();
937-
}
938-
if (input.operation === "GitCore.statusDetails.defaultRef") {
939-
return ok("refs/remotes/origin/main\n");
940-
}
941-
return Effect.fail(
942-
new GitCommandError({
943-
operation: input.operation,
944-
command: `git ${input.args.join(" ")}`,
945-
cwd: input.cwd,
946-
detail: "Unexpected git command in refresh failure cooldown test.",
947-
}),
948-
);
949-
});
973+
});
950974

951-
yield* core.statusDetails("/repo/worktrees/main");
952-
yield* core.statusDetails("/repo/worktrees/pr-123");
953-
expect(fetchCount).toBe(1);
954-
}),
975+
yield* core.statusDetails("/repo/worktrees/main");
976+
yield* core.statusDetails("/repo/worktrees/pr-123");
977+
expect(fetchCount).toBe(1);
978+
}),
955979
);
956980

957981
it.effect("throws when branch does not exist", () =>
@@ -1049,7 +1073,6 @@ it.layer(TestLayer)("git integration", (it) => {
10491073
"--quiet",
10501074
"--no-tags",
10511075
remoteName,
1052-
`+refs/heads/${featureBranch}:refs/remotes/${remoteName}/${featureBranch}`,
10531076
]);
10541077
}),
10551078
);

‎apps/server/src/git/Layers/GitCore.ts‎

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,9 @@ type TraceTailState = {
7979
remainder: string;
8080
};
8181

82-
class StatusUpstreamRefreshCacheKey extends Data.Class<{
82+
class StatusRemoteRefreshCacheKey extends Data.Class<{
8383
gitCommonDir: string;
84-
upstreamRef: string;
8584
remoteName: string;
86-
upstreamBranch: string;
8785
}> {}
8886

8987
interface ExecuteGitOptions {
@@ -920,17 +918,16 @@ export const makeGitCore = Effect.fn("makeGitCore")(function* (options?: {
920918
);
921919
});
922920

923-
const fetchUpstreamRefForStatus = (
921+
const fetchRemoteForStatus = (
924922
gitCommonDir: string,
925-
upstream: { upstreamRef: string; remoteName: string; upstreamBranch: string },
923+
remoteName: string,
926924
): Effect.Effect<void, GitCommandError> => {
927-
const refspec = `+refs/heads/${upstream.upstreamBranch}:refs/remotes/${upstream.upstreamRef}`;
928925
const fetchCwd =
929926
path.basename(gitCommonDir) === ".git" ? path.dirname(gitCommonDir) : gitCommonDir;
930927
return executeGit(
931-
"GitCore.fetchUpstreamRefForStatus",
928+
"GitCore.fetchRemoteForStatus",
932929
fetchCwd,
933-
["--git-dir", gitCommonDir, "fetch", "--quiet", "--no-tags", upstream.remoteName, refspec],
930+
["--git-dir", gitCommonDir, "fetch", "--quiet", "--no-tags", remoteName],
934931
{
935932
allowNonZeroExit: true,
936933
timeoutMs: Duration.toMillis(STATUS_UPSTREAM_REFRESH_TIMEOUT),
@@ -946,18 +943,14 @@ export const makeGitCore = Effect.fn("makeGitCore")(function* (options?: {
946943
return path.isAbsolute(gitCommonDir) ? gitCommonDir : path.resolve(cwd, gitCommonDir);
947944
});
948945

949-
const refreshStatusUpstreamCacheEntry = Effect.fn("refreshStatusUpstreamCacheEntry")(function* (
950-
cacheKey: StatusUpstreamRefreshCacheKey,
946+
const refreshStatusRemoteCacheEntry = Effect.fn("refreshStatusRemoteCacheEntry")(function* (
947+
cacheKey: StatusRemoteRefreshCacheKey,
951948
) {
952-
yield* fetchUpstreamRefForStatus(cacheKey.gitCommonDir, {
953-
upstreamRef: cacheKey.upstreamRef,
954-
remoteName: cacheKey.remoteName,
955-
upstreamBranch: cacheKey.upstreamBranch,
956-
});
949+
yield* fetchRemoteForStatus(cacheKey.gitCommonDir, cacheKey.remoteName);
957950
return true as const;
958951
});
959952

960-
const statusUpstreamRefreshCache = yield* Cache.makeWith(refreshStatusUpstreamCacheEntry, {
953+
const statusRemoteRefreshCache = yield* Cache.makeWith(refreshStatusRemoteCacheEntry, {
961954
capacity: STATUS_UPSTREAM_REFRESH_CACHE_CAPACITY,
962955
// Keep successful refreshes warm and briefly back off failed refreshes to avoid retry storms.
963956
timeToLive: (exit) =>
@@ -973,12 +966,10 @@ export const makeGitCore = Effect.fn("makeGitCore")(function* (options?: {
973966
if (!upstream) return;
974967
const gitCommonDir = yield* resolveGitCommonDir(cwd);
975968
yield* Cache.get(
976-
statusUpstreamRefreshCache,
977-
new StatusUpstreamRefreshCacheKey({
969+
statusRemoteRefreshCache,
970+
new StatusRemoteRefreshCacheKey({
978971
gitCommonDir,
979-
upstreamRef: upstream.upstreamRef,
980972
remoteName: upstream.remoteName,
981-
upstreamBranch: upstream.upstreamBranch,
982973
}),
983974
);
984975
});

0 commit comments

Comments
 (0)