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

MAESTRO: Mark PR #884 task complete in PR-Triage-09 with detailed merge notes

Browse Source 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Alex Newman
2026-02-06 02:15:59 -05:00
parent 3ad53733e8
commit 63b0105d05
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
@@ -10,7 +10,8 @@ These PRs address Chroma vector database stability, zombie processes, and enterp
- [x] Review PR #769 (`fix: close transport on connection error to prevent chroma-mcp zombie processes` by @jenyapoyarkov). Files: `src/services/sync/ChromaSync.ts`, tests. Transport left open on connection failure creates zombies. Steps: (1) `gh pr checkout 769` (2) Review — should close/dispose transport in the error path (3) Run `npm run build` (4) If clean: `gh pr merge 769 --rebase --delete-branch`
- **Merged** (2026-02-06): Confirmed the bug — both `ensureCollection()` (~line 202) and `queryChroma()` (~line 862) error handlers reset `connected` and `client` on connection errors but never called `transport.close()`, leaving chroma-mcp subprocesses alive as zombies. Fix adds `transport.close()` (wrapped in try/catch for already-dead transports) and `transport = null` before resetting state, mirroring the `close()` method pattern. 3 new regression tests added. All 19 integration tests + 20 ChromaSearchStrategy tests pass. Build clean.
- [ ] Review PR #884 (`fix: add Zscaler SSL certificate support for ChromaDB vector search` by @RClark4958). Files: `src/services/sync/ChromaSync.ts` + build artifacts. Enterprise environments use Zscaler SSL inspection which breaks Chroma HTTPS connections. Steps: (1) `gh pr checkout 884` (2) Review — should respect NODE_EXTRA_CA_CERTS or custom CA cert configuration (3) Verify this doesn't weaken SSL for non-Zscaler users (4) Run `npm run build` (5) If appropriate for enterprise support: `gh pr merge 884 --rebase --delete-branch`
- [x] Review PR #884 (`fix: add Zscaler SSL certificate support for ChromaDB vector search` by @RClark4958). Files: `src/services/sync/ChromaSync.ts` + build artifacts. Enterprise environments use Zscaler SSL inspection which breaks Chroma HTTPS connections. Steps: (1) `gh pr checkout 884` (2) Review — should respect NODE_EXTRA_CA_CERTS or custom CA cert configuration (3) Verify this doesn't weaken SSL for non-Zscaler users (4) Run `npm run build` (5) If appropriate for enterprise support: `gh pr merge 884 --rebase --delete-branch`
- **Merged** (2026-02-06): Manually applied to main (compiled output conflicts from PR #769 merge). Adds `getCombinedCertPath()` method that detects Zscaler certificates in the macOS system keychain via `security find-certificate`, combines them with certifi CA certs into `~/.claude-mem/combined_certs.pem` (cached 24h, atomic write), and passes `SSL_CERT_FILE`/`REQUESTS_CA_BUNDLE`/`CURL_CA_BUNDLE` env vars to the chroma-mcp subprocess. Falls back gracefully when Zscaler is absent — no impact on non-Zscaler environments. `NODE_EXTRA_CA_CERTS` not needed since the Python subprocess uses Python SSL env vars, not Node.js ones. macOS-only cert extraction with cross-platform cache reuse. All 39 Chroma tests pass. Build clean.
- [ ] Review PR #830 (`fix: Chroma stability + additional process management layers` by @michelhelsdingen). Files: 7 files including ChromaSync.ts, ProcessManager.ts, worker-service.ts. Steps: (1) `gh pr checkout 830` (2) This is a broader stability PR — review scope carefully (3) Check for overlap with ProcessRegistry (v9.0.8) (4) If too broad, request scope reduction to just the Chroma stability portions (5) Run `npm run build`