Skip to content

Commit 60aff13

Browse files
TheGreatAxiosTheGreatAxios
authored
Merge pull request #1506 from corbitsdev/cl-10805-stop-the-five-second-git-spawn-poll-that-runs-for-the-whole
fix(tui): stop the idle git branch poll
2 parents b6d3f74 + 5cbf382 commit 60aff13

4 files changed

Lines changed: 249 additions & 70 deletions

File tree

‎src/tui/runner-host.test.ts‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,68 @@ async function withRunnerHost(
8484
}
8585
}
8686

87+
describe("mountRunnerHost branch label", () => {
88+
test("looks the branch up once, then only when a turn ends", async () => {
89+
const emitter = new EventEmitter();
90+
let branch = "main";
91+
let calls = 0;
92+
await withRunnerHost(
93+
async (host) => {
94+
await Promise.resolve();
95+
await Promise.resolve();
96+
expect(calls).toBe(1);
97+
expect(host.shell.workspace.branch).toBe("main");
98+
expect(ruleOf(host.shell.promptBottomRule)).toContain("(main)");
99+
100+
emitter.emit("event", { type: "inference.start" });
101+
emitter.emit("event", { type: "connector.reply" });
102+
await Promise.resolve();
103+
expect(calls).toBe(1);
104+
105+
branch = "topic";
106+
emitter.emit("event", { type: "inference.done" });
107+
await Promise.resolve();
108+
await Promise.resolve();
109+
expect(calls).toBe(2);
110+
expect(host.shell.workspace.branch).toBe("topic");
111+
expect(ruleOf(host.shell.promptBottomRule)).toContain("(topic)");
112+
},
113+
{
114+
eventEmitter: emitter,
115+
cwd: "/repo",
116+
fetchBranch: async () => {
117+
calls += 1;
118+
return branch;
119+
},
120+
},
121+
);
122+
});
123+
124+
test("a null git result after a known branch leaves the previous suffix", async () => {
125+
const emitter = new EventEmitter();
126+
let branch: string | null = "main";
127+
await withRunnerHost(
128+
async (host) => {
129+
await Promise.resolve();
130+
await Promise.resolve();
131+
expect(ruleOf(host.shell.promptBottomRule)).toContain("(main)");
132+
133+
branch = null;
134+
emitter.emit("event", { type: "inference.done" });
135+
await Promise.resolve();
136+
await Promise.resolve();
137+
expect(host.shell.workspace.branch).toBe("main");
138+
expect(ruleOf(host.shell.promptBottomRule)).toContain("(main)");
139+
},
140+
{
141+
eventEmitter: emitter,
142+
cwd: "/repo",
143+
fetchBranch: async () => branch,
144+
},
145+
);
146+
});
147+
});
148+
87149
function session(over: Partial<SubAgentSession>): SubAgentSession {
88150
return {
89151
id: "s1",

‎src/tui/runner/host.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -435,13 +435,19 @@ export async function mountRunnerHost(
435435
};
436436
deps.eventEmitter.on("session.clear", onSessionClear);
437437

438-
const stopBranchWatch = watchGitBranch({
438+
const branchWatch = watchGitBranch({
439439
cwd,
440440
onBranch: (branch) => setPromptWorkspace(host.shell, { branch }),
441441
...(deps.fetchBranch !== undefined
442442
? { fetchBranch: deps.fetchBranch }
443443
: {}),
444444
});
445+
// A turn can run a shell that checks out another branch. Idle time does not
446+
// fork git; the label may stay stale until the next finished turn.
447+
const onBranchEvent = (event: { type: string }): void => {
448+
if (onTurnBoundary(event)) branchWatch.refresh();
449+
};
450+
deps.eventEmitter.on("event", onBranchEvent);
445451

446452
// Quitting is Ctrl+C twice, the binding this interface has always used.
447453
// The host claims no key of its own: a second exit chord would split the
@@ -451,8 +457,9 @@ export async function mountRunnerHost(
451457
const dispose = (): void => {
452458
if (disposed) return;
453459
disposed = true;
454-
stopBranchWatch();
460+
branchWatch.stop();
455461
deps.eventEmitter.off("event", onCostEvent);
462+
deps.eventEmitter.off("event", onBranchEvent);
456463
deps.eventEmitter.off("session.clear", onSessionClear);
457464
unsubscribeChrome?.();
458465
clearShellExitHandler(host.shell);

‎src/tui/workspace-watch.test.ts‎

Lines changed: 129 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,100 +1,187 @@
1-
import { describe, expect, test } from "bun:test";
1+
import { describe, expect, jest, test } from "bun:test";
22

33
import { watchGitBranch } from "./workspace-watch";
44

5-
function fakeClock(): {
6-
readonly schedule: (tick: () => void, intervalMs: number) => () => void;
7-
readonly tick: () => void;
8-
} {
9-
let ticker: (() => void) | null = null;
10-
return {
11-
schedule: (fn) => {
12-
ticker = fn;
13-
return () => {
14-
ticker = null;
15-
};
16-
},
17-
tick: () => ticker?.(),
18-
};
5+
/** The retired idle poll. An idle window of several of these must not spawn git. */
6+
const FORMER_POLL_INTERVAL_MS = 5_000;
7+
8+
async function flush(): Promise<void> {
9+
await Promise.resolve();
10+
await Promise.resolve();
1911
}
2012

2113
describe("watchGitBranch", () => {
22-
test("reports the branch on the first lookup, before any tick", async () => {
14+
test("reports the branch on the first lookup", async () => {
2315
const seen: (string | null)[] = [];
24-
const clock = fakeClock();
25-
const stop = watchGitBranch({
16+
const watch = watchGitBranch({
2617
cwd: "/repo",
2718
onBranch: (b) => seen.push(b),
2819
fetchBranch: async () => "main",
29-
schedule: clock.schedule,
3020
});
31-
await Promise.resolve();
21+
await flush();
3222
expect(seen).toEqual(["main"]);
33-
stop();
23+
watch.stop();
3424
});
3525

36-
test("skips a tick while the previous lookup is still outstanding", async () => {
26+
test("one refresh during an outstanding lookup runs a single follow-up and reports the later branch", async () => {
3727
let calls = 0;
3828
const pending: ((branch: string | null) => void)[] = [];
39-
const clock = fakeClock();
40-
const stop = watchGitBranch({
29+
const seen: (string | null)[] = [];
30+
const watch = watchGitBranch({
4131
cwd: "/repo",
42-
onBranch: () => undefined,
32+
onBranch: (b) => seen.push(b),
4333
fetchBranch: () => {
4434
calls += 1;
4535
return new Promise((resolve) => {
4636
pending.push(resolve);
4737
});
4838
},
49-
schedule: clock.schedule,
5039
});
51-
clock.tick();
52-
clock.tick();
40+
watch.refresh();
41+
watch.refresh();
5342
expect(calls).toBe(1);
5443

5544
pending[0]?.("main");
56-
await Promise.resolve();
57-
clock.tick();
45+
await flush();
46+
expect(calls).toBe(2);
47+
expect(seen).toEqual(["main"]);
48+
49+
pending[1]?.("topic");
50+
await flush();
51+
expect(calls).toBe(2);
52+
expect(seen).toEqual(["main", "topic"]);
53+
watch.stop();
54+
});
55+
56+
test("a throwing onBranch does not stick the guard, so a later refresh looks up again", async () => {
57+
let calls = 0;
58+
const pending: ((branch: string | null) => void)[] = [];
59+
const watch = watchGitBranch({
60+
cwd: "/repo",
61+
onBranch: () => {
62+
throw new Error("paint failed");
63+
},
64+
fetchBranch: () => {
65+
calls += 1;
66+
return new Promise((resolve) => {
67+
pending.push(resolve);
68+
});
69+
},
70+
});
71+
pending[0]?.("main");
72+
await flush();
73+
expect(calls).toBe(1);
74+
75+
watch.refresh();
76+
expect(calls).toBe(2);
77+
watch.stop();
78+
});
79+
80+
test("a follow-up armed before onBranch throws still runs", async () => {
81+
let calls = 0;
82+
const pending: ((branch: string | null) => void)[] = [];
83+
const watch = watchGitBranch({
84+
cwd: "/repo",
85+
onBranch: () => {
86+
throw new Error("paint failed");
87+
},
88+
fetchBranch: () => {
89+
calls += 1;
90+
return new Promise((resolve) => {
91+
pending.push(resolve);
92+
});
93+
},
94+
});
95+
watch.refresh();
96+
expect(calls).toBe(1);
97+
pending[0]?.("main");
98+
await flush();
5899
expect(calls).toBe(2);
59-
stop();
100+
watch.stop();
101+
});
102+
103+
test("a null result after a known branch is not reported", async () => {
104+
const seen: (string | null)[] = [];
105+
let branch: string | null = "main";
106+
const watch = watchGitBranch({
107+
cwd: "/repo",
108+
onBranch: (b) => seen.push(b),
109+
fetchBranch: async () => branch,
110+
});
111+
await flush();
112+
branch = null;
113+
watch.refresh();
114+
await flush();
115+
expect(seen).toEqual(["main"]);
116+
watch.stop();
60117
});
61118

62119
test("a lookup that lands after stop is dropped", async () => {
63120
const seen: (string | null)[] = [];
64121
const pending: ((branch: string | null) => void)[] = [];
65-
const clock = fakeClock();
66-
const stop = watchGitBranch({
122+
const watch = watchGitBranch({
67123
cwd: "/repo",
68124
onBranch: (b) => seen.push(b),
69125
fetchBranch: () =>
70126
new Promise((resolve) => {
71127
pending.push(resolve);
72128
}),
73-
schedule: clock.schedule,
74129
});
75-
stop();
130+
watch.stop();
76131
pending[0]?.("main");
77-
await Promise.resolve();
132+
await flush();
78133
expect(seen).toEqual([]);
79134
});
80135

81136
test("a failed lookup neither reports nor wedges the guard", async () => {
82137
let calls = 0;
83138
const seen: (string | null)[] = [];
84-
const clock = fakeClock();
85-
const stop = watchGitBranch({
139+
const watch = watchGitBranch({
86140
cwd: "/repo",
87141
onBranch: (b) => seen.push(b),
88142
fetchBranch: async () => {
89143
calls += 1;
90144
throw new Error("git exploded");
91145
},
92-
schedule: clock.schedule,
93146
});
94-
await Promise.resolve();
147+
await flush();
95148
expect(seen).toEqual([]);
96-
clock.tick();
149+
watch.refresh();
150+
await flush();
97151
expect(calls).toBe(2);
98-
stop();
152+
watch.stop();
153+
});
154+
155+
test("an idle window of several former intervals spawns git zero times, and refresh still updates", async () => {
156+
jest.useFakeTimers();
157+
try {
158+
let calls = 0;
159+
let branch = "main";
160+
const seen: (string | null)[] = [];
161+
const watch = watchGitBranch({
162+
cwd: "/repo",
163+
onBranch: (b) => seen.push(b),
164+
fetchBranch: async () => {
165+
calls += 1;
166+
return branch;
167+
},
168+
});
169+
await flush();
170+
expect(seen).toEqual(["main"]);
171+
const afterStart = calls;
172+
173+
jest.advanceTimersByTime(FORMER_POLL_INTERVAL_MS * 6);
174+
await flush();
175+
expect(calls).toBe(afterStart);
176+
177+
branch = "topic";
178+
watch.refresh();
179+
await flush();
180+
expect(calls).toBe(afterStart + 1);
181+
expect(seen).toEqual(["main", "topic"]);
182+
watch.stop();
183+
} finally {
184+
jest.useRealTimers();
185+
}
99186
});
100187
});

0 commit comments

Comments
 (0)