Skip to content

ADE-71: State/sync/config persistence & kvDb hardening: crash-safe writes, migration versioning, and god-file/dedup decomposition #495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
arul28 merged 2 commits into from
Jun 1, 2026
+331 −225
Merged

ADE-71: State/sync/config persistence & kvDb hardening: crash-safe writes, migration versioning, and god-file/dedup decomposition #495

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a173a21
Fix ADE-71 state persistence and kvDb hardening
arul28 May 31, 2026
ab42bc6
Document kvDb flush checkpoint semantics
arul28 May 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 28 additions & 12 deletions apps/desktop/src/main/services/config/projectConfigService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import path from "node:path";
import { createHash } from "node:crypto";
import YAML from "yaml";
import cron from "node-cron";
import { z } from "zod";
import type {
AiConfig,
AiFeatureKey,
Expand Down Expand Up @@ -91,6 +92,27 @@ const AUTOMATION_ACTION_TYPE_SET = new Set<string>([
"run-command",
"ade-action",
]);
const OPTIONAL_STRING_SCHEMA = z.string().optional().catch(undefined);
const OPTIONAL_NUMBER_SCHEMA = z.number().finite().optional().catch(undefined);
const OPTIONAL_BOOL_SCHEMA = z.boolean().optional().catch(undefined);
const STRING_ARRAY_SCHEMA = z.array(z.unknown())
.transform((values) => values
.filter((value): value is string => typeof value === "string")
.map((value) => value.trim())
.filter(Boolean))
.optional()
.catch(() => undefined);
const STRING_MAP_SCHEMA = z.record(z.string(), z.unknown())
.transform((value) => {
const out: Record<string, string> = {};
for (const [key, entry] of Object.entries(value)) {
if (typeof entry === "string") out[key] = entry;
}
return out;
})
.optional()
.catch(() => undefined);
Comment on lines +105 to +114
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Test whether z.record accepts arrays in Zod 4

cat > /tmp/test_zod_record.mjs << 'EOF'
import { z } from 'zod';

const STRING_MAP_SCHEMA = z.record(z.string(), z.unknown())
  .transform((value) => {
    const out = {};
    for (const [key, entry] of Object.entries(value)) {
      if (typeof entry === "string") out[key] = entry;
    }
    return out;
  })
  .optional()
  .catch(() => undefined);

// Test cases
const testCases = [
  { input: { a: "x", b: "y" }, label: "object with strings" },
  { input: { a: "x", b: 123 }, label: "object with mixed types" },
  { input: ["x", "y", "z"], label: "array of strings" },
  { input: [1, 2, 3], label: "array of numbers" },
  { input: "not-an-object", label: "string" },
  { input: undefined, label: "undefined" },
];

console.log("Testing STRING_MAP_SCHEMA with Zod 4:\n");
for (const { input, label } of testCases) {
  try {
    const result = STRING_MAP_SCHEMA.parse(input);
    console.log(`✓ ${label}:`, JSON.stringify(result));
  } catch (error) {
    console.log(`✗ ${label}: ERROR -`, error.message);
  }
}
EOF

cd /tmp && node test_zod_record.mjs
rm test_zod_record.mjs

Repository: arul28/ADE

Length of output: 1052

🏁 Script executed:

#!/bin/bash
# Check if zod is available from the repo and print its resolved path/version
node -e "try{console.log('zod resolve:',require.resolve('zod'))}catch(e){console.error('require.resolve failed:',e.message);process.exit(0)}"
node -e "try{const {z}=require('zod'); console.log('zod loaded (commonjs)');}catch(e){console.error('require(zod) failed:',e.message)}"
ls -la node_modules/zod 2>/dev/null || true

# Print zod version from package.json/lock if present
node -e "const fs=require('fs'); const p=['package.json','package-lock.json','npm-shrinkwrap.json','yarn.lock','pnpm-lock.yaml']; for(const f of p){ if(fs.existsSync(f)){ console.log('found',f); break}}"
rg -n "\"zod\"|zod@" package.json pnpm-lock.yaml yarn.lock package-lock.json 2>/dev/null || true

Repository: arul28/ADE

Length of output: 272

🌐 Web query:

Zod 4 z.record array input accepts or rejects Array.isArray behavior record schema

💡 Result:

In Zod, including in version 4, z.record() is designed specifically to validate objects (the Record<K, V> type in TypeScript) and does not inherently treat arrays as valid input [1][2]. If you pass an array to a z.record() schema, it will be rejected during validation [1]. While JavaScript arrays are technically objects, z.record() explicitly expects an object structure with keys and values that match the provided schemas [1]. If you need to validate a structure that might be an array, you should use z.array() [1][3][4]. If you need to support both types of input for the same field, you can use z.union([z.array(...), z.record(...)]) or z.discriminatedUnion(), depending on your requirements. For context, z.record() is used to validate objects where you define the schema for the keys and the schema for the values (e.g., z.record(z.string(), z.number())) [1]. It is not a generic collection validator for both arrays and objects [1][2].

Citations:

Fix/clarify STRING_MAP_SCHEMA array-handling vs PR summary

  • z.record(z.string(), z.unknown()) is designed for object inputs; passing an array should fail validation (so with .catch(() => undefined) the result should be undefined, not partially accepted numeric keys).
  • Either update the PR summary/tests to reflect arrays are rejected, or change the schema to explicitly accept arrays (e.g., z.union([z.array(...), z.record(...)]) plus conversion), if that’s the intended behavior.
  • Reconsider .catch(() => undefined) here if you want malformed config fields to be diagnosable rather than silently ignored.
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/main/services/config/projectConfigService.ts` around lines
105 - 114, STRING_MAP_SCHEMA currently uses z.record(z.string(), z.unknown())
with a transform and a final .catch(() => undefined), which means arrays will be
rejected by z.record but then silently turned into undefined (and numeric array
indices might be misinterpreted); either make the schema explicitly accept
arrays and convert them, or remove the silent catch so errors surface. Fix by
replacing STRING_MAP_SCHEMA with a union like z.union([z.record(z.string(),
z.unknown()), z.array(z.unknown())]) and update the transform in
STRING_MAP_SCHEMA to handle both shapes (for arrays, iterate values and map
numeric/indexed or string entries into the output Record<string,string>), and
remove or tighten the .catch(() => undefined) so malformed configs produce
validation errors instead of being swallowed.

const COMPUTE_BACKEND_SCHEMA = z.enum(["local", "vps", "daytona"]).optional().catch(undefined);

function isPathWithinProjectRoot(projectRoot: string, candidate: string, opts: { allowMissing?: boolean } = {}): boolean {
try {
Expand All @@ -102,12 +124,11 @@ function isPathWithinProjectRoot(projectRoot: string, candidate: string, opts: {
}

function asString(value: unknown): string | undefined {
return typeof value === "string" ? value : undefined;
return OPTIONAL_STRING_SCHEMA.parse(value);
}

function asStringArray(value: unknown): string[] | undefined {
if (!Array.isArray(value)) return undefined;
return value.filter((v): v is string => typeof v === "string").map((v) => v.trim()).filter(Boolean);
return STRING_ARRAY_SCHEMA.parse(value);
}

function asLaneTypeArray(value: unknown): LaneType[] | undefined {
Expand All @@ -120,15 +141,15 @@ function asLaneTypeArray(value: unknown): LaneType[] | undefined {
}

function asNumber(value: unknown): number | undefined {
return typeof value === "number" && Number.isFinite(value) ? value : undefined;
return OPTIONAL_NUMBER_SCHEMA.parse(value);
}

function asBool(value: unknown): boolean | undefined {
return typeof value === "boolean" ? value : undefined;
return OPTIONAL_BOOL_SCHEMA.parse(value);
}

function asComputeBackend(value: unknown): "local" | "vps" | "daytona" | undefined {
return value === "local" || value === "vps" || value === "daytona" ? value : undefined;
return COMPUTE_BACKEND_SCHEMA.parse(value);
}

function coerceOrchestratorHookConfig(value: unknown): { command: string; timeoutMs?: number } | null {
Expand All @@ -147,12 +168,7 @@ function coerceOrchestratorHookConfig(value: unknown): { command: string; timeou
}

function asStringMap(value: unknown): Record<string, string> | undefined {
if (!isRecord(value)) return undefined;
const out: Record<string, string> = {};
for (const [k, v] of Object.entries(value)) {
if (typeof v === "string") out[k] = v;
}
return out;
return STRING_MAP_SCHEMA.parse(value);
}

function normalizeConfigPath(value: string): string {
Expand Down
49 changes: 16 additions & 33 deletions apps/desktop/src/main/services/history/operationService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@ type OperationStatus = "running" | "succeeded" | "failed" | "canceled";
type OperationMetadata = Record<string, unknown>;
type HeadChangeOperationRecord = OperationRecord & { preHeadSha: string; postHeadSha: string };

const OPERATION_COLUMNS = `
o.id as id,
o.lane_id as laneId,
l.name as laneName,
o.kind as kind,
o.started_at as startedAt,
o.ended_at as endedAt,
o.status as status,
o.pre_head_sha as preHeadSha,
o.post_head_sha as postHeadSha,
o.metadata_json as metadataJson
`;

function safeParseMetadata(raw: string | null | undefined): OperationMetadata {
const parsed = safeJsonParse(raw, null);
return isRecord(parsed) ? parsed : {};
Expand Down Expand Up @@ -96,17 +109,7 @@ export function createOperationService({
if (!operationId.trim()) return null;
const row = db.get<OperationRecord>(
`
select
o.id as id,
o.lane_id as laneId,
l.name as laneName,
o.kind as kind,
o.started_at as startedAt,
o.ended_at as endedAt,
o.status as status,
o.pre_head_sha as preHeadSha,
o.post_head_sha as postHeadSha,
o.metadata_json as metadataJson
select ${OPERATION_COLUMNS}
from operations o
left join lanes l on l.id = o.lane_id
where o.project_id = ? and o.id = ?
Expand Down Expand Up @@ -144,17 +147,7 @@ export function createOperationService({
const limit = typeof args.limit === "number" ? Math.max(1, Math.min(1000, Math.floor(args.limit))) : 100;
return db.all<HeadChangeOperationRecord>(
`
select
o.id as id,
o.lane_id as laneId,
l.name as laneName,
o.kind as kind,
o.started_at as startedAt,
o.ended_at as endedAt,
o.status as status,
o.pre_head_sha as preHeadSha,
o.post_head_sha as postHeadSha,
o.metadata_json as metadataJson
select ${OPERATION_COLUMNS}
from operations o
left join lanes l on l.id = o.lane_id
where o.project_id = ?
Expand Down Expand Up @@ -197,17 +190,7 @@ export function createOperationService({

const rows = db.all<OperationRecord>(
`
select
o.id as id,
o.lane_id as laneId,
l.name as laneName,
o.kind as kind,
o.started_at as startedAt,
o.ended_at as endedAt,
o.status as status,
o.pre_head_sha as preHeadSha,
o.post_head_sha as postHeadSha,
o.metadata_json as metadataJson
select ${OPERATION_COLUMNS}
from operations o
left join lanes l on l.id = o.lane_id
where ${where.join(" and ")}
Expand Down
63 changes: 31 additions & 32 deletions apps/desktop/src/main/services/sessions/sessionService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,9 +532,13 @@ export function createSessionService({ db }: { db: AdeDb }) {
.map((toolType) => normalizeToolType(toolType))
.filter((toolType): toolType is TerminalToolType => toolType != null)
: [];
const exclusionSql = normalizedExcludedToolTypes.length
? ` and (tool_type is null or tool_type not in (${normalizedExcludedToolTypes.map(() => "?").join(", ")}))`
: "";
type SqlClause = { sql: string; params: Array<string | number> };
const exclusionClause: SqlClause = normalizedExcludedToolTypes.length
? {
sql: ` and (tool_type is null or tool_type not in (${normalizedExcludedToolTypes.map(() => "?").join(", ")}))`,
params: normalizedExcludedToolTypes,
}
: { sql: "", params: [] };
const ownerParams = liveOwnerPids
? Array.from(liveOwnerPids)
.map((pid) => normalizeOwnerPid(pid))
Expand Down Expand Up @@ -577,47 +581,43 @@ export function createSessionService({ db }: { db: AdeDb }) {
identity.pid,
identity.startedAt,
]);
const ownerGuardSql = (() => {
if (liveOwnerPids === undefined) return " and owner_pid is null";
const ownerGuardClause = ((): SqlClause => {
if (liveOwnerPids === undefined) return { sql: " and owner_pid is null", params: [] };
const hasKnownOwnerScope = knownOwnerPids !== undefined || knownOwnerIdentities !== undefined;
if (!hasKnownOwnerScope) {
if (ownerIdentities.length) {
return ` and (owner_pid is null or owner_process_started_at is null or not (${ownerIdentities.map(() => "(owner_pid = ? and owner_process_started_at = ?)").join(" or ")}))`;
return {
sql: ` and (owner_pid is null or owner_process_started_at is null or not (${ownerIdentities.map(() => "(owner_pid = ? and owner_process_started_at = ?)").join(" or ")}))`,
params: ownerIdentityParams,
};
}
return ownerParams.length
? ` and (owner_pid is null or owner_pid not in (${ownerParams.map(() => "?").join(", ")}))`
: "";
? {
sql: ` and (owner_pid is null or owner_pid not in (${ownerParams.map(() => "?").join(", ")}))`,
params: ownerParams,
}
: { sql: "", params: [] };
}

const staleKnownClauses = ["owner_pid is null"];
const params: Array<string | number> = [];
if (knownOwnerIdentityRows.length) {
const knownIdentitySql = knownOwnerIdentityRows.map(() => "(owner_pid = ? and owner_process_started_at = ?)").join(" or ");
const liveIdentitySql = ownerIdentities.length
? ownerIdentities.map(() => "(owner_pid = ? and owner_process_started_at = ?)").join(" or ")
: "0";
staleKnownClauses.push(`(owner_process_started_at is not null and (${knownIdentitySql}) and not (${liveIdentitySql}))`);
params.push(...knownOwnerIdentityParams, ...ownerIdentityParams);
}
if (knownOwnerParams.length) {
const knownPidSql = knownOwnerParams.map(() => "?").join(", ");
const livePidSql = ownerParams.length
? ` and owner_pid not in (${ownerParams.map(() => "?").join(", ")})`
: "";
staleKnownClauses.push(`(owner_process_started_at is null and owner_pid in (${knownPidSql})${livePidSql})`);
params.push(...knownOwnerParams, ...ownerParams);
}
return ` and (${staleKnownClauses.join(" or ")})`;
})();
const ownerGuardParams = (() => {
if (liveOwnerPids === undefined) return [];
const hasKnownOwnerScope = knownOwnerPids !== undefined || knownOwnerIdentities !== undefined;
if (!hasKnownOwnerScope) {
return ownerIdentities.length ? ownerIdentityParams : ownerParams;
}
return [
...knownOwnerIdentityParams,
...(knownOwnerIdentityRows.length ? ownerIdentityParams : []),
...knownOwnerParams,
...(knownOwnerParams.length ? ownerParams : []),
];
return { sql: ` and (${staleKnownClauses.join(" or ")})`, params };
})();
const graceMs = typeof freshActivityGraceMs === "number" && Number.isFinite(freshActivityGraceMs)
? Math.max(0, freshActivityGraceMs)
Expand All @@ -627,16 +627,15 @@ export function createSessionService({ db }: { db: AdeDb }) {
const activityCutoff = graceMs > 0 && Number.isFinite(cutoffMs)
? new Date(cutoffMs).toISOString()
: null;
const activityGuardSql = activityCutoff
? " and started_at < ? and (last_output_at is null or last_output_at < ?)"
: "";
const activityParams = activityCutoff ? [activityCutoff, activityCutoff] : [];
const whereSql = `status = 'running'${exclusionSql}${ownerGuardSql}${activityGuardSql}`;
const params = [
...normalizedExcludedToolTypes,
...ownerGuardParams,
...activityParams,
];
const activityClause: SqlClause = activityCutoff
? {
sql: " and started_at < ? and (last_output_at is null or last_output_at < ?)",
params: [activityCutoff, activityCutoff],
}
: { sql: "", params: [] };
const clauses = [exclusionClause, ownerGuardClause, activityClause];
const whereSql = `status = 'running'${clauses.map((clause) => clause.sql).join("")}`;
const params = clauses.flatMap((clause) => clause.params);
const rows = db.all<{ id: string }>(
`select id from terminal_sessions where ${whereSql}`,
params,
Expand Down
23 changes: 22 additions & 1 deletion apps/desktop/src/main/services/state/globalState.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { describe, expect, it } from "vitest";

import { upsertRecentProject, type GlobalState } from "./globalState";
import { readGlobalState, upsertRecentProject, writeGlobalState, type GlobalState } from "./globalState";

describe("upsertRecentProject", () => {
it("keeps an existing project in place when preserving recent order", () => {
Expand Down Expand Up @@ -62,3 +65,21 @@ describe("upsertRecentProject", () => {
expect(next.lastProjectRoot).toBe("/projects/a");
});
});

describe("writeGlobalState", () => {
it("persists state through an atomic temp-file rename", () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "ade-global-state-"));
const filePath = path.join(dir, "global-state.json");
const state: GlobalState = {
lastProjectRoot: "/repo/ade",
recentProjects: [
{ rootPath: "/repo/ade", displayName: "ADE", lastOpenedAt: "2026-05-31T00:00:00.000Z" },
],
};

writeGlobalState(filePath, state);

expect(readGlobalState(filePath)).toEqual(state);
expect(fs.readdirSync(dir).filter((entry) => entry.endsWith(".tmp"))).toEqual([]);
});
Comment on lines +69 to +84
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

This test doesn't actually prove the temp-file rename path.

A direct overwrite of filePath would still pass these assertions, so the atomic-write contract can regress unnoticed. Please assert the renameSync call from a .tmp path in the same directory, or inject a failure before rename and verify the original file stays intact.

🧪 One way to lock down the rename behavior 
-import { describe, expect, it } from "vitest";
+import { describe, expect, it, vi } from "vitest";
@@
   it("persists state through an atomic temp-file rename", () => {
     const dir = fs.mkdtempSync(path.join(os.tmpdir(), "ade-global-state-"));
     const filePath = path.join(dir, "global-state.json");
+    const renameSpy = vi.spyOn(fs, "renameSync");
     const state: GlobalState = {
@@
     writeGlobalState(filePath, state);
 
     expect(readGlobalState(filePath)).toEqual(state);
+    expect(renameSpy).toHaveBeenCalledTimes(1);
+    const [from, to] = renameSpy.mock.calls[0]!;
+    expect(path.dirname(from)).toBe(dir);
+    expect(from.endsWith(".tmp")).toBe(true);
+    expect(to).toBe(filePath);
     expect(fs.readdirSync(dir).filter((entry) => entry.endsWith(".tmp"))).toEqual([]);
+    renameSpy.mockRestore();
   });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
describe("writeGlobalState", () => {
it("persists state through an atomic temp-file rename", () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "ade-global-state-"));
const filePath = path.join(dir, "global-state.json");
const state: GlobalState = {
lastProjectRoot: "/repo/ade",
recentProjects: [
{ rootPath: "/repo/ade", displayName: "ADE", lastOpenedAt: "2026-05-31T00:00:00.000Z" },
],
};
writeGlobalState(filePath, state);
expect(readGlobalState(filePath)).toEqual(state);
expect(fs.readdirSync(dir).filter((entry) => entry.endsWith(".tmp"))).toEqual([]);
});
describe("writeGlobalState", () => {
it("persists state through an atomic temp-file rename", () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "ade-global-state-"));
const filePath = path.join(dir, "global-state.json");
const renameSpy = vi.spyOn(fs, "renameSync");
const state: GlobalState = {
lastProjectRoot: "/repo/ade",
recentProjects: [
{ rootPath: "/repo/ade", displayName: "ADE", lastOpenedAt: "2026-05-31T00:00:00.000Z" },
],
};
writeGlobalState(filePath, state);
expect(readGlobalState(filePath)).toEqual(state);
expect(renameSpy).toHaveBeenCalledTimes(1);
const [from, to] = renameSpy.mock.calls[0]!;
expect(path.dirname(from)).toBe(dir);
expect(from.endsWith(".tmp")).toBe(true);
expect(to).toBe(filePath);
expect(fs.readdirSync(dir).filter((entry) => entry.endsWith(".tmp"))).toEqual([]);
renameSpy.mockRestore();
});
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/main/services/state/globalState.test.ts` around lines 69 -
84, The test currently doesn't verify the atomic-temp rename; update the
"writeGlobalState" test to assert the actual rename behavior by spying/mocking
fs.renameSync (or fs.promises.rename) when calling writeGlobalState: create an
initial file at filePath, call writeGlobalState(filePath, state), and verify
fs.renameSync was invoked with a source path that endsWith(".tmp") and whose
directory equals path.dirname(filePath) and a destination equal to filePath;
alternatively inject a failure before rename and assert the original file
content remains unchanged. Reference writeGlobalState, readGlobalState and
fs.renameSync (or fs.promises.rename) when locating where to add the spy/assert.

});
32 changes: 30 additions & 2 deletions apps/desktop/src/main/services/state/globalState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import fs from "node:fs";
import path from "node:path";
import { randomUUID } from "node:crypto";
import type { OpenProjectBinding, RecentlyInstalledUpdate } from "../../../shared/types";

export type RecentProject = {
Expand Down Expand Up @@ -36,10 +37,37 @@ export function readGlobalState(filePath: string): GlobalState {
}

export function writeGlobalState(filePath: string, state: GlobalState): void {
let tempPath: string | null = null;
let fd: number | null = null;
try {
fs.mkdirSync(path.dirname(filePath), { recursive: true });
fs.writeFileSync(filePath, JSON.stringify(state, null, 2), "utf8");
const dir = path.dirname(filePath);
fs.mkdirSync(dir, { recursive: true });
tempPath = path.join(dir, `.${path.basename(filePath)}.${process.pid}.${randomUUID()}.tmp`);
const serialized = `${JSON.stringify(state, null, 2)}\n`;
fd = fs.openSync(tempPath, "w");
fs.writeFileSync(fd, serialized, "utf8");
fs.fsyncSync(fd);
fs.closeSync(fd);
fd = null;
fs.renameSync(tempPath, filePath);
tempPath = null;
try {
const dirFd = fs.openSync(dir, "r");
try {
fs.fsyncSync(dirFd);
} finally {
fs.closeSync(dirFd);
}
} catch {
// Directory fsync is best effort across filesystems/platforms.
}
} catch {
if (fd != null) {
try { fs.closeSync(fd); } catch {}
}
if (tempPath) {
try { fs.unlinkSync(tempPath); } catch {}
}
// Non-fatal; global state is a convenience.
}
}
Expand Down
11 changes: 11 additions & 0 deletions apps/desktop/src/main/services/state/kvDb.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,17 @@ describe("openKvDb SQL binding", () => {
db.all("select flag from db_value_test where flag = ?", [{} as any]),
).toThrow(/Unsupported database value at parameter 1: object .*sql=select flag from db_value_test/i);
});

it("checkpoints pending writes when flushed", async () => {
const projectRoot = makeProjectRoot("ade-kvdb-flush-");
const dbPath = path.join(projectRoot, ".ade", "ade.db");
const db = await openKvDb(dbPath, createLogger() as any);
activeDisposers.push(async () => db.close());

db.setJson("flush:probe", { ok: true });
expect(() => db.flushNow()).not.toThrow();
expect(db.getJson<{ ok: boolean }>("flush:probe")).toEqual({ ok: true });
});
Comment on lines +220 to +229
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

This assertion doesn't prove a checkpoint happened.

The same connection can read uncheckpointed WAL data, so this still passes if flushNow() becomes a no-op. Please assert an observable checkpoint effect instead, such as a truncated/absent -wal file after flushNow() or checkpoint stats from SQLite.

🧪 Example of checking the WAL side effect directly 
   db.setJson("flush:probe", { ok: true });
   expect(() => db.flushNow()).not.toThrow();
   expect(db.getJson<{ ok: boolean }>("flush:probe")).toEqual({ ok: true });
+  const walPath = `${dbPath}-wal`;
+  if (fs.existsSync(walPath)) {
+    expect(fs.statSync(walPath).size).toBe(0);
+  }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/main/services/state/kvDb.test.ts` around lines 220 - 229,
The test currently only verifies reads from the same open connection so it
doesn't prove a checkpoint occurred; update the "checkpoints pending writes when
flushed" test to assert a WAL-side observable effect after calling
db.flushNow(): e.g., call db.flushNow(), then either close the DB (db.close()),
and assert the WAL file for dbPath (dbPath + "-wal") is absent or truncated on
disk, or run a SQLite checkpoint query (PRAGMA wal_checkpoint or PRAGMA
wal_checkpoint(TRUNCATE)) via the DB handle and assert the returned checkpoint
stats indicate a completed checkpoint; use openKvDb, db.flushNow, db.close and
dbPath to locate the WAL and ensure the test fails if no checkpoint happened.

});

describe("lane_linear_issue_links schema", () => {
Expand Down
Loading
Loading