Skip to content

fix(snapshot): fix revert not restoring files due to absolute path issue#8631

Closed
Twisted928 wants to merge 5 commits intoanomalyco:devfrom
Twisted928:fix/snapshot-revert-absolute-path
Closed

fix(snapshot): fix revert not restoring files due to absolute path issue#8631
Twisted928 wants to merge 5 commits intoanomalyco:devfrom
Twisted928:fix/snapshot-revert-absolute-path

Conversation

@Twisted928
Copy link
Copy Markdown

@Twisted928 Twisted928 commented Jan 15, 2026

Fixes #8098

Summary
Fix TUI Revert feature not actually reverting file changes in workspace.
Root Cause
Snapshot.patch() returns absolute paths in the files array, but git checkout expects paths relative to the worktree. This caused checkout to fail silently, leaving files unchanged while the session state showed them as "reverted".
Changes
Core Fix (packages/opencode/src/snapshot/index.ts)

  • Convert absolute paths to relative using path.relative(Instance.worktree, file)
  • Use normalized gitPath for all git commands
    Safety Guards
  • Worktree escape protection: Skip files outside worktree using Filesystem.contains() + path.isAbsolute() check
  • Empty path protection: Skip when relativePath === ""
  • Windows compatibility: Normalize path separators (\/) for git pathspec
  • Directory creation: Add fs.mkdir() before fs.writeFile() in git show fallback
    Tests (packages/opencode/test/snapshot/snapshot.test.ts)
    Added 13 new test cases:
  • patch returns absolute paths and revert handles them correctly
  • revert restores modified file content correctly
  • revert handles deeply nested files with absolute paths
  • revert handles files with spaces in names
  • revert handles multiple sequential patches correctly
  • revert with patches array containing multiple items
  • revert deletes file created after snapshot
  • revert restores deleted file
  • patch files relative path computation is correct
  • revert works when process cwd differs from worktree
  • revert skips files outside worktree
  • revert skips patch with empty relativePath
  • revert creates parent directory when using fallback
    Testing
  • All 52 snapshot tests pass (39 existing + 13 new)
  • Typecheck passes

Root cause: Snapshot.patch() returns absolute paths in files array, but
git checkout expects paths relative to worktree, causing checkout to fail
silently and files not being restored.

Changes:
- Convert absolute paths to relative using path.relative()
- Add safety guards: worktree escape check, empty path check
- Normalize path separators for Windows compatibility
- Add fs.mkdir() before fs.writeFile() in git show fallback
- Add 13 test cases covering the fix and edge cases

Fixes TUI Revert not actually reverting file changes in workspace.
@github-actions
Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions
Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

No duplicate PRs found

@yimingll
Copy link
Copy Markdown

I tested this fix locally and can confirm it works correctly. The revert feature now properly restores files as expected. Would appreciate seeing this merged soon. 👍

yimingll pushed a commit to yimingll/opencode that referenced this pull request Jan 19, 2026
Based on PR anomalyco#8631 by @Twisted928, this adds 8 additional test scenarios:
- Unicode/Chinese character filenames (skipped - reveals bug)
- Emoji filenames (skipped - reveals bug)
- Symlink handling
- Large file handling (1MB)
- Binary file handling
- Concurrent operations
- Empty file handling
- Restore to empty state

Co-authored-by: Twisted <46923858+Twisted928@users.noreply.github.com>
@Twisted928
Copy link
Copy Markdown
Author

@rekram1-node review pr plz

yofriadi pushed a commit to yofriadi/opencode that referenced this pull request Jan 27, 2026
Root cause: Snapshot.patch() returns absolute paths in files array, but
git checkout expects paths relative to worktree, causing checkout to fail
silently and files not being restored.

Changes:
- Convert absolute paths to relative using path.relative()
- Add safety guards: worktree escape check, empty path check
- Normalize path separators for Windows compatibility
- Add fs.mkdir() before fs.writeFile() in git show fallback
- Add 13 test cases covering the fix and edge cases

Fixes TUI Revert not actually reverting file changes in workspace.
Based on PR anomalyco#8631 by Twisted928
yofriadi pushed a commit to yofriadi/opencode that referenced this pull request Jan 27, 2026
Root cause: Snapshot.patch() returns absolute paths in files array, but
git checkout expects paths relative to worktree, causing checkout to fail
silently and files not being restored.

Changes:
- Convert absolute paths to relative using path.relative()
- Add safety guards: worktree escape check, empty path check
- Normalize path separators for Windows compatibility
- Add fs.mkdir() before fs.writeFile() in git show fallback
- Add 13 test cases covering the fix and edge cases

Fixes TUI Revert not actually reverting file changes in workspace.
Based on PR anomalyco#8631 by Twisted928
@rekram1-node
Copy link
Copy Markdown
Collaborator

/review

@github-actions
Copy link
Copy Markdown
Contributor

lgtm

@rekram1-node
Copy link
Copy Markdown
Collaborator

I think this all makes sense, will review more tmr

yofriadi pushed a commit to yofriadi/opencode that referenced this pull request Jan 28, 2026
Root cause: Snapshot.patch() returns absolute paths in files array, but
git checkout expects paths relative to worktree, causing checkout to fail
silently and files not being restored.

Changes:
- Convert absolute paths to relative using path.relative()
- Add safety guards: worktree escape check, empty path check
- Normalize path separators for Windows compatibility
- Add fs.mkdir() before fs.writeFile() in git show fallback
- Add 13 test cases covering the fix and edge cases

Fixes TUI Revert not actually reverting file changes in workspace.
Based on PR anomalyco#8631 by Twisted928
@rekram1-node
Copy link
Copy Markdown
Collaborator

Looks like a test is failing now?

ENOENT: no such file or directory, open '/tmp/opencode-test-9o219frjhtu/文件.txt'
path: "/tmp/opencode-test-9o219frjhtu/文件.txt",
syscall: "open",
errno: -2,
code: "ENOENT"

(fail) unicode filenames modification and restore [68.14ms]

})
})

test("revert restores modified file content correctly", async () => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests don't really test anything it seems?

They still pass with the old code

yofriadi pushed a commit to yofriadi/opencode that referenced this pull request Jan 30, 2026
Root cause: Snapshot.patch() returns absolute paths in files array, but
git checkout expects paths relative to worktree, causing checkout to fail
silently and files not being restored.

Changes:
- Convert absolute paths to relative using path.relative()
- Add safety guards: worktree escape check, empty path check
- Normalize path separators for Windows compatibility
- Add fs.mkdir() before fs.writeFile() in git show fallback
- Add 13 test cases covering the fix and edge cases

Fixes TUI Revert not actually reverting file changes in workspace.
Based on PR anomalyco#8631 by Twisted928
@thdxr thdxr force-pushed the dev branch 2 times, most recently from f802701 to f1ae801 Compare January 30, 2026 14:36
yofriadi pushed a commit to yofriadi/opencode that referenced this pull request Feb 2, 2026
…ript

- Add list of upstream PRs (anomalyco#5547, anomalyco#8631, anomalyco#9658, anomalyco#10796) that are open but
  merged in this fork
- Update sync script to check if these PRs are merged in upstream before
  rebasing, preventing duplicate commits
yofriadi pushed a commit to yofriadi/opencode that referenced this pull request Feb 3, 2026
…ript

- Add list of upstream PRs (anomalyco#5547, anomalyco#8631, anomalyco#9658, anomalyco#10796) that are open but
  merged in this fork
- Update sync script to check if these PRs are merged in upstream before
  rebasing, preventing duplicate commits
yofriadi pushed a commit to yofriadi/opencode that referenced this pull request Feb 5, 2026
…ript

- Add list of upstream PRs (anomalyco#5547, anomalyco#8631, anomalyco#9658, anomalyco#10796) that are open but
  merged in this fork
- Update sync script to check if these PRs are merged in upstream before
  rebasing, preventing duplicate commits
@github-actions
Copy link
Copy Markdown
Contributor

Closing this pull request because it has had no updates for more than 60 days. If you plan to continue working on it, feel free to reopen or open a new PR.

@github-actions github-actions bot closed this Mar 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

i input /undo just undo conversation,not file

3 participants