[5283] fix(agent): Preserve Pi history after cold resume#5284
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds design documentation for Pi session workspace materialization, including native-history verification, canonical replay, durable transcripts, and append-only skill snapshots. It also changes Pi continuity seeding to bypass unverified native session loads and adds tests covering Pi fallback and unchanged non-Pi resume behavior. ChangesSession workspace materialization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
mmabrouk
added
Custom Workflows
Design
documentation
ChatGPT Codex Connector
|
🤖 The AI agent says: Please review two decisions before implementation: (1) only verified native history may use newest-message-only prompting; all uncertain loads replay canonical history, and (2) launch delivery stays split into the verified-load gate, durable transcripts, then append-only skills with no Claude reconciler. |
| naming. It uses the agent directory for credentials, private settings, global resources, custom | ||
| models, and native sessions. | ||
|
|
||
| Project `.pi/skills` and `.agents/skills` are trust-gated. RPC mode cannot display a trust prompt, |
There was a problem hiding this comment.
explain this more what are trusted gated you did not define that
There was a problem hiding this comment.
Defined now in both places (commit 5eac8cb). Trust gating is Pi's project consent mechanism: the first time Pi runs in a cwd it asks whether to trust that project, and stores the decision in <agent-dir>/trust.json keyed by the canonical cwd path. Trust authorizes loading the project's own in-cwd configuration: .pi/settings.json, executable .pi/extensions, .pi/skills, .agents/skills, prompts, themes, and SYSTEM.md. RPC mode cannot show the prompt, so the default ask policy resolves to untrusted and none of these load. Added as a term in context.md and expanded in research.md, including the nuance that an installed extension can answer the trust request programmatically, so the Agenta extension must never do that.
|
|
||
| Project `.pi/skills` and `.agents/skills` are trust-gated. RPC mode cannot display a trust prompt, | ||
| and the default `ask` policy leaves them unloaded. Trusting the whole cwd would also authorize | ||
| project settings and executable extensions. |
There was a problem hiding this comment.
arent the project settings and extensions outside of the cwd
There was a problem hiding this comment.
No, these specific ones are inside the cwd: project settings are <cwd>/.pi/settings.json and project extensions are <cwd>/.pi/extensions (verified in Pi 0.80.6 source: package-manager.js:710,1924). What lives outside the cwd is the agent directory (PI_CODING_AGENT_DIR): auth.json, the global settings.json, models.json, and sessions. Pi has both a global and a project settings file, and only the project one is trust-gated. research.md now spells out the split (commit 5eac8cb).
mmabrouk
added
lgtm
Launch gate ships without any pi-acp change: every cold Pi load starts unverified and replays; verified loads return via Phase 0b (pi-acp preferred, local runner header check as interim). Durable transcripts lead with Pi's settings.json sessionDir override, which pi-acp's scan paths also honor. Snapshots are immutable by convention. Defines trust gating and pins the upstream behaviors the gate defends against. Claude-Session: https://claude.ai/code/session_01XhENr63WL9npkKrJGnzDc1
Review updates (commit 5eac8cb)Fact-checked every claim in research.md against the runner source, pinned Pi 0.80.6, and 1. The launch gate no longer depends on changing 2. Durable transcripts now lead with Pi's supported session-directory override. Traced end to end: Pi resolves 3. Snapshot immutability is stated as convention, not enforcement. The cwd is agent-writable and the completion record does not hash contents; accepted threat model, documented in design.md. 4. QA pins the upstream behaviors the gate defends against: Pi Also defined trust gating in context.md/research.md and clarified that project settings/extensions live inside the cwd (see inline replies). |
|
🤖 The AI agent says: Implementation has started on the approved launch gate. I agree with the conservative replay decision. I am treating the sandbox-agent ID substitution as Phase 0b cleanup because the launch fix will bypass cold Pi native loading entirely, create a clean Pi session, and replay canonical history. The final handoff will include focused tests and a live port 8280 cold-resume reproduction. |
mmabrouk
added
the
implementing
ChatGPT Codex Connector
mmabrouk
changed the title
mmabrouk
added
needs-review
dosubot
Bot
added
size:XL
|
🤖 The AI agent says: Phase 0 is implemented, pushed, and ready for review.
@coderabbitai review |
|
✅ Action performedReview finished.
|
|
🤖 The AI agent says: Final-head CI is green: manually dispatched workflow run 29285246373 passed runner unit tests and typecheck, runner acceptance tests, runner integration tests, services tests, and SDK tests on commit The automatic unit-test event had skipped because the push occurred while the PR was draft. The separate preview and Railway workflow entries that show red are workflow-file startup failures with zero jobs, not branch test failures; they are pre-existing repository workflow configuration and are not required checks. |
![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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e8ec0a8c-7fb1-492d-adf5-6d49aa39e018
📒 Files selected for processing (9)
docs/design/agent-workflows/projects/session-workspace-materialization/README.mddocs/design/agent-workflows/projects/session-workspace-materialization/context.mddocs/design/agent-workflows/projects/session-workspace-materialization/design.mddocs/design/agent-workflows/projects/session-workspace-materialization/plan.mddocs/design/agent-workflows/projects/session-workspace-materialization/qa.mddocs/design/agent-workflows/projects/session-workspace-materialization/research.mddocs/design/agent-workflows/projects/session-workspace-materialization/status.mdservices/runner/src/engines/sandbox_agent.tsservices/runner/tests/unit/sandbox-agent-qa-transcript-replay.test.ts
| cold start, cold resume, or changed fingerprint | ||
| -> mount/create cwd and private transcript storage | ||
| -> materialize or validate the current skill snapshot | ||
| -> attempt verified native load | ||
| -> verified: send newest message only | ||
| -> anything else: create clean native session and send canonical replay |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the documentation consistent with the shipped cold-Pi gate.
Phase 0 does not verify or resume cold Pi sessions; it treats them as unverified, invalidates stale continuity, creates a clean native session, and replays canonical history. Verified native loads belong to Phase 0b.
docs/design/agent-workflows/projects/session-workspace-materialization/design.md#L146-L151: replace “attempt verified native load” in the cold-path sequence with the implemented clean-session replay flow.docs/design/agent-workflows/projects/session-workspace-materialization/README.md#L3-L7: describe the immediate gate as conservative bypass/replay rather than native-load verification.
📍 Affects 2 files
docs/design/agent-workflows/projects/session-workspace-materialization/design.md#L146-L151(this comment)docs/design/agent-workflows/projects/session-workspace-materialization/README.md#L3-L7
|
🤖 The AI agent says: Closing this PR because its Phase 0 fallback changed every cold Pi continuation to canonical replay. That is not the approved launch behavior. The replacement work will persist Pi transcripts first, then stack the skill-folder fix, while preserving healthy native resume behavior. |
1 participant
Problem
A cold local Pi continuation could report a successful native resume after its transcript had disappeared. The runner then sent only the newest user message, so Pi silently lost the earlier conversation. The same investigation found that Pi skill paths currently point into a throwaway per-run agent directory.
The immediate correctness bug is caused by identity-free loading: pinned
pi-acp@0.0.29does not return the loaded native session ID, while the compatibility wrapper substitutes the requested ID. That makes an unverified load look successful.Refs #5283.
What changes
session/loadwhile loaded identity cannot be verified.outcome=unverified replay=canonicalwithout transcript content.Before and after
Before: cold Pi load appeared successful, the runner sent only the latest message, and missing native history became user-visible conversation loss.
After: every cold Pi load is conservative until identity evidence exists. Pi receives the full canonical transcript, so correctness does not depend on the temporary native transcript surviving.
Verification
pnpm exec vitest run tests/unit/sandbox-agent-qa-transcript-replay.test.ts -t "cold Pi native-history fallback": 2 passed.pnpm exec vitest run tests/unit/continuation.test.ts: 7 passed.pnpm run typecheck: passed.Local QA
Tested against the live local deployment on port 8280 with
pi_core, local sandboxing, and the self-managed Codex subscription:BETA-9from turn 0.ALPHA-7from turn 0.outcome=unverified replay=canonical, a fresh native session create, and successful continuity sync.What to review
plan.turnText.