feat(hub): local-folder / cloud-drive-folder carrier completes AD-1#106
Merged
Conversation
Add FolderHub, the `folder:<abs-path>` hub carrier: a plain shared directory (a Dropbox/iCloud/Drive folder or network mount) carries the same zero-knowledge object set as R2 and the git carrier. It composes the proven R2Hub semantics over the existing fsObjectStore rooted directly in the shared folder, so there is no fetch/commit/push loop — the drive is the replication transport and each Hub method is lock -> delegate -> unlock. The cross-process lock (in-process mutex + O_EXCL lock file + heartbeat + stale-TTL break) is extracted from GitCarrierHub into a shared package- local `fsLock` helper used by both carriers (pure refactor; the git side is behaviorally unchanged). The lock file and per-clone observation floor live in the local home cache under ~/.devstrap/hub-folder/<hash>/, never in the replicated folder; only ciphertext objects and their timestamp sidecars live in the shared folder. Cross-device conditional writes are best-effort by nature (a cloud drive has no cross-writer linearization point), documented as an advisory-cooperation residual. CLI: selectBackendHub/hubConfigured gain a `folder:` case (hub id `folder:<workspace_id>`), doctor's isRemoteHubID classifies it as remote, and `hub init` stays git-only. Full conformance + constructor-rejection + cross-process-CAS + two-device tests, a folder:-scheme CLI test, and a sync_folder_hub.txtar e2e. Specs 01/03/13/14/15/16/18 updated; AD-1 is now complete. docs/audits/README.md unchanged (AD-1 is a spec/14 direction item; ledger counts unaffected). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds a new ChangesFolder hub carrier
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/cli/hub.go (1)
168-185: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winNo guard against the shared
folder:path overlapping the local cache root.
cacheRoot(Home/hub-folder) is whereNewFolderHubstores the per-device lock andobserved.json— deliberately kept out of the shared folder per the design rationale (replicating lock churn would cause false contention/conflicted copies). Nothing here (or in theNewFolderHubconstructor shown in context) rejects a user-suppliedfolder:<path>that is the same as, or an ancestor/descendant of,cacheRoot/opts.paths().Home. Misconfiguring the shared path to point inside the local devstrap home would violate that invariant and could leak per-device lock/observation state into the shared carrier.Consider validating that the resolved
folder:directory does not overlap withcacheRoot(or the devstrap home) before constructing the hub.Suggested guard
cacheRoot := filepath.Join(filepath.Dir(opts.paths().KeyDir()), "hub-folder") + if rel, err := filepath.Rel(cacheRoot, path); err == nil && !strings.HasPrefix(rel, "..") { + return nil, "", fmt.Errorf("folder hub path %q must not be inside the devstrap cache directory %q", path, cacheRoot) + } folderHub, err := hub.NewFolderHub(path, ws, cacheRoot)🤖 Prompt for 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. In `@internal/cli/hub.go` around lines 168 - 185, The folder hub setup in the folder: branch of hub.go does not prevent the shared path from overlapping the local cache area used by NewFolderHub. Add a validation step after parseFolderURI and before hub.NewFolderHub that resolves the requested folder path and rejects it if it is the same as, or nested with, cacheRoot or opts.paths().Home, returning a clear error. Use the existing folder: handling branch, cacheRoot computation, and NewFolderHub call as the key locations to update.
🤖 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 `@spec/03_SYSTEM_ARCHITECTURE.md`:
- Around line 372-374: Clarify the root-validation behavior in FolderHub: the
current wording implies the folder root is only resolved once, but the
implementation re-validates the root path under the cross-process lock on each
operation. Update the AD-1 folder-carrier description to explicitly state that
the resolved root is checked again during each Hub method call, while keeping
the one-time EvalSymlinks resolution and constructor validation contract tied to
FolderHub and fsLock.
---
Nitpick comments:
In `@internal/cli/hub.go`:
- Around line 168-185: The folder hub setup in the folder: branch of hub.go does
not prevent the shared path from overlapping the local cache area used by
NewFolderHub. Add a validation step after parseFolderURI and before
hub.NewFolderHub that resolves the requested folder path and rejects it if it is
the same as, or nested with, cacheRoot or opts.paths().Home, returning a clear
error. Use the existing folder: handling branch, cacheRoot computation, and
NewFolderHub call as the key locations to update.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d564d3be-a55c-461b-b984-fc014f7e22d0
📒 Files selected for processing (16)
cmd/devstrap/testdata/script/sync_folder_hub.txtarinternal/cli/doctor.gointernal/cli/doctor_test.gointernal/cli/hub.gointernal/cli/hub_folder_test.gointernal/hub/folder.gointernal/hub/folder_test.gointernal/hub/gitcarrier.gointernal/hub/gitcarrier_adversarial_test.gospec/01_ARCHITECTURE_DECISION.mdspec/03_SYSTEM_ARCHITECTURE.mdspec/13_CLI_DAEMON_API.mdspec/14_MVP_ROADMAP_AND_BACKLOG.mdspec/15_SECURITY_THREAT_MODEL.mdspec/16_TEST_PLAN.mdspec/18_WORK_LOG.md
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.
Summary
folder:<abs-path>hub carrier (internal/hub/folder.go):FolderHubcomposes the provenR2Hubsemantics over the existingfsObjectStorerooted directly in a shared directory — a cloud-drive folder or network mount is the replication transport, so everydssync.Hubop is lock → delegate → unlock (no fetch/commit/push loop). This is the last AD-1 slice: the zero-infrastructure hub backend direction is now COMPLETE.GitCarrierHubinto a sharedfsLockhelper used by both carriers — behavior-preserving refactor, verified by the existing gitcarrier suite and an independent Codex line-level check.~/.devstrap/hub-folder/<hash>/), never in the replicated folder — replicating lock churn would cause false contention and conflicted copies. Cross-device CAS is best-effort by design (no cross-writer linearization point on a cloud drive); documented in spec/15 alongside the existing sweep-lock residuals.folder:case inselectBackendHub/hubConfigured,parseFolderURIvalidation (absolute path, no?params),doctorworkspace-id-mismatch warning parity.Review round (dual review)
guard()now re-resolves the root under the lock (use-time root revalidation) and refuses when it no longer denotes the construction-time directory —safePathonly Lstats components below the root, so a shared root later swapped for a symlink would otherwise redirect every read/write outside the registered folder. Pinned byTestFolderHubRefusesReplacedRoot.TestFolderHubWorkspacesAreIsolatedpins two workspace ids sharing one folder (isolation rides theworkspaces/<ws>/key prefix).Test plan
TestFolderHubConformancereuses the full shared hub contract (round-trip, ack plane, retention/snapshot, sweep lock) the git carrier is proven against.sync_folder_hub.txtare2e: two device homes share one folder hub — founder seeds, joiner enrolls, converge + materialize.go test -race ./...all green post-rebase.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests