feat(/mine): translate client-side paths via PALACE_DAEMON_PATH_MAP - #1
Merged
Conversation
The daemon's /mine endpoint requires a directory that exists on the
daemon's filesystem. Hooks running on a client machine speak in their
own namespace (e.g. /home/<user>/.claude/projects/...), but a remote
daemon may see the same files at a different mount (e.g. via
Syncthing replication into /mnt/raid/...). Without translation the
hook's POST /mine fails with 400 "Directory does not exist" and
transcript ingest silently goes nowhere.
Add an opt-in PALACE_DAEMON_PATH_MAP env var and apply translation in
/mine before path validation. Format is comma-separated
client_prefix=daemon_prefix entries; first matching prefix wins,
non-matching paths pass through unchanged so existing direct
daemon-side absolute paths still work.
Example deploy on disks::
PALACE_DAEMON_PATH_MAP="/home/jp/.claude/=/mnt/raid/claude-config/,/home/jp/Projects/=/mnt/raid/projects/"
Companion to memorypalace fix/restore-transcript-ingest-via-daemon —
that PR un-skips the three hook mining paths so they POST to /mine;
this PR makes /mine accept the client-side paths those POSTs send.
Tests: new tests/test_path_translation.py with 10 stdlib-unittest
cases covering parse_path_map (empty, single, multi, malformed,
whitespace, env-var fallback) and _translate_client_path (passthrough,
prefix match, non-match passthrough, multi-rule first-match-wins).
Run via:
python -m unittest tests.test_path_translation -v
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
3 tasks
There was a problem hiding this comment.
Pull request overview
Adds support for translating client-side filesystem paths to daemon-side paths for the /mine endpoint via a new PALACE_DAEMON_PATH_MAP environment variable, enabling remote hooks to POST paths that exist under different mount points on the daemon host.
Changes:
- Added
_parse_path_mapand_translate_client_pathhelpers to parse and applyPALACE_DAEMON_PATH_MAP. - Applied client→daemon path translation in
/minebefore absolute/traversal/existence validation. - Added
unittestcoverage for parsing and translation behavior.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
main.py |
Introduces path-map parsing/translation helpers and applies translation in /mine before validation. |
tests/test_path_translation.py |
Adds unit tests covering parsing + translation logic via stdlib unittest. |
tests/__init__.py |
Adds tests package marker to support test discovery/imports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """ | ||
| for client_prefix, daemon_prefix in _parse_path_map(): | ||
| if path.startswith(client_prefix): | ||
| return daemon_prefix + path[len(client_prefix):] |
Comment on lines
1061
to
+1068
| directory = body.get("dir") | ||
| if not directory: | ||
| raise HTTPException(status_code=400, detail="'dir' is required") | ||
|
|
||
| # Hook clients send paths in their own filesystem namespace. Translate | ||
| # to the daemon's view via PALACE_DAEMON_PATH_MAP before validation. | ||
| directory = _translate_client_path(directory) | ||
|
|
| class TestParsePathMap(unittest.TestCase): | ||
| def test_empty_returns_empty(self): | ||
| self.assertEqual(main._parse_path_map(""), []) | ||
| self.assertEqual(main._parse_path_map(None), []) |
Three findings from Copilot's review pass:
1. **Path-join normalization** (Copilot main.py:223). _translate_client_path
built the rewrite via raw string concatenation; mismatched trailing
slashes between client_prefix and daemon_prefix could produce paths
like "/mnt/raid/ccprojects/-x". Strip exactly one trailing /
from daemon_prefix, lstrip from suffix, join with explicit "/" so
the result is a single separator regardless of operator slash style.
2. **Type validation in /mine** (Copilot main.py:1068). `directory`
comes from JSON and could be a number / object / list, which would
raise on _translate_client_path().startswith and surface as 500.
Add isinstance(directory, str) guard returning a clean 400.
3. **Test determinism for _parse_path_map** (Copilot
tests/test_path_translation.py:31). The previous test passed
_parse_path_map(None) and assumed env was unset, but None used to
mean "read env." Replace the None default with a private sentinel
(_PATH_MAP_USE_ENV) so:
- _parse_path_map() — read env (sentinel default)
- _parse_path_map(None) — no mapping
- _parse_path_map(str) — parse this
Updated tests are split: one for explicit None == empty, one for
no-arg + cleared env == empty, one for no-arg + set env == parsed.
New tests:
- test_explicit_none_returns_empty
- test_env_unset_no_arg_returns_empty
- test_join_normalizes_mismatched_trailing_slash (4 cases: each combo
of trailing/no-trailing slash on client and daemon prefix)
13 tests pass:
python -m unittest tests.test_path_translation -v
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
jphein
added a commit
that referenced
this pull request
May 6, 2026
Seven findings, all addressed: 1. **Path translation before is_dir check** (Copilot watcher.py:133). parse_watch_dirs() validated against the daemon filesystem before any path translation, so a PALACE_WATCH_DIRS value written in the client namespace was always rejected as "not a directory". Add a translator= kwarg; main.py now passes _translate_client_path so client-namespace entries reach the daemon's view first. 2. **Drop on_any_event in favor of specific subscriptions** (Copilot watcher.py:160). watchdog 3.x emits opened/closed events on plain file reads on Linux; routing those through the debounce would re-mine the project on every file open. Replaced on_any_event with explicit on_created / on_modified / on_moved / on_deleted. 3. **Check dest_path on FileMovedEvent** (Copilot watcher.py:171). Editor save-via-rename writes a temp file then renames to the real filename. The src_path on the move event is the temp; only dest_path has the real extension. Extracted _has_watchable_extension() helper; on_moved() checks both sides. 4. **Per-target schedule() failure isolation** (Copilot watcher.py:219). A bare for-loop let an exception in observer.schedule() (e.g. inotify watch limit on a large repo) abort the entire service. Wrap each schedule() in try/except; one failed target no longer disables the others. Logged at warning level. 5. **Cancel debounce timers on stop** (Copilot watcher.py:224). stop() tore down the observer but left armed threading.Timer objects to fire mid-teardown. Add cancel_pending() on the handler; service stop() walks all handlers first. 6. **Surface watcher-coroutine errors** (Copilot watcher.py:246). asyncio.run_coroutine_threadsafe() returns a Future the caller must observe; dropping it swallowed exceptions silently. Add a _log_future_exception done-callback. 7. **/watch reports running state accurately** (Copilot main.py:558). app.state.watcher was set unconditionally even when start() returned early (empty env, missing dep, all targets failed). Now only published to app.state when watcher.is_running. GET /watch reports running: false for idle/disabled instead of misleading running: true. Tests: 28 pass (was 24). New cases: - test_rename_to_watched_extension_fires (covers #3) - test_cancel_pending_drops_armed_timer (covers #5) - test_one_failed_target_doesnt_disable_others (covers #4) - test_translator_runs_before_is_dir (covers #1) Existing handler tests retargeted from on_any_event to on_modified since on_any_event no longer exists in this revision. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jphein
added a commit
that referenced
this pull request
May 6, 2026
Closes the "automining when files are added or updated" half of JP's ask. The other half (CLI add/remove of mined data) ships in techempower-org/mempalace#4 as `mempalace mined` + `mempalace purge --source-file`. Adds a watchdog-based file watcher that runs inside the daemon's lifespan. Configured via `PALACE_WATCH_DIRS` env var: PALACE_WATCH_DIRS="/home/jp/Projects/realmwatch=wing_realmwatch, /home/jp/Projects/oracle=wing_oracle" Each entry is `path` or `path=wing`. Bare path derives wing via `mempalace.config.normalize_wing_name(basename)` to match the local-spawn miner default. Non-existent paths are warned-and-skipped, not fatal — a misconfigured env var doesn't kill startup. Architecture: * watcher.py — pure-Python module: parse_watch_dirs(), _DebouncedMineHandler (subclass of FileSystemEventHandler with a per-target debounce timer), WatcherService (lifecycle wrapper around watchdog.Observer). * main.py lifespan — instantiate WatcherService, schedule one recursive watch per target, register stop() on shutdown. Failures are non-fatal; the daemon serves traffic regardless. * main.py — `_internal_mine()` async closure runs the same `mempalace mine ... --mode projects --wing X` argv as the existing /mine endpoint, gated by the same `_mine_sem`. * main.py — new `GET /watch` endpoint surfaces the running target list. No POST/DELETE — runtime add/remove requires daemon restart (operator just edits the systemd unit env and restarts). Debounce: events on the same target collapse via a 2-second per-target timer. Catches editor write+rename storms and *.swp / *.lock churn. Also pre-filters by extension allowlist (29 extensions: code, configs, markdown). Inotify fires for everything; the handler discards non-watched extensions before the timer schedules. Path translation: when a watch path lives in a Syncthing-replicated directory, the daemon-side path is translated through `PALACE_DAEMON_PATH_MAP` before the mine subprocess runs (same mechanism used by /mine). Tests: 11 new unittest cases in tests/test_watcher.py: - TestParseWatchDirs (7 cases): empty, single-with-derived-wing, explicit-wing, multiple, skips-nonexistent, skips-files, env-fallback - TestDebouncedMineHandler (4 cases): single-event-fires, burst-collapses-to-one, skips-directory-events, skips-bad-extensions Stacked on #1 (path-translation). When that merges, this PR auto-rebases against main; reviewers see only the new commits in this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jphein
added a commit
that referenced
this pull request
May 6, 2026
Seven findings, all addressed: 1. **Path translation before is_dir check** (Copilot watcher.py:133). parse_watch_dirs() validated against the daemon filesystem before any path translation, so a PALACE_WATCH_DIRS value written in the client namespace was always rejected as "not a directory". Add a translator= kwarg; main.py now passes _translate_client_path so client-namespace entries reach the daemon's view first. 2. **Drop on_any_event in favor of specific subscriptions** (Copilot watcher.py:160). watchdog 3.x emits opened/closed events on plain file reads on Linux; routing those through the debounce would re-mine the project on every file open. Replaced on_any_event with explicit on_created / on_modified / on_moved / on_deleted. 3. **Check dest_path on FileMovedEvent** (Copilot watcher.py:171). Editor save-via-rename writes a temp file then renames to the real filename. The src_path on the move event is the temp; only dest_path has the real extension. Extracted _has_watchable_extension() helper; on_moved() checks both sides. 4. **Per-target schedule() failure isolation** (Copilot watcher.py:219). A bare for-loop let an exception in observer.schedule() (e.g. inotify watch limit on a large repo) abort the entire service. Wrap each schedule() in try/except; one failed target no longer disables the others. Logged at warning level. 5. **Cancel debounce timers on stop** (Copilot watcher.py:224). stop() tore down the observer but left armed threading.Timer objects to fire mid-teardown. Add cancel_pending() on the handler; service stop() walks all handlers first. 6. **Surface watcher-coroutine errors** (Copilot watcher.py:246). asyncio.run_coroutine_threadsafe() returns a Future the caller must observe; dropping it swallowed exceptions silently. Add a _log_future_exception done-callback. 7. **/watch reports running state accurately** (Copilot main.py:558). app.state.watcher was set unconditionally even when start() returned early (empty env, missing dep, all targets failed). Now only published to app.state when watcher.is_running. GET /watch reports running: false for idle/disabled instead of misleading running: true. Tests: 28 pass (was 24). New cases: - test_rename_to_watched_extension_fires (covers #3) - test_cancel_pending_drops_armed_timer (covers #5) - test_one_failed_target_doesnt_disable_others (covers #4) - test_translator_runs_before_is_dir (covers #1) Existing handler tests retargeted from on_any_event to on_modified since on_any_event no longer exists in this revision. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jphein
added a commit
that referenced
this pull request
May 6, 2026
* feat(watcher): file-watcher service for auto-mining on file change Closes the "automining when files are added or updated" half of JP's ask. The other half (CLI add/remove of mined data) ships in techempower-org/mempalace#4 as `mempalace mined` + `mempalace purge --source-file`. Adds a watchdog-based file watcher that runs inside the daemon's lifespan. Configured via `PALACE_WATCH_DIRS` env var: PALACE_WATCH_DIRS="/home/jp/Projects/realmwatch=wing_realmwatch, /home/jp/Projects/oracle=wing_oracle" Each entry is `path` or `path=wing`. Bare path derives wing via `mempalace.config.normalize_wing_name(basename)` to match the local-spawn miner default. Non-existent paths are warned-and-skipped, not fatal — a misconfigured env var doesn't kill startup. Architecture: * watcher.py — pure-Python module: parse_watch_dirs(), _DebouncedMineHandler (subclass of FileSystemEventHandler with a per-target debounce timer), WatcherService (lifecycle wrapper around watchdog.Observer). * main.py lifespan — instantiate WatcherService, schedule one recursive watch per target, register stop() on shutdown. Failures are non-fatal; the daemon serves traffic regardless. * main.py — `_internal_mine()` async closure runs the same `mempalace mine ... --mode projects --wing X` argv as the existing /mine endpoint, gated by the same `_mine_sem`. * main.py — new `GET /watch` endpoint surfaces the running target list. No POST/DELETE — runtime add/remove requires daemon restart (operator just edits the systemd unit env and restarts). Debounce: events on the same target collapse via a 2-second per-target timer. Catches editor write+rename storms and *.swp / *.lock churn. Also pre-filters by extension allowlist (29 extensions: code, configs, markdown). Inotify fires for everything; the handler discards non-watched extensions before the timer schedules. Path translation: when a watch path lives in a Syncthing-replicated directory, the daemon-side path is translated through `PALACE_DAEMON_PATH_MAP` before the mine subprocess runs (same mechanism used by /mine). Tests: 11 new unittest cases in tests/test_watcher.py: - TestParseWatchDirs (7 cases): empty, single-with-derived-wing, explicit-wing, multiple, skips-nonexistent, skips-files, env-fallback - TestDebouncedMineHandler (4 cases): single-event-fires, burst-collapses-to-one, skips-directory-events, skips-bad-extensions Stacked on #1 (path-translation). When that merges, this PR auto-rebases against main; reviewers see only the new commits in this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(watcher): address Copilot review on #2 Seven findings, all addressed: 1. **Path translation before is_dir check** (Copilot watcher.py:133). parse_watch_dirs() validated against the daemon filesystem before any path translation, so a PALACE_WATCH_DIRS value written in the client namespace was always rejected as "not a directory". Add a translator= kwarg; main.py now passes _translate_client_path so client-namespace entries reach the daemon's view first. 2. **Drop on_any_event in favor of specific subscriptions** (Copilot watcher.py:160). watchdog 3.x emits opened/closed events on plain file reads on Linux; routing those through the debounce would re-mine the project on every file open. Replaced on_any_event with explicit on_created / on_modified / on_moved / on_deleted. 3. **Check dest_path on FileMovedEvent** (Copilot watcher.py:171). Editor save-via-rename writes a temp file then renames to the real filename. The src_path on the move event is the temp; only dest_path has the real extension. Extracted _has_watchable_extension() helper; on_moved() checks both sides. 4. **Per-target schedule() failure isolation** (Copilot watcher.py:219). A bare for-loop let an exception in observer.schedule() (e.g. inotify watch limit on a large repo) abort the entire service. Wrap each schedule() in try/except; one failed target no longer disables the others. Logged at warning level. 5. **Cancel debounce timers on stop** (Copilot watcher.py:224). stop() tore down the observer but left armed threading.Timer objects to fire mid-teardown. Add cancel_pending() on the handler; service stop() walks all handlers first. 6. **Surface watcher-coroutine errors** (Copilot watcher.py:246). asyncio.run_coroutine_threadsafe() returns a Future the caller must observe; dropping it swallowed exceptions silently. Add a _log_future_exception done-callback. 7. **/watch reports running state accurately** (Copilot main.py:558). app.state.watcher was set unconditionally even when start() returned early (empty env, missing dep, all targets failed). Now only published to app.state when watcher.is_running. GET /watch reports running: false for idle/disabled instead of misleading running: true. Tests: 28 pass (was 24). New cases: - test_rename_to_watched_extension_fires (covers #3) - test_cancel_pending_drops_armed_timer (covers #5) - test_one_failed_target_doesnt_disable_others (covers #4) - test_translator_runs_before_is_dir (covers #1) Existing handler tests retargeted from on_any_event to on_modified since on_any_event no longer exists in this revision. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
PALACE_DAEMON_PATH_MAPenv var that lets the operator declare client→daemon path rewrites (e.g. katana/home/jp/.claude/→ disks/mnt/raid/claude-config/)/minebefore path validation so hooks running on a client machine can POST their own filesystem paths and have the daemon find the corresponding files in its own namespacePALACE_DAEMON_PATH_MAPis unset, paths pass through unchanged so direct daemon-side absolute paths still workWhy
The daemon's
/mineendpoint validatesdir_path.exists()against its own filesystem. Hooks running on a remote client only know paths in the client's namespace. Without translation, a katana hook calling/minewith/home/jp/.claude/projects/-x/400s on disks where the same files live at/mnt/raid/claude-config/projects/-x/(Syncthing replication).This PR is the daemon half of a two-PR pair fixing transcript-ingest regression — see companion: jphein/mempalace#XXX (
fix/restore-transcript-ingest-via-daemon), which un-skips the three hook mining paths so they POST to/mineinstead of bailing in daemon-strict mode.Without this PR's translation, the companion PR's hook calls would all 400 — same end state as today (no transcript ingest), just noisier. With both deployed, transcripts flow into the palace again.
Format
Comma-separated
client_prefix=daemon_prefix. First matching prefix wins. Order matters when prefixes overlap — operator puts more-specific entries first. Empty / malformed entries skipped silently.Test plan
New
tests/test_path_translation.pywith 10 stdlibunittestcases — pure-function, no live daemon required:_parse_path_map: empty, single, multi, malformed, whitespace, env-var fallback_translate_client_path: passthrough (no map), prefix match, non-match passthrough, multi-rule first-match-winsManual smoke (post-merge, after setting env on disks):
curl -X POST /mine -d '{"dir":"/home/jp/.claude/projects/-x", ...}'should succeed if the same dir exists at the translated daemon-side path.🤖 Generated with Claude Code