Skip to content

[Bug]: Compaction lineage validation runs after replacing the active history #490

Open

Description

@knobusawa

This is a resubmission of #489, which was closed by the reporting account before switching accounts for follow-up. This is the same finding, not an independently observed second incident.

Product or interface

CLI - interactive TUI; source-level reproduction in the shared canonical-history provider.

Version and platform

The original repeated error occurred with the installed CLI reporting 0.6.11. The isolated source reproduction below targets public main commit e798b46, whose package metadata reports 0.6.5. I am not assuming these are the same build.

Windows 11 Enterprise x64, build 22631; Node.js 24.19.0. The isolated candidate was installed with pnpm 9.12.0 and the frozen lockfile. No provider credentials, model requests, permission changes, or real session data are needed for the reproduction.

Summary and evidence boundary

A real session repeatedly showed Conversation history could not be safely updated. Please retry.; the underlying redacted error was HistoryScannerError: Canonical history scan failed: artifact-revision-mismatch.

The cause of that original revision mismatch is not established. This issue reports a narrower, independently reproducible failure-atomicity problem: compaction can replace a usable active history before discovering that the new generation references an inconsistent parent snapshot. Retrying cannot undo the already-published generation.

In the current compact path, the provider scans the existing lineage, publishes its parent snapshot, replaces the active file, and only then validates the resulting lineage through index synchronization. See the publication and subsequent index synchronization.

Steps to reproduce

Check out the source commit above and install its frozen dependencies. Apply only the test diff below to the existing declared test file; do not apply the runtime diff yet. Run:

pnpm exec vitest run --config vitest.oss.config.mjs test/history-processing.test.ts -t 'staged compaction'

The regression uses the existing synthetic fixture and an injected file adapter. It starts from a readable generation 0 or 1, changes the newly published parent snapshot immediately before active replacement, and asks compact to proceed. The edit substitutes equal-length synthetic text, so it does not depend on timestamp or size changes. This is deterministic fault injection, not evidence that a concurrent writer caused the original incident.

Expected and actual behavior

Expected: compact rejects the inconsistent staged generation while leaving the original active bytes, messages, and identities authoritative. A fresh provider can still read the previous generation, and a later compaction with a fresh ID can succeed.

Actual on the baseline: both generation cases throw artifact-revision-mismatch, but the assertion that active bytes remain unchanged fails. The replacement has already been published. The separate missing-ancestor compatibility test passes.

Proposed scope

Pass an optional pre-publication validator through the existing canonical-history file adapter to the JSONL atomic-write staging callback. Validate the staged compaction lineage before rename, using the existing helper so the current missing-ancestor compatibility remains unchanged. Keep the post-publication checks as well.

This does not change the history schema, accept a mismatched revision, reset generations, add automatic recovery, or change retry policy. It leaves an unreferenced snapshot after the injected failure rather than trying to roll back or reseal snapshots. It is not cross-process locking: a writer can still change a snapshot after validation. The additional scan also needs the required full performance coverage before any merge.

Validation and remaining boundaries

With the test diff only, the selected tests produce 2 failed / 1 passed. With the runtime diff, all 3 selected tests pass, including unchanged active bytes, staging cleanup, fresh-provider reads, subsequent successful compaction, and missing-ancestor compatibility. Five related fork/compaction test files also pass (181 tests). TypeScript checking, the standalone build, source inventory, generated-path checks, build-boundary checks, and diff whitespace checks pass for the isolated candidate.

The latest combined run of those five files plus the complete history-processing file has 280 passed / 1 failed: an existing warm-scanner symlink test fails during Windows fixture cleanup with ENOTEMPTY. An earlier run before the additional compatibility test passed all 280 then-present cases. I am not treating the latest full history run as green.

The Windows verification profile was attempted twice, but stops in the unrelated release-tools fixture cleanup (suite runner pins its locale, preserves gate arguments and canonicalizes Windows temporary paths, ENOTEMPTY). Running that release-tools test on clean e798b46 reproduces the same cleanup failure. Later profile gates are therefore not claimed as passed. Linux/macOS, the full verification profile, live-model acceptance, and the full performance suite have not been run.

Apart from the earlier submission noted above, searches for the exact user-facing error, artifact-revision-mismatch, history, revision, and compaction did not identify another exact match. No raw logs, sessions, credentials, or private project data are included.

Maintainer guidance

I am starting with an issue in accordance with the collaborator-only PR policy. Would this narrow pre-publication validation boundary be appropriate for a maintainer-reviewed fix? The proposed test and runtime diffs are included below for review; I will not open an unsolicited PR.

Synthetic regression tests (apply to baseline first)
diff --git a/test/history-processing.test.ts b/test/history-processing.test.ts
index 3e88485..603cf20 100644
--- a/test/history-processing.test.ts
+++ b/test/history-processing.test.ts
@@ -10,7 +10,7 @@ import {
   type CanonicalHistoryEnvelope,
 } from '../packages/local-runtime-v2/src/infra/file/canonical-history-jsonl.js';
 import { canonicalJson } from '../packages/local-runtime-v2/src/infra/file/canonical-history-json-value.js';
-import { mkdtemp, rm, writeFile, mkdir, readFile, symlink } from 'node:fs/promises';
+import { mkdtemp, rm, writeFile, mkdir, readFile, readdir, symlink } from 'node:fs/promises';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { describe, expect, it, vi } from 'vitest';
@@ -1437,6 +1437,81 @@ describe('internal history snapshot handoff', () => {
     });
   });
 
+  it.each([0, 1])('keeps generation %i usable when staged compaction has an inconsistent parent', async (generation) => {
+    await fixture(async ({ provider, session, dataDir }) => {
+      const compact = (id: string) => ({
+        sessionId: session.sessionId, turnId: id, reason: 'replaceMessages' as const,
+        operation: { id, kind: 'compaction' }, compactionId: id,
+        method: 'llm_checkpoint' as const, summary: 'original',
+        messages: [{ role: 'compactionSummary', summary: 'original' }],
+      });
+      await provider.append({
+        sessionId: session.sessionId, turnId: 'old', reason: 'messageDelta',
+        operation: { id: 'old', kind: 'append' },
+        messages: [{ role: 'user', timestamp: 1, content: 'original' }],
+      });
+      if (generation === 1) await provider.compact(compact('first'));
+      const paths = resolveSessionHistoryPaths(dataDir, session);
+      const original = await readFile(paths.messages);
+      const before = await provider.readActive(session.sessionId);
+      await mkdir(paths.snapshots, { recursive: true });
+      const entries = await readdir(paths.sessionDir);
+      const base = createCanonicalHistoryFileAdapter();
+      // Deterministic fault injection, not a claim about the original cause:
+      // alter the newly published parent between publication and active staging.
+      const injected = new Proxy(base, {
+        get(target, key) {
+          if (key === 'replaceActive') return async (...args: Parameters<typeof base.replaceActive>) => {
+            const parent = join(paths.snapshots, `g${String(generation).padStart(12, '0')}--raced.jsonl`);
+            const text = await readFile(parent, 'utf8');
+            expect(text).toContain('original');
+            await writeFile(parent, text.replace('original', 'modified'));
+            return base.replaceActive(...args);
+          };
+          const value = Reflect.get(target, key);
+          return typeof value === 'function' ? value.bind(target) : value;
+        },
+      });
+      const faulty = createSessionSystemCanonicalHistoryProvider({
+        dataDir, sessions: { get: async () => session }, files: injected,
+      });
+      await expect(faulty.compact(compact('raced'))).rejects.toThrow('artifact-revision-mismatch');
+      expect(await readFile(paths.messages)).toEqual(original);
+      expect(await readdir(paths.sessionDir)).toEqual(entries);
+      const restarted = createSessionSystemCanonicalHistoryProvider({
+        dataDir, sessions: { get: async () => session },
+      });
+      expect(await restarted.readActive(session.sessionId)).toEqual(before);
+      expect((await restarted.compact(compact('after-failure'))).generation).toBe(generation + 1);
+      expect((await scanCanonicalHistoryArtifacts({
+        activePath: paths.messages, snapshotsPath: paths.snapshots, sessionId: session.sessionId,
+      })).catalog.activeGeneration).toBe(generation + 1);
+    });
+  });
+
+  it('preserves missing-ancestor compatibility when validating staged compaction', async () => {
+    await fixture(async ({ provider, session, dataDir }) => {
+      await provider.append({
+        sessionId: session.sessionId, turnId: 'old', reason: 'messageDelta',
+        operation: { id: 'old', kind: 'append' },
+        messages: [{ role: 'user', timestamp: 1, content: 'original' }],
+      });
+      const compact = (id: string) => provider.compact({
+        sessionId: session.sessionId, turnId: id, reason: 'replaceMessages',
+        operation: { id, kind: 'compaction' }, compactionId: id,
+        method: 'llm_checkpoint', summary: id,
+        messages: [{ role: 'compactionSummary', summary: id }],
+      });
+      await compact('first');
+      const paths = resolveSessionHistoryPaths(dataDir, session);
+      await rm(join(paths.snapshots, 'g000000000000--first.jsonl'));
+      expect((await compact('second')).generation).toBe(2);
+      expect((await provider.readActive(session.sessionId)).messages).toEqual([
+        { role: 'compactionSummary', summary: 'second' },
+      ]);
+    });
+  });
+
   it('keeps injected adapters outside the immutable handoff', async () => {
     await fixture(async ({ dataDir, session }) => {
       const provider = createSessionSystemCanonicalHistoryProvider({
Proposed minimal runtime diff (apply after observing the regression)
diff --git a/packages/local-runtime-v2/src/infra/file/canonical-history-jsonl.ts b/packages/local-runtime-v2/src/infra/file/canonical-history-jsonl.ts
index 9481e59..d0aef58 100644
--- a/packages/local-runtime-v2/src/infra/file/canonical-history-jsonl.ts
+++ b/packages/local-runtime-v2/src/infra/file/canonical-history-jsonl.ts
@@ -502,11 +502,15 @@ export class CanonicalHistoryJsonlDataSource {
     });
   }
 
-  async replaceActive(records: readonly CanonicalHistoryEnvelope[]): Promise<void> {
+  async replaceActive(
+    records: readonly CanonicalHistoryEnvelope[],
+    validateTemporary?: (path: string) => Promise<void>,
+  ): Promise<void> {
     const replacement = normalizeActiveRecords(records);
     const revision = revisionOfNormalized(replacement);
     await writeJsonlAtomically(this.options.activePath, replacement, async (temporaryPath) => {
       await assertActiveFileRevision(temporaryPath, revision, 'active replacement history');
+      await validateTemporary?.(temporaryPath);
     });
   }
 
diff --git a/packages/local-runtime-v2/src/service/session-system/messages/history/canonical-history-provider.ts b/packages/local-runtime-v2/src/service/session-system/messages/history/canonical-history-provider.ts
index ee1543e..286551f 100644
--- a/packages/local-runtime-v2/src/service/session-system/messages/history/canonical-history-provider.ts
+++ b/packages/local-runtime-v2/src/service/session-system/messages/history/canonical-history-provider.ts
@@ -312,7 +312,12 @@ export function createSessionSystemCanonicalHistoryProvider(
           };
           const markedReplacement = markHistoryArtifact(replacement, marker);
           await files.publishSnapshot(snapshotPath, active);
-          await files.replaceActive(paths.messages, markedReplacement);
+          await files.replaceActive(paths.messages, markedReplacement, async (temporaryPath) => {
+            // Reject inconsistent lineage before replacing the usable active file.
+            await scanCompactionLineageAllowingMissingParent(
+              { ...paths, messages: temporaryPath }, change.sessionId,
+            );
+          });
           const committed = await files.readActiveStrict(paths.messages);
           const committedGeneration = activeGeneration(committed);
           if (committedGeneration !== generation + 1) {
diff --git a/packages/local-runtime-v2/src/service/session-system/sessions/representation/canonical-history-contract.ts b/packages/local-runtime-v2/src/service/session-system/sessions/representation/canonical-history-contract.ts
index 1967596..0ea86a3 100644
--- a/packages/local-runtime-v2/src/service/session-system/sessions/representation/canonical-history-contract.ts
+++ b/packages/local-runtime-v2/src/service/session-system/sessions/representation/canonical-history-contract.ts
@@ -37,7 +37,12 @@ export interface CanonicalHistoryFileAdapter {
     verifiedActive?: readonly CanonicalHistoryEnvelope[],
   ): Promise<void>;
   replace(path: string, records: readonly CanonicalHistoryEnvelope[]): Promise<void>;
-  replaceActive(path: string, records: readonly CanonicalHistoryEnvelope[]): Promise<void>;
+  /** Additional validation of staging must complete before atomic publication. */
+  replaceActive(
+    path: string,
+    records: readonly CanonicalHistoryEnvelope[],
+    validateTemporary?: (path: string) => Promise<void>,
+  ): Promise<void>;
   publishSnapshot(
     snapshotPath: string,
     records: readonly CanonicalHistoryEnvelope[],
diff --git a/packages/local-runtime-v2/src/service/session-system/sessions/representation/canonical-history.ts b/packages/local-runtime-v2/src/service/session-system/sessions/representation/canonical-history.ts
index de6d10f..90310b5 100644
--- a/packages/local-runtime-v2/src/service/session-system/sessions/representation/canonical-history.ts
+++ b/packages/local-runtime-v2/src/service/session-system/sessions/representation/canonical-history.ts
@@ -66,8 +66,12 @@ class JsonlCanonicalHistoryFileAdapter implements CanonicalHistoryFileAdapter {
   async replace(path: string, records: readonly CanonicalHistoryEnvelope[]) {
     await this.source(path).replace(records);
   }
-  async replaceActive(path: string, records: readonly CanonicalHistoryEnvelope[]) {
-    await this.source(path).replaceActive(records);
+  async replaceActive(
+    path: string,
+    records: readonly CanonicalHistoryEnvelope[],
+    validateTemporary?: (path: string) => Promise<void>,
+  ) {
+    await this.source(path).replaceActive(records, validateTemporary);
   }
   async publishSnapshot(snapshotPath: string, records: readonly CanonicalHistoryEnvelope[]) {
     return this.source(snapshotPath).publishSnapshot(snapshotPath, records);

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions