P4 support: bugs, incomplete migrations, and Pi/OpenCode parity #475

Context

PR #472 adds Perforce (P4) pending changelist diff support via a VcsProvider abstraction layer. Two independent code reviews identified bugs in the new P4 code paths and incomplete migrations across entry points.

Bugs in P4 implementation (must fix before merge)

Critical

• Missing git binary crashes VCS detection (packages/server/vcs.ts:80-86)
  Bun.spawn(["git", ...]) throws ENOENT in P4-only environments (no git installed). gitProvider.detect() needs a try/catch so detectVcs() can fall through to p4Provider. This blocks the primary use case — Perforce-only shops are exactly who this feature serves.

High

• Hardcoded p4-default when default CL is empty (apps/hook/server/index.ts:237)
  initialDiffType is always set to "p4-default" for P4 workspaces, but getP4Context() omits it from diffOptions when no files are in the default changelist. Users with only numbered CLs land on an empty diff. Should pick gitContext.diffOptions[0]?.id or similar.

• Double diff --git header when ==== separator present (packages/server/p4.ts:203-226)
  P4 versions that emit ==== ...#N - /path ==== separators get one header from the separator match (line 207) and a second from the --- line match (line 224). parseDiffToFiles() splits on ^diff --git, producing phantom empty file entries.

• Deleted file paths become /dev/null (packages/server/p4.ts:216-218)
  For deletes, P4 outputs +++ /dev/null. The code trusts the +++ line to derive relativePath, producing diff --git a//dev/null b//dev/null. Deleted files are invisible or bogus in the review UI.

• batchResolveDepotPaths assumes 1:1 line correspondence (packages/server/p4.ts:~280-296)
  p4 where can emit error lines for unmapped paths, shifting the index-based mapping. Also contains dead rootIdx variable.

Medium

• Trailing newline produces wrong line count (packages/server/p4.ts:250-257)
  content.split("\n") on files ending with \n produces an extra empty element. Hunk header claims one more line than exists. Affects virtually every p4 add file.

Incomplete migrations

• OpenCode plugin not migrated (apps/opencode-plugin/commands.ts:21,80-81)
  Still imports getGitContext/runGitDiffWithContext from @plannotator/server/git. OpenCode users in P4 workspaces get Git diffs instead of P4 diffs. Should use getVcsContext/runVcsDiff from @plannotator/server/vcs.

Pi extension parity (follow-up work)

Pi is a parallel Node.js implementation that vendors shared files via vendor.sh. It has no equivalent of vcs.ts or p4.ts. After PR #472:

• Bun server (Claude Code hook): Git + P4
• Pi server: Git only
• OpenCode plugin: Git only

Adding P4 to Pi requires:

1. New serverP4.ts using spawnSync instead of Bun.spawn
2. VCS dispatch logic in serverReview.ts
3. Vendoring any new shared P4 types via vendor.sh

This is not a regression (Pi never had P4) but should be tracked for feature parity.

References

• PR feat: add Perforce (P4) pending changelist diff support #472: feat: add Perforce (P4) pending changelist diff support #472
• CLAUDE.md dual-server rule: "When adding or modifying server endpoints, both implementations must be updated"