feat(sync)!: remove assets sync step type#582
Merged
Conversation
The built-in `assets` sync step (`type: assets`), which uploaded a directory to an asset canister via `ic-asset`, is retired. Asset uploading now lives outside icp-cli core (e.g. a recipe-provided sync plugin). `type: assets` is still recognized at the parsing surface so a manifest that uses it fails with a targeted, helpful error instead of serde's generic "unknown variant" message. Everything downstream — the sync operation, its error variant, the dispatch arm, and the now-unused `ic-asset`/`slog` dependencies — is removed. The generated schemas drop the variant, and a stale advisory-ignore for `derivative` (only pulled in by `ic-asset`) is pruned. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove the asset-sync sections and `dir`/`dirs` tables and point asset uploads at a generic `script`/`plugin` sync step, without naming a specific recipe implementation (recipes are not part of icp-cli core). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Swap the `type: assets` sync step for the certified-assets sync plugin in the static-assets, static-react-site, and frontend-env-vars examples, and bump the asset-canister recipe example to v2.2.0 (which now generates a plugin-based sync step). Drop the stale v0.1.0 schema annotation from the manifests that now use a `plugin` step. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR removes the built-in type: assets sync step from icp-cli core and shifts asset uploading responsibility to external mechanisms (notably WASM sync plugins, e.g. via recipes). It also updates schema/docs/examples accordingly and adds a targeted parse-time error for legacy manifests that still specify type: assets.
Changes:
- Removes the
assetssync step implementation and its dependencies (ic-asset,slog), and drops theassetsvariant from generated JSON schemas. - Adds a custom
Deserializepath forSyncStepthat recognizestype: assetsonly to emit a clearer, purpose-built error message. - Updates docs/tests/examples to use
plugin(and bumps the asset-canister recipe example to@dfinity/asset-canister@v2.2.0).
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/icp-static-react-site/icp.yaml | Switches sync from assets to plugin for the React static site example. |
| examples/icp-static-assets/icp.yaml | Switches sync from assets to plugin for the static assets example. |
| examples/icp-static-assets-recipe/README.md | Bumps asset-canister recipe version reference to v2.2.0. |
| examples/icp-static-assets-recipe/icp.yaml | Bumps asset-canister recipe version to v2.2.0. |
| examples/icp-frontend-environment-variables/frontend/canister.yaml | Switches frontend sync step from assets to plugin. |
| docs/schemas/icp-yaml-schema.json | Removes the assets sync step variant from the generated icp.yaml schema. |
| docs/schemas/canister-yaml-schema.json | Removes the assets sync step variant from the generated canister.yaml schema. |
| docs/reference/configuration.md | Removes built-in “assets sync” docs and points users to plugin-based uploads. |
| docs/guides/managing-environments.md | Updates environment guide example to use a plugin sync step. |
| docs/concepts/build-deploy-sync.md | Updates conceptual docs to describe asset uploads via plugins instead of built-in assets sync. |
| crates/icp/src/manifest/canister.rs | Removes Assets variant; adds custom Deserialize to reject type: assets with a targeted error. |
| crates/icp/src/manifest/adapter/mod.rs | Removes assets adapter module export. |
| crates/icp/src/manifest/adapter/assets.rs | Deletes the assets adapter type definitions. |
| crates/icp/src/canister/sync/mod.rs | Removes assets sync runner and error variant. |
| crates/icp/src/canister/sync/assets.rs | Deletes the assets sync implementation (ic-asset based). |
| crates/icp/Cargo.toml | Drops ic-asset and slog dependencies. |
| crates/icp-cli/tests/sync_tests.rs | Replaces assets sync integration test with a rejection/error-message test. |
| crates/icp-cli/tests/deploy_tests.rs | Removes reliance on assets sync step in deploy URL printing test. |
| CHANGELOG.md | Documents the breaking removal of type: assets. |
| Cargo.toml | Removes workspace dependency versions for ic-asset and slog. |
| Cargo.lock | Lockfile updates reflecting dependency removals/resolution changes. |
| .cargo/audit.toml | Removes now-unneeded audit ignore entries tied to ic-asset transitive deps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Bump `@dfinity/asset-canister` recipe references from v2.1.0 to v2.2.0 across the docs guides and docs-site/icp.yaml. v2.1.0 emits the retired `type: assets` sync step and would no longer load after this change; v2.2.0 uses a `plugin` sync step. - Note the minimum compatible recipe version in the configuration reference. - Assert the `assets` removal error stays recipe-agnostic in sync_tests so the message can't regress to leaking a specific recipe identifier. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
adamspofford-dfinity
approved these changes
Jun 3, 2026
raymondk
reviewed
Jun 3, 2026
| @@ -112,31 +112,11 @@ build: | |||
|
|
|||
| Sync steps run after canister deployment to configure the running canister. | |||
Collaborator
There was a problem hiding this comment.
There should be a section about sync plugins:
- How it works
- An example of a plugin
- A link to the release of the migration asset canister sync plugin
Could be a different PR.
…elds Add user-facing documentation for sync plugins: - concepts/sync-plugins.md: the mechanism, WIT interface, canister-call routing, sandbox, and resource limits - guides/writing-sync-plugins.md: authoring a plugin in Rust - reference/configuration.md: a "Plugin Sync" section with the type: plugin fields (path/url/sha256/dirs/files) - cross-link from build-deploy-sync.md, index pages, and the docs sidebar Trim crates/icp-sync-plugin/DESIGN.md to the host-side implementation and design rationale not covered by the user docs (and fix its stale run_plugin return type and HostState fields). Remove TODO.md: both items (epoch-based timeout, end-to-end tests) are already implemented. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Existing 0.2.x projects that use the built-in `assets` sync step (`type: assets`) fail to load once it is removed in later releases (the 0.3 release candidate and 1.0.0). Add a migration guide that quotes the targeted load error and walks through both upgrade paths: bump the `@dfinity/asset-canister` recipe to v2.2.0, or replace a hand-written `type: assets` step with a `plugin` step pointing at the certified-assets sync plugin (including the singular `dir:` to list `dirs:` conversion). Wire it into the docs index, the docs-site sidebar, and cross-link it from the deprecation note in the configuration reference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lwshang
force-pushed
the
lwshang/SDK-2741_assets_sync_step_error
branch
from
8d0a714 to
f4e2b26
Compare
Two issues broke the Documentation workflow: - The frontmatter `description` contained an unquoted `type: assets`; the inner `: ` made the YAML parser read it as a nested mapping. Quote it. - The filename's dot (`upgrading-from-v0.2.md`) produced a Starlight content slug that didn't match the `migration/upgrading-from-v0.2` referenced in the sidebar config. Rename to a dot-free `upgrading-from-v0-2.md` and update the sidebar slug plus the two internal links. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lwshang
added a commit
that referenced
this pull request
Jun 4, 2026
* fix: reconcile project bundling with assets sync step removal The merge of #530 (project bundling) did not account for #582, which removed the `assets` sync step type, leaving `main` unable to compile: `bundle.rs` and `manifest/mod.rs` still referenced the deleted `adapter::assets` module and `SyncStep::Assets` variant. Mirror #582's removal in the bundle path: - drop the stale `adapter::assets` re-export - remove the asset-step handling from bundle.rs (import, match arms, `prepare_asset_step`, `asset_dirs` field and its archive-write loop); `type: assets` no longer deserializes, so this was dead code - convert the four asset-based bundle tests to plugin sync steps, which the bundle code already handles. `bundle_and_deploy` now uses the local example sync plugin (proven end-to-end by `sync_plugin_registers_seed_data`) instead of the retired `@dfinity/asset-canister@v2.1.0` recipe - hoist `build_sync_plugin_example` into the shared test `common` module Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(test): match bundle test doc comments to the plugin `dirs` field Address review feedback: the converted plugin-sync tests use the `dirs` list field, but two doc comments still said `dir` (singular). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Retires the built-in
assetssync step (type: assets). Asset uploading now lives outside icp-cli core (e.g. a recipe-provided sync plugin).A manifest that still uses
type: assetsfails to load with this error (it does not name a specific recipe):type: assetsis still recognized when parsing solely so we can emit this targeted message instead of serde's generic "unknown variant" error.Other changes:
ic-asset/slogdependencies are removed; generated schemas drop the variant.pluginsync step, and the asset-canister recipe example is bumped tov2.2.0(which generates a plugin-based step).v2.2.0, or swaptype: assetsfor the certified-assetspluginstep), cross-linked from the configuration reference, docs index, and docs-site sidebar.Test plan
cargo test(unit + integration),cargo fmt --check,cargo clippy— green.icp project showon atype: assetsmanifest prints the error above, and on the updated examples succeeds.🤖 Generated with Claude Code