CS-11062: protect local edits in boxel realm watch start#4844
Conversation
Flip the default so the watcher skips a remote-driven download (or unlink) when the local file's hash diverges from `.boxel-sync.json`, instead of silently overwriting. Pass `--overwrite-local` to opt back into the old unconditional mirror behavior. Skipped files keep their old `lastKnownMtimes` so the warning re-fires on every poll until the user reconciles via `boxel realm sync …` or reruns with `--overwrite-local`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f8063d966
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const manifest = this.overwriteLocal | ||
| ? null | ||
| : await loadManifest(this.options.localDir); |
There was a problem hiding this comment.
Ignore manifests from other realms before divergence checks
Treating any .boxel-sync.json as the divergence baseline can silently bypass the new local-edit protection when the manifest belongs to a different realm. In that case, localDivergesFromManifest may see a matching hash and allow overwrite/delete, even though this watch session has no valid sync baseline for this realm. initialize() already guards remoteMtimes with a realm URL match, so flushPending() should similarly discard mismatched manifests (or pass null) before hash comparisons to avoid unintended local data loss.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR changes boxel realm watch start so remote changes no longer overwrite locally diverged files by default, while adding an opt-in flag to restore prior mirroring behavior.
Changes:
- Adds divergence detection during watcher flushes and reports skipped files.
- Threads
--overwrite-localthrough the CLI and watcher options. - Adds integration coverage and updates realm-sync skill docs for the new behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
packages/boxel-cli/src/commands/realm/watch/start.ts |
Implements skipped flush results, divergence checks, logging, and CLI flag wiring. |
packages/boxel-cli/tests/integration/realm-watch.test.ts |
Adds integration tests for skipped downloads/deletes and overwrite behavior. |
packages/boxel-cli/plugin/skills/realm-sync/SKILL.md |
Documents the new watch default and --overwrite-local option. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * True when a local file exists at `localPath` but its content no longer | ||
| * matches the hash recorded for `relPath` in the sync manifest (or the | ||
| * manifest has no record of it at all). False when no local file exists | ||
| * — there's nothing to protect. | ||
| */ | ||
| private async localDivergesFromManifest( | ||
| localPath: string, | ||
| relPath: string, | ||
| manifest: SyncManifest | null, | ||
| ): Promise<boolean> { | ||
| let localHash: string; | ||
| try { | ||
| localHash = await computeFileHash(localPath); | ||
| } catch (err: any) { | ||
| if (err.code === 'ENOENT') return false; | ||
| throw err; | ||
| } | ||
| const manifestHash = manifest?.files[relPath]; |
| const manifest = this.overwriteLocal | ||
| ? null | ||
| : await loadManifest(this.options.localDir); | ||
|
|
||
| for (const [file, info] of drained) { | ||
| const localPath = path.join(this.options.localDir, file); | ||
|
|
||
| if ( | ||
| !this.overwriteLocal && | ||
| (await this.localDivergesFromManifest(localPath, file, manifest)) |
| let checkpoint: Checkpoint | null = null; | ||
| if (changes.length > 0) { | ||
| await this.persistManifest(pulled, deleted); | ||
| checkpoint = await this.checkpointManager.createCheckpoint( |
|
|
||
| `push` and `pull` have their own `--delete` and `--dry-run` flags but no `--prefer-*` flags (they're one-directional). When in doubt, dry-run first. | ||
|
|
||
| `watch` protects local edits without a flag: by default any file whose local hash differs from the sync manifest is skipped (with a yellow `⚠ skipped …` line) instead of overwritten. The warning re-fires on every poll until the user reconciles via `boxel realm sync …` (e.g. `--prefer-newest`) or rerun watch with `--overwrite-local` to accept the remote. |
Summary
boxel realm watch startno longer silently overwrites local edits. When the local copy of a remotely-changed file diverges from.boxel-sync.json, the watcher skips the download (or unlink), logs⚠ skipped <file>: local diverges from sync manifest …, and keeps polling.--overwrite-localflag restores the previous unconditional mirror behavior.lastKnownMtimesso the warning re-fires every poll until the user reconciles viaboxel realm sync …(e.g.--prefer-newest) or reruns watch with--overwrite-local.How
RealmWatcher.flushPendingso both first-tick (tickAll) and steady-state polls hit one code path. Per-flush, the manifest is loaded once and reused across all checks in that flush.FlushResultgainsskipped: string[];logFlushprints one yellow⚠ skipped …line per file.watchRealmsthreadsoverwriteLocalfrom CLI →RealmWatcherconstructor.sync-manifest.ts/sync-logic.ts/realm-sync-base.ts—computeFileHash/loadManifestare reused..boxel-historycheckpoint creation is now skipped on fully-skipped flushes (changes.length === 0); applied flushes still snapshot as before.Tests added (in
packages/boxel-cli/tests/integration/realm-watch.test.ts)skips download when the local file diverges from the sync manifestoverwrites diverged local files when overwriteLocal is enabledskips first-run downloads when local files exist at remote paths and no manifestdownloads when the local file still matches the manifest hashdoes not delete a locally-edited file when the remote disappearsdeletes the local file when the remote disappears and the local matches the manifestkeeps re-detecting a skipped divergence on subsequent polls until resolvedTest plan
packages/boxel-cliintegration suite green (it wasn't run locally — needs Synapse + realm-server up).watch start→ expect the yellow⚠ skipped …line and the local file untouched.--overwrite-local→ expect the local file replaced and no skipped line..boxel-sync.jsonfrom a populated dir, runwatch start→ expect every existing file to skip with a warning on the first poll; non-overlapping remote files still pulled.boxel realm sync --prefer-newest→ re-run watch and confirm the warning stops.Notes
plugin.jsonversion bump is intentionally deferred to the final pre-merge commit.plugin/skills/realm-sync/SKILL.mddocuments both the new default and the "runboxel realm pullfirst when seeding a pre-populated directory" caveat (otherwise every file looks diverged on first poll).🤖 Generated with Claude Code