cli: --no-always-on creates a scale-to-zero compute service

[tonychang04]

What

Companion to insta-platform #385 (new compute services are born always_on = true).

insta services add had a boolean-on --always-on and built the body with a truthiness spread, so once the platform default flips there is no way to create a scale-to-zero compute service from the CLI. Now:

• --no-always-on creates a scale-to-zero compute service; the flag is forwarded in both states, omitted means the platform default.
• Help text for services add --always-on and compute always-on no longer calls scale-to-zero the default.

Tests

services.test.ts: the create body carries alwaysOn: false for --no-always-on, true for --always-on, nothing when absent. Commander checked: the paired flags yield undefined / true / false. npm run typecheck clean.

Platform PR: https://github.com/InsForge/insta-platform/pull/385 · Agent-facing reference (skills/insta/cli-reference.md lives in the insta-skills repo): InsForge/instacloud-skills#77

Review round 1

• Critical, taken: the compute-only validation tested truthiness, so services add postgres db --no-always-on reached the platform with an explicit false; it is a presence check now, with a regression test for the false case.
• Critical, taken: the cli-reference.md update lives in insta-skills (the skills/ submodule of the superproject, not checked out here): insta-skills chore: release 0.0.27 #77.
• Suggestion, taken: the compute always-on off success message no longer calls scale-to-zero the default.

🤖 Generated with Claude Code

https://claude.ai/code/session_01G1fBttq231ZKpU5ivCAG2i

Review round 2 (ee55aad)

• john-bot COMMENTED, zero Criticals. Declined, recorded: a Commander-level parse test for --always-on / --no-always-on. Verified by hand (none: undefined | --always-on: true | --no-always-on: false) and the option registration is one line next to its siblings, none of which carry a parse test either.
• r2d2 COMMENTED, zero Criticals, same suggestion with a negative control (deleting the --no-always-on option leaves the suite green). Recorded: the Commander layer is unbound in this repo for every option, and binding it is its own change; the flag's semantics were verified by hand at this head.

[jwfing]

Summary
The create-body change handles the core alwaysOn: false case, but the PR is not ready because a required CLI reference update is missing and --no-always-on is not validated consistently.

Requirements context
I used the PR title/description as the behavior spec: --always-on sends alwaysOn: true, --no-always-on sends alwaysOn: false, and omitting both leaves the platform default. The repo instructions also make command/flag documentation mandatory: AGENTS.md:15-17 and .claude/skills/developing-insta-cli/SKILL.md:36-38 require mirroring command/flag changes in skills/insta/cli-reference.md. The linked platform PR was not readable from this environment; the public CLI reference still lists services add with --always-on but not --no-always-on: https://github.com/InsForge/insta-skills/blob/main/insta/cli-reference.md#L16-L17

Findings

Critical:

• AGENTS.md:15-17, .claude/skills/developing-insta-cli/SKILL.md:36-38, src/index.ts:138-139 - This PR adds a user-facing services add --no-always-on flag, but the mandatory skills/insta/cli-reference.md update is not included. That reference is explicitly the agent-facing CLI surface, so agents will still learn that only --always-on exists and will miss the new opt-out path. Please update the reference with --no-always-on, the new default semantics, and the revised scale-to-zero wording.
• src/commands/services.ts:98-115, src/index.ts:138-139, test/services.test.ts:149-158 - --no-always-on bypasses the compute-only validation. The request body now forwards any defined opts.alwaysOn, including false, but servicesAdd only rejects non-compute usage when opts.alwaysOn is truthy. A command such as insta services add postgres db --no-always-on can therefore reach the platform with alwaysOn: false instead of failing before config/network access like the other compute-only flags. Use an explicit presence check, for example opts.alwaysOn !== undefined && type !== 'compute', and add a regression test for the false case.

Suggestion:

• src/commands/compute.ts:589-602 - The compute always-on off success message still says scale-to-zero is the default. That is outside the changed help text, but it preserves the stale user-facing wording this PR is trying to remove after the platform default flips.

Information:

• test/services.test.ts:139-145 - The unit test covers the true, false, and absent request-body states and would catch the old truthiness spread regression.
• src/commands/services.ts:93-103, src/index.ts:138-140 - No security-relevant changes found: this only changes a boolean CLI option and outbound API body field, with no new auth, secret, SQL, shell, or dependency surface.
• src/commands/services.ts:93-103, src/index.ts:138-140 - No performance-relevant changes found: there are no new loops, blocking operations, queries, or large allocations.

Verdict
Request changes. I attempted npm run typecheck, but it failed because tsc is not installed in this checkout; I did not install dependencies or mutate the workspace.

[cubic-dev-ai]

All reported issues were addressed across 3 files

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

Re-trigger cubic</sub>

[jwfing]

Summary
The PR implements tri-state alwaysOn handling for insta services add and updates the user-facing default wording; I found no blocking issues.

Requirements context
I derived intent from the PR title/description: once platform PR InsForge/insta-platform#385 changes new compute services to always_on = true, insta services add compute ... --no-always-on must send explicit alwaysOn: false, --always-on must still send true, and omitting the flag must leave the platform default. The local README only shows generic service creation (README.md:66-67) and does not document compute always-on defaults. Repo guidance requires command/flag changes to be mirrored in the agent-facing reference (AGENTS.md:15-17, CONTRIBUTING.md:41-44); the linked docs companion insta-skills#77 covers that surface update. The project uses Commander for the command surface (CONTRIBUTING.md:22-23) and pins Commander 12.1.0 (package-lock.json:1026-1033); I also checked the Commander docs for boolean/negated option semantics. The linked platform PR page did not expose readable content from this environment, so I assessed the platform-default requirement from this PR's description and the linked skills PR.

Findings

Critical
(none)

Suggestion

• src/index.ts:138-139, test/services.test.ts:139-145 - Consider adding a small Commander-level regression test that parses services add compute api, --always-on, and --no-always-on to undefined/true/false. The request-body tests cover forwarding once alwaysOn reaches servicesAdd, but they would not fail if the command registration later dropped or renamed the new negated flag.

Information

• test/services.test.ts:139-162 - Software engineering: the added tests cover explicit true, explicit false, omission, and non-compute validation for the false case. git diff --check main...HEAD passed; I did not run npm run typecheck or npm test because this checkout has no node_modules and the review is read-only.
• src/index.ts:138-139, src/commands/services.ts:98-101, src/commands/services.ts:115-116 - Functionality: the implementation matches the PR intent by registering paired flags, preserving explicit false in the POST body, omitting the field when absent, and rejecting both forms for non-compute services before network/config access.
• src/commands/services.ts:93-102, src/commands/compute.ts:598-602 - Security: no security-relevant changes found. The new value is a Commander boolean serialized through the existing authenticated HTTP client path; there is no new SQL/shell sink, secret/PII logging, dependency, or auth/authorization weakening.
• src/commands/services.ts:93-102, src/index.ts:138-140, src/commands/compute.ts:601-602 - Performance: no performance-relevant changes found. The change adds only constant-time option checks/body construction and help/output text updates; no N+1, unbounded loop, large allocation, or blocking hot-path I/O is introduced.

Verdict
approved - No Critical findings. Per the team rule, this should be posted as a comment rather than a GitHub green-check approval; human approval is still required.

[jwfing]

Summary

Both round-1 Criticals are fixed and negative-control-proven, the tri-state alwaysOn is wire-compatible with the platform both before and after #385's flip, and the gates are green at ee55aad — no blocking issues; two non-blocking items, one about the untested Commander layer and one about help copy that is ahead of the live backends.

Requirements context

This repo has no /docs/superpowers/ (no docs/ at all) — no matching spec/plan exists, so I assessed against the PR title/body plus the repo's own requirement documents: AGENTS.md:11-24 (non-negotiables 3 and 4) and .claude/skills/developing-insta-cli/SKILL.md:36-38. The behavioural contract I held the code to is the one the body states: --no-always-on → explicit alwaysOn: false, --always-on → true, neither → omit the key and inherit the platform default.

AGENTS.md:14-17 / SKILL.md:36-38 make the skills/insta/cli-reference.md mirror mandatory. Verified satisfied: insta-skills#77 is open at b29df45 and its diff carries the paired-flag surface (insta/cli-reference.md: [--always-on|--no-always-on] plus the new default semantics; insta/SKILL.md, references/deploy.md, references/operate.md). The flag names and semantics there match what this PR registers — round 1's first Critical is genuinely closed, not just claimed.

Findings

Critical

(none)

Suggestion

• Software engineering / test coverage — src/index.ts:138-139. The whole Commander layer is unbound by tests, and its correctness silently depends on the declaration order of the two .option() calls. Two negative controls at this head, both fully green (51 files / 780 tests):

  1. Deleting .option('--no-always-on', …) from src/index.ts:139 outright — the PR's headline capability vanishes from the CLI surface and the suite still passes.
  2. Swapping lines 138 and 139 so the negated form is registered first — also green, but the behaviour is then wrong. Probed against the pinned commander 12.1.0 in this workspace: positive-first yields undefined / true / false for (no flag) / --always-on / --no-always-on (i.e. exactly what the body claims), while negated-first yields true / true / false. The mechanism is in Command.addOption: a --no-x option installs a true default only when no positive --x is already registered, so the current ordering is load-bearing.

  Composed with the new presence check at src/commands/services.ts:116, that reordering would make every non-compute creation throw with no flag typed — insta services add postgres db, storage assets, redis cache, and the interactive no-type path at src/index.ts:141-144 — with the message "--always-on / --no-always-on is only valid for compute services". test/services.test.ts:156-158 already proves the throw for { alwaysOn: true }; nothing proves the un-flagged parse never produces it. One argv-level test through the registered program (or, minimally, a comment on line 139 marking the order as load-bearing) would close both NCs. Same item the sibling review at this head raised; the NCs above are the evidence for it.

• Functionality / release ordering — src/index.ts:138 and src/index.ts:240. The new copy states always-on as fact ("the default for new compute services"), but that default is not live on any backend yet. Probed just now:

  • insta-platform#385 is still open, mergeable_state: blocked — not merged (as is the console companion insta-frontend#394).
  • GET https://api.instacloud.com/openapi.json and GET https://api.staging.instacloud.com/openapi.json both still describe POST /projects/{projectId}/services → alwaysOn as "create as always-on … (default false = idle machines suspend and wake on request)", and Service.always_on as "true: machines never scale to zero; false (default)".

  So today, against prod and staging alike, insta services add compute api with no flag creates a scale-to-zero service while --help tells the reader it is born always-on — the one direction where a reader is misled into omitting --always-on for a workload that wanted warm machines. The flag mechanics are correct in both platform states (see Information), so this is copy, not a defect; but the CLI releases on its own tag-triggered workflow, uncoupled from the platform deploy (which per the platform skill is a manual gh workflow run deploy-prod.yml). Worth either hedging the wording or holding the version-bump PR until #385 is on prod. The same assertion rides in companion insta-skills#77 (insta/cli-reference.md, insta/SKILL.md, dated "since 2026-09-07"), and src/ensure-skills.ts:63,75 installs that reference into user projects from insta-skills main, so both halves of the agent-facing surface would be ahead of the backend together.

Information

• Gates at ee55aad (run in a clean clone): npm ci clean · npm run typecheck clean · npx vitest run → 51 files / 780 tests passed. (Noting because the two prior reviews on this PR could not run them.)
• Both round-1 Criticals verified by negative control, each red at exactly one assertion — the remedies are load-bearing and correctly scoped:
  • reverting src/commands/services.ts:101 to ...(opts.alwaysOn ? { alwaysOn: true } : {}) → reds only test/services.test.ts:139 ("sends alwaysOn in BOTH states…"), 1 failed / 779 passed;
  • reverting src/commands/services.ts:116 to if (opts.alwaysOn && type !== 'compute') → reds only test/services.test.ts:159 ("rejects --no-always-on for a non-compute type too"), 1 failed / 779 passed.
• Functionality — the change is wire-compatible in both directions, so it can safely land before #385. Both live specs type the create-body alwaysOn as a plain boolean, so an explicit false is a valid body on today's un-flipped platform (?? false → false, same result as omitting) and overrides the flipped one (?? true). There is no merge-order dependency in the code itself.
• Functionality — no second truthiness spread hides in this repo. servicesAdd (src/commands/services.ts:124) is the only POST /projects/{projectId}/services caller in src/, and a repo-wide grep for alwaysOn|always_on|always-on turns up no other create path. The template route is also clean: src/commands/template.ts:397 ships the parsed manifest verbatim and parseManifestYaml (src/template-manifest.ts:203-214) does not whitelist keys, so a template's alwaysOn: false survives the CLI intact.
• Functionality — the postgres copy is correctly left alone. src/index.ts:267 and src/commands/db.ts:9,27 still call scale-to-zero the postgres default; that matches the live spec's "compute only" scoping of Service.always_on and #385's compute-only birth default, so leaving them unchanged is right, not an omission. The new error string's pointer is also runnable as printed: db always-on <mode> accepts on|off (src/commands/db.ts:16).
• Software engineering — the surface renders. npx tsx src/index.ts services add --help lists both --always-on and --no-always-on; --no-open (src/index.ts:335,338) is the existing --no-* precedent, though it is declared without a positive twin, which is why it defaults to true and this new pair does not.
• Security — no security-relevant changes in this PR. A Commander boolean is forwarded through the existing authenticated api.rawRequest path; no new user input reaches SQL/shell/HTTP unvalidated, no secret/token/PII is newly logged or returned, no auth or authorization check is touched, and no dependency is added or moved (package.json unchanged).
• Performance — no performance-relevant changes in this PR. The diff is one constant-time presence check, one constant-time object spread, and help/output strings; no new query, loop, allocation, or blocking I/O.
• Style — src/index.ts:240. On compute always-on <mode>, "(the default for new compute services)" sits on the on arm of a command that only ever mutates an existing service, where it can read as if on were the command's default mode. Consider "new compute services are born on" as a trailing clause instead. Take or leave freely.
• Process — this PR is mergeable_state: blocked and neither this review nor the last one clears it. The round-1 CHANGES_REQUESTED (review 5136234914 at 5db8bc6a) is still the standing state for this account; a COMMENTED review does not supersede it, so the later comment at this head (5136271144) left blocked in place and so will this one. Clearing it needs an APPROVED review via the approve flow, not another comment.

Verdict

approved — zero Critical findings; the two Suggestions and the Information items are non-blocking. Informational only: the GitHub green check is still a separate human action via the approve flow, and it is what will lift the standing CHANGES_REQUESTED.

[jwfing]

LGTM - approved.

[jwfing]

Summary

Post-merge round: the tree is byte-identical to the head I gated green last round, both companion PRs have landed, and the only delta since round 2 is the body's round-2 record — whose two declination claims I checked and both hold, so nothing here is blocking.

Requirements context

No /docs/superpowers/ and no docs/ directory in this repo — no matching spec/plan found, so this is assessed against the PR description, AGENTS.md (4 non-negotiables) and .claude/skills/developing-insta-cli/SKILL.md. AGENTS.md:14-17 (flag changes mirrored in skills/insta/cli-reference.md, which lives in the insta-skills repo) is satisfied by insta-skills#77, merged 2026-09-08T01:16:54Z.

State at this round

• PR merged 2026-09-08T01:10:44Z as 4849c4a; head never moved off ee55aad across all three rounds.
• tree(ee55aad) == tree(origin/main) == 9eb7adf — main was still at cbc7749 at merge time, so the merge is tree-identical to the reviewed head. My round-2 gate run therefore covers merged main unchanged: npm ci clean, npm run typecheck clean, npx vitest run 51 files / 780 passed. Nothing else landed on main in the window (git log cbc7749..origin/main = the merge + the 2 PR commits).
• Companion insta-platform#385 merged 01:14:07Z — i.e. after this PR. No merge-order bug results: the create-body alwaysOn is a plain boolean in the live spec, so an explicit false was valid on the un-flipped platform too.
• The standing CHANGES_REQUESTED from round 1 (5136234914 @ 5db8bc6) was superseded by the APPROVED at 01:10:11Z from the same account — a COMMENTED review never clears a review state, only APPROVED does. AGENTS.md:9 (1 approving review, not self-cast) is satisfied: author tonychang04, approver a different account.

---

Findings

Critical

(none)

Suggestion

(none — both of my round-2 Suggestions are re-graded down to Information below, one because the body's rebuttal is correct and one because the deploy window closed.)

Information

1. Software engineering — the declined parse test: the rebuttal is right, and stronger than stated. src/index.ts:138-139

Round 2's record declines a Commander-level parse test on the grounds that "the Commander layer is unbound in this repo for every option." I checked this rather than taking it at face value, and it holds — for a sharper reason than the body gives:

• No test imports src/index.ts. The only hit for that string in test/ is a path literal (test/scanner.test.ts:38, isSecretFile('src/index.ts')).
• It cannot be imported: src/index.ts runs selfUpdate.maybeUpdate(...) and program.parseAsync(computeArgv) at module top level, so importing it executes the CLI.
• test/telemetry.test.ts:33-34,244 does build Commander objects (new Command('insta').command('login').option('--env <name>')), but it re-declares the options inside the test. A test in that shape could never bind index.ts's actual registration — which means it would not catch the failure mode I raised last round. So the test I asked for is not merely absent by habit; the repo's structure can't make it load-bearing today.

The remedy that would work is extracting the registration into an exported function the test can call, which is correctly its own change and out of scope here. Recording this so the next reviewer doesn't re-raise it as a cheap ask.

2. Software engineering — the load-bearing thing is the line order, and nothing says so. src/index.ts:138-139

services.ts:98-100 carries a good comment explaining the payload semantics (presence, not truthiness). Nothing marks the other half: Commander 12.1.0 installs the true default for --no-x only when no positive --x is already registered, so the tri-state depends on --always-on being declared before --no-always-on. Probed against the pinned Commander in the clone last round:

declaration order	(no flag)	--always-on	--no-always-on
--always-on then --no-always-on (as merged)	undefined	true	false
lines 138/139 swapped	true	true	false

Composed with the presence gate at services.ts:116, the swapped order makes every non-compute creation throw with no flag typed — services add postgres db, storage assets, and the interactive no-type path. Swapping the two lines leaves all 780 tests green, so nothing but the line order protects this. Given finding 1, a one-line comment at the option pair is the realistic guard rather than a test. Non-blocking and post-merge; noting it for whoever next edits that option block. (For contrast, --no-open at index.ts:335,338 is the repo's only other --no-* and has no positive twin — which is exactly why it defaults to true and this pair does not.)

3. Functionality — help copy is currently ahead of every deployed backend; it converges on its own. src/index.ts:138

The help text asserts always-on as fact ("the default for new compute services"). Re-probed at this round: both https://api.instacloud.com/openapi.json and https://api.staging.instacloud.com/openapi.json still describe Service.always_on as "false (default)", so #385's flip is merged but not yet deployed to either environment. This was a Suggestion last round; it drops to Information because the window is now self-closing on both ends — #385 is in main, and this change is not in a published release either (package.json is still 0.0.61 and npm dist-tags.latest is 0.0.61, i.e. #184's bump). The copy ships to users at the next version bump. Worth a glance at deploy state when that bump is cut; no action on this PR.

4. Security — no security-relevant changes. The flag is a boolean forwarded on an already-authenticated POST /projects/{id}/services; no new user input reaches SQL, a shell or a URL, no new dependency, nothing new logged or returned. The compute-only check at services.ts:116 was strengthened (truthiness → presence), which closed round 1's real hole rather than opening one.

5. Performance — no performance-relevant changes. A presence test and one extra object-spread key on an existing request path; no new I/O, no loop, no query, no allocation of note. servicesAdd (services.ts:124) remains the only POST /projects/{id}/services caller in src/, so there is no second create path to keep in sync.

---

Verdict: approved — zero Critical findings. Merged, tree-identical to the head I gated green (780 passed), both companions landed, and the round-2 declinations check out. This is informational only; the GitHub approval was already cast separately by a human.