Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/node/services/agentSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export class AgentSession {
this.compactionHandler = new CompactionHandler({
workspaceId: this.workspaceId,
historyService: this.historyService,
partialService: this.partialService,
emitter: this.emitter,
});

Expand Down
41 changes: 41 additions & 0 deletions src/node/services/compactionHandler.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { describe, it, expect, beforeEach, mock } from "bun:test";
import { CompactionHandler } from "./compactionHandler";
import type { HistoryService } from "./historyService";
import type { PartialService } from "./partialService";
import type { EventEmitter } from "events";
import { createMuxMessage, type MuxMessage } from "@/common/types/message";
import type { StreamEndEvent } from "@/common/types/stream";
Expand Down Expand Up @@ -48,6 +49,26 @@ const createMockHistoryService = () => {
};
};

const createMockPartialService = () => {
let deletePartialResult: Result<void, string> = Ok(undefined);

const deletePartial = mock((_) => Promise.resolve(deletePartialResult));
const readPartial = mock((_) => Promise.resolve(null));
const writePartial = mock((_, __) => Promise.resolve(Ok(undefined)));
const commitToHistory = mock((_) => Promise.resolve(Ok(undefined)));

return {
deletePartial,
readPartial,
writePartial,
commitToHistory,
// Allow setting mock return values
mockDeletePartial: (result: Result<void, string>) => {
deletePartialResult = result;
},
};
};

const createMockEmitter = (): { emitter: EventEmitter; events: EmittedEvent[] } => {
const events: EmittedEvent[] = [];
const emitter = {
Expand Down Expand Up @@ -112,6 +133,7 @@ const setupSuccessfulCompaction = (
describe("CompactionHandler", () => {
let handler: CompactionHandler;
let mockHistoryService: ReturnType<typeof createMockHistoryService>;
let mockPartialService: ReturnType<typeof createMockPartialService>;
let mockEmitter: EventEmitter;
let emittedEvents: EmittedEvent[];
const workspaceId = "test-workspace";
Expand All @@ -122,10 +144,12 @@ describe("CompactionHandler", () => {
emittedEvents = events;

mockHistoryService = createMockHistoryService();
mockPartialService = createMockPartialService();

handler = new CompactionHandler({
workspaceId,
historyService: mockHistoryService as unknown as HistoryService,
partialService: mockPartialService as unknown as PartialService,
emitter: mockEmitter,
});
});
Expand Down Expand Up @@ -209,6 +233,23 @@ describe("CompactionHandler", () => {
);
});

it("should delete partial.json before clearing history (race condition fix)", async () => {
const compactionReq = createCompactionRequest();
mockHistoryService.mockGetHistory(Ok([compactionReq]));
mockHistoryService.mockClearHistory(Ok([0]));
mockHistoryService.mockAppendToHistory(Ok(undefined));

const event = createStreamEndEvent("Summary");
await handler.handleCompletion(event);

// deletePartial should be called once before clearHistory
expect(mockPartialService.deletePartial.mock.calls).toHaveLength(1);
expect(mockPartialService.deletePartial.mock.calls[0][0]).toBe(workspaceId);

// Verify deletePartial was called (we can't easily verify order without more complex mocking,
// but the important thing is that it IS called during compaction)
});

it("should call clearHistory() and appendToHistory()", async () => {
const compactionReq = createCompactionRequest();
mockHistoryService.mockGetHistory(Ok([compactionReq]));
Expand Down
16 changes: 16 additions & 0 deletions src/node/services/compactionHandler.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { EventEmitter } from "events";
import type { HistoryService } from "./historyService";
import type { PartialService } from "./partialService";
import type { StreamEndEvent } from "@/common/types/stream";
import type { WorkspaceChatMessage, DeleteMessage } from "@/common/orpc/types";
import type { Result } from "@/common/types/result";
Expand All @@ -13,6 +14,7 @@ import { log } from "@/node/services/log";
interface CompactionHandlerOptions {
workspaceId: string;
historyService: HistoryService;
partialService: PartialService;
emitter: EventEmitter;
}

Expand All @@ -27,12 +29,14 @@ interface CompactionHandlerOptions {
export class CompactionHandler {
private readonly workspaceId: string;
private readonly historyService: HistoryService;
private readonly partialService: PartialService;
private readonly emitter: EventEmitter;
private readonly processedCompactionRequestIds: Set<string> = new Set<string>();

constructor(options: CompactionHandlerOptions) {
this.workspaceId = options.workspaceId;
this.historyService = options.historyService;
this.partialService = options.partialService;
this.emitter = options.emitter;
}

Expand Down Expand Up @@ -106,6 +110,18 @@ export class CompactionHandler {

const historicalUsage = usageHistory.length > 0 ? sumUsageHistory(usageHistory) : undefined;

// CRITICAL: Delete partial.json BEFORE clearing history
// This prevents a race condition where:
// 1. CompactionHandler clears history and appends summary
// 2. sendQueuedMessages triggers commitToHistory
// 3. commitToHistory finds stale partial.json and appends it to history
// By deleting partial first, commitToHistory becomes a no-op
const deletePartialResult = await this.partialService.deletePartial(this.workspaceId);
if (!deletePartialResult.success) {
log.warn(`Failed to delete partial before compaction: ${deletePartialResult.error}`);
// Continue anyway - the partial may not exist, which is fine
}

// Clear entire history and get deleted sequences
const clearResult = await this.historyService.clearHistory(this.workspaceId);
if (!clearResult.success) {
Expand Down
Loading