Repository navigation
refactor(release): move the backend deploy path from dfx to icp-cli - #672
Draft
MRmarioruci wants to merge 2 commits into
Draft
MRmarioruci wants to merge 2 commits into
MRmarioruci wants to merge 2 commits into
Conversation
dfx now prints a deprecation notice pointing at icp-cli on every invocation, so the deploy path moves over. The script imports the signing key with `icp identity import` and upgrades the control panel with `icp canister install`, resolving the canister from canister_ids.json since icp-cli identifies a network by URL rather than by a dfx.json name. Isolating the identity store goes back to repointing HOME, which is what icp-cli keys its store off. That was the thing that broke under dfx, where HOME also carried the dfxvm version store; icp-cli is a single binary with no version shim, so there is nothing to hide. The toolchain check now verifies the version rather than just presence. An npm-installed icp under a node version manager can shadow a newer one on PATH, which is exactly how this was hit in testing: 0.3.0 winning over 1.3.0 and failing on argument parsing deep inside a deploy.
Follows the script: `registry publish` shells out per registry entry, and those calls were the last dfx dependency. icp-cli returns the raw reply bytes rather than a decoded value, so the reply is decoded against the generated control-panel interface, which is now emitted as a JS IDL factory alongside the existing type binding. Arguments go through argv instead of a shell string and a temp file. Registry entries carry free-form descriptions and tags, so a quote in one of those previously broke the Candid argument or injected into the command. The wasm chunk store id now comes from canister_ids.json rather than `dfx canister id`, and icx-asset gets its pem from `icp identity export`, which keeps it independent of the store layout. That removes the last reader of the dfx identity store, so the homedir lookup goes with it. `assertReplicaIsHealthy` is dropped rather than ported. It shelled out to `dfx ping` and every caller invoked it without awaiting, so it never gated anything and its rejection went unhandled.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Plaintext keys can remain on disk, and interpolated Candid arguments still fail for quoted values.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 2
Open (3)
What changed in this PR
Migrates backend deployment and registry operations from dfx to pinned icp-cli tooling.
Changes:
- Replaces deployment and registry calls with
icp-cli. - Adds Candid reply decoding and canister-ID resolution.
- Generates and packages the control-panel IDL factory.
| File | Description |
|---|---|
scripts/deploy-backend |
Migrates identity import and deployment to icp-cli. |
cli/src/utils.ts |
Removes dfx-specific utilities. |
cli/src/registry/remove.ts |
Migrates registry deletion. |
cli/src/registry/registry.core.ts |
Migrates lookup and search operations. |
cli/src/registry/publish.ts |
Migrates publishing and identity export. |
cli/src/registry/list.ts |
Removes ineffective replica health check. |
cli/src/registry/icp.ts |
Adds centralized icp-cli integration. |
cli/src/generated/control_panel.did.js |
Adds runtime control-panel IDL. |
cli/src/generated/control_panel.did.d.ts |
Types the generated IDL module. |
cli/package.json |
Generates and packages the IDL factory. |
.github/actions/deploy-backend/action.yaml |
Installs pinned icp-cli. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const replicaUrl = await getReplicaUrl(options.network); | ||
| const identityPemPath = await getIdentityPemFilePath(options.identity); | ||
| const identityPemPath = await exportIdentityPem(options.identity); | ||
| process.on('exit', () => rmSync(identityPemPath, { force: true })); |
| # control-panel install and to the `dfx canister call` that orbit-cli runs | ||
| # underneath `registry publish`. | ||
| export DFX_WARNING=-mainnet_plaintext_identity | ||
| ICP_HOME="$(mktemp -d)"; chmod 700 "$ICP_HOME" |
Comment on lines
+76
to
+78
| getCanisterId('control_panel', opts.network).toText(), | ||
| opts.method, | ||
| opts.arg, |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
dfx prints a deprecation notice pointing at icp-cli on every invocation, so the release deploy path moves over.
scripts/deploy-backendimports the key withicp identity importand upgrades the control panel withicp canister install; insideorbit-cli, everydfx canister callbecomes anicp canister call. The composite action installs a pinned@icp-sdk/icp-cliinstead of dfx.icp-cli returns raw reply bytes rather than a decoded value, so replies are decoded against the control-panel interface, now also emitted as a JS IDL factory next to the existing type binding. Arguments go through argv instead of a shell string and a temp file, which matters because registry entries carry free-form descriptions and tags and a quote in one of those previously broke the Candid argument. The wasm chunk store id comes from
canister_ids.jsonrather thandfx canister id, and icx-asset gets its pem fromicp identity export, so nothing reads the dfx identity store any more.assertReplicaIsHealthyis dropped rather than ported. It shelled out todfx ping, and every caller invoked it without awaiting, so it never gated anything and its rejection went unhandled.Identity isolation goes back to repointing HOME, which is what icp-cli keys its store off. That is what broke under dfx, where HOME also carried the dfxvm version store; icp-cli is a single binary with no version shim. The toolchain preflight now checks the version rather than presence, because an npm-installed icp under a node version manager can shadow a newer one on PATH. That happened while testing this: 0.3.0 winning over 1.3.0 and failing on argument parsing.
Verified read-only against the playground control panel:
registry list --app station --network playgroundreturns both entries and--app upgraderreturns one, so the call, the canister-id lookup, the decode and the pagination loop all work.Draft because the update calls are unproven. Only queries were exercised;
add_registry_entryandedit_registry_entrygo through the same decode path but have not been run. The first publish to playground is the test.