fix(acp): close round-4 rewind identity gaps

R4-1 fork selection falls back to positional turns on partial identity
coverage (legacy-resumed sessions lost pre-resume context). R4-2
computeApiTruncationIndex only fails closed when API history actually
carries identities, so /restore's JSON round-trip state rewinds
positionally again. R4-3 partial-coverage loop skips placeholder-ONLY
entries; mixed text+placeholder entries force the positional fallback.
R4-5 goal-runtime turns keep their snapshot so the legacy snapshot<->turn
zip stays aligned. R4-6 legacy snapshot pairing validates alignment and
promptId before accepting a target; misaligned histories fail closed in
rewindToPrompt/rewindToTurn/getRewindableSnapshotTargets. R4-7 session
swap clears the rewind checkpoint so a stale restore cannot apply the old
session's rollback to the new recorder. R4-8 retry tests pin the promptId
argument.
This commit is contained in:
jinjing.zzj 2026-08-21 04:17:47 +08:00
parent f219a6ff80
commit fa88812ca5
6 changed files with 391 additions and 23 deletions

View file

@ -2198,6 +2198,12 @@ describe('Session', () => {
} as PromptRequest);
expect(mockChatRecordingService.recordUserMessage).toHaveBeenCalledTimes(1);
expect(mockChatRecordingService.recordUserMessage).toHaveBeenCalledWith(
'notification text',
undefined,
undefined,
'test-session-id########1',
);
expect(mockChat.sendMessageStream).toHaveBeenCalledWith(
'qwen3-code-plus',
expect.any(Object),
@ -2220,6 +2226,12 @@ describe('Session', () => {
} as PromptRequest);
expect(mockChatRecordingService.recordUserMessage).toHaveBeenCalledTimes(1);
expect(mockChatRecordingService.recordUserMessage).toHaveBeenCalledWith(
'token-limit retry',
undefined,
undefined,
'test-session-id########1',
);
expect(mockChat.sendMessageStream).toHaveBeenCalledWith(
'qwen3-code-plus',
expect.any(Object),
@ -2252,6 +2264,12 @@ describe('Session', () => {
} as PromptRequest);
expect(mockChatRecordingService.recordUserMessage).toHaveBeenCalledTimes(1);
expect(mockChatRecordingService.recordUserMessage).toHaveBeenCalledWith(
'second failed prompt',
undefined,
undefined,
'test-session-id########1',
);
expect(mockChat.sendMessageStream).toHaveBeenCalledWith(
'qwen3-code-plus',
expect.any(Object),
@ -3515,6 +3533,73 @@ describe('Session', () => {
]);
});
it('counts a legacy turn that mixes genuine text with a media-clear placeholder', () => {
// Microcompaction rebuilds entries as { ...content, parts: newParts }
// and preserves sibling text parts, so an entry mixing real text with
// a placeholder is a genuine legacy turn — only placeholder-ONLY
// entries are structural and may be skipped by the partial-coverage
// loop.
const history: Content[] = [
{
role: 'user',
parts: [
{ text: 'describe this' },
{ text: '[Old inline media cleared: image/png]' },
],
},
{ role: 'model', parts: [{ text: 'legacy reply' }] },
{ role: 'user', parts: [{ text: 'new prompt' }] },
];
core.markApiHistoryPrompt(history[2]!, 'prompt-new');
vi.mocked(mockChat.getHistoryShallow).mockReturnValue(history);
mockChatRecordingService.getRewindableTurnPromptIds.mockReturnValue([
undefined,
'prompt-new',
]);
expect(session.getRewindableUserTurnCount()).toBe(2);
});
it('fails closed on legacy snapshot pairing when turns lack snapshots', () => {
// A resumed legacy prefix carries no snapshots, so zipping snapshot
// indexes against positional turn indexes past the prefix would land
// on the wrong turn's boundary — refuse the pairing instead.
const history: Content[] = [
{ role: 'user', parts: [{ text: 'legacy one' }] },
{ role: 'model', parts: [{ text: 'legacy reply one' }] },
{ role: 'user', parts: [{ text: 'new prompt' }] },
{ role: 'model', parts: [{ text: 'new reply' }] },
];
core.markApiHistoryPrompt(history[2]!, 'prompt-new');
vi.mocked(mockChat.getHistory).mockReturnValue(history);
vi.mocked(mockChat.getHistoryShallow).mockReturnValue(history);
mockChatRecordingService.getRewindableTurnPromptIds.mockReturnValue([
undefined,
'prompt-new',
]);
mockFileHistoryService.isEnabled.mockReturnValue(true);
mockFileHistoryService.getSnapshots.mockReturnValue([
{
promptId: 'prompt-new',
timestamp: new Date('2026-06-13T00:00:00.000Z'),
trackedFileBackups: {},
},
]);
expect(session.getRewindableSnapshotTargets()).toEqual([]);
expect(() => session.rewindToPrompt('prompt-new')).toThrow(
'Cannot rewind to the requested prompt',
);
expect(session.rewindToTurn(1)).toEqual({
targetTurnIndex: 1,
apiTruncateIndex: 2,
});
expect(
mockFileHistoryService.restoreFromSnapshots,
).not.toHaveBeenCalled();
expect(mockChat.truncateHistory).toHaveBeenCalledWith(2);
});
it('does not fall back to positional rewinds when recorder identities are missing from API history', () => {
const history: Content[] = [
{ role: 'user', parts: [{ text: 'unidentified prompt' }] },
@ -3737,6 +3822,41 @@ describe('Session', () => {
).toHaveBeenCalledWith({}, snapshots, false);
});
it('drops the rewind checkpoint when a new session is rebound', () => {
// /clear swaps in fresh recording/file-history services; the
// checkpoint captured the old services' state, and a later
// restoreSessionHistory carrying the still-held pair must not apply
// session-1's rollback to session-2's recorder.
const history: Content[] = [
{ role: 'user', parts: [{ text: 'first' }] },
{ role: 'model', parts: [{ text: 'first reply' }] },
];
core.markApiHistoryPrompt(history[0]!, 'prompt-1');
vi.mocked(mockChat.getHistoryShallow).mockReturnValue(history);
mockChatRecordingService.getRewindableTurnPromptIds.mockReturnValue([
'prompt-1',
]);
mockFileHistoryService.getSnapshots.mockReturnValue([
{
promptId: 'prompt-1',
timestamp: new Date('2026-06-13T00:00:00.000Z'),
trackedFileBackups: {},
},
]);
session.rewindToPrompt('prompt-1', { rewindFiles: false });
session.rebindGoalRuntimeForNewSession();
session.restoreHistory(history, ['prompt-1', null]);
expect(mockChat.setHistory).toHaveBeenCalled();
expect(
mockChatRecordingService.restoreRewindCheckpoint,
).not.toHaveBeenCalled();
expect(
mockFileHistoryService.restoreFromSnapshots,
).not.toHaveBeenCalled();
});
it('uses legacy recording identities when restored history omits prompt ids', () => {
session.restoreHistory([
{ role: 'user', parts: [{ text: 'legacy prompt' }] },
@ -18218,6 +18338,53 @@ describe('Session', () => {
).not.toHaveBeenCalled();
});
it('snapshots file state for goal-runtime turns to keep legacy rewind aligned', async () => {
// A goal-runtime turn still counts as a positional user turn
// (#isUserTextContent passes its plain text) and is unmarked, so
// the legacy rewind path zips snapshot indexes against positional
// turn indexes. Skipping its snapshot would desync every slot
// after the first goal turn.
const permit: core.GoalTurnPermit = {
goalId: 'goal-1',
revision: 1,
turnId: 'turn-snapshot',
};
mockGoalRuntime.getSnapshot.mockReturnValue({
v: 2,
activity: 'running',
goal: {
goalId: 'goal-1',
revision: 1,
objective: 'check weather',
status: 'active',
evidenceCursor: { recordId: 'cursor-1' },
turnCount: 0,
activeTimeMs: 0,
createdAt: 1234,
updatedAt: 1234,
},
});
mockGoalRuntime.permitForTurn.mockImplementation((turnKey: string) =>
turnKey === 'goal-runtime:turn-snapshot' ? permit : undefined,
);
mockChat.sendMessageStream = vi
.fn()
.mockResolvedValue(createEmptyStream());
await boundGoalHost!.startGoalTurn({
permit,
continuationContext: 'check weather',
});
await vi.waitFor(() => {
expect(mockGoalRuntime.finishTurn).toHaveBeenCalledWith(permit);
});
expect(mockFileHistoryService.makeSnapshot).toHaveBeenCalledTimes(1);
expect(mockFileHistoryService.makeSnapshot).toHaveBeenCalledWith(
expect.any(String),
);
});
it('settles a Goal turn whose prompt rejects before the turn body runs', async () => {
// `prompt()` rejects ahead of the try whose finally settles the turn
// when `assertCanStartTurn` throws — a session that began closing

View file

@ -2069,6 +2069,13 @@ export class Session implements SessionContext {
this.lastGoalSnapshot = undefined;
this.lastGoalPublicationKey = undefined;
this.suppressedRecoveredGoalId = undefined;
// /clear swaps in a fresh ChatRecordingService/FileHistoryService via
// startNewSession. The checkpoint captured the OLD services' state; a
// later restoreSessionHistory carrying the pair the client still holds
// passes the positional promptIds guard and would apply session-1's
// rollback to session-2's recorder — resurrecting the cleared
// conversation and re-rooting the new transcript into the old one.
this.rewindCheckpoint = undefined;
this.#bindGoalRuntime();
}
@ -3514,6 +3521,7 @@ export class Session implements SessionContext {
projection!.mode,
apiHistory,
opts,
projection!.mode === 'legacy' ? projection!.turns.length : undefined,
);
}
@ -3534,12 +3542,21 @@ export class Session implements SessionContext {
let target = projection?.turns.find((turn) => turn.promptId === promptId);
if (projection?.mode === 'legacy') {
const snapshots = this.config.getFileHistoryService().getSnapshots();
const snapshotIndexes = this.#getSnapshotIndexesByPromptId(snapshots);
const targetTurnIndex = snapshotIndexes?.get(promptId);
target =
targetTurnIndex === undefined
? undefined
: projection.turns[targetTurnIndex];
// A snapshot's array index is a positional turn index only while every
// positional turn has a snapshot. A resumed legacy prefix has none, so
// any pairing past it is misaligned — fail closed instead of rewinding
// to the wrong turn's boundary.
if (snapshots.length >= projection.turns.length) {
const snapshotIndexes = this.#getSnapshotIndexesByPromptId(snapshots);
const targetTurnIndex = snapshotIndexes?.get(promptId);
target =
targetTurnIndex === undefined
? undefined
: projection.turns[targetTurnIndex];
if (target?.promptId && target.promptId !== promptId) {
target = undefined;
}
}
}
if (!target) {
throw RequestError.invalidParams(
@ -3553,6 +3570,7 @@ export class Session implements SessionContext {
projection!.mode,
apiHistory,
opts,
projection!.mode === 'legacy' ? projection!.turns.length : undefined,
);
}
@ -3570,6 +3588,7 @@ export class Session implements SessionContext {
mode: RewindTurnProjection['mode'],
apiHistory: Content[],
opts?: { rewindFiles?: boolean },
legacyTurnCount?: number,
): SessionRewindResult {
if (target.recordingTurnIndex === undefined) {
throw RequestError.invalidParams(
@ -3579,9 +3598,18 @@ export class Session implements SessionContext {
}
const fileHistoryService = this.config.getFileHistoryService();
const snapshots = fileHistoryService.getSnapshots();
// Legacy mode pairs snapshot indexes with positional turn indexes; the
// pairing is only sound while every positional turn has a snapshot
// (resumed legacy turns have none).
const legacySnapshotsAligned =
mode === 'legacy' &&
legacyTurnCount !== undefined &&
snapshots.length >= legacyTurnCount;
const effectivePromptId =
target.promptId ??
(mode === 'legacy' ? snapshots[target.turnIndex]?.promptId : undefined);
(legacySnapshotsAligned
? snapshots[target.turnIndex]?.promptId
: undefined);
const rewindFiles = opts?.rewindFiles !== false;
let survivingSnapshots:
| ReturnType<typeof fileHistoryService.getSnapshots>
@ -3600,7 +3628,10 @@ export class Session implements SessionContext {
targetSnapshotIndex = effectivePromptId
? snapshotIndexes.get(effectivePromptId)
: undefined;
} else if (target.turnIndex < snapshots.length) {
} else if (
legacySnapshotsAligned &&
target.turnIndex < snapshots.length
) {
targetSnapshotIndex = target.turnIndex;
}
if (targetSnapshotIndex !== undefined) {
@ -3695,18 +3726,23 @@ export class Session implements SessionContext {
// structural replacements inside an identified session, not legacy
// turns. The shared predicate matches only complete placeholders, so a
// genuine user prompt that merely starts with the prefix still counts.
// Skip only placeholder-ONLY entries: microcompaction rebuilds entries
// as { ...content, parts: newParts } and preserves sibling text parts,
// so an entry mixing genuine text with a placeholder is a real legacy
// turn and must force the positional fallback (mirrors the TUI twin's
// documented rule in historyMapping.ts).
for (let index = startIndex; index < apiHistory.length; index++) {
const content = apiHistory[index]!;
if (!this.#isUserTextContent(content)) continue;
if (getApiHistoryPromptId(content)) continue;
if (
content.parts?.some(
(part) =>
'text' in part &&
typeof part.text === 'string' &&
isClearedMediaPlaceholder(part.text),
)
) {
const textParts =
content.parts
?.filter(
(part): part is Part & { text: string } =>
'text' in part && typeof part.text === 'string',
)
.map((part) => part.text) ?? [];
if (textParts.length > 0 && textParts.every(isClearedMediaPlaceholder)) {
continue;
}
return { mode: 'legacy', turns: positionalTurns() };
@ -3775,6 +3811,10 @@ export class Session implements SessionContext {
const snapshotIndexes = this.#getSnapshotIndexesByPromptId(snapshots);
if (!snapshotIndexes) return [];
if (projection.mode === 'legacy') {
// Same positional zip as the rewind path: unsound once a resumed
// legacy prefix leaves turns without snapshots — advertise nothing
// rather than mispaired slots.
if (snapshots.length < projection.turns.length) return [];
return snapshots
.slice(0, projection.turns.length)
.map((snapshot, turnIndex) => ({
@ -5098,7 +5138,13 @@ export class Session implements SessionContext {
// slash-command and hook early-returns so locally handled commands
// don't create phantom snapshots that desync the snapshot index.
// Resubmissions keep updating the original turn's latest snapshot.
if (!isContinue && !isRetry && goalTurn?.origin !== 'runtime') {
// Goal-runtime turns MUST keep their snapshot: they still count
// as positional user turns (#isUserTextContent passes their plain
// text) and the legacy rewind path zips snapshot indexes against
// positional turn indexes — skipping the snapshot desyncs every
// slot after the first goal turn (file restore lands on the wrong
// snapshot or is silently skipped).
if (!isContinue && !isRetry) {
try {
const fileHistoryService = this.config.getFileHistoryService();
await fileHistoryService.makeSnapshot(promptId);

View file

@ -715,10 +715,17 @@ describe('computeApiTruncationIndex', () => {
expect(computeApiTruncationIndex(ui, 5, api)).toBe(5);
});
it('fails closed when an identified target is absent from API history', () => {
it('rewinds positionally when the target carries a promptId but no API entry is marked', () => {
// Superseded by R4-2: this shape is exactly what /restore produces —
// the checkpoint round-trip strips the symbol identities from API
// history while the UI item keeps its string promptId. The target
// textually exists, so the pre-identity positional path must run
// instead of aborting the rewind. Fail-closed still applies when the
// API history DOES carry identities and the target is not among them
// (see 'identified mode' below).
const ui = [userItem(1, 'hello', undefined, 'prompt-1')];
expect(computeApiTruncationIndex(ui, 1, [userContent('hello')])).toBe(-1);
expect(computeApiTruncationIndex(ui, 1, [userContent('hello')])).toBe(0);
});
});
@ -834,6 +841,70 @@ describe('computeApiTruncationIndex', () => {
expect(computeApiTruncationIndex(ui, 1, api)).toBe(0);
});
});
describe('identified mode', () => {
it('resolves the target by promptId when API history carries identities', () => {
const ui: HistoryItem[] = [
userItem(1, 'prompt 1', true, 'prompt-id-1'),
geminiItem(2),
userItem(3, 'prompt 3', true, 'prompt-id-3'),
geminiItem(4),
];
const api: Content[] = [
userContent('prompt 1'),
modelContent('response 1'),
userContent('prompt 3'),
modelContent('response 3'),
];
markApiHistoryPrompt(api[0]!, 'prompt-id-1');
markApiHistoryPrompt(api[2]!, 'prompt-id-3');
expect(computeApiTruncationIndex(ui, 3, api)).toBe(2);
});
it('fails closed when identities exist but the target promptId is gone', () => {
const ui: HistoryItem[] = [
userItem(1, 'prompt 1', true, 'prompt-id-1'),
geminiItem(2),
userItem(3, 'prompt 3', true, 'prompt-id-3'),
geminiItem(4),
];
const api: Content[] = [
userContent('prompt 1'),
modelContent('response 1'),
userContent('prompt 3'),
modelContent('response 3'),
];
markApiHistoryPrompt(api[0]!, 'prompt-id-1');
// The second entry carries an identity, but the target's promptId is
// not among them (e.g. absorbed by compression) — fail closed.
markApiHistoryPrompt(api[2]!, 'prompt-id-other');
expect(computeApiTruncationIndex(ui, 3, api)).toBe(-1);
});
it('falls back to positional counting after a JSON round-trip stripped the identities', () => {
// /restore installs the checkpoint round-tripped through
// JSON.stringify, which drops the symbol-keyed identity from
// clientHistory while persisted UI items keep their string promptId.
// The target textually exists, so the positional path must run
// instead of aborting the rewind with -1.
const ui: HistoryItem[] = [
userItem(1, 'prompt 1', true, 'prompt-id-1'),
geminiItem(2),
userItem(3, 'prompt 3', true, 'prompt-id-3'),
geminiItem(4),
];
const api: Content[] = [
userContent('prompt 1'),
modelContent('response 1'),
userContent('prompt 3'),
modelContent('response 3'),
];
expect(computeApiTruncationIndex(ui, 3, api)).toBe(2);
});
});
});
describe('isRealUserTurn', () => {

View file

@ -9,6 +9,7 @@ import type { Content } from '@google/genai';
import {
CompressionStatus,
findApiHistoryPromptIndex,
getApiHistoryPromptIndexes,
getStartupContextLength,
isClearedMediaPlaceholder,
isSystemReminderContent,
@ -156,7 +157,21 @@ export function computeApiTruncationIndex(
const target = uiHistory[targetIndex]!;
if (isRealUserTurn(target) && target.promptId) {
return findApiHistoryPromptIndex(apiHistory, target.promptId);
const identifiedIndex = findApiHistoryPromptIndex(
apiHistory,
target.promptId,
);
// Fail closed only when the API history actually carries identities.
// /restore installs a checkpoint round-tripped through JSON.stringify,
// which drops the symbol-keyed identity from clientHistory while the
// persisted UI items keep their string promptId — the target textually
// exists in API history, so the positional fallback below is still
// sound and must run instead of aborting the rewind with -1.
const historyCarriesIdentities =
(getApiHistoryPromptIndexes(apiHistory) ?? []).length > 0;
if (identifiedIndex !== -1 || historyCarriesIdentities) {
return identifiedIndex;
}
}
// Legacy sessions have no promptId and still need the positional fallback.

View file

@ -175,6 +175,37 @@ describe('selectForkHistory', () => {
).toEqual([firstUser, firstModel, placeholder, secondUser, secondModel]);
});
it('falls back to positional turns when identity coverage is partial', () => {
// A session resumed from before stable identities existed rebuilds its
// legacy entries unmarked; the first new prompt lands marked. Slicing
// from the marked indexes alone would hand the fork only post-resume
// turns and silently drop the requested legacy context.
const identifiedNew = structuredClone(secondUser);
markApiHistoryPrompt(identifiedNew, 'prompt-new');
expect(
selectForkHistory(
[
startup,
firstUser,
firstModel,
secondUser,
secondModel,
identifiedNew,
{ role: 'model', parts: [{ text: 'new answer' }] },
],
2,
),
).toEqual([
secondUser,
secondModel,
// selectForkHistory structuredClones its result, which drops the
// symbol-keyed identity by design.
{ role: 'user', parts: [{ text: 'second question' }] },
{ role: 'model', parts: [{ text: 'new answer' }] },
]);
});
it('keeps all available context when fewer turns exist than requested', () => {
expect(selectForkHistory([startup, firstUser, firstModel], 3)).toEqual([
firstUser,

View file

@ -1,5 +1,5 @@
import { AsyncLocalStorage } from 'node:async_hooks';
import type { Content } from '@google/genai';
import type { Content, Part } from '@google/genai';
import type { Config } from '../../config/config.js';
import type { SubagentConfig } from '../../subagents/types.js';
import { BUBBLE_APPROVAL_MODE } from '../../subagents/types.js';
@ -8,7 +8,11 @@ import {
getStartupContextLength,
isSystemReminderContent,
} from '../../utils/environmentContext.js';
import { getApiHistoryPromptIndexes } from '../../services/session-api-history.js';
import {
getApiHistoryPromptId,
getApiHistoryPromptIndexes,
} from '../../services/session-api-history.js';
import { isClearedMediaPlaceholder } from '../../services/microcompaction/microcompact.js';
export const FORK_SUBAGENT_TYPE = 'fork';
@ -241,9 +245,43 @@ export function selectForkHistory(
const identifiedTurns = getApiHistoryPromptIndexes(history);
const realUserTurnIndexes =
identifiedTurns?.filter((index) => index >= syntheticPrefixLength) ?? [];
// Partial identity coverage: a session resumed from before stable
// identities existed gains one marked entry as soon as a new prompt
// lands. Slicing from the marked indexes alone would silently drop
// every older (unmarked) turn from the fork window, so while any
// identity-less real user turn exists, fall back to the positional
// enumeration — the same guard Session.#getRewindTurnProjection keeps,
// including its exception for placeholder-ONLY entries (structural
// media-clear replacements inside an identified session; an entry
// mixing genuine text with a placeholder is still a real legacy turn).
let hasUnmarkedRealUserTurn = false;
if (identifiedTurns !== undefined && realUserTurnIndexes.length > 0) {
for (let index = syntheticPrefixLength; index < history.length; index++) {
const content = history[index]!;
if (!isRealUserTurn(content) || getApiHistoryPromptId(content)) {
continue;
}
const textParts =
content.parts
?.filter(
(part): part is Part & { text: string } =>
typeof part.text === 'string',
)
.map((part) => part.text) ?? [];
if (
textParts.length > 0 &&
textParts.every(isClearedMediaPlaceholder)
) {
continue;
}
hasUnmarkedRealUserTurn = true;
break;
}
}
if (identifiedTurns === undefined) {
selected = [];
} else if (realUserTurnIndexes.length === 0) {
} else if (realUserTurnIndexes.length === 0 || hasUnmarkedRealUserTurn) {
realUserTurnIndexes.length = 0;
for (let index = syntheticPrefixLength; index < history.length; index++) {
const content = history[index]!;
if (isRealUserTurn(content)) {