feat(env): first-class staging environment + agents.staging.instacloud.com one-liner - #66
Conversation
Adds a staging sibling to the `curl -fsSL agents.instacloud.com | sh`
onboarding flow, and makes the environment a real concept rather than two
independent env vars.
curl -fsSL agents.staging.instacloud.com | sh
Why this is more than an alias:
- `api` and `mcp` hosts are now resolved TOGETHER from one switch (src/env.ts).
Previously INSTA_API_URL and INSTA_MCP_URL were independent, so pointing the
CLI at staging and forgetting the second var left the machine's agents
registered against the PRODUCTION MCP server — silent, and reported as
success.
- Staging registers as `insta-cloud-staging`, not `insta-cloud`. registerMcp
treats an already-registered name as "nothing to do", and the config-file
agents key their entry by name, so a shared name meant a staging install
silently kept prod's registration. Distinct names also let both environments
coexist on one machine.
- The installer persists the choice via `insta env use` instead of exporting
INSTA_ENV. A piped `curl … | sh` cannot export into the parent shell, so an
env var would evaporate before the user's next `insta project create`.
- Switching environments drops the stored session. prod and staging are
separate deployments (us-east-2 vs us-west-1, different databases), so the
old token cannot authenticate — and keeping it is unsafe, because api.ts's
401 path would POST one deployment's refresh token to the other. Same
reasoning as the retired-host path in config.ts.
- An unrecognised INSTA_ENV is a hard error, not a fallback to prod: a typo'd
`INSTA_ENV=stagng` that silently provisions production infrastructure would
stay invisible until the bill arrived.
Resolution order is unchanged at the top — INSTA_API_URL still wins outright,
so insta-oss and preview deployments behave exactly as before.
New surface: `insta env [--json]`, `insta env use <prod|staging>`,
`insta login --env <name>`, `install.sh --staging|--env <name>`, and `insta
status` now names the environment alongside the URL.
26 new tests; full suite 140 passing, typecheck clean.
Staging MCP: the URL in this PR is correct, but the host it points at needs migrating firstVerified against the live environments. 1. Staging MCP currently breaks OAuth discovery. It advertises itself as Prod correctly reports Until this is fixed, use 2. The cause is that staging MCP is still the legacy deployment. On that box The durable fix is to move staging onto Manufact like prod, which keeps this PR's URL exactly as written:
For reference, Manufact's default URL scheme is None of this blocks merging: the env plumbing here is correct either way, and |
Update: staging MCP now exists on Manufact — and a branch trap worth knowingCreated a dedicated staging server on Manufact (mcp-use Cloud), mirroring how prod is hosted.
Status
|
Completes `curl -fsSL agents.staging.instacloud.com | sh` so every piece it installs is staging, not just the API endpoint: CLI newest PRERELEASE build (v*-rc.N), falling back to latest stable API api.staging.instacloud.com MCP mcp.staging.instacloud.com, registered as `insta-cloud-staging` skills InsForge/insta-skills@devel Skill source joins the environment table, so `resolveEnv()` now returns api, mcp AND skills from one switch. Resolving them independently is silently wrong: a machine whose CLI talks to staging while its agents read prod's skill text describing a control plane it isn't using. `npx skills add` has no --ref/--branch flag but does accept `owner/repo@ref` (verified: it resolves to `…insta-skills.git @devel`), so the ref rides along in the spec string. `INSTA_SKILLS_REPO` overrides it, matching how INSTA_MCP_URL already works. Both skill-install paths follow the environment — `setup agent` (machine-global, the installer's path) and `ensure-skills` (per-project, on project create/link). ensure-skills falls back to prod's targets if the resolve throws, since it is best-effort and must never block the host command. The third-party stack skills (neon/tigris/better-auth) are untouched — they document those services, not our control plane. Release-channel plumbing, and two latent hazards fixed while adding it: - release.yml created every tag as a normal release, so a `v0.0.23-rc.1` tag would have been published as **"Latest"** and handed to every production installer by install.sh's /releases/latest lookup. Hyphenated (semver prerelease) tags now get --prerelease. - publish-npm ran a bare `npm publish`, which would have given a prerelease the `latest` dist-tag — so `npm i -g insta` would install a staging build. Those now publish under `next`. install.sh --staging resolves the newest non-draft prerelease via the releases list (/releases/latest deliberately excludes prereleases) and falls back to stable with a printed note when none exists — which is the case today, since no prerelease has ever been published. INSTA_VERSION still wins outright. Verified: typecheck clean, 146 tests passing (20 files, 6 new); prod's `setup agent` command sequence is byte-identical to before; staging's resolves to `npx skills add InsForge/insta-skills@devel …` + `claude mcp get insta-cloud-staging`; prerelease parser checked against fixtures (picks newest non-draft prerelease, skips drafts, empty when none) and the live API; installer arg matrix and channel fallback exercised directly; sh -n clean on all three scripts; release.yml still parses.
The staging one-liner now installs a fully staging stackPushed.
Skill source joined the environment table, so
Both skill-install paths follow the environment: Two latent release hazards fixed while adding the channelNeither is caused by this PR; both would have bitten the first time anyone cut a prerelease.
Note on the CLI channel todayNo prerelease has ever been published for this repo — every release is a plain version with Verification
Still outstanding (unchanged): attach |
…lied
Found by actually running `sh install.sh --staging` against the released
binary. v0.0.22 has no `insta env` command, so `insta env use staging` failed —
and the installer then printed
Environment: staging (api.staging.instacloud.com) — persisted
anyway, because the banner was gated on $ENV_NAME rather than on whether the
step succeeded. The install was still pointed at PRODUCTION while telling the
user it was on staging.
That is the exact failure this whole PR exists to prevent, and it is worst in
the canonical usage — `curl … | sh && insta project create`, frequently run
unattended by an agent — which would have provisioned real production
infrastructure believing it was staging.
Now: a requested-but-unappliable environment is a hard error (exit 1) naming
the version requirement, and it exits BEFORE `setup agent`, so the machine's
agents are never wired to the wrong environment either. The banner additionally
checks ENV_APPLIED, so it can't outlive the step it describes.
This is expected to fire for `--staging` until a release containing `insta env`
ships — failing is the correct behaviour there, and it self-resolves on release.
Verified with the real installer and a bun-compiled branch binary:
- released 0.0.22 + --staging -> error, exit 1, no false banner
- branch build + --staging -> "switched prod → staging", banner printed,
~/.insta/config.json apiUrl = staging
- end to end: env use staging -> `insta env --json` reports staging api/mcp/
skills -> `insta mcp install --agent cursor` writes insta-cloud-staging ->
https://mcp.staging.instacloud.com/mcp
|
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
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Review — feat(env): first-class staging environment
Summary: A carefully designed staging environment that resolves API/MCP/skills from one switch; the design and test coverage are strong, but the staging skills pin uses a ref syntax the skills CLI does not interpret as a git ref, so staging installs silently read the wrong (production/main) skill text.
Requirements context
This repo has no spec/plan directory — there is no docs/superpowers/, and no docs/ at all (only AGENTS.md / CLAUDE.md describing conventions: Node 20 + TS ESM, commander, PR-only to main, npm run typecheck && npm test gate, and AGENTS.md rule 4 = mirror command/flag changes into skills/insta/cli-reference.md). No matching spec/plan found — assessing against the PR description and AGENTS.md alone.
Critical
Functionality — the staging skills ref is not pinned; @devel is parsed as a skill-name filter, not a git ref — src/env.ts:31-35 (claim in the comment src/env.ts:18-22); test enshrines it at test/env-switch.test.ts:39,45-50.
ENVS.staging.skills = 'InsForge/insta-skills@devel' is passed verbatim as the source argument to npx skills add <spec> -s insta … (src/setup.ts:102-103, src/ensure-skills.ts:43-44). The comment asserts "The @ref form is supported by the skills tool (it resolves to …insta-skills.git @<ref>)." That is not how vercel-labs/skills parses it. I verified against the tool's own source-parser.ts (via context7):
- A git ref is taken from a
#-fragment:owner/repo#branch→ref = branch(parseFragmentRef). - The
@after the repo is a skill-name filter, not a ref: the documented form isnpx skills add <owner/repo@skill>.
So InsForge/insta-skills@devel resolves to source InsForge/insta-skills, ref undefined (⇒ HEAD/main), skill-filter devel — it never checks out the devel branch. A staging install therefore reads main's skill text, not the staging control-plane skill text. This is exactly the "silent and bad" cross-env drift the PR is built to prevent (the machine talks to staging while its agent docs describe prod), and it defeats the third leg of the unified-resolution guarantee. The correct syntax is the fragment form:
staging: { …, skills: 'InsForge/insta-skills#devel' }
Please also update the test at test/env-switch.test.ts:45-50 (and the format regex at line 39, which currently only accepts the @ref shape) so it asserts the #ref form — as written the test locks in the broken syntax and gives false confidence.
Suggestion
Security — the INSTA_ENV targeting path doesn't drop a mismatched session, so a 401 can POST the wrong deployment's refresh token — src/config.ts:36-44, src/api.ts:53,73-84, cf. the guard in src/commands/env.ts:41-43.
env use drops the stored session on a switch precisely because api.ts's 401 path POSTs refreshToken to whatever apiUrl now resolves to (api.ts:75). But the INSTA_ENV=staging env-var path only overrides apiUrl at read time and leaves the persisted prod session intact. A user with a logged-in prod session running INSTA_ENV=staging insta … will, on a 401, hand prod's refresh token to the staging backend — the exact cross-deployment credential leak the PR's rationale (src/env.ts:6-8) says must never happen. Blast radius is limited (first-party staging infra, and the token is rejected there), and this is inherited from the pre-existing INSTA_API_URL override behavior, so it's non-blocking — but it's worth closing given the PR makes this invariant explicit. One option: in api.ts's refresh path, skip the refresh POST when the persisted session's environment doesn't match the resolved apiUrl.
Software engineering — no automated test for the MCP-name coexistence guarantee — src/commands/mcp.ts:34,56,64, src/commands/setup.ts:120-123.
Point #2 of the PR ("both environments coexist on one machine") rests on the new name parameter threading through renderJsonConfig / renderCodexConfig / installFor, but the new test file exercises only the env.ts helpers. There's no test that installFor(..., 'insta-cloud-staging') adds a second entry alongside an existing insta-cloud (JSON) / [mcp_servers.insta-cloud] (TOML) without clobbering it. This was verified manually per the PR body; a unit test on renderJsonConfig/renderCodexConfig with a non-default name would guard it against regression cheaply.
Functionality — env use doesn't re-register the MCP server — src/commands/env.ts:24-50.
env use switches the API host and drops the session, but does not re-run MCP registration. The installer path is fine (install.sh:199-213 runs env use before setup agent). But a user who manually runs insta env use staging on a machine already set up for prod is left with agents still pointed at prod's MCP (insta-cloud) until they re-run insta mcp install / insta setup agent. Consider re-registering, or adding a hint to the env use output (it already prints the target MCP name — a "run insta setup agent to re-point your agents" line would close the loop).
Information
- Custom-host fallback to prod MCP/skills —
src/config.ts:60-67. For a customapiUrl(insta-oss / localhost / preview) with noINSTA_MCP_URL/INSTA_SKILLS_REPOset,resolveEnvfalls back to prod's MCP host and skills. Documented and preserves today's behavior; flagging only so it's a conscious choice (a self-hoster gets prod's MCP URL unless they setINSTA_MCP_URL). env useunder an activeINSTA_API_URL/INSTA_ENVoverride —src/commands/env.ts:29-35.ApiClient.load()reflects the override, so the "already on X" comparison and session-drop decision run against the overridden value, and the persisted switch is then shadowed by the still-set env var on the next read. Edge case (user explicitly exported an override), non-harmful.resolve_prereleasescans only 30 releases —install.sh:67-72. If none of the 30 most-recent releases is a prerelease it falls back to stable. Fine given the documented "DNS for agents.staging not live yet" status and theINSTA_VERSIONescape hatch.- CLI-reference mirror — per AGENTS.md rule 4, the new
insta env/login --env/statussurface must be mirrored intoskills/insta/cli-reference.md. The PR explicitly defers this to a separate PR in the skills submodule; noting for tracking (out of scope for this repo). - Release-channel plumbing looks correct —
.github/workflows/release.yml: hyphenated tags get--prereleaseon the GitHub release and--tag nexton npm publish, keeping prereleases off/releases/latestand the npmlatestdist-tag. Thecase … in *-*)semver-prerelease heuristic is standard and theshellcheck disablecomments are apt.
Security: aside from the Suggestion above, no secrets are newly logged or returned; --mcp-token handling is unchanged; the OAuth loopback still validates state; no new dependencies. Performance: no hot-path, N+1, or blocking-I/O concerns — the only added network work (resolve_prerelease, up-to-date check) is one-time at install.
Verdict
request_changes — one Critical: the staging skills pin (@devel) must use the #devel ref form (with the matching test update), or staging installs will silently ship production skill text. The Suggestions and Information items are non-blocking.
jwfing
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Review — feat(env): first-class staging environment
Summary: A clean, well-reasoned unified-environment switch (one source of truth for api/mcp/skills, distinct MCP registration names, thorough tests) — but the INSTA_ENV env-var path bypasses the session-drop that the PR's own security model treats as mandatory, leaking prod credentials to staging.
Requirements context: No /docs/superpowers/ (or docs/specs/) directory exists in this repo, and there is no matching spec/plan — assessing against the PR description, the repo AGENTS.md, and the developing-insta-cli skill alone. Gates are npm run typecheck && npm test.
Critical
[security / functionality] The INSTA_ENV switch changes the API host but does not drop the stored session — prod tokens get sent to staging. src/config.ts:36-44, src/env.ts:6-8, src/api.ts:53-56,73-84
The PR is explicitly built on this invariant (src/env.ts:6-8):
"a session minted by one environment can NEVER authenticate against the other, and its refresh token must never be POSTed there. So every environment switch drops the stored session."
envUse (src/commands/env.ts:41-44) and login --env honor this. But the INSTA_ENV env-var path — resolution-order item #2, a first-class documented switch — does not. readGlobal returns { ...parsed, apiUrl: <staging> }, i.e. the staging host with the persisted prod accessToken/refreshToken still attached (src/config.ts:38-44). There is no session-drop anywhere on this path.
Concrete failure scenario:
- Developer/CI logs into prod →
~/.insta/config.jsonhasapiUrl=api.instacloud.com+ prod tokens. - They export
INSTA_ENV=staging(the documented way to target staging) and run any authenticated command, e.g.insta project create. ApiClientnow holdsapiUrl=api.staging.instacloud.com+ prod tokens. The first request sendsAuthorization: Bearer <prodAccessToken>to staging (src/api.ts:61); staging returns 401; the 401 path then POSTs{ refreshToken: <prodRefreshToken> }toapi.staging.instacloud.com/auth/refresh(src/api.ts:53-56,73-84).
That is precisely the cross-deployment refresh-token transmission the PR says "must never" happen and that envUse was written to prevent — just reachable through the env-var switch instead of the command. (Blast radius is bounded: both hosts are first-party over TLS and staging rejects the token — but the transmission itself is the harm the PR declares unacceptable, and the invariant "every environment switch drops the stored session" is simply false for this path.)
Suggested fix: in readGlobal/resolveEnv, when the resolved env differs from the env of the persisted apiUrl and a session is present, drop (or refuse to attach) the persisted tokens — mirroring envUse's clearSession(). Alternatively gate api.ts's refresh on the token's origin host. Note the same shape applies to INSTA_API_URL pointing at a sibling env, but the named INSTA_ENV switch is the one this PR advertises as drift-proof.
Suggestion
- [software engineering / tests]
test/env-switch.test.tscovers the session-drop forenv use(lines 255-265) very well, but there is no test for theINSTA_ENV-with-persisted-session case above — a regression test asserting that resolvingINSTA_ENV=stagingover a persisted prod session does not carry the prod tokens would have caught the Critical and should land with its fix. - [functionality]
install.sh:67-72resolve_prereleaseparses the releases JSON withawkassuming field order (tag_namebeforedraft/prerelease) and a 30-release window. It works against GitHub's stable ordering and degrades safely (falls back to stable), so this is fine as-is — just flagging the ordering assumption as fragile if the response shape ever changes.
Information
- [software engineering]
.github/workflows/release.yml:107-108— thepublish-npmjob comment says it publishes "with a GRANULAR AUTOMATION token (repo secret NPM_TOKEN)", but the actual step (lines 123-136) and the repo skill both use OIDC trusted publishing with no token secret. The two descriptions contradict each other; drop the NPM_TOKEN comment to avoid implying a secret that doesn't exist. - [software engineering]
src/env.ts:40references "the retired-host path inconfig.ts", but no such path exists inconfig.tson this branch (it lives on the siblingfix/retired-api-url-migration). Reads as a stale/forward reference until that lands. - [project convention]
AGENTS.mdrule 4 and thedeveloping-insta-cliskill state command/flag changes "must be mirrored inskills/insta/cli-reference.mdin the same change set". The PR addsinsta env,env use, andlogin --envbut defers the mirror to a separate submodule PR. Acknowledged in the description; noting the convention deviation.
Dimensions with no findings
- Security (beyond the Critical): checksum verification (
install.sh:155-168) is intact; no secrets or PII are newly logged or returned; no auth check is weakened; no new npm dependencies. - Performance: no hot-path regressions — startup auto-update uses a tiny sync read (
upgrade.ts:128-131, unaffected by the new resolution), and env resolution is O(1) over a two-entry table.
Verdict: request_changes
One Critical finding (cross-deployment credential transmission via the INSTA_ENV switch, contradicting the PR's own session-drop invariant). The overall design — single resolution switch, distinct MCP names, hard-fail on bad env, the installer's persist-not-export approach, and the 26 new tests — is solid; closing the env-var session-drop gap (plus a regression test) should be enough to clear it.
jwfing
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Code review — feat(env): first-class staging environment
Summary: A well-factored, safety-first addition of a prod|staging environment switch that resolves API + MCP + skills from one source, persists the choice across the install pipe, and hard-fails rather than silently defaulting to prod — I found no blocking issues.
Requirements context
No /docs/superpowers/ (or docs/specs/) directory exists in this repo, and no spec/plan document matches this PR — assessed against the PR description and the repo conventions in AGENTS.md / .claude/skills/developing-insta-cli/SKILL.md alone. The three stated goals (unified resolution so api/mcp can't drift, distinct MCP registration name so prod+staging coexist, persistence surviving curl | sh) map cleanly onto the implementation.
Verification performed on the head commit (b30429d): npx tsc --noEmit clean; npx vitest run → 146 passing (20 files); sh -n clean on install.sh, agents.sh, agents-staging.sh. The PR's claims check out.
Critical
(none)
Suggestion
1. Test coverage gap for the headline "coexistence" behavior (software engineering).
Claim #2 of the PR — a staging install registers as insta-cloud-staging so it can coexist with a prod registration — is only unit-tested at the mcpServerName() level (test/env-switch.test.ts:53-64). The actual config-file write with a non-default name is not exercised: test/mcp-install.test.ts:12-70 calls installFor(..., DEFAULT_MCP_URL) with the default (prod) name only, and the name parameter threaded through mcp.ts:34-87 runs its non-default branch solely in the manual E2E. A test that installs prod, then staging, and asserts both insta-cloud and insta-cloud-staging entries survive in .cursor/mcp.json / ~/.codex/config.toml would lock in the guarantee the PR is built around.
2. install.sh gained 81 lines of untested logic (software engineering / functionality).
The env-parsing, hard-fail, and prerelease-channel resolution are verified manually only. The repo has no shell-test harness, so this stays a suggestion — but resolve_prerelease (install.sh:67-72) is the fragile part: its awk relies on GitHub emitting tag_name before draft/prerelease within each release object. That ordering is undocumented API behavior and would regress silently (staging would quietly fall back to stable). Worth at minimum a comment pinning the assumption (there is one) and ideally a smoke check against a real releases payload.
3. env use message can misreport when an env-var override is active (functionality / UX).
envUse (src/commands/env.ts:29-49) loads the resolved apiUrl via ApiClient.load() → readGlobal(), which already applies INSTA_API_URL/INSTA_ENV. It then persists the target and prints switched … → staging. But if INSTA_API_URL (or INSTA_ENV) is exported in the shell, that override still wins on the next read, so the persisted value stays masked and the CLI does not point where the success line claims. Low blast radius (not reachable from the installer, only a user with an override manually running env use), but a one-line "note: INSTA_API_URL is set and overrides this" guard would prevent a confusing false-success.
Information
skills/insta/cli-reference.mdmirror is deferred.AGENTS.mdrule 4 / SKILL.md require command/flag changes be mirrored in theskills/submodule, and the new surface (insta env,env use,login --env, thestatusenv line) isn't. The PR body explicitly defers this to a separate submodule PR — noted only so it lands before the feature is announced, since that doc is how agents learn the CLI surface. Since it's a separate repo it genuinely can't be in this diff.- Recovering from a bad
INSTA_ENVneeds the var unset first. BecausereadGlobal/resolveEnvthrow on an invalidINSTA_ENV(src/env.ts:60-68), eveninsta env use prodthrows (it callsApiClient.load→readGlobal). The fix is obvious (unset the var) and the fail-loud-on-typo behavior is the intended safety design — just flagging the small sharp edge. - Security: no security-relevant regressions. New host/name values written to agent configs come from the fixed
ENVStable, not user input;install.shvalidates--envagainst aprod|stagingallowlist (install.sh:46-49); the OAuth loopback keeps itsstatecheck; the session-drop-on-switch (env.ts/commands/env.ts:41-43) actively closes a real hole —api.ts's 401 refresh path would otherwise POST one deployment's refresh token to the other. No secrets are logged. - Performance: no relevant changes — CLI invocations, no hot paths, no new per-request work.
- Release safety (nicely done):
release.ymlnow marks hyphenated tags--prereleaseand publishes npm prereleases undernext, keeping an-rcbuild out of both/releases/latestandnpm i -g insta@latest. That's a genuine production footgun closed, and it's the correct counterpart toinstall.sh --stagingtracking the prerelease channel.
Verdict: approved (informational)
No Critical findings, so this posts as a COMMENT, not REQUEST_CHANGES. The suggestions above are non-blocking. Explicit GitHub approval remains a separate human action via the approval flow.
Both Criticals from John-bot's review on #66. 1. The staging skills pin silently did nothing. `ENVS.staging.skills` was `InsForge/insta-skills@devel`, but the skills tool parses `@` as a SKILL-NAME FILTER, not a git ref — the ref form is the `#fragment`. So staging installs read the DEFAULT branch: exactly the cross-environment drift this PR exists to prevent (machine on staging, agent docs describing prod). My original "verification" was worthless: the tool prints `Source: …insta-skills.git @devel` for the @ form too, so it reads as if the ref applied. Settled it properly by installing both forms and diffing the result — `#docs/staging-env` produced that branch's cli-reference.md, `@docs/staging-env` produced main's. Now `InsForge/insta-skills#devel`, and the tests assert the `#` form and that `@` never appears (the previous tests locked in the broken syntax and gave false confidence). 2. The INSTA_ENV / INSTA_API_URL override path leaked credentials across deployments. `env use` drops the stored session on a switch precisely because api.ts's 401 path POSTs the refresh token to whatever apiUrl now resolves to — but the env-var path only overrode apiUrl and left the persisted session attached. `INSTA_ENV=staging insta …` on a prod-logged-in machine sent prod's bearer to staging and then POSTed prod's REFRESH token to staging's /auth/refresh. readGlobal now scrubs the session when the override names a different deployment. In-memory only, so unsetting the override restores the real login (a stray INSTA_ENV must not log you out permanently); an override naming the SAME deployment keeps the session, trailing slash included. Also from the review, both non-blocking: - `env use` now points at `insta setup agent`, since switching the CLI does not re-point already-installed agents (their registration is keyed by the other environment's server name). - Added the missing MCP coexistence tests: a staging install adds a SECOND entry beside prod for cursor/codex/opencode without clobbering it, is idempotent per name, and — since `insta-cloud` is a strict prefix of `insta-cloud-staging` — that prefix does not confuse TOML detection. Verified: typecheck clean, 157 tests passing (20 files, +11). With a bun-compiled binary: `insta env --json` reports skills `InsForge/insta-skills#devel`; a planted prod session under INSTA_ENV=staging yields user=null, i.e. the token is not carried across.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found and verified against the latest diff
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="README.md">
<violation number="1" location="README.md:23">
P2: The one-liner `curl -fsSL agents.staging.instacloud.com | sh` is documented as a working staging command, but the PR description notes that DNS/CloudFront for this hostname is not yet provisioned. Users who try this command will get a DNS resolution error. Either add a note that this URL is coming soon (and link to the alternative `install.sh --staging` approach), or keep the documentation change scoped to the same PR that provisions the DNS.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| ```bash | ||
| curl -fsSL agents.instacloud.com | sh # production | ||
| curl -fsSL agents.staging.instacloud.com | sh # staging |
There was a problem hiding this comment.
P2: The one-liner curl -fsSL agents.staging.instacloud.com | sh is documented as a working staging command, but the PR description notes that DNS/CloudFront for this hostname is not yet provisioned. Users who try this command will get a DNS resolution error. Either add a note that this URL is coming soon (and link to the alternative install.sh --staging approach), or keep the documentation change scoped to the same PR that provisions the DNS.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 23:
<comment>The one-liner `curl -fsSL agents.staging.instacloud.com | sh` is documented as a working staging command, but the PR description notes that DNS/CloudFront for this hostname is not yet provisioned. Users who try this command will get a DNS resolution error. Either add a note that this URL is coming soon (and link to the alternative `install.sh --staging` approach), or keep the documentation change scoped to the same PR that provisions the DNS.</comment>
<file context>
@@ -16,6 +16,16 @@ curl -fsSL https://raw.githubusercontent.com/InsForge/insta-cli/main/install.sh
+
+```bash
+curl -fsSL agents.instacloud.com | sh # production
+curl -fsSL agents.staging.instacloud.com | sh # staging
+```
+
</file context>
jwfing
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Review — feat(env): first-class staging environment
Summary: A well-scoped, well-tested addition of a first-class staging environment that resolves API + MCP + skills from one switch and drops the deployment-specific session on any change; I found no blocking issues, one narrow functional gap in insta env use, and a doc bug that reintroduces the very @-vs-# footgun the code guards against.
Requirements context
This repo has no /docs/superpowers/ (or docs/specs/) directory — no matching spec/plan exists, so I assessed against the PR description, the AGENTS.md/developing-insta-cli conventions, and the linked follow-ups. The PR body is unusually explicit about intent (the three "why this isn't just an alias" invariants, the resolution order, and the session-drop reasoning), and the implementation matches it closely.
Critical
(none)
Suggestion
Functionality — src/commands/env.ts:32-35 — env use skips the persist when an override already resolves the target host.
envUse loads ApiClient.load() → readGlobal(), which applies the INSTA_ENV / INSTA_API_URL override. It then early-returns (already on <env>, no persist()) when api.apiUrl === nextApi. If INSTA_ENV=staging is present in the environment at the moment insta env use staging runs, readGlobal() (with no config file yet) already returns apiUrl = staging, so the switch is treated as a no-op and nothing is written to ~/.insta/config.json. A later command run without that env var then falls back to prod (§ resolution order step 4) — the exact silent-prod outcome the PR sets out to prevent — and install.sh:249-251 still prints "Environment: staging … persisted". The canonical path (agents-staging.sh → install.sh --staging, which does not export INSTA_ENV) is unaffected, so this is narrow, but install.sh documents INSTA_ENV as a supported input and the cubic summary steers CI toward it. Consider deciding the no-op against the persisted value (parsed.apiUrl), not the override-resolved one, so env use always writes the file.
Documentation — README.md:76 — the Environments table lists InsForge/insta-skills@devel, using the @ form the code explicitly forbids.
src/env.ts:22-27 documents (and the actual value at src/env.ts:39 uses) #devel, because the skills tool parses owner/repo@thing as a skill-name filter and silently leaves the source on the default branch. test/env-switch.test.ts:51-60 even adds assertions "never uses @ in a skill source … to stop that footgun coming back." A user copying @devel from the README would hit precisely that footgun. Change the table to InsForge/insta-skills#devel.
Information
- Software engineering / conventions — stale
@refin code comments.src/commands/setup.ts:100-101(owner/repo@ref) andsrc/ensure-skills.ts:40,42(owner/repo[@ref]) describe the spec format with the@form thatenv.tswarns against. Harmless at runtime (theENVSvalues use#), but the comments contradict the module they document — worth aligning to#refso a future edit doesn't follow the comment. - Conventions —
skills/insta/cli-reference.mdmirror is deferred.AGENTS.mdrule 4 / the repo skill require command+flag changes (insta env,insta env use,login --env) to be mirrored in the superprojectskills/submodule in the same change set. The PR body acknowledges this as a separate PR; noting it so the follow-up isn't lost, since it's the agent-facing surface doc. - Test coverage — the
env usegap above is the one behavior not covered.test/env-switch.test.tsis otherwise excellent (matched host table,@-guard, prefix-collision in Codex TOML, in-memory-only scrub, same-deployment keeps session, trailing-slash equivalence). A test exercisingenv use <target>whileINSTA_ENV=<target>is set — asserting the file is actually written — would have caught it.
Dimensions covered
- Software engineering: New behavior is thoroughly tested (26 new cases in
env-switch.test.ts+ 5 coexistence cases inmcp-install.test.ts); import/error-handling/DI-runner conventions all match the surrounding code. Could not execute the suite locally (the shallow clone has nonode_modules); relying on the PR's stated "140 passing" and code reading. One coverage gap noted above. - Functionality: Implements the three stated invariants (unified api+mcp resolution, distinct MCP registration name, persisted choice). The persist-skip edge in
env useis the one gap; the hard-error-on-bad-INSTA_ENV, session-drop, and prefix-safe Codex detection are all correct and tested. - Security: Net positive — the change closes a cross-deployment refresh-token leak (both
config.ts:50-56on the override path andcommands/env.ts:41-43on explicit switch drop the session; tested).install.shvalidates$ENV_NAMEagainst a whitelist before use; no secrets newly logged; no new dependencies. - Performance: No concerns — one config read per command; installer release-resolution is a bounded set of
curlcalls appropriate for an installer.
Verdict
approved (informational — no Critical findings). The two Suggestions are worth addressing before merge, especially the README.md @devel fix, but neither blocks. Human approval via the separate approve flow.
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…rcement Six findings from the automated review on #66, three of them P1. env use (src/commands/env.ts) had three real defects, all now covered by tests: - It decided against the OVERRIDE-resolved host, so `INSTA_ENV=staging insta env use staging` reported "already on staging", wrote nothing, and left the next process — without INSTA_ENV set — still on prod. It now reads the persisted config directly (new `readPersistedGlobal`), which is the only correct basis for "is this a real switch?". - It compared URLs raw, so a stored trailing slash looked like a switch and needlessly discarded a valid session. Now normalised, matching the trailing-slash equivalence envForApiUrl already documents. - `hadSession` checked accessToken alone, so a config holding only a refreshToken (interrupted login, hand-edited file) kept that token across a switch — and api.ts's 401 path would POST it to the new deployment. The session is now cleared unconditionally on a real change. release.yml: `--prerelease` was applied only on the `gh release create` path, so a release that already existed (created by hand, or a re-run of this job) stayed marked "Latest" — exactly the case install.sh's production /releases/latest channel would then serve. Now enforced with `gh release edit --prerelease`. install.sh: `--env=` (empty value) passed the later validation and silently fell through to the default instead of erroring like a bare `--env`; now rejected (exit 1, verified). Prerelease lookup raised to per_page=100 — with the default 30, thirty newer stable releases would push the newest prerelease onto page 2 and staging would quietly install stable. agents-staging.sh: piping `curl … | sh` means a failed download hands sh an EMPTY stream and sh exits 0 — the one-liner reported success having installed nothing, which matters most to automation reading only the exit code. Now it downloads to a file, checks curl's status and that the file is non-empty, then runs it (verified: exit 1 on a bad URL). README: the staging one-liner is documented with the 404-until-merged caveat and the raw-URL equivalent, matching what agents-staging.sh already says. Verified: typecheck clean, 161 tests passing (20 files, +4); sh -n clean on all three scripts; release.yml still parses; `--env=` exits 1 and `--env=staging` exits 0.
jwfing
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Review: first-class staging environment
Summary: A well-engineered, unusually well-tested addition of a staging environment with coherent api/mcp/skills resolution and a genuinely careful cross-deployment session-safety story; no blocking issues, a few documentation/parity nits.
Requirements context
No /docs/superpowers/ (or docs/specs/) directory exists in this repo — assessed against the PR description, AGENTS.md, and the repo-local .claude/skills/developing-insta-cli skill. Verified the build gates locally: npm run typecheck clean, npm test 161 passing (20 files), and sh -n clean on all three shell scripts.
Critical
(none)
The headline safety claim holds up under inspection. api.ts:51-84 retries a 401 by POSTing refreshToken to whatever this.apiUrl currently resolves to, so pointing INSTA_API_URL/INSTA_ENV at a different deployment while carrying the old session would indeed hand one deployment's refresh token to another. The in-memory scrub in config.ts:50-56 (and the persisted drop in commands/env.ts:41-53) closes that hole, and the tests exercise it precisely (mismatch scrubs, same-deployment keeps, trailing-slash keeps, refresh-token-only and user-only remnants dropped, scrub not persisted). The "bad INSTA_ENV exits 1, no stack trace" claim also holds — maybeUpdate uses its own sync read (upgrade.ts:129), not readGlobal, so startup doesn't throw an unguarded error, and command paths go through guard→die.
Suggestion
Software engineering / functionality
README.md:86documents the staging skill source asInsForge/insta-skills@devel— the@form. This directly contradicts the code (env.ts:39uses#devel), the invariant the tests enforce (test/env-switch.test.ts:56-60, "never uses @ in a skill source"), and the footgunenv.ts:22-27documents: the skills tool parses@thingas a skill-name filter, silently leaving the source on the default branch while its output still echoes the ref. A user who copies this intoINSTA_SKILLS_REPO=InsForge/insta-skills@develwould silently getmain's skill text — reintroducing exactly the silent failure this PR set out to fix. Should beInsForge/insta-skills#devel. Low blast radius (the CLI's own resolution uses theENVSconstant, not the README), but it's a user-facing doc that undermines a stated goal.
Software engineering (test coverage)
- The
install.shshell logic is not covered by any automated test — specificallyresolve_prerelease(install.sh:74-79), the--env/--staging/--env=argument matrix (install.sh:35-52), and the hard-fail-if-env-cannot-be-applied path (install.sh:206-220). The PR body says these were exercised manually, which is good, but nothing locks the behavior in against regressions.resolve_prereleasein particular leans on GitHub emittingtag_namebeforedraft/prereleasewithin each release object (correct today, but fragile and load-bearing for the staging channel). A small bats-style harness or a mocked-curltest would be worth it. Non-blocking.
Functionality (parity)
agents.sh:18(prod, unchanged) still pipescurl … | sh -s --, which is the exact "empty stream →shexits 0 → reports success while installing nothing" failure mode that the newagents-staging.sh:29-39was deliberately written to avoid (download to a temp file, check curl status and non-empty, then run). Out of strict scope for this PR, but aligning prod to the download-then-verify pattern would remove a real silent-success hazard from the more heavily used path.
Information
src/commands/setup.ts:100andsrc/ensure-skills.ts:40describe the skill spec asowner/repo@ref/owner/repo[@ref]in comments — the same misleading@notation thatenv.ts:22-27warns against. Updating these to#refwould keep a future maintainer from re-learning the footgun the hard way.insta login --env <name>(commands/auth.ts:18-28) persists an environment switch as a side effect (viaapi.persist()writing the newapiUrl) but, unlikeinsta env use, prints neither the "previous session dropped" nor the "re-point this machine's agents withinsta setup agent" guidance. Behavior is safe (login usesauth:false, so no token leaks; a failed login persists nothing), just a minor UX asymmetry between the two env-switching entry points.
Security: No new dependencies; no secrets newly logged or returned. OAuth loopback validates a random state. The release-channel gating (--prerelease + npm next dist-tag in release.yml:83-142) is a nice supply-chain safety measure preventing a staging build from reaching production installers. No security-blocking findings.
Performance: CLI one-shot paths only; resolve_prerelease is a single per_page=100 request; no N+1, unbounded loops, or hot-path work. No concerns.
Verdict: approved (informational)
No Critical findings. The session-safety design is correct and the test coverage for the TS surface is exemplary. The README.md:86 @devel typo is the one item I'd genuinely recommend fixing before merge, since it silently defeats a core goal of the change; the rest are non-blocking. (Actual GitHub approval remains a separate human action.)
3 participants
Adds a staging sibling to the agent onboarding one-liner:
Staging is already a complete environment —
api,mcp,sudo, dashboard, and the compute/db wildcards all resolve and are healthy under*.staging.instacloud.com(us-west-1). What was missing was any way for the installed CLI to target it coherently.Why this isn't just an alias
Three things would have broken a naive staging install, and all three are fixed here:
apiandmcpwere resolved independently.INSTA_API_URLandINSTA_MCP_URLare separate env vars. Set the first, forget the second, and the CLI talks to staging while this machine's agents stay registered against the production MCP server — silently, and reported as success. They now come from one switch (src/env.ts), so they cannot drift.The MCP server name was a single constant.
registerMcptreats an already-registered name as "nothing to do", and the config-file agents (Cursor/Codex/OpenCode/Copilot/Factory) key their entry by name. So installing staging on a machine that already had prod would keep prod's registration. Staging now registers asinsta-cloud-staging, which also lets both environments coexist on one machine.A piped installer can't export into the parent shell.
curl … | shmeans an exportedINSTA_ENVevaporates before the user's nextinsta project create. The installer therefore persists the choice viainsta env use, writing the same~/.insta/config.jsonthatlogin --api-urlalready writes.Session handling
Switching environments drops the stored session.
prodandstagingare separate deployments (us-east-2 vs us-west-1, different databases and auth), so the old token cannot authenticate there — and keeping it is actively unsafe, becauseapi.ts's 401 path would POST one deployment's refresh token to the other. This is the same reasoning as the retired-host path inconfig.ts, and it composes withfix/retired-api-url-migrationrather than fighting it.Note this means prod↔staging switching costs a re-login each way. Per-environment credential slots would remove that; deliberately left out of this PR as a separate, larger change to the config format.
Resolution order
Unchanged at the top, so
insta-ossand preview deployments behave exactly as before:INSTA_API_URL— a literal URL, the only way to reach a host no environment name covers. (INSTA_MCP_URLlikewise.)INSTA_ENV—prod|staging. An unrecognised value is a hard error, never a silent fallback to prod: a typo'dINSTA_ENV=stagngthat provisions real production infrastructure would stay invisible until the bill arrived.apiUrl.prod.New surface
insta env [--json]insta env use <prod|staging>insta login --env <name>install.sh --staging/--env <name>INSTA_ENV)insta statusVerification
npm run typecheckclean;npm test140 passing (20 files), 26 of them new intest/env-switch.test.ts. No existing test changed.$HOME: fresh install resolves prod;env use stagingpersists toconfig.json; a staging machine writesinsta-cloud-staging→https://mcp.staging.instacloud.com/mcpinto~/.cursor/mcp.jsonand~/.codex/config.toml; a subsequent prod install adds a second entry rather than colliding; a stored prod session is dropped on switch while unrelated keys (autoUpdate) survive.install.shargument matrix exercised directly (--agents -y,--staging,--env staging,--env=staging, flag-beats-INSTA_ENV, both error paths);sh -nclean on all three scripts.INSTA_ENVexits 1 with a one-line error, no stack trace.Not included / follow-ups
agents.staging.instacloud.comdo not exist yet — this PR adds the script the hostname will serve (agents-staging.sh); the hostname itself comes from a companion Terraform change ininsta-cloud/infra. Until that is applied, the staging path is reachable via the raw URL orinstall.sh --staging.skills/insta/cli-reference.mdmirror (perAGENTS.mdrule 4) — separate PR in theskillssubmodule.🤖 Generated with Claude Code
https://claude.ai/code/session_01DhegxHSDCTRVC3izhkvwyF
Summary by cubic
Adds a first-class staging environment and a
agents.staging.instacloud.comone‑liner. Also tightens env switching and release safeguards to prevent prod/staging drift and accidental production installs.New Features
curl -fsSL agents.staging.instacloud.com | shinstalls staging end to end — CLI prerelease when available, APIapi.staging.instacloud.com, MCPmcp.staging.instacloud.com/mcpregistered asinsta-cloud-staging, and skills fromInsForge/insta-skills#devel. Installer supports--staging/--env <prod|staging>and persists viainsta env use.insta env/insta env use <name>,insta login --env <name>.insta statusnow shows the environment.INSTA_API_URL>INSTA_ENV> persisted >prod(note:INSTA_MCP_URLandINSTA_SKILLS_REPOstill override).next;install.sh --stagingpicks the newest prerelease (per_page=100; falls back to stable if none).insta env), the script exits before agent setup and only prints the staging banner when persistence succeeds. It now rejects--env=with no value and verifies the download before execution to avoid empty-pipe success.Migration
INSTA_ENVorINSTA_API_URLthat points at a different deployment now clears the in-memory session; unsetting the override restores the stored login. Same-environment overrides (including trailing slashes) keep the session.--envorinsta env use. InvalidINSTA_ENVnow errors instead of falling back;INSTA_API_URLstill wins overINSTA_ENV.Written for commit f4aff4a. Summary will update on new commits.