Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .claude/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ cargo fmt && cargo clippy # Run after changes pass tests
- **`crates/icp-cli`**: Main CLI binary (`icp`) with command implementations
- **`crates/icp`**: Core library with project model, manifest loading, canister management, network configuration
- **`crates/icp-canister-interfaces`**: Canister interface definitions for ICP system canisters
- **`crates/icp-events`**: Progress and user-facing notices as data (`Event`, `Reporter`, `Task`, `OutputWriter`, `EventSink`), so operations can report without depending on the terminal. serde + futures only
- **`crates/schema-gen`**: JSON schema generation for manifest validation

### Command Structure
Expand Down
50 changes: 50 additions & 0 deletions .claude/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,56 @@ These constants are defined in `crates/icp/src/prelude.rs` as `LOCAL` and `IC` a

Store management is in `crates/icp/src/store_id.rs`.

## Progress & User-Facing Output

Operations in `crates/icp-cli/src/operations/` report progress as data, not as terminal
calls. `crates/icp-events` defines the vocabulary (`Event`, `Reporter`, `Task`,
`OutputWriter`, `EventSink`, `CancelToken`) and depends only on serde and futures — never on
`icp`, an async runtime, or anything terminal-shaped. `crates/icp-cli/src/events.rs` holds
`IndicatifSink`, the only place that maps events onto `indicatif` bars, and the styles they
are drawn in.

- Operations take a `&Reporter`, never a `debug: bool`. Callers build one per operation with
`events::indicatif_reporter(ctx.debug)`. The multi-canister operations
(`build_many`, `sync_many`, `create_bundle`) still take one bool, `all_step_output`: it
decides how much of a failure is replayed, not how anything is drawn.
- Nothing outside `events.rs` imports `indicatif`, with two exceptions that never went
through the shared renderer and build their own one-off spinners:
`commands/canister/migrate_id.rs` and `commands/identity/link/web.rs`. Everything else
reports events. To check that this still holds:

```bash
grep -rl 'use indicatif' crates/icp-cli/src crates/icp/src
```

- A library that produces output lines — a subprocess, a sync plugin — is handed an
`OutputWriter` rather than a channel. Each line becomes an `Event::StepOutput` and is kept
in the task's step log, capped at `MAX_RECORDED_LINES_PER_STEP`, so an operation can replay
the failing step after the bars are down. `operations/step_replay.rs` formats that replay;
read the steps back with `Task::recorded_steps` *before* finishing the task, since
finishing consumes it.
- The event model is deliberately not semver-stable: `publish = false`, `0.x`, all enums
`#[non_exhaustive]`, `TaskKind` closed.
- Events do not drive `--json`. `--json` means the command's final result; progress never
appears in it.
- `tracing` at INFO level is product output here, not logging — `logging.rs` installs a
`UserLayer` that prints `Level::INFO` to stderr unprefixed. `Event::Notice` is the event
model's equivalent; the `info!`/`warn!`/`error!` calls inside `operations/` have not been
converted yet.
- Under `--debug` the bars are hidden, so the `debug!` line the sink logs for each
`Event::StepOutput` is the only thing tying that line to the canister that printed it —
and canisters build in parallel, interleaving their output. `BarState` therefore keeps the
prefix it gave the bar, and the log line reuses it, so both paths name a canister the same
way. Anything else that has to name a task should read that prefix rather than the bar.
- A bar has to be fully styled and labelled before it is shown, and a spinner before
`enable_steady_tick`: that call spawns a thread which draws immediately, so anything set
afterwards races the first frame.

Operations are unit-tested by running them against `RecordingSink` and asserting on the
resulting `Vec<Event>`; see `operations/test_support.rs`. `events.rs::rendering`
additionally pins the frames `IndicatifSink` draws against the literal output of the
renderer it replaced, captured before that renderer was deleted.

## Telemetry

Anonymous usage telemetry implementation. User-facing documentation is in `docs/telemetry.md`.
Expand Down
5 changes: 5 additions & 0 deletions .claude/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ Tests are split between unit tests (in modules) and integration tests:
- Use `assert_cmd` for CLI assertions and `predicates` for output matching
- Use `serial_test` with file locks for tests that share resources (network ports)
- Some tests launch local networks and require available ports
- Only one process-global `tracing` subscriber may be installed per test binary. In `icp-cli`'s
unit tests that is `events.rs::tests::captured_debug_lines`, which captures `debug!` lines;
installing a second one panics it. It has to be global rather than thread-local because
`tracing` decides whether a callsite is enabled the first time any thread reaches it and
caches that answer for the process.

## Mock Helpers

Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ bump. Currently experimental: project bundling, project dependencies
* feat: `script` build steps now receive `ICP_CLI_ENVIRONMENT`, the name of the environment the canisters are being built for, so a build can vary by environment the way a sync step already could.
* feat: `icp completions <SHELL>` prints a shell completion script for `bash`, `zsh`, `fish`, `powershell`, or `elvish` to stdout. See the [installation guide](docs/guides/installation.md#shell-completions) for where to put it.
* fix: `icp canister logs` output formats are corrected. `--json` now emits machine-readable JSON and the default emits the human-readable lines (the two were swapped), and `--follow --json` emits newline-delimited JSON, one record per line, streamed as each record arrives. This is breaking for scripts: parsing the default output as JSON now requires `--json`, and consumers of `--follow --json` must read one JSON object per line.
* fix: under `--debug`, each line of a build or sync step's output is now prefixed with the canister that produced it (`[canister-a] compiling`), the way the progress bars label it without `--debug`. Previously the lines were unattributed, so canisters built in parallel interleaved into a stream you could not read.

# v1.3.0

Expand Down
48 changes: 30 additions & 18 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ ic-management-canister-types = { version = "0.8.0" }
ic-utils = { version = "0.49.1" }
icp = { path = "crates/icp" }
icp-canister-interfaces = { path = "crates/icp-canister-interfaces" }
icp-events = { path = "crates/icp-events" }
icp-sync-plugin = { path = "crates/icp-sync-plugin" }
ic-identity-hsm = "0.49.1"
icrc-ledger-types = "0.1.10"
Expand Down
1 change: 1 addition & 0 deletions crates/icp-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ ic-ledger-types.workspace = true
ic-management-canister-types.workspace = true
ic-utils.workspace = true
icp-canister-interfaces.workspace = true
icp-events.workspace = true
icp = { workspace = true, features = ["clap"] }
icrc-ledger-types.workspace = true
indicatif.workspace = true
Expand Down
6 changes: 4 additions & 2 deletions crates/icp-cli/src/commands/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use icp::context::{Context, EnvironmentSelection};
use tracing::info;

use crate::{
operations::build::build_many_with_progress_bar,
events::indicatif_reporter,
operations::build::build_many,
options::{EnvironmentOpt, arg_struct_change_help},
};

Expand Down Expand Up @@ -57,12 +58,13 @@ pub(crate) async fn exec(ctx: &Context, args: &BuildArgs) -> Result<(), anyhow::
// Build the selected canisters
info!("Building canisters:");

build_many_with_progress_bar(
build_many(
canisters_to_build,
environment_selection.name(),
ctx.builder.clone(),
ctx.artifacts.clone(),
&ctx.dirs.package_cache()?,
&indicatif_reporter(ctx.debug),
ctx.debug,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

These two adjacent arguments are both derived from ctx.debug but mean opposite things — hide the bars vs. replay more output on failure. Same shape in deploy.rs and commands/sync.rs.

The rename to all_step_output helps at the definition; the call site still reads as ctx.debug twice, which is easy to get wrong when someone edits this later.

)
.await?;
Expand Down
Loading
Loading