diff --git a/.specgit.yaml b/.specgit.yaml index 08d8ae885c..787fc00f56 100644 --- a/.specgit.yaml +++ b/.specgit.yaml @@ -1,8 +1,8 @@ version: 1 -delivery: macos-integrity-boundary +delivery: event-retention-reclamation context: kind: branch - branch: feat/498-macos-integrity-boundary + branch: feat/524-event-retention-reclamation issues: - - 498 -pr: 536 + - 524 +pr: 537 diff --git a/docs/adr/0001-event-residue-scrub-and-sqlite-auto-vacuum-reclamation.md b/docs/adr/0001-event-residue-scrub-and-sqlite-auto-vacuum-reclamation.md new file mode 100644 index 0000000000..996f8f2b25 --- /dev/null +++ b/docs/adr/0001-event-residue-scrub-and-sqlite-auto-vacuum-reclamation.md @@ -0,0 +1,118 @@ +# ADR 0001: Event residue scrub and SQLite auto-vacuum reclamation + +- **Status:** Accepted +- **Date:** 2026-09-04 +- **Issue:** #524 (delivery: PR #537 → `dev`) +- **Supersedes:** none + +## Context + +The durable event store had no reclamation semantics. `Event.remove(aggregateID)` ran only on +explicit session removal and covered the session aggregate alone, and SQLite ran without +`auto_vacuum`, so deleted rows never returned pages to the filesystem. Two failure shapes +motivated the decision: + +- A crash between the session-row delete and any cleanup stranded durable event aggregates + whose `SessionTable` and `WorkflowTable` read models were both gone. Replaying such an + aggregate is impossible: the `WorkflowCreated` projector INSERT dies on the + `workflow.session_id` foreign key once the session row is gone (pinned by + `packages/opencode/test/dag/dag-replay-idempotency.test.ts`), so residue can neither be + replayed nor re-materialized — it can only be deleted. +- `database.ts` initialized WAL and pragmas after driver open, so an application-level + `PRAGMA auto_vacuum` could not take effect: after WAL initialization the pragma silently + yields NONE even on an empty database. + +The decision checkpoint was approved on 2026-09-04 with the scope locked below. + +## Decisions + +1. **Explicit session + dag scrub.** `Session.remove` captures every related dag aggregate ID + before the `Deleted` publish (the projector's session-row delete FK-cascades the workflow + rows inside the publish transaction, so a post-publish lookup would see nothing) and removes + each dag event aggregate after the session aggregate — terminal workflows included. The + per-dag scrub is soft-degrading (a failure is logged and the aggregate is left for the + startup sweep) but preserves interruption (`Cause.hasInterrupts` re-raise, the + `EventResidueSweep` sibling discipline). +2. **Guarded default-on startup sweep.** `EventResidueSweep` runs one pass per process start, + forked into the layer scope so it can neither block nor fail startup. Eligibility is the + zero-live-read-model rule: an aggregate in `event_sequence` with neither a `session` nor a + `workflow` row. Removal is a single atomic guarded `DELETE` that re-evaluates both + NOT EXISTS guards inside the statement (no select-then-delete TOCTOU window), so an + aggregate recreated concurrently survives. Wired into `AppLayer` and the HttpApiApp node + graph, so every serving process sweeps; the pass is idempotent. +3. **New databases: FULL before WAL.** Both SQLite drivers (`sqlite.bun.ts`, `sqlite.node.ts`) + set `auto_vacuum=FULL` at the driver layer, before `journal_mode=WAL`, and only on a + genuinely empty (0-page) file. An immediate SQLITE_BUSY from a second opener racing the + first is tolerated: the pragma is a persistent header property and runs before any + WAL/migration write, so the first-write winner sets FULL for the database. +4. **Existing databases: explicit conversion only.** Legacy `auto_vacuum=NONE` databases are + never converted at startup — startup is detect-only (a warning pointing at the command). The + only conversion path is `opencode db vacuum --db `: the target must be named + explicitly and must already exist as a regular file (vacuum never creates a database), runs + FULL → VACUUM → `wal_checkpoint(TRUNCATE)` outside any startup path, and fails nonzero + unless the `PRAGMA auto_vacuum` readback is exactly FULL. Exclusive access is a hard + requirement (a concurrent writer fails VACUUM with SQLITE_BUSY). +5. **Archived-session retention: off and deferred.** No retention policy for archived sessions + ships in this decision (Phase 3). +6. **Active truncation: rejected.** Truncating active/retained session event history and event + snapshot folding are rejected; incremental replay (`seq > after`, ascending) and sync + cursors must keep observing unbroken per-aggregate histories. +7. **`incremental_vacuum` is forbidden.** A disposable bun:sqlite prototype reproduced an + exit-139 crash under the incremental mode; no code path may enable it. + +## Consequences and risks + +- Deleting events on legacy NONE databases still does not shrink the file until an operator + runs the explicit conversion; disk usage grows until then. +- `auto_vacuum=FULL` pays its known SQLite overhead (pointer-map pages, per-update mapping) on + every new database in exchange for automatic page reclamation. +- The sweep runs once per process start: residue created and abandoned within a single process + lifetime waits for the next start. This is accepted because the shapes it targets are + crash/in-flight zombies. +- The conversion command requires exclusive access; the error guidance says to close running + opencode processes and retry. +- Replay and sync contracts are preserved by construction: only whole aggregates with no live + read model are ever removed, and such aggregates are unreplayable anyway (FK death), so no + consumer can observe the removal as a gap in a replayable history. + +## Alternatives considered + +- **Rely on replay instead of scrubbing** — rejected: a wiped dag aggregate whose session row + is gone dies on the workflow foreign key during re-materialization, so replay cannot replace + deletion. +- **Silent startup conversion of legacy databases** — rejected: converting requires a blocking + full VACUUM; startup stays non-blocking and detect-only. +- **`PRAGMA incremental_vacuum`** — rejected (decision 7). +- **A recurring background reaper** — rejected in favor of one idempotent guarded pass per + process start; residue is crash-shaped, not steady-state throughput. +- **Truncate or fold active event histories** — rejected (decision 6). + +## Rollout and rollback + +Rollout lands as ordinary PRs through `dev` per the release train; no operator action is +required — new databases get FULL automatically, legacy databases keep working unchanged (with +a detect-only warning), and the sweep is default-on. Rollback is removing the sweep from the +app graphs and reverting the driver pragma: the sweep is additive and idempotent, and legacy +databases were never written by any of this. A database created with FULL keeps its header +mode; reverting one is itself an explicit operator VACUUM and is not automated. + +## Acceptance + +- Active/retained session replay is unchanged; only zero-live-read-model aggregates are + removed (guarded delete re-checked inside the statement). +- Cleanup failures never block the application path; interruption is preserved, not logged as + failure. +- Disposable-file tests demonstrate page reclamation and the new/existing database behavior; + no startup-time full VACUUM exists. +- `bun run test:dag-core`, focused event/session tests, package typecheck, and migration + freshness checks pass in CI. + +## Non-goals + +- **No global bounded-retention claim.** Live and retained sessions keep their full event + history indefinitely; this decision bounds nothing by age, size, or count. +- **No tombstones, unarchive, or sync changes.** Offline deletion tombstones, unarchive + semantics, and sync cursor/protocol changes stay out of scope. +- **No authorization for #531 or live-database work.** This decision does not authorize running + VACUUM or any cleanup against a live local database; the destructive operator procedure + remains the human-only issue #531. diff --git a/packages/core/src/database/database.ts b/packages/core/src/database/database.ts index 6879212c41..a8abe21276 100644 --- a/packages/core/src/database/database.ts +++ b/packages/core/src/database/database.ts @@ -2,7 +2,8 @@ export * as Database from "./database" import { EffectDrizzleSqlite } from "@opencode-ai/effect-drizzle-sqlite" import { layer as sqliteLayer } from "#sqlite" -import { Context, Effect, Layer } from "effect" +import { Cause, Context, Effect, Layer } from "effect" +import { sql } from "drizzle-orm" import { Global } from "../global" import { Flag } from "../flag/flag" import { isAbsolute, join } from "path" @@ -29,6 +30,27 @@ export const layer = Layer.effect( yield* db.run("PRAGMA busy_timeout = 5000") yield* db.run("PRAGMA cache_size = -64000") yield* db.run("PRAGMA foreign_keys = ON") + // #524: genuinely new databases were switched to auto_vacuum=FULL by the + // sqlite driver BEFORE WAL init. A legacy database keeps its NONE mode — + // converting one silently at startup would need a blocking full VACUUM — + // so it is only detected and surfaced softly here; conversion is the + // explicit user-triggered `opencode db vacuum --db ` command. + // Detect-only means detect-only: a failed readback degrades to a warning + // (the layer body is orDie'd, so an unhandled failure would kill startup), + // while an interruption is always re-raised. + const autoVacuum = yield* db.get<{ auto_vacuum: number }>(sql`PRAGMA auto_vacuum`).pipe( + Effect.catchCause((cause) => + Cause.hasInterrupts(cause) + ? Effect.interrupt + : Effect.logWarning("database auto_vacuum readback failed — skipping the detect-only check", { cause }).pipe( + Effect.as(undefined), + ), + ), + ) + if (autoVacuum?.auto_vacuum === 0) + yield* Effect.logWarning( + "database auto_vacuum is NONE — deleted pages stay allocated until converted; run `opencode db vacuum --db ` (prints its path with `opencode db path`)", + ) yield* db.run("PRAGMA wal_checkpoint(PASSIVE)") yield* DatabaseMigration.apply(db) diff --git a/packages/core/src/database/sqlite.bun.ts b/packages/core/src/database/sqlite.bun.ts index e15f4c117e..3d35f03193 100644 --- a/packages/core/src/database/sqlite.bun.ts +++ b/packages/core/src/database/sqlite.bun.ts @@ -161,11 +161,40 @@ const nativeLayer = (config: Config) => create: config.create ?? true, }) yield* Effect.addFinalizer(() => Effect.sync(() => native.close())) + // #524: auto_vacuum must be set BEFORE any WAL initialization — after + // WAL init the pragma silently yields NONE even on an empty database. + // Only a genuinely empty (0-page) file is eligible: on any existing + // database the pragma would be a no-op at best, so legacy NONE + // databases are never written here (startup stays detect-only; the + // explicit conversion lives in ./vacuum). + native.run("PRAGMA busy_timeout = 5000;") + if (config.readonly !== true) setAutoVacuumFull(native) if (config.disableWAL !== true) native.run("PRAGMA journal_mode = WAL;") return native }), ) +/** + * Setting auto_vacuum needs the write lock of a read-header-then-write + * upgrade, which SQLite fails with an immediate SQLITE_BUSY the busy handler + * cannot retry — a second opener racing the first one's initialization on the + * same new file hits it. Skipping on BUSY is safe: auto_vacuum is a + * persistent header property and every opener runs this pragma BEFORE any + * WAL/migration write, so whichever connection wins the first-write race sets + * FULL for the database. + */ +function setAutoVacuumFull(native: Database) { + const page = native.query<{ page_count: number }, []>("PRAGMA page_count").get() + if (!page || page.page_count !== 0) return + try { + native.run("PRAGMA auto_vacuum = FULL;") + } catch (cause) { + if (!isSqliteBusy(cause)) throw cause + } +} + +const isSqliteBusy = (cause: unknown) => cause instanceof Error && /SQLITE_BUSY|database is locked/i.test(cause.message) + const sqliteLayer = (config: Config) => Layer.effect(Client.SqlClient, make(config)) const drizzleLayer = Layer.effect( diff --git a/packages/core/src/database/sqlite.node.ts b/packages/core/src/database/sqlite.node.ts index 6eaecbee26..3484be76ac 100644 --- a/packages/core/src/database/sqlite.node.ts +++ b/packages/core/src/database/sqlite.node.ts @@ -156,11 +156,40 @@ const nativeLayer = (config: Config) => open: true, }) yield* Effect.addFinalizer(() => Effect.sync(() => native.close())) + // #524: auto_vacuum must be set BEFORE any WAL initialization — after + // WAL init the pragma silently yields NONE even on an empty database. + // On an existing non-empty database the pragma is a SQLite no-op, so + // legacy NONE databases are never converted here (startup stays + // detect-only; the explicit conversion lives in ./vacuum). + native.exec("PRAGMA busy_timeout = 5000;") + if (config.readonly !== true) setAutoVacuumFull(native) if (config.disableWAL !== true && config.readonly !== true) native.exec("PRAGMA journal_mode = WAL;") return native }), ) +/** + * Setting auto_vacuum needs the write lock of a read-header-then-write + * upgrade, which SQLite fails with an immediate SQLITE_BUSY the busy handler + * cannot retry — a second opener racing the first one's initialization on the + * same new file hits it. Skipping on BUSY is safe: auto_vacuum is a + * persistent header property and every opener runs this pragma BEFORE any + * WAL/migration write, so whichever connection wins the first-write race sets + * FULL for the database. + */ +function setAutoVacuumFull(native: DatabaseSync) { + const page: unknown = native.prepare("PRAGMA page_count").get() + const pageCount = typeof page === "object" && page !== null && "page_count" in page ? page.page_count : undefined + if (typeof pageCount !== "number" || pageCount !== 0) return + try { + native.exec("PRAGMA auto_vacuum = FULL;") + } catch (cause) { + if (!isSqliteBusy(cause)) throw cause + } +} + +const isSqliteBusy = (cause: unknown) => cause instanceof Error && /SQLITE_BUSY|database is locked/i.test(cause.message) + const sqliteLayer = (config: Config) => Layer.effect(Client.SqlClient, make(config)) const drizzleLayer = Layer.effect( diff --git a/packages/core/src/database/vacuum.ts b/packages/core/src/database/vacuum.ts new file mode 100644 index 0000000000..434f789cde --- /dev/null +++ b/packages/core/src/database/vacuum.ts @@ -0,0 +1,97 @@ +// SPDX-FileCopyrightText: 2026 LeXwDeX +// SPDX-License-Identifier: AGPL-3.0-or-later + +export * as Vacuum from "./vacuum" + +import { EffectDrizzleSqlite } from "@opencode-ai/effect-drizzle-sqlite" +import { NodeFileSystem } from "@effect/platform-node" +import { Effect, FileSystem, Schema } from "effect" +import { sql } from "drizzle-orm" +import { layer } from "#sqlite" + +const makeDb = EffectDrizzleSqlite.makeWithDefaults() + +export interface ConvertResult { + /** Readback of `PRAGMA auto_vacuum` after conversion: 1 == FULL. */ + readonly autoVacuum: number +} + +export class Refused extends Schema.TaggedErrorClass()("VacuumRefused", { + filename: Schema.String, + reason: Schema.String, +}) { + override get message() { + return `refusing to vacuum ${this.filename}: ${this.reason}` + } +} + +export class NotFull extends Schema.TaggedErrorClass()("VacuumNotFull", { + filename: Schema.String, + // -1 encodes an unreadable readback (the pragma returned no row). + autoVacuum: Schema.Number, +}) { + override get message() { + return `vacuum did not take full effect for ${this.filename}: PRAGMA auto_vacuum reads back ${this.autoVacuum}, expected 1 (FULL) — close running opencode processes that use this file and retry` + } +} + +/** + * #524 Phase 2 gate and the only success path of `convertToFull`: after the + * FULL -> VACUUM -> wal_checkpoint(TRUNCATE) sequence the readback must be + * exactly FULL (1), otherwise the conversion silently failed (e.g. a writer + * kept the file alive through VACUUM) and must surface as a nonzero failure + * with actionable diagnostics — never as a success result. Exported as the + * deterministic seam that lets tests prove a non-FULL readback cannot report + * success. + */ +export const verifyFull = (filename: string, readback: number | undefined): Effect.Effect => + readback === 1 + ? Effect.succeed({ autoVacuum: readback }) + : Effect.fail(new NotFull({ filename, autoVacuum: readback ?? -1 })) + +// #524: refuse every target that is not an existing regular file BEFORE any +// SQLite open — the driver opens with create enabled, so a typo'd path would +// otherwise silently materialize a fresh empty database. +const validateTarget = Effect.fn("Vacuum.validateTarget")(function* (fs: FileSystem.FileSystem, filename: string) { + if (filename === ":memory:") yield* new Refused({ filename, reason: ":memory: is not a file on disk" }) + const info = yield* fs.stat(filename).pipe(Effect.catch(() => Effect.void)) + if (info === undefined) { + yield* new Refused({ + filename, + reason: "no such file — vacuum never creates a database (print the default path with `opencode db path`)", + }) + return + } + if (info.type !== "File") yield* new Refused({ filename, reason: `not a regular file (${info.type})` }) +}) + +const convert = (filename: string) => + Effect.gen(function* () { + const db = yield* makeDb + yield* db.run(sql`PRAGMA busy_timeout = 5000`) + yield* db.run(sql`PRAGMA auto_vacuum = FULL`) + yield* db.run(sql`VACUUM`) + yield* db.run(sql`PRAGMA wal_checkpoint(TRUNCATE)`) + const mode = yield* db.get<{ auto_vacuum: number }>(sql`PRAGMA auto_vacuum`) + return mode?.auto_vacuum + }).pipe(Effect.provide(layer({ filename }))) + +/** + * #524 Phase 2: explicit, user-triggered conversion of a legacy + * auto_vacuum=NONE database to FULL. Runs OUTSIDE startup and never touches a + * default database path implicitly — the caller names the file (the CLI + * surface requires an explicit `--db`, so tests only ever pass disposable + * temp paths), and the target must already exist as a regular file: vacuum + * never creates a database. The FULL → VACUUM → wal_checkpoint(TRUNCATE) + * sequence rebuilds the database with FULL enabled and truncates the WAL; a + * concurrent writer makes VACUUM fail with SQLITE_BUSY instead of corrupting + * anything, and a non-FULL readback fails via `verifyFull`. Incremental + * auto-vacuum is deliberately never used anywhere. + */ +export const convertToFull = (filename: string) => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem + yield* validateTarget(fs, filename) + const readback = yield* convert(filename) + return yield* verifyFull(filename, readback) + }).pipe(Effect.provide(NodeFileSystem.layer)) diff --git a/packages/core/src/event/residue-sweep.ts b/packages/core/src/event/residue-sweep.ts new file mode 100644 index 0000000000..f16b87e8bc --- /dev/null +++ b/packages/core/src/event/residue-sweep.ts @@ -0,0 +1,129 @@ +// SPDX-FileCopyrightText: 2026 LeXwDeX +// SPDX-License-Identifier: AGPL-3.0-or-later + +export * as EventResidueSweep from "./residue-sweep" + +import { Cause, Context, Effect, Layer, Scope } from "effect" +import { sql } from "drizzle-orm" +import { Database } from "../database/database" +import { LayerNode } from "../effect/layer-node" + +/** + * #524 Phase 1: default-on residue sweep for crash/in-flight zombies. + * + * Session.remove scrubs its session aggregate and every related dag aggregate + * (terminal workflows included), but a crash between the session-row delete + * and the scrub — or a project cascade that removes read-model rows without + * any remove call — leaves durable event aggregates whose SessionTable and + * WorkflowTable read models are BOTH gone. Read models commit atomically with + * an aggregate's first event (the projectors run inside the publish + * transaction), so "events visible, both read models absent" is exactly the + * zombie shape. Live sessions, archived sessions (they keep their row), and + * live workflows never match the predicate. + * + * Removal is a single atomic guarded DELETE (see `removeResidue`): the + * both-read-models-absent guard is re-evaluated inside the delete statement + * itself, so a read model a concurrent replay/publish recreates after + * candidate selection survives — there is no select-then-remove TOCTOU + * window. Soft-degrading: a failed residue read or a failed per-aggregate + * removal is logged and left for a later pass — the sweep never fails the + * application path. The Durable manifest only contains session- and + * dag-family events, so every aggregate id in event_sequence is keyed by one + * of the two checked read models. + */ + +export interface Interface { + /** One sweep pass. Returns the number of residue aggregates removed. */ + readonly sweepOnce: () => Effect.Effect +} + +export class Service extends Context.Service()("@opencode/v2/EventResidueSweep") {} + +/** + * Selection half of the sweep pass. Exported as the seam that lets tests + * deterministically interleave a concurrent read-model recreation between + * candidate selection and deletion (no sleeps). + */ +export const selectResidues = (db: Database.Interface["db"]) => + db.all<{ aggregate_id: string }>(sql` + SELECT aggregate_id FROM event_sequence + WHERE NOT EXISTS (SELECT 1 FROM session WHERE session.id = event_sequence.aggregate_id) + AND NOT EXISTS (SELECT 1 FROM workflow WHERE workflow.id = event_sequence.aggregate_id) + `) + +/** + * Removal half of the sweep pass — the atomic guarded delete. The NOT EXISTS + * guards are re-evaluated inside the DELETE statement itself, so a read model + * recreated between candidate selection and this statement survives; the + * aggregate is only deleted while it is still a zombie. PRAGMA foreign_keys + * is ON on the Database layer's connection, so the delete cascades to the + * aggregate's event rows, and RETURNING makes the removed result reliable + * instead of inferred. Exported for the same test seam as `selectResidues`. + */ +export const removeResidue = (db: Database.Interface["db"], aggregateID: string) => + db + .all<{ aggregate_id: string }>(sql` + DELETE FROM event_sequence + WHERE aggregate_id = ${aggregateID} + AND NOT EXISTS (SELECT 1 FROM session WHERE session.id = event_sequence.aggregate_id) + AND NOT EXISTS (SELECT 1 FROM workflow WHERE workflow.id = event_sequence.aggregate_id) + RETURNING aggregate_id + `) + .pipe(Effect.map((rows) => rows.length > 0)) + +const serviceLayer = Layer.effect( + Service, + Effect.gen(function* () { + const { db } = yield* Database.Service + const scope = yield* Scope.Scope + + const sweepOnce = Effect.fn("EventResidueSweep.sweepOnce")(function* () { + const residues = yield* selectResidues(db).pipe( + Effect.catchCause((cause) => + Cause.hasInterrupts(cause) + ? Effect.interrupt + : Effect.gen(function* () { + yield* Effect.logWarning("EventResidueSweep residue query failed — skipping pass", { cause }) + return [] as Array<{ aggregate_id: string }> + }), + ), + ) + + let removed = 0 + for (const residue of residues) { + const done = yield* removeResidue(db, residue.aggregate_id).pipe( + Effect.catchCause((cause) => + Cause.hasInterrupts(cause) + ? Effect.interrupt + : Effect.gen(function* () { + yield* Effect.logWarning("EventResidueSweep failed to remove a residue aggregate — left for a later pass", { + aggregateID: residue.aggregate_id, + cause, + }) + return false + }), + ), + ) + if (done) removed++ + } + if (removed > 0) yield* Effect.logInfo("EventResidueSweep removed orphaned event aggregates", { removed }) + return removed + }) + + // Default-on: one pass per process start, forked into the layer scope so + // it can neither block nor fail startup (the AGENTS.md background-loop + // convention — no caller has to remember to init it). + yield* sweepOnce().pipe( + Effect.catchCause((cause) => + Cause.hasInterrupts(cause) ? Effect.interrupt : Effect.logWarning("EventResidueSweep startup pass failed", { cause }), + ), + Effect.forkIn(scope), + ) + + return Service.of({ sweepOnce }) + }), +) + +export const defaultLayer = serviceLayer.pipe(Layer.provide(Database.defaultLayer)) + +export const node = LayerNode.make(serviceLayer, [Database.node]) diff --git a/packages/core/test/database-vacuum.test.ts b/packages/core/test/database-vacuum.test.ts new file mode 100644 index 0000000000..fa6604b24d --- /dev/null +++ b/packages/core/test/database-vacuum.test.ts @@ -0,0 +1,244 @@ +import { describe, expect, test } from "bun:test" +import { Database as BunSqlite } from "bun:sqlite" +import { EffectDrizzleSqlite } from "@opencode-ai/effect-drizzle-sqlite" +import { SqliteClient } from "@effect/sql-sqlite-bun" +import { Cause, Effect, Exit, Layer } from "effect" +import { SqlClient } from "effect/unstable/sql/SqlClient" +import { classifySqliteError, SqlError } from "effect/unstable/sql/SqlError" +import { existsSync } from "fs" +import { sql } from "drizzle-orm" +import path from "path" +import { Database } from "@opencode-ai/core/database/database" +import { DatabaseMigration } from "@opencode-ai/core/database/migration" +import { Vacuum } from "@opencode-ai/core/database/vacuum" +import { ProjectV2 } from "@opencode-ai/core/project" +import { ProjectTable } from "@opencode-ai/core/project/sql" +import { SessionSchema } from "@opencode-ai/core/session/schema" +import { SessionTable } from "@opencode-ai/core/session/sql" +import { AbsolutePath } from "@opencode-ai/core/schema" +import { layer as repoSqliteLayer } from "#sqlite" +import { tmpdir } from "./fixture/tmpdir" + +const makeDb = EffectDrizzleSqlite.makeWithDefaults() + +// Seeds an application-created LEGACY database shape: WAL initialized, +// auto_vacuum left at its NONE default, real migrations applied, user rows +// present. Disposable temp files only — never a real opencode.db path. +const seedLegacyDatabase = (filename: string) => + Effect.runPromise( + Effect.gen(function* () { + const db = yield* makeDb + yield* db.run(sql`PRAGMA journal_mode = WAL`) + yield* DatabaseMigration.apply(db) + yield* db + .insert(ProjectTable) + .values({ id: ProjectV2.ID.make("proj_legacy"), worktree: AbsolutePath.make("/legacy"), sandboxes: [] }) + .run() + .pipe(Effect.orDie) + yield* db + .insert(SessionTable) + .values({ + id: SessionSchema.ID.make("ses_legacy"), + project_id: ProjectV2.ID.make("proj_legacy"), + slug: "legacy", + directory: "/legacy", + title: "legacy", + version: "test", + }) + .run() + .pipe(Effect.orDie) + }).pipe(Effect.provide(SqliteClient.layer({ filename })), Effect.scoped), + ) + +const readFileMode = (filename: string) => { + const native = new BunSqlite(filename, { readonly: true, create: false }) + try { + const autoVacuum = native.query<{ auto_vacuum: number }, []>("PRAGMA auto_vacuum").get() + const freelist = native.query<{ freelist_count: number }, []>("PRAGMA freelist_count").get() + const integrity = native.query<{ integrity_check: string }, []>("PRAGMA integrity_check").get() + const rows = native.query<{ count: number }, []>("SELECT COUNT(*) AS count FROM session").get() + return { + autoVacuum: autoVacuum?.auto_vacuum ?? -1, + freelist: freelist?.freelist_count ?? -1, + integrity: integrity?.integrity_check ?? "unknown", + sessionRows: rows?.count ?? -1, + } + } finally { + native.close() + } +} + +describe("Database auto_vacuum (#524 Phase 2)", () => { + test("initializes genuinely new databases with auto_vacuum=FULL before WAL", async () => { + await using tmp = await tmpdir() + const filename = path.join(tmp.path, "new.sqlite") + await Effect.runPromise( + Effect.gen(function* () { + const { db } = yield* Database.Service + + const mode = yield* db.get<{ auto_vacuum: number }>(sql`PRAGMA auto_vacuum`).pipe(Effect.orDie) + expect(mode?.auto_vacuum).toBe(1) + const journal = yield* db.get<{ journal_mode: string }>(sql`PRAGMA journal_mode`).pipe(Effect.orDie) + expect(String(journal?.journal_mode).toLowerCase()).toBe("wal") + + // The real application layer (migrations included) preserves the mode. + yield* db + .insert(ProjectTable) + .values({ id: ProjectV2.ID.make("proj_new"), worktree: AbsolutePath.make("/new"), sandboxes: [] }) + .run() + .pipe(Effect.orDie) + const after = yield* db.get<{ auto_vacuum: number }>(sql`PRAGMA auto_vacuum`).pipe(Effect.orDie) + expect(after?.auto_vacuum).toBe(1) + }).pipe(Effect.provide(Database.layerFromPath(filename))), + ) + }) + + test("never converts an existing auto_vacuum=NONE database at startup", async () => { + await using tmp = await tmpdir() + const filename = path.join(tmp.path, "legacy.sqlite") + await seedLegacyDatabase(filename) + expect(readFileMode(filename).autoVacuum).toBe(0) + + // The production startup sequence (driver pragmas + migrations) must be a + // silent no-op for the legacy mode — converting without the explicit + // user-triggered command is forbidden. + await Effect.runPromise( + Effect.gen(function* () { + const { db } = yield* Database.Service + const mode = yield* db.get<{ auto_vacuum: number }>(sql`PRAGMA auto_vacuum`).pipe(Effect.orDie) + expect(mode?.auto_vacuum).toBe(0) + expect( + (yield* db.get<{ count: number }>(sql`SELECT COUNT(*) AS count FROM session`).pipe(Effect.orDie))?.count, + ).toBe(1) + }).pipe(Effect.provide(Database.layerFromPath(filename))), + ) + const after = readFileMode(filename) + expect(after.autoVacuum).toBe(0) + expect(after.sessionRows).toBe(1) + expect(after.integrity).toBe("ok") + }) + + test("explicit conversion runs FULL -> VACUUM -> wal TRUNCATE with data intact", async () => { + await using tmp = await tmpdir() + const filename = path.join(tmp.path, "legacy-convert.sqlite") + await seedLegacyDatabase(filename) + expect(readFileMode(filename).autoVacuum).toBe(0) + + const result = await Effect.runPromise(Vacuum.convertToFull(filename)) + expect(result.autoVacuum).toBe(1) + + const after = readFileMode(filename) + expect(after.autoVacuum).toBe(1) + expect(after.freelist).toBe(0) + expect(after.sessionRows).toBe(1) + expect(after.integrity).toBe("ok") + }) + + test("refuses a nonexistent target before opening SQLite and never creates it", async () => { + await using tmp = await tmpdir() + const filename = path.join(tmp.path, "typo.sqlite") + + const exit = await Effect.runPromiseExit(Vacuum.convertToFull(filename)) + expect(Exit.isFailure(exit)).toBe(true) + if (Exit.isFailure(exit)) { + const rendered = Cause.pretty(exit.cause) + expect(rendered).toContain("refusing to vacuum") + expect(rendered).toContain("no such file") + } + // The typo must not have materialized a database (nor WAL/SHM siblings). + expect(existsSync(filename)).toBe(false) + expect(existsSync(`${filename}-wal`)).toBe(false) + expect(existsSync(`${filename}-shm`)).toBe(false) + }) + + test("refuses :memory: and non-file targets", async () => { + const memory = await Effect.runPromiseExit(Vacuum.convertToFull(":memory:")) + expect(Exit.isFailure(memory)).toBe(true) + if (Exit.isFailure(memory)) expect(Cause.pretty(memory.cause)).toContain("not a file on disk") + + await using tmp = await tmpdir() + const directory = await Effect.runPromiseExit(Vacuum.convertToFull(tmp.path)) + expect(Exit.isFailure(directory)).toBe(true) + if (Exit.isFailure(directory)) expect(Cause.pretty(directory.cause)).toContain("not a regular file") + expect(existsSync(tmp.path)).toBe(true) + }) + + // Deterministic proof that a non-FULL readback can never report success: + // `verifyFull` is the only success path of `convertToFull`. + test("a non-FULL readback fails with actionable diagnostics via verifyFull", async () => { + const zero = await Effect.runPromiseExit(Vacuum.verifyFull("stuck.sqlite", 0)) + expect(Exit.isFailure(zero)).toBe(true) + if (Exit.isFailure(zero)) { + const rendered = Cause.pretty(zero.cause) + expect(rendered).toContain("VacuumNotFull") + expect(rendered).toContain("stuck.sqlite") + expect(rendered).toContain("expected 1 (FULL)") + expect(rendered).toContain("retry") + } + + const unreadable = await Effect.runPromiseExit(Vacuum.verifyFull("stuck.sqlite", undefined)) + expect(Exit.isFailure(unreadable)).toBe(true) + + const ok = await Effect.runPromise(Vacuum.verifyFull("converted.sqlite", 1)) + expect(ok.autoVacuum).toBe(1) + }) + + // Layer/failure regression: a failed auto_vacuum readback must soft-degrade + // with a warning — the startup layer must not die (its body is orDie'd), so + // migrations still apply and the service stays usable. + test("a failed auto_vacuum readback soft-degrades instead of killing startup", async () => { + await using tmp = await tmpdir() + const filename = path.join(tmp.path, "readback-failure.sqlite") + + // Real repository sqlite client stack (#sqlite = the production driver), + // except every auto_vacuum statement fails at the Database.layer level. + const failingReadbackLayer = Layer.effect( + SqlClient, + Effect.gen(function* () { + const client = yield* SqlClient + const failure = new SqlError({ + reason: classifySqliteError(new Error("simulated auto_vacuum readback failure"), { + message: "Failed to execute statement", + operation: "execute", + }), + }) + // oxlint-disable-next-line typescript-eslint/no-unsafe-type-assertion -- test decorator over the real client, shape-preserving at runtime + return Object.assign({}, client, { + unsafe: (query: string, params?: ReadonlyArray) => { + const statement = client.unsafe(query, params) + if (!query.toLowerCase().includes("auto_vacuum")) return statement + return Object.assign({}, statement, { + withoutTransform: Effect.fail(failure), + values: Effect.fail(failure), + }) + }, + }) as SqlClient + }), + ).pipe(Layer.provide(repoSqliteLayer({ filename }))) + + await Effect.runPromise( + Effect.gen(function* () { + const { db } = yield* Database.Service + // Startup ran past the failed readback: migrations were applied and + // the service is usable. + const tables = yield* db + .get<{ count: number }>(sql`SELECT COUNT(*) AS count FROM sqlite_master WHERE type = 'table' AND name = 'session'`) + .pipe(Effect.orDie) + expect(tables?.count).toBe(1) + }).pipe(Effect.provide(Database.layer.pipe(Layer.provide(failingReadbackLayer)))), + ) + // The database file itself was created and initialized normally. + expect(readFileMode(filename).autoVacuum).toBe(1) + }) + + test("incremental_vacuum never appears in executable database code", async () => { + const databaseDir = path.join(import.meta.dir, "..", "src", "database") + const glob = new Bun.Glob("**/*.ts") + const offenders: string[] = [] + for await (const file of glob.scan({ cwd: databaseDir })) { + const content = await Bun.file(path.join(databaseDir, file)).text() + if (/incremental_vacuum/i.test(content)) offenders.push(file) + } + expect(offenders).toEqual([]) + }) +}) diff --git a/packages/core/test/event-residue-sweep.test.ts b/packages/core/test/event-residue-sweep.test.ts new file mode 100644 index 0000000000..63e400d4d4 --- /dev/null +++ b/packages/core/test/event-residue-sweep.test.ts @@ -0,0 +1,156 @@ +import { describe, expect, test } from "bun:test" +import { Effect, Layer } from "effect" +import { eq, inArray } from "drizzle-orm" +import { Database } from "@opencode-ai/core/database/database" +import { EventSequenceTable, EventTable } from "@opencode-ai/core/event/sql" +import { EventV2 } from "@opencode-ai/core/event" +import { EventResidueSweep } from "@opencode-ai/core/event/residue-sweep" +import { SessionSchema } from "@opencode-ai/core/session/schema" +import { SessionTable } from "@opencode-ai/core/session/sql" +import { WorkflowTable } from "@opencode-ai/core/dag/sql" +import { ProjectTable } from "@opencode-ai/core/project/sql" +import { ProjectV2 } from "@opencode-ai/core/project" +import { AbsolutePath } from "@opencode-ai/core/schema" + +// #524 Phase 1: crash/in-flight zombie residue — a Session.remove (or a project +// cascade) that crashed between the session-row delete and the event-store +// scrub leaves durable event aggregates whose SessionTable and WorkflowTable +// read models are both gone. The default-on residue sweep removes exactly +// those aggregates and never touches live or archived ones. +const testLayer = Layer.mergeAll(Database.defaultLayer, EventResidueSweep.defaultLayer) + +const seedAggregate = (aggregateID: string) => + Effect.gen(function* () { + const { db } = yield* Database.Service + yield* db.insert(EventSequenceTable).values({ aggregate_id: aggregateID, seq: 1 }).run().pipe(Effect.orDie) + yield* db + .insert(EventTable) + .values({ id: EventV2.ID.make(`evt_${aggregateID}`), aggregate_id: aggregateID, seq: 1, type: "session.updated.1", data: {} }) + .run() + .pipe(Effect.orDie) + }) + +const seedProject = Effect.gen(function* () { + const { db } = yield* Database.Service + yield* db + .insert(ProjectTable) + .values({ id: ProjectV2.ID.make("proj_sweep"), worktree: AbsolutePath.make("/project"), sandboxes: [] }) + .run() + .pipe(Effect.orDie) +}) + +const remainingAggregates = (ids: readonly string[]) => + Effect.gen(function* () { + const { db } = yield* Database.Service + return yield* db + .select({ aggregate: EventSequenceTable.aggregate_id }) + .from(EventSequenceTable) + .where(inArray(EventSequenceTable.aggregate_id, [...ids])) + .all() + .pipe(Effect.orDie) + }) + +describe("EventResidueSweep (#524)", () => { + test("removes only aggregates whose session and workflow read models are both absent", async () => { + await Effect.runPromise( + Effect.gen(function* () { + const { db } = yield* Database.Service + const sweep = yield* EventResidueSweep.Service + + yield* db + .insert(ProjectTable) + .values({ id: ProjectV2.ID.make("proj_sweep"), worktree: AbsolutePath.make("/project"), sandboxes: [] }) + .run() + .pipe(Effect.orDie) + yield* db + .insert(SessionTable) + .values({ id: SessionSchema.ID.make("ses_live"), project_id: ProjectV2.ID.make("proj_sweep"), slug: "live", directory: "/project", title: "live", version: "test" }) + .run() + .pipe(Effect.orDie) + // Archived sessions keep their read-model row — never eligible. + yield* db + .insert(SessionTable) + .values({ id: SessionSchema.ID.make("ses_archived"), project_id: ProjectV2.ID.make("proj_sweep"), slug: "archived", directory: "/project", title: "archived", version: "test", time_archived: 123 }) + .run() + .pipe(Effect.orDie) + yield* db + .insert(WorkflowTable) + .values({ id: "dag_live", project_id: ProjectV2.ID.make("proj_sweep"), session_id: "ses_live", title: "live", status: "running", config: "{}", seq: 0 }) + .run() + .pipe(Effect.orDie) + + yield* seedAggregate("ses_live") + yield* seedAggregate("ses_archived") + yield* seedAggregate("dag_live") + yield* seedAggregate("ses_zombie") + yield* seedAggregate("dag_zombie") + + const removed = yield* sweep.sweepOnce() + expect(removed).toBe(2) + + const survivors = yield* remainingAggregates(["ses_live", "ses_archived", "dag_live", "ses_zombie", "dag_zombie"]) + expect(survivors.map((row) => row.aggregate).sort()).toEqual(["dag_live", "ses_archived", "ses_live"]) + // Read models of live/archived aggregates are untouched. + expect((yield* db.select().from(SessionTable).where(eq(SessionTable.id, SessionSchema.ID.make("ses_live"))).all().pipe(Effect.orDie)).length).toBe(1) + expect((yield* db.select().from(SessionTable).where(eq(SessionTable.id, SessionSchema.ID.make("ses_archived"))).all().pipe(Effect.orDie)).length).toBe(1) + expect((yield* db.select().from(WorkflowTable).where(eq(WorkflowTable.id, "dag_live")).all().pipe(Effect.orDie)).length).toBe(1) + }).pipe(Effect.provide(testLayer)), + ) + }) + + test("a repeated pass finds nothing to remove", async () => { + await Effect.runPromise( + Effect.gen(function* () { + const sweep = yield* EventResidueSweep.Service + expect(yield* sweep.sweepOnce()).toBe(0) + }).pipe(Effect.provide(testLayer)), + ) + }) + + // Deterministic TOCTOU regression: a read model recreated between candidate + // selection and deletion (the concurrent replay/publish race) survives the + // guarded delete. Uses the sweep's own select/remove seam instead of sleeps. + test("a read model recreated after candidate selection survives the guarded delete", async () => { + await Effect.runPromise( + Effect.gen(function* () { + yield* seedProject + const { db } = yield* Database.Service + + yield* seedAggregate("ses_zombie") + yield* seedAggregate("dag_zombie") + + const candidates = yield* EventResidueSweep.selectResidues(db).pipe(Effect.orDie) + expect(candidates.map((row) => row.aggregate_id).sort()).toEqual(["dag_zombie", "ses_zombie"]) + + // Concurrent replay/publish lands here: the session read model is + // re-materialized after selection, before deletion. + yield* db + .insert(SessionTable) + .values({ + id: SessionSchema.ID.make("ses_zombie"), + project_id: ProjectV2.ID.make("proj_sweep"), + slug: "reanimated", + directory: "/project", + title: "reanimated", + version: "test", + }) + .run() + .pipe(Effect.orDie) + + expect(yield* EventResidueSweep.removeResidue(db, "ses_zombie").pipe(Effect.orDie)).toBe(false) + // The still-zombie aggregate is removed, its event rows cascading with it. + expect(yield* EventResidueSweep.removeResidue(db, "dag_zombie").pipe(Effect.orDie)).toBe(true) + + const survivors = yield* remainingAggregates(["ses_zombie", "dag_zombie"]) + expect(survivors.map((row) => row.aggregate)).toEqual(["ses_zombie"]) + expect( + (yield* db.select({ id: EventTable.id }).from(EventTable).where(eq(EventTable.aggregate_id, "ses_zombie")).all().pipe(Effect.orDie)) + .length, + ).toBe(1) + expect( + yield* db.select({ id: EventTable.id }).from(EventTable).where(eq(EventTable.aggregate_id, "dag_zombie")).all().pipe(Effect.orDie), + ).toEqual([]) + }).pipe(Effect.provide(testLayer)), + ) + }) +}) diff --git a/packages/core/test/npm.test.ts b/packages/core/test/npm.test.ts index f66734962e..7818ddfb6b 100644 --- a/packages/core/test/npm.test.ts +++ b/packages/core/test/npm.test.ts @@ -10,6 +10,12 @@ import { PluginSdk } from "@opencode-ai/core/plugin-sdk" import { EffectFlock } from "@opencode-ai/core/util/effect-flock" import { tmpdir } from "./fixture/tmpdir" +// CI runners blackhole the registry audit POST that arborist.reify issues, +// hanging these fixtures past Bun's default 5000ms test timeout. Audit is +// incidental to what these tests assert; NpmConfig.load spreads process.env +// into Arborist, so disabling it here keeps reify hermetic. +process.env.npm_config_audit = "false" + const win = process.platform === "win32" const writePackage = (dir: string, pkg: Record) => diff --git a/packages/opencode/src/cli/cmd/db.ts b/packages/opencode/src/cli/cmd/db.ts index 9e7e37e18e..5a3c980169 100644 --- a/packages/opencode/src/cli/cmd/db.ts +++ b/packages/opencode/src/cli/cmd/db.ts @@ -1,9 +1,10 @@ import type { Argv } from "yargs" import { spawn } from "child_process" import { Database } from "@opencode-ai/core/database/database" +import { Vacuum } from "@opencode-ai/core/database/vacuum" import { Effect } from "effect" import { sql } from "drizzle-orm" -import { effectCmd } from "../effect-cmd" +import { effectCmd, fail } from "../effect-cmd" const QueryCommand = effectCmd({ command: "$0 [query]", @@ -51,12 +52,43 @@ const PathCommand = effectCmd({ }), }) +// #524: the ONLY conversion path for legacy auto_vacuum=NONE databases. +// Deliberate invocation by design — the target file must be named explicitly +// with --db, never a default path; pair it with `opencode db path`. Refuses +// anything that is not an existing regular file (a typo must not create a +// database). Converts FULL -> VACUUM -> wal_checkpoint(TRUNCATE) outside any +// startup path and fails nonzero unless the readback is FULL. +const VacuumCommand = effectCmd({ + command: "vacuum", + describe: "convert a database file to full auto_vacuum (FULL -> VACUUM -> truncate WAL)", + instance: false, + builder: (yargs: Argv) => { + return yargs.option("db", { + type: "string", + demandOption: true, + describe: "path to the SQLite database file (print the default with `opencode db path`)", + }) + }, + handler: Effect.fn("Cli.db.vacuum")(function* (args: { db: string }) { + const result = yield* Vacuum.convertToFull(args.db).pipe( + Effect.catch((cause) => + cause._tag === "VacuumRefused" + ? fail(cause.message) + : fail( + `vacuum failed for ${args.db} — close running opencode processes that use this file and retry (${cause.message})`, + ), + ), + ) + console.log(`auto_vacuum=${result.autoVacuum}`) + }), +}) + export const DbCommand = effectCmd({ command: "db", describe: "database tools", instance: false, builder: (yargs: Argv) => { - return yargs.command(QueryCommand).command(PathCommand).demandCommand() + return yargs.command(QueryCommand).command(PathCommand).command(VacuumCommand).demandCommand() }, handler: Effect.fn("Cli.db")(function* () {}), }) diff --git a/packages/opencode/src/effect/app-runtime.ts b/packages/opencode/src/effect/app-runtime.ts index 435b173794..24a186854c 100644 --- a/packages/opencode/src/effect/app-runtime.ts +++ b/packages/opencode/src/effect/app-runtime.ts @@ -61,6 +61,7 @@ import { DagStore } from "@opencode-ai/core/dag/store" import { DagLoop } from "@/dag/runtime/loop" import { DagSummaryPublisher } from "@/dag/runtime/summary-publisher" import { DagSupervisionSweep } from "@/dag/runtime/supervision-sweep" +import { EventResidueSweep } from "@opencode-ai/core/event/residue-sweep" import { Memory } from "@/memory/memory" export const AppLayer = Layer.mergeAll( @@ -138,6 +139,11 @@ export const AppLayer = Layer.mergeAll( // DagLoop it must NOT die with a per-directory instance teardown, or a // `running` node with dead supervision would rot forever. Layer.provideMerge(DagSupervisionSweep.defaultLayer), + // #524: default-on startup residue sweep for crash/in-flight zombie event + // aggregates (both read models absent). Host-level like the supervision + // sweep: one pass per process start, forked into the layer scope, + // soft-degrading — never blocks or fails startup. + Layer.provideMerge(EventResidueSweep.defaultLayer), Layer.provideMerge(SettingsHook.defaultLayer), ) diff --git a/packages/opencode/src/server/routes/instance/httpapi/server.ts b/packages/opencode/src/server/routes/instance/httpapi/server.ts index 0e7f278937..5bae37ef5c 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/server.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/server.ts @@ -50,6 +50,7 @@ import { Storage } from "@/storage/storage" import { Goal } from "@/goal/goal" import { GoalLoop } from "@/goal/loop" import { DagSupervisionSweep } from "@/dag/runtime/supervision-sweep" +import { EventResidueSweep } from "@opencode-ai/core/event/residue-sweep" import { SettingsHook } from "@/hook/settings" import { HookRewakeLive } from "@/hook/rewake-live" import { SessionHooks } from "@/hook/session-hooks" @@ -318,6 +319,12 @@ export const app = LayerNode.group([ // conditional projector UPDATE), so the duplicate is safe — see the sweep // header's multi-host convergence notes. DagSupervisionSweep.node, + // EventResidueSweep (#524): default-on startup residue sweep for crash/ + // in-flight zombie event aggregates. Same app-graph-level placement + // rationale as DagSupervisionSweep above — the desktop sidecar and headless + // serving processes build this node graph without AppLayer, and the sweep + // is idempotent (a second pass in AppLayer processes removes nothing). + EventResidueSweep.node, ]) export function createRoutes( diff --git a/packages/opencode/src/session/session.ts b/packages/opencode/src/session/session.ts index 81c79c0e47..ddfe22e664 100644 --- a/packages/opencode/src/session/session.ts +++ b/packages/opencode/src/session/session.ts @@ -38,7 +38,7 @@ import { SessionID, MessageID, PartID } from "./schema" import type { Provider } from "@/provider/provider" import { Global } from "@opencode-ai/core/global" -import { Effect, Layer, Option, Context, Schema, Types } from "effect" +import { Cause, Effect, Layer, Option, Context, Schema, Types } from "effect" import { NonNegativeInt, optionalOmitUndefined } from "@opencode-ai/core/schema" import { RuntimeFlags } from "@/effect/runtime-flags" import { ProviderV2 } from "@opencode-ai/core/provider" @@ -709,6 +709,11 @@ export const layer: Layer.Layer< // the startup orphan-pending sweep (cancel is not a valid transition // from pending). const workflows = yield* dag.store.listBySession(sessionID).pipe(Effect.orDie) + // #524: capture EVERY related dag aggregate before the Deleted publish — + // the projector's session-row delete FK-cascades the workflow rows away + // inside the publish transaction, so listBySession after it returns [] + // and terminal aggregates would be stranded as event-store residue. + const dagIDs = workflows.map((workflow) => workflow.id) for (const workflow of workflows) { // oxlint-disable-next-line typescript-eslint/no-unsafe-type-assertion -- WorkflowRow.status is a plain string column whose values are the WorkflowStatus literals (only the projector writes it, via validated transitions). if (isWorkflowTerminalStatus(workflow.status as never)) continue @@ -734,6 +739,24 @@ export const layer: Layer.Layer< // comes LAST, after every cleanup step above. yield* events.publish(SessionV1.Event.Deleted, { sessionID, info: session }) yield* events.remove(sessionID) + // #524: scrub the related dag event aggregates after the session + // aggregate — terminal workflows included (the cancel loop above skips + // them). Soft-degrading like the EventResidueSweep sibling: a failed + // scrub is logged and leaves the aggregate for the startup residue + // sweep, never fails the removal. Interruption is preserved, not + // degraded into a warning (same hasInterrupts re-raise discipline). + yield* Effect.forEach( + dagIDs, + (dagID) => + events.remove(dagID).pipe( + Effect.catchCause((cause) => + Cause.hasInterrupts(cause) + ? Effect.interrupt + : Effect.logWarning("dag aggregate scrub failed during session remove", { sessionID, dagID, cause }), + ), + ), + { discard: true }, + ) } catch (error) { yield* Effect.logError("failed to remove session", { sessionID, error }) } diff --git a/packages/opencode/test/cli/help/__snapshots__/help-snapshots.test.ts.snap b/packages/opencode/test/cli/help/__snapshots__/help-snapshots.test.ts.snap index 25a4c38f90..dcc795f2bf 100644 --- a/packages/opencode/test/cli/help/__snapshots__/help-snapshots.test.ts.snap +++ b/packages/opencode/test/cli/help/__snapshots__/help-snapshots.test.ts.snap @@ -394,6 +394,8 @@ database tools Commands: opencode db [query] open an interactive sqlite3 shell or run a query [default] opencode db path print the database path + opencode db vacuum convert a database file to full auto_vacuum (FULL -> VACUUM -> truncate + WAL) Positionals: query SQL query to execute [string] @@ -625,3 +627,18 @@ Options: --log-level log level [string] [choices: "DEBUG", "INFO", "WARN", "ERROR"] --pure run without external plugins [boolean]" `; + +exports[`opencode CLI help-text snapshots every documented command emits stable help text: opencode db vacuum --help 1`] = ` +"opencode db vacuum + +convert a database file to full auto_vacuum (FULL -> VACUUM -> truncate WAL) + +Options: + -h, --help show help [boolean] + -v, --version show version number [boolean] + --print-logs print logs to stderr [boolean] + --log-level log level [string] [choices: "DEBUG", "INFO", "WARN", "ERROR"] + --pure run without external plugins [boolean] + --db path to the SQLite database file (print the default with \`opencode db path\`) + [string] [required]" +`; diff --git a/packages/opencode/test/cli/help/help-snapshots.test.ts b/packages/opencode/test/cli/help/help-snapshots.test.ts index 3a14d0d7ec..bdd2af786d 100644 --- a/packages/opencode/test/cli/help/help-snapshots.test.ts +++ b/packages/opencode/test/cli/help/help-snapshots.test.ts @@ -83,6 +83,7 @@ const SUBCOMMANDS = [ ["github", "install"], ["github", "run"], ["db", "path"], + ["db", "vacuum"], ] as const // Fixed wrap width so a developer's terminal doesn't affect snapshots. diff --git a/packages/opencode/test/dag/dag-replay-idempotency.test.ts b/packages/opencode/test/dag/dag-replay-idempotency.test.ts index cf9afe40ac..8b985ffb7f 100644 --- a/packages/opencode/test/dag/dag-replay-idempotency.test.ts +++ b/packages/opencode/test/dag/dag-replay-idempotency.test.ts @@ -1,12 +1,14 @@ import { describe, expect, it } from "bun:test" -import { DateTime, Effect, Layer } from "effect" -import { sql } from "drizzle-orm" +import { Cause, DateTime, Effect, Exit, Layer } from "effect" +import { eq, sql } from "drizzle-orm" import { Database } from "@opencode-ai/core/database/database" import { EventV2 } from "@opencode-ai/core/event" import { EventTable, EventSequenceTable } from "@opencode-ai/core/event/sql" import { DagProjector } from "@opencode-ai/core/dag/projector" import { DagStore } from "@opencode-ai/core/dag/store" -import { DagEvent } from "@opencode-ai/schema/dag-event" +import { DagEvent, DagID } from "@opencode-ai/schema/dag-event" +import { ProjectID } from "@opencode-ai/schema/project-id" +import { SessionID } from "@opencode-ai/schema/session-id" import { Project } from "@opencode-ai/core/project" import { ProjectTable } from "@opencode-ai/core/project/sql" import { SessionTable } from "@opencode-ai/core/session/sql" @@ -181,4 +183,45 @@ describe("DagProjector: replay idempotency", () => { }).pipe(Effect.provide(projectorLayer)) as Effect.Effect, ) }) + + it("re-materializing a dag aggregate without its session row dies on the workflow FK (#524 zombie shape)", async () => { + await Effect.runPromise( + Effect.gen(function* () { + yield* setupFKs() + const { db } = yield* Database.Service + const events = yield* EventV2.Service + const sessionID = SessionID.make("ses_doomed") + + yield* db + .insert(SessionTable) + .values({ id: sessionID, project_id: Project.ID.global, slug: "doomed", directory: "/project", title: "doomed", version: "test" }) + .run() + .pipe(Effect.orDie) + const dagID = DagID.make("dag_replay_zombie") + yield* events.publish(DagEvent.WorkflowCreated, { dagID, projectID: ProjectID.global, sessionID, title: "zombie-test", config: "{}", status: "pending", timestamp: ts(0) }) + yield* events.publish(DagEvent.WorkflowCompleted, { dagID, durationMs: 0, timestamp: ts(1) }) + + const serialized = yield* serializeAndWipe(dagID) + yield* db.delete(SessionTable).where(eq(SessionTable.id, sessionID)).run().pipe(Effect.orDie) + + // Crash/in-flight zombie shape (#524): the aggregate's session row is + // gone, so the WorkflowCreated read-model INSERT dies on the + // workflow.session_id FK — a wiped dag aggregate whose session was + // removed can never be re-materialized. This is why Session.remove + // scrubs the dag event aggregates instead of relying on replay. + const exit = yield* events.replayAll(serialized).pipe(Effect.exit) + expect(Exit.isFailure(exit)).toBe(true) + if (Exit.isFailure(exit)) expect(Cause.pretty(exit.cause)).toContain("FOREIGN KEY constraint failed") + + // The FK death aborts the replay transaction — no partial-commit garbage. + const residue = yield* db + .select({ id: EventTable.id }) + .from(EventTable) + .where(eq(EventTable.aggregate_id, dagID)) + .all() + .pipe(Effect.orDie) + expect(residue).toEqual([]) + }).pipe(Effect.provide(projectorLayer)), + ) + }) }) diff --git a/packages/opencode/test/mcp/fixtures/process-tree-probe.ts b/packages/opencode/test/mcp/fixtures/process-tree-probe.ts index 9806e2a5f7..1e91aaebd4 100644 --- a/packages/opencode/test/mcp/fixtures/process-tree-probe.ts +++ b/packages/opencode/test/mcp/fixtures/process-tree-probe.ts @@ -80,5 +80,5 @@ const result = await Effect.runPromise( ).pipe(Effect.scoped, Effect.provide(MCP.defaultLayer)), ) -console.log(JSON.stringify(result)) -if (!result.ok || !result.rootDead || !result.childDead) process.exit(1) +await Bun.write(Bun.stdout, `${JSON.stringify(result)}\n`) +process.exit(result.ok && result.rootDead && result.childDead ? 0 : 1) diff --git a/packages/opencode/test/server/httpapi-residue-sweep-wiring.test.ts b/packages/opencode/test/server/httpapi-residue-sweep-wiring.test.ts new file mode 100644 index 0000000000..68a5caee1c --- /dev/null +++ b/packages/opencode/test/server/httpapi-residue-sweep-wiring.test.ts @@ -0,0 +1,26 @@ +import { describe, expect } from "bun:test" +import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { EventResidueSweep } from "@opencode-ai/core/event/residue-sweep" +import { LayerNode } from "@opencode-ai/core/effect/layer-node" +import { Effect, Layer, Option } from "effect" +import { HttpApiApp } from "@/server/routes/instance/httpapi/server" +import { testEffect } from "../lib/effect" + +// #524 wiring regression: the default-on event residue sweep must reach every +// serving process. Mirrors httpapi-sweep-wiring.test.ts — the desktop sidecar +// and headless serve build this app node graph without AppLayer, so listing +// EventResidueSweep.node here (not just app-runtime.ts) is what makes the +// startup pass run on those paths. + +const appIt = testEffect( + Layer.mergeAll(LayerNode.buildLayer(HttpApiApp.app), CrossSpawnSpawner.defaultLayer), +) + +describe("server app graph event residue sweep wiring", () => { + appIt.instance("exposes EventResidueSweep.Service in the serving context", () => + Effect.gen(function* () { + const sweep = yield* Effect.serviceOption(EventResidueSweep.Service) + expect(Option.isSome(sweep)).toBe(true) + }), + ) +}) diff --git a/packages/opencode/test/session/session-remove-cleanup.test.ts b/packages/opencode/test/session/session-remove-cleanup.test.ts index 41e7488475..0212446324 100644 --- a/packages/opencode/test/session/session-remove-cleanup.test.ts +++ b/packages/opencode/test/session/session-remove-cleanup.test.ts @@ -1,12 +1,14 @@ import { describe, expect } from "bun:test" -import { Effect, Layer, Option } from "effect" -import { and, eq } from "drizzle-orm" +import { Cause, Context, Effect, Exit, Layer, Option } from "effect" +import { eq, inArray } from "drizzle-orm" import { Database } from "@opencode-ai/core/database/database" -import { EventTable } from "@opencode-ai/core/event/sql" import { EventV2 } from "@opencode-ai/core/event" -import { DagEvent } from "@opencode-ai/schema/dag-event" +import { LayerNode } from "@opencode-ai/core/effect/layer-node" +import { SessionProjector } from "@opencode-ai/core/session/projector" +import { EventSequenceTable, EventTable } from "@opencode-ai/core/event/sql" import { GoalOutcomeTable, GoalStateTable } from "@opencode-ai/core/goal/sql" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { EventV2Bridge } from "@/event-v2-bridge" import { Session as SessionNs } from "@/session/session" import { SessionAutomationLease } from "@/session/automation-lease" import { Goal } from "@/goal/goal" @@ -127,28 +129,29 @@ describe("Session.remove dag lease cleanup (GOAL-FP-01-06)", () => { }) expect((yield* dag.store.getWorkflow(dagID).pipe(Effect.orDie))?.status).toBe("running") + // #524 supersession: this pin used to observe the cancel through the + // durable dag.workflow.cancelled event row. Since Session.remove now + // scrubs the whole dag event aggregate AFTER the cancel transition (the + // transition itself is pinned by the dag lifecycle tests), the boundary + // observable is the absence of residue: the cancelled workflow leaves no + // read-model row and no event-store rows behind. yield* session.remove(sessionID) - // The workflow READ row is FK-cascaded away with the session row, so - // the cancellation contract observable here is the durable - // dag.workflow.cancelled event — the terminalization that stops the - // running DagLoop runtime (aborting child sessions and releasing the - // dag lease) and keeps the workflow out of the restart recovery scan. - const cancelledEvent = yield* db - .select() + const sequences = yield* db + .select({ aggregate: EventSequenceTable.aggregate_id }) + .from(EventSequenceTable) + .where(eq(EventSequenceTable.aggregate_id, dagID)) + .all() + .pipe(Effect.orDie) + expect(sequences).toEqual([]) + + const events = yield* db + .select({ aggregate: EventTable.aggregate_id }) .from(EventTable) - .where( - and( - eq(EventTable.aggregate_id, dagID), - eq(EventTable.type, EventV2.versionedType(DagEvent.WorkflowCancelled.type, 1)), - ), - ) - .get() + .where(eq(EventTable.aggregate_id, dagID)) + .all() .pipe(Effect.orDie) - // P2-B: toBeNull() was vacuous — drizzle .get() returns undefined for a - // missing row and `expect(undefined).not.toBeNull()` always passes. - // toBeDefined() actually pins the durable dag.workflow.cancelled event. - expect(cancelledEvent).toBeDefined() + expect(events).toEqual([]) // Recovery scan contract (dag/runtime/loop.ts adopts only // running/paused/stepping rows): the workflow must not be re-adoptable. @@ -157,3 +160,132 @@ describe("Session.remove dag lease cleanup (GOAL-FP-01-06)", () => { }), ) }) + +const workflowConfig = (name: string) => ({ + name, + nodes: [ + { + id: "n1", + name: "n1", + worker_type: "build", + depends_on: [], + required: true, + prompt_template: { inline: "do work" }, + }, + ], +}) + +describe("Session.remove dag aggregate scrub (#524)", () => { + it.instance("removes every related dag event aggregate including terminal workflows", () => + Effect.gen(function* () { + const session = yield* SessionNs.Service + const dag = yield* Dag.Service + const { db } = yield* Database.Service + + const info = yield* session.create({}) + const sessionID = info.id + const terminalDag = yield* dag.create({ + projectID: info.projectID, + sessionID, + title: "scrub-terminal", + config: workflowConfig("scrub-terminal"), + }) + // Terminal BEFORE remove: the pre-publish capture must include it even + // though the cancel loop skips terminal rows as already inert. + yield* dag.cancel(terminalDag) + const liveDag = yield* dag.create({ + projectID: info.projectID, + sessionID, + title: "scrub-live", + config: workflowConfig("scrub-live"), + }) + + const aggregateIDs = [terminalDag, liveDag, sessionID] + const pre = yield* db + .select({ aggregate: EventSequenceTable.aggregate_id }) + .from(EventSequenceTable) + .where(inArray(EventSequenceTable.aggregate_id, aggregateIDs)) + .all() + .pipe(Effect.orDie) + expect(new Set(pre.map((row) => row.aggregate)).size).toBe(3) + + yield* session.remove(sessionID) + + const sequences = yield* db + .select({ aggregate: EventSequenceTable.aggregate_id }) + .from(EventSequenceTable) + .where(inArray(EventSequenceTable.aggregate_id, aggregateIDs)) + .all() + .pipe(Effect.orDie) + expect(sequences).toEqual([]) + const events = yield* db + .select({ aggregate: EventTable.aggregate_id }) + .from(EventTable) + .where(inArray(EventTable.aggregate_id, aggregateIDs)) + .all() + .pipe(Effect.orDie) + expect(events).toEqual([]) + }), + ) +}) + +// #524 interrupt-contract regression: the per-dag scrub catchCause must +// preserve interruption (the EventResidueSweep sibling discipline) instead of +// degrading it into a logWarning. The stub bridge fails events.remove with a +// self-thrown interrupt cause — the only cause shape catchCause can +// intercept; external interrupts bypass it — for every aggregate EXCEPT the +// session's own, so any interrupt surfacing from session.remove can only +// originate from the dag scrub step. +function interruptingScrubBridgeNode(gate: { sessionID?: string }) { + return LayerNode.make( + Layer.effect( + EventV2Bridge.Service, + Effect.gen(function* () { + const bridge = Context.get(yield* Layer.build(EventV2Bridge.layer), EventV2Bridge.Service) + return EventV2Bridge.Service.of({ + ...bridge, + remove: (aggregateID) => + Effect.suspend(() => + gate.sessionID !== undefined && aggregateID !== gate.sessionID + ? Effect.interrupt + : bridge.remove(aggregateID), + ), + }) + }), + ), + [EventV2.node], + ) +} + +const scrubGate: { sessionID?: string } = {} +const scrubInterruptIt = testEffect( + Layer.mergeAll( + LayerNode.buildLayer(LayerNode.group([SessionNs.node, SessionProjector.node, Dag.node]), { + replacements: [LayerNode.replaceWithNode(EventV2Bridge.node, interruptingScrubBridgeNode(scrubGate))], + }), + CrossSpawnSpawner.defaultLayer, + ), +) + +describe("Session.remove dag aggregate scrub interrupt contract (#524)", () => { + scrubInterruptIt.instance("scrub interruption propagates out of remove instead of degrading to a warning", () => + Effect.gen(function* () { + const session = yield* SessionNs.Service + const dag = yield* Dag.Service + const info = yield* session.create({}) + const sessionID = info.id + yield* dag.create({ + projectID: info.projectID, + sessionID, + title: "scrub-interrupt", + config: workflowConfig("scrub-interrupt"), + }) + + scrubGate.sessionID = sessionID + const exit = yield* session.remove(sessionID).pipe(Effect.exit) + scrubGate.sessionID = undefined + + expect(Exit.isFailure(exit) && Cause.hasInterrupts(exit.cause)).toBe(true) + }), + ) +})