[OrcaWin]

	Files	Added	Deleted	Net
Test	14	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​19	$\color{#cf222e}{\Huge{\mathbf{−}}}$​2688	$\color{#cf222e}{\Huge{\mathbf{−}}}$​2669
Prod	0	0	0	0

Remove 116 low-value authored tests across 14 files, including the entire 35-test GitHub RPC forwarding suite. These tests mainly repeat mock wiring, implementation details, or cases already exercised more meaningfully in the surrounding suite.

Keep 338 selected tests. Trim two retained tests to focus on remote paste behavior and malformed-session diagnostics. No production code changes.

Validation: reviewed the complete patch and retained-test byte/span evidence; external formatting and lint checks passed for all 19 surviving selected files. Local project tests were not run; required PR CI must pass before merge. This audit does not claim replacement coverage or complete dependency review.

[coderabbitai]

Walkthrough

The pull request removes or revises tests across session handling, worktree creation, managed services, runtime IPC and RPC, and renderer behavior. The Git-status polling tests add coverage for refresh coalescing and related polling conditions. No implementation changes are described.

Merge Risk: Low · up to 2cc87

Restoring the wrapper test would protect against future accidental deletion outside the overlay directory. The current cleanup guard remains in place.

Pre-merge checks: 4 · 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Description check		The description is mostly complete. It gives a clear summary of the removed tests, the reason for removal, the retained test count, and the validation steps. It has no ELI5 section, no What Changed se…	Add the template sections that are missing or unfilled. (1) Add an ELI5 section in plain language. (2) Add a What Changed section that describes the before and after state. Keep the explanation of the removed tests. (3) Add a Why section th…

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title "test: remove 116 low-value tests" matches the main change. The diff only deletes test code across 14 test files, and the PR objectives state the same 116-test removal. The title uses the co…
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title "test: remove 116 low-value tests" matches the main change. The diff only deletes test code across 14 test files, and the PR objectives state the same 116-test removal. The title uses the conventional commit prefix and gives the count, so a reviewer scanning history can see the primary change.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 4 files.

Full details: Description check

Explanation

The description is mostly complete. It gives a clear summary of the removed tests, the reason for removal, the retained test count, and the validation steps. It has no ELI5 section, no What Changed section in the template format, and no Linked Issue. The Visual Proof, Testing checklist, and Review sections are also missing or unfilled. The Testing section does not state which platforms were tested. Since this is a test-only removal, the missing UI proof may be N/A, but the author should state that explicitly.

Resolution

Add the template sections that are missing or unfilled. (1) Add an ELI5 section in plain language. (2) Add a What Changed section that describes the before and after state. Keep the explanation of the removed tests. (3) Add a Why section that explains why deletion is preferred over keeping or rewriting the tests. (4) Add a Linked Issue section. Link an issue, or state that the change is maintainer-authored and no issue applies. (5) Write N/A with a reason in the Visual Proof section. (6) Complete the Testing section. State that local tests were not run and list the platforms, if any, that were checked. Mark the checklist items that apply. (7) Complete the AI Disclosure section or state that it does not apply. (8) Complete the Agent skill upstream boundary checkbox.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[coderabbitai]

Nitpick comments: 1

src/main/pi/titlebar-extension-service.test.ts: 1

720-720: Trivial · Security & Privacy · Quick win

Restore the service-wrapper guard test.

The retained tests cover the shared guard directly and successful clearPty cleanup, but neither detects a regression where PiTitlebarExtensionService.safeRemoveOverlay bypasses the guard. Restore the deleted test to ensure out-of-root targets cannot remove piHome.

Suggested fix

+  it('refuses to remove anything outside the overlay root', () => {
+    const svc = new PiTitlebarExtensionService() as unknown as {
+      safeRemoveOverlay: (p: string, kind: 'pi' | 'omp') => void
+    }
+    svc.safeRemoveOverlay(piHome, 'pi')
+    svc.safeRemoveOverlay(piHome, 'omp')
+    svc.safeRemoveOverlay('/', 'pi')
+    svc.safeRemoveOverlay(join(userDataDir, 'pi-agent-overlays'), 'pi')
+    svc.safeRemoveOverlay(join(userDataDir, 'omp-agent-overlays'), 'omp')
+    expectPiHomeIntact()
+  })

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 777572ca-8788-4845-959a-e01df9d88f99

Commits

Reviewing files that changed from the base of the PR and between af6dabd and 2cc87c4.

Files selected for processing: 14

• src/main/codex/codex-session-index-heal.test.ts
• src/main/daemon/daemon-pty-adapter-history-checkpoints.test.ts
• src/main/ipc/register-core-handlers/register-core-handlers.test.ts
• src/main/ipc/worktrees-local-create-flow.test.ts
• src/main/managed-server/install/remote-versioned-install.test.ts
• src/main/managed-server/lifecycle/orcad-managed-tunnel.test.ts
• src/main/pi/titlebar-extension-service.test.ts
• src/main/pty-descendant-termination.test.ts
• src/main/runtime/orca-runtime-tests/worktree-setup-and-startup-part-03.spec.ts
• src/main/runtime/rpc/methods/github.test.ts
• src/main/runtime/rpc/methods/session-tabs.test.ts
• src/renderer/src/components/native-chat/use-native-chat-composer-paste.test.tsx
• src/renderer/src/components/right-sidebar/useGitStatusPolling.test.ts
• src/renderer/src/hooks/useAutomationDispatchEvents.test.ts

Files with no reviewable changes: 10

• src/main/runtime/rpc/methods/github.test.ts
• src/main/daemon/daemon-pty-adapter-history-checkpoints.test.ts
• src/renderer/src/hooks/useAutomationDispatchEvents.test.ts
• src/main/codex/codex-session-index-heal.test.ts
• src/main/managed-server/lifecycle/orcad-managed-tunnel.test.ts
• src/main/runtime/orca-runtime-tests/worktree-setup-and-startup-part-03.spec.ts
• src/main/ipc/worktrees-local-create-flow.test.ts
• src/main/pi/titlebar-extension-service.test.ts
• src/main/pty-descendant-termination.test.ts
• src/renderer/src/components/right-sidebar/useGitStatusPolling.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.