File Provider: configure domains and shared state - #31
Open
obra wants to merge 4 commits into
Open
Conversation
Configure Remux with the accepted shared App Group root and explicit application and shared Keychain access groups. Keep the credential service identity centralized while allowing the existing Keychain store to target either access group through the accepted structured query path. Preserve the exact accepted F1 test boundary: shared-root resolution, explicit access-group query construction, and shared configuration lookup. The focused suite first failed at the expected missing symbols, then passed 3 tests with 0 failures and 0 skips. A Remux simulator build also succeeded. Generate the app entitlement, plist values, and Xcode project membership from the accepted configuration with XcodeGen 2.44.1; consecutive generations were deterministic. Shared-state migration, live application/shared Keychain separation, and all File Provider extension wiring remain deferred to F2 and later branches.
Copy profiles, credentials, and hostname-bound trust into the shared container without deleting application-local source state. Verify copied credentials and repository/trust snapshots before atomically writing the migration marker, so failed attempts remain retryable and completed attempts are idempotent. Expose only the application/shared Keychain store factory at this layer. Migration construction, authoritative shared-repository selection, and lifecycle activation remain intentionally absent for the final app-integration layer. Preserve the SSH setup rollback-only trust APIs while adding bulk migration access and binding accepted trust to both server identity and hostname. Verification: accepted tests produced the expected missing-helper compile RED; the focused migration, shared-storage, and trusted-host suites passed 11 tests with 0 failures and 0 skips; the Remux simulator build succeeded. XcodeGen 2.44.1 was deterministic, runtime production delta is 124 lines, and all complete blobs/hunks match their accepted provenance.
Transplant the accepted inactive File Provider domain model exactly as reviewed. Eligible records require a saved SSH credential and trusted host identity matching both server ID and hostname. UUID-derived domain identifiers remain stable, display-name changes reconcile as remove/add pairs, and a FIFO actor gate serializes concurrent callers. Keep this leaf deliberately unreachable: it links FileProvider.framework into the app target but adds no dependency factory, live instance, RootModel or lifecycle invocation, host-mutation hook, extension target, or compatibility path. Registry and storage failures propagate after the gate is released; no new retry or cleanup behavior is introduced. The accepted three-test blob covers eligibility, deterministic add/rename/remove behavior with concurrent reconciliation serialized to one mutation, and host-change removal until replacement hostname trust. The tests-first build failed at the expected missing F3 types. The final focused suite passed 3/3 with no failures or skips, and the Remux iPhone 17 simulator build succeeded. XcodeGen 2.44.1 generated the same PBX blob twice.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThe PR adds File Provider domain reconciliation and shared-storage migration. It adds keychain access-group support, shared container configuration, hostname-sensitive trust validation, tests, entitlements, and Xcode project integration. ChangesFileProvider integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant FileProviderDomainReconciler
participant Profiles
participant KeychainSSHCredentialStore
participant TrustedHostStore
participant NSFileProviderDomainRegistry
FileProviderDomainReconciler->>Profiles: Load saved servers
FileProviderDomainReconciler->>KeychainSSHCredentialStore: Load credentials
FileProviderDomainReconciler->>TrustedHostStore: Load trusted identities
FileProviderDomainReconciler->>NSFileProviderDomainRegistry: List registered domains
FileProviderDomainReconciler->>NSFileProviderDomainRegistry: Remove stale or changed domains
FileProviderDomainReconciler->>NSFileProviderDomainRegistry: Add missing or changed domains
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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. Comment |
Contributor
Author
|
@h3nock - Let me know if slicing into PRs and an attempt at logical commits like this is actually useful to you. If it's not, I'll make all of these go away and we can talk through whether there's a sane way to deliver something like the FileProvider work. |
h3nock
self-requested a review
August 12, 2026 14:53
h3nock
approved these changes
Aug 12, 2026
| let existingRecords = try await registry.records() | ||
|
|
||
| for record in existingRecords | ||
| where desiredRecords.first(where: { $0.serverID == record.serverID }) != record { |
Owner
There was a problem hiding this comment.
would it make sense to avoid removing the domain on rename? i think Apple supports updating the name by calling add with the same ID. i reviewed this with Codex against Apple’s docs.
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
2 participants
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.
Reviewability experiment
This is the head of an attempt to make the File Provider work more reviewable. If this stack or this boundary does not feel right, please say so instead of spending effort reviewing an unhelpful presentation—we can reshape the history first.
This is a history/presentation rewrite of the already-tested implementation. It does not add new product behavior.
Intended review slice
upstream/maincodex/file-provider-review/domain-foundationThe commits in this branch are the atomic commits intended for this slice:
Stack mechanics
All PRs in this stack intentionally target
main. The branches themselves remain sequential, so GitHub may show earlier ancestry in a later PR until its parents merge. Please review the commit range above as the intended slice and merge these PRs in order; after a parent lands, GitHub will reduce the next PR diff to its incremental changes.Verification
The final rewritten leaf was verified with 1164 tests passed, 0 failed, and 1 skipped.
xcodegen generateproduced no project diff, and the final tree is identical to the previously tested simplified result.Summary by CodeRabbit
New Features
Bug Fixes
Tests