From 970b4322272e566be65504c5e799678f0608f1f4 Mon Sep 17 00:00:00 2001 From: imccyu <276526105+imccyu@users.noreply.github.com> Date: Tue, 28 Jul 2026 23:09:15 +0800 Subject: [PATCH] fix(plan): commit an idle selection immediately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit set() on an idle agent appends plan/mode at once — no request boundary would arrive until the next prompt, so a queued intent used to hang as pending forever (the composer showed a dead pending target). A running agent keeps the boundary-flush path unchanged. set() now reports which branch ran (committed/queued/cancelled/noop); the /plan handler's copy follows the branch (idle: "Plan mode on/off", mid-turn: the next-step wording), and both commit paths share the header-delta narration. The invariant drops turn enclosure: plan/mode is a standalone whole-value event (the synthetic log-only turns removal already established the between-turns append shape). The fixture mirrors the idle commit. --- .../client/connection/src/client/fixture.ts | 17 ++++- packages/plan/plan-mode/README.i18n.yaml | 4 +- packages/plan/plan-mode/README.md | 2 +- packages/plan/plan-mode/README.zh.md | 2 +- packages/plan/plan-mode/src/index.ts | 65 ++++++++++++++----- packages/plan/plan-mode/src/invariant.ts | 35 ++++------ .../plan/plan-mode/tests/invariant.spec.ts | 8 +-- .../plan/plan-mode/tests/plan-mode.spec.ts | 49 ++++++++++++-- 8 files changed, 123 insertions(+), 59 deletions(-) diff --git a/packages/client/connection/src/client/fixture.ts b/packages/client/connection/src/client/fixture.ts index e27f6afa71..9b2af70169 100644 --- a/packages/client/connection/src/client/fixture.ts +++ b/packages/client/connection/src/client/fixture.ts @@ -998,19 +998,30 @@ export function createFixtureApi(options: FixtureOptions = {}): ApiProxy { const match = /^\/(\S+)((?:\s.*)?)$/.exec(request.payload.line.trim()) const name = match?.[1] const args = match?.[2] ?? '' + // Host parallel: /plan on an idle fixture session commits plan/mode + // immediately (the boundary flush covers only a running turn), so the + // outcome copy matches the immediate branch of the host handler. + const running = summaryOf(id)?.running === true const outcomes: Record = { compact: 'fixture：已压缩（假动作）', echo: args.trim(), 'goal-fixture': `fixture：goal 已设置（${id}）`, - // Mirrors the host handler's wording; the projection frame carries the state. plan: args.trim() === 'off' - ? 'Leaving plan mode (applies from the next step).' - : 'Entering plan mode (applies from the next step). Use /plan off to leave.', + ? (running ? 'Leaving plan mode (applies from the next step).' : 'Plan mode off.') + : (running + ? 'Entering plan mode (applies from the next step). Use /plan off to leave.' + : 'Plan mode on. Use /plan off to leave.'), } const text = name === undefined ? undefined : outcomes[name] if (name === undefined || text === undefined) return ok(request, { matched: false as const }) const commandId = `fx-cmd-${logOf(id).length}` as CommandId append(id, { type: 'command/run', data: { commandId, name, args, source: { kind: 'user' } } }) + if (name === 'plan' && !running) { + const plan = foldPlan(logOf(id)) + if (plan.wanted !== null && plan.wanted !== plan.active) { + append(id, { type: 'plan/mode', data: { active: plan.wanted } }) + } + } append(id, { type: 'command/done', data: { commandId, kind: 'success', ...text === '' ? {} : { text } } }) return ok(request, { matched: true as const, commandId }) }, diff --git a/packages/plan/plan-mode/README.i18n.yaml b/packages/plan/plan-mode/README.i18n.yaml index 230cf71562..c5a13bee7e 100644 --- a/packages/plan/plan-mode/README.i18n.yaml +++ b/packages/plan/plan-mode/README.i18n.yaml @@ -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: 33b3d1eacac68a5f0c4a248e4a5dc250bd1bd5e9 -README.zh.md: 28e4d2240c881cf44d340066dbc99c891eeb7b0c +README.md: d3c2c14fe616e1c9b4e33b716570b084db6474cf +README.zh.md: 6d6878c4b0300a716ad16be60fd86bc79f1514ba diff --git a/packages/plan/plan-mode/README.md b/packages/plan/plan-mode/README.md index 33b3d1eaca..d3c2c14fe6 100644 --- a/packages/plan/plan-mode/README.md +++ b/packages/plan/plan-mode/README.md @@ -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)` records a pending selection and flushes it inside the next turn boundary. `get(agent)` returns `{ active, pending? }`, separating the logged state shaping the current step from a user's optimistic selection. Prompt submission, ordinary continuation, and request-recovery retry are all covered; a changed user selection contributes one plugin-sourced `user/message` notice when the last logged request header described the other state. +`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. Prompt submission, ordinary continuation, and request-recovery retry are all 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). ## Model and human surfaces diff --git a/packages/plan/plan-mode/README.zh.md b/packages/plan/plan-mode/README.zh.md index 28e4d2240c..6d6878c4b0 100644 --- a/packages/plan/plan-mode/README.zh.md +++ b/packages/plan/plan-mode/README.zh.md @@ -8,7 +8,7 @@ `plan/mode`（`{ active: boolean }`）是一个仅写日志、整值替换的 `SessionEventMap` 成员。`foldPlanMode(events)` 返回最后记录的值，如果没有则返回 `false`，因此恢复、fork 和压缩（compaction）都能直接从会话日志恢复 plan 状态。UI 通过 `session/event` 观察已提交的切换。 -`ctx.planMode.set(agent, active)` 记录一个待生效选择，并在下一个轮次边界内刷新它。`get(agent)` 返回 `{ active, pending? }`，将塑造当前步骤的日志状态与用户的乐观选择分开。提示词提交、常规续行和请求恢复重试都在覆盖范围内；当最后记录的请求头描述了另一状态时，用户选择的变更会贡献一条插件来源的 `user/message` 通知。 +`ctx.planMode.set(agent, active)` 在 agent 空闲时立即提交——下一个 prompt 之前不会有任何边界到来，因此独立的 `plan/mode` 事件当场落账——在 agent 运行中则持有待生效选择、等下一个轮内请求边界；返回值说明发生了哪种（`committed`/`queued`）、一次 `cancelled` 反转或 `noop`。`get(agent)` 返回 `{ active, pending? }`，将塑造当前步骤的日志状态与用户的轮中选择分开。提示词提交、常规续行和请求恢复重试都在覆盖范围内；当最后记录的请求头描述了另一状态时，用户选择的变更会贡献一条插件来源的 `user/message` 通知（两条提交路径皆然）。 ## 模型与人类界面 diff --git a/packages/plan/plan-mode/src/index.ts b/packages/plan/plan-mode/src/index.ts index a876d5e594..79c6e30443 100644 --- a/packages/plan/plan-mode/src/index.ts +++ b/packages/plan/plan-mode/src/index.ts @@ -249,21 +249,28 @@ export class PlanModeService extends Service { handler: ({ agent, rawInput }) => { const message = rawInput.trim() if (message === 'off') { - const state = this.get(agent) - this.set(agent, false) - if (state.active) { - return { kind: 'success', text: 'Leaving plan mode (applies from the next step).' } + switch (this.set(agent, false)) { + case 'committed': + return { kind: 'success', text: 'Plan mode off.' } + case 'queued': + return { kind: 'success', text: 'Leaving plan mode (applies from the next step).' } + case 'cancelled': + return { kind: 'success', text: 'Plan mode entry cancelled.' } + case 'noop': + // Repeat the queued wording while an exit still awaits its + // boundary; only a truly inactive session reads idempotent. + return foldPlanMode(agent.session.events) + ? { kind: 'success', text: 'Leaving plan mode (applies from the next step).' } + : { kind: 'success', text: 'Plan mode is already inactive.' } } - if (state.pending === true) { - return { kind: 'success', text: 'Plan mode entry cancelled.' } - } - return { kind: 'success', text: 'Plan mode is already inactive.' } } - this.set(agent, true) + const outcome = this.set(agent, true) if (message !== '') agent.steer(createUserMessage({ content: [{ type: 'text', text: message }], source: { kind: 'user' } })) return { kind: 'success', - text: 'Entering plan mode (applies from the next step). Use /plan off to leave.', + text: outcome === 'committed' + ? 'Plan mode on. Use /plan off to leave.' + : 'Entering plan mode (applies from the next step). Use /plan off to leave.', } }, }) @@ -357,17 +364,37 @@ export class PlanModeService extends Service { } /** - * Select whether plan mode should be active from the next request boundary. - * Repeated selection of the current or already-pending state is a no-op. + * Select whether plan mode should be active. An idle agent commits the + * change immediately (no boundary would arrive until the next prompt); a + * running agent holds it as pending intent for the next in-turn request + * boundary. Repeated selection of the current or already-pending state is + * a no-op. * * @param agent The agent to switch. * @param active Whether plan mode should be active. + * @returns what happened: `committed` (logged now), `queued` (awaiting the + * next boundary), `cancelled` (an opposite pending selection was cleared; + * the logged state already matches), or `noop` (already in that state). */ - set(agent: Agent, active: boolean): void { + set(agent: Agent, active: boolean): 'committed' | 'queued' | 'cancelled' | 'noop' { const session = agent.session - const target = this.pendingIntents.get(session)?.active ?? foldPlanMode(session.events) - if (active === target) return - this.pendingIntents.set(session, { active, narrate: true }) + const pending = this.pendingIntents.get(session) + const target = pending?.active ?? foldPlanMode(session.events) + if (active === target) return 'noop' + if (agent.status === 'running') { + this.pendingIntents.set(session, { active, narrate: true }) + return foldPlanMode(session.events) === active ? 'cancelled' : 'queued' + } + // Idle: commit now. Delete only after append succeeds so a failed durable + // write leaves the selection retryable rather than silently dropped. + if (active === foldPlanMode(session.events)) { + this.pendingIntents.delete(session) + return 'cancelled' + } + session.append('plan/mode', { active }) + this.pendingIntents.delete(session) + this.narrate(session, active) + return 'committed' } /** Flush one pending selection before the next request assembly. */ @@ -384,7 +411,11 @@ export class PlanModeService extends Service { // Delete only after append succeeds so a later boundary can retry a failed // durable write. this.pendingIntents.delete(session) - if (!pending.narrate) return + if (pending.narrate) this.narrate(session, target) + } + + /** Tell the model about a user switch when the last logged header described the other mode. */ + private narrate(session: Session, target: boolean): void { const told = planModeAtLastHeader(session.events) if (told === undefined || told === target) return const text = target diff --git a/packages/plan/plan-mode/src/invariant.ts b/packages/plan/plan-mode/src/invariant.ts index 7c377daac6..797010c59f 100644 --- a/packages/plan/plan-mode/src/invariant.ts +++ b/packages/plan/plan-mode/src/invariant.ts @@ -11,46 +11,33 @@ export const name = 'plan-mode-invariant' /** Service required before the companion can reserve package ownership. */ export const inject = ['invariants'] -/** Validate one `plan/mode` event before it reaches the durable log. */ -function validateEvent(openTurn: number | null, event: SessionEvent, fail: InvariantFailure): void { +/** + * Validate one `plan/mode` event before it reaches the durable log. + * `plan/mode` is a standalone whole-value event: an idle selection commits + * between turns and a mid-turn selection commits at the step boundary, so + * no turn-enclosure relation exists — only the payload shape is checkable. + */ +function validateEvent(event: SessionEvent, fail: InvariantFailure): void { if (event.type !== 'plan/mode') return - if (openTurn === null) fail('plan/mode appended outside any open turn') const active = (event.data as { active?: unknown }).active if (typeof active !== 'boolean') { fail(`plan/mode carries invalid active state ${JSON.stringify(active)}; expected a boolean`) } } -/* jscpd:ignore-start -- package companions share replay and dispatch plumbing */ /** Install validation for loaded and newly appended plan-mode state. */ const install: InvariantInstaller = Object.assign((ctx: Context, fail: InvariantFailure) => { - const traces = new WeakMap() - const seed = (session: Session): number | null => { - let openTurn: number | null = null - traces.set(session, openTurn) - for (const event of session.events) { - if (event.type === 'turn/start') openTurn = event.data.turn - else if (event.type === 'turn/end') openTurn = null - validateEvent(openTurn, event, fail) - traces.set(session, openTurn) - } - return openTurn + const seed = (session: Session): void => { + for (const event of session.events) validateEvent(event, fail) } - const traceFor = (session: Session): number | null => traces.get(session) ?? seed(session) - for (const session of ctx.sessions.list()) seed(session) ctx.on('session/created', (session) => { seed(session) }, { global: true }) - ctx.on('session/event', (session, event) => { - if (event.type === 'turn/start') traces.set(session, event.data.turn) - else if (event.type === 'turn/end') traces.set(session, null) - }, { global: true }) ctx.on('internal/dispatch', (_mode, eventName, args) => { if (eventName !== 'session/event') return - const [session, event] = args as [Session, SessionEvent] - validateEvent(traceFor(session), event, fail) + const [, event] = args as [Session, SessionEvent] + validateEvent(event, fail) }, { global: true }) }, { inject: ['sessions'] }) -/* jscpd:ignore-end */ /** * Register the plan-mode invariant companion. diff --git a/packages/plan/plan-mode/tests/invariant.spec.ts b/packages/plan/plan-mode/tests/invariant.spec.ts index 569fbdd09e..fe31510be7 100644 --- a/packages/plan/plan-mode/tests/invariant.spec.ts +++ b/packages/plan/plan-mode/tests/invariant.spec.ts @@ -44,10 +44,10 @@ describe('plan-mode stream invariants', () => { .toThrow(/expected a boolean/) }) - it('rejects plan state outside any open turn', async () => { + it('accepts standalone plan state between turns (the idle immediate commit)', async () => { const ctx = await setup() expect(() => ctx.sessions.create().append('plan/mode', { active: true })) - .toThrow(/outside any open turn/) + .not.toThrow() }) it('ignores unrelated dispatches and session events', async () => { @@ -85,12 +85,12 @@ describe('plan-mode stream invariants', () => { await expect(ctx.plugin(PlanModeInvariant).then(() => undefined)).resolves.toBeUndefined() }) - it('rejects unenclosed existing plan state on late registration', async () => { + it('accepts standalone existing plan state on late registration', async () => { const ctx = new Context() await ctx.plugin(SessionStore) ctx.sessions.create().append('plan/mode', { active: true }) await ctx.plugin(InvariantService, { enabled: true }) - await expect(ctx.plugin(PlanModeInvariant).then(() => undefined)).rejects.toThrow(/outside any open turn/) + await expect(ctx.plugin(PlanModeInvariant).then(() => undefined)).resolves.toBeUndefined() }) }) diff --git a/packages/plan/plan-mode/tests/plan-mode.spec.ts b/packages/plan/plan-mode/tests/plan-mode.spec.ts index 9a4e326cfe..73dce25dbb 100644 --- a/packages/plan/plan-mode/tests/plan-mode.spec.ts +++ b/packages/plan/plan-mode/tests/plan-mode.spec.ts @@ -23,11 +23,13 @@ const PLAN_CONFIG = { section: TEST_PLAN_SECTION } satisfies PlanModeConfig * and between-step seams used by the loop. */ -async function agentWithSession(ctx: Context, id = 'agent-1', { active }: { active?: boolean } = {}): Promise { +async function agentWithSession(ctx: Context, id = 'agent-1', { active, status = 'running' }: { active?: boolean; status?: 'idle' | 'running' } = {}): Promise { // A live store session when a store is mounted (the command executor logs // lifecycle events through it); bare otherwise (fold/tool-only benches). + // Boundary tests default to a running agent (the mid-turn shape); the + // idle-commit tests pass status: 'idle' explicitly. const session = new Session(SessionId(id)) - const agent = { id: SessionId(id), session, options: {} } as unknown as Agent & { session: Session } + const agent = { id: SessionId(id), session, options: {}, status } as unknown as Agent & { session: Session } let scoped!: Context await ctx.plugin(Object.assign((inner: Context) => { scoped = createScope(inner, agent).ctx }, { inject: ['tools'], @@ -167,23 +169,56 @@ describe('ctx.planMode: get/set', () => { expect(ctx.planMode.get(agent)).toEqual({ active: true }) }) - it('selects inactive as the plan exit target', async () => { + it('selects inactive as the plan exit target while running', async () => { const ctx = await setup() const agent = await agentWithSession(ctx) agent.session.append('plan/mode', { active: true }) - ctx.planMode.set(agent, false) + expect(ctx.planMode.set(agent, false)).toBe('queued') expect(ctx.planMode.get(agent)).toEqual({ active: true, pending: false }) }) it('drops a no-op set (target equals pending, else the current fold)', async () => { const ctx = await setup() const agent = await agentWithSession(ctx) - ctx.planMode.set(agent, false) + expect(ctx.planMode.set(agent, false)).toBe('noop') expect(ctx.planMode.get(agent)).toEqual({ active: false }) - ctx.planMode.set(agent, true) - ctx.planMode.set(agent, true) + expect(ctx.planMode.set(agent, true)).toBe('queued') + expect(ctx.planMode.set(agent, true)).toBe('noop') expect(ctx.planMode.get(agent)).toEqual({ active: false, pending: true }) }) + + it('an idle selection commits plan/mode immediately (no boundary would come)', async () => { + const ctx = await setup() + const agent = await agentWithSession(ctx, 'agent-idle', { status: 'idle' }) + expect(ctx.planMode.set(agent, true)).toBe('committed') + expect(foldPlanMode(agent.session.events)).toBe(true) + expect(ctx.planMode.get(agent)).toEqual({ active: true }) + // Immediately reversible, still without a boundary. + expect(ctx.planMode.set(agent, false)).toBe('committed') + expect(foldPlanMode(agent.session.events)).toBe(false) + // A later boundary finds nothing pending — no double append. + await boundary(ctx, agent, 'step/end') + expect(agent.session.events.filter(event => event.type === 'plan/mode')).toHaveLength(2) + }) + + it('an idle reversal of a mid-turn pending intent cancels without logging', async () => { + const ctx = await setup() + const agent = await agentWithSession(ctx) + expect(ctx.planMode.set(agent, true)).toBe('queued') + ;(agent as { status: string }).status = 'idle' + // Back to the logged state: the pending intent clears, nothing lands. + expect(ctx.planMode.set(agent, false)).toBe('cancelled') + expect(agent.session.events.some(event => event.type === 'plan/mode')).toBe(false) + expect(ctx.planMode.get(agent)).toEqual({ active: false }) + }) + + it('an idle commit narrates when the last header told the model otherwise', async () => { + const ctx = await setup() + const agent = await agentWithSession(ctx, 'agent-idle-narrate', { status: 'idle' }) + header(agent.session) + ctx.planMode.set(agent, true) + expect(noticeTexts(agent.session)).toEqual(['The user switched this session to plan mode.']) + }) }) describe('the boundary flush', () => {