Skip to content

Commit 8947a20

Browse files
committed
Fix git status bugs: double normalization, dead code, stale subscriptions, loose typing
- Remove redundant normalizeCwd calls in retainRemotePoller/releaseRemotePoller since callers already pass pre-normalized cwd values - Remove unused enqueueRefreshStatus and associated refreshWorker/import - Re-subscribe existing watchers when GitStatusClient changes to prevent stale UI after WebSocket reconnection - Add toLocalStatusPart to extract only local fields from GitStatusResult in the remoteUpdated case for type-safe merging
1 parent a499106 commit 8947a20

File tree

4 files changed

+30
-31
lines changed

4 files changed

+30
-31
lines changed

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

Lines changed: 7 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import type {
1818
GitStatusRemoteResult,
1919
GitStatusStreamEvent,
2020
} from "@t3tools/contracts";
21-
import { makeKeyedCoalescingWorker } from "@t3tools/shared/KeyedCoalescingWorker";
2221
import { mergeGitStatusParts } from "@t3tools/shared/git";
2322

2423
import {
@@ -203,23 +202,6 @@ export const GitStatusBroadcasterLive = Layer.effect(
203202
},
204203
);
205204

206-
const refreshWorker = yield* makeKeyedCoalescingWorker<string, void, never, never>({
207-
merge: () => undefined,
208-
process: (cwd) =>
209-
refreshStatus(cwd).pipe(
210-
Effect.catchCause((cause) =>
211-
Effect.logWarning("git status refresh failed", {
212-
cwd,
213-
cause,
214-
}),
215-
),
216-
Effect.asVoid,
217-
),
218-
});
219-
220-
const enqueueRefreshStatus: GitStatusBroadcasterShape["enqueueRefreshStatus"] = (cwd) =>
221-
refreshWorker.enqueue(normalizeCwd(cwd), undefined);
222-
223205
const makeRemoteRefreshLoop = (cwd: string) => {
224206
const logRefreshFailure = (error: Error) =>
225207
Effect.logWarning("git remote status refresh failed", {
@@ -240,23 +222,22 @@ export const GitStatusBroadcasterLive = Layer.effect(
240222
};
241223

242224
const retainRemotePoller = Effect.fn("retainRemotePoller")(function* (cwd: string) {
243-
const normalizedCwd = normalizeCwd(cwd);
244225
yield* SynchronizedRef.modifyEffect(pollersRef, (activePollers) => {
245-
const existing = activePollers.get(normalizedCwd);
226+
const existing = activePollers.get(cwd);
246227
if (existing) {
247228
const nextPollers = new Map(activePollers);
248-
nextPollers.set(normalizedCwd, {
229+
nextPollers.set(cwd, {
249230
...existing,
250231
subscriberCount: existing.subscriberCount + 1,
251232
});
252233
return Effect.succeed([undefined, nextPollers] as const);
253234
}
254235

255-
return makeRemoteRefreshLoop(normalizedCwd).pipe(
236+
return makeRemoteRefreshLoop(cwd).pipe(
256237
Effect.forkIn(broadcasterScope),
257238
Effect.map((fiber) => {
258239
const nextPollers = new Map(activePollers);
259-
nextPollers.set(normalizedCwd, {
240+
nextPollers.set(cwd, {
260241
fiber,
261242
subscriberCount: 1,
262243
});
@@ -267,24 +248,23 @@ export const GitStatusBroadcasterLive = Layer.effect(
267248
});
268249

269250
const releaseRemotePoller = Effect.fn("releaseRemotePoller")(function* (cwd: string) {
270-
const normalizedCwd = normalizeCwd(cwd);
271251
const pollerToInterrupt = yield* SynchronizedRef.modify(pollersRef, (activePollers) => {
272-
const existing = activePollers.get(normalizedCwd);
252+
const existing = activePollers.get(cwd);
273253
if (!existing) {
274254
return [null, activePollers] as const;
275255
}
276256

277257
if (existing.subscriberCount > 1) {
278258
const nextPollers = new Map(activePollers);
279-
nextPollers.set(normalizedCwd, {
259+
nextPollers.set(cwd, {
280260
...existing,
281261
subscriberCount: existing.subscriberCount - 1,
282262
});
283263
return [null, nextPollers] as const;
284264
}
285265

286266
const nextPollers = new Map(activePollers);
287-
nextPollers.delete(normalizedCwd);
267+
nextPollers.delete(cwd);
288268
return [existing.fiber, nextPollers] as const;
289269
});
290270

@@ -320,7 +300,6 @@ export const GitStatusBroadcasterLive = Layer.effect(
320300

321301
return {
322302
getStatus,
323-
enqueueRefreshStatus,
324303
refreshStatus,
325304
streamStatus,
326305
} satisfies GitStatusBroadcasterShape;

apps/server/src/git/Services/GitStatusBroadcaster.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ export interface GitStatusBroadcasterShape {
1111
readonly getStatus: (
1212
input: GitStatusInput,
1313
) => Effect.Effect<GitStatusResult, GitManagerServiceError>;
14-
readonly enqueueRefreshStatus: (cwd: string) => Effect.Effect<void>;
1514
readonly refreshStatus: (cwd: string) => Effect.Effect<GitStatusResult, GitManagerServiceError>;
1615
readonly streamStatus: (
1716
input: GitStatusInput,

apps/web/src/lib/gitStatusState.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,19 @@ function ensureGitStatusClient(client: GitStatusClient): void {
147147
}
148148

149149
function resetLiveGitStatusSubscriptions(): void {
150-
for (const watched of watchedGitStatuses.values()) {
150+
const previousCwds = new Map<string, number>();
151+
for (const [cwd, watched] of watchedGitStatuses) {
152+
previousCwds.set(cwd, watched.refCount);
151153
watched.unsubscribe();
152154
}
153155
watchedGitStatuses.clear();
156+
157+
for (const [cwd, refCount] of previousCwds) {
158+
watchedGitStatuses.set(cwd, {
159+
refCount,
160+
unsubscribe: subscribeToGitStatus(cwd),
161+
});
162+
}
154163
}
155164

156165
function unwatchGitStatus(cwd: string): void {

packages/shared/src/git.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,18 @@ export function mergeGitStatusParts(
209209
};
210210
}
211211

212+
function toLocalStatusPart(status: GitStatusResult): GitStatusLocalResult {
213+
return {
214+
isRepo: status.isRepo,
215+
hostingProvider: status.hostingProvider,
216+
hasOriginRemote: status.hasOriginRemote,
217+
isDefaultBranch: status.isDefaultBranch,
218+
branch: status.branch,
219+
hasWorkingTreeChanges: status.hasWorkingTreeChanges,
220+
workingTree: status.workingTree,
221+
};
222+
}
223+
212224
function toRemoteStatusPart(status: GitStatusResult): GitStatusRemoteResult {
213225
return {
214226
hasUpstream: status.hasUpstream,
@@ -241,6 +253,6 @@ export function applyGitStatusStreamEvent(
241253
event.remote,
242254
);
243255
}
244-
return mergeGitStatusParts(current, event.remote);
256+
return mergeGitStatusParts(toLocalStatusPart(current), event.remote);
245257
}
246258
}

0 commit comments

Comments
 (0)