Skip to content

Fix round 6 audit: icp.yaml, deprecated APIs, stable-structures across 11 skills#8

Merged
JoshDFN merged 1 commit into
mainfrom
fix/round6-all-skills
Feb 26, 2026
Merged

Fix round 6 audit: icp.yaml, deprecated APIs, stable-structures across 11 skills#8
JoshDFN merged 1 commit into
mainfrom
fix/round6-all-skills

Conversation

@JoshDFN

@JoshDFN JoshDFN commented Feb 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Convert all remaining icp.json configs to icp.yaml (YAML format) for icp-cli compatibility
  • Fix deprecated ic-cdk 0.18 APIs: canister_balance128()canister_cycle_balance(), ic_cdk::id()canister_self(), api::management_canistermanagement_canister
  • Remove .expect() on ic-stable-structures 0.7 StableCell::init/set and StableLog::init (return Self/T not Result)
  • Fix --id-only flag → icp canister id across multi-canister, asset-canister, sns-launch
  • Fix --storage-mode--storage plaintext and icp identity defaulticp identity use in vetkd, internet-identity
  • Add missing transient markers on actor refs in vetkd
  • Correct stable memory limit from "4GB" to "hundreds of GB" in multi-canister

Context

Round 6 multi-agent verification (6 parallel agents, 2 skills each). 11 of 12 skills had findings; icrc-ledger was the only CLEAN skill.

stable-memory: Remove .expect() on StableCell::init/set and StableLog::init (v0.7 returns Self/T not Result)
ckbtc: ic_cdk::id() → ic_cdk::api::id(), icp.json → icp.yaml, getBalanceUpdate → getBalance
evm-rpc: icp.json → icp.yaml, fix deprecated import path, correct Cycles.add wording for mo:core
internet-identity: icp.json → icp.yaml, icp identity default → icp identity use
asset-canister: icp.json → icp.yaml, --id-only → icp canister id
multi-canister: icp.json → icp.yaml throughout, --id-only → icp canister id, fix 4GB stable memory claim
https-outcalls: api::management_canister → management_canister, ic_cdk::id() → ic_cdk::api::canister_self()
vetkd: --storage-mode → --storage, add transient to actor refs, icp identity default → icp identity use
certified-variables: ic_cdk::set_certified_data → ic_cdk::api::set_certified_data, same for data_certificate
sns-launch: --id-only → icp canister id
wallet: canister_balance128() → canister_cycle_balance(), ic_cdk::api::id() → ic_cdk::api::canister_self()
@JoshDFN JoshDFN merged commit 7752088 into main Feb 26, 2026
3 checks passed
@marc0olo marc0olo deleted the fix/round6-all-skills branch February 27, 2026 14:47
marc0olo added a commit that referenced this pull request Apr 2, 2026
…vals

Addresses #135 (Obs 2, 5, 6, 7), #136 (F2), #139, #141.

SKILL.md:
- Add pitfall #8: networks/environments must be YAML arrays, not maps
- Add pitfall #13: { agent } → { agentOptions } silent anonymous identity
- Add pitfall #15: mops.toml placement for inline vs path-based canisters
- Renumber pitfalls 8→16

binding-generation.md:
- Add createActor warning above code example ({ agentOptions } not { agent })
- Add opt T → T | null section (bindgen wrapper unwraps arrays)

dfx-migration.md:
- Add safeGetCanisterEnv before/after migration example
- Add createActor breaking API change with before/after code
- Add custom Motoko builds section (mops toolchain bin moc)
- Add frontend migration checklist items 11-14

evaluations/icp-cli.json:
- Add createActor adversarial eval (3/3 with skill, 0/3 without)
- Add opt T representation eval
marc0olo added a commit that referenced this pull request Apr 2, 2026
…vals (#144)

Addresses #135 (Obs 2, 5, 6, 7), #136 (F2), #139, #141.

SKILL.md:
- Add pitfall #8: networks/environments must be YAML arrays, not maps
- Add pitfall #13: { agent } → { agentOptions } silent anonymous identity
- Add pitfall #15: mops.toml placement for inline vs path-based canisters
- Renumber pitfalls 8→16

binding-generation.md:
- Add createActor warning above code example ({ agentOptions } not { agent })
- Add opt T → T | null section (bindgen wrapper unwraps arrays)

dfx-migration.md:
- Add safeGetCanisterEnv before/after migration example
- Add createActor breaking API change with before/after code
- Add custom Motoko builds section (mops toolchain bin moc)
- Add frontend migration checklist items 11-14

evaluations/icp-cli.json:
- Add createActor adversarial eval (3/3 with skill, 0/3 without)
- Add opt T representation eval
marc0olo added a commit that referenced this pull request May 30, 2026
- CLAUDE.md: clarify 'Body content' row in upstream table (not icskills-owned)
- CLAUDE.md: scope PR eval requirement to new skills only (line 67)
- CLAUDE.md: fix Project Structure paths (already committed, included here)
- improve-ic-skill: add #8-submit-a-pr anchor to CONTRIBUTING.md link
- CONTRIBUTING.md: add improve-ic-skill and branch naming note to sync section
- PATCHES.md: clarify upstream commit date vs vendored date labels
raymondk added a commit that referenced this pull request Jun 1, 2026
…utor workflow (#198)

* docs: recommend skill-creator for new skill drafting

Replace the template-copy workflow with the Anthropic skill-creator
skill as the recommended starting point. Add explicit callouts for the
IC-specific metadata block (title, category) that skill-creator does not
produce, and clarify the two-phase eval workflow: skill-creator's
internal loop for iterative drafting vs. the committed evaluations/
file required for PRs.

Also remove prescriptive body section recommendations from CLAUDE.md —
structure is individual to each skill and better left to skill-creator.

* docs: extend skill-creator guidance to cover skill improvements

* docs: require eval review and testrun before any skill PR

* docs: fix inconsistencies in contributing guide and agent instructions

* docs: clarify IC evals are kept as regression safety net

* feat: install skill-creator and fix remaining doc inconsistencies

Install skill-creator as a project skill via `npx skills add` so it is
auto-discovered by Claude Code without manual loading. Files land at
.agents/skills/skill-creator/ (multi-agent canonical location) with a
symlink at .claude/skills/skill-creator for Claude Code.

Doc fixes:
- Update skill-creator references to reflect it is pre-installed
- Step 3 renamed "Review and finalize" to avoid implying manual authoring
- Step 6 leads with porting evals from skill-creator's evals.json
- "Keep it flat" bullet now correctly allows references/ subdirectory
- "see step 5 above" cross-reference replaced with anchor link
- CLAUDE.md: add three-file instruction for adding a new category
- CLAUDE.md: mark _template/ as legacy in Project Structure

* fix: correct upstream diff direction in CONTRIBUTING.md (introduced by #194)

* chore: remove legacy skill template; use skill-creator to draft new skills

* feat: add improve-ic-skill, patch skill-creator bugs, remove skills-lock

- Add internal improve-ic-skill skill (.agents/skills/improve-ic-skill/)
  for token-efficient improvement of existing skills. Uses our toolchain
  (npm run validate, evaluate-skills.js with targeted flags), knows eval
  location (evaluations/<skill-name>.json), handles upstream-tracked skills,
  and seeds evals when none exist. Distinct from skill-creator which is for
  new skill creation only.

- Fix 4 confirmed bugs in vendored skill-creator (documented in PATCHES.md):
  1. generate_review.py: escape </script> in JSON output to prevent viewer breakage
  2. SKILL.md: add missing run-1/ level in output paths (aggregate_benchmark.py requires it)
  3. SKILL.md: require eval-0-descriptive-name/ format (aggregator globs eval-*)
  4. SKILL.md: clarify --static is wrong for Claude Code; use server mode instead

- Remove skills-lock.json from git and add to .gitignore. Our skill-creator
  copy is a patched fork — npx skills add would overwrite fixes silently.
  Intentional updates must re-apply patches from PATCHES.md.

- Update CLAUDE.md: distinguish improve-ic-skill vs skill-creator, warn
  against npx skills add updates, clarify that PR eval results must come
  from evaluate-skills.js (with-skill vs baseline), not skill-creator internals.

- Add gitignore entries for skill-creator artifacts (skills/*-workspace/,
  **/__pycache__/, skills-lock.json).

* docs: add targeted eval guidance to upstream sync checklist

* feat(improve-ic-skill): require explicit problem statement before starting any work

* docs(skill-creator): add upstream commit SHA and install date to PATCHES.md

* fix(skill-creator): replace PyYAML with stdlib parser in quick_validate.py (Patch 5)

* fix(skill-creator): add explicit field name warning to grader.md (text/passed/evidence)

* fix: address all review findings — improve-ic-skill, CONTRIBUTING.md, CLAUDE.md, PATCHES.md

- CONTRIBUTING.md: route improvements to improve-ic-skill (not skill-creator);
  fix nonexistent skills/<name>/evals/evals.json path reference
- improve-ic-skill: remove redundant eval run from Step 8 (Step 7 covers it);
  add upstream sync guidance for seeding evals from the diff when none exist
- CLAUDE.md: add improve-ic-skill mention in upstream sync workflow; align
  sync checklist eval policy with general improvement policy
- PATCHES.md: reorder patches 4 and 5 into sequential order

* fix: correct diff notation, clarify Cowork in PATCHES.md, improve Step 8 PR link

* fix: resolve all remaining review findings

- CONTRIBUTING.md: rename 'That's it' step to 'No site edits needed'
- CLAUDE.md: remove redundant eval command from Workflow section; point to
  Evaluations section for the full command reference
- CLAUDE.md: fix stale Project Structure (src/data/ → src/lib/, remove
  non-existent SiteLayout, components/*)
- CLAUDE.md: clarify branch naming — <skill-name> is the IC skill name;
  document combined-branch pattern for multi-skill syncs

* fix: final consistency pass — all third-review findings addressed

- CLAUDE.md: clarify 'Body content' row in upstream table (not icskills-owned)
- CLAUDE.md: scope PR eval requirement to new skills only (line 67)
- CLAUDE.md: fix Project Structure paths (already committed, included here)
- improve-ic-skill: add #8-submit-a-pr anchor to CONTRIBUTING.md link
- CONTRIBUTING.md: add improve-ic-skill and branch naming note to sync section
- PATCHES.md: clarify upstream commit date vs vendored date labels

* fix: clarify branch naming placeholder and PATCHES.md commit SHA label

* feat(improve-ic-skill): hold the line — do not proceed without a clear problem statement

* fix(improve-ic-skill): hard stop in Step 0 — no work without specific problem, no exceptions for pushback

* feat(icp-cli): add instructions for running parallel pocket-ics in worktrees (#200)

---------

Co-authored-by: raymondk <raymond.khalife@dfinity.org>
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.

1 participant