Skip to content

feat(domain): delegate a bought domain's zone, and take it to another registrar - #263

Merged
Fermionic-Lyu merged 1 commit into
mainfrom
feat/domain-nameservers-transfer
Sep 21, 2026
Merged

Fermionic-Lyu merged 1 commit into
mainfrom
feat/domain-nameservers-transfer

Conversation

@Fermionic-Lyu

@Fermionic-Lyu Fermionic-Lyu commented Sep 20, 2026

Copy link
Copy Markdown
Member

Platform #484 shipped both ways a bought domain leaves; the CLI had neither.

  • insta domain nameservers set <domain> <ns...> / reset <domain> — delegate the zone away, or put it back.
  • insta domain transfer lock <domain> <on|off> / code <domain> — the registrar lock and the EPP code.

Where the delegation shows up

Not in a command of its own. domainLines — the one renderer list and status already print — now carries it, so insta domain status myapp.com says the domain is delegated and names the reset that undoes it. My first draft added a nameservers show that re-read the same endpoint and re-implemented status's find-by-name; it left list and status still silent about a domain nothing can attach to, which is the surface a user is already looking at.

set and reset print that same renderer rather than a bespoke summary. It already renders a failed hostname as www.myapp.com failed → web — <reason>, de-dupes the apex/www pair, and names which hostname to re-attach — all of which my own version did worse, and one of whose lines printed the platform's 130-character reason once per hostname when delegatedReason is a single shared string.

Two things the first draft claimed that are not true

  • "attach a hostname to publish its records again" on reset. The platform deliberately does not delete the published records (service.ts: "The published records STAY: a zone nothing answers from is inert"), so reset republishes nothing — it un-delegates, and an attach re-adopts what is already there. The line was also unconditional, so a reset on a healthy domain told the user to go attach something.
  • "the registry's own 60-day lock". It is ICANN's, and a registry lock (serverTransferProhibited) is a different mechanism a user would go asking the wrong party about. PurchasedDomain.transferLockExpiresAt says "ICANN 60-day lock after registration". Corrected here and in the skills row; platform/docs/domain-purchase.md still says "registry's own" and is the remaining copy of the error.

transfer lock <domain> <on|off> is one command with a mode, matching compute always-on <mode> rather than inventing a pair of sibling verbs.

Coverage

40 tests in test/domain.test.ts. Five mechanisms my own review found unguarded are pinned now: the lapsed-ICANN-lock comparison (the platform never clears the date, so an old domain always carries a past one and only this comparison suppresses the claim), the trailing-dot strip, the delegated line in domainLines, the on|off guard, and transfer code --json — the one new shape that is not a purchased domain.

Gate

npm run typecheck, npx vitest run test/domain.test.ts 40/40, npm run build + all three --help pages.

npx vitest run over the whole suite fails 6 tests in test/setup-agent.test.ts with "org not found". That is an ancestor .insta/project.json in the parent checkout, it reproduces on origin/main, and it is not this diff.

Not here

The MCP server gets nothing. All four of these writes answer 403 to an agent credential — the auth code is a POST precisely so an agent cannot read it — so the tools would fail every call. Its real gap is that bought domains have no read tool at all, which is its own scope.

🤖 Generated with Claude Code


Summary by cubic

Adds the CLI half of the two ways a bought domain can leave InstaCloud, both already supported by the platform: delegating its DNS zone and transferring it to another registrar. Delegating takes down the hostnames that domain serves, and status/list now show when a domain is delegated.

New Features

  • insta domain nameservers set <domain> <ns...> delegates the zone; reset <domain> un-delegates without republishing records, and an attach re-adopts what is still there.
  • insta domain transfer lock <domain> <on|off> toggles the registrar transfer lock, and transfer code <domain> prints the EPP authorization code.
  • Opening the lock names ICANN's 60-day post-registration lock when it still applies and points to the code command.

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

Review in cubic

@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="test/domain.test.ts">

<violation number="1" location="test/domain.test.ts:422">
P3: The new commands `domainNameserversSet`, `domainNameserversReset`, and `domainTransferLock` each print the full `Purchased` domain object under `--json`, but none of the added tests exercises those `--json` branches, while the PR calls out JSON shape as an addressed edge case. Add a `--json` test for each (e.g. asserting `JSON.parse(out())` equals the returned domain) so the JSON contract of the new commands is locked down.</violation>
</file>

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

Re-trigger cubic

Comment thread src/commands/domain.ts Outdated
Comment thread src/commands/domain.ts Outdated
Comment thread test/domain.test.ts
@@ -1,5 +1,5 @@
import { describe, it, expect, vi, afterEach, afterAll } from 'vitest'

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: The new commands domainNameserversSet, domainNameserversReset, and domainTransferLock each print the full Purchased domain object under --json, but none of the added tests exercises those --json branches, while the PR calls out JSON shape as an addressed edge case. Add a --json test for each (e.g. asserting JSON.parse(out()) equals the returned domain) so the JSON contract of the new commands is locked down.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/domain.test.ts, line 422:

<comment>The new commands `domainNameserversSet`, `domainNameserversReset`, and `domainTransferLock` each print the full `Purchased` domain object under `--json`, but none of the added tests exercises those `--json` branches, while the PR calls out JSON shape as an addressed edge case. Add a `--json` test for each (e.g. asserting `JSON.parse(out())` equals the returned domain) so the JSON contract of the new commands is locked down.</comment>

<file context>
@@ -319,3 +319,107 @@ describe('domain records', () => {
+  // The one new shape that is not a purchased domain.
+  it('answers --json with the code object, not the domain', async () => {
+    const { deps: d } = deps({ '/auth-code': { authCode: 'EPP-123' } })
+    await domainTransferCode('myapp.com', { json: true }, d)
+    expect(JSON.parse(out())).toEqual({ authCode: 'EPP-123' })
+  })
</file context>

@agent-zhang-beihai agent-zhang-beihai 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.

Reviewed by Wang Miao

This adds domain nameservers set|reset and domain transfer lock|code, wired straight onto the platform routes that already exist for them, with the delegated state folded into the shared domain renderer. The verbs, the request shapes and the transfer-code POST all match the control plane, and the tests cover them well — but the renderer's follow-up hints were not reconciled with the new state or the new callers, so the primary command's own output tells you to run something the platform refuses, and it does it against the wrong org under --org. Both are one-line fixes in lines this diff already touches; I'd want them before merge.

After a delegation the CLI's first hint is an attach the platform refuses

important · defect · correctness · src/commands/domain.ts:105

Delegating marks every hostname failed on the platform side (src/domains/service.ts:627, :722), so d.hostnames.every(h => h.state === 'failed') is always true on the answer nameservers set prints — and vacuously true for a delegated domain with no hostnames. The "nothing serving" line therefore fires on the success path of the new command and names insta domain attach <host>, which the attach path rejects with a 409 (src/domains/service.ts:384), immediately contradicted by the very next line this change adds. I ran it: the output is nothing serving — insta domain attach www.myapp.com followed by delegated to … — attach is refused until: insta domain nameservers reset myapp.com. Gating the existing branch on !d.delegated leaves only the correct instruction.

Evidence

read-the-codesrc/commands/domain.ts:102-115, :151-158; platform src/domains/service.ts:204, :384, :620-630, :718-724; ran the renderer against the PR's own delegated fixture

The new hints re-emit follow-up commands for the wrong org under --org

important · defect · conventions · src/commands/domain.ts:112

domainLines' linked parameter exists precisely to drop next-action commands when --org names an org they do not act on — added in 2686697 and pinned by "drops the follow-up commands when --org names an org they do not act on". The new delegated line ignores linked entirely and prints insta domain nameservers reset <domain> with no --org, and domainNameserversSet/Reset call domainLines(r) with the default linked = true even when opts.org was given. So insta domain list --org org9 on a delegated domain, and insta domain nameservers set --org org9 …, both hand the reader a reset that resolves the linked project's org instead — and the attach hint above it, which takes no --org at all. Pass !opts.org from the new callers and gate the delegated line on linked (or carry the --org into the printed command).

Evidence

read-the-codesrc/commands/domain.ts:102-115, :124-130, :151-165; test/domain.test.ts:236-244; git log -L102,116 showing 2686697 introducing linked; ran domainNameserversSet with { org: 'org9' } and saw both hints printed unqualified

@agent-zhang-beihai agent-zhang-beihai 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.

Reviewed by Yang Dong

This adds registrar nameserver delegation/reset and transfer-lock/auth-code commands over the new platform endpoints. The endpoint wiring is sound, but the generated guidance includes commands that cannot succeed or target the wrong organization, and the required agent CLI reference is missing, so I would request changes.

Delegation prints an attach command that the platform will reject

important · defect · correctness · src/commands/domain.ts:105

After delegating, the platform marks every serving hostname failed, so this condition emits insta domain attach ...; an empty hostname list also passes it vacuously. Because the returned domain is delegated, that attach is necessarily refused—as the next output line itself says—so suppress the retry when d.delegated is true and present reset as the repair.

Evidence

read-the-codesrc/commands/domain.ts:102-114, test/domain.test.ts:354-366, platform/src/domains/service.ts:312-359

Follow-up commands discard the explicit --org target

important · defect · correctness · src/commands/domain.ts:112

All four new commands accept --org and correctly send their initial request there, but the printed nameservers reset, domain attach, and transfer code commands omit it. Running one after a cross-org operation therefore falls back to the linked project’s organization, producing a 404 or acting on a same-named domain in the wrong org; preserve opts.org in these commands or suppress the hints as existing list/status output does.

Evidence

read-the-codesrc/commands/domain.ts:102-112, src/commands/domain.ts:151-179, src/commands/billing.ts:7-13, src/index.ts:218-232, test/domain.test.ts:237-245

The new command surface is absent from the required agent reference

important · judgement · conventions · src/index.ts:218

The repository explicitly requires every command or flag change to update skills/insta/cli-reference.md in the same change set, but that reference still stops at the existing domain record commands. Add entries for nameserver set/reset and transfer lock/code, including their --org and human/agent authorization behavior, so agents can discover and safely hand off these operations.

Evidence

read-the-code.claude/skills/developing-insta-cli/SKILL.md:36-38, src/index.ts:218-232, skills/insta/cli-reference.md:75-80, platform/src/govern/agent-routes.ts:339-365

… registrar

Platform #484 added both exits; the CLI had neither.

- `domain nameservers show|set|reset` — whose nameservers the registrar holds,
  delegate the zone away, put it back. `set` takes down every hostname the domain
  serves, because the records an attach published live in the zone being left, so
  it prints each one with the platform's reason rather than reporting success.
- `domain transfer unlock|lock|code` — the registrar lock and the EPP code, two
  steps because the code alone moves nothing. `unlock` names the registry's own
  60-day lock when that still holds the domain.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Fermionic-Lyu
Fermionic-Lyu force-pushed the feat/domain-nameservers-transfer branch from 2294fed to e3ebaad Compare September 20, 2026 19:53
@Fermionic-Lyu

Copy link
Copy Markdown
Member Author

Head is now e3ebaad. Both correctness findings were real; the third is already done in the repo the rule names.

The success path offered an attach the same answer refuses (both reviewers)

Confirmed by running it. Delegating fails every hostname on the platform side, so hostnames.every(h => h.state === 'failed') is true on exactly the answer nameservers set prints — and vacuously true when the domain serves nothing. The retry line fired on the success path and was contradicted by the line printed immediately after it.

The branch is now gated on !d.delegated, so a delegated domain gets one instruction, which is the reset.

The hints ignored --org (both reviewers)

Also real, and it was three separate leaks: the delegated line ignored linked outright, set/reset called domainLines(r) with the default rather than !opts.org, and the transfer-code hint had no --org check at all. All three follow the existing convention now — suppress, not rewrite, which is what linked was added for and what list/status already do.

The delegated fact still prints under --org, shortened to delegated to … — attach is refused; it is state, not a next action.

skills/insta/cli-reference.md

AGENTS.md:16 points at the superproject skills/ submodule, not a path in this repo — there is no skills/ here. It is updated, in the same change set: instacloud-skills#117, open alongside this one and already reviewed. Both rows carry the --org flag and the agent-refusal behaviour the finding asks for.

Coverage

43 tests. Four mutations, four red: the !d.delegated guard on the retry, !opts.org on the nameserver hints, !opts.org on the code hint, and linked on the reset hint.

Separately

The "registry's own 60-day lock" wording this PR corrected turned out to come from platform/docs/domain-purchase.md, which I wrote in #484 and which still carried it. Fixed there: instacloud-platform#505.

@agent-zhang-beihai agent-zhang-beihai 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.

Reviewed by Wang Miao

Both findings from round 1 are fixed in e3ebaad, and I checked rather than took the word for it: the "nothing serving" retry is now gated on !d.delegated, so the success path of nameservers set no longer offers an attach the platform refuses; and the delegated hint, the reset command and the transfer-code hint all follow linked/opts.org, so --org gets the fact without a command that would act on the other org. I ran the renderer in both modes and the new tests pin each case. The rest of the change is unchanged from last round and still lines up with the platform routes, the PurchasedDomain shape and the repo's on|off argument convention.

No findings. Approve.

@agent-zhang-beihai agent-zhang-beihai 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.

Reviewed by Yang Dong

This adds registrar nameserver delegation/reset and transfer-lock/auth-code commands with sound endpoint wiring and user guidance. However, the new public command surface is absent from the mandatory agent reference, so I would request changes.

The new commands are missing from the required agent reference

important · judgement · conventions · src/index.ts:218

The repository requires every command or flag change to update skills/insta/cli-reference.md in the same change set, but its domain section still stops at the existing record commands. Add entries for nameserver set/reset and transfer lock/code, including --org, admin requirements, and the fact that agent credentials are refused, so agents can discover and safely hand off these operations.

Evidence

read-the-codeAGENTS.md:15-17, .claude/skills/developing-insta-cli/SKILL.md:36-38, src/index.ts:218-232, skills/insta/cli-reference.md:75-80, platform/src/govern/agent-routes.ts:339-366, platform/src/server.ts:3363-3436

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

@Fermionic-Lyu
Fermionic-Lyu merged commit 5aeb531 into main Sep 21, 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