fix pre-step lifecycle regressions
This commit is contained in:
@@ -2,5 +2,5 @@
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write packages/plan/plan-mode/README.md
|
||||
README.md: c23dd0c42d8b1d1975b30b08e5be8758a325a9c7
|
||||
README.zh.md: 88640381d736244ad56aa802cee6ed70c6c75c20
|
||||
README.md: cbd0f5a042a4ce67e10eb990189ad87329aaf9fb
|
||||
README.zh.md: b0fc2709e4665814fc10e950f39c62feec17c6b0
|
||||
|
||||
@@ -8,7 +8,7 @@ Logged, per-agent plan collaboration state with deployment-owned guidance, direc
|
||||
|
||||
`plan/mode` (`{ active: boolean }`) is a log-only, whole-value-replace `SessionEventMap` member. `foldPlanMode(events)` returns the last logged value or `false`, so resume, fork, and compaction recover plan state directly from the session log. UIs observe committed flips through `session/event`.
|
||||
|
||||
`ctx.planMode.set(agent, active)` commits immediately when the agent is idle — no boundary would arrive until the next prompt, so the standalone `plan/mode` event lands at once — and holds a pending selection for the next in-turn request boundary while the agent is running; it returns which of the two happened (`committed`/`queued`), a `cancelled` reversal, or a `noop`. `get(agent)` returns `{ active, pending? }`, separating the logged state shaping the current step from a user's mid-turn selection. Initial and continuation pre-step boundaries plus request-recovery retries are covered; a changed user selection contributes one plugin-sourced `user/message` notice when the last logged request header described the other state (both commit paths).
|
||||
`ctx.planMode.set(agent, active)` commits immediately when the agent is idle — no boundary would arrive until the next prompt, so the standalone `plan/mode` event lands at once — and holds a pending selection for the next accepted in-turn pre-step while the agent is running; it returns which of the two happened (`committed`/`queued`), a `cancelled` reversal, or a `noop`. `get(agent)` returns `{ active, pending? }`, separating the logged state shaping the current step from a user's mid-turn selection. Initial and continuation pre-step boundaries are covered; a same-step request-recovery retry reuses its frozen assembly and leaves the selection pending for the next pre-step. A changed user selection contributes one plugin-sourced `user/message` notice when the last logged request header described the other state (both commit paths).
|
||||
|
||||
## Model and human surfaces
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
|
||||
`plan/mode`(`{ active: boolean }`)是一个仅写日志、整值替换的 `SessionEventMap` 成员。`foldPlanMode(events)` 返回最后记录的值,如果没有则返回 `false`,因此恢复、fork 和压缩(compaction)都能直接从会话日志恢复 plan 状态。UI 通过 `session/event` 观察已提交的切换。
|
||||
|
||||
`ctx.planMode.set(agent, active)` 在 agent 空闲时立即提交——下一个 prompt 之前不会有任何边界到来,因此独立的 `plan/mode` 事件当场落账——在 agent 运行中则持有待生效选择、等下一个轮内请求边界;返回值说明发生了哪种(`committed`/`queued`)、一次 `cancelled` 反转或 `noop`。`get(agent)` 返回 `{ active, pending? }`,将塑造当前步骤的日志状态与用户的轮中选择分开。初始与续步 pre-step 边界以及请求恢复重试都在覆盖范围内;当最后记录的请求头描述了另一状态时,用户选择的变更会贡献一条插件来源的 `user/message` 通知(两条提交路径皆然)。
|
||||
`ctx.planMode.set(agent, active)` 在 agent 空闲时立即提交——下一个 prompt 之前不会有任何边界到来,因此独立的 `plan/mode` 事件当场落账——在 agent 运行中则持有待生效选择、等下一个被接受的轮内 pre-step;返回值说明发生了哪种(`committed`/`queued`)、一次 `cancelled` 反转或 `noop`。`get(agent)` 返回 `{ active, pending? }`,将塑造当前步骤的日志状态与用户的轮中选择分开。初始与续步 pre-step 边界都在覆盖范围内;同一步骤的请求恢复重试会复用已冻结的 assembly,并将该选择保留到下一个 pre-step。当最后记录的请求头描述了另一状态时,用户选择的变更会贡献一条插件来源的 `user/message` 通知(两条提交路径皆然)。
|
||||
|
||||
## 模型与人类界面
|
||||
|
||||
|
||||
@@ -8,9 +8,9 @@
|
||||
*
|
||||
* The state in force is folded from the session log (`plan/mode`, last one
|
||||
* wins), so resume and fork restore it without a live mirror. User selections
|
||||
* are held as pending intent until an in-turn step boundary because every
|
||||
* session event is turn-enclosed. The service flushes on `step/start` before
|
||||
* the affected request assembly, including retry turns.
|
||||
* are held as pending intent until an in-turn step boundary. The service
|
||||
* flushes from `agent/pre-step` before the affected request assembly;
|
||||
* same-step request retries reuse their assembly.
|
||||
*
|
||||
* The exit tool remains registered while plan mode is inactive so crossing a
|
||||
* boundary changes only the prompt section, not the request tool catalog.
|
||||
@@ -196,34 +196,31 @@ export class PlanModeService extends Service {
|
||||
super(ctx, 'planMode')
|
||||
this.section = resolveConfig(config).section
|
||||
let disposed = false
|
||||
|
||||
// Pre-step runs before the turn opens, so the turn-enclosed mode event
|
||||
// commits from the immediately following step/start observer. Request
|
||||
// assembly happens afterward. A failed append remains pending for a later
|
||||
// boundary, and policy cannot block the turn.
|
||||
ctx.on('session/event', (session, event) => {
|
||||
if (disposed || event.type !== 'step/start') return
|
||||
try {
|
||||
this.onBoundary(session)
|
||||
} catch (error) {
|
||||
ctx.logger.warn('dsh-plan-mode: boundary flush failed: %o', error)
|
||||
}
|
||||
}, { prepend: true })
|
||||
// Pre-step is outside Session.append publication, so its log-only mode
|
||||
// event can land between turns or inside an open turn without re-entering
|
||||
// the session. A failed append remains pending for a later boundary, and
|
||||
// policy cannot block the step.
|
||||
ctx.on('agent/pre-step', async (
|
||||
agent,
|
||||
_messages,
|
||||
_signal,
|
||||
{ signal },
|
||||
next,
|
||||
): Promise<PreStepDecision> => {
|
||||
const decision = await next()
|
||||
const pending = this.pendingIntents.get(agent.session)
|
||||
if (decision.kind === 'reject' || pending?.narrate !== true) return decision
|
||||
if (decision.kind === 'reject' || signal.aborted || pending === undefined) return decision
|
||||
const narration = this.narration(agent.session, pending.active)
|
||||
return narration === undefined
|
||||
try {
|
||||
this.onBoundary(agent.session)
|
||||
} catch (error) {
|
||||
ctx.logger.warn('dsh-plan-mode: boundary flush failed: %o', error)
|
||||
return decision
|
||||
}
|
||||
return !pending.narrate || narration === undefined
|
||||
? decision
|
||||
: { ...decision, messages: [...decision.messages, narration] }
|
||||
})
|
||||
ctx.effect(() => () => { disposed = true }, 'dsh-plan-mode: close boundary lifetime')
|
||||
ctx.effect(() => () => { disposed = true }, 'dsh-plan-mode: close service lifetime')
|
||||
|
||||
ctx.systemPrompt.section({
|
||||
name: 'plan:policy',
|
||||
|
||||
@@ -13,7 +13,7 @@ const PLAN_CONFIG = { section: 'Test plan mode instructions.' }
|
||||
|
||||
/**
|
||||
* Full-loop integration: a scripted mock model drives the REAL plan-mode plugin
|
||||
* through the agent loop — the pending-intent flush at the request boundary, the
|
||||
* through the agent loop — the pending-intent flush at the step boundary, the
|
||||
* assembly the soft layer shapes (the exit tool + mode section), and the
|
||||
* `request/header` snapshots every transition leaves.
|
||||
* Only the model is mocked; the loop, the session log, and the plugin are
|
||||
@@ -127,12 +127,16 @@ describe('plan mode through the agent loop', () => {
|
||||
expect(second.data.header.system).toContain('plan mode')
|
||||
})
|
||||
|
||||
it('a mode flip at error settlement shapes the retry before its assembly', async () => {
|
||||
it('a mode flip at error settlement waits until the step after a same-step retry', async () => {
|
||||
const failedRequest = [{
|
||||
type: 'finish',
|
||||
reason: { kind: 'error', failure: { message: 'temporarily unavailable', code: 'SERVER', status: 503 } },
|
||||
}] satisfies StreamChunk[]
|
||||
const adapter = new MockAdapter([failedRequest, textResponse('Recovered in plan mode.')])
|
||||
const adapter = new MockAdapter([
|
||||
failedRequest,
|
||||
textResponse('Recovered with the original step assembly.'),
|
||||
textResponse('Entered plan mode on the next step.'),
|
||||
])
|
||||
const ctx = await harness(adapter)
|
||||
const agent = ctx.agentLoop.create(SessionId('it-plan-retry-flip'), { provider: 'mock', model: 'mock' })
|
||||
ctx.on('agent/request-error', async (subject, _context, _signal, next) => {
|
||||
@@ -147,16 +151,26 @@ describe('plan mode through the agent loop', () => {
|
||||
|
||||
expect(adapter.requests).toHaveLength(2)
|
||||
expect(adapter.requests[0]?.system).not.toContain(PLAN_CONFIG.section)
|
||||
expect(adapter.requests[1]?.system).toContain(PLAN_CONFIG.section)
|
||||
expect(adapter.requests[1]?.system).not.toContain(PLAN_CONFIG.section)
|
||||
expect(adapter.requests[1]?.tools).toEqual(adapter.requests[0]?.tools)
|
||||
expect(ctx.planMode.get(agent)).toEqual({ active: false, pending: true })
|
||||
expect(agent.session.events.some(event => event.type === 'plan/mode')).toBe(false)
|
||||
|
||||
const nextIdle = waitForIdle(ctx, agent)
|
||||
agent.followup(createUserMessage({ content: [{ type: 'text', text: 'continue with the plan' }], source: { kind: 'user' } }))
|
||||
await nextIdle
|
||||
|
||||
expect(adapter.requests).toHaveLength(3)
|
||||
expect(adapter.requests[2]?.system).toContain(PLAN_CONFIG.section)
|
||||
expect(adapter.requests[2]?.tools).toEqual(adapter.requests[0]?.tools)
|
||||
const log = agent.session.events
|
||||
const planMode = findEvent(log, 'plan/mode')
|
||||
const firstEnd = log.find(event => event.type === 'step/end'
|
||||
&& event.data.turn === 1 && event.data.step === 1)
|
||||
const retryStart = log.find(event => event.type === 'step/start'
|
||||
const nextStart = log.find(event => event.type === 'step/start'
|
||||
&& event.data.turn === 2 && event.data.step === 1)
|
||||
expect(firstEnd?.seq).toBeLessThan(planMode.seq)
|
||||
expect(planMode.seq).toBeLessThan(retryStart?.seq ?? 0)
|
||||
expect(planMode.seq).toBeLessThan(nextStart?.seq ?? 0)
|
||||
expect(findEvent(log, 'request/header', 'last').data.header.system).toContain(PLAN_CONFIG.section)
|
||||
const notice = log.find(event => event.type === 'user/message' && event.data.source.kind === 'plugin')
|
||||
expect(notice?.type === 'user/message' && notice.data.content).toEqual([
|
||||
|
||||
@@ -249,22 +249,17 @@ describe('ctx.planMode: get/set', () => {
|
||||
})
|
||||
|
||||
describe('the boundary flush', () => {
|
||||
it('does not flush during pre-step and commits from the following step/start', async () => {
|
||||
it('flushes from pre-step before the following step/start', async () => {
|
||||
const ctx = await setup()
|
||||
const agent = await agentWithSession(ctx)
|
||||
openTurn(agent.session)
|
||||
ctx.planMode.set(agent, true)
|
||||
// Pre-step only composes narration. The pending intent survives until the
|
||||
// turn-enclosed step/start event commits it before request assembly.
|
||||
await boundary(ctx, agent, 'pre-step')
|
||||
expect(agent.session.events.some(event => event.type === 'plan/mode')).toBe(false)
|
||||
expect(ctx.planMode.get(agent)).toEqual({ active: false, pending: true })
|
||||
await boundary(ctx, agent, 'step-start')
|
||||
expect(foldPlanMode(agent.session.events)).toBe(true)
|
||||
expect(ctx.planMode.get(agent)).toEqual({ active: true })
|
||||
})
|
||||
|
||||
it('removes the step/start flush when the plugin fiber is disposed', async () => {
|
||||
it('removes the pre-step flush when the plugin fiber is disposed', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry)
|
||||
@@ -273,8 +268,7 @@ describe('the boundary flush', () => {
|
||||
openTurn(agent.session)
|
||||
ctx.planMode.set(agent, true)
|
||||
await fiber.dispose()
|
||||
const event = agent.session.append('step/start', { turn: 1, step: 1 })
|
||||
ctx.emit('session/event', agent.session, event)
|
||||
await boundary(ctx, agent, 'pre-step')
|
||||
expect(agent.session.events.some(event => event.type === 'plan/mode')).toBe(false)
|
||||
})
|
||||
|
||||
@@ -366,7 +360,7 @@ describe('the boundary flush', () => {
|
||||
expect(ctx.planMode.get(agent).pending).toBeUndefined()
|
||||
})
|
||||
|
||||
it('pre-step never appends, so a broken backend surfaces only at step/start', async () => {
|
||||
it('contains a pre-step append failure and keeps the intent pending', async () => {
|
||||
const ctx = await setup()
|
||||
const warn = vi.fn()
|
||||
ctx.logger.warn = warn as never
|
||||
@@ -379,8 +373,6 @@ describe('the boundary flush', () => {
|
||||
return (original as (...args: unknown[]) => unknown)(type, ...rest)
|
||||
}) as unknown) as typeof agent.session.append
|
||||
await boundary(ctx, agent, 'pre-step')
|
||||
expect(warn).not.toHaveBeenCalled()
|
||||
await boundary(ctx, agent, 'step-start')
|
||||
expect(warn).toHaveBeenCalledOnce()
|
||||
expect(ctx.planMode.get(agent)).toEqual({ active: false, pending: true })
|
||||
})
|
||||
@@ -602,7 +594,7 @@ describe('/plan', () => {
|
||||
.toEqual({ kind: 'success', text: 'Plan mode entry cancelled.' })
|
||||
expect(ctx.planMode.get(entering)).toEqual({ active: false, pending: false })
|
||||
expect(enteringSteer).not.toHaveBeenCalled()
|
||||
await boundary(ctx, entering, 'step/end')
|
||||
await boundary(ctx, entering, 'step-start')
|
||||
expect(ctx.planMode.get(entering)).toEqual({ active: false })
|
||||
expect(entering.session.events.some(event => event.type === 'plan/mode')).toBe(false)
|
||||
|
||||
@@ -616,7 +608,7 @@ describe('/plan', () => {
|
||||
expect((await ctx.commands.execute(active, '/plan off', signal))?.result)
|
||||
.toEqual({ kind: 'success', text: 'Leaving plan mode (applies from the next step).' })
|
||||
expect(activeSteer).not.toHaveBeenCalled()
|
||||
await boundary(ctx, active, 'step/end')
|
||||
await boundary(ctx, active, 'step-start')
|
||||
expect(ctx.planMode.get(active)).toEqual({ active: false })
|
||||
})
|
||||
|
||||
|
||||
Reference in New Issue
Block a user