Files
mcphub/BUGFIX_SUMMARY.md
copilot-swe-agent[bot] 1ff542ed45 Add bug fix summary documentation
Co-authored-by: samanhappy <2755122+samanhappy@users.noreply.github.com>
2025-10-11 15:27:47 +00:00

5.4 KiB

Bug Fix: Group Creation Not Persisting in v0.9.11

Issue Description

After deploying version 0.9.11, users were unable to add groups. The group creation appeared to succeed (no errors were reported), but the groups list remained empty.

Root Cause Analysis

The problem was in the mergeSettings implementations in both DataServiceImpl and DataServicex:

Before Fix

DataServiceImpl.mergeSettings:

mergeSettings(all: McpSettings, newSettings: McpSettings, _user?: IUser): McpSettings {
  return newSettings;  // Simply returns newSettings, discarding fields from 'all'
}

DataServicex.mergeSettings (admin user):

const result = { ...all };
result.users = newSettings.users;           // Only copied users
result.systemConfig = newSettings.systemConfig;  // Only copied systemConfig
return result;
// Missing: groups, mcpServers, userConfigs

The Problem Flow

When a user created a group through the API:

  1. groupService.createGroup() loaded settings: loadSettings() → returns complete settings
  2. Modified the groups array by adding new group
  3. Called saveSettings(modifiedSettings)
  4. saveSettings() called mergeSettings(originalSettings, modifiedSettings)
  5. mergeSettings() only preserved users and systemConfig, discarding the groups array
  6. The file was saved without groups
  7. Result: Groups were never persisted!

Why This Happened

The mergeSettings function is designed to selectively merge changes from user operations while preserving the rest of the original settings. However, the implementations were incomplete and only handled users and systemConfig, ignoring:

  • groups (the bug causing this issue!)
  • mcpServers
  • userConfigs (in DataServiceImpl)

Solution

Updated both mergeSettings implementations to properly preserve ALL fields:

DataServiceImpl.mergeSettings (Fixed)

mergeSettings(all: McpSettings, newSettings: McpSettings, _user?: IUser): McpSettings {
  return {
    ...all,
    ...newSettings,
    // Explicitly handle each field, preserving from 'all' when not in newSettings
    users: newSettings.users !== undefined ? newSettings.users : all.users,
    mcpServers: newSettings.mcpServers !== undefined ? newSettings.mcpServers : all.mcpServers,
    groups: newSettings.groups !== undefined ? newSettings.groups : all.groups,
    systemConfig: newSettings.systemConfig !== undefined ? newSettings.systemConfig : all.systemConfig,
    userConfigs: newSettings.userConfigs !== undefined ? newSettings.userConfigs : all.userConfigs,
  };
}

DataServicex.mergeSettings (Fixed)

if (!currentUser || currentUser.isAdmin) {
  const result = { ...all };
  // Merge all fields, using newSettings values when present
  if (newSettings.users !== undefined) result.users = newSettings.users;
  if (newSettings.mcpServers !== undefined) result.mcpServers = newSettings.mcpServers;
  if (newSettings.groups !== undefined) result.groups = newSettings.groups;  // FIXED!
  if (newSettings.systemConfig !== undefined) result.systemConfig = newSettings.systemConfig;
  if (newSettings.userConfigs !== undefined) result.userConfigs = newSettings.userConfigs;
  return result;
}

Changes Made

Modified Files

  1. src/services/dataService.ts - Fixed mergeSettings implementation
  2. src/services/dataServicex.ts - Fixed mergeSettings implementation

New Test Files

  1. tests/services/groupService.test.ts - 11 tests for group operations
  2. tests/services/dataServiceMerge.test.ts - 7 tests for mergeSettings behavior
  3. tests/integration/groupPersistence.test.ts - 5 integration tests

Test Coverage

Before Fix

  • 81 tests passing
  • No tests for group persistence or mergeSettings behavior

After Fix

  • 104 tests passing (23 new tests)
  • Comprehensive coverage of:
    • Group creation and persistence
    • mergeSettings behavior for both implementations
    • Integration tests verifying end-to-end group operations
    • Field preservation during merge operations

Verification

Automated Tests

pnpm test:ci
# Result: 104 tests passed

Manual Testing

Created a test script that:

  1. Creates a group
  2. Clears cache
  3. Reloads settings
  4. Verifies the group persists

Result: Group persists correctly

Integration Test Output

✅ Group creation works correctly
✅ Group persistence works correctly
✅ All tests passed! The group creation bug has been fixed.

Impact Assessment

Risk Level: LOW

  • Minimal code changes (only mergeSettings implementations)
  • All existing tests continue to pass
  • No breaking changes to API or behavior
  • Only fixes broken functionality

Affected Components

  • Group creation
  • Group updates
  • Server additions
  • User config updates
  • System config updates

No Impact On

  • MCP server operations
  • Authentication
  • API endpoints
  • Frontend components
  • Routing logic

Deployment Notes

This fix is backward compatible and can be deployed immediately:

  • No database migrations required
  • No configuration changes needed
  • Existing groups (if any managed to be saved) remain intact
  • Fix is transparent to users

Conclusion

The bug has been completely fixed with minimal, surgical changes to two functions. The fix:

  • Resolves the reported issue
  • Maintains backward compatibility
  • Adds comprehensive test coverage
  • Passes all existing tests
  • Has been verified manually

Users can now successfully create and persist groups as expected.