Skip to content

fix(memory): pass through unknown config keys in _wrap_configuration#322

Merged
tejaskash merged 1 commit into
mainfrom
fix/wrap-config-pass-through-unknown-keys
Mar 11, 2026
Merged

fix(memory): pass through unknown config keys in _wrap_configuration#322
tejaskash merged 1 commit into
mainfrom
fix/wrap-config-pass-through-unknown-keys

Conversation

@jariy17

@jariy17 jariy17 commented Mar 11, 2026

Copy link
Copy Markdown
Contributor

Summary

_wrap_configuration only copies extraction, consolidation, and reflection keys into the wrapped config dict. Any other top-level key — like selfManagedConfiguration — is silently dropped. This makes it impossible to modify SELF_MANAGED strategies through modify_strategy() or update_memory_strategies().

Root Cause

_wrap_configuration builds a new wrapped_config dict and only populates it with keys it explicitly handles (extraction, consolidation, reflection). It never copies unknown keys, so they disappear before reaching the API.

Fix

Add a pass-through loop at the end of _wrap_configuration that copies any config keys not already in wrapped_config. This preserves selfManagedConfiguration (and any future unknown config keys) so they reach the API intact.

for key in config:
    if key not in wrapped_config:
        wrapped_config[key] = config[key]

Why this approach

  • Runs per strategy_wrap_configuration is called once per strategy in the modify loop, so there's no cross-contamination between strategies
  • No behavior change for existing override types — extraction/consolidation/reflection are already in wrapped_config by the time the loop runs, so they're skipped
  • Fails loud — if an unknown key is invalid, the API rejects it with a clear error. The alternative (Method 3: hardcoding selfManagedConfiguration) fails silent — future unknown keys get dropped with no error

Example

# This currently sends {} to the API (config silently dropped)
# After this fix, selfManagedConfiguration reaches the API intact
client.modify_strategy(
    memory_id="my-memory",
    strategy_id="self_managed_xyz",
    configuration={
        "selfManagedConfiguration": {
            "triggerConditions": [
                {"messageBasedTrigger": {"messageCount": 5}}
            ],
            "invocationConfiguration": {
                "topicArn": "arn:aws:sns:us-west-2:123:my-topic",
                "payloadDeliveryBucketName": "my-bucket"
            },
            "historicalContextWindowSize": 10
        }
    }
)

Test plan

  • Existing _wrap_configuration tests pass (known override types still wrap correctly)
  • New test: SELF_MANAGED with selfManagedConfiguration key preserved in output
  • New test: known override type config unchanged (loop skips already-handled keys)

Relates to #212

_wrap_configuration only copies extraction, consolidation, and reflection
keys into the wrapped config dict. Any other top-level keys like
selfManagedConfiguration are silently dropped, making it impossible to
modify SELF_MANAGED strategies through modify_strategy or
update_memory_strategies.

Add a pass-through loop at the end of _wrap_configuration that copies
any config keys not already handled by the wrapping logic. This allows
selfManagedConfiguration (and any future unknown config keys) to reach
the API intact.

Relates to #212
@jariy17 jariy17 requested a review from a team March 11, 2026 20:02

@tejaskash tejaskash left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tejaskash tejaskash merged commit 2afa155 into main Mar 11, 2026
21 of 22 checks passed
@jariy17 jariy17 deleted the fix/wrap-config-pass-through-unknown-keys branch March 11, 2026 20:39
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.

2 participants