Repository navigation
Complete Rust-native memory lifecycle - #17
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (4)
📝 WalkthroughNote: bindings/python removal ranges were not present in the provided all_range_ids list (they appeared only in raw_summary text descriptions without rangeIds), so range_bindings_placeholder above is a placeholder that should be disregarded; no actual rangeId was omitted from assignment. WalkthroughThis PR removes the Python/PyO3 bindings crate, bumps the workspace version to 0.11.0, and introduces Rust-native DOX and Revolve source adapters plus agent-framework integration scanning in the core and CLI crates. It wires these into new CLI subcommands, TUI actions/rendering, adds CI and release-packaging workflows, an archive-based installer path, and updates documentation accordingly. ChangesCore adapters and CLI/TUI integration
CI, release, and installer tooling
Python bindings removal and documentation
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as tree-ring CLI
participant Adapter as dox/revolve adapter
participant Guard as SensitivityGuard
participant Store as SQLite store
User->>CLI: tree-ring dox sync --source-root <path>
CLI->>Adapter: collect_dox_memories(request)
Adapter->>Adapter: discover AGENTS.md files, parse sections
Adapter->>Guard: check sensitivity of each event
Guard-->>Adapter: allow / skip secret
Adapter-->>CLI: DoxSyncReport(events, warnings)
alt dry_run
CLI-->>User: print report (no persistence)
else persist
CLI->>Store: put_many(events)
Store-->>CLI: ack
CLI-->>User: print report with counts
end
sequenceDiagram
participant User
participant TUI as TUI App
participant Scanner as integrations::scan_integrations
participant Render as render_integrations
User->>TUI: /integrations
TUI->>Scanner: scan_integrations(project_root)
Scanner->>Scanner: detect markers per known integration
Scanner-->>TUI: IntegrationScanReport
TUI->>TUI: set AppMode::Integrations, store report
TUI->>Render: render_results/render_detail
Render-->>User: display detection status list
Estimated code review effort: 4 (Complex) | ~75 minutes Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request transitions the repository to a fully Rust-native implementation by removing Python bindings and PyO3 dependencies. It introduces Rust-native source adapters for DOX and Revolve, a read-only agent-framework discovery tool, and updates the CLI, Ratatui TUI, and installer to support these new features. Review feedback identifies a critical path traversal vulnerability in export path resolution, redundant lstat syscalls during directory traversal, a missing file size limit check in the DOX adapter, and case-sensitivity issues in both file extension matching and SHA-256 checksum verification.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if path.is_absolute() { | ||
| return Ok(path); | ||
| } | ||
| if path | ||
| .components() | ||
| .any(|component| matches!(component, Component::ParentDir)) | ||
| { | ||
| return Err("relative export paths cannot contain '..'".to_string()); | ||
| } |
There was a problem hiding this comment.
The path traversal check is bypassed when an absolute path is provided because path.is_absolute() returns early before the ParentDir (..) check is executed. This allows absolute paths containing .. components (e.g., /home/user/.tree-ring/exports/../../../../etc/passwd) to bypass validation and write files outside the intended directory.
To fix this, perform the ParentDir check before checking if the path is absolute.
| if path.is_absolute() { | |
| return Ok(path); | |
| } | |
| if path | |
| .components() | |
| .any(|component| matches!(component, Component::ParentDir)) | |
| { | |
| return Err("relative export paths cannot contain '..'".to_string()); | |
| } | |
| if path | |
| .components() | |
| .any(|component| matches!(component, Component::ParentDir)) | |
| { | |
| return Err("export paths cannot contain '..'".to_string()); | |
| } | |
| if path.is_absolute() { | |
| return Ok(path); | |
| } |
| let metadata = fs::symlink_metadata(&path).map_err(|err| sqlite_error(err.to_string()))?; | ||
| if metadata.file_type().is_symlink() { | ||
| continue; | ||
| } | ||
| if metadata.is_dir() { |
There was a problem hiding this comment.
Calling fs::symlink_metadata(&path) performs an extra lstat syscall for every directory entry. Since entry is a DirEntry, you can use entry.file_type() directly to check if it is a symlink or directory. This avoids redundant disk I/O and significantly improves performance during recursive directory traversal.
| let metadata = fs::symlink_metadata(&path).map_err(|err| sqlite_error(err.to_string()))?; | |
| if metadata.file_type().is_symlink() { | |
| continue; | |
| } | |
| if metadata.is_dir() { | |
| let file_type = entry.file_type().map_err(|err| sqlite_error(err.to_string()))?; | |
| if file_type.is_symlink() { | |
| continue; | |
| } | |
| if file_type.is_dir() { |
| let metadata = fs::symlink_metadata(&path).map_err(|err| sqlite_error(err.to_string()))?; | ||
| if metadata.file_type().is_symlink() { | ||
| continue; | ||
| } | ||
| if metadata.is_dir() { |
There was a problem hiding this comment.
Calling fs::symlink_metadata(&path) performs an extra lstat syscall for every directory entry. Since entry is a DirEntry, you can use entry.file_type() directly to check if it is a symlink or directory. This avoids redundant disk I/O and significantly improves performance during recursive directory traversal.
| let metadata = fs::symlink_metadata(&path).map_err(|err| sqlite_error(err.to_string()))?; | |
| if metadata.file_type().is_symlink() { | |
| continue; | |
| } | |
| if metadata.is_dir() { | |
| let file_type = entry.file_type().map_err(|err| sqlite_error(err.to_string()))?; | |
| if file_type.is_symlink() { | |
| continue; | |
| } | |
| if file_type.is_dir() { |
| fn events_from_agents_file( | ||
| request: &DoxSyncRequest, | ||
| path: &Path, | ||
| ) -> Result<Vec<MemoryEvent>, AdapterSkip> { | ||
| let content = | ||
| fs::read_to_string(path).map_err(|err| AdapterSkip::Unreadable(err.to_string()))?; |
There was a problem hiding this comment.
Unlike revolve.rs, dox.rs does not check the file size of AGENTS.md before reading it entirely into memory with fs::read_to_string. If a very large file is encountered, this could lead to high memory consumption or out-of-memory (OOM) errors. It is safer to query the file metadata and enforce a reasonable size limit (e.g., 256KB) before reading.
| fn events_from_agents_file( | |
| request: &DoxSyncRequest, | |
| path: &Path, | |
| ) -> Result<Vec<MemoryEvent>, AdapterSkip> { | |
| let content = | |
| fs::read_to_string(path).map_err(|err| AdapterSkip::Unreadable(err.to_string()))?; | |
| fn events_from_agents_file( | |
| request: &DoxSyncRequest, | |
| path: &Path, | |
| ) -> Result<Vec<MemoryEvent>, AdapterSkip> { | |
| let metadata = fs::metadata(path).map_err(|err| AdapterSkip::Unreadable(err.to_string()))?; | |
| if metadata.len() > 256 * 1024 { | |
| return Err(AdapterSkip::Unreadable("File too large".to_string())); | |
| } | |
| let content = | |
| fs::read_to_string(path).map_err(|err| AdapterSkip::Unreadable(err.to_string()))?; |
| fn is_supported_source(path: &Path) -> bool { | ||
| matches!( | ||
| path.extension().and_then(|extension| extension.to_str()), | ||
| Some("md" | "txt" | "json" | "jsonl") | ||
| ) | ||
| } |
There was a problem hiding this comment.
The file extension check is case-sensitive and will fail to match uppercase extensions like .MD, .TXT, or .JSON. Converting the extension to lowercase before matching ensures that files with uppercase extensions are correctly supported.
| fn is_supported_source(path: &Path) -> bool { | |
| matches!( | |
| path.extension().and_then(|extension| extension.to_str()), | |
| Some("md" | "txt" | "json" | "jsonl") | |
| ) | |
| } | |
| fn is_supported_source(path: &Path) -> bool { | |
| path.extension() | |
| .and_then(|extension| extension.to_str()) | |
| .map(|ext| matches!(ext.to_ascii_lowercase().as_str(), "md" | "txt" | "json" | "jsonl" )) | |
| .unwrap_or(false) | |
| } |
| verify_sha256() { | ||
| file=$1 | ||
| expected=$2 | ||
| [ "$expected" != "" ] || return | ||
| if command -v shasum >/dev/null 2>&1; then | ||
| actual=$(shasum -a 256 "$file" | awk '{print $1}') | ||
| elif command -v sha256sum >/dev/null 2>&1; then | ||
| actual=$(sha256sum "$file" | awk '{print $1}') | ||
| else | ||
| die "SHA-256 verification requested, but shasum/sha256sum was not found" | ||
| fi | ||
| [ "$actual" = "$expected" ] || die "archive checksum mismatch" | ||
| } |
There was a problem hiding this comment.
The SHA-256 checksum comparison is case-sensitive. If a user passes an uppercase SHA-256 hash via --archive-sha256, the comparison will fail because shasum and sha256sum output lowercase hex characters. Normalizing both hashes to lowercase before comparison makes the verification robust.
| verify_sha256() { | |
| file=$1 | |
| expected=$2 | |
| [ "$expected" != "" ] || return | |
| if command -v shasum >/dev/null 2>&1; then | |
| actual=$(shasum -a 256 "$file" | awk '{print $1}') | |
| elif command -v sha256sum >/dev/null 2>&1; then | |
| actual=$(sha256sum "$file" | awk '{print $1}') | |
| else | |
| die "SHA-256 verification requested, but shasum/sha256sum was not found" | |
| fi | |
| [ "$actual" = "$expected" ] || die "archive checksum mismatch" | |
| } | |
| verify_sha256() { | |
| file=$1 | |
| expected=$2 | |
| [ "$expected" != "" ] || return | |
| if command -v shasum >/dev/null 2>&1; then | |
| actual=$(shasum -a 256 "$file" | awk '{print $1}') | |
| elif command -v sha256sum >/dev/null 2>&1; then | |
| actual=$(sha256sum "$file" | awk '{print $1}') | |
| else | |
| die "SHA-256 verification requested, but shasum/sha256sum was not found" | |
| fi | |
| actual_lower=$(echo "$actual" | tr '[:upper:]' '[:lower:]') | |
| expected_lower=$(echo "$expected" | tr '[:upper:]' '[:lower:]') | |
| [ "$actual_lower" = "$expected_lower" ] || die "archive checksum mismatch" | |
| } |
1 participant
Summary:
Verification:
Review:
Summary by CodeRabbit
New Features
dox,revolve, andintegrationscommands across the CLI and TUI, including scan/sync flows and richer on-screen results.Documentation
Chores