Skip to content

Commit e6780f2

Browse files
MajorTalMajorTal
committed
Merge branch 'codex/bugs-gh-262-release-limits'
# Conflicts: # sdk/src/namespaces/deploy.test.ts # sdk/src/namespaces/deploy.ts
2 parents 3df8f0b + 2f7d065 commit e6780f2

2 files changed

Lines changed: 206 additions & 5 deletions

File tree

‎sdk/src/namespaces/deploy.test.ts‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3571,6 +3571,29 @@ describe("Deploy.list", () => {
35713571
assert.equal(w.requests[0].path, "/deploy/v2/operations?limit=5&cursor=op_cursor");
35723572
});
35733573

3574+
it("rejects invalid limit values with a LocalError before issuing a request", async () => {
3575+
const invalidLimits = [
3576+
{ label: "NaN", value: Number.NaN },
3577+
{ label: "zero", value: 0 },
3578+
{ label: "negative", value: -1 },
3579+
{ label: "fractional", value: 1.5 },
3580+
{ label: "infinite", value: Number.POSITIVE_INFINITY },
3581+
{ label: "unsafe", value: Number.MAX_SAFE_INTEGER + 1 },
3582+
];
3583+
3584+
for (const { label, value } of invalidLimits) {
3585+
const w = makeWiring();
3586+
const deploy = new Deploy(w.client);
3587+
await assert.rejects(
3588+
() => deploy.list({ project: "prj_test", limit: value }),
3589+
(err: unknown) =>
3590+
err instanceof LocalError && /limit/i.test((err as LocalError).message),
3591+
`${label} limit should be rejected locally`,
3592+
);
3593+
assert.equal(w.requests.length, 0, `${label}: no gateway request`);
3594+
}
3595+
});
3596+
35743597
it("accepts a bare projectId string and issues the same request as the options form", async () => {
35753598
const projectLookups: string[] = [];
35763599
const wiringFor = (): FakeWiring => {
@@ -3775,6 +3798,34 @@ describe("Deploy release observability", () => {
37753798
assert.equal(w.requests[0].headers?.apikey, "ak");
37763799
});
37773800

3801+
it("rejects invalid getRelease siteLimit values before issuing a request", async () => {
3802+
const invalidLimits = [
3803+
{ label: "NaN", value: Number.NaN },
3804+
{ label: "zero", value: 0 },
3805+
{ label: "negative", value: -1 },
3806+
{ label: "fractional", value: 1.5 },
3807+
{ label: "infinite", value: Number.POSITIVE_INFINITY },
3808+
{ label: "unsafe", value: Number.MAX_SAFE_INTEGER + 1 },
3809+
];
3810+
3811+
for (const { label, value } of invalidLimits) {
3812+
const w = makeWiring();
3813+
const deploy = new Deploy(w.client);
3814+
await assert.rejects(
3815+
() =>
3816+
deploy.getRelease({
3817+
project: "prj_test",
3818+
releaseId: "rel_1",
3819+
siteLimit: value,
3820+
}),
3821+
(err: unknown) =>
3822+
err instanceof LocalError && /siteLimit/i.test((err as LocalError).message),
3823+
`${label} siteLimit should be rejected locally`,
3824+
);
3825+
assert.equal(w.requests.length, 0, `${label}: no gateway request`);
3826+
}
3827+
});
3828+
37783829
it("fetches the active release inventory with site_limit", async () => {
37793830
const w = makeWiring();
37803831
const activeInventory = { ...inventory, release_id: "rel_active", state_kind: "current_live" };
@@ -3791,6 +3842,29 @@ describe("Deploy release observability", () => {
37913842
assert.equal(w.requests[0].headers?.apikey, "ak");
37923843
});
37933844

3845+
it("rejects invalid getActiveRelease siteLimit values before issuing a request", async () => {
3846+
const invalidLimits = [
3847+
{ label: "NaN", value: Number.NaN },
3848+
{ label: "zero", value: 0 },
3849+
{ label: "negative", value: -1 },
3850+
{ label: "fractional", value: 1.5 },
3851+
{ label: "infinite", value: Number.POSITIVE_INFINITY },
3852+
{ label: "unsafe", value: Number.MAX_SAFE_INTEGER + 1 },
3853+
];
3854+
3855+
for (const { label, value } of invalidLimits) {
3856+
const w = makeWiring();
3857+
const deploy = new Deploy(w.client);
3858+
await assert.rejects(
3859+
() => deploy.getActiveRelease({ project: "prj_test", siteLimit: value }),
3860+
(err: unknown) =>
3861+
err instanceof LocalError && /siteLimit/i.test((err as LocalError).message),
3862+
`${label} siteLimit should be rejected locally`,
3863+
);
3864+
assert.equal(w.requests.length, 0, `${label}: no gateway request`);
3865+
}
3866+
});
3867+
37943868
it("diffs release targets with encoded selectors, limit, and project apikey auth", async () => {
37953869
const w = makeWiring();
37963870
const diff = {
@@ -3826,4 +3900,78 @@ describe("Deploy release observability", () => {
38263900
assert.equal(w.requests[0].headers?.apikey, "ak");
38273901
});
38283902

3903+
it("rejects invalid diff limit values before issuing a request", async () => {
3904+
const invalidLimits = [
3905+
{ label: "NaN", value: Number.NaN },
3906+
{ label: "zero", value: 0 },
3907+
{ label: "negative", value: -1 },
3908+
{ label: "fractional", value: 1.5 },
3909+
{ label: "infinite", value: Number.POSITIVE_INFINITY },
3910+
{ label: "unsafe", value: Number.MAX_SAFE_INTEGER + 1 },
3911+
];
3912+
3913+
for (const { label, value } of invalidLimits) {
3914+
const w = makeWiring();
3915+
const deploy = new Deploy(w.client);
3916+
await assert.rejects(
3917+
() =>
3918+
deploy.diff({
3919+
project: "prj_test",
3920+
from: "empty",
3921+
to: "rel_2",
3922+
limit: value,
3923+
}),
3924+
(err: unknown) =>
3925+
err instanceof LocalError && /limit/i.test((err as LocalError).message),
3926+
`${label} limit should be rejected locally`,
3927+
);
3928+
assert.equal(w.requests.length, 0, `${label}: no gateway request`);
3929+
}
3930+
});
3931+
3932+
it("rejects missing or empty diff selectors before issuing a request", async () => {
3933+
const cases: Array<{
3934+
label: string;
3935+
opts: Parameters<Deploy["diff"]>[0];
3936+
expectedMessage: RegExp;
3937+
}> = [
3938+
{
3939+
label: "missing from",
3940+
opts: { project: "prj_test", to: "rel_2" } as unknown as Parameters<
3941+
Deploy["diff"]
3942+
>[0],
3943+
expectedMessage: /from/i,
3944+
},
3945+
{
3946+
label: "missing to",
3947+
opts: { project: "prj_test", from: "rel_1" } as unknown as Parameters<
3948+
Deploy["diff"]
3949+
>[0],
3950+
expectedMessage: /to/i,
3951+
},
3952+
{
3953+
label: "empty from",
3954+
opts: { project: "prj_test", from: "", to: "rel_2" },
3955+
expectedMessage: /from/i,
3956+
},
3957+
{
3958+
label: "empty to",
3959+
opts: { project: "prj_test", from: "rel_1", to: "" },
3960+
expectedMessage: /to/i,
3961+
},
3962+
];
3963+
3964+
for (const { label, opts, expectedMessage } of cases) {
3965+
const w = makeWiring();
3966+
const deploy = new Deploy(w.client);
3967+
await assert.rejects(
3968+
() => deploy.diff(opts),
3969+
(err: unknown) =>
3970+
err instanceof LocalError && expectedMessage.test((err as LocalError).message),
3971+
`${label} should be rejected locally`,
3972+
);
3973+
assert.equal(w.requests.length, 0, `${label}: no gateway request`);
3974+
}
3975+
});
3976+
38293977
});

‎sdk/src/namespaces/deploy.ts‎

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -290,9 +290,14 @@ export class Deploy {
290290
"listing deploy operations",
291291
);
292292
}
293+
const normalizedLimit = normalizePositiveSafeIntegerQueryOption(
294+
limit,
295+
"r.deploy.list limit",
296+
"listing deploy operations",
297+
);
293298
const headers = await apikeyHeaders(this.client, project);
294299
const qs = new URLSearchParams();
295-
if (limit !== undefined) qs.set("limit", String(limit));
300+
if (normalizedLimit !== undefined) qs.set("limit", String(normalizedLimit));
296301
if (cursor !== undefined) qs.set("cursor", cursor);
297302
const path =
298303
qs.toString().length > 0
@@ -353,10 +358,15 @@ export class Deploy {
353358
"fetching release inventory",
354359
);
355360
}
361+
const siteLimit = normalizePositiveSafeIntegerQueryOption(
362+
opts.siteLimit,
363+
"r.deploy.getRelease siteLimit",
364+
"fetching release inventory",
365+
);
356366
const headers = await apikeyHeaders(this.client, opts.project);
357367
return this.client.request<ReleaseInventory>(
358368
appendQuery(`/deploy/v2/releases/${encodeURIComponent(opts.releaseId)}`, {
359-
site_limit: opts.siteLimit,
369+
site_limit: siteLimit,
360370
}),
361371
{ headers, context: "fetching release inventory" },
362372
);
@@ -376,10 +386,15 @@ export class Deploy {
376386
"fetching active release inventory",
377387
);
378388
}
389+
const siteLimit = normalizePositiveSafeIntegerQueryOption(
390+
opts.siteLimit,
391+
"r.deploy.getActiveRelease siteLimit",
392+
"fetching active release inventory",
393+
);
379394
const headers = await apikeyHeaders(this.client, opts.project);
380395
return this.client.request<ActiveReleaseInventory>(
381396
appendQuery("/deploy/v2/releases/active", {
382-
site_limit: opts.siteLimit,
397+
site_limit: siteLimit,
383398
}),
384399
{ headers, context: "fetching active release inventory" },
385400
);
@@ -397,9 +412,24 @@ export class Deploy {
397412
"diffing releases",
398413
);
399414
}
415+
const from = requireNonEmptyStringQueryOption(
416+
opts.from,
417+
"r.deploy.diff from",
418+
"diffing releases",
419+
);
420+
const to = requireNonEmptyStringQueryOption(
421+
opts.to,
422+
"r.deploy.diff to",
423+
"diffing releases",
424+
);
425+
const limit = normalizePositiveSafeIntegerQueryOption(
426+
opts.limit,
427+
"r.deploy.diff limit",
428+
"diffing releases",
429+
);
400430
const headers = await apikeyHeaders(this.client, opts.project);
401-
const qs = new URLSearchParams({ from: opts.from, to: opts.to });
402-
if (opts.limit !== undefined) qs.set("limit", String(opts.limit));
431+
const qs = new URLSearchParams({ from, to });
432+
if (limit !== undefined) qs.set("limit", String(limit));
403433
return this.client.request<ReleaseToReleaseDiff>(
404434
`/deploy/v2/releases/diff?${qs.toString()}`,
405435
{ headers, context: "diffing releases" },
@@ -436,6 +466,29 @@ function appendQuery(
436466
return query.length > 0 ? `${path}?${query}` : path;
437467
}
438468

469+
function normalizePositiveSafeIntegerQueryOption(
470+
value: number | undefined,
471+
label: string,
472+
context: string,
473+
): number | undefined {
474+
if (value === undefined) return undefined;
475+
if (!Number.isSafeInteger(value) || value <= 0) {
476+
throw new LocalError(`${label} must be a positive safe integer`, context);
477+
}
478+
return value;
479+
}
480+
481+
function requireNonEmptyStringQueryOption(
482+
value: unknown,
483+
label: string,
484+
context: string,
485+
): string {
486+
if (typeof value !== "string" || value.length === 0) {
487+
throw new LocalError(`${label} must be a non-empty string`, context);
488+
}
489+
return value;
490+
}
491+
439492
// ─── Internal pipeline ───────────────────────────────────────────────────────
440493

441494
async function applyOnce(

0 commit comments

Comments
 (0)