Plan mode: xd://propose ignores slug plan files and only finds local://PLAN.md #37

New issue

Closed

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

omp-deck commented

2026-10-19 14:21:58 +02:00

Collaborator

Implementation: #43

Problem

In a deck plan-mode session, the agent wrote its plan to local://implement-review-replies-plan.md (as the plan-mode prompt requires) and wrote implement-review-replies plus a title line to xd://propose. Propose failed with Plan file not found at local://PLAN.md. Write the finalized plan to local://PLAN.md before requesting approval. Writing an identical copy to local://PLAN.md made propose succeed. The prompt mandates local://<slug>-plan.md; the deck behaves as if the path is fixed at local://PLAN.md.

Root cause (verified in code):

The SDK (node_modules/@oh-my-pi/pi-coding-agent/src/tools/resolve.ts, dispatchResolutionDevice) passes the entire trimmed propose body, including the title line, to the plan proposal handler.
resolveApprovedPlan (plan-mode/approved-plan.ts) derives the slug via normalizePlanTitle, which throws on /, \ or .. — so a title such as one mentioning .omp/commands/implement.md drops the slug entirely. A multi-line body without those characters gets whitespace collapsed into one long bogus slug.
It then falls back to listPlanFiles (a scan of the session-local dir for *plan.md, newest first) and the state path. The TUI, ACP and prewalk handlers pass listPlanFiles; the deck's PlanModeBridge.#handlePlanPropose (apps/server/src/bridge/plan-mode-bridge.ts) does not, so the only candidate left is the state path local://PLAN.md.
Deck-side text still tells agents to use local://PLAN.md (apps/web/src/lib/inbox-prompt.ts, protocol comments), contradicting the SDK's slug convention.

Plan

Approach

Bridge scan fallback — in apps/server/src/bridge/plan-mode-bridge.ts:
- Add import { listPlanFiles } from "@oh-my-pi/pi-coding-agent/plan-mode/plan-files"; (the package exports ./plan-mode/*). No deck equivalent exists; reuse the SDK helper.
- In #handlePlanPropose, add to the resolveApprovedPlan({...}) input: listPlanFiles: () => listPlanFiles({ localProtocolOptions: { getArtifactsDir: this.getArtifactsDir, getSessionId: this.getSessionId } }). These are the same callbacks #readPlanFile already feeds to resolveLocalUrlToPath, so the scan and the read resolve the same local:// root. listPlanFiles returns [] when the dir is missing; no extra handling.
- Keep PLAN_FILE_URL as the plan-mode state default (the SDK requires a state path), but rewrite its doc comment (lines ~67–69) to: it is only the state-path fallback; agents write local://<slug>-plan.md, found via the slug or the listPlanFiles scan.
- Update the header comment (lines ~14–16) to drop local://PLAN.md as an example write target and the #handlePlanPropose doc comment to mention the scan step (slug → state path → scanned plan files).
Align deck prompt text to the slug convention:
- apps/web/src/lib/inbox-prompt.ts lines ~83 and ~109: replace (local://PLAN.md). Do not impose your own layout — plan mode owns the format. with (local://<slug>-plan.md, slug written to xd://propose). Do not impose your own layout — plan mode owns the format. in both places.
- packages/protocol/src/index.ts: line ~792 comment → Plan-mode state path (\local://PLAN.md` default); the agent's actual plan lives at `local://-plan.md`.; line ~804 → `local://` path of the plan file the proposal resolved to (normally `local://-plan.md`).; lines ~1013 and ~1029: replace local://PLAN.mdwiththe proposed plan file (`planFilePath`)and drop "before the rename" (no rename happens since SDK v18.10.1 — confirm against#handleDetachedResponse/approve path in the bridge, which writes editedContenttopending.planFilePath`).
- Leave apps/web/src/lib/store.ts ?? "local://PLAN.md" fallback and the migration comment as-is (they describe the state default, which is still correct).

Critical files & anchors

apps/server/src/bridge/plan-mode-bridge.ts — #handlePlanPropose (resolveApprovedPlan call ~line 578), PLAN_FILE_URL ~line 70.
node_modules/@oh-my-pi/pi-coding-agent/src/plan-mode/plan-files.ts — listPlanFiles signature ({ localProtocolOptions }) => Promise<string[]>.
apps/server/src/bridge/plan-mode-bridge.test.ts — harness planFileForSlug, invokePropose, test "approve happy path" ~line 257.

Verification

Add a permanent test in apps/server/src/bridge/plan-mode-bridge.test.ts next to the happy-path test: enter(), write # Foo\n\nBody.\n to harness.planFileForSlug("foo"), invoke propose with body "foo\nImplement review replies in .omp/commands/implement.md"; expect the plan_proposed frame's planFilePath to be local://foo-plan.md and planContent to match /Body/. Then respond(..., { action: "reject" }) to settle. Before the fix this rejects with Plan file not found at local://PLAN.md.
Existing test "propose throws a ToolError when the plan file is missing" must still pass (empty dir → scan returns []).
Run from /workspace/omp-deck: bun test apps/server/src/bridge/plan-mode-bridge.test.ts, then the server typecheck used by the repo (bun run typecheck if defined in root package.json; otherwise bunx tsc --noEmit -p apps/server).
Manual: in the deck UI, enable plan mode, have the agent write local://x-plan.md and propose x + a title line containing a /; the approval card opens showing local://x-plan.md.

Assumptions & contingencies

If the @oh-my-pi/pi-coding-agent/plan-mode/plan-files import fails to resolve at runtime/typecheck, import listPlanFiles the same way approved-plan is imported; if still unavailable, implement an identical local helper in the bridge (readdir of resolveLocalUrlToPath("local://", …)'s root, *plan.md, sort by mtime desc).

Implementation: #43 ## Problem In a deck plan-mode session, the agent wrote its plan to `local://implement-review-replies-plan.md` (as the plan-mode prompt requires) and wrote `implement-review-replies` plus a title line to `xd://propose`. Propose failed with `Plan file not found at local://PLAN.md. Write the finalized plan to local://PLAN.md before requesting approval.` Writing an identical copy to `local://PLAN.md` made propose succeed. The prompt mandates `local://<slug>-plan.md`; the deck behaves as if the path is fixed at `local://PLAN.md`. Root cause (verified in code): - The SDK (`node_modules/@oh-my-pi/pi-coding-agent/src/tools/resolve.ts`, `dispatchResolutionDevice`) passes the entire trimmed propose body, including the title line, to the plan proposal handler. - `resolveApprovedPlan` (`plan-mode/approved-plan.ts`) derives the slug via `normalizePlanTitle`, which throws on `/`, `\` or `..` — so a title such as one mentioning `.omp/commands/implement.md` drops the slug entirely. A multi-line body without those characters gets whitespace collapsed into one long bogus slug. - It then falls back to `listPlanFiles` (a scan of the session-local dir for `*plan.md`, newest first) and the state path. The TUI, ACP and prewalk handlers pass `listPlanFiles`; the deck's `PlanModeBridge.#handlePlanPropose` (`apps/server/src/bridge/plan-mode-bridge.ts`) does not, so the only candidate left is the state path `local://PLAN.md`. - Deck-side text still tells agents to use `local://PLAN.md` (`apps/web/src/lib/inbox-prompt.ts`, protocol comments), contradicting the SDK's slug convention. ## Plan ### Approach 1. **Bridge scan fallback** — in `apps/server/src/bridge/plan-mode-bridge.ts`: - Add `import { listPlanFiles } from "@oh-my-pi/pi-coding-agent/plan-mode/plan-files";` (the package exports `./plan-mode/*`). No deck equivalent exists; reuse the SDK helper. - In `#handlePlanPropose`, add to the `resolveApprovedPlan({...})` input: `listPlanFiles: () => listPlanFiles({ localProtocolOptions: { getArtifactsDir: this.getArtifactsDir, getSessionId: this.getSessionId } })`. These are the same callbacks `#readPlanFile` already feeds to `resolveLocalUrlToPath`, so the scan and the read resolve the same `local://` root. `listPlanFiles` returns `[]` when the dir is missing; no extra handling. - Keep `PLAN_FILE_URL` as the plan-mode state default (the SDK requires a state path), but rewrite its doc comment (lines ~67–69) to: it is only the state-path fallback; agents write `local://<slug>-plan.md`, found via the slug or the `listPlanFiles` scan. - Update the header comment (lines ~14–16) to drop `local://PLAN.md` as an example write target and the `#handlePlanPropose` doc comment to mention the scan step (slug → state path → scanned plan files). 2. **Align deck prompt text to the slug convention**: - `apps/web/src/lib/inbox-prompt.ts` lines ~83 and ~109: replace ` (local://PLAN.md). Do not impose your own layout — plan mode owns the format.` with ` (local://<slug>-plan.md, slug written to xd://propose). Do not impose your own layout — plan mode owns the format.` in both places. - `packages/protocol/src/index.ts`: line ~792 comment → `Plan-mode state path (\`local://PLAN.md\` default); the agent's actual plan lives at \`local://<slug>-plan.md\`.`; line ~804 → `\`local://\` path of the plan file the proposal resolved to (normally \`local://<slug>-plan.md\`).`; lines ~1013 and ~1029: replace `local://PLAN.md` with `the proposed plan file (\`planFilePath\`)` and drop "before the rename" (no rename happens since SDK v18.10.1 — confirm against `#handleDetachedResponse`/approve path in the bridge, which writes `editedContent` to `pending.planFilePath`). - Leave `apps/web/src/lib/store.ts` `?? "local://PLAN.md"` fallback and the migration comment as-is (they describe the state default, which is still correct). ### Critical files & anchors - `apps/server/src/bridge/plan-mode-bridge.ts` — `#handlePlanPropose` (`resolveApprovedPlan` call ~line 578), `PLAN_FILE_URL` ~line 70. - `node_modules/@oh-my-pi/pi-coding-agent/src/plan-mode/plan-files.ts` — `listPlanFiles` signature `({ localProtocolOptions }) => Promise<string[]>`. - `apps/server/src/bridge/plan-mode-bridge.test.ts` — harness `planFileForSlug`, `invokePropose`, test "approve happy path" ~line 257. ### Verification - Add a permanent test in `apps/server/src/bridge/plan-mode-bridge.test.ts` next to the happy-path test: `enter()`, write `# Foo\n\nBody.\n` to `harness.planFileForSlug("foo")`, invoke propose with body `"foo\nImplement review replies in .omp/commands/implement.md"`; expect the `plan_proposed` frame's `planFilePath` to be `local://foo-plan.md` and `planContent` to match `/Body/`. Then `respond(..., { action: "reject" })` to settle. Before the fix this rejects with `Plan file not found at local://PLAN.md`. - Existing test "propose throws a ToolError when the plan file is missing" must still pass (empty dir → scan returns `[]`). - Run from `/workspace/omp-deck`: `bun test apps/server/src/bridge/plan-mode-bridge.test.ts`, then the server typecheck used by the repo (`bun run typecheck` if defined in root `package.json`; otherwise `bunx tsc --noEmit -p apps/server`). - Manual: in the deck UI, enable plan mode, have the agent write `local://x-plan.md` and propose `x` + a title line containing a `/`; the approval card opens showing `local://x-plan.md`. ### Assumptions & contingencies - If the `@oh-my-pi/pi-coding-agent/plan-mode/plan-files` import fails to resolve at runtime/typecheck, import `listPlanFiles` the same way `approved-plan` is imported; if still unavailable, implement an identical local helper in the bridge (readdir of `resolveLocalUrlToPath("local://", …)`'s root, `*plan.md`, sort by mtime desc).