Implement file watching and auto-sync#150
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
- Desktop: Added `notify` and `notify-debouncer-mini` for real-time file watching in Rust. - Mobile: Implemented polling-based file watcher service (`FileWatcher.ts`) using `AppState` and intervals. - Shared: Added `FileConflictBanner` UI and logic to handle external edits (auto-reload if clean, warn if dirty). - Indexing: Updated `SearchIndexContext` and `LinkIndexContext` to support incremental updates. - Documentation: Added `docs/FILE_WATCHING.md` and updated `AGENTS.md`.
- Desktop: Implemented `notify` watcher (debounced) in Rust and `FileWatcherContext`. - Mobile: Implemented `FileWatcher` service (polling) with `AppState` triggers. - UI: Added `FileConflictBanner` (SVG icon + active states) and handled editor conflicts (clean auto-reload vs dirty banner). - Fix: Prevented save loops on Desktop (content check) and Mobile (internal write notification). - Fix: Ensured Desktop `FileTree` updates on external changes. - Docs: Updated `AGENTS.md` and added `docs/FILE_WATCHING.md`.
- Desktop: Integrated `notify` crate with debouncing to detect filesystem changes. - Mobile: Implemented `FileWatcher` service using polling on app focus. - Indexing: Added incremental update logic to `SearchIndexContext` and `LinkIndexContext`. - UI: Created `FileConflictBanner` with theme-aware styling and SVG icons. - Fix: Prevented save loops by checking content equality (Desktop) and internal write flags (Mobile). - Fix: Ensured `FileTree` updates on external file events. - Theme: Added `--ln-warning-*` variables to `themes.ts` for consistent alert styling.
…anner - Added `--ln-warning-hover` and `--ln-warning-active` to `themes.ts`. - Updated `FileConflictBanner.css` to use these variables for consistent theming. - Mapped primary action background to `var(--ln-warning-active)` for emphasis.
71d90e5 to
e7c41d5
Compare
Desktop coverage summary
|
## Why\n- Desktop typecheck failed because imported from , but does not export that hook.\n- The event wiring logic was hard to test in isolation, which made regressions likely when changing watcher behaviour.\n\n## What Changed\n- Removed invalid/unused imports from .\n- Extracted listener wiring into with explicit typed handlers.\n- Kept provider behaviour unchanged by delegating setup/teardown to the extracted helper.\n- Added to verify:\n - registration of all watcher event listeners,\n - correct handler routing for create/change/delete events,\n - proper listener cleanup on dispose.\n\n## Verification\n-
> liminal-notes@0.1.0 ci:typecheck:desktop /workspaces/liminal-notes
> pnpm --filter @liminal-notes/desktop run typecheck
> @liminal-notes/desktop@0.1.0 typecheck /workspaces/liminal-notes/apps/desktop
> tsc --noEmit -p tsconfig.json\n-
> liminal-notes@0.1.0 ci:test:desktop /workspaces/liminal-notes
> pnpm --filter @liminal-notes/desktop run test:ci
> @liminal-notes/desktop@0.1.0 test:ci /workspaces/liminal-notes/apps/desktop
> mkdir -p ../../test-results/desktop && vitest run --reporter=default --reporter=junit --outputFile.junit=../../test-results/desktop/junit.xml
RUN v4.0.15 /workspaces/liminal-notes/apps/desktop
✓ src/utils/tags.test.ts (2 tests) 2ms
✓ src/adapters/DesktopVaultConfigAdapter.test.ts (5 tests) 5ms
✓ src/components/vaultPickerFlow.test.ts (3 tests) 4ms
✓ src/contexts/TabsContext.test.ts (9 tests) 4ms
✓ src/utils/sanitizeFilename.test.ts (5 tests) 2ms
✓ src/test/commands.test.ts (9 tests) 8ms
✓ src/contexts/FileWatcherContext.test.ts (1 test) 4ms
✓ src/components/Editor/__tests__/decorations.integration.test.tsx (1 test) 79ms
Test Files 8 passed (8)
Tests 35 passed (35)
Start at 04:41:16
Duration 671ms (transform 289ms, setup 376ms, import 507ms, tests 108ms, environment 2.84s)
JUNIT report written to /workspaces/liminal-notes/test-results/desktop/junit.xml
## Why - The new conflict banner path still allowed autosave to continue after an external modification was detected. - That could overwrite disk changes before the user chose `Reload` or `Keep Mine`. - Mobile typecheck also failed because `fileWatcher` was used without being imported. ## What Changed - Imported `fileWatcher` alongside `FileWatcherEvent` in the note screen. - Added a conflict guard in `requestSave` so save requests are skipped while the current note has an unresolved conflict. - Added a second guard in the write path to prevent late in-flight request responses from writing after a conflict appears. - Cleared pending autosave timers when conflict state is set. ## Verification - `pnpm --filter @liminal-notes/mobile run typecheck`
## Why - External change handling compared disk content against a stale `content` closure from the effect. - This could trigger unnecessary reload work and cursor/state churn when content had already been updated. ## What Changed - Added `contentRef` that is synchronised with the latest editor content. - Switched the equality check in the `vault:file-changed` handler to compare against `contentRef.current`. - Included `notify` in the effect dependencies for correct hook dependency coverage. ## Verification - `pnpm run ci:typecheck:desktop` - `pnpm run ci:test:desktop`
## Why - `should_ignore` examined absolute path components. - If a vault root itself contained a dot-prefixed segment (for example `/home/user/.vault`), all watcher events could be filtered out. - This broke file-change propagation for valid notes under that root. ## What Changed - Updated `should_ignore` to evaluate components relative to the configured vault root. - Updated `handle_event` to pass the root path into ignore filtering. - Added unit tests to cover: - ignoring hidden paths inside the vault (`.git`, `.liminal` style), - allowing regular note paths when the vault root itself is hidden. - Removed unused imports (`Mutex` in `lib.rs`, `Watcher` in `watcher.rs`) to keep the crate warning-free. ## Verification - `cargo test --manifest-path apps/desktop/src-tauri/Cargo.toml --locked`
## Why - The file-watching documentation listed `vault:file-created`, but the desktop watcher currently emits: - `vault:file-changed` for create/modify, - `vault:file-deleted` for deletions. - This mismatch made behaviour harder to reason about during review and debugging. ## What Changed - Updated the desktop events section in `docs/FILE_WATCHING.md` to match the actual emitted events. ## Verification - Cross-checked against `apps/desktop/src-tauri/src/watcher.rs` event emission.
… rename
## Why
- Desktop file watching still used a coarse `vault:file-changed` signal, which made downstream consumers (editor conflict handling, index refresh, tree refresh) less explicit.
- Mobile explorer had SAF rename disabled in UI even though the adapter implements rename logic, causing a core workflow regression.
## What Changed
- Desktop watcher (`apps/desktop/src-tauri/src/watcher.rs`):
- Added a known-path snapshot to classify filesystem changes as:
- `vault:file-created`
- `vault:file-modified`
- `vault:file-deleted`
- Kept hidden-path filtering rooted to vault-relative paths.
- Added/expanded unit tests for ignore rules and snapshot behaviour.
- Desktop consumers updated to semantic events:
- `FileWatcherContext` now listens to `file-created` + `file-modified` + `file-deleted`.
- `FileTree` refreshes on `file-created` + `file-modified` + `file-deleted`.
- `EditorPane` conflict/autoreload path now listens specifically to `file-modified`.
- Updated `FileWatcherContext` test expectations accordingly.
- Mobile explorer (`apps/mobile/app/vault/explorer.tsx`):
- Re-enabled rename affordance for SAF-backed vaults by removing the SAF-specific guard.
- `canRename` now follows adapter capability (`Boolean(adapter?.rename)`).
- Docs:
- Updated `docs/FILE_WATCHING.md` desktop event list to match emitted semantic events.
## Verification
- `pnpm run ci:typecheck:desktop`
- `pnpm run ci:test:desktop`
- `pnpm run ci:lint:desktop`
- `cargo test --manifest-path apps/desktop/src-tauri/Cargo.toml --locked`
- `pnpm --filter @liminal-notes/mobile run typecheck`
b30fc4f to
6842133
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6842133f46
ℹ️ 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".
24f38a4 to
6842133
Compare
…tests ## Why - Mobile file watching was hard-wired to `MobileSandboxVaultAdapter`, so SAF/bookmark vaults did not receive watcher-driven updates. - Desktop file tree refresh listened to `vault:file-modified`, causing expensive full refreshes on ordinary content edits. - These behaviours were flagged in review as performance and correctness regressions. ## What Changed - Mobile watcher binding: - Introduced `FileWatcherServiceCore` in `apps/mobile/src/services/fileWatcherCore.ts` for adapter-agnostic watcher logic. - Kept `apps/mobile/src/services/FileWatcher.ts` as the React Native wrapper. - Added `bindVault(vault, vaultId)` support and wired `IndexContext` to bind the active vault adapter. - Added `apps/mobile/src/context/fileWatcherBinding.ts` helper and used it in `IndexContext`. - Desktop file tree refresh: - Updated `FileTree` watcher subscriptions to refresh only on `vault:file-created` and `vault:file-deleted`. - Removed refresh subscription for `vault:file-modified`. - Added persistent tests: - `apps/desktop/src/components/FileTree.test.ts` - `apps/mobile/src/services/FileWatcher.test.ts` - `apps/mobile/src/context/fileWatcherBinding.test.ts` ## Verification - `pnpm --filter @liminal-notes/mobile run typecheck` - `pnpm --filter @liminal-notes/mobile exec vitest run src/services/FileWatcher.test.ts src/context/fileWatcherBinding.test.ts` - `pnpm run ci:typecheck:desktop` - `pnpm run ci:test:desktop`
Implemented file watching for both Desktop and Mobile to detect external changes. Desktop uses the
notifycrate for real-time events, while Mobile uses a polling strategy on focus. Both platforms support incremental indexing updates and handle editor conflicts with a banner UI.PR created automatically by Jules for task 16839906447103982705 started by @ScottMorris