Skip to content

fix(merge_utils): normalize string-shorthand entries in merge_module_lists#169

Merged
bkrabach merged 1 commit intomainfrom
fix/merge-module-lists-string-shorthand
Apr 29, 2026
Merged

fix(merge_utils): normalize string-shorthand entries in merge_module_lists#169
bkrabach merged 1 commit intomainfrom
fix/merge-module-lists-string-shorthand

Conversation

@bkrabach
Copy link
Copy Markdown
Collaborator

Summary

Fixes AttributeError: 'str' object has no attribute 'get' raised at
merge_utils.py:62 (and symmetrically in the base loop) when a
hooks / tools / providers list contains string-shorthand entries
instead of dict-shape entries.

Root cause

String-shorthand is intentional — agent .md files declare module
dependencies using a bare YAML list of strings, e.g.:

# reality-check/agents/terminal-tester.md, line 38
tools: [terminal_inspector]

rather than the full dict form:

tools:
  - module: terminal_inspector

The YAML parser returns ["terminal_inspector"] (a list[str]). When
this agent config is overlaid onto the parent session config,
merge_agent_dicts routes the tools key to merge_module_lists,
which unconditionally called .get() on each list element — crashing
immediately for strings.

This pattern is used across multiple bundles:

  • reality-check/agents/terminal-tester.mdtools: [terminal_inspector]
  • terminal-tester/agents/terminal-operator.mdtools: [terminal_inspector]
  • terminal-tester/agents/terminal-visual-tester.mdtools: [terminal_inspector]
  • terminal-tester/agents/terminal-debugger.mdtools: [terminal_inspector]

The fix belongs in merge_module_lists: normalize at the merge boundary,
not in every caller or every YAML author. No upstream caller change is
needed because all authors are writing intentional shorthand, not bugs.

Fix

Added _normalize_module_entry() helper that:

  • Converts str{key_field: <string>} (e.g. "bash"{"module": "bash"})
  • Passes dict through unchanged
  • Returns None for anything else (silently dropped, rather than raising)

merge_module_lists now applies this normalization to both base
and overlay before any .get() calls, via a single comprehension pass.
The type signature is widened from list[dict[str, Any]] to list[Any]
on both parameters (return type remains list[dict[str, Any]]).

Why this is hard to find

The exception is caught at _execute_recipe's broad except Exception as e:
in amplifier-bundle-recipes/__init__.py:464, which converts it to a string
without the traceback. Resolving this required reproducing in a manual
interactive amplifier tool invoke recipes invocation with the outer except
patched to print traceback.format_exc(), in order to find the actual line
in merge_utils.py:62.

PR microsoft/amplifier-bundle-recipes#71 (isinstance(agent_cfg, dict)
guards) addressed a similar class of bug but in executor.py:1785/1803
two-and-a-half stack frames upstream of this one. The defense-in-depth
pattern needs to be applied throughout the merge chain.

Test

Adds TestMergeModuleListsStringShorthand with 13 cases covering:

Test Scenario
test_string_overlay_single_entry overlay=["bash"][{"module": "bash"}]
test_string_overlay_multiple_entries Multiple strings normalized
test_string_overlay_merges_into_existing_base String normalised, merges into matching dict base
test_string_overlay_appends_new_entry String not in base is appended
test_mixed_overlay_strings_and_dicts Mixed overlay handled correctly
test_string_base_single_entry base=["bash"][{"module": "bash"}]
test_string_base_multiple_entries Multiple string base entries
test_mixed_base_strings_and_dicts Mixed base handled correctly
test_string_base_merged_with_dict_overlay String base + dict overlay deep-merges
test_real_world_terminal_inspector Exact regression: ["terminal_inspector"]
test_key_field_respected_for_string_normalization key_field param propagates
test_empty_lists Empty lists → no crash
test_garbage_entries_silently_dropped None, int dropped gracefully

Upstream fix

No upstream fix was needed — the strings come from intentional YAML authoring
conventions, not from a serialization bug. The normalization at the merge
boundary is the correct and complete fix.

Discovered while

Running the amplifier-bundle-reality-check recipe inside an Amplifier-Resolve
reality-check runner sub-container. Step 2 (terminal-validation) consistently
failed in ~11ms because spawning the reality-check:terminal-tester agent
(which declares tools: [terminal_inspector]) crashed in merge_module_lists
before the agent session could start.

🤖 Generated with Amplifier

…lists

When a hooks/tools/providers list contains string-shorthand entries
(e.g. tools: ["bash"] instead of tools: [{"module": "bash"}]),
merge_module_lists previously crashed with AttributeError: 'str' object
has no attribute 'get' because it called .get() directly on each list
element.

Add _normalize_module_entry() helper that converts strings to
{key_field: <string>} dict-form before processing. Apply to both base
and overlay loops for symmetry. Non-string, non-dict entries (None, int,
etc.) are silently dropped rather than raising.

Discovered during reality-check capability E2E (Amplifier Resolve
platform) where step 2 (terminal-validation) consistently failed in
~11ms with no agent fork. Path C reproduction with traceback capture
revealed the actual exception line (merge_utils.py:62) and the call
path (executor.py -> session_spawner -> merge_configs -> merge_agent_dicts
-> merge_module_lists).

Root cause: reality-check/agents/terminal-tester.md declares
tools: [terminal_inspector] as bare-string YAML shorthand (intentional
author convention used across multiple bundles). The YAML parser returns
["terminal_inspector"], which merge_module_lists then iterates without
normalization.
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