test: patch — maybeParseApplyPatchVerified coverage by anandgupta42 · Pull Request #396 · AltimateAI/altimate-code

anandgupta42 · 2026-03-23T07:19:13Z

What does this PR do?

1. maybeParseApplyPatchVerified — src/patch/index.ts (9 new tests)

This is the async verified entry point that production callers use to validate patch commands before any filesystem mutation happens. It had zero test coverage prior to this PR. The function handles:

Detecting implicit patch invocation (model sends raw patch without apply_patch command)
Resolving file paths relative to cwd
Reading existing files for delete/update verification
Returning structured CorrectnessError results for invalid inputs

Why it matters: If a regression breaks implicit invocation detection, the model could silently execute raw patch text as a shell command instead of routing it through the patch system. If path resolution breaks, patches target wrong files. If delete verification breaks, users get confusing errors instead of clean CorrectnessError responses.

Specific scenarios covered:

Implicit invocation detection (raw valid patch → CorrectnessError with ImplicitInvocation)
Single non-patch arg falls through to NotApplyPatch
Multi-arg non-patch commands return NotApplyPatch
Add hunk → Body with correct resolved path and content in changes map
Delete hunk → Body with file content read from disk
Delete of non-existent file → CorrectnessError with descriptive message
Update hunk → Body with unified_diff and new_content
Update with non-matching old_lines → CorrectnessError
Move directive → change keyed by resolved move_path
Invalid patch syntax → CorrectnessError

Type of change

New feature (non-breaking change which adds functionality)

Issue for this PR

N/A — proactive test coverage for untested production entry point

How did you verify your code works?

bun test test/patch/patch.test.ts       # 30 pass (22 existing + 8 new)

All 9 new maybeParseApplyPatchVerified tests pass. No existing tests were modified or broken.

Checklist

I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

https://claude.ai/code/session_01Y7WKPvTnLsBSqtDZPmUKrT

Summary by CodeRabbit

Tests
- Expanded test coverage for patch parsing and application verification, including validation of patch operation handling and error scenarios.

The async verified entry point had zero test coverage. New tests exercise implicit invocation detection, add/delete/update/move verification, non-existent file errors, and invalid patch syntax handling. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> https://claude.ai/code/session_01Y7WKPvTnLsBSqtDZPmUKrT

claude

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

_{Tip: disable this comment in your organization's Code Review settings.}

coderabbitai · 2026-03-23T07:19:28Z

📝 Walkthrough

Walkthrough

A new test suite for Patch.maybeParseApplyPatchVerified was added, verifying control-flow outcomes for patch parsing and application, including error cases for missing patches, invalid inputs, and file operation failures.

Changes

Cohort / File(s)	Summary
Patch Verification Tests `packages/opencode/test/patch/patch.test.ts`	Added test suite `maybeParseApplyPatchVerified` with tests for error returns (`CorrectnessError`, `NotApplyPatch`), successful patch parsing with validated `Body.action` structure, file operation error cases (delete non-existent, update with mismatched lines), and move directive path resolution.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

contributor

Poem

🐰 A patch test, so thorough and keen,
Verifies paths both old and clean,
Each error caught, each action bright,
Changes checked from left to right! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and specifically describes the main change: adding test coverage for maybeParseApplyPatchVerified, which aligns with the changeset.
Description check	✅ Passed	The description covers the template sections with detailed test scenarios, rationale, and verification steps, though the checklist items use checkboxes instead of explicit confirmation statements.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch claude/test-patch-verified/session_01Y7WKPvTnLsBSqtDZPmUKrT

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/opencode/test/patch/patch.test.ts`:
- Around line 264-423: The tests in this block use a manually-created shared
tempDir instead of the repo fixture pattern; update each test to obtain an
isolated temp directory via the tmpdir fixture using "await using tmp = await
tmpdir()" and pass tmp.path (realpath resolved) into
Patch.maybeParseApplyPatchVerified instead of the existing tempDir variable;
ensure any files are created under that tmp.path and any expected resolved paths
use path.resolve(tmp.path, ...). Target symbols:
Patch.maybeParseApplyPatchVerified, tmpdir fixture (from fixture/fixture.ts),
and any local tempDir usages in these tests.

🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 93fa502e-7df7-4607-94fb-10fb69af8a35

📥 Commits

Reviewing files that changed from the base of the PR and between 8eb13d4 and 47da404.

📒 Files selected for processing (1)

packages/opencode/test/patch/patch.test.ts

coderabbitai · 2026-03-23T07:22:54Z

packages/opencode/test/patch/patch.test.ts

+  describe("maybeParseApplyPatchVerified", () => {
+    test("detects implicit invocation (raw patch without apply_patch command)", async () => {
+      const patchText = `*** Begin Patch
+*** Add File: test.txt
+Content
+*** End Patch`
+
+      const result = await Patch.maybeParseApplyPatchVerified([patchText], tempDir)
+      expect(result.type).toBe(Patch.MaybeApplyPatchVerified.CorrectnessError)
+      if (result.type === Patch.MaybeApplyPatchVerified.CorrectnessError) {
+        expect(result.error.message).toBe(Patch.ApplyPatchError.ImplicitInvocation)
+      }
+    })
+
+    test("returns NotApplyPatch for single arg that is not a valid patch", async () => {
+      const result = await Patch.maybeParseApplyPatchVerified(["echo hello"], tempDir)
+      expect(result.type).toBe(Patch.MaybeApplyPatchVerified.NotApplyPatch)
+    })
+
+    test("returns NotApplyPatch for unrelated multi-arg commands", async () => {
+      const result = await Patch.maybeParseApplyPatchVerified(["ls", "-la", "/tmp"], tempDir)
+      expect(result.type).toBe(Patch.MaybeApplyPatchVerified.NotApplyPatch)
+    })
+
+    test("returns Body with add change for apply_patch add hunk", async () => {
+      const patchText = `*** Begin Patch
+*** Add File: new-file.txt
+line one
+line two
+*** End Patch`
+
+      const result = await Patch.maybeParseApplyPatchVerified(["apply_patch", patchText], tempDir)
+      expect(result.type).toBe(Patch.MaybeApplyPatchVerified.Body)
+      if (result.type === Patch.MaybeApplyPatchVerified.Body) {
+        const resolvedPath = path.resolve(tempDir, "new-file.txt")
+        const change = result.action.changes.get(resolvedPath)
+        expect(change).toBeDefined()
+        expect(change!.type).toBe("add")
+        if (change!.type === "add") {
+          expect(change!.content).toBe("line one\nline two")
+        }
+        expect(result.action.cwd).toBe(tempDir)
+      }
+    })
+
+    test("returns Body with delete change including file content", async () => {
+      const filePath = path.join(tempDir, "to-delete.txt")
+      await fs.writeFile(filePath, "original content here")
+
+      const patchText = `*** Begin Patch
+*** Delete File: to-delete.txt
+*** End Patch`
+
+      const result = await Patch.maybeParseApplyPatchVerified(["apply_patch", patchText], tempDir)
+      expect(result.type).toBe(Patch.MaybeApplyPatchVerified.Body)
+      if (result.type === Patch.MaybeApplyPatchVerified.Body) {
+        const resolvedPath = path.resolve(tempDir, "to-delete.txt")
+        const change = result.action.changes.get(resolvedPath)
+        expect(change).toBeDefined()
+        expect(change!.type).toBe("delete")
+        if (change!.type === "delete") {
+          expect(change!.content).toBe("original content here")
+        }
+      }
+    })
+
+    test("returns CorrectnessError when deleting non-existent file", async () => {
+      const patchText = `*** Begin Patch
+*** Delete File: no-such-file.txt
+*** End Patch`
+
+      const result = await Patch.maybeParseApplyPatchVerified(["apply_patch", patchText], tempDir)
+      expect(result.type).toBe(Patch.MaybeApplyPatchVerified.CorrectnessError)
+      if (result.type === Patch.MaybeApplyPatchVerified.CorrectnessError) {
+        expect(result.error.message).toContain("Failed to read file for deletion")
+      }
+    })
+
+    test("returns Body with update change including unified_diff and new_content", async () => {
+      const filePath = path.join(tempDir, "to-update.txt")
+      await fs.writeFile(filePath, "alpha\nbeta\ngamma\n")
+
+      const patchText = `*** Begin Patch
+*** Update File: to-update.txt
+@@
+ alpha
+-beta
+BETA
+ gamma
+*** End Patch`
+
+      const result = await Patch.maybeParseApplyPatchVerified(["apply_patch", patchText], tempDir)
+      expect(result.type).toBe(Patch.MaybeApplyPatchVerified.Body)
+      if (result.type === Patch.MaybeApplyPatchVerified.Body) {
+        const resolvedPath = path.resolve(tempDir, "to-update.txt")
+        const change = result.action.changes.get(resolvedPath)
+        expect(change).toBeDefined()
+        expect(change!.type).toBe("update")
+        if (change!.type === "update") {
+          expect(change!.new_content).toBe("alpha\nBETA\ngamma\n")
+          expect(change!.unified_diff).toContain("-beta")
+          expect(change!.unified_diff).toContain("+BETA")
+          expect(change!.move_path).toBeUndefined()
+        }
+      }
+    })
+
+    test("returns CorrectnessError when updating file with non-matching old_lines", async () => {
+      const filePath = path.join(tempDir, "mismatch.txt")
+      await fs.writeFile(filePath, "actual content\n")
+
+      const patchText = `*** Begin Patch
+*** Update File: mismatch.txt
+@@
+-completely different content
+replacement
+*** End Patch`
+
+      const result = await Patch.maybeParseApplyPatchVerified(["apply_patch", patchText], tempDir)
+      expect(result.type).toBe(Patch.MaybeApplyPatchVerified.CorrectnessError)
+      if (result.type === Patch.MaybeApplyPatchVerified.CorrectnessError) {
+        expect(result.error.message).toContain("Failed to find expected lines")
+      }
+    })
+
+    test("resolves move_path for update with move directive", async () => {
+      const filePath = path.join(tempDir, "old-name.txt")
+      await fs.writeFile(filePath, "keep this\n")
+
+      const patchText = `*** Begin Patch
+*** Update File: old-name.txt
+*** Move to: new-name.txt
+@@
+-keep this
+keep that
+*** End Patch`
+
+      const result = await Patch.maybeParseApplyPatchVerified(["apply_patch", patchText], tempDir)
+      expect(result.type).toBe(Patch.MaybeApplyPatchVerified.Body)
+      if (result.type === Patch.MaybeApplyPatchVerified.Body) {
+        // The change is keyed by the resolved move_path, not the original path
+        const resolvedMovePath = path.resolve(tempDir, "new-name.txt")
+        const change = result.action.changes.get(resolvedMovePath)
+        expect(change).toBeDefined()
+        expect(change!.type).toBe("update")
+        if (change!.type === "update") {
+          expect(change!.move_path).toBe(resolvedMovePath)
+          expect(change!.new_content).toBe("keep that\n")
+        }
+      }
+    })
+
+    test("returns CorrectnessError for invalid patch syntax", async () => {
+      const result = await Patch.maybeParseApplyPatchVerified(
+        ["apply_patch", "this is not a valid patch at all"],
+        tempDir,
+      )
+      expect(result.type).toBe(Patch.MaybeApplyPatchVerified.CorrectnessError)
+    })
+  })


⚠️ Potential issue | 🟡 Minor

Use the repo tmp fixture pattern in these new tests.

The new tests keep depending on a manually-created shared temp directory. In this test path, the project standard is await using tmp = await tmpdir(...) and reading the realpath-resolved directory from tmp.path. Please migrate these new cases to that pattern.

Suggested pattern for each new test

test("...", async () => { await using tmp = await tmpdir() const tempDir = tmp.path const result = await Patch.maybeParseApplyPatchVerified([...], tempDir) // assertions... })

As per coding guidelines: “Use the tmpdir function from fixture/fixture.ts… Always use await using syntax… Access the temporary directory path via the path property and ensure paths are realpath resolved”.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@packages/opencode/test/patch/patch.test.ts` around lines 264 - 423, The tests in this block use a manually-created shared tempDir instead of the repo fixture pattern; update each test to obtain an isolated temp directory via the tmpdir fixture using "await using tmp = await tmpdir()" and pass tmp.path (realpath resolved) into Patch.maybeParseApplyPatchVerified instead of the existing tempDir variable; ensure any files are created under that tmp.path and any expected resolved paths use path.resolve(tmp.path, ...). Target symbols: Patch.maybeParseApplyPatchVerified, tmpdir fixture (from fixture/fixture.ts), and any local tempDir usages in these tests.

Merges 9 test PRs (#390, #394, #395, #396, #397, #398, #399, #401, #387) into a single PR, discarding 4 duplicates (#391, #392, #393, #400). **New test coverage:** - `id.test.ts` — 14 tests: Identifier generation, monotonic ordering, timestamp extraction, Zod schema - `shell.test.ts` — 9 tests: shell blacklist enforcement for fish/nu, cache reset, fallback - `path-traversal.test.ts` — 7 tests (new): `Protected.isSensitiveWrite` for .pem/.key, .gnupg, .env variants - `ignore.test.ts` — 7 tests (new): `FileIgnore.match` directory patterns, globs, whitelist overrides - `paths-parsetext.test.ts` — 16 tests: `ConfigPaths.parseText` env/file substitution, JSONC parsing - `finops-formatting.test.ts` — 14 tests: `formatBytes`/`truncateQuery` edge cases - `finops-role-access.test.ts` — 10 tests: RBAC `formatGrants`/`formatHierarchy`/`formatUserRoles` - `tool-lookup.test.ts` — 4 tests: Zod schema introspection for tool parameters - `summary-git-path.test.ts` — 10 tests: `unquoteGitPath` decoding for non-ASCII filenames - `tracing.test.ts` — 2 tests (added): Recap loop detection boundaries - `patch.test.ts` — 9 tests (added): `maybeParseApplyPatchVerified` entry point coverage - `instruction.test.ts` — 13 tests (new): `InstructionPrompt.loaded()` dedup guards - `message-v2.test.ts` — 23 tests (new): `MessageV2.filterCompacted()` boundary detection **Bug fixes (discovered during testing):** - `finops-formatting.ts`: fix `formatBytes` crash on negative/NaN/fractional inputs - `finops-formatting.ts`: fix `truncateQuery` returning empty for whitespace-only, exceeding `maxLen` when < 4 - `training-import.test.ts`: fix pre-existing typecheck errors (missing `context`/`rule` mock keys) - `instruction.test.ts`, `message-v2.test.ts`: fix `MessageV2.Part` cast and branded ID type errors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

anandgupta42 · 2026-03-23T17:11:35Z

Consolidated into #403

* test: consolidated test coverage across 9 modules (133 new tests) Merges 9 test PRs (#390, #394, #395, #396, #397, #398, #399, #401, #387) into a single PR, discarding 4 duplicates (#391, #392, #393, #400). **New test coverage:** - `id.test.ts` — 14 tests: Identifier generation, monotonic ordering, timestamp extraction, Zod schema - `shell.test.ts` — 9 tests: shell blacklist enforcement for fish/nu, cache reset, fallback - `path-traversal.test.ts` — 7 tests (new): `Protected.isSensitiveWrite` for .pem/.key, .gnupg, .env variants - `ignore.test.ts` — 7 tests (new): `FileIgnore.match` directory patterns, globs, whitelist overrides - `paths-parsetext.test.ts` — 16 tests: `ConfigPaths.parseText` env/file substitution, JSONC parsing - `finops-formatting.test.ts` — 14 tests: `formatBytes`/`truncateQuery` edge cases - `finops-role-access.test.ts` — 10 tests: RBAC `formatGrants`/`formatHierarchy`/`formatUserRoles` - `tool-lookup.test.ts` — 4 tests: Zod schema introspection for tool parameters - `summary-git-path.test.ts` — 10 tests: `unquoteGitPath` decoding for non-ASCII filenames - `tracing.test.ts` — 2 tests (added): Recap loop detection boundaries - `patch.test.ts` — 9 tests (added): `maybeParseApplyPatchVerified` entry point coverage - `instruction.test.ts` — 13 tests (new): `InstructionPrompt.loaded()` dedup guards - `message-v2.test.ts` — 23 tests (new): `MessageV2.filterCompacted()` boundary detection **Bug fixes (discovered during testing):** - `finops-formatting.ts`: fix `formatBytes` crash on negative/NaN/fractional inputs - `finops-formatting.ts`: fix `truncateQuery` returning empty for whitespace-only, exceeding `maxLen` when < 4 - `training-import.test.ts`: fix pre-existing typecheck errors (missing `context`/`rule` mock keys) - `instruction.test.ts`, `message-v2.test.ts`: fix `MessageV2.Part` cast and branded ID type errors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address code review findings — test artifact leakage and cleanup - summary-git-path.test.ts: use tmpdir() instead of projectRoot to prevent Storage.write() from polluting the developer's .opencode/storage/ directory - finops-role-access.test.ts: consolidate duplicate afterAll hooks to ensure ALTIMATE_TELEMETRY_DISABLED env var is cleaned up after tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

claude bot reviewed Mar 23, 2026

View reviewed changes

github-actions bot added the contributor label Mar 23, 2026

coderabbitai bot reviewed Mar 23, 2026

View reviewed changes

anandgupta42 mentioned this pull request Mar 23, 2026

test: consolidated test coverage across 9 modules (133 new tests) #403

Merged

6 tasks

anandgupta42 closed this Mar 23, 2026

anandgupta42 deleted the claude/test-patch-verified/session_01Y7WKPvTnLsBSqtDZPmUKrT branch March 23, 2026 17:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: patch — maybeParseApplyPatchVerified coverage#396

test: patch — maybeParseApplyPatchVerified coverage#396
anandgupta42 wants to merge 1 commit intomainfrom
claude/test-patch-verified/session_01Y7WKPvTnLsBSqtDZPmUKrT

anandgupta42 commented Mar 23, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

claude bot left a comment

Uh oh!

coderabbitai bot commented Mar 23, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Suggested labels

Poem

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Mar 23, 2026

Uh oh!

anandgupta42 commented Mar 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

anandgupta42 commented Mar 23, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Type of change

Issue for this PR

How did you verify your code works?

Checklist

Summary by CodeRabbit

Uh oh!

claude bot left a comment

Choose a reason for hiding this comment

Claude Code Review

Uh oh!

coderabbitai bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested labels

Poem

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

anandgupta42 commented Mar 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

anandgupta42 commented Mar 23, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Mar 23, 2026 •

edited

Loading