feat(vcs): add status raw diff and guarded apply - #956
Merged
Conversation
github-actions
Bot
added
the
harness
Astro-Han
added
enhancement
Contributor
📝 WalkthroughWalkthroughAdds stdin-based git patch piping and binary patch support; exposes new git patch APIs; implements Vcs.status, Vcs.diffRaw (10MB budget, truncation errors), and Vcs.apply with Zod schemas and errors; wires HTTP routes for status/diff/raw/apply; and adds unit and integration tests covering success and failure cases. ChangesVCS Patch Operations
Sequence DiagramsequenceDiagram
participant Client
participant VcsRoute as /vcs/apply Route
participant VcsService as Vcs.apply
participant GitService as Git.applyPatch
participant ChildProcess
Client->>VcsRoute: POST { patch: "..." }
VcsRoute->>VcsService: apply({ patch })
VcsService->>GitService: applyPatch(cwd, patch)
GitService->>ChildProcess: run "git apply -" with stdin=patch
ChildProcess-->>GitService: exit code
alt exit code 0
GitService-->>VcsService: { ok: true }
VcsService-->>VcsRoute: { applied: true }
VcsRoute-->>Client: 200 { applied: true }
else exit code non-zero
GitService-->>VcsService: error
VcsService-->>VcsRoute: PatchApplyError
VcsRoute-->>Client: 400 { error: "vcs_apply_failed", reason: "..." }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces new Version Control System (VCS) capabilities, including endpoints and SDK support for retrieving working tree status summaries (/vcs/status), fetching raw patch diffs (/vcs/diff/raw), and applying git patches (/vcs/apply). Corresponding unit and integration tests have been added. A critical issue was identified in the test suite where the assertion for untracked file diff headers is incorrect and will cause test failures.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/opencode/test/server/vcs-routes.test.ts (1)
31-34: 💤 Low valueConsider order-insensitive assertion for status array.
The
toEqualassertion is order-sensitive. IfVcs.status()returns files in a different order (e.g., due to filesystem iteration or parallel processing), this test could become flaky.♻️ Suggested order-insensitive assertion
- expect(await response.json()).toEqual([ - { file: "tracked.txt", additions: 1, deletions: 1, status: "modified" }, - { file: "untracked.txt", additions: 1, deletions: 0, status: "added" }, - ]) + expect(await response.json()).toEqual( + expect.arrayContaining([ + { file: "tracked.txt", additions: 1, deletions: 1, status: "modified" }, + { file: "untracked.txt", additions: 1, deletions: 0, status: "added" }, + ]), + )🤖 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 `@packages/opencode/test/server/vcs-routes.test.ts` around lines 31 - 34, The test currently uses an order-sensitive toEqual on the response JSON; change it to an order-insensitive assertion by either sorting the returned array by the `file` property before comparing or using Jest's arrayContaining/matchers to assert the expected items exist regardless of order (locate the assertion in packages/opencode/test/server/vcs-routes.test.ts where response.json() is compared to the expected array for files "tracked.txt" and "untracked.txt"); ensure you still validate each object's additions, deletions, and status fields exactly.
🤖 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 `@packages/opencode/test/project/vcs.test.ts`:
- Around line 148-155: Tests using withVcsOnly currently call await
withVcsOnly(...) inside plain test blocks and use Instance.provide; refactor to
use the Effect test harness by creating const it = testEffect(describe) and
convert those tests to it.live(...) so they run in the Effect runtime, and
update withVcsOnly to accept and use provideTmpdirInstance(tmp.path) /
provideInstance(...) instead of Instance.provide; locate usages of withVcsOnly
in packages/opencode/test/project/vcs.test.ts and replace the await
withVcsOnly(...) pattern around calls like Vcs.status() with an it.live test
that provides the tmpdir and other fixtures via the Effect providers.
---
Nitpick comments:
In `@packages/opencode/test/server/vcs-routes.test.ts`:
- Around line 31-34: The test currently uses an order-sensitive toEqual on the
response JSON; change it to an order-insensitive assertion by either sorting the
returned array by the `file` property before comparing or using Jest's
arrayContaining/matchers to assert the expected items exist regardless of order
(locate the assertion in packages/opencode/test/server/vcs-routes.test.ts where
response.json() is compared to the expected array for files "tracked.txt" and
"untracked.txt"); ensure you still validate each object's additions, deletions,
and status fields exactly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3834a20d-190a-4c44-8c38-eb5fd445586c
⛔ Files ignored due to path filters (2)
packages/sdk/js/src/v2/gen/sdk.gen.tsis excluded by!**/gen/**packages/sdk/js/src/v2/gen/types.gen.tsis excluded by!**/gen/**
📒 Files selected for processing (5)
packages/opencode/src/git/index.tspackages/opencode/src/project/vcs.tspackages/opencode/src/server/instance/index.tspackages/opencode/test/project/vcs.test.tspackages/opencode/test/server/vcs-routes.test.ts
github-actions
Bot
added
the
platform
# Conflicts: # packages/desktop-electron/scripts/repair-electron-install.mjs # packages/desktop-electron/scripts/repair-electron-install.test.ts
github-actions
Bot
removed
the
platform
Perf delta summaryComparator: pass
|
# Conflicts: # packages/app/src/pages/session/use-session-followups.test.ts
1 task
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
Add VCS parity endpoints and SDK surface for the next #936 migration slice:
GET /vcs/statusreturns working-tree file summaries.GET /vcs/diff/rawreturns raw patch text for tracked, staged, unstaged, untracked, and binary changes, with a bounded output contract.POST /vcs/applyapplies a patch through git and returns structured failure reasons for git failures, oversized input, and invalid request bodies./vcs/diff?mode=unstaged|staged|branchcontract remains unchanged.Why
#936 identified VCS parity as the next low-risk backend slice before any broader Hono-to-Effect HttpApi migration. This adds the useful upstream VCS API shape while keeping PawWork's current Hono server path and existing diff modes intact.
Related Issue
Closes part of #936.
Human Review Status
Pending
Review Focus
Please review:
POST /vcs/applybehavior and error mapping, especially invalid input, oversized input, non-git projects, non-clean patches, and subdirectory requests.GET /vcs/diff/rawround-trip behavior for initial repositories, staged-then-modified files, binary files, subdirectory requests, and oversized patches./vcs/diffmode contract remains preserved./vcs/applypreserves the existing route behavior while moving stream/request reconstruction out ofInstanceRoutes.Risk Notes
POST /vcs/applyintentionally mutates the current git worktree by passing patch text togit apply -; tests cover success, non-git rejection, non-clean apply failure, subdirectory requests, invalid JSON/schema input, and oversized input.packages/sdk/openapi.jsonwas intentionally left unchanged because it is a route-inventory baseline, not the SDK build source.How To Verify
Screenshots or Recordings
Not applicable; no visible UI changes.
Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.