Skip to content

fix(ui): distinguish local Pi initialization - #4047

Merged
trunk-io[bot] merged 2 commits into
mainfrom
fix/local-pi-connecting-copy
Jul 31, 2026
Merged

fix(ui): distinguish local Pi initialization#4047
trunk-io[bot] merged 2 commits into
mainfrom
fix/local-pi-connecting-copy

Conversation

@jonathanlab

Copy link
Copy Markdown
Contributor

Problem

Local Pi sessions reuse cloud initialization copy, so the UI can claim it is connecting to a cloud runner.

Changes

  • Rename the initializer to a session-wide component with an explicit execution target.
  • Show local Pi copy for local sessions and retain cloud provisioning copy for cloud sessions.
  • Add coverage for both initialization targets.

How did you test this?

  • pnpm --filter @posthog/ui exec vitest run src/features/sessions/components/SessionInitializingView.test.tsx
  • pnpm --filter @posthog/ui typecheck
  • pnpm exec biome check --write packages/ui/src/features/sessions/components/SessionInitializingView.tsx packages/ui/src/features/sessions/components/SessionInitializingView.test.tsx packages/ui/src/features/sessions/components/SessionView.tsx packages/ui/src/features/pi-sessions/PiSessionView.tsx

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

@trunk-io

trunk-io Bot commented Jul 31, 2026

Copy link
Copy Markdown

😎 Merged successfully - details.

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown

React Doctor found 1 issue in 1 file · 1 warning.

1 warning

src/features/pi-sessions/PiSessionView.tsx

Reviewed by React Doctor for commit 395c243.

@greptile-apps

greptile-apps Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor
Prompt To Fix All With AI
### Issue 1
packages/ui/src/features/pi-sessions/PiSessionView.tsx:307
**Cloud sessions start as local**

When a cloud Pi session first enters the connecting state, `cloudStatus` remains undefined until the asynchronous session subscription resolves, so this expression selects the local target and shows “Connecting to Pi on this device” for a cloud run.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(ui): distinguish local Pi initializa..." | Re-trigger Greptile

);
const sessionAvailable =
session.connectionState === "connected" || hasTranscript;
const executionTarget = session.cloudStatus === undefined ? "local" : "cloud";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Cloud sessions start as local

When a cloud Pi session first enters the connecting state, cloudStatus remains undefined until the asynchronous session subscription resolves, so this expression selects the local target and shows “Connecting to Pi on this device” for a cloud run.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/pi-sessions/PiSessionView.tsx
Line: 307

Comment:
**Cloud sessions start as local**

When a cloud Pi session first enters the connecting state, `cloudStatus` remains undefined until the asynchronous session subscription resolves, so this expression selects the local target and shows “Connecting to Pi on this device” for a cloud run.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@jonathanlab jonathanlab added the Stamphog This will request an autostamp by stamphog on small changes label Jul 31, 2026
@stamphog

stamphog Bot commented Jul 31, 2026

Copy link
Copy Markdown

Note

🤖 stamphog reviewed 78ce5dd4375c93bbeea9e08b7f900cf9ec5b43c4 — verdict: REFUSED

Greptile's inline comment identifies a real, unfixed bug: executionTarget is derived from cloudStatus === undefined, but cloud sessions also start with cloudStatus undefined before the subscription resolves, so the diff still shows local-device copy for a cloud session at the exact moment the PR claims to fix — the current diff confirms the flagged code is unchanged.

  • Author wrote 36% of the modified lines and has 9 merged PRs in these paths (familiarity MODERATE).
  • greptile-apps[bot] reviewed the current head.
  • Unresolved reviewer concern from @greptile-apps on packages/ui/src/features/pi-sessions/PiSessionView.tsx:307 — cloud sessions can render local initialization copy because cloudStatus is undefined before the async subscription resolves, contradicting the PR's stated purpose.
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list no deny categories matched
size 42L, 3F substantive, 87L/4F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1b-small (87L, 4F, single-area, fix)
stamphog 2.0.0b3 .stamphog/policy.yml @ 1fac094 · reviewed head 78ce5dd

@stamphog stamphog Bot removed the Stamphog This will request an autostamp by stamphog on small changes label Jul 31, 2026
@jonathanlab

Copy link
Copy Markdown
Contributor Author

Fixed in 395c243. PiSessionView now receives the cloud/local target from TaskDetail, where task and workspace metadata are already available, instead of inferring it from the asynchronously populated cloudStatus. Added coverage for a cloud initializer before its status arrives.

@greptile-apps

greptile-apps Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

The original concern is confirmed. In piSessionController.ts, ensureSubscription sets cloudStatus in the store asynchronously via .then():

void this.getPiSession(taskId)
  .then((session) => {
    // ...
    this.updateSession(taskId, { cloudStatus: session.cloudStatus });
  })

The store starts from createEmptyPiControllerSession() which has no cloudStatus field — so it's undefined for all sessions until that promise resolves. The check at line 307:

const executionTarget = session.cloudStatus === undefined ? "local" : "cloud";

will incorrectly render local-device copy for a cloud session during that async gap.

The reliable discriminator is already available as a prop: taskRunId is only defined for cloud sessions. The fix is:

  const executionTarget = taskRunId !== undefined ? "cloud" : "local";

This is determined synchronously at render time with no async dependency, so there's no flash of incorrect copy for cloud sessions.

@jonathanlab jonathanlab added the Stamphog This will request an autostamp by stamphog on small changes label Jul 31, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contained UI-only fix (component rename plus a synchronous isCloud prop) with no risky-territory surface touched; the async cloudStatus race Greptile flagged was fixed by deriving executionTarget from a synchronously-known isCloud value passed from TaskDetail, and a new test covers exactly that race scenario.

  • Author wrote 44% of the modified lines and has 10 merged PRs in these paths (familiarity MODERATE).
  • 👍 on the PR from greptile-apps[bot].
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list no deny categories matched
size 55L, 4F substantive, 105L/5F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1c-medium (105L, 5F, single-area, fix)
stamphog 2.0.0b3 .stamphog/policy.yml @ 1fac094 · reviewed head 395c243

@trunk-io
trunk-io Bot merged commit b4ec45c into main Jul 31, 2026
39 checks passed
@trunk-io
trunk-io Bot deleted the fix/local-pi-connecting-copy branch July 31, 2026 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Stamphog This will request an autostamp by stamphog on small changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant