fix: preserve interrupted bash output in tool results (#21598)
This commit is contained in:
@@ -234,7 +234,11 @@ export namespace LLM {
|
|||||||
// from the workflow service are executed via opencode's tool system
|
// from the workflow service are executed via opencode's tool system
|
||||||
// and results sent back over the WebSocket.
|
// and results sent back over the WebSocket.
|
||||||
if (language instanceof GitLabWorkflowLanguageModel) {
|
if (language instanceof GitLabWorkflowLanguageModel) {
|
||||||
const workflowModel = language
|
const workflowModel = language as GitLabWorkflowLanguageModel & {
|
||||||
|
sessionID?: string
|
||||||
|
sessionPreapprovedTools?: string[]
|
||||||
|
approvalHandler?: (approvalTools: { name: string; args: string }[]) => Promise<{ approved: boolean }>
|
||||||
|
}
|
||||||
workflowModel.sessionID = input.sessionID
|
workflowModel.sessionID = input.sessionID
|
||||||
workflowModel.systemPrompt = system.join("\n")
|
workflowModel.systemPrompt = system.join("\n")
|
||||||
workflowModel.toolExecutor = async (toolName, argsJson, _requestID) => {
|
workflowModel.toolExecutor = async (toolName, argsJson, _requestID) => {
|
||||||
@@ -301,7 +305,7 @@ export namespace LLM {
|
|||||||
ruleset: [],
|
ruleset: [],
|
||||||
})
|
})
|
||||||
for (const name of uniqueNames) approvedToolsForSession.add(name)
|
for (const name of uniqueNames) approvedToolsForSession.add(name)
|
||||||
workflowModel.sessionPreapprovedTools = [...workflowModel.sessionPreapprovedTools, ...uniqueNames]
|
workflowModel.sessionPreapprovedTools = [...(workflowModel.sessionPreapprovedTools ?? []), ...uniqueNames]
|
||||||
return { approved: true }
|
return { approved: true }
|
||||||
} catch {
|
} catch {
|
||||||
return { approved: false }
|
return { approved: false }
|
||||||
|
|||||||
@@ -751,7 +751,19 @@ export namespace MessageV2 {
|
|||||||
...(differentModel ? {} : { callProviderMetadata: providerMeta(part.metadata) }),
|
...(differentModel ? {} : { callProviderMetadata: providerMeta(part.metadata) }),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
if (part.state.status === "error")
|
if (part.state.status === "error") {
|
||||||
|
const output = part.state.metadata?.interrupted === true ? part.state.metadata.output : undefined
|
||||||
|
if (typeof output === "string") {
|
||||||
|
assistantMessage.parts.push({
|
||||||
|
type: ("tool-" + part.tool) as `tool-${string}`,
|
||||||
|
state: "output-available",
|
||||||
|
toolCallId: part.callID,
|
||||||
|
input: part.state.input,
|
||||||
|
output,
|
||||||
|
...(part.metadata?.providerExecuted ? { providerExecuted: true } : {}),
|
||||||
|
...(differentModel ? {} : { callProviderMetadata: providerMeta(part.metadata) }),
|
||||||
|
})
|
||||||
|
} else {
|
||||||
assistantMessage.parts.push({
|
assistantMessage.parts.push({
|
||||||
type: ("tool-" + part.tool) as `tool-${string}`,
|
type: ("tool-" + part.tool) as `tool-${string}`,
|
||||||
state: "output-error",
|
state: "output-error",
|
||||||
@@ -761,6 +773,10 @@ export namespace MessageV2 {
|
|||||||
...(part.metadata?.providerExecuted ? { providerExecuted: true } : {}),
|
...(part.metadata?.providerExecuted ? { providerExecuted: true } : {}),
|
||||||
...(differentModel ? {} : { callProviderMetadata: providerMeta(part.metadata) }),
|
...(differentModel ? {} : { callProviderMetadata: providerMeta(part.metadata) }),
|
||||||
})
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Handle pending/running tool calls to prevent dangling tool_use blocks
|
||||||
|
// Anthropic/Claude APIs require every tool_use to have a corresponding tool_result
|
||||||
if (part.state.status === "pending" || part.state.status === "running")
|
if (part.state.status === "pending" || part.state.status === "running")
|
||||||
assistantMessage.parts.push({
|
assistantMessage.parts.push({
|
||||||
type: ("tool-" + part.tool) as `tool-${string}`,
|
type: ("tool-" + part.tool) as `tool-${string}`,
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ import { SessionStatus } from "./status"
|
|||||||
import { SessionSummary } from "./summary"
|
import { SessionSummary } from "./summary"
|
||||||
import type { Provider } from "@/provider/provider"
|
import type { Provider } from "@/provider/provider"
|
||||||
import { Question } from "@/question"
|
import { Question } from "@/question"
|
||||||
|
import { isRecord } from "@/util/record"
|
||||||
|
|
||||||
export namespace SessionProcessor {
|
export namespace SessionProcessor {
|
||||||
const DOOM_LOOP_THRESHOLD = 3
|
const DOOM_LOOP_THRESHOLD = 3
|
||||||
@@ -398,19 +399,21 @@ export namespace SessionProcessor {
|
|||||||
}
|
}
|
||||||
ctx.reasoningMap = {}
|
ctx.reasoningMap = {}
|
||||||
|
|
||||||
const parts = MessageV2.parts(ctx.assistantMessage.id)
|
for (const part of Object.values(ctx.toolcalls)) {
|
||||||
for (const part of parts) {
|
const end = Date.now()
|
||||||
if (part.type !== "tool" || part.state.status === "completed" || part.state.status === "error") continue
|
const metadata = "metadata" in part.state && isRecord(part.state.metadata) ? part.state.metadata : {}
|
||||||
yield* session.updatePart({
|
yield* session.updatePart({
|
||||||
...part,
|
...part,
|
||||||
state: {
|
state: {
|
||||||
...part.state,
|
...part.state,
|
||||||
status: "error",
|
status: "error",
|
||||||
error: "Tool execution aborted",
|
error: "Tool execution aborted",
|
||||||
time: { start: Date.now(), end: Date.now() },
|
metadata: { ...metadata, interrupted: true },
|
||||||
|
time: { start: "time" in part.state ? part.state.time.start : end, end },
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
ctx.toolcalls = {}
|
||||||
ctx.assistantMessage.time.completed = Date.now()
|
ctx.assistantMessage.time.completed = Date.now()
|
||||||
yield* session.updateMessage(ctx.assistantMessage)
|
yield* session.updateMessage(ctx.assistantMessage)
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -1507,7 +1507,7 @@ NOTE: At any point in time through this workflow you should feel free to ask the
|
|||||||
Effect.promise(() => SystemPrompt.skills(agent)),
|
Effect.promise(() => SystemPrompt.skills(agent)),
|
||||||
Effect.promise(() => SystemPrompt.environment(model)),
|
Effect.promise(() => SystemPrompt.environment(model)),
|
||||||
instruction.system().pipe(Effect.orDie),
|
instruction.system().pipe(Effect.orDie),
|
||||||
Effect.promise(() => MessageV2.toModelMessages(msgs, model)),
|
MessageV2.toModelMessagesEffect(msgs, model),
|
||||||
])
|
])
|
||||||
const system = [...env, ...(skills ? [skills] : []), ...instructions]
|
const system = [...env, ...(skills ? [skills] : []), ...instructions]
|
||||||
const format = lastUser.format ?? { type: "text" as const }
|
const format = lastUser.format ?? { type: "text" as const }
|
||||||
|
|||||||
@@ -570,6 +570,81 @@ describe("session.message-v2.toModelMessage", () => {
|
|||||||
])
|
])
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("forwards partial bash output for aborted tool calls", async () => {
|
||||||
|
const userID = "m-user"
|
||||||
|
const assistantID = "m-assistant"
|
||||||
|
const output = [
|
||||||
|
"31403",
|
||||||
|
"12179",
|
||||||
|
"4575",
|
||||||
|
"",
|
||||||
|
"<bash_metadata>",
|
||||||
|
"User aborted the command",
|
||||||
|
"</bash_metadata>",
|
||||||
|
].join("\n")
|
||||||
|
|
||||||
|
const input: MessageV2.WithParts[] = [
|
||||||
|
{
|
||||||
|
info: userInfo(userID),
|
||||||
|
parts: [
|
||||||
|
{
|
||||||
|
...basePart(userID, "u1"),
|
||||||
|
type: "text",
|
||||||
|
text: "run tool",
|
||||||
|
},
|
||||||
|
] as MessageV2.Part[],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
info: assistantInfo(assistantID, userID),
|
||||||
|
parts: [
|
||||||
|
{
|
||||||
|
...basePart(assistantID, "a1"),
|
||||||
|
type: "tool",
|
||||||
|
callID: "call-1",
|
||||||
|
tool: "bash",
|
||||||
|
state: {
|
||||||
|
status: "error",
|
||||||
|
input: { command: "for i in {1..20}; do print -- $RANDOM; sleep 1; done" },
|
||||||
|
error: "Tool execution aborted",
|
||||||
|
metadata: { interrupted: true, output },
|
||||||
|
time: { start: 0, end: 1 },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
] as MessageV2.Part[],
|
||||||
|
},
|
||||||
|
]
|
||||||
|
|
||||||
|
expect(await MessageV2.toModelMessages(input, model)).toStrictEqual([
|
||||||
|
{
|
||||||
|
role: "user",
|
||||||
|
content: [{ type: "text", text: "run tool" }],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
role: "assistant",
|
||||||
|
content: [
|
||||||
|
{
|
||||||
|
type: "tool-call",
|
||||||
|
toolCallId: "call-1",
|
||||||
|
toolName: "bash",
|
||||||
|
input: { command: "for i in {1..20}; do print -- $RANDOM; sleep 1; done" },
|
||||||
|
providerExecuted: undefined,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
role: "tool",
|
||||||
|
content: [
|
||||||
|
{
|
||||||
|
type: "tool-result",
|
||||||
|
toolCallId: "call-1",
|
||||||
|
toolName: "bash",
|
||||||
|
output: { type: "text", value: output },
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
])
|
||||||
|
})
|
||||||
|
|
||||||
test("filters assistant messages with non-abort errors", async () => {
|
test("filters assistant messages with non-abort errors", async () => {
|
||||||
const assistantID = "m-assistant"
|
const assistantID = "m-assistant"
|
||||||
|
|
||||||
|
|||||||
@@ -604,6 +604,7 @@ it.live("session.processor effect tests mark pending tools as aborted on cleanup
|
|||||||
expect(call?.state.status).toBe("error")
|
expect(call?.state.status).toBe("error")
|
||||||
if (call?.state.status === "error") {
|
if (call?.state.status === "error") {
|
||||||
expect(call.state.error).toBe("Tool execution aborted")
|
expect(call.state.error).toBe("Tool execution aborted")
|
||||||
|
expect(call.state.metadata?.interrupted).toBe(true)
|
||||||
expect(call.state.time.end).toBeDefined()
|
expect(call.state.time.end).toBeDefined()
|
||||||
}
|
}
|
||||||
}),
|
}),
|
||||||
|
|||||||
Reference in New Issue
Block a user