feat(rust): fenced whole-session deletion - #17
Conversation
… counterpart `agent-session-core::deletion` removes a session's own log files at a host's explicit request, and nothing else: targets must resolve strictly below one of the provider's roots under the given home, a symlinked target is refused, the file is re-parsed for its session id right before removal, and only Codex and Claude sessions are candidates. Claude's sidecar directory of the same stem goes with the file; directories are walked without following links. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: b61c7411fd
ℹ️ 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".
…t components Roots are walked component by component with a symlinked component refused, then canonicalized. A Codex file must carry session metadata in its head — the filename's uuid no longer stands in for it — and a name that disagrees with the metadata is refused. A Claude file must parse before a uuid stem is accepted, and a recorded sessionId must agree with it. Grok and Gemini, deletable in the Swift lane, are refused here as not implemented rather than planned for and then failed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80a4dd771a
ℹ️ 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".
… metadata as discovery reads it Containment now walks every component between the matched root and the target, refusing a symlinked directory inside the root. Codex revalidation accepts only a session_meta record's id or thread_id, from the head or the bounded tail — the records discovery itself accepts — and never an id on some other kind of record. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9be8d4425
ℹ️ 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".
…cars before the session file The component that first names a provider root must be the root itself, not a symlink to it. Removal now takes sidecars first and the session file last, so a failure part-way leaves the file that names the session in place for an honest outcome and a retry. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d50c375d87
ℹ️ 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".
…ng anything A sidecar is a flat directory of agent logs: its entries are unlinked one by one from the directory listing's own file type — a symlink is removed as a link — and a nested directory is refused rather than walked, so nothing is reopened by pathname after being checked. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6ea3477ab
ℹ️ 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".
| let Ok(meta) = std::fs::symlink_metadata(path) else { | ||
| continue; | ||
| }; | ||
| let result = if meta.file_type().is_dir() { | ||
| remove_sidecar_without_following_links(path) |
There was a problem hiding this comment.
When another same-user process replaces the top-level sidecar directory with a symlink after symlink_metadata succeeds, this call reopens that pathname with read_dir, follows the replacement, and removes entries from the symlink's external target. Fresh evidence beyond the previously fixed child-directory race is that the sidecar directory itself is still checked at line 161 and reopened here; use descriptor-relative, no-follow traversal for the directory and its entries.
AGENTS.md reference: AGENTS.md:L108-L110
Useful? React with 👍 / 👎.
| let result = if meta.file_type().is_dir() { | ||
| remove_sidecar_without_following_links(path) | ||
| } else { | ||
| std::fs::remove_file(path) |
There was a problem hiding this comment.
If another process atomically replaces the validation file with a different regular file after reparse_session_id returns, this pathname-based unlink deletes the replacement without checking its ID. The operation can therefore remove a different session despite the stale-plan safeguard; preserve and verify the validated file identity through the removal operation rather than reopening its name after validation.
AGENTS.md reference: AGENTS.md:L151-L157
Useful? React with 👍 / 👎.
1 participant
The Rust counterpart of the Swift
SessionDeleter, so the Desktop client can offer session deletion under the same rules the native app follows.deletion::{SessionToDelete, DeletionPlan, DeleteError, DeleteOutcome, provider_roots, plan, delete}.discovery::{codex_session_id, claude_session_id}, built from discovery's own rules), onlysupports_deletionproviders.cargo test,cargo clippy -D warnings,cargo fmt --checkgreen inimplementations/rust.CHANGELOG.mdUnreleased entry; the version constant is untouched per RELEASING.md.🤖 Generated with Claude Code