feat: add durable cloud Pi runtime sessions - #3719
Conversation
|
😎 Merged successfully - details. |
|
React Doctor found 7 issues in 2 files · 7 warnings. 7 warnings
Reviewed by React Doctor for commit |
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/ui/src/features/task-detail/components/TaskDetail.tsx:58-63
**Resumed Run Keeps Stale ID**
When a completed cloud Pi run is resumed, `taskId` stays the same while `latest_run.id` changes. This ref is only refreshed for a new task, so `PiSessionView` remains connected to the terminal run and subsequent commands can be routed to that stale run instead of the resumed one.
### Issue 2 of 3
packages/agent/src/server/pi-agent-server.ts:528-529
**Live Session Read Can Truncate**
The synchronization timer reads the JSONL file while the Pi child process can still be writing it. If the read occurs during a rewrite or between appending a record and its final bytes, the incomplete content is promoted as an authoritative revision and a later restore can load a truncated session.
### Issue 3 of 3
packages/agent/src/server/pi-agent-server.ts:440-475
**Delivery Outlives Session Replacement**
An in-flight user-message delivery retains the runtime selected before awaiting the RPC call, but replacing `this.session` does not cancel that delivery. If the run is recreated while a delivery is pending, the message can complete against the old Pi process and disappear from the newly active session.
Reviews (1): Last reviewed commit: "feat: add durable cloud Pi runtime sessi..." | Re-trigger Greptile |
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
a6ede65 to
f56b0d0
Compare
|
Stress-tested this a bit in cloud runs (replicating this also on the backed PR):
|
…-task-sessions # Conflicts: # packages/ui/src/features/sessions/components/chat-thread/ChatThread.tsx # pnpm-lock.yaml
This reverts commit 7a3e023.
|
✅ Visual changes approved by @jonathanlab — baseline updated in 12 new, 12 removed. |
12 updated, 12 removed Run: 9e2a836d-06fa-4eb0-80ab-9cafd1241ac9 Co-authored-by: jonathanlab <32547391+jonathanlab@users.noreply.github.com>
🦔 ReviewHog reviewed this pull requestFound 3 must fix, 15 should fix, 9 consider. Published 27 findings (view the review). |
|
ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
There was a problem hiding this comment.
ReviewHog Report
Business logic
Issues: 1 issue
Files (9)
packages/agent/src/pi/rpc-transport.tspackages/agent/src/pi/remote-rpc-client.tspackages/agent/src/pi/rpc-client.tspackages/agent/src/pi/rpc-host.tspackages/agent/src/pi/types.tspackages/agent/src/pi/queue-persistence.tspackages/agent/src/pi/runtime.tspackages/agent/package.jsonpackages/agent/tsup.config.ts
What were the main changes
- Adds browser-safe piRpcCommandSchema/parsePiRpcResponse transport and a RemotePiRpcClient adapter that maps typed Pi RPC methods onto it
- Adds IPC-based queue get/clear host requests to SecurePiRpcClient and rpc-host.ts, plus JSONL-persisted queue restoration
- PiRuntime now tracks in-flight user-message ids, emits direct-bash tool-call events, and exposes sendCommand for generic RPC dispatch
- Publishes new package export paths (pi/rpc-transport, pi/remote-rpc-client, pi/model-catalog) and switches server/bin build to ESM
Business logic
Issues: 6 issues
Files (6)
packages/agent/src/pi/conversation/translatePiConversation.tspackages/agent/src/pi/conversation/translatePiMessage.tspackages/agent/src/pi/model-catalog.tspackages/harness/src/extensions/posthog-provider/models.tspackages/harness/src/extensions/posthog-provider/provider.tspackages/harness/src/runtime.ts
What were the main changes
- Adds direct-bash live translation (beginDirectBash/completeDirectBash/failDirectBash) and queue_update event translation
- Generic tool result content fallback and inline image fileName support in translatePiMessage
- New resolvePosthogPiModelCatalog/fetchPosthogPiModelCatalog exposing PostHog gateway models with thinking levels for Pi
- Harness runtime migrated to pi-ai CredentialStore/ModelRuntime APIs replacing AuthStorage/modelRegistry
Feature
Issues: 6 issues
Files (5)
packages/agent/src/server/pi-agent-server.tspackages/agent/src/server/bin.tspackages/agent/src/server/types.tspackages/agent/src/server/event-stream-sender.tspackages/agent/src/posthog-api.ts
What were the main changes
- New PiAgentServer: cloud-hosted Pi runtime with JWT-authed SSE/command endpoints, session restore from TaskSession storage, and pi_run_started signaling
- Persists translated Pi conversation events as pi_event log entries and syncs native JSONL sessions back to Django on settle/shutdown/crash
- bin.ts wires POSTHOG_AGENT_RUNTIME/SANDBOX_ID env vars to select PiAgentServer vs AgentServer and resolve the packaged Pi RPC host path
- posthog-api.ts adds getTaskSession/downloadTaskSession/syncTaskSession for durable TaskSession revision storage
Business logic
Issues: 2 issues
Files (4)
packages/core/src/pi-runtime/piSessionController.tspackages/core/src/pi-runtime/piSessionStore.tspackages/core/src/pi-runtime/pi-runtime.module.tspackages/core/src/pi-runtime/piRunner.ts
What were the main changes
- PiSessionController rewritten to route through PiSessionProvider, track session versions, and unify local/cloud session lifecycle
- Adds retry/restart, queue capture-and-restore, optimistic user messages, auth-restoration waiting, and structured PiSessionError/PiOperationError reporting
- Session store gains queue snapshot, stats, cloudStatus, thinkingLevels and richer connection/error state
Business logic
Issues: 2 issues
Files (3)
packages/core/src/pi-runtime/cloudPiSessionClient.tspackages/core/src/pi-runtime/piSessionProvider.tspackages/core/src/pi-runtime/piSessionUsage.ts
What were the main changes
- New CloudPiSessionClient adapts cloud task snapshot/log updates into RemotePiRpcClient RPC responses, with terminal-run read-only fallback and pi_run_started readiness
- RoutingPiSessionProvider selects cloud vs local Pi session factory based on task.latest_run.environment
- piSessionUsage.ts maps Pi session stats into shared ContextUsage for the chat footer
Infrastructure
Issues: 3 issues
Files (16)
apps/code/src/renderer/di/bindings.tsapps/code/src/renderer/di/container.tsapps/web/src/web-container.tsapps/web/src/web-host-router.tspackages/host-router/package.jsonpackages/host-router/src/cloud-task-client.tspackages/host-router/src/pi-runner.tspackages/host-router/src/pi-session-client.tspackages/host-router/src/pi-session-factory.tspackages/host-router/src/routers/agent.router.tspackages/host-router/src/routers/cloud-task.router.tspackages/host-router/src/routers/pi-session.router.tspackages/workspace-server/src/services/agent/agent.tspackages/workspace-server/src/services/agent/schemas.tspackages/workspace-server/src/services/pi-session/pi-session.tspackages/workspace-server/src/services/pi-session/schemas.ts
What were the main changes
- Replaces TrpcPiSessionClient with TrpcPiSessionFactory (generic RPC pass-through) and adds TrpcCloudTaskClient/TrpcPiRunner shared between desktop and web DI containers
- host-router gains a cloud-task router (context/watch/retry/sendCommand) and slims pi-session.router to rpc/health/getQueue/clearQueue/stop
- workspace-server pi-session.ts replaces per-RPC-method methods with a generic request(command) dispatch, active-request tracking, and safer hot-pool eviction
- agent.ts renames getGatewayModels to getPiModelCatalog using the new model-catalog helper
Frontend
Issues: 6 issues
Files (7)
packages/ui/src/features/pi-sessions/PiQueuedMessagesDock.tsxpackages/ui/src/features/pi-sessions/PiSessionControls.tsxpackages/ui/src/features/pi-sessions/PiSessionView.tsxpackages/ui/src/features/task-detail/components/TaskDetail.tsxpackages/ui/src/features/task-detail/components/TaskInput.tsxpackages/ui/src/features/settings/settingsStore.tspackages/ui/src/shell/GlobalEventHandlers.tsx
What were the main changes
- PiSessionView reworked for cloud reconnect/retry/restart banners, queued-message dock, auth-restoration and usage-limit handling, and follows the live task.latest_run.id
- New PiQueuedMessagesDock renders/edits/removes the Pi steering/follow-up queue
- TaskInput gains a Pi runtime toggle with model/thinking-level selectors sourced from the new Pi model catalog query
- GlobalEventHandlers retries unhealthy cloud Pi sessions on window focus alongside ACP sessions
Frontend
Issues: 1 issue
Files (12)
packages/ui/src/features/sessions/components/CloudInitializingView.tsxpackages/ui/src/features/sessions/components/CloudSessionLifecycle.tsxpackages/ui/src/features/sessions/components/ContextBreakdownPopover.tsxpackages/ui/src/features/sessions/components/SessionView.tsxpackages/ui/src/features/sessions/components/UserMessageAttachments.tsxpackages/ui/src/features/sessions/components/buildConversationItems.tspackages/ui/src/features/sessions/components/chat-thread/ChatThread.tsxpackages/ui/src/features/sessions/components/chat-thread/ChatThreadFooter.tsxpackages/ui/src/features/sessions/components/session-update/ProgressGroupView.tsxpackages/ui/src/features/sessions/components/session-update/QueuedMessageView.tsxpackages/core/src/sessions/contextUsage.tspackages/core/src/sessions/promptContent.ts
What were the main changes
- Extracts CloudConnectionBanner/CloudStreamDisconnectedBanner/ConnectingToAgent into a shared CloudSessionLifecycle component reused by ACP and Pi
- buildConversationItems handles the new progress and queue_update AgentConversationEvent types runtime-neutrally
- ChatThread/ChatThreadFooter accept an externally supplied context-usage override so Pi can report its own stats
- QueuedMessageView's drag handle becomes optional and promptContent supports inline base64 image attachments
Other findings (outside the changed lines)
Valid issues on this PR's files that sit on lines GitHub won't let us comment on inline.
ChatThread hardcodes footerEvents=[] for Pi, discarding lastTurnInfo/isCompacting/completedToolCallCount
Priority: should_fix | File: packages/ui/src/features/sessions/components/chat-thread/ChatThread.tsx:974-985, 1133-1145 | Category: bug
Why we think it's a valid issue
- Checked: the full Pi render chain —
PiSessionView.tsx:207-212→ChatThread.tsx:972-983→ChatThreadRenderer(ChatThread.tsx:1130-1141) →ChatThreadFooter.tsx:39-64→SessionFooter.tsx:98-121,164-168, plusbuildAgentConversationItems(buildConversationItems.ts:293-312) and both footer-source hooks (useAgentConversationItems.ts,useConversationItems,useContextUsage.ts). - Found:
ChatThreadcallsuseAgentConversationItems(events, ...), whoseBuildResultalready carries correctlastTurnInfo/isCompacting/completedToolCallCountfor Pi, but destructures only{ items }(ChatThread.tsx:973) and rendersfooterEvents={[]}(ChatThread.tsx:980).ChatThreadFooterthen re-derives those three fields viauseConversationItems(events, ...)over that empty array (ChatThreadFooter.tsx:40-41).AcpChatThreadinstead passesfooterEvents={events}(ChatThread.tsx:996), confirming Pi is the broken branch.PiSessionViewsupplies no overrides (PiSessionView.tsx:207-212). - Impact: Confirmed live for every Pi cloud session — the footer permanently shows
lastGenerationDuration: null(so the "Generated in Xs" summary never renders,SessionFooter.tsx:98-121),completedToolCallCount: 0(soGeneratingIndicator'sactivityKeynever changes and the thinking word never advances,GeneratingIndicator.tsx:164-168), andisCompacting: false. Deterministic, always-on across the whole Pi runtime path, not an edge case. - Found (correction): In this checkout
ChatThreadFooterhas NOusageoverride prop — usage is also derived from the passedeventsviauseContextUsage(events)(ChatThreadFooter.tsx:39,useContextUsage.ts:11-17). So the issue's claim that usage was 'already patched via an override at L42-43' does not match the tree; all four footer signals are wired to the always-emptyfooterEvents. The fix should thread the already-computedBuildResultfields through uniformly rather than mirror a usage override that isn't present. - Priority: Lowering to should_fix — the defect is real and always-present, but it degrades only the footer status line (duration summary, thinking-word cadence, compacting indicator); Pi sessions, streaming, and tools all function, and the path is gated behind the
pi-harnessflag, so it is a non-blocking quality regression rather than a correctness/data/security blocker.
Issue description
ChatThread (the Pi-runtime entry point) calls useAgentConversationItems(events, props.isPromptPending), which returns a full BuildResult — { items, lastTurnInfo, isCompacting, completedToolCallCount } — computed correctly from Pi's native AgentConversationEvent[] via buildAgentConversationItems. But the component destructures only items and then renders <ChatThreadRenderer ... footerEvents={[]} />, permanently passing an empty array. ChatThreadRenderer forwards footerEvents verbatim to ChatThreadFooter (events={footerEvents}), which re-derives lastTurnInfo, isCompacting, and completedToolCallCount via its own useConversationItems(events, ...) call (ChatThreadFooter.tsx L44-45) — always over an empty array for Pi. The PR explicitly patched this same class of bug for context usage (usage prop override, ChatThreadFooter.tsx L43), but the equivalent turn/compaction/tool-count data was left wired to the always-empty footerEvents instead of the already-computed values. I confirmed this is live: packages/ui/src/features/pi-sessions/PiSessionView.tsx renders <ChatThread events={session.events} ... usage={contextUsage} /> with no override for the other three fields. As a result, for every Pi cloud session the footer will always show lastGenerationDuration: null, lastStopReason: undefined, isCompacting: false, and completedToolCallCount: 0 regardless of what actually happened — the 'last generation took Xs' summary, the compacting indicator, and the tool-call progress word in GeneratingIndicator/SessionFooter never reflect Pi's real state.
Suggested fix
Have ChatThread keep the full BuildResult from useAgentConversationItems (not just items) and thread lastTurnInfo, isCompacting, and completedToolCallCount down to ChatThreadFooter the same way usage was added — e.g. add optional override props to SharedChatThreadProps/ChatThreadFooterProps (lastTurnInfo?: LastTurnInfo | null, isCompacting?: boolean, completedToolCallCount?: number) and use value === undefined ? derivedFromEvents : value for each, mirroring the usage pattern already in ChatThreadFooter.tsx L42-43.
|
ReviewHog outside-diff finding — ChatThread hardcodes |
…-task-sessions # Conflicts: # packages/core/src/cloud-task/cloud-task.ts # packages/core/src/cloud-task/schemas.ts # packages/workspace-server/src/services/agent/agent.ts
|
/trunk merge |
1 similar comment
|
/trunk merge |
|
/trunk cancel |
…-task-sessions # Conflicts: # pnpm-lock.yaml
|
/trunk merge |
Summary
AgentConversationEvent/ ChatThread UISessionManager/RPC flow and wait forpi_run_startedbefore startup commandsBackend dependency: PostHog/posthog#72928
Architecture
Pi does not impersonate ACP. ACP and Pi independently adapt to
AgentConversationEvent; ChatThread renders that runtime-neutral model. Native Pi RPC remains canonical, while TaskSession JSONL is restored by Pi itself rather than parsed or rewritten by product code.Diff distribution
The current
+5,027 / -1,762diff is split between:Testing
maingit diff --checkRollout
Merge/deploy the backend dependency first, then publish the updated
@posthog/agentand rebuild the production sandbox image before enabling the client path behindpi-harness.