Skip to content

回滚tool#422

Merged
Nomikfk1215 merged 3 commits into1024XEngineer:mainfrom
voicepeak:tool-re
May 9, 2026
Merged

回滚tool#422
Nomikfk1215 merged 3 commits into1024XEngineer:mainfrom
voicepeak:tool-re

Conversation

@voicepeak
Copy link
Copy Markdown
Collaborator

变更内容

  • 新增 ByteMind 文件修改回退记录与快照存储
  • write_filereplace_in_fileapply_patch 接入回退操作记录
  • 新增 /rollback 命令,用于查看和执行回退
  • 补充回退、冲突检测、patch 失败自动恢复相关测试

@codecov
Copy link
Copy Markdown

codecov Bot commented May 9, 2026

Copy link
Copy Markdown

@fennoai fennoai Bot left a comment

Choose a reason for hiding this comment

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

Found 3 issues in the rollback implementation: one behavior regression, one rollback fidelity issue, and one writable-root validation gap.

if info.IsDir() {
return snapshotState{}, fmt.Errorf("rollback snapshot target is a directory: %s", path)
}
if info.Size() > maxSnapshotBytes {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

High: snapshot capture now hard-fails for any existing file larger than 5 MiB or any non-text file. Because Begin() propagates this error back through write_file, replace_in_file, and apply_patch, those tools can no longer modify files they previously could. That is a user-visible regression unrelated to whether rollback metadata is available; if a file is not snapshot-safe, the safer fallback is usually to skip rollback for that target rather than reject the edit entirely.

if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
return err
}
return os.WriteFile(path, data, 0o644)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium: rollback restores the previous bytes with a hard-coded 0644, so executable/read-only bits are lost after /rollback. readSnapshotCandidate() already captures the original mode, but FileChange never persists it and restoreSnapshot() does not use it. Rolling back a script or any non-0644 file will silently change its permissions.

return paths
}

func validateOperationPaths(op Operation, workspace string, writableRoots ...string) error {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

High: validateOperationPaths() only checks cleaned absolute paths and does not canonicalize symlinks the way normal tool access does in resolvePath(). If a recorded workspace path is replaced with a symlink before /rollback, this check still passes and the subsequent read/write/remove follows the link, which can target files outside the workspace or writable roots. Rollback needs the same canonical-path validation at execution time to preserve the trust boundary.

@Nomikfk1215 Nomikfk1215 merged commit 30bb1ff into 1024XEngineer:main May 9, 2026
6 checks passed
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.

2 participants