fix(acp): reject a mismatched fork cwd and resolve overlapping provider IDs (#53534)

This commit is contained in:
Shoubhit Dash
2026-10-06 22:49:59 +05:30
committed by GitHub
Unverified
parent de2cacdac8
commit ed0e7acecd
5 changed files with 35 additions and 28 deletions
+18 -20
View File
@@ -28,10 +28,10 @@ export function configOptions(catalog: Catalog, selection: Selection): SessionCo
name: "Model",
category: "model",
type: "select",
currentValue: `${model.providerID}/${model.id}`,
currentValue: advertisedModel(model),
options: catalog.models
.toSorted((a, b) => Order.String(a.providerID, b.providerID) || a.name.localeCompare(b.name))
.map((item) => ({ value: `${item.providerID}/${item.id}`, name: `${item.providerID}/${item.name}` })),
.map((item) => ({ value: advertisedModel(item), name: `${item.providerID}/${item.name}` })),
},
...(variants.length > 0
? [
@@ -101,24 +101,18 @@ export const resolveChange = Effect.fnUntraced(function* (
})
export function parseModelSelection(value: string, models: ReadonlyArray<Model.Info>): Model.Ref {
const providerID = models
.map((model) => model.providerID)
.toSorted()
.find((id) => value.startsWith(`${id}/`))
if (!providerID) {
const separator = value.indexOf("/")
if (separator === -1) return { providerID: Provider.ID.make(value), id: Model.ID.make("") }
return { providerID: Provider.ID.make(value.slice(0, separator)), id: Model.ID.make(value.slice(separator + 1)) }
}
const id = Model.ID.make(value.slice(providerID.length + 1))
if (findModel(models, { providerID, id })) return { providerID, id }
const separator = id.lastIndexOf("/")
const baseID = Model.ID.make(separator === -1 ? id : id.slice(0, separator))
const variant = separator === -1 ? undefined : id.slice(separator + 1)
const model = findModel(models, { providerID, id: baseID })
if (model && variant && model.variants.some((item) => item.id === variant))
return { providerID, id: baseID, variant: Model.VariantID.make(variant) }
return { providerID, id }
const exact = models.find((model) => advertisedModel(model) === value)
if (exact) return { providerID: exact.providerID, id: exact.id }
const separator = value.lastIndexOf("/")
const variant = Model.VariantID.make(value.slice(separator + 1))
const base = models.find(
(model) =>
advertisedModel(model) === value.slice(0, separator) && model.variants.some((item) => item.id === variant),
)
if (base) return { providerID: base.providerID, id: base.id, variant }
const providerEnd = value.indexOf("/")
if (providerEnd === -1) return { providerID: Provider.ID.make(value), id: Model.ID.make("") }
return { providerID: Provider.ID.make(value.slice(0, providerEnd)), id: Model.ID.make(value.slice(providerEnd + 1)) }
}
const requireModel = Effect.fnUntraced(function* (catalog: Catalog, value: string, current: Model.Ref) {
@@ -137,6 +131,10 @@ const requireModel = Effect.fnUntraced(function* (catalog: Catalog, value: strin
return { providerID: model.providerID, id: model.id, variant } satisfies Model.Ref
})
function advertisedModel(model: { readonly providerID: string; readonly id: string }) {
return `${model.providerID}/${model.id}`
}
function selectVariant(variant: string | undefined, variants: readonly string[]) {
if (!variant || variant === DEFAULT_VARIANT_VALUE) return DEFAULT_VARIANT_VALUE
if (variants.includes(variant)) return variant
+2 -2
View File
@@ -216,8 +216,8 @@ export function make(input: {
forkSession: Effect.fnUntraced(function* (params) {
const directories = yield* ACPDirectories.parse(params.cwd, params.additionalDirectories)
const mcpServers = yield* supportedMcpServers(params.mcpServers)
const sessionID = yield* ACPClient.decodeSessionID(params.sessionId)
const forked = yield* input.client.session.fork({ sessionID }).pipe(Effect.catch(ACPClient.classify))
const session = yield* getSession(params.sessionId, params.cwd)
const forked = yield* input.client.session.fork({ sessionID: session.id }).pipe(Effect.catch(ACPClient.classify))
// Forks inherit the source's grants; replace them with this request's.
yield* ACPDirectories.activate(input.client, forked, directories)
const attachment = yield* input.sessions.attach(forked, forked.location.directory, mcpServers)
+8 -3
View File
@@ -1,11 +1,13 @@
import { describe, expect, test } from "bun:test"
import { currentValue } from "./select-options"
import { rpcError, secondModel, startSession, startWire } from "./wire-fixture"
import { rpcError, secondModel, startWire } from "./wire-fixture"
describe("acp catalog and config options over the wire", () => {
test("switches model, effort, and mode against the warm catalog", async () => {
await using acp = await startSession()
const sessionId = acp.sessionId
await using acp = await startWire()
acp.server.catalog.models.push({ ...secondModel, providerID: "test/sub" })
await acp.initialize()
const sessionId = (await acp.newSession()).sessionId
const set = (configId: string, value: string) =>
acp.request("session/set_config_option", { sessionId, configId, value })
@@ -32,6 +34,9 @@ describe("acp catalog and config options over the wire", () => {
code: -32602,
data: { modelId: "test/missing-model" },
})
const overlap = "test/sub/second-model"
expect(currentValue(await set("model", overlap), "model")).toBe(overlap)
expect(currentValue(await set("model", `${overlap}/low`), "effort")).toBe("low")
})
test("answers the first session after a cold location activates its plugins (#52729, #52472)", async () => {
+5
View File
@@ -79,6 +79,11 @@ describe("acp session lifecycle over the wire", () => {
expect(
await rpcError(acp.request("session/load", { cwd: "/elsewhere", sessionId: "ses_loaded", mcpServers: [] })),
).toMatchObject({ code: -32602, data: { sessionId: "ses_loaded", cwd: "/elsewhere" } })
const sessions = new Set(acp.server.sessions.keys())
expect(
await rpcError(acp.request("session/fork", { cwd: "/elsewhere", sessionId: "ses_loaded", mcpServers: [] })),
).toMatchObject({ code: -32602, data: { sessionId: "ses_loaded", cwd: "/elsewhere" } })
expect(new Set(acp.server.sessions.keys())).toEqual(sessions)
expect(
await rpcError(acp.request("session/load", { cwd: "/workspace", sessionId: "ses_missing", mcpServers: [] })),
).toMatchObject({ code: -32602, data: { sessionId: "ses_missing" } })
+2 -3
View File
@@ -122,9 +122,8 @@ including its models, agents, commands, skills, instructions, plugins, and MCP s
- `cwd` must be an absolute path. OpenCode rejects a relative `cwd` with an invalid params error.
- `session/load` replays the saved messages. `session/resume` and `session/fork` don't replay them.
- To load or resume a session, the client must pass the session's own directory as `cwd`. OpenCode rejects a different
`cwd` with an invalid params error.
- A fork keeps the source session's directory.
- To load, resume, or fork a session, the client must pass that session's directory as `cwd`. OpenCode rejects a
different `cwd` with an invalid params error.
- `session/list` returns up to 100 sessions per page. It accepts a `cwd` filter and a `cursor`.
- `session/close` interrupts active work and detaches the session from the ACP process. OpenCode keeps the saved
session.