Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
19 changes: 19 additions & 0 deletions packages/cli/src/acp-integration/session/Session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,7 @@ describe('Session', () => {
truncateHistory: vi.fn(),
stripThoughtsFromHistory: vi.fn(),
stripOrphanedUserEntriesFromHistory: vi.fn().mockReturnValue([]),
reconcileLoadedSkillTracking: vi.fn(),
setTools: vi.fn(),
} as unknown as GeminiChat;
mockGeminiClient = {
Expand Down Expand Up @@ -2623,6 +2624,9 @@ describe('Session', () => {
mockChat.getHistory = vi
.fn()
.mockReturnValue([{ role: 'user', parts: [{ text: 'unanswered' }] }]);
mockChat.stripOrphanedUserEntriesFromHistory = vi
.fn()
.mockReturnValue([{ role: 'user', parts: [{ text: 'unanswered' }] }]);
// Force the continuation send to fail NON-cancelled (session token limit)
// so it hits the `!responseStream` branch — the data-loss window.
mockConfig.getSessionTokenLimit = vi.fn().mockReturnValue(100);
Expand Down Expand Up @@ -2651,6 +2655,12 @@ describe('Session', () => {
]),
}),
);
// The strip un-tracked any skill body it removed; once the orphan is
// preserved back, the settle reconcile rebuilds tracking from the
// settled history (residency aware — not an additive re-track).
expect(mockChat.reconcileLoadedSkillTracking).toHaveBeenCalledWith(
'acpTurnSettle',
);
});

it('restores the orphaned turn when a continuation send throws (no data loss)', async () => {
Expand Down Expand Up @@ -2688,6 +2698,9 @@ describe('Session', () => {
]),
}),
);
expect(mockChat.reconcileLoadedSkillTracking).toHaveBeenCalledWith(
'acpTurnSettle',
);
});

it('rejects (accepted:false) when a prompt is already in flight', async () => {
Expand Down Expand Up @@ -32035,6 +32048,12 @@ describe('Session', () => {
.mock.calls.map(([params]) => params.update._meta?.['attempt'])
.filter((attempt) => attempt !== undefined),
).toEqual([1, 2, 2]);
// The isRetry strip discards its return value but still arms the
// settle reconcile (R3-3): tracking must be rebuilt from the settled
// history even when the strip found nothing to return.
expect(mockChat.reconcileLoadedSkillTracking).toHaveBeenCalledWith(
'acpTurnSettle',
);
});

it('keeps a queued prompt ahead of related automatic input', async () => {
Expand Down
47 changes: 47 additions & 0 deletions packages/cli/src/acp-integration/session/Session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4446,6 +4446,19 @@ export class Session implements SessionContext {
// — so hold it (and a push-count snapshot) to restore on that path.
let strippedOrphanEntries: Content[] | null = null;
let orphanPushCountSnapshot = 0;
// The strip un-tracks any skill body it removes; every
// terminal path of this turn re-adds the stripped content
// (send re-push, catch restore, or the abort addHistory).
// This flag arms the settle reconcile in the outer finally,
// which rebuilds tracking from the SETTLED history —
// residency aware, because a mid-turn rewrite can blank or
// summarize away the re-pushed body again. It arms on strip
// OCCURRENCE, not on what the strip removed: an unresolvable
// stripped body makes the strip blanket-clear all tracking
// while resolving names would return [], and the isRetry
// strip below discards its return value entirely — both must
// still be reconciled at settle.
let orphanStripRan = false;
if (goalTurn?.origin === 'runtime') {
this.config.getChatRecordingService()?.recordGoalRuntimeMessage(
modelPromptBlocks
Expand Down Expand Up @@ -4481,6 +4494,7 @@ export class Session implements SessionContext {
strippedOrphanEntries =
this.#getCurrentChat().stripOrphanedUserEntriesFromHistory() ??
null;
orphanStripRan = true;
orphanPushCountSnapshot =
this.#getCurrentChat().getUserContentPushCount?.() ?? 0;
continuationParts = recoveryPlan.continuation.parts;
Expand All @@ -4497,6 +4511,7 @@ export class Session implements SessionContext {
// message would duplicate the turn in the transcript.
} else if (isRetry) {
this.#getCurrentChat().stripOrphanedUserEntriesFromHistory();
orphanStripRan = true;
Comment thread
ZijianZhang989 marked this conversation as resolved.
Resolved
} else if (!isSlashInput || slashCommandName !== 'advisor') {
// record user message for session management. Only `/advisor`
// defers its record to after command resolution below — a
Expand Down Expand Up @@ -5157,6 +5172,38 @@ export class Session implements SessionContext {
}
return result;
} finally {
// Fires on every terminal path that reaches this send-loop
// try — including the top-of-loop abort return that re-adds
// the stripped content via addHistory without entering the
// send-try's finally. Five terminal paths exit BEFORE this
// try (/advisor aborted return, slash-command result catch
// rethrow, locally-handled command return, #resolvePrompt
// throw, UserPromptSubmit hook block) and skip the reconcile:
// safe because the strip's own unload sync already applied,
// except the blanket-clear fallback corner — still-resident
// skills stay untracked until the next invoke re-injects one
// duplicate body and the re-track self-heals.
if (orphanStripRan) {
// Residency-aware, not additive: a mid-turn rewrite
// (tryCompress / microcompaction) can blank or summarize
// away the re-pushed body and correctly un-track it;
// re-adding the stashed names anyway would resurrect the
// ghost the strip just removed. Mirrors the TUI twin in
// restoreStrippedRetryEntries (client.ts).
//
// Accepted trade-off: the pre-send tryCompress can also
// summarize away the re-pushed body's pairing model-side
// functionCall, leaving the body resident-but-untracked at
// settle — the next invoke then injects one duplicate body
// and self-heals (the documented direction in
// unloadSkillsFromEntries). Rebuilding from the stashed ids
// would add a second residency-truth path beside the
// reconcile; not worth it for a self-healing duplicate.
this.#getCurrentChat().reconcileLoadedSkillTracking(
'acpTurnSettle',
);
orphanStripRan = false;
}
logConversationFinishedEvent(
this.config,
new ConversationFinishedEvent(
Expand Down
Loading
Loading