From 1912cf93abfb1908cf17fe2d9fc4b6546c152249 Mon Sep 17 00:00:00 2001 From: Dr Kennedy Umege Date: Fri, 26 Jun 2026 11:53:44 +0100 Subject: [PATCH] fix: align gc dry-run and wal failures --- packages/coding-agent/src/cli/gc-cli.ts | 51 +++--------- packages/coding-agent/src/config/settings.ts | 26 +++++- packages/coding-agent/test/gc-cli.test.ts | 87 +++++++++++++++++++- 3 files changed, 121 insertions(+), 43 deletions(-) diff --git a/packages/coding-agent/src/cli/gc-cli.ts b/packages/coding-agent/src/cli/gc-cli.ts index fd6d086a4..696da259f 100644 --- a/packages/coding-agent/src/cli/gc-cli.ts +++ b/packages/coding-agent/src/cli/gc-cli.ts @@ -3,9 +3,7 @@ import * as fs from "node:fs/promises"; import * as path from "node:path"; import { gunzipSync, gzipSync } from "node:zlib"; import { getAgentDir, getBlobsDir, getHistoryDbPath, getModelDbPath, getSessionsDir } from "@oh-my-pi/pi-utils"; -import { YAML } from "bun"; import { Settings } from "../config/settings"; -import { getDefault } from "../config/settings-schema"; import { listSessionsReadOnly, type SessionInfo, type SessionStatus } from "../session/session-listing"; import { FileSessionStorage } from "../session/session-storage"; @@ -129,53 +127,19 @@ interface GcLockSnapshot { text: string; } -type RawConfig = Record; - function numberSetting(value: number | undefined, fallback: number): number { if (value === undefined || !Number.isFinite(value)) return fallback; return Math.max(0, Math.floor(value)); } -function rawConfigValue(config: RawConfig, pathKey: string): unknown { - let current: unknown = config; - for (const segment of pathKey.split(".")) { - if (current === null || current === undefined || typeof current !== "object" || Array.isArray(current)) { - return undefined; - } - current = (current as RawConfig)[segment]; - } - return current; -} - -function booleanConfigValue(config: RawConfig, pathKey: string, fallback: boolean): boolean { - const value = rawConfigValue(config, pathKey); - return typeof value === "boolean" ? value : fallback; -} - -function numberConfigValue(config: RawConfig, pathKey: string, fallback: number): number { - const value = rawConfigValue(config, pathKey); - return typeof value === "number" && Number.isFinite(value) ? value : fallback; -} - -async function loadConfigReadOnly(agentDir: string): Promise { - try { - const parsed = YAML.parse(await Bun.file(path.join(agentDir, "config.yml")).text()); - if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return {}; - return parsed as RawConfig; - } catch { - return {}; - } -} - async function resolveOptions(flags: GcCommandFlags): Promise { const agentDir = path.resolve(flags.agentDir ?? getAgentDir()); const selected = flags.blobs === true || flags.archive === true || flags.wal === true; - const config = flags.apply === true ? undefined : await loadConfigReadOnly(agentDir); - const settings = flags.apply === true ? await Settings.init({ agentDir }) : undefined; - const getBoolean = (pathKey: "gc.blobs" | "gc.archive" | "gc.wal") => - settings?.get(pathKey) ?? booleanConfigValue(config ?? {}, pathKey, getDefault(pathKey)); + const settings = + flags.apply === true ? await Settings.init({ agentDir }) : await Settings.loadReadOnly({ agentDir }); + const getBoolean = (pathKey: "gc.blobs" | "gc.archive" | "gc.wal") => settings.get(pathKey); const getNumber = (pathKey: "gc.coldArchiveAfterDays" | "gc.retainNewestGlobal" | "gc.retainNewestPerCwd") => - settings?.get(pathKey) ?? numberConfigValue(config ?? {}, pathKey, getDefault(pathKey)); + settings.get(pathKey); return { apply: flags.apply === true, json: flags.json === true, @@ -674,10 +638,11 @@ async function checkpointWal(dbPath: string, apply: boolean): Promise 0 || result.walBytes > 0)) { + throw new Error(`WAL checkpoint failed for ${dbPath}: busy=${result.busy}, walBytes=${result.walBytes}`); + } + result.checkpointed = checkpointAttempted; return result; } diff --git a/packages/coding-agent/src/config/settings.ts b/packages/coding-agent/src/config/settings.ts index 977a8ae75..f41d6b135 100644 --- a/packages/coding-agent/src/config/settings.ts +++ b/packages/coding-agent/src/config/settings.ts @@ -61,6 +61,8 @@ export interface SettingsOptions { agentDir?: string; /** Don't persist to disk (for tests) */ inMemory?: boolean; + /** Read config sources without opening storage or writing migrations */ + readOnly?: boolean; /** Initial overrides */ overrides?: Partial>; /** Extra config.yml-style overlays loaded after global/project settings */ @@ -228,7 +230,7 @@ export class Settings { this.#agentDir = path.normalize(options.agentDir ?? getAgentDir()); this.#configPath = options.inMemory ? null : path.join(this.#agentDir, "config.yml"); this.#configFiles = options.configFiles?.map(file => path.resolve(this.#cwd, expandTilde(file))) ?? []; - this.#persist = !options.inMemory; + this.#persist = !options.inMemory && options.readOnly !== true; if (options.overrides) { for (const [key, value] of Object.entries(options.overrides)) { @@ -270,6 +272,15 @@ export class Settings { ); } + /** + * Load effective settings from config.yml and project providers without + * opening agent.db, migrating legacy settings, or writing marker files. + */ + static loadReadOnly(options: SettingsOptions = {}): Promise { + const instance = new Settings({ ...options, readOnly: true }); + return instance.#loadReadOnly(); + } + /** * Create an isolated instance for testing. * Does not affect the global singleton. @@ -583,6 +594,19 @@ export class Settings { return this; } + async #loadReadOnly(): Promise { + const projectPromise = this.#loadProjectSettings(); + + if (this.#configPath) { + this.#global = await this.#loadYaml(this.#configPath); + } + + this.#project = await projectPromise; + this.#configOverlay = await this.#loadConfigOverlays(); + this.#rebuildMerged(); + return this; + } + async #loadYaml(filePath: string): Promise { try { const content = await Bun.file(filePath).text(); diff --git a/packages/coding-agent/test/gc-cli.test.ts b/packages/coding-agent/test/gc-cli.test.ts index 7304e064c..97195bd39 100644 --- a/packages/coding-agent/test/gc-cli.test.ts +++ b/packages/coding-agent/test/gc-cli.test.ts @@ -5,7 +5,14 @@ import * as os from "node:os"; import * as path from "node:path"; import { gunzipSync } from "node:zlib"; import { runGcCommand } from "@oh-my-pi/pi-coding-agent/cli/gc-cli"; -import { getAgentDir, getBlobsDir, getHistoryDbPath, getSessionsDir, setAgentDir } from "@oh-my-pi/pi-utils"; +import { + getAgentDir, + getBlobsDir, + getHistoryDbPath, + getSessionsDir, + setAgentDir, + setProjectDir, +} from "@oh-my-pi/pi-utils"; import { runCli } from "../src/cli"; import { beginSettingsTest, restoreSettingsTestState, type SettingsTestState } from "./helpers/settings-test-state"; @@ -101,6 +108,12 @@ async function writeConfig(agentDir: string, body: string): Promise { await Bun.write(path.join(agentDir, "config.yml"), body); } +async function writeProjectConfig(projectDir: string, body: string): Promise { + const configDir = path.join(projectDir, ".omp"); + await fs.mkdir(configDir, { recursive: true }); + await Bun.write(path.join(configDir, "config.yml"), body); +} + describe("runGcCommand blob sweep", () => { test("uses the active configured agent dir when --agent-dir is omitted", async () => { const originalAgentDir = getAgentDir(); @@ -246,6 +259,50 @@ describe("runGcCommand blob sweep", () => { expect(await Bun.file(path.join(root, "agent.db")).exists()).toBe(false); expect(await Bun.file(path.join(root, "settings.json.bak")).exists()).toBe(false); }); + + test("dry-run merges project gc settings like apply without initializing settings storage", async () => { + const projectRoot = path.join(root, "project-root"); + await fs.mkdir(projectRoot, { recursive: true }); + setProjectDir(projectRoot); + await writeSession(root, "project", "archive-me", "complete", { ageDays: 10 }); + await writeConfig( + root, + [ + "gc:", + " blobs: false", + " archive: false", + " wal: false", + " coldArchiveAfterDays: 30", + " retainNewestGlobal: 1", + " retainNewestPerCwd: 1", + "", + ].join("\n"), + ); + await writeProjectConfig( + projectRoot, + [ + "gc:", + " archive: true", + " coldArchiveAfterDays: 7", + " retainNewestGlobal: 0", + " retainNewestPerCwd: 0", + "", + ].join("\n"), + ); + + const dryRun = await runGcCommand({ flags: { agentDir: root } }); + + expect(dryRun.blobs).toBeUndefined(); + expect(dryRun.archive?.wouldArchive).toBe(1); + expect(dryRun.archive?.archived).toBe(0); + expect(dryRun.wal).toBeUndefined(); + expect(await Bun.file(path.join(root, "agent.db")).exists()).toBe(false); + expect(await Bun.file(path.join(root, "settings.json.bak")).exists()).toBe(false); + + const applied = await runGcCommand({ flags: { agentDir: root, apply: true } }); + + expect(applied.archive?.archived).toBe(1); + }); }); describe("runGcCommand history checkpoint", () => { @@ -295,6 +352,34 @@ describe("runGcCommand history checkpoint", () => { expect(await Bun.file(path.join(root, "gc.lock")).exists()).toBe(false); }); + + test("--apply reports busy WAL checkpoints and releases the gc lock", async () => { + const dbPath = getHistoryDbPath(root); + await fs.mkdir(path.dirname(dbPath), { recursive: true }); + const writer = new Database(dbPath); + const reader = new Database(dbPath); + try { + writer.run("PRAGMA journal_mode=WAL"); + writer.run("CREATE TABLE history (id INTEGER PRIMARY KEY, prompt TEXT)"); + writer.run("INSERT INTO history (prompt) VALUES ('before-reader')"); + reader.run("PRAGMA journal_mode=WAL"); + reader.run("BEGIN"); + reader.prepare("SELECT * FROM history").all(); + writer.run("INSERT INTO history (prompt) VALUES ('after-reader')"); + + await expect(runGcCommand({ flags: { agentDir: root, wal: true, apply: true } })).rejects.toThrow( + `WAL checkpoint failed for ${dbPath}: busy=1`, + ); + + expect(await Bun.file(path.join(root, "gc.lock")).exists()).toBe(false); + } finally { + try { + reader.run("COMMIT"); + } catch {} + reader.close(); + writer.close(); + } + }, 10_000); }); describe("runGcCommand cold-session archive", () => {