fix: always pass --ssl flag to chroma-mcp in remote mode (#1286)
* fix: always pass --ssl flag to chroma-mcp in remote mode The chroma-mcp CLI defaults to SSL when using --client-type http. When CLAUDE_MEM_CHROMA_SSL is false (the common case for local ChromaDB servers), buildCommandArgs() omitted --ssl entirely, causing chroma-mcp to attempt an SSL connection to a plain HTTP server and fail with "Could not connect to a Chroma server". Always pass --ssl with an explicit true/false value so the user's CLAUDE_MEM_CHROMA_SSL setting is faithfully forwarded. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: add regression tests for ChromaMcpManager SSL flag fix Adds 4 focused test cases verifying buildCommandArgs() produces correct --ssl args, covering SSL=false, SSL=true, unset (defaults to false), and local mode (no --ssl flag). Requested by @xkonjin in PR #1286 review. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: rebuild checked-in bundles to include SSL flag fix Rebuild all bundles against upstream/main so the --ssl <true|false> fix is present in the runtime artifacts that hooks and the marketplace plugin actually execute. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
File diff suppressed because one or more lines are too long
+272
-277
File diff suppressed because one or more lines are too long
+11
-11
File diff suppressed because one or more lines are too long
@@ -201,9 +201,7 @@ export class ChromaMcpManager {
|
||||
'--port', chromaPort
|
||||
];
|
||||
|
||||
if (chromaSsl) {
|
||||
args.push('--ssl');
|
||||
}
|
||||
args.push('--ssl', chromaSsl ? 'true' : 'false');
|
||||
|
||||
if (chromaTenant !== 'default_tenant') {
|
||||
args.push('--tenant', chromaTenant);
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
/**
|
||||
* Regression tests for ChromaMcpManager SSL flag handling (PR #1286)
|
||||
*
|
||||
* Validates that buildCommandArgs() always emits the correct `--ssl` flag
|
||||
* based on CLAUDE_MEM_CHROMA_SSL, and omits it entirely in local mode.
|
||||
*
|
||||
* Strategy: mock StdioClientTransport to capture the spawned args without
|
||||
* actually launching a subprocess, then inspect the captured args array.
|
||||
*/
|
||||
import { describe, it, expect, beforeEach, mock } from 'bun:test';
|
||||
|
||||
// ── Mutable settings closure (updated per test) ────────────────────────
|
||||
let currentSettings: Record<string, string> = {};
|
||||
|
||||
// ── Mock modules BEFORE importing the module under test ────────────────
|
||||
// Capture the args passed to StdioClientTransport constructor
|
||||
let capturedTransportOpts: { command: string; args: string[] } | null = null;
|
||||
|
||||
mock.module('@modelcontextprotocol/sdk/client/stdio.js', () => ({
|
||||
StdioClientTransport: class FakeTransport {
|
||||
// Required: ChromaMcpManager assigns transport.onclose after connect()
|
||||
onclose: (() => void) | null = null;
|
||||
constructor(opts: { command: string; args: string[] }) {
|
||||
capturedTransportOpts = { command: opts.command, args: opts.args };
|
||||
}
|
||||
async close() {}
|
||||
},
|
||||
}));
|
||||
|
||||
mock.module('@modelcontextprotocol/sdk/client/index.js', () => ({
|
||||
Client: class FakeClient {
|
||||
constructor() {}
|
||||
async connect() {}
|
||||
async callTool() {
|
||||
return { content: [{ type: 'text', text: '{}' }] };
|
||||
}
|
||||
async close() {}
|
||||
},
|
||||
}));
|
||||
|
||||
mock.module('../../../src/shared/SettingsDefaultsManager.js', () => ({
|
||||
SettingsDefaultsManager: {
|
||||
get: (key: string) => currentSettings[key] ?? '',
|
||||
getInt: () => 0,
|
||||
loadFromFile: () => currentSettings,
|
||||
},
|
||||
}));
|
||||
|
||||
mock.module('../../../src/shared/paths.js', () => ({
|
||||
USER_SETTINGS_PATH: '/tmp/fake-settings.json',
|
||||
}));
|
||||
|
||||
mock.module('../../../src/utils/logger.js', () => ({
|
||||
logger: {
|
||||
info: () => {},
|
||||
debug: () => {},
|
||||
warn: () => {},
|
||||
error: () => {},
|
||||
failure: () => {},
|
||||
},
|
||||
}));
|
||||
|
||||
// ── Now import the module under test ───────────────────────────────────
|
||||
import { ChromaMcpManager } from '../../../src/services/sync/ChromaMcpManager.js';
|
||||
|
||||
// ── Helpers ────────────────────────────────────────────────────────────
|
||||
async function assertSslFlag(sslSetting: string | undefined, expectedValue: string) {
|
||||
currentSettings = { CLAUDE_MEM_CHROMA_MODE: 'remote' };
|
||||
if (sslSetting !== undefined) currentSettings.CLAUDE_MEM_CHROMA_SSL = sslSetting;
|
||||
|
||||
await mgr.callTool('chroma_list_collections', {});
|
||||
|
||||
expect(capturedTransportOpts).not.toBeNull();
|
||||
const sslIdx = capturedTransportOpts!.args.indexOf('--ssl');
|
||||
expect(sslIdx).not.toBe(-1);
|
||||
expect(capturedTransportOpts!.args[sslIdx + 1]).toBe(expectedValue);
|
||||
}
|
||||
|
||||
let mgr: ChromaMcpManager;
|
||||
|
||||
// ── Test suite ─────────────────────────────────────────────────────────
|
||||
describe('ChromaMcpManager SSL flag regression (#1286)', () => {
|
||||
beforeEach(async () => {
|
||||
await ChromaMcpManager.reset();
|
||||
capturedTransportOpts = null;
|
||||
currentSettings = {};
|
||||
mgr = ChromaMcpManager.getInstance();
|
||||
});
|
||||
|
||||
it('emits --ssl false when CLAUDE_MEM_CHROMA_SSL=false', async () => {
|
||||
await assertSslFlag('false', 'false');
|
||||
});
|
||||
|
||||
it('emits --ssl true when CLAUDE_MEM_CHROMA_SSL=true', async () => {
|
||||
await assertSslFlag('true', 'true');
|
||||
});
|
||||
|
||||
it('defaults --ssl false when CLAUDE_MEM_CHROMA_SSL is not set', async () => {
|
||||
await assertSslFlag(undefined, 'false');
|
||||
});
|
||||
|
||||
it('omits --ssl entirely in local mode', async () => {
|
||||
currentSettings = {
|
||||
CLAUDE_MEM_CHROMA_MODE: 'local',
|
||||
};
|
||||
|
||||
await mgr.callTool('chroma_list_collections', {});
|
||||
|
||||
expect(capturedTransportOpts).not.toBeNull();
|
||||
const args = capturedTransportOpts!.args;
|
||||
expect(args).not.toContain('--ssl');
|
||||
expect(args).toContain('--client-type');
|
||||
expect(args[args.indexOf('--client-type') + 1]).toBe('persistent');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user