Skip to content

Commit df38c4a

Browse files
committed
test(sessions): pin the record-ingest span_id contract in the runner persist test
Add a persist test whose fetch stub enforces the API contract (span_id must be a 16-hex OTel span id, else 422), so a regression to a non-span-shaped span_id fails here instead of silently dropping records. The old stub returned 200 for any body and hid the mismatch. Also switch the turn/span tagging test to a real 16-hex span id. Claude-Session: https://claude.ai/code/session_01DnWRxU3dCJ11hgDidm26vW
1 parent 3084aca commit df38c4a

1 file changed

Lines changed: 144 additions & 22 deletions

File tree

services/runner/tests/unit/session-persist.test.ts

Lines changed: 144 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,8 @@ vi.stubGlobal("fetch", async (_url: string, init?: RequestInit) => {
2525
return new Response(JSON.stringify({ ok: true }), { status: 200 });
2626
});
2727

28-
const {
29-
buildPersistingEmitter,
30-
drainPersist,
31-
} = await import("../../src/sessions/persist.ts");
28+
const { buildPersistingEmitter, drainPersist } =
29+
await import("../../src/sessions/persist.ts");
3230

3331
beforeEach(() => {
3432
postedBodies.length = 0;
@@ -89,8 +87,8 @@ describe("buildPersistingEmitter", () => {
8987
const live: unknown[] = [];
9088
const { emit, flush } = buildPersistingEmitter(
9189
"sess-2",
92-
() => "Secret t", (e) =>
93-
live.push(e),
90+
() => "Secret t",
91+
(e) => live.push(e),
9492
);
9593

9694
emit({ type: "message_start", id: "m1" });
@@ -147,8 +145,18 @@ describe("buildPersistingEmitter", () => {
147145

148146
// A tool call streams a growing partial-args snapshot for one id.
149147
emit({ type: "tool_call", id: "call_1", name: "bash", input: {} });
150-
emit({ type: "tool_call", id: "call_1", name: "bash", input: { command: "fi" } });
151-
emit({ type: "tool_call", id: "call_1", name: "bash", input: { command: "find ." } });
148+
emit({
149+
type: "tool_call",
150+
id: "call_1",
151+
name: "bash",
152+
input: { command: "fi" },
153+
});
154+
emit({
155+
type: "tool_call",
156+
id: "call_1",
157+
name: "bash",
158+
input: { command: "find ." },
159+
});
152160
// A different step closes the open call.
153161
emit({ type: "tool_result", id: "call_1", output: "ok" });
154162
emit({ type: "done" });
@@ -173,25 +181,49 @@ describe("buildPersistingEmitter", () => {
173181
});
174182

175183
it("a different tool id flushes the previous open call", async () => {
176-
const { emit, flush } = buildPersistingEmitter("sess-tc2", () => "Secret t");
184+
const { emit, flush } = buildPersistingEmitter(
185+
"sess-tc2",
186+
() => "Secret t",
187+
);
177188

178-
emit({ type: "tool_call", id: "call_a", name: "read", input: { path: "/x" } });
179-
emit({ type: "tool_call", id: "call_b", name: "read", input: { path: "/y" } });
189+
emit({
190+
type: "tool_call",
191+
id: "call_a",
192+
name: "read",
193+
input: { path: "/x" },
194+
});
195+
emit({
196+
type: "tool_call",
197+
id: "call_b",
198+
name: "read",
199+
input: { path: "/y" },
200+
});
180201
await flush();
181202

182203
const bodies = postedBodies as Array<Record<string, unknown>>;
183204
const inputs = bodies.map(
184205
(b) => (b["attributes"] as Record<string, unknown>)["input"],
185206
);
186207
assert.deepEqual(inputs, [{ path: "/x" }, { path: "/y" }]);
187-
assert.deepEqual(bodies.map((b) => b["record_index"]), [0, 1]);
208+
assert.deepEqual(
209+
bodies.map((b) => b["record_index"]),
210+
[0, 1],
211+
);
188212
});
189213

190214
it("flushes an open (paused) tool_call on drain", async () => {
191-
const { emit, flush } = buildPersistingEmitter("sess-tc3", () => "Secret t");
215+
const { emit, flush } = buildPersistingEmitter(
216+
"sess-tc3",
217+
() => "Secret t",
218+
);
192219

193220
// A paused call ends the turn with its slot still open.
194-
emit({ type: "tool_call", id: "call_p", name: "bash", input: { command: "ls" } });
221+
emit({
222+
type: "tool_call",
223+
id: "call_p",
224+
name: "bash",
225+
input: { command: "ls" },
226+
});
195227
await flush();
196228

197229
const bodies = postedBodies as Array<Record<string, unknown>>;
@@ -205,15 +237,28 @@ describe("buildPersistingEmitter", () => {
205237
it("flushes an open tool_call when the idle TTL fires", async () => {
206238
vi.useFakeTimers();
207239
try {
208-
const { emit, flush } = buildPersistingEmitter("sess-tc4", () => "Secret t");
240+
const { emit, flush } = buildPersistingEmitter(
241+
"sess-tc4",
242+
() => "Secret t",
243+
);
209244

210-
emit({ type: "tool_call", id: "call_ttl", name: "bash", input: { command: "x" } });
245+
emit({
246+
type: "tool_call",
247+
id: "call_ttl",
248+
name: "bash",
249+
input: { command: "x" },
250+
});
211251
// Nothing follows; only the TTL can close it.
212252
assert.equal(postedBodies.length, 0);
213253
await vi.advanceTimersByTimeAsync(3000);
214254
assert.equal(postedBodies.length, 1);
215255
assert.equal(
216-
((postedBodies[0] as Record<string, unknown>)["attributes"] as Record<string, unknown>)["type"],
256+
(
257+
(postedBodies[0] as Record<string, unknown>)["attributes"] as Record<
258+
string,
259+
unknown
260+
>
261+
)["type"],
217262
"tool_call",
218263
);
219264
await flush();
@@ -225,13 +270,16 @@ describe("buildPersistingEmitter", () => {
225270

226271
describe("buildPersistingEmitter turn/span tagging", () => {
227272
it("stamps turn_id and span_id on every posted record when both are supplied", async () => {
273+
// A real 16-hex OTel span id (the runContext.trace.span_id shape), NOT a UUID — the API
274+
// types this field as a 16-hex string, so anything else 422s (FINDING-record-ingest-422).
275+
const spanId = "a1b2c3d4e5f6a7b8";
228276
const { emit, persist, flush } = buildPersistingEmitter(
229277
"sess-turn",
230278
() => "Secret t",
231279
undefined,
232280
undefined,
233281
"turn-abc",
234-
"span-def",
282+
spanId,
235283
);
236284

237285
persist({ type: "message", text: "hi" }, "user");
@@ -243,12 +291,15 @@ describe("buildPersistingEmitter turn/span tagging", () => {
243291
assert.equal(bodies.length, 3);
244292
for (const body of bodies) {
245293
assert.equal(body["turn_id"], "turn-abc");
246-
assert.equal(body["span_id"], "span-def");
294+
assert.equal(body["span_id"], spanId);
247295
}
248296
});
249297

250298
it("omits turn_id/span_id from the body when neither is supplied", async () => {
251-
const { emit, flush } = buildPersistingEmitter("sess-noturn", () => "Secret t");
299+
const { emit, flush } = buildPersistingEmitter(
300+
"sess-noturn",
301+
() => "Secret t",
302+
);
252303

253304
emit({ type: "message", text: "hello" });
254305
await flush();
@@ -270,7 +321,12 @@ describe("buildPersistingEmitter turn/span tagging", () => {
270321
emit({ type: "message_start", id: "m1" });
271322
emit({ type: "message_delta", id: "m1", delta: "hi" });
272323
emit({ type: "message_end", id: "m1" });
273-
emit({ type: "tool_call", id: "call_1", name: "bash", input: { command: "ls" } });
324+
emit({
325+
type: "tool_call",
326+
id: "call_1",
327+
name: "bash",
328+
input: { command: "ls" },
329+
});
274330
emit({ type: "tool_result", id: "call_1", output: "ok" });
275331
await flush();
276332

@@ -283,13 +339,79 @@ describe("buildPersistingEmitter turn/span tagging", () => {
283339
});
284340
});
285341

342+
describe("buildPersistingEmitter API contract", () => {
343+
// The API's SessionRecordIngestRequest types span_id as a 16-hex OTel span id. A stub that
344+
// returns 200 for ANY body (like the other tests here) hid the real bug: the field used to
345+
// be typed as UUID and every ingest 422'd. This stub enforces the real contract, so a
346+
// regression to a non-span-shaped span_id fails here instead of silently dropping records.
347+
const OTEL_SPAN_ID = /^[0-9a-fA-F]{16}$/;
348+
349+
it("emits a span_id the API contract accepts (16-hex OTel span id, not a UUID)", async () => {
350+
const accepted: Array<Record<string, unknown>> = [];
351+
const rejected: Array<Record<string, unknown>> = [];
352+
const contractFetch = (async (_url: string, init?: RequestInit) => {
353+
const body = init?.body
354+
? (JSON.parse(init.body as string) as Record<string, unknown>)
355+
: {};
356+
if (
357+
body.span_id !== undefined &&
358+
!OTEL_SPAN_ID.test(String(body.span_id))
359+
) {
360+
rejected.push(body);
361+
return new Response(JSON.stringify({ detail: "uuid_parsing" }), {
362+
status: 422,
363+
});
364+
}
365+
accepted.push(body);
366+
return new Response(JSON.stringify({ ok: true }), { status: 200 });
367+
}) as typeof fetch;
368+
369+
const prior = globalThis.fetch;
370+
globalThis.fetch = contractFetch;
371+
try {
372+
const spanId = "a1b2c3d4e5f6a7b8"; // the runContext.trace.span_id shape
373+
const { emit, persist, flush } = buildPersistingEmitter(
374+
"sess-contract",
375+
() => "Secret t",
376+
undefined,
377+
undefined,
378+
"turn-1",
379+
spanId,
380+
);
381+
persist({ type: "message", text: "hi" }, "user");
382+
emit({ type: "message", text: "hello" });
383+
emit({ type: "done" });
384+
await flush();
385+
386+
// Every record cleared the contract-validating stub; nothing 422'd or dropped.
387+
assert.equal(rejected.length, 0);
388+
assert.equal(accepted.length, 3);
389+
for (const body of accepted)
390+
assert.match(String(body.span_id), OTEL_SPAN_ID);
391+
} finally {
392+
globalThis.fetch = prior;
393+
}
394+
});
395+
396+
it("the contract stub has teeth: a non-span-shaped span_id would 422", () => {
397+
// Guards the test above: the old mock returned 200 for any body, so a UUID (or any
398+
// arbitrary string) sailed through. The real contract rejects both.
399+
assert.equal(OTEL_SPAN_ID.test("a1b2c3d4e5f6a7b8"), true);
400+
assert.equal(OTEL_SPAN_ID.test("span-def"), false);
401+
assert.equal(OTEL_SPAN_ID.test("f".repeat(32)), false); // a UUID (32 hex) is not a span id
402+
});
403+
});
404+
286405
describe("drainPersist", () => {
287406
it("resolves immediately when no events are pending", async () => {
288407
await assert.doesNotReject(() => drainPersist("no-session"));
289408
});
290409

291410
it("waits for all queued events before resolving", async () => {
292-
const { emit, flush } = buildPersistingEmitter("sess-drain", () => "Secret t");
411+
const { emit, flush } = buildPersistingEmitter(
412+
"sess-drain",
413+
() => "Secret t",
414+
);
293415
emit({ type: "message", text: "x" });
294416
emit({ type: "done" });
295417
await flush(); // same as drainPersist internally

0 commit comments

Comments
 (0)