mirror of
https://github.com/deepseek-ai/deepseek-harness
synced 2026-08-15 21:04:50 +00:00
fix(mode): a failed flush re-parks the pending intent instead of dropping it
Review hardening (the finding's ordering premise did not hold — see the PR thread — but its failure-path kernel did): onBoundary cleared the pending intent BEFORE appending the mode/set, so a backend rejecting that one write lost the switch forever while the picker kept showing it optimistically. The intent is now cleared only after the append lands; a failed flush stays parked and the next healthy boundary converges the log with the picker. The containment test extends to pin the re-park and the retry. The bridge's re-notify keeps deriving from the logged event's value — now documented in place: the service holds ONE coalesced pending slot (every flush reads the latest selection, so a stale flush cannot exist), and for any other writer the logged value is the truth the picker should track, in log order.
This commit is contained in:
@@ -398,10 +398,17 @@ export class ModesService extends Service {
|
||||
if (turnStart) this.noticeDroppedDefinition(session)
|
||||
const pending = this.pendingIntents.get(session)
|
||||
if (pending === undefined) return
|
||||
this.pendingIntents.delete(session)
|
||||
const target = pending.mode
|
||||
if (target === foldMode(session.events)) return
|
||||
if (target === foldMode(session.events)) {
|
||||
this.pendingIntents.delete(session)
|
||||
return
|
||||
}
|
||||
session.append('mode/set', { mode: target })
|
||||
// Clear the intent only AFTER the append landed: if a backend rejects the
|
||||
// write, the intent stays parked and the next boundary retries — the UI's
|
||||
// optimistic picker state and the log re-converge instead of diverging
|
||||
// forever on a swallowed one-shot.
|
||||
this.pendingIntents.delete(session)
|
||||
if (!pending.narrate) return
|
||||
const told = modeAtLastHeader(session.events)
|
||||
if (told === undefined || told === target) return
|
||||
|
||||
@@ -278,6 +278,14 @@ describe('the boundary flush', () => {
|
||||
agent.session.append = (() => { throw new Error('backend gone') })
|
||||
ctx.emit('session/event', agent.session, event as SessionEvent)
|
||||
expect(warn).toHaveBeenCalledOnce()
|
||||
// The failed flush re-parks the intent (cleared only after a landed
|
||||
// append), so the next healthy boundary converges the log with the
|
||||
// picker's optimistic state instead of dropping the switch forever.
|
||||
expect(ctx.modes.get(agent)).toEqual({ current: DEFAULT_MODE, pending: PLAN_MODE })
|
||||
agent.session.append = original
|
||||
boundary(ctx, agent.session, 'step/end')
|
||||
expect(foldMode(agent.session.events)).toBe(PLAN_MODE)
|
||||
expect(ctx.modes.get(agent).pending).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -511,6 +511,11 @@ export function apply(ctx: Context, config: AcpConfig): void {
|
||||
enabled: rec.terminalEnabled,
|
||||
cwd: session.header.cwd,
|
||||
}, { includeUserMessages: false })
|
||||
// Re-notify from the EVENT's value, not from modes.get(): the service
|
||||
// holds one coalesced pending slot (every flush reads the latest
|
||||
// selection, so a flush can never be stale against the picker), and for
|
||||
// any other writer — the exit tool, a test, a foreign plugin — the logged
|
||||
// value IS the truth the picker should track, in log order.
|
||||
if (event.type === 'mode/set' && event.data.mode !== rec.lastModeId) {
|
||||
rec.lastModeId = event.data.mode
|
||||
notify({ sessionId: rec.sessionId, update: { sessionUpdate: 'current_mode_update', currentModeId: event.data.mode } })
|
||||
|
||||
Reference in New Issue
Block a user