DB File Lock (rebase of #1050 by @grfwings)#1175
Conversation
…ing eraly, derive expected counts from plan arrays
…vert to old behavior
…cannot reopen while a previous handle is still disposing
RhysSullivan
force-pushed
the
db-file-lock
branch
from
aff9a70 to
1109289
Compare
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-marketing | 1109289 | Commit Preview URL Branch Preview URL |
Jun 28 2026, 09:31 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-cloud | 1109289 | Jun 28 2026, 09:31 AM |
Cloudflare previewTorn down — the PR is closed. |
@executor-js/cli
@executor-js/config
@executor-js/execution
@executor-js/sdk
@executor-js/codemode-core
@executor-js/runtime-quickjs
@executor-js/plugin-file-secrets
@executor-js/plugin-graphql
@executor-js/plugin-keychain
@executor-js/plugin-mcp
@executor-js/plugin-onepassword
@executor-js/plugin-openapi
executor
commit: |
Greptile SummaryThis PR introduces a SQLite-based data-directory ownership lock (
Confidence Score: 4/5The ownership lock and migration journal logic are well-constructed and the test suite covers the core race (cross-process SIGKILL recovery). The two style fixes are trivial. The ownership primitive, crash-recovery journal, and shutdown cleanup chains are all correctly structured. The only substantive finding is that resolveLegacyMigrationsFolder can create up to two leaked tmpdirs per migration run, which accumulates on Windows across reboots. The em-dash violations are mechanical style fixes against the AGENTS.md rule. apps/local/src/db/v1-v2-migration.ts — the tmpdir materialization inside replayLegacyV1Migrations and tolerantReplayLegacyChain can be called twice in one migration without cleanup. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant C as CLI/Desktop
participant OD as openOwnedLocalDatabase
participant DO as acquireDataDirOwnership
participant M as migrateLocalV1ToV2IfNeeded
participant DB as createSqliteFumaDb
participant L as Lock DB (data.db.owner-lock)
C->>OD: openOwnedLocalDatabase(dataDir)
OD->>DO: acquireDataDirOwnership(dataDir)
DO->>L: "createClient (journal_mode=DELETE, busy_timeout=0)"
DO->>L: BEGIN EXCLUSIVE
alt Lock already held by another process
L-->>DO: SQLITE_BUSY
DO-->>C: throw DataDirOwnershipHeld
Note over C: CLI detects error, attaches to existing server
else Lock acquired
DO-->>OD: "{ lockPath, release }"
OD->>M: migrateLocalV1ToV2IfNeeded(sqlitePath)
M->>M: recoverV1V2Migration (journal check)
alt v1 database found
M->>M: copySqliteFileSet to staging
M->>M: replay legacy migrations
M->>M: insertPlan (BEGIN/COMMIT in staging)
M->>M: verifyStagingDatabase
M->>M: completeJournaledFlip (atomic rename)
end
M-->>OD: LocalV1V2MigrationResult
OD->>DB: createSqliteFumaDb(sqlitePath)
DB-->>OD: SqliteFumaDb
OD-->>C: "OwnedLocalDatabase { db, close }"
Note over L: Exclusive lock held for process lifetime
end
C->>OD: database.close()
OD->>DB: db.close()
OD->>DO: ownership.release()
DO->>L: ROLLBACK + close()
Note over L: OS releases file lock
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant C as CLI/Desktop
participant OD as openOwnedLocalDatabase
participant DO as acquireDataDirOwnership
participant M as migrateLocalV1ToV2IfNeeded
participant DB as createSqliteFumaDb
participant L as Lock DB (data.db.owner-lock)
C->>OD: openOwnedLocalDatabase(dataDir)
OD->>DO: acquireDataDirOwnership(dataDir)
DO->>L: "createClient (journal_mode=DELETE, busy_timeout=0)"
DO->>L: BEGIN EXCLUSIVE
alt Lock already held by another process
L-->>DO: SQLITE_BUSY
DO-->>C: throw DataDirOwnershipHeld
Note over C: CLI detects error, attaches to existing server
else Lock acquired
DO-->>OD: "{ lockPath, release }"
OD->>M: migrateLocalV1ToV2IfNeeded(sqlitePath)
M->>M: recoverV1V2Migration (journal check)
alt v1 database found
M->>M: copySqliteFileSet to staging
M->>M: replay legacy migrations
M->>M: insertPlan (BEGIN/COMMIT in staging)
M->>M: verifyStagingDatabase
M->>M: completeJournaledFlip (atomic rename)
end
M-->>OD: LocalV1V2MigrationResult
OD->>DB: createSqliteFumaDb(sqlitePath)
DB-->>OD: SqliteFumaDb
OD-->>C: "OwnedLocalDatabase { db, close }"
Note over L: Exclusive lock held for process lifetime
end
C->>OD: database.close()
OD->>DB: db.close()
OD->>DO: ownership.release()
DO->>L: ROLLBACK + close()
Note over L: OS releases file lock
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| import * as Effect from "effect/Effect"; | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Daemon discovery + spawn-dedup state — hints, not DB safety. |
There was a problem hiding this comment.
—), which is banned by AGENTS.md everywhere in the codebase (prose, docs, code comments, commit messages). Replace with a colon or restructure the sentence.
| // Daemon discovery + spawn-dedup state — hints, not DB safety. | |
| // Daemon discovery + spawn-dedup state: hints, not DB safety. |
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| readonly path: string; | ||
| } | ||
| // --------------------------------------------------------------------------- | ||
| // server-control/server.json — a discovery/attach HINT, not an ownership proof. |
There was a problem hiding this comment.
—), which is banned by AGENTS.md everywhere. The same pattern also appears in daemon-state.ts.
| // server-control/server.json — a discovery/attach HINT, not an ownership proof. | |
| // server-control/server.json: a discovery/attach HINT, not an ownership proof. |
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
2 participants
Rebase of #1050 ("DB File Lock") by @grfwings onto current
main, opened as the root of a stack so the desktop recovery fix can stack on top.All commits are authored by @grfwings. This branch only rebases that work onto
main(it was 65 commits behind) and resolves conflicts inapps/local/src/serve.tsandapps/local/src/main.tsagainst main's newer per-resource MCP handler and auth gate, keeping #1050's ownership/dispose changes.Credit: original work and design by @grfwings in #1050.
What it does
The DB file-lock / data-dir ownership layer:
apps/local/src/db/data-dir-ownership.ts,owned-database.tsbusy_timeoutapps/cli/src/daemon.ts,main.ts)The desktop "attach instead of wedge" recovery fix (#1086) is stacked as a child PR on this branch.
Validation
owned-database,data-dir-ownership,v1-v2-migration)Stack