Skip to content

fix(remote-workspace): observe Pi prompt failures before acknowledgement - #761

Open
luvs01 wants to merge 1 commit into
devfrom
fix/pi-prompt-rejection-lifetime
Open

luvs01 wants to merge 1 commit into
devfrom
fix/pi-prompt-rejection-lifetime

Conversation

@luvs01

@luvs01 luvs01 commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner

Summary

A Pi child can fail while its prompt acknowledgement is still pending. The runtime created a separate turn-settlement promise but did not observe it until after awaiting that acknowledgement. A process exit, malformed RPC output, or early extension error could therefore reject both promises while only the command rejection was handled. Even a caller that caught handle.prompt() still received a process-level unhandledRejection.

Observe prompt admission and turn settlement concurrently with Promise.all. Either failure rejects the prompt immediately, and success still requires both a successful prompt response and the turn's settlement event. Clear only the active settlement owned by this prompt. Process launch, remote permissions, tool admission, cleanup and replay policy are unchanged.

Add seven isolated synthetic RPC scenarios covering child exit, malformed output, extension errors before/after acknowledgement, explicit rejection, and successful completion in either order. The fixture records process-level unhandled rejections without changing the test runner's rejection policy. Update the architecture contract and English/Korean guide.

This is a pre-existing application lifetime bug reproduced on official Bun 1.4.3 and 1.4.4, not a newly introduced Bun 1.4.4 regression. The daemon's crash guard may absorb an unhandled rejection; this PR does not claim that every occurrence terminates the daemon.

Verification

Base: fork dev at c4d4bb2da2162e23c314befafc5ec104292902a0. Local execution: Linux x64. Tests use the repository's normal preload/shared lock in the host PID namespace.

Reproduction and regression

On untouched base, a real synthetic child exiting with code 7 before acknowledging a prompt produced both:

  • caught prompt error: Pi RPC exited with code 7;
  • one unhandled rejection with the same message.

This was reproduced on both Bun versions. Early extension failure and malformed RPC output independently produced the same double-reporting shape on Bun 1.4.4. With this patch, the prompt still rejects with the relevant error and the fixture records zero unhandled rejections.

The fixture accepts an absolute source path, so the exact same probe can target a baseline worktree:

bun tests/fixtures/pi-prompt-lifetime.ts /absolute/base/src/remote-control/workspace-pi-runtime.ts exit-before-ack
bun tests/fixtures/pi-prompt-lifetime.ts /absolute/base/src/remote-control/workspace-pi-runtime.ts extension-before-ack
bun tests/fixtures/pi-prompt-lifetime.ts /absolute/base/src/remote-control/workspace-pi-runtime.ts malformed-before-ack

Commands and results

Command Result
bun run test --parallel=1 ./tests/clients/remote-workspace-cli-runtimes.test.ts — pinned Bun 1.4.3 8 passed, 1 opt-in real-Pi skip, 0 failed; 22 assertions
bun test ./tests/clients/remote-workspace-cli-runtimes.test.ts — official Bun 1.4.4 8 passed, 1 opt-in real-Pi skip, 0 failed; 22 assertions
bun run test:changed --parallel=2 ./tests/clients/ ./tests/cli/ ./tests/ci-workflows/docs-link-targets.test.ts ./tests/ci-workflows/file-size-ratchet.test.ts — pinned Bun 1.4.3 1,396 passed, 33 opt-in/platform skips, 0 failed; 8,040 assertions across 60 files
bun run test --parallel=2 ./tests/ci-workflows/file-size-ratchet.test.ts ./tests/ci-workflows/docs-link-targets.test.ts ./tests/clients/remote-workspace-sessions.test.ts ./tests/clients/remote-workspace-management.test.ts — explicit source-reading and session oracles 50 passed, 0 failed; 247 assertions across 4 files
bun run typecheck; bun run structure:check; bun run privacy:scan; git diff --check --cached Passed
In docs-site/: bun install --frozen-lockfile --offline; OPENCODEX_DOCS_OFFLINE=1 bun run build Frozen install unchanged; 593 pages built, 84,861 internal links checked

The focused subprocess fixture is explicitly invoked because changed-mode import analysis cannot follow the source filename passed to a child. The broad repository suite was not rerun under the documented constrained-worktree/resource exception; validation covers the changed runtime and connected client/CLI paths plus explicit session, documentation and size guards. Exact-head hosted CI remains separate. Native Windows/macOS execution, real Pi/provider execution, and live CLI confinement are unverified; opt-in skips are not counted as passing evidence.

Independent source/test/documentation review by a separate agent found no material issue. This is an agent review, not a maintainer approval or hosted bot review.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. The patch changes promise observation only; it does not change credentials, process argv, approved roots, remote capabilities, tool execution, or automatic replay. Privacy scan passed.

Co-authored-by: luvs01 27862058+luvs01@users.noreply.github.com

Summary by CodeRabbit

  • Bug Fixes

    • Remote workspace sessions now report prompt-start failures promptly if Pi exits or an extension encounters an error, without waiting for a turn acknowledgment.
    • A prompt is considered successful only after both Pi accepts it and the turn settles, helping prevent failures from going unreported.
  • Documentation

    • Updated remote workspace guidance in English and Korean to explain how prompt failures are reported.

Observe prompt admission and turn settlement concurrently so early child failures cannot escape as unhandled rejections.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between c4d4bb2 and 1d56a1d.

📒 Files selected for processing: 6
  • docs-site/src/content/docs/guides/remote-workspace.md
  • docs-site/src/content/docs/ko/guides/remote-workspace.md
  • src/remote-control/workspace-pi-runtime.ts
  • structure/remote-workspace.md
  • tests/clients/remote-workspace-cli-runtimes.test.ts
  • tests/fixtures/pi-prompt-lifetime.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

PiRpcProcess.prompt now observes prompt acceptance and turn settlement concurrently. Either operation can reject the prompt without waiting for the other. Tests and documentation cover this behavior.

Changes: Pi prompt lifetime

Layer / File(s) Summary
Concurrent prompt observation and validation
src/remote-control/workspace-pi-runtime.ts, tests/fixtures/pi-prompt-lifetime.ts, tests/clients/remote-workspace-cli-runtimes.test.ts, docs-site/src/content/docs/guides/remote-workspace.md, docs-site/src/content/docs/ko/guides/remote-workspace.md, structure/remote-workspace.md
prompt observes acceptance and settlement concurrently, and clears activeSettle in finally only when it still matches the prompt. The subprocess fixture and parameterized test cover seven RPC modes and check for unhandled errors. The documentation describes failure reporting before Pi acknowledges a turn.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Suggested reviewers: lidge-jun, ingwannu

Merge Risk: ⚪ Minimal · up to 1d56a

The changed tests cover the reported early-failure regression. No actionable issue currently blocks merging after normal checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: observing Pi prompt failures before acknowledgement in the remote-workspace runtime.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3 files. (3 skipped: 3 unsupported.)

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 23, 2026
@luvs01

luvs01 commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner Author

No description provided.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant