Prevent monitor status TCC prompts in Git-Same 3.2.0 - #21
Open
manuelgruber wants to merge 21 commits into
Open
manuelgruber wants to merge 21 commits into
manuelgruber wants to merge 21 commits into
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughVersion 3.2.0 adds host-facing status mirroring, monitor version metadata, IPC configuration injection, filtered status watching, symlink migration handling, monitor/app version-skew detection, and guarded TUI status refreshes. ChangesHost IPC and monitor synchronization
TUI status refresh control
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Monitor
participant StatusFileWriter
participant HostStatusPath
participant StatusWatcher
participant TauriApp
Monitor->>StatusFileWriter: write FinderStatus
StatusFileWriter->>HostStatusPath: atomically write status.json
HostStatusPath->>StatusWatcher: notify status.json event
StatusWatcher->>TauriApp: emit status-updated snapshot
TauriApp->>HostStatusPath: read status snapshot
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit checks the status trail, Comment |
Reviewer's GuideAdds host-facing status mirroring and monitor version reporting to keep the macOS app and monitor in sync without triggering TCC prompts, refactors IPC/status handling to be safer and more robust in the presence of legacy symlinks and corrupt files, wires the Tauri host to read from the mirrored host path, and bumps the project to the 3.2.0 release line with dependency and test updates. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
manuelgruber
marked this pull request as ready for review
July 18, 2026 08:06
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/git-same-app/src/commands.rs`:
- Around line 1013-1026: Update the Monitor requirement’s passed condition in
the checks construction to also require monitor_version_mismatch to be false.
Preserve the existing running-agent and fresh-snapshot checks, while ensuring a
running, fresh monitor with a mismatched version is marked as failed.
In `@crates/git-same-core/src/ipc/status_file.rs`:
- Around line 117-129: Update remove_symlink_if_present to distinguish
symlink_metadata errors: return Ok(false) only for ErrorKind::NotFound, and
propagate all other inspection errors as AppError::path failures. Preserve the
existing symlink removal behavior and error handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cbc0eccc-5949-45b2-81ab-179756176bec
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockcrates/git-same-app/ui/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
Cargo.tomlcrates/git-same-app/Cargo.tomlcrates/git-same-app/src/commands.rscrates/git-same-app/src/commands_tests.rscrates/git-same-app/src/main.rscrates/git-same-app/src/status_stream.rscrates/git-same-app/src/status_stream_tests.rscrates/git-same-app/tauri.conf.jsoncrates/git-same-app/ui/package.jsoncrates/git-same-app/ui/src/lib/types.tscrates/git-same-cli/Cargo.tomlcrates/git-same-core/src/ipc/mod.rscrates/git-same-core/src/ipc/mod_tests.rscrates/git-same-core/src/ipc/status_file.rscrates/git-same-core/src/ipc/status_file_tests.rscrates/git-same-core/src/monitor/run.rscrates/git-same-core/src/monitor/socket_handler.rscrates/git-same-core/src/monitor/socket_handler_tests.rscrates/git-same-core/src/types/finder_status.rscrates/git-same-core/src/types/finder_status_tests.rsmacos/GitSameBadges/Info.plist
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 23 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 10 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 12 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/git-same-cli/src/tui/handler.rs">
<violation number="1" location="crates/git-same-cli/src/tui/handler.rs:138">
P2: The dashboard can show pre-sync repository statuses until the refresh interval expires when an older status scan is still running as sync completes: this guarded call is skipped while `app.status_loading` is true, and that older result is then treated as the fresh post-sync snapshot. A post-sync refresh should be queued and started after the in-flight scan finishes, or otherwise forced after sync completion.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 32 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
The non-sandboxed Tauri host read status.json from the App Group container, so macOS fired "Git-Same would like to access data from other apps" up to five times on launch and after each sync. The monitor now mirrors a real status.json into ~/.config/git-same/ finder/ (StatusFileWriter::new_with_mirrors) in addition to the container, and the host reads only that host-home copy via a shared tauri::State<HostIpc> resolved once at startup. ensure_legacy_symlinks no longer symlinks status.json (only finder.sock), and the host unlinks any leftover status.json symlink before reading so it never follows a link into the container during the upgrade window. The FinderSync extension, the socket, and all entitlements are unchanged, so no Apple re-sign or macOS-26 re-test is required.
time 0.3.52 changed its sealed Parsable::parse trait method from one argument to two (added defaults: Option<Parsed>). cookie 0.18.1, pulled in transitively via tauri, calls the one-argument form and fails to compile against time >= 0.3.52. No fixed cookie release exists (0.18.1 is the latest and tauri pins cookie 0.18), so cap time below 0.3.52 in the git-same-app manifest and re-pin the lockfile to the latest compatible time 0.3.51. Remove the cap once cookie ships a fix.
Updates 6 crates.io packages (clap_complete, console, indicatif, inotify-sys, libredox, tauri) and 3 npm packages (@lucide/svelte, @tauri-apps/cli, vite) to their latest semver-compatible releases. The time <0.3.52 cap in git-same-app/Cargo.toml stays in place since tauri's transitive cookie 0.18.1 still hasn't shipped a fix.
Address findings from an xhigh code review of the status-mirror change: - Gate the Windows-hostile suffix assert in the IPC test so S1 CI passes. - Make status mirror writes best-effort (warn, not error) so an unwritable host dir cannot crash-loop the monitor under launchd. - Add core remove_symlink_if_present (NotFound-tolerant), reuse it from the host, and drop the duplicated app-side helper and its synthetic test. - Move the mirror policy into IpcConfig::status_writer so a custom IpcConfig can never clobber the real user's host status.json. - Drop the false state-guard clones in the Tauri handlers, single-parse the status snapshot, and filter watcher events to status.json. - Correct the ipc module docs to describe the mirror design.
Add monitor_version to FinderStatus, stamped in FinderStatus::new with the building crate's CARGO_PKG_VERSION, so each status records the monitor build that wrote it. The Tauri Monitor requirement check compares it against the app's own version and, when a readable status reports a different build, tells the user to restart the monitor. Informational only: it does not flip the check to failed, and the stale hint still takes priority when no status is readable. Old status files without the field parse as None.
After an app upgrade the previously installed monitor keeps running the old build (launchd KeepAlive keeps the process alive; nothing restarts it). The old monitor only symlinks the host status.json into the container and never writes the mirror the new host reads, so the host deletes the leftover symlink and then shows stale/absent status until a manual restart. On startup, detect that leftover symlink (a reliable signal an old monitor is running) via symlink_metadata, which does not follow the link into the app-group container, and best-effort restart the installed monitor on a background thread so the on-disk build takes over and starts mirroring. Skip when no LaunchAgent is installed so a monitor is never created implicitly. This complements the stale-status guidance text by making the common upgrade case self-heal without user action.
Make the app's Monitor requirement fail its pass check on a build-version skew via a new monitor_requirement_passed helper, so the row no longer shows a green check while its message and suggestion say to restart the monitor. Log the status watcher's watcher-error and snapshot-read-error paths instead of swallowing them, so a stale dashboard leaves a diagnostic trail. Propagate non-NotFound symlink_metadata failures from remove_symlink_if_present so read_status_snapshot_with aborts rather than dereferencing a path it could not inspect, preserving the TCC-safety guarantee.
Run status scans as guarded background work, recover legacy Status states, refresh requirements, and show animated feedback. Add regression coverage for manual, automatic, and post-sync refresh paths.
Route the monitor LaunchAgent through Git-Same.app's main executable (headless `git-same-app monitor`) so one Full Disk Access grant covers both the app and the monitor. The helper at Contents/Helpers/git-same was a separate, path-based TCC identity that the grant never reached, so the monitor kept prompting for Desktop, Documents, Downloads, and volume access no matter what the user granted. Replace the "zero repos" Full Disk Access heuristic with a real probe (opening the user TCC database; silent, never prompts). The monitor stamps its own answer into status.json, the app reports host and monitor answers, and enable_finder_extension refuses to set the pluginkit election until the gate passes. The badge checklist now runs monitor, FDA, installed, enable; permissions re-probe on window focus; a lagging monitor is restarted once after the grant lands; the app re-renders an installed agent that still execs the helper. Shared monitor shim pieces (Options::from_config, the shutdown signal) move into git-same-core so the CLI and app use the same code. The cask renders the new agent path, the bundle Info.plist gains usage strings, and the dead Banner.svelte is removed.
Core and app test env guards now fail fast if Config::default_path() resolves outside the temp home, so a broken override can no longer register temp workspaces in the developer's real ~/.config/git-same/config.toml. The ten stale my-ws entries found there were historical; the current tree does not reproduce them.
Make the EPERM probe test portable (raw OS error 1 is not PermissionDenied on Windows), allow clippy's beta-only double_must_use on the async_trait Provider trait, and run the Security Audit job on current stable because cargo-audit's dependency graph (kstring 2.0.4) outgrew the pinned 1.93.1 toolchain. Also correct the LaunchAgent migration comment to the 3.2 release.
Address the unresolved review threads on PR #21, re-verified against the rebased tree. TUI: DiscoveryError no longer tears down an in-flight operation. Per-org discovery failures are non-fatal in the provider, so resetting to Idle dropped the guards that keep a status refresh, and a second concurrent sync, from starting mid-run. Also clear status_loading on OperationError and stop recording a short-circuited Status run as a sync. IPC: move aside a directory left at the host status.json path. The mirror write fails EISDIR forever otherwise, and a mirror failure is only a warning, so the monitor ran with host status permanently absent. App: withhold the host-probe fallback in the Full Disk Access gate when a fresh monitor reports no answer and the agent is not app-owned, since the grant does not reach a CLI-owned helper. Broaden startup recovery beyond the leftover symlink, which the first status read erases. Expose a guarded restart the UI can use without implicitly installing a service. UI: kickMonitorIfLagging now skips monitors it cannot restart, clears its latch on failure so transient launchctl errors retry, and re-reads Full Disk Access on success so the badge action unlocks immediately. Deps: drop the time cap. cookie 0.18.2 no longer calls Parsable::parse, so the 0.3.52 incompatibility no longer applies. Tests: assert the mirror entries are real files rather than symlinks, add a crate-wide env lock so the IPC path readers cannot race the config tests that swap HOME, and de-duplicate the TUI spinner onto one table.
Member
Author
|
Two findings raised in local review (not GitHub threads), recorded here for the trail. Both relate to commit 087b51a.
The bare
The premise is stale: The residual gap was the trigger. It keyed on a leftover symlink at the host status path, but
The recovery also moved off a raw |
The app-identity commit left `use std::process::Command` and the `render_for_app` plist test helper reachable only from macOS- and unix-gated code, so `-D warnings` failed the ubuntu and windows S1 legs while macOS stayed green. Spell the one bare `Command` call site out in full and drop the import, and gate `render_for_app` plus its `source` import with `#[cfg(unix)]` to match their only caller. Verified with a containerized Linux `cargo +stable clippy --workspace --all-targets --all-features -- -D warnings`, which is the exact S1 command and cannot be run natively on macOS.
The cask install test compared the rendered LaunchAgent against
`app.join("Contents/MacOS/git-same-app")`, a slash literal, while the
plist carries the three-join form that renders as backslashes on
Windows, so the only failing test of the S1 windows leg was a
separator mismatch rather than a behaviour difference. Build the
expectation with `source::app_main_executable` so the assertion keeps
its meaning on every platform instead of being gated off.
Section 8 still described the pre-3.2 helper model, so the release gate never checked the behaviour this version exists to deliver. The LaunchAgent now execs the bundle's own main executable in place, which is what makes one Full Disk Access grant cover the monitor, so the matrix gains rows for the pre-3.2 agent re-render, build skew after an app upgrade, the Full Disk Access gate in both directions, and a moved or deleted bundle. Rows that still assume a copied helper are reworded for CLI owners, where that copy and its separate TCC identity remain. Also record the two hand-maintained versions (ui/package.json and tauri.conf.json) in section 1, which no CI job gates.
All three open alerts are the same advisory: vitest >= 2.1.0 < 4.1.11 flagged against package.json and pnpm-lock.yaml, plus its transitive @vitest/mocker. The bump crosses a major, so it was verified rather than assumed: installed with the pinned pnpm 11.0.9 in a container, then svelte-check (0 errors), the 31 unit tests, and vite build all pass, and a second `--frozen-lockfile` install reproduces it exactly as CI does. The lockfile shrinks because vitest 3 carried its own vite 7 and vite-node, while vitest 4 reuses the vite 8 already pinned here. Only the vitest entry moves in package.json. Dev dependency only: nothing in a shipped artifact changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds host-facing atomic status mirroring through
IpcConfig, migrates legacy status symlinks safely, and keeps mirror failures non-fatal while primary writes still fail normally, preventing macOS cross-app access prompts in the Tauri host.It adds a backward-compatible optional
monitor_versionfield toFinderStatus, reports app/monitor build skew, and automatically restarts an already-installed stale monitor after upgrades so status recovers without manual intervention.It also bumps Git-Same to 3.2.0 and refreshes in-range Cargo and pnpm dependencies.
Validation passed with
cargo fmt --all -- --check,cargo test --workspace, andpnpm --dir crates/git-same-app/ui check.Summary by Sourcery
Add host-facing status mirroring and monitor version tracking to avoid macOS TCC prompts and handle monitor/app skew while bumping Git-Same to 3.2.0.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Summary by CodeRabbit
Monitor identity and Full Disk Access gate (added 2026-09-09)
Git-Same.app/Contents/MacOS/git-same-app monitor --foreground(headless monitor mode in the app binary) instead ofContents/Helpers/git-same. macOS TCC attributes a launchd-spawned process to its bundle only through the bundle's own executable, so this is what lets a single Full Disk Access grant for Git-Same cover the monitor. The app re-renders an installed agent that still points at the helper, and the cask renders the new path.status.jsonasfull_disk_access; the app reports host and monitor answers.enable_finder_extension/disable_finder_extensioncommands set the pluginkit election in-app. Enabling is refused until Full Disk Access is granted. The badge checklist is reordered (monitor, FDA, installed, enable badges), the banner asks for FDA before offering the extension, permissions re-probe on window focus, and a lagging monitor is restarted once after the grant lands.Options::from_config,default_shutdown_signal) moved intogit-same-core; usage strings added to the bundle Info.plist; deadBanner.svelteremoved.double_must_useallow, and the audit job runs on current stable.Note for release testing: macOS applies a new FDA grant only to processes started afterwards, so after granting it quit and reopen the app; the app restarts the monitor itself.
Merge readiness (updated 2026-09-21)
S1 is green on
01bb2b9across every job for the first time on this branch: run 35655452798. That includes the four Test legs, Code Coverage, Security Audit, and the Build and Tauri App Debug Build jobs, which earlier runs skipped because the test matrix gated them.Four commits were added after the rebase to get there:
eaf1dc4Gate macOS-only items so clippy passes off macOS.use std::process::Commandand therender_for_appplist helper were reachable only from macOS- and unix-gated code, so-D warningsfailed the ubuntu and windows legs while macOS stayed green.643ab4eDerive the plist path in the cask install test fromsource::app_main_executableinstead of a slash literal, so the string comparison matches the platform separator on Windows.12ada56Rewrite the section 8 acceptance matrix intoolkit/packaging/release-checklist.mdfor the app-identity agent, with new rows for the pre-3.2 agent re-render, build skew after an app upgrade, the Full Disk Access gate in both directions, and a moved or deleted bundle.01bb2b9Bump vitest to 4.1.11, clearing the three open Dependabot alerts (the advisory and its transitive@vitest/mocker). Dev dependency only.The
time < 0.3.52cap noted above was dropped earlier in the branch; the lockfile now resolvestime 0.3.55withcookie 0.18.2.Still outstanding after merge: tag
3.2.0, run S2 / S3 / S4, and work section 8 of the release checklist on a disposable macOS account, which is the only way to verify that one Full Disk Access grant reaches the monitor.