Conversation
🛡️ Jit Security Scan Results✅ No security findings were detected in this PR
Security scan by Jit
|
There was a problem hiding this comment.
Pull request overview
This PR removes deprecated backward-compatibility behavior from the GET /v1/working-memory/{session_id} endpoint that previously returned an empty session with unsaved=True for missing sessions to support old clients. After this change, all clients consistently receive a 404 error for missing sessions, which aligns with proper REST semantics and prevents bugs caused by parameter mismatches between PUT and GET operations.
Changes:
- Removed version-checking functions (
parse_client_version(),is_old_client()) and theX-Client-Versionheader parameter from the GET endpoint - Updated
get_working_memory()to always return 404 for missing sessions instead of conditionally creating empty unsaved sessions - Removed deprecated
new_sessionandunsavedfields fromWorkingMemoryResponsemodel - Updated tests to expect 404 responses and removed old client version test case
- Applied code formatting improvements to assert statements following Black/Ruff conventions
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| agent_memory_server/api.py | Removed version checking functions, X-Client-Version header parameter, and deprecated empty-session-on-missing behavior; simplified get_working_memory to always return 404 for missing sessions |
| agent_memory_server/models.py | Removed deprecated new_session and unsaved fields from WorkingMemoryResponse |
| tests/test_api.py | Updated test expectations to check for 404 on missing sessions, removed old client version test, and applied Black/Ruff formatting to assert statements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
The code changes correctly remove the deprecated backward-compatibility behavior. However, there's a documentation inconsistency that should be addressed.
Documentation Issue:
The file docs/api.md still documents the new_session and unsaved fields in the WorkingMemoryResponse model (lines 533-534), but these fields have been removed from the server implementation. This documentation should be updated to reflect the current API schema.
Client Library Consideration:
The client libraries (Python, JavaScript, and Java) still have the new_session and unsaved fields in their WorkingMemoryResponse models. While this won't cause immediate breakage (clients will receive null values), consider documenting the plan for deprecating these fields in the client libraries in a future release.
🤖 Automated review complete. Please react with 👍 or 👎 on the individual review comments to provide feedback on their usefulness.
…king-memory All clients now receive 404 for missing sessions instead of the old backward-compat path that returned an empty WorkingMemory with unsaved=True. Removes parse_client_version/is_old_client helpers, the X-Client-Version header parameter, and the new_session/unsaved response fields. Closes #136
c494726 to
47ad29d
Compare
Add integration test that confirms the server response no longer includes new_session/unsaved fields, and that the client library's WorkingMemoryResponse model (which still declares them with default=None) can deserialize the response without errors.
|
Addressed both points from the automated review:
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| until_summarization_percentage | ||
| ) | ||
| working_mem_data["new_session"] = new_session | ||
| working_mem_data["unsaved"] = unsaved |
There was a problem hiding this comment.
Stale comment references removed new_session field
Low Severity
The comment on this line still references new_session ("no new_session for PUT") even though this PR removes the new_session field entirely from WorkingMemoryResponse. The parenthetical is now meaningless and could confuse future readers into thinking new_session is still a concept in the codebase.


Summary
GET /v1/working-memory/{session_id}returned an emptyWorkingMemorywithunsaved=Truefor missing sessions (old clients withoutX-Client-Versionheader or version < 0.12.0)parse_client_version(),is_old_client(), theX-Client-Versionheader parameter, and thenew_session/unsavedfields fromWorkingMemoryResponsedocs/api.mdto remove the deprecated fields from the schema tableTest plan
uv run ruff check && uv run ruff format— passesuv run pytest tests/test_api.py -v— all tests passuv run pytest— full suite passes (704 passed, 102 skipped)new_session/unsavedand client model (fields default toNone) parses the response without errorsCloses #136
Note
Medium Risk
Behavior change in
GET /v1/working-memory/{session_id}(and its response schema) can break older clients that relied on implicit empty sessions or onnew_session/unsavedfields; otherwise the change is localized to a single endpoint and its model/docs/tests.Overview
GET /v1/working-memory/{session_id}now always returns 404 when a session is missing, removing the deprecated compatibility path that returned an empty, non-persisted session for older clients.The API drops the
X-Client-Versionhandling and removes deprecatedWorkingMemoryResponsefields (new_session,unsaved); docs and tests are updated accordingly, including a test ensuring the client model still deserializes responses when those fields are absent.Written by Cursor Bugbot for commit 7ee093d. This will update automatically on new commits. Configure here.