Skip to content

fix(core): microcompact hook continuations #4840

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wenshao merged 11 commits into from
Jun 10, 2026
Merged

fix(core): microcompact hook continuations #4840

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ef7d673
fix(core): microcompact hook continuations
qqqys Jun 8, 2026
1d97164
test(core): cover hook microcompaction edge cases
qqqys Jun 8, 2026
b5b1fc3
test(core): cover hook checkpoint fallback
qqqys Jun 8, 2026
5f9310d
chore(core): refresh hook microcompaction checks
qqqys Jun 8, 2026
f8a33cb
fix(core): keep hook microcompaction cleanup best-effort
qqqys Jun 9, 2026
3fb72f8
test(core): cover hook microcompaction checkpoint state
qqqys Jun 9, 2026
633b31a
test(core): cover hook microcompaction review cases
qqqys Jun 9, 2026
737ac01
fix(core): retain hook checkpoint after microcompaction failure
qqqys Jun 9, 2026
55f4a43
fix(core): clarify hook microcompaction comment
qqqys Jun 9, 2026
0e104e1
fix(ci): normalize dev launcher path assertions
qqqys Jun 9, 2026
92546b8
fix(core): preserve hook checkpoint on cron no-op
qqqys Jun 9, 2026
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
267 changes: 267 additions & 0 deletions packages/core/src/core/client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1588,6 +1588,14 @@ describe('Gemini Client (client.ts)', () => {
await client.resetChat();
expect(client['lastInjectedDate']).toBeUndefined();
});

it('resets Hook microcompaction checkpoint', async () => {
client['lastHookMicrocompactionTimestamp'] = Date.now();

await client.resetChat();

expect(client['lastHookMicrocompactionTimestamp']).toBeNull();
});
});

describe('history mutation invalidates FileReadCache', () => {
Expand Down Expand Up @@ -1829,6 +1837,25 @@ describe('Gemini Client (client.ts)', () => {

expect(client['lastApiCompletionTimestamp']).toBeNull();
});

it('seeds Hook microcompaction checkpoint on user turns', async () => {
client['lastHookMicrocompactionTimestamp'] = null;
const before = Date.now();

const gen = client.sendMessageStream(
[{ text: 'Hello' }],
new AbortController().signal,
'prompt-hook-seed',
{ type: SendMessageType.UserQuery },
);
for await (const _ of gen) {
/* drain */
}

expect(client['lastHookMicrocompactionTimestamp']).toBeGreaterThanOrEqual(
before,
);
});
});

describe('microcompaction FileReadCache invalidation', () => {
Expand Down Expand Up @@ -1924,6 +1951,246 @@ describe('Gemini Client (client.ts)', () => {
expect(markReadEvictedFromHistory).toHaveBeenCalledTimes(1);
});

it('does not abort the turn when microcompaction cleanup fails', async () => {
const { markReadEvictedFromHistory } = mockFileReadCacheStub();
markReadEvictedFromHistory.mockImplementation(() => {
throw new Error('cache disarm failed');
});

const { history } = await makeReadFileResponses(6);
client['chat'] = {
addHistory: vi.fn(),
getHistory: vi.fn().mockReturnValue(history),
setHistory: vi.fn(),
} as unknown as GeminiChat;
client['lastApiCompletionTimestamp'] = Date.now() - 90 * 60_000;

const events: ServerGeminiStreamEvent[] = [];
const stream = client.sendMessageStream(
[{ text: 'hi' }],
new AbortController().signal,
'prompt-mc-error-boundary',
{ type: SendMessageType.UserQuery },
);
for await (const event of stream) {
events.push(event);
}

expect(events).toEqual([
{ type: GeminiEventType.Content, value: 'response' },
]);
});

it('microcompacts old tool results on Hook continuations', async () => {

@wenshao wenshao Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This test covers the happy path (pre-set stale timestamp → trigger fires → cleanup runs), but several important paths are untested:

  1. ??= initialization: when lastHookMicrocompactionTimestamp is null and lastApiCompletionTimestamp is an old value, the Hook should inherit that timestamp and potentially trigger. This is the most logic-dense line in the change.
  2. Negative test: set lastHookMicrocompactionTimestamp = Date.now() (recent), send a Hook message, and assert setHistory is NOT called — guards against unconditional microcompaction on every Hook.
  3. Both-null fallback: both timestamps null → should use Date.now() and NOT trigger.
  4. Timestamp reset: after microcompaction fires, assert lastHookMicrocompactionTimestamp is updated to a recent value.

Adding at minimum the negative test and the ??= init test would significantly strengthen coverage for the subtle parts of this change.

— qwen3.7-max via Qwen Code /review

const { clear, markReadEvictedFromHistory } = mockFileReadCacheStub();

const { history } = await makeReadFileResponses(6);
const setHistory = vi.fn();
client['chat'] = {
addHistory: vi.fn(),
getHistory: vi.fn().mockReturnValue(history),
setHistory,
} as unknown as GeminiChat;
client['lastApiCompletionTimestamp'] = Date.now();
client['lastHookMicrocompactionTimestamp'] = Date.now() - 90 * 60_000;

const stream = client.sendMessageStream(
[{ text: 'continue goal' }],
new AbortController().signal,
'prompt-mc-hook',
{ type: SendMessageType.Hook },
);
for await (const _ of stream) {
/* drain */
}

expect(setHistory).toHaveBeenCalled();
expect(clear).not.toHaveBeenCalled();
expect(markReadEvictedFromHistory).toHaveBeenCalledTimes(1);
expect(client['lastHookMicrocompactionTimestamp']).toBeGreaterThan(
Date.now() - 60_000,
);
});

it('does not abort Hook continuations when microcompaction cleanup fails', async () => {
const { markReadEvictedFromHistory } = mockFileReadCacheStub();
markReadEvictedFromHistory.mockImplementation(() => {
throw new Error('hook cache disarm failed');
});

const { history } = await makeReadFileResponses(6);
client['chat'] = {
addHistory: vi.fn(),
getHistory: vi.fn().mockReturnValue(history),
setHistory: vi.fn(),
} as unknown as GeminiChat;
client['lastApiCompletionTimestamp'] = Date.now();
const checkpoint = Date.now() - 90 * 60_000;
client['lastHookMicrocompactionTimestamp'] = checkpoint;
mockClientDebugLogger.error.mockClear();

const events: ServerGeminiStreamEvent[] = [];
const stream = client.sendMessageStream(
[{ text: 'continue goal' }],
new AbortController().signal,
'prompt-mc-hook-error-boundary',
{ type: SendMessageType.Hook },
);
for await (const event of stream) {
events.push(event);
}

expect(events).toEqual([
{ type: GeminiEventType.Content, value: 'response' },
]);
expect(mockClientDebugLogger.error).toHaveBeenCalledWith(
expect.stringContaining(
'microcompactHistory failed: hook cache disarm failed',
),
);
expect(client['lastHookMicrocompactionTimestamp']).toBe(checkpoint);
});

it('skips the next Hook microcompaction after one just ran', async () => {
const { clear, markReadEvictedFromHistory } = mockFileReadCacheStub();

const { history } = await makeReadFileResponses(6);
const setHistory = vi.fn();
client['chat'] = {
addHistory: vi.fn(),
getHistory: vi.fn().mockReturnValue(history),
setHistory,
} as unknown as GeminiChat;
client['lastApiCompletionTimestamp'] = Date.now();
client['lastHookMicrocompactionTimestamp'] = Date.now() - 90 * 60_000;

const firstStream = client.sendMessageStream(
[{ text: 'continue goal' }],
new AbortController().signal,
'prompt-mc-hook-fire',
{ type: SendMessageType.Hook },
);
for await (const _ of firstStream) {
/* drain */
}

const checkpointAfterFire = client['lastHookMicrocompactionTimestamp'];
expect(setHistory).toHaveBeenCalled();
expect(checkpointAfterFire).toBeGreaterThan(Date.now() - 60_000);

setHistory.mockClear();
clear.mockClear();
markReadEvictedFromHistory.mockClear();

const secondStream = client.sendMessageStream(
[{ text: 'continue goal again' }],
new AbortController().signal,
'prompt-mc-hook-skip',
{ type: SendMessageType.Hook },
);
for await (const _ of secondStream) {
/* drain */
}

expect(client['lastHookMicrocompactionTimestamp']).toBe(
checkpointAfterFire,
);
expect(setHistory).not.toHaveBeenCalled();
expect(clear).not.toHaveBeenCalled();
expect(markReadEvictedFromHistory).not.toHaveBeenCalled();
});

it('initializes Hook microcompaction from the last API completion timestamp', async () => {
const { clear, markReadEvictedFromHistory } = mockFileReadCacheStub();

const { history } = await makeReadFileResponses(6);
const setHistory = vi.fn();
client['chat'] = {
addHistory: vi.fn(),
getHistory: vi.fn().mockReturnValue(history),
setHistory,
} as unknown as GeminiChat;
client['lastApiCompletionTimestamp'] = Date.now() - 90 * 60_000;
client['lastHookMicrocompactionTimestamp'] = null;

const stream = client.sendMessageStream(
[{ text: 'continue goal' }],
new AbortController().signal,
'prompt-mc-hook-init',
{ type: SendMessageType.Hook },
);
for await (const _ of stream) {
/* drain */
}

expect(setHistory).toHaveBeenCalled();
expect(clear).not.toHaveBeenCalled();
expect(markReadEvictedFromHistory).toHaveBeenCalledTimes(1);
expect(client['lastHookMicrocompactionTimestamp']).toBeGreaterThan(
Date.now() - 60_000,
);
});

it('does not microcompact Hook continuations when the checkpoint is recent', async () => {
const { clear, markReadEvictedFromHistory } = mockFileReadCacheStub();

const { history } = await makeReadFileResponses(6);
const setHistory = vi.fn();
client['chat'] = {
addHistory: vi.fn(),
getHistory: vi.fn().mockReturnValue(history),
setHistory,
} as unknown as GeminiChat;
client['lastApiCompletionTimestamp'] = Date.now() - 90 * 60_000;
client['lastHookMicrocompactionTimestamp'] = Date.now();

const stream = client.sendMessageStream(
[{ text: 'continue goal' }],
new AbortController().signal,
'prompt-mc-hook-recent',
{ type: SendMessageType.Hook },
);
for await (const _ of stream) {
/* drain */
}

expect(setHistory).not.toHaveBeenCalled();
expect(clear).not.toHaveBeenCalled();
expect(markReadEvictedFromHistory).not.toHaveBeenCalled();
});

it('seeds Hook microcompaction checkpoint to now when no API call completed', async () => {
const { clear, markReadEvictedFromHistory } = mockFileReadCacheStub();

const { history } = await makeReadFileResponses(6);
const setHistory = vi.fn();
client['chat'] = {
addHistory: vi.fn(),
getHistory: vi.fn().mockReturnValue(history),
setHistory,
} as unknown as GeminiChat;
client['lastApiCompletionTimestamp'] = null;
client['lastHookMicrocompactionTimestamp'] = null;
const before = Date.now();

const stream = client.sendMessageStream(
[{ text: 'continue goal' }],
new AbortController().signal,
'prompt-mc-hook-no-api-completion',
{ type: SendMessageType.Hook },
);
for await (const _ of stream) {
/* drain */
}

expect(client['lastHookMicrocompactionTimestamp']).toBeGreaterThanOrEqual(
before,
);
expect(setHistory).not.toHaveBeenCalled();
expect(clear).not.toHaveBeenCalled();
expect(markReadEvictedFromHistory).not.toHaveBeenCalled();
});

it('falls back to a blanket clear when blanked reads cannot be linked to a path (id-less provider)', async () => {
// Provider did not populate functionCall.id, so microcompaction
// cannot recover the blanked reads' file paths. Leaving their
Expand Down
Loading
Loading