Simplify delete file|dir to just delete path#2181
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77fbe9288e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
dobrac
left a comment
dobrac
left a comment
There was a problem hiding this comment.
lgtm, just the broken symlink handling
levb
left a comment
levb
left a comment
There was a problem hiding this comment.
++ to prior comment on symlink/test; No other comments.
# Conflicts: # packages/orchestrator/internal/volumes/file_delete.go # packages/orchestrator/internal/volumes/file_update.go # packages/orchestrator/internal/volumes/stat.go # packages/orchestrator/internal/volumes/stat_test.go # packages/orchestrator/pkg/volumes/dir_delete.go # packages/orchestrator/pkg/volumes/dir_delete_test.go # packages/orchestrator/pkg/volumes/file_delete.go # packages/orchestrator/pkg/volumes/file_delete_test.go # packages/orchestrator/pkg/volumes/file_update.go # packages/orchestrator/pkg/volumes/file_update_test.go # packages/orchestrator/pkg/volumes/internal_test.go # packages/orchestrator/pkg/volumes/path_delete.go # packages/orchestrator/pkg/volumes/path_delete_test.go # packages/orchestrator/pkg/volumes/path_stat.go # packages/orchestrator/pkg/volumes/path_stat_test.go # packages/orchestrator/pkg/volumes/path_update.go # packages/orchestrator/pkg/volumes/stat.go # packages/orchestrator/pkg/volumes/stat_test.go # packages/orchestrator/pkg/volumes/utils_test.go
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
|
|
||
| target := "symlink-target.txt" | ||
| link := "symlink-to-delete" | ||
| target := "broken-symlink-target.txt" |
There was a problem hiding this comment.
LOL add parallel-tests-are-hard-437-uniq... :)
3 participants
Note
Medium Risk
Medium risk because it changes the gRPC surface (renamed RPCs/messages) and alters deletion semantics to recursive
DeletePath, which could remove directories if callers pass broader paths than intended.Overview
This PR updates the volumes gRPC API to use clearer, operation-scoped RPC names (e.g.,
CreateVolume,DeleteVolume,StatPath,UpdatePath) and replaces separate file/dir delete operations with a single recursiveDeletePathendpoint that validates existence and blocks deleting/; the API service is updated accordingly, and orchestrator tests are reshaped to cover the new request/response types andDeletePathbehavior (including directories and symlinks).Written by Cursor Bugbot for commit fec8cfa. This will update automatically on new commits. Configure here.