Skip to content

refactor: improve wallet-integration skill for agent consumption#67

Open
marc0olo wants to merge 5 commits intomainfrom
refactor/wallet-integration-agent-focus
Open

refactor: improve wallet-integration skill for agent consumption#67
marc0olo wants to merge 5 commits intomainfrom
refactor/wallet-integration-agent-focus

Conversation

@marc0olo
Copy link
Member

@marc0olo marc0olo commented Mar 4, 2026

Summary

Refactors the wallet-integration skill (merged via #16) to be more efficient for agent consumption.

  • Rename headings: H1 now matches skill name; "Mistakes That Break Your Build" → "Pitfalls" (conventional, scannable)
  • Fix pitfall #5 accuracy: requestPermissionsNotGranted() is a UX optimization (batch permissions upfront), not a requirement — the signer prompts on demand via ask_on_use default. Verified against library source and e2e tests.
  • Self-contained code blocks: IcpWallet/IcrcWallet examples now include connect() + accounts() so agents get copy-paste correct code without needing to reference other sections
  • Add error class imports: RelyingPartyResponseError and RelyingPartyDisconnectedError added to the import map — these are used in the error handling block but were missing (not yet exported by the library; import path is a draft pending dfinity/oisy-wallet-signer update)
  • Remove low-value sections: "Build Your Project" (generic TS commands), "Verify It Works" manual QA checklist
  • Add "Expected Behavior": Condensed section with return types and behavioral facts (permission idempotency, account shape, block index types) not stated elsewhere in the skill

- Rename H1 to match skill name (Wallet Integration)
- Rename "Mistakes That Break Your Build" to "Pitfalls"
- Fix pitfall #5: clarify that requestPermissionsNotGranted() is a UX
  optimization, not a requirement (verified against library source)
- Make IcpWallet/IcrcWallet code blocks self-contained (add connect +
  accounts so agents get copy-paste correct code)
- Add error class imports to import map (RelyingPartyResponseError,
  RelyingPartyDisconnectedError)
- Remove "Build Your Project" section (generic TS commands)
- Replace "Verify It Works" with condensed "Expected Behavior" section
  focused on return types and behavioral facts not stated elsewhere
@marc0olo marc0olo requested a review from AntonioVentilii March 4, 2026 16:30
@marc0olo marc0olo marked this pull request as ready for review March 4, 2026 20:42
marc0olo and others added 3 commits March 5, 2026 16:05
RelyingPartyResponseError and RelyingPartyDisconnectedError are exported
from the main entry point (as of v4.1.2), not from the relying-party
subpath.
Same package path — no reason to split into two import statements.
Copy link
Collaborator

@AntonioVentilii AntonioVentilii left a comment

Choose a reason for hiding this comment

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

LGTM tks!

@raymondk raymondk removed the request for review from peterpeterparker March 5, 2026 17:41
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.

4 participants