Skip to content

Commit 0753f13

Browse files
authored
fix(agent-core-v2): clear the turn outcome when an undo rewinds it (#213)
## Related Issue No issue; maintainer fix found while reviewing undo behavior on the v2 engine. ## Problem After a turn was interrupted (or failed / completed) and the user then undid that turn, the "manually stopped" outcome stayed visible in the activity view and was still persisted in the session metadata. Undo rewound the context, but nothing cleared the outcome that described the rewound turn, so clients kept showing a stale state. ## What changed - `turnOps.ts`: the `ContextUndo` reducer now clears `lastEnded` when the undo rewinds to or past the turn it describes; it keeps the outcome when only later turns are rewound. - `activityViewService.ts`: subscribes to `ContextUndone` and drops `lastTurn` when the undone range covers it, so the activity view republishes without the stale outcome. - `sessionOutcomeMirrorService.ts`: tracks the turn id behind the persisted outcome, clears it on `ContextUndone` when that turn was rewound (including when the undo outruns the tracked anchors), and reconciles the persisted outcome against the replayed wire on restore so a session resumed after an undo does not revive it. - Tests for each of the three paths (rewind-covers-turn, rewind-later-turns-only, restore reconciliation). ## Verification - `pnpm --filter @pymodel/agent-core-v2 exec vitest run` — 347 files, 5,719 tests passed - `pnpm --filter @pymodel/agent-core-v2 typecheck`, `npx tsgo -p packages/agent-core-v2/tsconfig.json --noEmit`, `lint:imports`, `check-no-comments`, oxlint on the changed files — all exit 0 ## Checklist - [x] I have read the [CONTRIBUTING](https://github.com/PyModel/pythinker-code/blob/main/CONTRIBUTING.md) document. - [x] I have linked a related issue (external PRs: the issue must have a maintainer's `/approve`). - [x] I have added tests that prove my feature works. - [x] Ran `gen-changesets` skill, or this PR needs no changeset. - [x] Ran `gen-docs` skill, or this PR needs no doc update. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Fixed lingering “manually stopped” outcomes after undoing an interrupted turn. - Outcomes now clear when the associated turn is undone, while remaining intact when only later turns are undone. - Improved restoration behavior to prevent stale outcomes from reappearing. - Preserved outcome state reliably during updates and recovery. - **Tests** - Added coverage for undo, restoration, outcome clearing, and turn tracking scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 7040eed commit 0753f13

7 files changed

Lines changed: 266 additions & 21 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@pymodel/pythinker-code": patch
3+
---
4+
5+
Fix the "manually stopped" state lingering after undoing the interrupted turn.

packages/agent-core-v2/src/agent/activityView/activityViewService.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import { USER_PROMPT_ORIGIN } from '#/agent/contextMemory/types';
3434
import type { PromptOrigin } from '#/agent/contextMemory/types';
3535
import type { TurnEndReason } from '#/agent/loop/turnEvents';
3636
import { IEventDispatcher } from '#/state/eventDispatcher';
37+
import { ContextUndone } from '#/agent/undo/undoService';
3738

3839
import type {
3940
ActivityLastTurnState,
@@ -140,6 +141,9 @@ export class AgentActivityView extends Disposable implements IAgentActivityView
140141
this._register(
141142
this.eventBus.subscribe(TurnEnded, (e) => this.onTurnEnded(e.turnId, e.reason)),
142143
);
144+
this._register(
145+
this.eventBus.subscribe(ContextUndone, (e) => this.onContextUndone(e.fromTurnId)),
146+
);
143147
this._register(
144148
this.eventBus.subscribe(PermissionApprovalRequested, (e) =>
145149
this.onApprovalRequested(e.id ?? e.toolCallId, e.toolCallId),
@@ -296,6 +300,14 @@ export class AgentActivityView extends Disposable implements IAgentActivityView
296300
this.publish();
297301
}
298302

303+
private onContextUndone(fromTurnId: number | undefined): void {
304+
const last = this.lastTurn;
305+
if (last === undefined) return;
306+
if (fromTurnId !== undefined && last.turnId < fromTurnId) return;
307+
this.lastTurn = undefined;
308+
this.publish();
309+
}
310+
299311
private onStepStarted(step: number): void {
300312
this.mutateTurn((t) => {
301313
t.step = step;

packages/agent-core-v2/src/agent/loop/turnOps.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,19 @@ export const turnKey = defineState(
139139
return { ...next, anchorTurnIds: [...s.anchorTurnIds, s.nextTurnId] };
140140
})
141141
.on(TurnSteer, () => {})
142-
.on(ContextUndo, (s, e) => ({
143-
...s,
144-
anchorTurnIds: s.anchorTurnIds.slice(0, Math.max(0, s.anchorTurnIds.length - e.count)),
145-
}))
142+
.on(ContextUndo, (s, e) => {
143+
const firstRemoved = s.anchorTurnIds[s.anchorTurnIds.length - e.count];
144+
const lastEnded = s.lastEnded;
145+
return {
146+
...s,
147+
anchorTurnIds: s.anchorTurnIds.slice(0, Math.max(0, s.anchorTurnIds.length - e.count)),
148+
lastEnded:
149+
lastEnded !== undefined &&
150+
(firstRemoved === undefined || lastEnded.turnId >= firstRemoved)
151+
? undefined
152+
: lastEnded,
153+
};
154+
})
146155
.on(ContextApplyCompaction, (s) => ({ ...s, anchorTurnIds: [] }))
147156
.on(ContextClear, (s) => ({ ...s, anchorTurnIds: [] }))
148157
.on(TurnCancel, (s, e) => {

packages/agent-core-v2/src/session/sessionActivity/sessionOutcomeMirrorService.ts

Lines changed: 65 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@ import { LifecycleScope } from '#/app/scopes';
44
import { IEventBus } from '#/app/event/eventBus';
55
import { AgentActivityUpdated } from '#/agent/activityView/activityView';
66
import { TurnStarted } from '#/agent/loop/turnEvents';
7-
import { TurnEnded } from '#/agent/loop/turnOps';
7+
import { TurnEnded, turnKey } from '#/agent/loop/turnOps';
8+
import { ContextUndone } from '#/agent/undo/undoService';
9+
import { IAgentStateService } from '#/agent/state/agentState';
10+
import { IEventDispatcher } from '#/state/eventDispatcher';
811
import {
912
IAgentLifecycleService,
1013
MAIN_AGENT_ID,
@@ -18,16 +21,18 @@ export class SessionOutcomeMirror extends Disposable implements ISessionOutcomeM
1821
declare readonly _serviceBrand: undefined;
1922

2023
private lastPersisted: SessionTurnOutcome | undefined;
24+
private lastPersistedTurnId: number | undefined;
2125
private adopted = false;
2226
private turnStartedHere = false;
2327
private mainSubscription: DisposableStore | undefined;
28+
private readonly metadataReady: Promise<void>;
2429

2530
constructor(
2631
@IAgentLifecycleService private readonly agents: IAgentLifecycleService,
2732
@ISessionMetadata private readonly metadata: ISessionMetadata,
2833
) {
2934
super();
30-
void this.metadata
35+
this.metadataReady = this.metadata
3136
.read()
3237
.then((meta) => {
3338
if (!this.adopted) this.lastPersisted = meta.lastTurnReason;
@@ -52,24 +57,33 @@ export class SessionOutcomeMirror extends Disposable implements ISessionOutcomeM
5257

5358
private attachMain(): void {
5459
if (this.mainSubscription !== undefined) return;
55-
const bus = this.agents.handleOf(MAIN_AGENT_ID)?.accessor.get(IEventBus) as
56-
| IEventBus
57-
| undefined;
60+
const handle = this.agents.handleOf(MAIN_AGENT_ID);
61+
const bus = handle?.accessor.get(IEventBus) as IEventBus | undefined;
5862
if (bus === undefined) return;
5963
const subscription = new DisposableStore();
6064
this.mainSubscription = subscription;
65+
const dispatcher = handle?.accessor.get(IEventDispatcher) as IEventDispatcher | undefined;
66+
const agentStates = handle?.accessor.get(IAgentStateService) as IAgentStateService | undefined;
67+
if (dispatcher !== undefined && agentStates !== undefined) {
68+
subscription.add(
69+
dispatcher.hooks.onDidRestore.register('session-outcome-mirror', async (_ctx, next) => {
70+
await next();
71+
await this.reconcileAfterRestore(agentStates);
72+
}),
73+
);
74+
}
6175
subscription.add(
6276
bus.subscribe(TurnEnded, (event) => {
6377
if (event.reason === 'completed') {
64-
this.write('completed');
78+
this.write('completed', { turnId: event.turnId });
6579
return;
6680
}
6781
if (event.reason === 'failed' || event.reason === 'blocked') {
68-
this.write('failed');
82+
this.write('failed', { turnId: event.turnId });
6983
return;
7084
}
7185
if (event.reason === 'cancelled' && event.interruptReason === 'user_cancelled') {
72-
this.write('cancelled');
86+
this.write('cancelled', { turnId: event.turnId });
7387
}
7488
}),
7589
);
@@ -79,31 +93,67 @@ export class SessionOutcomeMirror extends Disposable implements ISessionOutcomeM
7993
this.write(undefined);
8094
}),
8195
);
96+
subscription.add(
97+
bus.subscribe(ContextUndone, (event) => {
98+
if (
99+
event.fromTurnId !== undefined &&
100+
this.lastPersistedTurnId !== undefined &&
101+
this.lastPersistedTurnId < event.fromTurnId
102+
) {
103+
return;
104+
}
105+
this.write(undefined);
106+
}),
107+
);
82108
subscription.add(
83109
bus.subscribe(AgentActivityUpdated, (event) => {
84110
if (this.turnStartedHere) return;
85111
if (this.lastPersisted !== undefined) return;
86112
const reason = event.lastTurn?.reason;
87113
if (reason === 'completed' || reason === 'cancelled') {
88-
this.write(reason, { touchUpdatedAt: false });
114+
this.write(reason, { touchUpdatedAt: false, turnId: event.lastTurn?.turnId });
89115
} else if (reason === 'failed' || reason === 'blocked') {
90-
this.write('failed', { touchUpdatedAt: false });
116+
this.write('failed', { touchUpdatedAt: false, turnId: event.lastTurn?.turnId });
91117
}
92118
}),
93119
);
94120
}
95121

122+
private async reconcileAfterRestore(agentStates: IAgentStateService): Promise<void> {
123+
await this.metadataReady;
124+
if (this.lastPersisted === undefined) return;
125+
if (this.turnStartedHere) return;
126+
if (!agentStates.has(turnKey)) return;
127+
const lastEnded = agentStates.get(turnKey).lastEnded;
128+
if (lastEnded === undefined) {
129+
this.write(undefined, { touchUpdatedAt: false });
130+
return;
131+
}
132+
if (this.lastPersistedTurnId === undefined) this.lastPersistedTurnId = lastEnded.turnId;
133+
}
134+
96135
private write(
97136
outcome: SessionTurnOutcome | undefined,
98-
opts?: { readonly touchUpdatedAt?: boolean },
137+
opts?: { readonly touchUpdatedAt?: boolean; readonly turnId?: number },
99138
): void {
100-
if (outcome === this.lastPersisted) return;
139+
if (outcome === this.lastPersisted) {
140+
if (opts?.turnId !== undefined) this.lastPersistedTurnId = opts.turnId;
141+
return;
142+
}
101143
this.adopted = true;
102144
const previous = this.lastPersisted;
145+
const previousTurnId = this.lastPersistedTurnId;
103146
this.lastPersisted = outcome;
104-
void this.metadata.update({ lastTurnReason: outcome }, opts).catch(() => {
105-
if (this.lastPersisted === outcome) this.lastPersisted = previous;
106-
});
147+
this.lastPersistedTurnId =
148+
outcome === undefined ? undefined : (opts?.turnId ?? this.lastPersistedTurnId);
149+
void this.metadata
150+
.update({ lastTurnReason: outcome }, { touchUpdatedAt: opts?.touchUpdatedAt })
151+
.catch(() => {
152+
if (this.lastPersisted === outcome) {
153+
this.lastPersisted = previous;
154+
this.lastPersistedTurnId = previousTurnId;
155+
}
156+
});
107157
}
108158
}
109159

packages/agent-core-v2/test/agent/activityView/activityView.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
} from '#/agent/toolApproval/toolApprovalService';
2727
import { IAgentFullCompactionService } from '#/agent/fullCompaction/fullCompaction';
2828
import type { FullCompactionTask } from '#/agent/fullCompaction/fullCompaction';
29+
import { ContextUndone } from '#/agent/undo/undoService';
2930
import { OrderedHookSlot } from '#/hooks';
3031
import { IEventDispatcher } from '#/state/eventDispatcher';
3132
import { stubAgentContext } from '../agentContext/stubs';
@@ -234,6 +235,37 @@ describe('AgentActivityView', () => {
234235
expect(view.state().lastTurn).toMatchObject({ turnId: 2, reason: 'completed' });
235236
});
236237

238+
it('clears the last outcome when an undo rewinds the turn it describes', () => {
239+
const { bus, view } = harness();
240+
241+
bus.publish(new TurnStarted({ agentId: 'main', turnId: 1, origin: { kind: 'user' } }));
242+
bus.publish(new TurnEnded({ agentId: 'main', turnId: 1, reason: 'cancelled' }));
243+
expect(view.state().lastTurn).toMatchObject({ turnId: 1, reason: 'cancelled' });
244+
245+
bus.publish(new ContextUndone({ agentId: 'main', turns: 1, fromTurnId: 1 }));
246+
expect(view.state().lastTurn).toBeUndefined();
247+
});
248+
249+
it('keeps the last outcome when an undo rewinds only later turns', () => {
250+
const { bus, view } = harness();
251+
252+
bus.publish(new TurnStarted({ agentId: 'main', turnId: 1, origin: { kind: 'user' } }));
253+
bus.publish(new TurnEnded({ agentId: 'main', turnId: 1, reason: 'completed' }));
254+
255+
bus.publish(new ContextUndone({ agentId: 'main', turns: 1, fromTurnId: 2 }));
256+
expect(view.state().lastTurn).toMatchObject({ turnId: 1, reason: 'completed' });
257+
});
258+
259+
it('clears the last outcome when the undo range cannot be determined', () => {
260+
const { bus, view } = harness();
261+
262+
bus.publish(new TurnStarted({ agentId: 'main', turnId: 1, origin: { kind: 'user' } }));
263+
bus.publish(new TurnEnded({ agentId: 'main', turnId: 1, reason: 'failed' }));
264+
265+
bus.publish(new ContextUndone({ agentId: 'main', turns: 1 }));
266+
expect(view.state().lastTurn).toBeUndefined();
267+
});
268+
237269
it('exposes the engine-minted interaction id as the approval id', () => {
238270
const { bus, view } = harness();
239271

packages/agent-core-v2/test/agent/loop/turnOps.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,36 @@ describe('turnKey lastEnded', () => {
6767
expect(s.lastEnded?.reason).toBe('completed');
6868
});
6969

70+
it('clears the stored outcome when an undo rewinds the turn it describes', () => {
71+
let s = turnKey.initial();
72+
s = fold(s, new TurnPrompt({ agentId: 'main', input: [], origin: { kind: 'user' } }));
73+
s = fold(s, new TurnEnded({ agentId: 'main', turnId: 0, reason: 'completed', durationMs: 10 }));
74+
s = fold(s, new TurnPrompt({ agentId: 'main', input: [], origin: { kind: 'user' } }));
75+
s = fold(s, new TurnEnded({ agentId: 'main', turnId: 1, reason: 'cancelled', durationMs: 10 }));
76+
expect(s.lastEnded?.reason).toBe('cancelled');
77+
s = fold(s, new ContextUndo({ agentId: 'main', count: 1 }));
78+
expect(s.anchorTurnIds).toEqual([0]);
79+
expect(s.lastEnded).toBeUndefined();
80+
});
81+
82+
it('keeps the stored outcome when an undo rewinds only later turns', () => {
83+
let s = turnKey.initial();
84+
s = fold(s, new TurnPrompt({ agentId: 'main', input: [], origin: { kind: 'user' } }));
85+
s = fold(s, new TurnEnded({ agentId: 'main', turnId: 0, reason: 'completed', durationMs: 10 }));
86+
s = fold(s, new TurnPrompt({ agentId: 'main', input: [], origin: { kind: 'user' } }));
87+
s = fold(s, new ContextUndo({ agentId: 'main', count: 1 }));
88+
expect(s.lastEnded).toMatchObject({ turnId: 0, reason: 'completed' });
89+
});
90+
91+
it('clears the stored outcome when the undo count exceeds the tracked anchors', () => {
92+
let s = turnKey.initial();
93+
s = fold(s, new TurnPrompt({ agentId: 'main', input: [], origin: { kind: 'user' } }));
94+
s = fold(s, new TurnEnded({ agentId: 'main', turnId: 0, reason: 'cancelled', durationMs: 10 }));
95+
s = fold(s, new ContextUndo({ agentId: 'main', count: 2 }));
96+
expect(s.anchorTurnIds).toEqual([]);
97+
expect(s.lastEnded).toBeUndefined();
98+
});
99+
70100
it('starts without a stored outcome', () => {
71101
expect(turnKey.initial().lastEnded).toBeUndefined();
72102
});

0 commit comments

Comments
 (0)