Skip to content

feat: add service rename command#62

Merged
jwfing merged 2 commits into
mainfrom
feat/service-rename
Jul 21, 2026
Merged

feat: add service rename command#62
jwfing merged 2 commits into
mainfrom
feat/service-rename

Conversation

@jwfing

@jwfing jwfing commented Jul 20, 2026

Copy link
Copy Markdown
Member

Summary

  • Add insta services rename .
  • Validate new service names locally before calling the platform rename endpoint.
  • Add CLI unit coverage for service-name validation.

Test Plan

  • npm run typecheck
  • npm test

Summary by cubic

Adds a services rename CLI command to rename a service and re-key its managed secret names. Validates the new name locally with assertServiceName to prevent invalid requests.

  • New Features
    • Command: insta services rename
    • Options: --json, --branch
    • Tests for service-name validation (lower-kebab: a-z, 0-9, -)

Written for commit 95d960e. Summary will update on new commits.

Review in cubic

@jwfing jwfing left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: feat: add service rename command

Summary: Clean, small addition of insta services rename <type> <name> <new-name> that mirrors the existing service-command pattern; no blocking issues found.

Requirements context

No spec/plan directory exists in this repo (no docs/superpowers/, no docs/specs/, no docs/ at all) — assessed against the PR description and the repo conventions in AGENTS.md / .claude/skills/developing-insta-cli/SKILL.md.

Findings

Critical

(none)

Suggestion

  • Functionality — name-validation asymmetry (src/commands/services.ts:53-65 vs 92-104). servicesRename now calls assertServiceName(newName), but servicesAdd still accepts any name (no client-side validation). This means a user can create a service with a name (e.g. Primary) that the platform/rename rule would later reject, and the two commands disagree on what a valid name is. Consider applying assertServiceName in servicesAdd as well so creation and rename share one rule, or intentionally document that rename is the stricter gate.
  • Software engineering / functionality — regex is looser than "lower-kebab" (src/commands/services.ts:7, 20-22). /^[a-z0-9][a-z0-9-]{0,38}$/ accepts a trailing hyphen (foo-) and consecutive hyphens (foo--bar), which aren't typical "lower-kebab". Since this is a client-side pre-check for an authoritative platform endpoint (the platform lives in the sibling ../platform submodule and isn't visible here), an over-permissive check just defers to the server error — not a correctness bug — but it's worth confirming the pattern (including the 39-char max from {0,38}) matches the platform's actual service-name rule so the local error is accurate. Adding a test for a trailing-hyphen / max-length case would lock that intent in.

Information

  • Docs mirroring (AGENTS.md non-negotiable #4 / dev skill). New/renamed commands must be mirrored in skills/insta/cli-reference.md in the insta-cloud superproject skills/ submodule "in the same change set." That doc isn't part of this repo, so it can't appear in this PR's diff — just a reminder to land the cli-reference.md update alongside this so the agent-facing surface doc stays current.
  • Test coverage matches convention. The added assertServiceName unit tests (accepts primary-db; rejects Primary and -db) follow the file's "pure, unit-tested helpers" pattern. The servicesRename flow itself is untested, but that's consistent with the sibling commands (servicesRemove, servicesScale, servicesUpgrade) which also unit-test only their pure helpers — no regression here.

Dimension notes

  • Security: No new risk. newName is regex-validated then sent as a JSON body field; the service id in the URL comes from the platform's own services list (resolved via resolveServiceId), branch is encodeURIComponent-escaped via q(). No secrets logged, no auth/authorization change.
  • Performance: One GET /services + one POST /.../rename, identical to the other service commands. No N+1, no hot-path/blocking concerns.
  • Conventions: servicesRename matches the established structure (assertTypeApiClient.loadrequireProject → resolve id → rawRequesthandleApproval--json/info), and the commander registration + guard arity in src/index.ts:114-116 are correct.

Verdict

approved — no Critical findings. The two Suggestions (validation asymmetry, regex strictness) are non-blocking; the human approval on GitHub remains a separate action. (Note: this bot cannot cast the GitHub approval; posting as a comment.)

@Fermionic-Lyu Fermionic-Lyu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Approved. (Relaying John-bot's approved verdict — approved with the maintainer account, since John-bot can't approve its own PR.)

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 3 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/services.ts">

<violation number="1" location="src/commands/services.ts:7">
P2: The `SERVICE_NAME_RE` regex allows service names to end with a hyphen (e.g., `my-db-`) or contain consecutive hyphens (`test--double`). The error message describes the convention as "lower-kebab" but kebab-case typically doesn't allow trailing or consecutive hyphens. Consider tightening the regex to reject trailing hyphens so the CLI catches these before the platform API does, keeping the error message consistent and actionable.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/commands/services.ts

export const SERVICE_TYPES = ['postgres', 'storage', 'compute'] as const
export type ServiceType = (typeof SERVICE_TYPES)[number]
const SERVICE_NAME_RE = /^[a-z0-9][a-z0-9-]{0,38}$/

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The SERVICE_NAME_RE regex allows service names to end with a hyphen (e.g., my-db-) or contain consecutive hyphens (test--double). The error message describes the convention as "lower-kebab" but kebab-case typically doesn't allow trailing or consecutive hyphens. Consider tightening the regex to reject trailing hyphens so the CLI catches these before the platform API does, keeping the error message consistent and actionable.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/services.ts, line 7:

<comment>The `SERVICE_NAME_RE` regex allows service names to end with a hyphen (e.g., `my-db-`) or contain consecutive hyphens (`test--double`). The error message describes the convention as "lower-kebab" but kebab-case typically doesn't allow trailing or consecutive hyphens. Consider tightening the regex to reject trailing hyphens so the CLI catches these before the platform API does, keeping the error message consistent and actionable.</comment>

<file context>
@@ -4,6 +4,7 @@ import { info, printJson, handleApproval, renderNextActions } from '../util.js'
 
 export const SERVICE_TYPES = ['postgres', 'storage', 'compute'] as const
 export type ServiceType = (typeof SERVICE_TYPES)[number]
+const SERVICE_NAME_RE = /^[a-z0-9][a-z0-9-]{0,38}$/
 
 export function q(branch?: string): string {
</file context>

@jwfing
jwfing merged commit 658aac3 into main Jul 21, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants