feat(sync-plugin): make plugin compute-time limit configurable via env var#669
Merged
Conversation
…v var
The 60s wasm compute-time limit for `plugin` sync steps was hard-coded.
It's a local runaway guard (host-call latency is already excluded), not a
security boundary, and legitimately heavy plugins — e.g. brotli-compressing
a large asset bundle — can exceed it, especially on slower CI runners where
the same work burns more wall-clock time. A fixed value can't be calibrated
from a fast local machine, so it's now overridable per-environment.
- Add ICP_CLI_PLUGIN_COMPUTE_LIMIT_SECS (default 60). Resolved up front in
the plugin sync step so a malformed value fails fast, before wasm
resolution or any network work, rather than being silently ignored.
- The limit-exceeded error now names the effective limit and the env var,
and phrases the fix relative to the current limit ("above Ns") so it reads
correctly whether the limit is the default or an override.
- Tests: unit coverage for env parsing (valid/invalid/zero) and the error
message; a runtime test that traps a spinning fixture under a 1s limit;
and an integration test proving the override is read on the real CLI sync
path (a bogus value aborts deploy with an actionable error).
- Document the variable alongside the other ICP_CLI_* overrides.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Makes sync-plugin compute-time limits configurable while retaining the 60-second default.
Changes:
- Adds environment-variable parsing and runtime limit propagation.
- Improves timeout diagnostics and test coverage.
- Documents the configuration and release change.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
CHANGELOG.md |
Records the configurable limit. |
docs/reference/environment-variables.md |
Documents the new environment variable. |
crates/icp/src/canister/sync/plugin.rs |
Parses and forwards the configured limit. |
crates/icp-sync-plugin/src/lib.rs |
Exports limit constants. |
crates/icp-sync-plugin/src/runtime.rs |
Applies the limit and improves timeout errors. |
crates/icp-sync-plugin/tests/fixtures/test-plugin/src/lib.rs |
Adds a CPU-spinning fixture. |
crates/icp-cli/tests/sync_tests.rs |
Tests malformed configuration end-to-end. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…docs - resolve_compute_limit_secs: only VarError::NotPresent selects the default; a present-but-non-UTF-8 value is now rejected as invalid rather than silently falling back to 60s, honoring the fail-loudly contract. - docs/concepts/sync-plugins.md: the resource-limits table and paragraph now present 60s as the default and point to ICP_CLI_PLUGIN_COMPUTE_LIMIT_SECS, so the two user-facing docs no longer conflict. - crates/icp-sync-plugin/DESIGN.md: add compute_limit_secs to the documented run_plugin signature and note its env-var origin. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lwshang
marked this pull request as ready for review
July 23, 2026 20:15
adamspofford-dfinity
approved these changes
Jul 23, 2026
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.
Summary
The 60s wasm compute-time limit for
pluginsync steps was hard-coded inicp-sync-plugin. It's a local runaway guard — host-call/network latency is already excluded from the budget, and the plugin runs locally in a read-only WASI sandbox — not a security or protocol boundary. Legitimately heavy plugins can exceed it, which is what surfaced in dfinity/examples#1449: migrating from theasset-canister:v2.2.1recipe tostatic-assets:v0.3.1made the sync plugin do brotli compression, pushing a realdist/past 60s and failing CI withplugin exceeded the 60s compute time limit.Because pure-compute time is machine-dependent (the same work burns more wall-clock on a throttled CI runner than on a dev laptop), a fixed value can't be sensibly calibrated locally. This makes the limit overridable per-environment via an env var, defaulting to 60s.
Changes
ICP_CLI_PLUGIN_COMPUTE_LIMIT_SECS(default60). Resolved up front in the plugin sync step, so a malformed value fails fast — before wasm resolution or any network work — rather than being silently ignored. A non-integer or0value is rejected with a clear error.… raise the limit by setting ICP_CLI_PLUGIN_COMPUTE_LIMIT_SECS above 60s.), so it reads correctly whether the limit is the default or an existing override.ICP_CLI_*overrides indocs/reference/environment-variables.md.Tests
0) and the error-message wording (fixture-free, runs everywhere).ICP_CLI_PLUGIN_COMPUTE_LIMIT_SECSon a realdeployand asserts it aborts with the actionable error — proving the override is read on the real CLI sync path, not just in unit tests.cargo fmt,cargo clippy --all-targets, and the affected test suites all pass.Unblocking dfinity/examples#1449
After this merges and ships in a release, the two failing example jobs can be unblocked by upgrading icp-cli and adding one line to the workflow env, e.g.:
No recipe changes required.