Skip to content

Commit 449382f

Browse files
authored
Codebase quality sweep: error handling, drift detection, test hygiene (#176)
## Summary Codebase-wide quality sweep addressing 6 categories of issues across 53 files: - **Error handling hardening** — wrap unguarded stream writes in try/catch (recorder.ts, agui-recorder.ts), add response termination for headers-already-sent paths (server.ts, a2a-mock.ts, mcp-mock.ts), fix vector-mock double body consumption - **Parse error detail surfacing** — capture and include the actual JSON.parse error message in all 25+ bare catch blocks across every provider handler, WebSocket handler, and stream-collapse function - **Drift detection gaps** — `compareSSESequences` now compares ALL events per type (not just the first), catching shape variations like Anthropic thinking deltas; Ollama drift tests use synchronous env-var gate instead of broken async `describe.skipIf` - **Test hygiene** — fix 12 unrestored spy/mock leaks across 6 test files, correct misleading "toolCalls should win" comments/assertions, move prototype-level spy to afterEach, update 17 test assertion strings for new error format ## Test plan - [x] 2867 tests pass (79 files, 37 skipped) - [x] TypeScript typecheck clean - [x] Prettier + ESLint clean (pre-commit hooks pass) - [x] 7-agent CR Round 1: 0 new bucket (a) findings from changes; 2 minor items fixed (comment + confirmed non-issue) - [x] 10-slot blitz execution with zero merge conflicts
2 parents 693b1bb + a3d9f95 commit 449382f

54 files changed

Lines changed: 358 additions & 164 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CHANGELOG.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,23 @@
11
# @copilotkit/aimock
22

3+
## [Unreleased]
4+
5+
### Fixed
6+
7+
- **Progressive relay for NDJSON and Bedrock binary event streams** — Ollama NDJSON and Bedrock binary event streams were fully buffered before relay, triggering downstream idle timeouts; now relayed progressively as chunks arrive
8+
- **JSON.parse error detail in bare catch blocks** — capture and surface parse-error detail in all bare catch blocks across 25+ provider/WebSocket/stream-collapse handlers instead of swallowing context
9+
- **Unguarded stream write/end calls** — wrap stream write/end in try/catch (recorder.ts, agui-recorder.ts) to prevent unhandled exceptions on client disconnect
10+
- **Response termination for headers-already-sent paths** — add response termination in error paths where headers were already sent (server.ts, a2a-mock.ts, mcp-mock.ts), preventing connection hangs
11+
- **Vector-mock double body consumption** — fix route passthrough consuming the request body twice, causing empty-body forwarding
12+
- **Drift detection compared only first event per type**`compareSSESequences` now compares ALL events per type, not just the first, catching previously invisible divergences
13+
- **Ollama drift tests used broken async describe.skipIf** — replaced with synchronous env-var gate so tests are correctly skipped or executed
14+
- **12 unrestored spy/mock leaks and misleading assertions** — fix spy/mock leaks across test files and correct assertions that passed for the wrong reasons
15+
16+
### Changed
17+
18+
- **Anti-buffering headers on all progressive stream relay paths** — standard headers (Cache-Control, Connection, X-Accel-Buffering) added to all progressive stream relay paths to prevent intermediate proxy buffering
19+
- **Stream-collapse returns firstDroppedSample** — stream-collapse functions now return the first dropped sample for forensic debugging of collapsed streams
20+
321
## [1.21.0] - 2026-05-11
422

523
### Added

src/__tests__/api-conformance.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1037,7 +1037,7 @@ describe("OpenAI Embeddings API conformance", () => {
10371037
);
10381038
expect(res.status).toBe(400);
10391039
const json = JSON.parse(res.body);
1040-
expect(json.error.message).toBe("Malformed JSON");
1040+
expect(json.error.message).toMatch(/^Malformed JSON body: /);
10411041
});
10421042

10431043
it("Content-Type is application/json", async () => {

src/__tests__/bedrock-stream.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1442,7 +1442,7 @@ describe("POST /model/{modelId}/converse (malformed JSON)", () => {
14421442

14431443
expect(res.status).toBe(400);
14441444
const body = JSON.parse(res.body);
1445-
expect(body.error.message).toBe("Malformed JSON");
1445+
expect(body.error.message).toMatch(/^Malformed JSON: /);
14461446
});
14471447
});
14481448

@@ -1569,7 +1569,7 @@ describe("POST /model/{modelId}/converse-stream (malformed JSON)", () => {
15691569

15701570
expect(res.status).toBe(400);
15711571
const body = JSON.parse(res.body);
1572-
expect(body.error.message).toBe("Malformed JSON");
1572+
expect(body.error.message).toMatch(/^Malformed JSON: /);
15731573
});
15741574
});
15751575

src/__tests__/bedrock.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ describe("POST /model/{modelId}/invoke (error handling)", () => {
243243

244244
expect(res.status).toBe(400);
245245
const body = JSON.parse(res.body);
246-
expect(body.error.message).toBe("Malformed JSON");
246+
expect(body.error.message).toMatch(/^Malformed JSON: /);
247247
});
248248
});
249249

src/__tests__/chaos.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,10 @@ describe("fixture-level chaos on non-OpenAI provider", () => {
590590
// ---------------------------------------------------------------------------
591591

592592
describe("chaos with logLevel silent: invalid header is ignored gracefully", () => {
593+
afterEach(() => {
594+
vi.restoreAllMocks();
595+
});
596+
593597
it("proceeds normally and does not throw when x-aimock-chaos-drop is not a number", async () => {
594598
const fixtures: Fixture[] = [
595599
{ match: { userMessage: "hello" }, response: { content: "Hi there" } },

src/__tests__/cohere.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ describe("POST /v2/chat (validation)", () => {
575575

576576
expect(res.status).toBe(400);
577577
const body = JSON.parse(res.body);
578-
expect(body.error.message).toBe("Malformed JSON");
578+
expect(body.error.message).toMatch(/^Malformed JSON body: /);
579579
});
580580

581581
it("returns 404 when no fixture matches", async () => {

src/__tests__/drift/ollama.drift.ts

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
/**
22
* Ollama drift tests.
33
*
4-
* Compares aimock's Ollama endpoint output shapes against a real local
5-
* Ollama instance. Skips automatically if Ollama is not reachable.
4+
* Compares aimock's Ollama endpoint output shapes against a real Ollama
5+
* instance. Skips unless OLLAMA_HOST is set in the environment.
66
*
7-
* Requires: local Ollama running at http://localhost:11434
7+
* Requires: OLLAMA_HOST env var (e.g. http://localhost:11434)
88
*/
99

1010
import { describe, it, expect, beforeAll, afterAll } from "vitest";
@@ -13,21 +13,10 @@ import { extractShape, triangulate, formatDriftReport } from "./schema.js";
1313
import { httpPost, startDriftServer, stopDriftServer } from "./helpers.js";
1414

1515
// ---------------------------------------------------------------------------
16-
// Connectivity check
16+
// Environment-based opt-in (consistent with other drift files)
1717
// ---------------------------------------------------------------------------
1818

19-
let OLLAMA_REACHABLE = false;
20-
21-
async function checkOllamaConnectivity(): Promise<boolean> {
22-
try {
23-
const res = await fetch("http://localhost:11434/api/tags", {
24-
signal: AbortSignal.timeout(3000),
25-
});
26-
return res.ok;
27-
} catch {
28-
return false;
29-
}
30-
}
19+
const OLLAMA_HOST = process.env.OLLAMA_HOST ?? "http://localhost:11434";
3120

3221
// ---------------------------------------------------------------------------
3322
// Server lifecycle
@@ -36,7 +25,6 @@ async function checkOllamaConnectivity(): Promise<boolean> {
3625
let instance: ServerInstance;
3726

3827
beforeAll(async () => {
39-
OLLAMA_REACHABLE = await checkOllamaConnectivity();
4028
instance = await startDriftServer();
4129
});
4230

@@ -119,7 +107,7 @@ function parseNDJSON(body: string): object[] {
119107
.map((line) => JSON.parse(line) as object);
120108
}
121109

122-
describe.skipIf(!OLLAMA_REACHABLE)("Ollama drift", () => {
110+
describe.skipIf(!process.env.OLLAMA_HOST)("Ollama drift", () => {
123111
it("/api/chat response shape matches", async () => {
124112
const sdkShape = ollamaChatResponseShape();
125113

@@ -130,7 +118,7 @@ describe.skipIf(!OLLAMA_REACHABLE)("Ollama drift", () => {
130118
};
131119

132120
const [realRes, mockRes] = await Promise.all([
133-
httpPost("http://localhost:11434/api/chat", body),
121+
httpPost(`${OLLAMA_HOST}/api/chat`, body),
134122
httpPost(`${instance.url}/api/chat`, body),
135123
]);
136124

@@ -161,7 +149,7 @@ describe.skipIf(!OLLAMA_REACHABLE)("Ollama drift", () => {
161149
};
162150

163151
const [realRes, mockRes] = await Promise.all([
164-
httpPost("http://localhost:11434/api/chat", body),
152+
httpPost(`${OLLAMA_HOST}/api/chat`, body),
165153
httpPost(`${instance.url}/api/chat`, body),
166154
]);
167155

@@ -199,7 +187,7 @@ describe.skipIf(!OLLAMA_REACHABLE)("Ollama drift", () => {
199187
};
200188

201189
const [realRes, mockRes] = await Promise.all([
202-
httpPost("http://localhost:11434/api/generate", body),
190+
httpPost(`${OLLAMA_HOST}/api/generate`, body),
203191
httpPost(`${instance.url}/api/generate`, body),
204192
]);
205193

src/__tests__/drift/schema.ts

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -424,25 +424,23 @@ export function compareSSESequences(
424424
}
425425
}
426426

427-
// Compare shapes of matching event types
427+
// Compare shapes of matching event types — collect ALL events per type and
428+
// merge their shapes so that variant payloads (e.g. text_delta vs
429+
// thinking_delta on the same Anthropic event type) are all represented.
428430
for (const type of realTypeSet) {
429431
if (!mockTypeSet.has(type)) continue;
430-
const realEvent = real.find((e) => e.type === type);
431-
const mockEvent = mock.find((e) => e.type === type);
432-
const sdkEvent = sdk.find((e) => e.type === type);
433-
434-
if (realEvent && mockEvent) {
435-
const eventDiffs = triangulate(
436-
sdkEvent?.dataShape ?? null,
437-
realEvent.dataShape,
438-
mockEvent.dataShape,
439-
);
440-
for (const d of eventDiffs) {
441-
diffs.push({
442-
...d,
443-
path: `SSE:${type}.${d.path}`,
444-
});
445-
}
432+
433+
const realEvents = real.filter((e) => e.type === type);
434+
const mockEvents = mock.filter((e) => e.type === type);
435+
const sdkEvents = sdk.filter((e) => e.type === type);
436+
437+
const realMerged = mergeShapes(realEvents.map((e) => e.dataShape));
438+
const mockMerged = mergeShapes(mockEvents.map((e) => e.dataShape));
439+
const sdkMerged = sdkEvents.length > 0 ? mergeShapes(sdkEvents.map((e) => e.dataShape)) : null;
440+
441+
const eventDiffs = triangulateAt(`SSE:${type}`, sdkMerged, realMerged, mockMerged);
442+
for (const d of eventDiffs) {
443+
diffs.push(d);
446444
}
447445
}
448446

src/__tests__/embeddings.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ describe("POST /v1/embeddings (error handling)", () => {
425425

426426
expect(res.status).toBe(400);
427427
const body = JSON.parse(res.body);
428-
expect(body.error.message).toBe("Malformed JSON");
428+
expect(body.error.message).toMatch(/^Malformed JSON body: /);
429429
expect(body.error.code).toBe("invalid_json");
430430
});
431431
});

src/__tests__/fixture-loader.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@ describe("loadFixtureFile", () => {
4949
});
5050

5151
afterEach(() => {
52+
// Restore console spies that individual tests create via vi.spyOn(console, "warn").
53+
// We cannot use vi.restoreAllMocks() here because the top-level vi.mock("node:fs")
54+
// overrides would also be wiped, breaking subsequent tests.
55+
if ("mockRestore" in console.warn) {
56+
(console.warn as ReturnType<typeof vi.spyOn>).mockRestore();
57+
}
5258
rmSync(tmpDir, { recursive: true, force: true });
5359
});
5460

@@ -360,6 +366,12 @@ describe("loadFixturesFromDir", () => {
360366
});
361367

362368
afterEach(() => {
369+
// Restore console spies that individual tests create via vi.spyOn(console, "warn").
370+
// We cannot use vi.restoreAllMocks() here because the top-level vi.mock("node:fs")
371+
// overrides would also be wiped, breaking subsequent tests.
372+
if ("mockRestore" in console.warn) {
373+
(console.warn as ReturnType<typeof vi.spyOn>).mockRestore();
374+
}
363375
rmSync(tmpDir, { recursive: true, force: true });
364376
});
365377

0 commit comments

Comments
 (0)