From 5dc80d0e05c1edb3d74024079bb5d7c75daa24aa Mon Sep 17 00:00:00 2001 From: Rohit Ghumare Date: Mon, 27 Apr 2026 00:13:59 +0100 Subject: [PATCH 1/2] fix(config): strip inline `#` comments and route /config/flags through merged env MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related env-loading bugs that combined to make valid `~/.agentmemory/.env` configs silently behave as if half the user's flags were unset: 1. `loadEnvFile()` only stripped full-line `#` comments. Trailing inline comments — natural when annotating boolean toggles like `AGENTMEMORY_AUTO_COMPRESS=true # opt in` — survived into the value, so the strict `=== "true"` checks in `isAutoCompressEnabled()`, `isConsolidationEnabled()`, and `isGraphExtractionEnabled()` always returned false. Now strips ` #` outside quotes; preserves `#` inside quoted values and as a non-leading character of unquoted values. 2. `api::config-flags` (`GET /agentmemory/config/flags`) read provider keys from `process.env` directly instead of `getMergedEnv()`. With the key set in `~/.agentmemory/.env` (the README's recommended path) the provider was actually working, but the diagnostics endpoint reported `provider: "noop"` and the viewer falsely warned "No LLM provider key set". Now reads via `getEnvVar()` so both surfaces agree. Adds `test/env-loader.test.ts` covering inline-comment stripping, quoted-value preservation, and embedded-hash values. Thanks @bloodcarter for the precise repros and root-cause pointers. Closes #200 Closes #201 --- src/config.ts | 9 ++++-- src/triggers/api.ts | 10 ++++-- test/env-loader.test.ts | 71 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 5 deletions(-) create mode 100644 test/env-loader.test.ts diff --git a/src/config.ts b/src/config.ts index 09bf229..733ecf7 100644 --- a/src/config.ts +++ b/src/config.ts @@ -30,11 +30,14 @@ function loadEnvFile(): Record { if (eqIdx === -1) continue; const key = trimmed.slice(0, eqIdx).trim(); let val = trimmed.slice(eqIdx + 1).trim(); - if ( + const quoted = (val.startsWith('"') && val.endsWith('"')) || - (val.startsWith("'") && val.endsWith("'")) - ) { + (val.startsWith("'") && val.endsWith("'")); + if (quoted) { val = val.slice(1, -1); + } else { + const hashIdx = val.indexOf(" #"); + if (hashIdx !== -1) val = val.slice(0, hashIdx).trim(); } vars[key] = val; } diff --git a/src/triggers/api.ts b/src/triggers/api.ts index 3c993f5..dc33b17 100644 --- a/src/triggers/api.ts +++ b/src/triggers/api.ts @@ -14,6 +14,7 @@ import { isAutoCompressEnabled, isContextInjectionEnabled, detectEmbeddingProvider, + getEnvVar, } from "../config.js"; type Response = { @@ -153,8 +154,13 @@ export function registerApiTriggers( async (req: ApiRequest): Promise => { const authErr = checkAuth(req, secret); if (authErr) return authErr; - const env = process.env; - const providerKind = env["ANTHROPIC_API_KEY"] || env["GEMINI_API_KEY"] || env["OPENROUTER_API_KEY"] || env["MINIMAX_API_KEY"] ? "llm" : "noop"; + const providerKind = + getEnvVar("ANTHROPIC_API_KEY") || + getEnvVar("GEMINI_API_KEY") || + getEnvVar("OPENROUTER_API_KEY") || + getEnvVar("MINIMAX_API_KEY") + ? "llm" + : "noop"; const embeddingProvider = detectEmbeddingProvider() ? "embeddings" : "none"; const flags = [ { diff --git a/test/env-loader.test.ts b/test/env-loader.test.ts new file mode 100644 index 0000000..ceb8c32 --- /dev/null +++ b/test/env-loader.test.ts @@ -0,0 +1,71 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { mkdtempSync, writeFileSync, mkdirSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +const ORIGINAL_HOME = process.env["HOME"]; +const ORIGINAL_USERPROFILE = process.env["USERPROFILE"]; + +let sandboxHome: string; + +async function freshConfig() { + vi.resetModules(); + return await import("../src/config.js"); +} + +function writeEnv(contents: string) { + const dir = join(sandboxHome, ".agentmemory"); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, ".env"), contents); +} + +describe("loadEnvFile", () => { + beforeEach(() => { + sandboxHome = mkdtempSync(join(tmpdir(), "agentmemory-env-")); + process.env["HOME"] = sandboxHome; + process.env["USERPROFILE"] = sandboxHome; + delete process.env["AGENTMEMORY_AUTO_COMPRESS"]; + delete process.env["CONSOLIDATION_ENABLED"]; + delete process.env["GRAPH_EXTRACTION_ENABLED"]; + delete process.env["TOKEN"]; + delete process.env["HASHVAL"]; + }); + + afterEach(() => { + process.env["HOME"] = ORIGINAL_HOME; + process.env["USERPROFILE"] = ORIGINAL_USERPROFILE; + rmSync(sandboxHome, { recursive: true, force: true }); + }); + + it("strips trailing inline # comments on unquoted values", async () => { + writeEnv( + [ + "AGENTMEMORY_AUTO_COMPRESS=true # opt in to LLM compression", + "CONSOLIDATION_ENABLED=true # daily summarization", + "GRAPH_EXTRACTION_ENABLED=true # entity graph", + ].join("\n"), + ); + const cfg = await freshConfig(); + expect(cfg.isAutoCompressEnabled()).toBe(true); + expect(cfg.isConsolidationEnabled()).toBe(true); + expect(cfg.isGraphExtractionEnabled()).toBe(true); + }); + + it("preserves # inside double-quoted values", async () => { + writeEnv('TOKEN="abc#def"'); + const cfg = await freshConfig(); + expect(cfg.getEnvVar("TOKEN")).toBe("abc#def"); + }); + + it("preserves # inside single-quoted values", async () => { + writeEnv("TOKEN='abc#def'"); + const cfg = await freshConfig(); + expect(cfg.getEnvVar("TOKEN")).toBe("abc#def"); + }); + + it("treats hash without leading space as part of value", async () => { + writeEnv("HASHVAL=abc#def"); + const cfg = await freshConfig(); + expect(cfg.getEnvVar("HASHVAL")).toBe("abc#def"); + }); +}); From df91d473624cb0d4773cfcd44729777100a8f509 Mon Sep 17 00:00:00 2001 From: Rohit Ghumare Date: Mon, 27 Apr 2026 00:57:49 +0100 Subject: [PATCH 2/2] fix(config): handle quoted values with trailing inline comments + reuse provider detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three follow-ups from CodeRabbit on the earlier .env-loading patch: 1. **Quoted values followed by inline comments** — \`TOKEN="abc" # note\` parsed as the literal string \`"abc"\` because the round-trip checked "ends with quote" before stripping the trailing comment. Reworked to detect the opening quote, find the matching close quote, and slice between them; falls through to the existing \` #\` strip for unquoted values. Adds two regression tests covering both single and double quotes followed by comments. 2. **Provider detection diverged from \`detectProvider()\`** — the \`api::config-flags\` helper missed \`GOOGLE_API_KEY\` (which \`detectProvider\` accepts as an alias for \`GEMINI_API_KEY\`) and used raw \`getEnvVar()\` instead of the trimmed \`hasRealValue()\` semantics, so whitespace-only keys read as set. Extracted a single \`detectLlmProviderKind()\` helper in \`src/config.ts\` that mirrors \`detectProvider\`'s checks; \`api::config-flags\` now calls it. 3. **Test env restore stringified \`undefined\`** — when \`HOME\` or \`USERPROFILE\` was unset on the host, \`process.env[k] = ORIGINAL\` coerced to the literal string \`"undefined"\`. Conditional restore (delete vs assign) preserves the original missing-state. --- src/config.ts | 23 ++++++++++++++++++----- src/triggers/api.ts | 10 ++-------- test/env-loader.test.ts | 18 ++++++++++++++++-- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/config.ts b/src/config.ts index 733ecf7..2898552 100644 --- a/src/config.ts +++ b/src/config.ts @@ -30,11 +30,10 @@ function loadEnvFile(): Record { if (eqIdx === -1) continue; const key = trimmed.slice(0, eqIdx).trim(); let val = trimmed.slice(eqIdx + 1).trim(); - const quoted = - (val.startsWith('"') && val.endsWith('"')) || - (val.startsWith("'") && val.endsWith("'")); - if (quoted) { - val = val.slice(1, -1); + const quoteChar = val[0] === '"' || val[0] === "'" ? val[0] : ""; + if (quoteChar) { + const closeIdx = val.indexOf(quoteChar, 1); + if (closeIdx !== -1) val = val.slice(1, closeIdx); } else { const hashIdx = val.indexOf(" #"); if (hashIdx !== -1) val = val.slice(0, hashIdx).trim(); @@ -150,6 +149,20 @@ export function getEnvVar(key: string): string | undefined { return getMergedEnv()[key]; } +export function detectLlmProviderKind(): "llm" | "noop" { + const env = getMergedEnv(); + if ( + hasRealValue(env["ANTHROPIC_API_KEY"]) || + hasRealValue(env["GEMINI_API_KEY"]) || + hasRealValue(env["GOOGLE_API_KEY"]) || + hasRealValue(env["OPENROUTER_API_KEY"]) || + hasRealValue(env["MINIMAX_API_KEY"]) + ) { + return "llm"; + } + return "noop"; +} + export function loadEmbeddingConfig(): EmbeddingConfig { const env = getMergedEnv(); let bm25Weight = parseFloat(env["BM25_WEIGHT"] || "0.4"); diff --git a/src/triggers/api.ts b/src/triggers/api.ts index dc33b17..884c284 100644 --- a/src/triggers/api.ts +++ b/src/triggers/api.ts @@ -14,7 +14,7 @@ import { isAutoCompressEnabled, isContextInjectionEnabled, detectEmbeddingProvider, - getEnvVar, + detectLlmProviderKind, } from "../config.js"; type Response = { @@ -154,13 +154,7 @@ export function registerApiTriggers( async (req: ApiRequest): Promise => { const authErr = checkAuth(req, secret); if (authErr) return authErr; - const providerKind = - getEnvVar("ANTHROPIC_API_KEY") || - getEnvVar("GEMINI_API_KEY") || - getEnvVar("OPENROUTER_API_KEY") || - getEnvVar("MINIMAX_API_KEY") - ? "llm" - : "noop"; + const providerKind = detectLlmProviderKind(); const embeddingProvider = detectEmbeddingProvider() ? "embeddings" : "none"; const flags = [ { diff --git a/test/env-loader.test.ts b/test/env-loader.test.ts index ceb8c32..9c6f295 100644 --- a/test/env-loader.test.ts +++ b/test/env-loader.test.ts @@ -32,8 +32,10 @@ describe("loadEnvFile", () => { }); afterEach(() => { - process.env["HOME"] = ORIGINAL_HOME; - process.env["USERPROFILE"] = ORIGINAL_USERPROFILE; + if (ORIGINAL_HOME === undefined) delete process.env["HOME"]; + else process.env["HOME"] = ORIGINAL_HOME; + if (ORIGINAL_USERPROFILE === undefined) delete process.env["USERPROFILE"]; + else process.env["USERPROFILE"] = ORIGINAL_USERPROFILE; rmSync(sandboxHome, { recursive: true, force: true }); }); @@ -68,4 +70,16 @@ describe("loadEnvFile", () => { const cfg = await freshConfig(); expect(cfg.getEnvVar("HASHVAL")).toBe("abc#def"); }); + + it("strips inline comment after a quoted value and unwraps quotes", async () => { + writeEnv('TOKEN="abc" # trailing comment'); + const cfg = await freshConfig(); + expect(cfg.getEnvVar("TOKEN")).toBe("abc"); + }); + + it("strips inline comment after a single-quoted value and unwraps quotes", async () => { + writeEnv("TOKEN='abc' # trailing comment"); + const cfg = await freshConfig(); + expect(cfg.getEnvVar("TOKEN")).toBe("abc"); + }); });