airkjw/claude-mem
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MAESTRO: Review PR #792 persistent Chroma HTTP server — changes requested due to Zscaler SSL regression, missing tests, merge conflicts

Browse Source 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Alex Newman
2026-02-06 02:24:03 -05:00
parent 0622ebf534
commit ed9ffe76a7
1 changed files with 2 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Auto Run Docs/PR-Triage/PR-Triage-09.md
+2 -1
View File
@@ -18,4 +18,5 @@ These PRs address Chroma vector database stability, zombie processes, and enterp
## Feature
- [ ] Evaluate PR #792 (`feat: Replace MCP subprocess with persistent Chroma HTTP server` by @bigph00t). Files: 12 files. Major architectural change — replaces the MCP subprocess model for Chroma with a persistent HTTP server. Steps: (1) `gh pr checkout 792` (2) This is a significant architecture change. Review the approach: persistent HTTP server vs. MCP subprocess (3) Check resource implications (always-running server vs. on-demand subprocess) (4) Verify graceful shutdown and cross-platform support (5) Run `npm run build` (6) If the architecture is sound and well-tested: `gh pr merge 792 --rebase --delete-branch`. If concerns exist, leave detailed review comments.
- [x] Evaluate PR #792 (`feat: Replace MCP subprocess with persistent Chroma HTTP server` by @bigph00t). Files: 12 files. Major architectural change — replaces the MCP subprocess model for Chroma with a persistent HTTP server. Steps: (1) `gh pr checkout 792` (2) This is a significant architecture change. Review the approach: persistent HTTP server vs. MCP subprocess (3) Check resource implications (always-running server vs. on-demand subprocess) (4) Verify graceful shutdown and cross-platform support (5) Run `npm run build` (6) If the architecture is sound and well-tested: `gh pr merge 792 --rebase --delete-branch`. If concerns exist, leave detailed review comments.
- **Changes Requested** (2026-02-06): Architecture direction is sound — persistent HTTP server via `npx chroma run` with ChromaServerManager singleton is the right long-term approach. Eliminates per-operation subprocess overhead, re-enables Windows support, clean separation between server lifecycle and data operations. Build succeeds and all 34 existing Chroma tests pass. However, 4 critical blockers prevent merge: (1) **Zscaler SSL regression** — PR #884's `getCombinedCertPath()` and SSL env var injection are completely removed, breaking enterprise users behind corporate proxies; (2) **No tests** for 290-line ChromaServerManager (process spawn, heartbeat polling, cross-platform shutdown); (3) **Merge conflicts** with PRs #769, #884, #887 already merged to main; (4) **macOS untested** despite being primary dev platform. Additional concerns: ~180MB memory overhead, port 8000 default conflicts, Python dependency not actually removed (shifted to npx chroma run). Left detailed review comment requesting rebase, Zscaler SSL port, ChromaServerManager tests, and macOS verification.