fix: make config sync directories configurable via config.yaml#17
Closed
LLQWQ wants to merge 27 commits into
Closed
fix: make config sync directories configurable via config.yaml#17LLQWQ wants to merge 27 commits into
LLQWQ wants to merge 27 commits into
Conversation
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
…-CN.md Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
…nk styling) Made-with: Cursor
…ine) Made-with: Cursor
Three bugs prevented attachments from syncing: 1. FileSyncUpdate handler silently skipped files without inline content. Attachments are binary and never include inline content — the client must send a FileChunkDownload request to initiate chunked transfer. 2. Binary message prefix mismatch: server sends "00" (VaultFileMsgType) but client checked for "BC", so all download chunks were ignored. 3. file_content_hash_binary used SHA-256 instead of the djb2 byte-hash the server/plugin expect, causing perpetual hash mismatches. Also: send local file hashes in FileSync request so the server can properly diff, wait for in-flight downloads before declaring sync complete, and increase file sync timeout to 300s. Closes #1
fix: attachment sync not downloading from server
Five bugs fixed: 1. FileSync: _on_sync_end set _sync_complete immediately, but the server streams FileSyncDelete/Update messages *after* FileSyncEnd. pull/sync-once closed the connection before those messages arrived, so remote deletions were never applied locally. Now mirrors NoteSync: tracks expected vs received counts and only marks complete when all messages are processed. 2. FileSync: _on_sync_update returned early on unexpected content type without incrementing _received_modify, causing _wait_file_sync to stall until the 300s timeout. 3. Watcher: on_moved did not check is_ignored, so server-triggered renames (which temporarily ignore both paths) could fire on_local_rename after the ignore window closed, pushing a redundant delete+upload back to the server. 4. Daemon (fns run): after a WebSocket reconnect _initial_sync was never retried, so any changes made on other clients during the outage (including deletions) were permanently missed. Added on_reconnect hook to WSClient; SyncEngine registers a callback that pauses the watcher and re-runs _initial_sync on every reconnect. 5. sync_once: called _initial_sync (incremental NoteSync) then immediately called request_full_sync, sending two NoteSync requests per session. sync_once now calls request_full_sync + file_sync.request_sync directly.
Cover the bugs fixed in the previous commit: - FileSyncEnd before FileSyncDelete: sync must stay incomplete until all expected delete messages are received - FileSyncDelete before FileSyncEnd: completes correctly on End arrival - Multiple deletes: only completes after the last one - Delete of nonexistent file: still counted so sync doesn't stall - Mixed modify + delete: both counters must drain before complete - request_sync counter reset: second sync not polluted by first - Unexpected content type: no longer stalls for 300s - Watcher on_moved: ignored src/dest paths are skipped
_try_remove_empty_parent previously only checked one level up, leaving ancestor directories (e.g. diagrams/, notes/) as empty shells after all their contents were deleted via pull sync. Now walks up the tree until vault_path, removing each empty directory in turn.
heartbeat_interval was defined in config (default 30s) but never used — ping_interval/ping_timeout were both None, so silently dead connections were never detected and the receive loop hung indefinitely. Now passes heartbeat_interval to websockets ping_interval and ping_timeout. A missed pong raises ConnectionClosed, exits _listen(), and triggers the reconnect loop which re-syncs any missed events.
When the CLI receives a sync event from the server and writes/deletes a file locally, the watchdog inotify event is queued by the kernel and delivered asynchronously. By the time the observer thread processes it, unignore_file had already been called, causing the watcher to treat the server-written file as a local change and push it back — creating an echo loop confirmed in logs (NoteSyncModify received → NoteModify sent back → server pushes again). Fix: add asyncio.sleep(0.6) before unignore_file in all server→client file operation handlers (write, delete, rename, chunked download) in both NoteSync and FileSync. This keeps the file ignored until the watchdog debounce window (0.5s) has expired.
… timeout (#8) PR #6 used ping_interval=30 but the server does not respond to WebSocket ping frames — confirmed by real-environment test: connection dropped every 30s with "keepalive ping timeout". Reverted to ping_interval=None. Instead, an asyncio background task (_inactivity_watchdog) tracks the last received message time and closes the connection if no data arrives for 2 × heartbeat_interval (60s default), triggering the reconnect loop. _last_received_at is updated on every text or binary frame received. Also extended the NoteSync timeout in _initial_sync from 60s to 300s. The server sends NoteSyncEnd immediately but may delay actual note delivery by over a minute on full syncs — the 60s window was too short and caused NoteSync to abort before any notes were written. Verified with real-server pull: 24 notes (including Test-456/123.md, 456.md, 789.md) fully synced, no ping disconnects, no echo push-backs.
Core changes across the sync stack: * client.py — re-enable websockets lib's own ping with generous timeout (ping_interval=45, ping_timeout=90). Auto-pongs to the server's 25s pings keep its 60s deadline refreshed; the long timeout tolerates Pong delay behind large binary chunk writes that was tripping keepalive at 30s and 60s during initial sync. * note_sync.py / file_sync.py — replace the 0.6s/2s time-based ignore with a content-hash echo cache. The sleep-in-handler serialized the whole WS receive loop: 178 modifies × 2s ate the 300s sync window. _echo_hashes[path] now tracks the last known synced state and is updated on BOTH inbound (server write) and outbound (push) so a legitimate revert to a previous value still propagates. _DELETED sentinel for deletes, with the same outbound update so recreate + re-delete works. push_modify accepts force=True and NoteSyncNeedPush uses it to bypass the echo check. * watcher.py — directory rename now enumerates the new tree and schedules per-file transitions. _schedule_move_transition handles the excluded-path edge cases: move into excluded → emit delete of old, move out of excluded → emit modify of new. * doc/server-protocol.md — new reference distilled from the Go source: heartbeat protocol, broadcast mechanics, sync flow, and the specific client pitfalls we hit. * tests — new test_echo_cache.py covers the revert, tombstone-reuse, NeedPush-force, and failed-write-no-poison cases; test_file_sync adds the excluded-path rename transitions. 23 unit tests pass. Bidirectional integration tests (not committed) confirm notes, attachments, and folders sync correctly in both directions for create / modify / delete / rename.
Fix realtime sync startup and directory deletes
…n, .agents, etc.) Adds full support for synchronizing Obsidian config directories (e.g. .obsidian, .agents/skills) via the SettingSync WebSocket protocol. - protocol.py: add SettingSync action constants - state.py: persist last_setting_sync_time - setting_sync.py: new module handling SettingSyncModify/Delete/Rename/Mtime/End - sync_engine.py: wire setting_sync into pull/push/run and watcher callbacks - file_sync.py: exclude all dot-prefixed dirs from FileSync to avoid conflicts Resolves the issue where config files uploaded by the Obsidian plugin never reached the CLI because the CLI only implemented NoteSync/FileSync.
feat: implement SettingSync protocol for config directories (.obsidian, .agents, etc.)
Config files (.obsidian/*, .agents/*, etc.) should only be synced via SettingSync protocol, not FileSync. This fixes three issues: 1. _initial_sync(): only call file_sync.request_sync() when sync_files is true, not when sync_config is true 2. _push_all_files(): skip config files entirely, let _push_all_settings handle them via SettingSync 3. file_sync._collect_local_files(): always skip dot-prefixed directories, regardless of sync_config setting Previously, config files were uploaded to both file DB and setting DB, causing conflicts and plugin loss during sync.
Config directories (.obsidian, .agents, etc.) are managed by SettingSync protocol, not FolderSync. This prevents accidental deletion of the entire .obsidian directory when the server sends FolderSyncDelete after config records are removed from the file database. The fix adds checks in all three FolderSync handlers: - _on_sync_modify: skip creating dot-prefixed dirs - _on_sync_delete: skip deleting dot-prefixed dirs (CRITICAL) - _on_sync_rename: skip renaming dot-prefixed dirs
Previously, FileSync and SettingSync had inconsistent hardcoded handling
of dot-prefixed directories. This caused custom config directories like
.agents to not be properly synchronized.
Changes:
- config.py: add config_sync_dirs field to SyncConfig (default: [.obsidian, .agents])
- sync_engine.py: use config_sync_dirs from config instead of hardcoded list
- setting_sync.py: pass config_sync_dirs to _is_config_path()
- file_sync.py: skip dot-prefixed dirs only when sync_config is enabled
- config.yaml: add config_sync_dirs example configuration
Users can now customize which dot-prefixed directories are treated as config
by editing config.yaml:
config_sync_dirs:
- .obsidian
- .agents
- .my-custom-config
Fixes: custom config directories (.agents) not being synced by CLI
Reviewer's GuideMakes handling of dot-prefixed config directories explicit and configurable so that only selected directories (e.g. .obsidian, .agents) are treated as config and are synced via SettingSync, while FolderSync/FileSync ignore them appropriately based on sync_config/sync_files and config_sync_dirs in config.yaml. Sequence diagram for deciding config vs file sync for dot-prefixed directoriessequenceDiagram
actor User
participant SyncEngine
participant FileSync
participant SettingSync
participant FolderSync
Note over User: User triggers a sync operation
User->>SyncEngine: start_sync()
SyncEngine->>SyncEngine: _initial_sync()
Note over SyncEngine: Config: sync_files, sync_config, config_sync_dirs
SyncEngine->>NoteSync: request_sync()
NoteSync-->>SyncEngine: notes_synced
alt sync_files is true
SyncEngine->>FileSync: request_sync()
FileSync-->>SyncEngine: file_sync_started
end
alt sync_config is true
SyncEngine->>SettingSync: request_sync()
SettingSync-->>SyncEngine: setting_sync_started
end
loop For each local file
SyncEngine->>SyncEngine: _is_config(rel_path)
Note over SyncEngine: first = rel_path.split("/")[0]
alt first not dot-prefixed
SyncEngine-->>SyncEngine: returns False
alt sync_files is true
SyncEngine->>FileSync: push_upload(rel_path)
else sync_files is false
SyncEngine-->>SyncEngine: skip file
end
else first dot-prefixed
alt first in config_sync_dirs
SyncEngine-->>SyncEngine: returns True
alt sync_config is true
SyncEngine->>SettingSync: push_upload(rel_path)
else sync_config is false
SyncEngine-->>SyncEngine: skip file
end
else first not in config_sync_dirs
alt sync_config is true
SyncEngine-->>SyncEngine: returns True
SyncEngine->>SettingSync: push_upload(rel_path)
else sync_config is false
SyncEngine-->>SyncEngine: returns False
alt sync_files is true
SyncEngine->>FileSync: push_upload(rel_path)
else sync_files is false
SyncEngine-->>SyncEngine: skip file
end
end
end
end
end
Note over FolderSync: Handles folder events from server
Server->>FolderSync: _on_sync_modify(msg)
FolderSync->>FolderSync: first = rel_path.split("/")[0]
alt first startswith dot
FolderSync-->>FolderSync: log ignore and return
else not dot-prefixed
FolderSync->>FolderSync: mkdir for folder
end
Server->>FolderSync: _on_sync_delete(msg)
FolderSync->>FolderSync: first = rel_path.split("/")[0]
alt first startswith dot
FolderSync-->>FolderSync: log ignore and return
else not dot-prefixed
FolderSync->>FolderSync: delete folder if exists
end
Server->>FolderSync: _on_sync_rename(msg)
FolderSync->>FolderSync: check first segments of old and new
alt either startswith dot
FolderSync-->>FolderSync: log ignore and return
else neither dot-prefixed
FolderSync->>FolderSync: rename folder
end
Class diagram for configurable config_sync_dirs and sync routingclassDiagram
class SyncConfig {
+list~str~ exclude_patterns
+int file_chunk_size
+list~str~ config_sync_dirs
}
class AppConfig {
+SyncConfig sync
+ClientConfig client
+ServerConfig server
}
class SyncEngine {
+AppConfig config
+Path vault_path
+FileSync file_sync
+SettingSync setting_sync
+NoteSync note_sync
+FolderSync folder_sync
+bool _is_note(rel_path)
+bool _is_config(rel_path)
+bool _should_sync_file(rel_path)
+async _initial_sync()
+async _push_all_files()
}
class FileSync {
+AppConfig config
+SyncEngine engine
+list~dict~ _collect_local_files()
}
class SettingSync {
+AppConfig config
+SyncEngine engine
+async request_sync()
+async push_upload(rel_path)
}
class SettingSyncHelpers {
+bool _is_config_path(rel, config_sync_dirs)
+dict _extract_inner(msg_data)
}
class FolderSync {
+Path vault_path
+SyncEngine engine
+async _on_sync_modify(msg)
+async _on_sync_delete(msg)
+async _on_sync_rename(msg)
}
class WSMessage {
+dict data
}
class ConfigLoader {
+AppConfig load_config(path)
}
SyncEngine --> SyncConfig : uses
SyncEngine --> FileSync : coordinates
SyncEngine --> SettingSync : coordinates
SyncEngine --> FolderSync : coordinates
AppConfig --> SyncConfig : has
FileSync --> SyncEngine : engine
SettingSync --> SyncEngine : engine
FolderSync --> SyncEngine : engine
ConfigLoader --> AppConfig : constructs
SettingSyncHelpers --> SettingSync : utility
SettingSyncHelpers ..> SyncConfig : uses config_sync_dirs
SyncEngine ..> SyncConfig : uses config_sync_dirs
FileSync ..> SyncConfig : uses config_sync_dirs and sync_config
FolderSync ..> SyncConfig : indirectly uses via engine
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- FolderSync currently hardcodes all dot-prefixed directories as config (skipping them unconditionally), which diverges from the new
config_sync_dirs+sync_configlogic used elsewhere; consider reusing the same_is_config/config logic here so FolderSync behavior stays consistent with FileSync/SettingSync. - The new
config_sync_dirssetting is only partially wired through (e.g.,_is_configuses it but_is_config_path’s default behavior remains "all dot dirs are config" unless it is explicitly passed); it would be cleaner to ensure all config-path checks consistently receive and honorconfig_sync_dirsso the effective rules don’t differ between components.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- FolderSync currently hardcodes all dot-prefixed directories as config (skipping them unconditionally), which diverges from the new `config_sync_dirs` + `sync_config` logic used elsewhere; consider reusing the same `_is_config`/config logic here so FolderSync behavior stays consistent with FileSync/SettingSync.
- The new `config_sync_dirs` setting is only partially wired through (e.g., `_is_config` uses it but `_is_config_path`’s default behavior remains "all dot dirs are config" unless it is explicitly passed); it would be cleaner to ensure all config-path checks consistently receive and honor `config_sync_dirs` so the effective rules don’t differ between components.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
09f4e40 to
57024ca
Compare
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
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.
Problem
CLI's
FileSyncandSettingSynchave inconsistent handling of dot-prefixed directories like.agents:FileSync._collect_local_files()skips ALL dot-prefixed directoriesSettingSyncuses_is_config_path()which treats ALL dot-prefixed dirs as configSyncEngine._is_config()also treats ALL dot-prefixed dirs as configThis causes problems when syncing custom config directories:
.agents/exists on Obsidian but not on CLI vault, CLI won't send it to server.agents/filesImpact
.agents,.my-config, etc.) are not properly synchronizedRoot Cause
In
file_sync.py:In
sync_engine.py:Proposed Fix
Make config directory handling explicit:
.obsidianand.agentsas config directoriessync_configsettingsync_config=True(so they go through SettingSync)
Files Changed
fns_cli/config.py: Addconfig_sync_dirsto config schemafns_cli/file_sync.py: Skip dot-prefixed dirs based onconfig_sync_dirsfns_cli/setting_sync.py: Useconfig_sync_dirsfor config path checkfns_cli/sync_engine.py: Use explicit config directory listTesting
.agentsin configSyncOtherDirs.agents/directory in Obsidian.agents/directory).agents/files are synced to CLI.agents/files should remainEnvironment
Summary by Sourcery
Make config directories explicitly configurable and adjust sync behavior so dot-prefixed folders are treated as config only when configured, aligning FileSync, SettingSync, FolderSync, and SyncEngine.
New Features:
Enhancements: