Skip to content

feat(compute): public-access command and private addresses - #373

Merged
jwfing merged 1 commit into
mainfrom
jwfing/compute-private-network
Oct 15, 2026
Merged

jwfing merged 1 commit into
mainfrom
jwfing/compute-private-network

Conversation

@jwfing

@jwfing jwfing commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

Summary

The CLI side of compute private networking (platform InsForge/instacloud-platform#672; the plane has had it since insta-compute #413).

  • insta compute public-access [on|off] [service] and insta <redis|mysql|mongodb> public-access [on|off] [service]:
    • With no mode, it prints public access and the private address the platform reports (private_host:port, plus private_fqdn). It never composes one.
    • off makes the service private-only:
      • compute: its URL answers 403 and stops waking it; only services in the same branch reach it.
      • database: *_URL stops connecting; apps use *_PRIVATE_URL.
    • Closing asks first. With no terminal, or with --json, it refuses (exit 2) unless --yes is passed. This is the same contract as insta postgres public-access off.
    • on re-opens without asking.
    • The platform's coded refusals are rewritten as CLI instructions: private_network_disabled, private_network_unavailable_in_region, no_private_name (names insta service rename), and private_name_taken.
  • insta service list shows private <host>:<port> and, for a closed compute service, public off. A row with no private_host prints exactly as before.

Command surface mirrored in skills (cli-reference.md, separate PR).

Tests

  • test/private-network.test.ts:
    • show;
    • off asks, declines, and refuses without --yes (TTY and --json);
    • --yes;
    • on;
    • one row per refusal code;
    • the command surfaces the mapped message;
    • list lines with and without an address.
  • test/help-surface.test.ts: the managed database verb list and the compute verbs include public-access.
  • Negative controls, each sabotaged in isolation; the named tests fail, and all pass after restore:
    • C1: drop the --yes gate.
    • C2: drop a code mapping.
    • C3: drop the private address from the list line.
    • C4: drop the public off badge.
  • npm run typecheck and npm test pass: 2189 tests, after updating help-surface.

🤖 Generated with Claude Code


Summary by cubic

Adds the CLI side of compute private networking: new public-access commands for compute and managed database services, plus a private address shown in insta service list.

  • insta compute public-access [on|off] [service] and insta <redis|mysql|mongodb> public-access [on|off] [service] show public access and the platform-reported private address; off makes the service private-only (compute URLs answer 403 and stop waking the service; database *_URLs stop connecting, apps use *_PRIVATE_URL), and on re-opens without asking.
  • Closing asks first and requires --yes when there is no terminal or with --json; the platform's coded refusals now surface as CLI instructions (for example, a missing private name suggests insta service rename).
  • insta service list shows private <host>:<port> and a public off badge for a closed compute service; rows without a private host print exactly as before.

Written for commit bfc1da8. Summary will update on new commits.

View guided diff Turn on auto-fix

- `insta compute public-access [on|off] [service]` and
  `insta <redis|mysql|mongodb> public-access [on|off] [service]`: show the
  service's public access and its private address (<name>.insta.internal,
  <name>-<type>.insta.internal for a database); off makes it private-only,
  asks first, --yes when non-interactive; the platform's coded refusals
  (private_network_disabled, private_network_unavailable_in_region,
  no_private_name, private_name_taken) read as CLI instructions
- `insta service list` shows `private <host>:<port>` and `public off`

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@jwfing
jwfing force-pushed the jwfing/compute-private-network branch from c6ee9aa to bfc1da8 Compare October 12, 2026 22:36

@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 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. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="test/private-network.test.ts">

<violation number="1" location="test/private-network.test.ts:23">
P3: These fixtures mirror the name-derived address convention, so they do not catch a regression that composes the host or FQDN instead of using the platform-reported fields. Use deliberately non-derived host and FQDN values in the show and list fixtures.</violation>
</file>

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

View guided diff | Turn on auto-fix | Re-trigger cubic

import { closeWarning, publicAccessErrorMessage, publicAccessLines, servicePublicAccess } from '../src/commands/private-network.js'
import { serviceListLine } from '../src/commands/services.js'

const API = { id: 's1', type: 'compute', name: 'api', public: true, port: 8080, private_host: 'api.insta.internal', private_fqdn: 'api.0123abcd.insta.internal' }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: These fixtures mirror the name-derived address convention, so they do not catch a regression that composes the host or FQDN instead of using the platform-reported fields. Use deliberately non-derived host and FQDN values in the show and list fixtures.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At test/private-network.test.ts, line 23:

<comment>These fixtures mirror the name-derived address convention, so they do not catch a regression that composes the host or FQDN instead of using the platform-reported fields. Use deliberately non-derived host and FQDN values in the show and list fixtures.</comment>

<file context>
@@ -0,0 +1,134 @@
+import { closeWarning, publicAccessErrorMessage, publicAccessLines, servicePublicAccess } from '../src/commands/private-network.js'
+import { serviceListLine } from '../src/commands/services.js'
+
+const API = { id: 's1', type: 'compute', name: 'api', public: true, port: 8080, private_host: 'api.insta.internal', private_fqdn: 'api.0123abcd.insta.internal' }
+const CACHE = { id: 's2', type: 'redis', name: 'cache', public: true, port: 6379, private_host: 'cache-redis.insta.internal', private_fqdn: 'cache-redis.0123abcd.insta.internal' }
+const printed = () => vi.mocked(info).mock.calls.map((c) => c[0]).join('\n')
</file context>

@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.

Summary
No blocking defects found in the reviewed head commit. The commands follow existing access-control conventions and match the linked platform contract. Local validation was blocked by sandbox startup failure; review used GitHub files pinned to the PR head.

Requirements context
Assessed against the PR description, platform PR #672’s explicit networking contract, README, supplied AGENTS.md, and the repository’s developing-insta-cli guide. The existing Postgres public-access implementation establishes confirmation and approval conventions. No separate CLI-specific networking design document was verified.

Findings

Critical
(none)

Suggestion

  • test/private-network.test.ts: Cover JSON, branch selection, and pending approval - The tests cover confirmation, refusals, rendering, and mutation requests well. Add cases for JSON show/change output, an explicit branch override, and approval_required responses that must not print success. These paths are implemented but currently unasserted.

Information

  • Functionality: Implementation matches the platform contract - Service resolution uses the selected branch and service type; addresses come from platform fields; closing requires confirmation; reopening skips confirmation; all four specified refusal codes have CLI guidance. No blocking functionality finding.
  • Security: Existing authentication and approval handling retained - Requests use the authenticated API client and handleApproval before reporting success. The change introduces no shell execution, SQL, dependencies, or credential-value logging. No security finding.
  • Performance: Bounded request count and rendering work - Show performs one service-list request; mutations add one PUT. List rendering adds constant work per row without additional requests. No performance finding.
  • Rollout: Coordinate the platform migration and reference update - Platform #672 changes compute’s public column from its historical false value to actual access state. Before that migration, this CLI can label a publicly reachable compute service as public off. Deploy the platform migration before releasing this surface, and include the separately described cli-reference.md update.

Tests
No tests or typecheck ran: shell commands failed before execution with a bwrap namespace-permission error. Inspected all changed files through GitHub at bfc1da8, plus relevant helpers and the platform contract. Could not cross-check local checkout contents. The reported 2189 passing tests were not independently verified.

Verdict
approved: No Critical findings were identified; Suggestions and Information do not block approval under the specified rule.

@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.

Summary
No blocking defects were identified in the reviewed changes. The commands implement the stated confirmation policy, platform error mappings, and private-address display using existing CLI helpers. Local verification was blocked by the shell sandbox; review continued through read-only GitHub access.

Requirements context
Assessed against the supplied PR description, linked platform PR #672 and its API changes, the repository README, development skill, and existing Postgres access implementation. No standalone CLI private-network requirements document was identified in the inspected material. The separate skills-reference update was reported by the author but not verified.

Findings

Critical
(none)

Suggestion

  • test/private-network.test.ts: Cover JSON output, branch selection, and pending approvals - The tests cover confirmation, refusal mappings, and list formatting, but do not exercise successful JSON output, explicit branch selection, or an approval_required response. Add cases asserting the exact branch query, JSON-only success output, and absence of a success message when approval is pending.
  • PR description: Document MCP availability for the new commands - The development guide asks new leaf commands to name their matching MCP tool or explain why none is appropriate, explicitly as a non-blocking expectation. The description does not record that decision; add the tool or companion issue reference.

Information

  • Functionality: Core behavior matches the platform contract - The handler selects services within the requested/current branch, submits a boolean public value to the service-ID access endpoint, and prints platform-reported private names without deriving them. The four refusal codes match the linked platform change. Confirmation and reopening behavior follow the existing Postgres conventions.
  • Security: Existing authentication and approval paths are retained - Requests use the authenticated ApiClient and shared approval handler. Closing without --yes refuses in noninteractive or JSON mode before issuing the PUT. No new shell/SQL execution, dependencies, or explicit credential-value logging was introduced in the reviewed diff.
  • Performance: No material performance issue identified - Showing access performs one service-list request; changing it adds one PUT. Service selection is linear in the returned list, and the added list formatting performs constant work per row without extra network requests.
  • Testing: Static review completed with execution limitations - Inspected the changed source and tests at commit bfc1da8 through GitHub, plus surrounding helpers and API behavior. The new pure rendering helpers and injected confirmation dependency fit the repository's testable-command conventions. The checked-out files could not be independently cross-checked.

Tests
No tests or typecheck ran: shell commands failed before execution with 'bwrap: No permissions to create a new namespace'. The reported 2189 passing tests and negative controls were not independently reproduced. No files or external state were modified.

Verdict
approved: No Critical findings were identified; Suggestions and Information do not block approval under the specified verdict rule.

@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.

@jwfing
jwfing merged commit 3270bed into main Oct 15, 2026
3 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