Skip to content

test: protect operator telemetry and remove recovery timeout race - #938

Merged
rynfar merged 3 commits into
mainfrom
codex/review-test-isolation
Sep 4, 2026
Merged

test: protect operator telemetry and remove recovery timeout race#938
rynfar merged 3 commits into
mainfrom
codex/review-test-isolation

Conversation

@rynfar

@rynfar rynfar commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Two test-environment problems make the suite unsafe or unreliable: inherited operator configuration can send test cleanup to the operator’s real telemetry database, and the stale-session CAS test cancels its own request after 50 ms while expecting an upstream 500 response.

This preserves Mate Remias’s original #927 fix and authorship in its own commit, then adds two separate maintainer commits:

  • A fresh-process regression test launches the real Bun test runner/preload with operator configuration under both MERIDIAN_* and CLAUDE_PROXY_*. After the child clears test telemetry, the parent verifies that seeded operator metrics and diagnostics survive. Every potentially affected path is disposable. The test also checks suite-owned settings and per-test overrides.
  • The stale-session test uses a 2-second hang guard with guaranteed timer cleanup. A deliberate 75 ms mock response fails with the old 50 ms timer (499) and passes with the corrected guard (500), while preserving the assertion that a second SDK call occurs. This removes a latency race without accepting cancellation or a quarantined source.

Validation at cace2af3:

  • Full local npm test: 3,297 pass, 1 existing skip, 0 fail; typecheck and build pass.
  • Before/after telemetry regression: old preload deletes seeded metrics under both prefixes; fixed preload preserves metrics and diagnostics.
  • Before/after delayed stale-session regression: old timer returns 499; corrected test returns the required 500 and observes two SDK calls.
  • Real Claude Max E41 chain E2E, streaming and non-streaming: both pass, including three tool calls, exact results, durable fork progression, follow-up resume, and cached-prefix continuity. Repeated after the final test correction.
  • Linux CI test/typecheck/build, Windows smoke, container smoke, and container build all pass.

One separate full-suite run returned 500 for a concurrent priority request after mock SDK completion. Thirty repetitions of the affected file (90 tests) did not reproduce it. This change fixes the demonstrated stale-session timing race, but does not claim that all of #917/#933 is resolved. Kept as a draft first review batch; no runtime code changes.

materemias and others added 3 commits September 4, 2026 13:05
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.
@rynfar rynfar changed the title test: isolate operator configuration and prove telemetry survives test: protect operator telemetry and remove recovery timeout race Sep 4, 2026
@rynfar
rynfar marked this pull request as ready for review September 4, 2026 19:24
@rynfar
rynfar merged commit b539f99 into main Sep 4, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants