Skip to content

Commit 042748a

Browse files
committed
fix(session): close shell cancellation races
1 parent 4e21861 commit 042748a

5 files changed

Lines changed: 38 additions & 11 deletions

File tree

packages/opencode/src/effect/runner.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ export interface Runner<A, E = never> {
44
readonly state: State<A, E>
55
readonly busy: boolean
66
readonly ensureRunning: (work: Effect.Effect<A, E>) => Effect.Effect<A, E>
7-
readonly startShell: (work: Effect.Effect<A, E>) => Effect.Effect<A, E>
7+
readonly startShell: (work: Effect.Effect<A, E>, ready?: Deferred.Deferred<void>) => Effect.Effect<A, E>
88
readonly cancel: Effect.Effect<void>
99
}
1010

@@ -19,6 +19,7 @@ interface RunHandle<A, E> {
1919
interface ShellHandle<A, E> {
2020
id: number
2121
cancelled: Deferred.Deferred<void>
22+
ready?: Deferred.Deferred<void>
2223
fiber: Fiber.Fiber<A, E>
2324
}
2425

@@ -106,6 +107,7 @@ export const make = <A, E = never>(
106107

107108
const stopShell = (shell: ShellHandle<A, E>) =>
108109
Effect.gen(function* () {
110+
if (shell.ready) yield* Deferred.await(shell.ready).pipe(Effect.exit, Effect.asVoid)
109111
yield* Deferred.succeed(shell.cancelled, undefined).pipe(Effect.asVoid)
110112
yield* Fiber.interrupt(shell.fiber)
111113
})
@@ -135,7 +137,7 @@ export const make = <A, E = never>(
135137
}),
136138
).pipe(Effect.flatten)
137139

138-
const startShell = (work: Effect.Effect<A, E>) =>
140+
const startShell = (work: Effect.Effect<A, E>, ready?: Deferred.Deferred<void>) =>
139141
SynchronizedRef.modifyEffect(
140142
ref,
141143
Effect.fnUntraced(function* (st) {
@@ -152,12 +154,12 @@ export const make = <A, E = never>(
152154
const id = next()
153155
const cancelled = yield* Deferred.make<void>()
154156
const fiber = yield* work.pipe(Effect.ensuring(finishShell(id)), Effect.forkChild)
155-
const shell = { id, cancelled, fiber } satisfies ShellHandle<A, E>
157+
const shell = { id, cancelled, ready, fiber } satisfies ShellHandle<A, E>
156158
return [
157159
Effect.gen(function* () {
158160
const exit = yield* Fiber.await(fiber)
159161
if (Exit.isSuccess(exit)) return exit.value
160-
if ((yield* Deferred.isDone(cancelled)) || Cause.hasInterruptsOnly(exit.cause)) {
162+
if (Cause.hasInterruptsOnly(exit.cause) || ((yield* Deferred.isDone(cancelled)) && !Cause.hasDies(exit.cause))) {
161163
if (onInterrupt) return yield* onInterrupt
162164
return yield* Effect.die(new Cancelled())
163165
}
@@ -192,8 +194,8 @@ export const make = <A, E = never>(
192194
case "ShellThenRun":
193195
return [
194196
Effect.gen(function* () {
195-
yield* Deferred.fail(st.run.done, new Cancelled()).pipe(Effect.asVoid)
196197
yield* stopShell(st.shell)
198+
yield* Deferred.fail(st.run.done, new Cancelled()).pipe(Effect.asVoid)
197199
yield* idleIfCurrent()
198200
}),
199201
{ _tag: "Idle" } as const,

packages/opencode/src/session/prompt.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ import { AppFileSystem } from "@opencode-ai/core/filesystem"
4545
import { Truncate } from "@/tool/truncate"
4646
import { decodeDataUrl } from "@/util/data-url"
4747
import { Process } from "@/util/process"
48-
import { Cause, Effect, Exit, Layer, Option, Scope, Context, Schema } from "effect"
48+
import { Cause, Deferred, Effect, Exit, Layer, Option, Scope, Context, Schema } from "effect"
4949
import { zod } from "@/util/effect-zod"
5050
import { withStatics } from "@/util/schema"
5151
import * as EffectLogger from "@opencode-ai/core/effect/logger"
@@ -720,9 +720,10 @@ NOTE: At any point in time through this workflow you should feel free to ask the
720720
} satisfies MessageV2.TextPart)
721721
})
722722

723-
const shellImpl = Effect.fn("SessionPrompt.shellImpl")(function* (input: ShellInput) {
723+
const shellImpl = Effect.fn("SessionPrompt.shellImpl")(function* (input: ShellInput, ready?: Deferred.Deferred<void>) {
724724
return yield* Effect.uninterruptibleMask((restore) =>
725725
Effect.gen(function* () {
726+
const markReady = ready ? Deferred.succeed(ready, undefined).pipe(Effect.asVoid) : Effect.void
726727
const { msg, part, cwd } = yield* Effect.gen(function* () {
727728
const ctx = yield* InstanceState.context
728729
const session = yield* sessions.get(input.sessionID)
@@ -786,8 +787,9 @@ NOTE: At any point in time through this workflow you should feel free to ask the
786787
},
787788
}
788789
yield* sessions.updatePart(part)
790+
yield* markReady
789791
return { msg, part, cwd: ctx.directory }
790-
})
792+
}).pipe(Effect.onExit(() => markReady))
791793

792794
const cfg = yield* config.get()
793795
const sh = Shell.preferred(cfg.shell)
@@ -1508,7 +1510,8 @@ NOTE: At any point in time through this workflow you should feel free to ask the
15081510

15091511
const shell: (input: ShellInput) => Effect.Effect<MessageV2.WithParts> = Effect.fn("SessionPrompt.shell")(
15101512
function* (input: ShellInput) {
1511-
return yield* state.startShell(input.sessionID, lastAssistant(input.sessionID), shellImpl(input))
1513+
const ready = yield* Deferred.make<void>()
1514+
return yield* state.startShell(input.sessionID, lastAssistant(input.sessionID), shellImpl(input, ready), ready)
15121515
},
15131516
)
15141517

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { InstanceState } from "@/effect/instance-state"
22
import { Runner } from "@/effect/runner"
3-
import { Effect, Layer, Scope, Context } from "effect"
3+
import { Deferred, Effect, Layer, Scope, Context } from "effect"
44
import * as Session from "./session"
55
import { MessageV2 } from "./message-v2"
66
import { SessionID } from "./schema"
@@ -18,6 +18,7 @@ export interface Interface {
1818
sessionID: SessionID,
1919
onInterrupt: Effect.Effect<MessageV2.WithParts>,
2020
work: Effect.Effect<MessageV2.WithParts>,
21+
ready?: Deferred.Deferred<void>,
2122
) => Effect.Effect<MessageV2.WithParts>
2223
}
2324

@@ -95,8 +96,9 @@ export const layer = Layer.effect(
9596
sessionID: SessionID,
9697
onInterrupt: Effect.Effect<MessageV2.WithParts>,
9798
work: Effect.Effect<MessageV2.WithParts>,
99+
ready?: Deferred.Deferred<void>,
98100
) {
99-
return yield* (yield* runner(sessionID, onInterrupt)).startShell(work)
101+
return yield* (yield* runner(sessionID, onInterrupt)).startShell(work, ready)
100102
})
101103

102104
return Service.of({ assertNotBusy, cancel, ensureRunning, startShell })

packages/opencode/test/effect/runner.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,22 @@ describe("Runner", () => {
334334
}),
335335
)
336336

337+
it.live(
338+
"cancel does not mask shell defects",
339+
Effect.gen(function* () {
340+
const s = yield* Scope.Scope
341+
const runner = Runner.make<string>(s, { onInterrupt: Effect.succeed("interrupted") })
342+
343+
const sh = yield* runner
344+
.startShell(Effect.never.pipe(Effect.ensuring(Effect.die("boom")), Effect.as("ignored")))
345+
.pipe(Effect.forkChild)
346+
yield* Effect.sleep("10 millis")
347+
348+
yield* runner.cancel
349+
expect(Exit.isFailure(yield* Fiber.await(sh))).toBe(true)
350+
}),
351+
)
352+
337353
// --- shell→run handoff ---
338354

339355
it.live(

packages/opencode/test/session/prompt.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1470,6 +1470,10 @@ unix(
14701470

14711471
const exit = yield* Fiber.await(loop)
14721472
expect(Exit.isSuccess(exit)).toBe(true)
1473+
if (Exit.isSuccess(exit)) {
1474+
const tool = completedTool(exit.value.parts)
1475+
expect(tool?.state.output).toContain("User aborted the command")
1476+
}
14731477

14741478
yield* Fiber.await(sh)
14751479
}),

0 commit comments

Comments
 (0)