From 5cf1b95c870c75ba912e9c136126af217edde811 Mon Sep 17 00:00:00 2001 From: Mario Zechner Date: Mon, 31 Aug 2026 00:17:31 +0200 Subject: [PATCH] docs(agent): align SQLite ownership with session workers --- packages/agent/docs/harness.md | 33 +- packages/agent/docs/post-wp05-roadmap.md | 28 +- packages/agent/docs/values.md | 2 +- .../07-sqlite-host-ownership-live-forks.md | 370 ++++++++++++++++++ .../07-sqlite-ownership-fencing.md | 332 ---------------- .../session-backends/sqlite-node/README.md | 4 +- 6 files changed, 405 insertions(+), 364 deletions(-) create mode 100644 packages/agent/docs/work-packages/07-sqlite-host-ownership-live-forks.md delete mode 100644 packages/agent/docs/work-packages/07-sqlite-ownership-fencing.md diff --git a/packages/agent/docs/harness.md b/packages/agent/docs/harness.md index 59b420505..1f9eccfad 100644 --- a/packages/agent/docs/harness.md +++ b/packages/agent/docs/harness.md @@ -117,7 +117,7 @@ Every tool call has a result and nothing ran twice; without a committed checkpoi - **Exactly-once external effects** — hooks with side effects must be idempotent, keyed by operation id. - **Provider stream resumption** — the harness never reattaches to a provider stream; committed frames (§3.7) preserve the latest durable partial for recovery and reconnect display, and a settled response is persisted *completely* before anything classifies it. -- **Multiple writers** — one process per session; the serving layer routes accordingly and SQLite uses a fenced lease, with two correctness gaps tracked by WP07 (§1.7). Lanes cover the workload that looks like multi-writer. +- **Multiple writable owners** — exactly one host-assigned owner may hold a writable Session at a time; normally that owner is its Session worker, while the server may temporarily own a newly created or forked destination before handing it off. Storage backends do not enforce this host-lifecycle rule. Read-only repository work such as a SQLite source snapshot may overlap the worker (§1.7, §2.7). Lanes cover the workload that looks like multi-writer. - **Work scheduling** — the harness never creates platform alarms, scans repositories for abandoned sessions, leases hosted submissions, or promises an HTTP receipt; it reports durable waits through `drive` and the serving layer decides when to call again. - **Replication** — a session lives in one place. - **Durable write history** — values retain only current state, lists only until whole-list deletion; no API or table exposes replaced values or deleted elements. Test write-order assertions use an instrumented decorator around `commit()` (Part 9); production auditing belongs to telemetry (§5.8). @@ -148,7 +148,7 @@ WP00–WP06 are complete (Part 8): the operation graph and public lane runtime a - **T1 — telemetry (§5.8):** span vocabulary declared; production starts only the tool-hook span. RPC ingress has request-ID cancellation but no trace propagation. - **S3 — search (§2.8):** design only; the current `src/search/index.ts` skeleton conflicts with it and has no implementation. - **R11 — schema migrations (Part 7):** mechanism specified; activation-gated; no migration exists or is required. -- **WP07 — SQLite ownership (§1.7):** commit fencing and deletion each have a race; the backend cannot yet guarantee its intended lease contract. +- **WP07 — SQLite host ownership (§1.7, §2.7):** the backend still contains a writer lease contrary to the host-authoritative ownership rule; live worker-owned fork sources also need an explicit independent read-only snapshot path. WP07 removes the lease and fixes repository-local deletion reservation, physical identity, paths, and close draining. - **SQLite branch divergence (§2.6):** the current compaction-bounded algorithm can copy O(history) on an uncompacted branch, contrary to its bounded-prefix goal. - **H1 — contract/test closure:** public `OperationStatus` includes `"running"` but current observations produce only `"open"`/`"aborting"` (§5.4); the pre-rewrite abort contract bound `operation_abort` before resolving/signalling but current code signals first and binds recipients before releasing the line (§4.6); Part 9 remains the required conformance matrix, not a claim that every row has a dedicated test. - **Source declaration corrections:** `CommitResult.stats` and `SessionReader.getStats()` are implemented; the old inline declarations omitted them even though other old sections depended on post-commit totals (§1.4, §2.8). The old execution-block declarations also predate the current source shapes: standalone `streamHarnessAssistant` permits an absent `afterResponse`, while the durable Harness caller always supplies it; tool phases directly carry `AgentHarnessTool`, `toolContext`, and invocation capabilities and create the canonical result message after an immediate raw result (§5.7). These are source-shape corrections, not changes to the durable boundaries. @@ -334,7 +334,7 @@ List read semantics: ascending returns `seq > cursor.seq`, descending `seq < cur Recovery and execution reads must be index-driven and bounded: never infer state from an absent value (no history exists to fold). Exact dereference is allowed — current typed state may name a bounded set of entries and values, and an exact list address derived from current state may be read in bounded pages and reduced by its consumer (assistant frames use `reduceAssistantMessageFrames`, §3.7). Base restore never reads lists (§4.4). Public inventory/debugging APIs expose explicit limits/pagination through Session and Branch. -`close()` is idempotent: seal admission, reject later reads/commits on that instance, drain commits admitted before the seal, then release resources and the writer claim. Durable data reopens through the repository. +`close()` is idempotent: seal admission, reject later reads/commits on that instance, drain commits admitted before the seal, then release backend resources. Durable data reopens through the repository; writable-owner handoff belongs to the host lifecycle, not Storage. ## 1.6 Usage ledger @@ -368,9 +368,9 @@ Until J1 lands, deleted pending payloads, superseded state revisions, superseded ### SQLite -Backend: `packages/session-backends/sqlite-node`. **One database file per session is the default; a shared container is supported.** Without `databasePath` the repository creates one `{id}.sqlite` per Session (WP07 makes that filename mapping path-safe for arbitrary explicit ids); with `databasePath` any number of Sessions share one container. Every authoritative/projection/lease row is scoped by `session_id`; shared containers are a supported deployment mode, not an implementation detail to remove. SQLite's one-writer-per-file rule is insufficient in either mode: a fenced per-Session lease enforces one writer per Session. +Backend: `packages/session-backends/sqlite-node`. **One database file per session is the default; a shared container is supported.** Without `databasePath` the repository creates one `{id}.sqlite` name per Session (WP07 replaces direct interpolation with path-safe encoding for arbitrary explicit ids); with `databasePath` any number of Sessions share one container. Every authoritative and projection row is scoped by `session_id`; shared containers are a supported deployment mode, not an implementation detail to remove. SQLite supplies atomic transactions and coherent WAL snapshots, not Session ownership. -`001_initial.sql` (storage version 1), all scoped by `session_id`: +Target `001_initial.sql` after WP07 (storage version 1), all scoped by `session_id`; current source still has the lease table identified below as removal debt: ```sql entries(id, parent_id, seq, type, custom_type, timestamp, payload) WITHOUT ROWID; @@ -392,16 +392,17 @@ branch_meta(branch_id PRIMARY KEY, tip_entry_id, tip_seq, base_branch_id, base_s sessions(id, created_at, parent_session_id, storage_version, metadata, message_count, usage_payload, next_seq); -- one row per Session -writer_lease(session_id PRIMARY KEY, owner_id, fence, expires_at_ms); ``` Triggers enforce the shared entry/usage id namespace and ordered parent insertion at the storage level. No pre-WP01 format-4 SQLite file is supported; migration machinery belongs to R11. -One `commit()` is one SQL transaction: insert entries and ledger rows, replace/delete scalar values, insert/whole-list-delete list elements, maintain the branch index, bump session stats (`message_count`, aggregate `usage_payload`). Never update or delete an entry or ledger row; mutability is confined to values/lists, the branch index, stats, sequences, the catalog row, and leases. List paging is `SELECT seq, value FROM list_values WHERE namespace = ? AND key = ? AND seq > ? ORDER BY seq ASC LIMIT ?` (descending symmetric; omit the predicate without a cursor); assert via `EXPLAIN QUERY PLAN` that it uses the primary key with no temporary sort. +One `commit()` is one SQL transaction: insert entries and ledger rows, replace/delete scalar values, insert/whole-list-delete list elements, maintain the branch index, bump session stats (`message_count`, aggregate `usage_payload`). Never update or delete an entry or ledger row; mutability is confined to values/lists, the branch index, stats, sequences, and the catalog row. List paging is `SELECT seq, value FROM list_values WHERE namespace = ? AND key = ? AND seq > ? ORDER BY seq ASC LIMIT ?` (descending symmetric; omit the predicate without a cursor); assert via `EXPLAIN QUERY PLAN` that it uses the primary key with no temporary sort. **Every transaction that may write must open with `BEGIN IMMEDIATE`.** A deferred `BEGIN` that reads before writing takes a read snapshot and must later upgrade to the write lock; if another writer committed in between, SQLite fails the upgrade — and `busy_timeout` cannot rescue it, because waiting cannot refresh a stale snapshot; the only recovery is rollback and full retry. Every commit reads the session row's `next_seq` before writing, so a read precedes a write in every writing transaction; branch creation (§2.6) also reads the newest compaction before inserting. Coherent read-only snapshot transactions — fork capture (§2.7) — may use a deferred `BEGIN` read transaction; they never upgrade to a write. The old blanket wording covered every transaction and conflicted with its own read-only fork rule; this narrows the rule to the source behavior without weakening any write path. -**`writer_lease` enforces the single-writer rule.** WAL lets two processes alternate writes to one file — exactly the interleaving the design forbids — so per-session files do not remove the need for the lease. It is expiring fenced ownership: `open()` acquires the claim, storage renews it on appends and while idle, and close stops renewal after the queue drains and deletes only its matching `(owner_id, fence)` pair, so a stale owner cannot release the replacement that succeeded it. This makes one-process-per-session enforced rather than a convention. Memory and JSONL have no equivalent and rely on process ownership; a JSONL session opened twice is corrupt and undetected. Two known lease races — renewal in a separate transaction before the data transaction, and check-then-delete in `repo.delete()` — are correctness debt owned by WP07. Atomicity itself needs no special handling: WAL frames become visible only when the commit record lands, so a concurrent reader sees none of a transaction's writes or all of them. +**Session ownership is host-authoritative.** Exactly one worker normally owns a writable Session; create/fork administration may own a destination only until it closes that Session and hands metadata to the worker. Memory, JSONL, and SQLite do not detect a second process opening the same Session for writes; bypassing the server/worker lifecycle is a trusted-host defect. A repository still rejects duplicate writable handles it owns in one process. The current SQLite backend's lease table, claims, renewal timer, and commit callback contradict this rule and are removed by WP07 rather than repaired. + +Read-only access is different: WAL permits the server's repository to capture a live worker-owned source while the worker continues committing. The source connection uses one deferred read transaction and never upgrades or claims writable authority; it sees every source transaction wholly before or wholly after its snapshot boundary. WAL frames become visible only when the commit record lands, so no fork sees part of a commit. Each physical segment of `scanBranch` uses one JOIN (§2.6 combines segment ranges): @@ -534,7 +535,7 @@ type ForkOptions = | { scope: "tree"; id?: string }; ``` -Branch scope requires source `main`, copies one selected path, and creates destination Branch `main` at that tip; `entryId` defaults to the source main tip, `position` to including the selected entry. Tree scope copies the whole tree and every Branch tip; a branchless source produces a branchless destination. A configured source AgentLane copies its total configuration and receives fresh idle lane state **together iff** it is configured; data-only Branches remain data-only. Operation state, pending content/progress/frames, result records, and usage rows never copy; destination usage starts at zero. Session name copies; labels copy only with their target; copied entries retain ids; destination metadata records the source id as `parentSessionId`. Memory/JSONL serialize snapshot capture with commits; SQLite uses one read transaction. Code that must admit a commit before starting the independently queued fork snapshot uses keyless `beginMutation()`, invokes `commit()`, starts the fork after admission, and calls `end()` in `finally`; storage queue ordering chooses one coherent boundary. +Branch scope requires source `main`, copies one selected path, and creates destination Branch `main` at that tip; `entryId` defaults to the source main tip, `position` to including the selected entry. Tree scope copies the whole tree and every Branch tip; a branchless source produces a branchless destination. A configured source AgentLane copies its total configuration and receives fresh idle lane state **together iff** it is configured; data-only Branches remain data-only. Operation state, pending content/progress/frames, result records, and usage rows never copy; destination usage starts at zero. Session name copies; labels copy only with their target; copied entries retain ids; destination metadata records the source id as `parentSessionId`. Memory/JSONL serialize snapshot capture with commits. SQLite uses two paths: a source open in the same repository queues its snapshot on that source Storage, preserving the admitted-commit ordering seam; a source owned by another process — including a live Session worker — opens through an independent read-only connection and one deferred read transaction, which may overlap later WAL commits while observing one whole committed boundary. Code that must admit a commit before starting the same-repository fork snapshot uses keyless `beginMutation()`, invokes `commit()`, starts the fork after admission, and calls `end()` in `finally`; storage queue ordering chooses the boundary. ## 2.8 Session and repository boundary @@ -557,7 +558,7 @@ A repository creates only metadata/header/catalog state: no Branch, configuratio Search is a **standalone service with its own store**; the repository knows nothing about it and exposes no search methods. A sync utility consumes `repo.list()` and read-only session opens to feed the index store; applications construct the service, run sync at startup or on a schedule, wire the notify utility to their event stream for freshness, query the service directly, and call `search.remove()` alongside `repo.delete()` (or leave stale rows to the next reconciliation). Callers join metadata and fetch entries through the repository they already hold. Draft interfaces: `SessionSearchHit { sessionId, entryId }`; `SessionSearchOptions { entryTypes?, limit?, signal? }`; `SessionSearch.search(text, options?): AsyncIterable`; `SessionSearchService { searchSessions({ text, limit? }): Promise; searchEntries?: SessionSearch; remove(sessionId); close() }` (`limit` counts sessions; `SessionSearchResult { sessionId }`; display services may extend hits/results with `timestamp`, `snippet`, `score`, `top`); catch-up targets implement `SessionSearchSyncTarget { getCursor(sessionId, storeGeneration), indexBatch(batch), remove(sessionId) }` with `SearchIndexBatch { sessionId, storeGeneration, fromSeq, toSeq, entries: { entryId, seq, text, timestamp }[] }`. -**Indexing is pull-based; events are only hints.** The store keeps a durable cursor per session — the highest entry `seq` indexed. Sync enumerates sessions via the repository (old, new, copied files alike), reads `scanEntries({ fromSeq: cursor + 1 })`, indexes message-entry text idempotently per `(sessionId, entryId)`, and advances the cursor in the same store transaction; a crash mid-batch re-indexes into the same state, and years of existing sessions catch up with the same loop. Notify carries no content — a poke triggering a debounced pull; a lost poke is caught by the next sweep. The index is a rebuildable projection with zero authority; indexing failures never affect the harness or commits. Reading a session another process writes is legal (the lease gates writers; WAL gives cross-process snapshot reads), but a sweep may skip lease-held sessions since notify covers the hot ones. The precise rewrite (§2.9) may renumber seqs, so cursors key on `(sessionId, storeGeneration)`; the rewrite bumps a generation counter and a mismatch triggers full re-index. Reference implementation: one standalone SQLite database — an FTS5 table over `(session_id, entry_id, text)` plus the cursor table — working unchanged over JSONL session files; several processes may share it (WAL, `busy_timeout`, `BEGIN IMMEDIATE`, idempotent rows, monotonic cursor updates; writers serialize). +**Indexing is pull-based; events are only hints.** The store keeps a durable cursor per session — the highest entry `seq` indexed. Sync enumerates sessions via the repository (old, new, copied files alike), reads `scanEntries({ fromSeq: cursor + 1 })`, indexes message-entry text idempotently per `(sessionId, entryId)`, and advances the cursor in the same store transaction; a crash mid-batch re-indexes into the same state, and years of existing sessions catch up with the same loop. Notify carries no content — a poke triggering a debounced pull; a lost poke is caught by the next sweep. The index is a rebuildable projection with zero authority; indexing failures never affect the harness or commits. Reading a Session its worker writes is legal through the backend's read-only path: host lifecycle prevents a second writable owner, and WAL gives cross-process snapshot reads. The precise rewrite (§2.9) may renumber seqs, so cursors key on `(sessionId, storeGeneration)`; the rewrite bumps a generation counter and a mismatch triggers full re-index. Reference implementation: one standalone SQLite database — an FTS5 table over `(session_id, entry_id, text)` plus the cursor table — working unchanged over JSONL session files; several processes may share it (WAL, `busy_timeout`, `BEGIN IMMEDIATE`, idempotent rows, monotonic cursor updates; writers serialize). **Open question — metadata filtering.** Coding-agent's resume flow filters by `cwd`; other repositories have no cwd concept, and search options are deliberately generic. Candidates: (a) typed filter passthrough (service generic over each repo's filter vocabulary); (b) pre-restrict via the repo's own listing, passing a possibly huge candidate id set; (c) post-filter in the app — **unsound**, filtering after ranked `limit` drops results; (d) index chosen metadata fields at sync time and filter natively, coupling the service to those fields and requiring re-sync when they change. Settled with S3. @@ -1230,13 +1231,13 @@ Telemetry attributes may contain declared ids, names, counts, durations, statuse # Part 6 — Future: partitioned retention (Postgres) -**Informative; no normative rule.** Memory, JSONL, and SQLite never partition and never delete entries or usage rows (§1.2); no core rule references this part. It records why §1.2's identity choices suffice for a possible Postgres deployment with TTL retention: UUIDv7 sorts bytewise in time order, so entries, the usage ledger, and `branch_entries` can use `PARTITION BY RANGE` on the id with period-boundary UUIDs as bounds and no partition column, while values, `branch_meta`, stats, leases, and sessions stay in a hot unpartitioned catalog. Dropping a period requires an online pre-pass repairer (reparent edges crossing into the period, null dormant tips via value-seq CAS, force-expire open operations through the §3.13 terminal transaction under exclusive administrative ownership, uuid-range-delete labels), then one transactional lock barrier around delta repair plus plain `DETACH PARTITION`, so every commit sees either the fully attached period or a fully repaired store without it. A `DEFAULT` partition absorbs stray inserts whose ids predate every attached partition and is never dropped. A backend admitting an external repairer must perform value reads and CAS checks inside the commit transaction; shipping single-writer backends need no such rule. Retention policy, period granularity, and partition-count limits stay unspecified until the backend is real. +**Informative; no normative rule.** Memory, JSONL, and SQLite never partition and never delete entries or usage rows (§1.2); no core rule references this part. It records why §1.2's identity choices suffice for a possible Postgres deployment with TTL retention: UUIDv7 sorts bytewise in time order, so entries, the usage ledger, and `branch_entries` can use `PARTITION BY RANGE` on the id with period-boundary UUIDs as bounds and no partition column, while values, `branch_meta`, stats, and sessions stay in a hot unpartitioned catalog. Dropping a period requires an online pre-pass repairer (reparent edges crossing into the period, null dormant tips via value-seq CAS, force-expire open operations through the §3.13 terminal transaction under exclusive administrative ownership, uuid-range-delete labels), then one transactional lock barrier around delta repair plus plain `DETACH PARTITION`, so every commit sees either the fully attached period or a fully repaired store without it. A `DEFAULT` partition absorbs stray inserts whose ids predate every attached partition and is never dropped. A backend admitting an external repairer must perform value reads and CAS checks inside the commit transaction; shipping single-writer backends need no such rule. Retention policy, period granularity, and partition-count limits stay unspecified until the backend is real. # Part 7 — Schema evolution **R11 status: mechanism specified, not implemented; activation-gated.** No format-4 migration exists or is required: Memory is current-only, JSONL and SQLite reject unsupported storage versions, and SQLite runs only idempotent `001_initial.sql`. R11 becomes required immediately before the first incompatible durable change after format 4 stabilizes; format 4 is still WIP and pre-stabilization shape changes happen in place without migrations. -**Problem and why it is small here.** Durability snapshots in-flight state shaped like *today's* state machine; ship a different machine and old durable state still exists mid-run. Migration cost is proportional to what must convert: entries and usage rows (years) cannot be rewritten and must stay read-compatible; lane/semantic values are a few per lane; `pi.op.*` exists only for open operations (usually zero); `pi.pending.entry` holds queued items plus staged tool outcomes; `pi.pending.tool_output` only optional open-call checkpoints; `pi.pending.assistant_frame` only open-response frames (usually zero). With no history retained, the entire mutable surface is a few dozen current values/lists, and the fenced single-writer lease (§1.7) gives the opening process exclusive ownership — migrate-on-open with no concurrency story. +**Problem and why it is small here.** Durability snapshots in-flight state shaped like *today's* state machine; ship a different machine and old durable state still exists mid-run. Migration cost is proportional to what must convert: entries and usage rows (years) cannot be rewritten and must stay read-compatible; lane/semantic values are a few per lane; `pi.op.*` exists only for open operations (usually zero); `pi.pending.entry` holds queued items plus staged tool outcomes; `pi.pending.tool_output` only optional open-call checkpoints; `pi.pending.assistant_frame` only open-response frames (usually zero). With no history retained, the entire mutable surface is a few dozen current values/lists, and the host assigns one writable owner before migration starts — migrate-on-open has no concurrent writer. **Mechanism: storage version plus migrate-on-open.** One session-level `storageVersion` lives in the catalog or header. A version number beats versioned namespace suffixes (`pi.lane.state.v2`): one number to check, chained `v1→v2→v3` migrations, no probing of historical namespace names, stable address components for point lookups. @@ -1249,11 +1250,11 @@ open session: version > current → refuse to open (older binary, newer session) ``` -Chained migrations run under the writer lease before `open()` returns. Each step commits its conversions and version bump atomically, so a crash mid-chain resumes at the recorded version; conversions must be idempotent over already-converted values, which plain field mappings naturally are. +Chained migrations run under exclusive host-assigned writable ownership before `open()` returns. Each step commits its conversions and version bump atomically, so a crash mid-chain resumes at the recorded version; conversions must be idempotent over already-converted values, which plain field mappings naturally are. JSONL has one wrinkle in each direction: when R11 adds migrations, replay must decode exactly the older-version value/list records the migration names, because pre-migration bytes remain in the file; a migration then triggers snapshot compaction (J1), whose temp-file-and-rename persists the new header version atomically and retires the old bytes. Between crash and compaction, version-specific decoding plus idempotent conversion keep the intermediate state harmless. None of this adds compatibility for the pre-WP01 WIP format-4 spelling. Legacy format 3 predates `storageVersion`; it normalizes through Appendix B on load and receives the current version with its first format-4 write. -**Migrations are total.** Value conversion is a field mapping; a state-machine shape change is more — an old `pi.op.state` mid-phase may have no field-by-field equivalent in the new machine. A vN→vN+1 migration translates every stored value/list: lane/semantic values, `pi.pending.entry`, optional `pi.pending.tool_output`, invocation memos, and open operations' `pi.op.meta`/`pi.op.state` included (a migration adding `outcome_ready`, for example, must distinguish staged finalized tool results from still-uncertain effects). The author of a state-machine change writes the mapping for every reachable old state in the same change; a state with no natural successor maps to an explicit safe choice — no force-settle path or silent partial escape hatch. This is tractable because migration runs at open under the writer lease over quiescent state: no task running, no effect in flight, every `pi.op.state` exactly what some transaction committed — a pure function over a small, fully enumerable, fully typed set of values. +**Migrations are total.** Value conversion is a field mapping; a state-machine shape change is more — an old `pi.op.state` mid-phase may have no field-by-field equivalent in the new machine. A vN→vN+1 migration translates every stored value/list: lane/semantic values, `pi.pending.entry`, optional `pi.pending.tool_output`, invocation memos, and open operations' `pi.op.meta`/`pi.op.state` included (a migration adding `outcome_ready`, for example, must distinguish staged finalized tool results from still-uncertain effects). The author of a state-machine change writes the mapping for every reachable old state in the same change; a state with no natural successor maps to an explicit safe choice — no force-settle path or silent partial escape hatch. This is tractable because migration runs at open under exclusive host-assigned ownership over quiescent state: no task running, no effect in flight, every `pi.op.state` exactly what some transaction committed — a pure function over a small, fully enumerable, fully typed set of values. Address and list rules (§1.3, §1.4) extend the discipline: a bound address's namespace, key grammar, and kind are static for one storage version — changing any component or value↔list kind is an explicit migration, storage never infers or coerces kind, changing the TypeScript value shape requires a total value migration when old values are incompatible, and adding a new address with no stored value rewrites nothing. A list migration pages current elements in sequence order and either maps values preserving each element's `seq` or deletes the whole key — never loading an unbounded list at once. A migration changing `AssistantMessageFrame` shape must map every surviving element or explicitly delete the whole list, leaving `effect_pending` recovery with no partial; it must never infer completion from legacy frames. @@ -1274,11 +1275,11 @@ Workflow: keep a future package's row here until actionable; move exact files/te | WP04 | complete | Synchronous `emitBatch` publication; Session owns committed lane publication. | [Mutation publication and event delivery](work-packages/04-mutation-publication.md) | | WP05 | complete | The total direct durable graph, public/replicated lane surfaces, immutable results, atomic boundaries, cancellation reconciliation, lane-safe provider identity. M11 is its only recorded follow-up. | [Direct durable drive](work-packages/05-direct-durable-drive.md) | | WP06 | complete | Separated Session, Branch, AgentLane, AgentHarness; one keyless Session mutation line. | [Session, Branch, Lane separation](work-packages/06-session-branch-lane-separation.md) | -| WP07 | planned | SQLite ownership fencing: atomic lease assertion with commits, exclusive deletion, path/source identity, close draining. | [SQLite ownership fencing](work-packages/07-sqlite-ownership-fencing.md) | +| WP07 | planned | Remove SQLite storage-layer ownership; support live read-only source forks; fix deletion reservation, path/source identity, and close draining. | [SQLite host ownership and live forks](work-packages/07-sqlite-host-ownership-live-forks.md) | WP05 subsumed the former R2–R12 execution rows; their implemented contract is in Parts 0–5 and the completed handoff. -Future candidates (detail and order in the roadmap): **H1** — resolve the `OperationStatus.running`, abort signal/event-order, and private gate-close typing contracts and audit Part 9 coverage; **C1** — resolve the §2.8 raw-RemoteSession contradiction before implementing either direction; **L1** — repository ownership of open handles and all-settled close across the three backends; **J1** — implement the §1.7 snapshot rewrite, dead-byte triggers, preserved high-water/list sequences, physical reclamation; **M11** — measure Memory/SQLite/JSONL frame cost, then bound assistant-frame writes without weakening unknown-outcome recovery; **R12** — implement `watchSession`; **T1** — reconcile the declared telemetry schema, then implement retained local spans (RPC trace propagation and an exporter are separate follow-ups); **S3** — reconcile the draft search API, then implement the standalone service, repository catch-up utilities, and the reference SQLite FTS5 projection (§2.8); **R11** — chained migrate-on-open under the writer lease with total mappings (Part 7), activated only before the first incompatible stabilized-format change. +Future candidates (detail and order in the roadmap): **H1** — resolve the `OperationStatus.running`, abort signal/event-order, and private gate-close typing contracts and audit Part 9 coverage; **C1** — resolve the §2.8 raw-RemoteSession contradiction before implementing either direction; **L1** — repository ownership of open handles and all-settled close across the three backends; **J1** — implement the §1.7 snapshot rewrite, dead-byte triggers, preserved high-water/list sequences, physical reclamation; **M11** — measure Memory/SQLite/JSONL frame cost, then bound assistant-frame writes without weakening unknown-outcome recovery; **R12** — implement `watchSession`; **T1** — reconcile the declared telemetry schema, then implement retained local spans (RPC trace propagation and an exporter are separate follow-ups); **S3** — reconcile the draft search API, then implement the standalone service, repository catch-up utilities, and the reference SQLite FTS5 projection (§2.8); **R11** — chained migrate-on-open under exclusive host ownership with total mappings (Part 7), activated only before the first incompatible stabilized-format change. Client watch/subscription incarnation fencing, SQLite branch/fork/query performance, pending-payload measurement, and optional presentation/plugin capabilities are inventoried in the roadmap; they do not alter the Harness state machine. Protocol, client/server resnapshot, and lane reducer surfaces required by WP05 are already implemented; future protocol work extends them rather than redefining the lane contract. diff --git a/packages/agent/docs/post-wp05-roadmap.md b/packages/agent/docs/post-wp05-roadmap.md index 4a04fa07a..760d4d280 100644 --- a/packages/agent/docs/post-wp05-roadmap.md +++ b/packages/agent/docs/post-wp05-roadmap.md @@ -22,7 +22,7 @@ WP05 is complete through M10. M11 is its only recorded follow-up. The current Ha That does **not** mean the surrounding durable system is complete. The audit found: -1. two SQLite ownership races that can violate single-writer fencing or delete an actively claimed Session; +1. SQLite storage-layer writer ownership that contradicts the server/Session-worker authority model, plus no explicit read-only path proving a server-side fork can snapshot a live worker-owned source; 2. normative JSONL snapshot-compaction behavior with no implementation; 3. one required Harness method stub (`watchSession`); 4. a deliberate removal of raw `RemoteSession` that conflicts with later normative WP06/`harness.md` text; @@ -30,7 +30,7 @@ That does **not** mean the surrounding durable system is complete. The audit fou 6. a full telemetry vocabulary whose only production span is the tool-hook span; 7. smaller repository, client-watch, query-bound, documentation, and end-to-end-test gaps. -The immediate package is therefore SQLite ownership fencing, not a combined “cleanup” package. Its handoff is [`work-packages/07-sqlite-ownership-fencing.md`](work-packages/07-sqlite-ownership-fencing.md). +The immediate package is therefore SQLite host-ownership alignment and live-source fork support, not generic storage cleanup. Its handoff is [`work-packages/07-sqlite-host-ownership-live-forks.md`](work-packages/07-sqlite-host-ownership-live-forks.md). ## Required missing functionality and contract contradictions @@ -118,26 +118,27 @@ When activated, it must provide ordered transactional migrate-on-open under excl ## Correctness and data-safety debt -### SQLite ownership and deletion — immediate +### SQLite host ownership and live-source forks — immediate -1. `SqliteStorage.applyCommit()` renews the writer lease in a separate transaction before the data transaction. A replacement writer can claim an expired lease in that gap; the stale owner’s data transaction never checks `(owner_id, fence)`. -2. `SqliteSessionRepo.delete()` checks the lease, closes that connection, then deletes through another connection or unlinks the file. Another process can claim between check and deletion. +The authoritative product rule is in `plugins.md`: exactly one host-assigned process owns writable Session authority; normally it is the Session worker, while the server may temporarily own a newly created or forked destination before closing it and handing it off. Storage backends do not implement writer ownership. The server closes a worker before destructive repository administration. -These violate the documented single-writer guarantee and can corrupt or remove active Session data. WP07 owns them. +Current SQLite source contradicts that rule with a `writer_lease` table, claim/release calls, renewal timer, lease-loss admission path, and a pre-commit renewal callback. This duplicates host ownership without correctly fencing commits. Remove it rather than repair it; a second process directly opening the same Session for writes is an unsupported host defect, as with Memory and JSONL. + +Forking is the supported cross-process overlap: the server repository may snapshot a source while its worker continues writing. Keep the same-repository source queue for its admitted-commit ordering seam, but use an independent no-create, read-only connection and one deferred transaction for a source owned elsewhere. WAL must allow later worker commits while the fork sees one complete before-or-after transaction boundary. WP07 owns the lease removal, live-source fork path, repository-local deletion reservation, physical identity/path safety, and SQLite close draining. It does not add a replacement storage lock, lease, tombstone, or takeover protocol. ### SQLite repository identity and path safety - `databasePath` creation makes `options.directory`, not the actual custom path’s parent. - Per-session filenames interpolate arbitrary explicit IDs without encoding; separators can escape `directory`. - Active fork lookup is keyed only by Session ID, so caller-supplied metadata for the same ID at another path can select the wrong open source. -- `repo.close()` uses fail-fast `Promise.all`, so it can reject before other Session closes settle and release their claims. +- `repo.close()` uses fail-fast `Promise.all`, so it can reject before other Session closes finish draining and release their connections. - Equal-`createdAt` list ordering has no deterministic tie-break. -Path/identity/close items that share writer ownership belong in WP07. Deterministic listing is a later behavior-preserving cleanup. +Path/identity/close items needed by the host-owned repository and live-fork path belong in WP07. Deterministic listing is a later behavior-preserving cleanup. ### Repository close ownership is unspecified -`JsonlSessionRepo.close()` contains the only active Agent source TODO and closes no open Session handles. Memory and SQLite repositories do close owned resources, but both currently use fail-fast `Promise.all`; `SessionRepo` itself declares no `close()` method and shared conformance does not define repository-to-handle ownership. Resolve ownership and all-settled cleanup in one repository-lifecycle package; WP07 may harden SQLite's claim release locally, but do not patch JSONL alone without deciding the common contract. +`JsonlSessionRepo.close()` contains the only active Agent source TODO and closes no open Session handles. Memory and SQLite repositories do close owned resources, but both currently use fail-fast `Promise.all`; `SessionRepo` itself declares no `close()` method and shared conformance does not define repository-to-handle ownership. Resolve ownership and all-settled cleanup in one repository-lifecycle package; WP07 may harden SQLite's backend-local resource cleanup, but do not patch JSONL alone without deciding the common contract. ### Client watch staleness after disconnect @@ -148,7 +149,7 @@ Path/identity/close items that share writer ownership belong in WP07. Determinis - SQLite `getEntries(ids)` emits one placeholder per requested ID and can exceed the engine’s variable limit. - Entry, usage, and branch limits use ad hoc `Math.max(0, limit)` behavior. Memory and SQLite diverge for `NaN`, infinities, fractions, and extreme values; unlike list reads, there is no shared normalization contract. -Define cross-backend query-limit semantics in agent conformance, then chunk SQLite ID lookups. This is a storage-contract hardening package, not part of lease fencing. +Define cross-backend query-limit semantics in agent conformance, then chunk SQLite ID lookups. This is a storage-contract hardening package, not part of WP07. ### Harness contract and conformance closure @@ -237,7 +238,7 @@ These are not blockers for the durable Harness: The order is by data safety first, then dependencies. Independent tracks may proceed in parallel only when they do not edit the same contracts. -1. **WP07 — SQLite ownership fencing.** Atomic lease assertion with commits, exclusive deletion, path/source identity, close draining, and documentation correction. +1. **WP07 — SQLite host ownership and live forks.** Remove storage-layer writer leases; add no-create read-only snapshots of live worker-owned sources; preserve same-repository fork ordering; add repository-local deletion reservation, path/source identity, and close draining. 2. **Harness contract/conformance closure.** Resolve `OperationStatus.running`, abort signal/event binding order, gate-close typing, and the Part 9 coverage matrix. 3. **Remote Session decision (decision only).** Resolve the false normative boundary early. If process-local wins, repair the docs. If raw RemoteSession wins, later create a dedicated protocol/client/server/worker package; do not fold it into telemetry or R12. 4. **Client watch/subscription staleness** and **repository lifecycle contract.** Small independent correctness packages; complete them before expanding server/worker lifecycle semantics. The lifecycle package must also address Memory's fail-fast repository close. @@ -245,7 +246,7 @@ The order is by data safety first, then dependencies. Independent tracks may pro 6. **M11 durable frame volume.** Memory/SQLite measurement can start earlier; set final JSONL budgets only with compaction measured, and preserve all recovery boundaries. 7. **R12 session-wide watch.** Complete the only Harness method stub before building revisioned Transcript/session-wide remote observation. 8. **Telemetry, if retained:** reconcile schemas, then local instrumentation, then RPC propagation, then an optional exporter. RPC propagation follows the Remote Session/product-boundary decision. -9. **SQLite branch/fork/query performance hardening.** Keep separate from lease correctness and require benchmarks. +9. **SQLite branch/fork/query performance hardening.** Keep separate from WP07 ownership alignment and require benchmarks. 10. **S3 search.** Resolve its API/filter/cursor decisions, then implement catch-up and the standalone FTS projection. 11. **R11 migrations.** Activate immediately before the first incompatible stabilized durable schema change, not earlier. @@ -258,4 +259,5 @@ This inventory must be updated when any of these facts changes: - JSONL snapshot compaction lands; - telemetry schemas are implemented or removed; - S3’s public API is reconciled; -- a durable format change activates R11. +- a durable format change activates R11; +- SQLite storage-layer ownership is removed or the host-authority contract changes. diff --git a/packages/agent/docs/values.md b/packages/agent/docs/values.md index e17f2d924..c8a2c8100 100644 --- a/packages/agent/docs/values.md +++ b/packages/agent/docs/values.md @@ -560,7 +560,7 @@ ORDER BY seq DESC LIMIT ?; DELETE FROM list_values WHERE namespace = ? AND key = ?; ``` -For a missing cursor, omit the sequence predicate. Every write participates in the existing `BEGIN IMMEDIATE` transaction under the writer lease. Assert with `EXPLAIN QUERY PLAN` that paging uses the primary key and no temporary sort. +For a missing cursor, omit the sequence predicate. Every write participates in the existing `BEGIN IMMEDIATE` transaction; writable Session ownership belongs to the host lifecycle, not SQLite storage. Assert with `EXPLAIN QUERY PLAN` that paging uses the primary key and no temporary sort. ## JSONL backend diff --git a/packages/agent/docs/work-packages/07-sqlite-host-ownership-live-forks.md b/packages/agent/docs/work-packages/07-sqlite-host-ownership-live-forks.md new file mode 100644 index 000000000..350688a6f --- /dev/null +++ b/packages/agent/docs/work-packages/07-sqlite-host-ownership-live-forks.md @@ -0,0 +1,370 @@ +# WP07 — SQLite host ownership and live-source forks + +**Status: actionable handoff; not implemented.** + +This package aligns `packages/session-backends/sqlite-node` with the product ownership model: the server owns Session records and worker lifecycle, and exactly one host-assigned process owns writable Session authority at a time. Normally that process is the Session worker. The server may temporarily own a newly created or forked destination, but it closes that Session before handing its metadata to a worker. + +Storage does not implement writer ownership. Remove the SQLite writer lease; do not repair or replace it. + +A server-side fork is intentionally different from a second writer: it may open a live worker-owned source concurrently for one coherent read-only snapshot while the worker keeps committing. Shared SQLite containers remain supported. + +## 0. Mandatory reading + +Read completely before editing: + +1. `packages/agent/docs/plugins.md` ownership, replacement, and removal sections. +2. `packages/server/README.md` and relevant Session routing/removal source and tests. +3. `packages/agent/docs/harness.md` §§0.6, 1.4–1.7, 2.7–2.8, 4.3, and Part 9. +4. `packages/agent/docs/post-wp05-roadmap.md`. +5. completed WP06 §7 and repository/fork conformance. +6. every source file under `packages/session-backends/sqlite-node/src`. +7. every test and benchmark under `packages/session-backends/sqlite-node`. +8. `packages/session-backends/sqlite-node/README.md` and `CHANGELOG.md`. + +Do not use stale files under `dist/` as implementation input. Completed WP01/WP06 documents are historical; do not rewrite them to hide the earlier lease implementation. + +## 1. Fixed architecture + +### 1.1 Writable authority belongs to the host + +Exactly one host-assigned process owns a writable Session. The Session worker is the normal owner. Worker replacement closes the old owner before the new worker opens the Session. Server management serializes creation, forking, removal, and attachment lifecycle around that ownership transfer. + +Memory, JSONL, and SQLite do not detect a second process opening the same Session for writes. Bypassing the server/worker lifecycle is a trusted-host defect, not a storage race to repair. A repository still rejects duplicate writable handles it owns in one process. + +Do not add a storage lease, filesystem lock, fencing token, heartbeat, timeout-based takeover, deletion tombstone, quarantine protocol, or generic lock manager. + +### 1.2 Read-only fork access may overlap the worker + +The server owns repository administration and may fork a source while its Session worker continues writing it. The source side of that fork: + +- opens the exact source container without creating it; +- is read-only; +- uses one deferred `BEGIN` transaction; +- reads the version-gated Session row, scalar values, and selected entries/branch index in that transaction; +- never upgrades to a write or claims writable authority; +- closes after `COMMIT`/`ROLLBACK`. + +SQLite WAL permits later worker commits while the read transaction remains open. The fork sees each source commit entirely before or entirely after its snapshot boundary, never a mixture. + +### 1.3 Same-repository fork ordering remains distinct + +A source already open in the same repository uses its active `SqliteStorage.snapshot()` path. That snapshot is queued on the source `commitQueue`, preserving WP06's admitted-commit ordering seam and the existing conformance case. + +Do not replace this path with an independent connection. Instead, bind active storage lookup to exact physical identity plus Session ID so metadata for another container cannot select it accidentally. + +### 1.4 Destination ownership does not overlap + +`SessionRepo.create()` and `fork()` continue returning an open Session. A server may temporarily own that new destination, capture its metadata, and close it before launching a worker. This is a valid ownership transfer, not a reason to redesign `SessionRepo` into a record-only API. + +## 2. Problems in current source + +### 2.1 SQLite duplicates host ownership + +Current source contains: + +- `writer_lease` schema state; +- claim, renew, and release helpers; +- lease claims in create/open/fork; +- an idle renewal timer and lease-loss path in `SqliteOpenSession`; +- a pre-commit renewal callback in `SqliteStorage`; +- lease-based deletion checks. + +This is a second, incomplete ownership system. Its pre-commit renewal is not atomic with the following data transaction, but the correct fix is deletion of the ownership mechanism, not transaction-local fencing. + +### 2.2 Non-creation access can create files + +The database factory exposes only `open(path)`, which creates a missing SQLite file. Metadata open, listing probes, deletion, and fork-source reads must not turn a removed path into an empty database. + +Fork-source reads also configure `PRAGMA journal_mode = WAL`, which is a write-oriented setup step and must not run on a read-only connection. + +### 2.3 Delete does not reserve its local critical section + +`delete()` checks `pendingIds` but does not reserve the ID. A same-repository create/open/fork destination can enter while asynchronous destructive work is in progress. Host lifecycle owns cross-process ordering; the repository still must serialize its own local operations. + +### 2.4 Physical identity, paths, and close need correction + +- `openStorages` is keyed only by Session ID, so the same ID at another physical path may select the wrong active source. +- `create()` makes `options.directory`, not the parent of an explicit `databasePath`. +- Per-session filenames interpolate arbitrary caller IDs directly; `/`, `\`, `..`, `%`, and platform separators must not escape `directory`. +- `repo.close()` uses fail-fast `Promise.all`, so it may return before every open Session has attempted to drain and close. + +Deterministic list ordering, bind-variable limits, branch-copy cost, fork scalar filtering, prepared statements, and VACUUM policy remain separate. + +## 3. Required result + +### 3.1 Remove storage-layer writer ownership + +Delete all runtime lease behavior: + +- delete `src/sqlite/session/writer-lease.ts`; +- remove `writer_lease` from WIP `001_initial.sql`; +- remove lease deletion from `deleteSessionRows()`; +- remove claim/renew/release code from `SqliteSessionRepo`; +- remove `beforeCommit` from `SqliteStorage`; +- remove renewal/release options, timer, and `leaseError` from `SqliteOpenSession`; +- remove lease-specific tests and replace them with host-authority and live-fork coverage. + +Keep: + +- the Storage `commitQueue`; +- one `BEGIN IMMEDIATE` transaction per commit; +- in-transaction `next_seq` allocation; +- entry/usage uniqueness and parent triggers; +- Session mutation admission and close draining; +- process-local duplicate-open rejection. + +Format 4 remains WIP. Remove the table from new schema in place; an old file containing an unused `writer_lease` table remains readable and the table and stale rows are ignored forever. Post-WP07 code does not delete them because doing so serves no runtime behavior. A pre-WP07 binary cannot open a new post-WP07 database without that table; backward compatibility for this WIP format is not required. Add no migration, compatibility path, or storage-version bump. + +### 3.2 Add explicit database open modes + +Extend `SqliteDatabaseFactory` with narrow operations: + +- `open(path)` — intentional creation or create-if-missing; +- `openExisting(path)` — read-write open that fails if the file does not exist; +- `openReadOnly(path)` — read-only open that fails if the file does not exist. + +The Node adapter uses `DatabaseSync(path, { readOnly: true })` for read-only access. Implement and test an actual no-create read-write mode for `openExisting`; do not rely on `access()` followed by a create-capable open. + +Split connection setup: + +- writable connections establish WAL mode and `busy_timeout`; +- read-only connections set only read-safe options such as `busy_timeout` and never attempt to change journal mode. + +Use no-create modes for metadata open, listing probes, deletion, and fork-source reads. + +### 3.3 Preserve both fork-source paths + +**Source open in this repository:** retain `SqliteStorage.snapshot()` and queue it after prior admitted commits. Replace the ID-only active map key with canonical `(containerPath, sessionId)` identity and use the same helper for publish, lookup, and removal. + +**Source not open in this repository:** this includes a closed source and a source currently owned by a worker in another process. Open the exact source through `openReadOnly`, then capture it in one deferred read transaction. Validate the Session row and storage version inside that transaction. Do not consult destination reservations, claim source ownership, or block the worker's later commits. + +The destination remains a normal writable create/fork transaction after source capture. In shared-container mode the source worker and destination transaction may use the same file; SQLite serializes destination writes while preserving Session row isolation. + +### 3.4 Make deletion locally exclusive + +The host must close the Session worker before calling `repo.delete()`. Direct cross-process deletion of a live Session is unsupported. + +Within one `SqliteSessionRepo`, deletion must reserve the Session ID from entry through completion and release it in `finally`: + +- an already open or reserved Session rejects deletion; +- create/open/fork destination for that ID rejects while deletion runs; +- shared-container deletion removes only the target Session's rows in one `BEGIN IMMEDIATE` transaction on one connection; +- per-file deletion removes the database and its WAL/SHM sidecars after a no-create open/existence check; +- a missing Session rejects without creating a file. + +Do not add a lease check, tombstone, quarantine rename, or stale-deleter protocol. Cross-process removal ordering is the server's responsibility. + +### 3.5 Bind metadata to physical identity and make paths safe + +- Canonical identity is `(canonical container path, sessionId)`. +- In per-file mode metadata must identify the repository-affine encoded path for its durable ID. +- In shared-container mode metadata must identify the configured canonical container and Session ID. +- A foreign or mismatched path must never alias a local active source. Pin in focused tests and the SQLite README whether foreign source metadata is rejected or read only from its exact path; do not silently substitute a local storage by ID. +- Create `dirname(databasePath)` when `databasePath` is configured. +- Encode arbitrary explicit IDs into safe per-session filenames without changing the durable ID. The encoding must prevent path escape and round-trip through metadata/list/open/fork. +- Two different Session IDs in one shared container remain independently addressable. + +### 3.6 Drain all repository-owned closes + +`SqliteSessionRepo.close(context)` must: + +1. seal repository admission once; +2. start close on every currently open Session; +3. wait for every close to settle; +4. resolve when all succeed; +5. otherwise reject only after all cleanup attempts, returning the one error or an `AggregateError` containing all failures; +6. return the same promise on repeated close. + +This is backend-local resource cleanup. Do not change the shared `SessionRepo` interface or JSONL lifecycle in this package. + +## 4. Implementation slices + +### Slice A — remove writer leases + +Files: + +- delete `src/sqlite/session/writer-lease.ts`; +- `src/sqlite/migrations/001_initial.sql`; +- `src/sqlite/session/session-row.ts`; +- `src/sqlite/storage.ts`; +- `src/sqlite/session.ts`; +- `src/sqlite/repo.ts`; +- lease-focused repository tests. + +Tasks: + +1. Remove schema/runtime lease state and timer behavior. +2. Preserve commit serialization, one write transaction, mutation draining, and process-local duplicate-open behavior. +3. Prove ordinary commits now use one write transaction rather than renewal plus write. + +### Slice B — no-create opens and deletion reservation + +Files: + +- `src/index.ts`; +- `src/sqlite/types.ts`; +- `src/sqlite/repo.ts`; +- focused adapter/repository tests. + +Tasks: + +1. Add `openExisting` and `openReadOnly` with tested no-create behavior. +2. Separate writable and read-only connection configuration. +3. Reserve deletion locally for its full critical section. +4. Delete one shared-container Session in one transaction; preserve unrelated Sessions. + +### Slice C — live-source read-only forks and identity + +Files: + +- `src/sqlite/repo.ts`; +- repository/conformance tests. + +Tasks: + +1. Key active storage by canonical container plus Session ID; keep same-repository queue ordering. +2. Use one independent read-only deferred transaction for non-open/live-worker sources. +3. Validate source metadata/version inside the snapshot. +4. Preserve shared-container destination behavior. + +### Slice D — paths and close draining + +Files: + +- `src/sqlite/repo.ts`; +- focused repository/conformance tests. + +Tasks: + +1. Create the actual custom container parent. +2. Encode arbitrary IDs safely. +3. Reject or exactly handle foreign metadata without active-source aliasing. +4. Make repository close all-settled and error-complete. + +### Slice E — documentation + +Files: + +- `packages/agent/docs/harness.md`; +- `packages/agent/docs/post-wp05-roadmap.md`; +- `packages/agent/docs/values.md`; +- `packages/session-backends/sqlite-node/README.md`; +- changelog only under normal branch rules. + +Document host-owned writable authority, the two fork-source paths, no-create opens, local deletion reservation, and absence of storage-layer ownership. + +## 5. Required tests + +Use real independent `node:sqlite` connections. Test-only wrappers may expose deterministic transaction boundaries; production code gets no sleeps or race flags. + +### Lease removal + +- new schema has no `writer_lease` table; +- create/open/fork/commit/close perform no lease reads or writes and start no renewal timer; +- same-repository duplicate writable open still rejects through process-local reservation; +- ordinary commit remains one `BEGIN IMMEDIATE` transaction. + +### Live fork source + +For both per-file and shared-container layouts: + +- a server repository forks a source held open by a separate repository/connection representing its worker; +- source capture uses a distinct read-only connection and claims no writable ownership; +- a source commit completed before the snapshot boundary appears wholly in the fork; +- a commit after the read snapshot is established can complete before the reader closes and is wholly absent from that fork; +- no fork contains an entry without the same commit's Branch tip/value/stats changes; +- a later fork includes the later commit; +- same-repository admitted-commit fork conformance remains unchanged. + +### Deletion + +- open/reserved Session first → same-repository delete rejects; +- delete reservation first → same-repository create/open/fork destination for that ID rejects; +- shared deletion removes only the target rows; +- per-file deletion removes database/WAL/SHM files; +- missing-path open/list/fork/delete does not create an empty database; +- tests state the host precondition: worker close precedes deletion; no cross-process bypass safety is promised. + +### Identity and paths + +- same Session ID at two physical paths cannot cross-select active source storage; +- two Session IDs in one shared container remain independent; +- `databasePath` succeeds when its parent does not exist; +- explicit IDs containing `../`, `/`, `\`, `%`, dots, and Unicode stay inside `directory` and preserve the metadata ID; +- metadata returned by create/list/open/fork names the actual container. + +### Close + +- one Session close failure does not prevent every other Session cleanup attempt; +- multiple failures are reported after all settle; +- repeated repository close returns the same promise; +- every successfully closed Session releases its connection. + +### Regression + +- existing storage and repository conformance remains semantically unchanged; +- fork destination reservation and same-repository source ordering remain intact; +- fork snapshots exclude operation/pending/result/usage/application state exactly as before; +- shared-container create/list/open/fork/delete remains supported; +- every write transaction still uses `BEGIN IMMEDIATE`; +- no migration, storage-version bump, compatibility layer, or replacement ownership primitive appears. + +## 6. Validation and review + +After each code slice: + +```bash +npm run check +``` + +Run each modified focused test from `packages/session-backends/sqlite-node` with the repository Vitest binary. Final validation: + +```bash +./test.sh +``` + +Review checkpoints: + +1. Fable after Slice A: verify no storage ownership mechanism remains and close draining is intact. +2. Fable after Slice C: verify same-repository ordering and live-worker read-only overlap both hold. +3. Final Fable review of source, tests, docs, and exclusions. + +Delegated reviews use provider `anthropic` and model `claude-fable-5`. + +## 7. Exclusions + +Do not include: + +- any storage lease, lock, fence, heartbeat, takeover, tombstone, or quarantine; +- a record-only `SessionRepo` redesign or compatibility facade; +- server/router/worker-manager redesign; record any backend-independent lifecycle race separately; +- SQLite branch-segment redesign or uncompacted-divergence optimization; +- fork scalar filtering/indexing; +- `getEntries` bind-limit chunking or general query-limit normalization; +- statement caches or stats aggregation optimization; +- catalog redesign, async database replacement, or VACUUM policy; +- search/FTS; +- R11 migration machinery or a storage-version bump; +- M11 assistant-frame changes or JSONL compaction; +- repository-wide `SessionRepo.close()` contract changes; +- removal of shared-container support; +- transaction DSLs, schedulers, generic lock managers, or compatibility layers. + +If implementation requires an excluded item, stop and revise the handoff rather than expanding silently. + +## 8. Exit condition + +WP07 is complete when: + +- SQLite contains no active or schema-defined writer lease and implements no replacement ownership mechanism; +- host ownership is the documented single-writer authority; +- same-repository active-source forks retain their queued commit boundary; +- live worker-owned sources fork through an independent read-only snapshot while later WAL commits continue; +- deletion reserves its same-repository critical section and assumes worker-first host removal; +- non-creation paths cannot create empty databases; +- active source identity includes physical container plus Session ID; +- explicit IDs cannot escape the directory and custom database parents are created; +- repository close waits for every cleanup attempt; +- shared-container mode remains fully covered; +- focused tests, `npm run check`, and `./test.sh` pass; +- final Fable review reports no blocker. diff --git a/packages/agent/docs/work-packages/07-sqlite-ownership-fencing.md b/packages/agent/docs/work-packages/07-sqlite-ownership-fencing.md deleted file mode 100644 index 5cee74654..000000000 --- a/packages/agent/docs/work-packages/07-sqlite-ownership-fencing.md +++ /dev/null @@ -1,332 +0,0 @@ -# WP07 — SQLite ownership fencing - -**Status: actionable handoff; not implemented.** - -This package fixes data-safety defects in `packages/session-backends/sqlite-node`. It does not optimize branch indexing, forks, search, frame persistence, or general query performance. - -Shared-container mode (`databasePath`) remains supported. Removing it is outside this package. - -## 0. Mandatory reading - -Read completely before editing: - -1. `packages/agent/docs/harness.md` §§1.4, 1.7, 2.7–2.8, 4.3, and Part 9. -2. `packages/agent/docs/post-wp05-roadmap.md`. -3. `packages/agent/src/harness/session/types.ts`, `session.ts`, `memory.ts`, and repository conformance. -4. every source file under `packages/session-backends/sqlite-node/src`. -5. every test and benchmark under `packages/session-backends/sqlite-node`. -6. `packages/session-backends/sqlite-node/README.md` and `CHANGELOG.md`. - -Do not use stale files under `dist/` as implementation input. - -## 1. Problem - -### 1.1 Commit fencing has a gap - -Current commit order: - -```text -TX A: renew/assert (owner_id, fence); COMMIT - ← replacement may claim here -TX B: read next_seq; write Session data; COMMIT -``` - -`SqliteStorage.applyCommit()` calls `beforeCommit()` before `db.transaction(...)`. `SqliteSessionRepo` supplies a `beforeCommit` callback that renews the writer lease in its own transaction. The data transaction never checks the lease. - -Concrete failure: - -```text -writer A renews fence 4 -A pauses until that lease expires -writer B claims fence 5 and commits -A resumes and commits Session rows without checking fence 4 -``` - -The lease table has then failed to enforce one writer. Sequence allocation and branch/value updates can interleave across owners that both believe they are authoritative. - -### 1.2 Deletion has the same check/use gap - -Current deletion order: - -```text -open connection A -read writer_lease; observe no live claim -close A - ← writer may claim here -open connection B and delete rows, or unlink the file -``` - -A shared-container delete can remove rows after a replacement writer claims them. A per-session delete can unlink a database that another process has just opened and claimed. - -### 1.3 Repository identity and close are not strong enough - -- `openStorages` is keyed only by Session ID. `open()` accepts metadata paths, so a fork using the same ID at another path can snapshot the wrong active storage. -- `create()` makes `options.directory`, not the parent of an explicit `databasePath`. -- Per-session filenames interpolate caller-supplied IDs directly. The public repository contract and conformance use explicit arbitrary IDs; path separators must not escape the configured directory. -- `repo.close()` uses fail-fast `Promise.all`. One rejection can return while other Session closes are still draining and releasing writer claims. - -These are included because they affect ownership identity or release. Deterministic list ordering, bind-variable limits, branch-copy cost, fork scalar scans, prepared statements, and VACUUM policy do not. - -## 2. Required result - -### 2.1 Fence inside every data transaction - -Every `SqliteStorage` commit must execute this order in **one** `BEGIN IMMEDIATE` transaction: - -```text -BEGIN IMMEDIATE -assert and renew this exact (session_id, owner_id, fence) -read/advance next_seq -apply entries, usage, values, lists, branch projection, and stats -COMMIT -``` - -If the exact lease is absent or replaced, the transaction rolls back without changing Session data. Lease assertion must not call a nested transaction. - -The idle renewal timer remains a separate transaction because no data transaction exists while idle. Any timer renewal failure leaves ownership uncertain and fail-closes that open Session; distinguishing transient SQLite availability failures from confirmed lease replacement is later policy work. A successful idle renewal does not authorize a later commit by itself: the commit still checks inside its own transaction. - -Repository-owned `SqliteStorage` must receive the exact lease identity or a transaction-local assertion callback, not an unfenced “before commit” side effect. Standalone storage instances used by storage conformance have no repository lease and remain unfenced. Keep the concrete sequence-allocation and write loop visible. - -### 2.2 Make deletion exclusive through the destructive boundary - -#### Shared container - -Validate lease state and delete all rows for the Session in one `BEGIN IMMEDIATE` transaction. No other writer may claim between the check and row deletion. A live unexpired claim rejects deletion without changing rows. - -#### Per-session file - -A normal expiring writer lease cannot fence unlink by itself: a deleter may stall past expiry after closing SQLite, a replacement may claim, and the stale deleter may then unlink the replacement's live database. Per-file deletion therefore uses one narrow path tombstone and an owner-specific quarantine path: - -1. atomically create a sibling deletion-tombstone file containing a fresh deletion owner id; `open`, source-fork reads, listing probes, and destination creation use open-existing/tombstone-aware helpers rather than create-on-open; -2. in one `BEGIN IMMEDIATE` transaction, reject a live ordinary writer or install a **non-expiring deletion claim** under that owner and a fresh fence; -3. close SQLite, atomically rename the canonical database and any WAL/SHM files to owner-specific quarantine names while the tombstone excludes new opens; -4. after rename, this deletion attempt removes only its owner-specific quarantine files, never the canonical database or canonical sidecars; -5. remove the tombstone last with an exact owner check; a later `delete` may idempotently finish the quarantine named by that tombstone, and concurrent finishers remain harmless because neither touches canonical paths; -6. if an ordinary writer won the database transaction after checking the path but before tombstone creation, deletion observes that live claim, removes only its own tombstone, and rejects. - -Extend `SqliteDatabaseFactory` with an `openExisting` operation whose contract is read-write open without file creation. The Node adapter uses SQLite URI `mode=rw`; missing paths fail instead of leaving empty containers. Reserve `open` for intentional creation. Destination creation checks the tombstone both before and immediately after its exclusive file reservation, before SQLite initialization; if deletion won, it removes only that reservation and rejects. Use `openExisting` for metadata open, list probes, deletion, and closed-source forks. - -This is a purpose-built deletion intent, not a general filesystem lock manager. It closes all three gaps: the database transaction orders already-in-flight openers against deletion, the tombstone covers canonical-path publication, and quarantine ensures a stale/concurrent finisher can never unlink a later recreated Session. A process crash may leave deletion pending and the Session unavailable; retrying `delete` completes it. Never let an ordinary writer take over a deletion claim, and never remove another owner's tombstone, claim, or quarantine. - -Deletion of an already-open Session in the same repository continues to reject before destructive work. Cross-process correctness must not depend on the process-local `pendingIds` set. - -### 2.3 Bind active storage to physical identity - -Active-source lookup for fork must include canonical container identity and Session ID. A foreign or mismatched metadata path must never select another active `SqliteStorage` merely because IDs match. - -Define one helper for repository-affine physical identity and use it consistently when publishing, looking up, and removing active storage. Preserve intentional shared-container behavior: two Session IDs in one canonical container remain distinct. - -Do not silently open arbitrary foreign metadata as though it belonged to this repository. Either validate repository affinity at `open`/`fork` or handle the foreign source only through its exact path without aliasing a local active Session; pin the chosen rule in tests and README. - -### 2.4 Make paths safe and predictable - -- Create the actual parent directory of `databasePath` when that option is present. -- Map explicit Session IDs to safe per-session filenames without changing the durable Session ID. Encoding must prevent `/`, `\\`, `..`, or platform separators from escaping `directory`. -- Metadata returned by create/list/open must continue to identify the actual container path. -- Existing metadata paths are opened as supplied after the repository-affinity checks above; this WIP format package adds no migration or filename-renaming pass. - -### 2.5 Drain all closes - -This is backend-local claim cleanup, not a decision about adding `close()` to the shared `SessionRepo` interface or changing JSONL repository ownership. - -`SqliteSessionRepo.close(context)` must: - -1. seal repository admission once; -2. start close on every currently open Session; -3. wait for every close to settle; -4. resolve when all succeed; -5. otherwise reject only after all claims/connections have had their cleanup attempt, preserving every failure in an `AggregateError` when more than one exists. - -Idempotent repeated close returns the same promise. - -## 3. Implementation slices - -### Slice A — transaction-local writer assertion - -Files: - -- `src/sqlite/storage.ts` -- `src/sqlite/repo.ts` -- `src/sqlite/session/writer-lease.ts` -- focused storage/repository tests - -Tasks: - -1. Replace `beforeCommit` with exact lease fencing available inside `SqliteStorage.applyCommit()`’s transaction. -2. Keep timer renewal transaction-wrapped in the open-Session owner. -3. Ensure lease loss propagates through `SqliteOpenSession` without releasing a replacement lease. -4. Add a deterministic two-connection regression that fails under the old renew-then-commit split. - -Stop after Slice A if making the lease check transaction-local requires hiding the storage write procedure behind a generic transaction abstraction. - -### Slice B — exclusive deletion - -Files: - -- `src/index.ts` -- `src/sqlite/types.ts` -- `src/sqlite/repo.ts` -- `src/sqlite/session/session-row.ts` -- `src/sqlite/session/writer-lease.ts` -- focused repository tests - -Tasks: - -1. Add a read-write, no-create `openExisting` factory operation and move every non-creation path to it. -2. Shared container: one lease-check-and-row-delete transaction. -3. Per-session file: atomic sibling tombstone, non-expiring database deletion claim, owner-specific quarantine, resumable cleanup, and tombstone-last removal. -4. Test both race orders, concurrent deletion finishers, and crashes at each tombstone/claim/quarantine boundary. - -### Slice C — physical identity, safe paths, and close draining - -Files: - -- `src/sqlite/repo.ts` -- focused repository/conformance tests - -Tasks: - -1. Canonical `(container, sessionId)` active-source keys and repository-affinity checks. -2. Actual `databasePath` parent creation. -3. Safe filename encoding for explicit IDs. -4. all-settled repository close with complete error reporting. - -### Slice D — documentation - -Files: - -- `packages/session-backends/sqlite-node/README.md` -- `packages/agent/docs/harness.md` SQLite subsection -- `packages/agent/docs/post-wp05-roadmap.md` -- changelog only under the repository’s normal changelog rules - -The roadmap audit already corrected the README's class/example/search/connection claims and `harness.md`'s shared-container wording. Preserve those corrections. After the implementation lands, document: - -- writer fencing is checked in the same transaction as Session writes; -- shared-container deletion is atomic with its lease check; -- per-file deletion uses a resumable path tombstone, database deletion claim, and owner-specific quarantine; -- non-creation opens use no-create mode, so deletion races cannot leave empty containers; -- a crash may leave deletion pending and ordinary open rejected until `delete` resumes it. - -Do not describe S3 search as implemented. - -## 4. Required tests - -Use real `node:sqlite` connections for ownership tests. Test helpers may wrap `SqliteDatabase` to expose deterministic transaction boundaries; production code must not gain sleep hooks or race-only flags. - -### Commit fencing - -- exact current owner/fence commits successfully and renews expiry in the same transaction; -- stale owner after another repository claims a higher fence cannot insert an entry, advance `next_seq`, change a value/list, update stats, or mutate branch projections; -- deterministically force a replacement claim at the old between-transactions seam: old code commits incorrectly, new code gives the replacement either the before-commit or after-commit history, never stale-owner data after replacement; -- idle timer renewal still retains ownership; -- timer-detected loss rejects later reads/mutations according to the existing open-Session lease-failure policy; -- stale close cannot release a replacement `(owner_id, fence)`. - -### Deletion - -For both shared-container and per-session-file modes: - -- live unexpired writer first → delete rejects and Session remains usable; -- delete owns exclusivity first → later open/claim rejects and deletion completes; -- expired lease without replacement → delete succeeds; -- shared-container deletion removes only the target Session; -- per-file rename/removal failure leaves a resumable tombstone/claim/quarantine and ordinary open stays rejected until deletion is retried; -- crash after tombstone creation, after the database deletion claim, after canonical-to-quarantine rename, and after each quarantine removal is resumable; -- concurrent deletion finishers cannot touch canonical paths after quarantine and cannot remove another owner's tombstone; -- a stale deleter never removes a replacement writer’s claim or data; -- metadata open, listing, source fork, and delete racing a removed path cannot recreate an empty container file; -- destination create checks the tombstone before and after exclusive reservation and cannot publish a Session during deletion. - -### Identity and paths - -- same Session ID at two physical paths cannot cross-select an active fork source; -- two different Session IDs in one shared container remain independently addressable; -- custom `databasePath` succeeds when its parent does not exist; -- explicit IDs containing `../`, `/`, `\\`, `%`, and Unicode remain inside `directory` and round-trip as metadata IDs, as required by the currently unconstrained `SessionCreateOptions.id`; -- foreign/mismatched metadata follows the documented reject-or-exact-path rule; -- list/create/open/fork return the actual path. - -### Close - -- one Session close failure does not prevent every other open Session from finishing cleanup; -- multiple failures are reported after all settle; -- repeated repository close is idempotent; -- no connection or writer claim remains owned by a Session whose cleanup succeeded. - -### Regression - -- existing storage and repository conformance remains unchanged; -- fork snapshot boundaries remain coherent; -- every write transaction still begins `BEGIN IMMEDIATE`; -- shared-container create/list/open/fork/delete behavior remains supported; -- no storage-version bump, migration, schema rewrite, or compatibility layer is added. - -## 5. Validation - -After each code slice: - -```bash -npm run check -``` - -Run each modified focused test from `packages/session-backends/sqlite-node` with the repository Vitest binary. Final validation: - -```bash -./test.sh -``` - -Do not run `npm test`, the full Vitest suite directly, or `npm run build` unless requested. - -Review checkpoints: - -1. Fable review after Slice A, focused on whether a stale owner can still write. -2. Fable review after Slice B, focused on both deletion race orders and unlink failure. -3. Final Fable review of implementation, tests, docs, and exclusions. - -Delegated reviews use provider `anthropic` and model `claude-fable-5`. - -## 6. Metrics - -Record before/after results for: - -- SQL transaction count per ordinary commit (target: one write transaction, not renew plus write); -- SQL transaction count per shared-container deletion (target: one destructive transaction); -- per-file tombstone/claim/quarantine transitions and no-create open failures; -- open writer-lease rows and connections after repository close; -- focused race-test histories for both orders. - -A small increase in statements inside the one commit transaction is expected and required. Throughput optimization is not an exit criterion. - -## 7. Exclusions - -Do not include: - -- SQLite branch-segment redesign or uncompacted-divergence optimization; -- fork scalar filtering/indexing; -- `getEntries` bind-limit chunking or general query-limit normalization; -- statement caches or stats aggregation optimization; -- catalog redesign, async database replacement, VACUUM/space-reclamation policy; -- search/FTS; -- R11 migration machinery or a storage-version bump; -- M11 assistant-frame changes or JSONL compaction; -- repository-wide `SessionRepo.close()` contract changes; -- removal of shared-container support; -- generic lock managers (the per-file deletion tombstone is the one narrow exception), transaction DSLs, schedulers, or compatibility layers. - -If a correctness fix requires one excluded item, stop and revise the handoff rather than expanding silently. - -## 8. Exit condition - -WP07 is complete when: - -- no repository-owned Session data transaction can commit without proving its exact writer fence inside that transaction; -- shared deletion is atomic with its lease check; -- per-file deletion orders in-flight openers through the database claim, quarantines canonical files before removal, and never lets stale cleanup touch a recreated canonical Session; -- active fork sources are selected by physical container identity plus Session ID; -- explicit IDs cannot escape the per-session directory and custom database parents are created; -- repository close waits for all Session cleanup attempts; -- shared-container mode remains fully covered; -- README and normative SQLite documentation describe the implementation that exists; -- focused tests, `npm run check`, and `./test.sh` pass; -- final Fable review reports no blocker. diff --git a/packages/session-backends/sqlite-node/README.md b/packages/session-backends/sqlite-node/README.md index ca8a94e75..750f0fb1a 100644 --- a/packages/session-backends/sqlite-node/README.md +++ b/packages/session-backends/sqlite-node/README.md @@ -24,6 +24,6 @@ await session.close(BACKGROUND_CONTEXT); await repository.close(BACKGROUND_CONTEXT); ``` -The default layout creates one `{sessionId}.sqlite` file per Session under `directory`; path-safe encoding for arbitrary explicit IDs is tracked by WP07. Pass `databasePath` to place multiple Sessions in one supported shared container. Open Sessions own separate SQLite connections; repository listing, deletion, and closed-source forks open temporary connections. +The default layout currently creates one `{sessionId}.sqlite` file per Session under `directory`; WP07 replaces that direct interpolation with path-safe encoding for arbitrary explicit IDs. Pass `databasePath` to place multiple Sessions in one supported shared container. Open Sessions own separate SQLite connections. A fork of a source open in the same repository queues on that source; WP07 adds an independent read-only WAL snapshot for a source owned by another process, including a live Session worker. -The backend provides the Session repository, storage implementation, writer leases, branch projections, and maintained statistics. WP07 tracks transaction-local fence enforcement, deletion fencing, and path safety. The package does not currently export a search service or FTS index; search is the separate S3 projection. +The host lifecycle, not this backend, guarantees one writable owner per Session. Directly opening the same Session for writes in another process is unsupported; read-only listing and fork-source access may overlap the worker. The backend provides atomic storage, branch projections, and maintained statistics. WP07 removes the current storage-layer writer lease and adds the read-only live-fork path, repository-local deletion reservation, physical/path safety, and complete close draining. The package does not currently export a search service or FTS index; search is the separate S3 projection.