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

fix: address gc review feedback

Browse Source
This commit is contained in:
Dr Kennedy Umege
2026-06-26 09:07:02 +01:00
parent 4ead62cace
commit 8da76c80fc
5 changed files with 124 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/src/cli/gc-cli.ts
+26 -10
View File
@@ -3,7 +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 { getDefault } from "../config/settings-schema";
import { Settings } from "../config/settings";
import { listSessionsReadOnly, type SessionInfo, type SessionStatus } from "../session/session-listing";
import { FileSessionStorage } from "../session/session-storage";
@@ -14,6 +14,7 @@ const JSONL_GLOB = new Bun.Glob("**/*.jsonl");
const JSONL_GZ_GLOB = new Bun.Glob("**/*.jsonl.gz");
const ACTIVE_STATUSES: ReadonlySet<SessionStatus> = new Set(["pending", "interrupted", "unknown"]);
const DAY_MS = 86_400_000;
const GC_WRITE_GRACE_MS = 5 * 60_000;
const SESSION_SUFFIX = ".jsonl";
const COMPRESSED_SESSION_SUFFIX = ".jsonl.gz";
@@ -120,18 +121,20 @@ function numberSetting(value: number | undefined, fallback: number): number {
return Math.max(0, Math.floor(value));
}
function resolveOptions(flags: GcCommandFlags): ResolvedGcOptions {
async function resolveOptions(flags: GcCommandFlags): Promise<ResolvedGcOptions> {
const agentDir = path.resolve(flags.agentDir ?? getAgentDir());
const settings = await Settings.init({ agentDir });
const selected = flags.blobs === true || flags.archive === true || flags.wal === true;
return {
apply: flags.apply === true,
json: flags.json === true,
agentDir: path.resolve(flags.agentDir ?? getAgentDir()),
runBlobs: selected ? flags.blobs === true : getDefault("gc.blobs"),
runArchive: selected ? flags.archive === true : getDefault("gc.archive"),
runWal: selected ? flags.wal === true : getDefault("gc.wal"),
coldArchiveAfterDays: numberSetting(flags.coldArchiveAfterDays, getDefault("gc.coldArchiveAfterDays")),
retainNewestGlobal: numberSetting(flags.retainNewestGlobal, getDefault("gc.retainNewestGlobal")),
retainNewestPerCwd: numberSetting(flags.retainNewestPerCwd, getDefault("gc.retainNewestPerCwd")),
agentDir,
runBlobs: selected ? flags.blobs === true : settings.get("gc.blobs"),
runArchive: selected ? flags.archive === true : settings.get("gc.archive"),
runWal: selected ? flags.wal === true : settings.get("gc.wal"),
coldArchiveAfterDays: numberSetting(flags.coldArchiveAfterDays, settings.get("gc.coldArchiveAfterDays")),
retainNewestGlobal: numberSetting(flags.retainNewestGlobal, settings.get("gc.retainNewestGlobal")),
retainNewestPerCwd: numberSetting(flags.retainNewestPerCwd, settings.get("gc.retainNewestPerCwd")),
};
}
@@ -258,8 +261,10 @@ async function runBlobGc(options: ResolvedGcOptions, archiveSessionsRoot: string
errors: [],
};
const deleteBeforeMs = Date.now() - GC_WRITE_GRACE_MS;
for (const candidate of candidates) {
if (referenced.has(candidate.hash)) continue;
if (candidate.mtimeMs > deleteBeforeMs) continue;
result.wouldDelete += candidate.paths.length;
result.bytes += candidate.bytes;
if (!options.apply) continue;
@@ -462,12 +467,17 @@ async function runArchiveGc(options: ResolvedGcOptions, archiveRoot: string): Pr
const candidates: ArchiveCandidate[] = [];
let inactiveSeen = 0;
const inactiveSeenByCwd = new Map<string, number>();
const archiveBeforeMs = Date.now() - GC_WRITE_GRACE_MS;
for (const session of sessions) {
if (session.status && ACTIVE_STATUSES.has(session.status)) {
result.skippedActive += 1;
continue;
}
if (session.modified.getTime() > archiveBeforeMs) {
result.skippedActive += 1;
continue;
}
const cwdKey = sessionCwdKey(sessionsRoot, session);
const cwdSeen = inactiveSeenByCwd.get(cwdKey) ?? 0;
const keepGlobal = inactiveSeen < options.retainNewestGlobal;
@@ -541,6 +551,12 @@ async function checkpointWal(dbPath: string, apply: boolean): Promise<WalCheckpo
} finally {
db.close();
}
try {
result.walBytes = (await fs.stat(walPath)).size;
} catch (error) {
if (codeOf(error) !== "ENOENT") throw error;
result.walBytes = 0;
}
return result;
}
@@ -624,7 +640,7 @@ function renderText(result: GcResult): string {
}
export async function runGcCommand(args: GcCommandArgs): Promise<GcResult> {
const options = resolveOptions(args.flags);
const options = await resolveOptions(args.flags);
const archiveRoot = getArchivedSessionsDir(options.agentDir);
const result = await withGcLock(options.agentDir, async lockPath => {
const next: GcResult = { agentDir: options.agentDir, apply: options.apply, lockPath };
packages/coding-agent/src/config/settings-schema.ts
-3
View File
@@ -4630,8 +4630,6 @@ export const SETTINGS_SCHEMA = {
default: "unset" as const,
},
"gc.auto": { type: "boolean", default: false },
"gc.blobs": { type: "boolean", default: true },
"gc.archive": { type: "boolean", default: true },
@@ -4890,7 +4888,6 @@ export interface CodexResetsSettings {
}
export interface GcSettings {
auto: boolean;
blobs: boolean;
archive: boolean;
wal: boolean;
packages/coding-agent/src/extensibility/plugins/legacy-pi-bundled-keys.ts
+2
View File
@@ -249,6 +249,7 @@ export const BUNDLED_PI_REGISTRY_KEYS: ReadonlySet<string> = new Set([
"@oh-my-pi/pi-coding-agent/cli/flag-tables",
"@oh-my-pi/pi-coding-agent/cli/gallery-cli",
"@oh-my-pi/pi-coding-agent/cli/gallery-screenshot",
"@oh-my-pi/pi-coding-agent/cli/gc-cli",
"@oh-my-pi/pi-coding-agent/cli/grep-cli",
"@oh-my-pi/pi-coding-agent/cli/grievances-cli",
"@oh-my-pi/pi-coding-agent/cli/initial-message",
@@ -293,6 +294,7 @@ export const BUNDLED_PI_REGISTRY_KEYS: ReadonlySet<string> = new Set([
"@oh-my-pi/pi-coding-agent/commands/config",
"@oh-my-pi/pi-coding-agent/commands/dry-balance",
"@oh-my-pi/pi-coding-agent/commands/gallery",
"@oh-my-pi/pi-coding-agent/commands/gc",
"@oh-my-pi/pi-coding-agent/commands/grep",
"@oh-my-pi/pi-coding-agent/commands/grievances",
"@oh-my-pi/pi-coding-agent/commands/install",
packages/coding-agent/src/extensibility/plugins/legacy-pi-bundled-registry.ts
+6
View File
@@ -223,6 +223,7 @@ import * as bundledPiCodingAgentCliGalleryFixturesShell from "@oh-my-pi/pi-codin
import * as bundledPiCodingAgentCliGalleryFixturesTypes from "@oh-my-pi/pi-coding-agent/cli/gallery-fixtures/types";
import * as bundledPiCodingAgentCliGalleryFixturesWeb from "@oh-my-pi/pi-coding-agent/cli/gallery-fixtures/web";
import * as bundledPiCodingAgentCliGalleryScreenshot from "@oh-my-pi/pi-coding-agent/cli/gallery-screenshot";
import * as bundledPiCodingAgentCliGcCli from "@oh-my-pi/pi-coding-agent/cli/gc-cli";
import * as bundledPiCodingAgentCliGrepCli from "@oh-my-pi/pi-coding-agent/cli/grep-cli";
import * as bundledPiCodingAgentCliGrievancesCli from "@oh-my-pi/pi-coding-agent/cli/grievances-cli";
import * as bundledPiCodingAgentCliInitialMessage from "@oh-my-pi/pi-coding-agent/cli/initial-message";
@@ -255,6 +256,7 @@ import * as bundledPiCodingAgentCommandsCompletions from "@oh-my-pi/pi-coding-ag
import * as bundledPiCodingAgentCommandsConfig from "@oh-my-pi/pi-coding-agent/commands/config";
import * as bundledPiCodingAgentCommandsDryBalance from "@oh-my-pi/pi-coding-agent/commands/dry-balance";
import * as bundledPiCodingAgentCommandsGallery from "@oh-my-pi/pi-coding-agent/commands/gallery";
import * as bundledPiCodingAgentCommandsGc from "@oh-my-pi/pi-coding-agent/commands/gc";
import * as bundledPiCodingAgentCommandsGrep from "@oh-my-pi/pi-coding-agent/commands/grep";
import * as bundledPiCodingAgentCommandsGrievances from "@oh-my-pi/pi-coding-agent/commands/grievances";
import * as bundledPiCodingAgentCommandsInstall from "@oh-my-pi/pi-coding-agent/commands/install";
@@ -1503,6 +1505,7 @@ export const BUNDLED_PI_REGISTRY: Readonly<Record<string, Readonly<Record<string
"@oh-my-pi/pi-coding-agent/cli/gallery-screenshot": bundledPiCodingAgentCliGalleryScreenshot as unknown as Readonly<
Record<string, unknown>
>,
"@oh-my-pi/pi-coding-agent/cli/gc-cli": bundledPiCodingAgentCliGcCli as unknown as Readonly<Record<string, unknown>>,
"@oh-my-pi/pi-coding-agent/cli/grep-cli": bundledPiCodingAgentCliGrepCli as unknown as Readonly<
Record<string, unknown>
>,
@@ -1625,6 +1628,9 @@ export const BUNDLED_PI_REGISTRY: Readonly<Record<string, Readonly<Record<string
"@oh-my-pi/pi-coding-agent/commands/gallery": bundledPiCodingAgentCommandsGallery as unknown as Readonly<
Record<string, unknown>
>,
"@oh-my-pi/pi-coding-agent/commands/gc": bundledPiCodingAgentCommandsGc as unknown as Readonly<
Record<string, unknown>
>,
"@oh-my-pi/pi-coding-agent/commands/grep": bundledPiCodingAgentCommandsGrep as unknown as Readonly<
Record<string, unknown>
>,
packages/coding-agent/test/gc-cli.test.ts
+90 -6
View File
@@ -5,14 +5,16 @@ 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 { getDefault } from "@oh-my-pi/pi-coding-agent/config/settings-schema";
import { getAgentDir, getBlobsDir, getHistoryDbPath, getSessionsDir, setAgentDir } from "@oh-my-pi/pi-utils";
import { beginSettingsTest, restoreSettingsTestState, type SettingsTestState } from "./helpers/settings-test-state";
let root: string;
let writes: string[] = [];
let stdoutSpy: { mockRestore(): void } | undefined;
let settingsState: SettingsTestState | undefined;
beforeEach(async () => {
settingsState = beginSettingsTest();
root = await fs.mkdtemp(path.join(os.tmpdir(), "omp-gc-"));
writes = [];
stdoutSpy = spyOn(process.stdout, "write").mockImplementation(chunk => {
@@ -24,6 +26,8 @@ beforeEach(async () => {
afterEach(async () => {
stdoutSpy?.mockRestore();
stdoutSpy = undefined;
restoreSettingsTestState(settingsState);
settingsState = undefined;
await fs.rm(root, { recursive: true, force: true });
});
@@ -74,16 +78,22 @@ async function writeBlob(agentDir: string, hash: string, content: string): Promi
return file;
}
describe("runGcCommand blob sweep", () => {
test("keeps automatic gc disabled by default", () => {
expect(getDefault("gc.auto")).toBe(false);
});
async function agePath(file: string, ageDays = 1): Promise<void> {
const ts = new Date(Date.now() - ageDays * 86_400_000);
await fs.utimes(file, ts, ts);
}
async function writeConfig(agentDir: string, body: string): Promise<void> {
await fs.mkdir(agentDir, { recursive: true });
await Bun.write(path.join(agentDir, "config.yml"), body);
}
describe("runGcCommand blob sweep", () => {
test("uses the active configured agent dir when --agent-dir is omitted", async () => {
const originalAgentDir = getAgentDir();
try {
setAgentDir(root);
await writeBlob(root, hashFor("orphan"), "orphan");
await agePath(await writeBlob(root, hashFor("orphan"), "orphan"));
const result = await runGcCommand({ flags: { blobs: true } });
@@ -97,6 +107,7 @@ describe("runGcCommand blob sweep", () => {
test("dry-run reports unreferenced blobs without deleting them", async () => {
const hash = hashFor("orphan");
const blob = await writeBlob(root, hash, "orphan");
await agePath(blob);
const result = await runGcCommand({ flags: { agentDir: root, blobs: true } });
@@ -110,6 +121,8 @@ describe("runGcCommand blob sweep", () => {
const referencedHash = hashFor("referenced");
const orphan = await writeBlob(root, orphanHash, "orphan");
const referenced = await writeBlob(root, referencedHash, "referenced");
await agePath(orphan);
await agePath(referenced);
await writeSession(root, "project", "session-1", "complete", {
blobRef: `blob:sha256:${referencedHash}`,
});
@@ -121,6 +134,56 @@ describe("runGcCommand blob sweep", () => {
expect(await Bun.file(orphan).exists()).toBe(false);
expect(await Bun.file(referenced).exists()).toBe(true);
});
test("--apply keeps fresh unreferenced blobs out of sweep candidates", async () => {
const blob = await writeBlob(root, hashFor("fresh-orphan"), "fresh");
const result = await runGcCommand({ flags: { agentDir: root, blobs: true, apply: true } });
expect(result.blobs?.wouldDelete).toBe(0);
expect(result.blobs?.deleted).toBe(0);
expect(await Bun.file(blob).exists()).toBe(true);
});
test("uses configured gc selectors and retention defaults", async () => {
await agePath(await writeBlob(root, hashFor("orphan"), "orphan"));
await writeSession(root, "project", "archive-me", "complete", { ageDays: 10 });
await writeConfig(
root,
[
"gc:",
" blobs: false",
" archive: true",
" wal: false",
" coldArchiveAfterDays: 7",
" retainNewestGlobal: 0",
" retainNewestPerCwd: 0",
"",
].join("\n"),
);
const result = await runGcCommand({ flags: { agentDir: root, apply: true } });
expect(result.blobs).toBeUndefined();
expect(result.wal).toBeUndefined();
expect(result.archive?.archived).toBe(1);
expect(await Bun.file(path.join(root, "archive", "sessions", "project", "archive-me.jsonl.gz")).exists()).toBe(
true,
);
});
test("explicit selectors override disabled gc config", async () => {
const blob = await writeBlob(root, hashFor("orphan"), "orphan");
await agePath(blob);
await writeConfig(root, ["gc:", " blobs: false", " archive: false", " wal: false", ""].join("\n"));
const result = await runGcCommand({ flags: { agentDir: root, blobs: true, apply: true } });
expect(result.blobs?.deleted).toBe(1);
expect(result.archive).toBeUndefined();
expect(result.wal).toBeUndefined();
expect(await Bun.file(blob).exists()).toBe(false);
});
});
describe("runGcCommand history checkpoint", () => {
@@ -156,6 +219,7 @@ describe("runGcCommand history checkpoint", () => {
const result = await runGcCommand({ flags: { agentDir: root, wal: true, apply: true } });
expect(result.wal?.checkpointed).toBe(true);
expect(result.wal?.walBytes).toBe(0);
expect((await fs.stat(`${dbPath}-wal`)).size).toBe(0);
});
});
@@ -254,6 +318,25 @@ describe("runGcCommand cold-session archive", () => {
expect(await Bun.file(session).exists()).toBe(false);
});
test("does not archive fresh completed sessions that may still be live", async () => {
const session = await writeSession(root, "project", "fresh-complete", "complete");
const result = await runGcCommand({
flags: {
agentDir: root,
archive: true,
coldArchiveAfterDays: 0,
retainNewestGlobal: 0,
retainNewestPerCwd: 0,
apply: true,
},
});
expect(result.archive?.archived).toBe(0);
expect(result.archive?.skippedActive).toBe(1);
expect(await Bun.file(session).exists()).toBe(true);
});
test("dry-run does not recover orphaned session backups", async () => {
const sessionDir = path.join(getSessionsDir(root), "project");
await fs.mkdir(sessionDir, { recursive: true });
@@ -275,6 +358,7 @@ describe("runGcCommand cold-session archive", () => {
const referencedHash = hashFor("archived-reference");
const referenced = await writeBlob(root, referencedHash, "referenced");
await writeBlob(root, hashFor("orphan"), "orphan");
await agePath(path.join(getBlobsDir(root), hashFor("orphan")));
await writeSession(root, "project", "archive-me", "complete", {
ageDays: 90,
blobRef: `blob:sha256:${referencedHash}`,