Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR merges the dev branch into master as preparation for the v0.3.0 release, bringing in the current Rust implementation of the changed CLI + changedd daemon, along with documentation, packaging artifacts, and CI configuration.
Changes:
- Introduces the core Rust crate implementing scope-aware tracking (user/system), journaling, diff/redaction, and daemon/service management.
- Adds extensive end-user/docs content (README, manpage-style docs, scope model, categories, examples) plus Arch packaging files.
- Adds Sonar configuration and a GitHub Actions workflow intended to run static analysis.
Reviewed changes
Copilot reviewed 20 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/scope.rs | Defines Scope enum (system/user) with serde + display helpers. |
| src/main.rs | Implements the changed CLI (clap-based) including scope flag handling and command dispatch. |
| src/lib.rs | Exposes crate modules for use by binaries. |
| src/journal.rs | Defines JournalEvent and related serialization structures. |
| src/config.rs | Defines TOML config schema and sorting/dedup logic for tracked targets. |
| src/category.rs | Defines Category enum, clap parsing, and string/display helpers. |
| src/bin/changedd.rs | Implements the dedicated daemon binary argument parsing + execution. |
| src/app.rs | Core application logic: paths, config/state IO, daemon loop, diffing, redaction, retention, systemd service integration, tests. |
| sonar-project.properties | Adds Sonar project configuration. |
| README.md | Project overview, usage, scope model summary, and workflow examples. |
| PKGBUILD | Adds Arch Linux packaging recipe for both binaries + docs + unit files. |
| packaging/systemd/user/changedd.service | Packaged systemd user unit for changedd. |
| packaging/systemd/system/changedd.service | Packaged systemd system unit for changedd. |
| example-log.md | Provides example output for “clean” and “full” history views. |
| docs/scope-model.md | Documents the scope/security model and CLI expectations. |
| docs/packaging-workflow.md | Documents how the repository maintains Arch packaging. |
| docs/help-text.md | Captures help-text drafts for CLI commands. |
| docs/changed.1.md | Man-page-style reference for changed and changedd. |
| docs/categories.md | Documents category semantics and intended usage. |
| Cargo.toml | Defines crate metadata, edition, and dependencies for v0.3.0. |
| Cargo.lock | Locks dependency versions for reproducible builds. |
| .gitignore | Expands ignored local/dev artifacts and packaging outputs. |
| .github/workflows/build.yml | Adds CI workflow (currently Sonar scan only). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn merge_reloaded_observed( | ||
| previous: &BTreeMap<String, ObservedPath>, | ||
| current: BTreeMap<String, ObservedPath>, | ||
| ) -> BTreeMap<String, ObservedPath> { | ||
| let mut merged = BTreeMap::new(); | ||
| for (path, current_observed) in current { | ||
| if let Some(previous_observed) = previous.get(&path) { | ||
| merged.insert(path, previous_observed.clone()); | ||
| } else { | ||
| merged.insert(path, current_observed); | ||
| } | ||
| } |
There was a problem hiding this comment.
merge_reloaded_observed() clones the entire previous ObservedPath when a path already existed, which prevents config changes (category/diff_mode/redaction) from taking effect after a reload. This can make diff/redaction toggles ineffective until the daemon restarts. Consider merging only the stateful fields you want to preserve (e.g., exists/fingerprint/text_snapshot) while taking policy fields from the newly observed entry.
| pub fn sort_and_dedup(&mut self) { | ||
| self.tracked_paths.sort_by(|a, b| { | ||
| a.category | ||
| .cmp(&b.category) | ||
| .then_with(|| a.path.cmp(&b.path)) | ||
| .then_with(|| path_kind_rank(a.kind).cmp(&path_kind_rank(b.kind))) | ||
| }); | ||
| self.tracked_paths.dedup_by(|a, b| a.path == b.path); | ||
|
|
There was a problem hiding this comment.
Config::sort_and_dedup() calls dedup_by(|a,b| a.path == b.path) but sorts tracked_paths by category first, so duplicate paths with different categories won't be adjacent and won't be deduped. If the intent is “one entry per path”, sort primarily by path (or dedup via a map/set) before calling dedup_by.
| fn resolve_single(self) -> Result<Scope> { | ||
| self.resolve_single_with_message("Error: unclear scope. Please specify -S or -U.") | ||
| } | ||
|
|
||
| fn resolve_single_with_message(self, missing_message: &str) -> Result<Scope> { | ||
| match (self.system, self.user) { | ||
| (true, false) => Ok(Scope::System), | ||
| (false, true) => Ok(Scope::User), | ||
| (true, true) => Err(anyhow!("Writes must target exactly one scope. `-SU` is invalid here.")), | ||
| (false, false) => Err(anyhow!(missing_message.to_owned())), | ||
| } |
There was a problem hiding this comment.
Some errors include an "Error: " prefix (e.g., resolve_single()/resolve_write_scope), but main() already prints errors as "Error: {error}". This results in messages like "Error: Error: unclear scope..." and is inconsistent with other error strings. Prefer returning errors without the redundant prefix and let the top-level handler add "Error:" once.
| with: | ||
| fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis | ||
| - name: SonarQube Scan | ||
| uses: SonarSource/sonarqube-scan-action@v6 |
There was a problem hiding this comment.
The workflow uses SonarSource/sonarqube-scan-action but sonar-project.properties is configured with sonar.organization (SonarCloud-style) and no sonar.host.url/SONAR_HOST_URL is provided. This is likely to make the scan fail or target the wrong server. Either switch to the SonarCloud GitHub Action or explicitly configure the host URL/server credentials for SonarQube.
| uses: SonarSource/sonarqube-scan-action@v6 | |
| uses: SonarSource/sonarcloud-github-action@master |
| - name: SonarQube Scan | ||
| uses: SonarSource/sonarqube-scan-action@v6 | ||
| env: | ||
| SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} |
There was a problem hiding this comment.
CI currently runs only a Sonar scan and does not build or test the Rust crate. Since this PR is release prep, consider adding steps/jobs for cargo fmt/cargo clippy (optional) and at minimum cargo build + cargo test so regressions are caught in PRs.
| SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} | |
| SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} | |
| rust-ci: | |
| name: Rust CI | |
| runs-on: ubuntu-latest | |
| steps: | |
| - uses: actions/checkout@v4 | |
| - name: Install Rust toolchain | |
| uses: dtolnay/rust-toolchain@stable | |
| with: | |
| components: rustfmt, clippy | |
| - name: Check formatting | |
| run: cargo fmt --check | |
| - name: Run clippy | |
| run: cargo clippy --all-targets --all-features -- -D warnings | |
| - name: Build crate | |
| run: cargo build --verbose | |
| - name: Run tests | |
| run: cargo test --verbose |
|
Not a fan of SonarQube ATM. Closing with intent to remove the workflow. |
Merging dev branch as prep for release.