fix(config): prevent phantom ~/basic-memory directory on load_config#1030
fix(config): prevent phantom ~/basic-memory directory on load_config#1030sanjibani wants to merge 2 commits into
Conversation
The basicmemory.com docs site restructured: /getting-started/note-formatting and /guides/cli-reference both 404, replaced with the canonical /concepts/knowledge-format and /reference/cli-reference paths (the old /guides prefix was renamed to /reference). The basicmemory.com/subscribe landing page was retired and now redirects to /pricing. Verified via curl: GET on the new URLs returns 200. Signed-off-by: sanjibani <18418553+sanjibani@users.noreply.github.com>
The env-default probe in ConfigManager.load_config instantiates a throwaway BasicMemoryConfig to dump env-derived field values. model_post_init seeds a default 'main' project at ~/basic-memory (when BASIC_MEMORY_HOME is unset), and ensure_project_paths_exists then mkdirs it. That writes to disk against a path the user never selected. Fix: pass skip_initialization_sync=True on the throwaway. Pydantic Settings still reads env vars as field defaults, so env_dict reflects env-derived values. model_post_init and ensure_project_paths_exists both gate on skip_local_initialization and now stay no-ops. Regression test asserts load_config does not recreate ~/basic-memory when a config file already specifies the user's chosen project path. Fixes basicmachines-co#1029 Signed-off-by: sanjibani <18418553+sanjibani@users.noreply.github.com>
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39a66bee8d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # ensure_project_paths_exists won't mkdir. Pydantic Settings still | ||
| # reads env vars as field defaults, so env_dict reflects env-derived | ||
| # values without writing to the filesystem. See GH#1029. | ||
| env_config = BasicMemoryConfig(skip_initialization_sync=True) |
There was a problem hiding this comment.
Preserve false skip-initialization env overrides
When a config file exists and the process sets BASIC_MEMORY_SKIP_INITIALIZATION_SYNC=false, this probe is built with the init kwarg forced to True. Pydantic-settings init kwargs take precedence over env sources, and the merge loop below copies env_dict["skip_initialization_sync"] whenever that env var is present, so an explicit false override is turned into true. That makes local runs report skip_local_initialization and skip project seeding/path creation plus initialize_app() database/reconcile work.
Useful? React with 👍 / 👎.
|
recheck |
|
Thanks for the contribution. The CLA check is still pending, so we cannot merge this PR yet. Please complete or re-authorize the contributor agreement so the check turns green. Once it is green, maintainers can take over any remaining implementation changes needed to get this merged. |
|
Thanks for reporting and working on this. Because the CLA remains unsigned, we cannot merge or incorporate this contribution. We independently reimplemented the issue from the public bug report in maintainer PR #1080, so this PR is superseded. |
Closes #1029.
ConfigManager.load_config instantiates a throwaway BasicMemoryConfig to dump env-derived field defaults. With BASIC_MEMORY_HOME unset, model_post_init seeds a default 'main' project at ~/basic-memory and ensure_project_paths_exists mkdirs it, recreating the phantom directory on every load.
Fix: pass skip_initialization_sync=True on the throwaway. Pydantic Settings still reads env vars as field defaults, so env_dict reflects env-derived values without filesystem side effects. model_post_init and ensure_project_paths_exists both gate on skip_local_initialization and now stay no-ops for the probe.
Regression test asserts load_config does not recreate ~/basic-memory when the config file already specifies the user's chosen project path. Verified the test fails before the fix and passes after.