Skip to content

Commit bc31111

Browse files
juliusmarmingeclaude
juliusmarminge
and
claude
committed
fix(server): branch status reports when a checkout stops being a repository
Main's sidebar rows subscribe to the lightweight branch stream, which only carried refName, so a removed worktree looked like a detached HEAD and the sidebar could not tell it was gone. The branch stream now carries isRepo (optional, so older servers decode as unknown and keep stored labels), and removing a worktree refreshes the removed path as well as the repository root so its subscribers hear about it. AI-assisted; verified with the tests and tools listed above. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent a8a91cc commit bc31111

7 files changed

Lines changed: 70 additions & 33 deletions

File tree

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

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import * as TestClock from "effect/testing/TestClock";
1919
import * as Tracer from "effect/Tracer";
2020
import type {
2121
BackgroundScope,
22+
VcsBranchStatus,
2223
VcsStatusLocalResult,
2324
VcsStatusRemoteResult,
2425
VcsStatusStreamEvent,
@@ -106,6 +107,7 @@ function layerTestFor(state: {
106107
Effect.as(state.currentLocalStatus),
107108
);
108109
}),
110+
repositoryRoot: () => Effect.sync(() => (state.currentLocalStatus.isRepo ? "/repo" : null)),
109111
currentBranch: () =>
110112
state.branchRead
111113
? state.branchRead()
@@ -233,14 +235,17 @@ describe("VcsStatusBroadcaster", () => {
233235
return Effect.gen(function* () {
234236
const broadcaster = yield* VcsStatusBroadcaster.VcsStatusBroadcaster;
235237
const branches = yield* Stream.toPull(broadcaster.streamBranchStatus({ cwd: "/repo" }));
236-
assert.deepEqual(yield* branches, [{ refName: baseLocalStatus.refName }]);
238+
assert.deepEqual(yield* branches, [{ refName: baseLocalStatus.refName, isRepo: true }]);
237239
assert.equal(state.localStatusCalls, 0);
238240
state.currentLocalStatus = { ...baseLocalStatus, refName: "external" };
239241
yield* broadcaster.refreshBranchStatus({ cwd: "/repo" });
240-
assert.deepEqual(yield* branches, [{ refName: "external" }]);
242+
assert.deepEqual(yield* branches, [{ refName: "external", isRepo: true }]);
241243
state.currentLocalStatus = { ...baseLocalStatus, refName: null };
242244
yield* broadcaster.refreshBranchStatus({ cwd: "/repo" });
243-
assert.deepEqual(yield* branches, [{ refName: null }]);
245+
assert.deepEqual(yield* branches, [{ refName: null, isRepo: true }]);
246+
state.currentLocalStatus = { ...baseLocalStatus, isRepo: false, refName: null };
247+
yield* broadcaster.refreshBranchStatus({ cwd: "/repo" });
248+
assert.deepEqual(yield* branches, [{ refName: null, isRepo: false }]);
244249
assert.equal(state.localStatusCalls, 0);
245250
const detailScope = yield* Scope.make();
246251
const details = yield* Stream.toPull(
@@ -265,7 +270,7 @@ describe("VcsStatusBroadcaster", () => {
265270
assert.deepEqual(yield* details, [
266271
{ _tag: "localUpdated", local: state.currentLocalStatus },
267272
]);
268-
assert.deepEqual(yield* branches, [{ refName: "changed" }]);
273+
assert.deepEqual(yield* branches, [{ refName: "changed", isRepo: true }]);
269274
const secondScope = yield* Scope.make();
270275
const secondDetails = yield* Stream.toPull(
271276
broadcaster.streamStatus({ cwd: "/alias", includeRemote: false }),
@@ -303,7 +308,7 @@ describe("VcsStatusBroadcaster", () => {
303308
assert.deepEqual(yield* reopened, [
304309
{ _tag: "localUpdated", local: state.currentLocalStatus },
305310
]);
306-
assert.deepEqual(yield* branches, [{ refName: null }]);
311+
assert.deepEqual(yield* branches, [{ refName: null, isRepo: false }]);
307312
assert.equal(state.remoteStatusCalls, 0);
308313
}).pipe(Effect.provide(layerTestFor(state)), Effect.scoped);
309314
},
@@ -1439,7 +1444,10 @@ it.effect("publishes overlapping branch reads in checkout order", () => {
14391444
yield* Deferred.succeed(resume, undefined);
14401445
yield* Fiber.join(older);
14411446
yield* Fiber.join(newer);
1442-
assert.deepEqual(yield* branches, [{ refName: "old" }, { refName: "new" }]);
1447+
assert.deepEqual(yield* branches, [
1448+
{ refName: "old", isRepo: true },
1449+
{ refName: "new", isRepo: true },
1450+
]);
14431451
}).pipe(Effect.provide(layerTestFor(state)), Effect.scoped);
14441452
});
14451453

@@ -1449,7 +1457,7 @@ it.effect("does not replay older queued branches after a new subscriber snapshot
14491457
const b = yield* VcsStatusBroadcaster.VcsStatusBroadcaster;
14501458
const entered = yield* Deferred.make<void>();
14511459
const resume = yield* Deferred.make<void>();
1452-
const values = yield* Queue.unbounded<{ readonly refName: string | null }>();
1460+
const values = yield* Queue.unbounded<VcsBranchStatus>();
14531461
let reads = 0;
14541462
state.branchRead = () =>
14551463
++reads === 1
@@ -1468,10 +1476,10 @@ it.effect("does not replay older queued branches after a new subscriber snapshot
14681476
);
14691477
yield* Deferred.succeed(resume, undefined);
14701478
yield* Fiber.join(older);
1471-
assert.deepEqual(yield* Queue.take(values), { refName: "new" });
1479+
assert.deepEqual(yield* Queue.take(values), { refName: "new", isRepo: true });
14721480
state.branchRead = () => Effect.succeed("latest");
14731481
yield* b.refreshBranchStatus({ cwd: "/repo" });
1474-
assert.deepEqual(yield* Queue.take(values), { refName: "latest" });
1482+
assert.deepEqual(yield* Queue.take(values), { refName: "latest", isRepo: true });
14751483
yield* Fiber.join(subscriber);
14761484
}).pipe(Effect.provide(layerTestFor(state)), Effect.scoped);
14771485
});

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

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,7 @@ export const make = Effect.gen(function* () {
260260
PubSub.unbounded<{
261261
readonly cwd: string;
262262
readonly refName: string | null;
263+
readonly isRepo: boolean;
263264
readonly revision: number;
264265
}>(),
265266
(pubsub) => PubSub.shutdown(pubsub),
@@ -270,17 +271,18 @@ export const make = Effect.gen(function* () {
270271
const detailedSubscribers = new Map<string, number>();
271272
const branches = new Map<
272273
string,
273-
{ readonly refName: string | null; readonly revision: number }
274+
{ readonly refName: string | null; readonly isRepo: boolean; readonly revision: number }
274275
>();
275276
let branchRevision = 0;
276277
const publishBranch = Effect.fn("VcsStatusBroadcaster.publishBranch")(function* (
277278
cwd: string,
278-
refName: string | null,
279+
{ refName, isRepo }: { readonly refName: string | null; readonly isRepo: boolean },
279280
) {
280-
if (branches.get(cwd)?.refName === refName) return;
281+
const previous = branches.get(cwd);
282+
if (previous?.refName === refName && previous.isRepo === isRepo) return;
281283
const revision = ++branchRevision;
282-
branches.set(cwd, { refName, revision });
283-
yield* PubSub.publish(branchChangesPubSub, { cwd, refName, revision });
284+
branches.set(cwd, { refName, isRepo, revision });
285+
yield* PubSub.publish(branchChangesPubSub, { cwd, refName, isRepo, revision });
284286
});
285287
const localRefreshLocks = yield* KeyedLock.make<string>();
286288
const cacheRef = yield* Ref.make(new Map<string, CachedVcsStatus>());
@@ -309,7 +311,7 @@ export const make = Effect.gen(function* () {
309311
new Map(cache).set(cwd, { local: nextLocal, remote }),
310312
] as const;
311313
});
312-
yield* publishBranch(cwd, local.refName);
314+
yield* publishBranch(cwd, local);
313315
if (options?.publish && remoteCleared) {
314316
yield* PubSub.publish(changesPubSub, {
315317
cwd,
@@ -801,9 +803,15 @@ export const make = Effect.gen(function* () {
801803
Effect.gen(function* () {
802804
if (detailedSubscribers.has(cwd)) {
803805
const local = yield* getOrLoadLocalStatusUnlocked(cwd);
804-
return { refName: local.refName, revision: branches.get(cwd)?.revision ?? 0 };
806+
return {
807+
refName: local.refName,
808+
isRepo: local.isRepo,
809+
revision: branches.get(cwd)?.revision ?? 0,
810+
};
805811
}
806812
const refName = yield* workflow.currentBranch(cwd);
813+
// A null branch is either a detached HEAD or a checkout that is no longer a repository.
814+
const isRepo = refName !== null || (yield* workflow.repositoryRoot(cwd)) !== null;
807815
// HEAD cannot certify working-tree totals, even when it has not moved.
808816
localRefreshResults.delete(cwd);
809817
yield* workflow.invalidateLocalStatus(cwd);
@@ -814,21 +822,25 @@ export const make = Effect.gen(function* () {
814822
if (detailedSubscribers.has(cwd)) {
815823
const local = yield* workflow.localStatus({ cwd });
816824
yield* updateCachedLocalStatus(cwd, local, { publish: true });
817-
return { refName: local.refName, revision: branches.get(cwd)?.revision ?? 0 };
825+
return {
826+
refName: local.refName,
827+
isRepo: local.isRepo,
828+
revision: branches.get(cwd)?.revision ?? 0,
829+
};
818830
}
819-
yield* publishBranch(cwd, refName);
820-
return { refName, revision: branches.get(cwd)?.revision ?? 0 };
831+
yield* publishBranch(cwd, { refName, isRepo });
832+
return { refName, isRepo, revision: branches.get(cwd)?.revision ?? 0 };
821833
}),
822834
),
823835
);
824836

825837
const refreshBranchStatus: VcsStatusBroadcaster["Service"]["refreshBranchStatus"] = Effect.fn(
826838
"VcsStatusBroadcaster.refreshBranchStatus",
827839
)(function* (input) {
828-
const { refName } = yield* refreshBranchStatusCore(
840+
const { refName, isRepo } = yield* refreshBranchStatusCore(
829841
yield* withFileSystem(normalizeCwd(input.cwd)),
830842
);
831-
return { refName };
843+
return { refName, isRepo };
832844
});
833845

834846
const streamBranchStatus: VcsStatusBroadcaster["Service"]["streamBranchStatus"] = (input) =>
@@ -838,12 +850,16 @@ export const make = Effect.gen(function* () {
838850
const subscription = yield* PubSub.subscribe(branchChangesPubSub);
839851
const branch = yield* refreshBranchStatusCore(cwd);
840852
return Stream.concat(
841-
Stream.make({ refName: branch.refName }),
853+
Stream.make({ refName: branch.refName, isRepo: branch.isRepo }),
842854
Stream.fromSubscription(subscription).pipe(
843855
Stream.filter((event) => event.cwd === cwd && event.revision > branch.revision),
844-
Stream.map(({ refName }) => ({ refName })),
856+
Stream.map(({ refName, isRepo }) => ({ refName, isRepo })),
845857
),
846-
).pipe(Stream.changesWith((left, right) => left.refName === right.refName));
858+
).pipe(
859+
Stream.changesWith(
860+
(left, right) => left.refName === right.refName && left.isRepo === right.isRepo,
861+
),
862+
);
847863
}),
848864
);
849865

‎apps/server/src/ws.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3210,7 +3210,13 @@ export const layerWsRpc = (
32103210
[WS_METHODS.vcsCreateWorktree]: (input) =>
32113211
gitWorkflow.createWorktree(input).pipe(Effect.tap(() => refreshGitStatus(input.cwd))),
32123212
[WS_METHODS.vcsRemoveWorktree]: (input) =>
3213-
gitWorkflow.removeWorktree(input).pipe(Effect.tap(() => refreshGitStatus(input.cwd))),
3213+
gitWorkflow
3214+
.removeWorktree(input)
3215+
.pipe(
3216+
Effect.tap(() =>
3217+
Effect.all([refreshGitStatus(input.cwd), refreshGitStatus(input.path)]),
3218+
),
3219+
),
32143220
[WS_METHODS.vcsCreateRef]: (input) =>
32153221
gitWorkflow.createRef(input).pipe(Effect.tap(() => refreshGitStatus(input.cwd))),
32163222
[WS_METHODS.vcsSwitchRef]: (input) =>

‎apps/web/src/components/Sidebar.logic.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ export function useRetainedValue<T>(key: string | null, value: T | null): T | nu
168168
// A known non-repository cwd means the stored checkout was removed; unknown status keeps it.
169169
export function withoutRemovedCheckout<
170170
T extends { branch: string | null; worktreePath: string | null },
171-
>(thread: T, gitStatus: { readonly isRepo: boolean } | null): T {
171+
>(thread: T, gitStatus: { readonly isRepo?: boolean | undefined } | null): T {
172172
return gitStatus?.isRepo === false ? { ...thread, branch: null, worktreePath: null } : thread;
173173
}
174174

‎packages/client-runtime/src/rpc/client.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,10 @@ export const subscribeVcsBranch = (
476476
includeRemote: false,
477477
}).pipe(
478478
Stream.filter((event) => event._tag !== "remoteUpdated"),
479-
Stream.map((event) => ({ refName: event.local.refName })),
479+
Stream.map((event) => ({
480+
refName: event.local.refName,
481+
isRepo: event.local.isRepo,
482+
})),
480483
),
481484
),
482485
),

‎packages/client-runtime/src/state/vcs.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ describe("branch subscriptions", () => {
140140
return Stream.concat(
141141
Stream.make(
142142
branchSupported
143-
? { refName }
143+
? { refName, isRepo: true }
144144
: { _tag: "snapshot", local: status({ refName }), remote: null },
145145
),
146146
Stream.never,
@@ -183,19 +183,19 @@ describe("branch subscriptions", () => {
183183
const first = yield* Stream.toPull(
184184
AtomRegistry.toStreamResult(registry, atoms.branch(target)),
185185
);
186-
expect(yield* first).toEqual([{ refName: "main" }]);
186+
expect(yield* first).toEqual([{ refName: "main", isRepo: true }]);
187187
const second = yield* Stream.toPull(
188188
AtomRegistry.toStreamResult(registry, atoms.branch(target)),
189189
);
190-
expect(yield* second).toEqual([{ refName: "main" }]);
190+
expect(yield* second).toEqual([{ refName: "main", isRepo: true }]);
191191
expect(subscriptions).toBe(1);
192192
yield* SubscriptionRef.set(
193193
activeSession,
194194
Option.some(branchSession("external", !supported)),
195195
);
196196
expect(yield* Queue.take(released)).toBe("main");
197-
expect(yield* first).toEqual([{ refName: "external" }]);
198-
expect(yield* second).toEqual([{ refName: "external" }]);
197+
expect(yield* first).toEqual([{ refName: "external", isRepo: true }]);
198+
expect(yield* second).toEqual([{ refName: "external", isRepo: true }]);
199199
registry.dispose();
200200
expect(yield* Queue.take(released)).toBe("external");
201201
}).pipe(Effect.scoped),

‎packages/contracts/src/git.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,11 @@ export const VcsStatusInput = Schema.Struct({
111111
});
112112
export type VcsStatusInput = typeof VcsStatusInput.Type;
113113

114-
export const VcsBranchStatus = Schema.Struct({ refName: Schema.NullOr(Schema.String) });
114+
export const VcsBranchStatus = Schema.Struct({
115+
refName: Schema.NullOr(Schema.String),
116+
/** Absent from older servers, where a removed checkout is indistinguishable from detached HEAD. */
117+
isRepo: Schema.optional(Schema.Boolean),
118+
});
115119
export type VcsBranchStatus = typeof VcsBranchStatus.Type;
116120

117121
export const VcsStatusSubscriptionInput = Schema.Struct({

0 commit comments

Comments
 (0)