diff --git a/.github/workflows/test-fork.yml b/.github/workflows/test-fork.yml new file mode 100644 index 000000000000..32324733e45e --- /dev/null +++ b/.github/workflows/test-fork.yml @@ -0,0 +1,70 @@ +# The upstream `test` workflow asks for Blacksmith runners (`blacksmith-4vcpu-*`), which this fork +# cannot schedule: every run of it here, on every branch including `dev` and dependabot PRs, is +# cancelled with no runner ever assigned. So nothing in this fork has had a CI signal. +# +# This runs typecheck and the affected packages' unit tests on GitHub's own runners, so a change +# made here is checked by something other than the author's laptop. +# +# Deliberately narrower than upstream's, in two ways worth being explicit about: +# - Linux only, no Windows matrix, no e2e, no generated-client or HttpApi gates. Those belong to +# upstream's runners, and pretending to cover them here would be worse than not claiming to. +# - `bun turbo test` is NOT run, because `@opencode-ai/app` currently fails one locale-detection +# assertion (`detectDesktopNativeLocale(["pa-PK"])` returns "en", not "pa") that depends on the +# ICU data in the bun build. It reproduces on the untouched base branch, so a whole-monorepo +# test job would be red on arrival and worth nothing. Typecheck still covers every package. +name: test (fork) + +on: + pull_request: + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +env: + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true + +jobs: + unit: + name: unit + typecheck (linux) + runs-on: ubuntu-latest + defaults: + run: + shell: bash + steps: + - name: Checkout repository + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + + - name: Setup Node + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version: "24" + + - name: Setup Bun + uses: ./.github/actions/setup-bun + + # Some suites shell out to git, which refuses to run without an identity. + - name: Configure git identity + run: | + git config --global user.email "bot@opencode.ai" + git config --global user.name "opencode" + + # Concurrency capped: a full-width turbo run puts enough `tsgo` processes on a hosted runner + # at once that several are killed for memory, which reads as a code failure and is not one. + - name: Typecheck + timeout-minutes: 20 + run: bun turbo typecheck --concurrency=2 + + - name: Unit tests (core) + timeout-minutes: 30 + working-directory: packages/core + run: bun test + + - name: Unit tests (temporal) + timeout-minutes: 15 + working-directory: packages/temporal + run: bun test diff --git a/packages/core/src/event.ts b/packages/core/src/event.ts index 97a003bdf132..9d3145ae96d4 100644 --- a/packages/core/src/event.ts +++ b/packages/core/src/event.ts @@ -1,11 +1,13 @@ export * as EventV2 from "./event" -import { Cause, Context, Effect, Layer, Option, PubSub, Queue, Schema, Stream } from "effect" +import { Cause, Context, Duration, Effect, Layer, Option, PubSub, Queue, Schema } from "effect" +import { Stream } from "effect" import { Event } from "@opencode-ai/schema/event" import type { Data, Definition, Payload } from "@opencode-ai/schema/event" import { and, asc, eq, gt, inArray } from "drizzle-orm" import { Database } from "./database/database" import { EventSequenceTable, EventTable } from "./event/sql" +import { Flag } from "./flag/flag" import { Location } from "./location" import { makeGlobalNode } from "./effect/app-node" import { isDeepStrictEqual } from "node:util" @@ -174,6 +176,27 @@ export const allBounded = (events: Interface, capacity: number) => export interface LayerOptions { readonly beforeAggregateRead?: (aggregateID: string) => Effect.Effect + /** How often a durable tail re-reads on its own, on top of the in-process wake. The wake only + * fires for commits made in THIS process, so without a tick a subscriber cannot see events + * another process appended: a standalone worker's whole turn is invisible to a tail on the + * HTTP server. + * Zero (the default) relies on the wake alone, which is the whole story for a deployment that + * runs in one process. */ + readonly livePollInterval?: Duration.Input +} + +/** Chosen to be well under what a person notices in a transcript while staying one cheap indexed + * read per subscribed session. In-process commits still wake instantly; this only catches what the + * wake cannot see, so it is worth its cost only where another process writes: see `pollingNode`. */ +const DEFAULT_LIVE_POLL = Duration.seconds(1) + +// An operator's override, in milliseconds, for either node. Read at layer build rather than at +// import, so a test or a CLI that sets it late still gets it. +const configuredPoll = (fallback: Duration.Input): Duration.Input => { + const raw = Flag.OPENCODE_EVENT_POLL_MS + if (raw === undefined) return fallback + const millis = Number(raw) + return Number.isFinite(millis) && millis >= 0 ? millis : fallback } export const layerWith = (options?: LayerOptions) => @@ -186,7 +209,8 @@ export const layerWith = (options?: LayerOptions) => typed: new Map>(), } const projectors = new Map() - // TODO: Bind durable projectors to exact type+version before supporting incompatible historical payloads. + // TODO: Bind durable projectors to exact type+version before supporting incompatible + // historical payloads. const listeners = new Array() const { db } = yield* Database.Service @@ -619,7 +643,18 @@ export const layerWith = (options?: LayerOptions) => ), ) const historical = yield* read - const live = Stream.fromSubscription(wakes).pipe( + // Wake on either an in-process commit or the tick. A tick that finds nothing new reads + // zero rows and emits nothing, so an idle subscriber costs one indexed query per + // period. + const pollInterval = configuredPoll(options?.livePollInterval ?? Duration.zero) + const woken = Stream.fromSubscription(wakes) + // haltStrategy "left" keeps the wake stream as what ends the tail. The tick never ends, + // so the default ("both") would leave a subscriber hanging past the layer's own + // PubSub.shutdown, still reading from a database being torn down. + const source = Duration.isZero(Duration.fromInputUnsafe(pollInterval)) + ? woken + : woken.pipe(Stream.merge(Stream.tick(pollInterval), { haltStrategy: "left" })) + const live = source.pipe( Stream.mapEffect(() => read), Stream.flattenIterable, ) @@ -660,3 +695,13 @@ export const layerWith = (options?: LayerOptions) => const layer = layerWith() export const node = makeGlobalNode({ service: Service, layer: layer, deps: [Database.node] }) + +// For a deployment where the process serving subscribers is not the only one appending: a serve +// process driving sessions on standalone workers. The wake is published in-process, so without the +// tick a worker's whole turn is invisible to a tail on the server. Composition roots that know they +// are in that shape swap this in for `node`; everything else keeps the wake alone. +export const pollingNode = makeGlobalNode({ + service: Service, + layer: layerWith({ livePollInterval: DEFAULT_LIVE_POLL }), + deps: [Database.node], +}) diff --git a/packages/core/src/flag/flag.ts b/packages/core/src/flag/flag.ts index 20364dccdfdb..7b586bcb7db8 100644 --- a/packages/core/src/flag/flag.ts +++ b/packages/core/src/flag/flag.ts @@ -79,4 +79,10 @@ export const Flag = { get OPENCODE_CLIENT() { return process.env["OPENCODE_CLIENT"] ?? "cli" }, + // Milliseconds between a live event tail's own re-reads, `0` to rely on the in-process wake + // alone. Only a deployment where another process appends to the log needs it, so it is off unless + // the composition root asks for it; this overrides either way. + get OPENCODE_EVENT_POLL_MS() { + return process.env["OPENCODE_EVENT_POLL_MS"] + }, } diff --git a/packages/core/src/session/execution/conformance.ts b/packages/core/src/session/execution/conformance.ts index a9386a7ed724..c5e1c4b93c58 100644 --- a/packages/core/src/session/execution/conformance.ts +++ b/packages/core/src/session/execution/conformance.ts @@ -24,6 +24,8 @@ import { SessionProjector } from "@opencode-ai/core/session/projector" import { SessionExecution } from "@opencode-ai/core/session/execution" import { SessionExecutionLocal } from "@opencode-ai/core/session/execution/local" import { SessionRunnerModel, ModelNotSelectedError } from "@opencode-ai/core/session/runner/model" +import { ApplicationTools } from "@opencode-ai/core/tool/application-tools" +import { Tool } from "@opencode-ai/core/tool/tool" import { ToolOutputStore } from "@opencode-ai/core/tool-output-store" import { SessionTable } from "@opencode-ai/core/session/sql" import { SessionStore } from "@opencode-ai/core/session/store" @@ -39,7 +41,7 @@ import { Auth } from "@opencode-ai/llm/route" import { describe, expect } from "bun:test" import { realpathSync } from "node:fs" import { tmpdir } from "node:os" -import { Cause, Context, DateTime, Effect, Exit, Layer, Stream } from "effect" +import { Cause, Context, DateTime, Effect, Exit, Layer, Schema, Stream } from "effect" import { testEffect } from "../../testing/effect" // The per-location service build resolves the session directory on disk, so it must exist. @@ -77,13 +79,66 @@ const countingModel = () => { return { requests, stream } } +// Asks for one tool on the first request and answers on the second, which is the smallest turn that +// makes a driver dispatch a tool and come back to the model with its result. +const toolCallingModel = () => { + const requests: number[] = [] + const stream: LLMClientShape["stream"] = () => { + requests.push(1) + if (requests.length > 1) + return Stream.fromIterable([ + LLMEvent.stepStart({ index: 0 }), + LLMEvent.stepFinish({ index: 0, reason: "stop" }), + ]) + return Stream.fromIterable([ + LLMEvent.stepStart({ index: 0 }), + LLMEvent.toolCall({ id: "call_contract", name: "contract_probe", input: {} }), + LLMEvent.stepFinish({ index: 0, reason: "tool-calls" }), + ]) + } + return { requests, stream } +} + +// Counts its own executions, so "ran exactly once" is checked against the tool rather than only +// against the projection. +const probeTool = (ran: { count: number }) => + Tool.make({ + description: "contract probe", + input: Schema.Struct({}), + output: Schema.String, + execute: () => + Effect.sync(() => { + ran.count += 1 + return "probed" + }), + }) + +// A tool that never finishes, so a turn can be interrupted with one in flight. +const hangingTool = () => + Tool.make({ + description: "contract probe that never finishes", + input: Schema.Struct({}), + output: Schema.String, + execute: () => Effect.never, + }) + +const toolParts = (messages: ReadonlyArray) => + messages.flatMap((message) => + message.type === "assistant" + ? message.content.filter((part): part is SessionMessage.AssistantTool => part.type === "tool") + : [], + ) + // The executor under test, built as its own graph over the shared database file (the same way the // serve process builds it), with the model/LLM mocked. Any SessionExecution node with the standard // dependency set (the local coordinator, the Temporal driver) plugs in here. export const makeExecutionFor = (node: typeof SessionExecutionLocal.node) => (stream: LLMClientShape["stream"], models = okModels) => - AppNodeBuilder.build(node, [ + // The tool registry is named in the group so a scenario can register a probe into the same + // graph the driver runs on. One graph, one instance: what the test registers is what the + // dispatch finds. + AppNodeBuilder.build(LayerNode.group([node, ApplicationTools.node]), [ [LayerNodePlatform.llmClient, mockClient(stream)], [PermissionV2.node, permission], [ToolOutputStore.node, ToolOutputStore.nodeWithoutConfig], @@ -273,5 +328,81 @@ export const runContract = (label: string, makeExec: ReturnType + withIdleOverride( + Effect.gen(function* () { + yield* seedSession(sessionID) + yield* seedPrompt(sessionID) + const graph = yield* Layer.build(makeExec(stream)) + yield* Context.get(graph, ApplicationTools.Service).register({ + contract_probe: probeTool(ran), + }) + const exec = Context.get(graph, SessionExecution.Service) + yield* exec.wake(sessionID) + // Waiting on the turn, not on the step: the first step's assistant is completed the + // moment the step is sealed, while the follow-up provider call is still to come. + yield* until(exec.active, (active) => !active.has(sessionID)) + // Where the drivers could drift: a step's model call, its tool and its close are one + // activity in one mode and three in the other, so the tool has to be dispatched once, + // settled in the log, and answered by a second provider turn either way. + expect(ran.count).toBe(1) + expect(requests).toHaveLength(2) + const store = yield* SessionStore.Service + const context = yield* store.context(sessionID) + const parts = context.flatMap((message) => + message.type === "assistant" ? message.content : [], + ) + const call = parts.findLast( + (part): part is SessionMessage.AssistantTool => + part.type === "tool" && part.id === "call_contract", + ) + expect(call?.state.status).toBe("completed") + }), + ), + 60000, + ) + } + + { + const { stream } = toolCallingModel() + const sessionID = SessionV2.ID.make(`ses_${slug}_interrupt_tool`) + it.live("closes a tool call the interrupt cut short", () => + withIdleOverride( + Effect.gen(function* () { + yield* seedSession(sessionID) + yield* seedPrompt(sessionID) + const graph = yield* Layer.build(makeExec(stream)) + yield* Context.get(graph, ApplicationTools.Service).register({ + contract_probe: hangingTool(), + }) + const exec = Context.get(graph, SessionExecution.Service) + const store = yield* SessionStore.Service + yield* exec.wake(sessionID) + yield* until(store.context(sessionID), (context) => + toolParts(context).some((part) => part.state.status === "running"), + ) + + yield* exec.interrupt(sessionID) + + // A call left running is not a cosmetic loose end: the transcript shows a tool still + // going, and the next turn has to reconstruct what happened to it. A whole step closes + // the tools it opened on its way out, and a step whose calls are their own units of + // work has to end up in the same place. + yield* until(store.context(sessionID), (context) => + toolParts(context).every((part) => part.state.status !== "running"), + ) + const closed = toolParts(yield* store.context(sessionID)) + expect(closed.map((part) => part.state.status)).toEqual(["error"]) + yield* until(exec.active, (active) => !active.has(sessionID)) + }), + ), + 60000, + ) + } }) } diff --git a/packages/core/src/session/execution/worktree.ts b/packages/core/src/session/execution/worktree.ts index 060af00577c7..9d3532d95b8d 100644 --- a/packages/core/src/session/execution/worktree.ts +++ b/packages/core/src/session/execution/worktree.ts @@ -1,31 +1,39 @@ export * as WorktreeMaterializer from "./worktree" -// Rebuilds a missing project worktree from the shared store before a drain runs. This closes the -// host-local gap in cross-host resume: file tools need the tree, and a fresh worker does not have -// it. The capture side (snapshot-sync.ts) ships each snapshot as an incremental git pack; this -// side indexes every pack for the worktree into a fresh repo and checks out the newest tree. -// Ignored files and dependencies are not captured, so a bootstrap step (install, build) stays the -// project's own concern. +// Brings the project worktree to the newest state in the shared store before a drain runs. This +// closes the host-local gap in cross-host resume: file tools need the tree, and a worker that never +// ran an earlier step either has no tree at all or has one from the step it last ran. The capture +// side (snapshot-sync.ts) ships each snapshot as an incremental git pack; this side indexes every +// pack for the worktree and checks out the newest tree. Ignored files and dependencies are not +// captured, so a bootstrap step (install, build) stays the project's own concern. +// +// A tree is only ever moved forward, and only when this host's own note (snapshot/tip.ts) says it +// is behind the store. What that leaves open: two workers running tools of the SAME step on two +// hosts still cannot see each other's writes, because those are not captured until the step is +// sealed. One worker per worktree is what makes a step's tools share a tree. import { rm, writeFile } from "node:fs/promises" import path from "path" import { Cause, Context, Effect, Layer } from "effect" import { ChildProcess } from "effect/unstable/process" -import { asc, desc, eq } from "drizzle-orm" +import { and, asc, desc, eq } from "drizzle-orm" import { Database } from "../../database/database" import { makeGlobalNode } from "../../effect/app-node" import { KeyedMutex } from "../../effect/keyed-mutex" import { FSUtil } from "../../fs-util" import { Git } from "../../git" +import { Global } from "../../global" import { AppProcess } from "../../process" import { AbsolutePath } from "../../schema" import { SnapshotPackTable } from "../../snapshot/sql" +import { readWorktreeTip, writeWorktreeTip } from "../../snapshot/tip" export interface Interface { /** - * Make sure the session's directory exists, rebuilding its worktree from stored snapshot packs - * when it does not. A directory with no stored packs, or one whose worktree root already - * exists, is left alone. Never fails the caller. + * Make sure the session's directory holds the newest state the shared store has for it, + * rebuilding its worktree from stored snapshot packs when it is missing or behind. A directory + * with no stored packs, and a tree this host has neither built nor captured from, are left + * alone. Never fails the caller. */ readonly ensure: (directory: string) => Effect.Effect } @@ -34,11 +42,15 @@ export class Service extends Context.Service()( "@opencode/v2/WorktreeMaterializer", ) {} +// HEAD of a rebuilt tree, which doubles as the mark that says the tree is ours to move. +const RESTORED = "refs/heads/opencode-restore" + const layer = Layer.effect( Service, Effect.gen(function* () { const fs = yield* FSUtil.Service const git = yield* Git.Service + const global = yield* Global.Service const proc = yield* AppProcess.Service const { db } = yield* Database.Service const locks = KeyedMutex.makeUnsafe() @@ -79,7 +91,7 @@ const layer = Layer.effect( .run( ChildProcess.make( "git", - ["--git-dir", repository.gitDirectory, "update-ref", "refs/heads/opencode-restore", tip.id], + ["--git-dir", repository.gitDirectory, "update-ref", RESTORED, tip.id], { cwd: worktree, extendEnv: true }, ), ) @@ -88,11 +100,12 @@ const layer = Layer.effect( .run( ChildProcess.make( "git", - ["--git-dir", repository.gitDirectory, "symbolic-ref", "HEAD", "refs/heads/opencode-restore"], + ["--git-dir", repository.gitDirectory, "symbolic-ref", "HEAD", RESTORED], { cwd: worktree, extendEnv: true }, ), ) .pipe(Effect.ignore) + yield* writeWorktreeTip(global.data, tip.worktree, tip.tree) yield* Effect.logInfo("materialized worktree from snapshot packs", { worktree: tip.worktree, packs: rows.length, @@ -100,9 +113,51 @@ const layer = Layer.effect( }) }) + // Whether the tree on this host is older than what the store holds. No note means the tree was + // neither built from packs nor captured from here, so it belongs to whoever put it there. A + // note the store has never seen means a capture that never shipped, so this host holds work + // nothing else has and must not be moved back to an older tree. + const behind = Effect.fnUntraced(function* (tip: typeof SnapshotPackTable.$inferSelect) { + const held = yield* readWorktreeTip(global.data, tip.worktree) + if (!held || held === tip.tree) return false + const shipped = yield* db + .select({ time: SnapshotPackTable.time_created }) + .from(SnapshotPackTable) + .where(and(eq(SnapshotPackTable.worktree, tip.worktree), eq(SnapshotPackTable.tree, held))) + .orderBy(desc(SnapshotPackTable.time_created)) + .limit(1) + .get() + .pipe(Effect.orDie) + return shipped !== undefined && shipped.time < tip.time_created + }) + + // Whether this tree is one we built from packs. A checkout the host already had is somebody's + // working copy: reading its captures is fine, but checking a stored tree out over it would + // rewrite files and HEAD under whoever owns it. + const rebuilt = (worktree: string) => + proc + .run( + ChildProcess.make( + "git", + [ + "--git-dir", + path.join(worktree, ".git"), + "rev-parse", + "--verify", + "--quiet", + RESTORED, + ], + { cwd: worktree, extendEnv: true }, + ), + ) + .pipe( + Effect.map((result) => result.exitCode === 0), + Effect.catchCause(() => Effect.succeed(false)), + ) + const ensure = Effect.fn("WorktreeMaterializer.ensure")(function* (directory: string) { - if (yield* fs.existsSafe(directory)) return - // The newest capture whose session ran in this directory decides which worktree to rebuild. + // The newest capture whose session ran in this directory decides which worktree to rebuild, + // and which state a tree that is already here has to be brought to. const tip = yield* db .select() .from(SnapshotPackTable) @@ -112,18 +167,31 @@ const layer = Layer.effect( .get() .pipe(Effect.orDie) if (!tip) return + const present = yield* fs.existsSafe(tip.worktree) + if (present) { + if (!(yield* behind(tip))) return + if (!(yield* rebuilt(tip.worktree))) { + yield* Effect.logWarning("worktree is behind the store and was not built from it", { + worktree: tip.worktree, + tree: tip.tree, + }) + return + } + } yield* locks.withLock(tip.worktree)( Effect.gen(function* () { - // Re-check inside the lock (a concurrent drain may have rebuilt it), and never touch a - // worktree root that already exists: something else owns that tree. - if (yield* fs.existsSafe(tip.worktree)) return + // Re-check inside the lock: a concurrent drain may have done this already. + if ((yield* fs.existsSafe(tip.worktree)) && !(yield* behind(tip))) return yield* materialize(tip).pipe( Effect.catchCauseIf( (cause) => !Cause.hasInterrupts(cause), (cause) => Effect.gen(function* () { - // A half-built tree would pass the exists check forever; remove what we created. - yield* Effect.promise(() => rm(tip.worktree, { recursive: true, force: true })) + // A half-built tree would pass the exists check forever, so what we created is + // removed. A tree that was already here is not ours to remove: a failed refresh + // leaves it as stale as it was. + if (!present) + yield* Effect.promise(() => rm(tip.worktree, { recursive: true, force: true })) yield* Effect.logWarning("failed to materialize worktree", { worktree: tip.worktree, cause, @@ -142,5 +210,5 @@ const layer = Layer.effect( export const node = makeGlobalNode({ service: Service, layer, - deps: [Database.node, FSUtil.node, Git.node, AppProcess.node], + deps: [Database.node, FSUtil.node, Git.node, Global.node, AppProcess.node], }) diff --git a/packages/core/src/session/message-updater.ts b/packages/core/src/session/message-updater.ts index 46118a89fe4b..de50688b748a 100644 --- a/packages/core/src/session/message-updater.ts +++ b/packages/core/src/session/message-updater.ts @@ -208,6 +208,10 @@ export function update(adapter: Adapter, event: SessionEvent.Event) { }, "session.next.step.ended": (event) => { return updateOwnedAssistant(event.data.assistantMessageID, (draft) => { + // A step's writers share one owner token, so two seal attempts are both admitted. The + // tool cases guard on status; this one has to guard on the step already being closed, or + // a late attempt overwrites the end snapshot and file diff with a different instant. + if (draft.time.completed) return draft.time.completed = event.data.timestamp draft.finish = event.data.finish draft.cost = event.data.cost diff --git a/packages/core/src/session/runner/index.ts b/packages/core/src/session/runner/index.ts index e3ef80b67d07..2a63cf5ae9db 100644 --- a/packages/core/src/session/runner/index.ts +++ b/packages/core/src/session/runner/index.ts @@ -6,6 +6,7 @@ import { SessionSchema } from "../schema" import type { ContextSnapshotDecodeError, MessageDecodeError, SessionRunDeclinedError } from "../error" import type { SessionInput } from "../input" import { SessionRunnerModel } from "./model" +import type { StepSettlement } from "./publish-llm-event" import type { SystemContext } from "../../system-context/index" import type { ToolOutputStore } from "../../tool-output-store" @@ -35,6 +36,79 @@ export interface StepResult { readonly promotion: SessionInput.Delivery | undefined } +/** A tool call the provider asked for, recorded but not run, handed to the caller to dispatch. + * Every id comes from the provider or the publisher and is carried, never regenerated: a second run + * of the same step would mint different ones and the results would not match the log. */ +export interface DeferredToolCall { + readonly id: string + readonly name: string + readonly input: unknown + readonly assistantMessageID: string +} + +/** Closing one step whose tools have been dispatched. */ +export interface SealStepInput { + readonly sessionID: SessionSchema.ID + readonly step: number + /** The provider's finish reason and token counts, from the attempt that opened this step. They + * live only in that process's memory, so they have to be carried here rather than read back. */ + readonly settlement?: StepSettlement + /** The message the attempt opened. Absent only for a step that produced nothing to close. */ + readonly assistantMessageID?: string + /** Whether the turn should keep going, decided by the attempt. Only the attempt knows it hit a + * provider error, so a seal that re-derives this from the log would keep calling a provider that + * just failed. Absent on a re-drive, where the log is all there is. */ + readonly needsContinuation?: boolean +} + +/** One recorded tool call, to run on its own. */ +export interface ToolCallInput { + readonly sessionID: SessionSchema.ID + readonly call: DeferredToolCall +} + +/** How a dispatched call ended. It is the dispatch's own account of itself: the log says what the + * model was told, not whether a tool was skipped on purpose or its result was lost. A durable + * executor keeps it with the rest of the call's record, and reports the ones that are not `settled` + * where an operator reads about the turn. + * - `settled`: the tool ran and its result is durable. + * - `already-settled`: the log already had a result, so nothing ran. The at-least-once case. + * - `failed`: the tool ran and could not be settled. The reason reaches the model, and the call is + * closed, because a repeat would not fix it. + * - `unknown`: a second dispatch of a tool that does not declare itself repeatable. Reported to the + * model as an unknown outcome rather than run again. */ +export type ToolCallOutcome = "settled" | "already-settled" | "failed" | "unknown" + +export interface ToolCallResult { + readonly outcome: ToolCallOutcome +} + +/** What one provider attempt produced. `calls` is empty unless the caller deferred dispatch, in + * which case the step is left open and `settlement` is what seals it. */ +export interface TurnAttemptResult { + readonly needsContinuation: boolean + readonly step: number + readonly calls: ReadonlyArray + readonly settlement?: StepSettlement + /** The assistant message this attempt opened, minted here so a seal in another process closes the + * right one even when the turn published no content of its own. */ + readonly assistantMessageID?: string +} + +/** What a model-only attempt produced. `settled` means the step is already over (a crashed step was + * finalized from the log, or the recovery gate found no work) and there is nothing to dispatch. + * `called` hands back the recorded calls plus the settlement whoever seals the step will need. */ +export type ModelCallResult = + | { readonly kind: "settled"; readonly result: StepResult } + | { + readonly kind: "called" + readonly step: number + readonly calls: ReadonlyArray + readonly settlement?: StepSettlement + readonly assistantMessageID?: string + readonly needsContinuation?: boolean + } + /** Runs one local continuation from already-recorded Session history. */ export interface Interface { /** Drains eligible durable work. Explicit runs perform one provider attempt even when no work is eligible. */ @@ -45,6 +119,22 @@ export interface Interface { /** Run exactly one step and report the next loop state, so a caller (e.g. a Temporal workflow) * can drive the turn one step at a time. Mirrors one iteration of `run`'s loop. */ readonly runStep: (input: StepInput) => Effect.Effect + /** Run the provider attempt of one step and stop, handing back the tool calls it asked for + * instead of running them. The caller dispatches each one and then seals the step. This is what + * puts the + * model-to-tools loop in a durable executor's hands rather than inside a single activity. */ + readonly runModelCall: (input: StepInput) => Effect.Effect + /** Run one recorded tool call and publish its result. Safe to call twice for the same call: the + * second sees the settled result and does nothing. */ + readonly runToolCall: (input: ToolCallInput) => Effect.Effect + /** Close a call a stop cut short, so it does not sit in the log as running until the next turn. + * A whole step closes the tools it opened on its way out; a dispatch that is its own unit of work + * has to be told. A call that never started, and one that already settled, are left alone. */ + readonly failToolCall: (input: ToolCallInput) => Effect.Effect + /** Close a step once its calls have been dispatched: snapshot, file diff, Step.Ended, and the + * loop decision. Safe to call twice: the second sees the step already closed and returns the same + * answer without publishing again. */ + readonly sealStep: (input: SealStepInput) => Effect.Effect } export class Service extends Context.Service()("@opencode/v2/SessionRunner") {} diff --git a/packages/core/src/session/runner/llm.ts b/packages/core/src/session/runner/llm.ts index f49205e2b8ef..df79bfc3a1cc 100644 --- a/packages/core/src/session/runner/llm.ts +++ b/packages/core/src/session/runner/llm.ts @@ -26,15 +26,26 @@ import { ToolRegistry } from "../../tool/registry" import { ToolOutputStore } from "../../tool-output-store" import { SessionContextEpoch } from "../context-epoch" import { SessionCompaction } from "../compaction" +import { SessionRunDeclinedError } from "../error" import { SessionEvent } from "../event" import { SessionHistory } from "../history" import { SessionInput } from "../input" import { SessionMessage } from "../message" import { SessionSchema } from "../schema" import { SessionStore } from "../store" -import { type RunError, Service } from "./index" +import { + type DeferredToolCall, + type ModelCallResult, + type RunError, + type SealStepInput, + type StepInput, + type ToolCallInput, + type ToolCallResult, + type TurnAttemptResult, + Service, +} from "./index" import { SessionRunnerModel } from "./model" -import { createLLMEventPublisher, emitToolResult } from "./publish-llm-event" +import { createLLMEventPublisher, emitToolResult, record } from "./publish-llm-event" import { toLLMMessages } from "./to-llm-message" import { MAX_STEPS_PROMPT } from "./max-steps" import { DEFAULT_MAX_STEPS, REPEAT_LIMIT, REPEATED_CALLS_PROMPT, trailingIdenticalToolSteps } from "./loop-guard" @@ -50,7 +61,8 @@ import { llmClient } from "../../effect/app-node-platform" * `SessionPrompt` monolith. Implement the unchecked items in small reviewed slices: * * - Session ownership and controls - * - [x] Coordinate one local active drain per Session; explicit resumes join and prompt wakeups coalesce. + * - [x] Coordinate one local active drain per Session; explicit resumes join and prompt wakeups + * coalesce. * - [ ] Replace local ownership with durable multi-node ownership when clustered. * - [ ] Mark busy, retrying, idle, interrupted, or terminal-failure status durably. * - [ ] Honor interruption and reject stale work after runtime attachment replacement. @@ -76,7 +88,8 @@ import { llmClient } from "../../effect/app-node-platform" * - [x] Start each recorded local call eagerly and await all settlements before continuation. * - [ ] Add scoped runtime context, progress updates, attachment normalization, * plugins, and cancellation settlement. - * - [x] Reload projected history and start the next explicit provider turn after local tool results. + * - [x] Reload projected history and start the next explicit provider turn after local tool + * results. * - [x] Continue for durable user steering accepted during an active provider turn. * - [ ] Continue for compaction or another continuation condition when required. * @@ -88,9 +101,11 @@ import { llmClient } from "../../effect/app-node-platform" * Use `llm.stream(request)` for each provider turn. Keep tool execution and continuation here. * Durable continuation recovery remains a separate future slice with an explicit retry policy. * - * The current slice loads V2 history, translates it, resolves a model through a core service, and persists one + * The current slice loads V2 history, translates it, resolves a model through a core service, and + * persists one * provider turn. Registry definitions are advertised, local tool calls are settled durably, and an - * explicit loop starts the next provider turn after local settlement. Configured agent step limits bound the loop. + * explicit loop starts the next provider turn after local settlement. Configured agent step limits + * bound the loop. */ const layer = Layer.effect( @@ -167,7 +182,8 @@ const layer = Layer.effect( return false }) - // Match V1: declining a user prompt halts the loop instead of becoming model-facing tool output. + // Match V1: declining a user prompt halts the loop instead of becoming model-facing tool + // output. const isUserDeclined = (cause: Cause.Cause) => cause.reasons.some( (reason) => @@ -201,6 +217,11 @@ const layer = Layer.effect( promotion: SessionInput.Delivery | undefined, step: number, recoverOverflow?: typeof compaction.compactAfterOverflow, + // When set, tool calls are recorded and returned instead of run, and the step is left open + // for + // whoever runs them. This is what lets a durable executor make each call its own unit of + // work. + deferTools = false, ) { const session = yield* getSession(sessionID) if (session.location.directory !== location.directory || session.location.workspaceID !== location.workspaceID) @@ -208,6 +229,7 @@ const layer = Layer.effect( const agent = yield* agents.select(session.agent) const initialized = yield* SessionContextEpoch.initialize(db, loadSystemContext(agent), session.id) const toolFibers = yield* FiberSet.make() + const deferred: DeferredToolCall[] = [] let needsContinuation = false let currentStep = step if (promotion) { @@ -260,6 +282,7 @@ const layer = Layer.effect( ...(session.model?.variant === undefined ? {} : { variant: session.model.variant }), }, snapshot: startSnapshot, + deferCalls: deferTools, }) const withPublication = Semaphore.makeUnsafe(1).withPermit const publish = (event: LLMEvent, outputPaths: ReadonlyArray = []) => @@ -283,6 +306,20 @@ const layer = Layer.effect( } needsContinuation = true const assistantMessageID = yield* publisher.assistantMessageID(event.id) + // Tool.Called is already durable (the publish above), so handing the call back is + // enough + // for the caller to run it later. Nothing forks here, which is why the step ends when + // the + // stream does and the overlap between the model and its tools is lost. + if (deferTools) { + deferred.push({ + id: event.id, + name: event.name, + input: event.input, + assistantMessageID, + }) + return + } yield* Effect.uninterruptibleMask((restore) => restore( toolMaterialization.settle({ @@ -350,7 +387,9 @@ const layer = Layer.effect( yield* withPublication(publisher.failUnsettledTools(`Tool execution failed: ${message}`)) } const stepSettlement = publisher.stepSettlement() - if (stepSettlement && !publisher.hasProviderError()) { + // Deferred tools have not run yet, so the step is not over and the end snapshot would be + // taken before their side effects. Whoever runs them seals it, with the settlement below. + if (stepSettlement && !publisher.hasProviderError() && !deferTools) { const endSnapshot = yield* snapshots.capture() // Ship the post-step tree: this is the state a resumed step on another host needs. if (endSnapshot) yield* snapshotSync.push(endSnapshot) @@ -380,7 +419,25 @@ const layer = Layer.effect( if (stream._tag === "Failure") return yield* Effect.failCause(stream.cause) if (settled._tag === "Failure" && Cause.hasInterrupts(settled.cause)) return yield* Effect.failCause(settled.cause) - return { needsContinuation: !publisher.hasProviderError() && needsContinuation, step: currentStep } + // A provider turn can finish having published nothing at all (no text, no tool call), and + // then no assistant message exists for the seal to close. The whole-step path never hits + // this because it mints one right here, inside Step.Ended. Mint it the same way and carry + // the id, rather than leave the seal to guess from the projection. + const assistantMessageID = + deferTools && stepSettlement && !publisher.hasProviderError() + ? yield* withPublication(publisher.startAssistant()) + : undefined + return { + needsContinuation: !publisher.hasProviderError() && needsContinuation, + step: currentStep, + // A provider error already failed every recorded call, so handing them back would only + // buy a dispatch that reads them as settled. + calls: (publisher.hasProviderError() + ? [] + : deferred) as ReadonlyArray, + settlement: stepSettlement, + assistantMessageID, + } }), ) }, Effect.scoped) @@ -388,31 +445,53 @@ const layer = Layer.effect( sessionID: SessionSchema.ID, promotion: SessionInput.Delivery | undefined, step: number, - ) => Effect.Effect<{ readonly needsContinuation: boolean; readonly step: number }, RunError> + deferTools?: boolean, + ) => Effect.Effect - const runAfterOverflowCompaction: RunTurn = Effect.fnUntraced(function* (sessionID, promotion, step) { - return yield* runTurnAttempt(sessionID, promotion, step).pipe( + const runAfterOverflowCompaction: RunTurn = Effect.fnUntraced(function* ( + sessionID, + promotion, + step, + deferTools, + ) { + return yield* runTurnAttempt(sessionID, promotion, step, undefined, deferTools).pipe( Effect.catchDefect( Effect.fnUntraced(function* (defect) { if (!(defect instanceof TurnTransitionError)) return yield* Effect.die(defect) if (defect.transition._tag === "ContinueAfterOverflowCompaction") return yield* Effect.die("Post-compaction provider attempt cannot recover another overflow") yield* Effect.yieldNow - return yield* runAfterOverflowCompaction(sessionID, undefined, defect.transition.step) + return yield* runAfterOverflowCompaction( + sessionID, + undefined, + defect.transition.step, + deferTools, + ) }), ), ) }) - const runTurn: RunTurn = Effect.fnUntraced(function* (sessionID, promotion, step) { - return yield* runTurnAttempt(sessionID, promotion, step, compaction.compactAfterOverflow).pipe( + const runTurn: RunTurn = Effect.fnUntraced(function* (sessionID, promotion, step, deferTools) { + return yield* runTurnAttempt( + sessionID, + promotion, + step, + compaction.compactAfterOverflow, + deferTools, + ).pipe( Effect.catchDefect( Effect.fnUntraced(function* (defect) { if (!(defect instanceof TurnTransitionError)) return yield* Effect.die(defect) yield* Effect.yieldNow if (defect.transition._tag === "ContinueAfterOverflowCompaction") - return yield* runAfterOverflowCompaction(sessionID, undefined, defect.transition.step) - return yield* runTurn(sessionID, undefined, defect.transition.step) + return yield* runAfterOverflowCompaction( + sessionID, + undefined, + defect.transition.step, + deferTools, + ) + return yield* runTurn(sessionID, undefined, defect.transition.step, deferTools) }), ), ) @@ -448,12 +527,15 @@ const layer = Layer.effect( }) // Resume a crashed step from the durable log instead of re-streaming it. A Temporal step retry - // re-invokes runStep on the same log; if the in-flight step already DISPATCHED tools (Tool.Called - // is recorded before the side effect runs, so a running/completed tool may have run), re-streaming + // re-invokes runStep on the same log; if the in-flight step already DISPATCHED tools + // (Tool.Called + // is recorded before the side effect runs, so a running/completed tool may have run), + // re-streaming // would re-run that side effect and duplicate the assistant message. Instead we close the step // from the log: keep completed tool results, fail the ones still unsettled (their result never // committed -- we can't know if they ran, so the model redoes them), and publish a synthesized - // Step.Ended. The model is NOT re-called. Returns undefined when there is nothing to finalize (a + // Step.Ended. The model is NOT re-called. Returns undefined when there is nothing to finalize + // (a // fresh step, or a partial with no dispatched tools, which is safe to re-stream). Token/cost // metering is 0 for the resumed step only; faithful metering would need a durable step-sealed // marker carrying the provider usage. @@ -468,7 +550,8 @@ const layer = Layer.effect( // At most one assistant is in flight (the projector supersedes older ones); it only exists at // step entry on a re-drive, never on a fresh step. const inFlight = context.findLast( - (message): message is SessionMessage.Assistant => message.type === "assistant" && !message.time.completed, + (message): message is SessionMessage.Assistant => + message.type === "assistant" && !message.time.completed, ) if (!inFlight) return undefined const toolParts = inFlight.content.filter( @@ -532,33 +615,23 @@ const layer = Layer.effect( snapshot: endSnapshot, files, }) - // Mirror runStep's continuation tail: a step with local tool calls continues so the model sees - // the (reused or failed) results. - let needsContinuation = localTools - if (!needsContinuation) needsContinuation = yield* SessionInput.hasPending(db, input.sessionID, "steer") - if (needsContinuation) - return { ran: true, continue: true, step: input.step + 1, promotion: "steer" as SessionInput.Delivery } - const moreQueue = yield* SessionInput.hasPending(db, input.sessionID, "queue") - if (moreQueue) return { ran: true, continue: true, step: 1, promotion: "queue" as SessionInput.Delivery } - return { ran: true, continue: false, step: input.step + 1, promotion: undefined } + // A step with local tool calls continues so the model sees the (reused or failed) results. + return yield* stepContinuation(input.sessionID, localTools, input.step) }) // One iteration of `run`'s loop, exposed so a Temporal workflow can drive the turn one step at // a time (each step = one runTurn = one provider attempt + its tools). Semantics match `run`. - const runStep = Effect.fn("SessionRunner.runStep")(function* (input: { - readonly sessionID: SessionSchema.ID - readonly step: number - readonly promotion: SessionInput.Delivery | undefined - readonly first: boolean - readonly force: boolean - }) { + // The checks every step runs before the provider is called. Shared by the whole-step path and + // the model-only path so the two cannot drift apart. Returns the terminal result when the step + // is already over, otherwise the promotion the turn should apply. + const stepPrologue = Effect.fn("SessionRunner.stepPrologue")(function* (input: StepInput) { // One projected-history load serves all the entry checks; nothing mutates the projection // between them. The turn itself reloads after the first mutation. const entryContext = yield* getContext(input.sessionID) // Re-drive of a crashed step: finalize it from the log rather than re-calling the model and // re-running its already-dispatched tools. const resumed = yield* resumeCrashedStep(input, entryContext) - if (resumed) return resumed + if (resumed) return { kind: "settled", result: resumed } as const let promotion = input.promotion if (input.first) { const hasSteer = yield* SessionInput.hasPending(db, input.sessionID, "steer") @@ -572,27 +645,278 @@ const layer = Layer.effect( !hasQueue && !(yield* hasRecoverableWork(input.sessionID, entryContext)) ) - return { ran: false, continue: false, step: input.step, promotion: undefined } + return { + kind: "settled", + result: { ran: false, continue: false, step: input.step, promotion: undefined }, + } as const promotion = hasSteer ? "steer" : hasQueue ? "queue" : undefined } // Close tools left pending/running by an interrupted attempt before every turn, not just the // first. A mid-turn re-drive (first=false, from a Temporal step retry) would otherwise // re-stream a request with a dangling tool_use and no tool_result, which the provider rejects - // -- a retry poison loop. This is a no-op on a healthy step (the prior step settled its tools). + // -- a retry poison loop. This is a no-op on a healthy step (the prior step settled its + // tools). yield* failInterruptedTools(input.sessionID, entryContext) - const result = yield* runTurn(input.sessionID, promotion, input.step) - let needsContinuation = result.needsContinuation - if (!needsContinuation) needsContinuation = yield* SessionInput.hasPending(db, input.sessionID, "steer") + return { kind: "run", promotion } as const + }) + + // What the loop does once a step's provider attempt and its tools are done: a pending steer + // wins + // over a queued prompt, and either one keeps the turn going. Shared for the same reason as the + // prologue, and read after the tools have run so work admitted during the step is seen. + const stepContinuation = Effect.fn("SessionRunner.stepContinuation")(function* ( + sessionID: SessionSchema.ID, + hadToolCalls: boolean, + step: number, + ) { + let needsContinuation = hadToolCalls + if (!needsContinuation) + needsContinuation = yield* SessionInput.hasPending(db, sessionID, "steer") if (needsContinuation) - return { ran: true, continue: true, step: result.step + 1, promotion: "steer" as SessionInput.Delivery } - const moreQueue = yield* SessionInput.hasPending(db, input.sessionID, "queue") + return { + ran: true, + continue: true, + step: step + 1, + promotion: "steer" as SessionInput.Delivery, + } + const moreQueue = yield* SessionInput.hasPending(db, sessionID, "queue") if (moreQueue) return { ran: true, continue: true, step: 1, promotion: "queue" as SessionInput.Delivery } - return { ran: true, continue: false, step: result.step + 1, promotion: undefined } + return { ran: true, continue: false, step: step + 1, promotion: undefined } + }) + + const sealStep = Effect.fn("SessionRunner.sealStep")(function* (input: SealStepInput) { + const context = yield* getContext(input.sessionID) + const inFlight = context.findLast( + (message): message is SessionMessage.Assistant => + message.type === "assistant" && !message.time.completed, + ) + // On a retry that lands after Step.Ended was published there is nothing open, but the loop + // decision still has to come out the same, so it is read off the step we just closed. + const carried = input.assistantMessageID + const target = + (carried + ? context.findLast( + (m): m is SessionMessage.Assistant => m.type === "assistant" && m.id === carried, + ) + : undefined) ?? + inFlight ?? + context.findLast( + (message): message is SessionMessage.Assistant => message.type === "assistant", + ) + if (!target) return yield* stepContinuation(input.sessionID, false, input.step) + const toolParts = target.content.filter( + (part): part is SessionMessage.AssistantTool => part.type === "tool", + ) + // A step continues so the model can see its tool results. Provider-executed calls need no + // follow-up turn, so a step holding only those finalizes as a plain stop. + const localTools = toolParts.some((part) => part.provider?.executed !== true) + if (target.time.completed) + return yield* stepContinuation( + input.sessionID, + input.needsContinuation ?? localTools, + input.step, + ) + // A dispatch that failed outright leaves its call open. Close it here, or the next attempt + // sends a request carrying a tool_use with no tool_result and the provider rejects it. + yield* failInterruptedTools(input.sessionID, context) + const startSnapshot = target.snapshot?.start + const endSnapshot = yield* snapshots.capture() + // Ship the post-step tree: this is the state a later step on another host needs. + if (endSnapshot) yield* snapshotSync.push(endSnapshot) + const files = + startSnapshot && endSnapshot + ? yield* snapshots + .files({ from: Snapshot.ID.make(startSnapshot), to: endSnapshot }) + .pipe(Effect.catch(() => Effect.succeed(undefined))) + : undefined + yield* events.publish(SessionEvent.Step.Ended, { + sessionID: input.sessionID, + timestamp: yield* DateTime.now, + assistantMessageID: target.id, + // The attempt's own settlement when the caller carried it; otherwise the same fallback the + // crash-resume path uses, which costs only the metering on that step. + finish: input.settlement?.finish ?? (localTools ? "tool-calls" : "stop"), + cost: 0, + tokens: input.settlement?.tokens ?? { + input: 0, + output: 0, + reasoning: 0, + cache: { read: 0, write: 0 }, + }, + snapshot: endSnapshot, + files, + }) + return yield* stepContinuation( + input.sessionID, + input.needsContinuation ?? localTools, + input.step, + ) + }) + + + // A point read, not the whole projected history. The call carries the message that owns it, so + // decoding every message to find one part would make a step cost O(session) per tool instead + // of O(1). Message ids are looked up on their own, so the session has to be checked too: a call + // from another session must not resolve here. + const recordedCall = Effect.fnUntraced(function* (input: ToolCallInput) { + const owner = yield* store.message(SessionMessage.ID.make(input.call.assistantMessageID)) + if (owner?.sessionID !== input.sessionID || owner.message.type !== "assistant") + return undefined + // findLast to agree with the projector, which writes tool updates the same way. + return owner.message.content.findLast( + (item): item is SessionMessage.AssistantTool => + item.type === "tool" && item.id === input.call.id, + ) + }) + + const runToolCall = Effect.fn("SessionRunner.runToolCall")(function* (input: ToolCallInput) { + const session = yield* getSession(input.sessionID) + const assistantMessageID = SessionMessage.ID.make(input.call.assistantMessageID) + const part = yield* recordedCall(input) + // The call has to be in the log already: the attempt that produced it recorded its input + // before handing it over. Missing means the log moved under us (a fence), and running a tool + // whose call is not recorded would leave an orphan result. + if (!part || part.type !== "tool") + return yield* Effect.die( + `Tool call ${input.call.id} is not recorded on session ${input.sessionID}`, + ) + // At-least-once: a duplicate dispatch landing after the result did must not run anything. + if (part.state.status !== "pending" && part.state.status !== "running") + return { outcome: "already-settled" } as ToolCallResult + const agent = yield* agents.select(session.agent) + const materialization = yield* tools.materialize(agent.info?.permissions) + // `running` means a dispatch already published Tool.Called and was about to run the tool, so + // the side effect may have happened and nothing that reads the log afterwards can tell. Only + // a tool that declares itself repeatable runs again. The rest are reported unknown and the + // model decides, because re-running the `git push` that may already have landed is the worse + // failure. The attempt number would say the same thing far less precisely: it counts every + // way a dispatch can die, including the ones that never reached the tool. + if (part.state.status === "running" && !materialization.idempotent(input.call.name)) { + yield* events.publish(SessionEvent.Tool.Failed, { + sessionID: input.sessionID, + timestamp: yield* DateTime.now, + assistantMessageID, + callID: input.call.id, + error: { type: "unknown", message: "The outcome of this tool call is unknown" }, + provider: { executed: false }, + }) + return { outcome: "unknown" } as ToolCallResult + } + // The durable record that this call is being run, published before the tool can do anything. + // It is also the last point a fenced dispatch dies at: under a superseded owner this publish + // fails and the tool never runs, instead of running and losing its result. + yield* events.publish(SessionEvent.Tool.Called, { + sessionID: input.sessionID, + timestamp: yield* DateTime.now, + assistantMessageID, + callID: input.call.id, + tool: input.call.name, + input: record(input.call.input), + // Deferred calls are never provider-executed: those are filtered out before the hand-off. + provider: { executed: false }, + }) + const settlement = yield* materialization + .settle({ + sessionID: input.sessionID, + agent: agent.id, + assistantMessageID, + call: LLMEvent.toolCall({ + id: input.call.id, + name: input.call.name, + input: input.call.input, + }), + }) + .pipe( + // The tool itself ran: what failed is storing its output. Letting that fail the dispatch + // would retry a side effect that already happened and finally tell the model the call was + // interrupted, which is not what happened to it. The whole-step path reports the same + // reason the same way. A decline is a defect, so it passes through this untouched. + Effect.catch((error) => Effect.succeed({ failure: error })), + // A decline is the user stopping the turn, not a tool that failed. It is named here + // rather than raised as a bare interrupt, so nothing downstream has to infer from the + // absence of a cancellation what the user meant. + Effect.catchCause((cause) => + isUserDeclined(cause) + ? Effect.fail(new SessionRunDeclinedError({ sessionID: input.sessionID })) + : Effect.failCause(cause), + ), + ) + if ("failure" in settlement) { + const reason = + settlement.failure instanceof Error + ? settlement.failure.message + : String(settlement.failure) + yield* Effect.uninterruptible( + events.publish(SessionEvent.Tool.Failed, { + sessionID: input.sessionID, + timestamp: yield* DateTime.now, + assistantMessageID, + callID: input.call.id, + error: { type: "unknown", message: `Tool execution failed: ${reason}` }, + provider: { executed: false }, + }), + ) + return { outcome: "failed" } as ToolCallResult + } + // The tool has run by here, so losing the result to an interrupt would hide a + // side effect that already happened. + yield* Effect.uninterruptible(emitToolResult(events, { + sessionID: input.sessionID, + assistantMessageID, + callID: input.call.id, + result: settlement.result, + output: settlement.output, + outputPaths: settlement.outputPaths, + // Deferred calls are never provider-executed: those are filtered out before the hand-off. + provider: { executed: false }, + })) + return { outcome: "settled" } as ToolCallResult + }) + + const failToolCall = Effect.fn("SessionRunner.failToolCall")(function* (input: ToolCallInput) { + const part = yield* recordedCall(input) + // Only a call a dispatch had started is closed. One that never reached its tool is left + // pending for the next turn's entry check, and a settled one keeps the result it earned. + if (part?.state.status !== "running") return + yield* events.publish(SessionEvent.Tool.Failed, { + sessionID: input.sessionID, + timestamp: yield* DateTime.now, + assistantMessageID: SessionMessage.ID.make(input.call.assistantMessageID), + callID: input.call.id, + error: { type: "unknown", message: "Tool execution interrupted" }, + provider: { executed: false }, + }) + }) + + const runStep = Effect.fn("SessionRunner.runStep")(function* (input: StepInput) { + const prologue = yield* stepPrologue(input) + if (prologue.kind === "settled") return prologue.result + const result = yield* runTurn(input.sessionID, prologue.promotion, input.step) + return yield* stepContinuation(input.sessionID, result.needsContinuation, result.step) + }) + + const runModelCall = Effect.fn("SessionRunner.runModelCall")(function* (input: StepInput) { + const prologue = yield* stepPrologue(input) + if (prologue.kind === "settled") + return { kind: "settled", result: prologue.result } as ModelCallResult + const result = yield* runTurn(input.sessionID, prologue.promotion, input.step, true) + return { + kind: "called", + step: result.step, + calls: result.calls, + settlement: result.settlement, + assistantMessageID: result.assistantMessageID, + needsContinuation: result.needsContinuation, + } as ModelCallResult }) return Service.of({ run, runStep, + runModelCall, + runToolCall, + failToolCall, + sealStep, }) }), ) diff --git a/packages/core/src/session/runner/publish-llm-event.ts b/packages/core/src/session/runner/publish-llm-event.ts index 93dd3e86bb9c..d634b86e8771 100644 --- a/packages/core/src/session/runner/publish-llm-event.ts +++ b/packages/core/src/session/runner/publish-llm-event.ts @@ -11,6 +11,10 @@ type Input = { readonly agent: string readonly model: ModelV2.Ref readonly snapshot?: string + /** Record the calls the provider asked for, but leave Tool.Called to whoever dispatches them. + * The log then says which calls were only asked for and which one a process had started, which + * is the difference between re-running a side effect and reporting it as unknown. */ + readonly deferCalls?: boolean } const safe = (value: number | undefined) => Math.max(0, Number.isFinite(value) ? (value ?? 0) : 0) @@ -27,7 +31,16 @@ const tokens = (usage: Usage | undefined) => { } } -const record = (value: unknown): Record => +/** What the provider said about how the step ended. Held in memory by the publisher, so a caller + * that defers Step.Ended to another process has to carry it there itself. */ +export interface StepSettlement { + readonly finish: string + readonly tokens: ReturnType +} + +/** The shape a tool call's input is recorded in. Exported so a dispatcher publishing Tool.Called + * for a deferred call records it exactly as the streaming path would. */ +export const record = (value: unknown): Record => typeof value === "object" && value !== null && !Array.isArray(value) ? (value as Record) : { value } const message = (value: unknown) => { @@ -113,7 +126,7 @@ export const createLLMEventPublisher = (events: EventV2.Interface, input: Input) let assistantActive = false let assistantFailed = false let providerFailed = false - let stepSettlement: { readonly finish: string; readonly tokens: ReturnType } | undefined + let stepSettlement: StepSettlement | undefined const startAssistant = Effect.fnUntraced(function* () { if (assistantMessageID !== undefined) return assistantMessageID @@ -364,6 +377,9 @@ export const createLLMEventPublisher = (events: EventV2.Interface, input: Input) tool.called = true tool.providerExecuted = event.providerExecuted === true tool.providerMetadata = event.providerMetadata + // A provider-executed call has already run, so it is never deferred and is recorded here + // whatever the caller asked for. + if (input.deferCalls && !tool.providerExecuted) return yield* events.publish(SessionEvent.Tool.Called, { sessionID: input.sessionID, timestamp: yield* timestamp, diff --git a/packages/core/src/snapshot-sync.ts b/packages/core/src/snapshot-sync.ts index edab2106d124..ede416f8300a 100644 --- a/packages/core/src/snapshot-sync.ts +++ b/packages/core/src/snapshot-sync.ts @@ -21,6 +21,7 @@ import { AppProcess } from "./process" import { AbsolutePath } from "./schema" import type { Snapshot } from "./snapshot" import { SnapshotPackTable } from "./snapshot/sql" +import { writeWorktreeTip } from "./snapshot/tip" import { Hash } from "./util/hash" export interface Interface { @@ -62,6 +63,10 @@ const layer = Layer.effect( ) const push = Effect.fn("SnapshotSync.push")(function* (tree: Snapshot.ID) { + // Noted before the packing, which is best-effort: what this host holds is true whether or not + // the pack reaches the store, and a note left behind would let a later drain check out an + // older tree over work only this host has. + if (source) yield* writeWorktreeTip(global.data, worktree, tree) yield* Effect.gen(function* () { if (!source) return const latest = yield* db diff --git a/packages/core/src/snapshot/tip.ts b/packages/core/src/snapshot/tip.ts new file mode 100644 index 000000000000..06e563a28eac --- /dev/null +++ b/packages/core/src/snapshot/tip.ts @@ -0,0 +1,35 @@ +// The state a host's worktree is known to hold, kept in that host's own data directory. +// +// The shared store says what the newest captured tree is; it cannot say whether THIS host has it. +// The git objects cannot say either: a capture writes its tree into the side snapshot repository +// while a materialization indexes packs into the project repository, so "the tree is present" has +// two different answers depending on which host produced it. A host-local note is the one place +// both paths can agree on. +// +// It is written whenever this host captures (snapshot-sync) or rebuilds (execution/worktree), and +// read to answer one question: is this tree behind the store, or ahead of it? Being wrong towards +// "ahead" only leaves a stale tree; being wrong towards "behind" checks out over work nothing else +// holds, so an unreadable or missing note always means "leave the tree alone". + +import { mkdir, readFile, writeFile } from "node:fs/promises" +import path from "path" +import { Effect } from "effect" +import { Hash } from "../util/hash" + +const noteFile = (data: string, worktree: string) => + path.join(data, "worktree-tip", `${Hash.fast(worktree)}.tree`) + +export const readWorktreeTip = (data: string, worktree: string) => + Effect.promise(() => + readFile(noteFile(data, worktree), "utf8").then( + (text) => text.trim() || undefined, + () => undefined, + ), + ) + +export const writeWorktreeTip = (data: string, worktree: string, tree: string) => + Effect.promise(async () => { + const file = noteFile(data, worktree) + await mkdir(path.dirname(file), { recursive: true }).catch(() => {}) + await writeFile(file, tree).catch(() => {}) + }) diff --git a/packages/core/test/event.test.ts b/packages/core/test/event.test.ts index e5e71c9a81ad..d45b2311faca 100644 --- a/packages/core/test/event.test.ts +++ b/packages/core/test/event.test.ts @@ -1,5 +1,7 @@ import { describe, expect } from "bun:test" -import { Cause, DateTime, Deferred, Effect, Exit, Fiber, Layer, Option, Schema, Stream } from "effect" +import { + Cause, DateTime, Deferred, Effect, Exit, Fiber, Layer, Option, Schema, Scope, Stream, +} from "effect" import { EventV2 } from "@opencode-ai/core/event" import { Event } from "@opencode-ai/schema/event" import { Session } from "@opencode-ai/schema/session" @@ -1131,6 +1133,100 @@ describe("EventV2", () => { }), ) + it.effect("admits every writer holding the claimed token, concurrently", () => + Effect.gen(function* () { + const events = yield* EventV2.Service + const { db } = yield* Database.Service + const aggregateID = Session.ID.create() + // One step's writers share the token its model attempt claimed: the model call, each tool + // call, and the seal. They run as separate activities, so the fence has to admit all of them + // while still rejecting the attempt they superseded. + const step = "run-1:model-1:1" + const superseded = "run-1:model-1:0" + + yield* events.claim(aggregateID, step) + yield* Effect.all( + ["model", "tool-a", "tool-b", "seal"].map((writer) => + events + .publish(DurableMessage, durableData(aggregateID, writer)) + .pipe(Effect.provideService(EventV2.EventOwner, step)), + ), + { concurrency: "unbounded" }, + ) + const stale = yield* events + .publish(DurableMessage, durableData(aggregateID, "zombie")) + .pipe(Effect.provideService(EventV2.EventOwner, superseded), Effect.exit) + + const rows = yield* db + .select() + .from(EventTable) + .where(eq(EventTable.aggregate_id, aggregateID)) + .all() + .pipe(Effect.orDie) + + expect(String(stale)).toContain("Owner fence") + expect(rows.map((row) => row.seq)).toEqual([0, 1, 2, 3]) + expect(rows.map((row) => (row.data as { messageID: string }).messageID).sort()).toEqual( + ["model", "tool-a", "tool-b", "seal"] + .map((writer) => durableData(aggregateID, writer).messageID) + .sort(), + ) + }), + ) + + it.live("tails another writer's events only when the layer asked for a tick", () => + Effect.gen(function* () { + // A second service over the SAME database with its OWN pubsub: exactly what a standalone + // worker is to the HTTP server. Its commits cannot reach the reader's wake map, so a tail + // that only listens for wakes sits there forever. + const tailSees = (options: EventV2.LayerOptions) => + Effect.gen(function* () { + const reader = yield* EventV2.Service.pipe(Effect.provide(EventV2.layerWith(options))) + const writer = yield* EventV2.Service.pipe(Effect.provide(EventV2.layerWith())) + const aggregateID = Session.ID.create() + const tail = yield* reader + .durable({ aggregateID, after: -1 }) + .pipe(Stream.take(1), Stream.runCollect, Effect.forkScoped) + yield* Effect.sleep("50 millis") + yield* writer.publish(DurableMessage, durableData(aggregateID, "from-elsewhere")) + const collected = yield* Fiber.join(tail).pipe(Effect.timeoutOption("1 second")) + return Option.isSome(collected) + }) + + expect(yield* tailSees({ livePollInterval: "50 millis" })).toBe(true) + // Off unless asked for. One process wakes its own subscribers, so a tick there is a query per + // second per subscribed session for events that cannot exist. + expect(yield* tailSees({})).toBe(false) + }), + ) + + it.live("ends a durable tail when the event layer is released under it", () => + Effect.gen(function* () { + const aggregateID = Session.ID.create() + // The layer needs its own closeable scope, and the consumer has to be forked into a scope + // that + // OUTLIVES it. Fork into the test's own scope and closing that scope interrupts the consumer + // before the finalizer runs, so a hung tail and a killed one look the same. + const outer = yield* Effect.scope + const layerScope = yield* Scope.make() + const ctx = yield* Layer.buildWithScope(EventV2.layerWith(), layerScope) + const events = yield* EventV2.Service.pipe(Effect.provideContext(ctx as never)) + yield* events.publish(DurableMessage, durableData(aggregateID, "seed")) + + const tail = yield* events + .durable({ aggregateID, after: -1 }) + .pipe(Stream.runDrain, Effect.forkIn(outer)) + yield* Effect.sleep("150 millis") + yield* Scope.close(layerScope, Exit.void) + + // The tick never ends on its own, so it must not become what holds the tail open: a + // subscriber + // would outlive the layer and keep reading a database being torn down. + const settled = yield* Fiber.await(tail).pipe(Effect.timeout("5 seconds"), Effect.exit) + expect(Exit.isSuccess(settled)).toBe(true) + }), + ) + it.effect("never fences a publish made outside a drain", () => Effect.gen(function* () { const events = yield* EventV2.Service diff --git a/packages/core/test/session-runner-model-call.test.ts b/packages/core/test/session-runner-model-call.test.ts new file mode 100644 index 000000000000..b5527599eca1 --- /dev/null +++ b/packages/core/test/session-runner-model-call.test.ts @@ -0,0 +1,950 @@ +// The model-only attempt (L2): runModelCall performs one provider attempt, records the tool calls +// it +// asked for, and stops. The caller dispatches each call as its own unit of work and seals the step +// afterwards, which is what puts the model-to-tools loop in a durable executor rather than inside a +// single activity. These tests pin the three properties that split depends on: +// - the call is durable but not started, so its side effect has NOT run +// - the provider-minted callID and the publisher's assistantMessageID are handed back, never +// regenerated, so the dispatcher's result can be matched to the recorded call +// - the step is left open (no Step.Ended), because the tools have not run yet +// The last test is the contrast: the same stream through runStep runs the tool, as L1 does today. +import { LLMClient, type LLMClientShape } from "@opencode-ai/llm/route" +import { LLMEvent } from "@opencode-ai/llm" +import { Database } from "@opencode-ai/core/database/database" +import { EventTable } from "@opencode-ai/core/event/sql" +import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" +import { LayerNodePlatform } from "@opencode-ai/core/effect/app-node-platform" +import { LayerNode } from "@opencode-ai/core/effect/layer-node" +import { EventV2 } from "@opencode-ai/core/event" +import { PermissionV2 } from "@opencode-ai/core/permission" +import { AgentV2 } from "@opencode-ai/core/agent" +import { Config } from "@opencode-ai/core/config" +import { Project } from "@opencode-ai/core/project" +import { ProjectTable } from "@opencode-ai/core/project/sql" +import { AbsolutePath } from "@opencode-ai/core/schema" +import { SessionV2 } from "@opencode-ai/core/session" +import { Snapshot } from "@opencode-ai/core/snapshot" +import { SessionProjector } from "@opencode-ai/core/session/projector" +import { SessionRunner } from "@opencode-ai/core/session/runner" +import * as SessionRunnerLLM from "@opencode-ai/core/session/runner/llm" +import { SessionRunnerModel } from "@opencode-ai/core/session/runner/model" +import { ToolRegistry } from "@opencode-ai/core/tool/registry" +import { ApplicationTools } from "@opencode-ai/core/tool/application-tools" +import { Tool } from "@opencode-ai/core/tool/tool" +import { ToolOutputStore } from "@opencode-ai/core/tool-output-store" +import { SessionTable } from "@opencode-ai/core/session/sql" +import { SessionStore } from "@opencode-ai/core/session/store" +import { SessionMessage } from "@opencode-ai/core/session/message" +import { SessionEvent } from "@opencode-ai/core/session/event" +import { SessionRunDeclinedError } from "@opencode-ai/core/session/error" +import { DateTime } from "effect" +import { ConfigCompaction } from "@opencode-ai/core/config/compaction" +import { SessionContextEpoch } from "@opencode-ai/core/session/context-epoch" +import { createLLMEventPublisher } from "@opencode-ai/core/session/runner/publish-llm-event" +import { ModelV2 } from "@opencode-ai/core/model" +import { ProviderV2 } from "@opencode-ai/core/provider" +import { eq } from "drizzle-orm" +import { Location } from "@opencode-ai/core/location" +import { SystemContextRegistry } from "@opencode-ai/core/system-context/registry" +import { SystemContext } from "@opencode-ai/core/system-context" +import { SkillGuidance } from "@opencode-ai/core/skill/guidance" +import { ReferenceGuidance } from "@opencode-ai/core/reference/guidance" +import * as OpenAIChat from "@opencode-ai/llm/protocols/openai-chat" +import { Auth } from "@opencode-ai/llm/route" +import { describe, expect } from "bun:test" +import { Cause, Effect, Exit, Layer, Schema, Stream } from "effect" +import { testEffect } from "./lib/effect" + +const model = OpenAIChat.route + .with({ endpoint: { baseURL: "https://api.openai.com/v1" }, auth: Auth.bearer("fixture") }) + .model({ id: "gpt-4o-mini" }) +const models = SessionRunnerModel.layerWith(() => Effect.succeed(model)) +const systemContext = AppNodeBuilder.build(SystemContextRegistry.node) +const skillGuidance = Layer.mock(SkillGuidance.Service, { + load: () => Effect.succeed(SystemContext.empty), +}) +const referenceGuidance = Layer.mock(ReferenceGuidance.Service, { + load: () => Effect.succeed(SystemContext.empty), +}) +const config = Layer.succeed( + Config.Service, + Config.Service.of({ entries: () => Effect.succeed([]) }), +) +const permission = Layer.mock(PermissionV2.Service, {}) + +const mockClient = (stream: LLMClientShape["stream"]) => + Layer.succeed( + LLMClient.Service, + LLMClient.Service.of({ + prepare: () => Effect.die("LLMClient.prepare should not be called"), + generate: () => Effect.die("LLMClient.generate should not be called"), + stream, + }), + ) + +// One tool call, then a clean finish: the shape a step that wants to keep going produces. +const callsTool: LLMClientShape["stream"] = () => + Stream.fromIterable([ + LLMEvent.stepStart({ index: 0 }), + LLMEvent.toolCall({ id: "call_probe", name: "probe_write", input: {} }), + LLMEvent.stepFinish({ index: 0, reason: "tool-calls" }), + ]) +// Calls the tool that declines. +const callsDecliningTool: LLMClientShape["stream"] = () => + Stream.fromIterable([ + LLMEvent.stepStart({ index: 0 }), + LLMEvent.toolCall({ id: "call_probe", name: "probe_declines", input: {} }), + LLMEvent.stepFinish({ index: 0, reason: "tool-calls" }), + ]) +// A tool call, then the provider dies mid-stream. The calls are recorded but the turn is over. +const callsToolThenFails: LLMClientShape["stream"] = () => + Stream.fromIterable([ + LLMEvent.stepStart({ index: 0 }), + LLMEvent.toolCall({ id: "call_probe", name: "probe_write", input: {} }), + LLMEvent.providerError({ message: "upstream exploded" }), + ]) +// The same, for the tool that declares itself repeatable. +const callsIdempotentTool: LLMClientShape["stream"] = () => + Stream.fromIterable([ + LLMEvent.stepStart({ index: 0 }), + LLMEvent.toolCall({ id: "call_probe", name: "probe_read", input: {} }), + LLMEvent.stepFinish({ index: 0, reason: "tool-calls" }), + ]) +// Calls the tools that die with their side effect in flight, which is what a worker crash leaves +// behind for the next dispatch to read. +const callsCrashingTool: LLMClientShape["stream"] = () => + Stream.fromIterable([ + LLMEvent.stepStart({ index: 0 }), + LLMEvent.toolCall({ id: "call_probe", name: "probe_crashes", input: {} }), + LLMEvent.stepFinish({ index: 0, reason: "tool-calls" }), + ]) +const callsCrashingIdempotentTool: LLMClientShape["stream"] = () => + Stream.fromIterable([ + LLMEvent.stepStart({ index: 0 }), + LLMEvent.toolCall({ id: "call_probe", name: "probe_crashes_read", input: {} }), + LLMEvent.stepFinish({ index: 0, reason: "tool-calls" }), + ]) +// A provider turn that publishes nothing at all: no text, no reasoning, no tool call. The publisher +// mints the assistant message lazily on first content, so after this stream there is no message in +// the log for a seal to find. The whole-step path survives it because Step.Ended mints one on the +// way past; a seal running in another process has no publisher to mint with. +const silent: LLMClientShape["stream"] = () => + Stream.fromIterable([ + LLMEvent.stepStart({ index: 0 }), + LLMEvent.stepFinish({ index: 0, reason: "stop" }), + ]) +// An answer and no tool call: the step is over as soon as the stream is, but the seal still has to +// happen, and there is a real assistant message for it to complete. +const textOnly: LLMClientShape["stream"] = () => + Stream.fromIterable([ + LLMEvent.stepStart({ index: 0 }), + LLMEvent.textStart({ id: "txt_1" }), + LLMEvent.textDelta({ id: "txt_1", text: "done" }), + LLMEvent.textEnd({ id: "txt_1" }), + LLMEvent.stepFinish({ index: 0, reason: "stop" }), + ]) + +// A store that cannot keep what a tool produced. The tool itself still ran by then, which is the +// case a dispatch has to report honestly rather than as a call that never finished. +const failingOutputStore = Layer.mock(ToolOutputStore.Service, { + bound: () => + Effect.fail( + new ToolOutputStore.StorageError({ operation: "write", cause: new Error("disk full") }), + ), +}) + +const harness = ( + stream: LLMClientShape["stream"], + outputStore: + | typeof ToolOutputStore.nodeWithoutConfig + | Layer.Layer = ToolOutputStore.nodeWithoutConfig, +) => + testEffect( + AppNodeBuilder.build( + LayerNode.group([ + Database.node, + EventV2.node, + SessionProjector.node, + SessionStore.node, + AgentV2.node, + ToolRegistry.node, + SessionRunnerModel.node, + SystemContextRegistry.node, + SkillGuidance.node, + ReferenceGuidance.node, + Config.node, + Snapshot.node, + SessionRunnerLLM.node, + ApplicationTools.node, + ]), + [ + [LayerNodePlatform.llmClient, mockClient(stream)], + [PermissionV2.node, permission], + [ToolOutputStore.node, outputStore], + [SessionRunnerModel.node, models], + [SystemContextRegistry.node, systemContext], + [Location.node, Location.boundNode({ directory: AbsolutePath.make("/project") })], + [SkillGuidance.node, skillGuidance], + [ReferenceGuidance.node, referenceGuidance], + [Config.node, config], + [Snapshot.node, Snapshot.noopLayer], + ], + ), + ) + +const sessionID = SessionV2.ID.make("ses_runner_model_call") + +const seedSession = Effect.gen(function* () { + const { db } = yield* Database.Service + yield* db + .insert(ProjectTable) + .values({ id: Project.ID.global, worktree: AbsolutePath.make("/project"), sandboxes: [] }) + .onConflictDoNothing() + .run() + .pipe(Effect.orDie) + yield* db + .insert(SessionTable) + .values({ + id: sessionID, + project_id: Project.ID.global, + slug: "t", + directory: "/project", + title: "t", + version: "t", + }) + .onConflictDoNothing() + .run() + .pipe(Effect.orDie) +}) + +// Probes that count their own executions, so "recorded but not run" and "not run twice" are +// checked against the tools themselves rather than only against the projection. The read probes +// declare themselves repeatable; the write probes do not, which is what decides whether a second +// dispatch runs the tool again. +const registerProbes = (ran: { write: number; read: number }) => + Effect.gen(function* () { + yield* (yield* ApplicationTools.Service).register({ + probe_write: Tool.make({ + description: "write probe", + input: Schema.Struct({}), + output: Schema.String, + execute: () => + Effect.sync(() => { + ran.write += 1 + return "wrote" + }), + }), + // permission.assert declines by dying with this, so a tool that dies the same way exercises + // the same classification without needing the permission service in the tool's context. + probe_declines: Tool.make({ + description: "declines", + input: Schema.Struct({}), + output: Schema.String, + execute: () => Effect.die(new PermissionV2.DeclinedError()), + }), + probe_read: Tool.make({ + description: "read probe", + idempotent: true, + input: Schema.Struct({}), + output: Schema.String, + execute: () => + Effect.sync(() => { + ran.read += 1 + return "read" + }), + }), + // Dying with the side effect already done is the case the log has to survive: the call is + // left in flight and no later reader can say whether the write landed. + probe_crashes: Tool.make({ + description: "crashing write probe", + input: Schema.Struct({}), + output: Schema.String, + execute: () => + Effect.gen(function* () { + ran.write += 1 + return yield* Effect.die(new Error("worker died")) + }), + }), + // The repeatable one, dying only on its first run so a second dispatch has a result to + // produce. + probe_crashes_read: Tool.make({ + description: "crashing read probe", + idempotent: true, + input: Schema.Struct({}), + output: Schema.String, + execute: () => + Effect.gen(function* () { + ran.read += 1 + if (ran.read === 1) return yield* Effect.die(new Error("worker died")) + return "read" + }), + }), + }) + }) + +const counters = () => ({ write: 0, read: 0 }) + +const toolPart = (messages: ReadonlyArray, callID: string) => { + for (const message of messages) { + if (message.type !== "assistant") continue + for (const part of message.content) if (part.type === "tool" && part.id === callID) return part + } + return undefined +} + +const assistant = (messages: ReadonlyArray) => + messages.find((message) => message.type === "assistant") + +describe("SessionRunner model-only attempt", () => { + harness(callsTool).effect("records the tool call, hands it back, and does not run it", () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + const runner = yield* SessionRunner.Service + const store = yield* SessionStore.Service + + const result = yield* runner.runModelCall({ + sessionID, + step: 2, + promotion: undefined, + first: false, + force: false, + }) + + expect(result.kind).toBe("called") + if (result.kind !== "called") return + // The provider's callID and the publisher's assistant message id are carried out, because a + // second run of this step would mint different ones and nothing would match the log. + expect(result.calls.map((call) => call.id)).toEqual(["call_probe"]) + expect(result.calls[0]?.name).toBe("probe_write") + expect(result.calls[0]?.assistantMessageID).toBeTruthy() + // The provider's finish reason has to travel with the calls: it lives only in the publisher's + // memory, so whoever seals the step in another process cannot read it back from the log. + expect(result.settlement?.finish).toBe("tool-calls") + expect(ran.write).toBe(0) + + const context = yield* store.context(sessionID) + const part = toolPart(context, "call_probe") + // Durably recorded and not started: whoever dispatches the call publishes Tool.Called, so + // `running` in the log means a process was about to run the tool. Recording it here instead + // would make every later dispatch of a crashed step report an unknown outcome for a tool + // nobody had touched. + expect(part?.type === "tool" ? part.state.status : undefined).toBe("pending") + // The step stays open, so no Step.Ended and no completed assistant message. + const message = assistant(context) + expect(message?.type === "assistant" ? Boolean(message.time.completed) : true).toBe(false) + }), + ) + + harness(textOnly).effect("leaves a text-only step open with no calls to dispatch", () => + Effect.gen(function* () { + yield* seedSession + const runner = yield* SessionRunner.Service + const store = yield* SessionStore.Service + + const result = yield* runner.runModelCall({ + sessionID, + step: 2, + promotion: undefined, + first: false, + force: false, + }) + + expect(result.kind).toBe("called") + if (result.kind !== "called") return + expect(result.calls).toHaveLength(0) + expect(result.settlement?.finish).toBe("stop") + // Sealing is uniform: even with nothing to dispatch, the step is closed by the seal, not + // here, + // so the answer is recorded but its message is still open. + const message = assistant(yield* store.context(sessionID)) + expect(message?.type).toBe("assistant") + expect(message?.type === "assistant" ? Boolean(message.time.completed) : true).toBe(false) + }), + ) + + harness(callsTool).effect("still runs the tool and closes the step through runStep", () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + const runner = yield* SessionRunner.Service + const store = yield* SessionStore.Service + + yield* runner.runStep({ + sessionID, + step: 2, + promotion: undefined, + first: false, + force: false, + }) + + // The contrast that makes the split meaningful: the whole-step path dispatches and seals. + expect(ran.write).toBe(1) + const context = yield* store.context(sessionID) + const part = toolPart(context, "call_probe") + expect(part?.type === "tool" ? part.state.status : undefined).toBe("completed") + const message = assistant(context) + expect(message?.type === "assistant" ? Boolean(message.time.completed) : false).toBe(true) + }), + ) +}) + +// Dispatching one recorded call on its own. The policy under test is what happens when a dispatch +// has already started the tool and died, so the side effect may have run and nothing can say: +// repeatable tools run again, the rest are reported unknown so the model decides. Same rule the +// crash-resume path already follows, off the same evidence. +describe("SessionRunner tool dispatch", () => { + const deferOneCall = Effect.gen(function* () { + const runner = yield* SessionRunner.Service + const result = yield* runner.runModelCall({ + sessionID, + step: 2, + promotion: undefined, + first: false, + force: false, + }) + if (result.kind !== "called" || !result.calls[0]) throw new Error("expected a deferred call") + return result.calls[0] + }) + + harness(callsTool).effect("runs a deferred call and records its result", () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + const call = yield* deferOneCall + const runner = yield* SessionRunner.Service + const store = yield* SessionStore.Service + + const result = yield* runner.runToolCall({ sessionID, call }) + + expect(result.outcome).toBe("settled") + expect(ran.write).toBe(1) + const part = toolPart(yield* store.context(sessionID), "call_probe") + expect(part?.type === "tool" ? part.state.status : undefined).toBe("completed") + }), + ) + + harness(callsTool).effect("does nothing when the call already has a result", () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + const call = yield* deferOneCall + const runner = yield* SessionRunner.Service + + yield* runner.runToolCall({ sessionID, call }) + // A duplicate dispatch: at-least-once delivery means this happens, and it must not re-run. + const second = yield* runner.runToolCall({ sessionID, call }) + + expect(second.outcome).toBe("already-settled") + expect(ran.write).toBe(1) + }), + ) + + harness(callsCrashingTool).effect( + "reports a side-effecting call whose dispatch already started as unknown", + () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + const call = yield* deferOneCall + const runner = yield* SessionRunner.Service + const store = yield* SessionStore.Service + + // A dispatch that died with the tool in flight. What it leaves in the log is the whole + // evidence a later one gets: the call recorded as running, and no result. + const crashed = yield* runner.runToolCall({ sessionID, call }).pipe(Effect.exit) + expect(Exit.isFailure(crashed)).toBe(true) + expect(ran.write).toBe(1) + + const result = yield* runner.runToolCall({ sessionID, call }) + + expect(result.outcome).toBe("unknown") + expect(ran.write).toBe(1) + const part = toolPart(yield* store.context(sessionID), "call_probe") + expect(part?.type === "tool" ? part.state.status : undefined).toBe("error") + }), + ) + + harness(callsTool, failingOutputStore).effect( + "tells the model why a dispatched call failed", + () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + const call = yield* deferOneCall + const runner = yield* SessionRunner.Service + const store = yield* SessionStore.Service + + const result = yield* runner.runToolCall({ sessionID, call }) + + // The tool ran and only its output was lost. Letting that fail the dispatch would repeat + // the write on the next attempt, and the step would finally close the call as interrupted: + // a reason the model cannot act on, and not what happened to it. + expect(result.outcome).toBe("failed") + expect(ran.write).toBe(1) + const part = toolPart(yield* store.context(sessionID), "call_probe") + const failure = + part?.type === "tool" && part.state.status === "error" ? part.state.error : undefined + expect(failure?.message).toContain("disk full") + }), + ) + + harness(callsCrashingTool).effect("closes a started call that a stop cut short", () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + const call = yield* deferOneCall + const runner = yield* SessionRunner.Service + const store = yield* SessionStore.Service + const recorded = Effect.map(store.context(sessionID), (context) => { + const part = toolPart(context, "call_probe") + return part?.type === "tool" ? part.state : undefined + }) + + // Nothing has started this call, so it is left for the next turn's entry check: closing it + // here would report a tool the model asked for as having been cut short. + yield* runner.failToolCall({ sessionID, call }) + expect((yield* recorded)?.status).toBe("pending") + + // A dispatch died with the tool in flight, which is what the log shows after a stop lands + // mid-tool: the call recorded as running, with no result. + yield* runner.runToolCall({ sessionID, call }).pipe(Effect.exit) + expect((yield* recorded)?.status).toBe("running") + + yield* runner.failToolCall({ sessionID, call }) + const closed = yield* recorded + expect(closed?.status).toBe("error") + const reason = closed?.status === "error" ? closed.error.message : "" + expect(reason).toBe("Tool execution interrupted") + + // A call that is already terminal keeps what it has, whatever lands afterwards. + yield* runner.failToolCall({ sessionID, call }) + expect(yield* recorded).toEqual(closed) + }), + ) + + harness(callsCrashingIdempotentTool).effect( + "re-runs a started call that declares itself repeatable", + () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + const call = yield* deferOneCall + const runner = yield* SessionRunner.Service + const store = yield* SessionStore.Service + + yield* runner.runToolCall({ sessionID, call }).pipe(Effect.exit) + const result = yield* runner.runToolCall({ sessionID, call }) + + expect(result.outcome).toBe("settled") + expect(ran.read).toBe(2) + const part = toolPart(yield* store.context(sessionID), "call_probe") + expect(part?.type === "tool" ? part.state.status : undefined).toBe("completed") + }), + ) +}) + +// Closing the step after its calls have been dispatched. This is the piece that cannot stay in the +// provider attempt: the end snapshot and the file diff have to be taken after the tools have run, +// and in a durable executor that is a different process. +describe("SessionRunner step seal", () => { + const stepEndedCount = Effect.gen(function* () { + const { db } = yield* Database.Service + const rows = yield* db + .select({ type: EventTable.type }) + .from(EventTable) + .where(eq(EventTable.aggregate_id, sessionID)) + .all() + .pipe(Effect.orDie) + return rows.filter((row) => row.type.includes("step.ended")).length + }) + + const deferOneCall = Effect.gen(function* () { + const runner = yield* SessionRunner.Service + const result = yield* runner.runModelCall({ + sessionID, + step: 2, + promotion: undefined, + first: false, + force: false, + }) + if (result.kind !== "called") throw new Error("expected a deferred step") + return result + }) + + harness(callsTool).effect("closes a dispatched step and keeps the turn going", () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + const model = yield* deferOneCall + const runner = yield* SessionRunner.Service + const store = yield* SessionStore.Service + yield* runner.runToolCall({ sessionID, call: model.calls[0]! }) + + const result = yield* runner.sealStep({ sessionID, step: 2, settlement: model.settlement }) + + // Local tool calls mean the model has results to look at, so the turn continues. + expect(result.continue).toBe(true) + expect(result.step).toBe(3) + const message = assistant(yield* store.context(sessionID)) + expect(message?.type === "assistant" ? Boolean(message.time.completed) : false).toBe(true) + expect(yield* stepEndedCount).toBe(1) + }), + ) + + harness(textOnly).effect("closes a text-only step and ends the turn", () => + Effect.gen(function* () { + yield* seedSession + const model = yield* deferOneCall + const runner = yield* SessionRunner.Service + const store = yield* SessionStore.Service + + const result = yield* runner.sealStep({ sessionID, step: 2, settlement: model.settlement }) + + expect(result.continue).toBe(false) + const message = assistant(yield* store.context(sessionID)) + expect(message?.type === "assistant" ? Boolean(message.time.completed) : false).toBe(true) + }), + ) + + harness(callsTool).effect("seals once and answers the same on a repeat", () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + const model = yield* deferOneCall + const runner = yield* SessionRunner.Service + yield* runner.runToolCall({ sessionID, call: model.calls[0]! }) + + const first = yield* runner.sealStep({ sessionID, step: 2, settlement: model.settlement }) + // A seal that published Step.Ended and then died is retried. The loop decision has to survive + // that, or the turn would stop one step early. + const second = yield* runner.sealStep({ sessionID, step: 2, settlement: model.settlement }) + + expect(second).toEqual(first) + expect(yield* stepEndedCount).toBe(1) + }), + ) + + harness(callsTool).effect("closes a call the dispatcher never settled", () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + const model = yield* deferOneCall + const runner = yield* SessionRunner.Service + const store = yield* SessionStore.Service + + // The tool activity exhausted its retries and never published a result. Sealing has to close + // the call anyway: a request carrying a tool_use with no tool_result is rejected outright, so + // leaving it open would poison every later attempt. + const result = yield* runner.sealStep({ sessionID, step: 2, settlement: model.settlement }) + + expect(result.continue).toBe(true) + const part = toolPart(yield* store.context(sessionID), "call_probe") + expect(part?.type === "tool" ? part.state.status : undefined).toBe("error") + expect(ran.write).toBe(0) + }), + ) +}) + +// Regression: a content-free provider turn must still close. This failed 2 of 4 conformance +// scenarios when the split first ran against them, and the turn hung open forever. The conformance +// suite catches it but needs a dev server and an opt-in env var, so nothing in CI would. +describe("SessionRunner seal of a silent turn", () => { + harness(silent).effect("closes a turn that published no content of its own", () => + Effect.gen(function* () { + yield* seedSession + const runner = yield* SessionRunner.Service + const store = yield* SessionStore.Service + + const model = yield* runner.runModelCall({ + sessionID, + step: 2, + promotion: undefined, + first: false, + force: false, + }) + expect(model.kind).toBe("called") + if (model.kind !== "called") return + // The attempt has to hand the seal a message id, because there is nothing in the projection + // for it to find and it cannot mint one without a publisher. + expect(model.assistantMessageID).toBeTruthy() + + const result = yield* runner.sealStep({ + sessionID, + step: model.step, + settlement: model.settlement, + assistantMessageID: model.assistantMessageID, + }) + + expect(result.continue).toBe(false) + const message = assistant(yield* store.context(sessionID)) + expect(message?.type).toBe("assistant") + expect(message?.type === "assistant" ? Boolean(message.time.completed) : false).toBe(true) + }), + ) +}) + +// Compaction restarts the provider attempt by dying with a transition defect that runTurn catches +// and re-enters. That recursion has to carry `deferTools`, or a step that happens to compact +// silently falls back to running its tools inline and the caller never gets the calls it was meant +// to dispatch. A first version of this dropped the flag, so this is a real regression test. +describe("SessionRunner model-only attempt under compaction", () => { + // The summary request is the one with no tools. It needs text back, while the turn itself needs a + // tool call, so the mock has to answer them differently. + const compactingStream: LLMClientShape["stream"] = (request) => + request.tools.length === 0 + ? Stream.fromIterable([ + LLMEvent.stepStart({ index: 0 }), + LLMEvent.textStart({ id: "sum" }), + LLMEvent.textDelta({ id: "sum", text: "## Objective\n- keep going" }), + LLMEvent.textEnd({ id: "sum" }), + LLMEvent.stepFinish({ index: 0, reason: "stop" }), + ]) + : callsTool(request) + + const tightModel = SessionRunnerModel.layerWith(() => + Effect.succeed( + OpenAIChat.route + .with({ endpoint: { baseURL: "https://api.openai.com/v1" }, auth: Auth.bearer("fixture") }) + // Small enough that a seeded turn already overflows the headroom, so the attempt compacts. + .with({ limits: { context: 4_000, output: 50 } }) + .model({ id: "gpt-4o-mini" }), + ), + ) + const compactingConfig = Layer.succeed( + Config.Service, + Config.Service.of({ + entries: () => + Effect.succeed([ + new Config.Document({ + type: "document", + info: new Config.Info({ + compaction: new ConfigCompaction.Info({ + buffer: 3_000, + keep: new ConfigCompaction.Keep({ tokens: 1_000 }), + }), + }), + }), + ]), + }), + ) + + const compactHarness = testEffect( + AppNodeBuilder.build( + LayerNode.group([ + Database.node, + EventV2.node, + SessionProjector.node, + SessionStore.node, + AgentV2.node, + ToolRegistry.node, + SessionRunnerModel.node, + SystemContextRegistry.node, + SkillGuidance.node, + ReferenceGuidance.node, + Config.node, + Snapshot.node, + SessionRunnerLLM.node, + ApplicationTools.node, + ]), + [ + [LayerNodePlatform.llmClient, mockClient(compactingStream)], + [PermissionV2.node, permission], + [ToolOutputStore.node, ToolOutputStore.nodeWithoutConfig], + [SessionRunnerModel.node, tightModel], + [SystemContextRegistry.node, systemContext], + [Location.node, Location.boundNode({ directory: AbsolutePath.make("/project") })], + [SkillGuidance.node, skillGuidance], + [ReferenceGuidance.node, referenceGuidance], + [Config.node, compactingConfig], + [Snapshot.node, Snapshot.noopLayer], + ], + ), + ) + + compactHarness.effect("still defers dispatch after a compaction restart", () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + // The context epoch has to exist BEFORE the history is seeded. It records the sequence it was + // created at, and the runner only reads entries after that baseline, so seeding first would + // put the whole conversation behind the baseline and the request would come out empty. + const { db } = yield* Database.Service + yield* SessionContextEpoch.initialize(db, Effect.succeed(SystemContext.empty), sessionID) + // A finished turn already in the log, sized into a narrow band. It has to exceed the request + // headroom (context - buffer = 1000 tokens) so the attempt compacts at all, while the summary + // prompt it produces has to stay under context - summaryOutput (3950) or compaction bails out + // on its own guard and never restarts the attempt. + const events = yield* EventV2.Service + const seeder = createLLMEventPublisher(events, { + sessionID, + agent: "build", + model: { id: ModelV2.ID.make("gpt-4o-mini"), providerID: ProviderV2.ID.make("openai") }, + }) + yield* seeder.publish(LLMEvent.textStart({ id: "old" })) + yield* seeder.publish(LLMEvent.textDelta({ id: "old", text: "Earlier answer. ".repeat(500) })) + yield* seeder.publish(LLMEvent.textEnd({ id: "old" })) + yield* seeder.publish(LLMEvent.stepFinish({ index: 0, reason: "stop" })) + + const runner = yield* SessionRunner.Service + const result = yield* runner.runModelCall({ + sessionID, + step: 2, + promotion: undefined, + first: false, + force: false, + }) + + // Guard against a vacuous pass: without compaction actually firing this proves nothing about + // the restart path. + const rows = yield* db + .select({ type: EventTable.type }) + .from(EventTable) + .where(eq(EventTable.aggregate_id, sessionID)) + .all() + .pipe(Effect.orDie) + expect(rows.filter((row) => row.type.includes("compaction")).length).toBeGreaterThan(0) + + // The point: whatever the attempt went through, dispatch is still the caller's. + expect(result.kind).toBe("called") + if (result.kind !== "called") return + expect(result.calls.map((call) => call.id)).toEqual(["call_probe"]) + expect(ran.write).toBe(0) + }), + ) +}) + +// A turn has to stop when the provider fails. Only the attempt knows that happened: the log shows a +// failed assistant with tool parts, which reads the same as a step that wants to keep going. If the +// seal re-derives the decision instead of being told, a hard provider failure loops on the durable +// path until a step ceiling catches it. +describe("SessionRunner provider failure in a stepped turn", () => { + harness(callsToolThenFails).effect("ends the turn instead of asking for another step", () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + const runner = yield* SessionRunner.Service + + const model = yield* runner.runModelCall({ + sessionID, + step: 2, + promotion: undefined, + first: false, + force: false, + }) + + expect(model.kind).toBe("called") + if (model.kind !== "called") return + expect(model.needsContinuation).toBe(false) + // The attempt already failed them on the way out, so dispatching would only re-read settled + // parts. + expect(model.calls).toHaveLength(0) + + const result = yield* runner.sealStep({ + sessionID, + step: model.step, + settlement: model.settlement, + assistantMessageID: model.assistantMessageID, + needsContinuation: model.needsContinuation, + }) + + expect(result.continue).toBe(false) + expect(ran.write).toBe(0) + }), + ) +}) + +// A decline is the user stopping the turn, not one tool failing. If it reaches the dispatcher as an +// ordinary tool error it gets swallowed, the step seals, and the agent carries on past a refusal. +describe("SessionRunner declined permission in a stepped turn", () => { + harness(callsDecliningTool).effect("halts the turn rather than reporting a failed tool", () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + const runner = yield* SessionRunner.Service + const model = yield* runner.runModelCall({ + sessionID, + step: 2, + promotion: undefined, + first: false, + force: false, + }) + if (model.kind !== "called" || !model.calls[0]) throw new Error("expected a deferred call") + + const exit = yield* runner + .runToolCall({ sessionID, call: model.calls[0] }) + .pipe(Effect.exit) + + // Named, not inferred: the activity boundary turns this error into a non-retryable halt, + // where a plain tool failure reads as one bad tool and the turn continues. Raising a bare + // interrupt instead would leave the boundary to guess the user's decision from the absence + // of a cancellation. + expect(Exit.isFailure(exit)).toBe(true) + if (!Exit.isFailure(exit)) return + expect(Cause.squash(exit.cause)).toBeInstanceOf(SessionRunDeclinedError) + }), + ) +}) + +// A step's writers share one owner token, so the fence does not separate two seal attempts. The +// projector is what holds the line, and step.ended was the one event in that window with no guard. +describe("SessionRunner duplicate seal", () => { + harness(callsTool).effect("keeps the first close when a late attempt lands", () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + const runner = yield* SessionRunner.Service + const store = yield* SessionStore.Service + const model = yield* runner.runModelCall({ + sessionID, + step: 2, + promotion: undefined, + first: false, + force: false, + }) + if (model.kind !== "called" || !model.calls[0]) throw new Error("expected a deferred call") + yield* runner.runToolCall({ sessionID, call: model.calls[0] }) + const seal = { + sessionID, + step: model.step, + settlement: model.settlement, + assistantMessageID: model.assistantMessageID, + needsContinuation: model.needsContinuation, + } + yield* runner.sealStep(seal) + const closedAt = assistant(yield* store.context(sessionID)) + const first = String(closedAt?.type === "assistant" ? closedAt.time.completed : undefined) + + // A zombie attempt publishing its own Step.Ended under the same token is admitted by the + // fence, so the projection has to reject it. + const events = yield* EventV2.Service + yield* events.publish(SessionEvent.Step.Ended, { + sessionID, + timestamp: yield* DateTime.now, + assistantMessageID: SessionMessage.ID.make(model.assistantMessageID!), + finish: "stop", + cost: 0, + tokens: { input: 99, output: 99, reasoning: 0, cache: { read: 0, write: 0 } }, + }) + + const after = assistant(yield* store.context(sessionID)) + expect(String(after?.type === "assistant" ? after.time.completed : undefined)).toBe(first) + expect(after?.type === "assistant" ? after.tokens?.input : undefined).not.toBe(99) + }), + ) +}) diff --git a/packages/core/test/step-overlap-bench.test.ts b/packages/core/test/step-overlap-bench.test.ts new file mode 100644 index 000000000000..f84fcc8fcdce --- /dev/null +++ b/packages/core/test/step-overlap-bench.test.ts @@ -0,0 +1,238 @@ +// What the stepped split actually costs, measured rather than argued. +// +// A whole-step activity forks a tool fiber the moment a tool-call event arrives, so the tool runs +// while the model is still streaming. Splitting the step means the attempt has to return before any +// tool starts, because a workflow cannot consume a stream. The claim under test is that the loss is +// exactly the part of the stream that happened after the first tool call, bounded by how long the +// tool takes: +// +// overlap loss per step = min(stream tail after the first tool call, tool duration) +// +// A mock model and a sleeping tool are used deliberately: both are dialled, so the number is the +// overlap and nothing else. No provider latency, no network, no Temporal round trip. The live cost +// is this plus two extra activity round trips per step, which is measured separately. +// +// Uses the harness's `live` variant: `.effect` installs a TestClock, under which Effect.sleep never +// advances and both the tool and the stream tail would hang forever. +// +// Opt-in, because it sleeps: +// OPENCODE_OVERLAP_BENCH=1 bun test --timeout 60000 test/step-overlap-bench.test.ts +import { LLMClient, type LLMClientShape } from "@opencode-ai/llm/route" +import { LLMEvent } from "@opencode-ai/llm" +import { Database } from "@opencode-ai/core/database/database" +import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" +import { LayerNodePlatform } from "@opencode-ai/core/effect/app-node-platform" +import { LayerNode } from "@opencode-ai/core/effect/layer-node" +import { EventV2 } from "@opencode-ai/core/event" +import { PermissionV2 } from "@opencode-ai/core/permission" +import { AgentV2 } from "@opencode-ai/core/agent" +import { Config } from "@opencode-ai/core/config" +import { Project } from "@opencode-ai/core/project" +import { ProjectTable } from "@opencode-ai/core/project/sql" +import { AbsolutePath } from "@opencode-ai/core/schema" +import { SessionV2 } from "@opencode-ai/core/session" +import { Snapshot } from "@opencode-ai/core/snapshot" +import { SessionProjector } from "@opencode-ai/core/session/projector" +import { SessionRunner } from "@opencode-ai/core/session/runner" +import * as SessionRunnerLLM from "@opencode-ai/core/session/runner/llm" +import { SessionRunnerModel } from "@opencode-ai/core/session/runner/model" +import { ToolRegistry } from "@opencode-ai/core/tool/registry" +import { ApplicationTools } from "@opencode-ai/core/tool/application-tools" +import { Tool } from "@opencode-ai/core/tool/tool" +import { ToolOutputStore } from "@opencode-ai/core/tool-output-store" +import { SessionTable } from "@opencode-ai/core/session/sql" +import { SessionStore } from "@opencode-ai/core/session/store" +import { Location } from "@opencode-ai/core/location" +import { SystemContextRegistry } from "@opencode-ai/core/system-context/registry" +import { SystemContext } from "@opencode-ai/core/system-context" +import { SkillGuidance } from "@opencode-ai/core/skill/guidance" +import { ReferenceGuidance } from "@opencode-ai/core/reference/guidance" +import * as OpenAIChat from "@opencode-ai/llm/protocols/openai-chat" +import { Auth } from "@opencode-ai/llm/route" +import { describe, expect } from "bun:test" +import { Duration, Effect, Layer, Schema, Stream } from "effect" +import { testEffect } from "./lib/effect" + +// The dials. TAIL_MS is how much stream arrives after the first tool call; TOOL_MS is how long each +// tool takes. The predicted loss is min(TAIL_MS, TOOL_MS). +// `||` not `??`: an empty env var must fall back too, or the dials silently become NaN. +const num = (name: string, fallback: number) => Number(process.env[name] || fallback) +const TAIL_MS = num("BENCH_TAIL_MS", 1500) +const TOOL_MS = num("BENCH_TOOL_MS", 1500) +const TAIL_CHUNKS = TAIL_MS === 0 ? 0 : 3 +const TAIL_CHUNK_MS = TAIL_CHUNKS === 0 ? 0 : TAIL_MS / TAIL_CHUNKS + +const model = OpenAIChat.route + .with({ endpoint: { baseURL: "https://api.openai.com/v1" }, auth: Auth.bearer("fixture") }) + .model({ id: "gpt-4o-mini" }) +const models = SessionRunnerModel.layerWith(() => Effect.succeed(model)) +const systemContext = AppNodeBuilder.build(SystemContextRegistry.node) +const skillGuidance = Layer.mock(SkillGuidance.Service, { + load: () => Effect.succeed(SystemContext.empty), +}) +const referenceGuidance = Layer.mock(ReferenceGuidance.Service, { + load: () => Effect.succeed(SystemContext.empty), +}) +const config = Layer.succeed( + Config.Service, + Config.Service.of({ entries: () => Effect.succeed([]) }), +) +const permission = Layer.mock(PermissionV2.Service, {}) + +// One tool call, then a stream that keeps going for TAIL_MS. That trailing stream is the whole +// question: it is what a whole-step activity overlaps with the tool, and what a split cannot. +const withTail: LLMClientShape["stream"] = () => + Stream.concat( + Stream.fromIterable([ + LLMEvent.stepStart({ index: 0 }), + LLMEvent.toolCall({ id: "call_bench", name: "bench_sleep", input: {} }), + LLMEvent.textStart({ id: "txt_1" }), + ]), + Stream.concat( + Stream.fromIterable( + Array.from({ length: TAIL_CHUNKS }, (_, i) => + LLMEvent.textDelta({ id: "txt_1", text: `chunk ${i} ` }), + ), + ).pipe( + Stream.mapEffect((event) => + Effect.sleep(Duration.millis(TAIL_CHUNK_MS)).pipe(Effect.as(event)), + ), + ), + Stream.fromIterable([ + LLMEvent.textEnd({ id: "txt_1" }), + LLMEvent.stepFinish({ index: 0, reason: "tool-calls" }), + ]), + ), + ) + +const harness = testEffect( + AppNodeBuilder.build( + LayerNode.group([ + Database.node, + EventV2.node, + SessionProjector.node, + SessionStore.node, + AgentV2.node, + ToolRegistry.node, + SessionRunnerModel.node, + SystemContextRegistry.node, + SkillGuidance.node, + ReferenceGuidance.node, + Config.node, + Snapshot.node, + SessionRunnerLLM.node, + ApplicationTools.node, + ]), + [ + [ + LayerNodePlatform.llmClient, + Layer.succeed( + LLMClient.Service, + LLMClient.Service.of({ + prepare: () => Effect.die("unused"), + generate: () => Effect.die("unused"), + stream: withTail, + }), + ), + ], + [PermissionV2.node, permission], + [ToolOutputStore.node, ToolOutputStore.nodeWithoutConfig], + [SessionRunnerModel.node, models], + [SystemContextRegistry.node, systemContext], + [Location.node, Location.boundNode({ directory: AbsolutePath.make("/project") })], + [SkillGuidance.node, skillGuidance], + [ReferenceGuidance.node, referenceGuidance], + [Config.node, config], + [Snapshot.node, Snapshot.noopLayer], + ], + ), +) + +const seed = (sessionID: SessionV2.ID) => + Effect.gen(function* () { + const { db } = yield* Database.Service + yield* db + .insert(ProjectTable) + .values({ id: Project.ID.global, worktree: AbsolutePath.make("/project"), sandboxes: [] }) + .onConflictDoNothing() + .run() + .pipe(Effect.orDie) + yield* db + .insert(SessionTable) + .values({ + id: sessionID, + project_id: Project.ID.global, + slug: "t", + directory: "/project", + title: "t", + version: "t", + }) + .onConflictDoNothing() + .run() + .pipe(Effect.orDie) + yield* (yield* ApplicationTools.Service).register({ + bench_sleep: Tool.make({ + description: "sleeps", + input: Schema.Struct({}), + output: Schema.String, + execute: () => Effect.sleep(Duration.millis(TOOL_MS)).pipe(Effect.as("slept")), + }), + }) + }) + +const elapsed = (body: Effect.Effect) => + Effect.gen(function* () { + const started = yield* Effect.sync(() => performance.now()) + yield* body + return Math.round((yield* Effect.sync(() => performance.now())) - started) + }) + +describe.skipIf(!process.env.OPENCODE_OVERLAP_BENCH)("step overlap cost", () => { + harness.live("measures what deferring dispatch costs one step", () => + Effect.gen(function* () { + const runner = yield* SessionRunner.Service + const step = { step: 2, promotion: undefined, first: false, force: false } as const + + const fused = SessionV2.ID.make("ses_bench_fused") + yield* seed(fused) + // Whole step in one go: the tool forks the instant its call arrives, so it runs under the + // tail. + const fusedMs = yield* elapsed(runner.runStep({ sessionID: fused, ...step })) + + const split = SessionV2.ID.make("ses_bench_split") + yield* seed(split) + // The same step, dispatched by a caller: the attempt returns first, then the tool runs. + const splitMs = yield* elapsed( + Effect.gen(function* () { + const result = yield* runner.runModelCall({ sessionID: split, ...step }) + if (result.kind !== "called") return + for (const call of result.calls) + yield* runner.runToolCall({ sessionID: split, call }) + yield* runner.sealStep({ + sessionID: split, + step: result.step, + settlement: result.settlement, + }) + }), + ) + + const loss = splitMs - fusedMs + const predicted = Math.min(TAIL_MS, TOOL_MS) + console.log( + `\n stream tail after first call: ${TAIL_MS}ms tool: ${TOOL_MS}ms` + + `\n whole step : ${fusedMs}ms` + + `\n split step : ${splitMs}ms` + + `\n loss : ${loss}ms (predicted min(tail, tool) = ${predicted}ms)\n`, + ) + + // The fused path overlaps the tool with the tail, so it cannot exceed the sum. The slack is + // the step's own fixed cost (projection reads, snapshot, event writes), not overlap. + expect(fusedMs).toBeLessThan(TAIL_MS + TOOL_MS + 400) + // The loss is the overlap, within scheduling slop. Loose bounds: this is a measurement, and a + // tight assertion here would only buy a flaky test. With no tail there is nothing to overlap, + // so the floor does not apply. + if (predicted > 200) expect(loss).toBeGreaterThan(predicted * 0.5) + expect(loss).toBeLessThan(predicted + 400) + }), + ) +}) diff --git a/packages/core/test/worktree-materialize.test.ts b/packages/core/test/worktree-materialize.test.ts index 55274f129acd..beb0eba19186 100644 --- a/packages/core/test/worktree-materialize.test.ts +++ b/packages/core/test/worktree-materialize.test.ts @@ -33,9 +33,14 @@ const captureStack = (file: string, worktree: string, data: string) => [Global.node, Layer.succeed(Global.Service, Global.make({ data }))], ]) -// The resuming host: only the shared store, no location, no snapshot repo, no worktree. -const materializeStack = (file: string) => - AppNodeBuilder.build(WorktreeMaterializer.node, [[Database.node, Database.layerFromPath(file)]]) +// The resuming host: the shared store and its own data directory, no location, no snapshot repo, +// no worktree. The data directory is what makes two of these independent hosts: it holds the note +// saying which state that host's tree is at. +const materializeStack = (file: string, data: string) => + AppNodeBuilder.build(WorktreeMaterializer.node, [ + [Database.node, Database.layerFromPath(file)], + [Global.node, Layer.succeed(Global.Service, Global.make({ data }))], + ]) describe("WorktreeMaterializer", () => { it.live("rebuilds a deleted worktree from shared-store packs, incremental chain included", () => @@ -80,7 +85,7 @@ describe("WorktreeMaterializer", () => { // The fresh host: the tree is gone, only the shared store remains. yield* Effect.promise(() => rm(worktree, { recursive: true, force: true })) - const B = yield* Layer.build(materializeStack(file)) + const B = yield* Layer.build(materializeStack(file, path.join(root, "host-b-data"))) yield* WorktreeMaterializer.Service.use((w) => w.ensure(worktree)).pipe(Effect.provide(B)) const [tracked, untracked, extra] = yield* Effect.promise(() => @@ -104,6 +109,73 @@ describe("WorktreeMaterializer", () => { }), ) + it.live("moves a rebuilt tree forward, and leaves a tree it did not build alone", () => + Effect.gen(function* () { + const tmp = yield* Effect.promise(() => tmpdir()) + const root = realpathSync(tmp.path) + const worktree = path.join(root, "project") + const file = path.join(root, "shared.db") + const tracked = path.join(worktree, "tracked.txt") + const content = () => Effect.promise(() => readFile(tracked, "utf8")) + const put = (text: string) => Effect.promise(() => writeFile(tracked, text)) + yield* Effect.promise(async () => { + await mkdir(worktree, { recursive: true }) + await $`git init -q ${worktree}`.quiet() + await $`git -C ${worktree} config user.email t@t`.quiet() + await $`git -C ${worktree} config user.name t`.quiet() + await writeFile(tracked, "v1\n") + await $`git -C ${worktree} add .`.quiet() + await $`git -C ${worktree} commit -qm seed`.quiet() + }) + + const A = yield* Layer.build(captureStack(file, worktree, path.join(root, "host-a-data"))) + const ship = Effect.gen(function* () { + const tree = yield* Snapshot.Service.use((s) => s.capture()) + if (!tree) throw new Error("expected a capture to produce a tree") + yield* SnapshotSync.Service.use((s) => s.push(tree)) + }).pipe(Effect.provide(A)) + yield* ship + + // A checkout this host already had is somebody's working copy: however far behind the store + // it is, checking a stored tree out over it would rewrite files under whoever owns them. + const C = yield* Layer.build(materializeStack(file, path.join(root, "host-c-data"))) + yield* put("mine\n") + yield* WorktreeMaterializer.Service.use((w) => w.ensure(worktree)).pipe(Effect.provide(C)) + expect(yield* content()).toBe("mine\n") + + // Host B builds the tree from the store, which is what makes it B's to move. + yield* Effect.promise(() => rm(worktree, { recursive: true, force: true })) + const B = yield* Layer.build(materializeStack(file, path.join(root, "host-b-data"))) + const ensureB = WorktreeMaterializer.Service.use((w) => w.ensure(worktree)).pipe( + Effect.provide(B), + ) + yield* ensureB + expect(yield* content()).toBe("v1\n") + + // The store moves on while B's tree does not: another host captured a newer state, which is + // what a worker picking up a later step of the same session arrives to. + yield* Effect.sleep(10) + yield* put("v2\n") + yield* ship + yield* put("v1\n") + + yield* ensureB + + // Rebuilding only a missing tree is not enough: a present one is served as it was left, and + // the tools of the step read files from whichever step this worker last ran. + expect(yield* content()).toBe("v2\n") + + // A tree already at the newest state is left alone whatever is in it. This is what keeps the + // later tools of a step from checking out over what its earlier tools wrote, since nothing + // captures those until the step is sealed. + yield* put("uncaptured\n") + yield* ensureB + expect(yield* content()).toBe("uncaptured\n") + + yield* Effect.promise(() => tmp[Symbol.asyncDispose]()) + }), + ) + // The shared-store deployment uses the libsql backend, so the pack blob has to survive that // driver's parameter path too, not only bun's. it.live("round-trips a pack blob through the libsql backend", () => diff --git a/packages/server/src/routes.ts b/packages/server/src/routes.ts index 94117d000399..96e558065e2a 100644 --- a/packages/server/src/routes.ts +++ b/packages/server/src/routes.ts @@ -59,11 +59,15 @@ export function createServiceLayer() { // (execution/local.ts) -- no server, no ports. Both drive SessionRunner over the same durable // event log; the local coordinator owns the wake/resume/interrupt lifecycle and is shared with // the v1 server path, so it is the well-exercised default. - const executionNode = - process.env.OPENCODE_SESSION_EXECUTION === "temporal" - ? SessionExecutionTemporal.node - : SessionExecutionLocal.node - return AppNodeBuilder.build(applicationServices, [[SessionExecution.node, executionNode]]) + const temporal = process.env.OPENCODE_SESSION_EXECUTION === "temporal" + const executionNode = temporal ? SessionExecutionTemporal.node : SessionExecutionLocal.node + return AppNodeBuilder.build(applicationServices, [ + [SessionExecution.node, executionNode], + // Only the durable executor can put a turn in another process, and only then does a live tail + // need to re-read on its own: the wake it otherwise runs on is published in-process. Local mode + // keeps the wake alone, so the default deployment pays nothing for a case it does not have. + [EventV2.node, temporal ? EventV2.pollingNode : EventV2.node], + ]) } // The context for a standalone worker (src/worker.ts). Same services as serve, but with @@ -73,6 +77,7 @@ export function createWorkerLayer() { // A standalone worker only makes sense in temporal mode. return AppNodeBuilder.build(LayerNode.group([applicationServices, SessionExecution.node]), [ [SessionExecution.node, SessionExecutionTemporal.node], + [EventV2.node, EventV2.pollingNode], ]) } diff --git a/packages/temporal/README.md b/packages/temporal/README.md index 1de02f76b2b8..d2d8ac5a6f95 100644 --- a/packages/temporal/README.md +++ b/packages/temporal/README.md @@ -22,7 +22,8 @@ One env var picks the executor. `temporal` runs each session as a per-session Te with one activity per step (this package: `executor.ts` wires the client and worker, `supervisor.ts` is the loop, `workflow.ts` adapts it to the sandbox, `drain.ts` is the step body). The default runs in-process on the proven `SessionRunCoordinator` (core's `execution/local.ts`), the same lifecycle -the v1 server uses, with no server and no ports (see [Two modes, one runner](#two-modes-one-runner)). +the v1 server uses, with no server and no ports (see [Two modes, one +runner](#two-modes-one-runner)). What an executor must do is defined executably: core's conformance suite (`session/execution/conformance.ts`) runs the same wake/resume/interrupt scenarios against the local executor in core's tests and against this package through real workflows. @@ -59,11 +60,13 @@ exposes a substitutable `SessionExecution` service (`active` / `resume` / `wake` whose local impl comments "Future remote placement belongs here." This change provides a Temporal-backed `SessionExecution` in `packages/core/src/session/execution/`: -- `temporal-workflow.ts`: the pure per-session workflow (the Temporal equivalent of +- `packages/temporal/src/workflow.ts`: the pure per-session workflow (the Temporal equivalent of `SessionRunCoordinator`: `wake`/`force` drive one drain, wakes coalesce, quiescent runs end). -- `temporal-activities.ts`: the `runTurnStep` activity (heartbeats; forwards cancellation; +- `packages/temporal/src/activities.ts`: the `runTurnStep` activity (heartbeats; forwards + cancellation; injects the attempt's event-log owner token). -- `temporal.ts`: the `SessionExecution` layer + node: `wake` → `signalWithStart`, `resume` → +- `packages/temporal/src/executor.ts`: the `SessionExecution` layer + node: `wake` → + `signalWithStart`, `resume` → forced `signalWithStart`, `interrupt` → cancel signal; each drain runs one step of the local coordinator's loop (`SessionRunner.runStep`) in an activity against the durable event log. The Temporal client and an embedded worker are co-hosted in the server process (both run under bun). @@ -93,6 +96,296 @@ session runs as a Temporal workflow `session-exec-`. the in-flight step activity (attempt 2), the run continues from the event log, and the workflow completes. +### A step as three activities (`OPENCODE_TEMPORAL_STEPPED=1`) + +By default one step (a provider attempt plus every tool it asks for) is a single activity. That is +the smallest unit the runner used to expose, and it means nothing can sit between the model asking +for a tool and the tool running. `OPENCODE_TEMPORAL_STEPPED=1` splits a step into three kinds of +activity instead: + +``` +runModelCall -> runToolCall (one per call, concurrent) -> sealStep +``` + +`SessionRunner.runModelCall` performs the attempt, records each call as `Tool.Called`, and hands the +calls back rather than running them. `runToolCall` settles one call. `sealStep` takes the end +snapshot, diffs it against the start, and publishes `Step.Ended`. The loop between them is workflow +code, so a retry policy, a timeout, an approval or a budget can live where the model-to-tools +handoff +used to be. Each activity also carries its own bounds: sealing does not inherit a turn-sized +backstop, and one tool waiting on a human no longer holds the attempt and its sibling tools under a +single timeout. + +The supervisor is unchanged. Wake, interrupt, idle self-termination and continue-as-new only ever +called one `runTurnStep`, so the stepped mode supplies a different one. The mode rides the workflow +input, so a session that rolls over keeps it. + +Two things are load-bearing and easy to get wrong: + +- **One owner token per step, not per activity.** The event log fences a publish behind the current + owner, so a step's writers have to share one. Only `runModelCall` claims; the tool and seal + activities publish under the token it returns. A token per activity execution (right when the + activity *is* the whole step) would make them fence each other out. +- **A call that already started is not silently repeated.** Whether a side effect may have happened + is read off the log: `runToolCall` publishes `Tool.Called` before it runs the tool, so a call the + log shows as running is one a dispatch was already inside. Then only a tool declaring `idempotent` + runs again; anything else is reported to the model as an unknown outcome. This is the rule the + crash-resume path already followed, off the same evidence. + + The attempt number would answer the same question far less precisely. It counts every way a + dispatch can die, including the ones that never reached the tool, so a call nobody had touched + would come back to the model as an unknown outcome. Publishing the call at dispatch also moves the + fence in front of the side effect: under a superseded owner the publish fails and the tool never + runs, where before it ran and then lost its result. + + It is also what closes the zombie window, which the settled-result check on its own does not: that + check is a read then a write, so an attempt that lost its heartbeat but kept running could race a + retry past it. For the case that matters, a non-idempotent side effect running twice, it cannot + happen anyway, because the second dispatch refuses to run the tool at all. What can still race is + which truthful outcome reaches the model, the zombie's real result or the "unknown", and both + describe something that did happen. Reporting success for a tool that never ran is not reachable. +- **A stop closes the calls it cut short.** A whole step closes the tools it opened on its way out. + A call that is its own activity has nobody to do that, so an interrupted turn used to leave it + recorded as running until the next prompt: a transcript showing a tool still going, and an entry + check left to work out what happened to it. The dispatch closes its own call when the cancellation + means the turn is over. When it means this attempt is being handed on (a worker shutting down, a + pause, a heartbeat timeout), the call is left exactly as it was, or the next attempt would read a + closed call and never run the tool. +- **A refusal is named, not inferred.** A declined permission crosses the activity boundary as its + own error type. The whole-step path still raises it as an interrupt, because halting the local + loop that way is the behaviour V1 defined and its tests pin, so that drain says so at the boundary + (`declineIsInterrupt`). What the boundary no longer does is read every interrupt with nothing + cancelling it as a refusal: a runner that stopped itself for another reason used to be reported as + a decision the user never made. + +#### What it costs, measured + +Two separate costs, and only one of them is usually real. + +**The lost overlap.** A whole-step activity starts each tool the moment the model asks for it, while +the stream is still going. Here the attempt has to return before any tool starts, because a workflow +cannot consume a stream. `packages/core/test/step-overlap-bench.test.ts` dials a mock model's stream +tail and a sleeping tool, so the number is the overlap and nothing else: + +| stream tail after 1st call | tool | whole step | split step | loss | `min(tail, tool)` | +|---:|---:|---:|---:|---:|---:| +| 1 ms | 1500 ms | 1534 ms | 1519 ms | **-15 ms** | 1 ms | +| 200 ms | 1500 ms | 1538 ms | 1734 ms | **196 ms** | 200 ms | +| 1500 ms | 200 ms | 1534 ms | 1743 ms | **209 ms** | 200 ms | +| 1500 ms | 1500 ms | 1536 ms | 3045 ms | **1509 ms** | 1500 ms | +| 3000 ms | 1500 ms | 3038 ms | 4542 ms | **1504 ms** | 1500 ms | + +So the loss per step is `min(stream tail after the first tool call, tool duration)`, within about +10 ms every time. It is zero when the tool call is the last thing in the stream, and that turns out +to be what real providers do. + +`packages/temporal/scripts/stream-tail-probe.ts` measures the tail against the live API, timed from +the point the runner actually forks: the `tool-call` event, which the protocol layer emits once a +call's arguments are complete and parsed, not when its id first appears. Six coding-agent-shaped +prompts, including ones asking for three and five tools at once and ones asking the model to narrate +around the call: + +| model | median tail | max tail | text after the first call | +|---|---:|---:|---| +| `gpt-4o-mini` (chat) | 0 ms | 50 ms | none, in any probe | +| `gpt-5-mini` (responses) | 33 ms | 36 ms | none, in any probe | +| `gpt-5` (responses) | 34 ms | 77 ms | none, in any probe | + +A single tool call *is* the end of the stream, so there is nothing to overlap. A tail appears only +when the model asks for several tools at once, and is then just the time to stream calls 2..N: tens +of milliseconds, one to three percent of the stream. No model emitted a single character of text +after asking for its first tool. + +Taken with the hand-offs below, the whole cost of the split is roughly 40-110 ms per step against +model calls of two to eight seconds. The overlap is not a reason to avoid it. + +**The extra round trips.** Three activities per step instead of one means two more hand-offs. +Measured from workflow history on a loopback dev server (mean of four, one turn): + +``` +done runModelCall -> sched runToolCall 10 ms +done runToolCall -> sched sealStep 3 ms +done sealStep -> sched runModelCall 4 ms +done runModelCall -> sched sealStep 3 ms +``` + +About 5 ms per hand-off, so ~10 ms per step, against model calls of 1.2 s and 3.2 s in the same run. +Per-step Temporal overhead tracks worker-to-namespace distance, so this is the floor: it grows with +placement, and a laptop driving a remote namespace pays it many times over. Put workers next to the +namespace and the split is close to free. + +Wall-clock totals are deliberately not quoted here. Model latency dominates and varies more between +two runs of the same cell than the effect being measured. + +#### Verified + +Live against a dev server and `gpt-5-mini`, with `OPENCODE_SESSION_EXECUTION=temporal` and +`OPENCODE_TEMPORAL_STEPPED=1`: + +- A turn using one tool recorded five activities: `runModelCall`, `runToolCall`, `sealStep` for the + step that called the tool, then `runModelCall`, `sealStep` for the answer. The tool ran once. +- Crash mid-tool: a turn ran `echo ... >> counter.txt` in one step and `sleep 60` in the next, and + the serve process (with its embedded worker) was killed with the sleep in flight. On a fresh + worker the interrupted `runToolCall` came back as **attempt 2**, `counter.txt` still held one + line (the settled tool was not re-run), the interrupted call reached the model as "The outcome of + this tool call is unknown", and the turn ran on to its answer. + +#### What has been exercised, and what has not + +Verified live (dev server, `gpt-5-mini`, stepped mode on): + +- **A step's calls fan out and share one log owner.** Four `read` calls in one step became four + concurrent `runToolCall` activities, started within 2 ms of each other and overlapping, all four + results durable, zero activity failures. A fenced write would have died and shown as a failed + activity, so this is the owner-token design holding under real concurrency, which is the thing it + exists for. +- **Interrupt stops the turn, not the session.** With `sleep 90` in flight, `POST /interrupt` + returned 204, the child process died, the call was closed as `Tool execution interrupted` so the + next attempt is not a poisoned request, the workflow stayed RUNNING, and the next prompt answered + normally. + + What the stop does not wait for is the tools. A step's dispatch runs under `allSettled` with the + activity default of `WAIT_CANCELLATION_COMPLETED`, which reads like the turn ends only once the + slowest tool has acknowledged. History says otherwise: with a tool that never finishes, the + workflow recorded `ActivityTaskCancelRequested` and moved on in the same second, and completed two + seconds later with that activity still running. The stop costs one workflow task, not the slowest + cleanup. The tool activity closes its own call on the way out, so the log is not left waiting on + it either. +- **An approval holds only the tool that is waiting.** Reading a `*.env` file parks on the default + agent's `ask` rule. While the human deliberated, `runModelCall` was **completed** and + `runToolCall` was the only outstanding activity. Replying `once` completed the tool and the turn. + Under the whole-step mode the entire step, model call included, sits in one activity for the whole + of that wait. + +- **A tool activity rebuilds a worktree it has never seen.** Each of the three drains calls + `worktrees.ensure`, so a tool call landing on a worker without the project tree materializes it + from the snapshot packs. Checked by deleting the entire working directory between two turns of a + live session: the next turn's tool activity rebuilt both files, the `read` completed, and the + model + answered with the file's contents. Worker affinity (below) skips the + materialization on warm paths; + the packs are the baseline that works without it. + +Covered by tests rather than by a live run: + +- **A compaction restart keeps deferring.** The `deferTools` flag is threaded through both + `ContinueAfterCompaction` recursions; a first version dropped it, which would have silently run + tools inline on a compacting step. +- **A provider error ends the turn.** The attempt's own continuation decision is carried to the + seal, because the log cannot tell a failed attempt from one that wants another step. +- **A declined permission halts the turn** rather than reaching the model as one failed tool. Both + halves matter: the runner raises it as an interrupt, and the workflow tells that failure apart + from a tool that merely failed. + +Not covered, stated plainly: + +- **A second machine.** The rebuild above is a worker meeting a missing tree, which is the mechanism + that matters, but it ran in one process on one host. + +#### The contract covers both modes + +The conformance suite is the executable definition of what an executor must do, and stepped mode has +to satisfy the same one or the two Temporal modes drift and only the tested one is trustworthy: + +```bash +temporal server start-dev --port 7237 --headless & +cd packages/temporal +# whole step per activity +OPENCODE_CONTRACT_TEMPORAL=1 \ + bun test --timeout 180000 test/session-execution-temporal-contract.test.ts +# model call, one activity per tool, seal +OPENCODE_CONTRACT_TEMPORAL=1 OPENCODE_TEMPORAL_STEPPED=1 bun test --timeout 180000 \ + test/session-execution-temporal-contract.test.ts +``` + +Both modes pass, and so does the local coordinator in core's own `bun test`. Running it the first +time was worth the effort: **stepped mode failed two scenarios**, on a case none of the live testing +had reached. `countingModel` streams a step start and a step finish and no content at all, so the +publisher never mints an assistant message. The whole-step path survives that because it mints one +inside `Step.Ended` via `startAssistant()`; the seal, running in another process with no publisher, +searched the projection, found nothing, and left the turn open forever. The fix carries the +assistant message id out of the attempt the same way the tool call ids are carried. + +Two scenarios are there for this split in particular. One is a turn that asks for a tool, runs it, +and goes back to the model with the result; the other interrupts a turn with a tool in flight and +demands that no call is left running. Everything else in the suite settles without a tool, so the +piece the split actually changes, one activity per call rather than one for the whole step, would +have had no scenario that both modes must pass. The interrupt one was the more useful of the two: +stepped mode failed it until the dispatch learned to close its own call. + +#### Worker affinity (`OPENCODE_TEMPORAL_WORKTREE_AFFINITY=1`) + +Off by default. Without it every worker polls one queue, and a worker drawing a session whose tree +it +has never seen rebuilds that tree from snapshot packs. That is the portable baseline and it works. +Affinity avoids the rebuild by routing instead: the queue name is derived from the session's +directory, and only workers serving that directory poll it. + +```bash +# a worker declares the tree it serves; defaults to the process directory +OPENCODE_TEMPORAL_WORKTREE_AFFINITY=1 OPENCODE_TEMPORAL_WORKTREE=/srv/trees/acme \ + OPENCODE_TEMPORAL_ROLE=worker ... bun run packages/server/src/worker.ts +``` + +The queue is keyed on the session's `location.directory`, not the project root, because that is the +tree `worktrees.ensure` has to produce and two sessions in one project can sit in different +directories. Paths are resolved through `realpath` first: on macOS `/tmp/x` and `/private/tmp/x` are +one tree, and a client and a worker that disagreed would sit on two queues and the session would +hang +with nothing to show for it. + +**This trades availability for latency, which is why it is opt-in.** With affinity on, a session +whose tree has no worker polling does not fall back to another worker. It waits. Reconstruction is +what makes any worker able to serve any session, and turning affinity on is choosing not to use it. + +Two consequences to plan for, both silent: + +- **A worker serves one tree.** In the default `role=both` deployment the embedded worker polls the + queue for the process directory, so a session in another project has no poller. Point + `OPENCODE_TEMPORAL_WORKTREE` at the project root, not at a subfolder, since the key is the project + worktree. +- **Flipping the flag strands workflows already running.** A workflow keeps the task queue it + started on for life, and its activities inherit it. Restarting workers with the flag changed + leaves in-flight sessions with nobody polling their queue. They do not fail; they stay `RUNNING` + forever, because the workflow task that would run their idle timer is never picked up either. + Drain before flipping, in either direction. + +Both processes log the queue they use (`pollQueue` on a worker, `worktree` on the client), so a +mismatch shows up as two names in the logs rather than as a session that never runs. + +Verified live, all three halves: + +- a worker serving the session's tree ran the turn, and the workflow sat on the derived queue +- with only a worker serving a *different* tree alive, the next prompt was not answered, and the + queue showed a workflow backlog of one, aged 50 seconds +- bringing the right worker back drained it and the answer arrived, so the work waits rather than + being lost + +#### Durable events across a process boundary + +Worth stating separately, because it is **not** specific to stepped mode: it is a property of +running a standalone worker at all, and the mechanism is in `event.ts`. + +`commitDurableEvent` publishes its wake in-process and `subscribeDurable` registers in that same +process's map, so a commit in another process cannot wake a tail. Split into +`OPENCODE_TEMPORAL_ROLE=client` serve plus a standalone worker, a turn ran correctly and its log was +complete, but a client subscribed *live* saw exactly one event, `prompt.admitted`, the only one the +serve process writes itself. A UI attached to serve saw a prompt admitted and then silence. + +Token deltas are not part of this. `Text.Delta`, `Reasoning.Delta` and `Tool.Input.Delta` are +live-only and never reach the durable log, so what crosses a process boundary is block-level: +`step.started`, `tool.called`, `tool.success`, `step.ended`. A durable tail can also re-read on a +tick (`LayerOptions.livePollInterval`). In-process commits still wake it instantly, so latency is +unchanged where it already worked; the tick only catches what the wake cannot see. An idle +subscriber costs one indexed read per period, and a tick with nothing new emits nothing. With it, +the same split delivers the worker's `step.started`, `tool.called`, `tool.success` and `step.ended` +to a live subscriber. + +It is off unless a composition root asks for it, and only the durable executor's does +(`EventV2.pollingNode`, wired in `server/src/routes.ts`). A deployment running in one process wakes +its own subscribers, so a tick there would be a query per second per subscribed session for events +that cannot exist. `OPENCODE_EVENT_POLL_MS` overrides either way, `0` to turn it off. + ### Two modes, one runner The factory has exactly two modes, both driving the same `SessionRunner` over the same durable event @@ -100,7 +393,8 @@ log. `OPENCODE_SESSION_EXECUTION=temporal` runs each session as a per-session Te `sessionTurn` supervisor (`supervisor.ts`) loops a `runTurnStep` drain, so each step (one provider attempt + its tools) is its own activity with its own retry/timeout/visibility, reusing `SessionRunner.runStep` (one iteration of `run`'s loop). Anything else (the default) runs in-process -on the proven `SessionRunCoordinator` (`execution/local.ts`) -- no server, no worker, no ports -- which +on the proven `SessionRunCoordinator` (`execution/local.ts`) -- no server, no worker, no ports -- +which drives whole turns with `SessionRunner.run` and owns the wake/resume/interrupt lifecycle. That coordinator is the same one the v1 server uses and has direct lifecycle tests (`session-run-coordinator.test.ts`), so the default path reuses well-exercised code rather than a @@ -116,7 +410,8 @@ local modes were folded away in favor of the coordinator for local.) A per-step re-drive resumes from the durable event log rather than re-running work. `runStep` closes any tool left dangling by an interrupted attempt on every entry, not just the first. Without that, a mid-turn retry (`first=false`) re-streamed a request carrying a `tool_use` with no `tool_result`, -which the provider rejects, a retry poison loop. And if the crashed step had already dispatched tools +which the provider rejects, a retry poison loop. And if the crashed step had already dispatched +tools it is finalized from the log: completed tool results are kept, still-unsettled tools are failed, and a synthesized `Step.Ended` closes the step without re-calling the model. A tool caught in flight at the crash is handled by declared idempotency: a side-effect-free tool (`read`/`glob`/`grep`, marked @@ -142,10 +437,14 @@ later. Verified by `packages/core/test/session-runner-resume.test.ts`. `startToCloseTimeout` is a 12-hour backstop for a drain that hangs while its process stays alive. When an attempt is retried while the previous one is still alive (a network partition, or the backstop firing), the old attempt could briefly keep publishing until its heartbeat is rejected - and the AbortSignal interrupts it. That overlap is now fenced: each drain claims the event log + and the AbortSignal interrupts it. That overlap is fenced per attempt in whole-step + mode: the drain claims the event log with an attempt token (`event_sequence.owner_id` via `claim()`), and a live durable append dies if a newer attempt has since claimed the log (the check is in `event.ts`, gated by the - `EventOwner` context the drain provides). The owner is set activity-side from the run id and + `EventOwner` context the drain provides). In stepped mode a step's three activity kinds share one + token, so the fence separates steps but not writers inside a step; what keeps the projection right + there is the projector applying a tool result only while the part is still open, in the same + transaction as the append. The owner is set activity-side from the run id and attempt, so it stays out of the workflow's deterministic input; the local driver uses a per-instance token. The projector's status guards still make any duplicate settlement a no-op. @@ -177,7 +476,8 @@ have the session's worktree present (see "What resumes cross-host"). ### Durable permission asks A tool waiting for user approval used to park on an in-memory deferred: invisible outside the asking -process (a standalone worker's ask could never be answered) and gone on restart. A pending ask is now +process (a standalone worker's ask could never be answered) and gone on restart. A pending ask is +now also a row in the shared store (`permission_request`). The blocked `assert` races its local deferred against a poll of the row, so a reply from ANY process sharing the store (the HTTP server answering for a detached worker) unblocks it; `list`/`get`/`forSession` read the rows, so serve can show asks @@ -250,27 +550,42 @@ distinct worker identities. ### What resumes cross-host, and what does not -The runner rebuilds a session's LLM context purely from the shared DB (`SessionHistory.entriesForRunner` -then `toLLMMessages`); it never reads local disk to reconstruct context. So the **conversation** resumes -on any worker: messages, tool results (the bounded preview and structured output that the model sees), -prompt attachments (stored inline as `data:` URIs in the prompt), and credentials (`CredentialTable`) +The runner rebuilds a session's LLM context purely from the shared DB +(`SessionHistory.entriesForRunner` +then `toLLMMessages`); it never reads local disk to reconstruct context. So the **conversation** +resumes +on any worker: messages, tool results (the bounded preview and structured output that the model +sees), +prompt attachments (stored inline as `data:` URIs in the prompt), and credentials +(`CredentialTable`) all ride the shared store. **The project working tree** now rides the store too. After each step capture the runner ships the snapshot tree as an incremental git pack (`snapshot-sync.ts`, `snapshot_pack` table). Before a -drain runs, a worker missing the session's directory rebuilds the worktree from those packs -(`session/execution/worktree.ts`): uncommitted edits and untracked files included, checked out at -the same absolute path it was captured at (a uniform fleet layout). Ignored files and dependencies -are not captured, so a rebuilt tree may need an install step before `bash` behaves identically. -Worker affinity or a shared volume skips the materialization latency on warm paths; the packs -are the portable baseline that works with neither. +drain runs, a worker whose tree is missing or older than the store's newest capture builds it from +those packs (`session/execution/worktree.ts`): uncommitted edits and untracked files included, +checked out at the same absolute path it was captured at (a uniform fleet layout). Ignored files +and dependencies are not captured, so a rebuilt tree may need an install step before `bash` behaves +identically. Worker affinity (below) or a shared volume skips the materialization latency on warm +paths; the packs are the portable baseline that works with neither. + +Two rules bound what that refresh may touch, because checking a stored tree out over the wrong one +destroys work. A tree is moved only when a host-local note (`snapshot/tip.ts`) says this host is +behind the store, so a host holding a capture that never shipped is left as it is. And it is moved +only when this host built the tree from packs, so a checkout the host already had, a developer's own +working copy, is never rewritten: that case is logged and left alone. What stays open is the tools +of ONE step running on two hosts, since nothing captures their writes until the step is sealed. +Affinity is what keeps a step's tools on one tree. Host-local state that does NOT ride the DB, so it is not reconstructed on a different host: - **The snapshot store (`${data}/snapshot`) and the retained full tool-output files - (`${data}/tool-output`).** The runner never reads these to rebuild context: snapshot file-diffs are - best-effort (`Effect.catch` to `undefined`), and the model sees the bounded tool-output preview, not - the file. They only affect the diff/restore/revert features and full-output viewing. Point `${data}` + (`${data}/tool-output`).** The runner never reads these to rebuild context: snapshot file-diffs + are + best-effort (`Effect.catch` to `undefined`), and the model sees the bounded tool-output preview, + not + the file. They only affect the diff/restore/revert features and full-output viewing. Point + `${data}` (the XDG data dir) at shared storage to make them portable. ## Porting this pattern diff --git a/packages/temporal/scripts/stream-tail-probe.ts b/packages/temporal/scripts/stream-tail-probe.ts new file mode 100644 index 000000000000..55fae4c6d7e2 --- /dev/null +++ b/packages/temporal/scripts/stream-tail-probe.ts @@ -0,0 +1,247 @@ +// How much of a real provider's stream arrives AFTER it has asked for its first tool? +// +// That gap is the only thing a stepped executor gives up: a whole-step activity forks the tool +// immediately and runs it under the rest of the stream, while a split has to wait for the attempt +// to +// return. The bench measured the loss as min(tail, tool duration) with a dialled mock. This +// measures +// the tail itself, against the real API, so the mock's dial can be set to something honest. +// +// The measuring point matters. OpenCode forks on the `tool-call` event, which the protocol layer +// emits only once that call's ARGUMENTS are complete and parsed, not when its id first appears. +// Measuring from the id overstates the tail by however long the arguments took to stream. +// +// Measured 2026-08-26, 3 runs per probe (2 for gpt-5), key from ~/.config/ai363/llm.key: +// +// model median tail max tail text after first call +// gpt-4o-mini (chat) 0 ms 50 ms none, in any probe +// gpt-5-mini (responses) 33 ms 36 ms none, in any probe +// gpt-5 (responses) 34 ms 77 ms none, in any probe +// +// So the tail is 0 to 77 ms, at most a few percent of the stream, and no model emitted a single +// character of text after asking for its first tool. A single tool call IS the end of the stream. +// The tail only appears with several calls at once, and is then the time to stream calls 2..N. +// +// bun run packages/temporal/scripts/stream-tail-probe.ts +// +// Reads the key from a file and never prints it. +import { readFileSync } from "node:fs" + +const KEY = readFileSync(`${process.env.HOME}/.config/ai363/llm.key`, "utf8").trim() + +const TOOLS = [ + { + name: "bash", + description: "Run a shell command and return its output.", + parameters: { + type: "object", + properties: { command: { type: "string", description: "The command to run" } }, + required: ["command"], + additionalProperties: false, + }, + }, + { + name: "read_file", + description: "Read a file and return its contents.", + parameters: { + type: "object", + properties: { path: { type: "string", description: "Path to the file" } }, + required: ["path"], + additionalProperties: false, + }, + }, +] + +interface Probe { + readonly label: string + readonly prompt: string +} + +// Shapes a coding agent actually produces. The interesting ones are the last two: several tools at +// once (streaming calls 2..N is the tail for call 1) and a model asked to narrate as it goes. +const PROBES: ReadonlyArray = [ + { label: "single-call", prompt: "Run `ls -la` in the project root. Use the bash tool." }, + { label: "single-call-terse", prompt: "What files are here? Use the bash tool once." }, + { + label: "three-calls", + prompt: + "Read these three files: src/a.ts, src/b.ts, src/c.ts. Use the read_file tool, one call per file.", + }, + { + label: "five-calls", + prompt: + "Read these five files: src/a.ts, src/b.ts, src/c.ts, src/d.ts, src/e.ts. Use the read_file tool, one call per file.", + }, + { + label: "narrate-then-call", + prompt: + "Say one short sentence about what you are going to check, then run `git status` with the bash tool.", + }, + { + label: "call-then-narrate", + prompt: + "Run `git status` with the bash tool, and in the same reply also write a two-sentence note explaining why you ran it.", + }, +] + +interface Result { + readonly firstToolAt?: number + readonly lastAt: number + readonly calls: number + readonly textAfterFirstCall: number + readonly totalMs: number +} + +const streamChat = async (model: string, prompt: string): Promise => { + const started = performance.now() + const res = await fetch("https://api.openai.com/v1/chat/completions", { + method: "POST", + headers: { authorization: `Bearer ${KEY}`, "content-type": "application/json" }, + body: JSON.stringify({ + model, + stream: true, + messages: [{ role: "user", content: prompt }], + tools: TOOLS.map((t) => ({ type: "function", function: t })), + }), + }) + if (!res.ok) throw new Error(`${res.status} ${(await res.text()).slice(0, 200)}`) + // chat.completions has no per-call "arguments done" event: a call is complete once a LATER index + // starts streaming, or when finish_reason arrives. + let openIndex: number | undefined + return consume(res, started, (obj) => { + const choice = obj?.choices?.[0] + const delta = choice?.delta + const done: string[] = [] + for (const tc of delta?.tool_calls ?? []) { + if (openIndex !== undefined && tc.index !== openIndex) done.push(`idx${openIndex}`) + openIndex = tc.index + } + if (choice?.finish_reason && openIndex !== undefined) { + done.push(`idx${openIndex}`) + openIndex = undefined + } + return { toolIds: done, text: typeof delta?.content === "string" ? delta.content.length : 0 } + }) +} + +const streamResponses = async (model: string, prompt: string): Promise => { + const started = performance.now() + const res = await fetch("https://api.openai.com/v1/responses", { + method: "POST", + headers: { authorization: `Bearer ${KEY}`, "content-type": "application/json" }, + body: JSON.stringify({ + model, + stream: true, + store: false, + input: [{ role: "user", content: [{ type: "input_text", text: prompt }] }], + tools: TOOLS.map((t) => ({ type: "function", ...t })), + }), + }) + if (!res.ok) throw new Error(`${res.status} ${(await res.text()).slice(0, 200)}`) + return consume(res, started, (obj) => { + const ids: string[] = [] + // The fork point: arguments complete, so the call can actually be dispatched. + if (obj?.type === "response.function_call_arguments.done") + ids.push(String(obj.item_id ?? obj.output_index)) + const text = obj?.type === "response.output_text.delta" ? String(obj.delta ?? "").length : 0 + return { toolIds: ids, text } + }) +} + +const consume = async ( + res: Response, + started: number, + parse: (obj: any) => { toolIds: string[]; text: number }, +): Promise => { + const reader = res.body!.getReader() + const decoder = new TextDecoder() + let buffer = "" + let firstToolAt: number | undefined + let lastAt = started + let textAfterFirstCall = 0 + const seen = new Set() + + for (;;) { + const { done, value } = await reader.read() + if (done) break + buffer += decoder.decode(value, { stream: true }) + const lines = buffer.split("\n") + buffer = lines.pop() ?? "" + for (const line of lines) { + if (!line.startsWith("data: ")) continue + const payload = line.slice(6).trim() + if (payload === "[DONE]") continue + let obj: any + try { + obj = JSON.parse(payload) + } catch { + continue + } + const now = performance.now() + lastAt = now + const { toolIds, text } = parse(obj) + for (const id of toolIds) { + if (seen.has(id)) continue + seen.add(id) + if (firstToolAt === undefined) firstToolAt = now + } + if (firstToolAt !== undefined && text > 0) textAfterFirstCall += text + } + } + return { + firstToolAt, + lastAt, + calls: seen.size, + textAfterFirstCall, + totalMs: Math.round(lastAt - started), + } +} + +const model = process.argv[2] ?? "gpt-4o-mini" +const api = process.argv[3] ?? "chat" +const runs = Number(process.argv[4] ?? 3) + +console.log(`\nmodel=${model} api=${api} runs=${runs}`) +console.log( + `${"probe".padEnd(20)}${"calls".padStart(6)}${"stream".padStart(9)}${"tail".padStart(8)}${"tail%".padStart(7)}${"text after".padStart(12)}`, +) + +const allTails: number[] = [] +for (const probe of PROBES) { + const tails: number[] = [] + let calls = 0 + let total = 0 + let after = 0 + for (let i = 0; i < runs; i++) { + try { + const r = + api === "responses" + ? await streamResponses(model, probe.prompt) + : await streamChat(model, probe.prompt) + if (r.firstToolAt === undefined) continue + tails.push(Math.round(r.lastAt - r.firstToolAt)) + calls = Math.max(calls, r.calls) + total += r.totalMs + after += r.textAfterFirstCall + } catch (e) { + console.log(`${probe.label.padEnd(20)} ERROR ${(e as Error).message}`) + } + } + if (!tails.length) { + console.log(`${probe.label.padEnd(20)}${"-".padStart(6)} (no tool call)`) + continue + } + const median = tails.slice().sort((a, b) => a - b)[Math.floor(tails.length / 2)]! + const avgTotal = Math.round(total / tails.length) + const pct = avgTotal > 0 ? Math.round((median / avgTotal) * 100) : 0 + allTails.push(median) + console.log( + `${probe.label.padEnd(20)}${String(calls).padStart(6)}${(avgTotal + "ms").padStart(9)}${(median + "ms").padStart(8)}${(pct + "%").padStart(7)}${String(Math.round(after / tails.length) + " ch").padStart(12)}`, + ) +} +if (allTails.length) { + const sorted = allTails.slice().sort((a, b) => a - b) + console.log( + `\nmedian tail across probes: ${sorted[Math.floor(sorted.length / 2)]}ms max: ${sorted[sorted.length - 1]}ms`, + ) +} diff --git a/packages/temporal/src/activities.ts b/packages/temporal/src/activities.ts index 68a37c8d8b03..8ce8b1310b06 100644 --- a/packages/temporal/src/activities.ts +++ b/packages/temporal/src/activities.ts @@ -3,10 +3,17 @@ // crash is detected quickly, and forwards Temporal cancellation as an AbortSignal so an interrupt // turns into Effect fiber interruption inside the runner. -import { heartbeat, Context } from "@temporalio/activity" +import { heartbeat, CancelledFailure, Context } from "@temporalio/activity" + +// The SDK's cancel reasons that mean nothing is coming back for this call: the workflow asked to +// stop, or it is gone. The rest (a worker shutting down, a pause, a heartbeat timeout, a reset) +// hand the same call to another attempt. They reach an activity as the reason on the abort, which +// every SDK version sets, and as `cancellationDetails`, which only a server that sends them does. +const ENDS_THE_TURN = ["CANCELLED", "NOT_FOUND"] // The event-log owner token for one activity execution: run id + activity id + attempt. A Temporal -// retry of the SAME step gets a fresh attempt, so once the retry claims the log the previous attempt +// retry of the SAME step gets a fresh attempt, so once the retry claims the log the previous +// attempt // (if still running) is fenced out. The activity id is essential: activity attempt numbers restart // at 1 for every step, so a run-id+attempt token alone would repeat across steps (step 1 attempt 1 // and step 2 attempt 1 both mint `run#1`), letting a zombie attempt from an earlier step re-match @@ -19,7 +26,8 @@ export function ownerTokenFrom(run: string, activityId: string, attempt: number) function ownerToken(): string { const info = Context.current().info // runId/workflowType are typed optional on ActivityInfo; activityId is always present and already - // makes the token unique per execution, so an empty run prefix in the (degenerate) missing case is + // makes the token unique per execution, so an empty run prefix in the (degenerate) missing case + // is // harmless. const run = info.workflowExecution?.runId ?? info.workflowType ?? "" return ownerTokenFrom(run, info.activityId, info.attempt) @@ -28,29 +36,96 @@ function ownerToken(): string { // The step contract lives with the drain in core; re-exported here so the workflow and its tests // keep one import site inside this package. import type { StepDrainInput, StepDrainResult } from "./drain" +import type { + ModelCallDrainInput, + ModelCallDrainResult, + SealDrainInput, + ToolCallDrainInput, + ToolCallDrainResult, +} from "./l2-drain" export type { StepDrainInput, StepDrainResult } export type StepActivities = { runTurnStep(input: StepDrainInput): Promise } +// Heartbeat while a drain runs so a dead worker is noticed quickly. This only proves the process is +// alive, not that the work is progressing. +const beating = async (body: () => Promise): Promise => { + const beat = setInterval(() => { + try { + heartbeat() + } catch { + // heartbeat outside an activity context is a no-op for our purposes + } + }, 3000) + try { + return await body() + } finally { + clearInterval(beat) + } +} + export function makeStepActivities( stepDrain: (input: StepDrainInput, signal: AbortSignal) => Promise, ): StepActivities { return { async runTurnStep(input) { - const beat = setInterval(() => { - try { - heartbeat() - } catch { - // heartbeat outside an activity context is a no-op for our purposes - } - }, 3000) - try { - return await stepDrain({ ...input, owner: ownerToken() }, Context.current().cancellationSignal) - } finally { - clearInterval(beat) + return beating(() => + stepDrain({ ...input, owner: ownerToken() }, Context.current().cancellationSignal), + ) + }, + } +} + +export type SteppedTurnActivities = { + runModelCall(input: ModelCallDrainInput): Promise + runToolCall(input: ToolCallDrainInput): Promise + sealStep(input: SealDrainInput): Promise +} + +// The three activities of a stepped step. Only the model call mints and claims an owner token: it +// is +// the writer that supersedes the attempt before it, and the tool and seal activities publish under +// the token it returned. Giving each of them its own token would make a step's writers fence each +// other out of the log. +export function makeSteppedTurnActivities(drains: { + modelCallDrain: ( + input: ModelCallDrainInput & { readonly owner: string }, + signal: AbortSignal, + ) => Promise + toolCallDrain: ( + input: ToolCallDrainInput, + signal: AbortSignal, + turnEnded: () => boolean, + ) => Promise + sealDrain: (input: SealDrainInput, signal: AbortSignal) => Promise +}): SteppedTurnActivities { + return { + async runModelCall(input) { + return beating(() => + drains.modelCallDrain( + { ...input, owner: ownerToken() }, + Context.current().cancellationSignal, + ), + ) + }, + async runToolCall(input) { + const context = Context.current() + // An attempt that is being handed on must leave the call as it found it, or its successor + // reads a closed call and never runs the tool. A superseded attempt learns of it the same way + // a stopped one does; closing a call its successor is re-running costs the model that result, + // which is the smaller loss. + const turnEnded = () => { + const details = context.cancellationDetails + if (details) return details.cancelRequested || details.notFound + const reason: unknown = context.cancellationSignal.reason + return reason instanceof CancelledFailure && ENDS_THE_TURN.includes(reason.message ?? "") } + return beating(() => drains.toolCallDrain(input, context.cancellationSignal, turnEnded)) + }, + async sealStep(input) { + return beating(() => drains.sealDrain(input, Context.current().cancellationSignal)) }, } } diff --git a/packages/temporal/src/boundary.ts b/packages/temporal/src/boundary.ts new file mode 100644 index 000000000000..579f52024096 --- /dev/null +++ b/packages/temporal/src/boundary.ts @@ -0,0 +1,69 @@ +// Crossing from Effect into an activity result. Every drain in this package ends here, so the way a +// failure is classified is decided in one place: +// - an interrupt caused by the driver's own cancellation rethrows the abort reason, so the +// attempt records Cancelled rather than Failed +// - a run the user refused (a declined permission) crosses non-retryable under its own type, or +// the supervisor re-drives a turn the user explicitly stopped +// - a genuine run error crosses non-retryable with the RunError encoded in `details`, so the +// caller reconstructs the exact typed error instead of a string +// Only crashes and task timeouts (never thrown here) go through the activity retry policy. + +import { Cause, Effect, Exit } from "effect" +import { ApplicationFailure } from "@temporalio/activity" +import { SessionSchema } from "@opencode-ai/core/session/schema" +import { SessionRunDeclinedError } from "@opencode-ai/core/session/error" +import { encodeRunError } from "@opencode-ai/core/session/execution/run-error-codec" +import { HALTED_FAILURE_TYPE } from "./protocol" + +export interface BoundaryOptions { + /** Whether an interrupt the body raises on its own, with nothing cancelling it, means the user + * refused the run. The whole-step path says yes: it turns a decline into an interrupt to halt + * the loop, which is the local behaviour V1 defined and its own tests pin. The stepped path says + * no, because its dispatch names the decline, so an interrupt there is the runner stopping for + * some other reason and reading it as a refusal would put words in the user's mouth. */ + readonly declineIsInterrupt?: boolean +} + +const halted = (sessionID: string, declined?: SessionRunDeclinedError) => { + const encoded = encodeRunError( + declined ?? new SessionRunDeclinedError({ sessionID: SessionSchema.ID.make(sessionID) }), + ) + return ApplicationFailure.create({ + message: "session run halted (user declined)", + type: HALTED_FAILURE_TYPE, + nonRetryable: true, + details: encoded === undefined ? undefined : [encoded], + }) +} + +export const runAtBoundary = async ( + sessionID: string, + signal: AbortSignal, + body: Effect.Effect, + options?: BoundaryOptions, +): Promise => { + const exit = await Effect.runPromiseExit(body, { signal }) + if (Exit.isSuccess(exit)) return exit.value + const cause = exit.cause + if (Cause.hasInterruptsOnly(cause)) { + if (signal.aborted) + throw signal.reason instanceof Error ? signal.reason : new Error("session run interrupted") + if (options?.declineIsInterrupt) throw halted(sessionID) + // Nothing cancelled this run and nothing named a reason: the runner stopped itself, which a + // session handed to a runner bound to another location does. + throw ApplicationFailure.create({ + message: "session run interrupted with no cancellation", + type: "SessionRunInterrupted", + nonRetryable: true, + }) + } + const squashed = Cause.squash(cause) as { _tag?: string; message?: string } + if (squashed instanceof SessionRunDeclinedError) throw halted(sessionID, squashed) + const encoded = encodeRunError(squashed) + throw ApplicationFailure.create({ + message: squashed?.message ?? Cause.pretty(cause), + type: squashed?._tag ?? "SessionRunError", + nonRetryable: true, + details: encoded === undefined ? undefined : [encoded], + }) +} diff --git a/packages/temporal/src/config.ts b/packages/temporal/src/config.ts index 6d4c7c70de3e..0fbe624b414c 100644 --- a/packages/temporal/src/config.ts +++ b/packages/temporal/src/config.ts @@ -18,6 +18,17 @@ export interface Interface { readonly role: Role /** Override for the supervisor's idle self-termination; local mode honors the same variable. */ readonly idleTimeout?: string + /** Drive each step as a provider attempt, one activity per tool call, and a seal. Off by default: + * the whole-step mode is what runs today, and this only changes how new sessions start. */ + readonly stepped?: boolean + /** Route a session's work to workers that already hold its project tree, instead of letting any + * worker draw it and rebuild the tree from snapshot packs. Off by default, because it trades the + * reconstruction fallback for latency: a session whose worktree has no worker polling waits + * rather than being served elsewhere. */ + readonly worktreeAffinity?: boolean + /** The worktree this worker serves, when affinity is on. Defaults to the process directory, which + * is what a serve process with an embedded worker is already sitting in. */ + readonly worktree?: string } export class Service extends Context.Service()("@opencode/temporal/Config") {} @@ -28,4 +39,7 @@ export const fromEnv = (): Interface => ({ taskQueue: process.env.OPENCODE_TEMPORAL_TASK_QUEUE ?? DEFAULTS.taskQueue, role: (process.env.OPENCODE_TEMPORAL_ROLE as Role | undefined) ?? "both", idleTimeout: process.env.OPENCODE_SESSION_IDLE_TIMEOUT, + stepped: process.env.OPENCODE_TEMPORAL_STEPPED === "1", + worktreeAffinity: process.env.OPENCODE_TEMPORAL_WORKTREE_AFFINITY === "1", + worktree: process.env.OPENCODE_TEMPORAL_WORKTREE, }) diff --git a/packages/temporal/src/drain.ts b/packages/temporal/src/drain.ts index db10397030d0..d3788c787200 100644 --- a/packages/temporal/src/drain.ts +++ b/packages/temporal/src/drain.ts @@ -1,11 +1,11 @@ // The per-step drain body for the Temporal layer: it runs inside the runTurnStep activity. It wraps -// one SessionRunner.runStep, claims the event log for the attempt, ensures the worktree, and encodes +// one SessionRunner.runStep, claims the event log for the attempt, ensures the worktree, and +// encodes // the error for the activity boundary. Local mode does not use this: it runs whole turns through // SessionRunner.run on the SessionRunCoordinator (execution/local.ts). Both modes go through the // same SessionRunner and the same durable event log. -import { Cause, Context, Effect, Exit, type LayerMap } from "effect" -import { ApplicationFailure } from "@temporalio/activity" +import { Context, Effect, type LayerMap } from "effect" import type { LocationServiceMap } from "@opencode-ai/core/location-service-map" import type { Location } from "@opencode-ai/core/location" import type { LocationError, LocationServices } from "@opencode-ai/core/location-services" @@ -14,9 +14,8 @@ import { WorktreeMaterializer } from "@opencode-ai/core/session/execution/worktr import { SessionRunner } from "@opencode-ai/core/session/runner" import { SessionSchema } from "@opencode-ai/core/session/schema" import { SessionStore } from "@opencode-ai/core/session/store" -import { SessionRunDeclinedError } from "@opencode-ai/core/session/error" import type { SessionInput } from "@opencode-ai/core/session/input" -import { encodeRunError } from "@opencode-ai/core/session/execution/run-error-codec" +import { runAtBoundary } from "./boundary" // One step of a turn, as any executor drives it. `promotion` is null (not undefined) so it // serializes cleanly across an executor's process boundary. export interface StepDrainInput { @@ -50,8 +49,10 @@ export interface DrainDeps { export const makeDrains = ({ store, locations, ctx, events, worktrees }: DrainDeps) => { // Run exactly one step of the turn (the supervisor loops it); returns the next loop state. - const stepDrain = async (input: StepDrainInput, signal: AbortSignal): Promise => { - const exit = await Effect.runPromiseExit( + const stepDrain = async (input: StepDrainInput, signal: AbortSignal): Promise => + runAtBoundary( + input.sessionID, + signal, Effect.gen(function* () { const session = yield* store.get(SessionSchema.ID.make(input.sessionID)) if (!session) return { ran: false, continue: false, step: input.step, promotion: null } @@ -70,39 +71,10 @@ export const makeDrains = ({ store, locations, ctx, events, worktrees }: DrainDe ).pipe(Effect.provide(locations.get(session.location))) return { ran: r.ran, continue: r.continue, step: r.step, promotion: r.promotion ?? null } }).pipe(Effect.provideService(EventV2.EventOwner, input.owner), Effect.provide(ctx), Effect.scoped), - { signal }, + // A whole step turns a declined permission into an interrupt to halt its own loop, so an + // interrupt with nothing cancelling it is that refusal and nothing else. + { declineIsInterrupt: true }, ) - if (Exit.isSuccess(exit)) return exit.value - const cause = exit.cause - if (Cause.hasInterruptsOnly(cause)) { - // Two interrupt sources: driver cancellation (the AbortSignal fired; rethrow its reason so - // the attempt records Cancelled, not Failed) and an internal halt like a user declining a - // permission (the signal did NOT fire). The latter must be non-retryable, or the supervisor - // re-drives a turn the user explicitly stopped. - if (signal.aborted) - throw signal.reason instanceof Error ? signal.reason : new Error("session run interrupted") - const declined = encodeRunError( - new SessionRunDeclinedError({ sessionID: SessionSchema.ID.make(input.sessionID) }), - ) - throw ApplicationFailure.create({ - message: "session run halted (user declined)", - type: "SessionRunDeclined", - nonRetryable: true, - details: declined === undefined ? undefined : [declined], - }) - } - // A genuine run error is thrown non-retryable so Temporal surfaces it (to resume) rather than - // retrying; only crashes / task timeouts (never thrown here) go through the retry policy. The - // error is encoded faithfully in `details` so the caller can reconstruct the exact RunError. - const squashed = Cause.squash(cause) as { _tag?: string; message?: string } - const encoded = encodeRunError(squashed) - throw ApplicationFailure.create({ - message: squashed?.message ?? Cause.pretty(cause), - type: squashed?._tag ?? "SessionRunError", - nonRetryable: true, - details: encoded === undefined ? undefined : [encoded], - }) - } return { stepDrain } } diff --git a/packages/temporal/src/executor.ts b/packages/temporal/src/executor.ts index 1d3e7042a0e6..94987e603aa5 100644 --- a/packages/temporal/src/executor.ts +++ b/packages/temporal/src/executor.ts @@ -13,8 +13,13 @@ import { makeGlobalNode } from "@opencode-ai/core/effect/app-node" import { SessionSchema } from "@opencode-ai/core/session/schema" import { SessionStore } from "@opencode-ai/core/session/store" import { SessionExecution } from "@opencode-ai/core/session/execution" -import { makeStepActivities } from "./activities" +import { makeStepActivities, makeSteppedTurnActivities } from "./activities" import { makeDrains } from "./drain" +import { makeL2Drains } from "./l2-drain" +import { queueForWorktree } from "./queue" +import { Database } from "@opencode-ai/core/database/database" +import { ProjectTable } from "@opencode-ai/core/project/sql" +import { eq } from "drizzle-orm" import { WorktreeMaterializer } from "@opencode-ai/core/session/execution/worktree" import { toRunError } from "@opencode-ai/core/session/execution/run-error-codec" import * as WF from "./workflow" @@ -22,7 +27,8 @@ import { TemporalConfig } from "./config" import { WORKFLOW_TYPE, WORKFLOW_ID_PREFIX, workflowId } from "./protocol" // Classify an interrupt-signal delivery error. "already completed"/"not found" means an idle -// session's workflow has already closed -- nothing to interrupt, a no-op. Anything else is a genuine +// session's workflow has already closed -- nothing to interrupt, a no-op. Anything else is a +// genuine // control-plane failure: the user's stop was not delivered, and it must NOT be reported as success. export function classifyInterruptError(e: unknown): "ignore" | "fail" { const message = String((e as { message?: unknown })?.message ?? e) @@ -47,6 +53,7 @@ const layer = Layer.effect( Effect.gen(function* () { const store = yield* SessionStore.Service const locations = yield* LocationServiceMap.Service + const db = (yield* Database.Service).db // The app context the local drain runs in: providing it, then the per-location layer, supplies // SessionRunner and all of its dependencies. const ctx = yield* Effect.context() @@ -58,12 +65,41 @@ const layer = Layer.effect( // Same knob local mode honors; the workflow sandbox cannot read env, so the client forwards the // override as a workflow argument. const IDLE_TIMEOUT = config.idleTimeout + const STEPPED = config.stepped === true + const AFFINITY = config.worktreeAffinity === true + // The tree this process serves when affinity is on. A serve process with an embedded worker is + // already sitting in it, so the process directory is the right default. + const SERVED_WORKTREE = config.worktree ?? process.cwd() + // Which queue a worker polls. With affinity off this is the one shared queue and any worker can + // draw any session, rebuilding the tree if it has to. + const POLL_QUEUE = AFFINITY ? queueForWorktree(TASK_QUEUE, SERVED_WORKTREE) : TASK_QUEUE + // Which queue a session's workflow runs on. Keyed on the PROJECT worktree, not the session's + // directory: `worktrees.ensure` rebuilds the project tree, so keying on the directory a session + // happened to start in would split one physical tree across a queue per subdirectory, and a + // session started from a subfolder would wait on a queue nobody polls. + const queueFor = (id: SessionSchema.ID) => + AFFINITY + ? Effect.gen(function* () { + const session = yield* store.get(id) + if (!session) return TASK_QUEUE + const project = yield* db + .select({ worktree: ProjectTable.worktree }) + .from(ProjectTable) + .where(eq(ProjectTable.id, session.projectID)) + .get() + .pipe(Effect.orDie) + return project ? queueForWorktree(TASK_QUEUE, project.worktree) : TASK_QUEUE + }) + : Effect.succeed(TASK_QUEUE) const events = yield* EventV2.Service const worktrees = yield* WorktreeMaterializer.Service // The per-step drain (drain.ts) wraps SessionRunner.runStep for the activity boundary. Local // mode runs whole turns through SessionRunner.run on the coordinator; both share SessionRunner. const { stepDrain } = makeDrains({ store, locations, ctx, events, worktrees }) + // The stepped mode's three drains. Registered unconditionally: which mode a session runs is a + // property of its workflow input, so a worker has to be able to serve either. + const l2 = makeL2Drains({ store, locations, ctx, events, worktrees }) // Worker connection (native) hosts the runTurnStep activity + the workflow. Skipped in // client-only role so serve can run without an embedded worker. @@ -86,9 +122,9 @@ const layer = Layer.effect( Worker.create({ connection: nativeConn, namespace: NAMESPACE, - taskQueue: TASK_QUEUE, + taskQueue: POLL_QUEUE, workflowsPath: fileURLToPath(new URL("./workflow.ts", import.meta.url)), - activities: makeStepActivities(stepDrain), + activities: { ...makeStepActivities(stepDrain), ...makeSteppedTurnActivities(l2) }, }), ) const runHandle = worker.run() @@ -101,12 +137,18 @@ const layer = Layer.effect( ) } - // Worker-only process: it hosts activities but drives no workflows, so the client methods are - // unused. Return a service whose driving methods fail loudly if something unexpectedly calls them. + // unused. Return a service whose driving methods fail loudly if something unexpectedly calls + // them. if (!HOST_CLIENT) { yield* Effect.logInfo("SessionExecutionTemporal worker ready").pipe( - Effect.annotateLogs({ address: ADDRESS, taskQueue: TASK_QUEUE, workflow: WORKFLOW_TYPE, role: config.role }), + Effect.annotateLogs({ + address: ADDRESS, + taskQueue: POLL_QUEUE, + workflow: WORKFLOW_TYPE, + role: config.role, + ...(AFFINITY ? { worktree: SERVED_WORKTREE } : {}), + }), ) const clientOnly = Effect.die("SessionExecution client is not hosted when OPENCODE_TEMPORAL_ROLE=worker") return SessionExecution.Service.of({ @@ -125,18 +167,34 @@ const layer = Layer.effect( const client = new Client({ connection: clientConn, namespace: NAMESPACE }) const drive = (id: SessionSchema.ID) => - Effect.promise(() => - client.workflow.signalWithStart(WORKFLOW_TYPE, { - taskQueue: TASK_QUEUE, - workflowId: workflowId(id), - args: [id, { startWithWake: true, idleTimeout: IDLE_TIMEOUT } satisfies WF.SessionTurnOptions], - signal: WF.wake, - signalArgs: [], - }), + Effect.flatMap(queueFor(id), (taskQueue) => + Effect.promise(() => + client.workflow.signalWithStart(WORKFLOW_TYPE, { + taskQueue, + workflowId: workflowId(id), + args: [ + id, + { + startWithWake: true, + idleTimeout: IDLE_TIMEOUT, + stepped: STEPPED, + } satisfies WF.SessionTurnOptions, + ], + signal: WF.wake, + signalArgs: [], + }), + ), ) yield* Effect.logInfo("SessionExecutionTemporal ready").pipe( - Effect.annotateLogs({ address: ADDRESS, taskQueue: TASK_QUEUE, workflow: WORKFLOW_TYPE }), + Effect.annotateLogs({ + address: ADDRESS, + taskQueue: TASK_QUEUE, + workflow: WORKFLOW_TYPE, + ...(AFFINITY + ? { worktreeAffinity: true, pollQueue: POLL_QUEUE, worktree: SERVED_WORKTREE } + : {}), + }), ) return SessionExecution.Service.of({ @@ -171,36 +229,47 @@ const layer = Layer.effect( // resume = coordinator.run: drive a forced run via an Update-with-Start and AWAIT its result, // so a run error is surfaced to the caller (as a RunError) instead of being swallowed. resume: (id) => - Effect.tryPromise({ - try: async () => { - const attempt = () => { - const startOp = new WithStartWorkflowOperation(WORKFLOW_TYPE, { - taskQueue: TASK_QUEUE, - workflowId: workflowId(id), - // startWithWake=false: a fresh resume-with-start must not manufacture a wake drain; - // its forced drain comes from the resume update. Ignored when USE_EXISTING joins a - // running workflow (which keeps its own state). - args: [id, { startWithWake: false, idleTimeout: IDLE_TIMEOUT } satisfies WF.SessionTurnOptions], - workflowIdConflictPolicy: "USE_EXISTING", - }) - return client.workflow.executeUpdateWithStart(WF.resume, { - startWorkflowOperation: startOp, - args: [], - }) - } - try { - await attempt() - } catch (e) { - // The long-lived workflow self-completes after its idle timeout; an update admitted - // in that instant fails against the just-completed run instead of starting a fresh - // one. Retry once so the caller gets a real run, not the race. - const message = String((e as { message?: unknown })?.message ?? "") - if (!/already completed|not found/i.test(message)) throw e - await attempt() - } - }, - catch: (e) => toRunError(id, e), - }), + Effect.flatMap(queueFor(id), (resumeQueue) => + Effect.tryPromise({ + try: async () => { + const attempt = () => { + const startOp = new WithStartWorkflowOperation(WORKFLOW_TYPE, { + taskQueue: resumeQueue, + workflowId: workflowId(id), + // startWithWake=false: a fresh resume-with-start must not manufacture a wake + // drain; + // its forced drain comes from the resume update. Ignored when USE_EXISTING joins + // a + // running workflow (which keeps its own state). + args: [ + id, + { + startWithWake: false, + idleTimeout: IDLE_TIMEOUT, + stepped: STEPPED, + } satisfies WF.SessionTurnOptions, + ], + workflowIdConflictPolicy: "USE_EXISTING", + }) + return client.workflow.executeUpdateWithStart(WF.resume, { + startWorkflowOperation: startOp, + args: [], + }) + } + try { + await attempt() + } catch (e) { + // The long-lived workflow self-completes after its idle timeout; an update admitted + // in that instant fails against the just-completed run instead of starting a fresh + // one. Retry once so the caller gets a real run, not the race. + const message = String((e as { message?: unknown })?.message ?? "") + if (!/already completed|not found/i.test(message)) throw e + await attempt() + } + }, + catch: (e) => toRunError(id, e), + }), + ), interrupt: (id) => Effect.tryPromise({ try: () => client.workflow.getHandle(workflowId(id)).signal(WF.interrupt), @@ -210,7 +279,8 @@ const layer = Layer.effect( // An idle session's workflow has already completed; nothing to interrupt is fine. if (classifyInterruptError(e) === "ignore") return Effect.void // A genuine delivery failure must not read as success: the stop did not happen. The - // interface has no error channel, so surface it as a defect rather than a false success. + // interface has no error channel, so surface it as a defect rather than a false + // success. const message = String((e as { message?: unknown })?.message ?? e) return Effect.logError("session interrupt signal failed").pipe( Effect.annotateLogs({ sessionID: id, error: message }), @@ -225,5 +295,11 @@ const layer = Layer.effect( export const node = makeGlobalNode({ service: SessionExecution.Service, layer, - deps: [SessionStore.node, LocationServiceMap.node, EventV2.node, WorktreeMaterializer.node], + deps: [ + SessionStore.node, + LocationServiceMap.node, + EventV2.node, + WorktreeMaterializer.node, + Database.node, + ], }) diff --git a/packages/temporal/src/l2-drain.ts b/packages/temporal/src/l2-drain.ts new file mode 100644 index 000000000000..b61d721eb3b9 --- /dev/null +++ b/packages/temporal/src/l2-drain.ts @@ -0,0 +1,197 @@ +// The three drain bodies a stepped turn is made of: the provider attempt, one tool call, and the +// seal. L1's drain runs a whole step (one attempt plus all of its tools) in a single activity, so +// nothing can sit between the model asking for a tool and the tool running. Splitting them is what +// gives each tool call its own retry policy, timeout and approval window. +// +// All three write to the same session log, so they publish under ONE owner token: the model call +// claims it and hands it back, the other two inherit it. Minting a token per activity execution (as +// L1 does, correctly, when the activity is the whole step) would make a step's writers fence each +// other out. + +import { Context, Effect, type LayerMap } from "effect" +import type { LocationServiceMap } from "@opencode-ai/core/location-service-map" +import type { Location } from "@opencode-ai/core/location" +import type { LocationError, LocationServices } from "@opencode-ai/core/location-services" +import { EventV2 } from "@opencode-ai/core/event" +import { WorktreeMaterializer } from "@opencode-ai/core/session/execution/worktree" +import { SessionRunner } from "@opencode-ai/core/session/runner" +import type { DeferredToolCall, ToolCallOutcome } from "@opencode-ai/core/session/runner" +import type { StepSettlement } from "@opencode-ai/core/session/runner/publish-llm-event" +import { SessionSchema } from "@opencode-ai/core/session/schema" +import { SessionStore } from "@opencode-ai/core/session/store" +import type { SessionInput } from "@opencode-ai/core/session/input" +import { runAtBoundary } from "./boundary" +import type { StepDrainInput, StepDrainResult } from "./drain" + +/** The provider attempt of one step. Same shape as a whole-step drain: the difference is what it + * does with the tool calls, not what it needs to start. */ +export type ModelCallDrainInput = StepDrainInput + +export type ModelCallDrainResult = + | { readonly kind: "settled"; readonly result: StepDrainResult } + | { + readonly kind: "called" + readonly step: number + readonly calls: ReadonlyArray + readonly settlement?: StepSettlement + readonly assistantMessageID?: string + readonly needsContinuation?: boolean + /** The event-log token this attempt claimed. The tool and seal activities of this step must + * publish under it, so it travels with the calls instead of being minted again. */ + readonly owner: string + } + +export interface ToolCallDrainInput { + readonly sessionID: string + readonly call: DeferredToolCall + readonly owner: string +} + +export interface ToolCallDrainResult { + readonly outcome: ToolCallOutcome +} + +export interface SealDrainInput { + readonly sessionID: string + readonly step: number + readonly settlement?: StepSettlement + readonly assistantMessageID?: string + readonly needsContinuation?: boolean + readonly owner: string +} + +export interface L2DrainDeps { + readonly store: SessionStore.Interface + readonly locations: LayerMap.LayerMap + readonly ctx: Context.Context + readonly events: EventV2.Interface + readonly worktrees: WorktreeMaterializer.Interface +} + +export const makeL2Drains = ({ store, locations, ctx, events, worktrees }: L2DrainDeps) => { + // One session, one owner, a present project tree. `claim` is true only for the model call: it is + // the writer that supersedes a previous attempt, and the rest of the step rides its token. + const inSession = ( + sessionID: string, + owner: string, + claim: boolean, + use: ( + runner: SessionRunner.Interface, + session: SessionSchema.Info, + ) => Effect.Effect, + ) => + Effect.gen(function* () { + const session = yield* store.get(SessionSchema.ID.make(sessionID)) + // Deleting a session while its workflow is alive is not a run error. The whole-step drain + // reports it as nothing to do, and a resume waiting on this should resolve, not reject. + if (!session) return undefined + if (claim) yield* events.claim(session.id, owner) + // A worker taking this step on a host without the project tree rebuilds it from snapshot + // packs. + yield* worktrees.ensure(session.location.directory) + return yield* SessionRunner.Service.use((runner) => use(runner, session)).pipe( + Effect.provide(locations.get(session.location)), + ) + }).pipe(Effect.provideService(EventV2.EventOwner, owner), Effect.provide(ctx), Effect.scoped) + + const modelCallDrain = async ( + input: ModelCallDrainInput & { readonly owner: string }, + signal: AbortSignal, + ): Promise => + runAtBoundary( + input.sessionID, + signal, + inSession(input.sessionID, input.owner, true, (runner, session) => + runner.runModelCall({ + sessionID: session.id, + step: input.step, + promotion: (input.promotion ?? undefined) as SessionInput.Delivery | undefined, + first: input.first, + force: input.force, + }), + ).pipe( + Effect.map( + (result): ModelCallDrainResult => + result === undefined + ? { + kind: "settled", + result: { ran: false, continue: false, step: input.step, promotion: null }, + } + : result.kind === "settled" + ? { + kind: "settled", + result: { + ran: result.result.ran, + continue: result.result.continue, + step: result.result.step, + promotion: result.result.promotion ?? null, + }, + } + : { + kind: "called", + step: result.step, + calls: result.calls, + settlement: result.settlement, + assistantMessageID: result.assistantMessageID, + needsContinuation: result.needsContinuation, + owner: input.owner, + }, + ), + ), + ) + + const toolCallDrain = async ( + input: ToolCallDrainInput, + signal: AbortSignal, + /** Whether a cancellation means the turn is over rather than this attempt being handed on. + * Only the first closes the call: a worker shutting down leaves it for the next attempt, which + * has to be free to decide whether the tool may run again. */ + turnEnded: () => boolean = () => false, + ): Promise => + runAtBoundary( + input.sessionID, + signal, + inSession(input.sessionID, input.owner, false, (runner, session) => + runner.runToolCall({ sessionID: session.id, call: input.call }).pipe( + // A stop landing mid-tool leaves the call recorded as running, where a whole step closes + // the tools it opened before it returns. Nothing else closes it until the next turn's + // entry check, so a transcript would show the call still going long after the stop. + Effect.onInterrupt(() => + turnEnded() + ? runner + .failToolCall({ sessionID: session.id, call: input.call }) + .pipe(Effect.ignore) + : Effect.void, + ), + ), + ).pipe(Effect.map((result) => result ?? { outcome: "already-settled" as const })), + ) + + const sealDrain = async (input: SealDrainInput, signal: AbortSignal): Promise => + runAtBoundary( + input.sessionID, + signal, + inSession(input.sessionID, input.owner, false, (runner, session) => + runner.sealStep({ + sessionID: session.id, + step: input.step, + settlement: input.settlement, + assistantMessageID: input.assistantMessageID, + needsContinuation: input.needsContinuation, + }), + ).pipe( + Effect.map((result) => + result === undefined + ? { ran: false, continue: false, step: input.step, promotion: null } + : { + ran: result.ran, + continue: result.continue, + step: result.step, + promotion: result.promotion ?? null, + }, + ), + ), + ) + + return { modelCallDrain, toolCallDrain, sealDrain } +} diff --git a/packages/temporal/src/l2-step.ts b/packages/temporal/src/l2-step.ts new file mode 100644 index 000000000000..60aaefd27240 --- /dev/null +++ b/packages/temporal/src/l2-step.ts @@ -0,0 +1,111 @@ +// One step as three units of work instead of one: the provider attempt, each tool call it asks for, +// and the seal that closes it. This is the whole point of the split, and it is workflow code, so +// the +// model-to-tools loop lives where retries, timers, approvals and budgets can sit between the two. +// +// MUST stay pure, like supervisor.ts: this is bundled into the workflow sandbox, so no `effect`, no +// `@opencode-ai/core` runtime imports, no Node builtins. Type-only imports are erased and safe. +// +// What this costs, stated plainly: a whole-step activity starts each tool the moment the model asks +// for it, while the stream is still going. Here the attempt has to return before any tool starts, +// because a workflow cannot consume a stream. The tools of one step still run concurrently with +// each +// other; what is lost is the overlap between the model and its own tools. + +import { ActivityFailure, type ApplicationFailure } from "@temporalio/workflow" +import { HALTED_FAILURE_TYPE } from "./protocol" +import type { StepDrainInput, StepDrainResult } from "./drain" +import type { + ModelCallDrainInput, + ModelCallDrainResult, + SealDrainInput, + ToolCallDrainInput, + ToolCallDrainResult, +} from "./l2-drain" + +/** + * A halt the user asked for, as it looks once it has crossed an activity boundary. The runner + * raises + * it as an interrupt, `boundary.ts` throws it as an `ApplicationFailure`, and the SDK wraps that in + * one `ActivityFailure`. It is not a cancellation, so `isCancellation` says no, and a dispatcher + * that checks only that would treat a refusal as one failed tool and carry on. + * + * A `TimeoutFailure` or a `CancelledFailure` cause has no `type` field, so neither matches. + */ +export const isHaltFailure = (error: unknown) => + error instanceof ActivityFailure && + (error.cause as ApplicationFailure | undefined)?.type === HALTED_FAILURE_TYPE + +/** The three activities a stepped turn drives. */ +export interface SteppedActivities { + readonly runModelCall: (input: ModelCallDrainInput) => Promise + readonly runToolCall: (input: ToolCallDrainInput) => Promise + readonly sealStep: (input: SealDrainInput) => Promise +} + +export interface SteppedTurnDeps { + readonly activities: SteppedActivities + /** Whether an error is the driver's cancellation. An interrupt has to end the turn, so it must + * not be swallowed the way a failed tool is. */ + readonly isCancellation: (error: unknown) => boolean + /** Whether an error is the user stopping the turn, like a declined permission. It arrives as an + * ordinary activity failure, so without this it reads as one bad tool and the turn carries on + * past the refusal. */ + readonly isHalt: (error: unknown) => boolean + /** Where a step says what became of the calls it dispatched. A dispatch that decided not to run + * a tool, or could not keep its result, is a step's most surprising outcome and the least + * visible: it reads as an ordinary success everywhere else. */ + readonly log?: (message: string, attributes: Record) => void +} + +/** + * Drives one step and reports the next loop state, so it drops straight into the supervisor in + * place + * of a whole-step activity. + */ +export const makeSteppedTurn = + ({ activities, isCancellation, isHalt, log }: SteppedTurnDeps) => + async (input: StepDrainInput): Promise => { + const model = await activities.runModelCall(input) + // A crashed step finalized from the log, or the recovery gate finding no work: the step is over + // and there is nothing to dispatch or seal. + if (model.kind === "settled") return model.result + + // Each call is its own unit of work. A tool that fails outright does not take the turn with it: + // the seal closes its call as an error and the model gets to react, which is better than losing + // the step. A cancel and a user halt are different, and both have to propagate. + const dispatched = await Promise.allSettled( + model.calls.map((call) => + activities.runToolCall({ sessionID: input.sessionID, call, owner: model.owner }), + ), + ) + for (const outcome of dispatched) { + if (outcome.status !== "rejected") continue + if (isCancellation(outcome.reason) || isHalt(outcome.reason)) throw outcome.reason + } + + // A dispatch that settled its call needs no telling. The rest are what an operator is looking + // for when a turn did something unexpected: a tool reported as unknown ran or did not, and + // nothing else in this workflow's history says which call that was. + const unsettled = model.calls + .map((call, index) => { + const result = dispatched[index] + return { + call: call.id, + tool: call.name, + outcome: result?.status === "fulfilled" ? result.value.outcome : "errored", + } + }) + .filter((entry) => entry.outcome !== "settled") + if (unsettled.length > 0) + log?.("step did not settle every call it dispatched", { step: model.step, calls: unsettled }) + + return activities.sealStep({ + sessionID: input.sessionID, + step: model.step, + settlement: model.settlement, + assistantMessageID: model.assistantMessageID, + needsContinuation: model.needsContinuation, + owner: model.owner, + }) + } diff --git a/packages/temporal/src/protocol.ts b/packages/temporal/src/protocol.ts index 3f7807fef94f..b1ed3af1560e 100644 --- a/packages/temporal/src/protocol.ts +++ b/packages/temporal/src/protocol.ts @@ -8,6 +8,11 @@ export const WORKFLOW_TYPE = "sessionTurn" export const SIGNALS = { wake: "wake", interrupt: "interrupt" } as const export const RESUME_UPDATE = "resume" +/** The failure type a drain raises when the user stopped the turn (a declined permission, a + * rejected question). The workflow has to tell it apart from a failed tool, so the name is shared + * rather than spelled twice. */ +export const HALTED_FAILURE_TYPE = "SessionRunDeclined" + export const WORKFLOW_ID_PREFIX = "session-exec-" export const workflowId = (sessionID: string) => `${WORKFLOW_ID_PREFIX}${sessionID}` diff --git a/packages/temporal/src/queue.ts b/packages/temporal/src/queue.ts new file mode 100644 index 000000000000..8e12e33f7047 --- /dev/null +++ b/packages/temporal/src/queue.ts @@ -0,0 +1,45 @@ +// Routing a session's work to a worker that already has its project tree. +// +// Without affinity every worker polls one queue, and a worker that draws a session whose worktree +// it +// has never seen rebuilds it from snapshot packs (`session/execution/worktree.ts`). That is the +// portable baseline and it works, but it costs a materialization on the first step a worker takes +// for that session. Affinity avoids the cost by routing instead: the queue name is derived from the +// session's directory, and only workers serving that directory poll it. +// +// The trade is availability for latency, and it is the whole reason this is opt-in. With affinity +// on, +// a session whose worktree has no worker polling for it does not fall back to another worker; it +// waits. Reconstruction is what makes any worker able to serve any session, and turning affinity on +// is choosing not to use it. +// +// Deliberately NOT imported by workflow code: this reaches for node:crypto, which the Temporal +// sandbox does not have. The queue is chosen by the client that starts the workflow and by the +// worker +// that polls, both of which are ordinary Node. +import { createHash } from "node:crypto" +import { resolve } from "node:path" + +/** Longer than needed to make a collision implausible, short enough to keep the queue name readable + * in the Temporal UI and in `temporal task-queue describe`. */ +const DIGEST_LENGTH = 12 + +/** + * The queue for one worktree. + * + * Derived from the path alone, never from the filesystem. The two sides that have to agree do not + * see the same disk: the client hashes the session's directory, the worker hashes the tree it was + * told to serve, and a worker may not have materialized that tree yet. Resolving through the + * filesystem would make the key depend on whether the path happens to exist locally, so one side + * would canonicalize and the other would not, and they would sit on different queues while nothing + * reported an error. + * + * The cost is that the operator has to spell the tree the same way on both sides. `/tmp/x` and + * `/private/tmp/x` are one tree on macOS and two keys here. Both processes log the queue they use, + * so a mismatch shows up as two names rather than as a session that never runs. + */ +export const queueForWorktree = (base: string, directory: string): string => { + const canonical = resolve(directory).replace(/[/\\]+$/, "") + const digest = createHash("sha256").update(canonical).digest("hex").slice(0, DIGEST_LENGTH) + return `${base}-wt-${digest}` +} diff --git a/packages/temporal/src/workflow.ts b/packages/temporal/src/workflow.ts index d19ac7df8228..a395a39dd674 100644 --- a/packages/temporal/src/workflow.ts +++ b/packages/temporal/src/workflow.ts @@ -18,8 +18,10 @@ import { CancellationScope, isCancellation, allHandlersFinished, + log, } from "@temporalio/workflow" -import type { StepActivities } from "./activities" +import type { StepActivities, SteppedTurnActivities } from "./activities" +import { isHaltFailure, makeSteppedTurn } from "./l2-step" import { SIGNALS, RESUME_UPDATE } from "./protocol" import { makeSupervisor, type SupervisorRuntime } from "./supervisor" @@ -27,7 +29,8 @@ const activityOptions = { // The heartbeat is the liveness bound (it stops within seconds of a worker death and Temporal // re-drives). startToClose is only the backstop for a drain that hangs while its process stays // alive, so it must comfortably exceed any legitimate turn: long tool runs, many steps, or a - // human taking their time over a permission ask. 30 minutes proved far too tight -- it hard-killed + // human taking their time over a permission ask. 30 minutes proved far too tight -- it + // hard-killed // legitimate turns and each kill opened a short two-writer window until the zombie attempt // noticed its heartbeat rejection. startToCloseTimeout: "12 hours", @@ -37,6 +40,17 @@ const activityOptions = { const { runTurnStep } = proxyActivities(activityOptions) +// The stepped mode's three activities, each with its own bounds. Separate proxies are the point of +// the split, not an accident of it: a tool that hangs, or that is waiting on a human, no longer +// holds the provider attempt and every other tool of the same step under one shared timeout. +const { runModelCall } = proxyActivities(activityOptions) +const { runToolCall } = proxyActivities(activityOptions) +// Sealing is a snapshot, a diff and one event. It should not inherit a turn-sized backstop. +const { sealStep } = proxyActivities({ + ...activityOptions, + startToCloseTimeout: "10 minutes", +}) + export const wake = defineSignal(SIGNALS.wake) export const interrupt = defineSignal(SIGNALS.interrupt) export const resume = defineUpdate(RESUME_UPDATE) @@ -46,7 +60,8 @@ const runtime: SupervisorRuntime = { // Short-circuit when the predicate already holds. Besides saving a round trip, this avoids a real // breakage: on @temporalio/workflow 1.21, calling condition(fn, timeout) when fn is already true // leaves the current CancellationScope cancelled, so the NEXT condition() throws CancelledFailure - // -- which the supervisor reads as an interrupt and the workflow completes without ever draining a + // -- which the supervisor reads as an interrupt and the workflow completes without ever draining + // a // turn. Checking fn() first keeps the timeout timer (and its scope) out of the already-true path. // A timed wait that does NOT use the SDK's condition(fn, timeout). On @temporalio/workflow 1.21 // that variant cancels its internal timer scope on resolve and the cancellation LEAKS into the @@ -87,7 +102,8 @@ const runtime: SupervisorRuntime = { cancelCurrentScope: () => activeDrainScope?.cancel(), isCancellation, // A per-turn interrupt cancels only the child drain scope; a real workflow cancellation cancels - // the root. Reliable now that the timed wait no longer cancels any scope (nothing contaminates the + // the root. Reliable now that the timed wait no longer cancels any scope (nothing contaminates + // the // root's consideredCancelled). isRootCancelled: () => rootScope?.consideredCancelled ?? false, allHandlersFinished, @@ -95,6 +111,19 @@ const runtime: SupervisorRuntime = { continueAsNew(sessionID, { startWithWake }), } +// Same supervisor, different step body: wake, interrupt, idle timeout and continue-as-new are +// unchanged, and only what "one step" means differs. +const steppedRuntime: SupervisorRuntime = { + ...runtime, + runTurnStep: makeSteppedTurn({ + activities: { runModelCall, runToolCall, sealStep }, + isCancellation, + isHalt: isHaltFailure, + // The SDK's logger, so a line carries its workflow and run id and is suppressed on replay. + log: (message, attributes) => log.info(message, attributes), + }), +} + // The scope of the drain currently running, so an interrupt signal can cancel exactly that turn. let activeDrainScope: CancellationScope | undefined // The workflow's root scope, captured at entry, to detect a whole-run cancellation. @@ -109,18 +138,25 @@ const workflows = makeSupervisor(runtime) export interface SessionTurnOptions { readonly startWithWake?: boolean readonly idleTimeout?: string + /** Drive each step as a provider attempt, one activity per tool call, and a seal, instead of one + * activity for the whole step. Off by default: the whole-step mode is what runs today. */ + readonly stepped?: boolean } export async function sessionTurn(sessionID: string, options?: SessionTurnOptions): Promise { rootScope = CancellationScope.current() const startWithWake = options?.startWithWake ?? true const idleTimeout = options?.idleTimeout - if (!idleTimeout) return workflows.sessionTurn(sessionID, startWithWake) + const stepped = options?.stepped === true + if (!idleTimeout && !stepped) return workflows.sessionTurn(sessionID, startWithWake) return makeSupervisor( { - ...runtime, - continueAsNew: (id, wake) => continueAsNew(id, { startWithWake: wake, idleTimeout }), + ...(stepped ? steppedRuntime : runtime), + // The mode has to survive the boundary, or a long session silently reverts to whole-step + // activities the first time it rolls over. + continueAsNew: (id, wake) => + continueAsNew(id, { startWithWake: wake, idleTimeout, stepped }), }, - { idleTimeout }, + idleTimeout ? { idleTimeout } : undefined, ).sessionTurn(sessionID, startWithWake) } diff --git a/packages/temporal/test/l2-step.test.ts b/packages/temporal/test/l2-step.test.ts new file mode 100644 index 000000000000..f92ad5abddd4 --- /dev/null +++ b/packages/temporal/test/l2-step.test.ts @@ -0,0 +1,238 @@ +// Deterministic unit tests for the stepped turn body (src/l2-step.ts) driven by fake activities -- +// no Temporal, no DB. This is the piece that turns one step into three units of work, so what is +// pinned here is the orchestration: the owner token reaches every writer, a settled step dispatches +// nothing, a failed tool still lets the step close, and an interrupt is not swallowed. +import { describe, it, expect } from "bun:test" +import { isHaltFailure, makeSteppedTurn, type SteppedActivities } from "../src/l2-step" +import { runAtBoundary } from "../src/boundary" +import { SessionRunDeclinedError } from "@opencode-ai/core/session/error" +import { SessionSchema } from "@opencode-ai/core/session/schema" +import { Effect } from "effect" +import { + ActivityFailure, + ApplicationFailure, + CancelledFailure, + TimeoutFailure, +} from "@temporalio/workflow" +import type { StepDrainInput, StepDrainResult } from "../src/activities" +import type { + ModelCallDrainResult, + SealDrainInput, + ToolCallDrainInput, + ToolCallDrainResult, +} from "../src/l2-drain" + +class FakeCancel extends Error {} +class FakeHalt extends Error {} +const isCancellation = (e: unknown) => e instanceof FakeCancel +const isHalt = (e: unknown) => e instanceof FakeHalt + +const INPUT: StepDrainInput = { + sessionID: "ses_1", + step: 2, + promotion: null, + first: false, + force: false, +} +const SEALED: StepDrainResult = { ran: true, continue: true, step: 3, promotion: "steer" } +const call = (id: string, name = "probe_write") => ({ + id, + name, + input: {}, + assistantMessageID: "msg_1", +}) + +const fakes = ( + model: ModelCallDrainResult, + onTool: (input: ToolCallDrainInput) => Promise = async () => ({ + outcome: "settled", + }), +) => { + const tools: ToolCallDrainInput[] = [] + const seals: SealDrainInput[] = [] + const activities: SteppedActivities = { + runModelCall: async () => model, + runToolCall: async (input) => { + tools.push(input) + return onTool(input) + }, + sealStep: async (input) => { + seals.push(input) + return SEALED + }, + } + return { activities, tools, seals } +} + +describe("stepped turn", () => { + it("dispatches nothing and does not seal when the step is already settled", async () => { + const settled: StepDrainResult = { ran: false, continue: false, step: 2, promotion: null } + const { activities, tools, seals } = fakes({ kind: "settled", result: settled }) + + const result = await makeSteppedTurn({ activities, isCancellation, isHalt })(INPUT) + + // A crashed step finalized from the log, or the recovery gate finding no work: there is nothing + // to run and nothing to close. + expect(result).toEqual(settled) + expect(tools).toHaveLength(0) + expect(seals).toHaveLength(0) + }) + + it("runs every call under the attempt's owner and then seals", async () => { + const { activities, tools, seals } = fakes({ + kind: "called", + step: 2, + calls: [call("call_a"), call("call_b")], + settlement: { + finish: "tool-calls", + tokens: { input: 1, output: 2, reasoning: 0, cache: { read: 0, write: 0 } }, + }, + owner: "run-1:model-1:1", + }) + + const result = await makeSteppedTurn({ activities, isCancellation, isHalt })(INPUT) + + expect(tools.map((t) => t.call.id)).toEqual(["call_a", "call_b"]) + // Every writer in the step publishes under the token the attempt claimed. A token minted per + // activity would make them fence each other out of the log. + expect(tools.map((t) => t.owner)).toEqual(["run-1:model-1:1", "run-1:model-1:1"]) + expect(seals).toHaveLength(1) + expect(seals[0]?.owner).toBe("run-1:model-1:1") + // The finish reason lives only in the attempt's memory, so the seal has to be handed it. + expect(seals[0]?.settlement?.finish).toBe("tool-calls") + expect(result).toEqual(SEALED) + }) + + it("reports the calls it did not settle, and says nothing when they all settled", async () => { + const lines: Array<{ message: string; attributes: Record }> = [] + const log = (message: string, attributes: Record) => + lines.push({ message, attributes }) + const model: ModelCallDrainResult = { + kind: "called", + step: 2, + calls: [call("call_a"), call("call_b", "probe_read")], + owner: "own", + } + const settling = fakes(model) + await makeSteppedTurn({ ...settling, isCancellation, isHalt, log })(INPUT) + // Nothing surprising happened, so nothing is said about it. + expect(lines).toHaveLength(0) + + const skipping = fakes(model, async (input) => ({ + outcome: input.call.id === "call_a" ? "unknown" : "settled", + })) + await makeSteppedTurn({ ...skipping, isCancellation, isHalt, log })(INPUT) + + // Which call was skipped, and why, is the dispatch's own knowledge: the log records what the + // model was told, and the workflow's history records an activity that succeeded. + expect(lines).toHaveLength(1) + expect(lines[0]?.attributes).toEqual({ + step: 2, + calls: [{ call: "call_a", tool: "probe_write", outcome: "unknown" }], + }) + }) + + it("seals even when a tool call fails outright", async () => { + const { activities, tools, seals } = fakes( + { kind: "called", step: 2, calls: [call("call_a"), call("call_b")], owner: "own" }, + async (input) => { + if (input.call.id === "call_a") throw new Error("activity exhausted its retries") + return { outcome: "settled" } + }, + ) + + const result = await makeSteppedTurn({ activities, isCancellation, isHalt })(INPUT) + + // One broken tool must not take the turn with it: the seal closes that call as an error and the + // model gets to react, which beats losing the step. + expect(tools).toHaveLength(2) + expect(seals).toHaveLength(1) + expect(result).toEqual(SEALED) + }) + + it("lets an interrupt end the turn instead of sealing it", async () => { + const { activities, seals } = fakes( + { kind: "called", step: 2, calls: [call("call_a")], owner: "own" }, + async () => { + throw new FakeCancel("interrupted") + }, + ) + + const run = makeSteppedTurn({ activities, isCancellation, isHalt })(INPUT) + + // A cancellation is not a failed tool. Swallowing it would close a step the user stopped. + await expect(run).rejects.toBeInstanceOf(FakeCancel) + expect(seals).toHaveLength(0) + }) + + it("lets a user halt end the turn instead of sealing it", async () => { + const { activities, seals } = fakes( + { kind: "called", step: 2, calls: [call("call_a")], owner: "own" }, + async () => { + throw new FakeHalt("declined") + }, + ) + + const run = makeSteppedTurn({ activities, isCancellation, isHalt })(INPUT) + + // A decline crosses the activity boundary as an ordinary failure, not a cancel, so without a + // separate test for it the dispatcher would seal the step and the turn would carry on past the + // user's refusal. + await expect(run).rejects.toBeInstanceOf(FakeHalt) + expect(seals).toHaveLength(0) + }) +}) + +// The bug this predicate exists for was a mismatch between what `boundary.ts` throws and what the +// dispatcher recognises. Injecting a fake predicate cannot catch that, so match against the real +// failure shapes. The negative cases are the point: a predicate that answered true for everything +// would pass the positive one alone. +describe("halt predicate", () => { + const wrap = (cause?: Error) => + new ActivityFailure("activity failed", "runToolCall", "1", 1 as never, undefined, cause) + + const throwsFrom = ( + body: Effect.Effect, + options?: { declineIsInterrupt: true }, + ) => + runAtBoundary("ses_1", new AbortController().signal, body, options).then( + () => undefined, + (error: unknown) => error, + ) + + it("recognises what the activity boundary throws for a named refusal", async () => { + // Built by running the boundary rather than by hand, so the two sides cannot drift: a dispatch + // reports a decline as this error, and whatever the boundary makes of it is what a dispatcher + // has to recognise. + const thrown = await throwsFrom( + Effect.fail(new SessionRunDeclinedError({ sessionID: SessionSchema.ID.make("ses_1") })), + ) + expect(thrown).toBeInstanceOf(ApplicationFailure) + expect(isHaltFailure(wrap(thrown as Error))).toBe(true) + }) + + it("recognises a whole step's refusal, which arrives as an interrupt", async () => { + const thrown = await throwsFrom(Effect.interrupt, { declineIsInterrupt: true }) + expect(isHaltFailure(wrap(thrown as Error))).toBe(true) + }) + + it("does not read an unexplained interrupt as a refusal", async () => { + // The stepped path names its refusals, so an interrupt with nothing cancelling it is the + // runner stopping for another reason. Calling that a decline would report a user decision + // nobody made. + const thrown = await throwsFrom(Effect.interrupt) + expect(thrown).toBeInstanceOf(ApplicationFailure) + expect((thrown as ApplicationFailure).type).toBe("SessionRunInterrupted") + expect(isHaltFailure(wrap(thrown as Error))).toBe(false) + }) + + it("says no to everything else that can come back from an activity", () => { + expect(isHaltFailure(wrap(new CancelledFailure("cancelled")))).toBe(false) + expect(isHaltFailure(wrap(new TimeoutFailure("timed out", undefined, 1 as never)))).toBe(false) + expect(isHaltFailure(wrap(ApplicationFailure.create({ type: "SessionRunError" })))).toBe(false) + expect(isHaltFailure(wrap())).toBe(false) + // Unwrapped, so not what a dispatcher ever sees. + expect(isHaltFailure(ApplicationFailure.create({ type: "SessionRunDeclined" }))).toBe(false) + expect(isHaltFailure(new Error("plain"))).toBe(false) + }) +}) diff --git a/packages/temporal/test/queue.test.ts b/packages/temporal/test/queue.test.ts new file mode 100644 index 000000000000..e8f753be0d7e --- /dev/null +++ b/packages/temporal/test/queue.test.ts @@ -0,0 +1,63 @@ +// Worktree affinity is only useful if the client starting a workflow and the worker polling for it +// derive the SAME queue name from the same tree. If they disagree they sit on two queues and the +// session waits forever, which is silent: nothing errors, the work simply never runs. +import { describe, it, expect } from "bun:test" +import { mkdtempSync, mkdirSync, realpathSync, rmSync } from "node:fs" +import { tmpdir } from "node:os" +import { join } from "node:path" +import { queueForWorktree } from "../src/queue" + +describe("worktree queue", () => { + it("gives one tree one name and different trees different names", () => { + const root = realpathSync(mkdtempSync(join(tmpdir(), "queue-"))) + try { + const a = join(root, "a") + const b = join(root, "b") + mkdirSync(a) + mkdirSync(b) + + expect(queueForWorktree("q", a)).toBe(queueForWorktree("q", a)) + expect(queueForWorktree("q", a)).not.toBe(queueForWorktree("q", b)) + // The base is kept as a readable prefix so a queue is identifiable in the Temporal UI. + expect(queueForWorktree("q", a).startsWith("q-wt-")).toBe(true) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + it("agrees whether or not the tree exists locally", () => { + const root = realpathSync(mkdtempSync(join(tmpdir(), "queue-"))) + try { + const present = join(root, "present") + mkdirSync(present) + const absent = join(root, "absent") + + // This is the case that matters. The client hashes the session's directory and the worker + // hashes the tree it was told to serve, and those two processes do not see the same disk. If + // the key depended on the path resolving locally, one side would canonicalize and the other + // would not, and they would sit on different queues with nothing reporting an error. + expect(queueForWorktree("q", present)).toBe(queueForWorktree("q", join(root, "present"))) + expect(queueForWorktree("q", absent)).toBe(queueForWorktree("q", join(root, "absent"))) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + it("normalizes a path without touching the filesystem", () => { + expect(queueForWorktree("q", "/srv/trees/acme/")).toBe(queueForWorktree("q", "/srv/trees/acme")) + expect(queueForWorktree("q", "/srv/trees/./acme")).toBe( + queueForWorktree("q", "/srv/trees/acme"), + ) + // Two spellings of one tree are two keys. That is the trade for a key both sides can compute + // without a filesystem, and it is why the derived queue is logged on both sides. + expect(queueForWorktree("q", "/tmp/x")).not.toBe(queueForWorktree("q", "/private/tmp/x")) + }) + + it("still names a tree that does not exist yet", () => { + // A worker can declare the tree it serves before anything has materialized it, so an + // unresolvable path has to stay a stable key rather than throw. + const missing = join(tmpdir(), "queue-missing-tree-that-is-not-there") + expect(queueForWorktree("q", missing)).toBe(queueForWorktree("q", missing)) + expect(queueForWorktree("q", missing).startsWith("q-wt-")).toBe(true) + }) +}) diff --git a/packages/temporal/test/session-execution-temporal-contract.test.ts b/packages/temporal/test/session-execution-temporal-contract.test.ts index bb9142008a62..dcc381a703ac 100644 --- a/packages/temporal/test/session-execution-temporal-contract.test.ts +++ b/packages/temporal/test/session-execution-temporal-contract.test.ts @@ -6,7 +6,8 @@ // opt-in: // // temporal server start-dev --port 7237 --headless & -// OPENCODE_CONTRACT_TEMPORAL=1 bun test --timeout 120000 test/session-execution-temporal-contract.test.ts +// OPENCODE_CONTRACT_TEMPORAL=1 \ +// bun test --timeout 120000 test/session-execution-temporal-contract.test.ts // // Without the opt-in the file registers nothing, so a plain `bun test` stays server-free. import { makeExecutionFor, runContract } from "@opencode-ai/core/session/execution/conformance" @@ -15,7 +16,15 @@ if (process.env.OPENCODE_CONTRACT_TEMPORAL === "1") { // One task queue per run: a stale worker from an earlier run against the same dev server would // otherwise steal activities and answer with its own (differently mocked) graph. process.env.OPENCODE_TEMPORAL_TASK_QUEUE ??= `contract-${crypto.randomUUID()}` + // The stepped mode has to satisfy the SAME contract, or the two Temporal modes drift and the + // parity story only holds for the one that happens to be tested. Which mode this run covers comes + // from the same env the executor reads, so the label never disagrees with what actually ran: + // + // OPENCODE_CONTRACT_TEMPORAL=1 -> whole step per activity + // OPENCODE_CONTRACT_TEMPORAL=1 OPENCODE_TEMPORAL_STEPPED=1 -> model call, per tool, seal + const stepped = process.env.OPENCODE_TEMPORAL_STEPPED === "1" // Imported dynamically because the driver reads its connection config at module load. const { SessionExecutionTemporal } = await import("@opencode-ai/temporal/executor") - runContract("temporal driver", makeExecutionFor(SessionExecutionTemporal.node)) + const label = stepped ? "temporal driver (stepped)" : "temporal driver" + runContract(label, makeExecutionFor(SessionExecutionTemporal.node)) }