Skip to content

Commit 52d1b9b

Browse files
authored
Embed testId in Veo/Grok video poll ids for multi-tenant isolation (#278) (#286)
## What Completes the #278 async-video providers (Veo, Grok) for **multi-tenant use**: each provider now embeds the `testId` into the poll/operation id it returns, so a poll that carries the testId only in the URL (not in a header) still resolves to the correct tenant's in-flight job. Mirrors the existing OpenRouter `polling_url` testId-embedding pattern. No version bump — stays `1.34.0`; the dedicated `chore: release v1.35.0` PR owns the bump. ## Why When the original #278 video work landed, the submit→poll round-trip relied on the testId arriving via header on the poll. Real provider SDKs poll the returned operation/job URL directly and don't necessarily re-send a custom header, so under concurrent multi-tenant use a header-less poll fell to the default testId → key miss → 404 (the job was stored under `${testId}:${id}`). Embedding the testId in the returned id makes it travel with the poll URL. ## Changes - `src/veo-video.ts` / `src/grok-video.ts`: append `?testId=<encoded>` to the returned poll/operation id (only when a non-default testId is present — single-tenant flows still return a bare id). The router strips the query before route-matching; `getTestId` re-derives the testId from the query when no header is present. - Tests: a decisive cross-tenant routing test in both suites — two tenants submit jobs concurrently, and each tenant's **header-less** poll of its returned suffix-bearing id resolves **its own** job (asserts the specific payload, not just "not 404"). Replaces an earlier test that was tautological (passed even pre-fix). Suffix is asserted with an anchored `?testId=<encoded>` regex; back-compat (default testId → bare id) retained. ## Test plan - Full suite: **144 files / 4250 tests** pass; `tsc`, `eslint`, `prettier --check`, `build`, `test:exports` (node10/node16-CJS/ESM/bundler) all green. - Red-green proven on the real submit+poll surface: reverting only `veo-video.ts`/`grok-video.ts` makes both concurrent-tenant tests fail (header-less poll 404s); restoring passes 31/31. ## Notes No cross-tenant data-leak existed pre-fix (a missing/mismatched testId yields a key miss → loud 404, never a tenant-agnostic scan); this change is about header-less polls resolving **correctly** per tenant, confirmed by an independent silent-failure review.
2 parents 2cdbea3 + 3f584c6 commit 52d1b9b

4 files changed

Lines changed: 266 additions & 6 deletions

File tree

src/__tests__/grok-video.test.ts

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,123 @@ describe("GET /v1/videos/{id} dispatch journals handler errors as 500 (Grok-firs
363363
});
364364
});
365365

366+
// ─── Multi-tenant poll isolation (#278) ──────────────────────────────────────
367+
// Submit stores the job keyed `${testId}:${requestId}`, but the returned
368+
// request_id is opaque. A multi-tenant client polls the returned id WITHOUT an
369+
// x-test-id header, so the testId must travel in the returned id (via
370+
// testIdSuffix) for getTestId's `?testId=` fallback to resolve the scope —
371+
// mirroring OpenRouter's polling_url treatment.
372+
373+
describe("Grok video — testId scoping of returned request_id", () => {
374+
let mock: LLMock | undefined;
375+
376+
afterEach(async () => {
377+
await mock?.stop();
378+
mock = undefined;
379+
});
380+
381+
test("returned request_id carries testId and resolves the poll without the header", async () => {
382+
mock = new LLMock({ port: 0 });
383+
mock.addFixture({
384+
match: { userMessage: "scoped poll", endpoint: "video" },
385+
response: { video: { id: "vid_scoped", status: "completed", url: "https://cdn.x.ai/s.mp4" } },
386+
});
387+
await mock.start();
388+
389+
const submit = await fetch(`${mock.url}/v1/videos/generations`, {
390+
method: "POST",
391+
headers: { "Content-Type": "application/json", "X-Test-Id": "test-a" },
392+
body: JSON.stringify({ model: "grok-imagine-video", prompt: "scoped poll" }),
393+
});
394+
const { request_id } = (await submit.json()) as { request_id: string };
395+
// Exact query param, not just a substring: `{uuid}?testId=test-a`.
396+
expect(request_id).toMatch(/^[^?]+\?testId=test-a$/);
397+
398+
// Bare poll of the returned id — no X-Test-Id header — must still resolve
399+
// via the embedded query parameter.
400+
const poll = await fetch(`${mock.url}/v1/videos/${request_id}`);
401+
expect(poll.status).toBe(200);
402+
const data = await poll.json();
403+
expect(data.status).toBe("done");
404+
});
405+
406+
// DECISIVE multi-tenant routing (#278). Two tenants submit DISTINCT jobs
407+
// concurrently — both live in the map at once — then each polls ONLY its own
408+
// RETURNED suffix-bearing request_id, sending NO x-test-id header. The
409+
// header-less poll must resolve via the embedded `?testId=` suffix to that
410+
// tenant's own job (not the other tenant's, not a Sora 404 fall-through).
411+
// This FAILS on pre-fix code: a bare returned request_id carries no testId, so
412+
// a header-less poll falls to the default testId, the `${testId}:${requestId}`
413+
// key misses, and the Grok-first dispatch falls through to Sora → 404. The
414+
// earlier header-stripping "isolation" test was tautological (it 404s pre- AND
415+
// post-fix); this one is the real cross-tenant routing proof.
416+
test("concurrent tenants each resolve their OWN job via the header-less suffix", async () => {
417+
mock = new LLMock({ port: 0 });
418+
mock.addFixture({
419+
match: { userMessage: "tenant A clip", endpoint: "video" },
420+
response: { video: { id: "vid_A", status: "completed", url: "https://cdn.x.ai/a.mp4" } },
421+
});
422+
mock.addFixture({
423+
match: { userMessage: "tenant B clip", endpoint: "video" },
424+
response: { video: { id: "vid_B", status: "completed", url: "https://cdn.x.ai/b.mp4" } },
425+
});
426+
await mock.start();
427+
428+
// Both tenants submit concurrently → both jobs coexist in the map.
429+
const [resA, resB] = await Promise.all([
430+
fetch(`${mock.url}/v1/videos/generations`, {
431+
method: "POST",
432+
headers: { "Content-Type": "application/json", "X-Test-Id": "tenant-a" },
433+
body: JSON.stringify({ model: "grok-imagine-video", prompt: "tenant A clip" }),
434+
}),
435+
fetch(`${mock.url}/v1/videos/generations`, {
436+
method: "POST",
437+
headers: { "Content-Type": "application/json", "X-Test-Id": "tenant-b" },
438+
body: JSON.stringify({ model: "grok-imagine-video", prompt: "tenant B clip" }),
439+
}),
440+
]);
441+
const { request_id: idA } = (await resA.json()) as { request_id: string };
442+
const { request_id: idB } = (await resB.json()) as { request_id: string };
443+
expect(idA).toMatch(/^[^?]+\?testId=tenant-a$/);
444+
expect(idB).toMatch(/^[^?]+\?testId=tenant-b$/);
445+
// Distinct underlying request ids (no accidental collision).
446+
expect(idA.split("?")[0]).not.toBe(idB.split("?")[0]);
447+
448+
// Header-less poll of A's returned id → resolves A's job (a.mp4), not B's.
449+
const pollA = await fetch(`${mock.url}/v1/videos/${idA}`);
450+
expect(pollA.status).toBe(200);
451+
const dataA = await pollA.json();
452+
expect(dataA.status).toBe("done");
453+
expect(dataA.video.url).toBe("https://cdn.x.ai/a.mp4");
454+
455+
// Header-less poll of B's returned id → resolves B's job (b.mp4), not A's.
456+
const pollB = await fetch(`${mock.url}/v1/videos/${idB}`);
457+
expect(pollB.status).toBe(200);
458+
const dataB = await pollB.json();
459+
expect(dataB.status).toBe("done");
460+
expect(dataB.video.url).toBe("https://cdn.x.ai/b.mp4");
461+
});
462+
463+
test("default testId returns the bare request_id (no testId param)", async () => {
464+
mock = new LLMock({ port: 0 });
465+
mock.addFixture({
466+
match: { userMessage: "clean id", endpoint: "video" },
467+
response: { video: { id: "vid_clean", status: "completed", url: "https://cdn.x.ai/c.mp4" } },
468+
});
469+
await mock.start();
470+
471+
const submit = await fetch(`${mock.url}/v1/videos/generations`, {
472+
method: "POST",
473+
headers: { "Content-Type": "application/json" },
474+
body: JSON.stringify({ model: "grok-imagine-video", prompt: "clean id" }),
475+
});
476+
const { request_id } = (await submit.json()) as { request_id: string };
477+
// Byte-identical to pre-fix behaviour: bare uuid, no query.
478+
expect(request_id).not.toContain("testId=");
479+
expect(request_id).not.toContain("?");
480+
});
481+
});
482+
366483
// Type-only smoke: the stored video status union remains the 3-value set
367484
// (no "done"); the Grok wire "done" is derived at serialization.
368485
describe("VideoResponse stored status", () => {

src/__tests__/veo-video.test.ts

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,3 +278,132 @@ describe("GET /v1beta/operations/{name} dispatch journals handler errors as 500
278278
expect(failed[0].method).toBe("GET");
279279
});
280280
});
281+
282+
// ─── Multi-tenant poll isolation (#278) ──────────────────────────────────────
283+
// Submit stores the job keyed `${testId}:${operationName}`, but the returned
284+
// operation name is opaque. A multi-tenant client polls the returned name
285+
// WITHOUT an x-test-id header, so the testId must travel in the returned name
286+
// (via testIdSuffix) for getTestId's `?testId=` fallback to resolve the scope —
287+
// mirroring OpenRouter's polling_url treatment.
288+
289+
describe("Veo video — testId scoping of returned operation name", () => {
290+
let mock: LLMock | undefined;
291+
292+
afterEach(async () => {
293+
await mock?.stop();
294+
mock = undefined;
295+
});
296+
297+
test("returned name carries testId and resolves the poll without the header", async () => {
298+
mock = new LLMock({ port: 0 });
299+
mock.addFixture({
300+
match: { userMessage: "scoped poll", endpoint: "video" },
301+
response: {
302+
video: { id: "veo_scoped", status: "completed", url: "https://files.example/s.mp4" },
303+
},
304+
});
305+
await mock.start();
306+
307+
const submit = await fetch(submitUrl(mock.url), {
308+
method: "POST",
309+
headers: { "Content-Type": "application/json", "X-Test-Id": "test-a" },
310+
body: JSON.stringify({ instances: [{ prompt: "scoped poll" }] }),
311+
});
312+
const { name } = (await submit.json()) as { name: string };
313+
// Exact query param, not just a substring: `operations/{uuid}?testId=test-a`.
314+
expect(name).toMatch(/^operations\/[^?]+\?testId=test-a$/);
315+
316+
// Bare poll of the returned name — no X-Test-Id header — must still resolve
317+
// via the embedded query parameter.
318+
const poll = await fetch(`${mock.url}/v1beta/${name}`);
319+
expect(poll.status).toBe(200);
320+
const data = await poll.json();
321+
expect(data.done).toBe(true);
322+
});
323+
324+
// DECISIVE multi-tenant routing (#278). Two tenants submit DISTINCT jobs
325+
// concurrently — both live in the map at once — then each polls ONLY its own
326+
// RETURNED suffix-bearing id, sending NO x-test-id header. The header-less
327+
// poll must resolve via the embedded `?testId=` suffix to that tenant's own
328+
// job (not the other tenant's, not 404). This FAILS on pre-fix code: a bare
329+
// returned name carries no testId, so a header-less poll falls to the default
330+
// testId and the `${testId}:${operationName}` key misses → 404. The earlier
331+
// header-stripping "isolation" test was tautological (it 404s pre- AND
332+
// post-fix); this one is the real cross-tenant routing proof.
333+
test("concurrent tenants each resolve their OWN job via the header-less suffix", async () => {
334+
mock = new LLMock({ port: 0 });
335+
mock.addFixture({
336+
match: { userMessage: "tenant A clip", endpoint: "video" },
337+
response: {
338+
video: { id: "veo_A", status: "completed", url: "https://files.example/a.mp4" },
339+
},
340+
});
341+
mock.addFixture({
342+
match: { userMessage: "tenant B clip", endpoint: "video" },
343+
response: {
344+
video: { id: "veo_B", status: "completed", url: "https://files.example/b.mp4" },
345+
},
346+
});
347+
await mock.start();
348+
349+
// Both tenants submit concurrently → both jobs coexist in the map.
350+
const [resA, resB] = await Promise.all([
351+
fetch(submitUrl(mock.url), {
352+
method: "POST",
353+
headers: { "Content-Type": "application/json", "X-Test-Id": "tenant-a" },
354+
body: JSON.stringify({ instances: [{ prompt: "tenant A clip" }] }),
355+
}),
356+
fetch(submitUrl(mock.url), {
357+
method: "POST",
358+
headers: { "Content-Type": "application/json", "X-Test-Id": "tenant-b" },
359+
body: JSON.stringify({ instances: [{ prompt: "tenant B clip" }] }),
360+
}),
361+
]);
362+
const { name: nameA } = (await resA.json()) as { name: string };
363+
const { name: nameB } = (await resB.json()) as { name: string };
364+
expect(nameA).toMatch(/^operations\/[^?]+\?testId=tenant-a$/);
365+
expect(nameB).toMatch(/^operations\/[^?]+\?testId=tenant-b$/);
366+
// Distinct underlying operation names (no accidental collision).
367+
expect(nameA.split("?")[0]).not.toBe(nameB.split("?")[0]);
368+
369+
// Header-less poll of A's returned name → resolves A's job (a.mp4), not B's.
370+
const pollA = await fetch(`${mock.url}/v1beta/${nameA}`);
371+
expect(pollA.status).toBe(200);
372+
const dataA = await pollA.json();
373+
expect(dataA.done).toBe(true);
374+
expect(dataA.response.generateVideoResponse.generatedSamples[0].video.uri).toBe(
375+
"https://files.example/a.mp4",
376+
);
377+
378+
// Header-less poll of B's returned name → resolves B's job (b.mp4), not A's.
379+
const pollB = await fetch(`${mock.url}/v1beta/${nameB}`);
380+
expect(pollB.status).toBe(200);
381+
const dataB = await pollB.json();
382+
expect(dataB.done).toBe(true);
383+
expect(dataB.response.generateVideoResponse.generatedSamples[0].video.uri).toBe(
384+
"https://files.example/b.mp4",
385+
);
386+
});
387+
388+
test("default testId returns the bare operation name (no testId param)", async () => {
389+
mock = new LLMock({ port: 0 });
390+
mock.addFixture({
391+
match: { userMessage: "clean name", endpoint: "video" },
392+
response: {
393+
video: { id: "veo_clean", status: "completed", url: "https://files.example/c.mp4" },
394+
},
395+
});
396+
await mock.start();
397+
398+
const submit = await fetch(submitUrl(mock.url), {
399+
method: "POST",
400+
headers: { "Content-Type": "application/json" },
401+
body: JSON.stringify({ instances: [{ prompt: "clean name" }] }),
402+
});
403+
const { name } = (await submit.json()) as { name: string };
404+
// Byte-identical to pre-fix behaviour: bare `operations/{uuid}`, no query.
405+
expect(name).not.toContain("testId=");
406+
expect(name).not.toContain("?");
407+
expect(name.startsWith("operations/")).toBe(true);
408+
});
409+
});

src/grok-video.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import {
3333
} from "./recorder.js";
3434
import { resolveUpstreamUrl } from "./url.js";
3535
import { handleVideoStatus, type VideoStateMap } from "./video.js";
36-
import { readEnvelopeText, upstreamTimeoutSignal } from "./video-proxy-shared.js";
36+
import { readEnvelopeText, testIdSuffix, upstreamTimeoutSignal } from "./video-proxy-shared.js";
3737

3838
/**
3939
* xAI Grok Imagine async video lifecycle mock. Submit
@@ -555,8 +555,12 @@ export async function handleGrokVideoCreate(
555555
);
556556
}
557557

558+
// Embed the testId in the RETURNED request_id (not the stored key) so a
559+
// multi-tenant client can poll the opaque id without an x-test-id header — the
560+
// testId travels via getTestId's `?testId=` fallback. Mirrors OpenRouter's
561+
// polling_url treatment; testIdSuffix is "" for the default testId (bare id).
558562
res.writeHead(200, { "Content-Type": "application/json" });
559-
res.end(JSON.stringify({ request_id: requestId }));
563+
res.end(JSON.stringify({ request_id: requestId + testIdSuffix(testId, "?") }));
560564
}
561565

562566
// ─── GET /v1/videos/{id} — status poll (Grok-first, Sora fall-through) ───────
@@ -872,8 +876,11 @@ async function proxyGrokVideoSubmit(args: {
872876
},
873877
});
874878

879+
// Embed the testId in the RETURNED request_id (see the replay submit) so
880+
// record-mode multi-tenant polls resolve via `?testId=`; the upstream poll
881+
// still uses the bare job.requestId / job.upstreamPollingUrl, untouched.
875882
res.writeHead(200, { "Content-Type": "application/json" });
876-
res.end(JSON.stringify({ request_id: requestId }));
883+
res.end(JSON.stringify({ request_id: requestId + testIdSuffix(testId, "?") }));
877884
return "handled";
878885
}
879886

src/veo-video.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import {
3232
sanitizeHeaderValue,
3333
} from "./recorder.js";
3434
import { resolveUpstreamUrl } from "./url.js";
35-
import { readEnvelopeText, upstreamTimeoutSignal } from "./video-proxy-shared.js";
35+
import { readEnvelopeText, testIdSuffix, upstreamTimeoutSignal } from "./video-proxy-shared.js";
3636

3737
/**
3838
* Google Veo async video lifecycle mock. Submit
@@ -461,8 +461,12 @@ export async function handleVeoVideoCreate(
461461
);
462462
}
463463

464+
// Embed the testId in the RETURNED name (not the stored key) so a multi-tenant
465+
// client can poll the opaque name without an x-test-id header — the testId
466+
// travels via getTestId's `?testId=` fallback. Mirrors OpenRouter's polling_url
467+
// treatment; testIdSuffix is "" for the default testId, keeping the name bare.
464468
res.writeHead(200, { "Content-Type": "application/json" });
465-
res.end(JSON.stringify({ name: operationName }));
469+
res.end(JSON.stringify({ name: operationName + testIdSuffix(testId, "?") }));
466470
}
467471

468472
// ─── GET /v1beta/operations/{name} — status poll ─────────────────────────────
@@ -801,8 +805,11 @@ async function proxyVeoVideoSubmit(args: {
801805
},
802806
});
803807

808+
// Embed the testId in the RETURNED name (see the replay submit) so record-mode
809+
// multi-tenant polls resolve via `?testId=`; the upstream poll still uses the
810+
// bare job.operationName / job.upstreamPollingUrl, untouched by the suffix.
804811
res.writeHead(200, { "Content-Type": "application/json" });
805-
res.end(JSON.stringify({ name: operationName }));
812+
res.end(JSON.stringify({ name: operationName + testIdSuffix(testId, "?") }));
806813
return "handled";
807814
}
808815

0 commit comments

Comments
 (0)