Skip to content

Commit 4ddcf4e

Browse files
committed
fix(providers): replay only after reconnect succeeds
1 parent 057c290 commit 4ddcf4e

9 files changed

Lines changed: 182 additions & 28 deletions

‎src/tui/command-surfaces.test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1840,6 +1840,37 @@ describe("mcp surface", () => {
18401840
});
18411841

18421842
describe("host-routed surfaces", () => {
1843+
test("scoped add-provider forwards its completion callback", async () => {
1844+
await withShell((shell) => {
1845+
let received:
1846+
| {
1847+
initialKind?: string;
1848+
initialProfile?: string;
1849+
onComplete?: (connected: boolean) => void;
1850+
}
1851+
| undefined;
1852+
const completed: boolean[] = [];
1853+
expect(
1854+
openCommandSurface(
1855+
shell,
1856+
"add-provider",
1857+
{
1858+
notify: () => undefined,
1859+
openAddProvider: (opts) => {
1860+
received = opts;
1861+
},
1862+
},
1863+
{ kind: "xai", profile: "default-2" },
1864+
(connected) => completed.push(connected),
1865+
),
1866+
).toBe(true);
1867+
expect(received?.initialKind).toBe("xai");
1868+
expect(received?.initialProfile).toBe("default-2");
1869+
received?.onComplete?.(true);
1870+
expect(completed).toEqual([true]);
1871+
});
1872+
});
1873+
18431874
test.each([
18441875
{
18451876
surface: "models" as const,

‎src/tui/command-surfaces.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,12 +238,14 @@ export interface CommandSurfaceDeps {
238238
/**
239239
* Opens the host's add-provider selector (owned by the product host).
240240
* `/connect` omits returnToModels. `/connect <kind> [profile]` pre-scopes
241-
* via initialKind/initialProfile (kind row focused, profile to the flow).
241+
* via initialKind/initialProfile (kind row focused, profile to the flow),
242+
* with completion reported after that interactive flow settles.
242243
*/
243244
readonly openAddProvider?: (opts?: {
244245
returnToModels?: boolean;
245246
initialKind?: string;
246247
initialProfile?: string;
248+
onComplete?: (connected: boolean) => void;
247249
}) => void;
248250
/** Fallback channel for surfaces with no live data source. */
249251
readonly notify: (text: string) => void;
@@ -1666,6 +1668,7 @@ export function openCommandSurface(
16661668
kind: CommandSurfaceKind,
16671669
deps: CommandSurfaceDeps,
16681670
connectScope?: ReconnectScope,
1671+
onConnectComplete?: (connected: boolean) => void,
16691672
): boolean {
16701673
switch (kind) {
16711674
case "help":
@@ -1697,6 +1700,9 @@ export function openCommandSurface(
16971700
? {
16981701
initialKind: connectScope.kind,
16991702
initialProfile: connectScope.profile,
1703+
...(onConnectComplete !== undefined
1704+
? { onComplete: onConnectComplete }
1705+
: {}),
17001706
}
17011707
: undefined,
17021708
);

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,6 +1106,36 @@ describe("flat type-to-filter model picker", () => {
11061106
}
11071107
});
11081108

1109+
test("scoped add-provider forwards completion with the reconnect request", async () => {
1110+
const completed: boolean[] = [];
1111+
let finish: ((connected: boolean) => void) | undefined;
1112+
const { harness, host } = await mountPicker({
1113+
onConnectProvider: (_name, req) => {
1114+
expect(req?.kind).toBe("xai");
1115+
expect(req?.profile).toBe("default-2");
1116+
finish = req?.onComplete;
1117+
},
1118+
addProviderChoices: () => [
1119+
{ id: "openai", label: "OpenAI", hint: "", accountCount: 0 },
1120+
{ id: "xai", label: "xAI", hint: "", accountCount: 1 },
1121+
],
1122+
});
1123+
try {
1124+
host.openAddProvider?.({
1125+
initialKind: "xai",
1126+
initialProfile: "default-2",
1127+
onComplete: (connected) => completed.push(connected),
1128+
});
1129+
await harness.renderOnce();
1130+
acceptOverlaySelection(host.shell);
1131+
finish?.(true);
1132+
expect(completed).toEqual([true]);
1133+
} finally {
1134+
host.dispose();
1135+
harness.destroy();
1136+
}
1137+
});
1138+
11091139
test("typed /connect then Enter opens add-provider and Esc leaves overlay null", async () => {
11101140
const queued: { open?: () => void } = {};
11111141
const { harness, host } = await mountPicker({

‎src/tui/product-host.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,8 @@ export interface ProductHostAddProviderChoice {
130130
export interface ProductHostConnectRequest {
131131
readonly kind?: string;
132132
readonly profile?: string;
133+
/** Reports whether the interactive connect flow completed successfully. */
134+
readonly onComplete?: (connected: boolean) => void;
133135
}
134136

135137
export interface ProductHostConfig {
@@ -268,6 +270,7 @@ export interface ProductHost {
268270
returnToModels?: boolean;
269271
initialKind?: string;
270272
initialProfile?: string;
273+
onComplete?: (connected: boolean) => void;
271274
}) => void;
272275
/** Swap the picker's rows/descriptions in place (e.g. after a provider connects). */
273276
readonly setModels?: (
@@ -598,6 +601,7 @@ export async function mountProductHost(
598601
returnToModels?: boolean;
599602
initialKind?: string;
600603
initialProfile?: string;
604+
onComplete?: (connected: boolean) => void;
601605
}) => void)
602606
| undefined;
603607
if (config.onModelSelect) {
@@ -620,6 +624,7 @@ export async function mountProductHost(
620624
returnToModels?: boolean;
621625
initialKind?: string;
622626
initialProfile?: string;
627+
onComplete?: (connected: boolean) => void;
623628
}): void => {
624629
const rows = addProviderChoices();
625630
const scopedIndex =
@@ -644,7 +649,13 @@ export async function mountProductHost(
644649
opts?.initialProfile !== undefined &&
645650
(opts.initialKind === undefined || opts.initialKind === id)
646651
) {
647-
onConnect(id, { kind: id, profile: opts.initialProfile });
652+
onConnect(id, {
653+
kind: id,
654+
profile: opts.initialProfile,
655+
...(opts.onComplete !== undefined
656+
? { onComplete: opts.onComplete }
657+
: {}),
658+
});
648659
return;
649660
}
650661
onConnect(id);

‎src/tui/runner/host.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ export type RunnerHost = ProductHost & {
180180
readonly openSurface: (
181181
kind: CommandSurfaceKind,
182182
connectScope?: ReconnectScope,
183+
onConnectComplete?: (connected: boolean) => void,
183184
) => boolean;
184185
/**
185186
* Recompute the models-first catalog from fresh recent/favorite refs and
@@ -511,8 +512,14 @@ export async function mountRunnerHost(
511512
return {
512513
...host,
513514
dispose,
514-
openSurface: (kind, connectScope) =>
515-
openCommandSurface(host.shell, kind, surfaceDeps, connectScope),
515+
openSurface: (kind, connectScope, onConnectComplete) =>
516+
openCommandSurface(
517+
host.shell,
518+
kind,
519+
surfaceDeps,
520+
connectScope,
521+
onConnectComplete,
522+
),
516523
refreshModels,
517524
refreshCostContext: pushCostContext,
518525
openCredentialRecovery,

‎src/tui/runner/index.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,8 @@ export async function runTUI(initialConfig: Config): Promise<number> {
257257
state.presentReconnectRecovery = createReconnectRecoveryPresenter({
258258
recovery: state.reconnectRecovery,
259259
openDialog: (dialog) => host.openReconnectRecovery(dialog),
260-
openReconnect: (scope) => host.openSurface("add-provider", scope),
260+
openReconnect: (scope, onComplete) =>
261+
host.openSurface("add-provider", scope, onComplete),
261262
readDirector: () => services.directorHolder.instance,
262263
deliverContinuation: (message) => liveAgent(state).deliver(message),
263264
});

‎src/tui/runner/reconnect-recovery.test.ts‎

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ describe("reconnect recovery accept gate", () => {
157157
});
158158

159159
describe("reconnect recovery selection", () => {
160-
test("reconnects the failed scope and replays the turn once", () => {
160+
test("reconnects the failed scope and replays only after success", () => {
161161
const state = createReconnectRecoveryState();
162162
const attempt = state.begin(operatorMessage(), "xai");
163163
state.observe(attempt, credentialRetry());
@@ -167,12 +167,14 @@ describe("reconnect recovery selection", () => {
167167
const seen: string[] = [];
168168
let armed: number | null = null;
169169
const delivered: InboundMessage[] = [];
170+
let completeReconnect: ((connected: boolean) => void) | undefined;
170171
const outcome = applyReconnectRecoverySelection({
171172
state,
172173
generation: pending.generation,
173174
selectedId: reconnectRecoveryItemId(pending.scope),
174-
reconnect: (scope) => {
175+
reconnect: (scope, onComplete) => {
175176
seen.push(`${scope.kind}/${scope.profile}`);
177+
completeReconnect = onComplete;
176178
},
177179
armContinuation: (generation) => {
178180
armed = generation;
@@ -184,10 +186,45 @@ describe("reconnect recovery selection", () => {
184186
delivered.push(message);
185187
},
186188
});
187-
expect(outcome).toBe("continued");
189+
expect(outcome).toBe("reconnected");
188190
expect(seen).toEqual(["xai/default-2"]);
191+
expect(armed).toBeNull();
192+
expect(delivered).toEqual([]);
193+
completeReconnect?.(true);
189194
expect(armed ?? -1).toBe(pending.generation);
190195
expect(delivered).toHaveLength(1);
196+
completeReconnect?.(true);
197+
expect(delivered).toHaveLength(1);
198+
});
199+
200+
test("cancel, failure, and committed attempts never replay", () => {
201+
for (const connected of [false, true]) {
202+
const state = createReconnectRecoveryState();
203+
const attempt = state.begin(operatorMessage(), "xai");
204+
state.observe(attempt, credentialRetry());
205+
if (connected) {
206+
state.observe(attempt, { type: "inference.tool-call", data: {} });
207+
}
208+
state.observe(attempt, credentialFailure("xai/default-2"));
209+
const pending = required(state.settle(attempt), "pending recovery");
210+
let completeReconnect: ((result: boolean) => void) | undefined;
211+
applyReconnectRecoverySelection({
212+
state,
213+
generation: pending.generation,
214+
selectedId: reconnectRecoveryItemId(pending.scope),
215+
reconnect: (_scope, onComplete) => {
216+
completeReconnect = onComplete;
217+
},
218+
armContinuation: () => {
219+
throw new Error("should not arm");
220+
},
221+
cancelContinuation: () => undefined,
222+
deliverContinuation: () => {
223+
throw new Error("should not deliver");
224+
},
225+
});
226+
completeReconnect?.(connected);
227+
}
191228
});
192229

193230
test("reconnect failure and stale generations never arm a continuation", () => {
@@ -254,6 +291,7 @@ describe("reconnect recovery presenter (runTUI wiring)", () => {
254291
};
255292
const dialogs: ReconnectDialog[] = [];
256293
const reconnected: ReconnectScope[] = [];
294+
let completeReconnect: ((connected: boolean) => void) | undefined;
257295
const armed: number[] = [];
258296
const cancelled: number[] = [];
259297
const delivered: InboundMessage[] = [];
@@ -263,8 +301,9 @@ describe("reconnect recovery presenter (runTUI wiring)", () => {
263301
dialogs.push(next);
264302
return dialogOpens;
265303
},
266-
openReconnect: (scope) => {
304+
openReconnect: (scope, onComplete?: (connected: boolean) => void) => {
267305
reconnected.push(scope);
306+
completeReconnect = onComplete;
268307
return options?.reconnectOpens ?? true;
269308
},
270309
readDirector: () =>
@@ -290,17 +329,22 @@ describe("reconnect recovery presenter (runTUI wiring)", () => {
290329
pending,
291330
dialog,
292331
reconnected,
332+
completeReconnect: (connected: boolean) => completeReconnect?.(connected),
293333
armed,
294334
cancelled,
295335
delivered,
296336
};
297337
}
298338

299-
test("accept re-keys the failed scope and replays the turn once", () => {
339+
test("accept waits for a successful re-key before replaying the turn once", () => {
300340
const wired = wirePresenter();
301341
expect(wired.dialog.scope).toEqual(wired.pending.scope);
302342
wired.dialog.onAccept(reconnectRecoveryItemId(wired.pending.scope));
303343
expect(wired.reconnected).toEqual([wired.pending.scope]);
344+
expect(wired.armed).toEqual([]);
345+
expect(wired.delivered).toEqual([]);
346+
347+
wired.completeReconnect(true);
304348
expect(wired.armed).toEqual([wired.pending.generation]);
305349
expect(wired.cancelled).toEqual([]);
306350
// The continuation message carries a build-time timestamp, so pin the
@@ -314,6 +358,9 @@ describe("reconnect recovery presenter (runTUI wiring)", () => {
314358
expect(continuation.headers.interchangeCorrelationId).toBe(
315359
String(wired.pending.generation),
316360
);
361+
wired.completeReconnect(true);
362+
expect(wired.armed).toEqual([wired.pending.generation]);
363+
expect(wired.delivered).toHaveLength(1);
317364
// The generation is consumed: a late cancel is a no-op.
318365
expect(wired.recovery.cancel(wired.pending.generation)).toBe(false);
319366
});

‎src/tui/runner/reconnect-recovery.ts‎

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -194,29 +194,36 @@ export function applyReconnectRecoverySelection(args: {
194194
state: ReturnType<typeof createReconnectRecoveryState>;
195195
generation: number;
196196
selectedId: string;
197-
reconnect: (scope: ReconnectScope) => void;
197+
reconnect: (
198+
scope: ReconnectScope,
199+
onComplete: (connected: boolean) => void,
200+
) => void;
198201
armContinuation: (generation: number) => void;
199202
cancelContinuation: (generation: number) => void;
200203
deliverContinuation: (message: InboundMessage) => void;
201-
}): "stale" | "invalid" | "reconnect-failed" | "reconnected" | "continued" {
204+
}): "stale" | "invalid" | "reconnect-failed" | "reconnected" {
202205
const acceptance = args.state.accept(args.generation, args.selectedId);
203206
if (acceptance.kind !== "accepted") return acceptance.kind;
207+
let completed = false;
208+
const onComplete = (connected: boolean): void => {
209+
if (completed) return;
210+
completed = true;
211+
if (!connected || !acceptance.replay) return;
212+
args.armContinuation(acceptance.generation);
213+
try {
214+
args.deliverContinuation(
215+
buildCredentialRecoveryContinuationMessage(acceptance.generation),
216+
);
217+
} catch {
218+
args.cancelContinuation(acceptance.generation);
219+
}
220+
};
204221
try {
205-
args.reconnect(acceptance.scope);
222+
args.reconnect(acceptance.scope, onComplete);
206223
} catch {
207224
return "reconnect-failed";
208225
}
209-
if (!acceptance.replay) return "reconnected";
210-
args.armContinuation(acceptance.generation);
211-
try {
212-
args.deliverContinuation(
213-
buildCredentialRecoveryContinuationMessage(acceptance.generation),
214-
);
215-
} catch {
216-
args.cancelContinuation(acceptance.generation);
217-
return "reconnected";
218-
}
219-
return "continued";
226+
return "reconnected";
220227
}
221228

222229
/**
@@ -236,7 +243,10 @@ export function createReconnectRecoveryPresenter(args: {
236243
onAccept: (id: string) => void;
237244
onCancel: () => void;
238245
}) => boolean;
239-
openReconnect: (scope: ReconnectScope) => boolean;
246+
openReconnect: (
247+
scope: ReconnectScope,
248+
onComplete: (connected: boolean) => void,
249+
) => boolean;
240250
readDirector: () =>
241251
| {
242252
armCredentialRecoveryContinuation: (generation: number) => void;
@@ -261,8 +271,8 @@ export function createReconnectRecoveryPresenter(args: {
261271
state: args.recovery,
262272
generation: pending.generation,
263273
selectedId: id,
264-
reconnect: (scope) => {
265-
if (!args.openReconnect(scope)) {
274+
reconnect: (scope, onComplete) => {
275+
if (!args.openReconnect(scope, onComplete)) {
266276
throw new Error(
267277
`reconnect surface unavailable for ${scope.kind}/${scope.profile}`,
268278
);

0 commit comments

Comments
 (0)