Skip to content

perf(opencode): overlap the pre-turn snapshot with the provider request #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
PierrotAWB wants to merge 1 commit into from
Closed

perf(opencode): overlap the pre-turn snapshot with the provider request #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions packages/opencode/src/session/processor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { LayerNode } from "@opencode-ai/core/effect/layer-node"
import { PermissionV1 } from "@opencode-ai/core/v1/permission"
import { Image } from "@/image/image"
import { SessionV1 } from "@opencode-ai/core/v1/session"
import { Cause, Deferred, Effect, Exit, Layer, Context, Scope, Schema } from "effect"
import { Cause, Deferred, Effect, Exit, Fiber, Layer, Context, Scope, Schema } from "effect"
import * as Stream from "effect/Stream"
import { Agent } from "@/agent/agent"
import { Config } from "@/config/config"
Expand Down Expand Up @@ -35,6 +35,7 @@ export interface Handle {
readonly requestStartAt: number | undefined
readonly firstRequestStartAt: number | undefined
readonly snapshotMs: number
readonly awaitSnapshot: Effect.Effect<string | undefined>
readonly updateToolCall: (
toolCallID: string,
update: (part: SessionV1.ToolPart) => SessionV1.ToolPart,
Expand Down Expand Up @@ -104,19 +105,14 @@ const layer = Layer.effect(
const database = yield* Database.Service

const create = Effect.fn("SessionProcessor.create")(function* (input: Input) {
// Pre-capture snapshot before the LLM stream starts. The AI SDK
// may execute tools internally before emitting start-step events,
// so capturing inside the event handler can be too late.
const snapshotStart = Date.now()
const initialSnapshot = yield* snapshot.track()
const ctx: ProcessorContext = {
assistantMessage: input.assistantMessage,
sessionID: input.sessionID,
model: input.model,
toolcalls: {},
shouldBreak: false,
snapshot: initialSnapshot,
snapshotMs: Date.now() - snapshotStart,
snapshot: undefined,
snapshotMs: 0,
blocked: false,
needsCompaction: false,
currentText: undefined,
Expand All @@ -127,6 +123,21 @@ const layer = Layer.effect(
}
let aborted = false

// Runs concurrently with the provider request; awaitSnapshot is the barrier
// every tool execution and step event takes before touching the result.
const snapshotStart = Date.now()
const snapshotFiber = yield* snapshot.track().pipe(
Effect.tap(() => Effect.sync(() => (ctx.snapshotMs = Date.now() - snapshotStart))),
Effect.orDie,
Effect.forkIn(scope),

@cubic-dev-ai cubic-dev-ai Bot Aug 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Effect.forkIn(scope) forks the snapshot fiber into the shared SessionProcessor layer scope, not a per-turn scope, so aborting a turn does not cancel that turn's snapshot git work as the PR description claims. The fiber keeps running git subprocesses after the turn ends, and since snapshot.track() is globally mutex-locked it can delay the next turn's snapshot. To get the claimed per-turn interruption, capture the current turn's scope (e.g. yield* Scope.Scope inside create) and fork into that.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/session/processor.ts, line 132:

<comment>`Effect.forkIn(scope)` forks the snapshot fiber into the shared SessionProcessor layer scope, not a per-turn scope, so aborting a turn does not cancel that turn's snapshot git work as the PR description claims. The fiber keeps running git subprocesses after the turn ends, and since `snapshot.track()` is globally mutex-locked it can delay the next turn's snapshot. To get the claimed per-turn interruption, capture the current turn's scope (e.g. `yield* Scope.Scope` inside `create`) and fork into that.</comment>

<file context>
@@ -127,6 +123,21 @@ const layer = Layer.effect(
+      const snapshotFiber = yield* snapshot.track().pipe(
+        Effect.tap(() => Effect.sync(() => (ctx.snapshotMs = Date.now() - snapshotStart))),
+        Effect.orDie,
+        Effect.forkIn(scope),
+      )
+      const awaitSnapshot = Effect.gen(function* () {
</file context>

)
const awaitSnapshot = Effect.gen(function* () {
if (ctx.snapshot === undefined) {

@cubic-dev-ai cubic-dev-ai Bot Aug 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: After the first provider step finishes, awaitSnapshot rejoins the already-completed initial snapshot fiber, so later step-start events record a stale pre-step snapshot. Rotate or re-track the barrier when ctx.snapshot is cleared so each provider step starts from the previous step's final tree.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/session/processor.ts, line 135:

<comment>After the first provider step finishes, `awaitSnapshot` rejoins the already-completed initial snapshot fiber, so later `step-start` events record a stale pre-step snapshot. Rotate or re-track the barrier when `ctx.snapshot` is cleared so each provider step starts from the previous step's final tree.</comment>

<file context>
@@ -127,6 +123,21 @@ const layer = Layer.effect(
+        Effect.forkIn(scope),
+      )
+      const awaitSnapshot = Effect.gen(function* () {
+        if (ctx.snapshot === undefined) {
+          ctx.snapshot = yield* Fiber.join(snapshotFiber)
+        }
</file context>

ctx.snapshot = yield* Fiber.join(snapshotFiber)
}
return ctx.snapshot
})

const parse = (e: unknown) =>
MessageV2.fromError(e, {
providerID: input.model.providerID,
Expand Down Expand Up @@ -436,7 +447,7 @@ const layer = Layer.effect(
throw new Error(value.message)

case "step-start":
if (!ctx.snapshot) ctx.snapshot = yield* snapshot.track()
yield* awaitSnapshot
yield* session.updatePart({
id: PartID.ascending(),
messageID: ctx.assistantMessage.id,
Expand Down Expand Up @@ -716,6 +727,7 @@ const layer = Layer.effect(
get snapshotMs() {
return ctx.snapshotMs
},
awaitSnapshot,
updateToolCall,
completeToolCall,
process,
Expand Down
4 changes: 3 additions & 1 deletion packages/opencode/src/session/tools.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: {
agent: Agent.Info
model: Provider.Model
session: Session.Info
processor: Pick<SessionProcessor.Handle, "message" | "updateToolCall" | "completeToolCall">
processor: Pick<SessionProcessor.Handle, "message" | "updateToolCall" | "completeToolCall" | "awaitSnapshot">
bypassAgentCheck: boolean
messages: SessionV1.WithParts[]
promptOps: TaskPromptOps
Expand Down Expand Up @@ -104,6 +104,8 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: {
return run.promise(
Effect.gen(function* () {
const ctx = context(args, options)
// Tools can run before any step event; the pre-turn snapshot must land before a tool mutates files.
yield* input.processor.awaitSnapshot

@cubic-dev-ai cubic-dev-ai Bot Aug 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When a generic MCP tool runs before step-start, this barrier does not protect it because only registry tools await the snapshot. A local MCP filesystem tool can therefore mutate the worktree before snapshot.track() finishes; await input.processor.awaitSnapshot in the MCP execute wrapper before the plugin hook and remote call.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/session/tools.ts, line 108:

<comment>When a generic MCP tool runs before `step-start`, this barrier does not protect it because only registry tools await the snapshot. A local MCP filesystem tool can therefore mutate the worktree before `snapshot.track()` finishes; await `input.processor.awaitSnapshot` in the MCP execute wrapper before the plugin hook and remote call.</comment>

<file context>
@@ -104,6 +104,8 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: {
           Effect.gen(function* () {
             const ctx = context(args, options)
+            // Tools can run before any step event; the pre-turn snapshot must land before a tool mutates files.
+            yield* input.processor.awaitSnapshot
             yield* plugin.trigger(
               "tool.execute.before",
</file context>

yield* plugin.trigger(
"tool.execute.before",
{ tool: item.id, sessionID: ctx.sessionID, callID: ctx.callID },
Expand Down
1 change: 1 addition & 0 deletions packages/opencode/test/session/compaction.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ function fake(
requestStartAt: undefined,
firstRequestStartAt: undefined,
snapshotMs: 0,
awaitSnapshot: Effect.succeed(undefined),
updateToolCall: Effect.fn("TestSessionProcessor.updateToolCall")(() => Effect.succeed(undefined)),
completeToolCall: Effect.fn("TestSessionProcessor.completeToolCall")(() => Effect.void),
process: Effect.fn("TestSessionProcessor.process")(() => Effect.succeed(result)),
Expand Down
Loading