Skip to content

Fix auto-sync reconcile cadence#157

Merged
willwashburn merged 2 commits into
mainfrom
fix/issue-135-auto-sync-reconcile
May 14, 2026
Merged

Fix auto-sync reconcile cadence#157
willwashburn merged 2 commits into
mainfrom
fix/issue-135-auto-sync-reconcile

Conversation

@willwashburn
Copy link
Copy Markdown
Member

Summary

  • Confirmed local-mount: 10 s periodic reconcile is redundant with @parcel/watcher subscriptions #135 is still relevant on current origin/main: startAutoSync still used a 10s fixed full-reconcile timer.
  • Replaced the fixed interval with a watcher-health-aware scheduler: healthy watchers default to 60s full scans, degraded watchers keep the 10s safety cadence, and explicit scanIntervalMs remains backwards-compatible unless healthyScanIntervalMs is set.
  • Added scanIntervalMs 0/Infinity disable support, docs/changelog updates, and mocked scheduler tests for disabled, healthy, and degraded modes.

Closes #135

Verification

  • npm run typecheck --workspace=packages/local-mount
  • npm run test --workspace=packages/local-mount
  • npm run build
  • go test ./...
  • ./scripts/check-contract-surface.sh

Note: root npm run test currently passes core, SDK, and local-mount, then fails in packages/file-observer because jsdom is not installed; that package is not touched by this PR and is not part of the current CI workflow.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 14, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ef2d8d73-515e-46c6-8924-ddb53995e9a0

📥 Commits

Reviewing files that changed from the base of the PR and between 1987e2c and 1b4e73f.

📒 Files selected for processing (2)
  • packages/local-mount/src/auto-sync-scheduler.test.ts
  • packages/local-mount/src/auto-sync.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/local-mount/src/auto-sync.ts

📝 Walkthrough

Walkthrough

Refactors periodic full-reconcile scheduling in startAutoSync to use a health-aware timeout scheduler: adds healthyScanIntervalMs, validates/normalizes intervals (disable via 0/Infinity), schedules reconciles based on watcher health, adds tests, and updates docs and changelog.

Changes

Health-aware periodic reconcile scheduling

Layer / File(s) Summary
Contract and validation: healthyScanIntervalMs option and scan interval normalization
packages/local-mount/src/auto-sync.ts
AutoSyncOptions adds healthyScanIntervalMs?: number. normalizeScanInterval validates scanIntervalMs and healthyScanIntervalMs, rejecting invalid values and treating 0/Infinity as disabled.
Health-aware scheduler implementation and watcher settlement
packages/local-mount/src/auto-sync.ts
Replaces fixed setInterval with a setTimeout-based scheduler (schedulePeriodicReconcile, reschedulePeriodicReconcile, clearPeriodicReconcile) that selects delays based on watcher health; markWatcherDegraded now flags degraded state and reschedules; periodic reconcile is scheduled only after watcher subscription outcomes; stop() clears the timer.
Test coverage: scheduler policy for disabled, healthy, and degraded states
packages/local-mount/src/auto-sync-scheduler.test.ts
New Vitest suite mocks @parcel/watcher and verifies disabled intervals (0/Infinity), too-large-interval validation, healthy-state usage of healthyScanIntervalMs, and degraded-state behavior including onError invocation and degraded-interval reconciles.
Documentation and release notes
packages/local-mount/README.md, packages/local-mount/src/mount.ts, packages/local-mount/CHANGELOG.md
README updated with healthyScanIntervalMs docs, examples, and "How it works" text; mount.ts JSDoc clarified; CHANGELOG Unreleased notes the watcher-health-dependent cadence and disable semantics.

Sequence Diagram

sequenceDiagram
  participant startAutoSync
  participant WatcherSubscriptions
  participant PeriodicScheduler
  participant Reconcile
  participant onError

  startAutoSync->>WatcherSubscriptions: start mount & project subscribe
  WatcherSubscriptions->>startAutoSync: settle (success or failure)
  alt both healthy
    startAutoSync->>PeriodicScheduler: schedule(healthyScanIntervalMs)
  else degraded
    startAutoSync->>PeriodicScheduler: schedule(scanIntervalMs)
    startAutoSync->>onError: forward subscription error
  end
  PeriodicScheduler->>Reconcile: setTimeout -> run reconcile
  Reconcile->>PeriodicScheduler: reschedule next tick
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

  • #135: This PR implements watcher-health-driven reconcile cadence and supports disabling periodic reconciles, addressing the objectives described in the issue.
  • #119: Related to watcher health signals (watchersHealthy) and state used by this scheduler change.

Poem

🐰 I hopped through timers, gentle and spry,
Healthy watchers hum while the big scans lie by,
If errors ring out, I thump a loud beat,
Reschedule, reconcile, keep trees in sync neat,
A rabbit’s nod to timers that sigh.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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 'Fix auto-sync reconcile cadence' accurately summarizes the main change: adjusting the periodic full-reconcile timer strategy based on watcher health.
Description check ✅ Passed The description is directly related to the changeset, detailing the fix for issue #135 with specific implementation details about watcher-health-aware scheduling and verification steps.
Linked Issues check ✅ Passed The PR addresses all core objectives from #135: implements watcher-health-aware scheduler (approach 2), supports disabling periodic scans via scanIntervalMs 0/Infinity (approach 3), defaults to 60s for healthy watchers and 10s for degraded watchers, and includes test coverage for these modes.
Out of Scope Changes check ✅ Passed All changes are scoped to the auto-sync reconcile cadence feature: AutoSyncOptions extended with healthyScanIntervalMs, periodic reconcile scheduler refactored, tests added, and documentation/changelog updated—no unrelated changes present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-135-auto-sync-reconcile

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.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 14, 2026

Relayfile Eval Review

Run: .relayfile/evals/runs/2026-05-14T12-37-06-422Z-HEAD-provider
Mode: provider
Git SHA: 1972716

Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0

Human Review Cases

No reviewable human-review cases captured Relayfile output.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/local-mount/src/auto-sync.ts`:
- Around line 258-266: markWatcherDegraded currently forces rescheduling on
every error which can repeatedly push the safety reconcile out; change
markWatcherDegraded to only call reschedulePeriodicReconcile when the degraded
state transitions from false to true (i.e., if (!watcherDegraded) {
watcherDegraded = true; reschedulePeriodicReconcile(); }) and otherwise just
call onError(err); keep references to markWatcherDegraded,
reschedulePeriodicReconcile, watcherDegraded and periodicReconcileRunning when
making the change.
- Around line 97-107: The normalizeScanInterval function currently allows any
finite non-negative number; enforce the setTimeout maximum by rejecting values >
2_147_483_647 (2**31-1) so callers don't inadvertently pass an overflowed delay
that becomes 1ms; inside normalizeScanInterval (function name) after computing
interval, treat 0 and Infinity as before, then if interval > 2147483647 throw a
RangeError (similar to the existing RangeError message) so only values within
[0, 2147483647] or Infinity are accepted.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 00ae8060-dbb3-4506-923a-7b727a84708b

📥 Commits

Reviewing files that changed from the base of the PR and between 4968fca and 1987e2c.

📒 Files selected for processing (5)
  • packages/local-mount/CHANGELOG.md
  • packages/local-mount/README.md
  • packages/local-mount/src/auto-sync-scheduler.test.ts
  • packages/local-mount/src/auto-sync.ts
  • packages/local-mount/src/mount.ts

Comment thread packages/local-mount/src/auto-sync.ts
Comment thread packages/local-mount/src/auto-sync.ts
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

@willwashburn
Copy link
Copy Markdown
Member Author

Addressed the two CodeRabbit findings in 1b4e73f: repeated watcher errors now only reschedule on the first transition to degraded, and scan intervals above Node's setTimeout maximum are rejected with regression coverage.

@willwashburn willwashburn merged commit f04e78f into main May 14, 2026
8 checks passed
@willwashburn willwashburn deleted the fix/issue-135-auto-sync-reconcile branch May 14, 2026 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

local-mount: 10 s periodic reconcile is redundant with @parcel/watcher subscriptions

1 participant