Skip to content
Draft
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
29 changes: 24 additions & 5 deletions packages/opencode/src/session/prompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,11 @@ export namespace SessionPrompt {
let structuredOutput: unknown | undefined

let step = 0
// altimate_change start — the agent's `steps` cap counts model requests only. `step` also
// counts iterations that run a compaction or a subtask and `continue`, which ended capped
// agents several requests early.
let modelSteps = 0
// altimate_change end
// altimate_change start — which tool catalogs of this loop run the workspace
// engine's turn boundary: the first, and the first after a new user request
// joins the loop. `step` counts loop iterations, and an iteration can
Expand Down Expand Up @@ -1516,7 +1521,10 @@ export namespace SessionPrompt {
// normal processing
const agent = await Agent.get(lastUser.agent)
const maxSteps = agent.steps ?? Infinity
const isLastStep = step >= maxSteps
// altimate_change start — see `modelSteps`
modelSteps++

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: a user request that joins the running loop after the cap gets no tools.

modelSteps lives for the generation and does not reset when a new request joins (where currentTurn changes). With steps: 2 and a request queued during the capped request, the follow-up at this head goes out with 0 tools and MAX_STEPS as the trailing user message, after the real question. At base it went out with all 94 tools and the assistant MAX_STEPS.

MAX_STEPS says "Tools are disabled until next user input", and a follow-up sent after idle gets a fresh count, so the outcome depends on timing. The cap text on that request is pre-existing; losing the tools is new.

Suggested fix: reset modelSteps where currentTurn moves to a new request.

Not blocking: edge input (a capped agent plus a follow-up queued in that window), and the model can still answer.

const isLastStep = modelSteps >= maxSteps
// altimate_change end
// altimate_change start — only a completed compaction may change this prefix.
// Recover the marker from persisted history too, including a process restart
// between summary completion and the next model call.
Expand Down Expand Up @@ -2040,6 +2048,10 @@ export namespace SessionPrompt {
// altimate_change start - every successful commit makes the next generation an echo, including after compaction
const echoCommittedAnswer =
format.type === "text" && agent.name === "analyst" && committedAnswer !== undefined
// altimate_change start — at the step cap a text answer is produced with tools off. A
// json_schema request keeps its tools: its answer is the StructuredOutput tool call.
const textFinalStep = isLastStep && !echoCommittedAnswer && format.type !== "json_schema"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: a tool call in answer to the toolless request keeps the loop going, with no bound.

If the provider returns a tool call anyway, the repair path maps it to invalid, the tool part errors with "Model tried to call unavailable tool 'invalid'", finish stays tool-calls, and the loop sends the same toolless request again. With steps: 3 on a mock that always returns a tool call, I saw 2 normal requests and then 10 capped requests, until the mock stopped. Repeating the identical call gives the same 12 requests, so the doom-loop guard does not end it.

Base also makes 12 requests, and there the tools really run, so this PR is an improvement, but the cap still does not end the run.

Suggested fix: after a textFinalStep request, break whatever the finish reason, and record an error if there is no text.

Not blocking: edge input (a provider returning tool calls with none declared), and main has the same loop in a worse form.

// altimate_change end
const deferredText: MessageV2.TextPart[] = []
// altimate_change end

Expand Down Expand Up @@ -2151,20 +2163,27 @@ export namespace SessionPrompt {
: isLastStep
? [
{
role: "assistant" as const,
// altimate_change start — the step-cap instruction is a user turn, like the echo
// above. Sent as a trailing assistant message with every tool still offered, the
// gateway rejected it and the run ended with no answer.
role: "user" as const,
// altimate_change end
content: MAX_STEPS,
},
]
: []),
// altimate_change end
],
// altimate_change start - the final echo cannot spend another step on tools
tools: echoCommittedAnswer ? {} : tools,
// altimate_change start - the final echo, and the step-cap answer, cannot spend another step on tools.
// Text finalization uses the same toolless shape as compaction and title generation: no
// tools and toolChoice "none", so llm.ts flattens historical tool calls for the gateway.
tools: echoCommittedAnswer || textFinalStep ? {} : tools,
deferText: echoCommittedAnswer ? deferredText : undefined,
// altimate_change end
model,
// altimate_change start - a user output schema still takes precedence over text finalization
toolChoice: echoCommittedAnswer ? "none" : format.type === "json_schema" ? "required" : undefined,
toolChoice:
echoCommittedAnswer || textFinalStep ? "none" : format.type === "json_schema" ? "required" : undefined,
// altimate_change end
})

Expand Down
17 changes: 17 additions & 0 deletions packages/opencode/src/session/task-output.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,20 @@ export async function interruptedTaskOutput(sessionID: SessionID, parentID?: Mes
].join("\n")
}
}

// altimate_change start — the newest model-written text of a child task's current prompt, for a
// task that ends on an error or without a final text answer (see tool/task.ts emptyTaskStatus)
export async function lastTaskText(sessionID: SessionID, parentID?: MessageID): Promise<string | undefined> {
for await (const message of MessageV2.stream(sessionID)) {
if (message.info.role === "user") break
if (message.info.role !== "assistant") continue
if (parentID && message.info.parentID !== parentID) continue

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: after a compaction in the child this loses all text, and the parent is told "It wrote no text".

After a compaction the newest user message is the compaction continuation. Post-compaction assistants carry parentID = continuation, so this filter skips them, and line 30 then breaks at that continuation, so text under the original prompt is never reached.

I ran it through the real prompt loop on a mock provider: the child writes "Found 3 candidates so far.", a usage-triggered compaction runs, and the next request returns 400. lastTaskText returns undefined, so emptyTaskStatus says "It wrote no text". The same run without the compaction returns the text. The PR says the result "carries the child's last written text if there is one".

Suggested fix: walk past user messages that carry a compaction part or are synthetic, stop only at the task's prompt id, and drop the parentID equality check.

Not blocking: it needs a compaction, then an error, with no later text. The error reason still reaches the parent, and the wording errs toward "unfinished".

const text = message.parts
.filter((part): part is MessageV2.TextPart => part.type === "text" && !part.ignored && !part.synthetic)
.map((part) => part.text)
.join("\n")
.trim()
if (text) return text
}
}
// altimate_change end
31 changes: 30 additions & 1 deletion packages/opencode/src/tool/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { RunBudget } from "@/altimate/run-budget"
import { ModelID, ProviderID } from "@/provider/schema"
import { Effect } from "effect"
// altimate_change start — upstream_fix: recover committed child findings when cancellation interrupts a task.
import { interruptedTaskOutput } from "../session/task-output"
import { interruptedTaskOutput, lastTaskText } from "../session/task-output"
// altimate_change end
const log = Log.create({ service: "tool.task" })

Expand Down Expand Up @@ -253,12 +253,17 @@ export const TaskTool = Tool.define("task", async (ctx) => {
if (ctx.abort.aborted) partialOutput ??= await interruptedTaskOutput(session.id, messageID)

const text = result?.parts.findLast((x) => x.type === "text")?.text ?? ""
// altimate_change start — a child that ends on an error or with no text must not reach the
// parent as an empty result, which reads as "done, nothing found".
const status = text.trim() || ctx.abort.aborted ? [] : await emptyTaskStatus(session.id, messageID, result)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: an errored child with a closed text block still reads as an answer.

status depends only on text.trim(). With a scripted stream of text-start, text-delta, text-end, error, result.info.error is set and the parts hold "Let me check the remaining tables.". The parent gets that narration inside <task_result> with no mention of the error. On an OpenAI-compatible stream that dies before the text block closes, the text is blanked to "", so that case does get the new status. This needs a provider that closes blocks before the failure.

Suggested fix: emit the reason whenever result.info.error is set, and keep text below it.

Not blocking: pre-existing on main, and the PR's claim is about empty results, so this is safe in a follow-up.

// altimate_change end

const output = partialOutput ?? [
`task_id: ${session.id} (for resuming to continue this task if needed)`,
"",
"<task_result>",
...(ctx.abort.aborted ? ["Task interrupted. Findings may be incomplete."] : []),
...status,
text,
"</task_result>",
].join("\n")
Expand All @@ -278,3 +283,27 @@ export const TaskTool = Tool.define("task", async (ctx) => {
},
}
})

// altimate_change start — explicit status for a child that produced no final text
async function emptyTaskStatus(
sessionID: SessionID,
messageID: MessageID,
result: MessageV2.WithParts | undefined,
): Promise<string[]> {
const error = result?.info.role === "assistant" ? result.info.error : undefined
const reason = error
? `Task stopped with an error before giving a final answer (${errorText(error)}).`
: "Task ended without a final text answer."
const partial = await lastTaskText(sessionID, messageID).catch(() => undefined)
return partial
? [reason, "Its last written text, which may be incomplete:", partial]
: [reason, "It wrote no text. Resume it with task_id or treat its work as unfinished."]
}

function errorText(error: NonNullable<MessageV2.Assistant["error"]>): string {
const message = "data" in error && error.data && typeof error.data === "object" && "message" in error.data
? String(error.data.message)
: ""
return message ? `${error.name}: ${message}` : error.name
}
// altimate_change end
82 changes: 82 additions & 0 deletions packages/opencode/test/session/prompt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3468,3 +3468,85 @@ noLLMServer.instance(
}),
30_000,
)

// altimate_change start - the step cap ends in a toolless text answer sent after a user turn,
// and only model requests count toward the cap (a compaction iteration does not).
for (const mode of ["plain", "compacted"] as const) {
noLLMServer.instance(
`the step cap asks for a final text answer as a user turn with tools off (${mode})`,
() =>
Effect.gen(function* () {
const { prompt, chat, sessions } = yield* boot()
const requests: Record<string, unknown>[] = []
// plain: tool, tool, final. compacted: tool (overflow), summary, tool, final.
const expected = mode === "compacted" ? 4 : 3
const language = createOpenAICompatible({
name: "test",
baseURL: "https://unused.invalid/v1",
fetch: Object.assign(
async (_url: string | URL | Request, init?: RequestInit) => {
if (typeof init?.body !== "string") throw new Error("expected a JSON request body")
const request = JSON.parse(init.body)
requests.push(request)
if (requests.length > expected) throw new Error("the step cap was not enforced")
const limited = JSON.stringify(request.messages).includes("CRITICAL - MAXIMUM STEPS REACHED")
const summary = mode === "compacted" && requests.length === 2
const response = limited
? reply().text("Maximum steps reached. Done so far: two probes.").stop().item()
: summary
? reply().text("Summary: two probes were run.").stop().item()
: reply()
.tool("glob", { pattern: `probe-${requests.length}-*` })
.usage({ input: mode === "compacted" && requests.length === 1 ? 80_000 : 1, output: 1 })
.item()
if (response.type !== "sse") throw new Error("expected SSE response")
return new Response(
[...response.head, ...response.tail].map((chunk) => `data: ${JSON.stringify(chunk)}\n\n`).join("") +
"data: [DONE]\n\n",
{ headers: { "Content-Type": "text/event-stream" } },
)
},
{ preconnect: () => undefined },
),
}).chatModel("test-model")
const languageSpy = spyOn(ProviderSvc, "getLanguage").mockResolvedValue(language)
try {
const result = yield* prompt.prompt({
sessionID: chat.id,
agent: "builder",
model: promptRef,
parts: [{ type: "text", text: "Probe the project, one glob at a time." }],
})
expect(requests).toHaveLength(expected)
const last = requests.at(-1)!
// Before the fix the cap message was a trailing assistant turn with every tool still offered.
expect(requestMessages(last).at(-1)!.role).toBe("user")
expect(requestText(requestMessages(last).at(-1)!.content)).toContain("CRITICAL - MAXIMUM STEPS REACHED")
expect(last.tools ?? []).toHaveLength(0)
for (const earlier of requests.slice(0, -1)) {
expect(JSON.stringify(earlier.messages)).not.toContain("CRITICAL - MAXIMUM STEPS REACHED")
}
if (mode === "compacted") {
const messages = yield* sessions.messages({ sessionID: chat.id })
expect(messages.some((message) => message.info.role === "assistant" && message.info.summary)).toBe(true)
}
expect(result.info).not.toHaveProperty("error")
expect(result.parts).toContainEqual(
expect.objectContaining({ type: "text", text: "Maximum steps reached. Done so far: two probes." }),
)
} finally {
languageSpy.mockRestore()
}
}),
{
config: {
...cfg,
snapshot: false,
learn: { enabled: false },
agent: { builder: { steps: 3, permission: { glob: "allow" } } },
},
},
30_000,
)
}
// altimate_change end
112 changes: 112 additions & 0 deletions packages/opencode/test/tool/task-empty-result.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// A child task that ends on an error (for example the request at its step cap) or with
// no text used to reach the parent as an empty <task_result>, which reads as "done, nothing found".
import { expect, spyOn, test } from "bun:test"
import { Effect } from "effect"
import { Agent } from "../../src/agent/agent"
import { Config } from "../../src/config/config"
import { Session } from "../../src/session"
import { MessageV2 } from "../../src/session/message-v2"
import { MessageID, SessionID } from "../../src/session/schema"
import { TaskTool, type TaskPromptOps } from "../../src/tool/task"
import { initTool } from "../altimate/tool-fixture"

const cases = [
{
name: "an errored child with earlier text returns the error and that text",
error: { name: "UnknownError", data: { message: "Could not get a response from the agent." } },
text: "",
earlier: "Found 3 candidates so far.",
contains: [
"Task stopped with an error before giving a final answer (UnknownError: Could not get a response from the agent.).",
"Its last written text, which may be incomplete:",
"Found 3 candidates so far.",
],
},
{
name: "a child with no text at all says so",
error: undefined,
text: "",
earlier: undefined,
contains: [
"Task ended without a final text answer.",
"It wrote no text. Resume it with task_id or treat its work as unfinished.",
],
},
{
name: "a child with a final answer is passed through unchanged",
error: undefined,
text: "Child answer",
earlier: undefined,
contains: ["<task_result>\nChild answer\n</task_result>"],
},
] as const

for (const item of cases) {
test(item.name, async () => {
const parentID = SessionID.make("ses_empty_parent")
const childID = SessionID.make("ses_empty_child")
const messageID = MessageID.make("msg_empty_parent")
const agent = { name: "general", mode: "subagent", permission: [], options: {} } as unknown as Agent.Info
let childPromptID: MessageID | undefined
const mocks = [
spyOn(Agent, "list").mockResolvedValue([agent]),
spyOn(Agent, "get").mockResolvedValue(agent),
spyOn(Config, "get").mockResolvedValue({}),
spyOn(Session, "get").mockImplementation(
Object.assign(
async (id: SessionID) =>
({
id,
permission: [],
...(id === childID ? { parentID, metadata: { taskAgent: "general" } } : {}),
}) as never,
Session.get,
),
),
spyOn(MessageV2, "get").mockResolvedValue({
info: { role: "assistant", parentID: messageID, modelID: "test", providerID: "test" },
parts: [],
} as never),
// Newest first, like the real stream: the failed final message, then an earlier one with text.
spyOn(MessageV2, "stream").mockImplementation(async function* () {
yield { info: { role: "assistant", parentID: childPromptID }, parts: [] } as never

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: the new tests miss the paths above, and this mock hides the compaction case.

The new tests do what they claim. Run against base, the 4 failing cases fail for the stated reasons (role assistant, 3 requests instead of 4, empty <task_result>), and the normal-answer case passes on both.

This mock gives every assistant the prompt's parentID, which real history does not do after a compaction, so lastTaskText never sees that shape. Not covered: a tool call at the cap, json_schema at the cap, a compaction followed by lastTaskText, a queued follow-up after the cap, and an error alongside text.

Not blocking: test-only.

if (item.earlier)
yield {
info: { role: "assistant", parentID: childPromptID },
parts: [{ type: "text", text: item.earlier }],
} as never
yield { info: { role: "user" }, parts: [] } as never
} as never),
]
const promptOps: TaskPromptOps = {
cancel: () => Effect.void,
resolvePromptParts: (text) => Effect.succeed([{ type: "text", text }]),
prompt: (input) =>
Effect.sync(() => {
childPromptID = input.messageID
return {
info: { role: "assistant", ...(item.error ? { error: item.error } : {}) },
parts: item.text ? [{ type: "text", text: item.text }] : [],
} as unknown as MessageV2.WithParts
}),
}
try {
const tool = await initTool(TaskTool)
const result = await tool.execute(
{ description: "Inspect records", prompt: "Summarize the records", subagent_type: "general", task_id: childID },
{
sessionID: parentID,
messageID,
agent: "build",
abort: new AbortController().signal,
messages: [],
extra: { promptOps, bypassAgentCheck: true },
},
)
for (const text of item.contains) expect(result.output).toContain(text)
if (item.text) expect(result.output).not.toContain("Task ended without")
} finally {
for (const mock of mocks) mock.mockRestore()
}
})
}
Loading