Skip to content

fix(cli): resolve plan exit approval submissions - #10895

Merged
imanolmzd-svg merged 2 commits into
mainfrom
fix/plan-exit-approval-submit
Jun 4, 2026
Merged

fix(cli): resolve plan exit approval submissions#10895
imanolmzd-svg merged 2 commits into
mainfrom
fix/plan-exit-approval-submit

Conversation

@lambertjosh

@lambertjosh lambertjosh commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes plan_exit from being stuck and unable to be dismissed or complete submission.

  1. Submitting does nothing.
  2. You can't dismiss.
  3. If you minimize, it stays stuck at the bottom of the chat session which is really annoying

Details

  • Route plan_exit approval questions through the listener-local question service so HTTP submissions resolve the pending request that produced the popup.
  • Preserve instance context for delayed cancellation cleanup and skip approval prompt creation when cancellation lands during plan resolution.
  • Remove the obsolete standalone plan-followup question runtime so production callers must use the listener-local service.
  • Add regression coverage for listener-local approval submissions and both cancellation windows.

Context

The approval popup was emitted through the global event bus, but its pending request lived in a separate module-level question runtime. The HTTP reply endpoint uses the listener-local service, so clicking Submit could reach a different pending-request map and appear to do nothing.

The fix removes that standalone question runtime. Active session loops now inject their listener-local service explicitly, while direct domain tests inject a test-owned adapter.

Root Cause Timeline

This regression is related to the recent Promise-to-Effect migration, but the behavior changed when the native Effect HttpApi server became the only server backend rather than when the Promise facades were removed.

  • On May 29, e4c9b4363e replaced Session.* Promise facade helpers with direct Effect service calls, and aec4c4f0f0 removed the Question compatibility facade as part of the same migration effort.
  • On June 2, 7cc6080646 removed the stable Hono backend path and made Server.listen() always use the native Effect HttpApi server.
  • The native listener intentionally builds services with a fresh listener-local memo map. The plan_exit follow-up popup still used the module-level PlanFollowupRuntime.question runtime, while /question/:requestID/reply used the listener-local Question.Service.
  • Before the server cutover, the Hono route ran through AppRuntime, so both paths resolved the same pending-question state. After the cutover, the popup could still render through the global event bus, but Submit and Dismiss targeted a different pending-question map.

Screenshots

See below.

@lambertjosh
lambertjosh marked this pull request as ready for review June 4, 2026 02:38
@lambertjosh

Copy link
Copy Markdown
Contributor Author

Plan -> New session:
image

Plan -> Existing session:
image

@kilo-code-bot

kilo-code-bot Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

The second commit finishes the cleanup started in the first: removes the PlanFollowupRuntime.question standalone fallback entirely, makes question a required parameter on PlanFollowup.ask, and updates both test files to build their own makeRuntime-based adapter. All remaining call sites pass the adapter explicitly. The check-opencode-promise-facades.ts allowlist count is correctly updated (7 → 4 AppRuntime references for plan-followup.test.ts) since three calls were migrated to the test-local runtime. No dangling references to the removed property remain.

Files Reviewed (4 files)
  • packages/opencode/src/kilocode/plan-followup.ts
  • packages/opencode/src/kilocode/session/prompt.ts (unchanged — carried forward)
  • packages/opencode/test/kilocode/plan-exit-detection.test.ts
  • packages/opencode/test/kilocode/plan-followup.test.ts
  • script/check-opencode-promise-facades.ts

Reviewed by claude-4.6-sonnet-20260217 · 321,631 tokens

Review guidance: REVIEW.md from base branch main

@imanolmzd-svg
imanolmzd-svg merged commit 2e1945c into main Jun 4, 2026
21 checks passed
@imanolmzd-svg
imanolmzd-svg deleted the fix/plan-exit-approval-submit branch June 4, 2026 10:08
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
* fix(cli): resolve plan exit approval submissions

* refactor(cli): remove plan followup question fallback
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