✨ feat: add SSH target support and enhance sync mapping functionality by SigureMo · Pull Request #73 · ShigureLab/watchfs

SigureMo · 2026-04-09T22:50:48Z

closes #72

Co-authored-by: Codex <codex@openai.com>

Copilot

Pull request overview

Adds support for syncing a local watched directory to a remote SSH destination, and refactors sync mapping parsing/queueing to support target-specific behavior and serialization.

Changes:

Introduce SshTarget/LocalTarget backends and a target factory for writes/removals.
Add robust sync-mapping parsing (SRC->DST, legacy SRC:DST, SSH/scp-like + ssh:// formats) and per-destination queue keying.
Improve exclude filtering to support relative/absolute glob patterns and add new unit tests.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`src/watchfs/__main__.py`	Refactors sync loop to use `SyncJob`/`SyncEvent`, per-target queues, and new mapping parser/queue-keying.
`src/watchfs/mappings.py`	New mapping/target spec models + parsing for local and SSH destinations.
`src/watchfs/targets.py`	New local and SSH sync target implementations for write/remove operations.
`src/watchfs/filters.py`	Revamps exclude matching to support glob semantics over absolute/relative patterns.
`tests/test_watchfs.py`	Adds coverage for mapping parsing and SSH queue key behavior.
`tests/test_filters.py`	Adds coverage for suffix iteration and glob matching behavior.
`README.md`	Documents SSH target usage and limitations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T22:56:32Z

src/watchfs/__main__.py

+        event = await queue.get()
+        try:
+            try:
+                await apply_event(event)


consume_target_queue catches Exception, which includes asyncio.CancelledError in Python 3.12+. If the task is cancelled while apply_event() is running, the cancellation can be swallowed and the worker will continue looping, potentially hanging shutdown in the finally gather. Handle CancelledError separately and re-raise it (or avoid catching it) so cancellation reliably stops the worker.

Suggested change

await apply_event(event)

await apply_event(event)

except asyncio.CancelledError:

raise

Copilot · 2026-04-09T22:56:33Z

src/watchfs/mappings.py

+                        abs_dst_dir = dst_spec.path.resolve()
+                        if abs_src_dir in abs_dst_dir.parents:
+                            return Err(
+                                ParseError(f"src_dir({abs_src_dir}) is a subdirectory of dst_dir({abs_dst_dir}).")


The subdirectory check compares abs_src_dir in abs_dst_dir.parents, which means dst_dir is inside src_dir. The error message currently says src_dir(...) is a subdirectory of dst_dir(...), which is reversed and misleading. Update the message (or the check) so the wording matches the actual condition being rejected.

Suggested change

ParseError(f"src_dir({abs_src_dir}) is a subdirectory of dst_dir({abs_dst_dir}).")

ParseError(f"dst_dir({abs_dst_dir}) is a subdirectory of src_dir({abs_src_dir}).")

Copilot · 2026-04-09T22:56:33Z

src/watchfs/mappings.py

+    if sync_mapping.count(":") == 1:
+        src_dir, dst_dir = sync_mapping.split(":", maxsplit=1)
+        return _validate_split(sync_mapping, src_dir, dst_dir, bidirectional=False)
+
+    return Err(ParseError("Invalid sync mapping. Use SRC->DST when the destination contains ':'."))


_split_sync_mapping only supports the legacy SRC:DST form when the mapping contains exactly one :. On Windows, absolute paths like C:\src:D:\dst contain multiple colons (drive letters) and will be rejected even though they're local-to-local. Consider either documenting that Windows users must use SRC->DST, or enhancing the split logic to handle Windows drive prefixes so SRC:DST remains usable cross-platform.

Copilot · 2026-04-09T22:56:33Z

src/watchfs/targets.py

+        dst = self.spec.path / Path(*relative_path.parts)
+        if dst.is_dir():
+            await _remove_directory(dst)
+        elif dst.exists():
+            dst.unlink()


LocalTarget.remove_path calls dst.unlink() directly inside an async def, which performs blocking filesystem IO on the event loop thread. Consider running unlink via asyncio.to_thread(...) (or using an async file removal wrapper) for consistency with _remove_directory and write_file.

Copilot · 2026-04-09T22:56:33Z

README.md

+Use `SRC->DST` when the destination is a remote SSH directory:
+
+```python
+watchfs ./src->meow@192.168.66.1:/tmp/watchfs-demo
+```


These fenced blocks show shell commands, but the code fence language is set to python. Consider switching the fence language to bash/shell so syntax highlighting and copy/paste expectations match the content.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5aa60e2bda

ℹ️ 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".

chatgpt-codex-connector · 2026-04-09T22:57:09Z

src/watchfs/mappings.py

+    if matched := _SSH_TARGET_RE.fullmatch(spec):
+        path = matched.group("path")


Handle drive-letter paths as local targets

parse_target_spec currently classifies any string matching <host>:<path> as SSH, so Windows-style paths such as C:/src or D:/dst are parsed as SshTargetSpec (host='C', path='/src') instead of local paths. In parse_sync_mapping, this causes local sources to be rejected as “Remote source is not supported” and local destinations to trigger SSH attempts to host D, which breaks normal local sync on Windows when forward-slash drive paths are used. Tightening SSH detection (or explicitly exempting drive-letter prefixes) would avoid this regression.

Useful? React with 👍 / 👎.

ShigureNyako

整体方向我认同：SSH target、target 抽象和按目标串行队列的拆分都比较清晰，测试也补到了关键路径，CI 目前也是绿的。

不过这里还有一个需要先处理的兼容性回归：src/watchfs/mappings.py 里当前的 SSH 识别会把 Windows drive-letter 路径当成远程地址，导致 C:/src->D:/dst 这种本地到本地同步在 Windows 上直接失败。这个问题会影响现有本地功能，不建议带着它合入。

建议修复后再继续，我这轮先 request changes。

ShigureNyako · 2026-04-10T01:38:10Z

src/watchfs/mappings.py

+        return _parse_ssh_url(spec)
+
+    if matched := _SSH_TARGET_RE.fullmatch(spec):
+        path = matched.group("path")


这里会把 C:/src、D:/dst 这类 Windows 本地路径误判成 SSH 目标：

parse_target_spec("C:/src") 当前会得到 SshTargetSpec(host="C", path="/src")

随后 parse_sync_mapping(r"C:/src->D:/dst") 会直接返回 Err(Remote source is not supported yet...)

也就是说，这个 PR 会让 Windows 上原本的本地同步场景回归失败。建议在 SSH 识别前先排除 drive-letter 路径（例如 C:/... / D:/...），或者把 SSH 匹配条件再收紧一些，并补一条对应的回归测试。

已在最新提交中修复并补上回归测试：src/watchfs/mappings.py:12-13,63-76,137-149 现在会先识别 Windows drive-letter 本地路径，再走 SSH / legacy mapping 解析；tests/test_watchfs.py:39-64 也覆盖了 C:/src->D:/dst 和 C:/src:D:/dst 两种场景。我这边复验 uv run pytest、uv run ruff check 以及 GitHub Actions 后，已经在本轮给出 approve。

…h parsing in mappings

ShigureNyako

LGTMeow

我复查了这次跟进提交，之前指出的 Windows drive-letter 兼容性回归已经修复：

先识别 Windows 本地路径，再处理 SSH / legacy mapping，和都能按本地路径解析。
补上了对应的回归测试。
里的命令示例也改成了 fenced block。

复验结果：

============================= test session starts ==============================
platform darwin -- Python 3.14.3, pytest-8.4.2, pluggy-1.5.0
rootdir: /Users/nyakku/Projects/tmp/watchfs-pr73-review
configfile: pyproject.toml
plugins: anyio-4.4.0, rerunfailures-16.0.1
collected 20 items

tests/test_filters.py ..... [ 25%]
tests/test_watchfs.py ............... [100%]

============================== 20 passed in 0.02s ==============================：20 passed

All checks passed!：passed
GitHub Actions 的 lint / unittest（Python 3.12、3.13、3.14）均已通过

这边可以 approve。

SigureMo · 2026-04-10T20:27:50Z

src/watchfs/filters.py

+            if stripped_arg.startswith(("~", "/")):
+                exclude_patterns.append(Path(stripped_arg).expanduser().as_posix())
+            else:
+                exclude_patterns.append(Path(stripped_arg).as_posix())


这里区分两个分支的意义是？

由 @codex 回复：这里原本是想让 ~ 走 expanduser()，其它模式直接保留原样；但你这个反馈是对的，这样拆成两个分支没有必要，因为统一用 Path(stripped_arg).expanduser().as_posix() 对相对路径、绝对路径和 ~ 都成立。

我已经在最新提交 db7dcb5 里把这里简化掉了，并复验了 uv run ruff check src tests、uv run pytest 和 PYRIGHT_PYTHON_FORCE_VERSION=latest uv run pyright src/watchfs tests。

To use Codex here, create an environment for this repo.

Co-authored-by: Codex <codex@openai.com>

✨ feat: add SSH target support and enhance sync mapping functionality

2744485

Co-authored-by: Codex <codex@openai.com>

Copilot AI review requested due to automatic review settings April 9, 2026 22:50

📖 docs: update example SSH target in README

5aa60e2

Copilot started reviewing on behalf of SigureMo April 9, 2026 22:51 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

chatgpt-codex-connector bot reviewed Apr 9, 2026

View reviewed changes

ShigureNyako suggested changes Apr 10, 2026

View reviewed changes

✨ feat: update README for bash usage examples and enhance Windows pat…

3b5e563

…h parsing in mappings

ShigureNyako approved these changes Apr 10, 2026

View reviewed changes

SigureMo commented Apr 10, 2026

View reviewed changes

♻️ refactor: simplify exclude pattern normalization

db7dcb5

Co-authored-by: Codex <codex@openai.com>

SigureMo merged commit 8e8afc8 into main Apr 10, 2026
4 checks passed

	ParseError(f"src_dir({abs_src_dir}) is a subdirectory of dst_dir({abs_dst_dir}).")
	ParseError(f"dst_dir({abs_dst_dir}) is a subdirectory of src_dir({abs_src_dir}).")

		if matched := _SSH_TARGET_RE.fullmatch(spec):
		path = matched.group("path")

Uh oh!

Conversation

SigureMo commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

ShigureNyako left a comment

Choose a reason for hiding this comment

Uh oh!

ShigureNyako Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

ShigureNyako Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

ShigureNyako left a comment

Choose a reason for hiding this comment

Uh oh!

SigureMo Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

SigureMo Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants