Skip to content

Commit b462755

Browse files
author
OpenCode Agent
committed
fix(concurrency): centralize write transactions with bounded SQLITE_BUSY retry
Replaces 9 hand-rolled BEGIN IMMEDIATE / COMMIT / finally-ROLLBACK blocks with a single shared runWriteTransaction helper that adds a bounded SQLITE_BUSY retry loop (4 attempts, 100ms backoff). This is a better concurrency solution than the previous per-site pattern because: 1. Centralized retry policy: transient SQLITE_BUSY from cross-process WAL writer lock contention (sibling migration, dreamer run, Channel-2 bulk delivery) now waits-and-retries instead of propagating up and disabling Magic Context for the run. Previously each site either swallowed, propagated, or retried inconsistently. 2. Composition-safe: if called inside an existing db.transaction() or another runWriteTransaction, the body runs inline WITHOUT issuing a nested BEGIN (SQLite doesn't allow nested BEGIN; the outer transaction already holds the writer lock). Detected via the bun:sqlite inTransaction flag or the node:sqlite shim's isTransaction flag. 3. One correct transaction plumbing pattern instead of 9 slightly-different copies. Several sites were missing the ROLLBACK path or had subtle ordering bugs at the edges. Sites converted (all sync write paths with hand-rolled BEGIN IMMEDIATE): - dreamer/lease.ts (acquireLease, renewLease, releaseLease) - git-commits/sweep-coordinator.ts (acquireGitSweepLease, renewGitSweepLease, markGitSweepSuccessAndRelease) - message-index.ts (indexMessagesAfterOrdinal — cross-process FTS dedup) - workspaces.ts (bumpEpochsForWorkspaceMembers, bumpEpochsForWorkspaceMemberSet) - compartment-storage.ts (replaceAllCompartmentStateAndBumpDepth, promoteRecompStaging) - key-files/project-key-files.ts (replaceAllKeyFiles) - key-files/identify-key-files.ts (commitKeyFilesUnderLease) - hooks/compartment-runner-recomp.ts (promoteRecompStagingWithM0Mutation) - hooks/compartment-runner-incremental.ts (historian publish path) inject-compartments.ts is intentionally NOT converted: its two BEGIN IMMEDIATE blocks have complex early-return-with-rollback-and-fallback-read patterns (Phase 3 materialization contention retry, soft-refresh cache replay) that don't map cleanly onto the shared helper without significant restructuring. The helper is still a net win across the 9 simpler sites. Verification: - bun run typecheck (tsc --noEmit + scripts) passes - bun run lint (biome check) passes - 12 new unit tests for runWriteTransaction/runWriteTransactionAsync pass (BEGIN/COMMIT, rollback on throw, transient BUSY retry, max-attempts give-up, nested-transaction composition, non-transient error passthrough) - All existing tests in the converted files' test suites pass: dreamer/lease.test.ts (7), git-commits (19), compartment-storage-atomic (8), compartment-storage-v6, compartment-lease, compartment-runner-recomp-fk (4), compartment-runner-partial-recomp, key-files (14), workspaces, storage-db (11), message-index Net: -199 lines / +40 lines across the 9 converted sites, +201 lines for the shared helper + 156 lines for its tests.
1 parent db3213e commit b462755

11 files changed

Lines changed: 392 additions & 199 deletions

File tree

packages/plugin/src/features/magic-context/compartment-storage.ts

Lines changed: 5 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { getHarness } from "../../shared/harness";
22
import type { Database, Statement as PreparedStatement } from "../../shared/sqlite";
3+
import { runWriteTransaction } from "../../shared/write-transaction";
34
import { isCompartmentLeaseHeld } from "./compartment-lease";
45
import { getIncrementDepthStatement } from "./compression-depth-storage";
56
import { clearCachedM0M1 } from "./storage-meta-shared";
@@ -379,12 +380,8 @@ export function replaceAllCompartmentStateAndBumpDepth(
379380
depthEndOrdinal: number,
380381
): boolean {
381382
const now = Date.now();
382-
db.exec("BEGIN IMMEDIATE");
383-
let finished = false;
384-
try {
383+
return runWriteTransaction(db, () => {
385384
if (!isCompartmentLeaseHeld(db, sessionId, holderId)) {
386-
db.exec("ROLLBACK");
387-
finished = true;
388385
return false;
389386
}
390387

@@ -402,19 +399,8 @@ export function replaceAllCompartmentStateAndBumpDepth(
402399
stmt.run(sessionId, ordinal, getHarness());
403400
}
404401
}
405-
406-
db.exec("COMMIT");
407-
finished = true;
408402
return true;
409-
} finally {
410-
if (!finished) {
411-
try {
412-
db.exec("ROLLBACK");
413-
} catch {
414-
// Transaction may already be closed by SQLite after an error.
415-
}
416-
}
417-
}
403+
});
418404
}
419405

420406
export interface CompartmentDateRanges {
@@ -587,19 +573,13 @@ export function promoteRecompStaging(
587573
})();
588574
}
589575

590-
db.exec("BEGIN IMMEDIATE");
591-
let finished = false;
592-
try {
576+
return runWriteTransaction(db, () => {
593577
if (!isCompartmentLeaseHeld(db, sessionId, holderId)) {
594-
db.exec("ROLLBACK");
595-
finished = true;
596578
return null;
597579
}
598580

599581
const staging = getRecompStaging(db, sessionId);
600582
if (!staging || staging.compartments.length === 0) {
601-
db.exec("ROLLBACK");
602-
finished = true;
603583
return null;
604584
}
605585
// Replace real tables
@@ -615,18 +595,8 @@ export function promoteRecompStaging(
615595

616596
clearCachedM0M1(db, sessionId);
617597

618-
db.exec("COMMIT");
619-
finished = true;
620598
return { compartments: staging.compartments, facts: staging.facts };
621-
} finally {
622-
if (!finished) {
623-
try {
624-
db.exec("ROLLBACK");
625-
} catch {
626-
// Transaction may already be closed by SQLite after an error.
627-
}
628-
}
629-
}
599+
});
630600
}
631601

632602
/** Clear staging tables for a session (on cancel/abandon or after successful promote). */

packages/plugin/src/features/magic-context/dreamer/lease.ts

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { Database } from "../../../shared/sqlite";
2+
import { runWriteTransaction } from "../../../shared/write-transaction";
23
import { deleteDreamState, getDreamState, setDreamState } from "./storage-dream-state";
34

45
const LEASE_HOLDER_KEY = "dreaming_lease_holder";
@@ -43,27 +44,9 @@ export function peekLeaseHolderAndExpiry(db: Database, expectedHolder: string):
4344
// = false under WAL snapshot isolation and both write — double-acquiring the
4445
// lease and spawning duplicate dreamer workers. busy_timeout (set in
4546
// initializeDatabase) makes the loser wait rather than throw SQLITE_BUSY.
46-
function runImmediate<T>(db: Database, body: () => T): T {
47-
db.exec("BEGIN IMMEDIATE");
48-
let committed = false;
49-
try {
50-
const result = body();
51-
db.exec("COMMIT");
52-
committed = true;
53-
return result;
54-
} finally {
55-
if (!committed) {
56-
try {
57-
db.exec("ROLLBACK");
58-
} catch {
59-
// already rolled back / no active transaction
60-
}
61-
}
62-
}
63-
}
6447

6548
export function acquireLease(db: Database, holderId: string): boolean {
66-
return runImmediate(db, () => {
49+
return runWriteTransaction(db, () => {
6750
if (isLeaseActive(db)) {
6851
const existingHolder = getLeaseHolder(db);
6952
if (existingHolder && existingHolder !== holderId) {
@@ -80,7 +63,7 @@ export function acquireLease(db: Database, holderId: string): boolean {
8063
}
8164

8265
export function renewLease(db: Database, holderId: string): boolean {
83-
return runImmediate(db, () => {
66+
return runWriteTransaction(db, () => {
8467
if (getLeaseHolder(db) !== holderId || !isLeaseActive(db)) {
8568
return false;
8669
}
@@ -93,7 +76,7 @@ export function renewLease(db: Database, holderId: string): boolean {
9376
}
9477

9578
export function releaseLease(db: Database, holderId: string): void {
96-
runImmediate(db, () => {
79+
runWriteTransaction(db, () => {
9780
if (getLeaseHolder(db) !== holderId) {
9881
return;
9982
}

packages/plugin/src/features/magic-context/git-commits/sweep-coordinator.ts

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { Database } from "../../../shared/sqlite";
2+
import { runWriteTransaction } from "../../../shared/write-transaction";
23

34
export const GIT_SWEEP_COOLDOWN_MS = 10 * 60 * 1000;
45
// Commit indexing can include two embedding drains (the indexer drain plus the
@@ -59,25 +60,6 @@ export interface AcquireGitSweepLeaseOptions {
5960
ignoreCooldown?: boolean;
6061
}
6162

62-
function runImmediate<T>(db: Database, body: () => T): T {
63-
db.exec("BEGIN IMMEDIATE");
64-
let committed = false;
65-
try {
66-
const result = body();
67-
db.exec("COMMIT");
68-
committed = true;
69-
return result;
70-
} finally {
71-
if (!committed) {
72-
try {
73-
db.exec("ROLLBACK");
74-
} catch {
75-
// already rolled back / no active transaction
76-
}
77-
}
78-
}
79-
}
80-
8163
function rowToState(row: GitSweepCoordinatorRow): GitSweepCoordinatorState {
8264
return {
8365
projectPath: row.project_path,
@@ -110,7 +92,7 @@ export function acquireGitSweepLease(
11092
const cooldownMs = options.cooldownMs ?? GIT_SWEEP_COOLDOWN_MS;
11193
const leaseTtlMs = options.leaseTtlMs ?? GIT_SWEEP_LEASE_TTL_MS;
11294

113-
return runImmediate(db, () => {
95+
return runWriteTransaction(db, () => {
11496
const now = Date.now();
11597
const row = getGitSweepCoordinatorState(db, projectPath);
11698
if (row?.leaseHolder && row.leaseExpiresAt !== null && row.leaseExpiresAt > now) {
@@ -173,7 +155,7 @@ export function renewGitSweepLease(
173155
holderId: string,
174156
leaseTtlMs = GIT_SWEEP_LEASE_TTL_MS,
175157
): boolean {
176-
return runImmediate(db, () => {
158+
return runWriteTransaction(db, () => {
177159
const now = Date.now();
178160
const leaseExpiresAt = now + leaseTtlMs;
179161
const result = db
@@ -194,7 +176,7 @@ export function markGitSweepSuccessAndRelease(
194176
projectPath: string,
195177
holderId: string,
196178
): boolean {
197-
return runImmediate(db, () => {
179+
return runWriteTransaction(db, () => {
198180
const now = Date.now();
199181
const result = db
200182
.prepare(
@@ -212,7 +194,7 @@ export function markGitSweepSuccessAndRelease(
212194
}
213195

214196
export function releaseGitSweepLease(db: Database, projectPath: string, holderId: string): void {
215-
runImmediate(db, () => {
197+
runWriteTransaction(db, () => {
216198
db.prepare(
217199
`UPDATE git_sweep_coordinator
218200
SET lease_holder = NULL,

packages/plugin/src/features/magic-context/key-files/identify-key-files.ts

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { getHarness } from "../../../shared/harness";
1010
import { shouldKeepSubagents } from "../../../shared/keep-subagents";
1111
import { log } from "../../../shared/logger";
1212
import type { Database } from "../../../shared/sqlite";
13+
import { runWriteTransaction } from "../../../shared/write-transaction";
1314
import { peekLeaseHolderAndExpiry, renewLease } from "../dreamer/lease";
1415
import { recordChildInvocation } from "../subagent-token-capture";
1516
import { isAftAvailable } from "./aft-availability";
@@ -423,9 +424,7 @@ export function commitKeyFiles(args: {
423424
);
424425
const generatedAt = Date.now();
425426
const bump = args.bumpVersion ?? bumpKeyFilesVersion;
426-
args.db.exec("BEGIN IMMEDIATE");
427-
let committed = false;
428-
try {
427+
return runWriteTransaction(args.db, () => {
429428
if (!peekLeaseHolderAndExpiry(args.db, args.leaseHolderId)) {
430429
log(`key-files commit aborted: lease lost (holder ${args.leaseHolderId})`);
431430
return null;
@@ -440,21 +439,11 @@ export function commitKeyFiles(args: {
440439
args.configHash,
441440
);
442441
const version = bump(args.db, projectPath);
443-
args.db.exec("COMMIT");
444-
committed = true;
445442
log(
446443
`key-files committed: ${resolved.length} files, version=${version}, ${resolved.filter((r) => r.staleReason).length} pre-stale`,
447444
);
448445
return version;
449-
} finally {
450-
if (!committed) {
451-
try {
452-
args.db.exec("ROLLBACK");
453-
} catch {
454-
// no active transaction
455-
}
456-
}
457-
}
446+
});
458447
}
459448

460449
async function runKeyFilesLlm(args: {

packages/plugin/src/features/magic-context/key-files/project-key-files.ts

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { existsSync, readFileSync, realpathSync } from "node:fs";
33
import { join, resolve, sep } from "node:path";
44
import { log } from "../../../shared/logger";
55
import type { Database } from "../../../shared/sqlite";
6+
import { runWriteTransaction } from "../../../shared/write-transaction";
67

78
export type KeyFileStaleReason = "missing" | "content_drift";
89

@@ -140,9 +141,7 @@ export function replaceProjectKeyFiles(
140141
const generatedByModel = files[0]?.generatedByModel ?? null;
141142
const configHash = files[0]?.generationConfigHash ?? sha256("{}");
142143

143-
db.exec("BEGIN IMMEDIATE");
144-
let committed = false;
145-
try {
144+
return runWriteTransaction(db, () => {
146145
db.prepare("DELETE FROM project_key_files WHERE project_path = ?").run(resolvedProjectPath);
147146
insertResolvedKeyFiles(
148147
db,
@@ -152,19 +151,8 @@ export function replaceProjectKeyFiles(
152151
generatedByModel,
153152
configHash,
154153
);
155-
const version = bumpKeyFilesVersion(db, resolvedProjectPath);
156-
db.exec("COMMIT");
157-
committed = true;
158-
return version;
159-
} finally {
160-
if (!committed) {
161-
try {
162-
db.exec("ROLLBACK");
163-
} catch {
164-
// no active transaction
165-
}
166-
}
167-
}
154+
return bumpKeyFilesVersion(db, resolvedProjectPath);
155+
});
168156
}
169157

170158
export function insertResolvedKeyFiles(

packages/plugin/src/features/magic-context/message-index.ts

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type { RawMessage } from "../../hooks/magic-context/read-session-raw";
77
import { getHarness } from "../../shared/harness";
88
import type { Database, Statement as PreparedStatement } from "../../shared/sqlite";
99
import { removeSystemReminders } from "../../shared/system-directive";
10+
import { runWriteTransaction } from "../../shared/write-transaction";
1011
import { clearCompressionDepth } from "./compression-depth-storage";
1112

1213
interface MessageHistoryIndexRow {
@@ -218,9 +219,7 @@ export function indexMessagesAfterOrdinal(
218219
// is reflected, so the second skips those ordinals and inserts nothing
219220
// duplicate. The bulk SELECT of existing message-ids is still avoided (it
220221
// held the writer lock too long on ~30k-row sessions).
221-
db.exec("BEGIN IMMEDIATE");
222-
let committed = false;
223-
try {
222+
return runWriteTransaction(db, () => {
224223
// Re-read under the lock: another process may have advanced the
225224
// watermark between the caller's out-of-transaction read and now.
226225
const effectiveWatermark = Math.max(
@@ -245,18 +244,8 @@ export function indexMessagesAfterOrdinal(
245244
// Never regress a higher watermark a concurrent writer may have set.
246245
const newWatermark = Math.max(effectiveWatermark, finalWatermark);
247246
getUpsertIndexStatement(db).run(sessionId, newWatermark, now, getHarness());
248-
db.exec("COMMIT");
249-
committed = true;
250-
} finally {
251-
if (!committed) {
252-
try {
253-
db.exec("ROLLBACK");
254-
} catch {
255-
// already rolled back / no active transaction
256-
}
257-
}
258-
}
259-
return inserted;
247+
return inserted;
248+
});
260249
}
261250

262251
export function ensureMessagesIndexed(

packages/plugin/src/features/magic-context/workspaces.ts

Lines changed: 3 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createHash } from "node:crypto";
22
import type { Database } from "../../shared/sqlite";
3+
import { runWriteTransaction } from "../../shared/write-transaction";
34
import { V2_MEMORY_CATEGORIES } from "./memory/constants";
45
import { normalizeStoredProjectPath, storedPathBelongsToIdentity } from "./project-identity";
56

@@ -271,11 +272,6 @@ export function computeWorkspaceEpochFingerprint(
271272
return hash.digest("hex");
272273
}
273274

274-
function isInTransaction(db: Database): boolean {
275-
const candidate = db as unknown as { inTransaction?: unknown; isTransaction?: unknown };
276-
return candidate.inTransaction === true || candidate.isTransaction === true;
277-
}
278-
279275
function workspaceMembersForIdentity(db: Database, identity: string): string[] {
280276
if (!tableExists(db, "workspace_members")) return [identity];
281277
const rows = db
@@ -313,22 +309,7 @@ export function bumpEpochsForWorkspaceMembers(
313309
now = Date.now(),
314310
): void {
315311
const run = () => bumpEpochRows(db, workspaceMembersForIdentity(db, identity), now);
316-
if (isInTransaction(db)) {
317-
run();
318-
return;
319-
}
320-
db.exec("BEGIN IMMEDIATE");
321-
try {
322-
run();
323-
db.exec("COMMIT");
324-
} catch (error) {
325-
try {
326-
db.exec("ROLLBACK");
327-
} catch {
328-
// ignore rollback failures from an already-closed transaction
329-
}
330-
throw error;
331-
}
312+
runWriteTransaction(db, run);
332313
}
333314

334315
export function bumpEpochsForWorkspaceMemberSet(
@@ -337,20 +318,5 @@ export function bumpEpochsForWorkspaceMemberSet(
337318
now = Date.now(),
338319
): void {
339320
const run = () => bumpEpochRows(db, identities, now);
340-
if (isInTransaction(db)) {
341-
run();
342-
return;
343-
}
344-
db.exec("BEGIN IMMEDIATE");
345-
try {
346-
run();
347-
db.exec("COMMIT");
348-
} catch (error) {
349-
try {
350-
db.exec("ROLLBACK");
351-
} catch {
352-
// ignore rollback failures from an already-closed transaction
353-
}
354-
throw error;
355-
}
321+
runWriteTransaction(db, run);
356322
}

0 commit comments

Comments
 (0)