Adding local XMem setup#200
Conversation
|
✅ Staging Deployment Report
🟢 Staging is live and healthy! Test your changes at the staging URL above. Ready to ship? Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive local-first development environment for XMem, including a new create-xmem scaffolding package, Docker-based local storage (Postgres/pgvector, MongoDB, Neo4j), and a suite of scripts for setup, diagnostics, and context management. Key code changes include improved API exception handling, user ID normalization, and configurable LLM timeouts. Feedback highlights several cross-platform compatibility issues in the PowerShell scripts—specifically regarding the hardcoded powershell executable name and the Windows-only System.Drawing assembly. Additionally, a security risk was identified where static keys could allow user impersonation in production if not properly guarded by environment checks, and an improvement was suggested for the greedy argument parsing logic in the workspace creation script.
| Write-Step ".env already exists; leaving it unchanged" | ||
| } | ||
|
|
||
| Invoke-Native { powershell -ExecutionPolicy Bypass -File (Join-Path $Root "scripts\configure-xmem-env.ps1") -EnvPath $envTarget } |
There was a problem hiding this comment.
Hardcoding powershell will fail on non-Windows systems where the executable is named pwsh. Since this script is already running in a PowerShell host, you should use the call operator & to execute the script in the current session, which is more efficient and cross-platform.
Invoke-Native { & (Join-Path $PSScriptRoot "configure-xmem-env.ps1") -EnvPath $envTarget }
| } | ||
|
|
||
| Write-Step "Patching extension for local API" | ||
| Invoke-Native { powershell -ExecutionPolicy Bypass -File (Join-Path $Root "scripts\patch-extension-local.ps1") -ExtensionDir $ExtensionDir } |
There was a problem hiding this comment.
Hardcoding powershell will fail on non-Windows systems where the executable is named pwsh. Use the call operator & to execute the script in the current session for better compatibility and performance.
Invoke-Native { & (Join-Path $PSScriptRoot "patch-extension-local.ps1") -ExtensionDir $ExtensionDir }
| [string]$Path | ||
| ) | ||
|
|
||
| Add-Type -AssemblyName System.Drawing |
There was a problem hiding this comment.
The System.Drawing assembly is a Windows-specific GDI+ wrapper and is not available in PowerShell Core (pwsh) on Linux or macOS. This will cause the setup script to fail on non-Windows platforms. Consider wrapping this in a platform check or providing pre-built icons.
if (-not $IsWindows) { return }
Add-Type -AssemblyName System.Drawing
| if requested_user_id and ( | ||
| user.get("email") == "static@xmem.ai" or user.get("name") == "Static Key User" | ||
| ): | ||
| return requested_user_id |
There was a problem hiding this comment.
This logic allows a user authenticated with a 'static' key to impersonate any user_id by providing it in the request. While useful for local development, this is a security risk if the static key is ever configured in a production-like environment. This check should be strictly guarded by an environment check to ensure it only runs in development or local modes.
| if requested_user_id and ( | |
| user.get("email") == "static@xmem.ai" or user.get("name") == "Static Key User" | |
| ): | |
| return requested_user_id | |
| if requested_user_id and settings.environment.lower() in {"development", "dev", "local"} and ( | |
| user.get("email") == "static@xmem.ai" or user.get("name") == "Static Key User" | |
| ): | |
| return requested_user_id |
| usage(1); | ||
| } | ||
|
|
||
| options.target = arg; |
There was a problem hiding this comment.
The positional argument handling is greedy and will overwrite options.target with every non-option argument provided. This could lead to unexpected behavior if extra arguments are passed. Consider ensuring the target is only set once if it's not the default value.
if (options.target === "xmem") {
options.target = arg;
}
🔍 API Schema Diff---REPORT--- 🔄 Modified
Auto-generated by API Schema Diff workflow |
|
| Filename | Overview |
|---|---|
| src/api/app.py | Adds RequestValidationError and HTTPException handlers, plus _public_exception_message to safely redact internal error details in production; previous issues with raw ConnectionError/ValueError/TimeoutError leakage are resolved. |
| src/api/routes/memory.py | Fixes per-item user_id in batch ingest, introduces _scoped_ingest_payload and _current_user_id with static-key local override, makes ingest timeouts configurable, and sanitizes 5xx error details in production. |
| src/api/schemas.py | Removes strict regex on user_id in favour of normalize_user_id; the normalization function uses value or "" which silently drops falsy non-None primitives (e.g. integer 0) before stringifying. |
| src/agents/base.py | LLM timeout is now configurable via settings.llm_timeout_seconds; asyncio.TimeoutError is re-raised as TimeoutError with a helpful message including agent name, model, and an Ollama hint. |
| src/config/settings.py | Adds llm_timeout_seconds (default 45s) and memory_ingest_timeout_seconds (default 120s) fields. |
| scripts/verify.py | New smoke-test script that polls /health and runs ingest/search/retrieve; sleep(3) is only in the exception handler so a reachable-but-not-ready API causes a tight polling loop. |
| packages/create-xmem/bin/create-xmem.js | New CLI tool that clones the XMem repo into a local workspace directory; straightforward and safe. |
| templates/xmem.env.local | Environment template for local dev; secrets are left empty with placeholders; JWT_SECRET_KEY is clearly labelled dev-only. |
| scripts/xmem.js | Main CLI dispatcher for local workspace commands (dev, setup, start, verify, doctor, context export/import); delegates to PowerShell scripts on all platforms. |
Reviews (5): Last reviewed commit: "Redact production ValueError responses" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
✅ Staging Deployment Report
🟢 Staging is live and healthy! Test your changes at the staging URL above. Ready to ship? Comment |
🔍 API Schema Diff---REPORT--- 🔄 Modified
Auto-generated by API Schema Diff workflow |
✅ Staging Deployment Report
🟢 Staging is live and healthy! Test your changes at the staging URL above. Ready to ship? Comment |
🔍 API Schema Diff---REPORT--- 🔄 Modified
Auto-generated by API Schema Diff workflow |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
✅ Staging Deployment Report
🟢 Staging is live and healthy! Test your changes at the staging URL above. Ready to ship? Comment |
🔍 API Schema Diff---REPORT--- 🔄 Modified
Auto-generated by API Schema Diff workflow |
✅ Staging Deployment Report
🟢 Staging is live and healthy! Test your changes at the staging URL above. Ready to ship? Comment |
🔍 API Schema Diff---REPORT--- 🔄 Modified
Auto-generated by API Schema Diff workflow |
2 participants
Summary
Adds local XMem setup directly inside the main
XortexAI/XMemrepo.This removes the need for a separate
xmem-devwrapper repo. Users can now create and run a local XMem workspace with:npx create-xmem@latest cd xmem npm run dev