Skip to content

Commit 1ff427b

Browse files
committed
fix(host-service): stop rescan() from desyncing interest on a transient gap
rescan()'s cleanup dropped this.interest entries for any workspace missing from its non-archived scan — but a workspace can be transiently absent without any client losing interest in it (e.g. archived-then-restored via the tombstone delete flow). A client that already sent one git:watch has no reason to send it again, and won't unless it remounts or its socket reconnects. Once interest was wiped, the retry loop (which only walks interest.keys()) could never bring the workspace back on its own — the watcher stayed unattached until an unrelated remount/reconnect, silently. interest is now touched exclusively by watchWorkspace()/unwatchWorkspace(). rescan() still tears down the live watcher (the real resource) for a gone workspace via stopWatching(), but leaves interest alone — a stale entry for a workspace gone for good costs one Map entry, bounded by the per-client git:watch cap, until the holding client unwatches or its socket closes. New regression test: hold interest, archive the workspace (rescan tears down the watcher, interest survives), restore it (rescan re-attaches via the retry loop, no second watchGit needed). Confirmed failing against the prior code, passing after. Found via a second-pass review of #6848. Claude-Session: https://claude.ai/code/session_01NmLFihnhebmL9bbbojGYCR
1 parent 88fe5a3 commit 1ff427b

2 files changed

Lines changed: 58 additions & 5 deletions

File tree

packages/host-service/src/events/git-watcher.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -572,14 +572,26 @@ export class GitWatcher {
572572
// debounce timer/batch for the removed workspace can't fire a stale
573573
// git:changed later — it also clears those, this loop used to leave
574574
// them dangling.
575+
//
576+
// Deliberately does NOT touch `interest` here. A workspace can be
577+
// transiently absent from this scan without any client ever losing
578+
// interest in it (e.g. archived-then-restored via the tombstone
579+
// delete flow) — a client that already sent one `git:watch` has no
580+
// reason to send it again, and won't unless it remounts or the
581+
// socket reconnects. Deleting `interest` for a merely-transient gap
582+
// would desync GitWatcher's bookkeeping from what the client (and
583+
// its socket's `gitSubscriptions`) still believes is watched, with
584+
// nothing left to ever resync it — the retry loop below only walks
585+
// `interest.keys()`, so a wrongly-cleared entry can never come back
586+
// on its own. `interest` is owned exclusively by
587+
// watchWorkspace()/unwatchWorkspace(); a stale entry for a
588+
// workspace gone for good costs one Map entry (bounded by the
589+
// per-client `git:watch` cap in event-bus.ts) until the holding
590+
// client unwatches or its socket closes — self-healing, unlike a
591+
// leaked live watcher.
575592
for (const id of [...this.watched.keys()]) {
576593
if (!existingIds.has(id)) this.stopWatching(id);
577594
}
578-
// Interest in a workspace that's gone is stale — drop it too, rather
579-
// than leaking a refcount nobody will ever unwatch.
580-
for (const id of this.interest.keys()) {
581-
if (!existingIds.has(id)) this.interest.delete(id);
582-
}
583595

584596
// Retry attaching for still-interested workspaces that never attached.
585597
for (const id of this.interest.keys()) {

packages/host-service/test/integration/git-watcher-lazy-registration.integration.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,47 @@ describe("GitWatcher lazy registration (regression coverage for #6729)", () => {
242242
expect(events).not.toContain(id);
243243
});
244244

245+
test("interest survives a workspace transiently disappearing from a rescan — self-heals without a new watchGit", async () => {
246+
const scenario = await createScenario(1);
247+
scenarios.push(scenario);
248+
scenario.gitWatcher.start();
249+
250+
const id = scenario.workspaceIds[0] as string;
251+
252+
// A single watchWorkspace() call, same as one client sending one
253+
// git:watch — nothing calls it again for the rest of this test.
254+
scenario.gitWatcher.watchWorkspace(id);
255+
await waitFor(() => internals(scenario.gitWatcher).watched.has(id), {
256+
timeoutMs: 10_000,
257+
});
258+
259+
// Simulate a transient disappearance (e.g. archive-then-restore via
260+
// the tombstone delete flow) spanning one rescan tick.
261+
scenario.host.db
262+
.update(workspaces)
263+
.set({ archivedAt: Date.now() })
264+
.where(eq(workspaces.id, id))
265+
.run();
266+
await internals(scenario.gitWatcher).rescan();
267+
268+
// The live watcher is torn down (real resource, correctly reclaimed)...
269+
expect(internals(scenario.gitWatcher).watched.has(id)).toBe(false);
270+
// ...but interest — which nothing but watchWorkspace/unwatchWorkspace
271+
// should ever touch — must survive the gap.
272+
expect(internals(scenario.gitWatcher).interest.has(id)).toBe(true);
273+
274+
// The workspace reappears (restored) before the next rescan tick.
275+
scenario.host.db
276+
.update(workspaces)
277+
.set({ archivedAt: null })
278+
.where(eq(workspaces.id, id))
279+
.run();
280+
await internals(scenario.gitWatcher).rescan();
281+
282+
// Self-healed via the retry loop, with no second watchWorkspace() call.
283+
expect(internals(scenario.gitWatcher).watched.has(id)).toBe(true);
284+
});
285+
245286
test("registration cost is paid only for workspaces someone actually watches, regardless of how many exist", async () => {
246287
const N = 30;
247288
const scenario = await createScenario(N);

0 commit comments

Comments
 (0)