fix(openmemory): fix settings page not saving — 3 UI/API bugs#4272
fix(openmemory): fix settings page not saving — 3 UI/API bugs#4272iamvaleriofantozzi wants to merge 2 commits intomem0ai:mainfrom
Conversation
…s unset
Two related bugs caused the OpenMemory settings page to always show
hardcoded OpenAI defaults instead of the real saved configuration.
Bug 1 — docker-compose.yml:
NEXT_PUBLIC_API_URL was passed as ${NEXT_PUBLIC_API_URL} with no
fallback. If the variable was unset in the host shell, the container
received an empty string. entrypoint.sh then replaced the
NEXT_PUBLIC_API_URL placeholder baked into the Next.js build with "",
causing all API calls to hit relative paths on port 3000 (the UI
itself) instead of the API on port 8765.
Fix: use shell default-value syntax ${NEXT_PUBLIC_API_URL:-http://localhost:8765}.
If the variable is set by the user (e.g. for a remote deploy), it is
respected; if unset or empty, the local default is used.
Bug 2 — useConfig.ts:
fetchConfig (GET) and saveConfig (PUT) called /api/v1/config without a
trailing slash. FastAPI's default redirect_slashes=True responds with a
307 redirect to /api/v1/config/. This caused the Redux store to stay on
its initial hardcoded OpenAI state, so the settings page never
reflected the actual persisted configuration.
Fix: align both axios calls with the FastAPI route definition
(@router.get("/") and @router.put("/") under prefix /api/v1/config),
which resolves to /api/v1/config/.
…uite
Three bugs fixed in this commit:
1. PUT /api/v1/config/ handler (update_configuration) was missing
save_config_to_db(), reset_memory_client() and return statement.
The handler updated the in-memory dict but never persisted it and
returned None, causing a ResponseValidationError on every save.
2. Added openmemory/api/tests/test_config_api.py with 10 tests covering:
- Bug 2 (trailing slash): 307 redirect on GET/PUT without slash,
200 OK on GET/PUT with slash, correct response shape
- Bug 1 (NEXT_PUBLIC_API_URL): API reachability, no-auth contract,
Redux store shape contract
Test setup uses StaticPool so all SQLAlchemy connections share one
in-memory SQLite database, with dependency_overrides for get_db.
All 10 tests pass (verified inside Docker container).
|
Valerio Fantozzi seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
CLA signed! |
There was a problem hiding this comment.
Pull request overview
Fixes the OpenMemory settings save flow by aligning UI endpoints with FastAPI’s trailing-slash behavior, ensuring the UI has a sane default API base URL in Docker, and completing the config update handler so updates persist and return a valid response.
Changes:
- Add a default fallback for
NEXT_PUBLIC_API_URLindocker-compose.ymlso the UI points at the API by default. - Update UI config GET/PUT calls to use
/api/v1/config/to avoid FastAPI’s redirect behavior. - Fix
PUT /api/v1/config/to persist changes, reset the in-memory client, and return the updated config. - Add a FastAPI test suite covering trailing-slash redirects and basic config endpoint contract.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| openmemory/ui/hooks/useConfig.ts | Updates config GET/PUT URLs to include trailing slash. |
| openmemory/docker-compose.yml | Adds a default value for NEXT_PUBLIC_API_URL when unset. |
| openmemory/api/app/routers/config.py | Completes PUT /api/v1/config/ by saving, resetting mem0 client, and returning updated config. |
| openmemory/api/tests/test_config_api.py | Adds tests documenting redirect behavior and validating config endpoint responses. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Update openmemory settings if provided | ||
| if config.openmemory is not None: | ||
| if "openmemory" not in updated_config: | ||
| updated_config["openmemory"] = {} | ||
| updated_config["openmemory"].update(config.openmemory.dict(exclude_none=True)) | ||
|
|
||
| # Update mem0 settings | ||
| updated_config["mem0"] = config.mem0.dict(exclude_none=True) | ||
|
|
||
|
|
||
| save_config_to_db(db, updated_config) | ||
| reset_memory_client() | ||
| return updated_config |
There was a problem hiding this comment.
update_configuration overwrites the entire mem0 section with config.mem0.dict(...) and also assumes config.mem0 is non-null. This can (a) raise an AttributeError if the request omits mem0 and (b) silently drop existing sub-keys like vector_store (or embedder) if the UI doesn’t send them, causing unintended config loss. Consider validating mem0 as required for PUT, or merging into the existing updated_config["mem0"] (preserving unknown keys) and only updating fields provided.
| const saveConfig = async (config: { openmemory?: OpenMemoryConfig; mem0: Mem0Config }) => { | ||
| setIsLoading(true); | ||
| setError(null); | ||
|
|
||
| try { | ||
| const response = await axios.put(`${URL}/api/v1/config`, config); | ||
| const response = await axios.put(`${URL}/api/v1/config/`, config); | ||
| dispatch(setConfigSuccess(response.data)); | ||
| setIsLoading(false); | ||
| return response.data; |
There was a problem hiding this comment.
UseConfigApiReturn declares saveConfig as returning Promise<void>, but the implementation returns response.data. This mismatch makes the hook’s API misleading for callers and can hide typing issues. Update the interface (and/or the function) so the return type matches the actual returned config payload.
| """ | ||
| Tests for the two bugs fixed in fix/ui-config-api-url. | ||
|
|
There was a problem hiding this comment.
The module docstring says this file tests “two bugs”, but this PR fixes (and this file exercises) three issues (including the PUT /api/v1/config/ handler behavior). Updating the description will avoid confusion about the test’s intent/coverage.
| def test_put_config_with_slash_returns_200(self, client_follow): | ||
| """Fix: PUT /api/v1/config/ (with slash) → 200 OK.""" | ||
| payload = { | ||
| "mem0": { | ||
| "llm": { | ||
| "provider": "openai", | ||
| "config": {"model": "gpt-4o-mini", "temperature": 0.1, "max_tokens": 2000}, | ||
| }, | ||
| "embedder": { | ||
| "provider": "openai", | ||
| "config": {"model": "text-embedding-3-small"}, | ||
| }, | ||
| } | ||
| } | ||
| response = client_follow.put("/api/v1/config/", json=payload) | ||
| assert response.status_code == 200, f"Expected 200 for PUT /api/v1/config/, got {response.status_code}" | ||
|
|
There was a problem hiding this comment.
The test_put_config_with_slash_returns_200 assertion only checks the status code. Since the bug fix includes persisting the updated config and returning the updated payload, it would be stronger to also assert on the response body (e.g., returned mem0 values match the payload) and then GET /api/v1/config/ to confirm the update was actually saved to the DB.
alvinttang
left a comment
There was a problem hiding this comment.
The three-part fix is solid — the missing save_config_to_db + reset_memory_client() call was the real data-loss bug, and the trailing-slash fix in useConfig.ts is the correct remedy for the 307 redirect (rather than disabling FastAPI's redirect_slashes). The docker-compose default ${NEXT_PUBLIC_API_URL:-http://localhost:8765} is a good DX improvement, though it silently succeeds even when the API container is on a different host/port — a startup health-check or warning log might save future confusion. The test suite is unusually thorough for a UI bug fix, and the StaticPool trick to share a single in-memory SQLite connection across the app and test sessions is the right approach. One nit: the PUT handler now returns updated_config directly, but it's worth confirming the PATCH handler below also calls reset_memory_client() on success, otherwise the client state can still diverge after a partial update.
Summary
The OpenMemory settings page had three compounding bugs that made it impossible to save configuration changes. Each bug independently breaks the save flow; together they made the settings page completely non-functional.
Bug 1 —
docker-compose.yml: missing default forNEXT_PUBLIC_API_URLFile:
openmemory/docker-compose.ymlRoot cause: The environment variable was passed with no fallback:
- NEXT_PUBLIC_API_URL=${NEXT_PUBLIC_API_URL}If
NEXT_PUBLIC_API_URLis unset in the host shell (the common case for new installs), the container receives an empty string.entrypoint.shthen replaces the placeholder with"", so every API call from the UI targets port 3000 (the UI itself) instead of port 8765 (the API).Fix: Add the standard shell fallback syntax:
- NEXT_PUBLIC_API_URL=${NEXT_PUBLIC_API_URL:-http://localhost:8765}Bug 2 —
useConfig.ts: missing trailing slash causes 307 redirectFile:
openmemory/ui/hooks/useConfig.tsRoot cause:
fetchConfig(GET) andsaveConfig(PUT) called/api/v1/configwithout a trailing slash. FastAPI's defaultredirect_slashes=Trueresponds with a307 Temporary Redirect. Axios follows the redirect but the Reduxdispatch(setConfigSuccess(...))path is never reached for the original call, so the store stays on its hardcoded OpenAI initial state.Fix: Add trailing slash to both calls →
/api/v1/config/Bug 3 —
config.py:PUT /api/v1/config/missing save, reset and returnFile:
openmemory/api/app/routers/config.pyRoot cause: The
update_configurationhandler updatedupdated_configin memory but never:save_config_to_db)reset_memory_client)return None→ResponseValidationErrorfrom FastAPI)All three other write endpoints (
PATCH,PUT /mem0/llm,PUT /mem0/embedder, etc.) correctly call all three. ThePUT /handler was simply incomplete.Fix: Add the three missing lines after
updated_config["mem0"] = config.mem0.dict(exclude_none=True):Tests
Added
openmemory/api/tests/test_config_api.py(10 tests, all passing) that:Files changed
openmemory/docker-compose.yml:-http://localhost:8765fallback toNEXT_PUBLIC_API_URLopenmemory/ui/hooks/useConfig.tsopenmemory/api/app/routers/config.pysave_config_to_db,reset_memory_client,returnto PUT handleropenmemory/api/tests/test_config_api.py