feat: limits commands (insta compute limits / insta db limits)

[tonychang04]

CLI surface for the resource ceilings in InsForge/insta-platform#156 (+ the insta-db resize in InsForge/insta-db#95).

insta compute limits                          # ceiling 1 vCPU / 256 MB (plan max 2 vCPU / 2 GB)
insta compute limits --memory 1gb             # set — cpu derives from memory
insta compute limits --memory 1gb --cpu 2     # explicit override for parallel workloads
insta db limits --memory 8Gi --cpu 4          # same dial for postgres (insta-db-backed)

• --memory is the dial. Memory is the ceiling users actually feel (hitting it OOM-kills the app); vCPU only throttles. Deriving cpu keeps the provider's size grid out of the CLI's vocabulary entirely.
• Bare limits is a safe read — prints the current ceiling and the plan max, the same pair a UI renders as a slider with its plan-limit marker.
• Both directions. Lowering costs the customer nothing under usage billing and hands back headroom.
• Paid plans (platform-side gate); free stays pinned at the minimum.

Tests: 168 passing, including the parseMemoryMb seam (512, 512mb, 1gb, 1.5gb, 8Gi, and the junk it must refuse rather than guess at).

Ship after #156 and insta-db#95 are deployed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JQ9NX4AN5XEmb3sjBHvTzS

---

Summary by cubic

Add insta compute limits and insta db limits to view or set resource ceilings that cap usage while billing remains actual usage. Memory is the dial; inputs are validated and the display is precise; paid plans only.

• New Features

  • Bare insta compute limits and insta db limits print the current ceiling; compute also shows the plan cap.
  • Set with --memory for compute (CPU derives unless --cpu is given and must be 1, 2, 4, 6, or 8); Postgres uses k8s quantities for --cpu/--memory (case‑exact).
  • Examples: insta compute limits [service] --memory 1gb [--cpu 2], insta db limits --memory 8Gi --cpu 4.

• Bug Fixes

  • Postgres read path is robust: Neon-backed services report “no manageable instance”; other API errors show status and message instead of crashing.
  • Display is honest: whole/half GB(GiB) collapse only; everything else stays exact in MB/MiB.

^{Written for commit 10ae254. Summary will update on new commits.}

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[jwfing]

Review: feat: limits commands (insta compute limits / insta db limits)

Summary: Adds insta compute limits and insta db limits (read-or-set resource ceilings) as thin, convention-matching wrappers over the platform API, with a well-tested memory-parsing seam on the compute path; nothing blocking.

Requirements context: No matching spec/plan found — this repo has no docs/ (no docs/superpowers/ or docs/specs/) and no DEVELOPMENT.md. Assessed against the PR description and the referenced (external, unmerged) contracts InsForge/insta-platform#156 and InsForge/insta-db#95. The PR itself notes it should ship only after those deploy.

Critical

(none) — no correctness bug, no security hole, no break to existing behavior. The commands follow the established shape in compute.ts/db.ts (ApiClient.load → requireProject → rawRequest/request → handleApproval/printJson) exactly.

Suggestion

• [functionality] --cpu on compute forwards NaN silently — src/commands/compute.ts:131. if (opts.cpu) body.cpu = Number(opts.cpu) has no NaN guard: --cpu 2gb (or any non-numeric) becomes NaN, which JSON.stringify serializes to null, sending an unintended value rather than raising a clear CLI error. This is exactly the "refuse junk rather than guess" behavior parseMemoryMb was built for — the cpu field deserves the same guard.
• [functionality/consistency] db --memory bypasses the parsing seam entirely — src/commands/db.ts:51. The PR frames the MB conversion as the risk ("getting that conversion wrong sets a ceiling an order of magnitude off"), and the compute path guards it carefully — but the db path forwards opts.memory as a raw string with no client-side validation and no test. Either validate it's a plausible quantity, or add a one-line note that db intentionally defers to the server's k8s-quantity parsing (it looks intentional given --memory 8Gi, but it's undocumented and asymmetric with compute).
• [test coverage] only parseMemoryMb is exercised — test/limits.test.ts. The two command functions (branch/group query building, the read-vs-set branch selection, --json output, response-shape reads) have no tests, unlike neighbors (services.test.ts, billing.test.ts mock the API client). Not blocking, but a wiring test would catch response-shape drift against #156/#95 before it reaches a user.

Information

• Response-shape coupling is unverified against the unmerged platform PRs. compute read assumes r.limits.{cpu,memoryMb} + r.cap.{cpu,memoryMb} (compute.ts:124), compute set assumes res.body.limits (compute.ts:135), db assumes cpuMilli/memoryMib (db.ts:41-42,55-56). These are the surfaces most likely to break if #156/#95 land with different field names — worth a quick reconcile before enabling.
• Display rounding on db read can misreport sub-GiB — src/commands/db.ts:42. Math.round(r.memoryMib / 1024) renders 512 MiB and 768 MiB both as "1 GiB". Consider the fractional formatting the compute path uses.
• fmtMb only collapses exact multiples of 1024 — src/commands/compute.ts:107. A 1.5gb ceiling round-trips to display as "1536 MB" rather than "1.5 GB". Cosmetic. Also note compute reports base-name memoryMb while db reports memoryMib/GiB — mixed units across the two commands.
• compute bare-read has no graceful fallback while db does. db.ts:39 wraps the GET in .catch(() => null) and prints a friendly "unavailable"; compute.ts:122 will surface a raw API error on e.g. a free plan or a missing endpoint. Minor consistency gap.

Security & Performance

• Security: no relevant issues. User input (branch/group) flows through URLSearchParams (encoded); memory/cpu go into JSON bodies validated server-side; the service id is resolved from the server's own list, not user-injected into the path. No SQL/shell, no secrets logged.
• Performance: no concern. Each command makes at most one extra GET (service resolution) plus the mutation — same pattern as the existing lifecycle commands; no loops or N+1.

Verdict

approved (informational — human approval still required via the approve flow). Zero Critical findings; the Suggestions above (notably the --cpu NaN guard and the db --memory validation asymmetry) are worth a look but non-blocking. Confirm the #156/#95 response shapes before shipping, as the PR already flags.

[jwfing]

LGTM - approved.

[cubic-dev-ai]

2 issues found across 4 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/commands/compute.ts">

<violation number="1" location="src/commands/compute.ts:104">
P2: Small positive memory values can become a zero ceiling, and oversized values can become `null` on the wire because finiteness and the rounded result are not validated; rejecting non-finite values and values that round below 1 MB would prevent malformed limit updates.</violation>
</file>

<file name="src/commands/db.ts">

<violation number="1" location="src/commands/db.ts:42">
P2: `insta db limits` omits the plan maximum from its bare read, unlike the compute limits command and the promised limits UX. Including only the current Postgres ceiling leaves users unable to tell how far they may raise it; the read should render the plan-cap fields returned by the database limits API (or the API should expose them before this command is shipped).</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Small positive memory values can become a zero ceiling, and oversized values can become null on the wire because finiteness and the rounded result are not validated; rejecting non-finite values and values that round below 1 MB would prevent malformed limit updates.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/compute.ts, line 104:

<comment>Small positive memory values can become a zero ceiling, and oversized values can become `null` on the wire because finiteness and the rounded result are not validated; rejecting non-finite values and values that round below 1 MB would prevent malformed limit updates.</comment>

<file context>
@@ -89,3 +89,49 @@ export async function computeAlwaysOn(mode: string, serviceName: string | undefi
+  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)
+}
+
</file context>

Suggested change

	return Math.round(mb)
	const rounded = Math.round(mb)
	if (!Number.isFinite(rounded) || rounded < 1) throw new Error(`invalid memory: ${raw}`)
	return rounded

[cubic-dev-ai]

P2: insta db limits omits the plan maximum from its bare read, unlike the compute limits command and the promised limits UX. Including only the current Postgres ceiling leaves users unable to tell how far they may raise it; the read should render the plan-cap fields returned by the database limits API (or the API should expose them before this command is shipped).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/db.ts, line 42:

<comment>`insta db limits` omits the plan maximum from its bare read, unlike the compute limits command and the promised limits UX. Including only the current Postgres ceiling leaves users unable to tell how far they may raise it; the read should render the plan-cap fields returned by the database limits API (or the API should expose them before this command is shipped).</comment>

<file context>
@@ -22,3 +22,37 @@ export async function dbAlwaysOn(mode: string, opts: Opts): Promise<void> {
+    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`)
</file context>

[jwfing]

Review — feat: limits commands (insta compute limits / insta db limits)

Summary: Two new commander commands (compute limits, db limits) that read/set a resource ceiling; the implementation follows the repo's established command shape faithfully, the memory-parsing seam is well-tested, and I found no blocking issues.

Requirements context

No spec/plan directory exists in this repo — there is no docs/superpowers/, docs/specs/, or docs/ at all (only README.md, CONTRIBUTING.md, CLAUDE.md, AGENTS.md, and .claude/skills/developing-insta-cli/SKILL.md). No matching spec/plan found — assessing against the PR description, the linked InsForge/insta-platform#156 / InsForge/insta-db#95, and the repo's developing-insta-cli skill conventions. The design intent (memory is the dial, cpu derives, bare limits is a safe read, paid-plan gated server-side) comes from the PR body.

---

Critical

(none) — no correctness, security, data-loss, or convention-breaking blockers.

Suggestion

Software engineering / functionality — --cpu on compute bypasses the repo's validated-parser convention (src/commands/compute.ts:131)
body.cpu = Number(opts.cpu) converts with a bare Number() and no guard. Number('abc') → NaN, which JSON.stringify serializes to null in the PUT body — the server sees {cpu: null} rather than a clean local rejection. This repo has a strong, consistent convention of pure, throwing, unit-tested parsers for exactly this (parseMemoryMb, and parseCount / parseAccess in services.ts:25,132). Consider a small parseCpu seam (positive number, matching the documented 1,2,4,6,8 grid) so a typo fails fast and locally, mirroring what you already did for memory.

Software engineering / functionality — the db limits set path is entirely unvalidated and untested (src/commands/db.ts:49-51)
--cpu / --memory are passed straight through as raw strings (body.cpu = opts.cpu; body.memory = opts.memory). I understand the asymmetry is real — the insta-db resize API takes strings like 2500m / 8Gi while the compute API takes an MB number, so the tested parseMemoryMb seam legitimately only applies to compute. But the consequence is that insta db limits --memory huge sends huge to the server with no local check, and unlike the compute path there's no pure seam and no test covering the db formatting/branch. At minimum, a light validation (or a shared normalizer) plus a unit test on the cpuMilli/memoryMib display math would bring parity.

Functionality — db limits read swallows every error (src/commands/db.ts:39)
.catch(() => null) on the GET /database/instance collapses auth failures, network errors, and a genuinely-missing endpoint into the same current ceiling unavailable message. That can mask a real problem (e.g. an expired token) as "no ceiling set". Consider only catching the not-found/unsupported case, or at least surfacing that the read failed vs. is unset.

Functionality — parity gap: db limits read shows no plan max (src/commands/db.ts:41-45)
compute limits prints ... (plan max N vCPU / M GB) and the PR frames the read as "the pair a UI renders as a slider with its plan-limit marker" — but the db read prints only the current ceiling, no cap. If the db instance endpoint doesn't return a cap that's fine, but the "slider marker" story then only holds for compute; worth a note so the two commands aren't assumed symmetric.

Information

• CLI surface changed — mirror it in the superproject reference. developing-insta-cli/SKILL.md states command/flag additions must be mirrored in skills/insta/cli-reference.md "in the same change set." That file is a superproject submodule and is not present in this repo (no skills/, no .gitmodules), and the analogous prior PR (82bd0e9, always-on) also didn't touch it in-repo — so this is a reminder for the parent-repo changeset, not a blocker on this diff.
• Response-shape coupling to unmerged upstreams. computeLimits assumes r.limits.{cpu,memoryMb}, r.cap.{cpu,memoryMb}, and res.body.limits; dbLimits assumes cpuMilli / memoryMib. These are tied to the still-undeployed #156 / insta-db#95 and there's no defensive handling if a shape differs (e.g. a missing res.body.limits throws on .cpu). The PR already gates on "ship after deploy," so this is just a flag, not a request to add fallbacks.
• parseMemoryMb treats GB/GiB (and MB/MiB) identically — both ×1024 / ×1. Intentional and correct for a cgroup limit, and the tests pin it (8Gi → 8192), but a user typing 1gb expecting 1000 MB gets 1024. Purely informational.

Non-findings (dimensions covered)

• Security: no security-relevant changes. All user input flows through api.rawRequest as a JSON body (no shell, no SQL, no string-built URLs); branch/group go through URLSearchParams encoding; service names resolve to server ids via resolveComputeServiceId; approval gating (handleApproval) and auth headers are preserved unchanged. No secrets logged.
• Performance: no concerns. Each command is 1–2 API round-trips (GET services → GET/PUT limits), consistent with the existing lifecycle/status commands; no loops, no N+1.
• Test coverage: the tested surface (parseMemoryMb) matches the repo convention — pure helpers are unit-tested with DI, network handlers are not (services.ts handlers are likewise untested). The parse tests are thorough (bare/mb/gb/Gi/decimals + a solid junk-rejection set incl. 1tb, -1gb, 0, 1 2gb, and the error-message assertion).

---

Verdict

approved (informational — the GitHub green-check is a separate human action). Zero Critical findings; the Suggestions above are worth a look but none block merge. Land after InsForge/insta-platform#156 and InsForge/insta-db#95 are deployed, per the PR.

[jwfing]

Review: feat: limits commands (insta compute limits / insta db limits)

Summary: Clean, well-commented CLI surface for the platform#156 / insta-db#95 resource-ceiling model, with a genuinely good parsing-seam test suite — but the insta db limits read path leans on a client contract that doesn't hold, leaving its 502/Neon and error-wrapping branches as unreachable dead code.

Requirements context

No matching spec/plan in this repo — insta-cli has no docs/superpowers/ (and no docs/ at all in this tree), so there is nothing to assess against locally. Intent taken from the PR body and the two upstream PRs it fronts: InsForge/insta-platform#156 (memoryMb-is-the-dial ceiling model, cpu derived, per-tier caps, both-directions, paid-plan gate) and InsForge/insta-db#95 (k8s-quantity resize). The CLI surface here matches that model (memory as the dial for compute, k8s-quantity pass-through for db, safe bare read + plan-max display).

---

Critical

Functionality — src/commands/db.ts:57-77: the 502/Neon soft-path and the error-wrap branch are unreachable dead code.
dbLimits reads the instance with api.rawRequest('GET', …/database/instance…) and then branches on the returned status:

const res = await api.rawRequest('GET', `…/database/instance${suffix}`)
if (res.status === 502) { info(`… no manageable instance (Neon-backed …)`); return }
if (res.status >= 400) throw new Error(`reading the instance failed (${res.status}): …`)

But rawRequest throws ApiError on any status ≥ 400 before it ever returns — see src/api.ts:45-49 (and the file header comment: "2xx (including 202 approval_required) returns the parsed body; >=400 throws ApiError"). rawRequest differs from request only in that it returns {status, body} for < 400 so callers can branch on 202; it does not hand back 4xx/5xx.

Consequences:

• insta db limits (the "safe read") against a Neon-backed service hits the platform's provider-shaped 502, rawRequest throws, and the user sees a raw ApiError instead of the intended no manageable instance (Neon-backed services manage their own resources) message. The documented graceful degradation never runs.
• The if (res.status >= 400) throw new Error('reading the instance failed …') friendly wrapper is likewise dead — every other read error (expired token, 500, etc.) surfaces as the bare ApiError message rather than the intended wrapped one.

Fix: this needs a non-throwing call. Either wrap in try/catch and inspect e instanceof ApiError && e.status === 502, or add/​use a client method that returns {status, body} without throwing on ≥400. The set path has the same dead if (res.status >= 400) throw at db.ts:85 — there it's only cosmetic (an ApiError is thrown either way), but it should be cleaned up with the same fix so the intended wrapped message is what users get.

Because a documented, guaranteed-to-be-hit behavior (Neon read → friendly message) is instead an error, this is a correctness bug rather than a style nit → request_changes.

---

Suggestion

• Software engineering — no test exercises the command functions themselves (db.ts / compute.ts). The test file (test/limits.test.ts) pins the pure parsers well, but computeLimits/dbLimits have zero coverage. A test that stubs ApiClient and drives the db read path would have caught the Critical above immediately (the 502 branch is never taken). Given the repo already stubs the API elsewhere (e.g. services.test.ts, billing.test.ts), a couple of read-path tests here would be in-convention and high-value.
• Software engineering — fmtMb (compute display) is untested while its twin fmtMib (db) has a dedicated describe. The 1536→"1.5 GB" / "don't claim a ceiling the API didn't set" logic is exactly the kind the db side found worth pinning; mirror it for compute (src/commands/compute.ts:109).
• Functionality — parseDbMemory regex uses the /i flag (src/commands/db.ts:35), so it accepts case variants k8s rejects. 4gi, 4g, 8m all pass local validation and are forwarded verbatim (raw.trim()), but k8s quantities are case-sensitive (Gi, Mi, G, M) — the server/insta-db then rejects them. Local validation that accepts a form the backend refuses defeats the stated goal ("junk must still fail LOCALLY with an example"). Consider dropping /i (and doing the same for the m suffix in parseDbCpu, db.ts:31).

Information

• Functionality — parseMemoryMb conflates binary and decimal units (src/commands/compute.ts:98-104). Gi/Mi/Gib/Mib are accepted but treated identically to Gb/Mb (512mib→512, 8Gi→8192 via n*1024). For a ceiling the imprecision is immaterial, but a user typing Gi expecting strict binary semantics gets decimal-ish MB. Fine to leave; noting for awareness.
• Functionality — parseCpu accepts any positive integer (compute.ts:114-117), not just the provider grid [1,2,4,6,8] the help text advertises (e.g. --cpu 100 passes). Relying on the server to reject out-of-grid values is a reasonable choice (server is source of truth); just flagging the CLI/help mismatch.
• Operational — ship ordering. The PR body already states this must land after platform#156 and insta-db#95 are deployed; per the upstream context the db PATCH …/database/settings {cpu,memory} resize only takes effect once insta-db's real resize is live (it was a FakeInstaDb-only no-op earlier). No code change needed here — just confirming the gate before merge/release.

Security

No security-relevant changes. All user input is validated locally and sent as a JSON body via the existing parameterized ApiClient (no SQL/shell/string interpolation into the request). No secrets, tokens, or PII are logged or newly returned. No auth/authorization logic is touched — the paid-plan gate stays server-side.

Performance

No concerns. Each command does one GET /services to resolve the service id followed by a single limits GET/PUT/PATCH; no loops, no N+1, no blocking work.

---

Verdict: request_changes

One Critical: the dbLimits read path branches on a rawRequest return value for status ≥ 400, but rawRequest throws on ≥400 (api.ts:45-49), so the Neon 502 soft-path and error-wrapping are unreachable — insta db limits against a Neon-backed service errors instead of printing the intended message. Everything else (parsing seam, tests for it, display formatters, command wiring) is solid and in-convention.

[tonychang04]

Round-3 Critical fixed — you were right, both branches were dead: rawRequest throws on ≥400 (api.ts), so the Neon 502 soft-path never ran. The read now goes through fetchDbInstance(api, …), a stub-drivable seam per the repo's pure-seam convention: 502 → soft no-instance message in a catch, other ApiErrors wrapped with what failed, non-API errors untouched. The set path's dead branch got the same treatment.

Suggestions: parseDbMemory is now case-exact (4gi rejected locally, matching the server), and fmtMb has the tests its twin had. 181 passing — including a test that drives the 502 branch, which is exactly the test that would have caught this.

🤖 Generated with Claude Code

[jwfing]

Review: feat: limits commands (insta compute limits / insta db limits)

Summary: A clean, well-tested CLI surface for the resource-ceiling endpoints from insta-platform#156 / insta-db#95; the previously-flagged blocking issue is fixed and the remaining notes are non-blocking.

Requirements context

No /docs/superpowers/ or docs/specs/ directory exists in this repo — the feature spec lives in the companion PRs referenced by the description (InsForge/insta-platform#156 for the ceiling model, InsForge/insta-db#95 for the postgres resize). Assessed against the PR description, the AGENTS.md / .claude/skills/developing-insta-cli conventions, and the platform API contract as used elsewhere in src/. This CLI is a thin client, so correctness here means faithfully surfacing those endpoints and validating input locally.

Verified locally at head d98b52c: npx tsc --noEmit clean, vitest run → 181 tests pass (incl. the new test/limits.test.ts), matching the pre-commit gate in AGENTS.md.

---

Findings

Critical

(none) — Notably, the earlier round's blocking issue (the Neon-502 soft-path was branching on res.status after rawRequest, which is unreachable because rawRequest throws ApiError on any status ≥ 400 — so a Neon read crashed with a raw ApiError) is now correctly fixed. fetchDbInstance (src/commands/db.ts:44-63) maps the soft case inside a catch via e instanceof ApiError && e.status === 502, re-wraps other API errors, and lets non-API errors propagate — and it's driven by a stubbed client in test/limits.test.ts:85-108, exactly the test that would have caught the original bug.

Suggestion

• Functionality / robustness — compute read & set display assume the exact response shape (src/commands/compute.ts:135, :146). The read prints ${r.limits.cpu} … ${fmtMb(r.limits.memoryMb)} … ${r.cap.cpu} … ${fmtMb(r.cap.memoryMb)} and the set prints res.body.limits.cpu, all unguarded. If the platform ever omits or renames limits/cap (e.g. an error envelope, or a shape drift from #156), the user gets a raw TypeError: Cannot read properties of undefined (reading 'cpu') instead of a clean CLI error. The sibling db path handles the same uncertainty defensively — it guards with typeof cpuMilli === 'number' and falls back to provider reported no ceiling (src/commands/db.ts:78-91). Mirroring that guarding on the compute path would make the two ceiling commands fail the same, friendly way. Low blast radius (depends on the API honoring its contract), hence non-blocking.

Information

• Error-handling asymmetry between the two set paths (informational, not a defect). dbLimits wraps a failed PATCH in throw new Error('setting the ceiling failed (…): …') (db.ts:110-114), while computeLimits lets the PUT's ApiError propagate to the top-level guard()/onError, which already renders ${message} (HTTP ${status}) (src/index.ts:28-35). Both produce a clean message, so neither is wrong — just noting the two paths chose different styles for the same situation.
• parseMemoryMb treats mib/mi as MB (compute.ts parseMemoryMb). 512mib → 512 (sent as memoryMb), conflating mebibytes with the server's nominal MB. This is intentional and explicitly pinned by test/limits.test.ts:15-17; noted only so the MiB≈MB conflation is a documented choice rather than an oversight.

Software engineering / Security / Performance

• Software engineering: Strong. New behavior is covered by focused, well-commented tests — parser seams (parseMemoryMb, parseCpu, parseDbCpu, parseDbMemory), both display formatters (fmtMb/fmtMib incl. the must-not-round-up property), and the fetchDbInstance read seam across ok / 502-soft / other-ApiError / non-ApiError. Import style (.js ESM), throw new Error for local validation, handleApproval 202 branching, and the api.request (body) vs api.rawRequest ({status,body}) split all match the surrounding code. --memory-is-the-dial with server-side cpu derivation correctly keeps the provider size grid out of the CLI.
• Security: No security-relevant changes. All user-typed sizes go through throwing validators before hitting the wire — the db path is deliberately case-exact so local validation rejects exactly what k8s rejects server-side (no permissive-local/strict-server gap). No shell/SQL interpolation, no new secrets logged or returned, auth/refresh untouched, no new dependencies.
• Performance: Negligible. Each command is one services GET to resolve the id plus a single GET/PUT/PATCH; no loops, no N+1, no blocking work.

---

Verdict

approved (informational — the GitHub green-check is a separate human action). Zero Critical findings; the two Suggestion/Information items are optional polish. The prior blocking issue has been resolved and regression-tested. As the description notes, this should merge only after insta-platform#156 and insta-db#95 are deployed, since the endpoints it calls come from those PRs.

[cubic-dev-ai]

3 issues found across 4 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/commands/db.ts">

<violation number="1" location="src/commands/db.ts:87">
P2: `insta db limits --json` emits plain text for Neon-backed/no-instance reads because this branch returns through `info` before the JSON handling. Machine-readable callers cannot safely parse that response; the no-instance branch could return a JSON sentinel (or preserve the prior `{}` response) before printing the human-readable message.</violation>

<violation number="2" location="src/commands/db.ts:94">
P2: Non-integer CPU ceilings are displayed as `1500m vCPU` instead of the decimal-vCPU format used after setting them. Format all `cpuMilli` values as `${cpuMilli / 1000}` before the existing `vCPU` suffix.</violation>

<violation number="3" location="src/commands/db.ts:104">
P2: Explicitly empty `--cpu`/`--memory` values bypass the new parsers and silently turn a requested set into a read. Check option presence with `!== undefined` in both the read predicate and payload construction so empty values are rejected locally.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Explicitly empty --cpu/--memory values bypass the new parsers and silently turn a requested set into a read. Check option presence with !== undefined in both the read predicate and payload construction so empty values are rejected locally.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/db.ts, line 104:

<comment>Explicitly empty `--cpu`/`--memory` values bypass the new parsers and silently turn a requested set into a read. Check option presence with `!== undefined` in both the read predicate and payload construction so empty values are rejected locally.</comment>

<file context>
@@ -36,23 +82,37 @@ export async function dbLimits(opts: Opts & { cpu?: string; memory?: string }):
-  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 (opts.cpu) body.cpu = parseDbCpu(opts.cpu)
+  if (opts.memory) body.memory = parseDbMemory(opts.memory)
+  let res
</file context>

[cubic-dev-ai]

P2: Non-integer CPU ceilings are displayed as 1500m vCPU instead of the decimal-vCPU format used after setting them. Format all cpuMilli values as ${cpuMilli / 1000} before the existing vCPU suffix.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/db.ts, line 94:

<comment>Non-integer CPU ceilings are displayed as `1500m vCPU` instead of the decimal-vCPU format used after setting them. Format all `cpuMilli` values as `${cpuMilli / 1000}` before the existing `vCPU` suffix.</comment>

<file context>
@@ -36,23 +82,37 @@ export async function dbLimits(opts: Opts & { cpu?: string; memory?: string }):
+    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)}`)
+      info('  billing is actual usage — the ceiling caps what the database may burn, it is not a price')
</file context>

Suggested change

	const cpu = cpuMilli % 1000 === 0 ? `${cpuMilli / 1000}` : `${cpuMilli}m`
	const cpu = `${cpuMilli / 1000}`

[cubic-dev-ai]

P2: insta db limits --json emits plain text for Neon-backed/no-instance reads because this branch returns through info before the JSON handling. Machine-readable callers cannot safely parse that response; the no-instance branch could return a JSON sentinel (or preserve the prior {} response) before printing the human-readable message.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/db.ts, line 87:

<comment>`insta db limits --json` emits plain text for Neon-backed/no-instance reads because this branch returns through `info` before the JSON handling. Machine-readable callers cannot safely parse that response; the no-instance branch could return a JSON sentinel (or preserve the prior `{}` response) before printing the human-readable message.</comment>

<file context>
@@ -36,23 +82,37 @@ export async function dbLimits(opts: Opts & { cpu?: string; memory?: string }):
-      info(`postgres ${opts.group ?? 'default'}: ceiling ${(r.cpuMilli / 1000).toFixed(r.cpuMilli % 1000 ? 1 : 0)} vCPU / ${Math.round(r.memoryMib / 1024)} GiB`)
+    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
+    }
</file context>

Suggested change

	info(`postgres ${opts.group ?? 'default'}: no manageable instance (Neon-backed services manage their own resources)`)
	if (opts.json) return printJson({})
	info(`postgres ${opts.group ?? 'default'}: no manageable instance (Neon-backed services manage their own resources)`)

[jwfing]

Review: feat: limits commands (insta compute limits / insta db limits)

Summary: Clean, well-tested addition of insta compute limits and insta db limits — the resource-ceiling CLI surface for InsForge/insta-platform#156 and InsForge/insta-db#95; parsing/formatting/read seams are extracted and unit-tested per repo convention, and the earlier round-3 Neon dead-code bug is correctly resolved in this head commit.

Requirements context: No matching spec/plan found — this repo has no docs/superpowers/ or docs/specs/ directory. Assessed against the PR description, the linked platform (#156) / insta-db (#95) PRs, and existing repo conventions (src/api.ts, src/util.ts, neighboring compute/db commands). No documented spec to check the surface against.

I verified the new tests locally: vitest run test/limits.test.ts → 18/18 passing.

---

Critical

(none) — nothing blocks merge.

Note for context: the prior review round flagged the Neon soft-path as unreachable dead code (status-branching after rawRequest, which throws on ≥400). This head commit fixes it correctly: fetchDbInstance now maps the provider 502 inside a catch (e instanceof ApiError) (src/commands/db.ts:55-71), which matches rawRequest's throw-on-≥400 contract (src/api.ts:45-49), and the seam is now driven by a stubbed client in tests (200 / 502 / 401 / non-API error). Good fix.

Suggestion

• Functionality — compute read path is less defensive than its db twin (src/commands/compute.ts:138). The bare limits read dereferences r.limits.cpu / r.limits.memoryMb / r.cap.cpu / r.cap.memoryMb directly. If /services/:id/limits ever returns a partial or unexpected body, r.limits being undefined throws TypeError: Cannot read properties of undefined, which guard/onError renders as a raw message. The db read path (src/commands/db.ts:82-92) already handles this gracefully with typeof checks and a "provider reported no ceiling" fallback. Low blast radius (it's your own new endpoint), but mirroring the db path's guard would keep the two commands consistent.
• Functionality — 502→no-instance mapping keys on status alone (src/commands/db.ts:66). Any ApiError with status 502 is rendered as no manageable instance (Neon-backed services manage their own resources) (db.ts:87). If the platform ever emits a transient gateway 502 for an insta-db-backed service, the user is told their DB manages its own resources rather than seeing a retryable error. The behavior matches the documented platform contract, so this is non-blocking — but a body-shape check (or a platform error code) would decouple the soft path from a bare status number.
• Test coverage — display formatting is untested (src/commands/db.ts:112-117, compute.ts:137-140). The pure seams (parseMemoryMb, parseCpu, parseDb*, fmtMb, fmtMib, fetchDbInstance) are thoroughly covered, but the command-level rendering (cpuMilli→vCPU collapse, the read-path info(...) lines) has no test. Simple logic, low risk — noting for completeness.

Information

• Style — inconsistent label source between the read and set paths: the read prints serviceName ?? id (compute.ts:138) while the set/lifecycle paths print res.body.service?.name ?? id (compute.ts:89, db.ts:118). Harmless, but the read label falls back to the raw id even when the API returns a friendlier name.
• parseMemoryMb conflates binary/decimal units (mib/mi treated as MB, gi/gib as ×1024 like gb) — intentional and documented in the comment; noting only so it's a conscious choice, not a slip.

Security: No security-relevant changes. All user-typed values pass through throwing validators before hitting the wire (parseMemoryMb/parseCpu/parseDbCpu/parseDbMemory), branch/group are URL-encoded via URLSearchParams, no new secrets are logged or returned, and no auth path is touched. No new dependencies.

Performance: No concerns — each command is one or two bounded round-trips (services list to resolve the id, then the limits read/write), consistent with the sibling compute/db commands. No loops, no hot-path work.

---

Verdict: approved (informational)

Zero Critical findings. The suggestions above are non-blocking polish. (Per bot policy this is posted as a COMMENT; the explicit GitHub green-check is a separate human action.)

[jwfing]

LGTM - approved.