feat(pi-acp): opt-in persistent sessions via PI_SESSION_DIR#1672
Open
khaled-mansour-zid wants to merge 1 commit into
Open
feat(pi-acp): opt-in persistent sessions via PI_SESSION_DIR#1672khaled-mansour-zid wants to merge 1 commit into
khaled-mansour-zid wants to merge 1 commit into
Conversation
The Pi ACP adapter hardcodes `SessionManager.inMemory(params.cwd)` for every new session, so nothing is written to disk and the whole conversation is lost when the adapter process restarts (e.g. an actor sleep/eviction). Pi already ships `SessionManager.continueRecent(cwd, sessionDir)` — persist + resume the most recent session — but there's no way for an embedder to select it without patching the adapter source. Add `resolveSessionManager()`: when the embedder provides a session directory via the `PI_SESSION_DIR` env var, use `continueRecent` (persist under that dir and resume); otherwise keep the current in-memory behavior. Env-driven and additive — no behavior change unless `PI_SESSION_DIR` is set (same pattern as the `SECURE_EXEC_FRAME_TIMEOUT_MS` override in rivet-dev#1641). The helper is exported and unit tested with a fake SessionManager (the real newSession path needs the Pi SDK). Co-authored-by: Khaled Mansour <khaled.mansour@zid.sa>
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.
feat(pi-acp): opt-in persistent sessions via
PI_SESSION_DIRProblem
registry/agent/pi/src/adapter.tshardcodesSessionManager.inMemory(params.cwd)for every new session. In-memory means pi never writes the session
.jsonl, so theentire conversation is lost the moment the adapter process restarts (actor
sleep/eviction, crash, redeploy). There is no way for an embedder to get persistent,
resumable sessions short of patching the adapter.
Pi already provides the other half —
SessionManager.continueRecent(cwd, sessionDir)persists the session under
sessionDirand resumes the most recent one. It justisn't reachable from the ACP adapter.
Change
Add
resolveSessionManager(SessionManager, cwd, env): ifPI_SESSION_DIRis set inthe session env, use
continueRecent(cwd, PI_SESSION_DIR); otherwiseinMemory(cwd).Wire the
newSessioncall site to it.the embedder sets
PI_SESSION_DIR.SECURE_EXEC_FRAME_TIMEOUT_MSoverride (feat(runtime-core): env-override the native sidecar frame timeout #1641).resolveSessionManageris exported and unit-tested with a fakeSessionManager (persist branch, default branch, blank-value guard); the real
newSessionpath needs the Pi SDK so the selection logic is tested directly.Verification
resolveSessionManagerlogic verified for all three branches;check-types+ thetests/*.test.mjsnode:test suite run in CI.