Skip to content

Commit 5055de3

Browse files
maria-rcksmaria-rcks
authored
fix(vcs): passive sidebar rows stop retaining remote pollers (#15666)
1 parent 876f865 commit 5055de3

11 files changed

Lines changed: 213 additions & 29 deletions

File tree

‎apps/mobile/src/connection/background-activity-scopes.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ function scopeForSubscription(
4545
if (observation.method !== WS_METHODS.subscribeVcsStatus) {
4646
return null;
4747
}
48-
const input = observation.input as { readonly cwd?: unknown };
48+
const input = observation.input as { readonly cwd?: unknown; readonly includeRemote?: unknown };
49+
if (input.includeRemote === false) return null;
4950
return typeof input.cwd === "string" ? { type: "vcs-status", cwd: input.cwd } : null;
5051
}
5152

‎apps/mobile/src/connection/background-activity.test.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ describe("mobile background activity", () => {
1212
it.effect("retains VCS demand only while the mobile subscription is active", () =>
1313
Effect.gen(function* () {
1414
const environmentId = EnvironmentId.make("mobile-environment");
15+
const releasePassive = yield* observeMobileBackgroundActivitySubscription({
16+
environmentId,
17+
method: WS_METHODS.subscribeVcsStatus,
18+
input: { cwd: "/workspace", includeRemote: false },
19+
});
20+
expect(retainedMobileBackgroundScopes(environmentId)).toEqual([]);
1521
const release = yield* observeMobileBackgroundActivitySubscription({
1622
environmentId,
1723
method: WS_METHODS.subscribeVcsStatus,
@@ -22,6 +28,11 @@ describe("mobile background activity", () => {
2228
{ type: "vcs-status", cwd: "/workspace" },
2329
]);
2430

31+
yield* releasePassive;
32+
expect(retainedMobileBackgroundScopes(environmentId)).toEqual([
33+
{ type: "vcs-status", cwd: "/workspace" },
34+
]);
35+
2536
yield* release;
2637
expect(retainedMobileBackgroundScopes(environmentId)).toEqual([]);
2738
}),

‎apps/server/src/git/GitManager.test.ts‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ import * as NodeChildProcess from "node:child_process";
66
import * as NodeServices from "@effect/platform-node/NodeServices";
77
import { it } from "@effect/vitest";
88
import * as Duration from "effect/Duration";
9+
import * as Context from "effect/Context";
910
import * as Effect from "effect/Effect";
11+
import * as Deferred from "effect/Deferred";
12+
import * as Exit from "effect/Exit";
1013
import * as FileSystem from "effect/FileSystem";
1114
import * as Layer from "effect/Layer";
1215
import * as Logger from "effect/Logger";
@@ -16,11 +19,13 @@ import * as References from "effect/References";
1619
import * as Result from "effect/Result";
1720
import * as Schema from "effect/Schema";
1821
import * as Scope from "effect/Scope";
22+
import * as Stream from "effect/Stream";
1923
import { TestClock } from "effect/testing";
2024
import { ChildProcessSpawner } from "effect/process";
2125
import { expect } from "vite-plus/test";
2226
import type {
2327
GitActionProgressEvent,
28+
GitManagerServiceError,
2429
GitPreparePullRequestThreadInput,
2530
ModelSelection,
2631
} from "@t3tools/contracts";
@@ -43,6 +48,11 @@ import * as GitLabCli from "../sourceControl/GitLabCli.ts";
4348
import * as TextGeneration from "../textGeneration/TextGeneration.ts";
4449
import * as GitVcsDriver from "../vcs/GitVcsDriver.ts";
4550
import * as VcsProcess from "../vcs/VcsProcess.ts";
51+
import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts";
52+
import * as VcsProjectConfig from "../vcs/VcsProjectConfig.ts";
53+
import * as VcsStatusBroadcaster from "../vcs/VcsStatusBroadcaster.ts";
54+
import * as BackgroundPolicy from "../background/BackgroundPolicy.ts";
55+
import * as GitWorkflowService from "./GitWorkflowService.ts";
4656
import * as GitHubSourceControlProvider from "../sourceControl/GitHubSourceControlProvider.ts";
4757
import * as GitLabSourceControlProvider from "../sourceControl/GitLabSourceControlProvider.ts";
4858
import {
@@ -778,6 +788,120 @@ const layerGitManagerTest = GitVcsDriver.layer.pipe(
778788
);
779789

780790
it.layer(layerGitManagerTest)("GitManager", (it) => {
791+
it.effect("passive worktree status streams do not start remote refreshes", () =>
792+
Effect.gen(function* () {
793+
const repoDir = yield* makeTempDir("t3code-passive-vcs-");
794+
yield* initRepo(repoDir);
795+
const remoteDir = yield* createBareRemote();
796+
yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]);
797+
yield* runGit(repoDir, ["push", "-u", "origin", "main"]);
798+
const worktreeDir = NodePath.join(repoDir, "worktree");
799+
yield* runGit(repoDir, ["worktree", "add", "-b", "feature/passive", worktreeDir]);
800+
yield* runGit(worktreeDir, ["push", "-u", "origin", "feature/passive"]);
801+
802+
const { manager } = yield* makeManager();
803+
let remoteReads = 0;
804+
const workflowContext = yield* Layer.build(
805+
GitWorkflowService.layer.pipe(
806+
Layer.provide(
807+
Layer.succeed(GitManager.GitManager, {
808+
...manager,
809+
remoteStatus: (input, options) =>
810+
manager.remoteStatus(input, options).pipe(
811+
Effect.tap(() =>
812+
Effect.sync(() => {
813+
remoteReads += 1;
814+
}),
815+
),
816+
),
817+
}),
818+
),
819+
Layer.provide(
820+
VcsDriverRegistry.layer.pipe(
821+
Layer.provide(VcsProjectConfig.layer),
822+
Layer.provide(VcsProcess.layer),
823+
),
824+
),
825+
),
826+
);
827+
const broadcasterContext = yield* Layer.build(
828+
VcsStatusBroadcaster.layer.pipe(
829+
Layer.provide(
830+
Layer.succeed(
831+
GitWorkflowService.GitWorkflowService,
832+
Context.get(workflowContext, GitWorkflowService.GitWorkflowService),
833+
),
834+
),
835+
Layer.provide(
836+
Layer.mock(BackgroundPolicy.BackgroundPolicy)({
837+
hasDemand: () => Effect.succeed(true),
838+
shouldRunScopeWork: () => Effect.succeed(true),
839+
}),
840+
),
841+
),
842+
);
843+
const broadcaster = Context.get(
844+
broadcasterContext,
845+
VcsStatusBroadcaster.VcsStatusBroadcaster,
846+
);
847+
const passiveScope = yield* Scope.make();
848+
const snapshots = yield* Deferred.make<void, GitManagerServiceError>();
849+
const localUpdated = yield* Deferred.make<void>();
850+
const remoteUpdated = yield* Deferred.make<void>();
851+
let snapshotCount = 0;
852+
for (const cwd of [repoDir, worktreeDir]) {
853+
yield* Stream.runForEach(
854+
broadcaster.streamStatus(
855+
{ cwd, includeRemote: false },
856+
{ automaticRemoteRefreshInterval: Effect.succeed(Duration.seconds(1)) },
857+
),
858+
(event) => {
859+
if (
860+
cwd === worktreeDir &&
861+
event._tag === "localUpdated" &&
862+
event.local.hasWorkingTreeChanges
863+
) {
864+
return Deferred.succeed(localUpdated, undefined);
865+
}
866+
if (cwd === worktreeDir && event._tag === "remoteUpdated") {
867+
expect(event.remote?.hasUpstream).toBe(true);
868+
return Deferred.succeed(remoteUpdated, undefined);
869+
}
870+
if (event._tag !== "snapshot") return Effect.void;
871+
expect(event.local.isRepo).toBe(true);
872+
expect(event.local.refName).toBe(cwd === repoDir ? "main" : "feature/passive");
873+
snapshotCount += 1;
874+
return snapshotCount === 2 ? Deferred.succeed(snapshots, undefined) : Effect.void;
875+
},
876+
).pipe(
877+
Effect.catchCause((cause) => Deferred.failCause(snapshots, cause)),
878+
Effect.forkIn(passiveScope),
879+
);
880+
}
881+
yield* Deferred.await(snapshots);
882+
expect(remoteReads).toBe(0);
883+
yield* TestClock.adjust("1 minute");
884+
expect(remoteReads).toBe(0);
885+
886+
const fs = yield* FileSystem.FileSystem;
887+
yield* fs.writeFileString(NodePath.join(worktreeDir, "README.md"), "changed\n");
888+
yield* broadcaster.refreshLocalStatus(worktreeDir);
889+
yield* Deferred.await(localUpdated);
890+
expect(remoteReads).toBe(0);
891+
892+
const activeScope = yield* Scope.make();
893+
yield* Stream.runDrain(broadcaster.streamStatus({ cwd: worktreeDir })).pipe(
894+
Effect.forkIn(activeScope),
895+
);
896+
yield* Deferred.await(remoteUpdated);
897+
expect(remoteReads).toBe(1);
898+
yield* Scope.close(activeScope, Exit.void);
899+
yield* TestClock.adjust("1 minute");
900+
expect(remoteReads).toBe(1);
901+
yield* Scope.close(passiveScope, Exit.void);
902+
}),
903+
);
904+
781905
it.effect("status includes draft PR metadata when branch already has a draft PR", () =>
782906
Effect.gen(function* () {
783907
const repoDir = yield* makeTempDir("t3code-git-manager-");

‎apps/server/src/vcs/VcsStatusBroadcaster.test.ts‎

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,7 @@ describe("VcsStatusBroadcaster", () => {
594594
},
595595
);
596596

597-
it.effect("streams a local snapshot first and remote updates later", () => {
597+
it.effect("passive streams retain cached remote status", () => {
598598
const state = {
599599
currentLocalStatus: baseLocalStatus,
600600
currentRemoteStatus: baseRemoteStatus,
@@ -608,18 +608,25 @@ describe("VcsStatusBroadcaster", () => {
608608
const broadcaster = yield* VcsStatusBroadcaster.VcsStatusBroadcaster;
609609
const snapshotDeferred = yield* Deferred.make<VcsStatusStreamEvent>();
610610
const remoteUpdatedDeferred = yield* Deferred.make<VcsStatusStreamEvent>();
611-
yield* Stream.runForEach(broadcaster.streamStatus({ cwd: "/repo" }), (event) => {
612-
if (event._tag === "snapshot") {
613-
return Deferred.succeed(snapshotDeferred, event).pipe(Effect.ignore);
614-
}
615-
if (event._tag === "remoteUpdated") {
616-
return Deferred.succeed(remoteUpdatedDeferred, event).pipe(Effect.ignore);
617-
}
618-
return Effect.void;
619-
}).pipe(Effect.forkScoped);
611+
yield* Stream.runForEach(
612+
broadcaster.streamStatus({ cwd: "/repo", includeRemote: false }),
613+
(event) => {
614+
if (event._tag === "snapshot") {
615+
return Deferred.succeed(snapshotDeferred, event).pipe(Effect.ignore);
616+
}
617+
if (event._tag === "remoteUpdated") {
618+
return Deferred.succeed(remoteUpdatedDeferred, event).pipe(Effect.ignore);
619+
}
620+
return Effect.void;
621+
},
622+
).pipe(Effect.forkScoped);
620623

621624
const snapshot = yield* Deferred.await(snapshotDeferred);
622-
yield* broadcaster.refreshStatus("/repo");
625+
yield* TestClock.adjust("1 minute");
626+
assert.equal(state.remoteStatusCalls, 0);
627+
yield* broadcaster.getStatus({ cwd: "/repo" });
628+
state.currentRemoteStatus = remoteStatusWithPr;
629+
yield* broadcaster.refreshPullRequestStatus("/repo");
623630
const remoteUpdated = yield* Deferred.await(remoteUpdatedDeferred);
624631

625632
assert.deepStrictEqual(snapshot, {
@@ -629,8 +636,16 @@ describe("VcsStatusBroadcaster", () => {
629636
} satisfies VcsStatusStreamEvent);
630637
assert.deepStrictEqual(remoteUpdated, {
631638
_tag: "remoteUpdated",
632-
remote: baseRemoteStatus,
639+
remote: remoteStatusWithPr,
633640
} satisfies VcsStatusStreamEvent);
641+
const cachedSnapshot = yield* Stream.runHead(
642+
broadcaster.streamStatus({ cwd: "/repo", includeRemote: false }),
643+
);
644+
assert.deepStrictEqual(Option.getOrThrow(cachedSnapshot), {
645+
_tag: "snapshot",
646+
local: baseLocalStatus,
647+
remote: remoteStatusWithPr,
648+
});
634649
}).pipe(Effect.provide(layerTestFor(state)));
635650
});
636651

@@ -1032,7 +1047,7 @@ describe("VcsStatusBroadcaster", () => {
10321047
}).pipe(Effect.provide(layerTest));
10331048
});
10341049

1035-
it.effect("stops the remote poller after the last stream subscriber disconnects", () => {
1050+
it.effect("releases remote demand while passive observers remain", () => {
10361051
const state = {
10371052
currentLocalStatus: baseLocalStatus,
10381053
currentRemoteStatus: baseRemoteStatus,
@@ -1092,6 +1107,16 @@ describe("VcsStatusBroadcaster", () => {
10921107
const secondSnapshot = yield* Deferred.make<VcsStatusStreamEvent>();
10931108
const firstScope = yield* Scope.make();
10941109
const secondScope = yield* Scope.make();
1110+
const passiveSnapshot = yield* Deferred.make<VcsStatusStreamEvent>();
1111+
yield* Stream.runForEach(
1112+
broadcaster.streamStatus({ cwd: "/repo", includeRemote: false }),
1113+
(event) =>
1114+
event._tag === "snapshot"
1115+
? Deferred.succeed(passiveSnapshot, event).pipe(Effect.ignore)
1116+
: Effect.void,
1117+
).pipe(Effect.forkScoped);
1118+
yield* Deferred.await(passiveSnapshot);
1119+
assert.equal(state.remoteStatusCalls, 0);
10951120
yield* Stream.runForEach(broadcaster.streamStatus({ cwd: "/repo" }), (event) =>
10961121
event._tag === "snapshot"
10971122
? Deferred.succeed(firstSnapshot, event).pipe(Effect.ignore)

‎apps/server/src/vcs/VcsStatusBroadcaster.ts‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import type {
1919
VcsStatusRemoteResult,
2020
VcsStatusResult,
2121
VcsStatusStreamEvent,
22+
VcsStatusSubscriptionInput,
2223
} from "@t3tools/contracts";
2324
import { mergeGitStatusParts } from "@t3tools/shared/git";
2425
import * as KeyedLock from "@t3tools/shared/KeyedLock";
@@ -201,7 +202,7 @@ export class VcsStatusBroadcaster extends Context.Service<
201202
cwd: string,
202203
) => Effect.Effect<VcsStatusRemoteResult | null, GitManagerServiceError>;
203204
readonly streamStatus: (
204-
input: VcsStatusInput,
205+
input: VcsStatusSubscriptionInput,
205206
options?: StreamStatusOptions,
206207
) => Stream.Stream<VcsStatusStreamEvent, GitManagerServiceError>;
207208
}
@@ -708,15 +709,19 @@ export const make = Effect.gen(function* () {
708709
const initialLocal = yield* getOrLoadLocalStatus(cwd);
709710
const cachedStatus = yield* getCachedStatus(cwd);
710711
const initialRemote = cachedStatus?.remote?.value ?? null;
711-
yield* retainRemotePoller(
712-
cwd,
713-
input.cwd,
714-
options?.automaticRemoteRefreshInterval ??
715-
Effect.succeed(DEFAULT_VCS_STATUS_REFRESH_INTERVAL),
716-
cachedStatus?.remote === null || cachedStatus?.remote === undefined,
717-
);
718-
719-
const release = releaseRemotePoller(cwd, input.cwd).pipe(Effect.ignore, Effect.asVoid);
712+
if (input.includeRemote !== false) {
713+
yield* retainRemotePoller(
714+
cwd,
715+
input.cwd,
716+
options?.automaticRemoteRefreshInterval ??
717+
Effect.succeed(DEFAULT_VCS_STATUS_REFRESH_INTERVAL),
718+
cachedStatus?.remote === null || cachedStatus?.remote === undefined,
719+
);
720+
}
721+
const release =
722+
input.includeRemote === false
723+
? Effect.void
724+
: releaseRemotePoller(cwd, input.cwd).pipe(Effect.ignore, Effect.asVoid);
720725

721726
return Stream.concat(
722727
Stream.make({

‎apps/web/src/components/Sidebar.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,7 +1209,7 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: {
12091209
leaseLiveStatus && (thread.branch != null || thread.worktreePath !== null) && gitCwd !== null
12101210
? vcsEnvironment.status({
12111211
environmentId: thread.environmentId,
1212-
input: { cwd: gitCwd },
1212+
input: { cwd: gitCwd, includeRemote: false },
12131213
})
12141214
: null,
12151215
);
@@ -2204,7 +2204,7 @@ const SidebarSearchResultRow = memo(function SidebarSearchResultRow(props: {
22042204
leaseLiveStatus && (thread.branch != null || thread.worktreePath !== null) && gitCwd !== null
22052205
? vcsEnvironment.status({
22062206
environmentId: thread.environmentId,
2207-
input: { cwd: gitCwd },
2207+
input: { cwd: gitCwd, includeRemote: false },
22082208
})
22092209
: null,
22102210
);

‎apps/web/src/components/ThreadStatusIndicators.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -910,7 +910,7 @@ export function ThreadRowLeadingStatus({
910910
gitCwd !== null
911911
? vcsEnvironment.status({
912912
environmentId: thread.environmentId,
913-
input: { cwd: gitCwd },
913+
input: { cwd: gitCwd, includeRemote: false },
914914
})
915915
: null,
916916
);

‎apps/web/src/lib/backgroundActivityReporter.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ describe("wasRecentlyInteracted", () => {
2222
Effect.gen(function* () {
2323
const environmentId = EnvironmentId.make("environment-observation-test");
2424
const scope = { type: "vcs-status" as const, cwd: "/repo" };
25+
const releasePassive = yield* observeBackgroundActivitySubscription({
26+
environmentId,
27+
method: WS_METHODS.subscribeVcsStatus,
28+
input: { cwd: scope.cwd, includeRemote: false },
29+
});
30+
expect(retainedBackgroundScopes(environmentId)).toEqual([]);
2531
const release = yield* observeBackgroundActivitySubscription({
2632
environmentId,
2733
method: WS_METHODS.subscribeVcsStatus,
@@ -30,6 +36,9 @@ describe("wasRecentlyInteracted", () => {
3036

3137
expect(retainedBackgroundScopes(environmentId)).toEqual([scope]);
3238

39+
yield* releasePassive;
40+
expect(retainedBackgroundScopes(environmentId)).toEqual([scope]);
41+
3342
yield* release;
3443
expect(retainedBackgroundScopes(environmentId)).toEqual([]);
3544
}),

‎apps/web/src/lib/backgroundActivityReporter.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,8 @@ function scopeForSubscription(
118118
if (observation.method !== WS_METHODS.subscribeVcsStatus) {
119119
return null;
120120
}
121-
const input = observation.input as { readonly cwd?: unknown };
121+
const input = observation.input as { readonly cwd?: unknown; readonly includeRemote?: unknown };
122+
if (input.includeRemote === false) return null;
122123
return typeof input.cwd === "string" ? { type: "vcs-status", cwd: input.cwd } : null;
123124
}
124125

0 commit comments

Comments
 (0)