Studio/opencode
5
0
Fork 0
mirror of https://gitee.com/zhangsken/opencode.git synced 2026-10-16 10:30:01 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(cli): preserve ACP reasoning boundaries and effort selections (#48493)

Browse Source
This commit is contained in:
Shoubhit Dash
2026-09-11 18:46:54 +05:30
committed by GitHub
parent e0deffa083
commit 2df00955cb
7 changed files with 96 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/cli/src/acp/config-option.ts
+5 -1
View File
@@ -76,7 +76,10 @@ export function buildEffortSelectOption(input: {
category: "thought_level",
type: "select",
currentValue: selectVariant(input.currentVariant, input.variants),
options: input.variants.map((variant) => ({ value: variant, name: formatVariantName(variant) })),
options: [...new Set([...input.variants, DEFAULT_VARIANT_VALUE])].map((variant) => ({
value: variant,
name: formatVariantName(variant),
})),
}
}
@@ -125,6 +128,7 @@ export function formatVariantName(variant: string) {
}
function selectVariant(variant: string | undefined, variants: readonly string[]) {
if (!variant || variant === DEFAULT_VARIANT_VALUE) return DEFAULT_VARIANT_VALUE
if (variant && variants.includes(variant)) return variant
if (variants.includes(DEFAULT_VARIANT_VALUE)) return DEFAULT_VARIANT_VALUE
return variants[0] ?? DEFAULT_VARIANT_VALUE
packages/cli/src/acp/event.ts
+4 -2
View File
@@ -201,7 +201,7 @@ export async function streamTurn(input: {
if (!child) assistantMessageID = event.data.assistantMessageID
await send({
sessionUpdate: "agent_thought_chunk",
messageId: event.data.assistantMessageID,
messageId: `${event.data.assistantMessageID}:reasoning:${event.data.ordinal}`,
content: { type: "text", text: event.data.delta },
})
continue
@@ -455,6 +455,8 @@ async function replayMessage(
return
}
if (message.type !== "assistant") return
// Live reasoning ordinals count only reasoning parts, not the mixed content array.
let reasoningOrdinal = 0
for (const part of message.content) {
if (part.type === "text") {
await connection.sessionUpdate({
@@ -472,7 +474,7 @@ async function replayMessage(
sessionId: sessionID,
update: {
sessionUpdate: "agent_thought_chunk",
messageId: message.id,
messageId: `${message.id}:reasoning:${reasoningOrdinal++}`,
content: { type: "text", text: part.text },
},
})
packages/cli/src/acp/service.ts
+20 -5
View File
@@ -41,7 +41,12 @@ import type {
} from "@agentclientprotocol/sdk"
import { OPENCODE_VERSION } from "../version"
import { SessionMessage } from "@opencode/schema/session-message"
import { buildConfigOptions, parseModelSelection, type ConfigOptionProvider } from "./config-option"
import {
buildConfigOptions,
DEFAULT_VARIANT_VALUE,
parseModelSelection,
type ConfigOptionProvider,
} from "./config-option"
import { promptContentToParts } from "./content"
import {
ChildSessionUpdateMethod,
@@ -275,7 +280,7 @@ export function make(input: { readonly client: OpenCodeClient; readonly connecti
if (typeof params.value !== "string") throw new ACPError.InvalidConfigOptionError({ configId: params.configId })
switch (params.configId) {
case "model": {
const selected = requireModel(state.catalog, params.value)
const selected = requireModel(state.catalog, params.value, state.model)
state.model = selected
await input.client.session.switchModel({ sessionID: state.id, model: selected })
break
@@ -284,7 +289,10 @@ export function make(input: { readonly client: OpenCodeClient; readonly connecti
const model = state.catalog.models.find(
(item) => item.providerID === state.model.providerID && item.id === state.model.id,
)
if (!model?.variants.some((variant) => variant.id === params.value))
if (
!model ||
(params.value !== DEFAULT_VARIANT_VALUE && !model.variants.some((variant) => variant.id === params.value))
)
throw new ACPError.InvalidEffortError({ effort: params.value })
state.model = { ...state.model, variant: params.value }
await input.client.session.switchModel({ sessionID: state.id, model: state.model })
@@ -453,7 +461,7 @@ function providers(models: readonly ModelInfo[]): ConfigOptionProvider[] {
}))
}
function requireModel(catalog: Catalog, modelID: string): ModelRef {
function requireModel(catalog: Catalog, modelID: string, current: ModelRef): ModelRef {
const selected = parseModelSelection(modelID, catalog.providers)
const model = catalog.models.find(
(item) => item.providerID === selected.model.providerID && item.id === selected.model.modelID,
@@ -461,7 +469,14 @@ function requireModel(catalog: Catalog, modelID: string): ModelRef {
if (!model) throw new ACPError.InvalidModelError({ providerId: selected.model.providerID, modelId: modelID })
if (selected.variant && !model.variants.some((variant) => variant.id === selected.variant))
throw new ACPError.InvalidEffortError({ effort: selected.variant })
return { providerID: model.providerID, id: model.id, variant: selected.variant }
const variant =
selected.variant ??
(current.providerID === model.providerID &&
current.id === model.id &&
(current.variant === DEFAULT_VARIANT_VALUE || model.variants.some((variant) => variant.id === current.variant))
? current.variant
: undefined)
return { providerID: model.providerID, id: model.id, variant }
}
async function selectMode(client: OpenCodeClient, state: Attached, modeID: string) {
packages/cli/test/acp/config-options.subprocess.test.ts
+21 -3
View File
@@ -50,11 +50,11 @@ describe("acp config option subprocess", () => {
const effort = requireSelectOption((await newSession(acp, fixture.home)).configOptions, "effort")
expect(effort.category).toBe("thought_level")
expect(effort.currentValue).toBe("low")
expect(flattenSelectOptions(effort).map((option) => option.value)).toEqual(["low", "high"])
expect(effort.currentValue).toBe("default")
expect(flattenSelectOptions(effort).map((option) => option.value)).toEqual(["low", "high", "default"])
}, 60_000)
test("effort switch updates currentValue", async () => {
test("effort survives model synchronization and can be reset to default", async () => {
await using fixture = await createAcpFixture()
const acp = fixture.spawn()
await initialize(acp)
@@ -70,5 +70,23 @@ describe("acp config option subprocess", () => {
)
expect(selectConfigOption(updated.configOptions, "effort")?.currentValue).toBe(nextEffort)
const synchronized = expectOk(
await acp.request<SetSessionConfigOptionResponse>("session/set_config_option", {
sessionId: session.sessionId,
configId: "model",
value: requireSelectOption(session.configOptions, "model").currentValue,
}),
)
expect(selectConfigOption(synchronized.configOptions, "effort")?.currentValue).toBe(nextEffort)
const reset = expectOk(
await acp.request<SetSessionConfigOptionResponse>("session/set_config_option", {
sessionId: session.sessionId,
configId: "effort",
value: "default",
}),
)
expect(selectConfigOption(reset.configOptions, "effort")?.currentValue).toBe("default")
}, 60_000)
})
packages/cli/test/acp/event-behavior.test.ts
+44 -6
View File
@@ -91,7 +91,7 @@ describe("acp event behavior", () => {
}
})
test("preserves text and reasoning order before returning the terminal response", async () => {
test("preserves reasoning boundaries and update order during streaming and replay", async () => {
const firstUpdate = Promise.withResolvers<void>()
const releaseUpdate = Promise.withResolvers<void>()
const allUpdates = Promise.withResolvers<void>()
@@ -108,6 +108,14 @@ describe("acp event behavior", () => {
delta: "think-1",
}),
)
send(
ephemeralEvent("session.reasoning.delta", {
sessionID: "ses_order",
assistantMessageID: "msg_order",
ordinal: 0,
delta: " continued",
}),
)
send(
ephemeralEvent("session.text.delta", {
sessionID: "ses_order",
@@ -120,7 +128,7 @@ describe("acp event behavior", () => {
ephemeralEvent("session.reasoning.delta", {
sessionID: "ses_order",
assistantMessageID: "msg_order",
ordinal: 2,
ordinal: 1,
delta: "think-2",
}),
)
@@ -144,7 +152,7 @@ describe("acp event behavior", () => {
firstUpdate.resolve()
await releaseUpdate.promise
}
if (updates.length === 3) allUpdates.resolve()
if (updates.length === 4) allUpdates.resolve()
},
requestPermission: async () => ({ outcome: { outcome: "cancelled" } }),
} satisfies Connection
@@ -171,18 +179,48 @@ describe("acp event behavior", () => {
) {
return [
item.update.sessionUpdate,
item.update.messageId,
item.update.content.type === "text" ? item.update.content.text : undefined,
]
}
return [item.update.sessionUpdate, undefined]
}),
).toEqual([
["agent_thought_chunk", "think-1"],
["agent_message_chunk", "answer"],
["agent_thought_chunk", "think-2"],
["agent_thought_chunk", "msg_order:reasoning:0", "think-1"],
["agent_thought_chunk", "msg_order:reasoning:0", " continued"],
["agent_message_chunk", "msg_order", "answer"],
["agent_thought_chunk", "msg_order:reasoning:1", "think-2"],
])
expect(fixture.requests.at(-1)?.path).toBe("/api/session/ses_order/message/msg_order")
expect(response).toMatchObject({ stopReason: "end_turn", usage: { totalTokens: 2 } })
const replayed: SessionUpdateParams[] = []
await replayMessages(recordingConnection(replayed), "ses_order", "/workspace", [
{
id: "msg_order",
type: "assistant",
agent: "build",
model: { providerID: "test", id: "test-model" },
time: { created: 1 },
content: [
{ type: "reasoning", text: "think-1 continued" },
{ type: "text", text: "answer" },
{ type: "reasoning", text: "think-2" },
],
},
])
expect(replayed).toEqual([
{
sessionId: "ses_order",
update: {
sessionUpdate: "agent_thought_chunk",
messageId: "msg_order:reasoning:0",
content: { type: "text", text: "think-1 continued" },
},
},
updates[2],
updates[3],
])
} finally {
releaseUpdate.resolve()
releaseSubmit.resolve()
packages/cli/test/acp/service-directory.test.ts
+1 -1
View File
@@ -222,7 +222,7 @@ describe("acp service directory behavior", () => {
await fixture.service.setSessionMode({ sessionId: session.sessionId, modeId: "build" })
expect(currentValue(selectedModel, "model")).toBe("test/second-model")
expect(currentValue(selectedModel, "effort")).toBe("low")
expect(currentValue(selectedModel, "effort")).toBe("default")
expect(currentValue(selectedEffort, "effort")).toBe("medium")
expect(currentValue(selectedMode, "mode")).toBe("plan")
expect(
packages/cli/test/acp/service-lifecycle.test.ts
+1 -1
View File
@@ -32,7 +32,7 @@ describe("acp service lifecycle", () => {
model: { providerID: "test", id: "second-model" },
},
})
expect(currentValue(created, "effort")).toBe("none")
expect(currentValue(created, "effort")).toBe("default")
})
test("loads and forks with paginated replay while resume does not replay", async () => {