File Provider: own request lifecycle - #41
Open
obra wants to merge 21 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.
Move transport startup tracing into a dedicated source that supplies the accepted no-op implementation when REMUX_FILE_PROVIDER_EXTENSION is active. Keep Ghostty tracing out of extension-conditioned SSH cleanup and SFTP setup, and expose root-key construction from the server and resolved-auth pair needed by extension callers. Regenerate deterministic app-target membership for the new source with XcodeGen 2.44.1. The existing SSH transport and exec selectors pass 74/74 and the normal iPhone 17 simulator build succeeds through XcodeBuildMCP with the required compiler wrappers. No test file changes are included. Global conditioned CLI builds remain a separate target-local extension gate because the setting propagates into SwiftPM dependencies and fails inside Crypto before Remux is compiled.
Define structured SFTP file type, metadata, directory-entry, readable-file, and read-only client values for listings and downloads. Bound downloads to monotonic chunks and ensure success, timeout, and cancellation close child handles and clean partial files. Reuse the existing connection lease and operation-timeout boundaries. Evidence: TerminalPreviewFileLoaderTests passed 7/7 at baseline; the focused suites retained the expected missing-read-layer compile failure before production and then passed 15/15; the normal iPhone 17 simulator build passed. XcodeGen 2.44.1 produced deterministic test membership.
Implement strict exclusive SFTP upload creation together with explicit directory creation, rename, file removal, and empty-directory removal operations. Normalize Citadel write statuses into typed permission and unsupported-mutation errors. Close remote upload handles on cancellation without issuing later mutations, and keep terminal attachments on the shared SFTP upload behavior. Evidence: the write tests produced a compile failure on the absent contracts before the source changes; all 38 focused tests passed afterward; the normal Remux iPhone 17 simulator build passed with isolated DerivedData and the required compiler probes; exact blob, scope, line-count, project-file, and diff checks passed.
Reconstruct the accepted remote-item model as one reviewable F7 commit directly on the strict SFTP write transport boundary. This moves only the immutable accepted bytes for canonical relative paths, File Provider identifier encoding, contained symlink projection, remote-item metadata/versioning, and their 12 focused tests. Accepted provenance: - FileProviderRemotePath.swift: 16813f6 - FileProviderRemoteItem.swift: 859c016 - FileProviderRemoteItemTests.swift: f8527a8 - source commits: df3cd2c, f2f82db, and 9fcd3fd Verification: - parent File Provider storage/domain suites: 10/10 passing - tests-first RED: FileProviderRemoteItemTests failed only for the absent accepted model types - GREEN: FileProviderRemoteItemTests 12/12 passing - normal iPhone 17 simulator build passed - XcodeGen 2.44.1 was deterministic across two generations - project.yml remains byte-identical to the parent - the 12 generated PBX membership lines are an accepted-oracle subset This deliberately excludes FileProviderRemoteService changes, stable opaque identity work, writable projection contracts, File Provider extension activation, compatibility behavior, and any newly authored product or test logic.
Reconstruct the accepted F8 identity boundary as a direct child of the contained remote-item leaf. This is a move-only commit: the production and test files retain their immutable accepted blob bytes, and the project membership is the deterministic XcodeGen output. Move FileProviderRemotePath.swift blob 4f097dc, FileProviderItemProjection.swift blob 5d99e0f, FileProviderItemIdentity.swift blob 6894a71, and FileProviderRemoteItemTests.swift blob b9bf198. The identity value blob composes accepted commits a8ea8cd, e9f19c9, and 1f85860; the projection blob composes a6cc7d3 and a8ea8cd. The F7 focused baseline passed 12/12. Moving only the accepted F8 test blob produced the expected compile RED for the missing identity type and codec API. After moving the three production blobs, the same suite passed 12/12 and a normal iPhone 17 simulator build succeeded. XcodeGen 2.44.1 was deterministic across two runs, and all eight generated PBX additions are present in the accepted oracle. This leaf intentionally excludes writable projection capabilities, snapshot or mutation behavior, extension activation, live-host qualification, compatibility work, and any newly authored glue or tests.
Reconstruct the accepted snapshot-generation store and F6-compatible File Provider error mapper as one atomic review leaf. The accepted snapshot tests call the mapper, while the mapper consumes FileProviderSnapshotStoreError, so separating them would require an authored temporary seam. Move exact accepted blobs 159f2cb and 26fc78b with exact test blobs 2e43e56 and e1d0586. XcodeGen 2.44.1 deterministically adds only the 16 accepted source and test membership lines. Evidence: the F8 baseline passed 12/12; tests-first boundary discovery exposed the missing mapper dependency; the combined accepted tests then failed for only the absent store and mapper; the earlier accepted mapper proved non-exhaustive against the already-accepted F6 write transport; the exact F6-compatible mapper resolved that compile dependency; the final combined suites passed 22/22 and a normal iPhone 17 simulator build succeeded. This coherent 592-line production unit is the documented first-pass exception to the 500-line ceiling. It excludes local mutation commits, receipts, identity reservations, relocations, deleted identities, and later writable error cases owned by F10 and F13. No product, test, or project behavior was authored.
Move the accepted mutation-journal layer onto the bounded snapshot-generation parent without rewriting behavior. Snapshot mutations now persist identity reservations, relocations, deletions, refreshed directories, replay receipts, and opt-in pending signals as one generation or not at all. The commit contains only the exact accepted FileProviderSnapshotStore blob 5c319ca and exact 23-test blob dd23955, sourced from immutable commits e9f19c9, 234e3cf, 962c8b8, 28135e5, and 999e676. Relative to F9 it is +255/-1 production and +278/-0 tests; project.yml and the PBX project remain byte-identical. Verification: the F9 combined baseline passed 22/22; the accepted tests-first state failed only on missing mutation-journal APIs and associated types; the final snapshot and mapper suites passed 31/31; and the normal Remux simulator build succeeded.
Reconstruct the accepted per-domain operation coordinator as an atomic stack layer on the exact F10 mutation-journal parent. The coordinator preserves same-directory refresh coalescing, serial refresh/mutation ordering, waiter-owned cancellation, and the existing DEBUG behavior gates without adding later enumeration or signaling contracts. The production and test files are byte-identical to accepted blobs c527084 and 50abd44. Their provenance is the immutable accepted sequence 5109406, e8ec5cf, and b0b0b27, with accepted test stabilization from 5396170 and 43c75ce. XcodeGen contributes only the eight expected project-membership lines; project.yml is unchanged. Verification before commit: F10 snapshot/error-mapper baseline 31/31; test-first compile RED confined to absent coordinator/polling types; coordinator GREEN 6/6; combined F10+F11 regression 37/37; normal Remux iPhone simulator build succeeded.
Reconstruct the reviewed remote-read boundary from accepted bytes. The remote service and its tests come from the archived read-only implementation, while the Citadel provider composes the accepted read-only operation signature with the reconciled shared SSHAuthenticationMethodFactory call. Keep the provider scoped to RemuxSFTPReadOnlyClient and do not restore the obsolete RemuxSSHAuthenticationMethodFactory wrapper. Seed only the two accepted contract tests for password authentication and root-key construction. The replicated-extension-core test is intentionally omitted here and remains deferred to F21. This commit adds only the two production files, two test files, and deterministic XcodeGen membership. Verification covered the 37-test F11 baseline, an expected test-first compile failure on absent F12 types, the 51-test focused and SFTP regression gate, and a normal Remux simulator build.
Reconstruct F13 strictly by moving the immutable accepted source and test blobs named in the File Provider stack plan. Add the request, validator, and initial SDK-item contracts; extend the accepted error mapper and writable item projection; and carry the exact validator, mapper, and remote-item test boundaries. The accepted tests moved first and produced the expected compile-only RED for the absent mutation validator contracts. After moving the accepted production bytes, the focused validator, mapper, and remote-item gate passed 30 tests, the retained F12 remote-service and shared-contract regression gate passed 13 tests, and the normal Remux simulator build succeeded. Regenerate the Xcode project with XcodeGen 2.44.1. project.yml remains byte-identical and the project file contains exactly the 16 accepted-oracle membership additions. No product behavior, test behavior, compatibility code, glue, formatting cleanup, or remote state was authored or mutated.
Add the accepted read-side enumeration layer. FileProviderEnumeratorCore owns initial refresh, anchored deltas, working-set aggregation, and acknowledgement only after signal delivery. FileProviderPollingLoop owns immediate polling, start-to-start cadence, and invalidation that detaches sleepers and drains or cancels in-flight refresh work. This is a move-only reconstruction from immutable accepted bytes: enumerator blob 7cf2e7a, polling blob ee5fc49, and 1,332-line contract-test blob d5d5458. XcodeGen contributes only the eight accepted project-membership lines. Evidence: the F13 baseline passed 19 tests; the tests-first compile failed only on absent enumerator and polling contracts; the focused contract suite passed 23 tests; the combined contract, coordinator, and remote-service gate passed 40 tests; and the normal Remux simulator build succeeded. project.yml and both standalone regression suites remain byte-identical.
Reconstruct Task F15 from accepted immutable history on exact F14 parent 2d5796f060017c2e53b12640909992c90a3a562f. The remote service and Citadel provider are the pinned final blobs; the two test files are the pinned F15 intermediate compositions. No product behavior, tests, glue, compatibility, migration, cleanup, or formatting were authored. This layer owns one short-lived writable SFTP lease for contained item, list, create-directory, upload, rename, file removal, and empty-directory removal operations. It rejects escaping parents and symlink mutation sources, and preserves lifetime ownership so cancellation cannot complete before the registered child operation is cancelled and drained. Tests were moved first. The RED build failed only for absent withMutationAccess, FileProviderRemoteMutationAccess, and writable provider conformance. The final focused remote suite passed 17 tests, including six new mutation-session tests; the combined remote, contract, and SFTP regression gate passed 78 tests with no failures or skips; and the normal Remux simulator build succeeded. Direct boundary: production +247/-3 and tests +471/-7 across exactly four modified paths. Remux.xcodeproj/project.pbxproj and project.yml remain byte-identical to F14. The accepted F21 invalidation test and later mutation-core behavior remain deferred.
Reconstruct the accepted F16 create-only mutation core on the exact F15 parent. The core reserves opaque identity before remote creation, uses strict temporary-upload and rename sequencing, and commits authoritative snapshot state and replay receipts only at the accepted boundary. Move only immutable accepted bytes: FileProviderMutationCore.swift is exact blob f83882b; FileProviderMutationCoreTests.swift is exact composed blob 0ed2348 from natural blob 51fb63c ranges 1-228, 327-328, 337-346, and 351-514. XcodeGen 2.44.1 adds only the eight deterministic accepted-oracle source and test membership lines; project.yml is unchanged. Tests first failed only for absent FileProviderMutationCore and FileProviderCreateMutationError contracts. The accepted implementation then passed FileProviderMutationCoreTests 12/12 and the combined mutation-core, snapshot-store, and operation-coordinator gate 41/41. A normal Remux iPhone 17 iOS 26.5 simulator build also succeeded. The accepted suite covers replay, collision and symlink rejection, upload/rename cleanup, progress, cancellation before rename, and authoritative commit after rename.
Reconstruct the accepted F17 rename/move boundary as exact immutable-history bytes. The mutation core is the accepted 339-line a699e5a blob, with metadata-only modify handling that preserves opaque identity, relocates known descendants, refreshes both affected parents, partitions pending fields, validates base versions and destinations, and records replay receipts across cancellation. The tests are the accepted 783-line cc45a1e blob composed from 4a3206f ranges 1-228, 317-629, 638-664, and 669-883. The five replicated-core tests remain deferred to F21; content replacement and deletion remain deferred to F18 and F19. Evidence: clean F16 baseline passed 12/12; tests-first compilation failed only on the absent F17 modify contracts and contextual cascades; focused F17 passed 26/26; mutation plus snapshot and coordinator regressions passed 55/55; and the normal Remux simulator build succeeded. Direct deltas are source +156/-2 and tests +384/-5. project.yml and the PBX remain byte-identical.
Reconstruct the accepted F18 mutation slice without authored behavior, tests, glue, compatibility, migration, cleanup, or project changes. FileProviderMutationCore owns content replacement inside the existing serialized mutation path. The accepted implementation uploads through a protected temporary sibling, commits the final rename before removing the old source, preserves opaque identity, and records the authoritative version, snapshot, and replay receipt atomically. Accepted failure paths remove the exact temporary upload while preserving the prior destination; cancellation before commit cleans up without recording a receipt, while cancellation after the commit boundary returns authoritative state. Moved exact immutable-history bytes: source blob 11b527e (408 lines, +76/-7), and test blob b646f74 (1,117 lines, 39 tests, +338/-4), composed from natural blob 92ded1f ranges 1-253, 342-939, 948-974, and 979-1217. project.yml and the PBX project remain parent-identical. Evidence: the clean F17 mutation baseline passed 26/26. Moving tests first produced direct missing-F18 failures and then waited at the absent upload callback, proving the cancellation contract was not yet implemented. With the accepted source present, the focused mutation suite passed 39/39, the combined mutation/snapshot/coordinator gate passed 68/68, the normal Remux Debug iPhone 17 simulator build succeeded, and git diff --check was clean.
Reconstruct the accepted F19 deletion boundary as a move-only commit on the exact F18 parent. The production and test files are byte-identical to their pinned immutable-history blobs; no product or test behavior was authored, adapted, reformatted, or reviewed anew. Project metadata remains parent-identical.\n\nThis commit adds the accepted non-recursive deletion contract: regular files and empty directories can be removed, while nonempty directories, root items, symlinks, special files, stale versions, and permission failures preserve the authoritative state. Receipt replay and cancellation after the remote commit retain their accepted behavior.\n\nVerification moved the exact 48-test slice first. Against F18 it failed only because FileProviderMutationCore.delete and FileProviderDeleteMutationError were absent. After moving the exact accepted source blob, all 48 FileProviderMutationCoreTests passed.
Complete the accepted F19 mutation-core boundary as a second move-only commit. The source and tests are byte-identical to the pinned accepted blobs composed from immutable history; no product or test behavior was authored, adapted, reformatted, or re-reviewed. Project metadata remains unchanged from the delete commit.\n\nThis commit normalizes the accepted stale-version policy across mutation paths. When fail-on-conflict is absent, stale content returns the authoritative item for fetch and stale metadata returns the authoritative item without fetch; explicit conflict behavior remains covered by the retained suite.\n\nVerification moved the exact 50-test slice first. Against the delete tip, 48 tests passed and only the two new stale-version policy tests failed by receiving conflict errors. After moving the exact final core blob, all 50 focused tests and all 79 combined mutation, snapshot-store, and operation-coordinator tests passed, and the normal Remux simulator build succeeded.
Introduce the accepted request-ownership layer as one atomic, move-only stack commit. Move the final 171-line controller blob once and extend the contract slice by the exact accepted 202-line four-test suffix; no product or test behavior was authored, adapted, refactored, or reformatted. Register FileProviderRequestController only in the main application target with the four accepted deterministic PBX membership lines. Leave project.yml byte-identical and retain the separate extension-target membership for the later extension-scaffolding branch. Verification: the untouched parent passed 23 contract tests; the tests-first run failed only at the four unresolved controller references; the accepted source then passed all 27 contract tests. A normal Remux simulator build succeeded, git diff --check was clean, and the source and test blobs matched 06945b4 and 678d58d.
|
Warning Review limit reached
Next review available in: 59 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: Pro Plus Run ID: 📒 Files selected for processing (45)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc4481ee8c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
| } | ||
| try Task.checkCancellation() | ||
| try await access.renameItem(from: temporary, to: destination) |
There was a problem hiding this comment.
When .contents changes without a rename, destination == sourcePath, so this attempts to rename the temporary upload over an existing remote file. Servers implementing ordinary SSH_FXP_RENAME reject an existing destination, causing saves of existing files to fail after upload; use an overwrite-capable extension or an explicit safe replacement sequence.
Useful? React with 👍 / 👎.
Comment on lines
+211
to
+212
| guard let canonicalTarget = try? await client.realPath(atPath: canonicalEntry), | ||
| let relativeTarget = try? safeLinkResolver.resolve( |
There was a problem hiding this comment.
When resolving a listed symlink fails because of a timeout, lost connection, or another transient SFTP error, try? treats the link as unsafe and omits it from the successful listing. The snapshot layer then records that omission as a deletion instead of letting the error mapper report the server as unreachable; only broken or out-of-home links should be filtered, while transport errors should be rethrown.
Useful? React with 👍 / 👎.
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
1 participant
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 an attempt to make the File Provider work more reviewable by presenting the existing atomic work as a small, coherent stack. If this shape or boundary does not feel right, please say so instead of spending effort reviewing an unhelpful presentation—we can reshape it.
This is a history/presentation rewrite of the already-tested implementation. It does not add new product behavior.
Intended review slice
codex/file-provider-review/rename-replace-deletecodex/file-provider-review/request-lifecycleThe 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.