feat: add persistent RPC subprocess mode#18
Closed
h0rv wants to merge 1 commit into
Closed
Conversation
WalkthroughThis PR adds persistent RPC process support to pi.nvim, allowing a long-lived subprocess to remain active between editor prompts. The implementation introduces a new configuration schema for Sequence Diagram(s)sequenceDiagram
participant User as NeovimUser
participant Main as pi.init (M.run)
participant RPC as pi.rpc
participant Child as "pi --mode rpc"
participant Runner as pi.runner
User->>Main: invoke run (prompt)
Main->>RPC: rpc.is_running() / rpc.start(cmd) (if persistent)
Main->>RPC: rpc.prompt(payload, handlers)
RPC->>Child: write payload (stdin)
Child->>RPC: stdout lines (events)
RPC->>Main: handlers.on_event / handlers.on_error
Note right of Main: or, when non-persistent:
Main->>Runner: runner.start(session, payload)
Runner->>Child: spawn process (per-session)
Child->>Runner: stdout lines -> on_event
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lua/pi/rpc.lua`:
- Around line 84-87: In M.stop(), avoid touching the internal
process._state.stdin; close stdin using the public API by calling
process:write(nil) (or the equivalent public close/write-nil on the process
handle) instead of accessing process._state.stdin. In M.prompt(), add a
reentrancy guard that checks is_busy at the start and returns (or queues) if
true before sending the {"type":"new_session"} message and before swapping
current_handlers so a concurrent call cannot orphan handlers; ensure M.abort()
still observes is_busy and can cancel the active session. Reference M.stop,
M.prompt, M.abort and the current_handlers/is_busy symbols when making the
changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab303433-244f-437d-8350-a49a4460c8ab
📒 Files selected for processing (7)
README.mdlua/pi/config.lualua/pi/init.lualua/pi/process.lualua/pi/rpc.lualua/pi/runner.luaplugin/pi.nvim.lua
h0rv
force-pushed
the
persistent-rpc
branch
2 times, most recently
from
7d5335f to
6e5940d
Compare
Add optional long-lived pi --mode rpc subprocess that replaces the per-request process spawn when rpc.persistent = true. New lua/pi/rpc.lua owns the persistent process handle and exposes start/stop/restart/is_running/send/prompt/abort. Sends new_session before each prompt to reset in-memory session state. New lua/pi/process.lua extracts shared primitives (feed_lines, spawn) used by both runner.lua and rpc.lua, removing duplicated newline-splitting and spawn boilerplate. New config: rpc.persistent (default false), rpc.start (default lazy). New commands: PiWarm, PiStop, PiRestart, PiStatus. Persistent mode cancellation sends RPC abort instead of SIGTERM. Process cleaned up on VimLeavePre.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lua/pi/rpc.lua`:
- Around line 15-30: feed_stdout is currently treating undecodable stdout as an
error by calling current_handlers.on_error, which forces persistent sessions to
finish; change feed_stdout so undecodable lines do NOT invoke
current_handlers.on_error directly — instead, if current_handlers provides a new
non-fatal hook (e.g., on_stdout_unparseable or on_raw_stdout) call that with the
raw line, otherwise silently ignore the line; update feed_stdout (and any
callers of runner.decode_event) to prefer current_handlers.on_event for decoded
events and only mark is_busy/handle done/error for normalized events, leaving
session-finishing logic (finish_session) untouched.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a148a9a-a27b-4294-b188-a1f26c022379
📒 Files selected for processing (7)
README.mdlua/pi/config.lualua/pi/init.lualua/pi/process.lualua/pi/rpc.lualua/pi/runner.luaplugin/pi.nvim.lua
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (5)
- plugin/pi.nvim.lua
- lua/pi/config.lua
- lua/pi/process.lua
- lua/pi/runner.lua
- lua/pi/init.lua
Comment on lines
+15
to
+30
| local function feed_stdout(chunk) | ||
| stdout_tail = proc.feed_lines(stdout_tail, chunk, function(line) | ||
| local event = runner.decode_event(line) | ||
| if event then | ||
| local normalized = runner.normalize(event) | ||
| if normalized and current_handlers and current_handlers.on_event then | ||
| current_handlers.on_event(normalized) | ||
| if normalized.type == "done" or normalized.type == "error" then | ||
| is_busy = false | ||
| end | ||
| end | ||
| elseif current_handlers and current_handlers.on_error then | ||
| current_handlers.on_error("unparseable RPC event: " .. line) | ||
| end | ||
| end) | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect runner.lua: exports of decode_event/normalize and how it treats undecodable stdout lines.
fd 'runner.lua' --exec sh -c 'echo "== {} =="; cat -n "{}"'
echo "== decode_event / normalize usage =="
rg -nP --type=lua -C3 '\b(decode_event|normalize)\b'Repository: pablopunk/pi.nvim
Length of output: 6791
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect persistent call path wiring in init.lua
fd 'init.lua' --exec sh -c 'echo "== {} =="; cat -n "{}"'
# Inspect rpc.lua around M.start/M.prompt/on_error/finish_session interactions
fd 'rpc.lua' --exec sh -c 'echo "== {} =="; rg -n --context=3 -S "finish_session|on_error|current_handlers|prompt|abort|new_session" "{}"; echo "---"; sed -n "1,260p" "{}"'Repository: pablopunk/pi.nvim
Length of output: 27027
Handle undecodable RPC stdout without failing the session
- Persistent mode (lua/pi/rpc.lua
feed_stdout) treats any stdout line thatrunner.decode_eventcan’t parse ason_error, and lua/pi/init.lua’son_errorends the session withfinish_session(..., "error", ...). - Non-persistent mode (lua/pi/runner.lua) ignores undecodable stdout lines because the stdout path passes
on_error=niltofeed_stream.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lua/pi/rpc.lua` around lines 15 - 30, feed_stdout is currently treating
undecodable stdout as an error by calling current_handlers.on_error, which
forces persistent sessions to finish; change feed_stdout so undecodable lines do
NOT invoke current_handlers.on_error directly — instead, if current_handlers
provides a new non-fatal hook (e.g., on_stdout_unparseable or on_raw_stdout)
call that with the raw line, otherwise silently ignore the line; update
feed_stdout (and any callers of runner.decode_event) to prefer
current_handlers.on_event for decoded events and only mark is_busy/handle
done/error for normalized events, leaving session-finishing logic
(finish_session) untouched.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add optional long-lived
pi --mode rpcsubprocess that replaces the per-request process spawn whenrpc.persistent = true.=== Config
=== New commands
PiWarm— start the persistent processPiStop— stop itPiRestart— restart itPiStatus— show whether it's running=== Architecture
lua/pi/rpc.lua: owns the persistent process handle, exposesstart/stop/restart/is_running/send/prompt/abort. Sendsnew_sessionbefore each prompt.lua/pi/process.lua: shared primitives (feed_lines,spawn) used by bothrunner.luaandrpc.lua.VimLeavePre.=== Compatibility
rpc.persistent = false)process.luahelpers but no behavioral changeSummary by CodeRabbit
New Features
Documentation