refactor: Factor out icp-app from icp - #771
Open
adamspofford-dfinity wants to merge 3 commits into
Open
Conversation
…or them
Three things the project layer was reaching out of itself to get, now
handed to it. Each is a dependency that has to be cut before the app half
of this crate can leave.
`telemetry_data` was on `Host`, so resolving an environment wrote into a
bag the application owns. Now `Host` reports what it resolved through
`host::Observe` and the telemetry bag implements it; what becomes of the
facts is no longer the project layer's concern.
`IC_ROOT_KEY` sat in `context`, which meant the identity loader and the
network layer both reached up into app-side code for the one constant they
needed. It is a property of the IC, so it moves to the prelude.
`PackageCache` was threaded as a parameter through `Build`, `Synchronize`,
`operations::{build, sync, bundle}` and `deploy` — about twenty
signatures — because the two leaves that actually use it fetch wasm
modules over HTTP. Those leaves now ask `canister::wasm::Fetch` instead,
which `Builder` and `Syncer` hold; the cache is the concern of whoever
implements it. The parameter is gone from every signature in between, and
the only remaining mentions of `PackageCache` are in the modules that will
own it.
One incidental fix: the wasm fetcher built a fresh `reqwest::Client` per
download, and every consumer of the package cache constructed its own
`DirectoryStructureLock` over the same directory. Both are now made once
and shared.
`icp` held two unrelated things behind no boundary at all: what a project
is and how to build, install and sync it; and what this machine is —
identities and the keyring, user settings, the global directory layout,
the package cache, local networks and the launcher that runs them,
telemetry, offline message signing, and the operations that act on a
canister by principal rather than by what a manifest says about it.
The second half is now `icp-app`, which depends on `icp` and is not
depended on by it. `icp-cli` depends on both directly: `icp-app`
re-exports nothing, so there are no facade modules standing between the
CLI and the project layer.
Four seams carry what project code needs from the machine. Each is
declared in `icp` and implemented in `icp-app`:
- `network::Access` — a network's endpoints, its root key, and the
friendly-domain file its gateway serves.
- `canister::wasm::Fetch` — a wasm module a manifest names by URL.
- `canister::recipe::Resolve` — a recipe's Handlebars template.
- `host::Observe` — what resolution turned up, for telemetry.
Two of those needed reshaping to stop leaking. `Access` no longer has
`get_network_directory`: its return type is a layout `icp-app` invents,
so it moved to an `icp_app::network::Directories` trait that `Context`
carries. And `Resolve::commit` no longer takes a `PendingCache` — where a
template belongs in the cache is the resolver's business, so it now says
only *whether* it deferred a write and rebuilds it from the recipe, which
means nothing cache-shaped crosses the trait.
Those three traits are implemented on the far side of a crate boundary, so
their errors can no longer name their own source trees. Each therefore
carries its cause boxed, rendered with `#[snafu(display("{source}"))]` so
what the user sees is unchanged. This is a deliberate exception to the
rule that every erroring action gets its own variant: on a trait whose
implementation this crate cannot name, there is no variant to write.
`icp`'s mocks move behind a `test-util` feature, since `icp-app`'s tests
need the same seams and `#[cfg(test)]` does not cross a crate boundary.
The inner crate is 30 dependencies lighter: reqwest, keyring, bollard,
sysinfo, notify, wslpath2, directories, the HSM and key-format crates and
the rest of the identity stack are gone from it. tokio, ic-agent and
wasmtime remain, and go in the stages that introduce `CanisterCalls` and
`PluginRunner`.
The four errors that carry a cause boxed, because the trait they come
from is implemented past a crate boundary, each rendered it with
`#[snafu(display("{source}"))]`. That renders the cause as the wrapper's
own message *and* still reports it from `Error::source()`, so every chain
the cause appears in shows it twice:
Error: unable to access network 'local', is it running?
Caused by:
0: the local network for this project is not running
1: the local network for this project is not running
`#[snafu(transparent)]` is what these want. Display still forwards to the
cause, so the message is the same, but `source()` returns the cause's own
source rather than the cause, and the wrapper stops being a link in the
chain. Nothing below it is lost: a cause with sources of its own still
contributes all of them.
`LazyAgentError` was hand-written to do exactly this before it became a
derive, down to a comment reading "as `snafu(transparent)` would", so for
that one this is a restoration.
adamspofford-dfinity
added this pull request to stack #777
September 11, 2026 15:05
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
1 participant
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.
Stack created with GitHub Stacks CLI • Give Feedback 💬