fix(config): strip inline # comments + route /config/flags through merged env (#200, #201)

[rohitg00]

Summary

Two related env-loading bugs that combined to make valid `~/.agentmemory/.env` configs silently behave as if half the user's flags were unset.

#200 — `.env` loader doesn't strip inline `#` comments

`loadEnvFile()` only handled full-line `#` comments. Trailing inline comments (a natural urge when annotating the boolean toggles) survived into the value, so the strict `=== "true"` checks downstream always returned false:

```env
AGENTMEMORY_AUTO_COMPRESS=true # opt in to LLM compression → parsed as 'true # opt in to LLM compression' → off
```

Fixed by stripping a trailing ` #...` segment outside quotes. Preserves `#` inside double/single quoted values and `#` as a non-leading character of unquoted values.

#201 — `/config/flags` reads `process.env` directly

`api::config-flags` read provider keys from `process.env` 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 showed the prominent "No LLM provider key set" warning.

Fixed by routing through the existing `getEnvVar()` helper.

Tests

• New `test/env-loader.test.ts` (4 cases) covering inline-comment stripping, quoted-value preservation, embedded-hash unquoted values
• Full suite: 831/831 passing

Thanks @bloodcarter for the precise repros and source-pointers in #200/#201.

Closes #200
Closes #201

Test plan

• Cold-start with `~/.agentmemory/.env` containing inline `#` comments — flags must boot `enabled: true`
• Cold-start with provider key only in `~/.agentmemory/.env` (not exported) — viewer banner must NOT warn

Summary by CodeRabbit

• New Features

  • Added automatic detection of whether an LLM provider is configured, impacting feature availability.

• Bug Fixes

  • Environment configuration parsing now correctly handles inline comments and quoted values, avoiding accidental inclusion of comment text.

• Tests

  • Added comprehensive tests for env-file parsing covering comment stripping and quote behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b0cc5428-ecd4-436e-8987-6776688aad52

📥 Commits

Reviewing files that changed from the base of the PR and between 5dc80d0 and df91d47.

📒 Files selected for processing (3)

• src/config.ts
• src/triggers/api.ts
• test/env-loader.test.ts

---

📝 Walkthrough

Walkthrough

loadEnvFile() now correctly strips inline # comments for unquoted values while preserving # inside quoted values. A new exported helper, detectLlmProviderKind(), centralizes detection of configured LLM providers and is used by the API flags endpoint. A Vitest suite was added to validate parsing behavior.

Changes

Cohort / File(s)	Summary
Env parsing & helper src/config.ts	Updated .env parsing to respect surrounding quotes and strip trailing inline # ... only for unquoted values; added `export function detectLlmProviderKind(): "llm"
API flags endpoint src/triggers/api.ts	Replaced direct process.env provider-key checks with the centralized detection/helper so the /agentmemory/config/flags response reflects .env files loaded by the app.
Tests for env loader test/env-loader.test.ts	Added Vitest tests creating an isolated sandbox .env to assert: inline # comments are stripped for unquoted values, # preserved inside single/double quotes, abc#def (no space) remains part of value, and quoted values are unwrapped and trimmed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nibbled a line where a hash used to hide,
Quotes kept their secrets, comments slipped aside.
Flags now speak true, no silent surprise,
Env files behave under moonlit skies.
Thump, hop, deploy—🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Title clearly identifies the two fixes: inline comment stripping and provider detection routing, with issue references.
Linked Issues check	✅ Passed	Changes directly address both #200 and #201: inline comment stripping in loadEnvFile, and routing provider detection through merged env via detectLlmProviderKind().
Out of Scope Changes check	✅ Passed	All changes are scoped to the stated objectives: env loader parsing, provider detection helper, and comprehensive test coverage for comment-stripping edge cases.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/200-201-env-loading

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

test/env-loader.test.ts (1)

40-70: Solid coverage for the documented behavior — consider one extra case.

The four cases cover the three rules called out in the PR description. To pin down the edge case flagged in src/config.ts (quoted value followed by an inline comment), a fifth assertion would be valuable and would have caught the regression noted there:

it("strips inline comment after a quoted value and unwraps quotes", async () => {
  writeEnv('TOKEN="abc" # trailing');
  const cfg = await freshConfig();
  expect(cfg.getEnvVar("TOKEN")).toBe("abc");
});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/env-loader.test.ts` around lines 40 - 70, Add a fifth test to cover the
edge case of a quoted value followed by an inline comment: using writeEnv with a
line like 'TOKEN="abc" # trailing', call freshConfig(), then assert
cfg.getEnvVar("TOKEN") === "abc"; place this alongside the other tests (use the
same async it block style and descriptive title like 'strips inline comment
after a quoted value and unwraps quotes') so the parser behavior in
src/config.ts is validated for quoted values followed by comments.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/config.ts`:
- Around line 33-41: The current logic in src/config.ts checks for quoted values
before stripping inline comments, so inputs like TOKEN="abc" # comment keep
their quotes; fix by first removing inline comments from val (e.g., strip the
first occurrence of a " #"/whitespace-plus-# and anything after it) and then
re-evaluate the quoted check and slice quotes if present; update the block that
computes quoted and mutates val (the variables named val and quoted) to perform
comment-stripping before the quoted boolean check (optionally use a regex for
whitespace+hash like /\s#/) so quoted values followed by comments are unwrapped
correctly.

In `@src/triggers/api.ts`:
- Around line 157-163: The provider detection in the providerKind assignment
diverges from detectProvider(): it omits GOOGLE_API_KEY and doesn't use
hasRealValue(), causing inconsistent provider state; change provider detection
to reuse the existing detectProvider() logic (or export a new helper like
detectLlmProvider() from the config module) instead of re-checking env keys
here, and ensure the check uses hasRealValue() semantics rather than raw
getEnvVar() so whitespace-only keys are treated as absent (update references to
providerKind and the getEnvVar checks accordingly).

In `@test/env-loader.test.ts`:
- Around line 6-7: The test saves ORIGINAL_HOME and ORIGINAL_USERPROFILE but
restoring them by assigning process.env["HOME"] = ORIGINAL_HOME (and
USERPROFILE) will set the literal string "undefined" if they were initially
undefined; change the restore logic to only assign when
ORIGINAL_HOME/ORIGINAL_USERPROFILE !== undefined and otherwise delete
process.env["HOME"] / delete process.env["USERPROFILE"]; update the restore code
paths around the symbols ORIGINAL_HOME and ORIGINAL_USERPROFILE (and
corresponding teardown code referenced at lines 34-38) to perform conditional
assignment or deletion so undefined values are not stringified.

---

Nitpick comments:
In `@test/env-loader.test.ts`:
- Around line 40-70: Add a fifth test to cover the edge case of a quoted value
followed by an inline comment: using writeEnv with a line like 'TOKEN="abc" #
trailing', call freshConfig(), then assert cfg.getEnvVar("TOKEN") === "abc";
place this alongside the other tests (use the same async it block style and
descriptive title like 'strips inline comment after a quoted value and unwraps
quotes') so the parser behavior in src/config.ts is validated for quoted values
followed by comments.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4532db8b-aaba-4963-9b3a-3cef43efcdfd

📥 Commits

Reviewing files that changed from the base of the PR and between 18e3257 and 5dc80d0.

📒 Files selected for processing (3)

• src/config.ts
• src/triggers/api.ts
• test/env-loader.test.ts