Skip to content

fix(internet-identity): rewrite skill to fix agent hallucinations#101

Open
marc0olo wants to merge 1 commit intomainfrom
fix/internet-identity-skill-improvements
Open

fix(internet-identity): rewrite skill to fix agent hallucinations#101
marc0olo wants to merge 1 commit intomainfrom
fix/internet-identity-skill-improvements

Conversation

@marc0olo
Copy link
Member

Summary

Rewrites the internet-identity skill to eliminate patterns that cause agents to generate broken code. Adds the first evaluation suite for this skill.

Closes #97. Supersedes #91.
Mentions #95, #96, #99, #100.

Why wrap await in functions instead of build.target: "esnext"?

The original #95 proposed adding build.target: "esnext" to Vite config. As analyzed in #95 (comment), this is the wrong fix:

  • The top-level await comes from skill code examples, not from @icp-sdk/bindgen output (verified against generated snapshots)
  • esnext disables all transpilation — overbroad for a scalpel problem
  • Wrapping in async function init() works with any Vite target and is how real apps structure initialization

Why hardcode the II canister ID?

Internet Identity has a well-known canister ID (rdmx6-jaaaa-aaaaa-aaadq-cai) that is the same on mainnet and local replicas. The previous code had a wrong fallback (be2us-64aaa-aaaaa-qaabq-cai) and a dynamic ic_env cookie lookup that was unnecessary for II.

Why no fetch: window.fetch.bind(window)?

PR #91 added this workaround. It's unnecessary — icp-js-core#1316 fixes this at the SDK level by binding window.fetch inside the agent constructor.

Why use rootKey from ic_env instead of shouldFetchRootKey?

The ic_env cookie (set by the asset canister or Vite dev server) already contains the root key as IC_ROOT_KEY. Passing it via the rootKey option to HttpAgent.create() works in both local and production without environment branching. This aligns with the pattern in icp-cli/references/binding-generation.md. fetchRootKey() is a security risk on mainnet.

Why remove backend Motoko/Rust examples?

The backend code (anonymous principal rejection, role guards, caller binding) is not II-specific — identical patterns apply regardless of auth method. The canister-security skill already has comprehensive coverage. Removing ~170 lines of duplicated content makes the skill leaner and avoids conflicting advice.

Changes

Skill (internet-identity/SKILL.md):

  • Wrap all await in async functions (no top-level await)
  • Hardcode II canister ID rdmx6-jaaaa-aaaaa-aaadq-cai (replaces wrong fallback + dynamic lookup)
  • Use rootKey from ic_env cookie instead of shouldFetchRootKey
  • Add npm versions to prerequisites (@icp-sdk/auth >= 5.0.0, @icp-sdk/core >= 5.0.0)
  • Remove backend Motoko/Rust examples (→ cross-reference to canister-security skill)
  • Remove Deploy & Test / Verify It Works sections (covered by icp-cli skill)
  • Trim icp.yaml section to just ii: true network config
  • Add new pitfalls: anonymous principal (2vxsx-fae), fetchRootKey warning
  • Renumber pitfalls (was 1,3,4,5,6,7,8,9 → now 1-6)

Evaluations (evaluations/internet-identity.json): New file with 6 output evals + 12 trigger evals

  • No top-level await in frontend code
  • Local II URL (hardcoded canister ID)
  • icp.yaml configuration (ii: true)
  • Authenticated actor creation (ic_env cookie pattern)
  • Debugging anonymous principal after login
  • Adversarial: build target suggestion (tests that esnext is NOT recommended)

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.

Fix top-level await in skill code examples

1 participant