Skip to content

Stabilize desktop identity and quiet passive keychain reads - #97

Merged
gajendraxdev merged 1 commit into
zync-sh:mainfrom
gajendraxdev:fix/macos-keychain-prompts
Aug 20, 2026
Merged

Stabilize desktop identity and quiet passive keychain reads#97
gajendraxdev merged 1 commit into
zync-sh:mainfrom
gajendraxdev:fix/macos-keychain-prompts

Conversation

@gajendraxdev

@gajendraxdev gajendraxdev commented Aug 20, 2026

Copy link
Copy Markdown
Member

Zync needs a stable cross-platform bundle identity before future releases, but existing users already have data under legacy default identities. This change standardizes the identifier to in.thesudoer.zync, removes the macOS-only CI patch, and adds a copy-only migration path for legacy default data/config directories while preserving custom dataPath setups.

The vault and sync status paths now avoid reading OS credential-store secrets for passive UI refreshes. Secret reads remain on real unlock/sync/token-use paths, which reduces repeated macOS Keychain prompts without pretending unsigned builds can avoid all system trust prompts.

Constraint: macOS builds are unsigned/not notarized, so Keychain trust prompts cannot be fully eliminated without Apple signing.

Constraint: Existing users may have default data under zync or com.zync.desktop identifiers.

Rejected: Keep macOS-only com.zync.desktop override | would leave future Windows/Linux identity migration work and inconsistent product identity.

Rejected: Directly copy legacy data into the final target | partial failures could leave mixed migrated state.

Confidence: high

Scope-risk: moderate

Directive: Keep OS Keychain service/account names stable unless a dedicated secret migration is shipped.

Tested: cargo check --manifest-path src-tauri\Cargo.toml

Tested: cargo test --manifest-path src-tauri\Cargo.toml identity_migration

Tested: cargo test --manifest-path src-tauri\Cargo.toml sync::collection

Tested: cargo test --manifest-path src-tauri\Cargo.toml google

Tested: cargo test --manifest-path src-tauri\Cargo.toml session_cache

Tested: npm run test:sync-passphrase

Not-tested: Manual macOS Keychain prompt behavior on an unsigned packaged build

Summary by CodeRabbit

  • Changed

    • Standardized the desktop app identity across platforms.
    • Added first-launch migration from legacy app-data and configuration folders while preserving custom data locations.
    • Reduced unnecessary macOS Keychain prompts during passive status checks.
  • Bug Fixes

    • Improved sync collection key recovery and clearly report unrecoverable remote links.
    • Preserved legacy Google refresh tokens during migration to secure storage.
    • Improved session-cache handling to prevent repeated restore attempts during status polling.

Zync needs a stable cross-platform bundle identity before future releases, but existing users already have data under legacy default identities. This change standardizes the identifier to in.thesudoer.zync, removes the macOS-only CI patch, and adds a copy-only migration path for legacy default data/config directories while preserving custom dataPath setups.

The vault and sync status paths now avoid reading OS credential-store secrets for passive UI refreshes. Secret reads remain on real unlock/sync/token-use paths, which reduces repeated macOS Keychain prompts without pretending unsigned builds can avoid all system trust prompts.

Constraint: macOS builds are unsigned/not notarized, so Keychain trust prompts cannot be fully eliminated without Apple signing.

Constraint: Existing users may have default data under zync or com.zync.desktop identifiers.

Rejected: Keep macOS-only com.zync.desktop override | would leave future Windows/Linux identity migration work and inconsistent product identity.

Rejected: Directly copy legacy data into the final target | partial failures could leave mixed migrated state.

Confidence: high

Scope-risk: moderate

Directive: Keep OS Keychain service/account names stable unless a dedicated secret migration is shipped.

Tested: cargo check --manifest-path src-tauri\\Cargo.toml

Tested: cargo test --manifest-path src-tauri\\Cargo.toml identity_migration

Tested: cargo test --manifest-path src-tauri\\Cargo.toml sync::collection

Tested: cargo test --manifest-path src-tauri\\Cargo.toml google

Tested: cargo test --manifest-path src-tauri\\Cargo.toml session_cache

Tested: npm run test:sync-passphrase

Not-tested: Manual macOS Keychain prompt behavior on an unsigned packaged build
@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e5d2bb51-7789-4935-b84a-54ceafa379c6

📥 Commits

Reviewing files that changed from the base of the PR and between eaed022 and 5862feb.

📒 Files selected for processing (12)
  • .github/workflows/release.yml
  • CHANGELOG.md
  • docs/SETTINGS_SYSTEM.md
  • src-tauri/src/commands.rs
  • src-tauri/src/identity_migration.rs
  • src-tauri/src/lib.rs
  • src-tauri/src/sync/collection.rs
  • src-tauri/src/sync/commands.rs
  • src-tauri/src/sync/providers/google.rs
  • src-tauri/src/vault/session_cache.rs
  • src-tauri/src/vault/store.rs
  • src-tauri/tauri.conf.json
💤 Files with no reviewable changes (1)
  • .github/workflows/release.yml

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change standardizes the Tauri application identity, adds safe migration from legacy default directories, reduces passive secret reads, and updates sync collection cache and key-recovery handling.

Changes

Application identity and migration

Layer / File(s) Summary
Canonical identity and legacy directory migration
.github/workflows/release.yml, CHANGELOG.md, docs/SETTINGS_SYSTEM.md, src-tauri/...
The Tauri identifier is now in.thesudoer.zync. First launch can copy validated legacy data and configuration directories into empty default targets without changing custom dataPath locations.
Passive credential and vault status handling
src-tauri/src/vault/*, src-tauri/src/sync/providers/google.rs, CHANGELOG.md
Vault status polling attempts session-cache restoration once per state cycle. Google status and legacy token snapshots use metadata-only loading, while plaintext refresh tokens migrate to the keyring during token loading.
Sync collection cache and key recovery
src-tauri/src/sync/collection.rs, src-tauri/src/sync/commands.rs, CHANGELOG.md
Cache status and expiration use persisted metadata freshness. Collection setup returns explicit errors when an existing remote collection has no recoverable key wrap.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to 5862f

The PR standardizes desktop identity and reduces passive keychain reads; no actionable merge-blocking risk remains based on the supplied evidence.

Suggested reviewers: type-delta

Sequence Diagram(s)

sequenceDiagram
  participant Application
  participant get_data_dir
  participant migrate_default_dirs
  participant Filesystem
  Application->>get_data_dir: resolve application data directory
  get_data_dir->>migrate_default_dirs: migrate default directories
  migrate_default_dirs->>Filesystem: validate and stage legacy directory contents
  Filesystem-->>migrate_default_dirs: install copied directory and marker
  migrate_default_dirs-->>get_data_dir: return migration result
  get_data_dir-->>Application: return effective data directory
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 24.49% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 49 functions across 8 files. (3 skipped: 3 unsupported.) Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the two primary changes: desktop identity standardization and reduced passive keychain reads.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Clippy (1.97.1)

Clippy execution timed out


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gajendraxdev
gajendraxdev merged commit e32f27a into zync-sh:main Aug 20, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant