feat: integrate server API, iOS app, and WebSocket support#123
Open
2witstudios wants to merge 121 commits intomainfrom
Open
feat: integrate server API, iOS app, and WebSocket support#1232witstudios wants to merge 121 commits intomainfrom
2witstudios wants to merge 121 commits intomainfrom
Conversation
Move merge orchestration (state transitions, git merge, cleanup) from commands/merge.ts into a reusable performMerge() function. The CLI command becomes a thin wrapper for arg parsing and output formatting. This enables both CLI and future REST routes to share the same merge logic with typed options and structured results. Closes #59
- Add `ppg serve` command with --port, --host, --token, --daemon, --json options - Create Fastify server with CORS, /health endpoint, bearer token auth - Graceful shutdown on SIGTERM/SIGINT with state file cleanup - State file (serve.json) and PID file with 0o600 permissions - LAN IP detection via os.networkInterfaces() - Path helpers: serveStatePath(), servePidPath() in lib/paths.ts - Lazy import registration in cli.ts Closes #63
Move agent restart logic from commands/restart.ts into a reusable performRestart() function in core/operations/restart.ts. The CLI command is now a thin wrapper handling arg parsing and output formatting. Closes #60
Add TokenStorage struct using iOS Keychain Services for secure token persistence. Tokens are keyed by connection UUID under the com.ppg.mobile service with whenUnlocked accessibility. Closes #80
Add Fastify error handler that maps PpgError codes to HTTP statuses, handles validation errors with field-level details, and returns structured JSON responses. Unknown errors return generic 500. Closes #66
Move worktree creation + agent spawning pipeline from commands/spawn.ts into core/operations/spawn.ts as performSpawn() with typed options and result. Commands layer now handles only arg parsing + output formatting. Closes #62
…ver connection Add Swift Codable structs matching the ppg server JSON schema: - Manifest.swift: Manifest, WorktreeEntry, AgentEntry structs with AgentStatus and WorktreeStatus enums including display properties (label, color, SF Symbol) - AgentVariant.swift: Known agent type definitions (claude, codex, opencode) with brand colors and icons, plus AgentEntry extensions - ServerConnection.swift: Connection config with URL builders for REST/WS endpoints and QR code parser for ppg://connect scheme Closes #77
Add production-grade token authentication for the serve command: - Token generation with crypto.randomBytes(24) and tk_ prefix - SHA-256 hashing (plaintext never stored) - Timing-safe validation via crypto.timingSafeEqual - Multi-token support with labels and lastUsedAt tracking - Token revocation by label - IP-based rate limiting (5 failures per 5-minute window) - Auth persistence in .ppg/serve/auth.json with 0o600 permissions - Fastify-compatible preHandler hook for Bearer token extraction Closes #64
Set up iOS project structure with XcodeGen for declarative project config: - project.yml targeting iOS 17.0, Swift 5.9 - Bundle ID: com.2witstudios.ppg-mobile - SwiftUI app entry point and ContentView - Asset catalog with AppIcon placeholder - .gitignore to exclude generated .xcodeproj Closes #76
Add `ppg serve start/stop/status` subcommands for daemon lifecycle management. The daemon runs in a tmux window (consistent with ppg cron pattern), writes PID and connection info to serve.pid/serve.json, and supports graceful shutdown via SIGTERM. Closes #67
Actor-based REST client covering all 13 server endpoints (7 read + 6 write): - Read: status, worktree detail, diff, agent logs, config, templates, prompts - Write: spawn, merge, kill, restart, send input, create PR - testConnection() for verifying reachability and auth - Typed error enum: network, unauthorized, notFound, conflict, serverError, decodingError - Custom URLSession delegate for self-signed TLS via pinned CA certificate - 15s request / 30s resource timeouts Closes #78
Observable WebSocket manager using URLSessionWebSocketTask with: - Connection states: disconnected, connecting, connected, reconnecting - Exponential backoff reconnect (1s→30s max) - Keepalive ping every 30s - Event parsing into typed enums (manifest updates, agent/worktree status) - Terminal subscribe/unsubscribe/input commands - NotificationCenter integration matching existing app patterns Closes #79
Move agent kill logic from commands/kill.ts into a reusable core/operations/kill.ts module. The CLI command is now a thin wrapper that handles arg parsing and output formatting, delegating all kill logic to performKill(). Closes #61
- QRScannerView with AVCaptureSession wrapped in UIViewRepresentable - QR code detection via AVCaptureMetadataOutput for .qr type - Parse ppg://connect?host=...&port=...&ca=...&token=... scheme - Double-scan prevention via hasScanned flag in coordinator - Error alerts for invalid QR codes with expected format hint - CameraPreviewView subclass for proper bounds management via layoutSubviews - Camera permission request handling with denied state UI and Settings link - Session lifecycle: start on create, stop on dismantle via dismantleUIView - ServerConnection updated with optional ca field for TLS certificate pinning
Add SwiftUI views for the main dashboard experience: - DashboardView: NavigationStack with Active/Completed worktree sections, pull-to-refresh, connection status indicator, empty and disconnected states - WorktreeCard: status card with name, branch, agent count, status badge - WorktreeDetailView: inspector with agent list, diff stats, merge/kill/PR actions - AgentRow: agent status row with kill/restart action buttons Includes @observable DashboardStore protocol and domain models (Worktree, Agent, WorktreeStatus, AgentStatus) aligned with the macOS app's model layer. Closes #83
Add fs.watch on manifest.json (300ms debounce) and status polling (3s interval) that broadcast events to connected WebSocket clients. Tracks previous agent statuses to emit granular agent:status change events. Gracefully handles in-flight writes and tmux unavailability. Closes #74
- TerminalView subscribes to WebSocket terminal events for a specific agent - Initial log fetch via REST (GET /api/agents/:id/logs, last 200 lines) - Auto-scroll to bottom on new output via ScrollViewReader - Monospace font with black background / green text - Kill button in toolbar with confirmation dialog - TerminalInputBar with monospaced text field + send button - Sends input via WebSocket terminal:input command - Chains onto existing onMessage handler to avoid overwriting AppState's handler Closes #84
…ent statuses Add Fastify-based HTTP API with Bearer token auth: - GET /api/status — full manifest with refreshed agent statuses and lifecycle - GET /api/worktrees/:id — single worktree detail by ID or name - GET /api/worktrees/:id/diff — branch diff in numstat format Closes #68
Observable state layer for the iOS app: - AppState: manages connections list, active connection, REST/WS lifecycle - ManifestStore: manifest cache with full refresh and incremental WS updates - UserDefaults-backed connection persistence - Auto-connect to last-used server on launch - Connection switching (disconnect current, connect to new) - Error state management with user-facing messages Closes #82
SwiftUI form for spawning new worktrees with agents: - Name field (required, used as branch suffix) - Multi-line prompt TextEditor - Agent type picker (claude/codex/opencode) - Count stepper (1-10) - Base branch picker (derived from manifest) - Quick prompt templates section with toggle selection - Form validation (name required, prompt or template required) - Loading state during spawn with disabled controls - Clear form on success - Navigate to WorktreeDetailView on completion Closes #85
Per-agent terminal output streaming with efficient longest-common-suffix diff algorithm that only sends new lines, not the full buffer. - Per-agent subscriptions with lazy initialization (polling starts on first subscriber, stops when last unsubscribes) - 500ms polling interval for tmux pane content via capturePane() - Longest common suffix diff — finds overlap between previous and current buffer snapshots to emit only new lines - Shared timer across clients watching the same agent (single capture per interval regardless of subscriber count) - Auto-cleanup when subscriber count drops to 0 - Error handling for dead/missing panes with subscriber notification - Injectable capture function for testability - 23 tests covering diff algorithm, subscription lifecycle, shared timer, polling behavior, error handling, and cleanup Closes #75
Add self-signed CA and server certificate generation using hand-coded ASN.1/DER encoding with only Node.js built-in crypto. Certificates include LAN IP SANs for mobile companion app connectivity. Implements cert reuse logic (CA preserved across server cert rotations), pairing URL generation, and LAN IP detection. Closes #65
Add POST /api/spawn endpoint as a Fastify plugin that creates worktrees and launches agents. Includes JSON Schema validation for request body (name required, agent type, prompt/template, base branch, count 1-20, template vars), template variable rendering, and full test coverage with mocked core modules. Closes #70
…rompts Add read-only Fastify route plugin with three endpoints: - GET /api/config — returns agent configuration from config.yaml - GET /api/templates — lists templates with source tracking (local vs global) - GET /api/prompts — lists prompts with deduplication across local/global Closes #72
Add Fastify route plugin with four agent interaction endpoints: - GET /api/agents/:id/logs?lines=N — capture tmux pane output (default 200) - POST /api/agents/:id/send — send text/keys with raw, literal, or with-enter modes - POST /api/agents/:id/kill — kill agent via core kill logic with manifest update - POST /api/agents/:id/restart — restart agent with optional prompt override Includes input validation, PpgError-to-HTTP status mapping, and 16 tests covering all endpoints with mocked core functions. Closes #69
Add WebSocket endpoint with token auth, client tracking, command dispatch, and typed discriminated union events for real-time server-client communication. - ws://<host>:<port>/ws?token=<token> endpoint via HTTP upgrade - Token validation from query parameter (sync and async) - Client set with per-client terminal subscription state - Command dispatch: ping, terminal:subscribe/unsubscribe, terminal:input - Server events: pong, manifest:updated, agent:status, terminal:output, error - broadcast() and sendEvent() helpers - Proper cleanup on disconnect and server shutdown - 26 tests covering auth, commands, broadcast, and cleanup Closes #73
- SettingsView: server list with connect/disconnect, swipe-to-delete with confirmation dialog, test connection button with async feedback, QR scanner sheet, connection status badge, about section with version and GitHub link - AddServerView: dedicated form for manual server entry (name, host, port, token) with validation, replacing the limited alert-based approach Closes #86
Add Fastify server infrastructure and three worktree action endpoints: - POST /api/worktrees/:id/merge — squash or no-ff merge with force flag and cleanup - POST /api/worktrees/:id/kill — kill all running agents in a worktree - POST /api/worktrees/:id/pr — create GitHub PR via gh CLI and store URL in manifest Includes 15 tests covering success paths, error handling (404, 409, 500, 502), merge strategies, cleanup toggles, draft PR flag, and gh/push failures. Closes #71
Add `ppg serve` command that starts an HTTPS server with self-signed TLS certificates and displays a pairing QR code for iOS device connectivity. - Generate self-signed EC certs (prime256v1) stored in .ppg/certs/ - Encode pairing URL: ppg://connect?host=...&port=...&ca=...&token=... - Include CA SHA-256 fingerprint for certificate pinning - Print auth token to stdout at generation time - Render QR code via qrcode-terminal (small mode) - Only display QR in interactive terminal (skip in --daemon mode) - Support --json for machine-readable output - Add qrcode-terminal to dependencies Closes #88
Register a Fastify plugin that catches unhandled route errors and returns structured JSON responses. PpgErrors map to their code/message, validation errors return VALIDATION_ERROR, and unexpected errors return INTERNAL_ERROR without leaking stack traces in production. Closes #87
…g/integration # Conflicts: # src/lib/errors.ts # src/lib/paths.ts
… ppg/integration # Conflicts: # package-lock.json
Resolve conflicts: combine daemon subcommand structure with token/TLS options.
…o ppg/integration
- Remove duplicate serve subcommand group in cli.ts - Rewrite spawn.test.ts to mock performSpawn instead of removed spawnAgentBatch - Remove stale sessionName from SpawnResult test fixture
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive architecture overhaul across three major areas: a new iOS mobile client with SwiftUI UI and network stack, a complete backend server implementation with WebSocket and authentication, and refactored CLI operations extracted into reusable core modules. The changes span ~4,500+ lines across 140+ files, adding support for cross-platform agent management, real-time terminal streaming, token-based authentication, and TLS certificate management. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (18)
src/core/operations/merge.ts-45-49 (1)
45-49:⚠️ Potential issue | 🟠 Major
dryRuncurrently performs a manifest write.
updateManifest(...refreshAllAgentStatuses...)executes before the dry-run return, sodryRuncan still persist state changes.🛠️ Proposed fix
- // Load and refresh manifest - await requireManifest(projectRoot); - const manifest = await updateManifest(projectRoot, async (m) => { - return refreshAllAgentStatuses(m, projectRoot); - }); + // Load and refresh manifest + const manifest = dryRun + ? await refreshAllAgentStatuses(await requireManifest(projectRoot), projectRoot) + : await updateManifest(projectRoot, (m) => refreshAllAgentStatuses(m, projectRoot));Also applies to: 66-78
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/operations/merge.ts` around lines 45 - 49, The dry-run path is performing a manifest write because updateManifest(projectRoot, async (m) => refreshAllAgentStatuses(m, projectRoot)) runs unconditionally; change the logic so that when dryRun is true you call refreshAllAgentStatuses directly (or operate on an in-memory clone) without invoking updateManifest, and only call updateManifest to persist changes when dryRun is false—apply the same guard for the other occurrence around the block at the section covering lines 66–78 so no manifest writes occur during dry runs.ios/PPGMobile/PPGMobile/Models/ServerConnection.swift-116-132 (1)
116-132:⚠️ Potential issue | 🟠 MajorReject non-routable
0.0.0.0during QR import.
fromQRCodecurrently accepts and storeshost = "0.0.0.0", which is not a usable remote target for an iOS client.🛠️ Proposed fix
- guard let host = params["host"], isValidHost(host), + guard let host = params["host"], isValidHost(host), host != "0.0.0.0", let token = params["token"], !token.isEmpty else { return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/Models/ServerConnection.swift` around lines 116 - 132, The QR import currently accepts host "0.0.0.0" which is non-routable; change the parsing logic in the ServerConnection QR import (the method that calls isValidHost and returns ServerConnection) to reject "0.0.0.0" instead of mapping it to "Local Mac". Concretely, update the guard that checks host/token to also ensure host != "0.0.0.0" (or update isValidHost to return false for "0.0.0.0"), so that the method returns nil for that value rather than creating a ServerConnection via the ServerConnection(...) initializer.ios/PPGMobile/PPGMobile/Models/ServerConnection.swift-69-74 (1)
69-74:⚠️ Potential issue | 🟠 MajorAvoid putting auth tokens in the WebSocket URL.
Line 73 places
tokenin query params, which is commonly exposed through logs, analytics, and crash reports. Prefer sending auth via headers.🔐 Proposed fix
- var webSocketURL: URL? { - makeURL( - scheme: wsScheme, - path: "/ws", - queryItems: [URLQueryItem(name: "token", value: token)] - ) - } + var webSocketURL: URL? { + makeURL(scheme: wsScheme, path: "/ws") + } + + var webSocketRequest: URLRequest? { + guard let url = webSocketURL else { return nil } + var request = URLRequest(url: url) + request.setValue(authorizationHeader, forHTTPHeaderField: "Authorization") + return request + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/Models/ServerConnection.swift` around lines 69 - 74, The webSocketURL computed property currently embeds the auth token in query params (token via makeURL), which is insecure; change webSocketURL to build the ws URL without the token (omit the URLQueryItem) and move sending authentication to the WebSocket handshake headers instead (e.g., include token as an Authorization or Sec-WebSocket-Protocol header when constructing the URLSessionWebSocketTask / request). Update any connection code that uses webSocketURL to set the header on the URLRequest used to create the socket, and leave wsScheme and makeURL unchanged except to remove the token query.src/core/operations/spawn.ts-432-447 (1)
432-447:⚠️ Potential issue | 🟠 Major
--opencan attach to the wrong window for existing worktrees.With
--worktreeandsplit !== true, Line 433 creates the first agent in a new window, but Line 446 still opens the originalwindowTarget.🛠️ Proposed fix
// Only open Terminal window when explicitly requested via --open (fire-and-forget) if (options.open === true) { - openTerminalWindow(manifest.sessionName, windowTarget, wt.name).catch(() => {}); + const openTarget = agents[0]?.tmuxTarget ?? windowTarget; + openTerminalWindow(manifest.sessionName, openTarget, wt.name).catch(() => {}); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/operations/spawn.ts` around lines 432 - 447, The bug is that openTerminalWindow(manifest.sessionName, windowTarget, wt.name) uses the original windowTarget even when the first agent was actually created in a different window (because reuseWindowForFirstAgent is false); fix it by using the actual window produced by the spawn code instead of the original windowTarget — capture the window identifier from the spawn result or the onAgentSpawned callback (e.g., read a window or target field on agentEntry passed to onAgentSpawned or return the created window from the spawn call) and call openTerminalWindow(manifest.sessionName, actualWindowTarget, wt.name) (or move the options.open handling inside onAgentSpawned to open the terminal for the real window); update references to reuseWindowForFirstAgent, onAgentSpawned, openTerminalWindow, options.open and windowTarget accordingly.src/core/merge.ts-33-41 (1)
33-41:⚠️ Potential issue | 🟠 MajorRunning-agent gate uses potentially stale statuses.
The pre-merge block checks
wt.agentsdirectly, but those statuses may be outdated. This can allow merges while agents are still running.Based on learnings,
src/**/*.tsshould detect agent status usingcore/agent.ts:checkAgentStatus()with the defined signal priority.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/merge.ts` around lines 33 - 41, The merge pre-check is reading potentially stale statuses from wt.agents; replace that by asynchronously resolving each agent's current status via core/agent.ts:checkAgentStatus for every agent in wt.agents (use agent.id or the agent objects as keys), await Promise.all to get up-to-date statuses, then filter for status === 'running' and, if any exist and force is false, throw the same PpgError (preserve message and 'AGENTS_RUNNING' code). Ensure you import/use checkAgentStatus and handle promise rejections so failures to check status surface as errors rather than allowing the merge to proceed.src/commands/merge.ts-16-35 (1)
16-35:⚠️ Potential issue | 🟠 Major
--jsonmode is currently broken for dry-run and polluted for normal runs.In JSON mode, human-readable
info/success/warnlogs still print, and dry-run returns before any JSON payload is emitted. This breaks machine consumption.Proposed fix
export async function mergeCommand(worktreeId: string, options: MergeCommandOptions): Promise<void> { const projectRoot = await getRepoRoot(); - - if (options.dryRun) { - info('Dry run — no changes will be made'); - } - const result = await performMerge({ projectRoot, worktreeRef: worktreeId, strategy: options.strategy, cleanup: options.cleanup, dryRun: options.dryRun, force: options.force, }); + if (options.json) { + output({ + success: true, + dryRun: result.dryRun, + merged: result.merged, + worktreeId: result.worktreeId, + branch: result.branch, + baseBranch: result.baseBranch, + strategy: result.strategy, + cleaned: result.cleaned, + selfProtected: result.selfProtected || undefined, + }, true); + return; + } + if (result.dryRun) { info(`Would merge branch ${result.branch} into ${result.baseBranch} using ${result.strategy} strategy`); if (options.cleanup !== false) { info(`Would remove worktree ${result.worktreeId} and delete branch ${result.branch}`); } return; } @@ - if (options.json) { - output({ - success: true, - worktreeId: result.worktreeId, - branch: result.branch, - baseBranch: result.baseBranch, - strategy: result.strategy, - cleaned: result.cleaned, - selfProtected: result.selfProtected || undefined, - }, true); - } }As per coding guidelines,
src/commands/**/*.tsmust support--jsonusing dual output (output(data, json)/outputError(error, json)), without mixing human logs into JSON mode.Also applies to: 37-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/merge.ts` around lines 16 - 35, The CLI prints human-readable logs during JSON mode and returns early on dry-run without emitting JSON; update the merge command to use the dual-output helpers (output(data, options.json) / outputError(err, options.json)) instead of info/success/warn when options.json is true, and ensure performMerge/dry-run handling emits a JSON payload from the result before returning (i.e., call output({ type: 'dry-run', ...result }, options.json) rather than printing info and returning); also remove or gate any other human logs around performMerge/use of result and replace them with output/outputError so normal runs still show human logs while JSON runs send only structured output.ios/PPGMobile/PPGMobile/Networking/PPGClient.swift-165-169 (1)
165-169:⚠️ Potential issue | 🟠 MajorAPI contract mismatch: client sends wrong parameters to server.
The server's
/api/agents/:id/sendendpoint expects{ text: string; mode?: 'raw' | 'literal' | 'with-enter' }(see lines 102-104 of src/server/routes/agents.ts), but this method sends{ text, keys, enter }. The server will ignore bothkeysandenterparameters and always use the default'with-enter'mode. Update the method to send the correctmodeparameter instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/Networking/PPGClient.swift` around lines 165 - 169, The sendToAgent method is sending the wrong body shape ({text, keys, enter}) while the server expects {text, mode?: 'raw' | 'literal' | 'with-enter'}; update PPGClient.sendToAgent to remove keys/enter and instead compute a mode string from those booleans (map keys/enter -> one of "raw", "literal", or "with-enter") and send body ["text": text, "mode": mode] so the POST to "/api/agents/\(agentId)/send" matches the server contract.src/server/routes/worktrees.ts-35-51 (1)
35-51:⚠️ Potential issue | 🟠 MajorRoute-local
errorReplyleaks internals and can drift from the global error contract.This block duplicates HTTP status mapping and sends raw unknown error messages (
err.message) to clients. That bypasses the centralized masking/formatting insrc/server/error-handler.tsand can create inconsistent API semantics.Based on learnings: Use `PpgError` hierarchy with typed error codes: `TMUX_NOT_FOUND`, `NOT_GIT_REPO`, `NOT_INITIALIZED`, `MANIFEST_LOCK`, `WORKTREE_NOT_FOUND`, `AGENT_NOT_FOUND`, `MERGE_FAILED`, `INVALID_ARGS`, `AGENTS_RUNNING`, `WAIT_TIMEOUT`, `AGENTS_FAILED`, `NO_SESSION_ID`.Suggested direction: rely on the global Fastify error handler
- try { - const wt = await resolveWorktreeFromRequest(projectRoot, request.params.id); - const { strategy, cleanup, force } = request.body ?? {}; - - const result = await mergeWorktree(projectRoot, wt, { strategy, cleanup, force }); - - return { success: true, ...result }; - } catch (err) { - return errorReply(reply, err); - } + const wt = await resolveWorktreeFromRequest(projectRoot, request.params.id); + const { strategy, cleanup, force } = request.body ?? {}; + const result = await mergeWorktree(projectRoot, wt, { strategy, cleanup, force }); + return { success: true, ...result };Apply the same
try/catchremoval to the other route handlers and let thrown errors be normalized byregisterErrorHandler.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/routes/worktrees.ts` around lines 35 - 51, The route-local errorReply function in worktrees.ts duplicates status mapping and exposes raw error messages; remove use of errorReply and stop catching/formatting errors in route handlers so that thrown PpgError instances are handled by the global registerErrorHandler in src/server/error-handler.ts; where you previously used errorReply or returned mapped statuses (function errorReply and any catch blocks around worktree route handlers), rethrow or let the original error propagate so the centralized handler can map the PpgError codes (use the canonical codes like WORKTREE_NOT_FOUND, AGENT_NOT_FOUND, NOT_INITIALIZED, MANIFEST_LOCK, MERGE_FAILED, INVALID_ARGS, AGENTS_RUNNING, WAIT_TIMEOUT, AGENTS_FAILED, NO_SESSION_ID, TMUX_NOT_FOUND, NOT_GIT_REPO) and mask unknown errors consistently.src/core/prompt.ts-18-24 (1)
18-24:⚠️ Potential issue | 🟠 Major
readMdNamesshould only suppress missing-directory errors.Returning
[]for every filesystem failure hides real operational problems (e.g., EACCES/EIO) and can silently degrade/api/promptsoutput.Safer error handling (ENOENT fallback only)
async function readMdNames(dir: string): Promise<string[]> { try { const files = await fs.readdir(dir); return files.filter((f) => f.endsWith('.md')).map((f) => f.replace(/\.md$/, '')).sort(); - } catch { - return []; + } catch (error) { + if (hasErrorCode(error, 'ENOENT')) return []; + throw error; } } + +function hasErrorCode(error: unknown, code: string): boolean { + return typeof error === 'object' + && error !== null + && 'code' in error + && (error as { code?: unknown }).code === code; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/prompt.ts` around lines 18 - 24, The readMdNames function is currently swallowing all filesystem errors; change its catch to only suppress missing-directory errors by checking the error code (e.g., if (err?.code === 'ENOENT') return []), and rethrow any other errors so operational problems (EACCES, EIO, etc.) surface; keep the rest of the logic (fs.readdir, filter .md, strip extension, sort) unchanged and ensure the thrown error propagates to callers of readMdNames.src/server/routes/config.ts-26-67 (1)
26-67:⚠️ Potential issue | 🟠 Major
configRoutesplugin is not registered in the main server and has inconsistent path declarations.The
config.tsroute module hardcodes/api/prefixes on all paths, but it's never registered insrc/server/index.ts. Meanwhile,agentRoutesandworktreeRoutesfollow a consistent pattern: they're registered withprefix: '/api'and declare unprefixed paths (e.g.,'/config'instead of'/api/config'). Either registerconfigRouteswith the same prefix pattern and remove/api/from the route declarations, or remove this unused code if it's no longer needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/routes/config.ts` around lines 26 - 67, The config routes module defines routes as app.get('/api/config'), app.get('/api/templates') and app.get('/api/prompts') but is not registered like the other plugins; fix by making it consistent with agentRoutes/worktreeRoutes: change the route declarations inside the module to unprefixed paths ('/config', '/templates', '/prompts') and ensure the module exports the plugin (e.g., configRoutes) and is registered on the main server with prefix: '/api' (same place where agentRoutes and worktreeRoutes are registered); alternatively, if the module is unused, remove the configRoutes file to avoid dead code.ios/PPGMobile/PPGMobile/State/AppState.swift-97-125 (1)
97-125:⚠️ Potential issue | 🟠 MajorGuard async connect/WS updates with a connection generation token.
State is applied after awaits without checking whether that connection is still current. A stale in-flight connect or old socket callback can mutate UI state after Line 128
disconnect()or a quick reconnect.🛡️ Suggested fix pattern
final class AppState { + private var connectionGeneration = 0 @@ func connect(to connection: ServerConnection) async { guard !isConnecting else { return } + connectionGeneration += 1 + let generation = connectionGeneration @@ do { try await client.testConnection() } catch { isConnecting = false errorMessage = "Cannot reach server: \(error.localizedDescription)" return } + guard generation == connectionGeneration else { + isConnecting = false + return + } activeConnection = connection @@ - startWebSocket(for: connection) + startWebSocket(for: connection, generation: generation) await manifestStore.refresh() + guard generation == connectionGeneration else { return } isConnecting = false } @@ func disconnect() { + connectionGeneration += 1 stopWebSocket() activeConnection = nil @@ - private func startWebSocket(for connection: ServerConnection) { + private func startWebSocket(for connection: ServerConnection, generation: Int) { stopWebSocket() let ws = WebSocketManager(url: connection.webSocketURL) @@ ws.onEvent = { [weak self] event in Task { `@MainActor` in - self?.handleWebSocketEvent(event) + guard let self, generation == self.connectionGeneration else { return } + self.handleWebSocketEvent(event) } }Also applies to: 128-133, 200-216, 223-241
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/State/AppState.swift` around lines 97 - 125, connect(to:) and websocket callbacks mutate state after await points without verifying the operation is still for the intended connection; introduce a generation/token (e.g., connectionGeneration Int or UUID property on AppState) that is incremented when disconnect() or a new connect starts, capture the current token at the start of connect(to:) and in startWebSocket, then before applying any state changes after awaits or inside async callbacks (setting activeConnection, isConnecting, errorMessage, UserDefaults, manifestStore.refresh completion, and socket event handlers) verify the captured token still equals the current connectionGeneration; if it differs, bail out to avoid stale updates. Ensure disconnect() increments/invalidates the token so in-flight tasks and old socket handlers cannot mutate UI state.ios/PPGMobile/PPGMobile/Models/DashboardModels.swift-127-133 (1)
127-133:⚠️ Potential issue | 🟠 MajorAvoid shipping silent no-op action methods.
Line 127–133 expose async actions that currently do nothing, which can make UI operations appear successful while no side effect occurs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/Models/DashboardModels.swift` around lines 127 - 133, The listed async methods (refresh, connect, killAgent(_:in:), restartAgent(_:in:), mergeWorktree(_:), killWorktree(_:), createPullRequest(for:)) are currently silent no-ops; replace each empty body with a non-silent placeholder: either implement the real side-effect or, if not ready, call a runtime fail/logging helper (e.g., assertionFailure or a Logger call) that clearly reports "Not implemented" and the method name so UI consumers don't assume success; update the bodies of those exact functions in DashboardModels.swift to either perform the intended work or surface a clear runtime/log message (and consider adding a TODO comment referencing the ticket/issue for later implementation).src/server/auth.ts-170-178 (1)
170-178:⚠️ Potential issue | 🟠 MajorAvoid atomic disk writes on every successful request authentication.
Line 209 + Line 210 persist
lastUsedAtfor every valid token check. This puts avoidable IO pressure on the auth hot path and can degrade throughput under load.⚡ Suggested mitigation (throttle metadata writes)
async validateToken(token: string): Promise<TokenEntry | null> { if (!token) return null; const data = await readData(); const incomingBuf = Buffer.from(hashToken(token), 'hex'); for (const entry of data.tokens) { const storedBuf = Buffer.from(entry.hash, 'hex'); if (incomingBuf.length === storedBuf.length && crypto.timingSafeEqual(incomingBuf, storedBuf)) { - entry.lastUsedAt = new Date().toISOString(); - await writeData(data); + const previous = entry.lastUsedAt; + entry.lastUsedAt = new Date().toISOString(); + if (!previous || Date.now() - Date.parse(previous) >= 60_000) { + await writeData(data); + } return cloneTokenEntry(entry); } } return null; },Also applies to: 200-211
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/auth.ts` around lines 170 - 178, The auth path currently calls writeData (which uses getWriteFileAtomic and filePath) on every successful token check to persist lastUsedAt, causing excessive disk IO; change this by throttling metadata writes: update in-memory cache.lastUsedAt on each auth but only call writeData when the previous persisted timestamp is older than a threshold (e.g., >N seconds) or by scheduling/debouncing writes (single delayed background flush) to batch updates, keeping writeData/filePath/getWriteFileAtomic usage but reducing frequency of actual filesystem writes.src/core/serve.ts-67-74 (1)
67-74:⚠️ Potential issue | 🟠 MajorUse strict PID parsing instead of
parseIntpermissive behavior.
parseIntaccepts partial strings and can treat corrupted PID files as valid.🔧 Proposed fix
- const pid = parseInt(raw, 10); - if (isNaN(pid)) { + const rawTrim = raw.trim(); + if (!/^\d+$/.test(rawTrim)) { await cleanupPidFile(pidPath); return null; } + const pid = Number(rawTrim);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/serve.ts` around lines 67 - 74, The code uses parseInt on the raw PID string which accepts partial/corrupted values; instead validate the PID strictly (e.g., trim raw and test it against /^\d+$/) before converting, so only fully numeric values are accepted; if the validation fails call cleanupPidFile(pidPath) and return null, otherwise parse to an integer (or Number) to produce pid and continue using process.kill(pid, 0) as before (update the code around raw, parseInt, pid, pidPath, cleanupPidFile, and process.kill).src/commands/serve.ts-70-75 (1)
70-75:⚠️ Potential issue | 🟠 MajorValidate and default
host/portinstead of non-null assertions.
options.port!/options.host!can becomeundefinedat runtime and still pass through to command construction.🔧 Proposed fix
- const port = options.port!; - const host = options.host!; + const port = options.port ?? 8080; + const host = options.host ?? '127.0.0.1'; + + if (!Number.isInteger(port) || port < 1 || port > 65535) { + throw new PpgError(`Invalid port: "${String(options.port)}"`, 'INVALID_ARGS'); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/serve.ts` around lines 70 - 75, The code uses non-null assertions for options.port and options.host which can be undefined; instead read options.host and options.port without "!" and apply safe defaults (e.g. defaultHost = '0.0.0.0' and defaultPort = 3000 or a configured default), validate host with VALID_HOST and validate/parse port as an integer within 1–65535 (reject and throw new PpgError('Invalid port: "...", 'INVALID_ARGS') if out of range or NaN), and then use the validated/defaulted values for the rest of the serve logic; update the variables named port and host (and any usage sites) accordingly so no non-null assertions are needed.src/server/routes/agents.ts-241-269 (1)
241-269:⚠️ Potential issue | 🟠 MajorRefresh live status before invoking restart.
oldAgentcomes from manifest and can be stale. Passing it directly into restart logic can miss killing an actually-running process.🔧 Proposed fix
const { worktree: wt, agent: oldAgent } = found; + const live = await checkAgentStatus(oldAgent, projectRoot); + const oldAgentForRestart = { ...oldAgent, status: live.status }; @@ const result = await restartAgent({ projectRoot, - agentId: oldAgent.id, + agentId: oldAgentForRestart.id, worktree: wt, - oldAgent, + oldAgent: oldAgentForRestart, sessionName: manifest.sessionName, agentConfig, promptText, });Based on learnings: Applies to src/**/*.ts : Detect agent status using
core/agent.ts:checkAgentStatus()with signal-stack priority: result file → pane exists → pane dead → current command → running.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/routes/agents.ts` around lines 241 - 269, Before calling restartAgent, refresh the live agent state instead of using the possibly-stale oldAgent from found: call checkAgentStatus(...) (from core/agent.ts) for the target agent id/worktree to get an updated agent status/object, use that updated agent in place of oldAgent when constructing agentConfig/arguments for restartAgent, and ensure any logic that decides to kill or restart uses the updated status (so restartAgent receives the current-running info rather than the manifest snapshot).src/core/operations/restart.ts-46-51 (1)
46-51:⚠️ Potential issue | 🟠 MajorRefresh live agent status before kill decision.
The kill gate is based on persisted manifest state, which can be stale. That can leave the old process running while a replacement is spawned.
🔧 Proposed fix
-import { spawnAgent, killAgent } from '../agent.js'; +import { spawnAgent, killAgent, checkAgentStatus } from '../agent.js'; @@ - if (oldAgent.status === 'running') { + const { status: liveStatus } = await checkAgentStatus(oldAgent, projectRoot); + if (liveStatus === 'running') { await killAgent(oldAgent); killedOldAgent = true; }Based on learnings: Applies to src/**/*.ts : Detect agent status using
core/agent.ts:checkAgentStatus()with signal-stack priority: result file → pane exists → pane dead → current command → running.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/operations/restart.ts` around lines 46 - 51, Before deciding to kill the previous agent in restart.ts, refresh its live status by calling the central status checker core/agent.ts:checkAgentStatus(oldAgent) and use the returned status (or update oldAgent.status) instead of the persisted manifest value; then only call killAgent(oldAgent) if that refreshed status equals 'running'. This ensures the kill gate uses the signal-stack-priority status (result file → pane exists → pane dead → current command → running) from checkAgentStatus rather than stale manifest data.src/server/index.ts-117-121 (1)
117-121:⚠️ Potential issue | 🟠 MajorRegister missing route plugins in server bootstrap.
The status, config, and spawn route implementations exist and are tested but are not imported or registered in
startServer(). Import and registerstatusRoutes,configRoutes, andspawnRoutealongside the existingagentRoutesandworktreeRoutesplugins.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/index.ts` around lines 117 - 121, startServer is only registering agentRoutes and worktreeRoutes; you need to also import and register the other route plugins so their endpoints are available. In src/server/index.ts, add dynamic imports for statusRoutes, configRoutes, and spawnRoute (similar to the worktreeRoutes import) and call await app.register(statusRoutes, { prefix: '/api' }), await app.register(configRoutes, { prefix: '/api' }), and await app.register(spawnRoute, { prefix: '/api' }) alongside the existing await app.register(agentRoutes, ...) and await app.register(worktreeRoutes, ...); use the same prefix and registration pattern so all route plugins are registered during server bootstrap.
🟡 Minor comments (9)
ios/PPGMobile/PPGMobile/Views/Terminal/TerminalInputBar.swift-15-21 (1)
15-21:⚠️ Potential issue | 🟡 MinorPrevent empty/whitespace submits from keyboard and send button.
Line 15 currently submits regardless of content, and Line 21 only checks
isEmpty, so whitespace-only input can still be sent.Suggested fix
struct TerminalInputBar: View { `@Binding` var text: String let onSend: () -> Void + + private var canSend: Bool { + !text.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty + } var body: some View { HStack(spacing: 8) { TextField("Send to terminal...", text: $text) .font(.system(.body, design: .monospaced)) .textInputAutocapitalization(.never) .autocorrectionDisabled() .textFieldStyle(.roundedBorder) - .onSubmit(onSend) + .onSubmit { + guard canSend else { return } + onSend() + } Button(action: onSend) { Image(systemName: "arrow.up.circle.fill") .font(.title2) } - .disabled(text.isEmpty) + .disabled(!canSend) } .padding(.horizontal, 12) .padding(.vertical, 8) .background(.bar) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/Views/Terminal/TerminalInputBar.swift` around lines 15 - 21, The onSubmit modifier and the Send Button allow whitespace-only input because they only check text.isEmpty; update both to validate the trimmed text before sending: make onSubmit call a small wrapper (e.g., validateAndSend) that trims text (using trimmingCharacters(in: .whitespacesAndNewlines)) and returns early if the trimmed string is empty, and change the Button .disabled condition from text.isEmpty to text.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty so the button is disabled for whitespace-only input; ensure the existing onSend logic is invoked only when the trimmed input is non-empty.ios/PPGMobile/PPGMobile/Views/Terminal/TerminalView.swift-203-222 (1)
203-222:⚠️ Potential issue | 🟡 MinorMinor race window when restoring
onMessagehandler.After releasing the lock on line 215 but before restoring
wsManager.onMessageon line 216, a concurrentdispatchcall could find no state for this manager and drop the message. While this is a narrow window and unlikely in practice (terminal output loss is recoverable), consider holding the lock through the restoration or accepting this as a known limitation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/Views/Terminal/TerminalView.swift` around lines 203 - 222, In removeSubscriber, there is a race between unlocking and restoring wsManager.onMessage: move the restoration of wsManager.onMessage (using state.previousOnMessage) to occur while the lock is still held (i.e., set wsManager.onMessage = state.previousOnMessage before calling lock.unlock()), or alternatively acquire the lock again around the restoration to guarantee atomicity; update the code paths that remove the state (states.removeValue and states[managerID] assignment) so the previousOnMessage is restored while the lock protects the shared states map and the wsManager.onMessage update.src/core/operations/kill.test.ts-107-109 (1)
107-109:⚠️ Potential issue | 🟡 Minor
updateManifestmock drops state across calls.Line 108 always starts from a fresh snapshot, so multi-step manifest flows can pass even if sequential mutations are broken.
🧪 Proposed fix
- vi.mocked(updateManifest).mockImplementation(async (_root: string, updater: (m: any) => any) => { - return updater(currentManifest()); - }); + vi.mocked(updateManifest).mockImplementation(async (_root: string, updater) => { + _manifest = await updater(currentManifest()); + return _manifest; + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/operations/kill.test.ts` around lines 107 - 109, The mocked updateManifest currently returns updater(currentManifest()) using a fresh snapshot each call, which drops state between invocations; change the mock used in vi.mocked(updateManifest) to keep an internal manifest variable (initialized from currentManifest()), call the provided updater on that stored manifest on each invocation, replace the stored manifest with the updater result, and return the updated manifest so sequential mutations are preserved across multiple calls to updateManifest.src/core/operations/spawn.test.ts-137-139 (1)
137-139:⚠️ Potential issue | 🟡 Minor
updateManifestmock resets state on every call.This prevents realistic validation of multi-step manifest transitions in default-path tests.
🧪 Proposed fix
+let manifestState: Manifest; + beforeEach(() => { vi.clearAllMocks(); setupDefaultMocks(); + manifestState = makeManifestState(); + mockedReadManifest.mockResolvedValue(manifestState); + mockedUpdateManifest.mockImplementation(async (_root, updater) => { + manifestState = await updater(structuredClone(manifestState)); + return manifestState; + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/operations/spawn.test.ts` around lines 137 - 139, The current mockedUpdateManifest implementation always calls updater(makeManifestState()), resetting manifest state on every call and preventing multi-step transitions; change the mock to hold a mutable manifestState initialized once (e.g., let manifestState = makeManifestState()) and on each call run manifestState = await updater(manifestState) and return manifestState, and ensure manifestState is reinitialized in the test setup (beforeEach) so tests start from the intended initial state; update references to mockedUpdateManifest and makeManifestState accordingly.src/server/ws/watcher.ts-68-69 (1)
68-69:⚠️ Potential issue | 🟡 MinorWatcher errors are being silently dropped.
Both watcher
errorhandlers ignore failures. This makes it hard to detect when filesystem monitoring degrades.Proposed fix
- fileWatcher.on('error', () => {}); + fileWatcher.on('error', (err) => onError?.(err)); @@ - dirWatcher.on('error', () => {}); + dirWatcher.on('error', (err) => onError?.(err));Also applies to: 88-89
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/ws/watcher.ts` around lines 68 - 69, The current fileWatcher error handlers silently swallow errors; update the handler(s) (the one attached to fileWatcher and the second watcher error handler used later) to accept the error argument and surface it instead of ignoring it—e.g. replace () => {} with (err) => { processLogger.error('fileWatcher error', err); throw err; } or, if throwing is inappropriate in context, at minimum log via processLogger.error/console.error and re-emit or propagate the error to the surrounding EventEmitter so callers can react.src/commands/kill.ts-75-81 (1)
75-81:⚠️ Potential issue | 🟡 MinorCondition logic may produce unexpected output.
Line 76's condition
result.killed.length > 0 || !result.skipped?.lengthwill show success message even whenkilled.length === 0andskippedis undefined/empty. This may not be the intended behavior—consider whether a success message should be shown when nothing was killed.🔍 Analysis
Current behavior:
killed: [], skipped: undefined→ shows "Killed 0 agent(s)..." ✓ (success shown)killed: [], skipped: ['ag-123']→ does NOT show success (only skipped warning)killed: ['ag-456'], skipped: ['ag-123']→ shows "Killed 1 agent(s)..." ✓If the intent is to show success only when something was actually killed:
- } else if (options.worktree) { - if (result.killed.length > 0 || !result.skipped?.length) { + } else if (options.worktree) { + if (result.killed.length > 0) { success(`Killed ${result.killed.length} agent(s) in worktree ${options.worktree}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/kill.ts` around lines 75 - 81, The success message logic is too permissive: in the options.worktree branch the condition `result.killed.length > 0 || !result.skipped?.length` can show "Killed 0 agent(s)..." when nothing was killed; update the condition to only show success when there are actually killed agents (use `result.killed.length > 0`) and leave the `warn` call for `result.skipped?.length` unchanged so skipped agents still produce a warning; modify the check around success() in the block that references options.worktree, result.killed, result.skipped, success(), and warn().ios/PPGMobile/PPGMobile/Views/Settings/SettingsView.swift-211-219 (1)
211-219:⚠️ Potential issue | 🟡 MinorDuplicate connection attempt and missing
await.Two issues:
addConnectionisasyncbut not awaited, so the connection may complete before it's saved.- Per
AppState.addConnection(_:connectImmediately:), the defaultconnectImmediately: truealready callsconnect(to:)internally, making line 215 redundant.🔧 Suggested fix
private func handleQRScan(_ result: String) { showQRScanner = false if let conn = ServerConnection.fromQRCode(result) { - appState.addConnection(conn) - Task { await appState.connect(to: conn) } + Task { await appState.addConnection(conn) } } else { showQRError = true } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/Views/Settings/SettingsView.swift` around lines 211 - 219, handleQRScan currently calls async addConnection without awaiting and then redundantly calls connect(to:), causing duplicate connection attempts; change the flow to await appState.addConnection(conn) and remove the separate Task { await appState.connect(to: conn) } call (or alternatively call addConnection(conn, connectImmediately: false) and then await connect), so that you either await the async addConnection(conn) which will handle connecting, or explicitly disable immediate connect and then await connect—to fix, update handleQRScan to await the async addConnection call and eliminate the redundant connect(to:) invocation.ios/PPGMobile/PPGMobile/Views/Settings/SettingsView.swift-54-56 (1)
54-56:⚠️ Potential issue | 🟡 MinorIncorrect expected format in error message.
The error message states
ppg://host:port/token, but perServerConnection.fromQRCode, the actual format isppg://connect?host=X&port=Y&token=Z&ca=.... This could mislead users attempting manual QR generation.📝 Suggested fix
} message: { - Text("The scanned code is not a valid ppg server. Expected format: ppg://host:port/token") + Text("The scanned code is not a valid ppg server. Expected format: ppg://connect?host=...&port=...&token=...") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/Views/Settings/SettingsView.swift` around lines 54 - 56, The alert text in SettingsView (the Text shown in the alert) incorrectly tells users the QR format is "ppg://host:port/token"; update that message to match the actual parser in ServerConnection.fromQRCode by describing the expected query format (e.g. "ppg://connect?host=HOST&port=PORT&token=TOKEN&ca=...") so the UI and the ServerConnection.fromQRCode parser agree; locate the Text(...) inside SettingsView.swift where the invalid message is defined and replace it with the correct expected URL/query format wording.ios/PPGMobile/PPGMobileTests/ServerConnectionTests.swift-110-120 (1)
110-120:⚠️ Potential issue | 🟡 MinorQR parsing assertions conflict with current
fromQRCodeport behavior.
fromQRCodecurrently defaults missing/invalidportto7700, so these== nilexpectations are inconsistent with the model logic. Align either the tests or parser contract.Possible test alignment (if default-port behavior is intentional)
- `#expect`(ServerConnection.fromQRCode("ppg://connect?host=x&token=t") == nil) // no port + `#expect`(ServerConnection.fromQRCode("ppg://connect?host=x&token=t")?.port == 7700) // default port - `#expect`(ServerConnection.fromQRCode("ppg://connect?host=x&port=abc&token=t") == nil) + `#expect`(ServerConnection.fromQRCode("ppg://connect?host=x&port=abc&token=t")?.port == 7700)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobileTests/ServerConnectionTests.swift` around lines 110 - 120, Update the tests to match ServerConnection.fromQRCode's default-port behavior: in rejectsMissingFields() assert that QR strings missing the port still return a non-nil ServerConnection (not nil) and verify the returned object's port equals 7700; keep assertions that missing host or missing token return nil; in rejectsNonNumericPort() assert a non-nil result and that its port equals 7700 instead of expecting nil; reference ServerConnection.fromQRCode, rejectsMissingFields, and rejectsNonNumericPort when making the changes.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (92)
.gitignorePPG CLI/PPG CLI/WebSocketManager.swiftPPG CLI/PPG CLITests/WebSocketManagerTests.swiftios/PPGMobile/PPGMobile/App/ContentView.swiftios/PPGMobile/PPGMobile/App/PPGMobileApp.swiftios/PPGMobile/PPGMobile/Assets.xcassets/AppIcon.appiconset/Contents.jsonios/PPGMobile/PPGMobile/Assets.xcassets/Contents.jsonios/PPGMobile/PPGMobile/Models/AgentVariant.swiftios/PPGMobile/PPGMobile/Models/DashboardModels.swiftios/PPGMobile/PPGMobile/Models/Manifest.swiftios/PPGMobile/PPGMobile/Models/ServerConnection.swiftios/PPGMobile/PPGMobile/Models/ServerConnectionTests.swiftios/PPGMobile/PPGMobile/Networking/PPGClient.swiftios/PPGMobile/PPGMobile/Networking/TokenStorage.swiftios/PPGMobile/PPGMobile/State/AppState.swiftios/PPGMobile/PPGMobile/State/ManifestStore.swiftios/PPGMobile/PPGMobile/Views/Dashboard/AgentRow.swiftios/PPGMobile/PPGMobile/Views/Dashboard/DashboardView.swiftios/PPGMobile/PPGMobile/Views/Dashboard/WorktreeCard.swiftios/PPGMobile/PPGMobile/Views/Dashboard/WorktreeDetailView.swiftios/PPGMobile/PPGMobile/Views/Settings/AddServerView.swiftios/PPGMobile/PPGMobile/Views/Settings/QRScannerView.swiftios/PPGMobile/PPGMobile/Views/Settings/SettingsView.swiftios/PPGMobile/PPGMobile/Views/Spawn/SpawnView.swiftios/PPGMobile/PPGMobile/Views/Terminal/TerminalInputBar.swiftios/PPGMobile/PPGMobile/Views/Terminal/TerminalView.swiftios/PPGMobile/PPGMobileTests/AgentVariantTests.swiftios/PPGMobile/PPGMobileTests/ManifestTests.swiftios/PPGMobile/PPGMobileTests/ServerConnectionTests.swiftios/PPGMobile/project.ymlpackage.jsonsrc/cli.tssrc/commands/kill.tssrc/commands/list.tssrc/commands/merge.tssrc/commands/pr.tssrc/commands/restart.tssrc/commands/serve.test.tssrc/commands/serve.tssrc/commands/spawn.test.tssrc/commands/spawn.tssrc/commands/status.tssrc/core/agent.tssrc/core/kill.test.tssrc/core/kill.tssrc/core/lifecycle.tssrc/core/merge.test.tssrc/core/merge.tssrc/core/operations/kill.test.tssrc/core/operations/kill.tssrc/core/operations/merge.test.tssrc/core/operations/merge.tssrc/core/operations/restart.test.tssrc/core/operations/restart.tssrc/core/operations/spawn.test.tssrc/core/operations/spawn.tssrc/core/pr.tssrc/core/prompt.test.tssrc/core/prompt.tssrc/core/serve.test.tssrc/core/serve.tssrc/core/spawn.tssrc/core/tls.tssrc/lib/errors.tssrc/lib/paths.test.tssrc/lib/paths.tssrc/server/auth.test.tssrc/server/auth.tssrc/server/error-handler.test.tssrc/server/error-handler.tssrc/server/index.test.tssrc/server/index.tssrc/server/routes/agents.test.tssrc/server/routes/agents.tssrc/server/routes/config.test.tssrc/server/routes/config.tssrc/server/routes/spawn.test.tssrc/server/routes/spawn.tssrc/server/routes/status.test.tssrc/server/routes/status.tssrc/server/routes/worktrees.test.tssrc/server/routes/worktrees.tssrc/server/tls.test.tssrc/server/tls.tssrc/server/ws/events.tssrc/server/ws/handler.test.tssrc/server/ws/handler.tssrc/server/ws/terminal.test.tssrc/server/ws/terminal.tssrc/server/ws/watcher.test.tssrc/server/ws/watcher.tssrc/test-fixtures.ts
Comment on lines
+3
to
+7
| { | ||
| "idiom": "universal", | ||
| "platform": "ios", | ||
| "size": "1024x1024" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
APPICON_DIR="ios/PPGMobile/PPGMobile/Assets.xcassets/AppIcon.appiconset"
JSON_FILE="$APPICON_DIR/Contents.json"
python - <<'PY'
import json, os, glob
appicon_dir = "ios/PPGMobile/PPGMobile/Assets.xcassets/AppIcon.appiconset"
json_file = f"{appicon_dir}/Contents.json"
with open(json_file, "r", encoding="utf-8") as f:
data = json.load(f)
images = data.get("images", [])
print(f"slots: {len(images)}")
for i, slot in enumerate(images):
missing = [k for k in ("idiom", "platform", "size", "filename", "scale") if k not in slot]
print(f"slot[{i}] missing: {missing}" if missing else f"slot[{i}] keys: OK")
if "filename" in slot:
icon_path = os.path.join(appicon_dir, slot["filename"])
print(f"slot[{i}] file exists: {os.path.exists(icon_path)} ({slot['filename']})")
files = [os.path.basename(p) for p in glob.glob(f"{appicon_dir}/*") if os.path.isfile(p)]
print("files in appiconset:", files)
PYRepository: 2witstudios/ppg-cli
Length of output: 149
Add the AppIcon image file and bind it to the slot with filename and scale.
The slot is missing filename and scale fields, and critically, no icon image files exist in the appiconset directory. Both must be addressed: add the actual icon file (e.g., AppIcon-1024.png) and update Contents.json to reference it.
Suggested patch
{
"images": [
{
+ "filename": "AppIcon-1024.png",
"idiom": "universal",
"platform": "ios",
+ "scale": "1x",
"size": "1024x1024"
}
],📝 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.
Suggested change
| { | |
| "idiom": "universal", | |
| "platform": "ios", | |
| "size": "1024x1024" | |
| } | |
| { | |
| "filename": "AppIcon-1024.png", | |
| "idiom": "universal", | |
| "platform": "ios", | |
| "scale": "1x", | |
| "size": "1024x1024" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ios/PPGMobile/PPGMobile/Assets.xcassets/AppIcon.appiconset/Contents.json`
around lines 3 - 7, Add the missing AppIcon image file to the appiconset and
update Contents.json to reference it: place a 1024×1024 PNG (e.g.,
AppIcon-1024.png) in ios/PPGMobile/PPGMobile/Assets.xcassets/AppIcon.appiconset,
then modify the entry with idiom "universal"/platform "ios"/size "1024x1024" to
include "filename": "AppIcon-1024.png" and "scale": "1x" (ensure the filename
matches exactly and commit both the image and the updated Contents.json).
Comment on lines
+31
to
+63
| enum WorktreeStatus: String { | ||
| case spawning | ||
| case running | ||
| case merged | ||
| case cleaned | ||
| case merging | ||
|
|
||
| var isTerminal: Bool { | ||
| self == .merged || self == .cleaned | ||
| } | ||
|
|
||
| var label: String { rawValue.capitalized } | ||
|
|
||
| var color: Color { | ||
| switch self { | ||
| case .spawning: .yellow | ||
| case .running: .green | ||
| case .merging: .orange | ||
| case .merged: .blue | ||
| case .cleaned: .secondary | ||
| } | ||
| } | ||
|
|
||
| var icon: String { | ||
| switch self { | ||
| case .spawning: "hourglass" | ||
| case .running: "play.circle.fill" | ||
| case .merging: "arrow.triangle.merge" | ||
| case .merged: "checkmark.circle.fill" | ||
| case .cleaned: "archivebox" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify duplicate top-level enum declarations across the iOS module
rg -nP '^\s*enum\s+(WorktreeStatus|AgentStatus)\b' ios/PPGMobile/PPGMobile -g '*.swift'Repository: 2witstudios/ppg-cli
Length of output: 446
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read DashboardModels.swift - WorktreeStatus enum (lines 31-63)
echo "=== DashboardModels.swift - WorktreeStatus (lines 31-63) ==="
sed -n '31,63p' ios/PPGMobile/PPGMobile/Models/DashboardModels.swift
echo ""
echo "=== DashboardModels.swift - AgentStatus (lines 79-116) ==="
sed -n '79,116p' ios/PPGMobile/PPGMobile/Models/DashboardModels.swift
echo ""
echo "=== Manifest.swift - WorktreeStatus (lines 80-112) ==="
sed -n '80,112p' ios/PPGMobile/PPGMobile/Models/Manifest.swift
echo ""
echo "=== Manifest.swift - AgentStatus (lines 12-44) ==="
sed -n '12,44p' ios/PPGMobile/PPGMobile/Models/Manifest.swiftRepository: 2witstudios/ppg-cli
Length of output: 3618
Resolve duplicate top-level enum declarations (WorktreeStatus, AgentStatus).
Lines 31 and 79 in DashboardModels.swift redefine WorktreeStatus and AgentStatus that already exist in Manifest.swift (lines 80 and 12). This is a Swift compile blocker (Invalid redeclaration).
Note: These enums are not identical—they have different cases and properties serving different purposes. DashboardModels enums are UI-focused with color/icon properties; Manifest enums are serialization-focused with Codable support and custom decoding logic. Consolidation requires a strategy to unify them while preserving both UI and serialization concerns.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ios/PPGMobile/PPGMobile/Models/DashboardModels.swift` around lines 31 - 63,
There are duplicate enum declarations for WorktreeStatus and AgentStatus causing
redeclaration errors; keep the canonical, serialization-focused enums in
Manifest.swift (the ones with Codable and custom decoding) and remove the
duplicates from DashboardModels.swift, then add extensions in
DashboardModels.swift that add the UI-specific computed properties (e.g.,
isTerminal, label, color, icon) to WorktreeStatus and AgentStatus — do not
re-declare the enums or their Codable conformance, just define extensions that
implement the UI logic so serialization and UI concerns are separated but both
enums are usable.
Comment on lines
+98
to
+102
| let command = `ppg serve _daemon --port ${port} --host ${host}`; | ||
| if (options.token) { | ||
| command += ` --token ${options.token}`; | ||
| } | ||
| await tmux.sendKeys(windowTarget, command); |
There was a problem hiding this comment.
Avoid shell interpolation of untrusted token input.
Appending --token ${options.token} directly to a shell command allows command injection via token contents.
🔒 Proposed fix
- let command = `ppg serve _daemon --port ${port} --host ${host}`;
- if (options.token) {
- command += ` --token ${options.token}`;
- }
+ const args = ['serve', '_daemon', '--port', String(port), '--host', host];
+ if (options.token) args.push('--token', options.token);
+ const command = `ppg ${args.map((arg) => JSON.stringify(arg)).join(' ')}`;
await tmux.sendKeys(windowTarget, command);📝 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.
Suggested change
| let command = `ppg serve _daemon --port ${port} --host ${host}`; | |
| if (options.token) { | |
| command += ` --token ${options.token}`; | |
| } | |
| await tmux.sendKeys(windowTarget, command); | |
| const args = ['serve', '_daemon', '--port', String(port), '--host', host]; | |
| if (options.token) args.push('--token', options.token); | |
| const command = `ppg ${args.map((arg) => JSON.stringify(arg)).join(' ')}`; | |
| await tmux.sendKeys(windowTarget, command); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/serve.ts` around lines 98 - 102, The code currently interpolates
options.token directly into the shell command (in the command variable) and
sends it via tmux.sendKeys(windowTarget), which allows command injection; modify
the serve command construction to avoid raw shell interpolation by either (a)
sending the token as a separate, safely-escaped argument (use a dedicated
shell-escaping helper, e.g., shellEscape(options.token)) before appending
`--token`, or (b) avoid embedding the token in the command string at all and
pass it via a safe channel such as an environment variable or an args array to
the process that runs `ppg serve`; update the code around the command
construction and tmux.sendKeys usage (references: command, options.token,
tmux.sendKeys, windowTarget) to use the chosen safe approach.
Comment on lines
+197
to
+200
| export async function serveDaemonCommand(options: { port: number; host: string; token?: string }): Promise<void> { | ||
| const projectRoot = await getRepoRoot(); | ||
| await requireManifest(projectRoot); | ||
| await runServeDaemon(projectRoot, options.port, options.host); |
There was a problem hiding this comment.
Daemon entrypoint drops token and skips server bootstrap.
serveDaemonCommand currently ignores options.token and runs runServeDaemon(...) instead of starting the HTTP server path.
🔧 Proposed fix
export async function serveDaemonCommand(options: { port: number; host: string; token?: string }): Promise<void> {
const projectRoot = await getRepoRoot();
await requireManifest(projectRoot);
- await runServeDaemon(projectRoot, options.port, options.host);
+ await startServer({
+ projectRoot,
+ port: options.port,
+ host: options.host,
+ token: options.token,
+ json: false,
+ });
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/serve.ts` around lines 197 - 200, serveDaemonCommand currently
ignores options.token and always calls runServeDaemon, skipping the HTTP server
bootstrap; update serveDaemonCommand to use options.token: if a token is
provided, invoke the HTTP server startup path (e.g., runServeHttp or
startHttpServer) with projectRoot, options.port, options.host and the token; if
no token is provided, continue to call runServeDaemon (or pass the token through
to runServeDaemon if that function should accept it). Modify the callsite in
serveDaemonCommand to pass the token into the appropriate server-start function
so the HTTP server path is executed when a token exists.
Comment on lines
+47
to
+53
| // Placeholder: the actual HTTP server will be implemented by issue #63. | ||
| // For now, keep the daemon alive so the lifecycle works end-to-end. | ||
| await logServe(projectRoot, 'Serve daemon ready (waiting for server implementation)'); | ||
|
|
||
| // Keep alive | ||
| await new Promise(() => {}); | ||
| } |
There was a problem hiding this comment.
Daemon never starts a server process.
This lifecycle currently becomes “alive but non-serving” and can mislead status/health checks.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/serve.ts` around lines 47 - 53, Replace the placeholder
infinite-wait in src/core/serve.ts with a real lightweight server so the daemon
is actually "serving": create and start a minimal HTTP server (or spawn the real
server process) instead of await new Promise(() => {}), bind it to a
configurable port, implement a /health (and optionally /ready) endpoint that
returns OK, call logServe(projectRoot, 'Serve daemon ready...') after the server
is listening, and wire up graceful shutdown (close the server on SIGINT/SIGTERM)
so lifecycle and health checks behave correctly; reference the existing logServe
call when placing the readiness log and replace the placeholder keep-alive with
the server instance lifecycle.
Comment on lines
+335
to
+347
| export function buildPairingUrl(params: { | ||
| host: string; | ||
| port: number; | ||
| caFingerprint: string; | ||
| token: string; | ||
| }): string { | ||
| const q = new URLSearchParams({ | ||
| host: params.host, | ||
| port: String(params.port), | ||
| ca: params.caFingerprint, | ||
| token: params.token, | ||
| }); | ||
| return `ppg://connect?${q.toString()}`; |
There was a problem hiding this comment.
ca query value is incompatible with the iOS QR parser contract.
Line 344 sends a fingerprint in ca, but ServerConnection.fromQRCode (ios/PPGMobile/PPGMobile/Models/ServerConnection.swift, Line 124-126) treats ca as base64-encoded certificate data. That means scanned connections lose CA material, which can break TLS pairing.
🔧 Contract-aligned fix sketch
export function buildPairingUrl(params: {
host: string;
port: number;
- caFingerprint: string;
+ caCertPem: string;
token: string;
}): string {
const q = new URLSearchParams({
host: params.host,
port: String(params.port),
- ca: params.caFingerprint,
+ ca: Buffer.from(params.caCertPem, 'utf8').toString('base64'),
token: params.token,
});
return `ppg://connect?${q.toString()}`;
}📝 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.
Suggested change
| export function buildPairingUrl(params: { | |
| host: string; | |
| port: number; | |
| caFingerprint: string; | |
| token: string; | |
| }): string { | |
| const q = new URLSearchParams({ | |
| host: params.host, | |
| port: String(params.port), | |
| ca: params.caFingerprint, | |
| token: params.token, | |
| }); | |
| return `ppg://connect?${q.toString()}`; | |
| export function buildPairingUrl(params: { | |
| host: string; | |
| port: number; | |
| caCertPem: string; | |
| token: string; | |
| }): string { | |
| const q = new URLSearchParams({ | |
| host: params.host, | |
| port: String(params.port), | |
| ca: Buffer.from(params.caCertPem, 'utf8').toString('base64'), | |
| token: params.token, | |
| }); | |
| return `ppg://connect?${q.toString()}`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/tls.ts` around lines 335 - 347, The pairing URL currently puts a
CA fingerprint into the ca query param, but ServerConnection.fromQRCode expects
ca to be base64-encoded certificate data; update buildPairingUrl to put the
certificate bytes (base64-encoded) into ca instead of the fingerprint: change
the params for buildPairingUrl (and all its callers) to accept the CA
certificate (PEM/DER) and encode it (e.g.,
Buffer.from(caCert).toString('base64')) when building URLSearchParams so the ca
query contains the base64 cert that ServerConnection.fromQRCode can decode.
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
1 participant
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.
Summary
Integrates 30 issue branches across 7 epics into a single integration branch. All conflicts resolved, typecheck passes, 632 tests pass.
core/operations/with pure-function signatures for reuse by both CLI and APIppg servewith daemon subcommands (start/stop/status/_daemon), bearer token auth, TLS cert generation, and structured error handler/api/Stats
npm run typecheckcleanTest plan
npm run typecheckpassesnpm test— 632/632 tests passppg serve start,ppg serve status,ppg serve stop/health,/api/status,/api/agents)xcodegenand builds in XcodeSummary by CodeRabbit
Release Notes
New Features
Dependencies