Skip to content

Conversation

@michelroegl-brunner
Copy link
Member

🐛 Fix: Autosync Disable Functionality and Multiple Instance Issues

Problem

The autosync feature had several critical issues:

  1. Autosync continued running after being disabled - The toggle in GUI would show disabled but autosync would keep executing
  2. Multiple autosync instances running simultaneously - Causing 6+ notifications and rate limit errors
  3. Cron jobs not properly destroyed - Only stopped but not destroyed, allowing continued execution
  4. GitHub API rate limit errors - No token authentication and poor error handling

Root Causes

  • 's method only stops current execution but doesn't destroy the scheduled job
  • Multiple service instances being created without proper cleanup
  • No global lock to prevent concurrent autosync execution
  • Missing GitHub token authentication in sync services
  • Poor error handling for rate limit scenarios

Solution

🔧 Core Fixes

  • Added method to properly remove cron jobs from scheduler
  • Implemented global lock system to prevent multiple autosync instances
  • Enhanced safety checks in cron job execution with multiple validation layers
  • Added GitHub token authentication to all sync services
  • Improved error handling with specific rate limit error detection

🛠️ Technical Changes

AutoSyncService ():

  • Added to prevent concurrent execution
  • Enhanced to call both and
  • Added multiple safety checks in cron job execution
  • Improved error handling with detection
  • Added proper TypeScript type definitions

GitHub Services:

  • Added GitHub token authentication to and
  • Enhanced error handling for 403 rate limit responses
  • Better error messages for users

API Routes:

  • Fixed validation fallback for custom cron expressions
  • Enhanced error reporting in autosync settings API
  • Proper global service instance management

Frontend:

  • Added auto-save functionality for autosync toggle
  • Better error handling for API validation errors
  • Enhanced UI feedback for autosync status

🧪 Testing

  • ✅ Autosync properly stops when disabled
  • ✅ No more multiple notifications (was 6+, now 1)
  • ✅ No more concurrent autosync instances
  • ✅ GitHub token authentication working
  • ✅ Proper error reporting for rate limits
  • ✅ All TypeScript linting errors resolved

📝 Commit History

  • 16947ce - Remove debug logs from autosync service
  • eca0cb5 - Fix TypeScript linting errors in autoSyncService
  • 011cbd2 - Fix autosync continuing to run after being disabled
  • 19e18b4 - Fix multiple autosync instances running simultaneously
  • ffef631 - Add error reporting for GitHub rate limit errors
  • 7b4daf8 - Fix autosync continuing to run after being disabled
  • fdeda6c - Add GitHub token authentication to sync services
  • 5acaf14 - Fix autosync toggle disable functionality
  • 926032e - Fix auto-sync service not stopping when disabled

🎯 Impact

  • User Experience: Autosync now properly respects the disable setting
  • Performance: Eliminated multiple concurrent instances and spam notifications
  • Reliability: Better error handling and GitHub API rate limit management
  • Maintainability: Cleaner code with proper TypeScript types

Fixes autosync disable functionality and resolves multiple instance issues that were causing spam notifications and rate limit errors.

- Fix type signature mismatch in handleCardClick functions
  - Updated ScriptsGrid.tsx and DownloadedScriptsTab.tsx to accept ScriptCardType instead of { slug: string }
  - This resolves the modal not opening when clicking script cards

- Fix server-side import issues
  - Updated autoSyncService.js to import from githubJsonService.ts instead of .js
  - Fixed path aliases in githubJsonService.ts to use relative imports
  - Updated scripts.ts to import from TypeScript service files directly

- Fix missing Install button
  - Resolved scriptDownloaderService.checkScriptExists method not being available
  - Install button now appears when script files exist locally

- Remove debug logging
  - Cleaned up temporary console.log statements and debug UI elements

All script card interactions now work properly:
- Cards open detail modal when clicked
- Install button appears when appropriate
- Server-side API calls work correctly
- Delete stub scriptDownloader.js that contained placeholder implementation
- Implement real JavaScript script downloader with GitHub fetch functionality
- Fix incremental JSON sync to only process newly synced files
- Add proper error handling and file structure management
- Support all script types (ct/, tools/, vm/, vw/) with directory preservation
- Download install scripts for CT scripts
- Re-enable auto-sync service to use real implementation

Scripts now download real content from GitHub instead of placeholders.
- Store full script objects instead of just names in results.newScripts and results.updatedScripts
- This allows groupScriptsByCategory to properly group scripts by their categories
- Notification will now show actual script names grouped by category instead of 'undefined synced, undefined up to date'
- Script objects contain name, slug, and categories fields needed for proper grouping
- Implement modifyScriptContent method to replace GitHub source line with local source
- Replace 'source <(curl -fsSL https://raw.githubusercontent.com/community-scripts/ProxmoxVE/main/misc/build.func)'
  with 'SCRIPT_DIR="." \nsource "/../core/build.func"'
- This ensures CT scripts use local build.func instead of downloading from GitHub
- Applied to all CT scripts during download process
- Tested with 2fauth script - replacement works correctly
- Add checkScriptExists method that was missing from JavaScript implementation
- This method is required by the checkScriptFiles API endpoint
- Method checks if CT script and install script files exist locally
- Returns ctExists, installExists, and files array
- Fixes issue where UI shows 'Load Script' instead of 'Install' button for downloaded scripts
- Add missing 'access' import from 'fs/promises'
- This was causing checkScriptExists method to fail with 'access is not defined' error
- Now properly detects when script files exist locally
- Fixes issue where UI shows 'Load Script' instead of 'Install' button for downloaded scripts
- Use global auto-sync service instance instead of creating new instances
- This ensures stopAutoSync() is called on the actual running service
- Fix .env file parsing to handle comments and empty lines properly
- Update both TRPC and REST API endpoints to use global instance
- Auto-sync service will now properly stop when disabled in settings
- Fix service instance management to use global instance for stopping autosync
- Add automatic saving when toggle is changed (no manual save required)
- Fix validation issue where custom sync type without cron expression caused 400 error
- Add comprehensive debugging and error handling
- Ensure .env file is properly updated with AUTO_SYNC_ENABLED value
- Improve service lifecycle management with proper state cleanup
- Add fallback logic for invalid sync interval configurations

Resolves issue where disabling autosync in GUI didn't update .env file or stop service
- Add GitHub token authentication to GitHubJsonService for API calls
- Add GitHub token authentication to GitHubService for API calls
- Update fetchFromGitHub methods to use GITHUB_TOKEN from .env
- Update downloadJsonFile methods to use GitHub token for raw file downloads
- Add proper error handling for rate limit exceeded (403) errors
- Add console logging to show when token is/isn't being used
- Improve error messages to suggest setting GITHUB_TOKEN for higher rate limits

This ensures that when a GitHub token is specified in .env, it will be used
for all GitHub API calls during sync operations, providing higher rate limits
and better reliability.
- Add defensive check in cron job execution to stop if autosync is disabled
- Ensure isRunning flag is set to false when stopping autosync
- Add logging to show when autosync is disabled and not scheduling
- Fix both API route and TRPC router to properly stop service
- Prevent multiple cron jobs from running simultaneously

This fixes the issue where autosync would continue running even after
being disabled in the GUI, causing rate limit errors and unwanted syncs.
- Add specific error handling for GitHub API rate limit (403) errors
- Create RateLimitError with proper error name for identification
- Store last sync error and error time in settings for UI display
- Add error status display in autosync settings modal
- Show user-friendly error messages with GitHub token suggestion
- Clear error status on successful sync
- Update both GitHubJsonService and GitHubService with rate limit error handling

This provides better user feedback when GitHub API rate limits are exceeded
and guides users to set up a GitHub token for higher rate limits.
- Add global lock to prevent multiple autosync instances from running
- Add initialization check to prevent multiple service creation
- Add global lock checks in cron job execution
- Prevent multiple notifications from being sent
- Fix TypeScript errors with error field types

This fixes the issue where 6+ autosync instances were running simultaneously,
causing multiple notifications and rate limit issues.
- Add destroy() method to properly stop cron jobs
- Add additional safety checks in cron job execution
- Add debugging logs to track cron job lifecycle
- Ensure isRunning is set to false even when no cron job exists
- Add null check for cronJob before execution

This should prevent autosync from continuing to run after being disabled.
- Add JSDoc type definitions for settings object
- Fix appriseUrls undefined checks
- Fix null assignment for lastAutoSyncError
- Add proper type annotations for error fields

All TypeScript linting errors are now resolved.
- Remove excessive debug logging from scheduleAutoSync and stopAutoSync
- Keep essential logging for troubleshooting
- Clean up console output for production use
@michelroegl-brunner michelroegl-brunner requested a review from a team as a code owner October 24, 2025 20:34
@michelroegl-brunner michelroegl-brunner merged commit 68b6fc8 into main Oct 24, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No option to install downloaded scripts Add auto json sync and auto download options

2 participants