From 042748a1b19aa1f7a069f2e7cc57429a5f017fda Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Mon, 27 Apr 2026 15:34:16 -0400 Subject: [PATCH] fix(session): close shell cancellation races --- packages/opencode/src/effect/runner.ts | 12 +++++++----- packages/opencode/src/session/prompt.ts | 11 +++++++---- packages/opencode/src/session/run-state.ts | 6 ++++-- packages/opencode/test/effect/runner.test.ts | 16 ++++++++++++++++ packages/opencode/test/session/prompt.test.ts | 4 ++++ 5 files changed, 38 insertions(+), 11 deletions(-) diff --git a/packages/opencode/src/effect/runner.ts b/packages/opencode/src/effect/runner.ts index e03cbffb11..bbc85309e8 100644 --- a/packages/opencode/src/effect/runner.ts +++ b/packages/opencode/src/effect/runner.ts @@ -4,7 +4,7 @@ export interface Runner { readonly state: State readonly busy: boolean readonly ensureRunning: (work: Effect.Effect) => Effect.Effect - readonly startShell: (work: Effect.Effect) => Effect.Effect + readonly startShell: (work: Effect.Effect, ready?: Deferred.Deferred) => Effect.Effect readonly cancel: Effect.Effect } @@ -19,6 +19,7 @@ interface RunHandle { interface ShellHandle { id: number cancelled: Deferred.Deferred + ready?: Deferred.Deferred fiber: Fiber.Fiber } @@ -106,6 +107,7 @@ export const make = ( const stopShell = (shell: ShellHandle) => Effect.gen(function* () { + if (shell.ready) yield* Deferred.await(shell.ready).pipe(Effect.exit, Effect.asVoid) yield* Deferred.succeed(shell.cancelled, undefined).pipe(Effect.asVoid) yield* Fiber.interrupt(shell.fiber) }) @@ -135,7 +137,7 @@ export const make = ( }), ).pipe(Effect.flatten) - const startShell = (work: Effect.Effect) => + const startShell = (work: Effect.Effect, ready?: Deferred.Deferred) => SynchronizedRef.modifyEffect( ref, Effect.fnUntraced(function* (st) { @@ -152,12 +154,12 @@ export const make = ( const id = next() const cancelled = yield* Deferred.make() const fiber = yield* work.pipe(Effect.ensuring(finishShell(id)), Effect.forkChild) - const shell = { id, cancelled, fiber } satisfies ShellHandle + const shell = { id, cancelled, ready, fiber } satisfies ShellHandle return [ Effect.gen(function* () { const exit = yield* Fiber.await(fiber) if (Exit.isSuccess(exit)) return exit.value - if ((yield* Deferred.isDone(cancelled)) || Cause.hasInterruptsOnly(exit.cause)) { + if (Cause.hasInterruptsOnly(exit.cause) || ((yield* Deferred.isDone(cancelled)) && !Cause.hasDies(exit.cause))) { if (onInterrupt) return yield* onInterrupt return yield* Effect.die(new Cancelled()) } @@ -192,8 +194,8 @@ export const make = ( case "ShellThenRun": return [ Effect.gen(function* () { - yield* Deferred.fail(st.run.done, new Cancelled()).pipe(Effect.asVoid) yield* stopShell(st.shell) + yield* Deferred.fail(st.run.done, new Cancelled()).pipe(Effect.asVoid) yield* idleIfCurrent() }), { _tag: "Idle" } as const, diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index a94f520c30..2f359610a3 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -45,7 +45,7 @@ import { AppFileSystem } from "@opencode-ai/core/filesystem" import { Truncate } from "@/tool/truncate" import { decodeDataUrl } from "@/util/data-url" import { Process } from "@/util/process" -import { Cause, Effect, Exit, Layer, Option, Scope, Context, Schema } from "effect" +import { Cause, Deferred, Effect, Exit, Layer, Option, Scope, Context, Schema } from "effect" import { zod } from "@/util/effect-zod" import { withStatics } from "@/util/schema" 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 } satisfies MessageV2.TextPart) }) - const shellImpl = Effect.fn("SessionPrompt.shellImpl")(function* (input: ShellInput) { + const shellImpl = Effect.fn("SessionPrompt.shellImpl")(function* (input: ShellInput, ready?: Deferred.Deferred) { return yield* Effect.uninterruptibleMask((restore) => Effect.gen(function* () { + const markReady = ready ? Deferred.succeed(ready, undefined).pipe(Effect.asVoid) : Effect.void const { msg, part, cwd } = yield* Effect.gen(function* () { const ctx = yield* InstanceState.context 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 }, } yield* sessions.updatePart(part) + yield* markReady return { msg, part, cwd: ctx.directory } - }) + }).pipe(Effect.onExit(() => markReady)) const cfg = yield* config.get() 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 const shell: (input: ShellInput) => Effect.Effect = Effect.fn("SessionPrompt.shell")( function* (input: ShellInput) { - return yield* state.startShell(input.sessionID, lastAssistant(input.sessionID), shellImpl(input)) + const ready = yield* Deferred.make() + return yield* state.startShell(input.sessionID, lastAssistant(input.sessionID), shellImpl(input, ready), ready) }, ) diff --git a/packages/opencode/src/session/run-state.ts b/packages/opencode/src/session/run-state.ts index 4b210d63d7..5ad95c9c7d 100644 --- a/packages/opencode/src/session/run-state.ts +++ b/packages/opencode/src/session/run-state.ts @@ -1,6 +1,6 @@ import { InstanceState } from "@/effect/instance-state" import { Runner } from "@/effect/runner" -import { Effect, Layer, Scope, Context } from "effect" +import { Deferred, Effect, Layer, Scope, Context } from "effect" import * as Session from "./session" import { MessageV2 } from "./message-v2" import { SessionID } from "./schema" @@ -18,6 +18,7 @@ export interface Interface { sessionID: SessionID, onInterrupt: Effect.Effect, work: Effect.Effect, + ready?: Deferred.Deferred, ) => Effect.Effect } @@ -95,8 +96,9 @@ export const layer = Layer.effect( sessionID: SessionID, onInterrupt: Effect.Effect, work: Effect.Effect, + ready?: Deferred.Deferred, ) { - return yield* (yield* runner(sessionID, onInterrupt)).startShell(work) + return yield* (yield* runner(sessionID, onInterrupt)).startShell(work, ready) }) return Service.of({ assertNotBusy, cancel, ensureRunning, startShell }) diff --git a/packages/opencode/test/effect/runner.test.ts b/packages/opencode/test/effect/runner.test.ts index 4b0fbc1b51..ee99050a8c 100644 --- a/packages/opencode/test/effect/runner.test.ts +++ b/packages/opencode/test/effect/runner.test.ts @@ -334,6 +334,22 @@ describe("Runner", () => { }), ) + it.live( + "cancel does not mask shell defects", + Effect.gen(function* () { + const s = yield* Scope.Scope + const runner = Runner.make(s, { onInterrupt: Effect.succeed("interrupted") }) + + const sh = yield* runner + .startShell(Effect.never.pipe(Effect.ensuring(Effect.die("boom")), Effect.as("ignored"))) + .pipe(Effect.forkChild) + yield* Effect.sleep("10 millis") + + yield* runner.cancel + expect(Exit.isFailure(yield* Fiber.await(sh))).toBe(true) + }), + ) + // --- shell→run handoff --- it.live( diff --git a/packages/opencode/test/session/prompt.test.ts b/packages/opencode/test/session/prompt.test.ts index 422c1400c9..5330569401 100644 --- a/packages/opencode/test/session/prompt.test.ts +++ b/packages/opencode/test/session/prompt.test.ts @@ -1470,6 +1470,10 @@ unix( const exit = yield* Fiber.await(loop) expect(Exit.isSuccess(exit)).toBe(true) + if (Exit.isSuccess(exit)) { + const tool = completedTool(exit.value.parts) + expect(tool?.state.output).toContain("User aborted the command") + } yield* Fiber.await(sh) }),