From 2484193354f6f579a494fa59419534fdff26b777 Mon Sep 17 00:00:00 2001 From: Evanfeenstra Date: Wed, 16 Sep 2026 09:02:48 -0700 Subject: [PATCH] agents: detect [END_OF_ANSWER] from text, not the provider stop reason MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every proper finish through Bifrost looked like a stall. The agents registered [END_OF_ANSWER] as a provider stop sequence, so Anthropic consumed the marker and the only evidence of a finish was the raw stop reason "stop_sequence". Bifrost maps that to "stop" inbound and "stop" back to "end_turn" outbound (core/providers/anthropic/utils.go, v1.6.2 and v1.8.6 alike), so needsContinuation saw end_turn with no marker in the text, nudged the model twice, and reported `incomplete: stall` — which Hive's propose_code_change then refused, diff and all. Every repo_agent run behind the gateway paid two wasted rounds. Drop the stop sequence from the repo, graph, log, and jarvis sub-agents. The marker stays in the text, where createHasEndMarkerCondition, needsContinuation, and extractFinalAnswer already detect and strip it — the path every non-Anthropic provider was on all along, and the only one that survives a gateway. Providers that honor `stop` and report a plain "stop" either way (OpenAI-compatible) gain stall detection they never had. The rawFinishReason "stop_sequence" check stays as harmless belt-and-braces. turns.ts strips the marker before text becomes a reasoning Turn, so the protocol token never reaches the graph. Hive already strips it at its stream and MCP boundaries. Verified against a local server with the exact Hive request that failed: completed, no continuation nudges, preview.ok with the two-file diff, no marker in the returned content. --- mcp/src/graph_agent/agent.ts | 6 +++++- mcp/src/log/agent.ts | 6 +++++- mcp/src/repo/__tests__/turns.test.ts | 12 ++++++++++++ mcp/src/repo/agent.ts | 6 +++++- mcp/src/repo/session.ts | 6 +++--- mcp/src/repo/toolsJarvis.ts | 6 +++++- mcp/src/repo/turns.ts | 15 +++++++++++++-- mcp/src/repo/utils.ts | 16 ++++++++++------ 8 files changed, 58 insertions(+), 15 deletions(-) diff --git a/mcp/src/graph_agent/agent.ts b/mcp/src/graph_agent/agent.ts index 5adb0c0e8..ffbe4c973 100644 --- a/mcp/src/graph_agent/agent.ts +++ b/mcp/src/graph_agent/agent.ts @@ -204,7 +204,11 @@ async function prepareGraphAgent( instructions: systemPrompt, tools, stopWhen, - stopSequences: ["[END_OF_ANSWER]"], + // No stopSequences for the marker: termination is detected from the text + // (stopWhen / needsContinuation / extractFinalAnswer), which works on every + // provider. A stop sequence consumes the marker and leaves only the raw + // stop reason, which gateways rewrite (Bifrost maps stop_sequence to + // end_turn), making every proper finish look like a stall. onStepFinish: (sf) => { const now = Date.now(); const elapsedMs = now - lastStepTime; diff --git a/mcp/src/log/agent.ts b/mcp/src/log/agent.ts index 06873eb39..b4d9b0210 100644 --- a/mcp/src/log/agent.ts +++ b/mcp/src/log/agent.ts @@ -105,7 +105,11 @@ export async function log_agent_context( tools, providerOptions: getProviderOptions(provider, undefined, modelId) as any, stopWhen: hasEndMarker, - stopSequences: ["[END_OF_ANSWER]"], + // No stopSequences for the marker: termination is detected from the text + // (stopWhen / needsContinuation / extractFinalAnswer), which works on every + // provider. A stop sequence consumes the marker and leaves only the raw + // stop reason, which gateways rewrite (Bifrost maps stop_sequence to + // end_turn), making every proper finish look like a stall. onStepFinish: (sf) => { logStepMaybe(redactSecretsDeep(sf.content, redactOpts), opts.printAgentProgress); const usage = normalizeUsage(sf.usage); diff --git a/mcp/src/repo/__tests__/turns.test.ts b/mcp/src/repo/__tests__/turns.test.ts index 6eeb9e3fc..89db7113a 100644 --- a/mcp/src/repo/__tests__/turns.test.ts +++ b/mcp/src/repo/__tests__/turns.test.ts @@ -120,6 +120,18 @@ test.describe("turn emission", () => { expect(turns).toEqual([]); }); + test("the [END_OF_ANSWER] protocol marker is stripped from reasoning turns", async () => { + const { emitStepTurns } = await import("../turns.js"); + const sid = `sess-${randomUUID().slice(0, 8)}`; + const turns = emitStepTurns(sid, "hive", [ + { type: "text", text: "The answer.\n[END_OF_ANSWER]" }, + { type: "text", text: "[END_OF_ANSWER]" }, + ]); + expect(turns.length).toBe(1); + expect(turns[0].turn_type).toBe("reasoning"); + expect(turns[0].content).toBe("The answer."); + }); + test("a Concept-bearing tool result carries the concept link", async () => { const { emitStepTurns } = await import("../turns.js"); const sid = `sess-${randomUUID().slice(0, 8)}`; diff --git a/mcp/src/repo/agent.ts b/mcp/src/repo/agent.ts index ffcbf5405..2f797a35c 100644 --- a/mcp/src/repo/agent.ts +++ b/mcp/src/repo/agent.ts @@ -925,7 +925,11 @@ If the user's prompt mentions a sub-agent with an @mention (e.g. "@${validSubAge instructions: transparent ? undefined : instructions, tools, stopWhen, - stopSequences: ["[END_OF_ANSWER]"], + // No stopSequences for the marker: termination is detected from the text + // (stopWhen / needsContinuation / extractFinalAnswer), which works on every + // provider. A stop sequence consumes the marker and leaves only the raw + // stop reason, which gateways rewrite (Bifrost maps stop_sequence to + // end_turn), making every proper finish look like a stall. onStepFinish: (sf) => { const now = Date.now(); const elapsedMs = now - lastStepTime; diff --git a/mcp/src/repo/session.ts b/mcp/src/repo/session.ts index 7527aab13..2741e14fe 100644 --- a/mcp/src/repo/session.ts +++ b/mcp/src/repo/session.ts @@ -81,9 +81,9 @@ export interface StepMeta { turn: number; label?: string; finishReason?: string; - // Raw provider stop reason (e.g. Anthropic "end_turn" vs "stop_sequence") — - // the unified finishReason maps both to "stop", hiding the difference - // between a proper [END_OF_ANSWER] finish and a stall. + // Raw provider stop reason (e.g. Anthropic "end_turn" vs "max_tokens") — + // the unified finishReason coarsens it. Diagnostic only: a proper finish is + // recognized by the [END_OF_ANSWER] marker in the text, not by this value. rawFinishReason?: string; usage: AiUsageWithLegacy; cumulativeInput: number; diff --git a/mcp/src/repo/toolsJarvis.ts b/mcp/src/repo/toolsJarvis.ts index c6df71a45..d0f10a824 100644 --- a/mcp/src/repo/toolsJarvis.ts +++ b/mcp/src/repo/toolsJarvis.ts @@ -782,7 +782,11 @@ function registerGraphSubAgentTool( tools: runTools, providerOptions: getProviderOptions(details.provider, undefined, modelId) as any, stopWhen: maxSteps > 0 ? [hasEndMarker, stepCountIs(maxSteps)] : hasEndMarker, - stopSequences: ["[END_OF_ANSWER]"], + // No stopSequences for the marker: termination is detected from the text + // (stopWhen / needsContinuation / extractFinalAnswer), which works on every + // provider. A stop sequence consumes the marker and leaves only the raw + // stop reason, which gateways rewrite (Bifrost maps stop_sequence to + // end_turn), making every proper finish look like a stall. onStepFinish: (sf) => { if (!childSessionId) return; const now = Date.now(); diff --git a/mcp/src/repo/turns.ts b/mcp/src/repo/turns.ts index 3f03ff4b5..465bb8355 100644 --- a/mcp/src/repo/turns.ts +++ b/mcp/src/repo/turns.ts @@ -10,6 +10,15 @@ import path from "path"; import { db } from "../graph/neo4j.js"; import { conceptReadsFrom } from "./concepts.js"; +/** + * The agents end their final answer with a literal [END_OF_ANSWER] marker + * (kept in the text on purpose — see needsContinuation in utils.ts). It is a + * protocol token, not content: drop it before the text becomes a Turn. + */ +function stripEndMarker(text: string): string { + return text.replace(/\[END_OF_ANSWER\]/g, "").trim(); +} + // ── Live Turn emission ─────────────────────────────────────────────── // Mirrors each agent step into the graph as it happens: // @@ -299,7 +308,8 @@ export function emitStepTurns( for (const part of assistantParts) { if (part?.type === "text" && part.text) { - turns.push(buildTurn(sessionId, state, "reasoning", part.text, null)); + const text = stripEndMarker(part.text); + if (text) turns.push(buildTurn(sessionId, state, "reasoning", text, null)); } else if (part?.type === "tool-call") { if (part.toolCallId) { state.pendingToolInputs.set(part.toolCallId, part.input); @@ -461,7 +471,8 @@ export function turnsFromTranscript( } } else if (message.role === "assistant") { if (part?.type === "text" && part.text) { - push("reasoning", part.text, null); + const text = stripEndMarker(part.text); + if (text) push("reasoning", text, null); } else if (part?.type === "tool-call") { if (part.toolCallId) pendingInputs.set(part.toolCallId, part.input); let inputJson: string; diff --git a/mcp/src/repo/utils.ts b/mcp/src/repo/utils.ts index e99fcd02d..56866fb90 100644 --- a/mcp/src/repo/utils.ts +++ b/mcp/src/repo/utils.ts @@ -117,16 +117,20 @@ export function createHasEndMarkerCondition< /** * True when a run ended without a proper termination and should be nudged to * continue. Proper terminations are: an ask_clarifying_questions call, or the - * [END_OF_ANSWER] marker — present in text, or matched as a stop sequence - * (surfaced as rawFinishReason "stop_sequence"). A run whose last step has no + * [END_OF_ANSWER] marker present in the text. A run whose last step has no * tool calls and none of those is either a voluntary early stop (raw * "end_turn" — the model narrated a plan or emitted only reasoning and quit) * or a truncation ("length"); both are recoverable by asking it to continue. * - * Detection relies on the raw provider stop reason: a proper Anthropic finish - * hits the [END_OF_ANSWER] stop sequence (raw "stop_sequence") while a stall - * ends with raw "end_turn". OpenAI-compatible providers report a plain "stop" - * either way, so their stalls are not detectable this way and are left alone. + * The marker is deliberately NOT registered as a provider stop sequence: a + * stop sequence consumes it, leaving only the raw stop reason to tell a + * finish from a stall, and that reason does not survive an LLM gateway + * (Bifrost maps Anthropic's "stop_sequence" to "stop" and back to "end_turn", + * so every proper finish looked like a stall and burned the nudge allowance). + * Marker-in-text works identically on every provider. The "stop_sequence" + * check below stays as a harmless belt-and-braces for any caller that still + * registers one. OpenAI-compatible providers report a plain "stop" for a + * voluntary stall, so those are not detectable and are left alone. */ /** * True when the step cap ended a run mid-work, rather than the model finishing.