mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
Compare commits
7 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 2c30922e65 | |||
| e8cda55ddb | |||
| c850450ddf | |||
| ad1f8621f1 | |||
| a7937d4066 | |||
| 9dbe313193 | |||
| e23363606a |
@@ -0,0 +1,62 @@
|
||||
---
|
||||
name: basic-machines-review
|
||||
description: Use when reviewing Basic Machines code for house style, architecture risk, pre-merge hardening, or whether a change fits basic-memory/basic-memory-cloud conventions.
|
||||
license: MIT
|
||||
---
|
||||
|
||||
# Basic Machines Review
|
||||
|
||||
Use this skill for repo-local review passes where ordinary code review needs Basic Machines
|
||||
house style and architecture judgment. Report findings only; do not edit code unless the user
|
||||
asks you to fix specific findings.
|
||||
|
||||
## Scope
|
||||
|
||||
Review the current diff or named files against:
|
||||
|
||||
- The repo's `AGENTS.md` / `CLAUDE.md`
|
||||
- `docs/ENGINEERING_STYLE.md`
|
||||
- The touched code paths and tests
|
||||
|
||||
For `basic-memory`, prioritize local-first file/database/MCP boundaries. For
|
||||
`basic-memory-cloud`, prioritize tenant/workspace isolation, cloud worker behavior, and
|
||||
web-v2 state/runtime boundaries.
|
||||
|
||||
## Review Rubric
|
||||
|
||||
Report only concrete, falsifiable risks:
|
||||
|
||||
- **Cognitive load:** Is the change harder to understand than the problem requires?
|
||||
- **Change propagation:** Will one product change force edits across unrelated layers?
|
||||
- **Knowledge duplication:** Is the same rule encoded in multiple places that can drift?
|
||||
- **Accidental complexity:** Did the change add abstractions, fallbacks, or state without need?
|
||||
- **Dependency direction:** Are API/MCP/CLI, services, repositories, and UI stores respecting
|
||||
their intended boundaries?
|
||||
- **Domain model distortion:** Do names and types still match the product concept, or did a
|
||||
transport/storage detail leak into the domain?
|
||||
- **Test oracle quality:** Would the tests fail for the bug or regression the change claims to
|
||||
protect against?
|
||||
|
||||
## House Rules To Check Explicitly
|
||||
|
||||
- No speculative `getattr(obj, "attr", default)` for unknown model shapes.
|
||||
- No broad exception swallowing, warning-only failure paths, or hidden fallback behavior.
|
||||
- No casts or `Any` that hide an unclear type relationship.
|
||||
- Dataclasses for internal value/result objects; Pydantic at validation/serialization
|
||||
boundaries.
|
||||
- Narrow `Protocol`s when only a capability is needed.
|
||||
- Explicit async/resource ownership, cancellation, and cleanup.
|
||||
- Meaningful regression tests or verification for risky changes.
|
||||
- Comments explain why, not what.
|
||||
|
||||
## Reporting Format
|
||||
|
||||
Lead with findings ordered by severity. Each finding should include:
|
||||
|
||||
```text
|
||||
severity | file:line | risk category | claim
|
||||
Why: concrete behavior or code path that proves the risk.
|
||||
Fix: smallest practical change, or "none obvious" if the risk needs product input.
|
||||
```
|
||||
|
||||
If there are no findings, say so and note any verification gaps that remain.
|
||||
+1
@@ -0,0 +1 @@
|
||||
../../.agents/skills/basic-machines-review
|
||||
@@ -108,6 +108,27 @@ Before opening or updating a PR, run the checks that mirror the common required
|
||||
- Follow the repository pattern for data access
|
||||
- Tools communicate to api routers via the httpx ASGI client (in process)
|
||||
|
||||
### Programming Style
|
||||
|
||||
See [docs/ENGINEERING_STYLE.md](docs/ENGINEERING_STYLE.md) for the fuller house style. The
|
||||
short version for agents:
|
||||
|
||||
- Prefer type-safe, explicit designs over object-heavy indirection. Use Python 3.12 `type`
|
||||
aliases, full annotations, and narrow `Protocol`s when a caller only needs a capability.
|
||||
- Use dataclasses for internal value objects and operation results; use Pydantic v2 at API,
|
||||
CLI, MCP, and persistence boundaries where validation and serialization matter.
|
||||
- Keep async boundaries obvious. Resource-owning code should use context managers, propagate
|
||||
cancellation, and avoid hidden background work unless the lifecycle is explicit.
|
||||
- Fail fast. Do not add silent fallback logic, broad exception swallowing, speculative
|
||||
`getattr`, or casts that hide an unclear model shape.
|
||||
- Keep control flow simple and local. Push branching decisions up, keep leaf helpers focused,
|
||||
and name values after the domain concept they carry.
|
||||
- Use evidence-first testing. Add or update meaningful regression tests for bugs and risky
|
||||
behavior, prefer real code paths over mocks, and run the narrowest command that proves the
|
||||
change before widening verification.
|
||||
- Comments should explain why a branch, invariant, or constraint exists. Avoid comments that
|
||||
merely narrate obvious code.
|
||||
|
||||
### Code Change Guidelines
|
||||
|
||||
- **Full file read before edits**: Before editing any file, read it in full first to ensure complete context; partial reads lead to corrupted edits
|
||||
@@ -348,6 +369,13 @@ See `.claude/commands/release/release.md` (and `beta.md`, `release-check.md`, `c
|
||||
- Manage snapshots: `basic-memory cloud snapshot [create|list|delete|show|browse]`
|
||||
- Restore from snapshot: `basic-memory cloud restore <path> --snapshot <id>`
|
||||
|
||||
**Cloud Sync Commands (Personal and Team workspaces):**
|
||||
- Fetch cloud changes (cloud -> local): `basic-memory cloud pull --name "name"` (Team-safe; additive, never deletes local)
|
||||
- Upload local changes (local -> cloud): `basic-memory cloud push --name "name"` (Team-safe; additive, never deletes cloud)
|
||||
- Resolve conflicts on push/pull: `--on-conflict [fail|keep-local|keep-cloud|keep-both]` (default `fail` lists conflicts and aborts, git-style)
|
||||
- One-way mirror (local -> cloud): `basic-memory cloud sync --name "name"` (Personal workspaces only; deletes cloud files missing locally)
|
||||
- Two-way mirror (local <-> cloud): `basic-memory cloud bisync --name "name"` (Personal workspaces only)
|
||||
|
||||
### MCP Capabilities
|
||||
|
||||
- Basic Memory exposes these MCP tools to LLMs:
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
# Basic Memory Engineering Style
|
||||
|
||||
Style is how we make code easier to verify. Prefer explicit, typed, local-first code that
|
||||
preserves the file system as the source of truth while keeping the database, API, and MCP
|
||||
surfaces in sync.
|
||||
|
||||
## Design Center
|
||||
|
||||
- Basic Memory is local-first. Markdown files are the durable source; SQLite/Postgres indexes
|
||||
are derived state that should be rebuilt or reconciled from files when needed.
|
||||
- Keep the existing boundary order: CLI/MCP/API entrypoints compose dependencies, services own
|
||||
business behavior, repositories own database access, and file services own filesystem writes.
|
||||
- MCP tools should remain atomic and composable. They should call API routers through typed MCP
|
||||
clients, not reach around into services.
|
||||
- Prefer small, explicit abstractions that match a real domain boundary. Avoid object
|
||||
hierarchies when a function, dataclass, type alias, or protocol describes the concept better.
|
||||
|
||||
## Types And Data
|
||||
|
||||
- Use full type annotations and Python 3.12 syntax. Introduce `type` aliases for repeated
|
||||
structured shapes, callback signatures, or domain concepts that would otherwise become
|
||||
anonymous `dict[str, Any]` values.
|
||||
- Use dataclasses for internal values, operation inputs, and service results. Prefer
|
||||
`frozen=True` when the value should not change and `slots=True` when identity/dynamic
|
||||
attributes are not needed.
|
||||
- Use Pydantic v2 at boundaries that validate, serialize, or deserialize data: API payloads,
|
||||
CLI/MCP schemas, configuration, and persistence-adjacent schemas.
|
||||
- Use narrow `Protocol`s when a caller needs a capability rather than a concrete repository or
|
||||
service. Keep protocols small enough that fake implementations in tests are obvious.
|
||||
- Avoid speculative `getattr`, broad casts, or `Any` as a way to paper over uncertainty. Read
|
||||
the model or schema definition and make the type relationship explicit.
|
||||
|
||||
## Control Flow And Resources
|
||||
|
||||
- Fail fast when an invariant is broken. Do not swallow exceptions, add warning-only error
|
||||
handling, or introduce fallback behavior unless the user explicitly agrees to that behavior.
|
||||
- Keep control flow simple and close to the domain decision. Push `if` statements up into the
|
||||
function that owns orchestration; keep leaf helpers focused on computation or one side effect.
|
||||
- Make async/resource boundaries visible with context managers and explicit lifecycles. Do not
|
||||
start background work without a clear owner, cancellation story, and verification path.
|
||||
- Keep file mutations centralized through the existing file utilities/services so checksum,
|
||||
atomic write, and index synchronization behavior stays coherent.
|
||||
|
||||
## Testing And Verification
|
||||
|
||||
- Use evidence-first testing, not mechanical TDD. For bugs and risky behavior, add or update a
|
||||
regression test that would catch the failure. For small documentation-only edits, use the
|
||||
relevant doc/repo hygiene checks.
|
||||
- Prefer tests that exercise real code paths. Use mocks, doubles, or `monkeypatch` only when
|
||||
the external boundary would be slow, nondeterministic, or impossible to trigger directly.
|
||||
- Keep coverage at 100% for new code. Use `# pragma: no cover` only for code that would require
|
||||
disproportionate mocking and is covered through an integration or runtime path.
|
||||
- Start with targeted commands, then widen as risk grows: focused pytest, `just fast-check`,
|
||||
`just doctor`, package checks for agent packaging changes, and full SQLite/Postgres gates
|
||||
when behavior crosses shared boundaries.
|
||||
|
||||
## Comments And Names
|
||||
|
||||
- Name values after the domain concept they carry: project, entity, permalink, tenant, route,
|
||||
checksum, observation, relation, batch, or index state.
|
||||
- Comments should say why a branch, invariant, retry, lifecycle, or compatibility constraint
|
||||
exists. Section headers are useful when a function or file has clear phases.
|
||||
- Avoid comments that restate the code. If a comment cannot explain a decision, simplify the
|
||||
code or improve the name instead.
|
||||
+171
-53
@@ -8,9 +8,25 @@ The cloud CLI enables you to:
|
||||
- **Authenticate cloud access** - OAuth/API key credentials are stored locally for cloud operations
|
||||
- **Project-scoped sync** - Each project independently manages its sync configuration
|
||||
- **Explicit operations** - Sync only what you want, when you want
|
||||
- **Bidirectional sync** - Keep local and cloud in sync with rclone bisync
|
||||
- **Team-safe push/pull** - Additive, git-style transfers that work on shared Team workspaces
|
||||
- **Bidirectional sync** - Keep local and cloud in sync with rclone bisync (Personal workspaces)
|
||||
- **Offline access** - Work locally, sync when ready
|
||||
|
||||
### Personal vs Team workspaces
|
||||
|
||||
The transfer commands fall into two groups:
|
||||
|
||||
| Command | Direction | Behavior | Personal | Team |
|
||||
|---|---|---|---|---|
|
||||
| `bm cloud pull` | cloud → local | **additive** — never deletes local | ✅ | ✅ |
|
||||
| `bm cloud push` | local → cloud | **additive** — never deletes cloud | ✅ | ✅ |
|
||||
| `bm cloud sync` | local → cloud | **mirror** — deletes cloud files missing locally | ✅ | ❌ |
|
||||
| `bm cloud bisync` | local ↔ cloud | **mirror** — two-way, deletes on both sides | ✅ | ❌ |
|
||||
|
||||
`sync` and `bisync` are mirror operations: one local tree becomes authoritative and files missing on the other side get deleted. That is correct for a Personal workspace (one user, one source of truth) but unsafe on a shared Team bucket, where it could delete a teammate's files. On Team workspaces these commands exit early with a clear error and point you at `push`/`pull`.
|
||||
|
||||
`push` and `pull` are additive (they use `rclone copy`, which never deletes on the destination), so they are safe on both Personal and Team workspaces.
|
||||
|
||||
## Prerequisites
|
||||
|
||||
Before using Basic Memory Cloud, you need:
|
||||
@@ -55,8 +71,8 @@ bm project add work --cloud --local-path ~/work-notes
|
||||
bm project add temp --cloud # No local sync
|
||||
|
||||
# Now you can sync individually (after initial --resync):
|
||||
bm project bisync --name research
|
||||
bm project bisync --name work
|
||||
bm cloud bisync --name research
|
||||
bm cloud bisync --name work
|
||||
# temp stays cloud-only
|
||||
```
|
||||
|
||||
@@ -137,10 +153,10 @@ Establish the initial sync baseline. **Best practice:** Always preview with `--d
|
||||
|
||||
```bash
|
||||
# Step 1: Preview the initial sync (recommended)
|
||||
bm project bisync --name research --resync --dry-run
|
||||
bm cloud bisync --name research --resync --dry-run
|
||||
|
||||
# Step 2: If all looks good, run the actual sync
|
||||
bm project bisync --name research --resync
|
||||
bm cloud bisync --name research --resync
|
||||
```
|
||||
|
||||
**What happens under the covers:**
|
||||
@@ -167,7 +183,7 @@ This will effectively make both Path1 and Path2 filesystems contain a matching s
|
||||
After the first sync, just run bisync without `--resync`:
|
||||
|
||||
```bash
|
||||
bm project bisync --name research
|
||||
bm cloud bisync --name research
|
||||
```
|
||||
|
||||
**What happens:**
|
||||
@@ -235,7 +251,7 @@ bm project add research --cloud --local-path ~/Documents/research
|
||||
- Stores sync config in `~/.basic-memory/config.json`
|
||||
- Prepares for bisync (but doesn't sync yet)
|
||||
|
||||
**Result:** Project ready to sync. Run `bm project bisync --name research --resync` to establish baseline.
|
||||
**Result:** Project ready to sync. Run `bm cloud bisync --name research --resync` to establish baseline.
|
||||
|
||||
**Use case 3: Add sync to existing cloud project**
|
||||
|
||||
@@ -294,18 +310,98 @@ For MCP stdio, routing is always local.
|
||||
|
||||
### Understanding the Sync Commands
|
||||
|
||||
**There are three sync-related commands:**
|
||||
**There are five sync-related commands:**
|
||||
|
||||
1. `bm project sync` - One-way: local → cloud (make cloud match local)
|
||||
2. `bm project bisync` - Two-way: local ↔ cloud (recommended)
|
||||
3. `bm project check` - Verify files match (no changes)
|
||||
| Command | Direction | Workspace | Summary |
|
||||
|---|---|---|---|
|
||||
| `bm cloud pull` | cloud → local | Personal + Team | Fetch cloud changes, additively (git-style) |
|
||||
| `bm cloud push` | local → cloud | Personal + Team | Upload local changes, additively (git-style) |
|
||||
| `bm cloud sync` | local → cloud | Personal only | One-way mirror (cloud becomes identical to local) |
|
||||
| `bm cloud bisync` | local ↔ cloud | Personal only | Two-way mirror (recommended for solo use) |
|
||||
| `bm cloud check` | — | Personal + Team | Verify files match (no changes) |
|
||||
|
||||
### One-Way Sync: Local → Cloud
|
||||
If you collaborate on a shared Team workspace, use **`push`/`pull`** (see [Team Workspaces](#team-workspaces-push--pull-additive-git-style)). If you are the only writer (a Personal workspace), the mirror commands `sync`/`bisync` give you a single source of truth.
|
||||
|
||||
### Team Workspaces: push / pull (additive, git-style)
|
||||
|
||||
`push` and `pull` are the Team-safe transfer commands. They model `git push` / `git pull`:
|
||||
|
||||
- **`bm cloud pull`** fetches changes from the cloud into your local directory.
|
||||
- **`bm cloud push`** uploads your local changes to the cloud.
|
||||
|
||||
Both use `rclone copy`, so they are **additive — they never delete on the destination**. A conflict (a file that differs on both sides) is never resolved silently: by default the command aborts and lists the conflicting files, exactly like git refusing to clobber your changes.
|
||||
|
||||
#### Pull: fetch cloud changes
|
||||
|
||||
```bash
|
||||
# Preview first (recommended)
|
||||
bm cloud pull --name research --dry-run
|
||||
|
||||
# Fetch new/changed cloud files into local
|
||||
bm cloud pull --name research
|
||||
```
|
||||
|
||||
**What happens:**
|
||||
1. Compares cloud and local with `rclone check`
|
||||
2. Downloads files that are new or changed on the cloud
|
||||
3. Leaves your local-only files untouched (never deletes local)
|
||||
4. If any file differs on both sides, aborts and lists the conflicts (unless you pass `--on-conflict`)
|
||||
|
||||
#### Push: upload local changes
|
||||
|
||||
```bash
|
||||
bm cloud push --name research --dry-run
|
||||
bm cloud push --name research
|
||||
```
|
||||
|
||||
**What happens:**
|
||||
1. Compares local and cloud with `rclone check`
|
||||
2. Uploads files that are new or changed locally
|
||||
3. Leaves cloud-only files untouched (never deletes cloud)
|
||||
4. If any file differs on both sides, aborts and lists the conflicts — pull first, like a rejected `git push`
|
||||
|
||||
#### Resolving conflicts
|
||||
|
||||
When `push`/`pull` reports conflicts, re-run with `--on-conflict` to choose how differing files are handled. The value names exactly what survives, so it reads the same in both directions:
|
||||
|
||||
| `--on-conflict` | Behavior |
|
||||
|---|---|
|
||||
| `fail` *(default)* | List the conflicting files and exit without transferring anything |
|
||||
| `keep-cloud` | Take the cloud version (pull: overwrite local; push: skip those files) |
|
||||
| `keep-local` | Keep the local version (pull: skip those files; push: overwrite cloud) |
|
||||
| `keep-both` | Keep both — write the incoming version beside the existing one as `name.conflict-<date>.md` |
|
||||
|
||||
```bash
|
||||
# A teammate edited notes you also changed locally — pull reports a conflict:
|
||||
bm cloud pull --name research
|
||||
# pull aborted: 1 file(s) differ between local and cloud.
|
||||
# * notes/decisions.md
|
||||
# Re-run with one of: --on-conflict keep-cloud | keep-local | keep-both
|
||||
|
||||
# Take the cloud copy:
|
||||
bm cloud pull --name research --on-conflict keep-cloud
|
||||
|
||||
# Or keep both versions to merge by hand:
|
||||
bm cloud pull --name research --on-conflict keep-both
|
||||
```
|
||||
|
||||
#### Limitations
|
||||
|
||||
`push`/`pull` are deliberately simple, conflict-aware byte transfers — not a full reconciler. Without a sync baseline:
|
||||
|
||||
- **Deletions are not propagated.** A note deleted on one side is not removed from the other (we cannot tell an intentional delete from a file the other side never had). This is surfaced in the command output.
|
||||
- **Every divergence is treated as a conflict.** We cannot tell a teammate's edit from your stale copy, so any differing file prompts a decision rather than auto-resolving.
|
||||
|
||||
For conflict-aware *editing*, write through the MCP/API tools (which merge at the note level). A Team-safe bidirectional reconciler with a real baseline is tracked in [issue #862](https://github.com/basicmachines-co/basic-memory/issues/862).
|
||||
|
||||
### One-Way Sync: Local → Cloud (Personal only)
|
||||
|
||||
**Use case:** You made changes locally and want to push to cloud (overwrite cloud).
|
||||
|
||||
> **Personal workspaces only.** `sync` is a destructive mirror — it deletes cloud files that are not present locally. On a Team workspace it would delete a teammate's files, so it is blocked there. Use `bm cloud push` (additive) on Team workspaces.
|
||||
|
||||
```bash
|
||||
bm project sync --name research
|
||||
bm cloud sync --name research
|
||||
```
|
||||
|
||||
**What happens:**
|
||||
@@ -321,16 +417,18 @@ bm project sync --name research
|
||||
- You want to force cloud to match local
|
||||
- You don't care about cloud changes
|
||||
|
||||
### Two-Way Sync: Local ↔ Cloud (Recommended)
|
||||
### Two-Way Sync: Local ↔ Cloud (Personal only, recommended for solo use)
|
||||
|
||||
**Use case:** You edit files both locally and in cloud UI, want both to stay in sync.
|
||||
|
||||
> **Personal workspaces only.** `bisync` is a two-way mirror that can delete and overwrite on both sides. It is blocked on Team workspaces — use `bm cloud pull` then `bm cloud push` there. A Team-safe bidirectional reconciler is tracked separately ([issue #862](https://github.com/basicmachines-co/basic-memory/issues/862)).
|
||||
|
||||
```bash
|
||||
# First time - establish baseline
|
||||
bm project bisync --name research --resync
|
||||
bm cloud bisync --name research --resync
|
||||
|
||||
# Subsequent syncs
|
||||
bm project bisync --name research
|
||||
bm cloud bisync --name research
|
||||
```
|
||||
|
||||
**What happens:**
|
||||
@@ -349,7 +447,7 @@ echo "Local change" > ~/Documents/research/notes.md
|
||||
# Cloud now has: "Cloud change"
|
||||
|
||||
# Run bisync
|
||||
bm project bisync --name research
|
||||
bm cloud bisync --name research
|
||||
|
||||
# Result: Newer file wins (based on modification time)
|
||||
# If cloud was more recent, cloud version kept
|
||||
@@ -366,7 +464,7 @@ bm project bisync --name research
|
||||
**Use case:** Check if local and cloud match without making changes.
|
||||
|
||||
```bash
|
||||
bm project check --name research
|
||||
bm cloud check --name research
|
||||
```
|
||||
|
||||
**What happens:**
|
||||
@@ -378,7 +476,7 @@ bm project check --name research
|
||||
|
||||
```bash
|
||||
# One-way check (faster)
|
||||
bm project check --name research --one-way
|
||||
bm cloud check --name research --one-way
|
||||
```
|
||||
|
||||
### Preview Changes (Dry Run)
|
||||
@@ -386,7 +484,7 @@ bm project check --name research --one-way
|
||||
**Use case:** See what would change without actually syncing.
|
||||
|
||||
```bash
|
||||
bm project bisync --name research --dry-run
|
||||
bm cloud bisync --name research --dry-run
|
||||
```
|
||||
|
||||
**What happens:**
|
||||
@@ -432,20 +530,20 @@ bm project add work --cloud --local-path ~/work-notes
|
||||
bm project add personal --cloud --local-path ~/personal
|
||||
|
||||
# Establish baselines
|
||||
bm project bisync --name research --resync
|
||||
bm project bisync --name work --resync
|
||||
bm project bisync --name personal --resync
|
||||
bm cloud bisync --name research --resync
|
||||
bm cloud bisync --name work --resync
|
||||
bm cloud bisync --name personal --resync
|
||||
|
||||
# Daily workflow: sync everything
|
||||
bm project bisync --name research
|
||||
bm project bisync --name work
|
||||
bm project bisync --name personal
|
||||
bm cloud bisync --name research
|
||||
bm cloud bisync --name work
|
||||
bm cloud bisync --name personal
|
||||
```
|
||||
|
||||
**Future:** `--all` flag will sync all configured projects:
|
||||
|
||||
```bash
|
||||
bm project bisync --all # Coming soon
|
||||
bm cloud bisync --all # Coming soon
|
||||
```
|
||||
|
||||
### Mixed Usage
|
||||
@@ -462,8 +560,8 @@ bm project add archive --cloud
|
||||
bm project add temp-notes --cloud
|
||||
|
||||
# Sync only the configured ones
|
||||
bm project bisync --name research
|
||||
bm project bisync --name work
|
||||
bm cloud bisync --name research
|
||||
bm cloud bisync --name work
|
||||
|
||||
# Archive and temp-notes stay cloud-only
|
||||
```
|
||||
@@ -661,7 +759,7 @@ code ~/.basic-memory/.bmignore
|
||||
echo "*.tmp" >> ~/.basic-memory/.bmignore
|
||||
|
||||
# Next sync uses updated patterns
|
||||
bm project bisync --name research
|
||||
bm cloud bisync --name research
|
||||
```
|
||||
|
||||
## Troubleshooting
|
||||
@@ -724,7 +822,7 @@ bm cloud login
|
||||
**Solution:**
|
||||
|
||||
```bash
|
||||
bm project bisync --name research --resync
|
||||
bm cloud bisync --name research --resync
|
||||
```
|
||||
|
||||
**What this does:**
|
||||
@@ -747,7 +845,7 @@ bm project bisync --name research --resync
|
||||
echo "# Research Notes" > ~/Documents/research/README.md
|
||||
|
||||
# Now run bisync
|
||||
bm project bisync --name research --resync
|
||||
bm cloud bisync --name research --resync
|
||||
```
|
||||
|
||||
**Why this happens:** Bisync creates listing files that track the state of each side. When both directories are completely empty, these listing files are considered invalid by rclone.
|
||||
@@ -764,10 +862,10 @@ bm project bisync --name research --resync
|
||||
|
||||
```bash
|
||||
# Clear bisync state
|
||||
bm project bisync-reset research
|
||||
bm cloud bisync-reset research
|
||||
|
||||
# Re-establish baseline
|
||||
bm project bisync --name research --resync
|
||||
bm cloud bisync --name research --resync
|
||||
```
|
||||
|
||||
**What this does:**
|
||||
@@ -787,16 +885,16 @@ bm project bisync --name research --resync
|
||||
|
||||
```bash
|
||||
# Check what would be deleted
|
||||
bm project bisync --name research --dry-run
|
||||
bm cloud bisync --name research --dry-run
|
||||
|
||||
# If correct, establish new baseline
|
||||
bm project bisync --name research --resync
|
||||
bm cloud bisync --name research --resync
|
||||
```
|
||||
|
||||
**Solution 2:** Use one-way sync if you know local is correct:
|
||||
|
||||
```bash
|
||||
bm project sync --name research
|
||||
bm cloud sync --name research
|
||||
```
|
||||
|
||||
### Project Not Configured for Sync
|
||||
@@ -809,7 +907,7 @@ bm project sync --name research
|
||||
|
||||
```bash
|
||||
bm cloud sync-setup research ~/Documents/research
|
||||
bm project bisync --name research --resync
|
||||
bm cloud bisync --name research --resync
|
||||
```
|
||||
|
||||
### Connection Issues
|
||||
@@ -880,20 +978,30 @@ bm project set-local <name> # Revert project to local mode
|
||||
### File Synchronization
|
||||
|
||||
```bash
|
||||
# One-way sync (local → cloud)
|
||||
bm project sync --name <project>
|
||||
bm project sync --name <project> --dry-run
|
||||
bm project sync --name <project> --verbose
|
||||
# Pull: fetch cloud changes (cloud → local) - Personal + Team, additive
|
||||
bm cloud pull --name <project>
|
||||
bm cloud pull --name <project> --dry-run
|
||||
bm cloud pull --name <project> --on-conflict [fail|keep-local|keep-cloud|keep-both]
|
||||
|
||||
# Two-way sync (local ↔ cloud) - Recommended
|
||||
bm project bisync --name <project> # After first --resync
|
||||
bm project bisync --name <project> --resync # First time / force baseline
|
||||
bm project bisync --name <project> --dry-run
|
||||
bm project bisync --name <project> --verbose
|
||||
# Push: upload local changes (local → cloud) - Personal + Team, additive
|
||||
bm cloud push --name <project>
|
||||
bm cloud push --name <project> --dry-run
|
||||
bm cloud push --name <project> --on-conflict [fail|keep-local|keep-cloud|keep-both]
|
||||
|
||||
# One-way mirror (local → cloud) - Personal workspaces only
|
||||
bm cloud sync --name <project>
|
||||
bm cloud sync --name <project> --dry-run
|
||||
bm cloud sync --name <project> --verbose
|
||||
|
||||
# Two-way mirror (local ↔ cloud) - Personal workspaces only
|
||||
bm cloud bisync --name <project> # After first --resync
|
||||
bm cloud bisync --name <project> --resync # First time / force baseline
|
||||
bm cloud bisync --name <project> --dry-run
|
||||
bm cloud bisync --name <project> --verbose
|
||||
|
||||
# Integrity check
|
||||
bm project check --name <project>
|
||||
bm project check --name <project> --one-way
|
||||
bm cloud check --name <project>
|
||||
bm cloud check --name <project> --one-way
|
||||
|
||||
# List project files by route
|
||||
bm project ls --name <project> # Default target: local
|
||||
@@ -909,15 +1017,25 @@ bm project ls --name <project> --cloud --path <subpath>
|
||||
1. **Authenticate cloud access** - `bm cloud login`
|
||||
2. **Install rclone** - `bm cloud setup`
|
||||
3. **Add projects with sync** - `bm project add research --cloud --local-path ~/Documents/research`
|
||||
4. **Preview first sync** - `bm project bisync --name research --resync --dry-run`
|
||||
5. **Establish baseline** - `bm project bisync --name research --resync`
|
||||
6. **Daily workflow** - `bm project bisync --name research`
|
||||
|
||||
**Personal workspace (solo, mirror) workflow:**
|
||||
|
||||
4. **Preview first sync** - `bm cloud bisync --name research --resync --dry-run`
|
||||
5. **Establish baseline** - `bm cloud bisync --name research --resync`
|
||||
6. **Daily workflow** - `bm cloud bisync --name research`
|
||||
|
||||
**Team workspace (shared, additive) workflow:**
|
||||
|
||||
4. **Fetch teammates' changes** - `bm cloud pull --name research`
|
||||
5. **Upload your changes** - `bm cloud push --name research`
|
||||
6. **Resolve conflicts explicitly** - re-run with `--on-conflict keep-cloud|keep-local|keep-both`
|
||||
|
||||
**Key benefits:**
|
||||
- ✅ Each project independently syncs (or doesn't)
|
||||
- ✅ Projects can live anywhere on disk
|
||||
- ✅ Explicit sync operations (no magic)
|
||||
- ✅ Safe by design (max delete limits, conflict resolution)
|
||||
- ✅ Team-safe push/pull that never delete on the destination
|
||||
- ✅ Safe by design (max delete limits, conflict resolution, git-style conflict aborts)
|
||||
- ✅ Full offline access (work locally, sync when ready)
|
||||
|
||||
**Future enhancements:**
|
||||
|
||||
@@ -7,6 +7,7 @@ they are cloud-specific operations.
|
||||
|
||||
import os
|
||||
from datetime import datetime
|
||||
from enum import Enum
|
||||
|
||||
import typer
|
||||
from rich.console import Console
|
||||
@@ -16,10 +17,14 @@ from basic_memory.cli.commands.cloud.bisync_commands import get_mount_info
|
||||
from basic_memory.cli.commands.cloud.rclone_commands import (
|
||||
RcloneError,
|
||||
SyncProject,
|
||||
TransferDirection,
|
||||
TransferPlan,
|
||||
get_project_bisync_state,
|
||||
project_bisync,
|
||||
project_check,
|
||||
project_diff,
|
||||
project_sync,
|
||||
project_transfer,
|
||||
)
|
||||
from basic_memory.cli.commands.command_utils import run_with_cleanup
|
||||
from basic_memory.cli.commands.routing import force_routing
|
||||
@@ -35,9 +40,34 @@ console = Console()
|
||||
|
||||
TEAM_WORKSPACE_BISYNC_UNSUPPORTED = (
|
||||
"The bisync operation is only supported on Personal workspaces.\n"
|
||||
"Use `bm cloud sync --name {name}` instead."
|
||||
"Use `bm cloud pull --name {name}` / `bm cloud push --name {name}` instead."
|
||||
)
|
||||
|
||||
TEAM_WORKSPACE_SYNC_UNSUPPORTED = (
|
||||
"The sync operation mirrors local onto the shared bucket and can delete a "
|
||||
"teammate's files, so it is only supported on Personal workspaces.\n"
|
||||
"Use `bm cloud pull --name {name}` (fetch) / `bm cloud push --name {name}` "
|
||||
"(additive upload) instead."
|
||||
)
|
||||
|
||||
|
||||
class ConflictStrategy(str, Enum):
|
||||
"""How push/pull resolves files that differ on both sides.
|
||||
|
||||
Default is ``fail``: surface the conflicts and abort before transferring,
|
||||
leaving the user to re-run with an explicit resolution — like git refusing
|
||||
to clobber local changes.
|
||||
|
||||
This is the Typer-facing enum; the engine in ``rclone_commands`` accepts the
|
||||
same values as a ``ConflictStrategy`` Literal. ``_run_directional_transfer``
|
||||
bridges the two by passing ``on_conflict.value``. Keep the values in sync.
|
||||
"""
|
||||
|
||||
fail = "fail"
|
||||
keep_local = "keep-local"
|
||||
keep_cloud = "keep-cloud"
|
||||
keep_both = "keep-both"
|
||||
|
||||
|
||||
# --- Shared helpers ---
|
||||
|
||||
@@ -92,8 +122,18 @@ async def _get_workspace_for_project(name: str, config: BasicMemoryConfig) -> Wo
|
||||
)
|
||||
|
||||
|
||||
def _require_personal_workspace(name: str, config: BasicMemoryConfig) -> WorkspaceInfo:
|
||||
"""Exit before bisync work when the target workspace is not personal."""
|
||||
def _require_personal_workspace(
|
||||
name: str,
|
||||
config: BasicMemoryConfig,
|
||||
*,
|
||||
unsupported_message: str = TEAM_WORKSPACE_BISYNC_UNSUPPORTED,
|
||||
) -> WorkspaceInfo:
|
||||
"""Exit before mirror work when the target workspace is not personal.
|
||||
|
||||
Used to gate the destructive mirror operations (`sync`, `bisync`) to
|
||||
Personal workspaces. ``unsupported_message`` lets each command point Team
|
||||
users at the right Team-safe alternative.
|
||||
"""
|
||||
try:
|
||||
workspace = run_with_cleanup(_get_workspace_for_project(name, config))
|
||||
except Exception as exc:
|
||||
@@ -101,7 +141,7 @@ def _require_personal_workspace(name: str, config: BasicMemoryConfig) -> Workspa
|
||||
raise typer.Exit(1)
|
||||
|
||||
if workspace.workspace_type != "personal":
|
||||
console.print(f"[red]{TEAM_WORKSPACE_BISYNC_UNSUPPORTED.format(name=name)}[/red]")
|
||||
console.print(f"[red]{unsupported_message.format(name=name)}[/red]")
|
||||
raise typer.Exit(1)
|
||||
|
||||
return workspace
|
||||
@@ -150,7 +190,11 @@ def sync_project_command(
|
||||
dry_run: bool = typer.Option(False, "--dry-run", help="Preview changes without syncing"),
|
||||
verbose: bool = typer.Option(False, "--verbose", "-v", help="Show detailed output"),
|
||||
) -> None:
|
||||
"""One-way sync: local -> cloud (make cloud identical to local).
|
||||
"""One-way mirror: local -> cloud (make cloud identical to local).
|
||||
|
||||
Personal workspaces only. This deletes cloud files not present locally, so
|
||||
on Team workspaces use `bm cloud push` (additive upload) / `bm cloud pull`
|
||||
(fetch) instead.
|
||||
|
||||
Example:
|
||||
bm cloud sync --name research
|
||||
@@ -158,6 +202,7 @@ def sync_project_command(
|
||||
"""
|
||||
config = ConfigManager().config
|
||||
_require_cloud_credentials(config)
|
||||
_require_personal_workspace(name, config, unsupported_message=TEAM_WORKSPACE_SYNC_UNSUPPORTED)
|
||||
|
||||
try:
|
||||
# Get tenant info for bucket name
|
||||
@@ -191,6 +236,172 @@ def sync_project_command(
|
||||
raise typer.Exit(1)
|
||||
|
||||
|
||||
def _print_conflict_abort(name: str, direction: TransferDirection, plan: TransferPlan) -> None:
|
||||
"""Explain a conflict abort and how to resolve it (git-pull style)."""
|
||||
console.print(
|
||||
f"[red]{direction.capitalize()} aborted: {len(plan.conflicts)} file(s) differ between "
|
||||
f"local and cloud.[/red]"
|
||||
)
|
||||
for path in plan.conflicts:
|
||||
console.print(f" [yellow]*[/yellow] {path}")
|
||||
console.print("\nRe-run with one of:")
|
||||
console.print(" [dim]--on-conflict keep-cloud[/dim] take the cloud version")
|
||||
console.print(" [dim]--on-conflict keep-local[/dim] keep your local version")
|
||||
console.print(
|
||||
" [dim]--on-conflict keep-both[/dim] keep both (writes <name>.conflict-<date>)"
|
||||
)
|
||||
|
||||
|
||||
def _run_directional_transfer(
|
||||
name: str,
|
||||
direction: TransferDirection,
|
||||
*,
|
||||
on_conflict: ConflictStrategy,
|
||||
dry_run: bool,
|
||||
verbose: bool,
|
||||
) -> None:
|
||||
"""Shared orchestration for `bm cloud push` / `bm cloud pull`.
|
||||
|
||||
Detects conflicts first, then aborts (the default) or applies the chosen
|
||||
resolution. Uses additive `rclone copy`, so it never deletes on the
|
||||
destination — safe for Team workspaces and therefore not gated.
|
||||
"""
|
||||
config = ConfigManager().config
|
||||
_require_cloud_credentials(config)
|
||||
|
||||
try:
|
||||
# Get tenant info for bucket name
|
||||
tenant_info = run_with_cleanup(get_mount_info())
|
||||
bucket_name = tenant_info.bucket_name
|
||||
|
||||
# Get project info
|
||||
with force_routing(cloud=True):
|
||||
project_data = run_with_cleanup(_get_cloud_project(name))
|
||||
if not project_data:
|
||||
console.print(f"[red]Error: Project '{name}' not found[/red]")
|
||||
raise typer.Exit(1)
|
||||
|
||||
sync_project, _ = _get_sync_project(name, config, project_data)
|
||||
|
||||
# --- Detect before transferring ---
|
||||
plan = project_diff(sync_project, bucket_name, direction)
|
||||
|
||||
# Trigger: rclone could not read/hash some files.
|
||||
# Why: comparing is the whole basis for a safe transfer — never guess.
|
||||
# Outcome: abort before moving any bytes.
|
||||
if plan.errors:
|
||||
console.print(
|
||||
f"[red]{direction.capitalize()} aborted: rclone could not compare "
|
||||
f"{len(plan.errors)} file(s)[/red]"
|
||||
)
|
||||
for path in plan.errors:
|
||||
console.print(f" [red]![/red] {path}")
|
||||
raise typer.Exit(1)
|
||||
|
||||
# Trigger: files differ on both sides and the user chose no resolution.
|
||||
# Why: "no surprises" — never silently pick a winner.
|
||||
# Outcome: list the conflicts and exit, like git refusing to clobber.
|
||||
if plan.conflicts and on_conflict is ConflictStrategy.fail:
|
||||
_print_conflict_abort(name, direction, plan)
|
||||
raise typer.Exit(1)
|
||||
|
||||
# --- Transfer ---
|
||||
arrow = "cloud -> local" if direction == "pull" else "local -> cloud"
|
||||
console.print(f"[blue]{direction.capitalize()} {name} ({arrow})...[/blue]")
|
||||
|
||||
conflict_suffix = datetime.now().strftime("%Y%m%d-%H%M%S")
|
||||
success = project_transfer(
|
||||
sync_project,
|
||||
bucket_name,
|
||||
direction,
|
||||
plan,
|
||||
strategy=on_conflict.value,
|
||||
conflict_suffix=conflict_suffix,
|
||||
dry_run=dry_run,
|
||||
verbose=verbose,
|
||||
)
|
||||
|
||||
if not success:
|
||||
console.print(f"[red]{name} {direction} failed[/red]")
|
||||
raise typer.Exit(1)
|
||||
|
||||
console.print(f"[green]{name} {direction} completed successfully[/green]")
|
||||
|
||||
# Without a sync baseline (see #862) we cannot tell an intentional delete
|
||||
# from a file the other side simply never had, so deletions never sync.
|
||||
if plan.dest_only:
|
||||
kept_on = "local" if direction == "pull" else "cloud"
|
||||
console.print(
|
||||
f"[dim]{len(plan.dest_only)} file(s) exist only on {kept_on} and were left "
|
||||
"untouched (deletions are not propagated).[/dim]"
|
||||
)
|
||||
|
||||
except RcloneError as e:
|
||||
console.print(f"[red]{direction.capitalize()} error: {e}[/red]")
|
||||
raise typer.Exit(1)
|
||||
except typer.Exit:
|
||||
# Already-handled exits (not found, conflicts, errors) propagate cleanly.
|
||||
raise
|
||||
except Exception as e:
|
||||
console.print(f"[red]Error: {e}[/red]")
|
||||
raise typer.Exit(1)
|
||||
|
||||
|
||||
@cloud_app.command("pull")
|
||||
def pull_project_command(
|
||||
name: str = typer.Option(..., "--name", "--project", help="Project name to pull"),
|
||||
on_conflict: ConflictStrategy = typer.Option(
|
||||
ConflictStrategy.fail,
|
||||
"--on-conflict",
|
||||
help="Resolve files that differ on both sides (default: fail and list them)",
|
||||
),
|
||||
dry_run: bool = typer.Option(False, "--dry-run", help="Preview changes without pulling"),
|
||||
verbose: bool = typer.Option(False, "--verbose", "-v", help="Show detailed output"),
|
||||
) -> None:
|
||||
"""Fetch cloud changes into local (cloud -> local), git-pull style.
|
||||
|
||||
Additive and Team-safe: downloads new/changed cloud files and never deletes
|
||||
local files. A file that differs on both sides is a conflict; by default
|
||||
pull aborts and lists them. Deletions are not propagated (see #862).
|
||||
|
||||
Examples:
|
||||
bm cloud pull --name research
|
||||
bm cloud pull --name research --dry-run
|
||||
bm cloud pull --name research --on-conflict keep-cloud
|
||||
"""
|
||||
_run_directional_transfer(
|
||||
name, "pull", on_conflict=on_conflict, dry_run=dry_run, verbose=verbose
|
||||
)
|
||||
|
||||
|
||||
@cloud_app.command("push")
|
||||
def push_project_command(
|
||||
name: str = typer.Option(..., "--name", "--project", help="Project name to push"),
|
||||
on_conflict: ConflictStrategy = typer.Option(
|
||||
ConflictStrategy.fail,
|
||||
"--on-conflict",
|
||||
help="Resolve files that differ on both sides (default: fail and list them)",
|
||||
),
|
||||
dry_run: bool = typer.Option(False, "--dry-run", help="Preview changes without pushing"),
|
||||
verbose: bool = typer.Option(False, "--verbose", "-v", help="Show detailed output"),
|
||||
) -> None:
|
||||
"""Upload local changes to cloud (local -> cloud), additive and Team-safe.
|
||||
|
||||
Uploads new/changed local files and never deletes cloud files. A file that
|
||||
differs on both sides is a conflict; by default push aborts and lists them
|
||||
(like git rejecting a push when the remote is ahead — pull first). Deletions
|
||||
are not propagated (see #862).
|
||||
|
||||
Examples:
|
||||
bm cloud push --name research
|
||||
bm cloud push --name research --dry-run
|
||||
bm cloud push --name research --on-conflict keep-local
|
||||
"""
|
||||
_run_directional_transfer(
|
||||
name, "push", on_conflict=on_conflict, dry_run=dry_run, verbose=verbose
|
||||
)
|
||||
|
||||
|
||||
@cloud_app.command("bisync")
|
||||
def bisync_project_command(
|
||||
name: str = typer.Option(..., "--name", "--project", help="Project name to bisync"),
|
||||
@@ -395,9 +606,15 @@ def setup_project_sync(
|
||||
|
||||
console.print(f"[green]Sync configured for project '{name}'[/green]")
|
||||
console.print(f"\nLocal sync path: {resolved_path}")
|
||||
# Lead with the Team-safe additive commands (work on any workspace); the
|
||||
# `sync`/`bisync` mirrors are Personal-workspace-only.
|
||||
console.print("\nNext steps:")
|
||||
console.print(f" 1. Preview: bm cloud sync --name {name} --dry-run")
|
||||
console.print(f" 2. Sync: bm cloud sync --name {name}")
|
||||
console.print(f" 1. Preview a pull: bm cloud pull --name {name} --dry-run")
|
||||
console.print(f" 2. Fetch from cloud: bm cloud pull --name {name}")
|
||||
console.print(f" 3. Upload local changes: bm cloud push --name {name}")
|
||||
console.print(
|
||||
f" Personal workspaces can also mirror with: bm cloud bisync --name {name} --resync"
|
||||
)
|
||||
except Exception as e:
|
||||
console.print(f"[red]Error configuring sync: {str(e)}[/red]")
|
||||
raise typer.Exit(1)
|
||||
|
||||
@@ -11,10 +11,10 @@ Replaces tenant-wide sync with project-scoped workflows.
|
||||
|
||||
import re
|
||||
import subprocess
|
||||
from dataclasses import dataclass
|
||||
from dataclasses import dataclass, field
|
||||
from functools import lru_cache
|
||||
from pathlib import Path
|
||||
from typing import Callable, Optional, Protocol
|
||||
from pathlib import Path, PurePosixPath
|
||||
from typing import Callable, Literal, Optional, Protocol
|
||||
|
||||
from loguru import logger
|
||||
from rich.console import Console
|
||||
@@ -42,6 +42,7 @@ TIGRIS_CONSISTENCY_HEADERS = [
|
||||
class RunResult(Protocol):
|
||||
returncode: int
|
||||
stdout: str
|
||||
stderr: str
|
||||
|
||||
|
||||
RunFunc = Callable[..., RunResult]
|
||||
@@ -184,6 +185,91 @@ def get_project_remote(project: SyncProject, bucket_name: str) -> str:
|
||||
return f"basic-memory-cloud:{bucket_name}/{cloud_path}"
|
||||
|
||||
|
||||
# --- Directional transfer primitives (push / pull) ---
|
||||
#
|
||||
# These power the Team-safe `bm cloud push` / `bm cloud pull` commands. Unlike
|
||||
# the mirror operations (`sync`/`bisync`), they use `rclone copy` so they never
|
||||
# delete on the destination, and conflicts are surfaced to the caller rather
|
||||
# than silently resolved. See issue #858 for the full design rationale.
|
||||
|
||||
# push = local -> cloud, pull = cloud -> local.
|
||||
TransferDirection = Literal["push", "pull"]
|
||||
|
||||
# How a directional transfer treats files that differ on both sides. "fail" is
|
||||
# the safe default: the caller is expected to abort before any transfer runs.
|
||||
ConflictStrategy = Literal["fail", "keep-local", "keep-cloud", "keep-both"]
|
||||
|
||||
|
||||
@dataclass
|
||||
class TransferPlan:
|
||||
"""Classification of how local and cloud differ for a directional transfer.
|
||||
|
||||
Built from ``rclone check --combined``. Paths are relative to the project
|
||||
root. ``conflicts`` are files present on both sides with differing content —
|
||||
without a sync baseline (see #862) every divergence is a conflict, because
|
||||
we cannot tell a teammate's edit from a stale local copy.
|
||||
"""
|
||||
|
||||
new: list[str] = field(default_factory=list) # only on source → safe to bring over
|
||||
conflicts: list[str] = field(default_factory=list) # differ on both sides
|
||||
dest_only: list[str] = field(default_factory=list) # only on destination → left untouched
|
||||
errors: list[str] = field(default_factory=list) # rclone could not read/hash
|
||||
|
||||
|
||||
def _transfer_endpoints(project: SyncProject, bucket_name: str) -> tuple[str, str]:
|
||||
"""Return (local_path, remote_path) strings for a project's transfer.
|
||||
|
||||
Raises:
|
||||
RcloneError: If the project has no local_sync_path configured.
|
||||
"""
|
||||
if not project.local_sync_path:
|
||||
raise RcloneError(f"Project {project.name} has no local_sync_path configured")
|
||||
local_path = str(Path(project.local_sync_path).expanduser())
|
||||
remote_path = get_project_remote(project, bucket_name)
|
||||
return local_path, remote_path
|
||||
|
||||
|
||||
def _build_transfer_cmd(
|
||||
operation: str,
|
||||
source: str,
|
||||
dest: str,
|
||||
*,
|
||||
filter_path: Path,
|
||||
dry_run: bool,
|
||||
verbose: bool,
|
||||
extra_flags: tuple[str, ...] = (),
|
||||
) -> list[str]:
|
||||
"""Build an rclone sync/copy command with the shared Basic Memory flags.
|
||||
|
||||
All directional transfers share the same tail: Tigris consistency headers,
|
||||
the .bmignore filter, and --local-no-preallocate (a no-op when local is the
|
||||
source, required when local is the destination on pull — see rclone#6801).
|
||||
"""
|
||||
cmd = [
|
||||
"rclone",
|
||||
operation,
|
||||
source,
|
||||
dest,
|
||||
*TIGRIS_CONSISTENCY_HEADERS,
|
||||
"--filter-from",
|
||||
str(filter_path),
|
||||
# Prevent NUL byte padding on virtual filesystems (e.g. Google Drive File Stream)
|
||||
# See: rclone/rclone#6801
|
||||
"--local-no-preallocate",
|
||||
*extra_flags,
|
||||
]
|
||||
|
||||
if verbose:
|
||||
cmd.append("--verbose")
|
||||
else:
|
||||
cmd.append("--progress")
|
||||
|
||||
if dry_run:
|
||||
cmd.append("--dry-run")
|
||||
|
||||
return cmd
|
||||
|
||||
|
||||
def project_sync(
|
||||
project: SyncProject,
|
||||
bucket_name: str,
|
||||
@@ -219,24 +305,199 @@ def project_sync(
|
||||
remote_path = get_project_remote(project, bucket_name)
|
||||
filter_path = filter_path or get_bmignore_filter_path()
|
||||
|
||||
cmd = [
|
||||
"rclone",
|
||||
cmd = _build_transfer_cmd(
|
||||
"sync",
|
||||
str(local_path),
|
||||
remote_path,
|
||||
filter_path=filter_path,
|
||||
dry_run=dry_run,
|
||||
verbose=verbose,
|
||||
)
|
||||
|
||||
result = run(cmd, text=True)
|
||||
return result.returncode == 0
|
||||
|
||||
|
||||
def _parse_check_combined(output: str) -> TransferPlan:
|
||||
"""Parse ``rclone check --combined`` output into a TransferPlan.
|
||||
|
||||
rclone emits one prefixed line per path (src is the transfer source):
|
||||
``=`` identical, ``+`` only on src, ``-`` only on dst, ``*`` differ,
|
||||
``!`` error reading/hashing. We ignore identical files.
|
||||
"""
|
||||
plan = TransferPlan()
|
||||
for line in output.splitlines():
|
||||
symbol, _, path = line.partition(" ")
|
||||
path = path.strip()
|
||||
if not path:
|
||||
continue
|
||||
if symbol == "+":
|
||||
plan.new.append(path)
|
||||
elif symbol == "*":
|
||||
plan.conflicts.append(path)
|
||||
elif symbol == "-":
|
||||
plan.dest_only.append(path)
|
||||
elif symbol == "!":
|
||||
plan.errors.append(path)
|
||||
# "=" (identical) is intentionally dropped.
|
||||
return plan
|
||||
|
||||
|
||||
def project_diff(
|
||||
project: SyncProject,
|
||||
bucket_name: str,
|
||||
direction: TransferDirection,
|
||||
*,
|
||||
run: RunFunc = subprocess.run,
|
||||
is_installed: IsInstalledFunc = is_rclone_installed,
|
||||
filter_path: Path | None = None,
|
||||
) -> TransferPlan:
|
||||
"""Classify how local and cloud differ for a push/pull, without transferring.
|
||||
|
||||
Uses ``rclone check`` (content comparison) so the caller can surface
|
||||
conflicts before any data moves. The source side depends on direction:
|
||||
pull compares cloud→local, push compares local→cloud.
|
||||
|
||||
Raises:
|
||||
RcloneError: If project has no local_sync_path configured or rclone not installed
|
||||
"""
|
||||
check_rclone_installed(is_installed=is_installed)
|
||||
|
||||
local_path, remote_path = _transfer_endpoints(project, bucket_name)
|
||||
filter_path = filter_path or get_bmignore_filter_path()
|
||||
|
||||
# Source/dest order matters: rclone check reports "+" for files only on the
|
||||
# source, which is what we want to bring over.
|
||||
source, dest = (remote_path, local_path) if direction == "pull" else (local_path, remote_path)
|
||||
|
||||
cmd = [
|
||||
"rclone",
|
||||
"check",
|
||||
source,
|
||||
dest,
|
||||
*TIGRIS_CONSISTENCY_HEADERS,
|
||||
"--filter-from",
|
||||
str(filter_path),
|
||||
# Prevent NUL byte padding on virtual filesystems (e.g. Google Drive File Stream)
|
||||
# See: rclone/rclone#6801
|
||||
"--local-no-preallocate",
|
||||
"--combined",
|
||||
"-",
|
||||
]
|
||||
|
||||
# rclone check exits non-zero when files differ — that's expected here, so we
|
||||
# parse the combined listing rather than trusting the return code.
|
||||
result = run(cmd, capture_output=True, text=True)
|
||||
plan = _parse_check_combined(result.stdout)
|
||||
|
||||
# Trigger: non-zero exit AND the combined listing produced no entries at all.
|
||||
# Why: a difference always yields +/-/*/! lines, so an empty listing on a
|
||||
# non-zero exit means the check itself failed (auth, missing remote, network,
|
||||
# bad filter) rather than finding zero differences. Without this guard the
|
||||
# caller would see an empty plan, transfer nothing, and report success.
|
||||
# Outcome: fail fast with rclone's stderr instead of a silent no-op.
|
||||
if result.returncode != 0 and not (plan.new or plan.conflicts or plan.dest_only or plan.errors):
|
||||
detail = result.stderr.strip() or f"rclone check exited with code {result.returncode}"
|
||||
raise RcloneError(f"Failed to compare {project.name} with cloud: {detail}")
|
||||
|
||||
return plan
|
||||
|
||||
|
||||
def project_copy(
|
||||
project: SyncProject,
|
||||
bucket_name: str,
|
||||
direction: TransferDirection,
|
||||
*,
|
||||
overwrite: bool,
|
||||
dry_run: bool = False,
|
||||
verbose: bool = False,
|
||||
run: RunFunc = subprocess.run,
|
||||
is_installed: IsInstalledFunc = is_rclone_installed,
|
||||
filter_path: Path | None = None,
|
||||
) -> bool:
|
||||
"""Additive transfer via ``rclone copy`` — never deletes on the destination.
|
||||
|
||||
Trigger: ``overwrite=False`` adds ``--ignore-existing`` so files already on
|
||||
the destination are left as-is (used when the destination side wins a
|
||||
conflict, and for the no-conflict fast path).
|
||||
Why: keeps the loser's bytes intact unless the caller explicitly chose to
|
||||
overwrite, matching the "no surprises" contract.
|
||||
|
||||
Raises:
|
||||
RcloneError: If project has no local_sync_path configured or rclone not installed
|
||||
"""
|
||||
check_rclone_installed(is_installed=is_installed)
|
||||
|
||||
local_path, remote_path = _transfer_endpoints(project, bucket_name)
|
||||
filter_path = filter_path or get_bmignore_filter_path()
|
||||
|
||||
source, dest = (remote_path, local_path) if direction == "pull" else (local_path, remote_path)
|
||||
# Overwrite mode compares by checksum so the transfer decision matches
|
||||
# project_diff's content-based conflict detection (rclone check). Without
|
||||
# --checksum, copy's default size+modtime comparison could skip a file the
|
||||
# diff flagged as a conflict (same size, destination not older) — silently
|
||||
# ignoring the user's explicit keep-cloud/keep-local choice. New-only mode
|
||||
# uses --ignore-existing, which skips by existence so the comparison basis
|
||||
# does not matter.
|
||||
extra_flags = ("--checksum",) if overwrite else ("--ignore-existing",)
|
||||
|
||||
cmd = _build_transfer_cmd(
|
||||
"copy",
|
||||
source,
|
||||
dest,
|
||||
filter_path=filter_path,
|
||||
dry_run=dry_run,
|
||||
verbose=verbose,
|
||||
extra_flags=extra_flags,
|
||||
)
|
||||
|
||||
result = run(cmd, text=True)
|
||||
return result.returncode == 0
|
||||
|
||||
|
||||
def _conflict_copy_name(rel_path: str, suffix: str) -> str:
|
||||
"""Insert a ``.conflict-<suffix>`` marker before the extension of a rel path."""
|
||||
p = PurePosixPath(rel_path)
|
||||
return str(p.with_name(f"{p.stem}.conflict-{suffix}{p.suffix}"))
|
||||
|
||||
|
||||
def project_copy_file(
|
||||
project: SyncProject,
|
||||
bucket_name: str,
|
||||
direction: TransferDirection,
|
||||
source_rel_path: str,
|
||||
dest_rel_path: str,
|
||||
*,
|
||||
dry_run: bool = False,
|
||||
verbose: bool = False,
|
||||
run: RunFunc = subprocess.run,
|
||||
is_installed: IsInstalledFunc = is_rclone_installed,
|
||||
) -> bool:
|
||||
"""Copy a single file from source to destination under a (possibly renamed) path.
|
||||
|
||||
Used for the ``keep-both`` strategy: the incoming version is written beside
|
||||
the destination's own copy as ``name.conflict-<date>`` so nothing is lost.
|
||||
|
||||
Raises:
|
||||
RcloneError: If project has no local_sync_path configured or rclone not installed
|
||||
"""
|
||||
check_rclone_installed(is_installed=is_installed)
|
||||
|
||||
local_path, remote_path = _transfer_endpoints(project, bucket_name)
|
||||
source_root, dest_root = (
|
||||
(remote_path, local_path) if direction == "pull" else (local_path, remote_path)
|
||||
)
|
||||
|
||||
cmd = [
|
||||
"rclone",
|
||||
"copyto",
|
||||
f"{source_root}/{source_rel_path}",
|
||||
f"{dest_root}/{dest_rel_path}",
|
||||
*TIGRIS_CONSISTENCY_HEADERS,
|
||||
# Matches _build_transfer_cmd: on pull this writes the conflict copy to
|
||||
# the local filesystem, where this prevents NUL byte padding on virtual
|
||||
# filesystems (e.g. Google Drive File Stream). See rclone/rclone#6801.
|
||||
"--local-no-preallocate",
|
||||
]
|
||||
if verbose:
|
||||
cmd.append("--verbose")
|
||||
else:
|
||||
cmd.append("--progress")
|
||||
|
||||
if dry_run:
|
||||
cmd.append("--dry-run")
|
||||
|
||||
@@ -244,6 +505,72 @@ def project_sync(
|
||||
return result.returncode == 0
|
||||
|
||||
|
||||
def _strategy_overwrites_dest(direction: TransferDirection, strategy: ConflictStrategy) -> bool:
|
||||
"""True when the strategy lets the source side overwrite the destination.
|
||||
|
||||
The source side is cloud on pull, local on push. "keep-cloud" wins on pull,
|
||||
"keep-local" wins on push; otherwise the destination is preserved.
|
||||
"""
|
||||
if strategy == "keep-cloud":
|
||||
return direction == "pull"
|
||||
if strategy == "keep-local":
|
||||
return direction == "push"
|
||||
return False # "fail" (no conflicts) and "keep-both" never overwrite existing dest files
|
||||
|
||||
|
||||
def project_transfer(
|
||||
project: SyncProject,
|
||||
bucket_name: str,
|
||||
direction: TransferDirection,
|
||||
plan: TransferPlan,
|
||||
*,
|
||||
strategy: ConflictStrategy = "fail",
|
||||
conflict_suffix: str = "",
|
||||
dry_run: bool = False,
|
||||
verbose: bool = False,
|
||||
run: RunFunc = subprocess.run,
|
||||
is_installed: IsInstalledFunc = is_rclone_installed,
|
||||
filter_path: Path | None = None,
|
||||
) -> bool:
|
||||
"""Execute a directional transfer for the chosen conflict strategy.
|
||||
|
||||
Callers detect conflicts with ``project_diff`` first and abort when
|
||||
``strategy == "fail"`` and conflicts exist; this function assumes that gate
|
||||
has already passed and applies the resolution.
|
||||
"""
|
||||
# keep-both: preserve the destination's version and drop the incoming one
|
||||
# beside it as a conflict copy, then do an additive (new-only) pass.
|
||||
if strategy == "keep-both":
|
||||
for rel_path in plan.conflicts:
|
||||
dest_rel = _conflict_copy_name(rel_path, conflict_suffix)
|
||||
copied = project_copy_file(
|
||||
project,
|
||||
bucket_name,
|
||||
direction,
|
||||
rel_path,
|
||||
dest_rel,
|
||||
dry_run=dry_run,
|
||||
verbose=verbose,
|
||||
run=run,
|
||||
is_installed=is_installed,
|
||||
)
|
||||
if not copied:
|
||||
return False
|
||||
|
||||
overwrite = _strategy_overwrites_dest(direction, strategy)
|
||||
return project_copy(
|
||||
project,
|
||||
bucket_name,
|
||||
direction,
|
||||
overwrite=overwrite,
|
||||
dry_run=dry_run,
|
||||
verbose=verbose,
|
||||
run=run,
|
||||
is_installed=is_installed,
|
||||
filter_path=filter_path,
|
||||
)
|
||||
|
||||
|
||||
def project_bisync(
|
||||
project: SyncProject,
|
||||
bucket_name: str,
|
||||
|
||||
@@ -8,6 +8,7 @@ import typer
|
||||
from typer.testing import CliRunner
|
||||
|
||||
from basic_memory.cli.app import app
|
||||
from basic_memory.cli.commands.cloud.rclone_commands import TransferPlan
|
||||
from basic_memory.config import ProjectEntry, ProjectMode
|
||||
from basic_memory.schemas.cloud import WorkspaceInfo
|
||||
|
||||
@@ -34,7 +35,7 @@ def test_cloud_sync_commands_skip_explicit_cloud_project_sync(monkeypatch, argv,
|
||||
|
||||
monkeypatch.setattr(project_sync_command, "_require_cloud_credentials", lambda _config: None)
|
||||
monkeypatch.setattr(
|
||||
project_sync_command, "_require_personal_workspace", lambda _name, _config: None
|
||||
project_sync_command, "_require_personal_workspace", lambda _name, _config, **_kwargs: None
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
project_sync_command,
|
||||
@@ -72,7 +73,7 @@ def test_cloud_bisync_fails_fast_when_sync_entry_disappears(monkeypatch, config_
|
||||
|
||||
monkeypatch.setattr(project_sync_command, "_require_cloud_credentials", lambda _config: None)
|
||||
monkeypatch.setattr(
|
||||
project_sync_command, "_require_personal_workspace", lambda _name, _config: None
|
||||
project_sync_command, "_require_personal_workspace", lambda _name, _config, **_kwargs: None
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
project_sync_command,
|
||||
@@ -141,11 +142,12 @@ def test_cloud_bisync_commands_block_organization_workspace(monkeypatch, argv, c
|
||||
assert result.exit_code == 1, result.output
|
||||
output = " ".join(result.output.split())
|
||||
assert "The bisync operation is only supported on Personal workspaces" in output
|
||||
assert "Use `bm cloud sync --name research` instead" in output
|
||||
assert "bm cloud pull --name research" in output
|
||||
assert "bm cloud push --name research" in output
|
||||
|
||||
|
||||
def test_cloud_sync_allows_organization_workspace(monkeypatch, config_manager):
|
||||
"""Team workspaces can still use the one-way sync command."""
|
||||
def test_cloud_sync_blocks_organization_workspace(monkeypatch, config_manager):
|
||||
"""The destructive mirror `sync` is now Personal-only and blocks Team workspaces."""
|
||||
project_sync_command = importlib.import_module("basic_memory.cli.commands.cloud.project_sync")
|
||||
|
||||
config = config_manager.load_config()
|
||||
@@ -161,7 +163,41 @@ def test_cloud_sync_allows_organization_workspace(monkeypatch, config_manager):
|
||||
monkeypatch.setattr(
|
||||
project_sync_command,
|
||||
"get_available_workspaces",
|
||||
lambda: pytest.fail("sync should not require a personal workspace"),
|
||||
lambda: _async_value([_workspace("team-tenant", "organization", "team")]),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
project_sync_command,
|
||||
"get_mount_info",
|
||||
lambda: pytest.fail("workspace guard should run before mount lookup"),
|
||||
)
|
||||
|
||||
result = runner.invoke(app, ["cloud", "sync", "--name", "research"])
|
||||
|
||||
assert result.exit_code == 1, result.output
|
||||
output = " ".join(result.output.split())
|
||||
assert "only supported on Personal workspaces" in output
|
||||
assert "bm cloud push --name research" in output
|
||||
assert "bm cloud pull --name research" in output
|
||||
|
||||
|
||||
def test_cloud_sync_allows_personal_workspace(monkeypatch, config_manager):
|
||||
"""Personal workspaces keep the one-way mirror sync available."""
|
||||
project_sync_command = importlib.import_module("basic_memory.cli.commands.cloud.project_sync")
|
||||
|
||||
config = config_manager.load_config()
|
||||
config.cloud_api_key = "bmc_test"
|
||||
config.projects["research"] = ProjectEntry(
|
||||
path="/tmp/research",
|
||||
mode=ProjectMode.CLOUD,
|
||||
workspace_id="personal-tenant",
|
||||
local_sync_path="/tmp/research",
|
||||
)
|
||||
config_manager.save_config(config)
|
||||
|
||||
monkeypatch.setattr(
|
||||
project_sync_command,
|
||||
"get_available_workspaces",
|
||||
lambda: _async_value([_workspace("personal-tenant", "personal", "personal")]),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
project_sync_command,
|
||||
@@ -351,6 +387,156 @@ def test_bisync_reset_skips_workspace_check_without_credentials(monkeypatch, tmp
|
||||
assert "No bisync state found for project 'research'" in result.output
|
||||
|
||||
|
||||
def _stub_transfer_env(monkeypatch, module, *, plan, transfer_result=True, recorder=None):
|
||||
"""Stub the push/pull dependency chain so only diff/transfer logic is exercised."""
|
||||
monkeypatch.setattr(module, "_require_cloud_credentials", lambda _config: None)
|
||||
monkeypatch.setattr(
|
||||
module,
|
||||
"get_mount_info",
|
||||
lambda: _async_value(SimpleNamespace(bucket_name="tenant-bucket")),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
module,
|
||||
"_get_cloud_project",
|
||||
lambda _name: _async_value(SimpleNamespace(name="research", path="research")),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
module,
|
||||
"_get_sync_project",
|
||||
lambda _name, _config, _project_data: (SimpleNamespace(name="research"), "/tmp/research"),
|
||||
)
|
||||
monkeypatch.setattr(module, "project_diff", lambda *args, **kwargs: plan)
|
||||
|
||||
def _fake_transfer(*args, **kwargs):
|
||||
if recorder is not None:
|
||||
recorder["args"] = args
|
||||
recorder["kwargs"] = kwargs
|
||||
return transfer_result
|
||||
|
||||
monkeypatch.setattr(module, "project_transfer", _fake_transfer)
|
||||
|
||||
|
||||
def test_cloud_pull_aborts_on_conflict_by_default(monkeypatch, config_manager):
|
||||
"""Pull refuses to clobber: it lists conflicts and exits without transferring."""
|
||||
module = importlib.import_module("basic_memory.cli.commands.cloud.project_sync")
|
||||
plan = TransferPlan(new=["new.md"], conflicts=["notes/dup.md"], dest_only=[], errors=[])
|
||||
recorder: dict = {}
|
||||
_stub_transfer_env(monkeypatch, module, plan=plan, recorder=recorder)
|
||||
|
||||
result = runner.invoke(app, ["cloud", "pull", "--name", "research"])
|
||||
|
||||
assert result.exit_code == 1, result.output
|
||||
output = " ".join(result.output.split())
|
||||
assert "notes/dup.md" in output
|
||||
assert "--on-conflict keep-cloud" in output
|
||||
assert "args" not in recorder # transfer never ran
|
||||
|
||||
|
||||
def test_cloud_pull_clean_transfers(monkeypatch, config_manager):
|
||||
"""With no conflicts, pull proceeds and reports success."""
|
||||
module = importlib.import_module("basic_memory.cli.commands.cloud.project_sync")
|
||||
plan = TransferPlan(new=["new.md"], conflicts=[], dest_only=["local-only.md"], errors=[])
|
||||
recorder: dict = {}
|
||||
_stub_transfer_env(monkeypatch, module, plan=plan, recorder=recorder)
|
||||
|
||||
result = runner.invoke(app, ["cloud", "pull", "--name", "research"])
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
output = " ".join(result.output.lower().split())
|
||||
assert "research pull completed successfully" in output
|
||||
# Deletions are surfaced, not propagated
|
||||
assert "deletions are not propagated" in output
|
||||
assert recorder["kwargs"]["strategy"] == "fail"
|
||||
assert recorder["args"][2] == "pull"
|
||||
|
||||
|
||||
def test_cloud_pull_keep_cloud_resolves_conflict(monkeypatch, config_manager):
|
||||
"""An explicit --on-conflict strategy lets pull proceed through conflicts."""
|
||||
module = importlib.import_module("basic_memory.cli.commands.cloud.project_sync")
|
||||
plan = TransferPlan(new=[], conflicts=["notes/dup.md"], dest_only=[], errors=[])
|
||||
recorder: dict = {}
|
||||
_stub_transfer_env(monkeypatch, module, plan=plan, recorder=recorder)
|
||||
|
||||
result = runner.invoke(
|
||||
app, ["cloud", "pull", "--name", "research", "--on-conflict", "keep-cloud"]
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert recorder["kwargs"]["strategy"] == "keep-cloud"
|
||||
|
||||
|
||||
def test_cloud_pull_aborts_on_compare_errors(monkeypatch, config_manager):
|
||||
"""If rclone cannot read/hash files, pull aborts before transferring."""
|
||||
module = importlib.import_module("basic_memory.cli.commands.cloud.project_sync")
|
||||
plan = TransferPlan(new=[], conflicts=[], dest_only=[], errors=["bad.md"])
|
||||
recorder: dict = {}
|
||||
_stub_transfer_env(monkeypatch, module, plan=plan, recorder=recorder)
|
||||
|
||||
result = runner.invoke(app, ["cloud", "pull", "--name", "research"])
|
||||
|
||||
assert result.exit_code == 1, result.output
|
||||
assert "could not compare" in result.output
|
||||
assert "args" not in recorder # transfer never ran
|
||||
|
||||
|
||||
def test_cloud_push_aborts_on_conflict_by_default(monkeypatch, config_manager):
|
||||
"""Push aborts on conflicts like a rejected git push (pull first)."""
|
||||
module = importlib.import_module("basic_memory.cli.commands.cloud.project_sync")
|
||||
plan = TransferPlan(new=["new.md"], conflicts=["notes/dup.md"], dest_only=[], errors=[])
|
||||
recorder: dict = {}
|
||||
_stub_transfer_env(monkeypatch, module, plan=plan, recorder=recorder)
|
||||
|
||||
result = runner.invoke(app, ["cloud", "push", "--name", "research"])
|
||||
|
||||
assert result.exit_code == 1, result.output
|
||||
assert "notes/dup.md" in result.output
|
||||
assert "args" not in recorder
|
||||
|
||||
|
||||
def test_cloud_push_keep_local_resolves_conflict(monkeypatch, config_manager):
|
||||
"""Push with --on-conflict keep-local overwrites cloud and reports the direction."""
|
||||
module = importlib.import_module("basic_memory.cli.commands.cloud.project_sync")
|
||||
plan = TransferPlan(new=[], conflicts=["notes/dup.md"], dest_only=[], errors=[])
|
||||
recorder: dict = {}
|
||||
_stub_transfer_env(monkeypatch, module, plan=plan, recorder=recorder)
|
||||
|
||||
result = runner.invoke(
|
||||
app, ["cloud", "push", "--name", "research", "--on-conflict", "keep-local"]
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert recorder["kwargs"]["strategy"] == "keep-local"
|
||||
assert recorder["args"][2] == "push"
|
||||
|
||||
|
||||
def test_cloud_push_allows_organization_workspace(monkeypatch, config_manager):
|
||||
"""push is additive and Team-safe — it must not invoke the Personal-only guard."""
|
||||
module = importlib.import_module("basic_memory.cli.commands.cloud.project_sync")
|
||||
|
||||
config = config_manager.load_config()
|
||||
config.cloud_api_key = "bmc_test"
|
||||
config.projects["research"] = ProjectEntry(
|
||||
path="/tmp/research",
|
||||
mode=ProjectMode.CLOUD,
|
||||
workspace_id="team-tenant",
|
||||
local_sync_path="/tmp/research",
|
||||
)
|
||||
config_manager.save_config(config)
|
||||
|
||||
plan = TransferPlan(new=["new.md"], conflicts=[], dest_only=[], errors=[])
|
||||
_stub_transfer_env(monkeypatch, module, plan=plan)
|
||||
monkeypatch.setattr(
|
||||
module,
|
||||
"get_available_workspaces",
|
||||
lambda: pytest.fail("push/pull must not gate on workspace type"),
|
||||
)
|
||||
|
||||
result = runner.invoke(app, ["cloud", "push", "--name", "research"])
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert "research push completed successfully" in result.output
|
||||
|
||||
|
||||
async def _async_value(value):
|
||||
return value
|
||||
|
||||
|
||||
@@ -10,6 +10,9 @@ from basic_memory.cli.commands.cloud.rclone_commands import (
|
||||
MIN_RCLONE_VERSION_EMPTY_DIRS,
|
||||
RcloneError,
|
||||
SyncProject,
|
||||
TransferPlan,
|
||||
_conflict_copy_name,
|
||||
_parse_check_combined,
|
||||
bisync_initialized,
|
||||
check_rclone_installed,
|
||||
get_project_bisync_state,
|
||||
@@ -17,27 +20,33 @@ from basic_memory.cli.commands.cloud.rclone_commands import (
|
||||
get_rclone_version,
|
||||
project_bisync,
|
||||
project_check,
|
||||
project_copy,
|
||||
project_copy_file,
|
||||
project_diff,
|
||||
project_ls,
|
||||
project_sync,
|
||||
project_transfer,
|
||||
supports_create_empty_src_dirs,
|
||||
)
|
||||
|
||||
|
||||
class _RunResult:
|
||||
def __init__(self, returncode: int = 0, stdout: str = ""):
|
||||
def __init__(self, returncode: int = 0, stdout: str = "", stderr: str = ""):
|
||||
self.returncode = returncode
|
||||
self.stdout = stdout
|
||||
self.stderr = stderr
|
||||
|
||||
|
||||
class _Runner:
|
||||
def __init__(self, *, returncode: int = 0, stdout: str = ""):
|
||||
def __init__(self, *, returncode: int = 0, stdout: str = "", stderr: str = ""):
|
||||
self.calls: list[tuple[list[str], dict]] = []
|
||||
self._returncode = returncode
|
||||
self._stdout = stdout
|
||||
self._stderr = stderr
|
||||
|
||||
def __call__(self, cmd: list[str], **kwargs):
|
||||
self.calls.append((cmd, kwargs))
|
||||
return _RunResult(returncode=self._returncode, stdout=self._stdout)
|
||||
return _RunResult(returncode=self._returncode, stdout=self._stdout, stderr=self._stderr)
|
||||
|
||||
|
||||
def _assert_has_consistency_headers(cmd: list[str]) -> None:
|
||||
@@ -562,3 +571,353 @@ def test_project_bisync_includes_no_preallocate_flag(tmp_path):
|
||||
|
||||
cmd, _ = runner.calls[0]
|
||||
assert "--local-no-preallocate" in cmd
|
||||
|
||||
|
||||
# --- Directional transfer primitives (push / pull, issue #858) ---
|
||||
|
||||
|
||||
def test_parse_check_combined_classifies_lines():
|
||||
output = "= same.md\n+ only-src.md\n- only-dst.md\n* differ.md\n! broken.md\n\n"
|
||||
plan = _parse_check_combined(output)
|
||||
assert plan.new == ["only-src.md"]
|
||||
assert plan.dest_only == ["only-dst.md"]
|
||||
assert plan.conflicts == ["differ.md"]
|
||||
assert plan.errors == ["broken.md"]
|
||||
|
||||
|
||||
def test_parse_check_combined_handles_paths_with_spaces():
|
||||
plan = _parse_check_combined("* notes/my file.md\n")
|
||||
assert plan.conflicts == ["notes/my file.md"]
|
||||
|
||||
|
||||
def test_conflict_copy_name_inserts_marker_before_extension():
|
||||
assert _conflict_copy_name("notes/x.md", "20260608-1030") == "notes/x.conflict-20260608-1030.md"
|
||||
assert _conflict_copy_name("top.md", "S") == "top.conflict-S.md"
|
||||
|
||||
|
||||
def test_project_diff_pull_uses_remote_as_source(tmp_path):
|
||||
runner = _Runner(returncode=1, stdout="+ a.md\n* b.md\n")
|
||||
filter_path = _write_filter_file(tmp_path)
|
||||
project = SyncProject(name="research", path="/research", local_sync_path="/tmp/research")
|
||||
|
||||
plan = project_diff(
|
||||
project,
|
||||
"my-bucket",
|
||||
"pull",
|
||||
run=runner,
|
||||
is_installed=lambda: True,
|
||||
filter_path=filter_path,
|
||||
)
|
||||
|
||||
cmd, kwargs = runner.calls[0]
|
||||
assert cmd[:2] == ["rclone", "check"]
|
||||
# pull: cloud is the source so "+" files (only on source) come down to local
|
||||
assert cmd[2] == "basic-memory-cloud:my-bucket/research"
|
||||
assert Path(cmd[3]) == Path("/tmp/research")
|
||||
assert "--combined" in cmd and cmd[cmd.index("--combined") + 1] == "-"
|
||||
_assert_has_consistency_headers(cmd)
|
||||
assert "--filter-from" in cmd
|
||||
assert kwargs["capture_output"] is True
|
||||
# rclone exits non-zero when files differ; we parse output rather than trust it
|
||||
assert plan.new == ["a.md"]
|
||||
assert plan.conflicts == ["b.md"]
|
||||
|
||||
|
||||
def test_project_diff_push_uses_local_as_source(tmp_path):
|
||||
runner = _Runner(returncode=0, stdout="")
|
||||
filter_path = _write_filter_file(tmp_path)
|
||||
project = SyncProject(name="research", path="/research", local_sync_path="/tmp/research")
|
||||
|
||||
project_diff(
|
||||
project,
|
||||
"my-bucket",
|
||||
"push",
|
||||
run=runner,
|
||||
is_installed=lambda: True,
|
||||
filter_path=filter_path,
|
||||
)
|
||||
|
||||
cmd, _ = runner.calls[0]
|
||||
assert Path(cmd[2]) == Path("/tmp/research")
|
||||
assert cmd[3] == "basic-memory-cloud:my-bucket/research"
|
||||
|
||||
|
||||
def test_project_copy_pull_new_only_adds_ignore_existing(tmp_path):
|
||||
runner = _Runner(returncode=0)
|
||||
filter_path = _write_filter_file(tmp_path)
|
||||
project = SyncProject(name="research", path="/research", local_sync_path="/tmp/research")
|
||||
|
||||
result = project_copy(
|
||||
project,
|
||||
"my-bucket",
|
||||
"pull",
|
||||
overwrite=False,
|
||||
run=runner,
|
||||
is_installed=lambda: True,
|
||||
filter_path=filter_path,
|
||||
)
|
||||
|
||||
assert result is True
|
||||
cmd, _ = runner.calls[0]
|
||||
assert cmd[:2] == ["rclone", "copy"]
|
||||
assert cmd[2] == "basic-memory-cloud:my-bucket/research"
|
||||
assert Path(cmd[3]) == Path("/tmp/research")
|
||||
assert "--ignore-existing" in cmd
|
||||
assert "--local-no-preallocate" in cmd
|
||||
# new-only skips by existence, so no content comparison is needed
|
||||
assert "--checksum" not in cmd
|
||||
|
||||
|
||||
def test_project_copy_pull_overwrite_omits_ignore_existing(tmp_path):
|
||||
runner = _Runner(returncode=0)
|
||||
filter_path = _write_filter_file(tmp_path)
|
||||
project = SyncProject(name="research", path="/research", local_sync_path="/tmp/research")
|
||||
|
||||
project_copy(
|
||||
project,
|
||||
"my-bucket",
|
||||
"pull",
|
||||
overwrite=True,
|
||||
run=runner,
|
||||
is_installed=lambda: True,
|
||||
filter_path=filter_path,
|
||||
)
|
||||
|
||||
cmd, _ = runner.calls[0]
|
||||
assert "--ignore-existing" not in cmd
|
||||
# overwrite compares by content so it matches hash-based conflict detection
|
||||
assert "--checksum" in cmd
|
||||
|
||||
|
||||
def test_project_copy_push_uses_local_as_source(tmp_path):
|
||||
runner = _Runner(returncode=0)
|
||||
filter_path = _write_filter_file(tmp_path)
|
||||
project = SyncProject(name="research", path="/research", local_sync_path="/tmp/research")
|
||||
|
||||
project_copy(
|
||||
project,
|
||||
"my-bucket",
|
||||
"push",
|
||||
overwrite=False,
|
||||
run=runner,
|
||||
is_installed=lambda: True,
|
||||
filter_path=filter_path,
|
||||
)
|
||||
|
||||
cmd, _ = runner.calls[0]
|
||||
assert Path(cmd[2]) == Path("/tmp/research")
|
||||
assert cmd[3] == "basic-memory-cloud:my-bucket/research"
|
||||
|
||||
|
||||
def test_project_copy_file_pull_copyto_renames_on_dest(tmp_path):
|
||||
runner = _Runner(returncode=0)
|
||||
project = SyncProject(name="research", path="/research", local_sync_path="/tmp/research")
|
||||
|
||||
project_copy_file(
|
||||
project,
|
||||
"my-bucket",
|
||||
"pull",
|
||||
"notes/dup.md",
|
||||
"notes/dup.conflict-S.md",
|
||||
run=runner,
|
||||
is_installed=lambda: True,
|
||||
)
|
||||
|
||||
cmd, _ = runner.calls[0]
|
||||
assert cmd[:2] == ["rclone", "copyto"]
|
||||
assert cmd[2] == "basic-memory-cloud:my-bucket/research/notes/dup.md"
|
||||
# Compare the local dest via Path so the assertion holds on Windows, where the
|
||||
# local root renders with backslashes (rclone accepts the mixed separators).
|
||||
assert Path(cmd[3]) == Path("/tmp/research/notes/dup.conflict-S.md")
|
||||
# pull writes the conflict copy locally → must guard virtual-FS NUL padding
|
||||
assert "--local-no-preallocate" in cmd
|
||||
|
||||
|
||||
def test_project_transfer_keep_both_copies_conflicts_then_additive(tmp_path):
|
||||
runner = _Runner(returncode=0)
|
||||
filter_path = _write_filter_file(tmp_path)
|
||||
project = SyncProject(name="research", path="/research", local_sync_path="/tmp/research")
|
||||
plan = TransferPlan(new=["a.md"], conflicts=["dup.md"], dest_only=[], errors=[])
|
||||
|
||||
result = project_transfer(
|
||||
project,
|
||||
"my-bucket",
|
||||
"pull",
|
||||
plan,
|
||||
strategy="keep-both",
|
||||
conflict_suffix="S",
|
||||
run=runner,
|
||||
is_installed=lambda: True,
|
||||
filter_path=filter_path,
|
||||
)
|
||||
|
||||
assert result is True
|
||||
# First a copyto for the conflict file, written beside the local copy...
|
||||
first_cmd, _ = runner.calls[0]
|
||||
assert first_cmd[:2] == ["rclone", "copyto"]
|
||||
# Path comparison so this holds on Windows (backslash local root).
|
||||
assert Path(first_cmd[3]) == Path("/tmp/research/dup.conflict-S.md")
|
||||
# ...then an additive (new-only) copy that won't overwrite existing local files.
|
||||
second_cmd, _ = runner.calls[1]
|
||||
assert second_cmd[:2] == ["rclone", "copy"]
|
||||
assert "--ignore-existing" in second_cmd
|
||||
|
||||
|
||||
def test_project_transfer_keep_cloud_on_pull_overwrites_local(tmp_path):
|
||||
runner = _Runner(returncode=0)
|
||||
filter_path = _write_filter_file(tmp_path)
|
||||
project = SyncProject(name="research", path="/research", local_sync_path="/tmp/research")
|
||||
plan = TransferPlan(new=[], conflicts=["dup.md"], dest_only=[], errors=[])
|
||||
|
||||
project_transfer(
|
||||
project,
|
||||
"my-bucket",
|
||||
"pull",
|
||||
plan,
|
||||
strategy="keep-cloud",
|
||||
run=runner,
|
||||
is_installed=lambda: True,
|
||||
filter_path=filter_path,
|
||||
)
|
||||
|
||||
# keep-cloud + pull → cloud (source) overwrites local (dest): no --ignore-existing,
|
||||
# and --checksum so the overwrite decision matches the content-based conflict.
|
||||
assert len(runner.calls) == 1
|
||||
cmd, _ = runner.calls[0]
|
||||
assert cmd[:2] == ["rclone", "copy"]
|
||||
assert "--ignore-existing" not in cmd
|
||||
assert "--checksum" in cmd
|
||||
|
||||
|
||||
def test_project_transfer_keep_local_on_push_overwrites_cloud(tmp_path):
|
||||
runner = _Runner(returncode=0)
|
||||
filter_path = _write_filter_file(tmp_path)
|
||||
project = SyncProject(name="research", path="/research", local_sync_path="/tmp/research")
|
||||
plan = TransferPlan(new=[], conflicts=["dup.md"], dest_only=[], errors=[])
|
||||
|
||||
project_transfer(
|
||||
project,
|
||||
"my-bucket",
|
||||
"push",
|
||||
plan,
|
||||
strategy="keep-local",
|
||||
run=runner,
|
||||
is_installed=lambda: True,
|
||||
filter_path=filter_path,
|
||||
)
|
||||
|
||||
cmd, _ = runner.calls[0]
|
||||
assert "--ignore-existing" not in cmd
|
||||
|
||||
|
||||
def test_project_diff_requires_local_path():
|
||||
project = SyncProject(name="research", path="/research")
|
||||
with pytest.raises(RcloneError):
|
||||
project_diff(project, "my-bucket", "pull", is_installed=lambda: True)
|
||||
|
||||
|
||||
def test_project_diff_raises_on_fatal_check_error(tmp_path):
|
||||
"""A non-zero exit with no combined listing means the check failed, not 'no diffs'."""
|
||||
runner = _Runner(returncode=7, stdout="", stderr="Failed to create file system: AccessDenied")
|
||||
filter_path = _write_filter_file(tmp_path)
|
||||
project = SyncProject(name="research", path="/research", local_sync_path="/tmp/research")
|
||||
|
||||
with pytest.raises(RcloneError) as exc_info:
|
||||
project_diff(
|
||||
project,
|
||||
"my-bucket",
|
||||
"pull",
|
||||
run=runner,
|
||||
is_installed=lambda: True,
|
||||
filter_path=filter_path,
|
||||
)
|
||||
|
||||
assert "AccessDenied" in str(exc_info.value)
|
||||
|
||||
|
||||
def test_project_diff_nonzero_with_differences_does_not_raise(tmp_path):
|
||||
"""Differences make rclone check exit non-zero, but that is expected — no raise."""
|
||||
runner = _Runner(returncode=1, stdout="* changed.md\n")
|
||||
filter_path = _write_filter_file(tmp_path)
|
||||
project = SyncProject(name="research", path="/research", local_sync_path="/tmp/research")
|
||||
|
||||
plan = project_diff(
|
||||
project,
|
||||
"my-bucket",
|
||||
"pull",
|
||||
run=runner,
|
||||
is_installed=lambda: True,
|
||||
filter_path=filter_path,
|
||||
)
|
||||
|
||||
assert plan.conflicts == ["changed.md"]
|
||||
|
||||
|
||||
def test_project_transfer_keep_local_on_pull_preserves_local(tmp_path):
|
||||
"""keep-local + pull → local is the destination and must be preserved (--ignore-existing)."""
|
||||
runner = _Runner(returncode=0)
|
||||
filter_path = _write_filter_file(tmp_path)
|
||||
project = SyncProject(name="research", path="/research", local_sync_path="/tmp/research")
|
||||
plan = TransferPlan(new=[], conflicts=["dup.md"], dest_only=[], errors=[])
|
||||
|
||||
project_transfer(
|
||||
project,
|
||||
"my-bucket",
|
||||
"pull",
|
||||
plan,
|
||||
strategy="keep-local",
|
||||
run=runner,
|
||||
is_installed=lambda: True,
|
||||
filter_path=filter_path,
|
||||
)
|
||||
|
||||
cmd, _ = runner.calls[0]
|
||||
assert "--ignore-existing" in cmd
|
||||
assert "--checksum" not in cmd
|
||||
|
||||
|
||||
def test_project_transfer_keep_cloud_on_push_preserves_cloud(tmp_path):
|
||||
"""keep-cloud + push → cloud is the destination and must be preserved (--ignore-existing)."""
|
||||
runner = _Runner(returncode=0)
|
||||
filter_path = _write_filter_file(tmp_path)
|
||||
project = SyncProject(name="research", path="/research", local_sync_path="/tmp/research")
|
||||
plan = TransferPlan(new=[], conflicts=["dup.md"], dest_only=[], errors=[])
|
||||
|
||||
project_transfer(
|
||||
project,
|
||||
"my-bucket",
|
||||
"push",
|
||||
plan,
|
||||
strategy="keep-cloud",
|
||||
run=runner,
|
||||
is_installed=lambda: True,
|
||||
filter_path=filter_path,
|
||||
)
|
||||
|
||||
cmd, _ = runner.calls[0]
|
||||
assert "--ignore-existing" in cmd
|
||||
|
||||
|
||||
def test_project_transfer_keep_both_returns_false_on_copy_failure(tmp_path):
|
||||
"""A failed conflict-copy aborts the transfer before the additive pass."""
|
||||
runner = _Runner(returncode=1) # every rclone invocation fails
|
||||
filter_path = _write_filter_file(tmp_path)
|
||||
project = SyncProject(name="research", path="/research", local_sync_path="/tmp/research")
|
||||
plan = TransferPlan(new=["a.md"], conflicts=["dup.md"], dest_only=[], errors=[])
|
||||
|
||||
result = project_transfer(
|
||||
project,
|
||||
"my-bucket",
|
||||
"pull",
|
||||
plan,
|
||||
strategy="keep-both",
|
||||
conflict_suffix="S",
|
||||
run=runner,
|
||||
is_installed=lambda: True,
|
||||
filter_path=filter_path,
|
||||
)
|
||||
|
||||
assert result is False
|
||||
# Stopped after the first failed copyto — the additive copy never ran.
|
||||
assert len(runner.calls) == 1
|
||||
assert runner.calls[0][0][:2] == ["rclone", "copyto"]
|
||||
|
||||
Reference in New Issue
Block a user