Skip to content

feat(assets-sync): warn on unmatched config rules; adopt stderr-persistent plugin output#51

Merged
lwshang merged 3 commits into
mainfrom
lwshang/SDK-2714-warn-on-unmatched-config-rules
May 19, 2026
Merged

feat(assets-sync): warn on unmatched config rules; adopt stderr-persistent plugin output#51
lwshang merged 3 commits into
mainfrom
lwshang/SDK-2714-warn-on-unmatched-config-rules

Conversation

@lwshang
Copy link
Copy Markdown
Collaborator

@lwshang lwshang commented May 15, 2026

Summary

  • Warn on unmatched config rules (the original SDK-2714 deliverable): .ic-assets.json[5] rules whose glob never matched any asset are reported to the user, mirroring ic-asset's get_unused_configs().
  • Migrate the sync plugin to the new stdout/stderr contract introduced by dfinity/icp-cli#556: the plugin's stderr is now treated as persistent output displayed after the sync step completes, stdout as transient progress.
    • WIT change: exec returns result<_, string> — the option<string> summary payload is gone.
    • Plugin eprintln!s its end-of-run summary; warnings from assets-sync (e.g. unmatched-config-rule patterns) are eprintln!ed inline at the point of detection.
    • plugin/wit/sync-plugin.wit is now byte-identical to the copy in icp-cli.

Release coordination

This PR requires dfinity/icp-cli#556.

CI on this repo installs the latest released icp-cli for the integration tests, so the expected order is:

  1. Land feat(sync-plugin): persist plugin stderr output after a sync step icp-cli#556.
  2. Cut a new icp-cli release that includes it.
  3. Re-run CI here — the integration tests will then resolve against the new release and pass.

Until that release is out, CI on this PR is expected to fail at the integration-test stage.

Test plan

  • cargo check --workspace --all-targets is clean
  • cargo test -p assets-sync — 71 unit tests pass (3 scan-level warning-Vec tests replaced by focused get_unused_configs tests in config.rs)
  • After the new icp-cli release is out: re-run CI here and confirm integration tests pass
  • End-to-end run against the new icp-cli: deploy a project containing an .ic-assets.json rule with a typo'd pattern, verify the warning shows up under the canister name after the step completes
  • End-to-end: a successful sync prints the summary line persistently (not lost to the rolling buffer)

🤖 Generated with Claude Code

lwshang and others added 3 commits May 14, 2026 14:02
Track which AssetConfigRule glob patterns matched at least one asset
during scanning and emit warnings for unused ones, so users are alerted
to typos in their .ic-assets.json[5] files.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The icp-cli sync-plugin runtime now treats plugin stdout as transient
progress and stderr as persistent output displayed after the step
completes. Update the WIT contract to match (`exec` returns
`result<_, string>` — no more `option<string>` payload), have the
plugin `eprintln!` its summary, and emit unmatched-config-rule
warnings inline from scan.rs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR completes the SDK-2714 deliverable by adding "unmatched config rule" warnings, and migrates the sync plugin to the new icp-cli stdout/stderr contract (where stderr is persisted past the rolling progress view). The WIT export exec no longer returns an optional summary; the plugin now emits its summary and warnings via eprintln!.

Changes:

  • Add AssetSourceDirectoryConfiguration::get_unused_configs() and is_own_for_dir(), capturing the original glob pattern on each rule, and emit per-directory unmatched-rule warnings to stderr from scan::walk.
  • Update WIT signature exec from result<option<string>, string> to result<_, string> and have the plugin eprintln! the summary instead of returning it.
  • Update plugin docs/WIT comments to describe the new stdout-transient / stderr-persistent contract, and tick the README checklist item.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
plugin/wit/sync-plugin.wit Document new stdout/stderr semantics; drop optional summary payload from exec return type.
plugin/src/lib.rs Switch Guest::exec to Result<(), String> and route the summary through eprintln!.
plugin/README.md Mark the "warn on unmatched config rules" checklist item done.
assets-sync/src/config.rs Store rule's original pattern, expose get_unused_configs / is_own_for_dir, drop dead_code allow on origin, add focused unit tests.
assets-sync/src/scan.rs After each owned directory walk, eprintln! warnings from get_unused_configs; replace removed Vec-based test with a smoke test; introduce scan_sources alias.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lwshang lwshang marked this pull request as ready for review May 18, 2026 22:24
@lwshang lwshang requested a review from a team as a code owner May 18, 2026 22:24
@lwshang lwshang merged commit 790d421 into main May 19, 2026
21 of 25 checks passed
@lwshang lwshang deleted the lwshang/SDK-2714-warn-on-unmatched-config-rules branch May 19, 2026 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants