diff --git a/packages/app/e2e/regression/composer.spec.ts b/packages/app/e2e/regression/composer.spec.ts index e775dbcec59..bd29b70ec15 100644 --- a/packages/app/e2e/regression/composer.spec.ts +++ b/packages/app/e2e/regression/composer.spec.ts @@ -93,7 +93,8 @@ test("a catalog refresh cannot replace an existing session's model", async ({ pa await expect(page.getByRole("button", { name: "Choose model variant", exact: true })).toHaveText("high") await view.editor.press("Enter") await expect.poll(() => prompts.length).toBe(1) - expect(switches).toEqual([{ model: { id: "alpha", providerID: "opencode", variant: "high" } }]) + // The session already runs Alpha high, so the send has no switch to make. + expect(switches).toEqual([]) catalog.current = provider(beta) await view.push([ @@ -115,10 +116,7 @@ test("a catalog refresh cannot replace an existing session's model", async ({ pa await view.editor.fill("Use the replacement I chose") await view.editor.press("Enter") await expect.poll(() => prompts.length).toBe(2) - expect(switches).toEqual([ - { model: { id: "alpha", providerID: "opencode", variant: "high" } }, - { model: { id: "beta", providerID: "opencode" } }, - ]) + expect(switches).toEqual([{ model: { id: "beta", providerID: "opencode" } }]) }) test("an existing session's model never shows a fallback while its agent loads", async ({ page }) => { diff --git a/packages/app/e2e/regression/session-queue.spec.ts b/packages/app/e2e/regression/session-queue.spec.ts index c175d9e470c..a59e688ba19 100644 --- a/packages/app/e2e/regression/session-queue.spec.ts +++ b/packages/app/e2e/regression/session-queue.spec.ts @@ -227,7 +227,7 @@ test("follow-up preference controls Enter while Mod+Enter uses the alternate del await expect(view.input).toHaveText("") }) -test("a steer shows as pending in the timeline as the composer clears, before its selection commits", async ({ +test("a steer shows as pending in the timeline as the composer clears, before the server admits it", async ({ page, }) => { // The running execution has delivered input, so a new steer waits behind its work. @@ -237,7 +237,7 @@ test("a steer shows as pending in the timeline as the composer clears, before it ) const view = await openQueue(page, mock, "steer") - const commit = await holdRoute(page, (url) => url.pathname === `/api/session/${sessionID}/model`, { method: "POST" }) + const commit = await holdRoute(page, (url) => url.pathname === `/api/session/${sessionID}/prompt`, { method: "POST" }) const prompt = page.locator('[data-timeline-virtual-content] [data-timeline-row="UserMessage"]', { hasText: "steer while the model commits", diff --git a/packages/app/e2e/regression/session-request-docks.spec.ts b/packages/app/e2e/regression/session-request-docks.spec.ts index 2206c0e83cd..6cab7463253 100644 --- a/packages/app/e2e/regression/session-request-docks.spec.ts +++ b/packages/app/e2e/regression/session-request-docks.spec.ts @@ -1,3 +1,4 @@ +import type { OpenCodeEvent } from "@opencode/client/promise" import { expect, test, type Page } from "@playwright/test" import { SERVER, sessionHref } from "../utils/app" import { installSseTransport } from "../utils/sse-transport" @@ -209,6 +210,67 @@ test("shows a pending permission dock", async ({ page }) => { await expect(page.locator('[data-component="composer"]')).toBeVisible() }) +test("loads a running subagent's question without polling idle subagents on every update", async ({ page }) => { + const runningID = "ses_request_running_subagent" + const idleIDs = Array.from({ length: 20 }, (_, index) => `ses_request_idle_${index}`) + + const workspace = await mockWorkspace(page, { + name: "RequestDocks", + directory, + sessions: [ + { id: sessionID, title }, + { id: runningID, title: "Running subagent", parentID: sessionID }, + ...idleIDs.map((id) => ({ id, title: id, parentID: sessionID })), + ], + seed: { tabs: [sessionID] }, + // The mock reports a Session with a pending form as running. + forms: [ + { + id: "frm_subagent_question", + sessionID: runningID, + title: "Questions", + metadata: { kind: "question" }, + fields: [{ key: "q0", type: "string", title: "Direction", description: "Which way should the subagent go?" }], + }, + ], + }) + + // Idle subagents whose worktrees were deleted; a failed read is retried on its next sync. + await page.route(/\/api\/session\/ses_request_idle_\d+\/form$/, (route) => + route.fulfill({ status: 404, json: { message: "Location not found" } }), + ) + + const forms: string[] = [] + page.on("request", (request) => { + const path = new URL(request.url()).pathname + + if (request.method() === "GET" && path.startsWith("/api/session/") && path.endsWith("/form")) forms.push(path) + }) + + await page.goto(sessionHref(sessionID)) + await expectSessionTitle(page, title) + await expect( + page.locator('[data-component="dock-prompt"][data-kind="question"]').getByText("Which way should the subagent go?"), + ).toBeVisible() + + // Every session update used to re-request the forms of the whole subagent tree. + await workspace.push( + [...idleIDs, sessionID].map( + (id, index) => + // SAFETY: the fixture event matches the generated OpenCodeEvent shape for its type. + ({ + id: `evt_request_rename_${index}`, + created: index + 2, + type: "session.renamed", + durable: { aggregateID: id, seq: 1, version: 1 }, + data: { sessionID: id, title: id === sessionID ? "Renamed request docks" : `${id} renamed` }, + }) as OpenCodeEvent, + ), + ) + await expect(page.getByRole("heading", { name: "Renamed request docks", exact: true })).toBeVisible() + expect(forms.toSorted()).toEqual([`/api/session/${runningID}/form`, `/api/session/${sessionID}/form`].toSorted()) +}) + test("restores the draft caret before typing after a request dock closes", async ({ page }) => { const transport = await installSseTransport(page, { server: SERVER, retry: 20 }) await mockServer(page, { forms: [] }) diff --git a/packages/app/src/composer/submit.test.ts b/packages/app/src/composer/submit.test.ts index cc9de33af70..73de80caa77 100644 --- a/packages/app/src/composer/submit.test.ts +++ b/packages/app/src/composer/submit.test.ts @@ -214,10 +214,10 @@ describe("Composer submission", () => { current: { agent: "plan", model: { id: "old", providerID: "old" } }, calls: ["admit", "switch-agent", "switch-model", "send"], }, - // The model still commits: cached session state may lag behind an earlier switch. + // An unchanged selection sends without a model request to wait on. { current: { agent: "build", model: { providerID: "provider-1", id: "model-1", variant: "balanced" } }, - calls: ["admit", "switch-model", "send"], + calls: ["admit", "send"], }, // A prompt commits a staged revert, which would delete the model switch made after its boundary. The revert // settles before admission, whose row it would also delete. diff --git a/packages/app/src/composer/submit.ts b/packages/app/src/composer/submit.ts index bbb329f80ba..73c123dfa84 100644 --- a/packages/app/src/composer/submit.ts +++ b/packages/app/src/composer/submit.ts @@ -16,6 +16,9 @@ import { parseSlashCommand } from "./client-slash-command" const submitting = new WeakSet() +// The model each session was last switched to from this client, by session ID. +const switched = new Map() + type ComposerSubmission = { id: SessionMessage.ID mode: "normal" | "shell" @@ -500,11 +503,23 @@ async function applySelection( await beforeModel?.() - // The server deduplicates unchanged selections; cached SSE state may still be behind an earlier switch. + // Skip an unchanged model so a send never waits on that request. An agent switch or a revert can move the + // model, and cached SSE state may lag this client's earlier switch, which must also match. + const model = `${selection.model.providerID}/${selection.model.modelID}/${selection.variant ?? ""}` + const saved = current?.model && `${current.model.providerID}/${current.model.id}/${current.model.variant ?? ""}` + + if ( + current?.agent === selection.agent && + !current.revert && + saved === model && + (switched.get(session.id) ?? model) === model + ) + return await session.api.switchModel({ sessionID: session.id, model: { id: selection.model.modelID, providerID: selection.model.providerID, variant: selection.variant }, }) + switched.set(session.id, model) } catch (error) { cancel?.() throw error diff --git a/packages/app/src/session/requests/model.ts b/packages/app/src/session/requests/model.ts index 219588ed1b5..ff85f5c9a13 100644 --- a/packages/app/src/session/requests/model.ts +++ b/packages/app/src/session/requests/model.ts @@ -28,13 +28,21 @@ export function createSessionRequestModel() { () => undefined, ) }) - createEffect(() => { - const id = params.id - if (!id || serverSDK.connection.status() !== "connected") return - void Promise.all( - sessionTreeIDs(data.session.list(), id).map((sessionID) => data.session.form.sync(sessionID)), - ).catch(() => undefined) + // Only a running Session can hold a pending form, and events keep loaded forms current. Comparing by value + // keeps every session update in a large subagent tree from re-requesting forms. + const formSessions = createMemo( + () => + sessionTreeIDs(data.session.list(), params.id).filter( + (id) => id === params.id || data.session.status(id) === "running", + ), + [], + { equals: (a, b) => a.join() === b.join() }, + ) + + createEffect(() => { + if (serverSDK.connection.status() !== "connected") return + void Promise.all(formSessions().map((sessionID) => data.session.form.sync(sessionID))).catch(() => undefined) }) const formRequest = createMemo((): FormInfo | undefined => { diff --git a/packages/core/src/effect/app-node-builder.ts b/packages/core/src/effect/app-node-builder.ts index dfefbf8d662..427b5259828 100644 --- a/packages/core/src/effect/app-node-builder.ts +++ b/packages/core/src/effect/app-node-builder.ts @@ -2,7 +2,7 @@ import { buildLocationServiceMap } from "../location-services.js" import { LocationServiceMap } from "../location-service-map.js" import { LayerNode } from "@opencode/util/effect/layer-node" import { makeGlobalNode } from "@opencode/util/effect/app-node" -import { Effect, Layer } from "effect" +import { Effect, Layer, RcMap } from "effect" import { Instance } from "../instance/service.js" const instances = makeGlobalNode({ @@ -13,6 +13,7 @@ const instances = makeGlobalNode({ const locations = yield* LocationServiceMap.Service return Instance.Service.of({ provide: (session) => Effect.provide(locations.get(session.location)), + live: (session) => RcMap.has(locations.rcMap, LocationServiceMap.canonical(session.location)), }) }), ), diff --git a/packages/core/src/instance/service.ts b/packages/core/src/instance/service.ts index f23d28f49f2..8299cfd16a2 100644 --- a/packages/core/src/instance/service.ts +++ b/packages/core/src/instance/service.ts @@ -12,6 +12,8 @@ export interface Interface { readonly provide: ( session: Session.Info, ) => (effect: Effect.Effect) => Effect.Effect> + /** Whether `provide` would reuse built or booting capabilities. Selectors without it are assumed live. */ + readonly live?: (session: Session.Info) => Effect.Effect } export class Service extends Context.Service()("@opencode/Instance") {} diff --git a/packages/sdk/src/internal/instances.ts b/packages/sdk/src/internal/instances.ts index 1adecd3a4ea..2f505856a54 100644 --- a/packages/sdk/src/internal/instances.ts +++ b/packages/sdk/src/internal/instances.ts @@ -8,7 +8,7 @@ import { Location } from "@opencode/schema/location" import type { Session } from "@opencode/schema/session" import { makeGlobalNode } from "@opencode/util/effect/app-node" import type { LayerNode } from "@opencode/util/effect/layer-node" -import { Context, Duration, Effect, Layer, LayerMap, Scope } from "effect" +import { Context, Duration, Effect, Layer, LayerMap, RcMap, Scope } from "effect" export interface Configuration { readonly plugins: InstancePlugins.List @@ -102,7 +102,7 @@ export function layer(options: Options, replacements: () => LayerNode.Replacemen ), { idleTimeToLive: Duration.infinity }, ) - return Instance.Service.of({ provide }) + return Instance.Service.of({ provide, live: (session) => RcMap.has(instances.rcMap, key(session)) }) }), ) } diff --git a/packages/server/src/middleware/form-location.ts b/packages/server/src/middleware/form-location.ts index b975d1aacfb..3bf93b33f66 100644 --- a/packages/server/src/middleware/form-location.ts +++ b/packages/server/src/middleware/form-location.ts @@ -3,7 +3,7 @@ import { LocationServiceMap } from "@opencode/core/location-services" import { Session } from "@opencode/core/session" import { LocationNotFoundError, InvalidRequestError, SessionNotFoundError } from "@opencode/protocol/errors" import { Effect, Layer } from "effect" -import { HttpRouter, HttpServerRequest } from "effect/http" +import { HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/http" import { HttpApiMiddleware } from "effect/http-api" import { locationErrors, requestRef, sessionInfo, type LocationServices } from "../location" @@ -21,7 +21,7 @@ export const formLocationLayer = Layer.effect( const instances = yield* Instance.Service const locations = yield* LocationServiceMap.Service - return FormLocationMiddleware.of((effect) => + return FormLocationMiddleware.of((effect, options) => Effect.gen(function* () { const route = yield* HttpRouter.RouteContext if (route.params.sessionID === "global") { @@ -33,6 +33,10 @@ export const formLocationLayer = Layer.effect( } const session = yield* sessionInfo(sessions, route.params.sessionID) + // Forms live in memory, so a Session without live capabilities has none pending. Listing must + // not boot its Location: clients poll whole subagent trees, including deleted worktrees. + if (options.endpoint.identifier === "session.form.list" && instances.live && !(yield* instances.live(session))) + return HttpServerResponse.jsonUnsafe({ data: [] }) return yield* effect.pipe(instances.provide(session), locationErrors) }), ) diff --git a/packages/server/test/fetch.test.ts b/packages/server/test/fetch.test.ts index 952a89aca0a..c0ad070084f 100644 --- a/packages/server/test/fetch.test.ts +++ b/packages/server/test/fetch.test.ts @@ -43,7 +43,6 @@ it.live("returns LocationNotFoundError for a missing folder and recovers once it "/api/integration", `/api/session/${session.id}/permission`, `/api/experimental/session/${session.id}/instructions/entries`, - `/api/session/${session.id}/form`, "/api/session/global/form", ] for (const endpoint of endpoints) { diff --git a/packages/server/test/session-form.test.ts b/packages/server/test/session-form.test.ts new file mode 100644 index 00000000000..1f22d8ffa07 --- /dev/null +++ b/packages/server/test/session-form.test.ts @@ -0,0 +1,50 @@ +import { expect } from "bun:test" +import fs from "fs/promises" +import { Effect } from "effect" +import { tmpdirScoped } from "../../core/test/fixture/tmpdir" +import { it } from "../../core/test/lib/effect" +import { ServerFetch } from "../src/fetch" + +it.live("lists no forms for a Session whose Location is not live, and pending forms once it is", () => + Effect.gen(function* () { + const handler = yield* ServerFetch.make({ + app: { version: "test" }, + database: { path: ":memory:" }, + fs: { filewatcher: false }, + models: { fetch: false }, + }) + const request = (path: string, body?: unknown) => + Effect.promise(async () => { + const response = await handler( + new Request(`http://opencode.local${path}`, { + method: body === undefined ? "GET" : "POST", + headers: { "content-type": "application/json" }, + body: body === undefined ? undefined : JSON.stringify(body), + }), + ) + return { status: response.status, body: await response.json() } + }) + const create = (directory: string) => + request("/api/session", { location: { directory } }).pipe( + Effect.map((response) => (response.body as { data: { id: string } }).data.id), + ) + + // A subagent whose worktree was deleted must not boot or 404. + const deleted = yield* tmpdirScoped() + const orphan = yield* create(deleted.path) + yield* Effect.promise(() => fs.rm(deleted.path, { recursive: true, force: true })) + expect(yield* request(`/api/session/${orphan}/form`)).toEqual({ status: 200, body: { data: [] } }) + + const directory = yield* tmpdirScoped() + const live = yield* create(directory.path) + const created = yield* request(`/api/session/${live}/form`, { + title: "Pick one", + fields: [{ key: "answer", type: "string" }], + }) + expect(created.status).toBe(200) + expect(yield* request(`/api/session/${live}/form`)).toEqual({ + status: 200, + body: { data: [(created.body as { data: unknown }).data] }, + }) + }).pipe(Effect.scoped), +)