update settings fetch to prioritize config_path config AND secrets

[jtanningbed]

Fix for loading secrets alongside explicit config files

Problem

When an explicit config path is provided to get_settings(), the function looks for secrets using Settings.find_secrets(), which starts from the current working directory rather than the directory containing the config file. This causes issues when running tools from a different directory from where the config files are located.

Solution

This PR modifies get_settings() to prioritize looking for secrets in the same directory as an explicitly provided config file:

1. When an explicit config path is provided, check for secrets files in the same directory
2. Support both naming conventions (mcp-agent.secrets.yaml and mcp_agent.secrets.yaml)
3. Only fall back to the standard discovery method if no secrets are found alongside the config

Benefits

• Improves usability for tools and CLIs that might be run from any directory
• Allows proper separation of config and secrets files while maintaining their association
• Maintains backward compatibility with the current behavior
• Provides a more intuitive experience when explicit paths are used

Testing

Tested with a CLI tool that uses MCP agent and has its config/secrets stored in a subdirectory. Before this change, the tool needed to be run from the directory containing the config/secrets. After this change, it can be run from any directory.

Worth mentioning that a previous version had this behavior, but I don't have any context on whether this was a deliberate change:

>> previous impl
            # Load main config
            with open(config_file, "r", encoding="utf-8") as f:
                yaml_settings = yaml.safe_load(f) or {}
                merged_settings = yaml_settings

            # Look for secrets file in the same directory
            secrets_file = config_file.parent / "mcp_agent.secrets.yaml"
            if secrets_file.exists():
                with open(secrets_file, "r", encoding="utf-8") as f:
                    yaml_secrets = yaml.safe_load(f) or {}
                    merged_settings = deep_merge(merged_settings, yaml_secrets)

[saqadri]

@jtanningbed thank you for the catch! really great find. I have one suggestion for how to improve the function further. LMK what you think and I can merge in the change after that!

[saqadri]

The issue with the new approach is that in the case we find the config using Settings.find_config, we don't look for the secrets in the same directory as that, so we have the same issue again.

What I'd recommend is checking for the secrets first in the same directory where we find the config file in, before resorting to walking up the tree.

How about something like this?

def get_settings(config_path: str | None = None) -> Settings:
    """Get settings, loading config and secrets from an explicit path or discovered locations.
    
    First, load the config file and check its directory for a secrets file using
    both naming conventions. If no secrets file is found there, fall back to using the
    discovery method to locate a secrets file.
    """
    def deep_merge(base: dict, update: dict) -> dict:
        """Recursively merge two dictionaries."""
        merged = base.copy()
        for key, value in update.items():
            if key in merged and isinstance(merged[key], dict) and isinstance(value, dict):
                merged[key] = deep_merge(merged[key], value)
            else:
                merged[key] = value
        return merged

    global _settings
    if _settings:
        return _settings

    import yaml  # pylint: disable=C0415

    merged_settings = {}

    # Determine the config file to use.
    if config_path:
        config_file = Path(config_path)
    else:
        config_file = Settings.find_config()

    if config_file and config_file.exists():
        # Load the main config file.
        with open(config_file, "r", encoding="utf-8") as f:
            merged_settings = yaml.safe_load(f) or {}

        # Try to load secrets from the same directory as the config file.
        config_dir = config_file.parent
        secrets_found = False
        for secrets_filename in ["mcp-agent.secrets.yaml", "mcp_agent.secrets.yaml"]:
            secrets_file = config_dir / secrets_filename
            if secrets_file.exists():
                with open(secrets_file, "r", encoding="utf-8") as f:
                    secrets = yaml.safe_load(f) or {}
                merged_settings = deep_merge(merged_settings, secrets)
                secrets_found = True

        # Fallback: If no secrets were found in the config's directory, use the discovery method.
        if not secrets_found:
            discovered_secrets = Settings.find_secrets()
            if discovered_secrets and discovered_secrets.exists():
                with open(discovered_secrets, "r", encoding="utf-8") as f:
                    secrets = yaml.safe_load(f) or {}
                merged_settings = deep_merge(merged_settings, secrets)

        _settings = Settings(**merged_settings)
        return _settings

    # No valid config file was found; return default settings.
    _settings = Settings()
    return _settings

[jtanningbed]

ah right, sounds good to me. i'll make that update.

[saqadri]

thank you @jtanningbed!

[jtanningbed]

I've made the update to the improved control flow, just wanted to mention another change I added and double check this makes sense:

• if config_path is specified (ie. if someone init MCPApp(settings='/path/to/config.yaml')), if the path doesn't resolve to a config file, we should NOT perform discovery. Instead, we fail immediately (this is to prevent obfuscating issues w/ configurations for users who may have many config files at different layers).

Basically, if someone tries to explicitly specify a path that doesn't resolve and we were to find_config() and successfully discover a config file via this strategy, there is a chance the config is not the one the user wants and their app runs successfully. but if they update their target config file and the app doesn't appear to use the new changes it could be a nightmare to try and debug why everything 'seems to work' but 'for some reason' their config changes aren't applying.

With the additional change - if the path doesn't resolve to a real file, we fail fast like so:
Config file not found: /Users/me/testpath.yaml
Error: Config file not found: /Users/me/testpath.yaml

[jtanningbed]

and -- if config_path not specified, we still do discovery and allow init for the default Settings() if no config file found. i'm unaware of whether there are working scenarios when there is no valid config yaml file found, so I left that part alone.

[saqadri]

I think this is perfectly reasonable, thank you!

[saqadri]

Thanks @jtanningbed, change is looking good. Just 2 more comments. Approving since we're almost there, but would love to get those updated before we merge in.

[jtanningbed]

Thanks @jtanningbed, change is looking good. Just 2 more comments. Approving since we're almost there, but would love to get those updated before we merge in.

done thanks!