fix(tests): drain call-log saves before chat-pipeline DB resets (#12780) (#12966)

Pending call-log saves can still be in flight when resetStorage closes
the DB instance. The orphaned save then writes into the next test's DB
after getDbInstance resolves.

- Drain pending saves in resetStorage before resetting DB instances.
- Bind DB instance at the start of saveCallLogOperation.
- Wait specifically for the Codex responses row in chat-pipeline tests.
This commit is contained in:
Vishesh
2026-09-19 08:32:56 +05:30
committed by GitHub
parent 88b928b8f1
commit b272aa2f23
4 changed files with 41 additions and 3 deletions

View File

@@ -452,6 +452,12 @@ function getLegacyInlineDetail(id: string) {
async function saveCallLogOperation(entry: any): Promise<void> {
try {
// Bind the DB instance up front, before any await (resolveAccountName,
// writeCallArtifactAsync). If the singleton is reset/closed while this
// operation awaits, the insert must target the instance this request
// started against — a closed handle fails into the catch below instead of
// silently writing into whatever database opened afterwards (#12780).
const db = getDbInstance();
const apiKeyContext = getCallLogApiKeyContext();
// `||` (not `??`): an empty-string apiKeyId/apiKeyName is "unattributed",
// same as before this fallback existed — it must not be persisted verbatim
@@ -591,7 +597,6 @@ async function saveCallLogOperation(entry: any): Promise<void> {
}
}
const db = getDbInstance();
db.prepare(
`
INSERT INTO call_logs (

View File

@@ -285,6 +285,16 @@ export async function createChatPipelineHarness(prefix) {
invalidateMemorySettingsCache();
clearSkillState();
await new Promise((resolve) => setTimeout(resolve, 20));
// Call-log persistence is fire-and-forget and the first cold artifact-worker
// spawn can take ~2.4s, so the previous test's saves may still be in flight.
// Drain before the DB reset so they land in the DB being torn down, not in the
// next test's fresh database (#12780).
const drained = await callLogsDb.waitForCallLogSaves(10_000);
if (!drained) {
console.warn(
`[chat-pipeline-harness:${prefix}] call-log saves did not drain within 10s; resetting anyway`
);
}
core.resetDbInstance();
fs.rmSync(testDataDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
fs.mkdirSync(testDataDir, { recursive: true });
@@ -299,6 +309,7 @@ export async function createChatPipelineHarness(prefix) {
semanticCacheModule.clearCache();
clearSkillState();
resetAllCircuitBreakers();
await callLogsDb.waitForCallLogSaves(10_000);
core.resetDbInstance();
fs.rmSync(testDataDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
}

View File

@@ -16,6 +16,7 @@ const settingsDb = await import("../../src/lib/db/settings.ts");
const apiKeysDb = await import("../../src/lib/db/apiKeys.ts");
const readCacheDb = await import("../../src/lib/db/readCache.ts");
const { getLatestCallLog, getResponsesCallLogs } = await import("./_chatPipelineCallLogs.ts");
const { waitForCallLogSaves } = await import("../../src/lib/usage/callLogs.ts");
const { invalidateMemorySettingsCache } = await import("../../src/lib/memory/settings.ts");
const { skillRegistry } = await import("../../src/lib/skills/registry.ts");
const { skillExecutor } = await import("../../src/lib/skills/executor.ts");
@@ -373,6 +374,15 @@ async function resetStorage() {
readCacheDb.invalidateDbCache();
invalidateMemorySettingsCache();
await new Promise((resolve) => setTimeout(resolve, 20));
// Call-log persistence is fire-and-forget (persistAttemptLogs → saveCallLog with
// a .catch(() => {})), and the first cold artifact-worker spawn can take ~2.4s, so
// the previous test's saves may still be in flight here. Draining before the DB
// reset keeps those rows in the DB being torn down instead of letting them land
// in the next test's fresh database (#12780).
const drained = await waitForCallLogSaves(10_000);
if (!drained) {
console.warn("[chat-pipeline] call-log saves did not drain within 10s; resetting anyway");
}
core.resetDbInstance();
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
@@ -663,7 +673,13 @@ test("chat pipeline persists Codex responses cache and reasoning tokens to call
);
const json = (await response.json()) as any;
const callLog = await waitFor(() => getLatestCallLog());
// Wait specifically for THIS request's Codex /v1/responses row instead of taking
// whatever the latest row happens to be: an unfiltered read can surface a row from
// a previous test that landed late in this database (#12780).
const callLog = await waitFor(async () => {
const rows = await getResponsesCallLogs();
return rows.find((row) => row.provider === "codex") ?? null;
});
assert.equal(response.status, 200);
assert.equal(fetchCalls.length, 1);

View File

@@ -21,7 +21,8 @@ const { clearInflight } = await import("../../open-sse/services/requestDedup.ts"
const { resetAll: resetAccountSemaphores } =
await import("../../open-sse/services/accountSemaphore.ts");
const { clearModelLock } = await import("../../open-sse/services/accountFallback.ts");
const { getCallLogs, getCallLogById } = await import("../../src/lib/usage/callLogs.ts");
const { getCallLogs, getCallLogById, waitForCallLogSaves } =
await import("../../src/lib/usage/callLogs.ts");
const { handleChatCore } = await import("../../open-sse/handlers/chatCore.ts");
const { resetPayloadRulesConfigForTests } = await import("../../open-sse/services/payloadRules.ts");
const { CLAUDE_CODE_COMPATIBLE_REDACT_THINKING_BETA, CONTEXT_1M_BETA_HEADER } =
@@ -56,6 +57,11 @@ async function resetStorage() {
clearIdempotency();
clearInflight();
clearModelLock();
// Call-log persistence is fire-and-forget and the first cold artifact-worker
// spawn can take ~2.4s, so this test's saves may still be in flight when the
// next test resets the DB. Drain so a late row cannot land in the next test's
// fresh database and get picked up by its waitFor(getLatestCallLog()) (#12780).
await waitForCallLogSaves(10_000);
core.resetDbInstance();
// A full reset must also drop the settings read-cache. Otherwise the cached
// value (e.g. call_log_pipeline_enabled=true seeded earlier) survives the DB