Skip to content

atlas: disable keep-alive on the test/proxy LLM client to fix Premature close#127

Merged
jpr5 merged 1 commit into
mainfrom
fix-atlas-artifact-sync-date-stability
Jun 25, 2026
Merged

atlas: disable keep-alive on the test/proxy LLM client to fix Premature close#127
jpr5 merged 1 commit into
mainfrom
fix-atlas-artifact-sync-date-stability

Conversation

@jpr5

@jpr5 jpr5 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

What

Disable HTTP keep-alive on OpenAIDistiller's client only when a baseURL is configured (in this repo, always a local aimock/proxy server in tests). Production (no baseURL → real OpenAI over HTTPS) keeps the SDK's default keep-alive agent. One-file change in src/atlas/llm.ts.

Root cause

The Static Quality test job went green on 2026-06-23 and red on 2026-06-25 with no material code change — it is a load-dependent flake that turned deterministic under CI's contention, not a date/time bomb.

The Atlas LLM seam (OpenAIDistiller) is exercised against an in-process aimock server through the OpenAI SDK, whose node-fetch transport pools keep-alive sockets. Under the full suite, every aimock-backed test file runs its own server in parallel — doubled by the committed-then-rebuilt dist/ copy. A server can close an idle pooled socket exactly as a request reuses it, which node-fetch surfaces as FetchError: ... Premature close. This affected all aimock-backed suites (atlas-llm, atlas-exclude, atlas-adapter-episodic, atlas-artifact-sync), including the 9 atlas-artifact-sync failures in this report.

Pinning a non-keep-alive http/https Agent for the mock/proxy case forces a fresh socket per request, removing the reuse race.

Verbatim RED (from CI run 28190002185, 2026-06-25)

FAIL src/__tests__/atlas-artifact-sync.test.ts > syncApprovalArtifact (S17) > rejects a checked candidate that an english exclusion rule drops (via aimock)
FAIL src/__tests__/atlas-artifact-sync.test.ts > syncApprovalArtifact (S17) > feeds a checked candidate's CHILD-BLOCK prose to english rules ...
FAIL src/__tests__/atlas-artifact-sync.test.ts > syncApprovalArtifact (S17) > stamps a DATE-ONLY freshness.as_of on a reconstructed badge-less row (X24)
... (9 failed in src + 9 in dist copy)
FetchError: Invalid response body while trying to fetch http://127.0.0.1:45235/v1/chat/completions: Premature close
AssertionError: expected [Function] to throw error including '[atlas/llm] expected a JSON object fr…' but got 'Invalid response body while trying to…'
  Received: "Invalid response body while trying to fetch http://127.0.0.1:37779/v1/chat/completions: Premature close"

Local RED-GREEN proof (real surface: OpenAIDistiller + real aimock server + OpenAI SDK transport)

The race is environment-sensitive (deterministic on CI's contended Ubuntu runner, invisible on a fast 18-core dev box under the natural suite). To prove the fix on the real failure surface, a harness drove a real OpenAIDistiller against a real LLMock through a TCP proxy that reaps idle keep-alive sockets — modeling a server closing a pooled socket as the client reuses it. The ONLY variable is the keep-alive setting the fix toggles:

RED  (keep-alive ON, pre-fix transport):  mode=keepalive ok=23 failures=17   ← socket-reuse race fires
GREEN (non-keep-alive Agent, the fix):    mode=fixed     ok=40 failures=0     ← race eliminated

The SDK masks the torn-socket-on-reuse as "Connection error" locally; CI's node-fetch transport surfaces the same race as "Premature close" — same root cause, same fix.

GREEN: affected suites (src + built dist), post-fix

$ npx vitest run "atlas-artifact-sync" "atlas-llm" "atlas-exclude" "atlas-adapter-episodic"  (src + dist)
 Test Files  8 passed (8)
      Tests  156 passed (156)

The 9 atlas-artifact-sync failures (and the parallel dist copies) all pass.

Other Static Quality gates (run locally, all green)

prettier --check          → All matched files use Prettier code style!
tsc --noEmit -p tsconfig.scripts.json → (no errors)
npm run build             → exit 0
check-version-sync.sh     → ✓ Versions in sync: 1.15.4

Scope

One file (src/atlas/llm.ts). No changes to notification workflows, RAG config, or anything unrelated. dist/ is gitignored and regenerated by CI's npm run build, so no committed compiled copy to sync.

…re close

The Atlas LLM seam (OpenAIDistiller) is exercised in tests against an
in-process aimock server via the OpenAI SDK, whose node-fetch transport
pools keep-alive sockets. Under the full suite — every aimock-backed test
file runs its own server in parallel, doubled by the built dist copy — a
server can close an idle pooled socket exactly as a request reuses it,
which node-fetch surfaces as 'FetchError: ... Premature close'. This made
all aimock-backed suites (atlas-llm, atlas-exclude, atlas-adapter-episodic,
atlas-artifact-sync, ...) fail under load: flaky locally, deterministic in
CI (64 tests, incl. the 9 atlas-artifact-sync failures).

When a baseURL is set (in this repo only ever a local aimock/proxy), pin a
non-keep-alive http/https Agent so each request uses a fresh socket,
removing the reuse race. Production (no baseURL -> real OpenAI over HTTPS)
keeps the SDK default keep-alive agent.
@jpr5 jpr5 merged commit e7e673f into main Jun 25, 2026
8 checks passed
@jpr5 jpr5 deleted the fix-atlas-artifact-sync-date-stability branch June 25, 2026 18:25
jpr5 added a commit that referenced this pull request Jun 25, 2026
## What

Add `vitest.config.ts` scoping test discovery to `src/**`, excluding
`dist/**` (and `.claude/**` worktree checkouts).

## Why

CI currently has no vitest config, so default discovery collects
**both** `src/__tests__/*.test.ts` **and** their compiled
`dist/__tests__/*.test.js` twins — every test runs twice. Beyond the
wasted wall-clock, the duplicate aimock-backed runs double the exposure
to the LLMock socket-reuse race (the "Premature close" failures
addressed by #127).

This change makes each test run once.

## Proof (deterministic)

`vitest list` collection:
- dist test cases collected: **23743 → 0**
- src test cases collected: **3186** (unchanged)

Build green, prettier clean.

## Sequencing

Independent of #127, but its CI is most reliably green once #127's
keep-alive root-cause fix is in `main` — recommend merging after #127.
Rebase + re-run available on request.
@jpr5 jpr5 mentioned this pull request Jun 25, 2026
jpr5 added a commit that referenced this pull request Jun 25, 2026
## Release v1.16.0 (minor)

Releases the user-visible changes already merged to `main`: **#124**
(config validation relaxation) and **#127** (Atlas distiller "Premature
close" runtime fix). Current `1.15.4` → `1.16.0`. #128 was a CI-only
vitest scoping change and is intentionally not in the changelog (does
not ship).

### CHANGELOG

## 1.16.0

### Minor Changes

- Config validation accepts minimal RAG configs — `indexing` and
per-source `chunk` now optional with defaults (#124)
- Fixed Atlas distiller LLM client "Premature close" errors, by
disabling keep-alive on the test/proxy client (#127)

### Version bumps
- `package.json` `version`: 1.15.4 → 1.16.0
- `src/cli.ts` `.version(...)`: 1.15.4 → 1.16.0
- version-sync verified in agreement.

### Publishing
Merging this PR triggers `publish-release.yml` to publish
`@copilotkit/pathfinder@1.16.0` to npm and create the tag/GitHub
Release. Do not tag or release by hand.
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.

1 participant