Plan approval with "compact" loses the execute instruction #38

New issue

Closed

opened 2026-10-19 14:25:37 +02:00 by omp-deck · 0 comments

omp-deck commented

2026-10-19 14:25:37 +02:00

Collaborator

Problem

Observed (PR #17 follow-up session, 2026-10-16): after approving a plan with the keep|compact toggle set to compact, the resumed agent did not receive "Plan approved. Execute the plan at …". Its first action after the compaction summary was to call xd://propose again, which failed with "Plan file not found at local://-

## Problem Observed (PR #17 follow-up session, 2026-10-16): after approving a plan with the keep|compact toggle set to **compact**, the resumed agent did not receive "Plan approved. Execute the plan at …". Its first action after the compaction summary was to call `xd://propose` again, which failed with "Plan file not found at local://<slug>-<title>-plan.md"; a retry returned "No plan is awaiting approval — plan mode is not active". Only then did the agent infer it should execute. Root cause (confirmed from SDK source, `CompactOptions.suppressContinuation` doc in `@oh-my-pi/pi-coding-agent/src/extensibility/extensions/types.ts`): a manual compaction **aborts any turn in flight** and, once the summary is committed, resumes it with a generic auto-continue nudge unless `suppressContinuation: true` is passed. In the non-detached path (`#handlePlanPropose`, `apps/server/src/bridge/plan-mode-bridge.ts` ~683-709) `session.compact(...)` runs inside the still-pending `xd://propose` tool call. The compaction aborts that turn, so the returned approval tool result never reaches the model; the nudge resumes the agent with a summary ending on an unanswered propose. The SDK's own interactive flow (`interactive-mode.ts` ~5631-5796) avoids this by compacting with `suppressContinuation: true` and then dispatching the approval as a synthetic prompt/follow-up. The detached path (~544-561) already sends a synthetic follow-up but also lacks `suppressContinuation`, so it can double-prompt (nudge + approval). ## Plan ### Context After "Approve and compact", the agent's first post-compaction input must be the `Plan approved. Execute the plan at \`<planFilePath>\`.` text (plus extra instructions), in both the live and detached (post-restart) paths, with no generic continuation nudge. ### Approach 1. **Widen the session surface.** In `apps/server/src/bridge/plan-mode-bridge.ts`, change `PlanModeSessionSurface.compact` (line ~139) to `compact(instructions: string, options?: { suppressContinuation?: boolean }): Promise<unknown>;` and update its doc comment to note that a manual compaction aborts the in-flight turn and that `suppressContinuation` stops the SDK's auto-continue nudge. The real session is the SDK `AgentSession` (passed via `Object.assign` in `apps/server/src/bridge/in-process.ts` ~752-768), whose signature is `compact(customInstructions?: string, options?: CompactOptions)` — no adapter change needed. 2. **Extract one shared helper** in `PlanModeBridge`: `async #compactForExecution(planFilePath: string): Promise<void>` that calls `this.session.compact(PLAN_COMPACT_INSTRUCTIONS_TEMPLATE.replaceAll("{{planFilePath}}", planFilePath), { suppressContinuation: true })` inside try/catch, logging `log.warn(...)` on failure and never throwing (keep the existing per-path warn messages' meaning: "compaction failed for <sessionId>; executing without compaction"). Replace both existing inline compact blocks (detached ~544-552, live ~685-696) with calls to it. 3. **Live path (`#handlePlanPropose`, approve branch), when `userResponse.mode === "compact"`:** - Keep `exit("approved")` and `markPlanReferenceSent()` before compaction (unchanged order). - `await this.#compactForExecution(planFilePath)`. - Then dispatch the approval text (same `approvalText` string built today at ~699) with `void this.session.prompt(approvalText, { synthetic: true, streamingBehavior: "followUp" }).catch((err) => log.warn(\`plan-approval follow-up prompt failed for ${this.sessionId}\`, err));` — **not awaited**, because `prompt` blocks for the whole execution run and we are still inside the propose tool call. - Still return the tool result with the same `approvalText` and `details` (harmless if the turn was aborted; correct if compaction had nothing to compact and did not abort). - `mode: "keep"` (and absent mode) is unchanged: no compaction, no prompt, approval delivered only as the tool result. 4. **Detached path (`#handleDetachedResponse`)**: switch to `#compactForExecution` (adds `suppressContinuation: true`); keep its existing awaited synthetic follow-up prompt unchanged. 5. **Update the class/file header comment** (lines ~19-31) to describe: compact path compacts with continuation suppressed, then queues the synthetic approval follow-up; keep path returns approval as the tool result. ### Critical files & anchors - `apps/server/src/bridge/plan-mode-bridge.ts` — `PlanModeSessionSurface.compact` (~139), `#handleDetachedResponse` (~544-561), `#handlePlanPropose` approve branch (~679-709). - `apps/server/src/bridge/plan-mode-bridge.test.ts` — `StubSession.compact` (~83), tests at ~521 ("approve with mode:compact…") and ~609 ("detached approve with mode:compact…"). ### Verification - Update `StubSession.compact` to `async compact(instructions: string, options?: { suppressContinuation?: boolean })` and record options (e.g. `compactOptions: unknown[]`). - Rewrite test ~521 ("approve with mode:compact distills the transcript before executing"): assert `compactCalls.length === 1`, compact options `{ suppressContinuation: true }`, and exactly one `promptCalls` entry whose text starts with ``Plan approved. Execute the plan at `local://compact-me-plan.md` `` and options `{ synthetic: true, streamingBehavior: "followUp" }`; assert the compact call happened before the prompt (record a shared call-order array in the stub). Remove its old "without queuing any follow-up prompt" assertion. Add a case with `extraInstructions: "Run tests"` asserting the prompt text ends with `\n\nRun tests`. - Extend test ~609 to assert `suppressContinuation: true` on the detached compact. - Keep test ~544 ("mode:keep") asserting zero compact and zero prompt calls. - Add a case where the stub's `compact` rejects: approval prompt still fires and the propose result still resolves with the approval text. - Run: `cd /workspace/omp-deck/apps/server && bun test src/bridge/plan-mode-bridge.test.ts` — all pass. Then `bun run typecheck` (or the repo's server typecheck script) for the surface change. - Manual: in the deck UI, enter plan mode, propose a small plan, approve with the toggle on **compact**; after the compaction marker, the next user-role (synthetic) message is "Plan approved. Execute the plan at …" and the agent does not re-call `xd://propose`. ### Assumptions & contingencies - If the SDK compaction, when called from inside a tool call, does NOT abort the turn in some build (agent receives the tool result and starts executing), the queued synthetic follow-up would run after that turn as a duplicate instruction. If manual verification shows a duplicate, change the live compact path to return a tool result text of `Plan approved; context is being compacted. Stop now — the execution instruction follows as the next message.` instead of `approvalText`, keeping the follow-up prompt as the sole execute instruction.