From a78838734a969b1e4a6d8f86b8bd0edb3e95f614 Mon Sep 17 00:00:00 2001 From: yaowenc2 Date: Mon, 3 Aug 2026 15:03:29 -0700 Subject: [PATCH 1/4] =?UTF-8?q?feat:=20limits=20commands=20=E2=80=94=20ins?= =?UTF-8?q?ta=20compute=20limits=20/=20insta=20db=20limits?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ceiling controls that replace picking a named spec. Compute bills actual usage, so size is no longer a price: what you set is a cap on what the app may burn, and it moves both directions. insta compute limits show ceiling + plan max insta compute limits --memory 1gb set it (cpu derives from memory) insta compute limits --memory 1gb --cpu 2 explicit override insta db limits --memory 8Gi --cpu 4 same dial for postgres (insta-db) --memory is the dial because memory is the ceiling users actually feel (it OOM-kills the app) while vCPU only throttles; deriving cpu also keeps the provider's size grid out of the CLI's vocabulary. Bare `limits` is a safe read that prints the plan cap alongside the current value. Paid plans, per the platform gate. Needs insta-platform#156. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JQ9NX4AN5XEmb3sjBHvTzS --- src/commands/compute.ts | 46 +++++++++++++++++++++++++++++++++++++++++ src/commands/db.ts | 34 ++++++++++++++++++++++++++++++ src/index.ts | 7 +++++++ test/limits.test.ts | 37 +++++++++++++++++++++++++++++++++ 4 files changed, 124 insertions(+) create mode 100644 test/limits.test.ts diff --git a/src/commands/compute.ts b/src/commands/compute.ts index 96c0189..fd13299 100644 --- a/src/commands/compute.ts +++ b/src/commands/compute.ts @@ -89,3 +89,49 @@ export async function computeAlwaysOn(mode: string, serviceName: string | undefi const on = res.body.service?.always_on info(`compute ${res.body.service?.name ?? id}: always-on ${on ? 'ENABLED — machines stay warm (no cold starts; idle RAM bills at actual usage)' : 'disabled — scales to zero when idle (default)'}`) } + +// ---- limits (the resource ceiling; paid plans) ---- + +// Parse a human memory value into MB: "512", "512mb", "1gb", "2g", "1.5gb". +// Exported for unit tests — this is the only place a user-typed size becomes a number. +export function parseMemoryMb(raw: string): number { + const m = /^\s*(\d+(?:\.\d+)?)\s*(g|gb|gi|gib|m|mb|mi|mib)?\s*$/i.exec(raw) + if (!m) throw new Error(`invalid memory: ${raw} (try 512mb, 1gb, 2gb)`) + const n = Number(m[1]) + const unit = (m[2] ?? 'mb').toLowerCase() + const mb = unit.startsWith('g') ? n * 1024 : n + if (!(mb > 0)) throw new Error(`invalid memory: ${raw}`) + return Math.round(mb) +} + +const fmtMb = (mb: number) => (mb >= 1024 && mb % 1024 === 0 ? `${mb / 1024} GB` : `${mb} MB`) + +type LimitsOpts = LifeOpts & { cpu?: string; memory?: string } + +// Show or set a compute service's ceiling. With no --memory it PRINTS the current limits and the +// plan cap (so `insta compute limits` is a safe read), which is also what a UI renders as a slider +// with its plan-limit marker. +export async function computeLimits(serviceName: string | undefined, opts: LimitsOpts): Promise { + const api = await ApiClient.load() + const p = await requireProject() + const branch = opts.branch ?? p.branch + const { services } = await api.request('GET', `/projects/${p.projectId}/services${q(branch)}`) + const id = resolveComputeServiceId(services, serviceName) + + if (!opts.memory && !opts.cpu) { + const r = await api.request('GET', `/projects/${p.projectId}/services/${id}/limits`) + if (opts.json) return printJson(r) + info(`compute ${serviceName ?? id}: ceiling ${r.limits.cpu} vCPU / ${fmtMb(r.limits.memoryMb)} (plan max ${r.cap.cpu} vCPU / ${fmtMb(r.cap.memoryMb)})`) + info(' billing is actual usage — the ceiling caps what the app may burn, it is not a price') + return + } + if (!opts.memory) throw new Error('--memory is required when setting limits (cpu is derived from it; pass --cpu only to override)') + + const body: Record = { memoryMb: parseMemoryMb(opts.memory) } + if (opts.cpu) body.cpu = Number(opts.cpu) + const res = await api.rawRequest('PUT', `/projects/${p.projectId}/services/${id}/limits`, body) + if (handleApproval(res)) return + if (opts.json) return printJson(res.body) + const l = res.body.limits + info(`compute ${res.body.service?.name ?? id}: ceiling set to ${l.cpu} vCPU / ${fmtMb(l.memoryMb)}`) +} diff --git a/src/commands/db.ts b/src/commands/db.ts index 98f775a..7bb78d5 100644 --- a/src/commands/db.ts +++ b/src/commands/db.ts @@ -22,3 +22,37 @@ export async function dbAlwaysOn(mode: string, opts: Opts): Promise { const s2z = res.body?.scaleToZero info(`postgres ${opts.group ?? 'default'}: always-on ${s2z === false ? 'ENABLED — instance stays warm (no cold starts; idle RAM bills at actual usage)' : 'disabled — scales to zero when idle (default; first connection after idle cold-starts)'}`) } + +// Show or set a postgres service's resource ceiling (insta-db-backed only). Paid plans — the +// ceiling is the tier lever now that billing follows actual usage. Moves both directions: +// unlike storage it is a cgroup limit, not a provisioned volume. +export async function dbLimits(opts: Opts & { cpu?: string; memory?: string }): Promise { + const api = await ApiClient.load() + const p = await requireProject() + const qs = new URLSearchParams() + const branch = opts.branch ?? p.branch + if (branch) qs.set('branch', branch) + if (opts.group) qs.set('group', opts.group) + const suffix = qs.toString() ? `?${qs}` : '' + + if (!opts.cpu && !opts.memory) { + const r = await api.request('GET', `/projects/${p.projectId}/database/instance${suffix}`).catch(() => null) + if (opts.json) return printJson(r ?? {}) + if (r?.cpuMilli || r?.memoryMib) { + info(`postgres ${opts.group ?? 'default'}: ceiling ${(r.cpuMilli / 1000).toFixed(r.cpuMilli % 1000 ? 1 : 0)} vCPU / ${Math.round(r.memoryMib / 1024)} GiB`) + } else { + info(`postgres ${opts.group ?? 'default'}: current ceiling unavailable — set one with --cpu/--memory`) + } + return + } + + const body: Record = {} + if (opts.cpu) body.cpu = opts.cpu + if (opts.memory) body.memory = opts.memory + const res = await api.rawRequest('PATCH', `/projects/${p.projectId}/database/settings${suffix}`, body) + if (handleApproval(res)) return + if (opts.json) return printJson(res.body) + const cpu = res.body?.cpuMilli ? `${res.body.cpuMilli / 1000} vCPU` : (opts.cpu ?? 'unchanged') + const mem = res.body?.memoryMib ? `${Math.round(res.body.memoryMib / 1024)} GiB` : (opts.memory ?? 'unchanged') + info(`postgres ${opts.group ?? 'default'}: ceiling set to ${cpu} / ${mem}`) +} diff --git a/src/index.ts b/src/index.ts index d724cc0..d15f0fa 100644 --- a/src/index.ts +++ b/src/index.ts @@ -174,11 +174,18 @@ compute.command('suspend [service]').description('Suspend a compute service (RAM .option('--json').option('--branch ', 'branch (default: current)').action(guard((service, o) => computeCmd.computeSuspend(service, o))) compute.command('status [service]').description("Show a compute service's desired vs. live state") .option('--json').option('--branch ', 'branch (default: current)').action(guard((service, o) => computeCmd.computeStatus(service, o))) +compute.command('limits [service]').description("Show or set a compute service's resource ceiling (paid plans). --memory is the dial; cpu derives from it unless --cpu is given. Billing is actual usage — the ceiling caps what the app may burn, it is not a price") + .option('--memory ', 'memory ceiling, e.g. 512mb or 1gb').option('--cpu ', 'vCPU ceiling override (provider sizes: 1, 2, 4, 6, 8)') + .option('--json').option('--branch ', 'branch (default: current)').action(guard((service, o) => computeCmd.computeLimits(service, o))) compute.command('always-on [service]').description('Set a compute service always-on (mode: on|off). on = machines never scale to zero; off = default scale-to-zero. All plans; billing is actual usage either way') .option('--json').option('--branch ', 'branch (default: current)').action(guard((mode, service, o) => computeCmd.computeAlwaysOn(mode, service, o))) // ---- db (postgres service controls) ---- const db = program.command('db').description('Postgres service controls (always-on / scale-to-zero)') +db.command('limits').description("Show or set a postgres service's resource ceiling (paid plans; insta-db-backed only). Moves both directions") + .option('--cpu ', "vCPU ceiling, e.g. 2 or 2500m").option('--memory ', "memory ceiling, e.g. 4Gi") + .option('--json').option('--branch ', 'branch (default: current)').option('--group ', 'postgres service name (default: the sole/default one)') + .action(guard((o) => dbCmd.dbLimits(o))) db.command('always-on ').description('Set a postgres service always-on (mode: on|off). on = instance stays warm, no cold starts; off = default scale-to-zero (idle instance suspends; first connection cold-starts). insta-db-backed services only') .option('--json').option('--branch ', 'branch (default: current)').option('--group ', 'postgres service name (default: the sole/default one)') .action(guard((mode, o) => dbCmd.dbAlwaysOn(mode, o))) diff --git a/test/limits.test.ts b/test/limits.test.ts new file mode 100644 index 0000000..d4a4716 --- /dev/null +++ b/test/limits.test.ts @@ -0,0 +1,37 @@ +// `insta compute limits` / `insta db limits` — the ceiling controls that replace spec picking. +// The parsing seam is what these pin: a user types "1gb", the API takes MB, and getting that +// conversion wrong sets a ceiling an order of magnitude off in either direction. +import { describe, it, expect } from 'vitest' +import { parseMemoryMb } from '../src/commands/compute.js' + +describe('parseMemoryMb', () => { + it('treats a bare number as MB', () => { + expect(parseMemoryMb('512')).toBe(512) + expect(parseMemoryMb('256')).toBe(256) + }) + + it('accepts MB units in the forms people actually type', () => { + for (const raw of ['512mb', '512MB', '512m', '512 MB', ' 512mib ']) { + expect(parseMemoryMb(raw), raw).toBe(512) + } + }) + + it('converts GB to MB', () => { + expect(parseMemoryMb('1gb')).toBe(1024) + expect(parseMemoryMb('2G')).toBe(2048) + expect(parseMemoryMb('1.5gb')).toBe(1536) + expect(parseMemoryMb('8Gi')).toBe(8192) + }) + + it('rejects nonsense rather than guessing', () => { + for (const raw of ['', 'lots', '1tb', '-1gb', '0', 'gb', '1 2gb']) { + expect(() => parseMemoryMb(raw), raw).toThrow() + } + }) + + // The error names an example, because the failure mode without one is a user retrying the same + // invalid string in a different case. + it('suggests a valid form in the error', () => { + expect(() => parseMemoryMb('huge')).toThrow(/try 512mb, 1gb, 2gb/) + }) +}) From cb0884208ec815ce584c0256cb08616c564f6d99 Mon Sep 17 00:00:00 2001 From: yaowenc2 Date: Mon, 3 Aug 2026 16:16:21 -0700 Subject: [PATCH 2/4] =?UTF-8?q?fix:=20review=20round=202=20=E2=80=94=20thr?= =?UTF-8?q?owing=20parsers,=20honest=20db=20read,=20precise=20display?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both reviewers independently hit the same two: a bare Number() on --cpu sent NaN→null to the server instead of failing locally (now parseCpu, matching the repo's throwing-parser convention), and the db read swallowed EVERY error into 'ceiling unavailable' (now a real GET /database/instance — added platform-side in #156 — with errors propagating; only the Neon-backed 502 is softened, since that genuinely has no manageable instance). Also: db --cpu/--memory validated locally as k8s quantities with examples in the errors; display no longer lies (1536 MiB was shown as '2 GiB' — fmtMib collapses only whole/half GiB and keeps everything else exact, fmtMb same for MB); db group --help mentions limits. 174 tests passing (+6 on the new seams). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JQ9NX4AN5XEmb3sjBHvTzS --- src/commands/compute.ts | 15 +++++++++++-- src/commands/db.ts | 50 +++++++++++++++++++++++++++++++++-------- src/index.ts | 2 +- test/limits.test.ts | 42 ++++++++++++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 12 deletions(-) diff --git a/src/commands/compute.ts b/src/commands/compute.ts index fd13299..23db700 100644 --- a/src/commands/compute.ts +++ b/src/commands/compute.ts @@ -104,7 +104,18 @@ export function parseMemoryMb(raw: string): number { return Math.round(mb) } -const fmtMb = (mb: number) => (mb >= 1024 && mb % 1024 === 0 ? `${mb / 1024} GB` : `${mb} MB`) +// Whole and half GB collapse (1536 → "1.5 GB"); anything else stays exact in MB — a display that +// rounds 1536 to "2 GB" claims a ceiling the API did not set. +const fmtMb = (mb: number) => (mb >= 1024 && mb % 512 === 0 ? `${mb / 1024} GB` : `${mb} MB`) + +// The --cpu override, through a throwing parser like every other user-typed number in this repo +// (parseCount, parseMemoryMb). A bare Number() turns a typo into NaN, which JSON.stringify +// serializes as null — the server then sees {cpu: null} instead of the user seeing an error. +export function parseCpu(raw: string): number { + const n = Number(raw) + if (!Number.isInteger(n) || n <= 0) throw new Error(`invalid cpu: ${raw} (provider sizes: 1, 2, 4, 6, 8)`) + return n +} type LimitsOpts = LifeOpts & { cpu?: string; memory?: string } @@ -128,7 +139,7 @@ export async function computeLimits(serviceName: string | undefined, opts: Limit if (!opts.memory) throw new Error('--memory is required when setting limits (cpu is derived from it; pass --cpu only to override)') const body: Record = { memoryMb: parseMemoryMb(opts.memory) } - if (opts.cpu) body.cpu = Number(opts.cpu) + if (opts.cpu) body.cpu = parseCpu(opts.cpu) const res = await api.rawRequest('PUT', `/projects/${p.projectId}/services/${id}/limits`, body) if (handleApproval(res)) return if (opts.json) return printJson(res.body) diff --git a/src/commands/db.ts b/src/commands/db.ts index 7bb78d5..9a65832 100644 --- a/src/commands/db.ts +++ b/src/commands/db.ts @@ -23,6 +23,25 @@ export async function dbAlwaysOn(mode: string, opts: Opts): Promise { info(`postgres ${opts.group ?? 'default'}: always-on ${s2z === false ? 'ENABLED — instance stays warm (no cold starts; idle RAM bills at actual usage)' : 'disabled — scales to zero when idle (default; first connection after idle cold-starts)'}`) } +// Validated pass-throughs for the provider's quantity strings. The insta-db resize API takes +// k8s-style quantities (cpu: "2", "2500m"; memory: "4Gi", "2048Mi"), so unlike the compute path +// there is no unit conversion here — but junk must still fail LOCALLY with an example, not travel +// to the server as-is. +export function parseDbCpu(raw: string): string { + if (!/^\d+(\.\d+)?m?$/.test(raw.trim())) throw new Error(`invalid cpu: ${raw} (try 2, 4, or 2500m)`) + return raw.trim() +} +export function parseDbMemory(raw: string): string { + if (!/^\d+(\.\d+)?(Gi|Mi|G|M)$/i.test(raw.trim())) throw new Error(`invalid memory: ${raw} (try 4Gi or 8Gi)`) + return raw.trim() +} + +// MiB → display without lying: whole/half GiB collapse, anything else stays exact in MiB +// (1536 MiB is "1.5 GiB", 1300 MiB is "1300 MiB" — never "1 GiB"). +export function fmtMib(mib: number): string { + return mib >= 1024 && mib % 512 === 0 ? `${mib / 1024} GiB` : `${mib} MiB` +} + // Show or set a postgres service's resource ceiling (insta-db-backed only). Paid plans — the // ceiling is the tier lever now that billing follows actual usage. Moves both directions: // unlike storage it is a cgroup limit, not a provisioned volume. @@ -36,23 +55,36 @@ export async function dbLimits(opts: Opts & { cpu?: string; memory?: string }): const suffix = qs.toString() ? `?${qs}` : '' if (!opts.cpu && !opts.memory) { - const r = await api.request('GET', `/projects/${p.projectId}/database/instance${suffix}`).catch(() => null) - if (opts.json) return printJson(r ?? {}) - if (r?.cpuMilli || r?.memoryMib) { - info(`postgres ${opts.group ?? 'default'}: ceiling ${(r.cpuMilli / 1000).toFixed(r.cpuMilli % 1000 ? 1 : 0)} vCPU / ${Math.round(r.memoryMib / 1024)} GiB`) + // A real read against GET /database/instance. Errors PROPAGATE — an expired token or a 502 + // must not render as "no ceiling set"; the only softened case is a Neon-backed service, where + // the platform answers 502 provider-shaped because there is no manageable instance. + const res = await api.rawRequest('GET', `/projects/${p.projectId}/database/instance${suffix}`) + if (res.status === 502) { + info(`postgres ${opts.group ?? 'default'}: no manageable instance (Neon-backed services manage their own resources)`) + return + } + if (res.status >= 400) throw new Error(`reading the instance failed (${res.status}): ${res.body?.error ?? 'unknown error'}`) + if (opts.json) return printJson(res.body) + const cpuMilli = res.body?.cpuMilli + const mib = res.body?.memoryMib + if (typeof cpuMilli === 'number' && typeof mib === 'number') { + const cpu = cpuMilli % 1000 === 0 ? `${cpuMilli / 1000}` : `${cpuMilli}m` + info(`postgres ${opts.group ?? 'default'}: ceiling ${cpu} vCPU / ${fmtMib(mib)}`) + info(' billing is actual usage — the ceiling caps what the database may burn, it is not a price') } else { - info(`postgres ${opts.group ?? 'default'}: current ceiling unavailable — set one with --cpu/--memory`) + info(`postgres ${opts.group ?? 'default'}: provider reported no ceiling — set one with --cpu/--memory`) } return } const body: Record = {} - if (opts.cpu) body.cpu = opts.cpu - if (opts.memory) body.memory = opts.memory + if (opts.cpu) body.cpu = parseDbCpu(opts.cpu) + if (opts.memory) body.memory = parseDbMemory(opts.memory) const res = await api.rawRequest('PATCH', `/projects/${p.projectId}/database/settings${suffix}`, body) if (handleApproval(res)) return + if (res.status >= 400) throw new Error(`setting the ceiling failed (${res.status}): ${res.body?.error ?? 'unknown error'}`) if (opts.json) return printJson(res.body) - const cpu = res.body?.cpuMilli ? `${res.body.cpuMilli / 1000} vCPU` : (opts.cpu ?? 'unchanged') - const mem = res.body?.memoryMib ? `${Math.round(res.body.memoryMib / 1024)} GiB` : (opts.memory ?? 'unchanged') + const cpu = typeof res.body?.cpuMilli === 'number' ? `${res.body.cpuMilli / 1000} vCPU` : (opts.cpu ?? 'unchanged') + const mem = typeof res.body?.memoryMib === 'number' ? fmtMib(res.body.memoryMib) : (opts.memory ?? 'unchanged') info(`postgres ${opts.group ?? 'default'}: ceiling set to ${cpu} / ${mem}`) } diff --git a/src/index.ts b/src/index.ts index d15f0fa..a9cdad5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -181,7 +181,7 @@ compute.command('always-on [service]').description('Set a compute service .option('--json').option('--branch ', 'branch (default: current)').action(guard((mode, service, o) => computeCmd.computeAlwaysOn(mode, service, o))) // ---- db (postgres service controls) ---- -const db = program.command('db').description('Postgres service controls (always-on / scale-to-zero)') +const db = program.command('db').description('Postgres service controls (limits / always-on / scale-to-zero)') db.command('limits').description("Show or set a postgres service's resource ceiling (paid plans; insta-db-backed only). Moves both directions") .option('--cpu ', "vCPU ceiling, e.g. 2 or 2500m").option('--memory ', "memory ceiling, e.g. 4Gi") .option('--json').option('--branch ', 'branch (default: current)').option('--group ', 'postgres service name (default: the sole/default one)') diff --git a/test/limits.test.ts b/test/limits.test.ts index d4a4716..08a3457 100644 --- a/test/limits.test.ts +++ b/test/limits.test.ts @@ -35,3 +35,45 @@ describe('parseMemoryMb', () => { expect(() => parseMemoryMb('huge')).toThrow(/try 512mb, 1gb, 2gb/) }) }) + +// Review round 2: both jwfing and cubic independently flagged the bare Number() on --cpu (NaN +// serializes to null on the wire) and the unvalidated db strings. These pin the new seams. +import { parseCpu } from '../src/commands/compute.js' +import { parseDbCpu, parseDbMemory, fmtMib } from '../src/commands/db.js' + +describe('parseCpu (compute --cpu override)', () => { + it('accepts positive integers', () => { + expect(parseCpu('1')).toBe(1) + expect(parseCpu('8')).toBe(8) + }) + it('throws locally on junk instead of sending null to the server', () => { + for (const raw of ['abc', '', '-2', '1.5', 'two']) { + expect(() => parseCpu(raw), raw).toThrow(/invalid cpu/) + } + }) +}) + +describe('parseDbCpu / parseDbMemory (provider quantity strings)', () => { + it('passes valid k8s quantities through untouched', () => { + expect(parseDbCpu('2')).toBe('2') + expect(parseDbCpu('2500m')).toBe('2500m') + expect(parseDbMemory('4Gi')).toBe('4Gi') + expect(parseDbMemory('2048Mi')).toBe('2048Mi') + }) + it('rejects junk locally with an example', () => { + expect(() => parseDbCpu('huge')).toThrow(/try 2, 4, or 2500m/) + expect(() => parseDbMemory('lots')).toThrow(/try 4Gi or 8Gi/) + expect(() => parseDbMemory('8')).toThrow() // unit required — a bare number is ambiguous here + }) +}) + +describe('fmtMib (display must not claim a ceiling the API did not set)', () => { + it('collapses whole and half GiB', () => { + expect(fmtMib(2048)).toBe('2 GiB') + expect(fmtMib(1536)).toBe('1.5 GiB') // the review example: was shown as "2 GiB" + }) + it('keeps everything else exact in MiB', () => { + expect(fmtMib(1300)).toBe('1300 MiB') + expect(fmtMib(512)).toBe('512 MiB') + }) +}) From d98b52c47ca7bf3178d8df053a4c7d8162ae3776 Mon Sep 17 00:00:00 2001 From: yaowenc2 Date: Mon, 3 Aug 2026 16:27:45 -0700 Subject: [PATCH 3/4] =?UTF-8?q?fix:=20dbLimits=20read=20path=20=E2=80=94?= =?UTF-8?q?=20the=20502/error=20branches=20were=20unreachable=20dead=20cod?= =?UTF-8?q?e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 Critical, and it was real: rawRequest THROWS ApiError on any status >= 400 (it differs from request only in returning {status,body} below 400 for 202 branching), so branching on res.status >= 400 after it never ran — a Neon-backed 'insta db limits' crashed with a raw ApiError instead of the documented friendly message, and the error-wrapping was equally dead. The fix extracts the read into fetchDbInstance(api, ...) — a seam that takes the client as an argument, per this repo's pure-seam convention — with the Neon 502 mapped to a soft no-instance case IN A CATCH, other ApiErrors wrapped with what failed, and non-API errors propagating untouched. The set path's dead >=400 branch is replaced by the same catch-and-wrap. Also from the review: parseDbMemory is now case-EXACT (k8s quantities are case-sensitive; accepting '4gi' locally just deferred the rejection to the server), and fmtMb gets the tests its twin fmtMib already had. 181 tests passing (+7: four driving the seam with a stub — including the 502 branch no test previously took — plus fmtMb and case-exactness). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JQ9NX4AN5XEmb3sjBHvTzS --- src/commands/compute.ts | 2 +- src/commands/db.ts | 56 ++++++++++++++++++++++++++++++----------- test/limits.test.ts | 54 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 96 insertions(+), 16 deletions(-) diff --git a/src/commands/compute.ts b/src/commands/compute.ts index 23db700..18b62f3 100644 --- a/src/commands/compute.ts +++ b/src/commands/compute.ts @@ -106,7 +106,7 @@ export function parseMemoryMb(raw: string): number { // Whole and half GB collapse (1536 → "1.5 GB"); anything else stays exact in MB — a display that // rounds 1536 to "2 GB" claims a ceiling the API did not set. -const fmtMb = (mb: number) => (mb >= 1024 && mb % 512 === 0 ? `${mb / 1024} GB` : `${mb} MB`) +export const fmtMb = (mb: number) => (mb >= 1024 && mb % 512 === 0 ? `${mb / 1024} GB` : `${mb} MB`) // The --cpu override, through a throwing parser like every other user-typed number in this repo // (parseCount, parseMemoryMb). A bare Number() turns a typo into NaN, which JSON.stringify diff --git a/src/commands/db.ts b/src/commands/db.ts index 9a65832..7bfd593 100644 --- a/src/commands/db.ts +++ b/src/commands/db.ts @@ -1,4 +1,4 @@ -import { ApiClient, requireProject } from '../api.js' +import { ApiClient, ApiError, requireProject } from '../api.js' import { info, printJson, handleApproval } from '../util.js' type Opts = { branch?: string; group?: string; json?: boolean } @@ -26,13 +26,15 @@ export async function dbAlwaysOn(mode: string, opts: Opts): Promise { // Validated pass-throughs for the provider's quantity strings. The insta-db resize API takes // k8s-style quantities (cpu: "2", "2500m"; memory: "4Gi", "2048Mi"), so unlike the compute path // there is no unit conversion here — but junk must still fail LOCALLY with an example, not travel -// to the server as-is. +// to the server as-is. CASE-EXACT deliberately: k8s quantities are case-sensitive ("4gi" is +// rejected server-side), and local validation that accepts a form the backend refuses would +// defeat its own purpose. export function parseDbCpu(raw: string): string { if (!/^\d+(\.\d+)?m?$/.test(raw.trim())) throw new Error(`invalid cpu: ${raw} (try 2, 4, or 2500m)`) return raw.trim() } export function parseDbMemory(raw: string): string { - if (!/^\d+(\.\d+)?(Gi|Mi|G|M)$/i.test(raw.trim())) throw new Error(`invalid memory: ${raw} (try 4Gi or 8Gi)`) + if (!/^\d+(\.\d+)?(Gi|Mi|G|M)$/.test(raw.trim())) throw new Error(`invalid memory: ${raw} (try 4Gi or 8Gi)`) return raw.trim() } @@ -42,6 +44,31 @@ export function fmtMib(mib: number): string { return mib >= 1024 && mib % 512 === 0 ? `${mib / 1024} GiB` : `${mib} MiB` } +// The read outcome, as a seam. rawRequest THROWS ApiError on any status >= 400 (api.ts — it only +// differs from request in returning {status,body} below 400, for 202 branching), so the Neon case +// and the friendly wrapping must live in a catch, not in status branching on the return value — +// branches on res.status >= 400 after rawRequest are unreachable. Takes the client as an argument +// so tests drive it with a stub, per this repo's pure-seam convention. +export type DbInstanceRead = { kind: 'ok'; body: any } | { kind: 'no-instance' } + +export async function fetchDbInstance( + api: { rawRequest: (m: string, p: string) => Promise<{ status: number; body: any }> }, + projectId: string, + suffix: string, +): Promise { + try { + const res = await api.rawRequest('GET', `/projects/${projectId}/database/instance${suffix}`) + return { kind: 'ok', body: res.body } + } catch (e) { + // The platform answers a provider-shaped 502 for services with no manageable instance + // (Neon-backed): a soft case, not a failure. Everything else stays an error — an expired + // token must not render as "no ceiling set" — but wrapped so the user sees what failed. + if (e instanceof ApiError && e.status === 502) return { kind: 'no-instance' } + if (e instanceof ApiError) throw new Error(`reading the instance failed (${e.status}): ${e.message}`) + throw e + } +} + // Show or set a postgres service's resource ceiling (insta-db-backed only). Paid plans — the // ceiling is the tier lever now that billing follows actual usage. Moves both directions: // unlike storage it is a cgroup limit, not a provisioned volume. @@ -55,18 +82,14 @@ export async function dbLimits(opts: Opts & { cpu?: string; memory?: string }): const suffix = qs.toString() ? `?${qs}` : '' if (!opts.cpu && !opts.memory) { - // A real read against GET /database/instance. Errors PROPAGATE — an expired token or a 502 - // must not render as "no ceiling set"; the only softened case is a Neon-backed service, where - // the platform answers 502 provider-shaped because there is no manageable instance. - const res = await api.rawRequest('GET', `/projects/${p.projectId}/database/instance${suffix}`) - if (res.status === 502) { + const read = await fetchDbInstance(api, p.projectId, suffix) + if (read.kind === 'no-instance') { info(`postgres ${opts.group ?? 'default'}: no manageable instance (Neon-backed services manage their own resources)`) return } - if (res.status >= 400) throw new Error(`reading the instance failed (${res.status}): ${res.body?.error ?? 'unknown error'}`) - if (opts.json) return printJson(res.body) - const cpuMilli = res.body?.cpuMilli - const mib = res.body?.memoryMib + if (opts.json) return printJson(read.body) + const cpuMilli = read.body?.cpuMilli + const mib = read.body?.memoryMib if (typeof cpuMilli === 'number' && typeof mib === 'number') { const cpu = cpuMilli % 1000 === 0 ? `${cpuMilli / 1000}` : `${cpuMilli}m` info(`postgres ${opts.group ?? 'default'}: ceiling ${cpu} vCPU / ${fmtMib(mib)}`) @@ -80,9 +103,14 @@ export async function dbLimits(opts: Opts & { cpu?: string; memory?: string }): const body: Record = {} if (opts.cpu) body.cpu = parseDbCpu(opts.cpu) if (opts.memory) body.memory = parseDbMemory(opts.memory) - const res = await api.rawRequest('PATCH', `/projects/${p.projectId}/database/settings${suffix}`, body) + let res + try { + res = await api.rawRequest('PATCH', `/projects/${p.projectId}/database/settings${suffix}`, body) + } catch (e) { + if (e instanceof ApiError) throw new Error(`setting the ceiling failed (${e.status}): ${e.message}`) + throw e + } if (handleApproval(res)) return - if (res.status >= 400) throw new Error(`setting the ceiling failed (${res.status}): ${res.body?.error ?? 'unknown error'}`) if (opts.json) return printJson(res.body) const cpu = typeof res.body?.cpuMilli === 'number' ? `${res.body.cpuMilli / 1000} vCPU` : (opts.cpu ?? 'unchanged') const mem = typeof res.body?.memoryMib === 'number' ? fmtMib(res.body.memoryMib) : (opts.memory ?? 'unchanged') diff --git a/test/limits.test.ts b/test/limits.test.ts index 08a3457..515aa5b 100644 --- a/test/limits.test.ts +++ b/test/limits.test.ts @@ -38,7 +38,9 @@ describe('parseMemoryMb', () => { // Review round 2: both jwfing and cubic independently flagged the bare Number() on --cpu (NaN // serializes to null on the wire) and the unvalidated db strings. These pin the new seams. -import { parseCpu } from '../src/commands/compute.js' +import { parseCpu, fmtMb } from '../src/commands/compute.js' +import { fetchDbInstance } from '../src/commands/db.js' +import { ApiError } from '../src/api.js' import { parseDbCpu, parseDbMemory, fmtMib } from '../src/commands/db.js' describe('parseCpu (compute --cpu override)', () => { @@ -77,3 +79,53 @@ describe('fmtMib (display must not claim a ceiling the API did not set)', () => expect(fmtMib(512)).toBe('512 MiB') }) }) + +// The round-3 Critical: rawRequest THROWS on >=400, so the Neon soft-path must live in a catch — +// status-branching on its return value was unreachable dead code and a Neon read crashed with a +// raw ApiError. These drive the seam with a stub client, which is exactly the test that would +// have caught it (the 502 branch was never taken by any test). +describe('fetchDbInstance (the read seam)', () => { + const stub = (fn: () => Promise) => ({ rawRequest: fn }) as any + + it('returns the body on success', async () => { + const read = await fetchDbInstance(stub(async () => ({ status: 200, body: { cpuMilli: 4000 } })), 'p1', '') + expect(read).toEqual({ kind: 'ok', body: { cpuMilli: 4000 } }) + }) + + it('maps the provider-shaped 502 (Neon-backed) to the soft no-instance case', async () => { + const read = await fetchDbInstance(stub(async () => { throw new ApiError(502, 'provider request failed') }), 'p1', '') + expect(read).toEqual({ kind: 'no-instance' }) + }) + + it('wraps other API errors instead of rendering them as "no ceiling"', async () => { + await expect(fetchDbInstance(stub(async () => { throw new ApiError(401, 'unauthorized') }), 'p1', '')) + .rejects.toThrow(/reading the instance failed \(401\): unauthorized/) + }) + + it('lets non-API errors (network, bugs) propagate untouched', async () => { + const boom = new TypeError('fetch failed') + await expect(fetchDbInstance(stub(async () => { throw boom }), 'p1', '')).rejects.toBe(boom) + }) +}) + +// fmtMb had no tests while its twin fmtMib did — same must-not-lie property. +describe('fmtMb (compute display)', () => { + it('collapses whole and half GB', () => { + expect(fmtMb(2048)).toBe('2 GB') + expect(fmtMb(1536)).toBe('1.5 GB') + }) + it('keeps everything else exact in MB', () => { + expect(fmtMb(768)).toBe('768 MB') + expect(fmtMb(256)).toBe('256 MB') + }) +}) + +// Case-exactness (round-3 suggestion): k8s quantities are case-sensitive, so local validation +// must reject what the server would. +describe('parseDbMemory case-exactness', () => { + it('rejects lowercase unit variants the backend refuses', () => { + for (const raw of ['4gi', '4GI', '8m', '2048mi']) { + expect(() => parseDbMemory(raw), raw).toThrow(/invalid memory/) + } + }) +}) From 10ae254835a3dc2d25ac0e9ef3c0092500dd1db0 Mon Sep 17 00:00:00 2001 From: yaowenc2 Date: Mon, 3 Aug 2026 16:59:31 -0700 Subject: [PATCH 4/4] polish: parseCpu enforces the provider grid its help text advertises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 information note: --cpu accepted any positive integer while --help promised 1/2/4/6/8 — a value the client knows is invalid should fail locally rather than round-trip to the server. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JQ9NX4AN5XEmb3sjBHvTzS --- src/commands/compute.ts | 5 ++++- test/limits.test.ts | 9 ++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/commands/compute.ts b/src/commands/compute.ts index 18b62f3..8a96424 100644 --- a/src/commands/compute.ts +++ b/src/commands/compute.ts @@ -111,9 +111,12 @@ export const fmtMb = (mb: number) => (mb >= 1024 && mb % 512 === 0 ? `${mb / 102 // The --cpu override, through a throwing parser like every other user-typed number in this repo // (parseCount, parseMemoryMb). A bare Number() turns a typo into NaN, which JSON.stringify // serializes as null — the server then sees {cpu: null} instead of the user seeing an error. +// Enforces the provider grid the help text advertises: the server would reject 100 anyway, but a +// value the client KNOWS is invalid should fail locally, matching what --help promises. +const CPU_SIZES = [1, 2, 4, 6, 8] export function parseCpu(raw: string): number { const n = Number(raw) - if (!Number.isInteger(n) || n <= 0) throw new Error(`invalid cpu: ${raw} (provider sizes: 1, 2, 4, 6, 8)`) + if (!CPU_SIZES.includes(n)) throw new Error(`invalid cpu: ${raw} (provider sizes: ${CPU_SIZES.join(', ')})`) return n } diff --git a/test/limits.test.ts b/test/limits.test.ts index 515aa5b..c63d275 100644 --- a/test/limits.test.ts +++ b/test/limits.test.ts @@ -44,12 +44,11 @@ import { ApiError } from '../src/api.js' import { parseDbCpu, parseDbMemory, fmtMib } from '../src/commands/db.js' describe('parseCpu (compute --cpu override)', () => { - it('accepts positive integers', () => { - expect(parseCpu('1')).toBe(1) - expect(parseCpu('8')).toBe(8) + it('accepts exactly the provider grid the help text advertises', () => { + for (const n of [1, 2, 4, 6, 8]) expect(parseCpu(String(n))).toBe(n) }) - it('throws locally on junk instead of sending null to the server', () => { - for (const raw of ['abc', '', '-2', '1.5', 'two']) { + it('throws locally on junk AND on off-grid sizes instead of deferring to the server', () => { + for (const raw of ['abc', '', '-2', '1.5', 'two', '3', '100']) { expect(() => parseCpu(raw), raw).toThrow(/invalid cpu/) } })