Pi Companion: port the app from Conductor to Pi - #8
Conversation
Replace the Conductor backend with Pi (pi.dev) end to end: Server (rewritten, all documented Pi surface — no undocumented internals): - Read projects/sessions/messages from Pi's session JSONL (~/.pi/agent/sessions) - Run turns via `pi --mode rpc` (resume, abort, mid-session model switch); replaces the four per-harness CLI adapters - /models from Pi's model catalog (deletes the bundle-scraping hack) - Projects are opt-in: added from the phone (with /browse folder picker), stored with added-at timestamps in ~/.pi-companion/projects.json - Delete endpoints: sessions move to ~/.pi-companion/trash, workspaces unregister - Worktrees now created under ~/pi-workspaces iOS app: - Rename to Pi Companion (bundle id com.matt-nz.pimobile, pi-companion:// pairing) - Pi logo header (drawn natively), per-provider model submenus - Add-project folder browser, long-press delete for chats/workspaces - v0.2.0 build 10 Docs: README/PRIVACY rewritten for Pi; install.sh paths renamed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe pull request renames the app to Pi Mobile and replaces the Conductor-backed server with a Pi session and RPC-based companion server. It adds project browsing, deletion, diffs, model discovery, Pi pairing, updated app identity, and corresponding setup documentation. ChangesPi Mobile migration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant iPhone
participant APIClient
participant PiCompanionServer
participant PiRPC
participant PiSessionFiles
iPhone->>APIClient: Start or continue a session
APIClient->>PiCompanionServer: Authenticated session request
PiCompanionServer->>PiRPC: Run pi --mode rpc
PiRPC->>PiSessionFiles: Read and append session JSONL events
PiRPC-->>PiCompanionServer: Stream activity events
PiCompanionServer-->>APIClient: Return session and message updates
APIClient-->>iPhone: Render updated chat state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
PiMobile/PiMobileApp.swift (1)
46-52: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRequire confirmation before accepting a pairing link.
Any app or webpage can invoke this scheme with an attacker-controlled
addr; the handler immediately replaces the pairing viaapi.pair. Require explicit user approval showing the endpoint before persisting it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PiMobile/PiMobileApp.swift` around lines 46 - 52, Update the onOpenURL pairing flow in PiMobileApp to require explicit user confirmation before calling api.pair. Present the parsed endpoint (addr, with relevant pairing details) in the approval UI, invoke api.pair only after acceptance, and leave the current early-return validation unchanged.
🧹 Nitpick comments (2)
PiMobile/APIClient.swift (1)
130-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider consolidating
get/post/deleteinto one request helper.
get,post, and nowdeleteeach rebuild the sameURLRequest(URL, method, Bearer header) and repeat the samestatusCode == 200check. Extracting a shared low-level helper would remove this triplication and centralize any future changes (e.g. error surfacing, retry).♻️ Proposed refactor
- private func get<T: Decodable>(_ path: String, on mac: MacServer? = nil) async throws -> T { - guard let mac = mac ?? activeMac, let url = URL(string: mac.baseURL + path) else { throw URLError(.badURL) } - var request = URLRequest(url: url) - request.setValue("Bearer \(mac.token)", forHTTPHeaderField: "Authorization") - let (data, response) = try await URLSession.shared.data(for: request) - guard let http = response as? HTTPURLResponse, http.statusCode == 200 else { - throw URLError(.badServerResponse) - } - return try decoder.decode(T.self, from: data) - } - - private func post(_ path: String, body: [String: String] = [:]) async throws -> Data { - guard let mac = activeMac, let url = URL(string: mac.baseURL + path) else { throw URLError(.badURL) } - var request = URLRequest(url: url) - request.httpMethod = "POST" - request.setValue("Bearer \(mac.token)", forHTTPHeaderField: "Authorization") - request.httpBody = try JSONEncoder().encode(body) - let (data, response) = try await URLSession.shared.data(for: request) - guard let http = response as? HTTPURLResponse, http.statusCode == 200 else { - throw URLError(.badServerResponse) - } - return data - } + private func requestData(_ path: String, method: String = "GET", body: Data? = nil, on mac: MacServer? = nil) async throws -> Data { + guard let mac = mac ?? activeMac, let url = URL(string: mac.baseURL + path) else { throw URLError(.badURL) } + var request = URLRequest(url: url) + request.httpMethod = method + request.setValue("Bearer \(mac.token)", forHTTPHeaderField: "Authorization") + request.httpBody = body + let (data, response) = try await URLSession.shared.data(for: request) + guard let http = response as? HTTPURLResponse, http.statusCode == 200 else { + throw URLError(.badServerResponse) + } + return data + } + + private func get<T: Decodable>(_ path: String, on mac: MacServer? = nil) async throws -> T { + try decoder.decode(T.self, from: try await requestData(path, on: mac)) + } + + private func post(_ path: String, body: [String: String] = [:]) async throws -> Data { + try await requestData(path, method: "POST", body: try JSONEncoder().encode(body)) + }- private func delete(_ path: String) async throws { - guard let mac = activeMac, let url = URL(string: mac.baseURL + path) else { throw URLError(.badURL) } - var request = URLRequest(url: url) - request.httpMethod = "DELETE" - request.setValue("Bearer \(mac.token)", forHTTPHeaderField: "Authorization") - let (_, response) = try await URLSession.shared.data(for: request) - guard let http = response as? HTTPURLResponse, http.statusCode == 200 else { - throw URLError(.badServerResponse) - } - } + private func delete(_ path: String) async throws { _ = try await requestData(path, method: "DELETE") }Also applies to: 180-205
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PiMobile/APIClient.swift` around lines 130 - 152, Consolidate the duplicated request construction and HTTP 200 validation from get, post, and delete into one shared low-level request helper. Have each method supply its path, HTTP method, and body as needed while preserving Bearer authorization, response data, decoding behavior, and existing error handling; update get, post, and delete to use that helper.PiMobile/Views/DiffView.swift (1)
19-26: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse
LazyVStackfor diff lines.Lines are laid out in a plain
VStackinsideScrollView, so the whole diff is built eagerly.ChatViewalready usesLazyVStackfor message rows — worth the same treatment here since diffs can run into thousands of lines.- ScrollView([.vertical, .horizontal]) { - VStack(alignment: .leading, spacing: 0) { + ScrollView([.vertical, .horizontal]) { + LazyVStack(alignment: .leading, spacing: 0) { ForEach(Array(diff.diff.split(separator: "\n", omittingEmptySubsequences: false).enumerated()), id: \.offset) { _, line in DiffLine(line: String(line)) } } .padding(12) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PiMobile/Views/DiffView.swift` around lines 19 - 26, Replace the eager VStack containing the ForEach in the DiffView ScrollView with a LazyVStack, preserving the existing alignment, spacing, padding, and DiffLine rendering behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@PiMobile/Views/ChatView.swift`:
- Around line 93-105: Update the active-session deletion logic in the
contextMenu action so that when deleting the current session leaves sessions
empty, running/activity state is explicitly reset along with session, messages,
and sessionId. Preserve selecting the first remaining session via select(next)
when one exists.
In `@PiMobile/Views/DiffView.swift`:
- Around line 67-83: Update the color and background classification in the
DiffView line styling properties so “+++” and “---” header lines are recognized
before the generic “+” and “-” checks. Preserve the existing muted header
styling and ensure file headers no longer use addition/deletion colors.
In `@README.md`:
- Line 15: Revise the README feature description to remove the claim that every
folder where pi was run is discovered automatically. Describe projects as
folders explicitly added by the user, while preserving the existing grouping,
worktree, branch, and live-status details.
In `@server/server.ts`:
- Around line 474-480: Update the /diffstat handler around wsById and
workspaceDiff to calculate git shortstat against the same merge-base with
origin/HEAD used by workspaceDiff, rather than HEAD. Preserve the existing
insertion/deletion parsing and 404 behavior so the badge reflects the full diff
view, including committed worktree changes.
- Around line 303-336: The turn runner around the stdout-processing async IIFE
must use try/finally so turn.running is reset and session cleanup/activity
clearing still occur when stdout iteration or proc.exited throws. Also consume
proc.stderr continuously, or configure it to inherit, so the stderr pipe cannot
accumulate unread output; update the runner associated with the turn variable
and Bun.spawn call.
---
Outside diff comments:
In `@PiMobile/PiMobileApp.swift`:
- Around line 46-52: Update the onOpenURL pairing flow in PiMobileApp to require
explicit user confirmation before calling api.pair. Present the parsed endpoint
(addr, with relevant pairing details) in the approval UI, invoke api.pair only
after acceptance, and leave the current early-return validation unchanged.
---
Nitpick comments:
In `@PiMobile/APIClient.swift`:
- Around line 130-152: Consolidate the duplicated request construction and HTTP
200 validation from get, post, and delete into one shared low-level request
helper. Have each method supply its path, HTTP method, and body as needed while
preserving Bearer authorization, response data, decoding behavior, and existing
error handling; update get, post, and delete to use that helper.
In `@PiMobile/Views/DiffView.swift`:
- Around line 19-26: Replace the eager VStack containing the ForEach in the
DiffView ScrollView with a LazyVStack, preserving the existing alignment,
spacing, padding, and DiffLine rendering behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0cead679-e25f-446c-ad3e-b089d23705d8
⛔ Files ignored due to path filters (2)
PiMobile/Assets.xcassets/AppIcon.appiconset/1024.pngis excluded by!**/*.pngdocs/images/pi-mobile-icon.pngis excluded by!**/*.png
📒 Files selected for processing (18)
.gitignoreConfig/Info.plistPRIVACY.mdPiMobile.xcodeproj/project.pbxprojPiMobile/APIClient.swiftPiMobile/Assets.xcassets/AppIcon.appiconset/Contents.jsonPiMobile/Assets.xcassets/Contents.jsonPiMobile/Models.swiftPiMobile/PiMobileApp.swiftPiMobile/Theme.swiftPiMobile/Views/ChatView.swiftPiMobile/Views/DiffView.swiftPiMobile/Views/ProjectsView.swiftPiMobile/Views/SettingsView.swiftPiMobile/Views/WorkspacesView.swiftREADME.mdserver/install.shserver/server.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
PiMobile/PiMobileApp.swift (1)
46-52: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRequire confirmation before accepting a pairing link.
Any app or webpage can invoke this scheme with an attacker-controlled
addr; the handler immediately replaces the pairing viaapi.pair. Require explicit user approval showing the endpoint before persisting it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PiMobile/PiMobileApp.swift` around lines 46 - 52, Update the onOpenURL pairing flow in PiMobileApp to require explicit user confirmation before calling api.pair. Present the parsed endpoint (addr, with relevant pairing details) in the approval UI, invoke api.pair only after acceptance, and leave the current early-return validation unchanged.
🧹 Nitpick comments (2)
PiMobile/APIClient.swift (1)
130-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider consolidating
get/post/deleteinto one request helper.
get,post, and nowdeleteeach rebuild the sameURLRequest(URL, method, Bearer header) and repeat the samestatusCode == 200check. Extracting a shared low-level helper would remove this triplication and centralize any future changes (e.g. error surfacing, retry).♻️ Proposed refactor
- private func get<T: Decodable>(_ path: String, on mac: MacServer? = nil) async throws -> T { - guard let mac = mac ?? activeMac, let url = URL(string: mac.baseURL + path) else { throw URLError(.badURL) } - var request = URLRequest(url: url) - request.setValue("Bearer \(mac.token)", forHTTPHeaderField: "Authorization") - let (data, response) = try await URLSession.shared.data(for: request) - guard let http = response as? HTTPURLResponse, http.statusCode == 200 else { - throw URLError(.badServerResponse) - } - return try decoder.decode(T.self, from: data) - } - - private func post(_ path: String, body: [String: String] = [:]) async throws -> Data { - guard let mac = activeMac, let url = URL(string: mac.baseURL + path) else { throw URLError(.badURL) } - var request = URLRequest(url: url) - request.httpMethod = "POST" - request.setValue("Bearer \(mac.token)", forHTTPHeaderField: "Authorization") - request.httpBody = try JSONEncoder().encode(body) - let (data, response) = try await URLSession.shared.data(for: request) - guard let http = response as? HTTPURLResponse, http.statusCode == 200 else { - throw URLError(.badServerResponse) - } - return data - } + private func requestData(_ path: String, method: String = "GET", body: Data? = nil, on mac: MacServer? = nil) async throws -> Data { + guard let mac = mac ?? activeMac, let url = URL(string: mac.baseURL + path) else { throw URLError(.badURL) } + var request = URLRequest(url: url) + request.httpMethod = method + request.setValue("Bearer \(mac.token)", forHTTPHeaderField: "Authorization") + request.httpBody = body + let (data, response) = try await URLSession.shared.data(for: request) + guard let http = response as? HTTPURLResponse, http.statusCode == 200 else { + throw URLError(.badServerResponse) + } + return data + } + + private func get<T: Decodable>(_ path: String, on mac: MacServer? = nil) async throws -> T { + try decoder.decode(T.self, from: try await requestData(path, on: mac)) + } + + private func post(_ path: String, body: [String: String] = [:]) async throws -> Data { + try await requestData(path, method: "POST", body: try JSONEncoder().encode(body)) + }- private func delete(_ path: String) async throws { - guard let mac = activeMac, let url = URL(string: mac.baseURL + path) else { throw URLError(.badURL) } - var request = URLRequest(url: url) - request.httpMethod = "DELETE" - request.setValue("Bearer \(mac.token)", forHTTPHeaderField: "Authorization") - let (_, response) = try await URLSession.shared.data(for: request) - guard let http = response as? HTTPURLResponse, http.statusCode == 200 else { - throw URLError(.badServerResponse) - } - } + private func delete(_ path: String) async throws { _ = try await requestData(path, method: "DELETE") }Also applies to: 180-205
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PiMobile/APIClient.swift` around lines 130 - 152, Consolidate the duplicated request construction and HTTP 200 validation from get, post, and delete into one shared low-level request helper. Have each method supply its path, HTTP method, and body as needed while preserving Bearer authorization, response data, decoding behavior, and existing error handling; update get, post, and delete to use that helper.PiMobile/Views/DiffView.swift (1)
19-26: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse
LazyVStackfor diff lines.Lines are laid out in a plain
VStackinsideScrollView, so the whole diff is built eagerly.ChatViewalready usesLazyVStackfor message rows — worth the same treatment here since diffs can run into thousands of lines.- ScrollView([.vertical, .horizontal]) { - VStack(alignment: .leading, spacing: 0) { + ScrollView([.vertical, .horizontal]) { + LazyVStack(alignment: .leading, spacing: 0) { ForEach(Array(diff.diff.split(separator: "\n", omittingEmptySubsequences: false).enumerated()), id: \.offset) { _, line in DiffLine(line: String(line)) } } .padding(12) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PiMobile/Views/DiffView.swift` around lines 19 - 26, Replace the eager VStack containing the ForEach in the DiffView ScrollView with a LazyVStack, preserving the existing alignment, spacing, padding, and DiffLine rendering behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@PiMobile/Views/ChatView.swift`:
- Around line 93-105: Update the active-session deletion logic in the
contextMenu action so that when deleting the current session leaves sessions
empty, running/activity state is explicitly reset along with session, messages,
and sessionId. Preserve selecting the first remaining session via select(next)
when one exists.
In `@PiMobile/Views/DiffView.swift`:
- Around line 67-83: Update the color and background classification in the
DiffView line styling properties so “+++” and “---” header lines are recognized
before the generic “+” and “-” checks. Preserve the existing muted header
styling and ensure file headers no longer use addition/deletion colors.
In `@README.md`:
- Line 15: Revise the README feature description to remove the claim that every
folder where pi was run is discovered automatically. Describe projects as
folders explicitly added by the user, while preserving the existing grouping,
worktree, branch, and live-status details.
In `@server/server.ts`:
- Around line 474-480: Update the /diffstat handler around wsById and
workspaceDiff to calculate git shortstat against the same merge-base with
origin/HEAD used by workspaceDiff, rather than HEAD. Preserve the existing
insertion/deletion parsing and 404 behavior so the badge reflects the full diff
view, including committed worktree changes.
- Around line 303-336: The turn runner around the stdout-processing async IIFE
must use try/finally so turn.running is reset and session cleanup/activity
clearing still occur when stdout iteration or proc.exited throws. Also consume
proc.stderr continuously, or configure it to inherit, so the stderr pipe cannot
accumulate unread output; update the runner associated with the turn variable
and Bun.spawn call.
---
Outside diff comments:
In `@PiMobile/PiMobileApp.swift`:
- Around line 46-52: Update the onOpenURL pairing flow in PiMobileApp to require
explicit user confirmation before calling api.pair. Present the parsed endpoint
(addr, with relevant pairing details) in the approval UI, invoke api.pair only
after acceptance, and leave the current early-return validation unchanged.
---
Nitpick comments:
In `@PiMobile/APIClient.swift`:
- Around line 130-152: Consolidate the duplicated request construction and HTTP
200 validation from get, post, and delete into one shared low-level request
helper. Have each method supply its path, HTTP method, and body as needed while
preserving Bearer authorization, response data, decoding behavior, and existing
error handling; update get, post, and delete to use that helper.
In `@PiMobile/Views/DiffView.swift`:
- Around line 19-26: Replace the eager VStack containing the ForEach in the
DiffView ScrollView with a LazyVStack, preserving the existing alignment,
spacing, padding, and DiffLine rendering behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0cead679-e25f-446c-ad3e-b089d23705d8
⛔ Files ignored due to path filters (2)
PiMobile/Assets.xcassets/AppIcon.appiconset/1024.pngis excluded by!**/*.pngdocs/images/pi-mobile-icon.pngis excluded by!**/*.png
📒 Files selected for processing (18)
.gitignoreConfig/Info.plistPRIVACY.mdPiMobile.xcodeproj/project.pbxprojPiMobile/APIClient.swiftPiMobile/Assets.xcassets/AppIcon.appiconset/Contents.jsonPiMobile/Assets.xcassets/Contents.jsonPiMobile/Models.swiftPiMobile/PiMobileApp.swiftPiMobile/Theme.swiftPiMobile/Views/ChatView.swiftPiMobile/Views/DiffView.swiftPiMobile/Views/ProjectsView.swiftPiMobile/Views/SettingsView.swiftPiMobile/Views/WorkspacesView.swiftREADME.mdserver/install.shserver/server.ts
🛑 Comments failed to post (5)
PiMobile/Views/ChatView.swift (1)
93-105: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Stuck streaming bar when deleting the last running session.
If the deleted session is both the active one and the only session left (
sessionsbecomes empty), theif let next = sessions.first { select(next) }branch never runs, sorunning/activityare never reset —select(_:)is the only place that clears them. The streaming bar and its "Stop" button (which requires a non-nilsessionId) are left stuck sincesessionIdis now nil.🐛 Proposed fix
.contextMenu { Button("Delete Chat", systemImage: "trash", role: .destructive) { Task { try? await api.deleteSession(s.id) sessions.removeAll { $0.id == s.id } if s.id == sessionId { session = nil messages = [] - if let next = sessions.first { select(next) } + if let next = sessions.first { + select(next) + } else { + running = false + activity = "" + } } } } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements..contextMenu { Button("Delete Chat", systemImage: "trash", role: .destructive) { Task { try? await api.deleteSession(s.id) sessions.removeAll { $0.id == s.id } if s.id == sessionId { session = nil messages = [] if let next = sessions.first { select(next) } else { running = false activity = "" } } } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PiMobile/Views/ChatView.swift` around lines 93 - 105, Update the active-session deletion logic in the contextMenu action so that when deleting the current session leaves sessions empty, running/activity state is explicitly reset along with session, messages, and sessionId. Preserve selecting the first remaining session via select(next) when one exists.PiMobile/Views/DiffView.swift (1)
67-83: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Header lines (
---/+++) are always miscolored as deletion/addition.
hasPrefix("+")/hasPrefix("-")are checked beforehasPrefix("+++")/hasPrefix("---"), so the latter branches are unreachable — every unified-diff file header pair (--- a/file/+++ b/file) matches the generic addition/deletion checks first. Since these two header lines appear once per file in every diff, this always mis-renders, not just on rare inputs.🐛 Proposed fix
private var color: Color { + if line.hasPrefix("+++") || line.hasPrefix("---") { return Theme.textMuted } if line.hasPrefix("+") { return Color(red: 0.5, green: 0.87, blue: 0.6) } if line.hasPrefix("-") { return Color(red: 0.95, green: 0.57, blue: 0.56) } if line.hasPrefix("@@") { return Theme.accent } - if line.hasPrefix("diff ") || line.hasPrefix("index ") || line.hasPrefix("+++") || line.hasPrefix("---") { + if line.hasPrefix("diff ") || line.hasPrefix("index ") { return Theme.textMuted } return Color(red: 0.44, green: 0.44, blue: 0.49) } private var background: Color { + if line.hasPrefix("+++") || line.hasPrefix("---") || line.hasPrefix("diff ") { return Color.white.opacity(0.04) } if line.hasPrefix("+") { return Theme.green.opacity(0.06) } if line.hasPrefix("-") { return Color(red: 0.97, green: 0.44, blue: 0.44).opacity(0.06) } - if line.hasPrefix("diff ") { return Color.white.opacity(0.04) } return .clear }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.private var color: Color { if line.hasPrefix("+++") || line.hasPrefix("---") { return Theme.textMuted } if line.hasPrefix("+") { return Color(red: 0.5, green: 0.87, blue: 0.6) } if line.hasPrefix("-") { return Color(red: 0.95, green: 0.57, blue: 0.56) } if line.hasPrefix("@@") { return Theme.accent } if line.hasPrefix("diff ") || line.hasPrefix("index ") { return Theme.textMuted } return Color(red: 0.44, green: 0.44, blue: 0.49) } private var background: Color { if line.hasPrefix("+++") || line.hasPrefix("---") || line.hasPrefix("diff ") { return Color.white.opacity(0.04) } if line.hasPrefix("+") { return Theme.green.opacity(0.06) } if line.hasPrefix("-") { return Color(red: 0.97, green: 0.44, blue: 0.44).opacity(0.06) } return .clear } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PiMobile/Views/DiffView.swift` around lines 67 - 83, Update the color and background classification in the DiffView line styling properties so “+++” and “---” header lines are recognized before the generic “+” and “-” checks. Preserve the existing muted header styling and ensure file headers no longer use addition/deletion colors.README.md (1)
15-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Overstates project discovery. The server only surfaces folders the user explicitly adds (plus phone-created worktrees) —
scan()skips any cwd not in the opt-in set, per the server's own comment ("Projects are opt-in: only folders the user added from the phone … appear"). "every folder you've runpiin" implies automatic discovery of all Pi runs, which is not what happens. Reword to reflect the add-a-folder flow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 15, Revise the README feature description to remove the claim that every folder where pi was run is discovered automatically. Describe projects as folders explicitly added by the user, while preserving the existing grouping, worktree, branch, and live-status details.server/server.ts (2)
303-336: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the relevant server code around the reported lines. sed -n '260,360p' server/server.ts # Find any existing stderr handling or related spawn patterns in the repo. rg -n 'stderr|Bun\.spawn|proc\.stdout|proc\.stderr|pendingSessions\.delete|turn\.running' server server.ts .Repository: MRL-00/pi-mobile
Length of output: 6648
🏁 Script executed:
#!/bin/bash set -euo pipefail # Check Bun spawn documentation in the repo if available. rg -n 'spawn\(|stderr: "pipe"|stdout: "pipe"|stderr: "inherit"|pipe backpressure|Bun.spawn' README.md docs . || true # Try a lightweight external check for Bun spawn stderr semantics. python3 - <<'PY' import urllib.request, ssl urls = [ "https://bun.sh/docs/api/spawn", "https://bun.sh/docs/runtime/bun-spawn", ] ctx = ssl.create_default_context() for u in urls: try: with urllib.request.urlopen(u, context=ctx, timeout=15) as r: data = r.read().decode("utf-8", "ignore") print("URL:", u) for needle in ["stderr", "pipe", "backpressure", "stream", "Bun.spawn"]: if needle in data: print(" found", needle) print(data[:1200].replace("\n", " ")[:1200]) break except Exception as e: print("URL_FAIL", u, type(e).__name__, e) PYRepository: MRL-00/pi-mobile
Length of output: 1235
🌐 Web query:
Bun.spawn stderr pipe unread backpressure pipe buffer behavior💡 Result:
In Bun, when a child process's stderr (or stdout) is configured as a "pipe", it is exposed as a ReadableStream [1][2]. The following behaviors define how Bun handles pipes, unread buffers, and backpressure: 1. Pipe Behavior and Buffering: By default, if a pipe is created, Bun's runtime reads from the child process's output pipes [1]. If you do not actively consume the ReadableStream (i.e., you do not read from it), the data will buffer in memory [3]. Bun has internal mechanisms to handle this, but leaving a pipe unread can lead to significant memory growth if the child process produces large amounts of output [3]. 2. MaxBuffer Protection: To prevent uncontrolled memory usage, Bun supports a maxBuffer constraint (specifically for synchronous spawns or when explicitly configured) [4][5]. If this buffer limit is exceeded, Bun stops reading from the pipe and effectively terminates the subprocess [4]. Recent updates have improved this to ensure that Bun stops reading immediately once the buffer limit is exceeded, preventing the runtime from "overshooting" and reading more data than necessary [4]. 3. Backpressure Mechanism: - For stderr/stdout, Bun's ReadableStream implementation follows standard web stream backpressure principles. If you are consuming the stream (e.g., via
for awaitorgetReader()), the runtime will pause reading from the pipe if the consumer is slow, allowing the OS pipe buffer to fill up, which in turn applies backpressure to the child process [3]. - Iflazy: trueis set in the SpawnOptions, Bun will not start reading from the pipe until you explicitly access the stderr/stdout property, which can prevent unnecessary resource allocation and buffering for processes where you do not need the output [6][2]. 4. Stdin Backpressure: While your query specifically asked about stderr, it is worth noting that Bun has specifically addressed backpressure for stdin as well. When piping a ReadableStream into a subprocess's stdin, Bun implements a mechanism to pause reading from the source stream when the underlying pipe buffer is full, preventing memory starvation [3][7]. Summary: If you ignore a pipe (do not consume it), it will buffer in memory [3]. If you must handle high-volume output, you should either consume the stream to apply backpressure or configure amaxBufferlimit if applicable to your use case [4][5]. Use thelazy: trueoption if you only need the output conditionally [6][2].Citations:
- 1: https://bun.sh/docs/runtime/child-process
- 2: https://bun.com/reference/bun/spawn
- 3: oven-sh/bun#33399
- 4: oven-sh/bun#33309
- 5: https://bun.com/docs/runtime/child-process
- 6: https://bun.sh/reference/bun/Spawn/SpawnOptions
- 7: https://bun.com/blog/bun-v1.2.18
🌐 Web query:
Bun spawn unread stderr pipe buffers in memory backpressure child process docs💡 Result:
When using Bun.spawn with stdio set to "pipe", Bun creates a ReadableStream for the child process's output [1][2]. Unread data in these pipes can lead to memory growth, and historically, Bun has encountered issues related to memory leaks in piped stdout/stderr [3][4]. Regarding backpressure and memory management: 1. Pipe Buffers and Memory: If you do not actively consume (read) the ReadableStream returned by proc.stderr or proc.stdout, data may accumulate in memory [2][3]. Bun has addressed specific memory leaks where piped streams were not correctly finalized after a process exited [3]. 2. Backpressure Support: Bun is actively improving its backpressure mechanisms [5][6]. Recent updates have introduced better handling for piping ReadableStreams into subprocess stdin, including applying backpressure to prevent unbounded memory growth [6]. Specifically, FileSink now reports backpressure status when buffered data exceeds thresholds (e.g., 256 KiB) while a ReadableStream is being pumped [6]. 3. Development Status: While significant improvements have been made, developers have noted that certain interfaces (like FileSink on stdin) have historically lacked explicit backpressure signals like writableLength or 'drain' events, which are common in Node.js [7]. However, work is ongoing to unify stdio write paths to route through standard Writable buffer accounting to ensure consistent backpressure behavior [5]. For applications requiring high-performance or streaming subprocess interaction, ensure you are using a recent version of Bun, as the runtime has seen continuous improvements in memory efficiency and stream management [3][4][5][6]. If you are not reading the output, setting stdio to "ignore" is the recommended way to avoid unnecessary memory consumption [2][4].
Citations:
- 1: https://bun.com/docs/guides/process/spawn-stderr
- 2: https://bun.com/docs/runtime/child-process
- 3: oven-sh/bun#18316
- 4: oven-sh/bun#18265
- 5: oven-sh/bun#33508
- 6: oven-sh/bun#33399
- 7: oven-sh/bun#29341
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the Bun/runtime version pinned by this repo, if any. rg -n '"bun"|bun\s*:|runtime|packageManager' package.json bun.lockb bun.lock . || trueRepository: MRL-00/pi-mobile
Length of output: 306
Wrap the turn runner in
try/finallyand handle stderr.
- If the stdout loop or
await proc.exitedthrows,turn.runningnever resets and latersendMessagecalls keep returning 409.stderr: "pipe"is never consumed; Bun buffers unread pipe output in memory, so noisy runs can grow memory. Drain it or switch it toinherit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/server.ts` around lines 303 - 336, The turn runner around the stdout-processing async IIFE must use try/finally so turn.running is reset and session cleanup/activity clearing still occur when stdout iteration or proc.exited throws. Also consume proc.stderr continuously, or configure it to inherit, so the stderr pipe cannot accumulate unread output; update the runner associated with the turn variable and Bun.spawn call.
474-480: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
/diffstatand/diffmeasure different things — the badge won't match the diff view.
diffstatcounts only working-tree changes (git diff --shortstat HEAD), whileworkspaceDiff(Lines 385-388) diffs against the merge-base withorigin/HEAD. For a phone-created worktree where the agent commits its work, the diff view shows all branch changes but this badge reports0 insertions / 0 deletions. Compute the shortstat against the same base ref used byworkspaceDiffso the two agree.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/server.ts` around lines 474 - 480, Update the /diffstat handler around wsById and workspaceDiff to calculate git shortstat against the same merge-base with origin/HEAD used by workspaceDiff, rather than HEAD. Preserve the existing insertion/deletion parsing and 404 behavior so the badge reflects the full diff view, including committed worktree changes.
1 participant
Summary
Conductor is shipping their own mobile app, so this pivots ours to be an unofficial companion for the Pi coding agent — Pi Companion. The phone UX is unchanged (projects → workspaces → chat, history, diffs, model picker); the whole backend now speaks Pi.
Companion server (rewritten)
~/.pi/agent/sessions/*.jsonl, documented v3 format) — replaces the undocumented-SQLite layer entirelypi --mode rpc: resume by session file, mint new sessions with--session-id, abort, mid-session model switching. One protocol replaces the four per-harness CLI adapters (claude/codex/cursor/opencode)pi -cin the terminal continues seamlessly/modelscomes from Pi's model catalog (deletes the app-bundle string-scraping)/browsefolder picker, stored with added-at timestamps in~/.pi-companion/projects.json(keeps emdash/scratch sessions out of the list)~/.pi-companion/trash(recoverable), workspace delete just unregisters the folder~/pi-workspaces/<repo>/<city>iOS app
com.matt-nz.pimobile,pi-companion://pairing scheme, Pi logo header (drawn as a native SwiftUI shape from the press-kit SVG)Tested
Not in this PR
/status)MRL-00/conductor-mobile; GitHub will redirect after rename)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements