Skip to content

Commit 341af4d

Browse files
authored
Merge pull request #272 from LeXwDeX/fix/adoption-state-machine
fix(dag): session-following location stamps + adoption-vs-deletion fencing (#269, #270)
2 parents 7effbc5 + fdfc3fe commit 341af4d

10 files changed

Lines changed: 432 additions & 37 deletions

File tree

packages/core/src/dag/sql.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,16 @@ export const WorkflowTable = sqliteTable(
3131
session_id: text()
3232
.notNull()
3333
.references(() => SessionTable.id, { onDelete: "cascade" }),
34-
// Execution-location key (DAG-LOC-01): the creating instance's directory,
35-
// stamped at dag.create. Only the instance whose directory matches may
36-
// adopt, recover, wake, or spawn for this workflow. Nullable: legacy rows
37-
// predating the column match no instance (conservative — never adopted
38-
// until recreated).
34+
// Execution-location key (DAG-LOC-01): the directory that owns this workflow.
35+
// Only the instance whose directory matches may adopt, recover, wake, or spawn
36+
// for it. TWO-WRITER WHITELIST (#269): the stamp is written at dag.create
37+
// (WorkflowCreated projection INSERT, onConflictDoNothing — a replay can never
38+
// rewrite an existing stamp) and re-stamped ONLY by the session projector's
39+
// SessionEvent.Moved projection (the stamp moves WITH the session in one
40+
// transaction, payload-sourced from the Moved event — no SessionTable read).
41+
// No other writer may set it; R7-ext pins the whitelist. Nullable: legacy rows
42+
// predating the column match no instance (fail-closed — never adopted until
43+
// recreated).
3944
directory: text(),
4045
title: text().notNull(),
4146
status: text().notNull(),

packages/core/src/dag/store.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ const wakeDeliverableNodePredicate = or(
148148

149149
export interface Interface {
150150
readonly getWorkflow: (id: string) => Effect.Effect<WorkflowRow | undefined>
151+
readonly tryClaimAdoption: (id: string) => Effect.Effect<boolean>
151152
readonly listWorkflows: () => Effect.Effect<WorkflowRow[]>
152153
readonly listBySession: (sessionId: string) => Effect.Effect<WorkflowRow[]>
153154
readonly listByProject: (projectId: string) => Effect.Effect<WorkflowRow[]>
@@ -182,6 +183,31 @@ export const layer = Layer.effect(
182183
return row ? mapWorkflow(row) : undefined
183184
}),
184185

186+
// #270 atomic-adoption fence (C2). The adoption sites previously re-read the
187+
// row (ownsWorkflow) and then published their entry into the in-memory map —
188+
// a check-then-act pair a deletion cascade could commit between. The claim
189+
// collapses the admission into ONE conditional UPDATE: it matches the row
190+
// only while the row STILL EXISTS and is in an adoptable (non-terminal)
191+
// status, and returns whether it claimed. A Session.remove (FK cascade) or a
192+
// terminal transition that commits before the claim therefore makes the claim
193+
// match zero rows and the adoption aborts atomically — no post-deletion
194+
// admission survives. Directory ownership is NOT re-asserted here: the caller
195+
// has already passed DagLocation.ownsWorkflow, which canonicalizes both sides;
196+
// duplicating a directory comparison in SQL would diverge from that
197+
// canonicalization (create stamps are realpathed, Moved re-stamps are not),
198+
// so status conditionality is the fence and the read authority keeps the
199+
// directory key. No lease column — the status conditionality IS the claim.
200+
tryClaimAdoption: Effect.fn("DagStore.tryClaimAdoption")(function* (id) {
201+
const claimed = yield* db
202+
.update(WorkflowTable)
203+
.set({ time_updated: Date.now() })
204+
.where(and(eq(WorkflowTable.id, id), inArray(WorkflowTable.status, ["pending", "running", "paused", "stepping"])))
205+
.returning({ id: WorkflowTable.id })
206+
.get()
207+
.pipe(Effect.orDie)
208+
return claimed !== undefined
209+
}),
210+
185211
listWorkflows: Effect.fn("DagStore.listWorkflows")(function* () {
186212
const rows = yield* db.select().from(WorkflowTable).orderBy(desc(WorkflowTable.time_created)).all().pipe(Effect.orDie)
187213
return rows.map(mapWorkflow)

packages/core/src/database/migration.gen.ts

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { Effect } from "effect"
2+
import type { DatabaseMigration } from "../migration"
3+
4+
export default {
5+
id: "20260815083000_workflow_directory_convergence",
6+
up(tx) {
7+
return Effect.gen(function* () {
8+
// #269 atomic-adoption convergence (C6). The execution-location stamp now
9+
// moves WITH the session at SessionEvent.Moved time (session projector),
10+
// but installs that moved a session BEFORE that transition shipped carry
11+
// divergent stamps: the session row points at the destination directory
12+
// while its pre-move workflow rows stay pinned at the old one. With the
13+
// fail-closed ownership conjunct, those mixed stamps leave the session's
14+
// wakes with NO owner (the wedge from v1.0.13). Converge every NON-NULL
15+
// workflow stamp to its session's CURRENT directory — the same direction
16+
// the live Moved re-stamp moves it. No ALTER: the directory column already
17+
// exists. Preserves the fail-closed invariants: a NULL stamp (legacy row
18+
// never backfilled) stays NULL, and a workflow whose session has no
19+
// directory is left untouched. Idempotent — re-running converges to the
20+
// same state.
21+
yield* tx.run(`
22+
UPDATE \`workflow\`
23+
SET \`directory\` = (
24+
SELECT \`directory\` FROM \`session\` WHERE \`session\`.\`id\` = \`workflow\`.\`session_id\`
25+
)
26+
WHERE \`workflow\`.\`directory\` IS NOT NULL
27+
AND EXISTS (
28+
SELECT 1 FROM \`session\`
29+
WHERE \`session\`.\`id\` = \`workflow\`.\`session_id\`
30+
AND \`session\`.\`directory\` IS NOT NULL
31+
);
32+
`)
33+
})
34+
},
35+
} satisfies DatabaseMigration.Migration

packages/core/src/session/projector.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { SessionMessageUpdater } from "./message-updater"
1313
import { SessionInput } from "./input"
1414
import { WorkspaceV2 } from "../workspace"
1515
import { SessionContextEpoch } from "./context-epoch"
16+
import { WorkflowTable } from "../dag/sql"
1617
import { MessageTable, PartTable, SessionInputTable, SessionMessageTable, SessionTable } from "./sql"
1718
import type { DeepMutable } from "../schema"
1819
import { SessionMessageID } from "./message-id"
@@ -254,6 +255,21 @@ export const layer = Layer.effectDiscard(
254255
.where(eq(SessionTable.id, event.data.sessionID))
255256
.run()
256257
.pipe(Effect.orDie)
258+
// #269 atomic-adoption resolution: the execution-location stamp must move
259+
// WITH the session in one transaction. Re-stamp every durable workflow
260+
// row of the session to the payload-sourced destination directory (NO
261+
// SessionTable read — the Moved payload carries it). This runs inside the
262+
// same durable publish transaction as the SessionTable update, so there is
263+
// never a window where the session's rows carry mixed stamps (fail-closed
264+
// ownership would otherwise wedge every wake for the session). This is the
265+
// second whitelisted directory writer (see WorkflowTable.directory): the
266+
// create-time INSERT is the first; only SessionEvent.Moved may re-stamp.
267+
yield* db
268+
.update(WorkflowTable)
269+
.set({ directory: event.data.location.directory })
270+
.where(eq(WorkflowTable.session_id, event.data.sessionID))
271+
.run()
272+
.pipe(Effect.orDie)
257273
yield* SessionContextEpoch.reset(db, event.data.sessionID)
258274
}),
259275
)

packages/opencode/src/dag/runtime/loop.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { SessionV1 } from "@opencode-ai/core/v1/session"
99
import { InstanceState } from "@/effect/instance-state"
1010
import { EventV2Bridge } from "@/event-v2-bridge"
1111
import { DagEvent } from "@opencode-ai/schema/dag-event"
12+
import { SessionEvent } from "@opencode-ai/schema/session-event"
1213
import { SessionStatusEvent } from "@opencode-ai/schema/session-status-event"
1314
import { DagStore } from "@opencode-ai/core/dag/store"
1415
import { DagLocation } from "../location"
@@ -433,6 +434,13 @@ const serviceLayer = Layer.effect(
433434
// publishing now would leak an inert entry the sweep can no longer
434435
// reach. The ensuring below still clears the recovering reservation.
435436
if (!(yield* DagLocation.ownsWorkflow(dagID, ctx.directory))) return
437+
// #270 atomic-admission fence (C3): the durable read above and the
438+
// runtimes publish below are still two statements — a deletion can
439+
// commit between them. Collapse the admission into ONE conditional
440+
// UPDATE that matches only while the row exists and is non-terminal.
441+
// A cascade committed in that final window matches zero rows and the
442+
// adoption aborts here, before it ever publishes an entry.
443+
if (!(yield* store.tryClaimAdoption(dagID))) return
436444
runtimes.set(dagID, entry)
437445
yield* automation.register(SessionID.make(wf.sessionId), { kind: "dag", id: dagID })
438446
// Reconciliation settles every persisted running attempt before the
@@ -552,6 +560,12 @@ const serviceLayer = Layer.effect(
552560
// through. A row cascade-deleted after the first guard must not
553561
// be adopted into an inert entry.
554562
if (!(yield* DagLocation.ownsWorkflow(dagID, ctx.directory))) return
563+
// #270 atomic-admission fence (C3, same as recoverWorkflow): the
564+
// durable read and the runtimes publish are two statements a
565+
// deletion can slip between; collapse the admission into one
566+
// conditional UPDATE (exists + non-terminal). A cascade committed
567+
// in the final window matches zero rows and the adoption aborts.
568+
if (!(yield* store.tryClaimAdoption(dagID))) return
555569
runtimes.set(dagID, entry)
556570
yield* automation.register(SessionID.make(wf.sessionId), { kind: "dag", id: dagID })
557571
yield* entry.evalLock.withPermits(1)(
@@ -1392,6 +1406,52 @@ const serviceLayer = Layer.effect(
13921406
Effect.forkScoped({ startImmediately: true }),
13931407
)
13941408

1409+
// #269 SessionMoved ownership convergence: the Moved projection
1410+
// (core session projector) re-stamps the session's workflow rows to the
1411+
// destination directory in the SAME durable transaction, so by the time
1412+
// this handler runs the durable rows already agree on ONE directory.
1413+
// Converge the in-memory side: (a) the instance that no LONGER owns the
1414+
// moved session's workflows evicts its stale runtime entries (fail-closed
1415+
// — its directory must not keep acting on them), and (b) the NEW owner
1416+
// re-forks the serialized wake drain so a terminal wake that was wedged
1417+
// behind the old mixed stamps delivers immediately (bounded time) instead
1418+
// of waiting for a fresh idle event or a restart.
1419+
yield* events.subscribe(SessionEvent.Moved).pipe(
1420+
Stream.runForEach((evt) =>
1421+
Effect.gen(function* () {
1422+
const sessionID = evt.data.sessionID as string
1423+
// Map iteration is mutation-safe for deletions of visited entries —
1424+
// only entries of THIS session are deleted, each inside its own
1425+
// evalLock. Evict only entries the re-stamp moved AWAY from this
1426+
// instance (ownsWorkflow re-reads the durable row).
1427+
for (const [dagID, entry] of runtimes) {
1428+
if (entry.parentSessionID !== sessionID) continue
1429+
if (yield* DagLocation.ownsWorkflow(dagID, ctx.directory)) continue
1430+
yield* entry.evalLock.withPermits(1)(
1431+
Effect.gen(function* () {
1432+
for (const [nodeID, fiber] of entry.fibers) {
1433+
const node = yield* store.getNode(dagID, nodeID)
1434+
yield* abortChild(nodeID, node?.childSessionId ?? null).pipe(Effect.ignore)
1435+
yield* Fiber.interrupt(fiber).pipe(Effect.ignore)
1436+
const watcher = entry.watchers.get(nodeID)
1437+
if (watcher) yield* Fiber.interrupt(watcher).pipe(Effect.ignore)
1438+
}
1439+
entry.fibers.clear()
1440+
entry.watchers.clear()
1441+
runtimes.delete(dagID)
1442+
}),
1443+
)
1444+
}
1445+
// New owner: the re-stamp moved ownership HERE, so wake rows that
1446+
// were wedged (mixed stamps → no owner) are now deliverable.
1447+
if (yield* DagLocation.ownsSession(sessionID, ctx.directory)) {
1448+
yield* tryDeliverWake(sessionID).pipe(Effect.ignore, Effect.forkScoped)
1449+
}
1450+
}).pipe(guarded("SessionMoved")),
1451+
),
1452+
Effect.forkScoped({ startImmediately: true }),
1453+
)
1454+
13951455
// Install all live event handlers before spawning recovery watchers so
13961456
// a child that settles immediately cannot leave the runtime stale.
13971457
// Orphan-pending sweep first: the WorkflowStarted it publishes for the

packages/opencode/src/dag/runtime/spawn.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,17 @@ export function spawnNode(
404404
return
405405
}
406406
try {
407+
// #270 window-2 spawn-admission fence (C4): the node was durably
408+
// admitted (nodeQueued above) but the child session is about to
409+
// materialize — a deletion cascade (Session.remove → FK) committed in
410+
// that window must fence the spawn instead of letting it create an
411+
// orphan child for a dead workflow. Re-admit ATOMICALLY right before
412+
// sessions.create: the claim matches only while the workflow row exists
413+
// and is non-terminal, so a committed deletion matches zero rows and the
414+
// spawn aborts before any child session exists (no post-deletion spawn
415+
// survives). This is the revalidation that closes the spawn window the
416+
// nodeQueued guard alone leaves open between its read and its publish.
417+
if (!(yield* dag.store.tryClaimAdoption(input.dagID))) return
407418
// Permit acquired — only NOW materialize the child session and mark
408419
// the node running (P0-2). Before this point the node is durably
409420
// "queued" with no session: a 100-node fan-out holds at most

0 commit comments

Comments
 (0)