fix(sync): re-establish file watch after delete and restore by mvanhorn · Pull Request #2001 · open-feature/flagd

mvanhorn · 2026-07-15T06:09:32Z

Summary

flagd now recovers its file watch when a watched flag-definition file is deleted and quickly restored, instead of silently going stale.

Why this matters

The reporter of #1876 hit this on Windows by deleting the flag file and immediately undoing (ctrl+z): after the restore, /ofrep/v1/evaluate/flags kept returning stale data and later edits to the file were never picked up. A maintainer confirmed the report, labeled it bug / help wanted, and left it open.

The cause is in core/pkg/sync/file/filepath_sync.go. On a Remove event the sync loop re-added the watcher with a single fs.watcher.Add(fs.URI) call (originally there to handle Kubernetes ConfigMap symlink swaps). During a delete+restore race the file can be momentarily absent at that instant, so Add fails, the code logs "error restoring watcher" and gives up. The watch is then lost permanently even though the file exists again a moment later.

Changes

Extracted the re-add into a bounded-retry helper, reAddWatcher(ctx), that retries Add a few times with a short backoff and aborts early on ctx cancellation. A file that returns within the retry window is re-watched. The retry is watcher-agnostic, so it fixes both the fsnotify and fileinfo paths. The retry bound and backoff are package constants (reAddMaxAttempts, reAddBackoff).

Testing

go test ./pkg/sync/file/ (including -race). Added three unit tests against a mock Watcher:

transient failure then success (delete + restore) re-establishes the watch,
a truly deleted file exhausts the bounded attempts and returns the error (existing default-state behavior preserved),
a context cancelled during backoff returns promptly instead of running out the attempt budget.

The existing TestSimpleSync delete/update cases (real fsnotify over a temp dir) stay green, confirming single-delete and ordinary-write behavior is unchanged.

Fixes #1876

netlify · 2026-07-15T06:09:37Z

✅ Deploy Preview for polite-licorice-3db33c canceled.

Name	Link
🔨 Latest commit	`ed4c05d`
🔍 Latest deploy log	https://app.netlify.com/projects/polite-licorice-3db33c/deploys/6a5e41f74577860008599e7c

coderabbitai · 2026-07-15T06:09:55Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 59069770-e2bd-46de-8b71-d527349c8f21

📥 Commits

Reviewing files that changed from the base of the PR and between be2ce02 and ed4c05d.

📒 Files selected for processing (2)

core/pkg/sync/file/filepath_sync.go
core/pkg/sync/file/filepath_sync_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

core/pkg/sync/file/filepath_sync.go
core/pkg/sync/file/filepath_sync_test.go

📝 Walkthrough

Walkthrough

The file sync loop retries watcher re-establishment after remove events with bounded attempts, backoff, and context cancellation. Tests cover transient failures, exhausted retries, and cancellation during backoff.

Changes

File watcher recovery

Layer / File(s)	Summary
Watcher re-add implementation `core/pkg/sync/file/filepath_sync.go`	The remove-event path calls `reAddWatcher`, which retries `watcher.Add` with bounded attempts, backoff, and cancellation handling.
Watcher re-add validation `core/pkg/sync/file/filepath_sync_test.go`	Mock watcher helpers and tests cover retry success, maximum-attempt failure, error identity, and prompt context cancellation.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main fix: re-establishing a file watch after delete and restore.
Description check	✅ Passed	The description is directly related to the file-watch recovery bug and explains the fix, rationale, and tests.
Linked Issues check	✅ Passed	The changes match `#1876` by retrying watcher re-add after delete/restore and adding tests for recovery and cancellation.
Out of Scope Changes check	✅ Passed	The patch stays focused on watcher re-add retry logic and its tests, with no obvious unrelated changes.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

leakonvalinka · 2026-07-20T14:50:52Z

+	// single attempt is not enough to recover the watch.
+	reAddMaxAttempts = 5
+	// reAddBackoff is the delay between re-add attempts.
+	reAddBackoff = 100 * time.Millisecond


Not sure if this is enough time to catch manual restores after accidental deletes?
I wonder if we should make it configurable too, but that's probably overkill for an edge case.

Ya, I had the same thought. I think we are less concerned with manual delete/add-back, and mostly concerned with automations/scripts and editors, some non-atomic mount/volume update mechanisms (plain hostPath scripts, certain network filesystems) that don't use K8s-style atomic swaps... But for the sake of picking a nice round number, I've made it 1s.

leakonvalinka

Looks good! I left a question but that's not a blocker for me.

toddbaert · 2026-07-20T15:15:26Z

+	reAddMaxAttempts = 5
+	// reAddBackoff is the delay between re-add attempts.
+	reAddBackoff = 100 * time.Millisecond


Suggested change

reAddMaxAttempts = 5

// reAddBackoff is the delay between re-add attempts.

reAddBackoff = 100 * time.Millisecond

reAddMaxAttempts = 6

// reAddBackoff is the delay between re-add attempts.

reAddBackoff = 200 * time.Millisecond

I think we should change this to 200, x 6, which with the logic below equals 1s total, which seems a bit less arbitrary than the 400ms total we get now.

Pushed this.

On a remove event the file sync loop re-added the watcher with a single Add call. During a delete quickly followed by a restore the file can be momentarily absent at that instant, so the Add fails, the watch is dropped, and later edits are never picked up. Retry the re-add a bounded number of times with a short backoff so a file that returns within the window is re-watched, aborting early on context cancellation. The retry is watcher-agnostic, covering both the fsnotify and fileinfo paths. Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>

toddbaert · 2026-07-20T15:28:09Z

Hey @mvanhorn ... I think this is a good change overall. I shortened some particularly wordy comments and tweaked your constants so that we re-attach for up to 1s here.

Thanks!

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>

sonarqubecloud · 2026-07-20T15:43:32Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

mvanhorn · 2026-07-23T01:25:30Z

Thanks for landing this, @toddbaert - the file watch surviving delete-and-restore closes a real sync gap.

mvanhorn requested review from a team as code owners July 15, 2026 06:09

dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jul 15, 2026

leakonvalinka reviewed Jul 20, 2026

View reviewed changes

leakonvalinka approved these changes Jul 20, 2026

View reviewed changes

toddbaert reviewed Jul 20, 2026

View reviewed changes

toddbaert force-pushed the fix/1876-refile-watcher-on-delete-restore branch from a3bb32d to be2ce02 Compare July 20, 2026 15:26

fixup: tweak comments, timing

ed4c05d

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>

toddbaert force-pushed the fix/1876-refile-watcher-on-delete-restore branch from be2ce02 to ed4c05d Compare July 20, 2026 15:42

toddbaert self-requested a review July 20, 2026 15:56

toddbaert approved these changes Jul 20, 2026

View reviewed changes

toddbaert merged commit d434adf into open-feature:main Jul 20, 2026
18 checks passed

github-actions Bot mentioned this pull request Jul 20, 2026

chore: release main #2000

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(sync): re-establish file watch after delete and restore#2001

fix(sync): re-establish file watch after delete and restore#2001
toddbaert merged 2 commits into
open-feature:mainfrom
mvanhorn:fix/1876-refile-watcher-on-delete-restore

mvanhorn commented Jul 15, 2026

Uh oh!

netlify Bot commented Jul 15, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 15, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

leakonvalinka Jul 20, 2026

Uh oh!

toddbaert Jul 20, 2026

Uh oh!

leakonvalinka left a comment

Uh oh!

toddbaert Jul 20, 2026

Uh oh!

toddbaert Jul 20, 2026

Uh oh!

toddbaert commented Jul 20, 2026 •

edited

Loading

Uh oh!

sonarqubecloud Bot commented Jul 20, 2026

Uh oh!

Uh oh!

mvanhorn commented Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

mvanhorn commented Jul 15, 2026

Summary

Why this matters

Changes

Testing

Uh oh!

netlify Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for polite-licorice-3db33c canceled.

Uh oh!

coderabbitai Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

leakonvalinka Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

toddbaert Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

leakonvalinka left a comment

Choose a reason for hiding this comment

Uh oh!

toddbaert Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

toddbaert Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

toddbaert commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud Bot commented Jul 20, 2026

Quality Gate passed

Uh oh!

Uh oh!

mvanhorn commented Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

netlify Bot commented Jul 15, 2026 •

edited

Loading

coderabbitai Bot commented Jul 15, 2026 •

edited

Loading

toddbaert commented Jul 20, 2026 •

edited

Loading