From 7d697315f67f1f71cc41558b846c1be08755afb5 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Mon, 20 Jul 2026 22:52:24 +0800 Subject: [PATCH] fix(mode): apply mode flips before request retries --- docs/event-producer-consumer.md | 2 +- packages/mode/mode/src/index.ts | 55 ++++++++++++++++-------- packages/mode/mode/tests/mode.spec.ts | 62 ++++++++++++++++++++++++--- 3 files changed, 93 insertions(+), 26 deletions(-) diff --git a/docs/event-producer-consumer.md b/docs/event-producer-consumer.md index 03c78f6f68..ec6c5df2ef 100644 --- a/docs/event-producer-consumer.md +++ b/docs/event-producer-consumer.md @@ -16,7 +16,7 @@ This matrix shows which packages dispatch each harness-owned event and which pac | `agent/prompt-submit` | `waterfall` | [`packages/core/agent/src/types.ts:217`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | [`acp`](../packages/ui/acp), [`hooks-claude`](../packages/hooks/hooks-claude), [`hooks-codex`](../packages/hooks/hooks-codex), [`mode`](../packages/mode/mode), [`repeat-tool-guard`](../packages/guard/repeat-tool-guard) | | `agent/queued` | `emit` | [`packages/core/agent/src/types.ts:178`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | - | | `agent/request` | `waterfall` | [`packages/core/agent/src/types.ts:229`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | [`acp`](../packages/ui/acp) | -| `agent/request-error` | `waterfall` | [`packages/core/agent/src/types.ts:282`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | [`compact-basic`](../packages/compact/compact-basic), [`llm-retry`](../packages/llm/llm-retry) | +| `agent/request-error` | `waterfall` | [`packages/core/agent/src/types.ts:282`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | [`compact-basic`](../packages/compact/compact-basic), [`llm-retry`](../packages/llm/llm-retry), [`mode`](../packages/mode/mode) | | `agent/session-prefix` | `waterfall` | [`packages/core/agent/src/types.ts:244`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | [`tool-skill`](../packages/skill/tool-skill), [`workspace-context`](../packages/context/workspace-context) | | `agent/session-start` | `emit` | [`packages/core/agent/src/types.ts:191`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | [`hooks-claude`](../packages/hooks/hooks-claude), [`hooks-codex`](../packages/hooks/hooks-codex) | | `agent/status` | `emit` | [`packages/core/agent/src/types.ts:168`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | [`invariants`](../packages/support/invariants), [`tui`](../packages/ui/tui) | diff --git a/packages/mode/mode/src/index.ts b/packages/mode/mode/src/index.ts index 9a51e83397..ae1f8e3bc3 100644 --- a/packages/mode/mode/src/index.ts +++ b/packages/mode/mode/src/index.ts @@ -18,14 +18,14 @@ * * User flips go through {@link ModesService.set}: every session event is * turn-enclosed and an idle agent has no open turn, so `set()` records a - * pending intent and the service flushes it on the loop's interception seams - * — `agent/prompt-submit` (inside the just-opened turn, before its first - * assembly) and `agent/turn-continuation` (after each step closed, before the - * next assembly) — both outside the step's tool-execution window and outside - * any log emit (post-commit `session/event` observers are observe-only and - * cannot append). A flush that changes what the last logged request header - * told the model appends one coalesced `context/message` notice in the same - * frame. + * pending intent and the service flushes it on the loop's interception seams: + * `agent/prompt-submit` before the first assembly, `agent/turn-continuation` + * before a normal successor step, and the post-composed + * `agent/request-error` retry decision before a recovery step. These seams are + * outside tool execution and log publication (post-commit `session/event` + * observers cannot append). A flush that changes what the last logged request + * header told the model appends one coalesced `context/message` notice in the + * same frame. * * Agent Note: .agents/notes/implemented/feature/2026-07-07-plan-mode.md * @@ -238,14 +238,14 @@ export class ModesService extends Service { // Boundary flushes ride the loop's interception seams, NOT the // `session/event` feed: post-commit session observers are observe-only // (an append from one would re-enter the publishing append and be - // contained away), while these two waterfalls fire OUTSIDE any log emit - // and bracket exactly the boundaries the flush wants — prompt-submit - // inside the just-opened turn before its first assembly, and - // turn-continuation after each step closed before the next assembly (or - // the turn's end), so a flushed mode always lands before the prompt that - // should reflect it. Contained: a policy plugin must never block a - // prompt or a turn; the only throw path in onBoundary is session.append - // rejecting mid-teardown. + // contained away). Prompt-submit runs before the first assembly; + // turn-continuation runs after an ordinary step and before its successor. + // Request retries bypass turn-continuation, so the prepended request-error + // wrapper delegates through recovery (including async backoff), then + // flushes a retry decision before that waterfall returns to the loop. A + // flushed mode therefore lands before the prompt that should reflect it. + // Contained: policy must never block a prompt or turn; onBoundary can throw + // only when session.append rejects during teardown. ctx.on('agent/prompt-submit', (agent, _content, _source, next) => { try { this.onBoundary(agent.session, true) @@ -262,6 +262,25 @@ export class ModesService extends Service { } return next() }) + ctx.on('agent/request-error', async ( + agent, + _turn, + _step, + _error, + _failure, + _priorFailures, + _signal, + next, + ) => { + const decision = await next() + if (decision.action !== 'retry') return decision + try { + this.onBoundary(agent.session, false) + } catch (error) { + ctx.logger.warn('dsh-mode: boundary flush failed: %o', error) + } + return decision + }, { prepend: true }) ctx.on('agent/created', (agent) => { const seed = agent.options.mode @@ -395,8 +414,8 @@ export class ModesService extends Service { } /** - * One boundary pass (`turnStart` = a prompt-submit flush, else a - * turn-continuation flush): narrate a folded mode the config dropped (once + * One boundary pass (`turnStart` = prompt-submit, else a normal-successor or + * recovery-retry boundary): narrate a folded mode the config dropped (once * per name, turn starts only), then flush the pending intent — append the * `mode/set` (skipped when the fold already matches: a net-zero flip * sequence) and the one coalesced notice when the flushed mode differs from diff --git a/packages/mode/mode/tests/mode.spec.ts b/packages/mode/mode/tests/mode.spec.ts index 565f4e88b0..0301e30e63 100644 --- a/packages/mode/mode/tests/mode.spec.ts +++ b/packages/mode/mode/tests/mode.spec.ts @@ -4,7 +4,7 @@ import { CallId } from '@deepseek-ai/dsh-llm' import SystemPrompt from '@deepseek-ai/dsh-system-prompt' import ToolRegistry, { RUN_CODE_NAME, defineTool } from '@deepseek-ai/dsh-tools' import { Session, SessionId } from '@deepseek-ai/dsh-session' -import { agentEvents, type Agent } from '@deepseek-ai/dsh-agent' +import { agentEvents, type Agent, type RequestErrorDecision } from '@deepseek-ai/dsh-agent' import UserInteractionService, { type AskUserQuestionRequest } from '@deepseek-ai/dsh-user-interaction' import { CodeRuntime, type CodeRunRequest, type CodeRunResult } from '@deepseek-ai/dsh-code-runtime' import ModesService, { DEFAULT_MODE, EXIT_PLAN_MODE, PLAN_MODE, foldMode, resolveConfig } from '../src/index.ts' @@ -17,9 +17,9 @@ const PLAN_CONFIG = { modes: { plan: { section: TEST_PLAN_SECTION } } } satisfie * Drives the REAL plugin: mounts `dsh-mode` beside real `SystemPrompt` and * `ToolRegistry` services, with fake Agents carrying real `Session`s (the * tool-todo test shape). Turn boundaries are simulated by appending the real - * boundary events and dispatching the interception seams the loop fires there - * (`agent/prompt-submit` / `agent/turn-continuation`) — exactly the seams the - * flush rides in production. + * boundary events and dispatching the ordinary interception seams the loop + * fires there (`agent/prompt-submit` / `agent/turn-continuation`). Recovery + * retry coverage lives in the full-loop integration suite. */ function agentWithSession(id = 'agent-1', options: { mode?: string } = {}): Agent & { session: Session } { @@ -38,8 +38,9 @@ async function setup(config: ModeConfig = PLAN_CONFIG): Promise { /** * Append a boundary event and dispatch the interception seam the loop fires * there — `agent/prompt-submit` inside the just-opened turn, - * `agent/turn-continuation` after the step closed — exactly the seams the - * flush rides (post-commit `session/event` observers are observe-only). + * `agent/turn-continuation` after the step closed. Recovery retries use the + * separately covered `agent/request-error` wrapper; post-commit + * `session/event` observers remain observe-only. */ async function boundary(ctx: Context, agent: Agent & { session: Session }, type: 'turn/start' | 'step/end'): Promise { const events = agentEvents(ctx, agent) @@ -52,6 +53,24 @@ async function boundary(ctx: Context, agent: Agent & { session: Session }, type: await events.waterfall('agent/turn-continuation', 1, { action: 'stop' }, () => Promise.resolve({ action: 'stop' })) } +/** Dispatch the closed-step recovery seam with one terminal decision. */ +function recoveryBoundary( + ctx: Context, + agent: Agent & { session: Session }, + decision: RequestErrorDecision, +): Promise { + return agentEvents(ctx, agent).waterfall( + 'agent/request-error', + 1, + 1, + new Error('request failed'), + { message: 'request failed', code: 'SERVER' }, + [], + new AbortController().signal, + () => Promise.resolve(decision), + ) +} + /** Append a minimal `request/header` snapshot so the log has a "what the model was told" anchor. */ function header(session: Session): void { session.append('request/header', { header: { config: { provider: 'test', model: 'test-model' } }, reason: 'initial' }) @@ -209,6 +228,31 @@ describe('the boundary flush', () => { expect(foldMode(agent.session.events)).toBe(PLAN_MODE) }) + it('keeps the pending intent parked when recovery does not retry', async () => { + const ctx = await setup() + const agent = agentWithSession() + ctx.modes.set(agent, PLAN_MODE) + expect(await recoveryBoundary(ctx, agent, { action: 'fail' })).toEqual({ action: 'fail' }) + expect(ctx.modes.get(agent)).toEqual({ current: DEFAULT_MODE, pending: PLAN_MODE }) + }) + + it('contains an append failure at the retry boundary without changing its decision', async () => { + const ctx = await setup() + const warn = vi.fn() + ctx.logger.warn = warn as never + const agent = agentWithSession() + ctx.modes.set(agent, PLAN_MODE) + const original = agent.session.append.bind(agent.session) + agent.session.append = (((type: string, ...rest: unknown[]) => { + if (type === 'mode/set') throw new Error('backend gone') + return (original as (...args: unknown[]) => unknown)(type, ...rest) + }) as unknown) as typeof agent.session.append + + expect(await recoveryBoundary(ctx, agent, { action: 'retry' })).toEqual({ action: 'retry' }) + expect(warn).toHaveBeenCalledOnce() + expect(ctx.modes.get(agent)).toEqual({ current: DEFAULT_MODE, pending: PLAN_MODE }) + }) + it('nets out a flip sequence that returns to the folded mode (no append, no notice)', async () => { const ctx = await setup() const agent = agentWithSession() @@ -830,11 +874,13 @@ describe('exit_plan_mode', () => { }) describe('HMR disposal', () => { - it('unregisters the service, prompt section, and stable exit tool with the plugin fiber', async () => { + it('unregisters the service, listeners, prompt section, and stable exit tool with the plugin fiber', async () => { const ctx = new Context() await ctx.plugin(SystemPrompt) await ctx.plugin(ToolRegistry) const fiber = await ctx.plugin(ModesService, PLAN_CONFIG) + const agent = agentWithSession('disposed-recovery') + ctx.modes.set(agent, PLAN_MODE) expect(ctx.get('modes')).toBeInstanceOf(ModesService) expect(ctx.tools.get(EXIT_PLAN_MODE)).toBeDefined() expect((await ctx.systemPrompt.assemble()).sections.map(section => section.name)).toContain('mode:policy') @@ -843,5 +889,7 @@ describe('HMR disposal', () => { expect(ctx.get('modes')).toBeUndefined() expect(ctx.tools.get(EXIT_PLAN_MODE)).toBeUndefined() expect((await ctx.systemPrompt.assemble()).sections.map(section => section.name)).not.toContain('mode:policy') + expect(await recoveryBoundary(ctx, agent, { action: 'retry' })).toEqual({ action: 'retry' }) + expect(agent.session.events.some(event => event.type === 'mode/set')).toBe(false) }) })