smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge PR #7240: fix(coding-agent): bound synchronous SQLite busy-waits in headless hosts (@pi3123)

Browse Source
This commit is contained in:
can1357
2026-08-01 20:13:39 +02:00
parent 539e7277d0 15ebb96f0f
commit b6e12240c9
8 changed files with 73 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/ai/src/auth-storage.ts
+5 -3
View File
@@ -12,7 +12,7 @@ import { createHash } from "node:crypto";
import * as fs from "node:fs/promises";
import * as path from "node:path";
import { parseAlibabaTokenPlanCredential } from "@oh-my-pi/pi-catalog/wire/alibaba-token-plan";
import { $env, getAgentDbPath, logger } from "@oh-my-pi/pi-utils";
import { $env, getAgentDbPath, getDbBusyTimeoutMs, logger } from "@oh-my-pi/pi-utils";
import type { ApiKeyResolver } from "./auth-retry";
import * as AIError from "./error";
import { isUsageLimitOutcome } from "./error/rate-limit";
@@ -6954,8 +6954,10 @@ export class SqliteAuthCredentialStore implements AuthCredentialStore {
// Install the busy handler BEFORE any lock-taking statement (incl.
// `PRAGMA journal_mode=WAL`, which acquires an exclusive lock during WAL
// recovery). Without this, concurrent omp startups can crash here with
// `SQLITE_BUSY` / `SQLITE_BUSY_RECOVERY`. See issue #2421.
this.#db.run("PRAGMA busy_timeout = 5000");
// `SQLITE_BUSY` / `SQLITE_BUSY_RECOVERY`. See issue #2421. Uses the
// centralized timeout so a headless host keeps its bounded busy wait
// instead of overwriting it with the interactive 5s value.
this.#db.run(`PRAGMA busy_timeout = ${getDbBusyTimeoutMs()}`);
this.#db.run(`
PRAGMA journal_mode=WAL;
PRAGMA synchronous=NORMAL;
packages/coding-agent/CHANGELOG.md
+3
View File
@@ -78,6 +78,9 @@
### Added
- Added `Shift+Up` as a second default for the message dequeue, so the shortcut is reachable in macOS Terminal.app where Option is consumed for character composition.
### Changed
- Headless hosts (print/RPC/ACP/eval/SDK) now use a 1s SQLite `busy_timeout` for the session-critical databases (agent.db, history.db, stats.db), so lock contention no longer freezes the protocol loop for the full interactive 5s timeout; interactive hosts keep the 5s timeout. The interactive-host flag is now declared before settings load so the first database opens see the correct timeout.
## [17.2.1] - 2026-07-30
packages/coding-agent/src/main.ts
+13 -9
View File
@@ -1205,6 +1205,19 @@ export async function runRootCommand(
});
let cwd = getProjectDir();
// Declare the interactive-host flag BEFORE Settings.init so the very first
// session-critical database opens (agent.db/stats.db during settings load)
// can pick the right busy timeout. See getDbBusyTimeoutMs().
const isProtocolMode = mode === "rpc" || mode === "rpc-ui" || mode === "acp";
// Protocol modes own stdin; treating it as prompt text would consume JSON-RPC frames before their transports start.
const pipedInput = isProtocolMode ? undefined : await logger.time("readPipedInput", readPipedInput);
const autoPrint = pipedInput !== undefined && !parsedArgs.print && parsedArgs.mode === undefined;
const isInteractive = !parsedArgs.print && !autoPrint && parsedArgs.mode === undefined;
// Only the interactive host renders a focusable Agent Hub / subagent session
// tree; declare it so headless subagent optimizations (e.g. skipping replan
// title refresh) can tell a focusable process from a print/RPC/eval one.
setInteractiveHost(isInteractive);
const settingsInstance =
deps.settings ?? (await logger.time("settings:init", Settings.init, { cwd, configFiles: parsedArgs.config }));
if (parsedArgs.approvalMode) {
@@ -1227,15 +1240,6 @@ export async function runRootCommand(
if (parsedArgs.noTitle || parsedArgs.mode === "rpc" || parsedArgs.mode === "rpc-ui" || parsedArgs.mode === "acp") {
Bun.env.PI_NO_TITLE = "1";
}
const isProtocolMode = mode === "rpc" || mode === "rpc-ui" || mode === "acp";
// Protocol modes own stdin; treating it as prompt text would consume JSON-RPC frames before their transports start.
const pipedInput = isProtocolMode ? undefined : await logger.time("readPipedInput", readPipedInput);
const autoPrint = pipedInput !== undefined && !parsedArgs.print && parsedArgs.mode === undefined;
const isInteractive = !parsedArgs.print && !autoPrint && parsedArgs.mode === undefined;
// Only the interactive host renders a focusable Agent Hub / subagent session
// tree; declare it so headless subagent optimizations (e.g. skipping replan
// title refresh) can tell a focusable process from a print/RPC/eval one.
setInteractiveHost(isInteractive);
// Initialize discovery system with settings for provider persistence
logger.time("initializeWithSettings", initializeWithSettings, settingsInstance);
packages/coding-agent/src/session/agent-storage.ts
+6 -4
View File
@@ -8,7 +8,7 @@ import {
SqliteAuthCredentialStore,
type StoredAuthCredential,
} from "@oh-my-pi/pi-ai";
import { AsyncDrain, getAgentDbPath, getStatsDbPath, isRecord, logger } from "@oh-my-pi/pi-utils";
import { AsyncDrain, getAgentDbPath, getDbBusyTimeoutMs, getStatsDbPath, isRecord, logger } from "@oh-my-pi/pi-utils";
import type { RawSettings as Settings } from "../config/settings";
/** Row shape for settings table queries */
@@ -199,8 +199,10 @@ ON CONFLICT(model_key) DO UPDATE SET
// Install the busy handler BEFORE any lock-taking statement (incl.
// `PRAGMA journal_mode=WAL`, which acquires an exclusive lock during WAL
// recovery). Without this, concurrent omp startups can crash here with
// `SQLITE_BUSY` / `SQLITE_BUSY_RECOVERY`. See issue #2421.
this.#db.run("PRAGMA busy_timeout = 5000");
// `SQLITE_BUSY` / `SQLITE_BUSY_RECOVERY`. See issue #2421. Headless
// hosts bound the wait so lock contention cannot freeze the protocol
// loop for the full interactive timeout.
this.#db.run(`PRAGMA busy_timeout = ${getDbBusyTimeoutMs()}`);
this.#db.run(`
PRAGMA journal_mode=WAL;
PRAGMA synchronous=NORMAL;
@@ -571,7 +573,7 @@ FROM model_usage_legacy
async backfillModelPerfFromStats(statsDbPath: string): Promise<number> {
const statsDb = new Database(statsDbPath, { readonly: true });
try {
statsDb.run("PRAGMA busy_timeout = 5000");
statsDb.run(`PRAGMA busy_timeout = ${getDbBusyTimeoutMs()}`);
const select = statsDb.prepare(
`SELECT rowid, timestamp, provider, model, output_tokens, duration, ttft
FROM messages
packages/coding-agent/src/session/history-storage.ts
+4 -2
View File
@@ -1,7 +1,7 @@
import { Database, type Statement } from "bun:sqlite";
import * as fs from "node:fs";
import * as path from "node:path";
import { AsyncDrain, getHistoryDbPath, logger } from "@oh-my-pi/pi-utils";
import { AsyncDrain, getDbBusyTimeoutMs, getHistoryDbPath, logger } from "@oh-my-pi/pi-utils";
export interface HistoryEntry {
id: number;
@@ -51,7 +51,9 @@ export class HistoryStorage {
this.#db = new Database(dbPath);
// Install the busy handler BEFORE any lock-taking statement. See #2421.
this.#db.run("PRAGMA busy_timeout = 5000");
// Headless hosts bound the wait so lock contention cannot freeze the
// protocol loop for the full interactive timeout.
this.#db.run(`PRAGMA busy_timeout = ${getDbBusyTimeoutMs()}`);
const hasFts = this.#db.prepare("SELECT 1 FROM sqlite_master WHERE type='table' AND name='history_fts'").get();
this.#db.run(`
packages/utils/CHANGELOG.md
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Changed
- Headless hosts (print/RPC/ACP/eval/SDK) now use a 1s SQLite `busy_timeout` for the session-critical databases (agent.db, history.db, stats.db) via `getDbBusyTimeoutMs()`, so lock contention no longer freezes the protocol loop for the full interactive 5s timeout; interactive hosts keep the 5s timeout.
## [17.2.1] - 2026-07-30
### Added
packages/utils/src/env.ts
+17
View File
@@ -314,6 +314,23 @@ export function setInteractiveHost(interactive: boolean): boolean {
return previous;
}
/**
* SQLite `busy_timeout` for the session-critical databases (agent.db,
* history.db, stats.db).
*
* Interactive hosts tolerate a longer synchronous wait on lock contention
* (SQLITE_BUSY during WAL recovery/checkpoint — see oh-my-pi#2421): the
* operator sees a brief freeze and the statement eventually completes.
* Headless hosts (print/RPC/ACP/eval/SDK) run a protocol on the same thread —
* a multi-second synchronous busy-wait freezes their event loop and stalls
* every in-flight frame with no liveness signal, so they use a short timeout
* and rely on the existing asynchronous open/retry paths to recover from
* contention instead of blocking.
*/
export function getDbBusyTimeoutMs(): number {
return isInteractiveHost() ? 5000 : 1000;
}
/**
* True when this code is running inside a `bun build --compile` standalone
* binary. Detects via the embedded virtual-filesystem path markers
packages/utils/test/env.test.ts
+21 -1
View File
@@ -2,7 +2,7 @@ import { afterEach, describe, expect, it } from "bun:test";
import * as fs from "node:fs";
import * as os from "node:os";
import * as path from "node:path";
import { filterProcessEnv, parseEnvFile } from "@oh-my-pi/pi-utils/env";
import { filterProcessEnv, getDbBusyTimeoutMs, parseEnvFile, setInteractiveHost } from "@oh-my-pi/pi-utils/env";
const tempDirs: string[] = [];
@@ -20,6 +20,26 @@ function writeTempEnv(content: string): string {
return filePath;
}
describe("getDbBusyTimeoutMs", () => {
it("defaults to the bounded headless timeout", () => {
const previous = setInteractiveHost(false);
try {
expect(getDbBusyTimeoutMs()).toBe(1000);
} finally {
setInteractiveHost(previous);
}
});
it("keeps the interactive timeout for interactive hosts", () => {
const previous = setInteractiveHost(true);
try {
expect(getDbBusyTimeoutMs()).toBe(5000);
} finally {
setInteractiveHost(previous);
}
});
});
describe("parseEnvFile", () => {
it("ignores malformed names and nul-containing values", () => {
const filePath = writeTempEnv(