fix(config): accept .yml profiles to match setup.sh selector#99
Open
0xghost42 wants to merge 1 commit into
Open
fix(config): accept .yml profiles to match setup.sh selector#990xghost42 wants to merge 1 commit into
0xghost42 wants to merge 1 commit into
Conversation
setup.sh's discover_profiles() already accepts both .yaml and .yml
files when listing the profile selector at startup, but
ConfigManager._load_profile / get_available_profiles only looked at
.yaml. A user that saved a profile as foo.yml would see it in the
selector then hit:
ValueError: Profile 'foo' not found.
at load time, because the loader checked exactly
'$config_dir/profiles/foo.yaml' and skipped the .yml twin.
Wire both extensions through the loader:
- _load_profile: try .yaml first (existing behaviour), then .yml.
- get_available_profiles: rglob both *.yaml and *.yml and deduplicate.
When both forms exist for the same name, .yaml wins so the selector and
loader agree on a single deterministic source. Tests in
tests/test_config.py cover the new yml-only path, the dual-extension
selector listing, and the .yaml-wins precedence.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
setup.sh'sdiscover_profiles()already accepts both.yamland.ymlfiles when listing the profile selector at startup (setup.sh:140), butConfigManager._load_profile/get_available_profilesonly looked at.yaml. A user that saved a profile asfoo.ymlsees it in the setup selector then hits the following at load time:because the loader checked exactly
<config_dir>/profiles/foo.yamland skipped the.ymltwin. The selector and the loader disagreed on the extension contract, which is the kind of mismatch that turns into "it shows up injust setupbut not viapnpm openclaw-style invocations" silently.Fix
Wire both extensions through the loader so the selector and load paths agree on one set:
_load_profile: try.yamlfirst (existing behaviour), then.yml.get_available_profiles:rglobboth*.yamland*.ymland deduplicate via a set before sorting.When both forms exist for the same profile name,
.yamlwins so the selector and loader pick a single deterministic source.Real behavior proof
setup.sh'sdiscover_profiles()lists.ymlprofiles, butConfigManager._load_profile()andget_available_profiles()only enumerate.yaml, so a profile saved asfoo.yml(a) shows up in thejust setupselector, (b) loads fromsetup.sh's own preview path, then (c) crashes at runtime withValueError: Profile 'foo' not found.once theConfigManagerre-resolves it. The selector / loader contract was inconsistent.ConfigManager, using real temp<config_dir>/profiles/trees on disk (no mocked filesystem). The selector enumeration and the load-time resolution were exercised against actual files written undertempfile.TemporaryDirectory().tests/test_config.pyexercises three realConfigManager(config_dir=…)instances against on-disk profile trees and asserts the selector / loader agree.test_load_profile_with_yml_extensionasserts that a profile saved exclusively asfoo.ymlresolves throughload_config(profile="foo")and yields the expectedagents.executor.llm.temperature;test_get_available_profiles_includes_both_extensionsasserts the dual-extension selector — including a nestednested/gamma.yml— appears inget_available_profiles();test_load_profile_prefers_yaml_over_ymlasserts that when bothdup.yamlanddup.ymlexist, the.yamlvalue wins, so the precedence is deterministic..yml-only profiles now resolve fromConfigManager.load_config(profile=…)(matching the setup-script selector), the available-profiles listing returns both extensions deduplicated, and.yamlwins when both forms exist for the same name. Before this patch, the.yml-only call raisedValueError: Profile 'foo' not found. Available profiles: […].just setupend-to-end pass against a real ROMA install in this contributor's local environment (would require the full bash setup pipeline + an installed config tree). Thetests/coverage exercises the actualConfigManageragainst on-disk profile files, which is the surface that was broken.Notes
Pure additive / discovery-side fix. Profiles already on
.yamlare unchanged; the extra.ymllookup only fires when the.yamlpath is absent.