test: fix CI-only failures from mock.module leakage and sqlite3 .recover

These surfaced once the lockfile fix let CI actually run the suite (it had
been dying at setup-node before install). All are test-environment issues;
no production code changed.

- logger.dataIn leak (9 summarize-tag-stripping fails): timeline-formatting,
  claude-md-utils, and runtime-selector mocked logger via the process-global
  mock.module but never restored it (mock.restore() does NOT undo mock.module).
  A partial logger mock without dataIn leaked into later files under CI's file
  order. Snapshot the real module and re-register it in afterAll, matching the
  pattern already used by the chroma-mcp-manager tests.

- summarize-subagent-skip (4 fails, exposed by the runtime-selector restore):
  it mocked SettingsDefaultsManager but not hook-settings, and loadFromFileOnce()
  module-caches — so it depended on a leaked hook-settings mock. Mock
  hook-settings directly and restore in afterAll so it is self-contained.

- schema-repair (3 fails): repairMalformedDatabase shells out to
  `sqlite3 .recover`, which needs the dbpage vtab. The ubuntu CI runner's
  sqlite3 lacks it ("no such table: sqlite_dbpage"). Probe the capability and
  skip the repair tests when absent, mirroring the existing hasPython() guard.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Alex Newman
2026-05-29 11:14:04 -07:00
parent 57612286f0
commit 54a3e6c46e
5 changed files with 93 additions and 9 deletions
@@ -6,8 +6,10 @@ import { join } from 'path';
// re-register the snapshots in afterAll so these mocks do not leak into later
// test files (bun's mock.module is process-global; mock.restore() does NOT undo it).
import * as realSettingsDefaultsManager from '../../../src/shared/SettingsDefaultsManager.js';
import * as realHookSettings from '../../../src/shared/hook-settings.js';
import * as realWorkerUtils from '../../../src/shared/worker-utils.js';
const realSettingsSnapshot = { ...realSettingsDefaultsManager };
const realHookSettingsSnapshot = { ...realHookSettings };
const realWorkerUtilsSnapshot = { ...realWorkerUtils };
mock.module('../../../src/shared/SettingsDefaultsManager.js', () => ({
@@ -21,6 +23,14 @@ mock.module('../../../src/shared/SettingsDefaultsManager.js', () => ({
},
}));
// loadFromFileOnce() module-caches its result, so mocking SettingsDefaultsManager
// alone is not enough — an earlier test may have already cached real settings.
// Mock hook-settings directly so shouldTrackProject() always sees a string
// CLAUDE_MEM_EXCLUDED_PROJECTS regardless of global mock/cache state.
mock.module('../../../src/shared/hook-settings.js', () => ({
loadFromFileOnce: () => ({ CLAUDE_MEM_EXCLUDED_PROJECTS: '' }),
}));
const workerCallLog: Array<{ path: string; options: any }> = [];
mock.module('../../../src/shared/worker-utils.js', () => ({
ensureWorkerRunning: () => Promise.resolve(true),
@@ -55,6 +65,7 @@ afterEach(() => {
afterAll(() => {
mock.module('../../../src/shared/SettingsDefaultsManager.js', () => realSettingsSnapshot);
mock.module('../../../src/shared/hook-settings.js', () => realHookSettingsSnapshot);
mock.module('../../../src/shared/worker-utils.js', () => realWorkerUtilsSnapshot);
});
+14 -1
View File
@@ -1,6 +1,14 @@
// SPDX-License-Identifier: Apache-2.0
import { describe, it, expect, mock, beforeEach } from 'bun:test';
import { describe, it, expect, mock, beforeEach, afterAll } from 'bun:test';
// Snapshot real modules BEFORE mock.module mutates the live namespace, then
// re-register in afterAll. bun's mock.module is process-global and survives
// mock.restore(), so these would otherwise leak into later test files.
import * as realHookSettings from '../../src/shared/hook-settings.js';
import * as realLogger from '../../src/utils/logger.js';
const realHookSettingsSnapshot = { ...realHookSettings };
const realLoggerSnapshot = { ...realLogger };
let mockSettings: Record<string, string> = {};
@@ -23,6 +31,11 @@ mock.module('../../src/utils/logger.js', () => ({
},
}));
afterAll(() => {
mock.module('../../src/shared/hook-settings.js', () => realHookSettingsSnapshot);
mock.module('../../src/utils/logger.js', () => realLoggerSnapshot);
});
import {
resolveRuntimeContext,
selectRuntime,
+39 -6
View File
@@ -27,6 +27,42 @@ function hasPython(): boolean {
}
}
// repairMalformedDatabase() shells out to `sqlite3 <db> .recover`, which depends
// on the dbpage virtual table. Some sqlite3 CLI builds (e.g. on the ubuntu CI
// runner) are compiled without it and fail with "no such table: sqlite_dbpage".
// The repair feature legitimately requires that capability, so — like the
// hasPython() guard above — we skip the repair tests when the host sqlite3
// cannot perform .recover rather than reporting a false failure.
function canRecoverViaSqlite3(): boolean {
const probe = tempDbPath();
try {
const db = new Database(probe, { create: true, readwrite: true });
db.run('CREATE TABLE probe(x)');
db.run('INSERT INTO probe(x) VALUES (1)');
db.close();
execFileSync('sqlite3', [probe, '.recover'], { stdio: 'pipe', encoding: 'utf-8' });
return true;
} catch {
return false;
} finally {
cleanup(probe);
}
}
const REPAIR_SUPPORTED = canRecoverViaSqlite3();
function skipUnlessRepairable(): boolean {
if (!hasPython()) {
console.log('Python3 not available, skipping repair test');
return true;
}
if (!REPAIR_SUPPORTED) {
console.log("sqlite3 CLI lacks .recover (no sqlite_dbpage), skipping repair test");
return true;
}
return false;
}
function corruptDbViaPython(dbPath: string): void {
const script = join(tmpdir(), `corrupt-${Date.now()}.py`);
writeFileSync(script, `
@@ -50,8 +86,7 @@ c.close()
describe('Schema repair on malformed database', () => {
it('should repair a database with an orphaned index referencing a non-existent column', () => {
if (!hasPython()) {
console.log('Python3 not available, skipping test');
if (skipUnlessRepairable()) {
return;
}
@@ -120,8 +155,7 @@ describe('Schema repair on malformed database', () => {
});
it('should repair a corrupted DB that has no schema_versions table', () => {
if (!hasPython()) {
console.log('Python3 not available, skipping test');
if (skipUnlessRepairable()) {
return;
}
@@ -172,8 +206,7 @@ c.close()
});
it('should preserve existing data through repair and re-migration', () => {
if (!hasPython()) {
console.log('Python3 not available, skipping test');
if (skipUnlessRepairable()) {
return;
}
+13 -1
View File
@@ -1,4 +1,12 @@
import { describe, it, expect, mock, afterEach } from 'bun:test';
import { describe, it, expect, mock, afterEach, afterAll } from 'bun:test';
// Snapshot the real logger BEFORE mock.module mutates the live namespace, then
// re-register it in afterAll. bun's mock.module is process-global and
// mock.restore() does NOT undo it, so a partial logger mock here would
// otherwise leak into later test files (e.g. summarize-tag-stripping, which
// needs logger.dataIn).
import * as realLogger from '../../src/utils/logger.js';
const realLoggerSnapshot = { ...realLogger };
mock.module('../../src/utils/logger.js', () => ({
logger: {
@@ -16,6 +24,10 @@ afterEach(() => {
mock.restore();
});
afterAll(() => {
mock.module('../../src/utils/logger.js', () => realLoggerSnapshot);
});
describe('extractFirstFile', () => {
const cwd = '/Users/test/project';
+16 -1
View File
@@ -1,8 +1,18 @@
import { describe, it, expect, mock, afterEach, beforeEach } from 'bun:test';
import { describe, it, expect, mock, afterEach, afterAll, beforeEach } from 'bun:test';
import { mkdirSync, writeFileSync, readFileSync, existsSync, rmSync } from 'fs';
import path, { join } from 'path';
import { tmpdir } from 'os';
// Snapshot the real modules BEFORE mock.module mutates the live namespace, then
// re-register them in afterAll. bun's mock.module is process-global and
// mock.restore() does NOT undo it, so a partial logger mock here would
// otherwise leak into later test files (e.g. summarize-tag-stripping, which
// needs logger.dataIn).
import * as realLogger from '../../src/utils/logger.js';
import * as realWorkerUtils from '../../src/shared/worker-utils.js';
const realLoggerSnapshot = { ...realLogger };
const realWorkerUtilsSnapshot = { ...realWorkerUtils };
mock.module('../../src/utils/logger.js', () => ({
logger: {
info: () => {},
@@ -30,6 +40,11 @@ mock.module('../../src/shared/worker-utils.js', () => ({
buildWorkerUrl: (apiPath: string) => `http://127.0.0.1:37777${apiPath}`,
}));
afterAll(() => {
mock.module('../../src/utils/logger.js', () => realLoggerSnapshot);
mock.module('../../src/shared/worker-utils.js', () => realWorkerUtilsSnapshot);
});
import {
replaceTaggedContent,
formatTimelineForClaudeMd,