Skip to content

Commit f9019cd

Browse files
Coalesce status refreshes by remote (#1940)
Co-authored-by: codex <codex@users.noreply.github.com>
1 parent f9372a4 commit f9019cd

File tree

2 files changed

+93
-79
lines changed

2 files changed

+93
-79
lines changed

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

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

829-
it.effect("shares upstream refreshes across worktrees that use the same git common dir", () =>
829+
it.effect("coalesces upstream refreshes across sibling worktrees on the same remote", () =>
830830
Effect.gen(function* () {
831831
const ok = (stdout = "") =>
832832
Effect.succeed({
@@ -845,7 +845,9 @@ it.layer(TestLayer)("git integration", (it) => {
845845
input.args[2] === "--symbolic-full-name" &&
846846
input.args[3] === "@{upstream}"
847847
) {
848-
return ok("origin/main\n");
848+
return ok(
849+
input.cwd === "/repo/worktrees/pr-123" ? "origin/feature/pr-123\n" : "origin/main\n",
850+
);
849851
}
850852
if (input.args[0] === "remote") {
851853
return ok("origin\n");
@@ -856,10 +858,22 @@ it.layer(TestLayer)("git integration", (it) => {
856858
if (input.args[0] === "--git-dir" && input.args[2] === "fetch") {
857859
fetchCount += 1;
858860
expect(input.cwd).toBe("/repo");
861+
expect(input.args).toEqual([
862+
"--git-dir",
863+
"/repo/.git",
864+
"fetch",
865+
"--quiet",
866+
"--no-tags",
867+
"origin",
868+
]);
859869
return ok();
860870
}
861871
if (input.operation === "GitCore.statusDetails.status") {
862-
return ok("# branch.head main\n# branch.upstream origin/main\n# branch.ab +0 -0\n");
872+
return ok(
873+
input.cwd === "/repo/worktrees/pr-123"
874+
? "# branch.head feature/pr-123\n# branch.upstream origin/feature/pr-123\n# branch.ab +0 -0\n"
875+
: "# branch.head main\n# branch.upstream origin/main\n# branch.ab +0 -0\n",
876+
);
863877
}
864878
if (
865879
input.operation === "GitCore.statusDetails.unstagedNumstat" ||
@@ -886,70 +900,80 @@ it.layer(TestLayer)("git integration", (it) => {
886900
}),
887901
);
888902

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

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

949-
yield* core.statusDetails("/repo/worktrees/main");
950-
yield* core.statusDetails("/repo/worktrees/pr-123");
951-
expect(fetchCount).toBe(1);
952-
}),
973+
yield* core.statusDetails("/repo/worktrees/main");
974+
yield* core.statusDetails("/repo/worktrees/pr-123");
975+
expect(fetchCount).toBe(1);
976+
}),
953977
);
954978

955979
it.effect("throws when branch does not exist", () =>
@@ -1047,7 +1071,6 @@ it.layer(TestLayer)("git integration", (it) => {
10471071
"--quiet",
10481072
"--no-tags",
10491073
remoteName,
1050-
`+refs/heads/${featureBranch}:refs/remotes/${remoteName}/${featureBranch}`,
10511074
]);
10521075
}),
10531076
);

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

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

81-
class StatusUpstreamRefreshCacheKey extends Data.Class<{
81+
class StatusRemoteRefreshCacheKey extends Data.Class<{
8282
gitCommonDir: string;
83-
upstreamRef: string;
8483
remoteName: string;
85-
upstreamBranch: string;
8684
}> {}
8785

8886
interface ExecuteGitOptions {
@@ -919,17 +917,16 @@ export const makeGitCore = Effect.fn("makeGitCore")(function* (options?: {
919917
);
920918
});
921919

922-
const fetchUpstreamRefForStatus = (
920+
const fetchRemoteForStatus = (
923921
gitCommonDir: string,
924-
upstream: { upstreamRef: string; remoteName: string; upstreamBranch: string },
922+
remoteName: string,
925923
): Effect.Effect<void, GitCommandError> => {
926-
const refspec = `+refs/heads/${upstream.upstreamBranch}:refs/remotes/${upstream.upstreamRef}`;
927924
const fetchCwd =
928925
path.basename(gitCommonDir) === ".git" ? path.dirname(gitCommonDir) : gitCommonDir;
929926
return executeGit(
930-
"GitCore.fetchUpstreamRefForStatus",
927+
"GitCore.fetchRemoteForStatus",
931928
fetchCwd,
932-
["--git-dir", gitCommonDir, "fetch", "--quiet", "--no-tags", upstream.remoteName, refspec],
929+
["--git-dir", gitCommonDir, "fetch", "--quiet", "--no-tags", remoteName],
933930
{
934931
allowNonZeroExit: true,
935932
timeoutMs: Duration.toMillis(STATUS_UPSTREAM_REFRESH_TIMEOUT),
@@ -945,18 +942,14 @@ export const makeGitCore = Effect.fn("makeGitCore")(function* (options?: {
945942
return path.isAbsolute(gitCommonDir) ? gitCommonDir : path.resolve(cwd, gitCommonDir);
946943
});
947944

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

959-
const statusUpstreamRefreshCache = yield* Cache.makeWith(refreshStatusUpstreamCacheEntry, {
952+
const statusRemoteRefreshCache = yield* Cache.makeWith(refreshStatusRemoteCacheEntry, {
960953
capacity: STATUS_UPSTREAM_REFRESH_CACHE_CAPACITY,
961954
// Keep successful refreshes warm and briefly back off failed refreshes to avoid retry storms.
962955
timeToLive: (exit) =>
@@ -972,12 +965,10 @@ export const makeGitCore = Effect.fn("makeGitCore")(function* (options?: {
972965
if (!upstream) return;
973966
const gitCommonDir = yield* resolveGitCommonDir(cwd);
974967
yield* Cache.get(
975-
statusUpstreamRefreshCache,
976-
new StatusUpstreamRefreshCacheKey({
968+
statusRemoteRefreshCache,
969+
new StatusRemoteRefreshCacheKey({
977970
gitCommonDir,
978-
upstreamRef: upstream.upstreamRef,
979971
remoteName: upstream.remoteName,
980-
upstreamBranch: upstream.upstreamBranch,
981972
}),
982973
);
983974
});

0 commit comments

Comments
 (0)