Stream git status updates over WebSocket#1763
Open
juliusmarminge wants to merge 21 commits intomainfrom
Open
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}
Contributor
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.
Contributor
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces a significant new feature (WebSocket streaming for git status) with new services, contracts, and client-side state management. Additionally, there are 3 unresolved review comments identifying potential bugs including a high-severity issue with 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
Contributor
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
Contributor
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
Contributor
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>
Contributor
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>
Comment on lines
+296
to
+319
| 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)); | ||
| }), | ||
| ); |
Contributor
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.
Contributor
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.
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>
Contributor
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.
| } | ||
|
|
||
| sharedGitStatusClient = client; | ||
| } |
Contributor
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.
Contributor
|
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
Contributor
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).
Autofix Details
Bugbot Autofix prepared a fix for 1 of the 2 issues found in the latest run.
- ✅ Fixed: Sidebar creates per-thread WebSocket subscriptions for git status
- Replaced useGitStatus with a new useGitStatusPassive hook in SidebarThreadRow that reads cached atom state without creating WebSocket stream subscriptions or server-side remote pollers.
Or push these changes by commenting:
@cursor push ad8081d9e7
Preview (ad8081d9e7)
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
@@ -69,7 +69,7 @@
threadJumpIndexFromCommand,
threadTraversalDirectionFromCommand,
} from "../keybindings";
-import { useGitStatus } from "../lib/gitStatusState";
+import { useGitStatusPassive } from "../lib/gitStatusState";
import { readNativeApi } from "../nativeApi";
import { useComposerDraftStore } from "../composerDraftStore";
import { useHandleNewThread } from "../hooks/useHandleNewThread";
@@ -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 = useGitStatusPassive(thread?.branch != null ? gitCwd : null);
if (!thread) {
return null;
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
@@ -134,6 +134,16 @@
return cwd === null ? EMPTY_GIT_STATUS_STATE : state;
}
+/**
+ * Passively reads the current git status for a cwd without creating a
+ * WebSocket subscription. Use this when you only need to display cached
+ * status data (e.g. sidebar PR badges) without driving server-side polling.
+ */
+export function useGitStatusPassive(cwd: string | null): GitStatusState {
+ const state = useAtomValue(cwd !== null ? gitStatusStateAtom(cwd) : EMPTY_GIT_STATUS_ATOM);
+ return cwd === null ? EMPTY_GIT_STATUS_STATE : state;
+}
+
function ensureGitStatusClient(client: GitStatusClient): void {
if (sharedGitStatusClient === client) {
return;You can send follow-ups to the cloud agent here.
- Revert optimistic branch selection when checkout fails - Co-authored-by: codex <codex@users.noreply.github.com>
- Keep working tree and branch metadata intact when applying remote updates - Remove the unused enqueueRefreshStatus API
- Keep remote pollers keyed by the original cwd - Initialize new cwd snapshots as pending in the web state
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Git status errors silently swallowed, never surfaced to UI
- Added an onError callback to WsTransport.subscribe and StreamSubscriptionOptions, and wired it into subscribeToGitStatus to populate the atom's error and cause fields when the stream disconnects with an error, enabling the existing UI error rendering in GitActionsControl.
Or push these changes by commenting:
@cursor push 5228114a10
Preview (5228114a10)
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
@@ -193,6 +193,17 @@
onResubscribe: () => {
markGitStatusPending(cwd);
},
+ onError: (error) => {
+ const atom = gitStatusStateAtom(cwd);
+ const current = appAtomRegistry.get(atom);
+ const cause = Cause.fail(error as GitStatusStreamError);
+ appAtomRegistry.set(atom, {
+ data: current.data,
+ error: error as GitStatusStreamError,
+ cause,
+ isPending: false,
+ });
+ },
},
);
}
diff --git a/apps/web/src/wsRpcClient.ts b/apps/web/src/wsRpcClient.ts
--- a/apps/web/src/wsRpcClient.ts
+++ b/apps/web/src/wsRpcClient.ts
@@ -22,6 +22,7 @@
interface StreamSubscriptionOptions {
readonly onResubscribe?: () => void;
+ readonly onError?: (error: unknown) => void;
}
type RpcUnaryMethod<TTag extends RpcTag> =
diff --git a/apps/web/src/wsTransport.ts b/apps/web/src/wsTransport.ts
--- a/apps/web/src/wsTransport.ts
+++ b/apps/web/src/wsTransport.ts
@@ -21,6 +21,7 @@
interface SubscribeOptions {
readonly retryDelay?: Duration.Input;
readonly onResubscribe?: () => void;
+ readonly onError?: (error: unknown) => void;
}
interface RequestOptions {
@@ -147,6 +148,11 @@
console.warn("WebSocket RPC subscription disconnected", {
error: formatErrorMessage(error),
});
+ try {
+ options?.onError?.(error);
+ } catch {
+ // Swallow onError callback errors so the retry loop continues.
+ }
await sleep(retryDelayMs);
}
}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit e234ed0. Configure here.
| markGitStatusPending(cwd); | ||
| }, | ||
| }, | ||
| ); |
Contributor
There was a problem hiding this comment.
Git status errors silently swallowed, never surfaced to UI
Medium Severity
The subscribeToGitStatus function only updates the atom's data field on stream events but never populates error or cause. In GitActionsControl.tsx, gitStatusError is destructured from useGitStatus and used at lines 904–905 to conditionally render an error message — but since error is always null, that UI path is dead. Any git status stream errors are silently lost, a regression from the previous useQuery-based approach which did surface errors.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e234ed0. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
git statusqueries to shared live git status state, including branch checkout handling for remote branches.Testing
Note
High Risk
High risk because it changes the Git RPC/IPC contract (replacing
git.statuswith streamingsubscribeGitStatus+ unarygit.refreshStatus) and rewires server/client status flow, which can break existing consumers and introduce concurrency/caching edge cases.Overview
Replaces polling-based git status reads with a WebSocket-driven status stream. The server introduces
GitStatusBroadcasterto cache per-repo local/remote status, publishGitStatusStreamEventupdates, and run a remote refresh loop while subscribers are connected.Git workflows now trigger background status refreshes after completion (e.g.
pull,runStackedAction, worktree/branch operations), andGitManager/GitCoreare refactored to expose split local vs remote status APIs pluscheckoutBranchreturning the actual checked-out branch.On the web client, components migrate from react-query
gitStatusqueries to a shared atom-backeduseGitStatusstate that maintains one subscription percwdand debounces refresh requests; contracts/tests are updated accordingly (new hosting provider metadata, new RPC methods, and removal of the oldgit.statuscall).Reviewed by Cursor Bugbot for commit e234ed0. 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 with a streamingsubscribeGitStatusWebSocket subscription that emitssnapshot,localUpdated, andremoteUpdatedevents, plus a separategitRefreshStatusRPC for on-demand refresh.GitStatusBroadcasteron the server, which maintains per-repo caches for local and remote status, polls remote every 30 seconds while subscribers are active, and publishes change events via PubSub.gitStatusState.tson the client with ref-counted shared subscriptions per cwd, atom-based state, debounced refresh coalescing, and auseGitStatusReact hook consumed byChatView,DiffPanel,Sidebar,GitActionsControl, andBranchToolbarBranchSelector.GitStatusResultintoGitStatusLocalResultandGitStatusRemoteResultin the contracts package and addsGitStatusStreamEvent,GitCheckoutResult, andGitHostingProviderschemas.gitStatusQueryOptionsandinvalidateGitStatusQueryfrom react-query helpers; mutation callbacks now only invalidate branch queries rather than all git queries.NativeApi.git.statusis replaced byrefreshStatusandonStatus; any callers outside this PR relying on the old interface will break.Macroscope summarized e234ed0.