Skip to content

Commit 1a509d6

Browse files
authored
refactor(session): destroy SessionRunState facade (#22064)
1 parent 4c4eef4 commit 1a509d6

4 files changed

Lines changed: 59 additions & 60 deletions

File tree

packages/opencode/specs/effect-migration.md

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ Fully migrated (single namespace, InstanceState where needed, flattened facade):
217217
- [x] `SessionSummary``session/summary.ts`
218218
- [x] `SessionRevert``session/revert.ts`
219219
- [x] `Instruction``session/instruction.ts`
220+
- [x] `SystemPrompt``session/system.ts`
220221
- [x] `Provider``provider/provider.ts`
221222
- [x] `Storage``storage/storage.ts`
222223
- [x] `ShareNext``share/share-next.ts`
@@ -340,3 +341,47 @@ For each service, the migration is roughly:
340341
- `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.
341342
- `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.
342343
- `Storage` — migrated 2026-04-10. One production caller (`Session.diff`) and all storage.test.ts tests converted to effectful style. Facades and `makeRuntime` removed.
344+
- `SessionRunState` — migrated 2026-04-11. Single caller in `server/routes/session.ts` converted; facade removed.
345+
- `Account` — migrated 2026-04-11. Callers in `server/routes/experimental.ts` and `cli/cmd/account.ts` converted; facade removed.
346+
- `Instruction` — migrated 2026-04-11. Test-only callers converted; facade removed.
347+
- `FileTime` — migrated 2026-04-11. Test-only callers converted; facade removed.
348+
- `FileWatcher` — migrated 2026-04-11. Callers in `project/bootstrap.ts` and test converted; facade removed.
349+
- `Question` — migrated 2026-04-11. Callers in `server/routes/question.ts` and test converted; facade removed.
350+
- `Truncate` — migrated 2026-04-11. Caller in `tool/tool.ts` and test converted; facade removed.
351+
352+
## Route handler effectification
353+
354+
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.
355+
356+
```ts
357+
// Before — one facade call per service
358+
async (c) => {
359+
await SessionRunState.assertNotBusy(id)
360+
await Session.removeMessage({ sessionID: id, messageID })
361+
return c.json(true)
362+
}
363+
364+
// After — one Effect.gen, yield services from context
365+
async (c) => {
366+
await AppRuntime.runPromise(
367+
Effect.gen(function* () {
368+
const state = yield* SessionRunState.Service
369+
const session = yield* Session.Service
370+
yield* state.assertNotBusy(id)
371+
yield* session.removeMessage({ sessionID: id, messageID })
372+
}),
373+
)
374+
return c.json(true)
375+
}
376+
```
377+
378+
When migrating, always use `{ concurrency: "unbounded" }` with `Effect.all` — route handlers should run independent service calls in parallel, not sequentially.
379+
380+
Route files to convert (each handler that calls facades should be wrapped):
381+
382+
- [ ] `server/routes/session.ts` — heaviest; uses Session, SessionPrompt, SessionRevert, SessionCompaction, SessionShare, SessionSummary, SessionRunState, Agent, Permission, Bus
383+
- [ ] `server/routes/global.ts` — uses Config, Project, Provider, Vcs, Snapshot, Agent
384+
- [ ] `server/routes/provider.ts` — uses Provider, Auth, Config
385+
- [ ] `server/routes/question.ts` — uses Question
386+
- [ ] `server/routes/pty.ts` — uses Pty
387+
- [ ] `server/routes/experimental.ts` — uses Account, ToolRegistry, Agent, MCP, Config

packages/opencode/src/server/instance/session.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { SessionShare } from "@/share/session"
1313
import { SessionStatus } from "@/session/status"
1414
import { SessionSummary } from "@/session/summary"
1515
import { Todo } from "../../session/todo"
16+
import { Effect } from "effect"
1617
import { AppRuntime } from "../../effect/app-runtime"
1718
import { Agent } from "../../agent/agent"
1819
import { Snapshot } from "@/snapshot"
@@ -724,11 +725,17 @@ export const SessionRoutes = lazy(() =>
724725
),
725726
async (c) => {
726727
const params = c.req.valid("param")
727-
await SessionRunState.assertNotBusy(params.sessionID)
728-
await Session.removeMessage({
729-
sessionID: params.sessionID,
730-
messageID: params.messageID,
731-
})
728+
await AppRuntime.runPromise(
729+
Effect.gen(function* () {
730+
const state = yield* SessionRunState.Service
731+
const session = yield* Session.Service
732+
yield* state.assertNotBusy(params.sessionID)
733+
yield* session.removeMessage({
734+
sessionID: params.sessionID,
735+
messageID: params.messageID,
736+
})
737+
}),
738+
)
732739
return c.json(true)
733740
},
734741
)

packages/opencode/src/session/run-state.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { InstanceState } from "@/effect/instance-state"
22
import { Runner } from "@/effect/runner"
3-
import { makeRuntime } from "@/effect/run-service"
43
import { Effect, Layer, Scope, Context } from "effect"
54
import { Session } from "."
65
import { MessageV2 } from "./message-v2"
@@ -106,9 +105,4 @@ export namespace SessionRunState {
106105
)
107106

108107
export const defaultLayer = layer.pipe(Layer.provide(SessionStatus.defaultLayer))
109-
const { runPromise } = makeRuntime(Service, defaultLayer)
110-
111-
export async function assertNotBusy(sessionID: SessionID) {
112-
return runPromise((svc) => svc.assertNotBusy(sessionID))
113-
}
114108
}
Lines changed: 2 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
import { afterEach, describe, expect, mock, spyOn, test } from "bun:test"
2+
import { Effect } from "effect"
23
import { Instance } from "../../src/project/instance"
34
import { Server } from "../../src/server/server"
45
import { Session } from "../../src/session"
5-
import { ModelID, ProviderID } from "../../src/provider/schema"
6-
import { MessageID, PartID, type SessionID } from "../../src/session/schema"
76
import { SessionPrompt } from "../../src/session/prompt"
8-
import { SessionRunState } from "../../src/session/run-state"
97
import { Log } from "../../src/util/log"
108
import { tmpdir } from "../fixture/fixture"
119

@@ -16,25 +14,6 @@ afterEach(async () => {
1614
await Instance.disposeAll()
1715
})
1816

19-
async function user(sessionID: SessionID, text: string) {
20-
const msg = await Session.updateMessage({
21-
id: MessageID.ascending(),
22-
role: "user",
23-
sessionID,
24-
agent: "build",
25-
model: { providerID: ProviderID.make("test"), modelID: ModelID.make("test") },
26-
time: { created: Date.now() },
27-
})
28-
await Session.updatePart({
29-
id: PartID.ascending(),
30-
sessionID,
31-
messageID: msg.id,
32-
type: "text",
33-
text,
34-
})
35-
return msg
36-
}
37-
3817
describe("session action routes", () => {
3918
test("abort route calls SessionPrompt.cancel", async () => {
4019
await using tmp = await tmpdir({ git: true })
@@ -45,9 +24,7 @@ describe("session action routes", () => {
4524
const cancel = spyOn(SessionPrompt, "cancel").mockResolvedValue()
4625
const app = Server.Default().app
4726

48-
const res = await app.request(`/session/${session.id}/abort`, {
49-
method: "POST",
50-
})
27+
const res = await app.request(`/session/${session.id}/abort`, { method: "POST" })
5128

5229
expect(res.status).toBe(200)
5330
expect(await res.json()).toBe(true)
@@ -57,28 +34,4 @@ describe("session action routes", () => {
5734
},
5835
})
5936
})
60-
61-
test("delete message route returns 400 when session is busy", async () => {
62-
await using tmp = await tmpdir({ git: true })
63-
await Instance.provide({
64-
directory: tmp.path,
65-
fn: async () => {
66-
const session = await Session.create({})
67-
const msg = await user(session.id, "hello")
68-
const busy = spyOn(SessionRunState, "assertNotBusy").mockRejectedValue(new Session.BusyError(session.id))
69-
const remove = spyOn(Session, "removeMessage").mockResolvedValue(msg.id)
70-
const app = Server.Default().app
71-
72-
const res = await app.request(`/session/${session.id}/message/${msg.id}`, {
73-
method: "DELETE",
74-
})
75-
76-
expect(res.status).toBe(400)
77-
expect(busy).toHaveBeenCalledWith(session.id)
78-
expect(remove).not.toHaveBeenCalled()
79-
80-
await Session.remove(session.id)
81-
},
82-
})
83-
})
8437
})

0 commit comments

Comments
 (0)