Pi: cold-attach native-store resolution, race fix, E2E coverage - #1173
Merged
Conversation
GetSessionDir / ResolveSessionFile previously returned the per-repo .entire/tmp/pi/ cache, which is only populated after the agent_end hook copies the live JSONL there. That meant `entire session attach <id>` for a Pi session that was never hooked (or whose hook capture failed) couldn't find the transcript at all — defeating attach's documented "hooks failed / weren't installed" recovery use case. Move resolution to Pi's native store at <piHome>/sessions/<encoded-repo-path>/<timestamp>_<id>.jsonl, with the encoded path matching Pi's own naming scheme (/Users/foo/repo → --Users-foo-repo--). Honor PI_CODING_AGENT_DIR (Pi's own override) and add ENTIRE_TEST_PI_SESSION_DIR for test isolation. Add GetSessionBaseDir so attach's cross-project fallback in searchTranscriptInProjectDirs can scan sibling project subdirs. ResolveSessionFile now globs *_<id>.jsonl and returns the most recent timestamp match; absolute paths from hook payloads still pass through unchanged. The .entire/tmp/pi/ cache stays as a hook-internal detail — captureTranscript writes there, and the framework records the recorded path as SessionRef in checkpoint metadata, so subsequent operations on hooked sessions go through the recorded path rather than re-resolving via GetSessionDir. Rename the cache constant to piHookCacheSubdir (lifecycle.go) to make the agent-public-API vs hook-internal split obvious. The trade-off vs the prior choice: AgentForTranscriptPath ownership matching now matches live Pi paths and misses cached paths, instead of the inverse. Cached paths are only reachable via Pi's own hook flow, where caller-agent-type fallback already resolves ownership correctly; live-path matching is the more important case because that's the one cold attach surfaces. Verified end-to-end: a Pi session created with no Entire hooks installed is now resolvable via `entire session attach <id>` against PR #1170's branch (transcript_path resolves to the live JSONL in ~/.pi/agent/sessions/, token usage parses correctly). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: ee3bb4d600af
session_shutdown previously emitted a SessionEnd lifecycle event, which races with agent_end during normal Pi session teardown: * Pi fires both events at session shutdown * The TypeScript extension's pi.on(...) handlers each dispatch an `entire hooks pi <event>` child process via execFile * Both child processes run concurrently in Node's event loop; their startup ordering decides which event reaches Entire first * When session_shutdown wins, the SessionEnd transition moves the session to "ended" and condenses it before agent_end can save its linkable checkpoint * prepare-commit-msg then sees a "fully-condensed ended session" and doesn't add a checkpoint trailer to the user's commit * entire/checkpoints/v1 never advances on the commit Reproduced reliably on a TestSingleSessionManualCommit/pi run before the fix (37s timeout failure); same test passes in 8.5s after. session_shutdown is now cleanup-only — clears the cached session ID and emits no lifecycle event. agent_end is the source of truth for "turn complete" and, for any single-turn `pi -p` invocation, effectively "session over" too. SessionEnd is left for the framework to derive from idle timeout or the next SessionStart's stale-state cleanup. Update GetSupportedHooks (drop HookSessionEnd) and the header comment to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 09a0086f82a2
Register the Pi adapter so the e2e harness picks it up via E2E_AGENT=pi. Same shape as the existing in-tree adapters: shells out to `pi -p <prompt> --no-skills --no-prompt-templates --no-themes` in non-interactive mode and uses the standard tmux helper for interactive sessions. Drives the regression test for both the cold-attach native-store fix (committed earlier) and the agent_end / session_shutdown race fix — TestSingleSessionManualCommit/pi exercises the full hooked flow against a real `pi` binary. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 9329fbd003b5
List Pi alongside the other native agents in the agent-package description, the per-agent test:e2e flag table, the e2e agent enumeration, and the E2E_AGENT env var values. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 67bcf0cbbd57
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the Pi agent integration by enabling cold-attach transcript resolution directly from Pi’s native session store, removing a lifecycle race caused by session_shutdown, adding an E2E Pi adapter, and documenting Pi support.
Changes:
- Resolve Pi session transcripts from Pi’s native
~/.pi/agent/sessions/...store (with test/home overrides) to support cold attach and cross-project fallback scanning. - Make
session_shutdowncleanup-only (noSessionEndemission) to avoid races withagent_endthat can prevent checkpoint trailers from being written. - Add Pi to the E2E agent registry and update
CLAUDE.mdto include Pi in agent/test documentation.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
e2e/agents/pi.go |
Adds Pi as an E2E agent runner (E2E_AGENT=pi). |
cmd/entire/cli/agent/pi/session_resolve_test.go |
Adds unit tests for Pi repo-path encoding and session file resolution/globbing behavior. |
cmd/entire/cli/agent/pi/pi.go |
Switches GetSessionDir/ResolveSessionFile to Pi native-store semantics; adds base-dir support for cross-project search. |
cmd/entire/cli/agent/pi/lifecycle.go |
Removes SessionEnd emission from session_shutdown and clarifies hook-cache vs native-store responsibilities. |
cmd/entire/cli/agent/pi/lifecycle_test.go |
Updates tests to assert cleanup-only behavior for session_shutdown and cache clearing. |
CLAUDE.md |
Documents Pi among supported agents and E2E usage flags/values. |
dipree
approved these changes
May 10, 2026
Pi's encoded session-dir name (--Users-foo-repo--) was produced by replacing only os.PathSeparator. On Windows that meant '\\' was substituted but '/' leaked through — git rev-parse --show-toplevel returns forward slashes regardless of platform, so the encoded name still contained directory separators and pi.GetSessionDir produced a nested path instead of a single subdirectory under <piHome>/sessions/. Result: cold-attach lookups missed, even though the live JSONL was on disk. Replace both '/' and '\\' uniformly so the encoding is identical across hosts. filepath.ToSlash isn't enough on its own — it only maps the host's separator. Add Windows-style fixtures to the encoding test (forward and back slashes, with and without trailing separator). Reported by Copilot review on #1173. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: c2bfe61c1a96
The Pi e2e adapter previously inherited the user's real ~/.pi/agent state, so parallel E2E runs could interleave session writes and a test could read transcripts from another test's session if encoded repo paths happened to overlap. Mirrors what codex.go does with CODEX_HOME. Per-test home directory under $XDG_CACHE_HOME/entire-e2e/pi-home-*, exported as PI_CODING_AGENT_DIR for both RunPrompt and StartSession (the tmux helper now wraps the binary in `env PI_CODING_AGENT_DIR=…` so the var propagates into the agent process). Cleanup runs via defer (RunPrompt) or s.OnClose (StartSession). Pi stores OAuth tokens at <piHome>/auth.json — a fresh isolated home breaks auth. seedPiHome symlinks auth.json + settings.json from the real home (best-effort) so the user's existing OAuth tokens flow through; tests with explicit ANTHROPIC_API_KEY / OPENAI_API_KEY env inherit those regardless. Sessions still land in the isolated home's sessions/ subdir, which is the actual hermeticity goal. PiSession exposes the home so future fixtures can introspect or clean it. TestSingleSessionManualCommit/pi passes against the isolated home in 8.9s (vs the previous 8.5s on the shared home). Reported by Copilot review on #1173. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: f2edd6ee34e2
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
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.
Stacks on #1170.
1. Resolve Pi sessions from native store — cold attach
GetSessionDir/ResolveSessionFilereturned the per-repo.entire/tmp/pi/cache, which is only populated afteragent_end.entire session attach <id>for a never-hooked session couldn't find the transcript — defeating attach's documented recovery case.Now: native store at
<piHome>/sessions/<encoded-repo>/<ts>_<id>.jsonl, encoded path matches Pi's own scheme (/Users/foo/repo→--Users-foo-repo--).ResolveSessionFileglobs*_<id>.jsonl, latest timestamp wins; absolute paths still pass through. NewGetSessionBaseDirenables cross-project fallback. HonorsPI_CODING_AGENT_DIR; addsENTIRE_TEST_PI_SESSION_DIRfor test isolation..entire/tmp/pi/stays as hook-internal cache (renamed constantpiHookCacheSubdirin lifecycle.go to make the split obvious).Trade-off:
AgentForTranscriptPathnow matches live Pi paths and misses cached paths instead of the inverse. Cached paths are only reachable via Pi's own hooks, where caller-agent-type fallback handles ownership correctly; live-path matching is what cold attach surfaces.2. Drop
SessionEndfromsession_shutdown— race fixSurfaced by the new e2e suite. The TS extension fires
agent_endandsession_shutdownasync via separateexecFilechild processes; they race. Whensession_shutdownwins, the emittedSessionEndtransitions the session toendedand condenses it beforeagent_endcan save its linkable checkpoint —prepare-commit-msgthen sees "fully-condensed ended" and the commit gets no trailer.Repro on
TestSingleSessionManualCommit/pi: before, 37.7s timeout; after, 8.5s pass.Fix:
session_shutdownis cleanup-only (clears cached session ID, emits nothing).agent_endis the source of truth for "turn complete" — and effectively "session over" for any single-turnpi -p.3. E2E agent registry
e2e/agents/pi.gosoE2E_AGENT=piworks. #1170's canary used Vogon, not pi — the hooked flow had no integration coverage. This adapter surfaced the race in #2.4. CLAUDE.md
Pi added to the agent enumeration, test:e2e flags, and
E2E_AGENTvalues.Verification
mise run lintclean; all unit tests pass (40+ existing + 9 new)pi -psession resolves the live JSONL and produces a valid trailerTestSingleSessionManualCommit/pipasses in 8.5s with the race fixTest plan
mise run test:e2e --agent pi TestSingleSessionManualCommitpi -p helloin an un-enabled repo, thenentire session attach --agent pi <id>🤖 Generated with Claude Code