summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorKit Langton <[email protected]>2026-04-11 20:01:52 -0400
committerGitHub <[email protected]>2026-04-11 20:01:52 -0400
commit1a509d62a0e2d21368d465e666228c26415fb5bd (patch)
tree7983761beb9c50a5f99c854991edf3e6fd92c480
parent4c4eef46f1c404a4ad2b205ae954bf4a17ef33a5 (diff)
downloadopencode-1a509d62a0e2d21368d465e666228c26415fb5bd.tar.gz
opencode-1a509d62a0e2d21368d465e666228c26415fb5bd.zip
refactor(session): destroy SessionRunState facade (#22064)
-rw-r--r--packages/opencode/specs/effect-migration.md45
-rw-r--r--packages/opencode/src/server/instance/session.ts17
-rw-r--r--packages/opencode/src/session/run-state.ts6
-rw-r--r--packages/opencode/test/server/session-actions.test.ts51
4 files changed, 59 insertions, 60 deletions
diff --git a/packages/opencode/specs/effect-migration.md b/packages/opencode/specs/effect-migration.md
index 1990f6a57..cde2d3bef 100644
--- a/packages/opencode/specs/effect-migration.md
+++ b/packages/opencode/specs/effect-migration.md
@@ -217,6 +217,7 @@ Fully migrated (single namespace, InstanceState where needed, flattened facade):
- [x] `SessionSummary` — `session/summary.ts`
- [x] `SessionRevert` — `session/revert.ts`
- [x] `Instruction` — `session/instruction.ts`
+- [x] `SystemPrompt` — `session/system.ts`
- [x] `Provider` — `provider/provider.ts`
- [x] `Storage` — `storage/storage.ts`
- [x] `ShareNext` — `share/share-next.ts`
@@ -340,3 +341,47 @@ For each service, the migration is roughly:
- `ShareNext` — migrated 2026-04-11. Swapped remaining async callers to `AppRuntime.runPromise(ShareNext.Service.use(...))`, removed the `makeRuntime(...)` facade, and kept instance bootstrap on the shared app runtime.
- `SessionTodo` — migrated 2026-04-10. Already matched the target service shape in `session/todo.ts`: single namespace, traced Effect methods, and no `makeRuntime(...)` facade remained; checklist updated to reflect the completed migration.
- `Storage` — migrated 2026-04-10. One production caller (`Session.diff`) and all storage.test.ts tests converted to effectful style. Facades and `makeRuntime` removed.
+- `SessionRunState` — migrated 2026-04-11. Single caller in `server/routes/session.ts` converted; facade removed.
+- `Account` — migrated 2026-04-11. Callers in `server/routes/experimental.ts` and `cli/cmd/account.ts` converted; facade removed.
+- `Instruction` — migrated 2026-04-11. Test-only callers converted; facade removed.
+- `FileTime` — migrated 2026-04-11. Test-only callers converted; facade removed.
+- `FileWatcher` — migrated 2026-04-11. Callers in `project/bootstrap.ts` and test converted; facade removed.
+- `Question` — migrated 2026-04-11. Callers in `server/routes/question.ts` and test converted; facade removed.
+- `Truncate` — migrated 2026-04-11. Caller in `tool/tool.ts` and test converted; facade removed.
+
+## Route handler effectification
+
+Route handlers should wrap their entire body in a single `AppRuntime.runPromise(Effect.gen(...))` call, yielding services from context rather than calling facades one-by-one. This eliminates multiple `runPromise` round-trips and lets handlers compose naturally.
+
+```ts
+// Before — one facade call per service
+async (c) => {
+ await SessionRunState.assertNotBusy(id)
+ await Session.removeMessage({ sessionID: id, messageID })
+ return c.json(true)
+}
+
+// After — one Effect.gen, yield services from context
+async (c) => {
+ await AppRuntime.runPromise(
+ Effect.gen(function* () {
+ const state = yield* SessionRunState.Service
+ const session = yield* Session.Service
+ yield* state.assertNotBusy(id)
+ yield* session.removeMessage({ sessionID: id, messageID })
+ }),
+ )
+ return c.json(true)
+}
+```
+
+When migrating, always use `{ concurrency: "unbounded" }` with `Effect.all` — route handlers should run independent service calls in parallel, not sequentially.
+
+Route files to convert (each handler that calls facades should be wrapped):
+
+- [ ] `server/routes/session.ts` — heaviest; uses Session, SessionPrompt, SessionRevert, SessionCompaction, SessionShare, SessionSummary, SessionRunState, Agent, Permission, Bus
+- [ ] `server/routes/global.ts` — uses Config, Project, Provider, Vcs, Snapshot, Agent
+- [ ] `server/routes/provider.ts` — uses Provider, Auth, Config
+- [ ] `server/routes/question.ts` — uses Question
+- [ ] `server/routes/pty.ts` — uses Pty
+- [ ] `server/routes/experimental.ts` — uses Account, ToolRegistry, Agent, MCP, Config
diff --git a/packages/opencode/src/server/instance/session.ts b/packages/opencode/src/server/instance/session.ts
index 016fb3ee1..b28db3a89 100644
--- a/packages/opencode/src/server/instance/session.ts
+++ b/packages/opencode/src/server/instance/session.ts
@@ -13,6 +13,7 @@ import { SessionShare } from "@/share/session"
import { SessionStatus } from "@/session/status"
import { SessionSummary } from "@/session/summary"
import { Todo } from "../../session/todo"
+import { Effect } from "effect"
import { AppRuntime } from "../../effect/app-runtime"
import { Agent } from "../../agent/agent"
import { Snapshot } from "@/snapshot"
@@ -724,11 +725,17 @@ export const SessionRoutes = lazy(() =>
),
async (c) => {
const params = c.req.valid("param")
- await SessionRunState.assertNotBusy(params.sessionID)
- await Session.removeMessage({
- sessionID: params.sessionID,
- messageID: params.messageID,
- })
+ await AppRuntime.runPromise(
+ Effect.gen(function* () {
+ const state = yield* SessionRunState.Service
+ const session = yield* Session.Service
+ yield* state.assertNotBusy(params.sessionID)
+ yield* session.removeMessage({
+ sessionID: params.sessionID,
+ messageID: params.messageID,
+ })
+ }),
+ )
return c.json(true)
},
)
diff --git a/packages/opencode/src/session/run-state.ts b/packages/opencode/src/session/run-state.ts
index 2c570f520..f67c726ec 100644
--- a/packages/opencode/src/session/run-state.ts
+++ b/packages/opencode/src/session/run-state.ts
@@ -1,6 +1,5 @@
import { InstanceState } from "@/effect/instance-state"
import { Runner } from "@/effect/runner"
-import { makeRuntime } from "@/effect/run-service"
import { Effect, Layer, Scope, Context } from "effect"
import { Session } from "."
import { MessageV2 } from "./message-v2"
@@ -106,9 +105,4 @@ export namespace SessionRunState {
)
export const defaultLayer = layer.pipe(Layer.provide(SessionStatus.defaultLayer))
- const { runPromise } = makeRuntime(Service, defaultLayer)
-
- export async function assertNotBusy(sessionID: SessionID) {
- return runPromise((svc) => svc.assertNotBusy(sessionID))
- }
}
diff --git a/packages/opencode/test/server/session-actions.test.ts b/packages/opencode/test/server/session-actions.test.ts
index 4ab485965..29032c69c 100644
--- a/packages/opencode/test/server/session-actions.test.ts
+++ b/packages/opencode/test/server/session-actions.test.ts
@@ -1,11 +1,9 @@
import { afterEach, describe, expect, mock, spyOn, test } from "bun:test"
+import { Effect } from "effect"
import { Instance } from "../../src/project/instance"
import { Server } from "../../src/server/server"
import { Session } from "../../src/session"
-import { ModelID, ProviderID } from "../../src/provider/schema"
-import { MessageID, PartID, type SessionID } from "../../src/session/schema"
import { SessionPrompt } from "../../src/session/prompt"
-import { SessionRunState } from "../../src/session/run-state"
import { Log } from "../../src/util/log"
import { tmpdir } from "../fixture/fixture"
@@ -16,25 +14,6 @@ afterEach(async () => {
await Instance.disposeAll()
})
-async function user(sessionID: SessionID, text: string) {
- const msg = await Session.updateMessage({
- id: MessageID.ascending(),
- role: "user",
- sessionID,
- agent: "build",
- model: { providerID: ProviderID.make("test"), modelID: ModelID.make("test") },
- time: { created: Date.now() },
- })
- await Session.updatePart({
- id: PartID.ascending(),
- sessionID,
- messageID: msg.id,
- type: "text",
- text,
- })
- return msg
-}
-
describe("session action routes", () => {
test("abort route calls SessionPrompt.cancel", async () => {
await using tmp = await tmpdir({ git: true })
@@ -45,9 +24,7 @@ describe("session action routes", () => {
const cancel = spyOn(SessionPrompt, "cancel").mockResolvedValue()
const app = Server.Default().app
- const res = await app.request(`/session/${session.id}/abort`, {
- method: "POST",
- })
+ const res = await app.request(`/session/${session.id}/abort`, { method: "POST" })
expect(res.status).toBe(200)
expect(await res.json()).toBe(true)
@@ -57,28 +34,4 @@ describe("session action routes", () => {
},
})
})
-
- test("delete message route returns 400 when session is busy", async () => {
- await using tmp = await tmpdir({ git: true })
- await Instance.provide({
- directory: tmp.path,
- fn: async () => {
- const session = await Session.create({})
- const msg = await user(session.id, "hello")
- const busy = spyOn(SessionRunState, "assertNotBusy").mockRejectedValue(new Session.BusyError(session.id))
- const remove = spyOn(Session, "removeMessage").mockResolvedValue(msg.id)
- const app = Server.Default().app
-
- const res = await app.request(`/session/${session.id}/message/${msg.id}`, {
- method: "DELETE",
- })
-
- expect(res.status).toBe(400)
- expect(busy).toHaveBeenCalledWith(session.id)
- expect(remove).not.toHaveBeenCalled()
-
- await Session.remove(session.id)
- },
- })
- })
})