test: isolate the suite from the operator's meridian environment - #927
test: isolate the suite from the operator's meridian environment#927materemias wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The change is confined to the test preload and directly enforces deterministic, non-destructive test isolation without altering production behavior.
Pull request overview
This PR hardens the test harness so local npm test runs are isolated from any MERIDIAN_* / legacy CLAUDE_PROXY_* environment variables a developer may have exported for running their own proxy, preventing silent test reconfiguration and (notably) accidental writes/deletes against a real on-disk telemetry DB.
Changes:
- Clears the entire
MERIDIAN_*andCLAUDE_PROXY_*env var namespaces in the test preload before the suite config is applied. - Continues to set suite-owned paths/knobs (config dir, session dir, SDK process gate) to throwaway values for test isolation.
File summaries
| File | Description |
|---|---|
src/__tests__/preload.ts |
Removes operator configuration influence by stripping MERIDIAN_* / CLAUDE_PROXY_* env vars before setting test-owned env state. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Every runtime knob is read through `env()` — `MERIDIAN_<X>` with a
`CLAUDE_PROXY_<X>` fallback — so anything a developer exports to run their
own proxy silently reconfigures the code under test. Two real cases on one
machine:
- `MERIDIAN_NO_FILE_CHANGES=1` disables the PostToolUse hook, failing the
three "other adapters still track" cases in proxy-file-changes.test.ts.
- `MERIDIAN_TELEMETRY_PERSIST=1` makes the global telemetry store the real
~/.config/meridian/telemetry.db, which the suite then DELETEs: tests call
`telemetryStore.clear()` and `diagnosticLog.clear()`, so a local run
destroys every metric and diagnostic the operator's proxy has recorded.
Redirecting MERIDIAN_CONFIG_DIR does not save it — telemetry resolves its
path from `env("TELEMETRY_DB")` alone.
Strip the whole namespace in the preload, before any module import. CI runs
with none of it set, so this is what makes a local run mean the same thing
as a CI run; tests that need a knob set it themselves, in-process.
53ffd0a to
ee086a7
Compare
|
Your The 11 failures here ( This branch predates that fix, so it is still running against the broken harness. Apologies for the delay in getting you CI signal on this — the approval was held while the flake made every result untrustworthy. |
Problem
Every runtime knob is read through
env()—MERIDIAN_<X>with aCLAUDE_PROXY_<X>fallback — so anything a developer exports to run their own proxy silently reconfigures the code under test. The suite inherits it, and CI (which exports none of it) cannot see the difference.Two cases observed on one machine, both from a normal operator environment:
MERIDIAN_NO_FILE_CHANGES=1disables the PostToolUse file-change hook, so the three "File change visibility: other adapters still track" cases insrc/__tests__/proxy-file-changes.test.tsfail onmain— with no local change in sight.MERIDIAN_TELEMETRY_PERSIST=1makes the process-global telemetry store the real~/.config/meridian/telemetry.db. The suite is written against the in-memory store and callstelemetryStore.clear()/diagnosticLog.clear(), which issueDELETE FROM metrics/DELETE FROM diagnostic_logs— so runningnpm testdestroys every metric and diagnostic log the operator's own proxy has recorded. RedirectingMERIDIAN_CONFIG_DIRdoes not help:src/telemetry/index.tsresolves its path fromenv("TELEMETRY_DB")and otherwise hard-codes~/.config/meridian/telemetry.db.The second one is data loss, and it is silent.
Fix
Strip the
MERIDIAN_*/CLAUDE_PROXY_*namespace insrc/__tests__/preload.ts, before any module import. The preload then sets the values the suite owns (config dir, session dir, SDK process gate) as it already did.CI runs with none of these set, so this is precisely what makes a local run mean the same thing as a CI run. Tests that need a knob set it themselves, in-process, after the preload.
Verification
npm teston this branch: all 13 invocations green (2982 + 251 tests, 0 fail). Before it, on the same machine,mainreported 3 failures inproxy-file-changes.test.tsand wiped the local telemetry database.