Skip to content

Commit 1d9360e

Browse files
committed
Fix the mid-session connect-a-new-provider flow crashing on a live terminal
Choosing "connect a new provider" from the running model picker called connectProviderInline without a renderer override, so it fell through to creating its own CliRenderer while the session's own was still live. Two renderers cannot share one terminal's stdin, so every real invocation failed with "stdin is already used by another CliRenderer" — invisible in the existing tests because they always supply a headless renderer override, masking the default path. The picker now hands connectProviderInline the session's own renderer, paused for the duration and resumed on completion, instead of letting it create one. That required provider-setup's teardown to stop unconditionally destroying whatever renderer it was given, since a caller-supplied renderer (a test harness, or here a live session's own) is owned by that caller, not by the setup form borrowing it.
1 parent f712dda commit 1d9360e

3 files changed

Lines changed: 45 additions & 16 deletions

File tree

src/tui-opentui/provider-setup.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,19 @@ async function mountLogin(opts: {
271271
return { done, harness }
272272
}
273273

274+
describe("runProviderSetup renderer ownership", () => {
275+
test("does not destroy a caller-supplied renderer on cancel", async () => {
276+
const { done, harness } = await mountSetup()
277+
harness.pressKey("Ctrl+C")
278+
expect(await done).toBe(false)
279+
280+
// A caller-owned renderer must still be usable for whatever mounted it
281+
// in the first place (a live session resuming its own UI after a
282+
// mid-session reconnect), not torn down out from under it.
283+
expect(harness.renderer.isDestroyed).toBe(false)
284+
})
285+
})
286+
274287
describe("runProviderSetup sign-in", () => {
275288
test("a subscription provider signs in in place and persists the selection", async () => {
276289
const seen: ProviderFormValues[] = []

src/tui-opentui/provider-setup.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,10 @@ const RAMP_TICK_MS = 120
605605
export async function runProviderSetup(
606606
config: ProviderSetupConfig,
607607
): Promise<boolean> {
608+
// A caller-supplied renderer (a headless test harness, or a live session's
609+
// renderer reused for a mid-session reconnect) is owned by that caller —
610+
// teardown here must not destroy it out from under them.
611+
const externalRenderer = config.createRenderer !== undefined
608612
const renderer = config.createRenderer
609613
? await config.createRenderer()
610614
: await createCliRenderer({
@@ -1075,10 +1079,12 @@ export async function runProviderSetup(
10751079
} catch {
10761080
// already unmounted
10771081
}
1078-
try {
1079-
renderer.destroy()
1080-
} catch {
1081-
// already destroyed
1082+
if (!externalRenderer) {
1083+
try {
1084+
renderer.destroy()
1085+
} catch {
1086+
// already destroyed
1087+
}
10821088
}
10831089
}
10841090

src/tui/runner.ts

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1984,20 +1984,30 @@ export async function runTUI(initialConfig: Config): Promise<number> {
19841984
unconnectedProviders: computeUnconnectedProviders(config.providers),
19851985
onConnectProvider: (providerName) => {
19861986
void (async () => {
1987+
// A second CliRenderer cannot claim the terminal while the host's is
1988+
// live (they contend for the same stdin) — the setup form reuses the
1989+
// host's own renderer, paused for the duration and handed back on
1990+
// completion, rather than creating one of its own.
1991+
host.renderer.pause();
19871992
let result: Awaited<ReturnType<typeof connectProviderInline>>;
19881993
try {
1989-
result = await connectProviderInline({
1990-
providerId: providerName,
1991-
settingsPath: trueGlobalSettingsPath,
1992-
localSettingsPath: localSettingsFile,
1993-
cwd: config.cwd,
1994-
existing: config.settings ?? null,
1995-
});
1996-
} catch (err) {
1997-
systemRow(
1998-
`Connecting ${providerName} failed: ${err instanceof Error ? err.message : String(err)}`,
1999-
);
2000-
return;
1994+
try {
1995+
result = await connectProviderInline({
1996+
providerId: providerName,
1997+
settingsPath: trueGlobalSettingsPath,
1998+
localSettingsPath: localSettingsFile,
1999+
cwd: config.cwd,
2000+
existing: config.settings ?? null,
2001+
createRenderer: () => Promise.resolve(host.renderer),
2002+
});
2003+
} catch (err) {
2004+
systemRow(
2005+
`Connecting ${providerName} failed: ${err instanceof Error ? err.message : String(err)}`,
2006+
);
2007+
return;
2008+
}
2009+
} finally {
2010+
host.renderer.resume();
20012011
}
20022012
if (!result.connected) return;
20032013

0 commit comments

Comments
 (0)