fix: make config sync directories configurable via config.yaml#18
Merged
Conversation
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 config directories explicitly configurable via config.yaml and routes dot-prefixed directories consistently through SettingSync instead of FileSync/FolderSync, using a new config_sync_dirs list and updated config detection logic across the sync engine. Sequence diagram for routing dot-prefixed config directories through SettingSyncsequenceDiagram
actor User
participant CLI as CliEntryPoint
participant Engine as SyncEngine
participant FSync as FileSync
participant SSync as SettingSync
participant Folder as FolderSync
User->>CLI: run sync
CLI->>Engine: start _initial_sync()
Engine->>Engine: _is_note(rel_path)
alt rel_path is note
Engine->>Engine: schedule note sync
else rel_path is not note
Engine->>Engine: _is_config(rel_path)
Engine->>Engine: first = rel_path.split("/")[0]
alt first not dot-prefixed
Engine-->>Engine: _is_config returns False
Engine->>FSync: handle as regular file
else first dot-prefixed
Engine->>Engine: check config.sync.config_sync_dirs
alt first in config_sync_dirs
Engine-->>Engine: _is_config returns True
Engine->>SSync: manage via SettingSync
note over SSync,FSync: FileSync and FolderSync skip this path
else first not in config_sync_dirs
Engine->>Engine: return config.sync.sync_config
alt sync_config is True
Engine-->>Engine: _is_config returns True
Engine->>SSync: manage via SettingSync
else sync_config is False
Engine-->>Engine: _is_config returns False
Engine->>FSync: manage via FileSync
end
end
end
end
par folder updates from server
Engine->>Folder: _on_sync_modify/_delete/_rename(rel_path)
Folder->>Folder: first = rel_path.split("/")[0]
alt first startswith(".")
Folder-->>Engine: ignore, config dir handled by SettingSync
else
Folder->>Folder: apply filesystem change
end
end
Class diagram for updated sync configuration and routingclassDiagram
class SyncConfig {
list~str~ exclude_patterns
int file_chunk_size
list~str~ config_sync_dirs
bool sync_files
bool sync_config
}
class AppConfig {
SyncConfig sync
}
class SyncEngine {
AppConfig config
Path vault_path
FileSync file_sync
SettingSync setting_sync
NoteSync note_sync
bool is_excluded(rel_path: str)
bool _is_note(rel_path: str)
bool _is_config(rel_path: str)
bool _should_sync_file(rel_path: str)
async _initial_sync()
async _push_all_files()
}
class FileSync {
AppConfig config
SyncEngine engine
list~dict~ _collect_local_files()
async request_sync()
async push_upload(rel_path: str)
}
class SettingSync {
AppConfig config
async request_sync()
}
class FolderSync {
Path vault_path
async _on_sync_modify(msg: WSMessage)
async _on_sync_delete(msg: WSMessage)
async _on_sync_rename(msg: WSMessage)
}
class ConfigLoader {
AppConfig load_config(path: str)
}
AppConfig *-- SyncConfig
SyncEngine *-- FileSync
SyncEngine *-- SettingSync
SyncEngine --> AppConfig
FileSync --> SyncEngine
FileSync --> AppConfig
SettingSync --> AppConfig
FolderSync --> SyncEngine
ConfigLoader --> AppConfig
SyncConfig "1" o-- "many" str : config_sync_dirs
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- FolderSync now unconditionally ignores all dot-prefixed directories, which bypasses the new
config_sync_dirs/sync_configlogic and may prevent non-config dot directories (or config directories whensync_configis disabled) from ever syncing; consider aligning FolderSync’s behavior with_is_config/config_sync_dirsinstead of hard-codingfirst.startswith('.'). - The default values for
config_sync_dirsare duplicated in both theSyncConfigdataclass andload_config; centralizing this default (e.g., via a shared constant or using the dataclass default when config is missing) would reduce the risk of these ever diverging.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- FolderSync now unconditionally ignores all dot-prefixed directories, which bypasses the new `config_sync_dirs`/`sync_config` logic and may prevent non-config dot directories (or config directories when `sync_config` is disabled) from ever syncing; consider aligning FolderSync’s behavior with `_is_config`/`config_sync_dirs` instead of hard-coding `first.startswith('.')`.
- The default values for `config_sync_dirs` are duplicated in both the `SyncConfig` dataclass and `load_config`; centralizing this default (e.g., via a shared constant or using the dataclass default when config is missing) would reduce the risk of these ever diverging.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- Centralize DEFAULT_CONFIG_SYNC_DIRS constant in config.py to avoid duplication
- Make FolderSync._is_config_dir() respect config_sync_dirs and sync_config settings
instead of hard-coding first.startswith('.') check
Closes review comments from sourcery-ai
Contributor
Author
|
@sourcery-ai Thanks for the review! Both issues have been addressed in commit 1b84efb:
Please re-review when convenient. |
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
2 participants
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 ensure only configured dot-prefixed folders are treated as settings, aligning FileSync, SettingSync, FolderSync, and SyncEngine behavior.
New Features:
Bug Fixes:
Enhancements: