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
icp-cli claimed that passing { agent } to bindgen's createActor "silently falls back to an anonymous identity". It doesn't. Every bindgen release (0.1.0–0.4.1) generates:
The claim appeared in pitfall 13, binding-generation.md, dfx-migration.md and eval case 17, so all four change together. They now describe what actually breaks when porting @dfinity/agent code:
Missing rootKey: the agent defaults to the mainnet root key, so calls against a local network fail verification. The ic_env cookie's key is for the network serving the page. A page calling a different network must not pass it.
new HttpAgent(): deprecated. Use { agentOptions }, or a pre-built agent from await HttpAgent.create({ identity, rootKey }). agentOptions is ignored when agent is passed.
identity: the migration example dropped it, which silently made authenticated calls anonymous. It is now carried over.
host: the old pitfall 13 recommended { host, rootKey }, which contradicted the reference. Pitfall 13 now says host stays unset in browser code. A new "Agent options" section in binding-generation.md adds the cases where it must be set: Node scripts and tests (api_url and root_key from icp network status --json), and pages that call a different network.
Migration checklist item 14 is aligned with this. { agentOptions } stays the recommended form.
Pitfall 13 is kept short on purpose. A full decision table (as a pitfall 23), and later a detailed pitfall 13, both regressed eval case 28: agents built their answers from the SKILL.md text instead of binding-generation.md. A bisect on main confirmed that the reference changes alone score 6/6. The decision table moves to the canister-calls skill (#406).
Verified on a local network (icp-cli 1.5.0, bindgen 0.4.1, core 6.1.0) with a whoami canister:
Node, host + rootKey from icp network status --json
works
Node, no host
ProtocolError (request goes to https://icp-api.io)
mainnet ledger query, no rootKey / local rootKey
ICP / TrustError: Certificate verification error
Refs #156 (the one remaining defect from it), #406.
Eval results
Changed or new cases, with baseline:
Case 17 (changed), "Adversarial: porting @dfinity/agent actor setup to bindgen": now also checks that the corrected setup keeps identity. WITH 5/5 in all three runs | WITHOUT 3/5 in both valid baseline runs (one more baseline run timed out). Without the skill, the model does not use safeGetCanisterEnv() for rootKey and keeps the hardcoded host. In one earlier baseline run it also asserted the false anonymous-identity fallback on its own.
Case 30 (new), "Adversarial: agent host and root key in a Node test against the local network": WITH 3/3 in 3 of 3 runs on the final text | WITHOUT 1/3 in both valid baseline runs (later baseline runs timed out).
Case 28 (item 3 split): 3a is "custom domain → https://icp-api.io", 3b is "the /api/v2 reason". main scored 18/18 full passes across two days. This branch, before the last edit (the pre-built agent's options): 3a 12/12 with no factual errors, and 7/12 full passes, where every miss only omits the reason (3b). On the final text: 3a 8/9, and 4/9 full passes. One run left rootKey unset and never named icp-api.io; the other misses only omit the reason. Earlier drafts with more detail in SKILL.md scored 0–3/6 and made factual errors.
Regression checks (touched content), with skill:
Case 3 "Migrate from dfx": 7/7 on the final text.
Case 6 "Frontend TypeScript bindings": 6/6 on the final text.
The generated createActor uses a passed agent as-is (bindgen 0.1.0-0.4.1).
Re-ground pitfall 13, binding-generation, dfx-migration and eval case 17
on what actually breaks when porting: a missing rootKey, the deprecated
HttpAgent constructor and a hardcoded host.
Pitfall 23 states the host rule with its reason and a browser snippet;
the full decision table moves to binding-generation.md. Case 28 now
checks the custom-domain fallback instead of the whole mechanism.
The table in SKILL.md stopped agents from reading binding-generation.md
and regressed eval case 28. It moves to the canister-calls skill (#406);
the short Node and cross-network notes stay in the reference.
Clarify root key selection for cross-network calls
skills/icp-cli/SKILL.md:117
This pitfall also presents the cookie root key as universal, but it is only the correct key when the actor targets the network serving the page. A local dev page calling mainnet must omit the local IC_ROOT_KEY (or use the target key), as described in binding-generation.md; qualify this sentence so the pitfall does not cause that cross-network setup to fail.
Limit cookie rootKey guidance to serving-network calls
The blanket “Either way, set rootKey from the cookie” is not correct for the cross-network browser case documented below: when a page served by a local network calls mainnet, passing the page’s IC_ROOT_KEY makes certificate verification use the local key. Qualify this to calls against the serving network and point to the exception on line 73.
Restrict ic_env rootKey migration to serving-network calls
skills/icp-cli/references/dfx-migration.md:41
“Always set rootKey from the ic_env cookie” is too broad here. A frontend served by a local network but targeting mainnet must not pass that local cookie key; otherwise calls fail certificate verification. Limit this migration rule to the serving network and refer to the cross-network exception in binding-generation.md.
rootKey scoped to the serving network (pitfall 13, binding-generation.md, dfx-migration.md): the cookie key is now only prescribed when calling the network that serves the page, and a page calling a different network must not pass it. Verified against mainnet: an ICP ledger icrc1_symbol query returns ICP without rootKey, and fails with TrustError: Certificate verification error when given a local root key.
Migration example keeps identity: the "after" snippet now passes agentOptions: { identity, rootKey }. Dropping it silently turned authenticated calls into anonymous ones.
While fixing these I found why eval case 28 kept regressing. I bisected on main with 6 runs each: our binding-generation.md changes alone score 6/6, and the long pitfall 13 alone scored 4/6 with factual errors. Pitfall 13 is short again and points to the reference, and the actor details moved into a separate "Agent options" section of binding-generation.md.
This tells readers how to await a pre-built agent but not that its own root key must be configured. When { agent } is supplied, agentOptions.rootKey is ignored; following this guidance with await HttpAgent.create({ identity }) against a local network still uses the mainnet key and produces the same certificate verification failure the new section is meant to prevent. Say to pass the target network's rootKey (and host when applicable) to HttpAgent.create, or recommend { agentOptions } for this case.
This issue also appears on line 101 of the same file.
Re the latest Copilot review: valid, fixed in 2a0337a. The pre-built { agent } bullet now says to build it with await HttpAgent.create({ identity, rootKey }) (plus host where required), because agentOptions is ignored when agent is passed. Case 17: WITH 5/5 | WITHOUT 3/5. Broader agent-setup guidance (the per-case decision table, including pre-built agents) is left to #406.
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.
icp-cliclaimed that passing{ agent }to bindgen'screateActor"silently falls back to an anonymous identity". It doesn't. Every bindgen release (0.1.0–0.4.1) generates:The claim appeared in pitfall 13,
binding-generation.md,dfx-migration.mdand eval case 17, so all four change together. They now describe what actually breaks when porting@dfinity/agentcode:rootKey: the agent defaults to the mainnet root key, so calls against a local network fail verification. Theic_envcookie's key is for the network serving the page. A page calling a different network must not pass it.new HttpAgent(): deprecated. Use{ agentOptions }, or a pre-built agent fromawait HttpAgent.create({ identity, rootKey }).agentOptionsis ignored whenagentis passed.identity: the migration example dropped it, which silently made authenticated calls anonymous. It is now carried over.host: the old pitfall 13 recommended{ host, rootKey }, which contradicted the reference. Pitfall 13 now sayshoststays unset in browser code. A new "Agent options" section inbinding-generation.mdadds the cases where it must be set: Node scripts and tests (api_urlandroot_keyfromicp network status --json), and pages that call a different network.Migration checklist item 14 is aligned with this.
{ agentOptions }stays the recommended form.Pitfall 13 is kept short on purpose. A full decision table (as a pitfall 23), and later a detailed pitfall 13, both regressed eval case 28: agents built their answers from the
SKILL.mdtext instead ofbinding-generation.md. A bisect onmainconfirmed that the reference changes alone score 6/6. The decision table moves to thecanister-callsskill (#406).Verified on a local network (icp-cli 1.5.0, bindgen 0.4.1, core 6.1.0) with a
whoamicanister:{ agent: await HttpAgent.create({ identity, rootKey }) }{ agentOptions: { identity, rootKey } }rootKeyTrustError: Certificate verification error: "Invalid signature"{ agent: HttpAgent.create(...) }(not awaited)TypeError: agent.query is not a functionagentandagentOptionshost+rootKeyfromicp network status --jsonhostProtocolError(request goes tohttps://icp-api.io)rootKey/ localrootKeyICP/TrustError: Certificate verification errorRefs #156 (the one remaining defect from it), #406.
Eval results
Changed or new cases, with baseline:
identity. WITH 5/5 in all three runs | WITHOUT 3/5 in both valid baseline runs (one more baseline run timed out). Without the skill, the model does not usesafeGetCanisterEnv()forrootKeyand keeps the hardcodedhost. In one earlier baseline run it also asserted the false anonymous-identity fallback on its own.https://icp-api.io", 3b is "the/api/v2reason".mainscored 18/18 full passes across two days. This branch, before the last edit (the pre-built agent's options): 3a 12/12 with no factual errors, and 7/12 full passes, where every miss only omits the reason (3b). On the final text: 3a 8/9, and 4/9 full passes. One run leftrootKeyunset and never namedicp-api.io; the other misses only omit the reason. Earlier drafts with more detail inSKILL.mdscored 0–3/6 and made factual errors.Regression checks (touched content), with skill: