GitNexus/gitnexus/test/unit/lbug-checkpoint.test.ts
Copilot 87b91c821e
fix(lbug): add WAL checkpoint-threshold control (#1772)
* Initial plan

* fix(analyze): add WAL auto-checkpoint CLI control and default-off behavior

* test(analyze): share lbug auto-checkpoint parsing and align validation

* fix(analyze): always enable lbug auto-checkpoint and expose threshold control

* refactor(lbug): inline always-on auto-checkpoint constructor arg

* fix(analyze): guide checkpoint-threshold on Ladybug WAL checkpoint IO failures

* test(analyze): cover checkpoint IO guidance and add integration guard

* fix(analyze): tighten checkpoint IO detection and remove test hook

* fix(analyze): remove checkpoint test hook and tighten error matching

* fix(analyze): rename to wal-checkpoint-threshold, raise default, add manual checkpoint driver with retry

Address review feedback on PR #1772:

- Rename CLI flag, env var, AnalyzeOptions field, recovery-hint tag, and
  parser/constants from lbug-* to engine-neutral wal-* (matches the existing
  WAL_RECOVERY_SUGGESTION / isWalCorruptionError convention).
- Raise default threshold from -1 (Ladybug stock ~16 MiB) to 64 MiB so users
  on the default config no longer hit the original rename/remove race.
- Align both READMEs to publish 67108864 (64 MiB) instead of 65536 (which
  would have made the crash more frequent).
- Add wal-checkpoint-driver.ts: a periodic manual CHECKPOINT driver wrapped
  in a 3-attempt jittered retry (50/200/500 ms), driven from runFullAnalysis.
  Opt-out via GITNEXUS_WAL_MANUAL_CHECKPOINT=0. Moves the race window into a
  JS-controllable retry surface while keeping native auto-checkpoint on.
- Move LBUG_CHECKPOINT_RENAME_RE / REMOVE_RE plus the predicate (renamed to
  isLbugCheckpointIoError) into lbug-config.ts alongside isWalCorruptionError.
  Predicate is now exported. Add a permissive fallback matcher and pin the
  matched Ladybug version in comments.
- Warn instead of silently defaulting when GITNEXUS_WAL_CHECKPOINT_THRESHOLD
  is set to a non-empty unparseable value (closes the CLI-vs-env asymmetry).
- Add a typed RecoveryHint string-literal union in cli-message.ts so future
  hint tags can't drift.
- Add a real integration test under test/integration/ that triggers a
  Ladybug checkpoint IO failure via a pre-existing directory at the rename
  target (portable across platforms; no test-only injection hook).
- Add small-disk / CI caveat (32 MiB secondary suggestion) to the recovery
  hint and README env-var rows.
- Document CLI/env precedence in the analyze --help block.
- Help placeholder: <value> -> <bytes>.
- Rename analyze-lbug-auto-checkpoint.test.ts to use the new wal-* token.

* chore(lbug): remove dead jitteredDelay helper and apply prettier

- Drop unused `jitteredDelay` function flagged by CodeQL in PR #1772; the
  retry loop already inlines the same calculation with the injectable
  `randomImpl` so the helper was dead. Move the non-cryptographic-by-design
  comment next to the actual jitter site.
- Apply `prettier --write` to wal-checkpoint-driver.ts and the new
  integration test to absorb the PR autofix bot's formatting findings.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Test <test@example.com>
2026-05-22 14:46:49 +01:00

106 lines
4.5 KiB
TypeScript

/**
* Structural + behavioural tests for the WAL-flush / close helpers (#1376).
*
* After the review-driven refactor, the module exposes two layers:
* - flushWAL — CHECKPOINT only (connection stays open)
* - safeClose — flushWAL + conn.close + db.close
*
* closeLbug delegates to safeClose for the CHECKPOINT + close step and
* then resets module-level state (currentDbPath, ftsLoaded, etc.).
*
* The structural tests read the adapter source and verify delegation
* contracts so a future refactor that inlines close logic is caught.
*
* The behavioural tests import flushWAL directly and exercise the
* runtime null-guard path (conn is null at module load) so a future
* refactor that accidentally throws is caught immediately.
*/
import { beforeAll, describe, expect, it } from 'vitest';
import fs from 'node:fs/promises';
import path from 'node:path';
import { flushWAL } from '../../src/core/lbug/lbug-adapter.js';
describe('flushWAL / safeClose — consolidation guard (#1376)', () => {
let adapterSource: string;
beforeAll(async () => {
adapterSource = await fs.readFile(
path.join(__dirname, '..', '..', 'src', 'core', 'lbug', 'lbug-adapter.ts'),
'utf-8',
);
});
it('exports flushWAL (CHECKPOINT-only helper)', () => {
expect(adapterSource).toMatch(/export const flushWAL/);
});
it('exports safeClose (CHECKPOINT + close helper)', () => {
expect(adapterSource).toMatch(/export const safeClose/);
});
it('safeClose delegates to flushWAL for the CHECKPOINT step', () => {
const safeCloseBody = adapterSource.slice(adapterSource.indexOf('export const safeClose'));
expect(safeCloseBody).toMatch(/await flushWAL\(\)/);
});
it('closeLbug delegates to safeClose instead of inlining conn.close/db.close', () => {
const closeLbugBody = adapterSource.slice(adapterSource.indexOf('export const closeLbug'));
expect(closeLbugBody).toMatch(/await safeClose\(\)/);
// closeLbug must NOT contain its own conn.close() or db.close() — those
// live exclusively inside safeClose now.
const closeLbugBlock = closeLbugBody.slice(0, closeLbugBody.indexOf('export const', 1) >>> 0);
expect(closeLbugBlock).not.toMatch(/conn\.close\(\)/);
expect(closeLbugBlock).not.toMatch(/db\.close\(\)/);
});
it('CHECKPOINT is issued only by flushWAL (best-effort) and tryFlushWAL (rethrows for the retry driver)', () => {
const matches = adapterSource.match(/conn\.query\('CHECKPOINT'\)/g) ?? [];
// Two authorized sites: `flushWAL` (swallows errors — used by
// `safeClose` and the server's best-effort flush) and `tryFlushWAL`
// (rethrows so the manual checkpoint driver in `wal-checkpoint-driver.ts`
// can apply its bounded retry). Any third occurrence is a regression —
// a CHECKPOINT outside these two helpers will be invisible to the
// retry/error policy.
expect(matches.length).toBe(2);
});
it('exports tryFlushWAL (CHECKPOINT-with-rethrow for the manual retry driver)', () => {
expect(adapterSource).toMatch(/export const tryFlushWAL/);
});
it('flushWAL drains and closes the CHECKPOINT result before returning', () => {
const flushBody = adapterSource.slice(
adapterSource.indexOf('export const flushWAL'),
adapterSource.indexOf('export const safeClose'),
);
expect(flushBody).toMatch(/await drainQueryResult\(checkpointResult\)/);
});
it('conn.close() only appears inside safeClose (with eslint-disable)', () => {
// Every conn.close() in the adapter must live inside safeClose, guarded
// by the eslint-disable comment. Count occurrences to catch leaks.
const matches = adapterSource.match(/await conn\.close\(\)/g) ?? [];
expect(matches.length).toBe(1);
});
it('db.close() only appears inside safeClose (with eslint-disable)', () => {
const matches = adapterSource.match(/await db\.close\(\)/g) ?? [];
expect(matches.length).toBe(1);
});
});
// Behavioural tests — exercise flushWAL at runtime rather than just
// grepping source text. At module load `conn` is null, so these hit
// the early-return guard without needing a real LadybugDB instance.
describe('flushWAL — runtime behaviour', () => {
it('resolves without error when no connection is open', async () => {
// conn is null at module load — flushWAL must not throw.
await expect(flushWAL()).resolves.toBeUndefined();
});
it('can be called repeatedly without throwing (idempotent)', async () => {
await flushWAL();
await flushWAL();
// No assertion needed beyond "did not throw".
});
});