From bb8ecafb11d8c9aaec39aad67b53c5bb89f0e467 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 13 Aug 2026 15:32:27 +0800 Subject: [PATCH 1/5] refactor(cli): unify daemon lifecycle management --- .../issues/01-foreground-daemon-ownership.md | 19 + .../issues/02-cross-platform-user-service.md | 23 + .../issues/03-daemon-status-and-logs.md | 18 + .../04-authentication-owned-lifecycle.md | 18 + .../issues/05-daemon-recovery-guidance.md | 15 + .../issues/06-contract-legacy-lifecycle.md | 19 + .scratch/daemon-service-management/spec.md | 180 ++++ AGENTS.md | 2 +- Cargo.lock | 184 +++- Cargo.toml | 2 + README.md | 44 +- dist-workspace.toml | 2 +- docs/daemon.md | 106 +++ flicknote-cli/Cargo.toml | 11 +- flicknote-cli/src/bin/flicknote-sync.rs | 5 - flicknote-cli/src/commands/daemon.rs | 851 ++++++++++++------ .../src/commands/daemon_lifecycle.rs | 673 ++++++++++++++ flicknote-cli/src/commands/login.rs | 290 ++++-- flicknote-cli/src/commands/logout.rs | 190 +++- flicknote-cli/src/commands/mod.rs | 3 +- flicknote-cli/src/commands/service_manager.rs | 551 ++++++++++++ flicknote-cli/src/commands/sync.rs | 344 ------- flicknote-cli/src/help/root.md | 2 +- flicknote-cli/src/help/share.md | 2 +- flicknote-cli/src/help/unshare.md | 2 +- flicknote-cli/src/main.rs | 24 +- flicknote-cli/src/main_tests.rs | 22 + flicknote-cli/src/mcp/server.rs | 10 +- flicknote-cli/tests/daemon_process.rs | 321 +++++++ flicknote-cli/tests/mcp_stdio.rs | 4 +- flicknote-core/src/config.rs | 2 + flicknote-core/src/services/error.rs | 4 +- flicknote-sync/Cargo.toml | 6 +- flicknote-sync/src/ipc/client.rs | 94 +- flicknote-sync/src/ipc/mod.rs | 5 +- flicknote-sync/src/ipc/protocol.rs | 47 +- flicknote-sync/src/ipc/server.rs | 130 ++- flicknote-sync/src/ipc/tests.rs | 77 +- flicknote-sync/src/lib.rs | 3 +- flicknote-sync/src/ownership.rs | 112 +++ flicknote-sync/src/runtime.rs | 534 ++++++++--- flicknote-sync/src/storage_maintenance.rs | 86 +- flicknote-sync/src/test_support.rs | 1 + flicknote-sync/tests/app_contract.rs | 19 +- justfile | 12 +- skills/flicknote.md | 4 + 46 files changed, 4088 insertions(+), 985 deletions(-) create mode 100644 .scratch/daemon-service-management/issues/01-foreground-daemon-ownership.md create mode 100644 .scratch/daemon-service-management/issues/02-cross-platform-user-service.md create mode 100644 .scratch/daemon-service-management/issues/03-daemon-status-and-logs.md create mode 100644 .scratch/daemon-service-management/issues/04-authentication-owned-lifecycle.md create mode 100644 .scratch/daemon-service-management/issues/05-daemon-recovery-guidance.md create mode 100644 .scratch/daemon-service-management/issues/06-contract-legacy-lifecycle.md create mode 100644 .scratch/daemon-service-management/spec.md create mode 100644 docs/daemon.md delete mode 100644 flicknote-cli/src/bin/flicknote-sync.rs create mode 100644 flicknote-cli/src/commands/daemon_lifecycle.rs create mode 100644 flicknote-cli/src/commands/service_manager.rs delete mode 100644 flicknote-cli/src/commands/sync.rs create mode 100644 flicknote-cli/tests/daemon_process.rs create mode 100644 flicknote-sync/src/ownership.rs diff --git a/.scratch/daemon-service-management/issues/01-foreground-daemon-ownership.md b/.scratch/daemon-service-management/issues/01-foreground-daemon-ownership.md new file mode 100644 index 0000000..8ea6bbc --- /dev/null +++ b/.scratch/daemon-service-management/issues/01-foreground-daemon-ownership.md @@ -0,0 +1,19 @@ +# 01 — Foreground daemon ownership and graceful shutdown + +**What to build:** Make `flicknote daemon run` the reliable foreground daemon entry point, with exclusive ownership of its configured data directory and one bounded shutdown path for terminal and service signals. + +**Blocked by:** None — can start immediately. + +Status: ready-for-agent + +- [ ] `flicknote daemon run` requires a valid login and runs synchronously without forking, detaching, creating a new session, or redirecting terminal output. +- [ ] The daemon obtains a non-blocking advisory exclusive lock for its configured data directory before touching its socket or PowerSync SQLite database, and holds the lock for its full lifetime. +- [ ] A second daemon for the same data directory fails promptly with actionable ownership diagnostics and does not unlink the active daemon's socket or open its database. +- [ ] Daemons using different isolated data directories can run concurrently. +- [ ] Lock ownership is released automatically after graceful exit, panic, or forced process termination without requiring lock-file deletion. +- [ ] Socket cleanup and binding occur only after lock acquisition, so stale endpoints can be cleaned without racing a live owner. +- [ ] SIGINT and SIGTERM enter the same graceful-shutdown coordinator instead of relying on default process termination. +- [ ] Shutdown stops new IPC work, bounds in-flight IPC, bounds PowerSync disconnect, attempts a bounded WAL truncate checkpoint, releases resources, and exits within an approximately eight-second total budget. +- [ ] A stalled disconnect or checkpoint is logged by stage but cannot prevent process exit. +- [ ] Explicit shutdown exits successfully; unexpected actor termination or panic remains distinguishable as process failure. +- [ ] Isolated real-process tests prove single ownership, stale socket handling, SIGINT/SIGTERM cleanup, bounded shutdown, forced-release recovery, and subsequent restart without touching user data or services. diff --git a/.scratch/daemon-service-management/issues/02-cross-platform-user-service.md b/.scratch/daemon-service-management/issues/02-cross-platform-user-service.md new file mode 100644 index 0000000..3b62da5 --- /dev/null +++ b/.scratch/daemon-service-management/issues/02-cross-platform-user-service.md @@ -0,0 +1,23 @@ +# 02 — Cross-platform user service lifecycle + +**What to build:** Let users install and control the foreground daemon as a user-level launchd service on macOS or systemd service on Linux through one `flicknote daemon` interface. + +**Blocked by:** 01 — Foreground daemon ownership and graceful shutdown. + +Status: ready-for-agent + +- [ ] A maintained `service-manager` adapter replaces direct platform command construction for user-level launchd and systemd lifecycle operations. +- [ ] `daemon install` installs a missing service, starts it, and reports success only after compatible IPC readiness is observed. +- [ ] Re-running `daemon install` reconciles changed service configuration or executable location, ensures the service is started, and succeeds without creating duplicate services. +- [ ] Installed services execute the same `flicknote daemon run` entry point used for foreground diagnosis. +- [ ] Installation preserves a stable package-manager symlink entry point when invoked through one rather than canonicalizing it into a versioned package directory. +- [ ] The selected executable is validated as existing, executable, and FlickNote-owned before service installation. +- [ ] `daemon start` starts only an installed service and waits for readiness; it does not install a missing service. +- [ ] `daemon stop` stops without uninstalling autostart configuration and waits until application readiness is gone. +- [ ] `daemon restart` restarts only an installed service and waits for readiness. +- [ ] `daemon uninstall` stops and removes the installed service. +- [ ] Readiness requires a compatible IPC health response after local application initialization but does not require remote PowerSync connectivity or a completed sync cycle. +- [ ] Protocol incompatibility reports CLI and daemon version/protocol details when available; exact package-version equality is not required. +- [ ] Services autostart and restart unexpected failures with reasonable platform-supported delay, while explicit shutdown and permanent startup errors do not create a tight restart loop. +- [ ] Adapter-level tests cover user-service selection, lifecycle state transitions, reconciliation, stable executable selection, readiness, and error translation without testing dependency internals. +- [ ] Bounded isolated launchd/systemd user-service system tests run where CI or the host supports them, use unique labels and data roots, and always clean up. diff --git a/.scratch/daemon-service-management/issues/03-daemon-status-and-logs.md b/.scratch/daemon-service-management/issues/03-daemon-status-and-logs.md new file mode 100644 index 0000000..6a344f3 --- /dev/null +++ b/.scratch/daemon-service-management/issues/03-daemon-status-and-logs.md @@ -0,0 +1,18 @@ +# 03 — Daemon status and logs experience + +**What to build:** Give users concise routine status, detailed and machine-readable diagnosis, and one logs interface that hides launchd/systemd differences. + +**Blocked by:** 02 — Cross-platform user service lifecycle. + +Status: ready-for-agent + +- [ ] Default `daemon status` output is one concise line for a ready daemon and includes an actionable recovery command automatically when unhealthy. +- [ ] `daemon status --verbose` separately reports service installation/running state, application readiness, FlickNote version, IPC protocol, PowerSync connection state, last observed error, and log guidance. +- [ ] `daemon status --json` always emits an object-root result with stable required fields and explicit enums for service, application, and sync states. +- [ ] JSON status represents unavailable observations predictably rather than silently changing the result shape. +- [ ] Status distinguishes at least not installed, installed/stopped, service running/application unavailable, ready/offline, ready/connected, protocol incompatible, and service-manager query failure. +- [ ] Unhealthy status still prints the requested human or JSON diagnosis and then exits nonzero; ready status exits successfully. +- [ ] `daemon logs` shows bounded recent managed-daemon logs on macOS and Linux without requiring users to know launchd or journal commands. +- [ ] `daemon logs --lines` controls the bounded history and `daemon logs --follow` streams new output. +- [ ] Managed macOS logs use the FlickNote data-directory log destination; managed Linux logs use the systemd user journal; foreground run continues writing to its terminal. +- [ ] Contract tests cover status JSON schema and enums, while behavioral tests cover human output, exit outcomes, service/application disagreement, and both logging backends. diff --git a/.scratch/daemon-service-management/issues/04-authentication-owned-lifecycle.md b/.scratch/daemon-service-management/issues/04-authentication-owned-lifecycle.md new file mode 100644 index 0000000..d778d0d --- /dev/null +++ b/.scratch/daemon-service-management/issues/04-authentication-owned-lifecycle.md @@ -0,0 +1,18 @@ +# 04 — Authentication-owned daemon lifecycle + +**What to build:** Make login establish a ready authenticated daemon and make logout remove it before credentials and local data are cleared, with explicit behavior for partial failures. + +**Blocked by:** 02 — Cross-platform user service lifecycle. + +Status: ready-for-agent + +- [ ] Successful login authenticates, reconciles the user service, starts it, waits for readiness, and prints concise authentication and daemon-ready confirmations. +- [ ] If authentication succeeds but service installation or readiness fails, the valid session is retained, the command identifies the failed daemon stage, recommends `daemon status --verbose`, and exits nonzero. +- [ ] `login --force` stops and uninstalls the current service before removing the prior session and beginning new authentication. +- [ ] Successful forced login reconciles and verifies a service using the new session. +- [ ] Failed forced authentication does not restore the old session or old service and leaves a clear logged-out state. +- [ ] Normal logout stops and uninstalls the service and confirms it is stopped before deleting the session and local database files. +- [ ] Normal logout preserves the session and local data if service stop or uninstall cannot be confirmed. +- [ ] `logout --force` explicitly permits session and local-data cleanup after service cleanup failure and reports the unresolved service state. +- [ ] Daemon installation and foreground execution reject missing authentication rather than creating an unauthenticated persistent process. +- [ ] Lifecycle orchestration tests cover login success, authentication failure, post-auth install/readiness failure, forced-login ordering and outcomes, logout success, cleanup failures, and forced logout. diff --git a/.scratch/daemon-service-management/issues/05-daemon-recovery-guidance.md b/.scratch/daemon-service-management/issues/05-daemon-recovery-guidance.md new file mode 100644 index 0000000..de7ad7e --- /dev/null +++ b/.scratch/daemon-service-management/issues/05-daemon-recovery-guidance.md @@ -0,0 +1,15 @@ +# 05 — Daemon recovery guidance for CLI and MCP clients + +**What to build:** Make every daemon-dependent CLI and MCP entry point fail consistently and safely when the application is unavailable, while preserving local-first operation when only remote sync is offline. + +**Blocked by:** 03 — Daemon status and logs experience. + +Status: ready-for-agent + +- [ ] Daemon-dependent CLI commands report that the daemon is unavailable and recommend `flicknote daemon status` and `flicknote daemon start` as appropriate. +- [ ] MCP startup and daemon-dependent MCP operations expose consistent actionable unavailability diagnostics without leaking platform-specific service details. +- [ ] Data commands and MCP never install, start, restart, or otherwise mutate OS service state implicitly. +- [ ] Data commands and MCP never fall back to opening the PowerSync SQLite database directly. +- [ ] A ready local application remains usable and reports ready when PowerSync is disconnected or the network is unavailable. +- [ ] Transient remote connectivity failures stay inside PowerSync's reconnect/backoff behavior and do not churn the OS service process. +- [ ] Behavioral tests prove recovery guidance, absence of implicit service mutations, absence of direct database fallback, and local readiness during remote outage. diff --git a/.scratch/daemon-service-management/issues/06-contract-legacy-lifecycle.md b/.scratch/daemon-service-management/issues/06-contract-legacy-lifecycle.md new file mode 100644 index 0000000..c2e4631 --- /dev/null +++ b/.scratch/daemon-service-management/issues/06-contract-legacy-lifecycle.md @@ -0,0 +1,19 @@ +# 06 — Contract legacy lifecycle and unify distribution + +**What to build:** Finish the daemon-management replacement by removing every legacy lifecycle path, shipping one executable and one vocabulary, documenting the clean pre-upgrade boundary, and verifying the complete behavior in one pull request. + +**Blocked by:** 01 — Foreground daemon ownership and graceful shutdown; 02 — Cross-platform user service lifecycle; 03 — Daemon status and logs experience; 04 — Authentication-owned daemon lifecycle; 05 — Daemon recovery guidance for CLI and MCP clients. + +Status: ready-for-agent + +- [ ] The `sync` command namespace is removed without a compatibility alias, and parser tests accept every agreed `daemon` command and option while rejecting the removed namespace. +- [ ] Custom PID-file lifecycle decisions, PID signaling, process-name scanning, SIGKILL stopgaps, detached background startup, direct launchctl handling, and hand-generated service files are removed. +- [ ] Lifecycle artifacts consistently use daemon terminology for service label, lock, and socket; old sync-named artifacts are not retained as runtime compatibility paths. +- [ ] The separately distributed daemon executable and sibling-binary discovery are removed; release metadata ships only the unified `flicknote` executable. +- [ ] No CLI or MCP code path opens SQLite directly, and the daemon remains the sole local backend and PowerSync database owner. +- [ ] User documentation covers login/logout symmetry, every `daemon` command, foreground diagnosis, status/logs usage, macOS and Linux user services, and recovery guidance. +- [ ] Upgrade documentation instructs installations using the old lifecycle to run the old version's uninstall command before installing this release; no automatic migration is added. +- [ ] Agent-facing workflow/reference documentation uses the new daemon vocabulary and commands where future implementation or verification depends on them. +- [ ] The final diff contains no temporary debug instrumentation or obsolete lifecycle tests, and replacement tests assert runtime/public contracts rather than deleted source shape. +- [ ] Workspace formatting, tests, checks, Clippy with warnings denied, strict MCP schema contracts, and the smallest available macOS/Linux service behavioral probes all pass. +- [ ] The staged final diff is reviewed as one coherent feature intended for a single pull request. diff --git a/.scratch/daemon-service-management/spec.md b/.scratch/daemon-service-management/spec.md new file mode 100644 index 0000000..aaf4bb4 --- /dev/null +++ b/.scratch/daemon-service-management/spec.md @@ -0,0 +1,180 @@ +Status: ready-for-agent + +## Problem Statement + +FlickNote currently manages its sync daemon with hand-written PID-file logic, direct Unix signals, manually generated launchd property lists, direct `launchctl` commands, detached child processes, socket-file cleanup, and health polling. These mechanisms do not share a reliable source of truth. + +A daemon can remain alive after its PID file is removed, and a second daemon can delete the first daemon's socket and open the same PowerSync SQLite database. This has produced simultaneous database owners and repeated SQLite `BUSY` failures. The daemon also listens only for terminal interrupt (`SIGINT`), while launchd and normal service-stop operations use termination (`SIGTERM`). Consequently, service stops bypass the daemon's PowerSync disconnect and WAL checkpoint path. + +The current CLI exposes these operations under `flicknote sync`, even though they manage a long-running daemon rather than request an immediate synchronization. The implementation supports launchd only and would require another custom lifecycle implementation for systemd. Users should not need to understand platform-specific service commands or repair PID/socket inconsistencies. + +## Solution + +Replace FlickNote's custom process and launchd lifecycle management with the maintained `service-manager` abstraction, using user-level launchd services on macOS and user-level systemd services on Linux. The operating-system service manager is the source of truth for installation and process lifecycle; FlickNote's IPC health endpoint remains the source of truth for application readiness. + +Replace the `flicknote sync` command family with `flicknote daemon`. Login automatically reconciles, starts, and verifies the user service. Logout stops and uninstalls the service before removing the session and local data. A foreground-only `flicknote daemon run` entry point supports development and diagnosis without detaching or creating a second lifecycle model. + +Protect each FlickNote data directory with a kernel-managed advisory exclusive file lock held for the daemon's full lifetime. The lock, rather than a PID file or socket existence, enforces single ownership of the PowerSync SQLite database. Socket cleanup is permitted only after the process acquires that lock. + +Handle `SIGINT` and `SIGTERM` as inputs to one bounded graceful-shutdown sequence. Stop accepting IPC work, bound in-flight work, disconnect PowerSync, attempt a bounded WAL checkpoint, release resources, and exit. Maintenance failures are recorded but do not leave the service stuck indefinitely. + +Provide concise normal output, detailed and JSON status modes, and a cross-platform logs command so users do not need to know launchd or systemd details. + +## User Stories + +1. As a FlickNote user, I want login to start everything required for note commands, so that I do not need a separate daemon setup step. +2. As a FlickNote user, I want logout to stop and remove the daemon service, so that no authenticated background process remains after logout. +3. As a FlickNote user, I want daemon installation to work on macOS, so that FlickNote starts automatically through my launchd user session. +4. As a FlickNote user, I want daemon installation to work on Linux, so that FlickNote starts automatically through my systemd user session. +5. As a FlickNote user, I want the same daemon commands on macOS and Linux, so that I do not need platform-specific service knowledge. +6. As a FlickNote user, I want daemon management commands to be named `daemon`, so that their purpose is clear and is not confused with an immediate sync operation. +7. As a FlickNote user, I want `daemon install` to install, start, and verify the service, so that success means FlickNote is actually usable. +8. As a FlickNote user, I want repeated `daemon install` calls to reconcile the installed service, so that I can repair configuration and executable-path changes safely. +9. As a FlickNote user, I want `daemon start` to start an installed service without silently installing one, so that command side effects remain predictable. +10. As a FlickNote user, I want `daemon stop` to stop the service without uninstalling autostart configuration, so that I can pause FlickNote temporarily. +11. As a FlickNote user, I want `daemon restart` to restart an installed service and wait for readiness, so that I have a reliable recovery command. +12. As a FlickNote user, I want `daemon uninstall` to stop and remove the service, so that it will not return on my next login. +13. As a developer, I want `daemon run` to run synchronously in the foreground, so that I can observe logs and stop it with my terminal. +14. As a developer, I want foreground daemon execution to avoid forking, detaching, or creating a session, so that its lifetime remains attached to my shell. +15. As a developer, I want both Ctrl-C and normal service termination to use the same shutdown path, so that foreground and managed execution behave consistently. +16. As a FlickNote user, I want only one daemon to own a data directory, so that concurrent processes cannot corrupt or starve the SQLite workload. +17. As a FlickNote user, I want a crashed or forcibly killed daemon to release single-instance ownership automatically, so that stale metadata does not block recovery. +18. As a developer, I want separate XDG data directories to permit separate daemon instances, so that isolated development and testing environments remain possible. +19. As a FlickNote user, I want a clear error when another daemon owns my data directory, so that I know why startup was refused. +20. As a FlickNote user, I want lock-conflict errors to show safe diagnostic metadata when available, so that I can identify the owner without FlickNote automatically killing it. +21. As a FlickNote user, I want `daemon run` to tell me how to stop an installed daemon when ownership conflicts, so that I can switch to foreground diagnosis safely. +22. As a FlickNote user, I want daemon startup success to require a valid IPC health response, so that a merely running but unusable process is not reported as ready. +23. As a FlickNote user, I want network disconnection not to prevent daemon readiness, so that local-first operations remain available offline. +24. As a FlickNote user, I want protocol incompatibility between the CLI and daemon to produce a clear error, so that I do not unknowingly use an incompatible process. +25. As a FlickNote user, I want compatible CLI and daemon package versions to interoperate even when their version strings differ, so that protocol compatibility—not incidental version equality—governs operation. +26. As a FlickNote user, I want `daemon status` to provide a concise healthy summary, so that routine checks are easy to read. +27. As a FlickNote user, I want `daemon status --verbose` to distinguish service state, application readiness, version, protocol, and sync state, so that failures can be diagnosed. +28. As an automation author, I want `daemon status --json` to return a stable object, so that scripts can inspect daemon state without parsing human text. +29. As an automation author, I want status to return a nonzero exit status when the application is not ready, so that health checks can fail reliably. +30. As a FlickNote user, I want `daemon logs` to show recent daemon logs on both macOS and Linux, so that I do not need to learn launchd and journal commands. +31. As a FlickNote user, I want `daemon logs --follow` to stream logs, so that I can observe startup and synchronization failures in real time. +32. As a FlickNote user, I want to choose the number of recent log lines, so that diagnostics remain bounded. +33. As a FlickNote user, I want a failed data command to recommend `daemon status` and `daemon start`, so that recovery is obvious. +34. As a FlickNote user, I want data commands never to install or start services implicitly, so that read and mutation commands do not change OS service state. +35. As a FlickNote user, I want data commands and the MCP server never to fall back to opening SQLite directly, so that the daemon remains the sole database owner. +36. As a FlickNote user, I want successful login to report authentication and daemon readiness concisely, so that I know FlickNote is usable. +37. As a FlickNote user, I want authentication to remain valid if daemon installation fails, so that I can retry service setup without logging in again. +38. As a FlickNote user, I want login to return a failure when authentication succeeds but daemon readiness fails, so that partial setup is not presented as complete success. +39. As a FlickNote user, I want `login --force` to stop the old service before replacing my session, so that the daemon never continues with superseded credentials. +40. As a FlickNote user, I want successful forced login to reinstall and verify the service, so that reauthentication restores the complete system. +41. As a FlickNote user, I want a failed forced authentication to leave me clearly logged out, so that an old session is not silently resurrected. +42. As a FlickNote user, I want normal logout to preserve my session if service shutdown or uninstall fails, so that a running daemon is not left with credentials removed underneath it. +43. As a FlickNote user, I want `logout --force` to let me clear the session and local data despite service cleanup failure, so that I retain an explicit emergency escape hatch. +44. As a FlickNote user, I want daemon installation and foreground execution to require a valid login, so that an unauthenticated background process is not created. +45. As a FlickNote user, I want transient network errors to be retried inside the running daemon, so that the OS service manager does not churn the process during offline periods. +46. As a FlickNote user, I want daemon crashes and unexpected actor exits to trigger OS-managed restart, so that the service recovers from transient internal failures. +47. As a FlickNote user, I want permanent configuration and authentication errors not to cause an infinite restart loop, so that logs and system resources are not flooded. +48. As a FlickNote user, I want service restart attempts to use a reasonable delay, so that repeated crashes do not create a tight loop. +49. As a FlickNote user, I want graceful shutdown to have a hard upper bound, so that stop, logout, restart, and upgrade operations cannot hang indefinitely. +50. As a FlickNote user, I want PowerSync disconnect to be attempted during shutdown, so that synchronization actors can stop cleanly. +51. As a FlickNote user, I want a shutdown WAL checkpoint to be attempted but not required for exit, so that normal SQLite WAL durability does not become a shutdown deadlock. +52. As a FlickNote user, I want shutdown logs to identify the stage that timed out or failed, so that PowerSync and SQLite issues can be distinguished. +53. As a Homebrew user, I want the service to reference a stable executable entry point, so that package upgrades do not leave launchd pointing into a removed Cellar version. +54. As a FlickNote user, I want the managed service and foreground command to execute the same daemon entry point, so that version and behavior cannot drift between binaries. +55. As a FlickNote maintainer, I want one distributed executable rather than separate CLI and daemon executables, so that packaging and upgrades have a single source of truth. +56. As a FlickNote maintainer, I want OS service state and application readiness to remain separate concepts, so that diagnostics accurately describe partial failures. +57. As a FlickNote maintainer, I want PID information to be diagnostic only, so that PID reuse and stale files cannot control correctness or automatic signaling. +58. As a FlickNote maintainer, I want socket cleanup to occur only while holding the data-directory lock, so that one daemon cannot unlink another live daemon's endpoint. +59. As a FlickNote maintainer, I want the service-manager adapter to own platform differences, so that FlickNote does not hand-generate launchd or systemd configuration. +60. As a FlickNote maintainer, I want observable lifecycle behavior covered at a high integration seam, so that replacing internal libraries does not invalidate the tests. + +## Implementation Decisions + +- Replace direct launchd commands and hand-written service files with the maintained `service-manager` crate. Configure user-level launchd on macOS and user-level systemd on Linux. Windows, OpenRC, rc.d, and system-level services are not promised by this feature. +- The OS service manager is the source of truth for whether a service is installed, started, stopped, or uninstalled. FlickNote must not use a PID file to infer or control managed service state. +- IPC health is the source of truth for application readiness. A service can be running while the application is unavailable; status and error messages must preserve that distinction. +- Rename the public command namespace from `sync` to `daemon`. Do not retain a compatibility alias. +- The public command family is `install`, `uninstall`, `start`, `stop`, `restart`, `status`, `logs`, and `run`. +- `install` is an idempotent reconciliation operation. It installs a missing service, updates changed service configuration or executable paths, ensures the service is started, and waits for IPC readiness. Re-running it with an equivalent configuration is successful. +- `start`, `stop`, `restart`, and `uninstall` operate only through the service manager. `start` and `restart` do not silently install a missing service. +- `run` executes the daemon synchronously in the foreground. It does not fork, detach, invoke `setsid`, redirect terminal output, or create an alternate background mode. +- Login and daemon lifecycle are symmetric. A successful login reconciles and starts the user service and waits for readiness. Logout stops and uninstalls the service before deleting the session and local database files. +- If authentication succeeds but service installation or readiness fails, retain the valid session, print that authentication succeeded and daemon startup failed, provide the status recovery command, and return nonzero. +- Forced login stops and uninstalls the current service before removing the prior session. It then authenticates, reconciles the service, and waits for readiness. Failed authentication does not restore the old session or service. +- Normal logout aborts before deleting session or local data when service stop/uninstall cannot be confirmed. Add an explicit force option that allows cleanup to continue despite that failure and clearly reports the unresolved service state. +- Daemon installation and foreground execution require a valid session. There is no supported unauthenticated daemon state. +- Data commands and MCP operations continue to require daemon health. They never open SQLite directly and never implicitly install or start the service. +- Use a kernel-managed, non-blocking advisory exclusive file lock to protect each configured data directory. Select a maintained Rust lock crate after checking current documentation and types rather than implementing raw `flock`/`fcntl` handling. +- Store the lock file in the configured FlickNote data directory under daemon terminology. Hold the open lock guard from before any socket or database ownership is acquired until daemon teardown completes. Process exit, panic, or forced termination must release the kernel lock automatically. +- Lock-file contents may contain PID, version, and start time for diagnostics. That content is not authoritative, may be stale, and must never be used to choose or signal a process. +- After obtaining the data-directory lock, the daemon may remove a stale daemon socket and bind the shared endpoint. It must never unlink the endpoint before lock ownership is established. +- Standardize lifecycle terminology on daemon. Use a daemon service label, daemon lock name, and daemon socket name. Do not preserve sync-named lifecycle artifacts for compatibility. +- Use one public `flicknote` executable. The managed service runs the same `daemon run` entry point used for foreground execution. Remove the separately distributed daemon executable and its sibling-path discovery logic. +- Preserve a stable symlink path when installation is invoked through one, rather than canonicalizing it into a versioned package-manager location. Validate that the selected executable exists, is executable, and identifies as FlickNote before installing the service. +- The release and package configuration distributes only the unified executable. +- Handle terminal interrupt and Unix termination as equivalent shutdown triggers feeding one shutdown coordinator. Platform-specific signal registration remains internal to the daemon runtime. +- Graceful shutdown uses an approximately eight-second total budget and logs stage boundaries and durations. Stop accepting new IPC immediately, allow up to approximately two seconds for in-flight IPC, allow up to approximately four seconds for PowerSync disconnect, and allow up to approximately two seconds for a shutdown WAL truncate checkpoint. Remaining cleanup releases the socket and lock as guards drop. +- A PowerSync disconnect timeout or checkpoint timeout/failure is logged and does not prevent process exit. SQLite WAL already provides durability; successful truncation is maintenance, not a correctness precondition. +- Unexpected internal actor exit, panic, and transient internal crashes produce failure semantics suitable for OS-managed restart. Explicit shutdown exits successfully. +- Permanent startup errors such as missing authentication, invalid configuration, or incompatible schema are classified separately so they do not create an unbounded restart loop. Network unavailability is not a startup failure; PowerSync retains its internal reconnect/backoff behavior while the daemon remains ready for local work. +- Configure autostart and restart-on-failure with a reasonable platform-supported delay. Explicit service stop must not cause immediate restart. +- A successful readiness check requires a valid IPC server-info response after configuration validation, database opening, backend creation, and socket serving. It does not require remote PowerSync connection or completion of a sync cycle. +- CLI/daemon compatibility is governed by the IPC protocol contract, not exact package-version equality. Status includes both values. Incompatible protocol responses fail readiness and report the executable path, CLI version/protocol, and daemon version/protocol when available. +- Default status output is one concise line. Unhealthy states include an actionable recovery command automatically. +- Verbose status distinguishes service installation/running state, application readiness, package version, IPC protocol, PowerSync connection state, last observed error, and the platform-appropriate log location or command. +- JSON status has an object root with stable fields for service state, application state, version, protocol, sync state, error details, and log guidance. Use explicit string enums for state fields and nullable object/string fields for unavailable observations; do not encode unavailable states by omitting unrelated required fields unpredictably. +- Status emits its human or JSON report even when unhealthy, then returns nonzero unless the application is ready. +- Logs provides bounded recent output by default, accepts a line-count option, and supports follow mode. On macOS it reads/follows launchd-directed stdout/stderr in the FlickNote data directory. On Linux it queries/follows the systemd user journal. Platform adaptation is internal to the command. +- Foreground run logs to the attached terminal. It does not redirect output to the managed-service log destination. +- Login success prints `Authenticated` followed by a concise daemon-ready confirmation. Partial success states identify the failed stage without exposing launchd/systemd implementation details in the normal path. +- When foreground run cannot acquire ownership because the managed daemon is active, the error tells the user to stop the daemon and retry. It does not offer or perform an automatic stop/kill option. +- Service manager errors retain enough platform context for verbose diagnostics but normal errors use FlickNote concepts and actionable `flicknote daemon` commands. +- There is no runtime migration or compatibility layer for old PID files, sync commands, sync socket names, old service labels, or manually detached daemons. Before installing the release containing this feature, development/release documentation instructs existing installations to run the old version's uninstall command. +- The temporary PID/SIGKILL stopgap on the current development branch is superseded by this design; the final implementation must remove custom PID signaling rather than layering service-manager behavior on top of it. + +## Testing Decisions + +- Tests assert observable lifecycle contracts rather than source shape, command strings, selected crate internals, generated plist/unit text, or private helper structure. +- Prefer two high seams because they cover distinct trust boundaries without duplicating low-level tests. +- The primary seam is the CLI lifecycle orchestration boundary with an injected/fake service-manager adapter and fake health endpoint. Exercise daemon install/start/stop/restart/uninstall/status behavior and login/logout composition through the same command orchestration used by the CLI. Assert calls only where they are externally meaningful state transitions; assert user-visible output, exit outcome, retained/deleted session state, and readiness behavior. +- The second seam is an isolated daemon process integration test using temporary XDG config/data roots and the real daemon entry point. It verifies exclusive ownership, stale socket handling under lock, SIGINT shutdown, SIGTERM shutdown, bounded exit, resource release, and subsequent restart. It must never touch the user's real service, session, socket, database, or logs. +- Extend the existing CLI parser test style to mechanically confirm every `daemon` subcommand and option parses and the removed `sync` namespace does not parse. This is a public CLI contract rather than a source-text test. +- Add machine-consumed contract tests for status JSON: object root, stable required fields, allowed state enums, healthy and unhealthy examples, and protocol/version representation. +- Test status behavior across at least: not installed, installed/stopped, service running/application unavailable, ready/offline, ready/connected, protocol incompatible, and service-manager query failure. +- Test that default status is concise, verbose status distinguishes service and application state, unhealthy status still emits diagnostics, and unhealthy status exits nonzero. +- Test install reconciliation across missing, equivalent, and changed service configurations. The changed configuration case must prove the current stable executable entry point is applied and the service becomes ready. +- Test that start/restart fail clearly when the service is not installed and do not install it implicitly. +- Test that data commands and MCP startup report daemon recovery guidance without attempting service installation, service start, or direct database access. +- Test login orchestration for full success, authentication failure, authentication success plus install failure, and authentication success plus readiness failure. Verify valid sessions are retained in partial-success cases. +- Test forced login ordering and outcomes: old service cleanup precedes old-session removal; failed new authentication leaves no restored old session; successful authentication reconciles and verifies the new service. +- Test logout success, stop failure, uninstall failure, and forced cleanup. Normal failure preserves session/local data; forced cleanup removes them while reporting unresolved service cleanup. +- Test foreground run rejects missing authentication before acquiring database ownership. +- Test two foreground daemon processes against one temporary data directory. The second must fail promptly without deleting the first process's socket or opening its SQLite database. After the first exits or is forcibly killed, another process must acquire ownership successfully without manual lock-file deletion. +- Test that different temporary data directories can run concurrently. +- Test lock diagnostic metadata only for user-visible diagnostics; do not test its exact serialized layout unless that layout is explicitly exposed as a machine-consumed contract. +- Test SIGINT and SIGTERM against the real isolated process. Both must enter the same shutdown sequence, release the socket and lock, and exit within the configured total budget. Capture stage logs to distinguish entering shutdown from default signal termination. +- Test a controllably stalled PowerSync disconnect and checkpoint at the runtime orchestration seam. Each timeout must allow later cleanup and process exit. Do not require real network timing or real SQLite lock contention to make these deterministic. +- Test that remote network failure does not make local application readiness fail and does not terminate the daemon. +- Test service restart classification through adapter-visible outcomes: explicit shutdown is successful; unexpected actor failure is unsuccessful; permanent startup errors do not request an endless retry path. +- Add platform adapter tests only for behavior not already guaranteed by `service-manager`, such as choosing user-service level, stable executable input, logging destination/guidance, and translating status into FlickNote's status model. Do not duplicate the dependency's launchd/systemd command-generation tests. +- Add bounded macOS and Linux system tests where CI runners permit them: install a uniquely labeled temporary user service, start it, observe readiness, stop it, and uninstall it. These tests must clean up through guards even on failure and must not use the production label or production data directory. +- Follow existing project conventions: Rust unit/integration tests, temporary directories, explicit XDG isolation, targeted package tests during iteration, then workspace format, test, check, and Clippy with warnings denied. +- The current parser tests are prior art for public CLI syntax. Existing daemon health tests are prior art for IPC readiness. Existing PowerSync actor tests are prior art for real connect/disconnect behavior. Existing temporary-directory configuration tests are prior art for XDG isolation. + +## Out of Scope + +- Supporting Windows Services, WinSW, OpenRC, rc.d, or system-level/root services. +- Preserving the `flicknote sync` command as an alias. +- Migrating or automatically cleaning old PID files, old sync sockets, old service labels, old plist files, or detached legacy processes. +- Automatically scanning for or killing processes by name or diagnostic PID metadata. +- Maintaining a standalone background-detach mode outside launchd/systemd. +- Allowing multiple daemons to share one data directory or SQLite database. +- Replacing the Unix-domain IPC protocol or changing note/MCP business operations. +- Requiring remote PowerSync connectivity before the daemon is ready. +- Guaranteeing that a shutdown WAL truncate checkpoint succeeds. +- Making exact package-version equality an IPC compatibility requirement. +- Adding direct SQLite fallback paths to CLI or MCP clients. +- Deploying, releasing, or migrating existing installations as part of implementation; release documentation only records the required pre-upgrade uninstall step. + +## Further Notes + +- Production evidence showed two daemon processes simultaneously holding the same SQLite database and separate Unix socket objects associated with the same socket path. Repeated SQLite `BUSY` errors occurred during this period. This supports treating single database ownership as a correctness requirement. +- Investigation disproved the initial assumption that PowerSync shutdown was known to hang. The existing daemon listens only through Tokio's Ctrl-C API, which maps to `SIGINT` on Unix, while the controller sends `SIGTERM`. An isolated probe showed `SIGTERM` exited without entering FlickNote shutdown, whereas `SIGINT` entered shutdown and completed PowerSync disconnect quickly. The implementation should still retain bounded shutdown stages because external dependencies must not be allowed to hang service operations. +- The repository currently has no domain glossary, context document, or architecture decision record for this area. This specification uses the established project terms daemon, user service, application readiness, IPC health, PowerSync database, and local backend. +- The repository currently distributes both `flicknote` and `flicknote-sync`; this specification intentionally collapses them into one executable and requires release metadata and documentation to follow that decision. +- The implementation agent should verify the current `service-manager` and advisory-lock crate documentation and types before adding dependencies. The architectural contract is fixed; exact dependency API usage is not. diff --git a/AGENTS.md b/AGENTS.md index 3681e0a..10bad13 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,7 +13,7 @@ Local-first note management CLI with cloud sync via PowerSync and Supabase. Rust workspace with 4 crates: -- **flicknote-cli** — CLI package (`flicknote`, `flicknote-sync`): thin CLI/MCP clients and daemon entrypoint; data commands never open SQLite or Postgres +- **flicknote-cli** — unified `flicknote` executable: thin CLI/MCP clients and foreground daemon entrypoint; data commands never open SQLite or Postgres - **flicknote-core** — Shared library (db, config, schema, types, session, services, DTOs, errors) - **flicknote-auth** — Supabase GoTrue authentication (OTP + OAuth2/PKCE) - **flicknote-sync** — Daemon application host, typed RPC boundary, backend ownership, and PowerSync ↔ Supabase sync diff --git a/Cargo.lock b/Cargo.lock index fe3c68a..6ad167a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -330,6 +330,15 @@ dependencies = [ "cc", ] +[[package]] +name = "codepage" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "48f68d061bc2828ae826206326e61251aca94c1e4a5305cf52d9138639c918b4" +dependencies = [ + "encoding_rs", +] + [[package]] name = "colorchoice" version = "1.0.4" @@ -515,13 +524,33 @@ dependencies = [ "crypto-common", ] +[[package]] +name = "dirs" +version = "4.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ca3aa72a6f96ea37bbc5aa912f6788242832f75369bdfdadcb0e38423f100059" +dependencies = [ + "dirs-sys 0.3.7", +] + [[package]] name = "dirs" version = "6.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c3e8aa94d75141228480295a7d0e7feb620b1a5ad9f12bc40be62411e38cce4e" dependencies = [ - "dirs-sys", + "dirs-sys 0.5.0", +] + +[[package]] +name = "dirs-sys" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1b1d1d91c932ef41c0f2663aa8b0ca0342d444d842c06914aa0a7e352d0bada6" +dependencies = [ + "libc", + "redox_users 0.4.6", + "winapi", ] [[package]] @@ -532,7 +561,7 @@ checksum = "e01a3366d27ee9890022452ee61b2b63a67e6f13f58900b651ff5665f0bb1fab" dependencies = [ "libc", "option-ext", - "redox_users", + "redox_users 0.5.2", "windows-sys 0.61.2", ] @@ -565,6 +594,17 @@ version = "1.15.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "48c757948c5ede0e46177b7add2e67155f70e33c07fea8284df6576da70b3719" +[[package]] +name = "encoding-utils" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "87b881ab2524b96a5ce932056c7482ba6152e2226fed3936b3e592adeb95ca6d" +dependencies = [ + "codepage", + "encoding_rs", + "windows-sys 0.52.0", +] + [[package]] name = "encoding_rs" version = "0.8.35" @@ -681,7 +721,7 @@ dependencies = [ "async-trait", "chrono", "clap", - "dirs", + "dirs 6.0.0", "env_logger", "flicknote-auth", "flicknote-core", @@ -698,7 +738,9 @@ dependencies = [ "schemars 1.2.1", "serde", "serde_json", + "service-manager", "tempfile", + "thiserror 2.0.18", "tokio", "url", "uuid", @@ -710,7 +752,7 @@ version = "1.0.0" dependencies = [ "async-trait", "chrono", - "dirs", + "dirs 6.0.0", "flicknote-auth", "log", "powersync", @@ -735,8 +777,8 @@ dependencies = [ "chrono", "flicknote-auth", "flicknote-core", + "fs4", "futures-lite", - "libc", "log", "powersync", "reqwest 0.13.2", @@ -744,6 +786,7 @@ dependencies = [ "serde", "serde_json", "tempfile", + "thiserror 2.0.18", "tokio", "uuid", ] @@ -775,6 +818,16 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs4" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e72ed92b67c146290f88e9c89d60ca163ea417a446f61ffd7b72df3e7f1dfd5" +dependencies = [ + "rustix 1.1.4", + "windows-sys 0.61.2", +] + [[package]] name = "fs_extra" version = "1.3.0" @@ -1011,6 +1064,15 @@ version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" +[[package]] +name = "home" +version = "0.5.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc627f471c528ff0c4a49e1d5e60450c8f6461dd6d10ba9dcd3a61d3dff7728d" +dependencies = [ + "windows-sys 0.61.2", +] + [[package]] name = "http" version = "1.4.0" @@ -1473,6 +1535,12 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2e126dda6f34391ab7b444f9922055facc83c07a910da3eb16f1e4d9c45dc777" +[[package]] +name = "linux-raw-sys" +version = "0.4.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d26c52dbd32dccf2d10cac7725f8eae5296885fb5703b261f7d0a0739ec807ab" + [[package]] name = "linux-raw-sys" version = "0.12.1" @@ -1660,6 +1728,19 @@ version = "0.3.32" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7edddbd0b52d732b21ad9a5fab5c704c14cd949e5e9a1ec5929a24fded1b904c" +[[package]] +name = "plist" +version = "1.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7da1d65da6dd5d1e44199ac0f58712d241c0f439f80adea8924d832384087f85" +dependencies = [ + "base64", + "indexmap 2.13.0", + "quick-xml", + "serde", + "time", +] + [[package]] name = "portable-atomic" version = "1.13.1" @@ -1789,6 +1870,15 @@ dependencies = [ "unicase", ] +[[package]] +name = "quick-xml" +version = "0.41.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e660451e55124f798a69a5af3f49ccfbefbd41910eefd25caf2393e1f3473ec1" +dependencies = [ + "memchr", +] + [[package]] name = "quinn" version = "0.11.9" @@ -1895,6 +1985,17 @@ dependencies = [ "getrandom 0.3.4", ] +[[package]] +name = "redox_users" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba009ff324d1fc1b900bd1fdb31564febe58a8ccc8a6fdbb93b543d33b13ca43" +dependencies = [ + "getrandom 0.2.17", + "libredox", + "thiserror 1.0.69", +] + [[package]] name = "redox_users" version = "0.5.2" @@ -2123,6 +2224,19 @@ version = "2.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "357703d41365b4b27c590e3ed91eabb1b663f07c4c084095e60cbed4362dff0d" +[[package]] +name = "rustix" +version = "0.38.44" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fdb5bc1ae2baa591800df16c9ca78619bf65c0488b41b96ccec5d11220d8c154" +dependencies = [ + "bitflags", + "errno", + "libc", + "linux-raw-sys 0.4.15", + "windows-sys 0.52.0", +] + [[package]] name = "rustix" version = "1.1.4" @@ -2132,7 +2246,7 @@ dependencies = [ "bitflags", "errno", "libc", - "linux-raw-sys", + "linux-raw-sys 0.12.1", "windows-sys 0.61.2", ] @@ -2412,6 +2526,22 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "service-manager" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9ff6975a4ea07dda326cf122fcc1b5cf6266daad42d1442a666a99fe50f0de9" +dependencies = [ + "cfg-if", + "dirs 4.0.0", + "encoding-utils", + "encoding_rs", + "log", + "plist", + "which", + "xml-rs", +] + [[package]] name = "sha2" version = "0.10.9" @@ -2563,7 +2693,7 @@ dependencies = [ "fastrand", "getrandom 0.4.2", "once_cell", - "rustix", + "rustix 1.1.4", "windows-sys 0.61.2", ] @@ -3078,6 +3208,34 @@ dependencies = [ "rustls-pki-types", ] +[[package]] +name = "which" +version = "4.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "87ba24419a2078cd2b0f2ede2691b6c66d8e47836da3b6db8265ebad47afbfc7" +dependencies = [ + "either", + "home", + "once_cell", + "rustix 0.38.44", +] + +[[package]] +name = "winapi" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419" +dependencies = [ + "winapi-i686-pc-windows-gnu", + "winapi-x86_64-pc-windows-gnu", +] + +[[package]] +name = "winapi-i686-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" + [[package]] name = "winapi-util" version = "0.1.11" @@ -3087,6 +3245,12 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "winapi-x86_64-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" + [[package]] name = "windows-core" version = "0.62.2" @@ -3473,6 +3637,12 @@ version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9edde0db4769d2dc68579893f2306b26c6ecfbe0ef499b013d731b7b9247e0b9" +[[package]] +name = "xml-rs" +version = "0.8.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e450f9b2ed1dff33c94c12589a87338689467b9c4f5d8a5710bd09a847d2c8a7" + [[package]] name = "yaml_serde" version = "0.10.4" diff --git a/Cargo.toml b/Cargo.toml index 68bf4d2..cc60bb0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,8 @@ futures-lite = "2" anyhow = "1" toml = "0.8" dirs = "6" +service-manager = "0.11" +fs4 = "1.1" [workspace.lints.rust] unsafe_code = "warn" diff --git a/README.md b/README.md index 1090c01..1f958d6 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ Daemon-backed note management CLI with local-first sync. The CLI and MCP server - **MCP server** — typed local note, source, and project tools over stdio - **Archive notes** — archive and unarchive - **Authentication** — email OTP or OAuth (Google/Apple) via Supabase -- **Background sync** — daemon process with launchd integration (macOS) +- **User daemon service** — foreground daemon managed by launchd (macOS) or systemd (Linux) ## Build @@ -47,7 +47,7 @@ cargo install --path flicknote-cli brew install GuionAI/tap/flicknote ``` -Installs both `flicknote` and `flicknote-sync`. +Installs the unified `flicknote` executable. ## Release @@ -107,16 +107,40 @@ echo "more content" | flicknote append # Delete flicknote delete -# Manage sync daemon -flicknote sync start -# Reports the running daemon and protocol version -flicknote sync status -flicknote sync stop +# Manage the user daemon service +flicknote daemon install +flicknote daemon status +flicknote daemon logs --lines 100 +flicknote daemon stop -# Install as launchd service (macOS) -flicknote sync install +# Foreground diagnosis (runs synchronously and keeps terminal output) +flicknote daemon run + +# Reconcile/start the service after an upgrade +flicknote daemon restart +``` + +## Daemon lifecycle + +`flicknote login` authenticates and then installs, starts, and verifies the user daemon. +`flicknote logout` stops and uninstalls it before clearing the session and local database. +Use `--force` only for explicit recovery when cleanup cannot be confirmed: + +```bash +flicknote login --force +flicknote logout --force ``` +The public lifecycle commands are `daemon install`, `uninstall`, `start`, `stop`, +`restart`, `status`, `logs`, and `run`. `status --verbose` separates service +state, application readiness, IPC protocol/version, PowerSync connectivity, and +log guidance. `status --json` emits a stable object for automation. Data commands +and MCP never start services or open SQLite directly; if the daemon is unavailable, +run `flicknote daemon status` and `flicknote daemon start`. + +See [docs/daemon.md](docs/daemon.md) for macOS/Linux service details and the +pre-upgrade uninstall boundary for installations using the old lifecycle. + ## MCP server `flicknote mcp` runs a local MCP server over stdio. Configure an MCP client to @@ -161,7 +185,7 @@ Rust workspace with 4 crates: | Crate | Type | Purpose | |-------|------|---------| -| `flicknote-cli` | binary | Thin CLI/MCP clients and installable daemon binary | +| `flicknote-cli` | binary | Unified CLI/MCP client and foreground daemon executable | | `flicknote-core` | library | Database, config, shared services, DTOs, types, schema | | `flicknote-auth` | library | Supabase auth (OTP + OAuth2/PKCE) | | `flicknote-sync` | library | Application RPC host, backend ownership, and PowerSync implementation | diff --git a/dist-workspace.toml b/dist-workspace.toml index 90c5838..b422536 100644 --- a/dist-workspace.toml +++ b/dist-workspace.toml @@ -11,7 +11,7 @@ ci = "github" installers = ["homebrew"] # A GitHub repo to push Homebrew formulas to tap = "GuionAI/homebrew-tap" -# Only flicknote-cli is distributed; it bundles both flicknote and flicknote-sync. +# Only the unified flicknote executable is distributed. packages = ["flicknote-cli"] # Whether to install an updater program install-updater = false diff --git a/docs/daemon.md b/docs/daemon.md new file mode 100644 index 0000000..11d2694 --- /dev/null +++ b/docs/daemon.md @@ -0,0 +1,106 @@ +# FlickNote daemon + +FlickNote has one distributed executable. The foreground entry point and the +managed user service both run: + +```bash +flicknote daemon run +``` + +The daemon owns the local PowerSync SQLite database and its Unix IPC socket. +Data commands and MCP use IPC only; they never start a service implicitly or +open the database directly. + +## Authentication symmetry + +Login establishes a usable local installation: + +```bash +flicknote login --email you@example.com +``` + +After authentication, login reconciles the user service, starts it, and waits +for a compatible IPC health response. If service setup fails, the valid session +is retained. Inspect and retry with: + +```bash +flicknote daemon status --verbose +flicknote daemon install +``` + +Logout removes the service before deleting credentials and local database files: + +```bash +flicknote logout +``` + +If cleanup cannot be confirmed, normal logout preserves the session and local +data. `flicknote logout --force` is the explicit emergency option; it clears +local state while reporting the unresolved service cleanup. + +`flicknote login --force` stops and uninstalls the existing service before +removing the old session. A failed forced authentication does not restore the +old session or service. + +## Service commands + +The same commands select a user-level launchd service on macOS and a user-level +systemd service on Linux: + +```text +flicknote daemon install # reconcile, start, and verify +flicknote daemon start # start an installed service only +flicknote daemon stop # stop without uninstalling +flicknote daemon restart # restart an installed service only +flicknote daemon uninstall # stop and remove the service +flicknote daemon status +flicknote daemon logs --lines 100 +flicknote daemon logs --follow +flicknote daemon run # attached foreground diagnosis +``` + +Installation validates the invoked FlickNote executable and preserves its +package-manager entry-point path. Unexpected process failures are left to the +OS service manager's restart policy; explicit stops and permanent startup +errors are not treated as successful restart events. + +## Diagnosis and recovery + +Routine status is one line. Use verbose output to distinguish the service from +the local application and remote sync: + +```bash +flicknote daemon status --verbose +flicknote daemon status --json +flicknote daemon logs +``` + +A ready local application can report `offline` PowerSync state. Remote network +failure does not make local IPC unavailable. If a data command or MCP startup +reports an unavailable daemon, use `flicknote daemon status` and then +`flicknote daemon start`; no data command or MCP operation changes service state. + +`daemon run` remains attached to the terminal, writes logs to the terminal, and +handles both Ctrl-C (`SIGINT`) and service termination (`SIGTERM`) through the +same bounded shutdown coordinator. Only one daemon can own a configured data +directory at a time. The kernel lock is released automatically after a crash or +forced process termination, so no lock-file deletion is required. + +On macOS managed logs are stored in the FlickNote data directory. On Linux +managed logs are available through the systemd user journal. `daemon logs` +hides that platform difference. + +## Upgrade boundary + +This release does not migrate old lifecycle artifacts. Before installing a +release containing the unified daemon lifecycle, run the old version's cleanup +command while it is still installed: + +```bash +# Run with the old FlickNote executable: +flicknote sync uninstall +``` + +Then install the new release and run `flicknote daemon install` (or log in). +There is no compatibility alias for `flicknote sync`, no automatic PID/socket +migration, and no cleanup of detached legacy processes. diff --git a/flicknote-cli/Cargo.toml b/flicknote-cli/Cargo.toml index d91e5fb..748f574 100644 --- a/flicknote-cli/Cargo.toml +++ b/flicknote-cli/Cargo.toml @@ -11,10 +11,6 @@ license = "MIT" name = "flicknote" path = "src/main.rs" -[[bin]] -name = "flicknote-sync" -path = "src/bin/flicknote-sync.rs" - [package.metadata.dist] formula = "flicknote" @@ -26,9 +22,12 @@ clap = { version = "4", features = ["derive"] } chrono = "0.4" url = "2" serde_json = { workspace = true } -tokio = { workspace = true } -libc = "0.2" +tokio = { workspace = true, features = ["process"] } dirs = { workspace = true } +service-manager = { workspace = true } +async-trait = { workspace = true } +thiserror = { workspace = true } +libc = "0.2" reqwest = { version = "0.12", default-features = false, features = ["json", "multipart", "stream", "charset", "http2", "rustls-tls"] } futures-util = "0.3" serde = { workspace = true } diff --git a/flicknote-cli/src/bin/flicknote-sync.rs b/flicknote-cli/src/bin/flicknote-sync.rs deleted file mode 100644 index 8e5dee2..0000000 --- a/flicknote-cli/src/bin/flicknote-sync.rs +++ /dev/null @@ -1,5 +0,0 @@ -#[tokio::main] -async fn main() -> Result<(), Box> { - env_logger::init(); - flicknote_sync::run().await -} diff --git a/flicknote-cli/src/commands/daemon.rs b/flicknote-cli/src/commands/daemon.rs index 0339060..e3f4d43 100644 --- a/flicknote-cli/src/commands/daemon.rs +++ b/flicknote-cli/src/commands/daemon.rs @@ -1,285 +1,471 @@ +use super::daemon_lifecycle::{DaemonHealthProbe, IpcHealthProbe, LifecycleController}; +use super::service_manager::{ + LogGuidance, NativeServiceManager, ServiceManagerAdapter, ServiceManagerError, ServiceState, + log_guidance, show_logs, +}; +use clap::{Args, Subcommand}; use flicknote_core::config::Config; use flicknote_core::error::CliError; -use std::ffi::OsStr; -use std::fs; -use std::path::PathBuf; +use flicknote_core::services::error::ServiceError; +use serde::Serialize; #[cfg(target_os = "macos")] -use std::process::Command; +use std::fs::{self, OpenOptions}; +#[cfg(target_os = "macos")] +use std::io::{self, Write}; +#[cfg(target_os = "macos")] +use std::os::fd::AsRawFd; -const DAEMON_BINARY_NAME: &str = "flicknote-sync"; +#[derive(Args)] +pub(crate) struct DaemonArgs { + #[command(subcommand)] + command: DaemonCommand, +} -pub(crate) fn pid_file(config: &Config) -> PathBuf { - config.paths.data_dir.join("sync.pid") +#[derive(Subcommand)] +enum DaemonCommand { + /// Install, reconcile, start, and verify the user daemon service + Install, + /// Stop and remove the user daemon service + Uninstall, + /// Start an installed user daemon service + Start, + /// Stop the user daemon service without removing its installation + Stop, + /// Restart an installed user daemon service + Restart, + /// Show daemon service, application, and PowerSync status + Status(StatusArgs), + /// Show recent managed-daemon logs + Logs(LogsArgs), + /// Run the daemon synchronously in the foreground + Run, } -pub(crate) fn read_pid(config: &Config) -> Option { - let path = pid_file(config); - let content = fs::read_to_string(&path).ok()?; - let pid: u32 = content.trim().parse().ok()?; - #[allow(unsafe_code)] - if unsafe { libc::kill(pid as i32, 0) } == 0 - && process_matches_executable(pid, std::path::Path::new(DAEMON_BINARY_NAME)) - { - return Some(pid); - } - #[allow(clippy::let_underscore_must_use, clippy::let_underscore_untyped)] - let _ = fs::remove_file(&path); - None +#[derive(Args)] +struct StatusArgs { + /// Include service, application, protocol, sync, and log details + #[arg(long, conflicts_with = "json")] + verbose: bool, + /// Emit a stable object-root JSON report + #[arg(long, conflicts_with = "verbose")] + json: bool, } -#[cfg(target_os = "linux")] -fn process_executable(pid: u32) -> Option { - fs::read_link(format!("/proc/{pid}/exe")).ok() +#[derive(Args)] +struct LogsArgs { + /// Number of recent lines to display + #[arg(long, default_value_t = 100)] + lines: usize, + /// Continue streaming new log output + #[arg(long)] + follow: bool, } -#[cfg(target_os = "macos")] -fn process_executable(pid: u32) -> Option { - use std::os::unix::ffi::OsStrExt; - - let mut buffer = vec![0_u8; libc::PROC_PIDPATHINFO_MAXSIZE as usize]; - #[allow(unsafe_code)] - let length = unsafe { - libc::proc_pidpath( - pid as libc::c_int, - buffer.as_mut_ptr().cast(), - buffer.len() as u32, - ) - }; - if length <= 0 { - return None; +pub(crate) async fn run(config: &Config, args: &DaemonArgs) -> Result<(), CliError> { + match &args.command { + DaemonCommand::Install => install(config).await, + DaemonCommand::Uninstall => uninstall(config).await, + DaemonCommand::Start => start(config).await, + DaemonCommand::Stop => stop(config).await, + DaemonCommand::Restart => restart(config).await, + DaemonCommand::Status(args) => status(config, args).await, + DaemonCommand::Logs(args) => logs(config, args).await, + DaemonCommand::Run => run_foreground(config).await, } - buffer.truncate(length as usize); - Some(PathBuf::from(OsStr::from_bytes(&buffer))) } -#[cfg(not(any(target_os = "linux", target_os = "macos")))] -fn process_executable(_pid: u32) -> Option { - None +async fn install(config: &Config) -> Result<(), CliError> { + ensure_authenticated(config)?; + install_and_wait(config).await?; + println!("FlickNote daemon installed and ready"); + Ok(()) } -fn process_matches_executable(pid: u32, expected: &std::path::Path) -> bool { - process_executable(pid).and_then(|path| path.file_name().map(OsStr::to_owned)) - == expected.file_name().map(OsStr::to_owned) +pub(crate) async fn install_and_wait(config: &Config) -> Result<(), CliError> { + config.validate()?; + ensure_authenticated(config)?; + LifecycleController::native(&IpcHealthProbe) + .install_and_wait(config) + .await +} + +async fn uninstall(config: &Config) -> Result<(), CliError> { + uninstall_service(config).await?; + println!("FlickNote daemon service uninstalled"); + Ok(()) +} + +pub(crate) async fn uninstall_service(config: &Config) -> Result<(), CliError> { + LifecycleController::native(&IpcHealthProbe) + .uninstall(config) + .await + .map(|_| ()) +} + +async fn start(config: &Config) -> Result<(), CliError> { + ensure_authenticated(config)?; + LifecycleController::native(&IpcHealthProbe) + .start(config) + .await?; + println!("FlickNote daemon started and ready"); + Ok(()) } -pub(crate) fn daemon_binary() -> Result { - let exe = std::env::current_exe() - .map_err(|e| CliError::Other(format!("Could not determine executable path: {e}")))?; - let dir = exe - .parent() - .ok_or_else(|| CliError::Other("Could not determine executable directory".into()))?; - let binary = dir.join(DAEMON_BINARY_NAME); - if !binary.exists() { - return Err(CliError::Other(format!( - "Sync daemon binary not found at {}: ensure flicknote-sync is installed alongside flicknote", - binary.display() - ))); +async fn stop(config: &Config) -> Result<(), CliError> { + let was_running = LifecycleController::native(&IpcHealthProbe) + .stop(config) + .await?; + if was_running { + println!("FlickNote daemon stopped"); + } else { + println!("FlickNote daemon service is already stopped"); } - Ok(binary) + Ok(()) } -/// Stop the sync daemon if running. Returns Ok(()) even if not running. -pub(crate) fn stop(config: &Config) -> Result<(), CliError> { - #[cfg(target_os = "macos")] - { - #[allow(unsafe_code)] - let uid = unsafe { libc::getuid() }; - bootout_service(uid, service_label())?; +async fn restart(config: &Config) -> Result<(), CliError> { + ensure_authenticated(config)?; + LifecycleController::native(&IpcHealthProbe) + .restart(config) + .await?; + println!("FlickNote daemon restarted and ready"); + Ok(()) +} + +async fn status(config: &Config, args: &StatusArgs) -> Result<(), CliError> { + let report = build_status_report(config, service_state_for_status()).await; + if args.json { + println!( + "{}", + serde_json::to_string(&report).map_err(|error| CliError::Other(error.to_string()))? + ); + } else if args.verbose { + println!("{}", report.verbose_text()); + } else { + println!("{}", report.concise_text()); + } + if report.is_ready() { + Ok(()) + } else { + Err(CliError::Other("FlickNote daemon is not ready".to_string())) } +} - let Some(pid) = read_pid(config) else { - return Ok(()); - }; +async fn logs(config: &Config, args: &LogsArgs) -> Result<(), CliError> { + show_logs(config, args.lines, args.follow) + .await + .map_err(CliError::Other) +} - #[allow(unsafe_code)] - let ret = unsafe { libc::kill(pid as i32, libc::SIGTERM) }; - if ret == -1 { - let err = std::io::Error::last_os_error(); - if err.raw_os_error() == Some(libc::ESRCH) { - // Process already gone — clean up stale PID file - } else { - return Err(CliError::Other(format!( - "Failed to stop sync daemon (pid {pid}): {err}" - ))); +async fn run_foreground(config: &Config) -> Result<(), CliError> { + let managed = std::env::var_os("FLICKNOTE_DAEMON_MANAGED").is_some(); + initialize_daemon_logging(config)?; + match flicknote_sync::run(config.clone()).await { + Ok(()) => Ok(()), + Err(error) if managed && error.is_permanent_startup() => { + log::error!("Permanent daemon startup failure: {error}"); + Ok(()) } + Err(error) => Err(CliError::Other(format!("FlickNote daemon failed: {error}"))), + } +} + +fn ensure_authenticated(config: &Config) -> Result<(), CliError> { + flicknote_core::session::get_user_id(config).map(|_| ()) +} + +fn initialize_daemon_logging(config: &Config) -> Result<(), CliError> { + #[cfg(target_os = "macos")] + if std::env::var_os("FLICKNOTE_DAEMON_MANAGED").is_some() { + redirect_managed_daemon_output(config)?; + } + let mut builder = env_logger::Builder::from_env( + env_logger::Env::default().default_filter_or("flicknote_sync=info,powersync=debug"), + ); + match builder.try_init() { + Ok(()) | Err(_) => {} } - #[allow(clippy::let_underscore_must_use, clippy::let_underscore_untyped)] - let _ = fs::remove_file(pid_file(config)); Ok(()) } -/// Uninstall the launchd service. Returns Ok(()) even if not installed. #[cfg(target_os = "macos")] -pub(crate) fn uninstall() -> Result<(), CliError> { - let label = service_label(); - let plist_path = service_plist_path()?; +fn redirect_managed_daemon_output(config: &Config) -> Result<(), CliError> { + fs::create_dir_all(&config.paths.data_dir)?; + let file = OpenOptions::new() + .create(true) + .append(true) + .open(&config.paths.log_file)?; + io::stdout().flush()?; + io::stderr().flush()?; + for descriptor in [libc::STDERR_FILENO, libc::STDOUT_FILENO] { + #[allow(unsafe_code)] + if unsafe { libc::dup2(file.as_raw_fd(), descriptor) } == -1 { + return Err(CliError::Io(io::Error::last_os_error())); + } + } + Ok(()) +} - #[allow(unsafe_code)] - let uid = unsafe { libc::getuid() }; - bootout_service(uid, label)?; +fn service_state_for_status() -> Result { + let manager = NativeServiceManager::new()?; + manager.status() +} + +#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +enum ServiceStatusState { + NotInstalled, + InstalledStopped, + Running, + QueryFailed, +} + +#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +enum ApplicationStatusState { + Ready, + Unavailable, + ProtocolIncompatible, + Unknown, +} + +#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +enum SyncStatusState { + Connected, + Connecting, + Offline, + Unavailable, +} + +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] +struct VersionStatus { + cli: String, + daemon: Option, +} - if plist_path.exists() { - fs::remove_file(&plist_path)?; +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] +struct ProtocolStatus { + cli: u16, + daemon: Option, +} + +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] +struct StatusError { + code: String, + message: String, +} + +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] +struct StatusReport { + service_state: ServiceStatusState, + application_state: ApplicationStatusState, + sync_state: SyncStatusState, + sync_errors: flicknote_sync::ipc::PowerSyncErrors, + daemon_executable: Option, + version: VersionStatus, + protocol: ProtocolStatus, + error: Option, + service_error: Option, + log_guidance: LogGuidance, +} + +impl StatusReport { + fn base(config: &Config) -> Self { + Self { + service_state: ServiceStatusState::QueryFailed, + application_state: ApplicationStatusState::Unknown, + sync_state: SyncStatusState::Unavailable, + sync_errors: flicknote_sync::ipc::PowerSyncErrors::default(), + daemon_executable: None, + version: VersionStatus { + cli: env!("CARGO_PKG_VERSION").to_string(), + daemon: None, + }, + protocol: ProtocolStatus { + cli: flicknote_sync::ipc::PROTOCOL_VERSION, + daemon: None, + }, + error: None, + service_error: None, + log_guidance: log_guidance(config), + } } - Ok(()) + fn is_ready(&self) -> bool { + self.application_state == ApplicationStatusState::Ready + } + + fn concise_text(&self) -> String { + if self.is_ready() { + return format!( + "FlickNote daemon: ready (service {}, sync {})", + format_service_state(self.service_state), + format_sync_state(self.sync_state) + ); + } + let action = match self.application_state { + ApplicationStatusState::ProtocolIncompatible => "restart", + _ if self.service_state == ServiceStatusState::NotInstalled => "install", + _ => "start", + }; + format!( + "FlickNote daemon: {} — run `flicknote daemon status --verbose`, then `flicknote daemon {action}`", + format_application_state(self.application_state) + ) + } + + fn verbose_text(&self) -> String { + let error = self + .error + .as_ref() + .map(|error| format!("{}: {}", error.code, error.message)) + .unwrap_or_else(|| "none".to_string()); + let service_error = self + .service_error + .as_ref() + .map(|error| format!("{}: {}", error.code, error.message)) + .unwrap_or_else(|| "none".to_string()); + let download_error = self.sync_errors.download.as_deref().unwrap_or("none"); + let upload_error = self.sync_errors.upload.as_deref().unwrap_or("none"); + format!( + "service: {}\napplication: {}\ndaemon executable: {}\nFlickNote version: cli {}, daemon {}\nIPC protocol: cli {}, daemon {}\nPowerSync: {}\nPowerSync download error: {}\nPowerSync upload error: {}\nlast error: {}\nservice error: {}\nlogs: {}\nlog command: {}", + format_service_state(self.service_state), + format_application_state(self.application_state), + self.daemon_executable.as_deref().unwrap_or("unavailable"), + self.version.cli, + self.version.daemon.as_deref().unwrap_or("unavailable"), + self.protocol.cli, + self.protocol + .daemon + .map_or_else(|| "unavailable".to_string(), |value| value.to_string()), + format_sync_state(self.sync_state), + download_error, + upload_error, + error, + service_error, + self.log_guidance.destination, + self.log_guidance.command, + ) + } } -#[cfg(not(target_os = "macos"))] -pub(crate) fn uninstall() -> Result<(), CliError> { - Ok(()) +async fn build_status_report( + config: &Config, + service: Result, +) -> StatusReport { + let probe = IpcHealthProbe; + build_status_report_with_probe(config, service, &probe).await } -/// Install the launchd service (does bootout first if already installed). -/// The service has KeepAlive + RunAtLoad, so the daemon starts immediately. -#[cfg(target_os = "macos")] -pub(crate) fn install(config: &Config) -> Result<(), CliError> { - let label = service_label(); - let plist_path = service_plist_path()?; - let daemon = daemon_binary()?; - - let plist = format!( - r#" - - - - Label - {label} - ProgramArguments - - {} - - EnvironmentVariables - - RUST_LOG - flicknote_sync=info,powersync=debug - - KeepAlive - - RunAtLoad - - StandardOutPath - {} - StandardErrorPath - {} - -"#, - xml_escape(&daemon.display().to_string()), - xml_escape(&config.paths.log_file.display().to_string()), - xml_escape(&config.paths.log_file.display().to_string()), - ); +async fn build_status_report_with_probe( + config: &Config, + service: Result, + health: &dyn DaemonHealthProbe, +) -> StatusReport { + let mut report = StatusReport::base(config); + match service { + Ok(ServiceState::NotInstalled) => report.service_state = ServiceStatusState::NotInstalled, + Ok(ServiceState::Stopped) => report.service_state = ServiceStatusState::InstalledStopped, + Ok(ServiceState::Running) => report.service_state = ServiceStatusState::Running, + Err(error) => { + report.service_state = ServiceStatusState::QueryFailed; + report.service_error = Some(StatusError { + code: "service_manager_query_failed".to_string(), + message: error.to_string(), + }); + } + } - fs::create_dir_all( - plist_path - .parent() - .ok_or_else(|| CliError::Other("Could not determine LaunchAgents directory".into()))?, - )?; - fs::write(&plist_path, &plist)?; - - #[allow(unsafe_code)] - let uid = unsafe { libc::getuid() }; - bootout_service(uid, label)?; - - for args in launchd_install_commands(uid, label, &plist_path) { - let command_name = args - .first() - .cloned() - .unwrap_or_else(|| "launchctl".to_string()); - let output = Command::new("launchctl") - .args(&args) - .output() - .map_err(|e| { - CliError::Other(format!("launchctl {command_name} failed to execute: {e}")) - })?; - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(CliError::Other(format!( - "launchctl {command_name} failed: {stderr}" - ))); + match health.health(config).await { + Ok(info) => { + report.application_state = ApplicationStatusState::Ready; + report.daemon_executable = Some(info.executable); + report.version.daemon = Some(info.version); + report.protocol.daemon = Some(info.protocol); + report.sync_errors = info.sync_errors; + report.sync_state = match info.sync { + Some(flicknote_sync::ipc::SyncConnectionState::Connected) => { + SyncStatusState::Connected + } + Some(flicknote_sync::ipc::SyncConnectionState::Connecting) => { + SyncStatusState::Connecting + } + Some(flicknote_sync::ipc::SyncConnectionState::Offline) | None => { + SyncStatusState::Offline + } + }; } + Err(error) => apply_health_error(&mut report, error), } + report +} - Ok(()) +fn apply_health_error(report: &mut StatusReport, error: ServiceError) { + let code = error.code().to_string(); + report.application_state = if code == flicknote_sync::ipc::PROTOCOL_MISMATCH_CODE { + ApplicationStatusState::ProtocolIncompatible + } else { + ApplicationStatusState::Unavailable + }; + report.error = Some(StatusError { + code, + message: error.to_string(), + }); + if let ServiceError::Remote { + details: Some(details), + .. + } = error + { + report.daemon_executable = details + .get("daemon_executable") + .and_then(serde_json::Value::as_str) + .map(ToOwned::to_owned); + report.version.daemon = details + .get("daemon_version") + .and_then(serde_json::Value::as_str) + .map(ToOwned::to_owned); + report.protocol.daemon = details + .get("daemon_protocol") + .and_then(serde_json::Value::as_u64) + .and_then(|value| u16::try_from(value).ok()); + } } -#[cfg(target_os = "macos")] -fn service_label() -> &'static str { - "io.guion.flicknote.sync" +fn format_service_state(state: ServiceStatusState) -> &'static str { + match state { + ServiceStatusState::NotInstalled => "not installed", + ServiceStatusState::InstalledStopped => "installed/stopped", + ServiceStatusState::Running => "running", + ServiceStatusState::QueryFailed => "query failed", + } } -#[cfg(target_os = "macos")] -fn service_plist_path() -> Result { - let label = service_label(); - let home = dirs::home_dir() - .ok_or_else(|| CliError::Other("Could not determine home directory".into()))?; - Ok(home - .join("Library/LaunchAgents") - .join(format!("{label}.plist"))) +fn format_application_state(state: ApplicationStatusState) -> &'static str { + match state { + ApplicationStatusState::Ready => "ready", + ApplicationStatusState::Unavailable => "application unavailable", + ApplicationStatusState::ProtocolIncompatible => "protocol incompatible", + ApplicationStatusState::Unknown => "unknown", + } } -#[cfg(target_os = "macos")] -fn xml_escape(s: &str) -> String { - s.replace('&', "&") - .replace('<', "<") - .replace('>', ">") -} - -#[cfg(any(target_os = "macos", test))] -fn launchd_install_commands( - uid: u32, - label: &str, - plist_path: &std::path::Path, -) -> Vec> { - vec![ - vec![ - "bootstrap".to_string(), - format!("gui/{uid}"), - plist_path.to_string_lossy().into_owned(), - ], - vec![ - "kickstart".to_string(), - "-k".to_string(), - format!("gui/{uid}/{label}"), - ], - ] -} - -#[cfg(any(target_os = "macos", test))] -fn launchd_stop_command(uid: u32, label: &str) -> Vec { - vec!["bootout".to_string(), format!("gui/{uid}/{label}")] -} - -/// Run `launchctl bootout`; an already-unloaded service is an idempotent success. -#[cfg(target_os = "macos")] -fn bootout_service(uid: u32, label: &str) -> Result<(), CliError> { - let args = launchd_stop_command(uid, label); - let result = Command::new("launchctl") - .args(&args) - .output() - .map_err(|error| CliError::Other(format!("launchctl bootout failed: {error}")))?; - if !result.status.success() { - let out = result; - let stderr = String::from_utf8_lossy(&out.stderr); - let is_expected = stderr.contains("No such process") - || stderr.contains("not loaded") - || stderr.contains("Could not find"); - if !is_expected { - return Err(CliError::Other(format!( - "launchctl bootout failed: {}", - stderr.trim() - ))); - } +fn format_sync_state(state: SyncStatusState) -> &'static str { + match state { + SyncStatusState::Connected => "connected", + SyncStatusState::Connecting => "connecting", + SyncStatusState::Offline => "offline", + SyncStatusState::Unavailable => "unavailable", } - Ok(()) } #[cfg(test)] mod tests { - use flicknote_core::config::ConfigPaths; - use super::*; + use flicknote_core::config::ConfigPaths; + use flicknote_sync::ipc::ServerInfo; - fn test_config(dir: &std::path::Path) -> Config { + fn test_config(directory: &std::path::Path) -> Config { Config { supabase_url: String::new(), supabase_anon_key: String::new(), @@ -287,68 +473,187 @@ mod tests { api_url: String::new(), web_url: None, paths: ConfigPaths { - config_dir: dir.to_path_buf(), - data_dir: dir.to_path_buf(), - config_file: dir.join("config.json"), - session_file: dir.join("session.json"), - db_file: dir.join("flicknote.db"), - log_file: dir.join("flicknote.log"), + config_dir: directory.to_path_buf(), + data_dir: directory.to_path_buf(), + config_file: directory.join("config.json"), + session_file: directory.join("session.json"), + db_file: directory.join("flicknote.db"), + log_file: directory.join("flicknote.log"), }, } } #[test] - fn launchd_install_runs_bootstrap_then_kickstart() { - let plist = PathBuf::from("/Users/neil/Library/LaunchAgents/io.guion.flicknote.sync.plist"); - let commands = launchd_install_commands(501, "io.guion.flicknote.sync", &plist); - - assert_eq!( - commands, - vec![ - vec![ - "bootstrap".to_string(), - "gui/501".to_string(), - plist.to_string_lossy().into_owned(), - ], - vec![ - "kickstart".to_string(), - "-k".to_string(), - "gui/501/io.guion.flicknote.sync".to_string(), - ], - ] + fn managed_service_restart_classification_uses_typed_startup_failures() { + let permanent = flicknote_sync::DaemonRunError::PermanentStartup( + "wording can change without affecting classification".to_string(), + ); + let ownership = flicknote_sync::DaemonRunError::OwnershipConflict( + "not configured and invalid argument are only words".to_string(), ); + + assert!(permanent.is_permanent_startup()); + assert!(!ownership.is_permanent_startup()); } #[test] - fn launchd_stop_boots_out_the_keepalive_service() { - assert_eq!( - launchd_stop_command(501, "io.guion.flicknote.sync"), - vec![ - "bootout".to_string(), - "gui/501/io.guion.flicknote.sync".to_string(), - ] - ); + fn status_json_is_an_object_with_stable_state_fields() { + let directory = tempfile::tempdir().unwrap(); + let report = StatusReport::base(&test_config(directory.path())); + let value = serde_json::to_value(report).unwrap(); + assert!(value.is_object()); + for field in [ + "service_state", + "application_state", + "sync_state", + "sync_errors", + "daemon_executable", + "version", + "protocol", + "error", + "service_error", + "log_guidance", + ] { + assert!(value.get(field).is_some(), "missing status field {field}"); + } + assert_eq!(value["service_state"], "query_failed"); + assert_eq!(value["application_state"], "unknown"); + assert_eq!(value["sync_state"], "unavailable"); } - #[test] - fn process_identity_must_match_expected_executable_before_signalling() { - let current = std::env::current_exe().unwrap(); - assert!(process_matches_executable(std::process::id(), ¤t)); - - let unrelated = tempfile::NamedTempFile::new().unwrap(); - assert!(!process_matches_executable( - std::process::id(), - unrelated.path(), - )); + struct FakeHealth(Option); + + #[async_trait::async_trait] + impl DaemonHealthProbe for FakeHealth { + async fn health(&self, _config: &Config) -> Result { + self.0 + .clone() + .ok_or_else(|| ServiceError::DaemonUnavailable("missing".to_string())) + } + } + + #[tokio::test] + async fn status_probes_application_even_when_service_is_not_installed() { + let directory = tempfile::tempdir().unwrap(); + let config = test_config(directory.path()); + let report = build_status_report_with_probe( + &config, + Ok(ServiceState::NotInstalled), + &FakeHealth(Some(ServerInfo::current())), + ) + .await; + assert_eq!(report.service_state, ServiceStatusState::NotInstalled); + assert_eq!(report.application_state, ApplicationStatusState::Ready); + } + + #[tokio::test] + async fn status_reports_ready_for_a_healthy_foreground_daemon_while_service_is_stopped() { + let directory = tempfile::tempdir().unwrap(); + let config = test_config(directory.path()); + let report = build_status_report_with_probe( + &config, + Ok(ServiceState::Stopped), + &FakeHealth(Some(ServerInfo::current())), + ) + .await; + + assert_eq!(report.service_state, ServiceStatusState::InstalledStopped); + assert_eq!(report.application_state, ApplicationStatusState::Ready); + assert!(report.is_ready()); + } + + #[tokio::test] + async fn status_keeps_application_health_when_service_query_fails() { + let directory = tempfile::tempdir().unwrap(); + let config = test_config(directory.path()); + let error = ServiceManagerError::new("query status", "unavailable"); + let report = build_status_report_with_probe( + &config, + Err(error), + &FakeHealth(Some(ServerInfo::current())), + ) + .await; + assert_eq!(report.service_state, ServiceStatusState::QueryFailed); + assert_eq!(report.application_state, ApplicationStatusState::Ready); + assert!(report.service_error.is_some()); } #[test] - fn stale_pid_for_an_unrelated_live_process_is_removed_without_being_accepted() { + fn status_reports_connected_and_protocol_incompatible_as_explicit_states() { let directory = tempfile::tempdir().unwrap(); let config = test_config(directory.path()); - fs::write(pid_file(&config), std::process::id().to_string()).unwrap(); + let mut healthy = StatusReport::base(&config); + healthy.service_state = ServiceStatusState::Running; + healthy.application_state = ApplicationStatusState::Ready; + healthy.sync_state = SyncStatusState::Connected; + healthy.version.daemon = Some("0.9.0".to_string()); + healthy.protocol.daemon = Some(flicknote_sync::ipc::PROTOCOL_VERSION); + let healthy_json = serde_json::to_value(healthy).unwrap(); + assert_eq!(healthy_json["application_state"], "ready"); + assert_eq!(healthy_json["sync_state"], "connected"); + + let mut incompatible = StatusReport::base(&config); + apply_health_error( + &mut incompatible, + ServiceError::Remote { + code: flicknote_sync::ipc::PROTOCOL_MISMATCH_CODE.to_string(), + message: "protocol mismatch".to_string(), + retryable: false, + details: Some(serde_json::json!({ + "daemon_executable": "/opt/flicknote/bin/flicknote", + "daemon_version": "0.8.0", + "daemon_protocol": 2 + })), + }, + ); + let incompatible_json = serde_json::to_value(incompatible).unwrap(); + assert_eq!( + incompatible_json["application_state"], + "protocol_incompatible" + ); + assert_eq!(incompatible_json["protocol"]["daemon"], 2); + assert_eq!( + incompatible_json["daemon_executable"], + "/opt/flicknote/bin/flicknote" + ); + } + + #[tokio::test] + async fn status_exposes_powersync_download_and_upload_errors() { + let directory = tempfile::tempdir().unwrap(); + let config = test_config(directory.path()); + let info = ServerInfo::current().with_sync_status( + flicknote_sync::ipc::SyncConnectionState::Offline, + flicknote_sync::ipc::PowerSyncErrors { + download: Some("download transport failed".to_string()), + upload: Some("upload rejected".to_string()), + }, + ); + + let report = build_status_report_with_probe( + &config, + Ok(ServiceState::Running), + &FakeHealth(Some(info)), + ) + .await; + + let json = serde_json::to_value(&report).unwrap(); + assert_eq!(json["sync_errors"]["download"], "download transport failed"); + assert_eq!(json["sync_errors"]["upload"], "upload rejected"); + let verbose = report.verbose_text(); + assert!(verbose.contains("PowerSync download error: download transport failed")); + assert!(verbose.contains("PowerSync upload error: upload rejected")); + } - assert_eq!(read_pid(&config), None); - assert!(!pid_file(&config).exists()); + #[test] + fn unhealthy_concise_status_contains_recovery_commands() { + let directory = tempfile::tempdir().unwrap(); + let mut report = StatusReport::base(&test_config(directory.path())); + report.service_state = ServiceStatusState::InstalledStopped; + report.application_state = ApplicationStatusState::Unavailable; + let text = report.concise_text(); + assert!(text.contains("flicknote daemon status --verbose")); + assert!(text.contains("flicknote daemon start")); + assert_eq!(text.lines().count(), 1); } } diff --git a/flicknote-cli/src/commands/daemon_lifecycle.rs b/flicknote-cli/src/commands/daemon_lifecycle.rs new file mode 100644 index 0000000..1399c10 --- /dev/null +++ b/flicknote-cli/src/commands/daemon_lifecycle.rs @@ -0,0 +1,673 @@ +use async_trait::async_trait; +use flicknote_core::config::Config; +use flicknote_core::error::CliError; +use flicknote_core::services::error::ServiceError; +use flicknote_sync::ipc::{DaemonClient, PROTOCOL_MISMATCH_CODE, ServerInfo}; +use std::time::Duration; + +use super::service_manager::{ + NativeServiceFactory, ServiceManagerAdapter, ServiceManagerError, ServiceManagerFactory, + ServiceState, +}; + +pub(crate) const SERVICE_OPERATION_TIMEOUT: Duration = Duration::from_secs(10); +const HEALTH_POLL_INTERVAL: Duration = Duration::from_millis(100); + +#[async_trait] +pub(crate) trait DaemonHealthProbe: Send + Sync { + async fn health(&self, config: &Config) -> Result; +} + +pub(crate) struct IpcHealthProbe; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ServiceCleanup { + NotInstalled, + Removed, +} + +#[async_trait] +pub(crate) trait DaemonLifecycle: Send + Sync { + async fn install_and_wait(&self, config: &Config) -> Result<(), CliError>; + async fn uninstall(&self, config: &Config) -> Result; +} + +pub(crate) struct NativeDaemonLifecycle; + +#[async_trait] +impl DaemonLifecycle for NativeDaemonLifecycle { + async fn install_and_wait(&self, config: &Config) -> Result<(), CliError> { + LifecycleController::native(&IpcHealthProbe) + .install_and_wait(config) + .await + } + + async fn uninstall(&self, config: &Config) -> Result { + LifecycleController::native(&IpcHealthProbe) + .uninstall(config) + .await + } +} + +#[async_trait] +impl DaemonHealthProbe for IpcHealthProbe { + async fn health(&self, config: &Config) -> Result { + DaemonClient::new(config).health().await + } +} + +pub(crate) struct LifecycleController<'a> { + factory: &'a dyn ServiceManagerFactory, + health: &'a dyn DaemonHealthProbe, +} + +impl<'a> LifecycleController<'a> { + pub(crate) fn new( + factory: &'a dyn ServiceManagerFactory, + health: &'a dyn DaemonHealthProbe, + ) -> Self { + Self { factory, health } + } + + pub(crate) fn native(health: &'a dyn DaemonHealthProbe) -> Self { + static FACTORY: NativeServiceFactory = NativeServiceFactory; + Self::new(&FACTORY, health) + } + + fn manager(&self, action: &'static str) -> Result, CliError> { + self.factory + .manager() + .map_err(|error| lifecycle_error(action, &error)) + } + + pub(crate) async fn install_and_wait(&self, config: &Config) -> Result<(), CliError> { + let state = self.service_state("query")?; + self.stop_running(config, state).await?; + self.service_call("install", |manager| manager.install(config))?; + self.service_call("reload", |manager| manager.reload())?; + self.service_call("start", |manager| manager.start())?; + self.wait_for_ready(config, SERVICE_OPERATION_TIMEOUT).await + } + + pub(crate) async fn uninstall(&self, config: &Config) -> Result { + self.uninstall_with_timeout(config, SERVICE_OPERATION_TIMEOUT) + .await + } + + async fn uninstall_with_timeout( + &self, + config: &Config, + timeout: Duration, + ) -> Result { + let state = self.service_state("query")?; + self.stop_running_with_timeout(config, state, timeout) + .await?; + if state == ServiceState::NotInstalled { + return Ok(ServiceCleanup::NotInstalled); + } + self.service_call("uninstall", |manager| manager.uninstall())?; + self.service_call("reload", |manager| manager.reload())?; + self.wait_for_stopped(config, timeout).await?; + Ok(ServiceCleanup::Removed) + } + + pub(crate) async fn start(&self, config: &Config) -> Result<(), CliError> { + if self.service_state("query")? == ServiceState::NotInstalled { + return Err(CliError::Other( + "FlickNote daemon service is not installed; run `flicknote daemon install`" + .to_string(), + )); + } + self.service_call("start", |manager| manager.start())?; + self.wait_for_ready(config, SERVICE_OPERATION_TIMEOUT).await + } + + pub(crate) async fn stop(&self, config: &Config) -> Result { + let state = self.service_state("query")?; + self.stop_running(config, state).await + } + + pub(crate) async fn restart(&self, config: &Config) -> Result<(), CliError> { + let state = self.service_state("query")?; + if state == ServiceState::NotInstalled { + return Err(CliError::Other( + "FlickNote daemon service is not installed; run `flicknote daemon install`" + .to_string(), + )); + } + self.stop_running(config, state).await?; + self.service_call("start", |manager| manager.start())?; + self.wait_for_ready(config, SERVICE_OPERATION_TIMEOUT).await + } + + async fn stop_running(&self, config: &Config, state: ServiceState) -> Result { + self.stop_running_with_timeout(config, state, SERVICE_OPERATION_TIMEOUT) + .await + } + + async fn stop_running_with_timeout( + &self, + config: &Config, + state: ServiceState, + timeout: Duration, + ) -> Result { + let was_running = state == ServiceState::Running; + if was_running { + self.service_call("stop", |manager| manager.stop())?; + } + self.wait_for_stopped(config, timeout).await?; + Ok(was_running) + } + + fn service_state(&self, action: &'static str) -> Result { + let manager = self.manager(action)?; + manager + .status() + .map_err(|error| lifecycle_error(action, &error)) + } + + fn service_call( + &self, + action: &'static str, + operation: impl FnOnce(&dyn ServiceManagerAdapter) -> Result<(), ServiceManagerError>, + ) -> Result<(), CliError> { + let manager = self.manager(action)?; + operation(&*manager).map_err(|error| lifecycle_error(action, &error)) + } + + async fn wait_for_ready(&self, config: &Config, timeout: Duration) -> Result<(), CliError> { + let wait = async { + loop { + match self.service_state("confirm running")? { + ServiceState::Running => {} + ServiceState::Stopped => { + return Err(CliError::Other( + "FlickNote daemon service stopped before becoming ready; run `flicknote daemon status --verbose`" + .to_string(), + )); + } + ServiceState::NotInstalled => { + return Err(CliError::Other( + "FlickNote daemon service was removed before becoming ready; run `flicknote daemon install`" + .to_string(), + )); + } + } + + match self.health.health(config).await { + Ok(_) => { + tokio::time::sleep(HEALTH_POLL_INTERVAL).await; + if self.service_state("confirm running")? == ServiceState::Running + && self.health.health(config).await.is_ok() + { + return Ok(()); + } + } + Err(error) if error.code() == PROTOCOL_MISMATCH_CODE => { + return Err(CliError::Other(error.to_string())); + } + Err(error) if !error.retryable() => return Err(CliError::from(error)), + Err(_) => tokio::time::sleep(HEALTH_POLL_INTERVAL).await, + } + } + }; + tokio::time::timeout(timeout, wait).await.map_err(|_| { + CliError::Other(format!( + "FlickNote daemon did not become ready within {timeout:?}; run `flicknote daemon status --verbose`" + )) + })? + } + + async fn wait_for_stopped(&self, config: &Config, timeout: Duration) -> Result<(), CliError> { + let wait = async { + loop { + match self.health.health(config).await { + Err(error) if error.code() == "daemon_unavailable" => return Ok(()), + _ => tokio::time::sleep(HEALTH_POLL_INTERVAL).await, + } + } + }; + tokio::time::timeout(timeout, wait).await.map_err(|_| { + CliError::Other(format!( + "FlickNote daemon did not stop within {timeout:?}; run `flicknote daemon status --verbose`" + )) + })? + } +} + +pub(crate) fn lifecycle_error(action: &str, error: &ServiceManagerError) -> CliError { + CliError::Other(format!( + "Could not {action} the FlickNote daemon service: {error}; run `flicknote daemon status --verbose` for diagnosis" + )) +} + +#[cfg(test)] +mod tests { + use super::*; + use flicknote_core::services::error::ServiceError; + use std::collections::VecDeque; + use std::sync::{Arc, Mutex}; + + struct FakeManager { + state: Mutex, + start_state: Mutex, + calls: Mutex>, + fail: Mutex>, + } + + impl FakeManager { + fn new(state: ServiceState) -> Self { + Self { + state: Mutex::new(state), + start_state: Mutex::new(ServiceState::Running), + calls: Mutex::new(Vec::new()), + fail: Mutex::new(None), + } + } + + fn calls(&self) -> Vec<&'static str> { + self.calls.lock().unwrap().clone() + } + + fn operation(&self, name: &'static str) -> Result<(), ServiceManagerError> { + self.calls.lock().unwrap().push(name); + if self.fail.lock().unwrap().as_ref() == Some(&name) { + return Err(ServiceManagerError::new(name, "forced failure")); + } + Ok(()) + } + } + + impl ServiceManagerAdapter for FakeManager { + fn status(&self) -> Result { + self.operation("status")?; + Ok(*self.state.lock().unwrap()) + } + + fn install(&self, _config: &Config) -> Result<(), ServiceManagerError> { + self.operation("install")?; + *self.state.lock().unwrap() = ServiceState::Stopped; + Ok(()) + } + + fn reload(&self) -> Result<(), ServiceManagerError> { + self.operation("reload") + } + + fn start(&self) -> Result<(), ServiceManagerError> { + self.operation("start")?; + *self.state.lock().unwrap() = *self.start_state.lock().unwrap(); + Ok(()) + } + + fn stop(&self) -> Result<(), ServiceManagerError> { + self.operation("stop")?; + *self.state.lock().unwrap() = ServiceState::Stopped; + Ok(()) + } + + fn uninstall(&self) -> Result<(), ServiceManagerError> { + self.operation("uninstall")?; + *self.state.lock().unwrap() = ServiceState::NotInstalled; + Ok(()) + } + } + + struct FakeHealth { + results: Mutex>>, + } + + impl FakeHealth { + fn new(results: impl IntoIterator>) -> Self { + Self { + results: Mutex::new(results.into_iter().collect()), + } + } + } + + #[async_trait] + impl DaemonHealthProbe for FakeHealth { + async fn health(&self, _config: &Config) -> Result { + self.results + .lock() + .unwrap() + .pop_front() + .unwrap_or_else(|| Ok(ServerInfo::current())) + } + } + + fn test_config() -> (tempfile::TempDir, Config) { + let directory = tempfile::tempdir().unwrap(); + let config = Config { + supabase_url: String::new(), + supabase_anon_key: String::new(), + powersync_url: String::new(), + api_url: String::new(), + web_url: None, + paths: flicknote_core::config::ConfigPaths { + config_dir: directory.path().to_path_buf(), + data_dir: directory.path().to_path_buf(), + config_file: directory.path().join("config.json"), + session_file: directory.path().join("session.json"), + db_file: directory.path().join("flicknote.db"), + log_file: directory.path().join("flicknote.log"), + }, + }; + (directory, config) + } + + fn unavailable() -> ServiceError { + ServiceError::DaemonUnavailable("stopped".to_string()) + } + + struct FakeFactory(Arc); + + impl ServiceManagerFactory for FakeFactory { + fn manager(&self) -> Result, ServiceManagerError> { + Ok(Box::new(FakeAdapter(Arc::clone(&self.0)))) + } + } + + struct FakeAdapter(Arc); + + impl ServiceManagerAdapter for FakeAdapter { + fn status(&self) -> Result { + self.0.status() + } + fn install(&self, config: &Config) -> Result<(), ServiceManagerError> { + self.0.install(config) + } + fn reload(&self) -> Result<(), ServiceManagerError> { + self.0.reload() + } + fn start(&self) -> Result<(), ServiceManagerError> { + self.0.start() + } + fn stop(&self) -> Result<(), ServiceManagerError> { + self.0.stop() + } + fn uninstall(&self) -> Result<(), ServiceManagerError> { + self.0.uninstall() + } + } + + #[tokio::test] + async fn install_reconciles_reload_start_and_waits_for_readiness() { + let (_directory, config) = test_config(); + let manager = Arc::new(FakeManager::new(ServiceState::Stopped)); + let factory = FakeFactory(Arc::clone(&manager)); + let health = FakeHealth::new([ + Err(unavailable()), + Ok(ServerInfo::current()), + Ok(ServerInfo::current()), + ]); + LifecycleController::new(&factory, &health) + .install_and_wait(&config) + .await + .unwrap(); + assert_eq!( + manager.calls(), + vec!["status", "install", "reload", "start", "status", "status"] + ); + } + + #[tokio::test] + async fn uninstall_stops_waits_then_removes_and_reloads() { + let (_directory, config) = test_config(); + let manager = Arc::new(FakeManager::new(ServiceState::Running)); + let factory = FakeFactory(Arc::clone(&manager)); + let health = FakeHealth::new([Err(unavailable()), Err(unavailable())]); + assert_eq!( + LifecycleController::new(&factory, &health) + .uninstall(&config) + .await + .unwrap(), + ServiceCleanup::Removed + ); + assert_eq!( + manager.calls(), + vec!["status", "stop", "uninstall", "reload"] + ); + } + + #[tokio::test] + async fn start_rejects_an_unrelated_healthy_daemon_when_the_service_does_not_run() { + let (_directory, config) = test_config(); + let manager = Arc::new(FakeManager::new(ServiceState::Stopped)); + *manager.start_state.lock().unwrap() = ServiceState::Stopped; + let factory = FakeFactory(Arc::clone(&manager)); + let health = FakeHealth::new([Ok(ServerInfo::current())]); + + let error = LifecycleController::new(&factory, &health) + .start(&config) + .await + .unwrap_err(); + + assert!(error.to_string().contains("service stopped")); + assert_eq!(manager.calls(), vec!["status", "start", "status"]); + } + + #[tokio::test] + async fn start_does_not_install_missing_service() { + let (_directory, config) = test_config(); + let manager = Arc::new(FakeManager::new(ServiceState::NotInstalled)); + let factory = FakeFactory(Arc::clone(&manager)); + let health = FakeHealth::new([]); + assert!( + LifecycleController::new(&factory, &health) + .start(&config) + .await + .is_err() + ); + assert_eq!(manager.calls(), vec!["status"]); + } + + #[tokio::test] + async fn install_creates_a_missing_service_then_reloads_and_starts_it() { + let (_directory, config) = test_config(); + let manager = Arc::new(FakeManager::new(ServiceState::NotInstalled)); + let factory = FakeFactory(Arc::clone(&manager)); + let health = FakeHealth::new([ + Err(unavailable()), + Ok(ServerInfo::current()), + Ok(ServerInfo::current()), + ]); + + LifecycleController::new(&factory, &health) + .install_and_wait(&config) + .await + .unwrap(); + + assert_eq!( + manager.calls(), + vec!["status", "install", "reload", "start", "status", "status"] + ); + } + + #[tokio::test] + async fn install_replaces_a_running_service_before_reinstalling() { + let (_directory, config) = test_config(); + let manager = Arc::new(FakeManager::new(ServiceState::Running)); + let factory = FakeFactory(Arc::clone(&manager)); + let health = FakeHealth::new([ + Err(unavailable()), + Ok(ServerInfo::current()), + Ok(ServerInfo::current()), + ]); + + LifecycleController::new(&factory, &health) + .install_and_wait(&config) + .await + .unwrap(); + + assert_eq!( + manager.calls(), + vec![ + "status", "stop", "install", "reload", "start", "status", "status" + ] + ); + } + + #[tokio::test] + async fn install_reload_failure_is_returned_before_start() { + let (_directory, config) = test_config(); + let manager = Arc::new(FakeManager::new(ServiceState::Stopped)); + *manager.fail.lock().unwrap() = Some("reload"); + let factory = FakeFactory(Arc::clone(&manager)); + let health = FakeHealth::new([Err(unavailable())]); + + assert!( + LifecycleController::new(&factory, &health) + .install_and_wait(&config) + .await + .is_err() + ); + assert_eq!(manager.calls(), vec!["status", "install", "reload"]); + } + + #[tokio::test] + async fn start_stop_and_restart_reconcile_without_installing() { + let (_directory, config) = test_config(); + let manager = Arc::new(FakeManager::new(ServiceState::Stopped)); + let factory = FakeFactory(Arc::clone(&manager)); + let health = FakeHealth::new([ + Ok(ServerInfo::current()), + Ok(ServerInfo::current()), + Err(unavailable()), + Err(unavailable()), + Ok(ServerInfo::current()), + Ok(ServerInfo::current()), + ]); + let controller = LifecycleController::new(&factory, &health); + + controller.start(&config).await.unwrap(); + assert!(controller.stop(&config).await.unwrap()); + controller.restart(&config).await.unwrap(); + + assert_eq!( + manager.calls(), + vec![ + "status", "start", "status", "status", "status", "stop", "status", "start", + "status", "status" + ] + ); + } + + #[tokio::test] + async fn restart_stops_a_running_service_before_starting() { + let (_directory, config) = test_config(); + let manager = Arc::new(FakeManager::new(ServiceState::Running)); + let factory = FakeFactory(Arc::clone(&manager)); + let health = FakeHealth::new([ + Err(unavailable()), + Ok(ServerInfo::current()), + Ok(ServerInfo::current()), + ]); + + LifecycleController::new(&factory, &health) + .restart(&config) + .await + .unwrap(); + + assert_eq!( + manager.calls(), + vec!["status", "stop", "start", "status", "status"] + ); + } + + #[tokio::test] + async fn uninstall_skips_mutations_when_service_is_absent() { + let (_directory, config) = test_config(); + let manager = Arc::new(FakeManager::new(ServiceState::NotInstalled)); + let factory = FakeFactory(Arc::clone(&manager)); + let health = FakeHealth::new([Err(unavailable())]); + + assert_eq!( + LifecycleController::new(&factory, &health) + .uninstall(&config) + .await + .unwrap(), + ServiceCleanup::NotInstalled + ); + assert_eq!(manager.calls(), vec!["status"]); + } + + #[tokio::test] + async fn uninstall_reload_failure_is_propagated_after_removal() { + let (_directory, config) = test_config(); + let manager = Arc::new(FakeManager::new(ServiceState::Stopped)); + *manager.fail.lock().unwrap() = Some("reload"); + let factory = FakeFactory(Arc::clone(&manager)); + let health = FakeHealth::new([Err(unavailable())]); + + assert!( + LifecycleController::new(&factory, &health) + .uninstall(&config) + .await + .is_err() + ); + assert_eq!(manager.calls(), vec!["status", "uninstall", "reload"]); + } + + #[tokio::test] + async fn stopped_service_does_not_hide_a_foreground_daemon_during_stop() { + let (_directory, config) = test_config(); + let manager = Arc::new(FakeManager::new(ServiceState::Stopped)); + let factory = FakeFactory(Arc::clone(&manager)); + let health = FakeHealth::new([Ok(ServerInfo::current())]); + + let error = LifecycleController::new(&factory, &health) + .stop_running_with_timeout(&config, ServiceState::Stopped, Duration::from_millis(10)) + .await + .unwrap_err(); + + assert!(error.to_string().contains("did not stop")); + assert!(manager.calls().is_empty()); + } + + #[tokio::test] + async fn absent_service_does_not_hide_a_foreground_daemon_during_uninstall() { + let (_directory, config) = test_config(); + let manager = Arc::new(FakeManager::new(ServiceState::NotInstalled)); + let factory = FakeFactory(Arc::clone(&manager)); + let health = FakeHealth::new([Ok(ServerInfo::current())]); + + let error = LifecycleController::new(&factory, &health) + .uninstall_with_timeout(&config, Duration::from_millis(10)) + .await + .unwrap_err(); + + assert!(error.to_string().contains("did not stop")); + assert_eq!(manager.calls(), vec!["status"]); + } + + #[test] + fn lifecycle_errors_preserve_platform_diagnostics() { + let platform = ServiceManagerError::new( + "start", + "launchctl bootstrap failed: Input/output error (code 5)", + ); + + let message = lifecycle_error("start", &platform).to_string(); + + assert!(message.contains("launchctl bootstrap failed")); + assert!(message.contains("Input/output error (code 5)")); + assert!(message.contains("flicknote daemon status --verbose")); + } + + #[tokio::test] + async fn cleanup_failure_stops_before_uninstall_is_attempted() { + let (_directory, config) = test_config(); + let manager = Arc::new(FakeManager::new(ServiceState::Running)); + *manager.fail.lock().unwrap() = Some("stop"); + let factory = FakeFactory(Arc::clone(&manager)); + let health = FakeHealth::new([]); + assert!( + LifecycleController::new(&factory, &health) + .uninstall(&config) + .await + .is_err() + ); + assert_eq!(manager.calls(), vec!["status", "stop"]); + } +} diff --git a/flicknote-cli/src/commands/login.rs b/flicknote-cli/src/commands/login.rs index 4122d5c..7051f12 100644 --- a/flicknote-cli/src/commands/login.rs +++ b/flicknote-cli/src/commands/login.rs @@ -1,3 +1,5 @@ +use super::daemon_lifecycle::{DaemonLifecycle, NativeDaemonLifecycle}; +use async_trait::async_trait; use clap::Args; use flicknote_auth::client::GoTrueClient; use flicknote_core::config::Config; @@ -11,45 +13,39 @@ pub(crate) struct LoginArgs { /// OAuth provider #[arg(long, conflicts_with = "email", value_parser = ["google", "apple"])] provider: Option, - /// Force re-authentication (fixes stuck sync without data loss) + /// Force re-authentication after removing the current daemon service and session #[arg(long)] force: bool, } -pub(crate) async fn run(config: &Config, args: &LoginArgs) -> Result<(), CliError> { - if config.paths.session_file.exists() && !args.force { - return Err(CliError::Other( - "Already logged in. Use `flicknote login --force` to re-authenticate (e.g. after sync issues).".into(), - )); - } +struct GoTrueAuthenticator; - let manage_local_daemon = manages_daemon_after_login_for(cfg!(target_os = "macos")); - if config.paths.session_file.exists() { - // --force: stop the macOS LaunchAgent before clearing the stale session. - if manage_local_daemon { - super::daemon::stop(config)?; - super::daemon::uninstall()?; - } - std::fs::remove_file(&config.paths.session_file)?; - } +#[async_trait] +trait LoginAuthenticator: Send + Sync { + async fn authenticate(&self, config: &Config, args: &LoginArgs) -> Result<(), CliError>; +} - let client = GoTrueClient::new( - &config.supabase_url, - &config.supabase_anon_key, - &config.paths.session_file, - ); +#[async_trait] +impl LoginAuthenticator for GoTrueAuthenticator { + async fn authenticate(&self, config: &Config, args: &LoginArgs) -> Result<(), CliError> { + let client = GoTrueClient::new( + &config.supabase_url, + &config.supabase_anon_key, + &config.paths.session_file, + ); + if let Some(provider) = &args.provider { + return client + .sign_in_with_oauth(provider) + .await + .map(|_| ()) + .map_err(|error| CliError::Auth { + operation: "signIn".into(), + description: error.to_string(), + }); + } - if let Some(ref provider) = args.provider { - client - .sign_in_with_oauth(provider) - .await - .map_err(|e| CliError::Auth { - operation: "signIn".into(), - description: e.to_string(), - })?; - } else { let email = match &args.email { - Some(e) => e.clone(), + Some(email) => email.clone(), None => { eprint!("Email: "); let mut input = String::new(); @@ -57,55 +53,239 @@ pub(crate) async fn run(config: &Config, args: &LoginArgs) -> Result<(), CliErro input.trim().to_string() } }; - client .sign_in_with_otp(&email) .await - .map_err(|e| CliError::Auth { + .map_err(|error| CliError::Auth { operation: "signIn".into(), - description: e.to_string(), + description: error.to_string(), })?; println!("OTP sent to {email}"); eprint!("Enter code: "); let mut code = String::new(); std::io::stdin().read_line(&mut code)?; - client .verify_otp(&email, code.trim()) .await - .map_err(|e| CliError::Auth { + .map(|_| ()) + .map_err(|error| CliError::Auth { operation: "verifyOtp".into(), - description: e.to_string(), - })?; + description: error.to_string(), + }) } +} - println!("Authenticated"); +pub(crate) async fn run(config: &Config, args: &LoginArgs) -> Result<(), CliError> { + run_with_dependencies(config, args, &NativeDaemonLifecycle, &GoTrueAuthenticator).await +} - if manage_local_daemon { - // The macOS login flow owns the per-user LaunchAgent lifecycle. - super::sync::install_local_daemon(config, std::time::Duration::from_secs(10)).await?; - println!("Sync daemon installed and started"); +async fn run_with_dependencies( + config: &Config, + args: &LoginArgs, + lifecycle: &dyn DaemonLifecycle, + authenticator: &dyn LoginAuthenticator, +) -> Result<(), CliError> { + if config.paths.session_file.exists() && !args.force { + return Err(CliError::Other( + "Already logged in. Use `flicknote login --force` to re-authenticate.".into(), + )); } - Ok(()) -} + if args.force { + lifecycle.uninstall(config).await?; + if config.paths.session_file.exists() { + std::fs::remove_file(&config.paths.session_file)?; + } + } -const fn manages_daemon_after_login_for(target_is_macos: bool) -> bool { - target_is_macos + authenticator.authenticate(config, args).await?; + println!("Authenticated"); + if let Err(error) = lifecycle.install_and_wait(config).await { + println!("Daemon startup failed: {error}"); + return Err(CliError::Other( + "Authentication succeeded but the daemon is not ready; run `flicknote daemon status --verbose` and retry `flicknote daemon install`".to_string(), + )); + } + println!("FlickNote daemon ready"); + Ok(()) } #[cfg(test)] mod tests { - #[cfg(not(target_os = "macos"))] - #[test] - fn non_macos_login_does_not_wait_for_a_launchd_daemon() { - assert!(!super::manages_daemon_after_login_for(false)); + use super::*; + use crate::commands::daemon_lifecycle::ServiceCleanup; + use std::sync::{Arc, Mutex}; + + struct FakeLifecycle { + events: Arc>>, + cleanup_error: bool, + install_error: bool, + } + + #[async_trait] + impl DaemonLifecycle for FakeLifecycle { + async fn install_and_wait(&self, _config: &Config) -> Result<(), CliError> { + self.events.lock().unwrap().push("install"); + if self.install_error { + return Err(CliError::Other("readiness failed".to_string())); + } + Ok(()) + } + + async fn uninstall(&self, _config: &Config) -> Result { + self.events.lock().unwrap().push("uninstall"); + if self.cleanup_error { + return Err(CliError::Other("cleanup failed".to_string())); + } + Ok(ServiceCleanup::Removed) + } + } + + struct FakeAuthenticator { + events: Arc>>, + succeeds: bool, + } + + #[async_trait] + impl LoginAuthenticator for FakeAuthenticator { + async fn authenticate(&self, config: &Config, _args: &LoginArgs) -> Result<(), CliError> { + self.events.lock().unwrap().push("authenticate"); + if !self.succeeds { + return Err(CliError::Other("authentication failed".to_string())); + } + std::fs::write(&config.paths.session_file, "new session")?; + Ok(()) + } + } + + fn config(directory: &std::path::Path) -> Config { + Config { + supabase_url: "http://127.0.0.1:9".to_string(), + supabase_anon_key: "key".to_string(), + powersync_url: "http://127.0.0.1:9".to_string(), + api_url: "http://127.0.0.1:9".to_string(), + web_url: None, + paths: flicknote_core::config::ConfigPaths { + config_dir: directory.to_path_buf(), + data_dir: directory.to_path_buf(), + config_file: directory.join("config.json"), + session_file: directory.join("session.json"), + db_file: directory.join("db"), + log_file: directory.join("log"), + }, + } + } + + fn force_args() -> LoginArgs { + LoginArgs { + email: Some("person@example.com".to_string()), + provider: None, + force: true, + } + } + + fn dependencies( + events: &Arc>>, + cleanup_error: bool, + install_error: bool, + authentication_succeeds: bool, + ) -> (FakeLifecycle, FakeAuthenticator) { + ( + FakeLifecycle { + events: Arc::clone(events), + cleanup_error, + install_error, + }, + FakeAuthenticator { + events: Arc::clone(events), + succeeds: authentication_succeeds, + }, + ) } - #[test] - fn macos_login_manages_the_local_launch_agent() { - assert!(super::manages_daemon_after_login_for(true)); - assert!(!super::manages_daemon_after_login_for(false)); + #[tokio::test] + async fn forced_login_cleans_stale_service_without_a_session_before_authentication() { + let directory = tempfile::tempdir().unwrap(); + let config = config(directory.path()); + let events = Arc::new(Mutex::new(Vec::new())); + let (lifecycle, authenticator) = dependencies(&events, false, false, false); + + assert!( + run_with_dependencies(&config, &force_args(), &lifecycle, &authenticator) + .await + .is_err() + ); + + assert_eq!( + events.lock().unwrap().as_slice(), + ["uninstall", "authenticate"] + ); + assert!(!config.paths.session_file.exists()); + } + + #[tokio::test] + async fn forced_login_cleanup_failure_preserves_old_session_and_skips_authentication() { + let directory = tempfile::tempdir().unwrap(); + let config = config(directory.path()); + std::fs::write(&config.paths.session_file, "old session").unwrap(); + let events = Arc::new(Mutex::new(Vec::new())); + let (lifecycle, authenticator) = dependencies(&events, true, false, true); + + assert!( + run_with_dependencies(&config, &force_args(), &lifecycle, &authenticator) + .await + .is_err() + ); + + assert_eq!(events.lock().unwrap().as_slice(), ["uninstall"]); + assert_eq!( + std::fs::read_to_string(&config.paths.session_file).unwrap(), + "old session" + ); + } + + #[tokio::test] + async fn failed_forced_authentication_does_not_restore_the_old_session() { + let directory = tempfile::tempdir().unwrap(); + let config = config(directory.path()); + std::fs::write(&config.paths.session_file, "old session").unwrap(); + let events = Arc::new(Mutex::new(Vec::new())); + let (lifecycle, authenticator) = dependencies(&events, false, false, false); + + assert!( + run_with_dependencies(&config, &force_args(), &lifecycle, &authenticator) + .await + .is_err() + ); + + assert_eq!( + events.lock().unwrap().as_slice(), + ["uninstall", "authenticate"] + ); + assert!(!config.paths.session_file.exists()); + } + + #[tokio::test] + async fn daemon_install_failure_retains_the_new_authenticated_session() { + let directory = tempfile::tempdir().unwrap(); + let config = config(directory.path()); + let events = Arc::new(Mutex::new(Vec::new())); + let (lifecycle, authenticator) = dependencies(&events, false, true, true); + + assert!( + run_with_dependencies(&config, &force_args(), &lifecycle, &authenticator) + .await + .is_err() + ); + + assert_eq!( + events.lock().unwrap().as_slice(), + ["uninstall", "authenticate", "install"] + ); + assert_eq!( + std::fs::read_to_string(&config.paths.session_file).unwrap(), + "new session" + ); } } diff --git a/flicknote-cli/src/commands/logout.rs b/flicknote-cli/src/commands/logout.rs index c39a684..90dcee8 100644 --- a/flicknote-cli/src/commands/logout.rs +++ b/flicknote-cli/src/commands/logout.rs @@ -1,38 +1,192 @@ +use super::daemon_lifecycle::{DaemonLifecycle, NativeDaemonLifecycle, ServiceCleanup}; +use clap::Args; use flicknote_core::config::Config; use flicknote_core::error::CliError; use std::fs; -pub(crate) async fn run(config: &Config) -> Result<(), CliError> { - if !config.paths.session_file.exists() { +#[derive(Args)] +pub(crate) struct LogoutArgs { + /// Continue clearing credentials and local data if service cleanup cannot be confirmed + #[arg(long)] + force: bool, +} + +pub(crate) async fn run(config: &Config, args: &LogoutArgs) -> Result<(), CliError> { + run_with_lifecycle(config, args, &NativeDaemonLifecycle).await +} + +async fn run_with_lifecycle( + config: &Config, + args: &LogoutArgs, + lifecycle: &dyn DaemonLifecycle, +) -> Result<(), CliError> { + let session_exists = config.paths.session_file.exists(); + let service_cleanup = lifecycle.uninstall(config).await; + if !session_exists && matches!(service_cleanup, Ok(ServiceCleanup::NotInstalled)) { println!("Already logged out"); return Ok(()); } + let service_error = service_cleanup.err(); + if let Some(error) = &service_error { + if !args.force { + return Err(CliError::Other(format!( + "Could not confirm daemon cleanup: {error}. Session and local data were retained; use `flicknote logout --force` only after reviewing `flicknote daemon status --verbose`" + ))); + } + eprintln!("Warning: daemon cleanup was not confirmed: {error}"); + } - super::daemon::stop(config)?; - super::daemon::uninstall()?; - - // 3. Delete local DB files — collect errors so session is always cleared let db_base = config.paths.db_file.with_extension(""); - let mut db_errors: Vec = Vec::new(); - for ext in ["db", "db-shm", "db-wal"] { - let path = db_base.with_extension(ext); + let mut data_errors = Vec::new(); + for extension in ["db", "db-shm", "db-wal"] { + let path = db_base.with_extension(extension); if path.exists() - && let Err(e) = fs::remove_file(&path) + && let Err(error) = fs::remove_file(&path) { - db_errors.push(format!("{}: {e}", path.display())); + data_errors.push(format!("{}: {error}", path.display())); } } + if session_exists { + fs::remove_file(&config.paths.session_file)?; + } - // 4. Delete session file regardless of DB deletion failures - fs::remove_file(&config.paths.session_file)?; - - if !db_errors.is_empty() { + if service_error.is_some() || !data_errors.is_empty() { + let mut problems = data_errors; + if service_error.is_some() { + problems.push("daemon service cleanup remains unresolved".to_string()); + } return Err(CliError::Other(format!( - "Logged out but some local data could not be deleted: {}", - db_errors.join(", ") + "Logged out, but cleanup needs attention: {}", + problems.join(", ") ))); } - println!("Logged out (session, daemon, and local data cleared)"); + println!("Logged out (daemon, session, and local data cleared)"); Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use async_trait::async_trait; + use std::sync::Mutex; + + struct FakeLifecycle { + result: Mutex>>, + } + + #[async_trait] + impl DaemonLifecycle for FakeLifecycle { + async fn install_and_wait(&self, _config: &Config) -> Result<(), CliError> { + Ok(()) + } + + async fn uninstall(&self, _config: &Config) -> Result { + self.result + .lock() + .unwrap() + .take() + .unwrap_or(Ok(ServiceCleanup::NotInstalled)) + } + } + + fn config(directory: &std::path::Path) -> Config { + Config { + supabase_url: String::new(), + supabase_anon_key: String::new(), + powersync_url: String::new(), + api_url: String::new(), + web_url: None, + paths: flicknote_core::config::ConfigPaths { + config_dir: directory.to_path_buf(), + data_dir: directory.to_path_buf(), + config_file: directory.join("config.json"), + session_file: directory.join("session.json"), + db_file: directory.join("flicknote.db"), + log_file: directory.join("log"), + }, + } + } + + struct OrderingLifecycle { + session_file: std::path::PathBuf, + db_file: std::path::PathBuf, + } + + #[async_trait] + impl DaemonLifecycle for OrderingLifecycle { + async fn install_and_wait(&self, _config: &Config) -> Result<(), CliError> { + Ok(()) + } + + async fn uninstall(&self, _config: &Config) -> Result { + assert!(self.session_file.exists()); + assert!(self.db_file.exists()); + Ok(ServiceCleanup::Removed) + } + } + + #[tokio::test] + async fn successful_logout_removes_service_before_session_and_local_data() { + let directory = tempfile::tempdir().unwrap(); + let config = config(directory.path()); + std::fs::write(&config.paths.session_file, "session").unwrap(); + std::fs::write(&config.paths.db_file, "data").unwrap(); + let lifecycle = OrderingLifecycle { + session_file: config.paths.session_file.clone(), + db_file: config.paths.db_file.clone(), + }; + + run_with_lifecycle(&config, &LogoutArgs { force: false }, &lifecycle) + .await + .unwrap(); + + assert!(!config.paths.session_file.exists()); + assert!(!config.paths.db_file.exists()); + } + + #[tokio::test] + async fn logout_without_session_still_cleans_data_after_service_cleanup() { + let directory = tempfile::tempdir().unwrap(); + let config = config(directory.path()); + std::fs::write(&config.paths.db_file, "stale").unwrap(); + let lifecycle = FakeLifecycle { + result: Mutex::new(Some(Ok(ServiceCleanup::Removed))), + }; + let args = LogoutArgs { force: false }; + + run_with_lifecycle(&config, &args, &lifecycle) + .await + .unwrap(); + + assert!(!config.paths.db_file.exists()); + } + + #[tokio::test] + async fn normal_logout_retains_session_when_service_cleanup_fails() { + let directory = tempfile::tempdir().unwrap(); + let config = config(directory.path()); + std::fs::write(&config.paths.session_file, "session").unwrap(); + let lifecycle = FakeLifecycle { + result: Mutex::new(Some(Err(CliError::Other("stop failed".to_string())))), + }; + let args = LogoutArgs { force: false }; + let result = run_with_lifecycle(&config, &args, &lifecycle).await; + assert!(result.is_err()); + assert!(config.paths.session_file.exists()); + } + + #[tokio::test] + async fn forced_logout_clears_session_after_service_cleanup_failure() { + let directory = tempfile::tempdir().unwrap(); + let config = config(directory.path()); + std::fs::write(&config.paths.session_file, "session").unwrap(); + let lifecycle = FakeLifecycle { + result: Mutex::new(Some(Err(CliError::Other("stop failed".to_string())))), + }; + let args = LogoutArgs { force: true }; + let result = run_with_lifecycle(&config, &args, &lifecycle).await; + assert!(result.is_err()); + assert!(!config.paths.session_file.exists()); + } +} diff --git a/flicknote-cli/src/commands/mod.rs b/flicknote-cli/src/commands/mod.rs index b2711c1..87b99dc 100644 --- a/flicknote-cli/src/commands/mod.rs +++ b/flicknote-cli/src/commands/mod.rs @@ -3,6 +3,7 @@ pub(crate) mod append; pub(crate) mod content; pub(crate) mod count; pub(crate) mod daemon; +pub(crate) mod daemon_lifecycle; pub(crate) mod delete; pub(crate) mod detail; pub(crate) mod edit; @@ -17,10 +18,10 @@ pub(crate) mod modify; pub(crate) mod open; pub(crate) mod project; pub(crate) mod restore; +pub(crate) mod service_manager; pub(crate) mod share; pub(crate) mod skill; pub(crate) mod source; -pub(crate) mod sync; pub(crate) mod topic; pub(crate) mod upload; pub(crate) mod util; diff --git a/flicknote-cli/src/commands/service_manager.rs b/flicknote-cli/src/commands/service_manager.rs new file mode 100644 index 0000000..f089979 --- /dev/null +++ b/flicknote-cli/src/commands/service_manager.rs @@ -0,0 +1,551 @@ +use flicknote_core::config::Config; +use std::ffi::OsString; +use std::fs; +use std::io::{self, Write}; +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; +use std::time::Duration; + +use serde::Serialize; +use thiserror::Error; + +use service_manager::{ + RestartPolicy, ServiceInstallCtx, ServiceLabel, ServiceLevel, ServiceManager as NativeManager, + ServiceManagerKind, ServiceStartCtx, ServiceStatus, ServiceStatusCtx, ServiceStopCtx, + ServiceUninstallCtx, +}; + +pub(crate) const SERVICE_LABEL: &str = "io.guion.flicknote.daemon"; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ServiceState { + NotInstalled, + Stopped, + Running, +} + +#[derive(Debug, Error)] +pub(crate) enum ServiceManagerError { + #[error("user daemon service {operation} failed: {detail}")] + Operation { + operation: &'static str, + detail: String, + }, +} + +impl ServiceManagerError { + pub(crate) fn new(operation: &'static str, detail: impl std::fmt::Display) -> Self { + Self::Operation { + operation, + detail: detail.to_string(), + } + } +} + +#[cfg(any(target_os = "linux", test))] +#[derive(Debug)] +struct CommandStatus { + success: bool, + description: String, +} + +#[cfg(any(target_os = "linux", test))] +trait CommandRunner { + fn status(&self, program: &str, args: &[&str]) -> io::Result; +} + +#[cfg(target_os = "linux")] +struct ProcessCommandRunner; + +#[cfg(target_os = "linux")] +impl CommandRunner for ProcessCommandRunner { + fn status(&self, program: &str, args: &[&str]) -> io::Result { + let status = Command::new(program).args(args).status()?; + Ok(CommandStatus { + success: status.success(), + description: status.to_string(), + }) + } +} + +pub(crate) trait ServiceManagerAdapter { + fn status(&self) -> Result; + fn install(&self, config: &Config) -> Result<(), ServiceManagerError>; + fn reload(&self) -> Result<(), ServiceManagerError>; + fn start(&self) -> Result<(), ServiceManagerError>; + fn stop(&self) -> Result<(), ServiceManagerError>; + fn uninstall(&self) -> Result<(), ServiceManagerError>; +} + +pub(crate) trait ServiceManagerFactory: Send + Sync { + fn manager(&self) -> Result, ServiceManagerError>; +} + +pub(crate) struct NativeServiceFactory; + +impl ServiceManagerFactory for NativeServiceFactory { + fn manager(&self) -> Result, ServiceManagerError> { + Ok(Box::new(NativeServiceManager::new()?)) + } +} + +pub(crate) struct NativeServiceManager { + manager: Box, +} + +impl NativeServiceManager { + pub(crate) fn new() -> Result { + let kind = native_kind()?; + let mut manager = ::target(kind); + manager + .set_level(ServiceLevel::User) + .map_err(|error| ServiceManagerError::new("select user level", error))?; + match manager + .available() + .map_err(|error| ServiceManagerError::new("query availability", error))? + { + true => Ok(Self { manager }), + false => Err(ServiceManagerError::new( + "query availability", + format!("{kind:?} is not available on this host"), + )), + } + } +} + +impl ServiceManagerAdapter for NativeServiceManager { + fn status(&self) -> Result { + self.manager + .status(ServiceStatusCtx { + label: service_label(), + }) + .map(|status| map_status(&status)) + .map_err(|error| ServiceManagerError::new("query status", error)) + } + + fn install(&self, config: &Config) -> Result<(), ServiceManagerError> { + let program = selected_executable() + .map_err(|error| ServiceManagerError::new("validate executable", error))?; + self.manager + .install(ServiceInstallCtx { + label: service_label(), + program: program.clone(), + args: vec![OsString::from("daemon"), OsString::from("run")], + username: None, + working_directory: Some(config.paths.data_dir.clone()), + environment: Some(service_environment(config)), + contents: None, + autostart: true, + restart_policy: RestartPolicy::OnFailure { + delay_secs: Some(5), + max_retries: Some(5), + reset_after_secs: Some(60), + }, + }) + .map_err(|error| ServiceManagerError::new("install", error)) + } + + fn reload(&self) -> Result<(), ServiceManagerError> { + #[cfg(target_os = "linux")] + reload_user_systemd(&ProcessCommandRunner)?; + Ok(()) + } + + fn start(&self) -> Result<(), ServiceManagerError> { + self.manager + .start(ServiceStartCtx { + label: service_label(), + }) + .map_err(|error| ServiceManagerError::new("start", error)) + } + + fn stop(&self) -> Result<(), ServiceManagerError> { + self.manager + .stop(ServiceStopCtx { + label: service_label(), + }) + .map_err(|error| ServiceManagerError::new("stop", error)) + } + + fn uninstall(&self) -> Result<(), ServiceManagerError> { + self.manager + .uninstall(ServiceUninstallCtx { + label: service_label(), + }) + .map_err(|error| ServiceManagerError::new("uninstall", error)) + } +} + +fn service_label() -> ServiceLabel { + SERVICE_LABEL + .parse() + .expect("the built-in daemon service label is valid") +} + +fn map_status(status: &ServiceStatus) -> ServiceState { + match status { + ServiceStatus::NotInstalled => ServiceState::NotInstalled, + ServiceStatus::Running => ServiceState::Running, + ServiceStatus::Stopped(_) => ServiceState::Stopped, + } +} + +fn native_kind() -> Result { + if cfg!(target_os = "macos") { + return Ok(ServiceManagerKind::Launchd); + } + if cfg!(target_os = "linux") { + return Ok(ServiceManagerKind::Systemd); + } + Err(ServiceManagerError::new( + "select platform", + "user launchd and systemd services are the supported platforms", + )) +} + +#[cfg(any(target_os = "linux", test))] +fn reload_user_systemd(runner: &dyn CommandRunner) -> Result<(), ServiceManagerError> { + let status = runner + .status("systemctl", &["--user", "daemon-reload"]) + .map_err(|error| ServiceManagerError::new("reload", error))?; + if status.success { + return Ok(()); + } + Err(ServiceManagerError::new( + "reload", + format!( + "systemctl --user daemon-reload exited with {}", + status.description + ), + )) +} + +fn service_environment(config: &Config) -> Vec<(String, String)> { + let config_root = config + .paths + .config_dir + .parent() + .unwrap_or(&config.paths.config_dir); + let data_root = config + .paths + .data_dir + .parent() + .unwrap_or(&config.paths.data_dir); + let mut environment = vec![ + ("FLICKNOTE_DAEMON_MANAGED".to_string(), "1".to_string()), + ( + "RUST_LOG".to_string(), + "flicknote_sync=info,powersync=debug".to_string(), + ), + ( + "XDG_CONFIG_HOME".to_string(), + config_root.display().to_string(), + ), + ("XDG_DATA_HOME".to_string(), data_root.display().to_string()), + ]; + for name in [ + "FLICKNOTE_ENV", + "FLICKNOTE_SUPABASE_URL", + "FLICKNOTE_SUPABASE_KEY", + "FLICKNOTE_POWERSYNC_URL", + "FLICKNOTE_API_URL", + "FLICKNOTE_WEB_URL", + ] { + if let Ok(value) = std::env::var(name) { + environment.push((name.to_string(), value)); + } + } + environment +} + +pub(crate) fn selected_executable() -> Result { + let argument = std::env::args_os() + .next() + .ok_or_else(|| "the FlickNote executable path is unavailable".to_string())?; + let path = resolve_program_path(&argument)?; + validate_executable(&path)?; + Ok(path) +} + +fn resolve_program_path(argument: &std::ffi::OsStr) -> Result { + let path = PathBuf::from(argument); + if path.components().count() > 1 { + return Ok(if path.is_absolute() { + path + } else { + std::env::current_dir() + .map_err(|error| error.to_string())? + .join(path) + }); + } + let path_variable = std::env::var_os("PATH").unwrap_or_default(); + for directory in std::env::split_paths(&path_variable) { + let candidate = directory.join(&path); + if candidate.is_file() { + return Ok(candidate); + } + } + std::env::current_exe().map_err(|error| error.to_string()) +} + +fn validate_executable(path: &Path) -> Result<(), String> { + let metadata = + fs::metadata(path).map_err(|error| format!("{} is not usable: {error}", path.display()))?; + if !metadata.is_file() { + return Err(format!("{} is not a regular file", path.display())); + } + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + if metadata.permissions().mode() & 0o111 == 0 { + return Err(format!("{} is not executable", path.display())); + } + } + let output = Command::new(path) + .arg("--version") + .stdin(Stdio::null()) + .output() + .map_err(|error| format!("{} could not be executed: {error}", path.display()))?; + if !output.status.success() { + return Err(format!("{} did not identify as FlickNote", path.display())); + } + let identity = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + if !identity.to_ascii_lowercase().contains("flicknote") { + return Err(format!("{} did not identify as FlickNote", path.display())); + } + Ok(()) +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub(crate) struct LogGuidance { + pub(crate) destination: String, + pub(crate) command: String, +} + +pub(crate) fn log_guidance(config: &Config) -> LogGuidance { + if cfg!(target_os = "macos") { + LogGuidance { + destination: config.paths.log_file.display().to_string(), + command: "flicknote daemon logs --follow".to_string(), + } + } else { + LogGuidance { + destination: "systemd user journal".to_string(), + command: "flicknote daemon logs --follow".to_string(), + } + } +} + +pub(crate) async fn show_logs(config: &Config, lines: usize, follow: bool) -> Result<(), String> { + if lines == 0 || lines > 10_000 { + return Err("--lines must be between 1 and 10000".to_string()); + } + if cfg!(target_os = "macos") { + return show_file_logs(&config.paths.log_file, lines, follow).await; + } + if cfg!(target_os = "linux") { + return show_journal_logs(lines, follow).await; + } + Err("daemon logs are supported on macOS and Linux".to_string()) +} + +async fn show_file_logs(path: &Path, lines: usize, follow: bool) -> Result<(), String> { + let initial = read_tail(path, lines)?; + print!("{initial}"); + io::stdout().flush().map_err(|error| error.to_string())?; + if !follow { + return Ok(()); + } + let mut offset = fs::metadata(path).map_err(|error| error.to_string())?.len(); + loop { + tokio::time::sleep(Duration::from_millis(250)).await; + let content = fs::read(path).map_err(|error| error.to_string())?; + if content.len() < offset as usize { + offset = 0; + } + if content.len() > offset as usize { + let chunk = String::from_utf8_lossy(&content[offset as usize..]); + print!("{chunk}"); + io::stdout().flush().map_err(|error| error.to_string())?; + offset = content.len() as u64; + } + } +} + +fn read_tail(path: &Path, lines: usize) -> Result { + let content = fs::read_to_string(path) + .map_err(|error| format!("could not read daemon logs at {}: {error}", path.display()))?; + let all_lines: Vec<&str> = content.lines().collect(); + let start = all_lines.len().saturating_sub(lines); + let mut output = all_lines[start..].join("\n"); + if !output.is_empty() { + output.push('\n'); + } + Ok(output) +} + +async fn show_journal_logs(lines: usize, follow: bool) -> Result<(), String> { + let mut command = tokio::process::Command::new("journalctl"); + command + .args([ + "--user", + "-u", + &format!("{}.service", service_unit_name()), + "-n", + ]) + .arg(lines.to_string()) + .arg("--no-pager"); + if follow { + command.arg("--follow"); + let status = command + .status() + .await + .map_err(|error| format!("could not read systemd user journal: {error}"))?; + if status.success() { + return Ok(()); + } + return Err(format!("journalctl exited with {status}")); + } + let output = command + .output() + .await + .map_err(|error| format!("could not read systemd user journal: {error}"))?; + if !output.status.success() { + return Err(format!( + "journalctl exited with {}: {}", + output.status, + String::from_utf8_lossy(&output.stderr).trim() + )); + } + print!("{}", String::from_utf8_lossy(&output.stdout)); + Ok(()) +} + +pub(crate) fn service_unit_name() -> String { + service_label().to_script_name() +} + +#[cfg(test)] +mod tests { + use super::*; + use flicknote_core::config::ConfigPaths; + + #[test] + fn service_environment_preserves_configured_xdg_roots() { + let root = tempfile::tempdir().unwrap(); + let config = Config { + supabase_url: "https://auth.example".to_string(), + supabase_anon_key: "key".to_string(), + powersync_url: "https://sync.example".to_string(), + api_url: "https://api.example".to_string(), + web_url: None, + paths: ConfigPaths { + config_dir: root.path().join("config/flicknote"), + data_dir: root.path().join("data/flicknote"), + config_file: root.path().join("config/flicknote/config.json"), + session_file: root.path().join("config/flicknote/session.json"), + db_file: root.path().join("data/flicknote/flicknote.db"), + log_file: root.path().join("data/flicknote/flicknote.log"), + }, + }; + let environment = service_environment(&config); + assert!(environment.contains(&( + "XDG_CONFIG_HOME".to_string(), + root.path().join("config").display().to_string() + ))); + assert!(environment.contains(&( + "XDG_DATA_HOME".to_string(), + root.path().join("data").display().to_string() + ))); + } + + #[derive(Default)] + struct FakeCommandRunner { + calls: std::sync::Mutex)>>, + result: std::sync::Mutex>>, + } + + impl CommandRunner for FakeCommandRunner { + fn status(&self, program: &str, args: &[&str]) -> io::Result { + self.calls.lock().unwrap().push(( + program.to_string(), + args.iter().map(ToString::to_string).collect(), + )); + self.result.lock().unwrap().take().unwrap_or_else(|| { + Ok(CommandStatus { + success: true, + description: "exit status: 0".to_string(), + }) + }) + } + } + + #[test] + fn systemd_reload_uses_the_user_manager() { + let runner = FakeCommandRunner::default(); + + reload_user_systemd(&runner).unwrap(); + + assert_eq!( + runner.calls.lock().unwrap().as_slice(), + [( + "systemctl".to_string(), + vec!["--user".to_string(), "daemon-reload".to_string()] + )] + ); + } + + #[test] + fn systemd_reload_propagates_command_failure() { + let runner = FakeCommandRunner { + result: std::sync::Mutex::new(Some(Ok(CommandStatus { + success: false, + description: "exit status: 1".to_string(), + }))), + ..FakeCommandRunner::default() + }; + + let error = reload_user_systemd(&runner).unwrap_err(); + + assert_eq!( + error.to_string(), + "user daemon service reload failed: systemctl --user daemon-reload exited with exit status: 1" + ); + } + + #[test] + fn user_service_uses_the_daemon_label_and_unit_name() { + assert_eq!(SERVICE_LABEL, "io.guion.flicknote.daemon"); + assert_eq!(service_unit_name(), "guion-flicknote.daemon"); + } + + #[test] + fn executable_validation_accepts_flicknote_and_rejects_other_programs() { + let executable = std::env::var("CARGO_BIN_EXE_flicknote") + .map(std::path::PathBuf::from) + .unwrap_or_else(|_| { + std::env::current_exe() + .unwrap() + .parent() + .unwrap() + .parent() + .unwrap() + .join("flicknote") + }); + validate_executable(&executable).unwrap(); + + let directory = tempfile::tempdir().unwrap(); + let other = directory.path().join("other"); + std::fs::write(&other, "#!/bin/sh\necho another-tool 1.0.0\n").unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&other, std::fs::Permissions::from_mode(0o755)).unwrap(); + } + assert!(validate_executable(&other).is_err()); + } +} diff --git a/flicknote-cli/src/commands/sync.rs b/flicknote-cli/src/commands/sync.rs deleted file mode 100644 index 96e16f9..0000000 --- a/flicknote-cli/src/commands/sync.rs +++ /dev/null @@ -1,344 +0,0 @@ -use clap::{Args, Subcommand}; -use flicknote_core::config::Config; -use flicknote_core::error::CliError; -use std::fs; -use std::path::Path; - -const DAEMON_START_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); -const HEALTH_POLL_INTERVAL: std::time::Duration = std::time::Duration::from_millis(100); - -#[derive(Args)] -pub(crate) struct SyncArgs { - #[command(subcommand)] - command: SyncCommand, -} - -#[derive(Subcommand)] -enum SyncCommand { - /// Start the FlickNote daemon in background - Start, - /// Stop the FlickNote daemon - Stop, - /// Check daemon status - Status, - /// Install the local PowerSync daemon as a launchd service (macOS only) - Install, - /// Uninstall the local PowerSync launchd service (macOS only) - Uninstall, -} - -pub(crate) async fn run(config: &Config, args: &SyncArgs) -> Result<(), CliError> { - match &args.command { - SyncCommand::Start => start(config).await, - SyncCommand::Stop => stop(config).await, - SyncCommand::Status => status(config).await, - SyncCommand::Install => install(config).await, - SyncCommand::Uninstall => uninstall(), - } -} - -async fn start(config: &Config) -> Result<(), CliError> { - if let Some(pid) = super::daemon::read_pid(config) { - wait_for_daemon_ready(config, DAEMON_START_TIMEOUT, HEALTH_POLL_INTERVAL).await?; - println!("FlickNote daemon already running (pid {pid})"); - return Ok(()); - } - - let daemon_binary = super::daemon::daemon_binary()?; - start_with_binary(config, &daemon_binary).await -} - -async fn start_with_binary(config: &Config, daemon_binary: &Path) -> Result<(), CliError> { - start_with_binary_and_timeout(config, daemon_binary, DAEMON_START_TIMEOUT).await -} - -async fn start_with_binary_and_timeout( - config: &Config, - daemon_binary: &Path, - timeout: std::time::Duration, -) -> Result<(), CliError> { - let log = fs::OpenOptions::new() - .create(true) - .append(true) - .open(&config.paths.log_file)?; - let log2 = log.try_clone()?; - - let mut command = std::process::Command::new(daemon_binary); - command - .env( - "RUST_LOG", - std::env::var("RUST_LOG") - .unwrap_or_else(|_| "flicknote_sync=info,powersync=debug".into()), - ) - .stdin(std::process::Stdio::null()) - .stdout(log) - .stderr(log2); - #[cfg(unix)] - { - use std::os::unix::process::CommandExt; - - // Manual background start must survive the invoking terminal/session. - // launchd already owns this responsibility for installed macOS services. - #[allow(unsafe_code)] - unsafe { - command.pre_exec(|| { - if libc::setsid() == -1 { - return Err(std::io::Error::last_os_error()); - } - Ok(()) - }); - } - } - let mut child = command.spawn()?; - - let pid = child.id(); - if let Err(error) = wait_for_daemon_ready(config, timeout, HEALTH_POLL_INTERVAL).await { - if let Err(kill_error) = child.kill() - && kill_error.kind() != std::io::ErrorKind::InvalidInput - { - log::warn!("Failed to stop unready daemon process {pid}: {kill_error}"); - } - if let Err(wait_error) = child.wait() { - log::warn!("Failed to reap unready daemon process {pid}: {wait_error}"); - } - return Err(error); - } - println!("FlickNote daemon started (pid {pid})"); - Ok(()) -} - -pub(super) async fn wait_for_daemon_ready( - config: &Config, - timeout: std::time::Duration, - interval: std::time::Duration, -) -> Result<(), CliError> { - let wait = async { - loop { - match flicknote_sync::ipc::DaemonClient::new(config) - .health() - .await - { - Ok(_) => return Ok(()), - Err(error) if !error.retryable() => return Err(CliError::from(error)), - Err(_) => tokio::time::sleep(interval).await, - } - } - }; - tokio::time::timeout(timeout, wait).await.map_err(|_| { - CliError::Other(format!( - "Sync daemon did not become ready within {timeout:?}; check {}", - config.paths.log_file.display() - )) - })? -} - -async fn stop(config: &Config) -> Result<(), CliError> { - let was_running = super::daemon::read_pid(config).is_some() - || flicknote_sync::ipc::socket_path(config).exists(); - super::daemon::stop(config)?; - if !was_running { - println!("FlickNote daemon not running"); - return Ok(()); - } - wait_for_daemon_stopped(config, DAEMON_START_TIMEOUT, HEALTH_POLL_INTERVAL).await?; - let socket = flicknote_sync::ipc::socket_path(config); - if socket.exists() { - fs::remove_file(socket)?; - } - println!("FlickNote daemon stopped"); - Ok(()) -} - -async fn wait_for_daemon_stopped( - config: &Config, - timeout: std::time::Duration, - interval: std::time::Duration, -) -> Result<(), CliError> { - let wait = async { - loop { - let health = flicknote_sync::ipc::DaemonClient::new(config) - .health() - .await; - if matches!(health, Err(ref error) if error.code() == "daemon_unavailable") { - return; - } - tokio::time::sleep(interval).await; - } - }; - tokio::time::timeout(timeout, wait).await.map_err(|_| { - CliError::Other(format!( - "Sync daemon did not stop within {timeout:?}; check {}", - config.paths.log_file.display() - )) - }) -} - -async fn status(config: &Config) -> Result<(), CliError> { - match super::daemon::read_pid(config) { - Some(pid) => { - let info = flicknote_sync::ipc::DaemonClient::new(config) - .health() - .await?; - println!("{}", format_running_status(pid, &info)); - } - None => println!("FlickNote daemon: not running"), - } - Ok(()) -} - -fn format_running_status(pid: u32, info: &flicknote_sync::ipc::ServerInfo) -> String { - format!( - "FlickNote daemon: running (pid {pid}, version {}, protocol {})", - info.version, info.protocol - ) -} - -async fn install(config: &Config) -> Result<(), CliError> { - install_with_timeout(config, DAEMON_START_TIMEOUT).await -} - -async fn install_with_timeout( - config: &Config, - timeout: std::time::Duration, -) -> Result<(), CliError> { - install_local_daemon(config, timeout).await?; - println!("Installed and started: io.guion.flicknote.sync"); - Ok(()) -} - -pub(super) async fn install_local_daemon( - config: &Config, - timeout: std::time::Duration, -) -> Result<(), CliError> { - #[cfg(not(target_os = "macos"))] - { - let (_config, _timeout) = (config, timeout); - validate_launchd_platform() - } - - #[cfg(target_os = "macos")] - { - validate_launchd_platform()?; - // Prove that the shared endpoint is no longer owned by an old launchd or - // standalone daemon before starting the new local LaunchAgent. - super::daemon::stop(config)?; - wait_for_daemon_stopped(config, timeout, HEALTH_POLL_INTERVAL).await?; - super::daemon::install(config)?; - wait_for_daemon_ready(config, timeout, HEALTH_POLL_INTERVAL).await - } -} - -fn validate_launchd_platform() -> Result<(), CliError> { - if !cfg!(target_os = "macos") { - return Err(CliError::Other( - "launchd installation is only supported on macOS; use `flicknote sync start` on this platform".to_string(), - )); - } - Ok(()) -} - -fn uninstall() -> Result<(), CliError> { - validate_launchd_platform()?; - super::daemon::uninstall()?; - println!("Uninstalled: io.guion.flicknote.sync"); - Ok(()) -} - -#[cfg(test)] -mod tests { - use flicknote_core::config::{Config, ConfigPaths}; - #[cfg(unix)] - use std::os::unix::fs::PermissionsExt; - - use super::*; - - fn test_config(dir: &std::path::Path) -> Config { - Config { - supabase_url: String::new(), - supabase_anon_key: String::new(), - powersync_url: String::new(), - api_url: String::new(), - web_url: None, - paths: ConfigPaths { - config_dir: dir.to_path_buf(), - data_dir: dir.to_path_buf(), - config_file: dir.join("config.json"), - session_file: dir.join("session.json"), - db_file: dir.join("flicknote.db"), - log_file: dir.join("flicknote.log"), - }, - } - } - - #[cfg(unix)] - #[tokio::test] - async fn start_does_not_report_success_before_daemon_health_is_ready() { - let dir = tempfile::tempdir().expect("temp dir"); - let config = test_config(dir.path()); - let daemon = dir.path().join("fake-daemon"); - fs::write(&daemon, "#!/bin/sh\nexit 0\n").expect("write fake daemon"); - #[cfg(unix)] - fs::set_permissions(&daemon, fs::Permissions::from_mode(0o700)).expect("chmod fake daemon"); - - let error = - start_with_binary_and_timeout(&config, &daemon, std::time::Duration::from_millis(50)) - .await - .expect_err("a process that exits without serving health is not ready"); - - assert!(!super::super::daemon::pid_file(&config).exists()); - assert!(error.to_string().contains("did not become ready")); - } - - #[cfg(not(target_os = "macos"))] - #[tokio::test] - async fn install_rejects_unsupported_platform_without_waiting() { - let dir = tempfile::tempdir().expect("temp dir"); - let config = test_config(dir.path()); - - let error = install_with_timeout(&config, std::time::Duration::from_millis(20)) - .await - .expect_err("non-macOS install must be rejected immediately"); - - assert!(error.to_string().contains("only supported on macOS")); - } - - #[tokio::test] - async fn stop_waits_until_daemon_health_is_unavailable() { - use tokio::io::{AsyncBufReadExt, AsyncWriteExt}; - - let dir = tempfile::tempdir().expect("temp dir"); - let config = test_config(dir.path()); - let listener = tokio::net::UnixListener::bind(flicknote_sync::ipc::socket_path(&config)) - .expect("bind socket"); - let server = tokio::spawn(async move { - let (stream, _) = listener.accept().await.unwrap(); - let (reader, mut writer) = stream.into_split(); - let mut reader = tokio::io::BufReader::new(reader); - let mut request = String::new(); - reader.read_line(&mut request).await.unwrap(); - let response = serde_json::to_vec(&flicknote_sync::ipc::DaemonResponse::ServerInfo( - flicknote_sync::ipc::ServerInfo::current(), - )) - .unwrap(); - writer.write_all(&response).await.unwrap(); - }); - - wait_for_daemon_stopped( - &config, - std::time::Duration::from_millis(500), - std::time::Duration::from_millis(10), - ) - .await - .unwrap(); - server.await.unwrap(); - } - - #[test] - fn status_line_reports_runtime_version_and_protocol() { - let line = format_running_status(42, &flicknote_sync::ipc::ServerInfo::current()); - - assert!(line.contains("pid 42")); - assert!(line.contains(env!("CARGO_PKG_VERSION"))); - assert!(line.contains("protocol 3")); - } -} diff --git a/flicknote-cli/src/help/root.md b/flicknote-cli/src/help/root.md index e70406b..de2ab5c 100644 --- a/flicknote-cli/src/help/root.md +++ b/flicknote-cli/src/help/root.md @@ -1,4 +1,4 @@ -Data commands require the FlickNote daemon. Start it with `flicknote sync start`. +Data commands require the FlickNote daemon. Start it with `flicknote daemon start`. The daemon owns the local PowerSync database and remote synchronization. Run `flicknote --help` for exact flags and examples. diff --git a/flicknote-cli/src/help/share.md b/flicknote-cli/src/help/share.md index c028d1f..31233d7 100644 --- a/flicknote-cli/src/help/share.md +++ b/flicknote-cli/src/help/share.md @@ -3,4 +3,4 @@ Examples: flicknote share Returns the existing share URL, or creates a permanent share link when none exists. -Uses the sync daemon and current authenticated session. +Uses the FlickNote daemon and current authenticated session. diff --git a/flicknote-cli/src/help/unshare.md b/flicknote-cli/src/help/unshare.md index f5d1381..8fe5d84 100644 --- a/flicknote-cli/src/help/unshare.md +++ b/flicknote-cli/src/help/unshare.md @@ -3,4 +3,4 @@ Examples: flicknote unshare Revokes the note's current share link. -Uses the sync daemon and current authenticated session. +Uses the FlickNote daemon and current authenticated session. diff --git a/flicknote-cli/src/main.rs b/flicknote-cli/src/main.rs index 7a09810..705ddd0 100644 --- a/flicknote-cli/src/main.rs +++ b/flicknote-cli/src/main.rs @@ -65,10 +65,10 @@ enum Commands { Project(commands::project::ProjectArgs), /// Authenticate with FlickNote Login(commands::login::LoginArgs), - /// Log out — remove saved session - Logout, + /// Log out — remove the daemon, session, and local data + Logout(commands::logout::LogoutArgs), /// Manage the FlickNote daemon - Sync(commands::sync::SyncArgs), + Daemon(commands::daemon::DaemonArgs), /// Install agent skills Skill(commands::skill::SkillArgs), /// Import markdown files as notes @@ -81,8 +81,14 @@ enum Commands { #[tokio::main(flavor = "current_thread")] async fn main() { - if let Err(e) = run().await { - eprintln!("Error: {e:#}"); + if let Err(error) = run().await { + if std::env::var_os("FLICKNOTE_DAEMON_MANAGED").is_some() + && matches!(error, CliError::Json(_)) + { + eprintln!("Permanent daemon startup failure: {error:#}"); + return; + } + eprintln!("Error: {error:#}"); std::process::exit(1); } } @@ -101,8 +107,8 @@ async fn run() -> Result<(), CliError> { if let Some(ref cmd) = cli.command { match cmd { Commands::Login(args) => return commands::login::run(&config, args).await, - Commands::Logout => return commands::logout::run(&config).await, - Commands::Sync(args) => return commands::sync::run(&config, args).await, + Commands::Logout(args) => return commands::logout::run(&config, args).await, + Commands::Daemon(args) => return commands::daemon::run(&config, args).await, Commands::Skill(args) => return commands::skill::run(args), Commands::Gateway(args) => return commands::gateway::run(&config, args).await, _ => {} @@ -147,8 +153,8 @@ async fn dispatch(cli: &Cli, daemon: &DaemonClient<'_>) -> Result<(), CliError> Commands::Modify(args) => commands::modify::run(daemon, args).await, Commands::Open(args) => commands::open::run(daemon, args).await, Commands::Import(args) => commands::import::run(daemon, args).await, - // Login/Logout/Sync/Skill are handled before dispatch() is called - Commands::Login(_) | Commands::Logout | Commands::Sync(_) | Commands::Skill(_) => { + // Login/Logout/Daemon/Skill are handled before dispatch() is called + Commands::Login(_) | Commands::Logout(_) | Commands::Daemon(_) | Commands::Skill(_) => { unreachable!() } } diff --git a/flicknote-cli/src/main_tests.rs b/flicknote-cli/src/main_tests.rs index adc3d76..b9dfc63 100644 --- a/flicknote-cli/src/main_tests.rs +++ b/flicknote-cli/src/main_tests.rs @@ -111,6 +111,28 @@ fn modify_rejects_content_editing_and_section_arguments() { } } +#[test] +fn daemon_command_family_parses_and_legacy_sync_is_rejected() { + for command in ["install", "uninstall", "start", "stop", "restart", "run"] { + assert!( + Cli::try_parse_from(["flicknote", "daemon", command]).is_ok(), + "daemon {command} should parse" + ); + } + assert!(Cli::try_parse_from(["flicknote", "daemon", "status"]).is_ok()); + assert!(Cli::try_parse_from(["flicknote", "daemon", "status", "--verbose"]).is_ok()); + assert!(Cli::try_parse_from(["flicknote", "daemon", "status", "--json"]).is_ok()); + assert!( + Cli::try_parse_from(["flicknote", "daemon", "logs", "--lines", "25", "--follow"]).is_ok() + ); + assert!(Cli::try_parse_from(["flicknote", "sync", "start"]).is_err()); +} + +#[test] +fn logout_force_option_parses() { + assert!(Cli::try_parse_from(["flicknote", "logout", "--force"]).is_ok()); +} + #[test] fn mcp_subcommand_parses() { assert!(Cli::try_parse_from(["flicknote", "mcp"]).is_ok()); diff --git a/flicknote-cli/src/mcp/server.rs b/flicknote-cli/src/mcp/server.rs index 8402058..4a518c3 100644 --- a/flicknote-cli/src/mcp/server.rs +++ b/flicknote-cli/src/mcp/server.rs @@ -295,7 +295,7 @@ impl FlickNoteMcp { #[tool( name = "note_add", - description = "Create a note through the sync daemon. A leading H1 becomes the title; a pure HTTP(S) value becomes a link note.", + description = "Create a note through the FlickNote daemon. A leading H1 becomes the title; a pure HTTP(S) value becomes a link note.", annotations(open_world_hint = true) )] async fn note_add( @@ -469,7 +469,7 @@ impl FlickNoteMcp { #[tool( name = "note_share", - description = "Get or create a note share URL through the sync daemon.", + description = "Get or create a note share URL through the FlickNote daemon.", annotations(open_world_hint = true) )] async fn note_share( @@ -486,7 +486,7 @@ impl FlickNoteMcp { #[tool( name = "note_unshare", - description = "Revoke a note share URL through the sync daemon.", + description = "Revoke a note share URL through the FlickNote daemon.", annotations(open_world_hint = true) )] async fn note_unshare( @@ -623,7 +623,7 @@ impl FlickNoteMcp { #[tool( name = "project_share", - description = "Get or create a project share URL through the sync daemon.", + description = "Get or create a project share URL through the FlickNote daemon.", annotations(open_world_hint = true) )] async fn project_share( @@ -639,7 +639,7 @@ impl FlickNoteMcp { #[tool( name = "project_unshare", - description = "Revoke a project share URL through the sync daemon.", + description = "Revoke a project share URL through the FlickNote daemon.", annotations(open_world_hint = true) )] async fn project_unshare( diff --git a/flicknote-cli/tests/daemon_process.rs b/flicknote-cli/tests/daemon_process.rs new file mode 100644 index 0000000..ada018b --- /dev/null +++ b/flicknote-cli/tests/daemon_process.rs @@ -0,0 +1,321 @@ +#![cfg(unix)] + +use std::io::{Read, Write}; +use std::os::unix::net::UnixStream; +use std::path::PathBuf; +use std::process::{Child, Command, Stdio}; +use std::time::{Duration, Instant}; + +use flicknote_sync::ipc::{DaemonRequest, DaemonResponse, PROTOCOL_VERSION}; +use serde_json::json; +use tempfile::TempDir; + +struct DaemonProcess { + _directory: TempDir, + config_home: PathBuf, + data_home: PathBuf, + child: Child, +} + +struct DaemonExit { + status: std::process::ExitStatus, + logs: String, +} + +impl DaemonExit { + fn success(&self) -> bool { + self.status.success() + } +} + +impl DaemonProcess { + fn start() -> Self { + let directory = tempfile::tempdir().unwrap(); + let config_home = directory.path().join("config"); + let data_home = directory.path().join("data"); + let config_dir = config_home.join("flicknote"); + std::fs::create_dir_all(&config_dir).unwrap(); + std::fs::create_dir_all(data_home.join("flicknote")).unwrap(); + let session = json!({ + "sb-test-auth-token": serde_json::to_string(&json!({ + "access_token": "test-token", + "refresh_token": "test-refresh", + "expires_at": 4102444800_u64, + "user": { "id": "daemon-process-test-user", "email": null } + })).unwrap() + }); + std::fs::write( + config_dir.join("session.json"), + serde_json::to_vec(&session).unwrap(), + ) + .unwrap(); + + let child = Command::new(env!("CARGO_BIN_EXE_flicknote")) + .args(["daemon", "run"]) + .env("XDG_CONFIG_HOME", &config_home) + .env("XDG_DATA_HOME", &data_home) + .env("FLICKNOTE_ENV", "dev") + .stdout(Stdio::null()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + Self { + _directory: directory, + config_home, + data_home, + child, + } + } + + fn socket(&self) -> PathBuf { + socket_path_for(&self.data_home) + } + + fn wait_ready(&mut self) { + for _ in 0..200 { + if self.health() { + return; + } + if let Some(status) = self.child.try_wait().unwrap() { + panic!("daemon exited before readiness: {status}"); + } + std::thread::sleep(Duration::from_millis(50)); + } + panic!("daemon did not become IPC-ready"); + } + + fn health(&self) -> bool { + let Ok(mut stream) = UnixStream::connect(self.socket()) else { + return false; + }; + let request = serde_json::to_vec(&DaemonRequest::Health { + protocol: PROTOCOL_VERSION, + }) + .unwrap(); + if stream.write_all(&request).is_err() + || stream.shutdown(std::net::Shutdown::Write).is_err() + { + return false; + } + let mut response = Vec::new(); + if stream.read_to_end(&mut response).is_err() { + return false; + } + matches!( + serde_json::from_slice::(&response), + Ok(DaemonResponse::ServerInfo(info)) if info.protocol == PROTOCOL_VERSION + ) + } + + #[allow(unsafe_code)] + fn signal(&mut self, signal: libc::c_int) -> DaemonExit { + let result = unsafe { libc::kill(self.child.id() as libc::pid_t, signal) }; + assert_eq!(result, 0, "failed to signal daemon process"); + let status = wait_for_exit(&mut self.child, Duration::from_secs(12)) + .unwrap_or_else(|| panic!("daemon did not exit within the shutdown budget")); + let mut logs = String::new(); + if let Some(mut stderr) = self.child.stderr.take() { + stderr.read_to_string(&mut logs).unwrap(); + } + DaemonExit { status, logs } + } +} + +impl Drop for DaemonProcess { + fn drop(&mut self) { + if self.child.try_wait().unwrap().is_none() { + #[allow(unsafe_code)] + unsafe { + libc::kill(self.child.id() as libc::pid_t, libc::SIGKILL); + } + #[allow(clippy::let_underscore_untyped)] + let _ = wait_for_exit(&mut self.child, Duration::from_secs(2)); + } + } +} + +fn wait_for_exit(child: &mut Child, timeout: Duration) -> Option { + let deadline = Instant::now() + timeout; + loop { + if let Some(status) = child.try_wait().unwrap() { + return Some(status); + } + if Instant::now() >= deadline { + return None; + } + std::thread::sleep(Duration::from_millis(25)); + } +} + +fn socket_path_for(data_home: &std::path::Path) -> PathBuf { + data_home.join("flicknote").join("daemon.sock") +} + +fn start_with_roots(process: &DaemonProcess) -> DaemonProcess { + let child = Command::new(env!("CARGO_BIN_EXE_flicknote")) + .args(["daemon", "run"]) + .env("XDG_CONFIG_HOME", &process.config_home) + .env("XDG_DATA_HOME", &process.data_home) + .env("FLICKNOTE_ENV", "dev") + .stdout(Stdio::null()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + DaemonProcess { + _directory: tempfile::tempdir().unwrap(), + config_home: process.config_home.clone(), + data_home: process.data_home.clone(), + child, + } +} + +#[test] +fn foreground_run_rejects_missing_auth_before_ownership() { + let directory = tempfile::tempdir().unwrap(); + let config_home = directory.path().join("config"); + let data_home = directory.path().join("data"); + let output = Command::new(env!("CARGO_BIN_EXE_flicknote")) + .args(["daemon", "run"]) + .env("XDG_CONFIG_HOME", &config_home) + .env("XDG_DATA_HOME", &data_home) + .output() + .unwrap(); + assert!(!output.status.success()); + assert!(String::from_utf8_lossy(&output.stderr).contains("Not authenticated")); + assert!(!data_home.join("flicknote").join("daemon.lock").exists()); +} + +#[test] +fn managed_foreground_permanent_auth_failure_exits_without_restarting() { + let directory = tempfile::tempdir().unwrap(); + let config_home = directory.path().join("config"); + let data_home = directory.path().join("data"); + let output = Command::new(env!("CARGO_BIN_EXE_flicknote")) + .args(["daemon", "run"]) + .env("XDG_CONFIG_HOME", &config_home) + .env("XDG_DATA_HOME", &data_home) + .env("FLICKNOTE_DAEMON_MANAGED", "1") + .output() + .unwrap(); + assert!(output.status.success()); + assert!(!data_home.join("flicknote").join("daemon.lock").exists()); +} + +#[cfg(target_os = "macos")] +#[test] +fn managed_daemon_redirects_main_error_output_to_its_log() { + let directory = tempfile::tempdir().unwrap(); + let config_home = directory.path().join("config"); + let data_home = directory.path().join("data"); + let config_dir = config_home.join("flicknote"); + let data_dir = data_home.join("flicknote"); + std::fs::create_dir_all(&config_dir).unwrap(); + std::fs::create_dir_all(&data_dir).unwrap(); + let session = json!({ + "sb-test-auth-token": serde_json::to_string(&json!({ + "access_token": "test-token", + "refresh_token": "test-refresh", + "expires_at": 4102444800_u64, + "user": { "id": "daemon-log-test-user", "email": null } + })).unwrap() + }); + std::fs::write( + config_dir.join("session.json"), + serde_json::to_vec(&session).unwrap(), + ) + .unwrap(); + std::fs::create_dir(data_dir.join("flicknote.db")).unwrap(); + + let output = Command::new(env!("CARGO_BIN_EXE_flicknote")) + .args(["daemon", "run"]) + .env("XDG_CONFIG_HOME", &config_home) + .env("XDG_DATA_HOME", &data_home) + .env("FLICKNOTE_ENV", "dev") + .env("FLICKNOTE_DAEMON_MANAGED", "1") + .output() + .unwrap(); + + assert!(!output.status.success()); + assert!(output.stderr.is_empty()); + let logs = std::fs::read_to_string(data_dir.join("flicknote.log")).unwrap(); + assert!(logs.contains("Error: FlickNote daemon failed")); +} + +#[test] +fn second_foreground_daemon_cannot_take_ownership_or_remove_the_socket() { + let mut first = DaemonProcess::start(); + first.wait_ready(); + let second = Command::new(env!("CARGO_BIN_EXE_flicknote")) + .args(["daemon", "run"]) + .env("XDG_CONFIG_HOME", &first.config_home) + .env("XDG_DATA_HOME", &first.data_home) + .output() + .unwrap(); + assert!(!second.status.success()); + assert!(String::from_utf8_lossy(&second.stderr).contains("already owns")); + + let managed = Command::new(env!("CARGO_BIN_EXE_flicknote")) + .args(["daemon", "run"]) + .env("XDG_CONFIG_HOME", &first.config_home) + .env("XDG_DATA_HOME", &first.data_home) + .env("FLICKNOTE_DAEMON_MANAGED", "1") + .output() + .unwrap(); + assert!( + !managed.status.success(), + "a managed ownership conflict must request an OS restart" + ); + let mut managed_diagnostics = String::from_utf8_lossy(&managed.stderr).into_owned(); + let managed_log = first.data_home.join("flicknote").join("flicknote.log"); + if managed_log.exists() { + managed_diagnostics.push_str(&std::fs::read_to_string(managed_log).unwrap()); + } + assert!(managed_diagnostics.contains("already owns")); + + assert!(first.socket().exists()); + assert!(first.signal(libc::SIGTERM).success()); + assert!(!first.socket().exists()); +} + +#[test] +fn both_signals_use_graceful_shutdown_and_allow_restart() { + for signal in [libc::SIGINT, libc::SIGTERM] { + let mut process = DaemonProcess::start(); + process.wait_ready(); + let exit = process.signal(signal); + assert!(exit.success()); + assert!( + exit.logs + .contains("Shutdown stage: stop accepting and drain IPC") + ); + assert!(exit.logs.contains("Shutdown stage: disconnect PowerSync")); + assert!(exit.logs.contains("Shutdown stage: truncate WAL")); + assert!(exit.logs.contains("Daemon shutdown coordinator finished")); + assert!(!process.socket().exists()); + let mut restarted = start_with_roots(&process); + restarted.wait_ready(); + assert!(restarted.signal(libc::SIGINT).success()); + } +} + +#[test] +fn forced_termination_releases_lock_and_stale_socket_is_reclaimed() { + let mut first = DaemonProcess::start(); + first.wait_ready(); + assert!(!first.signal(libc::SIGKILL).success()); + assert!(first.socket().exists()); + + let mut restarted = start_with_roots(&first); + restarted.wait_ready(); + assert!(restarted.signal(libc::SIGINT).success()); +} + +#[test] +fn different_data_directories_can_run_concurrently() { + let mut first = DaemonProcess::start(); + let mut second = DaemonProcess::start(); + first.wait_ready(); + second.wait_ready(); + assert!(first.signal(libc::SIGINT).success()); + assert!(second.signal(libc::SIGINT).success()); +} diff --git a/flicknote-cli/tests/mcp_stdio.rs b/flicknote-cli/tests/mcp_stdio.rs index 34c5e29..2d61fa7 100644 --- a/flicknote-cli/tests/mcp_stdio.rs +++ b/flicknote-cli/tests/mcp_stdio.rs @@ -844,7 +844,7 @@ fn mcp_requires_daemon_before_protocol_output() { assert!(!output.status.success()); assert!(output.stdout.is_empty()); - assert!(String::from_utf8_lossy(&output.stderr).contains("Sync daemon is unavailable")); + assert!(String::from_utf8_lossy(&output.stderr).contains("FlickNote daemon is unavailable")); } #[test] @@ -859,7 +859,7 @@ fn data_commands_require_the_daemon() { assert!(!output.status.success()); assert!(output.stdout.is_empty()); - assert!(String::from_utf8_lossy(&output.stderr).contains("Sync daemon is unavailable")); + assert!(String::from_utf8_lossy(&output.stderr).contains("FlickNote daemon is unavailable")); } #[test] diff --git a/flicknote-core/src/config.rs b/flicknote-core/src/config.rs index eca3b76..0eaaf38 100644 --- a/flicknote-core/src/config.rs +++ b/flicknote-core/src/config.rs @@ -1,6 +1,7 @@ use std::fs; use std::path::PathBuf; +#[derive(Clone)] pub struct Config { pub supabase_url: String, pub supabase_anon_key: String, @@ -10,6 +11,7 @@ pub struct Config { pub paths: ConfigPaths, } +#[derive(Clone)] pub struct ConfigPaths { pub config_dir: PathBuf, pub data_dir: PathBuf, diff --git a/flicknote-core/src/services/error.rs b/flicknote-core/src/services/error.rs index 46e65d1..13064dd 100644 --- a/flicknote-core/src/services/error.rs +++ b/flicknote-core/src/services/error.rs @@ -22,9 +22,9 @@ pub enum ServiceError { NoSource, #[error("Nothing to modify")] NothingToModify, - #[error("Sync daemon is unavailable: {0}")] + #[error("FlickNote daemon is unavailable: {0}")] DaemonUnavailable(String), - #[error("Sync daemon request failed: {0}")] + #[error("FlickNote daemon request failed: {0}")] Daemon(String), #[error("{message}")] Remote { diff --git a/flicknote-sync/Cargo.toml b/flicknote-sync/Cargo.toml index d87a154..fc06732 100644 --- a/flicknote-sync/Cargo.toml +++ b/flicknote-sync/Cargo.toml @@ -2,7 +2,7 @@ name = "flicknote-sync" version.workspace = true edition = "2024" -description = "Background sync daemon for FlickNote CLI" +description = "Daemon application host for FlickNote CLI" homepage = "https://github.com/guionai/flicknote-cli" repository = "https://github.com/guionai/flicknote-cli" license = "MIT" @@ -22,12 +22,14 @@ serde = { workspace = true } serde_json = { workspace = true } futures-lite = { workspace = true } log = "0.4" -libc = "0.2.182" uuid = { workspace = true } chrono = "0.4" +fs4 = { workspace = true } +thiserror = { workspace = true } [lints] workspace = true [dev-dependencies] tempfile = "3" +tokio = { workspace = true, features = ["test-util"] } diff --git a/flicknote-sync/src/ipc/client.rs b/flicknote-sync/src/ipc/client.rs index 99a1230..4186bb8 100644 --- a/flicknote-sync/src/ipc/client.rs +++ b/flicknote-sync/src/ipc/client.rs @@ -6,7 +6,7 @@ const IPC_HEALTH_RESPONSE_TIMEOUT: std::time::Duration = std::time::Duration::fr const IPC_APP_RESPONSE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(300); pub fn socket_path(config: &Config) -> PathBuf { - config.paths.data_dir.join("sync.sock") + config.paths.data_dir.join("daemon.sock") } pub(crate) fn unavailable(path: &std::path::Path, stage: &str) -> DaemonError { @@ -26,8 +26,7 @@ pub(crate) fn request_timeout_error( } DaemonError::Other { message: format!( - "Timed out while {stage} from the sync daemon at {}; the application request outcome is unknown. Do not retry it automatically.", - path.display() + "Timed out while {stage}; the application request outcome is unknown. Do not retry it automatically." ), } } @@ -52,8 +51,8 @@ pub async fn send_request( request: &DaemonRequest, ) -> Result { let path = socket_path(config); - let request_bytes = serde_json::to_vec(request).map_err(|e| DaemonError::Other { - message: format!("Failed to serialize daemon request: {e}"), + let request_bytes = serde_json::to_vec(request).map_err(|error| DaemonError::Other { + message: format!("Failed to serialize daemon request: {error}"), })?; let mut stream = tokio::time::timeout(IPC_CONNECT_TIMEOUT, UnixStream::connect(&path)) .await @@ -89,18 +88,20 @@ pub async fn send_request( } None => stream.read_to_end(&mut buf).await, } - .map_err(|e| DaemonError::PostConnectTransport { - message: format!("Failed to read daemon response: {e}"), + .map_err(|error| DaemonError::PostConnectTransport { + message: format!("Failed to read daemon response: {error}"), })?; - serde_json::from_slice(&buf).map_err(|e| { - if e.is_eof() { + serde_json::from_slice(&buf).map_err(|error| { + if error.is_eof() { return DaemonError::IncompleteResponse { - message: format!("Daemon closed the connection before a complete response: {e}"), + message: format!( + "Daemon closed the connection before a complete response: {error}" + ), }; } match serde_json::from_slice::(&buf) { Ok(_) => DaemonError::InvalidResponse { - message: format!("Daemon returned an incompatible response: {e}"), + message: format!("Daemon returned an incompatible response: {error}"), }, Err(raw_error) => DaemonError::MalformedResponse { message: format!("Daemon returned a malformed response: {raw_error}"), @@ -123,17 +124,19 @@ impl<'a> DaemonClient<'a> { send_request(self.config, &request) .await .map_err(|error| match error { - DaemonError::Unavailable { .. } => ServiceError::DaemonUnavailable(format!( - "{error}. Start it with `flicknote sync start`." - )), + DaemonError::Unavailable { .. } => ServiceError::DaemonUnavailable( + "Check `flicknote daemon status` and start it with `flicknote daemon start`." + .to_string(), + ), DaemonError::IncompleteResponse { .. } | DaemonError::MalformedResponse { .. } | DaemonError::PostConnectTransport { .. } if !is_mutating => { - ServiceError::DaemonUnavailable(format!( - "Sync daemon is not ready: {error}. Start it with `flicknote sync start`." - )) + ServiceError::DaemonUnavailable( + "The FlickNote daemon is not ready. Check `flicknote daemon status` and start it with `flicknote daemon start`." + .to_string(), + ) } DaemonError::IncompleteResponse { message } | DaemonError::MalformedResponse { message } @@ -143,7 +146,7 @@ impl<'a> DaemonClient<'a> { { Self::outcome_unknown(message) } - DaemonError::InvalidResponse { .. } => Self::protocol_mismatch(), + DaemonError::InvalidResponse { .. } => Self::protocol_mismatch(None), other => ServiceError::Daemon(other.to_string()), }) } @@ -156,8 +159,12 @@ impl<'a> DaemonClient<'a> { .await? { DaemonResponse::ServerInfo(info) if info.protocol == PROTOCOL_VERSION => Ok(info), + DaemonResponse::ServerInfo(info) => Err(Self::protocol_mismatch(Some(&info))), + DaemonResponse::AppError(error) if error.code == PROTOCOL_MISMATCH_CODE => { + Err(Self::protocol_mismatch_from_details(error.details.as_ref())) + } DaemonResponse::AppError(error) => Err(Self::remote_error(error)), - _ => Err(Self::protocol_mismatch()), + _ => Err(Self::protocol_mismatch(None)), } } @@ -176,7 +183,7 @@ impl<'a> DaemonClient<'a> { "The daemon returned an unexpected envelope after a mutating request; the operation outcome is unknown." .to_string(), )), - _ => Err(Self::protocol_mismatch()), + _ => Err(Self::protocol_mismatch(None)), } } @@ -189,7 +196,7 @@ impl<'a> DaemonClient<'a> { "The daemon returned an unexpected response after a mutating request; the operation outcome is unknown.".to_string(), ) } else { - Self::protocol_mismatch() + Self::protocol_mismatch(None) } }) } @@ -203,12 +210,49 @@ impl<'a> DaemonClient<'a> { } } - fn protocol_mismatch() -> ServiceError { + fn protocol_mismatch(info: Option<&ServerInfo>) -> ServiceError { + let details = info.map(|info| { + serde_json::json!({ + "daemon_executable": info.executable, + "daemon_version": info.version, + "daemon_protocol": info.protocol, + }) + }); + Self::protocol_mismatch_from_details(details.as_ref()) + } + + fn protocol_mismatch_from_details(details: Option<&serde_json::Value>) -> ServiceError { + let daemon_executable = details + .and_then(|value| value.get("daemon_executable")) + .and_then(serde_json::Value::as_str); + let daemon_version = details + .and_then(|value| value.get("daemon_version")) + .and_then(serde_json::Value::as_str); + let daemon_protocol = details + .and_then(|value| value.get("daemon_protocol")) + .and_then(serde_json::Value::as_u64); + let daemon_diagnostics = match (daemon_executable, daemon_version, daemon_protocol) { + (Some(executable), Some(version), Some(protocol)) => format!( + "daemon executable {executable}, daemon version {version} protocol {protocol}" + ), + _ => "daemon executable/version/protocol unavailable".to_string(), + }; + let message = format!( + "The running FlickNote daemon is incompatible: CLI version {} protocol {}; {daemon_diagnostics}. Restart it with `flicknote daemon restart`.", + env!("CARGO_PKG_VERSION"), + PROTOCOL_VERSION, + ); ServiceError::Remote { - code: "daemon_protocol_mismatch".to_string(), - message: "The running sync daemon uses an incompatible protocol. Restart it with `flicknote sync stop && flicknote sync start`.".to_string(), + code: PROTOCOL_MISMATCH_CODE.to_string(), + message, retryable: false, - details: None, + details: Some(serde_json::json!({ + "cli_version": env!("CARGO_PKG_VERSION"), + "cli_protocol": PROTOCOL_VERSION, + "daemon_executable": daemon_executable, + "daemon_version": daemon_version, + "daemon_protocol": daemon_protocol, + })), } } diff --git a/flicknote-sync/src/ipc/mod.rs b/flicknote-sync/src/ipc/mod.rs index 90e692e..0d9ce5b 100644 --- a/flicknote-sync/src/ipc/mod.rs +++ b/flicknote-sync/src/ipc/mod.rs @@ -28,7 +28,10 @@ pub use client::{DaemonClient, send_request, socket_path}; pub use protocol::*; #[cfg(test)] pub(crate) use server::write_json; -pub use server::{read_request, serve_app, serve_app_once, write_response}; +pub use server::{ + ServerInfoProvider, read_request, serve_app, serve_app_once, serve_app_until, + serve_app_until_with_provider, write_response, +}; #[cfg(test)] mod tests; diff --git a/flicknote-sync/src/ipc/protocol.rs b/flicknote-sync/src/ipc/protocol.rs index c3a8a94..bd7f55c 100644 --- a/flicknote-sync/src/ipc/protocol.rs +++ b/flicknote-sync/src/ipc/protocol.rs @@ -1,11 +1,31 @@ use super::*; -pub const PROTOCOL_VERSION: u16 = 3; +pub const PROTOCOL_VERSION: u16 = 4; +pub const PROTOCOL_MISMATCH_CODE: &str = "daemon_protocol_mismatch"; + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum SyncConnectionState { + Connected, + Connecting, + Offline, +} + +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] +pub struct PowerSyncErrors { + pub download: Option, + pub upload: Option, +} #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ServerInfo { pub protocol: u16, pub version: String, + pub executable: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub sync: Option, + #[serde(default)] + pub sync_errors: PowerSyncErrors, } impl ServerInfo { @@ -13,8 +33,31 @@ impl ServerInfo { Self { protocol: PROTOCOL_VERSION, version: env!("CARGO_PKG_VERSION").to_string(), + executable: current_executable(), + sync: None, + sync_errors: PowerSyncErrors::default(), } } + + pub fn with_sync_status( + mut self, + sync: SyncConnectionState, + sync_errors: PowerSyncErrors, + ) -> Self { + self.sync = Some(sync); + self.sync_errors = sync_errors; + self + } +} + +fn current_executable() -> String { + std::env::current_exe() + .ok() + .or_else(|| std::env::args_os().next().map(std::path::PathBuf::from)) + .map_or_else( + || "unavailable".to_string(), + |path| path.display().to_string(), + ) } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -347,7 +390,7 @@ impl fmt::Display for DaemonError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::Unavailable { path, message } => { - write!(f, "Sync daemon is not available at {path}: {message}") + write!(f, "FlickNote daemon is not available at {path}: {message}") } Self::PartialCreate { message, .. } | Self::AmbiguousCreate { message, .. } diff --git a/flicknote-sync/src/ipc/server.rs b/flicknote-sync/src/ipc/server.rs index 6844f2b..f536961 100644 --- a/flicknote-sync/src/ipc/server.rs +++ b/flicknote-sync/src/ipc/server.rs @@ -1,15 +1,22 @@ use super::*; +use std::sync::Arc; +use std::time::Duration; + +use tokio::sync::watch; +use tokio::task::JoinSet; + +pub type ServerInfoProvider = Arc ServerInfo + Send + Sync>; pub async fn read_request(stream: &mut UnixStream) -> Result { let mut buf = Vec::new(); stream .read_to_end(&mut buf) .await - .map_err(|e| DaemonError::Other { - message: format!("Failed to read daemon request: {e}"), + .map_err(|error| DaemonError::Other { + message: format!("Failed to read daemon request: {error}"), })?; - serde_json::from_slice(&buf).map_err(|e| DaemonError::Other { - message: format!("Failed to parse daemon request: {e}"), + serde_json::from_slice(&buf).map_err(|error| DaemonError::Other { + message: format!("Failed to parse daemon request: {error}"), }) } @@ -22,7 +29,7 @@ pub async fn write_response( pub async fn serve_app_once( listener: UnixListener, - app: std::sync::Arc, + app: Arc, info: ServerInfo, ) -> Result<(), DaemonError> { let (mut stream, _) = listener @@ -31,39 +38,89 @@ pub async fn serve_app_once( .map_err(|error| DaemonError::Other { message: format!("Failed to accept daemon request: {error}"), })?; - serve_app_stream(&mut stream, &app, &info).await + let provider = static_info_provider(info); + serve_app_stream(&mut stream, &app, &provider).await } pub async fn serve_app( listener: UnixListener, - app: std::sync::Arc, + app: Arc, + info: ServerInfo, +) -> Result<(), DaemonError> { + let (shutdown_tx, shutdown_rx) = watch::channel(false); + let _keep_shutdown_sender_alive = shutdown_tx; + serve_app_until(listener, app, info, shutdown_rx).await +} + +pub async fn serve_app_until( + listener: UnixListener, + app: Arc, info: ServerInfo, + shutdown: watch::Receiver, +) -> Result<(), DaemonError> { + serve_app_until_with_provider(listener, app, static_info_provider(info), shutdown).await +} + +pub async fn serve_app_until_with_provider( + listener: UnixListener, + app: Arc, + provider: ServerInfoProvider, + mut shutdown: watch::Receiver, ) -> Result<(), DaemonError> { + let mut requests = JoinSet::new(); loop { - let (mut stream, _) = listener - .accept() - .await - .map_err(|error| DaemonError::Other { - message: format!("Failed to accept daemon request: {error}"), - })?; - let app = std::sync::Arc::clone(&app); - let info = info.clone(); - tokio::spawn(async move { - if let Err(error) = serve_app_stream(&mut stream, &app, &info).await { - log::warn!("application IPC request failed: {error}"); + tokio::select! { + changed = shutdown.changed() => { + if changed.is_err() || *shutdown.borrow() { + break; + } } - }); + accepted = listener.accept() => { + let (mut stream, _) = accepted.map_err(|error| DaemonError::Other { + message: format!("Failed to accept daemon request: {error}"), + })?; + let app = Arc::clone(&app); + let provider = Arc::clone(&provider); + requests.spawn(async move { + if let Err(error) = serve_app_stream(&mut stream, &app, &provider).await { + log::warn!("application IPC request failed: {error}"); + } + }); + } + Some(result) = requests.join_next() => { + if let Err(error) = result { + log::warn!("application IPC task failed: {error}"); + } + } + } + } + + let drain = async { + while let Some(result) = requests.join_next().await { + if let Err(error) = result { + log::warn!("application IPC task failed while draining: {error}"); + } + } + }; + if tokio::time::timeout(Duration::from_secs(2), drain) + .await + .is_err() + { + log::warn!("IPC in-flight request drain exceeded 2s"); + requests.abort_all(); + while requests.join_next().await.is_some() {} } + Ok(()) } -pub(crate) async fn serve_app_stream( +async fn serve_app_stream( stream: &mut UnixStream, app: &Application, - info: &ServerInfo, + provider: &ServerInfoProvider, ) -> Result<(), DaemonError> { let response = match read_request(stream).await? { DaemonRequest::Health { protocol } if protocol == PROTOCOL_VERSION => { - DaemonResponse::ServerInfo(info.clone()) + DaemonResponse::ServerInfo(provider()) } DaemonRequest::App { protocol, request } if protocol == PROTOCOL_VERSION => { match app.handle(*request).await { @@ -72,33 +129,44 @@ pub(crate) async fn serve_app_stream( } } DaemonRequest::Health { protocol } | DaemonRequest::App { protocol, .. } => { + let info = ServerInfo::current(); DaemonResponse::AppError(WireError { - code: "daemon_protocol_mismatch".to_string(), + code: PROTOCOL_MISMATCH_CODE.to_string(), message: format!( - "daemon protocol {PROTOCOL_VERSION} does not support client protocol {protocol}" + "FlickNote daemon version {} uses protocol {PROTOCOL_VERSION}; client sent protocol {protocol}", + env!("CARGO_PKG_VERSION") ), retryable: false, - details: None, + details: Some(serde_json::json!({ + "daemon_executable": info.executable, + "daemon_version": info.version, + "daemon_protocol": info.protocol, + "client_protocol": protocol, + })), }) } }; write_response(stream, &response).await } +fn static_info_provider(info: ServerInfo) -> ServerInfoProvider { + Arc::new(move || info.clone()) +} + pub(crate) async fn write_json( stream: &mut UnixStream, value: &T, ) -> Result<(), DaemonError> { - let bytes = serde_json::to_vec(value).map_err(|e| DaemonError::Other { - message: format!("Failed to serialize daemon message: {e}"), + let bytes = serde_json::to_vec(value).map_err(|error| DaemonError::Other { + message: format!("Failed to serialize daemon message: {error}"), })?; stream .write_all(&bytes) .await - .map_err(|e| DaemonError::Other { - message: format!("Failed to write daemon message: {e}"), + .map_err(|error| DaemonError::Other { + message: format!("Failed to write daemon message: {error}"), })?; - stream.shutdown().await.map_err(|e| DaemonError::Other { - message: format!("Failed to close daemon message: {e}"), + stream.shutdown().await.map_err(|error| DaemonError::Other { + message: format!("Failed to close daemon message: {error}"), }) } diff --git a/flicknote-sync/src/ipc/tests.rs b/flicknote-sync/src/ipc/tests.rs index 7ea0799..b7c878d 100644 --- a/flicknote-sync/src/ipc/tests.rs +++ b/flicknote-sync/src/ipc/tests.rs @@ -16,7 +16,7 @@ fn test_config(directory: &std::path::Path) -> Config { config_file: directory.join("config.json"), session_file: directory.join("session.json"), db_file: directory.join("flicknote.db"), - log_file: directory.join("sync.log"), + log_file: directory.join("daemon.log"), }, } } @@ -56,16 +56,16 @@ fn socket_path_lives_in_data_dir() { config_file: dir.join("config.json"), session_file: dir.join("session.json"), db_file: dir.join("flicknote.db"), - log_file: dir.join("sync.log"), + log_file: dir.join("daemon.log"), }, }; - assert_eq!(socket_path(&config), dir.join("sync.sock")); + assert_eq!(socket_path(&config), dir.join("daemon.sock")); } #[test] fn versioned_health_and_app_requests_have_stable_contracts() { - assert_eq!(PROTOCOL_VERSION, 3); + assert_eq!(PROTOCOL_VERSION, 4); let health = DaemonRequest::Health { protocol: PROTOCOL_VERSION, }; @@ -94,15 +94,21 @@ fn versioned_health_and_app_requests_have_stable_contracts() { } #[test] -fn server_info_only_reports_protocol_and_version() { +fn server_info_reports_precise_runtime_status_contract() { let info = ServerInfo::current(); assert_eq!(info.protocol, PROTOCOL_VERSION); assert!(!info.version.is_empty()); + assert!(!info.executable.is_empty()); assert_eq!( serde_json::to_value(&info).unwrap(), json!({ "protocol": PROTOCOL_VERSION, "version": env!("CARGO_PKG_VERSION"), + "executable": info.executable, + "sync_errors": { + "download": null, + "upload": null, + }, }) ); } @@ -130,7 +136,8 @@ async fn daemon_client_maps_missing_socket_to_retryable_unavailable() { assert_eq!(error.code(), "daemon_unavailable"); assert!(error.retryable()); - assert!(error.to_string().contains("flicknote sync start")); + assert!(error.to_string().contains("flicknote daemon start")); + assert!(!error.to_string().contains("daemon.sock")); } #[tokio::test] @@ -226,13 +233,13 @@ async fn health_rejects_unexpected_daemon_responses() { ) .await; let error = DaemonClient::new(&config).health().await.unwrap_err(); - assert_eq!(error.code(), "daemon_protocol_mismatch"); - assert!(error.to_string().contains("sync stop")); + assert_eq!(error.code(), PROTOCOL_MISMATCH_CODE); + assert!(error.to_string().contains("daemon restart")); server.await.unwrap(); } #[tokio::test] -async fn protocol_v3_client_rejects_protocol_v2_server_info() { +async fn protocol_v4_client_rejects_protocol_v2_server_info() { let directory = tempfile::tempdir().unwrap(); let config = test_config(directory.path()); let server = serve_response( @@ -240,14 +247,54 @@ async fn protocol_v3_client_rejects_protocol_v2_server_info() { DaemonResponse::ServerInfo(ServerInfo { protocol: 2, version: "legacy".to_string(), + executable: "/opt/legacy/flicknote".to_string(), + sync: None, + sync_errors: PowerSyncErrors::default(), + }), + ) + .await; + + let error = DaemonClient::new(&config).health().await.unwrap_err(); + + assert_eq!(error.code(), PROTOCOL_MISMATCH_CODE); + let message = error.to_string(); + assert!(message.contains("CLI version 1.0.0 protocol 4")); + assert!(message.contains("daemon executable /opt/legacy/flicknote")); + assert!(message.contains("daemon version legacy protocol 2")); + assert!(message.contains("daemon restart")); + server.await.unwrap(); +} + +#[tokio::test] +async fn health_preserves_protocol_mismatch_details_from_daemon() { + let directory = tempfile::tempdir().unwrap(); + let config = test_config(directory.path()); + let server = serve_response( + &config, + DaemonResponse::AppError(WireError { + code: PROTOCOL_MISMATCH_CODE.to_string(), + message: "old daemon".to_string(), + retryable: false, + details: Some(json!({ + "daemon_executable": "/usr/local/bin/flicknote", + "daemon_version": "0.8.0", + "daemon_protocol": 2 + })), }), ) .await; let error = DaemonClient::new(&config).health().await.unwrap_err(); - assert_eq!(error.code(), "daemon_protocol_mismatch"); - assert!(error.to_string().contains("sync stop")); + assert_eq!(error.code(), PROTOCOL_MISMATCH_CODE); + match error { + ServiceError::Remote { details, .. } => { + let details = details.unwrap(); + assert_eq!(details["daemon_executable"], "/usr/local/bin/flicknote"); + assert_eq!(details["daemon_version"], "0.8.0"); + } + other => panic!("expected remote protocol details, got {other:?}"), + } server.await.unwrap(); } @@ -274,7 +321,7 @@ async fn application_maps_unknown_envelope_to_protocol_mismatch() { .await .unwrap_err(); - assert_eq!(error.code(), "daemon_protocol_mismatch"); + assert_eq!(error.code(), PROTOCOL_MISMATCH_CODE); assert!(!error.retryable()); server.await.unwrap(); } @@ -360,7 +407,7 @@ async fn unexpected_typed_responses_are_classified_by_mutation_safety() { })) .await .unwrap_err(); - assert_eq!(error.code(), "daemon_protocol_mismatch"); + assert_eq!(error.code(), PROTOCOL_MISMATCH_CODE); server.await.unwrap(); let directory = tempfile::tempdir().unwrap(); @@ -419,9 +466,9 @@ async fn health_maps_legacy_daemon_error_to_protocol_mismatch() { let error = DaemonClient::new(&config).health().await.unwrap_err(); - assert_eq!(error.code(), "daemon_protocol_mismatch"); + assert_eq!(error.code(), PROTOCOL_MISMATCH_CODE); assert!(!error.retryable()); - assert!(error.to_string().contains("sync stop")); + assert!(error.to_string().contains("daemon restart")); assert!(matches!( server.await.unwrap(), DaemonRequest::Health { .. } diff --git a/flicknote-sync/src/lib.rs b/flicknote-sync/src/lib.rs index 67e044c..f139435 100644 --- a/flicknote-sync/src/lib.rs +++ b/flicknote-sync/src/lib.rs @@ -1,12 +1,13 @@ pub mod app; mod connector; pub mod ipc; +mod ownership; mod remote; mod runtime; mod storage_maintenance; mod upload; -pub use runtime::run; +pub use runtime::{DaemonRunError, run}; #[cfg(test)] mod test_support; diff --git a/flicknote-sync/src/ownership.rs b/flicknote-sync/src/ownership.rs new file mode 100644 index 0000000..72fd5e0 --- /dev/null +++ b/flicknote-sync/src/ownership.rs @@ -0,0 +1,112 @@ +use fs4::{FileExt, TryLockError}; +use std::fs::{self, File, OpenOptions}; +use std::io::{self, Seek, SeekFrom, Write}; +use std::path::{Path, PathBuf}; +use std::time::{SystemTime, UNIX_EPOCH}; +use thiserror::Error; + +pub(crate) const LOCK_FILE_NAME: &str = "daemon.lock"; + +#[derive(Debug, Error)] +pub(crate) enum OwnershipError { + #[error( + "FlickNote daemon already owns this data directory ({lock_path}); stop it with `flicknote daemon stop` before running the foreground daemon; lock diagnostics: {metadata}" + )] + AlreadyOwned { + lock_path: PathBuf, + metadata: String, + }, + #[error("Unable to acquire daemon data-directory lock: {0}")] + Io(#[from] io::Error), +} + +/// Holds the kernel lock for the complete lifetime of a daemon. +/// +/// The file remains on disk after the guard is dropped. Its contents are only +/// diagnostic metadata; the open file descriptor is the ownership authority. +#[derive(Debug)] +pub(crate) struct DataDirectoryLock { + _file: File, +} + +impl DataDirectoryLock { + pub(crate) fn acquire(data_dir: &Path) -> Result { + fs::create_dir_all(data_dir)?; + let path = data_dir.join(LOCK_FILE_NAME); + let mut file = OpenOptions::new() + .create(true) + .read(true) + .write(true) + .truncate(false) + .open(&path)?; + + match FileExt::try_lock(&file) { + Ok(()) => {} + Err(TryLockError::WouldBlock) => { + return Err(OwnershipError::AlreadyOwned { + lock_path: path, + metadata: diagnostic_metadata(&file), + }); + } + Err(TryLockError::Error(error)) => return Err(OwnershipError::Io(error)), + } + + file.set_len(0)?; + file.seek(SeekFrom::Start(0))?; + writeln!(file, "pid={}", std::process::id())?; + writeln!(file, "version={}", env!("CARGO_PKG_VERSION"))?; + writeln!(file, "started_at={}", unix_timestamp())?; + file.flush()?; + + Ok(Self { _file: file }) + } +} + +fn diagnostic_metadata(file: &File) -> String { + let Some(mut clone) = file.try_clone().ok() else { + return "unavailable".to_string(); + }; + let mut contents = String::new(); + use std::io::Read; + if clone.read_to_string(&mut contents).is_err() { + return "unavailable".to_string(); + } + let metadata = contents.trim(); + if metadata.is_empty() { + "unavailable".to_string() + } else { + metadata.chars().take(256).collect() + } +} + +fn unix_timestamp() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |duration| duration.as_secs()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn only_one_daemon_can_hold_a_data_directory_lock() { + let directory = tempfile::tempdir().unwrap(); + let first = DataDirectoryLock::acquire(directory.path()).unwrap(); + + let error = DataDirectoryLock::acquire(directory.path()).unwrap_err(); + assert!(error.to_string().contains("already owns")); + assert!(error.to_string().contains("pid=")); + + drop(first); + DataDirectoryLock::acquire(directory.path()).unwrap(); + } + + #[test] + fn different_data_directories_have_independent_ownership() { + let first_directory = tempfile::tempdir().unwrap(); + let second_directory = tempfile::tempdir().unwrap(); + let _first = DataDirectoryLock::acquire(first_directory.path()).unwrap(); + let _second = DataDirectoryLock::acquire(second_directory.path()).unwrap(); + } +} diff --git a/flicknote-sync/src/runtime.rs b/flicknote-sync/src/runtime.rs index 874746c..47ce97e 100644 --- a/flicknote-sync/src/runtime.rs +++ b/flicknote-sync/src/runtime.rs @@ -1,5 +1,7 @@ -use std::path::{Path, PathBuf}; +use std::future::Future; +use std::path::PathBuf; use std::sync::Arc; +use std::time::{Duration, Instant}; use flicknote_auth::client::GoTrueClient; use flicknote_core::{ @@ -10,33 +12,33 @@ use flicknote_core::{ }; use powersync::{ConnectionPool, PowerSyncDatabase, SyncOptions, env::PowerSyncEnvironment}; use tokio::net::UnixListener; +use tokio::sync::watch; +use tokio::task::{JoinHandle, JoinSet}; use crate::app::Application; use crate::ipc; +use crate::ownership::{DataDirectoryLock, OwnershipError}; use crate::remote::{RemoteNoteCreator, RemoteShareGateway}; -use crate::storage_maintenance::{WalCheckpointMode, checkpoint_wal_standalone}; +use crate::storage_maintenance::{ + WalCheckpointMode, checkpoint_wal_standalone, checkpoint_wal_standalone_with_timeout, +}; use crate::upload::FlickNoteConnector; -fn pid_path(config: &Config) -> PathBuf { - PathBuf::from(&config.paths.data_dir).join("sync.pid") -} - -struct PidGuard(PathBuf); - -impl Drop for PidGuard { - fn drop(&mut self) { - if let Err(error) = std::fs::remove_file(&self.0) { - log::warn!("Failed to remove PID file: {error}"); - } - } -} +const IPC_DRAIN_TIMEOUT: Duration = Duration::from_secs(2); +const POWERSYNC_DISCONNECT_TIMEOUT: Duration = Duration::from_secs(4); +const WAL_CHECKPOINT_TIMEOUT: Duration = Duration::from_secs(2); struct SocketGuard(PathBuf); impl Drop for SocketGuard { fn drop(&mut self) { - if let Err(error) = std::fs::remove_file(&self.0) { - log::warn!("Failed to remove socket file: {error}"); + if let Err(error) = std::fs::remove_file(&self.0) + && error.kind() != std::io::ErrorKind::NotFound + { + log::warn!( + "Failed to remove daemon socket {}: {error}", + self.0.display() + ); } } } @@ -58,66 +60,97 @@ fn bind_socket(config: &Config) -> Result<(UnixListener, SocketGuard), Box Result> { - if let Ok(contents) = std::fs::read_to_string(path) - && let Ok(pid) = contents.trim().parse::() - { - let result = unsafe { libc::kill(pid, 0) }; - if result == 0 - || (result == -1 && std::io::Error::last_os_error().raw_os_error() == Some(libc::EPERM)) - { - return Err(format!( - "Sync daemon already running (pid={pid}). Kill it first or delete {}", - path.display() - ) - .into()); - } - log::info!("Removing stale PID file (pid={pid} no longer running)"); - } +struct ActorHandles { + checkpoint: JoinHandle<()>, + socket: JoinHandle>, + powersync: JoinSet<()>, +} - std::fs::write(path, std::process::id().to_string()) - .map_err(|error| format!("Failed to write PID file {}: {error}", path.display()))?; - Ok(PidGuard(path.to_path_buf())) +#[derive(Debug, thiserror::Error)] +pub enum DaemonRunError { + #[error("{0}")] + PermanentStartup(String), + #[error("{0}")] + OwnershipConflict(String), + #[error("{0}")] + Startup(String), + #[error("{0}")] + Runtime(String), } -struct ActorHandles { - checkpoint: tokio::task::JoinHandle<()>, - socket: tokio::task::JoinHandle<()>, +impl DaemonRunError { + pub fn is_permanent_startup(&self) -> bool { + matches!(self, Self::PermanentStartup(_)) + } } -pub async fn run() -> Result<(), Box> { - let config = Arc::new(Config::load()?); - let _pid_guard = check_and_write_pid(&pid_path(&config))?; - let (socket_listener, _socket_guard) = bind_socket(&config)?; - config.validate()?; +/// Run the daemon synchronously in the caller's process. +/// +/// The caller owns the process lifetime. This function never forks, detaches, +/// creates a session, or redirects terminal output. +pub async fn run(config: Config) -> Result<(), DaemonRunError> { + config + .validate() + .map_err(|error| DaemonRunError::PermanentStartup(error.to_string()))?; + // Authentication is checked before the ownership lock, socket, and database + // so an unauthenticated invocation cannot create persistent daemon state. + flicknote_core::session::get_user_id(&config) + .map_err(|error| DaemonRunError::PermanentStartup(error.to_string()))?; + + let config = Arc::new(config); + let _ownership = + DataDirectoryLock::acquire(&config.paths.data_dir).map_err(|error| match error { + OwnershipError::AlreadyOwned { .. } => { + DaemonRunError::OwnershipConflict(error.to_string()) + } + OwnershipError::Io(_) => DaemonRunError::Startup(error.to_string()), + })?; + let db = open_powersync_database(&config) + .map_err(|error| DaemonRunError::Startup(error.to_string()))?; + let powersync_tasks = spawn_powersync_actors(&db); + + startup_checkpoint(config.paths.db_file.clone()).await; + // Force PowerSync's local initialization before advertising IPC readiness. + let reader = db + .reader() + .await + .map_err(|error| DaemonRunError::PermanentStartup(error.to_string()))?; + drop(reader); - let db = open_powersync_database(&config)?; let auth = Arc::new(GoTrueClient::new( &config.supabase_url, &config.supabase_anon_key, &config.paths.session_file, )); - let connector = build_connector(&db, &auth, &config); - - startup_checkpoint(config.paths.db_file.clone()).await; - let backend = open_local_backend(&db, &config)?; + let backend = open_local_backend(&db, &config) + .map_err(|error| DaemonRunError::PermanentStartup(error.to_string()))?; + let app = build_application(backend, &db, &auth, &config); + let (socket_listener, _socket_guard) = + bind_socket(&config).map_err(|error| DaemonRunError::Startup(error.to_string()))?; - log::info!("Sync daemon connecting (pid {})", std::process::id()); - db.connect(SyncOptions::new(connector)).await; - log::info!("Sync daemon connected (pid {})", std::process::id()); + log::info!("FlickNote daemon initialized (pid {})", std::process::id()); + db.connect(SyncOptions::new(build_connector(&db, &auth, &config))) + .await; + log::info!("FlickNote daemon accepting local requests"); + let (shutdown_tx, shutdown_rx) = watch::channel(false); + let socket = spawn_socket_server(socket_listener, app, &db, shutdown_rx); let mut actors = ActorHandles { checkpoint: spawn_checkpoint_worker(config.paths.db_file.clone()), - socket: spawn_socket_server(socket_listener, backend, &db, &auth, &config), + socket, + powersync: powersync_tasks, }; + let result = wait_for_shutdown(&mut actors).await; - shutdown_daemon(&mut actors, &db, config.paths.db_file.clone()).await; - result.map_err(Into::into) + shutdown_daemon(&mut actors, &db, config.paths.db_file.clone(), &shutdown_tx).await; + result.map_err(DaemonRunError::Runtime) +} + +fn spawn_powersync_actors(db: &PowerSyncDatabase) -> JoinSet<()> { + let mut actors = JoinSet::new(); + let abort_handles = db.async_tasks().spawn_with(|future| actors.spawn(future)); + drop(abort_handles); + actors } fn open_powersync_database( @@ -131,7 +164,6 @@ fn open_powersync_database( PowerSyncEnvironment::tokio_timer(), ); let db = PowerSyncDatabase::new(environment, app_schema()); - db.async_tasks().spawn_with_tokio(); Ok(db) } @@ -152,12 +184,19 @@ fn build_connector( async fn startup_checkpoint(db_path: PathBuf) { log::info!("Running startup WAL checkpoint"); - if let Err(error) = tokio::task::spawn_blocking(move || { - checkpoint_wal_standalone(&db_path, "startup", WalCheckpointMode::Truncate) - }) - .await + let task = tokio::task::spawn_blocking(move || { + checkpoint_wal_standalone_with_timeout( + &db_path, + "startup", + WalCheckpointMode::Truncate, + 1_000, + ) + }); + if tokio::time::timeout(WAL_CHECKPOINT_TIMEOUT, task) + .await + .is_err() { - log::error!("Startup WAL checkpoint task panicked: {error}"); + log::warn!("Startup WAL checkpoint exceeded its budget"); } } @@ -169,9 +208,30 @@ fn open_local_backend( Ok(Arc::new(LocalPowerSyncBackend::new(db.clone(), user_id))) } -fn spawn_checkpoint_worker(db_path: PathBuf) -> tokio::task::JoinHandle<()> { +fn build_application( + backend: Arc, + db: &PowerSyncDatabase, + auth: &Arc, + config: &Arc, +) -> Arc { + let http = reqwest::Client::new(); + let creator: Arc = Arc::new(RemoteNoteCreator::new( + db.clone(), + Arc::clone(auth), + http.clone(), + Arc::clone(config), + )); + let gateway: Arc = Arc::new(RemoteShareGateway::new( + http, + Arc::clone(auth), + Arc::clone(config), + )); + Arc::new(Application::new(backend, creator, gateway).with_web_url(config.web_url.clone())) +} + +fn spawn_checkpoint_worker(db_path: PathBuf) -> JoinHandle<()> { tokio::spawn(async move { - let mut interval = tokio::time::interval(std::time::Duration::from_secs(30)); + let mut interval = tokio::time::interval(Duration::from_secs(30)); interval.tick().await; loop { interval.tick().await; @@ -189,35 +249,59 @@ fn spawn_checkpoint_worker(db_path: PathBuf) -> tokio::task::JoinHandle<()> { fn spawn_socket_server( listener: UnixListener, - backend: Arc, + app: Arc, db: &PowerSyncDatabase, - auth: &Arc, - config: &Arc, -) -> tokio::task::JoinHandle<()> { - let http = reqwest::Client::new(); - let creator: Arc = Arc::new(RemoteNoteCreator::new( - db.clone(), - Arc::clone(auth), - http.clone(), - Arc::clone(config), - )); - let gateway: Arc = Arc::new(RemoteShareGateway::new( - http, - Arc::clone(auth), - Arc::clone(config), - )); - let app = - Arc::new(Application::new(backend, creator, gateway).with_web_url(config.web_url.clone())); + shutdown: watch::Receiver, +) -> JoinHandle> { + let db = db.clone(); + let info_provider: ipc::ServerInfoProvider = Arc::new(move || { + let status = db.status(); + let sync = if status.is_connected() { + ipc::SyncConnectionState::Connected + } else if status.is_connecting() { + ipc::SyncConnectionState::Connecting + } else { + ipc::SyncConnectionState::Offline + }; + let sync_errors = ipc::PowerSyncErrors { + download: status.download_error().map(ToString::to_string), + upload: status.upload_error().map(ToString::to_string), + }; + ipc::ServerInfo::current().with_sync_status(sync, sync_errors) + }); tokio::spawn(async move { - if let Err(error) = ipc::serve_app(listener, app, ipc::ServerInfo::current()).await { - log::error!("Application socket server failed: {error}"); - } + ipc::serve_app_until_with_provider(listener, app, info_provider, shutdown).await }) } async fn wait_for_shutdown(actors: &mut ActorHandles) -> Result<(), String> { + wait_for_runtime_event(actors, shutdown_signal()).await +} + +async fn shutdown_signal() -> Result<(), String> { + let mut terminate = tokio::signal::unix::signal(tokio::signal::unix::SignalKind::terminate()) + .map_err(|error| format!("Failed to register SIGTERM handler: {error}"))?; + tokio::select! { + result = tokio::signal::ctrl_c() => { + result.map_err(|error| format!("Failed to receive SIGINT: {error}")) + } + signal = terminate.recv() => { + if signal.is_some() { + Ok(()) + } else { + Err("SIGTERM handler closed unexpectedly".to_string()) + } + } + } +} + +async fn wait_for_runtime_event(actors: &mut ActorHandles, shutdown: F) -> Result<(), String> +where + F: Future>, +{ + tokio::pin!(shutdown); tokio::select! { - _ = tokio::signal::ctrl_c() => Ok(()), + result = &mut shutdown => result, result = &mut actors.checkpoint => { if let Err(error) = &result { log::error!("Checkpoint task panicked: {error}"); @@ -227,26 +311,266 @@ async fn wait_for_shutdown(actors: &mut ActorHandles) -> Result<(), String> { Err(format!("Checkpoint task exited: {result:?}")) } result = &mut actors.socket => { - if let Err(error) = &result { - log::error!("Socket task panicked: {error}"); - } else { - log::error!("Socket task exited unexpectedly"); + match result { + Ok(Ok(())) => Err("IPC server exited unexpectedly".to_string()), + Ok(Err(error)) => Err(format!("IPC server failed: {error}")), + Err(error) => Err(format!("IPC server task panicked: {error}")), + } + } + result = actors.powersync.join_next(), if !actors.powersync.is_empty() => { + match result { + Some(Ok(())) => Err("PowerSync actor exited unexpectedly".to_string()), + Some(Err(error)) => Err(format!("PowerSync actor task panicked: {error}")), + None => Err("PowerSync actor set became empty unexpectedly".to_string()), } - Err(format!("Socket task exited: {result:?}")) } } } -async fn shutdown_daemon(actors: &mut ActorHandles, db: &PowerSyncDatabase, db_path: PathBuf) { - actors.checkpoint.abort(); - actors.socket.abort(); - db.disconnect().await; - if let Err(error) = tokio::task::spawn_blocking(move || { - checkpoint_wal_standalone(&db_path, "shutdown", WalCheckpointMode::Truncate) +#[derive(Debug, Clone, PartialEq, Eq)] +enum ShutdownStageOutcome { + Completed, + TimedOut, + Failed(String), +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct ShutdownStageResult { + stage: &'static str, + outcome: ShutdownStageOutcome, +} + +#[async_trait::async_trait] +trait ShutdownOperations: Sync { + async fn disconnect(&self) -> Result<(), String>; + async fn checkpoint(&self) -> Result<(), String>; +} + +struct RuntimeShutdownOperations<'a> { + db: &'a PowerSyncDatabase, + db_path: PathBuf, +} + +#[async_trait::async_trait] +impl ShutdownOperations for RuntimeShutdownOperations<'_> { + async fn disconnect(&self) -> Result<(), String> { + self.db.disconnect().await; + Ok(()) + } + + async fn checkpoint(&self) -> Result<(), String> { + let checkpoint_path = self.db_path.clone(); + tokio::task::spawn_blocking(move || { + checkpoint_wal_standalone_with_timeout( + &checkpoint_path, + "shutdown", + WalCheckpointMode::Truncate, + 1_000, + ); + }) + .await + .map_err(|error| format!("WAL checkpoint task panicked: {error}")) + } +} + +async fn run_shutdown_stage( + stage: &'static str, + timeout: Duration, + operation: F, +) -> ShutdownStageResult +where + F: Future>, +{ + let started = Instant::now(); + log::info!("Shutdown stage: {stage}"); + let outcome = match tokio::time::timeout(timeout, operation).await { + Ok(Ok(())) => { + log::info!( + "Shutdown stage {stage} completed in {:?}", + started.elapsed() + ); + ShutdownStageOutcome::Completed + } + Ok(Err(error)) => { + log::warn!( + "Shutdown stage {stage} failed after {:?}: {error}", + started.elapsed() + ); + ShutdownStageOutcome::Failed(error) + } + Err(_) => { + log::warn!("Shutdown stage {stage} exceeded {timeout:?}; continuing cleanup"); + ShutdownStageOutcome::TimedOut + } + }; + ShutdownStageResult { stage, outcome } +} + +async fn run_storage_shutdown( + operations: &dyn ShutdownOperations, + disconnect_timeout: Duration, + checkpoint_timeout: Duration, +) -> Vec { + vec![ + run_shutdown_stage( + "disconnect PowerSync", + disconnect_timeout, + operations.disconnect(), + ) + .await, + run_shutdown_stage("truncate WAL", checkpoint_timeout, operations.checkpoint()).await, + ] +} + +async fn shutdown_daemon( + actors: &mut ActorHandles, + db: &PowerSyncDatabase, + db_path: PathBuf, + shutdown: &watch::Sender, +) { + if shutdown.send(true).is_err() { + log::debug!("IPC shutdown coordinator had no active receiver"); + } + let ipc_result = run_shutdown_stage("stop accepting and drain IPC", IPC_DRAIN_TIMEOUT, async { + if actors.socket.is_finished() { + return Ok(()); + } + (&mut actors.socket) + .await + .map_err(|error| format!("IPC server task panicked: {error}"))? + .map_err(|error| format!("IPC server failed during shutdown: {error}")) }) - .await - { - log::error!("Shutdown WAL checkpoint task panicked: {error}"); + .await; + if ipc_result.outcome == ShutdownStageOutcome::TimedOut { + actors.socket.abort(); + } + + let operations = RuntimeShutdownOperations { db, db_path }; + let _stage_results = run_storage_shutdown( + &operations, + POWERSYNC_DISCONNECT_TIMEOUT, + WAL_CHECKPOINT_TIMEOUT, + ) + .await; + actors.powersync.abort_all(); + while actors.powersync.join_next().await.is_some() {} + actors.checkpoint.abort(); + log::info!("Daemon shutdown coordinator finished"); +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Mutex; + + #[test] + fn shutdown_budget_is_the_sum_of_bounded_stages() { + assert_eq!( + IPC_DRAIN_TIMEOUT + POWERSYNC_DISCONNECT_TIMEOUT + WAL_CHECKPOINT_TIMEOUT, + Duration::from_secs(8) + ); + } + + struct FakeShutdownOperations { + events: Mutex>, + stall_disconnect: bool, + stall_checkpoint: bool, + } + + #[async_trait::async_trait] + impl ShutdownOperations for FakeShutdownOperations { + async fn disconnect(&self) -> Result<(), String> { + self.events.lock().unwrap().push("disconnect"); + if self.stall_disconnect { + std::future::pending().await + } else { + Ok(()) + } + } + + async fn checkpoint(&self) -> Result<(), String> { + self.events.lock().unwrap().push("checkpoint"); + if self.stall_checkpoint { + std::future::pending().await + } else { + Ok(()) + } + } + } + + #[tokio::test(start_paused = true)] + async fn stalled_disconnect_times_out_and_checkpoint_still_runs() { + let operations = FakeShutdownOperations { + events: Mutex::new(Vec::new()), + stall_disconnect: true, + stall_checkpoint: false, + }; + let started = Instant::now(); + + let stages = run_storage_shutdown( + &operations, + Duration::from_millis(10), + Duration::from_millis(10), + ) + .await; + + assert!(started.elapsed() < Duration::from_secs(1)); + assert_eq!( + stages, + vec![ + ShutdownStageResult { + stage: "disconnect PowerSync", + outcome: ShutdownStageOutcome::TimedOut, + }, + ShutdownStageResult { + stage: "truncate WAL", + outcome: ShutdownStageOutcome::Completed, + }, + ] + ); + assert_eq!( + operations.events.lock().unwrap().as_slice(), + ["disconnect", "checkpoint"] + ); + } + + #[tokio::test(start_paused = true)] + async fn stalled_checkpoint_times_out_after_disconnect_completes() { + let operations = FakeShutdownOperations { + events: Mutex::new(Vec::new()), + stall_disconnect: false, + stall_checkpoint: true, + }; + + let stages = run_storage_shutdown( + &operations, + Duration::from_millis(10), + Duration::from_millis(10), + ) + .await; + + assert_eq!(stages[0].outcome, ShutdownStageOutcome::Completed); + assert_eq!(stages[1].outcome, ShutdownStageOutcome::TimedOut); + assert_eq!( + operations.events.lock().unwrap().as_slice(), + ["disconnect", "checkpoint"] + ); + } + + #[tokio::test] + async fn unexpected_powersync_actor_completion_is_a_runtime_failure() { + let mut powersync = JoinSet::new(); + powersync.spawn(async {}); + let mut actors = ActorHandles { + checkpoint: tokio::spawn(std::future::pending()), + socket: tokio::spawn(std::future::pending()), + powersync, + }; + + let error = wait_for_runtime_event(&mut actors, std::future::pending()) + .await + .unwrap_err(); + + assert_eq!(error, "PowerSync actor exited unexpectedly"); } - log::info!("Sync daemon stopped"); } diff --git a/flicknote-sync/src/storage_maintenance.rs b/flicknote-sync/src/storage_maintenance.rs index 305d7b6..053b5ea 100644 --- a/flicknote-sync/src/storage_maintenance.rs +++ b/flicknote-sync/src/storage_maintenance.rs @@ -4,60 +4,50 @@ use std::path::Path; /// WAL checkpoint mode passed to [`checkpoint_wal_standalone`]. #[derive(Clone, Copy)] pub(crate) enum WalCheckpointMode { - /// Checkpoints frames up to the oldest active reader's mark. Never acquires - /// PENDING or EXCLUSIVE locks — returns immediately. Safe at any time alongside - /// active pool connections. Returns `busy=1` when readers constrain the - /// checkpoint to an earlier WAL position (normal during runtime). + /// Checkpoints frames up to the oldest active reader's mark without waiting. Passive, - /// Acquires a PENDING lock while waiting for readers to finish, then resets - /// the WAL to zero length. Use only when no pool connections exist (startup, - /// shutdown) to avoid the PENDING lock blocking pool writers. + /// Resets the WAL to zero length after readers finish. Truncate, } impl fmt::Display for WalCheckpointMode { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Self::Passive => write!(f, "PASSIVE"), - Self::Truncate => write!(f, "TRUNCATE"), + Self::Passive => formatter.write_str("PASSIVE"), + Self::Truncate => formatter.write_str("TRUNCATE"), } } } -/// Run a WAL checkpoint using a standalone rusqlite connection. -/// -/// Opens its own connection to the DB file, bypassing PowerSync's writer mutex -/// entirely — competes only at the SQLite file-lock level, not the Rust mutex level. -/// -/// `mode` controls the checkpoint type — see [`WalCheckpointMode`] for semantics. -/// -/// `busy_timeout` is set to 5 000 ms for TRUNCATE so it retries at the SQLite level -/// while pool readers finish their short transactions. It is irrelevant for PASSIVE -/// (which never waits) but harmless to keep set. -/// -/// Reads the `(busy, log, checkpointed)` return tuple from PRAGMA so failures -/// are never silently swallowed. For PASSIVE, `busy=1` when active readers -/// constrain the checkpoint to an earlier WAL position (normal and expected during -/// runtime). For TRUNCATE, `busy=1` means the reset could not complete. -/// -/// This function is **synchronous** (blocking rusqlite I/O). Async callers must -/// wrap it with `tokio::task::spawn_blocking`. -/// -/// `label` identifies the call site in log output (e.g. `"startup"`, `"post-upload"`, -/// `"periodic"`, `"shutdown"`) so production logs are unambiguous. +/// Run the normal WAL checkpoint with the maintenance timeout used by periodic +/// and startup checkpoints. pub(crate) fn checkpoint_wal_standalone(db_path: &Path, label: &str, mode: WalCheckpointMode) { + checkpoint_wal_standalone_with_timeout(db_path, label, mode, 5_000); +} + +/// Run a WAL checkpoint with an explicit SQLite busy timeout. +/// +/// The synchronous operation is always called from a blocking task by async +/// callers. The shutdown coordinator uses a short timeout so a busy database +/// cannot hold process exit open. +pub(crate) fn checkpoint_wal_standalone_with_timeout( + db_path: &Path, + label: &str, + mode: WalCheckpointMode, + busy_timeout_ms: u64, +) { let conn = match rusqlite::Connection::open(db_path) { - Ok(c) => c, - Err(e) => { - log::warn!("WAL checkpoint [{label}]: could not open db: {e}"); + Ok(connection) => connection, + Err(error) => { + log::warn!("WAL checkpoint [{label}]: could not open db: {error}"); return; } }; - if let Err(e) = conn.pragma_update(None, "busy_timeout", 5_000i64) { - log::warn!("WAL checkpoint [{label}]: could not set busy_timeout: {e}"); + if let Err(error) = conn.pragma_update(None, "busy_timeout", busy_timeout_ms as i64) { + log::warn!("WAL checkpoint [{label}]: could not set busy_timeout: {error}"); return; } - let pragma = format!("PRAGMA wal_checkpoint({})", mode); + let pragma = format!("PRAGMA wal_checkpoint({mode})"); match conn.query_row(&pragma, [], |row| { Ok(( row.get::<_, i32>(0)?, @@ -65,18 +55,16 @@ pub(crate) fn checkpoint_wal_standalone(db_path: &Path, label: &str, mode: WalCh row.get::<_, i32>(2)?, )) }) { - Ok((busy, log, checkpointed)) => { - if busy == 0 { - log::info!( - "WAL checkpoint [{label}] ({mode}): {log} pages, {checkpointed} checkpointed" - ); - } else { - log::warn!( - "WAL checkpoint [{label}]: incomplete (busy={busy}, {log} log pages, {checkpointed} checkpointed)" - ); - } + Ok((0, log_pages, checkpointed)) => { + log::info!( + "WAL checkpoint [{label}] ({mode}): {log_pages} pages, {checkpointed} checkpointed" + ); + } + Ok((busy, log_pages, checkpointed)) => { + log::warn!( + "WAL checkpoint [{label}]: incomplete (busy={busy}, {log_pages} log pages, {checkpointed} checkpointed)" + ); } - Err(e) => log::warn!("WAL checkpoint [{label}]: failed: {e}"), + Err(error) => log::warn!("WAL checkpoint [{label}]: failed: {error}"), } - // Connection dropped here — no persistent state } diff --git a/flicknote-sync/src/test_support.rs b/flicknote-sync/src/test_support.rs index 0ca9dbf..33618cd 100644 --- a/flicknote-sync/src/test_support.rs +++ b/flicknote-sync/src/test_support.rs @@ -204,6 +204,7 @@ pub(crate) fn spawn_disconnected_then_retry_responses( let Some((mut stream, _)) = accept() else { break; }; + stream.set_nonblocking(false).unwrap(); let count = stream.read(&mut buffer).unwrap(); requests.push( String::from_utf8_lossy(&buffer[..count]) diff --git a/flicknote-sync/tests/app_contract.rs b/flicknote-sync/tests/app_contract.rs index 0b3825a..88716e9 100644 --- a/flicknote-sync/tests/app_contract.rs +++ b/flicknote-sync/tests/app_contract.rs @@ -11,8 +11,8 @@ use flicknote_core::services::ports::{ }; use flicknote_sync::app::Application; use flicknote_sync::ipc::{ - AppRequest, AppResponse, DaemonClient, DaemonRequest, DaemonResponse, ServerInfo, - serve_app_once, socket_path, + AppRequest, AppResponse, DaemonClient, DaemonRequest, DaemonResponse, PROTOCOL_MISMATCH_CODE, + ServerInfo, serve_app_once, socket_path, }; use powersync::{ConnectionPool, PowerSyncDatabase, env::PowerSyncEnvironment}; @@ -29,7 +29,7 @@ fn test_config(directory: &std::path::Path) -> Config { config_file: directory.join("config.json"), session_file: directory.join("session.json"), db_file: directory.join("flicknote.db"), - log_file: directory.join("sync.log"), + log_file: directory.join("daemon.log"), }, } } @@ -338,7 +338,18 @@ async fn protocol_v1_app_request_is_rejected_before_application_dispatch() { let DaemonResponse::AppError(error) = response else { panic!("expected protocol mismatch") }; - assert_eq!(error.code, "daemon_protocol_mismatch"); + assert_eq!(error.code, PROTOCOL_MISMATCH_CODE); + let details = error.details.unwrap(); + assert!( + details["daemon_executable"] + .as_str() + .is_some_and(|path| !path.is_empty()) + ); + assert_eq!(details["daemon_version"], env!("CARGO_PKG_VERSION")); + assert_eq!( + details["daemon_protocol"], + flicknote_sync::ipc::PROTOCOL_VERSION + ); server.await.unwrap().unwrap(); } diff --git a/justfile b/justfile index f869338..07aea8f 100644 --- a/justfile +++ b/justfile @@ -28,22 +28,18 @@ fmt: clippy: cargo clippy --workspace --all-targets --all-features -- -D warnings -# Install the Rust CLI and sync daemon. +# Install the unified Rust CLI and daemon. install: install-rust # Install the Rust CLI. install-rust: cargo install --path flicknote-cli -# Restart FlickNote launchd services. +# Restart the installed FlickNote user daemon service. restart: - @for label in $(launchctl list 2>/dev/null | awk '/io\.guion\.flicknote/ {print $3}'); do \ - echo "Restarting $label..."; \ - launchctl kickstart -k "gui/$(id -u)/$label"; \ - echo "✓ $label restarted"; \ - done + flicknote daemon restart -# Reinstall both Rust binaries and restart FlickNote launchd services. +# Reinstall the unified executable and restart the FlickNote user daemon service. reinstall: reinstall-rust restart # Force-reinstall the Rust CLI. diff --git a/skills/flicknote.md b/skills/flicknote.md index 76b3e36..be63573 100644 --- a/skills/flicknote.md +++ b/skills/flicknote.md @@ -36,6 +36,10 @@ use restore only when the user explicitly wants the identified archived note back. Do not assume processing or synchronization status is part of the public note contract. +## Daemon recovery + +The MCP server is daemon-backed and never starts services implicitly. If startup or a tool reports an unavailable daemon, recommend `flicknote daemon status` and then `flicknote daemon start`; do not open the PowerSync database directly. A ready local daemon can remain usable while remote PowerSync is offline. + ## Recommended flow Discover with the topic/entity tools, list or find notes, read the selected note, From 6525057dc072b0fd9f528accb90140ca63a9ab1a Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 13 Aug 2026 17:35:51 +0800 Subject: [PATCH 2/5] fix(sync): bound daemon shutdown lifecycle --- flicknote-cli/src/commands/daemon.rs | 25 +- .../src/commands/daemon_lifecycle.rs | 89 +++++-- flicknote-cli/tests/daemon_process.rs | 29 +++ flicknote-sync/src/ipc/mod.rs | 4 +- flicknote-sync/src/ipc/server.rs | 14 +- flicknote-sync/src/runtime.rs | 242 ++++++++++++------ flicknote-sync/src/storage_maintenance.rs | 10 +- 7 files changed, 284 insertions(+), 129 deletions(-) diff --git a/flicknote-cli/src/commands/daemon.rs b/flicknote-cli/src/commands/daemon.rs index e3f4d43..ef6b033 100644 --- a/flicknote-cli/src/commands/daemon.rs +++ b/flicknote-cli/src/commands/daemon.rs @@ -267,6 +267,8 @@ struct StatusReport { protocol: ProtocolStatus, error: Option, service_error: Option, + #[serde(skip)] + service_diagnostic: Option, log_guidance: LogGuidance, } @@ -288,6 +290,7 @@ impl StatusReport { }, error: None, service_error: None, + service_diagnostic: None, log_guidance: log_guidance(config), } } @@ -326,10 +329,11 @@ impl StatusReport { .as_ref() .map(|error| format!("{}: {}", error.code, error.message)) .unwrap_or_else(|| "none".to_string()); + let service_diagnostic = self.service_diagnostic.as_deref().unwrap_or("none"); let download_error = self.sync_errors.download.as_deref().unwrap_or("none"); let upload_error = self.sync_errors.upload.as_deref().unwrap_or("none"); format!( - "service: {}\napplication: {}\ndaemon executable: {}\nFlickNote version: cli {}, daemon {}\nIPC protocol: cli {}, daemon {}\nPowerSync: {}\nPowerSync download error: {}\nPowerSync upload error: {}\nlast error: {}\nservice error: {}\nlogs: {}\nlog command: {}", + "service: {}\napplication: {}\ndaemon executable: {}\nFlickNote version: cli {}, daemon {}\nIPC protocol: cli {}, daemon {}\nPowerSync: {}\nPowerSync download error: {}\nPowerSync upload error: {}\nlast error: {}\nservice error: {}\nservice diagnostics: {}\nlogs: {}\nlog command: {}", format_service_state(self.service_state), format_application_state(self.application_state), self.daemon_executable.as_deref().unwrap_or("unavailable"), @@ -344,6 +348,7 @@ impl StatusReport { upload_error, error, service_error, + service_diagnostic, self.log_guidance.destination, self.log_guidance.command, ) @@ -372,8 +377,11 @@ async fn build_status_report_with_probe( report.service_state = ServiceStatusState::QueryFailed; report.service_error = Some(StatusError { code: "service_manager_query_failed".to_string(), - message: error.to_string(), + message: + "Could not inspect the FlickNote daemon service; see verbose logs for diagnosis" + .to_string(), }); + report.service_diagnostic = Some(error.to_string()); } } @@ -483,19 +491,6 @@ mod tests { } } - #[test] - fn managed_service_restart_classification_uses_typed_startup_failures() { - let permanent = flicknote_sync::DaemonRunError::PermanentStartup( - "wording can change without affecting classification".to_string(), - ); - let ownership = flicknote_sync::DaemonRunError::OwnershipConflict( - "not configured and invalid argument are only words".to_string(), - ); - - assert!(permanent.is_permanent_startup()); - assert!(!ownership.is_permanent_startup()); - } - #[test] fn status_json_is_an_object_with_stable_state_fields() { let directory = tempfile::tempdir().unwrap(); diff --git a/flicknote-cli/src/commands/daemon_lifecycle.rs b/flicknote-cli/src/commands/daemon_lifecycle.rs index 1399c10..9aa1f64 100644 --- a/flicknote-cli/src/commands/daemon_lifecycle.rs +++ b/flicknote-cli/src/commands/daemon_lifecycle.rs @@ -81,12 +81,11 @@ impl<'a> LifecycleController<'a> { } pub(crate) async fn install_and_wait(&self, config: &Config) -> Result<(), CliError> { - let state = self.service_state("query")?; - self.stop_running(config, state).await?; + self.stop_running(config, self.service_state("query")?) + .await?; self.service_call("install", |manager| manager.install(config))?; self.service_call("reload", |manager| manager.reload())?; - self.service_call("start", |manager| manager.start())?; - self.wait_for_ready(config, SERVICE_OPERATION_TIMEOUT).await + self.start_and_wait(config).await } pub(crate) async fn uninstall(&self, config: &Config) -> Result { @@ -112,14 +111,8 @@ impl<'a> LifecycleController<'a> { } pub(crate) async fn start(&self, config: &Config) -> Result<(), CliError> { - if self.service_state("query")? == ServiceState::NotInstalled { - return Err(CliError::Other( - "FlickNote daemon service is not installed; run `flicknote daemon install`" - .to_string(), - )); - } - self.service_call("start", |manager| manager.start())?; - self.wait_for_ready(config, SERVICE_OPERATION_TIMEOUT).await + self.ensure_installed("query")?; + self.start_and_wait(config).await } pub(crate) async fn stop(&self, config: &Config) -> Result { @@ -128,14 +121,23 @@ impl<'a> LifecycleController<'a> { } pub(crate) async fn restart(&self, config: &Config) -> Result<(), CliError> { - let state = self.service_state("query")?; + self.stop_running(config, self.ensure_installed("query")?) + .await?; + self.start_and_wait(config).await + } + + fn ensure_installed(&self, action: &'static str) -> Result { + let state = self.service_state(action)?; if state == ServiceState::NotInstalled { return Err(CliError::Other( "FlickNote daemon service is not installed; run `flicknote daemon install`" .to_string(), )); } - self.stop_running(config, state).await?; + Ok(state) + } + + async fn start_and_wait(&self, config: &Config) -> Result<(), CliError> { self.service_call("start", |manager| manager.start())?; self.wait_for_ready(config, SERVICE_OPERATION_TIMEOUT).await } @@ -236,8 +238,16 @@ impl<'a> LifecycleController<'a> { } pub(crate) fn lifecycle_error(action: &str, error: &ServiceManagerError) -> CliError { + log::error!("FlickNote daemon service {action} operation failed: {error}"); + let guidance = match action { + "start" | "install" => { + "run `flicknote daemon status --verbose` and retry `flicknote daemon install`" + } + "stop" | "uninstall" => "run `flicknote daemon status --verbose` before retrying cleanup", + _ => "run `flicknote daemon status --verbose`", + }; CliError::Other(format!( - "Could not {action} the FlickNote daemon service: {error}; run `flicknote daemon status --verbose` for diagnosis" + "Could not {action} the FlickNote daemon service; {guidance}" )) } @@ -246,7 +256,39 @@ mod tests { use super::*; use flicknote_core::services::error::ServiceError; use std::collections::VecDeque; - use std::sync::{Arc, Mutex}; + use std::sync::{Arc, Mutex, Once}; + + static TEST_LOGGER: LifecycleTestLogger = LifecycleTestLogger; + static TEST_LOGGER_INIT: Once = Once::new(); + static TEST_LOG_RECORDS: Mutex> = Mutex::new(Vec::new()); + + struct LifecycleTestLogger; + + impl log::Log for LifecycleTestLogger { + fn enabled(&self, metadata: &log::Metadata<'_>) -> bool { + metadata.level() == log::Level::Error + && metadata.target().ends_with("commands::daemon_lifecycle") + } + + fn log(&self, record: &log::Record<'_>) { + if self.enabled(record.metadata()) { + TEST_LOG_RECORDS + .lock() + .unwrap() + .push((record.level(), record.args().to_string())); + } + } + + fn flush(&self) {} + } + + fn enable_test_logger() { + TEST_LOGGER_INIT.call_once(|| { + log::set_logger(&TEST_LOGGER) + .expect("test logger should be the first logger installed"); + log::set_max_level(log::LevelFilter::Error); + }); + } struct FakeManager { state: Mutex, @@ -642,7 +684,8 @@ mod tests { } #[test] - fn lifecycle_errors_preserve_platform_diagnostics() { + fn lifecycle_errors_use_flicknote_guidance_and_log_platform_diagnostics() { + enable_test_logger(); let platform = ServiceManagerError::new( "start", "launchctl bootstrap failed: Input/output error (code 5)", @@ -650,9 +693,15 @@ mod tests { let message = lifecycle_error("start", &platform).to_string(); - assert!(message.contains("launchctl bootstrap failed")); - assert!(message.contains("Input/output error (code 5)")); - assert!(message.contains("flicknote daemon status --verbose")); + assert_eq!( + message, + "Could not start the FlickNote daemon service; run `flicknote daemon status --verbose` and retry `flicknote daemon install`" + ); + assert!(TEST_LOG_RECORDS.lock().unwrap().iter().any(|record| { + record.0 == log::Level::Error + && record.1.contains("launchctl bootstrap failed") + && record.1.contains("Input/output error (code 5)") + })); } #[tokio::test] diff --git a/flicknote-cli/tests/daemon_process.rs b/flicknote-cli/tests/daemon_process.rs index ada018b..6239f2c 100644 --- a/flicknote-cli/tests/daemon_process.rs +++ b/flicknote-cli/tests/daemon_process.rs @@ -71,6 +71,24 @@ impl DaemonProcess { socket_path_for(&self.data_home) } + fn wait_for_lock(&mut self) { + for _ in 0..200 { + if self + .data_home + .join("flicknote") + .join("daemon.lock") + .exists() + { + return; + } + if let Some(status) = self.child.try_wait().unwrap() { + panic!("daemon exited before acquiring ownership: {status}"); + } + std::thread::sleep(Duration::from_millis(10)); + } + panic!("daemon did not acquire ownership"); + } + fn wait_ready(&mut self) { for _ in 0..200 { if self.health() { @@ -277,6 +295,17 @@ fn second_foreground_daemon_cannot_take_ownership_or_remove_the_socket() { assert!(!first.socket().exists()); } +#[test] +fn startup_signals_use_graceful_shutdown_before_ipc_readiness() { + for signal in [libc::SIGINT, libc::SIGTERM] { + let mut process = DaemonProcess::start(); + process.wait_for_lock(); + let exit = process.signal(signal); + assert!(exit.success()); + assert!(!process.socket().exists()); + } +} + #[test] fn both_signals_use_graceful_shutdown_and_allow_restart() { for signal in [libc::SIGINT, libc::SIGTERM] { diff --git a/flicknote-sync/src/ipc/mod.rs b/flicknote-sync/src/ipc/mod.rs index 0d9ce5b..4f8ef51 100644 --- a/flicknote-sync/src/ipc/mod.rs +++ b/flicknote-sync/src/ipc/mod.rs @@ -29,8 +29,8 @@ pub use protocol::*; #[cfg(test)] pub(crate) use server::write_json; pub use server::{ - ServerInfoProvider, read_request, serve_app, serve_app_once, serve_app_until, - serve_app_until_with_provider, write_response, + ServerInfoProvider, read_request, serve_app, serve_app_once, serve_app_until_with_provider, + write_response, }; #[cfg(test)] diff --git a/flicknote-sync/src/ipc/server.rs b/flicknote-sync/src/ipc/server.rs index f536961..4e0b971 100644 --- a/flicknote-sync/src/ipc/server.rs +++ b/flicknote-sync/src/ipc/server.rs @@ -47,18 +47,8 @@ pub async fn serve_app( app: Arc, info: ServerInfo, ) -> Result<(), DaemonError> { - let (shutdown_tx, shutdown_rx) = watch::channel(false); - let _keep_shutdown_sender_alive = shutdown_tx; - serve_app_until(listener, app, info, shutdown_rx).await -} - -pub async fn serve_app_until( - listener: UnixListener, - app: Arc, - info: ServerInfo, - shutdown: watch::Receiver, -) -> Result<(), DaemonError> { - serve_app_until_with_provider(listener, app, static_info_provider(info), shutdown).await + let (_shutdown_tx, shutdown_rx) = watch::channel(false); + serve_app_until_with_provider(listener, app, static_info_provider(info), shutdown_rx).await } pub async fn serve_app_until_with_provider( diff --git a/flicknote-sync/src/runtime.rs b/flicknote-sync/src/runtime.rs index 47ce97e..28c4d8e 100644 --- a/flicknote-sync/src/runtime.rs +++ b/flicknote-sync/src/runtime.rs @@ -15,13 +15,14 @@ use tokio::net::UnixListener; use tokio::sync::watch; use tokio::task::{JoinHandle, JoinSet}; +#[cfg(unix)] +use tokio::signal::unix::SignalKind; + use crate::app::Application; use crate::ipc; use crate::ownership::{DataDirectoryLock, OwnershipError}; use crate::remote::{RemoteNoteCreator, RemoteShareGateway}; -use crate::storage_maintenance::{ - WalCheckpointMode, checkpoint_wal_standalone, checkpoint_wal_standalone_with_timeout, -}; +use crate::storage_maintenance::{WalCheckpointMode, checkpoint_wal_standalone_with_timeout}; use crate::upload::FlickNoteConnector; const IPC_DRAIN_TIMEOUT: Duration = Duration::from_secs(2); @@ -66,6 +67,57 @@ struct ActorHandles { powersync: JoinSet<()>, } +struct StartupSignals { + receiver: watch::Receiver, + task: JoinHandle<()>, +} + +impl StartupSignals { + fn register() -> Result { + let (sender, receiver) = watch::channel(false); + #[cfg(unix)] + { + let mut interrupt = tokio::signal::unix::signal(SignalKind::interrupt())?; + let mut terminate = tokio::signal::unix::signal(SignalKind::terminate())?; + let task = tokio::spawn(async move { + tokio::select! { + _ = interrupt.recv() => {} + _ = terminate.recv() => {} + } + if sender.send(true).is_err() { + log::debug!("daemon startup signal receiver was dropped"); + } + }); + Ok(Self { receiver, task }) + } + #[cfg(not(unix))] + { + let task = tokio::spawn(std::future::pending::<()>()); + Ok(Self { receiver, task }) + } + } + + fn requested(&self) -> bool { + *self.receiver.borrow() + } + + async fn wait(&self) { + let mut receiver = self.receiver.clone(); + if *receiver.borrow() { + return; + } + if receiver.changed().await.is_err() { + log::debug!("daemon startup signal watcher was dropped"); + } + } +} + +impl Drop for StartupSignals { + fn drop(&mut self) { + self.task.abort(); + } +} + #[derive(Debug, thiserror::Error)] pub enum DaemonRunError { #[error("{0}")] @@ -89,6 +141,8 @@ impl DaemonRunError { /// The caller owns the process lifetime. This function never forks, detaches, /// creates a session, or redirects terminal output. pub async fn run(config: Config) -> Result<(), DaemonRunError> { + let startup_signals = + StartupSignals::register().map_err(|error| DaemonRunError::Startup(error.to_string()))?; config .validate() .map_err(|error| DaemonRunError::PermanentStartup(error.to_string()))?; @@ -96,6 +150,9 @@ pub async fn run(config: Config) -> Result<(), DaemonRunError> { // so an unauthenticated invocation cannot create persistent daemon state. flicknote_core::session::get_user_id(&config) .map_err(|error| DaemonRunError::PermanentStartup(error.to_string()))?; + if startup_signals.requested() { + return Ok(()); + } let config = Arc::new(config); let _ownership = @@ -105,16 +162,27 @@ pub async fn run(config: Config) -> Result<(), DaemonRunError> { } OwnershipError::Io(_) => DaemonRunError::Startup(error.to_string()), })?; + if startup_signals.requested() { + return Ok(()); + } let db = open_powersync_database(&config) .map_err(|error| DaemonRunError::Startup(error.to_string()))?; - let powersync_tasks = spawn_powersync_actors(&db); + let mut powersync_tasks = spawn_powersync_actors(&db); - startup_checkpoint(config.paths.db_file.clone()).await; + if startup_checkpoint(config.paths.db_file.clone(), &startup_signals).await { + shutdown_startup(&mut powersync_tasks, &db, config.paths.db_file.clone()).await; + return Ok(()); + } // Force PowerSync's local initialization before advertising IPC readiness. - let reader = db - .reader() - .await - .map_err(|error| DaemonRunError::PermanentStartup(error.to_string()))?; + let reader = tokio::select! { + reader = db.reader() => reader + .map_err(|error| DaemonRunError::PermanentStartup(error.to_string()))?, + _signal = startup_signals.wait() => { + log::info!("Shutdown signal received during daemon startup"); + shutdown_startup(&mut powersync_tasks, &db, config.paths.db_file.clone()).await; + return Ok(()); + } + }; drop(reader); let auth = Arc::new(GoTrueClient::new( @@ -129,8 +197,14 @@ pub async fn run(config: Config) -> Result<(), DaemonRunError> { bind_socket(&config).map_err(|error| DaemonRunError::Startup(error.to_string()))?; log::info!("FlickNote daemon initialized (pid {})", std::process::id()); - db.connect(SyncOptions::new(build_connector(&db, &auth, &config))) - .await; + tokio::select! { + _ = db.connect(SyncOptions::new(build_connector(&db, &auth, &config))) => {} + _signal = startup_signals.wait() => { + log::info!("Shutdown signal received during daemon startup"); + shutdown_startup(&mut powersync_tasks, &db, config.paths.db_file.clone()).await; + return Ok(()); + } + } log::info!("FlickNote daemon accepting local requests"); let (shutdown_tx, shutdown_rx) = watch::channel(false); @@ -141,7 +215,7 @@ pub async fn run(config: Config) -> Result<(), DaemonRunError> { powersync: powersync_tasks, }; - let result = wait_for_shutdown(&mut actors).await; + let result = wait_for_shutdown(&mut actors, &startup_signals).await; shutdown_daemon(&mut actors, &db, config.paths.db_file.clone(), &shutdown_tx).await; result.map_err(DaemonRunError::Runtime) } @@ -182,21 +256,31 @@ fn build_connector( } } -async fn startup_checkpoint(db_path: PathBuf) { +async fn startup_checkpoint(db_path: PathBuf, signals: &StartupSignals) -> bool { log::info!("Running startup WAL checkpoint"); - let task = tokio::task::spawn_blocking(move || { - checkpoint_wal_standalone_with_timeout( - &db_path, - "startup", - WalCheckpointMode::Truncate, - 1_000, - ) - }); - if tokio::time::timeout(WAL_CHECKPOINT_TIMEOUT, task) - .await - .is_err() - { - log::warn!("Startup WAL checkpoint exceeded its budget"); + let task = match spawn_wal_checkpoint(db_path, "startup", WalCheckpointMode::Truncate, 1_000) { + Ok(task) => task, + Err(error) => { + log::warn!("Startup WAL checkpoint could not start: {error}"); + return false; + } + }; + tokio::pin!(task); + tokio::select! { + result = &mut task => { + if result.is_err() { + log::warn!("Startup WAL checkpoint worker ended before reporting completion"); + } + false + } + _ = signals.wait() => { + log::info!("Shutdown signal received during startup WAL checkpoint"); + true + } + _ = tokio::time::sleep(WAL_CHECKPOINT_TIMEOUT) => { + log::warn!("Startup WAL checkpoint exceeded its budget"); + false + } } } @@ -235,18 +319,42 @@ fn spawn_checkpoint_worker(db_path: PathBuf) -> JoinHandle<()> { interval.tick().await; loop { interval.tick().await; - let path = db_path.clone(); - if let Err(error) = tokio::task::spawn_blocking(move || { - checkpoint_wal_standalone(&path, "periodic", WalCheckpointMode::Passive) - }) - .await - { - log::error!("Periodic WAL checkpoint task panicked: {error}"); + match spawn_wal_checkpoint( + db_path.clone(), + "periodic", + WalCheckpointMode::Passive, + 5_000, + ) { + Ok(done) => { + if let Err(error) = done.await { + log::error!("Periodic WAL checkpoint task failed: {error}"); + } + } + Err(error) => log::error!("Periodic WAL checkpoint could not start: {error}"), } } }) } +fn spawn_wal_checkpoint( + db_path: PathBuf, + label: &'static str, + mode: WalCheckpointMode, + busy_timeout_ms: u64, +) -> Result, String> { + let (done_tx, done_rx) = tokio::sync::oneshot::channel(); + std::thread::Builder::new() + .name(format!("flicknote-wal-{label}")) + .spawn(move || { + checkpoint_wal_standalone_with_timeout(&db_path, label, mode, busy_timeout_ms); + if done_tx.send(()).is_err() { + log::debug!("WAL checkpoint completion receiver was dropped"); + } + }) + .map_err(|error| format!("could not start WAL checkpoint thread: {error}"))?; + Ok(done_rx) +} + fn spawn_socket_server( listener: UnixListener, app: Arc, @@ -274,25 +382,15 @@ fn spawn_socket_server( }) } -async fn wait_for_shutdown(actors: &mut ActorHandles) -> Result<(), String> { - wait_for_runtime_event(actors, shutdown_signal()).await -} - -async fn shutdown_signal() -> Result<(), String> { - let mut terminate = tokio::signal::unix::signal(tokio::signal::unix::SignalKind::terminate()) - .map_err(|error| format!("Failed to register SIGTERM handler: {error}"))?; - tokio::select! { - result = tokio::signal::ctrl_c() => { - result.map_err(|error| format!("Failed to receive SIGINT: {error}")) - } - signal = terminate.recv() => { - if signal.is_some() { - Ok(()) - } else { - Err("SIGTERM handler closed unexpectedly".to_string()) - } - } - } +async fn wait_for_shutdown( + actors: &mut ActorHandles, + signals: &StartupSignals, +) -> Result<(), String> { + wait_for_runtime_event(actors, async { + signals.wait().await; + Ok(()) + }) + .await } async fn wait_for_runtime_event(actors: &mut ActorHandles, shutdown: F) -> Result<(), String> @@ -359,17 +457,14 @@ impl ShutdownOperations for RuntimeShutdownOperations<'_> { } async fn checkpoint(&self) -> Result<(), String> { - let checkpoint_path = self.db_path.clone(); - tokio::task::spawn_blocking(move || { - checkpoint_wal_standalone_with_timeout( - &checkpoint_path, - "shutdown", - WalCheckpointMode::Truncate, - 1_000, - ); - }) - .await - .map_err(|error| format!("WAL checkpoint task panicked: {error}")) + let task = spawn_wal_checkpoint( + self.db_path.clone(), + "shutdown", + WalCheckpointMode::Truncate, + 1_000, + )?; + task.await + .map_err(|_| "WAL checkpoint worker ended before reporting completion".to_string()) } } @@ -422,6 +517,18 @@ async fn run_storage_shutdown( ] } +async fn shutdown_startup(actors: &mut JoinSet<()>, db: &PowerSyncDatabase, db_path: PathBuf) { + let operations = RuntimeShutdownOperations { db, db_path }; + let _stage_results = run_storage_shutdown( + &operations, + POWERSYNC_DISCONNECT_TIMEOUT, + WAL_CHECKPOINT_TIMEOUT, + ) + .await; + actors.abort_all(); + log::info!("Daemon startup shutdown coordinator finished"); +} + async fn shutdown_daemon( actors: &mut ActorHandles, db: &PowerSyncDatabase, @@ -453,7 +560,6 @@ async fn shutdown_daemon( ) .await; actors.powersync.abort_all(); - while actors.powersync.join_next().await.is_some() {} actors.checkpoint.abort(); log::info!("Daemon shutdown coordinator finished"); } @@ -463,14 +569,6 @@ mod tests { use super::*; use std::sync::Mutex; - #[test] - fn shutdown_budget_is_the_sum_of_bounded_stages() { - assert_eq!( - IPC_DRAIN_TIMEOUT + POWERSYNC_DISCONNECT_TIMEOUT + WAL_CHECKPOINT_TIMEOUT, - Duration::from_secs(8) - ); - } - struct FakeShutdownOperations { events: Mutex>, stall_disconnect: bool, diff --git a/flicknote-sync/src/storage_maintenance.rs b/flicknote-sync/src/storage_maintenance.rs index 053b5ea..6ded3af 100644 --- a/flicknote-sync/src/storage_maintenance.rs +++ b/flicknote-sync/src/storage_maintenance.rs @@ -1,7 +1,7 @@ use std::fmt; use std::path::Path; -/// WAL checkpoint mode passed to [`checkpoint_wal_standalone`]. +/// WAL checkpoint mode passed to [`checkpoint_wal_standalone_with_timeout`]. #[derive(Clone, Copy)] pub(crate) enum WalCheckpointMode { /// Checkpoints frames up to the oldest active reader's mark without waiting. @@ -19,15 +19,9 @@ impl fmt::Display for WalCheckpointMode { } } -/// Run the normal WAL checkpoint with the maintenance timeout used by periodic -/// and startup checkpoints. -pub(crate) fn checkpoint_wal_standalone(db_path: &Path, label: &str, mode: WalCheckpointMode) { - checkpoint_wal_standalone_with_timeout(db_path, label, mode, 5_000); -} - /// Run a WAL checkpoint with an explicit SQLite busy timeout. /// -/// The synchronous operation is always called from a blocking task by async +/// The synchronous operation is called from a detached worker thread by async /// callers. The shutdown coordinator uses a short timeout so a busy database /// cannot hold process exit open. pub(crate) fn checkpoint_wal_standalone_with_timeout( From 50e2d219390950eaff54b7f94ca6e6507795e95d Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 13 Aug 2026 17:38:41 +0800 Subject: [PATCH 3/5] chore(cli): ignore scratch artifacts --- .gitignore | 1 + .../issues/01-foreground-daemon-ownership.md | 19 -- .../issues/02-cross-platform-user-service.md | 23 --- .../issues/03-daemon-status-and-logs.md | 18 -- .../04-authentication-owned-lifecycle.md | 18 -- .../issues/05-daemon-recovery-guidance.md | 15 -- .../issues/06-contract-legacy-lifecycle.md | 19 -- .scratch/daemon-service-management/spec.md | 180 ------------------ 8 files changed, 1 insertion(+), 292 deletions(-) delete mode 100644 .scratch/daemon-service-management/issues/01-foreground-daemon-ownership.md delete mode 100644 .scratch/daemon-service-management/issues/02-cross-platform-user-service.md delete mode 100644 .scratch/daemon-service-management/issues/03-daemon-status-and-logs.md delete mode 100644 .scratch/daemon-service-management/issues/04-authentication-owned-lifecycle.md delete mode 100644 .scratch/daemon-service-management/issues/05-daemon-recovery-guidance.md delete mode 100644 .scratch/daemon-service-management/issues/06-contract-legacy-lifecycle.md delete mode 100644 .scratch/daemon-service-management/spec.md diff --git a/.gitignore b/.gitignore index 714b5af..5eff447 100644 --- a/.gitignore +++ b/.gitignore @@ -23,6 +23,7 @@ target/ # option (not recommended) you can uncomment the following to ignore the entire idea folder. #.idea/ .worktrees +.scratch/ .claude/settings.local.json # Go binaries diff --git a/.scratch/daemon-service-management/issues/01-foreground-daemon-ownership.md b/.scratch/daemon-service-management/issues/01-foreground-daemon-ownership.md deleted file mode 100644 index 8ea6bbc..0000000 --- a/.scratch/daemon-service-management/issues/01-foreground-daemon-ownership.md +++ /dev/null @@ -1,19 +0,0 @@ -# 01 — Foreground daemon ownership and graceful shutdown - -**What to build:** Make `flicknote daemon run` the reliable foreground daemon entry point, with exclusive ownership of its configured data directory and one bounded shutdown path for terminal and service signals. - -**Blocked by:** None — can start immediately. - -Status: ready-for-agent - -- [ ] `flicknote daemon run` requires a valid login and runs synchronously without forking, detaching, creating a new session, or redirecting terminal output. -- [ ] The daemon obtains a non-blocking advisory exclusive lock for its configured data directory before touching its socket or PowerSync SQLite database, and holds the lock for its full lifetime. -- [ ] A second daemon for the same data directory fails promptly with actionable ownership diagnostics and does not unlink the active daemon's socket or open its database. -- [ ] Daemons using different isolated data directories can run concurrently. -- [ ] Lock ownership is released automatically after graceful exit, panic, or forced process termination without requiring lock-file deletion. -- [ ] Socket cleanup and binding occur only after lock acquisition, so stale endpoints can be cleaned without racing a live owner. -- [ ] SIGINT and SIGTERM enter the same graceful-shutdown coordinator instead of relying on default process termination. -- [ ] Shutdown stops new IPC work, bounds in-flight IPC, bounds PowerSync disconnect, attempts a bounded WAL truncate checkpoint, releases resources, and exits within an approximately eight-second total budget. -- [ ] A stalled disconnect or checkpoint is logged by stage but cannot prevent process exit. -- [ ] Explicit shutdown exits successfully; unexpected actor termination or panic remains distinguishable as process failure. -- [ ] Isolated real-process tests prove single ownership, stale socket handling, SIGINT/SIGTERM cleanup, bounded shutdown, forced-release recovery, and subsequent restart without touching user data or services. diff --git a/.scratch/daemon-service-management/issues/02-cross-platform-user-service.md b/.scratch/daemon-service-management/issues/02-cross-platform-user-service.md deleted file mode 100644 index 3b62da5..0000000 --- a/.scratch/daemon-service-management/issues/02-cross-platform-user-service.md +++ /dev/null @@ -1,23 +0,0 @@ -# 02 — Cross-platform user service lifecycle - -**What to build:** Let users install and control the foreground daemon as a user-level launchd service on macOS or systemd service on Linux through one `flicknote daemon` interface. - -**Blocked by:** 01 — Foreground daemon ownership and graceful shutdown. - -Status: ready-for-agent - -- [ ] A maintained `service-manager` adapter replaces direct platform command construction for user-level launchd and systemd lifecycle operations. -- [ ] `daemon install` installs a missing service, starts it, and reports success only after compatible IPC readiness is observed. -- [ ] Re-running `daemon install` reconciles changed service configuration or executable location, ensures the service is started, and succeeds without creating duplicate services. -- [ ] Installed services execute the same `flicknote daemon run` entry point used for foreground diagnosis. -- [ ] Installation preserves a stable package-manager symlink entry point when invoked through one rather than canonicalizing it into a versioned package directory. -- [ ] The selected executable is validated as existing, executable, and FlickNote-owned before service installation. -- [ ] `daemon start` starts only an installed service and waits for readiness; it does not install a missing service. -- [ ] `daemon stop` stops without uninstalling autostart configuration and waits until application readiness is gone. -- [ ] `daemon restart` restarts only an installed service and waits for readiness. -- [ ] `daemon uninstall` stops and removes the installed service. -- [ ] Readiness requires a compatible IPC health response after local application initialization but does not require remote PowerSync connectivity or a completed sync cycle. -- [ ] Protocol incompatibility reports CLI and daemon version/protocol details when available; exact package-version equality is not required. -- [ ] Services autostart and restart unexpected failures with reasonable platform-supported delay, while explicit shutdown and permanent startup errors do not create a tight restart loop. -- [ ] Adapter-level tests cover user-service selection, lifecycle state transitions, reconciliation, stable executable selection, readiness, and error translation without testing dependency internals. -- [ ] Bounded isolated launchd/systemd user-service system tests run where CI or the host supports them, use unique labels and data roots, and always clean up. diff --git a/.scratch/daemon-service-management/issues/03-daemon-status-and-logs.md b/.scratch/daemon-service-management/issues/03-daemon-status-and-logs.md deleted file mode 100644 index 6a344f3..0000000 --- a/.scratch/daemon-service-management/issues/03-daemon-status-and-logs.md +++ /dev/null @@ -1,18 +0,0 @@ -# 03 — Daemon status and logs experience - -**What to build:** Give users concise routine status, detailed and machine-readable diagnosis, and one logs interface that hides launchd/systemd differences. - -**Blocked by:** 02 — Cross-platform user service lifecycle. - -Status: ready-for-agent - -- [ ] Default `daemon status` output is one concise line for a ready daemon and includes an actionable recovery command automatically when unhealthy. -- [ ] `daemon status --verbose` separately reports service installation/running state, application readiness, FlickNote version, IPC protocol, PowerSync connection state, last observed error, and log guidance. -- [ ] `daemon status --json` always emits an object-root result with stable required fields and explicit enums for service, application, and sync states. -- [ ] JSON status represents unavailable observations predictably rather than silently changing the result shape. -- [ ] Status distinguishes at least not installed, installed/stopped, service running/application unavailable, ready/offline, ready/connected, protocol incompatible, and service-manager query failure. -- [ ] Unhealthy status still prints the requested human or JSON diagnosis and then exits nonzero; ready status exits successfully. -- [ ] `daemon logs` shows bounded recent managed-daemon logs on macOS and Linux without requiring users to know launchd or journal commands. -- [ ] `daemon logs --lines` controls the bounded history and `daemon logs --follow` streams new output. -- [ ] Managed macOS logs use the FlickNote data-directory log destination; managed Linux logs use the systemd user journal; foreground run continues writing to its terminal. -- [ ] Contract tests cover status JSON schema and enums, while behavioral tests cover human output, exit outcomes, service/application disagreement, and both logging backends. diff --git a/.scratch/daemon-service-management/issues/04-authentication-owned-lifecycle.md b/.scratch/daemon-service-management/issues/04-authentication-owned-lifecycle.md deleted file mode 100644 index d778d0d..0000000 --- a/.scratch/daemon-service-management/issues/04-authentication-owned-lifecycle.md +++ /dev/null @@ -1,18 +0,0 @@ -# 04 — Authentication-owned daemon lifecycle - -**What to build:** Make login establish a ready authenticated daemon and make logout remove it before credentials and local data are cleared, with explicit behavior for partial failures. - -**Blocked by:** 02 — Cross-platform user service lifecycle. - -Status: ready-for-agent - -- [ ] Successful login authenticates, reconciles the user service, starts it, waits for readiness, and prints concise authentication and daemon-ready confirmations. -- [ ] If authentication succeeds but service installation or readiness fails, the valid session is retained, the command identifies the failed daemon stage, recommends `daemon status --verbose`, and exits nonzero. -- [ ] `login --force` stops and uninstalls the current service before removing the prior session and beginning new authentication. -- [ ] Successful forced login reconciles and verifies a service using the new session. -- [ ] Failed forced authentication does not restore the old session or old service and leaves a clear logged-out state. -- [ ] Normal logout stops and uninstalls the service and confirms it is stopped before deleting the session and local database files. -- [ ] Normal logout preserves the session and local data if service stop or uninstall cannot be confirmed. -- [ ] `logout --force` explicitly permits session and local-data cleanup after service cleanup failure and reports the unresolved service state. -- [ ] Daemon installation and foreground execution reject missing authentication rather than creating an unauthenticated persistent process. -- [ ] Lifecycle orchestration tests cover login success, authentication failure, post-auth install/readiness failure, forced-login ordering and outcomes, logout success, cleanup failures, and forced logout. diff --git a/.scratch/daemon-service-management/issues/05-daemon-recovery-guidance.md b/.scratch/daemon-service-management/issues/05-daemon-recovery-guidance.md deleted file mode 100644 index de7ad7e..0000000 --- a/.scratch/daemon-service-management/issues/05-daemon-recovery-guidance.md +++ /dev/null @@ -1,15 +0,0 @@ -# 05 — Daemon recovery guidance for CLI and MCP clients - -**What to build:** Make every daemon-dependent CLI and MCP entry point fail consistently and safely when the application is unavailable, while preserving local-first operation when only remote sync is offline. - -**Blocked by:** 03 — Daemon status and logs experience. - -Status: ready-for-agent - -- [ ] Daemon-dependent CLI commands report that the daemon is unavailable and recommend `flicknote daemon status` and `flicknote daemon start` as appropriate. -- [ ] MCP startup and daemon-dependent MCP operations expose consistent actionable unavailability diagnostics without leaking platform-specific service details. -- [ ] Data commands and MCP never install, start, restart, or otherwise mutate OS service state implicitly. -- [ ] Data commands and MCP never fall back to opening the PowerSync SQLite database directly. -- [ ] A ready local application remains usable and reports ready when PowerSync is disconnected or the network is unavailable. -- [ ] Transient remote connectivity failures stay inside PowerSync's reconnect/backoff behavior and do not churn the OS service process. -- [ ] Behavioral tests prove recovery guidance, absence of implicit service mutations, absence of direct database fallback, and local readiness during remote outage. diff --git a/.scratch/daemon-service-management/issues/06-contract-legacy-lifecycle.md b/.scratch/daemon-service-management/issues/06-contract-legacy-lifecycle.md deleted file mode 100644 index c2e4631..0000000 --- a/.scratch/daemon-service-management/issues/06-contract-legacy-lifecycle.md +++ /dev/null @@ -1,19 +0,0 @@ -# 06 — Contract legacy lifecycle and unify distribution - -**What to build:** Finish the daemon-management replacement by removing every legacy lifecycle path, shipping one executable and one vocabulary, documenting the clean pre-upgrade boundary, and verifying the complete behavior in one pull request. - -**Blocked by:** 01 — Foreground daemon ownership and graceful shutdown; 02 — Cross-platform user service lifecycle; 03 — Daemon status and logs experience; 04 — Authentication-owned daemon lifecycle; 05 — Daemon recovery guidance for CLI and MCP clients. - -Status: ready-for-agent - -- [ ] The `sync` command namespace is removed without a compatibility alias, and parser tests accept every agreed `daemon` command and option while rejecting the removed namespace. -- [ ] Custom PID-file lifecycle decisions, PID signaling, process-name scanning, SIGKILL stopgaps, detached background startup, direct launchctl handling, and hand-generated service files are removed. -- [ ] Lifecycle artifacts consistently use daemon terminology for service label, lock, and socket; old sync-named artifacts are not retained as runtime compatibility paths. -- [ ] The separately distributed daemon executable and sibling-binary discovery are removed; release metadata ships only the unified `flicknote` executable. -- [ ] No CLI or MCP code path opens SQLite directly, and the daemon remains the sole local backend and PowerSync database owner. -- [ ] User documentation covers login/logout symmetry, every `daemon` command, foreground diagnosis, status/logs usage, macOS and Linux user services, and recovery guidance. -- [ ] Upgrade documentation instructs installations using the old lifecycle to run the old version's uninstall command before installing this release; no automatic migration is added. -- [ ] Agent-facing workflow/reference documentation uses the new daemon vocabulary and commands where future implementation or verification depends on them. -- [ ] The final diff contains no temporary debug instrumentation or obsolete lifecycle tests, and replacement tests assert runtime/public contracts rather than deleted source shape. -- [ ] Workspace formatting, tests, checks, Clippy with warnings denied, strict MCP schema contracts, and the smallest available macOS/Linux service behavioral probes all pass. -- [ ] The staged final diff is reviewed as one coherent feature intended for a single pull request. diff --git a/.scratch/daemon-service-management/spec.md b/.scratch/daemon-service-management/spec.md deleted file mode 100644 index aaf4bb4..0000000 --- a/.scratch/daemon-service-management/spec.md +++ /dev/null @@ -1,180 +0,0 @@ -Status: ready-for-agent - -## Problem Statement - -FlickNote currently manages its sync daemon with hand-written PID-file logic, direct Unix signals, manually generated launchd property lists, direct `launchctl` commands, detached child processes, socket-file cleanup, and health polling. These mechanisms do not share a reliable source of truth. - -A daemon can remain alive after its PID file is removed, and a second daemon can delete the first daemon's socket and open the same PowerSync SQLite database. This has produced simultaneous database owners and repeated SQLite `BUSY` failures. The daemon also listens only for terminal interrupt (`SIGINT`), while launchd and normal service-stop operations use termination (`SIGTERM`). Consequently, service stops bypass the daemon's PowerSync disconnect and WAL checkpoint path. - -The current CLI exposes these operations under `flicknote sync`, even though they manage a long-running daemon rather than request an immediate synchronization. The implementation supports launchd only and would require another custom lifecycle implementation for systemd. Users should not need to understand platform-specific service commands or repair PID/socket inconsistencies. - -## Solution - -Replace FlickNote's custom process and launchd lifecycle management with the maintained `service-manager` abstraction, using user-level launchd services on macOS and user-level systemd services on Linux. The operating-system service manager is the source of truth for installation and process lifecycle; FlickNote's IPC health endpoint remains the source of truth for application readiness. - -Replace the `flicknote sync` command family with `flicknote daemon`. Login automatically reconciles, starts, and verifies the user service. Logout stops and uninstalls the service before removing the session and local data. A foreground-only `flicknote daemon run` entry point supports development and diagnosis without detaching or creating a second lifecycle model. - -Protect each FlickNote data directory with a kernel-managed advisory exclusive file lock held for the daemon's full lifetime. The lock, rather than a PID file or socket existence, enforces single ownership of the PowerSync SQLite database. Socket cleanup is permitted only after the process acquires that lock. - -Handle `SIGINT` and `SIGTERM` as inputs to one bounded graceful-shutdown sequence. Stop accepting IPC work, bound in-flight work, disconnect PowerSync, attempt a bounded WAL checkpoint, release resources, and exit. Maintenance failures are recorded but do not leave the service stuck indefinitely. - -Provide concise normal output, detailed and JSON status modes, and a cross-platform logs command so users do not need to know launchd or systemd details. - -## User Stories - -1. As a FlickNote user, I want login to start everything required for note commands, so that I do not need a separate daemon setup step. -2. As a FlickNote user, I want logout to stop and remove the daemon service, so that no authenticated background process remains after logout. -3. As a FlickNote user, I want daemon installation to work on macOS, so that FlickNote starts automatically through my launchd user session. -4. As a FlickNote user, I want daemon installation to work on Linux, so that FlickNote starts automatically through my systemd user session. -5. As a FlickNote user, I want the same daemon commands on macOS and Linux, so that I do not need platform-specific service knowledge. -6. As a FlickNote user, I want daemon management commands to be named `daemon`, so that their purpose is clear and is not confused with an immediate sync operation. -7. As a FlickNote user, I want `daemon install` to install, start, and verify the service, so that success means FlickNote is actually usable. -8. As a FlickNote user, I want repeated `daemon install` calls to reconcile the installed service, so that I can repair configuration and executable-path changes safely. -9. As a FlickNote user, I want `daemon start` to start an installed service without silently installing one, so that command side effects remain predictable. -10. As a FlickNote user, I want `daemon stop` to stop the service without uninstalling autostart configuration, so that I can pause FlickNote temporarily. -11. As a FlickNote user, I want `daemon restart` to restart an installed service and wait for readiness, so that I have a reliable recovery command. -12. As a FlickNote user, I want `daemon uninstall` to stop and remove the service, so that it will not return on my next login. -13. As a developer, I want `daemon run` to run synchronously in the foreground, so that I can observe logs and stop it with my terminal. -14. As a developer, I want foreground daemon execution to avoid forking, detaching, or creating a session, so that its lifetime remains attached to my shell. -15. As a developer, I want both Ctrl-C and normal service termination to use the same shutdown path, so that foreground and managed execution behave consistently. -16. As a FlickNote user, I want only one daemon to own a data directory, so that concurrent processes cannot corrupt or starve the SQLite workload. -17. As a FlickNote user, I want a crashed or forcibly killed daemon to release single-instance ownership automatically, so that stale metadata does not block recovery. -18. As a developer, I want separate XDG data directories to permit separate daemon instances, so that isolated development and testing environments remain possible. -19. As a FlickNote user, I want a clear error when another daemon owns my data directory, so that I know why startup was refused. -20. As a FlickNote user, I want lock-conflict errors to show safe diagnostic metadata when available, so that I can identify the owner without FlickNote automatically killing it. -21. As a FlickNote user, I want `daemon run` to tell me how to stop an installed daemon when ownership conflicts, so that I can switch to foreground diagnosis safely. -22. As a FlickNote user, I want daemon startup success to require a valid IPC health response, so that a merely running but unusable process is not reported as ready. -23. As a FlickNote user, I want network disconnection not to prevent daemon readiness, so that local-first operations remain available offline. -24. As a FlickNote user, I want protocol incompatibility between the CLI and daemon to produce a clear error, so that I do not unknowingly use an incompatible process. -25. As a FlickNote user, I want compatible CLI and daemon package versions to interoperate even when their version strings differ, so that protocol compatibility—not incidental version equality—governs operation. -26. As a FlickNote user, I want `daemon status` to provide a concise healthy summary, so that routine checks are easy to read. -27. As a FlickNote user, I want `daemon status --verbose` to distinguish service state, application readiness, version, protocol, and sync state, so that failures can be diagnosed. -28. As an automation author, I want `daemon status --json` to return a stable object, so that scripts can inspect daemon state without parsing human text. -29. As an automation author, I want status to return a nonzero exit status when the application is not ready, so that health checks can fail reliably. -30. As a FlickNote user, I want `daemon logs` to show recent daemon logs on both macOS and Linux, so that I do not need to learn launchd and journal commands. -31. As a FlickNote user, I want `daemon logs --follow` to stream logs, so that I can observe startup and synchronization failures in real time. -32. As a FlickNote user, I want to choose the number of recent log lines, so that diagnostics remain bounded. -33. As a FlickNote user, I want a failed data command to recommend `daemon status` and `daemon start`, so that recovery is obvious. -34. As a FlickNote user, I want data commands never to install or start services implicitly, so that read and mutation commands do not change OS service state. -35. As a FlickNote user, I want data commands and the MCP server never to fall back to opening SQLite directly, so that the daemon remains the sole database owner. -36. As a FlickNote user, I want successful login to report authentication and daemon readiness concisely, so that I know FlickNote is usable. -37. As a FlickNote user, I want authentication to remain valid if daemon installation fails, so that I can retry service setup without logging in again. -38. As a FlickNote user, I want login to return a failure when authentication succeeds but daemon readiness fails, so that partial setup is not presented as complete success. -39. As a FlickNote user, I want `login --force` to stop the old service before replacing my session, so that the daemon never continues with superseded credentials. -40. As a FlickNote user, I want successful forced login to reinstall and verify the service, so that reauthentication restores the complete system. -41. As a FlickNote user, I want a failed forced authentication to leave me clearly logged out, so that an old session is not silently resurrected. -42. As a FlickNote user, I want normal logout to preserve my session if service shutdown or uninstall fails, so that a running daemon is not left with credentials removed underneath it. -43. As a FlickNote user, I want `logout --force` to let me clear the session and local data despite service cleanup failure, so that I retain an explicit emergency escape hatch. -44. As a FlickNote user, I want daemon installation and foreground execution to require a valid login, so that an unauthenticated background process is not created. -45. As a FlickNote user, I want transient network errors to be retried inside the running daemon, so that the OS service manager does not churn the process during offline periods. -46. As a FlickNote user, I want daemon crashes and unexpected actor exits to trigger OS-managed restart, so that the service recovers from transient internal failures. -47. As a FlickNote user, I want permanent configuration and authentication errors not to cause an infinite restart loop, so that logs and system resources are not flooded. -48. As a FlickNote user, I want service restart attempts to use a reasonable delay, so that repeated crashes do not create a tight loop. -49. As a FlickNote user, I want graceful shutdown to have a hard upper bound, so that stop, logout, restart, and upgrade operations cannot hang indefinitely. -50. As a FlickNote user, I want PowerSync disconnect to be attempted during shutdown, so that synchronization actors can stop cleanly. -51. As a FlickNote user, I want a shutdown WAL checkpoint to be attempted but not required for exit, so that normal SQLite WAL durability does not become a shutdown deadlock. -52. As a FlickNote user, I want shutdown logs to identify the stage that timed out or failed, so that PowerSync and SQLite issues can be distinguished. -53. As a Homebrew user, I want the service to reference a stable executable entry point, so that package upgrades do not leave launchd pointing into a removed Cellar version. -54. As a FlickNote user, I want the managed service and foreground command to execute the same daemon entry point, so that version and behavior cannot drift between binaries. -55. As a FlickNote maintainer, I want one distributed executable rather than separate CLI and daemon executables, so that packaging and upgrades have a single source of truth. -56. As a FlickNote maintainer, I want OS service state and application readiness to remain separate concepts, so that diagnostics accurately describe partial failures. -57. As a FlickNote maintainer, I want PID information to be diagnostic only, so that PID reuse and stale files cannot control correctness or automatic signaling. -58. As a FlickNote maintainer, I want socket cleanup to occur only while holding the data-directory lock, so that one daemon cannot unlink another live daemon's endpoint. -59. As a FlickNote maintainer, I want the service-manager adapter to own platform differences, so that FlickNote does not hand-generate launchd or systemd configuration. -60. As a FlickNote maintainer, I want observable lifecycle behavior covered at a high integration seam, so that replacing internal libraries does not invalidate the tests. - -## Implementation Decisions - -- Replace direct launchd commands and hand-written service files with the maintained `service-manager` crate. Configure user-level launchd on macOS and user-level systemd on Linux. Windows, OpenRC, rc.d, and system-level services are not promised by this feature. -- The OS service manager is the source of truth for whether a service is installed, started, stopped, or uninstalled. FlickNote must not use a PID file to infer or control managed service state. -- IPC health is the source of truth for application readiness. A service can be running while the application is unavailable; status and error messages must preserve that distinction. -- Rename the public command namespace from `sync` to `daemon`. Do not retain a compatibility alias. -- The public command family is `install`, `uninstall`, `start`, `stop`, `restart`, `status`, `logs`, and `run`. -- `install` is an idempotent reconciliation operation. It installs a missing service, updates changed service configuration or executable paths, ensures the service is started, and waits for IPC readiness. Re-running it with an equivalent configuration is successful. -- `start`, `stop`, `restart`, and `uninstall` operate only through the service manager. `start` and `restart` do not silently install a missing service. -- `run` executes the daemon synchronously in the foreground. It does not fork, detach, invoke `setsid`, redirect terminal output, or create an alternate background mode. -- Login and daemon lifecycle are symmetric. A successful login reconciles and starts the user service and waits for readiness. Logout stops and uninstalls the service before deleting the session and local database files. -- If authentication succeeds but service installation or readiness fails, retain the valid session, print that authentication succeeded and daemon startup failed, provide the status recovery command, and return nonzero. -- Forced login stops and uninstalls the current service before removing the prior session. It then authenticates, reconciles the service, and waits for readiness. Failed authentication does not restore the old session or service. -- Normal logout aborts before deleting session or local data when service stop/uninstall cannot be confirmed. Add an explicit force option that allows cleanup to continue despite that failure and clearly reports the unresolved service state. -- Daemon installation and foreground execution require a valid session. There is no supported unauthenticated daemon state. -- Data commands and MCP operations continue to require daemon health. They never open SQLite directly and never implicitly install or start the service. -- Use a kernel-managed, non-blocking advisory exclusive file lock to protect each configured data directory. Select a maintained Rust lock crate after checking current documentation and types rather than implementing raw `flock`/`fcntl` handling. -- Store the lock file in the configured FlickNote data directory under daemon terminology. Hold the open lock guard from before any socket or database ownership is acquired until daemon teardown completes. Process exit, panic, or forced termination must release the kernel lock automatically. -- Lock-file contents may contain PID, version, and start time for diagnostics. That content is not authoritative, may be stale, and must never be used to choose or signal a process. -- After obtaining the data-directory lock, the daemon may remove a stale daemon socket and bind the shared endpoint. It must never unlink the endpoint before lock ownership is established. -- Standardize lifecycle terminology on daemon. Use a daemon service label, daemon lock name, and daemon socket name. Do not preserve sync-named lifecycle artifacts for compatibility. -- Use one public `flicknote` executable. The managed service runs the same `daemon run` entry point used for foreground execution. Remove the separately distributed daemon executable and its sibling-path discovery logic. -- Preserve a stable symlink path when installation is invoked through one, rather than canonicalizing it into a versioned package-manager location. Validate that the selected executable exists, is executable, and identifies as FlickNote before installing the service. -- The release and package configuration distributes only the unified executable. -- Handle terminal interrupt and Unix termination as equivalent shutdown triggers feeding one shutdown coordinator. Platform-specific signal registration remains internal to the daemon runtime. -- Graceful shutdown uses an approximately eight-second total budget and logs stage boundaries and durations. Stop accepting new IPC immediately, allow up to approximately two seconds for in-flight IPC, allow up to approximately four seconds for PowerSync disconnect, and allow up to approximately two seconds for a shutdown WAL truncate checkpoint. Remaining cleanup releases the socket and lock as guards drop. -- A PowerSync disconnect timeout or checkpoint timeout/failure is logged and does not prevent process exit. SQLite WAL already provides durability; successful truncation is maintenance, not a correctness precondition. -- Unexpected internal actor exit, panic, and transient internal crashes produce failure semantics suitable for OS-managed restart. Explicit shutdown exits successfully. -- Permanent startup errors such as missing authentication, invalid configuration, or incompatible schema are classified separately so they do not create an unbounded restart loop. Network unavailability is not a startup failure; PowerSync retains its internal reconnect/backoff behavior while the daemon remains ready for local work. -- Configure autostart and restart-on-failure with a reasonable platform-supported delay. Explicit service stop must not cause immediate restart. -- A successful readiness check requires a valid IPC server-info response after configuration validation, database opening, backend creation, and socket serving. It does not require remote PowerSync connection or completion of a sync cycle. -- CLI/daemon compatibility is governed by the IPC protocol contract, not exact package-version equality. Status includes both values. Incompatible protocol responses fail readiness and report the executable path, CLI version/protocol, and daemon version/protocol when available. -- Default status output is one concise line. Unhealthy states include an actionable recovery command automatically. -- Verbose status distinguishes service installation/running state, application readiness, package version, IPC protocol, PowerSync connection state, last observed error, and the platform-appropriate log location or command. -- JSON status has an object root with stable fields for service state, application state, version, protocol, sync state, error details, and log guidance. Use explicit string enums for state fields and nullable object/string fields for unavailable observations; do not encode unavailable states by omitting unrelated required fields unpredictably. -- Status emits its human or JSON report even when unhealthy, then returns nonzero unless the application is ready. -- Logs provides bounded recent output by default, accepts a line-count option, and supports follow mode. On macOS it reads/follows launchd-directed stdout/stderr in the FlickNote data directory. On Linux it queries/follows the systemd user journal. Platform adaptation is internal to the command. -- Foreground run logs to the attached terminal. It does not redirect output to the managed-service log destination. -- Login success prints `Authenticated` followed by a concise daemon-ready confirmation. Partial success states identify the failed stage without exposing launchd/systemd implementation details in the normal path. -- When foreground run cannot acquire ownership because the managed daemon is active, the error tells the user to stop the daemon and retry. It does not offer or perform an automatic stop/kill option. -- Service manager errors retain enough platform context for verbose diagnostics but normal errors use FlickNote concepts and actionable `flicknote daemon` commands. -- There is no runtime migration or compatibility layer for old PID files, sync commands, sync socket names, old service labels, or manually detached daemons. Before installing the release containing this feature, development/release documentation instructs existing installations to run the old version's uninstall command. -- The temporary PID/SIGKILL stopgap on the current development branch is superseded by this design; the final implementation must remove custom PID signaling rather than layering service-manager behavior on top of it. - -## Testing Decisions - -- Tests assert observable lifecycle contracts rather than source shape, command strings, selected crate internals, generated plist/unit text, or private helper structure. -- Prefer two high seams because they cover distinct trust boundaries without duplicating low-level tests. -- The primary seam is the CLI lifecycle orchestration boundary with an injected/fake service-manager adapter and fake health endpoint. Exercise daemon install/start/stop/restart/uninstall/status behavior and login/logout composition through the same command orchestration used by the CLI. Assert calls only where they are externally meaningful state transitions; assert user-visible output, exit outcome, retained/deleted session state, and readiness behavior. -- The second seam is an isolated daemon process integration test using temporary XDG config/data roots and the real daemon entry point. It verifies exclusive ownership, stale socket handling under lock, SIGINT shutdown, SIGTERM shutdown, bounded exit, resource release, and subsequent restart. It must never touch the user's real service, session, socket, database, or logs. -- Extend the existing CLI parser test style to mechanically confirm every `daemon` subcommand and option parses and the removed `sync` namespace does not parse. This is a public CLI contract rather than a source-text test. -- Add machine-consumed contract tests for status JSON: object root, stable required fields, allowed state enums, healthy and unhealthy examples, and protocol/version representation. -- Test status behavior across at least: not installed, installed/stopped, service running/application unavailable, ready/offline, ready/connected, protocol incompatible, and service-manager query failure. -- Test that default status is concise, verbose status distinguishes service and application state, unhealthy status still emits diagnostics, and unhealthy status exits nonzero. -- Test install reconciliation across missing, equivalent, and changed service configurations. The changed configuration case must prove the current stable executable entry point is applied and the service becomes ready. -- Test that start/restart fail clearly when the service is not installed and do not install it implicitly. -- Test that data commands and MCP startup report daemon recovery guidance without attempting service installation, service start, or direct database access. -- Test login orchestration for full success, authentication failure, authentication success plus install failure, and authentication success plus readiness failure. Verify valid sessions are retained in partial-success cases. -- Test forced login ordering and outcomes: old service cleanup precedes old-session removal; failed new authentication leaves no restored old session; successful authentication reconciles and verifies the new service. -- Test logout success, stop failure, uninstall failure, and forced cleanup. Normal failure preserves session/local data; forced cleanup removes them while reporting unresolved service cleanup. -- Test foreground run rejects missing authentication before acquiring database ownership. -- Test two foreground daemon processes against one temporary data directory. The second must fail promptly without deleting the first process's socket or opening its SQLite database. After the first exits or is forcibly killed, another process must acquire ownership successfully without manual lock-file deletion. -- Test that different temporary data directories can run concurrently. -- Test lock diagnostic metadata only for user-visible diagnostics; do not test its exact serialized layout unless that layout is explicitly exposed as a machine-consumed contract. -- Test SIGINT and SIGTERM against the real isolated process. Both must enter the same shutdown sequence, release the socket and lock, and exit within the configured total budget. Capture stage logs to distinguish entering shutdown from default signal termination. -- Test a controllably stalled PowerSync disconnect and checkpoint at the runtime orchestration seam. Each timeout must allow later cleanup and process exit. Do not require real network timing or real SQLite lock contention to make these deterministic. -- Test that remote network failure does not make local application readiness fail and does not terminate the daemon. -- Test service restart classification through adapter-visible outcomes: explicit shutdown is successful; unexpected actor failure is unsuccessful; permanent startup errors do not request an endless retry path. -- Add platform adapter tests only for behavior not already guaranteed by `service-manager`, such as choosing user-service level, stable executable input, logging destination/guidance, and translating status into FlickNote's status model. Do not duplicate the dependency's launchd/systemd command-generation tests. -- Add bounded macOS and Linux system tests where CI runners permit them: install a uniquely labeled temporary user service, start it, observe readiness, stop it, and uninstall it. These tests must clean up through guards even on failure and must not use the production label or production data directory. -- Follow existing project conventions: Rust unit/integration tests, temporary directories, explicit XDG isolation, targeted package tests during iteration, then workspace format, test, check, and Clippy with warnings denied. -- The current parser tests are prior art for public CLI syntax. Existing daemon health tests are prior art for IPC readiness. Existing PowerSync actor tests are prior art for real connect/disconnect behavior. Existing temporary-directory configuration tests are prior art for XDG isolation. - -## Out of Scope - -- Supporting Windows Services, WinSW, OpenRC, rc.d, or system-level/root services. -- Preserving the `flicknote sync` command as an alias. -- Migrating or automatically cleaning old PID files, old sync sockets, old service labels, old plist files, or detached legacy processes. -- Automatically scanning for or killing processes by name or diagnostic PID metadata. -- Maintaining a standalone background-detach mode outside launchd/systemd. -- Allowing multiple daemons to share one data directory or SQLite database. -- Replacing the Unix-domain IPC protocol or changing note/MCP business operations. -- Requiring remote PowerSync connectivity before the daemon is ready. -- Guaranteeing that a shutdown WAL truncate checkpoint succeeds. -- Making exact package-version equality an IPC compatibility requirement. -- Adding direct SQLite fallback paths to CLI or MCP clients. -- Deploying, releasing, or migrating existing installations as part of implementation; release documentation only records the required pre-upgrade uninstall step. - -## Further Notes - -- Production evidence showed two daemon processes simultaneously holding the same SQLite database and separate Unix socket objects associated with the same socket path. Repeated SQLite `BUSY` errors occurred during this period. This supports treating single database ownership as a correctness requirement. -- Investigation disproved the initial assumption that PowerSync shutdown was known to hang. The existing daemon listens only through Tokio's Ctrl-C API, which maps to `SIGINT` on Unix, while the controller sends `SIGTERM`. An isolated probe showed `SIGTERM` exited without entering FlickNote shutdown, whereas `SIGINT` entered shutdown and completed PowerSync disconnect quickly. The implementation should still retain bounded shutdown stages because external dependencies must not be allowed to hang service operations. -- The repository currently has no domain glossary, context document, or architecture decision record for this area. This specification uses the established project terms daemon, user service, application readiness, IPC health, PowerSync database, and local backend. -- The repository currently distributes both `flicknote` and `flicknote-sync`; this specification intentionally collapses them into one executable and requires release metadata and documentation to follow that decision. -- The implementation agent should verify the current `service-manager` and advisory-lock crate documentation and types before adding dependencies. The architectural contract is fixed; exact dependency API usage is not. From 593d8a51c2214acb771df8c78769333866b30aad Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 13 Aug 2026 18:04:26 +0800 Subject: [PATCH 4/5] fix(cli): silence platform-specific logging parameter --- flicknote-cli/src/commands/daemon.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flicknote-cli/src/commands/daemon.rs b/flicknote-cli/src/commands/daemon.rs index ef6b033..8f521e0 100644 --- a/flicknote-cli/src/commands/daemon.rs +++ b/flicknote-cli/src/commands/daemon.rs @@ -174,10 +174,10 @@ fn ensure_authenticated(config: &Config) -> Result<(), CliError> { flicknote_core::session::get_user_id(config).map(|_| ()) } -fn initialize_daemon_logging(config: &Config) -> Result<(), CliError> { +fn initialize_daemon_logging(_config: &Config) -> Result<(), CliError> { #[cfg(target_os = "macos")] if std::env::var_os("FLICKNOTE_DAEMON_MANAGED").is_some() { - redirect_managed_daemon_output(config)?; + redirect_managed_daemon_output(_config)?; } let mut builder = env_logger::Builder::from_env( env_logger::Env::default().default_filter_or("flicknote_sync=info,powersync=debug"), From 9bd555bee08dd3e9050efe9763d950d8a8c9c970 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 13 Aug 2026 18:12:58 +0800 Subject: [PATCH 5/5] fix(sync): use workspace version in protocol test --- flicknote-sync/src/ipc/tests.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/flicknote-sync/src/ipc/tests.rs b/flicknote-sync/src/ipc/tests.rs index b7c878d..2dcb4ac 100644 --- a/flicknote-sync/src/ipc/tests.rs +++ b/flicknote-sync/src/ipc/tests.rs @@ -258,7 +258,10 @@ async fn protocol_v4_client_rejects_protocol_v2_server_info() { assert_eq!(error.code(), PROTOCOL_MISMATCH_CODE); let message = error.to_string(); - assert!(message.contains("CLI version 1.0.0 protocol 4")); + assert!(message.contains(&format!( + "CLI version {} protocol 4", + env!("CARGO_PKG_VERSION") + ))); assert!(message.contains("daemon executable /opt/legacy/flicknote")); assert!(message.contains("daemon version legacy protocol 2")); assert!(message.contains("daemon restart"));