From ed9ffe76a7110fea3dbdb47522e6468d0536c8f0 Mon Sep 17 00:00:00 2001 From: Alex Newman Date: Fri, 6 Feb 2026 02:24:03 -0500 Subject: [PATCH] =?UTF-8?q?MAESTRO:=20Review=20PR=20#792=20persistent=20Ch?= =?UTF-8?q?roma=20HTTP=20server=20=E2=80=94=20changes=20requested=20due=20?= =?UTF-8?q?to=20Zscaler=20SSL=20regression,=20missing=20tests,=20merge=20c?= =?UTF-8?q?onflicts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.6 --- Auto Run Docs/PR-Triage/PR-Triage-09.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Auto Run Docs/PR-Triage/PR-Triage-09.md b/Auto Run Docs/PR-Triage/PR-Triage-09.md index 9d52b958..f57bdf29 100644 --- a/Auto Run Docs/PR-Triage/PR-Triage-09.md +++ b/Auto Run Docs/PR-Triage/PR-Triage-09.md @@ -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.