config: env wins over config.json + config.d/ fragment layer (fixes #248)#249
Merged
config: env wins over config.json + config.d/ fragment layer (fixes #248)#249
Conversation
Previously, ``load_config_overrides`` unconditionally ``setattr``'d every field in ``~/.memtomem/config.json`` on top of the env-var-bound config, which meant ``MEMTOMEM_*`` env vars set in ``.mcp.json`` / MCP client ``env`` blocks were silently ignored for any field the ``mm init`` wizard had persisted. Every integration doc in the repo (cloud-sync, getting-started, mcp-clients, integrations/*) assumes env vars override file values; the code did the opposite. Invert the precedence: before applying a ``config.json`` key, check whether ``MEMTOMEM_<SECTION>__<FIELD>`` is set in the environment and skip the override if so. Fields without an env var still load from ``config.json`` as before. New tests in ``test_config_overrides.py`` pin the expected precedence, including an explicit regression case for the ``.mcp.json`` env block shipped in #247 (one scalar ``SQLITE_PATH``, one list ``MEMORY_DIRS``) — both must take effect post-``mm init``.
Annotate every ``list[*]`` field in config.py with an explicit merge strategy via ``typing.Annotated`` so future fragment-based loaders (``~/.memtomem/config.d/``) can pick append vs replace semantics per field instead of inferring from the type. Keeps the declaration co-located with the field and makes the strategy part of the type annotation rather than a separate registry. - ``APPEND`` (4 fields): memory_dirs, exclude_patterns, system_namespace_prefixes, webhook.events — elements are independent, duplicates dedup on merge. - ``REPLACE`` (2 fields): rrf_weights, importance.weights — positional tuning knobs where element order/length is semantic; highest-priority source wins entirely. No runtime behaviour change yet; loader landing in a follow-up commit will read the ``MergeStrategy`` via ``FieldInfo.metadata``.
New ``load_config_d`` scans ``~/.memtomem/config.d/*.json`` in
lexicographic filename order and merges each fragment into the running
config. Intended for integration-installed drop-ins: ``mm init
<client>`` writes one fragment, ``mm uninstall <client>`` removes it.
Idempotent, no mutation of shared state.
Merge rules per field (strategy from ``MergeStrategy`` annotation):
- APPEND list → concatenate with current list, dedup by value
(``Path`` normalised to string form for comparison).
- REPLACE list → fragment value replaces whatever was there.
- Scalar → last fragment applied wins.
- ``MEMTOMEM_<SECTION>__<FIELD>`` env var set → fragment value for that
field is skipped (env wins, same rule as config.json).
Layering in ``create_components`` (lowest → highest priority):
defaults → config.d/*.json → config.json → env. ``config.json`` keeps
its current REPLACE-on-set semantics so the ``mm init`` wizard UX
("set my memory dir to X") is unambiguous; fragments are the layer
that respects per-field merge strategies.
Invalid fragments (malformed JSON, non-object top level, unknown
sections) are logged and skipped without crashing startup, matching the
existing ``config.json`` resilience.
Adds ``test_every_list_field_declares_merge_strategy`` that walks every ``Mem2MemConfig`` section and fails if a ``list[*]`` field lacks a ``MergeStrategy`` annotation in its metadata. The failure message lists the offending fields and tells the contributor to wrap the type in ``Annotated[list[X], APPEND]`` or ``Annotated[list[X], REPLACE]``. Guards against silent fragment-merge footguns: without this, someone adding a new positional ``list[float]`` tuning knob without declaring REPLACE would let a ``config.d/`` fragment accidentally append to it via the default-to-REPLACE fallback.
Adds a "Precedence and merge behaviour" section to ``docs/guides/configuration.md`` covering the full resolution order (defaults → ``config.d/*.json`` → ``config.json`` → env vars) and a table of which ``list[*]`` fields use APPEND vs REPLACE under the fragment layer. Also amends the auto-discovered-memory-dirs note to mention the new ``config.d/`` layer alongside ``config.json`` so it stays accurate. Other integration docs (``mcp-clients.md``, ``cloud-sync.md``, ``integrations/*``) already advertised the env-wins semantic that this branch just made true in the code, so no wording change is needed there — the advertised examples now work as documented. (Commit also folds in ``ruff format`` over ``config.py`` and the new test file, a no-op reflow of the lines touched in earlier commits.)
…/ embedding-reset) ``mm config show`` and the ``reset`` / ``embedding-reset`` diagnostics only called ``load_config_overrides`` before the new fragment layer landed, so ``config.d/*.json`` contributions (e.g. an integration's ``exclude_patterns`` APPEND) were invisible to these commands even though the running server sees them. Add ``load_config_d`` before ``load_config_overrides`` in all three read-path call sites so the CLI tells the truth about what the runtime resolves. ``config_set`` is intentionally left alone — ``save_config_overrides`` uses read-merge-write over ``MUTABLE_FIELDS``, so pulling fragment values into the runtime before save would risk baking fragment state into ``config.json`` (where it would persist past the fragment file's removal). Fixing that is a separate refactor of ``save_config_overrides`` to track field provenance; out of scope here.
Owner
Author
Manual smoke — PASS3 scenarios with S1 — no env, both layers:
S2 —
S3 — config.json deleted, fragment only:
Followup commit Test-plan checkbox for manual smoke now checked. |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Closes #248.
Summary
Resolves the precedence bug where
~/.memtomem/config.jsonsilentlyoverrode
MEMTOMEM_*env vars set in.mcp.json/ MCP clientenvblocks — i.e. every example our docs ship. Also adds the
config.d/drop-in layer discussed in #248 so integration installers can drop a
fragment without mutating shared state.
Five commits, each independently reviewable:
fix(config): env vars win over ~/.memtomem/config.json—load_config_overridesnow checks whether the matchingMEMTOMEM_<SECTION>__<FIELD>env var is set and skips the override if so. Regression test pins PR docs(mcp): document Claude Code install scopes (local/project/user) #247's exact.mcp.jsonenv block (1 scalarSQLITE_PATH+ 1 listMEMORY_DIRS) — both must survive a fullmm init+config.json.feat(config): add MergeStrategy annotations to list fields— co-locates the merge strategy with the field viaAnnotated[list[X], APPEND | REPLACE]. 4 APPEND (memory_dirs, exclude_patterns, system_namespace_prefixes, webhook.events), 2 REPLACE (rrf_weights, importance.weights) — REPLACE on positional tuning knobs where appending would misalign element slots. No runtime change on its own.feat(config): add ~/.memtomem/config.d/ fragment loader— scansconfig.d/*.jsonin lex order, respects per-field strategy, skips fields where env is set.config.jsonremains the REPLACE-on-set user-override layer (mm initUX unchanged).test(config): enforce merge strategy declaration on all list fields— walks everyMem2MemConfigsection and fails if alist[*]lacks aMergeStrategyin its metadata. Guards against silent "default-to-REPLACE" drift when new fields get added.docs(config): document precedence and list merge strategies— adds a "Precedence and merge behaviour" section todocs/guides/configuration.mdcovering the full resolution order and the APPEND/REPLACE table. Other integration docs already advertised the env-wins semantic, so they now work as written.Resolution order (lowest → highest priority)
Testing
test_config_overrides.pycovering Phase 1 precedence, Phase 2b merge semantics (APPEND dedup, REPLACE last-wins, scalar last-wins, env-wins-over-fragments, unknown section warn, malformed JSON warn, non-JSON files ignored), and the enforcement test.ruff checkandruff format --checkboth clean.mypyclean on the touched files.Release notes
Test plan
.mcp.jsonenv blockconfig.d/test.json+ overlappingconfig.json+ env, confirm resolution matches the documented order end-to-end