Stream git status updates over WebSocket#1763
Conversation
- Add server-side status broadcaster and cache invalidation - Refresh git status after git actions and checkout - Co-authored-by: codex <codex@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
bot
added
size:XL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Exported function
gitCreateBranchMutationOptionsis never used- Removed the dead
gitCreateBranchMutationOptionsfunction which had zero callers in the codebase.
- Removed the dead
- ✅ Fixed: Stale entries persist in
useGitStatusesstate map- Added a pruning step at the start of the useEffect that removes map entries for cwds no longer in the subscribed set.
Or push these changes by commenting:
@cursor push de59b59aff
Preview (de59b59aff)
diff --git a/apps/web/src/lib/gitReactQuery.ts b/apps/web/src/lib/gitReactQuery.ts
--- a/apps/web/src/lib/gitReactQuery.ts
+++ b/apps/web/src/lib/gitReactQuery.ts
@@ -206,24 +206,6 @@
},
});
}
-
-export function gitCreateBranchMutationOptions(input: {
- cwd: string | null;
- queryClient: QueryClient;
-}) {
- return mutationOptions({
- mutationKey: ["git", "mutation", "create-branch", input.cwd] as const,
- mutationFn: async (branch: string) => {
- const api = ensureNativeApi();
- if (!input.cwd) throw new Error("Git branch creation is unavailable.");
- return api.git.createBranch({ cwd: input.cwd, branch });
- },
- onSuccess: async () => {
- await invalidateGitBranchQueries(input.queryClient, input.cwd);
- },
- });
-}
-
export function gitPreparePullRequestThreadMutationOptions(input: {
cwd: string | null;
queryClient: QueryClient;
diff --git a/apps/web/src/lib/gitStatusState.ts b/apps/web/src/lib/gitStatusState.ts
--- a/apps/web/src/lib/gitStatusState.ts
+++ b/apps/web/src/lib/gitStatusState.ts
@@ -90,6 +90,24 @@
);
useEffect(() => {
+ const cwdSet = new Set(cwds);
+
+ setStatusByCwd((current) => {
+ let pruned = false;
+ for (const key of current.keys()) {
+ if (!cwdSet.has(key)) {
+ pruned = true;
+ break;
+ }
+ }
+ if (!pruned) return current;
+ const next = new Map<string, GitStatusResult>();
+ for (const [key, value] of current) {
+ if (cwdSet.has(key)) next.set(key, value);
+ }
+ return next;
+ });
+
const cleanups = cwds.map((cwd) =>
appAtomRegistry.subscribe(
gitStatusStateAtom(cwd),You can send follow-ups to the cloud agent here.
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces a new streaming architecture for git status updates with substantial new logic across server and client. Multiple unresolved review comments identify potential bugs, including a high-severity issue where You can customize Macroscope's approvability policy. Learn more. |
- prune stale cwd status entries - make native API resets and test bootstrap async-safe
- Cache one live git-status stream per cwd - Reset status state for tests and native API teardown - Keep tracked cwd subscriptions stable across re-renders
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: PubSub subscription race in streamStatus misses updates
- Moved PubSub.subscribe before getStatus and replaced Stream.fromPubSub with Stream.fromEffectRepeat(PubSub.take(subscription)) so the subscription is established before the initial status is fetched, closing the race window.
- ✅ Fixed: Mutation failure no longer invalidates cached query state
- Changed onSuccess to onSettled for gitRunStackedActionMutationOptions and gitPullMutationOptions on the client, and added refreshGitStatus to the failure path of gitRunStackedAction and used Effect.ensuring for gitPull on the server, so cache invalidation and status broadcast fire regardless of success or failure.
Or push these changes by commenting:
@cursor push eb80e7016d
Preview (eb80e7016d)
diff --git a/apps/server/src/git/Layers/GitStatusBroadcaster.ts b/apps/server/src/git/Layers/GitStatusBroadcaster.ts
--- a/apps/server/src/git/Layers/GitStatusBroadcaster.ts
+++ b/apps/server/src/git/Layers/GitStatusBroadcaster.ts
@@ -127,11 +127,13 @@
Effect.gen(function* () {
const normalizedCwd = normalizeCwd(input.cwd);
yield* ensurePoller(normalizedCwd);
+
+ const subscription = yield* PubSub.subscribe(changesPubSub);
const initialStatus = yield* getStatus({ cwd: normalizedCwd });
return Stream.concat(
Stream.make(initialStatus),
- Stream.fromPubSub(changesPubSub).pipe(
+ Stream.fromEffectRepeat(PubSub.take(subscription)).pipe(
Stream.filter((event) => event.cwd === normalizedCwd),
Stream.map((event) => event.status),
),
diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts
--- a/apps/server/src/ws.ts
+++ b/apps/server/src/ws.ts
@@ -573,7 +573,7 @@
[WS_METHODS.gitPull]: (input) =>
observeRpcEffect(
WS_METHODS.gitPull,
- git.pullCurrentBranch(input.cwd).pipe(Effect.tap(() => refreshGitStatus(input.cwd))),
+ git.pullCurrentBranch(input.cwd).pipe(Effect.ensuring(refreshGitStatus(input.cwd))),
{ "rpc.aggregate": "git" },
),
[WS_METHODS.gitRunStackedAction]: (input) =>
@@ -589,7 +589,8 @@
})
.pipe(
Effect.matchCauseEffect({
- onFailure: (cause) => Queue.failCause(queue, cause),
+ onFailure: (cause) =>
+ refreshGitStatus(input.cwd).pipe(Effect.andThen(Queue.failCause(queue, cause))),
onSuccess: () =>
refreshGitStatus(input.cwd).pipe(
Effect.andThen(Queue.end(queue).pipe(Effect.asVoid)),
diff --git a/apps/web/src/lib/gitReactQuery.ts b/apps/web/src/lib/gitReactQuery.ts
--- a/apps/web/src/lib/gitReactQuery.ts
+++ b/apps/web/src/lib/gitReactQuery.ts
@@ -163,7 +163,7 @@
...(onProgress ? [{ onProgress }] : []),
);
},
- onSuccess: async () => {
+ onSettled: async () => {
await invalidateGitBranchQueries(input.queryClient, input.cwd);
},
});
@@ -177,7 +177,7 @@
if (!input.cwd) throw new Error("Git pull is unavailable.");
return api.git.pull({ cwd: input.cwd });
},
- onSuccess: async () => {
+ onSettled: async () => {
await invalidateGitBranchQueries(input.queryClient, input.cwd);
},
});You can send follow-ups to the cloud agent here.
- Resolve PR status from each thread’s cwd in the sidebar - Refactor git status state to shared per-cwd watches - Update git status state tests - Co-authored-by: codex <codex@users.noreply.github.com>
- Compute git status inputs before the null guard - Preserve hook order while rendering thread rows
- Rely on the existing watcher instead of resubscribing when the menu opens - Drop the obsolete refresh helper and its test
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Optional chaining produces undefined, bypassing null guard
- Changed
!== nullto!= null(loose inequality) so that whenthreadis undefined,thread?.branch(which isundefined) correctly evaluates as nullish, preventing a spurious git status subscription.
- Changed
Or push these changes by commenting:
@cursor push 75ca43d040
Preview (75ca43d040)
diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx
--- a/apps/web/src/components/Sidebar.tsx
+++ b/apps/web/src/components/Sidebar.tsx
@@ -298,7 +298,7 @@
selectThreadTerminalState(state.terminalStateByThreadId, props.threadId).runningTerminalIds,
);
const gitCwd = thread?.worktreePath ?? props.projectCwd;
- const gitStatus = useGitStatus(thread?.branch !== null ? gitCwd : null);
+ const gitStatus = useGitStatus(thread?.branch != null ? gitCwd : null);
if (!thread) {
return null;You can send follow-ups to the cloud agent here.
- Refresh git status after pull and stacked actions - Rehydrate status on window focus and menu open - Wire refresh through server, web, and contracts
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Unnecessary persistent atom created for empty-string key
- Replaced
gitStatusStateAtom(cwd ?? "")with a conditional that uses a static sentinel atom (EMPTY_GIT_STATUS_ATOM) whencwdis null, preventing the atom family from being called with an empty string and avoiding the phantom keepAlive atom and knownGitStatusCwds registration.
- Replaced
Or push these changes by commenting:
@cursor push 2b40cec530
Preview (2b40cec530)
diff --git a/apps/web/src/lib/gitStatusState.ts b/apps/web/src/lib/gitStatusState.ts
--- a/apps/web/src/lib/gitStatusState.ts
+++ b/apps/web/src/lib/gitStatusState.ts
@@ -30,6 +30,10 @@
isPending: false,
});
+const EMPTY_GIT_STATUS_ATOM = Atom.make(EMPTY_GIT_STATUS_STATE).pipe(
+ Atom.keepAlive,
+ Atom.withLabel("git-status:null"),
+);
const NOOP: () => void = () => undefined;
const watchedGitStatuses = new Map<string, WatchedGitStatus>();
const knownGitStatusCwds = new Set<string>();
@@ -126,7 +130,7 @@
export function useGitStatus(cwd: string | null): GitStatusState {
useEffect(() => watchGitStatus(cwd), [cwd]);
- const state = useAtomValue(gitStatusStateAtom(cwd ?? ""));
+ const state = useAtomValue(cwd !== null ? gitStatusStateAtom(cwd) : EMPTY_GIT_STATUS_ATOM);
return cwd === null ? EMPTY_GIT_STATUS_STATE : state;
}You can send follow-ups to the cloud agent here.
Co-authored-by: codex <codex@users.noreply.github.com>
github-actions
bot
added
size:XXL
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Blocking status refresh delays RPC response and stream completion
- Changed refreshGitStatus to use Effect.forkDetach() so the status refresh (including potential git fetch) runs as a detached fiber, returning the RPC response immediately instead of blocking on it.
- ✅ Fixed: Redundant error suppression on already-infallible effect
- Removed the redundant Effect.ignore({ log: true }) calls at gitPull and gitRunStackedAction call sites since refreshGitStatus is already infallible after Effect.ignoreCause({ log: true }).
Or push these changes by commenting:
@cursor push 3bba9f7511
Preview (3bba9f7511)
diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts
--- a/apps/server/src/ws.ts
+++ b/apps/server/src/ws.ts
@@ -353,7 +353,7 @@
const refreshGitStatus = (cwd: string) =>
gitStatusBroadcaster
.refreshStatus(cwd)
- .pipe(Effect.ignoreCause({ log: true }), Effect.asVoid);
+ .pipe(Effect.ignoreCause({ log: true }), Effect.forkDetach(), Effect.asVoid);
return WsRpcGroup.of({
[ORCHESTRATION_WS_METHODS.getSnapshot]: (_input) =>
@@ -581,9 +581,7 @@
[WS_METHODS.gitPull]: (input) =>
observeRpcEffect(
WS_METHODS.gitPull,
- git
- .pullCurrentBranch(input.cwd)
- .pipe(Effect.ensuring(refreshGitStatus(input.cwd).pipe(Effect.ignore({ log: true })))),
+ git.pullCurrentBranch(input.cwd).pipe(Effect.ensuring(refreshGitStatus(input.cwd))),
{ "rpc.aggregate": "git" },
),
[WS_METHODS.gitRunStackedAction]: (input) =>
@@ -600,10 +598,7 @@
.pipe(
Effect.matchCauseEffect({
onFailure: (cause) =>
- refreshGitStatus(input.cwd).pipe(
- Effect.ignore({ log: true }),
- Effect.andThen(Queue.failCause(queue, cause)),
- ),
+ refreshGitStatus(input.cwd).pipe(Effect.andThen(Queue.failCause(queue, cause))),
onSuccess: () =>
refreshGitStatus(input.cwd).pipe(
Effect.andThen(Queue.end(queue).pipe(Effect.asVoid)),You can send follow-ups to the cloud agent here.
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
| const streamStatus: GitStatusBroadcasterShape["streamStatus"] = (input) => | ||
| Stream.unwrap( | ||
| Effect.gen(function* () { | ||
| const normalizedCwd = normalizeCwd(input.cwd); | ||
| const subscription = yield* PubSub.subscribe(changesPubSub); | ||
| const initialLocal = yield* getOrLoadLocalStatus(normalizedCwd); | ||
| const initialRemote = (yield* getCachedStatus(normalizedCwd))?.remote?.value ?? null; | ||
| yield* retainRemotePoller(normalizedCwd); | ||
|
|
||
| const release = releaseRemotePoller(normalizedCwd).pipe(Effect.ignore, Effect.asVoid); | ||
|
|
||
| return Stream.concat( | ||
| Stream.make({ | ||
| _tag: "snapshot" as const, | ||
| local: initialLocal, | ||
| remote: initialRemote, | ||
| }), | ||
| Stream.fromSubscription(subscription).pipe( | ||
| Stream.filter((event) => event.cwd === normalizedCwd), | ||
| Stream.map((event) => event.event), | ||
| ), | ||
| ).pipe(Stream.ensuring(release)); | ||
| }), | ||
| ); |
There was a problem hiding this comment.
🟠 High Layers/GitStatusBroadcaster.ts:296
streamStatus uses Stream.unwrap with a PubSub.subscribe subscription, but Stream.unwrap does not extend the scope to the returned stream. When the inner effect completes, the subscription's scope closes — causing missed events or crashes when the stream is later consumed. Consider using Stream.unwrapScoped to tie the subscription's lifetime to stream consumption.
- const streamStatus: GitStatusBroadcasterShape["streamStatus"] = (input) =>
- Stream.unwrap(
+ const streamStatus: GitStatusBroadcasterShape["streamStatus"] = (input) =>
+ Stream.unwrapScoped(🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/git/Layers/GitStatusBroadcaster.ts around lines 296-319:
`streamStatus` uses `Stream.unwrap` with a `PubSub.subscribe` subscription, but `Stream.unwrap` does not extend the scope to the returned stream. When the inner effect completes, the subscription's scope closes — causing missed events or crashes when the stream is later consumed. Consider using `Stream.unwrapScoped` to tie the subscription's lifetime to stream consumption.
Evidence trail:
1. apps/server/src/git/Layers/GitStatusBroadcaster.ts lines 296-315 at REVIEWED_COMMIT - shows `Stream.unwrap` used with `PubSub.subscribe`
2. Effect-TS documentation at https://effect-ts.github.io/effect/effect/Stream.ts.html - shows type signatures for `unwrap` vs `unwrapScoped`, confirming `unwrapScoped` handles `Scope` differently by excluding it from requirements
3. Effect's own code at https://github.com/Effect-TS/effect/blob/main/packages/sql/src/internal/client.ts uses `Stream.unwrapScoped` for scoped resources (Mailbox)
Co-authored-by: codex <codex@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Initial
useGitStatusstate incorrectly reports not pending- Introduced PENDING_GIT_STATUS_STATE with isPending: true and used it as the initial value for per-cwd gitStatusStateAtom atoms, so the first render correctly shows a pending state before useEffect fires.
Or push these changes by commenting:
@cursor push d379c0f7b9
Preview (d379c0f7b9)
diff --git a/apps/web/src/lib/gitStatusState.ts b/apps/web/src/lib/gitStatusState.ts
--- a/apps/web/src/lib/gitStatusState.ts
+++ b/apps/web/src/lib/gitStatusState.ts
@@ -44,9 +44,16 @@
let sharedGitStatusClient: GitStatusClient | null = null;
+const PENDING_GIT_STATUS_STATE = Object.freeze<GitStatusState>({
+ data: null,
+ error: null,
+ cause: null,
+ isPending: true,
+});
+
const gitStatusStateAtom = Atom.family((cwd: string) => {
knownGitStatusCwds.add(cwd);
- return Atom.make(EMPTY_GIT_STATUS_STATE).pipe(
+ return Atom.make(PENDING_GIT_STATUS_STATE).pipe(
Atom.keepAlive,
Atom.withLabel(`git-status:${cwd}`),
);You can send follow-ups to the cloud agent here.
| error: null, | ||
| cause: null, | ||
| isPending: false, | ||
| }); |
There was a problem hiding this comment.
Initial useGitStatus state incorrectly reports not pending
Medium Severity
EMPTY_GIT_STATUS_STATE has isPending: false, and this is used as the initial value for the gitStatusStateAtom family. When useGitStatus(cwd) is called with a non-null cwd, the useEffect that calls watchGitStatus (and markGitStatusPending) runs after the first render. So during the first render, the hook returns { data: null, isPending: false }, incorrectly signaling that loading is complete when it hasn't even started. This can cause a single-frame flash where components behave as if no git data exists rather than showing a loading state.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit fc50e65. Configure here.
Co-authored-by: codex <codex@users.noreply.github.com>
- Replace manual release callback with `Scope.close` - Co-authored-by: codex <codex@users.noreply.github.com>
- Avoid duplicate refreshes when focus and visibility events fire together - Add coverage for the 250ms debounce
Co-authored-by: codex <codex@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
There are 6 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
- ✅ Fixed: Double normalization of cwd in poller retain/release
- Removed the redundant
normalizeCwd(cwd)calls insideretainRemotePollerandreleaseRemotePollersince their only call site instreamStatusalready passes a pre-normalized cwd.
- Removed the redundant
- ✅ Fixed:
enqueueRefreshStatusis exported but never consumed- Removed
enqueueRefreshStatusfrom the interface, its implementation, therefreshWorkerthat powered it, and the unusedmakeKeyedCoalescingWorkerimport.
- Removed
- ✅ Fixed: Stale subscriptions after client swap in
ensureGitStatusClient- Updated
resetLiveGitStatusSubscriptionsto capture existing cwd/refCount pairs before clearing, then re-subscribe each one with the new client so mounted components continue receiving updates.
- Updated
- ✅ Fixed: Git status stream
hostingProviderlost on remote update- Added
toLocalStatusPartto explicitly extract only local fields fromGitStatusResultand used it in theremoteUpdatedcase to ensure type-safe merging without relying on field-name disjointness.
- Added
Or push these changes by commenting:
@cursor push 8947a20575
Preview (8947a20575)
diff --git a/apps/server/src/git/Layers/GitStatusBroadcaster.ts b/apps/server/src/git/Layers/GitStatusBroadcaster.ts
--- a/apps/server/src/git/Layers/GitStatusBroadcaster.ts
+++ b/apps/server/src/git/Layers/GitStatusBroadcaster.ts
@@ -18,7 +18,6 @@
GitStatusRemoteResult,
GitStatusStreamEvent,
} from "@t3tools/contracts";
-import { makeKeyedCoalescingWorker } from "@t3tools/shared/KeyedCoalescingWorker";
import { mergeGitStatusParts } from "@t3tools/shared/git";
import {
@@ -203,23 +202,6 @@
},
);
- const refreshWorker = yield* makeKeyedCoalescingWorker<string, void, never, never>({
- merge: () => undefined,
- process: (cwd) =>
- refreshStatus(cwd).pipe(
- Effect.catchCause((cause) =>
- Effect.logWarning("git status refresh failed", {
- cwd,
- cause,
- }),
- ),
- Effect.asVoid,
- ),
- });
-
- const enqueueRefreshStatus: GitStatusBroadcasterShape["enqueueRefreshStatus"] = (cwd) =>
- refreshWorker.enqueue(normalizeCwd(cwd), undefined);
-
const makeRemoteRefreshLoop = (cwd: string) => {
const logRefreshFailure = (error: Error) =>
Effect.logWarning("git remote status refresh failed", {
@@ -240,23 +222,22 @@
};
const retainRemotePoller = Effect.fn("retainRemotePoller")(function* (cwd: string) {
- const normalizedCwd = normalizeCwd(cwd);
yield* SynchronizedRef.modifyEffect(pollersRef, (activePollers) => {
- const existing = activePollers.get(normalizedCwd);
+ const existing = activePollers.get(cwd);
if (existing) {
const nextPollers = new Map(activePollers);
- nextPollers.set(normalizedCwd, {
+ nextPollers.set(cwd, {
...existing,
subscriberCount: existing.subscriberCount + 1,
});
return Effect.succeed([undefined, nextPollers] as const);
}
- return makeRemoteRefreshLoop(normalizedCwd).pipe(
+ return makeRemoteRefreshLoop(cwd).pipe(
Effect.forkIn(broadcasterScope),
Effect.map((fiber) => {
const nextPollers = new Map(activePollers);
- nextPollers.set(normalizedCwd, {
+ nextPollers.set(cwd, {
fiber,
subscriberCount: 1,
});
@@ -267,16 +248,15 @@
});
const releaseRemotePoller = Effect.fn("releaseRemotePoller")(function* (cwd: string) {
- const normalizedCwd = normalizeCwd(cwd);
const pollerToInterrupt = yield* SynchronizedRef.modify(pollersRef, (activePollers) => {
- const existing = activePollers.get(normalizedCwd);
+ const existing = activePollers.get(cwd);
if (!existing) {
return [null, activePollers] as const;
}
if (existing.subscriberCount > 1) {
const nextPollers = new Map(activePollers);
- nextPollers.set(normalizedCwd, {
+ nextPollers.set(cwd, {
...existing,
subscriberCount: existing.subscriberCount - 1,
});
@@ -284,7 +264,7 @@
}
const nextPollers = new Map(activePollers);
- nextPollers.delete(normalizedCwd);
+ nextPollers.delete(cwd);
return [existing.fiber, nextPollers] as const;
});
@@ -320,7 +300,6 @@
return {
getStatus,
- enqueueRefreshStatus,
refreshStatus,
streamStatus,
} satisfies GitStatusBroadcasterShape;
diff --git a/apps/server/src/git/Services/GitStatusBroadcaster.ts b/apps/server/src/git/Services/GitStatusBroadcaster.ts
--- a/apps/server/src/git/Services/GitStatusBroadcaster.ts
+++ b/apps/server/src/git/Services/GitStatusBroadcaster.ts
@@ -11,7 +11,6 @@
readonly getStatus: (
input: GitStatusInput,
) => Effect.Effect<GitStatusResult, GitManagerServiceError>;
- readonly enqueueRefreshStatus: (cwd: string) => Effect.Effect<void>;
readonly refreshStatus: (cwd: string) => Effect.Effect<GitStatusResult, GitManagerServiceError>;
readonly streamStatus: (
input: GitStatusInput,
diff --git a/apps/web/src/lib/gitStatusState.ts b/apps/web/src/lib/gitStatusState.ts
--- a/apps/web/src/lib/gitStatusState.ts
+++ b/apps/web/src/lib/gitStatusState.ts
@@ -147,10 +147,19 @@
}
function resetLiveGitStatusSubscriptions(): void {
- for (const watched of watchedGitStatuses.values()) {
+ const previousCwds = new Map<string, number>();
+ for (const [cwd, watched] of watchedGitStatuses) {
+ previousCwds.set(cwd, watched.refCount);
watched.unsubscribe();
}
watchedGitStatuses.clear();
+
+ for (const [cwd, refCount] of previousCwds) {
+ watchedGitStatuses.set(cwd, {
+ refCount,
+ unsubscribe: subscribeToGitStatus(cwd),
+ });
+ }
}
function unwatchGitStatus(cwd: string): void {
diff --git a/packages/shared/src/git.ts b/packages/shared/src/git.ts
--- a/packages/shared/src/git.ts
+++ b/packages/shared/src/git.ts
@@ -209,6 +209,18 @@
};
}
+function toLocalStatusPart(status: GitStatusResult): GitStatusLocalResult {
+ return {
+ isRepo: status.isRepo,
+ hostingProvider: status.hostingProvider,
+ hasOriginRemote: status.hasOriginRemote,
+ isDefaultBranch: status.isDefaultBranch,
+ branch: status.branch,
+ hasWorkingTreeChanges: status.hasWorkingTreeChanges,
+ workingTree: status.workingTree,
+ };
+}
+
function toRemoteStatusPart(status: GitStatusResult): GitStatusRemoteResult {
return {
hasUpstream: status.hasUpstream,
@@ -241,6 +253,6 @@
event.remote,
);
}
- return mergeGitStatusParts(current, event.remote);
+ return mergeGitStatusParts(toLocalStatusPart(current), event.remote);
}
}You can send follow-ups to the cloud agent here.
| }; | ||
|
|
||
| const retainRemotePoller = Effect.fn("retainRemotePoller")(function* (cwd: string) { | ||
| const normalizedCwd = normalizeCwd(cwd); |
There was a problem hiding this comment.
Double normalization of cwd in poller retain/release
Low Severity
retainRemotePoller and releaseRemotePoller each call normalizeCwd(cwd) internally, but their only call site in streamStatus already passes a pre-normalized cwd (normalizedCwd). This redundant double normalization adds unnecessary filesystem syscalls on every subscribe/unsubscribe.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a499106. Configure here.
| }); | ||
|
|
||
| const enqueueRefreshStatus: GitStatusBroadcasterShape["enqueueRefreshStatus"] = (cwd) => | ||
| refreshWorker.enqueue(normalizeCwd(cwd), undefined); |
There was a problem hiding this comment.
enqueueRefreshStatus is exported but never consumed
Low Severity
enqueueRefreshStatus is defined on the GitStatusBroadcasterShape interface, implemented in GitStatusBroadcasterLive, and returned from the service, but is never called from any route handler in ws.ts or anywhere else in the codebase. This is dead code that adds surface area without providing value.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a499106. Configure here.
| } | ||
|
|
||
| sharedGitStatusClient = client; | ||
| } |
There was a problem hiding this comment.
Stale subscriptions after client swap in ensureGitStatusClient
Medium Severity
When ensureGitStatusClient detects a new client, resetLiveGitStatusSubscriptions clears watchedGitStatuses and unsubscribes all streams, but active useGitStatus hooks whose cwd dependency hasn't changed won't re-run their useEffect. Those components silently lose their live subscription and stop receiving updates until the cwd prop changes.
Reviewed by Cursor Bugbot for commit a499106. Configure here.
| event.remote, | ||
| ); | ||
| } | ||
| return mergeGitStatusParts(current, event.remote); |
There was a problem hiding this comment.
Git status stream hostingProvider lost on remote update
Medium Severity
In applyGitStatusStreamEvent, the remoteUpdated case passes the full current (GitStatusResult) to mergeGitStatusParts as the local parameter. Since GitStatusResult includes remote fields (hasUpstream, aheadCount, etc.) that are then overwritten by the spread, this works. However, the localUpdated case passes event.local (a GitStatusLocalResult) which may have hostingProvider, and the current object may also carry hostingProvider from a previous snapshot — but if a remoteUpdated event arrives and current carries hostingProvider, it's preserved because current is spread as local. This is fine. But the more subtle issue is that mergeGitStatusParts spreads local first and remote second, so any matching field names would be overwritten by remote. Currently the field sets are disjoint, but the typing is loose — current as GitStatusResult being passed as GitStatusLocalResult silently drops type safety. This is a correctness concern if future fields overlap.
Reviewed by Cursor Bugbot for commit a499106. Configure here.
|
Bugbot Autofix prepared fixes for both issues found in the latest run.
Or push these changes by commenting: Preview (5254e8bbe0)diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts
--- a/apps/server/src/ws.ts
+++ b/apps/server/src/ws.ts
@@ -354,7 +354,11 @@
const refreshGitStatus = (cwd: string) =>
gitStatusBroadcaster
.enqueueRefreshStatus(cwd)
- .pipe(Effect.ignoreCause({ log: true }), Effect.forkIn(wsBackgroundScope), Effect.asVoid);
+ .pipe(
+ Effect.ignoreCause({ log: true }),
+ Effect.forkIn(wsBackgroundScope),
+ Effect.ignore({ log: true }),
+ );
return WsRpcGroup.of({
[ORCHESTRATION_WS_METHODS.getSnapshot]: (_input) =>
diff --git a/apps/web/src/lib/gitReactQuery.ts b/apps/web/src/lib/gitReactQuery.ts
--- a/apps/web/src/lib/gitReactQuery.ts
+++ b/apps/web/src/lib/gitReactQuery.ts
@@ -41,14 +41,6 @@
return queryClient.invalidateQueries({ queryKey: gitQueryKeys.all });
}
-function invalidateGitBranchQueries(queryClient: QueryClient, cwd: string | null) {
- if (cwd === null) {
- return Promise.resolve();
- }
-
- return queryClient.invalidateQueries({ queryKey: gitQueryKeys.branches(cwd) });
-}
-
export function gitBranchSearchInfiniteQueryOptions(input: {
cwd: string | null;
query: string;
@@ -107,7 +99,7 @@
return api.git.init({ cwd: input.cwd });
},
onSettled: async () => {
- await invalidateGitBranchQueries(input.queryClient, input.cwd);
+ if (input.cwd) await invalidateGitQueries(input.queryClient, { cwd: input.cwd });
},
});
}
@@ -124,7 +116,7 @@
return api.git.checkout({ cwd: input.cwd, branch });
},
onSettled: async () => {
- await invalidateGitBranchQueries(input.queryClient, input.cwd);
+ if (input.cwd) await invalidateGitQueries(input.queryClient, { cwd: input.cwd });
},
});
}
@@ -164,7 +156,7 @@
);
},
onSuccess: async () => {
- await invalidateGitBranchQueries(input.queryClient, input.cwd);
+ if (input.cwd) await invalidateGitQueries(input.queryClient, { cwd: input.cwd });
},
});
}
@@ -178,7 +170,7 @@
return api.git.pull({ cwd: input.cwd });
},
onSuccess: async () => {
- await invalidateGitBranchQueries(input.queryClient, input.cwd);
+ if (input.cwd) await invalidateGitQueries(input.queryClient, { cwd: input.cwd });
},
});
}
@@ -228,7 +220,7 @@
});
},
onSuccess: async () => {
- await invalidateGitBranchQueries(input.queryClient, input.cwd);
+ if (input.cwd) await invalidateGitQueries(input.queryClient, { cwd: input.cwd });
},
});
}You can send follow-ups to the cloud agent here. |
- Invalidate only the active branch search query after branch actions - Avoid broad git query refreshes
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 7 total unresolved issues (including 5 from previous reviews).
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issues. You can view the agent here.
Reviewed by Cursor Bugbot for commit acb9bf1. Configure here.
| selectThreadTerminalState(state.terminalStateByThreadId, props.threadId).runningTerminalIds, | ||
| ); | ||
| const gitCwd = thread?.worktreePath ?? props.projectCwd; | ||
| const gitStatus = useGitStatus(thread?.branch != null ? gitCwd : null); |
There was a problem hiding this comment.
Sidebar creates per-thread WebSocket subscriptions for git status
Medium Severity
Each SidebarThreadRow now calls useGitStatus, which opens a WebSocket stream subscription per unique cwd. The previous implementation batched all thread status queries with useQueries and shared results by cwd. With many projects, each with threads on different worktree paths, this creates many concurrent WebSocket subscriptions and server-side remote pollers (each polling every 30 seconds), significantly increasing resource usage.
Reviewed by Cursor Bugbot for commit acb9bf1. Configure here.
| let nextBranchName = selectedBranchName; | ||
| if (branch.isRemote) { | ||
| const status = await api.git.status({ cwd: selectionTarget.checkoutCwd }).catch(() => null); | ||
| if (status?.branch) { |
There was a problem hiding this comment.
Checkout error no longer prevents onSetThreadBranch call
Medium Severity
When checkout fails, the old code used return after the toast to prevent onSetThreadBranch from being called. The new code places onSetThreadBranch inside the try block, which correctly avoids calling it on error. However, setOptimisticBranch is called before the try and is never reverted on failure, leaving the UI showing a branch name for a checkout that never completed.
Reviewed by Cursor Bugbot for commit acb9bf1. Configure here.
1 participant
Summary
git statusqueries to shared live git status state, including branch checkout handling for remote branches.Testing
Note
Medium Risk
Replaces the existing one-shot git status RPC with a streaming + refresh API and rewires both server and client state management, which can break consumers and introduce subtle cache/refresh timing issues. Also changes git checkout to return the resolved branch name, impacting IPC and UI flows.
Overview
Adds a server-side
GitStatusBroadcasterthat caches git status, publishesGitStatusStreamEventupdates, and exposes new WebSocket methodssubscribeGitStatus(stream) andgitRefreshStatus(unary), while removing the oldgit.statusRPC.Refactors
GitManager/GitCoreto split local vs remote status reads (with separate caches + invalidation), detect hosting provider from remote URLs, and returnGitCheckoutResultfromcheckoutBranch; git operations now trigger non-blocking background status refreshes.Updates the web app to use a shared
gitStatusStatesubscription (useGitStatus+ debouncedrefreshGitStatus) instead of react-query polling, adjusts branch checkout UI to use the checkout result, and updates tests/contracts/shared helpers accordingly (including new status merge/apply utilities).Reviewed by Cursor Bugbot for commit acb9bf1. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Stream git status updates over WebSocket instead of polling via react-query
gitStatusRPC and react-query polling with a persistentsubscribeGitStatusstream that pushesGitStatusStreamEventupdates (snapshot,localUpdated,remoteUpdated) to subscribers.GitStatusBroadcasterservice on the server that caches local and remote status independently, detects changes via fingerprints, and broadcasts diffs over a PubSub; remote status is polled on a 30-second interval only while subscribers are active.GitManagerstatus intolocalStatus/remoteStatuswith separate caches and invalidation methods;GitCore.checkoutBranchnow returns the resulting branch name.gitStatusStatemodule (gitStatusState.ts) manages ref-counted per-cwd stream subscriptions and exposes auseGitStatushook; all UI components (ChatView,DiffPanel,Sidebar,BranchToolbarBranchSelector,GitActionsControl) now use this hook instead of react-query.GitActionsControladds a debounced (250 ms) refresh on window focus and visibility change; multiple git-mutating RPCs (pull, checkout, worktree create/remove, etc.) now trigger a background status refresh on success.gitStatusQueryOptionsandinvalidateGitStatusQueryare removed fromgitReactQuery.ts; any code still importing those exports will break.Macroscope summarized acb9bf1.