Overhaul ERC-8004 agent integration#584
Conversation
Fix critical bug: wallet binding deadline from 3600s to 300s (contract enforces max 5 minutes). Add missing ABI entries (getAgentWallet, unsetAgentWallet, setAgentURI, setMetadata, ownerOf, balanceOf, tokenOfOwnerByIndex). Detect existing agents on page load via agentIdByWallet + balanceOf, distinguish owner vs agent wallet roles. New AgentManage component for existing agents: view info, update URI, change/unset agent wallet. Dashboard now handles both owner and agent wallet connections. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The critical deadline fix is in place, but two owner/external-agent flows are still incorrect. They both break scenarios that issue #583 explicitly calls out.
Findings
- [high] Owner dashboards still lose storylines after
unsetAgentWallet()or any transfer-reset to the zero address.- File:
src/components/AgentDashboard.tsx:55 - Suggestion: treat
0x0000000000000000000000000000000000000000as "no bound wallet" and fall back to the owner wallet for storyline lookup so owner-connected dashboards keep working.
- File:
- [medium] Existing agents registered from other ERC-8004 apps will not show their current metadata in the new management UI.
- File:
src/components/AgentManage.tsx:93 - Suggestion: resolve the
agentURIby scheme (data:,https://,ipfs://, etc.) and parse the fetched payload, instead of callingJSON.parse()directly on the URI string.
- File:
Decision
Request changes. The PR is close, but these two paths still violate the issue requirements for owner dashboards and cross-app existing-agent management.
Dashboard: fall back to owner address when getAgentWallet returns zero address (after unset/transfer), instead of querying 0x000...000. AgentManage: handle https://, ipfs://, and data: URIs from agents registered via other apps, not just raw JSON stored in the URI field. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up fixes address the two blocking issues from my previous review. Owner dashboards now fall back correctly when the bound wallet is unset, and the management UI can resolve non-JSON ERC-8004 URIs from other apps.
Findings
- No blocking issues found in the updated changes.
Decision
Approve. The PR now satisfies issue #583's critical deadline fix, existing-agent detection, owner-vs-agent wallet handling, and management UI requirements.
Summary
Fixes #583
getAgentWallet,unsetAgentWallet,setAgentURI,setMetadata,getMetadata,ownerOf,balanceOf,tokenOfOwnerByIndex, plusURIUpdatedandMetadataSetevents/agentspage load, checks bothagentIdByWallet()(bound agent wallet) andbalanceOf()+tokenOfOwnerByIndex()(NFT owner) to detect registered agentsAgentManagecomponent: View agent info, update URI metadata, change/unset agent wallet — all with correct 5-minute EIP-712 deadlinesTest plan
tsc --noEmitclean)🤖 Generated with Claude Code