Skip to content

Commit 99ce7c3

Browse files
Improve git status refresh and pull error messaging
- Refresh cached local status without repeating PR lookups - Expire remote status cache after 30s and surface pull blockers - Ignore legacy provider bindings with unknown provider names
1 parent e103187 commit 99ce7c3

13 files changed

Lines changed: 846 additions & 113 deletions

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2043,6 +2043,39 @@ it.layer(TestLayer)("git integration", (it) => {
20432043
}),
20442044
);
20452045

2046+
it.effect("explains local changes that block pull", () =>
2047+
Effect.gen(function* () {
2048+
const remote = yield* makeTmpDir();
2049+
const source = yield* makeTmpDir();
2050+
const clone = yield* makeTmpDir();
2051+
yield* git(remote, ["init", "--bare"]);
2052+
2053+
yield* initRepoWithCommit(source);
2054+
const initialBranch = (yield* (yield* GitCore).listBranches({ cwd: source })).branches.find(
2055+
(branch) => branch.current,
2056+
)!.name;
2057+
yield* git(source, ["remote", "add", "origin", remote]);
2058+
yield* git(source, ["push", "-u", "origin", initialBranch]);
2059+
2060+
yield* git(clone, ["clone", remote, "."]);
2061+
yield* git(clone, ["config", "user.email", "test@test.com"]);
2062+
yield* git(clone, ["config", "user.name", "Test"]);
2063+
yield* writeTextFile(path.join(clone, "README.md"), "remote change\n");
2064+
yield* git(clone, ["add", "README.md"]);
2065+
yield* git(clone, ["commit", "-m", "remote update"]);
2066+
yield* git(clone, ["push", "origin", initialBranch]);
2067+
2068+
yield* writeTextFile(path.join(source, "README.md"), "local change\n");
2069+
2070+
const result = yield* Effect.result((yield* GitCore).pullCurrentBranch(source));
2071+
expect(result._tag).toBe("Failure");
2072+
if (result._tag === "Failure") {
2073+
expect(result.failure.detail).toContain("Local changes block pull");
2074+
expect(result.failure.detail).toContain("README.md");
2075+
}
2076+
}),
2077+
);
2078+
20462079
it.effect("lists branches when recency lookup fails", () =>
20472080
Effect.gen(function* () {
20482081
const tmp = yield* makeTmpDir();

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

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ function createGitCommandError(
376376
const DIRTY_WORKTREE_PATTERN =
377377
/Your local changes to the following files would be overwritten by (?:checkout|merge):\s*([\s\S]*?)Please commit your changes or stash them/;
378378
const UNTRACKED_OVERWRITE_PATTERN =
379-
/The following untracked working tree files would be overwritten by checkout:\s*([\s\S]*?)Please move or remove them/;
379+
/The following untracked working tree files would be overwritten by (?:checkout|merge):\s*([\s\S]*?)Please move or remove them/;
380380

381381
function parseDirtyWorktreeFiles(stderr: string): string[] | null {
382382
const match = DIRTY_WORKTREE_PATTERN.exec(stderr) ?? UNTRACKED_OVERWRITE_PATTERN.exec(stderr);
@@ -388,6 +388,13 @@ function parseDirtyWorktreeFiles(stderr: string): string[] | null {
388388
return files.length > 0 ? files : null;
389389
}
390390

391+
function explainPullBlockedByLocalChanges(error: GitCommandError): string | null {
392+
const files = parseDirtyWorktreeFiles(error.detail);
393+
if (!files) return null;
394+
const fileList = files.map((file) => ` - ${file}`).join("\n");
395+
return `Local changes block pull. Commit or stash these files first:\n${fileList}`;
396+
}
397+
391398
function parseNonEmptyLineList(input: string): string[] {
392399
return input
393400
.trim()
@@ -1709,7 +1716,19 @@ export const makeGitCore = (options?: { executeOverride?: GitCoreShape["execute"
17091716
yield* executeGit("GitCore.pullCurrentBranch.pull", cwd, ["pull", "--ff-only"], {
17101717
timeoutMs: 30_000,
17111718
fallbackErrorMessage: "git pull failed",
1712-
});
1719+
}).pipe(
1720+
Effect.mapError((error) => {
1721+
const friendlyDetail = explainPullBlockedByLocalChanges(error);
1722+
if (!friendlyDetail) return error;
1723+
return createGitCommandError(
1724+
"GitCore.pullCurrentBranch.pull",
1725+
cwd,
1726+
["pull", "--ff-only"],
1727+
friendlyDetail,
1728+
error,
1729+
);
1730+
}),
1731+
);
17131732
const afterSha = yield* runGitStdout(
17141733
"GitCore.pullCurrentBranch.afterSha",
17151734
cwd,

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

Lines changed: 154 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import type { GitStatusResult, GitStatusStreamEvent } from "@t3tools/contracts";
22
import { Deferred, Effect, Layer, Scope, Stream } from "effect";
3-
import { describe, expect, it } from "vitest";
3+
import { afterEach, describe, expect, it, vi } from "vitest";
44

55
import type { GitManagerServiceError } from "../Errors";
6+
import { GitCore, type GitCoreShape, type GitStatusDetails } from "../Services/GitCore";
67
import { GitManager, type GitManagerShape } from "../Services/GitManager";
78
import { GitStatusBroadcaster } from "../Services/GitStatusBroadcaster";
89
import { GitStatusBroadcasterLive } from "./GitStatusBroadcaster";
@@ -17,7 +18,29 @@ const baseStatus: GitStatusResult = {
1718
pr: null,
1819
};
1920

20-
function makeTestLayer(state: { currentStatus: GitStatusResult; statusCalls: number }) {
21+
const baseDetails: GitStatusDetails = {
22+
branch: baseStatus.branch,
23+
upstreamRef: "origin/feature/status-broadcast",
24+
hasWorkingTreeChanges: baseStatus.hasWorkingTreeChanges,
25+
workingTree: baseStatus.workingTree,
26+
hasUpstream: baseStatus.hasUpstream,
27+
aheadCount: baseStatus.aheadCount,
28+
behindCount: baseStatus.behindCount,
29+
};
30+
31+
function makeTestLayer(state: {
32+
currentDetails: GitStatusDetails;
33+
currentStatus: GitStatusResult;
34+
detailsCalls: number;
35+
statusCalls: number;
36+
}) {
37+
const gitCore = {
38+
statusDetails: () =>
39+
Effect.sync(() => {
40+
state.detailsCalls += 1;
41+
return state.currentDetails;
42+
}),
43+
} as unknown as GitCoreShape;
2144
const gitManager: GitManagerShape = {
2245
status: () =>
2346
Effect.sync(() => {
@@ -33,35 +56,146 @@ function makeTestLayer(state: { currentStatus: GitStatusResult; statusCalls: num
3356
runStackedAction: () => Effect.die("runStackedAction should not be called in this test"),
3457
};
3558

36-
return GitStatusBroadcasterLive.pipe(Layer.provide(Layer.succeed(GitManager, gitManager)));
59+
return GitStatusBroadcasterLive.pipe(
60+
Layer.provide(
61+
Layer.mergeAll(Layer.succeed(GitCore, gitCore), Layer.succeed(GitManager, gitManager)),
62+
),
63+
);
3764
}
3865

3966
const runBroadcasterTest = (
40-
state: { currentStatus: GitStatusResult; statusCalls: number },
67+
state: {
68+
currentDetails: GitStatusDetails;
69+
currentStatus: GitStatusResult;
70+
detailsCalls: number;
71+
statusCalls: number;
72+
},
4173
effect: Effect.Effect<void, GitManagerServiceError, GitStatusBroadcaster | Scope.Scope>,
4274
) => effect.pipe(Effect.provide(makeTestLayer(state)), Effect.scoped, Effect.runPromise);
4375

76+
afterEach(() => {
77+
vi.useRealTimers();
78+
});
79+
4480
describe("GitStatusBroadcasterLive", () => {
45-
it("reuses the cached git status across repeated reads", async () => {
46-
const state = { currentStatus: baseStatus, statusCalls: 0 };
81+
it("refreshes local git status on repeated reads without repeating PR lookup", async () => {
82+
const state = {
83+
currentDetails: baseDetails,
84+
currentStatus: baseStatus,
85+
detailsCalls: 0,
86+
statusCalls: 0,
87+
};
4788

4889
await runBroadcasterTest(
4990
state,
5091
Effect.gen(function* () {
5192
const broadcaster = yield* GitStatusBroadcaster;
5293

5394
const first = yield* broadcaster.getStatus({ cwd: "/repo" });
95+
state.currentDetails = {
96+
...baseDetails,
97+
hasWorkingTreeChanges: true,
98+
workingTree: {
99+
files: [{ path: "src/app.ts", insertions: 5, deletions: 1 }],
100+
insertions: 5,
101+
deletions: 1,
102+
},
103+
};
54104
const second = yield* broadcaster.getStatus({ cwd: "/repo" });
55105

56106
expect(first).toEqual(baseStatus);
57-
expect(second).toEqual(baseStatus);
107+
expect(second).toEqual({
108+
...baseStatus,
109+
hasWorkingTreeChanges: true,
110+
workingTree: state.currentDetails.workingTree,
111+
});
58112
expect(state.statusCalls).toBe(1);
113+
expect(state.detailsCalls).toBe(1);
114+
}),
115+
);
116+
});
117+
118+
it("refreshes full status when cached remote metadata expires", async () => {
119+
vi.useFakeTimers();
120+
vi.setSystemTime(0);
121+
const state = {
122+
currentDetails: baseDetails,
123+
currentStatus: baseStatus,
124+
detailsCalls: 0,
125+
statusCalls: 0,
126+
};
127+
128+
await runBroadcasterTest(
129+
state,
130+
Effect.gen(function* () {
131+
const broadcaster = yield* GitStatusBroadcaster;
132+
133+
const first = yield* broadcaster.getStatus({ cwd: "/repo" });
134+
vi.setSystemTime(31_000);
135+
state.currentStatus = {
136+
...baseStatus,
137+
pr: {
138+
number: 42,
139+
title: "Open PR",
140+
url: "https://github.com/acme/repo/pull/42",
141+
state: "open",
142+
},
143+
};
144+
const second = yield* broadcaster.getStatus({ cwd: "/repo" });
145+
146+
expect(first.pr).toBeNull();
147+
expect(second.pr?.number).toBe(42);
148+
expect(state.statusCalls).toBe(2);
149+
expect(state.detailsCalls).toBe(1);
150+
}),
151+
);
152+
});
153+
154+
it("does not extend the remote metadata TTL when reusing cached remote status", async () => {
155+
vi.useFakeTimers();
156+
vi.setSystemTime(0);
157+
const state = {
158+
currentDetails: baseDetails,
159+
currentStatus: baseStatus,
160+
detailsCalls: 0,
161+
statusCalls: 0,
162+
};
163+
164+
await runBroadcasterTest(
165+
state,
166+
Effect.gen(function* () {
167+
const broadcaster = yield* GitStatusBroadcaster;
168+
169+
yield* broadcaster.getStatus({ cwd: "/repo" });
170+
vi.setSystemTime(20_000);
171+
yield* broadcaster.getStatus({ cwd: "/repo" });
172+
173+
vi.setSystemTime(31_000);
174+
state.currentStatus = {
175+
...baseStatus,
176+
pr: {
177+
number: 43,
178+
title: "Fresh PR",
179+
url: "https://github.com/acme/repo/pull/43",
180+
state: "open",
181+
},
182+
};
183+
const third = yield* broadcaster.getStatus({ cwd: "/repo" });
184+
185+
expect(third.pr?.number).toBe(43);
186+
expect(state.statusCalls).toBe(2);
187+
expect(state.detailsCalls).toBe(2);
59188
}),
60189
);
61190
});
62191

63192
it("refreshes the cached snapshot after explicit invalidation", async () => {
64-
const state = { currentStatus: baseStatus, statusCalls: 0 };
193+
const state = {
194+
currentDetails: baseDetails,
195+
currentStatus: baseStatus,
196+
detailsCalls: 0,
197+
statusCalls: 0,
198+
};
65199

66200
await runBroadcasterTest(
67201
state,
@@ -74,19 +208,30 @@ describe("GitStatusBroadcasterLive", () => {
74208
branch: "feature/updated-status",
75209
aheadCount: 2,
76210
};
211+
state.currentDetails = {
212+
...baseDetails,
213+
branch: "feature/updated-status",
214+
aheadCount: 2,
215+
};
77216
const refreshed = yield* broadcaster.refreshStatus("/repo");
78217
const cached = yield* broadcaster.getStatus({ cwd: "/repo" });
79218

80219
expect(initial).toEqual(baseStatus);
81220
expect(refreshed).toEqual(state.currentStatus);
82221
expect(cached).toEqual(state.currentStatus);
83222
expect(state.statusCalls).toBe(2);
223+
expect(state.detailsCalls).toBe(1);
84224
}),
85225
);
86226
});
87227

88228
it("streams a status snapshot first and later refresh updates", async () => {
89-
const state = { currentStatus: baseStatus, statusCalls: 0 };
229+
const state = {
230+
currentDetails: baseDetails,
231+
currentStatus: baseStatus,
232+
detailsCalls: 0,
233+
statusCalls: 0,
234+
};
90235

91236
await runBroadcasterTest(
92237
state,

0 commit comments

Comments
 (0)