Skip to content

fix(config): accept .yml profiles to match setup.sh selector#99

Open
0xghost42 wants to merge 1 commit into
sentient-agi:mainfrom
0xghost42:fix/profile-yml-extension
Open

fix(config): accept .yml profiles to match setup.sh selector#99
0xghost42 wants to merge 1 commit into
sentient-agi:mainfrom
0xghost42:fix/profile-yml-extension

Conversation

@0xghost42

Copy link
Copy Markdown

Summary

setup.sh's discover_profiles() already accepts both .yaml and .yml files when listing the profile selector at startup (setup.sh:140), but ConfigManager._load_profile / get_available_profiles only looked at .yaml. A user that saved a profile as foo.yml sees it in the setup selector then hits the following at load time:

ValueError: Profile 'foo' not found.

because the loader checked exactly <config_dir>/profiles/foo.yaml and skipped the .yml twin. The selector and the loader disagreed on the extension contract, which is the kind of mismatch that turns into "it shows up in just setup but not via pnpm openclaw-style invocations" silently.

Fix

Wire both extensions through the loader so the selector and load paths agree on one set:

  • _load_profile: try .yaml first (existing behaviour), then .yml.
  • get_available_profiles: rglob both *.yaml and *.yml and deduplicate via a set before sorting.

When both forms exist for the same profile name, .yaml wins so the selector and loader pick a single deterministic source.

Real behavior proof

  • Behavior addressed: setup.sh's discover_profiles() lists .yml profiles, but ConfigManager._load_profile() and get_available_profiles() only enumerate .yaml, so a profile saved as foo.yml (a) shows up in the just setup selector, (b) loads from setup.sh's own preview path, then (c) crashes at runtime with ValueError: Profile 'foo' not found. once the ConfigManager re-resolves it. The selector / loader contract was inconsistent.
  • Real environment tested: local dev shell on darwin/arm64 against the patched 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 under tempfile.TemporaryDirectory().
  • Exact steps or command run after this patch: regression coverage in tests/test_config.py exercises three real ConfigManager(config_dir=…) instances against on-disk profile trees and asserts the selector / loader agree.
  • Evidence after fix: the new test_load_profile_with_yml_extension asserts that a profile saved exclusively as foo.yml resolves through load_config(profile="foo") and yields the expected agents.executor.llm.temperature; test_get_available_profiles_includes_both_extensions asserts the dual-extension selector — including a nested nested/gamma.yml — appears in get_available_profiles(); test_load_profile_prefers_yaml_over_yml asserts that when both dup.yaml and dup.yml exist, the .yaml value wins, so the precedence is deterministic.
  • Observed result after fix: .yml-only profiles now resolve from ConfigManager.load_config(profile=…) (matching the setup-script selector), the available-profiles listing returns both extensions deduplicated, and .yaml wins when both forms exist for the same name. Before this patch, the .yml-only call raised ValueError: Profile 'foo' not found. Available profiles: […].
  • What was not tested: a live just setup end-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). The tests/ coverage exercises the actual ConfigManager against on-disk profile files, which is the surface that was broken.

Notes

Pure additive / discovery-side fix. Profiles already on .yaml are unchanged; the extra .yml lookup only fires when the .yaml path is absent.

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.
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.

1 participant