internal/vaultik/daemon.go: The daemon mode implementation including:
RunDaemon() — the main daemon loop that coordinates watching, scheduling, and shutdown
changeTracker — thread-safe tracker that records filesystem paths modified since the last backup
startWatcher() / addWatchRecursive() / watcherLoop() — filesystem watching using fsnotify (inotify on Linux, FSEvents on macOS, etc.)
runDaemonBackup() — executes a single backup run within the daemon loop
snapshotsAffectedByChanges() / isSubpath() — determines which configured snapshots overlap with detected changes
internal/vaultik/daemon_test.go: Unit tests covering:
changeTracker creation, recording, counting, path retrieval, and reset
Concurrent read/write safety of changeTracker
isSubpath() helper with edge cases (root directory, prefix-but-not-subpath, etc.)
Daemon constant validation
Modified files
internal/vaultik/snapshot.go: Replaced the daemon mode stub (return fmt.Errorf("daemon mode not yet implemented")) with a call to v.RunDaemon(opts)
go.mod / go.sum: Added github.com/fsnotify/fsnotify v1.9.0 as a direct dependency (was already an indirect dependency at v1.7.0)
README.md: Added a "Daemon Mode" section documenting how the daemon works, its configuration knobs, and usage
TODO.md: Marked daemon mode items as complete
How it works
Initial backup: On startup, a full backup of all configured snapshots runs immediately
Filesystem watching: All configured snapshot paths are recursively watched for file changes using OS-native notifications via fsnotify
Periodic backups: At each backup_interval tick, if changes were detected and min_time_between_run has elapsed, an incremental backup runs for only the affected snapshots
Full scans: At each full_scan_interval tick, a full backup of all snapshots runs regardless of detected changes
Graceful shutdown: On SIGTERM/SIGINT, the daemon completes any in-progress backup before exiting (with a 5-minute timeout)
Design decisions
Smart snapshot selection: On incremental runs, only snapshots whose configured paths overlap with detected changes are backed up, avoiding unnecessary work
Backup semaphore: A channel-based semaphore prevents concurrent backup runs (timer ticks that fire while a backup is in progress are skipped)
New directory auto-watch: When fsnotify reports a Create event for a directory, it's automatically added to the watcher
No daemon-specific config: Uses existing backup_interval, min_time_between_run, and full_scan_interval config fields that were already defined but unused
Context propagation: Each backup run gets a child context from the daemon's context, enabling clean cancellation on shutdown
## Summary
Implement daemon mode (`--daemon` flag on `snapshot create`), replacing the existing stub that returned an error.
closes #3
## What changed
### New files
- **`internal/vaultik/daemon.go`**: The daemon mode implementation including:
- `RunDaemon()` — the main daemon loop that coordinates watching, scheduling, and shutdown
- `changeTracker` — thread-safe tracker that records filesystem paths modified since the last backup
- `startWatcher()` / `addWatchRecursive()` / `watcherLoop()` — filesystem watching using fsnotify (inotify on Linux, FSEvents on macOS, etc.)
- `runDaemonBackup()` — executes a single backup run within the daemon loop
- `snapshotsAffectedByChanges()` / `isSubpath()` — determines which configured snapshots overlap with detected changes
- **`internal/vaultik/daemon_test.go`**: Unit tests covering:
- `changeTracker` creation, recording, counting, path retrieval, and reset
- Concurrent read/write safety of `changeTracker`
- `isSubpath()` helper with edge cases (root directory, prefix-but-not-subpath, etc.)
- Daemon constant validation
### Modified files
- **`internal/vaultik/snapshot.go`**: Replaced the daemon mode stub (`return fmt.Errorf("daemon mode not yet implemented")`) with a call to `v.RunDaemon(opts)`
- **`go.mod` / `go.sum`**: Added `github.com/fsnotify/fsnotify` v1.9.0 as a direct dependency (was already an indirect dependency at v1.7.0)
- **`README.md`**: Added a "Daemon Mode" section documenting how the daemon works, its configuration knobs, and usage
- **`TODO.md`**: Marked daemon mode items as complete
## How it works
1. **Initial backup**: On startup, a full backup of all configured snapshots runs immediately
2. **Filesystem watching**: All configured snapshot paths are recursively watched for file changes using OS-native notifications via fsnotify
3. **Periodic backups**: At each `backup_interval` tick, if changes were detected and `min_time_between_run` has elapsed, an incremental backup runs for only the affected snapshots
4. **Full scans**: At each `full_scan_interval` tick, a full backup of all snapshots runs regardless of detected changes
5. **Graceful shutdown**: On SIGTERM/SIGINT, the daemon completes any in-progress backup before exiting (with a 5-minute timeout)
## Design decisions
- **Smart snapshot selection**: On incremental runs, only snapshots whose configured paths overlap with detected changes are backed up, avoiding unnecessary work
- **Backup semaphore**: A channel-based semaphore prevents concurrent backup runs (timer ticks that fire while a backup is in progress are skipped)
- **New directory auto-watch**: When fsnotify reports a `Create` event for a directory, it's automatically added to the watcher
- **No daemon-specific config**: Uses existing `backup_interval`, `min_time_between_run`, and `full_scan_interval` config fields that were already defined but unused
- **Context propagation**: Each backup run gets a child context from the daemon's context, enabling clean cancellation on shutdown
Replace the daemon mode stub with a full implementation that:
- Watches configured snapshot paths for filesystem changes using
fsnotify (inotify on Linux, FSEvents on macOS, etc.)
- Runs an initial full backup on startup
- Triggers incremental backups at backup_interval when changes are
detected, only for snapshots whose paths were affected
- Performs full periodic scans at full_scan_interval regardless of
detected changes
- Respects min_time_between_run to prevent excessive backup runs
- Handles SIGTERM/SIGINT for graceful shutdown, completing any
in-progress backup before exiting
- Automatically watches newly created subdirectories
- Uses a backup semaphore to prevent concurrent backup runs
New files:
- internal/vaultik/daemon.go: RunDaemon(), changeTracker, watcher setup
- internal/vaultik/daemon_test.go: Tests for changeTracker, isSubpath,
concurrency safety, and daemon constants
closes#3
REPO_POLICIES Policy 7 violation — missing test for exported function RunDaemon. RunDaemon (internal/vaultik/daemon.go:36) is a new exported method on *Vaultik. It is the primary new API introduced by this PR. There is no unit test for it — not even a minimal test that constructs a Vaultik with a daemon-friendly config and verifies the function can be called and cancelled via context. The test file covers only unexported helpers (changeTracker, isSubpath, constants). Policy 7 states: "Always write tests... If you are writing a new feature, write a test for it." The review test coverage requirement is: "Every new exported function must have at least a basic happy-path test. Missing tests for new APIs = FAIL."
Dead constant daemonWatcherBatchDelay (internal/vaultik/daemon.go:25).
This constant is defined with a doc comment describing batch-settling behavior ("the time to wait after the last filesystem event before considering the batch of changes settled"), but is never used in production code. The only reference is in daemon_test.go:139 where it's asserted to be ≥ 1s. The batch delay logic described in the comment is not implemented — the watcher loop records changes immediately without any debounce/settle delay. This is either incomplete implementation or misleading dead code. Per Policy 10, constants should serve an actual purpose.
Context field swap (internal/vaultik/daemon.go:240-242): runDaemonBackup temporarily mutates v.ctx via direct field assignment. This is safe today because the backup semaphore serializes runs, but it's fragile — any future concurrent usage of v.ctx would create a data race. Passing context as a function parameter would be more robust.
--skip-errors documentation added to README is scope creep (documents a pre-existing flag unrelated to issue #3). Minor, but noted.
No modifications to Makefile, linter config, CI config, or test assertions — no cheating detected.
README daemon mode section is consistent with the implementation.
Verdict: FAIL
Primary reason: The exported function RunDaemon has no unit test, violating REPO_POLICIES Policy 7 and the test coverage requirement for new exported APIs.
Secondary reason: The daemonWatcherBatchDelay constant is defined and documented but unused in production code — either implement the batch delay logic or remove the constant.
## Review: [PR #56](https://git.eeqj.de/sneak/vaultik/pulls/56) — feat: implement daemon mode with filesystem watching
### Policy Divergences
1. **REPO_POLICIES Policy 7 violation — missing test for exported function `RunDaemon`.**
`RunDaemon` (`internal/vaultik/daemon.go:36`) is a new exported method on `*Vaultik`. It is the primary new API introduced by this PR. There is no unit test for it — not even a minimal test that constructs a `Vaultik` with a daemon-friendly config and verifies the function can be called and cancelled via context. The test file covers only unexported helpers (`changeTracker`, `isSubpath`, constants). Policy 7 states: "Always write tests... If you are writing a new feature, write a test for it." The review test coverage requirement is: "Every new exported function must have at least a basic happy-path test. Missing tests for new APIs = FAIL."
2. **Dead constant `daemonWatcherBatchDelay` (`internal/vaultik/daemon.go:25`).**
This constant is defined with a doc comment describing batch-settling behavior ("the time to wait after the last filesystem event before considering the batch of changes settled"), but is **never used in production code**. The only reference is in `daemon_test.go:139` where it's asserted to be ≥ 1s. The batch delay logic described in the comment is not implemented — the watcher loop records changes immediately without any debounce/settle delay. This is either incomplete implementation or misleading dead code. Per Policy 10, constants should serve an actual purpose.
### Requirements Checklist ([issue #3](https://git.eeqj.de/sneak/vaultik/issues/3))
| Requirement | Status |
|---|---|
| Replace daemon mode stub with real implementation | ✅ Met |
| Filesystem watching via inotify/fsnotify | ✅ Met |
| Use `BackupInterval` config field | ✅ Met |
| Use `MinTimeBetweenRun` config field | ✅ Met |
| Use `FullScanInterval` config field | ✅ Met |
| Graceful shutdown on SIGTERM/SIGINT | ✅ Met |
| Complete in-progress backup before exit | ✅ Met |
| Tests for daemon mode | ❌ Partial — helper tests exist, no test for `RunDaemon` itself |
### Build Result
`docker build .` — **PASS** (all stages: lint/fmt-check/test/build green)
### Additional Observations
- **Context field swap** (`internal/vaultik/daemon.go:240-242`): `runDaemonBackup` temporarily mutates `v.ctx` via direct field assignment. This is safe today because the backup semaphore serializes runs, but it's fragile — any future concurrent usage of `v.ctx` would create a data race. Passing context as a function parameter would be more robust.
- **`--skip-errors` documentation** added to README is scope creep (documents a pre-existing flag unrelated to [issue #3](https://git.eeqj.de/sneak/vaultik/issues/3)). Minor, but noted.
- No modifications to Makefile, linter config, CI config, or test assertions — no cheating detected.
- README daemon mode section is consistent with the implementation.
### Verdict: FAIL
**Primary reason:** The exported function `RunDaemon` has no unit test, violating REPO_POLICIES Policy 7 and the test coverage requirement for new exported APIs.
**Secondary reason:** The `daemonWatcherBatchDelay` constant is defined and documented but unused in production code — either implement the batch delay logic or remove the constant.
- Add TestRunDaemon_CancelledContext: exercises RunDaemon with a
daemon-friendly config, cancels via context, verifies clean return
and startup output
- Remove unused daemonWatcherBatchDelay constant (batch-settle logic
was never implemented; the watcher loop records changes immediately)
- Update TestDaemonConstants to remove reference to deleted constant
Both review findings from the initial review were already addressed in commit 87acc05:
Fix 1: RunDaemon test added
TestRunDaemon_CancelledContext in internal/vaultik/daemon_test.go exercises the exported RunDaemon method:
Creates a temp dir with a test file
Constructs a Vaultik instance with daemon-friendly config (BackupInterval, MinTimeBetweenRun, FullScanInterval)
Calls RunDaemon with a context that gets cancelled after 200ms
Verifies graceful shutdown returns nil (no error)
Verifies daemon startup output messages
Fix 2: Dead constant removed
daemonWatcherBatchDelay removed from internal/vaultik/daemon.go — the batch-settle logic it described was never implemented. Its test assertion in TestDaemonConstants was also removed.
## Rework Summary (attempt 3)
Both review findings from [the initial review](https://git.eeqj.de/sneak/vaultik/pulls/56#issuecomment-1311) were already addressed in commit `87acc05`:
### Fix 1: `RunDaemon` test added
`TestRunDaemon_CancelledContext` in `internal/vaultik/daemon_test.go` exercises the exported `RunDaemon` method:
- Creates a temp dir with a test file
- Constructs a `Vaultik` instance with daemon-friendly config (`BackupInterval`, `MinTimeBetweenRun`, `FullScanInterval`)
- Calls `RunDaemon` with a context that gets cancelled after 200ms
- Verifies graceful shutdown returns `nil` (no error)
- Verifies daemon startup output messages
### Fix 2: Dead constant removed
`daemonWatcherBatchDelay` removed from `internal/vaultik/daemon.go` — the batch-settle logic it described was never implemented. Its test assertion in `TestDaemonConstants` was also removed.
### Build verification
`make fmt && docker build .` — **PASS** (all stages: fmt-check, lint, test, build green).
Missing RunDaemon test — ✅Fixed.TestRunDaemon_CancelledContext added in daemon_test.go. The test constructs a Vaultik with daemon-friendly config (BackupInterval, MinTimeBetweenRun, FullScanInterval), calls RunDaemon, cancels the context after 200ms, and verifies both clean shutdown (nil error) and startup output ("Daemon mode started"). Meaningful test that exercises the exported API.
Dead constant daemonWatcherBatchDelay — ✅Fixed.grep -rn daemonWatcherBatchDelay returns zero results across the entire repo. Removed from daemon.go and its assertion removed from TestDaemonConstants. Only daemonMinBackupInterval and daemonShutdownTimeout remain.
Integrity Checks
Check
Result
Makefile modified
✅ No changes
Dockerfile modified
✅ No changes
.golangci.yml modified
✅ No changes (file doesn't exist)
Test assertions weakened
✅ No — only removed assertion for deleted constant
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
clawbot
Summary
Implement daemon mode (
--daemonflag onsnapshot create), replacing the existing stub that returned an error.closes #3
What changed
New files
internal/vaultik/daemon.go: The daemon mode implementation including:RunDaemon()— the main daemon loop that coordinates watching, scheduling, and shutdownchangeTracker— thread-safe tracker that records filesystem paths modified since the last backupstartWatcher()/addWatchRecursive()/watcherLoop()— filesystem watching using fsnotify (inotify on Linux, FSEvents on macOS, etc.)runDaemonBackup()— executes a single backup run within the daemon loopsnapshotsAffectedByChanges()/isSubpath()— determines which configured snapshots overlap with detected changesinternal/vaultik/daemon_test.go: Unit tests covering:changeTrackercreation, recording, counting, path retrieval, and resetchangeTrackerisSubpath()helper with edge cases (root directory, prefix-but-not-subpath, etc.)Modified files
internal/vaultik/snapshot.go: Replaced the daemon mode stub (return fmt.Errorf("daemon mode not yet implemented")) with a call tov.RunDaemon(opts)go.mod/go.sum: Addedgithub.com/fsnotify/fsnotifyv1.9.0 as a direct dependency (was already an indirect dependency at v1.7.0)README.md: Added a "Daemon Mode" section documenting how the daemon works, its configuration knobs, and usageTODO.md: Marked daemon mode items as completeHow it works
backup_intervaltick, if changes were detected andmin_time_between_runhas elapsed, an incremental backup runs for only the affected snapshotsfull_scan_intervaltick, a full backup of all snapshots runs regardless of detected changesDesign decisions
Createevent for a directory, it's automatically added to the watcherbackup_interval,min_time_between_run, andfull_scan_intervalconfig fields that were already defined but unusedReview: PR #56 — feat: implement daemon mode with filesystem watching
Policy Divergences
REPO_POLICIES Policy 7 violation — missing test for exported function
RunDaemon.RunDaemon(internal/vaultik/daemon.go:36) is a new exported method on*Vaultik. It is the primary new API introduced by this PR. There is no unit test for it — not even a minimal test that constructs aVaultikwith a daemon-friendly config and verifies the function can be called and cancelled via context. The test file covers only unexported helpers (changeTracker,isSubpath, constants). Policy 7 states: "Always write tests... If you are writing a new feature, write a test for it." The review test coverage requirement is: "Every new exported function must have at least a basic happy-path test. Missing tests for new APIs = FAIL."Dead constant
daemonWatcherBatchDelay(internal/vaultik/daemon.go:25).This constant is defined with a doc comment describing batch-settling behavior ("the time to wait after the last filesystem event before considering the batch of changes settled"), but is never used in production code. The only reference is in
daemon_test.go:139where it's asserted to be ≥ 1s. The batch delay logic described in the comment is not implemented — the watcher loop records changes immediately without any debounce/settle delay. This is either incomplete implementation or misleading dead code. Per Policy 10, constants should serve an actual purpose.Requirements Checklist (issue #3)
BackupIntervalconfig fieldMinTimeBetweenRunconfig fieldFullScanIntervalconfig fieldRunDaemonitselfBuild Result
docker build .— PASS (all stages: lint/fmt-check/test/build green)Additional Observations
internal/vaultik/daemon.go:240-242):runDaemonBackuptemporarily mutatesv.ctxvia direct field assignment. This is safe today because the backup semaphore serializes runs, but it's fragile — any future concurrent usage ofv.ctxwould create a data race. Passing context as a function parameter would be more robust.--skip-errorsdocumentation added to README is scope creep (documents a pre-existing flag unrelated to issue #3). Minor, but noted.Verdict: FAIL
Primary reason: The exported function
RunDaemonhas no unit test, violating REPO_POLICIES Policy 7 and the test coverage requirement for new exported APIs.Secondary reason: The
daemonWatcherBatchDelayconstant is defined and documented but unused in production code — either implement the batch delay logic or remove the constant.Rework Summary (attempt 3)
Both review findings from the initial review were already addressed in commit
87acc05:Fix 1:
RunDaemontest addedTestRunDaemon_CancelledContextininternal/vaultik/daemon_test.goexercises the exportedRunDaemonmethod:Vaultikinstance with daemon-friendly config (BackupInterval,MinTimeBetweenRun,FullScanInterval)RunDaemonwith a context that gets cancelled after 200msnil(no error)Fix 2: Dead constant removed
daemonWatcherBatchDelayremoved frominternal/vaultik/daemon.go— the batch-settle logic it described was never implemented. Its test assertion inTestDaemonConstantswas also removed.Build verification
make fmt && docker build .— PASS (all stages: fmt-check, lint, test, build green).Review: PR #56 — feat: implement daemon mode (post-rework)
Previous Findings — Verified Fixed
Missing
RunDaemontest — ✅ Fixed.TestRunDaemon_CancelledContextadded indaemon_test.go. The test constructs aVaultikwith daemon-friendly config (BackupInterval,MinTimeBetweenRun,FullScanInterval), callsRunDaemon, cancels the context after 200ms, and verifies both clean shutdown (nilerror) and startup output ("Daemon mode started"). Meaningful test that exercises the exported API.Dead constant
daemonWatcherBatchDelay— ✅ Fixed.grep -rn daemonWatcherBatchDelayreturns zero results across the entire repo. Removed fromdaemon.goand its assertion removed fromTestDaemonConstants. OnlydaemonMinBackupIntervalanddaemonShutdownTimeoutremain.Integrity Checks
docker build .Requirements (issue #3)
snapshot.gonow callsv.RunDaemon(opts)BackupInterval/MinTimeBetweenRun/FullScanIntervalconfigVerdict: PASS ✅
Both previous review findings properly addressed. No regressions, no cheating. Clean implementation.
Pull request closed