Skip to content

fix: encode dynamic URL path segments#5818

Closed
VectorPeak wants to merge 3 commits into
mem0ai:mainfrom
VectorPeak:fix
Closed

fix: encode dynamic URL path segments#5818
VectorPeak wants to merge 3 commits into
mem0ai:mainfrom
VectorPeak:fix

Conversation

@VectorPeak

@VectorPeak VectorPeak commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Linked Issue

Fixes #5893. This fixes a URL construction bug found while reviewing SDK and CLI path handling.

Description

What Problem This Solves

Several hosted SDK and CLI methods build API paths by directly interpolating caller-provided IDs into URL path segments:

/v1/memories/{memory_id}/
/v1/memories/{memory_id}/history/
/v2/entities/{entity_type}/{entity_id}/
/v1/event/{event_id}/

Those values are identifiers from the SDK or CLI caller's perspective, but /, ?, and # are structural URL characters. If an ID contains one of those characters, the request no longer targets the intended single path segment.

For example, deleting a user/entity named:

org/team?active#frag

can construct a path like:

/v2/entities/user/org/team?active#frag/

That can be parsed as an extra path segment plus query/fragment syntax instead of one literal entity identifier. The same class of issue applies to memory IDs and event IDs in the affected SDK and CLI paths.

The intended request path is:

/v2/entities/user/org%2Fteam%3Factive%23frag/

This keeps the identifier as a single URL path segment and lets the server receive the literal ID value after decoding.

Change

This PR adds local path-segment encoding helpers and uses them only where dynamic IDs are inserted into hosted API paths.

  • Python SDK and Python CLI use urllib.parse.quote(str(value), safe="") so / is encoded instead of being preserved by quote()'s default behavior.
  • TypeScript SDK and Node CLI use encodeURIComponent(String(value)) for the same single-segment encoding behavior.
  • Query-string parameters are unchanged; this PR only touches dynamic path segments.
  • The public API surface is unchanged. Callers can continue passing the same IDs, including IDs that contain URL syntax characters.

Affected hosted API paths include:

/v1/memories/{memory_id}/
/v1/memories/{memory_id}/history/
/v2/entities/{entity_type}/{entity_id}/
/v1/event/{event_id}/

Evidence

The bug is reproducible by comparing raw interpolation with path-segment encoding:

Input entity id:
org/team?active#frag

Before:
/v2/entities/user/org/team?active#frag/

After:
/v2/entities/user/org%2Fteam%3Factive%23frag/

For memory and event IDs, the same behavior appears with values such as:

mem/a?b#c  -> /v1/memories/mem%2Fa%3Fb%23c/
evt/a?b#c  -> /v1/event/evt%2Fa%3Fb%23c/

This is a URL construction issue in the clients before the request reaches the API. It does not require a server-side behavior change to demonstrate the problem: unencoded path separators and query/fragment delimiters change the request URL semantics.

Possible call chain / impact

Python SDK:

application code
  -> MemoryClient.get/update/delete/history(memory_id)
  -> f"/v1/memories/{memory_id}/..."
  -> httpx client request

Async Python SDK:

application code
  -> AsyncMemoryClient.get/update/delete/history(memory_id)
  -> f"/v1/memories/{memory_id}/..."
  -> async httpx client request

Python CLI:

mem0 CLI command
  -> PlatformBackend.get/update/delete/delete_entities/get_event(...)
  -> f-string path construction
  -> httpx request

Node CLI:

mem0 CLI command
  -> PlatformBackend.get/update/delete/deleteEntities/getEvent(...)
  -> template-literal path construction
  -> fetch request

TypeScript SDK:

application code
  -> MemoryClient.get/update/delete/history/deleteUser/deleteUsers(...)
  -> template-literal path construction
  -> fetch or axios request

Paths using query parameters, such as list/search filters, are not affected by this PR because they already go through query/body construction rather than path-segment interpolation.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactor (no functional changes)
  • Documentation update

Breaking Changes

N/A

Test Coverage

  • I added/updated unit tests
  • I added/updated integration tests
  • I tested manually (describe below)
  • No tests needed (explain why)

Manual validation performed by inspecting the generated paths before and after encoding for representative IDs containing /, ?, and #.

This validation checked that those characters are preserved as part of one path segment after encoding, for example:

id/with?query#frag -> id%2Fwith%3Fquery%23frag
org/team?active#frag -> org%2Fteam%3Factive%23frag

Expected encoded request examples:

/v1/memories/mem%2Fa%3Fb%23c/
/v1/memories/mem%2Fa%3Fb%23c/history/
/v2/entities/user/org%2Fteam%3Factive%23frag/
/v1/event/evt%2Fa%3Fb%23c/

Validation run locally:

git diff --check upstream/main...HEAD

Additional local validation was posted in the PR comments. No test files were added or committed in this PR.

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have added tests that prove my fix/feature works
  • New and existing tests pass locally
  • I have updated documentation if needed

@CLAassistant

CLAassistant commented Jun 24, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@VectorPeak
VectorPeak marked this pull request as ready for review June 24, 2026 12:30
@VectorPeak

VectorPeak commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Validation update for this PR. No test files were added or committed; the checks below were run locally against the current PR branch.

What was validated

The URL path-segment encoding behavior was checked across the four changed production files:

  • mem0/client/main.py
  • cli/python/src/mem0_cli/backend/platform.py
  • mem0-ts/src/client/mem0.ts
  • cli/node/src/backend/platform.ts

Representative IDs containing URL-structural characters were used, for example:

id/with?query#frag
org/team?active#frag

Expected encoded path segment:

id%2Fwith%3Fquery%23frag
org%2Fteam%3Factive%23frag

Results

Python SDK

Validated with httpx.MockTransport by capturing the generated request paths for sync and async SDK calls. No real API request was sent.

Covered paths included:

GET    /v1/memories/id%2Fwith%3Fquery%23frag/
PUT    /v1/memories/id%2Fwith%3Fquery%23frag/
DELETE /v1/memories/id%2Fwith%3Fquery%23frag/
GET    /v1/memories/id%2Fwith%3Fquery%23frag/history/
DELETE /v2/entities/user/id%2Fwith%3Fquery%23frag/
DELETE /v2/entities/agent/id%2Fwith%3Fquery%23frag/
DELETE /v2/entities/app/id%2Fwith%3Fquery%23frag/
DELETE /v2/entities/run/id%2Fwith%3Fquery%23frag/

Result: PASS. Checked path segments encoded /, ?, and # as %2F, %3F, and %23.

Python CLI

Validated by monkeypatching _request and capturing the generated method/path values. No real API request was sent.

Covered paths included:

GET    /v1/memories/mem%2Fory%3Fx%23frag/
PUT    /v1/memories/mem%2Fory%3Fx%23frag/
DELETE /v1/memories/mem%2Fory%3Fx%23frag/
DELETE /v2/entities/user/entity%2Fid%3Fx%23frag/
DELETE /v2/entities/agent/entity%2Fid%3Fx%23frag/
DELETE /v2/entities/app/entity%2Fid%3Fx%23frag/
DELETE /v2/entities/run/entity%2Fid%3Fx%23frag/
GET    /v1/event/event%2Fid%3Fx%23frag/

Result: PASS.

TypeScript SDK

Validated by loading the TS source with ts-node/register/transpile-only, mocking request helpers, and capturing generated paths. No real API request was sent.

Covered paths included:

/v1/memories/id%2Fwith%3Fhash%23frag/
/v1/memories/id%2Fwith%3Fhash%23frag/?user_id=id%2Fwith%3Fhash%23frag
/v1/memories/id%2Fwith%3Fhash%23frag/history/
/v1/entities/user%2Ftype%3Fx%23y/id%2Fwith%3Fhash%23frag/
/v2/entities/user/id%2Fwith%3Fhash%23frag/

Result: PASS. badCount: 0 for the checked path segments.

Node CLI

Validated by mocking globalThis.fetch and capturing generated request paths.

Covered paths included:

/v1/memories/id%2Fwith%3Fhash%23frag/?source=CLI
/v1/memories/id%2Fwith%3Fhash%23frag/
/v2/entities/user/id%2Fwith%3Fhash%23frag/?source=CLI
/v2/entities/agent/id%2Fwith%3Fhash%23frag/?source=CLI
/v2/entities/app/id%2Fwith%3Fhash%23frag/?source=CLI
/v2/entities/run/id%2Fwith%3Fhash%23frag/?source=CLI
/v1/event/id%2Fwith%3Fhash%23frag/

Result: PASS. badCount: 0 for the checked path segments.

Local commands

git diff --check upstream/main...HEAD

Result: PASS.

cd cli/node
./node_modules/.bin/biome.cmd check src/backend/platform.ts
./node_modules/.bin/tsc.cmd --noEmit
./node_modules/.bin/vitest.cmd run

Result: PASS.

Test Files  8 passed (8)
Tests       116 passed (116)
cd mem0-ts
# Verified against the Git blob for this PR, not the local Windows working-tree copy.
node -e "const { execFileSync } = require('child_process'); const prettier = require('./node_modules/prettier'); (async () => { const file = 'src/client/mem0.ts'; const head = execFileSync('git', ['show', 'HEAD:mem0-ts/src/client/mem0.ts'], { encoding: 'utf8', cwd: '..' }); const opts = (await prettier.resolveConfig(file)) || {}; opts.filepath = file; const out = await prettier.format(head, opts); console.log('HEAD blob equals prettier output:', head === out); })();"

Result: PASS.

HEAD blob equals prettier output: true

Note: a direct local Windows working-tree check initially reported a Prettier warning for src/client/mem0.ts. Follow-up validation showed this came from mixed line endings in the local checkout (git ls-files --eol reported w/mixed), while the Git blob in this PR matches Prettier output.

Not covered

  • These checks did not send real requests to the hosted API; they validate client-side URL construction before transport.
  • The Python SDK check did not cover event_id in mem0/client/main.py because no matching event_id path was found there.
  • Some other dynamic path parameters, such as project_id, org_id, webhook_id, organizationId, projectId, and webhookId, still appear outside this PR's current memory_id / entity / event focused scope.
  • No test files were added or committed for this validation.

@kartik-mem0

Copy link
Copy Markdown
Contributor

Changes Requested

The URL encoding logic is correct and the cross-package coverage is solid — this is good, mergeable work once two blocking items are addressed.

Strengths

  • urllib.parse.quote(str(value), safe="") (mem0/client/main.py:46, cli/python/src/mem0_cli/backend/platform.py:15) is the right primitive: safe="" encodes /, ?, #, %, and space — the full set of path-segment-breaking characters. No double-encoding risk with httpx.
  • encodeURIComponent(String(value)) (mem0-ts/src/client/mem0.ts:71, cli/node/src/backend/platform.ts:20) is the idiomatic choice for JS/TS path segments. Same coverage.
  • Sync/async symmetry in Python SDK: all five affected methods (get, update, delete, history, delete_users) are patched in both MemoryClient and AsyncMemoryClient — no divergence.
  • The deprecated deleteUser method in mem0-ts/src/client/mem0.ts:469 is also patched — good catch, not skipping it just because it's deprecated.
  • All four packages addressed in one PR (Python SDK, Python CLI, TypeScript SDK, Node CLI).

Issues

Important (should fix before merge)

  1. mem0/client/main.py:1374 — line is 121 chars, over the 120-char Ruff limit (verified: ruff check --select E501 mem0/client/main.py flags this line). Wrap it:

    response = await self.async_client.get(
        f"/v1/memories/{_encode_path_segment(memory_id)}/history/",
        params=params,
    )
  2. cli/python/src/mem0_cli/backend/platform.py — four lines exceed the CLI's 100-char Ruff limit (verified with ruff check):

    • Line 204: 113 chars (get() method)
    • Line 258: 101 chars (update() method)
    • Line 283: 102 chars (delete() method)
    • Line 311: 103 chars (delete_entities() method)

    The delete() and delete_entities() call sites in the same file already use the correct multi-line style — please apply the same wrapping to get() and update().

  3. Regression tests are missing. Commit 1 of this PR added a solid set of regression tests (TestPathSegmentEncoding in tests/test_client.py, test_platform_backend_paths.py for the CLI, and path-encoding cases in the TypeScript and Node test files). Commit 2 deleted all of them. I ran them against the PR head and confirmed they pass; I also confirmed they fail against the pre-fix implementation — the red-green proof is solid. Please restore those tests. For a multi-site URL encoding fix across four packages, automated regression coverage is essential so a future refactor can't quietly reintroduce this class of bug.

Minor (optional)

  • mem0/client/main.py:46 types the helper as _encode_path_segment(value: Any) while the CLI version types it str. All call sites pass str; using str in both is more precise. The str(value) cast inside the body is fine as a guard.
  • The webhook project_id paths (mem0/client/main.py:817,845) are a separate, pre-existing instance of the same class — out of scope for this PR but worth a follow-up issue (project_id: str, caller-supplied).

Evidence

  • CI: Only Vercel (advisory — deploy auth fails on all forks; not a required check) and CLA (pass). No real test/lint/build checks from CI Gate have run against this head.
  • Targeted tests: hatch -e dev_py_3_12 run pytest tests/test_client.py -v --tb=short on PR head → 22 pre-existing tests passed (0 path-encoding tests present, all deleted in commit 2).
  • Red-green proof: Restored TestPathSegmentEncoding from commit 1 (b50d8e31); ran against pre-fix main.py → 4 FAILED (raw paths like /v1/memories/mem/a?b#c/ instead of /v1/memories/mem%2Fa%3Fb%23c/). Ran against PR head → 4 PASSED. Fix bites.
  • Ruff line-length check: hatch -e dev_py_3_12 run ruff check --select E501 mem0/client/main.py cli/python/src/mem0_cli/backend/platform.py → 4 violations (lines 1374, 204, 258, 283 as above). Will fail CI lint.
  • Linked issue: None. For tracking on our side, mind opening a quick issue for this bug and linking it here? Makes it easier to prioritize and reference.
  • Could NOT verify: TypeScript/Node test restoration via jest/vitest (no Node env available in review context) — checked the test code from the removed commit by code tracing and it looks correct; the Python red-green is the primary signal.

Assessment

Ready to merge: With fixes — please restore the regression tests (commit 1 has them ready), fix the four line-length violations (two files), and push. I'll re-review once you push.

@VectorPeak

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review ? I addressed the blocking points you called out.

Updates in this push:

  • wrapped the Python long lines in mem0/client/main.py and cli/python/src/mem0_cli/backend/platform.py
  • restored the path-segment regression coverage across all four surfaces you mentioned:
    • Python SDK: tests/test_client.py
    • Python CLI: cli/python/tests/test_platform_backend_paths.py
    • TypeScript SDK: mem0-ts/src/client/tests/memoryClient.crud.test.ts and mem0-ts/src/client/tests/memoryClient.users.test.ts
    • Node CLI: cli/node/tests/platform-backend.test.ts

I also reran the relevant checks locally:

  • python -m pytest .\\tests\\test_client.py -q
  • python -m pytest .\\cli\\python\\tests\\test_platform_backend_paths.py -q
  • python -m ruff check --select E501 mem0/client/main.py cli/python/src/mem0_cli/backend/platform.py
  • npm test -- memoryClient.crud.test.ts memoryClient.users.test.ts
  • npm test -- platform-backend.test.ts

On the size of the patch: this commit is 310 insertions / 7 deletions, and 288 of those changed lines are test-side changes (287 insertions + 1 deletion). So most of the delta here is restoring regression coverage rather than expanding the production fix surface.

@kartik-mem0

Copy link
Copy Markdown
Contributor

LGTM

Both blocking items from the prior review are resolved — confirmed by re-running targeted tests locally at e6ed84f.

What's Fixed

Regression tests restored (all four packages):

  • Python SDK: TestPathSegmentEncoding (4 tests — sync + async get, update, delete, history, delete_users) in tests/test_client.py
  • Python CLI: test_memory_id_path_segments_are_encoded + test_entity_and_event_path_segments_are_encoded in cli/python/tests/test_platform_backend_paths.py
  • TypeScript SDK: 4 tests added to memoryClient.crud.test.ts and 2 to memoryClient.users.test.ts
  • Node CLI: cli/node/tests/platform-backend.test.ts (new file, 2 tests)

Line-length violations resolved: mem0/client/main.py and cli/python/src/mem0_cli/backend/platform.py are both ruff-clean.

Evidence

  • Red-green proof (Python SDK, re-run this session):
    Tests at PR head → 4 PASSED. main.py substituted with main's implementation → 4 FAILED (raw /v1/memories/mem/a?b#c/ vs expected /v1/memories/mem%2Fa%3Fb%23c/). Restored → 4 PASSED.
  • Python CLI tests: 2/2 PASSED at PR head.
  • Ruff E501: ruff check --select E501 mem0/client/main.py → clean. cd cli/python && ruff check . → clean. ruff format --check . → 1 file already formatted.
  • CI: Vercel (advisory — deploy auth fails on all forks, not merge-blocking). CLA: pass. No required CI Gate checks pending — the fix is confirmed by local red-green above.

Remaining (non-blocking, optional)

  • tests/test_client.py — the async tests use asyncio.run() wrapping rather than @pytest.mark.asyncio. Functional as written; if the test suite ever moves to asyncio_mode = "auto", these would need updating.
  • mem0/client/main.py_encode_path_segment(value: Any) accepts None and silently produces the string "None". All real call sites pass str; the type annotation and a guard could make intent explicit, but this is not a regression.

Neither item is worth holding the PR for.

Assessment

Ready to squash-merge. Thanks for working through the two rounds — the cross-package coverage and the restored tests make this a solid, maintainable fix.

One small ask: would you mind opening a tracking issue for this bug? No linked issue at the moment, and having one makes it easier to trace back if the encoding class comes up again (the project_id/webhook_id paths are still unencoded in the same files, worth a follow-up).

Copy link
Copy Markdown
Contributor Author

@kartik-mem0 thanks again for the review and for the quick follow-up. I opened and linked the tracking issue here: #5893.

Happy to contribute to the project and help with any follow-ups where useful.

@VectorPeak VectorPeak closed this by deleting the head repository Jun 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Encode dynamic URL path segments in SDK and CLI requests

3 participants