feat(config): implement config layering for .mittorc and settings.json

[inercia]

Summary

Implements a layered configuration approach that merges .mittorc and settings.json for ACP servers, fixing the issue where servers defined in .mittorc would be lost when settings were saved via the UI.

Closes #24

Problem

When a user configured ACP servers in .mittorc, those servers would be lost from settings.json after any UI settings change. This happened because:

1. Config loaded from .mittorc exclusively (ignoring settings.json)
2. UI saves wrote to settings.json with only the in-memory servers
3. .mittorc couldn't be modified by the UI (would destroy formatting/comments)

Solution

Implemented a config layering approach:

1. Merge instead of replace: LoadSettingsWithFallback() now merges servers from both .mittorc (higher priority) and settings.json
2. Source tracking: Each server has a Source field (rcfile or settings) to track its origin
3. Read-only RC servers: Servers from .mittorc are shown with a 🔒 lock icon in the UI and cannot be edited/deleted
4. Selective saving: Only settings-sourced servers are saved to settings.json; RC file servers stay in .mittorc untouched
5. Orphaned workspace warning: Shows a warning when workspaces reference servers that were removed from .mittorc

Changes

New Files

• internal/config/merger.go - Generic config merger framework with GenericMerger[T]
• internal/config/merger_test.go - Comprehensive unit tests

Modified Files

• internal/config/config.go - Added Source field to ACPServer
• internal/config/settings.go - Rewrote LoadSettingsWithFallback() for merging
• internal/web/server.go - Added HasRCFileServers field
• internal/web/config_handlers.go - Include source in API responses, filter RC servers on save
• internal/cmd/web.go - Pass HasRCFileServers to server config
• cmd/mitto-app/main.go - Pass HasRCFileServers to server config
• web/static/components/SettingsDialog.js - Lock icon for RC servers, orphaned workspace warning
• .augment/rules/08-config.md - Documentation for layering system

Testing

• ✅ All existing unit tests pass
• ✅ New unit tests for merger functionality
• ✅ Manual testing with Playwright MCP:
  • RC file servers show lock icon and are read-only
  • New servers added via UI are saved to settings.json only
  • .mittorc is never modified
  • Orphaned workspace warning displays correctly

Extensibility

The GenericMerger[T] framework is designed to be reusable for merging other config sections in the future (e.g., prompts, hooks).

---

Pull Request opened by Augment Code with guidance from the PR author

[Copilot]

Pull request overview

This PR implements a layered configuration system that merges .mittorc and settings.json for ACP servers, solving issue #24 where servers defined in .mittorc were lost when settings were saved via the UI.

Changes:

• Introduces a generic merger framework (GenericMerger[T]) for combining configuration from multiple sources with priority-based resolution
• Adds source tracking (SourceRCFile, SourceSettings) to distinguish server origins and enable selective persistence
• Implements read-only UI treatment for RC file servers (lock icons, disabled edit/delete buttons, orphaned workspace warnings)

Reviewed changes

Copilot reviewed 11 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
internal/config/merger.go	New generic merger framework with union/replace strategies for config merging
internal/config/merger_test.go	Comprehensive unit tests for merger functionality
internal/config/config.go	Added Source field to ACPServer for tracking origin
internal/config/settings.go	Rewrote LoadSettingsWithFallback() to merge RC and settings configs with priority handling
internal/web/server.go	Added HasRCFileServers field to track RC file server presence
internal/web/config_handlers.go	Filters RC file servers from saves, includes source in API responses
internal/web/config_validation_test.go	Updated test structs to include Source field
internal/cmd/web.go	Updated config initialization to pass HasRCFileServers
cmd/mitto-app/main.go	Updated macOS app config initialization for layering
web/static/components/SettingsDialog.js	Added lock icons, read-only indicators, and orphaned workspace warnings
.augment/rules/08-config.md	Documented config layering system and merger patterns

Comments suppressed due to low confidence (1)

web/static/components/SettingsDialog.js:1081

• Consider adding a check to prevent modification of RC file servers in the updateServer function. While the UI prevents showing edit buttons for RC file servers (line 1617), adding an explicit guard would provide defense in depth against potential UI bugs or race conditions. For example: if (acpServers.find(s => s.name === oldName)?.source === 'rcfile') { setError('Cannot modify RC file servers'); return; }

  const updateServer = (oldName, newName, newCommand) => {
    if (!newName.trim() || !newCommand.trim()) {
      setError("Server name and command cannot be empty");
      return;
    }

    // Check for duplicate name (excluding current)
    if (
      newName !== oldName &&
      acpServers.some((s) => s.name === newName.trim())
    ) {
      setError("A server with this name already exists");
      return;
    }

    // Update server (prompts are now read-only from files)
    setAcpServers(
      acpServers.map((s) =>
        s.name === oldName
          ? {
              name: newName.trim(),
              command: newCommand.trim(),
              prompts: s.prompts, // Preserve existing prompts (read-only from files)
              source: s.source, // Preserve source (rcfile or settings)
            }
          : s,
      ),
    );

    // Update workspaces that reference this server
    if (newName !== oldName) {
      setWorkspaces(
        workspaces.map((ws) =>
          ws.acp_server === oldName
            ? { ...ws, acp_server: newName.trim() }
            : ws,
        ),
      );
    }

    setEditingServer(null);
    setError("");
  };

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[inercia]

Addressed Review Comments

Thanks for the thorough review! Both issues have been fixed in commit d0a3605.

1. Keychain Password Loading (settings.go:434)

Issue: When an RC file exists, the keychain password loaded for settingsCfg was discarded when using rcCfg as the base for the merged config.

Fix: Added loadKeychainPassword(mergedCfg) call after creating the merged config to ensure the keychain password is properly loaded regardless of config source.

// Load keychain password for the merged config
// This is needed because settingsCfg had the password loaded but we used rcCfg as base
if err := loadKeychainPassword(mergedCfg); err != nil {
    // Non-fatal, just log and continue
    _ = err
}

2. Preserving Cwd and RestrictedRunners (config_handlers.go:329)

Issue: When saving ACP servers via the UI, the Cwd and RestrictedRunners fields were not being preserved from existing server configurations.

Fix: Added a lookup map of existing servers and now preserve these fields when building the new server settings:

// Build a map of existing server settings for preserving fields not exposed in UI
existingServers := make(map[string]configPkg.ACPServer)
if s.config.MittoConfig != nil {
    for _, srv := range s.config.MittoConfig.ACPServers {
        existingServers[srv.Name] = srv
    }
}

// ... then when building each server:

// Preserve Cwd and RestrictedRunners from existing server if present
// These fields are not exposed in the UI but should not be lost on save
if existing, ok := existingServers[srv.Name]; ok {
    newServer.Cwd = existing.Cwd
    newServer.RestrictedRunners = existing.RestrictedRunners
}

All CI checks are now passing ✅

[Copilot]

Pull request overview

Copilot reviewed 11 out of 13 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Filtering RC-file servers during save relies solely on the client-provided srv.Source. A client that omits/fakes this field could cause RC-file servers to be written into settings.json (duplicated) or hidden settings fields to be pulled from the wrong source. Consider also consulting the current in-memory config (e.g., if existingServers[srv.Name].Source == SourceRCFile then always skip) and treating an empty Source as SourceSettings explicitly.

Suggested change

	// Skip RC file servers - they are managed in .mittorc, not settings.json
	if srv.Source == configPkg.SourceRCFile {
	continue
	}
	// Determine whether this server should be persisted to settings.json.
	// RC file servers are managed in .mittorc and must never be written here.
	// First, consult the existing in-memory config as the authoritative source.
	if existing, ok := existingServers[srv.Name]; ok && existing.Source == configPkg.SourceRCFile {
	// Existing server is from RC file; always skip, regardless of client-provided Source.
	continue
	}
	// Then, consider the client-provided Source, normalizing an empty value to SourceSettings.
	effectiveSource := srv.Source
	if effectiveSource == "" {
	// Treat missing Source from the client as settings-sourced by default.
	effectiveSource = configPkg.SourceSettings
	}
	// If the (normalized) client source is RC file, skip as well.
	if effectiveSource == configPkg.SourceRCFile {
	continue
	}

[Copilot]

The LoadResult comment says RCFilePath is set even when Source is ConfigSourceSettingsJSON if an RC file was merged, but LoadSettingsWithFallback() currently returns Source: ConfigSourceRCFile whenever an RC file exists. Either adjust the comment or change the returned Source semantics so the metadata matches the behavior.

Suggested change

	// RCFilePath is the path to the RC file if one was used in the merge.
	// This is set even when Source is ConfigSourceSettingsJSON if an RC file was merged.
	// RCFilePath is the path to the RC file if one contributed to the configuration.
	// It is empty when no RC file was used.

[Copilot]

These branches say “Non-fatal, just log and continue” but the error is silently discarded (_ = err). Either log the error (if a logger is available in this package) or change the comment to reflect that errors are intentionally ignored, otherwise troubleshooting keychain issues will be difficult.

Suggested change

	_ = err
	fmt.Fprintf(os.Stderr, "warning: failed to load keychain password: %v\n", err)

[Copilot]

rcFilePath is populated only from configResult.RCFilePath. When running with --config, ConfigReadOnly is true but RCFilePath is empty (the path is in configResult.SourcePath), so the frontend cannot show the user which config file made settings read-only. Consider passing configResult.SourcePath (or a dedicated field) when configReadOnly is true.

Suggested change

	hasRCFileServers = configResult.HasRCFileServers
	hasRCFileServers = configResult.HasRCFileServers
	// When running with --config, RCFilePath may be empty and the actual
	// config file path is stored in SourcePath. Use it so the frontend
	// can display which config file made settings read-only.
	if rcFilePath == "" && configReadOnly {
	rcFilePath = configResult.SourcePath
	}