Skip to content

Fix Pi approval continuation follow-up display#843

Merged
backnotprop merged 1 commit into
backnotprop:mainfrom
xz-dev:fix/pi-approval-visible-followup
Jun 3, 2026
Merged

Fix Pi approval continuation follow-up display#843
backnotprop merged 1 commit into
backnotprop:mainfrom
xz-dev:fix/pi-approval-visible-followup

Conversation

@xz-dev
Copy link
Copy Markdown
Contributor

@xz-dev xz-dev commented Jun 3, 2026

Summary

Follow-up to #805.

  • Keep the Pi approved-plan continuation, but stop sending it as an immediate followUp from agent_end.
  • Wait until Pi reports the session is idle, then send the continuation as a normal extension user message.
  • Cap the wait at 10 seconds so the extension cannot poll forever.

Background

#804 fixed an approval-path crash in the Pi extension. Approved plans return terminate: true, then Plannotator sends a small continuation message so Pi starts executing the approved plan:

pi.sendUserMessage("Continue with the approved plan.");

That direct call was unsafe from inside the agent_end handler because Pi can still consider the agent run active while agent_end extension listeners are settling. #805 fixed the crash by sending the continuation as:

pi.sendUserMessage("Continue with the approved plan.", { deliverAs: "followUp" });

That keeps execution working, but it has a UI side effect in current Pi TUI: the continuation is rendered as a queued pending message even though the approved plan is already starting:

Follow-up: Continue with the approved plan.
↳ Alt+Up to edit all queued messages

This is confusing because users see a pending follow-up while the agent is already working.

Why this approach

I checked the current Pi source instead of relying on observed behavior:

  • agent_end is emitted before the run is fully idle; isStreaming remains true until awaited agent_end listeners settle.
  • sendUserMessage() without deliverAs throws while streaming.
  • deliverAs: "followUp" pushes the text into Pi's follow-up queue, and the interactive UI renders queued follow-ups with the Follow-up: prefix.

So the extension needs to avoid both extremes:

  1. Sending immediately without deliverAs can recreate Pi extension errors after approving a plan #804.
  2. Sending immediately with deliverAs: "followUp" creates the visible queued-message row.

The smallest plugin-side solution is to defer the continuation until Pi reports idle, then send a normal user message. That starts the implementation turn without using the visible follow-up queue.

The polling is bounded at 200 attempts × 50ms = 10 seconds. If Pi never becomes idle, Plannotator stops waiting instead of looping forever or creating a stale follow-up queue entry.

Testing

  • git diff --check
  • Manual local Pi extension test:
    • Approve plan in browser.
    • Agent continues with Continue with the approved plan. as a normal message.
    • No persistent Follow-up: Continue with the approved plan. pending row remains.
  • Not run: bun run typecheck (Bun is not installed in this environment: bun: command not found).

@backnotprop backnotprop merged commit 3c2f520 into backnotprop:main Jun 3, 2026
13 checks passed
@backnotprop
Copy link
Copy Markdown
Owner

This is now released in v0.19.27. Thanks for your contribution!

Skogix added a commit to skogai/plannotator that referenced this pull request Jun 3, 2026
commit b2b8ce8
Author: Michael Ramos <mdramos8@gmail.com>
Date:   Wed Jun 3 14:32:40 2026 -0700

    chore: bump version to 0.19.27

    For provenance purposes, this commit was AI assisted.

commit ac26300
Author: Michael Ramos <mdramos8@gmail.com>
Date:   Wed Jun 3 14:19:31 2026 -0700

    feat: add PLANNOTATOR_GLIMPSE env var to disable Glimpse native window

    Add glimpse config option to PlannotatorConfig and resolveUseGlimpse()
    resolver. Wire into both Bun and Pi browser-opening paths. Document
    PLANNOTATOR_GLIMPSE, PLANNOTATOR_GLIMPSE_WIDTH, and
    PLANNOTATOR_GLIMPSE_HEIGHT in env var reference and AGENTS.md.

    For provenance purposes, this commit was AI assisted.

commit 33907d3
Author: Michael Ramos <mdramos8@gmail.com>
Date:   Wed Jun 3 14:09:13 2026 -0700

    feat(marketing): add Kiro CLI to homepage agent selector

    For provenance purposes, this commit was AI assisted.

commit 1fa6052
Author: Michael Ramos <mdramos8@gmail.com>
Date:   Wed Jun 3 14:04:46 2026 -0700

    perf: defer multi-message annotation export to submit time

    Move messageAnnotationEntries from a reactive useMemo to an on-demand
    useCallback. The expensive parseMarkdownToBlocks calls for all messages
    now only run when the user submits, downloads, or copies feedback —
    not on every annotation change. Derive annotatedMessageIds from the
    lightweight counts map instead of the full parsed entries.

    For provenance purposes, this commit was AI assisted.

commit be2c81f
Author: Graham Lipsman <glipsman@gmail.com>
Date:   Wed Jun 3 13:01:36 2026 -0700

    annotate-last: pick which message to annotate (fixes backnotprop#800) (backnotprop#809)

    * feat: message picker for annotate-last (backnotprop#800)

    When running /plannotator-last after /rewind, the newest transcript
    entry is no longer the message the user intended to annotate, and there
    was no affordance to pick a different one.

    Adds a picker UI that surfaces the recent assistant messages so the
    user can choose which one to annotate:

    - A "Message N of M" button in the Viewer's sticky-top action bar
      (alongside Copy / Global comment / Attachments), so it stays
      accessible while scrolling.
    - A "Messages" tab in the left sidebar with the full list
      (newest-first, preview + timestamp, default ★), mirroring the
      existing Files / Versions / Archive tab pattern.

    Wired for Claude Code, Codex, and Droid (all share apps/hook/server).
    OpenCode, Pi, and Copilot still get the original single-message
    behavior — they don't emit recentMessages, so the picker affordances
    hide cleanly.

    Default selection (index 0) matches today's "last message" behavior,
    so users who don't interact with the picker see no change.

    * feat: extend annotate-last picker to Copilot and OpenCode

    The picker UI from backnotprop#800 was wired for Claude / Codex / Droid only. Pull
    Copilot and OpenCode onto the same shape so users on those harnesses
    also get the recent-messages picker when annotating the last assistant
    message.

    - Copilot: replace getLastCopilotMessage with getRecentCopilotMessages,
      walking events.jsonl newest-first up to 25 assistant.message events.
    - OpenCode: rewrite the session walk to collect up to 25 messages
      (newest first) instead of bailing on the first hit; normalize the SDK
      time.created (ms epoch) to ISO to match the shared picker contract.
    - Both pass recentMessages to startAnnotateServer only when length > 1,
      matching the existing Claude/Codex/Droid behavior.

    Also trims a leftover narrating comment in MessagesBrowser and refreshes
    the stale Copilot session-parser header.

    Pi parity follows in the next commit (needs round-trip of the picker
    selection through /api/feedback so its post-submit anchoring quotes the
    right message).

    * feat(pi): wire annotate-last picker with feedback round-trip

    Extends the picker UI (backnotprop#800) to Pi and fixes a Pi-specific anchoring bug
    the picker would otherwise introduce.

    Picker plumbing
    - assistant-message: getRecentAssistantMessages walks the active branch
      newest-first, returning { messageId, text, timestamp? } in the same
      shape the other harnesses produce.
    - Plumbed through plannotator-browser / plannotator-events so the Bun
      server's recentMessages option is populated when the branch has more
      than one assistant message.

    Anchoring fix
    - Pi quotes the targeted assistant message back to the agent because its
      UX is async — the conversation may have moved on by feedback time.
      With the picker, that target is no longer guaranteed to be the
      snapshot taken when the UI opened. The editor now sends the user's
      selectedMessageId with /api/feedback; Pi looks it up in the current
      branch via findAssistantMessageByEntryId and quotes that message
      instead. Falls back to the original snapshot if the entry is gone.
    - The round-trip field is optional and only meaningful in annotate-last
      mode; other harnesses (and other modes) ignore it.

    Timestamp safety
    - Pi's SDK currently types SessionEntryBase.timestamp as string, but the
      picker contract everywhere else is ISO. Treat the value as unknown and
      normalize string/number(ms)/Date to ISO; drop anything else, rather
      than blind-casting and risking silent drift if the SDK changes.

    * chore: strip issue-number references from comments

    Comments shouldn't rely on external references — issue numbers age out
    of context, link rot is a thing, and a reader shouldn't need to open
    GitHub to understand why a line exists. Strip the `(backnotprop#800)` and `(backnotprop#570)`
    parentheticals from comments and doc strings across the picker and
    review-gate code; the surrounding "why" content is preserved.

    * fix: prevent removeChild crash when switching annotate-last messages

    Switching the picked message remounted nothing, so React reconciled new
    content against DOM that web-highlighter had mutated with <mark> nodes,
    throwing removeChild. Drive the Viewer key (and StickyHeaderLane's
    remount token) off a shared viewerContentKey so a message switch fully
    remounts the Viewer and re-anchors the sticky-header observer.

    Also cap MessagesBrowser row previews via previewText() and drop the
    redundant 'block' class that was overriding line-clamp-2.

    * feat: persist annotate-last feedback across messages

    ---------

    Co-authored-by: Michael Ramos <mdramos8@gmail.com>

commit 96aad59
Author: Benjamin Jesuiter <bjesuiter@gmail.com>
Date:   Wed Jun 3 21:17:11 2026 +0200

    Open Plannotator in Glimpse when available (backnotprop#840)

    * feat: open plannotator in glimpse

    * fix: harden Glimpse browser integration

    * chore: refresh lockfile after rebase

    ---------

    Co-authored-by: Michael Ramos <mdramos8@gmail.com>

commit 3c2f520
Author: Xiangzhe <32761048+xz-dev@users.noreply.github.com>
Date:   Wed Jun 3 23:45:37 2026 +0800

    fix(pi-extension): avoid visible approval follow-up queue (backnotprop#843)

commit a6fb2f5
Author: dgrissen2 <123790187+dgrissen2@users.noreply.github.com>
Date:   Wed Jun 3 02:38:12 2026 +0200

    feat(setup-goal): opt-in "grill first" interview step in Phase 1 (backnotprop#839)

commit cbc6186
Author: Robert Dailey <git@rdailey.me>
Date:   Tue Jun 2 13:18:52 2026 -0500

    fix(ui): align list markers to first line in clean diff view (backnotprop#838)

    * fix(ui): align list marker to first line in clean diff view

    * refactor(ui): extract list-item marker and body into ListItemBody

    Three render paths (BlockRenderer and two in PlanCleanDiffView)
    duplicated the same marker-plus-paragraph scaffold. A recent alignment
    fix had to be applied to each surface independently, which exposed the
    duplication. The shared structure now lives in ListItemBody; call sites
    keep their own row wrapper with surface-specific concerns (indent, data
    attributes, hover props, interactivity).

commit 6a7fd41
Author: Ashwin John Chempolil <57299550+ashwinjohn3@users.noreply.github.com>
Date:   Tue Jun 2 14:04:18 2026 -0400

    Add Kiro CLI integration (skills + custom agent, cross-platform installers) (backnotprop#837)

    Universal, auto-detected Kiro CLI support — no flag, no separate installer.
    When ~/.kiro exists (or kiro-cli is on PATH), the installer copies Plannotator
    skills and a custom agent into ~/.kiro, the same convention used for Codex and
    Gemini.

    - packages/shared: add the kiro-cli agent origin (badge, prompt runtime,
      PLAN_TOOL_NAMES). Origin is cosmetic for Kiro (no dedicated AI provider).
    - apps/kiro-cli: 3 origin-baked skills (review/annotate/archive) + an example
      custom agent that wires every skill via skill:// resources and a
      plannotator-scoped shell tool. setup-goal + visual-explainer install from
      apps/skills (no content duplication).
    - scripts/install.sh: auto-detect ~/.kiro, sparse-checkout apps/kiro-cli,
      install 3 kiro + 2 shared skills + the agent (never clobbering an existing
      one); covered by scripts/install.test.ts.
    - scripts/install.ps1 + install.cmd: Windows parity mirroring the Codex
      pattern. Statically verified only; not yet runtime-tested on Windows.
    - docs: installation + Kiro guides, AGENTS.md, env-var reference updated.

    Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

commit d8e0b86
Author: Michael Ramos <mdramos8@gmail.com>
Date:   Sun May 31 20:12:19 2026 -0700

    chore: bump version to 0.19.26

    For provenance purposes, this commit was AI assisted.

commit 07b7a55
Author: Michael Ramos <mdramos8@gmail.com>
Date:   Sun May 31 20:10:56 2026 -0700

    fix: skip update toast for shared plan viewers

    For provenance purposes, this commit was AI assisted.

commit ffe2be5
Author: Michael Ramos <mdramos8@gmail.com>
Date:   Sun May 31 20:04:33 2026 -0700

    feat: improve update notification with toast, shimmer, and prominent button

    Add a sonner toast when a new version is detected, apply TextShimmer to
    the Options button label, and restyle the copy-update button as a filled
    primary action. Applied to both plan and review editors.

    For provenance purposes, this commit was AI assisted.

commit 3638190
Author: Michael Ramos <mdramos8@gmail.com>
Date:   Sun May 31 19:19:56 2026 -0700

    chore: bump version to 0.19.25

    For provenance purposes, this commit was AI assisted.

commit 2a8eb9d
Author: J-token <43957100+j-token@users.noreply.github.com>
Date:   Mon Jun 1 01:58:00 2026 +0900

    Fix flickering AI-tip input in label settings (backnotprop#830)

    The "Add AI instruction tip" editor in Settings > Labels flickered
    intensely on open. Its `tip-slide-open` keyframe animated `max-height`
    (a layout property) to 60px -- larger than the input's natural height --
    with no animation-fill-mode. Inside the OverlayScrollbars viewport and a
    flex `min-h-0` container this formed a layout feedback loop that
    restarted the animation ~40x/second, jittering the editor height.

    Animate `opacity` and `transform` instead, so no layout property is
    animated. Measured on the live demo, animation restarts and height
    reversals dropped from ~17/10 to 0, with height stable at its natural
    value.

    Fixes backnotprop#829

    Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants