You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Moves the skills' @icp-sdk/core anchor from 5.x to 6.x. The anchor sits wherever @icp-sdk/auth is, because almost every dapp authenticates — and auth has moved.
Refs #364; does not close it. wallet-integration is still on the old stack and is rewritten in #401, which closes the issue.
Why now
Every package the skills install agrees on core ^6: @icp-sdk/auth@10, @icp-sdk/signer@6, @icp-sdk/canisters@4, @dfinity/utils@5, and @icp-sdk/vetkeys@0.7 (^5 || ^6). Verified by install, peers honoured:
internet-identity Prerequisites paired an open-ended @icp-sdk/auth (>= 9.0.0) with @icp-sdk/core (>= 5.3.0). That floor admits auth 10, which peers core ^6, so the advertised combination does not install:
npm error Found: @icp-sdk/core@5.4.0
npm error peer @icp-sdk/core@"^6" from @icp-sdk/auth@10.0.0
Both majors are now pinned together, with a pitfall for the mismatch. Everything else is a version bump.
Verified, not assumed
auth 9 → 10 is a pure peer bump.diff -rq over the whole dist/esm type surface: no differences. The v10 guide agrees ("This package's own API is unchanged"). So the II flow needed no code changes.
bindgen 0.4.0 output is core-6 clean. Bindings generated from a .did exercising variant/record/blob/opt/principal, typechecked against core 6.1.0 under strict with skipLibCheck: false.
canisters 4.0.0 changed nothing we document. The only changed file in its hand-written surface is nns/types/governance_converters.d.ts; the rest is regenerated declarations. AssetManager and both ledger clients are untouched.
vetkeys 0.5 → 0.7 is API-identical. The >= 0.7 floor is load-bearing for a different reason: 0.5/0.6 carried core as a plain dependency, so an app on core 6 got 6.1.0 at the root and 5.4.0 nested under vetkeys. 0.7 peers it.
Two claims inherited from main are also corrected: @icp-sdk/core does have stable 4.x releases (so "starts at 5.x, no 0.x or 1.x" was false — dropped, since an agent needs the reason to pin, not version history), and --legacy-peer-deps does not duplicate core on a peer conflict — it skips the check and installs the mismatched pair. The nested-copy wording is kept only where it is accurate, on vetkeys.
Not in scope
wallet-integration is left to #401 to avoid a conflict on the same file, and is currently self-consistent. caffeine-app and certified-variables are handled separately per maintainer. The false { agent } / createActor claim in binding-generation.md and dfx-migration.md belongs with #156, since retracting it also means changing an eval that enforces it.
That PR pinned everything to ^5. Carried over: the bindgen floor, the correction that bindgen does not depend on core, and the vetkeys 0.7 bump.
Eval results — added/changed cases, with baseline
internet-identity 26 (new) WITH 4/4 | WITHOUT 2/4
icp-cli 6 (changed) WITH 6/6 | WITHOUT 0/6
icp-cli 15 (changed) WITH 6/6 | WITHOUT 4/6
The new II case puts a core-^5 project in front of an @icp-sdk/auth install; the baseline never mentions the peer conflict and "sidesteps the issue entirely by suggesting a different, unrelated package".
Regression check — internet-identity 20, the one existing case whose subject I touched: 3/4 with the change, 3/4 against origin/main's content (control run). Not a regression. Its failing behaviour varies run to run — the model invents a specific session default rather than saying unset falls back to II's own. Pre-existing gap, not addressed here.
icp-cli 15 scored 5/6 on an earlier run, failing on the candid: / didFile: confusion of #367. It did not reproduce on the latest run, which fits that issue's non-determinism. #367 stays open.
Pin core to ^6 in both install paths and state why: auth, signer,
canisters (>= 4) and @dfinity/utils (>= 5) all peer ^6, so a project on
another major fails with ERESOLVE, and --legacy-peer-deps papers over it
by installing two copies of core.
Also corrects the claim that @icp-sdk/bindgen depends on @icp-sdk/core.
Its only dependency is commander; the generated code imports core, which
the project installs itself.
The Prerequisites line paired an open-ended "auth >= 9.0.0" with
"core >= 5.3.0". auth 10 peers @icp-sdk/core@^6, so that floor resolves
to a combination that does not install:
npm error Found: @icp-sdk/core@5.4.0
npm error peer @icp-sdk/core@"^6" from @icp-sdk/auth@10.0.0
Pin both majors together and add a pitfall for the mismatch. auth 10's
API is identical to 9's -- only the peer moved -- so the flow above is
unchanged and the Older API notes now record 9.x as the core-5 option.
0.5 and 0.6 declared @icp-sdk/core as a plain dependency, so they could
nest a second copy of core beside the app's own rather than failing.
0.7 peers it as ^5 || ^6. The type surface is identical across 0.5-0.7,
so no code sample changes.
Also moves the vetkeys frontend core requirement from ^5.4 to ^6: the
^5.4 floor came from 0.5.0's dependency pin and no longer applies.
… core ^6
@icp-sdk/canisters 4.0.0 and @dfinity/utils 5.0.0 peer @icp-sdk/core@^6,
so the legacy asset-canister upload path no longer has to stay on core 5.
Of the hand-written API surface, canisters 4 changed only the NNS
governance converters -- AssetManager is untouched, and the sample here
typechecks unchanged against core 6.1.0.
Records the core-5 fallback (canisters ^3.6) for projects not yet moved.
icp-cli 6 and 15: bindgen floor -> >= 0.4.0, core -> an explicit ^6 pin.
The core expectation now asserts "pinned" rather than a floor, so it
tests the durable behaviour instead of a version snapshot.
internet-identity 26 (new): a project pinned to core ^5 asking to add
sign-in. @icp-sdk/auth@latest is 10.x and peers core ^6, so the install
fails; the case checks the conflict is named and resolved without
--legacy-peer-deps.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🟡 Changes recommended
A few updated docs contain verifiably incorrect statements about @icp-sdk/core version history and --legacy-peer-deps behavior that should be corrected before merging.
Get a fresh assessment by requesting another Copilot review.
Updates the IC Skills documentation and evaluation fixtures to align the JavaScript/TypeScript ecosystem guidance on @icp-sdk/core^6 (and related package majors), primarily by anchoring version pins around @icp-sdk/auth and updating bindgen/canisters/vetkeys references accordingly.
Changes:
Move skill guidance and prerequisite pins from core 5.x → core ^6 (and corresponding package majors like @icp-sdk/auth@^10, @icp-sdk/canisters@^4, @icp-sdk/bindgen@^0.4.0).
Update vetkeys/encrypted-maps docs to recommend @icp-sdk/vetkeys0.7+ and explain the peer/dependency rationale.
Update evaluation cases/expectations to enforce the new version-pinning guidance and peer-compatibility warnings.
File
Description
skills/vetkeys/SKILL.md
Bumps frontend vetkeys guidance to 0.7+ and switches the frontend core pin to ^6.
Both claims were wrong, per Copilot review on #400:
- "@icp-sdk/core starts at 5.x; there is no 0.x or 1.x release" is false.
The registry has 12 stable 4.x releases (4.0.0-4.2.3) plus 1.x betas,
which is also why @dfinity/oisy-wallet-signer@6 peers core ^4. The
claim is dropped rather than corrected: an agent does not need core's
version history, only the reason to pin ^6.
- "--legacy-peer-deps installs two copies of core" is wrong for a PEER
conflict. Verified: core@^5 + auth@^10 under that flag installs ONE
core (5.4.0) beside auth 10.0.0 -- an incompatible pair, not a
duplicate. It skips the peer check, so the mismatch surfaces at
runtime instead of at install time.
The nested-second-copy wording stays where it is accurate: vetkeys
0.5/0.6 carried core as a plain dependency, and an app on core 6 does
end up with 6.1.0 at the root and 5.4.0 nested under vetkeys.
Evals re-run after dropping the vestigial 0.x/1.x expectation:
icp-cli 6 WITH 6/6 | WITHOUT 0/6, icp-cli 15 WITH 6/6 | WITHOUT 4/6.
Same error as the one Copilot caught on #400: for a PEER conflict the
flag does not install two copies of core. Verified -- core@^5 + auth@^10
under that flag installs one core (5.4.0) beside auth 10.0.0, an
incompatible pair. It skips the check, so the mismatch shows up at
runtime rather than at install time.
All three findings were valid and are fixed in a3ffa71. Both were my errors, and I verified each against the registry rather than reasoning about it.
1. The core release-history claim was false.@icp-sdk/core has 12 stable 4.x releases (4.0.0–4.2.3) plus 1.x betas:
major 1: 6 versions (1.0.0-beta.0 .. 1.0.0-beta.5)
major 4: 12 versions (4.0.0 .. 4.2.3)
major 5: 11 versions
major 6: 2 versions
That is also why @dfinity/oisy-wallet-signer@6.0.0 peers @icp-sdk/core@^4 — core 4 is real, and I had a package in front of me depending on it while the text said it didn't exist. I took the suggestion to drop the claim rather than correct it: an agent doesn't need core's version history, only the reason to pin ^6. (The claim predates this PR; I carried it forward and repeated it, so it's fair game here.)
2. --legacy-peer-deps does not install two copies for a peer conflict. Verified directly:
One core, incompatible pair. Reworded to: it skips the peer check and installs the mismatched pair anyway, so the incompatibility surfaces at runtime instead of at install time.
One refinement on that — the "nested second copy" wording is accurate elsewhere and is deliberately kept, because it describes a different mechanism:
Outcome
Plain dependency mismatch
npm nests a second copy — silent duplication
Peer conflict + --legacy-peer-deps
one copy, incompatible pair — silent incompatibility
The vetkeys note is the first case, not the second, and it reproduces:
I had blurred the two mechanisms into one sentence; they're now stated separately where each applies.
Also dropped the vestigial "does NOT suggest a 0.x or 1.x version" clause from evaluations/icp-cli.json cases 6 and 15, since it enforced the framing being removed. Both re-run with baseline: icp-cli 6 — WITH 6/6 | WITHOUT 0/6, icp-cli 15 — WITH 6/6 | WITHOUT 4/6 (case 15's known #367 failure did not reproduce this run, which fits its non-deterministic behaviour).
The same --legacy-peer-deps error appeared in #401's wallet-integration rewrite and is fixed there too.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
There are a couple of documentation consistency issues (notably bindgen install scope and a core-version requirement phrasing) that should be corrected to avoid misleading guidance.
Step 2 installs @icp-sdk/bindgen as a regular dependency, but the binding-generation reference in this skill installs bindgen as a dev dependency (npm install -D ...). Since bindgen is a build-time tool, keeping it in devDependencies avoids shipping it in production bundles and keeps the docs consistent.
Clarify core version recommendation for vetKeys
skills/vetkeys/SKILL.md:27
This line reads like core ^6 is a hard requirement for the vetKeys frontend, but two lines above you state @icp-sdk/vetkeys@0.7 peers core as ^5 || ^6. Consider rewording this to present core ^6 as the recommended repo-wide anchor (and acknowledge core ^5 works if the app is still on the older SDK stack), to avoid an internal contradiction.
…s' core floor
Two findings Copilot had suppressed on the first pass of #400:
- dfx-migration step 2 installed @icp-sdk/bindgen as a regular
dependency while binding-generation.md uses -D. bindgen is build-time
only (CLI + Vite plugin), so devDependencies is right and the two
references now agree.
- vetkeys listed "@icp-sdk/core ^6" under "Also required", two lines
below the note that 0.7 peers core as ^5 || ^6. Reworded so ^6 reads
as the repo-wide anchor rather than a vetKeys requirement.
No evals cover either line, so none were re-run.
The latest Copilot review lists no open findings and confirms the three earlier threads as resolved, but it carries two items under Previously missed ("in code that hasn't changed since last review"). Both are valid and both are fixed in the latest commit. They have no review threads attached, so there is nothing to resolve — recording them here instead.
1. dfx-migration.md:63 — bindgen belongs in devDependencies. Correct, and it was an internal inconsistency: binding-generation.md:86 already uses npm install -D, while the migration step installed it as a regular dependency. bindgen is build-time only (a CLI plus a Vite plugin), so it has no business in a production bundle. Step 2 now reads:
2. vetkeys/SKILL.md:27 — core ^6 read as a vetKeys requirement. Also correct, and a contradiction two lines apart: the note above says 0.7 peers core as ^5 || ^6, then "Also required" listed @icp-sdk/core ^6. vetKeys is the one package in this set that spans both majors, so the distinction is worth keeping rather than flattening. Now:
frontend @icp-sdk/core ^6 — vetKeys itself accepts ^5 || ^6, but the rest of the SDK peers ^6.
That keeps ^6 as the repo-wide anchor (the point of this PR) without claiming vetKeys demands it — an agent on a core-5 project should not conclude it must upgrade core before it can use vetKeys.
No evals cover either line — icp-cli case 3 ("Migrate from dfx") tests recipes, identity migration and canister-ID mapping, not the install step, and no vetkeys case touches the compatibility line — so none were re-run. The PR body is updated with both fixes.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
There is at least one documentation typo in the updated version spec (@icp-sdk/core ^6 vs @icp-sdk/core@^6) that could mislead users copying the guidance.
Review effort: Lite Findings: None
Previously missed (1)
In code that hasn't changed since last review
Fix invalid @icp-sdk/core version syntax
skills/vetkeys/SKILL.md:27
The frontend core version is written as @icp-sdk/core ^6, which is not a valid npm package@version spec and is inconsistent with the rest of the docs (e.g., @icp-sdk/core@^6). This is likely to confuse users copying the constraint.
The frontend requirement was written `@icp-sdk/core ^6` with a space,
which is not a valid npm package@version spec and was the only such
form left in skills/. Everywhere else in this PR uses the @ form, and
an agent assembling an install command may splice the string verbatim.
Third review pass: no open findings, one item under Previously missed (vetkeys/SKILL.md:27). No thread attached, so recording here.
Valid, and fixed. The frontend requirement read @icp-sdk/core ^6 with a space. Marginal — it sits in a prose sentence that also carries ic-cdk = "0.20", moc ≥ 1.13.0 and core ≥ 2.6.1, so nobody was going to npm install that line wholesale. But it was the only space-separated form left anywhere under skills/:
Since these files are written for agents, and an agent assembling an install command may splice a version spec verbatim, the canonical @icp-sdk/core@^6 is worth the one character. Done.
No eval covers that line, so nothing re-run. PR body updated.
Rewrites `wallet-integration` on `@icp-sdk/signer` and narrows it to
**integrating** a signer. `@dfinity/oisy-wallet-signer` is dropped.
Closes#364 — the last skill still anchored to an incompatible
`@icp-sdk/core` major. Install lines assume the `^6` baseline from #400;
the two PRs touch disjoint files and merge in either order, but #400
first avoids a window where this skill says `^6` and its neighbours say
`^5`.
Reviewed by @sea-snake, who maintains `@icp-sdk/signer` — his changes
are in, across two rounds. The second round: `SignerAgent` is now
recommended over calling `callCanister` directly rather than left to
inference, and the redirect example's URL no longer implies ICRC-167
dictates a path (`/sign`, not `/icrc-167`).
Pitfall 9 also now covers what declaring a `callbackUrl` does *not* buy
you: the wallet reads `/.well-known/ii-auth-callbacks` cross-origin, so
the document needs a JSON response and CORS headers or a correctly
listed callback still fails validation. Raised by Copilot, confirmed by
the maintainer, and documented the same way the **internet-identity**
skill documents the same file.
## Scope
`@icp-sdk/signer` is the relying-party client, so the skill covers the
app side only. **Deleted:** the `Wallet Side (Signer)` section,
`Signer.init` prompt registration, the ICRC-21 consent-message state
machine, and the pseudo-wallet dev setup. Consent rendering and account
custody are the wallet's job.
The model is one thing: **every write is an individual, user-approved
ICRC-49 call.** Wanting a session instead means wanting authentication,
so that is an explicit *When NOT to use* pointing at
**internet-identity**. ICRC-34 is not covered — per the maintainer, a
delegation is scoped and issued for auth purposes, not for wallet
purposes with explicit approval. That restores the boundary the old
skill drew, with the right reason: ICRC-34 exists, it just is not a
wallet mechanism.
Generic over ICRC-25 signers with OISY as the worked example. For
another web signer the transport URL is usually the only change, and
`BrowserExtensionTransport.discover()` finds extension signers you never
hardcoded — but which transport reaches a signer is a separate question
from what that signer supports, so the skill negotiates rather than
assumes. Standards covered: ICRC-25, 27, 29, 49, 94, 167.
## Structure
Transport choice is the one decision up front — `PostMessageTransport`
(ICRC-29 popup), `UrlTransport` (ICRC-167 redirect, new in signer 6),
`BrowserExtensionTransport` (ICRC-94 discovery) — then capability
negotiation, the permission and account lifecycle, and calls through
`SignerAgent`.
`IcpWallet`/`IcrcWallet` are replaced by `SignerAgent` + a ledger client
from `@icp-sdk/canisters@^4`, which needed the canisters 4.0.0 release
to be installable alongside signer 6.
## Claims corrected rather than carried over
- **"Concurrent requests return `503 BUSY`."** No such code in ICRC-25,
and the library does not serialize — `UrlFlow` deliberately batches
concurrent requests. It was an oisy extension. The old error table mixed
five vendor codes in with the standard ones.
- **Reads through `SignerAgent` "cost cycles."** They do not; the cost
is the user's approval interaction. Public data needs no wallet at all —
read a ledger balance with an ordinary `HttpAgent`, anonymous by
default.
- **The error taxonomy was wrong in both directions.**
`SignerAgentError` is not a transport failure — it fires when the wallet
*did* respond and the response failed validation, so "reconnect and
retry" was the wrong advice. And transport failures do not arrive as
`PostMessageTransportError` at all: `Signer.openChannel()` rethrows them
as `SignerError` with code `4000`, original as `cause`. Runtime-verified
against a transport that fails to establish:
```
err instanceof SignerError : true
err instanceof PostMessageTransportError : false
err.code : 4000
err.cause instanceof PostMessageTransportError: true
```
The handler narrows on `err.cause`, then falls back by **range** —
ICRC-25 owns `1xxx`/`2xxx`/`3xxx`/`4xxx` and names only a few codes
inside each, so a signer may return `3002` and the old `default: throw`
would have mishandled it. The library itself emits only `1000` and
`4000`; `4001` reaches you only if the signer returns it. An unnamed
`3xxx` reports that the action did not go through rather than returning
silently — `3001` is the only code where silence is right, because there
you know the user cancelled on purpose.
- **The redirect example returned unverified wallet output.**
`callCanister` validates only that `contentMap` and `certificate` are
present and decodable; the content-map match and the certificate check
live exclusively in `SignerAgent`. The maintainer confirmed
`SignerAgent` works over `UrlTransport`, so the example now uses it and
the raw primitive is gone from the skill — which removes the problem
rather than warning about it.
- **The `memoize()` rule was too strict.** The skill said it "is the
only place a flow may await anything that is not a signer request",
taken from the `UrlTransport` docstring. Per the maintainer the journal
cares whether a value comes back *identical*, not whether an `await`
happened, so deterministic async work needs no `memoize`. Corrected, and
flagged upstream since the docstring wording is what produced the error.
- **`getAccounts()` was indexed blindly.** ICRC-27 defines `accounts` as
a `vec` with no minimum and lets the signer prompt the user to choose
which to share, so an empty list means declined, not failed, and several
means the user should pick. `return accounts[0]` gave `undefined` on an
empty list, crashing later at `.owner`. `connect()` returns the list;
pitfall 4 covers both cases.
## Account identity — one rule
Every account-shaped value is an `IcrcAccount` (exactly what
`getAccounts()` returns and what `balance()` takes), converted at the
ledger boundary only:
| Operation | Helper |
|---|---|
| compare | `encodeIcrcAccount(a) === encodeIcrcAccount(b)` |
| persist | `encodeIcrcAccount()` / `decodeIcrcAccount()` |
| send | `from_subaccount` (sender), `toCandidAccount()` (recipient) |
Comparing encodings normalizes by construction — absent, `undefined` and
32 zero bytes all encode to the bare principal, a real subaccount does
not — which is what the reconnect check needs; comparing `owner` alone
accepts a stale selection. `SignerAgent` is the documented exception:
its `account` is a `Principal` and cannot carry a subaccount.
The subaccount is usually *absent*, because signers commonly offer only
the default one. Carrying the account whole costs nothing, so the skill
does, without implying subaccounts are the common case.
## Verified
- **Every block compiles** against `@icp-sdk/signer@6.0.0` +
`@icp-sdk/core@6.1.0` + `@icp-sdk/canisters@4.0.0`, `strict`,
`skipLibCheck: false`, two ways: **merged** (whole document as one
module — catches a block referencing anything the skill never defines)
and **isolated** (each block alone — catches a block relying on another
block's imports). The merged pass found an example reaching into another
function's scope, three blocks using top-level `await`, and one
shadowing `account`; the isolated pass found nine blocks short of their
own imports. Neither can see an unreachable branch, which is how the
error-handling bug above survived them.
- **Prose is checked against the code too, not only the code against the
library.** Twelve defects here were a claim contradicting, or looser
than, guidance elsewhere in the same file; for a skill the prose is the
payload. Every API name in prose resolves against the libraries and the
skill's own blocks (55 identifiers, 0 unresolved), and every prose
assertion about an API is enumerated for review (24) — the latter caught
the testing section crediting `localhost` as a secure context, when
`isSecureContextUrl` is applied to the *signer's* URL, not the relying
party's origin.
- **One runtime bug caught by compiling:** an early redirect example
journaled a `CryptoKey`-backed identity through `memoize()`. It compiles
and fails at runtime, because `memoize` persists via JSON. Pitfall 6 now
documents that trap generally.
- Patterns cross-checked against
[`hosting/oisy-signer-demo`](https://github.com/dfinity/examples/tree/master/hosting/oisy-signer-demo),
already on `@icp-sdk/signer` (5.3.0). Two things it taught the skill:
**reads belong on a plain `HttpAgent`**, and **a connection cannot
survive a reload** — persist the account, render read-only, reconnect
lazily on first write. That demo is a candidate for its own bump to
signer 6 / core 6 / canisters 4.
## Not covered
The skill tells the agent to call `getSupportedStandards()` rather than
asserting which standards OISY advertises — a specific wallet's
capability set cannot be verified from here, and negotiating is the
right instruction for a generic skill anyway.
<details>
<summary>Eval results — full replacement, 10 cases with
baseline</summary>
All four previous cases tested oisy specifics (`IcpWallet` vs
`IcrcWallet`, signer-side implementation) and no longer described the
skill.
```
Adversarial: reaches for the superseded oisy library WITH 3/3 | WITHOUT 0/3
Adversarial: reading a balance through SignerAgent WITH 3/3 | WITHOUT 1/3
Adversarial: establishing the wallet popup on mount WITH 3/3 | WITHOUT 1/3
Adversarial: signer 6 against a core ^5 project WITH 3/3 | WITHOUT 2/3
Adversarial: assumes every wallet can do what the app … WITH 3/3 | WITHOUT 1/3
Adversarial: getAccounts() returns a list, not an account WITH 3/3 | WITHOUT 0/3
Connection does not survive a page reload WITH 4/4 | WITHOUT 2/4
Adversarial: a blocked popup is not the error class … WITH 4/4 | WITHOUT 0/4
Adversarial: an ICRC-25 code the table does not name WITH 3/3 | WITHOUT 0/3
Adversarial: redirect flow loses a value across the nav WITH 2/2 | WITHOUT 0/2
TOTAL WITH 31/31 | WITHOUT 7/31
```
Trigger evals: **should-trigger 6/6**, **should-not-trigger 7/7** —
including three boundary cases that must *not* match: "I'm building a
wallet — how do I handle incoming ICRC-49 call requests from dapps?"
(signer implementation), "Log my CLI agent into oisy.com so it can act
as me" (→ agent-web-identity), "Add Internet Identity login to my app"
(→ internet-identity).
Latest run per case; cases were re-run as content changed, so the
baseline column is not from one sitting.
A delegation case was removed along with ICRC-34. Four were added for
behaviour that had landed uncovered: `getAccounts()` returning a list,
an ICRC-25 code outside the named set, a redirect value lost across the
navigation, and capability negotiation.
**The blocked-popup case is flaky** — 2/4, 4/4, 4/4, 3/4 across four
runs. The failures are legitimate rather than mis-scored: the model
sometimes answers narrowly, splitting `4000` by `err.cause` for the
popup case but rethrowing the rest, so a `4000` with no cause goes
unhandled. Baseline is 0/4 every time, so the delta is stable even where
the absolute is not.
Six expectations were encoding my own errors or over-reaching past their
prompt, and were corrected — I was writing them from the skill's content
rather than from what a correct answer to that exact prompt would
contain. The suite caught each one: one asserted reads "cost cycles";
one demanded `4000` route to reconnect when the skill deliberately
splits it by `err.cause`; one asked a "short answer" prompt to enumerate
all four ICRC-25 ranges.
</details>
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
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.
Moves the skills'
@icp-sdk/coreanchor from5.xto6.x. The anchor sits wherever@icp-sdk/authis, because almost every dapp authenticates — and auth has moved.Refs #364; does not close it.
wallet-integrationis still on the old stack and is rewritten in #401, which closes the issue.Why now
Every package the skills install agrees on core
^6:@icp-sdk/auth@10,@icp-sdk/signer@6,@icp-sdk/canisters@4,@dfinity/utils@5, and@icp-sdk/vetkeys@0.7(^5 || ^6). Verified by install, peers honoured:The one real bug
internet-identityPrerequisites paired an open-ended@icp-sdk/auth (>= 9.0.0)with@icp-sdk/core (>= 5.3.0). That floor admits auth 10, which peers core^6, so the advertised combination does not install:Both majors are now pinned together, with a pitfall for the mismatch. Everything else is a version bump.
Verified, not assumed
diff -rqover the wholedist/esmtype surface: no differences. The v10 guide agrees ("This package's own API is unchanged"). So the II flow needed no code changes..didexercisingvariant/record/blob/opt/principal, typechecked against core 6.1.0 understrictwithskipLibCheck: false.nns/types/governance_converters.d.ts; the rest is regenerated declarations.AssetManagerand both ledger clients are untouched.>= 0.7floor is load-bearing for a different reason: 0.5/0.6 carried core as a plain dependency, so an app on core 6 got 6.1.0 at the root and 5.4.0 nested under vetkeys. 0.7 peers it.Two claims inherited from
mainare also corrected:@icp-sdk/coredoes have stable 4.x releases (so "starts at 5.x, no 0.x or 1.x" was false — dropped, since an agent needs the reason to pin, not version history), and--legacy-peer-depsdoes not duplicate core on a peer conflict — it skips the check and installs the mismatched pair. The nested-copy wording is kept only where it is accurate, on vetkeys.Not in scope
wallet-integrationis left to #401 to avoid a conflict on the same file, and is currently self-consistent.caffeine-appandcertified-variablesare handled separately per maintainer. The false{ agent }/createActorclaim inbinding-generation.mdanddfx-migration.mdbelongs with #156, since retracting it also means changing an eval that enforces it.Supersedes #368
That PR pinned everything to
^5. Carried over: the bindgen floor, the correction that bindgen does not depend on core, and the vetkeys 0.7 bump.Eval results — added/changed cases, with baseline
The new II case puts a core-
^5project in front of an@icp-sdk/authinstall; the baseline never mentions the peer conflict and "sidesteps the issue entirely by suggesting a different, unrelated package".Regression check —
internet-identity20, the one existing case whose subject I touched: 3/4 with the change, 3/4 againstorigin/main's content (control run). Not a regression. Its failing behaviour varies run to run — the model invents a specific session default rather than saying unset falls back to II's own. Pre-existing gap, not addressed here.icp-cli15 scored 5/6 on an earlier run, failing on thecandid:/didFile:confusion of #367. It did not reproduce on the latest run, which fits that issue's non-determinism. #367 stays open.