Skip to content

refactor: Factor out icp-app from icp - #771

Open
adamspofford-dfinity wants to merge 3 commits into
spofford/de-context-operationsfrom
spofford/factor-app
Open

refactor: Factor out icp-app from icp#771
adamspofford-dfinity wants to merge 3 commits into
spofford/de-context-operationsfrom
spofford/factor-app

Conversation

@adamspofford-dfinity

@adamspofford-dfinity adamspofford-dfinity commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Stack created with GitHub Stacks CLI • Give Feedback 💬

@adamspofford-dfinity
adamspofford-dfinity requested a review from a team as a code owner September 11, 2026 15:05
@adamspofford-dfinity
adamspofford-dfinity added this pull request to stack #777 September 11, 2026 15:05
@adamspofford-dfinity
adamspofford-dfinity requested a balanced review from Copilot September 14, 2026 12:47
Copilot AI reviewed Sep 14, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new test utility features do not fully expose the mocks they advertise to downstream crates.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Separates machine- and application-specific functionality into the new icp-app crate, leaving project-oriented logic in icp and updating the CLI to use both layers.

Changes:

  • Moves identity, settings, networking, package-cache, telemetry, and direct canister operations into icp-app.
  • Introduces injected seams for WASM fetching, recipe resolution, network access, and telemetry observation.
  • Rewires CLI commands and dependencies to the new crate boundaries.
File summaries
File Description
Cargo.toml Registers icp-app.
Cargo.lock Updates workspace dependencies.
crates/icp/Cargo.toml Adds test utilities and trims dependencies.
crates/icp/src/agent.rs Retains lazy-agent abstraction.
crates/icp/src/canister/build/mod.rs Injects WASM fetching into builders.
crates/icp/src/canister/build/prebuilt.rs Uses the WASM fetch seam.
crates/icp/src/canister/recipe/mod.rs Defines recipe-resolution interfaces.
crates/icp/src/canister/sync/mod.rs Injects WASM fetching into sync.
crates/icp/src/canister/sync/plugin.rs Uses the WASM fetch seam.
crates/icp/src/canister/wasm.rs Replaces fetching implementation with a trait.
crates/icp/src/host.rs Adds WASM and observation seams.
crates/icp/src/lib.rs Removes application modules and exposes state constants.
crates/icp/src/manifest/mod.rs Exposes recipe definitions.
crates/icp/src/network/access.rs Retains network-access value types.
crates/icp/src/network/mod.rs Reduces networking to project-facing interfaces.
crates/icp/src/operations/build.rs Removes package-cache parameters.
crates/icp/src/operations/bundle.rs Uses injected WASM fetching.
crates/icp/src/operations/deploy.rs Removes direct package-cache dependency.
crates/icp/src/operations/mod.rs Removes application-specific operations.
crates/icp/src/operations/sync.rs Removes package-cache parameters.
crates/icp/src/prelude.rs Exposes the pinned mainnet root key.
crates/icp/src/project.rs Adapts deferred recipe caching.
crates/icp/src/store_artifact.rs Exposes artifact-store implementation and mock gating.
crates/icp/src/store_id.rs Exposes ID-store implementation and mocks.
crates/icp-app/Cargo.toml Defines the new application crate.
crates/icp-app/src/agent.rs Houses concrete agent creation.
crates/icp-app/src/context/init.rs Wires concrete application services.
crates/icp-app/src/context/mod.rs Moves CLI context and network directories.
crates/icp-app/src/context/tests.rs Updates context tests for split crates.
crates/icp-app/src/directories.rs Moves global directory management.
crates/icp-app/src/identity/delegation.rs Moves delegation support.
crates/icp-app/src/identity/key.rs Moves identity key management.
crates/icp-app/src/identity/keyring_mock.rs Moves keyring test support.
crates/icp-app/src/identity/manifest.rs Moves identity manifests.
crates/icp-app/src/identity/mod.rs Moves identity loading.
crates/icp-app/src/identity/seed/mod.rs Moves mnemonic key derivation.
crates/icp-app/src/identity/seed/slip10.rs Moves SLIP-0010 implementation.
crates/icp-app/src/lib.rs Defines the application-layer API.
crates/icp-app/src/network/accessor.rs Implements project network access.
crates/icp-app/src/network/config.rs Moves runtime network descriptors.
crates/icp-app/src/network/custom_domains.rs Moves custom-domain handling.
crates/icp-app/src/network/directory.rs Moves network state directories.
crates/icp-app/src/network/managed/cache.rs Moves launcher caching.
crates/icp-app/src/network/managed/docker.rs Moves Docker launcher support.
crates/icp-app/src/network/managed/launcher.rs Moves native launcher support.
crates/icp-app/src/network/managed/mod.rs Defines managed-network modules.
crates/icp-app/src/network/managed/run.rs Moves network lifecycle handling.
crates/icp-app/src/network/mod.rs Defines application networking.
crates/icp-app/src/network/resolve.rs Implements network endpoint resolution.
crates/icp-app/src/operations/canister_migration.rs Moves migration operations.
crates/icp-app/src/operations/mod.rs Defines application operations.
crates/icp-app/src/operations/snapshot_transfer.rs Moves snapshot transfers.
crates/icp-app/src/operations/token/allowance.rs Moves allowance queries.
crates/icp-app/src/operations/token/approve.rs Moves token approvals.
crates/icp-app/src/operations/token/balance.rs Moves balance queries.
crates/icp-app/src/operations/token/mint.rs Moves cycle minting.
crates/icp-app/src/operations/token/mod.rs Defines token operations.
crates/icp-app/src/operations/token/transfer.rs Moves token transfers.
crates/icp-app/src/package.rs Moves package-cache management.
crates/icp-app/src/recipe.rs Implements recipe fetching and caching.
crates/icp-app/src/settings.rs Moves user settings.
crates/icp-app/src/signed_message.rs Moves signed-message handling.
crates/icp-app/src/telemetry_data.rs Implements host observation for telemetry.
crates/icp-app/src/wasm.rs Implements HTTP-backed WASM fetching.
crates/icp-cli/Cargo.toml Adds the icp-app dependency.
crates/icp-cli/src/commands/args.rs Updates shared command imports.
crates/icp-cli/src/commands/build.rs Uses application context and injected cache services.
crates/icp-cli/src/commands/canister/call.rs Updates context and message imports.
crates/icp-cli/src/commands/canister/create.rs Updates context and identity imports.
crates/icp-cli/src/commands/canister/delete.rs Updates context imports.
crates/icp-cli/src/commands/canister/install.rs Updates context imports.
crates/icp-cli/src/commands/canister/link.rs Updates context imports.
crates/icp-cli/src/commands/canister/list.rs Updates context imports.
crates/icp-cli/src/commands/canister/logs.rs Updates context imports.
crates/icp-cli/src/commands/canister/metadata.rs Updates context imports.
crates/icp-cli/src/commands/canister/migrate_id.rs Uses migrated migration operations.
crates/icp-cli/src/commands/canister/settings/show.rs Updates context imports.
crates/icp-cli/src/commands/canister/settings/sync.rs Updates context imports.
crates/icp-cli/src/commands/canister/settings/update.rs Updates context imports.
crates/icp-cli/src/commands/canister/snapshot/create.rs Updates context imports.
crates/icp-cli/src/commands/canister/snapshot/delete.rs Updates context imports.
crates/icp-cli/src/commands/canister/snapshot/download.rs Uses migrated snapshot operations.
crates/icp-cli/src/commands/canister/snapshot/list.rs Updates context imports.
crates/icp-cli/src/commands/canister/snapshot/restore.rs Updates context imports.
crates/icp-cli/src/commands/canister/snapshot/upload.rs Uses migrated snapshot operations.
crates/icp-cli/src/commands/canister/start.rs Updates context imports.
crates/icp-cli/src/commands/canister/status.rs Updates context and identity imports.
crates/icp-cli/src/commands/canister/stop.rs Updates context imports.
crates/icp-cli/src/commands/canister/top_up.rs Uses migrated token types.
crates/icp-cli/src/commands/cycles/balance.rs Uses migrated token operations.
crates/icp-cli/src/commands/cycles/mint.rs Uses migrated mint operation.
crates/icp-cli/src/commands/cycles/transfer.rs Uses migrated transfer operation.
crates/icp-cli/src/commands/deploy.rs Uses application context and network directories.
crates/icp-cli/src/commands/environment/list.rs Updates context imports.
crates/icp-cli/src/commands/identity/account_id.rs Updates context imports.
crates/icp-cli/src/commands/identity/default.rs Uses migrated identity manifests.
crates/icp-cli/src/commands/identity/delegation/request.rs Uses migrated identity support.
crates/icp-cli/src/commands/identity/delegation/sign.rs Uses migrated delegation support.
crates/icp-cli/src/commands/identity/delegation/use.rs Uses migrated delegation support.
crates/icp-cli/src/commands/identity/delete.rs Uses migrated identity operations.
crates/icp-cli/src/commands/identity/export.rs Uses migrated identity operations.
crates/icp-cli/src/commands/identity/import.rs Uses migrated identity operations.
crates/icp-cli/src/commands/identity/link/hsm.rs Uses migrated HSM support.
crates/icp-cli/src/commands/identity/link/web.rs Uses migrated web identity support.
crates/icp-cli/src/commands/identity/list.rs Uses migrated identity manifests.
crates/icp-cli/src/commands/identity/new.rs Uses migrated key derivation.
crates/icp-cli/src/commands/identity/principal.rs Updates context imports.
crates/icp-cli/src/commands/identity/reauth.rs Uses migrated identity and settings APIs.
crates/icp-cli/src/commands/identity/rename.rs Uses migrated identity operations.
crates/icp-cli/src/commands/message/send.rs Uses migrated signed-message support.
crates/icp-cli/src/commands/network/args.rs Uses application network selections.
crates/icp-cli/src/commands/network/list.rs Updates context imports.
crates/icp-cli/src/commands/network/ping.rs Updates context and identity imports.
crates/icp-cli/src/commands/network/start.rs Uses application network runtime.
crates/icp-cli/src/commands/network/status.rs Uses dedicated network directories.
crates/icp-cli/src/commands/network/stop.rs Uses application network runtime.
crates/icp-cli/src/commands/network/update.rs Uses migrated launcher cache.
crates/icp-cli/src/commands/new.rs Updates context imports.
crates/icp-cli/src/commands/project/bundle.rs Supplies the injected WASM fetcher.
crates/icp-cli/src/commands/project/show.rs Updates context imports.
crates/icp-cli/src/commands/settings.rs Uses migrated settings APIs.
crates/icp-cli/src/commands/sync.rs Removes direct package-cache handling.
crates/icp-cli/src/commands/token/allowance.rs Uses migrated allowance operation.
crates/icp-cli/src/commands/token/approve.rs Uses migrated approval operation.
crates/icp-cli/src/commands/token/balance.rs Uses migrated balance operation.
crates/icp-cli/src/commands/token/transfer.rs Uses migrated transfer operation.
crates/icp-cli/src/complete.rs Uses application context and identities.
crates/icp-cli/src/dist.rs Uses migrated settings.
crates/icp-cli/src/main.rs Initializes and dispatches with icp-app.
crates/icp-cli/src/options.rs Updates identity and network selections.
crates/icp-cli/src/telemetry.rs Uses application telemetry and settings.
crates/icp-cli/tests/message_send_tests.rs Updates signed-message test imports.
Review details
  • Files reviewed: 123/132 changed files
  • Comments generated: 2
  • Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread crates/icp-app/Cargo.toml
Comment thread crates/icp/src/store_artifact.rs
@adamspofford-dfinity
adamspofford-dfinity force-pushed the spofford/factor-app branch 2 times, most recently from a30abaa to 561c209 Compare September 14, 2026 18:34
@adamspofford-dfinity
refactor: ask for wasm modules and observation rather than reaching f…
309e97e 
…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.
@adamspofford-dfinity
refactor: split the app half of icp into icp-app
6278c4e 
`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`.
@adamspofford-dfinity
fix: pass a boxed cause through instead of restating it
2428267 
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Balanced
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@adamspofford-dfinity