diff --git a/packages/core/package.json b/packages/core/package.json index 546771c08f02..1b413af770df 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -40,15 +40,6 @@ } }, "devDependencies": { - "@opencode-ai/http-recorder": "workspace:*", - "@parcel/watcher-darwin-arm64": "2.5.1", - "@parcel/watcher-darwin-x64": "2.5.1", - "@parcel/watcher-linux-arm64-glibc": "2.5.1", - "@parcel/watcher-linux-arm64-musl": "2.5.1", - "@parcel/watcher-linux-x64-glibc": "2.5.1", - "@parcel/watcher-linux-x64-musl": "2.5.1", - "@parcel/watcher-win32-arm64": "2.5.1", - "@parcel/watcher-win32-x64": "2.5.1", "@tsconfig/bun": "catalog:", "@types/bun": "catalog:", "@types/cross-spawn": "catalog:", @@ -58,6 +49,15 @@ "@types/semver": "catalog:", "@types/turndown": "5.0.5", "@types/which": "3.0.4", + "@parcel/watcher-darwin-arm64": "2.5.1", + "@parcel/watcher-darwin-x64": "2.5.1", + "@parcel/watcher-linux-arm64-glibc": "2.5.1", + "@parcel/watcher-linux-arm64-musl": "2.5.1", + "@parcel/watcher-linux-x64-glibc": "2.5.1", + "@parcel/watcher-linux-x64-musl": "2.5.1", + "@parcel/watcher-win32-arm64": "2.5.1", + "@parcel/watcher-win32-x64": "2.5.1", + "@opencode-ai/http-recorder": "workspace:*", "drizzle-kit": "catalog:" }, "dependencies": { @@ -85,23 +85,23 @@ "@effect/opentelemetry": "catalog:", "@effect/platform-node": "catalog:", "@effect/sql-sqlite-bun": "catalog:", + "@lydell/node-pty": "catalog:", "@ff-labs/fff-bun": "0.9.4", "@libsql/client": "^0.17.0", - "@lydell/node-pty": "catalog:", "@npmcli/arborist": "9.4.0", "@npmcli/config": "10.8.1", "@opencode-ai/effect-drizzle-sqlite": "workspace:*", "@opencode-ai/effect-sqlite-node": "workspace:*", "@opencode-ai/llm": "workspace:*", - "@opencode-ai/plugin": "workspace:*", "@opencode-ai/schema": "workspace:*", - "@openrouter/ai-sdk-provider": "2.9.0", + "@opencode-ai/plugin": "workspace:*", "@opentelemetry/api": "1.9.0", "@opentelemetry/context-async-hooks": "2.6.1", "@opentelemetry/exporter-trace-otlp-http": "0.214.0", "@opentelemetry/sdk-trace-base": "2.6.1", "@parcel/watcher": "2.5.1", "@silvia-odwyer/photon-node": "0.3.4", + "@openrouter/ai-sdk-provider": "2.9.0", "ai-gateway-provider": "3.2.0", "bun-pty": "0.4.8", "cross-spawn": "catalog:", @@ -114,8 +114,8 @@ "google-auth-library": "10.5.0", "gray-matter": "4.0.3", "htmlparser2": "8.0.2", - "ignore": "7.0.5", "immer": "11.1.4", + "ignore": "7.0.5", "jsonc-parser": "3.3.1", "mime-types": "3.0.2", "minimatch": "10.2.5", diff --git a/packages/core/src/event.ts b/packages/core/src/event.ts index 9d3145ae96d4..edddf3f64d25 100644 --- a/packages/core/src/event.ts +++ b/packages/core/src/event.ts @@ -4,7 +4,7 @@ import { Cause, Context, Duration, Effect, Layer, Option, PubSub, Queue, Schema 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 { and, asc, eq, gt, inArray, isNull } from "drizzle-orm" import { Database } from "./database/database" import { EventSequenceTable, EventTable } from "./event/sql" import { Flag } from "./flag/flag" @@ -188,6 +188,39 @@ export interface LayerOptions { /** 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`. */ +// Same-run tokens order retries and generated activity IDs. Cross-run age needs a separate epoch. +const supersededBy = (held: string, claimer: string): boolean => { + const split = (token: string) => { + const cut = token.lastIndexOf(":") + const head = token.slice(0, cut) + const idAt = head.lastIndexOf(":") + const id = head.slice(idAt + 1) + return { + run: head.slice(0, idAt), + id, + // Temporal hands out activity ids as an increasing sequence within a run, so they order the + // units of work. A token from an earlier step is a zombie, whatever its attempt number says. + activity: Number(id), + attempt: Number(token.slice(cut + 1)), + } + } + const a = split(held) + const b = split(claimer) + // Different runs cannot be ordered from the tokens alone, and a continue-as-new legitimately + // starts a new one, so those are allowed through. A zombie from a run that rolled over is the + // case this does not cover. + if (a.run !== b.run) return false + const ordered = Number.isInteger(a.activity) && Number.isInteger(b.activity) + // Activity ids are an increasing sequence when Temporal assigns them, but a caller may set its + // own. Without numbers to compare, two different units of work cannot be ordered, and only two + // attempts of the same one can. + if (ordered && a.activity !== b.activity) return a.activity > b.activity + // Compared as written, not as parsed: two ids that are not numbers both parse to NaN, and NaN + // read as equal made every later activity look like a retry of the one before it. + if (!ordered && a.id !== b.id) return false + return Number.isInteger(a.attempt) && Number.isInteger(b.attempt) && a.attempt > b.attempt +} + const DEFAULT_LIVE_POLL = Duration.seconds(1) // An operator's override, in milliseconds, for either node. Read at layer build rather than at @@ -570,13 +603,64 @@ export const layerWith = (options?: LayerOptions) => .pipe(Effect.orDie) } + // A compare and set, not a write. Two attempts of one activity can be alive at once and they + // do not arrive in order: a paused attempt 1 that resumes after attempt 2 has claimed used to + // take the log back, and then every publish from attempt 2's tool activities died on the + // fence for a step that was going fine. function claim(aggregateID: string, ownerID: string) { - return db - .update(EventSequenceTable) - .set({ owner_id: ownerID }) - .where(eq(EventSequenceTable.aggregate_id, aggregateID)) - .run() - .pipe(Effect.orDie) + return Effect.gen(function* () { + const row = yield* db + .select({ ownerID: EventSequenceTable.owner_id }) + .from(EventSequenceTable) + .where(eq(EventSequenceTable.aggregate_id, aggregateID)) + .get() + .pipe(Effect.orDie) + // No sequence row yet, so there is nothing to fence and nothing to lose a race to: the + // first publish inserts the row with this owner on it. + if (row === undefined) return + if (row.ownerID != null && supersededBy(row.ownerID, ownerID)) { + yield* Effect.die( + new InvalidDurableEventError({ + type: "session.claim", + message: `Stale claim for aggregate ${aggregateID}: held by ${row.ownerID}, claimer ${ownerID}`, + }), + ) + } + // Conditional on what was just read, because the read and the write are two statements + // and over a network store they are two requests. Two attempts of one activity reaching + // here together both passed the check above, and an unconditional write let the loser + // land last and fence out the winner's tools. + yield* db + .update(EventSequenceTable) + .set({ owner_id: ownerID }) + .where( + and( + eq(EventSequenceTable.aggregate_id, aggregateID), + row.ownerID == null + ? isNull(EventSequenceTable.owner_id) + : eq(EventSequenceTable.owner_id, row.ownerID), + ), + ) + .run() + .pipe(Effect.orDie) + // Read back rather than trusting a driver-specific affected-row count. Losing means + // somebody claimed between the two statements, and a loser that carried on would publish + // under a token the fence rejects. + const after = yield* db + .select({ ownerID: EventSequenceTable.owner_id }) + .from(EventSequenceTable) + .where(eq(EventSequenceTable.aggregate_id, aggregateID)) + .get() + .pipe(Effect.orDie) + if (after?.ownerID !== ownerID) { + yield* Effect.die( + new InvalidDurableEventError({ + type: "session.claim", + message: `Lost the claim for aggregate ${aggregateID}: held by ${after?.ownerID}, claimer ${ownerID}`, + }), + ) + } + }) } const subscribe = (definition: D): Stream.Stream> => diff --git a/packages/core/src/session/execution/worktree.ts b/packages/core/src/session/execution/worktree.ts index 9d3532d95b8d..0a9259c264ea 100644 --- a/packages/core/src/session/execution/worktree.ts +++ b/packages/core/src/session/execution/worktree.ts @@ -12,9 +12,9 @@ export * as WorktreeMaterializer from "./worktree" // 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 { readdir, rm, writeFile } from "node:fs/promises" import path from "path" -import { Cause, Context, Effect, Layer } from "effect" +import { Cause, Context, Effect, Layer, Schema } from "effect" import { ChildProcess } from "effect/unstable/process" import { and, asc, desc, eq } from "drizzle-orm" import { Database } from "../../database/database" @@ -26,25 +26,77 @@ import { Global } from "../../global" import { AppProcess } from "../../process" import { AbsolutePath } from "../../schema" import { SnapshotPackTable } from "../../snapshot/sql" +import { chainHead, isBehind, orderChain } from "../../snapshot/chain" import { readWorktreeTip, writeWorktreeTip } from "../../snapshot/tip" +import * as Writers from "../../snapshot/writers" export interface Interface { /** * 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. + * alone. + * + * A rebuild that fails dies with `WorktreeMaterializeError` rather than returning: running the + * step against whatever is in the directory tells the model those files are the project, and for + * a fresh host that is nothing at all. Tagged so the activity boundary retries it elsewhere. + * + * `pauseBeforeLock` waits between reading the directory and taking the lock. Zero everywhere but + * the check that reproduces what a concurrent drain does in that gap: nothing outside this module + * can hold a caller there, and what the check asserts is the real outcome, whether a failed + * rebuild removes a directory this call did not create. */ - readonly ensure: (directory: string) => Effect.Effect + readonly ensure: ( + directory: string, + options?: { + readonly pauseBeforeLock?: number + /** + * The step asking for the directory. A call of another step that never came back keeps it, + * because a settled workflow promise does not stop the process behind it. Omitted by callers + * that are not a step, and then any stranded call refuses them. + */ + readonly current?: Writers.Writer + }, + ) => Effect.Effect + + /** Say a call is about to write the directory, and that its body came back. */ + readonly beginWrite: (directory: string, writer: Writers.Writer) => Effect.Effect + readonly endWrite: (directory: string, callID: string) => Effect.Effect } 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. +// HEAD of a rebuilt tree, so the rebuilt repo reads as a clean checkout rather than an unborn +// branch over a full untracked tree. const RESTORED = "refs/heads/opencode-restore" +/** + * The directory holds a call from another step that never came back, so this step may not have it. + * Tagged separately from a rebuild failure because it is not a transient one: it stands until that + * call returns or an operator clears it. + */ +export class WorktreeQuarantinedError extends Schema.TaggedErrorClass()( + "WorktreeMaterializer.QuarantinedError", + { message: Schema.String }, +) {} + +/** A rebuild that did not finish. Tagged so the boundary can tell it from a refusal and retry it. */ +export class WorktreeMaterializeError extends Schema.TaggedErrorClass()( + "WorktreeMaterializer.MaterializeError", + { message: Schema.String }, +) {} + +// Nothing in it at all, so there is no work to protect and nothing to lose by checking a tree out +// over it. Unreadable counts as not empty: a directory we cannot look into is not one to overwrite. +const isEmptyDir = (dir: string) => + Effect.promise(() => + readdir(dir) + .then((entries) => entries.length === 0) + .catch(() => false), + ) + const layer = Layer.effect( Service, Effect.gen(function* () { @@ -61,13 +113,15 @@ const layer = Layer.effect( .create({ worktree, gitDirectory: AbsolutePath.make(path.join(worktree, ".git")) }) .pipe(Effect.orDie) // Index every pack shipped for this worktree; objects accumulate, the newest tree wins. - const rows = yield* db + const stored = yield* db .select() .from(SnapshotPackTable) .where(eq(SnapshotPackTable.worktree, tip.worktree)) - .orderBy(asc(SnapshotPackTable.time_created)) .all() .pipe(Effect.orDie) + // A pack cannot be indexed before the one it was built on, and the write clock does not order + // them: two hosts disagree about the time, and one behind puts its pack first. + const rows = orderChain(stored) const packDirectory = path.join(repository.gitDirectory, "objects", "pack") yield* fs.ensureDir(packDirectory).pipe(Effect.orDie) for (const row of rows) { @@ -120,68 +174,76 @@ const layer = Layer.effect( 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 }) + const rows = yield* db + .select() .from(SnapshotPackTable) - .where(and(eq(SnapshotPackTable.worktree, tip.worktree), eq(SnapshotPackTable.tree, held))) - .orderBy(desc(SnapshotPackTable.time_created)) - .limit(1) - .get() + .where(eq(SnapshotPackTable.worktree, tip.worktree)) + .all() .pipe(Effect.orDie) - return shipped !== undefined && shipped.time < tip.time_created + return isBehind(rows, held) }) - // 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 refuseWhenStranded = Effect.fn("WorktreeMaterializer.refuseWhenStranded")(function* ( + worktree: string, + current?: Writers.Writer, + ) { + const stranded = yield* Writers.strandedWriters(global.data, worktree, current) + if (stranded.length === 0) return + const one = stranded[0] + // Dies, like a rebuild that could not finish: the activity boundary turns it into a failure + // Temporal schedules again, and the next attempt can be taken by a host that is not refused. + return yield* Effect.die( + new WorktreeQuarantinedError({ + message: + `not using ${worktree}: ${stranded.length} tool call(s) from an earlier step never ` + + `returned (${one.callID} of session ${one.sessionID} step ${one.step}, pid ${one.pid}, ` + + `started ${one.started}). Stop them before this directory is used again.`, + }), + ) + }) - const ensure = Effect.fn("WorktreeMaterializer.ensure")(function* (directory: string) { + const ensure = Effect.fn("WorktreeMaterializer.ensure")(function* ( + directory: string, + options?: { readonly pauseBeforeLock?: number; readonly current?: Writers.Writer }, + ) { // 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) - .where(eq(SnapshotPackTable.directory, directory)) - .orderBy(desc(SnapshotPackTable.time_created)) - .limit(1) - .get() - .pipe(Effect.orDie) + const tip = chainHead( + yield* db + .select() + .from(SnapshotPackTable) + .where(eq(SnapshotPackTable.directory, directory)) + .all() + .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 - } - } + // Before anything is rebuilt. A restore is what brings this host to the newest tree, and + // doing that under a call nobody can account for is what makes its later capture look current. + yield* refuseWhenStranded(tip.worktree, options?.current) + // An empty directory is not somebody's working copy, so the rule that protects one does not + // apply to it. Treating it as present is what stops a fresh host from ever building the tree: + // it has no tip note, so `behind` says no, and the tools then run against nothing. A mounted + // path that exists but holds nothing is the ordinary shape of a host that has never seen this + // project, which is exactly the case the packs are for. + const present = (yield* fs.existsSafe(tip.worktree)) && !(yield* isEmptyDir(tip.worktree)) + // `behind` is already the whole rule. It is false unless this host has a note of its own, and + // a note means this host agreed to that state: either it built the tree from packs or it + // captured the tree from here. Moving it forward from a state it agreed to loses nothing. + // + // What used to gate this as well was whether the tree carried the marker `materialize` + // writes. Only a rebuilt tree ever has that, so a host that seeded the session from its own + // checkout never did, and once any other host shipped, every activity that host drew died + // here. A developer's checkout is protected by having no note at all, not by the marker. + if (present && !(yield* behind(tip))) return + if (options?.pauseBeforeLock) yield* Effect.sleep(options.pauseBeforeLock) yield* locks.withLock(tip.worktree)( Effect.gen(function* () { - // Re-check inside the lock: a concurrent drain may have done this already. - if ((yield* fs.existsSafe(tip.worktree)) && !(yield* behind(tip))) return + // Re-check inside the lock: a concurrent drain may have done this already. Same notion of + // present as above, or an empty directory bails out here instead and the tree that the + // outer check just decided to build never gets built. + const here = + (yield* fs.existsSafe(tip.worktree)) && !(yield* isEmptyDir(tip.worktree)) + if (here && !(yield* behind(tip))) return yield* materialize(tip).pipe( Effect.catchCauseIf( (cause) => !Cause.hasInterrupts(cause), @@ -189,13 +251,25 @@ const layer = Layer.effect( Effect.gen(function* () { // 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) + // leaves it as stale as it was. Asked of the reading taken inside the lock, which + // is the only one that describes the directory this attempt started from: the + // outer one is why the re-check exists, and a drain that materialized while this + // one waited makes it name a directory that no longer exists. + if (!here) yield* Effect.promise(() => rm(tip.worktree, { recursive: true, force: true })) - yield* Effect.logWarning("failed to materialize worktree", { + yield* Effect.logError("failed to materialize worktree", { worktree: tip.worktree, cause, }) + // Swallowing this ran the step against whatever was in the directory, which for + // a fresh host is nothing at all. Tagged, so the activity boundary can retry it: + // git and the filesystem fail for reasons that pass, and the alternative is a + // turn that fails for good because one worker had a bad minute. + return yield* Effect.die( + new WorktreeMaterializeError({ + message: `could not materialize ${tip.worktree} at ${tip.tree}`, + }), + ) }), ), ) @@ -203,7 +277,11 @@ const layer = Layer.effect( ) }) - return Service.of({ ensure }) + return Service.of({ + ensure, + beginWrite: (directory, writer) => Writers.beginWrite(global.data, directory, writer), + endWrite: (directory, callID) => Writers.endWrite(global.data, directory, callID), + }) }), ) diff --git a/packages/core/src/session/runner/index.ts b/packages/core/src/session/runner/index.ts index 2a63cf5ae9db..cc6a63dc2a86 100644 --- a/packages/core/src/session/runner/index.ts +++ b/packages/core/src/session/runner/index.ts @@ -36,13 +36,10 @@ 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. */ +/** Carry recorded identities so dispatch reads the matching arguments without duplicating payloads. */ export interface DeferredToolCall { readonly id: string readonly name: string - readonly input: unknown readonly assistantMessageID: string } @@ -59,6 +56,10 @@ export interface SealStepInput { * 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 + /** This step is being closed away from the host that ran it, so the files are not this seal's to + * touch: it is standing in a directory that never saw the tools, and the host that did may still + * be inside one of them. Writing the step down is the whole job here. */ + readonly withoutTheTree?: boolean } /** One recorded tool call, to run on its own. */ @@ -124,16 +125,13 @@ export interface Interface { * 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. */ + /** Recorded admission prevents repeating non-idempotent calls after an uncertain result. */ 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. */ + /** The target message keeps a retried seal from closing a different step. */ readonly sealStep: (input: SealStepInput) => Effect.Effect } diff --git a/packages/core/src/session/runner/llm.ts b/packages/core/src/session/runner/llm.ts index f19a829f313b..79a3504f39f7 100644 --- a/packages/core/src/session/runner/llm.ts +++ b/packages/core/src/session/runner/llm.ts @@ -52,6 +52,7 @@ import { DEFAULT_MAX_STEPS, REPEAT_LIMIT, REPEATED_CALLS_PROMPT, trailingIdentic import { Snapshot } from "../../snapshot" import { SnapshotSync } from "../../snapshot-sync" import { makeLocationNode } from "../../effect/app-node" +import { KeyedMutex } from "../../effect/keyed-mutex" import { llmClient } from "../../effect/app-node-platform" /** @@ -108,6 +109,13 @@ import { llmClient } from "../../effect/app-node-platform" * bound the loop. */ +// Shipping the tree, one at a time per directory. Two tools of one step run at once and both end by +// capturing and pushing: a capture writes the git index and a push compares against the store's +// head, so two of them in one directory race on both, and the loser's work is refused rather than +// shipped. Module-level, because what has to be excluded is two activities in one process, and each +// builds its own runner. +const shipping = KeyedMutex.makeUnsafe() + const layer = Layer.effect( Service, Effect.gen(function* () { @@ -279,7 +287,7 @@ const layer = Layer.effect( return yield* Effect.die(continueAfterCompaction(currentStep)) const startSnapshot = yield* snapshots.capture() // Ship the pre-step tree so another host can rebuild the worktree; best-effort inside push. - if (startSnapshot) yield* snapshotSync.push(startSnapshot) + if (startSnapshot) yield* snapshotSync.push(startSnapshot, session.id) const publisher = createLLMEventPublisher(events, { sessionID: session.id, agent: agent.id, @@ -319,12 +327,7 @@ const layer = Layer.effect( // 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, - }) + deferred.push({ id: event.id, name: event.name, assistantMessageID }) return } yield* Effect.uninterruptibleMask((restore) => @@ -399,7 +402,7 @@ const layer = Layer.effect( 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) + if (endSnapshot) yield* snapshotSync.push(endSnapshot, session.id) const files = startSnapshot && endSnapshot ? yield* snapshots @@ -602,7 +605,7 @@ const layer = Layer.effect( yield* failInterruptedTools(input.sessionID) const startSnapshot = inFlight.snapshot?.start const endSnapshot = yield* snapshots.capture() - if (endSnapshot) yield* snapshotSync.push(endSnapshot) + if (endSnapshot) yield* snapshotSync.push(endSnapshot, input.sessionID) const files = startSnapshot && endSnapshot ? yield* snapshots @@ -688,6 +691,18 @@ const layer = Layer.effect( } const moreQueue = yield* SessionInput.hasPending(db, sessionID, "queue") if (moreQueue) return { ran: true, continue: true, step: 1, promotion: "queue" as SessionInput.Delivery } + // The turn is over, and this is the only place that knows it: a step ending is not a turn + // ending, because a steer or a queued prompt continues the same turn through another step. + // Everything watching from outside had to infer it from a finish reason and a silence. Said + // once, here, for both modes, since both come through this function. + // + // Only the ordinary ending. A turn the user stopped, or one a provider error ended, does not + // reach here, so a follower still needs its other reasons to stop waiting. + yield* events.publish(SessionEvent.Turn.Ended, { + sessionID, + timestamp: yield* DateTime.now, + finish: "stop", + }) return { ran: true, continue: false, step: step + 1, promotion: undefined } }) @@ -727,9 +742,11 @@ const layer = Layer.effect( // 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() + // A seal closing a step away from its host captures a directory that never ran the tools, so + // what it would ship is the state before them. The host that has them is the one that ships. + const endSnapshot = input.withoutTheTree ? undefined : 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) + if (endSnapshot) yield* snapshotSync.push(endSnapshot, input.sessionID) const files = startSnapshot && endSnapshot ? yield* snapshots @@ -809,19 +826,48 @@ const layer = Layer.effect( }) return { outcome: "unknown" } as ToolCallResult } + // The arguments, off the log rather than off the hand-off. A pending call holds the provider's + // raw JSON text, which is what the stream delivered; a re-dispatch of a running one reads the + // object the first dispatch recorded. A defect either way if the text is not JSON, because + // only the recording path could have written that, and a tool handed a string it cannot parse + // reports a wrong reason to the model. + const recorded = part.state + const args = + recorded.status === "pending" + ? yield* Effect.try({ + try: () => JSON.parse(recorded.input) as unknown, + catch: () => + new Error( + recorded.input === "" + ? `Tool call ${input.call.id} has no recorded input to run it with` + : `Tool call ${input.call.id} has a recorded input that is not JSON`, + ), + }).pipe(Effect.orDie) + : recorded.input // 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 }, - }) + yield* events.publish( + SessionEvent.Tool.Called, + { + sessionID: input.sessionID, + timestamp: yield* DateTime.now, + assistantMessageID, + callID: input.call.id, + tool: input.call.name, + input: record(args), + // Deferred calls are never provider-executed: those are filtered out before the hand-off. + provider: { executed: false }, + }, + materialization.idempotent(input.call.name) + ? undefined + : { + // A shared step owner cannot distinguish overlapping dispatches of one call. + id: EventV2.ID.make( + `evt_dispatch_${JSON.stringify([input.sessionID, assistantMessageID, input.call.id])}`, + ), + }, + ) const settlement = yield* materialization .settle({ sessionID: input.sessionID, @@ -830,7 +876,7 @@ const layer = Layer.effect( call: LLMEvent.toolCall({ id: input.call.id, name: input.call.name, - input: input.call.input, + input: args, }), }) .pipe( @@ -877,6 +923,16 @@ const layer = Layer.effect( // Deferred calls are never provider-executed: those are filtered out before the hand-off. provider: { executed: false }, })) + // Shipped from the host that ran the tool, because it is the only one holding what the tool + // did. The seal can land anywhere, and a capture there would ship a tree that never saw this + // write. Best effort in the same sense the seal's is: the result is already durable, and a + // pack that does not reach the store costs the next host a rebuild from further back. + yield* shipping.withLock(location.directory)( + Effect.gen(function* () { + const afterTool = yield* snapshots.capture().pipe(Effect.catch(() => Effect.succeed(undefined))) + if (afterTool) yield* snapshotSync.push(afterTool, input.sessionID) + }), + ) return { outcome: "settled" } as ToolCallResult }) diff --git a/packages/core/src/session/runner/publish-llm-event.ts b/packages/core/src/session/runner/publish-llm-event.ts index d634b86e8771..fd345cccf47b 100644 --- a/packages/core/src/session/runner/publish-llm-event.ts +++ b/packages/core/src/session/runner/publish-llm-event.ts @@ -114,6 +114,9 @@ export const createLLMEventPublisher = (events: EventV2.Interface, input: Input) { readonly assistantMessageID: SessionMessage.ID readonly name: string + // Whether the provider streamed the arguments. One that delivers the call whole sends none, + // and the fragment end would then record an empty input for a call that has one. + inputSeen: boolean inputEnded: boolean called: boolean settled: boolean @@ -225,6 +228,7 @@ export const createLLMEventPublisher = (events: EventV2.Interface, input: Input) tools.set(event.id, { assistantMessageID, name: event.name, + inputSeen: false, inputEnded: false, called: false, settled: false, @@ -354,6 +358,7 @@ export const createLLMEventPublisher = (events: EventV2.Interface, input: Input) if (tool.name !== event.name) return yield* Effect.die(`Tool input name changed for ${event.id}: ${tool.name} -> ${event.name}`) if (tool.inputEnded) return yield* Effect.die(`Tool input delta after end: ${event.id}`) + tool.inputSeen = true yield* toolInput.append(event.id, event.text) yield* events.publish(SessionEvent.Tool.Input.Delta, { sessionID: input.sessionID, @@ -370,6 +375,11 @@ export const createLLMEventPublisher = (events: EventV2.Interface, input: Input) case "tool-call": { if (!tools.has(event.id)) yield* startToolInput(event) const tool = tools.get(event.id)! + // The call carries the arguments whether or not they were streamed, and the record has to + // hold them either way: it is what a dispatcher reads to run the tool, and the fragment end + // would otherwise write an empty input for a provider that sends no deltas. + if (!tool.inputEnded && !tool.inputSeen) + yield* toolInput.append(event.id, JSON.stringify(event.input ?? {})) if (!tool.inputEnded) yield* endToolInput(event) if (tool.name !== event.name) return yield* Effect.die(`Tool call name changed for ${event.id}: ${tool.name} -> ${event.name}`) diff --git a/packages/core/src/snapshot-sync.ts b/packages/core/src/snapshot-sync.ts index ede416f8300a..97133da3a523 100644 --- a/packages/core/src/snapshot-sync.ts +++ b/packages/core/src/snapshot-sync.ts @@ -1,9 +1,6 @@ export * as SnapshotSync from "./snapshot-sync" -// Ships captured snapshot trees to the shared store as git packs, so a worker on another host can -// rebuild the project worktree before it drains a session (see session/execution/worktree.ts). -// Each push wraps the tree in a sync commit chained onto the previous push and packs only the -// delta. Best-effort by design: a failed push degrades portability, never the turn. +// Packs let a worker rebuild tracked files without sharing the live directory. import { readFile, rm } from "node:fs/promises" import os from "node:os" @@ -13,6 +10,8 @@ import { ChildProcess } from "effect/unstable/process" import { desc, eq } from "drizzle-orm" import { Database } from "./database/database" import { makeLocationNode } from "./effect/app-node" +import { EventV2 } from "./event" +import { EventSequenceTable } from "./event/sql" import { FSUtil } from "./fs-util" import { Git } from "./git" import { Global } from "./global" @@ -21,12 +20,20 @@ import { AppProcess } from "./process" import { AbsolutePath } from "./schema" import type { Snapshot } from "./snapshot" import { SnapshotPackTable } from "./snapshot/sql" -import { writeWorktreeTip } from "./snapshot/tip" +import { chainHead } from "./snapshot/chain" +import { readWorktreeTip, writeWorktreeTip } from "./snapshot/tip" import { Hash } from "./util/hash" export interface Interface { - /** Ship a captured tree to the shared store as an incremental pack. Never fails the caller. */ - readonly push: (tree: Snapshot.ID) => Effect.Effect + /** + * A stale tip fails the caller; packing and insertion errors are logged. + * + * `sessionID` is what the files are being shipped for. Given it, a publisher a newer attempt has + * superseded is refused, which is the one case the tip check cannot answer: a tool whose dispatch + * was abandoned is still standing on the tree it read, so its pack is clean and reverts whatever + * ran in its place. Omitted by callers with no session behind them, and then nothing is fenced. + */ + readonly push: (tree: Snapshot.ID, sessionID?: string) => Effect.Effect } export class Service extends Context.Service()("@opencode/v2/SnapshotSync") {} @@ -62,23 +69,63 @@ const layer = Layer.effect( { stdin }, ) - 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) + // The newest state the store holds for this worktree, read off the chain the packs form rather + // than off `time_created`, which is whichever host wrote the row. + const newest = () => + db + .select() + .from(SnapshotPackTable) + .where(eq(SnapshotPackTable.worktree, worktree)) + .all() + .pipe(Effect.orDie, Effect.map(chainHead)) + + // Whether a newer attempt holds this session's log. The token is the one the drain claimed and + // provided, so this asks the same question the log's own fence asks of every append: is what is + // writing still the attempt the session is on? + const superseded = Effect.fn("SnapshotSync.superseded")(function* (sessionID: string) { + const owner = yield* EventV2.EventOwner + // Outside a drain nothing claimed anything, so there is nothing to be superseded by. + if (owner === undefined) return false + const row = yield* db + .select({ ownerID: EventSequenceTable.owner_id }) + .from(EventSequenceTable) + .where(eq(EventSequenceTable.aggregate_id, sessionID)) + .get() + .pipe(Effect.orDie) + return row?.ownerID != null && row.ownerID !== owner + }) + + const push = Effect.fn("SnapshotSync.push")(function* (tree: Snapshot.ID, sessionID?: string) { + // The files travel under the token the transcript does. A dispatch the session stopped + // waiting for keeps running, and what it publishes afterwards would otherwise be the newest + // state the store holds, because it is standing exactly where it was told to stand. + if (source && sessionID && (yield* superseded(sessionID))) { + return yield* Effect.die( + new Error(`refusing to ship ${worktree}: a newer attempt holds ${sessionID}`), + ) + } + // A host that has not caught up must not publish its older files as the next tree. + // This reading is not an atomic head claim and does not fence concurrent publishers. + if (source) { + const stoodOn = yield* readWorktreeTip(global.data, worktree) + const ahead = yield* newest() + if (ahead && ahead.tree !== tree && stoodOn !== ahead.tree) { + yield* Effect.die( + new Error( + `refusing to ship ${worktree}: this host stood on ${stoodOn ?? "nothing"}, ` + + `and the store is at ${ahead.tree}`, + ), + ) + } + } yield* Effect.gen(function* () { if (!source) return - const latest = yield* db - .select() - .from(SnapshotPackTable) - .where(eq(SnapshotPackTable.worktree, worktree)) - .orderBy(desc(SnapshotPackTable.time_created)) - .limit(1) - .get() - .pipe(Effect.orDie) - // The newest shipped state already is this tree: nothing to pack. - if (latest?.tree === tree) return + const latest = yield* newest() + // The newest shipped state already is this tree: nothing to pack, and the note is true. + if (latest?.tree === tree) { + yield* writeWorktreeTip(global.data, worktree, tree) + return + } // Chain onto the previous sync commit only when this host has it; a base absent locally // would produce a delta pack the pack builder cannot compute. const base = @@ -115,6 +162,8 @@ const layer = Layer.effect( .onConflictDoNothing() .run() .pipe(Effect.orDie) + // A note must not name a state whose insertion failed. + yield* writeWorktreeTip(global.data, worktree, tree) }).pipe( Effect.catchCauseIf( (cause) => !Cause.hasInterrupts(cause), diff --git a/packages/core/src/snapshot/chain.ts b/packages/core/src/snapshot/chain.ts new file mode 100644 index 000000000000..e8fb2a229430 --- /dev/null +++ b/packages/core/src/snapshot/chain.ts @@ -0,0 +1,72 @@ +// Parent depth avoids ordering an intact chain by clocks from different hosts. +// Forks still use the timestamp tiebreaker. Ordering does not reject a competing publication. + +export interface ChainRow { + readonly id: string + readonly base: string | null + readonly time_created: number +} + +const depths = (rows: readonly T[]): Map => { + const byID = new Map(rows.map((row) => [row.id, row])) + const depth = new Map() + // Iterative, because the chain is one link per capture and nothing prunes it: a long session + // would put a stack frame per tool call that changed a file. + for (const start of rows) { + if (depth.has(start.id)) continue + const pending: T[] = [] + const seen = new Set() + let at: T | undefined = start + while (at && !depth.has(at.id) && !seen.has(at.id)) { + seen.add(at.id) + pending.push(at) + at = at.base ? (byID.get(at.base) as T | undefined) : undefined + } + // A root, a row whose base is not in the store, or a cycle: all start the count at zero. + let below = at && depth.has(at.id) ? depth.get(at.id)! : -1 + for (const row of pending.reverse()) depth.set(row.id, ++below) + } + return depth +} + +/** Packs in an order where a pack's base always comes before it, which is what indexing them needs. */ +export const orderChain = (rows: readonly T[]): T[] => { + const depth = depths(rows) + return [...rows].sort( + (a, b) => (depth.get(a.id) ?? 0) - (depth.get(b.id) ?? 0) || a.time_created - b.time_created, + ) +} + +/** The newest state the store holds, which is the deepest link in the chain. */ +export const chainHead = (rows: readonly T[]): T | undefined => { + const depth = depths(rows) + let head: T | undefined + for (const row of rows) { + if (!head) { + head = row + continue + } + const here = depth.get(row.id) ?? 0 + const best = depth.get(head.id) ?? 0 + if (here > best || (here === best && row.time_created > head.time_created)) head = row + } + return head +} + +/** + * Whether `tree` is an earlier state than the head, as opposed to one the store has never seen. + * A tree the store does not hold is this host's own uncaptured work, and moving off it would drop + * work nothing else has. + */ +export const isBehind = ( + rows: readonly T[], + tree: string, +): boolean => { + const head = chainHead(rows) + if (!head || head.tree === tree) return false + const depth = depths(rows) + const mine = rows.filter((row) => row.tree === tree) + if (mine.length === 0) return false + const deepest = Math.max(...mine.map((row) => depth.get(row.id) ?? 0)) + return deepest < (depth.get(head.id) ?? 0) +} diff --git a/packages/core/src/snapshot/writers.ts b/packages/core/src/snapshot/writers.ts new file mode 100644 index 000000000000..9adf17f6d3d8 --- /dev/null +++ b/packages/core/src/snapshot/writers.ts @@ -0,0 +1,243 @@ +// Which tool calls are inside their own execution on this host, kept in this host's data directory. +// +// A timeout settles the workflow's promise; it does not stop the process behind it. So after a +// step is abandoned, the host can still be running that step's tool, and nothing the workflow can +// see says whether it is. The step that was abandoned is protected by refusing to move it. What is +// not protected by that is everything after: the turn ends, the next prompt lands wherever there is +// room, and the directory that tool is writing is free again. +// +// A marker is written before a call can have any effect and removed when its body returns. While +// one stands, the directory belongs to that call's step and no other step may rebuild or capture +// it. The refusal outlives the process that made it, and takes itself back where this host can show +// that it is over: the writer's process is gone, nothing carrying its name is still running, +// nothing is left in its process group, and the machine has not restarted underneath the pids that +// say so. What is left after that is a tool that both left the group and was handed an environment +// of somebody else's choosing. Until then the directory stays refused, which strands a directory +// and not a session: the work is scheduled again and another host can take it. + +import { execFile } from "node:child_process" +import { randomBytes } from "node:crypto" +import { mkdir, readdir, readFile, rm, writeFile } from "node:fs/promises" +import os from "node:os" +import path from "path" +import { promisify } from "node:util" +import { Effect } from "effect" +import { Hash } from "../util/hash" + +const run = promisify(execFile) + +// Put in the environment rather than kept in memory, because the point of it is to be inherited: +// a tool's children carry it wherever they end up, including through `setsid`, and a scan finds +// them after the worker that spawned them is gone. Fresh per process, so a worker never answers for +// its predecessor's children, and set at load rather than at the first write, before anything can +// be spawned. +const WORKER_ENV = "OPENCODE_WORKTREE_WRITER" +const worker = (process.env[WORKER_ENV] = randomBytes(8).toString("hex")) + +/** What a tool call is, for telling this step's writers from an earlier one's. */ +export interface Writer { + readonly sessionID: string + readonly step: number + readonly callID: string +} + +interface WriterNote extends Writer { + readonly pid: number + // The group the writer's process was in. What a tool starts stays in it unless it asks for a + // group of its own, which is what every daemonizing wrapper does. + readonly pgid?: number + // The worker that ran the call, as a name it put in its own environment. Everything a tool starts + // inherits it, so this is what finds a child the group check lost. + readonly worker?: string + // When this machine last started, so a pid from before a restart is not read as a live one. + readonly bootAt?: number + readonly host?: string + readonly started: string +} + +// os.uptime has second granularity and drifts between reads, so this is a stamp to compare with a +// tolerance rather than an identifier. A restart moves it by the whole of the last uptime. +const bootAt = () => Math.round(Date.now() - os.uptime() * 1000) +const SAME_BOOT = 60_000 + +const alive = (pid: number) => { + try { + process.kill(pid, 0) + return true + } catch (err) { + // Somebody else's process is still a process. + return (err as NodeJS.ErrnoException).code === "EPERM" + } +} + +/** What is running on this host: which groups hold something, and which workers' work is still in + * them. Undefined when the host cannot be asked, which is answered as "everything is still here". + * `ps` is not installed on a slim container image, which is where most of these run, so Linux is + * read from `/proc` and everything else asks `ps`. */ +const liveHere = async (): Promise<{ groups: Set; workers: Set } | undefined> => { + const groups = new Set() + const workers = new Set() + const found = (text: string) => { + // The environment is NUL-separated in `/proc` and space-separated in `ps`, so this reads the + // name off either without pretending to parse the whole of it. + const at = text.indexOf(`${WORKER_ENV}=`) + if (at >= 0) workers.add(text.slice(at + WORKER_ENV.length + 1).split(/[\0\s]/)[0]) + } + try { + if (process.platform === "linux") { + for (const name of await readdir("/proc")) { + if (!/^\d+$/.test(name) || name === String(process.pid)) continue + const stat = await readFile(`/proc/${name}/stat`, "utf8").catch(() => undefined) + // A command can hold spaces and brackets, so the fields after it are counted from the last + // close bracket: state, ppid, pgrp. + const pgrp = stat ? Number.parseInt(stat.slice(stat.lastIndexOf(")") + 2).split(" ")[2], 10) : NaN + if (Number.isFinite(pgrp)) groups.add(pgrp) + // Readable for this user's processes, which is what a tool of ours is. Anything else is not + // something this worker started. + found(await readFile(`/proc/${name}/environ`, "utf8").catch(() => "")) + } + return { groups, workers } + } + // `-E` prints each process's environment after its command, for the processes this user owns. + // Without the name in its own environment: `ps` is a child of this process, so it inherits + // whatever we hold, and it would otherwise report itself as work this worker left running. + const { [WORKER_ENV]: _ours, ...env } = process.env + const { stdout } = await run("ps", ["-A", "-E", "-o", "pid=,pgid=,command="], { + maxBuffer: 32 * 1024 * 1024, + env, + }) + for (const line of stdout.split("\n")) { + const [pid, pgid] = line + .trim() + .split(/\s+/, 2) + .map((n) => Number.parseInt(n, 10)) + if (pid === process.pid) continue + if (Number.isFinite(pgid)) groups.add(pgid) + found(line) + } + return { groups, workers } + } catch { + return undefined + } +} + +const readGroup = async (pid: number): Promise => { + try { + if (process.platform === "linux") { + const stat = await readFile(`/proc/${pid}/stat`, "utf8") + const pgrp = Number.parseInt(stat.slice(stat.lastIndexOf(")") + 2).split(" ")[2], 10) + return Number.isFinite(pgrp) ? pgrp : undefined + } + const { stdout } = await run("ps", ["-o", "pgid=", "-p", String(pid)]) + const pgid = Number.parseInt(stdout.trim(), 10) + return Number.isFinite(pgid) ? pgid : undefined + } catch { + return undefined + } +} + +// Once per process: a process cannot change the group it is in. +let ourGroup: Promise | undefined +const group = () => (ourGroup ??= readGroup(process.pid)) + +/** + * Whether this marker can still be a tool inside its own execution, which is the only thing the + * refusal is worth its cost for. Everything here is a reason to stop refusing, never a reason to + * start: what cannot be answered is answered as still running. + */ +const maybeInside = async ( + note: WriterNote, + live: Awaited>, +): Promise => { + // A pid from another machine says nothing here, and two hosts sharing one data directory is the + // only way to get one. Neither of them can see the other's processes. + if (note.host !== undefined && note.host !== os.hostname()) return true + // Written by a worker that did not date its marker, so there is nothing to tell a live pid from + // a reused one. + if (note.bootAt === undefined) return true + // The machine restarted. Nothing it was running came back with it. + if (Math.abs(note.bootAt - bootAt()) > SAME_BOOT) return false + if (alive(note.pid)) return true + // The writer is gone, and what a tool starts can outlive it. Nothing here is asked of the pids + // themselves, which come round again; it is asked of what those processes are carrying. + if (live === undefined) return true + // Anything the worker started, wherever it ended up. A tool that daemonizes leaves the group and + // keeps the environment, which is why this is the check that decides most cases. + if (note.worker !== undefined && live.workers.has(note.worker)) return true + // And the group, for a tool that was given an environment of somebody else's choosing. Only when + // this process is somewhere else: a worker restarted from the same shell is in the group its + // predecessor was in, and finding ourselves there is not evidence about anything. + if (note.pgid !== undefined && note.pgid !== (await group())) return live.groups.has(note.pgid) + return false +} + +const writersDir = (data: string, directory: string) => path.join(data, "worktree-writers", Hash.fast(directory)) + +const writerFile = (data: string, directory: string, callID: string) => + path.join(writersDir(data, directory), `${Hash.fast(callID)}.json`) + +/** Say a call is about to write this directory. `endWrite` says its body came back. */ +export const beginWrite = (data: string, directory: string, writer: Writer) => + Effect.promise(async () => { + const file = writerFile(data, directory, writer.callID) + await mkdir(path.dirname(file), { recursive: true }).catch(() => {}) + const note: WriterNote = { + ...writer, + pid: process.pid, + ...((await group()) === undefined ? {} : { pgid: await group() }), + worker, + bootAt: bootAt(), + host: os.hostname(), + started: new Date().toISOString(), + } + await writeFile(file, JSON.stringify(note)).catch(() => {}) + }) + +/** Whatever it did to the directory, it is not still doing it. */ +export const endWrite = (data: string, directory: string, callID: string) => + Effect.promise(() => rm(writerFile(data, directory, callID), { force: true }).catch(() => {})) + +/** + * The calls of some other step that never came back. A step's own tools run at once on one host by + * design, so their markers are not a reason to refuse; a marker from another step is the case the + * workflow cannot see. + */ +export const strandedWriters = (data: string, directory: string, current?: Writer) => + Effect.promise(async () => { + const dir = writersDir(data, directory) + const names = await readdir(dir).catch(() => [] as string[]) + if (names.length === 0) return [] + const live = await liveHere() + const found: WriterNote[] = [] + for (const name of names) { + const file = path.join(dir, name) + const text = await readFile(file, "utf8").catch(() => undefined) + if (text === undefined) continue + let note: WriterNote + try { + note = JSON.parse(text) as WriterNote + } catch { + // A note nothing can parse is still a note somebody wrote before running a tool. + found.push({ sessionID: "unknown", step: -1, callID: name, pid: -1, started: "unknown" }) + continue + } + // A marker that cannot be a live tool any more is dropped rather than reported: the refusal + // exists because nothing could prove the tool stopped, so where something can, it stops + // standing. Its own step's siblings are not a refusal either way. + if (!(await maybeInside(note, live))) { + await rm(file, { force: true }).catch(() => {}) + continue + } + if (!current || note.sessionID !== current.sessionID || note.step !== current.step) found.push(note) + } + return found + }) + +/** Forget them, for an operator who has stopped whatever was left running. */ +export const clearWriters = (data: string, directory: string) => + Effect.promise(async () => { + const dir = writersDir(data, directory) + const names = await readdir(dir).catch(() => [] as string[]) + await rm(dir, { recursive: true, force: true }).catch(() => {}) + return names.length + }) diff --git a/packages/core/src/testing/effect.ts b/packages/core/src/testing/effect.ts deleted file mode 100644 index 131ec5cc6bc2..000000000000 --- a/packages/core/src/testing/effect.ts +++ /dev/null @@ -1,53 +0,0 @@ -import { test, type TestOptions } from "bun:test" -import { Cause, Effect, Exit, Layer } from "effect" -import type * as Scope from "effect/Scope" -import * as TestClock from "effect/testing/TestClock" -import * as TestConsole from "effect/testing/TestConsole" - -type Body = Effect.Effect | (() => Effect.Effect) - -const body = (value: Body) => Effect.suspend(() => (typeof value === "function" ? value() : value)) - -const run = (value: Body, layer: Layer.Layer) => - Effect.gen(function* () { - const exit = yield* body(value).pipe(Effect.scoped, Effect.provide(layer), Effect.exit) - if (Exit.isFailure(exit)) { - for (const err of Cause.prettyErrors(exit.cause)) { - yield* Effect.logError(err) - } - } - return yield* exit - }).pipe(Effect.runPromise) - -const make = (testLayer: Layer.Layer, liveLayer: Layer.Layer) => { - const effect = (name: string, value: Body, opts?: number | TestOptions) => - test(name, () => run(value, testLayer), opts) - - effect.only = (name: string, value: Body, opts?: number | TestOptions) => - test.only(name, () => run(value, testLayer), opts) - - effect.skip = (name: string, value: Body, opts?: number | TestOptions) => - test.skip(name, () => run(value, testLayer), opts) - - const live = (name: string, value: Body, opts?: number | TestOptions) => - test(name, () => run(value, liveLayer), opts) - - live.only = (name: string, value: Body, opts?: number | TestOptions) => - test.only(name, () => run(value, liveLayer), opts) - - live.skip = (name: string, value: Body, opts?: number | TestOptions) => - test.skip(name, () => run(value, liveLayer), opts) - - return { effect, live } -} - -// Test environment with TestClock and TestConsole -const testEnv = Layer.mergeAll(TestConsole.layer, TestClock.layer()) - -// Live environment - uses real clock, but keeps TestConsole for output capture -const liveEnv = TestConsole.layer - -export const it = make(testEnv, liveEnv) - -export const testEffect = (layer: Layer.Layer) => - make(Layer.provideMerge(layer, testEnv), Layer.provideMerge(layer, liveEnv)) diff --git a/packages/core/test/event-claim.test.ts b/packages/core/test/event-claim.test.ts new file mode 100644 index 000000000000..04a0a6194e7f --- /dev/null +++ b/packages/core/test/event-claim.test.ts @@ -0,0 +1,131 @@ +// The compare-and-set in `claim`, under the interleaving it exists for. +// +// Two attempts of one activity claim the log from two processes, so both can read the current owner +// before either writes. A pair of claims started together in one process never does that: they run +// to completion one after the other, which is why the concurrent test beside this one passes with +// the fix reverted. The seam here is at the database, not in `claim`: reads of the sequence table +// wait for each other while the barrier is armed, and `claim` itself is untouched. +import { describe, expect } from "bun:test" +import { Deferred, Effect, Exit, Layer } from "effect" +import { eq } from "drizzle-orm" +import { EventV2 } from "@opencode-ai/core/event" +import { EventSequenceTable } from "@opencode-ai/core/event/sql" +import { Database } from "@opencode-ai/core/database/database" +import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" +import { LayerNode } from "@opencode-ai/core/effect/layer-node" +import { Location } from "@opencode-ai/core/location" +import { AbsolutePath } from "@opencode-ai/core/schema" +import { WorkspaceV2 } from "@opencode-ai/core/workspace" +import { Session } from "@opencode-ai/schema/session" +import { SessionV1 } from "@opencode-ai/schema/session-v1" +import { location } from "./fixture/location" +import { testEffect } from "./lib/effect" + +const locationLayer = Layer.succeed( + Location.Service, + Location.Service.of( + location({ directory: AbsolutePath.make("project"), workspaceID: WorkspaceV2.ID.make("wrk_test") }), + ), +) + +// While armed, the first `want` reads wait for each other and are then released together. +const barrier = { + held: 0, + want: 0, + gate: undefined as Deferred.Deferred | undefined, +} + +const hold = () => + Effect.gen(function* () { + const gate = barrier.gate + if (!gate || barrier.want === 0) return + barrier.held++ + if (barrier.held >= barrier.want) { + barrier.want = 0 + yield* Deferred.succeed(gate, void 0) + return + } + yield* Deferred.await(gate) + }) + +// Waits after the read rather than before it. What has to interleave is two claims that both saw +// the same owner; holding before the read would serialize them and prove nothing. +const gated = (db: any): any => { + const wrap = (node: any): any => + new Proxy(node, { + get(target, prop, recv) { + const value = Reflect.get(target, prop, recv) + if (typeof value !== "function") return value + if (prop === "get" || prop === "all") + return (...args: any[]) => value.apply(target, args).pipe(Effect.tap(() => hold())) + return (...args: any[]) => { + const out = value.apply(target, args) + return out && typeof out === "object" ? wrap(out) : out + } + }, + }) + return new Proxy(db, { + get(target, prop, recv) { + const value = Reflect.get(target, prop, recv) + if (prop !== "select") return typeof value === "function" ? value.bind(target) : value + return (...args: any[]) => wrap(value.apply(target, args)) + }, + }) +} + +const gatedDatabase = Layer.effect( + Database.Service, + Effect.gen(function* () { + const real = yield* Database.Service + return { db: gated(real.db) } + }), +).pipe(Layer.provide(Database.layerFromPath(":memory:"))) + +const it = testEffect( + AppNodeBuilder.build(LayerNode.group([Database.node, EventV2.node, Location.node]), [ + [Location.node, locationLayer], + [Database.node, gatedDatabase], + ]), +) + +const DurableMessage = SessionV1.Event.MessageRemoved + +describe("claim under a real interleaving", () => { + it.effect("only one of two claims that read the same owner is told it won", () => + Effect.gen(function* () { + const events = yield* EventV2.Service + const aggregateID = Session.ID.create() + yield* events.publish(DurableMessage, { + sessionID: aggregateID, + messageID: SessionV1.MessageID.ascending("msg_seed"), + }) + yield* events.claim(aggregateID, "run:11:1") + + barrier.gate = yield* Deferred.make() + barrier.held = 0 + barrier.want = 2 + + const outcomes = yield* Effect.all( + ["run:11:2", "run:11:3"].map((token) => events.claim(aggregateID, token).pipe(Effect.exit)), + { concurrency: "unbounded" }, + ) + barrier.gate = undefined + barrier.want = 0 + + const { db } = yield* Database.Service + const row = yield* db + .select({ ownerID: EventSequenceTable.owner_id }) + .from(EventSequenceTable) + .where(eq(EventSequenceTable.aggregate_id, aggregateID)) + .get() + + // Asserted first, because without it the rest proves nothing: it says both claims really did + // read the same owner before either wrote. + expect(barrier.held).toBe(2) + // The one that loses must be told so. Two winners means the loser goes on to publish under a + // token the log has already fenced, and its tools die on a step that is running. + expect(outcomes.filter(Exit.isSuccess).length).toBe(1) + expect(["run:11:2", "run:11:3"]).toContain(row?.ownerID ?? "") + }), + ) +}) diff --git a/packages/core/test/event.test.ts b/packages/core/test/event.test.ts index d45b2311faca..e788d2344bfc 100644 --- a/packages/core/test/event.test.ts +++ b/packages/core/test/event.test.ts @@ -775,6 +775,73 @@ describe("EventV2", () => { }), ) + // Two attempts of one activity can be alive at once, and they do not arrive in order. A paused + // attempt 1 resuming after attempt 2 has claimed used to take the log back, which fenced out the + // tool activities of the step that was actually going. + it.effect("a resumed earlier attempt cannot take the log back", () => + Effect.gen(function* () { + const events = yield* EventV2.Service + const aggregateID = Session.ID.create() + yield* events.publish(DurableMessage, durableData(aggregateID, "seed")) + + // Activity ids as Temporal writes them: an increasing sequence within the run. + yield* events.claim(aggregateID, "run:11:1") + yield* events.claim(aggregateID, "run:11:2") + const stale = yield* events.claim(aggregateID, "run:11:1").pipe(Effect.exit) + expect(Exit.isFailure(stale)).toBe(true) + + const { db } = yield* Database.Service + const row = yield* db + .select({ ownerID: EventSequenceTable.owner_id }) + .from(EventSequenceTable) + .where(eq(EventSequenceTable.aggregate_id, aggregateID)) + .get() + expect(row?.ownerID).toBe("run:11:2") + + // A later activity is a different unit of work, so it still takes the log: this is the seal + // claiming after the model call, not a zombie. + yield* events.claim(aggregateID, "run:12:1") + + // And an earlier one never does, whatever its attempt says. A model call paused before it + // claimed, with three steps completing under other activity ids while it was away, used to + // come back and fence out the step that was actually running. + const fromAnEarlierStep = yield* events.claim(aggregateID, "run:4:1").pipe(Effect.exit) + expect(Exit.isFailure(fromAnEarlierStep)).toBe(true) + }), + ) + + // What this pins is the outcome, not the race: whoever the row names is the one that was told it + // won. It does NOT pin the compare-and-set that makes that true under a real interleaving. Two + // claims started together here run to completion one after the other, so this passes with the + // condition on the write removed. `event-claim.test.ts` forces that interleaving, with the seam + // at the database rather than in `claim`. Named for what it does. + it.effect("two claims for one log leave a single owner, and it is one that was told so", () => + Effect.gen(function* () { + const events = yield* EventV2.Service + const aggregateID = Session.ID.create() + yield* events.publish(DurableMessage, durableData(aggregateID, "seed")) + + const outcomes = yield* Effect.all( + ["run:11:1", "run:11:2"].map((token) => events.claim(aggregateID, token).pipe(Effect.exit)), + { concurrency: "unbounded" }, + ) + const won = outcomes.filter(Exit.isSuccess).length + + const { db } = yield* Database.Service + const row = yield* db + .select({ ownerID: EventSequenceTable.owner_id }) + .from(EventSequenceTable) + .where(eq(EventSequenceTable.aggregate_id, aggregateID)) + .get() + + // Whoever the row names is the one that must have been told it won. Any other pairing means a + // claimer carried on believing it held a log it does not. + expect(won).toBeGreaterThanOrEqual(1) + expect(["run:11:1", "run:11:2"]).toContain(row?.ownerID ?? "") + if (won === 2) expect(row?.ownerID).toBe("run:11:2") + }), + ) + it.effect("claim fences replay owners", () => Effect.gen(function* () { const events = yield* EventV2.Service diff --git a/packages/core/test/lib/effect.ts b/packages/core/test/lib/effect.ts index ad94c5fe615d..131ec5cc6bc2 100644 --- a/packages/core/test/lib/effect.ts +++ b/packages/core/test/lib/effect.ts @@ -1,3 +1,53 @@ -// Re-export so the test tree keeps its historical import path; the implementation lives in src so -// packages outside core (and the conformance suite) can use the same harness. -export * from "@opencode-ai/core/testing/effect" +import { test, type TestOptions } from "bun:test" +import { Cause, Effect, Exit, Layer } from "effect" +import type * as Scope from "effect/Scope" +import * as TestClock from "effect/testing/TestClock" +import * as TestConsole from "effect/testing/TestConsole" + +type Body = Effect.Effect | (() => Effect.Effect) + +const body = (value: Body) => Effect.suspend(() => (typeof value === "function" ? value() : value)) + +const run = (value: Body, layer: Layer.Layer) => + Effect.gen(function* () { + const exit = yield* body(value).pipe(Effect.scoped, Effect.provide(layer), Effect.exit) + if (Exit.isFailure(exit)) { + for (const err of Cause.prettyErrors(exit.cause)) { + yield* Effect.logError(err) + } + } + return yield* exit + }).pipe(Effect.runPromise) + +const make = (testLayer: Layer.Layer, liveLayer: Layer.Layer) => { + const effect = (name: string, value: Body, opts?: number | TestOptions) => + test(name, () => run(value, testLayer), opts) + + effect.only = (name: string, value: Body, opts?: number | TestOptions) => + test.only(name, () => run(value, testLayer), opts) + + effect.skip = (name: string, value: Body, opts?: number | TestOptions) => + test.skip(name, () => run(value, testLayer), opts) + + const live = (name: string, value: Body, opts?: number | TestOptions) => + test(name, () => run(value, liveLayer), opts) + + live.only = (name: string, value: Body, opts?: number | TestOptions) => + test.only(name, () => run(value, liveLayer), opts) + + live.skip = (name: string, value: Body, opts?: number | TestOptions) => + test.skip(name, () => run(value, liveLayer), opts) + + return { effect, live } +} + +// Test environment with TestClock and TestConsole +const testEnv = Layer.mergeAll(TestConsole.layer, TestClock.layer()) + +// Live environment - uses real clock, but keeps TestConsole for output capture +const liveEnv = TestConsole.layer + +export const it = make(testEnv, liveEnv) + +export const testEffect = (layer: Layer.Layer) => + make(Layer.provideMerge(layer, testEnv), Layer.provideMerge(layer, liveEnv)) diff --git a/packages/core/src/session/execution/conformance.ts b/packages/core/test/lib/execution-conformance.ts similarity index 99% rename from packages/core/src/session/execution/conformance.ts rename to packages/core/test/lib/execution-conformance.ts index c5e1c4b93c58..b1cdce2d7c5d 100644 --- a/packages/core/src/session/execution/conformance.ts +++ b/packages/core/test/lib/execution-conformance.ts @@ -42,7 +42,7 @@ import { describe, expect } from "bun:test" import { realpathSync } from "node:fs" import { tmpdir } from "node:os" import { Cause, Context, DateTime, Effect, Exit, Layer, Schema, Stream } from "effect" -import { testEffect } from "../../testing/effect" +import { testEffect } from "./effect" // The per-location service build resolves the session directory on disk, so it must exist. const WORKSPACE = AbsolutePath.make(realpathSync(tmpdir())) diff --git a/packages/core/test/session-execution-local.test.ts b/packages/core/test/session-execution-local.test.ts index 1b5930fe2e5e..fd8bcd4b5c5f 100644 --- a/packages/core/test/session-execution-local.test.ts +++ b/packages/core/test/session-execution-local.test.ts @@ -3,6 +3,6 @@ // against the Temporal executor in packages/temporal; the shared suite is what holds any executor // to one behavior. import { SessionExecutionLocal } from "@opencode-ai/core/session/execution/local" -import { makeExecutionFor, runContract } from "@opencode-ai/core/session/execution/conformance" +import { makeExecutionFor, runContract } from "./lib/execution-conformance" runContract("local executor", makeExecutionFor(SessionExecutionLocal.node)) diff --git a/packages/core/test/session-runner-model-call.test.ts b/packages/core/test/session-runner-model-call.test.ts index b5527599eca1..614c6da56759 100644 --- a/packages/core/test/session-runner-model-call.test.ts +++ b/packages/core/test/session-runner-model-call.test.ts @@ -51,8 +51,8 @@ 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 { describe, expect, spyOn } from "bun:test" +import { Cause, Deferred, Effect, Exit, Fiber, Layer, Schema, Stream } from "effect" import { testEffect } from "./lib/effect" const model = OpenAIChat.route @@ -124,6 +124,27 @@ const callsCrashingIdempotentTool: LLMClientShape["stream"] = () => LLMEvent.toolCall({ id: "call_probe", name: "probe_crashes_read", input: {} }), LLMEvent.stepFinish({ index: 0, reason: "tool-calls" }), ]) +// The two shapes a provider delivers arguments in, against a tool that actually wants some. The +// hand-off names the call and nothing else, so what the tool receives comes off the log, and both +// shapes have to leave the same thing there. Whole first: no input deltas at all, which is what the +// fragment buffer would otherwise record as an empty input. +const callsEchoWhole: LLMClientShape["stream"] = () => + Stream.fromIterable([ + LLMEvent.stepStart({ index: 0 }), + LLMEvent.toolCall({ id: "call_probe", name: "probe_echo", input: { text: "hello" } }), + LLMEvent.stepFinish({ index: 0, reason: "tool-calls" }), + ]) +// Streamed, in pieces, which is what a provider that emits partial JSON does. +const callsEchoStreamed: LLMClientShape["stream"] = () => + Stream.fromIterable([ + LLMEvent.stepStart({ index: 0 }), + LLMEvent.toolInputStart({ id: "call_probe", name: "probe_echo" }), + LLMEvent.toolInputDelta({ id: "call_probe", name: "probe_echo", text: '{"text":' }), + LLMEvent.toolInputDelta({ id: "call_probe", name: "probe_echo", text: '"hello"}' }), + LLMEvent.toolInputEnd({ id: "call_probe", name: "probe_echo" }), + LLMEvent.toolCall({ id: "call_probe", name: "probe_echo", input: { text: "hello" } }), + 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 @@ -221,9 +242,22 @@ const seedSession = Effect.gen(function* () { // 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 }) => +const registerProbes = (ran: { write: number; read: number; echoed?: string }) => Effect.gen(function* () { yield* (yield* ApplicationTools.Service).register({ + // The one probe that wants an argument. Every other schema here is an empty struct, which + // accepts anything, so none of them can tell whether a tool was handed what the model asked + // for. This one records it. + probe_echo: Tool.make({ + description: "echo probe", + input: Schema.Struct({ text: Schema.String }), + output: Schema.String, + execute: (args: { readonly text: string }) => + Effect.sync(() => { + ran.echoed = args.text + return args.text + }), + }), probe_write: Tool.make({ description: "write probe", input: Schema.Struct({}), @@ -282,7 +316,7 @@ const registerProbes = (ran: { write: number; read: number }) => }) }) -const counters = () => ({ write: 0, read: 0 }) +const counters = () => ({ write: 0, read: 0 }) as { write: number; read: number; echoed?: string } const toolPart = (messages: ReadonlyArray, callID: string) => { for (const message of messages) { @@ -389,6 +423,26 @@ describe("SessionRunner model-only attempt", () => { expect(message?.type === "assistant" ? Boolean(message.time.completed) : false).toBe(true) }), ) + + // The turn saying it is over, as opposed to a step saying it is. Everything watching a session + // from outside used to infer the difference from a finish reason and then a silence, because a + // steer or a queued prompt continues the same turn through another step. + harness(textOnly).effect("says the turn ended, once, when nothing follows it", () => + Effect.gen(function* () { + yield* seedSession + const events = yield* EventV2.Service + const ended = yield* events + .subscribe(SessionEvent.Turn.Ended) + .pipe(Stream.take(1), Stream.runCollect, Effect.forkScoped) + const runner = yield* SessionRunner.Service + + yield* runner.runStep({ sessionID, step: 2, promotion: undefined, first: false, force: false }) + + const seen = yield* Fiber.join(ended) + expect(seen.length).toBe(1) + expect(seen[0]?.data.sessionID).toBe(sessionID) + }), + ) }) // Dispatching one recorded call on its own. The policy under test is what happens when a dispatch @@ -427,6 +481,77 @@ describe("SessionRunner tool dispatch", () => { }), ) + // What the hand-off no longer carries. The arguments come off the recorded call, so a dispatch + // that reads them wrongly hands the tool something its schema refuses, and the model spends a + // turn being told its own input was not an object. Every other probe here takes an empty struct, + // which accepts that silently. + harness(callsEchoWhole).effect("hands the tool the arguments the model asked with", () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + const call = yield* deferOneCall + const runner = yield* SessionRunner.Service + + const result = yield* runner.runToolCall({ sessionID, call }) + + expect(result.outcome).toBe("settled") + expect(ran.echoed).toBe("hello") + }), + ) + + // The same call, streamed in pieces instead of delivered whole. Both shapes have to leave the + // arguments in the log, because the dispatcher cannot tell which one produced the call. + harness(callsEchoStreamed).effect("and the same when the provider streamed them", () => + Effect.gen(function* () { + yield* seedSession + const ran = counters() + yield* registerProbes(ran) + const call = yield* deferOneCall + const runner = yield* SessionRunner.Service + + const result = yield* runner.runToolCall({ sessionID, call }) + + expect(result.outcome).toBe("settled") + expect(ran.echoed).toBe("hello") + }), + ) + + harness(callsTool).effect("executes one non-idempotent call once under overlapping dispatches", () => + 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 read = store.message + const gate = yield* Deferred.make() + let readers = 0 + const spy = spyOn(store, "message").mockImplementation((id) => + read(id).pipe( + Effect.tap((value) => + Effect.gen(function* () { + if (readers >= 2) return + expect(toolPart(value ? [value.message] : [], call.id)?.state.status).toBe("pending") + readers++ + if (readers === 2) yield* Deferred.succeed(gate, undefined) + yield* Deferred.await(gate) + }), + ), + ), + ) + const outcomes = yield* Effect.all( + [runner.runToolCall({ sessionID, call }), runner.runToolCall({ sessionID, call })].map(Effect.exit), + { concurrency: "unbounded" }, + ).pipe(Effect.ensuring(Effect.sync(() => spy.mockRestore()))) + + expect(readers).toBe(2) + expect(ran.write).toBe(1) + expect(outcomes.filter(Exit.isSuccess)).toHaveLength(1) + }), + ) + harness(callsTool).effect("does nothing when the call already has a result", () => Effect.gen(function* () { yield* seedSession diff --git a/packages/core/test/snapshot-chain.test.ts b/packages/core/test/snapshot-chain.test.ts new file mode 100644 index 000000000000..37412fd0023c --- /dev/null +++ b/packages/core/test/snapshot-chain.test.ts @@ -0,0 +1,58 @@ +// The packs form a chain, and the chain is what orders them. `time_created` is whichever host +// wrote the row, and hosts do not agree on the time, so a worker whose clock is behind used to make +// its older tree the newest one that every other host then checked out. +import { describe, expect, test } from "bun:test" +import { chainHead, isBehind, orderChain } from "@opencode-ai/core/snapshot/chain" + +const row = (id: string, base: string | null, time: number, tree = `tree-${id}`) => ({ + id, + base, + time_created: time, + tree, +}) + +describe("snapshot chain", () => { + test("orders a chain by its links, not by the write clock", () => { + // Written by a host five minutes behind, so `b` claims an earlier time than its own parent. + const rows = [row("b", "a", 1_000), row("a", null, 300_000), row("c", "b", 2_000)] + expect(orderChain(rows).map((r) => r.id)).toEqual(["a", "b", "c"]) + }) + + test("the head is the deepest link, whatever the clock says", () => { + const rows = [row("a", null, 300_000), row("b", "a", 1_000), row("c", "b", 2_000)] + expect(chainHead(rows)?.id).toBe("c") + }) + + test("an empty store has no head", () => { + expect(chainHead([])).toBeUndefined() + }) + + test("a fork is decided by depth, and the clock only breaks a tie", () => { + // `x` and `y` both build on `a`. `y` is deeper, so it wins even though `x` was written later. + const rows = [row("a", null, 1), row("x", "a", 99_000), row("y", "a", 2), row("z", "y", 3)] + expect(chainHead(rows)?.id).toBe("z") + }) + + test("a row whose base is not in the store is treated as a root", () => { + const rows = [row("b", "missing", 5), row("c", "b", 6)] + expect(orderChain(rows).map((r) => r.id)).toEqual(["b", "c"]) + expect(chainHead(rows)?.id).toBe("c") + }) + + test("behind means earlier in the chain, not earlier on a clock", () => { + const rows = [row("a", null, 300_000), row("b", "a", 1_000)] + expect(isBehind(rows, "tree-a")).toBe(true) + expect(isBehind(rows, "tree-b")).toBe(false) + }) + + test("a tree the store has never seen is this host's own work, not a state behind", () => { + const rows = [row("a", null, 1), row("b", "a", 2)] + expect(isBehind(rows, "tree-never-shipped")).toBe(false) + }) + + test("a row that names itself as its base does not run the stack out", () => { + const rows = [row("a", "a", 1), row("b", "a", 2)] + expect(() => chainHead(rows)).not.toThrow() + expect(chainHead(rows)?.id).toBe("b") + }) +}) diff --git a/packages/core/test/worktree-materialize.test.ts b/packages/core/test/worktree-materialize.test.ts index beb0eba19186..5fca628e1438 100644 --- a/packages/core/test/worktree-materialize.test.ts +++ b/packages/core/test/worktree-materialize.test.ts @@ -4,20 +4,26 @@ // simulate a fresh host, "host B" materializes it back from the store alone. import { describe, expect } from "bun:test" import { $ } from "bun" +import { execFile, spawn } from "node:child_process" +import { randomBytes } from "node:crypto" +import { promisify } from "node:util" import { realpathSync } from "node:fs" -import { mkdir, readFile, rm, writeFile } from "node:fs/promises" +import { mkdir, readdir, readFile, rm, writeFile } from "node:fs/promises" import path from "path" import { asc } from "drizzle-orm" -import { Effect, Layer } from "effect" +import { Effect, Fiber, Layer } from "effect" import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { Database } from "@opencode-ai/core/database/database" +import { EventV2 } from "@opencode-ai/core/event" +import { EventSequenceTable } from "@opencode-ai/core/event/sql" import { Global } from "@opencode-ai/core/global" import { Location } from "@opencode-ai/core/location" import { AbsolutePath } from "@opencode-ai/core/schema" import { Snapshot } from "@opencode-ai/core/snapshot" import { SnapshotSync } from "@opencode-ai/core/snapshot-sync" import { SnapshotPackTable } from "@opencode-ai/core/snapshot/sql" +import { writeWorktreeTip } from "@opencode-ai/core/snapshot/tip" import { WorktreeMaterializer } from "@opencode-ai/core/session/execution/worktree" import { testEffect } from "./lib/effect" import { tmpdir } from "./fixture/tmpdir" @@ -101,15 +107,136 @@ describe("WorktreeMaterializer", () => { // A second ensure on an existing tree is a no-op, not a rebuild. yield* WorktreeMaterializer.Service.use((w) => w.ensure(worktree)).pipe(Effect.provide(B)) - expect(yield* Effect.promise(() => readFile(path.join(worktree, "tracked.txt"), "utf8"))).toBe( - "v3\n", + expect(yield* Effect.promise(() => readFile(path.join(worktree, "tracked.txt"), "utf8"))).toBe("v3\n") + + yield* Effect.promise(() => tmp[Symbol.asyncDispose]()) + }), + ) + + // A rebuild that fails removes what it created, and only that. The reading it asks is the one + // taken inside the lock: another drain can fill the directory while this one waits for it, and + // the reading from before the wait then names a directory that no longer exists. What that costs + // is not the rebuild, which retries, but the files git ignores in what it removed: an install, a + // build, a `.env`. `pauseBeforeLock` is the wait, and it is the only thing invented here. + it.live("keeps a directory another drain filled while a failed rebuild waited for the lock", () => + 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 data = path.join(root, "host-b-data") + 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(path.join(worktree, "tracked.txt"), "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 first = yield* Snapshot.Service.use((s) => s.capture()).pipe(Effect.provide(A)) + if (!first) throw new Error("expected a capture") + yield* SnapshotSync.Service.use((s) => s.push(first)).pipe(Effect.provide(A)) + const stored = yield* Database.Service.use(({ db }) => db.select().from(SnapshotPackTable).all()).pipe( + Effect.orDie, + Effect.provide(Database.layerFromPath(file)), + Effect.scoped, ) + // The newest state in the store, and a pack that is not a pack: indexing it is how a rebuild + // fails for reasons the store cannot rule out. + yield* Effect.sleep(10) + yield* Database.Service.use(({ db }) => + db + .insert(SnapshotPackTable) + .values([ + { + id: "f".repeat(40), + directory: worktree, + worktree, + tree: "e".repeat(40), + base: stored[0]!.id, + pack: Buffer.from([0x50, 0x41, 0x43, 0x4b]), + }, + ]) + .run(), + ).pipe(Effect.orDie, Effect.provide(Database.layerFromPath(file)), Effect.scoped) + + // This host is empty and behind, which is the state that decides to rebuild. + yield* Effect.promise(() => rm(worktree, { recursive: true, force: true })) + const B = yield* Layer.build(materializeStack(file, data)) + const rebuilding = yield* WorktreeMaterializer.Service.use((w) => + w.ensure(worktree, { pauseBeforeLock: 400 }), + ).pipe(Effect.provide(B), Effect.exit, Effect.forkChild) + + // What another drain leaves behind while this one waits: a checkout, the files git ignores, + // and the note saying this host agreed to that state. + yield* Effect.sleep(150) + yield* Effect.promise(async () => { + await mkdir(worktree, { recursive: true }) + await writeFile(path.join(worktree, "tracked.txt"), "v1\n") + await writeFile(path.join(worktree, ".env"), "SECRET=1\n") + }) + yield* writeWorktreeTip(data, worktree, stored[0]!.tree) + + const outcome = yield* Fiber.join(rebuilding) + // The rebuild really did fail, which is the premise: a check where it succeeded would say + // nothing about what a failure removes. + expect(outcome._tag).toBe("Failure") + + // The rebuild failed on the bad pack. The packs would restore `tracked.txt` on a retry; the + // ignored file is in no pack and nothing else has a copy. + const left = yield* Effect.promise(() => readdir(worktree).catch(() => [] as string[])) + // A `.git` the failed rebuild made on its way is fine; what must survive is the other drain's + // work, and above all the file no pack carries. + expect(left).toContain("tracked.txt") + expect(left).toContain(".env") + + yield* Effect.promise(() => tmp[Symbol.asyncDispose]()) + }), + ) + + it.live("rebuilds into a directory that exists but is empty", () => + 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") + 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(path.join(worktree, "note.txt"), "travelled\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 captured = yield* Snapshot.Service.use((s) => s.capture()).pipe(Effect.provide(A)) + if (!captured) throw new Error("expected a capture") + yield* SnapshotSync.Service.use((s) => s.push(captured)).pipe(Effect.provide(A)) + + // The shape a container gives a fresh host: the path is there because something mounted it, + // and there is nothing in it. Deleting the directory instead is the case already covered, + // and it is the easy one: an absent tree is obviously safe to build. + yield* Effect.promise(async () => { + await rm(worktree, { recursive: true, force: true }) + await mkdir(worktree, { recursive: true }) + }) + + 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)) + + expect(yield* Effect.promise(() => readFile(path.join(worktree, "note.txt"), "utf8"))).toBe("travelled\n") + yield* Effect.promise(() => tmp[Symbol.asyncDispose]()) }), ) - it.live("moves a rebuilt tree forward, and leaves a tree it did not build alone", () => + it.live("moves a tree that is behind forward, and leaves one already at the tip alone", () => Effect.gen(function* () { const tmp = yield* Effect.promise(() => tmpdir()) const root = realpathSync(tmp.path) @@ -146,9 +273,7 @@ describe("WorktreeMaterializer", () => { // 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), - ) + const ensureB = WorktreeMaterializer.Service.use((w) => w.ensure(worktree)).pipe(Effect.provide(B)) yield* ensureB expect(yield* content()).toBe("v1\n") @@ -176,6 +301,134 @@ describe("WorktreeMaterializer", () => { }), ) + // The write direction. A host the store has moved past used to pack its older files, become the + // newest by time, and every other host then checked that out over the work they were shipped to + // carry. This is the same rule the read direction already had, in the direction nothing checked. + it.live("refuses to ship from a host the store has moved past", () => + 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") + 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(path.join(worktree, "f.txt"), "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, "a-data"))) + const first = yield* Snapshot.Service.use((s) => s.capture()).pipe(Effect.provide(A)) + yield* SnapshotSync.Service.use((s) => s.push(first!)).pipe(Effect.provide(A)) + + // Another host ships while this one is not looking. Written straight into the store, because + // two capture stacks for one worktree resolve to the same host: the node builder keys them by + // location, so the second host has to be the row rather than a second stack. + const elsewhere = "e".repeat(40) + yield* Effect.sleep(10) + yield* Database.Service.use(({ db }) => + db + .insert(SnapshotPackTable) + .values([ + { + id: "d".repeat(40), + directory: worktree, + worktree, + tree: elsewhere, + pack: Buffer.from([0x50, 0x41, 0x43, 0x4b]), + }, + ]) + .run(), + ).pipe(Effect.orDie, Effect.provide(Database.layerFromPath(file)), Effect.scoped) + + // This host is still standing on `first`, so what it holds is not built on what the store now + // says the project is. Shipping it would revert the other host. + yield* Effect.promise(() => writeFile(path.join(worktree, "f.txt"), "stale\n")) + const stale = yield* Snapshot.Service.use((s) => s.capture()).pipe(Effect.provide(A)) + const exit = yield* SnapshotSync.Service.use((s) => s.push(stale!)).pipe(Effect.provide(A), Effect.exit) + expect(exit._tag).toBe("Failure") + + // Nothing was added, and the note was not moved either: a refused ship must leave this host + // saying what it actually holds. + const rows = yield* Database.Service.use(({ db }) => + db.select().from(SnapshotPackTable).orderBy(asc(SnapshotPackTable.time_created)).all(), + ).pipe(Effect.orDie, Effect.provide(Database.layerFromPath(file)), Effect.scoped) + expect(rows).toHaveLength(2) + expect(rows[1]?.tree).toBe(elsewhere) + + yield* Effect.promise(() => tmp[Symbol.asyncDispose]()) + }), + ) + + // A host that seeded the session from its own checkout has a note but no rebuild marker, because + // only a rebuild writes one. Gating the move on that marker meant every activity such a host drew + // died as soon as any other host shipped, and the comment said it would be sent elsewhere when the + // boundary marks it non-retryable. The note is the rule: a host that agreed to a state may be + // moved off it. + it.live("moves a tree the host captured rather than rebuilt", () => + 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 data = path.join(root, "seed-host-data") + 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(path.join(worktree, "f.txt"), "seeded\n") + await $`git -C ${worktree} add .`.quiet() + await $`git -C ${worktree} commit -qm seed`.quiet() + }) + + // This host captures from its own checkout, so it gets a note and no rebuild marker. + const A = yield* Layer.build(captureStack(file, worktree, data)) + const first = yield* Snapshot.Service.use((s) => s.capture()).pipe(Effect.provide(A)) + yield* SnapshotSync.Service.use((s) => s.push(first!)).pipe(Effect.provide(A)) + const packs = yield* Database.Service.use(({ db }) => db.select().from(SnapshotPackTable).all()).pipe( + Effect.orDie, + Effect.provide(Database.layerFromPath(file)), + Effect.scoped, + ) + + // Another host ships on top, so this one is behind. + yield* Effect.sleep(10) + yield* Database.Service.use(({ db }) => + db + .insert(SnapshotPackTable) + .values([ + { + id: "d".repeat(40), + directory: worktree, + worktree, + tree: "e".repeat(40), + base: packs[0]!.id, + pack: Buffer.from([0x50, 0x41, 0x43, 0x4b]), + }, + ]) + .run(), + ).pipe(Effect.orDie, Effect.provide(Database.layerFromPath(file)), Effect.scoped) + + const B = yield* Layer.build(materializeStack(file, data)) + const exit = yield* WorktreeMaterializer.Service.use((w) => w.ensure(worktree)).pipe( + Effect.provide(B), + Effect.exit, + ) + + // The pack above is not a real one, so the rebuild itself cannot succeed here. What this pins + // is which failure: a rebuild that was attempted and failed, not a refusal to try. + const why = String(exit) + expect(why).not.toContain("was not built") + expect(why).toContain("could not materialize") + + 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", () => @@ -190,11 +443,223 @@ describe("WorktreeMaterializer", () => { .values([{ id: "c".repeat(40), directory: "/w", worktree: "/w", tree: "t".repeat(40), pack: bytes }]) .run(), ).pipe(Effect.orDie, Effect.provide(layer), Effect.scoped) - const row = yield* Database.Service.use(({ db }) => - db.select().from(SnapshotPackTable).get(), - ).pipe(Effect.orDie, Effect.provide(layer), Effect.scoped) + const row = yield* Database.Service.use(({ db }) => db.select().from(SnapshotPackTable).get()).pipe( + Effect.orDie, + Effect.provide(layer), + Effect.scoped, + ) expect(Buffer.from(row!.pack).equals(bytes)).toBeTrue() yield* Effect.promise(() => tmp[Symbol.asyncDispose]()) }), ) }) + +// A process in a group of its own, which is what a worker somebody's supervisor started has, and +// what a tool that daemonizes gives itself. It carries a worker name of this test's choosing, so +// what each case turns on is the one thing that case is about. Without one it gets this process's +// environment, which is the case that says the name is exported rather than only written down. +const run = promisify(execFile) + +// Per run, because a name is what the host looks for and an assertion that fails before its child +// is killed leaves that child running. A fixed name would then answer for every later run. +const runToken = randomBytes(4).toString("hex") +const named = (worker: string) => `${worker}-${runToken}` + +const spawned = (worker?: string) => { + const child = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { + detached: true, + stdio: "ignore", + env: + worker === undefined ? process.env : { ...process.env, OPENCODE_WORKTREE_WRITER: named(worker) }, + }) + return { child, pid: child.pid! } +} + +const ended = async (started: ReturnType) => { + const exited = new Promise((resolve) => started.child.once("exit", resolve)) + started.child.kill("SIGKILL") + await exited + return { pid: started.pid, pgid: started.pid } +} + +/** Say the one marker on this host was written by another process, or before a restart. */ +const editMarker = async (data: string, worktree: string, patch: Record) => { + const root = path.join(data, "worktree-writers") + const [dir] = await readdir(root) + const [name] = await readdir(path.join(root, dir)) + const file = path.join(root, dir, name) + const note = JSON.parse(await readFile(file, "utf8")) + await writeFile(file, JSON.stringify({ ...note, ...patch })) +} + +describe("WorktreeMaterializer quarantine", () => { + // Refusing to move a step off a host protects that step and nothing after it: the turn ends, the + // next prompt lands wherever there is room, and the tool from before can still be writing. A + // marker says which calls are inside their own execution, and the directory belongs to that + // call's step until it returns. + it.live("refuses a directory to another step while an earlier call has not returned", () => + 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 data = path.join(root, "host-data") + 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(path.join(worktree, "tracked.txt"), "v1\n") + await $`git -C ${worktree} add .`.quiet() + await $`git -C ${worktree} commit -qm seed`.quiet() + }) + + const A = yield* Layer.build(captureStack(file, worktree, data)) + const captured = yield* Snapshot.Service.use((s) => s.capture()).pipe(Effect.provide(A)) + if (!captured) throw new Error("expected a capture") + yield* SnapshotSync.Service.use((s) => s.push(captured)).pipe(Effect.provide(A)) + + const B = yield* Layer.build(materializeStack(file, data)) + const worktrees = yield* WorktreeMaterializer.Service.pipe(Effect.provide(B)) + + // A call of an earlier step that never came back. + const stranded = { sessionID: "ses_one", step: 1, callID: "call_stranded" } + yield* worktrees.beginWrite(worktree, stranded) + + // Another step wants the directory. It is not this call's step, so it is refused, and the + // refusal is a defect the activity boundary turns into a failure Temporal schedules again. + const later = { sessionID: "ses_one", step: 2, callID: "call_later" } + const refused = yield* Effect.exit(worktrees.ensure(worktree, { current: later })) + expect(refused._tag).toBe("Failure") + + // A sibling of the same step is not stranded: two tools of one step share this directory by + // design, and refusing them would be refusing the feature. + const sibling = { sessionID: "ses_one", step: 1, callID: "call_sibling" } + const allowed = yield* Effect.exit(worktrees.ensure(worktree, { current: sibling })) + expect(allowed._tag).toBe("Success") + + // When the call comes back, whatever it did to the directory, it is not still doing it. + yield* worktrees.endWrite(worktree, stranded.callID) + const afterReturn = yield* Effect.exit(worktrees.ensure(worktree, { current: later })) + expect(afterReturn._tag).toBe("Success") + + // A worker that died mid-tool leaves its marker behind, and that used to need a person. Most + // of it is answerable without one: the writer's process is gone, nothing it started is left + // in its group, and the machine has not restarted underneath the pids that say so. + const usable = () => + Effect.exit(worktrees.ensure(worktree, { current: later })).pipe(Effect.map((exit) => exit._tag === "Success")) + + // Written by this process, which is running. Nothing to conclude, so the refusal stands. + yield* worktrees.beginWrite(worktree, { sessionID: "ses_one", step: 3, callID: "call_dead" }) + expect(yield* usable()).toBe(false) + + // The worker died and left nothing behind: no process of its own, nothing carrying its name, + // and an empty group. That is the whole of the proof that its tools are over. + const gone = yield* Effect.promise(() => ended(spawned("gone"))) + const stale = { pid: gone.pid, pgid: gone.pgid, worker: named("gone") } + yield* Effect.promise(() => editMarker(data, worktree, stale)) + expect(yield* usable()).toBe(true) + + // The worker died and something it started did not. That is what the refusal is for. + const orphan = spawned("orphaned") + yield* worktrees.beginWrite(worktree, { sessionID: "ses_one", step: 3, callID: "call_dead" }) + yield* Effect.promise(() => + editMarker(data, worktree, { ...stale, pgid: orphan.pid, worker: named("orphaned") }), + ) + expect(yield* usable()).toBe(false) + yield* Effect.promise(() => ended(orphan)) + expect(yield* usable()).toBe(true) + + // A tool that asks for a group of its own is out of the group check's reach. What it cannot + // put down is the name its worker left in the environment it inherited. + const escaped = spawned("escaped") + yield* worktrees.beginWrite(worktree, { sessionID: "ses_one", step: 3, callID: "call_dead" }) + yield* Effect.promise(() => editMarker(data, worktree, { ...stale, worker: named("escaped") })) + expect(yield* usable()).toBe(false) + yield* Effect.promise(() => ended(escaped)) + expect(yield* usable()).toBe(true) + + // And the name has to reach the tool, not only the marker. This child is given no environment + // of its own, so the only way it carries the name is that the worker exported it. + const inheriting = spawned() + yield* worktrees.beginWrite(worktree, { sessionID: "ses_one", step: 3, callID: "call_dead" }) + yield* Effect.promise(() => + editMarker(data, worktree, { ...stale, worker: process.env.OPENCODE_WORKTREE_WRITER }), + ) + expect(yield* usable()).toBe(false) + yield* Effect.promise(() => ended(inheriting)) + expect(yield* usable()).toBe(true) + + // A worker restarted from the same shell is in the group its predecessor was in, so the group + // answers for this process rather than for the marker. What the dead worker started is what + // decides, and it started nothing. + const ourGroup = yield* Effect.promise(async () => { + const { stdout } = await run("ps", ["-o", "pgid=", "-p", String(process.pid)]) + return Number.parseInt(stdout.trim(), 10) + }) + yield* worktrees.beginWrite(worktree, { sessionID: "ses_one", step: 3, callID: "call_dead" }) + yield* Effect.promise(() => editMarker(data, worktree, { ...stale, pgid: ourGroup })) + expect(yield* usable()).toBe(true) + + // A pid means nothing across a restart, so a marker from before one is not read as live. + yield* worktrees.beginWrite(worktree, { sessionID: "ses_one", step: 3, callID: "call_dead" }) + yield* Effect.promise(() => editMarker(data, worktree, { bootAt: 0 })) + expect(yield* usable()).toBe(true) + }), + ) +}) + +// A dispatch the session stopped waiting for keeps running, and the files it ships afterwards would +// be the newest state the store holds: the tip check cannot refuse them, because the host is still +// standing exactly where it was told to stand. The event log already fences a superseded attempt +// out of the transcript, and the packs travel under the same token. +describe("SnapshotSync owner fence", () => { + it.live("refuses a pack from an attempt the session has moved past", () => + 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") + 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(path.join(worktree, "tracked.txt"), "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 onDatabase = (use: (db: Database.Interface["db"]) => Effect.Effect) => + Database.Service.use(({ db }) => use(db)).pipe( + Effect.orDie, + Effect.provide(Database.layerFromPath(file)), + Effect.scoped, + ) + // The session is on a later attempt than the one that is about to publish. + yield* onDatabase((db) => + db.insert(EventSequenceTable).values({ aggregate_id: "ses_fenced", seq: 1, owner_id: "run:1:2" }).run(), + ) + + const captured = yield* Snapshot.Service.use((s) => s.capture()).pipe(Effect.provide(A)) + if (!captured) throw new Error("expected a capture") + const shipped = (owner: string) => + Effect.exit( + SnapshotSync.Service.use((s) => s.push(captured, "ses_fenced")).pipe( + Effect.provideService(EventV2.EventOwner, owner), + Effect.provide(A), + ), + ) + + const stale = yield* shipped("run:1:1") + expect(stale._tag).toBe("Failure") + expect(yield* onDatabase((db) => db.select().from(SnapshotPackTable).all())).toHaveLength(0) + + // The attempt the session is actually on ships as usual. + const current = yield* shipped("run:1:2") + expect(current._tag).toBe("Success") + expect(yield* onDatabase((db) => db.select().from(SnapshotPackTable).all())).toHaveLength(1) + }), + ) +}) diff --git a/packages/opencode/src/cli/cmd/detached.ts b/packages/opencode/src/cli/cmd/detached.ts new file mode 100644 index 000000000000..b6888e4a9075 --- /dev/null +++ b/packages/opencode/src/cli/cmd/detached.ts @@ -0,0 +1,397 @@ +// Commands for a session nobody is sitting in front of: start one and walk away, ask the +// deployment what it is still running, and follow one from a machine that never had it. +// +// These are thin HTTP clients on purpose. In a durable deployment the serve processes are +// interchangeable (any of them reads the shared store and signals the same workflows), so a client +// needs an endpoint and a session id, never a particular host. That is the whole reason a session +// can outlive the process that started it, and it is why nothing here talks to Temporal. The one +// exception is `doctor`, which reads the driver's own configuration module: it answers what this +// deployment resolved, and a second copy of those rules living here is how the two would disagree. + +import type { Argv } from "yargs" +import { cmd } from "./cmd" +import { UI } from "../ui" +import { ServerAuth } from "@/server/auth" +import { TemporalConfig } from "@opencode-ai/temporal/config" +// Type-only: the SDK types a duration as a template literal, and this takes one from a person. +import type { Duration } from "@temporalio/common" + +const DEFAULT_URL = "http://127.0.0.1:4096" + +type Remote = { readonly url: string; readonly headers: Record } + +function remote(args: { attach?: string; password?: string; username?: string }): Remote { + const url = (args.attach ?? process.env["OPENCODE_SERVER"] ?? DEFAULT_URL).replace(/\/+$/, "") + // No password configured is a valid deployment, so absent auth is absent headers, not an error. + return { url, headers: ServerAuth.headers({ password: args.password, username: args.username }) ?? {} } +} + +async function call(r: Remote, path: string, init?: RequestInit): Promise { + const response = await fetch(`${r.url}/api${path}`, { + ...init, + headers: { ...r.headers, ...(init?.body ? { "content-type": "application/json" } : {}), ...init?.headers }, + }) + if (!response.ok) { + const detail = await response.text().catch(() => "") + throw new Error(`${init?.method ?? "GET"} /api${path} failed: ${response.status} ${detail.slice(0, 200)}`) + } + if (response.status === 204) return undefined as T + const body = (await response.json()) as { data: T } + return body.data +} + +// The remote-facing options every command here shares. Kept in one builder so a second endpoint +// flag can never drift between them. +function remoteOptions(yargs: Argv) { + return yargs + .option("attach", { + type: "string", + describe: `server to talk to (default ${DEFAULT_URL}, or $OPENCODE_SERVER)`, + }) + .option("password", { alias: "p", type: "string", describe: "basic auth password" }) + .option("username", { alias: "u", type: "string", describe: "basic auth username" }) + .option("json", { type: "boolean", describe: "print machine-readable output", default: false }) +} + +interface SessionInfo { + id: string + title?: string + time?: { created?: number; updated?: number } + location?: { directory?: string } +} + +const stamp = (ms?: number) => (ms ? new Date(ms).toISOString().replace("T", " ").slice(0, 19) : "") + +// UI.println writes to stderr, which is right for a person and wrong for a pipe. Anything a script +// is meant to read goes to stdout instead. +const emit = (line: string) => process.stdout.write(line + "\n") + +export const SessionStartCommand = cmd({ + command: "start ", + describe: "start a session, hand it a prompt, and return without waiting for it", + builder: (yargs: Argv) => + remoteOptions(yargs) + .positional("prompt", { type: "string", describe: "what the agent should do", demandOption: true }) + .option("dir", { type: "string", describe: "session working directory (default: this one)" }) + .option("model", { type: "string", describe: "provider/model, e.g. openai/gpt-5-mini" }), + handler: async (args) => { + const r = remote(args) + try { + const directory = args.dir ?? process.cwd() + const session = await call(r, "/session", { + method: "POST", + body: JSON.stringify({ directory }), + }) + if (args.model) { + const slash = args.model.indexOf("/") + if (slash < 1) throw new Error(`--model wants provider/model, got ${args.model}`) + const model = { providerID: args.model.slice(0, slash), id: args.model.slice(slash + 1) } + await call(r, `/session/${session.id}/model`, { method: "POST", body: JSON.stringify({ model }) }) + } + // The prompt is admitted, not awaited. Whoever is polling the task queue runs the turn, and + // this process has nothing left to do with it. + await call(r, `/session/${session.id}/prompt`, { + method: "POST", + body: JSON.stringify({ prompt: { text: args.prompt } }), + }) + if (args.json) { + emit(JSON.stringify({ id: session.id, directory, url: r.url })) + return + } + emit(session.id) + UI.println(` follow it with: opencode session watch ${session.id}`) + } catch (error) { + UI.error(error instanceof Error ? error.message : String(error)) + process.exitCode = 1 + } + }, +}) + +// What this process resolved, and what is wrong with it. Deploying was a handful of variables that +// have to agree, with no way to ask whether they did: every mistake in them fails as something +// else, hours later, on whoever prompted the session rather than on whoever deployed it. +export const SessionDoctorCommand = cmd({ + command: "doctor", + describe: "what this deployment resolved, and what is wrong with it", + builder: (yargs: Argv) => yargs, + handler: async () => { + const config = TemporalConfig.fromEnv() + UI.println("opencode, temporal execution") + for (const [name, value] of Object.entries(TemporalConfig.describe(config))) { + UI.println(` ${name}: ${value}`) + } + for (const note of TemporalConfig.notes(config)) UI.println(`note: ${note}`) + const problems = TemporalConfig.preflight(config) + for (const problem of problems) UI.println(`problem: ${problem}`) + if (problems.length > 0) { + process.exitCode = 1 + return + } + UI.println("this deployment looks consistent") + }, +}) + +// A turn nobody starts. `start` still needs something running to hand the prompt to; a schedule +// does not, which is the difference between a session you can walk away from and one that runs +// without you. The session is created once, here, over HTTP like everything else in this file; the +// firing itself reaches only Temporal, and a deployment with no serve process at all still runs it. +export const SessionScheduleCommand = cmd({ + command: "schedule ", + describe: "run a prompt on a schedule, with no client at firing time", + builder: (yargs: Argv) => + remoteOptions(yargs) + .positional("prompt", { type: "string", describe: "what the agent should do", demandOption: true }) + .option("every", { type: "string", describe: "interval, e.g. 1h" }) + .option("cron", { type: "string", describe: "cron expression, e.g. '0 9 * * *'" }) + .option("id", { type: "string", describe: "schedule id (default: generated)" }) + .option("session", { type: "string", describe: "an existing session to prompt (default: a new one)" }) + .option("dir", { type: "string", describe: "session working directory (default: this one)" }), + handler: async (args) => { + const r = remote(args) + try { + if (!args.every && !args.cron) throw new Error("schedule wants --every= or --cron=") + const sessionID = + args.session ?? + ( + await call(r, "/session", { + method: "POST", + body: JSON.stringify({ directory: args.dir ?? process.cwd() }), + }) + ).id + const config = TemporalConfig.fromEnv() + const { Client, Connection, ScheduleOverlapPolicy } = await import("@temporalio/client") + const connection = await Connection.connect(TemporalConfig.connectionOptions(config)) + try { + const client = new Client({ connection, namespace: config.namespace }) + const scheduleId = args.id ?? `opencode-${sessionID}` + await client.schedule.create({ + scheduleId, + spec: { + ...(args.cron ? { cronExpressions: [args.cron] } : {}), + ...(args.every ? { intervals: [{ every: args.every as Duration }] } : {}), + }, + // A firing that lands while the last one is still working is skipped rather than queued. + // An agent task is not a metrics scrape: two of them on one project is a bad day. + policies: { overlap: ScheduleOverlapPolicy.SKIP }, + action: { + type: "startWorkflow", + workflowType: "scheduledPrompt", + taskQueue: config.taskQueue, + args: [ + { + sessionID, + text: args.prompt, + session: { idleTimeout: config.idleTimeout, stepped: config.stepped === true }, + }, + ], + }, + }) + if (args.json) { + emit(JSON.stringify({ schedule: scheduleId, session: sessionID })) + return + } + emit(scheduleId) + UI.println(` every firing prompts ${sessionID}; follow it with: opencode session watch ${sessionID}`) + } finally { + await connection.close() + } + } catch (error) { + UI.error(error instanceof Error ? error.message : String(error)) + process.exitCode = 1 + } + }, +}) + +export const SessionRunningCommand = cmd({ + command: "running", + describe: "list the sessions this deployment is executing right now", + builder: (yargs: Argv) => remoteOptions(yargs), + handler: async (args) => { + const r = remote(args) + try { + // Which sessions are running is the executor's answer, not a guess from the transcript: a + // durable deployment reads it from the running workflows, so it survives a restart of + // whichever process happens to be answering this call. + const active = await call>(r, "/session/active") + const ids = Object.keys(active) + if (args.json) { + emit(JSON.stringify(ids.map((id) => ({ id, status: active[id]?.type })))) + return + } + if (ids.length === 0) { + UI.println("nothing running") + return + } + const all = await call(r, "/session").catch(() => [] as SessionInfo[]) + const byId = new Map(all.map((s) => [s.id, s])) + for (const id of ids) { + const session = byId.get(id) + const cells = [id, active[id]?.type ?? "?", stamp(session?.time?.updated), session?.title ?? ""] + emit(cells.join(" ")) + } + } catch (error) { + UI.error(error instanceof Error ? error.message : String(error)) + process.exitCode = 1 + } + }, +}) + +// What a follower prints. The stream carries far more than a person watching wants to read, so this +// keeps the events that say the work moved and drops the token-level ones. +const INTERESTING: Record string | undefined> = { + "session.next.prompted": () => "prompted", + "session.next.step.started": () => "step", + "session.next.tool.called": (d) => `tool ${d.tool}: ${JSON.stringify(d.input ?? {}).slice(0, 120)}`, + "session.next.tool.success": (d) => `tool ok ${firstText(d.content).slice(0, 200)}`, + "session.next.tool.failed": (d) => `tool failed ${firstText(d.content).slice(0, 200)}`, + "session.next.text.ended": (d) => (d.text ? `said: ${String(d.text).slice(0, 400)}` : undefined), + "session.next.step.failed": (d) => `step failed: ${d.error?.message ?? ""}`, +} + +function firstText(content: unknown): string { + if (!Array.isArray(content)) return "" + const part = content.find((c) => c && typeof c === "object" && (c as any).type === "text") as any + return part?.text ? String(part.text).trim() : "" +} + +export const SessionWatchCommand = cmd({ + command: "watch ", + describe: "follow a running session from anywhere, and exit when it goes idle", + builder: (yargs: Argv) => + remoteOptions(yargs) + .positional("sessionID", { type: "string", describe: "session to follow", demandOption: true }) + .option("wait", { type: "boolean", default: true, describe: "keep following until the session is idle" }), + handler: async (args) => { + const r = remote(args) + const sessionID = args.sessionID + + // The turn saying so itself, which is the only thing that knows: a step ending is not a turn + // ending, because a steer or a queued prompt continues the same turn. It covers the ordinary + // ending only, so the reasons below are still what a stopped or failed turn ends this on. + const turnEnded = (event: { type?: string }) => event.type === "session.next.turn.ended" + + // A step ending is where a turn usually ends, from the model's own finish reason: `tool-calls` + // is the one that means another step follows. It is not on its own proof the turn is over, + // because a steer or a queued prompt continues it, so the executor's own answer decides. + const looksDone = (event: { type?: string; data?: any }) => + event.type === "session.next.step.failed" || + (event.type === "session.next.step.ended" && event.data?.finish !== "tool-calls") + + // What says the turn did NOT end after all: a steer or a queued prompt continues the same turn + // through `stepContinuation`, and the next thing on the wire is another step starting. + const carriesOn = (event: { type?: string }) => + event.type === "session.next.step.started" || event.type === "session.next.prompted" + + // The running set cannot end a `watch` on its own. It holds a session for the whole idle + // timeout after the work is done, so gating the exit on it means never exiting. Nothing + // publishes a turn-level ending either, so what usually ends this is the wire going quiet: a + // terminal step, then no continuation within a grace window. A steer arrives in milliseconds, + // so the window only has to outlast the hop between two activities. + const GRACE_MS = Number(process.env.OPENCODE_WATCH_GRACE_MS ?? 5_000) + // The wire cannot answer the other case. A turn that ends while this is reconnecting publishes + // its last step into a gap, and the stream has no replay, so nothing arrives afterwards and the + // quiet means nothing. Absence from the running set is slow but certain, and it is the one + // reading that only ever says "finished", so it can end a watch without being able to hang one. + const POLL_MS = Number(process.env.OPENCODE_WATCH_POLL_MS ?? 30_000) + const inactive = async () => { + const active = await call>(r, "/session/active").catch(() => undefined) + // Unreachable is not finished, which is the whole point of this command. + return active !== undefined && !(sessionID in active) + } + + // Kept across reconnects. The terminal step lands on one connection and the quiet that follows + // it on the next, and starting this again per connection is what followed a finished turn for + // as long as the terminal stayed open. + let settleAt: number | undefined + + // The stream ends when the serve this is attached to restarts, which is the event this command + // exists for. Exiting 0 there reports a turn that is still running as done. + const attempts = 30 + try { + for (let attempt = 0; ; attempt++) { + const response = await fetch(`${r.url}/api/session/${sessionID}/event`, { + headers: r.headers, + }).catch(() => undefined) + if (!response?.ok || !response.body) { + if (attempt >= attempts) + throw new Error(`cannot follow ${sessionID}: ${response?.status ?? "unreachable"}`) + await new Promise((resolve) => setTimeout(resolve, 1000)) + continue + } + + const reader = response.body.getReader() + const decoder = new TextDecoder() + let buffer = "" + let ended = false + // Held across iterations rather than started fresh each time: a read that loses the race is + // still queued on the stream, and dropping it drops whatever it goes on to deliver. + let pending: ReturnType | undefined + for (;;) { + const next = (pending ??= reader.read()) + let timer: ReturnType | undefined + const chunk = args.wait + ? await Promise.race([ + next, + new Promise<"quiet">((resolve) => { + const wait = settleAt ? Math.max(0, settleAt - Date.now()) : POLL_MS + timer = setTimeout(() => resolve("quiet"), wait) + }), + ]) + : await next + if (timer) clearTimeout(timer) + if (chunk === "quiet") { + // A terminal step and then nothing: the turn is over. Otherwise this is the periodic + // ask, and only the running set can end the wait. + if ((settleAt && Date.now() >= settleAt) || (await inactive())) { + ended = true + break + } + continue + } + pending = undefined + const { done, value } = chunk + if (done) break + buffer += decoder.decode(value, { stream: true }) + const lines = buffer.split("\n") + buffer = lines.pop() ?? "" + for (const raw of lines) { + const line = raw.startsWith("data:") ? raw.slice(5).trim() : raw.trim() + if (!line.startsWith("{")) continue + let event: { type?: string; data?: any } + try { + event = JSON.parse(line) + } catch { + continue + } + if (args.json) { + emit(line) + } else { + const render = event.type ? INTERESTING[event.type] : undefined + const text = render?.(event.data ?? {}) + if (text) UI.println(`${stamp(event.data?.timestamp)} ${text}`) + } + if (args.wait) { + // The turn saying it is over ends this now: there is nothing to wait out, and the + // grace window exists only because nothing used to say it. + if (turnEnded(event)) { + ended = true + break + } + if (carriesOn(event)) settleAt = undefined + else if (looksDone(event)) settleAt = Date.now() + GRACE_MS + } + } + } + await reader.cancel().catch(() => {}) + // Without `--wait` the stream itself is the whole command, so its end is this one's too. + if (ended || !args.wait) return + // The stream dropped with the turn still going. Reconnect and keep following. + if (attempt >= attempts) throw new Error(`lost the stream for ${sessionID}`) + await new Promise((resolve) => setTimeout(resolve, 1000)) + } + } catch (error) { + UI.error(error instanceof Error ? error.message : String(error)) + process.exitCode = 1 + } + }, +}) diff --git a/packages/opencode/src/cli/cmd/session.ts b/packages/opencode/src/cli/cmd/session.ts index 9e6ddda9d2d8..94b746b63b72 100644 --- a/packages/opencode/src/cli/cmd/session.ts +++ b/packages/opencode/src/cli/cmd/session.ts @@ -1,6 +1,13 @@ import type { Argv } from "yargs" import { Effect } from "effect" import { cmd } from "./cmd" +import { + SessionDoctorCommand, + SessionRunningCommand, + SessionScheduleCommand, + SessionStartCommand, + SessionWatchCommand, +} from "./detached" import { effectCmd, fail } from "../effect-cmd" import { Session } from "@/session/session" import { SessionID } from "../../session/schema" @@ -44,7 +51,16 @@ function pagerCmd(): string[] { export const SessionCommand = cmd({ command: "session", describe: "manage sessions", - builder: (yargs: Argv) => yargs.command(SessionListCommand).command(SessionDeleteCommand).demandCommand(), + builder: (yargs: Argv) => + yargs + .command(SessionListCommand) + .command(SessionDeleteCommand) + .command(SessionStartCommand) + .command(SessionRunningCommand) + .command(SessionWatchCommand) + .command(SessionScheduleCommand) + .command(SessionDoctorCommand) + .demandCommand(), async handler() {}, }) diff --git a/packages/schema/src/session-event.ts b/packages/schema/src/session-event.ts index 3a559c3e38a4..4b72989e5608 100644 --- a/packages/schema/src/session-event.ts +++ b/packages/schema/src/session-event.ts @@ -145,6 +145,24 @@ export namespace Shell { export type Ended = typeof Ended.Type } +// The turn, as opposed to the steps it was made of. A step ending is not a turn ending: a steer or +// a queued prompt continues the same turn through another step, and everything watching a session +// from outside had to guess at the difference from a finish reason plus a silence. Live-only, and +// deliberately: it says nothing the durable events do not already say, and the record of what a +// turn did is those events. What it adds is a boundary, published by the one thing that knows it. +export namespace Turn { + export const Ended = Event.define({ + type: "session.next.turn.ended", + schema: { + ...Base, + // What the last step of the turn came to, so a follower can say why it stopped rather than + // only that it did. + finish: Schema.String, + }, + }) + export type Ended = typeof Ended.Type +} + export namespace Step { export const Started = Event.define({ type: "session.next.step.started", @@ -458,6 +476,7 @@ export const DurableDefinitions = Event.inventory( Step.Started, Step.Ended, Step.Failed, + Turn.Ended, Text.Started, Text.Ended, Tool.Input.Started, diff --git a/packages/temporal/README.md b/packages/temporal/README.md index d2d8ac5a6f95..cc4da9086ccd 100644 --- a/packages/temporal/README.md +++ b/packages/temporal/README.md @@ -1,16 +1,22 @@ # @opencode-ai/temporal -The Temporal executor for opencode's `SessionExecution` seam, packaged as a plugin: core carries -the seam, the built-in local executor, and the executor-agnostic toolkit; this package is one -dependency that makes an opencode **session** a durable Temporal workflow. A coding session -survives worker loss, can run detached or in the background, and can be driven from anywhere by -signal. opencode's loop, tools, model, storage, and HTTP API are untouched, and nothing Temporal -exists in core. +This package implements `SessionExecution` with Temporal workflows and activities. The default +executor remains local. Both drive the fork's `SessionRunner` and read its application event log. + +The integration changes runner APIs, tool admission, event fencing, approvals, questions, and +storage. The TUI bridge and deployment wiring are additional changes. There are no Temporal SDK +imports in core, but this is not an unchanged harness with a package added. Recovery depends on the +failure point and mode; the contract below names the cases that fail rather than migrate. A lighter increment exists as its own change: the `2026/08/opencode-temporal-http` branch wraps the **shipping** `opencode serve` over its HTTP API, for the agent-as-black-box case. It makes the -orchestration durable but cannot recover a partial turn. This change is the deeper one: durability -inside the engine, so a crashed turn resumes mid-step instead of being re-attached to. +orchestration durable. That prototype did not add host-side partial-turn recovery. A coarse +activity can recover partial progress when the host exposes a recorded continuation, as this +fork does. + +Reported live measurements and historical verification below are the author's supplied evidence. +They were not rerun during this review. Current offline and Temporal test results are recorded in +`work/opencode-review.md` in the review package. ## How it fits together @@ -25,7 +31,7 @@ in-process on the proven `SessionRunCoordinator` (core's `execution/local.ts`), 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 +(`packages/core/test/lib/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. That forces six things: @@ -38,8 +44,8 @@ That forces six things: (`loop-guard.ts`: a step ceiling plus a repeated-identical-call detector), because a runaway turn would otherwise be a durable runaway turn ([Two modes, one runner](#two-modes-one-runner)). -3. **Two writers must be fenced.** A superseded attempt cannot keep appending to the log; each - drain claims the log with an attempt token ([Notes](#notes)). +3. **Event appends check the owner.** Each drain claims an attempt token. Same-run ordering + rejects older claims; cross-run age is not encoded ([Notes](#notes)). 4. **The worktree must travel.** Snapshot trees ship as incremental git packs, and a worker without the project tree rebuilds it before the run ([What resumes cross-host](#what-resumes-cross-host-and-what-does-not)). @@ -58,7 +64,7 @@ visibility, so it survives restarts. opencode's v2 engine (`packages/core` + `packages/server`) is already event-sourced per session and exposes a substitutable `SessionExecution` service (`active` / `resume` / `wake` / `interrupt`) whose local impl comments "Future remote placement belongs here." This change provides a -Temporal-backed `SessionExecution` in `packages/core/src/session/execution/`: +Temporal-backed `SessionExecution` in `packages/temporal/src/`: - `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). @@ -66,8 +72,8 @@ Temporal-backed `SessionExecution` in `packages/core/src/session/execution/`: cancellation; injects the attempt's event-log owner token). - `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 + `signalWithStart`, `resume` → `executeUpdateWithStart`, `interrupt` → a signal cancelling + the current drain scope; 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). @@ -99,28 +105,27 @@ session runs as a Temporal workflow `session-exec-`. ### 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 +the smallest activity boundary in whole-step mode. Workflow code has no boundary between the +model and tools there; application callbacks can still gate dispatch. `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. +`SessionRunner.runModelCall` records pending tool inputs and returns call identities. At dispatch, +`runToolCall` commits `Tool.Called` before execution, then publishes the result. `sealStep` captures +the final tree and publishes `Step.Ended`. Workflow code connects those activities. + +Each activity has its own bounds. The seal's `startToCloseTimeout` is `10 minutes`, compared with +`12 hours` for the model and tool activities. Pinned activities have one attempt; shared activities +permit up to `100`. These proxies do not set a total `scheduleToCloseTimeout`. 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: +Two conditions govern dispatch: - **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 @@ -138,12 +143,11 @@ Two things are load-bearing and easy to get wrong: 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 non-idempotent dispatch uses an event ID derived from the session, assistant message, and + call ID. The unique event insert commits before execution. Concurrent attempts that both read + `pending` therefore compete for one admission. This prevents a second dispatch from starting the + same call. It does not stop an admitted process after a timeout, undo an external effect, or stop + the model requesting a new call with a new ID. - **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 @@ -164,7 +168,7 @@ 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 +receives the completed model activity result, not streamed tool-call events. `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)` | @@ -191,15 +195,13 @@ around the call: | `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. +In these probes, the remaining stream carried later tool calls. None contained text after the +first call. This sample does not establish the tail for other providers, prompts, or model versions. -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 reported runs added roughly 40-110 ms per step against model calls of two to eight seconds. +That result describes those runs, not a deployment-independent cost. -**The extra round trips.** Three activities per step instead of one means two more hand-offs. +**The extra round trips.** A one-tool step has three activities instead of one. Measured from workflow history on a loopback dev server (mean of four, one turn): ``` @@ -212,7 +214,12 @@ 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. +namespace to reduce this term. + +With `N` tools, the split schedules `N+2` activities. That is an activity count, not the latency +of a parallel batch. With available slots, the critical path includes the model, slowest tool, and +seal. Serial fallback instead adds each tool's dispatch and execution time. Activity cost, history +bytes, queue delay, transcript reads, and tree transfers need separate measurements. 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. @@ -329,10 +336,8 @@ OPENCODE_TEMPORAL_WORKTREE_AFFINITY=1 OPENCODE_TEMPORAL_WORKTREE=/srv/trees/acme 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. +directories. Queue derivation normalizes paths without depending on whether the directory exists +locally. Every participant must still use the same logical directory. **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 @@ -340,10 +345,15 @@ what makes any worker able to serve any session, and turning affinity on is choo Two consequences to plan for, both silent: +- **The key is the directory, not the host.** In a container fleet where every worker's project is + the same path, this affinity is a no-op: they all poll the same queue and a step's tools still + land wherever. It is a real routing decision only where hosts serve different paths. + What keeps a step's writes together in a container fleet is step affinity below, which is keyed by + host as well as by path. + - **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. + `OPENCODE_TEMPORAL_WORKTREE` at the session directory used to derive that queue. - **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` @@ -418,9 +428,8 @@ the crash is handled by declared idempotency: a side-effect-free tool (`read`/`g `idempotent: true`) is re-run for a real result, while a side-effecting tool is marked interrupted and left for the model to redo. The harness cannot know whether the side-effecting one already ran and must not re-run `git push`, so the default is non-idempotent; the blanket case (idempotency keys -against an external system) is per-integration and out of scope. Finer granularity (the model call -and each tool as separate Temporal activities) would un-fuse the eager tool dispatch and is left for -later. Verified by `packages/core/test/session-runner-resume.test.ts`. +against an external system) is per-integration and out of scope. The optional split mode also reuses recorded calls, with the model, tools, and seal in separate +activities. Verified by `packages/core/test/session-runner-resume.test.ts`. ### Notes @@ -451,6 +460,66 @@ later. Verified by `packages/core/test/session-runner-resume.test.ts`. Resume is verified end to end: it resolves on a healthy session and rejects on a failing one with the original tagged error (`LLM.Error`) reconstructed across the boundary. +### A turn nobody starts + +`session start` still needs something running to hand the prompt to. A schedule does not: it is a +Temporal object, and what it fires is a workflow that admits the prompt itself and then starts the +session's own supervisor. At firing time there is no client and no serve process, only workers. + +The session is created once, when the schedule is made, because a session is a row in the store +before it is anything else. After that the firing reaches only Temporal. The prompt is admitted as +queued rather than delivered, so a firing that lands while the last turn is still working is not +lost: it is drained when that turn ends, and overlapping firings are skipped rather than stacked. + +### Picking a deployment rather than assembling one + +The settings below are not independent, and getting them wrong fails as something else later: a +store only one process can see reads as a worker that never picks anything up. `OPENCODE_TEMPORAL_PROFILE` +picks one deployment and the rest follow. + +| | `local` (default) | `fleet` | +|---|---|---| +| what it is | one serve, worker inside it | serve processes and workers, separate | +| store | this process only | **you set** `OPENCODE_DB_URL` | +| role | `both` | `client` for serve, `worker` for workers | +| unit of work | a whole step | the model call, each tool call, the seal | + +Individual settings override profile defaults. Preflight checks this process's configuration and +rejects combinations it declares unsupported. It does not compare processes, probe another host's +storage, or prove fleet availability. + +Reaching a server that is not the dev server: + +```bash +TEMPORAL_ADDRESS=your-ns.a1b2c.tmprl.cloud:7233 TEMPORAL_NAMESPACE=your-ns.a1b2c \ + OPENCODE_TEMPORAL_API_KEY_FILE=/run/secrets/temporal-key # Temporal Cloud +TEMPORAL_ADDRESS=temporal.internal:7233 \ + OPENCODE_TEMPORAL_TLS_CERT=/run/secrets/tls.crt \ + OPENCODE_TEMPORAL_TLS_KEY=/run/secrets/tls.key # a cluster with mTLS +``` + +Both roles read credentials through the same connection helper. Their environment and credential +files can still differ. Operators must align the address, namespace, queues, and storage settings. + +Ask before deploying rather than after: + +```bash +opencode session doctor +``` + +It prints this process's resolved settings, configuration errors, and warnings. An incomplete +certificate pair or unsupported fleet store setting fails preflight. A remote plaintext connection +produces a warning. None of these checks verifies another process's actual storage access. + +Sessions that are already running do not have to be drained first. What a stepped turn does after a +pinned dispatch fails is a workflow decision, so it is written into every history that reached it, +and a run recorded before that rule changed would replay into a nondeterminism error. Those rules +sit behind `patched()`, so an old run keeps the behaviour it recorded and a new one gets the current +rule. The same holds for the step ceiling, which is also something the supervisor schedules. +`packages/temporal/test/l2-replay.test.ts` replays both directions: a history this code writes, and +the kept ones under `test/fixture/histories`, each recorded by the code that predates a rule. +Removing a patch fails it. + ### Running workers separately By default the serve process hosts both the Temporal activity worker and the workflow client @@ -492,7 +561,8 @@ re-drives without a saved rule). Graceful shutdown retires the process's pending and a revived attempt flips them back to pending; after a hard crash the pending row feeds the retry. A pending ask whose session is abandoned lingers in the list until a reply retires it. Verified by `packages/core/test/permission-durable.test.ts` (two independent stacks over one store). -The `question` tool still uses an in-process deferred and needs the same treatment. +`question_request` also persists pending questions and their answers. Its local deferred races a +store poll. `question-durable.test.ts` covers separate service stacks over one database. ## Shared, durable event store (any-worker resume) @@ -569,13 +639,47 @@ and dependencies are not captured, so a rebuilt tree may need an install step be 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. +The rules that bound it, because checking a stored tree out over the wrong one destroys work: + +- **Chain depth orders packs.** Parent links determine depth. Equal-depth branches still use + `time_created` as a tiebreaker. This is ordering, not publication admission. +- **A stale host fails the initial push check.** `SnapshotSync.push` compares the host's tip note + with one head reading. Packing and insertion follow outside a shared head transaction. The check + does not exclude two concurrent publishers, or a late process using a note another activity + refreshed. The existing tests cover sequential stale publication and clock ordering. +- **A tree is moved only when this host has a note for it**, which means this host agreed to that + state: it either built the tree from packs or captured the tree from there. A developer's own + checkout has no note, so it is never rewritten. Gating this on whether the tree carried the marker + a rebuild writes was wrong in the other direction: a host that seeded the session from its own + checkout never has that marker, so once anyone else shipped, every activity that host drew failed + for good. +- **The tip note is written after the insert, never before.** The packing swallows its own failures, + so a note written first and an insert that then failed named a tree the store never saw: the host + was behind nothing it could see, and every later ship from it was refused. +- **A tool ships from the host that ran it.** The seal can land anywhere, and it used to be the only + thing that captured, so a tool's writes reached the store only when the seal happened to be on + the same host. +- **Step affinity keeps parallel tools on one worker's directory.** + `OPENCODE_TEMPORAL_STEP_AFFINITY` defaults on. Each worker polls a queue keyed by host and + directory. Pinned activities have one attempt and a 30-second `scheduleToStartTimeout`. + Fallback waits for all pinned promises and runs shared dispatches sequentially. Only a + schedule-to-start failure permits this move. A started attempt that fails can still write its + directory, so its call stays where it is and is never dispatched again. The step is closed on + the shared queue instead, and the turn continues with the next one. That seal does not touch the + tree: it is standing in a directory that never ran the tools, so rebuilding there would put it on + the newest state while the host that did run them may still be writing, and what it captured + would be the state before the step. What makes carrying on safe is the publication fence: the + files a superseded attempt ships are refused under the same owner token that already fences its + event appends. Between the dispatch failing and the next step claiming the log, nothing fences + that host, and what makes the window harmless is that nobody else publishes during it. A declined + permission and cancellation retain their stop semantics. +- **Without affinity, shared-store tools run sequentially.** + `OPENCODE_TEMPORAL_SERIAL_TOOLS=1` is the default when affinity is off. This prevents ordinary + same-step dispatches from concurrently editing separate copies. It does not stop an attempt + that outlives its timeout. +- **Capturing and shipping the tree is one at a time per directory.** Two tools of one step now run + at once on one host, and both end by capturing and pushing: a capture writes the git index and a + push compares against the store's head, so two of them in one directory race on both. Host-local state that does NOT ride the DB, so it is not reconstructed on a different host: @@ -588,13 +692,122 @@ Host-local state that does NOT ride the DB, so it is not reconstructed on a diff `${data}` (the XDG data dir) at shared storage to make them portable. +### What recovers, and what a person has to answer for + +Local mode uses the in-process coordinator. Temporal mode is +`OPENCODE_SESSION_EXECUTION=temporal`. Each row states the boundary of the cited check. + +| Failure | Local mode | Temporal mode | Evidence and limit | +|---|---|---|---| +| Prompt committed before its wake | A later `wake` or `resume` can consume the recorded prompt | A wake already accepted by Temporal is durable. The application write and wake are separate operations | Schedule tests cover retried firing admission, not a crash between an ordinary HTTP admission and wake | +| Process dies mid-turn | No automatic restart reconciler; explicit execution can read the record | Whole-step activities retry. Split model calls can retry; an uncertain started pin closes its step elsewhere and the turn goes on | `session-runner-resume.test.ts`; `l2-pinned-retry.test.ts`. A workflow promise ending does not prove process death, which is why the call itself never moves | +| Tool outcome is absent | Recovery retains completed results, re-executes declared idempotent tools, and reports other started calls as unknown | Same runner rule after execution resumes | `session-runner-resume.test.ts`; external effects are not reconciled | +| Two dispatches read one pending call | One coordinator serializes its own turn | Non-idempotent L2 dispatches compete for one deterministic admission event; declared idempotent tools can execute again | `session-runner-model-call.test.ts` forces both reads before either admission | +| Attempts overlap | The local coordinator governs one process | Same-run ordered owner tokens fence later event appends. Tools and seals share their model attempt's owner | `event-claim.test.ts`; cross-run age is not encoded, and event fencing cannot stop filesystem effects | +| User stops the turn | The coordinator cancels its run | The drain is cancelled; the supervisor remains available. Declined permission must not trigger queue fallback | Interrupt tests and `l2-step.test.ts`; cancellation does not prove the child process has stopped | +| Restart during approval or question | Pending state is stored; execution still needs restarting | Retried asks adopt stored state and can receive a reply from another process | `permission-durable.test.ts`, `question-durable.test.ts`; answers are stored in tables, not workflow signals | +| Behind host publishes files | Shared-file concurrency is outside the one-coordinator deployment | Initial tip check rejects sequential stale publication | `worktree-materialize.test.ts`, `snapshot-chain.test.ts`; neither proves atomic concurrent head admission | +| Fresh worker receives a project | Requires access to the directory | Packs rebuild tracked files at the recorded absolute path | `worktree-materialize.test.ts`; ignored files and external tool effects do not travel | +| Workflow history grows | No workflow history | Drain count or `continueAsNewSuggested` requests rollover. The supervisor waits for a drain boundary and finished handlers | `session-supervisor-rollover.test.ts`; a long active turn does not roll over mid-step | +| Pinned queue is unavailable | No queue | Conclusively unstarted work migrates after the pinned batch settles; uncertain started work stays with its host and its step is closed on the shared queue | `l2-step.test.ts`, `l2-pinned-retry.test.ts`; the old process is fenced out of the store rather than accounted for, and its directory stays refused until it is provably free | +| A later turn reuses a directory an abandoned tool may still write | Not reachable: one coordinator holds the directory | The host records each call inside its own execution and refuses the directory to any other step until that call returns. The refusal is a defect, so the work is scheduled again and another host can take it. A marker retires itself when this host can show the call is over: the writer's process is gone, nothing carrying the name that worker put in its environment is still running, its process group is empty, and the machine has not restarted underneath those pids. Everything a tool starts inherits that name, including through `setsid`, so what still needs an operator is a tool that both left the group and was handed an environment of somebody else's choosing. The refusal is retryable and carries its own short delay, so it does not climb the backoff a failing activity earns | `worktree-materialize.test.ts` covers the refusal, its step scope, and each way a marker retires; removing any of them fails it. `l2-drain-writers.test.ts` drives the real drain, so the marker being written at all is covered, and `boundary-refusal.test.ts` pins how a refusal crosses the activity boundary | +| A superseded attempt ships files afterwards | Not reachable | Its pack is refused under the owner token the session has moved past, the same one that fences its event appends. Before anything supersedes it, its publication is ordinary and the next host builds on it | `worktree-materialize.test.ts` covers the refusal and the attempt the session is on shipping as usual; the window before the next claim holds only because the seal that closes the step does not publish, which `l2-drain-writers.test.ts` and `l2-pinned-retry.test.ts` pin | +| A turn never stops stepping | The coordinator's own loop | The supervisor stops driving it after 200 steps and says so. Each step is its own activity and each one succeeds, so nothing below the supervisor can see it | `session-supervisor-ceiling.test.ts` drives a runtime whose steps always ask for another; the ceiling is behind a patch, so a run recorded before it keeps what it recorded | + +An unknown tool outcome is a loss of evidence, not proof that execution stopped or failed. The +model can request a new call after reading that result. A non-idempotent external effect needs a +remote idempotency key, outcome query, or human reconciliation to decide what happened. + +## A session that outlives its client + +Everything above makes a session survive a worker. Together the same pieces make it survive the +*client*, which is the part a user can feel: start something, close the laptop, and pick it up from +a machine that has never seen it. + +Nothing new is needed underneath. A session is already a workflow rather than a process, the +running set already comes from Temporal visibility, the store is already shared, and a live tail +already re-reads so a subscriber sees work another process is doing. What was missing was a way to +say so from a command line, which is these three: + +```bash +# hand over a prompt and walk away; prints the session id and exits +opencode session start "port the auth module to the new API" --attach http://gateway:4096 + +# what is this deployment running right now, across every client that ever connected +opencode session running --attach http://gateway:4096 + +# follow one from anywhere, and stop when the turn stops +opencode session watch ses_abc123 --attach http://gateway:4096 + +# a turn nobody starts: the firing needs no client and no serve process +opencode session schedule "review yesterday.s merges" --cron "0 9 * * *" --attach http://gateway:4096 +``` + +`--attach` takes any serve in the deployment, because they are interchangeable: each one reads the +same store and signals the same workflows. There is no "the server that owns this session". That is +the property, and it is why these commands are plain HTTP clients with no Temporal dependency. +`$OPENCODE_SERVER` sets the endpoint once. For an interactive terminal instead of a follower, +`opencode attach --session ` already puts the TUI on a remote session. + +To run it as a deployment rather than a laptop: + +```bash +export OPENCODE_SESSION_EXECUTION=temporal +export OPENCODE_DB_URL=libsql://... # one store, so any worker resumes any session +export TEMPORAL_ADDRESS=... + +OPENCODE_TEMPORAL_ROLE=worker bun run packages/server/src/worker.ts # as many as you want +OPENCODE_TEMPORAL_ROLE=client opencode serve --port 4096 # as many as you want +``` + +### Verified + +`packages/temporal/scripts/detached-session-check.sh` runs the whole claim against real processes: +serve A starts a turn and is killed with a tool still running, the turn finishes on a standalone +worker, and serve B (which never saw the session) reports it running and replays the transcript. +Then `session start` returns without waiting, `session running` lists it, and `session watch` +follows it live from a cold client and exits when the turn ends. `session schedule` then creates a +schedule and the check waits for a firing to run a turn with no client involved at all. + +The runner publishes `session.next.turn.ended` for an ordinary ending. It is live-only, so a +separate HTTP process polling durable events does not receive it. `watch` also uses terminal steps, +quiet time retained across reconnects, and periodic absence from the running set. Its stream does +not replay a missed ending. These backstops do not create a durable per-turn outcome. + +The shared-store check includes a mutation: give serve B its +own `OPENCODE_DB` and the three cross-process assertions fail (`active` returns `{}`, the replay is +empty, the follower hangs) while the serve-A-and-worker ones still pass. + +### Across two machines + +`packages/temporal/scripts/cross-host-check.sh` runs the claim against containers, where each worker +has its own filesystem and hostname and the store is a real libSQL server. A session writes a file +on worker A, worker A's host is killed, and worker B, whose project volume is empty, continues the +same session and reads that file back. + +That check found a bug a single host cannot show. `WorktreeMaterializer.ensure` treated any existing +directory as somebody's working copy, and a fresh host has no tip note, so `behind` said no and the +tree was never built. The tools then ran against an empty directory and the model was told a wrong +answer, which is worse than a failure. On one host the case never appears: worker B either has the +project already or has no directory at all, and an absent directory materializes fine. A mounted +empty directory is the shape of a machine that has never seen the session, and it now materializes +too (`packages/core/test/worktree-materialize.test.ts` covers it). + +The compose file mounts the engine's source over the image, so a code change does not need a new +image. One libSQL server, so this shows a shared store over a network rather than one that survives +losing a node. + +Schedules have an entry point, and deployment profiles validate local settings. Storage +availability, fleet-wide configuration agreement, and uncertain process recovery remain operator +responsibilities. A profile check does not test another host's filesystem or repair its state. + ## Porting this pattern The shape transfers to any agent engine; Temporal is one executor behind a seam the engine owns. 1. Find the engine's coordination seam and name it: here, four verbs (`active`, `wake`, `resume`, `interrupt`) behind one substitutable service, with the in-process coordinator as the default. -2. Make the turn body an idempotent, fenced step function: claim the log with an owner token, +2. Give the step an explicit recovery contract: claim the log with an owner token, reuse recorded results on re-drive, encode errors so they survive a process boundary. 3. Write the executor as a thin workflow that loops the step as activities; keep the loop free of engine imports so it stays deterministic and sandbox-safe. diff --git a/packages/temporal/docker/Dockerfile b/packages/temporal/docker/Dockerfile new file mode 100644 index 000000000000..48f699b5829f --- /dev/null +++ b/packages/temporal/docker/Dockerfile @@ -0,0 +1,31 @@ +# A worker (or a serve) as its own machine. Running this in containers is what turns "any worker +# resumes any session" from a claim about processes into a claim about hosts: each of these has its +# own filesystem, its own hostname, and nothing of the session on disk. What they share is the +# Temporal cluster and one libSQL store, which is exactly what the README asks an operator to set up. + +FROM oven/bun:1.3.14 + +# python3 and a compiler are here for one dependency: a tree-sitter grammar builds from source at +# install time. git is not incidental either: the snapshot packs a worker rebuilds a worktree from are git packs, so a +# host that has never seen the project needs it to materialize the tree. +RUN apt-get update \ + && apt-get install -y --no-install-recommends git ca-certificates procps curl \ + python3 make g++ \ + && rm -rf /var/lib/apt/lists/* \ + && git config --system user.email opencode@example.com \ + && git config --system user.name opencode \ + && git config --system init.defaultBranch main \ + && git config --system --add safe.directory '*' + +WORKDIR /app + +COPY package.json bun.lock bunfig.toml tsconfig.json* ./ +COPY patches ./patches +COPY packages ./packages +RUN bun install --frozen-lockfile + +ENV OPENCODE_SESSION_EXECUTION=temporal + +# The worker by default. The serve role overrides this in compose; both build the same application +# context, so the only difference is whether an HTTP surface comes with it. +CMD ["bun", "run", "packages/server/src/worker.ts"] diff --git a/packages/temporal/docker/compose.yml b/packages/temporal/docker/compose.yml new file mode 100644 index 000000000000..a063433fa795 --- /dev/null +++ b/packages/temporal/docker/compose.yml @@ -0,0 +1,103 @@ +# Two workers that are two machines, not two processes on one. +# +# What they share is what an operator is told to share: one Temporal cluster and one libSQL store. +# What they do not share is the session's working tree. `worker-a` has the project, `worker-b` gets +# an empty volume, so a session that moves between them has to rebuild the tree from the snapshot +# packs in the store. That is the part a single host can never really test, because there the tree +# is already sitting on the disk the other process is reading. +# +# docker compose -f packages/temporal/docker/compose.yml up -d temporal sqld serve worker-a +# +# Not covered: one libSQL server, so this shows a shared store over a network rather than a store +# that survives losing a node. + +name: opencode-l3 + +# A mapping rather than a list, because a list cannot be merged: a service that adds one variable +# would otherwise replace the whole set and silently lose the store. +x-env: &env + OPENCODE_SESSION_EXECUTION: temporal + TEMPORAL_ADDRESS: temporal:7233 + # One store for every host. Without it a session belongs to whichever machine holds its file. + OPENCODE_DB_URL: http://sqld:8080 + OPENCODE_TEMPORAL_STEPPED: "1" + OPENCODE_SERVER_PASSWORD: ${OPENCODE_SERVER_PASSWORD:-l3-check} + OPENAI_API_KEY: ${OPENAI_API_KEY:?set OPENAI_API_KEY} + +x-app: &app + image: opencode-temporal:l3 + # The engine's own source, over the copy baked into the image. bun runs TypeScript directly, so + # this is the same code the image would have had; mounting it keeps a one-file change from + # costing a full dependency install, which is most of the build. + volumes: + - ../../core/src:/app/packages/core/src:ro + - ../src:/app/packages/temporal/src:ro + depends_on: + temporal: + condition: service_healthy + sqld: + condition: service_started + +services: + temporal: + image: temporalio/admin-tools:1.29 + # The image's own entrypoint is `sleep infinity`, so a command alone becomes arguments to sleep. + entrypoint: ["temporal"] + command: ["server", "start-dev", "--ip", "0.0.0.0", "--log-level", "warn"] + ports: + - "7243:7233" + healthcheck: + test: ["CMD", "temporal", "operator", "cluster", "health", "--address", "127.0.0.1:7233"] + interval: 5s + timeout: 5s + retries: 40 + + sqld: + image: ghcr.io/tursodatabase/libsql-server:latest + environment: + - SQLD_NODE=primary + ports: + - "8081:8080" + + # Drives workflows, hosts no worker, and is the only thing with an HTTP surface. + serve: + <<: *app + environment: + <<: *env + OPENCODE_TEMPORAL_ROLE: client + # Absolute, because working_dir is the project rather than the checkout: a relative entry path + # would be looked for inside the session's tree. + command: ["bun", "run", "/app/packages/cli/src/index.ts", "serve", "--port", "4096", "--hostname", "0.0.0.0"] + working_dir: /project + ports: + - "4096:4096" + volumes: + - ../../core/src:/app/packages/core/src:ro + - ../src:/app/packages/temporal/src:ro + - project-a:/project + + worker-a: + <<: *app + environment: + <<: *env + OPENCODE_TEMPORAL_ROLE: worker + volumes: + - ../../core/src:/app/packages/core/src:ro + - ../src:/app/packages/temporal/src:ro + - project-a:/project + + # No project volume of its own that has ever seen this session: an empty tree, so the worktree has + # to come from the packs in the store. + worker-b: + <<: *app + environment: + <<: *env + OPENCODE_TEMPORAL_ROLE: worker + volumes: + - ../../core/src:/app/packages/core/src:ro + - ../src:/app/packages/temporal/src:ro + - project-b:/project + +volumes: + project-a: + project-b: diff --git a/packages/temporal/scripts/cross-host-check.sh b/packages/temporal/scripts/cross-host-check.sh new file mode 100755 index 000000000000..5d5fd9e83244 --- /dev/null +++ b/packages/temporal/scripts/cross-host-check.sh @@ -0,0 +1,144 @@ +#!/usr/bin/env bash +# Any worker resumes any session, across machines rather than across processes. +# +# On one host the second worker already has the project on disk, so the interesting half of the +# claim is never exercised: the tree is there whether or not anything shipped it. Here worker B is a +# container with an empty project volume, so a session that moves to it has to bring its worktree +# along, out of the snapshot packs in the shared store. +# +# Usage: OPENAI_API_KEY=... packages/temporal/scripts/cross-host-check.sh +# +# Not covered: one libSQL server, so this shows a shared store over a network rather than one that +# survives losing a node. + +set -uo pipefail + +cd "$(dirname "${BASH_SOURCE[0]}")/../../.." +COMPOSE="docker compose -f packages/temporal/docker/compose.yml" +MODEL_ID="${MODEL_ID:-gpt-5-mini}" + +fails=0 +ok() { printf 'PASS %s\n' "$1"; } +bad() { printf 'FAIL %s (%s)\n' "$1" "${2:-}"; fails=$((fails + 1)); } + +# KEEP=1 leaves the stack up, which is the difference between reading a failure and guessing at it. +cleanup() { [ -n "${KEEP:-}" ] || $COMPOSE down -v >/dev/null 2>&1; } +trap cleanup EXIT + +[ -n "${OPENAI_API_KEY:-}" ] || { echo "set OPENAI_API_KEY"; exit 1; } + +# A workspace link pointing at a package the store no longer holds. `bun install` leaves these +# behind when node_modules was pruned by hand, and what a broken one produces is an ENOENT from +# whichever command first needs that package, which reads like a bug in the command. Cheap to ask +# here, and the answer is always the same: install again from a clean tree. +dangling="" +for link in packages/*/node_modules/* packages/*/node_modules/@*/* \ + packages/*/*/node_modules/* packages/*/*/node_modules/@*/*; do + [ -L "$link" ] && [ ! -e "$link" ] && dangling="$dangling $link"$'\n' +done +if [ -n "$dangling" ]; then + printf 'the install is stale; these workspace links point at nothing:\n%s' "$dangling" + echo "run: find . -name node_modules -type d -prune -exec rm -rf {} + && bun install" + exit 1 +fi + +$COMPOSE down -v >/dev/null 2>&1 +# Only when the image is missing. The compose file mounts the engine's source over the image, so a +# code change does not need a new one, and the dependency install is most of the build. +if ! docker image inspect opencode-temporal:l3 >/dev/null 2>&1; then + docker build -f packages/temporal/docker/Dockerfile -t opencode-temporal:l3 . >/dev/null \ + || { echo "build failed"; exit 1; } +fi +$COMPOSE up -d temporal sqld serve worker-a >/dev/null 2>&1 || { echo "stack failed"; exit 1; } + +api() { curl -s -u "opencode:$PW" "$@"; } + +# The serve generates its own password on first boot and prefers it over the environment, so ask it +# rather than tell it. +PW="" +for _ in $(seq 1 60); do + PW=$($COMPOSE exec -T serve sh -c 'cat /root/.local/state/opencode/password 2>/dev/null' 2>/dev/null | tr -d '\r\n') + [ -n "$PW" ] && break + sleep 3 +done +[ -n "$PW" ] && ok "serve is up" || { bad "serve never came up"; exit 1; } + +hostA=$($COMPOSE exec -T worker-a hostname 2>/dev/null | tr -d '\r') +[ -n "$hostA" ] && ok "worker A is a host of its own ($hostA)" || bad "worker A came up" + +# A project only worker A and serve can see. +$COMPOSE exec -T serve sh -c \ + 'cd /project && git init -q 2>/dev/null; echo hello > README.md; git add -A; git commit -qm init' \ + >/dev/null 2>&1 + +new_session() { + api -X POST http://127.0.0.1:4096/api/session -H 'content-type: application/json' \ + -d '{"directory":"/project"}' | sed -n 's/^{"data":{"id":"\([^"]*\)".*/\1/p' +} +prompt() { + api -o /dev/null -X POST "http://127.0.0.1:4096/api/session/$1/prompt" \ + -H 'content-type: application/json' -d "{\"prompt\":{\"text\":$2}}" +} +# A turn is over when a step of it ends on "stop", which is not the same as the session leaving the +# running set: the supervisor stays open for its idle timeout with nothing left to do. Counted +# rather than matched, because the history of a second turn still contains the first one's ending, +# and matching would call every later turn finished before it started. +stops() { + local body + body=$(api "http://127.0.0.1:4096/api/session/$1/history?limit=100" 2>/dev/null) + case "$body" in *InvalidRequestError*) echo " history rejected: $body" >&2; echo -1; return ;; esac + printf '%s' "$body" | grep -o '"finish":"stop"' | wc -l | tr -d ' ' +} +await_turn() { + local before=$2 + for _ in $(seq 1 90); do + [ "$(stops "$1")" -gt "$before" ] && return 0 + sleep 4 + done + return 1 +} + +sid=$(new_session) +[ -n "$sid" ] && ok "a session was created ($sid)" || { bad "no session"; exit 1; } +api -o /dev/null -X POST "http://127.0.0.1:4096/api/session/$sid/model" \ + -H 'content-type: application/json' -d "{\"model\":{\"id\":\"$MODEL_ID\",\"providerID\":\"openai\"}}" + +# --- turn 1 on worker A: writes a file, so a snapshot of the tree is captured and shipped +before=$(stops "$sid") +prompt "$sid" '"Use the bash tool to run exactly: echo TRAVELLED > /project/note.txt && cat /project/note.txt. Report the output."' +await_turn "$sid" "$before" && ok "turn 1 finished on worker A" || bad "turn 1 never finished" + +packs=$(curl -s http://127.0.0.1:8081/v2/pipeline -H 'content-type: application/json' \ + -d '{"requests":[{"type":"execute","stmt":{"sql":"select count(*) from snapshot_pack"}},{"type":"close"}]}' \ + 2>/dev/null | grep -o '"value":"[0-9]*"' | head -1 | grep -o '[0-9]*') +[ "${packs:-0}" -gt 0 ] && ok "the tree was shipped to the shared store ($packs packs)" \ + || bad "no snapshot packs reached the store" "$packs" + +# --- worker A's host goes away, and a host that has never seen this project takes over +docker kill "$($COMPOSE ps -q worker-a)" >/dev/null 2>&1 +sleep 2 +[ -z "$($COMPOSE ps -q --status running worker-a)" ] && ok "worker A's host is gone" || bad "worker A's host is gone" + +$COMPOSE up -d worker-b >/dev/null 2>&1 +sleep 8 +hostB=$($COMPOSE exec -T worker-b hostname 2>/dev/null | tr -d '\r') +[ "$hostB" != "$hostA" ] && ok "worker B is a different host ($hostB)" || bad "worker B is a different host" +empty=$($COMPOSE exec -T worker-b sh -c 'ls -A /project | wc -l' 2>/dev/null | tr -d '\r ') +[ "${empty:-1}" = "0" ] && ok "worker B's project is empty before the turn" || bad "worker B's project was not empty" "$empty" + +# --- turn 2 on worker B: the file only exists there if the worktree travelled +before=$(stops "$sid") +prompt "$sid" '"Use the bash tool to run exactly: cat /project/note.txt. Report exactly what it printed."' +await_turn "$sid" "$before" && ok "turn 2 finished on worker B" || bad "turn 2 never finished" + +# Asked of worker B's own disk rather than of the transcript. The transcript still holds turn 1, +# where the file did exist, so anything matched across the whole of it proves nothing about B. +landed=$($COMPOSE exec -T worker-b sh -c 'cat /project/note.txt 2>&1' 2>/dev/null | tr -d '\r') +case "$landed" in + TRAVELLED*) ok "the worktree travelled to worker B" ;; + *) bad "the worktree travelled to worker B" "$landed" ;; +esac + +echo +[ "$fails" -eq 0 ] && echo "cross-host-check: OK" || echo "cross-host-check: $fails failed" +exit $([ "$fails" -eq 0 ] && echo 0 || echo 1) diff --git a/packages/temporal/scripts/detached-session-check.sh b/packages/temporal/scripts/detached-session-check.sh new file mode 100755 index 000000000000..5320eb52b877 --- /dev/null +++ b/packages/temporal/scripts/detached-session-check.sh @@ -0,0 +1,192 @@ +#!/usr/bin/env bash +# Proves the claim a durable session is supposed to make: it belongs to the deployment, not to +# whoever started it. One worker, two serve processes, one shared store, and a client that is only +# ever a client. +# +# 1. serve A starts a turn, then A is killed while a tool is still running +# 2. the turn finishes anyway, on a worker that is a separate process +# 3. serve B, which never saw the session, reports it running and replays the whole transcript +# 4. `session start` hands over a prompt and returns, holding no terminal +# 5. `session watch` follows that turn live from a cold client and stops when the turn stops +# +# Needs: bun, the temporal CLI, and an OpenAI key. Nothing here is a unit test; it is the evidence +# for a claim that only shows up across processes. +# +# Usage: OPENAI_API_KEY=... packages/temporal/scripts/detached-session-check.sh + +set -uo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd)" +OC="$ROOT/packages/opencode/src/index.ts" +RUN="${RUN_DIR:-/private/tmp/opencode-l3}" +PORT_TEMPORAL="${PORT_TEMPORAL:-7240}" +PORT_A="${PORT_A:-4610}" +PORT_B="${PORT_B:-4611}" +MODEL="${MODEL:-openai/gpt-5-mini}" + +fails=0 +ok() { printf 'PASS %s\n' "$1"; } +bad() { printf 'FAIL %s (%s)\n' "$1" "${2:-}"; fails=$((fails + 1)); } + +pids=() +cleanup() { + for pid in "${pids[@]:-}"; do + [ -n "$pid" ] || continue + kill -9 $(pgrep -P "$pid" 2>/dev/null) "$pid" 2>/dev/null + done +} +trap cleanup EXIT + +[ -n "${OPENAI_API_KEY:-}" ] || { echo "set OPENAI_API_KEY"; exit 1; } + +# A workspace link pointing at a package the store no longer holds. `bun install` leaves these +# behind when node_modules was pruned by hand, and what a broken one produces is an ENOENT from +# whichever command first needs that package, which reads like a bug in the command. Cheap to ask +# here, and the answer is always the same: install again from a clean tree. +dangling="" +for link in "$ROOT"/packages/*/node_modules/* "$ROOT"/packages/*/node_modules/@*/* \ + "$ROOT"/packages/*/*/node_modules/* "$ROOT"/packages/*/*/node_modules/@*/*; do + [ -L "$link" ] && [ ! -e "$link" ] && dangling="$dangling $link"$'\n' +done +if [ -n "$dangling" ]; then + printf 'the install is stale; these workspace links point at nothing:\n%s' "$dangling" + echo "run: find . -name node_modules -type d -prune -exec rm -rf {} + && bun install" + exit 1 +fi + +rm -rf "$RUN"; mkdir -p "$RUN/proj" "$RUN/logs" +git -C "$RUN/proj" init -q +echo hello > "$RUN/proj/README.md" +git -C "$RUN/proj" add -A +git -C "$RUN/proj" -c user.email=a@b.c -c user.name=t commit -qm init + +export OPENCODE_SESSION_EXECUTION=temporal +export TEMPORAL_ADDRESS="127.0.0.1:$PORT_TEMPORAL" +# One store both serves and the worker read. This is what makes any process able to answer for any +# session; without it a session belongs to the host holding its file. +export OPENCODE_DB="$RUN/shared.db" +export OPENCODE_TEMPORAL_STEPPED=1 +# A stored password wins over the environment for the v2 serve, so a script that invents one gets +# 401 on every call. Take what the server will actually be asking for. +STORED="${XDG_STATE_HOME:-$HOME/.local/state}/opencode/password" +if [ -f "$STORED" ]; then + OPENCODE_SERVER_PASSWORD="$(cat "$STORED")" +else + OPENCODE_SERVER_PASSWORD="${OPENCODE_SERVER_PASSWORD:-l3-check}" +fi +export OPENCODE_SERVER_PASSWORD + +temporal server start-dev --port "$PORT_TEMPORAL" --ui-port $((PORT_TEMPORAL + 1000)) --log-level warn \ + > "$RUN/logs/temporal.log" 2>&1 & +pids+=($!) +sleep 6 + +OPENCODE_TEMPORAL_ROLE=worker bun run "$ROOT/packages/server/src/worker.ts" > "$RUN/logs/worker.log" 2>&1 & +worker=$!; pids+=($worker) + +cd "$RUN/proj" +OPENCODE_TEMPORAL_ROLE=client bun run "$ROOT/packages/cli/src/index.ts" serve --port "$PORT_A" \ + > "$RUN/logs/serveA.log" 2>&1 & +serveA=$!; pids+=($serveA) +OPENCODE_TEMPORAL_ROLE=client bun run "$ROOT/packages/cli/src/index.ts" serve --port "$PORT_B" \ + > "$RUN/logs/serveB.log" 2>&1 & +pids+=($!) + +A="http://127.0.0.1:$PORT_A" +B="http://127.0.0.1:$PORT_B" +AUTH="opencode:$OPENCODE_SERVER_PASSWORD" + +# Bounded, because a fixed sleep is either a slow script or a flaky one. Both serves boot a whole +# application context, which on a cold module cache is not quick. +# Answering at all is not enough: an unauthorized answer is still an answer, and treating it as +# ready turns a credentials problem into a confusing timeout later. +wait_for() { + for _ in $(seq 1 60); do + [ "$(curl -s -o /dev/null -w '%{http_code}' -u "$AUTH" "$1/api/session")" = "200" ] && return 0 + sleep 2 + done + return 1 +} +wait_for "$A" && wait_for "$B" || { echo "serves never came up; see $RUN/logs"; exit 1; } + +# The id of the session, not of anything nested in it: the field is read off the first line of the +# document, so a later `"id"` (a model, a message) cannot be picked up instead. +session_id() { sed -n 's/^{"data":{"id":"\([^"]*\)".*/\1/p' | head -1; } + +# --- 1. a turn started on serve A, long enough to still be running when A dies +created=$(curl -s -u "$AUTH" -X POST "$A/api/session" -H 'content-type: application/json' \ + -d "{\"directory\":\"$RUN/proj\"}") +sid=$(printf '%s' "$created" | session_id) +[ -n "$sid" ] && ok "serve A created a session" || { bad "serve A created a session" "$created"; exit 1; } + +provider=${MODEL%%/*}; model=${MODEL#*/} +curl -s -o /dev/null -u "$AUTH" -X POST "$A/api/session/$sid/model" -H 'content-type: application/json' \ + -d "{\"model\":{\"id\":\"$model\",\"providerID\":\"$provider\"}}" +curl -s -o /dev/null -u "$AUTH" -X POST "$A/api/session/$sid/prompt" -H 'content-type: application/json' \ + -d '{"prompt":{"text":"Use the bash tool to run exactly: sleep 40 && echo SURVIVED. Then report the output."}}' +sleep 18 +pgrep -f "sleep 40 && echo SURVIVED" > /dev/null && ok "the tool is running on the worker" \ + || bad "the tool is running on the worker" "it never started" + +# --- 2. kill the process that started it, mid-tool +kill -9 $(pgrep -P $serveA 2>/dev/null) $serveA 2>/dev/null +sleep 3 +[ -z "$(lsof -nP -iTCP:$PORT_A -sTCP:LISTEN 2>/dev/null)" ] && ok "serve A is gone" || bad "serve A is gone" +pgrep -f "sleep 40 && echo SURVIVED" > /dev/null && ok "the turn outlived the client that started it" \ + || bad "the turn outlived the client that started it" "the tool died with serve A" + +# --- 3. serve B, which never saw this session, knows it and can replay it +running=$(curl -s -u "$AUTH" "$B/api/session/active") +case "$running" in *"$sid"*) ok "serve B reports it running" ;; *) bad "serve B reports it running" "$running" ;; esac + +sleep 35 +timeout 30 curl -s -N -u "$AUTH" "$B/api/session/$sid/event" > "$RUN/logs/replay.txt" 2>&1 +grep -q "SURVIVED" "$RUN/logs/replay.txt" && ok "serve B replays work done while no client existed" \ + || bad "serve B replays work done while no client existed" + +# --- 4. start a turn and walk away +started=$(timeout 90 bun run "$OC" session start \ + "Use the bash tool to run exactly: sleep 20 && echo WATCHED. Then report the output." \ + --attach "$B" --model "$MODEL" --dir "$RUN/proj" --json 2>/dev/null) +sid2=$(printf '%s' "$started" | sed -n 's/.*"id":"\([^"]*\)".*/\1/p') +[ -n "$sid2" ] && ok "session start returned an id without waiting" || bad "session start returned an id" "$started" + +listed=$(timeout 60 bun run "$OC" session running --attach "$B" --json 2>/dev/null) +case "$listed" in *"$sid2"*) ok "session running lists it" ;; *) bad "session running lists it" "$listed" ;; esac + +# --- 5. follow it live from a client that has never seen it, and stop when the turn stops +began=$(date +%s) +timeout 120 bun run "$OC" session watch "$sid2" --attach "$B" > "$RUN/logs/watch.txt" 2>&1 +took=$(( $(date +%s) - began )) +grep -q "WATCHED" "$RUN/logs/watch.txt" && ok "session watch followed the turn" \ + || bad "session watch followed the turn" "$(tail -3 "$RUN/logs/watch.txt")" +[ "$took" -lt 100 ] && ok "session watch stopped when the turn did (${took}s)" \ + || bad "session watch stopped when the turn did" "${took}s, so it hung" + +# --- 6. a turn nobody starts. The schedule is a Temporal object, so at firing time there is no +# client and no HTTP call: the workflow admits the prompt itself and starts the session's own +# supervisor. Prompted into a fresh session so what arrives can only have come from the firing. +sched=$(timeout 90 bun run "$OC" session schedule \ + "Use the bash tool to run exactly: echo SCHEDULED. Then report the output." \ + --every 10s --attach "$B" --dir "$RUN/proj" --json 2>/dev/null) +sid3=$(printf '%s' "$sched" | sed -n 's/.*"session":"\([^"]*\)".*/\1/p') +scheduleId=$(printf '%s' "$sched" | sed -n 's/.*"schedule":"\([^"]*\)".*/\1/p') +[ -n "$sid3" ] && ok "session schedule created one" || bad "session schedule created one" "$sched" + +# Long enough for a firing plus a turn, and nothing here prompts it. +answered="" +for _ in $(seq 1 24); do + sleep 5 + answered=$(curl -s -u "$AUTH" "$B/api/session/$sid3/message" 2>/dev/null || true) + case "$answered" in *SCHEDULED*) break ;; esac +done +case "$answered" in + *SCHEDULED*) ok "a firing ran a turn with no client involved" ;; + *) bad "a firing ran a turn with no client involved" "$(printf '%s' "$answered" | head -c 200)" ;; +esac +[ -n "$scheduleId" ] && temporal schedule delete --schedule-id "$scheduleId" \ + --address "127.0.0.1:$PORT_TEMPORAL" >/dev/null 2>&1 + +echo +[ "$fails" -eq 0 ] && echo "detached-session-check: OK" || echo "detached-session-check: $fails failed" +exit $([ "$fails" -eq 0 ] && echo 0 || echo 1) diff --git a/packages/temporal/src/boundary.ts b/packages/temporal/src/boundary.ts index 579f52024096..73f08bfda8c0 100644 --- a/packages/temporal/src/boundary.ts +++ b/packages/temporal/src/boundary.ts @@ -12,7 +12,7 @@ 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 { encodeRunError } from "./run-error-codec" import { HALTED_FAILURE_TYPE } from "./protocol" export interface BoundaryOptions { @@ -36,6 +36,30 @@ const halted = (sessionID: string, declined?: SessionRunDeclinedError) => { }) } +// Failures that say something about the moment rather than about the work: storage that was not +// reachable, a defect from a database call that `orDie` turned into one. Everything else stays +// non-retryable, because re-running a step whose input the model already answered is worse than +// failing it. Without this a libsql blip during a seal failed the step for good rather than moving +// it to another worker. +const QUARANTINED = "WorktreeMaterializer.QuarantinedError" +// A refusal must not climb the backoff a failing activity earns: the interval doubles per attempt, +// and the host that answers first and refuses fastest is exactly the one that would push the next +// attempt minutes out while a free host sits idle. Long enough not to spin, short enough that the +// work reaches another host in about the time one dispatch takes. +const REFUSAL_RETRY = "2 seconds" + +const TRANSIENT = new Set([ + "ToolOutputStore.StorageError", + "SqlError", + "SqliteError", + // A rebuild that did not finish. git and the filesystem fail for reasons that pass, and the + // alternative is a turn failing for good because one worker had a bad minute. + "WorktreeMaterializer.MaterializeError", + // And a directory this host is refused. It is this host saying no, not the work failing: the + // same dispatch runs fine on a host that is not holding somebody's abandoned tool. + QUARANTINED, +]) + export const runAtBoundary = async ( sessionID: string, signal: AbortSignal, @@ -63,7 +87,8 @@ export const runAtBoundary = async ( throw ApplicationFailure.create({ message: squashed?.message ?? Cause.pretty(cause), type: squashed?._tag ?? "SessionRunError", - nonRetryable: true, + nonRetryable: !(squashed?._tag !== undefined && TRANSIENT.has(squashed._tag)), details: encoded === undefined ? undefined : [encoded], + ...(squashed?._tag === QUARANTINED ? { nextRetryDelay: REFUSAL_RETRY } : {}), }) } diff --git a/packages/temporal/src/config.ts b/packages/temporal/src/config.ts index 0fbe624b414c..93adfef85194 100644 --- a/packages/temporal/src/config.ts +++ b/packages/temporal/src/config.ts @@ -3,6 +3,7 @@ export * as TemporalConfig from "./config" // Connection and behavior settings for the Temporal executor. The executor reads them at layer // build: an embedder or a test provides the service to override, and absent that the values come // from env. Nothing reads env at module load, so import order carries no configuration. +import { readFileSync } from "node:fs" import { Context } from "effect" import { DEFAULTS } from "./protocol" @@ -11,11 +12,22 @@ import { DEFAULTS } from "./protocol" // worker's bundler); `worker` runs a standalone activity worker with no HTTP surface. export type Role = "both" | "client" | "worker" +// Which deployment this is. The settings below are not independent: a fleet whose store is not +// shared is a set of workers that cannot see each other's sessions, and finding that out takes a +// session that answers with the wrong files. `fleet` sets what has to agree, and `preflight` +// refuses what cannot. +export type Profile = "local" | "fleet" + export interface Interface { + readonly profile: Profile readonly address: string readonly namespace: string readonly taskQueue: string readonly role: Role + /** How a server that is not the dev server is reached: an API key for Cloud, a certificate pair + * for a cluster with mTLS. Read from files, never from argv, and never logged. */ + readonly apiKey?: string + readonly tls?: { readonly cert: string; readonly key: string; readonly ca?: string } | true /** 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: @@ -29,17 +41,134 @@ export interface Interface { /** 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 + /** Run a step's tool calls one at a time instead of together. Tools of one step write the same + * tree and each ships from the host that ran it, so two on two hosts each publish a tree without + * the other's work: the second is refused rather than reverting the first, which leaves its work + * stranded there. `OPENCODE_TEMPORAL_SERIAL_TOOLS=1` forces it on; it is not needed while a step + * is pinned to one worker, which is the default. */ + readonly serialTools?: boolean + /** Send the tools and the seal of a step back to the worker that made its model call, on a queue + * that worker polls on its own. That worker is standing in the tree the tools are about to write, + * so the step's tools see each other's writes through the filesystem and can run at once. On by + * default: a pin nobody answers falls back to the shared queue after a schedule-to-start bound, + * so the worst it costs is that wait. `OPENCODE_TEMPORAL_STEP_AFFINITY=0` turns it off. */ + readonly stepAffinity?: boolean } export class Service extends Context.Service()("@opencode/temporal/Config") {} +const read = (path: string | undefined) => (path ? readFileSync(path, "utf8") : undefined) +const given = (name: string) => process.env[name] !== undefined && process.env[name] !== "" +const onOff = (name: string, fallback: boolean) => (given(name) ? process.env[name] === "1" : fallback) + export const fromEnv = (): Interface => ({ + profile: process.env.OPENCODE_TEMPORAL_PROFILE === "fleet" ? "fleet" : "local", address: process.env.TEMPORAL_ADDRESS ?? DEFAULTS.address, namespace: process.env.TEMPORAL_NAMESPACE ?? DEFAULTS.namespace, taskQueue: process.env.OPENCODE_TEMPORAL_TASK_QUEUE ?? DEFAULTS.taskQueue, role: (process.env.OPENCODE_TEMPORAL_ROLE as Role | undefined) ?? "both", + apiKey: process.env.OPENCODE_TEMPORAL_API_KEY ?? read(process.env.OPENCODE_TEMPORAL_API_KEY_FILE), + tls: + process.env.OPENCODE_TEMPORAL_TLS_CERT && process.env.OPENCODE_TEMPORAL_TLS_KEY + ? { + cert: readFileSync(process.env.OPENCODE_TEMPORAL_TLS_CERT, "utf8"), + key: readFileSync(process.env.OPENCODE_TEMPORAL_TLS_KEY, "utf8"), + ca: read(process.env.OPENCODE_TEMPORAL_TLS_CA), + } + : process.env.OPENCODE_TEMPORAL_TLS === "1" + ? true + : undefined, idleTimeout: process.env.OPENCODE_SESSION_IDLE_TIMEOUT, - stepped: process.env.OPENCODE_TEMPORAL_STEPPED === "1", + // A fleet's unit of work is the smaller one: a worker dying takes one tool call with it rather + // than a whole step, and a tool call is where the retry policy and the approval belong. + stepped: onOff("OPENCODE_TEMPORAL_STEPPED", process.env.OPENCODE_TEMPORAL_PROFILE === "fleet"), worktreeAffinity: process.env.OPENCODE_TEMPORAL_WORKTREE_AFFINITY === "1", worktree: process.env.OPENCODE_TEMPORAL_WORKTREE, + stepAffinity: process.env.OPENCODE_TEMPORAL_STEP_AFFINITY !== "0", + serialTools: + process.env.OPENCODE_TEMPORAL_SERIAL_TOOLS === "1" || + (process.env.OPENCODE_TEMPORAL_SERIAL_TOOLS !== "0" && + process.env.OPENCODE_TEMPORAL_STEP_AFFINITY === "0" && + process.env.OPENCODE_TEMPORAL_WORKTREE_AFFINITY !== "1" && + !!process.env.OPENCODE_DB_URL), +}) + +/** What `Connection.connect` and `NativeConnection.connect` both take, built once so a client and a + * worker in different processes cannot disagree about how the cluster is reached. */ +export const connectionOptions = (config: Interface) => { + const tls = + config.tls === true || (config.apiKey && config.tls === undefined) + ? true + : config.tls + ? { + clientCertPair: { crt: Buffer.from(config.tls.cert), key: Buffer.from(config.tls.key) }, + ...(config.tls.ca ? { serverRootCACertificate: Buffer.from(config.tls.ca) } : {}), + } + : undefined + return { + address: config.address, + ...(tls ? { tls } : {}), + ...(config.apiKey ? { apiKey: config.apiKey } : {}), + } +} + +const LOOPBACK = /^(127\.0\.0\.1|localhost|\[::1\]|0\.0\.0\.0)(:|$)/ + +/** + * What is wrong with this deployment, said before it takes work rather than after. Each of these + * fails as something else: a store only one process can see reads as a worker that never picks + * anything up, and a client with no worker anywhere reads as a session that accepts a prompt and + * never answers it. + */ +export const preflight = (config: Interface): string[] => { + const problems: string[] = [] + const shared = !!process.env.OPENCODE_DB_URL + if (config.profile === "fleet") { + if (!shared) + problems.push( + "the fleet profile needs OPENCODE_DB_URL: the store is the record, and workers that do " + + "not share it cannot serve each other's sessions", + ) + if (config.role === "both") + problems.push( + "OPENCODE_TEMPORAL_ROLE is `both` in a fleet: a serve that also polls is a laptop " + + "deployment. Run `client` next to standalone `worker` processes", + ) + } + if (config.apiKey && LOOPBACK.test(config.address)) + problems.push(`an API key is set but TEMPORAL_ADDRESS is ${config.address}, which is a dev server`) + if (config.apiKey && config.namespace === "default") + problems.push("an API key is set but TEMPORAL_NAMESPACE is `default`, which is not a Cloud namespace") + if (!!process.env.OPENCODE_TEMPORAL_TLS_CERT !== !!process.env.OPENCODE_TEMPORAL_TLS_KEY) + problems.push("OPENCODE_TEMPORAL_TLS_CERT and OPENCODE_TEMPORAL_TLS_KEY come as a pair") + return problems +} + +/** + * Worth saying, not worth refusing. Only what cannot work belongs in `preflight`, because a process + * that exits takes a deployment with it, and plaintext to an address that is not loopback is a + * private network in most deployments and a mistake in some. Nothing here can tell which. + */ +export const notes = (config: Interface): string[] => { + const said: string[] = [] + if (!LOOPBACK.test(config.address) && !config.apiKey && !config.tls) + said.push( + `reaching ${config.address} in plaintext. For Temporal Cloud set OPENCODE_TEMPORAL_API_KEY; ` + + "for a cluster with mTLS set OPENCODE_TEMPORAL_TLS_CERT and OPENCODE_TEMPORAL_TLS_KEY", + ) + return said +} + +/** Every setting that decides how this process behaves, and nothing that is a credential. */ +export const describe = (config: Interface): Record => ({ + profile: config.profile, + address: config.address, + namespace: config.namespace, + taskQueue: config.taskQueue, + role: config.role, + store: process.env.OPENCODE_DB_URL ? "shared (OPENCODE_DB_URL)" : "this process only", + stepped: String(config.stepped === true), + stepAffinity: String(config.stepAffinity !== false), + serialTools: String(config.serialTools === true), + credentials: config.apiKey ? "api key" : config.tls ? "certificate pair" : "none (plaintext)", }) diff --git a/packages/temporal/src/executor.ts b/packages/temporal/src/executor.ts index 94987e603aa5..27ae50ca55ad 100644 --- a/packages/temporal/src/executor.ts +++ b/packages/temporal/src/executor.ts @@ -1,11 +1,11 @@ export * as SessionExecutionTemporal from "./executor" import { fileURLToPath } from "node:url" +import { hostname } from "node:os" import { Effect, Layer, Option } from "effect" import { Client, Connection, WithStartWorkflowOperation } from "@temporalio/client" -// Imported lazily inside the worker branch: the worker package drags webpack and swc (it bundles -// the workflow from source at startup), which a compiled binary can neither bundle nor run. A -// packaged serve runs OPENCODE_TEMPORAL_ROLE=client next to standalone workers instead. +// This build leaves worker bundling dependencies outside the compiled client binary. +// Standalone workers use the source-based workflow entry point below. import { LocationServiceMap } from "@opencode-ai/core/location-service-map" import { EventV2 } from "@opencode-ai/core/event" @@ -15,13 +15,13 @@ import { SessionStore } from "@opencode-ai/core/session/store" import { SessionExecution } from "@opencode-ai/core/session/execution" import { makeStepActivities, makeSteppedTurnActivities } from "./activities" import { makeDrains } from "./drain" -import { makeL2Drains } from "./l2-drain" -import { queueForWorktree } from "./queue" +import { makeL2Drains, makeScheduleDrains } from "./l2-drain" +import { queueForWorktree, queueForWorker } 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 { toRunError } from "./run-error-codec" import * as WF from "./workflow" import { TemporalConfig } from "./config" import { WORKFLOW_TYPE, WORKFLOW_ID_PREFIX, workflowId } from "./protocol" @@ -66,6 +66,9 @@ const layer = Layer.effect( // override as a workflow argument. const IDLE_TIMEOUT = config.idleTimeout const STEPPED = config.stepped === true + // Only the client can read whether the store is shared, so whether a step's tools may overlap + // is decided here and rides the workflow input. + const SERIAL_TOOLS = config.serialTools === 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. @@ -73,6 +76,13 @@ const layer = Layer.effect( // 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 + // The queue this worker polls on its own, so a step can be sent back to it. Keyed by host as + // well as directory: two containers serve `/project` and share none of it. Only workers have + // one, and only they report it, so a client-only process never pins a step to itself. + const STEP_QUEUE = + HOST_WORKER && config.stepAffinity !== false + ? queueForWorker(TASK_QUEUE, hostname(), SERVED_WORKTREE) + : undefined // 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 @@ -91,6 +101,13 @@ const layer = Layer.effect( return project ? queueForWorktree(TASK_QUEUE, project.worktree) : TASK_QUEUE }) : Effect.succeed(TASK_QUEUE) + // Before anything is accepted, not after: every one of these fails as something else later, and + // the failure lands on whoever prompted the session rather than on whoever deployed it. + for (const note of TemporalConfig.notes(config)) yield* Effect.logInfo(`configuration: ${note}`) + const problems = TemporalConfig.preflight(config) + for (const problem of problems) yield* Effect.logError(`configuration: ${problem}`) + if (problems.length > 0) yield* Effect.die(`this deployment cannot serve sessions: ${problems[0]}`) + const events = yield* EventV2.Service const worktrees = yield* WorktreeMaterializer.Service @@ -99,7 +116,10 @@ const layer = Layer.effect( 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 }) + const l2 = makeL2Drains({ store, locations, ctx, events, worktrees, stepQueue: STEP_QUEUE }) + // What a schedule fires into: admitting a prompt is a row in the store, and a workflow cannot + // write one. Registered on every worker, because a firing lands wherever one is polling. + const schedules = makeScheduleDrains({ db, events }) // Worker connection (native) hosts the runTurnStep activity + the workflow. Skipped in // client-only role so serve can run without an embedded worker. @@ -115,7 +135,7 @@ const layer = Layer.effect( ), ) const nativeConn = yield* Effect.acquireRelease( - Effect.promise(() => NativeConnection.connect({ address: ADDRESS })), + Effect.promise(() => NativeConnection.connect(TemporalConfig.connectionOptions(config))), (conn) => Effect.promise(() => conn.close().catch(() => {})), ) const worker = yield* Effect.promise(() => @@ -124,7 +144,11 @@ const layer = Layer.effect( namespace: NAMESPACE, taskQueue: POLL_QUEUE, workflowsPath: fileURLToPath(new URL("./workflow.ts", import.meta.url)), - activities: { ...makeStepActivities(stepDrain), ...makeSteppedTurnActivities(l2) }, + activities: { + ...makeStepActivities(stepDrain), + ...makeSteppedTurnActivities(l2), + promptSession: schedules.promptDrain, + }, }), ) const runHandle = worker.run() @@ -135,6 +159,33 @@ const layer = Layer.effect( await runHandle.catch(() => {}) }), ) + + // A second poller, on this worker's own queue, for the steps pinned to it. Activities only: + // the workflow runs wherever it was started, and only the work that has to come back here is + // addressed here. Without it a pin has nobody to answer it and every step pays the + // schedule-to-start wait before falling back. + if (STEP_QUEUE) { + const pinnedWorker = yield* Effect.promise(() => + Worker.create({ + connection: nativeConn, + namespace: NAMESPACE, + taskQueue: STEP_QUEUE, + activities: { + ...makeStepActivities(stepDrain), + ...makeSteppedTurnActivities(l2), + promptSession: schedules.promptDrain, + }, + }), + ) + const pinnedHandle = pinnedWorker.run() + pinnedHandle.catch(() => {}) + yield* Effect.addFinalizer(() => + Effect.promise(async () => { + pinnedWorker.shutdown() + await pinnedHandle.catch(() => {}) + }), + ) + } } // Worker-only process: it hosts activities but drives no workflows, so the client methods are @@ -161,7 +212,7 @@ const layer = Layer.effect( // Client connection drives the per-session workflows. const clientConn = yield* Effect.acquireRelease( - Effect.promise(() => Connection.connect({ address: ADDRESS })), + Effect.promise(() => Connection.connect(TemporalConfig.connectionOptions(config))), (conn) => Effect.promise(() => conn.close().catch(() => {})), ) const client = new Client({ connection: clientConn, namespace: NAMESPACE }) @@ -178,6 +229,7 @@ const layer = Layer.effect( startWithWake: true, idleTimeout: IDLE_TIMEOUT, stepped: STEPPED, + serialTools: SERIAL_TOOLS, } satisfies WF.SessionTurnOptions, ], signal: WF.wake, @@ -247,6 +299,7 @@ const layer = Layer.effect( startWithWake: false, idleTimeout: IDLE_TIMEOUT, stepped: STEPPED, + serialTools: SERIAL_TOOLS, } satisfies WF.SessionTurnOptions, ], workflowIdConflictPolicy: "USE_EXISTING", diff --git a/packages/temporal/src/l2-drain.ts b/packages/temporal/src/l2-drain.ts index b61d721eb3b9..82794bce2a6b 100644 --- a/packages/temporal/src/l2-drain.ts +++ b/packages/temporal/src/l2-drain.ts @@ -19,7 +19,10 @@ import type { DeferredToolCall, ToolCallOutcome } from "@opencode-ai/core/sessio 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 { SessionInput } from "@opencode-ai/core/session/input" +import { SessionMessage } from "@opencode-ai/core/session/message" +import { Prompt } from "@opencode-ai/schema/prompt" +import type { Database } from "@opencode-ai/core/database/database" import { runAtBoundary } from "./boundary" import type { StepDrainInput, StepDrainResult } from "./drain" @@ -39,12 +42,19 @@ export type ModelCallDrainResult = /** 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 + /** The queue this worker polls on its own, when it has one. The tools of this step write the + * tree this worker is standing in, so sending them here keeps them on it. Absent when the + * worker was not given a queue of its own, and never required: the step falls back to the + * shared queue and the tree is rebuilt there. */ + readonly queue?: string } export interface ToolCallDrainInput { readonly sessionID: string readonly call: DeferredToolCall readonly owner: string + /** Which step this call belongs to, so a host can tell it from a call an earlier step left. */ + readonly step: number } export interface ToolCallDrainResult { @@ -58,6 +68,10 @@ export interface SealDrainInput { readonly assistantMessageID?: string readonly needsContinuation?: boolean readonly owner: string + /** This step is being closed away from the host that was running it. The tree is not this seal's + * to rebuild or to ship: the host that ran the tools is the only one holding what they did, and + * it may still be inside one of them. Writing the step down is the whole job. */ + readonly withoutTheTree?: boolean } export interface L2DrainDeps { @@ -66,9 +80,52 @@ export interface L2DrainDeps { readonly ctx: Context.Context readonly events: EventV2.Interface readonly worktrees: WorktreeMaterializer.Interface + /** The queue this worker polls on its own, reported by the model call so the rest of the step can + * be sent back to it. Absent when the worker has none. */ + readonly stepQueue?: string } -export const makeL2Drains = ({ store, locations, ctx, events, worktrees }: L2DrainDeps) => { +/** + * Admit a prompt to a session that already exists, without waking anything. + * + * This is what a start with no client is made of. A prompt is a durable row before it is work, and + * writing that row needs the store, which a workflow cannot reach; waking the session is the + * workflow's own job (it starts or signals the session's supervisor). Separating the two is what + * lets a schedule fire into a deployment where nothing is running but workers. + * + * Idempotent on the message id, which the workflow derives from the firing, so a re-driven activity + * admits nothing twice. + */ +export const makeScheduleDrains = ({ + db, + events, +}: { + readonly db: Database.Interface["db"] + readonly events: EventV2.Interface +}) => ({ + promptDrain: async (input: { readonly sessionID: string; readonly messageID: string; readonly text: string }) => + SessionInput.admit(db, events, { + id: SessionMessage.ID.make(input.messageID), + sessionID: SessionSchema.ID.make(input.sessionID), + prompt: Prompt.make({ text: input.text }), + delivery: "queue", + }).pipe( + Effect.asVoid, + // Prompt admission must leave the active drain's ownership unchanged. + Effect.provideService(EventV2.EventOwner, undefined), + Effect.scoped, + Effect.runPromise, + ), +}) + +// A call, as the host records it: enough to tell this step's writers from an earlier step's. +const writer = (input: ToolCallDrainInput) => ({ + sessionID: input.sessionID, + step: input.step, + callID: input.call.id, +}) + +export const makeL2Drains = ({ store, locations, ctx, events, worktrees, stepQueue }: 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 = ( @@ -79,6 +136,11 @@ export const makeL2Drains = ({ store, locations, ctx, events, worktrees }: L2Dra runner: SessionRunner.Interface, session: SessionSchema.Info, ) => Effect.Effect, + current?: { readonly sessionID: string; readonly step: number; readonly callID: string }, + /** Leave the project tree alone. For a seal closing a step away from its host: rebuilding here + * would put this host on the newest state while the one that ran the tools may still be + * writing, and nothing this seal does needs the files. */ + withoutTheTree = false, ) => Effect.gen(function* () { const session = yield* store.get(SessionSchema.ID.make(sessionID)) @@ -87,8 +149,9 @@ export const makeL2Drains = ({ store, locations, ctx, events, worktrees }: L2Dra 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) + // packs, unless a call of an earlier step never came back on this host. + if (!withoutTheTree) + yield* worktrees.ensure(session.location.directory, current ? { current } : undefined) return yield* SessionRunner.Service.use((runner) => use(runner, session)).pipe( Effect.provide(locations.get(session.location)), ) @@ -135,6 +198,7 @@ export const makeL2Drains = ({ store, locations, ctx, events, worktrees }: L2Dra assistantMessageID: result.assistantMessageID, needsContinuation: result.needsContinuation, owner: input.owner, + ...(stepQueue === undefined ? {} : { queue: stepQueue }), }, ), ), @@ -151,19 +215,32 @@ export const makeL2Drains = ({ store, locations, ctx, events, worktrees }: L2Dra 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, + inSession( + input.sessionID, + input.owner, + false, + (runner, session) => + Effect.acquireUseRelease( + // Said before the tool can touch anything, and taken back when its body returns. It is + // the only record on this host of a call still inside its own execution, and what a + // later step reads before it uses this directory: a timeout settles the workflow's + // promise without stopping the process behind it. + worktrees.beginWrite(session.location.directory, writer(input)), + () => + 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, + ), + ), + () => worktrees.endWrite(session.location.directory, input.call.id), ), - ), + writer(input), ).pipe(Effect.map((result) => result ?? { outcome: "already-settled" as const })), ) @@ -171,14 +248,21 @@ export const makeL2Drains = ({ store, locations, ctx, events, worktrees }: L2Dra 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, - }), + 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, + withoutTheTree: input.withoutTheTree, + }), + undefined, + input.withoutTheTree, ).pipe( Effect.map((result) => result === undefined diff --git a/packages/temporal/src/l2-step.ts b/packages/temporal/src/l2-step.ts index 60aaefd27240..61cc8f93bac3 100644 --- a/packages/temporal/src/l2-step.ts +++ b/packages/temporal/src/l2-step.ts @@ -1,18 +1,16 @@ // 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 +// and the seal that closes it. That 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. +// 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. +// each other; what is lost is the overlap between the model and its own tools. -import { ActivityFailure, type ApplicationFailure } from "@temporalio/workflow" +import { ActivityFailure, type ApplicationFailure, TimeoutFailure } from "@temporalio/workflow" import { HALTED_FAILURE_TYPE } from "./protocol" import type { StepDrainInput, StepDrainResult } from "./drain" import type { @@ -36,6 +34,14 @@ export const isHaltFailure = (error: unknown) => error instanceof ActivityFailure && (error.cause as ApplicationFailure | undefined)?.type === HALTED_FAILURE_TYPE +// This permits migration only when pinned dispatches have no automatic retries. +// A later attempt can time out in the queue after an earlier attempt took effect. +export const isUnclaimedFailure = (error: unknown) => + error instanceof ActivityFailure && + error.cause instanceof TimeoutFailure && + error.cause.timeoutType === "SCHEDULE_TO_START" + + /** The three activities a stepped turn drives. */ export interface SteppedActivities { readonly runModelCall: (input: ModelCallDrainInput) => Promise @@ -56,6 +62,28 @@ export interface SteppedTurnDeps { * 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 + /** Run the calls one at a time. Each tool ships the tree from the host that ran it, so two on two + * hosts each publish a tree without the other's work and the second is refused, leaving its work + * stranded there. Serial is what moving files between hosts costs, and it is what pinning a + * step's tools to one worker buys back. */ + readonly serial?: boolean + /** The same activities, addressed to one worker's own queue. A step's tools write the tree the + * model call's worker is standing in, so keeping them there is what lets them run at once: they + * see each other's writes through the filesystem rather than through the store. Only offered a + * queue the model call reported. Only a dispatch that queue never started may move off it. */ + readonly pinnedTo?: (queue: string) => Pick + /** Whether a failure means nobody took the work, which is the one kind a pinned dispatch answers + * by trying the shared queue instead. */ + readonly isUnclaimed?: (error: unknown) => boolean + /** Whether this run was started after a lost host stopped ending the turn. Which activities a + * step schedules is what a workflow writes down, so changing that rule changes histories that + * already exist: a run recorded under the old one failed the turn where this code seals and goes + * on. A run that predates the change answers false here and keeps what it recorded. */ + readonly resumesAfterLostHost?: () => boolean + /** Run something where the driver's cancellation cannot reach it. An interrupt landing during the + * tool phase otherwise leaves the step with no ending published at all, so a follower waiting on + * the turn never hears it stop. */ + readonly nonCancellable?: (fn: () => Promise) => Promise } /** @@ -64,24 +92,143 @@ export interface SteppedTurnDeps { * of a whole-step activity. */ export const makeSteppedTurn = - ({ activities, isCancellation, isHalt, log }: SteppedTurnDeps) => + ({ + activities, + isCancellation, + isHalt, + log, + serial, + nonCancellable, + pinnedTo, + isUnclaimed, + resumesAfterLostHost, + }: 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 + // The worker that made the model call, when it offered its own queue. Everything else in this + // step goes to it first, because it is the host holding the tree the tools are about to write. + const pinned = model.queue && pinnedTo ? pinnedTo(model.queue) : undefined + let unclaimed = false + let uncertain: { error: unknown } | undefined + let stopped: { error: unknown } | undefined + const observeStop = (error: unknown): never => { + if (isCancellation(error) || isHalt(error)) stopped = { error } + throw error + } + // Shared dispatches must wait for the pinned batch because the hosts do not share a worktree. + let shared: Promise = Promise.resolve() + const pendingPins = new Set>() + const onShared = (run: (on: SteppedActivities) => Promise, allowStopped = false): Promise => { + const next = shared.then(async () => { + // Queue saturation can leave a sibling running on the pinned host. + await Promise.allSettled(pendingPins) + if (uncertain) throw uncertain.error + if (stopped && !allowStopped) throw stopped.error + return run(activities).catch(observeStop) + }) + shared = next.then( + () => undefined, + () => undefined, + ) + return next + } + // A timeout settles the workflow promise; it does not stop the tool process behind it. So a + // pinned attempt that started is not evidence that its directory is free, and the rest of the + // step stays off that host until the process is known to have stopped or its workspace is its + // own. Only a dispatch nobody started moves, and only after every pinned sibling has settled. + const viaPinned = async ( + run: (on: Pick) => Promise, + allowStopped = false, + ): Promise => { + if (stopped && !allowStopped) throw stopped.error + if (uncertain) throw uncertain.error + if (!pinned) return run(activities).catch(observeStop) + if (unclaimed) return onShared(run, allowStopped) + const attempt = run(pinned) + pendingPins.add(attempt) + try { + return await attempt + } catch (error) { + if (isCancellation(error) || isHalt(error)) { + stopped = { error } + throw error + } + if (!(isUnclaimed ?? isUnclaimedFailure)(error)) { + uncertain = { error } + throw error + } + unclaimed = true + log?.("the pinned activity did not start; remaining calls move to the shared queue", { + sessionID: input.sessionID, + step: model.step, + }) + return onShared(run, allowStopped) + } finally { + pendingPins.delete(attempt) + } + } + // 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 }), - ), - ) + const dispatch = (call: (typeof model.calls)[number]) => + viaPinned((on) => + // The step travels with the call, because the host tells this step's writers from an + // earlier step's by it. + on.runToolCall({ sessionID: input.sessionID, call, owner: model.owner, step: model.step }), + ) + const dispatched: PromiseSettledResult[] = [] + if (serial) { + // One at a time, and still settled rather than thrown, so a tool that fails does not take the + // rest of the batch with it. The loop keeps going: the seal closes each call and the model + // reacts to what it is told. + for (const call of model.calls) { + dispatched.push( + await dispatch(call).then( + (value) => ({ status: "fulfilled", value }) as const, + (reason) => ({ status: "rejected", reason }) as const, + ), + ) + } + } else { + dispatched.push(...(await Promise.allSettled(model.calls.map(dispatch)))) + } + const sealing = (stopped: boolean): SealDrainInput => ({ + sessionID: input.sessionID, + step: model.step, + // A stopped step is not one that continues. The settlement carries the model's own finish + // reason, and for a step that asked for tools that is `tool-calls`, which every follower + // reads as "another step follows". Passing it through on the way out recorded a turn the + // user stopped as a turn still going. + settlement: + stopped && model.settlement ? { ...model.settlement, finish: "stop" } : model.settlement, + assistantMessageID: model.assistantMessageID, + needsContinuation: stopped ? false : model.needsContinuation, + owner: model.owner, + }) + const seal = (stopped: boolean) => viaPinned((on) => on.sealStep(sealing(stopped)), stopped) + for (const outcome of dispatched) { if (outcome.status !== "rejected") continue - if (isCancellation(outcome.reason) || isHalt(outcome.reason)) throw outcome.reason + if (isCancellation(outcome.reason) || isHalt(outcome.reason)) { + // Seal on the way out, out of reach of the cancellation, so the calls that did return keep + // their results and the step is recorded as ended. Without it a stop landing during the + // tools publishes no step event at all: the interrupt is only visible during the model + // call, and a follower waiting on the turn hangs. + // + // Explicitly with no continuation. Letting the seal decide is what once carried the agent + // on past a declined permission, because it re-derived "keep going" from the tool parts. + // The reason the turn is stopping is rethrown either way, and a seal that fails here must + // not replace it. + await (nonCancellable ?? ((fn: () => Promise) => fn()))(() => seal(true)).catch( + (err) => log?.("could not seal an interrupted step", { step: model.step, error: String(err) }), + ) + throw outcome.reason + } } // A dispatch that settled its call needs no telling. The rest are what an operator is looking @@ -100,12 +247,35 @@ export const makeSteppedTurn = 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, - }) + // A pinned attempt that started and failed used to take the turn with it, because nothing could + // say the tool over there had stopped. Nothing can say that now either, and it no longer has to: + // the log fences a superseded attempt out of the transcript, and the pack store refuses its + // files under the same token, so what that host is still doing cannot reach the session. The + // step is closed on the shared queue, where a worker that is answering can take it, and the + // turn goes on with whatever the seal says follows. What still does not move is the rest of + // this step: its calls stay where their host has them. + const closeElsewhere = () => { + log?.("the step lost its host; closing it elsewhere and carrying the turn on", { + sessionID: input.sessionID, + step: model.step, + }) + // Without the tree. This seal is standing in a directory that never ran the step's tools, so + // what it would ship is the state before them, and the host that did run them may still be + // inside one. Between the dispatch failing and the next step claiming the log there is a + // window where nothing fences that host, and the only thing that makes the window harmless + // is that nobody else publishes during it. + return activities.sealStep({ ...sealing(false), withoutTheTree: true }) + } + const resumes = () => (uncertain !== undefined && (resumesAfterLostHost?.() ?? false)) + if (resumes()) return closeElsewhere() + try { + return await seal(false) + } catch (error) { + // The seal is the other way a step loses its host, and it is the half that has to be written + // down: a step with no ending recorded is one no follower ever hears about. Re-sealing where + // a worker is answering is what an ordinary retry of this activity does; the pinned rule is + // the only reason it did not already happen. + if (stopped || !resumes()) throw error + return await closeElsewhere() + } } diff --git a/packages/temporal/src/queue.ts b/packages/temporal/src/queue.ts index 8e12e33f7047..def81b1e8063 100644 --- a/packages/temporal/src/queue.ts +++ b/packages/temporal/src/queue.ts @@ -43,3 +43,25 @@ export const queueForWorktree = (base: string, directory: string): string => { const digest = createHash("sha256").update(canonical).digest("hex").slice(0, DIGEST_LENGTH) return `${base}-wt-${digest}` } + +/** + * The queue a worker polls on its own, alongside the shared one. + * + * This is what a step is pinned to: the tools of one step write the tree the worker that made the + * model call is standing in, so they are sent back to it rather than to whoever is free. Keyed by + * host as well as directory, unlike the worktree queue above, because two hosts can serve the same + * path without sharing a byte of it: a key on the path alone routes a step to a host whose tree is + * a different tree. + * + * Stable across a restart, so a worker that comes back keeps serving what it served. A pin nobody + * answers can release unstarted dispatches after their schedule-to-start bound, subject to + * the other pinned attempts' outcomes. + */ +export const queueForWorker = (base: string, host: string, directory: string): string => { + const canonical = resolve(directory).replace(/[/\\]+$/, "") + const digest = createHash("sha256") + .update(`${host}\0${canonical}`) + .digest("hex") + .slice(0, DIGEST_LENGTH) + return `${base}-w-${digest}` +} diff --git a/packages/core/src/session/execution/run-error-codec.ts b/packages/temporal/src/run-error-codec.ts similarity index 79% rename from packages/core/src/session/execution/run-error-codec.ts rename to packages/temporal/src/run-error-codec.ts index 472a0eba90ba..ed160bc446d3 100644 --- a/packages/core/src/session/execution/run-error-codec.ts +++ b/packages/temporal/src/run-error-codec.ts @@ -1,21 +1,17 @@ -// Faithful round-trip of a SessionRunner.RunError across the Temporal boundary. Every member of the -// union is a Schema.TaggedErrorClass, so we can encode the error to JSON in the activity and decode -// it back into the exact tagged instance in the layer, instead of flattening it to a carrier. - import { Schema } from "effect" import { LLMError } from "@opencode-ai/llm" -import { Integration } from "../../integration" -import { SystemContext } from "../../system-context/index" -import { ToolOutputStore } from "../../tool-output-store" -import type { SessionSchema } from "../schema" -import { ContextSnapshotDecodeError, MessageDecodeError, SessionRunDeclinedError } from "../error" +import { Integration } from "@opencode-ai/core/integration" +import { SystemContext } from "@opencode-ai/core/system-context" +import { ToolOutputStore } from "@opencode-ai/core/tool-output-store" +import type { SessionSchema } from "@opencode-ai/core/session/schema" +import { ContextSnapshotDecodeError, MessageDecodeError, SessionRunDeclinedError } from "@opencode-ai/core/session/error" import { ModelNotSelectedError, ModelUnavailableError, UnsupportedApiError, VariantUnavailableError, -} from "../runner/model" -import type { SessionRunner } from "../runner" +} from "@opencode-ai/core/session/runner/model" +import type { SessionRunner } from "@opencode-ai/core/session/runner" const RunErrorSchema = Schema.Union([ LLMError, diff --git a/packages/temporal/src/supervisor.ts b/packages/temporal/src/supervisor.ts index d279dcfc5164..86b47454dc5b 100644 --- a/packages/temporal/src/supervisor.ts +++ b/packages/temporal/src/supervisor.ts @@ -45,6 +45,18 @@ export interface SupervisorRuntime { /** Restart the run with fresh history, carrying whether work is still pending. History-keeping * drivers only (Temporal). */ readonly continueAsNew?: (sessionID: string, startWithWake: boolean) => Promise + /** Whether the driver says this run's history is large enough to roll over. A drain count cannot + * answer this: one drain is a whole turn, and a stepped turn of 200 steps is thousands of events, + * so a handful of drains can cross the server's limit long before the count does. Optional: + * drivers without a history return false. */ + readonly historyWantsRollover?: () => boolean + /** Whether this run was started after a turn got a step ceiling. A turn that keeps stepping is + * what the ceiling is for, and a run recorded before it exists would replay into a step this + * code refuses to schedule, so only a run that recorded the change may take it. Optional: + * drivers with no history to replay always have it. */ + readonly boundsStepsPerTurn?: () => boolean + /** Where a turn says it stopped because it ran out of steps rather than because it finished. */ + readonly warn?: (message: string, attributes: Record) => void } export interface WorkflowOptions { @@ -52,6 +64,8 @@ export interface WorkflowOptions { readonly idleTimeout?: string /** Drains per run before continue-as-new, when the driver supports it. */ readonly maxDrainsPerRun?: number + /** Steps one turn may take before the supervisor stops driving it. */ + readonly maxStepsPerTurn?: number } export const makeSupervisor = (rt: SupervisorRuntime, options?: WorkflowOptions) => { @@ -60,6 +74,11 @@ export const makeSupervisor = (rt: SupervisorRuntime, options?: WorkflowOptions) // until Temporal terminates the workflow. continue-as-new carries the pending-wake state, so no // queued work is lost across the boundary. const MAX_DRAINS_PER_RUN = options?.maxDrainsPerRun ?? 30 + // A turn that never stops stepping is a bug in the loop above this one: a model asking for the + // same tool forever, or a step that keeps handing itself back because its host keeps dying. Only + // the supervisor can see it, because each step is its own activity and each one succeeds. High + // enough that real work never reaches it. + const MAX_STEPS_PER_TURN = options?.maxStepsPerTurn ?? 200 // Each step (one provider attempt + its tools) is its own activity; the step loop is supervisor // control flow (step / promotion / first mirror SessionRunner.run's loop). `startWithWake` is the @@ -81,12 +100,31 @@ export const makeSupervisor = (rt: SupervisorRuntime, options?: WorkflowOptions) .runInDrainScope(async () => { drains++ if (drains >= MAX_DRAINS_PER_RUN) rolloverPending = true + if (rt.historyWantsRollover?.()) rolloverPending = true let step = 1 let promotion: string | null = null let first = true - for (;;) { + for (let taken = 0; ; taken++) { + if (taken >= MAX_STEPS_PER_TURN && (rt.boundsStepsPerTurn?.() ?? true)) { + rt.warn?.("turn hit the step ceiling and was left where it stopped", { + sessionID, + steps: taken, + }) + break + } const r: StepDrainResult = await rt.runTurnStep({ sessionID, step, promotion, first, force }) + // Inside the loop as well, because one drain is a whole turn: a long one outgrows the + // history without ever reaching the next drain's check. + if (rt.historyWantsRollover?.()) rolloverPending = true if (!r.continue) break + // A queued prompt continues this same drain as a fresh turn, so a session fed without a + // gap never goes quiet and the rollover it is waiting for never happens. Stop at that + // boundary instead and let the new run pick the queue up: the work is not lost, it is + // one turn later. A steer is not a boundary, so it still rides this drain through. + if (rolloverPending && r.promotion === "queue") { + pendingWake = true + break + } step = r.step promotion = r.promotion first = false diff --git a/packages/temporal/src/workflow.ts b/packages/temporal/src/workflow.ts index a395a39dd674..50a837a4cf9b 100644 --- a/packages/temporal/src/workflow.ts +++ b/packages/temporal/src/workflow.ts @@ -8,6 +8,7 @@ // `@opencode-ai/core` runtime imports, no Node builtins. import { + patched, proxyActivities, defineSignal, defineUpdate, @@ -18,21 +19,20 @@ import { CancellationScope, isCancellation, allHandlersFinished, + workflowInfo, log, + startChild, + getExternalWorkflowHandle, + ParentClosePolicy, } from "@temporalio/workflow" +import { WorkflowExecutionAlreadyStartedError } from "@temporalio/common" import type { StepActivities, SteppedTurnActivities } from "./activities" -import { isHaltFailure, makeSteppedTurn } from "./l2-step" -import { SIGNALS, RESUME_UPDATE } from "./protocol" +import { isHaltFailure, isUnclaimedFailure, makeSteppedTurn } from "./l2-step" +import { SIGNALS, RESUME_UPDATE, WORKFLOW_ID_PREFIX } from "./protocol" import { makeSupervisor, type SupervisorRuntime } from "./supervisor" 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 - // legitimate turns and each kill opened a short two-writer window until the zombie attempt - // noticed its heartbeat rejection. + // Human approvals can outlast a normal turn. Heartbeat expiry does not terminate the body. startToCloseTimeout: "12 hours", heartbeatTimeout: "10 seconds", retry: { maximumAttempts: 100 }, @@ -46,9 +46,38 @@ const { runTurnStep } = proxyActivities(activityOptions) 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", +const sealOptions = { ...activityOptions, startToCloseTimeout: "10 minutes" } as const +const { sealStep } = proxyActivities(sealOptions) + +// A private queue can stop polling or run out of slots. Bound the wait before unstarted work moves. +const PINNED_SCHEDULE_TO_START = "30 seconds" + +/** The same two activities, addressed to one worker's own queue. Built per queue rather than once, + * because the queue is not known until the model call reports it; that report comes out of history, + * so this is deterministic on replay. */ +const pinnedTo = (taskQueue: string) => ({ + runToolCall: proxyActivities({ + ...activityOptions, + retry: { maximumAttempts: 1 }, + taskQueue, + scheduleToStartTimeout: PINNED_SCHEDULE_TO_START, + }).runToolCall, + sealStep: proxyActivities({ + ...sealOptions, + retry: { maximumAttempts: 1 }, + taskQueue, + scheduleToStartTimeout: PINNED_SCHEDULE_TO_START, + }).sealStep, +}) + +// Admitting a prompt is a row in the store, so it is an activity; it is small and it must not hold +// a firing open if no worker is polling. +const { promptSession } = proxyActivities<{ + promptSession(input: { sessionID: string; messageID: string; text: string }): Promise +}>({ + startToCloseTimeout: "2 minutes", + scheduleToCloseTimeout: "30 minutes", + retry: { maximumAttempts: 10 }, }) export const wake = defineSignal(SIGNALS.wake) @@ -107,22 +136,34 @@ const runtime: SupervisorRuntime = { // root's consideredCancelled). isRootCancelled: () => rootScope?.consideredCancelled ?? false, allHandlersFinished, - continueAsNew: (sessionID, startWithWake) => - continueAsNew(sessionID, { startWithWake }), + continueAsNew: (sessionID, startWithWake) => continueAsNew(sessionID, { startWithWake }), + // The server's own read of whether this run has grown enough to roll over. The drain count alone + // misses it: a stepped turn is thousands of events, so a handful of drains can cross the limit. + historyWantsRollover: () => workflowInfo().continueAsNewSuggested, + // False only while replaying a history written before the ceiling existed. See the runtime field. + boundsStepsPerTurn: () => patched("a-turn-has-a-step-ceiling"), + warn: (message, attributes) => log.warn(message, attributes), } // 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 = { +// unchanged, and only what "one step" means differs. Built per run rather than once, because +// whether a step's tools may overlap rides the workflow input: the sandbox cannot read env. +const steppedRuntime = (serial: boolean): SupervisorRuntime => ({ ...runtime, runTurnStep: makeSteppedTurn({ activities: { runModelCall, runToolCall, sealStep }, isCancellation, isHalt: isHaltFailure, + isUnclaimed: isUnclaimedFailure, + // False only while replaying a history written before this rule existed. See the dep. + resumesAfterLostHost: () => patched("lost-host-does-not-end-the-turn"), + pinnedTo, + serial, + nonCancellable: (fn) => CancellationScope.nonCancellable(fn), // 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 @@ -141,6 +182,10 @@ export interface SessionTurnOptions { /** 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 + /** Run a step's tool calls one at a time. Each ships the tree from the host that ran it, so two + * on two hosts each publish a tree without the other's work. The client decides, because only it + * can read whether the store is shared. */ + readonly serialTools?: boolean } export async function sessionTurn(sessionID: string, options?: SessionTurnOptions): Promise { @@ -148,15 +193,56 @@ export async function sessionTurn(sessionID: string, options?: SessionTurnOption const startWithWake = options?.startWithWake ?? true const idleTimeout = options?.idleTimeout const stepped = options?.stepped === true + const serialTools = options?.serialTools === true if (!idleTimeout && !stepped) return workflows.sessionTurn(sessionID, startWithWake) return makeSupervisor( { - ...(stepped ? steppedRuntime : runtime), + ...(stepped ? steppedRuntime(serialTools) : 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 }), + continueAsNew(id, { + startWithWake: wake, + idleTimeout, + stepped, + serialTools, + }), }, idleTimeout ? { idleTimeout } : undefined, ).sessionTurn(sessionID, startWithWake) } + +/** + * A turn nobody started. + * + * A schedule fires this, and it runs where no client and no serve process exist: the prompt is + * admitted by an activity, because it is a row in the store, and the session's own supervisor is + * started as an abandoned child (or signalled, when it is already running). Nothing here waits for + * the turn: this workflow's job is to hand the work over and finish, which is what makes a firing + * cheap and a missed one visible in the schedule rather than in a run that never ends. + * + * The message id comes from the firing's own workflow id, so a re-drive admits the same prompt + * rather than a second one. + */ +export async function scheduledPrompt(input: { + readonly sessionID: string + readonly text: string + readonly session?: SessionTurnOptions +}): Promise { + const messageID = `msg_sched_${workflowInfo().workflowId}` + await promptSession({ sessionID: input.sessionID, messageID, text: input.text }) + const options: SessionTurnOptions = { ...input.session, startWithWake: true } + try { + await startChild(sessionTurn, { + workflowId: `${WORKFLOW_ID_PREFIX}${input.sessionID}`, + args: [input.sessionID, options], + parentClosePolicy: ParentClosePolicy.ABANDON, + }) + } catch (error) { + // The session is already being driven, which is the ordinary case for a schedule that fires + // faster than a turn takes. The prompt is admitted either way; what it needs is a wake, because + // a supervisor waiting out its idle period is not watching the store. + if (!(error instanceof WorkflowExecutionAlreadyStartedError)) throw error + await getExternalWorkflowHandle(`${WORKFLOW_ID_PREFIX}${input.sessionID}`).signal(wake) + } +} diff --git a/packages/temporal/test/boundary-refusal.test.ts b/packages/temporal/test/boundary-refusal.test.ts new file mode 100644 index 000000000000..99c524a26c17 --- /dev/null +++ b/packages/temporal/test/boundary-refusal.test.ts @@ -0,0 +1,42 @@ +// What a refused directory costs the session is decided at the activity boundary, not in the +// materializer that raises it. +// +// Two things have to be true of it. It has to be retryable, or a host holding somebody's abandoned +// tool ends the turn for every host: the refusal is this host saying no, and the same dispatch runs +// fine on a host that is not holding one. And it has to carry its own retry delay, because the +// backoff a failing activity earns doubles per attempt, and the host that answers first and refuses +// fastest is exactly the one that would push the next attempt minutes out while a free host idles. + +import { expect, it } from "bun:test" +import { Effect } from "effect" +import { ApplicationFailure } from "@temporalio/common" +import { WorktreeMaterializer } from "@opencode-ai/core/session/execution/worktree" +import { runAtBoundary } from "../src/boundary" + +const crossing = (body: Effect.Effect) => + runAtBoundary("ses_refused", new AbortController().signal, body).then( + () => undefined, + (err: unknown) => err as ApplicationFailure, + ) + +it("hands a refused directory back as retryable work with its own delay", async () => { + const refused = await crossing( + Effect.die( + new WorktreeMaterializer.WorktreeQuarantinedError({ + message: "not using /project: a call of an earlier step never returned", + }), + ), + ) + expect(refused).toBeInstanceOf(ApplicationFailure) + expect(refused?.type).toBe("WorktreeMaterializer.QuarantinedError") + expect(refused?.nonRetryable).toBeFalsy() + expect(refused?.nextRetryDelay).toBeDefined() +}) + +it("leaves an ordinary run error non-retryable and on the backoff", async () => { + // The contrast that makes the above mean something: re-running a step whose input the model has + // already answered is worse than failing it, so everything that is about the work stays as it was. + const failed = await crossing(Effect.die(new Error("the tool blew up"))) + expect(failed?.nonRetryable).toBe(true) + expect(failed?.nextRetryDelay).toBeUndefined() +}) diff --git a/packages/temporal/test/fixture/histories/lost-host-ends-turn.bin b/packages/temporal/test/fixture/histories/lost-host-ends-turn.bin new file mode 100644 index 000000000000..d3a9b0d0f44f Binary files /dev/null and b/packages/temporal/test/fixture/histories/lost-host-ends-turn.bin differ diff --git a/packages/temporal/test/l2-drain-writers.test.ts b/packages/temporal/test/l2-drain-writers.test.ts new file mode 100644 index 000000000000..3cf4a8fafcbd --- /dev/null +++ b/packages/temporal/test/l2-drain-writers.test.ts @@ -0,0 +1,172 @@ +// The marker a tool call writes is the only record on a host of a call still inside its own +// execution, and every refusal the worktree materializer makes is built on it being there. The store +// side has its own tests; this one drives the real drain, because the three lines that bracket the +// tool body are what put a marker on the host at all. +// +// Take the bracket out of `toolCallDrain` and the first assertion fails: the tool runs with nothing +// on the host saying it is inside, and a later step is free to take the directory. + +import { expect } from "bun:test" +import { mkdtemp } from "node:fs/promises" +import { tmpdir } from "node:os" +import path from "path" +import { Context, Effect, Layer } from "effect" +import { Database } from "@opencode-ai/core/database/database" +import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" +import { Global } from "@opencode-ai/core/global" +import { WorktreeMaterializer } from "@opencode-ai/core/session/execution/worktree" +import { SessionRunner } from "@opencode-ai/core/session/runner" +import * as Writers from "@opencode-ai/core/snapshot/writers" +import { testEffect } from "../../core/test/lib/effect" +import { makeL2Drains, type L2DrainDeps } from "../src/l2-drain" + +const it = testEffect(Layer.empty) + +const materializerStack = (data: string) => + AppNodeBuilder.build(WorktreeMaterializer.node, [ + [Database.node, Database.layerFromPath(":memory:")], + [Global.node, Layer.succeed(Global.Service, Global.make({ data }))], + ]) + +/** A drain over a runner that reports what the host said about it while it was running. */ +const drainsOver = ( + worktrees: WorktreeMaterializer.Interface, + directory: string, + runner: Partial, +) => + makeL2Drains({ + store: { + get: () => Effect.succeed({ id: "ses_writers", location: { directory } }), + } as unknown as L2DrainDeps["store"], + // The location resolves to a runner and nothing else: what is under test is the bracket around + // the call, not what the tool does inside it. + locations: { + get: () => Layer.succeed(SessionRunner.Service, runner as SessionRunner.Interface), + } as unknown as L2DrainDeps["locations"], + ctx: Context.empty() as L2DrainDeps["ctx"], + events: { claim: () => Effect.void } as unknown as L2DrainDeps["events"], + worktrees, + }) + +it.live("marks the directory while a tool call runs and takes the mark back after it", () => + Effect.gen(function* () { + const root = yield* Effect.promise(() => mkdtemp(path.join(tmpdir(), "opencode-l2-writers-"))) + const data = path.join(root, "host-data") + const directory = path.join(root, "project") + const worktrees = yield* WorktreeMaterializer.Service.pipe( + Effect.provide(yield* Layer.build(materializerStack(data))), + ) + + const seen: Writers.Writer[][] = [] + const { toolCallDrain } = drainsOver(worktrees, directory, { runToolCall: ((input: { call: { id: string } }) => + Effect.gen(function* () { + // Inside the body, which is the window the refusal exists for. Asked with no step of its + // own, so what comes back is every marker the host is holding. + seen.push(yield* Writers.strandedWriters(data, directory)) + return { outcome: "settled" as const, call: input.call.id } + })) as unknown as SessionRunner.Interface["runToolCall"] }) + + yield* Effect.promise(() => + toolCallDrain( + { + sessionID: "ses_writers", + call: { id: "call_one", name: "write", assistantMessageID: "msg_one" }, + owner: "run:1:1", + step: 3, + }, + new AbortController().signal, + ), + ) + + expect(seen).toHaveLength(1) + expect(seen[0].map((w) => ({ session: w.sessionID, step: w.step, call: w.callID }))).toEqual([ + { session: "ses_writers", step: 3, call: "call_one" }, + ]) + // And the mark is gone once the body returned, or every later step would be refused a directory + // nothing is writing. + expect(yield* Writers.strandedWriters(data, directory)).toEqual([]) + }), +) + +it.live("takes the mark back when the tool fails rather than returns", () => + Effect.gen(function* () { + const root = yield* Effect.promise(() => mkdtemp(path.join(tmpdir(), "opencode-l2-writers-"))) + const data = path.join(root, "host-data") + const directory = path.join(root, "project") + const worktrees = yield* WorktreeMaterializer.Service.pipe( + Effect.provide(yield* Layer.build(materializerStack(data))), + ) + + const { toolCallDrain } = drainsOver(worktrees, directory, { + runToolCall: (() => + Effect.die(new Error("the tool blew up"))) as unknown as SessionRunner.Interface["runToolCall"], + }) + + const failed = yield* Effect.promise(() => + toolCallDrain( + { + sessionID: "ses_writers", + call: { id: "call_two", name: "write", assistantMessageID: "msg_two" }, + owner: "run:1:1", + step: 4, + }, + new AbortController().signal, + ).then( + () => undefined, + (err: unknown) => err, + ), + ) + expect(String(failed)).toContain("the tool blew up") + // A call that failed is not a call still inside its own execution. Leaving the mark would refuse + // the directory to everything after it for a tool that is over. + expect(yield* Writers.strandedWriters(data, directory)).toEqual([]) + }), +) + +it.live("seals a step away from its host without touching the tree", () => + Effect.gen(function* () { + const root = yield* Effect.promise(() => mkdtemp(path.join(tmpdir(), "opencode-l2-writers-"))) + const data = path.join(root, "host-data") + const directory = path.join(root, "project") + const real = yield* WorktreeMaterializer.Service.pipe( + Effect.provide(yield* Layer.build(materializerStack(data))), + ) + // The rebuild is the part that must not happen, and with no packs stored it would return + // without doing anything, so what is counted is the call rather than its effect. + let rebuilds = 0 + const worktrees: WorktreeMaterializer.Interface = { + ...real, + ensure: (dir, options) => { + rebuilds++ + return real.ensure(dir, options) + }, + } + + const sealed: unknown[] = [] + const { sealDrain } = drainsOver(worktrees, directory, { + sealStep: ((input: unknown) => { + sealed.push(input) + return Effect.succeed({ ran: true, continue: true, step: 2, promotion: undefined }) + }) as unknown as SessionRunner.Interface["sealStep"], + }) + const seal = (withoutTheTree?: boolean) => + Effect.promise(() => + sealDrain( + { sessionID: "ses_writers", step: 1, owner: "run:1:1", withoutTheTree }, + new AbortController().signal, + ), + ) + + // An ordinary seal is on the host that ran the step, and it ships what the step produced. + yield* seal() + expect(rebuilds).toBe(1) + expect((sealed[0] as { withoutTheTree?: boolean }).withoutTheTree).toBeUndefined() + + // One closing a step away from that host is not. Rebuilding here would put this host on the + // newest state while the one that ran the tools may still be writing, and what it captured + // would be the state before them. + yield* seal(true) + expect(rebuilds).toBe(1) + expect((sealed[1] as { withoutTheTree?: boolean }).withoutTheTree).toBe(true) + }), +) diff --git a/packages/temporal/test/l2-pinned-retry.test.ts b/packages/temporal/test/l2-pinned-retry.test.ts new file mode 100644 index 000000000000..26bcbd45e1f5 --- /dev/null +++ b/packages/temporal/test/l2-pinned-retry.test.ts @@ -0,0 +1,105 @@ +import { expect, it } from "bun:test" +import { fileURLToPath } from "node:url" +import { ApplicationFailure } from "@temporalio/common" +import { TestWorkflowEnvironment } from "@temporalio/testing" +import { Worker } from "@temporalio/worker" + +// The server must not retry a pinned body or move it while its physical state is unknown. What the +// step does instead is close itself where a worker is answering: the call stays with the host that +// has it, and the turn goes on rather than ending with that host. +it("gives a pinned dispatch one attempt, keeps its call, and closes the step elsewhere", async () => { + const env = await TestWorkflowEnvironment.createLocal() + let phase: "tool" | "seal" = "tool" + const attempts = { tool: 0, seal: 0 } + const shared = { tool: 0, seal: 0 } + const sealed: Array = [] + try { + const worker = await Worker.create({ + connection: env.nativeConnection, + namespace: env.namespace, + taskQueue: "pin-retry-main", + workflowsPath: fileURLToPath(new URL("../src/workflow.ts", import.meta.url)), + activities: { + runModelCall: async () => ({ + kind: "called", + step: 1, + calls: phase === "tool" ? [{ id: "call_write", name: "write", assistantMessageID: "msg_write" }] : [], + owner: "run:1:1", + queue: "pin-retry-tools", + }), + runToolCall: async () => { + shared.tool++ + return { outcome: "settled" } + }, + sealStep: async (input: { withoutTheTree?: boolean }) => { + shared.seal++ + sealed.push(input.withoutTheTree === true) + return { ran: true, continue: false, step: 1, promotion: null } + }, + }, + }) + const pinned = await Worker.create({ + connection: env.nativeConnection, + namespace: env.namespace, + taskQueue: "pin-retry-tools", + activities: { + runToolCall: async () => { + attempts.tool++ + throw ApplicationFailure.create({ message: "tool failed after dispatch", type: "ToolUnavailable" }) + }, + sealStep: async () => { + attempts.seal++ + throw ApplicationFailure.create({ message: "seal failed after dispatch", type: "SealUnavailable" }) + }, + }, + }) + await worker.runUntil(() => + pinned.runUntil(async () => { + for (const kind of ["tool", "seal"] as const) { + phase = kind + shared.tool = 0 + shared.seal = 0 + sealed.length = 0 + const handle = await env.client.workflow.start("sessionTurn", { + workflowId: `pin-retry-session-${kind}`, + taskQueue: "pin-retry-main", + args: ["ses_pin_retry", { stepped: true, startWithWake: false }], + }) + const resumed = handle.executeUpdate("resume").then( + () => "completed", + () => "failed", + ) + let timer: ReturnType | undefined + try { + const outcome = await Promise.race([ + resumed, + new Promise((resolve) => { + timer = setTimeout(() => resolve("waiting for retries"), 2_000) + }), + ]) + // The turn is handed back, not ended: the step was closed on the shared queue and the + // supervisor gets a result to carry on from. + expect(outcome).toBe("completed") + // One attempt on the pinned queue, and never a second one anywhere: a retry's queue + // timeout cannot rule out the first attempt still running. + expect(attempts[kind]).toBe(1) + // The call itself does not move. Only the seal does, and that is the step saying what + // it had rather than the tool being run somewhere else. + expect(shared.tool).toBe(0) + expect(shared.seal).toBe(1) + // And it seals without the tree: this worker never ran the step, and the one that did + // may still be inside a tool, so rebuilding here would put it on the newest state and + // shipping from here would publish the state before the step. + expect(sealed).toEqual([true]) + } finally { + clearTimeout(timer) + await handle.terminate() + await resumed + } + } + }), + ) + } finally { + await env.teardown() + } +}, 120_000) diff --git a/packages/temporal/test/l2-replay.test.ts b/packages/temporal/test/l2-replay.test.ts new file mode 100644 index 000000000000..104b84ec0408 --- /dev/null +++ b/packages/temporal/test/l2-replay.test.ts @@ -0,0 +1,108 @@ +// Whether a session that is already running can be served by a worker carrying this code. +// +// What a stepped turn does after a pinned dispatch fails is a workflow decision, so it is written +// into every history that hits it: a run recorded before the rule failed the turn where this code +// seals it somewhere else and carries on. Replaying one of those against this code is a +// nondeterminism error unless the rule is behind a patch. It is, so the old runs keep the behaviour +// they recorded and nothing has to be drained before a deploy. +// +// Two directions, because a patch nothing replays through is a patch nobody knows is wired up: a +// history this code writes, and the kept ones under `fixture/histories`. Record a new fixture by +// reverting the rule, running this file with `RECORD_HISTORY=`, and keeping the file it +// writes. They are the server's own wire form, because a fetched history does not survive a trip +// through proto3 JSON. + +import { expect, it } from "bun:test" +import { readdir, readFile, writeFile } from "node:fs/promises" +import path from "path" +import { fileURLToPath } from "node:url" +import { ApplicationFailure } from "@temporalio/common" +import { temporal } from "@temporalio/proto" +import { TestWorkflowEnvironment } from "@temporalio/testing" +import { Worker } from "@temporalio/worker" + +const workflowsPath = fileURLToPath(new URL("../src/workflow.ts", import.meta.url)) +const histories = fileURLToPath(new URL("./fixture/histories", import.meta.url)) + +it("replays a stepped turn that lost its host, its own and the kept ones", async () => { + const env = await TestWorkflowEnvironment.createLocal() + try { + let called = false + const worker = await Worker.create({ + connection: env.nativeConnection, + namespace: env.namespace, + taskQueue: "replay-main", + workflowsPath, + activities: { + runModelCall: async () => { + // One step with a tool, then nothing left to do: the turn has to be able to end after the + // step that lost its host, or the replay would only ever see the failure. + if (called) return { kind: "settled", result: { ran: true, continue: false, step: 2, promotion: null } } + called = true + return { + kind: "called", + step: 1, + calls: [{ id: "call_lost", name: "write", assistantMessageID: "msg_lost" }], + owner: "run:1:1", + queue: "replay-tools", + } + }, + runToolCall: async () => ({ outcome: "settled" }), + sealStep: async () => ({ ran: true, continue: true, step: 2, promotion: null }), + }, + }) + const pinned = await Worker.create({ + connection: env.nativeConnection, + namespace: env.namespace, + taskQueue: "replay-tools", + activities: { + runToolCall: async () => { + throw ApplicationFailure.create({ message: "the tool failed after starting", type: "ToolUnavailable" }) + }, + sealStep: async () => ({ ran: true, continue: false, step: 2, promotion: null }), + }, + }) + + const history = await worker.runUntil(() => + pinned.runUntil(async () => { + const handle = await env.client.workflow.start("sessionTurn", { + workflowId: "replay-session", + taskQueue: "replay-main", + args: ["ses_replay", { stepped: true, startWithWake: false }], + }) + // Recording runs against the code that predates the rule, where this update fails, and the + // history is what the run is for either way. + const resumed = await handle.executeUpdate("resume").then( + () => "completed", + () => "failed", + ) + if (!process.env.RECORD_HISTORY) expect(resumed).toBe("completed") + await handle.terminate().catch(() => undefined) + return handle.fetchHistory() + }), + ) + + const record = process.env.RECORD_HISTORY + if (record) { + // The wire form rather than JSON: a fetched history holds payloads the proto3 JSON converter + // will not take, and a fixture that has been through a lossy encoding is not the history the + // server wrote. + const encoded = temporal.api.history.v1.History.encode(history).finish() + await writeFile(path.join(histories, `${record}.bin`), Buffer.from(encoded)) + console.log(`recorded ${record}.bin`) + } + + // The same code replaying its own history is the case that must always work. + await Worker.runReplayHistory({ workflowsPath }, history) + + // And the kept ones, each written by the code that predates a rule this one changed. + const kept = (await readdir(histories).catch(() => [] as string[])).filter((name) => name.endsWith(".bin")) + expect(kept.length).toBeGreaterThan(0) + for (const name of kept) { + const older = temporal.api.history.v1.History.decode(await readFile(path.join(histories, name))) + await Worker.runReplayHistory({ workflowsPath }, older) + } + } finally { + await env.teardown() + } +}, 120_000) diff --git a/packages/temporal/test/l2-step.test.ts b/packages/temporal/test/l2-step.test.ts index f92ad5abddd4..72f2aa108ecb 100644 --- a/packages/temporal/test/l2-step.test.ts +++ b/packages/temporal/test/l2-step.test.ts @@ -3,7 +3,7 @@ // 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 { isHaltFailure, isUnclaimedFailure, 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" @@ -35,12 +35,7 @@ const INPUT: StepDrainInput = { 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 call = (id: string, name = "probe_write") => ({ id, name, assistantMessageID: "msg_1" }) const fakes = ( model: ModelCallDrainResult, @@ -150,7 +145,47 @@ describe("stepped turn", () => { expect(result).toEqual(SEALED) }) - it("lets an interrupt end the turn instead of sealing it", async () => { + // Each tool ships the project tree from the host that ran it, so two on two hosts each publish a + // tree without the other's work. Serial is what moving files between hosts costs. + it("runs a step's tools one at a time when told to", async () => { + let inFlight = 0 + let overlapped = false + const { activities, tools } = fakes( + { kind: "called", step: 2, calls: [call("a"), call("b"), call("c")], owner: "own" }, + async () => { + inFlight += 1 + if (inFlight > 1) overlapped = true + await new Promise((resolve) => setTimeout(resolve, 5)) + inFlight -= 1 + return { outcome: "settled" } + }, + ) + + await makeSteppedTurn({ activities, isCancellation, isHalt, serial: true })(INPUT) + expect(tools).toHaveLength(3) + expect(overlapped).toBe(false) + }) + + // And still overlap when nothing is moving, which is the case the split was measured on. + it("runs them together when it is not", async () => { + let inFlight = 0 + let overlapped = false + const { activities } = fakes( + { kind: "called", step: 2, calls: [call("a"), call("b"), call("c")], owner: "own" }, + async () => { + inFlight += 1 + if (inFlight > 1) overlapped = true + await new Promise((resolve) => setTimeout(resolve, 5)) + inFlight -= 1 + return { outcome: "settled" } + }, + ) + + await makeSteppedTurn({ activities, isCancellation, isHalt })(INPUT) + expect(overlapped).toBe(true) + }) + + it("closes an interrupted step without letting it ask for another", async () => { const { activities, seals } = fakes( { kind: "called", step: 2, calls: [call("call_a")], owner: "own" }, async () => { @@ -160,12 +195,15 @@ describe("stepped turn", () => { const run = makeSteppedTurn({ activities, isCancellation, isHalt })(INPUT) - // A cancellation is not a failed tool. Swallowing it would close a step the user stopped. + // A cancellation is not a failed tool, so it still ends the turn. The step is closed on the way + // out all the same: a stop landing here used to publish no step event at all, and a follower + // waiting on the turn hung. What the seal must not do is decide the turn keeps going. await expect(run).rejects.toBeInstanceOf(FakeCancel) - expect(seals).toHaveLength(0) + expect(seals).toHaveLength(1) + expect(seals[0]?.needsContinuation).toBe(false) }) - it("lets a user halt end the turn instead of sealing it", async () => { + it("closes a halted step without carrying on past the refusal", async () => { const { activities, seals } = fakes( { kind: "called", step: 2, calls: [call("call_a")], owner: "own" }, async () => { @@ -175,12 +213,330 @@ describe("stepped turn", () => { 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. + // A decline crosses the activity boundary as an ordinary failure, not a cancel. The halt is + // still what ends the turn, and the seal is told not to continue, which is what once let the + // agent run on past the user's refusal. await expect(run).rejects.toBeInstanceOf(FakeHalt) + expect(seals).toHaveLength(1) + expect(seals[0]?.needsContinuation).toBe(false) + }) +}) + +// Pinning a step to the worker that made its model call, and what happens when that worker is gone. +// The pin is what lets a step's tools run at once: they write one tree through one filesystem +// instead of shipping it to each other. The fallback is what keeps that from being a worse kind of +// stuck than the shared queue was. +describe("stepped turn, pinned to a worker", () => { + const unclaimed = () => + new ActivityFailure( + "activity failed", + "runToolCall", + "1", + 1 as never, + undefined, + new TimeoutFailure("schedule to start timed out", undefined, "SCHEDULE_TO_START" as never), + ) + const isUnclaimed = (error: unknown) => + error instanceof ActivityFailure && + error.cause instanceof TimeoutFailure && + error.cause.timeoutType === "SCHEDULE_TO_START" + + const called = (model: ModelCallDrainResult) => { + const shared = fakes(model) + const pinnedTools: ToolCallDrainInput[] = [] + const pinnedSeals: SealDrainInput[] = [] + let refuse = false + let refused = 0 + const pinned = { + runToolCall: async (input: ToolCallDrainInput): Promise => { + if (refuse) { + refused++ + throw unclaimed() + } + pinnedTools.push(input) + return { outcome: "settled" } + }, + sealStep: async (input: SealDrainInput): Promise => { + if (refuse) { + refused++ + throw unclaimed() + } + pinnedSeals.push(input) + return SEALED + }, + } + return { + ...shared, + pinnedTools, + pinnedSeals, + refusals: () => refused, + goneAfterModelCall: () => { + refuse = true + }, + run: () => + makeSteppedTurn({ + activities: shared.activities, + isCancellation, + isHalt, + isUnclaimed, + pinnedTo: (queue) => { + expect(queue).toBe("queue-of-the-worker") + return pinned + }, + })(INPUT), + } + } + + const withQueue: ModelCallDrainResult = { + kind: "called", + step: 2, + calls: [call("call_a"), call("call_b")], + owner: "run:1:1", + queue: "queue-of-the-worker", + } + + it("sends the tools and the seal back to the worker that made the model call", async () => { + const { run, pinnedTools, pinnedSeals, tools, seals } = called(withQueue) + + await run() + + expect(pinnedTools.map((t) => t.call.id)).toEqual(["call_a", "call_b"]) + expect(pinnedSeals).toHaveLength(1) + // Nothing reached the shared queue, which is the point: the tree the tools wrote is on that + // worker and nowhere else until the step ships it. + expect(tools).toHaveLength(0) expect(seals).toHaveLength(0) }) + + it("moves the step to the shared queue when nobody takes the pinned work", async () => { + const { run, goneAfterModelCall, pinnedTools, tools, seals } = called(withQueue) + goneAfterModelCall() + + const result = await run() + + // Schedule-to-start is the one failure that says the activity never started, so moving the work + // cannot run a tool twice. Both calls end up on the shared queue, and the step still closes. + expect(pinnedTools).toHaveLength(0) + expect(tools.map((t) => t.call.id).sort()).toEqual(["call_a", "call_b"]) + expect(seals).toHaveLength(1) + expect(result).toEqual(SEALED) + }) + + it("does not offer the pin again once the worker has failed to answer", async () => { + const { run, goneAfterModelCall, tools, seals, refusals } = called({ + ...withQueue, + calls: [call("call_a")], + }) + goneAfterModelCall() + + await run() + + // One refusal, from the tool. The seal that follows goes straight to the shared queue rather + // than spending another schedule-to-start bound on a worker already known to be gone. Counting + // the refusals is the assertion: the work reaches the shared queue either way, so where it + // ended up says nothing about how long the step spent finding out. Calls dispatched together + // do each pay it once, because none of them has learned anything yet when they start. + expect(refusals()).toBe(1) + expect(tools).toHaveLength(1) + expect(seals).toHaveLength(1) + }) + + it("waits for a started pinned sibling before moving another call to the shared queue", async () => { + const release = Promise.withResolvers() + const started = Promise.withResolvers() + const refused = Promise.withResolvers() + let completed = false + let overlap = false + const shared = fakes(withQueue, async () => { + overlap ||= !completed + return { outcome: "settled" } + }) + const run = makeSteppedTurn({ + activities: shared.activities, + isCancellation, + isHalt, + isUnclaimed: isUnclaimedFailure, + pinnedTo: () => ({ + runToolCall: async (input) => { + if (input.call.id === "call_a") { + started.resolve() + const result = await release.promise + completed = true + return result + } + await started.promise + refused.resolve() + throw unclaimed() + }, + sealStep: async () => SEALED, + }), + })(INPUT) + await refused.promise + await new Promise((resolve) => setTimeout(resolve, 0)) + const sharedBeforeRelease = shared.tools.length + release.resolve({ outcome: "settled" }) + await run + + expect(sharedBeforeRelease).toBe(0) + expect(overlap).toBe(false) + expect(shared.tools.map((input) => input.call.id)).toEqual(["call_b"]) + expect(shared.seals).toHaveLength(1) + }) + + it("keeps a pinned permission refusal out of the shared queue", async () => { + const shared = fakes({ ...withQueue, calls: [call("call_a")] }) + const declined = new FakeHalt("declined") + const seals: SealDrainInput[] = [] + const run = makeSteppedTurn({ + activities: shared.activities, + isCancellation, + isHalt, + pinnedTo: () => ({ + runToolCall: async () => { throw declined }, + sealStep: async (input) => { seals.push(input); return SEALED }, + }), + })(INPUT) + + await expect(run).rejects.toBe(declined) + expect(shared.tools).toHaveLength(0) + expect(seals).toHaveLength(1) + expect(seals[0]?.needsContinuation).toBe(false) + }) + + it("stops an unclaimed sibling after a pinned permission refusal", async () => { + const release = Promise.withResolvers() + const refused = Promise.withResolvers() + const shared = fakes(withQueue) + const declined = new FakeHalt("declined") + const run = makeSteppedTurn({ + activities: shared.activities, isCancellation, isHalt, + pinnedTo: () => ({ + runToolCall: async (input) => { + if (input.call.id === "call_a") return release.promise + refused.resolve() + throw unclaimed() + }, + sealStep: async () => SEALED, + }), + })(INPUT) + await refused.promise + release.reject(declined) + await expect(run).rejects.toBe(declined) + expect(shared.tools).toHaveLength(0) + expect(shared.seals).toHaveLength(1) + expect(shared.seals[0]?.needsContinuation).toBe(false) + }) + + it("stops later serial calls after a pinned permission refusal", async () => { + const shared = fakes(withQueue) + const declined = new FakeHalt("declined") + const pinned: string[] = [] + const seals: SealDrainInput[] = [] + const run = makeSteppedTurn({ + activities: shared.activities, isCancellation, isHalt, serial: true, + pinnedTo: () => ({ + runToolCall: async (input) => { + pinned.push(input.call.id) + throw declined + }, + sealStep: async (input) => { seals.push(input); return SEALED }, + }), + })(INPUT) + await expect(run).rejects.toBe(declined) + expect(pinned).toEqual(["call_a"]) + expect(shared.tools).toHaveLength(0) + expect(seals).toHaveLength(1) + expect(seals[0]?.needsContinuation).toBe(false) + }) + + it("stops later serial calls when the shared queue returns a refusal", async () => { + for (const queue of [undefined, "worker-queue"]) { + const declined = new FakeHalt("declined") + const shared = fakes({ ...withQueue, queue }, async () => { throw declined }) + const run = makeSteppedTurn({ + activities: shared.activities, isCancellation, isHalt, serial: true, + pinnedTo: () => ({ + runToolCall: async () => { throw unclaimed() }, + sealStep: async () => SEALED, + }), + })(INPUT) + await expect(run).rejects.toBe(declined) + expect(shared.tools.map((input) => input.call.id)).toEqual(["call_a"]) + expect(shared.seals).toHaveLength(1) + expect(shared.seals[0]?.needsContinuation).toBe(false) + } + }) + + it("refuses migration when a started pinned attempt times out", async () => { + // Heartbeat expiry also covers a process that can still write its directory. + const hostGone = () => + new ActivityFailure( + "activity Heartbeat timeout", + "runToolCall", + "1", + undefined, + undefined, + new TimeoutFailure("heartbeat timed out", undefined, "HEARTBEAT" as never), + ) + const shared = fakes(withQueue) + const failed = hostGone() + const run = makeSteppedTurn({ + activities: shared.activities, + isCancellation, + isHalt, + isUnclaimed: isUnclaimedFailure, + pinnedTo: () => ({ + runToolCall: async () => { + throw failed + }, + sealStep: async () => SEALED, + }), + })(INPUT) + + await expect(run).rejects.toBe(failed) + expect(shared.tools).toHaveLength(0) + expect(shared.seals).toHaveLength(0) + }) + + it("refuses shared dispatch after an uncertain pinned sibling settles", async () => { + const release = Promise.withResolvers() + const refused = Promise.withResolvers() + let sharedBeforeTheSiblingEnded = 0 + const shared = fakes(withQueue) + const run = makeSteppedTurn({ + activities: shared.activities, + isCancellation, + isHalt, + isUnclaimed: isUnclaimedFailure, + pinnedTo: () => ({ + runToolCall: async (input) => { + if (input.call.id === "call_a") return release.promise + refused.resolve() + throw unclaimed() + }, + sealStep: async () => SEALED, + }), + })(INPUT) + await refused.promise + await new Promise((resolve) => setTimeout(resolve, 0)) + sharedBeforeTheSiblingEnded = shared.tools.length + const failed = new Error("the started activity timed out") + release.reject(failed) + await expect(run).rejects.toBe(failed) + + expect(sharedBeforeTheSiblingEnded).toBe(0) + expect(shared.tools).toHaveLength(0) + expect(shared.seals).toHaveLength(0) + }) + + it("uses the shared queue when the model call reported no queue of its own", async () => { + const { run, tools, pinnedTools } = called({ ...withQueue, queue: undefined }) + + await run() + + expect(tools).toHaveLength(2) + expect(pinnedTools).toHaveLength(0) + }) }) // The bug this predicate exists for was a mismatch between what `boundary.ts` throws and what the diff --git a/packages/temporal/test/schedule-drain.test.ts b/packages/temporal/test/schedule-drain.test.ts new file mode 100644 index 000000000000..3a8b4082a7e3 --- /dev/null +++ b/packages/temporal/test/schedule-drain.test.ts @@ -0,0 +1,64 @@ +import { expect } from "bun:test" +import { Effect } from "effect" +import { eq } from "drizzle-orm" +import { Database } from "@opencode-ai/core/database/database" +import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" +import { LayerNode } from "@opencode-ai/core/effect/layer-node" +import { EventV2 } from "@opencode-ai/core/event" +import { EventSequenceTable } from "@opencode-ai/core/event/sql" +import { Project } from "@opencode-ai/core/project" +import { ProjectTable } from "@opencode-ai/core/project/sql" +import { AbsolutePath } from "@opencode-ai/core/schema" +import { SessionSchema } from "@opencode-ai/core/session/schema" +import { SessionInputTable, SessionTable } from "@opencode-ai/core/session/sql" +import { SessionProjector } from "@opencode-ai/core/session/projector" +import { testEffect } from "../../core/test/lib/effect" +import { makeScheduleDrains } from "../src/l2-drain" + +const it = testEffect( + AppNodeBuilder.build(LayerNode.group([Database.node, EventV2.node, SessionProjector.node]), [ + [Database.node, Database.layerFromPath(":memory:")], + ]), +) + +it.effect("admits later schedule firings without taking the runner's owner", () => + Effect.gen(function* () { + const { db } = yield* Database.Service + const events = yield* EventV2.Service + const sessionID = SessionSchema.ID.make("ses_schedule") + yield* db + .insert(ProjectTable) + .values({ + id: Project.ID.global, + worktree: AbsolutePath.make("/project"), + sandboxes: [], + }) + .run() + yield* db + .insert(SessionTable) + .values({ + id: sessionID, + project_id: Project.ID.global, + slug: "schedule", + directory: "/project", + title: "schedule", + version: "test", + }) + .run() + const { promptDrain } = makeScheduleDrains({ db, events }) + const first = { sessionID, messageID: "msg_schedule_first", text: "check the project" } + yield* Effect.promise(() => promptDrain(first)) + yield* events.claim(sessionID, "run:1:1") + yield* Effect.promise(() => promptDrain({ ...first, messageID: "msg_schedule_second" })) + yield* Effect.promise(() => promptDrain(first)) + + const prompts = yield* db.select().from(SessionInputTable).all() + const owner = yield* db + .select() + .from(EventSequenceTable) + .where(eq(EventSequenceTable.aggregate_id, sessionID)) + .get() + expect(prompts.map((row) => String(row.id)).sort()).toEqual(["msg_schedule_first", "msg_schedule_second"]) + expect(owner?.owner_id).toBe("run:1:1") + }), +) diff --git a/packages/temporal/test/scheduled-prompt-workflow.test.ts b/packages/temporal/test/scheduled-prompt-workflow.test.ts new file mode 100644 index 000000000000..35c55f72c026 --- /dev/null +++ b/packages/temporal/test/scheduled-prompt-workflow.test.ts @@ -0,0 +1,37 @@ +import { expect, it } from "bun:test" +import { fileURLToPath } from "node:url" +import { TestWorkflowEnvironment } from "@temporalio/testing" +import { Worker } from "@temporalio/worker" + +it("keeps distinct firing IDs after a long schedule name", async () => { + const env = await TestWorkflowEnvironment.createLocal() + const received: string[] = [] + const prefix = "daily-review-".repeat(5) + try { + const worker = await Worker.create({ + connection: env.nativeConnection, + namespace: env.namespace, + taskQueue: "schedule-id-check", + workflowsPath: fileURLToPath(new URL("../src/workflow.ts", import.meta.url)), + activities: { + promptSession: async (input: { messageID: string }) => { + received.push(input.messageID) + }, + runTurnStep: async () => ({ ran: true, continue: false, step: 1, promotion: null }), + }, + }) + await worker.runUntil(async () => { + for (const day of ["01", "02"]) { + await env.client.workflow.execute("scheduledPrompt", { + workflowId: `${prefix}-workflow-2026-09-${day}T09:00:00Z`, + taskQueue: "schedule-id-check", + args: [{ sessionID: "ses_schedule_id", text: "review yesterday's changes" }], + }) + } + }) + expect(received).toHaveLength(2) + expect(new Set(received).size).toBe(2) + } finally { + await env.teardown() + } +}, 120_000) diff --git a/packages/temporal/test/session-execution-temporal-contract.test.ts b/packages/temporal/test/session-execution-temporal-contract.test.ts index dcc381a703ac..3c7b8bf83fcf 100644 --- a/packages/temporal/test/session-execution-temporal-contract.test.ts +++ b/packages/temporal/test/session-execution-temporal-contract.test.ts @@ -10,7 +10,7 @@ // 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" +import { makeExecutionFor, runContract } from "../../core/test/lib/execution-conformance" 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 diff --git a/packages/temporal/test/session-supervisor-ceiling.test.ts b/packages/temporal/test/session-supervisor-ceiling.test.ts new file mode 100644 index 000000000000..782d130d2b44 --- /dev/null +++ b/packages/temporal/test/session-supervisor-ceiling.test.ts @@ -0,0 +1,65 @@ +// A turn that never stops stepping is a bug in the loop above this one: a model asking for the same +// tool forever, or a step that keeps handing itself back because its host keeps dying. Every one of +// those steps is a model call somebody pays for, and each of them succeeds, so nothing below the +// supervisor can see it. Driven by a fake runtime whose steps always ask for another. +import { it, expect } from "bun:test" +import { makeSupervisor, type SupervisorRuntime } from "../src/supervisor" +import type { StepDrainResult } from "../src/activities" + +const MORE: StepDrainResult = { ran: true, continue: true, step: 1, promotion: null } +const DONE: StepDrainResult = { ran: true, continue: false, step: 1, promotion: null } + +class LoopingRuntime implements SupervisorRuntime { + steps = 0 + warnings: string[] = [] + // A wake for the first turn and an idle timeout after it, which is how the supervisor gets to run + // one turn and then return rather than waiting for a signal this test never sends. + private woken = false + condition = async (predicate: () => boolean) => { + if (!this.woken) { + this.woken = true + this.wake?.() + return predicate() + } + return false + } + private wake: (() => void) | undefined + setSignalHandler = (name: string, handler: () => void) => { + if (name === "wake") this.wake = handler + } + setUpdateHandler = () => {} + // Always another step, up to a bound of its own: without one a ceiling that failed to hold would + // hang this test rather than fail it. + runTurnStep = async () => (++this.steps < 50 ? MORE : DONE) + runInDrainScope = (fn: () => Promise) => fn() + cancelCurrentScope = () => {} + isCancellation = () => false + isRootCancelled = () => false + warn = (message: string) => { + this.warnings.push(message) + } +} + +it("stops driving a turn that keeps asking for another step", async () => { + const rt = new LoopingRuntime() + const supervisor = makeSupervisor(rt, { maxStepsPerTurn: 5, idleTimeout: "1 millisecond" }) + await supervisor.sessionTurn("ses_ceiling") + + expect(rt.steps).toBe(5) + expect(rt.warnings).toEqual(["turn hit the step ceiling and was left where it stopped"]) +}) + +it("leaves the ceiling off a run that was recorded before it existed", async () => { + // The ceiling changes what the supervisor schedules, so a run that predates it would replay into + // a step this code refuses to take. Those runs keep what they recorded, and their own bound is + // the one the fake supplies. + const rt = new LoopingRuntime() + const supervisor = makeSupervisor( + { ...rt, boundsStepsPerTurn: () => false, runTurnStep: () => rt.runTurnStep() }, + { maxStepsPerTurn: 5, idleTimeout: "1 millisecond" }, + ) + await supervisor.sessionTurn("ses_unbounded") + + expect(rt.steps).toBe(50) + expect(rt.warnings).toEqual([]) +})