Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 5 additions & 20 deletions assistant/src/__tests__/conversation-process-callsite.test.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,11 @@
/**
* PR 6 — verify that `callSite` threads from `Conversation.processMessage`
* options all the way down to the per-call provider config.
* Verify that `callSite` threads from `Conversation.processMessage` options
* all the way down to the per-call provider config, and that user-initiated
* turns default to `'mainAgent'` when no caller-supplied `callSite` is set.
*
* The test mocks `AgentLoop.run()` so it can capture the `callSite` parameter
* the conversation passes after `processMessage` runs the slash-resolver and
* runtime-injection pipeline.
*
* NOTE: PR 6 originally defaulted absent callers to `'mainAgent'`, but
* Codex/Devin flagged that this routes every conversation turn through
* `RetryProvider`'s new call-site resolver while `config-model.setModel`
* still writes to `services.inference` without syncing `llm.default`.
* Defer the cutover to a future PR that handles the model-sync. Until
* then, agent-loop turns leave `callSite` undefined so the legacy
* `modelIntent` path remains in effect for the user-message conversation
* flow, while adapter callers (heartbeat/filing/scheduler) keep passing
* their explicit `callSite` through.
*/
import { describe, expect, mock, test } from "bun:test";

Expand Down Expand Up @@ -274,13 +265,7 @@ describe("processMessage callSite threading", () => {
expect(captured.callSite).toBe("heartbeatAgent");
});

test("leaves callSite undefined when not supplied (legacy modelIntent path)", async () => {
// PR 6 originally defaulted absent callers to 'mainAgent', but
// Codex/Devin flagged that this routes every conversation turn through
// the new RetryProvider call-site resolver while `services.inference`
// writes don't sync to `llm.default`. Defer the cutover to a future PR
// that handles the model-sync. Until then, agent-loop turns keep using
// the legacy modelIntent path by leaving `callSite` undefined.
test("defaults to 'mainAgent' when not supplied", async () => {
mockConversation = {
id: "conv-1",
contextSummary: null,
Expand All @@ -297,6 +282,6 @@ describe("processMessage callSite threading", () => {

await conversation.processMessage("Plain user message", [], () => {});

expect(captured.callSite).toBeUndefined();
expect(captured.callSite).toBe("mainAgent");
});
});
8 changes: 4 additions & 4 deletions assistant/src/agent/loop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,10 +268,10 @@ export class AgentLoop {

// Per-call LLM call-site identifier. Surfaces on the per-call
// `config.callSite` so `RetryProvider.normalizeSendMessageOptions`
// can route through `resolveCallSiteConfig`. PRs 7-11 will switch
// individual callers from legacy `modelIntent`/hardcoded providers
// to call-site routing one at a time; until then the parameter is
// optional and absence preserves the legacy code path.
// can route through `resolveCallSiteConfig`. User-initiated
// conversation turns default to `mainAgent` in the agent loop's
// caller; other invocation contexts (heartbeat, filing, analyze,
// etc.) pass their own `callSite`.
if (callSite) {
providerConfig.callSite = callSite;
}
Expand Down
26 changes: 6 additions & 20 deletions assistant/src/daemon/conversation-agent-loop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,26 +388,12 @@ export async function runAgentLoopImpl(
});
let yieldedForHandoff = false;

// Resolve the LLM call-site for this turn.
//
// Reviewers (Codex P1 + Devin) flagged that defaulting absent callers to
// 'mainAgent' would route every conversation turn through
// `RetryProvider`'s new call-site resolver, which reads model/provider
// settings from `config.llm`. That creates a regression because
// `config-model.setModel` still writes to `services.inference` without
// syncing `llm.default`, so a model switch could leave agent-loop turns
// using stale or incompatible model IDs (e.g. an Anthropic model on an
// OpenAI transport).
//
// Defer the cutover. Leave `turnCallSite` undefined when the caller does
// not pass one, so `agentLoop.run()` omits `callSite` from the per-call
// provider config and `RetryProvider` keeps using the legacy
// `modelIntent` path. Adapter callers (heartbeat/filing/scheduler in
// PRs 7-11) still pass an explicit `callSite` and route through the new
// resolver as intended. A future PR will migrate the agent-loop turn to
// an explicit `'mainAgent'` callSite once the model-sync writers are
// wired up.
const turnCallSite: LLMCallSite | undefined = options?.callSite;
// Default user-initiated turns to the `mainAgent` call site. Other
// invocation contexts (heartbeat, filing, analyze, etc.) pass their own
// `callSite`. The provider layer resolves provider/model/maxTokens via
// `resolveCallSiteConfig`, picking up any user overrides under
// `llm.callSites.mainAgent`.
const turnCallSite: LLMCallSite = options?.callSite ?? "mainAgent";

// Capture the turn channel context *before* any awaits so a second
// message from a different channel can't overwrite it mid-flight.
Expand Down
Loading