Skip to content

fix: resolve test upgrade staying pending with Invalid Date#2127

Merged
Yeraze merged 1 commit intomainfrom
fix/upgrade-test-pending-invalid-date
Mar 4, 2026
Merged

fix: resolve test upgrade staying pending with Invalid Date#2127
Yeraze merged 1 commit intomainfrom
fix/upgrade-test-pending-invalid-date

Conversation

@Yeraze
Copy link
Owner

@Yeraze Yeraze commented Mar 4, 2026

Summary

Fixes #2125

  • Watchdog JSON parsing: grep patterns now handle optional whitespace in pretty-printed JSON trigger files (BusyBox-compatible [ ]* character classes instead of \s)
  • Frontend field mismatch: UpgradeStatus interface updated to match backend API (startedAt/completedAt/toVersion/error instead of startTime/endTime/targetVersion/message), eliminating "Invalid Date" display
  • Per-ID status sync: getUpgradeStatus() now checks the watchdog status file for terminal states (complete/failed) and syncs to DB, matching the existing pattern in getActiveUpgrade()

Test plan

  • Build and deploy dev container
  • Navigate to Auto Upgrade test section
  • Click "Test Upgrade Process"
  • Verify UI shows valid dates and progresses through states
  • After container restart, verify upgrade history shows "complete" with valid timestamps
  • Run npm test — all 2911 tests pass

🤖 Generated with Claude Code

Three bugs prevented the test upgrade UI from reflecting actual status:

1. Watchdog couldn't parse pretty-printed JSON trigger file - grep patterns
   now handle optional whitespace (BusyBox-compatible)
2. Frontend UpgradeStatus interface used wrong field names (startTime vs
   startedAt, targetVersion vs toVersion, etc.) causing "Invalid Date"
3. Per-ID status endpoint never synced watchdog file status to DB - added
   terminal-state file-sync matching the pattern in getActiveUpgrade()

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Mar 4, 2026

Claude finished @Yeraze's task —— View job


Code Review Complete ✅

Summary

This PR effectively addresses the "Invalid Date" issue in the upgrade test section. The changes are well-focused and improve reliability across the upgrade system. Overall code quality is good with some minor suggestions below.

Detailed Analysis

✅ Code Quality & Best Practices

scripts/upgrade-watchdog.sh:401-403

  • Good: The BusyBox-compatible regex fix using [ ]* instead of \s for optional whitespace is the right approach for cross-platform compatibility
  • Minor: Could benefit from comments explaining the BusyBox compatibility requirement

src/components/configuration/AutoUpgradeTestSection.tsx:30-41

  • Good: Updated UpgradeStatus interface properly aligns with backend API (toVersion, startedAt, completedAt, error)
  • Good: TypeScript interface provides clear type safety
  • Good: Clean date handling with new Date().toISOString() format

src/server/services/upgradeService.ts:237-258

  • Excellent: Status sync logic in getUpgradeStatus() properly handles terminal state detection from watchdog file
  • Good: Error handling for file operations with graceful degradation
  • Good: Consistent logging with upgrade ID for traceability

✅ Bug Fixes

  1. Frontend field mismatch: Interface now correctly uses startedAt/completedAt instead of startTime/endTime - this directly fixes the "Invalid Date" display
  2. JSON parsing: BusyBox-compatible regex patterns prevent parsing failures in minimal environments
  3. Status synchronization: Backend now properly syncs terminal states from watchdog file to database

✅ Performance Considerations

  • File system operations are lightweight and appropriately cached
  • Status polling interval (2000ms) is reasonable for UI responsiveness
  • Database queries are efficient with targeted lookups by upgrade ID

⚠️ Minor Concerns

  1. scripts/upgrade-watchdog.sh:401-403 - The regex patterns could benefit from more robust error handling:

    # Consider adding validation
    if [ -z "$version" ] && echo "$trigger_data" | grep -q '"version"'; then
      log_error "Failed to parse version from trigger file"
      return 1
    fi
  2. src/components/configuration/AutoUpgradeTestSection.tsx:307-309 - Date parsing could use defensive programming:

    // Consider validating dates before parsing
    const startDate = upgradeStatus.startedAt ? new Date(upgradeStatus.startedAt) : new Date();
    {startDate.getTime() ? startDate.toLocaleString() : 'Invalid Date'}
  3. src/server/services/upgradeService.ts:242 - File reading could specify encoding explicitly:

    const fileStatus = fs.readFileSync(UPGRADE_STATUS_FILE, { encoding: 'utf-8' }).trim().toLowerCase();

✅ Security

  • File operations use proper error handling
  • No injection vulnerabilities in JSON parsing
  • Atomic file writes prevent race conditions

⚠️ Test Coverage

The PR description indicates all 2,911 tests pass, but consider adding specific tests for:

  • Invalid date handling in the frontend component
  • BusyBox regex compatibility in shell script parsing
  • Status sync edge cases in the upgrade service

📋 Recommendations

  1. Add integration test for the complete upgrade flow to catch future "Invalid Date" regressions
  2. Consider defensive date parsing in the React component to handle any future API changes
  3. Document BusyBox compatibility requirement in the shell script comments

Approval Status

Approve - This is a solid fix that addresses the core issue effectively. The minor suggestions above are non-blocking improvements.


@Yeraze Yeraze merged commit 9bf38f7 into main Mar 4, 2026
13 checks passed
@Yeraze Yeraze deleted the fix/upgrade-test-pending-invalid-date branch March 4, 2026 21:59
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.

[BUG] Test Upgrade Process completes in logs but stays pending in upgrade_history and UI shows Invalid Date

1 participant