Repository navigation
fix(core): keep environment overrides out of config.json - #1701
Merged
Merged
Conversation
Every config save dumped the env-merged model, so a one-off BASIC_MEMORY_* variable became a permanent file setting. Saves now write the on-disk value (or omit the key) for every env-overridden field. `bm config set/unset`, `bm cloud api-key save/create`, and `bm cloud workspace set-default` still persist the key they name. Fixes #1631. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3324bc7a06
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Commands that change one named setting now pass it in persist_env_keys, so the change lands in config.json even while an env var overrides it: - bm cloud logout: default_workspace - bm cloud promo --on/--off: cloud_promo_opt_out - ConfigManager.set_default_project (bm project default): default_project Automatic writes (promo shown stamps, auto-update check, doctor's default repair, project list edits) stay env-respecting. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3573216bc4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
phernandez
added a commit
that referenced
this pull request
Oct 9, 2026
Entries for #1697, #1698, #1699, #1700, #1701, #1702, #1704 and #1705, which landed without changelog edits so the parallel PRs would not conflict. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
phernandez
added a commit
that referenced
this pull request
Oct 9, 2026
Entries for #1697, #1698, #1699, #1700, #1701, #1702, #1704 and #1705, which landed without changelog edits so the parallel PRs would not conflict. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
phernandez
added a commit
that referenced
this pull request
Oct 9, 2026
#1696 removed `semantic_search_enabled` from BasicMemoryConfig, but it was tested against a main that predated several milestone merges that still read the flag, so main failed static checks and most test jobs: - `index_project_and_report_readiness` (#1704) passed `embeddings=app_config.semantic_search_enabled`, which raised AttributeError on every `bm project add`. Semantic search is always on, so the add always runs the embedding pass. - test_db_reindex (#1704) parametrized the add on the flag; it now asserts the embedding pass runs. - test_deferred_embedding_resume (#1700) set the flag on its config. - test_config_env_overrides_not_persisted (#1701) used the flag as its example env-overridden boolean; it now uses `format_on_save`. Also makes #1704's cloud-prune test compare against str(Path) so it holds on Windows, where the path prints with backslashes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
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
1 participant
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.
Fixes #1631.
Why
load_configmergesBASIC_MEMORY_*environment values overconfig.json. Every config save then dumped that merged model. A variable set for one command became a permanent file setting. For example,BASIC_MEMORY_LOG_LEVEL=DEBUG bm project add ...leftlog_level: DEBUGin the file. A strayBASIC_MEMORY_PROJECT_ROOTkept refusing project names with/after the variable was unset (#1598).Change
env_overridden_fields(file_data)inconfig.pyis the one place that decides which fields come from the environment.load_configuses it to drop file values, and the save path uses it to keep env values out of the file. It also covers the legacyBASIC_MEMORY_SYNC_CHANGES/BASIC_MEMORY_SYNC_DELAYvariables, which map ontoindex_changes/index_delay.save_basic_memory_configwrites back, for each env-overridden field, the value the file had on disk. If the file never had the key, it omits it. A file that only had the legacysync_changes/sync_delaykey keeps that value under the new name. This covers every save, including the migration resave and the first-run save. If config.json on disk is corrupt JSON, the save now returns the error instead of overwriting the file.save_config/save_basic_memory_configtake a keyword-onlypersist_env_keys. Every command that explicitly changes one named setting passes it, so that key is persisted even while an env var overrides it:bm config set/bm config unset: the named key (the existing warning is kept)bm cloud api-key save/create:cloud_api_keybm cloud workspace set-defaultandbm cloud logout:default_workspacebm cloud promo --on/--off:cloud_promo_opt_outConfigManager.set_default_project(bm project default):default_projectBASIC_MEMORY_PROJECTSset, the env owns the project list, soproject adddoes not persist a change to it.Verification
tests/test_config_env_overrides_not_persisted.py:project addunder env overrides keeps the file values and adds no keys the file never had.BASIC_MEMORY_PROJECT_ROOTis not persisted, andproject_rootisNoneonce the variable is unset.persist_env_keyskey is written.sync_changesfile value survives a save.set_default_projectwritesdefault_projectunder an env override.tests/cli/test_config_command.py:config seton a different key leaves the overridden keys untouched,config seton the overridden key persists it, andconfig unsetwrites the default.tests/cli/test_cloud_promo.py:bm cloud promo --offagainst a real config file under env overrides persistscloud_promo_opt_outand keepslog_levelat its file value. The logout and promo stub tests assert the persisted key.just fast-checkpasses.just fast-testovertests/cli, the config tests, the project service, initialization and project router tests: 1376 passed. An earlier run overtests/mcp,tests/services,tests/indexand the permalink integration suites: 2524 passed.🤖 Generated with Claude Code
https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea