fix(desktop): treat busy-path switch_model sent ack as provisional

The harness acks status:"sent" the moment a live model switch is
delivered to an in-flight turn, before the adapter is consulted — the
real verdict (failure/unsupported_model, or silence on success) lands
later from the requeued session. awaitLiveSwitchOutcome counted sent
toward the success threshold, so a single-channel switch resolved "ok"
and unsubscribed before the terminal frame could arrive, popping a false
"Model switched" toast on an adapter-rejected switch.

Make sent provisional: it no longer counts toward success and keeps the
subscription alive, so a later failure/unsupported_model settles the
pick and the 8s timeout is the no-terminal-frame success fallback. The
terminal-success statuses (switched/turn_ending/no_active_turn) keep
their per-channel count semantics.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
Duncan
2026-08-11 10:15:44 -04:00
parent 679937f0ce
commit a4e36e216a
3 changed files with 94 additions and 16 deletions

View File

@@ -88,15 +88,17 @@ test("awaitLiveSwitchOutcome resolves ok only after the last channel acks", asyn
}
};
h.push(frame("sent"));
// The three terminal-success statuses (`switched` / `turn_ending` /
// `no_active_turn`) each count for one channel; only the third settles "ok".
h.push(frame("switched"));
await drainMicrotasks();
assert.equal(settled, false, "must not resolve on the first ack");
h.push(frame("switched"));
h.push(frame("turn_ending"));
await drainMicrotasks();
assert.equal(settled, false, "must not resolve before the last ack");
h.push(frame("turn_ending"));
h.push(frame("no_active_turn"));
assert.equal(await h.outcome, "ok");
});
@@ -131,6 +133,60 @@ test("awaitLiveSwitchOutcome settles failed immediately on an adapter-refused fr
assert.equal(h.unsubscribeCalls, 1, "no double-unsubscribe on a late frame");
});
test("awaitLiveSwitchOutcome stays unsettled after a provisional sent, then settles failed when the adapter rejection arrives", async () => {
// The real busy-path producer order: the harness acks `sent` immediately
// (the switch was delivered to the in-flight turn), then — after the requeued
// session consults the adapter — emits `failure` seconds later. With one
// active channel a first-ack-resolves impl would settle "ok" on `sent` and
// detach before `failure` arrives; this regresses that.
const h = harness(1);
let settled = false;
void h.outcome.then(() => {
settled = true;
});
const drainMicrotasks = async () => {
for (let i = 0; i < 5; i++) {
await Promise.resolve();
}
};
h.push(frame("sent"));
await drainMicrotasks();
assert.equal(settled, false, "provisional `sent` must not resolve the pick");
assert.equal(h.unsubscribeCalls, 0, "subscription stays alive after `sent`");
assert.equal(h.cancelTimeoutCalls, 0, "timeout still armed after `sent`");
h.push(frame("failure"));
assert.equal(await h.outcome, "failed");
assert.equal(h.cancelTimeoutCalls, 1, "timeout cancelled, not awaited");
assert.equal(h.unsubscribeCalls, 1);
});
test("awaitLiveSwitchOutcome stays unsettled after a provisional sent, then resolves ok via the timeout when no terminal frame arrives", async () => {
// Busy-path success emits no positive confirmation frame — after `sent`, the
// backend stays silent. The pick must remain unsettled until the fallback
// timeout resolves "ok".
const h = harness(1);
let settled = false;
void h.outcome.then(() => {
settled = true;
});
const drainMicrotasks = async () => {
for (let i = 0; i < 5; i++) {
await Promise.resolve();
}
};
h.push(frame("sent"));
await drainMicrotasks();
assert.equal(settled, false, "provisional `sent` must not resolve the pick");
assert.equal(h.cancelTimeoutCalls, 0, "timeout still armed after `sent`");
h.fireTimeout();
assert.equal(await h.outcome, "ok");
assert.equal(h.unsubscribeCalls, 1, "timeout fallback unsubscribes");
});
test("awaitLiveSwitchOutcome ignores frames for a different model or control type", async () => {
const h = harness(1);
h.push(frame("sent", { modelId: "some-other-model" }));
@@ -156,9 +212,11 @@ test("awaitLiveSwitchOutcome resolves ok via the timeout fallback when the harne
test("awaitLiveSwitchOutcome fires the per-channel sends after subscribing", async () => {
const h = harness(1);
// The subscription is registered before the sends fire, so a frame arriving
// mid-send is never dropped. Awaiting sendStarted proves sends ran.
// mid-send is never dropped. Awaiting sendStarted proves sends ran. Uses a
// terminal-success frame (`switched`) since the provisional `sent` no longer
// settles the pick on its own.
await h.sendStarted;
h.push(frame("sent"));
h.push(frame("switched"));
assert.equal(await h.outcome, "ok");
});

View File

@@ -10,12 +10,21 @@ import type { ControlResultFrame } from "@/shared/api/types";
* - `unsupported_model` → the target model isn't available for this agent.
* - `failure` → the adapter refused the switch (the session stays
* on its current model).
* The causes differ, so they resolve to distinct outcomes (`"unsupported"` vs
* `"failed"`) the caller can message separately. Every other status
* (`sent` / `switched` / `turn_ending`) must arrive from every channel before
* resolving success. If the harness never replies, the fallback timeout
* resolves `"ok"` — the override still rides the requeued/next session, we just
* can't confirm it synchronously.
* Their causes differ, so they resolve to distinct outcomes (`"unsupported"`
* vs `"failed"`) the caller can message separately.
*
* `sent` is the busy-path PROVISIONAL ack: the switch was delivered to the
* in-flight turn, but the adapter isn't consulted until the requeued session
* runs. The real verdict lands later as a `failure` / `unsupported_model` frame,
* or — on success — as silence (the backend emits no positive confirmation). So
* `sent` never counts toward success; the subscription stays alive so a later
* terminal frame can still settle the pick.
*
* The remaining statuses — `switched` / `turn_ending` / `no_active_turn` — are
* terminal success for their channel and must arrive from every channel before
* resolving `"ok"`. When no terminal frame settles the pick (the common
* busy-path success case), the fallback timeout resolves `"ok"` — the override
* still rides the requeued/next session, we just can't confirm it synchronously.
*
* The counting lives here, isolated from React and the relay so it can be unit
* tested with synthetic frames and a fake clock. The caller injects the
@@ -64,7 +73,16 @@ export async function awaitLiveSwitchOutcome({
finish("failed");
return;
}
// sent / switched / turn_ending — count as success for this channel.
if (frame.status === "sent") {
// Busy-path provisional ack: the switch was delivered to the in-flight
// turn, but the adapter isn't consulted until the requeued session runs.
// Don't count it — a later `failure`/`unsupported_model` may still
// settle the pick, and on success the backend stays silent, so the
// timeout fallback is what confirms.
return;
}
// switched / turn_ending / no_active_turn — terminal success for this
// channel.
remaining -= 1;
if (remaining <= 0) {
finish("ok");

View File

@@ -110,8 +110,10 @@ export function ModelPicker({
// Send a live `switch_model` frame to each channel the agent is working in
// and wait for the harness to acknowledge. A single `unsupported_model`
// (model unavailable) or `failure` (adapter refused) result rejects the whole
// pick immediately; all other statuses must arrive from every channel before
// resolving success.
// pick immediately. The busy-path `sent` ack is provisional (the adapter
// isn't consulted until the requeued session), so a later terminal frame can
// still reject; the remaining success statuses must arrive from every channel,
// else the timeout fallback confirms.
const sendLiveSwitch = React.useCallback(
(modelId: string) => {
const channelIds = activeTurns.map((turn) => turn.channelId);
@@ -127,8 +129,8 @@ export function ModelPicker({
),
);
},
// No reply in time: treat as sent. The override still rides the
// requeued/next session; we just can't confirm synchronously.
// No terminal frame in time: treat as success. The override still
// rides the requeued/next session; we just can't confirm synchronously.
scheduleTimeout: (onTimeout) => {
const timeout = window.setTimeout(onTimeout, 8_000);
return () => window.clearTimeout(timeout);