style: trim live model switch comment noise

This commit is contained in:
Ayaan Zaidi
2026-04-04 22:42:30 +05:30
parent e4bd4b8b49
commit ef7c84ae92
2 changed files with 2 additions and 65 deletions

View File

@@ -1,10 +1,6 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { LiveSessionModelSwitchError } from "./live-model-switch.js";
// ---------------------------------------------------------------------------
// Hoisted mocks these must be declared before any vi.mock() calls so the
// mock factories can reference them.
// ---------------------------------------------------------------------------
const state = vi.hoisted(() => ({
runWithModelFallbackMock: vi.fn(),
runAgentAttemptMock: vi.fn(),
@@ -15,9 +11,6 @@ const state = vi.hoisted(() => ({
deliverAgentCommandResultMock: vi.fn(),
}));
// ---------------------------------------------------------------------------
// Module mocks
// ---------------------------------------------------------------------------
vi.mock("./model-fallback.js", () => ({
runWithModelFallback: (params: unknown) => state.runWithModelFallbackMock(params),
}));
@@ -269,16 +262,12 @@ vi.mock("./workspace.js", () => ({
ensureAgentWorkspace: async () => ({ dir: "/tmp/workspace" }),
}));
// ACP session manager mock resolveSession returns null (non-ACP path).
vi.mock("../acp/control-plane/manager.js", () => ({
getAcpSessionManager: () => ({
resolveSession: () => null,
}),
}));
// ---------------------------------------------------------------------------
// Test suite
// ---------------------------------------------------------------------------
async function getAgentCommand() {
return (await import("./agent-command.js")).agentCommand;
}
@@ -317,14 +306,11 @@ describe("agentCommand LiveSessionModelSwitchError retry", () => {
state.runWithModelFallbackMock.mockImplementation(async (params: FallbackRunnerParams) => {
invocation += 1;
if (invocation === 1) {
// First invocation: simulate the embedded runner detecting a live
// session model switch and throwing LiveSessionModelSwitchError.
throw new LiveSessionModelSwitchError({
provider: "openai",
model: "gpt-5.4",
});
}
// Second invocation: succeed with the switched model.
const result = await params.run(params.provider, params.model);
return {
result,
@@ -343,9 +329,6 @@ describe("agentCommand LiveSessionModelSwitchError retry", () => {
senderIsOwner: true,
});
// runWithModelFallback should have been called twice:
// 1st time with the default provider/model (throws LiveSessionModelSwitchError)
// 2nd time with the switched provider/model
expect(state.runWithModelFallbackMock).toHaveBeenCalledTimes(2);
const secondCall = state.runWithModelFallbackMock.mock.calls[1]?.[0] as
@@ -382,7 +365,6 @@ describe("agentCommand LiveSessionModelSwitchError retry", () => {
}),
).rejects.toThrow("provider down");
// Verify that a lifecycle error event was emitted.
const lifecycleErrorCalls = state.emitAgentEventMock.mock.calls.filter((call: unknown[]) => {
const arg = call[0] as { stream?: string; data?: { phase?: string } };
return arg?.stream === "lifecycle" && arg?.data?.phase === "error";
@@ -418,8 +400,6 @@ describe("agentCommand LiveSessionModelSwitchError retry", () => {
senderIsOwner: true,
});
// After successful retry, a lifecycle "end" event should be emitted
// (not an "error" event from the first iteration's switch).
const lifecycleEndCalls = state.emitAgentEventMock.mock.calls.filter((call: unknown[]) => {
const arg = call[0] as { stream?: string; data?: { phase?: string } };
return arg?.stream === "lifecycle" && arg?.data?.phase === "end";
@@ -433,7 +413,6 @@ describe("agentCommand LiveSessionModelSwitchError retry", () => {
state.runWithModelFallbackMock.mockImplementation(async (params: FallbackRunnerParams) => {
invocation += 1;
if (invocation === 1) {
// First invocation: switch includes an auth profile change.
throw new LiveSessionModelSwitchError({
provider: "openai",
model: "gpt-5.4",
@@ -441,7 +420,6 @@ describe("agentCommand LiveSessionModelSwitchError retry", () => {
authProfileIdSource: "user",
});
}
// Second invocation: succeed with the switched model.
const result = await params.run(params.provider, params.model);
return {
result,
@@ -464,11 +442,7 @@ describe("agentCommand LiveSessionModelSwitchError retry", () => {
senderIsOwner: true,
});
// The retried attempt must receive the switched provider for auth profile
// validation, not the original default provider.
expect(capturedAuthProfileProvider).toBe("openai");
// runWithModelFallback should have been called twice.
expect(state.runWithModelFallbackMock).toHaveBeenCalledTimes(2);
});
@@ -501,12 +475,10 @@ describe("agentCommand LiveSessionModelSwitchError retry", () => {
senderIsOwner: true,
});
// First call: no session override (initial state).
expect(resolveEffectiveModelFallbacksMock).toHaveBeenCalledTimes(2);
expect(resolveEffectiveModelFallbacksMock.mock.calls[0][0]).toMatchObject({
hasSessionModelOverride: false,
});
// Second call (retry): session now has a model override from the switch.
expect(resolveEffectiveModelFallbacksMock.mock.calls[1][0]).toMatchObject({
hasSessionModelOverride: true,
});
@@ -517,7 +489,6 @@ describe("agentCommand LiveSessionModelSwitchError retry", () => {
state.runWithModelFallbackMock.mockImplementation(async (params: FallbackRunnerParams) => {
invocation += 1;
if (invocation === 1) {
// Auth-only switch: same default provider/model, only authProfileId changes.
throw new LiveSessionModelSwitchError({
provider: "anthropic",
model: "claude",
@@ -544,8 +515,6 @@ describe("agentCommand LiveSessionModelSwitchError retry", () => {
senderIsOwner: true,
});
// Both calls should see hasSessionModelOverride: false because the model
// did not actually change — only the auth profile was switched.
expect(resolveEffectiveModelFallbacksMock).toHaveBeenCalledTimes(2);
expect(resolveEffectiveModelFallbacksMock.mock.calls[0][0]).toMatchObject({
hasSessionModelOverride: false,
@@ -560,7 +529,6 @@ describe("agentCommand LiveSessionModelSwitchError retry", () => {
state.runWithModelFallbackMock.mockImplementation(async (params: FallbackRunnerParams) => {
invocation += 1;
if (invocation === 1) {
// Provider-only switch: model name stays the same, only provider changes.
throw new LiveSessionModelSwitchError({
provider: "openai",
model: "claude",
@@ -585,12 +553,10 @@ describe("agentCommand LiveSessionModelSwitchError retry", () => {
senderIsOwner: true,
});
// First call: no session override (initial state).
expect(resolveEffectiveModelFallbacksMock).toHaveBeenCalledTimes(2);
expect(resolveEffectiveModelFallbacksMock.mock.calls[0][0]).toMatchObject({
hasSessionModelOverride: false,
});
// Second call (retry): provider changed so session should have a model override.
expect(resolveEffectiveModelFallbacksMock.mock.calls[1][0]).toMatchObject({
hasSessionModelOverride: true,
});

View File

@@ -742,14 +742,7 @@ async function agentCommandInternal(
let fallbackModel = model;
const MAX_LIVE_SWITCH_RETRIES = 5;
let liveSwitchRetries = 0;
// Retry loop: when the embedded runner detects a live session model switch
// (e.g. a subagent whose persisted session targets a different provider/model),
// it throws LiveSessionModelSwitchError. Catch it and restart the full
// runWithModelFallback cycle with the updated provider/model, mirroring the
// same retry logic used by the main auto-reply runner
// (see: agent-runner-execution.ts).
// eslint-disable-next-line no-constant-condition
while (true) {
for (;;) {
try {
const runContext = resolveAgentRunContext(opts);
const messageChannel = resolveMessageChannel(
@@ -757,16 +750,12 @@ async function agentCommandInternal(
opts.replyChannel ?? opts.channel,
);
const spawnedBy = normalizedSpawned.spawnedBy ?? sessionEntry?.spawnedBy;
// Keep fallback candidate resolution centralized so session model overrides,
// per-agent overrides, and default fallbacks stay consistent across callers.
const effectiveFallbacksOverride = resolveEffectiveModelFallbacks({
cfg,
agentId: sessionAgentId,
hasSessionModelOverride: Boolean(storedModelOverride),
});
// Track model fallback attempts so retries on an existing session don't
// re-inject the original prompt as a duplicate user message.
let fallbackAttemptIndex = 0;
const fallbackResult = await runWithModelFallback({
cfg,
@@ -806,7 +795,6 @@ async function agentCommandInternal(
allowTransientCooldownProbe: runOptions?.allowTransientCooldownProbe,
sessionHasHistory: !isNewSession || (await sessionFileHasContent(sessionFile)),
onAgentEvent: (evt) => {
// Track lifecycle end for fallback emission below.
if (
evt.stream === "lifecycle" &&
typeof evt.data?.phase === "string" &&
@@ -863,8 +851,6 @@ async function agentCommandInternal(
{ cause: err },
);
}
// Validate the switched model against the agent allowlist before
// accepting the switch, matching the stored/explicit override paths.
const switchRef = normalizeModelRef(err.provider, err.model);
const switchKey = modelKey(switchRef.provider, switchRef.model);
if (!allowAnyModel && !allowedModelKeys.has(switchKey)) {
@@ -889,24 +875,14 @@ async function agentCommandInternal(
{ cause: err },
);
}
// Capture the pre-switch provider and model before mutating, so the
// guard below can detect whether either actually changed.
const previousProvider = provider;
const previousModel = model;
provider = err.provider;
model = err.model;
fallbackProvider = err.provider;
fallbackModel = err.model;
// Keep auth-profile validation in sync so the next attempt resolves
// the correct session-level auth profile for the new provider.
providerForAuthProfileValidation = err.provider;
// Forward auth-profile fields carried by the switch request so the
// retried run uses the right credentials (mirrors the main runner).
// Clear the stale compaction count so the new profile is not
// treated as already aged by resolveSessionAuthProfileOverride.
if (sessionEntry) {
// Shallow-copy to avoid mutating the cached/shared original
// entry in-place, which could leak overrides across runs.
sessionEntry = { ...sessionEntry };
sessionEntry.authProfileOverride = err.authProfileId;
sessionEntry.authProfileOverrideSource = err.authProfileId
@@ -914,10 +890,6 @@ async function agentCommandInternal(
: undefined;
sessionEntry.authProfileOverrideCompactionCount = undefined;
}
// Only update storedModelOverride when the model or provider actually
// changed (or was already overridden). Auth-only switches that keep
// the same provider/model should not flip hasSessionModelOverride to
// true, because that would alter fallback candidate resolution.
if (
storedModelOverride ||
err.model !== previousModel ||
@@ -925,7 +897,6 @@ async function agentCommandInternal(
) {
storedModelOverride = err.model;
}
// Reset lifecycle tracking for the retry iteration.
lifecycleEnded = false;
log.info(
`Live session model switch in subagent run ${runId}: switching to ${sanitizeForLog(err.provider)}/${sanitizeForLog(err.model)}`,
@@ -946,7 +917,7 @@ async function agentCommandInternal(
}
throw err;
}
} // end while
}
// Update token+model fields in the session store.
if (sessionStore && sessionKey) {